From 7d3bfac523e1d82c2e967506028a46cdc6baf82c Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Thu, 18 Jul 2024 17:25:22 +0200 Subject: [PATCH 1/5] Add `SearchKeys` functionality to MacKMS --- .../corefoundation/core_foundation_darwin.go | 46 +++++ internal/darwin/security/security_darwin.go | 20 ++ kms/apiv1/options.go | 12 ++ kms/apiv1/requests.go | 17 ++ kms/mackms/mackms.go | 178 ++++++++++++++++++ kms/mackms/mackms_test.go | 42 +++++ 6 files changed, 315 insertions(+) diff --git a/internal/darwin/corefoundation/core_foundation_darwin.go b/internal/darwin/corefoundation/core_foundation_darwin.go index 1008ddfb..aa199440 100644 --- a/internal/darwin/corefoundation/core_foundation_darwin.go +++ b/internal/darwin/corefoundation/core_foundation_darwin.go @@ -33,6 +33,7 @@ const ( nilCFData C.CFDataRef = 0 nilCFString C.CFStringRef = 0 nilCFDictionary C.CFDictionaryRef = 0 + nilCFArray C.CFArrayRef = 0 nilCFError C.CFErrorRef = 0 nilCFType C.CFTypeRef = 0 ) @@ -45,10 +46,15 @@ func Release(ref TypeReferer) { C.CFRelease(ref.TypeRef()) } +func Retain(ref TypeReferer) { + C.CFRetain(ref.TypeRef()) +} + type CFTypeRef = C.CFTypeRef type CFStringRef = C.CFStringRef type CFErrorRef = C.CFErrorRef type CFDictionaryRef = C.CFDictionaryRef +type CFArrayRef = C.CFArrayRef type CFDataRef = C.CFDataRef type TypeRef C.CFTypeRef @@ -167,6 +173,46 @@ func NewDictionaryRef(ref TypeRef) *DictionaryRef { func (v *DictionaryRef) Release() { Release(v) } func (v *DictionaryRef) TypeRef() CFTypeRef { return C.CFTypeRef(v.Value) } +func (v *DictionaryRef) XML() []byte { + if v == nil { + return nil + } + + ref := C.CFPropertyListCreateData(AllocatorDefault, v.TypeRef(), C.kCFPropertyListXMLFormat_v1_0, 0, nil) + + // TODO: release? + return C.GoBytes( + unsafe.Pointer(C.CFDataGetBytePtr(ref)), + C.int(C.CFDataGetLength(ref)), + ) +} + +type ArrayRef struct { + Value C.CFArrayRef +} + +func NewArrayRef(ref TypeRef) *ArrayRef { + return &ArrayRef{ + Value: C.CFArrayRef(ref), + } +} + +func (v *ArrayRef) Release() { Release(v) } +func (v *ArrayRef) TypeRef() CFTypeRef { return C.CFTypeRef(v.Value) } + +func (v *ArrayRef) Len() int { + arrayCount := C.CFArrayGetCount(v.Value) + + // TODO: release array count? + + return int(arrayCount) +} + +func (v *ArrayRef) Get(index int) TypeRef { + item := C.CFArrayGetValueAtIndex(v.Value, C.CFIndex(index)) + return TypeRef(item) +} + //nolint:errname // type name matches original name type ErrorRef C.CFErrorRef diff --git a/internal/darwin/security/security_darwin.go b/internal/darwin/security/security_darwin.go index 7ce8a7d3..70cd570a 100644 --- a/internal/darwin/security/security_darwin.go +++ b/internal/darwin/security/security_darwin.go @@ -80,6 +80,7 @@ var ( KSecClassIdentity = cf.TypeRef(C.kSecClassIdentity) KSecMatchLimit = cf.TypeRef(C.kSecMatchLimit) KSecMatchLimitOne = cf.TypeRef(C.kSecMatchLimitOne) + KSecMatchLimitAll = cf.TypeRef(C.kSecMatchLimitAll) KSecPublicKeyAttrs = cf.TypeRef(C.kSecPublicKeyAttrs) KSecPrivateKeyAttrs = cf.TypeRef(C.kSecPrivateKeyAttrs) KSecReturnRef = cf.TypeRef(C.kSecReturnRef) @@ -150,6 +151,7 @@ func NewSecKeyRef(ref cf.TypeRef) *SecKeyRef { func (v *SecKeyRef) Release() { cf.Release(v) } func (v *SecKeyRef) TypeRef() cf.CFTypeRef { return cf.CFTypeRef(v.Value) } +func (v *SecKeyRef) Retain() { cf.Retain(v) } type SecCertificateRef struct { Value C.SecCertificateRef @@ -309,6 +311,24 @@ func GetSecAttrApplicationLabel(v *cf.DictionaryRef) []byte { ) } +func GetSecAttrApplicationTag(v *cf.DictionaryRef) string { + ref := C.CFStringRef(C.CFDictionaryGetValue(C.CFDictionaryRef(v.Value), unsafe.Pointer(C.kSecAttrApplicationTag))) + tag := "" + if cstr := C.CFStringGetCStringPtr(ref, C.kCFStringEncodingUTF8); cstr != nil { + tag = C.GoString(cstr) + } + return tag +} + +func GetSecAttrLabel(v *cf.DictionaryRef) string { + ref := C.CFStringRef(C.CFDictionaryGetValue(C.CFDictionaryRef(v.Value), unsafe.Pointer(C.kSecAttrLabel))) + label := "" + if cstr := C.CFStringGetCStringPtr(ref, C.kCFStringEncodingUTF8); cstr != nil { + label = C.GoString(cstr) + } + return label +} + func GetSecValueData(v *cf.DictionaryRef) []byte { data := C.CFDataRef(C.CFDictionaryGetValue(C.CFDictionaryRef(v.Value), unsafe.Pointer(C.kSecValueData))) return C.GoBytes( diff --git a/kms/apiv1/options.go b/kms/apiv1/options.go index faa7737b..d8af2db2 100644 --- a/kms/apiv1/options.go +++ b/kms/apiv1/options.go @@ -18,6 +18,18 @@ type KeyManager interface { Close() error } +// SearchableKeyManager is an optional interface for KMS implementations +// that support searching for keys based on crtain attributes. +// +// # Experimental +// +// Notice: This API is EXPERIMENTAL and may be changed or removed in a later +// release. +type SearchableKeyManager interface { + KeyManager + SearchKeys(req *SearchKeysRequest) (*SearchKeysResponse, error) +} + // Decrypter is an interface implemented by KMSes that are used // in operations that require decryption type Decrypter interface { diff --git a/kms/apiv1/requests.go b/kms/apiv1/requests.go index f1430e1c..eb55f8ed 100644 --- a/kms/apiv1/requests.go +++ b/kms/apiv1/requests.go @@ -178,6 +178,23 @@ type CreateKeyResponse struct { CreateSignerRequest CreateSignerRequest } +// SearchKeysRequest is the request for the SearchKeys method. It takes +// a single Query string with the attributes to match when searching the +// KMS. +type SearchKeysRequest struct { + Query string +} + +// SearchKeyResult is a single results returned from the SearchKeys +// method. +type SearchKeyResult CreateKeyResponse + +// SearchKeysResponse is the response for the SearchKeys method. It +// wraps a slice of SearchKeyResponse structs. +type SearchKeysResponse struct { + Results []SearchKeyResult +} + // CreateSignerRequest is the parameter used in the kms.CreateSigner method. type CreateSignerRequest struct { Signer crypto.Signer diff --git a/kms/mackms/mackms.go b/kms/mackms/mackms.go index 4efdb366..4cbf1f36 100644 --- a/kms/mackms/mackms.go +++ b/kms/mackms/mackms.go @@ -555,6 +555,80 @@ func (*MacKMS) DeleteCertificate(req *apiv1.DeleteCertificateRequest) error { return nil } +func (*MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysResponse, error) { + if req.Query == "" { + return nil, fmt.Errorf("searchKeysRequest 'query' cannot be empty") + } + + u, err := parseSearchURI(req.Query) + if err != nil { + return nil, fmt.Errorf("mackms CreateKey failed: %w", err) + } + + keys, err := getPrivateKeys(u) + if err != nil { + return nil, fmt.Errorf("failed getting keys: %w", err) + } + + results := make([]apiv1.SearchKeyResult, len(keys)) + for i, key := range keys { + pub, hash, err := extractPublicKey(key) + if err != nil { + return nil, fmt.Errorf("failed extracting public key: %w", err) + } + + attrs := security.SecKeyCopyAttributes(key) + defer attrs.Release() + + h := security.GetSecAttrApplicationLabel(attrs) + fmt.Println("h", h, string(h)) // TODO: remove debugging + + a := security.GetSecAttrApplicationTag(attrs) + fmt.Println("a", a, string(a)) + + l := security.GetSecAttrLabel(attrs) + fmt.Println("l", l) + + j := attrs.XML() + fmt.Println("j", string(j)) + + name := uri.New(Scheme, url.Values{ + "hash": []string{hex.EncodeToString(hash)}, + }) + + // TODO: should we rely on the values from u only? Or can we get them from key properties too? + if u.label != "" { + name.Values.Set("label", u.label) + } + if u.tag != "" { + name.Values.Set("tag", u.tag) + } + if u.useSecureEnclave { + name.Values.Set("se", "true") + } + if u.useBiometrics { + name.Values.Set("bio", "true") + } + + results[i] = apiv1.SearchKeyResult{ + Name: name.String(), + PublicKey: pub, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: name.String(), + }, + } + + // ensure the resource is released + key.Release() + } + + return &apiv1.SearchKeysResponse{ + Results: results, + }, nil +} + +var _ apiv1.SearchableKeyManager = (*MacKMS)(nil) + func deleteItem(dict cf.Dictionary, hash []byte) error { if len(hash) > 0 { d, err := cf.NewData(hash) @@ -625,6 +699,67 @@ func getPrivateKey(u *keyAttributes) (*security.SecKeyRef, error) { return security.NewSecKeyRef(key), nil } +func getPrivateKeys(u *keyAttributes) ([]*security.SecKeyRef, error) { + dict := cf.Dictionary{ + security.KSecClass: security.KSecClassKey, + security.KSecAttrKeyClass: security.KSecAttrKeyClassPrivate, + security.KSecReturnRef: cf.True, + security.KSecMatchLimit: security.KSecMatchLimitAll, + } + + if u.tag != "" { + cfTag, err := cf.NewData([]byte(u.tag)) + if err != nil { + return nil, err + } + defer cfTag.Release() + dict[security.KSecAttrApplicationTag] = cfTag + } + if u.label != "" { + cfLabel, err := cf.NewString(u.label) + if err != nil { + return nil, err + } + defer cfLabel.Release() + dict[security.KSecAttrLabel] = cfLabel + } + if len(u.hash) > 0 { + cfHash, err := cf.NewData(u.hash) + if err != nil { + return nil, err + } + defer cfHash.Release() + dict[security.KSecAttrApplicationLabel] = cfHash + } + + // construct the query + query, err := cf.NewDictionary(dict) + if err != nil { + return nil, err + } + defer query.Release() + + // perform the query + var result cf.TypeRef + err = security.SecItemCopyMatching(query, &result) + if err != nil { + return nil, fmt.Errorf("failed matching: %w", err) + } + + array := cf.NewArrayRef(result) + defer array.Release() + + keys := make([]*security.SecKeyRef, array.Len()) + for i := 0; i < array.Len(); i++ { + item := array.Get(i) + key := security.NewSecKeyRef(item) + key.Retain() // retain the key, so that it's not released early + keys[i] = key + } + + return keys, nil +} + func extractPublicKey(secKeyRef *security.SecKeyRef) (crypto.PublicKey, []byte, error) { // Get the hash of the public key. We can also calculate this from the // external representation below, but in case Apple decides to switch from @@ -915,6 +1050,49 @@ func parseCertURI(rawuri string, requireValue bool) (*certAttributes, error) { }, nil } +func parseSearchURI(rawuri string) (*keyAttributes, error) { + // When rawuri is just the key name + if !strings.HasPrefix(strings.ToLower(rawuri), Scheme) { + return &keyAttributes{ + label: rawuri, + tag: DefaultTag, + }, nil + } + + // When rawuri is a mackms uri. + u, err := uri.ParseWithScheme(Scheme, rawuri) + if err != nil { + return nil, err + } + + // Special case for mackms:label + if len(u.Values) == 1 { + for k, v := range u.Values { + if (len(v) == 1 && v[0] == "") || len(v) == 0 { + return &keyAttributes{ + label: k, + tag: DefaultTag, + }, nil + } + } + } + + // With regular values, uris look like this: + // mackms:label=my-key;tag=my-tag;hash=010a...;se=true;bio=true + label := u.Get("label") // when searching, the label can be empty + tag := u.Get("tag") + if tag == "" { + tag = DefaultTag + } + return &keyAttributes{ + label: label, + tag: tag, + hash: u.GetEncoded("hash"), + useSecureEnclave: u.GetBool("se"), + useBiometrics: u.GetBool("bio"), + }, nil +} + // encodeSerialNumber encodes the serial number of a certificate in ASN.1. // Negative serial numbers are not allowed. func encodeSerialNumber(s *big.Int) []byte { diff --git a/kms/mackms/mackms_test.go b/kms/mackms/mackms_test.go index a93b6d59..e265a0d9 100644 --- a/kms/mackms/mackms_test.go +++ b/kms/mackms/mackms_test.go @@ -1233,3 +1233,45 @@ func Test_apiv1Error(t *testing.T) { }) } } + +func TestMacKMS_SearchKeys(t *testing.T) { + name, err := randutil.Hex(10) + require.NoError(t, err) + tag := fmt.Sprintf("com.smallstep.crypto.test.%s", name) // unique tag per test execution + + k := &MacKMS{} + key1, err := k.CreateKey(&apiv1.CreateKeyRequest{Name: fmt.Sprintf("mackms:name=test-step-1;label=test-step-1;tag=%s;se=false", tag)}) + require.NoError(t, err) + key2, err := k.CreateKey(&apiv1.CreateKeyRequest{Name: fmt.Sprintf("mackms:name=test-step-2;label=test-step-2;tag=%s;se=false", tag)}) + require.NoError(t, err) + u1, err := uri.ParseWithScheme(Scheme, key1.Name) + require.NoError(t, err) + u2, err := uri.ParseWithScheme(Scheme, key2.Name) + require.NoError(t, err) + expectedHashes := []string{u1.Get("hash"), u2.Get("hash")} + require.Len(t, expectedHashes, 2) + t.Cleanup(func() { + err = k.DeleteKey(&apiv1.DeleteKeyRequest{Name: key1.Name}) + require.NoError(t, err) + err = k.DeleteKey(&apiv1.DeleteKeyRequest{Name: key2.Name}) + require.NoError(t, err) + }) + + // search by tag + got, err := k.SearchKeys(&apiv1.SearchKeysRequest{Query: fmt.Sprintf("mackms:tag=%s", tag)}) + require.NoError(t, err) + require.NotNil(t, got) + require.Len(t, got.Results, 2) + + // check if the correct keys were found by comparing hashes + var hashes []string + for _, key := range got.Results { + u, err := uri.ParseWithScheme(Scheme, key.Name) + require.NoError(t, err) + if hash := u.Get("hash"); hash != "" { + hashes = append(hashes, hash) + } + } + + assert.Equal(t, expectedHashes, hashes) +} From a870eb8c63c5f5409d12c73569c29227b8d1410e Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 19 Jul 2024 00:03:34 +0200 Subject: [PATCH 2/5] Make the MacKMS search functionality work with keychain attributes --- internal/darwin/security/security_darwin.go | 26 +++++++-- kms/mackms/mackms.go | 61 ++++++++------------- kms/mackms/mackms_test.go | 2 + 3 files changed, 46 insertions(+), 43 deletions(-) diff --git a/internal/darwin/security/security_darwin.go b/internal/darwin/security/security_darwin.go index 70cd570a..2c0c2123 100644 --- a/internal/darwin/security/security_darwin.go +++ b/internal/darwin/security/security_darwin.go @@ -84,6 +84,7 @@ var ( KSecPublicKeyAttrs = cf.TypeRef(C.kSecPublicKeyAttrs) KSecPrivateKeyAttrs = cf.TypeRef(C.kSecPrivateKeyAttrs) KSecReturnRef = cf.TypeRef(C.kSecReturnRef) + KSecReturnAttributes = cf.TypeRef(C.kSecReturnAttributes) KSecValueRef = cf.TypeRef(C.kSecValueRef) KSecValueData = cf.TypeRef(C.kSecValueData) ) @@ -139,6 +140,20 @@ const ( KSecAccessControlOr = SecAccessControlCreateFlags(C.kSecAccessControlOr) ) +type SecKeychainItemRef struct { + Value C.SecKeychainItemRef +} + +func NewSecKeychainItemRef(ref cf.TypeRef) *SecKeychainItemRef { + return &SecKeychainItemRef{ + Value: C.SecKeychainItemRef(ref), + } +} + +func (v *SecKeychainItemRef) Release() { cf.Release(v) } +func (v *SecKeychainItemRef) TypeRef() cf.CFTypeRef { return cf.CFTypeRef(v.Value) } +func (v *SecKeychainItemRef) Retain() { cf.Retain(v) } + type SecKeyRef struct { Value C.SecKeyRef } @@ -312,12 +327,11 @@ func GetSecAttrApplicationLabel(v *cf.DictionaryRef) []byte { } func GetSecAttrApplicationTag(v *cf.DictionaryRef) string { - ref := C.CFStringRef(C.CFDictionaryGetValue(C.CFDictionaryRef(v.Value), unsafe.Pointer(C.kSecAttrApplicationTag))) - tag := "" - if cstr := C.CFStringGetCStringPtr(ref, C.kCFStringEncodingUTF8); cstr != nil { - tag = C.GoString(cstr) - } - return tag + data := C.CFDataRef(C.CFDictionaryGetValue(C.CFDictionaryRef(v.Value), unsafe.Pointer(C.kSecAttrApplicationTag))) + return string(C.GoBytes( + unsafe.Pointer(C.CFDataGetBytePtr(data)), + C.int(C.CFDataGetLength(data)), + )) } func GetSecAttrLabel(v *cf.DictionaryRef) string { diff --git a/kms/mackms/mackms.go b/kms/mackms/mackms.go index 4cbf1f36..ebfcab45 100644 --- a/kms/mackms/mackms.go +++ b/kms/mackms/mackms.go @@ -555,7 +555,7 @@ func (*MacKMS) DeleteCertificate(req *apiv1.DeleteCertificateRequest) error { return nil } -func (*MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysResponse, error) { +func (k *MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysResponse, error) { if req.Query == "" { return nil, fmt.Errorf("searchKeysRequest 'query' cannot be empty") } @@ -572,37 +572,18 @@ func (*MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysRespon results := make([]apiv1.SearchKeyResult, len(keys)) for i, key := range keys { - pub, hash, err := extractPublicKey(key) - if err != nil { - return nil, fmt.Errorf("failed extracting public key: %w", err) - } - - attrs := security.SecKeyCopyAttributes(key) - defer attrs.Release() - - h := security.GetSecAttrApplicationLabel(attrs) - fmt.Println("h", h, string(h)) // TODO: remove debugging - - a := security.GetSecAttrApplicationTag(attrs) - fmt.Println("a", a, string(a)) - - l := security.GetSecAttrLabel(attrs) - fmt.Println("l", l) + d := cf.NewDictionaryRef(cf.TypeRef(key.TypeRef())) + defer d.Release() - j := attrs.XML() - fmt.Println("j", string(j)) + fmt.Println(string(d.XML())) // TODO: remove debug name := uri.New(Scheme, url.Values{ - "hash": []string{hex.EncodeToString(hash)}, + "hash": []string{hex.EncodeToString(security.GetSecAttrApplicationLabel(d))}, + "label": []string{security.GetSecAttrLabel(d)}, + "tag": []string{security.GetSecAttrApplicationTag(d)}, }) - // TODO: should we rely on the values from u only? Or can we get them from key properties too? - if u.label != "" { - name.Values.Set("label", u.label) - } - if u.tag != "" { - name.Values.Set("tag", u.tag) - } + // TODO: extract those from the attributes too if u.useSecureEnclave { name.Values.Set("se", "true") } @@ -610,6 +591,15 @@ func (*MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysRespon name.Values.Set("bio", "true") } + // obtain the public key by requesting it, as the current + // representation of the key are just the attributes. + pub, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ + Name: name.String(), + }) + if err != nil { + return nil, fmt.Errorf("failed getting public key: %w", err) + } + results[i] = apiv1.SearchKeyResult{ Name: name.String(), PublicKey: pub, @@ -617,9 +607,6 @@ func (*MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysRespon SigningKey: name.String(), }, } - - // ensure the resource is released - key.Release() } return &apiv1.SearchKeysResponse{ @@ -699,12 +686,12 @@ func getPrivateKey(u *keyAttributes) (*security.SecKeyRef, error) { return security.NewSecKeyRef(key), nil } -func getPrivateKeys(u *keyAttributes) ([]*security.SecKeyRef, error) { +func getPrivateKeys(u *keyAttributes) ([]*security.SecKeychainItemRef, error) { dict := cf.Dictionary{ - security.KSecClass: security.KSecClassKey, - security.KSecAttrKeyClass: security.KSecAttrKeyClassPrivate, - security.KSecReturnRef: cf.True, - security.KSecMatchLimit: security.KSecMatchLimitAll, + security.KSecClass: security.KSecClassKey, + security.KSecAttrKeyClass: security.KSecAttrKeyClassPrivate, + security.KSecReturnAttributes: cf.True, // return keychain attributes, i.e. tag and label + security.KSecMatchLimit: security.KSecMatchLimitAll, } if u.tag != "" { @@ -749,10 +736,10 @@ func getPrivateKeys(u *keyAttributes) ([]*security.SecKeyRef, error) { array := cf.NewArrayRef(result) defer array.Release() - keys := make([]*security.SecKeyRef, array.Len()) + keys := make([]*security.SecKeychainItemRef, array.Len()) for i := 0; i < array.Len(); i++ { item := array.Get(i) - key := security.NewSecKeyRef(item) + key := security.NewSecKeychainItemRef(item) key.Retain() // retain the key, so that it's not released early keys[i] = key } diff --git a/kms/mackms/mackms_test.go b/kms/mackms/mackms_test.go index e265a0d9..d233dc52 100644 --- a/kms/mackms/mackms_test.go +++ b/kms/mackms/mackms_test.go @@ -1268,9 +1268,11 @@ func TestMacKMS_SearchKeys(t *testing.T) { for _, key := range got.Results { u, err := uri.ParseWithScheme(Scheme, key.Name) require.NoError(t, err) + assert.Equal(t, tag, u.Get("tag")) if hash := u.Get("hash"); hash != "" { hashes = append(hashes, hash) } + } assert.Equal(t, expectedHashes, hashes) From 9c3c6e217b77815c67b4794fead2036cfc5d76e0 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Thu, 25 Jul 2024 14:13:30 +0200 Subject: [PATCH 3/5] Extract `se` fact from key properties --- .../corefoundation/core_foundation_darwin.go | 20 +---------- internal/darwin/security/security_darwin.go | 35 +++++++++++++++++-- kms/apiv1/options.go | 2 +- kms/mackms/mackms.go | 10 ++---- 4 files changed, 37 insertions(+), 30 deletions(-) diff --git a/internal/darwin/corefoundation/core_foundation_darwin.go b/internal/darwin/corefoundation/core_foundation_darwin.go index aa199440..552269df 100644 --- a/internal/darwin/corefoundation/core_foundation_darwin.go +++ b/internal/darwin/corefoundation/core_foundation_darwin.go @@ -173,20 +173,6 @@ func NewDictionaryRef(ref TypeRef) *DictionaryRef { func (v *DictionaryRef) Release() { Release(v) } func (v *DictionaryRef) TypeRef() CFTypeRef { return C.CFTypeRef(v.Value) } -func (v *DictionaryRef) XML() []byte { - if v == nil { - return nil - } - - ref := C.CFPropertyListCreateData(AllocatorDefault, v.TypeRef(), C.kCFPropertyListXMLFormat_v1_0, 0, nil) - - // TODO: release? - return C.GoBytes( - unsafe.Pointer(C.CFDataGetBytePtr(ref)), - C.int(C.CFDataGetLength(ref)), - ) -} - type ArrayRef struct { Value C.CFArrayRef } @@ -201,11 +187,7 @@ func (v *ArrayRef) Release() { Release(v) } func (v *ArrayRef) TypeRef() CFTypeRef { return C.CFTypeRef(v.Value) } func (v *ArrayRef) Len() int { - arrayCount := C.CFArrayGetCount(v.Value) - - // TODO: release array count? - - return int(arrayCount) + return int(C.CFArrayGetCount(v.Value)) } func (v *ArrayRef) Get(index int) TypeRef { diff --git a/internal/darwin/security/security_darwin.go b/internal/darwin/security/security_darwin.go index 2c0c2123..6c83b93b 100644 --- a/internal/darwin/security/security_darwin.go +++ b/internal/darwin/security/security_darwin.go @@ -334,15 +334,46 @@ func GetSecAttrApplicationTag(v *cf.DictionaryRef) string { )) } -func GetSecAttrLabel(v *cf.DictionaryRef) string { +func GetSecAttrLabel(v *cf.DictionaryRef) (label string) { ref := C.CFStringRef(C.CFDictionaryGetValue(C.CFDictionaryRef(v.Value), unsafe.Pointer(C.kSecAttrLabel))) - label := "" if cstr := C.CFStringGetCStringPtr(ref, C.kCFStringEncodingUTF8); cstr != nil { label = C.GoString(cstr) } return label } +func GetSecAttrTokenID(v *cf.DictionaryRef) (tokenID string) { + ref := C.CFStringRef(C.CFDictionaryGetValue(C.CFDictionaryRef(v.Value), unsafe.Pointer(C.kSecAttrTokenID))) + if cstr := C.CFStringGetCStringPtr(ref, C.kCFStringEncodingUTF8); cstr != nil { + tokenID = C.GoString(cstr) + } + return tokenID +} + +func GetSecAttrAccessControl(v *cf.DictionaryRef) *SecAccessControlRef { + var keyAttributes unsafe.Pointer + tokenID := GetSecAttrTokenID(v) + if tokenID == "com.apple.setoken" { + keyAttributes = C.CFDictionaryGetValue(C.CFDictionaryRef(v.Value), unsafe.Pointer(C.kSecPrivateKeyAttrs)) + } else { + keyAttributes = C.CFDictionaryGetValue(C.CFDictionaryRef(v.Value), unsafe.Pointer(C.kSecPublicKeyAttrs)) + } + if keyAttributes == nil { + return nil + } + + dv := C.CFDictionaryGetValue(C.CFDictionaryRef(keyAttributes), unsafe.Pointer(C.kSecAttrAccessControl)) + if dv == nil { + return nil + } + + ref := &SecAccessControlRef{ + ref: C.SecAccessControlRef(dv), + } + + return ref +} + func GetSecValueData(v *cf.DictionaryRef) []byte { data := C.CFDataRef(C.CFDictionaryGetValue(C.CFDictionaryRef(v.Value), unsafe.Pointer(C.kSecValueData))) return C.GoBytes( diff --git a/kms/apiv1/options.go b/kms/apiv1/options.go index d8af2db2..3b50b942 100644 --- a/kms/apiv1/options.go +++ b/kms/apiv1/options.go @@ -19,7 +19,7 @@ type KeyManager interface { } // SearchableKeyManager is an optional interface for KMS implementations -// that support searching for keys based on crtain attributes. +// that support searching for keys based on certain attributes. // // # Experimental // diff --git a/kms/mackms/mackms.go b/kms/mackms/mackms.go index ebfcab45..db7ce941 100644 --- a/kms/mackms/mackms.go +++ b/kms/mackms/mackms.go @@ -562,7 +562,7 @@ func (k *MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysResp u, err := parseSearchURI(req.Query) if err != nil { - return nil, fmt.Errorf("mackms CreateKey failed: %w", err) + return nil, fmt.Errorf("failed parsing query: %w", err) } keys, err := getPrivateKeys(u) @@ -575,21 +575,15 @@ func (k *MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysResp d := cf.NewDictionaryRef(cf.TypeRef(key.TypeRef())) defer d.Release() - fmt.Println(string(d.XML())) // TODO: remove debug - name := uri.New(Scheme, url.Values{ "hash": []string{hex.EncodeToString(security.GetSecAttrApplicationLabel(d))}, "label": []string{security.GetSecAttrLabel(d)}, "tag": []string{security.GetSecAttrApplicationTag(d)}, }) - // TODO: extract those from the attributes too - if u.useSecureEnclave { + if tokenID := security.GetSecAttrTokenID(d); tokenID == "com.apple.setoken" { name.Values.Set("se", "true") } - if u.useBiometrics { - name.Values.Set("bio", "true") - } // obtain the public key by requesting it, as the current // representation of the key are just the attributes. From 4baf2cec18fcfe8fffd155655d92e1519ffa5b93 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Fri, 26 Jul 2024 14:01:50 +0200 Subject: [PATCH 4/5] Return empty slice when no keys are found for the `SearchKeys` query --- kms/apiv1/requests.go | 7 ++++--- kms/mackms/mackms.go | 29 +++++++++++++++++++++-------- kms/mackms/mackms_test.go | 10 +++++++++- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/kms/apiv1/requests.go b/kms/apiv1/requests.go index eb55f8ed..394dd0ac 100644 --- a/kms/apiv1/requests.go +++ b/kms/apiv1/requests.go @@ -179,18 +179,19 @@ type CreateKeyResponse struct { } // SearchKeysRequest is the request for the SearchKeys method. It takes -// a single Query string with the attributes to match when searching the +// a Query string with the attributes to match when searching the // KMS. type SearchKeysRequest struct { Query string } -// SearchKeyResult is a single results returned from the SearchKeys +// SearchKeyResult is a single result returned from the SearchKeys // method. type SearchKeyResult CreateKeyResponse // SearchKeysResponse is the response for the SearchKeys method. It -// wraps a slice of SearchKeyResponse structs. +// wraps a slice of SearchKeyResult structs. The Results slice can +// be empty in case no key was found for the search query. type SearchKeysResponse struct { Results []SearchKeyResult } diff --git a/kms/mackms/mackms.go b/kms/mackms/mackms.go index db7ce941..9398075d 100644 --- a/kms/mackms/mackms.go +++ b/kms/mackms/mackms.go @@ -555,6 +555,18 @@ func (*MacKMS) DeleteCertificate(req *apiv1.DeleteCertificateRequest) error { return nil } +// SearchKeys searches for keys according to the query URI in the request. +// +// - "" will return all keys managed by the KMS (using the default tag) +// - "mackms:" will return all keys managed by the KMS (using the default tag) +// - "mackms:label=my-label" will return all keys using label "my-label" (and the default tag) +// - "mackms:hash=the-hash" will return all keys having hash "hash" (and the default tag; generally one result) +// - "mackms:tag=my-tag" will search for all keys with "my-tag" +// +// # Experimental +// +// Notice: This API is EXPERIMENTAL and may be changed or removed in a later +// release. func (k *MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysResponse, error) { if req.Query == "" { return nil, fmt.Errorf("searchKeysRequest 'query' cannot be empty") @@ -586,7 +598,7 @@ func (k *MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysResp } // obtain the public key by requesting it, as the current - // representation of the key are just the attributes. + // representation of the key includes just the attributes. pub, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ Name: name.String(), }) @@ -724,7 +736,10 @@ func getPrivateKeys(u *keyAttributes) ([]*security.SecKeychainItemRef, error) { var result cf.TypeRef err = security.SecItemCopyMatching(query, &result) if err != nil { - return nil, fmt.Errorf("failed matching: %w", err) + if errors.Is(err, security.ErrNotFound) { + return []*security.SecKeychainItemRef{}, nil + } + return nil, fmt.Errorf("macOS SecItemCopyMatching failed: %w", err) } array := cf.NewArrayRef(result) @@ -1041,7 +1056,7 @@ func parseSearchURI(rawuri string) (*keyAttributes, error) { } // When rawuri is a mackms uri. - u, err := uri.ParseWithScheme(Scheme, rawuri) + u, err := uri.Parse(rawuri) if err != nil { return nil, err } @@ -1066,11 +1081,9 @@ func parseSearchURI(rawuri string) (*keyAttributes, error) { tag = DefaultTag } return &keyAttributes{ - label: label, - tag: tag, - hash: u.GetEncoded("hash"), - useSecureEnclave: u.GetBool("se"), - useBiometrics: u.GetBool("bio"), + label: label, + tag: tag, + hash: u.GetEncoded("hash"), }, nil } diff --git a/kms/mackms/mackms_test.go b/kms/mackms/mackms_test.go index d233dc52..6bf27402 100644 --- a/kms/mackms/mackms_test.go +++ b/kms/mackms/mackms_test.go @@ -1239,7 +1239,15 @@ func TestMacKMS_SearchKeys(t *testing.T) { require.NoError(t, err) tag := fmt.Sprintf("com.smallstep.crypto.test.%s", name) // unique tag per test execution + // initialize MacKMS k := &MacKMS{} + + // search by tag; expect 0 keys before the test + got, err := k.SearchKeys(&apiv1.SearchKeysRequest{Query: fmt.Sprintf("mackms:tag=%s", tag)}) + require.NoError(t, err) + require.NotNil(t, got) + require.Len(t, got.Results, 0) + key1, err := k.CreateKey(&apiv1.CreateKeyRequest{Name: fmt.Sprintf("mackms:name=test-step-1;label=test-step-1;tag=%s;se=false", tag)}) require.NoError(t, err) key2, err := k.CreateKey(&apiv1.CreateKeyRequest{Name: fmt.Sprintf("mackms:name=test-step-2;label=test-step-2;tag=%s;se=false", tag)}) @@ -1258,7 +1266,7 @@ func TestMacKMS_SearchKeys(t *testing.T) { }) // search by tag - got, err := k.SearchKeys(&apiv1.SearchKeysRequest{Query: fmt.Sprintf("mackms:tag=%s", tag)}) + got, err = k.SearchKeys(&apiv1.SearchKeysRequest{Query: fmt.Sprintf("mackms:tag=%s", tag)}) require.NoError(t, err) require.NotNil(t, got) require.Len(t, got.Results, 2) From 606fcd9b12dd745665f5469b900de70568d31610 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 29 Jul 2024 13:07:00 +0200 Subject: [PATCH 5/5] Support filtering search query on being in Secure Enclave or not If `se` is not specified in the search query, all keys managed by the KMS (using the default tag) will be returned. When `se=true`, or `se=false`, keys will be filtered based on whether they were created inside the Secure Enclave or not, respectively. --- kms/mackms/mackms.go | 58 ++++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/kms/mackms/mackms.go b/kms/mackms/mackms.go index 9398075d..889bafe1 100644 --- a/kms/mackms/mackms.go +++ b/kms/mackms/mackms.go @@ -58,6 +58,14 @@ type keyAttributes struct { keySize int } +type keySearchAttributes struct { + label string + tag string + hash []byte + secureEnclaveSet bool + useSecureEnclave bool +} + type certAttributes struct { label string serialNumber *big.Int @@ -555,13 +563,17 @@ func (*MacKMS) DeleteCertificate(req *apiv1.DeleteCertificateRequest) error { return nil } -// SearchKeys searches for keys according to the query URI in the request. +// SearchKeys searches for keys according to the query URI in the request. By default, +// all keys managed by the KMS using the default tag, and both Secure Enclave as well as +// non-Secure Enclave keys will be returned. // // - "" will return all keys managed by the KMS (using the default tag) // - "mackms:" will return all keys managed by the KMS (using the default tag) // - "mackms:label=my-label" will return all keys using label "my-label" (and the default tag) // - "mackms:hash=the-hash" will return all keys having hash "hash" (and the default tag; generally one result) // - "mackms:tag=my-tag" will search for all keys with "my-tag" +// - "mackms:se=true" will return all Secure Enclave keys managed by the KMS (using the default tag) +// - "mackms:se=false" will return all non-Secure Enclave keys managed by the KMS (using the default tag) // // # Experimental // @@ -585,18 +597,30 @@ func (k *MacKMS) SearchKeys(req *apiv1.SearchKeysRequest) (*apiv1.SearchKeysResp results := make([]apiv1.SearchKeyResult, len(keys)) for i, key := range keys { d := cf.NewDictionaryRef(cf.TypeRef(key.TypeRef())) + defer key.Release() defer d.Release() - name := uri.New(Scheme, url.Values{ - "hash": []string{hex.EncodeToString(security.GetSecAttrApplicationLabel(d))}, - "label": []string{security.GetSecAttrLabel(d)}, - "tag": []string{security.GetSecAttrApplicationTag(d)}, - }) - - if tokenID := security.GetSecAttrTokenID(d); tokenID == "com.apple.setoken" { + name := uri.New(Scheme, url.Values{}) + tokenID := security.GetSecAttrTokenID(d) + keyInSecureEnclave := tokenID == "com.apple.setoken" + switch { + case !u.secureEnclaveSet && keyInSecureEnclave: + name.Values.Set("se", "true") + case !u.secureEnclaveSet && !keyInSecureEnclave: + name.Values.Set("se", "false") + case u.useSecureEnclave && keyInSecureEnclave: name.Values.Set("se", "true") + case !u.useSecureEnclave && !keyInSecureEnclave: + name.Values.Set("se", "false") + default: + // skip in case the query doesn't match the actual property + continue } + name.Values.Set("hash", hex.EncodeToString(security.GetSecAttrApplicationLabel(d))) + name.Values.Set("label", security.GetSecAttrLabel(d)) + name.Values.Set("tag", security.GetSecAttrApplicationTag(d)) + // obtain the public key by requesting it, as the current // representation of the key includes just the attributes. pub, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ @@ -692,7 +716,7 @@ func getPrivateKey(u *keyAttributes) (*security.SecKeyRef, error) { return security.NewSecKeyRef(key), nil } -func getPrivateKeys(u *keyAttributes) ([]*security.SecKeychainItemRef, error) { +func getPrivateKeys(u *keySearchAttributes) ([]*security.SecKeychainItemRef, error) { dict := cf.Dictionary{ security.KSecClass: security.KSecClassKey, security.KSecAttrKeyClass: security.KSecAttrKeyClassPrivate, @@ -1046,10 +1070,10 @@ func parseCertURI(rawuri string, requireValue bool) (*certAttributes, error) { }, nil } -func parseSearchURI(rawuri string) (*keyAttributes, error) { +func parseSearchURI(rawuri string) (*keySearchAttributes, error) { // When rawuri is just the key name if !strings.HasPrefix(strings.ToLower(rawuri), Scheme) { - return &keyAttributes{ + return &keySearchAttributes{ label: rawuri, tag: DefaultTag, }, nil @@ -1065,7 +1089,7 @@ func parseSearchURI(rawuri string) (*keyAttributes, error) { if len(u.Values) == 1 { for k, v := range u.Values { if (len(v) == 1 && v[0] == "") || len(v) == 0 { - return &keyAttributes{ + return &keySearchAttributes{ label: k, tag: DefaultTag, }, nil @@ -1080,10 +1104,12 @@ func parseSearchURI(rawuri string) (*keyAttributes, error) { if tag == "" { tag = DefaultTag } - return &keyAttributes{ - label: label, - tag: tag, - hash: u.GetEncoded("hash"), + return &keySearchAttributes{ + label: label, + tag: tag, + hash: u.GetEncoded("hash"), + secureEnclaveSet: u.Values.Has("se"), + useSecureEnclave: u.GetBool("se"), }, nil }