reject non-zero unused bits in public key bit strings

CHANGELOG.rst

@@ -13,6 +13,9 @@ Changelog
   silently ignoring them.
 * Added support for using :class:`~cryptography.x509.Name` as a field type in
   the :doc:`/hazmat/asn1/index` module.
+* Loading a public key or an EC private key now rejects DER where the
+  ``subjectPublicKey`` (or EC ``publicKey``) ``BIT STRING`` declares a non-zero
+  number of unused bits, instead of silently ignoring it.
 
 .. _v49-0-0:
 

src/rust/cryptography-key-parsing/src/ec.rs

@@ -155,6 +155,11 @@ pub fn parse_pkcs1_private_key(
     let private_number = openssl::bn::BigNum::from_slice(ec_private_key.private_key)?;
     let mut bn_ctx = openssl::bn::BigNumContext::new()?;
     let public_point = if let Some(point_bytes) = ec_private_key.public_key {
+        // The publicKey BIT STRING holds an octet-aligned EC point, so a
+        // non-zero unused-bits count is a malformed encoding.
+        if point_bytes.padding_bits() != 0 {
+            return Err(crate::KeyParsingError::InvalidKey);
+        }
         openssl::ec::EcPoint::from_bytes(&group, point_bytes.as_bytes(), &mut bn_ctx)
             .map_err(|_| crate::KeyParsingError::InvalidKey)?
     } else {

src/rust/cryptography-key-parsing/src/spki.rs

@@ -12,6 +12,13 @@ use crate::{KeyParsingError, KeyParsingResult, KeySerializationResult, ParsedPub
 pub fn parse_public_key(data: &[u8]) -> KeyParsingResult<ParsedPublicKey> {
     let k = asn1::parse_single::<SubjectPublicKeyInfo<'_>>(data)?;
 
+    // The subjectPublicKey BIT STRING wraps whole octets for every key type
+    // we support, so a non-zero unused-bits count is a malformed encoding.
+    // `parse_spki_for_data` already rejects this; do the same here.
+    if k.subject_public_key.padding_bits() != 0 {
+        return Err(KeyParsingError::InvalidKey);
+    }
+
     match k.algorithm.params {
         AlgorithmParameters::Ec(ec_params) => {
             let group = crate::ec::ec_params_to_group(&ec_params)?;

tests/hazmat/primitives/test_serialization.py

@@ -362,6 +362,48 @@ def test_load_der_invalid_public_key(self, backend):
         with pytest.raises(ValueError):
             load_der_public_key(b"invalid data", backend)
 
+    def test_load_der_public_key_bit_string_padding(self, backend):
+        # The subjectPublicKey BIT STRING must use 0 unused bits. Take a valid
+        # Ed25519 SPKI and bump the unused-bits count; the last data bit is 0
+        # so it is still a well-formed BIT STRING, just an invalid encoding for
+        # a public key.
+        for _ in range(2000):
+            pub = ed25519.Ed25519PrivateKey.generate().public_key()
+            spki = pub.public_bytes(
+                Encoding.DER, PublicFormat.SubjectPublicKeyInfo
+            )
+            if spki[-1] & 0x07 == 0:
+                break
+        else:
+            pytest.fail("could not generate a suitable key")
+
+        data = bytearray(spki)
+        idx = data.rfind(b"\x03\x21\x00")
+        assert idx != -1
+        data[idx + 2] = 0x01
+
+        with pytest.raises(ValueError):
+            load_der_public_key(bytes(data), backend)
+
+    def test_load_der_ec_private_key_bit_string_padding(self, backend):
+        _skip_curve_unsupported(backend, ec.SECP256R1())
+        # The publicKey BIT STRING in an EC private key must use 0 unused bits.
+        for _ in range(3000):
+            der = ec.generate_private_key(ec.SECP256R1()).private_bytes(
+                Encoding.DER, PrivateFormat.TraditionalOpenSSL, NoEncryption()
+            )
+            idx = der.find(b"\x03\x42\x00\x04")
+            if idx != -1 and der[-1] & 0x07 == 0:
+                break
+        else:
+            pytest.fail("could not generate a suitable key")
+
+        data = bytearray(der)
+        data[idx + 2] = 0x01
+
+        with pytest.raises(ValueError):
+            load_der_private_key(bytes(data), password=None, backend=backend)
+
     @pytest.mark.supported(
         only_if=lambda backend: backend.dsa_supported(),
         skip_message="Does not support DSA.",