Merge pull request #2353 from dmbaturin/T5634-no-more-blowfish

openvpn: T5634: Remove support for insecure DES and Blowfish ciphers

5 files changed, 58 insertions, 28 deletions

diff --git a/data/templates/openvpn/server.conf.j2 b/data/templates/openvpn/server.conf.j2
index 2eb9416fe..746155c37 100644
--- a/data/templates/openvpn/server.conf.j2
+++ b/data/templates/openvpn/server.conf.j2
@@ -205,19 +205,12 @@ tls-server
 {% if encryption is vyos_defined %}
 {%     if encryption.cipher is vyos_defined %}
 cipher {{ encryption.cipher | openvpn_cipher }}
-{%         if encryption.cipher is vyos_defined('bf128') %}
-keysize 128
-{%         elif encryption.cipher is vyos_defined('bf256') %}
-keysize 256
-{%         endif %}
 {%     endif %}
 {%     if encryption.ncp_ciphers is vyos_defined %}
 data-ciphers {{ encryption.ncp_ciphers | openvpn_ncp_ciphers }}
 {%     endif %}
 {% endif %}
-# https://vyos.dev/T5027
-# Required to support BF-CBC (default ciphername when none given)
-providers legacy default
+providers default
 
 {% if hash is vyos_defined %}
 auth {{ hash }}
diff --git a/interface-definitions/include/version/openvpn-version.xml.i b/interface-definitions/include/version/openvpn-version.xml.i
new file mode 100644
index 000000000..f2f60dcd6
--- /dev/null
+++ b/interface-definitions/include/version/openvpn-version.xml.i
@@ -0,0 +1,3 @@
+!-- include start from include/version/openvpn-version.xml.i -->
+<syntaxVersion component='openvpn' version='1'></syntaxVersion>
+<!-- include end -->
diff --git a/interface-definitions/interfaces-openvpn.xml.in b/interface-definitions/interfaces-openvpn.xml.in
index 831659250..b8b04334c 100644
--- a/interface-definitions/interfaces-openvpn.xml.in
+++ b/interface-definitions/interfaces-openvpn.xml.in
@@ -48,29 +48,17 @@
                 <properties>
                   <help>Standard Data Encryption Algorithm</help>
                   <completionHelp>
-                    <list>none des 3des bf128 bf256 aes128 aes128gcm aes192 aes192gcm aes256 aes256gcm</list>
+                    <list>none 3des aes128 aes128gcm aes192 aes192gcm aes256 aes256gcm</list>
                   </completionHelp>
                   <valueHelp>
                     <format>none</format>
                     <description>Disable encryption</description>
                   </valueHelp>
                   <valueHelp>
-                    <format>des</format>
-                    <description>DES algorithm</description>
-                  </valueHelp>
-                  <valueHelp>
                     <format>3des</format>
                     <description>DES algorithm with triple encryption</description>
                   </valueHelp>
                   <valueHelp>
-                    <format>bf128</format>
-                    <description>Blowfish algorithm with 128-bit key</description>
-                  </valueHelp>
-                  <valueHelp>
-                    <format>bf256</format>
-                    <description>Blowfish algorithm with 256-bit key</description>
-                  </valueHelp>
-                  <valueHelp>
                     <format>aes128</format>
                     <description>AES algorithm with 128-bit key CBC</description>
                   </valueHelp>
@@ -95,7 +83,7 @@
                     <description>AES algorithm with 256-bit key GCM</description>
                   </valueHelp>
                   <constraint>
-                    <regex>(none|des|3des|bf128|bf256|aes128|aes128gcm|aes192|aes192gcm|aes256|aes256gcm)</regex>
+                    <regex>(none|3des|aes128|aes128gcm|aes192|aes192gcm|aes256|aes256gcm)</regex>
                   </constraint>
                 </properties>
               </leafNode>
@@ -103,17 +91,13 @@
                 <properties>
                   <help>Cipher negotiation list for use in server or client mode</help>
                   <completionHelp>
-                    <list>none des 3des aes128 aes128gcm aes192 aes192gcm aes256 aes256gcm</list>
+                    <list>none 3des aes128 aes128gcm aes192 aes192gcm aes256 aes256gcm</list>
                   </completionHelp>
                   <valueHelp>
                     <format>none</format>
                     <description>Disable encryption</description>
                   </valueHelp>
                   <valueHelp>
-                    <format>des</format>
-                    <description>DES algorithm</description>
-                  </valueHelp>
-                  <valueHelp>
                     <format>3des</format>
                     <description>DES algorithm with triple encryption</description>
                   </valueHelp>
@@ -142,7 +126,7 @@
                     <description>AES algorithm with 256-bit key GCM</description>
                   </valueHelp>
                   <constraint>
-                    <regex>(none|des|3des|aes128|aes128gcm|aes192|aes192gcm|aes256|aes256gcm)</regex>
+                    <regex>(none|3des|aes128|aes128gcm|aes192|aes192gcm|aes256|aes256gcm)</regex>
                   </constraint>
                   <multi/>
                 </properties>
diff --git a/interface-definitions/xml-component-version.xml.in b/interface-definitions/xml-component-version.xml.in
index 8c9e816d1..cae3423dc 100644
--- a/interface-definitions/xml-component-version.xml.in
+++ b/interface-definitions/xml-component-version.xml.in
@@ -19,6 +19,7 @@
   #include <include/version/ids-version.xml.i>
   #include <include/version/ipoe-server-version.xml.i>
   #include <include/version/ipsec-version.xml.i>
+  #include <include/version/openvpn-version.xml.i>
   #include <include/version/isis-version.xml.i>
   #include <include/version/l2tp-version.xml.i>
   #include <include/version/lldp-version.xml.i>
diff --git a/src/migration-scripts/openvpn/0-to-1 b/src/migration-scripts/openvpn/0-to-1
new file mode 100644
index 000000000..24bb38d3c
--- /dev/null
+++ b/src/migration-scripts/openvpn/0-to-1
@@ -0,0 +1,49 @@
+#!/usr/bin/env python3
+
+# Removes outdated ciphers (DES and Blowfish) from OpenVPN configs
+
+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)
+
+if not config.exists(['interfaces', 'openvpn']):
+    # Nothing to do
+    sys.exit(0)
+else:
+    ovpn_intfs = config.list_nodes(['interfaces', 'openvpn'])
+    for	i in ovpn_intfs:
+        # Remove DES and Blowfish from 'encryption cipher'
+        cipher_path = ['interfaces', 'openvpn', i, 'encryption', 'cipher']
+        if config.exists(cipher_path):
+            cipher = config.return_value(cipher_path)
+            if cipher in ['des', 'bf128', 'bf256']:
+                config.delete(cipher_path)
+
+        ncp_cipher_path = ['interfaces', 'openvpn', i, 'encryption', 'ncp-ciphers']
+        if config.exists(ncp_cipher_path):
+            ncp_ciphers = config.return_values(['interfaces', 'openvpn', i, 'encryption', 'ncp-ciphers'])
+            if 'des' in ncp_ciphers:
+                config.delete_value(['interfaces', 'openvpn', i, 'encryption', 'ncp-ciphers'], 'des')
+
+        # Clean up the encryption subtree if the migration procedure left it empty
+        if config.exists(['interfaces', 'openvpn', i, 'encryption']) and \
+           (config.list_nodes(['interfaces', 'openvpn', i, 'encryption']) == []):
+            config.delete(['interfaces', 'openvpn', i, 'encryption'])
+
+    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)