Make be_pad_into infallible

src/algorithms/pad.rs

@@ -1,57 +1,52 @@
 //! Special handling for converting the BigUint to u8 vectors
 
 use alloc::vec::Vec;
-use crypto_bigint::BoxedUint;
-use zeroize::{Zeroize, Zeroizing};
+use crypto_bigint::{BoxedUint, Choice, CtEq};
+use zeroize::Zeroizing;
 
-use crate::errors::{Error, Result};
-
-/// Writes the big-endian encoding of `bytes` into a buffer of exactly `padded_len` bytes
+/// Writes the big-endian encoding of `bytes` into `out`.
 ///
-/// Constant-time with respect to the *value*: the only branches depend on the public lengths
-/// The previous implementation sliced at `leading_zeros() / 8`, so the amount of data copied (and
-/// whether an error was returned) depended on how many leading zero bytes the secret plaintext
-/// had, which is exactly the signal the Marvin attack measures
+/// The value must fit in `out`: any bytes truncated from the front must be zero.
+/// In release builds, the amount of data copied depends only on the slice lengths.
 #[inline]
-fn be_pad_into(bytes: &[u8], out: &mut [u8]) -> Result<()> {
-    let padded_len = out.len();
-    if bytes.len() >= padded_len {
-        let (hi, lo) = bytes.split_at(bytes.len() - padded_len);
-        let overflow = hi.iter().fold(0u8, |acc, b| acc | b);
-        out.copy_from_slice(lo);
-        if core::hint::black_box(overflow) != 0 {
-            out.zeroize();
-            return Err(Error::InvalidPadLen);
-        }
-    } else {
-        let (zero, tail) = out.split_at_mut(padded_len - bytes.len());
-        zero.fill(0);
-        tail.copy_from_slice(bytes);
-    }
-    Ok(())
+fn be_pad_into(bytes: &[u8], out: &mut [u8]) {
+    let excess = bytes.len().saturating_sub(out.len());
+    debug_assert!(
+        !bytes[..excess]
+            .iter()
+            .fold(Choice::FALSE, |overflow, byte| overflow | !byte.ct_eq(&0u8))
+            .to_bool(),
+        "integer exceeds output width"
+    );
+
+    out.fill(0);
+    let tail = &bytes[excess..];
+    let start = out.len() - tail.len();
+    out[start..].copy_from_slice(tail);
 }
 
 /// Converts input to the new vector of the given length, using BE and with 0s left padded.
+///
+/// `input` must fit in `padded_len` bytes.
 #[inline]
-pub(crate) fn uint_to_be_pad(input: BoxedUint, padded_len: usize) -> Result<Vec<u8>> {
+pub(crate) fn uint_to_be_pad(input: BoxedUint, padded_len: usize) -> Vec<u8> {
     let mut out = vec![0u8; padded_len];
-    be_pad_into(&input.to_be_bytes(), &mut out)?;
-    Ok(out)
+    be_pad_into(&input.to_be_bytes(), &mut out);
+    out
 }
 
 /// Converts input to the new vector of the given length, using BE and with 0s left padded.
 ///
+/// `input` must fit in `padded_len` bytes.
+///
 /// For secret values: every intermediate buffer, including the returned one, is zeroized on drop.
 #[inline]
-pub(crate) fn uint_to_zeroizing_be_pad(
-    input: BoxedUint,
-    padded_len: usize,
-) -> Result<Zeroizing<Vec<u8>>> {
+pub(crate) fn uint_to_zeroizing_be_pad(input: BoxedUint, padded_len: usize) -> Zeroizing<Vec<u8>> {
     let input = Zeroizing::new(input);
     let bytes = Zeroizing::new(input.to_be_bytes());
     let mut out = Zeroizing::new(vec![0u8; padded_len]);
-    be_pad_into(&bytes, &mut out)?;
-    Ok(out)
+    be_pad_into(&bytes, &mut out);
+    out
 }
 
 #[cfg(test)]
@@ -60,20 +55,24 @@ mod tests {
     #[test]
     fn be_pad() {
         let x = BoxedUint::from(0x0102u64);
-        let p = uint_to_be_pad(x.clone(), 16).unwrap();
+        let p = uint_to_be_pad(x.clone(), 16);
         assert_eq!(&p[..14], &[0u8; 14]);
         assert_eq!(&p[14..], &[1, 2]);
 
-        let p = uint_to_zeroizing_be_pad(x.clone(), 3).unwrap();
+        let p = uint_to_zeroizing_be_pad(x.clone(), 3);
         assert_eq!(&p[..], &[0, 1, 2]);
 
-        let p = uint_to_be_pad(x.clone(), 8).unwrap();
+        let p = uint_to_be_pad(x.clone(), 8);
         assert_eq!(&p[..], &[0, 0, 0, 0, 0, 0, 1, 2]);
 
-        assert!(uint_to_be_pad(x.clone(), 1).is_err());
-        assert!(uint_to_zeroizing_be_pad(x, 1).is_err());
-
-        let p = uint_to_be_pad(BoxedUint::zero_with_precision(128), 4).unwrap();
+        let p = uint_to_be_pad(BoxedUint::zero_with_precision(128), 4);
         assert_eq!(&p[..], &[0u8; 4]);
     }
+
+    #[cfg(debug_assertions)]
+    #[test]
+    #[should_panic(expected = "integer exceeds output width")]
+    fn be_pad_rejects_broken_precondition() {
+        uint_to_zeroizing_be_pad(BoxedUint::from(0x0102u64), 1);
+    }
 }

src/oaep.rs

@@ -215,7 +215,7 @@ where
     let em = oaep_encrypt(rng, msg, digest, mgf_digest, label, pub_key.size())?;
 
     let int = BoxedUint::from_be_slice(&em, pub_key.n_bits_precision())?;
-    uint_to_be_pad(rsa_encrypt(pub_key, &int)?, pub_key.size())
+    Ok(uint_to_be_pad(rsa_encrypt(pub_key, &int)?, pub_key.size()))
 }
 
 /// Encrypts the given message with RSA and the padding scheme from
@@ -241,7 +241,7 @@ where
     let em = oaep_encrypt_digest::<_, D, MGD>(rng, msg, label, pub_key.size())?;
 
     let int = BoxedUint::from_be_slice(&em, pub_key.n_bits_precision())?;
-    uint_to_be_pad(rsa_encrypt(pub_key, &int)?, pub_key.size())
+    Ok(uint_to_be_pad(rsa_encrypt(pub_key, &int)?, pub_key.size()))
 }
 
 /// Decrypts a plaintext using RSA and the padding scheme from [PKCS#1 OAEP].
@@ -277,7 +277,7 @@ where
     let ciphertext = BoxedUint::from_be_slice(ciphertext, priv_key.n_bits_precision())?;
 
     let em = rsa_decrypt_and_check(priv_key, rng, &ciphertext)?;
-    let mut em = uint_to_zeroizing_be_pad(em, priv_key.size())?;
+    let mut em = uint_to_zeroizing_be_pad(em, priv_key.size());
 
     oaep_decrypt(&mut em, digest, mgf_digest, label, priv_key.size())
 }
@@ -314,7 +314,7 @@ where
 
     let ciphertext = BoxedUint::from_be_slice(ciphertext, priv_key.n_bits_precision())?;
     let em = rsa_decrypt_and_check(priv_key, rng, &ciphertext)?;
-    let mut em = uint_to_zeroizing_be_pad(em, priv_key.size())?;
+    let mut em = uint_to_zeroizing_be_pad(em, priv_key.size());
 
     oaep_decrypt_digest::<D, MGD>(&mut em, label, priv_key.size())
 }

src/pkcs1v15.rs

@@ -156,7 +156,7 @@ fn encrypt<R: TryCryptoRng + ?Sized>(
 
     let em = pkcs1v15_encrypt_pad(rng, msg, pub_key.size())?;
     let int = BoxedUint::from_be_slice(&em, pub_key.n_bits_precision())?;
-    uint_to_be_pad(rsa_encrypt(pub_key, &int)?, pub_key.size())
+    Ok(uint_to_be_pad(rsa_encrypt(pub_key, &int)?, pub_key.size()))
 }
 
 /// Decrypts a plaintext using RSA and the padding scheme from PKCS#1 v1.5.
@@ -177,13 +177,13 @@ fn decrypt<R: TryCryptoRng + ?Sized>(
     key::check_public(priv_key)?;
 
     // RFC 8017 § 7.2.2 step 1
-    if ciphertext.len() != priv_key.size() {
+    if priv_key.size() < 11 || ciphertext.len() != priv_key.size() {
         return Err(Error::Decryption);
     }
 
     let ciphertext = BoxedUint::from_be_slice(ciphertext, priv_key.n_bits_precision())?;
     let em = rsa_decrypt_and_check(priv_key, rng, &ciphertext)?;
-    let em = uint_to_zeroizing_be_pad(em, priv_key.size())?;
+    let em = uint_to_zeroizing_be_pad(em, priv_key.size());
 
     pkcs1v15_encrypt_unpad(&em, priv_key.size())
 }
@@ -212,7 +212,10 @@ fn sign<R: TryCryptoRng + ?Sized>(
 
     let em = BoxedUint::from_be_slice(&em, priv_key.n_bits_precision())?;
     // The signature itself is public; only the computation is secret
-    uint_to_be_pad(rsa_decrypt_and_check(priv_key, rng, &em)?, priv_key.size())
+    Ok(uint_to_be_pad(
+        rsa_decrypt_and_check(priv_key, rng, &em)?,
+        priv_key.size(),
+    ))
 }
 
 /// Verifies an RSA PKCS#1 v1.5 signature.
@@ -223,7 +226,7 @@ fn verify(pub_key: &RsaPublicKey, prefix: &[u8], hashed: &[u8], sig: &BoxedUint)
         return Err(Error::Verification);
     }
 
-    let em = uint_to_be_pad(rsa_encrypt(pub_key, sig)?, pub_key.size())?;
+    let em = uint_to_be_pad(rsa_encrypt(pub_key, sig)?, pub_key.size());
 
     pkcs1v15_sign_unpad(prefix, hashed, &em, pub_key.size())
 }
@@ -395,18 +398,25 @@ mod tests {
     #[test]
     fn test_decrypt_pkcs1v15_rejects_wrong_length() {
         let priv_key = get_private_key();
-        let ct = Base64::decode_vec(
-                "arReP9DJtEVyV2Dg3dDp4c/PSk1O6lxkoJ8HcFupoRorBZG+7+1fDAwT1olNddFnQMjmkb8vxwmNMoTAT/BFjQ==",
-            )
-            .unwrap();
-        assert!(priv_key.decrypt(Pkcs1v15Encrypt, &ct).is_ok());
+        // The leading zero is part of the valid k-byte encoding. Dropping it preserves
+        // the ciphertext integer, so only the length check can reject the short form.
+        let ct = hex!(
+            "00a40774c6e201e41714d1ab89c245116ff8f0baada675f22a50c7b6c1210e"
+            "3d2c3b7c595462aa8cb9447c43ef7d50ebcf544789b19ca5c2a26daea72cf9dbde"
+        );
+        assert_eq!(priv_key.decrypt(Pkcs1v15Encrypt, &ct).unwrap(), b"x");
 
-        // RFC 8017 § 7.2.2 step 1: a ciphertext shorter than `k`, even if numerically valid,
-        // must be rejected
-        assert!(priv_key.decrypt(Pkcs1v15Encrypt, &ct[1..]).is_err());
+        // RFC 8017 § 7.2.2 step 1: a ciphertext shorter than `k` must be rejected.
+        assert_eq!(
+            priv_key.decrypt(Pkcs1v15Encrypt, &ct[1..]),
+            Err(Error::Decryption)
+        );
         let mut long = vec![0u8];
         long.extend_from_slice(&ct);
-        assert!(priv_key.decrypt(Pkcs1v15Encrypt, &long).is_err());
+        assert_eq!(
+            priv_key.decrypt(Pkcs1v15Encrypt, &long),
+            Err(Error::Decryption)
+        );
     }
 
     #[test]

src/pss.rs

@@ -157,7 +157,7 @@ where
         return Err(Error::Verification);
     }
     let raw = rsa_encrypt(pub_key, sig)?;
-    let mut em = uint_to_be_pad(raw, pub_key.size())?;
+    let mut em = uint_to_be_pad(raw, pub_key.size());
 
     emsa_pss_verify(hashed, &mut em, salt_len, digest, pub_key.n().bits() as _)
 }
@@ -176,7 +176,7 @@ where
         return Err(Error::Verification);
     }
 
-    let mut em = uint_to_be_pad(rsa_encrypt(pub_key, sig)?, pub_key.size())?;
+    let mut em = uint_to_be_pad(rsa_encrypt(pub_key, sig)?, pub_key.size());
 
     emsa_pss_verify_digest::<D>(hashed, &mut em, salt_len, pub_key.n().bits() as _)
 }
@@ -243,7 +243,7 @@ where
 
     let em = BoxedUint::from_be_slice(&em, priv_key.n_bits_precision())?;
     let raw = rsa_decrypt_and_check(priv_key, blind_rng, &em)?;
-    uint_to_be_pad(raw, priv_key.size())
+    Ok(uint_to_be_pad(raw, priv_key.size()))
 }
 
 fn sign_pss_with_salt_digest<T, D>(
@@ -260,10 +260,10 @@ where
     let em = emsa_pss_encode_digest::<D>(hashed, em_bits as _, salt)?;
 
     let em = BoxedUint::from_be_slice(&em, priv_key.n_bits_precision())?;
-    uint_to_be_pad(
+    Ok(uint_to_be_pad(
         rsa_decrypt_and_check(priv_key, blind_rng, &em)?,
         priv_key.size(),
-    )
+    ))
 }
 
 /// Returns the [`AlgorithmIdentifierOwned`] associated with PSS signature using a given digest.