Merge pull request #794 from networktocode/u/joewesch-fix-palo-banner-parsing

[NTC-4957] Fixed parsing Palo banners that start on the same line

Author: joewesch

changes/+invoke.housekeeping

@@ -0,0 +1 @@
+Added `--pattern` and `--label` options to the `invoke pytest` task.

changes/780.fixed

@@ -0,0 +1 @@
+Fixed parsing of login banner in Palo Alto Networks config.

netutils/config/conversion.py

@@ -125,6 +125,7 @@ def paloalto_panos_clean_newlines(cfg: str) -> str:
     return newlines_cleaned_cfg
 
 
+# pylint: disable=too-many-branches
 def paloalto_panos_brace_to_set(cfg: str, cfg_type: str = "file") -> str:
     r"""Convert Palo Alto Brace format configuration to set format.
 
@@ -182,21 +183,44 @@ def paloalto_panos_brace_to_set(cfg: str, cfg_type: str = "file") -> str:
         cfg_raw = paloalto_panos_clean_newlines(cfg=cfg)
         cfg_list = cfg_raw.splitlines()
 
-    for i, line in enumerate(cfg_list):
+    def cfg_generator(cfg_list: t.List[str]) -> t.Generator[str, None, None]:
+        """We use a generator to avoid parsing the banner lines twice."""
+        yield from cfg_list
+
+    cfg_gen = cfg_generator(cfg_list)
+
+    for line in cfg_gen:
         line = line.strip()
         if line.endswith(";") and not line.startswith('";'):
-            line = line.split(";", 1)[0]
+            line = line[:-1]
             line = "".join(str(s) for s in stack) + line
             line = line.split("config ", 1)[1]
             line = "set " + line
             cfg_value.append(line.strip())
-        elif line.endswith('login-banner "') or line.endswith('content "'):
+        elif "login-banner" in line or line.endswith('content "'):
             _first_banner_line = "".join(str(s) for s in stack) + line
             cfg_value.append("set " + _first_banner_line.split("config ", 1)[1])
-
-            for banner_line in cfg_list[i + 1:]:  # fmt: skip
-                if '"' in banner_line:
-                    banner_line = banner_line.split(";", 1)[0]
+            # Palo Alto uses either double or single quotes for the banner delimiter,
+            # but only if there are certain characters or spaces in the banner.
+            if 'login-banner "' in line:
+                delimiter = '"'
+            elif "login-banner '" in line:
+                delimiter = "'"
+            else:
+                delimiter = ""
+
+            # Deal with single line banners first
+            if line.endswith(f"{delimiter};"):
+                line = line[:-1]
+                cfg_value.append(line.strip())
+                continue
+
+            # Multi-line banners
+            for banner_line in cfg_gen:  # fmt: skip
+                # This is a little brittle and will break if any line in the middle of the banner
+                # ends with the expected delimiter and semicolon.
+                if banner_line.endswith(f"{delimiter};"):
+                    banner_line = banner_line[:-1]
                     cfg_value.append(banner_line.strip())
                     break
                 cfg_value.append(banner_line.strip())

netutils/config/parser.py

@@ -1492,17 +1492,11 @@ class PaloAltoNetworksConfigParser(BaseSpaceConfigParser):
 
     comment_chars: t.List[str] = []
     banner_start: t.List[str] = [
-        'set system login-banner "',
-        'login-banner "',
-        'set deviceconfig system login-banner "',
+        "set system login-banner",
+        "set deviceconfig system login-banner",
     ]
-    banner_end = '"'
-
-    def is_banner_end(self, line: str) -> bool:
-        """Determine if end of banner."""
-        if line.endswith('"') or line.startswith('";') or line.startswith("set") or line.endswith(self.banner_end):
-            return True
-        return False
+    # Not used, but must be defined
+    banner_end = ""
 
     def _build_banner(self, config_line: str) -> t.Optional[str]:
         """Handle banner config lines.
@@ -1518,24 +1512,24 @@ def _build_banner(self, config_line: str) -> t.Optional[str]:
         """
         self._update_config_lines(config_line)
         self._current_parents += (config_line,)
-        banner_config = []
+        banner_config: t.List[str] = []
         for line in self.generator_config:
-            if not self.is_banner_end(line):
-                banner_config.append(line)
-            else:
-                banner_config.append(line.strip())
-                line = "\n".join(banner_config)
-                if line.endswith('"'):
-                    banner, end, _ = line.rpartition('"')
-                    line = banner + end
-                self._update_config_lines(line.strip())
+            # Note, this is a little fragile and will cause false positives if any line in
+            # the middle of a multi-line banner starts with "set ".
+            if line.startswith("set "):
+                # New command, save the banner and return the next line
+                if banner_config:
+                    banner_string = "\n".join(banner_config)
+                    self._update_config_lines(banner_string)
                 self._current_parents = self._current_parents[:-1]
-                try:
-                    return next(self.generator_config)
-                except StopIteration:
-                    return None
+                return line
+            banner_config.append(line)
 
-        raise ValueError("Unable to parse banner end.")
+        # Edge case, the last line of the config is the banner
+        banner_string = "\n".join(banner_config)
+        self._update_config_lines(banner_string)
+        self._current_parents = self._current_parents[:-1]
+        return None
 
     def build_config_relationship(self) -> t.List[ConfigLine]:  # pylint: disable=too-many-branches
         r"""Parse text of config lines and find their parents.
@@ -1558,42 +1552,27 @@ def build_config_relationship(self) -> t.List[ConfigLine]:  # pylint: disable=to
             ... ]
             True
         """
-        # assume configuration does not need conversion
-        _needs_conversion = False
+        if self.config_lines_only is None:
+            raise ValueError("Config is empty.")
 
-        # if config is in palo brace format, convert to set
-        if self.config_lines_only is not None:
-            for line in self.config_lines_only.splitlines():
-                if line.endswith("{"):
-                    _needs_conversion = True
-        if _needs_conversion:
+        if "@dirtyId" in self.config_lines_only:
+            # We have to specifically check for JSON format because it can be confused with the brace format
+            raise ValueError("Found 'json' configuration format. Please provide in 'set' or 'default' (brace) format.")
+        config_lines = self.config_lines_only.splitlines()
+        if any(line.endswith("{") for line in config_lines):
             converted_config = paloalto_panos_brace_to_set(cfg=self.config, cfg_type="string")
             list_config = converted_config.splitlines()
             self.generator_config = (line for line in list_config)
+        elif not any(line.startswith("set ") for line in config_lines):
+            raise ValueError("Unexpected configuration format. Please provide in 'set' or 'default' (brace) format.")
 
         # build config relationships
         for line in self.generator_config:
-            if not line[0].isspace():
-                self._current_parents = ()
-                if self.is_banner_start(line):
-                    line = self._build_banner(line)  # type: ignore
-            else:
-                previous_config = self.config_lines[-1]
-                self._current_parents = (previous_config.config_line,)
-                self.indent_level = self.get_leading_space_count(line)
-                if not self.is_banner_start(line):
-                    line = self._build_nested_config(line)  # type: ignore
-                else:
-                    line = self._build_banner(line)  # type: ignore
-                    if line is not None and line[0].isspace():
-                        line = self._build_nested_config(line)  # type: ignore
-                    else:
-                        self._current_parents = ()
+            if self.is_banner_start(line):
+                line = self._build_banner(line)  # type: ignore
 
             if line is None:
                 break
-            elif self.is_banner_start(line):
-                line = self._build_banner(line)  # type: ignore
 
             self._update_config_lines(line)
         return self.config_lines

tasks.py

@@ -169,10 +169,23 @@ def coverage(context):
     run_command(context, "coverage html")
 
 
-@task
-def pytest(context):
+@task(
+    help={
+        "pattern": "Only run tests which match the given substring. Can be used multiple times.",
+        "label": "Module path to run (e.g., tests/unit/test_foo.py). Can be used multiple times.",
+    },
+    iterable=["pattern", "label"],
+)
+def pytest(context, pattern=None, label=None):
     """Run pytest test cases."""
-    exec_cmd = "pytest -vv --doctest-modules netutils/ && coverage run --source=netutils -m pytest && coverage report"
+    doc_test_cmd = "pytest -vv --doctest-modules netutils/"
+    pytest_cmd = "coverage run --source=netutils -m pytest"
+    if pattern:
+        pytest_cmd += "".join([f" -k {_pattern}" for _pattern in pattern])
+    if label:
+        pytest_cmd += "".join([f" {_label}" for _label in label])
+    coverage_cmd = "coverage report"
+    exec_cmd = " && ".join([doc_test_cmd, pytest_cmd, coverage_cmd])
     run_command(context, exec_cmd)
 
 

tests/unit/mock/config/compliance/compliance/paloalto_panos/paloalto_backup.txt

@@ -21,6 +21,20 @@ set deviceconfig system service disable-http yes
 set deviceconfig system service disable-snmp no
 set deviceconfig system snmp-setting snmp-system 
 set deviceconfig system hostname firewall1
+set deviceconfig system login-banner "
+************************************************************************
+*                        firewall1.example.com                         *                         [PROD VM500  firewalls]
+************************************************************************
+*                               WARNING                                *
+*   Unauthorized access to this device or devices attached to          *
+*   or accessible from this network is strictly prohibited.            *
+*   Possession of passwords or devices enabling access to this         *
+*   device or devices does not constitute authorization. Unauthorized  *
+*   access will be prosecuted to the fullest extent of the law.        *
+*                                                                      *
+************************************************************************
+
+"
 set deviceconfig system default-gateway 10.1.1.1
 set deviceconfig system dns-setting servers primary 10.1.1.3
 set deviceconfig system dns-setting servers secondary 10.1.1.4

tests/unit/mock/config/compliance/compliance/paloalto_panos/paloalto_feature.py

@@ -1,3 +1,4 @@
 features = [
     {"name": "management", "ordered": False, "section": ["set mgt-config "]},
+    {"name": "banner", "ordered": False, "section": ["set deviceconfig system login-banner "]},
 ]

tests/unit/mock/config/compliance/compliance/paloalto_panos/paloalto_intended.txt

@@ -21,6 +21,22 @@ set deviceconfig system service disable-http yes
 set deviceconfig system service disable-snmp no
 set deviceconfig system snmp-setting snmp-system 
 set deviceconfig system hostname firewall1
+set deviceconfig system login-banner "####################################################
+WARNING TO UNAUTHORIZED USERS:
+This system is for use by authorized users only.
+Any individual using this system, by such use,
+acknowledges and consents to the right of the
+company to monitor, access, use, and disclose any
+information generated, received, or stored on the
+systems, and waives any right of privacy or
+expectation of privacy on the part of that
+individual in connection with his or her use of
+this system. Unauthorized and/or improper use of
+this system, as delineated by corporate policies,
+is not tolerated and the company may take formal
+action against such individuals.
+####################################################
+"
 set deviceconfig system default-gateway 10.1.1.1
 set deviceconfig system dns-setting servers primary 10.1.1.3
 set deviceconfig system dns-setting servers secondary 10.1.1.4

tests/unit/mock/config/compliance/compliance/paloalto_panos/paloalto_received.json

@@ -8,5 +8,15 @@
     "missing": "",
     "ordered_compliant": true,
     "unordered_compliant": true
+  },
+  "banner": {
+    "compliant": false,
+    "missing": "set deviceconfig system login-banner \"####################################################\nWARNING TO UNAUTHORIZED USERS:\nThis system is for use by authorized users only.\nAny individual using this system, by such use,\nacknowledges and consents to the right of the\ncompany to monitor, access, use, and disclose any\ninformation generated, received, or stored on the\nsystems, and waives any right of privacy or\nexpectation of privacy on the part of that\nindividual in connection with his or her use of\nthis system. Unauthorized and/or improper use of\nthis system, as delineated by corporate policies,\nis not tolerated and the company may take formal\naction against such individuals.\n####################################################\n\"",
+    "extra": "set deviceconfig system login-banner \"\n************************************************************************\n*                        firewall1.example.com                         *                         [PROD VM500  firewalls]\n************************************************************************\n*                               WARNING                                *\n*   Unauthorized access to this device or devices attached to          *\n*   or accessible from this network is strictly prohibited.            *\n*   Possession of passwords or devices enabling access to this         *\n*   device or devices does not constitute authorization. Unauthorized  *\n*   access will be prosecuted to the fullest extent of the law.        *\n*                                                                      *\n************************************************************************\n\"",
+    "cannot_parse": true,
+    "unordered_compliant": false,
+    "ordered_compliant": false,
+    "actual": "set deviceconfig system login-banner \"\n************************************************************************\n*                        firewall1.example.com                         *                         [PROD VM500  firewalls]\n************************************************************************\n*                               WARNING                                *\n*   Unauthorized access to this device or devices attached to          *\n*   or accessible from this network is strictly prohibited.            *\n*   Possession of passwords or devices enabling access to this         *\n*   device or devices does not constitute authorization. Unauthorized  *\n*   access will be prosecuted to the fullest extent of the law.        *\n*                                                                      *\n************************************************************************\n\"",
+    "intended": "set deviceconfig system login-banner \"####################################################\nWARNING TO UNAUTHORIZED USERS:\nThis system is for use by authorized users only.\nAny individual using this system, by such use,\nacknowledges and consents to the right of the\ncompany to monitor, access, use, and disclose any\ninformation generated, received, or stored on the\nsystems, and waives any right of privacy or\nexpectation of privacy on the part of that\nindividual in connection with his or her use of\nthis system. Unauthorized and/or improper use of\nthis system, as delineated by corporate policies,\nis not tolerated and the company may take formal\naction against such individuals.\n####################################################\n\""
   }
 }

tests/unit/mock/config/conversion/paloalto_panos/paloalto_banner_double_quote_converted.txt

@@ -0,0 +1,3 @@
+set deviceconfig system hostname pa-ntc
+set deviceconfig system login-banner '"BANNER"'
+set deviceconfig system domain ntc