go-piv / piv-go

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

Add ability to change allowed number of PIN and PUK retries #155

Closed Quantu closed 2 months ago

Quantu commented 4 months ago

Add the ability to change PIN/PUK retries using SetRetries. Changing the retries does have the side effect on yubikeys of resetting the PIN and PUK to their default values, so I made sure that is very clear in the comments and README.

Quantu commented 4 months ago

I considered having SetRetries then call ykChangePIN and/or ykChangePUK after calling ykSetRetries if relevant (if PIN or PUK differ from piv.DefaultPIN or piv.DefaultPUK), but that seems a bit too "magic". It also requires you pass the PUK in to a function that doesn't need it for any sort of authentication, which is a little messy. It would just set the PUK to the one provided without validating it is the current PUK.

Leaving the behavior of changing retries resetting the PIN and PUK is in line with Yubico's documentation, as well as the behavior you get when doing the same operation with yubico-piv-tool.

Quantu commented 3 months ago

@ericchiang I know you've been really busy, but if you get a chance to look at this at some point that'd be greatly appreciated. I've been using this already, but it would be really nice to no longer have to use my fork.

Quantu commented 2 months ago

@ericchiang please take a look again, it should be good now.

ericchiang commented 2 months ago

Thanks! I unfortunately am seeing "This branch cannot be rebased due to too many changes" on my end. Does this have any conflicts with the v2 branch?

Quantu commented 2 months ago

Yeah, I had this sitting around before the v2 release and I missed the commit updating the readme. I rebased against v2 and it should be good now.

ericchiang commented 2 months ago

Thanks for your patience here!