From 0b036170842e33c98f2132c9ba00ff97f07c709f Mon Sep 17 00:00:00 2001 From: Marko Kreen Date: Mon, 15 Jun 2020 00:12:06 +0300 Subject: Unify X.509 signature algorithm validation (#5276) - Use common implementation - OCSP signing was using different validation - Check if private key is usable for signing --- .../hazmat/backends/openssl/backend.py | 55 +++++++--------------- src/cryptography/x509/ocsp.py | 10 ---- tests/x509/test_x509.py | 55 ++++++++++++++++++++++ 3 files changed, 72 insertions(+), 48 deletions(-) diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py index 0a6b7bc3..952c0f65 100644 --- a/src/cryptography/hazmat/backends/openssl/backend.py +++ b/src/cryptography/hazmat/backends/openssl/backend.py @@ -732,26 +732,33 @@ class Backend(object): def create_cmac_ctx(self, algorithm): return _CMACContext(self, algorithm) - def create_x509_csr(self, builder, private_key, algorithm): - if not isinstance(builder, x509.CertificateSigningRequestBuilder): - raise TypeError('Builder type mismatch.') - + def _x509_check_signature_params(self, private_key, algorithm): if isinstance(private_key, (ed25519.Ed25519PrivateKey, ed448.Ed448PrivateKey)): if algorithm is not None: raise ValueError( "algorithm must be None when signing via ed25519 or ed448" ) + elif not isinstance(private_key, (rsa.RSAPrivateKey, dsa.DSAPrivateKey, + ec.EllipticCurvePrivateKey)): + raise TypeError( + "Key must be rsa, dsa, ec, ed25519 or ed448 private key." + ) elif not isinstance(algorithm, hashes.HashAlgorithm): - raise TypeError('Algorithm must be a registered hash algorithm.') + raise TypeError("Algorithm must be a registered hash algorithm.") elif ( isinstance(algorithm, hashes.MD5) and not isinstance(private_key, rsa.RSAPrivateKey) ): raise ValueError( - "MD5 is not a supported hash algorithm for EC/DSA CSRs" + "MD5 hash algorithm is only supported with RSA keys" ) + def create_x509_csr(self, builder, private_key, algorithm): + if not isinstance(builder, x509.CertificateSigningRequestBuilder): + raise TypeError('Builder type mismatch.') + self._x509_check_signature_params(private_key, algorithm) + # Resolve the signature algorithm. evp_md = self._evp_md_x509_null_if_eddsa(private_key, algorithm) @@ -820,22 +827,7 @@ class Backend(object): def create_x509_certificate(self, builder, private_key, algorithm): if not isinstance(builder, x509.CertificateBuilder): raise TypeError('Builder type mismatch.') - if isinstance(private_key, - (ed25519.Ed25519PrivateKey, ed448.Ed448PrivateKey)): - if algorithm is not None: - raise ValueError( - "algorithm must be None when signing via ed25519 or ed448" - ) - elif 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 only (reluctantly) supported for RSA certificates" - ) + self._x509_check_signature_params(private_key, algorithm) # Resolve the signature algorithm. evp_md = self._evp_md_x509_null_if_eddsa(private_key, algorithm) @@ -932,22 +924,7 @@ class Backend(object): def create_x509_crl(self, builder, private_key, algorithm): if not isinstance(builder, x509.CertificateRevocationListBuilder): raise TypeError('Builder type mismatch.') - if isinstance(private_key, - (ed25519.Ed25519PrivateKey, ed448.Ed448PrivateKey)): - if algorithm is not None: - raise ValueError( - "algorithm must be None when signing via ed25519 or ed448" - ) - elif 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" - ) + self._x509_check_signature_params(private_key, algorithm) evp_md = self._evp_md_x509_null_if_eddsa(private_key, algorithm) @@ -1559,6 +1536,8 @@ class Backend(object): return _OCSPRequest(self, ocsp_req) def _create_ocsp_basic_response(self, builder, private_key, algorithm): + self._x509_check_signature_params(private_key, algorithm) + basic = self._lib.OCSP_BASICRESP_new() self.openssl_assert(basic != self._ffi.NULL) basic = self._ffi.gc(basic, self._lib.OCSP_BASICRESP_free) diff --git a/src/cryptography/x509/ocsp.py b/src/cryptography/x509/ocsp.py index 4e0c985a..7db92b90 100644 --- a/src/cryptography/x509/ocsp.py +++ b/src/cryptography/x509/ocsp.py @@ -12,7 +12,6 @@ import six from cryptography import x509 from cryptography.hazmat.primitives import hashes -from cryptography.hazmat.primitives.asymmetric import ed25519, ed448 from cryptography.x509.base import ( _EARLIEST_UTC_TIME, _convert_to_naive_utc_time, _reject_duplicate_extension ) @@ -242,15 +241,6 @@ class OCSPResponseBuilder(object): if self._responder_id is None: raise ValueError("You must add a responder_id before signing") - if isinstance(private_key, - (ed25519.Ed25519PrivateKey, ed448.Ed448PrivateKey)): - if algorithm is not None: - raise ValueError( - "algorithm must be None when signing via ed25519 or ed448" - ) - elif not isinstance(algorithm, hashes.HashAlgorithm): - raise TypeError("Algorithm must be a registered hash algorithm.") - return backend.create_ocsp_response( OCSPResponseStatus.SUCCESSFUL, self, private_key, algorithm ) diff --git a/tests/x509/test_x509.py b/tests/x509/test_x509.py index 7c45660f..38fe6bf8 100644 --- a/tests/x509/test_x509.py +++ b/tests/x509/test_x509.py @@ -4674,6 +4674,61 @@ class TestEd448Certificate(object): assert cert.signature_algorithm_oid == SignatureAlgorithmOID.ED448 +@pytest.mark.requires_backend_interface(interface=X509Backend) +class TestSignatureRejection(object): + """Test if signing rejects DH keys properly. + """ + def load_key(self, backend): + data = load_vectors_from_file( + os.path.join("asymmetric", "DH", "dhkey.pem"), + lambda pemfile: pemfile.read(), + mode="rb" + ) + return serialization.load_pem_private_key(data, None, backend) + + def test_crt_signing_check(self, backend): + issuer_private_key = self.load_key(backend) + public_key = RSA_KEY_2048.private_key(backend).public_key() + not_valid_before = datetime.datetime(2020, 1, 1, 1, 1) + not_valid_after = datetime.datetime(2050, 12, 31, 8, 30) + builder = x509.CertificateBuilder().serial_number( + 777 + ).issuer_name(x509.Name([ + x509.NameAttribute(NameOID.COUNTRY_NAME, u'US'), + ])).subject_name(x509.Name([ + x509.NameAttribute(NameOID.COUNTRY_NAME, u'US'), + ])).public_key( + public_key + ).not_valid_before( + not_valid_before + ).not_valid_after( + not_valid_after + ) + + with pytest.raises(TypeError): + builder.sign(issuer_private_key, hashes.SHA256(), backend) + + def test_csr_signing_check(self, backend): + private_key = self.load_key(backend) + builder = x509.CertificateSigningRequestBuilder().subject_name( + x509.Name([x509.NameAttribute(NameOID.COUNTRY_NAME, u'US')]) + ) + + with pytest.raises(TypeError): + builder.sign(private_key, hashes.SHA256(), backend) + + def test_crl_signing_check(self, backend): + private_key = self.load_key(backend) + last_time = datetime.datetime.utcnow().replace(microsecond=0) + next_time = last_time + builder = x509.CertificateRevocationListBuilder().issuer_name( + x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, u"CA")]) + ).last_update(last_time).next_update(next_time) + + with pytest.raises(TypeError): + builder.sign(private_key, hashes.SHA256(), backend) + + def test_random_serial_number(monkeypatch): sample_data = os.urandom(20) -- cgit v1.2.3