Skip to content

Allow to retry with no tags when loading privatekeys #631

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.

Sign up for GitHub

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

Merged
maraino merged 2 commits into from
Nov 14, 2024
Merged

Allow to retry with no tags when loading privatekeys #631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
05f8a4b
Allow to retry with no tags when loading privatekeys
maraino Nov 14, 2024
d01bccd
Remove debug comments
maraino Nov 14, 2024
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
25 changes: 25 additions & 0 deletions kms/mackms/mackms.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,28 @@ 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 {
return nil
}
return &keyAttributes{
label: k.label,
hash: k.hash,
retry: false,
}
}

type keySearchAttributes struct {
label string
tag string
Expand Down Expand Up @@ -715,6 +731,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 +1012,7 @@ func parseURI(rawuri string) (*keyAttributes, error) {
return &keyAttributes{
label: rawuri,
tag: DefaultTag,
retry: true,
}, nil
}

Expand All @@ -1006,6 +1029,7 @@ func parseURI(rawuri string) (*keyAttributes, error) {
return &keyAttributes{
label: k,
tag: DefaultTag,
retry: true,
}, nil
}
}
Expand All @@ -1025,6 +1049,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
Open in desktop
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())
})
}
}