Nitrokey / opcard-rs

OpenPGP card implementation
50 stars 1 forks source link

Revisit PIN verification #16

Closed robin-nitrokey closed 1 year ago

robin-nitrokey commented 2 years ago

Currently (#15), we store the unencrypted PIN in the persistent state. This should either be outsourced to the secure element or replaced with a hash (that could be handled by Trussed) and the PIN length (required for some OpenPGP operations).

robin-nitrokey commented 2 years ago

PIN handling in Trussed is now discussed here: https://github.com/trussed-dev/trussed/discussions/32

sosthene-nitrokey commented 2 years ago

There were some ideas about having only one PIN for the entire key (i.e. the fido PIN is the same as the PGP PIN). I'm not sure it's a good idea (for example one might not expect a "factory reset" for PGP to factory reset all the apps). Anyway it might also not be compatible with some features like the KDF-DO?

jans23 commented 2 years ago

I think that's a good idea. Because otherwise users would end up having almost 10 PINs for a single device (anticipating ca. 5 different apps eventually) which is too complicated. Viewing it as individual apps is a technical view and may not reflect the user's view.

szszszsz commented 2 years ago

Reminding, that we already have devices with multiple applications sharing a single PIN, most notably Nitrokey Storage. Indeed KDF-DO setup needs special treatment if applied, but perhaps this could be combined.

robin-nitrokey commented 2 years ago

There are two more aspects to keep in mind:

sosthene-nitrokey commented 2 years ago

It also means that the security of the PIN is brought to the lowest security for each app. For example the OpenPGP spec forces us to leak information about the length of the PIN. Sharing the PIN means leaking the length of every PIN.

szszszsz commented 2 years ago

We do not need to worry about brute-force attack unless in cases where we do not use SE - then it might be a good idea to have a separate PIN.

robin-nitrokey commented 2 years ago

Also keep in mind this error for the current Nitrokey devices that is caused by the PIN being shared between the OpenPGP card and the custom OTP and PWS applications. We should not reproduce this issue with the NK3.

szszszsz commented 2 years ago

Also keep in mind this error for the current Nitrokey devices that is caused by the PIN being shared between the OpenPGP card and the custom OTP and PWS applications. We should not reproduce this issue with the NK3.

The issue you mention is not about the PIN sharing per se, but smart card sharing, and it's inability to work in a concurrent context. I do not see this as an argument.

robin-nitrokey commented 2 years ago

It’s not necessarily an argument against PIN sharing, but we should definitely have tests for this if we implement PIN sharing. As far as I remember and read from the discussion, we never found out specifically what causes GnuPG to abandon the session.

jans23 commented 2 years ago

For example the OpenPGP spec forces us to leak information about the length of the PIN.

What is the background of this? Is it really necessary and could it be avoided or worked-around?

sosthene-nitrokey commented 2 years ago

The command to change password is serialized old_pin || new_pin, so to parse it we necessarily need to know the length of the old pin, so AFAIK isn't really possible to handle this without having side-channel leakage.

It also means we need to store at least the length of the PIN in the flash, not on the SE050, which makes it vulnerable to hardware attacks.

sosthene-nitrokey commented 2 years ago

Though if KDF-DO is enabled (it's off by default and requires user action to enable it) the PIN length as seen by the card becomes 32 bytes so we don't really leak anything.

EliteTK commented 1 year ago

I am not familiar with the design of the NK3 or how these kinds of devices work, so I apologise in advance for my ignorance of how this all works.

I was researching what the intended features were for the NK3's NFC facilities. Specifically I was curious if, like the yubikey neo (and I believe yubikey 5), I would be able to use NFC to decrypt gpg encrypted passwords on my phone (using Password Store and OpenKechain) as this is a major feature for me and I have been seriously considering the NK3 as a successor to my YKN.

By the sounds of it, the intention is to require use of SE050 to store the PIN thereby preventing this usecase (due to the power requirements of the secure element).

Am I correct in my understanding? If the pin was not stored within SE050, would it be possible to use NK3 as a NFC smartcard? If so, would it instead be possible (to mitigate brute force attacks) to simply implement a 3 attempt pin lockout?

Thanks in advance.

robin-nitrokey commented 1 year ago

@EliteTK Generally speaking, our design goal for the Nitrokey 3 is to let users configure whether they want to use the SE050 (higher security) or software implementations (usable over NFC, easier to review and audit) where applicable.

We will also consider this when adding SE050 support to opcard. As the implementation details are abstracted by the Trussed framework, it will likely be possible to configure opcard to not use the SE050.

By the way, there already is a PIN retry counter (3) as a protection against brute force attacks. The SE050 adds additional protection against attackers with access to the hardware and the possibility to read out the flash memory.

robin-nitrokey commented 1 year ago

To address your specific point regarding NFC usage, I should add that there are other factors than only the SE050, e. g. access to the external flash (which also requires more power than NFC can provide). We will evaluate this but it is not clear yet if, how and when it will be implemented.

robin-nitrokey commented 1 year ago

Fixed by using trussed-auth in https://github.com/Nitrokey/opcard-rs/pull/125.