Skip to content

Commit

Permalink
piv: let user specify pin policy instead of using attestation cert
Browse files Browse the repository at this point in the history
I'm still seeing issues with my older YubiKey, but it looks like it's
not respecting the pin policy on the generated key. For example,
settings PINPolicyNone results in a "security condition not satisfied"
error.
  • Loading branch information
ericchiang committed May 16, 2020
1 parent d5ec95e commit f08f572
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 8 deletions.
22 changes: 16 additions & 6 deletions piv/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,11 @@ type Key struct {
// Algorithm to use when generating the key.
Algorithm Algorithm
// PINPolicy for the key.
//
// BUG(ericchiang): some older YubiKeys (third generation) will silently
// drop this value. If PINPolicyNever or PINPolicyOnce is supplied but the
// key still requires a PIN every time, you may be using a buggy key and
// should supply PINPolicyAlways. See https://github.com/go-piv/piv-go/issues/60
PINPolicy PINPolicy
// TouchPolicy for the key.
TouchPolicy TouchPolicy
Expand Down Expand Up @@ -536,12 +541,13 @@ type KeyAuth struct {
// 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
// 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)

// PINPolicy can used to specify the PIN caching strategy for the slot. If not
// provided, this will be inferred from the attestation certificate.
//
// This field is only required on older (<4.3.0) YubiKeys.
PINPolicy PINPolicy
}

func isAuthErr(err error) bool {
Expand Down Expand Up @@ -624,7 +630,11 @@ func pinPolicy(yk *YubiKey, slot Slot) (PINPolicy, error) {
//
func (yk *YubiKey) PrivateKey(slot Slot, public crypto.PublicKey, auth KeyAuth) (crypto.PrivateKey, error) {
pp := PINPolicyNever
if auth.PIN != "" || auth.PINPrompt != nil {
if _, ok := pinPolicyMap[auth.PINPolicy]; ok {
// If the PIN policy is manually specified, trust that value instead of
// trying to use the attestation certificate.
pp = auth.PINPolicy
} else 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.
policy, err := pinPolicy(yk, slot)
Expand Down
7 changes: 5 additions & 2 deletions piv/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ 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 All @@ -111,6 +109,11 @@ func TestPINPrompt(t *testing.T) {
return DefaultPIN, nil
},
}

if !supportsAttestation(yk) {
auth.PINPolicy = test.policy
}

priv, err := yk.PrivateKey(SlotAuthentication, pub, auth)
if err != nil {
t.Fatalf("building private key: %v", err)
Expand Down

0 comments on commit f08f572

Please sign in to comment.