Do not exit 1 if pkill killed Pi-hole. Killing is not an error

Signed-off-by: Christian König <ckoenig@posteo.de>

advanced/Scripts/query.sh

@@ -64,8 +64,8 @@ Example: 'pihole -q -exact domain.com'
 Query the adlists for a specified domain
 
 Options:
-  -exact              Search the adlists for exact domain matches
-  -all                Return all query matches within the adlists
+  -exact              Search the block lists for exact domain matches
+  -all                Return all query matches within a block list
   -h, --help          Show this help dialog"
   exit 0
 fi
@@ -210,7 +210,7 @@ mapfile -t results <<< "$(scanDatabaseTable "${domainQuery}" "gravity")"
 
 # Handle notices
 if [[ -z "${wbMatch:-}" ]] && [[ -z "${wcMatch:-}" ]] && [[ -z "${results[*]}" ]]; then
-    echo -e "  ${INFO} No ${exact/t/t }results found for ${COL_BOLD}${domainQuery}${COL_NC} within the adlists"
+    echo -e "  ${INFO} No ${exact/t/t }results found for ${COL_BOLD}${domainQuery}${COL_NC} within the block lists"
     exit 0
 elif [[ -z "${results[*]}" ]]; then
     # Result found in WL/BL/Wildcards

advanced/Scripts/utils.sh

@@ -1,6 +1,4 @@
-#!/usr/bin/env sh
-# shellcheck disable=SC3043 #https://github.com/koalaman/shellcheck/wiki/SC3043#exceptions
-
+#!/usr/bin/env bash
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)
 # Network-wide ad blocking via your own hardware.
@@ -17,80 +15,46 @@
 #  - New functions must have a test added for them in test/test_any_utils.py
 
 #######################
-# Takes Three arguments: file, key, and value.
-#
+# Takes three arguments key, value, and file.
 # Checks the target file for the existence of the key
 #   - If it exists, it changes the value
 #   - If it does not exist, it adds the value
 #
 # Example usage:
-# addOrEditKeyValPair "/etc/pihole/setupVars.conf" "BLOCKING_ENABLED" "true"
+# addOrEditKeyValuePair "BLOCKING_ENABLED" "true" "/etc/pihole/setupVars.conf"
 #######################
 addOrEditKeyValPair() {
-  local file="${1}"
-  local key="${2}"
-  local value="${3}"
-
+  local key="${1}"
+  local value="${2}"
+  local file="${3}"
   if grep -q "^${key}=" "${file}"; then
-      # Key already exists in file, modify the value
-      sed -i "/^${key}=/c\\${key}=${value}" "${file}"
+    sed -i "/^${key}=/c\\${key}=${value}" "${file}"
   else
-    # Key does not already exist, add it and it's value
     echo "${key}=${value}" >> "${file}"
   fi
 }
 
-#######################
-# Takes two arguments: file, and key.
-# Adds a key to target file
-#
-# Example usage:
-# addKey "/etc/dnsmasq.d/01-pihole.conf" "log-queries"
-#######################
-addKey(){
-  local file="${1}"
-  local key="${2}"
-
-  if ! grep -q "^${key}" "${file}"; then
-      # Key does not exist, add it.
-      echo "${key}" >> "${file}"
-  fi
-}
-
-#######################
-# Takes two arguments: file, and key.
-# Deletes a key or key/value pair from target file
-#
-# Example usage:
-# removeKey "/etc/pihole/setupVars.conf" "PIHOLE_DNS_1"
-#######################
-removeKey() {
-  local file="${1}"
-  local key="${2}"
-  sed -i "/^${key}/d" "${file}"
-}
-
 #######################
 # returns FTL's current telnet API port
 #######################
 getFTLAPIPort(){
-  local FTLCONFFILE="/etc/pihole/pihole-FTL.conf"
-  local DEFAULT_PORT_FILE="/run/pihole-FTL.port"
-  local DEFAULT_FTL_PORT=4711
+  local -r FTLCONFFILE="/etc/pihole/pihole-FTL.conf"
+  local -r DEFAULT_PORT_FILE="/run/pihole-FTL.port"
+  local -r DEFAULT_FTL_PORT=4711
   local PORTFILE
   local ftl_api_port
 
-  if [ -f "$FTLCONFFILE" ]; then
+  if [[ -f "$FTLCONFFILE" ]]; then
     # if PORTFILE is not set in pihole-FTL.conf, use the default path
     PORTFILE="$( (grep "^PORTFILE=" $FTLCONFFILE || echo "$DEFAULT_PORT_FILE") | cut -d"=" -f2-)"
   fi
 
-  if [ -s "$PORTFILE" ]; then
+  if [[ -s "$PORTFILE" ]]; then
     # -s: FILE exists and has a size greater than zero
-    ftl_api_port=$(cat "${PORTFILE}")
+    ftl_api_port=$(<"$PORTFILE")
     # Exploit prevention: unset the variable if there is malicious content
-    # Verify that the value read from the file is numeric    
-    expr "$ftl_api_port" : "[^[:digit:]]" > /dev/null && unset ftl_api_port
+    # Verify that the value read from the file is numeric
+    [[ "$ftl_api_port" =~ [^[:digit:]] ]] && unset ftl_api_port
   fi
 
   # echo the port found in the portfile or default to the default port

advanced/Scripts/webpage.sh

@@ -1,7 +1,5 @@
 #!/usr/bin/env bash
 # shellcheck disable=SC1090
-# shellcheck disable=SC2154
-
 
 # Pi-hole: A black hole for Internet advertisements
 # (c) 2017 Pi-hole, LLC (https://pi-hole.net)
@@ -28,9 +26,6 @@ readonly PI_HOLE_FILES_DIR="/etc/.pihole"
 PH_TEST="true"
 source "${PI_HOLE_FILES_DIR}/automated install/basic-install.sh"
 
-utilsfile="/opt/pihole/utils.sh"
-source "${utilsfile}"
-
 coltable="/opt/pihole/COL_TABLE"
 if [[ -f ${coltable} ]]; then
     source ${coltable}
@@ -56,39 +51,45 @@ Options:
 }
 
 add_setting() {
-    addOrEditKeyValPair "${setupVars}" "${1}" "${2}"
+    echo "${1}=${2}" >> "${setupVars}"
 }
 
 delete_setting() {
-    removeKey "${setupVars}" "${1}"
+    sed -i "/^${1}/d" "${setupVars}"
 }
 
 change_setting() {
-    addOrEditKeyValPair "${setupVars}" "${1}" "${2}"
+    delete_setting "${1}"
+    add_setting "${1}" "${2}"
 }
 
 addFTLsetting() {
-    addOrEditKeyValPair "${FTLconf}" "${1}" "${2}"
+    echo "${1}=${2}" >> "${FTLconf}"
 }
 
 deleteFTLsetting() {
-    removeKey "${FTLconf}" "${1}"
+    sed -i "/^${1}/d" "${FTLconf}"
 }
 
 changeFTLsetting() {
-    addOrEditKeyValPair "${FTLconf}" "${1}" "${2}"
+    deleteFTLsetting "${1}"
+    addFTLsetting "${1}" "${2}"
 }
 
 add_dnsmasq_setting() {
-    addOrEditKeyValPair "${dnsmasqconfig}" "${1}" "${2}"
+    if [[ "${2}" != "" ]]; then
+        echo "${1}=${2}" >> "${dnsmasqconfig}"
+    else
+        echo "${1}" >> "${dnsmasqconfig}"
+    fi
 }
 
 delete_dnsmasq_setting() {
-    removeKey "${dnsmasqconfig}" "${1}"
+    sed -i "/^${1}/d" "${dnsmasqconfig}"
 }
 
 SetTemperatureUnit() {
-    addOrEditKeyValPair "${setupVars}" "TEMPERATUREUNIT" "${unit}"
+    change_setting "TEMPERATUREUNIT" "${unit}"
     echo -e "  ${TICK} Set temperature unit to ${unit}"
 }
 
@@ -123,7 +124,7 @@ SetWebPassword() {
         echo ""
 
         if [ "${PASSWORD}" == "" ]; then
-            addOrEditKeyValPair "${setupVars}" "WEBPASSWORD" ""
+            change_setting "WEBPASSWORD" ""
             echo -e "  ${TICK} Password Removed"
             exit 0
         fi
@@ -136,7 +137,7 @@ SetWebPassword() {
         # We do not wrap this in brackets, otherwise BASH will expand any appropriate syntax
         hash=$(HashPassword "$PASSWORD")
         # Save hash to file
-        addOrEditKeyValPair "${setupVars}" "WEBPASSWORD" "${hash}"
+        change_setting "WEBPASSWORD" "${hash}"
         echo -e "  ${TICK} New password set"
     else
         echo -e "  ${CROSS} Passwords don't match. Your password has not been changed"
@@ -147,7 +148,7 @@ SetWebPassword() {
 ProcessDNSSettings() {
     source "${setupVars}"
 
-    removeKey "${dnsmasqconfig}" "server"
+    delete_dnsmasq_setting "server"
 
     COUNTER=1
     while true ; do
@@ -155,34 +156,34 @@ ProcessDNSSettings() {
         if [ -z "${!var}" ]; then
             break;
         fi
-        addOrEditKeyValPair "${dnsmasqconfig}" "server" "${!var}"
+        add_dnsmasq_setting "server" "${!var}"
         (( COUNTER++ ))
     done
 
     # The option LOCAL_DNS_PORT is deprecated
     # We apply it once more, and then convert it into the current format
     if [ -n "${LOCAL_DNS_PORT}" ]; then
-        addOrEditKeyValPair "${dnsmasqconfig}" "server" "127.0.0.1#${LOCAL_DNS_PORT}"
-        addOrEditKeyValPair "${setupVars}" "PIHOLE_DNS_${COUNTER}" "127.0.0.1#${LOCAL_DNS_PORT}"
-        removeKey "${setupVars}" "LOCAL_DNS_PORT"
+        add_dnsmasq_setting "server" "127.0.0.1#${LOCAL_DNS_PORT}"
+        add_setting "PIHOLE_DNS_${COUNTER}" "127.0.0.1#${LOCAL_DNS_PORT}"
+        delete_setting "LOCAL_DNS_PORT"
     fi
 
-    removeKey "${dnsmasqconfig}" "domain-needed"
-    removeKey "${dnsmasqconfig}" "expand-hosts"
+    delete_dnsmasq_setting "domain-needed"
+    delete_dnsmasq_setting "expand-hosts"
 
     if [[ "${DNS_FQDN_REQUIRED}" == true ]]; then
-        addKey "${dnsmasqconfig}" "domain-needed"
-        addKey "${dnsmasqconfig}" "expand-hosts"
+        add_dnsmasq_setting "domain-needed"
+        add_dnsmasq_setting "expand-hosts"
     fi
 
-    removeKey "${dnsmasqconfig}" "bogus-priv"
+    delete_dnsmasq_setting "bogus-priv"
 
     if [[ "${DNS_BOGUS_PRIV}" == true ]]; then
-        addKey "${dnsmasqconfig}" "bogus-priv"
+        add_dnsmasq_setting "bogus-priv"
     fi
 
-    removeKey "${dnsmasqconfig}" "dnssec"
-    removeKey "${dnsmasqconfig}" "trust-anchor"
+    delete_dnsmasq_setting "dnssec"
+    delete_dnsmasq_setting "trust-anchor="
 
     if [[ "${DNSSEC}" == true ]]; then
         echo "dnssec
@@ -190,24 +191,24 @@ trust-anchor=.,20326,8,2,E06D44B80B8F1D39A95C0B0D7C65D08458E880409BBC68345710423
 " >> "${dnsmasqconfig}"
     fi
 
-    removeKey "${dnsmasqconfig}" "host-record"
+    delete_dnsmasq_setting "host-record"
 
     if [ -n "${HOSTRECORD}" ]; then
-        addOrEditKeyValPair "${dnsmasqconfig}" "host-record" "${HOSTRECORD}"
+        add_dnsmasq_setting "host-record" "${HOSTRECORD}"
     fi
 
     # Setup interface listening behavior of dnsmasq
-    removeKey "${dnsmasqconfig}" "interface"
-    removeKey "${dnsmasqconfig}" "local-service"
-    removeKey "${dnsmasqconfig}" "except-interface"
-    removeKey "${dnsmasqconfig}" "bind-interfaces"
+    delete_dnsmasq_setting "interface"
+    delete_dnsmasq_setting "local-service"
+    delete_dnsmasq_setting "except-interface"
+    delete_dnsmasq_setting "bind-interfaces"
 
     if [[ "${DNSMASQ_LISTENING}" == "all" ]]; then
         # Listen on all interfaces, permit all origins
-        addOrEditKeyValPair "${dnsmasqconfig}" "except-interface" "nonexisting"
+        add_dnsmasq_setting "except-interface" "nonexisting"
     elif [[ "${DNSMASQ_LISTENING}" == "local" ]]; then
         # Listen only on all interfaces, but only local subnets
-        addKey "${dnsmasqconfig}" "local-service"
+        add_dnsmasq_setting "local-service"
     else
         # Options "bind" and "single"
         # Listen only on one interface
@@ -216,30 +217,30 @@ trust-anchor=.,20326,8,2,E06D44B80B8F1D39A95C0B0D7C65D08458E880409BBC68345710423
             PIHOLE_INTERFACE="eth0"
         fi
 
-        addOrEditKeyValPair "${dnsmasqconfig}" "interface" "${PIHOLE_INTERFACE}"
+        add_dnsmasq_setting "interface" "${PIHOLE_INTERFACE}"
 
         if [[ "${DNSMASQ_LISTENING}" == "bind" ]]; then
             # Really bind to interface
-            addKey "${dnsmasqconfig}" "bind-interfaces"
+            add_dnsmasq_setting "bind-interfaces"
         fi
     fi
 
     if [[ "${CONDITIONAL_FORWARDING}" == true ]]; then
         # Convert legacy "conditional forwarding" to rev-server configuration
         # Remove any existing REV_SERVER settings
-        removeKey "${setupVars}" "REV_SERVER"
-        removeKey "${setupVars}" "REV_SERVER_DOMAIN"
-        removeKey "${setupVars}" "REV_SERVER_TARGET"
-        removeKey "${setupVars}" "REV_SERVER_CIDR"
+        delete_setting "REV_SERVER"
+        delete_setting "REV_SERVER_DOMAIN"
+        delete_setting "REV_SERVER_TARGET"
+        delete_setting "REV_SERVER_CIDR"
 
         REV_SERVER=true
-        addOrEditKeyValPair "${setupVars}" "REV_SERVER" "true"
+        add_setting "REV_SERVER" "true"
 
         REV_SERVER_DOMAIN="${CONDITIONAL_FORWARDING_DOMAIN}"
-        addOrEditKeyValPair "${setupVars}" "REV_SERVER_DOMAIN" "${REV_SERVER_DOMAIN}"
+        add_setting "REV_SERVER_DOMAIN" "${REV_SERVER_DOMAIN}"
 
         REV_SERVER_TARGET="${CONDITIONAL_FORWARDING_IP}"
-        addOrEditKeyValPair "${setupVars}" "REV_SERVER_TARGET" "${REV_SERVER_TARGET}"
+        add_setting "REV_SERVER_TARGET" "${REV_SERVER_TARGET}"
 
         #Convert CONDITIONAL_FORWARDING_REVERSE if necessary e.g:
         #          1.1.168.192.in-addr.arpa to 192.168.1.1/32
@@ -266,28 +267,28 @@ trust-anchor=.,20326,8,2,E06D44B80B8F1D39A95C0B0D7C65D08458E880409BBC68345710423
             # shellcheck disable=2001
             REV_SERVER_CIDR="$(sed "s+\\.[0-9]*$+\\.0/24+" <<< "${REV_SERVER_TARGET}")"
         fi
-        addOrEditKeyValPair "${setupVars}" "REV_SERVER_CIDR" "${REV_SERVER_CIDR}"
+        add_setting "REV_SERVER_CIDR" "${REV_SERVER_CIDR}"
 
         # Remove obsolete settings from setupVars.conf
-        removeKey "${setupVars}" "CONDITIONAL_FORWARDING"
-        removeKey "${setupVars}" "CONDITIONAL_FORWARDING_REVERSE"
-        removeKey "${setupVars}" "CONDITIONAL_FORWARDING_DOMAIN"
-        removeKey "${setupVars}" "CONDITIONAL_FORWARDING_IP"
+        delete_setting "CONDITIONAL_FORWARDING"
+        delete_setting "CONDITIONAL_FORWARDING_REVERSE"
+        delete_setting "CONDITIONAL_FORWARDING_DOMAIN"
+        delete_setting "CONDITIONAL_FORWARDING_IP"
     fi
 
-    removeKey "${dnsmasqconfig}" "rev-server"
+    delete_dnsmasq_setting "rev-server"
 
     if [[ "${REV_SERVER}" == true ]]; then
-        addKey "${dnsmasqconfig}" "rev-server=${REV_SERVER_CIDR},${REV_SERVER_TARGET}"
+        add_dnsmasq_setting "rev-server=${REV_SERVER_CIDR},${REV_SERVER_TARGET}"
         if [ -n "${REV_SERVER_DOMAIN}" ]; then
             # Forward local domain names to the CF target, too
-            addKey "${dnsmasqconfig}" "server=/${REV_SERVER_DOMAIN}/${REV_SERVER_TARGET}"
+            add_dnsmasq_setting "server=/${REV_SERVER_DOMAIN}/${REV_SERVER_TARGET}"
         fi
 
         if [[ "${DNS_FQDN_REQUIRED}" != true ]]; then
             # Forward unqualified names to the CF target only when the "never
             # forward non-FQDN" option is unticked
-            addKey "${dnsmasqconfig}" "server=//${REV_SERVER_TARGET}"
+            add_dnsmasq_setting "server=//${REV_SERVER_TARGET}"
         fi
 
     fi
@@ -302,7 +303,7 @@ trust-anchor=.,20326,8,2,E06D44B80B8F1D39A95C0B0D7C65D08458E880409BBC68345710423
 
 SetDNSServers() {
     # Save setting to file
-    removeKey "${setupVars}" "PIHOLE_DNS"
+    delete_setting "PIHOLE_DNS"
     IFS=',' read -r -a array <<< "${args[2]}"
     for index in "${!array[@]}"
     do
@@ -311,7 +312,7 @@ SetDNSServers() {
         ip="${array[index]//\\#/#}"
 
         if valid_ip "${ip}" || valid_ip6 "${ip}" ; then
-            addOrEditKeyValPair "${setupVars}" "PIHOLE_DNS_$((index+1))" "${ip}"
+            add_setting "PIHOLE_DNS_$((index+1))" "${ip}"
         else
             echo -e "  ${CROSS} Invalid IP has been passed"
             exit 1
@@ -319,30 +320,30 @@ SetDNSServers() {
     done
 
     if [[ "${args[3]}" == "domain-needed" ]]; then
-        addOrEditKeyValPair "${setupVars}" "DNS_FQDN_REQUIRED" "true"
+        change_setting "DNS_FQDN_REQUIRED" "true"
     else
-        addOrEditKeyValPair "${setupVars}" "DNS_FQDN_REQUIRED" "false"
+        change_setting "DNS_FQDN_REQUIRED" "false"
     fi
 
     if [[ "${args[4]}" == "bogus-priv" ]]; then
-        addOrEditKeyValPair "${setupVars}" "DNS_BOGUS_PRIV" "true"
+        change_setting "DNS_BOGUS_PRIV" "true"
     else
-        addOrEditKeyValPair "${setupVars}" "DNS_BOGUS_PRIV" "false"
+        change_setting "DNS_BOGUS_PRIV" "false"
     fi
 
     if [[ "${args[5]}" == "dnssec" ]]; then
-        addOrEditKeyValPair "${setupVars}" "DNSSEC" "true"
+        change_setting "DNSSEC" "true"
     else
-        addOrEditKeyValPair "${setupVars}" "DNSSEC" "false"
+        change_setting "DNSSEC" "false"
     fi
 
     if [[ "${args[6]}" == "rev-server" ]]; then
-        addOrEditKeyValPair "${setupVars}" "REV_SERVER" "true"
-        addOrEditKeyValPair "${setupVars}" "REV_SERVER_CIDR" "${args[7]}"
-        addOrEditKeyValPair "${setupVars}" "REV_SERVER_TARGET" "${args[8]}"
-        addOrEditKeyValPair "${setupVars}" "REV_SERVER_DOMAIN" "${args[9]}"
+        change_setting "REV_SERVER" "true"
+        change_setting "REV_SERVER_CIDR" "${args[7]}"
+        change_setting "REV_SERVER_TARGET" "${args[8]}"
+        change_setting "REV_SERVER_DOMAIN" "${args[9]}"
     else
-        addOrEditKeyValPair "${setupVars}" "REV_SERVER" "false"
+        change_setting "REV_SERVER" "false"
     fi
 
     ProcessDNSSettings
@@ -352,11 +353,11 @@ SetDNSServers() {
 }
 
 SetExcludeDomains() {
-    addOrEditKeyValPair "${setupVars}" "API_EXCLUDE_DOMAINS" "${args[2]}"
+    change_setting "API_EXCLUDE_DOMAINS" "${args[2]}"
 }
 
 SetExcludeClients() {
-    addOrEditKeyValPair "${setupVars}" "API_EXCLUDE_CLIENTS" "${args[2]}"
+    change_setting "API_EXCLUDE_CLIENTS" "${args[2]}"
 }
 
 Poweroff(){
@@ -372,7 +373,7 @@ RestartDNS() {
 }
 
 SetQueryLogOptions() {
-    addOrEditKeyValPair "${setupVars}" "API_QUERY_LOG_SHOW" "${args[2]}"
+    change_setting "API_QUERY_LOG_SHOW" "${args[2]}"
 }
 
 ProcessDHCPSettings() {
@@ -388,19 +389,19 @@ ProcessDHCPSettings() {
 
         if [[ "${PIHOLE_DOMAIN}" == "" ]]; then
             PIHOLE_DOMAIN="lan"
-            addOrEditKeyValPair "${setupVars}" "PIHOLE_DOMAIN" "${PIHOLE_DOMAIN}"
+            change_setting "PIHOLE_DOMAIN" "${PIHOLE_DOMAIN}"
         fi
 
         if [[ "${DHCP_LEASETIME}" == "0" ]]; then
             leasetime="infinite"
         elif [[ "${DHCP_LEASETIME}" == "" ]]; then
             leasetime="24"
-            addOrEditKeyValPair "${setupVars}" "DHCP_LEASETIME" "${leasetime}"
+            change_setting "DHCP_LEASETIME" "${leasetime}"
         elif [[ "${DHCP_LEASETIME}" == "24h" ]]; then
             #Installation is affected by known bug, introduced in a previous version.
             #This will automatically clean up setupVars.conf and remove the unnecessary "h"
             leasetime="24"
-            addOrEditKeyValPair "${setupVars}" "DHCP_LEASETIME" "${leasetime}"
+            change_setting "DHCP_LEASETIME" "${leasetime}"
         else
             leasetime="${DHCP_LEASETIME}h"
         fi
@@ -453,24 +454,24 @@ ra-param=*,0,0
 }
 
 EnableDHCP() {
-    addOrEditKeyValPair "${setupVars}" "DHCP_ACTIVE" "true"
-    addOrEditKeyValPair "${setupVars}" "DHCP_START" "${args[2]}"
-    addOrEditKeyValPair "${setupVars}" "DHCP_END" "${args[3]}"
-    addOrEditKeyValPair "${setupVars}" "DHCP_ROUTER" "${args[4]}"
-    addOrEditKeyValPair "${setupVars}" "DHCP_LEASETIME" "${args[5]}"
-    addOrEditKeyValPair "${setupVars}" "PIHOLE_DOMAIN" "${args[6]}"
-    addOrEditKeyValPair "${setupVars}" "DHCP_IPv6" "${args[7]}"
-    addOrEditKeyValPair "${setupVars}" "DHCP_rapid_commit" "${args[8]}"
+    change_setting "DHCP_ACTIVE" "true"
+    change_setting "DHCP_START" "${args[2]}"
+    change_setting "DHCP_END" "${args[3]}"
+    change_setting "DHCP_ROUTER" "${args[4]}"
+    change_setting "DHCP_LEASETIME" "${args[5]}"
+    change_setting "PIHOLE_DOMAIN" "${args[6]}"
+    change_setting "DHCP_IPv6" "${args[7]}"
+    change_setting "DHCP_rapid_commit" "${args[8]}"
 
     # Remove possible old setting from file
-    removeKey "${dnsmasqconfig}" "dhcp-"
-    removeKey "${dnsmasqconfig}" "quiet-dhcp"
+    delete_dnsmasq_setting "dhcp-"
+    delete_dnsmasq_setting "quiet-dhcp"
 
     # If a DHCP client claims that its name is "wpad", ignore that.
     # This fixes a security hole. see CERT Vulnerability VU#598349
     # We also ignore "localhost" as Windows behaves strangely if a
     # device claims this host name
-    addKey "${dnsmasqconfig}" "dhcp-name-match=set:hostname-ignore,wpad
+    add_dnsmasq_setting "dhcp-name-match=set:hostname-ignore,wpad
 dhcp-name-match=set:hostname-ignore,localhost
 dhcp-ignore-names=tag:hostname-ignore"
 
@@ -480,11 +481,11 @@ dhcp-ignore-names=tag:hostname-ignore"
 }
 
 DisableDHCP() {
-    addOrEditKeyValPair "${setupVars}" "DHCP_ACTIVE" "false"
+    change_setting "DHCP_ACTIVE" "false"
 
     # Remove possible old setting from file
-    removeKey "${dnsmasqconfig}" "dhcp-"
-    removeKey "${dnsmasqconfig}" "quiet-dhcp"
+    delete_dnsmasq_setting "dhcp-"
+    delete_dnsmasq_setting "quiet-dhcp"
 
     ProcessDHCPSettings
 
@@ -492,11 +493,11 @@ DisableDHCP() {
 }
 
 SetWebUILayout() {
-    addOrEditKeyValPair "${setupVars}" "WEBUIBOXEDLAYOUT" "${args[2]}"
+    change_setting "WEBUIBOXEDLAYOUT" "${args[2]}"
 }
 
 SetWebUITheme() {
-    addOrEditKeyValPair "${setupVars}" "WEBTHEME" "${args[2]}"
+    change_setting "WEBTHEME" "${args[2]}"
 }
 
 CheckUrl(){
@@ -591,10 +592,10 @@ Options:
             exit 0
         fi
 
-        addOrEditKeyValPair "${setupVars}" "ADMIN_EMAIL" "${args[2]}"
+        change_setting "ADMIN_EMAIL" "${args[2]}"
         echo -e "  ${TICK} Setting admin contact to ${args[2]}"
     else
-        addOrEditKeyValPair "${setupVars}" "ADMIN_EMAIL" ""
+        change_setting "ADMIN_EMAIL" ""
         echo -e "  ${TICK} Removing admin contact"
     fi
 }
@@ -618,16 +619,16 @@ Interfaces:
 
     if [[ "${args[2]}" == "all" ]]; then
         echo -e "  ${INFO} Listening on all interfaces, permitting all origins. Please use a firewall!"
-        addOrEditKeyValPair "${setupVars}" "DNSMASQ_LISTENING" "all"
+        change_setting "DNSMASQ_LISTENING" "all"
     elif [[ "${args[2]}" == "local" ]]; then
         echo -e "  ${INFO} Listening on all interfaces, permitting origins from one hop away (LAN)"
-        addOrEditKeyValPair "${setupVars}" "DNSMASQ_LISTENING" "local"
+        change_setting "DNSMASQ_LISTENING" "local"
     elif [[ "${args[2]}" == "bind" ]]; then
         echo -e "  ${INFO} Binding on interface ${PIHOLE_INTERFACE}"
-        addOrEditKeyValPair "${setupVars}" "DNSMASQ_LISTENING" "bind"
+        change_setting "DNSMASQ_LISTENING" "bind"
     else
         echo -e "  ${INFO} Listening only on interface ${PIHOLE_INTERFACE}"
-        addOrEditKeyValPair "${setupVars}" "DNSMASQ_LISTENING" "single"
+        change_setting "DNSMASQ_LISTENING" "single"
     fi
 
     # Don't restart DNS server yet because other settings
@@ -697,7 +698,7 @@ clearAudit()
 SetPrivacyLevel() {
     # Set privacy level. Minimum is 0, maximum is 3
     if [ "${args[2]}" -ge 0 ] && [ "${args[2]}" -le 3 ]; then
-        addOrEditKeyValPair "${FTLconf}" "PRIVACYLEVEL" "${args[2]}"
+        changeFTLsetting "PRIVACYLEVEL" "${args[2]}"
         pihole restartdns reload-lists
     fi
 }
@@ -815,7 +816,7 @@ SetRateLimit() {
 
     # Set rate-limit setting inf valid
     if [ "${rate_limit_count}" -ge 0 ] && [ "${rate_limit_interval}" -ge 0 ]; then
-        addOrEditKeyValPair "${FTLconf}" "RATE_LIMIT" "${rate_limit_count}/${rate_limit_interval}"
+        changeFTLsetting "RATE_LIMIT" "${rate_limit_count}/${rate_limit_interval}"
     fi
 
     # Restart FTL to update rate-limit settings only if $reload not false

advanced/Templates/pihole-FTL.service

@@ -9,8 +9,39 @@
 # Description:       Enable service provided by pihole-FTL daemon
 ### END INIT INFO
 
+# Get PID of main pihole-FTL process
+FTLCONFFILE="/etc/pihole/pihole-FTL.conf"
+DEFAULT_PID_FILE="/run/pihole-FTL.pid"
+getFTLPID() {
+  if [ -s "$FTLCONFFILE" ]; then
+    # if PIDFILE is not set in pihole-FTL.conf, use the default path
+    FTL_PID_FILE="$( (grep "^PIDFILE=" $FTLCONFFILE || echo "$DEFAULT_PID_FILE") | cut -d"=" -f2-)"
+  else
+    # if there is no pihole-FTL.conf, use the default path
+    FTL_PID_FILE="$DEFAULT_PID_FILE"
+  fi
+
+  if [ -s "${FTL_PID_FILE}" ]; then
+    # -s: FILE exists and has a size greater than zero
+    FTL_PID="$(cat "$FTL_PID_FILE")"
+    # Exploit prevention: unset the variable if there is malicious content
+    # Verify that the value read from the file is numeric
+    expr "$FTL_PID" : "[^[:digit:]]" > /dev/null && unset FTL_PID
+  fi
+
+  # If FTL is not running, or the PID file contains malicious stuff, substitute
+  # negative PID to signal this to the caller
+  echo "${FTL_PID:=-1}"
+}
+
+
 is_running() {
-  pgrep -xo "pihole-FTL" > /dev/null
+  FTL_PID="$(getFTLPID)"
+  if [ "$FTL_PID" -eq "-1" ]; then
+    return 1
+  else
+    echo 0
+  fi
 }
 
 
@@ -21,8 +52,6 @@ start() {
   else
     # Touch files to ensure they exist (create if non-existing, preserve if existing)
     mkdir -pm 0755 /run/pihole
-    [ ! -f /run/pihole-FTL.pid ] && install -m 644 -o pihole -g pihole /dev/null /run/pihole-FTL.pid
-    [ ! -f /run/pihole-FTL.port ] && install -m 644 -o pihole -g pihole /dev/null /run/pihole-FTL.port
     [ ! -f /var/log/pihole-FTL.log ] && install -m 644 -o pihole -g pihole /dev/null /var/log/pihole-FTL.log
     [ ! -f /var/log/pihole.log ] && install -m 644 -o pihole -g pihole /dev/null /var/log/pihole.log
     [ ! -f /etc/pihole/dhcp.leases ] && install -m 644 -o pihole -g pihole /dev/null /etc/pihole/dhcp.leases
@@ -47,7 +76,7 @@ start() {
 # Stop the service
 stop() {
   if is_running; then
-    pkill -xo "pihole-FTL"
+    pkill -F "${FTL_PID_FILE}"
     for i in 1 2 3 4 5; do
       if ! is_running; then
         break
@@ -60,8 +89,8 @@ stop() {
 
     if is_running; then
       echo "Not stopped; may still be shutting down or shutdown may have failed, killing now"
-      pkill -xo -9 "pihole-FTL"
-      exit 1
+      pkill -9 -F "${FTL_PID_FILE}"
+      exit 0
     else
       echo "Stopped"
     fi
@@ -69,7 +98,7 @@ stop() {
     echo "Not running"
   fi
   # Cleanup
-  rm -f /run/pihole/FTL.sock /dev/shm/FTL-*
+  rm -f /run/pihole/FTL.sock /dev/shm/FTL-* "${FTL_PID_FILE}"
   echo
 }
 

automated install/basic-install.sh

@@ -259,29 +259,6 @@ os_check() {
     fi
 }
 
-# This function waits for dpkg to unlock, which signals that the previous apt-get command has finished.
-test_dpkg_lock() {
-    i=0
-    printf "  %b Waiting for package manager to finish (up to 30 seconds)\\n" "${INFO}"
-    # fuser is a program to show which processes use the named files, sockets, or filesystems
-    # So while the lock is held,
-    while fuser /var/lib/dpkg/lock >/dev/null 2>&1
-    do
-        # we wait half a second,
-        sleep 0.5
-        # increase the iterator,
-        ((i=i+1))
-        # exit if waiting for more then 30 seconds
-        if [[ $i -gt 60 ]]; then
-            printf "  %b %bError: Could not verify package manager finished and released lock. %b\\n" "${CROSS}" "${COL_LIGHT_RED}" "${COL_NC}"
-            printf "       Attempt to install packages manually and retry.\\n"
-            exit 1;
-        fi
-    done
-    # and then report success once dpkg is unlocked.
-    return 0
-}
-
 # Compatibility
 package_manager_detect() {
     # First check to see if apt-get is installed.
@@ -325,6 +302,22 @@ package_manager_detect() {
         # and config file
         LIGHTTPD_CFG="lighttpd.conf.debian"
 
+        # This function waits for dpkg to unlock, which signals that the previous apt-get command has finished.
+        test_dpkg_lock() {
+            i=0
+            # fuser is a program to show which processes use the named files, sockets, or filesystems
+            # So while the lock is held,
+            while fuser /var/lib/dpkg/lock >/dev/null 2>&1
+            do
+                # we wait half a second,
+                sleep 0.5
+                # increase the iterator,
+                ((i=i+1))
+            done
+            # and then report success once dpkg is unlocked.
+            return 0
+        }
+
     # If apt-get is not found, check for rpm.
     elif is_command rpm ; then
         # Then check if dnf or yum is the package manager

automated install/uninstall.sh

@@ -11,9 +11,10 @@
 source "/opt/pihole/COL_TABLE"
 
 while true; do
-    read -rp "  ${QST} Are you sure you would like to remove ${COL_WHITE}Pi-hole${COL_NC}? [y/N] " answer
-    case ${answer} in
+    read -rp "  ${QST} Are you sure you would like to remove ${COL_WHITE}Pi-hole${COL_NC}? [y/N] " yn
+    case ${yn} in
         [Yy]* ) break;;
+        [Nn]* ) echo -e "${OVER}  ${COL_LIGHT_GREEN}Uninstall has been canceled${COL_NC}"; exit 0;;
         * ) echo -e "${OVER}  ${COL_LIGHT_GREEN}Uninstall has been canceled${COL_NC}"; exit 0;;
     esac
 done
@@ -75,8 +76,8 @@ removeAndPurge() {
     for i in "${DEPS[@]}"; do
         if package_check "${i}" > /dev/null; then
             while true; do
-                read -rp "  ${QST} Do you wish to remove ${COL_WHITE}${i}${COL_NC} from your system? [Y/N] " answer
-                case ${answer} in
+                read -rp "  ${QST} Do you wish to remove ${COL_WHITE}${i}${COL_NC} from your system? [Y/N] " yn
+                case ${yn} in
                     [Yy]* )
                         echo -ne "  ${INFO} Removing ${i}...";
                         ${SUDO} "${PKG_REMOVE[@]}" "${i}" &> /dev/null;
@@ -214,8 +215,8 @@ while true; do
         echo -n "${i} "
     done
     echo "${COL_NC}"
-    read -rp "  ${QST} Do you wish to go through each dependency for removal? (Choosing No will leave all dependencies installed) [Y/n] " answer
-    case ${answer} in
+    read -rp "  ${QST} Do you wish to go through each dependency for removal? (Choosing No will leave all dependencies installed) [Y/n] " yn
+    case ${yn} in
         [Yy]* ) removeAndPurge; break;;
         [Nn]* ) removeNoPurge; break;;
         * ) removeAndPurge; break;;

pihole

@@ -21,7 +21,7 @@ readonly FTL_PID_FILE="/run/pihole-FTL.pid"
 readonly colfile="${PI_HOLE_SCRIPT_DIR}/COL_TABLE"
 source "${colfile}"
 
-utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
+readonly utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
 source "${utilsfile}"
 
 webpageFunc() {
@@ -226,7 +226,7 @@ Time:
       fi
 
       local str="Pi-hole Disabled"
-      addOrEditKeyValPair "${setupVars}" "BLOCKING_ENABLED" "false"
+      addOrEditKeyValPair "BLOCKING_ENABLED" "false" "${setupVars}"
     fi
   else
     # Enable Pi-hole
@@ -238,7 +238,7 @@ Time:
     echo -e "  ${INFO} Enabling blocking"
     local str="Pi-hole Enabled"
 
-    addOrEditKeyValPair "${setupVars}" "BLOCKING_ENABLED" "true"
+    addOrEditKeyValPair "BLOCKING_ENABLED" "true" "${setupVars}"
   fi
 
   restartDNS reload-lists
@@ -260,8 +260,8 @@ Options:
     exit 0
   elif [[ "${1}" == "off" ]]; then
     # Disable logging
-    removeKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
-    addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "false"
+    sed -i 's/^log-queries/#log-queries/' /etc/dnsmasq.d/01-pihole.conf
+    addOrEditKeyValPair "QUERY_LOGGING" "false" "${setupVars}"
     if [[ "${2}" != "noflush" ]]; then
       # Flush logs
       "${PI_HOLE_BIN_DIR}"/pihole -f
@@ -270,8 +270,8 @@ Options:
     local str="Logging has been disabled!"
   elif [[ "${1}" == "on" ]]; then
     # Enable logging
-    addKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
-    addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "true"
+    sed -i 's/^#log-queries/log-queries/' /etc/dnsmasq.d/01-pihole.conf
+    addOrEditKeyValPair "QUERY_LOGGING" "true" "${setupVars}"
     echo -e "  ${INFO} Enabling logging..."
     local str="Logging has been enabled!"
   else
@@ -365,7 +365,7 @@ statusFunc() {
     # Enable blocking
     "${PI_HOLE_BIN_DIR}"/pihole enable
   fi
-exit 0
+
 }
 
 tailFunc() {
@@ -496,38 +496,8 @@ if [[ $# = 0 ]]; then
   helpFunc
 fi
 
-# functions that do not require sudo power
 case "${1}" in
   "-h" | "help" | "--help"      ) helpFunc;;
-  "-v" | "version"              ) versionFunc "$@";;
-  "-c" | "chronometer"          ) chronometerFunc "$@";;
-  "-q" | "query"                ) queryFunc "$@";;
-  "status"                      ) statusFunc "$2";;
-  "-t" | "tail"                 ) tailFunc "$2";;
-  "tricorder"                   ) tricorderFunc;;
-
-  # we need to add all arguments that require sudo power to not trigger the * argument
-  "-w" | "whitelist"            ) ;;
-  "-b" | "blacklist"            ) ;;
-  "--wild" | "wildcard"         ) ;;
-  "--regex" | "regex"           ) ;;
-  "--white-regex" | "white-regex" ) ;;
-  "--white-wild" | "white-wild"   ) ;;
-  "-f" | "flush"                ) ;;
-  "-up" | "updatePihole"        ) ;;
-  "-r"  | "reconfigure"         ) ;;
-  "-g" | "updateGravity"        ) ;;
-  "-l" | "logging"              ) ;;
-  "uninstall"                   ) ;;
-  "enable"                      ) ;;
-  "disable"                     ) ;;
-  "-d" | "debug"                ) ;;
-  "restartdns"                  ) ;;
-  "-a" | "admin"                ) ;;
-  "checkout"                    ) ;;
-  "updatechecker"               ) ;;
-  "arpflush"                    ) ;;
-  *                             ) helpFunc;;
 esac
 
 # Must be root to use this tool
@@ -554,13 +524,21 @@ case "${1}" in
   "-up" | "updatePihole"        ) updatePiholeFunc "$@";;
   "-r"  | "reconfigure"         ) reconfigurePiholeFunc;;
   "-g" | "updateGravity"        ) updateGravityFunc "$@";;
+  "-c" | "chronometer"          ) chronometerFunc "$@";;
+  "-h" | "help"                 ) helpFunc;;
+  "-v" | "version"              ) versionFunc "$@";;
+  "-q" | "query"                ) queryFunc "$@";;
   "-l" | "logging"              ) piholeLogging "$@";;
   "uninstall"                   ) uninstallFunc;;
   "enable"                      ) piholeEnable 1;;
   "disable"                     ) piholeEnable 0 "$2";;
+  "status"                      ) statusFunc "$2";;
   "restartdns"                  ) restartDNS "$2";;
   "-a" | "admin"                ) webpageFunc "$@";;
+  "-t" | "tail"                 ) tailFunc "$2";;
   "checkout"                    ) piholeCheckoutFunc "$@";;
+  "tricorder"                   ) tricorderFunc;;
   "updatechecker"               ) updateCheckFunc "$@";;
   "arpflush"                    ) arpFunc "$@";;
+  *                             ) helpFunc;;
 esac

test/test_any_utils.py

@@ -2,33 +2,15 @@ def test_key_val_replacement_works(host):
     ''' Confirms addOrEditKeyValPair provides the expected output '''
     host.run('''
     source /opt/pihole/utils.sh
-    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1"
-    addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
-    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value3"
-    addOrEditKeyValPair "./testoutput" "KEY_FOUR" "value4"
-    addKey "./testoutput" "KEY_FIVE_NO_VALUE"
-    addKey "./testoutput" "KEY_FIVE_NO_VALUE"
+    addOrEditKeyValPair "KEY_ONE" "value1" "./testoutput"
+    addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput"
+    addOrEditKeyValPair "KEY_ONE" "value3" "./testoutput"
+    addOrEditKeyValPair "KEY_FOUR" "value4" "./testoutput"
     ''')
     output = host.run('''
     cat ./testoutput
     ''')
-    expected_stdout = 'KEY_ONE=value3\nKEY_TWO=value2\nKEY_FOUR=value4\nKEY_FIVE_NO_VALUE\n'
-    assert expected_stdout == output.stdout
-
-
-def test_key_val_removal_works(host):
-    ''' Confirms removeKey provides the expected output '''
-    host.run('''
-    source /opt/pihole/utils.sh
-    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1"
-    addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
-    addOrEditKeyValPair "./testoutput" "KEY_THREE" "value3"
-    removeKey "./testoutput" "KEY_TWO"
-    ''')
-    output = host.run('''
-    cat ./testoutput
-    ''')
-    expected_stdout = 'KEY_ONE=value1\nKEY_THREE=value3\n'
+    expected_stdout = 'KEY_ONE=value3\nKEY_TWO=value2\nKEY_FOUR=value4\n'
     assert expected_stdout == output.stdout