Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

main.c: remove output when RET_NO_PIN_ATTEMPTS (no PIN retry counter) is true #32

Closed
wants to merge 1 commit into from

Conversation

tlaurion
Copy link
Contributor

@tlaurion tlaurion commented Apr 17, 2024

The dongle is in a clean state here without bad PIN entered (when RET_NO_PIN_ATTEMPTS is true).
This doesn't imply that we are using the default PINS.


This seems to have been based on wrong assumption that if no prior PIN attempt we are in factory state with default PINS (USER 123456, ADMIN 12345678). Calling code should be, and is responsible of interpreting artifacts telling that the USB Security dongle is not in factory reset mode with default PINS.

Fixes #30


History:

Prior of linuxboot/heads@99673d3, Heads was doing the same wrong assumption. Heads was consuming 1/3 of the PIN to check if it was the default one without, resulting with the user only having 2/3 PIN input attempts before being locked out.

Because of Nitrokey/nitrokey-pro-firmware#54 (unfixed, linking to unfixed Nitrokey/libnitrokey#137), if Heads attempts to use scdaemon/libnitrokey, the dongle hangs. Let if be libnitrokey/gpg expecting exclusive dongle access, this cause hangs.

Therefore linuxboot/heads@99673d3 bases its assumptions on Heads previously created gpg keyring without relaying on neither scdaemon/libnitrokey. It uses public key creation vs current timsetamp to determine if the user should be reminded that is using default PINs, they should be changed.

TODO:

Otherwise, if on any situation, libnitrokey/scdaemon operations are intertwined, this causes Nitrokey/heads#48, which i'll reopen.

Any Heads developer will come to the same problems:

  • Develop on host. Push signed commits with said dongle, which use scdaemon on host to interact with USB Security dongle to do signing ops.
  • Call make BOARD=qemu-coreboot-fbwhiptail-tpm2-hotp USB_TOKEN=NitrokeyPro/NitrokeyStorage/Nitrokey3NFC/LibremKey to test Heads.
  • Land under kvm/qemu, observe reported locked problems, blame QubesOS, blame Heads, blame gnupg.
  • Truth is that its libnitrokey/firmware bug.

hotp-verification should only report on : firmware version(currently wrong), serial number and success/fail state and not do any assumption reporting false information, confusing the end user.

… is true: the dongle is in a clean state here without bad PIN entered.

This doesn't imply that we are using the default PINS.

This seems to have been based on wrong assumption that if no prior PIN attempt we are in factory state with default PINS (USER 123456, ADMIN 12345678).
Calling code should be, and is responsible of interpreting artifacts telling that the USB Security dongle is not in factory reset mode with default PINS.

---

History:

Prior of linuxboot/heads@99673d3, Heads was doing the same wrong assumption.
Heads was consuming 1/3 of the PIN to check if it was the default one without, resulting with the user only having 2/3 PIN input attempts before being locked out.

Because of Nitrokey/nitrokey-pro-firmware#54 (unfixed, linking to unfixed Nitrokey/libnitrokey#137), if Heads attempts to use scdaemon/libnitrokey, the dongle hangs.
Let if be libnitrokey/gpg expecting exclusive dongle access, this cause hangs.

Therefore linuxboot/heads@99673d3 bases its assumptions on Heads previously created gpg keyring without relaying on neither scdaemon/libnitrokey.
It uses public key creation vs current timsetamp to determine if the user should be reminded that is using default PINs, they should be changed.

TODO:
- Fix Nitrokey/nitrokey-pro-firmware#54
- Fix Nitrokey/libnitrokey#137

Otherwise, if on any situation, libnitrokey/scdaemon operations are intertwined, this causes Nitrokey/heads#48, which i'll reopen.

Any Heads developer will come to the same problems:
- Develop on host. Push signed commits with said dongle, which use scdaemon on host to interact with USB Security dongle to do signing ops.
- Call make BOARD=qemu-coreboot-fbwhiptail-tpm2-hotp USB_TOKEN=NitrokeyPro/NitrokeyStorage/Nitrokey3NFC/LibremKey to test Heads.
- Land under kvm/qemu, observe reported locked problems, blame QubesOS, blame Heads, blame gnupg.
- Truth is that its libnitrokey/firmware bug.

----

hotp-verification should only report on : firmware version(currently wrong), serial number and success/fail state and not do any assumption reporting false information, confusing the end user.

Signed-off-by: Thierry Laurion <[email protected]>
@tlaurion
Copy link
Contributor Author

#30
image.jpg

Here:
PXL_20240417_162419594

This commit is included under linuxboot/heads#1640

@tlaurion tlaurion changed the title main.c: remove output when RET_NO_PIN_ATTEMPTS (no PIN retry counter) is true: the dongle is in a clean state here without bad PIN entered. main.c: remove output when RET_NO_PIN_ATTEMPTS (no PIN retry counter) is true Apr 17, 2024
@tlaurion
Copy link
Contributor Author

Well, with this in (or not, will have to retest linuxboot/heads#1640 without this commit), it seems that default Admin PIN is never used anymore (was it?) even on a freshly factory reset USB Security dongle with the defaults (so User PIN 123456 and Admin 12345678)

PXL_20240417_210745497
PXL_20240417_210633717

@tlaurion
Copy link
Contributor Author

tlaurion commented Apr 17, 2024

Nah.

This PR doesn't fix it, it just hides the fact that internal structures of hotp-verification are completely broken.

I reverted my fix and tested https://github.com/tlaurion/heads/compare/nitrokey_board_unification_clean-enable_htop_validated_autoboot...tlaurion:heads:nitrokey_board_unification_clean-enable_htop_validated_autoboot-without_hotp_verification_change?expand=1

Master is too bogus to attempt with a quick fix like that.

PXL_20240417_233201434

  • This is true when doing factory reset with defaults, while untrue with my long term key. Hiding it, as I first intended with quick fix, is not a solution. This output should give more information then this, and I won't bissect it on free community time. Ok, fine, so we have factory reset PINs, let's say, it should work.

PXL_20240417_233131895
But it doesn't. We have all 3 attemptss of Admin PINs, we just factory reset to 12345678.

Logic is at https://github.com/linuxboot/heads/blob/82179e4e9894c5cb503f85522ff57088bdd444dc/initrd/bin/seal-hotpkey#L94-L104. Do factory reset. Do hotp_verification info. See wrong ouput expected to be there to fill counters since we couldn't use gpg --card-status because possible locks on older NK1 Pro/NK2 Pro/NK2 storage and now NK3 with implemented workaround.

@sosthene-nitrokey @szszszsz Please test hotp-verification.

  • firmware version is wrong because internal structure parsing is wrong.
  • internal structures verified for number of PIN attempts are wrong.

I can only guess a lot of other things are wrong here.

@tlaurion tlaurion closed this Apr 18, 2024
tlaurion added a commit to tlaurion/heads that referenced this pull request Apr 18, 2024
…ification@8a1f125"

This reverts commit 3a3916b.

This was Nitrokey/nitrokey-hotp-verification#32 and requires hotp-verification deeper cleanup which won't happen here.
tlaurion added a commit to tlaurion/heads that referenced this pull request Apr 18, 2024
…ification@8a1f125"

This reverts commit 3a3916b.

This was Nitrokey/nitrokey-hotp-verification#32 and requires hotp-verification deeper cleanup which won't happen here.

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Apr 18, 2024
…ification@8a1f125"

This reverts commit 3a3916b.

This was Nitrokey/nitrokey-hotp-verification#32 and requires hotp-verification deeper cleanup which won't happen here.

Signed-off-by: Thierry Laurion <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant