go-piv / piv-go

Keys and certificates for YubiKeys, written in Go
Apache License 2.0
366 stars 65 forks source link

incorrect TouchPolicy mapping in the attestation code #73

Closed joemiller closed 4 years ago

joemiller commented 4 years ago

The PIV spec says this about the TouchPolicy stored as byte 2 in the KeyPolicy OID:

1.3.6.1.4.1.41482.3.8: Two bytes, the first encoding pin policy and the second touch policy

Pin policy: 01 - never, 02 - once per session, 03 - always

Touch policy: 01 - never, 02 - always, 03 - cached for 15s

in key.go the const's are out of order:

// Touch policies supported by this package.
const (
    TouchPolicyNever TouchPolicy = iota + 1
    TouchPolicyCached
    TouchPolicyAlways
)

👆 Cached (2) and Always (3) are reversed. Should be Always=2 and Cached=3

The other spots where touchpolicy parsing/mapping is implemented have the correct values:

var touchPolicyMap = map[TouchPolicy]byte{
    TouchPolicyNever:  0x01,
    TouchPolicyAlways: 0x02,
    TouchPolicyCached: 0x03,
}
        case 0x01:
            a.TouchPolicy = TouchPolicyNever
        case 0x02:
            a.TouchPolicy = TouchPolicyAlways
        case 0x03:
            a.TouchPolicy = TouchPolicyCached
ericchiang commented 4 years ago

Thanks for pointing that out :) I actually never intended those to map to their spec values. They're just enum-ish fields that get mapped to byte values on the internals (that's why they're an int instead of a byte, for example). It's just coincidence that they're close to their real values and that PINPolicy matches.

Feel free to send a PR to switch the order if you'd like.

joemiller commented 4 years ago

@ericchiang ok, I may do so when I find a few moments. I noticed this while parsing attestation certs and observing the incorrect value in the parsed Attestation TouchPolicy field