diff --git a/docs/development/test-vectors.rst b/docs/development/test-vectors.rst index 56bc9361c555..3e54c40ae43d 100644 --- a/docs/development/test-vectors.rst +++ b/docs/development/test-vectors.rst @@ -499,6 +499,8 @@ Custom X.509 Vectors * ``rsa_pss_sha256_no_null.pem`` - A certificate with an RSA PSS signature with no encoded ``NULL`` for the PSS hash algorithm parameters. This certificate was generated by LibreSSL. +* ``ecdsa_null_alg.pem`` - A certificate with an ECDSA signature with ``NULL`` + algorithm parameters. This encoding is invalid, but was generated by Java 11. Custom X.509 Request Vectors ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/rust/cryptography-x509/src/common.rs b/src/rust/cryptography-x509/src/common.rs index 466d4b5bd179..a882d985e9cb 100644 --- a/src/rust/cryptography-x509/src/common.rs +++ b/src/rust/cryptography-x509/src/common.rs @@ -45,14 +45,18 @@ pub enum AlgorithmParameters<'a> { #[defined_by(oid::ED448_OID)] Ed448, + // These ECDSA algorithms should have no parameters, + // but Java 11 (up to at least 11.0.19) encodes them + // with NULL parameters. The JDK team is looking to + // backport the fix as of June 2023. #[defined_by(oid::ECDSA_WITH_SHA224_OID)] - EcDsaWithSha224, + EcDsaWithSha224(Option), #[defined_by(oid::ECDSA_WITH_SHA256_OID)] - EcDsaWithSha256, + EcDsaWithSha256(Option), #[defined_by(oid::ECDSA_WITH_SHA384_OID)] - EcDsaWithSha384, + EcDsaWithSha384(Option), #[defined_by(oid::ECDSA_WITH_SHA512_OID)] - EcDsaWithSha512, + EcDsaWithSha512(Option), #[defined_by(oid::ECDSA_WITH_SHA3_224_OID)] EcDsaWithSha3_224, diff --git a/src/rust/src/x509/certificate.rs b/src/rust/src/x509/certificate.rs index 3446bbbbb604..9204d730ba03 100644 --- a/src/rust/src/x509/certificate.rs +++ b/src/rust/src/x509/certificate.rs @@ -8,7 +8,7 @@ use crate::asn1::{ use crate::error::{CryptographyError, CryptographyResult}; use crate::x509::{extensions, sct, sign}; use crate::{exceptions, x509}; -use cryptography_x509::common::Asn1ReadableOrWritable; +use cryptography_x509::common::{AlgorithmParameters, Asn1ReadableOrWritable}; use cryptography_x509::extensions::Extension; use cryptography_x509::extensions::{ AuthorityKeyIdentifier, BasicConstraints, DisplayText, DistributionPoint, @@ -391,6 +391,10 @@ fn load_der_x509_certificate( // 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())?; + // determine if the signature algorithm has incorrect parameters and raise a warning if it + // does. this is a bug in JDK11 and we want to drop support for it eventually. + warn_if_invalid_ecdsa_params(py, raw.borrow_value().signature_alg.params.clone())?; + warn_if_invalid_ecdsa_params(py, raw.borrow_value().tbs_cert.signature_alg.params.clone())?; Ok(Certificate { raw, @@ -413,6 +417,30 @@ fn warn_if_negative_serial(py: pyo3::Python<'_>, bytes: &'_ [u8]) -> pyo3::PyRes Ok(()) } +fn warn_if_invalid_ecdsa_params( + py: pyo3::Python<'_>, + params: AlgorithmParameters<'_>, +) -> pyo3::PyResult<()> { + match params { + AlgorithmParameters::EcDsaWithSha224(Some(..)) + | AlgorithmParameters::EcDsaWithSha256(Some(..)) + | AlgorithmParameters::EcDsaWithSha384(Some(..)) + | AlgorithmParameters::EcDsaWithSha512(Some(..)) => { + let cryptography_warning = py + .import(pyo3::intern!(py, "cryptography.utils"))? + .getattr(pyo3::intern!(py, "DeprecatedIn41"))?; + pyo3::PyErr::warn( + py, + cryptography_warning, + "The parsed certificate contains a NULL parameter value in its signature algorithm parameters. This is invalid and will be rejected in a future version of cryptography. If this certificate was created via Java, please upgrade to JDK16+ or the latest JDK11 once a fix is issued. If this certificate was created in some other fashion please report the issue to the cryptography issue tracker. See https://github.com/pyca/cryptography/issues/8996 for more details.", + 2, + )?; + } + _ => {} + } + Ok(()) +} + fn parse_display_text( py: pyo3::Python<'_>, text: DisplayText<'_>, diff --git a/src/rust/src/x509/sign.rs b/src/rust/src/x509/sign.rs index b3a799b8cb01..4b03a2d9ab8e 100644 --- a/src/rust/src/x509/sign.rs +++ b/src/rust/src/x509/sign.rs @@ -234,19 +234,19 @@ pub(crate) fn compute_signature_algorithm<'p>( (KeyType::Ec, HashType::Sha224) => Ok(common::AlgorithmIdentifier { oid: asn1::DefinedByMarker::marker(), - params: common::AlgorithmParameters::EcDsaWithSha224, + params: common::AlgorithmParameters::EcDsaWithSha224(None), }), (KeyType::Ec, HashType::Sha256) => Ok(common::AlgorithmIdentifier { oid: asn1::DefinedByMarker::marker(), - params: common::AlgorithmParameters::EcDsaWithSha256, + params: common::AlgorithmParameters::EcDsaWithSha256(None), }), (KeyType::Ec, HashType::Sha384) => Ok(common::AlgorithmIdentifier { oid: asn1::DefinedByMarker::marker(), - params: common::AlgorithmParameters::EcDsaWithSha384, + params: common::AlgorithmParameters::EcDsaWithSha384(None), }), (KeyType::Ec, HashType::Sha512) => Ok(common::AlgorithmIdentifier { oid: asn1::DefinedByMarker::marker(), - params: common::AlgorithmParameters::EcDsaWithSha512, + params: common::AlgorithmParameters::EcDsaWithSha512(None), }), (KeyType::Ec, HashType::Sha3_224) => Ok(common::AlgorithmIdentifier { oid: asn1::DefinedByMarker::marker(), @@ -483,10 +483,10 @@ fn identify_key_type_for_algorithm_params( | common::AlgorithmParameters::RsaWithSha3_384(..) | common::AlgorithmParameters::RsaWithSha3_512(..) | common::AlgorithmParameters::RsaPss(..) => Ok(KeyType::Rsa), - common::AlgorithmParameters::EcDsaWithSha224 - | common::AlgorithmParameters::EcDsaWithSha256 - | common::AlgorithmParameters::EcDsaWithSha384 - | common::AlgorithmParameters::EcDsaWithSha512 + common::AlgorithmParameters::EcDsaWithSha224(..) + | common::AlgorithmParameters::EcDsaWithSha256(..) + | common::AlgorithmParameters::EcDsaWithSha384(..) + | common::AlgorithmParameters::EcDsaWithSha512(..) | common::AlgorithmParameters::EcDsaWithSha3_224 | common::AlgorithmParameters::EcDsaWithSha3_256 | common::AlgorithmParameters::EcDsaWithSha3_384 @@ -616,10 +616,10 @@ pub(crate) fn identify_signature_algorithm_parameters<'p>( .call0()?; Ok(pkcs) } - common::AlgorithmParameters::EcDsaWithSha224 - | common::AlgorithmParameters::EcDsaWithSha256 - | common::AlgorithmParameters::EcDsaWithSha384 - | common::AlgorithmParameters::EcDsaWithSha512 + common::AlgorithmParameters::EcDsaWithSha224(_) + | common::AlgorithmParameters::EcDsaWithSha256(_) + | common::AlgorithmParameters::EcDsaWithSha384(_) + | common::AlgorithmParameters::EcDsaWithSha512(_) | common::AlgorithmParameters::EcDsaWithSha3_224 | common::AlgorithmParameters::EcDsaWithSha3_256 | common::AlgorithmParameters::EcDsaWithSha3_384 @@ -682,10 +682,22 @@ mod tests { &common::AlgorithmParameters::RsaWithSha3_512(Some(())), KeyType::Rsa, ), - (&common::AlgorithmParameters::EcDsaWithSha224, KeyType::Ec), - (&common::AlgorithmParameters::EcDsaWithSha256, KeyType::Ec), - (&common::AlgorithmParameters::EcDsaWithSha384, KeyType::Ec), - (&common::AlgorithmParameters::EcDsaWithSha512, KeyType::Ec), + ( + &common::AlgorithmParameters::EcDsaWithSha224(None), + KeyType::Ec, + ), + ( + &common::AlgorithmParameters::EcDsaWithSha256(None), + KeyType::Ec, + ), + ( + &common::AlgorithmParameters::EcDsaWithSha384(None), + KeyType::Ec, + ), + ( + &common::AlgorithmParameters::EcDsaWithSha512(None), + KeyType::Ec, + ), (&common::AlgorithmParameters::EcDsaWithSha3_224, KeyType::Ec), (&common::AlgorithmParameters::EcDsaWithSha3_256, KeyType::Ec), (&common::AlgorithmParameters::EcDsaWithSha3_384, KeyType::Ec), diff --git a/tests/x509/test_x509.py b/tests/x509/test_x509.py index 662cb9af2b8e..0bac1c271cfb 100644 --- a/tests/x509/test_x509.py +++ b/tests/x509/test_x509.py @@ -5199,6 +5199,21 @@ def test_load_ecdsa_cert(self, backend): cert.signature_algorithm_parameters, ) + def test_load_ecdsa_cert_null_alg_params(self, backend): + """ + This test verifies that we successfully load certificates with encoded + null parameters in the signature AlgorithmIdentifier. This is invalid, + but Java 11 (up to at least 11.0.19) generates certificates with this + encoding so we need to tolerate it at the moment. + """ + with pytest.warns(utils.DeprecatedIn41): + cert = _load_cert( + os.path.join("x509", "custom", "ecdsa_null_alg.pem"), + x509.load_pem_x509_certificate, + ) + assert isinstance(cert.signature_hash_algorithm, hashes.SHA256) + assert isinstance(cert.public_key(), ec.EllipticCurvePublicKey) + def test_load_bitstring_dn(self, backend): cert = _load_cert( os.path.join("x509", "scottishpower-bitstring-dn.pem"), diff --git a/vectors/cryptography_vectors/x509/custom/ecdsa_null_alg.pem b/vectors/cryptography_vectors/x509/custom/ecdsa_null_alg.pem new file mode 100644 index 000000000000..327ad553ae7f --- /dev/null +++ b/vectors/cryptography_vectors/x509/custom/ecdsa_null_alg.pem @@ -0,0 +1,9 @@ +-----BEGIN CERTIFICATE----- +MIIBNDCB2aADAgECAgRnI7YfMAwGCCqGSM49BAMCBQAwDzENMAsGA1UEAxMEdGVz +dDAeFw0yMzA1MzExMjI5MDNaFw0yNDA1MjUxMjI5MDNaMA8xDTALBgNVBAMTBHRl +c3QwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAS2LuMFnF5OcuYcldiufvppacg2 +8fF/KeJ/4QLMOTbnkatgx5wNPOUvlkzfT31MscwYyzkv1oTqe58iQ+R75C27oyEw +HzAdBgNVHQ4EFgQUD6COpW8C9Ns86r2BDE0jP0teCTswDAYIKoZIzj0EAwIFAANI +ADBFAiBKOlNsFpW6Bz7CK7Z5zXrCetnMiSH3NrbKSZBXJV62KQIhAKmjGu3rxlJr +xXpK+Uz8AsoFJ0BlgqPpdMtTGSrDq1AN +-----END CERTIFICATE-----