diff --git a/Makefile b/Makefile index c73b560e..12b8cb43 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ DNSMASQOPTS = -DHAVE_DNSSEC -DHAVE_DNSSEC_STATIC # Flags for compiling with libidn2: -DHAVE_LIBIDN2 -DIDN2_VERSION_NUMBER=0x02000003 FTLDEPS = FTL.h routines.h version.h api.h dnsmasq_interface.h shmem.h -FTLOBJ = main.o memory.o log.o daemon.o datastructure.o signals.o socket.o request.o grep.o setupVars.o args.o gc.o config.o database.o msgpack.o api.o dnsmasq_interface.o resolve.o regex.o shmem.o capabilities.o networktable.o overTime.o gravity.o +FTLOBJ = main.o memory.o log.o daemon.o datastructure.o signals.o socket.o request.o files.o setupVars.o args.o gc.o config.o database.o msgpack.o api.o dnsmasq_interface.o resolve.o regex.o shmem.o capabilities.o networktable.o overTime.o gravity.o DNSMASQDEPS = config.h dhcp-protocol.h dns-protocol.h radv-protocol.h dhcp6-protocol.h dnsmasq.h ip6addr.h metrics.h ../dnsmasq_interface.h DNSMASQOBJ = arp.o dbus.o domain.o lease.o outpacket.o rrfilter.o auth.o dhcp6.o edns0.o log.o poll.o slaac.o blockdata.o dhcp.o forward.o loop.o radv.o tables.o bpf.o dhcp-common.o helper.o netlink.o rfc1035.o tftp.o cache.o dnsmasq.o inotify.o network.o rfc2131.o util.o conntrack.o dnssec.o ipset.o option.o rfc3315.o crypto.o dump.o ubus.o metrics.o diff --git a/database.c b/database.c index 2be5c301..36cee33b 100644 --- a/database.c +++ b/database.c @@ -196,6 +196,15 @@ static bool db_create(void) if(!create_network_table()) return false; + // Done initializing the database + // Close database handle + dbclose(); + + // Explicitly set permissions to 0644 + // 644 = u+w u+r g+r o+r + const mode_t mode = S_IWUSR | S_IRUSR | S_IRGRP | S_IROTH; + chmod_file(FTLfiles.db, mode); + return true; } diff --git a/grep.c b/files.c similarity index 74% rename from grep.c rename to files.c index 628940a3..bd1becc4 100644 --- a/grep.c +++ b/files.c @@ -3,7 +3,7 @@ * Network-wide ad blocking via your own hardware. * * FTL Engine -* grep-like routines +* File operation routines * * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ @@ -125,3 +125,33 @@ void check_blocking_status(void) logg("Blocking status is %s", message); } + +// chmod_file() changes the file mode bits of a given file (relative +// to the directory file descriptor) according to mode. mode is an +// octal number representing the bit pattern for the new mode bits +bool chmod_file(const char *filename, const mode_t mode) +{ + if(chmod(filename, mode) < 0) + { + logg("WARNING: chmod(%s, %d): chmod() failed: %s (%d)", filename, mode, strerror(errno), errno); + return false; + } + + struct stat st; + if(stat(filename, &st) < 0) + { + logg("WARNING: chmod(%s, %d): stat() failed: %s (%d)", filename, mode, strerror(errno), errno); + return false; + } + + // We need to apply a bitmask on st.st_mode as the upper bits may contain random data + // 0x1FF = 0b111_111_111 corresponding to the three-digit octal mode number + if((st.st_mode & 0x1FF) != mode) + { + logg("WARNING: chmod(%s, %d): Verification failed, %d != %d", filename, mode, st.st_mode, mode); + return false; + } + + return true; +} + diff --git a/routines.h b/routines.h index 35873bb7..0e3d973d 100644 --- a/routines.h +++ b/routines.h @@ -38,6 +38,7 @@ const char *getDomainString(const int queryID); const char *getClientIPString(const int queryID); const char *getClientNameString(const int queryID); +// socket.c void close_telnet_socket(void); void close_unix_socket(void); void seom(const int sock); @@ -45,32 +46,33 @@ void ssend(const int sock, const char *format, ...) __attribute__ ((format (gnu_ void swrite(const int sock, const void* value, const size_t size); void *telnet_listening_thread_IPv4(void *args); void *telnet_listening_thread_IPv6(void *args); - void *socket_listening_thread(void *args); bool ipv6_available(void); void bind_sockets(void); +// request.c void process_request(const char *client_message, int *sock); bool command(const char *client_message, const char* cmd) __attribute__((pure)); -// grep.c +// files.c int countlines(const char* fname); int countlineswith(const char* str, const char* fname); void check_blocking_status(void); +bool chmod_file(const char *filename, const mode_t mode); +// setupVars.c void check_setupVarsconf(void); char * read_setupVarsconf(const char * key); void getSetupVarsArray(const char * input); void clearSetupVarsArray(void); bool insetupVarsArray(const char * str); bool getSetupVarsBool(const char * input) __attribute__((pure)); - -void parse_args(int argc, char* argv[]); - -// setupVars.c char* find_equals(const char* s) __attribute__((pure)); void trim_whitespace(char *string); +// args.c +void parse_args(int argc, char* argv[]); + // config.c void getLogFilePath(void); void read_FTLconf(void); @@ -102,6 +104,7 @@ void *FTLcalloc(size_t nmemb, size_t size, const char *file, const char *functio void *FTLrealloc(void *ptr_in, size_t size, const char *file, const char *function, const int line) __attribute__((alloc_size(2))); void FTLfree(void *ptr, const char* file, const char *function, const int line); +// dnsmasq/dnsmasq.c int main_dnsmasq(int argc, const char ** argv); // signals.c diff --git a/test/run.sh b/test/run.sh index d78e538f..ce1ea4f4 100755 --- a/test/run.sh +++ b/test/run.sh @@ -7,10 +7,23 @@ if [[ ${CI} == "true" && "${CIRCLE_JOB}" != "x86_64" ]]; then fi # Install necessary additional components for testing -apt-get -qq install dnsutils -y > /dev/null +apt-get -qq install dnsutils libcap2-bin -y > /dev/null -# Create necessary directories +# Create pihole user if it does not exist +if ! id -u pihole &> /dev/null; then + useradd -m -s /usr/sbin/nologin pihole +fi + +# Create necessary directories and files mkdir -p /etc/pihole /var/run/pihole /var/log +touch /var/log/pihole-FTL.log /var/run/pihole-FTL.pid /var/run/pihole-FTL.port +chown pihole:pihole /etc/pihole /var/run/pihole /var/log/pihole-FTL.log /var/run/pihole-FTL.pid /var/run/pihole-FTL.port + +# Copy binary into a location the new user pihole can access +cp ./pihole-FTL /home/pihole +chmod +x /home/pihole/pihole-FTL +# Note: We cannot add CAP_NET_RAW and CAP_NET_ADMIN at this point +setcap CAP_NET_BIND_SERVICE+eip /home/pihole/pihole-FTL # Prepare gravity database sqlite3 /etc/pihole/gravity.db < test/gravity.db.sql @@ -21,15 +34,19 @@ echo "BLOCKING_ENABLED=true" > /etc/pihole/setupVars.conf # Prepare pihole-FTL.conf echo "" > /etc/pihole/pihole-FTL.conf +# Set restrictive umask +OLDUMASK=$(umask) +umask 0022 + # Start FTL -if ! ./pihole-FTL; then +if ! runuser -l pihole -s /bin/sh -c /home/pihole/pihole-FTL; then echo "pihole-FTL failed to start" exit 1 fi # Prepare BATS mkdir -p test/libs -git clone --depth=1 https://github.com/bats-core/bats-core test/libs/bats > /dev/null +git clone --depth=1 --quiet https://github.com/bats-core/bats-core test/libs/bats > /dev/null # Block until FTL is ready, retry once per second for 45 seconds sleep 2 @@ -45,4 +62,13 @@ cat /var/log/pihole-FTL.log # Run tests test/libs/bats/bin/bats "test/test_suite.bats" -exit $? +RET=$? + +# Restore umask +umask $OLDUMASK + +# Remove copied file +rm /home/pihole/pihole-FTL + +# Exit with return code of bats tests +exit $RET diff --git a/test/test_suite.bats b/test/test_suite.bats index e64d47da..cd4051ba 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -250,25 +250,43 @@ } @test "Fail on invalid argument" { - run bash -c './pihole-FTL abc' + run bash -c '/home/pihole/pihole-FTL abc' printf "%s\n" "${lines[@]}" [[ ${lines[0]} == "pihole-FTL: invalid option -- 'abc'" ]] - [[ ${lines[1]} == "Try './pihole-FTL --help' for more information" ]] + [[ ${lines[1]} == "Try '/home/pihole/pihole-FTL --help' for more information" ]] } @test "Help argument return help text" { - run bash -c './pihole-FTL help' + run bash -c '/home/pihole/pihole-FTL help' printf "%s\n" "${lines[@]}" [[ ${lines[0]} == "pihole-FTL - The Pi-hole FTL engine" ]] [[ ${lines[3]} == "Available arguments:" ]] } -@test "No FATAL messages in pihole-FTL.log" { - run bash -c 'grep -c "FATAL" /var/log/pihole-FTL.log' +@test "No WARNING messages in pihole-FTL.log (besides known capability issues)" { + run bash -c 'grep "WARNING:" /var/log/pihole-FTL.log | grep -c -v -E "CAP_NET_ADMIN|CAP_NET_RAW"' printf "%s\n" "${lines[@]}" [[ ${lines[0]} == "0" ]] } +@test "No ERROR messages in pihole-FTL.log" { + run bash -c 'grep -c "ERROR:" /var/log/pihole-FTL.log' + printf "%s\n" "${lines[@]}" + [[ ${lines[0]} == "0" ]] +} + +@test "No FATAL messages in pihole-FTL.log" { + run bash -c 'grep -c "FATAL:" /var/log/pihole-FTL.log' + printf "%s\n" "${lines[@]}" + [[ ${lines[0]} == "0" ]] +} + +@test "Ownership and permissions of pihole-FTL.db correct" { + run bash -c 'ls -l /etc/pihole/pihole-FTL.db' + printf "%s\n" "${lines[@]}" + [[ ${lines[0]} == "-rw-r--r-- 1 pihole pihole"* ]] +} + @test "Final part of the tests: Kill pihole-FTL process" { run bash -c 'kill $(pidof pihole-FTL)' printf "%s\n" "${lines[@]}"