diff --git a/src/algorithms/pad.rs b/src/algorithms/pad.rs index d5c68bbc..4b73d591 100644 --- a/src/algorithms/pad.rs +++ b/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> { +pub(crate) fn uint_to_be_pad(input: BoxedUint, padded_len: usize) -> Vec { 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>> { +pub(crate) fn uint_to_zeroizing_be_pad(input: BoxedUint, padded_len: usize) -> Zeroizing> { 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); + } } diff --git a/src/oaep.rs b/src/oaep.rs index 96cd2ef9..9cfb00f9 100644 --- a/src/oaep.rs +++ b/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::(&mut em, label, priv_key.size()) } diff --git a/src/pkcs1v15.rs b/src/pkcs1v15.rs index 5ed03f82..150eb7ef 100644 --- a/src/pkcs1v15.rs +++ b/src/pkcs1v15.rs @@ -156,7 +156,7 @@ fn encrypt( 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( 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( 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] diff --git a/src/pss.rs b/src/pss.rs index 7073c540..a186e520 100644 --- a/src/pss.rs +++ b/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::(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( @@ -260,10 +260,10 @@ where let em = emsa_pss_encode_digest::(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.