ufw: add support for interface_in and interface_out (#65382)

* ufw: escalate privileges in integration tests

A few of the integration tests for the UFW module forgot to `become`.
This is problematic if the test suite is executed as a non-privileged
user.  This commit amends that by adding `become` when appropriate.

* ufw: add unit tests for direction and interface

Extend the unit tests for the UFW module to test the `direction` and
`interface` parameters.  This will help in the implementation of a fix
for issue #63903.

* ufw: add support for interface_in and interface_out

The UFW module has support for specifying `direction` and `interface`
for UFW rules.  Rules with these parameters are built such that
per-interface filtering only apply to a single direction based on the
value of `direction`.

Not being able to specify multiple interfaces complicates things for
`routed` rules where one might want to apply filtering only for a
specific combination of `in` and `out` interfaces.

This commit introduces two new parameters to the UFW module:
`interface_in` and `interface_out`.  These rules are mutually exclusive
with the old `direction` and `interface` parameter because of the
ambiguity of having e.g.:

    direction: XXX
    interface: foo
    interface_XXX: bar

Fixes #63903

changelogs/fragments/63903-ufw.yaml

@@ -0,0 +1,2 @@
+minor_changes:
+  - ufw - accept ``interface_in`` and ``interface_out`` as parameters.

lib/ansible/modules/system/ufw.py

@@ -46,7 +46,8 @@ options:
     aliases: [ policy ]
   direction:
     description:
-      - Select direction for a rule or default policy command.
+      - Select direction for a rule or default policy command.  Mutually
+        exclusive with I(interface_in) and I(interface_out).
     type: str
     choices: [ in, incoming, out, outgoing, routed ]
   logging:
@@ -127,9 +128,29 @@ options:
     type: bool
   interface:
     description:
-      - Specify interface for rule.
+      - Specify interface for the rule.  The direction (in or out) used
+        for the interface depends on the value of I(direction).  See
+        I(interface_in) and I(interface_out) for routed rules that needs
+        to supply both an input and output interface.  Mutually
+        exclusive with I(interface_in) and I(interface_out).
     type: str
     aliases: [ if ]
+  interface_in:
+    description:
+      - Specify input interface for the rule.  This is mutually
+        exclusive with I(direction) and I(interface).  However, it is
+        compatible with I(interface_out) for routed rules.
+    type: str
+    aliases: [ if_in ]
+    version_added: "2.10"
+  interface_out:
+    description:
+      - Specify output interface for the rule.  This is mutually
+        exclusive with I(direction) and I(interface).  However, it is
+        compatible with I(interface_in) for routed rules.
+    type: str
+    aliases: [ if_out ]
+    version_added: "2.10"
   route:
     description:
       - Apply the rule to routed/forwarded packets.
@@ -315,6 +336,8 @@ def main():
             insert_relative_to=dict(choices=['zero', 'first-ipv4', 'last-ipv4', 'first-ipv6', 'last-ipv6'], default='zero'),
             rule=dict(type='str', choices=['allow', 'deny', 'limit', 'reject']),
             interface=dict(type='str', aliases=['if']),
+            interface_in=dict(type='str', aliases=['if_in']),
+            interface_out=dict(type='str', aliases=['if_out']),
             log=dict(type='bool', default=False),
             from_ip=dict(type='str', default='any', aliases=['from', 'src']),
             from_port=dict(type='str'),
@@ -327,6 +350,9 @@ def main():
         supports_check_mode=True,
         mutually_exclusive=[
             ['name', 'proto', 'logging'],
+            # Mutual exclusivity with `interface` implied by `required_by`.
+            ['direction', 'interface_in'],
+            ['direction', 'interface_out'],
         ],
         required_one_of=([command_keys]),
         required_by=dict(
@@ -484,6 +510,9 @@ def main():
         elif command == 'rule':
             if params['direction'] not in ['in', 'out', None]:
                 module.fail_json(msg='For rules, direction must be one of "in" and "out", or direction must not be specified.')
+            if not params['route'] and params['interface_in'] and params['interface_out']:
+                module.fail_json(msg='Only route rules can combine '
+                                 'interface_in and interface_out')
             # Rules are constructed according to the long format
             #
             # ufw [--dry-run] [route] [delete] [insert NUM] allow|deny|reject|limit [in|out on INTERFACE] [log|log-all] \
@@ -520,6 +549,8 @@ def main():
             cmd.append([value])
             cmd.append([params['direction'], "%s" % params['direction']])
             cmd.append([params['interface'], "on %s" % params['interface']])
+            cmd.append([params['interface_in'], "in on %s" % params['interface_in']])
+            cmd.append([params['interface_out'], "out on %s" % params['interface_out']])
             cmd.append([module.boolean(params['log']), 'log'])
 
             for (key, template) in [('from_ip', "from %s"), ('from_port', "port %s"),

test/integration/targets/ufw/tasks/main.yml

@@ -7,8 +7,10 @@
 - name: Install iptables (SuSE only)
   package:
     name: iptables
+  become: yes
   when: ansible_os_family == 'Suse'
 - name: Install ufw
+  become: yes
   package:
     name: ufw
 
@@ -17,6 +19,7 @@
   - include_tasks: run-test.yml
     with_fileglob:
     - "tests/*.yml"
+  become: yes
 
   # Cleanup
   always:

test/integration/targets/ufw/tasks/tests/interface.yml

@@ -0,0 +1,81 @@
+- name: Enable
+  ufw:
+    state: enabled
+
+- name: Route with interface in and out
+  ufw:
+    rule: allow
+    route: yes
+    interface_in: foo
+    interface_out: bar
+    proto: tcp
+    from_ip: 1.1.1.1
+    to_ip: 8.8.8.8
+    from_port: 1111
+    to_port: 2222
+
+- name: Route with interface in
+  ufw:
+    rule: allow
+    route: yes
+    interface_in: foo
+    proto: tcp
+    from_ip: 1.1.1.1
+    from_port: 1111
+
+- name: Route with interface out
+  ufw:
+    rule: allow
+    route: yes
+    interface_out: bar
+    proto: tcp
+    from_ip: 1.1.1.1
+    from_port: 1111
+
+- name: Non-route with interface in
+  ufw:
+    rule: allow
+    interface_in: foo
+    proto: tcp
+    from_ip: 1.1.1.1
+    from_port: 3333
+
+- name: Non-route with interface out
+  ufw:
+    rule: allow
+    interface_out: bar
+    proto: tcp
+    from_ip: 1.1.1.1
+    from_port: 4444
+
+- name: Check result
+  shell: ufw status |grep -E '(ALLOW|DENY|REJECT|LIMIT)' |sed -E 's/[ \t]+/ /g'
+  register: ufw_status
+
+- assert:
+    that:
+      - '"8.8.8.8 2222/tcp on bar ALLOW FWD 1.1.1.1 1111/tcp on foo " in stdout'
+      - '"Anywhere ALLOW FWD 1.1.1.1 1111/tcp on foo " in stdout'
+      - '"Anywhere on bar ALLOW FWD 1.1.1.1 1111/tcp " in stdout'
+      - '"Anywhere on foo ALLOW 1.1.1.1 3333/tcp " in stdout'
+      - '"Anywhere ALLOW OUT 1.1.1.1 4444/tcp on bar " in stdout'
+  vars:
+    stdout: '{{ ufw_status.stdout_lines }}'
+
+- name: Non-route with interface_in and interface_out
+  ufw:
+    rule: allow
+    interface_in: foo
+    interface_out: bar
+    proto: tcp
+    from_ip: 1.1.1.1
+    from_port: 1111
+    to_ip: 8.8.8.8
+    to_port: 2222
+  ignore_errors: yes
+  register: ufw_non_route_iface
+
+- assert:
+    that:
+      - ufw_non_route_iface is failed
+      - '"Only route rules" in ufw_non_route_iface.msg'

test/units/modules/system/test_ufw.py

@@ -52,6 +52,11 @@ dry_mode_cmd_with_port_700 = {
     "ufw --dry-run allow from any to any port 7000 proto tcp": skippg_adding_existing_rules,
     "ufw --dry-run delete allow from any to any port 7000 proto tcp": "",
     "ufw --dry-run delete allow from any to any port 7001 proto tcp": user_rules_with_port_7000,
+    "ufw --dry-run route allow in on foo out on bar from 1.1.1.1 port 7000 to 8.8.8.8 port 7001 proto tcp": "",
+    "ufw --dry-run allow in on foo from any to any port 7003 proto tcp": "",
+    "ufw --dry-run allow in on foo from 1.1.1.1 port 7002 to 8.8.8.8 port 7003 proto tcp": "",
+    "ufw --dry-run allow out on foo from any to any port 7004 proto tcp": "",
+    "ufw --dry-run allow out on foo from 1.1.1.1 port 7003 to 8.8.8.8 port 7004 proto tcp": "",
     grep_config_cli: user_rules_with_port_7000
 }
 
@@ -169,6 +174,134 @@ class TestUFW(unittest.TestCase):
         result = self.__getResult(do_nothing_func_port_7000)
         self.assertFalse(result.exception.args[0]['changed'])
 
+    def test_check_mode_add_detailed_route(self):
+        set_module_args({
+            'rule': 'allow',
+            'route': 'yes',
+            'interface_in': 'foo',
+            'interface_out': 'bar',
+            'proto': 'tcp',
+            'from_ip': '1.1.1.1',
+            'to_ip': '8.8.8.8',
+            'from_port': '7000',
+            'to_port': '7001',
+            '_ansible_check_mode': True
+        })
+
+        result = self.__getResult(do_nothing_func_port_7000)
+        self.assertTrue(result.exception.args[0]['changed'])
+
+    def test_check_mode_add_ambiguous_route(self):
+        set_module_args({
+            'rule': 'allow',
+            'route': 'yes',
+            'interface_in': 'foo',
+            'interface_out': 'bar',
+            'direction': 'in',
+            'interface': 'baz',
+            '_ansible_check_mode': True
+        })
+
+        with self.assertRaises(AnsibleFailJson) as result:
+            self.__getResult(do_nothing_func_port_7000)
+
+        exc = result.exception.args[0]
+        self.assertTrue(exc['failed'])
+        self.assertIn('mutually exclusive', exc['msg'])
+
+    def test_check_mode_add_interface_in(self):
+        set_module_args({
+            'rule': 'allow',
+            'proto': 'tcp',
+            'port': '7003',
+            'interface_in': 'foo',
+            '_ansible_check_mode': True
+        })
+        result = self.__getResult(do_nothing_func_port_7000)
+        self.assertTrue(result.exception.args[0]['changed'])
+
+    def test_check_mode_add_interface_out(self):
+        set_module_args({
+            'rule': 'allow',
+            'proto': 'tcp',
+            'port': '7004',
+            'interface_out': 'foo',
+            '_ansible_check_mode': True
+        })
+        result = self.__getResult(do_nothing_func_port_7000)
+        self.assertTrue(result.exception.args[0]['changed'])
+
+    def test_check_mode_add_non_route_interface_both(self):
+        set_module_args({
+            'rule': 'allow',
+            'proto': 'tcp',
+            'port': '7004',
+            'interface_in': 'foo',
+            'interface_out': 'bar',
+            '_ansible_check_mode': True
+        })
+
+        with self.assertRaises(AnsibleFailJson) as result:
+            self.__getResult(do_nothing_func_port_7000)
+
+        exc = result.exception.args[0]
+        self.assertTrue(exc['failed'])
+        self.assertIn('combine', exc['msg'])
+
+    def test_check_mode_add_direction_in(self):
+        set_module_args({
+            'rule': 'allow',
+            'proto': 'tcp',
+            'port': '7003',
+            'direction': 'in',
+            'interface': 'foo',
+            '_ansible_check_mode': True
+        })
+        result = self.__getResult(do_nothing_func_port_7000)
+        self.assertTrue(result.exception.args[0]['changed'])
+
+    def test_check_mode_add_direction_in_with_ip(self):
+        set_module_args({
+            'rule': 'allow',
+            'proto': 'tcp',
+            'from_ip': '1.1.1.1',
+            'from_port': '7002',
+            'to_ip': '8.8.8.8',
+            'to_port': '7003',
+            'direction': 'in',
+            'interface': 'foo',
+            '_ansible_check_mode': True
+        })
+        result = self.__getResult(do_nothing_func_port_7000)
+        self.assertTrue(result.exception.args[0]['changed'])
+
+    def test_check_mode_add_direction_out(self):
+        set_module_args({
+            'rule': 'allow',
+            'proto': 'tcp',
+            'port': '7004',
+            'direction': 'out',
+            'interface': 'foo',
+            '_ansible_check_mode': True
+        })
+        result = self.__getResult(do_nothing_func_port_7000)
+        self.assertTrue(result.exception.args[0]['changed'])
+
+    def test_check_mode_add_direction_out_with_ip(self):
+        set_module_args({
+            'rule': 'allow',
+            'proto': 'tcp',
+            'from_ip': '1.1.1.1',
+            'from_port': '7003',
+            'to_ip': '8.8.8.8',
+            'to_port': '7004',
+            'direction': 'out',
+            'interface': 'foo',
+            '_ansible_check_mode': True
+        })
+        result = self.__getResult(do_nothing_func_port_7000)
+        self.assertTrue(result.exception.args[0]['changed'])
+
     def test_check_mode_delete_existing_rules(self):
 
         set_module_args({