disallow MD5 in CertificateBuilder and CertificateSigningRequestBuilder (#3738)

* disallow MD5 in CertificateBuilder and CertificateSigningRequestBuilder

* only error on ECDSA and DSA

lots of duplication in tests here, bleh

* remove changelog entry, also handle this for CRLBuilder

* pep8

3 files changed, 157 insertions, 1 deletions

diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py
index 18238e1c..cf0300e0 100644
--- a/src/cryptography/hazmat/backends/openssl/backend.py
+++ b/src/cryptography/hazmat/backends/openssl/backend.py
@@ -662,6 +662,14 @@ class Backend(object):
         if not isinstance(algorithm, hashes.HashAlgorithm):
             raise TypeError('Algorithm must be a registered hash algorithm.')
 
+        if (
+            isinstance(algorithm, hashes.MD5) and not
+            isinstance(private_key, rsa.RSAPrivateKey)
+        ):
+            raise ValueError(
+                "MD5 is not a supported hash algorithm for EC/DSA CSRs"
+            )
+
         # Resolve the signature algorithm.
         evp_md = self._lib.EVP_get_digestbyname(
             algorithm.name.encode('ascii')
@@ -731,6 +739,14 @@ class Backend(object):
         if not isinstance(algorithm, hashes.HashAlgorithm):
             raise TypeError('Algorithm must be a registered hash algorithm.')
 
+        if (
+            isinstance(algorithm, hashes.MD5) and not
+            isinstance(private_key, rsa.RSAPrivateKey)
+        ):
+            raise ValueError(
+                "MD5 is not a supported hash algorithm for EC/DSA certificates"
+            )
+
         # Resolve the signature algorithm.
         evp_md = self._lib.EVP_get_digestbyname(
             algorithm.name.encode('ascii')
@@ -828,6 +844,14 @@ class Backend(object):
         if not isinstance(algorithm, hashes.HashAlgorithm):
             raise TypeError('Algorithm must be a registered hash algorithm.')
 
+        if (
+            isinstance(algorithm, hashes.MD5) and not
+            isinstance(private_key, rsa.RSAPrivateKey)
+        ):
+            raise ValueError(
+                "MD5 is not a supported hash algorithm for EC/DSA CRLs"
+            )
+
         evp_md = self._lib.EVP_get_digestbyname(
             algorithm.name.encode('ascii')
         )
diff --git a/tests/test_x509.py b/tests/test_x509.py
index 7a99ff3d..41ccbed8 100644
--- a/tests/test_x509.py
+++ b/tests/test_x509.py
@@ -34,6 +34,7 @@ from cryptography.x509.oid import (
 )
 
 from .hazmat.primitives.fixtures_dsa import DSA_KEY_2048
+from .hazmat.primitives.fixtures_ec import EC_KEY_SECP256R1
 from .hazmat.primitives.fixtures_rsa import RSA_KEY_2048, RSA_KEY_512
 from .hazmat.primitives.test_ec import _skip_curve_unsupported
 from .utils import load_vectors_from_file
@@ -1933,6 +1934,70 @@ class TestCertificateBuilder(object):
         with pytest.raises(TypeError):
             builder.sign(private_key, object(), backend)
 
+    @pytest.mark.requires_backend_interface(interface=RSABackend)
+    @pytest.mark.requires_backend_interface(interface=X509Backend)
+    def test_sign_rsa_with_md5(self, backend):
+        private_key = RSA_KEY_2048.private_key(backend)
+        builder = x509.CertificateBuilder()
+        builder = builder.subject_name(
+            x509.Name([x509.NameAttribute(NameOID.COUNTRY_NAME, u'US')])
+        ).issuer_name(
+            x509.Name([x509.NameAttribute(NameOID.COUNTRY_NAME, u'US')])
+        ).serial_number(
+            1
+        ).public_key(
+            private_key.public_key()
+        ).not_valid_before(
+            datetime.datetime(2002, 1, 1, 12, 1)
+        ).not_valid_after(
+            datetime.datetime(2032, 1, 1, 12, 1)
+        )
+        cert = builder.sign(private_key, hashes.MD5(), backend)
+        assert isinstance(cert.signature_hash_algorithm, hashes.MD5)
+
+    @pytest.mark.requires_backend_interface(interface=DSABackend)
+    @pytest.mark.requires_backend_interface(interface=X509Backend)
+    def test_sign_dsa_with_md5(self, backend):
+        private_key = DSA_KEY_2048.private_key(backend)
+        builder = x509.CertificateBuilder()
+        builder = builder.subject_name(
+            x509.Name([x509.NameAttribute(NameOID.COUNTRY_NAME, u'US')])
+        ).issuer_name(
+            x509.Name([x509.NameAttribute(NameOID.COUNTRY_NAME, u'US')])
+        ).serial_number(
+            1
+        ).public_key(
+            private_key.public_key()
+        ).not_valid_before(
+            datetime.datetime(2002, 1, 1, 12, 1)
+        ).not_valid_after(
+            datetime.datetime(2032, 1, 1, 12, 1)
+        )
+        with pytest.raises(ValueError):
+            builder.sign(private_key, hashes.MD5(), backend)
+
+    @pytest.mark.requires_backend_interface(interface=EllipticCurveBackend)
+    @pytest.mark.requires_backend_interface(interface=X509Backend)
+    def test_sign_ec_with_md5(self, backend):
+        _skip_curve_unsupported(backend, ec.SECP256R1())
+        private_key = EC_KEY_SECP256R1.private_key(backend)
+        builder = x509.CertificateBuilder()
+        builder = builder.subject_name(
+            x509.Name([x509.NameAttribute(NameOID.COUNTRY_NAME, u'US')])
+        ).issuer_name(
+            x509.Name([x509.NameAttribute(NameOID.COUNTRY_NAME, u'US')])
+        ).serial_number(
+            1
+        ).public_key(
+            private_key.public_key()
+        ).not_valid_before(
+            datetime.datetime(2002, 1, 1, 12, 1)
+        ).not_valid_after(
+            datetime.datetime(2032, 1, 1, 12, 1)
+        )
+        with pytest.raises(ValueError):
+            builder.sign(private_key, hashes.MD5(), backend)
+
     @pytest.mark.parametrize(
         "cdp",
         [
@@ -2623,6 +2688,41 @@ class TestCertificateSigningRequestBuilder(object):
             builder.sign(private_key, 'NotAHash', backend)
 
     @pytest.mark.requires_backend_interface(interface=RSABackend)
+    def test_sign_rsa_with_md5(self, backend):
+        private_key = RSA_KEY_2048.private_key(backend)
+
+        builder = x509.CertificateSigningRequestBuilder().subject_name(
+            x509.Name([
+                x509.NameAttribute(NameOID.ORGANIZATION_NAME, u'PyCA'),
+            ])
+        )
+        request = builder.sign(private_key, hashes.MD5(), backend)
+        assert isinstance(request.signature_hash_algorithm, hashes.MD5)
+
+    @pytest.mark.requires_backend_interface(interface=DSABackend)
+    def test_sign_dsa_with_md5(self, backend):
+        private_key = DSA_KEY_2048.private_key(backend)
+        builder = x509.CertificateSigningRequestBuilder().subject_name(
+            x509.Name([
+                x509.NameAttribute(NameOID.ORGANIZATION_NAME, u'PyCA'),
+            ])
+        )
+        with pytest.raises(ValueError):
+            builder.sign(private_key, hashes.MD5(), backend)
+
+    @pytest.mark.requires_backend_interface(interface=EllipticCurveBackend)
+    def test_sign_ec_with_md5(self, backend):
+        _skip_curve_unsupported(backend, ec.SECP256R1())
+        private_key = EC_KEY_SECP256R1.private_key(backend)
+        builder = x509.CertificateSigningRequestBuilder().subject_name(
+            x509.Name([
+                x509.NameAttribute(NameOID.ORGANIZATION_NAME, u'PyCA'),
+            ])
+        )
+        with pytest.raises(ValueError):
+            builder.sign(private_key, hashes.MD5(), backend)
+
+    @pytest.mark.requires_backend_interface(interface=RSABackend)
     def test_no_subject_name(self, backend):
         private_key = RSA_KEY_2048.private_key(backend)
 
diff --git a/tests/test_x509_crlbuilder.py b/tests/test_x509_crlbuilder.py
index f0306ef0..b3c789f6 100644
--- a/tests/test_x509_crlbuilder.py
+++ b/tests/test_x509_crlbuilder.py
@@ -19,6 +19,7 @@ from cryptography.hazmat.primitives.asymmetric import ec
 from cryptography.x509.oid import AuthorityInformationAccessOID, NameOID
 
 from .hazmat.primitives.fixtures_dsa import DSA_KEY_2048
+from .hazmat.primitives.fixtures_ec import EC_KEY_SECP256R1
 from .hazmat.primitives.fixtures_rsa import RSA_KEY_2048, RSA_KEY_512
 from .hazmat.primitives.test_ec import _skip_curve_unsupported
 
@@ -390,7 +391,7 @@ class TestCertificateRevocationListBuilder(object):
 
     @pytest.mark.requires_backend_interface(interface=EllipticCurveBackend)
     @pytest.mark.requires_backend_interface(interface=X509Backend)
-    def test_sign_ec_key_unsupported(self, backend):
+    def test_sign_ec_key(self, backend):
         _skip_curve_unsupported(backend, ec.SECP256R1())
         private_key = ec.generate_private_key(ec.SECP256R1(), backend)
         invalidity_date = x509.InvalidityDate(
@@ -433,6 +434,37 @@ class TestCertificateRevocationListBuilder(object):
         assert ext.critical is False
         assert ext.value == invalidity_date
 
+    @pytest.mark.requires_backend_interface(interface=DSABackend)
+    @pytest.mark.requires_backend_interface(interface=X509Backend)
+    def test_dsa_key_sign_md5(self, backend):
+        private_key = DSA_KEY_2048.private_key(backend)
+        last_time = datetime.datetime(2012, 1, 16, 22, 43)
+        next_time = datetime.datetime(2022, 1, 17, 6, 43)
+        builder = x509.CertificateRevocationListBuilder().issuer_name(
+            x509.Name([
+                x509.NameAttribute(NameOID.COMMON_NAME, u"cryptography.io CA")
+            ])
+        ).last_update(last_time).next_update(next_time)
+
+        with pytest.raises(ValueError):
+            builder.sign(private_key, hashes.MD5(), backend)
+
+    @pytest.mark.requires_backend_interface(interface=EllipticCurveBackend)
+    @pytest.mark.requires_backend_interface(interface=X509Backend)
+    def test_ec_key_sign_md5(self, backend):
+        _skip_curve_unsupported(backend, ec.SECP256R1())
+        private_key = EC_KEY_SECP256R1.private_key(backend)
+        last_time = datetime.datetime(2012, 1, 16, 22, 43)
+        next_time = datetime.datetime(2022, 1, 17, 6, 43)
+        builder = x509.CertificateRevocationListBuilder().issuer_name(
+            x509.Name([
+                x509.NameAttribute(NameOID.COMMON_NAME, u"cryptography.io CA")
+            ])
+        ).last_update(last_time).next_update(next_time)
+
+        with pytest.raises(ValueError):
+            builder.sign(private_key, hashes.MD5(), backend)
+
     @pytest.mark.requires_backend_interface(interface=RSABackend)
     @pytest.mark.requires_backend_interface(interface=X509Backend)
     def test_sign_with_revoked_certificates(self, backend):