Merge pull request #1380 from sarthurdev/ovpn-multi-ca

openvpn: T4485: Accept multiple tls ca-certificate values

7 files changed, 137 insertions, 29 deletions

diff --git a/interface-definitions/include/pki/ca-certificate-multi.xml.i b/interface-definitions/include/pki/ca-certificate-multi.xml.i
new file mode 100644
index 000000000..646131b54
--- /dev/null
+++ b/interface-definitions/include/pki/ca-certificate-multi.xml.i
@@ -0,0 +1,15 @@
+<!-- include start from pki/ca-certificate-multi.xml.i -->
+<leafNode name="ca-certificate">
+  <properties>
+    <help>Certificate Authority chain in PKI configuration</help>
+    <completionHelp>
+      <path>pki ca</path>
+    </completionHelp>
+    <valueHelp>
+      <format>txt</format>
+      <description>Name of CA in PKI configuration</description>
+    </valueHelp>
+    <multi/>
+  </properties>
+</leafNode>
+<!-- include end -->
diff --git a/interface-definitions/interfaces-openvpn.xml.in b/interface-definitions/interfaces-openvpn.xml.in
index f1cbf8468..6cbd91ff4 100644
--- a/interface-definitions/interfaces-openvpn.xml.in
+++ b/interface-definitions/interfaces-openvpn.xml.in
@@ -741,7 +741,7 @@
                 </properties>
               </leafNode>
               #include <include/pki/certificate.xml.i>
-              #include <include/pki/ca-certificate.xml.i>
+              #include <include/pki/ca-certificate-multi.xml.i>
               <leafNode name="dh-params">
                 <properties>
                   <help>Diffie Hellman parameters (server only)</help>
diff --git a/python/vyos/pki.py b/python/vyos/pki.py
index fd91fc9bf..cd15e3878 100644
--- a/python/vyos/pki.py
+++ b/python/vyos/pki.py
@@ -332,6 +332,54 @@ def verify_certificate(cert, ca_cert):
     except InvalidSignature:
         return False
 
+def verify_crl(crl, ca_cert):
+    # Verify CRL was signed by specified CA
+    if ca_cert.subject != crl.issuer:
+        return False
+
+    ca_public_key = ca_cert.public_key()
+    try:
+        if isinstance(ca_public_key, rsa.RSAPublicKeyWithSerialization):
+            ca_public_key.verify(
+                crl.signature,
+                crl.tbs_certlist_bytes,
+                padding=padding.PKCS1v15(),
+                algorithm=crl.signature_hash_algorithm)
+        elif isinstance(ca_public_key, dsa.DSAPublicKeyWithSerialization):
+            ca_public_key.verify(
+                crl.signature,
+                crl.tbs_certlist_bytes,
+                algorithm=crl.signature_hash_algorithm)
+        elif isinstance(ca_public_key, ec.EllipticCurvePublicKeyWithSerialization):
+            ca_public_key.verify(
+                crl.signature,
+                crl.tbs_certlist_bytes,
+                signature_algorithm=ec.ECDSA(crl.signature_hash_algorithm))
+        else:
+            return False # We cannot verify it
+        return True
+    except InvalidSignature:
+        return False
+
+def verify_ca_chain(sorted_names, pki_node):
+    if len(sorted_names) == 1: # Single cert, no chain
+        return True
+
+    for name in sorted_names:
+        cert = load_certificate(pki_node[name]['certificate'])
+        verified = False
+        for ca_name in sorted_names:
+            if name == ca_name:
+                continue
+            ca_cert = load_certificate(pki_node[ca_name]['certificate'])
+            if verify_certificate(cert, ca_cert):
+                verified = True
+                break
+        if not verified and name != sorted_names[-1]:
+            # Only permit top-most certificate to fail verify (e.g. signed by public CA not explicitly in chain)
+            return False
+    return True
+
 # Certificate chain
 
 def find_parent(cert, ca_certs):
@@ -357,3 +405,16 @@ def find_chain(cert, ca_certs):
             chain.append(parent)
 
     return chain
+
+def sort_ca_chain(ca_names, pki_node):
+    def ca_cmp(ca_name1, ca_name2, pki_node):
+        cert1 = load_certificate(pki_node[ca_name1]['certificate'])
+        cert2 = load_certificate(pki_node[ca_name2]['certificate'])
+
+        if verify_certificate(cert1, cert2): # cert1 is child of cert2
+            return -1
+        return 1
+
+    from functools import cmp_to_key
+    return sorted(ca_names, key=cmp_to_key(lambda cert1, cert2: ca_cmp(cert1, cert2, pki_node)))
+
diff --git a/python/vyos/util.py b/python/vyos/util.py
index 0d62fbfe9..bee5d7aec 100644
--- a/python/vyos/util.py
+++ b/python/vyos/util.py
@@ -197,7 +197,7 @@ def read_file(fname, defaultonfailure=None):
             return defaultonfailure
         raise e
 
-def write_file(fname, data, defaultonfailure=None, user=None, group=None, mode=None):
+def write_file(fname, data, defaultonfailure=None, user=None, group=None, mode=None, append=False):
     """
     Write content of data to given fname, should defaultonfailure be not None,
     it is returned on failure to read.
@@ -212,7 +212,7 @@ def write_file(fname, data, defaultonfailure=None, user=None, group=None, mode=N
     try:
         """ Write a file to string """
         bytes = 0
-        with open(fname, 'w') as f:
+        with open(fname, 'w' if not append else 'a') as f:
             bytes = f.write(data)
         chown(fname, user, group)
         chmod(fname, mode)
diff --git a/smoketest/configs/dialup-router-medium-vpn b/smoketest/configs/dialup-router-medium-vpn
index 63d955738..fb8ed2714 100644
--- a/smoketest/configs/dialup-router-medium-vpn
+++ b/smoketest/configs/dialup-router-medium-vpn
@@ -120,7 +120,7 @@ interfaces {
         persistent-tunnel
         remote-host 192.0.2.10
         tls {
-            ca-cert-file /config/auth/ovpn_test_ca.pem
+            ca-cert-file /config/auth/ovpn_test_chain.pem
             cert-file /config/auth/ovpn_test_server.pem
             key-file /config/auth/ovpn_test_server.key
             auth-file /config/auth/ovpn_test_tls_auth.key
@@ -152,7 +152,7 @@ interfaces {
         remote-host 01.foo.com
         remote-port 1194
         tls {
-            ca-cert-file /config/auth/ovpn_test_ca.pem
+            ca-cert-file /config/auth/ovpn_test_chain.pem
             auth-file /config/auth/ovpn_test_tls_auth.key
         }
     }
diff --git a/src/conf_mode/interfaces-openvpn.py b/src/conf_mode/interfaces-openvpn.py
index 4750ca3e8..280a62b9a 100755
--- a/src/conf_mode/interfaces-openvpn.py
+++ b/src/conf_mode/interfaces-openvpn.py
@@ -39,6 +39,8 @@ from vyos.configverify import verify_mirror_redirect
 from vyos.ifconfig import VTunIf
 from vyos.pki import load_dh_parameters
 from vyos.pki import load_private_key
+from vyos.pki import sort_ca_chain
+from vyos.pki import verify_ca_chain
 from vyos.pki import wrap_certificate
 from vyos.pki import wrap_crl
 from vyos.pki import wrap_dh_parameters
@@ -148,8 +150,14 @@ def verify_pki(openvpn):
         if 'ca_certificate' not in tls:
             raise ConfigError(f'Must specify "tls ca-certificate" on openvpn interface {interface}')
 
-        if tls['ca_certificate'] not in pki['ca']:
-            raise ConfigError(f'Invalid CA certificate on openvpn interface {interface}')
+        for ca_name in tls['ca_certificate']:
+            if ca_name not in pki['ca']:
+                raise ConfigError(f'Invalid CA certificate on openvpn interface {interface}')
+
+        if len(tls['ca_certificate']) > 1:
+            sorted_chain = sort_ca_chain(tls['ca_certificate'], pki['ca'])
+            if not verify_ca_chain(sorted_chain, pki['ca']):
+                raise ConfigError(f'CA certificates are not a valid chain')
 
         if mode != 'client' and 'auth_key' not in tls:
             if 'certificate' not in tls:
@@ -516,21 +524,28 @@ def generate_pki_files(openvpn):
 
     if tls:
         if 'ca_certificate' in tls:
-            cert_name = tls['ca_certificate']
-            pki_ca = pki['ca'][cert_name]
+            cert_path = os.path.join(cfg_dir, f'{interface}_ca.pem')
+            crl_path = os.path.join(cfg_dir, f'{interface}_crl.pem')
 
-            if 'certificate' in pki_ca:
-                cert_path = os.path.join(cfg_dir, f'{interface}_ca.pem')
-                write_file(cert_path, wrap_certificate(pki_ca['certificate']),
-                           user=user, group=group, mode=0o600)
+            if os.path.exists(cert_path):
+                os.unlink(cert_path)
+
+            if os.path.exists(crl_path):
+                os.unlink(crl_path)
+
+            for cert_name in sort_ca_chain(tls['ca_certificate'], pki['ca']):
+                pki_ca = pki['ca'][cert_name]
+
+                if 'certificate' in pki_ca:
+                    write_file(cert_path, wrap_certificate(pki_ca['certificate']) + "\n",
+                               user=user, group=group, mode=0o600, append=True)
 
-            if 'crl' in pki_ca:
-                for crl in pki_ca['crl']:
-                    crl_path = os.path.join(cfg_dir, f'{interface}_crl.pem')
-                    write_file(crl_path, wrap_crl(crl), user=user, group=group,
-                               mode=0o600)
+                if 'crl' in pki_ca:
+                    for crl in pki_ca['crl']:
+                        write_file(crl_path, wrap_crl(crl) + "\n", user=user, group=group,
+                                   mode=0o600, append=True)
 
-                openvpn['tls']['crl'] = True
+                    openvpn['tls']['crl'] = True
 
         if 'certificate' in tls:
             cert_name = tls['certificate']
diff --git a/src/migration-scripts/interfaces/24-to-25 b/src/migration-scripts/interfaces/24-to-25
index 93ce9215f..4095f2a3e 100755
--- a/src/migration-scripts/interfaces/24-to-25
+++ b/src/migration-scripts/interfaces/24-to-25
@@ -20,6 +20,7 @@
 import os
 import sys
 from vyos.configtree import ConfigTree
+from vyos.pki import CERT_BEGIN
 from vyos.pki import load_certificate
 from vyos.pki import load_crl
 from vyos.pki import load_dh_parameters
@@ -27,6 +28,7 @@ from vyos.pki import load_private_key
 from vyos.pki import encode_certificate
 from vyos.pki import encode_dh_parameters
 from vyos.pki import encode_private_key
+from vyos.pki import verify_crl
 from vyos.util import run
 
 def wrapped_pem_to_config_value(pem):
@@ -129,6 +131,8 @@ if config.exists(base):
 
             config.delete(base + [interface, 'tls', 'crypt-file'])
 
+        ca_certs = {}
+
         if config.exists(x509_base + ['ca-cert-file']):
             if not config.exists(pki_base + ['ca']):
                 config.set(pki_base + ['ca'])
@@ -136,20 +140,27 @@ if config.exists(base):
 
             cert_file = config.return_value(x509_base + ['ca-cert-file'])
             cert_path = os.path.join(AUTH_DIR, cert_file)
-            cert = None
 
             if os.path.isfile(cert_path):
                 if not os.access(cert_path, os.R_OK):
                     run(f'sudo chmod 644 {cert_path}')
 
                 with open(cert_path, 'r') as f:
-                    cert_data = f.read()
-                    cert = load_certificate(cert_data, wrap_tags=False)
-
-            if cert:
-                cert_pem = encode_certificate(cert)
-                config.set(pki_base + ['ca', pki_name, 'certificate'], value=wrapped_pem_to_config_value(cert_pem))
-                config.set(x509_base + ['ca-certificate'], value=pki_name)
+                    certs_str = f.read()
+                    certs_data = certs_str.split(CERT_BEGIN)
+                    index = 1
+                    for cert_data in certs_data[1:]:
+                        cert = load_certificate(CERT_BEGIN + cert_data, wrap_tags=False)
+
+                        if cert:
+                            ca_certs[f'{pki_name}_{index}'] = cert
+                            cert_pem = encode_certificate(cert)
+                            config.set(pki_base + ['ca', f'{pki_name}_{index}', 'certificate'], value=wrapped_pem_to_config_value(cert_pem))
+                            config.set(x509_base + ['ca-certificate'], value=f'{pki_name}_{index}', replace=False)
+                        else:
+                            print(f'Failed to migrate CA certificate on openvpn interface {interface}')
+
+                        index += 1
             else:
                 print(f'Failed to migrate CA certificate on openvpn interface {interface}')
 
@@ -163,6 +174,7 @@ if config.exists(base):
             crl_file = config.return_value(x509_base + ['crl-file'])
             crl_path = os.path.join(AUTH_DIR, crl_file)
             crl = None
+            crl_ca_name = None
 
             if os.path.isfile(crl_path):
                 if not os.access(crl_path, os.R_OK):
@@ -172,9 +184,14 @@ if config.exists(base):
                     crl_data = f.read()
                     crl = load_crl(crl_data, wrap_tags=False)
 
-            if crl:
+                    for ca_name, ca_cert in ca_certs.items():
+                        if verify_crl(crl, ca_cert):
+                            crl_ca_name = ca_name
+                            break
+
+            if crl and crl_ca_name:
                 crl_pem = encode_certificate(crl)
-                config.set(pki_base + ['ca', pki_name, 'crl'], value=wrapped_pem_to_config_value(crl_pem))
+                config.set(pki_base + ['ca', crl_ca_name, 'crl'], value=wrapped_pem_to_config_value(crl_pem))
             else:
                 print(f'Failed to migrate CRL on openvpn interface {interface}')