Nitrokey / opcard-rs

OpenPGP card implementation
49 stars 1 forks source link

Encrypt keys with trussed-auth #127

Closed sosthene-nitrokey closed 1 year ago

sosthene-nitrokey commented 1 year ago

Built on top of #125 and https://github.com/trussed-dev/trussed-auth/pull/17

This PR adds encryption for the user keys that will end up stored on the external flash.

sosthene-nitrokey commented 1 year ago

This PR encrypts:

Am I missing something?

sosthene-nitrokey commented 1 year ago

The PIN length is still stored unencrypted but I don't see anyway to avoid this. Maybe store it in the internal flash? The only really secure option I see would be to only store it in the volatile state and require a Verify call before CHANGE REFERENCE DATA, but that would violate the spec and it would also not work for the resetting code, which doesn't have a VERIFY call.

jans23 commented 1 year ago

Why do we need to store the PIN length?

sosthene-nitrokey commented 1 year ago

Because CHANGE REFERENCE DATA, the command to change the PIN, transmits the new and the old pin concatenated. We have no way to split the input in the correct place if we don't know the current PIN length.

jans23 commented 1 year ago

Is this demanded by the OpenPGP Card specification?

szszszsz commented 1 year ago

@jans23 Yes, that's the way of changing the PIN per specification - giving both concatenated without additional information.

sosthene-nitrokey commented 1 year ago

More detailed encryption mechanism:

User data that can be encrypted is. This applies to:

The same is true for admin data (private use data object 4).

All other data need to be accessible without verifying a PIN, and therefore cannot be encrypted using user-provided entropy.

Encryption uses keys wrapped using the user and admin PIN coming from trussed-auth. Cryptographic keys are stored wrapped and are unwrapped only when necessary with the user PIN.

Encryption path

Encryption (normal) path:

The 3 PGP keys are stored wrapped with the user key. They are only stored unwrapped in volatile memory temporarly when being generated, imported or used (with some caching).

User key wrapping

This path is by itself incompatible with the OpenPGP standard.

Key generation

Only the admin pin is required generate and import PGP keys. Without the user PIN, it would be impossible to wrap them.

User PIN unblock

The admin pin and the resetting code need to be able to reset the user pin, but the user must then still be able to unwrap the PGP keys.

User key "backups"

To adapt to the two above use cases, the user key is backed up twice. One bakcup is wrapped with the admin key (ADMIN_USER_KEY_BACKUP) and the other is wrapped with the resetting code key (RC_USER_KEY_BACKUP).

Card administration

When the admin generates or imports a PGP key, the card uses the admin key to unwrap ADMIN_USER_KEY_BACKUP, giving it access to the user key. The user key is then used to wrap the imported/generated PGP key.

User PIN reset

When RESET RETRY COUNTER is called, the card unwraps either ADMIN_USER_KEY_BACKUP or RC_USER_KEY_BACKUP depending on wether the admin key or the resetting code is used. This gives the card access to the user key, which can unblock the user pin with trussed-auth's reset_pin_with_key.

Private use data objects

Objects are majoritarly readable without authentication and therefore are store in plain text. There are two exceptions:

With a schema in nextcloud: https://cloud.nitrokey.com/f/522841

sosthene-nitrokey commented 1 year ago

I'll break this PR into some smaller PRs for easier review.

sosthene-nitrokey commented 1 year ago

PRs #134, #135, #136, #137, #138 are the parts of this PRs separated for easier review. They all point to the data-encryption branch so that they can be merged as soon as reviewed, and then we merge the data-encryption PR and close this one.

robin-nitrokey commented 1 year ago

To do before the final merge:

robin-nitrokey commented 1 year ago

Thanks for splitting up the PR! The code changes look good to me.

sosthene-nitrokey commented 1 year ago

Replaced by #139