fix: reject low-order X25519 public keys (#418)

Return `Error::InvalidData` when an X25519 exchange is not contributory
(low-order peer public key) in `mix_key_exchange` and in asymmetric
encryption/decryption, including online ciphertexts.

Author: mmorrissette-devolutions

.github/workflows/other/webassembly-build/action.yml

@@ -7,7 +7,7 @@ runs:
       shell: bash
       run: |
         rustup update
-        cargo install wasm-pack
+        cargo install wasm-pack --locked
 
     - uses: actions/setup-node@v4
       with:

.github/workflows/tests/webassembly/safari/action.yml

@@ -9,7 +9,7 @@ runs:
         curl https://sh.rustup.rs -sSf | sh -s -- -y --default-toolchain stable
         echo "$HOME/.cargo/bin" >> $GITHUB_PATH
         source $HOME/.cargo/env
-        cargo install wasm-pack
+        cargo install wasm-pack --locked
 
     - name: Unit tests
       working-directory: ./

src/ciphertext/ciphertext_v2.rs

@@ -191,6 +191,10 @@ impl CiphertextV2Asymmetric {
 
         let key = ephemeral_private_key.diffie_hellman(&public_key);
 
+        if !key.was_contributory() {
+            return Err(Error::InvalidData);
+        }
+
         let ciphertext = CiphertextV2Symmetric::encrypt(data, key.as_bytes(), aad, header)?;
 
         Ok(Self {
@@ -209,6 +213,10 @@ impl CiphertextV2Asymmetric {
 
         let key = private_key.diffie_hellman(&self.public_key);
 
+        if !key.was_contributory() {
+            return Err(Error::InvalidData);
+        }
+
         self.ciphertext.decrypt(key.as_bytes(), aad, header)
     }
 }

src/key/key_v1.rs

@@ -108,8 +108,15 @@ pub fn generate_keypair() -> KeyV1Pair {
     }
 }
 
-pub fn mix_key_exchange(private: &KeyV1Private, public: &KeyV1Public) -> Vec<u8> {
-    private.key.diffie_hellman(&public.key).as_bytes().to_vec()
+pub fn mix_key_exchange(private: &KeyV1Private, public: &KeyV1Public) -> Result<Vec<u8>> {
+    let shared = private.key.diffie_hellman(&public.key);
+
+    // Reject low-order public keys, which yield an all-zero shared secret
+    if !shared.was_contributory() {
+        return Err(Error::InvalidData);
+    }
+
+    Ok(shared.as_bytes().to_vec())
 }
 
 impl From<&KeyV1Public> for x25519_dalek::PublicKey {

src/key/mod.rs

@@ -210,11 +210,11 @@ pub fn generate_keypair(version: KeyVersion) -> KeyPair {
 /// assert_eq!(bob_shared, alice_shared);
 /// ```
 pub fn mix_key_exchange(private_key: &PrivateKey, public_key: &PublicKey) -> Result<Vec<u8>> {
-    Ok(match (&private_key.payload, &public_key.payload) {
+    match (&private_key.payload, &public_key.payload) {
         (PrivateKeyPayload::V1(private_key), PublicKeyPayload::V1(public_key)) => {
             key_v1::mix_key_exchange(private_key, public_key)
         } //_ => Err(DevoCryptoError::InvalidDataType),
-    })
+    }
 }
 
 fn keypair_headers(version: KeyVersion) -> (Header<PrivateKey>, Header<PublicKey>) {

src/online_ciphertext/mod.rs

@@ -128,7 +128,8 @@ impl OnlineCiphertextHeader {
         match self.payload {
             OnlineCiphertextHeaderPayload::V1(header) => match header {
                 OnlineCiphertextV1Header::Asymmetric(header) => {
-                    let cipher = OnlineCiphertextV1Decryptor::new_asymmetric(key, full_aad, header);
+                    let cipher =
+                        OnlineCiphertextV1Decryptor::new_asymmetric(key, full_aad, header)?;
 
                     Ok(OnlineCiphertextDecryptor::V1(cipher))
                 }

src/online_ciphertext/online_ciphertext_v1.rs

@@ -334,6 +334,10 @@ impl OnlineCiphertextV1Encryptor {
 
         let key = ephemeral_private_key.diffie_hellman(&public_key);
 
+        if !key.was_contributory() {
+            return Err(Error::InvalidData);
+        }
+
         // Generate a new nonce
         let mut nonce = [0u8; 20];
         rand::rngs::SysRng
@@ -389,12 +393,16 @@ impl OnlineCiphertextV1Decryptor {
         private_key: &PrivateKey,
         mut aad: Vec<u8>,
         header: OnlineCiphertextV1HeaderAsymmetric,
-    ) -> Self {
+    ) -> Result<Self> {
         // Perform a ECDH exchange as per ECIES
         let private_key = x25519_dalek::StaticSecret::from(private_key);
 
         let key = private_key.diffie_hellman(&header.public_key);
 
+        if !key.was_contributory() {
+            return Err(Error::InvalidData);
+        }
+
         // Derive the key
         let key = Zeroizing::new(blake3::derive_key(CONTEXT, key.as_bytes()));
         let cipher = XChaCha20Poly1305::new_from_slice(key.as_ref())
@@ -406,11 +414,11 @@ impl OnlineCiphertextV1Decryptor {
         let mut header_bytes: Vec<u8> = header.borrow().into();
         aad.append(&mut header_bytes);
 
-        Self {
+        Ok(Self {
             header: OnlineCiphertextV1Header::Asymmetric(header),
             aad,
             cipher,
-        }
+        })
     }
 }
 

tests/low_order_points.rs

@@ -0,0 +1,119 @@
+use devolutions_crypto::Error;
+use devolutions_crypto::ciphertext::{Ciphertext, CiphertextVersion, encrypt_asymmetric};
+use devolutions_crypto::key::{KeyVersion, PublicKey, generate_keypair, mix_key_exchange};
+use devolutions_crypto::online_ciphertext::{
+    OnlineCiphertextEncryptor, OnlineCiphertextHeader, OnlineCiphertextVersion,
+};
+
+use std::convert::TryFrom;
+
+// Curve25519 points of small order, including non-canonical encodings
+const LOW_ORDER_POINTS: [[u8; 32]; 7] = [
+    hex("0000000000000000000000000000000000000000000000000000000000000000"),
+    hex("0100000000000000000000000000000000000000000000000000000000000000"),
+    hex("e0eb7a7c3b41b8ae1656e3faf19fc46ada098deb9c32b1fd866205165f49b800"),
+    hex("5f9c95bca3508c24b1d0b1559c83ef5b04445cc4581c8e86d8224eddd09f1157"),
+    hex("ecffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f"),
+    hex("edffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f"),
+    hex("eeffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f"),
+];
+
+const fn hex(s: &str) -> [u8; 32] {
+    const fn nibble(c: u8) -> u8 {
+        match c {
+            b'0'..=b'9' => c - b'0',
+            b'a'..=b'f' => c - b'a' + 10,
+            _ => panic!("invalid hex"),
+        }
+    }
+
+    let s = s.as_bytes();
+    let mut out = [0u8; 32];
+    let mut i = 0;
+    while i < 32 {
+        out[i] = (nibble(s[i * 2]) << 4) | nibble(s[i * 2 + 1]);
+        i += 1;
+    }
+    out
+}
+
+fn with_last_32(bytes: &[u8], point: &[u8; 32]) -> Vec<u8> {
+    let mut bytes = bytes.to_vec();
+    let offset = bytes.len() - 32;
+    bytes[offset..].copy_from_slice(point);
+    bytes
+}
+
+fn low_order_public_key(point: &[u8; 32]) -> PublicKey {
+    let template: Vec<u8> = generate_keypair(KeyVersion::Latest).public_key.into();
+    PublicKey::try_from(with_last_32(&template, point).as_slice()).unwrap()
+}
+
+#[test]
+fn mix_key_exchange_rejects_low_order_points() {
+    let keypair = generate_keypair(KeyVersion::Latest);
+
+    for point in &LOW_ORDER_POINTS {
+        let public_key = low_order_public_key(point);
+        let result = mix_key_exchange(&keypair.private_key, &public_key);
+        assert!(matches!(result, Err(Error::InvalidData)));
+    }
+}
+
+#[test]
+fn encrypt_asymmetric_rejects_low_order_points() {
+    for point in &LOW_ORDER_POINTS {
+        let public_key = low_order_public_key(point);
+        let result = encrypt_asymmetric(b"data", &public_key, CiphertextVersion::Latest);
+        assert!(matches!(result, Err(Error::InvalidData)));
+    }
+}
+
+#[test]
+fn decrypt_asymmetric_rejects_low_order_points() {
+    let keypair = generate_keypair(KeyVersion::Latest);
+    let ciphertext: Vec<u8> =
+        encrypt_asymmetric(b"data", &keypair.public_key, CiphertextVersion::Latest)
+            .unwrap()
+            .into();
+
+    for point in &LOW_ORDER_POINTS {
+        // The ephemeral public key follows the 8-byte header
+        let mut forged = ciphertext.clone();
+        forged[8..40].copy_from_slice(point);
+
+        let forged = Ciphertext::try_from(forged.as_slice()).unwrap();
+        let result = forged.decrypt_asymmetric(&keypair.private_key);
+        assert!(matches!(result, Err(Error::InvalidData)));
+    }
+}
+
+#[test]
+fn online_ciphertext_rejects_low_order_points() {
+    let keypair = generate_keypair(KeyVersion::Latest);
+    let encryptor = OnlineCiphertextEncryptor::new_asymmetric(
+        &keypair.public_key,
+        b"",
+        16,
+        OnlineCiphertextVersion::Latest,
+    )
+    .unwrap();
+    let header: Vec<u8> = (&encryptor.get_header()).into();
+
+    for point in &LOW_ORDER_POINTS {
+        let public_key = low_order_public_key(point);
+        let result = OnlineCiphertextEncryptor::new_asymmetric(
+            &public_key,
+            b"",
+            16,
+            OnlineCiphertextVersion::Latest,
+        );
+        assert!(matches!(result, Err(Error::InvalidData)));
+
+        // The ephemeral public key is the last field of the header
+        let forged =
+            OnlineCiphertextHeader::try_from(with_last_32(&header, point).as_slice()).unwrap();
+        let result = forged.into_decryptor_asymmetric(&keypair.private_key, b"");
+        assert!(matches!(result, Err(Error::InvalidData)));
+    }
+}