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

Add support for nitrokey 3 distinction between the secrets app firmware and the device firmware versions #43

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

sosthene-nitrokey
Copy link
Contributor

Fix #38

@tlaurion
Copy link
Contributor

@sosthene-nitrokey just want to make sure we are at the same page:

<nk3

HOTP code verification application, version 1.7
Connected device status:
Card serial: 0x7BE66C6C
         Firmware Nitrokey 2: v1.7.1
OpenPGP smartcard PIN counters : Admin: 3, User: 3
Operation success

nk3:

HOTP code verification application, version 1.7
Connected device status:
Card serial: 0x7BE66C6C
         Firmware Nitrokey 3: v1.7.1
         Firmware Secrets app: v4.13
Secret app PIN counter : 6
OpenPGP smartcard PIN counters : Admin: 3, User: 3
Operation success

Originally posted by @tlaurion in #38 (comment)

@nestire
Copy link

nestire commented Nov 27, 2024

Test with nk pr + nk stroage

ls -lh hotp_verification
-rwxr-xr-x 1 user user 59K Nov 27 11:20 hotp_verification
sha256sum hotp_verification
4a6db3c562d75bc637e23394080f3821c47774175780d8f72e45430fb9897181  hotp_verification
[user@work nitrokey-hotp-verification (more-info)]$ lsusb
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 013: ID 20a0:4108 Clay Logic Nitrokey Pro
Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
[user@work nitrokey-hotp-verification (more-info)]$ ./hotp_verification info
HOTP code verification application, version 1.6

Connected device status:
	Card serial: 0xFFFFFFFF
	Firmware Nitrokey 3: v3.14980.0
	Firmware Secrets App: v255.0
	Card counters: Admin 3, User 3
Operation success
[user@work nitrokey-hotp-verification (more-info)]$ lsusb
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 014: ID 20a0:4109 Clay Logic Nitrokey Storage
Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
[user@work nitrokey-hotp-verification (more-info)]$ ./hotp_verification info
HOTP code verification application, version 1.6

Connected device status:
	Card serial: 0xFFFFFFFF
	Firmware Nitrokey 3: v0.0.0
	Firmware Secrets App: v255.0
	Card counters: Admin 3, User 3
Operation success
[user@work nitrokey-hotp-verification (more-info)]$ 

@tlaurion
Copy link
Contributor

tlaurion commented Dec 2, 2024

I repeat

You proposed :

See also issue #36
Change this:


HOTP code verification application, version 1.6
Connected device status:
	Card serial: 0x7BE66C6C
	Firmware: v4.13
	Card counters: Admin 6, User 6
Operation success

To

HOTP code verification application, version 1.6
Connected device status:
	Card serial: 0x7BE66C6C
        Firmware Nitrokey 3: v1.7.1
	Firmware Secrets app: v4.13
	Secret app pin counters : Admin 6, User 6
Operation success

I proposed and restated:

Even more sensical: no secret app even named anywhere because there is none on non nk3(regression), so no version of non existing secret app, no secret app pin, just real information :

HOTP code verification application, version 1.7
Connected device status:
Card serial: 0x7BE66C6C
         Firmware Nitrokey 2: v1.7.1
OpenPGP smartcard PIN counters : Admin: 3, User: 3
Operation success

For nk3:

HOTP code verification application, version 1.7
Connected device status:
Card serial: 0x7BE66C6C
         Firmware Nitrokey 3: v1.7.1
         Firmware Secrets app: v4.13
Secret app PIN counter : 6
OpenPGP smartcard PIN counters : Admin: 3, User: 3
Operation success

Originally posted by @tlaurion in #38 (comment)

This now adds the secrets app version and the nitrokey 3 firmware version,
and also the gpg pins
@sosthene-nitrokey sosthene-nitrokey marked this pull request as ready for review December 5, 2024 15:50
@sosthene-nitrokey
Copy link
Contributor Author

sosthene-nitrokey commented Dec 5, 2024

Updated to give the following with an NK3:

$ hotp_verification info
HOTP code verification application, version 1.6
...
Connected device status:
	Card serial: 0xADF72B1E
	Firmware Nitrokey 3: v1.8.0
	Firmware Secrets App: v4.13
	Secrets app PIN counter: 8
	GPG Card counters: Admin 3, User 3
Operation success

No change with other devices.

tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 5, 2024
TODO: check logic in this file because assumptions on PINs retry count are wrong and will depend on Nitrokey/nitrokey-hotp-verification#43 not tested here

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 6, 2024
…instead of Nitrokey/nitrokey-hotp-verification#46 for hotp-verification info parsing and validation of oem-factory-reset and seal-hotp

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 6, 2024
- oem-factory-reset: fix strings for nk3 is from Nitrokey/nitrokey-hotp-verification#43 is Secrets app, not Secret App singular, not App capitalized
- initrd/bin/seal-hotpkey: adapt to check nk3 Secrets App PIN counter if nk3, keep Card counters for <nk3 from Nitrokey/nitrokey-hotp-verification#43
  - Unattended hotp_initialize output removed since we need physical presence to seal HOTP until Nitrokey/nitrokey-hotp-verification#41 is fixed
  - Finally make seal_hotp use logic to detect if public key <1m old, use HOTP related PIN by default if counter is not <3, warn that re-ownership needs to be ran to change it since no security offered at all otherwise with HOTP

Tested in local tree against https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/43.patch, removing https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/46.patch
 - will revert the change above in PR once testing is over

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 6, 2024
…fef5d1c82a014e0e2bf79346 directory: waiting for Nitrokey/nitrokey-hotp-verification#43 and Nitrokey/nitrokey-hotp-verification#46 to be merged to change modules/hotp-verification commit

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

tlaurion commented Dec 6, 2024

@sosthene-nitrokey Adapted linuxboot/heads#1850 (comment) and tested this successfully.

Please tell me when #43 merged alongside #46

tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 6, 2024
…instead of Nitrokey/nitrokey-hotp-verification#46 for hotp-verification info parsing and validation of oem-factory-reset and seal-hotp

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 6, 2024
- oem-factory-reset: fix strings for nk3 is from Nitrokey/nitrokey-hotp-verification#43 is Secrets app, not Secret App singular, not App capitalized
- initrd/bin/seal-hotpkey: adapt to check nk3 Secrets App PIN counter if nk3, keep Card counters for <nk3 from Nitrokey/nitrokey-hotp-verification#43
  - Unattended hotp_initialize output removed since we need physical presence to seal HOTP until Nitrokey/nitrokey-hotp-verification#41 is fixed
  - Finally make seal_hotp use logic to detect if public key <1m old, use HOTP related PIN by default if counter is not <3, warn that re-ownership needs to be ran to change it since no security offered at all otherwise with HOTP
- unify format with linting tool

Tested in local tree against https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/43.patch, removing https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/46.patch
 - will revert the change above in PR once testing is over

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 6, 2024
…fef5d1c82a014e0e2bf79346 directory: waiting for Nitrokey/nitrokey-hotp-verification#43 and Nitrokey/nitrokey-hotp-verification#46 to be merged to change modules/hotp-verification commit

Signed-off-by: Thierry Laurion <[email protected]>
@sosthene-nitrokey sosthene-nitrokey merged commit 08e654b into master Dec 9, 2024
1 check passed
sosthene-nitrokey added a commit that referenced this pull request Dec 9, 2024
Add support for nitrokey 3 distinction between the secrets app firmware and the device firmware versions
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 21, 2024
TODO: check logic in this file because assumptions on PINs retry count are wrong and will depend on Nitrokey/nitrokey-hotp-verification#43 not tested here

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 21, 2024
…instead of Nitrokey/nitrokey-hotp-verification#46 for hotp-verification info parsing and validation of oem-factory-reset and seal-hotp

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 21, 2024
- oem-factory-reset: fix strings for nk3 is from Nitrokey/nitrokey-hotp-verification#43 is Secrets app, not Secret App singular, not App capitalized
- initrd/bin/seal-hotpkey: adapt to check nk3 Secrets App PIN counter if nk3, keep Card counters for <nk3 from Nitrokey/nitrokey-hotp-verification#43
  - Unattended hotp_initialize output removed since we need physical presence to seal HOTP until Nitrokey/nitrokey-hotp-verification#41 is fixed
  - Finally make seal_hotp use logic to detect if public key <1m old, use HOTP related PIN by default if counter is not <3, warn that re-ownership needs to be ran to change it since no security offered at all otherwise with HOTP
- unify format with linting tool

Tested in local tree against https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/43.patch, removing https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/46.patch
 - will revert the change above in PR once testing is over

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 21, 2024
…fef5d1c82a014e0e2bf79346 directory: waiting for Nitrokey/nitrokey-hotp-verification#43 and Nitrokey/nitrokey-hotp-verification#46 to be merged to change modules/hotp-verification commit

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 21, 2024
TODO: check logic in this file because assumptions on PINs retry count are wrong and will depend on Nitrokey/nitrokey-hotp-verification#43 not tested here

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 21, 2024
…instead of Nitrokey/nitrokey-hotp-verification#46 for hotp-verification info parsing and validation of oem-factory-reset and seal-hotp

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 21, 2024
- oem-factory-reset: fix strings for nk3 is from Nitrokey/nitrokey-hotp-verification#43 is Secrets app, not Secret App singular, not App capitalized
- initrd/bin/seal-hotpkey: adapt to check nk3 Secrets App PIN counter if nk3, keep Card counters for <nk3 from Nitrokey/nitrokey-hotp-verification#43
  - Unattended hotp_initialize output removed since we need physical presence to seal HOTP until Nitrokey/nitrokey-hotp-verification#41 is fixed
  - Finally make seal_hotp use logic to detect if public key <1m old, use HOTP related PIN by default if counter is not <3, warn that re-ownership needs to be ran to change it since no security offered at all otherwise with HOTP
- unify format with linting tool

Tested in local tree against https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/43.patch, removing https://patch-diff.githubusercontent.com/raw/Nitrokey/nitrokey-hotp-verification/pull/46.patch
 - will revert the change above in PR once testing is over

Signed-off-by: Thierry Laurion <[email protected]>
tlaurion added a commit to tlaurion/heads that referenced this pull request Dec 21, 2024
…fef5d1c82a014e0e2bf79346 directory: waiting for Nitrokey/nitrokey-hotp-verification#43 and Nitrokey/nitrokey-hotp-verification#46 to be merged to change modules/hotp-verification commit

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
Development

Successfully merging this pull request may close these issues.

Make Info Output more verbose for nk3
4 participants