From 3d01e4d0cfffbfe61c03d4591ac9020917e9cada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Wed, 21 Sep 2022 09:24:44 +0200 Subject: [PATCH 1/3] No detour - use pihole-FTL.conf to get the API port number MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- advanced/Scripts/utils.sh | 47 +++++++++------------------ advanced/Templates/pihole-FTL.service | 6 ++-- pihole | 5 ++- test/test_any_utils.py | 42 ++++++++++++------------ 4 files changed, 42 insertions(+), 58 deletions(-) diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh index a9e05692..511dfc13 100755 --- a/advanced/Scripts/utils.sh +++ b/advanced/Scripts/utils.sh @@ -32,8 +32,8 @@ addOrEditKeyValPair() { local value="${3}" if grep -q "^${key}=" "${file}"; then - # Key already exists in file, modify the value - sed -i "/^${key}=/c\\${key}=${value}" "${file}" + # 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}" @@ -52,8 +52,8 @@ addKey(){ local key="${2}" if ! grep -q "^${key}" "${file}"; then - # Key does not exist, add it. - echo "${key}" >> "${file}" + # Key does not exist, add it. + echo "${key}" >> "${file}" fi } @@ -70,47 +70,32 @@ removeKey() { sed -i "/^${key}/d" "${file}" } -####################### -# returns path of FTL's port file -####################### -getFTLAPIPortFile() { - local FTLCONFFILE="/etc/pihole/pihole-FTL.conf" - local DEFAULT_PORT_FILE="/run/pihole-FTL.port" - local FTL_APIPORT_FILE - - if [ -s "${FTLCONFFILE}" ]; then - # if PORTFILE is not set in pihole-FTL.conf, use the default path - FTL_APIPORT_FILE="$({ grep '^PORTFILE=' "${FTLCONFFILE}" || echo "${DEFAULT_PORT_FILE}"; } | cut -d'=' -f2-)" - else - # if there is no pihole-FTL.conf, use the default path - FTL_APIPORT_FILE="${DEFAULT_PORT_FILE}" - fi - - echo "${FTL_APIPORT_FILE}" -} - ####################### -# returns FTL's current telnet API port based on the content of the pihole-FTL.port file +# returns FTL's current telnet API port based on the setting in /etc/pihole-FTL.conf # # Takes one argument: path to pihole-FTL.port # Example getFTLAPIPort "/run/pihole-FTL.port" ####################### getFTLAPIPort(){ - local PORTFILE="${1}" + local FTLCONFFILE="/etc/pihole/pihole-FTL.conf" local DEFAULT_FTL_PORT=4711 local ftl_api_port - if [ -s "$PORTFILE" ]; then - # -s: FILE exists and has a size greater than zero - ftl_api_port=$(cat "${PORTFILE}") + if [ -s "$FTLCONFFILE" ]; then + # if FTLPORT is not set in pihole-FTL.conf, use the default port + ftl_api_port="$({ grep '^FTLPORT=' "${FTLCONFFILE}" || echo "${DEFAULT_FTL_PORT}"; } | cut -d'=' -f2-)" # 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 + expr "${ftl_api_port}" : "[^[:digit:]]" > /dev/null && unset ftl_api_port + else + # if there is no pihole-FTL.conf, use the default port + ftl_api_port="${DEFAULT_FTL_PORT}" fi - # echo the port found in the portfile or default to the default port - echo "${ftl_api_port:=$DEFAULT_FTL_PORT}" + # If the ftl_api_port contained malicious stuff, substitute with -1 + ftl_api_port=${ftl_api_port:=-1} + echo "${ftl_api_port}" } ####################### diff --git a/advanced/Templates/pihole-FTL.service b/advanced/Templates/pihole-FTL.service index 46e5c1f2..bc1b1d20 100644 --- a/advanced/Templates/pihole-FTL.service +++ b/advanced/Templates/pihole-FTL.service @@ -9,7 +9,7 @@ # Description: Enable service provided by pihole-FTL daemon ### END INIT INFO -#source utils.sh for getFTLPIDFile(), getFTLPID (), getFTLAPIPortFile() +#source utils.sh for getFTLPIDFile(), getFTLPID () PI_HOLE_SCRIPT_DIR="/opt/pihole" utilsfile="${PI_HOLE_SCRIPT_DIR}/utils.sh" . "${utilsfile}" @@ -31,7 +31,6 @@ start() { # Touch files to ensure they exist (create if non-existing, preserve if existing) mkdir -pm 0755 /run/pihole /var/log/pihole [ ! -f "${FTL_PID_FILE}" ] && install -D -m 644 -o pihole -g pihole /dev/null "${FTL_PID_FILE}" - [ ! -f "${FTL_PORT_FILE}" ] && install -D -m 644 -o pihole -g pihole /dev/null "${FTL_PORT_FILE}" [ ! -f /var/log/pihole/FTL.log ] && install -m 644 -o pihole -g pihole /dev/null /var/log/pihole/FTL.log [ ! -f /var/log/pihole/pihole.log ] && install -m 640 -o pihole -g pihole /dev/null /var/log/pihole/pihole.log [ ! -f /etc/pihole/dhcp.leases ] && install -m 644 -o pihole -g pihole /dev/null /etc/pihole/dhcp.leases @@ -91,7 +90,7 @@ stop() { echo "Not running" fi # Cleanup - rm -f /run/pihole/FTL.sock /dev/shm/FTL-* "${FTL_PID_FILE}" "${FTL_PORT_FILE}" + rm -f /run/pihole/FTL.sock /dev/shm/FTL-* "${FTL_PID_FILE}" echo } @@ -111,7 +110,6 @@ status() { # Get file paths FTL_PID_FILE="$(getFTLPIDFile)" -FTL_PORT_FILE="$(getFTLAPIPortFile)" # Get FTL's current PID FTL_PID="$(getFTLPID ${FTL_PID_FILE})" diff --git a/pihole b/pihole index 1047d152..aad83451 100755 --- a/pihole +++ b/pihole @@ -303,14 +303,13 @@ analyze_ports() { statusFunc() { # Determine if there is pihole-FTL service is listening - local pid port ftl_api_port ftl_pid_file ftl_apiport_file + local pid port ftl_api_port ftl_pid_file ftl_pid_file="$(getFTLPIDFile)" pid="$(getFTLPID ${ftl_pid_file})" - ftl_apiport_file="${getFTLAPIPortFile}" - ftl_api_port="$(getFTLAPIPort ${ftl_apiport_file})" + ftl_api_port="$(getFTLAPIPort)" if [[ "$pid" -eq "-1" ]]; then case "${1}" in "web") echo "-1";; diff --git a/test/test_any_utils.py b/test/test_any_utils.py index a2604dc2..6a1146ee 100644 --- a/test/test_any_utils.py +++ b/test/test_any_utils.py @@ -62,50 +62,52 @@ def test_key_removal_works(host): assert expected_stdout == output.stdout -def test_getFTLAPIPortFile_default(host): - """Confirms getFTLAPIPortFile returns the default API port file path""" - output = host.run( - """ - source /opt/pihole/utils.sh - getFTLAPIPortFile - """ - ) - expected_stdout = "/run/pihole-FTL.port\n" - assert expected_stdout == output.stdout - - def test_getFTLAPIPort_default(host): """Confirms getFTLAPIPort returns the default API port""" output = host.run( """ source /opt/pihole/utils.sh - getFTLAPIPort "/run/pihole-FTL.port" + getFTLAPIPort """ ) expected_stdout = "4711\n" assert expected_stdout == output.stdout -def test_getFTLAPIPortFile_and_getFTLAPIPort_custom(host): - """Confirms getFTLAPIPort returns a custom API port in a custom PORTFILE location""" +def test_getFTLAPIPort_custom(host): + """Confirms getFTLAPIPort returns a custom API port""" host.run( """ - tmpfile=$(mktemp) - echo "PORTFILE=${tmpfile}" > /etc/pihole/pihole-FTL.conf - echo "1234" > ${tmpfile} + echo "FTLPORT=1234" > /etc/pihole/pihole-FTL.conf """ ) output = host.run( """ source /opt/pihole/utils.sh - FTL_API_PORT_FILE=$(getFTLAPIPortFile) - getFTLAPIPort "${FTL_API_PORT_FILE}" + getFTLAPIPort """ ) expected_stdout = "1234\n" assert expected_stdout == output.stdout +def test_getFTLAPIPort_malicious(host): + """Confirms getFTLAPIPort returns -1 if the setting in pihole-FTL.conf contains non-digits""" + host.run( + """ + echo "FTLPORT=*$ssdfsd" > /etc/pihole/pihole-FTL.conf + """ + ) + output = host.run( + """ + source /opt/pihole/utils.sh + getFTLAPIPort + """ + ) + expected_stdout = "-1\n" + assert expected_stdout == output.stdout + + def test_getFTLPIDFile_default(host): """Confirms getFTLPIDFile returns the default PID file path""" output = host.run( From 25ba68104b1b9c6300d45920514a06c1cccdb516 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Sun, 25 Sep 2022 18:16:20 +0200 Subject: [PATCH 2/3] Remove last traces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- advanced/Scripts/chronometer.sh | 4 +++- advanced/Scripts/piholeDebug.sh | 2 -- advanced/Scripts/utils.sh | 5 +---- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/advanced/Scripts/chronometer.sh b/advanced/Scripts/chronometer.sh index af007994..d69a56d3 100755 --- a/advanced/Scripts/chronometer.sh +++ b/advanced/Scripts/chronometer.sh @@ -14,7 +14,9 @@ LC_NUMERIC=C # Retrieve stats from FTL engine pihole-FTL() { local ftl_port LINE - ftl_port=$(cat /run/pihole-FTL.port 2> /dev/null) + # shellcheck disable=SC1091 + . /opt/pihole/utils.sh + ftl_port=$(getFTLAPIPort) if [[ -n "$ftl_port" ]]; then # Open connection to FTL exec 3<>"/dev/tcp/127.0.0.1/$ftl_port" diff --git a/advanced/Scripts/piholeDebug.sh b/advanced/Scripts/piholeDebug.sh index 91e16850..dbf56709 100755 --- a/advanced/Scripts/piholeDebug.sh +++ b/advanced/Scripts/piholeDebug.sh @@ -126,7 +126,6 @@ PIHOLE_COMMAND="${BIN_DIRECTORY}/pihole" PIHOLE_COLTABLE_FILE="${BIN_DIRECTORY}/COL_TABLE" FTL_PID="${RUN_DIRECTORY}/pihole-FTL.pid" -FTL_PORT="${RUN_DIRECTORY}/pihole-FTL.port" PIHOLE_LOG="${LOG_DIRECTORY}/pihole.log" PIHOLE_LOG_GZIPS="${LOG_DIRECTORY}/pihole.log.[0-9].*" @@ -155,7 +154,6 @@ REQUIRED_FILES=("${PIHOLE_CRON_FILE}" "${PIHOLE_COMMAND}" "${PIHOLE_COLTABLE_FILE}" "${FTL_PID}" -"${FTL_PORT}" "${PIHOLE_LOG}" "${PIHOLE_LOG_GZIPS}" "${PIHOLE_DEBUG_LOG}" diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh index 511dfc13..ef7ad219 100755 --- a/advanced/Scripts/utils.sh +++ b/advanced/Scripts/utils.sh @@ -73,10 +73,7 @@ removeKey() { ####################### # returns FTL's current telnet API port based on the setting in /etc/pihole-FTL.conf -# -# Takes one argument: path to pihole-FTL.port -# Example getFTLAPIPort "/run/pihole-FTL.port" -####################### +######################## getFTLAPIPort(){ local FTLCONFFILE="/etc/pihole/pihole-FTL.conf" local DEFAULT_FTL_PORT=4711 From 276c480f5001465d994dacf6e30d1e1c2d0a3b0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Mon, 26 Sep 2022 23:40:09 +0200 Subject: [PATCH 3/3] Return default port if non-numeric characters are set in pihole-FTL.conf for FTLPORT. FTL does the same in such case and provide the API on 4711 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- advanced/Scripts/utils.sh | 8 +++----- test/test_any_utils.py | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/advanced/Scripts/utils.sh b/advanced/Scripts/utils.sh index ef7ad219..1174fa62 100755 --- a/advanced/Scripts/utils.sh +++ b/advanced/Scripts/utils.sh @@ -82,16 +82,14 @@ getFTLAPIPort(){ if [ -s "$FTLCONFFILE" ]; then # if FTLPORT is not set in pihole-FTL.conf, use the default port ftl_api_port="$({ grep '^FTLPORT=' "${FTLCONFFILE}" || echo "${DEFAULT_FTL_PORT}"; } | cut -d'=' -f2-)" - # 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 + # Exploit prevention: set the port to the default port if there is malicious (non-numeric) + # content set in pihole-FTL.conf + expr "${ftl_api_port}" : "[^[:digit:]]" > /dev/null && ftl_api_port="${DEFAULT_FTL_PORT}" else # if there is no pihole-FTL.conf, use the default port ftl_api_port="${DEFAULT_FTL_PORT}" fi - # If the ftl_api_port contained malicious stuff, substitute with -1 - ftl_api_port=${ftl_api_port:=-1} echo "${ftl_api_port}" } diff --git a/test/test_any_utils.py b/test/test_any_utils.py index 6a1146ee..5b4075d9 100644 --- a/test/test_any_utils.py +++ b/test/test_any_utils.py @@ -92,7 +92,7 @@ def test_getFTLAPIPort_custom(host): def test_getFTLAPIPort_malicious(host): - """Confirms getFTLAPIPort returns -1 if the setting in pihole-FTL.conf contains non-digits""" + """Confirms getFTLAPIPort returns 4711 if the setting in pihole-FTL.conf contains non-digits""" host.run( """ echo "FTLPORT=*$ssdfsd" > /etc/pihole/pihole-FTL.conf @@ -104,7 +104,7 @@ def test_getFTLAPIPort_malicious(host): getFTLAPIPort """ ) - expected_stdout = "-1\n" + expected_stdout = "4711\n" assert expected_stdout == output.stdout