From 2184bb6b48d35daef927bc6e5a8330d3e2b877fd Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Mon, 11 May 2020 19:08:10 -0700 Subject: [PATCH] piv: fixes for older YubiKey versions This change: * Introduces a fallback when creating a PrivateKey if the YubiKey doesn't support attestation certificates. * Fixes tests for older YubiKeys. * Notes a bug in PIN caching for older YubiKeys. Despite the spec[1], older YubiKeys don't let you determine if a PIN is or isn't needed. This makes it impossible for the package to figure out if a PIN is cached or we need to prompt. Add a BUG comment warning against PINPolicyOnce for older YubiKeys. [1] https://csrc.nist.gov/CSRC/media/Publications/sp/800-73/4/archive/2015-05-29/documents/sp800_73-4_pt2_draft.pdf#page=20 --- piv/key.go | 49 +++++++++++++++++++++++++++++++++++++++---------- piv/key_test.go | 33 +++++++++++++++++++++++++++++---- piv/piv.go | 18 ++++++++++++++++-- piv/piv_test.go | 10 ++++++++++ 4 files changed, 94 insertions(+), 16 deletions(-) diff --git a/piv/key.go b/piv/key.go index 05aed0c..f8679b1 100644 --- a/piv/key.go +++ b/piv/key.go @@ -260,6 +260,10 @@ const ( type PINPolicy int // PIN policies supported by this package. +// +// BUG(ericchiang): Caching for PINPolicyOnce isn't supported on YubiKey +// versions older than 4.3.0 due to issues with verifying if a PIN is needed. +// If specified, a PIN will be required for every operation. const ( PINPolicyNever PINPolicy = iota + 1 PINPolicyOnce @@ -311,6 +315,9 @@ func (yk *YubiKey) AttestationCertificate() (*x509.Certificate, error) { // certificate. This can be used to prove a key was generate on a specific // YubiKey. // +// This method is only supported for YubiKey versions >= 4.3.0. +// https://developers.yubico.com/PIV/Introduction/PIV_attestation.html +// // Certificates returned by this method MUST NOT be used for anything other than // attestion or determining the slots public key. For example, the certificate // is NOT suitable for TLS. @@ -525,7 +532,12 @@ type KeyAuth struct { PIN string // PINPrompt can be used to interactively request the PIN from the user. The // method is only called when needed. For example, if a key specifies - // PINPromptOnce, PINPrompt will only be called once per YubiKey struct. + // PINPolicyOnce, PINPrompt will only be called once per YubiKey struct. + // + // BUG(ericchiang): On YubiKey versions older than 4.3.0 PIN caching isn't + // supported and PINPrompt will be called on every signing operation, even if + // PINPolicyOnce is specified. This is due to a bug in the VERIFY card + // command for older YubiKeys. PINPrompt func() (pin string, err error) } @@ -571,6 +583,29 @@ func (k KeyAuth) do(yk *YubiKey, pp PINPolicy, f func(tx *scTx) ([]byte, error)) return f(yk.tx) } +func pinPolicy(yk *YubiKey, slot Slot) (PINPolicy, error) { + cert, err := yk.Attest(slot) + if err != nil { + var e *apduErr + if errors.As(err, &e) && e.sw1 == 0x6d && e.sw2 == 0x00 { + // Attestation cert command not supported, probably an older YubiKey. + // Guess PINPolicyAlways. + // + // See https://github.com/go-piv/piv-go/issues/55 + return PINPolicyAlways, nil + } + return 0, fmt.Errorf("get attestation cert: %v", err) + } + a, err := parseAttestation(cert) + if err != nil { + return 0, fmt.Errorf("parse attestation cert: %v", err) + } + if _, ok := pinPolicyMap[a.PINPolicy]; ok { + return a.PINPolicy, nil + } + return PINPolicyOnce, nil +} + // PrivateKey is used to access signing and decryption options for the key // stored in the slot. The returned key implements crypto.Signer and/or // crypto.Decrypter depending on the key type. @@ -589,17 +624,11 @@ func (yk *YubiKey) PrivateKey(slot Slot, public crypto.PublicKey, auth KeyAuth) if auth.PIN != "" || auth.PINPrompt != nil { // Attempt to determine the key's PIN policy. This helps inform the // strategy for when to prompt for a PIN. - cert, err := yk.Attest(slot) + policy, err := pinPolicy(yk, slot) if err != nil { - return nil, fmt.Errorf("get attestation cert: %v", err) - } - a, err := parseAttestation(cert) - if err != nil { - return nil, fmt.Errorf("parse attestation cert: %v", err) - } - if _, ok := pinPolicyMap[a.PINPolicy]; ok { - pp = a.PINPolicy + return nil, err } + pp = policy } switch pub := public.(type) { diff --git a/piv/key_test.go b/piv/key_test.go index 3aa3240..94e5339 100644 --- a/piv/key_test.go +++ b/piv/key_test.go @@ -35,6 +35,10 @@ func TestYubiKeySignECDSA(t *testing.T) { yk, close := newTestYubiKey(t) defer close() + if err := yk.Reset(); err != nil { + t.Fatalf("reset yubikey: %v", err) + } + slot := SlotAuthentication key := Key{ @@ -89,6 +93,8 @@ func TestPINPrompt(t *testing.T) { yk, close := newTestYubiKey(t) defer close() + testRequiresVersion(t, yk, 4, 3, 0) + k := Key{ Algorithm: AlgorithmEC256, PINPolicy: test.policy, @@ -127,6 +133,11 @@ func TestPINPrompt(t *testing.T) { } } +func supportsAttestation(yk *YubiKey) bool { + v := yk.Version() + return !(v.Major < 4 || v.Minor < 3) +} + func TestSlots(t *testing.T) { yk, close := newTestYubiKey(t) if err := yk.Reset(); err != nil { @@ -149,8 +160,10 @@ func TestSlots(t *testing.T) { yk, close := newTestYubiKey(t) defer close() - if _, err := yk.Attest(test.slot); err == nil || !errors.Is(err, ErrNotFound) { - t.Errorf("attest: got err=%v, want=ErrNotFound", err) + if supportsAttestation(yk) { + if _, err := yk.Attest(test.slot); err == nil || !errors.Is(err, ErrNotFound) { + t.Errorf("attest: got err=%v, want=ErrNotFound", err) + } } k := Key{ @@ -162,8 +175,11 @@ func TestSlots(t *testing.T) { if err != nil { t.Fatalf("generating key on slot: %v", err) } - if _, err := yk.Attest(test.slot); err != nil { - t.Errorf("attest: %v", err) + + if supportsAttestation(yk) { + if _, err := yk.Attest(test.slot); err != nil { + t.Errorf("attest: %v", err) + } } priv, err := yk.PrivateKey(test.slot, pub, KeyAuth{PIN: DefaultPIN}) @@ -320,6 +336,8 @@ func TestYubiKeyAttestation(t *testing.T) { TouchPolicy: TouchPolicyNever, } + testRequiresVersion(t, yk, 4, 3, 0) + cert, err := yk.AttestationCertificate() if err != nil { t.Fatalf("getting attestation certificate: %v", err) @@ -351,6 +369,9 @@ func TestYubiKeyAttestation(t *testing.T) { if a.TouchPolicy != key.TouchPolicy { t.Errorf("attestation touch policy got=0x%x, wanted=0x%x", a.TouchPolicy, key.TouchPolicy) } + if a.Version != yk.Version() { + t.Errorf("attestation version got=%#v, wanted=%#v", a.Version, yk.Version()) + } } func TestYubiKeyStoreCertificate(t *testing.T) { @@ -453,6 +474,10 @@ func TestYubiKeyGenerateKey(t *testing.T) { } yk, close := newTestYubiKey(t) defer close() + if test.alg == AlgorithmEC384 { + testRequiresVersion(t, yk, 4, 3, 0) + } + key := Key{ Algorithm: test.alg, TouchPolicy: TouchPolicyNever, diff --git a/piv/piv.go b/piv/piv.go index 5bb70b8..9f381d3 100644 --- a/piv/piv.go +++ b/piv/piv.go @@ -180,6 +180,19 @@ func (c *client) Open(card string) (*YubiKey, error) { return yk, nil } +// Version returns the version as reported by the PIV applet. For newer +// YubiKeys (>=4.0.0) this corresponds to the version of the YubiKey itself. +// +// Older YubiKeys return values that aren't directly related to the YubiKey +// version. For example, 3rd generation YubiKeys report 1.0.X. +func (yk *YubiKey) Version() Version { + return Version{ + Major: int(yk.version.major), + Minor: int(yk.version.minor), + Patch: int(yk.version.patch), + } +} + // Serial returns the YubiKey's serial number. func (yk *YubiKey) Serial() (uint32, error) { return ykSerial(yk.tx, yk.version) @@ -218,6 +231,7 @@ func ykLogin(tx *scTx, pin string) error { return err } + // https://csrc.nist.gov/CSRC/media/Publications/sp/800-73/4/archive/2015-05-29/documents/sp800_73-4_pt2_draft.pdf#page=20 cmd := apdu{instruction: insVerify, param2: 0x80, data: data} if _, err := tx.Transmit(cmd); err != nil { return fmt.Errorf("verify pin: %w", err) @@ -588,8 +602,8 @@ func ykVersion(tx *scTx) (*version, error) { if err != nil { return nil, fmt.Errorf("command failed: %w", err) } - if n := len(resp); n < 3 { - return nil, fmt.Errorf("response was too short: %d", n) + if n := len(resp); n != 3 { + return nil, fmt.Errorf("expected response to have 3 bytes, got: %d", n) } return &version{resp[0], resp[1], resp[2]}, nil } diff --git a/piv/piv_test.go b/piv/piv_test.go index 06a1452..f37cbb2 100644 --- a/piv/piv_test.go +++ b/piv/piv_test.go @@ -41,6 +41,13 @@ func testGetVersion(t *testing.T, h *scHandle) { } } +func testRequiresVersion(t *testing.T, yk *YubiKey, major, minor, patch int) { + v := yk.Version() + if v.Major < major || v.Minor < minor || v.Patch < patch { + t.Skipf("test requires yubikey version %d.%d.%d: got %d.%d.%d", major, minor, patch, v.Major, v.Minor, v.Patch) + } +} + func TestGetVersion(t *testing.T) { runHandleTest(t, testGetVersion) } func TestCards(t *testing.T) { @@ -130,6 +137,9 @@ func TestYubiKeySerial(t *testing.T) { func TestYubiKeyLoginNeeded(t *testing.T) { yk, close := newTestYubiKey(t) defer close() + + testRequiresVersion(t, yk, 4, 3, 0) + if !ykLoginNeeded(yk.tx) { t.Errorf("expected login needed") }