From 360928fcd58e57e999ec660641a288aa88e6e84e Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 2 Apr 2025 17:23:09 -0700 Subject: [PATCH 1/8] Remove YubiKeyPrivateKey --- api/utils/keys/yubikey.go | 58 --------------------------------------- 1 file changed, 58 deletions(-) diff --git a/api/utils/keys/yubikey.go b/api/utils/keys/yubikey.go index 4bab4df28200a..20f60b64271b5 100644 --- a/api/utils/keys/yubikey.go +++ b/api/utils/keys/yubikey.go @@ -38,7 +38,6 @@ import ( "github.com/gravitational/trace" "github.com/gravitational/teleport/api" - attestation "github.com/gravitational/teleport/api/gen/proto/go/attestation/v1" "github.com/gravitational/teleport/api/utils/keys/hardwarekey" "github.com/gravitational/teleport/api/utils/retryutils" ) @@ -206,46 +205,6 @@ Slot %s: ) } -// YubiKeyPrivateKey is a YubiKey PIV private key. Cryptographical operations open -// a new temporary connection to the PIV card to perform the operation. -type YubiKeyPrivateKey struct { - // YubiKey is a specific YubiKey PIV module. - *YubiKey - - pivSlot piv.Slot - signMux sync.Mutex - - slotCert *x509.Certificate - attestationCert *x509.Certificate - attestation *piv.Attestation -} - -// Public returns the public key corresponding to this private key. -func (y *YubiKeyPrivateKey) Public() crypto.PublicKey { - return y.slotCert.PublicKey -} - -// WarmupHardwareKey performs a bogus sign() call to prompt the user for -// a PIN/touch (if needed). -func (y *YubiKeyPrivateKey) WarmupHardwareKey(ctx context.Context) error { - hash := sha256.Sum256(make([]byte, 256)) - _, err := y.sign(ctx, rand.Reader, hash[:], crypto.SHA256) - return trace.Wrap(err, "failed to access a YubiKey private key") -} - -// Sign implements crypto.Signer. -func (y *YubiKeyPrivateKey) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - signature, err := y.sign(ctx, rand, digest, opts) - if err != nil { - return nil, trace.Wrap(err) - } - - return signature, nil -} - // YubiKeys require touch when signing with a private key that requires touch. // Unfortunately, there is no good way to check whether touch is cached by the // PIV module at a given time. In order to require touch only when needed, we @@ -399,23 +358,6 @@ func abandonableSign(ctx context.Context, signer crypto.Signer, rand io.Reader, } } -// GetAttestationStatement returns an AttestationStatement for this YubiKeyPrivateKey. -func (y *YubiKeyPrivateKey) GetAttestationStatement() *hardwarekey.AttestationStatement { - return &hardwarekey.AttestationStatement{ - AttestationStatement: &attestation.AttestationStatement_YubikeyAttestationStatement{ - YubikeyAttestationStatement: &attestation.YubiKeyAttestationStatement{ - SlotCert: y.slotCert.Raw, - AttestationCert: y.attestationCert.Raw, - }, - }, - } -} - -// GetPrivateKeyPolicy returns the PrivateKeyPolicy supported by this YubiKeyPrivateKey. -func (y *YubiKeyPrivateKey) GetPrivateKeyPolicy() PrivateKeyPolicy { - return GetPrivateKeyPolicyFromAttestation(y.attestation) -} - // YubiKey is a specific YubiKey PIV card. type YubiKey struct { // conn is a shared YubiKey PIV connection. From 8cc6700f4e8abfe0fea186841c24e0283d523a02 Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 2 Apr 2025 17:24:51 -0700 Subject: [PATCH 2/8] Remove keys package dependencies from PIV implementation. --- api/utils/keys/piv_service.go | 75 +------- api/utils/keys/piv_service_fake.go | 2 +- api/utils/keys/yubikey.go | 277 +++++++++++++++-------------- 3 files changed, 151 insertions(+), 203 deletions(-) diff --git a/api/utils/keys/piv_service.go b/api/utils/keys/piv_service.go index ae37e9f1e7541..5ee8a2280393f 100644 --- a/api/utils/keys/piv_service.go +++ b/api/utils/keys/piv_service.go @@ -68,49 +68,7 @@ func NewYubiKeyService(prompt hardwarekey.Prompt) *YubiKeyService { // - touch & pin -> 9d // - touch & !pin -> 9e func (s *YubiKeyService) NewPrivateKey(ctx context.Context, config hardwarekey.PrivateKeyConfig) (*hardwarekey.Signer, error) { - var requiredKeyPolicy PrivateKeyPolicy - switch config.Policy { - case hardwarekey.PromptPolicyNone: - requiredKeyPolicy = PrivateKeyPolicyHardwareKey - case hardwarekey.PromptPolicyTouch: - requiredKeyPolicy = PrivateKeyPolicyHardwareKeyTouch - case hardwarekey.PromptPolicyPIN: - requiredKeyPolicy = PrivateKeyPolicyHardwareKeyPIN - case hardwarekey.PromptPolicyTouchAndPIN: - requiredKeyPolicy = PrivateKeyPolicyHardwareKeyTouchAndPIN - } - - ykPriv, err := getOrGenerateYubiKeyPrivateKey(ctx, requiredKeyPolicy, config.CustomSlot, s.prompt) - if err != nil { - return nil, trace.Wrap(err) - } - - ref := &hardwarekey.PrivateKeyRef{ - SerialNumber: ykPriv.serialNumber, - SlotKey: hardwarekey.PIVSlotKey(ykPriv.pivSlot.Key), - PublicKey: ykPriv.Public(), - Policy: hardwarekey.PromptPolicy{ - TouchRequired: ykPriv.attestation.TouchPolicy != piv.TouchPolicyNever, - PINRequired: ykPriv.attestation.PINPolicy != piv.PINPolicyNever, - }, - AttestationStatement: &hardwarekey.AttestationStatement{ - AttestationStatement: &attestationv1.AttestationStatement_YubikeyAttestationStatement{ - YubikeyAttestationStatement: &attestationv1.YubiKeyAttestationStatement{ - SlotCert: ykPriv.slotCert.Raw, - AttestationCert: ykPriv.attestationCert.Raw, - }, - }, - }, - } - - keyRefsMux.Lock() - defer keyRefsMux.Unlock() - keyRefs[baseKeyRef{ - serialNumber: ref.SerialNumber, - slotKey: ref.SlotKey, - }] = ref - - return hardwarekey.NewSigner(s, ref), nil + return newPrivateKey(ctx, config, s.prompt, s) } // Sign performs a cryptographic signature using the specified hardware @@ -121,24 +79,7 @@ func (s *YubiKeyService) Sign(ctx context.Context, ref *hardwarekey.PrivateKeyRe return nil, trace.Wrap(err) } - pivSlot, err := parsePIVSlot(ref.SlotKey) - if err != nil { - return nil, trace.Wrap(err) - } - - priv, err := y.getPrivateKey(pivSlot) - if err != nil { - return nil, trace.Wrap(err) - } - - return priv.Sign(rand, digest, opts) -} - -// SetPrompt sets the hardware key prompt used by the hardware key service, if applicable. -// This is used by Teleport Connect which sets the prompt later than the hardware key service, -// due to process initialization constraints. -func (s *YubiKeyService) SetPrompt(prompt hardwarekey.Prompt) { - s.prompt = prompt + return y.sign(ctx, ref, s.prompt, rand, digest, opts) } // TODO(Joerger): Re-attesting the key every time we decode a hardware key signer is very resource @@ -177,7 +118,7 @@ func (s *YubiKeyService) GetFullKeyRef(serialNumber uint32, slotKey hardwarekey. return nil, trace.Wrap(err) } - ykPriv, err := y.getPrivateKey(pivSlot) + slotCert, attCert, att, err := y.attestKey(pivSlot) if err != nil { return nil, trace.Wrap(err) } @@ -185,16 +126,16 @@ func (s *YubiKeyService) GetFullKeyRef(serialNumber uint32, slotKey hardwarekey. ref := &hardwarekey.PrivateKeyRef{ SerialNumber: serialNumber, SlotKey: slotKey, - PublicKey: ykPriv.Public(), + PublicKey: slotCert.PublicKey, Policy: hardwarekey.PromptPolicy{ - TouchRequired: ykPriv.attestation.TouchPolicy != piv.TouchPolicyNever, - PINRequired: ykPriv.attestation.PINPolicy != piv.PINPolicyNever, + TouchRequired: att.TouchPolicy != piv.TouchPolicyNever, + PINRequired: att.PINPolicy != piv.PINPolicyNever, }, AttestationStatement: &hardwarekey.AttestationStatement{ AttestationStatement: &attestationv1.AttestationStatement_YubikeyAttestationStatement{ YubikeyAttestationStatement: &attestationv1.YubiKeyAttestationStatement{ - SlotCert: ykPriv.slotCert.Raw, - AttestationCert: ykPriv.attestationCert.Raw, + SlotCert: slotCert.Raw, + AttestationCert: attCert.Raw, }, }, }, diff --git a/api/utils/keys/piv_service_fake.go b/api/utils/keys/piv_service_fake.go index d5281906315b9..7d38248e84cfc 100644 --- a/api/utils/keys/piv_service_fake.go +++ b/api/utils/keys/piv_service_fake.go @@ -6,7 +6,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, diff --git a/api/utils/keys/yubikey.go b/api/utils/keys/yubikey.go index 20f60b64271b5..78bb113d96c03 100644 --- a/api/utils/keys/yubikey.go +++ b/api/utils/keys/yubikey.go @@ -38,6 +38,7 @@ import ( "github.com/gravitational/trace" "github.com/gravitational/teleport/api" + attestationv1 "github.com/gravitational/teleport/api/gen/proto/go/attestation/v1" "github.com/gravitational/teleport/api/utils/keys/hardwarekey" "github.com/gravitational/teleport/api/utils/retryutils" ) @@ -58,27 +59,18 @@ const ( // setups, this PIN caching does not actually work as expected, so we handle it instead. // See https://github.com/go-piv/piv-go/issues/47 var ( - cachedKeys = map[piv.Slot]*YubiKeyPrivateKey{} + cachedKeys = map[piv.Slot]*hardwarekey.Signer{} cachedKeysMu sync.Mutex ) -// getOrGenerateYubiKeyPrivateKey connects to a connected yubiKey and gets a private key -// matching the given touch requirement. This private key will either be newly generated -// or previously generated by a Teleport client and reused. -func getOrGenerateYubiKeyPrivateKey(ctx context.Context, requiredKeyPolicy PrivateKeyPolicy, slot hardwarekey.PIVSlotKeyString, prompt hardwarekey.Prompt) (*YubiKeyPrivateKey, error) { - if prompt == nil { - prompt = hardwarekey.NewStdCLIPrompt() - } - - promptPolicy := requiredKeyPolicy.GetPromptPolicy() - +func newPrivateKey(ctx context.Context, config hardwarekey.PrivateKeyConfig, prompt hardwarekey.Prompt, s hardwarekey.Service) (*hardwarekey.Signer, error) { // Get the requested or default PIV slot. var slotKey hardwarekey.PIVSlotKey var err error - if slot != "" { - slotKey, err = slot.Parse() + if config.CustomSlot != "" { + slotKey, err = config.CustomSlot.Parse() } else { - slotKey, err = hardwarekey.GetDefaultSlotKey(promptPolicy) + slotKey, err = hardwarekey.GetDefaultSlotKey(config.Policy) } if err != nil { return nil, trace.Wrap(err) @@ -93,7 +85,7 @@ func getOrGenerateYubiKeyPrivateKey(ctx context.Context, requiredKeyPolicy Priva cachedKeysMu.Lock() defer cachedKeysMu.Unlock() - if key, ok := cachedKeys[pivSlot]; ok && key.GetPrivateKeyPolicy() == requiredKeyPolicy { + if key, ok := cachedKeys[pivSlot]; ok && key.GetPromptPolicy() == config.Policy { return key, nil } @@ -104,80 +96,94 @@ func getOrGenerateYubiKeyPrivateKey(ctx context.Context, requiredKeyPolicy Priva } // If PIN is required, check that PIN and PUK are not the defaults. - if requiredKeyPolicy.isHardwareKeyPINVerified() { + if config.Policy.PINRequired { if err := y.checkOrSetPIN(ctx); err != nil { return nil, trace.Wrap(err) } } - promptOverwriteSlot := func(msg string) error { - promptQuestion := fmt.Sprintf("%v\nWould you like to overwrite this slot's private key and certificate?", msg) - if confirmed, confirmErr := prompt.ConfirmSlotOverwrite(ctx, promptQuestion); confirmErr != nil { - return trace.Wrap(confirmErr) - } else if !confirmed { - return trace.Wrap(trace.CompareFailed(msg), "user declined to overwrite slot") + generatePrivateKey := func() (*hardwarekey.Signer, error) { + ref, err := y.generatePrivateKey(pivSlot, config.Policy) + if err != nil { + return nil, trace.Wrap(err) } - return nil + hwPriv := hardwarekey.NewSigner(s, ref) + cachedKeys[pivSlot] = hwPriv + return hwPriv, nil } // If a custom slot was not specified, check for a key in the // default slot for the given policy and generate a new one if needed. - if slot == "" { - // Check the client certificate in the slot. + if config.CustomSlot == "" { switch cert, err := y.getCertificate(pivSlot); { - case err == nil && (len(cert.Subject.Organization) == 0 || cert.Subject.Organization[0] != certOrgName): - // Unknown cert found, prompt the user before we overwrite the slot. - if err := promptOverwriteSlot(nonTeleportCertificateMessage(pivSlot, cert)); err != nil { - return nil, trace.Wrap(err) - } - - // user confirmed, generate a new key. - fallthrough case errors.Is(err, piv.ErrNotFound): - // no cert found, generate a new key. - priv, err := y.generatePrivateKeyAndCert(pivSlot, requiredKeyPolicy) - return priv, trace.Wrap(err) + return generatePrivateKey() + case err != nil: return nil, trace.Wrap(err) + + // Unknown cert found, this slot could be in use by a non-teleport client. + // Prompt the user before we overwrite the slot. + case len(cert.Subject.Organization) == 0 || cert.Subject.Organization[0] != certOrgName: + if err := promptOverwriteSlot(ctx, prompt, nonTeleportCertificateMessage(pivSlot, cert)); err != nil { + return nil, trace.Wrap(err) + } + return generatePrivateKey() } } - // Get the key in the slot, or generate a new one if needed. - priv, err := y.newYubiKeyPrivateKey(pivSlot) + // Check for an existing key in the slot that satisfies the required + // prompt policy, or generate a new one if needed. + slotCert, attCert, att, err := y.attestKey(pivSlot) switch { - case err == nil && !requiredKeyPolicy.IsSatisfiedBy(priv.GetPrivateKeyPolicy()): - // Key does not meet the required key policy, prompt the user before we overwrite the slot. - msg := fmt.Sprintf("private key in YubiKey PIV slot %q does not meet private key policy %q.", pivSlot, requiredKeyPolicy) - if err := promptOverwriteSlot(msg); err != nil { - return nil, trace.Wrap(err) - } + case errors.Is(err, piv.ErrNotFound): + return generatePrivateKey() - // user confirmed, generate a new key. - fallthrough - case trace.IsNotFound(err): - // no key found, generate a new key. - priv, err = y.generatePrivateKeyAndCert(pivSlot, requiredKeyPolicy) - return priv, trace.Wrap(err) case err != nil: return nil, trace.Wrap(err) + + case config.Policy.TouchRequired && att.TouchPolicy == piv.TouchPolicyNever: + msg := fmt.Sprintf("private key in YubiKey PIV slot %q does not require touch.", pivSlot) + if err := promptOverwriteSlot(ctx, prompt, msg); err != nil { + return nil, trace.Wrap(err) + } + return generatePrivateKey() + + case config.Policy.PINRequired && att.PINPolicy == piv.PINPolicyNever: + msg := fmt.Sprintf("private key in YubiKey PIV slot %q does not require PIN", pivSlot) + if err := promptOverwriteSlot(ctx, prompt, msg); err != nil { + return nil, trace.Wrap(err) + } + return generatePrivateKey() } - return priv, nil + return hardwarekey.NewSigner(s, &hardwarekey.PrivateKeyRef{ + SerialNumber: y.serialNumber, + SlotKey: slotKey, + PublicKey: slotCert.PublicKey, + Policy: hardwarekey.PromptPolicy{ + TouchRequired: att.TouchPolicy != piv.TouchPolicyNever, + PINRequired: att.PINPolicy != piv.PINPolicyNever, + }, + AttestationStatement: &hardwarekey.AttestationStatement{ + AttestationStatement: &attestationv1.AttestationStatement_YubikeyAttestationStatement{ + YubikeyAttestationStatement: &attestationv1.YubiKeyAttestationStatement{ + SlotCert: slotCert.Raw, + AttestationCert: attCert.Raw, + }, + }, + }, + }), nil } -func getKeyPolicies(policy PrivateKeyPolicy) (piv.TouchPolicy, piv.PINPolicy, error) { - switch policy { - case PrivateKeyPolicyHardwareKey: - return piv.TouchPolicyNever, piv.PINPolicyNever, nil - case PrivateKeyPolicyHardwareKeyTouch: - return piv.TouchPolicyCached, piv.PINPolicyNever, nil - case PrivateKeyPolicyHardwareKeyPIN: - return piv.TouchPolicyNever, piv.PINPolicyOnce, nil - case PrivateKeyPolicyHardwareKeyTouchAndPIN: - return piv.TouchPolicyCached, piv.PINPolicyOnce, nil - default: - return piv.TouchPolicyNever, piv.PINPolicyNever, trace.BadParameter("unexpected private key policy %v", policy) +func promptOverwriteSlot(ctx context.Context, prompt hardwarekey.Prompt, msg string) error { + promptQuestion := fmt.Sprintf("%v\nWould you like to overwrite this slot's private key and certificate?", msg) + if confirmed, confirmErr := prompt.ConfirmSlotOverwrite(ctx, promptQuestion); confirmErr != nil { + return trace.Wrap(confirmErr) + } else if !confirmed { + return trace.Wrap(trace.CompareFailed(msg), "user declined to overwrite slot") } + return nil } func nonTeleportCertificateMessage(slot piv.Slot, cert *x509.Certificate) string { @@ -222,12 +228,9 @@ const ( signTouchPromptDelay = time.Millisecond * 200 ) -func (y *YubiKeyPrivateKey) sign(ctx context.Context, rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { - // To prevent concurrent calls to sign from failing due to PIV only handling a - // single connection, use a lock to queue through signature requests one at a time. - y.signMux.Lock() - defer y.signMux.Unlock() +func (y *YubiKey) sign(ctx context.Context, ref *hardwarekey.PrivateKeyRef, prompt hardwarekey.Prompt, rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { ctx, cancel := context.WithCancelCause(ctx) + defer cancel(nil) // Lock the connection for the entire duration of the sign // process. Without this, the connection will be released, @@ -240,7 +243,7 @@ func (y *YubiKeyPrivateKey) sign(ctx context.Context, rand io.Reader, digest []b defer release() var touchPromptDelayTimer *time.Timer - if y.attestation.TouchPolicy != piv.TouchPolicyNever { + if ref.Policy.TouchRequired { touchPromptDelayTimer = time.NewTimer(signTouchPromptDelay) defer touchPromptDelayTimer.Stop() @@ -248,7 +251,7 @@ func (y *YubiKeyPrivateKey) sign(ctx context.Context, rand io.Reader, digest []b select { case <-touchPromptDelayTimer.C: // Prompt for touch after a delay, in case the function succeeds without touch due to a cached touch. - err := y.prompt.Touch(ctx) + err := prompt.Touch(ctx) if err != nil { // Cancel the entire function when an error occurs. // This is typically used for aborting the prompt. @@ -270,13 +273,18 @@ func (y *YubiKeyPrivateKey) sign(ctx context.Context, rand io.Reader, digest []b defer touchPromptDelayTimer.Reset(signTouchPromptDelay) } } - pass, err := y.prompt.AskPIN(ctx, hardwarekey.PINRequired) + pass, err := prompt.AskPIN(ctx, hardwarekey.PINRequired) return pass, trace.Wrap(err) } + pinPolicy := piv.PINPolicyNever + if ref.Policy.PINRequired { + pinPolicy = piv.PINPolicyOnce + } + auth := piv.KeyAuth{ PINPrompt: promptPIN, - PINPolicy: y.attestation.PINPolicy, + PINPolicy: pinPolicy, } // YubiKeys with firmware version 5.3.1 have a bug where insVerify(0x20, 0x00, 0x80, nil) @@ -286,14 +294,19 @@ func (y *YubiKeyPrivateKey) sign(ctx context.Context, rand io.Reader, digest []b // the signature fails. manualRetryWithPIN := false fw531 := piv.Version{Major: 5, Minor: 3, Patch: 1} - if auth.PINPolicy == piv.PINPolicyOnce && y.attestation.Version == fw531 { + if auth.PINPolicy == piv.PINPolicyOnce && y.conn.Version() == fw531 { // Set the keys PIN policy to never to skip the insVerify check. If PIN was provided in // a previous recent call, the signature will succeed as expected of the "once" policy. auth.PINPolicy = piv.PINPolicyNever manualRetryWithPIN = true } - privateKey, err := y.privateKey(y.pivSlot, y.Public(), auth) + pivSlot, err := parsePIVSlot(ref.SlotKey) + if err != nil { + return nil, trace.Wrap(err) + } + + privateKey, err := y.privateKey(pivSlot, ref.PublicKey, auth) if err != nil { return nil, trace.Wrap(err) } @@ -396,9 +409,36 @@ func (y *YubiKey) Reset() error { return trace.Wrap(err) } -// generatePrivateKeyAndCert generates a new private key and client metadata cert in the given PIV slot. -func (y *YubiKey) generatePrivateKeyAndCert(slot piv.Slot, requiredKeyPolicy PrivateKeyPolicy) (*YubiKeyPrivateKey, error) { - if err := y.generatePrivateKey(slot, requiredKeyPolicy); err != nil { +// generatePrivateKey generates a new private key in the given PIV slot. +func (y *YubiKey) generatePrivateKey(slot piv.Slot, policy hardwarekey.PromptPolicy) (*hardwarekey.PrivateKeyRef, error) { + touchPolicy := piv.TouchPolicyNever + if policy.TouchRequired { + touchPolicy = piv.TouchPolicyCached + } + + pinPolicy := piv.PINPolicyNever + if policy.PINRequired { + pinPolicy = piv.PINPolicyOnce + } + + opts := piv.Key{ + Algorithm: piv.AlgorithmEC256, + PINPolicy: pinPolicy, + TouchPolicy: touchPolicy, + } + + pub, err := y.generateKey(piv.DefaultManagementKey, slot, opts) + if err != nil { + return nil, trace.Wrap(err) + } + + slotCert, err := y.attest(slot) + if err != nil { + return nil, trace.Wrap(err) + } + + attCert, err := y.attestationCertificate() + if err != nil { return nil, trace.Wrap(err) } @@ -409,7 +449,20 @@ func (y *YubiKey) generatePrivateKeyAndCert(slot piv.Slot, requiredKeyPolicy Pri return nil, trace.Wrap(err) } - return y.newYubiKeyPrivateKey(slot) + return &hardwarekey.PrivateKeyRef{ + SerialNumber: y.serialNumber, + SlotKey: hardwarekey.PIVSlotKey(slot.Key), + PublicKey: pub, + Policy: policy, + AttestationStatement: &hardwarekey.AttestationStatement{ + AttestationStatement: &attestationv1.AttestationStatement_YubikeyAttestationStatement{ + YubikeyAttestationStatement: &attestationv1.YubiKeyAttestationStatement{ + SlotCert: slotCert.Raw, + AttestationCert: attCert.Raw, + }, + }, + }, + }, nil } // SetMetadataCertificate creates a self signed certificate and stores it in the YubiKey's @@ -432,71 +485,25 @@ func (y *YubiKey) getCertificate(slot piv.Slot) (*x509.Certificate, error) { return cert, trace.Wrap(err) } -// generatePrivateKey generates a new private key in the given PIV slot. -func (y *YubiKey) generatePrivateKey(slot piv.Slot, requiredKeyPolicy PrivateKeyPolicy) error { - touchPolicy, pinPolicy, err := getKeyPolicies(requiredKeyPolicy) +// attestKey attests the key in the given PIV slot. +// The key's public key can be found in the returned slotCert. +func (y *YubiKey) attestKey(slot piv.Slot) (slotCert *x509.Certificate, attCert *x509.Certificate, att *piv.Attestation, err error) { + slotCert, err = y.attest(slot) if err != nil { - return trace.Wrap(err) + return nil, nil, nil, trace.Wrap(err) } - opts := piv.Key{ - Algorithm: piv.AlgorithmEC256, - PINPolicy: pinPolicy, - TouchPolicy: touchPolicy, - } - - _, err = y.generateKey(piv.DefaultManagementKey, slot, opts) - return trace.Wrap(err) -} - -// getPrivateKey gets an existing private key from the given PIV slot. -func (y *YubiKey) getPrivateKey(slot piv.Slot) (*YubiKeyPrivateKey, error) { - cachedKeysMu.Lock() - defer cachedKeysMu.Unlock() - - if key, ok := cachedKeys[slot]; ok { - return key, nil - } - - priv, err := y.newYubiKeyPrivateKey(slot) + attCert, err = y.attestationCertificate() if err != nil { - return nil, trace.Wrap(err) + return nil, nil, nil, trace.Wrap(err) } - cachedKeys[slot] = priv - return priv, nil -} - -// newYubiKeyPrivateKey prepares a [YubiKeyPrivateKey] from an existing private key in the given PIV slot. -// This method must be called under [cachedKeysMu] lock. -func (y *YubiKey) newYubiKeyPrivateKey(slot piv.Slot) (*YubiKeyPrivateKey, error) { - slotCert, err := y.attest(slot) - if errors.Is(err, piv.ErrNotFound) { - return nil, trace.NotFound("private key in YubiKey PIV slot %q not found.", slot.String()) - } else if err != nil { - return nil, trace.Wrap(err) - } - - attCert, err := y.attestationCertificate() - if err != nil { - return nil, trace.Wrap(err) - } - - attestation, err := piv.Verify(attCert, slotCert) + att, err = piv.Verify(attCert, slotCert) if err != nil { - return nil, trace.Wrap(err) - } - - priv := &YubiKeyPrivateKey{ - YubiKey: y, - pivSlot: slot, - slotCert: slotCert, - attestationCert: attCert, - attestation: attestation, + return nil, nil, nil, trace.Wrap(err) } - cachedKeys[slot] = priv - return priv, nil + return slotCert, attCert, att, nil } // SetPIN sets the YubiKey PIV PIN. This doesn't require user interaction like touch, just the correct old PIN. @@ -638,7 +645,7 @@ func (c *sharedPIVConnection) reset() error { // Clear cached keys. cachedKeysMu.Lock() defer cachedKeysMu.Unlock() - cachedKeys = make(map[piv.Slot]*YubiKeyPrivateKey) + cachedKeys = make(map[piv.Slot]*hardwarekey.Signer) return trace.Wrap(c.conn.Reset()) } From a9c916dd21f4b145df2ec3fc310405618540f29f Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 2 Apr 2025 16:25:59 -0700 Subject: [PATCH 3/8] Move PIV implementation into new piv package. --- .../keys/{piv_service.go => piv/service.go} | 2 +- .../service_fake.go} | 2 +- .../service_test.go} | 15 +++++----- .../service_unavailable.go} | 2 +- api/utils/keys/{ => piv}/yubikey.go | 29 ++++++++++--------- api/utils/keys/privatekey.go | 3 +- api/utils/keys/privatekey_test.go | 3 +- lib/client/api.go | 3 +- 8 files changed, 32 insertions(+), 27 deletions(-) rename api/utils/keys/{piv_service.go => piv/service.go} (99%) rename api/utils/keys/{piv_service_fake.go => piv/service_fake.go} (99%) rename api/utils/keys/{piv_service_test.go => piv/service_test.go} (93%) rename api/utils/keys/{piv_service_unavailable.go => piv/service_unavailable.go} (99%) rename api/utils/keys/{ => piv}/yubikey.go (97%) diff --git a/api/utils/keys/piv_service.go b/api/utils/keys/piv/service.go similarity index 99% rename from api/utils/keys/piv_service.go rename to api/utils/keys/piv/service.go index 5ee8a2280393f..3bb253815c5a2 100644 --- a/api/utils/keys/piv_service.go +++ b/api/utils/keys/piv/service.go @@ -14,7 +14,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package keys +package piv import ( "context" diff --git a/api/utils/keys/piv_service_fake.go b/api/utils/keys/piv/service_fake.go similarity index 99% rename from api/utils/keys/piv_service_fake.go rename to api/utils/keys/piv/service_fake.go index 7d38248e84cfc..58f4fac8447df 100644 --- a/api/utils/keys/piv_service_fake.go +++ b/api/utils/keys/piv/service_fake.go @@ -14,7 +14,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package keys +package piv import ( "github.com/gravitational/teleport/api/utils/keys/hardwarekey" diff --git a/api/utils/keys/piv_service_test.go b/api/utils/keys/piv/service_test.go similarity index 93% rename from api/utils/keys/piv_service_test.go rename to api/utils/keys/piv/service_test.go index 2fe2fc3024176..a71ae5ec04c40 100644 --- a/api/utils/keys/piv_service_test.go +++ b/api/utils/keys/piv/service_test.go @@ -14,7 +14,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package keys_test +package piv_test import ( "context" @@ -29,6 +29,7 @@ import ( "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/keys/hardwarekey" + "github.com/gravitational/teleport/api/utils/keys/piv" "github.com/gravitational/teleport/api/utils/prompt" ) @@ -47,9 +48,9 @@ func TestGetYubiKeyPrivateKey_Interactive(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - s := keys.NewYubiKeyService(hardwarekey.NewStdCLIPrompt()) + s := piv.NewYubiKeyService(hardwarekey.NewStdCLIPrompt()) - y, err := keys.FindYubiKey(0, hardwarekey.NewStdCLIPrompt()) + y, err := piv.FindYubiKey(0, hardwarekey.NewStdCLIPrompt()) require.NoError(t, err) resetYubikey(t, y) @@ -121,9 +122,9 @@ func TestOverwritePrompt(t *testing.T) { ctx := context.Background() - s := keys.NewYubiKeyService(hardwarekey.NewStdCLIPrompt()) + s := piv.NewYubiKeyService(hardwarekey.NewStdCLIPrompt()) - y, err := keys.FindYubiKey(0, hardwarekey.NewStdCLIPrompt()) + y, err := piv.FindYubiKey(0, hardwarekey.NewStdCLIPrompt()) require.NoError(t, err) resetYubikey(t, y) @@ -173,12 +174,12 @@ func TestOverwritePrompt(t *testing.T) { } // resetYubikey connects to the first yubiKey and resets it to defaults. -func resetYubikey(t *testing.T, y *keys.YubiKey) { +func resetYubikey(t *testing.T, y *piv.YubiKey) { t.Helper() require.NoError(t, y.Reset()) } -func setupPINPrompt(t *testing.T, y *keys.YubiKey) { +func setupPINPrompt(t *testing.T, y *piv.YubiKey) { t.Helper() // Set pin for tests. diff --git a/api/utils/keys/piv_service_unavailable.go b/api/utils/keys/piv/service_unavailable.go similarity index 99% rename from api/utils/keys/piv_service_unavailable.go rename to api/utils/keys/piv/service_unavailable.go index 52463d31e2568..211f1aff934c5 100644 --- a/api/utils/keys/piv_service_unavailable.go +++ b/api/utils/keys/piv/service_unavailable.go @@ -14,7 +14,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package keys +package piv import ( "context" diff --git a/api/utils/keys/yubikey.go b/api/utils/keys/piv/yubikey.go similarity index 97% rename from api/utils/keys/yubikey.go rename to api/utils/keys/piv/yubikey.go index 78bb113d96c03..505222ac7503f 100644 --- a/api/utils/keys/yubikey.go +++ b/api/utils/keys/piv/yubikey.go @@ -1,19 +1,20 @@ //go:build piv && !pivtest -/* -Copyright 2022 Gravitational, Inc. -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package keys +// Copyright 2022 Gravitational, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package piv import ( "context" diff --git a/api/utils/keys/privatekey.go b/api/utils/keys/privatekey.go index 4c20f5ee89006..84fabeb44854c 100644 --- a/api/utils/keys/privatekey.go +++ b/api/utils/keys/privatekey.go @@ -33,6 +33,7 @@ import ( "golang.org/x/crypto/ssh" "github.com/gravitational/teleport/api/utils/keys/hardwarekey" + "github.com/gravitational/teleport/api/utils/keys/piv" "github.com/gravitational/teleport/api/utils/sshutils/ppk" ) @@ -284,7 +285,7 @@ func ParsePrivateKey(keyPEM []byte, opts ...ParsePrivateKeyOpt) (*PrivateKey, er // TODO(Joerger): Initialize the hardware key service early in the process and store // it in the client store. This allows the process to properly share PIV connections // and prompt logic (pin caching, etc.). - hwKeyService := NewYubiKeyService(appliedOpts.CustomHardwareKeyPrompt) + hwKeyService := piv.NewYubiKeyService(appliedOpts.CustomHardwareKeyPrompt) hwPrivateKey, err := hardwarekey.DecodeSigner(hwKeyService, block.Bytes) if err != nil { return nil, trace.Wrap(err, "failed to parse hardware key signer") diff --git a/api/utils/keys/privatekey_test.go b/api/utils/keys/privatekey_test.go index 621a032fbbc64..e624c7d8cdd85 100644 --- a/api/utils/keys/privatekey_test.go +++ b/api/utils/keys/privatekey_test.go @@ -40,6 +40,7 @@ import ( "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/keys/hardwarekey" + "github.com/gravitational/teleport/api/utils/keys/piv" ) func TestMarshalAndParseKey(t *testing.T) { @@ -53,7 +54,7 @@ func TestMarshalAndParseKey(t *testing.T) { // TODO(Joerger): Once the hardware key service is provided to the key parsing logic, // use [hardwarekey.NewMockHardwareKeyService] and remove pivtest build tag - s := keys.NewYubiKeyService(nil) + s := piv.NewYubiKeyService(nil) hwPriv, err := s.NewPrivateKey(context.TODO(), hardwarekey.PrivateKeyConfig{}) require.NoError(t, err) diff --git a/lib/client/api.go b/lib/client/api.go index 20174f8d8ab27..f3bb0aa11cbab 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -72,6 +72,7 @@ import ( "github.com/gravitational/teleport/api/utils/grpc/interceptors" "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/keys/hardwarekey" + "github.com/gravitational/teleport/api/utils/keys/piv" "github.com/gravitational/teleport/api/utils/prompt" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/auth/touchid" @@ -4006,7 +4007,7 @@ func (tc *TeleportClient) GetNewLoginKeyRing(ctx context.Context) (keyRing *KeyR // TODO(Joerger): Initialize the hardware key service early in the process and store // it in the client store. This allows the process to properly share PIV connections // and prompt logic (pin caching, etc.). - hwks := keys.NewYubiKeyService(tc.CustomHardwareKeyPrompt) + hwks := piv.NewYubiKeyService(tc.CustomHardwareKeyPrompt) priv, err := keys.NewHardwarePrivateKey(ctx, hwks, hardwarekey.PrivateKeyConfig{ Policy: tc.PrivateKeyPolicy.GetPromptPolicy(), CustomSlot: tc.PIVSlot, From 12d51f8d0ec573a5c8cf25b99cdfa48a48d1cf6a Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 2 Apr 2025 16:44:11 -0700 Subject: [PATCH 4/8] Move newPrivateKey into YubiKeyService.NewPrivateKey. --- api/utils/keys/piv/service.go | 151 +++++++++++++++++++++++++++++++++- api/utils/keys/piv/yubikey.go | 151 ---------------------------------- 2 files changed, 150 insertions(+), 152 deletions(-) diff --git a/api/utils/keys/piv/service.go b/api/utils/keys/piv/service.go index 3bb253815c5a2..36b126047238c 100644 --- a/api/utils/keys/piv/service.go +++ b/api/utils/keys/piv/service.go @@ -19,6 +19,11 @@ package piv import ( "context" "crypto" + "crypto/sha256" + "crypto/x509" + "encoding/hex" + "errors" + "fmt" "io" "sync" @@ -68,7 +73,116 @@ func NewYubiKeyService(prompt hardwarekey.Prompt) *YubiKeyService { // - touch & pin -> 9d // - touch & !pin -> 9e func (s *YubiKeyService) NewPrivateKey(ctx context.Context, config hardwarekey.PrivateKeyConfig) (*hardwarekey.Signer, error) { - return newPrivateKey(ctx, config, s.prompt, s) + // Get the requested or default PIV slot. + var slotKey hardwarekey.PIVSlotKey + var err error + if config.CustomSlot != "" { + slotKey, err = config.CustomSlot.Parse() + } else { + slotKey, err = hardwarekey.GetDefaultSlotKey(config.Policy) + } + if err != nil { + return nil, trace.Wrap(err) + } + + pivSlot, err := parsePIVSlot(slotKey) + if err != nil { + return nil, trace.Wrap(err) + } + + // If the program has already retrieved and cached this key, return it. + cachedKeysMu.Lock() + defer cachedKeysMu.Unlock() + + if key, ok := cachedKeys[pivSlot]; ok && key.GetPromptPolicy() == config.Policy { + return key, nil + } + + // Use the first yubiKey we find. + y, err := FindYubiKey(0, s.prompt) + if err != nil { + return nil, trace.Wrap(err) + } + + // If PIN is required, check that PIN and PUK are not the defaults. + if config.Policy.PINRequired { + if err := y.checkOrSetPIN(ctx); err != nil { + return nil, trace.Wrap(err) + } + } + + generatePrivateKey := func() (*hardwarekey.Signer, error) { + ref, err := y.generatePrivateKey(pivSlot, config.Policy) + if err != nil { + return nil, trace.Wrap(err) + } + hwPriv := hardwarekey.NewSigner(s, ref) + cachedKeys[pivSlot] = hwPriv + return hwPriv, nil + } + + // If a custom slot was not specified, check for a key in the + // default slot for the given policy and generate a new one if needed. + if config.CustomSlot == "" { + switch cert, err := y.getCertificate(pivSlot); { + case errors.Is(err, piv.ErrNotFound): + return generatePrivateKey() + + case err != nil: + return nil, trace.Wrap(err) + + // Unknown cert found, this slot could be in use by a non-teleport client. + // Prompt the user before we overwrite the slot. + case len(cert.Subject.Organization) == 0 || cert.Subject.Organization[0] != certOrgName: + if err := s.promptOverwriteSlot(ctx, nonTeleportCertificateMessage(pivSlot, cert)); err != nil { + return nil, trace.Wrap(err) + } + return generatePrivateKey() + } + } + + // Check for an existing key in the slot that satisfies the required + // prompt policy, or generate a new one if needed. + slotCert, attCert, att, err := y.attestKey(pivSlot) + switch { + case errors.Is(err, piv.ErrNotFound): + return generatePrivateKey() + + case err != nil: + return nil, trace.Wrap(err) + + case config.Policy.TouchRequired && att.TouchPolicy == piv.TouchPolicyNever: + msg := fmt.Sprintf("private key in YubiKey PIV slot %q does not require touch.", pivSlot) + if err := s.promptOverwriteSlot(ctx, msg); err != nil { + return nil, trace.Wrap(err) + } + return generatePrivateKey() + + case config.Policy.PINRequired && att.PINPolicy == piv.PINPolicyNever: + msg := fmt.Sprintf("private key in YubiKey PIV slot %q does not require PIN", pivSlot) + if err := s.promptOverwriteSlot(ctx, msg); err != nil { + return nil, trace.Wrap(err) + } + return generatePrivateKey() + } + + return hardwarekey.NewSigner(s, &hardwarekey.PrivateKeyRef{ + SerialNumber: y.serialNumber, + SlotKey: slotKey, + PublicKey: slotCert.PublicKey, + Policy: hardwarekey.PromptPolicy{ + TouchRequired: att.TouchPolicy != piv.TouchPolicyNever, + PINRequired: att.PINPolicy != piv.PINPolicyNever, + }, + AttestationStatement: &hardwarekey.AttestationStatement{ + AttestationStatement: &attestationv1.AttestationStatement_YubikeyAttestationStatement{ + YubikeyAttestationStatement: &attestationv1.YubiKeyAttestationStatement{ + SlotCert: slotCert.Raw, + AttestationCert: attCert.Raw, + }, + }, + }, + }), nil } // Sign performs a cryptographic signature using the specified hardware @@ -163,3 +277,38 @@ func getYubiKey(serialNumber uint32, prompt hardwarekey.Prompt) (*YubiKey, error yubiKeys[y.serialNumber] = y return y, nil } + +func (s *YubiKeyService) promptOverwriteSlot(ctx context.Context, msg string) error { + promptQuestion := fmt.Sprintf("%v\nWould you like to overwrite this slot's private key and certificate?", msg) + if confirmed, confirmErr := s.prompt.ConfirmSlotOverwrite(ctx, promptQuestion); confirmErr != nil { + return trace.Wrap(confirmErr) + } else if !confirmed { + return trace.Wrap(trace.CompareFailed(msg), "user declined to overwrite slot") + } + return nil +} + +func nonTeleportCertificateMessage(slot piv.Slot, cert *x509.Certificate) string { + // Gather a small list of user-readable x509 certificate fields to display to the user. + sum := sha256.Sum256(cert.Raw) + fingerPrint := hex.EncodeToString(sum[:]) + return fmt.Sprintf(`Certificate in YubiKey PIV slot %q is not a Teleport client cert: +Slot %s: + Algorithm: %v + Subject DN: %v + Issuer DN: %v + Serial: %v + Fingerprint: %v + Not before: %v + Not after: %v +`, + slot, slot, + cert.SignatureAlgorithm, + cert.Subject, + cert.Issuer, + cert.SerialNumber, + fingerPrint, + cert.NotBefore, + cert.NotAfter, + ) +} diff --git a/api/utils/keys/piv/yubikey.go b/api/utils/keys/piv/yubikey.go index 505222ac7503f..ac8e366410104 100644 --- a/api/utils/keys/piv/yubikey.go +++ b/api/utils/keys/piv/yubikey.go @@ -22,11 +22,8 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/rand" - "crypto/sha256" "crypto/x509" "crypto/x509/pkix" - "encoding/hex" - "errors" "fmt" "io" "math/big" @@ -64,154 +61,6 @@ var ( cachedKeysMu sync.Mutex ) -func newPrivateKey(ctx context.Context, config hardwarekey.PrivateKeyConfig, prompt hardwarekey.Prompt, s hardwarekey.Service) (*hardwarekey.Signer, error) { - // Get the requested or default PIV slot. - var slotKey hardwarekey.PIVSlotKey - var err error - if config.CustomSlot != "" { - slotKey, err = config.CustomSlot.Parse() - } else { - slotKey, err = hardwarekey.GetDefaultSlotKey(config.Policy) - } - if err != nil { - return nil, trace.Wrap(err) - } - - pivSlot, err := parsePIVSlot(slotKey) - if err != nil { - return nil, trace.Wrap(err) - } - - // If the program has already retrieved and cached this key, return it. - cachedKeysMu.Lock() - defer cachedKeysMu.Unlock() - - if key, ok := cachedKeys[pivSlot]; ok && key.GetPromptPolicy() == config.Policy { - return key, nil - } - - // Use the first yubiKey we find. - y, err := getYubiKey(0, prompt) - if err != nil { - return nil, trace.Wrap(err) - } - - // If PIN is required, check that PIN and PUK are not the defaults. - if config.Policy.PINRequired { - if err := y.checkOrSetPIN(ctx); err != nil { - return nil, trace.Wrap(err) - } - } - - generatePrivateKey := func() (*hardwarekey.Signer, error) { - ref, err := y.generatePrivateKey(pivSlot, config.Policy) - if err != nil { - return nil, trace.Wrap(err) - } - hwPriv := hardwarekey.NewSigner(s, ref) - cachedKeys[pivSlot] = hwPriv - return hwPriv, nil - } - - // If a custom slot was not specified, check for a key in the - // default slot for the given policy and generate a new one if needed. - if config.CustomSlot == "" { - switch cert, err := y.getCertificate(pivSlot); { - case errors.Is(err, piv.ErrNotFound): - return generatePrivateKey() - - case err != nil: - return nil, trace.Wrap(err) - - // Unknown cert found, this slot could be in use by a non-teleport client. - // Prompt the user before we overwrite the slot. - case len(cert.Subject.Organization) == 0 || cert.Subject.Organization[0] != certOrgName: - if err := promptOverwriteSlot(ctx, prompt, nonTeleportCertificateMessage(pivSlot, cert)); err != nil { - return nil, trace.Wrap(err) - } - return generatePrivateKey() - } - } - - // Check for an existing key in the slot that satisfies the required - // prompt policy, or generate a new one if needed. - slotCert, attCert, att, err := y.attestKey(pivSlot) - switch { - case errors.Is(err, piv.ErrNotFound): - return generatePrivateKey() - - case err != nil: - return nil, trace.Wrap(err) - - case config.Policy.TouchRequired && att.TouchPolicy == piv.TouchPolicyNever: - msg := fmt.Sprintf("private key in YubiKey PIV slot %q does not require touch.", pivSlot) - if err := promptOverwriteSlot(ctx, prompt, msg); err != nil { - return nil, trace.Wrap(err) - } - return generatePrivateKey() - - case config.Policy.PINRequired && att.PINPolicy == piv.PINPolicyNever: - msg := fmt.Sprintf("private key in YubiKey PIV slot %q does not require PIN", pivSlot) - if err := promptOverwriteSlot(ctx, prompt, msg); err != nil { - return nil, trace.Wrap(err) - } - return generatePrivateKey() - } - - return hardwarekey.NewSigner(s, &hardwarekey.PrivateKeyRef{ - SerialNumber: y.serialNumber, - SlotKey: slotKey, - PublicKey: slotCert.PublicKey, - Policy: hardwarekey.PromptPolicy{ - TouchRequired: att.TouchPolicy != piv.TouchPolicyNever, - PINRequired: att.PINPolicy != piv.PINPolicyNever, - }, - AttestationStatement: &hardwarekey.AttestationStatement{ - AttestationStatement: &attestationv1.AttestationStatement_YubikeyAttestationStatement{ - YubikeyAttestationStatement: &attestationv1.YubiKeyAttestationStatement{ - SlotCert: slotCert.Raw, - AttestationCert: attCert.Raw, - }, - }, - }, - }), nil -} - -func promptOverwriteSlot(ctx context.Context, prompt hardwarekey.Prompt, msg string) error { - promptQuestion := fmt.Sprintf("%v\nWould you like to overwrite this slot's private key and certificate?", msg) - if confirmed, confirmErr := prompt.ConfirmSlotOverwrite(ctx, promptQuestion); confirmErr != nil { - return trace.Wrap(confirmErr) - } else if !confirmed { - return trace.Wrap(trace.CompareFailed(msg), "user declined to overwrite slot") - } - return nil -} - -func nonTeleportCertificateMessage(slot piv.Slot, cert *x509.Certificate) string { - // Gather a small list of user-readable x509 certificate fields to display to the user. - sum := sha256.Sum256(cert.Raw) - fingerPrint := hex.EncodeToString(sum[:]) - return fmt.Sprintf(`Certificate in YubiKey PIV slot %q is not a Teleport client cert: -Slot %s: - Algorithm: %v - Subject DN: %v - Issuer DN: %v - Serial: %v - Fingerprint: %v - Not before: %v - Not after: %v -`, - slot, slot, - cert.SignatureAlgorithm, - cert.Subject, - cert.Issuer, - cert.SerialNumber, - fingerPrint, - cert.NotBefore, - cert.NotAfter, - ) -} - // YubiKeys require touch when signing with a private key that requires touch. // Unfortunately, there is no good way to check whether touch is cached by the // PIV module at a given time. In order to require touch only when needed, we From ca72bb60c5315f606031b65ee791b1adf708f93e Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 2 Apr 2025 16:46:47 -0700 Subject: [PATCH 5/8] * Clean up separation of concerns between YubiKey and YubiKeyService * Replace hardwarekey.PrivateKey cache with YubiKey cache for proper support for multiple clusters or yubikeys * Add YubiKey version field for use in version specific signature edge cases --- api/utils/keys/piv/service.go | 76 +++++--- api/utils/keys/piv/service_test.go | 4 +- api/utils/keys/piv/service_unavailable.go | 2 - api/utils/keys/piv/yubikey.go | 218 +++++++++------------- 4 files changed, 144 insertions(+), 156 deletions(-) diff --git a/api/utils/keys/piv/service.go b/api/utils/keys/piv/service.go index 36b126047238c..cd8d05bc2e5ba 100644 --- a/api/utils/keys/piv/service.go +++ b/api/utils/keys/piv/service.go @@ -14,6 +14,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Package piv provides a PIV implementation of [hardwarekey.Service]. package piv import ( @@ -25,6 +26,7 @@ import ( "errors" "fmt" "io" + "os" "sync" "github.com/go-piv/piv-go/piv" @@ -40,10 +42,12 @@ var ( // yubiKeys is a shared, thread-safe [YubiKey] cache by serial number. It allows for // separate goroutines to share a YubiKey connection to work around the single PC/SC // transaction (connection) limit. - // - // TODO(Joerger): This will replace the key cache in yubikey.go yubiKeys map[uint32]*YubiKey = map[uint32]*YubiKey{} yubiKeysMux sync.Mutex + + // promptMux is used to prevent over-prompting, especially for back-to-back sign requests + // since touch/PIN from the first signature should be cached for following signatures. + promptMux sync.Mutex ) // YubiKeyService is a YubiKey PIV implementation of [hardwarekey.Service]. @@ -59,6 +63,7 @@ func NewYubiKeyService(prompt hardwarekey.Prompt) *YubiKeyService { if prompt == nil { prompt = hardwarekey.NewStdCLIPrompt() } + return &YubiKeyService{ prompt: prompt, } @@ -73,9 +78,14 @@ func NewYubiKeyService(prompt hardwarekey.Prompt) *YubiKeyService { // - touch & pin -> 9d // - touch & !pin -> 9e func (s *YubiKeyService) NewPrivateKey(ctx context.Context, config hardwarekey.PrivateKeyConfig) (*hardwarekey.Signer, error) { + // Use the first yubiKey we find. + y, err := s.getYubiKey(0) + if err != nil { + return nil, trace.Wrap(err) + } + // Get the requested or default PIV slot. var slotKey hardwarekey.PIVSlotKey - var err error if config.CustomSlot != "" { slotKey, err = config.CustomSlot.Parse() } else { @@ -90,23 +100,9 @@ func (s *YubiKeyService) NewPrivateKey(ctx context.Context, config hardwarekey.P return nil, trace.Wrap(err) } - // If the program has already retrieved and cached this key, return it. - cachedKeysMu.Lock() - defer cachedKeysMu.Unlock() - - if key, ok := cachedKeys[pivSlot]; ok && key.GetPromptPolicy() == config.Policy { - return key, nil - } - - // Use the first yubiKey we find. - y, err := FindYubiKey(0, s.prompt) - if err != nil { - return nil, trace.Wrap(err) - } - // If PIN is required, check that PIN and PUK are not the defaults. if config.Policy.PINRequired { - if err := y.checkOrSetPIN(ctx); err != nil { + if err := s.checkOrSetPIN(ctx, y); err != nil { return nil, trace.Wrap(err) } } @@ -116,9 +112,7 @@ func (s *YubiKeyService) NewPrivateKey(ctx context.Context, config hardwarekey.P if err != nil { return nil, trace.Wrap(err) } - hwPriv := hardwarekey.NewSigner(s, ref) - cachedKeys[pivSlot] = hwPriv - return hwPriv, nil + return hardwarekey.NewSigner(s, ref), nil } // If a custom slot was not specified, check for a key in the @@ -188,11 +182,14 @@ func (s *YubiKeyService) NewPrivateKey(ctx context.Context, config hardwarekey.P // Sign performs a cryptographic signature using the specified hardware // private key and provided signature parameters. func (s *YubiKeyService) Sign(ctx context.Context, ref *hardwarekey.PrivateKeyRef, rand io.Reader, digest []byte, opts crypto.SignerOpts) (signature []byte, err error) { - y, err := getYubiKey(ref.SerialNumber, s.prompt) + y, err := s.getYubiKey(ref.SerialNumber) if err != nil { return nil, trace.Wrap(err) } + promptMux.Lock() + defer promptMux.Unlock() + return y.sign(ctx, ref, s.prompt, rand, digest, opts) } @@ -222,7 +219,7 @@ func (s *YubiKeyService) GetFullKeyRef(serialNumber uint32, slotKey hardwarekey. return ref, nil } - y, err := getYubiKey(serialNumber, s.prompt) + y, err := s.getYubiKey(serialNumber) if err != nil { return nil, trace.Wrap(err) } @@ -261,7 +258,7 @@ func (s *YubiKeyService) GetFullKeyRef(serialNumber uint32, slotKey hardwarekey. // Get the given YubiKey with the serial number. If the provided serialNumber is "0", // return the first YubiKey found in the smart card list. -func getYubiKey(serialNumber uint32, prompt hardwarekey.Prompt) (*YubiKey, error) { +func (s *YubiKeyService) getYubiKey(serialNumber uint32) (*YubiKey, error) { yubiKeysMux.Lock() defer yubiKeysMux.Unlock() @@ -269,7 +266,7 @@ func getYubiKey(serialNumber uint32, prompt hardwarekey.Prompt) (*YubiKey, error return y, nil } - y, err := FindYubiKey(serialNumber, prompt) + y, err := FindYubiKey(serialNumber) if err != nil { return nil, trace.Wrap(err) } @@ -278,7 +275,36 @@ func getYubiKey(serialNumber uint32, prompt hardwarekey.Prompt) (*YubiKey, error return y, nil } +// checkOrSetPIN prompts the user for PIN and verifies it with the YubiKey. +// If the user provides the default PIN, they will be prompted to set a +// non-default PIN and PUK before continuing. +func (s *YubiKeyService) checkOrSetPIN(ctx context.Context, y *YubiKey) error { + promptMux.Lock() + defer promptMux.Unlock() + + pin, err := s.prompt.AskPIN(ctx, hardwarekey.PINOptional) + if err != nil { + return trace.Wrap(err) + } + + switch pin { + case piv.DefaultPIN: + fmt.Fprintf(os.Stderr, "The default PIN %q is not supported.\n", piv.DefaultPIN) + fallthrough + case "": + pin, err = y.setPINAndPUKFromDefault(ctx, s.prompt) + if err != nil { + return trace.Wrap(err) + } + } + + return trace.Wrap(y.verifyPIN(pin)) +} + func (s *YubiKeyService) promptOverwriteSlot(ctx context.Context, msg string) error { + promptMux.Lock() + defer promptMux.Unlock() + promptQuestion := fmt.Sprintf("%v\nWould you like to overwrite this slot's private key and certificate?", msg) if confirmed, confirmErr := s.prompt.ConfirmSlotOverwrite(ctx, promptQuestion); confirmErr != nil { return trace.Wrap(confirmErr) diff --git a/api/utils/keys/piv/service_test.go b/api/utils/keys/piv/service_test.go index a71ae5ec04c40..3f08ab31192de 100644 --- a/api/utils/keys/piv/service_test.go +++ b/api/utils/keys/piv/service_test.go @@ -50,7 +50,7 @@ func TestGetYubiKeyPrivateKey_Interactive(t *testing.T) { s := piv.NewYubiKeyService(hardwarekey.NewStdCLIPrompt()) - y, err := piv.FindYubiKey(0, hardwarekey.NewStdCLIPrompt()) + y, err := piv.FindYubiKey(0) require.NoError(t, err) resetYubikey(t, y) @@ -124,7 +124,7 @@ func TestOverwritePrompt(t *testing.T) { s := piv.NewYubiKeyService(hardwarekey.NewStdCLIPrompt()) - y, err := piv.FindYubiKey(0, hardwarekey.NewStdCLIPrompt()) + y, err := piv.FindYubiKey(0) require.NoError(t, err) resetYubikey(t, y) diff --git a/api/utils/keys/piv/service_unavailable.go b/api/utils/keys/piv/service_unavailable.go index 211f1aff934c5..8cdbeb428f213 100644 --- a/api/utils/keys/piv/service_unavailable.go +++ b/api/utils/keys/piv/service_unavailable.go @@ -45,8 +45,6 @@ func (s *unavailableYubiKeyPIVService) Sign(_ context.Context, _ *hardwarekey.Pr return nil, trace.Wrap(errPIVUnavailable) } -func (s *unavailableYubiKeyPIVService) SetPrompt(_ hardwarekey.Prompt) {} - func (s *unavailableYubiKeyPIVService) GetFullKeyRef(serialNumber uint32, slotKey hardwarekey.PIVSlotKey) (*hardwarekey.PrivateKeyRef, error) { return nil, trace.Wrap(errPIVUnavailable) } diff --git a/api/utils/keys/piv/yubikey.go b/api/utils/keys/piv/yubikey.go index ac8e366410104..851509e417f4d 100644 --- a/api/utils/keys/piv/yubikey.go +++ b/api/utils/keys/piv/yubikey.go @@ -24,10 +24,8 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" - "fmt" "io" "math/big" - "os" "strings" "sync" "time" @@ -41,25 +39,88 @@ import ( "github.com/gravitational/teleport/api/utils/retryutils" ) -const ( - // PIVCardTypeYubiKey is the PIV card type assigned to yubiKeys. - PIVCardTypeYubiKey = "yubikey" -) +// YubiKey is a specific YubiKey PIV card. +type YubiKey struct { + // conn is a shared YubiKey PIV connection. + // + // PIV connections claim an exclusive lock on the PIV module until closed. + // In order to improve connection sharing for this program without locking + // out other programs during extended program executions (like "tsh proxy ssh"), + // this connections is opportunistically formed and released after being + // unused for a few seconds. + *sharedPIVConnection + // serialNumber is the YubiKey's 8 digit serial number. + serialNumber uint32 + // version is the YubiKey's version. + version piv.Version +} -// Cache keys to prevent reconnecting to PIV module to discover a known key. -// -// Additionally, this allows the program to cache the key's PIN (if applicable) -// after the user is prompted the first time, preventing redundant prompts when -// the key is retrieved multiple times. -// -// Note: in most cases the connection caches the PIN itself, and connections can be -// reclaimed before they are fully closed (within a few seconds). However, in uncommon -// setups, this PIN caching does not actually work as expected, so we handle it instead. -// See https://github.com/go-piv/piv-go/issues/47 -var ( - cachedKeys = map[piv.Slot]*hardwarekey.Signer{} - cachedKeysMu sync.Mutex -) +// FindYubiKey finds a YubiKey PIV card by serial number. If no serial +// number is provided, the first YubiKey found will be returned. +func FindYubiKey(serialNumber uint32) (*YubiKey, error) { + yubiKeyCards, err := findYubiKeyCards() + if err != nil { + return nil, trace.Wrap(err) + } + + if len(yubiKeyCards) == 0 { + if serialNumber != 0 { + return nil, trace.ConnectionProblem(nil, "no YubiKey device connected with serial number %d", serialNumber) + } + return nil, trace.ConnectionProblem(nil, "no YubiKey device connected") + } + + for _, card := range yubiKeyCards { + y, err := newYubiKey(card) + if err != nil { + return nil, trace.Wrap(err) + } + + if serialNumber == 0 || y.serialNumber == serialNumber { + return y, nil + } + } + + return nil, trace.ConnectionProblem(nil, "no YubiKey device connected with serial number %d", serialNumber) +} + +// pivCardTypeYubiKey is the PIV card type assigned to yubiKeys. +const pivCardTypeYubiKey = "yubikey" + +// findYubiKeyCards returns a list of connected yubiKey PIV card names. +func findYubiKeyCards() ([]string, error) { + cards, err := piv.Cards() + if err != nil { + return nil, trace.Wrap(err) + } + + var yubiKeyCards []string + for _, card := range cards { + if strings.Contains(strings.ToLower(card), pivCardTypeYubiKey) { + yubiKeyCards = append(yubiKeyCards, card) + } + } + + return yubiKeyCards, nil +} + +func newYubiKey(card string) (*YubiKey, error) { + y := &YubiKey{ + sharedPIVConnection: &sharedPIVConnection{ + card: card, + }, + } + + var err error + if y.serialNumber, err = y.getSerialNumber(); err != nil { + return nil, trace.Wrap(err) + } + if y.version, err = y.getVersion(); err != nil { + return nil, trace.Wrap(err) + } + + return y, nil +} // YubiKeys require touch when signing with a private key that requires touch. // Unfortunately, there is no good way to check whether touch is cached by the @@ -221,38 +282,6 @@ func abandonableSign(ctx context.Context, signer crypto.Signer, rand io.Reader, } } -// YubiKey is a specific YubiKey PIV card. -type YubiKey struct { - // conn is a shared YubiKey PIV connection. - // - // PIV connections claim an exclusive lock on the PIV module until closed. - // In order to improve connection sharing for this program without locking - // out other programs during extended program executions (like "tsh proxy ssh"), - // this connections is opportunistically formed and released after being - // unused for a few seconds. - *sharedPIVConnection - // serialNumber is the yubiKey's 8 digit serial number. - serialNumber uint32 - prompt hardwarekey.Prompt -} - -func newYubiKey(card string, prompt hardwarekey.Prompt) (*YubiKey, error) { - y := &YubiKey{ - sharedPIVConnection: &sharedPIVConnection{ - card: card, - }, - prompt: prompt, - } - - serialNumber, err := y.serial() - if err != nil { - return nil, trace.Wrap(err) - } - - y.serialNumber = serialNumber - return y, nil -} - // Reset resets the YubiKey PIV module to default settings. func (y *YubiKey) Reset() error { err := y.reset() @@ -362,28 +391,6 @@ func (y *YubiKey) SetPIN(oldPin, newPin string) error { return trace.Wrap(err) } -// checkOrSetPIN prompts the user for PIN and verifies it with the YubiKey. -// If the user provides the default PIN, they will be prompted to set a -// non-default PIN and PUK before continuing. -func (y *YubiKey) checkOrSetPIN(ctx context.Context) error { - pin, err := y.prompt.AskPIN(ctx, hardwarekey.PINOptional) - if err != nil { - return trace.Wrap(err) - } - - switch pin { - case piv.DefaultPIN: - fmt.Fprintf(os.Stderr, "The default PIN %q is not supported.\n", piv.DefaultPIN) - fallthrough - case "": - if pin, err = y.setPINAndPUKFromDefault(ctx, y.prompt); err != nil { - return trace.Wrap(err) - } - } - - return trace.Wrap(y.verifyPIN(pin)) -} - type sharedPIVConnection struct { // card is a reader name used to find and connect to this yubiKey. // This value may change between OS's, or with other system changes. @@ -475,7 +482,7 @@ func (c *sharedPIVConnection) privateKey(slot piv.Slot, public crypto.PublicKey, return privateKey, trace.Wrap(err) } -func (c *sharedPIVConnection) serial() (uint32, error) { +func (c *sharedPIVConnection) getSerialNumber() (uint32, error) { release, err := c.connect() if err != nil { return 0, trace.Wrap(err) @@ -485,18 +492,21 @@ func (c *sharedPIVConnection) serial() (uint32, error) { return serial, trace.Wrap(err) } +func (c *sharedPIVConnection) getVersion() (piv.Version, error) { + release, err := c.connect() + if err != nil { + return piv.Version{}, trace.Wrap(err) + } + defer release() + return c.conn.Version(), nil +} + func (c *sharedPIVConnection) reset() error { release, err := c.connect() if err != nil { return trace.Wrap(err) } defer release() - - // Clear cached keys. - cachedKeysMu.Lock() - defer cachedKeysMu.Unlock() - cachedKeys = make(map[piv.Slot]*hardwarekey.Signer) - return trace.Wrap(c.conn.Reset()) } @@ -613,52 +623,6 @@ func isRetryError(err error) bool { return strings.Contains(err.Error(), retryError) } -// FindYubiKey finds a yubiKey PIV card by serial number. If no serial -// number is provided, the first yubiKey found will be returned. -func FindYubiKey(serialNumber uint32, prompt hardwarekey.Prompt) (*YubiKey, error) { - yubiKeyCards, err := findYubiKeyCards() - if err != nil { - return nil, trace.Wrap(err) - } - - if len(yubiKeyCards) == 0 { - if serialNumber != 0 { - return nil, trace.ConnectionProblem(nil, "no YubiKey device connected with serial number %d", serialNumber) - } - return nil, trace.ConnectionProblem(nil, "no YubiKey device connected") - } - - for _, card := range yubiKeyCards { - y, err := newYubiKey(card, prompt) - if err != nil { - return nil, trace.Wrap(err) - } - - if serialNumber == 0 || y.serialNumber == serialNumber { - return y, nil - } - } - - return nil, trace.ConnectionProblem(nil, "no YubiKey device connected with serial number %d", serialNumber) -} - -// findYubiKeyCards returns a list of connected yubiKey PIV card names. -func findYubiKeyCards() ([]string, error) { - cards, err := piv.Cards() - if err != nil { - return nil, trace.Wrap(err) - } - - var yubiKeyCards []string - for _, card := range cards { - if strings.Contains(strings.ToLower(card), PIVCardTypeYubiKey) { - yubiKeyCards = append(yubiKeyCards, card) - } - } - - return yubiKeyCards, nil -} - func parsePIVSlot(slotKey hardwarekey.PIVSlotKey) (piv.Slot, error) { switch uint32(slotKey) { case piv.SlotAuthentication.Key: From 69e8f53e046c51b3d24e574ed5204aba3c30feae Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 2 Apr 2025 17:01:21 -0700 Subject: [PATCH 6/8] Change sharedPIVConnection connection from an embedded field. --- api/utils/keys/piv/yubikey.go | 85 ++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 40 deletions(-) diff --git a/api/utils/keys/piv/yubikey.go b/api/utils/keys/piv/yubikey.go index 851509e417f4d..a935519921eef 100644 --- a/api/utils/keys/piv/yubikey.go +++ b/api/utils/keys/piv/yubikey.go @@ -48,7 +48,7 @@ type YubiKey struct { // out other programs during extended program executions (like "tsh proxy ssh"), // this connections is opportunistically formed and released after being // unused for a few seconds. - *sharedPIVConnection + c *sharedPIVConnection // serialNumber is the YubiKey's 8 digit serial number. serialNumber uint32 // version is the YubiKey's version. @@ -106,16 +106,16 @@ func findYubiKeyCards() ([]string, error) { func newYubiKey(card string) (*YubiKey, error) { y := &YubiKey{ - sharedPIVConnection: &sharedPIVConnection{ + c: &sharedPIVConnection{ card: card, }, } var err error - if y.serialNumber, err = y.getSerialNumber(); err != nil { + if y.serialNumber, err = y.c.getSerialNumber(); err != nil { return nil, trace.Wrap(err) } - if y.version, err = y.getVersion(); err != nil { + if y.version, err = y.c.getVersion(); err != nil { return nil, trace.Wrap(err) } @@ -147,7 +147,7 @@ func (y *YubiKey) sign(ctx context.Context, ref *hardwarekey.PrivateKeyRef, prom // process. Without this, the connection will be released, // leading to a failure when providing PIN or touch input: // "verify pin: transmitting request: the supplied handle was invalid". - release, err := y.connect() + release, err := y.c.connect() if err != nil { return nil, trace.Wrap(err) } @@ -205,7 +205,7 @@ func (y *YubiKey) sign(ctx context.Context, ref *hardwarekey.PrivateKeyRef, prom // the signature fails. manualRetryWithPIN := false fw531 := piv.Version{Major: 5, Minor: 3, Patch: 1} - if auth.PINPolicy == piv.PINPolicyOnce && y.conn.Version() == fw531 { + if auth.PINPolicy == piv.PINPolicyOnce && y.c.conn.Version() == fw531 { // Set the keys PIN policy to never to skip the insVerify check. If PIN was provided in // a previous recent call, the signature will succeed as expected of the "once" policy. auth.PINPolicy = piv.PINPolicyNever @@ -217,7 +217,7 @@ func (y *YubiKey) sign(ctx context.Context, ref *hardwarekey.PrivateKeyRef, prom return nil, trace.Wrap(err) } - privateKey, err := y.privateKey(pivSlot, ref.PublicKey, auth) + privateKey, err := y.c.privateKey(pivSlot, ref.PublicKey, auth) if err != nil { return nil, trace.Wrap(err) } @@ -240,7 +240,7 @@ func (y *YubiKey) sign(ctx context.Context, ref *hardwarekey.PrivateKeyRef, prom if err != nil { return nil, trace.Wrap(err) } - if err := y.verifyPIN(pin); err != nil { + if err := y.c.verifyPIN(pin); err != nil { return nil, trace.Wrap(err) } signature, err := abandonableSign(ctx, signer, rand, digest, opts) @@ -284,7 +284,7 @@ func abandonableSign(ctx context.Context, signer crypto.Signer, rand io.Reader, // Reset resets the YubiKey PIV module to default settings. func (y *YubiKey) Reset() error { - err := y.reset() + err := y.c.reset() return trace.Wrap(err) } @@ -306,17 +306,17 @@ func (y *YubiKey) generatePrivateKey(slot piv.Slot, policy hardwarekey.PromptPol TouchPolicy: touchPolicy, } - pub, err := y.generateKey(piv.DefaultManagementKey, slot, opts) + pub, err := y.c.generateKey(piv.DefaultManagementKey, slot, opts) if err != nil { return nil, trace.Wrap(err) } - slotCert, err := y.attest(slot) + slotCert, err := y.c.attest(slot) if err != nil { return nil, trace.Wrap(err) } - attCert, err := y.attestationCertificate() + attCert, err := y.c.attestationCertificate() if err != nil { return nil, trace.Wrap(err) } @@ -354,25 +354,25 @@ func (y *YubiKey) SetMetadataCertificate(slot piv.Slot, subject pkix.Name) error return trace.Wrap(err) } - err = y.setCertificate(piv.DefaultManagementKey, slot, cert) + err = y.c.setCertificate(piv.DefaultManagementKey, slot, cert) return trace.Wrap(err) } // getCertificate gets a certificate from the given PIV slot. func (y *YubiKey) getCertificate(slot piv.Slot) (*x509.Certificate, error) { - cert, err := y.certificate(slot) + cert, err := y.c.certificate(slot) return cert, trace.Wrap(err) } // attestKey attests the key in the given PIV slot. // The key's public key can be found in the returned slotCert. func (y *YubiKey) attestKey(slot piv.Slot) (slotCert *x509.Certificate, attCert *x509.Certificate, att *piv.Attestation, err error) { - slotCert, err = y.attest(slot) + slotCert, err = y.c.attest(slot) if err != nil { return nil, nil, nil, trace.Wrap(err) } - attCert, err = y.attestationCertificate() + attCert, err = y.c.attestationCertificate() if err != nil { return nil, nil, nil, trace.Wrap(err) } @@ -387,7 +387,35 @@ func (y *YubiKey) attestKey(slot piv.Slot) (slotCert *x509.Certificate, attCert // SetPIN sets the YubiKey PIV PIN. This doesn't require user interaction like touch, just the correct old PIN. func (y *YubiKey) SetPIN(oldPin, newPin string) error { - err := y.setPIN(oldPin, newPin) + err := y.c.setPIN(oldPin, newPin) + return trace.Wrap(err) +} + +func (y *YubiKey) setPINAndPUKFromDefault(ctx context.Context, prompt hardwarekey.Prompt) (string, error) { + pinAndPUK, err := prompt.ChangePIN(ctx) + if err != nil { + return "", trace.Wrap(err) + } + + if err := pinAndPUK.Validate(); err != nil { + return "", trace.Wrap(err) + } + + if pinAndPUK.PUKChanged { + if err := y.c.setPUK(piv.DefaultPUK, pinAndPUK.PUK); err != nil { + return "", trace.Wrap(err) + } + } + + if err := y.c.unblock(pinAndPUK.PUK, pinAndPUK.PIN); err != nil { + return "", trace.Wrap(err) + } + + return pinAndPUK.PIN, nil +} + +func (y *YubiKey) verifyPIN(pin string) error { + err := y.c.verifyPIN(pin) return trace.Wrap(err) } @@ -595,29 +623,6 @@ func (c *sharedPIVConnection) verifyPIN(pin string) error { return trace.Wrap(c.conn.VerifyPIN(pin)) } -func (c *sharedPIVConnection) setPINAndPUKFromDefault(ctx context.Context, prompt hardwarekey.Prompt) (string, error) { - pinAndPUK, err := prompt.ChangePIN(ctx) - if err != nil { - return "", trace.Wrap(err) - } - - if err := pinAndPUK.Validate(); err != nil { - return "", trace.Wrap(err) - } - - if pinAndPUK.PUKChanged { - if err := c.setPUK(piv.DefaultPUK, pinAndPUK.PUK); err != nil { - return "", trace.Wrap(err) - } - } - - if err := c.unblock(pinAndPUK.PUK, pinAndPUK.PIN); err != nil { - return "", trace.Wrap(err) - } - - return pinAndPUK.PIN, nil -} - func isRetryError(err error) bool { const retryError = "connecting to smart card: the smart card cannot be accessed because of other connections outstanding" return strings.Contains(err.Error(), retryError) From 0a9d7e8c47c6cd36c330e264a9b11424b7851856 Mon Sep 17 00:00:00 2001 From: joerger Date: Fri, 4 Apr 2025 11:45:43 -0700 Subject: [PATCH 7/8] Address comments. --- api/utils/keys/piv/service.go | 2 +- api/utils/keys/piv/yubikey.go | 55 ++++++++++++++++++----------------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/api/utils/keys/piv/service.go b/api/utils/keys/piv/service.go index cd8d05bc2e5ba..5c55ca83d6871 100644 --- a/api/utils/keys/piv/service.go +++ b/api/utils/keys/piv/service.go @@ -41,7 +41,7 @@ import ( var ( // yubiKeys is a shared, thread-safe [YubiKey] cache by serial number. It allows for // separate goroutines to share a YubiKey connection to work around the single PC/SC - // transaction (connection) limit. + // transaction (connection) per-yubikey limit. yubiKeys map[uint32]*YubiKey = map[uint32]*YubiKey{} yubiKeysMux sync.Mutex diff --git a/api/utils/keys/piv/yubikey.go b/api/utils/keys/piv/yubikey.go index a935519921eef..70bf2a07ed12a 100644 --- a/api/utils/keys/piv/yubikey.go +++ b/api/utils/keys/piv/yubikey.go @@ -40,23 +40,24 @@ import ( ) // YubiKey is a specific YubiKey PIV card. +// The [sharedPIVConnection] field makes its methods thread-safe. type YubiKey struct { // conn is a shared YubiKey PIV connection. // - // PIV connections claim an exclusive lock on the PIV module until closed. - // In order to improve connection sharing for this program without locking - // out other programs during extended program executions (like "tsh proxy ssh"), - // this connections is opportunistically formed and released after being - // unused for a few seconds. - c *sharedPIVConnection + // For each YubiKey, PIV connections claim an exclusive lock on the key's + // PIV module until closed. In order to improve connection sharing for this + // program without locking out other programs during extended program executions + // (like "tsh proxy ssh"), this connections is opportunistically formed and + // released after being unused for a few seconds. + conn *sharedPIVConnection // serialNumber is the YubiKey's 8 digit serial number. serialNumber uint32 // version is the YubiKey's version. version piv.Version } -// FindYubiKey finds a YubiKey PIV card by serial number. If no serial -// number is provided, the first YubiKey found will be returned. +// FindYubiKey finds a YubiKey PIV card by serial number. If the provided +// [serialNumber] is "0", the first YubiKey found will be returned. func FindYubiKey(serialNumber uint32) (*YubiKey, error) { yubiKeyCards, err := findYubiKeyCards() if err != nil { @@ -106,16 +107,16 @@ func findYubiKeyCards() ([]string, error) { func newYubiKey(card string) (*YubiKey, error) { y := &YubiKey{ - c: &sharedPIVConnection{ + conn: &sharedPIVConnection{ card: card, }, } var err error - if y.serialNumber, err = y.c.getSerialNumber(); err != nil { + if y.serialNumber, err = y.conn.getSerialNumber(); err != nil { return nil, trace.Wrap(err) } - if y.version, err = y.c.getVersion(); err != nil { + if y.version, err = y.conn.getVersion(); err != nil { return nil, trace.Wrap(err) } @@ -147,7 +148,7 @@ func (y *YubiKey) sign(ctx context.Context, ref *hardwarekey.PrivateKeyRef, prom // process. Without this, the connection will be released, // leading to a failure when providing PIN or touch input: // "verify pin: transmitting request: the supplied handle was invalid". - release, err := y.c.connect() + release, err := y.conn.connect() if err != nil { return nil, trace.Wrap(err) } @@ -205,7 +206,7 @@ func (y *YubiKey) sign(ctx context.Context, ref *hardwarekey.PrivateKeyRef, prom // the signature fails. manualRetryWithPIN := false fw531 := piv.Version{Major: 5, Minor: 3, Patch: 1} - if auth.PINPolicy == piv.PINPolicyOnce && y.c.conn.Version() == fw531 { + if auth.PINPolicy == piv.PINPolicyOnce && y.conn.conn.Version() == fw531 { // Set the keys PIN policy to never to skip the insVerify check. If PIN was provided in // a previous recent call, the signature will succeed as expected of the "once" policy. auth.PINPolicy = piv.PINPolicyNever @@ -217,7 +218,7 @@ func (y *YubiKey) sign(ctx context.Context, ref *hardwarekey.PrivateKeyRef, prom return nil, trace.Wrap(err) } - privateKey, err := y.c.privateKey(pivSlot, ref.PublicKey, auth) + privateKey, err := y.conn.privateKey(pivSlot, ref.PublicKey, auth) if err != nil { return nil, trace.Wrap(err) } @@ -240,7 +241,7 @@ func (y *YubiKey) sign(ctx context.Context, ref *hardwarekey.PrivateKeyRef, prom if err != nil { return nil, trace.Wrap(err) } - if err := y.c.verifyPIN(pin); err != nil { + if err := y.conn.verifyPIN(pin); err != nil { return nil, trace.Wrap(err) } signature, err := abandonableSign(ctx, signer, rand, digest, opts) @@ -284,7 +285,7 @@ func abandonableSign(ctx context.Context, signer crypto.Signer, rand io.Reader, // Reset resets the YubiKey PIV module to default settings. func (y *YubiKey) Reset() error { - err := y.c.reset() + err := y.conn.reset() return trace.Wrap(err) } @@ -306,17 +307,17 @@ func (y *YubiKey) generatePrivateKey(slot piv.Slot, policy hardwarekey.PromptPol TouchPolicy: touchPolicy, } - pub, err := y.c.generateKey(piv.DefaultManagementKey, slot, opts) + pub, err := y.conn.generateKey(piv.DefaultManagementKey, slot, opts) if err != nil { return nil, trace.Wrap(err) } - slotCert, err := y.c.attest(slot) + slotCert, err := y.conn.attest(slot) if err != nil { return nil, trace.Wrap(err) } - attCert, err := y.c.attestationCertificate() + attCert, err := y.conn.attestationCertificate() if err != nil { return nil, trace.Wrap(err) } @@ -354,25 +355,25 @@ func (y *YubiKey) SetMetadataCertificate(slot piv.Slot, subject pkix.Name) error return trace.Wrap(err) } - err = y.c.setCertificate(piv.DefaultManagementKey, slot, cert) + err = y.conn.setCertificate(piv.DefaultManagementKey, slot, cert) return trace.Wrap(err) } // getCertificate gets a certificate from the given PIV slot. func (y *YubiKey) getCertificate(slot piv.Slot) (*x509.Certificate, error) { - cert, err := y.c.certificate(slot) + cert, err := y.conn.certificate(slot) return cert, trace.Wrap(err) } // attestKey attests the key in the given PIV slot. // The key's public key can be found in the returned slotCert. func (y *YubiKey) attestKey(slot piv.Slot) (slotCert *x509.Certificate, attCert *x509.Certificate, att *piv.Attestation, err error) { - slotCert, err = y.c.attest(slot) + slotCert, err = y.conn.attest(slot) if err != nil { return nil, nil, nil, trace.Wrap(err) } - attCert, err = y.c.attestationCertificate() + attCert, err = y.conn.attestationCertificate() if err != nil { return nil, nil, nil, trace.Wrap(err) } @@ -387,7 +388,7 @@ func (y *YubiKey) attestKey(slot piv.Slot) (slotCert *x509.Certificate, attCert // SetPIN sets the YubiKey PIV PIN. This doesn't require user interaction like touch, just the correct old PIN. func (y *YubiKey) SetPIN(oldPin, newPin string) error { - err := y.c.setPIN(oldPin, newPin) + err := y.conn.setPIN(oldPin, newPin) return trace.Wrap(err) } @@ -402,12 +403,12 @@ func (y *YubiKey) setPINAndPUKFromDefault(ctx context.Context, prompt hardwareke } if pinAndPUK.PUKChanged { - if err := y.c.setPUK(piv.DefaultPUK, pinAndPUK.PUK); err != nil { + if err := y.conn.setPUK(piv.DefaultPUK, pinAndPUK.PUK); err != nil { return "", trace.Wrap(err) } } - if err := y.c.unblock(pinAndPUK.PUK, pinAndPUK.PIN); err != nil { + if err := y.conn.unblock(pinAndPUK.PUK, pinAndPUK.PIN); err != nil { return "", trace.Wrap(err) } @@ -415,7 +416,7 @@ func (y *YubiKey) setPINAndPUKFromDefault(ctx context.Context, prompt hardwareke } func (y *YubiKey) verifyPIN(pin string) error { - err := y.c.verifyPIN(pin) + err := y.conn.verifyPIN(pin) return trace.Wrap(err) } From f17f602a217decc52868caade4b9c6830f96a3ed Mon Sep 17 00:00:00 2001 From: joerger Date: Wed, 9 Apr 2025 11:27:52 -0700 Subject: [PATCH 8/8] Add getKeyRef helper method. --- api/utils/keys/piv/service.go | 45 ++++------------------------- api/utils/keys/piv/yubikey.go | 53 ++++++++++++++++++----------------- 2 files changed, 32 insertions(+), 66 deletions(-) diff --git a/api/utils/keys/piv/service.go b/api/utils/keys/piv/service.go index 5c55ca83d6871..2fb408c79be7b 100644 --- a/api/utils/keys/piv/service.go +++ b/api/utils/keys/piv/service.go @@ -32,7 +32,6 @@ import ( "github.com/go-piv/piv-go/piv" "github.com/gravitational/trace" - attestationv1 "github.com/gravitational/teleport/api/gen/proto/go/attestation/v1" "github.com/gravitational/teleport/api/utils/keys/hardwarekey" ) @@ -137,7 +136,7 @@ func (s *YubiKeyService) NewPrivateKey(ctx context.Context, config hardwarekey.P // Check for an existing key in the slot that satisfies the required // prompt policy, or generate a new one if needed. - slotCert, attCert, att, err := y.attestKey(pivSlot) + keyRef, err := y.getKeyRef(pivSlot) switch { case errors.Is(err, piv.ErrNotFound): return generatePrivateKey() @@ -145,14 +144,14 @@ func (s *YubiKeyService) NewPrivateKey(ctx context.Context, config hardwarekey.P case err != nil: return nil, trace.Wrap(err) - case config.Policy.TouchRequired && att.TouchPolicy == piv.TouchPolicyNever: + case config.Policy.TouchRequired && !keyRef.Policy.TouchRequired: msg := fmt.Sprintf("private key in YubiKey PIV slot %q does not require touch.", pivSlot) if err := s.promptOverwriteSlot(ctx, msg); err != nil { return nil, trace.Wrap(err) } return generatePrivateKey() - case config.Policy.PINRequired && att.PINPolicy == piv.PINPolicyNever: + case config.Policy.PINRequired && !keyRef.Policy.PINRequired: msg := fmt.Sprintf("private key in YubiKey PIV slot %q does not require PIN", pivSlot) if err := s.promptOverwriteSlot(ctx, msg); err != nil { return nil, trace.Wrap(err) @@ -160,23 +159,7 @@ func (s *YubiKeyService) NewPrivateKey(ctx context.Context, config hardwarekey.P return generatePrivateKey() } - return hardwarekey.NewSigner(s, &hardwarekey.PrivateKeyRef{ - SerialNumber: y.serialNumber, - SlotKey: slotKey, - PublicKey: slotCert.PublicKey, - Policy: hardwarekey.PromptPolicy{ - TouchRequired: att.TouchPolicy != piv.TouchPolicyNever, - PINRequired: att.PINPolicy != piv.PINPolicyNever, - }, - AttestationStatement: &hardwarekey.AttestationStatement{ - AttestationStatement: &attestationv1.AttestationStatement_YubikeyAttestationStatement{ - YubikeyAttestationStatement: &attestationv1.YubiKeyAttestationStatement{ - SlotCert: slotCert.Raw, - AttestationCert: attCert.Raw, - }, - }, - }, - }), nil + return hardwarekey.NewSigner(s, keyRef), nil } // Sign performs a cryptographic signature using the specified hardware @@ -229,29 +212,11 @@ func (s *YubiKeyService) GetFullKeyRef(serialNumber uint32, slotKey hardwarekey. return nil, trace.Wrap(err) } - slotCert, attCert, att, err := y.attestKey(pivSlot) + ref, err := y.getKeyRef(pivSlot) if err != nil { return nil, trace.Wrap(err) } - ref := &hardwarekey.PrivateKeyRef{ - SerialNumber: serialNumber, - SlotKey: slotKey, - PublicKey: slotCert.PublicKey, - Policy: hardwarekey.PromptPolicy{ - TouchRequired: att.TouchPolicy != piv.TouchPolicyNever, - PINRequired: att.PINPolicy != piv.PINPolicyNever, - }, - AttestationStatement: &hardwarekey.AttestationStatement{ - AttestationStatement: &attestationv1.AttestationStatement_YubikeyAttestationStatement{ - YubikeyAttestationStatement: &attestationv1.YubiKeyAttestationStatement{ - SlotCert: slotCert.Raw, - AttestationCert: attCert.Raw, - }, - }, - }, - } - keyRefs[baseRef] = ref return ref, nil } diff --git a/api/utils/keys/piv/yubikey.go b/api/utils/keys/piv/yubikey.go index 70bf2a07ed12a..81f9ddd5d7419 100644 --- a/api/utils/keys/piv/yubikey.go +++ b/api/utils/keys/piv/yubikey.go @@ -307,18 +307,7 @@ func (y *YubiKey) generatePrivateKey(slot piv.Slot, policy hardwarekey.PromptPol TouchPolicy: touchPolicy, } - pub, err := y.conn.generateKey(piv.DefaultManagementKey, slot, opts) - if err != nil { - return nil, trace.Wrap(err) - } - - slotCert, err := y.conn.attest(slot) - if err != nil { - return nil, trace.Wrap(err) - } - - attCert, err := y.conn.attestationCertificate() - if err != nil { + if _, err := y.conn.generateKey(piv.DefaultManagementKey, slot, opts); err != nil { return nil, trace.Wrap(err) } @@ -329,20 +318,7 @@ func (y *YubiKey) generatePrivateKey(slot piv.Slot, policy hardwarekey.PromptPol return nil, trace.Wrap(err) } - return &hardwarekey.PrivateKeyRef{ - SerialNumber: y.serialNumber, - SlotKey: hardwarekey.PIVSlotKey(slot.Key), - PublicKey: pub, - Policy: policy, - AttestationStatement: &hardwarekey.AttestationStatement{ - AttestationStatement: &attestationv1.AttestationStatement_YubikeyAttestationStatement{ - YubikeyAttestationStatement: &attestationv1.YubiKeyAttestationStatement{ - SlotCert: slotCert.Raw, - AttestationCert: attCert.Raw, - }, - }, - }, - }, nil + return y.getKeyRef(slot) } // SetMetadataCertificate creates a self signed certificate and stores it in the YubiKey's @@ -386,6 +362,31 @@ func (y *YubiKey) attestKey(slot piv.Slot) (slotCert *x509.Certificate, attCert return slotCert, attCert, att, nil } +func (y *YubiKey) getKeyRef(slot piv.Slot) (*hardwarekey.PrivateKeyRef, error) { + slotCert, attCert, att, err := y.attestKey(slot) + if err != nil { + return nil, trace.Wrap(err) + } + + return &hardwarekey.PrivateKeyRef{ + SerialNumber: y.serialNumber, + SlotKey: hardwarekey.PIVSlotKey(slot.Key), + PublicKey: slotCert.PublicKey, + Policy: hardwarekey.PromptPolicy{ + TouchRequired: att.TouchPolicy != piv.TouchPolicyNever, + PINRequired: att.PINPolicy != piv.PINPolicyNever, + }, + AttestationStatement: &hardwarekey.AttestationStatement{ + AttestationStatement: &attestationv1.AttestationStatement_YubikeyAttestationStatement{ + YubikeyAttestationStatement: &attestationv1.YubiKeyAttestationStatement{ + SlotCert: slotCert.Raw, + AttestationCert: attCert.Raw, + }, + }, + }, + }, nil +} + // SetPIN sets the YubiKey PIV PIN. This doesn't require user interaction like touch, just the correct old PIN. func (y *YubiKey) SetPIN(oldPin, newPin string) error { err := y.conn.setPIN(oldPin, newPin)