Changes requested by dmbaturin to improve testing. This also exposed a bug so

that was corrected.

Author: cblackburn-igl

interface-definitions/include/dhcp/dhcp-server-common-config.xml.i

@@ -14,21 +14,29 @@
         <help>Match DHCP option 82 (relay agent information)</help>
       </properties>
       <children>
-      <leafNode name="circuit-id">
-        <properties>
-          <help>Filters on the contents of the circuit-id sub option</help>
+        <leafNode name="circuit-id">
+          <properties>
+            <help>Filters on the contents of the circuit-id sub option</help>
+            <valueHelp>
+              <format>hex</format>
+              <description>Values that start with 0x are interpreted as raw hex. This must only be hexadecimal characters e.g. 0x1234567890ABCDEF</description>
+            </valueHelp>
             <valueHelp>
               <format>txt</format>
-              <description>Assumes ASCII text unless input starts with 0x in which case it is interpreted as raw hex</description>
+              <description>Any other text string is interpreted as ASCII text</description>
             </valueHelp>
           </properties>
         </leafNode>
         <leafNode name="remote-id">
           <properties>
             <help>Filters on the contents of the remote-id sub option.</help>
+            <valueHelp>
+              <format>hex</format>
+              <description>Values that start with 0x are interpreted as raw hex. This must only be hexadecimal characters e.g. 0x1234567890ABCDEF</description>
+            </valueHelp>
             <valueHelp>
               <format>txt</format>
-              <description>Assumes ASCII text unless input starts with 0x in which case it is interpreted as raw hex</description>
+              <description>Any other text string is interpreted as ASCII text</description>
             </valueHelp>
           </properties>
         </leafNode>

python/vyos/kea.py

@@ -679,10 +679,15 @@ def kea_build_client_class_test(config):
 
     if "option82" in config:
         if "circuit_id" in config["option82"]:
-            conditions.append("relay4[1].hex == 0x" + config["option82"]["circuit_id"].encode().hex().lower())
+            if config["option82"]["circuit_id"].startswith("0x"):
+                conditions.append("relay4[1].hex == " + config["option82"]["circuit_id"])
+            else:
+                conditions.append("relay4[1].hex == 0x" + config["option82"]["circuit_id"].encode().hex().lower())
         if "remote_id" in config["option82"]:
-            conditions.append("relay4[2].hex == 0x" + config["option82"]["remote_id"].encode().hex().lower())
-
+            if config["option82"]["remote_id"].startswith("0x"):
+                conditions.append("relay4[1].hex == " + config["option82"]["remote_id"])
+            else:
+                conditions.append("relay4[1].hex == 0x" + config["option82"]["remote_id"].encode().hex().lower())
 
     test = " and ".join(conditions)
 

smoketest/scripts/cli/test_service_dhcp-server.py

@@ -240,30 +240,58 @@ def test_dhcp_client_class(self):
         self.cli_set(base_path + ['shared-network-name', shared_net_name, 'subnet', subnet, 'client-class', 'test'])
 
         client_class = base_path + ['client-class', 'test']
+
+        # Test that invalid hex is rejected
+        self.cli_set(client_class + ['option82', 'circuit-id', '0xHELLOWORLD'])
+
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+
+        self.cli_delete(client_class + ['option82', 'circuit-id'])
+        self.cli_set(client_class + ['option82', 'remote-id', '0xHELLOWORLD'])
+
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+
+        self.cli_delete(client_class + ['option82', 'remote-id'])
+
+        # Test string literals
         self.cli_set(client_class + ['option82', 'circuit-id', 'foo'])
         self.cli_set(client_class + ['option82', 'remote-id', 'bar'])
 
         self.cli_commit()
 
+        self.check_client_class_in_config()
+
+        self.cli_delete(client_class + ['option82', 'circuit-id'])
+        self.cli_delete(client_class + ['option82', 'remote-id'])
+
+        # Test hex strings
+        self.cli_set(client_class + ['option82', 'circuit-id', '0x666f6f'])
+        self.cli_set(client_class + ['option82', 'remote-id', '0x626172'])
+
+        self.cli_commit()
+
+        self.check_client_class_in_config()
+
+    def check_client_class_in_config(self):
         config = read_file(KEA4_CONF)
         obj = loads(config)
-
         self.verify_config_value(
             obj, ['Dhcp4', 'client-classes', 0], 'name', 'test'
         )
-
         self.verify_config_value(
-            obj, ['Dhcp4', 'client-classes', 0], 'test', 'relay4[1].hex == 0x666f6f and relay4[2].hex == 0x626172'
+            obj, ['Dhcp4', 'client-classes', 0], 'test',
+            'relay4[1].hex == 0x666f6f and relay4[2].hex == 0x626172'
         )
-
         self.verify_config_value(
-            obj, ['Dhcp4', 'shared-networks', 0, 'subnet4', 0], 'client-class', 'test'
+            obj, ['Dhcp4', 'shared-networks', 0, 'subnet4', 0], 'client-class',
+            'test'
         )
-
         self.verify_config_value(
-            obj, ['Dhcp4', 'shared-networks', 0, 'subnet4', 0, 'pools', 0], 'client-class', 'test'
+            obj, ['Dhcp4', 'shared-networks', 0, 'subnet4', 0, 'pools', 0],
+            'client-class', 'test'
         )
-
         # Check for running process
         self.verify_service_running()
 

src/conf_mode/service_dhcp-server.py

@@ -15,6 +15,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
+import re
 
 from sys import exit
 from sys import argv
@@ -521,6 +522,26 @@ def verify(dhcp):
         if 'reverse_domain' in ddns:
             verify_ddns_domain_servers('Reverse', ddns['reverse_domain'])
 
+    if 'client_class' in dhcp:
+        # Check client class values are valid
+        for class_name, class_config in dhcp['client_class'].items():
+            if 'option82' in class_config:
+                option82_config = class_config['option82']
+                # Compile a regex that will scan for valid inputs. Input can be
+                # either hex in the form 0x0123456789ABCDEF or a string that
+                # does *not* start with 0x. i.e. 0xHELLOWORLD is bad
+                pattern = re.compile(r'^(?:0x[0-9A-Fa-f]+|(?!0x).+)$')
+
+                if 'circuit_id' in option82_config:
+                    circuit_id = option82_config['circuit_id']
+                    if not pattern.match(circuit_id):
+                        raise ConfigError(f'Invalid circuit-id "{circuit_id}" must be either text literal or hex string starting with 0x')
+
+                if 'remote_id' in option82_config:
+                    remote_id = option82_config['remote_id']
+                    if not pattern.match(remote_id):
+                        raise ConfigError(f'Invalid remote-id "{remote_id}" must be either text literal or hex string starting with 0x')
+
     return None