Change storage of PKI entries from colons to hyphens

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/cert_util_test.go

@@ -0,0 +1,86 @@
+package pki
+
+import (
+	"testing"
+
+	"github.com/hashicorp/vault/logical"
+)
+
+func TestFetchCertBySerial(t *testing.T) {
+	storage := &logical.InmemStorage{}
+
+	cases := map[string]struct {
+		Req        *logical.Request
+		StorageKey string
+	}{
+		"cert, valid colon": {
+			&logical.Request{
+				Operation: logical.ReadOperation,
+				Path:      "certs/10:e6:fc:62:b7:41:8a:d5:00:5e:45:b6",
+				Storage:   storage,
+			},
+			"10:e6:fc:62:b7:41:8a:d5:00:5e:45:b6",
+		},
+		"cert, revoked colon": {
+			&logical.Request{
+				Operation: logical.ReadOperation,
+				Path:      "revoked/10:e6:fc:62:b7:41:8a:d5:00:5e:45:b6",
+				Storage:   storage,
+			},
+			"10:e6:fc:62:b7:41:8a:d5:00:5e:45:b6",
+		},
+		"cert, valid hyphen": {
+			&logical.Request{
+				Operation: logical.ReadOperation,
+				Path:      "certs/10:e6:fc:62:b7:41:8a:d5:00:5e:45:b6",
+				Storage:   storage,
+			},
+			"10-e6-fc-62-b7-41-8a-d5-00-5e-45-b6",
+		},
+		"cert, revoked hyphen": {
+			&logical.Request{
+				Operation: logical.ReadOperation,
+				Path:      "revoked/10:e6:fc:62:b7:41:8a:d5:00:5e:45:b6",
+				Storage:   storage,
+			},
+			"10-e6-fc-62-b7-41-8a-d5-00-5e-45-b6",
+		},
+		"cert, ca": {
+			&logical.Request{
+				Operation: logical.ReadOperation,
+				Path:      "ca",
+				Storage:   storage,
+			},
+			"",
+		},
+		"cert, crl": {
+			&logical.Request{
+				Operation: logical.ReadOperation,
+				Path:      "crl",
+				Storage:   storage,
+			},
+			"",
+		},
+	}
+
+	for name, tc := range cases {
+		// Put pseudo-cert in inmem storage
+		err := storage.Put(&logical.StorageEntry{
+			Key:   tc.Req.Path,
+			Value: []byte("some data"),
+		})
+		if err != nil {
+			t.Fatalf("error writing to storage on %s: %s", name, err)
+		}
+
+		certEntry, err := fetchCertBySerial(tc.Req, tc.Req.Path, tc.StorageKey)
+		if err != nil {
+			t.Fatalf("fetchBySerial error on %s: %s", name, err)
+		}
+
+		// Check for non-nil on valid/revoked certs
+		if certEntry == nil && tc.Req.Path != "ca" && tc.Req.Path != "crl" {
+			t.Fatalf("fetchBySerial returned nil on %s", name)
+		}
+	}
+}

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"
@@ -65,7 +66,7 @@ func revokeCert(b *backend, req *logical.Request, serial string, fromLease bool)
 
 		cert, err := x509.ParseCertificate(certEntry.Value)
 		if err != nil {
-			return nil, fmt.Errorf("Error parsing certificate")
+			return nil, fmt.Errorf("Error parsing certificate: %s", err)
 		}
 		if cert == nil {
 			return nil, fmt.Errorf("Got a nil certificate")
@@ -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/crl_util_test.go

@@ -0,0 +1,126 @@
+package pki
+
+import (
+	"encoding/pem"
+	"io/ioutil"
+	"testing"
+	"time"
+
+	"github.com/hashicorp/vault/helper/certutil"
+	"github.com/hashicorp/vault/helper/jsonutil"
+	"github.com/hashicorp/vault/logical"
+)
+
+func TestRevokeCert(t *testing.T) {
+	storage := &logical.InmemStorage{}
+	config := logical.TestBackendConfig()
+	config.StorageView = storage
+
+	b := Backend()
+	_, err := b.Setup(config)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Place CA cert in storage
+	rootCAKeyPEM, err := ioutil.ReadFile("test-fixtures/root/rootcakey.pem")
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	rootCACertPEM, err := ioutil.ReadFile("test-fixtures/root/rootcacert.pem")
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	cb := &certutil.CertBundle{}
+	cb.PrivateKey = string(rootCAKeyPEM)
+	cb.PrivateKeyType = certutil.RSAPrivateKey
+	cb.Certificate = string(rootCACertPEM)
+
+	bundleEntry, err := logical.StorageEntryJSON("config/ca_bundle", cb)
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = storage.Put(bundleEntry)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	certValue, err := ioutil.ReadFile("test-fixtures/keys/cert.pem")
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	block, _ := pem.Decode(certValue)
+	if block == nil {
+		t.Fatal("failed to decode PEM cert into DER")
+	}
+	certDER := block.Bytes
+
+	var revInfo revocationInfo
+	currTime := time.Now()
+	revInfo.CertificateBytes = certDER
+	revInfo.RevocationTime = currTime.Unix()
+	revInfo.RevocationTimeUTC = currTime.UTC()
+	encodedCertDER, err := jsonutil.EncodeJSON(revInfo)
+	if err != nil {
+		t.Fatalf("error encoding pseudo cert value: %s", err)
+	}
+
+	cases := map[string]struct {
+		Req          *logical.Request
+		StorageKey   string
+		StorageValue []byte
+	}{
+		"cert, valid colon": {
+			&logical.Request{
+				Operation: logical.UpdateOperation,
+				Path:      "certs/7f:e8:e1:29:31:41:9e:a4:ac:df:82:08:d1:64:b5:2f:84:2c:6d:b0",
+				Storage:   storage,
+			},
+			"7f:e8:e1:29:31:41:9e:a4:ac:df:82:08:d1:64:b5:2f:84:2c:6d:b0",
+			certDER,
+		},
+		"cert, revoked colon": {
+			&logical.Request{
+				Operation: logical.UpdateOperation,
+				Path:      "revoked/7f:e8:e1:29:31:41:9e:a4:ac:df:82:08:d1:64:b5:2f:84:2c:6d:b0",
+				Storage:   storage,
+			},
+			"7f:e8:e1:29:31:41:9e:a4:ac:df:82:08:d1:64:b5:2f:84:2c:6d:b0",
+			encodedCertDER,
+		},
+		"cert, valid hyphen": {
+			&logical.Request{
+				Operation: logical.UpdateOperation,
+				Path:      "certs/7f:e8:e1:29:31:41:9e:a4:ac:df:82:08:d1:64:b5:2f:84:2c:6d:b0",
+				Storage:   storage,
+			},
+			"7f-e8-e1-29-31-41-9e-a4-ac-df-82-08-d1-64-b5-2f-84-2c-6d-b0",
+			certDER,
+		},
+		"cert, revoked hyphen": {
+			&logical.Request{
+				Operation: logical.UpdateOperation,
+				Path:      "revoked/7f:e8:e1:29:31:41:9e:a4:ac:df:82:08:d1:64:b5:2f:84:2c:6d:b0",
+				Storage:   storage,
+			},
+			"7f-e8-e1-29-31-41-9e-a4-ac-df-82-08-d1-64-b5-2f-84-2c-6d-b0",
+			encodedCertDER,
+		},
+	}
+
+	for name, tc := range cases {
+		// Put pseudo-cert in inmem storage
+		err := storage.Put(&logical.StorageEntry{
+			Key:   tc.Req.Path,
+			Value: tc.StorageValue,
+		})
+		if err != nil {
+			t.Fatalf("error writing to storage on %s: %s", name, err)
+		}
+
+		resp, err := revokeCert(b, tc.Req, tc.StorageKey, false)
+		if err != nil || (resp != nil && resp.IsError()) {
+			t.Fatalf("bad: err: %v resp: %#v", err, resp)
+		}
+	}
+}

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 {