Don't use shared secret directly as encryption key #2

New issue

Closed

opened 2025-03-07 13:51:42 +00:00 by G1gg1L3s · 1 comment

G1gg1L3s commented

2025-03-07 13:51:42 +00:00

(Migrated from github.com)

Shared secret as encryption key

Hewwo, fren! Just wanted to point out a teeny, fuzzy issue in your project! It looks like the shared secret is being used directly as an encryption key! 🫣

github.com/vxfemboy/purrcrypt@3e430180b2/src/keys.rs (L113-L116)

According to the elliptic-curve docs, raw secrets aren't all that purr-fect for encryption! 🚨

⚠️ WARNING: NOT UNIFORMLY RANDOM! ⚠️

This value is not uniformly random and should not be used directly as a cryptographic key for anything which requires that property (e.g. symmetric ciphers).

Instead, the resulting value should be used as input to a Key Derivation Function (KDF) or cryptographic hash function to produce a symmetric key. The SharedSecret::extract function will do this for you.

A shared secret isn't all snuggly and safe by itself - AES needs it to be uniformly random! If we don't boop it through a KDF first, we're left with a cryptographic weakness (oh no! 😿). Even big-brain cryptography hoomans like NIST recommend always deriving keys properly!

Shared secret as nonce

Oh nyoo! Not only is the shared secret used as a key, but part of it is also being used as a nonce! 😵

github.com/vxfemboy/purrcrypt@3e430180b2/src/keys.rs (L120)

Reusing key material like this can lead to unexpected cryptographic chaos! 😿

Solution

No need to hiss in distress! 🐍 We can fix this by deriving both the encryption key and nonce properly using a KDF! And guess what? The elliptic curve SharedSecret type already has a built-in API for this! ✨

Here's a pawsome example of how to do it (not tested, but looks fluff-tastic!):

let shared_secret = ephemeral_secret.diffie_hellman(recipient_public_key).extract::<Sha256>();
let mut encryption_key = [0; 32];
let mut nonce = [0; 12];
shared_secret.expand(b"encryption key", &mut encryption_key).expect("length should be ok"); 
shared_secret.expand(b"nonce", &mut nonce).expect("length should be ok");

Stay fluffy and secure! 🐾

## Shared secret as encryption key Hewwo, fren! Just wanted to point out a teeny, fuzzy issue in your project! It looks like the shared secret is being used directly as an encryption key! 🫣 https://github.com/vxfemboy/purrcrypt/blob/3e430180b21a2b6743f48a21f94a5487f374438a/src/keys.rs#L113-L116 According to the [elliptic-curve docs](https://docs.rs/elliptic-curve/0.13.8/elliptic_curve/ecdh/struct.SharedSecret.html#method.raw_secret_bytes), raw secrets aren't all that purr-fect for encryption! 🚨 > ⚠️ WARNING: NOT UNIFORMLY RANDOM! ⚠️ > > This value is not uniformly random and should not be used directly as a cryptographic key for anything which requires that property (e.g. symmetric ciphers). > > Instead, the resulting value should be used as input to a Key Derivation Function (KDF) or cryptographic hash function to produce a symmetric key. The [SharedSecret::extract](https://docs.rs/elliptic-curve/0.13.8/elliptic_curve/ecdh/struct.SharedSecret.html#method.extract) function will do this for you. A shared secret isn't all snuggly and safe by itself - AES needs it to be uniformly random! If we don't boop it through a KDF first, we're left with a cryptographic weakness (oh no! 😿). Even big-brain cryptography hoomans like [NIST](https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-56Ar3.pdf) recommend always deriving keys properly! ## Shared secret as nonce Oh nyoo! Not only is the shared secret used as a key, but part of it is also being used as a nonce! 😵 https://github.com/vxfemboy/purrcrypt/blob/3e430180b21a2b6743f48a21f94a5487f374438a/src/keys.rs#L120 Reusing key material like this can lead to unexpected cryptographic chaos! 😿 ## Solution No need to hiss in distress! 🐍 We can fix this by deriving both the encryption key and nonce properly using a KDF! And guess what? The elliptic curve [SharedSecret](https://docs.rs/elliptic-curve/0.13.8/elliptic_curve/ecdh/struct.SharedSecret.html#method.extract) type already has a built-in API for this! ✨ Here's a pawsome example of how to do it (not tested, but looks fluff-tastic!): ```rust let shared_secret = ephemeral_secret.diffie_hellman(recipient_public_key).extract::<Sha256>(); let mut encryption_key = [0; 32]; let mut nonce = [0; 12]; shared_secret.expand(b"encryption key", &mut encryption_key).expect("length should be ok"); shared_secret.expand(b"nonce", &mut nonce).expect("length should be ok"); ``` Stay fluffy and secure! 🐾

❤️ 2

nicoonoclaste commented

2025-03-10 15:54:07 +00:00

(Migrated from github.com)

Meowy thanks, @G1gg1L3s ! I had just spotted this, so you saved mew some writing.

I have just a couple small things to nyadd on that specific though:

With a single-use encryption key, the nonce can be a kmeown constant (typically all zeroes) rather than derived by a KDF:
this is safe, as it would be very difficult¹ to find multiple shared secrets which produce the same encryption key but distinct nonces.
Generally, using higher-level harder-to-misuse constructions (and APIs) is a lot safer than building one's own clawptography: since the necessary changes would break compatibility anymeow, it would be straightfurward to switch to libsodium's “sealed box”, or equivalently from HACL* (which is provably-correct) or dryoc (pure Rust, but hasn't been audited)

A compatibility break would also be a good timew address some other issues:

a versioned purrtocol (can take a single byte from the decoded stream) so future changes can be made more easily;
this exposes a compression oracle (for doing gay CRIMEs) which is exploitable if a user interactively encrypts a mix of secrets and attacker-controlled data; there are two main solutions there, best implemented in tandem:
- make compression opt-in, so the user can enable it only when it's safe to do so: this relies on the user understanding a pretty-subtle cryptographic concern, so it's not sufficient on its own;
- be a good boi and use padding to limit the information leakage via ciphertext size.

Equivalently, the likelyhood of random secp256k1 points being mapped (by the encoding and KDF) to the same 256b AES key but distinct 96b nonces, is negligible (less than the PRF–PRP gap) as long as the KDF is a PRF. ↩︎

Meowy thanks, @G1gg1L3s ! I had just spotted this, so you saved mew some writing. I have just a couple small things to nyadd on that specific though: - With a single-use encryption key, the nonce can be a kmeown constant (typically all zeroes) rather than derived by a KDF: this is safe, as it would be very difficult[^1] to find multiple shared secrets which produce the same encryption key but distinct nonces. - Generally, using higher-level harder-to-misuse constructions (and APIs) is a lot safer than building one's own clawptography: since the necessary changes would break compatibility anymeow, it would be straightfurward to switch to [libsodium's “sealed box”], or equivalently from [HACL*] (which is provably-correct) or [dryoc] (pure Rust, but hasn't been audited) A compatibility break would also be a good timew address some other issues: - a versioned purrtocol (can take a single byte from the decoded stream) so future changes can be made more easily; - this exposes a compression oracle (for doing gay [CRIME]s) which is exploitable if a user interactively encrypts a mix of secrets and attacker-controlled data; there are two main solutions there, best implemented in tandem: - make compression opt-in, so the user can enable it only when it's safe to do so: this relies on the user understanding a pretty-subtle cryptographic concern, so it's not sufficient on its own; - be a good boi and use [padding to limit the information leakage][padme] via ciphertext size. [^1]: Equivalently, the likelyhood of random secp256k1 points being mapped (by the encoding and KDF) to the same 256b AES key but distinct 96b nonces, is negligible (less than the PRF–PRP gap) as long as the KDF is a PRF. [libsodium's “sealed box”]: https://doc.libsodium.org/public-key_cryptography/sealed_boxes [HACL*]: https://hacl-star.github.io/HaclNaCl.html [dryoc]: https://docs.rs/dryoc/latest/dryoc/dryocbox/index.html#rustaceous-api-example [CRIME]: https://en.wikipedia.org/wiki/CRIME [padme]: https://bford.info/pub/sec/purb.pdf

👍 1