From 7a9ba71180abb557beca02b3f340c6af6c96f9b9 Mon Sep 17 00:00:00 2001 From: Josh Drake Date: Mon, 13 Oct 2025 15:43:33 -0500 Subject: [PATCH 01/13] Allow certificate retrieval by friendly name or description. --- kms/capi/capi.go | 81 +++++++++++++++++++++++++++++--------- kms/capi/ncrypt_windows.go | 76 +++++++++++++++++++++++++++++++++++ kms/tpmkms/uri.go | 6 ++- 3 files changed, 143 insertions(+), 20 deletions(-) diff --git a/kms/capi/capi.go b/kms/capi/capi.go index 22555195..761e3f93 100644 --- a/kms/capi/capi.go +++ b/kms/capi/capi.go @@ -44,6 +44,8 @@ const ( HashArg = "sha1" StoreLocationArg = "store-location" // 'machine', 'user', etc StoreNameArg = "store" // 'MY', 'CA', 'ROOT', etc + FriendlyNameArg = "friendly-name" + DescriptionArg = "description" IntermediateStoreLocationArg = "intermediate-store-location" IntermediateStoreNameArg = "intermediate-store" KeyIDArg = "key-id" @@ -91,6 +93,8 @@ type uriAttributes struct { subjectCN string serialNumber *big.Int issuerName string + friendlyName string + description string keySpec string skipFindCertificateKey bool pin string @@ -132,6 +136,8 @@ func parseURI(rawuri string) (*uriAttributes, error) { subjectCN: u.Get(SubjectCNArg), serialNumber: serialNumber, issuerName: u.Get(IssuerNameArg), + friendlyName: u.Get(FriendlyNameArg), + description: u.Get(DescriptionArg), keySpec: u.Get(KeySpec), skipFindCertificateKey: u.GetBool(SkipFindCertificateKey), pin: u.Pin(), @@ -397,6 +403,7 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) return nil, fmt.Errorf("CertOpenStore for the %q store %q returned: %w", u.storeLocation, u.storeName, err) } + canLookupByIssuer := issuerName != "" && (serialNumber != "" || subjectCN != "" || friendlyName != "" || description != "") var handle *windows.CertContext switch { @@ -424,6 +431,42 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) return nil, err } case u.issuerName != "" && (u.serialNumber != nil || u.subjectCN != ""): + handle, err = findCertificateInStore(st, + encodingX509ASN|encodingPKCS7, + 0, + findCertID, + uintptr(unsafe.Pointer(&searchData)), nil) + if err != nil { + return nil, fmt.Errorf("findCertificateInStore failed: %w", err) + } + if handle == nil { + return nil, apiv1.NotFoundError{Message: fmt.Sprintf("certificate with %s=%x not found", KeyIDArg, u.keyID)} + } + case u.containerName != "": + key, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ + Name: uri.New(Scheme, url.Values{ + ContainerNameArg: []string{u.containerName}, + }).String(), + }) + if err != nil { + return nil, err + } + keyID, err := x509util.GenerateSubjectKeyID(key) + if err != nil { + return nil, fmt.Errorf("error generating SubjectKeyID: %w", err) + } + if handle, err = findCertificateBySubjectKeyID(st, keyID); err != nil { + return nil, err + } + default: + return nil, fmt.Errorf("%q, %q, or %q and one of %q or %q is required to find a certificate", HashArg, KeyIDArg, IssuerNameArg, SerialNumberArg, SubjectCNArg) + } + + if handle != nil { + return handle, err + } + + if canLookupByIssuer { var prevCert *windows.CertContext for { handle, err = findCertificateInStore(st, @@ -456,31 +499,31 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) if x509Cert.Subject.CommonName == u.subjectCN { return handle, nil } + case len(friendlyName) > 0: + val, err := cryptFindCertificateFriendlyName(prevCert) + if err != nil { + return nil, fmt.Errorf("cryptFindCertificateFriendlyName failed: %w", err) + } + + if val == friendlyName { + return handle, nil + } + case len(description) > 0: + val, err := cryptFindCertificateDescription(prevCert) + if err != nil { + return nil, fmt.Errorf("cryptFindCertificateDescription failed: %w", err) + } + + if val == description { + return handle, nil + } } prevCert = handle } - case u.containerName != "": - key, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ - Name: uri.New(Scheme, url.Values{ - ContainerNameArg: []string{u.containerName}, - }).String(), - }) - if err != nil { - return nil, err - } - keyID, err := x509util.GenerateSubjectKeyID(key) - if err != nil { - return nil, fmt.Errorf("error generating SubjectKeyID: %w", err) - } - if handle, err = findCertificateBySubjectKeyID(st, keyID); err != nil { - return nil, err - } - default: + } else { return nil, fmt.Errorf("%q, %q, or %q and one of %q or %q is required to find a certificate", HashArg, KeyIDArg, IssuerNameArg, SerialNumberArg, SubjectCNArg) } - - return handle, err } // CreateSigner returns a crypto.Signer that will sign using the key passed in via the URI. diff --git a/kms/capi/ncrypt_windows.go b/kms/capi/ncrypt_windows.go index 0126d99c..f87dcf2f 100644 --- a/kms/capi/ncrypt_windows.go +++ b/kms/capi/ncrypt_windows.go @@ -62,9 +62,11 @@ const ( compareShift = 16 // CERT_COMPARE_SHIFT compareSHA1Hash = 1 // CERT_COMPARE_SHA1_HASH compareCertID = 16 // CERT_COMPARE_CERT_ID + compareProp = 5 // CERT_COMPARE_CERT_ID findIssuerStr = compareNameStrW< Date: Mon, 13 Oct 2025 16:19:49 -0500 Subject: [PATCH 02/13] Improve friendly name and description CAPI getters. --- kms/capi/ncrypt_windows.go | 75 +++++++++++++++----------------------- 1 file changed, 29 insertions(+), 46 deletions(-) diff --git a/kms/capi/ncrypt_windows.go b/kms/capi/ncrypt_windows.go index f87dcf2f..31df466c 100644 --- a/kms/capi/ncrypt_windows.go +++ b/kms/capi/ncrypt_windows.go @@ -637,36 +637,34 @@ func cryptFindCertificateKeyContainerName(certContext *windows.CertContext) (str return "", nil } -func cryptFindCertificateFriendlyName(certContext *windows.CertContext) (string, error) { - var size uint32 - - r1, _, err := procCertGetCertificateContextProperty.Call( +func certGetCertificateContextProperty(certContext *windows.CertContext, propID uint32, pvData *byte, pcbData *uint32) error { + r0, _, err := procCertGetCertificateContextProperty.Call( uintptr(unsafe.Pointer(certContext)), - uintptr(CERT_FRIENDLY_NAME_PROP_ID), - uintptr(0), - uintptr(unsafe.Pointer(&size)), + uintptr(propID), + uintptr(unsafe.Pointer(pvData)), + uintptr(unsafe.Pointer(pcbData)), ) - if !errors.Is(err, windows.Errno(0)) { - return "", fmt.Errorf("CertGetCertificateContextProperty returned %w", err) - } - if r1 == 0 { - return "", fmt.Errorf("finding certificate friendly name failed: %v", errNoToStr(uint32(r1))) + if r0 == 0 { + return err } + return nil +} - buf := make([]byte, size) - r2, _, err := procCertGetCertificateContextProperty.Call( - uintptr(unsafe.Pointer(certContext)), - uintptr(CERT_KEY_PROV_INFO_PROP_ID), - uintptr(0), - uintptr(unsafe.Pointer(&buf[0])), - ) +func cryptFindCertificateFriendlyName(certContext *windows.CertContext) (string, error) { + var size uint32 - if !errors.Is(err, windows.Errno(0)) { - return "", fmt.Errorf("CertGetCertificateContextProperty returned %w", err) + err := certGetCertificateContextProperty(certContext, CERT_FRIENDLY_NAME_PROP_ID, nil, &size) + if err != nil { + return "", err + } + if size == 0 { + return "", fmt.Errorf("certificate has no friendly name") } - if r2 == 0 { - return "", fmt.Errorf("finding certificate friendly name failed: %v", errNoToStr(uint32(r2))) + buf := make([]byte, size) + err = certGetCertificateContextProperty(certContext, CERT_FRIENDLY_NAME_PROP_ID, &buf[0], &size) + if err != nil { + return "", err } uc := bytes.ReplaceAll(buf, []byte{0x00}, []byte("")) @@ -676,33 +674,18 @@ func cryptFindCertificateFriendlyName(certContext *windows.CertContext) (string, func cryptFindCertificateDescription(certContext *windows.CertContext) (string, error) { var size uint32 - r1, _, err := procCertGetCertificateContextProperty.Call( - uintptr(unsafe.Pointer(certContext)), - uintptr(CERT_DESCRIPTION_PROP_ID), - uintptr(0), - uintptr(unsafe.Pointer(&size)), - ) - if !errors.Is(err, windows.Errno(0)) { - return "", fmt.Errorf("CertGetCertificateContextProperty returned %w", err) + err := certGetCertificateContextProperty(certContext, CERT_DESCRIPTION_PROP_ID, nil, &size) + if err != nil { + return "", err } - if r1 == 0 { - return "", fmt.Errorf("finding certificate description failed: %v", errNoToStr(uint32(r1))) + if size == 0 { + return "", fmt.Errorf("certificate has no description") } buf := make([]byte, size) - r2, _, err := procCertGetCertificateContextProperty.Call( - uintptr(unsafe.Pointer(certContext)), - uintptr(CERT_KEY_PROV_INFO_PROP_ID), - uintptr(0), - uintptr(unsafe.Pointer(&buf[0])), - ) - - if !errors.Is(err, windows.Errno(0)) { - return "", fmt.Errorf("CertGetCertificateContextProperty returned %w", err) - } - - if r2 == 0 { - return "", fmt.Errorf("finding certificate description failed: %v", errNoToStr(uint32(r2))) + err = certGetCertificateContextProperty(certContext, CERT_DESCRIPTION_PROP_ID, &buf[0], &size) + if err != nil { + return "", err } uc := bytes.ReplaceAll(buf, []byte{0x00}, []byte("")) From f459ee2704f3177be93170314587f5b459e10797 Mon Sep 17 00:00:00 2001 From: Josh Drake Date: Mon, 13 Oct 2025 16:34:06 -0500 Subject: [PATCH 03/13] Correctly handle NOT_FOUND errors for CAPI certificate props. --- kms/capi/capi.go | 4 ++-- kms/capi/ncrypt_windows.go | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/kms/capi/capi.go b/kms/capi/capi.go index 761e3f93..7323a84c 100644 --- a/kms/capi/capi.go +++ b/kms/capi/capi.go @@ -500,7 +500,7 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) return handle, nil } case len(friendlyName) > 0: - val, err := cryptFindCertificateFriendlyName(prevCert) + val, err := cryptFindCertificateFriendlyName(handle) if err != nil { return nil, fmt.Errorf("cryptFindCertificateFriendlyName failed: %w", err) } @@ -509,7 +509,7 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) return handle, nil } case len(description) > 0: - val, err := cryptFindCertificateDescription(prevCert) + val, err := cryptFindCertificateDescription(handle) if err != nil { return nil, fmt.Errorf("cryptFindCertificateDescription failed: %w", err) } diff --git a/kms/capi/ncrypt_windows.go b/kms/capi/ncrypt_windows.go index 31df466c..692672fa 100644 --- a/kms/capi/ncrypt_windows.go +++ b/kms/capi/ncrypt_windows.go @@ -62,7 +62,7 @@ const ( compareShift = 16 // CERT_COMPARE_SHIFT compareSHA1Hash = 1 // CERT_COMPARE_SHA1_HASH compareCertID = 16 // CERT_COMPARE_CERT_ID - compareProp = 5 // CERT_COMPARE_CERT_ID + compareProp = 5 // CERT_COMPARE_CERT_ID findIssuerStr = compareNameStrW< Date: Mon, 13 Oct 2025 17:37:55 -0500 Subject: [PATCH 04/13] Add friendly name and description CAPI setters. --- kms/capi/capi.go | 8 ++++++++ kms/capi/ncrypt_windows.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/kms/capi/capi.go b/kms/capi/capi.go index 7323a84c..cddd0c1d 100644 --- a/kms/capi/capi.go +++ b/kms/capi/capi.go @@ -861,6 +861,14 @@ func (k *CAPIKMS) StoreCertificate(req *apiv1.StoreCertificateRequest) error { cryptFindCertificateKeyProvInfo(certContext) } + if friendlyName := u.Get(FriendlyNameArg); friendlyName != "" { + cryptSetCertificateFriendlyName(certContext, friendlyName) + } + + if description := u.Get(DescriptionArg); description != "" { + cryptSetCertificateDescription(certContext, description) + } + st, err := windows.CertOpenStore( certStoreProvSystem, 0, diff --git a/kms/capi/ncrypt_windows.go b/kms/capi/ncrypt_windows.go index 692672fa..01ae5766 100644 --- a/kms/capi/ncrypt_windows.go +++ b/kms/capi/ncrypt_windows.go @@ -157,6 +157,7 @@ var ( procCertFindCertificateInStore = crypt32.MustFindProc("CertFindCertificateInStore") procCryptFindCertificateKeyProvInfo = crypt32.MustFindProc("CryptFindCertificateKeyProvInfo") procCertGetCertificateContextProperty = crypt32.MustFindProc("CertGetCertificateContextProperty") + procCertSetCertificateContextProperty = crypt32.MustFindProc("CertSetCertificateContextProperty") procCertStrToName = crypt32.MustFindProc("CertStrToNameW") ) @@ -637,6 +638,38 @@ func cryptFindCertificateKeyContainerName(certContext *windows.CertContext) (str return "", nil } +func certSetCertificateContextProperty(certContext *windows.CertContext, propID uint32, pvData uintptr) error { + r0, _, err := procCertSetCertificateContextProperty.Call( + uintptr(unsafe.Pointer(certContext)), + uintptr(propID), + 0, + pvData, + ) + + if r0 == 0 { + return err + } + return nil +} + +func cryptSetCertificateFriendlyName(certContext *windows.CertContext, val string) error { + data := CRYPTOAPI_BLOB{ + len: uint32(len(val)+1) * 2, + data: uintptr(unsafe.Pointer(wide(val))), + } + + return certSetCertificateContextProperty(certContext, CERT_FRIENDLY_NAME_PROP_ID, uintptr(unsafe.Pointer(&data))) +} + +func cryptSetCertificateDescription(certContext *windows.CertContext, val string) error { + data := CRYPTOAPI_BLOB{ + len: uint32(len(val)+1) * 2, + data: uintptr(unsafe.Pointer(wide(val))), + } + + return certSetCertificateContextProperty(certContext, CERT_DESCRIPTION_PROP_ID, uintptr(unsafe.Pointer(&data))) +} + func certGetCertificateContextProperty(certContext *windows.CertContext, propID uint32, pvData *byte, pcbData *uint32) error { r0, _, err := procCertGetCertificateContextProperty.Call( uintptr(unsafe.Pointer(certContext)), From 2d843f0edaf414b370f07d41101c3be11e23c21a Mon Sep 17 00:00:00 2001 From: Josh Drake Date: Mon, 13 Oct 2025 17:39:01 -0500 Subject: [PATCH 05/13] Pass through 'friendly-name' and 'description' URI params to CAPI KMS. --- kms/tpmkms/tpmkms.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kms/tpmkms/tpmkms.go b/kms/tpmkms/tpmkms.go index f766e4e0..18221343 100644 --- a/kms/tpmkms/tpmkms.go +++ b/kms/tpmkms/tpmkms.go @@ -967,6 +967,8 @@ func (k *TPMKMS) storeCertificateChainToWindowsCertificateStore(req *apiv1.Store Name: uri.New("capi", url.Values{ "store-location": []string{location}, "store": []string{store}, + "friendly-name": []string{o.friendlyName}, + "description": []string{o.description}, "skip-find-certificate-key": []string{skipFindCertificateKey}, "intermediate-store-location": []string{intermediateCAStoreLocation}, "intermediate-store": []string{intermediateCAStore}, From e7322a2590b070cf6250d12aef8ba79b52e7a9eb Mon Sep 17 00:00:00 2001 From: Josh Drake Date: Mon, 13 Oct 2025 17:51:06 -0500 Subject: [PATCH 06/13] Handle additional parameters in Windows certificate chain loading. --- kms/tpmkms/tpmkms.go | 2 ++ kms/tpmkms/uri.go | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/kms/tpmkms/tpmkms.go b/kms/tpmkms/tpmkms.go index 18221343..9ed3bde0 100644 --- a/kms/tpmkms/tpmkms.go +++ b/kms/tpmkms/tpmkms.go @@ -870,6 +870,8 @@ func (k *TPMKMS) loadCertificateChainFromWindowsCertificateStore(req *apiv1.Load "store": []string{store}, "intermediate-store-location": []string{intermediateCAStoreLocation}, "intermediate-store": []string{intermediateCAStore}, + "friendly-name": []string{o.friendlyName}, + "description": []string{o.description}, }).String(), }) } diff --git a/kms/tpmkms/uri.go b/kms/tpmkms/uri.go index d3cb4f05..567f1e1e 100644 --- a/kms/tpmkms/uri.go +++ b/kms/tpmkms/uri.go @@ -59,8 +59,10 @@ func parseNameURI(nameURI string) (o objectProperties, err error) { o.qualifyingData = qualifyingData } - // store location, store options are used on Windows to override + // store location and store options are used on Windows to override // which store(s) are used for storing and loading (intermediate) certificates + // friendly-name and description are used on Windows to populate additional certificate + // context properties to aid in retrieval o.storeLocation = u.Get("store-location") o.store = u.Get("store") o.friendlyName = u.Get("friendly-name") From de60c2ad32f68a298ecf402dd88bc1f4c9c313aa Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Tue, 9 Dec 2025 16:21:11 -0300 Subject: [PATCH 07/13] Attempt to lookup certificate by issuer when key-id lookup fails This fixes an issue with agent, where it was unable to find device certificates due to using a key-id derived from a random string for certificate lookups. If the key-id lookup fails automatically attempt lookup by cert+attr where attr can be one of the certificate atributes, serial number, friendly-name, description, etc. --- kms/capi/capi.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/kms/capi/capi.go b/kms/capi/capi.go index cddd0c1d..66ab7d86 100644 --- a/kms/capi/capi.go +++ b/kms/capi/capi.go @@ -436,11 +436,12 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) 0, findCertID, uintptr(unsafe.Pointer(&searchData)), nil) - if err != nil { + if err != nil && !canLookupByIssuer() { return nil, fmt.Errorf("findCertificateInStore failed: %w", err) } - if handle == nil { - return nil, apiv1.NotFoundError{Message: fmt.Sprintf("certificate with %s=%x not found", KeyIDArg, u.keyID)} + + if handle == nil && !canLookupByIssuer { + return nil, apiv1.NotFoundError{Message: fmt.Sprintf("certificate with %s=%s not found", KeyIDArg, u.keyID)} } case u.containerName != "": key, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ @@ -499,22 +500,22 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) if x509Cert.Subject.CommonName == u.subjectCN { return handle, nil } - case len(friendlyName) > 0: + case len(u.friendlyName) > 0: val, err := cryptFindCertificateFriendlyName(handle) if err != nil { return nil, fmt.Errorf("cryptFindCertificateFriendlyName failed: %w", err) } - if val == friendlyName { + if val == u.friendlyName { return handle, nil } - case len(description) > 0: + case len(u.description) > 0: val, err := cryptFindCertificateDescription(handle) if err != nil { return nil, fmt.Errorf("cryptFindCertificateDescription failed: %w", err) } - if val == description { + if val == u.description { return handle, nil } } From ee6fd1a3638a4fb0b64f147e15c3eca3a15469cd Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Thu, 8 Jan 2026 11:20:35 -0300 Subject: [PATCH 08/13] turn `canLookupByIssuer` into variable The function was a relic from past refactoring code, not necessary anymore. --- kms/capi/capi.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kms/capi/capi.go b/kms/capi/capi.go index 66ab7d86..dda66f56 100644 --- a/kms/capi/capi.go +++ b/kms/capi/capi.go @@ -398,12 +398,13 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) 0, 0, certStoreLocation, - uintptr(unsafe.Pointer(wide(u.storeName)))) + uintptr(unsafe.Pointer(wide(u.storeName))), + ) if err != nil { return nil, fmt.Errorf("CertOpenStore for the %q store %q returned: %w", u.storeLocation, u.storeName, err) } - canLookupByIssuer := issuerName != "" && (serialNumber != "" || subjectCN != "" || friendlyName != "" || description != "") + canLookupByIssuer := u.issuerName != "" && (u.serialNumber != "" || u.subjectCN != "" || u.friendlyName != "" || u.description != "") var handle *windows.CertContext switch { From 54614602dd76cb9a76ba2e8992d4ab3a1ea6c001 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Thu, 8 Jan 2026 18:27:56 -0300 Subject: [PATCH 09/13] Fix syntax and logic errors after rebasing - Wrong variable names. - Added missing forwarding issuer,description,friendly-name params to CAPI. --- kms/capi/capi.go | 12 +++++++----- kms/tpmkms/tpmkms.go | 15 +++++++++------ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/kms/capi/capi.go b/kms/capi/capi.go index dda66f56..8ce7414c 100644 --- a/kms/capi/capi.go +++ b/kms/capi/capi.go @@ -437,7 +437,7 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) 0, findCertID, uintptr(unsafe.Pointer(&searchData)), nil) - if err != nil && !canLookupByIssuer() { + if err != nil && !canLookupByIssuer { return nil, fmt.Errorf("findCertificateInStore failed: %w", err) } @@ -863,12 +863,12 @@ func (k *CAPIKMS) StoreCertificate(req *apiv1.StoreCertificateRequest) error { cryptFindCertificateKeyProvInfo(certContext) } - if friendlyName := u.Get(FriendlyNameArg); friendlyName != "" { - cryptSetCertificateFriendlyName(certContext, friendlyName) + if u.friendlyName != "" { + cryptSetCertificateFriendlyName(certContext, u.friendlyName) } - if description := u.Get(DescriptionArg); description != "" { - cryptSetCertificateDescription(certContext, description) + if u.description != "" { + cryptSetCertificateDescription(certContext, u.description) } st, err := windows.CertOpenStore( @@ -906,6 +906,8 @@ func (k *CAPIKMS) StoreCertificateChain(req *apiv1.StoreCertificateChainRequest) HashArg: []string{fp}, StoreLocationArg: []string{u.storeLocation}, StoreNameArg: []string{u.storeName}, + FriendlyNameArg: []string{u.friendlyName}, + DescriptionArg: []string{u.description}, SkipFindCertificateKey: []string{strconv.FormatBool(u.skipFindCertificateKey)}, }).String(), Certificate: leaf, diff --git a/kms/tpmkms/tpmkms.go b/kms/tpmkms/tpmkms.go index 9ed3bde0..d7f80a52 100644 --- a/kms/tpmkms/tpmkms.go +++ b/kms/tpmkms/tpmkms.go @@ -870,6 +870,7 @@ func (k *TPMKMS) loadCertificateChainFromWindowsCertificateStore(req *apiv1.Load "store": []string{store}, "intermediate-store-location": []string{intermediateCAStoreLocation}, "intermediate-store": []string{intermediateCAStore}, + "issuer": []string{o.issuer}, "friendly-name": []string{o.friendlyName}, "description": []string{o.description}, }).String(), @@ -1548,9 +1549,11 @@ type deletingCertificateChainManager interface { DeleteCertificate(req *apiv1.DeleteCertificateRequest) error } -var _ apiv1.KeyManager = (*TPMKMS)(nil) -var _ apiv1.Attester = (*TPMKMS)(nil) -var _ apiv1.CertificateManager = (*TPMKMS)(nil) -var _ apiv1.CertificateChainManager = (*TPMKMS)(nil) -var _ deletingCertificateChainManager = (*TPMKMS)(nil) -var _ apiv1.AttestationClient = (*attestationClient)(nil) +var ( + _ apiv1.KeyManager = (*TPMKMS)(nil) + _ apiv1.Attester = (*TPMKMS)(nil) + _ apiv1.CertificateManager = (*TPMKMS)(nil) + _ apiv1.CertificateChainManager = (*TPMKMS)(nil) + _ deletingCertificateChainManager = (*TPMKMS)(nil) + _ apiv1.AttestationClient = (*attestationClient)(nil) +) From cb11e6996c1d7903c29216a5e7305e97e4a7252f Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Tue, 13 Jan 2026 12:01:56 -0300 Subject: [PATCH 10/13] cleanup certificate lookup code - Document lookup by issuer strategy. - Attempt lookup by issuer only when lookup by KeyID fails with not found, on error return. - Simplify code a little bit. --- kms/capi/capi.go | 114 ++++++++++++++++++++++------------------------- 1 file changed, 53 insertions(+), 61 deletions(-) diff --git a/kms/capi/capi.go b/kms/capi/capi.go index 8ce7414c..1f41f5c4 100644 --- a/kms/capi/capi.go +++ b/kms/capi/capi.go @@ -404,6 +404,10 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) return nil, fmt.Errorf("CertOpenStore for the %q store %q returned: %w", u.storeLocation, u.storeName, err) } + // if issuer + any of the other fields in the list below is provided, then attempt a second certificate lookup when + // lookup by KeyID fails (not found). This fix an issue when looking up device certificates, as in that case the KeyID is + // derived from a randomly generate string each time agent runs, thus not being able to find certificates installed from + // a previous run. canLookupByIssuer := u.issuerName != "" && (u.serialNumber != "" || u.subjectCN != "" || u.friendlyName != "" || u.description != "") var handle *windows.CertContext @@ -431,19 +435,6 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) if handle, err = findCertificateBySubjectKeyID(st, u.keyID); err != nil { return nil, err } - case u.issuerName != "" && (u.serialNumber != nil || u.subjectCN != ""): - handle, err = findCertificateInStore(st, - encodingX509ASN|encodingPKCS7, - 0, - findCertID, - uintptr(unsafe.Pointer(&searchData)), nil) - if err != nil && !canLookupByIssuer { - return nil, fmt.Errorf("findCertificateInStore failed: %w", err) - } - - if handle == nil && !canLookupByIssuer { - return nil, apiv1.NotFoundError{Message: fmt.Sprintf("certificate with %s=%s not found", KeyIDArg, u.keyID)} - } case u.containerName != "": key, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ Name: uri.New(Scheme, url.Values{ @@ -468,63 +459,64 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) return handle, err } - if canLookupByIssuer { - var prevCert *windows.CertContext - for { - handle, err = findCertificateInStore(st, - encodingX509ASN|encodingPKCS7, - 0, - findIssuerStr, - uintptr(unsafe.Pointer(wide(u.issuerName))), prevCert) + if !canLookupByIssuer { + return nil, fmt.Errorf("%q, %q, or %q and one of %q or %q is required to find a certificate", HashArg, KeyIDArg, IssuerNameArg, SerialNumberArg, SubjectCNArg) + } + + // lookup certificate by issuer + another field (serial, CN, friendlyName, description) + var prevCert *windows.CertContext + for { + handle, err = findCertificateInStore(st, + encodingX509ASN|encodingPKCS7, + 0, + findIssuerStr, + uintptr(unsafe.Pointer(wide(u.issuerName))), prevCert) + if err != nil { + return nil, fmt.Errorf("findCertificateInStore failed: %w", err) + } + + if handle == nil { + return nil, apiv1.NotFoundError{Message: fmt.Sprintf("certificate with %s=%q not found", IssuerNameArg, u.issuerName)} + } + + x509Cert, err := certContextToX509(handle) + if err != nil { + return nil, fmt.Errorf("could not unmarshal certificate to DER: %w", err) + } + + switch { + case u.serialNumber != nil: + // TODO: Replace this search with a CERT_ID + CERT_ISSUER_SERIAL_NUMBER search instead + // https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/ns-wincrypt-cert_id + // https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/ns-wincrypt-cert_issuer_serial_number + if x509Cert.SerialNumber.Cmp(u.serialNumber) == 0 { + return handle, nil + } + case len(u.subjectCN) > 0: + if x509Cert.Subject.CommonName == u.subjectCN { + return handle, nil + } + case len(u.friendlyName) > 0: + val, err := cryptFindCertificateFriendlyName(handle) if err != nil { - return nil, fmt.Errorf("findCertificateInStore failed: %w", err) + return nil, fmt.Errorf("cryptFindCertificateFriendlyName failed: %w", err) } - if handle == nil { - return nil, apiv1.NotFoundError{Message: fmt.Sprintf("certificate with %s=%q not found", IssuerNameArg, u.issuerName)} + if val == u.friendlyName { + return handle, nil } - - x509Cert, err := certContextToX509(handle) + case len(u.description) > 0: + val, err := cryptFindCertificateDescription(handle) if err != nil { - return nil, fmt.Errorf("could not unmarshal certificate to DER: %w", err) + return nil, fmt.Errorf("cryptFindCertificateDescription failed: %w", err) } - switch { - case u.serialNumber != nil: - // TODO: Replace this search with a CERT_ID + CERT_ISSUER_SERIAL_NUMBER search instead - // https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/ns-wincrypt-cert_id - // https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/ns-wincrypt-cert_issuer_serial_number - if x509Cert.SerialNumber.Cmp(u.serialNumber) == 0 { - return handle, nil - } - case len(u.subjectCN) > 0: - if x509Cert.Subject.CommonName == u.subjectCN { - return handle, nil - } - case len(u.friendlyName) > 0: - val, err := cryptFindCertificateFriendlyName(handle) - if err != nil { - return nil, fmt.Errorf("cryptFindCertificateFriendlyName failed: %w", err) - } - - if val == u.friendlyName { - return handle, nil - } - case len(u.description) > 0: - val, err := cryptFindCertificateDescription(handle) - if err != nil { - return nil, fmt.Errorf("cryptFindCertificateDescription failed: %w", err) - } - - if val == u.description { - return handle, nil - } + if val == u.description { + return handle, nil } - - prevCert = handle } - } else { - return nil, fmt.Errorf("%q, %q, or %q and one of %q or %q is required to find a certificate", HashArg, KeyIDArg, IssuerNameArg, SerialNumberArg, SubjectCNArg) + + prevCert = handle } } From b42b53683ceabe2a90b7671d503a6d7e9d449e30 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Tue, 24 Mar 2026 14:44:11 -0300 Subject: [PATCH 11/13] Attempt cert lookup by issuer+other_field when initial lookup fail If lookup by keyID or container fail and issuer+(serial|friendly_name) or the other fields specified are provided, then attempt another certificate lookup using those. --- kms/capi/capi.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/kms/capi/capi.go b/kms/capi/capi.go index 1f41f5c4..fc0da60b 100644 --- a/kms/capi/capi.go +++ b/kms/capi/capi.go @@ -435,6 +435,10 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) if handle, err = findCertificateBySubjectKeyID(st, u.keyID); err != nil { return nil, err } + + if handle == nil && !canLookupByIssuer { + return nil, apiv1.NotFoundError{Message: fmt.Sprintf("certificate with %s=%s not found", KeyIDArg, u.keyID)} + } case u.containerName != "": key, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ Name: uri.New(Scheme, url.Values{ @@ -451,8 +455,10 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) if handle, err = findCertificateBySubjectKeyID(st, keyID); err != nil { return nil, err } - default: - return nil, fmt.Errorf("%q, %q, or %q and one of %q or %q is required to find a certificate", HashArg, KeyIDArg, IssuerNameArg, SerialNumberArg, SubjectCNArg) + + if handle == nil && !canLookupByIssuer { + return nil, apiv1.NotFoundError{Message: fmt.Sprintf("certificate with %s=%s not found", KeyIDArg, u.keyID)} + } } if handle != nil { From c899aebc451e149c1c829c85794fb38485178fa3 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Tue, 24 Mar 2026 15:04:30 -0300 Subject: [PATCH 12/13] Fix serial number comparison The serial object was updated to be *big.Int instead of string. --- kms/capi/capi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kms/capi/capi.go b/kms/capi/capi.go index fc0da60b..38a49bbe 100644 --- a/kms/capi/capi.go +++ b/kms/capi/capi.go @@ -408,7 +408,7 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) // lookup by KeyID fails (not found). This fix an issue when looking up device certificates, as in that case the KeyID is // derived from a randomly generate string each time agent runs, thus not being able to find certificates installed from // a previous run. - canLookupByIssuer := u.issuerName != "" && (u.serialNumber != "" || u.subjectCN != "" || u.friendlyName != "" || u.description != "") + canLookupByIssuer := u.issuerName != "" && (u.serialNumber != nil || u.subjectCN != "" || u.friendlyName != "" || u.description != "") var handle *windows.CertContext switch { From dcefc6c95f950e502fa0b046309e666dc498e002 Mon Sep 17 00:00:00 2001 From: Diego Fronza Date: Tue, 24 Mar 2026 15:46:29 -0300 Subject: [PATCH 13/13] Fix cert lookup fallback when findCertificateBySubjectKeyID returns NotFoundError Previously, any error from findCertificateBySubjectKeyID would cause an immediate return, preventing the issuer-based fallback from running. Now only non-NotFoundError errors (or NotFoundError when issuer lookup is unavailable) are returned early. Co-Authored-By: Claude --- kms/capi/capi.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/kms/capi/capi.go b/kms/capi/capi.go index 38a49bbe..3f44b143 100644 --- a/kms/capi/capi.go +++ b/kms/capi/capi.go @@ -433,11 +433,9 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) } case len(u.keyID) > 0: if handle, err = findCertificateBySubjectKeyID(st, u.keyID); err != nil { - return nil, err - } - - if handle == nil && !canLookupByIssuer { - return nil, apiv1.NotFoundError{Message: fmt.Sprintf("certificate with %s=%s not found", KeyIDArg, u.keyID)} + if !errors.Is(err, apiv1.NotFoundError{}) || !canLookupByIssuer { + return nil, err + } } case u.containerName != "": key, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ @@ -453,11 +451,9 @@ func (k *CAPIKMS) getCertContext(u *uriAttributes) (*windows.CertContext, error) return nil, fmt.Errorf("error generating SubjectKeyID: %w", err) } if handle, err = findCertificateBySubjectKeyID(st, keyID); err != nil { - return nil, err - } - - if handle == nil && !canLookupByIssuer { - return nil, apiv1.NotFoundError{Message: fmt.Sprintf("certificate with %s=%s not found", KeyIDArg, u.keyID)} + if !errors.Is(err, apiv1.NotFoundError{}) || !canLookupByIssuer { + return nil, err + } } }