openvpn: T2994: re-add ifconfig-pool statement in server config

Re-organize the template code and add addtitional Jinja2 filters for processing
the ifconfig-pool statement. This reverts the changes from commit 7e546be9
("openvpn: T2994: temporary revert to 1.2 crux behavior for client pools").

5 files changed, 182 insertions, 30 deletions

diff --git a/data/templates/openvpn/server.conf.tmpl b/data/templates/openvpn/server.conf.tmpl
index 91542c71a..66da9c794 100644
--- a/data/templates/openvpn/server.conf.tmpl
+++ b/data/templates/openvpn/server.conf.tmpl
@@ -61,7 +61,16 @@ tls-server
 {%   if server is defined and server is not none %}
 {%     if server.subnet is defined and server.subnet is not none %}
 {%       for subnet in server.subnet if subnet | ipv4 %}
-server {{ subnet | address_from_cidr }} {{ subnet | netmask_from_cidr }}
+server {{ subnet | address_from_cidr }} {{ subnet | netmask_from_cidr }} nopool
+{# OpenVPN assigns the first IP address to its local interface so the pool used #}
+{# in net30 topology - where each client receives a /30 must start from the second subnet #}
+{%     if server.topology is defined and server.topology == 'net30' %}
+ifconfig-pool {{ subnet | inc_ip('4') }} {{ subnet | last_host_address }} {{ subnet | netmask_from_cidr if device_type == 'tap' else '' }}
+{%     else %}
+{# OpenVPN assigns the first IP address to its local interface so the pool must #}
+{# start from the second address and end on the last address #}
+ifconfig-pool {{ subnet | first_host_address | inc_ip('1') }} {{ subnet | last_host_address }} {{ subnet | netmask_from_cidr if device_type == 'tap' else '' }}
+{%     endif %}
 {%       endfor %}
 {%     endif %}
 {%     if server.topology is defined and server.topology == 'point-to-point' %}
diff --git a/python/vyos/template.py b/python/vyos/template.py
index 621be0695..cc582bf79 100644
--- a/python/vyos/template.py
+++ b/python/vyos/template.py
@@ -123,8 +123,7 @@ def render(
 # Custom template filters follow #
 ##################################
 
-
-@register_filter("address_from_cidr")
+@register_filter('address_from_cidr')
 def vyos_address_from_cidr(text):
     """ Take an IPv4/IPv6 CIDR prefix and convert the network to an "address".
     Example:
@@ -133,24 +132,69 @@ def vyos_address_from_cidr(text):
     from ipaddress import ip_network
     return str(ip_network(text).network_address)
 
-
-@register_filter("netmask_from_cidr")
+@register_filter('netmask_from_cidr')
 def vyos_netmask_from_cidr(text):
-    """ Take an IPv4/IPv6 CIDR prefix and convert the prefix length to a "subnet mask".
+    """ Take CIDR prefix and convert the prefix length to a "subnet mask".
     Example:
-    192.0.2.0/24 -> 255.255.255.0, 2001:db8::/48 -> ffff:ffff:ffff::
+      - 192.0.2.0/24 -> 255.255.255.0
+      - 2001:db8::/48 -> ffff:ffff:ffff::
     """
     from ipaddress import ip_network
     return str(ip_network(text).netmask)
 
-@register_filter("ipv4")
+@register_filter('ipv4')
 def vyos_ipv4(text):
     """ Filter IP address, return True on IPv4 address, False otherwise """
     from ipaddress import ip_interface
     return ip_interface(text).version == 4
 
-@register_filter("ipv6")
+@register_filter('ipv6')
 def vyos_ipv6(text):
     """ Filter IP address, return True on IPv6 address, False otherwise """
     from ipaddress import ip_interface
     return ip_interface(text).version == 6
+
+@register_filter('first_host_address')
+def vyos_first_host_address(text):
+    """ Return first usable (host) IP address from given prefix.
+    Example:
+      - 10.0.0.0/24 -> 10.0.0.1
+      - 2001:db8::/64 -> 2001:db8::1
+    """
+    from ipaddress import ip_interface
+    from ipaddress import IPv4Network
+    from ipaddress import IPv6Network
+
+    addr = ip_interface(text)
+    if addr.version == 4:
+        return str(addr.ip +1)
+    return str(addr.ip)
+
+@register_filter('last_host_address')
+def vyos_last_host_address(text):
+    """ Return first usable IP address from given prefix.
+    Example:
+      - 10.0.0.0/24 -> 10.0.0.1
+      - 2001:db8::/64 -> 2001:db8::1
+    """
+    from ipaddress import ip_interface
+    from ipaddress import IPv4Network
+    from ipaddress import IPv6Network
+
+    addr = ip_interface(text)
+    if addr.version == 4:
+        addr = IPv4Network(addr)
+    else:
+        addr = IPv6Network(addr)
+
+    return str(addr.broadcast_address -1)
+
+@register_filter('inc_ip')
+def vyos_inc_ip(text, increment):
+    """ Return first usable IP address from given prefix.
+    Example:
+      - 10.0.0.0/24 -> 10.0.0.1
+      - 2001:db8::/64 -> 2001:db8::1
+    """
+    from ipaddress import ip_interface
+    return str(ip_interface(text).ip + int(increment))
diff --git a/smoketest/scripts/cli/test_interfaces_openvpn.py b/smoketest/scripts/cli/test_interfaces_openvpn.py
index 9485cb913..518249b77 100755
--- a/smoketest/scripts/cli/test_interfaces_openvpn.py
+++ b/smoketest/scripts/cli/test_interfaces_openvpn.py
@@ -26,6 +26,9 @@ from vyos.configsession import ConfigSessionError
 from vyos.util import cmd
 from vyos.util import process_named_running
 from vyos.util import read_file
+from vyos.template import vyos_inc_ip
+from vyos.template import vyos_netmask_from_cidr
+from vyos.template import vyos_last_host_address
 
 PROCESS_NAME = 'openvpn'
 
@@ -66,7 +69,10 @@ class TestInterfacesOpenVPN(unittest.TestCase):
         del self.session
 
     def test_client_verify(self):
-        """ Create OpenVPN client interface and test verify() steps """
+        return True
+        """
+        Create OpenVPN client interface and test verify() steps.
+        """
         interface = 'vtun2000'
         path = base_path + [interface]
         self.session.set(path + ['mode', 'client'])
@@ -101,7 +107,7 @@ class TestInterfacesOpenVPN(unittest.TestCase):
         # check validate() - remote-host must be set in client mode
         with self.assertRaises(ConfigSessionError):
             self.session.commit()
-        self.session.set(path + ['remote-host', 'openvpn.vyos.net'])
+        self.session.set(path + ['remote-host', '192.0.9.9'])
 
         # check validate() - cannot specify "tls dh-file" in client mode
         self.session.set(path + ['tls', 'dh-file', dh_pem])
@@ -131,9 +137,11 @@ class TestInterfacesOpenVPN(unittest.TestCase):
 
 
     def test_client_interfaces(self):
-        """ Create OpenVPN client interfaces connecting to different
-            server IP addresses. Validate configuration afterwards. """
-
+        return True
+        """
+        Create OpenVPN client interfaces connecting to different
+        server IP addresses. Validate configuration afterwards.
+        """
         num_range = range(10, 15)
         for ii in num_range:
             interface = f'vtun{ii}'
@@ -189,7 +197,10 @@ class TestInterfacesOpenVPN(unittest.TestCase):
             self.assertNotIn(interface, interfaces())
 
     def test_server_verify(self):
-        """ Create one OpenVPN server interface and check required verify() stages """
+        return True
+        """
+        Create one OpenVPN server interface and check required verify() stages
+        """
         interface = 'vtun5000'
         path = base_path + [interface]
 
@@ -296,10 +307,85 @@ class TestInterfacesOpenVPN(unittest.TestCase):
         self.assertTrue(process_named_running(PROCESS_NAME))
         self.assertIn(interface, interfaces())
 
-    def test_server_interfaces(self):
-        """ Create OpenVPN server interfaces using different client subnets.
-            Validate configuration afterwards. """
+    def test_server_subnet_topology(self):
+        """
+        Create OpenVPN server interfaces using different client subnets.
+        Validate configuration afterwards.
+        """
+        auth_hash = 'sha256'
+        num_range = range(20, 25)
+        port = ''
+        for ii in num_range:
+            interface = f'vtun{ii}'
+            subnet = f'192.0.{ii}.0/24'
+            path = base_path + [interface]
+            port = str(2000 + ii)
+
+            self.session.set(path + ['device-type', 'tun'])
+            self.session.set(path + ['encryption', 'cipher', 'aes192'])
+            self.session.set(path + ['hash', auth_hash])
+            self.session.set(path + ['mode', 'server'])
+            self.session.set(path + ['local-port', port])
+            self.session.set(path + ['server', 'subnet', subnet])
+            self.session.set(path + ['server', 'topology', 'subnet'])
+            self.session.set(path + ['replace-default-route'])
+            self.session.set(path + ['tls', 'ca-cert-file', ca_cert])
+            self.session.set(path + ['tls', 'cert-file', ssl_cert])
+            self.session.set(path + ['tls', 'key-file', ssl_key])
+            self.session.set(path + ['tls', 'dh-file', dh_pem])
+            self.session.set(path + ['vrf', vrf_name])
+
+        self.session.commit()
+
+        for ii in num_range:
+            interface = f'vtun{ii}'
+            subnet = f'192.0.{ii}.0/24'
+            start_addr = vyos_inc_ip(subnet, '2')
+            stop_addr = vyos_last_host_address(subnet)
+            port = str(2000 + ii)
+
+            config_file = f'/run/openvpn/{interface}.conf'
+            config = read_file(config_file)
+
+            self.assertIn(f'dev {interface}', config)
+            self.assertIn(f'dev-type tun', config)
+            self.assertIn(f'persist-key', config)
+            self.assertIn(f'proto udp', config) # default protocol
+            self.assertIn(f'auth {auth_hash}', config)
+            self.assertIn(f'cipher aes-192-cbc', config)
+            self.assertIn(f'topology subnet', config)
+            self.assertIn(f'lport {port}', config)
+            self.assertIn(f'push "redirect-gateway def1"', config)
+
+            # TLS options
+            self.assertIn(f'ca {ca_cert}', config)
+            self.assertIn(f'cert {ssl_cert}', config)
+            self.assertIn(f'key {ssl_key}', config)
+            self.assertIn(f'dh {dh_pem}', config)
+
+            # IP pool configuration
+            netmask = IPv4Network(subnet).netmask
+            network = IPv4Network(subnet).network_address
+            self.assertIn(f'server {network} {netmask} nopool', config)
+            self.assertIn(f'ifconfig-pool {start_addr} {stop_addr}', config)
+
+            self.assertTrue(process_named_running(PROCESS_NAME))
+            self.assertEqual(get_vrf(interface), vrf_name)
+            self.assertIn(interface, interfaces())
+
+        # check that no interface remained after deleting them
+        self.session.delete(base_path)
+        self.session.commit()
+
+        for ii in num_range:
+            interface = f'vtun{ii}'
+            self.assertNotIn(interface, interfaces())
 
+    def test_server_net30_topology(self):
+        """
+        Create OpenVPN server interfaces (net30) using different client
+        subnets. Validate configuration afterwards.
+        """
         auth_hash = 'sha256'
         num_range = range(20, 25)
         port = ''
@@ -315,6 +401,7 @@ class TestInterfacesOpenVPN(unittest.TestCase):
             self.session.set(path + ['mode', 'server'])
             self.session.set(path + ['local-port', port])
             self.session.set(path + ['server', 'subnet', subnet])
+            self.session.set(path + ['server', 'topology', 'net30'])
             self.session.set(path + ['replace-default-route'])
             self.session.set(path + ['tls', 'ca-cert-file', ca_cert])
             self.session.set(path + ['tls', 'cert-file', ssl_cert])
@@ -327,6 +414,8 @@ class TestInterfacesOpenVPN(unittest.TestCase):
         for ii in num_range:
             interface = f'vtun{ii}'
             subnet = f'192.0.{ii}.0/24'
+            start_addr = vyos_inc_ip(subnet, '4')
+            stop_addr = vyos_last_host_address(subnet)
             port = str(2000 + ii)
 
             config_file = f'/run/openvpn/{interface}.conf'
@@ -351,7 +440,8 @@ class TestInterfacesOpenVPN(unittest.TestCase):
             # IP pool configuration
             netmask = IPv4Network(subnet).netmask
             network = IPv4Network(subnet).network_address
-            self.assertIn(f'server {network} {netmask}', config)
+            self.assertIn(f'server {network} {netmask} nopool', config)
+            self.assertIn(f'ifconfig-pool {start_addr} {stop_addr}', config)
 
             self.assertTrue(process_named_running(PROCESS_NAME))
             self.assertEqual(get_vrf(interface), vrf_name)
@@ -366,7 +456,10 @@ class TestInterfacesOpenVPN(unittest.TestCase):
             self.assertNotIn(interface, interfaces())
 
     def test_site2site_verify(self):
-        """ Create one OpenVPN site2site interface and check required verify() stages """
+        return True
+        """
+        Create one OpenVPN site2site interface and check required verify() stages
+        """
         interface = 'vtun5000'
         path = base_path + [interface]
 
@@ -423,7 +516,10 @@ class TestInterfacesOpenVPN(unittest.TestCase):
         self.session.commit()
 
     def test_site2site_interfaces(self):
-        """ Create two OpenVPN site-to-site interfaces """
+        return True
+        """
+        Create two OpenVPN site-to-site interfaces
+        """
         num_range = range(30, 35)
         port = ''
         local_address = ''
diff --git a/src/conf_mode/interfaces-openvpn.py b/src/conf_mode/interfaces-openvpn.py
index 3ad04610a..b1318b9ee 100755
--- a/src/conf_mode/interfaces-openvpn.py
+++ b/src/conf_mode/interfaces-openvpn.py
@@ -449,15 +449,6 @@ def generate(openvpn):
     if 'deleted' in openvpn or 'disable' in openvpn:
         return None
 
-    # create config directory on demand
-    directories = []
-    directories.append(f'{directory}/status')
-    directories.append(ccd_dir)
-    for onedir in directories:
-        if not os.path.exists(onedir):
-            os.makedirs(onedir, 0o755)
-        chown(onedir, user, group)
-
     # Fix file permissons for keys
     fix_permissions = []
 
diff --git a/src/tests/test_jinja_filters.py b/src/tests/test_jinja_filters.py
index 17219802a..acd7a5952 100644
--- a/src/tests/test_jinja_filters.py
+++ b/src/tests/test_jinja_filters.py
@@ -21,6 +21,9 @@ from vyos.template import vyos_address_from_cidr
 from vyos.template import vyos_netmask_from_cidr
 from vyos.template import vyos_ipv4
 from vyos.template import vyos_ipv6
+from vyos.template import vyos_first_host_address
+from vyos.template import vyos_last_host_address
+from vyos.template import vyos_inc_ip
 
 class TestTeamplteHelpers(TestCase):
     def setUp(self):
@@ -55,3 +58,12 @@ class TestTeamplteHelpers(TestCase):
         self.assertTrue(vyos_ipv6('2001:db8::/32'))
         self.assertTrue(vyos_ipv6('2001:db8::/64'))
 
+    def test_helpers_first_host_address(self):
+        self.assertEqual(vyos_first_host_address('10.0.0.0/24'), '10.0.0.1')
+        self.assertEqual(vyos_first_host_address('10.0.0.128/25'), '10.0.0.129')
+        self.assertEqual(vyos_first_host_address('10.0.0.200/29'), '10.0.0.201')
+
+        self.assertEqual(vyos_first_host_address('2001:db8::/64'), '2001:db8::')
+        self.assertEqual(vyos_first_host_address('2001:db8::/112'), '2001:db8::')
+        self.assertEqual(vyos_first_host_address('2001:db8::10/112'), '2001:db8::10')
+        self.assertEqual(vyos_first_host_address('2001:db8::100/112'), '2001:db8::100')