Nitrokey 3 support

[daringer]

• Bumping version of hotp-verification to 1.4
• Add support under oem-factory-reset to provision subkeys with p256
• move enable-usb earlier in oem-factory-reset
• Add USB Security dongle detction by USB ID
• If NK3, use p256 algo in oem-factory-reset

---

Originally first introduced in: #1417

[daringer]

The goal should be to have a runtime mechanism for switching to another algorithm during oem-factory-reset so that the Nitrokey 3 can be used with any HEADS board and there is no need for a compile-time configuration.
I would propose to change the current implementation as followed:

• remove the configuration variable CONFIG_GPG_DEFAULT_ALGO we keep it based on the discussions below
• decide on the "right" algorithm to be used during runtime (right before the key generation)
  • based on vid:pid the decision shall be taken
  • p256 will be used for the time being to properly support the NK3 for all boards
  • once the full secure-element-crypto-backend is available we have to define how we proceed (there will still be the option on the Nitrokey 3 to use software-only / reviewable crypto primitives)

@tlaurion is this ok for you ?

[daringer]

Further I like to address this comment: #1417 (comment) (@saper)

Generally, I would also prefer ed25519, but for now this is obviously not a real option, the main issue which is addressed is this one:

Most importantly, this would mean we would use ECDSA. I am not sure how reliable is random number generation under heads, it might be hard to guarantee the uniqueness of the k semi-private key. libgcrypt seems to have a "rfc6979" flag but I have no idea whether GnuPG uses it/can use it and how.

From what I understand, this does not apply here. We generate the key on the Nitrokey 3, thus also signing is done on the Nitrokey 3 through GnuGPG, right? Then the host-rng is not used, but the Nitrokey 3 rng - means this indeed is a valid point for host-only gpg keys and signing, but not for this situation using a token like the Nitrokey 3.

[tlaurion]

The goal should be to have a runtime mechanism for switching to another algorithm during oem-factory-reset so that the Nitrokey 3 can be used with any HEADS board and there is no need for a compile-time configuration. I would propose to change the current implementation as followed:

* remove the configuration variable `CONFIG_GPG_DEFAULT_ALGO`

* decide on the "right" algorithm to be used during runtime (right before the key generation)
  
  * based on `vid:pid` the decision shall be taken
  * `p256` will be used for the time being to properly support the NK3 for all boards
  * once the full secure-element-crypto-backend is available we have to define how we proceed (there will still be the option on the Nitrokey 3 to use software-only / reviewable crypto primitives)

@tlaurion is this ok for you ?

This seems to be the way, yes. Some notes from past discussions and current Purism implementation that was upstreamed from Pureboot #1419 : The re-ownership now proposes to the user to use defaults and then goes into prompts for advanced choices. I think #1417 was not "wrong" and having a default algo either, but we need to have Heads pick the right one if some USB Security dongles cannot deal with the default.

Past discussions made us decide for RSA3076 since RSA4096 was taking 5 minutes for subkeys for a total of 15 minutes of key gen and was considered unacceptable. RSA3076 was chosen for tradeoff as default to keygen in USB Security dongle in 3 minutes.

Since NK3 cannot do RSA3076 now, I think the code to select default should warn user of the limitation as of now, and welcome the user to track documented issue (temporary measure) and then select the proper algo and continue its business with

decide on the "right" algorithm to be used during runtime (right before the key generation) based on vid:pid the decision shall be taken

@daringer : We have a plan?

[tlaurion]

Generally, I would also prefer ed25519, but for now this is obviously not a real option, the main issue which is addressed is this one:
Most importantly, this would mean we would use ECDSA. I am not sure how reliable is random number generation under heads, it might be hard to guarantee the uniqueness of the k semi-private key. libgcrypt seems to have a "rfc6979" flag but I have no idea whether GnuPG uses it/can use it and how.

@saper @daringer I have experimented on that matter under #1476, which discussion should continue there. Some get home messages at time of writing these lines:

• This subjects is always evolving. Current HW CPU (RDRAND) was questioned a lot in the past and now the user needs to decide for itself what to trust.
  • CPU RDRAND is the fastests entropy feeder. Sandy/Ivy bridge being the first adding this CPU extension, it can feed at a rate of 400Mb+/s
  • TPM can be used as entropy source (with additional tooling), but generate entropy at a rate of 20bytes/second
  • Jitter can be used as a source of entropy, but rate depends of activity happening on the computer, which is basically none in our case.
• As of in master 45a4f9d tpm and shred operations in code are using /dev/urandom, which is tapping into unready crng (spurs errors in dmesg)
• The kernel doesn't aggregate multiple entropy sources as of now, deferring the task to what would be iteractive changes there, including rng-tools, which depends on libcap-ng (stupid if you ask me in our use case, we do not need to have the deamon run as another user....) to aggregate tpm. jitter and cpu entropy
• By enabling and trusting CPU RDRAND, AMD/Intel/Vortio(qemu) have a ready cnrg in early init, removing errors from code tapping into /dev/urandom (unblocking), while we could tap into /dev/random (blocking) with CRNG not ready and entropy used early at boot #1476
• I checked what libgcrypt offers, and as of now, it is built without jitter backend.
  • by enabling the jitter backend, gnupg operations would use the kernel pool, but would mix it with jitter when generating in-memory keys instead of the USB Security dongle.

So TLDR:

• Trusting CPU HW RDRAND is the best option we have now to have early crng ready (calls to tpm and shred of secrets will happen with proper entropy, not being the case now)
• Generating to be inserted in LUKS slot Disk Unlock Key will take randomness in a better pool already (128bytes)
• When generating gnupg keys and subkeys in memory, reconfigured libgcrypt to include jitter will not trust RDRAND totally, but will mix jitterness in its keygen function.

@saper @daringer : opinions on that? I think the above will is going to be the state of #1478 prior of merging.

[tlaurion]

@daringer my hunch from the root of the problem in code is that RSA3096 is enforced as default without forcing usb stack enablement (. /etc/functions enable_usb) to make sure product supports what we otherwise would force to be supported.

I think its wrong for many reasons, most of which not supported right now just like gnuk and everything else https://gitlab.com/openpgp-card/virtual-cards which would be nice eventually to be supported since OpenGPG smartcards afterall. For example, gnuk only support RSA2048, which would also fail oem-factory-reset as of now.

But to stay on point with current issue, I think the solution is:

• Keep the defaults, including override in board config of nv41/ns50 for EC p256, nothing wrong with that per se.
• Have oem-factory-reset call enable_usb earlier, and check for needed overrides for specific products. As of now, NK3 needing p256, but could later on be adapted or further overrides (being strongest algo supported by smartcard not taking more then 3 minutes to generate subkeys just like now.
• be happy!

@daringer @JonathonHall-Purism ? thoughts?

[JonathonHall-Purism]

But to stay on point with current issue, I think the solution is:

* Keep the defaults, including override in board config of nv41/ns50 for EC p256, nothing wrong with that per se.

* Have oem-factory-reset call enable_usb earlier, and check for needed overrides for specific products. As of now, NK3 needing p256, but could later on be adapted or further overrides (being strongest algo supported by smartcard not taking more then 3 minutes to generate subkeys just like now.

* be happy!

I think this is a good plan @tlaurion . I don't see a downside, we can enable more hardware to work without user intervention. I don't see any problem calling enable_usb earlier in oem-factory-reset.

[daringer]

pushed an initial draft:

• enable_usb is called quite early now
• GPG_ALGO now defines what is used for gpg key generation
• CONFIG_GPG_DEFAULT_ALGO is set as initial preference
• which is then overwritten by the usb-token capabilities based on vid:pid

canceled the circle-ci build and did the change on top of the current implementation, once we define that this is the correct solution I can squash and run CircleCI builds.

Also not extensively tested yet, for us it would reduce efforts for testing once #1485 is merged...

[tlaurion]

Since NK3 cannot do RSA3076 now, I think the code to select default should warn user of the limitation as of now, and welcome the user to track documented issue (temporary measure) and then select the proper algo and continue its business with

I generally like what I see under 5435131

Some quick comments as shared through direct comunication channels

• TRACE "oem-factory-reset:usb_security_token_capabilities_check" where the override should take place
• DEBUG "NK3 detected: overriding $DEFAULT_ALGO with $ALGO for the time being, follow xyz development"

Would be ideal. We do not want oem-factory-reset to become spaghetti code more then it is right now, calling usb_security_token_capabilities_check or whatever you see fit should be clear to anyone running Heads in debug to understand what is happening.

The reasoning behind this is that if clean enough, gnuk keys, hsm keys or whatever token/usb security dongle used will be easy to support.

[tlaurion]

I would also love to see NK3 usb security dongles supported under qemu boards for more general testing (which is what I will use to test this PR) under

# To forward a USB token, set USB_TOKEN to one of the following:
# - NitrokeyPro - forwards a Nitrokey Pro by VID:PID
# - LibremKey - forwards a Librem Key by VID:PID
# - <other> - Provide the QEMU usb-host parameters, such as
#   'hostbus=<#>,hostport=<#>' or 'vendorid=<#>,productid=<#>'
ifeq "$(USB_TOKEN)" "NitrokeyPro"
QEMU_USB_TOKEN_DEV := -device usb-host,vendorid=8352,productid=16648
else ifeq "$(USB_TOKEN)" "NitrokeyStorage"
QEMU_USB_TOKEN_DEV := -device usb-host,vendorid=8352,productid=16649
else ifeq "$(USB_TOKEN)" "Nitrokey3NFC"
QEMU_USB_TOKEN_DEV := -device usb-host,vendorid=8352,productid=17074
else ifeq "$(USB_TOKEN)" "LibremKey"
QEMU_USB_TOKEN_DEV := -device usb-host,vendorid=12653,productid=19531
else ifneq "$(USB_TOKEN)" ""
QEMU_USB_TOKEN_DEV := -device "usb-host,$(USB_TOKEN)"
endif

Which block can be found under all qemu-coreboot-* boards under boards

[daringer]

• moved GPG_ALGO into own function, inside bin/oem-factory-reset. There is not one TRACE call inside any other function, so is this the correct location? or did you mean to move it to etc/functions ? (my feeling is no, as this is clearly only useful in oem-factory-reset)
• inside the qemu-* boards the Nitrokey 3 is already listed: https://github.com/osresearch/heads/blob/45a4f9d0f3edc0e9fbf232402eb9e357f5a5c6e5/boards/qemu-coreboot-whiptail-tpm2/qemu-coreboot-whiptail-tpm2.config#L128-L129 nevertheless the introduction-comment did not reflect this (same for Storage), so I updated this instead

[tlaurion]

@JonathonHall-Purism @daringer: outside of the comments here, I am ok to merge
@JonathonHall-Purism ?

[daringer]

• fixed indetation
• update DEBUG wording - keeping DEBUG instead of echo to keep noise for the user low

CircleCI run not canceled (don't expect any issues here) - once this is done I will squash things down to 2 commits and switch to a "Ready for Review"-PR so it can be merged...

Generally about formatting: Is there any reason HEADS is not using shfmt ?

[tlaurion]

Generally about formatting: Is there any reason HEADS is not using shfmt ?

I have absolutely no problem doing the cleaning iteratively, I started doing in my other PRs recently (files touched were identation unified while I adapted code on those files).

There is an issue opened to do that in the final goal of having shellcheck added in CI as an early failing step prior of building boards. Nobody did it yet. We would need to move all scripts including functions scripts to sh suffixes filenames and adapt codebase to call those new names.

Tldr: no reason to not use shfmt on top of changes as of today. In my opinion that will reduce the review cost of doing everything said earlier to make sure nothing broke.

[daringer]

• properly squashed down
• once b47da0b CircleCI run is done, this can be merged

[tlaurion]

@daringer I took liberty to updated OP content
@JonathonHall-Purism I am ok to merge.

[JonathonHall-Purism]

Thanks @daringer and @tlaurion , yes let's merge 👍