Verify external CA's basic constraint pathlen

IPA no verifies that intermediate certs of external CAs have a basic
constraint path len of at least 1 and increasing.

Fixes: https://pagure.io/freeipa/issue/7877
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Fraser Tweedale <ftweedal@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

ipapython/certdb.py

@@ -541,6 +541,9 @@ class NSSDatabase:
     def get_trust_chain(self, nickname):
         """Return names of certs in a given cert's trust chain
 
+        The list starts with root ca, then first intermediate CA, second
+        intermediate, and so on.
+
         :param nickname: Name of the cert
         :return: List of certificate names
         """
@@ -912,7 +915,7 @@ class NSSDatabase:
         except ValueError:
             raise ValueError('invalid for server %s' % hostname)
 
-    def verify_ca_cert_validity(self, nickname):
+    def verify_ca_cert_validity(self, nickname, minpathlen=None):
         cert = self.get_cert(nickname)
 
         if not cert.subject:
@@ -926,6 +929,15 @@ class NSSDatabase:
 
         if not bc.value.ca:
             raise ValueError("not a CA certificate")
+        if minpathlen is not None:
+            # path_length is None means no limitation
+            pl = bc.value.path_length
+            if pl is not None and pl < minpathlen:
+                raise ValueError(
+                    "basic contraint pathlen {}, must be at least {}".format(
+                        pl, minpathlen
+                    )
+                )
 
         try:
             ski = cert.extensions.get_extension_for_class(

ipaserver/install/installutils.py

@@ -893,9 +893,12 @@ def load_pkcs12(cert_files, key_password, key_nickname, ca_cert_files,
                 "The full certificate chain is not present in %s" %
                 (", ".join(cert_files)))
 
-        for nickname in trust_chain[1:]:
+        # verify CA validity and pathlen. The trust_chain list is in reverse
+        # order. trust_chain[1] is the first intermediate CA cert and must
+        # have pathlen >= 0.
+        for minpathlen, nickname in enumerate(trust_chain[1:], start=0):
             try:
-                nssdb.verify_ca_cert_validity(nickname)
+                nssdb.verify_ca_cert_validity(nickname, minpathlen)
             except ValueError as e:
                 raise ScriptError(
                     "CA certificate %s in %s is not valid: %s" %
@@ -1038,9 +1041,12 @@ def load_external_cert(files, ca_subject):
                 "missing certificate with subject '%s'" %
                 (", ".join(files), issuer))
 
-        for nickname in trust_chain:
+        # verify CA validity and pathlen. The trust_chain list is in reverse
+        # order. The first entry is the signed IPA-CA and must have a
+        # pathlen of >= 0.
+        for minpathlen, nickname in enumerate(trust_chain, start=0):
             try:
-                nssdb.verify_ca_cert_validity(nickname)
+                nssdb.verify_ca_cert_validity(nickname, minpathlen)
             except ValueError as e:
                 cert, subject, issuer = cache[nickname]
                 raise ScriptError(

ipatests/create_external_ca.py

@@ -38,7 +38,7 @@ class ExternalCA:
         self.now = datetime.datetime.utcnow()
         self.delta = datetime.timedelta(days=days)
 
-    def create_ca(self, cn=ISSUER_CN):
+    def create_ca(self, cn=ISSUER_CN, path_length=None):
         """Create root CA.
 
         :returns: bytes -- Root CA in PEM format.
@@ -79,7 +79,7 @@ class ExternalCA:
         )
 
         builder = builder.add_extension(
-            x509.BasicConstraints(ca=True, path_length=None),
+            x509.BasicConstraints(ca=True, path_length=path_length),
             critical=True,
         )
 

ipatests/prci_definitions/nightly_f28.yaml

@@ -1152,7 +1152,7 @@ jobs:
       class: RunPytest
       args:
         build_url: '{fedora-28/build_url}'
-        test_suite: test_integration/test_external_ca.py::TestExternalCAInvalidCert
+        test_suite: test_integration/test_external_ca.py::TestExternalCAInvalidCert test_integration/test_external_ca.py::TestExternalCAInvalidIntermediate
         template: *ci-master-f28
         timeout: 3600
         topology: *master_1repl

ipatests/prci_definitions/nightly_master.yaml

@@ -1152,7 +1152,7 @@ jobs:
       class: RunPytest
       args:
         build_url: '{fedora-29/build_url}'
-        test_suite: test_integration/test_external_ca.py::TestExternalCAInvalidCert
+        test_suite: test_integration/test_external_ca.py::TestExternalCAInvalidCert test_integration/test_external_ca.py::TestExternalCAInvalidIntermediate
         template: *ci-master-f29
         timeout: 3600
         topology: *master_1repl

ipatests/prci_definitions/nightly_rawhide.yaml

@@ -1152,7 +1152,7 @@ jobs:
       class: RunPytest
       args:
         build_url: '{fedora-rawhide/build_url}'
-        test_suite: test_integration/test_external_ca.py::TestExternalCAInvalidCert
+        test_suite: test_integration/test_external_ca.py::TestExternalCAInvalidCert test_integration/test_external_ca.py::TestExternalCAInvalidIntermediate
         template: *ci-master-frawhide
         timeout: 3600
         topology: *master_1repl

ipatests/pytest_ipa/integration/tasks.py

@@ -1632,7 +1632,8 @@ def add_dns_zone(master, zone, skip_overlap_check=False,
         logger.debug('Zone %s already added.', zone)
 
 
-def sign_ca_and_transport(host, csr_name, root_ca_name, ipa_ca_name):
+def sign_ca_and_transport(host, csr_name, root_ca_name, ipa_ca_name,
+                          root_ca_path_length=None, ipa_ca_path_length=1):
     """
     Sign ipa csr and save signed CA together with root CA back to the host.
     Returns root CA and IPA CA paths on the host.
@@ -1645,9 +1646,9 @@ def sign_ca_and_transport(host, csr_name, root_ca_name, ipa_ca_name):
 
     external_ca = ExternalCA()
     # Create root CA
-    root_ca = external_ca.create_ca()
+    root_ca = external_ca.create_ca(path_length=root_ca_path_length)
     # Sign CSR
-    ipa_ca = external_ca.sign_csr(ipa_csr)
+    ipa_ca = external_ca.sign_csr(ipa_csr, path_length=ipa_ca_path_length)
 
     root_ca_fname = os.path.join(test_dir, root_ca_name)
     ipa_ca_fname = os.path.join(test_dir, ipa_ca_name)
@@ -1656,7 +1657,7 @@ def sign_ca_and_transport(host, csr_name, root_ca_name, ipa_ca_name):
     host.put_file_contents(root_ca_fname, root_ca)
     host.put_file_contents(ipa_ca_fname, ipa_ca)
 
-    return (root_ca_fname, ipa_ca_fname)
+    return root_ca_fname, ipa_ca_fname
 
 
 def generate_ssh_keypair():

ipatests/test_integration/test_external_ca.py

@@ -80,7 +80,8 @@ def install_server_external_ca_step1(host, extra_args=()):
     )
 
 
-def install_server_external_ca_step2(host, ipa_ca_cert, root_ca_cert):
+def install_server_external_ca_step2(host, ipa_ca_cert, root_ca_cert,
+                                     raiseonerr=True):
     """Step 2 to install the ipa server with external ca"""
     args = ['ipa-server-install',
             '-a', host.config.admin_password,
@@ -88,7 +89,7 @@ def install_server_external_ca_step2(host, ipa_ca_cert, root_ca_cert):
             '--external-cert-file', ipa_ca_cert,
             '--external-cert-file', root_ca_cert]
 
-    cmd = host.run_command(args)
+    cmd = host.run_command(args, raiseonerr=raiseonerr)
     return cmd
 
 
@@ -333,6 +334,22 @@ class TestExternalCAInvalidCert(IntegrationTest):
         assert result.returncode == 1
 
 
+class TestExternalCAInvalidIntermediate(IntegrationTest):
+    """Test case for https://pagure.io/freeipa/issue/7877"""
+
+    def test_invalid_intermediate(self):
+        install_server_external_ca_step1(self.master)
+        root_ca_fname, ipa_ca_fname = tasks.sign_ca_and_transport(
+            self.master, paths.ROOT_IPA_CSR, ROOT_CA, IPA_CA,
+            root_ca_path_length=0
+        )
+        result = install_server_external_ca_step2(
+            self.master, ipa_ca_fname, root_ca_fname, raiseonerr=False
+        )
+        assert result.returncode > 0
+        assert "basic contraint pathlen" in result.stderr_text
+
+
 class TestExternalCAInstall(IntegrationTest):
     """install CA cert manually """