Merge pull request #3377 from vyos/mergify/bp/sagitta/pr-3371

openconnect: T4982: Support defining minimum TLS version in openconnect VPN (backport #3371)
vyos · Apr 30, 2024 · 982221b · 982221b
2 parents 021cc22 + ef665ad
commit 982221b
Show file tree

Hide file tree

Showing 7 changed files with 104 additions and 28 deletions.
diff --git a/data/templates/ocserv/ocserv_config.j2 b/data/templates/ocserv/ocserv_config.j2
@@ -61,7 +61,15 @@ keepalive = 300
 dpd = 60
 mobile-dpd = 300
 switch-to-tcp-timeout = 30
+{% if tls_version_min == '1.0' %}
 tls-priorities = "NORMAL:%SERVER_PRECEDENCE:%COMPAT:-RSA:-VERS-SSL3.0:-ARCFOUR-128"
+{% elif tls_version_min == '1.1' %}
+tls-priorities = "NORMAL:%SERVER_PRECEDENCE:%COMPAT:-RSA:-VERS-SSL3.0:-ARCFOUR-128:-VERS-TLS1.0"
+{% elif tls_version_min == '1.2' %}
+tls-priorities = "NORMAL:%SERVER_PRECEDENCE:%COMPAT:-RSA:-VERS-SSL3.0:-ARCFOUR-128:-VERS-TLS1.0:-VERS-TLS1.1"
+{% elif tls_version_min == '1.3' %}
+tls-priorities = "NORMAL:%SERVER_PRECEDENCE:%COMPAT:-RSA:-VERS-SSL3.0:-ARCFOUR-128:-VERS-TLS1.0:-VERS-TLS1.1:-VERS-TLS1.2"
+{% endif %}
 auth-timeout = 240
 idle-timeout = 1200
 mobile-idle-timeout = 1800

diff --git a/interface-definitions/include/tls-version-min.xml.i b/interface-definitions/include/tls-version-min.xml.i
@@ -0,0 +1,29 @@
+<!-- include start from tls-version-min.xml.i -->
+<leafNode name="tls-version-min">
+  <properties>
+    <help>Specify the minimum required TLS version</help>
+    <completionHelp>
+      <list>1.0 1.1 1.2 1.3</list>
+    </completionHelp>
+    <valueHelp>
+      <format>1.0</format>
+      <description>TLS v1.0</description>
+    </valueHelp>
+    <valueHelp>
+      <format>1.1</format>
+      <description>TLS v1.1</description>
+    </valueHelp>
+    <valueHelp>
+      <format>1.2</format>
+      <description>TLS v1.2</description>
+    </valueHelp>
+    <valueHelp>
+      <format>1.3</format>
+      <description>TLS v1.3</description>
+    </valueHelp>
+    <constraint>
+      <regex>(1.0|1.1|1.2|1.3)</regex>
+    </constraint>
+  </properties>
+</leafNode>
+<!-- include end -->
diff --git a/interface-definitions/include/version/openconnect-version.xml.i b/interface-definitions/include/version/openconnect-version.xml.i
@@ -1,3 +1,3 @@
 <!-- include start from include/version/openconnect-version.xml.i -->
-<syntaxVersion component='openconnect' version='2'></syntaxVersion>
+<syntaxVersion component='openconnect' version='3'></syntaxVersion>
 <!-- include end -->
diff --git a/interface-definitions/interfaces_openvpn.xml.in b/interface-definitions/interfaces_openvpn.xml.in
@@ -755,33 +755,7 @@
                   <constraintErrorMessage>Peer certificate fingerprint must be a colon-separated SHA256 hex digest</constraintErrorMessage>
                 </properties>
               </leafNode>
-              <leafNode name="tls-version-min">
-                <properties>
-                  <help>Specify the minimum required TLS version</help>
-                  <completionHelp>
-                    <list>1.0 1.1 1.2 1.3</list>
-                  </completionHelp>
-                  <valueHelp>
-                    <format>1.0</format>
-                    <description>TLS v1.0</description>
-                  </valueHelp>
-                  <valueHelp>
-                    <format>1.1</format>
-                    <description>TLS v1.1</description>
-                  </valueHelp>
-                  <valueHelp>
-                    <format>1.2</format>
-                    <description>TLS v1.2</description>
-                  </valueHelp>
-                  <valueHelp>
-                    <format>1.3</format>
-                    <description>TLS v1.3</description>
-                  </valueHelp>
-                  <constraint>
-                    <regex>(1.0|1.1|1.2|1.3)</regex>
-                  </constraint>
-                </properties>
-              </leafNode>
+              #include <include/tls-version-min.xml.i>
               <leafNode name="role">
                 <properties>
                   <help>TLS negotiation role</help>

diff --git a/interface-definitions/vpn_openconnect.xml.in b/interface-definitions/vpn_openconnect.xml.in
@@ -266,6 +266,10 @@
               <valueless/>
             </properties>
           </leafNode>
+          #include <include/tls-version-min.xml.i>
+          <leafNode name="tls-version-min">
+            <defaultValue>1.2</defaultValue>
+          </leafNode>
           <node name="ssl">
             <properties>
               <help>SSL Certificate, SSL Key and CA</help>

diff --git a/smoketest/scripts/cli/test_vpn_openconnect.py b/smoketest/scripts/cli/test_vpn_openconnect.py
@@ -210,6 +210,9 @@ def test_ocserv(self):
         # Verify configuration
         daemon_config = read_file(config_file)
 
+        # Verify TLS string (with default setting)
+        self.assertIn('tls-priorities = "NORMAL:%SERVER_PRECEDENCE:%COMPAT:-RSA:-VERS-SSL3.0:-ARCFOUR-128:-VERS-TLS1.0:-VERS-TLS1.1"', daemon_config)
+
         # authentication mode local password-otp
         self.assertIn(f'auth = "plain[passwd=/run/ocserv/ocpasswd,otp=/run/ocserv/users.oath]"', daemon_config)
         self.assertIn(f'listen-host = {listen_ip_no_cidr}', daemon_config)
@@ -253,5 +256,13 @@ def test_ocserv(self):
         self.assertIn('included-http-headers = Pragma: no-cache', daemon_config)
         self.assertIn('included-http-headers = Cache-control: no-store, no-cache', daemon_config)
 
+        # Set TLS version to the highest security (v1.3 min)
+        self.cli_set(base_path + ['tls-version-min', '1.3'])
+        self.cli_commit()
+
+        # Verify TLS string
+        daemon_config = read_file(config_file)
+        self.assertIn('tls-priorities = "NORMAL:%SERVER_PRECEDENCE:%COMPAT:-RSA:-VERS-SSL3.0:-ARCFOUR-128:-VERS-TLS1.0:-VERS-TLS1.1:-VERS-TLS1.2"', daemon_config)
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)
diff --git a/src/migration-scripts/openconnect/2-to-3 b/src/migration-scripts/openconnect/2-to-3
@@ -0,0 +1,50 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2024 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# T4982: Retain prior default TLS version (v1.0) when upgrading installations with existing openconnect configurations
+
+import sys
+
+from vyos.configtree import ConfigTree
+
+if len(sys.argv) < 2:
+    print("Must specify file name!")
+    sys.exit(1)
+
+file_name = sys.argv[1]
+
+with open(file_name, 'r') as f:
+    config_file = f.read()
+
+
+config = ConfigTree(config_file)
+cfg_base = ['vpn', 'openconnect']
+
+# bail out early if service is unconfigured
+if not config.exists(cfg_base):
+    sys.exit(0)
+
+# new default is TLS 1.2 - set explicit old default value of TLS 1.0 for upgraded configurations to keep compatibility
+tls_min_path = cfg_base + ['tls-version-min']
+if not config.exists(tls_min_path):
+    config.set(tls_min_path, value='1.0')
+
+try:
+    with open(file_name, 'w') as f:
+        f.write(config.to_string())
+except OSError as e:
+    print("Failed to save the modified config: {}".format(e))
+    sys.exit(1)