From 9a78475eb57285a9d55cf04da472bc1da9aa2466 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Wed, 3 Jan 2024 19:36:18 -0500 Subject: [PATCH] Migrate SPKI parsing from OpenSSL to Rust --- src/rust/Cargo.lock | 12 +- src/rust/Cargo.toml | 4 + src/rust/cryptography-key-parsing/Cargo.toml | 3 + src/rust/cryptography-key-parsing/build.rs | 15 ++ src/rust/cryptography-key-parsing/src/lib.rs | 5 + src/rust/cryptography-key-parsing/src/rsa.rs | 2 +- src/rust/cryptography-key-parsing/src/spki.rs | 132 ++++++++++++++++++ src/rust/cryptography-x509/src/common.rs | 22 ++- src/rust/cryptography-x509/src/oid.rs | 25 ++++ src/rust/src/backend/dh.rs | 26 +++- src/rust/src/backend/keys.rs | 45 ++---- src/rust/src/error.rs | 19 +++ tests/hazmat/backends/test_openssl.py | 20 --- tests/hazmat/primitives/test_dh.py | 19 ++- tests/x509/test_x509_ext.py | 16 --- 15 files changed, 285 insertions(+), 80 deletions(-) create mode 100644 src/rust/cryptography-key-parsing/build.rs create mode 100644 src/rust/cryptography-key-parsing/src/spki.rs diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock index 06fb324df4583..668fee248c072 100644 --- a/src/rust/Cargo.lock +++ b/src/rust/Cargo.lock @@ -75,7 +75,10 @@ name = "cryptography-key-parsing" version = "0.1.0" dependencies = [ "asn1", + "cfg-if", + "cryptography-x509", "openssl", + "openssl-sys", ] [[package]] @@ -187,8 +190,7 @@ checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" [[package]] name = "openssl" version = "0.10.62" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8cde4d2d9200ad5909f8dac647e29482e07c3a35de8a13fce7c9c7747ad9f671" +source = "git+https://github.com/sfackler/rust-openssl#772b0b11c6efa7a834e77b2fd230feaea26fa83e" dependencies = [ "bitflags 2.4.1", "cfg-if", @@ -202,8 +204,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/sfackler/rust-openssl#772b0b11c6efa7a834e77b2fd230feaea26fa83e" dependencies = [ "proc-macro2", "quote", @@ -213,8 +214,7 @@ dependencies = [ [[package]] name = "openssl-sys" version = "0.9.98" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c1665caf8ab2dc9aef43d1c0023bd904633a6a05cb30b0ad59bec2ae986e57a7" +source = "git+https://github.com/sfackler/rust-openssl#772b0b11c6efa7a834e77b2fd230feaea26fa83e" dependencies = [ "cc", "libc", diff --git a/src/rust/Cargo.toml b/src/rust/Cargo.toml index d816efc291e61..baf1b42993b7c 100644 --- a/src/rust/Cargo.toml +++ b/src/rust/Cargo.toml @@ -45,3 +45,7 @@ members = [ "cryptography-x509", "cryptography-x509-verification", ] + +[patch.crates-io] +openssl = { git = "https://github.com/sfackler/rust-openssl" } +openssl-sys = { git = "https://github.com/sfackler/rust-openssl" } diff --git a/src/rust/cryptography-key-parsing/Cargo.toml b/src/rust/cryptography-key-parsing/Cargo.toml index a6fed36e22b28..5d09df95d32bf 100644 --- a/src/rust/cryptography-key-parsing/Cargo.toml +++ b/src/rust/cryptography-key-parsing/Cargo.toml @@ -9,4 +9,7 @@ rust-version = "1.63.0" [dependencies] asn1 = { version = "0.15.5", default-features = false } +cfg-if = "1" openssl = "0.10.62" +openssl-sys = "0.9.98" +cryptography-x509 = { path = "../cryptography-x509" } diff --git a/src/rust/cryptography-key-parsing/build.rs b/src/rust/cryptography-key-parsing/build.rs new file mode 100644 index 0000000000000..cd318b35ff356 --- /dev/null +++ b/src/rust/cryptography-key-parsing/build.rs @@ -0,0 +1,15 @@ +// 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 std::env; + +fn main() { + if env::var("DEP_OPENSSL_LIBRESSL_VERSION_NUMBER").is_ok() { + println!("cargo:rustc-cfg=CRYPTOGRAPHY_IS_LIBRESSL"); + } + + if env::var("DEP_OPENSSL_BORINGSSL").is_ok() { + println!("cargo:rustc-cfg=CRYPTOGRAPHY_IS_BORINGSSL"); + } +} diff --git a/src/rust/cryptography-key-parsing/src/lib.rs b/src/rust/cryptography-key-parsing/src/lib.rs index a5f7bc1d55796..93c49181c1fea 100644 --- a/src/rust/cryptography-key-parsing/src/lib.rs +++ b/src/rust/cryptography-key-parsing/src/lib.rs @@ -3,8 +3,13 @@ // for complete details. pub mod rsa; +pub mod spki; pub enum KeyParsingError { + InvalidKey, + ExplicitCurveUnsupported, + UnsupportedKeyType(asn1::ObjectIdentifier), + UnsupportedEllipticCurve(asn1::ObjectIdentifier), Parse(asn1::ParseError), OpenSSL(openssl::error::ErrorStack), } diff --git a/src/rust/cryptography-key-parsing/src/rsa.rs b/src/rust/cryptography-key-parsing/src/rsa.rs index b1bbe2c13d38d..066e7053cb520 100644 --- a/src/rust/cryptography-key-parsing/src/rsa.rs +++ b/src/rust/cryptography-key-parsing/src/rsa.rs @@ -10,7 +10,7 @@ struct Pksc1RsaPublicKey<'a> { e: asn1::BigUint<'a>, } -pub fn parse_pkcs1_rsa_public_key( +pub fn parse_pkcs1_public_key( data: &[u8], ) -> KeyParsingResult> { let k = asn1::parse_single::(data)?; diff --git a/src/rust/cryptography-key-parsing/src/spki.rs b/src/rust/cryptography-key-parsing/src/spki.rs new file mode 100644 index 0000000000000..dbb25b8f13dfa --- /dev/null +++ b/src/rust/cryptography-key-parsing/src/spki.rs @@ -0,0 +1,132 @@ +// 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::{AlgorithmParameters, EcParameters, SubjectPublicKeyInfo}; + +use crate::{KeyParsingError, KeyParsingResult}; + +pub fn parse_public_key( + data: &[u8], +) -> KeyParsingResult> { + let k = asn1::parse_single::(data)?; + + match k.algorithm.params { + AlgorithmParameters::Ec(ec_params) => match ec_params { + EcParameters::NamedCurve(curve_oid) => { + let curve_nid = match curve_oid { + cryptography_x509::oid::EC_SECP192R1 => openssl::nid::Nid::X9_62_PRIME192V1, + cryptography_x509::oid::EC_SECP224R1 => openssl::nid::Nid::SECP224R1, + cryptography_x509::oid::EC_SECP256R1 => openssl::nid::Nid::X9_62_PRIME256V1, + cryptography_x509::oid::EC_SECP384R1 => openssl::nid::Nid::SECP384R1, + cryptography_x509::oid::EC_SECP521R1 => openssl::nid::Nid::SECP521R1, + + cryptography_x509::oid::EC_SECP256K1 => openssl::nid::Nid::SECP256K1, + + cryptography_x509::oid::EC_SECT283R1 => openssl::nid::Nid::SECT283R1, + cryptography_x509::oid::EC_SECT409R1 => openssl::nid::Nid::SECT409R1, + cryptography_x509::oid::EC_SECT571R1 => openssl::nid::Nid::SECT571R1, + + cryptography_x509::oid::EC_SECT283K1 => openssl::nid::Nid::SECT283K1, + cryptography_x509::oid::EC_SECT409K1 => openssl::nid::Nid::SECT409K1, + cryptography_x509::oid::EC_SECT571K1 => openssl::nid::Nid::SECT571K1, + + #[cfg(not(any(CRYPTOGRAPHY_IS_LIBRESSL, CRYPTOGRAPHY_IS_BORINGSSL)))] + cryptography_x509::oid::EC_BRAINPOOLP256R1 => { + openssl::nid::Nid::BRAINPOOL_P256R1 + } + #[cfg(not(any(CRYPTOGRAPHY_IS_LIBRESSL, CRYPTOGRAPHY_IS_BORINGSSL)))] + cryptography_x509::oid::EC_BRAINPOOLP384R1 => { + openssl::nid::Nid::BRAINPOOL_P384R1 + } + #[cfg(not(any(CRYPTOGRAPHY_IS_LIBRESSL, CRYPTOGRAPHY_IS_BORINGSSL)))] + cryptography_x509::oid::EC_BRAINPOOLP512R1 => { + openssl::nid::Nid::BRAINPOOL_P512R1 + } + + _ => return Err(KeyParsingError::UnsupportedEllipticCurve(curve_oid)), + }; + + let group = openssl::ec::EcGroup::from_curve_name(curve_nid) + .map_err(|_| KeyParsingError::UnsupportedKeyType(curve_oid))?; + let mut bn_ctx = openssl::bn::BigNumContext::new()?; + let ec_point = openssl::ec::EcPoint::from_bytes( + &group, + k.subject_public_key.as_bytes(), + &mut bn_ctx, + ) + .map_err(|_| KeyParsingError::InvalidKey)?; + let ec_key = openssl::ec::EcKey::from_public_key(&group, &ec_point)?; + Ok(openssl::pkey::PKey::from_ec_key(ec_key)?) + } + EcParameters::ImplicitCurve(_) | EcParameters::SpecifiedCurve(_) => { + Err(KeyParsingError::ExplicitCurveUnsupported) + } + }, + AlgorithmParameters::Ed25519 => Ok(openssl::pkey::PKey::public_key_from_raw_bytes( + k.subject_public_key.as_bytes(), + openssl::pkey::Id::ED25519, + )?), + #[cfg(all(not(CRYPTOGRAPHY_IS_LIBRESSL), not(CRYPTOGRAPHY_IS_BORINGSSL)))] + AlgorithmParameters::Ed448 => Ok(openssl::pkey::PKey::public_key_from_raw_bytes( + k.subject_public_key.as_bytes(), + openssl::pkey::Id::ED448, + )?), + AlgorithmParameters::X25519 => Ok(openssl::pkey::PKey::public_key_from_raw_bytes( + k.subject_public_key.as_bytes(), + openssl::pkey::Id::X25519, + )?), + #[cfg(all(not(CRYPTOGRAPHY_IS_LIBRESSL), not(CRYPTOGRAPHY_IS_BORINGSSL)))] + AlgorithmParameters::X448 => Ok(openssl::pkey::PKey::public_key_from_raw_bytes( + k.subject_public_key.as_bytes(), + openssl::pkey::Id::X448, + )?), + AlgorithmParameters::Rsa(_) | AlgorithmParameters::RsaPss(_) => { + // RSA-PSS keys are treated the same as bare RSA keys. + crate::rsa::parse_pkcs1_public_key(k.subject_public_key.as_bytes()) + } + AlgorithmParameters::Dsa(dsa_params) => { + 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 pub_key_int = + asn1::parse_single::>(k.subject_public_key.as_bytes())?; + let pub_key = openssl::bn::BigNum::from_slice(pub_key_int.as_bytes())?; + + let dsa = openssl::dsa::Dsa::from_public_components(p, q, g, pub_key)?; + Ok(openssl::pkey::PKey::from_dsa(dsa)?) + } + #[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))] + AlgorithmParameters::Dh(dh_params) | AlgorithmParameters::DhKeyAgreement(dh_params) => { + let p = openssl::bn::BigNum::from_slice(dh_params.p.as_bytes())?; + let q = dh_params + .q + .as_ref() + .map(|v| openssl::bn::BigNum::from_slice(v.as_bytes())) + .transpose()?; + let g = openssl::bn::BigNum::from_slice(dh_params.g.as_bytes())?; + let dh = openssl::dh::Dh::from_pqg(p, q, g)?; + + let pub_key_int = + asn1::parse_single::>(k.subject_public_key.as_bytes())?; + let pub_key = openssl::bn::BigNum::from_slice(pub_key_int.as_bytes())?; + let dh = dh.set_public_key(pub_key)?; + + cfg_if::cfg_if! { + if #[cfg(CRYPTOGRAPHY_IS_LIBRESSL)] { + Ok(openssl::pkey::PKey::from_dh(dh)?) + } else { + if dh_params.q.is_some() { + Ok(openssl::pkey::PKey::from_dhx(dh)?) + } else { + Ok(openssl::pkey::PKey::from_dh(dh)?) + } + } + } + } + _ => Err(KeyParsingError::UnsupportedKeyType( + k.algorithm.oid().clone(), + )), + } +} diff --git a/src/rust/cryptography-x509/src/common.rs b/src/rust/cryptography-x509/src/common.rs index 8366edcfbaff6..4a6e75e271977 100644 --- a/src/rust/cryptography-x509/src/common.rs +++ b/src/rust/cryptography-x509/src/common.rs @@ -45,6 +45,11 @@ pub enum AlgorithmParameters<'a> { #[defined_by(oid::ED448_OID)] Ed448, + #[defined_by(oid::X25519_OID)] + X25519, + #[defined_by(oid::X448_OID)] + X448, + // These encodings are only used in SPKI AlgorithmIdentifiers. #[defined_by(oid::EC_OID)] Ec(EcParameters<'a>), @@ -103,6 +108,9 @@ pub enum AlgorithmParameters<'a> { #[defined_by(oid::RSASSA_PSS_OID)] RsaPss(Option>>), + #[defined_by(oid::DSA_OID)] + Dsa(DssParms<'a>), + #[defined_by(oid::DSA_WITH_SHA224_OID)] DsaWithSha224(Option), #[defined_by(oid::DSA_WITH_SHA256_OID)] @@ -112,6 +120,11 @@ pub enum AlgorithmParameters<'a> { #[defined_by(oid::DSA_WITH_SHA512_OID)] DsaWithSha512(Option), + #[defined_by(oid::DH_OID)] + Dh(DHParams<'a>), + #[defined_by(oid::DH_KEY_AGREEMENT_OID)] + DhKeyAgreement(DHParams<'a>), + #[default] Other(asn1::ObjectIdentifier, Option>), } @@ -229,7 +242,7 @@ pub struct DssSignature<'a> { pub s: asn1::BigUint<'a>, } -#[derive(asn1::Asn1Read, asn1::Asn1Write)] +#[derive(asn1::Asn1Read, asn1::Asn1Write, Clone, Hash, PartialEq, Eq, Debug)] pub struct DHParams<'a> { pub p: asn1::BigUint<'a>, pub g: asn1::BigUint<'a>, @@ -327,6 +340,13 @@ pub struct RsaPssParameters<'a> { pub _trailer_field: u8, } +#[derive(asn1::Asn1Read, asn1::Asn1Write, Hash, Clone, PartialEq, Eq, Debug)] +pub struct DssParms<'a> { + pub p: asn1::BigUint<'a>, + pub q: asn1::BigUint<'a>, + pub g: asn1::BigUint<'a>, +} + /// 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. diff --git a/src/rust/cryptography-x509/src/oid.rs b/src/rust/cryptography-x509/src/oid.rs index 8d3e3543d1b5b..ea8a96d9dff91 100644 --- a/src/rust/cryptography-x509/src/oid.rs +++ b/src/rust/cryptography-x509/src/oid.rs @@ -47,9 +47,27 @@ pub const ACCEPTABLE_RESPONSES_OID: asn1::ObjectIdentifier = // Public key identifiers pub const EC_OID: asn1::ObjectIdentifier = asn1::oid!(1, 2, 840, 10045, 2, 1); + +pub const EC_SECP192R1: asn1::ObjectIdentifier = asn1::oid!(1, 2, 840, 10045, 3, 1, 1); +pub const EC_SECP224R1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 33); pub const EC_SECP256R1: asn1::ObjectIdentifier = asn1::oid!(1, 2, 840, 10045, 3, 1, 7); pub const EC_SECP384R1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 34); pub const EC_SECP521R1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 35); + +pub const EC_SECP256K1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 10); + +pub const EC_SECT283R1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 17); +pub const EC_SECT409R1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 37); +pub const EC_SECT571R1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 39); + +pub const EC_SECT283K1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 16); +pub const EC_SECT409K1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 36); +pub const EC_SECT571K1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 38); + +pub const EC_BRAINPOOLP256R1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 36, 3, 3, 2, 8, 1, 1, 7); +pub const EC_BRAINPOOLP384R1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 36, 3, 3, 2, 8, 1, 1, 11); +pub const EC_BRAINPOOLP512R1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 36, 3, 3, 2, 8, 1, 1, 13); + pub const RSA_OID: asn1::ObjectIdentifier = asn1::oid!(1, 2, 840, 113549, 1, 1, 1); // Signing methods @@ -81,11 +99,18 @@ pub const RSA_WITH_SHA3_384_OID: asn1::ObjectIdentifier = pub const RSA_WITH_SHA3_512_OID: asn1::ObjectIdentifier = asn1::oid!(2, 16, 840, 1, 101, 3, 4, 3, 16); +pub const DSA_OID: asn1::ObjectIdentifier = asn1::oid!(1, 2, 840, 10040, 4, 1); pub const DSA_WITH_SHA224_OID: asn1::ObjectIdentifier = asn1::oid!(2, 16, 840, 1, 101, 3, 4, 3, 1); pub const DSA_WITH_SHA256_OID: asn1::ObjectIdentifier = asn1::oid!(2, 16, 840, 1, 101, 3, 4, 3, 2); pub const DSA_WITH_SHA384_OID: asn1::ObjectIdentifier = asn1::oid!(2, 16, 840, 1, 101, 3, 4, 3, 3); pub const DSA_WITH_SHA512_OID: asn1::ObjectIdentifier = asn1::oid!(2, 16, 840, 1, 101, 3, 4, 3, 4); +pub const DH_OID: asn1::ObjectIdentifier = asn1::oid!(1, 2, 840, 10046, 2, 1); +pub const DH_KEY_AGREEMENT_OID: asn1::ObjectIdentifier = asn1::oid!(1, 2, 840, 113549, 1, 3, 1); + +pub const X25519_OID: asn1::ObjectIdentifier = asn1::oid!(1, 3, 101, 110); +pub const X448_OID: asn1::ObjectIdentifier = asn1::oid!(1, 3, 101, 111); + pub const ED25519_OID: asn1::ObjectIdentifier = asn1::oid!(1, 3, 101, 112); pub const ED448_OID: asn1::ObjectIdentifier = asn1::oid!(1, 3, 101, 113); diff --git a/src/rust/src/backend/dh.rs b/src/rust/src/backend/dh.rs index f4a80d7acc1e0..43e43a75935b2 100644 --- a/src/rust/src/backend/dh.rs +++ b/src/rust/src/backend/dh.rs @@ -62,6 +62,23 @@ pub(crate) fn public_key_from_pkey( } } +#[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))] +fn pkey_from_dh( + dh: openssl::dh::Dh, +) -> CryptographyResult> { + cfg_if::cfg_if! { + if #[cfg(CRYPTOGRAPHY_IS_LIBRESSL)] { + Ok(openssl::pkey::PKey::from_dh(dh)?) + } else { + if dh.prime_q().is_some() { + Ok(openssl::pkey::PKey::from_dhx(dh)?) + } else { + Ok(openssl::pkey::PKey::from_dh(dh)?) + } + } + } +} + #[pyo3::prelude::pyfunction] fn from_der_parameters( data: &[u8], @@ -186,8 +203,7 @@ impl DHPrivateKey { let orig_dh = self.pkey.dh().unwrap(); let dh = clone_dh(&orig_dh)?; - let pkey = - openssl::pkey::PKey::from_dh(dh.set_public_key(orig_dh.public_key().to_owned()?)?)?; + let pkey = pkey_from_dh(dh.set_public_key(orig_dh.public_key().to_owned()?)?)?; Ok(DHPublicKey { pkey }) } @@ -295,7 +311,7 @@ impl DHParameters { fn generate_private_key(&self) -> CryptographyResult { let dh = clone_dh(&self.dh)?.generate_key()?; Ok(DHPrivateKey { - pkey: openssl::pkey::PKey::from_dh(dh)?, + pkey: pkey_from_dh(dh)?, }) } @@ -407,7 +423,7 @@ impl DHPrivateNumbers { )); } - let pkey = openssl::pkey::PKey::from_dh(dh)?; + let pkey = pkey_from_dh(dh)?; Ok(DHPrivateKey { pkey }) } @@ -449,7 +465,7 @@ impl DHPublicNumbers { let pub_key = utils::py_int_to_bn(py, self.y.as_ref(py))?; - let pkey = openssl::pkey::PKey::from_dh(dh.set_public_key(pub_key)?)?; + let pkey = pkey_from_dh(dh.set_public_key(pub_key)?)?; Ok(DHPublicKey { pkey }) } diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 18b20becf9486..bd3e8eb28e3b8 100644 --- a/src/rust/src/backend/keys.rs +++ b/src/rust/src/backend/keys.rs @@ -7,8 +7,8 @@ use pyo3::IntoPy; use crate::backend::utils; use crate::buf::CffiBuf; -use crate::error::{self, CryptographyError, CryptographyResult}; -use crate::{exceptions, types}; +use crate::error::{CryptographyError, CryptographyResult}; +use crate::exceptions; #[pyo3::prelude::pyfunction] #[pyo3(signature = (data, password, backend=None, *, unsafe_skip_rsa_key_validation=false))] @@ -142,14 +142,18 @@ pub(crate) fn load_der_public_key_bytes( py: pyo3::Python<'_>, data: &[u8], ) -> CryptographyResult { - if let Ok(pkey) = openssl::pkey::PKey::public_key_from_der(data) { - return public_key_from_pkey(py, &pkey, pkey.id()); + match cryptography_key_parsing::spki::parse_public_key(data) { + Ok(pkey) => public_key_from_pkey(py, &pkey, pkey.id()), + // It's not a (RSA/DSA/ECDSA) subjectPublicKeyInfo, but we still need + // to check to see if it is a pure PKCS1 RSA public key (not embedded + // in a subjectPublicKeyInfo) + Err(e) => { + // Use the original error. + let pkey = + cryptography_key_parsing::rsa::parse_pkcs1_public_key(data).map_err(|_| e)?; + public_key_from_pkey(py, &pkey, pkey.id()) + } } - // It's not a (RSA/DSA/ECDSA) subjectPublicKeyInfo, but we still need to - // check to see if it is a pure PKCS1 RSA public key (not embedded in a - // subjectPublicKeyInfo) - let pkey = cryptography_key_parsing::rsa::parse_pkcs1_rsa_public_key(data)?; - public_key_from_pkey(py, &pkey, pkey.id()) } #[pyo3::prelude::pyfunction] @@ -161,16 +165,8 @@ fn load_pem_public_key( let _ = backend; let p = pem::parse(data.as_bytes())?; let pkey = match p.tag() { - "RSA PUBLIC KEY" => { - cryptography_key_parsing::rsa::parse_pkcs1_rsa_public_key(p.contents())? - } - "PUBLIC KEY" => openssl::pkey::PKey::public_key_from_der(p.contents()).or_else(|e| { - let errors = error::list_from_openssl_error(py, e); - Err(types::BACKEND_HANDLE_KEY_LOADING_ERROR - .get(py)? - .call1((errors,)) - .unwrap_err()) - })?, + "RSA PUBLIC KEY" => cryptography_key_parsing::rsa::parse_pkcs1_public_key(p.contents())?, + "PUBLIC KEY" => cryptography_key_parsing::spki::parse_public_key(p.contents())?, _ => return Err(CryptographyError::from(pem::PemError::MalformedFraming)), }; public_key_from_pkey(py, &pkey, pkey.id()) @@ -185,17 +181,6 @@ fn public_key_from_pkey( // unsupported. match id { openssl::pkey::Id::RSA => Ok(crate::backend::rsa::public_key_from_pkey(pkey).into_py(py)), - #[cfg(any(not(CRYPTOGRAPHY_IS_LIBRESSL), CRYPTOGRAPHY_LIBRESSL_380_OR_GREATER))] - 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()?.public_key_to_der()?; - let rsa = openssl::rsa::Rsa::public_key_from_der(&der_bytes)?; - let pkey = openssl::pkey::PKey::from_rsa(rsa)?; - Ok(crate::backend::rsa::public_key_from_pkey(&pkey).into_py(py)) - } openssl::pkey::Id::EC => { Ok(crate::backend::ec::public_key_from_pkey(py, pkey)?.into_py(py)) } diff --git a/src/rust/src/error.rs b/src/rust/src/error.rs index 79ca0ea63c166..a4461d05a87a9 100644 --- a/src/rust/src/error.rs +++ b/src/rust/src/error.rs @@ -57,6 +57,25 @@ impl From for CryptographyError { match e { cryptography_key_parsing::KeyParsingError::Parse(e) => CryptographyError::KeyParsing(e), cryptography_key_parsing::KeyParsingError::OpenSSL(e) => CryptographyError::OpenSSL(e), + cryptography_key_parsing::KeyParsingError::InvalidKey => { + CryptographyError::Py(pyo3::exceptions::PyValueError::new_err("Invalid key")) + } + cryptography_key_parsing::KeyParsingError::ExplicitCurveUnsupported => { + CryptographyError::Py(pyo3::exceptions::PyValueError::new_err( + "ECDSA keys with explicit parameters are unsupported at this time", + )) + } + cryptography_key_parsing::KeyParsingError::UnsupportedKeyType(oid) => { + CryptographyError::Py(pyo3::exceptions::PyValueError::new_err(format!( + "Unknown key type: {oid}" + ))) + } + cryptography_key_parsing::KeyParsingError::UnsupportedEllipticCurve(oid) => { + CryptographyError::Py(exceptions::UnsupportedAlgorithm::new_err(( + format!("Curve {oid} is not supported"), + exceptions::Reasons::UNSUPPORTED_ELLIPTIC_CURVE, + ))) + } } } } diff --git a/tests/hazmat/backends/test_openssl.py b/tests/hazmat/backends/test_openssl.py index f5bc6233f35c6..50316549182cc 100644 --- a/tests/hazmat/backends/test_openssl.py +++ b/tests/hazmat/backends/test_openssl.py @@ -349,23 +349,3 @@ def test_private_load_dhx_unsupported( ) with pytest.raises(ValueError): loader_func(key_bytes, None, backend) - - @pytest.mark.parametrize( - ("key_path", "loader_func"), - [ - ( - os.path.join("asymmetric", "DH", "dhpub_rfc5114_2.pem"), - serialization.load_pem_public_key, - ), - ( - os.path.join("asymmetric", "DH", "dhpub_rfc5114_2.der"), - serialization.load_der_public_key, - ), - ], - ) - def test_public_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, backend) diff --git a/tests/hazmat/primitives/test_dh.py b/tests/hazmat/primitives/test_dh.py index 33ab4121c30c2..3d80c62c89453 100644 --- a/tests/hazmat/primitives/test_dh.py +++ b/tests/hazmat/primitives/test_dh.py @@ -696,7 +696,24 @@ def test_public_bytes(self, backend, encoding, loader_func): loaded_key = loader_func(serialized, backend) loaded_pub_num = loaded_key.public_numbers() pub_num = key.public_numbers() - assert loaded_pub_num == pub_num + + assert loaded_pub_num.y == pub_num.y + assert ( + loaded_pub_num.parameter_numbers.p == pub_num.parameter_numbers.p + ) + assert ( + loaded_pub_num.parameter_numbers.g == pub_num.parameter_numbers.g + ) + if pub_num.parameter_numbers.q and loaded_pub_num.parameter_numbers.q: + assert ( + loaded_pub_num.parameter_numbers.q + == pub_num.parameter_numbers.q + ) + else: + # When this branch becomes unreachable by coverage (when support + # for RHEL8 is dropped), all this code can be replaced with: + # assert loaded_pub_num == pub_num + assert True @pytest.mark.skip_fips(reason="non-FIPS parameters") @pytest.mark.parametrize( diff --git a/tests/x509/test_x509_ext.py b/tests/x509/test_x509_ext.py index 7048c025d3123..fc3e3e06f00eb 100644 --- a/tests/x509/test_x509_ext.py +++ b/tests/x509/test_x509_ext.py @@ -1758,22 +1758,6 @@ def test_invalid_bit_string_padding_from_public_key(self, backend): with pytest.raises(ValueError, match="Invalid public key encoding"): _key_identifier_from_public_key(pretend_key) - def test_no_optional_params_allowed_from_public_key(self, backend): - data = load_vectors_from_file( - filename=os.path.join( - "asymmetric", - "DER_Serialization", - "dsa_public_key_no_params.der", - ), - loader=lambda data: data.read(), - mode="rb", - ) - pretend_key = pretend.stub(public_bytes=lambda x, y: data) - key_identifier = _key_identifier_from_public_key(pretend_key) - assert key_identifier == binascii.unhexlify( - b"24c0133a6a492f2c48a18c7648e515db5ac76749" - ) - def test_from_ec_public_key(self, backend): _skip_curve_unsupported(backend, ec.SECP384R1()) cert = _load_cert(