Merge pull request #1149 from pi-hole/tweak/firewall_warning

Add warning to firewall configuration

automated install/basic-install.sh

@@ -880,23 +880,31 @@ create_pihole_user() {
 configureFirewall() {
   # Allow HTTP and DNS traffic
   if firewall-cmd --state &> /dev/null; then
-    echo "::: Configuring FirewallD for httpd and dnsmasq.."
+    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 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
+    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 0; }
+      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() {
@@ -975,7 +983,6 @@ updatePihole() {
   CreateLogFile
   installPiholeWeb
   installCron
-  configureFirewall
   finalExports #re-export setupVars.conf to account for any new vars added in new versions
   runGravity
 }

test/test_automated_install.py

@@ -64,35 +64,130 @@ 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 appopriate '''
+    ''' confirms firewalld rules are applied when firewallD is running '''
-    mock_command('firewall-cmd', '0', Pihole)
+    # 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
     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
     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', {'*':('not running', '1')}, 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
+
+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 = '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', {'-S':('-P INPUT DENY', '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 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)
+    # 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' 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, result, 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}
-    exit {retcode}
+    case "\$1" in'''.format(script=script))
-    '''.format(script=script, retcode=result))
+    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)