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.

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

Backport of pki: add subject key identifier to read key response into release/1.13.x #20658

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 29 additions & 2 deletions builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2382,6 +2382,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)
Expand All @@ -2396,6 +2404,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)
Expand Down Expand Up @@ -2501,12 +2517,19 @@ func TestBackend_SignIntermediate_AllowedPastCA(t *testing.T) {
resp, err := CBWrite(b_int, s_int, "intermediate/generate/internal", map[string]interface{}{
"common_name": "myint.com",
})
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,
Expand Down Expand Up @@ -2541,6 +2564,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) {
Expand Down
1 change: 1 addition & 0 deletions builtin/logical/pki/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,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
Expand Down
19 changes: 19 additions & 0 deletions builtin/logical/pki/path_fetch_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package pki

import (
"context"
"crypto"
"fmt"

"github.com/hashicorp/vault/sdk/helper/certutil"
"github.com/hashicorp/vault/sdk/helper/errutil"

"github.com/hashicorp/vault/sdk/framework"
Expand Down Expand Up @@ -155,6 +157,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 {
Expand All @@ -166,12 +169,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
}
Expand Down
3 changes: 3 additions & 0 deletions changelog/20642.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
secrets/pki: add subject key identifier to read key response
```
6 changes: 3 additions & 3 deletions sdk/helper/certutil/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,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
Expand All @@ -133,10 +133,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:
Expand Down