Skip to content

Commit 3865631

Browse files
committed
fixes #13672 -- reject key when loading an EC key that's not encoded properly
1 parent db3d1ef commit 3865631

File tree

7 files changed

+29
-2
lines changed

7 files changed

+29
-2
lines changed

CHANGELOG.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ Changelog
2222
:func:`~cryptography.hazmat.primitives.serialization.load_der_public_key`,
2323
and :meth:`~cryptography.x509.Certificate.public_key` when called on
2424
certificates with unsupported public key algorithms.
25+
* **BACKWARDS INCOMPATIBLE:** When parsing elliptic curve private keys, we now
26+
reject keys that incorrectly encode a private key of the wrong length because
27+
such keys are impossible to process in a constant-time manner. We do not
28+
believe keys with this problem are in wide use, however we may revert this
29+
change based on the feedback we reiceve.
2530
* Updated the minimum supported Rust version (MSRV) to 1.83.0, from 1.74.0.
2631
* Added support for loading elliptic curve keys that contain explicit encodings
2732
of the curves ``secp256r1``, ``secp384r1``, and ``secp521r1``.

docs/development/test-vectors.rst

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,10 @@ Custom asymmetric vectors
295295
encrypted with a 20 byte salt with the password ``password``.
296296
* ``asymmetric/PKCS8/wrong-pem-delimiter-rsa.pem`` - A PKCS8 encoded RSA key
297297
with the wrong PEM delimiter.
298-
* ``asymmetric/EC/high-bit-set.pem`` - A PKCS#1 encoded EC key the private key
299-
value has its high bit set.
298+
* ``asymmetric/EC/high-bit-set.pem`` - A PKCS#1 encoded EC key where the
299+
private key value has its high bit set.
300+
* ``asymmetric/EC/truncated-private-key.der`` - A PKCS#1 encoded EC key where
301+
the private key value is not encoded to the order's length.
300302

301303
Key exchange
302304
~~~~~~~~~~~~

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,10 @@ pub fn parse_pkcs1_private_key(
171171
(None, None) => return Err(crate::KeyParsingError::InvalidKey),
172172
};
173173

174+
if ec_private_key.private_key.len() != group.order_bits().div_ceil(8).try_into().unwrap() {
175+
return Err(crate::KeyParsingError::TruncatedEcPrivateKey);
176+
}
177+
174178
let private_number = openssl::bn::BigNum::from_slice(ec_private_key.private_key)?;
175179
let mut bn_ctx = openssl::bn::BigNumContext::new()?;
176180
let public_point = if let Some(point_bytes) = ec_private_key.public_key {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ pub enum KeyParsingError {
2727
UnsupportedEncryptionAlgorithm(asn1::ObjectIdentifier),
2828
EncryptedKeyWithoutPassword,
2929
IncorrectPassword,
30+
TruncatedEcPrivateKey,
3031
// PEM encryption errors
3132
PemMissingDekInfo,
3233
PemInvalidDekInfo,

src/rust/src/error.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ impl From<cryptography_key_parsing::KeyParsingError> for CryptographyError {
103103
"Incorrect password, could not decrypt key",
104104
))
105105
}
106+
cryptography_key_parsing::KeyParsingError::TruncatedEcPrivateKey => {
107+
CryptographyError::Py(pyo3::exceptions::PyValueError::new_err(
108+
"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.",
109+
))
110+
}
106111
cryptography_key_parsing::KeyParsingError::PemMissingDekInfo => {
107112
CryptographyError::Py(pyo3::exceptions::PyValueError::new_err(
108113
"Encrypted PEM doesn't have a DEK-Info header.",

tests/hazmat/primitives/test_ec.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1274,6 +1274,16 @@ def test_private_bytes_small_key(self):
12741274
# length.
12751275
assert (b"\x00" * 31 + b"\x01") in der
12761276

1277+
def test_load_private_key_short_key_warngs(self):
1278+
data = load_vectors_from_file(
1279+
os.path.join("asymmetric", "EC", "truncated-private-key.der"),
1280+
lambda f: f.read(),
1281+
mode="rb",
1282+
)
1283+
1284+
with pytest.raises(ValueError, match="private key value is too short"):
1285+
serialization.load_der_private_key(data, password=None)
1286+
12771287

12781288
class TestEllipticCurvePEMPublicKeySerialization:
12791289
@pytest.mark.parametrize(
Binary file not shown.

0 commit comments

Comments
 (0)