diff --git a/go.mod b/go.mod index 7bc45f9f..d712f006 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 97fa5dd7..5724597d 100644 --- a/go.sum +++ b/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= diff --git a/kms/tpmkms/tpmkms.go b/kms/tpmkms/tpmkms.go index 14a1efb7..7dcf5ee0 100644 --- a/kms/tpmkms/tpmkms.go +++ b/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, diff --git a/kms/tpmkms/uri.go b/kms/tpmkms/uri.go index 567f1e1e..1b2f4105 100644 --- a/kms/tpmkms/uri.go +++ b/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 } diff --git a/tpm/ak.go b/tpm/ak.go index 82903b51..af6dea3b 100644 --- a/tpm/ak.go +++ b/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,10 +494,11 @@ 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, } } @@ -474,10 +506,11 @@ func (ak *AK) toStorage() *storage.AK { // 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, } } diff --git a/tpm/context.go b/tpm/context.go index d8625921..3c1b3b44 100644 --- a/tpm/context.go +++ b/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 +} diff --git a/tpm/errors.go b/tpm/errors.go index 08c6b66f..fbb5bac0 100644 --- a/tpm/errors.go +++ b/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 } diff --git a/tpm/internal/key/key.go b/tpm/internal/key/key.go index cd9141d1..90569244 100644 --- a/tpm/internal/key/key.go +++ b/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 ( diff --git a/tpm/internal/key/key_windows.go b/tpm/internal/key/key_windows.go index 65636a6d..13e86752 100644 --- a/tpm/internal/key/key_windows.go +++ b/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) } diff --git a/tpm/internal/key/pcp_windows.go b/tpm/internal/key/pcp_windows.go index 3626d9a6..af363069 100644 --- a/tpm/internal/key/pcp_windows.go +++ b/tpm/internal/key/pcp_windows.go @@ -35,6 +35,10 @@ const ( // The below is documented in this Microsoft whitepaper: // https://github.com/Microsoft/TSS.MSR/blob/master/PCPTool.v11/Using%20the%20Windows%208%20Platform%20Crypto%20Provider%20and%20Associated%20TPM%20Functionality.pdf ncryptOverwriteKeyFlag = 0x80 + // ncryptMachineKeyFlag instructs NCrypt to create or open the key in the + // local machine key store rather than the current user key store. Defined + // in ncrypt.h as NCRYPT_MACHINE_KEY_FLAG. + ncryptMachineKeyFlag uint32 = 0x00000020 // Key usage value for generic keys nCryptPropertyPCPKeyUsagePolicyGeneric = 0x3 // Key usage value for AKs. @@ -290,7 +294,7 @@ func (h *winPCP) Close() error { return closeNCryptObject(h.hProv) } -func (h *winPCP) newKey(name string, alg string, length uint32, policy uint32) (uintptr, []byte, []byte, error) { +func (h *winPCP) newKey(name string, alg string, length uint32, policy uint32, machineKey bool) (uintptr, []byte, []byte, error) { var kh uintptr utf16Name, err := windows.UTF16FromString(name) if err != nil { @@ -301,8 +305,13 @@ func (h *winPCP) newKey(name string, alg string, length uint32, policy uint32) ( return 0, nil, nil, err } + var flags uint32 + if machineKey { + flags = ncryptMachineKeyFlag + } + // Create a persistent RSA key of the specified name. - r, _, msg := nCryptCreatePersistedKey.Call(h.hProv, uintptr(unsafe.Pointer(&kh)), uintptr(unsafe.Pointer(&utf16RSA[0])), uintptr(unsafe.Pointer(&utf16Name[0])), 0, 0) + r, _, msg := nCryptCreatePersistedKey.Call(h.hProv, uintptr(unsafe.Pointer(&kh)), uintptr(unsafe.Pointer(&utf16RSA[0])), uintptr(unsafe.Pointer(&utf16Name[0])), 0, uintptr(flags)) if r != 0 { if tpmErr := maybeWinErr(r); tpmErr != nil { msg = tpmErr @@ -382,15 +391,15 @@ func (h *winPCP) newKey(name string, alg string, length uint32, policy uint32) ( // NewKey creates a persistent application key of the specified name. func (h *winPCP) NewKey(name string, config *KeyConfig) (uintptr, []byte, []byte, error) { if config.Algorithm == RSA { - return h.newKey(name, "RSA", uint32(config.Size), 0) + return h.newKey(name, "RSA", uint32(config.Size), 0, config.MachineKey) } else if config.Algorithm == ECDSA { switch config.Size { case 256: - return h.newKey(name, "ECDSA_P256", 0, 0) + return h.newKey(name, "ECDSA_P256", 0, 0, config.MachineKey) case 384: - return h.newKey(name, "ECDSA_P384", 0, 0) + return h.newKey(name, "ECDSA_P384", 0, 0, config.MachineKey) case 521: - return h.newKey(name, "ECDSA_P521", 0, 0) + return h.newKey(name, "ECDSA_P521", 0, 0, config.MachineKey) default: return 0, nil, nil, fmt.Errorf("unsupported ECDSA key size: %v", config.Size) } diff --git a/tpm/key.go b/tpm/key.go index b8c267b3..d6646940 100644 --- a/tpm/key.go +++ b/tpm/key.go @@ -25,6 +25,7 @@ type Key struct { attestedBy string chain []*x509.Certificate createdAt time.Time + machineKey bool blobs *Blobs tpm *TPM } @@ -69,7 +70,7 @@ func (k *Key) WasAttestedBy(ak *AK) bool { func (k *Key) Public() crypto.PublicKey { var ( err error - ctx = context.Background() + ctx = withMachineKey(context.Background(), k.machineKey) ) if err = k.tpm.open(ctx); err != nil { return nil @@ -141,6 +142,14 @@ type CreateKeyConfig 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 underlying private key be + // created in the local machine key store rather than in the current + // user's key store. On Windows this causes NCRYPT_MACHINE_KEY_FLAG to + // be passed to the NCrypt PCP provider on creation, and to be applied + // every time the key is subsequently opened. The value is persisted + // alongside the key in storage so that load operations use the matching + // scope. Has no effect on non-Windows platforms. + MachineKey bool // TODO(hs): move key name to this struct? } @@ -159,6 +168,10 @@ type AttestKeyConfig struct { // When used with ACME `device-attest-01`, this contains a hash of // the key authorization. QualifyingData []byte + // MachineKey, when true, requests that the underlying private key be + // created in the local machine key store rather than in the current + // user's key store. See [CreateKeyConfig.MachineKey] for details. + MachineKey bool // TODO(hs): add akName and key name to this struct? } @@ -167,7 +180,7 @@ type AttestKeyConfig struct { // a random 10 character name is generated. If a Key with the same name exists, // `ErrExists` is returned. The Key won't be attested by an AK. func (t *TPM) CreateKey(ctx context.Context, name string, config CreateKeyConfig) (key *Key, err error) { - if err = t.open(goTPMCall(ctx)); err != nil { + if err = t.open(withMachineKey(goTPMCall(ctx), config.MachineKey)); err != nil { return nil, fmt.Errorf("failed opening TPM: %w", err) } defer closeTPM(ctx, t, &err) @@ -186,8 +199,9 @@ func (t *TPM) CreateKey(ctx context.Context, name string, config CreateKeyConfig } createConfig := internalkey.CreateConfig{ - Algorithm: config.Algorithm, - Size: config.Size, + Algorithm: config.Algorithm, + Size: config.Size, + MachineKey: config.MachineKey, } if err := t.validate(&createConfig); err != nil { return nil, fmt.Errorf("invalid key creation parameters: %w", err) @@ -198,10 +212,11 @@ func (t *TPM) CreateKey(ctx context.Context, name string, config CreateKeyConfig } key = &Key{ - name: name, - data: data, - createdAt: now, - tpm: t, + name: name, + data: data, + createdAt: now, + machineKey: config.MachineKey, + tpm: t, } if err := t.store.AddKey(key.toStorage()); err != nil { @@ -236,7 +251,32 @@ func (w attestValidationWrapper) Validate() error { // name is generated. If a Key with the same name exists, `ErrExists` is // returned. func (t *TPM) AttestKey(ctx context.Context, akName, name string, config AttestKeyConfig) (key *Key, err error) { - if err = t.open(ctx); err != nil { + // Look up the AK before opening attest so we can use its scope for the + // attest session. An attested key inherits its AK's scope: the attest + // session can only operate in one scope at a time, so the AK and the + // new key it certifies must share it. Any explicit + // [AttestKeyConfig.MachineKey] that conflicts with the AK's scope is + // rejected here rather than failing later inside attest. + ak, err := t.store.GetAK(akName) + if err != nil { + if errors.Is(err, storage.ErrNotFound) { + return nil, fmt.Errorf("failed getting AK %q: %w", akName, ErrNotFound) + } + return nil, fmt.Errorf("failed getting AK %q: %w", akName, err) + } + // An attested key inherits its AK's scope. Reject explicit + // [AttestKeyConfig.MachineKey] mismatches in either direction so + // callers don't get a silently-promoted (or silently-demoted) key. + // To opt into "use whatever scope the AK is in," leave + // AttestKeyConfig.MachineKey at its zero value AND make sure the AK + // is in user scope; for a machine-scope AK, set MachineKey: true + // explicitly. + if config.MachineKey != ak.MachineKey { + return nil, fmt.Errorf("AttestKeyConfig.MachineKey=%v does not match AK %q scope (MachineKey=%v); attested keys inherit their AK's scope", config.MachineKey, akName, ak.MachineKey) + } + machineKey := ak.MachineKey + + if err = t.open(withMachineKey(ctx, machineKey)); err != nil { return nil, fmt.Errorf("failed opening TPM: %w", err) } defer closeTPM(ctx, t, &err) @@ -254,14 +294,6 @@ func (t *TPM) AttestKey(ctx context.Context, akName, name string, config AttestK return nil, fmt.Errorf("failed creating key %q: %w", name, err) } - ak, err := t.store.GetAK(akName) - if err != nil { - if errors.Is(err, storage.ErrNotFound) { - return nil, fmt.Errorf("failed getting AK %q: %w", akName, ErrNotFound) - } - return nil, fmt.Errorf("failed getting AK %q: %w", akName, err) - } - loadedAK, err := t.attestTPM.LoadAK(ak.Data) if err != nil { return nil, fmt.Errorf("failed loading AK %q: %w", akName, err) @@ -293,6 +325,7 @@ func (t *TPM) AttestKey(ctx context.Context, akName, name string, config AttestK data: data, attestedBy: akName, createdAt: now, + machineKey: machineKey, tpm: t, } @@ -372,33 +405,50 @@ func (t *TPM) GetKeysAttestedBy(ctx context.Context, akName string) (keys []*Key // DeleteKey removes the Key identified by `name`. It returns `ErrNotfound` // if it doesn't exist. +// +// If the in-TPM deletion fails but the file-store entry is successfully +// removed, the returned error is wrapped in a [PartialDeleteError] so +// callers can distinguish "leaked from the TPM but not from the file +// store" from a complete failure. Cleaning up the file-store entry even +// on PCP failure prevents repeated retries from re-encountering the same +// failing entry. func (t *TPM) DeleteKey(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) - - key, err := t.store.GetKey(name) - if err != nil { - if errors.Is(err, storage.ErrNotFound) { + // Read storage first so we know the key's machine-key scope before + // opening attest with the right flag. Cheaper than doing it after + // open() because the store load is unconditional anyway. + key, getErr := t.store.GetKey(name) + if getErr != nil { + if errors.Is(getErr, storage.ErrNotFound) { return fmt.Errorf("failed getting key %q: %w", name, ErrNotFound) } - return fmt.Errorf("failed getting key %q: %w", name, err) + return fmt.Errorf("failed getting key %q: %w", name, getErr) } - if err := t.attestTPM.DeleteKey(key.Data); err != nil { - return fmt.Errorf("failed deleting key %q: %w", name, err) + if err := t.open(withMachineKey(ctx, key.MachineKey)); err != nil { + return fmt.Errorf("failed opening TPM: %w", err) } + defer closeTPM(ctx, t, &err) + + attestErr := t.attestTPM.DeleteKey(key.Data) if err := t.store.DeleteKey(name); err != nil { + if attestErr != nil { + return fmt.Errorf("failed deleting key %q from storage after TPM delete failed (%v): %w", name, attestErr, err) + } return fmt.Errorf("failed deleting key %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 } // Signer returns a crypto.Signer backed by the Key. @@ -409,7 +459,7 @@ func (k *Key) Signer(ctx context.Context) (crypto.Signer, error) { // CertificationParameters returns information about the key that can be used to // verify key certification. func (k *Key) CertificationParameters(ctx context.Context) (params attest.CertificationParameters, err error) { - if err = k.tpm.open(ctx); err != nil { + if err = k.tpm.open(withMachineKey(ctx, k.machineKey)); err != nil { return params, fmt.Errorf("failed opening TPM: %w", err) } defer closeTPM(ctx, k.tpm, &err) @@ -435,7 +485,7 @@ func (k *Key) Blobs(ctx context.Context) (blobs *Blobs, err error) { return k.blobs, nil } - if err = k.tpm.open(ctx); err != nil { + if err = k.tpm.open(withMachineKey(ctx, k.machineKey)); err != nil { return nil, fmt.Errorf("failed opening TPM: %w", err) } defer closeTPM(ctx, k.tpm, &err) @@ -459,7 +509,7 @@ func (k *Key) Blobs(ctx context.Context) (blobs *Blobs, err error) { // If the public key doesn't match the public key in the first certificate // in the chain (the leaf), an error is returned. func (k *Key) SetCertificateChain(ctx context.Context, chain []*x509.Certificate) (err error) { - if err = k.tpm.open(ctx); err != nil { + if err = k.tpm.open(withMachineKey(ctx, k.machineKey)); err != nil { return fmt.Errorf("failed opening TPM: %w", err) } defer closeTPM(ctx, k.tpm, &err) @@ -504,6 +554,7 @@ func (k *Key) toStorage() *storage.Key { AttestedBy: k.attestedBy, Chain: k.chain, CreatedAt: k.createdAt.UTC(), + MachineKey: k.machineKey, } } @@ -516,6 +567,7 @@ func keyFromStorage(sk *storage.Key, t *TPM) *Key { attestedBy: sk.AttestedBy, chain: sk.Chain, createdAt: sk.CreatedAt.Local(), + machineKey: sk.MachineKey, tpm: t, } } diff --git a/tpm/signer.go b/tpm/signer.go index 5b992e2a..b0fad108 100644 --- a/tpm/signer.go +++ b/tpm/signer.go @@ -27,7 +27,7 @@ func (s *signer) Public() crypto.PublicKey { // The TPM key is loaded lazily, meaning that every call to Sign() // will reload the TPM key to be used. func (s *signer) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) (signature []byte, err error) { - ctx := context.Background() + ctx := withMachineKey(context.Background(), s.key.machineKey) if err = s.tpm.open(ctx); err != nil { return nil, fmt.Errorf("failed opening TPM: %w", err) } @@ -55,19 +55,22 @@ func (s *signer) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) (si // GetSigner returns a crypto.Signer for a TPM Key identified by `name`. func (t *TPM) GetSigner(ctx context.Context, name string) (csigner crypto.Signer, err error) { - if err = t.open(ctx); err != nil { - return nil, fmt.Errorf("failed opening TPM: %w", err) - } - defer closeTPM(ctx, t, &err) - - key, err := t.store.GetKey(name) - if err != nil { - if errors.Is(err, storage.ErrNotFound) { + // Look up the key in the store first so we know its machine-key + // scope before opening attest. The store load is unconditional in + // open() anyway, so there's no extra round trip here. + key, getErr := t.store.GetKey(name) + if getErr != nil { + if errors.Is(getErr, storage.ErrNotFound) { return nil, fmt.Errorf("failed getting signer for key %q: %w", name, ErrNotFound) } - return nil, fmt.Errorf("failed getting signer for key %q: %w", name, err) + return nil, fmt.Errorf("failed getting signer for key %q: %w", name, getErr) } + if err = t.open(withMachineKey(ctx, key.MachineKey)); err != nil { + return nil, fmt.Errorf("failed opening TPM: %w", err) + } + defer closeTPM(ctx, t, &err) + loadedKey, err := t.attestTPM.LoadKey(key.Data) if err != nil { return nil, err @@ -85,7 +88,7 @@ func (t *TPM) GetSigner(ctx context.Context, name string) (csigner crypto.Signer csigner = &signer{ tpm: t, - key: Key{name: name, data: key.Data, attestedBy: key.AttestedBy, createdAt: key.CreatedAt, tpm: t}, + key: Key{name: name, data: key.Data, attestedBy: key.AttestedBy, createdAt: key.CreatedAt, machineKey: key.MachineKey, tpm: t}, public: loadedKey.Public(), } diff --git a/tpm/storage/types.go b/tpm/storage/types.go index b3fd5c5c..587bdef4 100644 --- a/tpm/storage/types.go +++ b/tpm/storage/types.go @@ -13,6 +13,11 @@ type AK struct { Data []byte Chain []*x509.Certificate CreatedAt time.Time + // MachineKey records whether the AK was created in the local machine + // key store (Windows: NCRYPT_MACHINE_KEY_FLAG). Persisted so that + // reload uses the matching key scope. Defaults to false for AKs + // created before this field was introduced. + MachineKey bool } // MarshalJSON marshals the AK into JSON. @@ -23,10 +28,11 @@ func (ak *AK) MarshalJSON() ([]byte, error) { } sak := serializedAK{ - Name: ak.Name, - Type: typeAK, - Data: ak.Data, - CreatedAt: ak.CreatedAt, + Name: ak.Name, + Type: typeAK, + Data: ak.Data, + CreatedAt: ak.CreatedAt, + MachineKey: ak.MachineKey, } if len(chain) > 0 { @@ -50,6 +56,7 @@ func (ak *AK) UnmarshalJSON(data []byte) error { ak.Name = sak.Name ak.Data = sak.Data ak.CreatedAt = sak.CreatedAt + ak.MachineKey = sak.MachineKey if len(sak.Chain) > 0 { chain := make([]*x509.Certificate, len(sak.Chain)) @@ -73,6 +80,11 @@ type Key struct { AttestedBy string Chain []*x509.Certificate CreatedAt time.Time + // MachineKey records whether the Key was created in the local machine + // key store (Windows: NCRYPT_MACHINE_KEY_FLAG). Persisted so that + // reload uses the matching key scope. Defaults to false for Keys + // created before this field was introduced. + MachineKey bool } // MarshalJSON marshals the Key into JSON. @@ -88,6 +100,7 @@ func (key *Key) MarshalJSON() ([]byte, error) { Data: key.Data, AttestedBy: key.AttestedBy, CreatedAt: key.CreatedAt, + MachineKey: key.MachineKey, } if len(chain) > 0 { @@ -112,6 +125,7 @@ func (key *Key) UnmarshalJSON(data []byte) error { key.Data = sk.Data key.AttestedBy = sk.AttestedBy key.CreatedAt = sk.CreatedAt + key.MachineKey = sk.MachineKey if len(sk.Chain) > 0 { chain := make([]*x509.Certificate, len(sk.Chain)) @@ -143,11 +157,12 @@ const ( // serializedAK is the struct used when marshaling // a storage AK to JSON. type serializedAK struct { - Name string `json:"name"` - Type tpmObjectType `json:"type"` - Data []byte `json:"data"` - Chain [][]byte `json:"chain"` - CreatedAt time.Time `json:"createdAt"` + Name string `json:"name"` + Type tpmObjectType `json:"type"` + Data []byte `json:"data"` + Chain [][]byte `json:"chain"` + CreatedAt time.Time `json:"createdAt"` + MachineKey bool `json:"machineKey,omitempty"` } // serializedKey is the struct used when marshaling @@ -159,6 +174,7 @@ type serializedKey struct { AttestedBy string `json:"attestedBy"` Chain [][]byte `json:"chain"` CreatedAt time.Time `json:"createdAt"` + MachineKey bool `json:"machineKey,omitempty"` } // keyForAK returns the key to use when storing an AK. diff --git a/tpm/tpm.go b/tpm/tpm.go index bd0dde01..cb2d9b53 100644 --- a/tpm/tpm.go +++ b/tpm/tpm.go @@ -190,6 +190,12 @@ func (t *TPM) open(ctx context.Context) (err error) { return fmt.Errorf("failed initializing command channel: %w", err) } + // Apply per-call machine-key scope to the attest open config. We hold + // t.lock at this point and t.attestTPM is re-created on every open, so + // mutating attestConfig here is safe and does not leak across callers. + // Always set explicitly so we never carry a previous caller's choice. + t.attestConfig.MachineKey = machineKeyFromContext(ctx) + // if a simulator was set, use it as the backing TPM device. // The simulator is currently only used for testing. if t.simulator != nil {