Backport tolerate (#9006)

* tolerate NULL params in ECDSA SHA2 AlgorithmIdentifier (#9002)

* tolerate NULL params in ECDSA SHA2 AlgorithmIdentifier

Java 11 does this incorrectly. It was fixed in Java16+ and they are
planning to do a backport, but we'll need to tolerate this invalid
encoding for a while.

* test both inner and outer

* changelog entry

* language

Author: reaperhulk

CHANGELOG.rst

@@ -1,6 +1,14 @@
 Changelog
 =========
 
+.. _v41-0-1:
+
+41.0.1 - 2023-06-01
+~~~~~~~~~~~~~~~~~~~
+
+* Temporarily allow invalid ECDSA signature algorithm parameters in X.509
+  certificates, which are generated by older versions of Java.
+
 .. _v41-0-0:
 
 41.0.0 - 2023-05-30

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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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<asn1::Null>),
     #[defined_by(oid::ECDSA_WITH_SHA256_OID)]
-    EcDsaWithSha256,
+    EcDsaWithSha256(Option<asn1::Null>),
     #[defined_by(oid::ECDSA_WITH_SHA384_OID)]
-    EcDsaWithSha384,
+    EcDsaWithSha384(Option<asn1::Null>),
     #[defined_by(oid::ECDSA_WITH_SHA512_OID)]
-    EcDsaWithSha512,
+    EcDsaWithSha512(Option<asn1::Null>),
 
     #[defined_by(oid::ECDSA_WITH_SHA3_224_OID)]
     EcDsaWithSha3_224,

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<'_>,

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),

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"),

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-----