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 cli option to request secret app admin/user pin counts #39

Open
2 tasks
nestire opened this issue Nov 13, 2024 · 12 comments
Open
2 tasks

Add cli option to request secret app admin/user pin counts #39

nestire opened this issue Nov 13, 2024 · 12 comments
Labels
enhancement New feature or request

Comments

@nestire
Copy link

nestire commented Nov 13, 2024

see also issue #36

to prevent complicated parsing of the info output which is not stable create a option that allows requesting the pin counters direct like

  • get_adminpin_counter
  • get_userpin_counter

which then can be used directly in heads, this should be backward compatible with older nitrokeys

@nestire nestire added the enhancement New feature or request label Nov 13, 2024
@daringer
Copy link
Collaborator

daringer commented Nov 14, 2024

sounds good, means:

  • "admin pin counter" and "user pin counter" will always be identical for the nk3 as this actually is the "secrets pin counter"
  • a new commandline argument to this output has to be added pin-info
  • the output should be:
Admin Pin Counter: X
User Pin Counter: Y

@sosthene-nitrokey
Copy link
Contributor

For older hardware, If I follow correctly this should return the GPG pin retry counter?

@nestire
Copy link
Author

nestire commented Nov 18, 2024

correct as it does in the info output this way we could simply replace the parsing of the info output with that command. To minimize parsing in heads i would like to just get the number as output, because of this I prefer 2 cli options.
That for NK3 this is the same is fine for backwards compatibility , since the UX will only show the "info" output #38 anyways.

@tlaurion
Copy link
Contributor

@JonathonHall-Purism fyi

@sosthene-nitrokey
Copy link
Contributor

For the NK3, it is possible that the PIN is not set. The output format will follow:

If the pins are set:

Admin PIN Counter: 3
User PIN Counter: 3

If the pins are not set:

Admin PIN Counter: Not set
User PIN Counter: Not set

@tlaurion

This comment was marked as off-topic.

@sosthene-nitrokey

This comment was marked as off-topic.

@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)

@daringer
Copy link
Collaborator

daringer commented Dec 3, 2024

but isn't this the info output you are showing an example for?

  1. do you want this there (info) or here pin-info
  2. or don't you want two different commands for that ?
  3. this issue is about pin-info not info those are distinct arguments for the tool
  4. if you only want Add support for nitrokey 3 distinction between the secrets app firmware and the device firmware versions #43 without adding pin-info we don't need this issue (Add cli option to request secret app admin/user pin counts #39), right ?

@tlaurion
Copy link
Contributor

tlaurion commented Dec 3, 2024

Heads, users, scripts etc simply intelligible output without false info. There is no need to add User/Admin PIN if those are not GPG related for nk3, pin-info is not needed if reported by info, correctly, as above.

  • no ADMIN/User PIN for nk3 secret app PIN, only secret app PIN exists, only secret App PIN should exist/be presented for nk3 HOTP, where GPG PINs need to be reported if possible (it was agreed this was possible) so that Heads can help the user understand what is going on if HOTP sealing doesn't work.
  • As said in seperate issue/PR, secret app pin should always be set, and then not requiring a change pin, simply because there is no admin PIN to unlock secret app pin if all 6 counters have been used: a rest is needed.
    • if secret app PIN is reset with a PIN by default, then physical presence still required twice as of today to set secret app pin and then to change PIN; therefore because of physical presence, it is advise to set a custom PIN in reownership/factory reset so that we don't trigger physical presence twice in the goal of streamlining transfer of ownership.

To this issue:

  • there is no need for pin-info, the two functions presented here would be useful internally to provide real and useful info, like GPG PINs for info output to provide real state of the security dongles GPG counter info. That is, no secure app PIN for <nk3, secure app PIN for nk3 and GPG user PIN, GPG Admin PIN, useful firmware version and if Nitrokey finds it useful, secret app version.

I keep providing the same output for consistency, Heads uses hotp_verification info everywhere, doesn't know about pin-info and doesn't need this separately. What we need is true, actionable info for operations and users understanding of what is going on in case of admin PIN failing for <nk3, and secure app PIN for nk3, right? We need the user, Heads to do the right thing in case HOTP sealing doesn't work as intended. For <nk3, this is through gpg admin PIN, for nk3 this is with secure app PIN. The rest will be for documentation purposes, I keep not understanding how secure app user PIN and secure APP admin PIN would be documented as of today: "they are the same" ? Then don't show duplicate and fix hotp_verification info to provide "GPG Admin PIN", "GPG User PIN" and "Secure APP PIN" only where/if relevant and true, don't output anything that doesn't exist, don't add new arguments for hotp_verification; just deal with it internally to provided needed output to be actionnable upon by users, heads, and docs.

@tlaurion
Copy link
Contributor

tlaurion commented Dec 3, 2024

but isn't this the info output you are showing an example for?

  1. do you want this there (info) or here pin-info

info is all that is needed if provided with GPG Admin/GPG User/Secure App PIN retry counts, real high level firmware version and nk3 secure element version if useful for something for nitrokey (who else needs secure app version and what is this useful to know, use?

  1. or don't you want two different commands for that ?
    I want the counts for GPG Admin PIN, GPG User PIN and Secure App PIN to be real across all variations of nitrokey/librem key existing without regression of UX
  1. this issue is about pin-info not info those are distinct arguments for the tool

Functions to get GPG User/GPG Admin PIN Secure App PIN retry counts are missing to give real, consitent information. Here was about providing admin/user counts, which if we talk about secure app, don't exist, there is only one Secure App PIN for NK3, and GPG Admin PIN used for <nk3. The info needs to account for that and show only real, existing and actionnable informmation.

  1. if you only want Add support for nitrokey 3 distinction between the secrets app firmware and the device firmware versions #43 without adding pin-info we don't need this issue (Add cli option to request secret app admin/user pin counts #39), right ?

If we finally have clarity on what is useful, then we agree that reset SECRET_APP_PIN should set 12345678 if not set, permit SECRET_APP_PIN to be set only once and we can revisit once physical presence removed optionally in firmware next version, unrelated here. info is all that is needed if providing real, actionable and useful information.

@sosthene-nitrokey
Copy link
Contributor

I think I understand. I have all the devices I need to debug and implement the complete info command that includes the device information and the distinct PINs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants