search

go-piv / piv-go

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

Reordering the TouchPolicy in #74 makes the strictness order wrong #102

Closed grahamc closed 2 years ago

grahamc commented 2 years ago

TouchPolicyAlways is "stricter" than TouchPolicyCached, however it is "less than" TouchPolicyCached in the code due to #74. This makes it hard to verify a minimum strictness. Compare to PINPolicy:


if cond.PINPolicy > attestation.PINPolicy {
    return fmt.Errorf("Key's PINPolicy %v is less strict than the minimum %v", attestation.PINPolicy, cond.PINPolicy)
}

Because TouchPolicy isn't ordered by strictness, checking it is much harder. It seems a bit silly to make the (once written) parsing code simpler at the expense of use of the values. What do you think?

ericchiang commented 2 years ago

Per https://github.com/go-piv/piv-go/issues/73#issuecomment-647881049 I never intended the actual value to be meaningful. If you want to mandate specific touch policies, compare the attestation's policy against your allow list.

I don't think it's worth changing now. If there was ever any significance outside of the spec values, it wasn't on purpose.

grahamc commented 2 years ago

Hmm... I didn't read that to mean they weren't intended to be meaningful at all, just that the order wasn't intended to map the byte value. Thanks, though, I understand.