Add encryption key derivation from PIN

[sosthene-nitrokey]

The test currently fail because of a bug in cbor-smol fixed in https://github.com/nickray/cbor-smol/pulls

The tests now fails because of the littlefs2 lookahead bug fixed in version 0.4

[sosthene-nitrokey]

SetPin has a flag to enable a random key to be associated with the PIN. This key can be accessed after validation of the pin with GetPinKey. Changing the Pin with ChangePin doesn't change the key that is derived.

When creating a SoftwareAuth instance, the runner can give it some key material that would come from the hardware itself. This makes it slightly harder to extract data required for any brute-forcing.

Here is a pseudocode overview of the key derivation process.

// length depends on hardware
let device_ikm: [u8]= values_from_hardware();

// generated on first power-up, stays constant for the lifetime of the device
let device_salt: [u8;32] = csprng();

// The salt is useful for the security proof of HKDF
let device_prk = hkdf_extract(salt: device_salt, ikm: device_ikm);

// There is domain separation between each app
for app_id in apps {
    let app_encryption_key: [u8; 32] = hkdf_expand(device_prk, info: app_id, 32);

    // Deriving keys from a PIN or Password provided by the user
    for pwd in passwords {
        let salt = csprng();
        // Per-pin key is fully random, then wrapped using the Pin
        let wrapped_key = csprng();

        // This is fine because app_encryption_key is uniform, therefore pin_key is too
        //
        // We can't use PBKDF or argon2 here because of limited hardware. 
        // Ideally such a step would be done on the host
        //
        // `pin_key` is never stored, and derived on each call to `CheckPin` and `GetPinKey`
        let pin_key = HMAC(key: app_encryption_key, pin_id || len(pwd) || pwd || salt);

        // On pin creation or change, the key is wrapped and stored on a persistent filesystem
        // The constant nonce is acceptable and won't lead to nonce reuse because the `pin_key` is only used to encrypt data once
        // Any change of pin changes the `pin_key` also changes the salt
        // which means that it is not possible to use the same key twice
        store(aead_encrypt(key: pin_key, data: wrapped_key, nonce: [0;12]))


        // On GetPinKey requests, this gets us the key
        aead_decrypt(key: pin_key, data: load(), nonce: [0;12]))
   } 
}

[robin-nitrokey]

Please add a patch for littlefs2 to fix the CI. https://github.com/Nitrokey/littlefs2/releases/tag/v0.3.2-nitrokey-2 should work.

[sosthene-nitrokey]

This patch prevents trussed from compiling because of the read(&mut self) change

[sosthene-nitrokey]

It appears to work by using your littlefs 0.4 branch.

[robin-nitrokey]

Use https://github.com/Nitrokey/trussed/releases/tag/v0.1.0-nitrokey-5 instead for easier readability and for reproducibility?

[sosthene-nitrokey]

Actually the branch we were currently using was missing trussed-dev/trussed#94 so I fixed it. I thought it was the other way around

[sosthene-nitrokey]

After a bit of discussion with @szszszsz here is a clearer version of the pseudocode:

// length depends on hardware
let device_ikm: [u8]= values_from_hardware();

// generated on first power-up, stays constant for the lifetime of the device
let device_salt: [u8;32] = csprng();

// The salt is useful for the security proof of HKDF
let device_prk = hkdf_extract(salt: device_salt, ikm: device_ikm);

fn get_app_key(app_id) {
    // Domain separation between the apps
    // This means the `app_key` can also be used for purposes other than Pin key derivation.
    return hkdf_expand(prk: device_prk, info: app_id, output_len: 32);
}

fn register_pin(app_id, pin_id, pin) {
    let app_key = get_app_key(app_id);
    let salt = csprng();
    let key_to_be_wrapped = csprng();

    // Get a pseudo-random key from the pin and the salt
    //
    // This is fine because app_key is uniform, therefore pin_key is too
    // because HMAC is a PRF
    //
    // We can't use PBKDF or argon2 here because of limited hardware. 
    // Ideally such a step would be done on the host
    //
    // `pin_kek` is never stored
    let pin_kek = HMAC(key: app_key, pin_id || len(pin) || pin || salt);

    // On pin creation or change, the key is wrapped and stored on a persistent filesystem
    // The constant nonce is acceptable and won't lead to nonce reuse because the `pin_kek` is only used to encrypt this data once
    // 
    // Any change of pin changes also changes the salt
    // which means that it is not possible to get the `pin_kek` twice
    let wrapped_key = aead_encrypt(key: pin_kek, data: key_to_be_wrapped, nonce: [0;12]);

    to_presistent_storage(salt, wrapped_key);
}

fn get_pin_key(app_id, pin_id, pin) {
    let app_key = get_app_key(app_id);
    let (salt, wrapped_key) = from_persistent_storage();

    // re-derive the pin kek
    let pin_kek = HMAC(key: app_key, pin_id || len(pin) || pin || salt);

    // Unwrap the key
    let unwrapped_key = aead_decrypt(key: pin_kek, data: wrapped_key , nonce: [0;12])
    return unwrapped_key;
}


fn change_pin(app_id, pin_id, old_pin, new_pin) {
    let app_key = get_app_key(app_id);
    let key_to_be_wrapped = get_pin_key(app_id, pin_id, pin);

    // The procedure is the same as for `register_pin` but it reuses the `key_to_be_wrapped` instead of generating it

    // Generate a new salt for the new pin
    let salt = csprng();

    let pin_kek = HMAC(key: app_key, pin_id || len(new_pin) || new_pin || salt);

    let wrapped_key = aead_encrypt(key: pin_kek, data: key_to_be_wrapped, nonce: [0;12]);

    to_presistent_storage(salt, wrapped_key);
}

The algorithm used are SHA-256 (for HKDF and HMAC) and ChaCha8Poly1305 (for AEAD). The CSPRNG comes from Trussed, so it's a ChaCha8Rng from the ChaCha20 crate

References:

• HKDF
• HMAC
• ChaCha20Poly1305
• ChaCha8Poly1305

[szszszsz]

The reference to ChaCha8Poly1305 seems to be wrong. Can you check that?
I would like to have it describing the key generation process.

[szszszsz]

Supporting diagram to the pseudocode:

[sosthene-nitrokey]

There isn't really a standard to point to for ChaCha8 so it points instead to a publication justifying reduced rounds. I also added the ChaCha20 RFC for completeness

[szszszsz]

In other words, response to my request is at:

• https://datatracker.ietf.org/doc/html/rfc8439#section-4

  The Poly1305 key MUST be unpredictable to an attacker. Randomly
  generating the key would fulfill this requirement, except that
  Poly1305 is often used in communications protocols, so the receiver
  should know the key. Pseudorandom number generation such as by
  encrypting a counter is acceptable. Using ChaCha with a secret key
  and a nonce is also acceptable.

• https://datatracker.ietf.org/doc/html/rfc8439#section-2.6

  As said in Section 2.5, it is acceptable to generate the one-time
  Poly1305 key pseudorandomly. This section defines such a method.

• https://datatracker.ietf.org/doc/html/rfc8439#section-2.5

[sosthene-nitrokey]

• https://datatracker.ietf.org/doc/html/rfc8439#section-4

The Poly1305 key MUST be unpredictable to an attacker. Randomly
generating the key would fulfill this requirement, except that
Poly1305 is often used in communications protocols, so the receiver
should know the key. Pseudorandom number generation such as by
encrypting a counter is acceptable. Using ChaCha with a secret key
and a nonce is also acceptable.

• https://datatracker.ietf.org/doc/html/rfc8439#section-2.6

As said in Section 2.5, it is acceptable to generate the one-time
Poly1305 key pseudorandomly. This section defines such a method.

These are irrelevant as we don't use Poly1305 directly but instead use it as part of the ChaCha20Poly1305 AEAD construction, so the Poly1305 key is generated using ChaCha: https://datatracker.ietf.org/doc/html/rfc8439#section-2.6 using the key and the nonce, making it unpredictable.

The relevant part applies to the nonce:

https://datatracker.ietf.org/doc/html/rfc8439#section-2.8

A 96-bit nonce -- different for each invocation with the same key

In our case it is ok because the pin_kek is used for encryption only once on registration, so there is no risk of reuse.

[szszszsz]

There isn't really a standard to point to for ChaCha8 so it points instead to a publication justifying reduced rounds. I also added the ChaCha20 RFC for completeness

Here is the proper Chacha8 description:

• http://cr.yp.to/chacha/chacha-20080128.pdf

[robin-nitrokey]

Looks good to me, though I did not double-check the implementation details yet.

The only issue I see is that it is easy to make a mistake when choosing between the SetPin and ChangePin requests. Should we drop the SetPin request to avoid accidentally dropping the derived key? See also: Nitrokey/trussed-secrets-app#34 (comment)

[robin-nitrokey]

I think we also talked about having a way to access the application key (or deriving a key from it) when no PIN is set. That’s not implemented yet, right?

[sosthene-nitrokey]

The only issue I see is that it is easy to make a mistake when choosing between the SetPin and ChangePin requests. Should we drop the SetPin request to avoid accidentally dropping the derived key? See also: Nitrokey/trussed-secrets-app#34 (comment)

We could instead make it fail when the Pin type is not a simple hash to ensure such bugs are obvious in testing. I still think we want to have it available.

I think we also talked about having a way to access the application key (or deriving a key from it) when no PIN is set. That’s not implemented yet, right?

No that is not implemented. I think it can be a separate PR. Do we need it now?

[robin-nitrokey]

We could instead make it fail when the Pin type is not a simple hash to ensure such bugs are obvious in testing.

Sounds good.

[sosthene-nitrokey]

Another solution I see would be to separate SetPin to CreatePin and ResetPin, so that the intent is clearer. This will also make more sense with policies when they are implemented.

[robin-nitrokey]

Another solution I see would be to separate SetPin to CreatePin and ResetPin

Would there be a difference between the two requests? Or just two names for the same request?

[sosthene-nitrokey]

CreatePin would fail if the Pin already exists. ResetPin would fail if it doesn't.

[robin-nitrokey]

CreatePin would fail if the Pin already exists. ResetPin would fail if it doesn't.

Makes sense. Do you want to add it to this PR or should we do it separately?

[sosthene-nitrokey]

We can do it in another PR.