From 749ea38534ebbe3d7200291cb98ab83499968fb3 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 | 3 + 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 | 142 ++++++++++++++++++ src/rust/cryptography-x509/src/common.rs | 59 ++++++++ src/rust/cryptography-x509/src/oid.rs | 30 ++++ src/rust/src/backend/dh.rs | 26 +++- src/rust/src/backend/ec.rs | 4 +- 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 | 27 ++-- tests/x509/test_x509_ext.py | 16 -- 15 files changed, 332 insertions(+), 84 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 c16a60e8f8fd..b2e0ac4aad38 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]] diff --git a/src/rust/cryptography-key-parsing/Cargo.toml b/src/rust/cryptography-key-parsing/Cargo.toml index dfa6b1d72182..3dd0b31fa1a6 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.63" +openssl-sys = "0.9.99" +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 000000000000..cd318b35ff35 --- /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 a5f7bc1d5579..93c49181c1fe 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 b1bbe2c13d38..066e7053cb52 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 000000000000..e6e1133c490a --- /dev/null +++ b/src/rust/cryptography-key-parsing/src/spki.rs @@ -0,0 +1,142 @@ +// 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_SECT233R1 => openssl::nid::Nid::SECT233R1, + 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_SECT163R2 => openssl::nid::Nid::SECT163R2, + + cryptography_x509::oid::EC_SECT163K1 => openssl::nid::Nid::SECT163K1, + cryptography_x509::oid::EC_SECT233K1 => openssl::nid::Nid::SECT233K1, + 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(CRYPTOGRAPHY_IS_BORINGSSL))] + cryptography_x509::oid::EC_BRAINPOOLP256R1 => { + openssl::nid::Nid::BRAINPOOL_P256R1 + } + #[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))] + cryptography_x509::oid::EC_BRAINPOOLP384R1 => { + openssl::nid::Nid::BRAINPOOL_P384R1 + } + #[cfg(not(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::UnsupportedEllipticCurve(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) => { + let p = openssl::bn::BigNum::from_slice(dh_params.p.as_bytes())?; + let q = openssl::bn::BigNum::from_slice(dh_params.q.as_bytes())?; + let g = openssl::bn::BigNum::from_slice(dh_params.g.as_bytes())?; + let dh = openssl::dh::Dh::from_pqg(p, Some(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 { + Ok(openssl::pkey::PKey::from_dhx(dh)?) + } + } + } + #[cfg(not(CRYPTOGRAPHY_IS_BORINGSSL))] + AlgorithmParameters::DhKeyAgreement(dh_params) => { + 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 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)?; + + 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 8366edcfbaff..77cebc30464e 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(DssParams<'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(DHXParams<'a>), + #[defined_by(oid::DH_KEY_AGREEMENT_OID)] + DhKeyAgreement(BasicDHParams<'a>), + #[default] Other(asn1::ObjectIdentifier, Option>), } @@ -235,6 +248,38 @@ pub struct DHParams<'a> { pub g: asn1::BigUint<'a>, pub q: Option>, } + +// From PKCS#3 Section 9 +// DHParameter ::= SEQUENCE { +// prime INTEGER, -- p +// base INTEGER, -- g +// privateValueLength INTEGER OPTIONAL +// } +#[derive(asn1::Asn1Read, asn1::Asn1Write, Clone, PartialEq, Eq, Debug, Hash)] +pub struct BasicDHParams<'a> { + pub p: asn1::BigUint<'a>, + pub g: asn1::BigUint<'a>, + pub private_value_length: Option, +} + +// From https://www.rfc-editor.org/rfc/rfc3279#section-2.3.3 +// DomainParameters ::= SEQUENCE { +// p INTEGER, -- odd prime, p=jq +1 +// g INTEGER, -- generator, g +// q INTEGER, -- factor of p-1 +// j INTEGER OPTIONAL, -- subgroup factor +// validationParms ValidationParms OPTIONAL +// } +#[derive(asn1::Asn1Read, asn1::Asn1Write, Clone, PartialEq, Eq, Debug, Hash)] +pub struct DHXParams<'a> { + pub p: asn1::BigUint<'a>, + pub g: asn1::BigUint<'a>, + pub q: asn1::BigUint<'a>, + pub j: Option>, + // No support for this, so don't bother filling out the fields. + pub validation_params: Option>, +} + // RSA-PSS ASN.1 default hash algorithm pub const PSS_SHA1_HASH_ALG: AlgorithmIdentifier<'_> = AlgorithmIdentifier { oid: asn1::DefinedByMarker::marker(), @@ -327,6 +372,20 @@ pub struct RsaPssParameters<'a> { pub _trailer_field: u8, } +// https://datatracker.ietf.org/doc/html/rfc3279#section-2.3.2 +// +// Dss-Parms ::= SEQUENCE { +// p INTEGER, +// q INTEGER, +// g INTEGER +// } +#[derive(asn1::Asn1Read, asn1::Asn1Write, Hash, Clone, PartialEq, Eq, Debug)] +pub struct DssParams<'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 8d3e3543d1b5..bf5d0ba29689 100644 --- a/src/rust/cryptography-x509/src/oid.rs +++ b/src/rust/cryptography-x509/src/oid.rs @@ -47,9 +47,32 @@ 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_SECT233R1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 27); +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_SECT163R2: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 15); + +pub const EC_SECT163K1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 1); +pub const EC_SECT233K1: asn1::ObjectIdentifier = asn1::oid!(1, 3, 132, 0, 26); +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 +104,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 0319a96f0d12..5ec1804e0df8 100644 --- a/src/rust/src/backend/dh.rs +++ b/src/rust/src/backend/dh.rs @@ -68,6 +68,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], @@ -192,8 +209,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 }) } @@ -301,7 +317,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)?, }) } @@ -413,7 +429,7 @@ impl DHPrivateNumbers { )); } - let pkey = openssl::pkey::PKey::from_dh(dh)?; + let pkey = pkey_from_dh(dh)?; Ok(DHPrivateKey { pkey }) } @@ -455,7 +471,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/ec.rs b/src/rust/src/backend/ec.rs index ed525f7d1502..07a3ce6aac72 100644 --- a/src/rust/src/backend/ec.rs +++ b/src/rust/src/backend/ec.rs @@ -152,9 +152,7 @@ pub(crate) fn public_key_from_pkey( py: pyo3::Python<'_>, pkey: &openssl::pkey::PKeyRef, ) -> CryptographyResult { - let ec = pkey.ec_key().map_err(|e| { - pyo3::exceptions::PyValueError::new_err(format!("Unable to load EC key: {e}")) - })?; + let ec = pkey.ec_key()?; let curve = py_curve_from_curve(py, ec.group())?; check_key_infinity(&ec)?; Ok(ECPublicKey { diff --git a/src/rust/src/backend/keys.rs b/src/rust/src/backend/keys.rs index 18b20becf948..bd3e8eb28e3b 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 79ca0ea63c16..a4461d05a87a 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 ca3a82abcbf7..a289c5ba7415 100644 --- a/tests/hazmat/backends/test_openssl.py +++ b/tests/hazmat/backends/test_openssl.py @@ -308,23 +308,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 33ab4121c30c..a8c5325891d2 100644 --- a/tests/hazmat/primitives/test_dh.py +++ b/tests/hazmat/primitives/test_dh.py @@ -443,10 +443,6 @@ def test_dh_vectors_with_q(self, backend, vector): assert int.from_bytes(symkey1, "big") == int(vector["z"], 16) assert int.from_bytes(symkey2, "big") == int(vector["z"], 16) - @pytest.mark.supported( - only_if=lambda backend: backend.dh_x942_serialization_supported(), - skip_message="DH X9.42 not supported", - ) def test_public_key_equality(self, backend): key_bytes = load_vectors_from_file( os.path.join("asymmetric", "DH", "dhpub.pem"), @@ -468,10 +464,6 @@ def test_public_key_equality(self, backend): with pytest.raises(TypeError): key1 < key2 # type: ignore[operator] - @pytest.mark.supported( - only_if=lambda backend: backend.dh_x942_serialization_supported(), - skip_message="DH X9.42 not supported", - ) def test_public_key_copy(self): key_bytes = load_vectors_from_file( os.path.join("asymmetric", "DH", "dhpub.pem"), @@ -696,7 +688,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 7048c025d312..fc3e3e06f00e 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(