pki: add subject key identifier to read key response (hashicorp#20642)

* pki: add subject key identifier to read key response This will be helpful for the Terraform Vault Provider to detect migration of pre-1.11 exported keys (from CA generation) into post-1.11 Vault. * add changelog * Update builtin/logical/pki/path_fetch_keys.go Co-authored-by: Alexander Scheel <alex.scheel@hashicorp.com> * check for managed key first * Validate the SKID matches on root CAs Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Validate SKID matches on int CAs Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Fix formatting of tests Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> --------- Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> Co-authored-by: Alexander Scheel <alex.scheel@hashicorp.com>
fopina · May 18, 2023 · 6e6ca07 · 6e6ca07
1 parent ef3db02
commit 6e6ca07
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 5 deletions.
diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go
@@ -2400,6 +2400,14 @@ func TestBackend_Root_Idempotency(t *testing.T) {
 	require.NotNil(t, resp, "expected ca info")
 	keyId1 := resp.Data["key_id"]
 	issuerId1 := resp.Data["issuer_id"]
+	cert := parseCert(t, resp.Data["certificate"].(string))
+	certSkid := certutil.GetHexFormatted(cert.SubjectKeyId, ":")
+
+	//  -> Validate the SKID matches between the root cert and the key
+	resp, err = CBRead(b, s, "key/"+keyId1.(keyID).String())
+	require.NoError(t, err)
+	require.NotNil(t, resp, "expected a response")
+	require.Equal(t, resp.Data["subject_key_id"], certSkid)
 
 	resp, err = CBRead(b, s, "cert/ca_chain")
 	require.NoError(t, err, "error reading ca_chain: %v", err)
@@ -2414,6 +2422,14 @@ func TestBackend_Root_Idempotency(t *testing.T) {
 	require.NotNil(t, resp, "expected ca info")
 	keyId2 := resp.Data["key_id"]
 	issuerId2 := resp.Data["issuer_id"]
+	cert = parseCert(t, resp.Data["certificate"].(string))
+	certSkid = certutil.GetHexFormatted(cert.SubjectKeyId, ":")
+
+	//  -> Validate the SKID matches between the root cert and the key
+	resp, err = CBRead(b, s, "key/"+keyId2.(keyID).String())
+	require.NoError(t, err)
+	require.NotNil(t, resp, "expected a response")
+	require.Equal(t, resp.Data["subject_key_id"], certSkid)
 
 	// Make sure that we actually generated different issuer and key values
 	require.NotEqual(t, keyId1, keyId2)
@@ -2543,13 +2559,19 @@ func TestBackend_SignIntermediate_AllowedPastCA(t *testing.T) {
 		"common_name": "myint.com",
 	})
 	schema.ValidateResponse(t, schema.GetResponseSchema(t, b_root.Route("intermediate/generate/internal"), logical.UpdateOperation), resp, true)
+	require.Contains(t, resp.Data, "key_id")
+	intKeyId := resp.Data["key_id"].(keyID)
+	csr := resp.Data["csr"]
+
+	resp, err = CBRead(b_int, s_int, "key/"+intKeyId.String())
+	require.NoError(t, err)
+	require.NotNil(t, resp, "expected a response")
+	intSkid := resp.Data["subject_key_id"].(string)
 
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	csr := resp.Data["csr"]
-
 	_, err = CBWrite(b_root, s_root, "sign/test", map[string]interface{}{
 		"common_name": "myint.com",
 		"csr":         csr,
@@ -2584,6 +2606,10 @@ func TestBackend_SignIntermediate_AllowedPastCA(t *testing.T) {
 	if len(resp.Warnings) == 0 {
 		t.Fatalf("expected warnings, got %#v", *resp)
 	}
+
+	cert := parseCert(t, resp.Data["certificate"].(string))
+	certSkid := certutil.GetHexFormatted(cert.SubjectKeyId, ":")
+	require.Equal(t, intSkid, certSkid)
 }
 
 func TestBackend_ConsulSignLeafWithLegacyRole(t *testing.T) {

diff --git a/builtin/logical/pki/fields.go b/builtin/logical/pki/fields.go
@@ -16,6 +16,7 @@ const (
 	keyIdParam     = "key_id"
 	keyTypeParam   = "key_type"
 	keyBitsParam   = "key_bits"
+	skidParam      = "subject_key_id"
 )
 
 // addIssueAndSignCommonFields adds fields common to both CA and non-CA issuing

diff --git a/builtin/logical/pki/path_fetch_keys.go b/builtin/logical/pki/path_fetch_keys.go
@@ -5,9 +5,11 @@ package pki
 
 import (
 	"context"
+	"crypto"
 	"fmt"
 	"net/http"
 
+	"github.com/hashicorp/vault/sdk/helper/certutil"
 	"github.com/hashicorp/vault/sdk/helper/errutil"
 
 	"github.com/hashicorp/vault/sdk/framework"
@@ -144,6 +146,11 @@ func buildPathKey(b *backend, pattern string, displayAttrs *framework.DisplayAtt
 								Description: `Key Type`,
 								Required:    true,
 							},
+							"subject_key_id": {
+								Type:        framework.TypeString,
+								Description: `RFC 5280 Subject Key Identifier of the public counterpart`,
+								Required:    false,
+							},
 							"managed_key_id": {
 								Type:        framework.TypeString,
 								Description: `Managed Key Id`,
@@ -247,6 +254,7 @@ func (b *backend) pathGetKeyHandler(ctx context.Context, req *logical.Request, d
 		keyTypeParam: string(key.PrivateKeyType),
 	}
 
+	var pkForSkid crypto.PublicKey
 	if key.isManagedPrivateKey() {
 		managedKeyUUID, err := key.getManagedKeyUUID()
 		if err != nil {
@@ -258,12 +266,28 @@ func (b *backend) pathGetKeyHandler(ctx context.Context, req *logical.Request, d
 			return nil, errutil.InternalError{Err: fmt.Sprintf("failed fetching managed key info from key id %s (%s): %v", key.ID, key.Name, err)}
 		}
 
+		pkForSkid, err = getManagedKeyPublicKey(sc.Context, sc.Backend, managedKeyUUID)
+		if err != nil {
+			return nil, err
+		}
+
 		// To remain consistent across the api responses (mainly generate root/intermediate calls), return the actual
 		// type of key, not that it is a managed key.
 		respData[keyTypeParam] = string(keyInfo.keyType)
 		respData[managedKeyIdArg] = string(keyInfo.uuid)
 		respData[managedKeyNameArg] = string(keyInfo.name)
+	} else {
+		pkForSkid, err = getPublicKeyFromBytes([]byte(key.PrivateKey))
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	skid, err := certutil.GetSubjectKeyID(pkForSkid)
+	if err != nil {
+		return nil, err
 	}
+	respData[skidParam] = certutil.GetHexFormatted([]byte(skid), ":")
 
 	return &logical.Response{Data: respData}, nil
 }

diff --git a/changelog/20642.txt b/changelog/20642.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/pki: add subject key identifier to read key response
+```
diff --git a/sdk/helper/certutil/helpers.go b/sdk/helper/certutil/helpers.go
@@ -126,7 +126,7 @@ func GetSubjKeyID(privateKey crypto.Signer) ([]byte, error) {
 	if privateKey == nil {
 		return nil, errutil.InternalError{Err: "passed-in private key is nil"}
 	}
-	return getSubjectKeyID(privateKey.Public())
+	return GetSubjectKeyID(privateKey.Public())
 }
 
 // Returns the explicit SKID when used for cross-signing, else computes a new
@@ -136,10 +136,10 @@ func getSubjectKeyIDFromBundle(data *CreationBundle) ([]byte, error) {
 		return data.Params.SKID, nil
 	}
 
-	return getSubjectKeyID(data.CSR.PublicKey)
+	return GetSubjectKeyID(data.CSR.PublicKey)
 }
 
-func getSubjectKeyID(pub interface{}) ([]byte, error) {
+func GetSubjectKeyID(pub interface{}) ([]byte, error) {
 	var publicKeyBytes []byte
 	switch pub := pub.(type) {
 	case *rsa.PublicKey: