Harden provider contract enforcement

Author: thieman

src/aead.rs

@@ -12,6 +12,7 @@ use windows::Win32::Security::Cryptography::{
     BCRYPT_AUTHENTICATED_CIPHER_MODE_INFO_VERSION, BCRYPT_CHACHA20_POLY1305_ALG_HANDLE,
     BCRYPT_FLAGS, BCRYPT_KEY_HANDLE,
 };
+use zeroize::Zeroizing;
 
 /// The tag length is 16 bytes for all supported ciphers.
 pub(crate) const TAG_LEN: usize = 16;
@@ -113,14 +114,13 @@ impl AeadKey {
             ..Default::default()
         };
 
+        let input = Zeroizing::new(data.to_vec());
         unsafe {
-            // SAFETY: CNG supports in-place encryption, so the input and output buffers can be the same.
             let mut size = 0u32;
-            let input = std::slice::from_raw_parts(data.as_ptr().cast(), data.len());
 
             BCryptEncrypt(
                 *self.handle,
-                Some(input),
+                Some(input.as_slice()),
                 Some(std::ptr::from_ref(&info) as *mut _),
                 None,
                 Some(data),
@@ -161,14 +161,11 @@ impl AeadKey {
 
         let mut size = 0u32;
 
+        let input = ciphertext.to_vec();
         unsafe {
-            // SAFETY: CNG supports in-place decryption, so the input and output buffers can be the same.
-
-            let input = std::slice::from_raw_parts(ciphertext.as_ptr().cast(), ciphertext.len());
-
             BCryptDecrypt(
                 *self.handle,
-                Some(input),
+                Some(&input),
                 Some(std::ptr::from_ref(&info) as *mut _),
                 None,
                 Some(ciphertext),

src/keys.rs

@@ -16,6 +16,7 @@ use windows::{
         BCRYPT_RSAPUBLIC_MAGIC, BCRYPT_RSA_ALG_HANDLE,
     },
 };
+use zeroize::Zeroizing;
 
 /// Wrapper for an owned key handle that can be sent between threads.
 #[derive(Debug)]
@@ -47,7 +48,7 @@ pub(crate) fn import_rsa_private_key(
         + prime1.len()
         + prime2.len();
 
-    let mut blob = Vec::with_capacity(size);
+    let mut blob = Zeroizing::new(Vec::with_capacity(size));
     unsafe {
         let p: *const BCRYPT_RSAKEY_BLOB = &header;
         let p: *const u8 = p.cast::<u8>();
@@ -67,7 +68,7 @@ pub(crate) fn import_rsa_private_key(
             None,
             BCRYPT_RSAPRIVATE_BLOB,
             &mut *key_handle,
-            &blob,
+            blob.as_slice(),
             0,
         )
         .ok()
@@ -146,14 +147,14 @@ fn import_ec_private_key(
         cbKey: key_len as u32,
     };
     let header_size = core::mem::size_of::<BCRYPT_ECCKEY_BLOB>();
-    let mut blob = Vec::with_capacity(header_size + key_len * 3);
+    let mut blob = Zeroizing::new(Vec::with_capacity(header_size + key_len * 3));
     unsafe {
         let p: *const BCRYPT_ECCKEY_BLOB = &header;
         let p: *const u8 = p.cast::<u8>();
         let slice = std::slice::from_raw_parts(p, header_size);
         blob.extend_from_slice(slice);
     }
-    blob.extend_from_slice(&vec![0u8; key_len * 2]);
+    blob.extend(std::iter::repeat_n(0, key_len * 2));
     blob.extend_from_slice(private_key);
     let mut key_handle = Owned::default();
     unsafe {
@@ -162,7 +163,7 @@ fn import_ec_private_key(
             None,
             BCRYPT_ECCPRIVATE_BLOB,
             &mut *key_handle,
-            &blob,
+            blob.as_slice(),
             0,
         )
         .ok()

src/tls12.rs

@@ -20,9 +20,17 @@ use rustls::{
 
 const GCM_EXPLICIT_NONCE_LENGTH: usize = 8;
 const GCM_IMPLICIT_NONCE_LENGTH: usize = 4;
+const MAX_TLS12_PLAINTEXT_FRAGMENT_LEN: usize = 16_384;
+
+fn reject_oversized_tls12_plaintext(plaintext_len: usize) -> Result<(), Error> {
+    if plaintext_len > MAX_TLS12_PLAINTEXT_FRAGMENT_LEN {
+        return Err(Error::PeerSentOversizedRecord);
+    }
+
+    Ok(())
+}
 
 static ECDSA_SCHEMES: &[SignatureScheme] = &[
-    SignatureScheme::ED25519,
     SignatureScheme::ECDSA_NISTP521_SHA512,
     SignatureScheme::ECDSA_NISTP384_SHA384,
     SignatureScheme::ECDSA_NISTP256_SHA256,
@@ -282,6 +290,7 @@ impl MessageDecrypter for AesGcmDecrypter {
             &aad,
             &mut payload.as_mut()[GCM_EXPLICIT_NONCE_LENGTH..],
         )?;
+        reject_oversized_tls12_plaintext(plaintext_len)?;
 
         // Remove the explicit nonce from the front of the buffer, as it's not part of the plaintext.
         payload.copy_within(
@@ -335,7 +344,34 @@ impl MessageDecrypter for ChaCha20Poly1305Crypter {
         tag.copy_from_slice(&payload[message_len..]);
 
         let plaintext_len = self.key.open(nonce.0, &aad, payload)?;
+        reject_oversized_tls12_plaintext(plaintext_len)?;
         payload.truncate(plaintext_len);
         Ok(msg.into_plain_message())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn tls12_ecdsa_sign_schemes_do_not_advertise_ed25519() {
+        assert!(!ECDSA_SCHEMES.contains(&SignatureScheme::ED25519));
+    }
+
+    #[test]
+    fn aes_gcm_decrypter_rejects_oversized_plaintext() {
+        assert!(matches!(
+            reject_oversized_tls12_plaintext(MAX_TLS12_PLAINTEXT_FRAGMENT_LEN + 1),
+            Err(Error::PeerSentOversizedRecord)
+        ));
+    }
+
+    #[test]
+    fn chacha20_poly1305_decrypter_rejects_oversized_plaintext() {
+        assert!(matches!(
+            reject_oversized_tls12_plaintext(MAX_TLS12_PLAINTEXT_FRAGMENT_LEN + 1),
+            Err(Error::PeerSentOversizedRecord)
+        ));
+    }
+}

src/verify.rs

@@ -36,8 +36,11 @@ pub static SUPPORTED_SIG_ALGS: WebPkiSupportedAlgorithms = WebPkiSupportedAlgori
         RSA_PSS_SHA384,
         RSA_PSS_SHA256,
         RSA_PKCS1_SHA512,
+        RSA_PKCS1_SHA512_ABSENT_PARAMS,
         RSA_PKCS1_SHA384,
+        RSA_PKCS1_SHA384_ABSENT_PARAMS,
         RSA_PKCS1_SHA256,
+        RSA_PKCS1_SHA256_ABSENT_PARAMS,
     ],
     mapping: &[
         //Note: for TLS1.2 the curve is not fixed by SignatureScheme. For TLS1.3 it is.
@@ -69,6 +72,18 @@ pub(crate) static RSA_PKCS1_SHA256: &dyn SignatureVerificationAlgorithm = &Verif
     params: Params::Rsa(RsaPadding::PKCS1),
 };
 
+/// RSA PKCS#1 1.5 signatures using SHA-256 with absent AlgorithmIdentifier parameters.
+pub(crate) static RSA_PKCS1_SHA256_ABSENT_PARAMS: &dyn SignatureVerificationAlgorithm =
+    &VerificationAlgorithm {
+        display_name: "RSA_PKCS1_SHA256_ABSENT_PARAMS",
+        public_key_alg_id: alg_id::RSA_ENCRYPTION,
+        signature_alg_id: AlgorithmIdentifier::from_slice(&[
+            0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b,
+        ]),
+        hash: SHA256,
+        params: Params::Rsa(RsaPadding::PKCS1),
+    };
+
 /// RSA PKCS#1 1.5 signatures using SHA-384.
 pub(crate) static RSA_PKCS1_SHA384: &dyn SignatureVerificationAlgorithm = &VerificationAlgorithm {
     display_name: "RSA_PKCS1_SHA384",
@@ -78,6 +93,18 @@ pub(crate) static RSA_PKCS1_SHA384: &dyn SignatureVerificationAlgorithm = &Verif
     params: Params::Rsa(RsaPadding::PKCS1),
 };
 
+/// RSA PKCS#1 1.5 signatures using SHA-384 with absent AlgorithmIdentifier parameters.
+pub(crate) static RSA_PKCS1_SHA384_ABSENT_PARAMS: &dyn SignatureVerificationAlgorithm =
+    &VerificationAlgorithm {
+        display_name: "RSA_PKCS1_SHA384_ABSENT_PARAMS",
+        public_key_alg_id: alg_id::RSA_ENCRYPTION,
+        signature_alg_id: AlgorithmIdentifier::from_slice(&[
+            0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0c,
+        ]),
+        hash: SHA384,
+        params: Params::Rsa(RsaPadding::PKCS1),
+    };
+
 /// RSA PKCS#1 1.5 signatures using SHA-512.
 pub(crate) static RSA_PKCS1_SHA512: &dyn SignatureVerificationAlgorithm = &VerificationAlgorithm {
     display_name: "RSA_PKCS1_SHA512",
@@ -87,6 +114,18 @@ pub(crate) static RSA_PKCS1_SHA512: &dyn SignatureVerificationAlgorithm = &Verif
     params: Params::Rsa(RsaPadding::PKCS1),
 };
 
+/// RSA PKCS#1 1.5 signatures using SHA-512 with absent AlgorithmIdentifier parameters.
+pub(crate) static RSA_PKCS1_SHA512_ABSENT_PARAMS: &dyn SignatureVerificationAlgorithm =
+    &VerificationAlgorithm {
+        display_name: "RSA_PKCS1_SHA512_ABSENT_PARAMS",
+        public_key_alg_id: alg_id::RSA_ENCRYPTION,
+        signature_alg_id: AlgorithmIdentifier::from_slice(&[
+            0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0d,
+        ]),
+        hash: SHA512,
+        params: Params::Rsa(RsaPadding::PKCS1),
+    };
+
 /// RSA PSS signatures using SHA-256.
 pub(crate) static RSA_PSS_SHA256: &dyn SignatureVerificationAlgorithm = &VerificationAlgorithm {
     display_name: "RSA_PSS_SHA256",
@@ -210,6 +249,22 @@ enum Params {
 unsafe impl Send for Params {}
 unsafe impl Sync for Params {}
 
+const RSA_MIN_MODULUS_BITS: usize = 2048;
+const RSA_MAX_MODULUS_BITS: usize = 8192;
+
+fn rsa_public_key_allowed_by_webpki(key: &RsaPublicKey<'_>) -> bool {
+    (RSA_MIN_MODULUS_BITS..=RSA_MAX_MODULUS_BITS)
+        .contains(&rsa_modulus_bit_len(key.modulus.as_bytes()))
+}
+
+fn rsa_modulus_bit_len(modulus: &[u8]) -> usize {
+    let Some(first) = modulus.first() else {
+        return 0;
+    };
+
+    (modulus.len() - 1) * 8 + (u8::BITS as usize - first.leading_zeros() as usize)
+}
+
 #[derive(Debug)]
 enum RsaPadding {
     PKCS1,
@@ -236,6 +291,9 @@ impl<const HASH_SIZE: usize> SignatureVerificationAlgorithm for VerificationAlgo
         match &self.params {
             Params::Rsa(padding) => {
                 let key = RsaPublicKey::try_from(public_key).map_err(|_| InvalidSignature)?;
+                if !rsa_public_key_allowed_by_webpki(&key) {
+                    return Err(InvalidSignature);
+                }
                 let handle = import_rsa_public_key(&key).map_err(|_| InvalidSignature)?;
 
                 match padding {
@@ -269,10 +327,7 @@ impl<const HASH_SIZE: usize> SignatureVerificationAlgorithm for VerificationAlgo
                                 BCRYPT_PAD_PSS,
                             )
                             .ok()
-                            .map_err(|e| {
-                                dbg!(e);
-                                InvalidSignature
-                            })
+                            .map_err(|_| InvalidSignature)
                         }
                     }
                 }
@@ -343,6 +398,88 @@ mod tests {
     use super::*;
     use wycheproof::TestResult;
 
+    const RSA_PKCS1_SHA256_ABSENT_PARAMS_DER: &[u8] = &[
+        0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b,
+    ];
+    const RSA_PKCS1_SHA384_ABSENT_PARAMS_DER: &[u8] = &[
+        0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0c,
+    ];
+    const RSA_PKCS1_SHA512_ABSENT_PARAMS_DER: &[u8] = &[
+        0x30, 0x0b, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0d,
+    ];
+
+    #[test]
+    fn supported_algorithms_include_rsa_pkcs1_absent_parameter_variants() {
+        for signature_alg_id in [
+            AlgorithmIdentifier::from_slice(RSA_PKCS1_SHA256_ABSENT_PARAMS_DER),
+            AlgorithmIdentifier::from_slice(RSA_PKCS1_SHA384_ABSENT_PARAMS_DER),
+            AlgorithmIdentifier::from_slice(RSA_PKCS1_SHA512_ABSENT_PARAMS_DER),
+        ] {
+            assert!(SUPPORTED_SIG_ALGS
+                .all
+                .iter()
+                .any(|alg| alg.signature_alg_id() == signature_alg_id));
+        }
+    }
+
+    #[test]
+    fn rsa_public_key_policy_matches_webpki_2048_to_8192_bit_bounds() {
+        let key_2047 = rsa_public_key_with_modulus(&modulus_with_bit_len(2047));
+        let key_2048 = rsa_public_key_with_modulus(&modulus_with_bit_len(2048));
+        let key_8192 = rsa_public_key_with_modulus(&modulus_with_bit_len(8192));
+        let key_8193 = rsa_public_key_with_modulus(&modulus_with_bit_len(8193));
+
+        assert!(!rsa_public_key_allowed_by_webpki(&key_2047));
+        assert!(rsa_public_key_allowed_by_webpki(&key_2048));
+        assert!(rsa_public_key_allowed_by_webpki(&key_8192));
+        assert!(!rsa_public_key_allowed_by_webpki(&key_8193));
+    }
+
+    fn rsa_public_key_with_modulus(modulus: &[u8]) -> RsaPublicKey<'static> {
+        let mut der = Vec::new();
+        append_der_integer(&mut der, modulus);
+        append_der_integer(&mut der, &[0x01, 0x00, 0x01]);
+
+        let mut sequence = Vec::new();
+        sequence.push(0x30);
+        append_der_len(&mut sequence, der.len());
+        sequence.extend_from_slice(&der);
+
+        RsaPublicKey::try_from(Box::leak(sequence.into_boxed_slice()).as_ref()).unwrap()
+    }
+
+    fn modulus_with_bit_len(bit_len: usize) -> Vec<u8> {
+        let len = bit_len.div_ceil(8);
+        let mut modulus = vec![0xff; len];
+        modulus[0] = 1 << ((bit_len - 1) % 8);
+        modulus
+    }
+
+    fn append_der_integer(der: &mut Vec<u8>, value: &[u8]) {
+        der.push(0x02);
+        let needs_leading_zero = value.first().is_some_and(|byte| byte & 0x80 != 0);
+        append_der_len(der, value.len() + usize::from(needs_leading_zero));
+        if needs_leading_zero {
+            der.push(0);
+        }
+        der.extend_from_slice(value);
+    }
+
+    fn append_der_len(der: &mut Vec<u8>, len: usize) {
+        if len < 128 {
+            der.push(len as u8);
+            return;
+        }
+
+        let len_bytes = len.to_be_bytes();
+        let first = len_bytes
+            .iter()
+            .position(|byte| *byte != 0)
+            .unwrap_or(len_bytes.len() - 1);
+        der.push(0x80 | (len_bytes.len() - first) as u8);
+        der.extend_from_slice(&len_bytes[first..]);
+    }
+
     #[test]
     fn test_open_ssl_algorithm_debug() {
         assert_eq!(