fixes #13672 -- emit a warning when loading an EC key that's not encoded properly

Author: alex

docs/development/test-vectors.rst

@@ -294,8 +294,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
 ~~~~~~~~~~~~

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

@@ -2,6 +2,8 @@
 // 2.0, and the BSD License. See the LICENSE file in the root of this repository
 // for complete details.
 
+use std::ffi::CStr;
+
 use cryptography_x509::common::EcParameters;
 use cryptography_x509::ec_constants;
 
@@ -153,6 +155,7 @@ pub fn serialize_pkcs1_private_key(
 pub fn parse_pkcs1_private_key(
     data: &[u8],
     ec_params: Option<EcParameters<'_>>,
+    warn: impl Fn(&CStr),
 ) -> KeyParsingResult<openssl::pkey::PKey<openssl::pkey::Private>> {
     let ec_private_key = asn1::parse_single::<EcPrivateKey<'_>>(data)?;
     if ec_private_key.version != 1 {
@@ -171,6 +174,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() {
+        warn(c"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. In the future this may raise an error.");
+    }
+
     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 {

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

@@ -2,6 +2,8 @@
 // 2.0, and the BSD License. See the LICENSE file in the root of this repository
 // for complete details.
 
+use std::ffi::CStr;
+
 use cryptography_x509::common::{
     AlgorithmIdentifier, AlgorithmParameters, PbeParams, Pkcs12PbeParams,
 };
@@ -24,6 +26,7 @@ pub struct PrivateKeyInfo<'a> {
 
 pub fn parse_private_key(
     data: &[u8],
+    warn: impl Fn(&CStr),
 ) -> KeyParsingResult<openssl::pkey::PKey<openssl::pkey::Private>> {
     let k = asn1::parse_single::<PrivateKeyInfo<'_>>(data)?;
     if k.version != 0 {
@@ -34,7 +37,7 @@ pub fn parse_private_key(
             rsa::parse_pkcs1_private_key(k.private_key)
         }
         AlgorithmParameters::Ec(ec_params) => {
-            ec::parse_pkcs1_private_key(k.private_key, Some(ec_params))
+            ec::parse_pkcs1_private_key(k.private_key, Some(ec_params), warn)
         }
 
         AlgorithmParameters::Dsa(dsa_params) => {
@@ -192,6 +195,7 @@ fn pkcs5_pbe_decrypt(
 pub fn parse_encrypted_private_key(
     data: &[u8],
     password: Option<&[u8]>,
+    warn: impl Fn(&CStr),
 ) -> KeyParsingResult<openssl::pkey::PKey<openssl::pkey::Private>> {
     let epki = asn1::parse_single::<EncryptedPrivateKeyInfo<'_>>(data)?;
     let password = match password {
@@ -329,7 +333,7 @@ pub fn parse_encrypted_private_key(
         }
     };
 
-    parse_private_key(&plaintext)
+    parse_private_key(&plaintext, warn)
 }
 
 pub fn serialize_private_key(

src/rust/src/backend/keys.rs

@@ -2,7 +2,8 @@
 // 2.0, and the BSD License. See the LICENSE file in the root of this repository
 // for complete details.
 
-use pyo3::IntoPyObject;
+use pyo3::{IntoPyObject, PyTypeInfo};
+use std::ffi::CStr;
 
 use crate::buf::CffiBuf;
 use crate::error::{CryptographyError, CryptographyResult};
@@ -27,14 +28,22 @@ fn load_der_private_key<'p>(
     )
 }
 
+fn py_warn(py: pyo3::Python<'_>) -> impl Fn(&CStr) + '_ {
+    move |msg| {
+        let warning_cls = pyo3::exceptions::PyUserWarning::type_object(py);
+        // If warn fails, we ignore it.
+        _ = pyo3::PyErr::warn(py, &warning_cls, msg, 1);
+    }
+}
+
 pub(crate) fn load_der_private_key_bytes<'p>(
     py: pyo3::Python<'p>,
     data: &[u8],
     password: Option<&[u8]>,
     unsafe_skip_rsa_key_validation: bool,
 ) -> CryptographyResult<pyo3::Bound<'p, pyo3::PyAny>> {
-    let pkey = cryptography_key_parsing::pkcs8::parse_private_key(data)
-        .or_else(|_| cryptography_key_parsing::ec::parse_pkcs1_private_key(data, None))
+    let pkey = cryptography_key_parsing::pkcs8::parse_private_key(data, py_warn(py))
+        .or_else(|_| cryptography_key_parsing::ec::parse_pkcs1_private_key(data, None, py_warn(py)))
         .or_else(|_| cryptography_key_parsing::rsa::parse_pkcs1_private_key(data))
         .or_else(|_| cryptography_key_parsing::dsa::parse_pkcs1_private_key(data));
 
@@ -49,7 +58,8 @@ pub(crate) fn load_der_private_key_bytes<'p>(
         return private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation);
     }
 
-    let pkey = cryptography_key_parsing::pkcs8::parse_encrypted_private_key(data, password)?;
+    let pkey =
+        cryptography_key_parsing::pkcs8::parse_encrypted_private_key(data, password, py_warn(py))?;
 
     private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation)
 }
@@ -74,11 +84,11 @@ fn load_pem_private_key<'p>(
     let (data, mut password_used) = cryptography_key_parsing::pem::decrypt_pem(&p, password)?;
 
     let pkey = match p.tag() {
-        "PRIVATE KEY" => cryptography_key_parsing::pkcs8::parse_private_key(&data)?,
+        "PRIVATE KEY" => cryptography_key_parsing::pkcs8::parse_private_key(&data, py_warn(py))?,
         "RSA PRIVATE KEY" => cryptography_key_parsing::rsa::parse_pkcs1_private_key(&data).map_err(|e| {
             CryptographyError::from(e).add_note(py, "If your key is in PKCS#8 format, you must use BEGIN/END PRIVATE KEY PEM delimiters")
         })?,
-        "EC PRIVATE KEY" => cryptography_key_parsing::ec::parse_pkcs1_private_key(&data, None).map_err(|e| {
+        "EC PRIVATE KEY" => cryptography_key_parsing::ec::parse_pkcs1_private_key(&data, None, py_warn(py)).map_err(|e| {
             CryptographyError::from(e).add_note(py, "If your key is in PKCS#8 format, you must use BEGIN/END PRIVATE KEY PEM delimiters")
         })?,
         "DSA PRIVATE KEY" => cryptography_key_parsing::dsa::parse_pkcs1_private_key(&data).map_err(|e| {
@@ -87,7 +97,7 @@ fn load_pem_private_key<'p>(
         _ => {
             assert_eq!(p.tag(), "ENCRYPTED PRIVATE KEY");
             password_used = true;
-            cryptography_key_parsing::pkcs8::parse_encrypted_private_key(&data, password)?
+            cryptography_key_parsing::pkcs8::parse_encrypted_private_key(&data, password, py_warn(py))?
         }
     };
     if password.is_some() && !password_used {

tests/hazmat/primitives/test_ec.py

@@ -1248,6 +1248,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.warns(UserWarning, match="private key value is too short"):
+            serialization.load_der_private_key(data, password=None)
+
 
 class TestEllipticCurvePEMPublicKeySerialization:
     @pytest.mark.parametrize(