Add per-key key-scope option for Windows machine vs user key store

[joshdrake]

Summary

• Adds key-scope=machine|user URI parameter to kms/tpmkms, controlling whether the underlying private key lives in the local machine key store (NCRYPT_MACHINE_KEY_FLAG) or the current user's key store. Independent of store-location, so callers can pair a machine-store certificate with a user-owned key (e.g., a LocalSystem service that wants its cert visible machine-wide but its key only openable by SYSTEM) or vice versa.
• Threads MachineKey per-key through tpm.CreateKeyConfig / tpm.AttestKeyConfig / tpm.CreateAKConfig and into the underlying NCrypt PCP create call. Per-key, not per-TPM-instance — a single tpm.TPM handle can mint or open keys in either scope.
• t.open() reads the requested scope from context and applies it to attestConfig.MachineKey under the lock just before attest.OpenTPM. Safe because attestTPM is recreated on every open().
• Persists MachineKey in storage.AK and storage.Key so reload after process restart uses the matching scope without the URI needing to carry it. JSON omitempty so existing on-disk stores remain readable and default to false (= existing user-scope behavior).
• tpmkms.CreateKey preserves key-scope=machine in the returned URI so downstream CreateSigner/DeleteKey calls don't lose scope (closes a returned-URI bug we hit during testing).
• AttestKey reads the AK's persisted scope and uses it for the attest session — attested keys inherit their AK's scope; conflicting AttestKeyConfig.MachineKey is rejected with a clear error.
• DeleteKey/DeleteAK always clean up the file-store entry even if the in-TPM (NCrypt) deletion fails, preventing repeated retries from re-encountering the same failing entry. When the file-store cleanup succeeded but the in-TPM step did not, returns a typed *PartialDeleteError so callers can distinguish a partial delete from total failure.
• Bumps github.com/smallstep/go-attestation to v0.4.9 for the OpenConfig.MachineKey field this change drives.

Defaults: when key-scope is unset, derive from store-location for back-compat (store-location=machine → key-scope=machine).

Background

Code review and a Windows test matrix (interactive admin + LocalSystem via psexec -s -i 0, both x86_64) showed that the agent's TPM-backed device-identity keys land under LocalSystem's user-profile PCPKSP scope rather than the machine scope, even when store-location=machine is set. The kms/capi machine-key fix from #802 doesn't apply because the agent uses kms/tpmkms (with enable-cng=true) — that path goes through tpm/internal/key/pcp_windows.go, where nCryptCreatePersistedKey was hardcoded to dwFlags=0. Fixing it requires (a) plumbing a MachineKey flag through the tpm package and (b) deciding how to express user intent at the URI layer.

This PR is an alternative shape to #1006. Differences:

• Per-key, not per-TPM-instance. Herman's review feedback on Add store-location=machine URI option for Windows machine key store #1006 pushed back on WithMachineKey() being a per-TPM option (would affect all keys going through that handle); the discussion converged on per-key being the right model. This PR puts MachineKey on the per-key configs (CreateKeyConfig, AttestKeyConfig, CreateAKConfig) and uses a context value to thread it into t.open() for the operations that need it.
• MachineKey lives on options, not attestConfig. Also from Herman's review.
• key-scope is decoupled from store-location. Cert location and key ownership are orthogonal Windows concepts. There are real scenarios where a service wants its cert in LocalMachine\My for visibility but its key restricted to one identity (e.g., LocalSystem-only) — key-scope=user with store-location=machine expresses that. Default-derive from store-location for back-compat.
• Typed *PartialDeleteError for DeleteKey/DeleteAK. Also from Herman's review.

Test plan

• go build ./... clean on linux/amd64 and windows/amd64
• go vet ./... clean (one pre-existing master warning in kms/capi/ncrypt_windows.go:569, unrelated to this change)
• go test ./... green across all packages
• Empirical Windows matrix (throwaway diagnostic harness, run interactive admin + psexec -s -i 0 LocalSystem):
  • key-scope=machine lands keys at C:\ProgramData\Microsoft\Crypto\PCPKSP\<sid-hash>\<file>.PCPKEY (interactive admin + LocalSystem).
  • key-scope=user lands keys at <userprofile>\AppData\Local\Microsoft\Crypto\PCPKSP\<sid-hash>\<file>.PCPKEY (LocalSystem profile when running as SYSTEM).
  • Decoupling proven: key-scope=machine + store-location=user puts cert in user store and key in machine PCPKSP; key-scope=user + store-location=machine puts cert in machine store and key in user PCPKSP.
  • Back-compat: key-scope unset + store-location=machine correctly resolves to machine scope.
  • CreateSigner via the URI returned by CreateKey round-trips successfully in every TPM scenario; sign+verify ok.

Out of scope

• Agent-side change to set key-scope=machine on the device-identity TPM URI; lands separately in the agent repo.
• Equivalent fix for kms/capi's returned-URI bug (different root cause, different file). Tracked separately.

🤖 Generated with Claude Code

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[smst-jeff]

Suggested change

	// machineKey returns true if the resolved key scope is "machine".
	// When keyScope is unset on the object, the value is derived from
	// storeLocation: "machine" → true, anything else → false.
	func (o objectProperties) machineKey() bool {
	scope := o.keyScope
	if scope == "" {
	if o.storeLocation == "machine" {
	scope = "machine"
	} else {
	scope = "user"
	}
	}
	return scope == "machine"
	}
	// isMachineKey returns true if the resolved key scope is "machine".
	// When keyScope is unset on the object, the value is derived from
	// storeLocation: "machine" → true, anything else → false.
	func (o objectProperties) isMachineKey() bool {
	return o.keyScope == "machine" ||
	(o.keyScope == "" && o.storeLocation == "machine")
	}

[joshdrake]

5f2cf52

[hslatman]

Changing the order of t.open (which calls into t.store.load) and t.store.X could result in unexpected behavior depending on the Storage implementation. With the Dirstore implementation it won't be an issue in practice, but for other implementations it can be different.

Maybe t.store.Load can be extracted from t.open, and be called before any t.store operation.

[joshdrake]

c3de912

I simply added an explicit t.store.Load for now to preserve the order. There will be duplicate Load calls, but I figured that's probably okay for now to hold off on refactoring open.

[maraino]

Make sure to use a release version of go-attestation once merged.

[hslatman]

Make sure to use a release version of go-attestation once merged.

So far we've always used a commit. We didn't release from surrogate, because we always wanted to upstream it. There were some erroneous tags while you were on vacation, but I deleted those again.

[maraino]

Add goTPMCall as an option.

[maraino]

The code looks good, but I have a couple of questions, one is related to the use of errors.Is() and you can see a comment about that.

The second is if there might be any issues getting, listing or deleting AK or keys that MachineKey

[maraino]

If you want to use something like errors.Is(err, &PartialDeleteError{}), as it is it will return always false. However this will work and return true:

var partialDeleteErr *PartialDeleteError
errors.As(err, &partialDeleteErr)

If you want to make it work with errors.Is() you need to implement something like this:

func (e *PartialDeleteError) Is(target error) bool {
	_, ok := target.(*PartialDeleteError)
	return ok
}

[joshdrake]

I wasn't really doing this for value checking, more for unwrapping with errors.As since the the struct carries the Underlying error and key Name. The idea being that DeleteKey callers can unwrap PartialDeleteError and then take additional action based on the underlying error (eg: delete the PCPKSP file handle, maybe).

[joshdrake]

The second is if there might be any issues getting, listing or deleting AK or keys that MachineKey

by default only administrators or LocalSystem can perform key operations, but otherwise, no