From c9d3bb59a431f75d8dafa11c4a6acc968a3c3e80 Mon Sep 17 00:00:00 2001
From: Sam Doran <sdoran@redhat.com>
Date: Thu, 17 May 2018 13:53:51 -0400
Subject: [PATCH] Do not join flag parameters in iptables module (#36658)

* Do not join flag parameters

This put a comma between every character of the tcp flag parameters, resulting in a bad iptables command.

Fixes #36490

* Use suboptions to ensure tcp_flags options are lists

* Add unit tests for tcp_flags

* Add example of how to use tcp_flags
---
 lib/ansible/modules/system/iptables.py     | 30 +++++++++--
 test/units/modules/system/test_iptables.py | 60 +++++++++++++++++++++-
 2 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/lib/ansible/modules/system/iptables.py b/lib/ansible/modules/system/iptables.py
index 73b3fdc0ec..e645bea36e 100644
--- a/lib/ansible/modules/system/iptables.py
+++ b/lib/ansible/modules/system/iptables.py
@@ -106,11 +106,15 @@ options:
     description:
       - TCP flags specification.
       - C(tcp_flags) expects a dict with the two keys C(flags) and C(flags_set).
-        - The C(flags) list is the mask, a list of flags you want to examine.
-        - The C(flags_set) list tells which one(s) should be set.
-        If one of the two values is missing, the --tcp-flags option will be ignored.
     default: {}
     version_added: "2.4"
+    suboptions:
+        flags:
+            description:
+                - List of flags you want to examine.
+        flags_set:
+            description:
+                - Flags to be set.
   match:
     description:
       - Specifies a match to use, that is, an extension module that tests for
@@ -342,6 +346,19 @@ EXAMPLES = '''
     protocol: tcp
     reject_with: tcp-reset
     ip_version: ipv4
+
+# Set tcp flags
+- iptables:
+    chain: OUTPUT
+    jump: DROP
+    protocol: tcp
+    tcp_flags:
+      flags: ALL
+      flags_set:
+        - ACK
+        - RST
+        - SYN
+        - FIN
 '''
 
 import re
@@ -521,7 +538,11 @@ def main():
             destination=dict(type='str'),
             to_destination=dict(type='str'),
             match=dict(type='list', default=[]),
-            tcp_flags=dict(type='dict', default={}),
+            tcp_flags=dict(type='dict',
+                           options=dict(
+                                flags=dict(type='list'),
+                                flags_set=dict(type='list'))
+                           ),
             jump=dict(type='str'),
             log_prefix=dict(type='str'),
             goto=dict(type='str'),
@@ -608,5 +629,6 @@ def main():
 
     module.exit_json(**args)
 
+
 if __name__ == '__main__':
     main()
diff --git a/test/units/modules/system/test_iptables.py b/test/units/modules/system/test_iptables.py
index f573a60ff9..b9b94ef3d1 100644
--- a/test/units/modules/system/test_iptables.py
+++ b/test/units/modules/system/test_iptables.py
@@ -1,4 +1,3 @@
-from ansible.compat.tests import unittest
 from ansible.compat.tests.mock import patch
 from ansible.module_utils import basic
 from ansible.modules.system import iptables
@@ -578,3 +577,62 @@ class TestIptables(ModuleTestCase):
             '--reject-with',
             'tcp-reset',
         ])
+
+    def test_tcp_flags(self):
+        """ Test various ways of inputting tcp_flags """
+        args = [
+            {
+                'chain': 'OUTPUT',
+                'protocol': 'tcp',
+                'jump': 'DROP',
+                'tcp_flags': 'flags=ALL flags_set="ACK,RST,SYN,FIN"'
+            },
+            {
+                'chain': 'OUTPUT',
+                'protocol': 'tcp',
+                'jump': 'DROP',
+                'tcp_flags': {
+                    'flags': 'ALL',
+                    'flags_set': 'ACK,RST,SYN,FIN'
+                }
+            },
+            {
+                'chain': 'OUTPUT',
+                'protocol': 'tcp',
+                'jump': 'DROP',
+                'tcp_flags': {
+                    'flags': ['ALL'],
+                    'flags_set': ['ACK', 'RST', 'SYN', 'FIN']
+                }
+            },
+
+        ]
+
+        for item in args:
+            set_module_args(item)
+
+            commands_results = [
+                (0, '', ''),
+            ]
+
+            with patch.object(basic.AnsibleModule, 'run_command') as run_command:
+                run_command.side_effect = commands_results
+                with self.assertRaises(AnsibleExitJson) as result:
+                    iptables.main()
+                    self.assertTrue(result.exception.args[0]['changed'])
+
+            self.assertEqual(run_command.call_count, 1)
+            self.assertEqual(run_command.call_args_list[0][0][0], [
+                '/sbin/iptables',
+                '-t',
+                'filter',
+                '-C',
+                'OUTPUT',
+                '-p',
+                'tcp',
+                '--tcp-flags',
+                'ALL',
+                'ACK,RST,SYN,FIN',
+                '-j',
+                'DROP'
+            ])