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

Allow factory reset/pin change without user presence #41

Open
nestire opened this issue Nov 13, 2024 · 9 comments
Open

Allow factory reset/pin change without user presence #41

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

Comments

@nestire
Copy link

nestire commented Nov 13, 2024

see also #36

Reason behind this is to allow for a mostly seamless owner change.

This might not be possible with the current firmware of the nk3 and may also need some more thought since enabling this could essentially allow any process able to talk to the nk3 to destroy all saved secrets or reset the pin to a unkown value for the user without user interaction.

So this issue is more for a discussion if this is even possible in a good way.

@nestire nestire added the enhancement New feature or request label Nov 13, 2024
@nestire nestire changed the title Allow factory reset/pin change without user pressents Allow factory reset/pin change without user presence Nov 13, 2024
@tlaurion
Copy link
Contributor

tlaurion commented Nov 13, 2024

@nestire : Glad you opened this issue.

My reasoning is to compare the nk3 with nk2/librem key once more for regression on remote attestation/re-ownership with heads use case in mind.

Before, reverse sealing HOTP into the USB Security Dongle only required a valid GPG Admin PIN, which otherwise was decrementing GPG Admin PIN starting from 3 to 0 where 0 required to reset the dongle (unless a reset code was added to OpenGPG smartcard which oem-factory-reset never implemented because meh, yet another PIN/secret to remember). But now, we have a new PIN to deal with, so we should implement this correctly and if secrets stored in USB Security dongle, then them not being usable by reset woukd be clear sign of tamper evidence, no?

  1. In which way is touch better then authenticating which validates user presence in a better way?
  2. In which way is this any different then gpg being able to reset OpenPGP card from OS/nitropy nk3 secret reset from OS, but with touch ?
  • I get the concern which may have led to enforcing physical presence in nk3 for FIDO use case and for other secrets, now wirh additional PINs for different credentials (complexity++)
    • But I would first need to understanding the reasoning that led to enforcing physical presence+authentication and how this was an improvement over nk2 for remote attestation/re-ownership.
  • if re-ownership sets same PIN for different credentials, then evil-maid can test PIN across different credentials for success and known PIN (8 for secret app Admin PIN, 3 for GPG Admin PIN, lessening security vs nk2 that was 3 for Admin PIN and that's it, 0=lock out).

The main problem I see is a user not using his key for nothing else then HOTP, where resetting the dongle would not be noticed from a oem-reownership (Heads factory reset) where firmware would be tampered with. There is no simple solution to that today, and current phsycial presence requirement doesn't stop anybody havign access to a unattended dongle to be resetted on second computer with nitropy, today.

But if the secrets are wiped on the dongle for 2FA: the user would notice on daily usage, no? Even so if oem-factory-reset'ting the dongle when user attempts to use dongle to authenticate on websites?

TLDR: what is the added security provided by physical presence vs gpg --factory-reset on same dongle, or nitropy nk3 secret reset today. Raise the bar of time needed to accomplish same action?

@daringer
Copy link
Collaborator

we can look into this mid-term, this is a firmware change

@tlaurion
Copy link
Contributor

tlaurion commented Nov 18, 2024

@nestire then another issue needs to be opened seperating need for factory reset of secure element of physical presence.

wnitropy nk3 secret reset equivalent should be implemented into hotp-verification, which would not be bound to physical presence and not bound to new firmware version.

Heads needs to be able to do oem factory reset of htop (seal secret prior of shipping), and re-ownership needs to be able to reset that, just like gpg pins.

For feature freeze: oem will not have a completely unattended experience provisioning randomized secrets because of lack of firmware update, but at least they will be able to seal that secret, and end users be able to reown that part as expected without nk3 being a regression as compared to nk2 for transfer of ownership.

@tlaurion
Copy link
Contributor

For feature freeze: oem will not have a completely unattended experience provisioning randomized secrets because of lack of firmware update, but at least they will be able to seal that secret, and end users be able to reown that part as expected without nk3 being a regression as compared to nk2 for transfer of ownership.

Will not land for linuxboot/heads#1850 (unattended OEM/re-ownership won't be possible until this is fixed) consequtnely won't be part of feature freeze linuxboot/heads#1827.

So linuxboot/heads#1850 will require physical presence (touch).

tlaurion added a commit to tlaurion/heads that referenced this issue 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 issue 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
Copy link
Contributor

tlaurion commented Dec 17, 2024

see also #36

Reason behind this is to allow for a mostly seamless owner change.

This might not be possible with the current firmware of the nk3 and may also need some more thought since enabling this could essentially allow any process able to talk to the nk3 to destroy all saved secrets or reset the pin to a unkown value for the user without user interaction.

So this issue is more for a discussion if this is even possible in a good way.

TLDR @nestire :
Nothing prevents you today to do a gpg factory-reset for GPG Admin/User PIN/wipe OpenPGP smartcard.
So the reasoning here is that what is stored in secret app of secure element were more important than PGP Smartcard? I would love to understand the reasoning first.

FYI: on a remote attestation perspective where usb security dongle should be handle with case and not left unattended, where users sign/encrypt/authenticate with OpenPGP smartcard, I see no reasoning for physical presence requirements pushed for secrets app on secure element, but not on smartcard and the change pushed on ecosystem prior of any impact discussion and causing unknown regression until discovered under Heads.

As said, I can understand the need of this on servers, maybe, where dongle is connected to the motherboard with usb port directly on the motherboard? Was that the reasoning? Again, I still don't understand why wiping OpenPGP would be permitted without physical presence and where factory reset is expected to wipe openpgp private key, preventing to sign and therefore showing tampering evidence (the key was tampered with).

But again, if dongle is inside of server chassis, its physical security. On those mobile dongles, the idea of their physical presence up to now was to plug device at boot for HOTP reverse integrity attestation, to plug device on signature prompts and user left responsible of not leaving the dongle unattended (which means plugged in machine, which as opposed to server, is not physically secured and without cameras looking at it 24/7).

Please explain reasoning and let's go from there @nestire @daringer.


As said everywhere, if there is no payment agreement for my time involvement in Nitrokey fixing of regression/code up to Heads master, expect less fixing/testing. If no payment agreement, I expect shifting here: Nitrokey will push changes under Heads to mitigate regressions introduced and my participation with Nitrokey should be considered as any other user. I will redirect any issues related to Nitrokey to Nitrokey ticketing systems and will remove any accountability related to my responsibilities of making things work as expected. At most, I will open issue under Heads so users know its a know issue, and point to Nitrokey issue and won't be part of the solution. I will solely raise issues. This whole physical presence/touch requirement, absence of setting PIN leaving it to be set on first use, and prior of 1.7.1 firmware, accepting any PIN provided to seal HOTP is a total disaster for everyone. Work related to nk3 17.1 firmware, hotp-verification bump to 1.6 was 40h. This damage control/making nk3 par with nk2 was another 40h. All unpaid still today. I've learned. Your turn to show the same.

@tlaurion
Copy link
Contributor

tlaurion commented Dec 17, 2024

we can look into this mid-term, this is a firmware change

FYI: @jans23 said this firmware change would be part of Heads feature freeze. Maybe increase internal/external communications? Sure fact, don't rely on me doing that part from now on.

How my time, solely fixing Heads/testing your hotp-verification, nk3 firmware changes is calculated on Nitrokey side?
Oh yeah. Not planned/calculated/compensated, right? How convenient. I wish I could offload work to others for free like that. Must be nice! That worked for you long term? How? With who?

Well. PR expected under Heads from this date and forward for accountability reasons. If collaboration from my side is desired/recognized, there will be payment agreement/legally bounding contract with clear separation of duties and responsabilities for quality control/regression prevention.

I consider this message understood by all from now on. If not, it will be your problem, Nitrokey.

@tlaurion
Copy link
Contributor

No more changes under Heads on my side to point with ever moving commits not under 1.7 released version bump.

As stated, this touch thing was under engineered. Waiting for PR under Heads side from Nitrokey side since unpaid, and also removal of physical presence altogether which creates a bunch of unpaid regression validation, changes needed under heads which Nitrokey are not contributing changes for resulting in unpaid work on my side which stops here.

Please remove physical presence requirements from firmware version bump #41

Note that this is not tested under linuxboot/heads#1866

@sosthene-nitrokey
Copy link
Contributor

Please remove physical presence requirements from firmware version bump #41

Physical presence requirements cannot be changed from the nitrokey-hotp-verification side. The device itself request touch confirmation for factory-reset, PIN related operations and some secrets related operations to prevent a compromised host from being able to attack the device without alerting the user. Removing them would require a new firmware release and we do not wish at this time to remove the security benefits of having user presence confirmation for these operations.

As said in the first comment:

This might not be possible with the current firmware of the nk3 and may also need some more thought since enabling this could essentially allow any process able to talk to the nk3 to destroy all saved secrets or reset the pin to a unkown value for the user without user interaction.

So this issue is more for a discussion if this is even possible in a good way.

Right now the answer is: we don't have a good solution, so we don't plan on making re-ownership work without user presence check.

@tlaurion
Copy link
Contributor

tlaurion commented Dec 19, 2024

If you see unexpected behaviour you think is a bug, please include the information on:

  • How to reproduce it
  • Expected output
  • Observed output

Regressions to nk2 is not mine to document nor flag more then it was done. Please use Heads and make it par without causing regressions. Separation of duties need to be clarified and I won't repeat myself anymore. Nitrokey is expected to propose mitigations themselves if they cause regressions for rest of Heads ecosystem.

  • Physical presence was not supposed to be a replacement to authentication which led to hotp-verification 1.6 and nk3 firmware 1.7.1 massive security vuln passed under the rug because lack of CVE attributed leading to mere blog post notice:

With previous firmware versions, re-creating HOTP secrets only required User Presence, but did not verify the user PIN, which was a less strict security policy than intended.

  • PIN being set on first use prior of hotp-verification 1.7 was a regression compared to nk2.

  • Physical presence under nk3 fw 1.7.2 and hotp-verification 1.7 still prevents unattended workflows with no good justification but regression and required mitgation under Heads to un-silence output under seal-hotpkey (which should have been Nitrokey to do PR upstream under Heads by Nitrokey, not me as well as other mitigations, or delay nk3 initial release by proper regression testing)

  • next downstream releases of Heads+dasharo will be able to resolve most personally discovered regressions and provision secrets app PIN alongside all other secrets, because of hotp-verification 1.7 related bugs reports and testing and nitrokey fixes. But next nk3 firmware + left over patches not under versioned hotp-verification should have been under nk3+heads first initial release, not discovered by my time and involvement on which there is no paid compensation agreement, @jan23 considering I should not be paid for my time (because unplanned. As if I planned this!) leading to those regression discoveries, bug report, PR involvement, code fixing under heads and my own PR under heads.... To be used back by nitrokey users and rest of ecosystem.

As a result of unpaid compensation and lack of prior do diligence, I expect Nitrokey to do way more regression testing under Heads qemu+nk3 and arrive to feature parity with <nk3 and resolve any regression that will be discovered by themselves, otherwise by downstream dasharo+heads releases users, and reported by those Heads users from now on to nitrokey directly.

Originally posted by @tlaurion in #52 (comment) (with some clarifications, since this issue is not fixed even though it was supposed to be part of feature freeze per @jans23 words under non-public business related partners business channel, but those discussions were though non-business related. So those issues are now Nitrokey's to fix and propose PR upstream to Heads from now on to be free of charge).

tlaurion added a commit to tlaurion/heads that referenced this issue 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 issue 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]>
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

No branches or pull requests

4 participants