Require certificate verification callback to pass for the chain to be…

… valid (pyca#2381)

Formerly unused 'cert_verif_cb' is called now. Caller of X509.verify() must
pass a callable which would return True for the certificate in the chain which
complies with the user's policy.

A minimal reasonalbe callback which can be used as default is provided.

dev-requirements.txt

@@ -1,5 +1,6 @@
 click
 coverage
+freezegun
 tox >= 2.4.1
 twine >= 1.8.0
 -e .[test,docs,docstest,pep8test]

src/cryptography/hazmat/backends/openssl/x509.py

@@ -215,10 +215,9 @@ def verify(self, intermediates, trusted_roots, cert_verif_cb):
         for cert in all_certs:
             g.add_vertex(cert)
         for cert in all_certs:
-            issuer_name = cert.issuer
             issuer = None
             for issuer_candidate in all_certs:
-                if issuer_candidate.subject == issuer_name:
+                if cert.is_issued_by(issuer_candidate):
                     issuer = issuer_candidate
                     break
             if issuer:
@@ -230,7 +229,9 @@ def verify(self, intermediates, trusted_roots, cert_verif_cb):
         trust_chains = []
         for c in g.all_paths(self, 'Fake Top'):
             del c[-1]       # Remove 'Fake Top'
-            trust_chains.append(c)
+            chain_is_good = all([cert_verif_cb(c, i) for i in range(len(c))])
+            if chain_is_good:
+                trust_chains.append(c)
 
         if len(trust_chains) == 0:
             raise x509.base.NoValidTrustChain

src/cryptography/x509/base.py

@@ -20,7 +20,8 @@
     ed448,
     rsa,
 )
-from cryptography.x509.extensions import Extension, ExtensionType, Extensions
+
+from cryptography.x509.extensions import Extension, ExtensionType, Extensions, ExtensionOID
 from cryptography.x509.name import Name
 from cryptography.x509.oid import ObjectIdentifier
 
@@ -228,7 +229,7 @@ def verify(self, intermediates, trusted_roots, cert_verif_cb):
         using certificates from `intermediates`.
         :param intermediates: list of Certificate objects to build chains from
         :param trusted_roots: list of Certificate objects which are trusted
-        :param cert_verif_cb: ignored for now. Will take (chain: List(Certificate), position: int)
+        :param cert_verif_cb: Will take (chain: List(Certificate), position: int)
         :return: Iterable of lists of Certificate objects. Each list is a trust
             chain starting with `self` and ending with one of `trusted_roots`.
         """
@@ -239,6 +240,41 @@ def extensions(self) -> Extensions:
         Returns an Extensions object containing a list of Revoked extensions.
         """
 
+def default_cert_verif_cb(chain, i):
+    """
+    Reasonable minimal default for the callback, applied to each certificate in the
+    trust chain.
+    For the chain to be valid, the callback must return True for all its certificates.
+    This example checks validity time, CA bits and path length.
+
+    :param chain: list of Certificate objects such as for i in range(len(chain) - 1),
+                  issuer(chain[i]) == subject(chain[i+1].
+                  The last (chain[-1]) certificate can be self-signed or not.
+    :param i: index of the checked certificate in the chain
+    :return: True if the certificate passes checks, False otherwise
+    """
+    cert = chain[i]
+
+    # Check that CA bit is set for all certs but the leaf
+    if i > 0:
+        bc = cert.extensions.get_extension_for_oid(
+            ExtensionOID.BASIC_CONSTRAINTS
+        )
+        if bc is not None:
+            if not bc.value.ca:
+                return False
+
+            # Check that path length constraint, if present, is fulfilled
+            if bc.value.path_length is not None and bc.value.path_length > i:
+                return False
+
+    # Check validity period
+    now = datetime.datetime.now()
+    if cert.not_valid_before > now or cert.not_valid_after < now:
+        return False
+
+    return True
+
 
 class CertificateRevocationList(metaclass=abc.ABCMeta):
     @abc.abstractmethod

tests/x509/test_verify.py

@@ -9,6 +9,7 @@
 
 from cryptography import x509
 from cryptography.hazmat.backends import default_backend
+from freezegun import freeze_time
 
 
 def load_test_pem(filename):
@@ -28,7 +29,20 @@ def test_correct_3link_chain(self):
 
         chains = leaf.verify(intermediates=[intermediate],
                              trusted_roots=[trusted_root],
-                             cert_verif_cb=lambda: True)
+                             cert_verif_cb=lambda c, i: True)
+
+        assert len(chains) == 1
+        chain = chains[0]
+        assert len(chain) == 3
+        assert chain[0] == leaf
+        assert chain[1] == intermediate
+        assert chain[2] == trusted_root
+
+        # Let the test pass also after expiration of the certificates
+        with freeze_time("2019-12-26"):
+            chains = leaf.verify(intermediates=[intermediate],
+                                 trusted_roots=[trusted_root],
+                                 cert_verif_cb=x509.base.default_cert_verif_cb)
 
         assert len(chains) == 1
         chain = chains[0]