Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change storage of PKI entries from colons to hyphens #2575

Merged
merged 11 commits into from
May 3, 2017
Merged

Change storage of PKI entries from colons to hyphens #2575

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
dba2de5
Change storage of entries from colons to hyphens and add a
jefferai Apr 6, 2017
c269fe1
Tests for cert and crl util
calvn Apr 26, 2017
4bf51ca
Fix crl_util test
calvn Apr 26, 2017
ced4c88
Add remaining tests
calvn Apr 26, 2017
a5ddaab
Rename tests, use HandleRequest() for existing paths
calvn Apr 27, 2017
7fdf4ac
Verify update operation was performed on revokeCert
calvn Apr 27, 2017
38a01b8
Refactor cert_util_test
calvn Apr 27, 2017
74965a8
Add test for ca and crl case
calvn Apr 28, 2017
8c03765
Use variables for string replacements on cert_util
calvn May 2, 2017
96bcd50
Include and use normalizeSerial func
calvn May 3, 2017
29e5ce6
Minor readability enhancements for migration path from old to new
May 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 8 additions & 16 deletions builtin/logical/pki/cert_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func fetchCAInfo(req *logical.Request) (*caInfoBundle, error) {
// Allows fetching certificates from the backend; it handles the slightly
// separate pathing for CA, CRL, and revoked certificates.
func fetchCertBySerial(req *logical.Request, prefix, serial string) (*logical.StorageEntry, error) {
var path string
var path, legacyPath string
var err error
var certEntry *logical.StorageEntry

Expand All @@ -195,12 +195,14 @@ func fetchCertBySerial(req *logical.Request, prefix, serial string) (*logical.St
// Revoked goes first as otherwise ca/crl get hardcoded paths which fail if
// we actually want revocation info
case strings.HasPrefix(prefix, "revoked/"):
legacyPath = "revoked/" + colonSerial
path = "revoked/" + hyphenSerial
case serial == "ca":
path = "ca"
case serial == "crl":
path = "crl"
default:
legacyPath = "certs/" + colonSerial
path = "certs/" + hyphenSerial
}

Expand All @@ -216,22 +218,12 @@ func fetchCertBySerial(req *logical.Request, prefix, serial string) (*logical.St
}

// No point checking these, no old/new style colons/hyphens
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this comment makes sense any more. We should keep the check with path for clarity, or update the comment to say why return if legacyPath is empty.

if path == "ca" || path == "crl" {
if legacyPath == "" {
return nil, nil
}

// Save the desired path
desiredPath := path

// If we get here we need to check for old-style paths using colons
switch {
case strings.HasPrefix(prefix, "revoked/"):
path = "revoked/" + colonSerial
default:
path = "certs/" + colonSerial
}

certEntry, err = req.Storage.Get(path)
// Retrieve the old-style path
certEntry, err = req.Storage.Get(legacyPath)
if err != nil {
return nil, errutil.InternalError{Err: fmt.Sprintf("error fetching certificate %s: %s", serial, err)}
}
Expand All @@ -243,11 +235,11 @@ func fetchCertBySerial(req *logical.Request, prefix, serial string) (*logical.St
}

// Update old-style paths to new-style paths
certEntry.Key = desiredPath
certEntry.Key = path
if err = req.Storage.Put(certEntry); err != nil {
return nil, errutil.InternalError{Err: fmt.Sprintf("error saving certificate with serial %s to new location", serial)}
}
if err = req.Storage.Delete(path); err != nil {
if err = req.Storage.Delete(legacyPath); err != nil {
return nil, errutil.InternalError{Err: fmt.Sprintf("error deleting certificate with serial %s from old location", serial)}
}

Expand Down