Repository: openwisp/netjsonconfig

Issue:
[feature] Add data-ciphers option
E.g.:

```
"data_ciphers": [
                "AES-128-CBC",
                "AES-256-GCM",
                "AES-128-GCM"
            ],
```

I would use an array of strings, each string can be chosen from the cipher list.

Discussion:

Solution patch:
diff --git a/netjsonconfig/backends/openvpn/converters.py b/netjsonconfig/backends/openvpn/converters.py
index 5853e82..d8e5ed4 100644
--- a/netjsonconfig/backends/openvpn/converters.py
+++ b/netjsonconfig/backends/openvpn/converters.py
@@ -53,9 +53,23 @@ class OpenVpn(BaseConverter):
         # do not display status-version if status directive not present
         if 'status' not in config and 'status_version' in config:
             del config['status_version']
+        config = self.__output_data_ciphers(config)
         config = self.__add_tls_auth_key(config)
         return self.sorted_dict(config)
 
+    def __output_data_ciphers(self, config):
+        data_ciphers = config.get('data_ciphers', None)
+        if not data_ciphers:
+            return config
+        output = ''
+        for cipher in data_ciphers:
+            cipher_text = cipher['cipher']
+            if cipher['optional']:
+                cipher_text = f'?{cipher_text}'
+            output = f'{output}:{cipher_text}'
+        config['data_ciphers'] = output[1:]
+        return config
+
     def __add_tls_auth_key(self, config):
         tls_auth = config.get('tls_auth', None)
         if not tls_auth:
@@ -109,4 +123,18 @@ class OpenVpn(BaseConverter):
                 else:
                     remote.append(dict(host=items[0], port=int(items[1])))
             vpn['remote'] = remote
+        vpn = self.__netjson_data_ciphers(vpn)
+        return vpn
+
+    def __netjson_data_ciphers(self, vpn):
+        data_ciphers_text = vpn.get('data_ciphers')
+        if not data_ciphers_text:
+            return vpn
+        data_ciphers = []
+        ciphers = data_ciphers_text.split(':')
+        for cipher in ciphers:
+            optional = cipher.startswith('?')
+            cipher_text = cipher if not optional else cipher[1:]
+            data_ciphers.append({'cipher': cipher_text, 'optional': optional})
+        vpn['data_ciphers'] = data_ciphers
         return vpn
diff --git a/netjsonconfig/backends/openvpn/schema.py b/netjsonconfig/backends/openvpn/schema.py
index 6d50a31..afa3f0e 100644
--- a/netjsonconfig/backends/openvpn/schema.py
+++ b/netjsonconfig/backends/openvpn/schema.py
@@ -5,6 +5,91 @@ from copy import deepcopy
 
 from ...schema import schema as default_schema
 
+data_ciphers = [
+    "AES-128-CBC",
+    "AES-128-CFB",
+    "AES-128-CFB1",
+    "AES-128-CFB8",
+    "AES-128-GCM",
+    "AES-128-OFB",
+    "AES-192-CBC",
+    "AES-192-CFB",
+    "AES-192-CFB1",
+    "AES-192-CFB8",
+    "AES-192-GCM",
+    "AES-192-OFB",
+    "AES-256-CBC",
+    "AES-256-CFB",
+    "AES-256-CFB1",
+    "AES-256-CFB8",
+    "AES-256-GCM",
+    "AES-256-OFB",
+    "ARIA-128-CBC",
+    "ARIA-128-CFB",
+    "ARIA-128-CFB1",
+    "ARIA-128-CFB8",
+    "ARIA-128-OFB",
+    "ARIA-192-CBC",
+    "ARIA-192-CFB",
+    "ARIA-192-CFB1",
+    "ARIA-192-CFB8",
+    "ARIA-192-OFB",
+    "ARIA-256-CBC",
+    "ARIA-256-CFB",
+    "ARIA-256-CFB1",
+    "ARIA-256-CFB8",
+    "ARIA-256-OFB",
+    "CAMELLIA-128-CBC",
+    "CAMELLIA-128-CFB",
+    "CAMELLIA-128-CFB1",
+    "CAMELLIA-128-CFB8",
+    "CAMELLIA-128-OFB",
+    "CAMELLIA-192-CBC",
+    "CAMELLIA-192-CFB",
+    "CAMELLIA-192-CFB1",
+    "CAMELLIA-192-CFB8",
+    "CAMELLIA-192-OFB",
+    "CAMELLIA-256-CBC",
+    "CAMELLIA-256-CFB",
+    "CAMELLIA-256-CFB1",
+    "CAMELLIA-256-CFB8",
+    "CAMELLIA-256-OFB",
+    "CHACHA20-POLY1305",
+    "SEED-CBC",
+    "SEED-CFB",
+    "SEED-OFB",
+    "SM4-CBC",
+    "SM4-CFB",
+    "SM4-OFB",
+    "BF-CBC",
+    "BF-CFB",
+    "BF-OFB",
+    "CAST5-CBC",
+    "CAST5-CFB",
+    "CAST5-OFB",
+    "DES-CBC",
+    "DES-CFB",
+    "DES-CFB1",
+    "DES-CFB8",
+    "DES-EDE-CBC",
+    "DES-EDE-CFB",
+    "DES-EDE-OFB",
+    "DES-EDE3-CBC",
+    "DES-EDE3-CFB",
+    "DES-EDE3-CFB1",
+    "DES-EDE3-CFB8",
+    "DES-EDE3-OFB",
+    "DES-OFB",
+    "DESX-CBC",
+    "RC2-40-CBC",
+    "RC2-64-CBC",
+    "RC2-CBC",
+    "RC2-CFB",
+    "RC2-OFB",
+    "none",
+]
+default_cipher = "AES-256-GCM"
+
 base_openvpn_schema = {
     "$schema": "http://json-schema.org/draft-04/schema#",
     "type": "object",
@@ -102,46 +187,61 @@ base_openvpn_schema = {
                     "default": "SHA1",
                     "propertyOrder": 11,
                 },
+                "data_ciphers": {
+                    "title": "data ciphers",
+                    "description": (
+                        "Restrict the allowed ciphers to be negotiated "
+                        "to the ciphers in this list."
+                    ),
+                    "type": "array",
+                    "additionalItems": True,
+                    "propertyOrder": 12.0,
+                    "minItems": 1,
+                    "default": [
+                        {"cipher": "AES-256-GCM", "optional": False},
+                        {"cipher": "AES-128-GCM", "optional": False},
+                    ],
+                    "items": {
+                        "type": "object",
+                        "required": ["cipher", "optional"],
+                        "properties": {
+                            "cipher": {
+                                "type": "string",
+                                "enum": [""] + data_ciphers,
+                                "default": "",
+                                "propertyOrder": 1,
+                            },
+                            "optional": {
+                                "type": "boolean",
+                                "default": False,
+                                "format": "checkbox",
+                                "propertyOrder": 2,
+                            },
+                        },
+                    },
+                },
+                "data_ciphers_fallback": {
+                    "title": "data ciphers fallback",
+                    "type": "string",
+                    "description": (
+                        "Configure a cipher that is used to fall back to if we "
+                        "could not determine which cipher the peer is willing to use."
+                    ),
+                    "enum": data_ciphers,
+                    "default": default_cipher,
+                    "propertyOrder": 12.1,
+                },
                 "cipher": {
                     "title": "cipher",
                     "type": "string",
-                    "description": "Encrypt data channel packets with cipher algorithm",
-                    "enum": [
-                        "AES-128-CBC",
-                        "AES-128-CFB",
-                        "AES-128-CFB1",
-                        "AES-128-CFB8",
-                        "AES-128-GCM",
-                        "AES-128-OFB",
-                        "AES-192-CBC",
-                        "AES-192-CFB",
-                        "AES-192-CFB1",
-                        "AES-192-CFB8",
-                        "AES-192-GCM",
-                        "AES-192-OFB",
-                        "AES-256-CBC",
-                        "AES-256-CFB",
-                        "AES-256-CFB1",
-                        "AES-256-CFB8",
-                        "AES-256-GCM",
-                        "AES-256-OFB",
-                        "BF-CBC",
-                        "BF-CFB",
-                        "BF-OFB",
-                        "CAMELLIA-128-CBC",
-                        "CAMELLIA-128-CFB1",
-                        "CAMELLIA-128-CFB8",
-                        "CAMELLIA-128-OFB",
-                        "CAMELLIA-192-CBC",
-                        "CAMELLIA-192-CFB",
-                        "CAMELLIA-192-CFB1",
-                        "CAMELLIA-192-CFB8",
-                        "CAMELLIA-192-OFB",
-                        "CAMELLIA-256-CBC",
-                        "none",
-                    ],
-                    "default": "BF-CBC",
-                    "propertyOrder": 12,
+                    "description": (
+                        "Encrypt data channel packets with cipher algorithm. "
+                        "This option is deprecated in favour of data-ciphers "
+                        "and data-ciphers-fallback."
+                    ),
+                    "enum": data_ciphers,
+                    "default": default_cipher,
+                    "propertyOrder": 12.2,
                 },
                 "engine": {
                     "title": "engine",


Test patch:
diff --git a/tests/openvpn/test_backend.py b/tests/openvpn/test_backend.py
index dc15981..8661400 100644
--- a/tests/openvpn/test_backend.py
+++ b/tests/openvpn/test_backend.py
@@ -103,6 +103,107 @@ tls-auth tls_auth.key 0
 tls-server
 user nobody
 verb 3
+"""
+        self.assertEqual(c.render(), expected)
+
+    def test_server_mode_with_data_ciphers(self):
+        c = OpenVpn(
+            {
+                "openvpn": [
+                    {
+                        "auth": "SHA1",
+                        "auth_user_pass_verify": "",
+                        "auth_nocache": True,
+                        "ca": "ca.pem",
+                        "cert": "cert.pem",
+                        "data_ciphers": [
+                            {"cipher": "AES-256-GCM", "optional": False},
+                            {"cipher": "AES-128-GCM", "optional": False},
+                            {"cipher": "CHACHA20-POLY1305", "optional": True},
+                        ],
+                        "data_ciphers_fallback": "AES-128-GCM",
+                        "cipher": "AES-128-GCM",
+                        "client_cert_not_required": False,
+                        "client_to_client": False,
+                        "comp_lzo": "adaptive",
+                        "crl_verify": "crl.pem",
+                        "dev": "tap0",
+                        "dev_type": "tap",
+                        "dh": "dh.pem",
+                        "down": "",
+                        "duplicate_cn": True,
+                        "engine": "rsax",
+                        "fast_io": True,
+                        "fragment": 0,
+                        "group": "nogroup",
+                        "keepalive": "20 60",
+                        "key": "key.pem",
+                        "local": "",
+                        "log": "/var/log/openvpn.log",
+                        "mode": "server",
+                        "name": "test-server",
+                        "mssfix": 1450,
+                        "mtu_disc": "no",
+                        "mtu_test": False,
+                        "mute": 0,
+                        "mute_replay_warnings": True,
+                        "ns_cert_type": "",
+                        "persist_key": True,
+                        "persist_tun": True,
+                        "port": 1194,
+                        "proto": "udp",
+                        "script_security": 0,
+                        "secret": "",
+                        "status": "/var/log/openvpn.status 10",
+                        "status_version": 1,
+                        "tls_server": True,
+                        "tls_auth": "tls_auth.key 0",
+                        "tun_ipv6": False,
+                        "up": "",
+                        "up_delay": 0,
+                        "user": "nobody",
+                        "username_as_common_name": False,
+                        "verb": 3,
+                    }
+                ]
+            }
+        )
+        expected = """# openvpn config: test-server
+
+auth SHA1
+auth-nocache
+ca ca.pem
+cert cert.pem
+cipher AES-128-GCM
+comp-lzo adaptive
+crl-verify crl.pem
+data-ciphers AES-256-GCM:AES-128-GCM:?CHACHA20-POLY1305
+data-ciphers-fallback AES-128-GCM
+dev tap0
+dev-type tap
+dh dh.pem
+duplicate-cn
+engine rsax
+fast-io
+group nogroup
+keepalive 20 60
+key key.pem
+log /var/log/openvpn.log
+mode server
+mssfix 1450
+mtu-disc no
+mute-replay-warnings
+persist-key
+persist-tun
+port 1194
+proto udp
+script-security 0
+status /var/log/openvpn.status 10
+status-version 1
+tls-auth tls_auth.key 0
+tls-server
+user nobody
+verb 3
 """
         self.assertEqual(c.render(), expected)
 
@@ -208,6 +309,119 @@ up /home/user/up-command.sh
 up-delay 10
 user nobody
 verb 1
+"""
+        self.assertEqual(c.render(), expected)
+
+    def test_client_mode_data_cihpers(self):
+        c = OpenVpn(
+            {
+                "openvpn": [
+                    {
+                        "auth": "SHA256",
+                        "auth_user_pass": "",
+                        "auth_nocache": True,
+                        "ca": "ca.pem",
+                        "cert": "cert.pem",
+                        "cipher": "AES-128-GCM",
+                        "data_ciphers": [
+                            {"cipher": "AES-256-GCM", "optional": False},
+                            {"cipher": "AES-128-GCM", "optional": False},
+                            {"cipher": "CHACHA20-POLY1305", "optional": True},
+                        ],
+                        "data_ciphers_fallback": "AES-128-GCM",
+                        "comp_lzo": "adaptive",
+                        "dev": "tun0",
+                        "dev_type": "tun",
+                        "down": "/home/user/down-command.sh",
+                        "engine": "",
+                        "fast_io": False,
+                        "fragment": 0,
+                        "group": "",
+                        "keepalive": "",
+                        "key": "key.pem",
+                        "local": "",
+                        "log": "/var/log/openvpn.log",
+                        "mode": "p2p",
+                        "mssfix": 1450,
+                        "mtu_disc": "yes",
+                        "mtu_test": True,
+                        "mute": 10,
+                        "mute_replay_warnings": True,
+                        "name": "test-client",
+                        "nobind": True,
+                        "ns_cert_type": "server",
+                        "persist_key": True,
+                        "persist_tun": True,
+                        "port": 1195,
+                        "proto": "tcp-client",
+                        "pull": True,
+                        "remote": [
+                            {"host": "vpn1.test.com", "port": 1194},
+                            {"host": "176.9.43.231", "port": 1195, "proto": "udp4"},
+                            {"host": "176.9.43.232", "port": 1195, "proto": "auto"},
+                            {"host": "176.9.43.233", "port": 1196, "proto": "udp6"},
+                        ],
+                        "resolv_retry": "infinite",
+                        "script_security": 1,
+                        "secret": "",
+                        "status": "/var/log/openvpn.status 30",
+                        "status_version": 1,
+                        "tls_client": True,
+                        "tls_auth": "tls_auth.key 1",
+                        "topology": "p2p",
+                        "tun_ipv6": True,
+                        "up": "/home/user/up-command.sh",
+                        "up_delay": 10,
+                        "user": "nobody",
+                        "verb": 1,
+                    }
+                ]
+            }
+        )
+        expected = """# openvpn config: test-client
+
+auth SHA256
+auth-nocache
+ca ca.pem
+cert cert.pem
+cipher AES-128-GCM
+comp-lzo adaptive
+data-ciphers AES-256-GCM:AES-128-GCM:?CHACHA20-POLY1305
+data-ciphers-fallback AES-128-GCM
+dev tun0
+dev-type tun
+down /home/user/down-command.sh
+key key.pem
+log /var/log/openvpn.log
+mode p2p
+mssfix 1450
+mtu-disc yes
+mtu-test
+mute 10
+mute-replay-warnings
+nobind
+ns-cert-type server
+persist-key
+persist-tun
+port 1195
+proto tcp-client
+pull
+remote vpn1.test.com 1194
+remote 176.9.43.231 1195 udp4
+remote 176.9.43.232 1195
+remote 176.9.43.233 1196 udp6
+resolv-retry infinite
+script-security 1
+status /var/log/openvpn.status 30
+status-version 1
+tls-auth tls_auth.key 1
+tls-client
+topology p2p
+tun-ipv6
+up /home/user/up-command.sh
+up-delay 10
+user nobody
+verb 1
 """
         self.assertEqual(c.render(), expected)
 
diff --git a/tests/openvpn/test_parser.py b/tests/openvpn/test_parser.py
index b153d8a..6eac2bb 100644
--- a/tests/openvpn/test_parser.py
+++ b/tests/openvpn/test_parser.py
@@ -43,6 +43,166 @@ tls-server
         }
         self.assertDictEqual(o.config, expected)
 
+    def test_parse_server(self):
+        native = """# openvpn config: test-server
+
+auth SHA1
+auth-nocache
+ca ca.pem
+cert cert.pem
+cipher AES-128-GCM
+comp-lzo adaptive
+crl-verify crl.pem
+dev tap0
+dev-type tap
+dh dh.pem
+duplicate-cn
+engine rsax
+fast-io
+group nogroup
+keepalive 20 60
+key key.pem
+log /var/log/openvpn.log
+mode server
+mssfix 1450
+mtu-disc no
+mute-replay-warnings
+persist-key
+persist-tun
+port 1194
+proto udp
+script-security 0
+status /var/log/openvpn.status 10
+status-version 1
+tls-server
+user nobody
+verb 3
+"""
+        expected = {
+            "openvpn": [
+                {
+                    "auth": "SHA1",
+                    "auth_nocache": True,
+                    "ca": "ca.pem",
+                    "cert": "cert.pem",
+                    "cipher": "AES-128-GCM",
+                    "comp_lzo": "adaptive",
+                    "crl_verify": "crl.pem",
+                    "dev": "tap0",
+                    "dev_type": "tap",
+                    "dh": "dh.pem",
+                    "duplicate_cn": True,
+                    "engine": "rsax",
+                    "fast_io": True,
+                    "group": "nogroup",
+                    "keepalive": "20 60",
+                    "key": "key.pem",
+                    "log": "/var/log/openvpn.log",
+                    "mode": "server",
+                    "name": "test-server",
+                    "mssfix": 1450,
+                    "mtu_disc": "no",
+                    "mute_replay_warnings": True,
+                    "persist_key": True,
+                    "persist_tun": True,
+                    "port": 1194,
+                    "proto": "udp",
+                    "script_security": 0,
+                    "status": "/var/log/openvpn.status 10",
+                    "status_version": 1,
+                    "tls_server": True,
+                    "user": "nobody",
+                    "verb": 3,
+                }
+            ]
+        }
+        o = OpenVpn(native=native)
+        self.assertDictEqual(o.config, expected)
+
+    def test_parse_data_ciphers(self):
+        native = """# openvpn config: test-server
+
+auth SHA1
+auth-nocache
+ca ca.pem
+cert cert.pem
+cipher AES-128-GCM
+comp-lzo adaptive
+crl-verify crl.pem
+data-ciphers AES-256-GCM:AES-128-GCM:?CHACHA20-POLY1305
+data-ciphers-fallback AES-128-GCM
+dev tap0
+dev-type tap
+dh dh.pem
+duplicate-cn
+engine rsax
+fast-io
+group nogroup
+keepalive 20 60
+key key.pem
+log /var/log/openvpn.log
+mode server
+mssfix 1450
+mtu-disc no
+mute-replay-warnings
+persist-key
+persist-tun
+port 1194
+proto udp
+script-security 0
+status /var/log/openvpn.status 10
+status-version 1
+tls-server
+user nobody
+verb 3
+"""
+        expected = {
+            "openvpn": [
+                {
+                    "auth": "SHA1",
+                    "auth_nocache": True,
+                    "ca": "ca.pem",
+                    "cert": "cert.pem",
+                    "cipher": "AES-128-GCM",
+                    "comp_lzo": "adaptive",
+                    "crl_verify": "crl.pem",
+                    "data_ciphers": [
+                        {"cipher": "AES-256-GCM", "optional": False},
+                        {"cipher": "AES-128-GCM", "optional": False},
+                        {"cipher": "CHACHA20-POLY1305", "optional": True},
+                    ],
+                    "data_ciphers_fallback": "AES-128-GCM",
+                    "dev": "tap0",
+                    "dev_type": "tap",
+                    "dh": "dh.pem",
+                    "duplicate_cn": True,
+                    "engine": "rsax",
+                    "fast_io": True,
+                    "group": "nogroup",
+                    "keepalive": "20 60",
+                    "key": "key.pem",
+                    "log": "/var/log/openvpn.log",
+                    "mode": "server",
+                    "name": "test-server",
+                    "mssfix": 1450,
+                    "mtu_disc": "no",
+                    "mute_replay_warnings": True,
+                    "persist_key": True,
+                    "persist_tun": True,
+                    "port": 1194,
+                    "proto": "udp",
+                    "script_security": 0,
+                    "status": "/var/log/openvpn.status 10",
+                    "status_version": 1,
+                    "tls_server": True,
+                    "user": "nobody",
+                    "verb": 3,
+                }
+            ]
+        }
+        o = OpenVpn(native=native)
+        self.assertDictEqual(o.config, expected)
+
     def test_parse_exception(self):
         try:
             OpenVpn(native=10)
diff --git a/tests/openwrt/test_openvpn.py b/tests/openwrt/test_openvpn.py
index 4bfe812..1e40753 100644
--- a/tests/openwrt/test_openvpn.py
+++ b/tests/openwrt/test_openvpn.py
@@ -54,6 +54,17 @@ class TestOpenVpn(_TabsMixin, unittest.TestCase):
             }
         ]
     }
+    _server_netjson_data_ciphers = deepcopy(_server_netjson)
+    _server_netjson_data_ciphers['openvpn'][0].update(
+        {
+            'data_ciphers': [
+                {'cipher': 'AES-256-GCM', 'optional': False},
+                {'cipher': 'AES-128-GCM', 'optional': False},
+                {'cipher': 'CHACHA20-POLY1305', 'optional': True},
+            ],
+            'data_ciphers_fallback': 'AES-128-GCM',
+        }
+    )
     _server_uci = """package openvpn
 
 config openvpn 'test_server'
@@ -96,18 +107,73 @@ config openvpn 'test_server'
     option username_as_common_name '0'
     option verb '3'
 """
+    _server_uci_data_ciphers = """package openvpn
+
+config openvpn 'test_server'
+    option auth 'SHA1'
+    option ca 'ca.pem'
+    option cert 'cert.pem'
+    option cipher 'BF-CBC'
+    option client_cert_not_required '0'
+    option client_to_client '0'
+    option comp_lzo 'yes'
+    option crl_verify 'crl.pem'
+    option data_ciphers 'AES-256-GCM:AES-128-GCM:?CHACHA20-POLY1305'
+    option data_ciphers_fallback 'AES-128-GCM'
+    option dev 'tap0'
+    option dev_type 'tap'
+    option dh 'dh.pem'
+    option duplicate_cn '1'
+    option enabled '1'
+    option engine 'rsax'
+    option fast_io '1'
+    option group 'nogroup'
+    option keepalive '20 60'
+    option key 'key.pem'
+    option log '/var/log/openvpn.log'
+    option mode 'server'
+    option mssfix '1450'
+    option mtu_disc 'no'
+    option mtu_test '0'
+    option mute '0'
+    option mute_replay_warnings '1'
+    option persist_key '1'
+    option persist_tun '1'
+    option port '1194'
+    option proto 'udp'
+    option script_security '0'
+    option status '/var/log/openvpn.status 10'
+    option status_version '1'
+    option tls_server '1'
+    option tun_ipv6 '0'
+    option up_delay '0'
+    option user 'nobody'
+    option username_as_common_name '0'
+    option verb '3'
+"""
 
     def test_render_server_mode(self):
         c = OpenWrt(self._server_netjson)
         expected = self._tabs(self._server_uci)
         self.assertEqual(c.render(), expected)
 
+    def test_render_server_mode_data_ciphers(self):
+        c = OpenWrt(self._server_netjson_data_ciphers)
+        expected = self._tabs(self._server_uci_data_ciphers)
+        self.assertEqual(c.render(), expected)
+
     def test_parse_server_mode(self):
         c = OpenWrt(native=self._server_uci)
         expected = deepcopy(self._server_netjson)
         del expected['openvpn'][0]['fragment']
         self.assertEqual(c.config, expected)
 
+    def test_parse_server_mode_data_ciphers(self):
+        c = OpenWrt(native=self._server_uci_data_ciphers)
+        expected = deepcopy(self._server_netjson_data_ciphers)
+        del expected['openvpn'][0]['fragment']
+        self.assertEqual(c.config, expected)
+
     _client_netjson = {
         "openvpn": [
             {