Change storage of entries from colons to hyphens and add a

lookup/migration path

Still TODO: tests on migration path

Fixes #2552

builtin/logical/pki/cert_util.go

@@ -185,32 +185,68 @@ func fetchCAInfo(req *logical.Request) (*caInfoBundle, error) {
 // separate pathing for CA, CRL, and revoked certificates.
 func fetchCertBySerial(req *logical.Request, prefix, serial string) (*logical.StorageEntry, error) {
 	var path string
+	var err error
+	var certEntry *logical.StorageEntry
 
 	switch {
 	// Revoked goes first as otherwise ca/crl get hardcoded paths which fail if
 	// we actually want revocation info
 	case strings.HasPrefix(prefix, "revoked/"):
-		path = "revoked/" + strings.Replace(strings.ToLower(serial), "-", ":", -1)
+		path = "revoked/" + strings.Replace(strings.ToLower(serial), ":", "-", -1)
 	case serial == "ca":
 		path = "ca"
 	case serial == "crl":
 		path = "crl"
+	default:
+		path = "certs/" + strings.Replace(strings.ToLower(serial), ":", "-", -1)
+	}
+
+	certEntry, err = req.Storage.Get(path)
+	if err != nil {
+		return nil, errutil.InternalError{Err: fmt.Sprintf("error fetching certificate %s: %s", serial, err)}
+	}
+	if certEntry != nil {
+		if certEntry.Value == nil || len(certEntry.Value) == 0 {
+			return nil, errutil.InternalError{Err: fmt.Sprintf("returned certificate bytes for serial %s were empty", serial)}
+		}
+		return certEntry, nil
+	}
+
+	// No point checking these, no old/new style colons/hyphens
+	if path == "ca" || path == "crl" {
+		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/" + strings.Replace(strings.ToLower(serial), "-", ":", -1)
 	default:
 		path = "certs/" + strings.Replace(strings.ToLower(serial), "-", ":", -1)
 	}
 
-	certEntry, err := req.Storage.Get(path)
+	certEntry, err = req.Storage.Get(path)
 	if err != nil {
 		return nil, errutil.InternalError{Err: fmt.Sprintf("error fetching certificate %s: %s", serial, err)}
 	}
 	if certEntry == nil {
 		return nil, nil
 	}
-
 	if certEntry.Value == nil || len(certEntry.Value) == 0 {
 		return nil, errutil.InternalError{Err: fmt.Sprintf("returned certificate bytes for serial %s were empty", serial)}
 	}
 
+	certEntry.Key = desiredPath
+	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 {
+		return nil, errutil.InternalError{Err: fmt.Sprintf("error deleting certificate with serial %s from old location", serial)}
+	}
+
 	return certEntry, nil
 }
 

builtin/logical/pki/crl_util.go

@@ -5,6 +5,7 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/hashicorp/vault/helper/errutil"
@@ -86,7 +87,7 @@ func revokeCert(b *backend, req *logical.Request, serial string, fromLease bool)
 		revInfo.RevocationTime = currTime.Unix()
 		revInfo.RevocationTimeUTC = currTime.UTC()
 
-		revEntry, err = logical.StorageEntryJSON("revoked/"+serial, revInfo)
+		revEntry, err = logical.StorageEntryJSON("revoked/"+strings.ToLower(strings.Replace(serial, ":", "-", -1)), revInfo)
 		if err != nil {
 			return nil, fmt.Errorf("Error creating revocation entry")
 		}

builtin/logical/pki/path_intermediate.go

@@ -3,6 +3,7 @@ package pki
 import (
 	"encoding/base64"
 	"fmt"
+	"strings"
 
 	"github.com/hashicorp/vault/helper/certutil"
 	"github.com/hashicorp/vault/helper/errutil"
@@ -196,7 +197,7 @@ func (b *backend) pathSetSignedIntermediate(
 		return nil, err
 	}
 
-	entry.Key = "certs/" + cb.SerialNumber
+	entry.Key = "certs/" + strings.ToLower(strings.Replace(cb.SerialNumber, ":", "-", -1))
 	entry.Value = inputBundle.CertificateBytes
 	err = req.Storage.Put(entry)
 	if err != nil {

builtin/logical/pki/path_issue_sign.go

@@ -3,6 +3,7 @@ package pki
 import (
 	"encoding/base64"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/hashicorp/vault/helper/certutil"
@@ -241,7 +242,7 @@ func (b *backend) pathIssueSignCert(
 	}
 
 	err = req.Storage.Put(&logical.StorageEntry{
-		Key:   "certs/" + cb.SerialNumber,
+		Key:   "certs/" + strings.ToLower(strings.Replace(cb.SerialNumber, ":", "-", -1)),
 		Value: parsedBundle.CertificateBytes,
 	})
 	if err != nil {

builtin/logical/pki/path_root.go

@@ -3,6 +3,7 @@ package pki
 import (
 	"encoding/base64"
 	"fmt"
+	"strings"
 
 	"github.com/hashicorp/vault/helper/errutil"
 	"github.com/hashicorp/vault/logical"
@@ -145,7 +146,7 @@ func (b *backend) pathCAGenerateRoot(
 	// Also store it as just the certificate identified by serial number, so it
 	// can be revoked
 	err = req.Storage.Put(&logical.StorageEntry{
-		Key:   "certs/" + cb.SerialNumber,
+		Key:   "certs/" + strings.ToLower(strings.Replace(cb.SerialNumber, ":", "-", -1)),
 		Value: parsedBundle.CertificateBytes,
 	})
 	if err != nil {
@@ -277,7 +278,7 @@ func (b *backend) pathCASignIntermediate(
 	}
 
 	err = req.Storage.Put(&logical.StorageEntry{
-		Key:   "certs/" + cb.SerialNumber,
+		Key:   "certs/" + strings.ToLower(strings.Replace(cb.SerialNumber, ":", "-", -1)),
 		Value: parsedBundle.CertificateBytes,
 	})
 	if err != nil {

builtin/logical/pki/secret_certs.go

@@ -2,7 +2,6 @@ package pki
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/hashicorp/vault/logical"
 	"github.com/hashicorp/vault/logical/framework"
@@ -46,10 +45,8 @@ func (b *backend) secretCredsRevoke(
 		return nil, fmt.Errorf("could not find serial in internal secret data")
 	}
 
-	serial := strings.Replace(strings.ToLower(serialInt.(string)), "-", ":", -1)
-
 	b.revokeStorageLock.Lock()
 	defer b.revokeStorageLock.Unlock()
 
-	return revokeCert(b, req, serial, true)
+	return revokeCert(b, req, serialInt.(string), true)
 }