feat: add per-key key-scope option for Windows machine vs user key store

go.mod

@@ -22,7 +22,7 @@ require (
 	github.com/peterbourgon/diskv/v3 v3.0.1
 	github.com/pkg/errors v0.9.1
 	github.com/schollz/jsonstore v1.1.0
-	github.com/smallstep/go-attestation v0.4.4-0.20241119153605-2306d5b464ca
+	github.com/smallstep/go-attestation v0.4.9
 	github.com/stretchr/testify v1.11.1
 	go.uber.org/mock v0.6.0
 	golang.org/x/crypto v0.50.0

go.sum

@@ -758,8 +758,8 @@ github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPx
 github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
 github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88=
 github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
-github.com/smallstep/go-attestation v0.4.4-0.20241119153605-2306d5b464ca h1:VX8L0r8vybH0bPeaIxh4NQzafKQiqvlOn8pmOXbFLO4=
-github.com/smallstep/go-attestation v0.4.4-0.20241119153605-2306d5b464ca/go.mod h1:vNAduivU014fubg6ewygkAvQC0IQVXqdc8vaGl/0er4=
+github.com/smallstep/go-attestation v0.4.9 h1:/fVmzB8A8tk7B2PCGPlszWzyl/GDcEQbynqcg2PMaZ0=
+github.com/smallstep/go-attestation v0.4.9/go.mod h1:vNAduivU014fubg6ewygkAvQC0IQVXqdc8vaGl/0er4=
 github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc=
 github.com/smartystreets/assertions v1.0.0/go.mod h1:kHHU4qYBaI3q23Pp3VPrmWhuIUrLW/7eUrw0BU5VaoM=
 github.com/smartystreets/go-aws-auth v0.0.0-20180515143844-0c1422d1fdb9/go.mod h1:SnhjPscd9TpLiy1LpzGSKh3bXCfxxXuqd9xmQJy3slM=

kms/tpmkms/tpmkms.go

@@ -557,12 +557,15 @@ func (k *TPMKMS) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyRespons
 		}, nil
 	}
 
+	machineKey := properties.machineKey()
+
 	var key *tpm.Key
 	if properties.attestBy != "" {
 		config := tpm.AttestKeyConfig{
 			Algorithm:      v.Type,
 			Size:           size,
 			QualifyingData: properties.qualifyingData,
+			MachineKey:     machineKey,
 		}
 		key, err = k.tpm.AttestKey(ctx, properties.attestBy, properties.name, config)
 		if err != nil {
@@ -573,8 +576,9 @@ func (k *TPMKMS) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyRespons
 		}
 	} else {
 		config := tpm.CreateKeyConfig{
-			Algorithm: v.Type,
-			Size:      size,
+			Algorithm:  v.Type,
+			Size:       size,
+			MachineKey: machineKey,
 		}
 		key, err = k.tpm.CreateKey(ctx, properties.name, config)
 		if err != nil {
@@ -598,10 +602,17 @@ func (k *TPMKMS) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyRespons
 		return nil, fmt.Errorf("failed getting signer for key: %w", err)
 	}
 
+	// Preserve key-scope in the returned URI so subsequent CreateSigner /
+	// DeleteKey calls land in the right scope. The bug we hit before was
+	// exactly this: the returned URI had only "name=", so re-opens
+	// defaulted to user scope and failed to find machine-stored keys.
 	createdKeyURI := fmt.Sprintf("tpmkms:name=%s", key.Name())
 	if properties.attestBy != "" {
 		createdKeyURI = fmt.Sprintf("%s;attest-by=%s", createdKeyURI, key.AttestedBy())
 	}
+	if machineKey {
+		createdKeyURI = fmt.Sprintf("%s;key-scope=machine", createdKeyURI)
+	}
 
 	return &apiv1.CreateKeyResponse{
 		Name:       createdKeyURI,

kms/tpmkms/uri.go

@@ -27,6 +27,28 @@ type objectProperties struct {
 	sha1                      string
 	serial                    string
 	issuer                    string
+	// keyScope, if set, is one of "machine" or "user". It controls
+	// whether the underlying private key lives in the local machine key
+	// store or in the current user's key store, and is independent of
+	// where the certificate is stored. When unset, [parseNameURI] derives
+	// it from storeLocation for backwards compatibility (machine cert
+	// store implies machine key scope).
+	keyScope string
+}
+
+// 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"
 }
 
 func parseNameURI(nameURI string) (o objectProperties, err error) {
@@ -75,10 +97,20 @@ func parseNameURI(nameURI string) (o objectProperties, err error) {
 		o.serial = u.Get("serial")
 		o.issuer = u.Get("issuer")
 
+		// key-scope is independent of store-location: cert location and
+		// key ownership are orthogonal Windows concepts. See [machineKey]
+		// for the back-compat default when this is unset.
+		o.keyScope = u.Get("key-scope")
+
 		// validation
 		if o.ak && o.attestBy != "" {
 			return o, errors.New(`"ak" and "attest-by" are mutually exclusive`)
 		}
+		switch o.keyScope {
+		case "", "machine", "user":
+		default:
+			return o, fmt.Errorf(`"key-scope" must be "machine" or "user", got %q`, o.keyScope)
+		}
 
 		return
 	}

tpm/ak.go

@@ -26,11 +26,20 @@ type AK struct {
 	data         []byte
 	chain        []*x509.Certificate
 	createdAt    time.Time
+	machineKey   bool
 	blobs        *Blobs
 	attestParams *attest.AttestationParameters
 	tpm          *TPM
 }
 
+// CreateAKConfig is used to pass configuration when creating AKs.
+type CreateAKConfig struct {
+	// MachineKey, when true, requests that the AK be created in the local
+	// machine key store rather than in the current user's key store. See
+	// [CreateKeyConfig.MachineKey] for details.
+	MachineKey bool
+}
+
 // Name returns the AK name. The name uniquely
 // identifies an AK if a TPM with persistent
 // storage is used.
@@ -133,8 +142,16 @@ func (ak *AK) MarshalJSON() ([]byte, error) {
 // CreateAK creates and stores a new AK identified by `name`.
 // If no name is  provided, a random 10 character name is generated.
 // If an AK with the same name exists, `ErrExists` is returned.
+//
+// To request a machine-scoped AK on Windows, use [TPM.CreateAKWithConfig].
 func (t *TPM) CreateAK(ctx context.Context, name string) (ak *AK, err error) {
-	if err = t.open(ctx); err != nil {
+	return t.CreateAKWithConfig(ctx, name, CreateAKConfig{})
+}
+
+// CreateAKWithConfig creates and stores a new AK with the given config.
+// See [TPM.CreateAK] for the simpler signature.
+func (t *TPM) CreateAKWithConfig(ctx context.Context, name string, config CreateAKConfig) (ak *AK, err error) {
+	if err = t.open(withMachineKey(ctx, config.MachineKey)); err != nil {
 		return nil, fmt.Errorf("failed opening TPM: %w", err)
 	}
 	defer closeTPM(ctx, t, &err)
@@ -167,10 +184,11 @@ func (t *TPM) CreateAK(ctx context.Context, name string) (ak *AK, err error) {
 	}
 
 	ak = &AK{
-		name:      name,
-		data:      data,
-		createdAt: now,
-		tpm:       t,
+		name:       name,
+		data:       data,
+		createdAt:  now,
+		machineKey: config.MachineKey,
+		tpm:        t,
 	}
 
 	if err := t.store.AddAK(ak.toStorage()); err != nil {
@@ -259,19 +277,25 @@ func (t *TPM) ListAKs(ctx context.Context) (aks []*AK, err error) {
 // DeleteAK removes the AK identified by `name`. It returns `ErrNotfound`
 // if it doesn't exist. Keys that were attested by the AK have to be removed
 // before removing the AK, otherwise an error will be returned.
+//
+// If the in-TPM deletion fails but the file-store entry is successfully
+// removed, the returned error is wrapped in a [PartialDeleteError]. See
+// [TPM.DeleteKey] for the rationale.
 func (t *TPM) DeleteAK(ctx context.Context, name string) (err error) {
-	if err := t.open(ctx); err != nil {
-		return fmt.Errorf("failed opening TPM: %w", err)
-	}
-	defer closeTPM(ctx, t, &err)
-
-	ak, err := t.store.GetAK(name)
-	if err != nil {
-		if errors.Is(err, storage.ErrNotFound) {
+	// Look up first so we know the AK's machine-key scope before opening
+	// attest with the right flag.
+	ak, getErr := t.store.GetAK(name)
+	if getErr != nil {
+		if errors.Is(getErr, storage.ErrNotFound) {
 			return fmt.Errorf("failed getting AK %q: %w", name, ErrNotFound)
 		}
-		return fmt.Errorf("failed getting AK %q: %w", name, err)
+		return fmt.Errorf("failed getting AK %q: %w", name, getErr)
+	}
+
+	if err := t.open(withMachineKey(ctx, ak.MachineKey)); err != nil {
+		return fmt.Errorf("failed opening TPM: %w", err)
 	}
+	defer closeTPM(ctx, t, &err)
 
 	// prevent deleting the AK if the TPM (storage) contains keys that
 	// were attested by it. While keys would still work if the AK were
@@ -286,19 +310,26 @@ func (t *TPM) DeleteAK(ctx context.Context, name string) (err error) {
 		return fmt.Errorf("failed deleting AK %q because %d key(s) exist that were attested by it", name, len(keys))
 	}
 
-	if err := t.attestTPM.DeleteKey(ak.Data); err != nil { // TODO: we could add a DeleteAK to go-attestation; under the hood it's loaded the same as a key though.
-		return fmt.Errorf("failed deleting AK %q: %w", name, err)
-	}
+	attestErr := t.attestTPM.DeleteKey(ak.Data) // TODO: we could add a DeleteAK to go-attestation; under the hood it's loaded the same as a key though.
 
 	if err := t.store.DeleteAK(name); err != nil {
+		if attestErr != nil {
+			return fmt.Errorf("failed deleting AK %q from storage after TPM delete failed (%v): %w", name, attestErr, err)
+		}
 		return fmt.Errorf("failed deleting AK %q from storage: %w", name, err)
 	}
 
 	if err := t.store.Persist(); err != nil {
+		if attestErr != nil {
+			return fmt.Errorf("failed persisting storage after TPM delete failed (%v): %w", attestErr, err)
+		}
 		return fmt.Errorf("failed persisting storage: %w", err)
 	}
 
-	return
+	if attestErr != nil {
+		return &PartialDeleteError{Name: name, Underlying: attestErr}
+	}
+	return nil
 }
 
 // AttestationParameters returns information about the AK, typically used to
@@ -308,7 +339,7 @@ func (ak *AK) AttestationParameters(ctx context.Context) (params attest.Attestat
 		return *ak.attestParams, nil
 	}
 
-	if err = ak.tpm.open(ctx); err != nil {
+	if err = ak.tpm.open(withMachineKey(ctx, ak.machineKey)); err != nil {
 		return params, fmt.Errorf("failed opening TPM: %w", err)
 	}
 	defer closeTPM(ctx, ak.tpm, &err)
@@ -333,7 +364,7 @@ type EncryptedCredential attest.EncryptedCredential
 // generated on the same TPM as the EK. This operation is synonymous with
 // TPM2_ActivateCredential.
 func (ak *AK) ActivateCredential(ctx context.Context, in EncryptedCredential) (secret []byte, err error) {
-	if err := ak.tpm.open(ctx); err != nil {
+	if err := ak.tpm.open(withMachineKey(ctx, ak.machineKey)); err != nil {
 		return secret, fmt.Errorf("failed opening TPM: %w", err)
 	}
 	defer closeTPM(ctx, ak.tpm, &err)
@@ -359,7 +390,7 @@ func (ak *AK) Blobs(ctx context.Context) (blobs *Blobs, err error) {
 		return ak.blobs, nil
 	}
 
-	if err = ak.tpm.open(ctx); err != nil {
+	if err = ak.tpm.open(withMachineKey(ctx, ak.machineKey)); err != nil {
 		return nil, fmt.Errorf("failed opening TPM: %w", err)
 	}
 	defer closeTPM(ctx, ak.tpm, &err)
@@ -383,7 +414,7 @@ func (ak *AK) Blobs(ctx context.Context) (blobs *Blobs, err error) {
 // If the AK public key doesn't match the public key in the first certificate
 // in the chain (the leaf), an error is returned.
 func (ak *AK) SetCertificateChain(ctx context.Context, chain []*x509.Certificate) (err error) {
-	if err := ak.tpm.open(ctx); err != nil {
+	if err := ak.tpm.open(withMachineKey(ctx, ak.machineKey)); err != nil {
 		return fmt.Errorf("failed opening TPM: %w", err)
 	}
 	defer closeTPM(ctx, ak.tpm, &err)
@@ -463,21 +494,23 @@ func (ak *AK) HasValidPermanentIdentifier(permanentIdentifier string) bool {
 // persisting AKs.
 func (ak *AK) toStorage() *storage.AK {
 	return &storage.AK{
-		Name:      ak.name,
-		Data:      ak.data,
-		Chain:     ak.chain,
-		CreatedAt: ak.createdAt.UTC(),
+		Name:       ak.name,
+		Data:       ak.data,
+		Chain:      ak.chain,
+		CreatedAt:  ak.createdAt.UTC(),
+		MachineKey: ak.machineKey,
 	}
 }
 
 // akFromStorage recreates an AK from the struct used for
 // persisting AKs.
 func akFromStorage(sak *storage.AK, t *TPM) *AK {
 	return &AK{
-		name:      sak.Name,
-		data:      sak.Data,
-		chain:     sak.Chain,
-		createdAt: sak.CreatedAt.Local(),
-		tpm:       t,
+		name:       sak.Name,
+		data:       sak.Data,
+		chain:      sak.Chain,
+		createdAt:  sak.CreatedAt.Local(),
+		machineKey: sak.MachineKey,
+		tpm:        t,
 	}
 }

tpm/context.go

@@ -37,3 +37,21 @@ func isGoTPMCall(ctx context.Context) bool {
 	v, ok := ctx.Value(goTPMCallContextKey{}).(bool)
 	return ok && v
 }
+
+type machineKeyContextKey struct{}
+
+// withMachineKey annotates ctx with a machine-key flag that the TPM open
+// path consults when initializing the underlying attest.TPM. This lets
+// individual key operations (CreateKey, GetKey/Public/Signer/...,
+// DeleteKey) request machine-scoped vs user-scoped behaviour without
+// turning MachineKey into a per-TPM-instance setting.
+func withMachineKey(ctx context.Context, machineKey bool) context.Context {
+	return context.WithValue(ctx, machineKeyContextKey{}, machineKey)
+}
+
+// machineKeyFromContext returns the machine-key flag annotated on ctx, or
+// false if none is set.
+func machineKeyFromContext(ctx context.Context) bool {
+	v, _ := ctx.Value(machineKeyContextKey{}).(bool)
+	return v
+}

tpm/errors.go

@@ -2,6 +2,7 @@ package tpm
 
 import (
 	"errors"
+	"fmt"
 
 	"go.step.sm/crypto/tpm/storage"
 )
@@ -15,3 +16,26 @@ var ErrExists = errors.New("already exists")
 // ErrNoStorageConfigured is returned when a TPM operation is
 // performed that requires a storage to have been configured
 var ErrNoStorageConfigured = storage.ErrNoStorageConfigured
+
+// PartialDeleteError is returned by DeleteKey or DeleteAK when the
+// in-TPM (NCrypt PCP / attest) deletion failed but the file-store
+// entry was successfully removed. Callers can use [errors.As] to
+// detect this case and decide whether to treat it as a transient
+// failure (the named entry is gone from local bookkeeping; the
+// underlying TPM key may or may not still exist).
+//
+// Cleaning up the file-store entry even on PCP failure prevents
+// repeated retries from re-encountering the same failing entry,
+// which is the failure mode that motivated this type.
+type PartialDeleteError struct {
+	// Name is the key/AK name that was being deleted.
+	Name string
+	// Underlying is the error returned by the in-TPM deletion.
+	Underlying error
+}
+
+func (e *PartialDeleteError) Error() string {
+	return fmt.Sprintf("file-store entry %q removed but TPM deletion failed: %v", e.Name, e.Underlying)
+}
+
+func (e *PartialDeleteError) Unwrap() error { return e.Underlying }

tpm/internal/key/key.go

@@ -106,6 +106,11 @@ type CreateConfig struct {
 	// Size is used to specify the bit size of the key or elliptic curve. For
 	// example, '256' is used to specify curve P-256.
 	Size int
+	// MachineKey, when true, requests that the key be created in the local
+	// machine key store rather than the current user key store. On Windows
+	// this causes NCRYPT_MACHINE_KEY_FLAG to be passed to the underlying
+	// NCrypt PCP provider. Ignored on other platforms.
+	MachineKey bool
 }
 
 func (c *CreateConfig) Validate() error {
@@ -184,8 +189,9 @@ const (
 )
 
 type KeyConfig struct {
-	Algorithm Algorithm
-	Size      int
+	Algorithm  Algorithm
+	Size       int
+	MachineKey bool
 }
 
 var (

tpm/internal/key/key_windows.go

@@ -14,7 +14,11 @@ func create(_ io.ReadWriteCloser, keyName string, config CreateConfig) ([]byte,
 	}
 	defer pcp.Close()
 
-	_, pub, _, err := pcp.NewKey(keyName, &KeyConfig{Algorithm: Algorithm(config.Algorithm), Size: config.Size})
+	_, pub, _, err := pcp.NewKey(keyName, &KeyConfig{
+		Algorithm:  Algorithm(config.Algorithm),
+		Size:       config.Size,
+		MachineKey: config.MachineKey,
+	})
 	if err != nil {
 		return nil, fmt.Errorf("pcp failed to mint application key: %w", err)
 	}