Merge branch 'topic/dopheide/ipv6' of github.com:/dopheide-esnet/zeek…

…-client

* 'topic/dopheide/ipv6' of github.com:/dopheide-esnet/zeek-client:
  switch to parser_from_string
  Code formatting
  Instance constructor now checks if the IP addr is valid.  Tests check for ValueError if not.
  add .DS_Store to gitignore
  Change error message back to key/val to match test output
  Also strip whitespace from the IP
  Cleanup based on Benjamin's suggestions to make this more readable and fix a bug.
  cleanup after github suggestion commit
  Update zeekclient/brokertypes.py
  Some suggestions for better IPv6 support

.gitignore

@@ -4,3 +4,4 @@ __pycache__
 build
 dist
 zeek_client.egg-info
+.DS_Store

CHANGES

@@ -1,3 +1,15 @@
+1.3.1-15 | 2023-09-18 11:00:08 -0700
+
+  * Instance constructor now checks if the IP addr is valid.  Tests check for ValueError if not. (Michael Dopheide)
+
+  * add .DS_Store to gitignore (Michael Dopheide)
+
+  * Change error message back to key/val to match test output (Michael Dopheide)
+
+  * Also strip whitespace from the IP (Michael Dopheide)
+
+  * Some suggestions for better IPv6 support (Michael Dopheide)
+
 1.3.1-3 | 2023-09-12 17:30:15 -0700
 
   * Add manpage generation/checking to pre-commit setup (Christian Kreibich, Corelight)

VERSION

@@ -1 +1 @@
-1.3.1-3
+1.3.1-15

tests/test_config_io.py

@@ -238,6 +238,74 @@ def test_config_addl_key(self):
             "warning: ignoring unexpected keys: also_not_a_key, not_a_key",
         )
 
+    def test_config_ipv4_ipv6_instances(self):
+        # This test creates a Configuration from an INI file with various IP addresses
+        # and ports specified for the agents.
+
+        ini_input = """[instances]
+agent = 127.0.0.1:2151
+agent2 = ::1:2151
+agent3 = [::2]:2151
+
+[manager]
+instance = agent
+role = MANAGER
+port = 5000
+"""
+        ini_expected = """[instances]
+agent = 127.0.0.1:2151
+agent2 = ::1:2151
+agent3 = ::2:2151
+
+[manager]
+instance = agent
+role = MANAGER
+port = 5000
+"""
+        cfp = self.parser_from_string(ini_input)
+        config = zeekclient.types.Configuration.from_config_parser(cfp)
+        self.assertTrue(config is not None)
+
+        # Turning that back into a config parser should have expected content:
+        cfp = config.to_config_parser()
+        with io.StringIO() as buf:
+            cfp.write(buf)
+            self.assertEqualStripped(buf.getvalue(), ini_expected)
+
+    def test_config_invalid_ipv4_instance(self):
+        # This test creates a Configuration with an invalid IPv4 address
+
+        ini_input = """[instances]
+agent = 127.0.0.1.1:2151
+
+[manager]
+instance = agent
+role = MANAGER
+port = 5000
+"""
+        cfp = self.parser_from_string(ini_input)
+        with self.assertRaisesRegex(
+            ValueError, "'127.0.0.1.1' does not appear to be an IPv4 or IPv6 address"
+        ):
+            zeekclient.types.Configuration.from_config_parser(cfp)
+
+    def test_config_invalid_ipv6_instance(self):
+        # This test creates a Configuration with an invalid IPv6 address
+
+        ini_input = """[instances]
+agent = ::2/128:2151
+
+[manager]
+instance = agent
+role = MANAGER
+port = 5000
+"""
+        cfp = self.parser_from_string(ini_input)
+        with self.assertRaisesRegex(
+            ValueError, "'::2/128' does not appear to be an IPv4 or IPv6 address"
+        ):
+            zeekclient.types.Configuration.from_config_parser(cfp)
+
     def test_config_invalid_instances(self):
         ini_input = """
 [instances]

zeekclient/__init__.py

@@ -26,7 +26,7 @@
 
 from .logs import LOG
 
-__version__ = "1.3.1-3"
+__version__ = "1.3.1-15"
 __all__ = [
     "brokertypes",
     "cli",

zeekclient/brokertypes.py

@@ -525,7 +525,11 @@ def __lt__(self, other):
         res = super().__lt__(other)
         if res != NotImplemented:
             return res
-        return self._addr < other._addr
+        if self._addr.version == other._addr.version:
+            return self._addr < other._addr
+
+        # Make a blanket assumption that an IPv4 address is "less than" an IPv6 address
+        return self._addr.version < other._addr.version
 
     def __hash__(self):
         return hash(self._value)

zeekclient/types.py

@@ -3,6 +3,7 @@
 import enum
 import shlex
 import socket
+from ipaddress import ip_address
 
 from . import brokertypes as bt
 from .utils import make_uuid
@@ -238,7 +239,9 @@ def __init__(self, name, addr=None, port=None):
         self.name = name
         # This is a workaround until we've resolved addresses in instances
         self.host = "0.0.0.0"  # XXX needs proper optionality
-        if addr is not None:
+
+        # If addr isn't a valid address, the ipaddress module will raise a ValueError
+        if addr is not None and ip_address(addr):
             self.host = str(addr)
         self.port = port  # None or integer value; we always mean TCP
 
@@ -650,17 +653,18 @@ def from_config_parser(cls, cfp, _section=None):
                         config.instances.append(Instance(key))
                     else:
                         hostport = val
-                        parts = hostport.split(":", 1)
-                        if len(parts) != 2 or not parts[0] or not parts[1]:
+                        host, _, port = hostport.rpartition(":")
+                        if host == "" or port == "":
                             LOG.error(
                                 'invalid spec for instance "%s": "%s" should be <host>:<port>',
                                 key,
                                 val,
                             )
                             return None
-                        config.instances.append(
-                            Instance(key, parts[0].strip(), parts[1].strip())
-                        )
+                        # remove brackets to support [ipv6]:port formats
+                        host = host.strip("[] ")
+                        port = port.strip()
+                        config.instances.append(Instance(key, host, port))
                 continue
 
             # All keys for sections other than "instances" need to have a value.