From e1f12fc06777a5e137209241e2e1dd6587bcf286 Mon Sep 17 00:00:00 2001 From: TicketWindowSeat <255788533+TicketWindowSeat@users.noreply.github.com> Date: Sat, 18 Jul 2026 16:50:53 -0400 Subject: [PATCH] Say when a failed PIN verification tried the default PIN When no PIN is available from pin-value, pin-source or pin-prompt, New falls back to the YubiKey factory default PIN. If the card uses a different PIN, every Sign or Decrypt then sends that wrong PIN: each attempt consumes one of the card's PIN retries and the error verify pin: smart card error 63c2: verification failed (2 retries remaining) reads as if the user mistyped a PIN, when it was this package that chose it. Three such runs block the PIN. This happened in smallstep/cli#1492, where a pin that did not reach the step-kms-plugin subprocess ended in a locked YubiKey whose PIV application had to be reset. Record in New whether the PIN came from the options or from the default, and when a PIN verification fails with the default PIN, say so and name the remedies: verify pin: smart card error 63c2: verification failed (2 retries remaining); the YubiKey factory default PIN was tried because no PIN was available; provide one with pin-value, pin-source or pin-prompt The default itself stays documented, intentional behavior; only the error changes. Fixes #1074 --- kms/yubikey/yubikey.go | 39 ++++++++-- kms/yubikey/yubikey_test.go | 146 +++++++++++++++++++++++++++++++++++- 2 files changed, 177 insertions(+), 8 deletions(-) diff --git a/kms/yubikey/yubikey.go b/kms/yubikey/yubikey.go index a39e5f05..aa2ef481 100644 --- a/kms/yubikey/yubikey.go +++ b/kms/yubikey/yubikey.go @@ -126,6 +126,7 @@ CErVHSJIs+BdtTVNY9AwtyPmnyb0v4mSTzvWdw== type YubiKey struct { yk pivKey pin string + defaultPIN bool card string managementKey []byte } @@ -197,7 +198,9 @@ const maximumManagementKeyLength = 32 // yubikey:slot-id=9a?pin-value=123456 // // If the pin or the management key are not provided, we will use the default -// ones. +// ones. Note that a failed PIN verification consumes one of the card's PIN +// retries, so if the card does not use the default PIN, always provide it +// with pin-value, pin-source or pin-prompt. func New(_ context.Context, opts apiv1.Options) (*YubiKey, error) { pin := "123456" var managementKey [maximumManagementKeyLength]byte @@ -244,7 +247,8 @@ func New(_ context.Context, opts apiv1.Options) (*YubiKey, error) { copy(managementKey[:managementKeyLength], b[:managementKeyLength]) } - if opts.Pin != "" { + defaultPIN := opts.Pin == "" + if !defaultPIN { pin = opts.Pin } @@ -281,6 +285,7 @@ func New(_ context.Context, opts apiv1.Options) (*YubiKey, error) { return &YubiKey{ yk: yk, pin: pin, + defaultPIN: defaultPIN, card: card, managementKey: managementKey[:managementKeyLength], }, nil @@ -406,7 +411,8 @@ func (k *YubiKey) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, e return nil, errors.New("private key is not a crypto.Signer") } return &syncSigner{ - Signer: signer, + Signer: signer, + defaultPIN: k.defaultPIN, }, nil } @@ -444,7 +450,8 @@ func (k *YubiKey) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Dec return nil, errors.New("private key is not a crypto.Decrypter") } return &syncDecrypter{ - Decrypter: decrypter, + Decrypter: decrypter, + defaultPIN: k.defaultPIN, }, nil } @@ -712,12 +719,14 @@ var m sync.Mutex // error 6982: security status not satisfied" with two concurrent signs. type syncSigner struct { crypto.Signer + defaultPIN bool } func (s *syncSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { m.Lock() defer m.Unlock() - return s.Signer.Sign(rand, digest, opts) + sig, err := s.Signer.Sign(rand, digest, opts) + return sig, wrapPINError(err, s.defaultPIN) } // syncDecrypter wraps a crypto.Decrypter with a mutex to avoid the error "smart @@ -725,12 +734,30 @@ func (s *syncSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) // concurrent decryptions. type syncDecrypter struct { crypto.Decrypter + defaultPIN bool } func (s *syncDecrypter) Decrypt(rand io.Reader, msg []byte, opts crypto.DecrypterOpts) ([]byte, error) { m.Lock() defer m.Unlock() - return s.Decrypter.Decrypt(rand, msg, opts) + plain, err := s.Decrypter.Decrypt(rand, msg, opts) + return plain, wrapPINError(err, s.defaultPIN) +} + +// wrapPINError explains where the PIN came from when PIN verification fails +// and no PIN was ever provided: New falls back to the YubiKey factory default +// PIN, each failed verification consumes one of the card's PIN retries, and +// without this context the error reads as if the user mistyped a PIN that +// this package chose itself. +func wrapPINError(err error, defaultPIN bool) error { + if err == nil || !defaultPIN { + return err + } + var authErr piv.AuthErr + if !errors.As(err, &authErr) { + return err + } + return fmt.Errorf("%w; the YubiKey factory default PIN was tried because no PIN was available; provide one with pin-value, pin-source or pin-prompt", err) } var _ apiv1.CertificateManager = (*YubiKey)(nil) diff --git a/kms/yubikey/yubikey_test.go b/kms/yubikey/yubikey_test.go index a8268c9a..2f55c3b9 100644 --- a/kms/yubikey/yubikey_test.go +++ b/kms/yubikey/yubikey_test.go @@ -18,6 +18,7 @@ import ( "encoding/hex" "errors" "fmt" + "io" "os" "path/filepath" "reflect" @@ -351,7 +352,7 @@ func TestNew(t *testing.T) { pivMap = sync.Map{} pivCards = okPivCards pivOpen = okPivOpen - }, &YubiKey{yk: yk, pin: "123456", card: "Yubico YubiKey OTP+FIDO+CCID", managementKey: piv.DefaultManagementKey}, false}, + }, &YubiKey{yk: yk, pin: "123456", defaultPIN: true, card: "Yubico YubiKey OTP+FIDO+CCID", managementKey: piv.DefaultManagementKey}, false}, {"ok with uri", args{ctx, apiv1.Options{ URI: "yubikey:pin-value=111111;management-key=001122334455667788990011223344556677889900112233", }}, func() { @@ -398,11 +399,18 @@ func TestNew(t *testing.T) { pivCards = okPivCards pivOpen = okPivOpen }, &YubiKey{yk: yk, pin: "222222", card: "Yubico YubiKey OTP+FIDO+CCID", managementKey: piv.DefaultManagementKey}, false}, + {"ok with missing pin-source", args{ctx, apiv1.Options{ + URI: "yubikey:pin-source=" + filepath.Join(t.TempDir(), "missing.pin"), + }}, func() { + pivMap = sync.Map{} + pivCards = okPivCards + pivOpen = okPivOpen + }, &YubiKey{yk: yk, pin: "123456", defaultPIN: true, card: "Yubico YubiKey OTP+FIDO+CCID", managementKey: piv.DefaultManagementKey}, false}, {"ok with ManagementKey", args{ctx, apiv1.Options{ManagementKey: "001122334455667788990011223344556677889900112233"}}, func() { pivMap = sync.Map{} pivCards = okPivCards pivOpen = okPivOpen - }, &YubiKey{yk: yk, pin: "123456", card: "Yubico YubiKey OTP+FIDO+CCID", managementKey: []byte{0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0x00, 0x11, 0x22, 0x33}}, false}, + }, &YubiKey{yk: yk, pin: "123456", defaultPIN: true, card: "Yubico YubiKey OTP+FIDO+CCID", managementKey: []byte{0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0x00, 0x11, 0x22, 0x33}}, false}, {"fail uri", args{ctx, apiv1.Options{URI: "badschema:"}}, func() { pivMap = sync.Map{} pivCards = okPivCards @@ -1392,6 +1400,140 @@ func Test_syncDecrypter_Decrypt(t *testing.T) { assert.Equal(t, data, plain) } +type failingSigner struct { + err error +} + +func (f failingSigner) Public() crypto.PublicKey { return nil } + +func (f failingSigner) Sign(io.Reader, []byte, crypto.SignerOpts) ([]byte, error) { + return nil, f.err +} + +type failingDecrypter struct { + err error +} + +func (f failingDecrypter) Public() crypto.PublicKey { return nil } + +func (f failingDecrypter) Decrypt(io.Reader, []byte, crypto.DecrypterOpts) ([]byte, error) { + return nil, f.err +} + +func Test_wrapPINError(t *testing.T) { + pinErr := fmt.Errorf("verify pin: %w", piv.AuthErr{Retries: 2}) + blockedErr := fmt.Errorf("verify pin: %w", piv.AuthErr{}) + otherErr := errors.New("some error") + + type args struct { + err error + defaultPIN bool + } + tests := []struct { + name string + args args + wantHint bool + wantRetries int + }{ + {"wraps auth errors from the default pin", args{pinErr, true}, true, 2}, + {"wraps blocked pin errors from the default pin", args{blockedErr, true}, true, 0}, + {"skips auth errors from a provided pin", args{pinErr, false}, false, 0}, + {"skips other errors", args{otherErr, true}, false, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := wrapPINError(tt.args.err, tt.args.defaultPIN) + require.Error(t, err) + assert.ErrorIs(t, err, tt.args.err) + if tt.wantHint { + assert.Contains(t, err.Error(), "the YubiKey factory default PIN was tried") + var authErr piv.AuthErr + require.ErrorAs(t, err, &authErr) + assert.Equal(t, tt.wantRetries, authErr.Retries) + } else { + assert.NotContains(t, err.Error(), "default PIN") + } + }) + } + + assert.NoError(t, wrapPINError(nil, true)) + assert.NoError(t, wrapPINError(nil, false)) +} + +func TestYubiKey_CreateSigner_defaultPIN(t *testing.T) { + pinErr := fmt.Errorf("verify pin: %w", piv.AuthErr{Retries: 2}) + + yk := newStubPivKey(t, ECDSA) + yk.signerMap[piv.SlotSignature] = failingSigner{err: pinErr} + + tests := []struct { + name string + defaultPIN bool + wantHint bool + }{ + {"hints when the pin was defaulted", true, true}, + {"does not hint when the pin was provided", false, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k := &YubiKey{yk: yk, pin: "123456", defaultPIN: tt.defaultPIN, managementKey: piv.DefaultManagementKey} + signer, err := k.CreateSigner(&apiv1.CreateSignerRequest{ + SigningKey: "yubikey:slot-id=9c", + }) + require.NoError(t, err) + + _, err = signer.Sign(rand.Reader, []byte("digest"), crypto.SHA256) + require.Error(t, err) + assert.ErrorIs(t, err, pinErr) + var authErr piv.AuthErr + require.ErrorAs(t, err, &authErr) + assert.Equal(t, 2, authErr.Retries) + if tt.wantHint { + assert.Contains(t, err.Error(), "the YubiKey factory default PIN was tried") + } else { + assert.NotContains(t, err.Error(), "default PIN") + } + }) + } +} + +func TestYubiKey_CreateDecrypter_defaultPIN(t *testing.T) { + pinErr := fmt.Errorf("verify pin: %w", piv.AuthErr{Retries: 2}) + + yk := newStubPivKey(t, RSA) + yk.signerMap[piv.SlotSignature] = failingDecrypter{err: pinErr} + + tests := []struct { + name string + defaultPIN bool + wantHint bool + }{ + {"hints when the pin was defaulted", true, true}, + {"does not hint when the pin was provided", false, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k := &YubiKey{yk: yk, pin: "123456", defaultPIN: tt.defaultPIN, managementKey: piv.DefaultManagementKey} + decrypter, err := k.CreateDecrypter(&apiv1.CreateDecrypterRequest{ + DecryptionKey: "yubikey:slot-id=9c", + }) + require.NoError(t, err) + + _, err = decrypter.Decrypt(rand.Reader, []byte("ciphertext"), nil) + require.Error(t, err) + assert.ErrorIs(t, err, pinErr) + var authErr piv.AuthErr + require.ErrorAs(t, err, &authErr) + assert.Equal(t, 2, authErr.Retries) + if tt.wantHint { + assert.Contains(t, err.Error(), "the YubiKey factory default PIN was tried") + } else { + assert.NotContains(t, err.Error(), "default PIN") + } + }) + } +} + func TestYubicoNewRoots(t *testing.T) { const rootPEM = `-----BEGIN CERTIFICATE----- MIIDPjCCAiagAwIBAgIUXzeiEDJEOTt14F5n0o6Zf/bBwiUwDQYJKoZIhvcNAQEN