feature - add/remove interfaces by PCI ID

BrennanPaciorek · BrennanPaciorek · commit 2b0bb1b9fb51 · 2022-07-21T20:16:13.000-04:00
FEATURE OVERVIEW - allows users to add by what a device is (vendor:device_type) instead of interface names - interface names that match the wildcard XXXX:XXXX (X = hex) will be converted to interface names. - Multiple matches will result in play being done on multiple devices - Add Network Manager interaction when adding/removing interfaces from zones - Add functions that convert PCI IDs into network interface names FILE CHANGES library/firewall_lib.py: - new parameter interface_pci_id - can be used with or in place of interface argument - uses interfaces identified by PCI device ID - new function parse_pci_id - converts PCI ID in interface_pci to logical network interface name - module fails if PCI ID format invalid - new global variable NM_IMPORTED: bool - true if Network Manager imported by firewalld - new function try_set_zone_of_interface: - Tries to add an interface to a zone using Network Manager - new function try_get_connection_of_interface - Tries to get a network interface's Network Manager connection - Helper function for try_set_zone_of_interface - Adding/removing interfaces to zones using permanent configuration now tries to use Network Manager before adding to firewalld tests/unit/test_firewall_lib.py: - unit test for parse_pci_id function - TEST_DATA entries for interface_pci_id tests/tests_interface_pci: - File added - Tests adding, readding, and removing interface by PCI ID - only works for Virtual Machine virtio network interface - Checks firewall_backend rules to ensure the feature works properly README.md: - Updated to reflect new parameter interface_pci_id - examples differentiating interface and interface_pci_id Fixes #87
diff --git a/README.md b/README.md
@@ -200,10 +200,63 @@ source: 192.0.2.0/24
 
 String or list of interface name strings.
 
-```
+```yaml
 interface: eth2
 ```
 
+This role handles interface arguments similar to
+how firewalld's cli, `firewall-cmd` does, i.e.
+manages the interface through NetworkManger if possible,
+and handles the interface binding purely through firewalld
+otherwise.
+
+```
+WARNING: Neither firewalld nor this role throw any
+errors if the interface name specified is not
+tied to any existing network interface. This can cause confusion
+when attempting to add an interface via PCI device ID,
+for which you should use the parameter `interface_pci_id`
+instead of the `interface` parameter.
+
+Allow interface named '8086:15d7' in dmz zone
+
+firewall:
+  - zone: dmz
+    interface: 8086:15d7
+    state: enabled
+
+The above will successfully add a nftables/iptables rule
+for an interface named `8086:15d7`, but no traffic should/will
+ever match to an interface with this name.
+
+TLDR - When using this parameter, please stick only to using
+logical interface names that you know exist on the device to 
+avoid confusing behavior.
+```
+
+### interface_pci_id
+
+String or list of interface PCI device IDs.
+Accepts PCI IDs if the wildcard `XXXX:YYYY` applies
+where:
+- XXXX: Hexadecimal, corresponds to Vendor ID
+- YYYY: Hexadecimal, corresponds to Device ID
+
+```yaml
+# PCI id for Intel Corporation Ethernet Connection
+interface_pci_id: 8086:15d7
+```
+
+Only accepts PCI devices IDs that correspond to a named network interface,
+and converts all PCI device IDs to their respective logical interface names.
+
+If a PCI id corresponds to more than one logical interface name,
+all interfaces with the PCI id specified will have the play applied.
+
+A list of PCI devices with their IDs can be retrieved using `lcpci -nn`.
+For more information on PCI device IDs, see the linux man page at:
+https://man7.org/linux/man-pages/man5/pci.ids.5.html
+
 ### icmp_block
 
 String or list of ICMP type strings to block.  The ICMP type names needs to be
@@ -420,6 +473,14 @@ firewall:
     state: disabled
 ```
 
+Allow interface with PCI device ID '8086:15d7' in dmz zone
+```yaml
+firewall:
+  - zone: dmz
+    interface_pci_id: 8086:15d7
+    state: enabled
+```
+
 Example Playbooks
 -----------------
 
diff --git a/library/firewall_lib.py b/library/firewall_lib.py
@@ -98,6 +98,13 @@
     required: false
     type: list
     elements: str
+  interface_pci_id:
+    description:
+      List of inteface PCI device ID strings.
+      PCI device ID needs to correspond to a named network interface.
+    required: false
+    type: list
+    elements: str
   icmp_block:
     description:
       List of ICMP type strings to block.
@@ -205,6 +212,8 @@
 
 from distutils.version import LooseVersion
 from ansible.module_utils.basic import AnsibleModule
+import re
+import os
 
 try:
     import firewall.config
@@ -222,6 +231,52 @@
 except ImportError:
     HAS_FIREWALLD = False
 
+try:
+    from firewall.core.fw_nm import (
+        nm_is_imported,
+        nm_get_connection_of_interface,
+        nm_get_zone_of_connection,
+        nm_set_zone_of_connection,
+        nm_get_interfaces,
+        nm_get_client,
+    )
+
+    NM_IMPORTED = nm_is_imported()
+except ImportError:
+    NM_IMPORTED = False
+
+
+PCI_REGEX = re.compile("[0-9a-fA-F]{4}:[0-9a-fA-F]{4}")
+
+
+def try_get_connection_of_interface(interface):
+    try:
+        return nm_get_connection_of_interface(interface)
+    except Exception:
+        return None
+
+
+def try_set_zone_of_interface(module, _zone, interface):
+    if NM_IMPORTED:
+        connection = try_get_connection_of_interface(interface)
+        if connection is not None:
+            if _zone == "":
+                zone_string = "the default zone"
+            else:
+                zone_string = _zone
+            if _zone == nm_get_zone_of_connection(connection):
+                module.log(
+                    msg="The interface is under control of NetworkManager and already bound to '%s'"
+                    % zone_string
+                )
+            elif not module.check_mode:
+                nm_set_zone_of_connection(_zone, connection)
+            return True
+    return False
+
+
+# Above: adapted from firewall-cmd source code
+
 
 def create_service(module, fw, service):
     if not module.check_mode:
@@ -269,6 +324,46 @@ def handle_interface_permanent(
             fw_settings.addInterface(item)
 
 
+pci_ids = None
+
+
+def get_interface_pci():
+    pci_dict = {}
+    for interface in nm_get_interfaces():
+        # udi/device/[vendor, device]
+        interface_ids = []
+        device_udi = nm_get_client().get_device_by_iface(interface).get_udi()
+        device_path = os.path.join(device_udi, "device")
+        for field in ["vendor", "device"]:
+            with open(os.path.join(device_path, field)) as _file:
+                interface_ids.append(_file.readline().strip(" \n")[2:])
+        interface_ids = ":".join(interface_ids)
+        if interface_ids not in pci_dict:
+            pci_dict[interface_ids] = [interface]
+        else:
+            pci_dict[interface_ids].append(interface)
+    return pci_dict
+
+
+def parse_pci_id(module, item):
+    if PCI_REGEX.search(item):
+        global pci_ids
+        if not pci_ids:
+            pci_ids = get_interface_pci()
+
+        interface_name = pci_ids.get(item)
+        if interface_name:
+            return interface_name
+
+        module.warn(msg="No network interfaces found with PCI device ID %s" % item)
+    else:
+        module.fail_json(
+            msg="PCI id %s does not match format: XXXX:XXXX (X = hexadecimal number)"
+            % item
+        )
+    return []
+
+
 def parse_port(module, item):
     _port, _protocol = item.split("/")
     if _protocol is None:
@@ -447,6 +542,9 @@ def main():
             rich_rule=dict(required=False, type="list", elements="str", default=[]),
             source=dict(required=False, type="list", elements="str", default=[]),
             interface=dict(required=False, type="list", elements="str", default=[]),
+            interface_pci_id=dict(
+                required=False, type="list", elements="str", default=[]
+            ),
             icmp_block=dict(required=False, type="list", elements="str", default=[]),
             icmp_block_inversion=dict(required=False, type="bool", default=None),
             timeout=dict(required=False, type="int", default=0),
@@ -532,6 +630,10 @@ def main():
         elif destination_ipv6 and ip_type == "ipv6":
             module.fail_json(msg="cannot have more than one destination ipv6")
     interface = module.params["interface"]
+    for _interface in module.params["interface_pci_id"]:
+        for interface_name in parse_pci_id(module, _interface):
+            if interface_name not in interface:
+                interface.append(interface_name)
     icmp_block = module.params["icmp_block"]
     icmp_block_inversion = module.params["icmp_block_inversion"]
     timeout = module.params["timeout"]
@@ -1091,21 +1193,27 @@ def exception_handler(exception_message):
                 if not module.check_mode:
                     fw.changeZoneOfInterface(zone, item)
                 changed = True
-            if permanent and not fw_settings.queryInterface(item):
-                if not module.check_mode:
-                    handle_interface_permanent(
-                        zone, item, fw_zone, fw_settings, fw, fw_offline, module
-                    )
-                changed = True
+            if permanent:
+                if try_set_zone_of_interface(module, zone, item):
+                    changed = True
+                elif not fw_settings.queryInterface(item):
+                    if not module.check_mode:
+                        handle_interface_permanent(
+                            zone, item, fw_zone, fw_settings, fw, fw_offline, module
+                        )
+                    changed = True
         elif state == "disabled":
             if runtime and fw.queryInterface(zone, item):
                 if not module.check_mode:
                     fw.removeInterface(zone, item)
                 changed = True
-            if permanent and fw_settings.queryInterface(item):
-                if not module.check_mode:
-                    fw_settings.removeInterface(item)
-                changed = True
+            if permanent:
+                if try_set_zone_of_interface(module, "", item):
+                    changed = True
+                elif fw_settings.queryInterface(item):
+                    if not module.check_mode:
+                        fw_settings.removeInterface(item)
+                    changed = True
 
     # icmp_block
     for item in icmp_block:
diff --git a/tests/tests_interface_pci.yml b/tests/tests_interface_pci.yml
@@ -0,0 +1,79 @@
+---
+# Tests interface_pci field, test must be used in VM
+- name: Test interfaces with PCI ids
+  hosts: all
+  become: true
+  roles:
+    - linux-system-roles.firewall
+
+  tasks:
+    - name: get backend from dbus
+      command: >-
+        dbus-send --system --print-reply --type=method_call
+        --dest=org.fedoraproject.FirewallD1
+        /org/fedoraproject/FirewallD1/config
+        org.freedesktop.DBus.Properties.Get
+        string:"org.fedoraproject.FirewallD1.config"
+        string:"FirewallBackend"
+      ignore_errors: yes
+      register: result
+
+    - name: get backend from result
+      set_fact:
+        nftables_backend:
+          "{{ result is not failed and 'nftables' in result.stdout }}"
+
+    - name: test interfaces with PCI ids
+      block:
+        - name: add pci device ethernet controller
+          include_role:
+            name: linux-system-roles.firewall
+          vars:
+            firewall:
+              zone: internal
+              interface_pci_id: 1af4:0001
+              state: enabled
+              permanent: yes
+
+        - name: add pci device again
+          include_role:
+            name: linux-system-roles.firewall
+          vars:
+            firewall:
+              zone: internal
+              interface_pci_id: 1af4:0001
+              state: enabled
+              permanent: yes
+
+        - name: assert pcid not in nftable ruleset
+          command: nft list ruleset
+          register: result
+          failed_when: result is failed or '1af4:0001' in result.stdout
+          when: nftables_backend | bool
+
+        - name: assert pcid not in iptables rules
+          command: iptables -S
+          register: result
+          failed_when: result is failed or '1af4:0001' in result.stdout
+          when: not nftables_backend | bool
+
+        - name: remove interface from internal
+          include_role:
+            name: linux-system-roles.firewall
+          vars:
+            firewall:
+              zone: internal
+              interface_pci_id: 1af4:0001
+              state: disabled
+              permament: yes
+      always:
+        - name: Cleanup
+          tags:
+            - tests::cleanup
+          include_role:
+            name: linux-system-roles.firewall
+          vars:
+            firewall:
+              - previous: replaced
+
+...
diff --git a/tests/unit/test_firewall_lib.py b/tests/unit/test_firewall_lib.py
@@ -188,6 +188,20 @@
             }
         },
     },
+    "InterfacePciId": {
+        "input": {"interface_pci_id": ["600D:7C1D"]},
+        "enabled": {
+            "expected": {
+                "runtime": [call("default", "600D:7C1D")],
+            },
+        },
+        "disabled": {
+            "expected": {
+                "runtime": [call("default", "600D:7C1D")],
+                "permanent": [call("600D:7C1D")],
+            },
+        },
+    },
     "IcmpBlock": {
         "input": {
             "icmp_block": ["echo-request"],
@@ -360,6 +374,24 @@ def test_parse_forward_port(self):
         rc = firewall_lib.parse_forward_port(module, item)
         self.assertEqual(("a", "b", None, None), rc)
 
+    @patch("firewall_lib.AnsibleModule", new_callable=MockAnsibleModule)
+    @patch("firewall_lib.HAS_FIREWALLD", True)
+    @patch("firewall_lib.pci_ids", {"600D:7C1D": ["eth0"]})
+    def test_parse_pci_id(self, am_class):
+        am = am_class.return_value
+
+        am.params = {"interface_pci_id": ["123G:1111"]}
+        with self.assertRaises(MockException):
+            firewall_lib.main()
+        am.fail_json.assert_called_with(
+            msg="PCI id 123G:1111 does not match format: XXXX:XXXX (X = hexadecimal number)"
+        )
+
+        am.params = {"interface_pci_id": ["600D:7C1D"]}
+        with self.assertRaises(MockException):
+            firewall_lib.main()
+        am.fail_json.assert_called_with(msg="Options invalid without state option set")
+
 
 @patch("firewall_lib.AnsibleModule", new_callable=MockAnsibleModule)
 class FirewallLibMain(unittest.TestCase):
