trussed icon indicating copy to clipboard operation
trussed copied to clipboard

Fix key confusion

Open sosthene-nitrokey opened this issue 1 year ago • 3 comments

This PR fixes key type confusions that could happen previously. Because many symmetric operations didn't check the Kind of the key they used, it was possible to use an ECC key for HMAC or AES.

This PR also prevents using Shared as a key unless it's been through DeriveKey. I don't think this breaks anything deployed (I checked for fido-authenticator.

I also find it weird that unsafe_inject_shared_secret yields a key of Kind::Shared IMO it should be Symmetric so that it can be used for symmetric cryptography. I feel like right now the distinction between Kind::Shared and Kind::Symmetric doesn't bring anything.

sosthene-nitrokey avatar Aug 18 '22 13:08 sosthene-nitrokey

The difference between "shared secret" and "symmetric key" is quite important, and IMHO we should try and uphold it in our type system. The point is that the output of a Diffie-Hellman is considered to be random, but not uniformly random. On the other hand, a symmetric key should be uniformly random. Hence, a KDF should be applied to shared secrets before use as key. There are fancy and less fancy ways to KDF, but already applying some cryptographic hash like SHA-256 is enough (although typically one would use something with better security proofs).

nickray avatar Aug 22 '22 12:08 nickray

I understand that. What I don't understand is why unsafe_inject_key has been removed to be replaced by unsafe_inject_shared_key. The only situation I see this being currently used is in trussed-totp-pc-tutorial, but in this case the secret is supposed to be randomly generated and therefore of high entropy. The current situation means that the TOTP mechanism must work with kind::Shared keys, which doesn't make sense to me. unsafe_inject_shared_key is also the only way to obtain a symmetric 24 byte key for 3-DES, which should probably require a high entropy kind::Symmetric.

sosthene-nitrokey avatar Aug 22 '22 12:08 sosthene-nitrokey

I'll allow using kind::Shared for HMAC and TOTP and not for anything else.

sosthene-nitrokey avatar Aug 25 '22 12:08 sosthene-nitrokey