aes: tweak weak key test (#469)

newpavlov · web-flow · commit 5ac1ffda0a8d · 2025-02-14T15:06:04.000+03:00
The new code results in a better codegen with resulting binaries still being const time: https://rust.godbolt.org/z/x1a8aTsbY
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/aes/Cargo.toml b/aes/Cargo.toml
@@ -15,7 +15,6 @@ categories = ["cryptography", "no-std"]
 [dependencies]
 cfg-if = "1"
 cipher = "=0.5.0-pre.7"
-subtle = { version = "2.6", default-features = false }
 zeroize = { version = "1.5.6", optional = true, default-features = false, features = [
     "aarch64",
 ] }
diff --git a/aes/src/armv8.rs b/aes/src/armv8.rs
@@ -110,8 +110,9 @@ macro_rules! define_aes_impl {
                 Self { encrypt, decrypt }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
@@ -199,8 +200,9 @@ macro_rules! define_aes_impl {
                 Self { backend }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
@@ -265,8 +267,9 @@ macro_rules! define_aes_impl {
                 Self { backend }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
diff --git a/aes/src/autodetect.rs b/aes/src/autodetect.rs
@@ -105,8 +105,9 @@ macro_rules! define_aes_impl {
                 Self { inner, token }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
@@ -226,8 +227,9 @@ macro_rules! define_aes_impl {
                 Self { inner, token }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
@@ -357,8 +359,9 @@ macro_rules! define_aes_impl {
                 Self { inner, token }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
diff --git a/aes/src/lib.rs b/aes/src/lib.rs
@@ -143,11 +143,41 @@ cfg_if! {
 }
 
 pub use cipher;
-use cipher::{array::Array, consts::U16};
+use cipher::{array::Array, consts::U16, crypto_common::WeakKeyError};
 
 /// 128-bit AES block
 pub type Block = Array<u8, U16>;
 
+/// Check if any bit of the upper half of the key is set.
+///
+/// This follows the interpretation laid out in section `11.4.10.4 Reject of weak keys`
+/// from the [TPM specification][0]:
+/// ```text
+/// In the case of AES, at least one bit in the upper half of the key must be set
+/// ```
+///
+/// [0]: https://trustedcomputinggroup.org/wp-content/uploads/TPM-2.0-1.83-Part-1-Architecture.pdf#page=82
+pub(crate) fn weak_key_test<const N: usize>(key: &[u8; N]) -> Result<(), WeakKeyError> {
+    let t = match N {
+        16 => u64::from_ne_bytes(key[..8].try_into().unwrap()),
+        24 => {
+            let t1 = u64::from_ne_bytes(key[..8].try_into().unwrap());
+            let t2 = u32::from_ne_bytes(key[8..12].try_into().unwrap());
+            t1 | u64::from(t2)
+        }
+        32 => {
+            let t1 = u64::from_ne_bytes(key[..8].try_into().unwrap());
+            let t2 = u64::from_ne_bytes(key[8..16].try_into().unwrap());
+            t1 | t2
+        }
+        _ => unreachable!(),
+    };
+    match t {
+        0 => Err(WeakKeyError),
+        _ => Ok(()),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     #[cfg(feature = "zeroize")]
diff --git a/aes/src/macros.rs b/aes/src/macros.rs
@@ -103,29 +103,3 @@ macro_rules! impl_backends {
         }
     };
 }
-
-macro_rules! weak_key_test {
-    ($key: expr, $k: ty) => {{
-        // Check if any bit of the upper half of the key is set
-        //
-        // This follows the interpretation laid out in section `11.4.10.4 Reject of weak keys`
-        // from the TPM specification:
-        // ```
-        // In the case of AES, at least one bit in the upper half of the key must be set
-        // ```
-        // See: https://trustedcomputinggroup.org/wp-content/uploads/TPM-2.0-1.83-Part-1-Architecture.pdf#page=82
-        let mut weak = subtle::Choice::from(0);
-
-        for v in &$key
-            [..(<<$k as cipher::KeySizeUser>::KeySize as cipher::typenum::Unsigned>::USIZE / 2)]
-        {
-            weak |= <_ as subtle::ConstantTimeGreater>::ct_gt(v, &0);
-        }
-
-        if weak.unwrap_u8() == 0 {
-            Err(cipher::crypto_common::WeakKeyError)
-        } else {
-            Ok(())
-        }
-    }};
-}
diff --git a/aes/src/ni.rs b/aes/src/ni.rs
@@ -120,8 +120,9 @@ macro_rules! define_aes_impl {
                 Self { encrypt, decrypt }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
@@ -199,8 +200,9 @@ macro_rules! define_aes_impl {
                 }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
@@ -263,8 +265,9 @@ macro_rules! define_aes_impl {
                 $name_enc::new(key).into()
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
diff --git a/aes/src/soft.rs b/aes/src/soft.rs
@@ -69,8 +69,9 @@ macro_rules! define_aes_impl {
                 }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
@@ -152,8 +153,9 @@ macro_rules! define_aes_impl {
                 Self { inner }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 
@@ -207,8 +209,9 @@ macro_rules! define_aes_impl {
                 Self { inner }
             }
 
+            #[inline]
             fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {
-                weak_key_test!(key, Self)
+                crate::weak_key_test(&key.0)
             }
         }
 

Original file line number	Diff line number	Diff line change
`@@ -110,8 +110,9 @@ macro_rules! define_aes_impl {`
`110`	`110`	`Self { encrypt, decrypt }`
`111`	`111`	`}`
`112`	`112`
	`113`	`+ #[inline]`
`113`	`114`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`114`		`- weak_key_test!(key, Self)`
	`115`	`+ crate::weak_key_test(&key.0)`
`115`	`116`	`}`
`116`	`117`	`}`
`117`	`118`
`@@ -199,8 +200,9 @@ macro_rules! define_aes_impl {`
`199`	`200`	`Self { backend }`
`200`	`201`	`}`
`201`	`202`
	`203`	`+ #[inline]`
`202`	`204`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`203`		`- weak_key_test!(key, Self)`
	`205`	`+ crate::weak_key_test(&key.0)`
`204`	`206`	`}`
`205`	`207`	`}`
`206`	`208`
`@@ -265,8 +267,9 @@ macro_rules! define_aes_impl {`
`265`	`267`	`Self { backend }`
`266`	`268`	`}`
`267`	`269`
	`270`	`+ #[inline]`
`268`	`271`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`269`		`- weak_key_test!(key, Self)`
	`272`	`+ crate::weak_key_test(&key.0)`
`270`	`273`	`}`
`271`	`274`	`}`
`272`	`275`
Original file line number	Diff line number	Diff line change
`@@ -105,8 +105,9 @@ macro_rules! define_aes_impl {`
`105`	`105`	`Self { inner, token }`
`106`	`106`	`}`
`107`	`107`
	`108`	`+ #[inline]`
`108`	`109`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`109`		`- weak_key_test!(key, Self)`
	`110`	`+ crate::weak_key_test(&key.0)`
`110`	`111`	`}`
`111`	`112`	`}`
`112`	`113`
`@@ -226,8 +227,9 @@ macro_rules! define_aes_impl {`
`226`	`227`	`Self { inner, token }`
`227`	`228`	`}`
`228`	`229`
	`230`	`+ #[inline]`
`229`	`231`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`230`		`- weak_key_test!(key, Self)`
	`232`	`+ crate::weak_key_test(&key.0)`
`231`	`233`	`}`
`232`	`234`	`}`
`233`	`235`
`@@ -357,8 +359,9 @@ macro_rules! define_aes_impl {`
`357`	`359`	`Self { inner, token }`
`358`	`360`	`}`
`359`	`361`
	`362`	`+ #[inline]`
`360`	`363`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`361`		`- weak_key_test!(key, Self)`
	`364`	`+ crate::weak_key_test(&key.0)`
`362`	`365`	`}`
`363`	`366`	`}`
`364`	`367`
Original file line number	Diff line number	Diff line change
`@@ -120,8 +120,9 @@ macro_rules! define_aes_impl {`
`120`	`120`	`Self { encrypt, decrypt }`
`121`	`121`	`}`
`122`	`122`
	`123`	`+ #[inline]`
`123`	`124`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`124`		`- weak_key_test!(key, Self)`
	`125`	`+ crate::weak_key_test(&key.0)`
`125`	`126`	`}`
`126`	`127`	`}`
`127`	`128`
`@@ -199,8 +200,9 @@ macro_rules! define_aes_impl {`
`199`	`200`	`}`
`200`	`201`	`}`
`201`	`202`
	`203`	`+ #[inline]`
`202`	`204`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`203`		`- weak_key_test!(key, Self)`
	`205`	`+ crate::weak_key_test(&key.0)`
`204`	`206`	`}`
`205`	`207`	`}`
`206`	`208`
`@@ -263,8 +265,9 @@ macro_rules! define_aes_impl {`
`263`	`265`	`$name_enc::new(key).into()`
`264`	`266`	`}`
`265`	`267`
	`268`	`+ #[inline]`
`266`	`269`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`267`		`- weak_key_test!(key, Self)`
	`270`	`+ crate::weak_key_test(&key.0)`
`268`	`271`	`}`
`269`	`272`	`}`
`270`	`273`
Original file line number	Diff line number	Diff line change
`@@ -69,8 +69,9 @@ macro_rules! define_aes_impl {`
`69`	`69`	`}`
`70`	`70`	`}`
`71`	`71`
	`72`	`+ #[inline]`
`72`	`73`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`73`		`- weak_key_test!(key, Self)`
	`74`	`+ crate::weak_key_test(&key.0)`
`74`	`75`	`}`
`75`	`76`	`}`
`76`	`77`
`@@ -152,8 +153,9 @@ macro_rules! define_aes_impl {`
`152`	`153`	`Self { inner }`
`153`	`154`	`}`
`154`	`155`
	`156`	`+ #[inline]`
`155`	`157`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`156`		`- weak_key_test!(key, Self)`
	`158`	`+ crate::weak_key_test(&key.0)`
`157`	`159`	`}`
`158`	`160`	`}`
`159`	`161`
`@@ -207,8 +209,9 @@ macro_rules! define_aes_impl {`
`207`	`209`	`Self { inner }`
`208`	`210`	`}`
`209`	`211`
	`212`	`+ #[inline]`
`210`	`213`	`fn weak_key_test(key: &Key<Self>) -> Result<(), WeakKeyError> {`
`211`		`- weak_key_test!(key, Self)`
	`214`	`+ crate::weak_key_test(&key.0)`
`212`	`215`	`}`
`213`	`216`	`}`
`214`	`217`