capi: machine-keyset lookups and ImportKey by darkfronza · Pull Request #1145 · smallstep/crypto

darkfronza · 2026-10-13T19:26:26Z

Two changes to kms/capi: keys in the machine keyset stay addressable through the URIs and lookups that name them, and an existing private key, such as one kept in a key file, can be imported into the software key storage provider.

Machine-keyset keys stay addressable

A key in the machine keyset can only be opened with NCRYPT_MACHINE_KEY_FLAG, but two paths dropped the keyset and failed with NTE_BAD_KEYSET:

CreateKey returned a URI naming only the provider and container, so CreateSigner on that URI opened the user keyset.
LoadCertificate, LoadCertificateChain and DeleteCertificate, given a key= container, opened it without the keyset to derive the subject key identifier.

CreateKey now carries store-location in the URI it returns. A new containerKeyID helper forwards the machine keyset to the container lookup. user is not forwarded, because providers whose keys are machine-wide refuse it, so user-scope lookups behave as before. A certificate in a machine store can be bound to a user-keyset key, so when key-scope is unset and the machine keyset has no such key, the user's keyset is searched too. An explicit key-scope is followed alone.

`ImportKey`

CAPIKMS.ImportKey(*ImportKeyRequest) (*apiv1.CreateKeyResponse, error) stores an existing ECDSA or RSA private key in a named container of the Microsoft Software Key Storage Provider, in the user or machine keyset the URI selects. It lets the provider hold a key that also lives elsewhere, such as a key file.

The key goes through NCryptImportKey with the container name as a parameter, unfinalized. Its export policy is set to 0 before finalizing, so the provider holds it non-exportable.
ECDSA keys are imported as BCRYPT_ECCPRIVATE_BLOB with the ECDSA private magic, because as PKCS#8 the KSP files them under the ECDH algorithm group, where they cannot sign. RSA keys are imported as PKCS#8.
An existing container is refused with apiv1.AlreadyExistsError, because the KSP would otherwise silently replace its key. Only a container that doesn't exist (NTE_BAD_KEYSET) is imported; any other failure to open it is returned. A key that fails after being persisted is deleted.
A key-spec on the URI is refused, an incomplete key is refused, and the import blob is cleared once the provider has its copy.
Only the software KSP is supported so far; other providers take other blob formats and aren't covered yet. Ed25519 is not supported: CNG has no Ed25519.

`DeleteKey` handle fix

nCryptDeleteKey now consumes the key handle whatever the outcome: NCryptDeleteKey frees it on success, and it is freed here on failure. DeleteKey freed it a second time, and the cleanup paths leaked it when the deletion failed.

Testing

The CNG tests are in capi_windows_test.go and only run on Windows, while CI runs tests on Linux. They were run on a Windows VM, as administrator for the machine-keyset cases. This is why ImportKey is marked EXPERIMENTAL.

TestCAPIKMS_machineKeyset
TestCAPIKMS_containerKeyset: a machine-store certificate bound to a user-keyset key is found with no key-scope and with key-scope=user, but not with key-scope=machine.
TestCAPIKMS_ImportKey: P-256, P-384 and P-521, and RSA 2048, 3072 and 4096, in the user keyset; ECDSA in the machine keyset. Each key must sign, verify against its public key, refuse export, and refuse a second import with AlreadyExistsError.
TestCAPIKMS_ImportKey_refusals: no key, no container, Ed25519, a typed-nil or empty ECDSA key, a typed-nil RSA key, a key-spec, and a non-software provider.

go test ./kms/capi/ ./kms/tpmkms/ passes on the VM. TestKMS_CleanupCredentials_capi in kms/platform fails ("CNG private key should be removed") on this branch, on its base, and on master alike, so it isn't caused by this change.

The govulncheck failure is the golang.org/x/net v0.59.0 advisories, unrelated to this change.

🤖 Generated with Claude Code

A key created in the machine keyset can only be opened with NCRYPT_MACHINE_KEY_FLAG, but two paths dropped the keyset: - CreateKey returned a URI naming the provider and container only, so CreateSigner on that URI opened the user keyset and failed with NTE_BAD_KEYSET. - LoadCertificate, LoadCertificateChain and DeleteCertificate, given a "key" container, opened it to derive the subject key identifier with the container alone, so a machine-store lookup by key failed the same way. CreateKey now carries store-location in the returned URI, and the container lookup forwards the machine keyset (key-scope, defaulting to store-location). "user" is not forwarded because providers whose keys are machine-wide refuse it, so user-scope lookups behave as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ImportKey persists an existing ECDSA or RSA private key in a named container of the Microsoft Software Key Storage Provider, in the user or machine keyset the URI selects, where CreateKey would generate a new one. It lets the provider hold a key that also lives somewhere else, such as a key file. The key goes through NCryptImportKey with the container name in the parameter list, and is finalized only after its export policy is cleared, so the provider holds it non-exportable. ECDSA keys are imported as a BCRYPT_ECCPRIVATE_BLOB with the ECDSA private magic: as PKCS#8 the KSP files them under the ECDH algorithm group, where they cannot sign. RSA keys are imported as PKCS#8. An existing container is refused before the import, since the KSP otherwise replaces the key in it, and a key that fails after being persisted is deleted. The response has the same shape as CreateKey's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ImportKey is now exercised with P-521, RSA 3072 and RSA 4096 keys as well. The export policy is a DWORD, so the test decodes it from exactly four bytes: nCryptGetPropertyInt reads a whole int, whose upper half is whatever follows the property in memory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

dopey

No blocking issues. Notes:

containerKeyID follows isMachineKeyset(), so it respects key-scope as well as store-location. A certificate in the machine store whose key is explicitly key-scope=user still opens in the user keyset.
All new tests are in capi_windows_test.go, and CI runs tests only on Linux. ImportKey's behavior (ECC private blob with the key name, export policy 0, NTE_EXISTS refusal) is covered only by manual VM runs. That's reasonable for an EXPERIMENTAL method, but the PR description should say so.
Not from this PR: CAPIKMS.DeleteKey calls nCryptDeleteKey(kh) and also defers nCryptFreeObject(kh). The comment this PR adds in ImportKey says nCryptDeleteKey frees the handle, so that's a double free. A caller that replaces an imported key goes through DeleteKey, so it may be worth fixing here or in a follow-up.
The govulncheck failure is the golang.org/x/net@v0.59.0 advisories, unrelated to this change.

hslatman · 2026-10-20T13:18:23Z

+// container that already exists is not replaced; the import fails with
+// NTE_EXISTS.
+//
+// Only the Microsoft Software Key Storage Provider is supported.


Is this (just) a limitation of the current state of code? It is possible to import a key to a TPM, so that may be possible on Windows too. I haven't looked into this yet, though.

It's a scope limit, not something Windows rules out. The agent only needs the software KSP, and I haven't tried importing into the Platform Crypto Provider. The PCP may take some imports, but its blob formats and its non-exportable handling differ, so it would need its own code and tests. The doc comment now says other providers "are not covered yet" instead of implying they can't be (90d55b0). Happy to open a follow-up issue for PCP import.

hslatman · 2026-10-20T13:21:16Z

+	ecs2Magic = 0x32534345 // "ECS2" BCRYPT_ECDSA_PRIVATE_P256_MAGIC
+	ecs4Magic = 0x34534345 // "ECS4" BCRYPT_ECDSA_PRIVATE_P384_MAGIC
+	ecs6Magic = 0x36534345 // "ECS6" BCRYPT_ECDSA_PRIVATE_P521_MAGIC


These can be ordered numerically with the three above

Done in aa8ae99: RSA1, then ECS1 through ECS6.

hslatman · 2026-10-20T13:28:55Z

+	if u.isMachineKeyset() {
+		keyName.Set(StoreLocationArg, MachineStoreLocation)


This looks like a small behavior change that could break certain keys, depending on their configuration. Is this an intended change for this PR?

Good catch, it was a behavior change. A certificate in a machine store can be bound to a key in the user's keyset, and with no key-scope the lookup now opened only the machine keyset, so that key wasn't found. In 69d24bb, when key-scope is unset and the machine keyset has no such key (NTE_BAD_KEYSET), the user's keyset is searched as before. An explicit key-scope is still followed alone. TestCAPIKMS_containerKeyset covers a machine-store certificate with a user-keyset key: no key-scope and key-scope=user find it, key-scope=machine doesn't.

hslatman · 2026-10-20T13:32:12Z

+		return nil, fmt.Errorf("%v not specified", ContainerNameArg)
+	}
+
+	blobType, blob, err := privateKeyBlob(req.PrivateKey)


The blob contains the key material. Clear it as soon as it's no longer required.

Done in 90d55b0: defer clear(blob) right after building it, for both the ECC blob and the RSA PKCS#8 bytes. The caller's own crypto.PrivateKey still holds the key; that's outside what ImportKey can clear.

hslatman · 2026-10-20T13:34:07Z

+	switch k := key.(type) {
+	case *ecdsa.PrivateKey:


A nil key can be passed, and can panic in here

Fixed in af8c09c. A typed nil, or a key missing its components, now returns "incomplete ECDSA/RSA private key" instead of panicking. The refusal test covers (*ecdsa.PrivateKey)(nil), &ecdsa.PrivateKey{} and (*rsa.PrivateKey)(nil).

hslatman · 2026-10-20T13:42:05Z

+	runtime.KeepAlive(name)
+	runtime.KeepAlive(&buffer)
+	runtime.KeepAlive(blob)
+	if r != 0 {


Should param be added to these?

Added runtime.KeepAlive(&params) in aa8ae99. params is passed directly as a call argument, so it was already kept alive for the call, but now it matches the other three.

hslatman · 2026-10-20T13:45:28Z

+
+	// Checked first because the import does not: given a container that
+	// already exists, the software KSP replaces the key in it.
+	if existing, err := nCryptOpenKey(k.providerHandle, u.keyContainerName, 0, keyFlags); err == nil {


Besides err == nil, should this check for other types of errors while trying to open the key container, instead of continuing to import?

Yes, fixed in 90d55b0. Only NTE_BAD_KEYSET, meaning the container isn't there, goes on to import. Any other failure to open it, such as access denied, is returned.

hslatman · 2026-10-20T13:46:39Z

+	// already exists, the software KSP replaces the key in it.
+	if existing, err := nCryptOpenKey(k.providerHandle, u.keyContainerName, 0, keyFlags); err == nil {
+		nCryptFreeObject(existing)
+		return nil, fmt.Errorf("unable to import key: container %q returned %v", u.keyContainerName, errNoToStr(NTE_EXISTS))


There's an apiv1.AlreadyExistsError that can be returned from this.

Done in 90d55b0: an existing container now returns apiv1.AlreadyExistsError, still with NTE_EXISTS in the message, and the test asserts ErrorIs(AlreadyExistsError{}). I also added capi to the list in its doc comment.

hslatman · 2026-10-20T13:53:13Z

+	if u.storeLocation != "" {
+		importedKeyURI += fmt.Sprintf(";%s=%s", StoreLocationArg, u.storeLocation)


parseURI always sets storeLocation, so the if can be removed

Right, it defaults to user. Removed in 90d55b0, so the URI always carries store-location. The output is unchanged.

hslatman · 2026-10-20T13:53:48Z

+	if u.storeLocation != "" {
+		createdKeyURI += fmt.Sprintf(";%s=%s", StoreLocationArg, u.storeLocation)


parseURI always sets storeLocation, so the if can be removed

Same here, removed in 90d55b0.

… alive Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A typed nil *ecdsa.PrivateKey or *rsa.PrivateKey, or one missing its components, passed the nil check on the request and panicked while building the import blob. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

NCryptDeleteKey frees the handle it deletes, and DeleteKey also freed it afterwards. nCryptDeleteKey now consumes the handle whatever the outcome, freeing it itself when the deletion fails, so DeleteKey, the cleanup paths of ImportKey and the certificate cleanup neither free it twice nor leak it. It also decides the outcome by the status NCryptDeleteKey returns rather than the thread's last error, which is not part of its contract. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- Only a container that does not exist is imported. A failure to open it other than NTE_BAD_KEYSET, such as access denied, is returned rather than taken for a missing container. - An existing container is refused with apiv1.AlreadyExistsError. - A key-spec on the URI is refused: an imported key has no legacy key spec, and ignoring it would hand back a key that is not what was asked. - The import blob, which holds the private key, is cleared once the provider has its copy. - CreateKey and ImportKey always add the store-location to the URI they return; parseURI defaults it to user, so it is never empty. The doc comment says other providers are not supported yet, rather than that they cannot be. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A certificate in a machine store can be bound to a key in the user's keyset. Looking it up by key container opened the machine keyset whenever the store was the machine's, so such a key was not found. With no key-scope, a key missing from the machine keyset is now looked up in the user's keyset as well; an explicit key-scope is followed alone. The tests cover the lookup with and without key-scope, ImportKey's refusals of an incomplete key and a key-spec, and AlreadyExistsError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

darkfronza · 2026-10-23T21:03:23Z

Thanks @dopey, addressed:

key-scope in containerKeyID: kept. With an explicit key-scope=user, a machine-store certificate's key opens in the user keyset. With no key-scope, the lookup now also falls back to the user's keyset when the machine keyset has no such key (69d24bb). See the thread on containerKeyID.
Windows-only tests: the PR description's Testing section now says the CNG tests only run on Windows, were run on a VM (as administrator for the machine-keyset cases), and that this is why ImportKey is EXPERIMENTAL.
DeleteKey double free: fixed here in dfcd5d4. nCryptDeleteKey now consumes the handle whatever the outcome. NCryptDeleteKey frees it on success, and it is freed here on failure. So DeleteKey no longer frees it twice, and ImportKey's cleanup and the certificate cleanup no longer leak it when the deletion fails. It also decides the outcome from the returned status rather than the thread's last error.
govulncheck: agreed it's unrelated (x/net v0.59.0). I've left it out of this PR.

Tested on a Windows VM: go test ./kms/capi/ ./kms/tpmkms/ passes. TestKMS_CleanupCredentials_capi in kms/platform fails the same way on this branch, its base and master, so it's unrelated.

darkfronza and others added 2 commits October 12, 2026 15:15

github-actions Bot added the needs triage label Oct 13, 2026

dopey reviewed Oct 15, 2026

View reviewed changes

hslatman reviewed Oct 20, 2026

View reviewed changes

darkfronza and others added 5 commits October 23, 2026 16:20

capi: order the ECC blob magics and keep NCryptImportKey's parameters…

aa8ae99

… alive Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

capi: refuse an incomplete private key in ImportKey

af8c09c

A typed nil *ecdsa.PrivateKey or *rsa.PrivateKey, or one missing its components, passed the nil check on the request and panicked while building the import blob. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

darkfronza force-pushed the diego/capi-machine-keyset-lookups branch from 90e56ee to 69d24bb Compare October 23, 2026 19:26

		if u.isMachineKeyset() {
		keyName.Set(StoreLocationArg, MachineStoreLocation)

		if u.storeLocation != "" {
		importedKeyURI += fmt.Sprintf(";%s=%s", StoreLocationArg, u.storeLocation)

		if u.storeLocation != "" {
		createdKeyURI += fmt.Sprintf(";%s=%s", StoreLocationArg, u.storeLocation)

Conversation

darkfronza commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Machine-keyset keys stay addressable

ImportKey

DeleteKey handle fix

Testing

Uh oh!

dopey left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darkfronza commented Oct 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

darkfronza commented Oct 13, 2026 •

edited

Loading

`ImportKey`

`DeleteKey` handle fix

dopey left a comment •

edited

Loading