From 00b28f0aedf6edac6c0528624604ae72fdc457c8 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dan.schaper@pi-hole.net>
Date: Tue, 17 Jan 2017 13:56:34 -0800
Subject: [PATCH 01/17] IPTables Whiptal option to install.

Signed-off-by: Dan Schaper <dan.schaper@pi-hole.net>
---
 automated install/basic-install.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh
index 6448aa0f..df9db556 100755
--- a/automated install/basic-install.sh	
+++ b/automated install/basic-install.sh	
@@ -869,20 +869,26 @@ configureFirewall() {
     echo "::: Configuring FirewallD for httpd and dnsmasq.."
     firewall-cmd --permanent --add-port=80/tcp --add-port=53/tcp --add-port=53/udp
     firewall-cmd --reload
+    return 0
   # Check for proper kernel modules to prevent failure
   elif modinfo ip_tables &> /dev/null && command -v iptables &> /dev/null; then
     # If chain Policy is not ACCEPT or last Rule is not ACCEPT
     # then check and insert our Rules above the DROP/REJECT Rule.
     if iptables -S INPUT | head -n1 | grep -qv '^-P.*ACCEPT$' || iptables -S INPUT | tail -n1 | grep -qv '^-\(A\|P\).*ACCEPT$'; then
+      whiptail --title "Firewall in use" --yesno "We have detected a running firewall\n\nPi-hole currently requires HTTP and DNS port access.\n\n\n\nInstall Pi-hole default firewall rules?" ${r} ${c} || \
+      { echo -e ":::\n::: Not installing firewall rulesets."; return 1; }
+      echo -e ":::\n::: Installing new IPTables firewall rulesets."
       # Check chain first, otherwise a new rule will duplicate old ones
-      echo "::: Configuring iptables for httpd and dnsmasq.."
       iptables -C INPUT -p tcp -m tcp --dport 80 -j ACCEPT &> /dev/null || iptables -I INPUT 1 -p tcp -m tcp --dport 80 -j ACCEPT
       iptables -C INPUT -p tcp -m tcp --dport 53 -j ACCEPT &> /dev/null || iptables -I INPUT 1 -p tcp -m tcp --dport 53 -j ACCEPT
       iptables -C INPUT -p udp -m udp --dport 53 -j ACCEPT &> /dev/null || iptables -I INPUT 1 -p udp -m udp --dport 53 -j ACCEPT
+      return 0
     fi
   else
-    echo "::: No active firewall detected.. skipping firewall configuration."
+    echo -e ":::\n::: No active firewall detected.. skipping firewall configuration."
+    return 0
   fi
+  echo -e ":::\n::: Skipping firewall configuration."
 }
 
 finalExports() {

From 62a5e36afde7bb85dd6f7aebbad92e987ba4e7b2 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dan.schaper@pi-hole.net>
Date: Tue, 17 Jan 2017 14:40:30 -0800
Subject: [PATCH 02/17] Remove firewall configuration from update portion of
 script.

Signed-off-by: Dan Schaper <dan.schaper@pi-hole.net>
---
 automated install/basic-install.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh
index df9db556..e60f2f2d 100755
--- a/automated install/basic-install.sh	
+++ b/automated install/basic-install.sh	
@@ -967,7 +967,6 @@ updatePihole() {
   CreateLogFile
   installPiholeWeb
   installCron
-  configureFirewall
   finalExports #re-export setupVars.conf to account for any new vars added in new versions
   runGravity
 }

From f7a17248b730cd0e37e283e939759f0263867eef Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Mon, 23 Jan 2017 14:19:51 -0800
Subject: [PATCH 03/17] Warn with whiptail if `firewall-cmd` is running.

---
 automated install/basic-install.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh
index e60f2f2d..aaaa43ce 100755
--- a/automated install/basic-install.sh	
+++ b/automated install/basic-install.sh	
@@ -865,8 +865,10 @@ create_pihole_user() {
 
 configureFirewall() {
   # Allow HTTP and DNS traffic
-  if firewall-cmd --state &> /dev/null; then
-    echo "::: Configuring FirewallD for httpd and dnsmasq.."
+  if [[ $(firewall-cmd --state) == "running" ]]; then
+    whiptail --title "Firewall in use" --yesno "We have detected a running firewall\n\nPi-hole currently requires HTTP and DNS port access.\n\n\n\nInstall Pi-hole default firewall rules?" ${r} ${c} || \
+    { echo -e ":::\n::: Not installing firewall rulesets."; return 1; }
+    echo -e ":::\n:::\n Configuring FirewallD for httpd and dnsmasq."
     firewall-cmd --permanent --add-port=80/tcp --add-port=53/tcp --add-port=53/udp
     firewall-cmd --reload
     return 0

From 1317b67657874e7c2080366149670922b9c6fdbf Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Mon, 23 Jan 2017 18:12:26 -0800
Subject: [PATCH 04/17] Attempt at modifying firewall rules for testing.

---
 test/test_automated_install.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index 58aefe91..13df8c61 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -65,8 +65,8 @@ def test_setupVars_saved_to_file(Pihole):
         assert "{}={}".format(k, v) in output
 
 def test_configureFirewall_firewalld_no_errors(Pihole):
-    ''' confirms firewalld rules are applied when appopriate '''
-    mock_command('firewall-cmd', '0', Pihole)
+    ''' confirms firewalld rules are applied when appropriate '''
+    mock_command('firewall-cmd', 'running', '0', Pihole)
     configureFirewall = Pihole.run('''
     source /opt/pihole/basic-install.sh
     configureFirewall
@@ -80,19 +80,21 @@ def test_configureFirewall_firewalld_no_errors(Pihole):
 
 
 # Helper functions
-def mock_command(script, result, container):
+def mock_command(script, result, retVal, container):
     ''' Allows for setup of commands we don't really want to have to run for real in unit tests '''
     ''' TODO: support array of results that enable the results to change over multiple executions of a command '''
+    container.run('''
+    cat <<EOF> {script}\n{content}\nEOF
+    echo {result}
+    chmod +x {script}
+    '''.format(script=full_script_path, content=mock_script, result=result))
     full_script_path = '/usr/local/bin/{}'.format(script)
     mock_script = dedent('''\
     #!/bin/bash -e
     echo "\$0 \$@" >> /var/log/{script}
+
     exit {retcode}
-    '''.format(script=script, retcode=result))
-    container.run('''
-    cat <<EOF> {script}\n{content}\nEOF
-    chmod +x {script}
-    '''.format(script=full_script_path, content=mock_script))
+    '''.format(script=script, retcode=retVal))
 
 def run_script(Pihole, script):
     result = Pihole.run(script)

From 28bafe742732888daf8fc157dacb4c8a750b7bbf Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Mon, 23 Jan 2017 18:14:40 -0800
Subject: [PATCH 05/17] Rename test for firewallD, running state.

---
 test/test_automated_install.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index 13df8c61..e444aa8b 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -64,7 +64,7 @@ def test_setupVars_saved_to_file(Pihole):
     for k,v in SETUPVARS.iteritems():
         assert "{}={}".format(k, v) in output
 
-def test_configureFirewall_firewalld_no_errors(Pihole):
+def test_configureFirewall_firewalld_running_no_errors(Pihole):
     ''' confirms firewalld rules are applied when appropriate '''
     mock_command('firewall-cmd', 'running', '0', Pihole)
     configureFirewall = Pihole.run('''

From 3aeb378b565538c93bf7221ddae99a0913ce4aac Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Mon, 23 Jan 2017 18:21:50 -0800
Subject: [PATCH 06/17] Fix moved incorrect blocks.

---
 test/test_automated_install.py | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index e444aa8b..9c3754f5 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -64,7 +64,7 @@ def test_setupVars_saved_to_file(Pihole):
     for k,v in SETUPVARS.iteritems():
         assert "{}={}".format(k, v) in output
 
-def test_configureFirewall_firewalld_running_no_errors(Pihole):
+def test_configureFirewall_firewalld_no_errors(Pihole):
     ''' confirms firewalld rules are applied when appropriate '''
     mock_command('firewall-cmd', 'running', '0', Pihole)
     configureFirewall = Pihole.run('''
@@ -83,18 +83,17 @@ def test_configureFirewall_firewalld_running_no_errors(Pihole):
 def mock_command(script, result, retVal, container):
     ''' Allows for setup of commands we don't really want to have to run for real in unit tests '''
     ''' TODO: support array of results that enable the results to change over multiple executions of a command '''
-    container.run('''
-    cat <<EOF> {script}\n{content}\nEOF
-    echo {result}
-    chmod +x {script}
-    '''.format(script=full_script_path, content=mock_script, result=result))
     full_script_path = '/usr/local/bin/{}'.format(script)
     mock_script = dedent('''\
     #!/bin/bash -e
     echo "\$0 \$@" >> /var/log/{script}
-
+    echo {result}
     exit {retcode}
-    '''.format(script=script, retcode=retVal))
+    '''.format(script=script, result=result,retcode=retVal))
+    container.run('''
+    cat <<EOF> {script}\n{content}\nEOF
+    chmod +x {script}
+    '''.format(script=full_script_path, content=mock_script))
 
 def run_script(Pihole, script):
     result = Pihole.run(script)

From 968b981ecbebd86838aedd4090a3a4980970bc7a Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Mon, 23 Jan 2017 18:47:52 -0800
Subject: [PATCH 07/17] Try mocking whiptail

---
 test/test_automated_install.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index 9c3754f5..141bedd7 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -67,6 +67,7 @@ def test_setupVars_saved_to_file(Pihole):
 def test_configureFirewall_firewalld_no_errors(Pihole):
     ''' confirms firewalld rules are applied when appropriate '''
     mock_command('firewall-cmd', 'running', '0', Pihole)
+    mock_command('whiptail', '', '0', Pihole)
     configureFirewall = Pihole.run('''
     source /opt/pihole/basic-install.sh
     configureFirewall

From 95796e1978f4e5c4388f21c23f71f6dc39c94b30 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Mon, 23 Jan 2017 18:52:05 -0800
Subject: [PATCH 08/17] Only expect text output, not the leader `:::`

---
 test/test_automated_install.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index 141bedd7..a4478d1c 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -72,7 +72,7 @@ def test_configureFirewall_firewalld_no_errors(Pihole):
     source /opt/pihole/basic-install.sh
     configureFirewall
     ''')
-    expected_stdout = '::: Configuring FirewallD for httpd and dnsmasq.'
+    expected_stdout = 'Configuring FirewallD for httpd and dnsmasq.'
     assert expected_stdout in configureFirewall.stdout
     firewall_calls = Pihole.run('cat /var/log/firewall-cmd').stdout
     assert 'firewall-cmd --state' in firewall_calls

From f1cfb16bf98bd2b6e5a5f52e66c880a918b58f36 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Mon, 23 Jan 2017 18:59:48 -0800
Subject: [PATCH 09/17] Test firewallD enabled and disabled.

---
 test/test_automated_install.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index a4478d1c..a69f449c 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -64,7 +64,7 @@ def test_setupVars_saved_to_file(Pihole):
     for k,v in SETUPVARS.iteritems():
         assert "{}={}".format(k, v) in output
 
-def test_configureFirewall_firewalld_no_errors(Pihole):
+def test_configureFirewall_firewalld_running_no_errors(Pihole):
     ''' confirms firewalld rules are applied when appropriate '''
     mock_command('firewall-cmd', 'running', '0', Pihole)
     mock_command('whiptail', '', '0', Pihole)
@@ -79,6 +79,15 @@ def test_configureFirewall_firewalld_no_errors(Pihole):
     assert 'firewall-cmd --permanent --add-port=80/tcp --add-port=53/tcp --add-port=53/udp' in firewall_calls
     assert 'firewall-cmd --reload' in firewall_calls
 
+def test_configureFirewall_firewalld_disabled_no_errors(Pihole):
+    ''' confirms firewalld rules are applied when appropriate '''
+    mock_command('firewall-cmd', 'stopped', '0', Pihole)
+    configureFirewall = Pihole.run('''
+    source /opt/pihole/basic-install.sh
+    configureFirewall
+    ''')
+    expected_stdout = 'No active firewall detected.. skipping firewall configuration.'
+    assert expected_stdout in configureFirewall.stdout
 
 # Helper functions
 def mock_command(script, result, retVal, container):

From 9c7f7756b4ebe6465f03c37f3855bb0946e0c3d1 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Mon, 23 Jan 2017 19:03:00 -0800
Subject: [PATCH 10/17] Revert second test.

---
 test/test_automated_install.py | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index a69f449c..0a011bc1 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -79,16 +79,6 @@ def test_configureFirewall_firewalld_running_no_errors(Pihole):
     assert 'firewall-cmd --permanent --add-port=80/tcp --add-port=53/tcp --add-port=53/udp' in firewall_calls
     assert 'firewall-cmd --reload' in firewall_calls
 
-def test_configureFirewall_firewalld_disabled_no_errors(Pihole):
-    ''' confirms firewalld rules are applied when appropriate '''
-    mock_command('firewall-cmd', 'stopped', '0', Pihole)
-    configureFirewall = Pihole.run('''
-    source /opt/pihole/basic-install.sh
-    configureFirewall
-    ''')
-    expected_stdout = 'No active firewall detected.. skipping firewall configuration.'
-    assert expected_stdout in configureFirewall.stdout
-
 # Helper functions
 def mock_command(script, result, retVal, container):
     ''' Allows for setup of commands we don't really want to have to run for real in unit tests '''

From 8529c1287fa405912ff4fe0be41b891b94884d42 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Tue, 24 Jan 2017 11:03:37 -0800
Subject: [PATCH 11/17] Full test suite for firewallD configuration.

---
 test/test_automated_install.py | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index 0a011bc1..8ecba458 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -65,8 +65,10 @@ def test_setupVars_saved_to_file(Pihole):
         assert "{}={}".format(k, v) in output
 
 def test_configureFirewall_firewalld_running_no_errors(Pihole):
-    ''' confirms firewalld rules are applied when appropriate '''
+    ''' confirms firewalld rules are applied when firewallD is running '''
+    # firewallD returns 'running' as status
     mock_command('firewall-cmd', 'running', '0', Pihole)
+    # Whiptail dialog returns Ok for user prompt
     mock_command('whiptail', '', '0', Pihole)
     configureFirewall = Pihole.run('''
     source /opt/pihole/basic-install.sh
@@ -79,6 +81,30 @@ def test_configureFirewall_firewalld_running_no_errors(Pihole):
     assert 'firewall-cmd --permanent --add-port=80/tcp --add-port=53/tcp --add-port=53/udp' in firewall_calls
     assert 'firewall-cmd --reload' in firewall_calls
 
+def test_configureFirewall_firewalld_disabled_no_errors(Pihole):
+    ''' confirms firewalld rules are not applied when firewallD is not running '''
+    # firewallD returns non-running status
+    mock_command('firewall-cmd', 'stopped', '0', Pihole)
+    configureFirewall = Pihole.run('''
+    source /opt/pihole/basic-install.sh
+    configureFirewall
+    ''')
+    expected_stdout = 'No active firewall detected.. skipping firewall configuration.'
+    assert expected_stdout in configureFirewall.stdout
+
+def test_configureFirewall_firewalld_enabled_declined_no_errors(Pihole):
+    ''' confirms firewalld rules are not applied when firewallD is running, user declines ruleset '''
+    # firewallD returns running status
+    mock_command('firewall-cmd', 'running', '0', Pihole)
+    # Whiptail dialog returns Cancel for user prompt
+    mock_command('whiptail', '', '1', Pihole)
+    configureFirewall = Pihole.run('''
+    source /opt/pihole/basic-install.sh
+    configureFirewall
+    ''')
+    expected_stdout = 'Not installing firewall rulesets.'
+    assert expected_stdout in configureFirewall.stdout
+
 # Helper functions
 def mock_command(script, result, retVal, container):
     ''' Allows for setup of commands we don't really want to have to run for real in unit tests '''

From e0e88fdb52f0dd87f39575170e586c33c80f0176 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Tue, 24 Jan 2017 11:58:22 -0800
Subject: [PATCH 12/17] Start IPTables test, get baseline for commands in the
 container. See what we have to mock to start testing.

---
 test/test_automated_install.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index 8ecba458..4864e4a2 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -105,6 +105,15 @@ def test_configureFirewall_firewalld_enabled_declined_no_errors(Pihole):
     expected_stdout = 'Not installing firewall rulesets.'
     assert expected_stdout in configureFirewall.stdout
 
+def test_configureFirewall_iptables_baseline(Pihole):
+    ''' confirms IPTables rules are not applied when firewallD is not running '''
+    configureFirewall = Pihole.run('''
+    source /opt/pihole/basic-install.sh
+    configureFirewall
+    ''')
+    expected_stdout = 'Filler Text'
+    assert expected_stdout in configureFirewall.stdout
+
 # Helper functions
 def mock_command(script, result, retVal, container):
     ''' Allows for setup of commands we don't really want to have to run for real in unit tests '''

From 679b098aa7246d99d334a57129f86a6f5954c712 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Tue, 24 Jan 2017 12:04:45 -0800
Subject: [PATCH 13/17] No firewall enabled test.

---
 test/test_automated_install.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index 4864e4a2..48166fae 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -105,13 +105,13 @@ def test_configureFirewall_firewalld_enabled_declined_no_errors(Pihole):
     expected_stdout = 'Not installing firewall rulesets.'
     assert expected_stdout in configureFirewall.stdout
 
-def test_configureFirewall_iptables_baseline(Pihole):
-    ''' confirms IPTables rules are not applied when firewallD is not running '''
+def test_configureFirewall_no_firewall(Pihole):
+    ''' confirms firewall skipped no daemon is running '''
     configureFirewall = Pihole.run('''
     source /opt/pihole/basic-install.sh
     configureFirewall
     ''')
-    expected_stdout = 'Filler Text'
+    expected_stdout = 'No active firewall detected'
     assert expected_stdout in configureFirewall.stdout
 
 # Helper functions

From 4bb71ae046a40f60eb23dcf584875ba6d5d46440 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Tue, 24 Jan 2017 12:09:58 -0800
Subject: [PATCH 14/17] IPtables tests.

mock commands for iptables check.

Test setting IPTables ruleset.

Test setting IPTables ruleset.

Test for already configured IPTables rules.

Test for addition of iptables rules.

Can only mock so deep in the commands.
---
 test/test_automated_install.py | 52 ++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index 48166fae..58d98939 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -114,6 +114,58 @@ def test_configureFirewall_no_firewall(Pihole):
     expected_stdout = 'No active firewall detected'
     assert expected_stdout in configureFirewall.stdout
 
+def test_configureFirewall_IPTables_enabled_declined_no_errors(Pihole):
+    ''' confirms IPTables rules are not applied when IPTables is running, user declines ruleset '''
+    # iptables command exists
+    mock_command('iptables', '', '0', Pihole)
+    # modinfo returns always true (ip_tables module check)
+    mock_command('modinfo', '', '0', Pihole)
+    # Whiptail dialog returns Cancel for user prompt
+    mock_command('whiptail', '', '1', Pihole)
+    configureFirewall = Pihole.run('''
+    source /opt/pihole/basic-install.sh
+    configureFirewall
+    ''')
+    expected_stdout = 'Not installing firewall rulesets.'
+    assert expected_stdout in configureFirewall.stdout
+
+def test_configureFirewall_IPTables_enabled_rules_exist_no_errors(Pihole):
+    ''' confirms IPTables rules are not applied when IPTables is running and rules exist '''
+    # iptables command exists and returns 0 on calls (should return 0 on iptables -C)
+    mock_command('iptables', '', '0', Pihole)
+    # modinfo returns always true (ip_tables module check)
+    mock_command('modinfo', '', '0', Pihole)
+    # Whiptail dialog returns Cancel for user prompt
+    mock_command('whiptail', '', '0', Pihole)
+    configureFirewall = Pihole.run('''
+    source /opt/pihole/basic-install.sh
+    configureFirewall
+    ''')
+    expected_stdout = 'Installing new IPTables firewall rulesets'
+    assert expected_stdout in configureFirewall.stdout
+    firewall_calls = Pihole.run('cat /var/log/iptables').stdout
+    assert 'iptables -I INPUT 1 -p tcp -m tcp --dport 80 -j ACCEPT' not in firewall_calls
+    assert 'iptables -I INPUT 1 -p tcp -m tcp --dport 53 -j ACCEPT' not in firewall_calls
+    assert 'iptables -I INPUT 1 -p udp -m udp --dport 53 -j ACCEPT' not in firewall_calls
+
+def test_configureFirewall_IPTables_enabled_not_exist_no_errors(Pihole):
+    ''' confirms IPTables rules are applied when IPTables is running and rules do not exist '''
+    # iptables command and returns 1 on calls (should return 1 on iptables -C)
+    mock_command('iptables', '', '1', Pihole)
+    # modinfo returns always true (ip_tables module check)
+    mock_command('modinfo', '', '0', Pihole)
+    # Whiptail dialog returns Cancel for user prompt
+    mock_command('whiptail', '', '0', Pihole)
+    configureFirewall = Pihole.run('''
+    source /opt/pihole/basic-install.sh
+    configureFirewall
+    ''')
+    expected_stdout = 'Installing new IPTables firewall rulesets'
+    assert expected_stdout in configureFirewall.stdout
+    firewall_calls = Pihole.run('cat /var/log/iptables').stdout
+    # Only check the first rule, since iptables returns a 1 from the mock command.
+    assert 'iptables -I INPUT 1 -p tcp -m tcp --dport 80 -j ACCEPT' in firewall_calls
+
 # Helper functions
 def mock_command(script, result, retVal, container):
     ''' Allows for setup of commands we don't really want to have to run for real in unit tests '''

From b16f7973174fc34466f4265c27d41e3429b78bb9 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Tue, 24 Jan 2017 15:44:48 -0800
Subject: [PATCH 15/17] `firewall-cmd --state` returns 0 on 'running' non 0 on
 'not running', so check retval and not text returned.

FirewallD conversion to multicall

IPTables test
---
 automated install/basic-install.sh |  2 +-
 test/test_automated_install.py     | 48 +++++++++++++++++-------------
 2 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh
index fe4d7bfb..18bff00d 100755
--- a/automated install/basic-install.sh	
+++ b/automated install/basic-install.sh	
@@ -879,7 +879,7 @@ create_pihole_user() {
 
 configureFirewall() {
   # Allow HTTP and DNS traffic
-  if [[ $(firewall-cmd --state) == "running" ]]; then
+  if firewall-cmd --state &> /dev/null; then
     whiptail --title "Firewall in use" --yesno "We have detected a running firewall\n\nPi-hole currently requires HTTP and DNS port access.\n\n\n\nInstall Pi-hole default firewall rules?" ${r} ${c} || \
     { echo -e ":::\n::: Not installing firewall rulesets."; return 1; }
     echo -e ":::\n:::\n Configuring FirewallD for httpd and dnsmasq."
diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index 58d98939..14b2add2 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -67,9 +67,9 @@ def test_setupVars_saved_to_file(Pihole):
 def test_configureFirewall_firewalld_running_no_errors(Pihole):
     ''' confirms firewalld rules are applied when firewallD is running '''
     # firewallD returns 'running' as status
-    mock_command('firewall-cmd', 'running', '0', Pihole)
+    mock_command('firewall-cmd', {'*':('running', 0)}, Pihole)
     # Whiptail dialog returns Ok for user prompt
-    mock_command('whiptail', '', '0', Pihole)
+    mock_command('whiptail', {'*':('', 0)}, Pihole)
     configureFirewall = Pihole.run('''
     source /opt/pihole/basic-install.sh
     configureFirewall
@@ -84,7 +84,7 @@ def test_configureFirewall_firewalld_running_no_errors(Pihole):
 def test_configureFirewall_firewalld_disabled_no_errors(Pihole):
     ''' confirms firewalld rules are not applied when firewallD is not running '''
     # firewallD returns non-running status
-    mock_command('firewall-cmd', 'stopped', '0', Pihole)
+    mock_command('firewall-cmd', {'*':('not running', '1')}, Pihole)
     configureFirewall = Pihole.run('''
     source /opt/pihole/basic-install.sh
     configureFirewall
@@ -95,9 +95,9 @@ def test_configureFirewall_firewalld_disabled_no_errors(Pihole):
 def test_configureFirewall_firewalld_enabled_declined_no_errors(Pihole):
     ''' confirms firewalld rules are not applied when firewallD is running, user declines ruleset '''
     # firewallD returns running status
-    mock_command('firewall-cmd', 'running', '0', Pihole)
+    mock_command('firewall-cmd', {'*':('running', 0)}, Pihole)
     # Whiptail dialog returns Cancel for user prompt
-    mock_command('whiptail', '', '1', Pihole)
+    mock_command('whiptail', {'*':('', 1)}, Pihole)
     configureFirewall = Pihole.run('''
     source /opt/pihole/basic-install.sh
     configureFirewall
@@ -117,11 +117,11 @@ def test_configureFirewall_no_firewall(Pihole):
 def test_configureFirewall_IPTables_enabled_declined_no_errors(Pihole):
     ''' confirms IPTables rules are not applied when IPTables is running, user declines ruleset '''
     # iptables command exists
-    mock_command('iptables', '', '0', Pihole)
+    mock_command('iptables', {'*':('', '0')}, Pihole)
     # modinfo returns always true (ip_tables module check)
-    mock_command('modinfo', '', '0', Pihole)
+    mock_command('modinfo', {'*':('', '0')}, Pihole)
     # Whiptail dialog returns Cancel for user prompt
-    mock_command('whiptail', '', '1', Pihole)
+    mock_command('whiptail', {'*':('', '1')}, Pihole)
     configureFirewall = Pihole.run('''
     source /opt/pihole/basic-install.sh
     configureFirewall
@@ -132,11 +132,11 @@ def test_configureFirewall_IPTables_enabled_declined_no_errors(Pihole):
 def test_configureFirewall_IPTables_enabled_rules_exist_no_errors(Pihole):
     ''' confirms IPTables rules are not applied when IPTables is running and rules exist '''
     # iptables command exists and returns 0 on calls (should return 0 on iptables -C)
-    mock_command('iptables', '', '0', Pihole)
+    mock_command('iptables', {'-S':('-P INPUT DENY', '0')}, Pihole)
     # modinfo returns always true (ip_tables module check)
-    mock_command('modinfo', '', '0', Pihole)
+    mock_command('modinfo', {'*':('', '0')}, Pihole)
     # Whiptail dialog returns Cancel for user prompt
-    mock_command('whiptail', '', '0', Pihole)
+    mock_command('whiptail', {'*':('', '0')}, Pihole)
     configureFirewall = Pihole.run('''
     source /opt/pihole/basic-install.sh
     configureFirewall
@@ -150,12 +150,12 @@ def test_configureFirewall_IPTables_enabled_rules_exist_no_errors(Pihole):
 
 def test_configureFirewall_IPTables_enabled_not_exist_no_errors(Pihole):
     ''' confirms IPTables rules are applied when IPTables is running and rules do not exist '''
-    # iptables command and returns 1 on calls (should return 1 on iptables -C)
-    mock_command('iptables', '', '1', Pihole)
+    # iptables command and returns 0 on calls (should return 1 on iptables -C)
+    mock_command('iptables', {'-S':('-P INPUT DENY', '0'), '-C':('', 1), '-I':('', 0)}, Pihole)
     # modinfo returns always true (ip_tables module check)
-    mock_command('modinfo', '', '0', Pihole)
+    mock_command('modinfo', {'*':('', '0')}, Pihole)
     # Whiptail dialog returns Cancel for user prompt
-    mock_command('whiptail', '', '0', Pihole)
+    mock_command('whiptail', {'*':('', '0')}, Pihole)
     configureFirewall = Pihole.run('''
     source /opt/pihole/basic-install.sh
     configureFirewall
@@ -167,20 +167,26 @@ def test_configureFirewall_IPTables_enabled_not_exist_no_errors(Pihole):
     assert 'iptables -I INPUT 1 -p tcp -m tcp --dport 80 -j ACCEPT' in firewall_calls
 
 # Helper functions
-def mock_command(script, result, retVal, container):
+def mock_command(script, args, container):
     ''' Allows for setup of commands we don't really want to have to run for real in unit tests '''
-    ''' TODO: support array of results that enable the results to change over multiple executions of a command '''
     full_script_path = '/usr/local/bin/{}'.format(script)
     mock_script = dedent('''\
     #!/bin/bash -e
     echo "\$0 \$@" >> /var/log/{script}
-    echo {result}
-    exit {retcode}
-    '''.format(script=script, result=result,retcode=retVal))
+    case "\$1" in'''.format(script=script))
+    for k, v in args.iteritems():
+        case = dedent('''
+        {arg})
+        echo {res}
+        exit {retcode}
+        ;;'''.format(arg=k, res=v[0], retcode=v[1]))
+        mock_script += case
+    mock_script += dedent('''
+    esac''')
     container.run('''
     cat <<EOF> {script}\n{content}\nEOF
     chmod +x {script}
-    '''.format(script=full_script_path, content=mock_script))
+    rm -f /var/log/{scriptlog}'''.format(script=full_script_path, content=mock_script, scriptlog=script))
 
 def run_script(Pihole, script):
     result = Pihole.run(script)

From 0bbe0aed83e8b9efa28250625749f87844a3ac33 Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Tue, 24 Jan 2017 20:09:10 -0800
Subject: [PATCH 16/17] Last of the IPTables tests, new test rig is operating.

---
 test/test_automated_install.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/test_automated_install.py b/test/test_automated_install.py
index 14b2add2..211364ed 100644
--- a/test/test_automated_install.py
+++ b/test/test_automated_install.py
@@ -163,8 +163,9 @@ def test_configureFirewall_IPTables_enabled_not_exist_no_errors(Pihole):
     expected_stdout = 'Installing new IPTables firewall rulesets'
     assert expected_stdout in configureFirewall.stdout
     firewall_calls = Pihole.run('cat /var/log/iptables').stdout
-    # Only check the first rule, since iptables returns a 1 from the mock command.
     assert 'iptables -I INPUT 1 -p tcp -m tcp --dport 80 -j ACCEPT' in firewall_calls
+    assert 'iptables -I INPUT 1 -p tcp -m tcp --dport 53 -j ACCEPT' in firewall_calls
+    assert 'iptables -I INPUT 1 -p udp -m udp --dport 53 -j ACCEPT' in firewall_calls
 
 # Helper functions
 def mock_command(script, args, container):

From cec214f9000ff800d07bcd05233d155c396c17bd Mon Sep 17 00:00:00 2001
From: Dan Schaper <dschaper@ganymeade.com>
Date: Thu, 26 Jan 2017 14:38:02 -0800
Subject: [PATCH 17/17] User decline to install shouldn't be an error return.

---
 automated install/basic-install.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/automated install/basic-install.sh b/automated install/basic-install.sh
index 18bff00d..ef5c82c6 100755
--- a/automated install/basic-install.sh	
+++ b/automated install/basic-install.sh	
@@ -881,7 +881,7 @@ configureFirewall() {
   # Allow HTTP and DNS traffic
   if firewall-cmd --state &> /dev/null; then
     whiptail --title "Firewall in use" --yesno "We have detected a running firewall\n\nPi-hole currently requires HTTP and DNS port access.\n\n\n\nInstall Pi-hole default firewall rules?" ${r} ${c} || \
-    { echo -e ":::\n::: Not installing firewall rulesets."; return 1; }
+    { echo -e ":::\n::: Not installing firewall rulesets."; return 0; }
     echo -e ":::\n:::\n Configuring FirewallD for httpd and dnsmasq."
     firewall-cmd --permanent --add-port=80/tcp --add-port=53/tcp --add-port=53/udp
     firewall-cmd --reload
@@ -892,7 +892,7 @@ configureFirewall() {
     # then check and insert our Rules above the DROP/REJECT Rule.
     if iptables -S INPUT | head -n1 | grep -qv '^-P.*ACCEPT$' || iptables -S INPUT | tail -n1 | grep -qv '^-\(A\|P\).*ACCEPT$'; then
       whiptail --title "Firewall in use" --yesno "We have detected a running firewall\n\nPi-hole currently requires HTTP and DNS port access.\n\n\n\nInstall Pi-hole default firewall rules?" ${r} ${c} || \
-      { echo -e ":::\n::: Not installing firewall rulesets."; return 1; }
+      { echo -e ":::\n::: Not installing firewall rulesets."; return 0; }
       echo -e ":::\n::: Installing new IPTables firewall rulesets."
       # Check chain first, otherwise a new rule will duplicate old ones
       iptables -C INPUT -p tcp -m tcp --dport 80 -j ACCEPT &> /dev/null || iptables -I INPUT 1 -p tcp -m tcp --dport 80 -j ACCEPT