Allow to retry with no tags when loading privatekeys

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.

Author: maraino

kms/mackms/mackms.go

@@ -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
@@ -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
@@ -990,6 +1014,7 @@ func parseURI(rawuri string) (*keyAttributes, error) {
 		return &keyAttributes{
 			label: rawuri,
 			tag:   DefaultTag,
+			retry: true,
 		}, nil
 	}
 
@@ -1006,6 +1031,7 @@ func parseURI(rawuri string) (*keyAttributes, error) {
 				return &keyAttributes{
 					label: k,
 					tag:   DefaultTag,
+					retry: true,
 				}, nil
 			}
 		}
@@ -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

kms/mackms/mackms_test.go

@@ -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{
@@ -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},
@@ -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 {
@@ -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())
+		})
+	}
+}