diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f727bb9e479d..e4b674120644 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -22,6 +22,11 @@ Changelog :func:`~cryptography.hazmat.primitives.serialization.load_der_public_key`, and :meth:`~cryptography.x509.Certificate.public_key` when called on certificates with unsupported public key algorithms. +* **BACKWARDS INCOMPATIBLE:** When parsing elliptic curve private keys, we now + reject keys that incorrectly encode a private key of the wrong length because + such keys are impossible to process in a constant-time manner. We do not + believe keys with this problem are in wide use, however we may revert this + change based on the feedback we receive. * Updated the minimum supported Rust version (MSRV) to 1.83.0, from 1.74.0. * Added support for loading elliptic curve keys that contain explicit encodings of the curves ``secp256r1``, ``secp384r1``, and ``secp521r1``. diff --git a/docs/development/test-vectors.rst b/docs/development/test-vectors.rst index 5730c6432b87..75ff9680de67 100644 --- a/docs/development/test-vectors.rst +++ b/docs/development/test-vectors.rst @@ -295,8 +295,10 @@ Custom asymmetric vectors encrypted with a 20 byte salt with the password ``password``. * ``asymmetric/PKCS8/wrong-pem-delimiter-rsa.pem`` - A PKCS8 encoded RSA key with the wrong PEM delimiter. -* ``asymmetric/EC/high-bit-set.pem`` - A PKCS#1 encoded EC key the private key - value has its high bit set. +* ``asymmetric/EC/high-bit-set.pem`` - A PKCS#1 encoded EC key where the + private key value has its high bit set. +* ``asymmetric/EC/truncated-private-key.der`` - A PKCS#1 encoded EC key where + the private key value is not encoded to the order's length. Key exchange ~~~~~~~~~~~~ diff --git a/src/rust/cryptography-key-parsing/src/ec.rs b/src/rust/cryptography-key-parsing/src/ec.rs index f92f67ff029d..be0186657e7e 100644 --- a/src/rust/cryptography-key-parsing/src/ec.rs +++ b/src/rust/cryptography-key-parsing/src/ec.rs @@ -171,6 +171,10 @@ pub fn parse_pkcs1_private_key( (None, None) => return Err(crate::KeyParsingError::InvalidKey), }; + if ec_private_key.private_key.len() != group.order_bits().div_ceil(8).try_into().unwrap() { + return Err(crate::KeyParsingError::TruncatedEcPrivateKey); + } + 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 { diff --git a/src/rust/cryptography-key-parsing/src/lib.rs b/src/rust/cryptography-key-parsing/src/lib.rs index 7d19bebea39e..77191745c563 100644 --- a/src/rust/cryptography-key-parsing/src/lib.rs +++ b/src/rust/cryptography-key-parsing/src/lib.rs @@ -27,6 +27,7 @@ pub enum KeyParsingError { UnsupportedEncryptionAlgorithm(asn1::ObjectIdentifier), EncryptedKeyWithoutPassword, IncorrectPassword, + TruncatedEcPrivateKey, // PEM encryption errors PemMissingDekInfo, PemInvalidDekInfo, diff --git a/src/rust/src/error.rs b/src/rust/src/error.rs index 868cd5aadeda..dccf10943481 100644 --- a/src/rust/src/error.rs +++ b/src/rust/src/error.rs @@ -103,6 +103,11 @@ impl From for CryptographyError { "Incorrect password, could not decrypt key", )) } + cryptography_key_parsing::KeyParsingError::TruncatedEcPrivateKey => { + CryptographyError::Py(pyo3::exceptions::PyValueError::new_err( + "EC private key is not encoded properly: private key value is too short. Please file an issue at https://github.com/pyca/cryptography/issues explaining how your private key was created.", + )) + } cryptography_key_parsing::KeyParsingError::PemMissingDekInfo => { CryptographyError::Py(pyo3::exceptions::PyValueError::new_err( "Encrypted PEM doesn't have a DEK-Info header.", diff --git a/tests/hazmat/primitives/test_ec.py b/tests/hazmat/primitives/test_ec.py index a11cd64a8b89..40d19ac1ec43 100644 --- a/tests/hazmat/primitives/test_ec.py +++ b/tests/hazmat/primitives/test_ec.py @@ -1274,6 +1274,16 @@ def test_private_bytes_small_key(self): # length. assert (b"\x00" * 31 + b"\x01") in der + def test_load_private_key_short_key_warngs(self): + data = load_vectors_from_file( + os.path.join("asymmetric", "EC", "truncated-private-key.der"), + lambda f: f.read(), + mode="rb", + ) + + with pytest.raises(ValueError, match="private key value is too short"): + serialization.load_der_private_key(data, password=None) + class TestEllipticCurvePEMPublicKeySerialization: @pytest.mark.parametrize( diff --git a/vectors/cryptography_vectors/asymmetric/EC/truncated-private-key.der b/vectors/cryptography_vectors/asymmetric/EC/truncated-private-key.der new file mode 100644 index 000000000000..2a1731d4a939 Binary files /dev/null and b/vectors/cryptography_vectors/asymmetric/EC/truncated-private-key.der differ