Replace wrapper function calls with direct utils.sh calls. Leave warpper functions until next release as docker currently uses them, and new changes to utils.sh need to be in the master branch before docker can use them

Signed-off-by: Adam Warner <me@adamwarner.co.uk>

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 block lists for exact domain matches
+  -exact              Search the adlists for exact domain matches
-  -all                Return all query matches within a block list
+  -all                Return all query matches within the adlists
   -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 block lists"
+    echo -e "  ${INFO} No ${exact/t/t }results found for ${COL_BOLD}${domainQuery}${COL_NC} within the adlists"
     exit 0
 elif [[ -z "${results[*]}" ]]; then
     # Result found in WL/BL/Wildcards

advanced/Scripts/utils.sh

@@ -1,4 +1,6 @@
-#!/usr/bin/env bash
+#!/usr/bin/env sh
+# shellcheck disable=SC3043 #https://github.com/koalaman/shellcheck/wiki/SC3043#exceptions
+
 # 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.
@@ -15,46 +17,80 @@
 #  - New functions must have a test added for them in test/test_any_utils.py
 
 #######################
-# Takes three arguments key, value, and file.
+# Takes Three arguments: file, key, and value.
+#
 # 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:
-# addOrEditKeyValuePair "BLOCKING_ENABLED" "true" "/etc/pihole/setupVars.conf"
+# addOrEditKeyValPair "/etc/pihole/setupVars.conf" "BLOCKING_ENABLED" "true"
 #######################
 addOrEditKeyValPair() {
-  local key="${1}"
+  local file="${1}"
-  local value="${2}"
+  local key="${2}"
-  local file="${3}"
+  local value="${3}"
+
   if grep -q "^${key}=" "${file}"; then
+      # Key already exists in file, modify the value
       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 -r FTLCONFFILE="/etc/pihole/pihole-FTL.conf"
+  local FTLCONFFILE="/etc/pihole/pihole-FTL.conf"
-  local -r DEFAULT_PORT_FILE="/run/pihole-FTL.port"
+  local DEFAULT_PORT_FILE="/run/pihole-FTL.port"
-  local -r DEFAULT_FTL_PORT=4711
+  local 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=$(<"$PORTFILE")
+    ftl_api_port=$(cat "${PORTFILE}")
     # Exploit prevention: unset the variable if there is malicious content
     # Verify that the value read from the file is numeric    
-    [[ "$ftl_api_port" =~ [^[:digit:]] ]] && unset ftl_api_port
+    expr "$ftl_api_port" : "[^[:digit:]]" > /dev/null && unset ftl_api_port
   fi
 
   # echo the port found in the portfile or default to the default port

advanced/Scripts/webpage.sh

@@ -1,5 +1,7 @@
 #!/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)
@@ -26,6 +28,9 @@ 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}
@@ -51,45 +56,39 @@ Options:
 }
 
 add_setting() {
-    echo "${1}=${2}" >> "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "${1}" "${2}"
 }
 
 delete_setting() {
-    sed -i "/^${1}/d" "${setupVars}"
+    removeKey "${setupVars}" "${1}"
 }
 
 change_setting() {
-    delete_setting "${1}"
+    addOrEditKeyValPair "${setupVars}" "${1}" "${2}"
-    add_setting "${1}" "${2}"
 }
 
 addFTLsetting() {
-    echo "${1}=${2}" >> "${FTLconf}"
+    addOrEditKeyValPair "${FTLconf}" "${1}" "${2}"
 }
 
 deleteFTLsetting() {
-    sed -i "/^${1}/d" "${FTLconf}"
+    removeKey "${FTLconf}" "${1}"
 }
 
 changeFTLsetting() {
-    deleteFTLsetting "${1}"
+    addOrEditKeyValPair "${FTLconf}" "${1}" "${2}"
-    addFTLsetting "${1}" "${2}"
 }
 
 add_dnsmasq_setting() {
-    if [[ "${2}" != "" ]]; then
+    addOrEditKeyValPair "${dnsmasqconfig}" "${1}" "${2}"
-        echo "${1}=${2}" >> "${dnsmasqconfig}"
-    else
-        echo "${1}" >> "${dnsmasqconfig}"
-    fi
 }
 
 delete_dnsmasq_setting() {
-    sed -i "/^${1}/d" "${dnsmasqconfig}"
+    removeKey "${dnsmasqconfig}" "${1}"
 }
 
 SetTemperatureUnit() {
-    change_setting "TEMPERATUREUNIT" "${unit}"
+    addOrEditKeyValPair "${setupVars}" "TEMPERATUREUNIT" "${unit}"
     echo -e "  ${TICK} Set temperature unit to ${unit}"
 }
 
@@ -124,7 +123,7 @@ SetWebPassword() {
         echo ""
 
         if [ "${PASSWORD}" == "" ]; then
-            change_setting "WEBPASSWORD" ""
+            addOrEditKeyValPair "${setupVars}" "WEBPASSWORD" ""
             echo -e "  ${TICK} Password Removed"
             exit 0
         fi
@@ -137,7 +136,7 @@ SetWebPassword() {
         # We do not wrap this in brackets, otherwise BASH will expand any appropriate syntax
         hash=$(HashPassword "$PASSWORD")
         # Save hash to file
-        change_setting "WEBPASSWORD" "${hash}"
+        addOrEditKeyValPair "${setupVars}" "WEBPASSWORD" "${hash}"
         echo -e "  ${TICK} New password set"
     else
         echo -e "  ${CROSS} Passwords don't match. Your password has not been changed"
@@ -148,7 +147,7 @@ SetWebPassword() {
 ProcessDNSSettings() {
     source "${setupVars}"
 
-    delete_dnsmasq_setting "server"
+    removeKey "${dnsmasqconfig}" "server"
 
     COUNTER=1
     while true ; do
@@ -156,34 +155,34 @@ ProcessDNSSettings() {
         if [ -z "${!var}" ]; then
             break;
         fi
-        add_dnsmasq_setting "server" "${!var}"
+        addOrEditKeyValPair "${dnsmasqconfig}" "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
-        add_dnsmasq_setting "server" "127.0.0.1#${LOCAL_DNS_PORT}"
+        addOrEditKeyValPair "${dnsmasqconfig}" "server" "127.0.0.1#${LOCAL_DNS_PORT}"
-        add_setting "PIHOLE_DNS_${COUNTER}" "127.0.0.1#${LOCAL_DNS_PORT}"
+        addOrEditKeyValPair "${setupVars}" "PIHOLE_DNS_${COUNTER}" "127.0.0.1#${LOCAL_DNS_PORT}"
-        delete_setting "LOCAL_DNS_PORT"
+        removeKey "${setupVars}" "LOCAL_DNS_PORT"
     fi
 
-    delete_dnsmasq_setting "domain-needed"
+    removeKey "${dnsmasqconfig}" "domain-needed"
-    delete_dnsmasq_setting "expand-hosts"
+    removeKey "${dnsmasqconfig}" "expand-hosts"
 
     if [[ "${DNS_FQDN_REQUIRED}" == true ]]; then
-        add_dnsmasq_setting "domain-needed"
+        addKey "${dnsmasqconfig}" "domain-needed"
-        add_dnsmasq_setting "expand-hosts"
+        addKey "${dnsmasqconfig}" "expand-hosts"
     fi
 
-    delete_dnsmasq_setting "bogus-priv"
+    removeKey "${dnsmasqconfig}" "bogus-priv"
 
     if [[ "${DNS_BOGUS_PRIV}" == true ]]; then
-        add_dnsmasq_setting "bogus-priv"
+        addKey "${dnsmasqconfig}" "bogus-priv"
     fi
 
-    delete_dnsmasq_setting "dnssec"
+    removeKey "${dnsmasqconfig}" "dnssec"
-    delete_dnsmasq_setting "trust-anchor="
+    removeKey "${dnsmasqconfig}" "trust-anchor"
 
     if [[ "${DNSSEC}" == true ]]; then
         echo "dnssec
@@ -191,24 +190,24 @@ trust-anchor=.,20326,8,2,E06D44B80B8F1D39A95C0B0D7C65D08458E880409BBC68345710423
 " >> "${dnsmasqconfig}"
     fi
 
-    delete_dnsmasq_setting "host-record"
+    removeKey "${dnsmasqconfig}" "host-record"
 
     if [ -n "${HOSTRECORD}" ]; then
-        add_dnsmasq_setting "host-record" "${HOSTRECORD}"
+        addOrEditKeyValPair "${dnsmasqconfig}" "host-record" "${HOSTRECORD}"
     fi
 
     # Setup interface listening behavior of dnsmasq
-    delete_dnsmasq_setting "interface"
+    removeKey "${dnsmasqconfig}" "interface"
-    delete_dnsmasq_setting "local-service"
+    removeKey "${dnsmasqconfig}" "local-service"
-    delete_dnsmasq_setting "except-interface"
+    removeKey "${dnsmasqconfig}" "except-interface"
-    delete_dnsmasq_setting "bind-interfaces"
+    removeKey "${dnsmasqconfig}" "bind-interfaces"
 
     if [[ "${DNSMASQ_LISTENING}" == "all" ]]; then
         # Listen on all interfaces, permit all origins
-        add_dnsmasq_setting "except-interface" "nonexisting"
+        addOrEditKeyValPair "${dnsmasqconfig}" "except-interface" "nonexisting"
     elif [[ "${DNSMASQ_LISTENING}" == "local" ]]; then
         # Listen only on all interfaces, but only local subnets
-        add_dnsmasq_setting "local-service"
+        addKey "${dnsmasqconfig}" "local-service"
     else
         # Options "bind" and "single"
         # Listen only on one interface
@@ -217,30 +216,30 @@ trust-anchor=.,20326,8,2,E06D44B80B8F1D39A95C0B0D7C65D08458E880409BBC68345710423
             PIHOLE_INTERFACE="eth0"
         fi
 
-        add_dnsmasq_setting "interface" "${PIHOLE_INTERFACE}"
+        addOrEditKeyValPair "${dnsmasqconfig}" "interface" "${PIHOLE_INTERFACE}"
 
         if [[ "${DNSMASQ_LISTENING}" == "bind" ]]; then
             # Really bind to interface
-            add_dnsmasq_setting "bind-interfaces"
+            addKey "${dnsmasqconfig}" "bind-interfaces"
         fi
     fi
 
     if [[ "${CONDITIONAL_FORWARDING}" == true ]]; then
         # Convert legacy "conditional forwarding" to rev-server configuration
         # Remove any existing REV_SERVER settings
-        delete_setting "REV_SERVER"
+        removeKey "${setupVars}" "REV_SERVER"
-        delete_setting "REV_SERVER_DOMAIN"
+        removeKey "${setupVars}" "REV_SERVER_DOMAIN"
-        delete_setting "REV_SERVER_TARGET"
+        removeKey "${setupVars}" "REV_SERVER_TARGET"
-        delete_setting "REV_SERVER_CIDR"
+        removeKey "${setupVars}" "REV_SERVER_CIDR"
 
         REV_SERVER=true
-        add_setting "REV_SERVER" "true"
+        addOrEditKeyValPair "${setupVars}" "REV_SERVER" "true"
 
         REV_SERVER_DOMAIN="${CONDITIONAL_FORWARDING_DOMAIN}"
-        add_setting "REV_SERVER_DOMAIN" "${REV_SERVER_DOMAIN}"
+        addOrEditKeyValPair "${setupVars}" "REV_SERVER_DOMAIN" "${REV_SERVER_DOMAIN}"
 
         REV_SERVER_TARGET="${CONDITIONAL_FORWARDING_IP}"
-        add_setting "REV_SERVER_TARGET" "${REV_SERVER_TARGET}"
+        addOrEditKeyValPair "${setupVars}" "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
@@ -267,28 +266,28 @@ trust-anchor=.,20326,8,2,E06D44B80B8F1D39A95C0B0D7C65D08458E880409BBC68345710423
             # shellcheck disable=2001
             REV_SERVER_CIDR="$(sed "s+\\.[0-9]*$+\\.0/24+" <<< "${REV_SERVER_TARGET}")"
         fi
-        add_setting "REV_SERVER_CIDR" "${REV_SERVER_CIDR}"
+        addOrEditKeyValPair "${setupVars}" "REV_SERVER_CIDR" "${REV_SERVER_CIDR}"
 
         # Remove obsolete settings from setupVars.conf
-        delete_setting "CONDITIONAL_FORWARDING"
+        removeKey "${setupVars}" "CONDITIONAL_FORWARDING"
-        delete_setting "CONDITIONAL_FORWARDING_REVERSE"
+        removeKey "${setupVars}" "CONDITIONAL_FORWARDING_REVERSE"
-        delete_setting "CONDITIONAL_FORWARDING_DOMAIN"
+        removeKey "${setupVars}" "CONDITIONAL_FORWARDING_DOMAIN"
-        delete_setting "CONDITIONAL_FORWARDING_IP"
+        removeKey "${setupVars}" "CONDITIONAL_FORWARDING_IP"
     fi
 
-    delete_dnsmasq_setting "rev-server"
+    removeKey "${dnsmasqconfig}" "rev-server"
 
     if [[ "${REV_SERVER}" == true ]]; then
-        add_dnsmasq_setting "rev-server=${REV_SERVER_CIDR},${REV_SERVER_TARGET}"
+        addKey "${dnsmasqconfig}" "rev-server=${REV_SERVER_CIDR},${REV_SERVER_TARGET}"
         if [ -n "${REV_SERVER_DOMAIN}" ]; then
             # Forward local domain names to the CF target, too
-            add_dnsmasq_setting "server=/${REV_SERVER_DOMAIN}/${REV_SERVER_TARGET}"
+            addKey "${dnsmasqconfig}" "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
-            add_dnsmasq_setting "server=//${REV_SERVER_TARGET}"
+            addKey "${dnsmasqconfig}" "server=//${REV_SERVER_TARGET}"
         fi
 
     fi
@@ -303,7 +302,7 @@ trust-anchor=.,20326,8,2,E06D44B80B8F1D39A95C0B0D7C65D08458E880409BBC68345710423
 
 SetDNSServers() {
     # Save setting to file
-    delete_setting "PIHOLE_DNS"
+    removeKey "${setupVars}" "PIHOLE_DNS"
     IFS=',' read -r -a array <<< "${args[2]}"
     for index in "${!array[@]}"
     do
@@ -312,7 +311,7 @@ SetDNSServers() {
         ip="${array[index]//\\#/#}"
 
         if valid_ip "${ip}" || valid_ip6 "${ip}" ; then
-            add_setting "PIHOLE_DNS_$((index+1))" "${ip}"
+            addOrEditKeyValPair "${setupVars}" "PIHOLE_DNS_$((index+1))" "${ip}"
         else
             echo -e "  ${CROSS} Invalid IP has been passed"
             exit 1
@@ -320,30 +319,30 @@ SetDNSServers() {
     done
 
     if [[ "${args[3]}" == "domain-needed" ]]; then
-        change_setting "DNS_FQDN_REQUIRED" "true"
+        addOrEditKeyValPair "${setupVars}" "DNS_FQDN_REQUIRED" "true"
     else
-        change_setting "DNS_FQDN_REQUIRED" "false"
+        addOrEditKeyValPair "${setupVars}" "DNS_FQDN_REQUIRED" "false"
     fi
 
     if [[ "${args[4]}" == "bogus-priv" ]]; then
-        change_setting "DNS_BOGUS_PRIV" "true"
+        addOrEditKeyValPair "${setupVars}" "DNS_BOGUS_PRIV" "true"
     else
-        change_setting "DNS_BOGUS_PRIV" "false"
+        addOrEditKeyValPair "${setupVars}" "DNS_BOGUS_PRIV" "false"
     fi
 
     if [[ "${args[5]}" == "dnssec" ]]; then
-        change_setting "DNSSEC" "true"
+        addOrEditKeyValPair "${setupVars}" "DNSSEC" "true"
     else
-        change_setting "DNSSEC" "false"
+        addOrEditKeyValPair "${setupVars}" "DNSSEC" "false"
     fi
 
     if [[ "${args[6]}" == "rev-server" ]]; then
-        change_setting "REV_SERVER" "true"
+        addOrEditKeyValPair "${setupVars}" "REV_SERVER" "true"
-        change_setting "REV_SERVER_CIDR" "${args[7]}"
+        addOrEditKeyValPair "${setupVars}" "REV_SERVER_CIDR" "${args[7]}"
-        change_setting "REV_SERVER_TARGET" "${args[8]}"
+        addOrEditKeyValPair "${setupVars}" "REV_SERVER_TARGET" "${args[8]}"
-        change_setting "REV_SERVER_DOMAIN" "${args[9]}"
+        addOrEditKeyValPair "${setupVars}" "REV_SERVER_DOMAIN" "${args[9]}"
     else
-        change_setting "REV_SERVER" "false"
+        addOrEditKeyValPair "${setupVars}" "REV_SERVER" "false"
     fi
 
     ProcessDNSSettings
@@ -353,11 +352,11 @@ SetDNSServers() {
 }
 
 SetExcludeDomains() {
-    change_setting "API_EXCLUDE_DOMAINS" "${args[2]}"
+    addOrEditKeyValPair "${setupVars}" "API_EXCLUDE_DOMAINS" "${args[2]}"
 }
 
 SetExcludeClients() {
-    change_setting "API_EXCLUDE_CLIENTS" "${args[2]}"
+    addOrEditKeyValPair "${setupVars}" "API_EXCLUDE_CLIENTS" "${args[2]}"
 }
 
 Poweroff(){
@@ -373,7 +372,7 @@ RestartDNS() {
 }
 
 SetQueryLogOptions() {
-    change_setting "API_QUERY_LOG_SHOW" "${args[2]}"
+    addOrEditKeyValPair "${setupVars}" "API_QUERY_LOG_SHOW" "${args[2]}"
 }
 
 ProcessDHCPSettings() {
@@ -389,19 +388,19 @@ ProcessDHCPSettings() {
 
         if [[ "${PIHOLE_DOMAIN}" == "" ]]; then
             PIHOLE_DOMAIN="lan"
-            change_setting "PIHOLE_DOMAIN" "${PIHOLE_DOMAIN}"
+            addOrEditKeyValPair "${setupVars}" "PIHOLE_DOMAIN" "${PIHOLE_DOMAIN}"
         fi
 
         if [[ "${DHCP_LEASETIME}" == "0" ]]; then
             leasetime="infinite"
         elif [[ "${DHCP_LEASETIME}" == "" ]]; then
             leasetime="24"
-            change_setting "DHCP_LEASETIME" "${leasetime}"
+            addOrEditKeyValPair "${setupVars}" "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"
-            change_setting "DHCP_LEASETIME" "${leasetime}"
+            addOrEditKeyValPair "${setupVars}" "DHCP_LEASETIME" "${leasetime}"
         else
             leasetime="${DHCP_LEASETIME}h"
         fi
@@ -454,24 +453,24 @@ ra-param=*,0,0
 }
 
 EnableDHCP() {
-    change_setting "DHCP_ACTIVE" "true"
+    addOrEditKeyValPair "${setupVars}" "DHCP_ACTIVE" "true"
-    change_setting "DHCP_START" "${args[2]}"
+    addOrEditKeyValPair "${setupVars}" "DHCP_START" "${args[2]}"
-    change_setting "DHCP_END" "${args[3]}"
+    addOrEditKeyValPair "${setupVars}" "DHCP_END" "${args[3]}"
-    change_setting "DHCP_ROUTER" "${args[4]}"
+    addOrEditKeyValPair "${setupVars}" "DHCP_ROUTER" "${args[4]}"
-    change_setting "DHCP_LEASETIME" "${args[5]}"
+    addOrEditKeyValPair "${setupVars}" "DHCP_LEASETIME" "${args[5]}"
-    change_setting "PIHOLE_DOMAIN" "${args[6]}"
+    addOrEditKeyValPair "${setupVars}" "PIHOLE_DOMAIN" "${args[6]}"
-    change_setting "DHCP_IPv6" "${args[7]}"
+    addOrEditKeyValPair "${setupVars}" "DHCP_IPv6" "${args[7]}"
-    change_setting "DHCP_rapid_commit" "${args[8]}"
+    addOrEditKeyValPair "${setupVars}" "DHCP_rapid_commit" "${args[8]}"
 
     # Remove possible old setting from file
-    delete_dnsmasq_setting "dhcp-"
+    removeKey "${dnsmasqconfig}" "dhcp-"
-    delete_dnsmasq_setting "quiet-dhcp"
+    removeKey "${dnsmasqconfig}" "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
-    add_dnsmasq_setting "dhcp-name-match=set:hostname-ignore,wpad
+    addKey "${dnsmasqconfig}" "dhcp-name-match=set:hostname-ignore,wpad
 dhcp-name-match=set:hostname-ignore,localhost
 dhcp-ignore-names=tag:hostname-ignore"
 
@@ -481,11 +480,11 @@ dhcp-ignore-names=tag:hostname-ignore"
 }
 
 DisableDHCP() {
-    change_setting "DHCP_ACTIVE" "false"
+    addOrEditKeyValPair "${setupVars}" "DHCP_ACTIVE" "false"
 
     # Remove possible old setting from file
-    delete_dnsmasq_setting "dhcp-"
+    removeKey "${dnsmasqconfig}" "dhcp-"
-    delete_dnsmasq_setting "quiet-dhcp"
+    removeKey "${dnsmasqconfig}" "quiet-dhcp"
 
     ProcessDHCPSettings
 
@@ -493,11 +492,11 @@ DisableDHCP() {
 }
 
 SetWebUILayout() {
-    change_setting "WEBUIBOXEDLAYOUT" "${args[2]}"
+    addOrEditKeyValPair "${setupVars}" "WEBUIBOXEDLAYOUT" "${args[2]}"
 }
 
 SetWebUITheme() {
-    change_setting "WEBTHEME" "${args[2]}"
+    addOrEditKeyValPair "${setupVars}" "WEBTHEME" "${args[2]}"
 }
 
 CheckUrl(){
@@ -592,10 +591,10 @@ Options:
             exit 0
         fi
 
-        change_setting "ADMIN_EMAIL" "${args[2]}"
+        addOrEditKeyValPair "${setupVars}" "ADMIN_EMAIL" "${args[2]}"
         echo -e "  ${TICK} Setting admin contact to ${args[2]}"
     else
-        change_setting "ADMIN_EMAIL" ""
+        addOrEditKeyValPair "${setupVars}" "ADMIN_EMAIL" ""
         echo -e "  ${TICK} Removing admin contact"
     fi
 }
@@ -619,16 +618,16 @@ Interfaces:
 
     if [[ "${args[2]}" == "all" ]]; then
         echo -e "  ${INFO} Listening on all interfaces, permitting all origins. Please use a firewall!"
-        change_setting "DNSMASQ_LISTENING" "all"
+        addOrEditKeyValPair "${setupVars}" "DNSMASQ_LISTENING" "all"
     elif [[ "${args[2]}" == "local" ]]; then
         echo -e "  ${INFO} Listening on all interfaces, permitting origins from one hop away (LAN)"
-        change_setting "DNSMASQ_LISTENING" "local"
+        addOrEditKeyValPair "${setupVars}" "DNSMASQ_LISTENING" "local"
     elif [[ "${args[2]}" == "bind" ]]; then
         echo -e "  ${INFO} Binding on interface ${PIHOLE_INTERFACE}"
-        change_setting "DNSMASQ_LISTENING" "bind"
+        addOrEditKeyValPair "${setupVars}" "DNSMASQ_LISTENING" "bind"
     else
         echo -e "  ${INFO} Listening only on interface ${PIHOLE_INTERFACE}"
-        change_setting "DNSMASQ_LISTENING" "single"
+        addOrEditKeyValPair "${setupVars}" "DNSMASQ_LISTENING" "single"
     fi
 
     # Don't restart DNS server yet because other settings
@@ -698,7 +697,7 @@ clearAudit()
 SetPrivacyLevel() {
     # Set privacy level. Minimum is 0, maximum is 3
     if [ "${args[2]}" -ge 0 ] && [ "${args[2]}" -le 3 ]; then
-        changeFTLsetting "PRIVACYLEVEL" "${args[2]}"
+        addOrEditKeyValPair "${FTLconf}" "PRIVACYLEVEL" "${args[2]}"
         pihole restartdns reload-lists
     fi
 }
@@ -816,7 +815,7 @@ SetRateLimit() {
 
     # Set rate-limit setting inf valid
     if [ "${rate_limit_count}" -ge 0 ] && [ "${rate_limit_interval}" -ge 0 ]; then
-        changeFTLsetting "RATE_LIMIT" "${rate_limit_count}/${rate_limit_interval}"
+        addOrEditKeyValPair "${FTLconf}" "RATE_LIMIT" "${rate_limit_count}/${rate_limit_interval}"
     fi
 
     # Restart FTL to update rate-limit settings only if $reload not false

automated install/basic-install.sh

@@ -259,6 +259,29 @@ 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.
@@ -302,22 +325,6 @@ 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,10 +11,9 @@
 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] " yn
+    read -rp "  ${QST} Are you sure you would like to remove ${COL_WHITE}Pi-hole${COL_NC}? [y/N] " answer
-    case ${yn} in
+    case ${answer} 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
@@ -76,8 +75,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] " yn
+                read -rp "  ${QST} Do you wish to remove ${COL_WHITE}${i}${COL_NC} from your system? [Y/N] " answer
-                case ${yn} in
+                case ${answer} in
                     [Yy]* )
                         echo -ne "  ${INFO} Removing ${i}...";
                         ${SUDO} "${PKG_REMOVE[@]}" "${i}" &> /dev/null;
@@ -215,8 +214,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] " yn
+    read -rp "  ${QST} Do you wish to go through each dependency for removal? (Choosing No will leave all dependencies installed) [Y/n] " answer
-    case ${yn} in
+    case ${answer} 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}"
 
-readonly utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
+utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh"
 source "${utilsfile}"
 
 webpageFunc() {
@@ -226,7 +226,7 @@ Time:
       fi
 
       local str="Pi-hole Disabled"
-      addOrEditKeyValPair "BLOCKING_ENABLED" "false" "${setupVars}"
+      addOrEditKeyValPair "${setupVars}" "BLOCKING_ENABLED" "false"
     fi
   else
     # Enable Pi-hole
@@ -238,7 +238,7 @@ Time:
     echo -e "  ${INFO} Enabling blocking"
     local str="Pi-hole Enabled"
 
-    addOrEditKeyValPair "BLOCKING_ENABLED" "true" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "BLOCKING_ENABLED" "true"
   fi
 
   restartDNS reload-lists
@@ -260,8 +260,8 @@ Options:
     exit 0
   elif [[ "${1}" == "off" ]]; then
     # Disable logging
-    sed -i 's/^log-queries/#log-queries/' /etc/dnsmasq.d/01-pihole.conf
+    removeKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
-    addOrEditKeyValPair "QUERY_LOGGING" "false" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "false"
     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
-    sed -i 's/^#log-queries/log-queries/' /etc/dnsmasq.d/01-pihole.conf
+    addKey /etc/dnsmasq.d/01-pihole.conf "log-queries"
-    addOrEditKeyValPair "QUERY_LOGGING" "true" "${setupVars}"
+    addOrEditKeyValPair "${setupVars}" "QUERY_LOGGING" "true"
     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,8 +496,38 @@ 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
@@ -524,21 +554,13 @@ 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,15 +2,33 @@ def test_key_val_replacement_works(host):
     ''' Confirms addOrEditKeyValPair provides the expected output '''
     host.run('''
     source /opt/pihole/utils.sh
-    addOrEditKeyValPair "KEY_ONE" "value1" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value1"
-    addOrEditKeyValPair "KEY_TWO" "value2" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_TWO" "value2"
-    addOrEditKeyValPair "KEY_ONE" "value3" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_ONE" "value3"
-    addOrEditKeyValPair "KEY_FOUR" "value4" "./testoutput"
+    addOrEditKeyValPair "./testoutput" "KEY_FOUR" "value4"
+    addKey "./testoutput" "KEY_FIVE_NO_VALUE"
+    addKey "./testoutput" "KEY_FIVE_NO_VALUE"
     ''')
     output = host.run('''
     cat ./testoutput
     ''')
-    expected_stdout = 'KEY_ONE=value3\nKEY_TWO=value2\nKEY_FOUR=value4\n'
+    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'
     assert expected_stdout == output.stdout