yubikey-manager icon indicating copy to clipboard operation
yubikey-manager copied to clipboard

ykman openpgp set-pin-retries 3 0 3 isn't allowed

Open roconnor opened this issue 4 years ago • 6 comments

  • YubiKey Manager (ykman) version: 3.1.2
  • How was it installed?: From source
  • Operating system and version: NixOS 21.03pre269343
  • YubiKey model and version: Nano 5.2.6
  • Bug description summary:
$ ykman openpgp set-pin-retries 3 0 3
Usage: ykman openpgp set-pin-retries [OPTIONS] PIN-RETRIES RESET-CODE-RETRIES ADMIN-PIN-RETRIES
Try 'ykman openpgp set-pin-retries -h' for help.

Error: Invalid value for 'RESET-CODE-RETRIES': 0 is not in the valid range of 1 to 99.

Expected result

At least for the RESET-CODE-RETRIES, a value of 0 should be allowed in order to disable it. This is especially true in light of YSA-2020-05.

roconnor avatar Feb 20 '21 14:02 roconnor

Good point, thanks!

dainnilsson avatar Feb 20 '21 15:02 dainnilsson

I think it is worth considering letting the PIN-RETRIES to be 0 as well for cases where the user believe the PIN has been compromised but cannot (yet) be reset. Keeping the ADMIN-PIN-RETRIES with a minimum value of 1 seems reasonable though.

roconnor avatar Feb 20 '21 15:02 roconnor

Unfortunately I've had to revert this change as I've found out that setting any of these to 0 isn't supported by the firmware. Instead, you can block the Reset Code by entering it incorrectly "too many" times if you wish to disable it (or never set it in the first place, since the default after Factory Reset is to not have it set at all).

dainnilsson avatar Feb 22 '21 14:02 dainnilsson

Can we allow a counter to be "set" to 0 if the counter is already at 0? That appears to work for me Otherwise we are in an awkward position where we have to unblock the Reset Code if we want to change the values of the other counters.

roconnor avatar Feb 22 '21 15:02 roconnor

I think you're on to something there. That seems like it should work, both for the case that the Reset Code is set-but-blocked, but also when it isn't set at all (in both cases info will report 0 retries for it). If the counter is > 0 then the command can fail.

dainnilsson avatar Feb 22 '21 15:02 dainnilsson

Ok, I've given this some more thought (and think I've managed to lessen my confusion somewhat). This is as much for my own future reference as anyone else who reads it. My current understanding is this:

We have 3 different codes: "PIN", "Reset Code" and "Admin PIN". Of these, only "Reset Code" can be unset, the others always have a value. On initialization (and factory reset), the Reset Code will not have any value.

For each of these 3 codes, we have 2 counters: current_tries and max_tries. max_retries is 3 by default (after factory reset), and is the value changed by the set-pin-retries command. This value can never be 0. current_tries will start equal to max_tries, and decrement by one each time an incorrect code is entered, until it reaches 0 and is blocked. A correct entry of the code will reset it to be equal to max_tries. If the corresponding code is unset (so Reset Code only) the current_tries will be effectively locked at 0.

The counts we can read out of the YubiKey are the 3 current_tries. Given the defaults above, this will show 3 0 3 after factory reset. The middle 0 due to the Reset Code being unset, however its max_tries is in actuality 3 at this point. If a Reset Code is set, then current_tries is set to max_tries at this point (eg 3).

When setting the max_tries counters using the set-pin-retries command, any 0 will be ignored.

The above has some unintuitive implications. For example, if we start by setting retries with set-pin-retries 5 5 5, the current_tries counters will show 5 0 5. If we then set a Reset Code, it will show 5 5 5.

If instead we issue set-pin-retries 5 0 5 the current_tries counters will still show 5 0 5 but the max_tries counters will be set to 5 3 5 (since the 0 is ignored, the 3 is kept), so when a Reset Code is set, the counters will show 5 3 5.

With this understanding, my conclusion is that:

  • This is all very confusing.
  • Setting a (max_tries) counter to 0 isn't possible and doesn't really make sense (if possible, this would allow us to then set a Reset Code which would immediately be unusable).
  • ykman should offer the ability to manage these PINs and provide the ability to unset the Reset Code.

I think for now we leave functionality as-is (allowing to pass 0 seems more misleading than helpful, since it doesn't actually do anything), with the longer-term goal of having more complete PIN/Reset Code/Admin PIN management in ykman, which can hopefully be made a bit more intuitive.

dainnilsson avatar Feb 23 '21 08:02 dainnilsson