Skip to content

Commit

Permalink
Allow to retry with no tags when loading privatekeys
Browse files Browse the repository at this point in the history 
This function allows to retry the retrieval of a key without tags if the
original uri didn't have one.

This is used only in GetPublicKey and CreateSigner. It is not used in
SearchKeys as we will need to remove duplicated keys.
  • Loading branch information
@maraino
maraino committed Nov 14, 2024
1 parent 3a8464f commit 05f8a4b
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 5 deletions.
27 changes: 27 additions & 0 deletions kms/mackms/mackms.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,30 @@ type keyAttributes struct {
label string
tag string
hash []byte
retry bool
useSecureEnclave bool
useBiometrics bool
sigAlgorithm apiv1.SignatureAlgorithm
keySize int
}

// retryAttributes returns the original URI attributes used to get a private
// key, but only if they are different that the ones set. It will return nil, if
// they are the same. The only attribute that can change is the tag. This method
// would return the tag empty if it was set using the default value.
func (k *keyAttributes) retryAttributes() *keyAttributes {
if !k.retry {
fmt.Println("no retry")
return nil
}
fmt.Println("retry")
return &keyAttributes{
label: k.label,
hash: k.hash,
retry: false,
}
}

type keySearchAttributes struct {
label string
tag string
Expand Down Expand Up @@ -715,6 +733,12 @@ func getPrivateKey(u *keyAttributes) (*security.SecKeyRef, error) {

var key cf.TypeRef
if err := security.SecItemCopyMatching(query, &key); err != nil {
// If not found retry without the tag if it wasn't set.
if errors.Is(err, security.ErrNotFound) {
if ru := u.retryAttributes(); ru != nil {
return getPrivateKey(ru)
}
}
return nil, fmt.Errorf("macOS SecItemCopyMatching failed: %w", err)
}
return security.NewSecKeyRef(key), nil
Expand Down Expand Up @@ -990,6 +1014,7 @@ func parseURI(rawuri string) (*keyAttributes, error) {
return &keyAttributes{
label: rawuri,
tag: DefaultTag,
retry: true,
}, nil
}

Expand All @@ -1006,6 +1031,7 @@ func parseURI(rawuri string) (*keyAttributes, error) {
return &keyAttributes{
label: k,
tag: DefaultTag,
retry: true,
}, nil
}
}
Expand All @@ -1025,6 +1051,7 @@ func parseURI(rawuri string) (*keyAttributes, error) {
label: label,
tag: tag,
hash: u.GetEncoded("hash"),
retry: !u.Has("tag"),
useSecureEnclave: u.GetBool("se"),
useBiometrics: u.GetBool("bio"),
}, nil
Expand Down
67 changes: 62 additions & 5 deletions kms/mackms/mackms_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ func TestMacKMS_GetPublicKey(t *testing.T) {

// Create private keys only
r2 := createPrivateKeyOnly(t, "mackms:label=test-ecdsa", apiv1.ECDSAWithSHA256)
r3 := createPrivateKeyOnly(t, "mackms:label=test-rsa", apiv1.SHA256WithRSA)
r3 := createPrivateKeyOnly(t, "mackms:label=test-rsa;tag=", apiv1.SHA256WithRSA)

t.Cleanup(func() {
assert.NoError(t, kms.DeleteKey(&apiv1.DeleteKeyRequest{
Expand All @@ -334,7 +334,8 @@ func TestMacKMS_GetPublicKey(t *testing.T) {
{"ok", &MacKMS{}, args{&apiv1.GetPublicKeyRequest{Name: r1.Name}}, r1.PublicKey, assert.NoError},
{"ok no tag", &MacKMS{}, args{&apiv1.GetPublicKeyRequest{Name: "mackms:label=test-p256;tag="}}, r1.PublicKey, assert.NoError},
{"ok private only ECDSA ", &MacKMS{}, args{&apiv1.GetPublicKeyRequest{Name: "mackms:label=test-ecdsa"}}, r2.PublicKey, assert.NoError},
{"ok private only RSA ", &MacKMS{}, args{&apiv1.GetPublicKeyRequest{Name: r3.Name}}, r3.PublicKey, assert.NoError},
{"ok private only RSA", &MacKMS{}, args{&apiv1.GetPublicKeyRequest{Name: r3.Name}}, r3.PublicKey, assert.NoError},
{"ok private only RSA with retry", &MacKMS{}, args{&apiv1.GetPublicKeyRequest{Name: "mackms:label=test-rsa"}}, r3.PublicKey, assert.NoError},
{"ok no uri", &MacKMS{}, args{&apiv1.GetPublicKeyRequest{Name: "test-p256"}}, r1.PublicKey, assert.NoError},
{"ok uri simple", &MacKMS{}, args{&apiv1.GetPublicKeyRequest{Name: "mackms:test-p256"}}, r1.PublicKey, assert.NoError},
{"ok uri label", &MacKMS{}, args{&apiv1.GetPublicKeyRequest{Name: "mackms:label=test-p256"}}, r1.PublicKey, assert.NoError},
Expand Down Expand Up @@ -541,11 +542,12 @@ func Test_parseURI(t *testing.T) {
assertion assert.ErrorAssertionFunc
}{
{"ok", args{"mackms:label=the-label;tag=the-tag;hash=0102abcd"}, &keyAttributes{label: "the-label", tag: "the-tag", hash: []byte{1, 2, 171, 205}}, assert.NoError},
{"ok label", args{"the-label"}, &keyAttributes{label: "the-label", tag: DefaultTag}, assert.NoError},
{"ok label uri", args{"mackms:label=the-label"}, &keyAttributes{label: "the-label", tag: DefaultTag}, assert.NoError},
{"ok label", args{"the-label"}, &keyAttributes{label: "the-label", tag: DefaultTag, retry: true}, assert.NoError},
{"ok label uri", args{"mackms:label=the-label"}, &keyAttributes{label: "the-label", tag: DefaultTag, retry: true}, assert.NoError},
{"ok label uri simple", args{"mackms:the-label"}, &keyAttributes{label: "the-label", tag: DefaultTag, retry: true}, assert.NoError},
{"ok label empty tag", args{"mackms:label=the-label;tag="}, &keyAttributes{label: "the-label", tag: ""}, assert.NoError},
{"ok label empty tag no equal", args{"mackms:label=the-label;tag"}, &keyAttributes{label: "the-label", tag: ""}, assert.NoError},
{"fail parse", args{"mackms:::label=the-label"}, nil, assert.Error},
{"fail parse", args{"mackms:%label=the-label"}, nil, assert.Error},
{"fail missing label", args{"mackms:hash=0102abcd"}, nil, assert.Error},
}
for _, tt := range tests {
Expand Down Expand Up @@ -1306,3 +1308,58 @@ func TestMacKMS_SearchKeys(t *testing.T) {

assert.Equal(t, expectedHashes, hashes)
}

func Test_keyAttributes_retryAttributes(t *testing.T) {
type fields struct {
label string
tag string
hash []byte
retry bool
}

mustFields := func(s string) fields {
t.Helper()
u, err := parseURI(s)
require.NoError(t, err)
return fields{
label: u.label,
tag: u.tag,
hash: u.hash,
retry: u.retry,
}
}

tests := []struct {
name string
fields fields
want *keyAttributes
}{
{"with tag", mustFields("mackms:label=label;tag=tag"), nil},
{"with tag and hash", mustFields("mackms:label=label;hash=FF00;tag=tag"), nil},
{"with empty tag", mustFields("mackms:label=label;tag="), nil},
{"with no tag", mustFields("mackms:label=label;hash=FF00"), &keyAttributes{
label: "label",
hash: []byte{0xFF, 0x00},
}},
{"legacy name only", mustFields("label"), &keyAttributes{
label: "label",
}},
{"legacy with schema", mustFields("mackms:label"), &keyAttributes{
label: "label",
}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
k := &keyAttributes{
label: tt.fields.label,
tag: tt.fields.tag,
hash: tt.fields.hash,
retry: tt.fields.retry,
}
if tt.name == "with no tag" {
t.Log("foo")
}
assert.Equal(t, tt.want, k.retryAttributes())
})
}
}

0 comments on commit 05f8a4b

Please sign in to comment.