Skip to content

Commit

Permalink
piv: fixes for older YubiKey versions
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ericchiang committed May 12, 2020
1 parent 564f246 commit 2184bb6
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 16 deletions.
49 changes: 39 additions & 10 deletions piv/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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.
Expand All @@ -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) {
Expand Down
33 changes: 29 additions & 4 deletions piv/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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{
Expand All @@ -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})
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 16 additions & 2 deletions piv/piv.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
10 changes: 10 additions & 0 deletions piv/piv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
}
Expand Down

0 comments on commit 2184bb6

Please sign in to comment.