tendermint-rs: Reject low order points (fixes #142)

tendermint-rs/src/secret_connection.rs

@@ -18,6 +18,7 @@ use std::{
     io::{self, Read, Write},
     marker::{Send, Sync},
 };
+use subtle::ConstantTimeEq;
 use x25519_dalek::{EphemeralSecret, PublicKey as EphemeralPublic};
 
 /// Size of the MAC tag
@@ -62,6 +63,17 @@ impl<IoHandler: Read + Write + Send + Sync> SecretConnection<IoHandler> {
         // Compute common shared secret.
         let shared_secret = EphemeralSecret::diffie_hellman(local_eph_privkey, &remote_eph_pubkey);
 
+        // Reject all-zero outputs from X25519 (i.e. from low-order points)
+        //
+        // See the following for information on potential attacks this check
+        // aids in mitigating:
+        //
+        // - https://github.com/tendermint/kms/issues/142
+        // - https://eprint.iacr.org/2019/526.pdf
+        if shared_secret.as_bytes().ct_eq(&[0x00; 32]).unwrap_u8() == 1 {
+            return Err(Error::InvalidKey);
+        }
+
         // Sort by lexical order.
         let local_eph_pubkey_bytes = *local_eph_pubkey.as_bytes();
         let (low_eph_pubkey_bytes, _) =
@@ -262,15 +274,15 @@ where
     }
 }
 
-// Returns pubkey, private key
+/// Returns pubkey, private key
 fn gen_eph_keys() -> (EphemeralPublic, EphemeralSecret) {
     let mut local_csprng = OsRng::new().unwrap();
     let local_privkey = EphemeralSecret::new(&mut local_csprng);
     let local_pubkey = EphemeralPublic::from(&local_privkey);
     (local_pubkey, local_privkey)
 }
 
-// Returns remote_eph_pubkey
+/// Returns remote_eph_pubkey
 fn share_eph_pubkey<IoHandler: Read + Write + Send + Sync>(
     handler: &mut IoHandler,
     local_eph_pubkey: &EphemeralPublic,
@@ -306,10 +318,64 @@ fn share_eph_pubkey<IoHandler: Read + Write + Send + Sync>(
     // of the pub key:
     remote_eph_pubkey_fixed.copy_from_slice(&buf[2..34]);
 
-    Ok(EphemeralPublic::from(remote_eph_pubkey_fixed))
+    if is_blacklisted_point(&remote_eph_pubkey_fixed) {
+        Err(Error::InvalidKey)
+    } else {
+        Ok(EphemeralPublic::from(remote_eph_pubkey_fixed))
+    }
+}
+
+/// Reject the blacklist of degenerate points listed on <https://cr.yp.to/ecdh.html>
+///
+/// These points contain low-order elements. Rejecting them is suggested in
+/// the "May the Fourth" paper under Section 5: Software Countermeasures
+/// (see "Rejecting Known Bad Points" subsection):
+///
+/// <https://eprint.iacr.org/2017/806.pdf>
+fn is_blacklisted_point(point: &[u8; 32]) -> bool {
+    // Note: as these are public points and do not interact with secret-key
+    // material in any way, this check does not need to be performed in
+    // constant-time.
+    match point {
+        // 0 (order 4)
+        &[0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00] => {
+            true
+        }
+
+        // 1 (order 1)
+        [0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00] => {
+            true
+        }
+
+        // 325606250916557431795983626356110631294008115727848805560023387167927233504 (order 8)
+        &[0xe0, 0xeb, 0x7a, 0x7c, 0x3b, 0x41, 0xb8, 0xae, 0x16, 0x56, 0xe3, 0xfa, 0xf1, 0x9f, 0xc4, 0x6a, 0xda, 0x09, 0x8d, 0xeb, 0x9c, 0x32, 0xb1, 0xfd, 0x86, 0x62, 0x05, 0x16, 0x5f, 0x49, 0xb8, 0x00] => {
+            true
+        }
+
+        // 39382357235489614581723060781553021112529911719440698176882885853963445705823 (order 8)
+        &[0x5f, 0x9c, 0x95, 0xbc, 0xa3, 0x50, 0x8c, 0x24, 0xb1, 0xd0, 0xb1, 0x55, 0x9c, 0x83, 0xef, 0x5b, 0x04, 0x44, 0x5c, 0xc4, 0x58, 0x1c, 0x8e, 0x86, 0xd8, 0x22, 0x4e, 0xdd, 0xd0, 0x9f, 0x11, 0x57] => {
+            true
+        }
+
+        // p - 1 (order 2)
+        [0xec, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f] => {
+            true
+        }
+
+        // p (order 4) */
+        [0xed, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f] => {
+            true
+        }
+
+        // p + 1 (order 1)
+        [0xee, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f] => {
+            true
+        }
+        _ => false,
+    }
 }
 
-// Return is of the form lo, hi
+/// Return is of the form lo, hi
 fn sort32(first: [u8; 32], second: [u8; 32]) -> ([u8; 32], [u8; 32]) {
     if second > first {
         (first, second)
@@ -318,7 +384,7 @@ fn sort32(first: [u8; 32], second: [u8; 32]) -> ([u8; 32], [u8; 32]) {
     }
 }
 
-// Sign the challenge with the local private key
+/// Sign the challenge with the local private key
 fn sign_challenge(
     challenge: &[u8; 32],
     local_privkey: &dyn Signer<ed25519::Signature>,