From 9cc4b7acc407e2aa1bcea3b36a7ab80a97f39e56 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 15 Jan 2025 16:40:18 -0500 Subject: [PATCH 01/46] Migrate parsing of unencrypted PKCS#8 private keys to Rust --- src/rust/cryptography-key-parsing/src/dsa.rs | 32 ++++++ src/rust/cryptography-key-parsing/src/ec.rs | 51 +++++++++ src/rust/cryptography-key-parsing/src/lib.rs | 4 +- .../cryptography-key-parsing/src/pkcs8.rs | 100 ++++++++++++++++++ src/rust/cryptography-key-parsing/src/rsa.rs | 36 +++++++ src/rust/src/backend/keys.rs | 13 ++- tests/hazmat/backends/test_openssl.py | 4 - 7 files changed, 234 insertions(+), 6 deletions(-) create mode 100644 src/rust/cryptography-key-parsing/src/dsa.rs create mode 100644 src/rust/cryptography-key-parsing/src/pkcs8.rs diff --git a/src/rust/cryptography-key-parsing/src/dsa.rs b/src/rust/cryptography-key-parsing/src/dsa.rs new file mode 100644 index 000000000000..6391a31991fd --- /dev/null +++ b/src/rust/cryptography-key-parsing/src/dsa.rs @@ -0,0 +1,32 @@ +// This file is dual licensed under the terms of the Apache License, Version +// 2.0, and the BSD License. See the LICENSE file in the root of this repository +// for complete details. + +use crate::KeyParsingResult; + +#[derive(asn1::Asn1Read)] +struct DsaPrivateKey<'a> { + version: u8, + p: asn1::BigUint<'a>, + q: asn1::BigUint<'a>, + g: asn1::BigUint<'a>, + pub_key: asn1::BigUint<'a>, + priv_key: asn1::BigUint<'a>, +} + +pub fn parse_pkcs1_private_key( + data: &[u8], +) -> KeyParsingResult> { + let dsa_private_key = asn1::parse_single::>(data)?; + if dsa_private_key.version != 0 { + return Err(crate::KeyParsingError::InvalidKey); + } + let dsa = openssl::dsa::Dsa::from_private_components( + openssl::bn::BigNum::from_slice(dsa_private_key.p.as_bytes())?, + openssl::bn::BigNum::from_slice(dsa_private_key.q.as_bytes())?, + openssl::bn::BigNum::from_slice(dsa_private_key.g.as_bytes())?, + openssl::bn::BigNum::from_slice(dsa_private_key.priv_key.as_bytes())?, + openssl::bn::BigNum::from_slice(dsa_private_key.pub_key.as_bytes())?, + )?; + Ok(openssl::pkey::PKey::from_dsa(dsa)?) +} diff --git a/src/rust/cryptography-key-parsing/src/ec.rs b/src/rust/cryptography-key-parsing/src/ec.rs index a15730e67cd7..0d248f916166 100644 --- a/src/rust/cryptography-key-parsing/src/ec.rs +++ b/src/rust/cryptography-key-parsing/src/ec.rs @@ -6,6 +6,17 @@ use cryptography_x509::common::EcParameters; use crate::{KeyParsingError, KeyParsingResult}; +// From RFC 5915 Section 3 +#[derive(asn1::Asn1Read)] +pub(crate) struct EcPrivateKey<'a> { + pub(crate) version: u8, + pub(crate) private_key: &'a [u8], + #[explicit(0)] + pub(crate) parameters: Option>, + #[explicit(1)] + pub(crate) public_key: Option>, +} + pub(crate) fn ec_params_to_group( params: &EcParameters<'_>, ) -> KeyParsingResult { @@ -51,3 +62,43 @@ pub(crate) fn ec_params_to_group( } } } + +pub fn parse_pkcs1_private_key( + data: &[u8], + ec_params: Option>, +) -> KeyParsingResult> { + let ec_private_key = asn1::parse_single::>(data)?; + if ec_private_key.version != 1 { + return Err(crate::KeyParsingError::InvalidKey); + } + + let group = match (ec_params, ec_private_key.parameters) { + (Some(outer_params), Some(inner_params)) => { + if outer_params != inner_params { + return Err(crate::KeyParsingError::InvalidKey); + } + ec_params_to_group(&outer_params)? + } + (Some(outer_params), None) => ec_params_to_group(&outer_params)?, + (None, Some(inner_params)) => ec_params_to_group(&inner_params)?, + (None, None) => return Err(crate::KeyParsingError::InvalidKey), + }; + + 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 { + openssl::ec::EcPoint::from_bytes(&group, point_bytes.as_bytes(), &mut bn_ctx) + .map_err(|_| crate::KeyParsingError::InvalidKey)? + } else { + let mut public_point = openssl::ec::EcPoint::new(&group)?; + public_point + .mul(&group, group.generator(), &private_number, &bn_ctx) + .map_err(|_| crate::KeyParsingError::InvalidKey)?; + public_point + }; + + let ec_key = + openssl::ec::EcKey::from_private_components(&group, &private_number, &public_point)?; + Ok(openssl::pkey::PKey::from_ec_key(ec_key)?) +} +>>>>>>> 1a8c3d5ef (Migrate parsing of unencrypted PKCS#8 private keys to Rust) diff --git a/src/rust/cryptography-key-parsing/src/lib.rs b/src/rust/cryptography-key-parsing/src/lib.rs index 640eb8ae99d2..7088b1efbdfc 100644 --- a/src/rust/cryptography-key-parsing/src/lib.rs +++ b/src/rust/cryptography-key-parsing/src/lib.rs @@ -6,7 +6,9 @@ #![deny(rust_2018_idioms, clippy::undocumented_unsafe_blocks)] #![allow(unknown_lints, clippy::result_large_err)] -mod ec; +pub mod dsa; +pub mod ec; +pub mod pkcs8; pub mod rsa; pub mod spki; diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs new file mode 100644 index 000000000000..aacbc00e8abb --- /dev/null +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -0,0 +1,100 @@ +// This file is dual licensed under the terms of the Apache License, Version +// 2.0, and the BSD License. See the LICENSE file in the root of this repository +// for complete details. + +use cryptography_x509::common::{AlgorithmIdentifier, AlgorithmParameters}; +use cryptography_x509::csr::Attributes; + +use crate::{ec, rsa, KeyParsingError, KeyParsingResult}; + +// RFC 5208 Section 5 +#[derive(asn1::Asn1Read)] +struct PrivateKeyInfo<'a> { + version: u8, + algorithm: AlgorithmIdentifier<'a>, + private_key: &'a [u8], + #[implicit(0)] + _attributes: Option>, +} + +pub fn parse_private_key( + data: &[u8], +) -> KeyParsingResult> { + let k = asn1::parse_single::>(data)?; + if k.version != 0 { + return Err(crate::KeyParsingError::InvalidKey); + } + match k.algorithm.params { + AlgorithmParameters::Rsa(_) => rsa::parse_pkcs1_private_key(k.private_key), + AlgorithmParameters::Ec(ec_params) => { + ec::parse_pkcs1_private_key(k.private_key, Some(ec_params)) + } + + AlgorithmParameters::Dsa(dsa_params) => { + let dsa_private_key = openssl::bn::BigNum::from_slice( + asn1::parse_single::>(k.private_key)?.as_bytes(), + )?; + let p = openssl::bn::BigNum::from_slice(dsa_params.p.as_bytes())?; + let q = openssl::bn::BigNum::from_slice(dsa_params.q.as_bytes())?; + let g = openssl::bn::BigNum::from_slice(dsa_params.g.as_bytes())?; + + let mut bn_ctx = openssl::bn::BigNumContext::new()?; + let mut pub_key = openssl::bn::BigNum::new()?; + pub_key.mod_exp(&g, &dsa_private_key, &p, &mut bn_ctx)?; + + let dsa = + openssl::dsa::Dsa::from_private_components(p, q, g, dsa_private_key, pub_key)?; + Ok(openssl::pkey::PKey::from_dsa(dsa)?) + } + + #[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))] + AlgorithmParameters::Dh(dh_params) => { + let dh_private_key = openssl::bn::BigNum::from_slice( + asn1::parse_single::>(k.private_key)?.as_bytes(), + )?; + let p = openssl::bn::BigNum::from_slice(dh_params.p.as_bytes())?; + let g = openssl::bn::BigNum::from_slice(dh_params.g.as_bytes())?; + let q = openssl::bn::BigNum::from_slice(dh_params.q.as_bytes())?; + + let dh = openssl::dh::Dh::from_params(p, g, q)?; + let dh = dh.set_private_key(dh_private_key)?; + Ok(openssl::pkey::PKey::from_dh(dh)?) + } + + #[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))] + AlgorithmParameters::DhKeyAgreement(dh_params) => { + let dh_private_key = openssl::bn::BigNum::from_slice( + asn1::parse_single::>(k.private_key)?.as_bytes(), + )?; + let p = openssl::bn::BigNum::from_slice(dh_params.p.as_bytes())?; + let g = openssl::bn::BigNum::from_slice(dh_params.g.as_bytes())?; + + let dh = openssl::dh::Dh::from_pqg(p, None, g)?; + let dh = dh.set_private_key(dh_private_key)?; + Ok(openssl::pkey::PKey::from_dh(dh)?) + } + + AlgorithmParameters::X25519 => Ok(openssl::pkey::PKey::private_key_from_raw_bytes( + asn1::parse_single(k.private_key)?, + openssl::pkey::Id::X25519, + )?), + #[cfg(all(not(CRYPTOGRAPHY_IS_LIBRESSL), not(CRYPTOGRAPHY_IS_BORINGSSL)))] + AlgorithmParameters::X448 => Ok(openssl::pkey::PKey::private_key_from_raw_bytes( + asn1::parse_single(k.private_key)?, + openssl::pkey::Id::X448, + )?), + AlgorithmParameters::Ed25519 => Ok(openssl::pkey::PKey::private_key_from_raw_bytes( + asn1::parse_single(k.private_key)?, + openssl::pkey::Id::ED25519, + )?), + #[cfg(all(not(CRYPTOGRAPHY_IS_LIBRESSL), not(CRYPTOGRAPHY_IS_BORINGSSL)))] + AlgorithmParameters::Ed448 => Ok(openssl::pkey::PKey::private_key_from_raw_bytes( + asn1::parse_single(k.private_key)?, + openssl::pkey::Id::ED448, + )?), + + _ => Err(KeyParsingError::UnsupportedKeyType( + k.algorithm.oid().clone(), + )), + } +} diff --git a/src/rust/cryptography-key-parsing/src/rsa.rs b/src/rust/cryptography-key-parsing/src/rsa.rs index bf33a492352e..a775c5447e6f 100644 --- a/src/rust/cryptography-key-parsing/src/rsa.rs +++ b/src/rust/cryptography-key-parsing/src/rsa.rs @@ -10,6 +10,22 @@ pub struct Pkcs1RsaPublicKey<'a> { e: asn1::BigUint<'a>, } +// RFC 8017, Section A.1.2 +#[derive(asn1::Asn1Read)] +pub(crate) struct RsaPrivateKey<'a> { + pub(crate) version: u8, + pub(crate) n: asn1::BigUint<'a>, + pub(crate) e: asn1::BigUint<'a>, + pub(crate) d: asn1::BigUint<'a>, + pub(crate) p: asn1::BigUint<'a>, + pub(crate) q: asn1::BigUint<'a>, + pub(crate) dmp1: asn1::BigUint<'a>, + pub(crate) dmq1: asn1::BigUint<'a>, + pub(crate) iqmp: asn1::BigUint<'a>, + // We don't support these, so don't bother to parse the inner fields. + pub(crate) other_prime_infos: Option, 1>>, +} + pub fn parse_pkcs1_public_key( data: &[u8], ) -> KeyParsingResult> { @@ -21,3 +37,23 @@ pub fn parse_pkcs1_public_key( let rsa = openssl::rsa::Rsa::from_public_components(n, e)?; Ok(openssl::pkey::PKey::from_rsa(rsa)?) } + +pub fn parse_pkcs1_private_key( + data: &[u8], +) -> KeyParsingResult> { + let rsa_private_key = asn1::parse_single::>(data)?; + if rsa_private_key.version != 0 || rsa_private_key.other_prime_infos.is_some() { + return Err(crate::KeyParsingError::InvalidKey); + } + let rsa_key = openssl::rsa::Rsa::from_private_components( + openssl::bn::BigNum::from_slice(rsa_private_key.n.as_bytes())?, + openssl::bn::BigNum::from_slice(rsa_private_key.e.as_bytes())?, + openssl::bn::BigNum::from_slice(rsa_private_key.d.as_bytes())?, + openssl::bn::BigNum::from_slice(rsa_private_key.p.as_bytes())?, + openssl::bn::BigNum::from_slice(rsa_private_key.q.as_bytes())?, + openssl::bn::BigNum::from_slice(rsa_private_key.dmp1.as_bytes())?, + openssl::bn::BigNum::from_slice(rsa_private_key.dmq1.as_bytes())?, + openssl::bn::BigNum::from_slice(rsa_private_key.iqmp.as_bytes())?, + )?; + Ok(openssl::pkey::PKey::from_rsa(rsa_key)?) +} diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 94c859a436f0..37ba58e75d00 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -34,7 +34,18 @@ pub(crate) fn load_der_private_key_bytes<'p>( password: Option<&[u8]>, unsafe_skip_rsa_key_validation: bool, ) -> CryptographyResult> { - if let Ok(pkey) = openssl::pkey::PKey::private_key_from_der(data) { + let pkey = if let Ok(pkey) = cryptography_key_parsing::pkcs8::parse_private_key(data) { + Some(pkey) + } else if let Ok(pkey) = cryptography_key_parsing::dsa::parse_pkcs1_private_key(data) { + Some(pkey) + } else if let Ok(pkey) = cryptography_key_parsing::ec::parse_pkcs1_private_key(data, None) { + Some(pkey) + } else if let Ok(pkey) = cryptography_key_parsing::rsa::parse_pkcs1_private_key(data) { + Some(pkey) + } else { + None + }; + if let Some(pkey) = pkey { if password.is_some() { return Err(CryptographyError::from( pyo3::exceptions::PyTypeError::new_err( diff --git a/tests/hazmat/backends/test_openssl.py b/tests/hazmat/backends/test_openssl.py index 901eec59776f..aa89caa926c0 100644 --- a/tests/hazmat/backends/test_openssl.py +++ b/tests/hazmat/backends/test_openssl.py @@ -248,10 +248,6 @@ class TestOpenSSLDHSerialization: os.path.join("asymmetric", "DH", "dhkey_rfc5114_2.pem"), serialization.load_pem_private_key, ), - ( - os.path.join("asymmetric", "DH", "dhkey_rfc5114_2.der"), - serialization.load_der_private_key, - ), ], ) def test_private_load_dhx_unsupported( From 2ae22585c52297d9efc9674ff1f0188e3dc2d98d Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 17 Jan 2025 15:47:02 -0500 Subject: [PATCH 02/46] handle unencrypted PEMs as well --- src/rust/cryptography-key-parsing/src/ec.rs | 2 +- .../cryptography-key-parsing/src/pkcs8.rs | 4 ++- src/rust/src/backend/keys.rs | 31 +++++++++++++++++++ tests/hazmat/primitives/test_ec.py | 16 ++++------ 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/rust/cryptography-key-parsing/src/ec.rs b/src/rust/cryptography-key-parsing/src/ec.rs index 0d248f916166..ec06fa1dd074 100644 --- a/src/rust/cryptography-key-parsing/src/ec.rs +++ b/src/rust/cryptography-key-parsing/src/ec.rs @@ -92,7 +92,7 @@ pub fn parse_pkcs1_private_key( } else { let mut public_point = openssl::ec::EcPoint::new(&group)?; public_point - .mul(&group, group.generator(), &private_number, &bn_ctx) + .mul_generator(&group, &private_number, &bn_ctx) .map_err(|_| crate::KeyParsingError::InvalidKey)?; public_point }; diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index aacbc00e8abb..30388790efab 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -25,7 +25,9 @@ pub fn parse_private_key( return Err(crate::KeyParsingError::InvalidKey); } match k.algorithm.params { - AlgorithmParameters::Rsa(_) => rsa::parse_pkcs1_private_key(k.private_key), + AlgorithmParameters::Rsa(_) | AlgorithmParameters::RsaPss(_) => { + rsa::parse_pkcs1_private_key(k.private_key) + } AlgorithmParameters::Ec(ec_params) => { ec::parse_pkcs1_private_key(k.private_key, Some(ec_params)) } diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 37ba58e75d00..8de98f9a4bfd 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -74,6 +74,37 @@ fn load_pem_private_key<'p>( backend: Option>, unsafe_skip_rsa_key_validation: bool, ) -> CryptographyResult> { + let p = pem::parse(data.as_bytes())?; + if p.headers().get("Proc-Type").is_none() { + let pkey = match p.tag() { + "PRIVATE KEY" => Some(cryptography_key_parsing::pkcs8::parse_private_key( + p.contents(), + )?), + "ENCRYPTED PRIVATE KEY" => None, + "RSA PRIVATE KEY" => Some(cryptography_key_parsing::rsa::parse_pkcs1_private_key( + p.contents(), + )?), + "EC PRIVATE KEY" => Some(cryptography_key_parsing::ec::parse_pkcs1_private_key( + p.contents(), + None, + )?), + "DSA PRIVATE KEY" => Some(cryptography_key_parsing::dsa::parse_pkcs1_private_key( + p.contents(), + )?), + _ => panic!("{:?}", p.tag()), + }; + if let Some(pkey) = pkey { + if password.is_some() { + return Err(CryptographyError::from( + pyo3::exceptions::PyTypeError::new_err( + "Password was given but private key is not encrypted.", + ), + )); + } + return private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation); + } + } + let _ = backend; let password = password.as_ref().map(CffiBuf::as_bytes); let mut status = utils::PasswordCallbackStatus::Unused; diff --git a/tests/hazmat/primitives/test_ec.py b/tests/hazmat/primitives/test_ec.py index 1a86c76b2465..ae1a633d2dfc 100644 --- a/tests/hazmat/primitives/test_ec.py +++ b/tests/hazmat/primitives/test_ec.py @@ -482,14 +482,7 @@ def test_load_invalid_ec_key_from_pem(self, backend): backend=backend, ) - @pytest.mark.supported( - only_if=( - lambda backend: rust_openssl.CRYPTOGRAPHY_OPENSSL_300_OR_GREATER - or rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL - ), - skip_message="LibreSSL and OpenSSL 1.1.1 handle this differently", - ) - def test_load_invalid_private_scalar_pem(self, backend): + def test_load_large_private_scalar_pem(self, backend): _skip_curve_unsupported(backend, ec.SECP256R1()) data = load_vectors_from_file( @@ -498,8 +491,11 @@ def test_load_invalid_private_scalar_pem(self, backend): ), lambda pemfile: pemfile.read().encode(), ) - with pytest.raises(ValueError): - serialization.load_pem_private_key(data, None) + key = serialization.load_pem_private_key(data, password=None) + assert isinstance(key, EllipticCurvePrivateKey) + assert key.private_numbers().private_value == ( + 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00 + ) def test_signatures(self, backend, subtests): vectors = itertools.chain( From 12c4e24c02be1c7eea52cef6a592c91fa14baca9 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 17 Jan 2025 16:09:04 -0500 Subject: [PATCH 03/46] fixes --- src/rust/cryptography-key-parsing/src/ec.rs | 3 ++- src/rust/src/backend/keys.rs | 9 +++++-- tests/hazmat/backends/test_openssl.py | 28 --------------------- 3 files changed, 9 insertions(+), 31 deletions(-) diff --git a/src/rust/cryptography-key-parsing/src/ec.rs b/src/rust/cryptography-key-parsing/src/ec.rs index ec06fa1dd074..f3cbf2c30ae1 100644 --- a/src/rust/cryptography-key-parsing/src/ec.rs +++ b/src/rust/cryptography-key-parsing/src/ec.rs @@ -98,7 +98,8 @@ pub fn parse_pkcs1_private_key( }; let ec_key = - openssl::ec::EcKey::from_private_components(&group, &private_number, &public_point)?; + openssl::ec::EcKey::from_private_components(&group, &private_number, &public_point) + .map_err(|_| KeyParsingError::InvalidKey)?; Ok(openssl::pkey::PKey::from_ec_key(ec_key)?) } >>>>>>> 1a8c3d5ef (Migrate parsing of unencrypted PKCS#8 private keys to Rust) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 8de98f9a4bfd..22f4dc8722c1 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -8,6 +8,7 @@ use crate::backend::utils; use crate::buf::CffiBuf; use crate::error::{CryptographyError, CryptographyResult}; use crate::exceptions; +use crate::x509; #[pyo3::pyfunction] #[pyo3(signature = (data, password, backend=None, *, unsafe_skip_rsa_key_validation=false))] @@ -74,7 +75,11 @@ fn load_pem_private_key<'p>( backend: Option>, unsafe_skip_rsa_key_validation: bool, ) -> CryptographyResult> { - let p = pem::parse(data.as_bytes())?; + let p = x509::find_in_pem( + data.as_bytes(), + |p| ["PRIVATE KEY", "ENCRYPTED PRIVATE KEY", "RSA PRIVATE KEY", "EC PRIVATE KEY", "DSA PRIVATE KEY"].contains(&p.tag()), + "Valid PEM but no BEGIN/END delimiters for a private key found. Are you sure this is a private key?" + )?; if p.headers().get("Proc-Type").is_none() { let pkey = match p.tag() { "PRIVATE KEY" => Some(cryptography_key_parsing::pkcs8::parse_private_key( @@ -91,7 +96,7 @@ fn load_pem_private_key<'p>( "DSA PRIVATE KEY" => Some(cryptography_key_parsing::dsa::parse_pkcs1_private_key( p.contents(), )?), - _ => panic!("{:?}", p.tag()), + _ => unreachable!(), }; if let Some(pkey) = pkey { if password.is_some() { diff --git a/tests/hazmat/backends/test_openssl.py b/tests/hazmat/backends/test_openssl.py index aa89caa926c0..f43f03435539 100644 --- a/tests/hazmat/backends/test_openssl.py +++ b/tests/hazmat/backends/test_openssl.py @@ -230,31 +230,3 @@ def test_password_length_limit(self, rsa_key_2048): serialization.PrivateFormat.PKCS8, serialization.BestAvailableEncryption(password), ) - - -@pytest.mark.skipif( - backend._lib.Cryptography_HAS_EVP_PKEY_DHX == 1, - reason="Requires OpenSSL without EVP_PKEY_DHX", -) -@pytest.mark.supported( - only_if=lambda backend: backend.dh_supported(), - skip_message="Requires DH support", -) -class TestOpenSSLDHSerialization: - @pytest.mark.parametrize( - ("key_path", "loader_func"), - [ - ( - os.path.join("asymmetric", "DH", "dhkey_rfc5114_2.pem"), - serialization.load_pem_private_key, - ), - ], - ) - def test_private_load_dhx_unsupported( - self, key_path, loader_func, backend - ): - key_bytes = load_vectors_from_file( - key_path, lambda pemfile: pemfile.read(), mode="rb" - ) - with pytest.raises(ValueError): - loader_func(key_bytes, None, backend) From 673d51d82849f2d10060bcda4c6e9525b012bf61 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 17 Jan 2025 16:23:06 -0500 Subject: [PATCH 04/46] remove boringssl branches --- tests/hazmat/primitives/test_rsa.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/tests/hazmat/primitives/test_rsa.py b/tests/hazmat/primitives/test_rsa.py index c5a98bea8ca6..dba811aa099b 100644 --- a/tests/hazmat/primitives/test_rsa.py +++ b/tests/hazmat/primitives/test_rsa.py @@ -251,10 +251,6 @@ def test_load_pss_vect_example_keys(self, pkcs1_example): assert public_num.n == public_num2.n assert public_num.e == public_num2.e - @pytest.mark.supported( - only_if=lambda backend: not rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL, - skip_message="Does not support RSA PSS loading", - ) @pytest.mark.parametrize( "path", [ @@ -302,24 +298,6 @@ def test_load_pss_pub_keys_strips_constraints(self, backend): b"badsig", b"whatever", padding.PKCS1v15(), hashes.SHA256() ) - @pytest.mark.supported( - only_if=lambda backend: rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL, - skip_message="Test requires a backend without RSA-PSS key support", - ) - def test_load_pss_unsupported(self, backend): - # Key loading errors unfortunately have multiple paths so - # we need to allow ValueError and UnsupportedAlgorithm - with pytest.raises((UnsupportedAlgorithm, ValueError)): - load_vectors_from_file( - filename=os.path.join( - "asymmetric", "PKCS8", "rsa_pss_2048.pem" - ), - loader=lambda p: serialization.load_pem_private_key( - p.read(), password=None - ), - mode="rb", - ) - @pytest.mark.parametrize( "vector", load_vectors_from_file( From d5b2cd5a152ceb67ad179729b5ad8d8dd8315ef6 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 17 Jan 2025 16:37:34 -0500 Subject: [PATCH 05/46] ruff --- tests/hazmat/primitives/test_rsa.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/hazmat/primitives/test_rsa.py b/tests/hazmat/primitives/test_rsa.py index dba811aa099b..820b9aee503f 100644 --- a/tests/hazmat/primitives/test_rsa.py +++ b/tests/hazmat/primitives/test_rsa.py @@ -10,12 +10,7 @@ import pytest -from cryptography.exceptions import ( - InvalidSignature, - UnsupportedAlgorithm, - _Reasons, -) -from cryptography.hazmat.bindings._rust import openssl as rust_openssl +from cryptography.exceptions import InvalidSignature, _Reasons from cryptography.hazmat.primitives import hashes, serialization from cryptography.hazmat.primitives.asymmetric import padding, rsa from cryptography.hazmat.primitives.asymmetric import utils as asym_utils From 3aef309a7ba28c842040b5f55f2c213f56d8efca Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 17 Jan 2025 16:45:55 -0500 Subject: [PATCH 06/46] fixes --- src/rust/cryptography-key-parsing/src/ec.rs | 3 +++ tests/hazmat/primitives/test_ec.py | 17 ++++------------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/rust/cryptography-key-parsing/src/ec.rs b/src/rust/cryptography-key-parsing/src/ec.rs index f3cbf2c30ae1..458fd5744d28 100644 --- a/src/rust/cryptography-key-parsing/src/ec.rs +++ b/src/rust/cryptography-key-parsing/src/ec.rs @@ -100,6 +100,9 @@ pub fn parse_pkcs1_private_key( let ec_key = openssl::ec::EcKey::from_private_components(&group, &private_number, &public_point) .map_err(|_| KeyParsingError::InvalidKey)?; + ec_key + .check_key() + .map_err(|_| KeyParsingError::InvalidKey)?; Ok(openssl::pkey::PKey::from_ec_key(ec_key)?) } >>>>>>> 1a8c3d5ef (Migrate parsing of unencrypted PKCS#8 private keys to Rust) diff --git a/tests/hazmat/primitives/test_ec.py b/tests/hazmat/primitives/test_ec.py index ae1a633d2dfc..3a2136653ec2 100644 --- a/tests/hazmat/primitives/test_ec.py +++ b/tests/hazmat/primitives/test_ec.py @@ -450,13 +450,7 @@ def test_load_invalid_public_ec_key_from_numbers(self, backend): def test_load_invalid_ec_key_from_pem(self, backend): _skip_curve_unsupported(backend, ec.SECP256R1()) - # BoringSSL rejects infinity points before it ever gets to us, so it - # uses a more generic error message. - match = ( - r"infinity|invalid form" - if not rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL - else None - ) + match = r"infinity|invalid form|Invalid key" with pytest.raises(ValueError, match=match): serialization.load_pem_public_key( textwrap.dedent( @@ -482,7 +476,7 @@ def test_load_invalid_ec_key_from_pem(self, backend): backend=backend, ) - def test_load_large_private_scalar_pem(self, backend): + def test_load_private_scalar_greater_than_order_pem(self, backend): _skip_curve_unsupported(backend, ec.SECP256R1()) data = load_vectors_from_file( @@ -491,11 +485,8 @@ def test_load_large_private_scalar_pem(self, backend): ), lambda pemfile: pemfile.read().encode(), ) - key = serialization.load_pem_private_key(data, password=None) - assert isinstance(key, EllipticCurvePrivateKey) - assert key.private_numbers().private_value == ( - 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00 - ) + with pytest.raises(ValueError): + serialization.load_pem_private_key(data, password=None) def test_signatures(self, backend, subtests): vectors = itertools.chain( From e3d1554af427ecd9663a9df30cebc93d4f89122d Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 17 Jan 2025 23:10:08 -0500 Subject: [PATCH 07/46] xxx --- tests/hazmat/primitives/test_serialization.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/hazmat/primitives/test_serialization.py b/tests/hazmat/primitives/test_serialization.py index f78c22fde43c..4e8589b95950 100644 --- a/tests/hazmat/primitives/test_serialization.py +++ b/tests/hazmat/primitives/test_serialization.py @@ -408,7 +408,6 @@ def test_wrong_parameters_format(self, backend): with pytest.raises(ValueError): load_der_parameters(param_data, backend) - @pytest.mark.xfail() def test_load_pkcs8_private_key_invalid_version(self): data = load_vectors_from_file( os.path.join("asymmetric", "PKCS8", "invalid-version.der"), From 52bbe8041cd829fbe2b946a1936ad9f1f677f1f4 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 17 Jan 2025 23:21:38 -0500 Subject: [PATCH 08/46] this is fine now, I guess --- tests/hazmat/primitives/test_dsa.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/hazmat/primitives/test_dsa.py b/tests/hazmat/primitives/test_dsa.py index b8ccaa28fa54..ff6311b59091 100644 --- a/tests/hazmat/primitives/test_dsa.py +++ b/tests/hazmat/primitives/test_dsa.py @@ -570,16 +570,12 @@ def test_prehashed_digest_mismatch(self, backend): skip_message="Requires OpenSSL 3.0.9+, LibreSSL, or BoringSSL", ) def test_nilpotent(self): - try: - key = load_vectors_from_file( - os.path.join("asymmetric", "DSA", "custom", "nilpotent.pem"), - lambda pemfile: serialization.load_pem_private_key( - pemfile.read().encode(), password=None - ), - ) - except ValueError: - # LibreSSL simply rejects this key on load. - return + key = load_vectors_from_file( + os.path.join("asymmetric", "DSA", "custom", "nilpotent.pem"), + lambda pemfile: serialization.load_pem_private_key( + pemfile.read().encode(), password=None + ), + ) assert isinstance(key, dsa.DSAPrivateKey) with pytest.raises(ValueError): From ab984a9667d53daa06cc37be882bd3b60eb78944 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 17 Jan 2025 23:28:14 -0500 Subject: [PATCH 09/46] cleanups --- src/rust/cryptography-key-parsing/src/dsa.rs | 13 ++-- .../cryptography-key-parsing/src/pkcs8.rs | 62 +++++++++++-------- src/rust/cryptography-key-parsing/src/rsa.rs | 19 +++--- 3 files changed, 52 insertions(+), 42 deletions(-) diff --git a/src/rust/cryptography-key-parsing/src/dsa.rs b/src/rust/cryptography-key-parsing/src/dsa.rs index 6391a31991fd..0b0461370dd2 100644 --- a/src/rust/cryptography-key-parsing/src/dsa.rs +++ b/src/rust/cryptography-key-parsing/src/dsa.rs @@ -21,12 +21,11 @@ pub fn parse_pkcs1_private_key( if dsa_private_key.version != 0 { return Err(crate::KeyParsingError::InvalidKey); } - let dsa = openssl::dsa::Dsa::from_private_components( - openssl::bn::BigNum::from_slice(dsa_private_key.p.as_bytes())?, - openssl::bn::BigNum::from_slice(dsa_private_key.q.as_bytes())?, - openssl::bn::BigNum::from_slice(dsa_private_key.g.as_bytes())?, - openssl::bn::BigNum::from_slice(dsa_private_key.priv_key.as_bytes())?, - openssl::bn::BigNum::from_slice(dsa_private_key.pub_key.as_bytes())?, - )?; + let p = openssl::bn::BigNum::from_slice(dsa_private_key.p.as_bytes())?; + let q = openssl::bn::BigNum::from_slice(dsa_private_key.q.as_bytes())?; + let g = openssl::bn::BigNum::from_slice(dsa_private_key.g.as_bytes())?; + let priv_key = openssl::bn::BigNum::from_slice(dsa_private_key.priv_key.as_bytes())?; + let pub_key = openssl::bn::BigNum::from_slice(dsa_private_key.pub_key.as_bytes())?; + let dsa = openssl::dsa::Dsa::from_private_components(p, q, g, priv_key, pub_key)?; Ok(openssl::pkey::PKey::from_dsa(dsa)?) } diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index 30388790efab..b8817d85c25a 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -33,9 +33,9 @@ pub fn parse_private_key( } AlgorithmParameters::Dsa(dsa_params) => { - let dsa_private_key = openssl::bn::BigNum::from_slice( - asn1::parse_single::>(k.private_key)?.as_bytes(), - )?; + let private_key_bytes = + asn1::parse_single::>(k.private_key)?.as_bytes(); + let dsa_private_key = openssl::bn::BigNum::from_slice(private_key_bytes)?; let p = openssl::bn::BigNum::from_slice(dsa_params.p.as_bytes())?; let q = openssl::bn::BigNum::from_slice(dsa_params.q.as_bytes())?; let g = openssl::bn::BigNum::from_slice(dsa_params.g.as_bytes())?; @@ -51,9 +51,9 @@ pub fn parse_private_key( #[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))] AlgorithmParameters::Dh(dh_params) => { - let dh_private_key = openssl::bn::BigNum::from_slice( - asn1::parse_single::>(k.private_key)?.as_bytes(), - )?; + let private_key_bytes = + asn1::parse_single::>(k.private_key)?.as_bytes(); + let dh_private_key = openssl::bn::BigNum::from_slice(private_key_bytes)?; let p = openssl::bn::BigNum::from_slice(dh_params.p.as_bytes())?; let g = openssl::bn::BigNum::from_slice(dh_params.g.as_bytes())?; let q = openssl::bn::BigNum::from_slice(dh_params.q.as_bytes())?; @@ -65,9 +65,9 @@ pub fn parse_private_key( #[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))] AlgorithmParameters::DhKeyAgreement(dh_params) => { - let dh_private_key = openssl::bn::BigNum::from_slice( - asn1::parse_single::>(k.private_key)?.as_bytes(), - )?; + let private_key_bytes = + asn1::parse_single::>(k.private_key)?.as_bytes(); + let dh_private_key = openssl::bn::BigNum::from_slice(private_key_bytes)?; let p = openssl::bn::BigNum::from_slice(dh_params.p.as_bytes())?; let g = openssl::bn::BigNum::from_slice(dh_params.g.as_bytes())?; @@ -76,24 +76,36 @@ pub fn parse_private_key( Ok(openssl::pkey::PKey::from_dh(dh)?) } - AlgorithmParameters::X25519 => Ok(openssl::pkey::PKey::private_key_from_raw_bytes( - asn1::parse_single(k.private_key)?, - openssl::pkey::Id::X25519, - )?), + AlgorithmParameters::X25519 => { + let key_bytes = asn1::parse_single(k.private_key)?; + Ok(openssl::pkey::PKey::private_key_from_raw_bytes( + key_bytes, + openssl::pkey::Id::X25519, + )?) + } #[cfg(all(not(CRYPTOGRAPHY_IS_LIBRESSL), not(CRYPTOGRAPHY_IS_BORINGSSL)))] - AlgorithmParameters::X448 => Ok(openssl::pkey::PKey::private_key_from_raw_bytes( - asn1::parse_single(k.private_key)?, - openssl::pkey::Id::X448, - )?), - AlgorithmParameters::Ed25519 => Ok(openssl::pkey::PKey::private_key_from_raw_bytes( - asn1::parse_single(k.private_key)?, - openssl::pkey::Id::ED25519, - )?), + AlgorithmParameters::X448 => { + let key_bytes = asn1::parse_single(k.private_key)?; + Ok(openssl::pkey::PKey::private_key_from_raw_bytes( + key_bytes, + openssl::pkey::Id::X448, + )?) + } + AlgorithmParameters::Ed25519 => { + let key_bytes = asn1::parse_single(k.private_key)?; + Ok(openssl::pkey::PKey::private_key_from_raw_bytes( + key_bytes, + openssl::pkey::Id::ED25519, + )?) + } #[cfg(all(not(CRYPTOGRAPHY_IS_LIBRESSL), not(CRYPTOGRAPHY_IS_BORINGSSL)))] - AlgorithmParameters::Ed448 => Ok(openssl::pkey::PKey::private_key_from_raw_bytes( - asn1::parse_single(k.private_key)?, - openssl::pkey::Id::ED448, - )?), + AlgorithmParameters::Ed448 => { + let key_bytes = asn1::parse_single(k.private_key)?; + Ok(openssl::pkey::PKey::private_key_from_raw_bytes( + key_bytes, + openssl::pkey::Id::ED448, + )?) + } _ => Err(KeyParsingError::UnsupportedKeyType( k.algorithm.oid().clone(), diff --git a/src/rust/cryptography-key-parsing/src/rsa.rs b/src/rust/cryptography-key-parsing/src/rsa.rs index a775c5447e6f..2de5a0be3401 100644 --- a/src/rust/cryptography-key-parsing/src/rsa.rs +++ b/src/rust/cryptography-key-parsing/src/rsa.rs @@ -45,15 +45,14 @@ pub fn parse_pkcs1_private_key( if rsa_private_key.version != 0 || rsa_private_key.other_prime_infos.is_some() { return Err(crate::KeyParsingError::InvalidKey); } - let rsa_key = openssl::rsa::Rsa::from_private_components( - openssl::bn::BigNum::from_slice(rsa_private_key.n.as_bytes())?, - openssl::bn::BigNum::from_slice(rsa_private_key.e.as_bytes())?, - openssl::bn::BigNum::from_slice(rsa_private_key.d.as_bytes())?, - openssl::bn::BigNum::from_slice(rsa_private_key.p.as_bytes())?, - openssl::bn::BigNum::from_slice(rsa_private_key.q.as_bytes())?, - openssl::bn::BigNum::from_slice(rsa_private_key.dmp1.as_bytes())?, - openssl::bn::BigNum::from_slice(rsa_private_key.dmq1.as_bytes())?, - openssl::bn::BigNum::from_slice(rsa_private_key.iqmp.as_bytes())?, - )?; + let n = openssl::bn::BigNum::from_slice(rsa_private_key.n.as_bytes())?; + let e = openssl::bn::BigNum::from_slice(rsa_private_key.e.as_bytes())?; + let d = openssl::bn::BigNum::from_slice(rsa_private_key.d.as_bytes())?; + let p = openssl::bn::BigNum::from_slice(rsa_private_key.p.as_bytes())?; + let q = openssl::bn::BigNum::from_slice(rsa_private_key.q.as_bytes())?; + let dmp1 = openssl::bn::BigNum::from_slice(rsa_private_key.dmp1.as_bytes())?; + let dmq1 = openssl::bn::BigNum::from_slice(rsa_private_key.dmq1.as_bytes())?; + let iqmp = openssl::bn::BigNum::from_slice(rsa_private_key.iqmp.as_bytes())?; + let rsa_key = openssl::rsa::Rsa::from_private_components(n, e, d, p, q, dmp1, dmq1, iqmp)?; Ok(openssl::pkey::PKey::from_rsa(rsa_key)?) } From bde0dbc41b27ba763fbbcbbb9606156ecc463eec Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sat, 18 Jan 2025 15:30:15 -0500 Subject: [PATCH 10/46] these pass --- tests/hazmat/primitives/test_serialization.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/hazmat/primitives/test_serialization.py b/tests/hazmat/primitives/test_serialization.py index 4e8589b95950..410d8e6df79d 100644 --- a/tests/hazmat/primitives/test_serialization.py +++ b/tests/hazmat/primitives/test_serialization.py @@ -1251,7 +1251,6 @@ def test_encrypted_pkcs8_non_utf_password(self): with pytest.raises(ValueError): load_pem_private_key(data, password=b"\xff") - @pytest.mark.xfail() def test_rsa_private_key_invalid_version(self): data = load_vectors_from_file( os.path.join( @@ -1265,7 +1264,6 @@ def test_rsa_private_key_invalid_version(self): with pytest.raises(ValueError): load_pem_private_key(data, password=None) - @pytest.mark.xfail() def test_dsa_private_key_invalid_version(self): data = load_vectors_from_file( os.path.join( From 5faca347f62f0f15030da7f7415356db70941180 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sat, 18 Jan 2025 23:06:53 -0500 Subject: [PATCH 11/46] no more xfail! --- tests/hazmat/primitives/test_ec.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/hazmat/primitives/test_ec.py b/tests/hazmat/primitives/test_ec.py index 3a2136653ec2..d04e4cd7088b 100644 --- a/tests/hazmat/primitives/test_ec.py +++ b/tests/hazmat/primitives/test_ec.py @@ -1114,7 +1114,6 @@ def test_load_public_keys(self, key_file, curve, backend): assert isinstance(key, ec.EllipticCurvePublicKey) assert isinstance(key.curve, curve) - @pytest.mark.xfail def test_pkcs8_inconsistent_curve(self): # The curve can appear twice in a PKCS8 EC key, error if they're not # consistent From f8164ae9e08fce19cc294caacda6c5440c83c3b2 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sun, 19 Jan 2025 11:05:46 -0500 Subject: [PATCH 12/46] we pass --- tests/hazmat/primitives/test_ec.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/hazmat/primitives/test_ec.py b/tests/hazmat/primitives/test_ec.py index d04e4cd7088b..00afe830f36b 100644 --- a/tests/hazmat/primitives/test_ec.py +++ b/tests/hazmat/primitives/test_ec.py @@ -1154,7 +1154,6 @@ def test_load_private_key_missing_curve(self): with pytest.raises(ValueError): serialization.load_pem_private_key(data, password=None) - @pytest.mark.xfail def test_load_private_key_invalid_version(self): data = load_vectors_from_file( os.path.join("asymmetric", "PKCS8", "ec-invalid-version.pem"), From 0c634af3225702df0c72686a6bdb28b41a12e873 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sun, 19 Jan 2025 12:14:18 -0500 Subject: [PATCH 13/46] added TODOs to sketch out missing bits --- src/rust/src/backend/keys.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 22f4dc8722c1..0f33f38fdea9 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -57,6 +57,7 @@ pub(crate) fn load_der_private_key_bytes<'p>( return private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation); } + // TODO: parse as encrypted private key let mut status = utils::PasswordCallbackStatus::Unused; let pkey = openssl::pkey::PKey::private_key_from_pkcs8_callback( data, @@ -80,11 +81,13 @@ fn load_pem_private_key<'p>( |p| ["PRIVATE KEY", "ENCRYPTED PRIVATE KEY", "RSA PRIVATE KEY", "EC PRIVATE KEY", "DSA PRIVATE KEY"].contains(&p.tag()), "Valid PEM but no BEGIN/END delimiters for a private key found. Are you sure this is a private key?" )?; + // TODO: if proc-type is present, decrypt PEM layer. if p.headers().get("Proc-Type").is_none() { let pkey = match p.tag() { "PRIVATE KEY" => Some(cryptography_key_parsing::pkcs8::parse_private_key( p.contents(), )?), + // TODO: Add ENCRYPTED PRIVATE KEY support "ENCRYPTED PRIVATE KEY" => None, "RSA PRIVATE KEY" => Some(cryptography_key_parsing::rsa::parse_pkcs1_private_key( p.contents(), From e0cbaec9b887fd4ec798d455743929d62f21a06b Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Mon, 20 Jan 2025 14:45:51 -0500 Subject: [PATCH 14/46] general progress --- src/rust/cryptography-key-parsing/src/lib.rs | 1 + .../cryptography-key-parsing/src/pkcs8.rs | 19 +++++++ src/rust/cryptography-x509/src/pkcs8.rs | 2 +- src/rust/src/backend/keys.rs | 55 ++++++++----------- src/rust/src/error.rs | 3 + 5 files changed, 48 insertions(+), 32 deletions(-) diff --git a/src/rust/cryptography-key-parsing/src/lib.rs b/src/rust/cryptography-key-parsing/src/lib.rs index 7088b1efbdfc..12baa36c7f4d 100644 --- a/src/rust/cryptography-key-parsing/src/lib.rs +++ b/src/rust/cryptography-key-parsing/src/lib.rs @@ -19,6 +19,7 @@ pub enum KeyParsingError { UnsupportedEllipticCurve(asn1::ObjectIdentifier), Parse(asn1::ParseError), OpenSSL(openssl::error::ErrorStack), + UnsupportedEncryptionAlgorithm(asn1::ObjectIdentifier), } impl From for KeyParsingError { diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index b8817d85c25a..aaec098edcd9 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -4,6 +4,7 @@ use cryptography_x509::common::{AlgorithmIdentifier, AlgorithmParameters}; use cryptography_x509::csr::Attributes; +use cryptography_x509::pkcs8::EncryptedPrivateKeyInfo; use crate::{ec, rsa, KeyParsingError, KeyParsingResult}; @@ -112,3 +113,21 @@ pub fn parse_private_key( )), } } + +pub fn parse_encrypted_private_key( + data: &[u8], + _password: Option<&[u8]>, +) -> KeyParsingResult> { + let epki = asn1::parse_single::>(data)?; + match epki.encryption_algorithm.params { + AlgorithmParameters::Pbes1WithShaAnd3KeyTripleDesCbc(_params) => { + todo!() + } + AlgorithmParameters::Pbes2(_params) => { + todo!() + } + _ => Err(KeyParsingError::UnsupportedEncryptionAlgorithm( + epki.encryption_algorithm.oid().clone(), + )), + } +} diff --git a/src/rust/cryptography-x509/src/pkcs8.rs b/src/rust/cryptography-x509/src/pkcs8.rs index 29be546a7572..c1f2b88c5b37 100644 --- a/src/rust/cryptography-x509/src/pkcs8.rs +++ b/src/rust/cryptography-x509/src/pkcs8.rs @@ -5,7 +5,7 @@ use crate::common::AlgorithmIdentifier; // RFC 5208, Section 6 -#[derive(asn1::Asn1Write)] +#[derive(asn1::Asn1Write, asn1::Asn1Read)] pub struct EncryptedPrivateKeyInfo<'a> { pub encryption_algorithm: AlgorithmIdentifier<'a>, pub encrypted_data: &'a [u8], diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 0f33f38fdea9..eb51b307eda9 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -57,13 +57,7 @@ pub(crate) fn load_der_private_key_bytes<'p>( return private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation); } - // TODO: parse as encrypted private key - let mut status = utils::PasswordCallbackStatus::Unused; - let pkey = openssl::pkey::PKey::private_key_from_pkcs8_callback( - data, - utils::password_callback(&mut status, password), - ); - let pkey = utils::handle_key_load_result(py, pkey, status, password)?; + let pkey = cryptography_key_parsing::pkcs8::parse_encrypted_private_key(data, password)?; private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation) } @@ -84,33 +78,32 @@ fn load_pem_private_key<'p>( // TODO: if proc-type is present, decrypt PEM layer. if p.headers().get("Proc-Type").is_none() { let pkey = match p.tag() { - "PRIVATE KEY" => Some(cryptography_key_parsing::pkcs8::parse_private_key( - p.contents(), - )?), - // TODO: Add ENCRYPTED PRIVATE KEY support - "ENCRYPTED PRIVATE KEY" => None, - "RSA PRIVATE KEY" => Some(cryptography_key_parsing::rsa::parse_pkcs1_private_key( - p.contents(), - )?), - "EC PRIVATE KEY" => Some(cryptography_key_parsing::ec::parse_pkcs1_private_key( - p.contents(), - None, - )?), - "DSA PRIVATE KEY" => Some(cryptography_key_parsing::dsa::parse_pkcs1_private_key( - p.contents(), - )?), + "PRIVATE KEY" => cryptography_key_parsing::pkcs8::parse_private_key(p.contents())?, + "ENCRYPTED PRIVATE KEY" => { + cryptography_key_parsing::pkcs8::parse_encrypted_private_key( + p.contents(), + password.as_ref().map(|v| v.as_bytes()), + )? + } + "RSA PRIVATE KEY" => { + cryptography_key_parsing::rsa::parse_pkcs1_private_key(p.contents())? + } + "EC PRIVATE KEY" => { + cryptography_key_parsing::ec::parse_pkcs1_private_key(p.contents(), None)? + } + "DSA PRIVATE KEY" => { + cryptography_key_parsing::dsa::parse_pkcs1_private_key(p.contents())? + } _ => unreachable!(), }; - if let Some(pkey) = pkey { - if password.is_some() { - return Err(CryptographyError::from( - pyo3::exceptions::PyTypeError::new_err( - "Password was given but private key is not encrypted.", - ), - )); - } - return private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation); + if password.is_some() && p.tag() != "ENCRYPTED PRIVATE KEY" { + return Err(CryptographyError::from( + pyo3::exceptions::PyTypeError::new_err( + "Password was given but private key is not encrypted.", + ), + )); } + return private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation); } let _ = backend; diff --git a/src/rust/src/error.rs b/src/rust/src/error.rs index 165b2b782483..4b25e81a31de 100644 --- a/src/rust/src/error.rs +++ b/src/rust/src/error.rs @@ -78,6 +78,9 @@ impl From for CryptographyError { exceptions::Reasons::UNSUPPORTED_ELLIPTIC_CURVE, ))) } + cryptography_key_parsing::KeyParsingError::UnsupportedEncryptionAlgorithm(_oid) => { + todo!() + } } } } From 8610ec30956ff5bbd054e1a58c1756182b7f90f1 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 21 Jan 2025 19:21:58 -0500 Subject: [PATCH 15/46] encrypted pkcs#8 support --- Cargo.lock | 1 + src/rust/cryptography-key-parsing/Cargo.toml | 1 + src/rust/cryptography-key-parsing/src/lib.rs | 2 + .../cryptography-key-parsing/src/pkcs8.rs | 88 ++++++++++++++++--- src/rust/src/error.rs | 16 +++- 5 files changed, 96 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f1db037a28f1..210531ecf514 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -85,6 +85,7 @@ version = "0.1.0" dependencies = [ "asn1", "cfg-if", + "cryptography-crypto", "cryptography-x509", "openssl", "openssl-sys", diff --git a/src/rust/cryptography-key-parsing/Cargo.toml b/src/rust/cryptography-key-parsing/Cargo.toml index 255ef408716e..ff9350bca60f 100644 --- a/src/rust/cryptography-key-parsing/Cargo.toml +++ b/src/rust/cryptography-key-parsing/Cargo.toml @@ -11,6 +11,7 @@ asn1.workspace = true cfg-if = "1" openssl.workspace = true openssl-sys.workspace = true +cryptography-crypto = { path = "../cryptography-crypto" } cryptography-x509 = { path = "../cryptography-x509" } [lints.rust] diff --git a/src/rust/cryptography-key-parsing/src/lib.rs b/src/rust/cryptography-key-parsing/src/lib.rs index 12baa36c7f4d..8d54f3adf718 100644 --- a/src/rust/cryptography-key-parsing/src/lib.rs +++ b/src/rust/cryptography-key-parsing/src/lib.rs @@ -20,6 +20,8 @@ pub enum KeyParsingError { Parse(asn1::ParseError), OpenSSL(openssl::error::ErrorStack), UnsupportedEncryptionAlgorithm(asn1::ObjectIdentifier), + EncryptedKeyWithoutPassword, + IncorrectPassword, } impl From for KeyParsingError { diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index aaec098edcd9..ea64c9cd02c6 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -116,18 +116,86 @@ pub fn parse_private_key( pub fn parse_encrypted_private_key( data: &[u8], - _password: Option<&[u8]>, + password: Option<&[u8]>, ) -> KeyParsingResult> { let epki = asn1::parse_single::>(data)?; - match epki.encryption_algorithm.params { - AlgorithmParameters::Pbes1WithShaAnd3KeyTripleDesCbc(_params) => { - todo!() + let password = match password { + None | Some(b"") => return Err(KeyParsingError::EncryptedKeyWithoutPassword), + Some(p) => p, + }; + + let plaintext = match epki.encryption_algorithm.params { + AlgorithmParameters::Pbes1WithShaAnd3KeyTripleDesCbc(params) => { + // XXX: + // - handle invalid utf8 + let password = std::str::from_utf8(password).unwrap(); + let key = cryptography_crypto::pkcs12::kdf( + password, + ¶ms.salt, + cryptography_crypto::pkcs12::KDF_ENCRYPTION_KEY_ID, + params.iterations, + 24, + openssl::hash::MessageDigest::sha1(), + )?; + let iv = cryptography_crypto::pkcs12::kdf( + password, + ¶ms.salt, + cryptography_crypto::pkcs12::KDF_IV_ID, + params.iterations, + 8, + openssl::hash::MessageDigest::sha1(), + )?; + + openssl::symm::decrypt( + openssl::symm::Cipher::des_ede3_cbc(), + &key, + Some(&iv), + epki.encrypted_data, + ) + .map_err(|_| KeyParsingError::IncorrectPassword)? + } + AlgorithmParameters::Pbes2(params) => { + let (cipher, iv) = match params.encryption_scheme.params { + AlgorithmParameters::Aes128Cbc(iv) => (openssl::symm::Cipher::aes_128_cbc(), iv), + AlgorithmParameters::Aes256Cbc(iv) => (openssl::symm::Cipher::aes_256_cbc(), iv), + _ => todo!(), + }; + + let key = match params.key_derivation_func.params { + AlgorithmParameters::Pbkdf2(pbkdf2_params) => { + let mut key = vec![0; cipher.key_len()]; + let md = match pbkdf2_params.prf.params { + AlgorithmParameters::HmacWithSha1(_) => { + openssl::hash::MessageDigest::sha1() + } + AlgorithmParameters::HmacWithSha256(_) => { + openssl::hash::MessageDigest::sha256() + } + _ => todo!(), + }; + openssl::pkcs5::pbkdf2_hmac( + password, + pbkdf2_params.salt, + // XXX + pbkdf2_params.iteration_count.try_into().expect("XXX"), + md, + &mut key, + ) + .unwrap(); + key + } + _ => todo!(), + }; + + openssl::symm::decrypt(cipher, &key, Some(&iv), epki.encrypted_data) + .map_err(|_| KeyParsingError::IncorrectPassword)? } - AlgorithmParameters::Pbes2(_params) => { - todo!() + _ => { + return Err(KeyParsingError::UnsupportedEncryptionAlgorithm( + epki.encryption_algorithm.oid().clone(), + )) } - _ => Err(KeyParsingError::UnsupportedEncryptionAlgorithm( - epki.encryption_algorithm.oid().clone(), - )), - } + }; + + parse_private_key(&plaintext) } diff --git a/src/rust/src/error.rs b/src/rust/src/error.rs index 4b25e81a31de..761665d2859d 100644 --- a/src/rust/src/error.rs +++ b/src/rust/src/error.rs @@ -78,8 +78,20 @@ impl From for CryptographyError { exceptions::Reasons::UNSUPPORTED_ELLIPTIC_CURVE, ))) } - cryptography_key_parsing::KeyParsingError::UnsupportedEncryptionAlgorithm(_oid) => { - todo!() + cryptography_key_parsing::KeyParsingError::UnsupportedEncryptionAlgorithm(oid) => { + CryptographyError::Py(pyo3::exceptions::PyValueError::new_err(format!( + "Unknown key encryption algorithm: {oid}" + ))) + } + cryptography_key_parsing::KeyParsingError::EncryptedKeyWithoutPassword => { + CryptographyError::Py(pyo3::exceptions::PyTypeError::new_err( + "Password was not given but private key is encrypted", + )) + } + cryptography_key_parsing::KeyParsingError::IncorrectPassword => { + CryptographyError::Py(pyo3::exceptions::PyValueError::new_err( + "Incorrect password, could not decrypt key", + )) } } } From 85d3ab280237489f079bf8b54b40cb6ed0b667e9 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 21 Jan 2025 19:31:20 -0500 Subject: [PATCH 16/46] 3des decryption --- src/rust/cryptography-key-parsing/src/pkcs8.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index ea64c9cd02c6..170eb6caddca 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -156,8 +156,15 @@ pub fn parse_encrypted_private_key( } AlgorithmParameters::Pbes2(params) => { let (cipher, iv) = match params.encryption_scheme.params { - AlgorithmParameters::Aes128Cbc(iv) => (openssl::symm::Cipher::aes_128_cbc(), iv), - AlgorithmParameters::Aes256Cbc(iv) => (openssl::symm::Cipher::aes_256_cbc(), iv), + AlgorithmParameters::DesEde3Cbc(ref iv) => { + (openssl::symm::Cipher::des_ede3_cbc(), &iv[..]) + } + AlgorithmParameters::Aes128Cbc(ref iv) => { + (openssl::symm::Cipher::aes_128_cbc(), &iv[..]) + } + AlgorithmParameters::Aes256Cbc(ref iv) => { + (openssl::symm::Cipher::aes_256_cbc(), &iv[..]) + } _ => todo!(), }; @@ -187,7 +194,7 @@ pub fn parse_encrypted_private_key( _ => todo!(), }; - openssl::symm::decrypt(cipher, &key, Some(&iv), epki.encrypted_data) + openssl::symm::decrypt(cipher, &key, Some(iv), epki.encrypted_data) .map_err(|_| KeyParsingError::IncorrectPassword)? } _ => { From 9fecbe1fa651e2448babb5ab52a82c773af54779 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 22 Jan 2025 20:47:33 -0500 Subject: [PATCH 17/46] PEM decryption --- src/rust/src/backend/keys.rs | 109 +++++++++++++++++--------- src/rust/src/backend/utils.rs | 88 ++++++++------------- tests/hazmat/backends/test_openssl.py | 27 +------ 3 files changed, 106 insertions(+), 118 deletions(-) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index eb51b307eda9..9ab8c08167f2 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -70,50 +70,89 @@ fn load_pem_private_key<'p>( backend: Option>, unsafe_skip_rsa_key_validation: bool, ) -> CryptographyResult> { + let _ = backend; + let p = x509::find_in_pem( data.as_bytes(), |p| ["PRIVATE KEY", "ENCRYPTED PRIVATE KEY", "RSA PRIVATE KEY", "EC PRIVATE KEY", "DSA PRIVATE KEY"].contains(&p.tag()), "Valid PEM but no BEGIN/END delimiters for a private key found. Are you sure this is a private key?" )?; - // TODO: if proc-type is present, decrypt PEM layer. - if p.headers().get("Proc-Type").is_none() { - let pkey = match p.tag() { - "PRIVATE KEY" => cryptography_key_parsing::pkcs8::parse_private_key(p.contents())?, - "ENCRYPTED PRIVATE KEY" => { - cryptography_key_parsing::pkcs8::parse_encrypted_private_key( - p.contents(), - password.as_ref().map(|v| v.as_bytes()), - )? - } - "RSA PRIVATE KEY" => { - cryptography_key_parsing::rsa::parse_pkcs1_private_key(p.contents())? - } - "EC PRIVATE KEY" => { - cryptography_key_parsing::ec::parse_pkcs1_private_key(p.contents(), None)? - } - "DSA PRIVATE KEY" => { - cryptography_key_parsing::dsa::parse_pkcs1_private_key(p.contents())? - } - _ => unreachable!(), - }; - if password.is_some() && p.tag() != "ENCRYPTED PRIVATE KEY" { + let password = password.as_ref().map(|v| v.as_bytes()); + let mut password_used = false; + // TODO: Surely we can avoid this clone? + let tag = p.tag().to_string(); + let data = match p.headers().get("Proc-Type") { + Some("4,ENCRYPTED") => { + password_used = true; + let Some(dek_info) = p.headers().get("DEK-Info") else { + todo!() + }; + let Some((cipher_algorithm, iv)) = dek_info.split_once(',') else { + todo!() + }; + + let password = match password { + None | Some(b"") => { + return Err(CryptographyError::from( + pyo3::exceptions::PyTypeError::new_err( + "Password was not given but private key is encrypted", + ), + )) + } + Some(p) => p, + }; + + let cipher = match cipher_algorithm { + "AES-128-CBC" => openssl::symm::Cipher::aes_128_cbc(), + "AES-256-CBC" => openssl::symm::Cipher::aes_256_cbc(), + "DES-EDE3-CBC" => openssl::symm::Cipher::des_ede3_cbc(), + _ => { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err( + "Key encrypted with unknown cipher.", + ), + )) + } + }; + let iv = utils::hex_decode(iv)?; + let key = cryptography_crypto::pbkdf1::openssl_kdf( + openssl::hash::MessageDigest::md5(), + password, + &iv, + cipher.key_len(), + )?; + openssl::symm::decrypt(cipher, &key, Some(&iv), p.contents()).map_err(|_| { + pyo3::exceptions::PyValueError::new_err("Incorrect password, could not decrypt key") + })? + } + Some(_) => { return Err(CryptographyError::from( - pyo3::exceptions::PyTypeError::new_err( - "Password was given but private key is not encrypted.", + pyo3::exceptions::PyValueError::new_err( + "Proc-Type PEM header is not valid, key could not be decrypted.", ), - )); + )) } - return private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation); - } + None => p.into_contents(), + }; - let _ = backend; - let password = password.as_ref().map(CffiBuf::as_bytes); - let mut status = utils::PasswordCallbackStatus::Unused; - let pkey = openssl::pkey::PKey::private_key_from_pem_callback( - data.as_bytes(), - utils::password_callback(&mut status, password), - ); - let pkey = utils::handle_key_load_result(py, pkey, status, password)?; + let pkey = match tag.as_str() { + "PRIVATE KEY" => cryptography_key_parsing::pkcs8::parse_private_key(&data)?, + "ENCRYPTED PRIVATE KEY" => { + password_used = true; + cryptography_key_parsing::pkcs8::parse_encrypted_private_key(&data, password)? + } + "RSA PRIVATE KEY" => cryptography_key_parsing::rsa::parse_pkcs1_private_key(&data)?, + "EC PRIVATE KEY" => cryptography_key_parsing::ec::parse_pkcs1_private_key(&data, None)?, + "DSA PRIVATE KEY" => cryptography_key_parsing::dsa::parse_pkcs1_private_key(&data)?, + _ => unreachable!(), + }; + if password.is_some() && !password_used { + return Err(CryptographyError::from( + pyo3::exceptions::PyTypeError::new_err( + "Password was given but private key is not encrypted.", + ), + )); + } private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation) } diff --git a/src/rust/src/backend/utils.rs b/src/rust/src/backend/utils.rs index 7b2ceaffb6eb..03cf6008882a 100644 --- a/src/rust/src/backend/utils.rs +++ b/src/rust/src/backend/utils.rs @@ -404,66 +404,40 @@ pub(crate) fn calculate_digest_and_algorithm<'p>( Ok((data, algorithm)) } -pub(crate) enum PasswordCallbackStatus { - Unused, - Used, - BufferTooSmall(usize), -} - -pub(crate) fn password_callback<'a>( - status: &'a mut PasswordCallbackStatus, - password: Option<&'a [u8]>, -) -> impl FnOnce(&mut [u8]) -> Result + 'a { - move |buf| { - *status = PasswordCallbackStatus::Used; - match password.as_ref() { - Some(p) if p.len() <= buf.len() => { - buf[..p.len()].copy_from_slice(p); - Ok(p.len()) - } - Some(_) => { - *status = PasswordCallbackStatus::BufferTooSmall(buf.len()); - Ok(0) - } - None => Ok(0), - } +pub(crate) fn hex_decode(v: &str) -> CryptographyResult> { + if v.len() % 2 != 0 { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err("Invalid hex value - odd length"), + )); } -} -pub(crate) fn handle_key_load_result( - py: pyo3::Python<'_>, - pkey: Result, openssl::error::ErrorStack>, - status: PasswordCallbackStatus, - password: Option<&[u8]>, -) -> CryptographyResult> { - match (pkey, status, password) { - (Ok(k), PasswordCallbackStatus::Unused, None) - | (Ok(k), PasswordCallbackStatus::Used, Some(_)) => Ok(k), - - (Ok(_), PasswordCallbackStatus::Unused, Some(_)) => Err(CryptographyError::from( - pyo3::exceptions::PyTypeError::new_err( - "Password was given but private key is not encrypted.", - ), - )), + let mut b = Vec::with_capacity(v.len() / 2); + let v = v.as_bytes(); + for i in (0..v.len()).step_by(2) { + let high = match v[i] { + b @ b'0'..=b'9' => b - b'0', + b @ b'a'..=b'f' => b - b'a' + 10, + b @ b'A'..=b'F' => b - b'A' + 10, + _ => { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err("Invalid hex value"), + )) + } + }; - (_, PasswordCallbackStatus::Used, None | Some(b"")) => Err(CryptographyError::from( - pyo3::exceptions::PyTypeError::new_err( - "Password was not given but private key is encrypted", - ), - )), - (_, PasswordCallbackStatus::BufferTooSmall(size), _) => Err(CryptographyError::from( - pyo3::exceptions::PyValueError::new_err(format!( - "Passwords longer than {size} bytes are not supported" - )), - )), - (Err(e), _, _) => { - let errors = error::list_from_openssl_error(py, &e); - Err(CryptographyError::from( - pyo3::exceptions::PyValueError::new_err(( - "Could not deserialize key data. The data may be in an incorrect format, the provided password may be incorrect, it may be encrypted with an unsupported algorithm, or it may be an unsupported key type (e.g. EC curves with explicit parameters).", - errors.unbind(), + let low = match v[i + 1] { + b @ b'0'..=b'9' => b - b'0', + b @ b'a'..=b'f' => b - b'a' + 10, + b @ b'A'..=b'F' => b - b'A' + 10, + _ => { + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err("Invalid hex value"), )) - )) - } + } + }; + + b.push((high << 4) | low); } + + Ok(b) } diff --git a/tests/hazmat/backends/test_openssl.py b/tests/hazmat/backends/test_openssl.py index f43f03435539..9a5d4f60e6ac 100644 --- a/tests/hazmat/backends/test_openssl.py +++ b/tests/hazmat/backends/test_openssl.py @@ -4,7 +4,6 @@ import itertools -import os import pytest @@ -22,10 +21,7 @@ DummyMode, ) from ...hazmat.primitives.test_rsa import rsa_key_2048 -from ...utils import ( - load_vectors_from_file, - raises_unsupported_algorithm, -) +from ...utils import raises_unsupported_algorithm # Make ruff happy since we're importing fixtures that pytest patches in as # func args @@ -200,27 +196,6 @@ def test_unsupported_mgf1_hash_algorithm_md5_decrypt(self, rsa_key_2048): ) -class TestOpenSSLSerializationWithOpenSSL: - def test_very_long_pem_serialization_password(self): - password = b"x" * 1025 - - with pytest.raises(ValueError, match="Passwords longer than"): - load_vectors_from_file( - os.path.join( - "asymmetric", - "Traditional_OpenSSL_Serialization", - "key1.pem", - ), - lambda pemfile: ( - serialization.load_pem_private_key( - pemfile.read().encode(), - password, - unsafe_skip_rsa_key_validation=False, - ) - ), - ) - - class TestRSAPEMSerialization: def test_password_length_limit(self, rsa_key_2048): password = b"x" * 1024 From b943e3b0492ccc26c15c80ef24c12e18d8bfe365 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 22 Jan 2025 20:48:04 -0500 Subject: [PATCH 18/46] forgotten file --- src/rust/cryptography-crypto/src/pbkdf1.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rust/cryptography-crypto/src/pbkdf1.rs b/src/rust/cryptography-crypto/src/pbkdf1.rs index 9a0cda4a116a..c16d2bec5dbf 100644 --- a/src/rust/cryptography-crypto/src/pbkdf1.rs +++ b/src/rust/cryptography-crypto/src/pbkdf1.rs @@ -12,7 +12,6 @@ pub fn openssl_kdf( ) -> Result, openssl::error::ErrorStack> { let mut key = Vec::with_capacity(length); - while key.len() < length { let mut h = openssl::hash::Hasher::new(hash_alg)?; if !key.is_empty() { From c9d253513a78c6a92867bc04a9d9e2487fc2b39a Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Thu, 23 Jan 2025 08:33:48 -0500 Subject: [PATCH 19/46] cleanup --- src/rust/src/backend/keys.rs | 4 +++- src/rust/src/backend/utils.rs | 20 +++++--------------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 9ab8c08167f2..cce8bc440ac1 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -114,7 +114,9 @@ fn load_pem_private_key<'p>( )) } }; - let iv = utils::hex_decode(iv)?; + let iv = utils::hex_decode(iv).ok_or_else(|| { + pyo3::exceptions::PyValueError::new_err("DEK-Info IV is not valid hex") + })?; let key = cryptography_crypto::pbkdf1::openssl_kdf( openssl::hash::MessageDigest::md5(), password, diff --git a/src/rust/src/backend/utils.rs b/src/rust/src/backend/utils.rs index 03cf6008882a..139e711f3883 100644 --- a/src/rust/src/backend/utils.rs +++ b/src/rust/src/backend/utils.rs @@ -404,11 +404,9 @@ pub(crate) fn calculate_digest_and_algorithm<'p>( Ok((data, algorithm)) } -pub(crate) fn hex_decode(v: &str) -> CryptographyResult> { +pub(crate) fn hex_decode(v: &str) -> Option> { if v.len() % 2 != 0 { - return Err(CryptographyError::from( - pyo3::exceptions::PyValueError::new_err("Invalid hex value - odd length"), - )); + return None; } let mut b = Vec::with_capacity(v.len() / 2); @@ -418,26 +416,18 @@ pub(crate) fn hex_decode(v: &str) -> CryptographyResult> { b @ b'0'..=b'9' => b - b'0', b @ b'a'..=b'f' => b - b'a' + 10, b @ b'A'..=b'F' => b - b'A' + 10, - _ => { - return Err(CryptographyError::from( - pyo3::exceptions::PyValueError::new_err("Invalid hex value"), - )) - } + _ => return None, }; let low = match v[i + 1] { b @ b'0'..=b'9' => b - b'0', b @ b'a'..=b'f' => b - b'a' + 10, b @ b'A'..=b'F' => b - b'A' + 10, - _ => { - return Err(CryptographyError::from( - pyo3::exceptions::PyValueError::new_err("Invalid hex value"), - )) - } + _ => return None, }; b.push((high << 4) | low); } - Ok(b) + Some(b) } From ca3c32e613daa489fedc73a2379590ba0454aa9a Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Thu, 23 Jan 2025 16:29:19 -0500 Subject: [PATCH 20/46] Fix FIPS? --- src/rust/src/backend/keys.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index cce8bc440ac1..46c7efb462fb 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -122,7 +122,12 @@ fn load_pem_private_key<'p>( password, &iv, cipher.key_len(), - )?; + ) + .map_err(|_| { + pyo3::exceptions::PyValueError::new_err( + "Unable to derive key from password (are you in FIPS mode?)", + ) + })?; openssl::symm::decrypt(cipher, &key, Some(&iv), p.contents()).map_err(|_| { pyo3::exceptions::PyValueError::new_err("Incorrect password, could not decrypt key") })? From cd5086f9b5bc71d3183d351f14c7345d441a05dc Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Thu, 23 Jan 2025 18:22:32 -0500 Subject: [PATCH 21/46] Unit tests for hex_decode --- src/rust/src/backend/keys.rs | 14 -------------- src/rust/src/backend/utils.rs | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 46c7efb462fb..82e2b0b49cc8 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -175,20 +175,6 @@ fn private_key_from_pkey<'p>( )? .into_pyobject(py)? .into_any()), - openssl::pkey::Id::RSA_PSS => { - // At the moment the way we handle RSA PSS keys is to strip the - // PSS constraints from them and treat them as normal RSA keys - // Unfortunately the RSA * itself tracks this data so we need to - // extract, serialize, and reload it without the constraints. - let der_bytes = pkey.rsa()?.private_key_to_der()?; - let rsa = openssl::rsa::Rsa::private_key_from_der(&der_bytes)?; - let pkey = openssl::pkey::PKey::from_rsa(rsa)?; - Ok( - crate::backend::rsa::private_key_from_pkey(&pkey, unsafe_skip_rsa_key_validation)? - .into_pyobject(py)? - .into_any(), - ) - } openssl::pkey::Id::EC => Ok(crate::backend::ec::private_key_from_pkey(py, pkey)? .into_pyobject(py)? .into_any()), diff --git a/src/rust/src/backend/utils.rs b/src/rust/src/backend/utils.rs index 139e711f3883..f7dca709c261 100644 --- a/src/rust/src/backend/utils.rs +++ b/src/rust/src/backend/utils.rs @@ -431,3 +431,21 @@ pub(crate) fn hex_decode(v: &str) -> Option> { Some(b) } + +#[cfg(test)] +mod tests { + use super::hex_decode; + + #[test] + fn test_hex_decode() { + for (text, expected) in [ + ("", Some(vec![])), + ("00", Some(vec![0])), + ("0", None), + ("12-0", None), + ("120-", None), + ] { + assert_eq!(hex_decode(text), expected); + } + } +} From 1813a6982f2174f8e609e59535fcdd7f51ccc7bb Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Thu, 23 Jan 2025 18:31:20 -0500 Subject: [PATCH 22/46] Another test --- src/rust/src/backend/utils.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rust/src/backend/utils.rs b/src/rust/src/backend/utils.rs index f7dca709c261..9d2490f0e7bf 100644 --- a/src/rust/src/backend/utils.rs +++ b/src/rust/src/backend/utils.rs @@ -444,6 +444,8 @@ mod tests { ("0", None), ("12-0", None), ("120-", None), + ("ab", Some(vec![0xAB])), + ("AB", Some(vec![0xAB])), ] { assert_eq!(hex_decode(text), expected); } From d6775f809352162ccad021d48a30e6a930d31c68 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 24 Jan 2025 14:09:02 -0500 Subject: [PATCH 23/46] replace todos with code where we have tests --- src/rust/cryptography-key-parsing/src/pkcs8.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index 170eb6caddca..7139a4ec7b6e 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -165,7 +165,11 @@ pub fn parse_encrypted_private_key( AlgorithmParameters::Aes256Cbc(ref iv) => { (openssl::symm::Cipher::aes_256_cbc(), &iv[..]) } - _ => todo!(), + _ => { + return Err(KeyParsingError::UnsupportedEncryptionAlgorithm( + params.encryption_scheme.oid().clone(), + )) + } }; let key = match params.key_derivation_func.params { @@ -178,7 +182,11 @@ pub fn parse_encrypted_private_key( AlgorithmParameters::HmacWithSha256(_) => { openssl::hash::MessageDigest::sha256() } - _ => todo!(), + _ => { + return Err(KeyParsingError::UnsupportedEncryptionAlgorithm( + pbkdf2_params.prf.oid().clone(), + )) + } }; openssl::pkcs5::pbkdf2_hmac( password, @@ -191,7 +199,11 @@ pub fn parse_encrypted_private_key( .unwrap(); key } - _ => todo!(), + _ => { + return Err(KeyParsingError::UnsupportedEncryptionAlgorithm( + params.key_derivation_func.oid().clone(), + )) + } }; openssl::symm::decrypt(cipher, &key, Some(iv), epki.encrypted_data) From a9acbbf49ef747bf5b59adff6cfebc855d32cd99 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 24 Jan 2025 14:45:07 -0500 Subject: [PATCH 24/46] roundtrip PKCS#12 through us --- src/rust/src/backend/keys.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 82e2b0b49cc8..efab89001870 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -58,6 +58,7 @@ pub(crate) fn load_der_private_key_bytes<'p>( } let pkey = cryptography_key_parsing::pkcs8::parse_encrypted_private_key(data, password)?; + private_key_from_pkey(py, &pkey, unsafe_skip_rsa_key_validation) } From 74f62d266d634dc444fda0423c97ee46814c843e Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 24 Jan 2025 19:14:41 -0500 Subject: [PATCH 25/46] pass PEM tests --- src/rust/src/backend/keys.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index efab89001870..e62fc1c2d3c1 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -86,10 +86,18 @@ fn load_pem_private_key<'p>( Some("4,ENCRYPTED") => { password_used = true; let Some(dek_info) = p.headers().get("DEK-Info") else { - todo!() + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err( + "Encrypted PEM doesn't have a DEK-Info header.", + ), + )); }; let Some((cipher_algorithm, iv)) = dek_info.split_once(',') else { - todo!() + return Err(CryptographyError::from( + pyo3::exceptions::PyValueError::new_err( + "Encrypted PEM's DEK-Info header is not valid.", + ), + )); }; let password = match password { From 89909074c8b34f35e794624ab1c67e5697ba4f4e Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 24 Jan 2025 19:16:39 -0500 Subject: [PATCH 26/46] Remove unreachable code --- src/rust/src/backend/ec.rs | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/rust/src/backend/ec.rs b/src/rust/src/backend/ec.rs index 2d0d781c0a34..7c42a5d9bf87 100644 --- a/src/rust/src/backend/ec.rs +++ b/src/rust/src/backend/ec.rs @@ -5,7 +5,7 @@ use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; -use pyo3::types::{PyAnyMethods, PyDictMethods}; +use pyo3::types::PyAnyMethods; use crate::backend::utils; use crate::buf::CffiBuf; @@ -97,26 +97,11 @@ fn py_curve_from_curve<'p>( py: pyo3::Python<'p>, curve: &openssl::ec::EcGroupRef, ) -> CryptographyResult> { - if curve.asn1_flag() == openssl::ec::Asn1Flag::EXPLICIT_CURVE { - return Err(CryptographyError::from( - pyo3::exceptions::PyValueError::new_err( - "ECDSA keys with explicit parameters are unsupported at this time", - ), - )); - } + assert!(curve.asn1_flag() != openssl::ec::Asn1Flag::EXPLICIT_CURVE); let name = curve.curve_name().unwrap().short_name()?; - types::CURVE_TYPES - .get(py)? - .extract::>()? - .get_item(name)? - .ok_or_else(|| { - CryptographyError::from(exceptions::UnsupportedAlgorithm::new_err(( - format!("{name} is not a supported elliptic curve"), - exceptions::Reasons::UNSUPPORTED_ELLIPTIC_CURVE, - ))) - }) + Ok(types::CURVE_TYPES.get(py)?.get_item(name)?) } fn check_key_infinity( From f7796c864906518f4fe828279279f3c5aa307f66 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 24 Jan 2025 19:25:18 -0500 Subject: [PATCH 27/46] for coverage --- src/rust/src/backend/keys.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index e62fc1c2d3c1..5ed3b1f38bc5 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -153,14 +153,14 @@ fn load_pem_private_key<'p>( let pkey = match tag.as_str() { "PRIVATE KEY" => cryptography_key_parsing::pkcs8::parse_private_key(&data)?, - "ENCRYPTED PRIVATE KEY" => { - password_used = true; - cryptography_key_parsing::pkcs8::parse_encrypted_private_key(&data, password)? - } "RSA PRIVATE KEY" => cryptography_key_parsing::rsa::parse_pkcs1_private_key(&data)?, "EC PRIVATE KEY" => cryptography_key_parsing::ec::parse_pkcs1_private_key(&data, None)?, "DSA PRIVATE KEY" => cryptography_key_parsing::dsa::parse_pkcs1_private_key(&data)?, - _ => unreachable!(), + _ => { + assert_eq!(tag, "ENCRYPTED PRIVATE KEY"); + password_used = true; + cryptography_key_parsing::pkcs8::parse_encrypted_private_key(&data, password)? + } }; if password.is_some() && !password_used { return Err(CryptographyError::from( From ad269f91c5c257a0a6bec7addc91ada274d238b4 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sat, 25 Jan 2025 18:52:45 -0500 Subject: [PATCH 28/46] paramiko branch --- .github/downstream.d/paramiko.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/downstream.d/paramiko.sh b/.github/downstream.d/paramiko.sh index 82ab9c1f9748..24d34139777b 100755 --- a/.github/downstream.d/paramiko.sh +++ b/.github/downstream.d/paramiko.sh @@ -2,7 +2,7 @@ case "${1}" in install) - git clone --depth=1 https://github.com/paramiko/paramiko + git clone --depth=1 --branch=unpad-keys https://github.com/alex/paramiko cd paramiko git rev-parse HEAD pip install -e . From f371eaa71975c1898351a9e0242c8d7c76440bca Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Mon, 27 Jan 2025 09:57:42 -0500 Subject: [PATCH 29/46] oops --- src/rust/cryptography-key-parsing/src/ec.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rust/cryptography-key-parsing/src/ec.rs b/src/rust/cryptography-key-parsing/src/ec.rs index 458fd5744d28..099005480fa2 100644 --- a/src/rust/cryptography-key-parsing/src/ec.rs +++ b/src/rust/cryptography-key-parsing/src/ec.rs @@ -105,4 +105,3 @@ pub fn parse_pkcs1_private_key( .map_err(|_| KeyParsingError::InvalidKey)?; Ok(openssl::pkey::PKey::from_ec_key(ec_key)?) } ->>>>>>> 1a8c3d5ef (Migrate parsing of unencrypted PKCS#8 private keys to Rust) From a83e1fadb548341b3df76c408f88822d14c449bd Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 31 Jan 2025 09:30:19 -0800 Subject: [PATCH 30/46] cleanups and fixes --- src/rust/cryptography-crypto/src/pbkdf1.rs | 1 + src/rust/cryptography-key-parsing/src/pkcs8.rs | 12 +++++++----- src/rust/src/backend/keys.rs | 9 ++++++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/rust/cryptography-crypto/src/pbkdf1.rs b/src/rust/cryptography-crypto/src/pbkdf1.rs index c16d2bec5dbf..9a0cda4a116a 100644 --- a/src/rust/cryptography-crypto/src/pbkdf1.rs +++ b/src/rust/cryptography-crypto/src/pbkdf1.rs @@ -12,6 +12,7 @@ pub fn openssl_kdf( ) -> Result, openssl::error::ErrorStack> { let mut key = Vec::with_capacity(length); + while key.len() < length { let mut h = openssl::hash::Hasher::new(hash_alg)?; if !key.is_empty() { diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index 7139a4ec7b6e..d29b521b0ff0 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -126,9 +126,9 @@ pub fn parse_encrypted_private_key( let plaintext = match epki.encryption_algorithm.params { AlgorithmParameters::Pbes1WithShaAnd3KeyTripleDesCbc(params) => { - // XXX: - // - handle invalid utf8 - let password = std::str::from_utf8(password).unwrap(); + let Ok(password) = std::str::from_utf8(password) else { + return Err(KeyParsingError::IncorrectPassword); + }; let key = cryptography_crypto::pkcs12::kdf( password, ¶ms.salt, @@ -191,8 +191,10 @@ pub fn parse_encrypted_private_key( openssl::pkcs5::pbkdf2_hmac( password, pbkdf2_params.salt, - // XXX - pbkdf2_params.iteration_count.try_into().expect("XXX"), + pbkdf2_params + .iteration_count + .try_into() + .map_err(|_| KeyParsingError::InvalidKey)?, md, &mut key, ) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 5ed3b1f38bc5..526c3010a14e 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -129,7 +129,14 @@ fn load_pem_private_key<'p>( let key = cryptography_crypto::pbkdf1::openssl_kdf( openssl::hash::MessageDigest::md5(), password, - &iv, + iv.get(..8) + .ok_or_else(|| { + pyo3::exceptions::PyValueError::new_err( + "DEK-Info IV must be at least 8 bytes", + ) + })? + .try_into() + .unwrap(), cipher.key_len(), ) .map_err(|_| { From 2c23cc5014fe7d25122c51879933bb2f9f6afb01 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 31 Jan 2025 09:36:11 -0800 Subject: [PATCH 31/46] cleanup --- src/rust/src/backend/keys.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 526c3010a14e..072b3cf4bdea 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -35,18 +35,12 @@ pub(crate) fn load_der_private_key_bytes<'p>( password: Option<&[u8]>, unsafe_skip_rsa_key_validation: bool, ) -> CryptographyResult> { - let pkey = if let Ok(pkey) = cryptography_key_parsing::pkcs8::parse_private_key(data) { - Some(pkey) - } else if let Ok(pkey) = cryptography_key_parsing::dsa::parse_pkcs1_private_key(data) { - Some(pkey) - } else if let Ok(pkey) = cryptography_key_parsing::ec::parse_pkcs1_private_key(data, None) { - Some(pkey) - } else if let Ok(pkey) = cryptography_key_parsing::rsa::parse_pkcs1_private_key(data) { - Some(pkey) - } else { - None - }; - if let Some(pkey) = pkey { + let pkey = cryptography_key_parsing::pkcs8::parse_private_key(data) + .or_else(|_| cryptography_key_parsing::dsa::parse_pkcs1_private_key(data)) + .or_else(|_| cryptography_key_parsing::ec::parse_pkcs1_private_key(data, None)) + .or_else(|_| cryptography_key_parsing::rsa::parse_pkcs1_private_key(data)); + + if let Ok(pkey) = pkey { if password.is_some() { return Err(CryptographyError::from( pyo3::exceptions::PyTypeError::new_err( From b2ec075fb663d4437efe0d2a327566e88e0127de Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 31 Jan 2025 15:56:52 -0800 Subject: [PATCH 32/46] cleanup --- src/rust/src/backend/keys.rs | 3 +-- src/rust/src/backend/utils.rs | 48 ----------------------------------- 2 files changed, 1 insertion(+), 50 deletions(-) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 072b3cf4bdea..38609a1da46a 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -4,7 +4,6 @@ use pyo3::IntoPyObject; -use crate::backend::utils; use crate::buf::CffiBuf; use crate::error::{CryptographyError, CryptographyResult}; use crate::exceptions; @@ -117,7 +116,7 @@ fn load_pem_private_key<'p>( )) } }; - let iv = utils::hex_decode(iv).ok_or_else(|| { + let iv = cryptography_crypto::encoding::hex_decode(iv).ok_or_else(|| { pyo3::exceptions::PyValueError::new_err("DEK-Info IV is not valid hex") })?; let key = cryptography_crypto::pbkdf1::openssl_kdf( diff --git a/src/rust/src/backend/utils.rs b/src/rust/src/backend/utils.rs index 9d2490f0e7bf..9ba8eda68ef2 100644 --- a/src/rust/src/backend/utils.rs +++ b/src/rust/src/backend/utils.rs @@ -403,51 +403,3 @@ pub(crate) fn calculate_digest_and_algorithm<'p>( Ok((data, algorithm)) } - -pub(crate) fn hex_decode(v: &str) -> Option> { - if v.len() % 2 != 0 { - return None; - } - - let mut b = Vec::with_capacity(v.len() / 2); - let v = v.as_bytes(); - for i in (0..v.len()).step_by(2) { - let high = match v[i] { - b @ b'0'..=b'9' => b - b'0', - b @ b'a'..=b'f' => b - b'a' + 10, - b @ b'A'..=b'F' => b - b'A' + 10, - _ => return None, - }; - - let low = match v[i + 1] { - b @ b'0'..=b'9' => b - b'0', - b @ b'a'..=b'f' => b - b'a' + 10, - b @ b'A'..=b'F' => b - b'A' + 10, - _ => return None, - }; - - b.push((high << 4) | low); - } - - Some(b) -} - -#[cfg(test)] -mod tests { - use super::hex_decode; - - #[test] - fn test_hex_decode() { - for (text, expected) in [ - ("", Some(vec![])), - ("00", Some(vec![0])), - ("0", None), - ("12-0", None), - ("120-", None), - ("ab", Some(vec![0xAB])), - ("AB", Some(vec![0xAB])), - ] { - assert_eq!(hex_decode(text), expected); - } - } -} From 989964e09de9fe6066c82bd1de837d285b94ae18 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Sat, 1 Feb 2025 16:50:54 -0800 Subject: [PATCH 33/46] fix --- src/rust/src/backend/utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rust/src/backend/utils.rs b/src/rust/src/backend/utils.rs index 9ba8eda68ef2..d61466da4870 100644 --- a/src/rust/src/backend/utils.rs +++ b/src/rust/src/backend/utils.rs @@ -6,7 +6,7 @@ use pyo3::types::{PyAnyMethods, PyBytesMethods}; use crate::backend::hashes::Hash; use crate::error::{CryptographyError, CryptographyResult}; -use crate::{error, types}; +use crate::types; pub(crate) fn py_int_to_bn( py: pyo3::Python<'_>, From 853c67a9bb7403f6295cb84b66d2eb40d4e7ccff Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Mon, 3 Feb 2025 18:40:09 -0800 Subject: [PATCH 34/46] revert that --- .github/downstream.d/paramiko.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/downstream.d/paramiko.sh b/.github/downstream.d/paramiko.sh index 24d34139777b..82ab9c1f9748 100755 --- a/.github/downstream.d/paramiko.sh +++ b/.github/downstream.d/paramiko.sh @@ -2,7 +2,7 @@ case "${1}" in install) - git clone --depth=1 --branch=unpad-keys https://github.com/alex/paramiko + git clone --depth=1 https://github.com/paramiko/paramiko cd paramiko git rev-parse HEAD pip install -e . From 47eac4d240eaac3f485aa2de29b270e94b62d4b0 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 4 Feb 2025 10:58:01 -0800 Subject: [PATCH 35/46] changelog --- CHANGELOG.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8c68083b1995..86397d317769 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -19,6 +19,13 @@ Changelog provided (previously no exception was raised), and raises a ``TypeError`` if the key is encrypted but no password is provided (previously a ``ValueError`` was raised). +* We significantly refactored how private key loading ( + :func:`~cryptography.hazmat.primitives.serialization.load_pem_private_key` + and + :func:`~cryptography.hazmat.primitives.serialization.load_der_private_key`) + works. This is intended to be backwards compatible for all well-formed keys, + therefore if you discover a key that now raises an exception, please file a + bug with instructions for reproducing. * Added ``unsafe_skip_rsa_key_validation`` keyword-argument to :func:`~cryptography.hazmat.primitives.serialization.load_ssh_private_key`. * Added :class:`~cryptography.hazmat.primitives.hashes.XOFHash` to support From f485b93a38d3294f72d197ee6c7b4f610241a827 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 4 Feb 2025 15:09:28 -0800 Subject: [PATCH 36/46] Support a variety of additional encryption algorithms --- .../cryptography-key-parsing/src/pkcs8.rs | 112 +++++++++++++----- src/rust/cryptography-x509/src/common.rs | 23 ++++ 2 files changed, 105 insertions(+), 30 deletions(-) diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index d29b521b0ff0..0d1183a9f66b 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -2,7 +2,7 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. -use cryptography_x509::common::{AlgorithmIdentifier, AlgorithmParameters}; +use cryptography_x509::common::{AlgorithmIdentifier, AlgorithmParameters, PBES1Params}; use cryptography_x509::csr::Attributes; use cryptography_x509::pkcs8::EncryptedPrivateKeyInfo; @@ -114,6 +114,37 @@ pub fn parse_private_key( } } +fn pbes1_decrypt( + data: &[u8], + password: &[u8], + cipher: openssl::symm::Cipher, + hash: openssl::hash::MessageDigest, + params: &PBES1Params, +) -> KeyParsingResult> { + let Ok(password) = std::str::from_utf8(password) else { + return Err(KeyParsingError::IncorrectPassword); + }; + let key = cryptography_crypto::pkcs12::kdf( + password, + ¶ms.salt, + cryptography_crypto::pkcs12::KDF_ENCRYPTION_KEY_ID, + params.iterations, + cipher.key_len(), + hash, + )?; + let iv = cryptography_crypto::pkcs12::kdf( + password, + ¶ms.salt, + cryptography_crypto::pkcs12::KDF_IV_ID, + params.iterations, + cipher.block_size(), + hash, + )?; + + openssl::symm::decrypt(cipher, &key, Some(&iv), data) + .map_err(|_| KeyParsingError::IncorrectPassword) +} + pub fn parse_encrypted_private_key( data: &[u8], password: Option<&[u8]>, @@ -125,35 +156,20 @@ pub fn parse_encrypted_private_key( }; let plaintext = match epki.encryption_algorithm.params { - AlgorithmParameters::Pbes1WithShaAnd3KeyTripleDesCbc(params) => { - let Ok(password) = std::str::from_utf8(password) else { - return Err(KeyParsingError::IncorrectPassword); - }; - let key = cryptography_crypto::pkcs12::kdf( - password, - ¶ms.salt, - cryptography_crypto::pkcs12::KDF_ENCRYPTION_KEY_ID, - params.iterations, - 24, - openssl::hash::MessageDigest::sha1(), - )?; - let iv = cryptography_crypto::pkcs12::kdf( - password, - ¶ms.salt, - cryptography_crypto::pkcs12::KDF_IV_ID, - params.iterations, - 8, - openssl::hash::MessageDigest::sha1(), - )?; - - openssl::symm::decrypt( - openssl::symm::Cipher::des_ede3_cbc(), - &key, - Some(&iv), - epki.encrypted_data, - ) - .map_err(|_| KeyParsingError::IncorrectPassword)? - } + AlgorithmParameters::Pbes1WithShaAnd3KeyTripleDesCbc(params) => pbes1_decrypt( + epki.encrypted_data, + password, + openssl::symm::Cipher::des_ede3_cbc(), + openssl::hash::MessageDigest::sha1(), + ¶ms, + )?, + AlgorithmParameters::Pbe1WithShaAnd40BitRc2Cbc(params) => pbes1_decrypt( + epki.encrypted_data, + password, + openssl::symm::Cipher::from_nid(openssl::nid::Nid::RC2_40_CBC).unwrap(), + openssl::hash::MessageDigest::sha1(), + ¶ms, + )?, AlgorithmParameters::Pbes2(params) => { let (cipher, iv) = match params.encryption_scheme.params { AlgorithmParameters::DesEde3Cbc(ref iv) => { @@ -162,9 +178,23 @@ pub fn parse_encrypted_private_key( AlgorithmParameters::Aes128Cbc(ref iv) => { (openssl::symm::Cipher::aes_128_cbc(), &iv[..]) } + AlgorithmParameters::Aes192Cbc(ref iv) => { + (openssl::symm::Cipher::aes_192_cbc(), &iv[..]) + } AlgorithmParameters::Aes256Cbc(ref iv) => { (openssl::symm::Cipher::aes_256_cbc(), &iv[..]) } + AlgorithmParameters::Rc2Cbc(ref params) => { + // A version of 58 == 128 bits effective key length. The + // default is 32. See RFC 8018 B.2.3. + if params.version.unwrap_or(32) != 58 { + return Err(KeyParsingError::InvalidKey); + } + ( + openssl::symm::Cipher::from_nid(openssl::nid::Nid::RC2_CBC).unwrap(), + ¶ms.iv[..], + ) + } _ => { return Err(KeyParsingError::UnsupportedEncryptionAlgorithm( params.encryption_scheme.oid().clone(), @@ -179,9 +209,18 @@ pub fn parse_encrypted_private_key( AlgorithmParameters::HmacWithSha1(_) => { openssl::hash::MessageDigest::sha1() } + AlgorithmParameters::HmacWithSha224(_) => { + openssl::hash::MessageDigest::sha224() + } AlgorithmParameters::HmacWithSha256(_) => { openssl::hash::MessageDigest::sha256() } + AlgorithmParameters::HmacWithSha384(_) => { + openssl::hash::MessageDigest::sha384() + } + AlgorithmParameters::HmacWithSha512(_) => { + openssl::hash::MessageDigest::sha512() + } _ => { return Err(KeyParsingError::UnsupportedEncryptionAlgorithm( pbkdf2_params.prf.oid().clone(), @@ -201,6 +240,19 @@ pub fn parse_encrypted_private_key( .unwrap(); key } + AlgorithmParameters::Scrypt(scrypt_params) => { + let mut key = vec![0; cipher.key_len()]; + openssl::pkcs5::scrypt( + password, + scrypt_params.salt, + scrypt_params.cost_parameter, + scrypt_params.block_size, + scrypt_params.parallelization_parameter, + (usize::MAX / 2).try_into().unwrap(), + &mut key, + )?; + key + } _ => { return Err(KeyParsingError::UnsupportedEncryptionAlgorithm( params.key_derivation_func.oid().clone(), diff --git a/src/rust/cryptography-x509/src/common.rs b/src/rust/cryptography-x509/src/common.rs index be0dd929c8fa..af4188f96e82 100644 --- a/src/rust/cryptography-x509/src/common.rs +++ b/src/rust/cryptography-x509/src/common.rs @@ -130,6 +130,8 @@ pub enum AlgorithmParameters<'a> { #[defined_by(oid::PBKDF2_OID)] Pbkdf2(PBKDF2Params<'a>), + #[defined_by(oid::SCRYPT_OID)] + Scrypt(ScryptParams<'a>), #[defined_by(oid::HMAC_WITH_SHA1_OID)] HmacWithSha1(Option), @@ -152,12 +154,17 @@ pub enum AlgorithmParameters<'a> { // AES-IV ::= OCTET STRING (SIZE(16)) #[defined_by(oid::AES_128_CBC_OID)] Aes128Cbc([u8; 16]), + #[defined_by(oid::AES_192_CBC_OID)] + Aes192Cbc([u8; 16]), #[defined_by(oid::AES_256_CBC_OID)] Aes256Cbc([u8; 16]), #[defined_by(oid::DES_EDE3_CBC_OID)] DesEde3Cbc([u8; 8]), + #[defined_by(oid::RC2_CBC)] + Rc2Cbc(Rc2CbcParams), + #[defined_by(oid::PBES1_WITH_SHA_AND_3KEY_TRIPLEDES_CBC)] Pbes1WithShaAnd3KeyTripleDesCbc(PBES1Params), #[defined_by(oid::PBES1_WITH_SHA_AND_40_BIT_RC2_CBC)] @@ -491,12 +498,28 @@ pub struct PBKDF2Params<'a> { pub prf: Box>, } +// RFC 7914 Section 7 +#[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Eq, Hash, Clone, Debug)] +pub struct ScryptParams<'a> { + pub salt: &'a [u8], + pub cost_parameter: u64, + pub block_size: u64, + pub parallelization_parameter: u64, + pub key_length: Option, +} + #[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Eq, Hash, Clone, Debug)] pub struct PBES1Params { pub salt: [u8; 8], pub iterations: u64, } +#[derive(asn1::Asn1Read, asn1::Asn1Write, PartialEq, Eq, Hash, Clone, Debug)] +pub struct Rc2CbcParams { + pub version: Option, + pub iv: [u8; 8], +} + /// A VisibleString ASN.1 element whose contents is not validated as meeting the /// requirements (visible characters of IA5), and instead is only known to be /// valid UTF-8. From 07c483d2202e590c8bb475236eb225f68274507e Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 4 Feb 2025 15:12:06 -0800 Subject: [PATCH 37/46] scrypt == no libressl --- src/rust/cryptography-key-parsing/src/pkcs8.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index 0d1183a9f66b..774ce1592539 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -240,6 +240,7 @@ pub fn parse_encrypted_private_key( .unwrap(); key } + #[cfg(not(CRYPTOGRAPHY_IS_LIBRESSL))] AlgorithmParameters::Scrypt(scrypt_params) => { let mut key = vec![0; cipher.key_len()]; openssl::pkcs5::scrypt( From 1dc142254c883749751caa8dba52e1b44c625bea Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 5 Feb 2025 14:12:02 -0800 Subject: [PATCH 38/46] remove xfail --- .../cryptography-key-parsing/src/pkcs8.rs | 2 ++ tests/hazmat/primitives/test_serialization.py | 25 ------------------- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index 774ce1592539..f69660daae22 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -166,6 +166,7 @@ pub fn parse_encrypted_private_key( AlgorithmParameters::Pbe1WithShaAnd40BitRc2Cbc(params) => pbes1_decrypt( epki.encrypted_data, password, + // XXX: unwrwap openssl::symm::Cipher::from_nid(openssl::nid::Nid::RC2_40_CBC).unwrap(), openssl::hash::MessageDigest::sha1(), ¶ms, @@ -191,6 +192,7 @@ pub fn parse_encrypted_private_key( return Err(KeyParsingError::InvalidKey); } ( + // XXX: unwrap openssl::symm::Cipher::from_nid(openssl::nid::Nid::RC2_CBC).unwrap(), ¶ms.iv[..], ) diff --git a/tests/hazmat/primitives/test_serialization.py b/tests/hazmat/primitives/test_serialization.py index 410d8e6df79d..3228f307a28d 100644 --- a/tests/hazmat/primitives/test_serialization.py +++ b/tests/hazmat/primitives/test_serialization.py @@ -463,11 +463,6 @@ def test_load_pkcs8_private_key_unknown_kdf(self): with pytest.raises(ValueError): load_pem_private_key(data, password=b"password") - @pytest.mark.xfail( - rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL, - strict=True, - reason="Temp fail on boring", - ) @pytest.mark.parametrize( "filename", [ @@ -486,11 +481,6 @@ def test_load_pkscs8_pbkdf_prf(self, filename: str): assert isinstance(key, rsa.RSAPrivateKey) assert key.key_size == 2048 - @pytest.mark.xfail( - rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL, - strict=True, - reason="Temp fail on boring", - ) @pytest.mark.supported( only_if=lambda backend: backend.cipher_supported( RC2(b"\x00" * 16), modes.CBC(b"\x00" * 8) @@ -506,11 +496,6 @@ def test_load_pkcs8_40_bit_rc2(self): assert isinstance(key, rsa.RSAPrivateKey) assert key.key_size == 1024 - @pytest.mark.xfail( - rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL, - strict=True, - reason="Temp fail on boring", - ) @pytest.mark.supported( only_if=lambda backend: backend.cipher_supported( RC2(b"\x00" * 16), modes.CBC(b"\x00" * 8) @@ -545,11 +530,6 @@ def test_load_pkcs8_rc2_cbc_effective_key_length(self): with pytest.raises(ValueError): load_pem_private_key(data, password=b"password") - @pytest.mark.xfail( - rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL, - strict=True, - reason="Temp fail on boring", - ) def test_load_pkcs8_aes_192_cbc(self): key = load_vectors_from_file( os.path.join("asymmetric", "PKCS8", "rsa-aes-192-cbc.pem"), @@ -559,11 +539,6 @@ def test_load_pkcs8_aes_192_cbc(self): assert isinstance(key, rsa.RSAPrivateKey) assert key.key_size == 2048 - @pytest.mark.xfail( - rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL, - strict=True, - reason="Temp fail on boring", - ) @pytest.mark.supported( only_if=lambda backend: backend.scrypt_supported(), skip_message="Scrypt required", From ce58b0e4b56a5d09cba0f8668180de12f7abb84a Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 5 Feb 2025 14:14:32 -0800 Subject: [PATCH 39/46] unused --- tests/hazmat/primitives/test_serialization.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/hazmat/primitives/test_serialization.py b/tests/hazmat/primitives/test_serialization.py index 3228f307a28d..feebca359736 100644 --- a/tests/hazmat/primitives/test_serialization.py +++ b/tests/hazmat/primitives/test_serialization.py @@ -10,7 +10,6 @@ import pytest -from cryptography.hazmat.bindings._rust import openssl as rust_openssl from cryptography.hazmat.decrepit.ciphers.algorithms import RC2 from cryptography.hazmat.primitives.asymmetric import ( dsa, From 5d6f93b8c9dda009a9d23e3dbf14ddae1023e9d9 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 5 Feb 2025 16:54:04 -0800 Subject: [PATCH 40/46] improve rc2 --- Cargo.lock | 3 +-- src/rust/cryptography-key-parsing/src/pkcs8.rs | 9 ++------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 210531ecf514..f99284ee71c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -214,8 +214,7 @@ dependencies = [ [[package]] name = "openssl-macros" version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" +source = "git+https://github.com/alex/rust-openssl?branch=rc2#ae495dc27d9a20ab943c06085c8bb9c47ee90b5c" dependencies = [ "proc-macro2", "quote", diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index f69660daae22..e21ee6395240 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -166,8 +166,7 @@ pub fn parse_encrypted_private_key( AlgorithmParameters::Pbe1WithShaAnd40BitRc2Cbc(params) => pbes1_decrypt( epki.encrypted_data, password, - // XXX: unwrwap - openssl::symm::Cipher::from_nid(openssl::nid::Nid::RC2_40_CBC).unwrap(), + openssl::symm::Cipher::rc2_40_cbc(), openssl::hash::MessageDigest::sha1(), ¶ms, )?, @@ -191,11 +190,7 @@ pub fn parse_encrypted_private_key( if params.version.unwrap_or(32) != 58 { return Err(KeyParsingError::InvalidKey); } - ( - // XXX: unwrap - openssl::symm::Cipher::from_nid(openssl::nid::Nid::RC2_CBC).unwrap(), - ¶ms.iv[..], - ) + (openssl::symm::Cipher::rc2_cbc(), ¶ms.iv[..]) } _ => { return Err(KeyParsingError::UnsupportedEncryptionAlgorithm( From 878b7cd927ccf775432cad8730af47075519b32e Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 5 Feb 2025 16:59:01 -0800 Subject: [PATCH 41/46] support no rc2 --- src/rust/cryptography-key-parsing/Cargo.toml | 2 +- src/rust/cryptography-key-parsing/build.rs | 6 ++++++ src/rust/cryptography-key-parsing/src/pkcs8.rs | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/rust/cryptography-key-parsing/Cargo.toml b/src/rust/cryptography-key-parsing/Cargo.toml index ff9350bca60f..275e30baab4c 100644 --- a/src/rust/cryptography-key-parsing/Cargo.toml +++ b/src/rust/cryptography-key-parsing/Cargo.toml @@ -15,4 +15,4 @@ cryptography-crypto = { path = "../cryptography-crypto" } cryptography-x509 = { path = "../cryptography-x509" } [lints.rust] -unexpected_cfgs = { level = "warn", check-cfg = ['cfg(CRYPTOGRAPHY_IS_LIBRESSL)', 'cfg(CRYPTOGRAPHY_IS_BORINGSSL)'] } +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(CRYPTOGRAPHY_IS_LIBRESSL)', 'cfg(CRYPTOGRAPHY_IS_BORINGSSL)', 'cfg(CRYPTOGRAPHY_OSSLCONF, values("OPENSSL_NO_RC2"))'] } diff --git a/src/rust/cryptography-key-parsing/build.rs b/src/rust/cryptography-key-parsing/build.rs index cd318b35ff35..e46e71209a3f 100644 --- a/src/rust/cryptography-key-parsing/build.rs +++ b/src/rust/cryptography-key-parsing/build.rs @@ -12,4 +12,10 @@ fn main() { if env::var("DEP_OPENSSL_BORINGSSL").is_ok() { println!("cargo:rustc-cfg=CRYPTOGRAPHY_IS_BORINGSSL"); } + + if let Ok(vars) = env::var("DEP_OPENSSL_CONF") { + for var in vars.split(',') { + println!("cargo:rustc-cfg=CRYPTOGRAPHY_OSSLCONF=\"{var}\""); + } + } } diff --git a/src/rust/cryptography-key-parsing/src/pkcs8.rs b/src/rust/cryptography-key-parsing/src/pkcs8.rs index e21ee6395240..46dcf10716c5 100644 --- a/src/rust/cryptography-key-parsing/src/pkcs8.rs +++ b/src/rust/cryptography-key-parsing/src/pkcs8.rs @@ -163,6 +163,7 @@ pub fn parse_encrypted_private_key( openssl::hash::MessageDigest::sha1(), ¶ms, )?, + #[cfg(not(CRYPTOGRAPHY_OSSLCONF = "OPENSSL_NO_RC2"))] AlgorithmParameters::Pbe1WithShaAnd40BitRc2Cbc(params) => pbes1_decrypt( epki.encrypted_data, password, @@ -184,6 +185,7 @@ pub fn parse_encrypted_private_key( AlgorithmParameters::Aes256Cbc(ref iv) => { (openssl::symm::Cipher::aes_256_cbc(), &iv[..]) } + #[cfg(not(CRYPTOGRAPHY_OSSLCONF = "OPENSSL_NO_RC2"))] AlgorithmParameters::Rc2Cbc(ref params) => { // A version of 58 == 128 bits effective key length. The // default is 32. See RFC 8018 B.2.3. From a519f0ac0c2b5c518a6767f71c9d6ab6a3ce198d Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 5 Feb 2025 17:03:42 -0800 Subject: [PATCH 42/46] lol --- src/rust/src/backend/cipher_registry.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/rust/src/backend/cipher_registry.rs b/src/rust/src/backend/cipher_registry.rs index 8c50d52fd3b8..a64bc8d72c7e 100644 --- a/src/rust/src/backend/cipher_registry.rs +++ b/src/rust/src/backend/cipher_registry.rs @@ -302,9 +302,8 @@ fn get_cipher_registry( #[cfg(not(CRYPTOGRAPHY_OSSLCONF = "OPENSSL_NO_RC4"))] m.add(&arc4, none_type.as_any(), None, Cipher::rc4())?; - if let Some(rc2_cbc) = Cipher::from_nid(openssl::nid::Nid::RC2_CBC) { - m.add(&rc2, &cbc, Some(128), rc2_cbc)?; - } + #[cfg(not(CRYPTOGRAPHY_OSSLCONF = "OPENSSL_NO_RC2"))] + m.add(&rc2, &cbc, Some(128), Cipher::rc2_cbc()); } Ok(m.build()) From 4d5d4354511511e642e743ed363ffc7ad866cc7c Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 5 Feb 2025 17:04:40 -0800 Subject: [PATCH 43/46] Revert "lol" This reverts commit 0eec1a3ce97e883ec0f6cc0ace8726bd1938bb6b. --- src/rust/src/backend/cipher_registry.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rust/src/backend/cipher_registry.rs b/src/rust/src/backend/cipher_registry.rs index a64bc8d72c7e..8c50d52fd3b8 100644 --- a/src/rust/src/backend/cipher_registry.rs +++ b/src/rust/src/backend/cipher_registry.rs @@ -302,8 +302,9 @@ fn get_cipher_registry( #[cfg(not(CRYPTOGRAPHY_OSSLCONF = "OPENSSL_NO_RC4"))] m.add(&arc4, none_type.as_any(), None, Cipher::rc4())?; - #[cfg(not(CRYPTOGRAPHY_OSSLCONF = "OPENSSL_NO_RC2"))] - m.add(&rc2, &cbc, Some(128), Cipher::rc2_cbc()); + if let Some(rc2_cbc) = Cipher::from_nid(openssl::nid::Nid::RC2_CBC) { + m.add(&rc2, &cbc, Some(128), rc2_cbc)?; + } } Ok(m.build()) From 656b54d8197ff364cafa1575e977b89c987ad8d7 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 5 Feb 2025 17:06:06 -0800 Subject: [PATCH 44/46] skip on boringssl --- tests/hazmat/primitives/test_serialization.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/hazmat/primitives/test_serialization.py b/tests/hazmat/primitives/test_serialization.py index feebca359736..7792ecdaf9d1 100644 --- a/tests/hazmat/primitives/test_serialization.py +++ b/tests/hazmat/primitives/test_serialization.py @@ -10,6 +10,7 @@ import pytest +from cryptography.hazmat.bindings._rust import openssl as rust_openssl from cryptography.hazmat.decrepit.ciphers.algorithms import RC2 from cryptography.hazmat.primitives.asymmetric import ( dsa, @@ -483,7 +484,8 @@ def test_load_pkscs8_pbkdf_prf(self, filename: str): @pytest.mark.supported( only_if=lambda backend: backend.cipher_supported( RC2(b"\x00" * 16), modes.CBC(b"\x00" * 8) - ), + ) + and not rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL, skip_message="Does not support RC2 CBC", ) def test_load_pkcs8_40_bit_rc2(self): @@ -498,7 +500,8 @@ def test_load_pkcs8_40_bit_rc2(self): @pytest.mark.supported( only_if=lambda backend: backend.cipher_supported( RC2(b"\x00" * 16), modes.CBC(b"\x00" * 8) - ), + ) + and not rust_openssl.CRYPTOGRAPHY_IS_BORINGSSL, skip_message="Does not support RC2 CBC", ) def test_load_pkcs8_rc2_cbc(self): From 054b4448fff3e78438f6a045872baf462c877a98 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 5 Feb 2025 20:39:12 -0800 Subject: [PATCH 45/46] update rust-openssl --- Cargo.lock | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index f99284ee71c5..210531ecf514 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -214,7 +214,8 @@ dependencies = [ [[package]] name = "openssl-macros" version = "0.1.1" -source = "git+https://github.com/alex/rust-openssl?branch=rc2#ae495dc27d9a20ab943c06085c8bb9c47ee90b5c" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" dependencies = [ "proc-macro2", "quote", From 281fae4f738fcc35c1d8f74d61ef189cc574a172 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Fri, 11 Apr 2025 13:39:07 -0500 Subject: [PATCH 46/46] review comments --- src/rust/src/backend/keys.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 38609a1da46a..a9e961adc398 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -35,9 +35,9 @@ pub(crate) fn load_der_private_key_bytes<'p>( unsafe_skip_rsa_key_validation: bool, ) -> CryptographyResult> { let pkey = cryptography_key_parsing::pkcs8::parse_private_key(data) - .or_else(|_| cryptography_key_parsing::dsa::parse_pkcs1_private_key(data)) .or_else(|_| cryptography_key_parsing::ec::parse_pkcs1_private_key(data, None)) - .or_else(|_| cryptography_key_parsing::rsa::parse_pkcs1_private_key(data)); + .or_else(|_| cryptography_key_parsing::rsa::parse_pkcs1_private_key(data)) + .or_else(|_| cryptography_key_parsing::dsa::parse_pkcs1_private_key(data)); if let Ok(pkey) = pkey { if password.is_some() { @@ -104,6 +104,8 @@ fn load_pem_private_key<'p>( Some(p) => p, }; + // There's no RFC that defines these, but these are the ones in + // very wide use that we support. let cipher = match cipher_algorithm { "AES-128-CBC" => openssl::symm::Cipher::aes_128_cbc(), "AES-256-CBC" => openssl::symm::Cipher::aes_256_cbc(),