diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3ce8283b43e0..1c88a69d2fe0 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -28,6 +28,10 @@ Changelog in the same file. * Fixed parsing of :class:`~cryptography.x509.CertificatePolicies` extensions containing legacy ``BMPString`` values in their ``explicitText``. +* Allow parsing of negative serial numbers in certificates. Negative serial + numbers are prohibited by :rfc:`5280` so a deprecation warning will be + raised whenever they are encountered. A future version of ``cryptography`` + will drop support for parsing them. * Added support for parsing PKCS12 files with friendly names for all certificates with :func:`~cryptography.hazmat.primitives.serialization.pkcs12.load_pkcs12`, diff --git a/src/rust/src/asn1.rs b/src/rust/src/asn1.rs index 49f44ee45fe4..efb340f040fd 100644 --- a/src/rust/src/asn1.rs +++ b/src/rust/src/asn1.rs @@ -4,6 +4,7 @@ use crate::x509::Name; use pyo3::basic::CompareOp; +use pyo3::types::IntoPyDict; use pyo3::ToPyObject; pub enum PyAsn1Error { @@ -77,12 +78,13 @@ struct DssSignature<'a> { s: asn1::BigUint<'a>, } -pub(crate) fn big_asn1_uint_to_py<'p>( +pub(crate) fn big_byte_slice_to_py_int<'p>( py: pyo3::Python<'p>, - v: asn1::BigUint<'_>, + v: &'_ [u8], ) -> pyo3::PyResult<&'p pyo3::PyAny> { let int_type = py.get_type::(); - int_type.call_method1("from_bytes", (v.as_bytes(), "big")) + let kwargs = [("signed", true)].into_py_dict(py); + int_type.call_method("from_bytes", (v, "big"), Some(kwargs)) } #[pyo3::prelude::pyfunction] @@ -90,8 +92,8 @@ fn decode_dss_signature(py: pyo3::Python<'_>, data: &[u8]) -> Result>(data)?; Ok(( - big_asn1_uint_to_py(py, sig.r)?, - big_asn1_uint_to_py(py, sig.s)?, + big_byte_slice_to_py_int(py, sig.r.as_bytes())?, + big_byte_slice_to_py_int(py, sig.s.as_bytes())?, ) .to_object(py)) } diff --git a/src/rust/src/x509/certificate.rs b/src/rust/src/x509/certificate.rs index b582640a22e0..4ab8d37025e6 100644 --- a/src/rust/src/x509/certificate.rs +++ b/src/rust/src/x509/certificate.rs @@ -2,7 +2,9 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. -use crate::asn1::{big_asn1_uint_to_py, py_uint_to_big_endian_bytes, PyAsn1Error, PyAsn1Result}; +use crate::asn1::{ + big_byte_slice_to_py_int, py_uint_to_big_endian_bytes, PyAsn1Error, PyAsn1Result, +}; use crate::x509; use crate::x509::{crl, extensions, oid, sct}; use chrono::Datelike; @@ -23,7 +25,7 @@ pub(crate) struct TbsCertificate<'a> { #[explicit(0)] #[default(0)] version: u8, - pub(crate) serial: asn1::BigUint<'a>, + pub(crate) serial: asn1::BigInt<'a>, signature_alg: x509::AlgorithmIdentifier<'a>, pub(crate) issuer: x509::Name<'a>, @@ -178,10 +180,9 @@ impl Certificate { #[getter] fn serial_number<'p>(&self, py: pyo3::Python<'p>) -> Result<&'p pyo3::PyAny, PyAsn1Error> { - Ok(big_asn1_uint_to_py( - py, - self.raw.borrow_value().tbs_cert.serial, - )?) + let bytes = self.raw.borrow_value().tbs_cert.serial.as_bytes(); + warn_if_negative_serial(py, bytes)?; + Ok(big_byte_slice_to_py_int(py, bytes)?) } #[getter] @@ -350,12 +351,31 @@ fn load_der_x509_certificate(py: pyo3::Python<'_>, data: &[u8]) -> PyAsn1Result< let raw = OwnedRawCertificate::try_new(Arc::from(data), |data| asn1::parse_single(data))?; // Parse cert version immediately so we can raise error on parse if it is invalid. cert_version(py, raw.borrow_value().tbs_cert.version)?; + // determine if the serial is negative and raise a warning if it is. We want to drop support + // for this sort of invalid encoding eventually. + warn_if_negative_serial(py, raw.borrow_value().tbs_cert.serial.as_bytes())?; + Ok(Certificate { raw, cached_extensions: None, }) } +fn warn_if_negative_serial(py: pyo3::Python<'_>, bytes: &'_ [u8]) -> pyo3::PyResult<()> { + if bytes[0] & 0x80 != 0 { + let cryptography_warning = py.import("cryptography.utils")?.getattr("DeprecatedIn36")?; + let warnings = py.import("warnings")?; + warnings.call_method1( + "warn", + ( + "Parsed a negative serial number, which is disallowed by RFC 5280.", + cryptography_warning, + ), + )?; + } + Ok(()) +} + // Needed due to clippy type complexity warning. type SequenceOfPolicyQualifiers<'a> = x509::Asn1ReadableOrWritable< 'a, @@ -441,7 +461,7 @@ fn parse_user_notice( let org = parse_display_text(py, data.organization)?; let numbers = pyo3::types::PyList::empty(py); for num in data.notice_numbers.unwrap_read().clone() { - numbers.append(big_asn1_uint_to_py(py, num)?.to_object(py))?; + numbers.append(big_byte_slice_to_py_int(py, num.as_bytes())?.to_object(py))?; } x509_module .call_method1("NoticeReference", (org, numbers))? @@ -699,7 +719,7 @@ pub(crate) fn parse_authority_key_identifier<'p>( let x509_module = py.import("cryptography.x509")?; let aki = asn1::parse_single::>(ext_data)?; let serial = match aki.authority_cert_serial_number { - Some(biguint) => big_asn1_uint_to_py(py, biguint)?.to_object(py), + Some(biguint) => big_byte_slice_to_py_int(py, biguint.as_bytes())?.to_object(py), None => py.None(), }; let issuer = match aki.authority_cert_issuer { @@ -836,7 +856,7 @@ pub fn parse_cert_ext<'p>( Ok(Some(x509_module.getattr("OCSPNoCheck")?.call0()?)) } else if oid == *oid::INHIBIT_ANY_POLICY_OID { let bignum = asn1::parse_single::>(ext_data)?; - let pynum = big_asn1_uint_to_py(py, bignum)?; + let pynum = big_byte_slice_to_py_int(py, bignum.as_bytes())?; Ok(Some( x509_module.getattr("InhibitAnyPolicy")?.call1((pynum,))?, )) @@ -909,7 +929,7 @@ fn create_x509_certificate( let tbs_cert = TbsCertificate { version: builder.getattr("_version")?.getattr("value")?.extract()?, - serial: asn1::BigUint::new(py_uint_to_big_endian_bytes(py, py_serial)?).unwrap(), + serial: asn1::BigInt::new(py_uint_to_big_endian_bytes(py, py_serial)?).unwrap(), signature_alg: sigalg.clone(), issuer: x509::common::encode_name(py, builder.getattr("_issuer_name")?)?, validity: Validity { diff --git a/src/rust/src/x509/crl.rs b/src/rust/src/x509/crl.rs index 94117940a0b1..e76fd740c819 100644 --- a/src/rust/src/x509/crl.rs +++ b/src/rust/src/x509/crl.rs @@ -2,7 +2,9 @@ // 2.0, and the BSD License. See the LICENSE file in the root of this repository // for complete details. -use crate::asn1::{big_asn1_uint_to_py, py_uint_to_big_endian_bytes, PyAsn1Error, PyAsn1Result}; +use crate::asn1::{ + big_byte_slice_to_py_int, py_uint_to_big_endian_bytes, PyAsn1Error, PyAsn1Result, +}; use crate::x509; use crate::x509::{certificate, extensions, oid}; use pyo3::ToPyObject; @@ -262,11 +264,11 @@ impl CertificateRevocationList { |oid, ext_data| { if oid == &*oid::CRL_NUMBER_OID { let bignum = asn1::parse_single::>(ext_data)?; - let pynum = big_asn1_uint_to_py(py, bignum)?; + let pynum = big_byte_slice_to_py_int(py, bignum.as_bytes())?; Ok(Some(x509_module.getattr("CRLNumber")?.call1((pynum,))?)) } else if oid == &*oid::DELTA_CRL_INDICATOR_OID { let bignum = asn1::parse_single::>(ext_data)?; - let pynum = big_asn1_uint_to_py(py, bignum)?; + let pynum = big_byte_slice_to_py_int(py, bignum.as_bytes())?; Ok(Some( x509_module.getattr("DeltaCRLIndicator")?.call1((pynum,))?, )) @@ -530,7 +532,7 @@ struct RevokedCertificate { impl RevokedCertificate { #[getter] fn serial_number<'p>(&self, py: pyo3::Python<'p>) -> pyo3::PyResult<&'p pyo3::PyAny> { - big_asn1_uint_to_py(py, self.raw.borrow_value().user_certificate) + big_byte_slice_to_py_int(py, self.raw.borrow_value().user_certificate.as_bytes()) } #[getter] diff --git a/src/rust/src/x509/ocsp.rs b/src/rust/src/x509/ocsp.rs index ca340d0b189a..0d8e1c071034 100644 --- a/src/rust/src/x509/ocsp.rs +++ b/src/rust/src/x509/ocsp.rs @@ -33,7 +33,7 @@ pub(crate) struct CertID<'a> { pub(crate) hash_algorithm: x509::AlgorithmIdentifier<'a>, pub(crate) issuer_name_hash: &'a [u8], pub(crate) issuer_key_hash: &'a [u8], - pub(crate) serial_number: asn1::BigUint<'a>, + pub(crate) serial_number: asn1::BigInt<'a>, } impl CertID<'_> { diff --git a/src/rust/src/x509/ocsp_req.rs b/src/rust/src/x509/ocsp_req.rs index a3e161b9f280..57b1391c6076 100644 --- a/src/rust/src/x509/ocsp_req.rs +++ b/src/rust/src/x509/ocsp_req.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 crate::asn1::{big_asn1_uint_to_py, PyAsn1Error, PyAsn1Result}; +use crate::asn1::{big_byte_slice_to_py_int, PyAsn1Error, PyAsn1Result}; use crate::x509; use crate::x509::{extensions, ocsp, oid}; use std::sync::Arc; @@ -95,7 +95,8 @@ impl OCSPRequest { #[getter] fn serial_number<'p>(&self, py: pyo3::Python<'p>) -> Result<&'p pyo3::PyAny, PyAsn1Error> { - Ok(big_asn1_uint_to_py(py, self.cert_id()?.serial_number)?) + let bytes = self.cert_id()?.serial_number.as_bytes(); + Ok(big_byte_slice_to_py_int(py, bytes)?) } #[getter] diff --git a/src/rust/src/x509/ocsp_resp.rs b/src/rust/src/x509/ocsp_resp.rs index f8df0b282a7a..66b5e2324f76 100644 --- a/src/rust/src/x509/ocsp_resp.rs +++ b/src/rust/src/x509/ocsp_resp.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 crate::asn1::{big_asn1_uint_to_py, PyAsn1Error, PyAsn1Result}; +use crate::asn1::{big_byte_slice_to_py_int, PyAsn1Error, PyAsn1Result}; use crate::x509; use crate::x509::{certificate, crl, extensions, ocsp, oid, py_to_chrono, sct}; use std::sync::Arc; @@ -238,7 +238,7 @@ impl OCSPResponse { fn serial_number<'p>(&self, py: pyo3::Python<'p>) -> pyo3::PyResult<&'p pyo3::PyAny> { let resp = self.requires_successful_response()?; let single_resp = resp.single_response(); - big_asn1_uint_to_py(py, single_resp.cert_id.serial_number) + big_byte_slice_to_py_int(py, single_resp.cert_id.serial_number.as_bytes()) } #[getter] diff --git a/tests/x509/test_x509.py b/tests/x509/test_x509.py index 9e0a35bdee16..17076d992c34 100644 --- a/tests/x509/test_x509.py +++ b/tests/x509/test_x509.py @@ -735,13 +735,18 @@ def test_load_multiple_sections(self, backend): assert cert == cert2 def test_negative_serial_number(self, backend): - with pytest.raises(ValueError, match="TbsCertificate::serial"): - _load_cert( + # We load certificates with negative serial numbers but on load + # and on access of the attribute we raise a warning + with pytest.warns(utils.DeprecatedIn36): + cert = _load_cert( os.path.join("x509", "custom", "negative_serial.pem"), x509.load_pem_x509_certificate, backend, ) + with pytest.warns(utils.DeprecatedIn36): + assert cert.serial_number == -18008675309 + def test_alternate_rsa_with_sha1_oid(self, backend): cert = _load_cert( os.path.join("x509", "custom", "alternate-rsa-sha1-oid.der"),