From ccd564c2328a086b326957fdde8b07ca560bd6b2 Mon Sep 17 00:00:00 2001
From: Christian Breunig <christian@breunig.cc>
Date: Sun, 26 May 2024 10:06:02 +0200
Subject: dhcpv6-server: T3493: add proper validation for prefix-delegation
 start/stop address

ISC DHCP server expects a string: "prefix6 2001:db8:290:: 2001:db8:29f:: /64;"
where the IPv6 prefix/range must be :: terminaated with a delegated prefix
length at the end.

This commit changes the validator that the IPv6 address defined on the CLI must
always end with ::. In addition a verify() step is added to check that the
stop address is greater than start address.
---
 interface-definitions/service_dhcpv6-server.xml.in  |  6 ++++--
 smoketest/scripts/cli/test_service_dhcpv6-server.py |  8 +++++++-
 src/conf_mode/service_dhcpv6-server.py              | 19 +++++++++++++------
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/interface-definitions/service_dhcpv6-server.xml.in b/interface-definitions/service_dhcpv6-server.xml.in
index a64da83ae..effba3884 100644
--- a/interface-definitions/service_dhcpv6-server.xml.in
+++ b/interface-definitions/service_dhcpv6-server.xml.in
@@ -229,7 +229,8 @@
                             <description>IPv6 address used in prefix delegation</description>
                           </valueHelp>
                           <constraint>
-                            <validator name="ipv6-address"/>
+                            <!-- IPv6 address used MUST end with :: -->
+                            <regex>([a-fA-F0-9]{1,4}:)+:</regex>
                           </constraint>
                         </properties>
                         <children>
@@ -254,7 +255,8 @@
                                 <description>IPv6 address used in prefix delegation</description>
                               </valueHelp>
                               <constraint>
-                                <validator name="ipv6-address"/>
+                                <!-- IPv6 address used MUST end with :: -->
+                                <regex>([a-fA-F0-9]{1,4}:)+:</regex>
                               </constraint>
                             </properties>
                           </leafNode>
diff --git a/smoketest/scripts/cli/test_service_dhcpv6-server.py b/smoketest/scripts/cli/test_service_dhcpv6-server.py
index cb6206632..c07d8509e 100755
--- a/smoketest/scripts/cli/test_service_dhcpv6-server.py
+++ b/smoketest/scripts/cli/test_service_dhcpv6-server.py
@@ -18,6 +18,7 @@ import unittest
 
 from base_vyostest_shim import VyOSUnitTestSHIM
 
+from vyos.configsession import ConfigSessionError
 from vyos.template import inc_ip
 from vyos.utils.process import process_named_running
 from vyos.utils.file import read_file
@@ -143,9 +144,14 @@ class TestServiceDHCPv6Server(VyOSUnitTestSHIM.TestCase):
         pool = base_path + ['shared-network-name', shared_net_name, 'subnet', subnet]
 
         self.cli_set(pool + ['address-range', 'start', range_start, 'stop', range_stop])
-        self.cli_set(pool + ['prefix-delegation', 'start', delegate_start, 'stop', delegate_stop])
         self.cli_set(pool + ['prefix-delegation', 'start', delegate_start, 'prefix-length', delegate_len])
 
+        self.cli_set(pool + ['prefix-delegation', 'start', delegate_start, 'stop', delegate_start])
+        # Prefix delegation stop address must be greater then start address
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+        self.cli_set(pool + ['prefix-delegation', 'start', delegate_start, 'stop', delegate_stop])
+
         # commit changes
         self.cli_commit()
 
diff --git a/src/conf_mode/service_dhcpv6-server.py b/src/conf_mode/service_dhcpv6-server.py
index 560251336..25f19285c 100755
--- a/src/conf_mode/service_dhcpv6-server.py
+++ b/src/conf_mode/service_dhcpv6-server.py
@@ -105,22 +105,29 @@ def verify(dhcpv6):
                 if 'prefix' in subnet_config:
                     for prefix in subnet_config['prefix']:
                         if ip_network(prefix) not in ip_network(subnet):
-                            raise ConfigError(f'address-range prefix "{prefix}" is not in subnet "{subnet}""')
+                            raise ConfigError(f'address-range prefix "{prefix}" is not in subnet "{subnet}"!')
 
             # Prefix delegation sanity checks
             if 'prefix_delegation' in subnet_config:
                 if 'start' not in subnet_config['prefix_delegation']:
-                    raise ConfigError('prefix-delegation start address not defined!')
+                        raise ConfigError(f'Start address of delegated IPv6 prefix range "{prefix}" '\
+                                          f'must be configured!')
 
                 for prefix, prefix_config in subnet_config['prefix_delegation']['start'].items():
                     if 'stop' not in prefix_config:
-                        raise ConfigError(f'Stop address of delegated IPv6 '\
-                                          f'prefix range "{prefix}" '\
-                                          f'must be configured')
+                        raise ConfigError(f'Stop address of delegated IPv6 prefix range "{prefix}" '\
+                                          f'must be configured!')
+
+                    start_addr = prefix
+                    stop_addr = prefix_config['stop']
+
+                    if ip_address(stop_addr) <= ip_address(start_addr):
+                        raise ConfigError(f'Stop address of delegated IPv6 prefix range "{prefix}" '\
+                                          f'must be greater than start address!')
 
                     if 'prefix_length' not in prefix_config:
                         raise ConfigError(f'Length of delegated IPv6 prefix '\
-                                          f'must be configured')
+                                          f'must be configured!')
 
             # Static mappings don't require anything (but check if IP is in subnet if it's set)
             if 'static_mapping' in subnet_config:
-- 
cgit v1.2.3