From cd4cec008983943213ac3bb721ab69c3a5214c54 Mon Sep 17 00:00:00 2001
From: Steven Daniele <sdaniele@gatech.edu>
Date: Thu, 26 Mar 2020 08:36:02 -0400
Subject: [PATCH 1/5] feat: allow rich_rules to be specified as a dict

When specifying rich rules as a dictionary ipsets and services can be
specified as lists. They will be expanded out by the jinja template into
individual rich rules for the parent zone.
---
 firewalld/files/zone.xml | 39 +++++++++++++++++++++++++++++++++++++--
 pillar.example           | 15 +++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/firewalld/files/zone.xml b/firewalld/files/zone.xml
index dabe00a..39bdfd7 100644
--- a/firewalld/files/zone.xml
+++ b/firewalld/files/zone.xml
@@ -83,8 +83,7 @@
   {%- endfor %}
 {%- endif %}
 
-{%- if 'rich_rules' in zone %}
-  {%- for rule in zone.rich_rules %}
+{%- macro rich_rule(rule) -%}
   {%- if 'family' in rule %}
   <rule family="{{ rule.family }}">
   {%- else %}
@@ -149,6 +148,42 @@
     <drop/>
   {%- endif %}    
   </rule>
+{%- endmacro %}
+
+{%- if 'rich_rules' in zone %}
+  {%- if zone.rich_rules is list %}
+    {%- set rich_rules = zone.rich_rules %}
+  {%- else %}
+    {%- set expanded_ipset_rules = [] %}
+    {%- for name,rule in zone.rich_rules|dictsort %}
+      {%- if 'ipsets' in rule %}
+        {%- for ipset in rule.ipsets %}
+          {%- set tmp_rule = {} %}
+          {%- set _dummy = tmp_rule.update(rule) %}
+          {%- set _dummy = tmp_rule.update({'ipset':{'name':ipset}}) %}
+          {%- set _dummy = expanded_ipset_rules.append(tmp_rule) %}
+        {%- endfor %}
+      {%- else %}
+        {%- set _dummy = expanded_ipset_rules.append(rule) %}
+      {%- endif %}
+    {%- endfor %}
+    {%- set rich_rules = [] %}
+    {%- for rule in expanded_ipset_rules %}
+      {%- if 'services' in rule %}
+        {%- for service in rule.services %}
+          {%- set tmp_rule = {} %}
+          {%- set _dummy = tmp_rule.update(rule) %}
+          {%- set _dummy = tmp_rule.update({'service':service}) %}
+          {%- set _dummy = rich_rules.append(tmp_rule) %}
+        {%- endfor %}
+      {%- else %}
+        {%- set _dummy = rich_rules.append(rule) %}
+      {%- endif %}
+    {%- endfor %}
+  {%- endif %}
+  {%- for rule in rich_rules %}
+{{ rich_rule(rule) }}
   {%- endfor %}
 {%- endif %}
+
 </zone>
diff --git a/pillar.example b/pillar.example
index bdb1854..3f64319 100644
--- a/pillar.example
+++ b/pillar.example
@@ -151,6 +151,21 @@ firewalld:
           port: 4444
           protocol: tcp
 
+    rich_public:
+      short: rich_public
+      description: "Example"
+      # Rich rules can be specified as a dictionary. All keys from standard rich rules
+      # can be used. Special keys "ipsets" and "services", if defined, take precedence.
+      # They will be auto-expanded into separate rich rules per value in the list.
+      rich_rules:
+        ssh-csg:
+          accept: true
+          ipsets:
+            - fail2ban-ssh
+            - other-ipset
+          services:
+            - ssh
+
   direct:
     chain:
       MYCHAIN:

From 6ebfc6f20cfd72c2785514ab35484c9575401648 Mon Sep 17 00:00:00 2001
From: Imran Iqbal <iqbalmy@hotmail.com>
Date: Wed, 1 Apr 2020 17:52:09 +0100
Subject: [PATCH 2/5] test(zones_spec): check content of rendered zone files

---
 .../default/controls/zones_spec.rb            | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 test/integration/default/controls/zones_spec.rb

diff --git a/test/integration/default/controls/zones_spec.rb b/test/integration/default/controls/zones_spec.rb
new file mode 100644
index 0000000..82382ba
--- /dev/null
+++ b/test/integration/default/controls/zones_spec.rb
@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+control 'zones/public.xml configuration' do
+  title 'should match desired lines'
+
+  describe file('/etc/firewalld/zones/public.xml') do
+    it { should be_file }
+    it { should be_owned_by 'root' }
+    it { should be_grouped_into 'root' }
+    its('mode') { should cmp '0644' }
+    its('content') do
+      should include <<~ZONE_XML
+        <zone>
+          <short>Public</short>
+          <description>For use in public areas. You do not trust the other computers on networks to not harm your computer. Only selected incoming connections are accepted.</description>
+          <service name="zabbixcustom" />
+          <service name="http" />
+          <service name="https" />
+          <service name="ssh" />
+          <service name="salt-minion" />
+          <!-- zabbix-agent -->
+          <port port="10050" protocol="tcp" />
+          <!-- bacula-client -->
+          <port port="9102" protocol="tcp" />
+          <!-- vsftpd -->
+          <port port="21" protocol="tcp" />
+          <protocol value="igmp" />
+          <!-- something -->
+          <source-port port="2222" protocol="tcp" />
+          <!-- something_else -->
+          <source-port port="4444" protocol="tcp" />
+          <rule family="ipv4">
+            <source address="8.8.8.8/24" />
+            <accept/>
+          </rule>
+          <rule family="ipv4">
+            <source ipset="fail2ban-ssh" />
+            <reject type="icmp-port-unreachable" />
+          </rule>
+        </zone>
+      ZONE_XML
+    end
+  end
+end
+
+control 'zones/rich_public.xml configuration' do
+  title 'should match desired lines'
+
+  describe file('/etc/firewalld/zones/rich_public.xml') do
+    it { should be_file }
+    it { should be_owned_by 'root' }
+    it { should be_grouped_into 'root' }
+    its('mode') { should cmp '0644' }
+    its('content') do
+      should include <<~ZONE_XML
+        <zone>
+          <short>rich_public</short>
+          <description>Example</description>
+          <rule>
+            <source ipset="fail2ban-ssh" />
+            <service name="ssh" />
+            <accept/>
+          </rule>
+          <rule>
+            <source ipset="other-ipset" />
+            <service name="ssh" />
+            <accept/>
+          </rule>
+        </zone>
+      ZONE_XML
+    end
+  end
+end

From d8f0f47a5408bde763050c457269ef129a48b050 Mon Sep 17 00:00:00 2001
From: Imran Iqbal <iqbalmy@hotmail.com>
Date: Wed, 1 Apr 2020 18:00:58 +0100
Subject: [PATCH 3/5] fix(zone.xml): adjust whitespacing to pass tests & macro
 at top of file

---
 firewalld/files/zone.xml | 137 +++++++++++++++++++--------------------
 1 file changed, 67 insertions(+), 70 deletions(-)

diff --git a/firewalld/files/zone.xml b/firewalld/files/zone.xml
index 39bdfd7..61d7c7b 100644
--- a/firewalld/files/zone.xml
+++ b/firewalld/files/zone.xml
@@ -4,6 +4,72 @@
   Do not edit this file manually, it will be overwritten!
   Modify the salt pillar for firewalld instead
 -->
+{%- macro rich_rule(rule) -%}
+  {%- if 'family' in rule %}
+  <rule family="{{ rule.family }}">
+  {%- else %}
+  <rule>
+  {%- endif %}    
+  {%- if 'ipset' in rule %}
+    <source ipset="{{ rule.ipset.name }}" />
+  {%- endif %}
+  {%- if 'source' in rule %}
+    <source address="{{ rule.source.address }}" {%- if 'invert' in rule.source %}invert="{{ rule.source.invert }}"{%- endif %} />
+  {%- endif %}    
+  {%- if 'destination' in rule %}
+    <destination address="{{ rule.destination.address }}" {%- if 'invert' in rule.destination %}invert="{{ rule.destination.invert }}"{%- endif %} />
+  {%- endif %}    
+  {%- if 'service' in rule %}
+    <service name="{{ rule.service }}" />
+  {%- endif %}    
+  {%- if 'port' in rule %}
+    <port port="{{ rule.port.portid }}" protocol="{{ rule.port.protocol }}" />
+  {%- endif %}    
+  {%- if 'protocol' in rule %}
+    <protocol value="{{ rule.protocol }}" />
+  {%- endif %}    
+  {%- if 'icmp_block' in rule %}
+    <icmp-block name="{{ rule.icmp_block }}" />
+  {%- endif %}    
+  {%- if 'icmp_type' in rule %}
+    <icmp-type name="{{ rule.icmp_type }}" />
+  {%- endif %}    
+  {%- if 'masquerade' in rule %}
+    {%- if rule.masquerade %}<masquerade/>{%- endif %}
+  {%- endif %}    
+  {%- if 'forward_port' in rule %}
+    {%- if 'comment' in rule.forward_port %}
+      <!-- {{ rule.forward_port.comment }} -->
+    {%- endif %}
+    <forward-port port="{{ rule.forward_port.portid }}" protocol="{{ rule.forward_port.protocol }}"{%- if 'to_port' in rule.forward_port %} to-port="{{ rule.forward_port.to_port }}"{%- endif %}{%- if 'to_addr' in rule.forward_port %} to-addr="{{ rule.forward_port.to_addr }}"{%- endif %} />
+  {%- endif %}    
+  {%- if 'source_port' in rule %}
+    {%- if 'comment' in rule.source_port %}
+      <!-- {{ rule.source_port.comment }} -->
+    {%- endif %}
+    <source-port port="{{ rule.source_port.portid }}" protocol="{{ rule.source_port.protocol }}"{%- if 'to_port' in rule.source_port %} to-port="{{ rule.source_port.to_port }}"{%- endif %}{%- if 'to_addr' in rule.source_port %} to-addr="{{ rule.source_port.to_addr }}"{%- endif %} />
+  {%- endif %}    
+  {%- if 'log' in rule %}
+    <log{%- if 'prefix' in rule.log %} prefix="{{ rule.log.prefix }}"{%- endif %}{%- if 'level' in rule.log %} level="{{ rule.log.level }}"{%- endif %}>
+    {%- if 'limit' in rule.log %}
+      <limit value="{{ rule.log.limit }}"/>
+    {%- endif %}
+    </log>
+  {%- endif %}    
+  {%- if 'audit' in rule %}
+    <audit>{%- if 'limit' in rule.audit %} <limit value="{{ rule.audit.limit }}"/>{%- endif %}</audit>
+  {%- endif %}    
+  {%- if 'accept' in rule %}
+    <accept/>
+  {%- endif %}
+  {%- if 'reject' in rule %}
+    <reject{%- if 'type' in rule.reject %} type="{{ rule.reject.type }}"{%- endif %} />
+  {%- endif %}    
+  {%- if 'drop' in rule %}
+    <drop/>
+  {%- endif %}    
+  </rule>
+{%- endmacro %}
 <zone{%- if 'target' in zone %} target="{{ zone.target }}"{%- endif %}>
   {% if 'short' in zone %}<short>{{ zone.short }}</short>{% else %}<short>{{ name }}</short>{% endif %}
   {% if 'description' in zone %}<description>{{ zone.description }}</description>{% endif %}
@@ -82,74 +148,6 @@
   <source-port port="{{ v.port }}" protocol="{{ v.protocol }}" />
   {%- endfor %}
 {%- endif %}
-
-{%- macro rich_rule(rule) -%}
-  {%- if 'family' in rule %}
-  <rule family="{{ rule.family }}">
-  {%- else %}
-  <rule>
-  {%- endif %}    
-  {%- if 'ipset' in rule %}
-    <source ipset="{{ rule.ipset.name }}" />
-  {%- endif %}
-  {%- if 'source' in rule %}
-    <source address="{{ rule.source.address }}" {%- if 'invert' in rule.source %}invert="{{ rule.source.invert }}"{%- endif %} />
-  {%- endif %}    
-  {%- if 'destination' in rule %}
-    <destination address="{{ rule.destination.address }}" {%- if 'invert' in rule.destination %}invert="{{ rule.destination.invert }}"{%- endif %} />
-  {%- endif %}    
-  {%- if 'service' in rule %}
-    <service name="{{ rule.service }}" />
-  {%- endif %}    
-  {%- if 'port' in rule %}
-    <port port="{{ rule.port.portid }}" protocol="{{ rule.port.protocol }}" />
-  {%- endif %}    
-  {%- if 'protocol' in rule %}
-    <protocol value="{{ rule.protocol }}" />
-  {%- endif %}    
-  {%- if 'icmp_block' in rule %}
-    <icmp-block name="{{ rule.icmp_block }}" />
-  {%- endif %}    
-  {%- if 'icmp_type' in rule %}
-    <icmp-type name="{{ rule.icmp_type }}" />
-  {%- endif %}    
-  {%- if 'masquerade' in rule %}
-    {%- if rule.masquerade %}<masquerade/>{%- endif %}
-  {%- endif %}    
-  {%- if 'forward_port' in rule %}
-    {%- if 'comment' in rule.forward_port %}
-      <!-- {{ rule.forward_port.comment }} -->
-    {%- endif %}
-    <forward-port port="{{ rule.forward_port.portid }}" protocol="{{ rule.forward_port.protocol }}"{%- if 'to_port' in rule.forward_port %} to-port="{{ rule.forward_port.to_port }}"{%- endif %}{%- if 'to_addr' in rule.forward_port %} to-addr="{{ rule.forward_port.to_addr }}"{%- endif %} />
-  {%- endif %}    
-  {%- if 'source_port' in rule %}
-    {%- if 'comment' in rule.source_port %}
-      <!-- {{ rule.source_port.comment }} -->
-    {%- endif %}
-    <source-port port="{{ rule.source_port.portid }}" protocol="{{ rule.source_port.protocol }}"{%- if 'to_port' in rule.source_port %} to-port="{{ rule.source_port.to_port }}"{%- endif %}{%- if 'to_addr' in rule.source_port %} to-addr="{{ rule.source_port.to_addr }}"{%- endif %} />
-  {%- endif %}    
-  {%- if 'log' in rule %}
-    <log{%- if 'prefix' in rule.log %} prefix="{{ rule.log.prefix }}"{%- endif %}{%- if 'level' in rule.log %} level="{{ rule.log.level }}"{%- endif %}>
-    {%- if 'limit' in rule.log %}
-      <limit value="{{ rule.log.limit }}"/>
-    {%- endif %}
-    </log>
-  {%- endif %}    
-  {%- if 'audit' in rule %}
-    <audit>{%- if 'limit' in rule.audit %} <limit value="{{ rule.audit.limit }}"/>{%- endif %}</audit>
-  {%- endif %}    
-  {%- if 'accept' in rule %}
-    <accept/>
-  {%- endif %}
-  {%- if 'reject' in rule %}
-    <reject{%- if 'type' in rule.reject %} type="{{ rule.reject.type }}"{%- endif %} />
-  {%- endif %}    
-  {%- if 'drop' in rule %}
-    <drop/>
-  {%- endif %}    
-  </rule>
-{%- endmacro %}
-
 {%- if 'rich_rules' in zone %}
   {%- if zone.rich_rules is list %}
     {%- set rich_rules = zone.rich_rules %}
@@ -182,8 +180,7 @@
     {%- endfor %}
   {%- endif %}
   {%- for rule in rich_rules %}
-{{ rich_rule(rule) }}
+{{- rich_rule(rule) }}
   {%- endfor %}
 {%- endif %}
-
 </zone>

From 204efe5fc7065a2c2f4f55aa0138bf98675cba4e Mon Sep 17 00:00:00 2001
From: Imran Iqbal <iqbalmy@hotmail.com>
Date: Wed, 1 Apr 2020 18:41:42 +0100
Subject: [PATCH 4/5] style(zone.xml): remove all trailing whitespaces

---
 firewalld/files/zone.xml | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/firewalld/files/zone.xml b/firewalld/files/zone.xml
index 61d7c7b..c730b1c 100644
--- a/firewalld/files/zone.xml
+++ b/firewalld/files/zone.xml
@@ -9,65 +9,65 @@
   <rule family="{{ rule.family }}">
   {%- else %}
   <rule>
-  {%- endif %}    
+  {%- endif %}
   {%- if 'ipset' in rule %}
     <source ipset="{{ rule.ipset.name }}" />
   {%- endif %}
   {%- if 'source' in rule %}
     <source address="{{ rule.source.address }}" {%- if 'invert' in rule.source %}invert="{{ rule.source.invert }}"{%- endif %} />
-  {%- endif %}    
+  {%- endif %}
   {%- if 'destination' in rule %}
     <destination address="{{ rule.destination.address }}" {%- if 'invert' in rule.destination %}invert="{{ rule.destination.invert }}"{%- endif %} />
-  {%- endif %}    
+  {%- endif %}
   {%- if 'service' in rule %}
     <service name="{{ rule.service }}" />
-  {%- endif %}    
+  {%- endif %}
   {%- if 'port' in rule %}
     <port port="{{ rule.port.portid }}" protocol="{{ rule.port.protocol }}" />
-  {%- endif %}    
+  {%- endif %}
   {%- if 'protocol' in rule %}
     <protocol value="{{ rule.protocol }}" />
-  {%- endif %}    
+  {%- endif %}
   {%- if 'icmp_block' in rule %}
     <icmp-block name="{{ rule.icmp_block }}" />
-  {%- endif %}    
+  {%- endif %}
   {%- if 'icmp_type' in rule %}
     <icmp-type name="{{ rule.icmp_type }}" />
-  {%- endif %}    
+  {%- endif %}
   {%- if 'masquerade' in rule %}
     {%- if rule.masquerade %}<masquerade/>{%- endif %}
-  {%- endif %}    
+  {%- endif %}
   {%- if 'forward_port' in rule %}
     {%- if 'comment' in rule.forward_port %}
       <!-- {{ rule.forward_port.comment }} -->
     {%- endif %}
     <forward-port port="{{ rule.forward_port.portid }}" protocol="{{ rule.forward_port.protocol }}"{%- if 'to_port' in rule.forward_port %} to-port="{{ rule.forward_port.to_port }}"{%- endif %}{%- if 'to_addr' in rule.forward_port %} to-addr="{{ rule.forward_port.to_addr }}"{%- endif %} />
-  {%- endif %}    
+  {%- endif %}
   {%- if 'source_port' in rule %}
     {%- if 'comment' in rule.source_port %}
       <!-- {{ rule.source_port.comment }} -->
     {%- endif %}
     <source-port port="{{ rule.source_port.portid }}" protocol="{{ rule.source_port.protocol }}"{%- if 'to_port' in rule.source_port %} to-port="{{ rule.source_port.to_port }}"{%- endif %}{%- if 'to_addr' in rule.source_port %} to-addr="{{ rule.source_port.to_addr }}"{%- endif %} />
-  {%- endif %}    
+  {%- endif %}
   {%- if 'log' in rule %}
     <log{%- if 'prefix' in rule.log %} prefix="{{ rule.log.prefix }}"{%- endif %}{%- if 'level' in rule.log %} level="{{ rule.log.level }}"{%- endif %}>
     {%- if 'limit' in rule.log %}
       <limit value="{{ rule.log.limit }}"/>
     {%- endif %}
     </log>
-  {%- endif %}    
+  {%- endif %}
   {%- if 'audit' in rule %}
     <audit>{%- if 'limit' in rule.audit %} <limit value="{{ rule.audit.limit }}"/>{%- endif %}</audit>
-  {%- endif %}    
+  {%- endif %}
   {%- if 'accept' in rule %}
     <accept/>
   {%- endif %}
   {%- if 'reject' in rule %}
     <reject{%- if 'type' in rule.reject %} type="{{ rule.reject.type }}"{%- endif %} />
-  {%- endif %}    
+  {%- endif %}
   {%- if 'drop' in rule %}
     <drop/>
-  {%- endif %}    
+  {%- endif %}
   </rule>
 {%- endmacro %}
 <zone{%- if 'target' in zone %} target="{{ zone.target }}"{%- endif %}>

From c69fd6bf047cf426a94447e06450ca7f39594e23 Mon Sep 17 00:00:00 2001
From: Imran Iqbal <iqbalmy@hotmail.com>
Date: Wed, 1 Apr 2020 18:43:56 +0100
Subject: [PATCH 5/5] chore(rubocop): relax `Metrics/BlockLength` for new tests

* Checked using https://github.com/myii/ssf-formula/pull/145
---
 .rubocop.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.rubocop.yml b/.rubocop.yml
index 96fd6e5..fa4b05d 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -8,3 +8,5 @@ Layout/LineLength:
   Max: 88
 
 # Any offenses that should be fixed, e.g. collected via. `rubocop --auto-gen-config`
+Metrics/BlockLength:
+  Max: 39