diff --git a/src/args.c b/src/args.c index ca6c481b..4b1d89c8 100644 --- a/src/args.c +++ b/src/args.c @@ -811,12 +811,6 @@ void parse_args(int argc, char* argv[]) exit(EXIT_SUCCESS); } - // Return number of errors on this undocumented flag - if(strcmp(argv[i], "--check-structs") == 0) - { - exit(check_struct_sizes()); - } - // Complain if invalid options have been found if(!ok) { diff --git a/src/args.h b/src/args.h index 2d58584b..fa188f3f 100644 --- a/src/args.h +++ b/src/args.h @@ -27,7 +27,4 @@ const char *cli_over(void) __attribute__ ((pure)); void test_dnsmasq_options(int argc, const char *argv[]); -// defined in dnsmasq_interface.c -int check_struct_sizes(void); - #endif //ARGS_H diff --git a/src/config/password.c b/src/config/password.c index de476c25..c3b3d7f4 100644 --- a/src/config/password.c +++ b/src/config/password.c @@ -12,8 +12,8 @@ #include "log.h" #include "config/config.h" #include "password.h" -// genrandom() -#include +// genrandom() with fallback +#include "daemon.h" // Randomness generator #include "webserver/x509.h" diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 7de2a193..d9884ed6 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -3398,35 +3398,6 @@ void FTL_dnsmasq_log(const char *payload, const int length) unlock_shm(); } -// Check sizes of all important in-memory objects. This routine returns the number of -// errors found (i.e., a return value of 0 is what we want and expect) -int check_struct_sizes(void) -{ - int result = 0; - const size_t time_t_size = sizeof(time_t); - result += check_one_struct("struct conf_item", sizeof(struct conf_item), 72, 52); - result += check_one_struct("struct config", sizeof(struct config), - CONFIG_ELEMENTS*sizeof(struct conf_item), - CONFIG_ELEMENTS*sizeof(struct conf_item)); - result += check_one_struct("queriesData", sizeof(queriesData), 72, 64); - result += check_one_struct("upstreamsData", sizeof(upstreamsData), 640, 628); - result += check_one_struct("clientsData", sizeof(clientsData), 664 + time_t_size, 648 + time_t_size); - result += check_one_struct("domainsData", sizeof(domainsData), 24, 20); - result += check_one_struct("DNSCacheData", sizeof(DNSCacheData), 24, 20); - result += check_one_struct("ednsData", sizeof(ednsData), 76, 76); - result += check_one_struct("overTimeData", sizeof(overTimeData), 24 + time_t_size, 20 + time_t_size); - result += check_one_struct("regexData", sizeof(regexData), 80, 52); - result += check_one_struct("SharedMemory", sizeof(SharedMemory), 24, 12); - result += check_one_struct("ShmSettings", sizeof(ShmSettings), 16, 16); - result += check_one_struct("countersStruct", sizeof(countersStruct), 292, 292); - result += check_one_struct("sqlite3_stmt_vec", sizeof(sqlite3_stmt_vec), 32, 16); - - if(result == 0) - printf("All okay\n"); - - return result; -} - static const char *check_dnsmasq_name(const char *name) { // Special domain name handling diff --git a/src/procps.c b/src/procps.c index 673cc05f..8e3656a4 100644 --- a/src/procps.c +++ b/src/procps.c @@ -15,10 +15,20 @@ // getpid() #include #include +// config +#include "config/config.h" #define PROCESS_NAME "pihole-FTL" +#define PROC_PATH_SIZ 32 -static bool get_process_name(const pid_t pid, char name[16]) +// This function tries to obtain the process name of a given PID +// It returns true on success, false otherwise and stores the process name in +// the given buffer +// The preferred mechanism is to use /proc//exe, but if that fails, we try +// to parse /proc//comm. The latter is not guaranteed to be correct (e.g. +// processes can easily change it themselves using prctl with PR_SET_NAME), but +// it is better than nothing. +static bool get_process_name(const pid_t pid, char name[PROC_PATH_SIZ]) { if(pid == 0) { @@ -27,7 +37,25 @@ static bool get_process_name(const pid_t pid, char name[16]) } // Try to open comm file - char filename[sizeof("/proc/%u/task/%u/comm") + sizeof(int)*3 * 2]; + char filename[sizeof("/proc/%d/exe") + sizeof(int)*3]; + snprintf(filename, sizeof(filename), "/proc/%d/exe", pid); + + // Read link destination + ssize_t len = readlink(filename, name, PROC_PATH_SIZ); + if(len > 0) + { + // If readlink() succeeded, terminate string + name[len] = '\0'; + + // Strip path from name + char *ptr = strrchr(name, '/'); + if(ptr != NULL) + memmove(name, ptr+1, len - (ptr - name)); + + return true; + } + + // If readlink() failed, try to open comm file snprintf(filename, sizeof(filename), "/proc/%d/comm", pid); FILE *f = fopen(filename, "r"); if(f == NULL) @@ -35,13 +63,20 @@ static bool get_process_name(const pid_t pid, char name[16]) // Read name from opened file if(fscanf(f, "%15s", name) != 1) - false; + { + fclose(f); + return false; + } + + // Close file fclose(f); return true; } - +// This function tries to obtain the parent process ID of a given PID +// It returns true on success, false otherwise and stores the parent PID in +// the given pid_t pointer static bool get_process_ppid(const pid_t pid, pid_t *ppid) { // Try to open status file @@ -63,6 +98,9 @@ static bool get_process_ppid(const pid_t pid, pid_t *ppid) return true; } +// This function tries to obtain the process creation time of a given PID +// It returns true on success, false otherwise and stores the creation time in +// the given buffer static bool get_process_creation_time(const pid_t pid, char timestr[TIMESTR_SIZE]) { // Try to open comm file @@ -76,13 +114,73 @@ static bool get_process_creation_time(const pid_t pid, char timestr[TIMESTR_SIZE return true; } +// This function prints an info message about if another FTL process is already +// running. It returns true if another FTL process is already running, false +// otherwise. bool check_running_FTL(void) { DIR *dirPos; struct dirent *entry; const pid_t ourselves = getpid(); - bool process_running = false; + bool already_running = false; + pid_t pid = 0; + // First we try to read the PID file and compare the PID in there with + // our own PID. If the PID file does not exist or does not contain our + // PID, we try to find another FTL process by looking at the process + // list further down. + if(config.files.pid.v.s != NULL) + { + FILE *pidFile = fopen(config.files.pid.v.s, "r"); + if(pidFile != NULL) + { + if(fscanf(pidFile, "%d", &pid) == 1) + { + if(pid == ourselves) + { + log_debug(DEBUG_SHMEM, "PID file contains our own PID"); + } + else + { + // Note: kill(pid, 0) does not send a + // signal, but merely checks if the + // process exists If the process does + // not exist, kill() returns -1 and sets + // errno to ESRCH. However, if the + // process exists, but security + // restrictions tell the system to deny + // its existence, we cannot distinguish + // between the process not existing and + // the process existing but being denied + // to us. In that case, our fallback + // solution below kicks in and iterates + // over /proc instead. + already_running = kill(pid, 0) == 0; + log_debug(DEBUG_SHMEM, "PID file contains PID %d (%s), we are %d", + pid, already_running ? "running" : "dead", ourselves); + } + } + else + { + log_debug(DEBUG_SHMEM, "Failed to parse PID in PID file"); + } + fclose(pidFile); + } + else + { + log_debug(DEBUG_SHMEM, "Failed to open PID file"); + } + } + + // If already_running is true, we are done + if(already_running) + { + log_info("%s is already running (PID %d)!", PROCESS_NAME, pid); + return true; + } + + // If the PID file does not contain our own PID, we try to find a running + // process with the same name as our own process // Open /proc errno = 0; if ((dirPos = opendir("/proc")) == NULL) @@ -95,6 +193,7 @@ bool check_running_FTL(void) // This is much more efficient than iterating over all possible PIDs pid_t last_pid = 0; size_t last_len = 0u; + log_debug(DEBUG_SHMEM, "Reading /proc/[0-9]*"); while ((entry = readdir(dirPos)) != NULL) { // We are only interested in subdirectories of /proc @@ -105,17 +204,19 @@ bool check_running_FTL(void) continue; // Extract PID - const pid_t pid = strtol(entry->d_name, NULL, 10); + pid = strtol(entry->d_name, NULL, 10); + + // Get process name + char name[PROC_PATH_SIZ] = { 0 }; + if(!get_process_name(pid, name)) + continue; + + log_debug(DEBUG_SHMEM, "PID: %d -> name: %s%s", pid, name, pid == ourselves ? " (us)" : ""); // Skip our own process if(pid == ourselves) continue; - // Get process name - char name[16] = { 0 }; - if(!get_process_name(pid, name)) - continue; - // Only process this is this is our own process if(strcasecmp(name, PROCESS_NAME) != 0) continue; @@ -124,27 +225,29 @@ bool check_running_FTL(void) pid_t ppid; if(!get_process_ppid(pid, &ppid)) continue; - char ppid_name[16] = { 0 }; + char ppid_name[PROC_PATH_SIZ] = { 0 }; if(!get_process_name(ppid, ppid_name)) continue; + log_debug(DEBUG_SHMEM, " └ PPID: %d -> name: %s", ppid, ppid_name); + char timestr[TIMESTR_SIZE] = { 0 }; get_process_creation_time(pid, timestr); // If this is the first process we log, add a header - if(!process_running) + if(!already_running) { - process_running = true; + already_running = true; log_info("%s is already running!", PROCESS_NAME); } if(last_pid != ppid) { // Independent process, may be child of init/systemd - log_info("%s (%d) ──> %s (PID %d, started %s)", + log_info("%s (PID %d) ──> %s (PID %d, started %s)", ppid_name, ppid, name, pid, timestr); last_pid = pid; - last_len = snprintf(NULL, 0, "%s (%d) ──> ", ppid_name, ppid); + last_len = snprintf(NULL, 0, "%s (PID %d) ──> ", ppid_name, ppid); } else { @@ -154,9 +257,10 @@ bool check_running_FTL(void) (int)last_len, "", name, pid, timestr); } } + log_debug(DEBUG_SHMEM, "Done reading /proc/[0-9]*"); closedir(dirPos); - return process_running; + return already_running; } bool read_self_memory_status(struct statm_t *result) diff --git a/test/arch_test.sh b/test/arch_test.sh index bb88b75c..9bef260f 100644 --- a/test/arch_test.sh +++ b/test/arch_test.sh @@ -9,13 +9,16 @@ # This file is copyright under the latest version of the EUPL. # Please see LICENSE file for your rights under this license. */ +okay=true + check_libs() { mapfile -t libs < <(readelf -d ./pihole-FTL | grep "Shared library" | grep -oE "\[.*\]") if [[ "${libs[*]}" != "${1}" ]]; then echo "Wrong libraries" - echo "Found: ${libs[*]}" - echo "Expected: ${1}" - exit 1 + echo " Expected: ${1}" + echo " Found: ${libs[*]}" + okay=false + return fi echo "Library checks: OK (using ${#libs[*]} shared libraries)" } @@ -24,9 +27,10 @@ check_machine() { mapfile -t header < <(readelf -h ./pihole-FTL | grep -E "(Class)|(Machine)" | sed "s/.*://;s/ \{2,\}//g;") if [[ "${header[0]}" != "${1}" || "${header[1]}" != "${2}" ]]; then echo "Wrong machine" - echo "Expected: Class: ${1} Machine: ${2}" - echo "Found: Class: ${header[0]} Machine: ${header[1]}" - exit 1 + echo " Expected: Class: ${1} Machine: ${2}" + echo " Found: Class: ${header[0]} Machine: ${header[1]}" + okay=false + return fi echo "Machine checks: OK (${1} binary for ${2})" } @@ -35,9 +39,10 @@ check_CPU_arch() { cpuarch="$(readelf -A ./pihole-FTL | grep "Tag_CPU_arch:" | sed "s/^ *//")" if [[ "${cpuarch}" != "Tag_CPU_arch: ${1}" ]]; then echo "Wrong CPU arch" - echo "Expected: Tag_CPU_arch: ${1}" - echo "Found: ${cpuarch}" - exit 1 + echo " Expected: Tag_CPU_arch: ${1}" + echo " Found: ${cpuarch}" + okay=false + return fi echo "CPU architecture checks: OK (${1})" } @@ -46,9 +51,10 @@ check_FP_arch() { fparch="$(readelf -A ./pihole-FTL | grep "Tag_FP_arch:" | sed "s/^ *//")" if [[ "${fparch}" != "Tag_FP_arch: ${1}" && -n "${1}" ]]; then echo "Wrong FP arch" - echo "Expected: Tag_FP_arch: ${1}" - echo "Found: ${fparch}" - exit 1 + echo " Expected: Tag_FP_arch: ${1}" + echo " Found: ${fparch}" + okay=false + return fi echo "FP architecture checks: OK (${1})" } @@ -57,9 +63,10 @@ check_file() { filedetails="$(file -b pihole-FTL | sed "s/, BuildID[^,]*//g")" if [[ "${filedetails}" != "${1}" ]]; then echo "Wrong binary classification" - echo "Expected: ${1}" - echo "Found: ${filedetails}" - exit 1 + echo " Expected: ${1}" + echo " Found: ${filedetails}" + okay=false + return fi echo "Binary classification checks: OK (${1})" } @@ -68,7 +75,8 @@ check_static() { if readelf -l ./pihole-FTL | grep -q INTERP; then echo "Not a static executable, depends on dynamic interpreter" ldd ./pihole-FTL - exit 1 + okay=false + return fi echo "Static executable check: OK" } @@ -87,12 +95,14 @@ elif [[ "${CI_ARCH}" == "linux/386" ]]; then check_libs "" # No dependency on any shared library is intended check_file "ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), statically linked, with debug_info, not stripped" -elif [[ "${CI_ARCH}" == "linux/arm64/v8" || "${CI_ARCH}" == "linux/arm64" ]]; then +elif [[ "${CI_ARCH}" == "linux/arm/v5" ]]; then - check_machine "ELF64" "AArch64" - check_static # Binary should not rely on any dynamic interpreter - check_libs "" # No dependency on any shared library is intended - check_file "ELF 64-bit LSB shared object, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-aarch64.so.1, for GNU/Linux 3.7.0, with debug_info, not stripped" + check_machine "ELF32" "ARM" + check_libs "[libm.so.6] [librt.so.1] [libgcc_s.so.1] [libpthread.so.0] [libc.so.6] [ld-linux.so.3]" + check_file "ELF 32-bit LSB pie executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.3, for GNU/Linux 3.2.0, with debug_info, not stripped" + + check_CPU_arch "v5TE" + check_FP_arch "" elif [[ "${CI_ARCH}" == "linux/arm/v6" ]]; then @@ -101,8 +111,9 @@ elif [[ "${CI_ARCH}" == "linux/arm/v6" ]]; then check_libs "" # No dependency on any shared library is intended check_file "ELF 32-bit LSB executable, ARM, EABI5 version 1 (SYSV), statically linked, with debug_info, not stripped" - check_CPU_arch "v6" - check_FP_arch "VFPv2" + check_CPU_arch "v6KZ" + # VFPv3 is backwards compatible with VFPv2 + check_FP_arch "VFPv3" elif [[ "${CI_ARCH}" == "linux/arm/v7" ]]; then @@ -112,7 +123,14 @@ elif [[ "${CI_ARCH}" == "linux/arm/v7" ]]; then check_file "ELF 32-bit LSB executable, ARM, EABI5 version 1 (SYSV), statically linked, with debug_info, not stripped" check_CPU_arch "v7" - check_FP_arch "VFPv3-D16" + check_FP_arch "VFPv3" + +elif [[ "${CI_ARCH}" == "linux/arm64/v8" || "${CI_ARCH}" == "linux/arm64" ]]; then + + check_machine "ELF64" "AArch64" + check_static # Binary should not rely on any dynamic interpreter + check_libs "" # No dependency on any shared library is intended + check_file "ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), statically linked, with debug_info, not stripped" elif [[ "${CI_ARCH}" == "linux/riscv64" ]]; then @@ -123,9 +141,14 @@ elif [[ "${CI_ARCH}" == "linux/riscv64" ]]; then else - echo "Invalid job ${CI_ARCH}" + echo "Unknown architecture '${CI_ARCH}'" exit 1 fi +if [[ "${okay}" == "false" ]]; then + echo "Binary checks failed" + exit 1 +fi + exit 0 diff --git a/test/run.sh b/test/run.sh index b907700f..2574e34d 100755 --- a/test/run.sh +++ b/test/run.sh @@ -1,7 +1,7 @@ #!/bin/bash -# Only run tests on x86_* targets (where the CI can natively run the binaries) -if [[ ${CI} == "true" && "${CI_ARCH}" != "x86_"* ]]; then +# Skip tests on targets not supporting them +if [[ ${TEST} == "false" ]]; then echo "Skipping tests (CI_ARCH: ${CI_ARCH})!" exit 0 fi diff --git a/test/test_suite.bats b/test/test_suite.bats index 8a5f5982..75262c3f 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -29,7 +29,7 @@ run bash -c 'su pihole -s /bin/sh -c "/home/pihole/pihole-FTL -f"' printf "%s\n" "${lines[@]}" [[ "${lines[@]}" == *"CRIT: Initialization of shared memory failed."* ]] - [[ "${lines[@]}" == *"INFO: pihole-FTL is already running!"* ]] + [[ "${lines[@]}" == *"INFO: pihole-FTL is already running"* ]] } @test "dnsmasq options as expected" { @@ -969,12 +969,6 @@ [[ ${lines[0]} == *"using ${compiler_version}"* ]] } -@test "Struct sizes are as expected" { - run bash -c './pihole-FTL --check-structs' - printf "%s\n" "${lines[@]}" - [[ $status == 0 ]] -} - @test "No errors on setting busy handlers for the databases" { run bash -c 'grep -c "Cannot set busy handler" /var/log/pihole/FTL.log' printf "%s\n" "${lines[@]}" diff --git a/tools/socket_client.c b/tools/socket_client.c deleted file mode 100644 index 311fb0e6..00000000 --- a/tools/socket_client.c +++ /dev/null @@ -1,85 +0,0 @@ -/* 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. -* -* FTL Engine -* Unix socket connection test program -* -* This file is copyright under the latest version of the EUPL. -* Please see LICENSE file for your rights under this license. */ -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#define BUF 1024 - -int main (int argc, char **argv) { - int socketfd; - char *buffer = malloc (BUF); - struct sockaddr_un address; - ssize_t size; - int ret; - - // Create socket - socketfd = socket(PF_LOCAL, SOCK_STREAM, 0); - if(socketfd <= 0) - { - printf("Error creating socket!\n"); - exit(EXIT_FAILURE); - } - printf("Socket created\n"); - - // Set socket family to local socket (not an Internet socket) - address.sun_family = AF_LOCAL; - - char *command = ">stats"; - strcpy(address.sun_path,"/run/pihole/FTL.sock"); - - int i; - for(i = 1; i < argc; i++) { - // Get command - if(strstr(argv[i], ">") == argv[i]) { - command = argv[i]; - continue; - } - } - - // Connect to the socket provided by pihole-FTL - ret = connect(socketfd, (struct sockaddr *) &address, sizeof (address)); - if (ret != 0) - { - printf("Error establishing connection! %s\n", strerror(errno)); - exit(EXIT_FAILURE); - } - printf("Connection established\n"); - - // As an example, we query the current statistics from FTL through the socket here - sprintf(buffer, command); - send(socketfd, buffer, strlen (buffer), 0); - - // Try to receive data until either recv() fails or we see "--EOM--" - while((size = recv(socketfd, buffer, BUF-1, 0)) > -1) - { - // Print received data to stdout - for(i = 0; i < size; ++i) { - printf("%02x ", (unsigned char) buffer[i]); - } - - // Exit on End Of Message - if((unsigned char) buffer[size-1] == 0xc1) - break; - } - - printf("\n"); - - // Close Unix socket connection - close(socketfd); - return EXIT_SUCCESS; -}