From 53a7e23c927e5169373f2981838d76e1eb252d17 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Thu, 21 Nov 2024 18:25:01 +0100 Subject: [PATCH] [v14] Cache PIV connections to share across the program execution (#47954) * Cache PIV connections to share across the program execution (#47091) * Cache yubikey objects. * Cache PIV connections to share across the program execution. * Do not release the connection until `sign` returns * Do not ignore errors * Perform a "warm up" call to YubiKey * Fix tests * Use a specific interface to check if the key can be "warmed up" * Allow abandoning `signer.Sign` call when context is canceled * Make sure that the cached key is valid for the given private key policy The reason for adding this check was failing `invalid key policies` test. * Make `hardwareKeyWarmer` private * Force callers to release connection * Improve comments * Fix lint * Improve `connect` comment * Fix race condition * Simplify `release` logic * Trigger license/cla --------- Co-authored-by: joerger (cherry picked from commit bd6fdbf2c1038984e94572e66f44806866b1afb6) * Sign a hashed message in hardware key warmup call (#48206) Otherwise, signing may fail with "input must be a hashed message" error. (cherry picked from commit 47494db923e61bb12af54f65748584947c59bd9d) * Remove delayed closing of yubikey connection to prevent the connection from leaking after program execution. (#48414) (cherry picked from commit b7c0e79fedf4f6e8043a6cf948cabe2928df89a2) --------- Co-authored-by: Brian Joerger --- api/client/proxy/client.go | 25 +- api/utils/keys/yubikey.go | 560 ++++++++++++++++++++++----------- api/utils/keys/yubikey_test.go | 30 +- 3 files changed, 412 insertions(+), 203 deletions(-) diff --git a/api/client/proxy/client.go b/api/client/proxy/client.go index f8237c0f0c3aa..9782aba41ab16 100644 --- a/api/client/proxy/client.go +++ b/api/client/proxy/client.go @@ -83,7 +83,7 @@ type ClientConfig struct { // CheckAndSetDefaults ensures required options are present and // sets the default value of any that are omitted. -func (c *ClientConfig) CheckAndSetDefaults() error { +func (c *ClientConfig) CheckAndSetDefaults(ctx context.Context) error { if c.ProxyAddress == "" { return trace.BadParameter("missing required parameter ProxyAddress") } @@ -116,6 +116,21 @@ func (c *ClientConfig) CheckAndSetDefaults() error { // not to send the client certificate by looking at certificate request. if len(tlsCfg.Certificates) > 0 { cert := tlsCfg.Certificates[0] + + // When a hardware key is used to store the private key, the user may fail to provide + // a PIN or touch before a gRPC dial timeout occurs. + // The resulting "dial timeout" error is generic and doesn't indicate an issue with the + // hardware key itself (since YubiKey is treated like any other key). + // To avoid this, we perform a "warm-up" call to the key, ensuring it is ready + // before initiating the gRPC dial. + // This approach works because the connection is cached for a few seconds, + // allowing subsequent calls without requiring additional user action. + if priv, ok := cert.PrivateKey.(hardwareKeyWarmer); ok { + err := priv.WarmupHardwareKey(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + } tlsCfg.Certificates = nil tlsCfg.GetClientCertificate = func(_ *tls.CertificateRequestInfo) (*tls.Certificate, error) { return &cert, nil @@ -183,7 +198,7 @@ const protocolProxySSHGRPC string = "teleport-proxy-ssh-grpc" // of the caller, then prefer to use NewSSHClient instead which omits // the gRPC dialing altogether. func NewClient(ctx context.Context, cfg ClientConfig) (*Client, error) { - if err := cfg.CheckAndSetDefaults(); err != nil { + if err := cfg.CheckAndSetDefaults(ctx); err != nil { return nil, trace.Wrap(err) } @@ -439,3 +454,9 @@ func (c *Client) Ping(ctx context.Context) error { _, _ = clt.Ping(ctx, &authpb.PingRequest{}) return nil } + +// hardwareKeyWarmer performs a bogus call to the hardware key, +// to proactively prompt the user for a PIN/touch (if needed). +type hardwareKeyWarmer interface { + WarmupHardwareKey(ctx context.Context) error +} diff --git a/api/utils/keys/yubikey.go b/api/utils/keys/yubikey.go index 3b38c43f4d0f1..4092feacc3bf1 100644 --- a/api/utils/keys/yubikey.go +++ b/api/utils/keys/yubikey.go @@ -51,10 +51,45 @@ const ( PIVCardTypeYubiKey = "yubikey" ) +// 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]*PrivateKey{} + 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 PIVSlot) (*PrivateKey, error) { + cachedKeysMu.Lock() + defer cachedKeysMu.Unlock() + + // Get the default PIV slot or the piv slot requested. + pivSlot, err := GetDefaultKeySlot(requiredKeyPolicy) + if err != nil { + return nil, trace.Wrap(err) + } + if slot != "" { + pivSlot, err = slot.parse() + if err != nil { + return nil, trace.Wrap(err) + } + } + + // If the program has already retrieved and cached this key, return it. + if key, ok := cachedKeys[pivSlot]; ok && key.GetPrivateKeyPolicy() == requiredKeyPolicy { + return key, nil + } + // Use the first yubiKey we find. y, err := FindYubiKey(0) if err != nil { @@ -78,15 +113,9 @@ func getOrGenerateYubiKeyPrivateKey(ctx context.Context, requiredKeyPolicy Priva return nil } - // If a specific slot was specified, use that. Otherwise, check for a key in the + // 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. - var pivSlot piv.Slot - if slot != "" { - pivSlot, err = slot.parse() - if err != nil { - return nil, trace.Wrap(err) - } - } else { + if slot == "" { pivSlot, err = GetDefaultKeySlot(requiredKeyPolicy) if err != nil { return nil, trace.Wrap(err) @@ -214,6 +243,9 @@ type yubiKeyPrivateKeyData struct { } func parseYubiKeyPrivateKeyData(keyDataBytes []byte) (*PrivateKey, error) { + cachedKeysMu.Lock() + defer cachedKeysMu.Unlock() + var keyData yubiKeyPrivateKeyData if err := json.Unmarshal(keyDataBytes, &keyData); err != nil { return nil, trace.Wrap(err) @@ -224,6 +256,11 @@ func parseYubiKeyPrivateKeyData(keyDataBytes []byte) (*PrivateKey, error) { return nil, trace.Wrap(err) } + // If the program has already retrieved and cached this key, return it. + if key, ok := cachedKeys[pivSlot]; ok { + return key, nil + } + y, err := FindYubiKey(keyData.SerialNumber) if err != nil { return nil, trace.Wrap(err) @@ -242,16 +279,19 @@ 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() - // 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() - signature, err := y.sign(ctx, rand, digest, opts) if err != nil { return nil, trace.Wrap(err) @@ -260,12 +300,38 @@ func (y *YubiKeyPrivateKey) Sign(rand io.Reader, digest []byte, opts crypto.Sign 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 +// prompt for touch after a short delay when we expect the request would succeed +// if touch were not required. +// +// There are some X factors which determine how long a request may take, such as the +// YubiKey model and firmware version, so the delays below may need to be adjusted to +// suit more models. The durations mentioned below were retrieved from testing with a +// YubiKey 5 nano (5.2.7) and a YubiKey NFC (5.4.3). +const ( + // piv.ECDSAPrivateKey.Sign consistently takes ~70 milliseconds. However, 200ms + // should be imperceptible the the user and should avoid misfired prompts for + // slower cards (if there are any). + signTouchPromptDelay = time.Millisecond * 200 +) + func (y *YubiKeyPrivateKey) sign(ctx context.Context, rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { - yk, err := y.open() + // 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() + + // Lock the connection for the entire duration of the sign + // 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() if err != nil { return nil, trace.Wrap(err) } - defer yk.Close() + defer release() var touchPromptDelayTimer *time.Timer if y.attestation.TouchPolicy != piv.TouchPolicyNever { @@ -315,7 +381,7 @@ func (y *YubiKeyPrivateKey) sign(ctx context.Context, rand io.Reader, digest []b manualRetryWithPIN = true } - privateKey, err := yk.PrivateKey(y.pivSlot, y.slotCert.PublicKey, auth) + privateKey, err := y.privateKey(y.pivSlot, y.Public(), auth) if err != nil { return nil, trace.Wrap(err) } @@ -329,7 +395,7 @@ func (y *YubiKeyPrivateKey) sign(ctx context.Context, rand io.Reader, digest []b // The piv-go library wraps error codes like this with a user readable message: "security status not satisfied". const pivGenericAuthErrCodeString = "6982" - signature, err := signer.Sign(rand, digest, opts) + signature, err := abandonableSign(ctx, signer, rand, digest, opts) switch { case err == nil: return signature, nil @@ -338,16 +404,48 @@ func (y *YubiKeyPrivateKey) sign(ctx context.Context, rand io.Reader, digest []b if err != nil { return nil, trace.Wrap(err) } - if err := yk.VerifyPIN(pin); err != nil { + if err := y.verifyPIN(pin); err != nil { return nil, trace.Wrap(err) } - signature, err := signer.Sign(rand, digest, opts) + signature, err := abandonableSign(ctx, signer, rand, digest, opts) return signature, trace.Wrap(err) default: return nil, trace.Wrap(err) } } +// abandonableSign is a wrapper around signer.Sign. +// It enhances the functionality of signer.Sign by allowing the caller to stop +// waiting for the result if the provided context is canceled. +// It is especially important for WarmupHardwareKey, +// where waiting for the user providing a PIN/touch could block program termination. +// Important: this function only abandons the signer.Sign result, doesn't cancel it. +func abandonableSign(ctx context.Context, signer crypto.Signer, rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + type signResult struct { + signature []byte + err error + } + + signResultCh := make(chan signResult) + go func() { + if err := ctx.Err(); err != nil { + return + } + signature, err := signer.Sign(rand, digest, opts) + select { + case <-ctx.Done(): + case signResultCh <- signResult{signature: signature, err: trace.Wrap(err)}: + } + }() + + select { + case <-ctx.Done(): + return nil, ctx.Err() + case result := <-signResultCh: + return result.signature, trace.Wrap(result.err) + } +} + func (y *YubiKeyPrivateKey) toPrivateKey() (*PrivateKey, error) { keyPEM, err := y.keyPEM() if err != nil { @@ -412,39 +510,37 @@ func GetPrivateKeyPolicyFromAttestation(att *piv.Attestation) PrivateKeyPolicy { // YubiKey is a specific YubiKey PIV card. type YubiKey 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. - card string + // 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 } func newYubiKey(card string) (*YubiKey, error) { - y := &YubiKey{card: card} - - yk, err := y.open() - if err != nil { - return nil, trace.Wrap(err) + y := &YubiKey{ + sharedPIVConnection: &sharedPIVConnection{ + card: card, + }, } - defer yk.Close() - y.serialNumber, err = yk.Serial() + 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 { - yk, err := y.open() - if err != nil { - return trace.Wrap(err) - } - defer yk.Close() - - err = yk.Reset() + err := y.reset() return trace.Wrap(err) } @@ -469,41 +565,23 @@ func (y *YubiKey) generatePrivateKeyAndCert(slot piv.Slot, requiredKeyPolicy Pri // slot is in used by a Teleport Client and is not fit to be used in cryptographic operations. // This cert is also useful for users to discern where the key came with tools like `ykman piv info`. func (y *YubiKey) SetMetadataCertificate(slot piv.Slot, subject pkix.Name) error { - yk, err := y.open() - if err != nil { - return trace.Wrap(err) - } - defer yk.Close() - cert, err := SelfSignedMetadataCertificate(subject) if err != nil { return trace.Wrap(err) } - err = yk.SetCertificate(piv.DefaultManagementKey, slot, cert) + err = y.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) { - yk, err := y.open() - if err != nil { - return nil, trace.Wrap(err) - } - defer yk.Close() - - cert, err := yk.Certificate(slot) + cert, err := y.certificate(slot) 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 { - yk, err := y.open() - if err != nil { - return trace.Wrap(err) - } - defer yk.Close() - touchPolicy, pinPolicy, err := getKeyPolicies(requiredKeyPolicy) if err != nil { return trace.Wrap(err) @@ -515,26 +593,20 @@ func (y *YubiKey) generatePrivateKey(slot piv.Slot, requiredKeyPolicy PrivateKey TouchPolicy: touchPolicy, } - _, err = yk.GenerateKey(piv.DefaultManagementKey, slot, opts) + _, 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) (*PrivateKey, error) { - yk, err := y.open() - if err != nil { - return nil, trace.Wrap(err) - } - defer yk.Close() - - slotCert, err := yk.Attest(slot) + 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 := yk.AttestationCertificate() + attCert, err := y.attestationCertificate() if err != nil { return nil, trace.Wrap(err) } @@ -562,18 +634,13 @@ func (y *YubiKey) getPrivateKey(slot piv.Slot) (*PrivateKey, error) { return nil, trace.Wrap(err) } + cachedKeys[slot] = key return key, nil } -// SetPin sets the YubiKey PIV PIN. This doesn't require user interaction like touch, just the correct old PIN. +// 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 { - yk, err := y.open() - if err != nil { - return trace.Wrap(err) - } - defer yk.Close() - - err = yk.SetPIN(oldPin, newPin) + err := y.setPIN(oldPin, newPin) return trace.Wrap(err) } @@ -586,29 +653,56 @@ func (y *YubiKey) checkOrSetPIN(ctx context.Context) error { return trace.Wrap(err) } - yk, err := y.open() - if err != nil { - return trace.Wrap(err) - } - defer yk.Close() - switch pin { case piv.DefaultPIN: fmt.Fprintf(os.Stderr, "The default PIN %q is not supported.\n", piv.DefaultPIN) fallthrough case "": - if pin, err = setPINAndPUKFromDefault(ctx, yk); err != nil { + if pin, err = y.setPINAndPUKFromDefault(ctx); err != nil { return trace.Wrap(err) } } - return trace.Wrap(yk.VerifyPIN(pin)) + return trace.Wrap(y.verifyPIN(pin)) } -// open a connection to YubiKey PIV module. The returned connection should be closed once -// it's been used. The YubiKey PIV module itself takes some additional time to handle closed +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. + card string + + // conn is the shared PIV connection. + conn *piv.YubiKey + mu sync.Mutex + activeConnections int +} + +// connect establishes a connection to a YubiKey PIV module and returns a release function. +// The release function should be called to properly close the shared connection. +// The connection is not immediately terminated, allowing other callers to +// use it before it's released. +// The YubiKey PIV module itself takes some additional time to handle closed // connections, so we use a retry loop to give the PIV module time to close prior connections. -func (y *YubiKey) open() (yk *piv.YubiKey, err error) { +func (c *sharedPIVConnection) connect() (func(), error) { + c.mu.Lock() + defer c.mu.Unlock() + + release := func() { + c.mu.Lock() + defer c.mu.Unlock() + + c.activeConnections-- + if c.activeConnections == 0 { + c.conn.Close() + c.conn = nil + } + } + + if c.conn != nil { + c.activeConnections++ + return release, nil + } + linearRetry, err := retryutils.NewLinear(retryutils.LinearConfig{ // If a PIV connection has just been closed, it take ~5 ms to become // available to new connections. For this reason, we initially wait a @@ -629,7 +723,7 @@ func (y *YubiKey) open() (yk *piv.YubiKey, err error) { defer cancel() err = linearRetry.For(retryCtx, func() error { - yk, err = piv.Open(y.card) + c.conn, err = piv.Open(c.card) if err != nil && !isRetryError(err) { return retryutils.PermanentRetryError(err) } @@ -648,146 +742,128 @@ func (y *YubiKey) open() (yk *piv.YubiKey, err error) { } else if err != nil { return nil, trace.Wrap(err) } - return yk, 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) + c.activeConnections++ + return release, nil } -// 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() +func (c *sharedPIVConnection) privateKey(slot piv.Slot, public crypto.PublicKey, auth piv.KeyAuth) (crypto.PrivateKey, error) { + release, err := c.connect() if err != nil { return nil, trace.Wrap(err) } + defer release() + privateKey, err := c.conn.PrivateKey(slot, public, auth) + return privateKey, 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") +func (c *sharedPIVConnection) serial() (uint32, error) { + release, err := c.connect() + if err != nil { + return 0, trace.Wrap(err) } + defer release() + serial, err := c.conn.Serial() + return serial, trace.Wrap(err) +} - 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 - } +func (c *sharedPIVConnection) reset() error { + release, err := c.connect() + if err != nil { + return trace.Wrap(err) } - - return nil, trace.ConnectionProblem(nil, "no YubiKey device connected with serial number %d", serialNumber) + defer release() + // Clear cached keys. + cachedKeys = make(map[piv.Slot]*PrivateKey) + return trace.Wrap(c.conn.Reset()) } -// findYubiKeyCards returns a list of connected yubiKey PIV card names. -func findYubiKeyCards() ([]string, error) { - cards, err := piv.Cards() +func (c *sharedPIVConnection) setCertificate(key [24]byte, slot piv.Slot, cert *x509.Certificate) error { + release, err := c.connect() if err != nil { - return nil, trace.Wrap(err) + return trace.Wrap(err) } + defer release() + return trace.Wrap(c.conn.SetCertificate(key, slot, cert)) +} - var yubiKeyCards []string - for _, card := range cards { - if strings.Contains(strings.ToLower(card), PIVCardTypeYubiKey) { - yubiKeyCards = append(yubiKeyCards, card) - } +func (c *sharedPIVConnection) certificate(slot piv.Slot) (*x509.Certificate, error) { + release, err := c.connect() + if err != nil { + return nil, trace.Wrap(err) } - - return yubiKeyCards, nil + defer release() + cert, err := c.conn.Certificate(slot) + return cert, trace.Wrap(err) } -func (s PIVSlot) validate() error { - _, err := s.parse() - return trace.Wrap(err) +func (c *sharedPIVConnection) generateKey(key [24]byte, slot piv.Slot, opts piv.Key) (crypto.PublicKey, error) { + release, err := c.connect() + if err != nil { + return nil, trace.Wrap(err) + } + defer release() + pubKey, err := c.conn.GenerateKey(key, slot, opts) + return pubKey, trace.Wrap(err) } -func (s PIVSlot) parse() (piv.Slot, error) { - slotKey, err := strconv.ParseUint(string(s), 16, 32) +func (c *sharedPIVConnection) attest(slot piv.Slot) (*x509.Certificate, error) { + release, err := c.connect() if err != nil { - return piv.Slot{}, trace.Wrap(err) + return nil, trace.Wrap(err) } - - return parsePIVSlot(uint32(slotKey)) + defer release() + cert, err := c.conn.Attest(slot) + return cert, trace.Wrap(err) } -func parsePIVSlotString(slotKeyString string) (piv.Slot, error) { - slotKey, err := strconv.ParseUint(slotKeyString, 16, 32) +func (c *sharedPIVConnection) attestationCertificate() (*x509.Certificate, error) { + release, err := c.connect() if err != nil { - return piv.Slot{}, trace.Wrap(err) + return nil, trace.Wrap(err) } - - return parsePIVSlot(uint32(slotKey)) + defer release() + cert, err := c.conn.AttestationCertificate() + return cert, trace.Wrap(err) } -func parsePIVSlot(slotKey uint32) (piv.Slot, error) { - switch slotKey { - case piv.SlotAuthentication.Key: - return piv.SlotAuthentication, nil - case piv.SlotSignature.Key: - return piv.SlotSignature, nil - case piv.SlotKeyManagement.Key: - return piv.SlotKeyManagement, nil - case piv.SlotCardAuthentication.Key: - return piv.SlotCardAuthentication, nil - default: - retiredSlot, ok := piv.RetiredKeyManagementSlot(slotKey) - if !ok { - return piv.Slot{}, trace.BadParameter("slot %X does not exist", slotKey) - } - return retiredSlot, nil +func (c *sharedPIVConnection) setPIN(oldPIN string, newPIN string) error { + release, err := c.connect() + if err != nil { + return trace.Wrap(err) } + defer release() + return trace.Wrap(c.conn.SetPIN(oldPIN, newPIN)) } -// certOrgName is used to identify Teleport Client self-signed certificates stored in yubiKey PIV slots. -const certOrgName = "teleport" - -func SelfSignedMetadataCertificate(subject pkix.Name) (*x509.Certificate, error) { - priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) +func (c *sharedPIVConnection) setPUK(oldPUK string, newPUK string) error { + release, err := c.connect() if err != nil { - return nil, trace.Wrap(err) + return trace.Wrap(err) } + defer release() + return trace.Wrap(c.conn.SetPUK(oldPUK, newPUK)) +} - serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) - serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) // see crypto/tls/generate_cert.go +func (c *sharedPIVConnection) unblock(puk string, newPIN string) error { + release, err := c.connect() if err != nil { - return nil, trace.Wrap(err) - } - cert := &x509.Certificate{ - SerialNumber: serialNumber, - Subject: subject, - PublicKey: priv.Public(), + return trace.Wrap(err) } + defer release() + return trace.Wrap(c.conn.Unblock(puk, newPIN)) +} - if cert.Raw, err = x509.CreateCertificate(rand.Reader, cert, cert, priv.Public(), priv); err != nil { - return nil, trace.Wrap(err) +func (c *sharedPIVConnection) verifyPIN(pin string) error { + release, err := c.connect() + if err != nil { + return trace.Wrap(err) } - return cert, nil + defer release() + return trace.Wrap(c.conn.VerifyPIN(pin)) } -// 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 -// prompt for touch after a short delay when we expect the request would succeed -// if touch were not required. -// -// There are some X factors which determine how long a request may take, such as the -// YubiKey model and firmware version, so the delays below may need to be adjusted to -// suit more models. The durations mentioned below were retrieved from testing with a -// YubiKey 5 nano (5.2.7) and a YubiKey NFC (5.4.3). -const ( - // piv.ECDSAPrivateKey.Sign consistently takes ~70 milliseconds. However, 200ms - // should be imperceptible the the user and should avoid misfired prompts for - // slower cards (if there are any). - signTouchPromptDelay = time.Millisecond * 200 -) - -func setPINAndPUKFromDefault(ctx context.Context, yk *piv.YubiKey) (string, error) { +func (c *sharedPIVConnection) setPINAndPUKFromDefault(ctx context.Context) (string, error) { // YubiKey requires that PIN and PUK be 6-8 characters. isValid := func(pin string) bool { return len(pin) >= 6 && len(pin) <= 8 @@ -860,7 +936,7 @@ func setPINAndPUKFromDefault(ctx context.Context, yk *piv.YubiKey) (string, erro continue } - if err := yk.SetPUK(piv.DefaultPUK, newPUK); err != nil { + if err := c.setPUK(piv.DefaultPUK, newPUK); err != nil { return "", trace.Wrap(err) } @@ -869,9 +945,119 @@ func setPINAndPUKFromDefault(ctx context.Context, yk *piv.YubiKey) (string, erro } } - if err := yk.Unblock(puk, pin); err != nil { + if err := c.unblock(puk, pin); err != nil { return "", trace.Wrap(err) } return 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) +} + +// 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) +} + +// 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 (s PIVSlot) validate() error { + _, err := s.parse() + return trace.Wrap(err) +} + +func (s PIVSlot) parse() (piv.Slot, error) { + slotKey, err := strconv.ParseUint(string(s), 16, 32) + if err != nil { + return piv.Slot{}, trace.Wrap(err) + } + + return parsePIVSlot(uint32(slotKey)) +} + +func parsePIVSlot(slotKey uint32) (piv.Slot, error) { + switch slotKey { + case piv.SlotAuthentication.Key: + return piv.SlotAuthentication, nil + case piv.SlotSignature.Key: + return piv.SlotSignature, nil + case piv.SlotKeyManagement.Key: + return piv.SlotKeyManagement, nil + case piv.SlotCardAuthentication.Key: + return piv.SlotCardAuthentication, nil + default: + retiredSlot, ok := piv.RetiredKeyManagementSlot(slotKey) + if !ok { + return piv.Slot{}, trace.BadParameter("slot %X does not exist", slotKey) + } + return retiredSlot, nil + } +} + +// certOrgName is used to identify Teleport Client self-signed certificates stored in yubiKey PIV slots. +const certOrgName = "teleport" + +func SelfSignedMetadataCertificate(subject pkix.Name) (*x509.Certificate, error) { + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + return nil, trace.Wrap(err) + } + + serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) + serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) // see crypto/tls/generate_cert.go + if err != nil { + return nil, trace.Wrap(err) + } + cert := &x509.Certificate{ + SerialNumber: serialNumber, + Subject: subject, + PublicKey: priv.Public(), + } + + if cert.Raw, err = x509.CreateCertificate(rand.Reader, cert, cert, priv.Public(), priv); err != nil { + return nil, trace.Wrap(err) + } + return cert, nil +} diff --git a/api/utils/keys/yubikey_test.go b/api/utils/keys/yubikey_test.go index 2dacee96bf28e..4e49315a56bad 100644 --- a/api/utils/keys/yubikey_test.go +++ b/api/utils/keys/yubikey_test.go @@ -45,7 +45,11 @@ func TestGetYubiKeyPrivateKey_Interactive(t *testing.T) { fmt.Println("This test is interactive, tap your YubiKey when prompted.") ctx := context.Background() - t.Cleanup(func() { resetYubikey(t) }) + + y, err := keys.FindYubiKey(0) + require.NoError(t, err) + + t.Cleanup(func() { resetYubikey(t, y) }) for _, policy := range []keys.PrivateKeyPolicy{ keys.PrivateKeyPolicyHardwareKey, @@ -56,8 +60,8 @@ func TestGetYubiKeyPrivateKey_Interactive(t *testing.T) { for _, customSlot := range []bool{true, false} { t.Run(fmt.Sprintf("policy:%q", policy), func(t *testing.T) { t.Run(fmt.Sprintf("custom slot:%v", customSlot), func(t *testing.T) { - resetYubikey(t) - setupPINPrompt(t) + resetYubikey(t, y) + setupPINPrompt(t, y) var slot keys.PIVSlot = "" if customSlot { @@ -100,7 +104,11 @@ func TestOverwritePrompt(t *testing.T) { } ctx := context.Background() - t.Cleanup(func() { resetYubikey(t) }) + + y, err := keys.FindYubiKey(0) + require.NoError(t, err) + + t.Cleanup(func() { resetYubikey(t, y) }) // Use a custom slot. pivSlot, err := keys.GetDefaultKeySlot(keys.PrivateKeyPolicyHardwareKeyTouch) @@ -119,11 +127,9 @@ func TestOverwritePrompt(t *testing.T) { } t.Run("invalid metadata cert", func(t *testing.T) { - resetYubikey(t) + resetYubikey(t, y) // Set a non-teleport certificate in the slot. - y, err := keys.FindYubiKey(0) - require.NoError(t, err) err = y.SetMetadataCertificate(pivSlot, pkix.Name{Organization: []string{"not-teleport"}}) require.NoError(t, err) @@ -131,7 +137,7 @@ func TestOverwritePrompt(t *testing.T) { }) t.Run("invalid key policies", func(t *testing.T) { - resetYubikey(t) + resetYubikey(t, y) // Generate a key that does not require touch in the slot that Teleport expects to require touch. _, err := keys.GetYubiKeyPrivateKey(ctx, keys.PrivateKeyPolicyHardwareKey, keys.PIVSlot(pivSlot.String())) @@ -142,17 +148,13 @@ func TestOverwritePrompt(t *testing.T) { } // resetYubikey connects to the first yubiKey and resets it to defaults. -func resetYubikey(t *testing.T) { +func resetYubikey(t *testing.T, y *keys.YubiKey) { t.Helper() - y, err := keys.FindYubiKey(0) - require.NoError(t, err) require.NoError(t, y.Reset()) } -func setupPINPrompt(t *testing.T) { +func setupPINPrompt(t *testing.T, y *keys.YubiKey) { t.Helper() - y, err := keys.FindYubiKey(0) - require.NoError(t, err) // Set pin for tests. const testPIN = "123123"