From 9ed84ff7172841063d8b4668d01e047f6ad499f6 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 17 Mar 2019 02:13:33 +0100 Subject: [PATCH 01/30] Do not mark cached PTR queries as externally blocked even if NXDOMAIN (#543) * Add debug logging for externally blocked domains Signed-off-by: DL6ER * Also output which domain was queries in debug output Signed-off-by: DL6ER * Do not mark PTR requests as externally blocked. Add new DEBUG_EXTBLOCKED flag. Signed-off-by: DL6ER * Improve new DEBUG_EXTBLOCKED messages. Signed-off-by: DL6ER * Domain -> Answer Signed-off-by: DL6ER --- FTL.h | 1 + config.c | 7 ++++++ dnsmasq_interface.c | 59 +++++++++++++++++++++++++++++++++++++++------ 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/FTL.h b/FTL.h index b5de3e87..7c2cf88c 100644 --- a/FTL.h +++ b/FTL.h @@ -103,6 +103,7 @@ enum { DEBUG_REGEX = (1 << 8), /* 00000001 00000000 */ DEBUG_API = (1 << 9), /* 00000010 00000000 */ DEBUG_OVERTIME = (1 << 10), /* 00000100 00000000 */ + DEBUG_EXTBLOCKED = (1 << 11), /* 00001000 00000000 */ }; // Database table "ftl" diff --git a/config.c b/config.c index 5664f3c5..a2f68ba9 100644 --- a/config.c +++ b/config.c @@ -588,6 +588,12 @@ void read_debuging_settings(FILE *fp) if(buffer != NULL && strcasecmp(buffer, "true") == 0) config.debug |= DEBUG_OVERTIME; + // DEBUG_EXTBLOCKED + // defaults to: false + buffer = parse_FTLconf(fp, "DEBUG_EXTBLOCKED"); + if(buffer != NULL && strcasecmp(buffer, "true") == 0) + config.debug |= DEBUG_EXTBLOCKED; + // DEBUG_ALL // defaults to: false buffer = parse_FTLconf(fp, "DEBUG_ALL"); @@ -609,6 +615,7 @@ void read_debuging_settings(FILE *fp) logg("* DEBUG_REGEX %s *", (config.debug & DEBUG_REGEX)? "YES":"NO "); logg("* DEBUG_API %s *", (config.debug & DEBUG_API)? "YES":"NO "); logg("* DEBUG_OVERTIME %s *", (config.debug & DEBUG_OVERTIME)? "YES":"NO "); + logg("* DEBUG_EXTBLOCKED %s *", (config.debug & DEBUG_EXTBLOCKED)? "YES":"NO "); logg("************************"); } diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index 36077e12..7b396cc4 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -115,7 +115,10 @@ void _FTL_new_query(unsigned int flags, char *name, struct all_addr *addr, char // Log new query if in debug mode const char *proto = (type == UDP) ? "UDP" : "TCP"; if(config.debug & DEBUG_QUERIES) - logg("**** new %s %s \"%s\" from %s (ID %i, FTL %i, %s:%i)", proto, types, domain, client, id, queryID, file, line); + { + logg("**** new %s %s \"%s\" from %s (ID %i, FTL %i, %s:%i)", + proto, types, domain, client, id, queryID, file, line); + } // Update counters counters->querytype[querytype-1]++; @@ -518,10 +521,23 @@ void _FTL_reply(unsigned short flags, char *name, struct all_addr *addr, int id, static void detect_blocked_IP(unsigned short flags, const char* answer, int queryID) { - // Skip replies which originated locally. Otherwise, we would count - // gravity.list blocked queries as externally blocked. if(flags & F_HOSTS) { + // Skip replies which originated locally. Otherwise, we would + // count gravity.list blocked queries as externally blocked. + if(config.debug & DEBUG_EXTBLOCKED) + { + logg("Skipping detection of external blocking IP for ID %i as origin is HOSTS", queryID); + } + return; + } + else if(flags & F_REVERSE) + { + // Do not mark responses of PTR requests as externally blocked. + if(config.debug & DEBUG_EXTBLOCKED) + { + logg("Skipping detection of external blocking IP for ID %i as query is PTR", queryID); + } return; } @@ -538,7 +554,14 @@ static void detect_blocked_IP(unsigned short flags, const char* answer, int quer strcmp("146.112.61.109", answer) == 0 || strcmp("146.112.61.110", answer) == 0 )) { - query_externally_blocked(queryID, QUERY_EXTERNAL_BLOCKED_IP); + if(config.debug & DEBUG_EXTBLOCKED) + { + logg("Upstream responded with known blocking page (IPv4), ID %i:\n\t\"%s\" -> \"%s\"", + queryID, getstr(domains[queryID].domainpos), answer); + } + + // Update status + query_externally_blocked(queryID, QUERY_EXTERNAL_BLOCKED_IP); } else if(flags & F_IPV6 && answer != NULL && @@ -550,7 +573,14 @@ static void detect_blocked_IP(unsigned short flags, const char* answer, int quer strcmp("::ffff:146.112.61.109", answer) == 0 || strcmp("::ffff:146.112.61.110", answer) == 0 )) { - query_externally_blocked(queryID, QUERY_EXTERNAL_BLOCKED_IP); + if(config.debug & DEBUG_EXTBLOCKED) + { + logg("Upstream responded with known blocking page (IPv6), ID %i:\n\t\"%s\" -> \"%s\"", + queryID, getstr(domains[queryID].domainpos), answer); + } + + // Update status + query_externally_blocked(queryID, QUERY_EXTERNAL_BLOCKED_IP); } // If upstream replied with 0.0.0.0 or ::, @@ -559,13 +589,27 @@ static void detect_blocked_IP(unsigned short flags, const char* answer, int quer else if(flags & F_IPV4 && answer != NULL && strcmp("0.0.0.0", answer) == 0) { - query_externally_blocked(queryID, QUERY_EXTERNAL_BLOCKED_NULL); + if(config.debug & DEBUG_EXTBLOCKED) + { + logg("Upstream responded with 0.0.0.0, ID %i:\n\t\"%s\" -> \"%s\"", + queryID, getstr(domains[queryID].domainpos), answer); + } + + // Update status + query_externally_blocked(queryID, QUERY_EXTERNAL_BLOCKED_NULL); } else if(flags & F_IPV6 && answer != NULL && strcmp("::", answer) == 0) { - query_externally_blocked(queryID, QUERY_EXTERNAL_BLOCKED_NULL); + if(config.debug & DEBUG_EXTBLOCKED) + { + logg("Upstream responded with ::, ID %i:\n\t\"%s\" -> \"%s\"", + queryID, getstr(domains[queryID].domainpos), answer); + } + + // Update status + query_externally_blocked(queryID, QUERY_EXTERNAL_BLOCKED_NULL); } } @@ -597,6 +641,7 @@ static void query_externally_blocked(int i, unsigned char status) validate_access("clients", queries[i].clientID, true, __LINE__, __FUNCTION__, __FILE__); clients[queries[i].clientID].blockedcount++; + // Update status queries[i].status = status; } From 6186cd1cf6e180f45ad7fe79583172651bdf4c19 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 17 Mar 2019 10:57:48 +0100 Subject: [PATCH 02/30] Check capabilities with kernel methods. Removes the need for libcap on the target system. Signed-off-by: DL6ER --- Makefile | 2 +- capabilities.c | 48 ++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 6f7f78c7..009e18f0 100644 --- a/Makefile +++ b/Makefile @@ -98,7 +98,7 @@ CCFLAGS=-std=gnu11 -I$(IDIR) $(WARNFLAGS) -D_FILE_OFFSET_BITS=64 $(HARDENING_FLA # for dnsmasq we need the nettle crypto library and the gmp maths library # We link the two libraries statically. Although this increases the binary file size by about 1 MB, it saves about 5 MB of shared libraries and makes deployment easier #LIBS=-pthread -lnettle -lgmp -lhogweed -LIBS=-pthread -lrt -lcap -Wl,-Bstatic -L/usr/local/lib -lhogweed -lgmp -lnettle -Wl,-Bdynamic +LIBS=-pthread -lrt -Wl,-Bstatic -L/usr/local/lib -lhogweed -lgmp -lnettle -Wl,-Bdynamic # Flags for compiling with libidn : -lidn # Flags for compiling with libidn2: -lidn2 diff --git a/capabilities.c b/capabilities.c index 1a33fee8..a583f440 100644 --- a/capabilities.c +++ b/capabilities.c @@ -8,36 +8,68 @@ * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ +// Definition of LINUX_CAPABILITY_VERSION_* +#define FTLDNS +#include "dnsmasq/dnsmasq.h" +#undef __USE_XOPEN #include "FTL.h" -#include bool check_capabilities() { - if(!cap_get_bound(CAP_NET_ADMIN)) + int capsize = 1; /* for header version 1 */ + cap_user_header_t hdr = NULL; + cap_user_data_t data = NULL; + + /* find version supported by kernel */ + hdr = calloc(sizeof(*hdr), capsize); + memset(hdr, 0, sizeof(*hdr)); + capget(hdr, NULL); + + if (hdr->version != LINUX_CAPABILITY_VERSION_1) + { + /* if unknown version, use largest supported version (3) */ + if (hdr->version != LINUX_CAPABILITY_VERSION_2) + hdr->version = LINUX_CAPABILITY_VERSION_3; + capsize = 2; + } + + data = calloc(sizeof(*data), capsize); + capget(hdr, data); /* Get current values, for verification */ + + bool missing = true; + if (!(data->permitted & (1 << CAP_NET_ADMIN))) { // Needed for ARP-injection (used when we're the DHCP server) logg("**************************************************************"); logg("WARNING: Required linux capability CAP_NET_ADMIN not available"); logg("**************************************************************"); - return false; + missing = true; } - if(!cap_get_bound(CAP_NET_RAW)) + if (!(data->permitted & (1 << CAP_NET_RAW))) { // Needed for raw socket access (necessary for ICMP) logg("************************************************************"); logg("WARNING: Required linux capability CAP_NET_RAW not available"); logg("************************************************************"); - return false; + missing = true; } - if(!cap_get_bound(CAP_NET_BIND_SERVICE)) + if (!(data->permitted & (1 << CAP_NET_BIND_SERVICE))) { // Necessary for dynamic port binding logg("*********************************************************************"); logg("WARNING: Required linux capability CAP_NET_BIND_SERVICE not available"); logg("*********************************************************************"); - return false; + missing = true; + } + if (!(data->permitted & (1 << CAP_SETUID))) + { + // Necessary for changing our own user ID ("daemonizing") + logg("*********************************************************************"); + logg("WARNING: Required linux capability CAP_SETUID not available"); + logg("*********************************************************************"); + missing = true; } // All okay! - return true; + return missing; } From 8d615d6011fa839e4d7b55329a13b23b500ea0b6 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 17 Mar 2019 14:53:27 +0100 Subject: [PATCH 03/30] Add capabilities debug mode that prints all capabilities which are enabled when pihole-FTL is started. Signed-off-by: DL6ER --- FTL.h | 1 + capabilities.c | 39 +++++++++++++++++++++++++++++---------- config.c | 7 +++++++ 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/FTL.h b/FTL.h index 7c2cf88c..a4bba095 100644 --- a/FTL.h +++ b/FTL.h @@ -104,6 +104,7 @@ enum { DEBUG_API = (1 << 9), /* 00000010 00000000 */ DEBUG_OVERTIME = (1 << 10), /* 00000100 00000000 */ DEBUG_EXTBLOCKED = (1 << 11), /* 00001000 00000000 */ + DEBUG_CAPS = (1 << 12), /* 00010000 00000000 */ }; // Database table "ftl" diff --git a/capabilities.c b/capabilities.c index a583f440..16a77e75 100644 --- a/capabilities.c +++ b/capabilities.c @@ -14,6 +14,9 @@ #undef __USE_XOPEN #include "FTL.h" +const int capabilityIntegers[] = { CAP_CHOWN , CAP_DAC_OVERRIDE , CAP_DAC_READ_SEARCH , CAP_FOWNER , CAP_FSETID , CAP_KILL , CAP_SETGID , CAP_SETUID , CAP_SETPCAP , CAP_LINUX_IMMUTABLE , CAP_NET_BIND_SERVICE , CAP_NET_BROADCAST , CAP_NET_ADMIN , CAP_NET_RAW , CAP_IPC_LOCK , CAP_IPC_OWNER , CAP_SYS_MODULE , CAP_SYS_RAWIO , CAP_SYS_CHROOT , CAP_SYS_PTRACE , CAP_SYS_PACCT , CAP_SYS_ADMIN , CAP_SYS_BOOT , CAP_SYS_NICE , CAP_SYS_RESOURCE , CAP_SYS_TIME , CAP_SYS_TTY_CONFIG , CAP_MKNOD , CAP_LEASE , CAP_AUDIT_WRITE , CAP_AUDIT_CONTROL , CAP_SETFCAP , CAP_MAC_OVERRIDE , CAP_MAC_ADMIN , CAP_SYSLOG , CAP_WAKE_ALARM , CAP_BLOCK_SUSPEND , CAP_AUDIT_READ }; +const char* capabilityNames[] = {"CAP_CHOWN", "CAP_DAC_OVERRIDE", "CAP_DAC_READ_SEARCH", "CAP_FOWNER", "CAP_FSETID", "CAP_KILL", "CAP_SETGID", "CAP_SETUID", "CAP_SETPCAP", "CAP_LINUX_IMMUTABLE", "CAP_NET_BIND_SERVICE", "CAP_NET_BROADCAST", "CAP_NET_ADMIN", "CAP_NET_RAW", "CAP_IPC_LOCK", "CAP_IPC_OWNER", "CAP_SYS_MODULE", "CAP_SYS_RAWIO", "CAP_SYS_CHROOT", "CAP_SYS_PTRACE", "CAP_SYS_PACCT", "CAP_SYS_ADMIN", "CAP_SYS_BOOT", "CAP_SYS_NICE", "CAP_SYS_RESOURCE", "CAP_SYS_TIME", "CAP_SYS_TTY_CONFIG", "CAP_MKNOD", "CAP_LEASE", "CAP_AUDIT_WRITE", "CAP_AUDIT_CONTROL", "CAP_SETFCAP", "CAP_MAC_OVERRIDE", "CAP_MAC_ADMIN", "CAP_SYSLOG", "CAP_WAKE_ALARM", "CAP_BLOCK_SUSPEND", "CAP_AUDIT_READ"}; + bool check_capabilities() { int capsize = 1; /* for header version 1 */ @@ -36,22 +39,38 @@ bool check_capabilities() data = calloc(sizeof(*data), capsize); capget(hdr, data); /* Get current values, for verification */ - bool missing = true; + if(config.debug & DEBUG_CAPS) + { + logg("*********************************************************************"); + for(unsigned int i = 0u; i < (sizeof(capabilityIntegers)/sizeof(const int)); i++) + { + unsigned int capid = capabilityIntegers[i]; + logg("DEBUG: Capability %-24s (%02u) = %s%s%s", + capabilityNames[capid], + capid, + ((data->effective & (1 << capid)) ? "E":"-"), + ((data->permitted & (1 << capid)) ? "P":"-"), + ((data->inheritable & (1 << capid)) ? "I":"-")); + } + logg("*********************************************************************"); + } + + bool capabilities_okay = true; if (!(data->permitted & (1 << CAP_NET_ADMIN))) { // Needed for ARP-injection (used when we're the DHCP server) - logg("**************************************************************"); + logg("*********************************************************************"); logg("WARNING: Required linux capability CAP_NET_ADMIN not available"); - logg("**************************************************************"); - missing = true; + logg("*********************************************************************"); + capabilities_okay = false; } if (!(data->permitted & (1 << CAP_NET_RAW))) { // Needed for raw socket access (necessary for ICMP) - logg("************************************************************"); + logg("*********************************************************************"); logg("WARNING: Required linux capability CAP_NET_RAW not available"); - logg("************************************************************"); - missing = true; + logg("*********************************************************************"); + capabilities_okay = false; } if (!(data->permitted & (1 << CAP_NET_BIND_SERVICE))) { @@ -59,7 +78,7 @@ bool check_capabilities() logg("*********************************************************************"); logg("WARNING: Required linux capability CAP_NET_BIND_SERVICE not available"); logg("*********************************************************************"); - missing = true; + capabilities_okay = false; } if (!(data->permitted & (1 << CAP_SETUID))) { @@ -67,9 +86,9 @@ bool check_capabilities() logg("*********************************************************************"); logg("WARNING: Required linux capability CAP_SETUID not available"); logg("*********************************************************************"); - missing = true; + capabilities_okay = false; } // All okay! - return missing; + return capabilities_okay; } diff --git a/config.c b/config.c index a2f68ba9..c867a6a7 100644 --- a/config.c +++ b/config.c @@ -594,6 +594,12 @@ void read_debuging_settings(FILE *fp) if(buffer != NULL && strcasecmp(buffer, "true") == 0) config.debug |= DEBUG_EXTBLOCKED; + // DEBUG_CAPS + // defaults to: false + buffer = parse_FTLconf(fp, "DEBUG_CAPS"); + if(buffer != NULL && strcasecmp(buffer, "true") == 0) + config.debug |= DEBUG_CAPS; + // DEBUG_ALL // defaults to: false buffer = parse_FTLconf(fp, "DEBUG_ALL"); @@ -616,6 +622,7 @@ void read_debuging_settings(FILE *fp) logg("* DEBUG_API %s *", (config.debug & DEBUG_API)? "YES":"NO "); logg("* DEBUG_OVERTIME %s *", (config.debug & DEBUG_OVERTIME)? "YES":"NO "); logg("* DEBUG_EXTBLOCKED %s *", (config.debug & DEBUG_EXTBLOCKED)? "YES":"NO "); + logg("* DEBUG_CAPS %s *", (config.debug & DEBUG_CAPS)? "YES":"NO "); logg("************************"); } From 73cbb911ae8b2a4ab8e013a56b2bafb22e9dd4c7 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 17 Mar 2019 15:30:05 +0100 Subject: [PATCH 04/30] Change caps sorting in debug messages. The prefered order seems to be PIE. Signed-off-by: DL6ER --- capabilities.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/capabilities.c b/capabilities.c index 16a77e75..77cb9da9 100644 --- a/capabilities.c +++ b/capabilities.c @@ -48,9 +48,9 @@ bool check_capabilities() logg("DEBUG: Capability %-24s (%02u) = %s%s%s", capabilityNames[capid], capid, - ((data->effective & (1 << capid)) ? "E":"-"), ((data->permitted & (1 << capid)) ? "P":"-"), - ((data->inheritable & (1 << capid)) ? "I":"-")); + ((data->inheritable & (1 << capid)) ? "I":"-"), + ((data->effective & (1 << capid)) ? "E":"-")); } logg("*********************************************************************"); } From 4a15b93fea64cf86cdb422b898f75818a5a2f2f5 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 17 Mar 2019 15:33:04 +0100 Subject: [PATCH 05/30] Explicitly check availability of capability in current kernel before printing its value. Accessing a capability that is not valid can never lead to a crash, however, it is also no a menaingful thing to do so we don't do it. Signed-off-by: DL6ER --- capabilities.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/capabilities.c b/capabilities.c index 77cb9da9..5ae71709 100644 --- a/capabilities.c +++ b/capabilities.c @@ -45,6 +45,12 @@ bool check_capabilities() for(unsigned int i = 0u; i < (sizeof(capabilityIntegers)/sizeof(const int)); i++) { unsigned int capid = capabilityIntegers[i]; + + // Check if capability is valid for the current kernel + // If not, exit loop early + if(!cap_valid(capid)) + break; + logg("DEBUG: Capability %-24s (%02u) = %s%s%s", capabilityNames[capid], capid, From 35daec46d67116669d00d66ff588609e7852adc6 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 17 Mar 2019 15:35:45 +0100 Subject: [PATCH 06/30] Print current set of capabilities when receiving SIGHUP and DEBUG_CAPS is set to true. Signed-off-by: DL6ER --- dnsmasq_interface.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index 7b396cc4..690c8e0f 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -382,6 +382,10 @@ void FTL_dnsmasq_reload(void) // Reread pihole-FTL.conf to see which debugging flags are set read_debuging_settings(NULL); + + // Print current set of capabilities if requested via debug flag + if(config.debug & DEBUG_CAPS) + check_capabilities(); } void _FTL_reply(unsigned short flags, char *name, struct all_addr *addr, int id, const char* file, const int line) From 91beeec82425075a9e40ccb7d0fc478dfa9ff05f Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Mar 2019 06:57:48 +0100 Subject: [PATCH 07/30] Free allocated memory Signed-off-by: DL6ER --- capabilities.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/capabilities.c b/capabilities.c index 5ae71709..b5f9438e 100644 --- a/capabilities.c +++ b/capabilities.c @@ -19,25 +19,35 @@ const char* capabilityNames[] = {"CAP_CHOWN", "CAP_DAC_OVERRIDE", "CAP_DAC_READ bool check_capabilities() { - int capsize = 1; /* for header version 1 */ - cap_user_header_t hdr = NULL; + // First assume header version 1 + int capsize = VFS_CAP_U32_1; cap_user_data_t data = NULL; + cap_user_header_t hdr = calloc(sizeof(*hdr), capsize); - /* find version supported by kernel */ - hdr = calloc(sizeof(*hdr), capsize); + // Determine capabilities version used by the current kernel memset(hdr, 0, sizeof(*hdr)); capget(hdr, NULL); + // Check version if (hdr->version != LINUX_CAPABILITY_VERSION_1) { - /* if unknown version, use largest supported version (3) */ - if (hdr->version != LINUX_CAPABILITY_VERSION_2) - hdr->version = LINUX_CAPABILITY_VERSION_3; - capsize = 2; + // If unknown version, use largest supported version (3) + // Version 2 is deprecated according to linux/capability.h + if (hdr->version != LINUX_CAPABILITY_VERSION_2) + { + hdr->version = LINUX_CAPABILITY_VERSION_3; + capsize = VFS_CAP_U32_3; + } + else + { + // Use version 2 + capsize = VFS_CAP_U32_2; + } } + // Get current capabilities data = calloc(sizeof(*data), capsize); - capget(hdr, data); /* Get current values, for verification */ + capget(hdr, data); if(config.debug & DEBUG_CAPS) { @@ -95,6 +105,10 @@ bool check_capabilities() capabilities_okay = false; } + // Free allocated memory + free(hdr); + free(data); + // All okay! return capabilities_okay; } From efbe69cdf26f5ba88e8ce3c6f11c3417264828eb Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Mar 2019 07:42:04 +0100 Subject: [PATCH 08/30] Declare array only to be used within capabilitites.c as static Signed-off-by: DL6ER --- capabilities.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/capabilities.c b/capabilities.c index b5f9438e..c671969d 100644 --- a/capabilities.c +++ b/capabilities.c @@ -14,8 +14,9 @@ #undef __USE_XOPEN #include "FTL.h" -const int capabilityIntegers[] = { CAP_CHOWN , CAP_DAC_OVERRIDE , CAP_DAC_READ_SEARCH , CAP_FOWNER , CAP_FSETID , CAP_KILL , CAP_SETGID , CAP_SETUID , CAP_SETPCAP , CAP_LINUX_IMMUTABLE , CAP_NET_BIND_SERVICE , CAP_NET_BROADCAST , CAP_NET_ADMIN , CAP_NET_RAW , CAP_IPC_LOCK , CAP_IPC_OWNER , CAP_SYS_MODULE , CAP_SYS_RAWIO , CAP_SYS_CHROOT , CAP_SYS_PTRACE , CAP_SYS_PACCT , CAP_SYS_ADMIN , CAP_SYS_BOOT , CAP_SYS_NICE , CAP_SYS_RESOURCE , CAP_SYS_TIME , CAP_SYS_TTY_CONFIG , CAP_MKNOD , CAP_LEASE , CAP_AUDIT_WRITE , CAP_AUDIT_CONTROL , CAP_SETFCAP , CAP_MAC_OVERRIDE , CAP_MAC_ADMIN , CAP_SYSLOG , CAP_WAKE_ALARM , CAP_BLOCK_SUSPEND , CAP_AUDIT_READ }; -const char* capabilityNames[] = {"CAP_CHOWN", "CAP_DAC_OVERRIDE", "CAP_DAC_READ_SEARCH", "CAP_FOWNER", "CAP_FSETID", "CAP_KILL", "CAP_SETGID", "CAP_SETUID", "CAP_SETPCAP", "CAP_LINUX_IMMUTABLE", "CAP_NET_BIND_SERVICE", "CAP_NET_BROADCAST", "CAP_NET_ADMIN", "CAP_NET_RAW", "CAP_IPC_LOCK", "CAP_IPC_OWNER", "CAP_SYS_MODULE", "CAP_SYS_RAWIO", "CAP_SYS_CHROOT", "CAP_SYS_PTRACE", "CAP_SYS_PACCT", "CAP_SYS_ADMIN", "CAP_SYS_BOOT", "CAP_SYS_NICE", "CAP_SYS_RESOURCE", "CAP_SYS_TIME", "CAP_SYS_TTY_CONFIG", "CAP_MKNOD", "CAP_LEASE", "CAP_AUDIT_WRITE", "CAP_AUDIT_CONTROL", "CAP_SETFCAP", "CAP_MAC_OVERRIDE", "CAP_MAC_ADMIN", "CAP_SYSLOG", "CAP_WAKE_ALARM", "CAP_BLOCK_SUSPEND", "CAP_AUDIT_READ"}; +static const int capabilityIntegers[] = { CAP_CHOWN , CAP_DAC_OVERRIDE , CAP_DAC_READ_SEARCH , CAP_FOWNER , CAP_FSETID , CAP_KILL , CAP_SETGID , CAP_SETUID , CAP_SETPCAP , CAP_LINUX_IMMUTABLE , CAP_NET_BIND_SERVICE , CAP_NET_BROADCAST , CAP_NET_ADMIN , CAP_NET_RAW , CAP_IPC_LOCK , CAP_IPC_OWNER , CAP_SYS_MODULE , CAP_SYS_RAWIO , CAP_SYS_CHROOT , CAP_SYS_PTRACE , CAP_SYS_PACCT , CAP_SYS_ADMIN , CAP_SYS_BOOT , CAP_SYS_NICE , CAP_SYS_RESOURCE , CAP_SYS_TIME , CAP_SYS_TTY_CONFIG , CAP_MKNOD , CAP_LEASE , CAP_AUDIT_WRITE , CAP_AUDIT_CONTROL , CAP_SETFCAP , CAP_MAC_OVERRIDE , CAP_MAC_ADMIN , CAP_SYSLOG , CAP_WAKE_ALARM , CAP_BLOCK_SUSPEND , CAP_AUDIT_READ }; +static const char* capabilityNames[] = {"CAP_CHOWN", "CAP_DAC_OVERRIDE", "CAP_DAC_READ_SEARCH", "CAP_FOWNER", "CAP_FSETID", "CAP_KILL", "CAP_SETGID", "CAP_SETUID", "CAP_SETPCAP", "CAP_LINUX_IMMUTABLE", "CAP_NET_BIND_SERVICE", "CAP_NET_BROADCAST", "CAP_NET_ADMIN", "CAP_NET_RAW", "CAP_IPC_LOCK", "CAP_IPC_OWNER", "CAP_SYS_MODULE", "CAP_SYS_RAWIO", "CAP_SYS_CHROOT", "CAP_SYS_PTRACE", "CAP_SYS_PACCT", "CAP_SYS_ADMIN", "CAP_SYS_BOOT", "CAP_SYS_NICE", "CAP_SYS_RESOURCE", "CAP_SYS_TIME", "CAP_SYS_TTY_CONFIG", "CAP_MKNOD", "CAP_LEASE", "CAP_AUDIT_WRITE", "CAP_AUDIT_CONTROL", "CAP_SETFCAP", "CAP_MAC_OVERRIDE", "CAP_MAC_ADMIN", "CAP_SYSLOG", "CAP_WAKE_ALARM", "CAP_BLOCK_SUSPEND", "CAP_AUDIT_READ"}; +static const int numCaps = sizeof(capabilityIntegers)/sizeof(const int); bool check_capabilities() { @@ -52,7 +53,7 @@ bool check_capabilities() if(config.debug & DEBUG_CAPS) { logg("*********************************************************************"); - for(unsigned int i = 0u; i < (sizeof(capabilityIntegers)/sizeof(const int)); i++) + for(unsigned int i = 0u; i < numCaps; i++) { unsigned int capid = capabilityIntegers[i]; @@ -62,8 +63,7 @@ bool check_capabilities() break; logg("DEBUG: Capability %-24s (%02u) = %s%s%s", - capabilityNames[capid], - capid, + capabilityNames[capid], capid, ((data->permitted & (1 << capid)) ? "P":"-"), ((data->inheritable & (1 << capid)) ? "I":"-"), ((data->effective & (1 << capid)) ? "E":"-")); From f48fa15040cf9463324ddacbb1a550780d96705b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Mar 2019 07:48:56 +0100 Subject: [PATCH 09/30] Use actual values for capsize as the macros are not available on all CI machines, causing the jobs to fail. Signed-off-by: DL6ER --- capabilities.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/capabilities.c b/capabilities.c index c671969d..f1ccdf99 100644 --- a/capabilities.c +++ b/capabilities.c @@ -16,12 +16,12 @@ static const int capabilityIntegers[] = { CAP_CHOWN , CAP_DAC_OVERRIDE , CAP_DAC_READ_SEARCH , CAP_FOWNER , CAP_FSETID , CAP_KILL , CAP_SETGID , CAP_SETUID , CAP_SETPCAP , CAP_LINUX_IMMUTABLE , CAP_NET_BIND_SERVICE , CAP_NET_BROADCAST , CAP_NET_ADMIN , CAP_NET_RAW , CAP_IPC_LOCK , CAP_IPC_OWNER , CAP_SYS_MODULE , CAP_SYS_RAWIO , CAP_SYS_CHROOT , CAP_SYS_PTRACE , CAP_SYS_PACCT , CAP_SYS_ADMIN , CAP_SYS_BOOT , CAP_SYS_NICE , CAP_SYS_RESOURCE , CAP_SYS_TIME , CAP_SYS_TTY_CONFIG , CAP_MKNOD , CAP_LEASE , CAP_AUDIT_WRITE , CAP_AUDIT_CONTROL , CAP_SETFCAP , CAP_MAC_OVERRIDE , CAP_MAC_ADMIN , CAP_SYSLOG , CAP_WAKE_ALARM , CAP_BLOCK_SUSPEND , CAP_AUDIT_READ }; static const char* capabilityNames[] = {"CAP_CHOWN", "CAP_DAC_OVERRIDE", "CAP_DAC_READ_SEARCH", "CAP_FOWNER", "CAP_FSETID", "CAP_KILL", "CAP_SETGID", "CAP_SETUID", "CAP_SETPCAP", "CAP_LINUX_IMMUTABLE", "CAP_NET_BIND_SERVICE", "CAP_NET_BROADCAST", "CAP_NET_ADMIN", "CAP_NET_RAW", "CAP_IPC_LOCK", "CAP_IPC_OWNER", "CAP_SYS_MODULE", "CAP_SYS_RAWIO", "CAP_SYS_CHROOT", "CAP_SYS_PTRACE", "CAP_SYS_PACCT", "CAP_SYS_ADMIN", "CAP_SYS_BOOT", "CAP_SYS_NICE", "CAP_SYS_RESOURCE", "CAP_SYS_TIME", "CAP_SYS_TTY_CONFIG", "CAP_MKNOD", "CAP_LEASE", "CAP_AUDIT_WRITE", "CAP_AUDIT_CONTROL", "CAP_SETFCAP", "CAP_MAC_OVERRIDE", "CAP_MAC_ADMIN", "CAP_SYSLOG", "CAP_WAKE_ALARM", "CAP_BLOCK_SUSPEND", "CAP_AUDIT_READ"}; -static const int numCaps = sizeof(capabilityIntegers)/sizeof(const int); +static const unsigned int numCaps = sizeof(capabilityIntegers)/sizeof(const int); bool check_capabilities() { // First assume header version 1 - int capsize = VFS_CAP_U32_1; + int capsize = 1; // VFS_CAP_U32_1 cap_user_data_t data = NULL; cap_user_header_t hdr = calloc(sizeof(*hdr), capsize); @@ -37,12 +37,12 @@ bool check_capabilities() if (hdr->version != LINUX_CAPABILITY_VERSION_2) { hdr->version = LINUX_CAPABILITY_VERSION_3; - capsize = VFS_CAP_U32_3; + capsize = 2; // VFS_CAP_U32_3 } else { // Use version 2 - capsize = VFS_CAP_U32_2; + capsize = 2; // VFS_CAP_U32_2 } } From 526ec7e27b92435797235a3495a829895134a5a3 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Mar 2019 11:43:15 +0100 Subject: [PATCH 10/30] Missing CAP_SETUID is not a problem as we only need it when we start as root (which has all capabiltites). Signed-off-by: DL6ER --- capabilities.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/capabilities.c b/capabilities.c index f1ccdf99..35a6a261 100644 --- a/capabilities.c +++ b/capabilities.c @@ -96,14 +96,6 @@ bool check_capabilities() logg("*********************************************************************"); capabilities_okay = false; } - if (!(data->permitted & (1 << CAP_SETUID))) - { - // Necessary for changing our own user ID ("daemonizing") - logg("*********************************************************************"); - logg("WARNING: Required linux capability CAP_SETUID not available"); - logg("*********************************************************************"); - capabilities_okay = false; - } // Free allocated memory free(hdr); From f7af506ab516f3f37f8ff78916806003ff42a232 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Mar 2019 11:51:57 +0100 Subject: [PATCH 11/30] Print message that we received SIGHUP into pihole-FTL.log when this happens Signed-off-by: DL6ER --- capabilities.c | 24 ++++++++++++------------ dnsmasq_interface.c | 2 ++ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/capabilities.c b/capabilities.c index 35a6a261..4bd16b53 100644 --- a/capabilities.c +++ b/capabilities.c @@ -52,7 +52,7 @@ bool check_capabilities() if(config.debug & DEBUG_CAPS) { - logg("*********************************************************************"); + logg("*********************************************************"); for(unsigned int i = 0u; i < numCaps; i++) { unsigned int capid = capabilityIntegers[i]; @@ -62,38 +62,38 @@ bool check_capabilities() if(!cap_valid(capid)) break; - logg("DEBUG: Capability %-24s (%02u) = %s%s%s", + logg("* DEBUG: Capability %-24s (%02u) = %s%s%s *", capabilityNames[capid], capid, ((data->permitted & (1 << capid)) ? "P":"-"), ((data->inheritable & (1 << capid)) ? "I":"-"), ((data->effective & (1 << capid)) ? "E":"-")); } - logg("*********************************************************************"); + logg("*********************************************************"); } bool capabilities_okay = true; if (!(data->permitted & (1 << CAP_NET_ADMIN))) { // Needed for ARP-injection (used when we're the DHCP server) - logg("*********************************************************************"); - logg("WARNING: Required linux capability CAP_NET_ADMIN not available"); - logg("*********************************************************************"); + logg("*************************************************************************"); + logg("* WARNING: Required Linux capability CAP_NET_ADMIN not available *"); + logg("*************************************************************************"); capabilities_okay = false; } if (!(data->permitted & (1 << CAP_NET_RAW))) { // Needed for raw socket access (necessary for ICMP) - logg("*********************************************************************"); - logg("WARNING: Required linux capability CAP_NET_RAW not available"); - logg("*********************************************************************"); + logg("*************************************************************************"); + logg("* WARNING: Required Linux capability CAP_NET_RAW not available *"); + logg("*************************************************************************"); capabilities_okay = false; } if (!(data->permitted & (1 << CAP_NET_BIND_SERVICE))) { // Necessary for dynamic port binding - logg("*********************************************************************"); - logg("WARNING: Required linux capability CAP_NET_BIND_SERVICE not available"); - logg("*********************************************************************"); + logg("*************************************************************************"); + logg("* WARNING: Required Linux capability CAP_NET_BIND_SERVICE not available *"); + logg("*************************************************************************"); capabilities_okay = false; } diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index 690c8e0f..a62945f3 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -361,6 +361,8 @@ void FTL_dnsmasq_reload(void) // *before* clearing the cache and rereading the lists // This is the only hook that is not skipped in PRIVACY_NOSTATS mode + logg("Received SIGHUP, reloading cache"); + // Called when dnsmasq re-reads its config and hosts files // Reset number of blocked domains counters->gravity = 0; From dbf6e0831eea5f9d74353f18127b90512f5edad2 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Mar 2019 12:00:25 +0100 Subject: [PATCH 12/30] Prettify capabilities debugging output Signed-off-by: DL6ER --- capabilities.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/capabilities.c b/capabilities.c index 4bd16b53..dd20089a 100644 --- a/capabilities.c +++ b/capabilities.c @@ -14,9 +14,9 @@ #undef __USE_XOPEN #include "FTL.h" -static const int capabilityIntegers[] = { CAP_CHOWN , CAP_DAC_OVERRIDE , CAP_DAC_READ_SEARCH , CAP_FOWNER , CAP_FSETID , CAP_KILL , CAP_SETGID , CAP_SETUID , CAP_SETPCAP , CAP_LINUX_IMMUTABLE , CAP_NET_BIND_SERVICE , CAP_NET_BROADCAST , CAP_NET_ADMIN , CAP_NET_RAW , CAP_IPC_LOCK , CAP_IPC_OWNER , CAP_SYS_MODULE , CAP_SYS_RAWIO , CAP_SYS_CHROOT , CAP_SYS_PTRACE , CAP_SYS_PACCT , CAP_SYS_ADMIN , CAP_SYS_BOOT , CAP_SYS_NICE , CAP_SYS_RESOURCE , CAP_SYS_TIME , CAP_SYS_TTY_CONFIG , CAP_MKNOD , CAP_LEASE , CAP_AUDIT_WRITE , CAP_AUDIT_CONTROL , CAP_SETFCAP , CAP_MAC_OVERRIDE , CAP_MAC_ADMIN , CAP_SYSLOG , CAP_WAKE_ALARM , CAP_BLOCK_SUSPEND , CAP_AUDIT_READ }; -static const char* capabilityNames[] = {"CAP_CHOWN", "CAP_DAC_OVERRIDE", "CAP_DAC_READ_SEARCH", "CAP_FOWNER", "CAP_FSETID", "CAP_KILL", "CAP_SETGID", "CAP_SETUID", "CAP_SETPCAP", "CAP_LINUX_IMMUTABLE", "CAP_NET_BIND_SERVICE", "CAP_NET_BROADCAST", "CAP_NET_ADMIN", "CAP_NET_RAW", "CAP_IPC_LOCK", "CAP_IPC_OWNER", "CAP_SYS_MODULE", "CAP_SYS_RAWIO", "CAP_SYS_CHROOT", "CAP_SYS_PTRACE", "CAP_SYS_PACCT", "CAP_SYS_ADMIN", "CAP_SYS_BOOT", "CAP_SYS_NICE", "CAP_SYS_RESOURCE", "CAP_SYS_TIME", "CAP_SYS_TTY_CONFIG", "CAP_MKNOD", "CAP_LEASE", "CAP_AUDIT_WRITE", "CAP_AUDIT_CONTROL", "CAP_SETFCAP", "CAP_MAC_OVERRIDE", "CAP_MAC_ADMIN", "CAP_SYSLOG", "CAP_WAKE_ALARM", "CAP_BLOCK_SUSPEND", "CAP_AUDIT_READ"}; -static const unsigned int numCaps = sizeof(capabilityIntegers)/sizeof(const int); +static const unsigned int capabilityIDs[] = { CAP_CHOWN , CAP_DAC_OVERRIDE , CAP_DAC_READ_SEARCH , CAP_FOWNER , CAP_FSETID , CAP_KILL , CAP_SETGID , CAP_SETUID , CAP_SETPCAP , CAP_LINUX_IMMUTABLE , CAP_NET_BIND_SERVICE , CAP_NET_BROADCAST , CAP_NET_ADMIN , CAP_NET_RAW , CAP_IPC_LOCK , CAP_IPC_OWNER , CAP_SYS_MODULE , CAP_SYS_RAWIO , CAP_SYS_CHROOT , CAP_SYS_PTRACE , CAP_SYS_PACCT , CAP_SYS_ADMIN , CAP_SYS_BOOT , CAP_SYS_NICE , CAP_SYS_RESOURCE , CAP_SYS_TIME , CAP_SYS_TTY_CONFIG , CAP_MKNOD , CAP_LEASE , CAP_AUDIT_WRITE , CAP_AUDIT_CONTROL , CAP_SETFCAP , CAP_MAC_OVERRIDE , CAP_MAC_ADMIN , CAP_SYSLOG , CAP_WAKE_ALARM , CAP_BLOCK_SUSPEND , CAP_AUDIT_READ }; +static const char* capabilityNames[] = {"CAP_CHOWN", "CAP_DAC_OVERRIDE", "CAP_DAC_READ_SEARCH", "CAP_FOWNER", "CAP_FSETID", "CAP_KILL", "CAP_SETGID", "CAP_SETUID", "CAP_SETPCAP", "CAP_LINUX_IMMUTABLE", "CAP_NET_BIND_SERVICE", "CAP_NET_BROADCAST", "CAP_NET_ADMIN", "CAP_NET_RAW", "CAP_IPC_LOCK", "CAP_IPC_OWNER", "CAP_SYS_MODULE", "CAP_SYS_RAWIO", "CAP_SYS_CHROOT", "CAP_SYS_PTRACE", "CAP_SYS_PACCT", "CAP_SYS_ADMIN", "CAP_SYS_BOOT", "CAP_SYS_NICE", "CAP_SYS_RESOURCE", "CAP_SYS_TIME", "CAP_SYS_TTY_CONFIG", "CAP_MKNOD", "CAP_LEASE", "CAP_AUDIT_WRITE", "CAP_AUDIT_CONTROL", "CAP_SETFCAP", "CAP_MAC_OVERRIDE", "CAP_MAC_ADMIN", "CAP_SYSLOG", "CAP_WAKE_ALARM", "CAP_BLOCK_SUSPEND", "CAP_AUDIT_READ"}; +static const unsigned int numCaps = sizeof(capabilityIDs) / sizeof(const int); bool check_capabilities() { @@ -52,23 +52,24 @@ bool check_capabilities() if(config.debug & DEBUG_CAPS) { - logg("*********************************************************"); + logg("***************************************"); + logg("* Linux capability debugging enabled *"); for(unsigned int i = 0u; i < numCaps; i++) { - unsigned int capid = capabilityIntegers[i]; + const unsigned int capid = capabilityIDs[i]; // Check if capability is valid for the current kernel // If not, exit loop early if(!cap_valid(capid)) break; - logg("* DEBUG: Capability %-24s (%02u) = %s%s%s *", + logg("* %-24s (%02u) = %s%s%s *", capabilityNames[capid], capid, - ((data->permitted & (1 << capid)) ? "P":"-"), + ((data->permitted & (1 << capid)) ? "P":"-"), ((data->inheritable & (1 << capid)) ? "I":"-"), - ((data->effective & (1 << capid)) ? "E":"-")); + ((data->effective & (1 << capid)) ? "E":"-")); } - logg("*********************************************************"); + logg("***************************************"); } bool capabilities_okay = true; From 9461e0c9b08f875c7911d77aa0772caf25c21f17 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 19 Mar 2019 06:55:19 +0100 Subject: [PATCH 13/30] Review comments Signed-off-by: DL6ER --- capabilities.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/capabilities.c b/capabilities.c index dd20089a..5bb39e32 100644 --- a/capabilities.c +++ b/capabilities.c @@ -26,7 +26,6 @@ bool check_capabilities() cap_user_header_t hdr = calloc(sizeof(*hdr), capsize); // Determine capabilities version used by the current kernel - memset(hdr, 0, sizeof(*hdr)); capget(hdr, NULL); // Check version @@ -102,6 +101,6 @@ bool check_capabilities() free(hdr); free(data); - // All okay! + // Return whether capabilities are all okay return capabilities_okay; } From 09f0727e04e9fe2a3c51816340d8f0b8e1682489 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 19 Mar 2019 07:35:50 +0100 Subject: [PATCH 14/30] Type of capabilityIDs is const unsigned int Signed-off-by: DL6ER --- capabilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/capabilities.c b/capabilities.c index 5bb39e32..9290cc2c 100644 --- a/capabilities.c +++ b/capabilities.c @@ -16,7 +16,7 @@ static const unsigned int capabilityIDs[] = { CAP_CHOWN , CAP_DAC_OVERRIDE , CAP_DAC_READ_SEARCH , CAP_FOWNER , CAP_FSETID , CAP_KILL , CAP_SETGID , CAP_SETUID , CAP_SETPCAP , CAP_LINUX_IMMUTABLE , CAP_NET_BIND_SERVICE , CAP_NET_BROADCAST , CAP_NET_ADMIN , CAP_NET_RAW , CAP_IPC_LOCK , CAP_IPC_OWNER , CAP_SYS_MODULE , CAP_SYS_RAWIO , CAP_SYS_CHROOT , CAP_SYS_PTRACE , CAP_SYS_PACCT , CAP_SYS_ADMIN , CAP_SYS_BOOT , CAP_SYS_NICE , CAP_SYS_RESOURCE , CAP_SYS_TIME , CAP_SYS_TTY_CONFIG , CAP_MKNOD , CAP_LEASE , CAP_AUDIT_WRITE , CAP_AUDIT_CONTROL , CAP_SETFCAP , CAP_MAC_OVERRIDE , CAP_MAC_ADMIN , CAP_SYSLOG , CAP_WAKE_ALARM , CAP_BLOCK_SUSPEND , CAP_AUDIT_READ }; static const char* capabilityNames[] = {"CAP_CHOWN", "CAP_DAC_OVERRIDE", "CAP_DAC_READ_SEARCH", "CAP_FOWNER", "CAP_FSETID", "CAP_KILL", "CAP_SETGID", "CAP_SETUID", "CAP_SETPCAP", "CAP_LINUX_IMMUTABLE", "CAP_NET_BIND_SERVICE", "CAP_NET_BROADCAST", "CAP_NET_ADMIN", "CAP_NET_RAW", "CAP_IPC_LOCK", "CAP_IPC_OWNER", "CAP_SYS_MODULE", "CAP_SYS_RAWIO", "CAP_SYS_CHROOT", "CAP_SYS_PTRACE", "CAP_SYS_PACCT", "CAP_SYS_ADMIN", "CAP_SYS_BOOT", "CAP_SYS_NICE", "CAP_SYS_RESOURCE", "CAP_SYS_TIME", "CAP_SYS_TTY_CONFIG", "CAP_MKNOD", "CAP_LEASE", "CAP_AUDIT_WRITE", "CAP_AUDIT_CONTROL", "CAP_SETFCAP", "CAP_MAC_OVERRIDE", "CAP_MAC_ADMIN", "CAP_SYSLOG", "CAP_WAKE_ALARM", "CAP_BLOCK_SUSPEND", "CAP_AUDIT_READ"}; -static const unsigned int numCaps = sizeof(capabilityIDs) / sizeof(const int); +static const unsigned int numCaps = sizeof(capabilityIDs) / sizeof(const unsigned int); bool check_capabilities() { From a9dc5564aa1bc44d882fc013ed06bc7151480b6b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 5 Apr 2019 09:18:25 +0200 Subject: [PATCH 15/30] Only store hostnames of domains and upstream servers if they have changed. Currently, we are storing their host names each time we reresolve the IP addresses, even if they have been found to be identical. This allows us to save memory in FTL processes that are running for a long time without being restarted. Signed-off-by: DL6ER --- resolve.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/resolve.c b/resolve.c index 3bd7b0c3..3db35864 100644 --- a/resolve.c +++ b/resolve.c @@ -88,6 +88,7 @@ void resolveClients(bool onlynew) // Lock data when obtaining IP of this client lock_shm(); const char* ipaddr = getstr(clients[clientID].ippos); + const char* name = getstr(clients[clientID].namepos); unlock_shm(); // Important: Don't hold a lock while resolving as the main thread @@ -96,7 +97,13 @@ void resolveClients(bool onlynew) // Finally, lock data when storing obtained hostname lock_shm(); - clients[clientID].namepos = addstr(hostname); + // Only store new hostname if it changed + if(hostname != NULL && strcmp(name, hostname) != 0) + { + // We do not need to check for name == NULL as name is + // always initialized with an empty string at position 0 + clients[clientID].namepos = addstr(hostname); + } clients[clientID].new = false; unlock_shm(); } @@ -119,6 +126,7 @@ void resolveForwardDestinations(bool onlynew) // Lock data when obtaining IP of this forward destination lock_shm(); const char* ipaddr = getstr(forwarded[forwardID].ippos); + const char* name = getstr(forwarded[forwardID].namepos); unlock_shm(); @@ -128,7 +136,13 @@ void resolveForwardDestinations(bool onlynew) // Finally, lock data when storing obtained hostname lock_shm(); - forwarded[forwardID].namepos = addstr(hostname); + // Only store new hostname if it changed + if(hostname != NULL && strcmp(name, hostname) != 0) + { + // We do not need to check for name == NULL as name is + // always initialized with an empty string at position 0 + forwarded[forwardID].namepos = addstr(hostname); + } forwarded[forwardID].new = false; unlock_shm(); } From 4184141557d8f6e42b5af012368f04de76476db9 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 5 Apr 2019 09:28:54 +0200 Subject: [PATCH 16/30] Add debug output Signed-off-by: DL6ER --- resolve.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/resolve.c b/resolve.c index 3db35864..cdfbf24b 100644 --- a/resolve.c +++ b/resolve.c @@ -104,6 +104,12 @@ void resolveClients(bool onlynew) // always initialized with an empty string at position 0 clients[clientID].namepos = addstr(hostname); } + else if(config.debug & DEBUG_SHMEM) + { + // Debug output + logg("Not adding \"%s\" (unchanged)", name); + } + // Mark entry as not new clients[clientID].new = false; unlock_shm(); } @@ -143,6 +149,12 @@ void resolveForwardDestinations(bool onlynew) // always initialized with an empty string at position 0 forwarded[forwardID].namepos = addstr(hostname); } + else if(config.debug & DEBUG_SHMEM) + { + // Debug output + logg("Not adding \"%s\" (unchanged)", name); + } + // Mark entry as not new forwarded[forwardID].new = false; unlock_shm(); } From 540dba6a53e2c85e742eb730472633f51c6dd989 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 5 Apr 2019 09:31:38 +0200 Subject: [PATCH 17/30] Make sure we release allocated memory once we don't need it any longer Signed-off-by: DL6ER --- resolve.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/resolve.c b/resolve.c index cdfbf24b..46b73d32 100644 --- a/resolve.c +++ b/resolve.c @@ -19,7 +19,7 @@ // once every hour #define RERESOLVE_INTERVAL 3600 -static const char *resolveHostname(const char *addr) +static char *resolveHostname(const char *addr) { // Get host name struct hostent *he = NULL; @@ -93,10 +93,11 @@ void resolveClients(bool onlynew) // Important: Don't hold a lock while resolving as the main thread // (dnsmasq) needs to be operable during the call to resolveHostname() - const char* hostname = resolveHostname(ipaddr); + char* hostname = resolveHostname(ipaddr); // Finally, lock data when storing obtained hostname lock_shm(); + // Only store new hostname if it changed if(hostname != NULL && strcmp(name, hostname) != 0) { @@ -106,11 +107,17 @@ void resolveClients(bool onlynew) } else if(config.debug & DEBUG_SHMEM) { - // Debug output + // Debugging output logg("Not adding \"%s\" (unchanged)", name); } + + // Release allocated memory + free(hostname); + // Mark entry as not new clients[clientID].new = false; + + // Unlock shared memory unlock_shm(); } } @@ -138,7 +145,7 @@ void resolveForwardDestinations(bool onlynew) // Important: Don't hold a lock while resolving as the main thread // (dnsmasq) needs to be operable during the call to resolveHostname() - const char* hostname = resolveHostname(ipaddr); + char* hostname = resolveHostname(ipaddr); // Finally, lock data when storing obtained hostname lock_shm(); @@ -151,11 +158,17 @@ void resolveForwardDestinations(bool onlynew) } else if(config.debug & DEBUG_SHMEM) { - // Debug output + // Debugging output logg("Not adding \"%s\" (unchanged)", name); } + + // Release allocated memory + free(hostname); + // Mark entry as not new forwarded[forwardID].new = false; + + // Unlock shared memory unlock_shm(); } } From e24360517f16c75b07be8979f0c6ebfd8906d623 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 5 Apr 2019 09:35:33 +0200 Subject: [PATCH 18/30] Align debugging output to the output produced by addstr() Signed-off-by: DL6ER --- resolve.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resolve.c b/resolve.c index 46b73d32..7ff194d4 100644 --- a/resolve.c +++ b/resolve.c @@ -108,7 +108,7 @@ void resolveClients(bool onlynew) else if(config.debug & DEBUG_SHMEM) { // Debugging output - logg("Not adding \"%s\" (unchanged)", name); + logg("Not adding \"%s\" to buffer (unchanged)", name); } // Release allocated memory @@ -159,7 +159,7 @@ void resolveForwardDestinations(bool onlynew) else if(config.debug & DEBUG_SHMEM) { // Debugging output - logg("Not adding \"%s\" (unchanged)", name); + logg("Not adding \"%s\" to buffer (unchanged)", name); } // Release allocated memory From cc0b3782f14067aabe719edff0d871a09fe25369 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 10 Apr 2019 16:49:16 +0200 Subject: [PATCH 19/30] Review comments 1 Signed-off-by: DL6ER --- resolve.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/resolve.c b/resolve.c index 7ff194d4..d45a1d1d 100644 --- a/resolve.c +++ b/resolve.c @@ -99,10 +99,10 @@ void resolveClients(bool onlynew) lock_shm(); // Only store new hostname if it changed + // We do not need to check for name == NULL as name is + // always initialized with an empty string at position 0 if(hostname != NULL && strcmp(name, hostname) != 0) { - // We do not need to check for name == NULL as name is - // always initialized with an empty string at position 0 clients[clientID].namepos = addstr(hostname); } else if(config.debug & DEBUG_SHMEM) @@ -112,7 +112,8 @@ void resolveClients(bool onlynew) } // Release allocated memory - free(hostname); + if(hostname != NULL) + free(hostname); // Mark entry as not new clients[clientID].new = false; @@ -150,10 +151,10 @@ void resolveForwardDestinations(bool onlynew) // Finally, lock data when storing obtained hostname lock_shm(); // Only store new hostname if it changed + // We do not need to check for name == NULL as name is + // always initialized with an empty string at position 0 if(hostname != NULL && strcmp(name, hostname) != 0) { - // We do not need to check for name == NULL as name is - // always initialized with an empty string at position 0 forwarded[forwardID].namepos = addstr(hostname); } else if(config.debug & DEBUG_SHMEM) @@ -163,7 +164,8 @@ void resolveForwardDestinations(bool onlynew) } // Release allocated memory - free(hostname); + if(hostname != NULL) + free(hostname); // Mark entry as not new forwarded[forwardID].new = false; From 8e67784b81b4cb3817917bfcee3e2aeaa1276d45 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 12 Apr 2019 16:17:34 +0200 Subject: [PATCH 20/30] Swap out common code in resolve.c into its own subroutine Signed-off-by: DL6ER --- resolve.c | 105 ++++++++++++++++++++++++------------------------------ 1 file changed, 47 insertions(+), 58 deletions(-) diff --git a/resolve.c b/resolve.c index d45a1d1d..ad238d1c 100644 --- a/resolve.c +++ b/resolve.c @@ -71,6 +71,37 @@ static char *resolveHostname(const char *addr) return hostname; } +// Resolve upstream destination host names +static size_t resolveAndAddHostname(size_t ippos, size_t oldnamepos) +{ + // Get IP and host name strings + const char* ipaddr = getstr(ippos); + const char* oldname = getstr(oldnamepos); + + // Important: Don't hold a lock while resolving as the main thread + // (dnsmasq) needs to be operable during the call to resolveHostname() + char* newname = resolveHostname(ipaddr); + + // Only store new newname if it changed + if(newname != NULL && strcmp(oldname, newname) != 0) + { + // We do not need to check for name == NULL as name is + // always initialized with an empty string at position 0 + size_t newnamepos = addstr(newname); + if(newname != NULL) + free(newname); + return newnamepos; + } + else if(config.debug & DEBUG_SHMEM) + { + // Debugging output + logg("Not adding \"%s\" to buffer (unchanged)", oldname); + } + + // Not changed, return old namepos + return oldnamepos; +} + // Resolve client host names void resolveClients(bool onlynew) { @@ -85,41 +116,20 @@ void resolveClients(bool onlynew) if(onlynew && !clients[clientID].new) continue; - // Lock data when obtaining IP of this client - lock_shm(); - const char* ipaddr = getstr(clients[clientID].ippos); - const char* name = getstr(clients[clientID].namepos); - unlock_shm(); + // Obtain/update hostname of this client + size_t oldnamepos = clients[clientID].namepos; + size_t newnamepos = resolveAndAddHostname(clients[clientID].ippos, oldnamepos); - // Important: Don't hold a lock while resolving as the main thread - // (dnsmasq) needs to be operable during the call to resolveHostname() - char* hostname = resolveHostname(ipaddr); - - // Finally, lock data when storing obtained hostname - lock_shm(); - - // Only store new hostname if it changed - // We do not need to check for name == NULL as name is - // always initialized with an empty string at position 0 - if(hostname != NULL && strcmp(name, hostname) != 0) + if(newnamepos != oldnamepos) { - clients[clientID].namepos = addstr(hostname); + // Need lock when storing obtained hostname + lock_shm(); + clients[clientID].namepos = newnamepos; + unlock_shm(); } - else if(config.debug & DEBUG_SHMEM) - { - // Debugging output - logg("Not adding \"%s\" to buffer (unchanged)", name); - } - - // Release allocated memory - if(hostname != NULL) - free(hostname); // Mark entry as not new clients[clientID].new = false; - - // Unlock shared memory - unlock_shm(); } } @@ -137,41 +147,20 @@ void resolveForwardDestinations(bool onlynew) if(onlynew && !forwarded[forwardID].new) continue; - // Lock data when obtaining IP of this forward destination - lock_shm(); - const char* ipaddr = getstr(forwarded[forwardID].ippos); - const char* name = getstr(forwarded[forwardID].namepos); - unlock_shm(); + // Obtain/update hostname of this client + size_t oldnamepos = forwarded[forwardID].namepos; + size_t newnamepos = resolveAndAddHostname(forwarded[forwardID].ippos, oldnamepos); - - // Important: Don't hold a lock while resolving as the main thread - // (dnsmasq) needs to be operable during the call to resolveHostname() - char* hostname = resolveHostname(ipaddr); - - // Finally, lock data when storing obtained hostname - lock_shm(); - // Only store new hostname if it changed - // We do not need to check for name == NULL as name is - // always initialized with an empty string at position 0 - if(hostname != NULL && strcmp(name, hostname) != 0) + if(newnamepos != oldnamepos) { - forwarded[forwardID].namepos = addstr(hostname); + // Need lock when storing obtained hostname + lock_shm(); + forwarded[forwardID].namepos = newnamepos; + unlock_shm(); } - else if(config.debug & DEBUG_SHMEM) - { - // Debugging output - logg("Not adding \"%s\" to buffer (unchanged)", name); - } - - // Release allocated memory - if(hostname != NULL) - free(hostname); // Mark entry as not new forwarded[forwardID].new = false; - - // Unlock shared memory - unlock_shm(); } } From 9c4c9d701f0ed8b2eab7c07d207ed8f34d82bcd7 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 12 Apr 2019 16:23:50 +0200 Subject: [PATCH 21/30] Move global constants RESOLVE_INTERVAL and RERESOLVE_INTERVAL to FTL.h Signed-off-by: DL6ER --- FTL.h | 8 ++++++++ resolve.c | 8 -------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/FTL.h b/FTL.h index a4bba095..b1840879 100644 --- a/FTL.h +++ b/FTL.h @@ -80,6 +80,14 @@ // can be 24 hours + 59 minutes #define OVERTIME_SLOTS ((MAXLOGAGE+1)*3600/OVERTIME_INTERVAL) +// Interval for resolving NEW client and upstream server host names [seconds] +// Default: 60 (once every minute) +#define RESOLVE_INTERVAL 60 + +// Interval for re-resolving ALL known host names [seconds] +// Default: 3600 (once every hour) +#define RERESOLVE_INTERVAL 3600 + // FTLDNS enums enum { DATABASE_WRITE_TIMER, EXIT_TIMER, GC_TIMER, LISTS_TIMER, REGEX_TIMER, ARP_TIMER, LAST_TIMER }; enum { QUERIES, FORWARDED, CLIENTS, DOMAINS, OVERTIME, WILDCARD }; diff --git a/resolve.c b/resolve.c index ad238d1c..1191c676 100644 --- a/resolve.c +++ b/resolve.c @@ -11,14 +11,6 @@ #include "FTL.h" #include "shmem.h" -// Resolve new client and upstream server host names -// once every minute -#define RESOLVE_INTERVAL 60 - -// Re-resolve client names -// once every hour -#define RERESOLVE_INTERVAL 3600 - static char *resolveHostname(const char *addr) { // Get host name From 3ab4076738fed9b8888e93a7165b7eb02fdacb39 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 14:14:32 +0200 Subject: [PATCH 22/30] Lock SHM when using getstr() and addstr() Signed-off-by: DL6ER --- resolve.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/resolve.c b/resolve.c index 1191c676..219dd114 100644 --- a/resolve.c +++ b/resolve.c @@ -67,8 +67,10 @@ static char *resolveHostname(const char *addr) static size_t resolveAndAddHostname(size_t ippos, size_t oldnamepos) { // Get IP and host name strings + lock_shm(); const char* ipaddr = getstr(ippos); const char* oldname = getstr(oldnamepos); + unlock_shm(); // Important: Don't hold a lock while resolving as the main thread // (dnsmasq) needs to be operable during the call to resolveHostname() @@ -79,7 +81,9 @@ static size_t resolveAndAddHostname(size_t ippos, size_t oldnamepos) { // We do not need to check for name == NULL as name is // always initialized with an empty string at position 0 + lock_shm(); size_t newnamepos = addstr(newname); + unlock_shm(); if(newname != NULL) free(newname); return newnamepos; From 89957271ea6c45542b548b03d30de0645f4b8abd Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 20:56:24 +0200 Subject: [PATCH 23/30] Newname can be freed without if as we check for NULL already one level higher Signed-off-by: DL6ER --- resolve.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/resolve.c b/resolve.c index 219dd114..3052b663 100644 --- a/resolve.c +++ b/resolve.c @@ -76,16 +76,17 @@ static size_t resolveAndAddHostname(size_t ippos, size_t oldnamepos) // (dnsmasq) needs to be operable during the call to resolveHostname() char* newname = resolveHostname(ipaddr); - // Only store new newname if it changed + // Only store new newname if it is valid and differs from oldname + // We do not need to check for oldname == NULL as names are + // always initialized with an empty string at position 0 if(newname != NULL && strcmp(oldname, newname) != 0) { - // We do not need to check for name == NULL as name is - // always initialized with an empty string at position 0 lock_shm(); size_t newnamepos = addstr(newname); + // newname has already been checked against NULL + // so we can safely free it + free(newname); unlock_shm(); - if(newname != NULL) - free(newname); return newnamepos; } else if(config.debug & DEBUG_SHMEM) From cc88ed765afd6f260ff4a4c0371d8afc252acbbc Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 21:00:52 +0200 Subject: [PATCH 24/30] Lock more than less Signed-off-by: DL6ER --- resolve.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/resolve.c b/resolve.c index 3052b663..33a99861 100644 --- a/resolve.c +++ b/resolve.c @@ -126,7 +126,9 @@ void resolveClients(bool onlynew) } // Mark entry as not new + lock_shm(); clients[clientID].new = false; + unlock_shm(); } } @@ -157,7 +159,9 @@ void resolveForwardDestinations(bool onlynew) } // Mark entry as not new + lock_shm(); forwarded[forwardID].new = false; + unlock_shm(); } } From 9c0da2f2091bc5edb00456d6fcca8f9370cba271 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 21:10:24 +0200 Subject: [PATCH 25/30] Wrap all access to the forwarded and clients structures in locks. This is sometimes a bit cumbersome, however, it is necessary as we cannot completely lock the routines because this would prevent DNS resolution from being able to work Signed-off-by: DL6ER --- resolve.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/resolve.c b/resolve.c index 33a99861..251c1263 100644 --- a/resolve.c +++ b/resolve.c @@ -110,12 +110,18 @@ void resolveClients(bool onlynew) // If onlynew flag is set, we will only resolve new clients // If not, we will try to re-resolve all known clients - if(onlynew && !clients[clientID].new) + lock_shm(); + bool newlock = clients[clientID].new; + unlock_shm(); + if(onlynew && !newlock) continue; // Obtain/update hostname of this client + lock_shm(); size_t oldnamepos = clients[clientID].namepos; - size_t newnamepos = resolveAndAddHostname(clients[clientID].ippos, oldnamepos); + size_t ippos = clients[clientID].ippos; + unlock_shm(); + size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); if(newnamepos != oldnamepos) { @@ -143,12 +149,18 @@ void resolveForwardDestinations(bool onlynew) // If onlynew flag is set, we will only resolve new upstream destinations // If not, we will try to re-resolve all known upstream destinations - if(onlynew && !forwarded[forwardID].new) + lock_shm(); + bool newflag = forwarded[forwardID].new; + unlock_shm(); + if(onlynew && !newflag) continue; // Obtain/update hostname of this client + lock_shm(); size_t oldnamepos = forwarded[forwardID].namepos; - size_t newnamepos = resolveAndAddHostname(forwarded[forwardID].ippos, oldnamepos); + size_t ippos = forwarded[forwardID].ippos; + unlock_shm(); + size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); if(newnamepos != oldnamepos) { From b29b9abb49506a06dd7c5d866240d187f93dddbd Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 21:19:03 +0200 Subject: [PATCH 26/30] Also lock access to the counters and combine locks where convenient Signed-off-by: DL6ER --- resolve.c | 60 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/resolve.c b/resolve.c index 251c1263..80ad74f2 100644 --- a/resolve.c +++ b/resolve.c @@ -102,25 +102,28 @@ static size_t resolveAndAddHostname(size_t ippos, size_t oldnamepos) // Resolve client host names void resolveClients(bool onlynew) { - int clientID; - for(clientID = 0; clientID < counters->clients; clientID++) + // Lock counter access here, we use a copy in the following loop + lock_shm(); + int clientscount = counters->clients; + unlock_shm(); + for(int clientID = 0; clientID < clientscount; clientID++) { // Memory validation validate_access("clients", clientID, true, __LINE__, __FUNCTION__, __FILE__); - // If onlynew flag is set, we will only resolve new clients - // If not, we will try to re-resolve all known clients + // Memory access needs to get locked lock_shm(); bool newlock = clients[clientID].new; + size_t ippos = clients[clientID].ippos; + size_t oldnamepos = clients[clientID].namepos; unlock_shm(); + + // If onlynew flag is set, we will only resolve new clients + // If not, we will try to re-resolve all known clients if(onlynew && !newlock) continue; // Obtain/update hostname of this client - lock_shm(); - size_t oldnamepos = clients[clientID].namepos; - size_t ippos = clients[clientID].ippos; - unlock_shm(); size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); if(newnamepos != oldnamepos) @@ -128,38 +131,45 @@ void resolveClients(bool onlynew) // Need lock when storing obtained hostname lock_shm(); clients[clientID].namepos = newnamepos; + clients[clientID].new = false; unlock_shm(); } - // Mark entry as not new - lock_shm(); - clients[clientID].new = false; - unlock_shm(); + // Mark entry as not new even when we don't have a new host name + if(clients[clientID].new) + { + lock_shm(); + clients[clientID].new = false; + unlock_shm(); + } } } // Resolve upstream destination host names void resolveForwardDestinations(bool onlynew) { - int forwardID; - for(forwardID = 0; forwardID < counters->forwarded; forwardID++) + // Lock counter access here, we use a copy in the following loop + lock_shm(); + int forwardedcount = counters->forwarded; + unlock_shm(); + for(int forwardID = 0; forwardID < forwardedcount; forwardID++) { // Memory validation validate_access("forwarded", forwardID, true, __LINE__, __FUNCTION__, __FILE__); - // If onlynew flag is set, we will only resolve new upstream destinations - // If not, we will try to re-resolve all known upstream destinations + // Memory access needs to get locked lock_shm(); bool newflag = forwarded[forwardID].new; + size_t ippos = forwarded[forwardID].ippos; + size_t oldnamepos = forwarded[forwardID].namepos; unlock_shm(); + + // If onlynew flag is set, we will only resolve new upstream destinations + // If not, we will try to re-resolve all known upstream destinations if(onlynew && !newflag) continue; // Obtain/update hostname of this client - lock_shm(); - size_t oldnamepos = forwarded[forwardID].namepos; - size_t ippos = forwarded[forwardID].ippos; - unlock_shm(); size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); if(newnamepos != oldnamepos) @@ -167,13 +177,17 @@ void resolveForwardDestinations(bool onlynew) // Need lock when storing obtained hostname lock_shm(); forwarded[forwardID].namepos = newnamepos; + forwarded[forwardID].new = false; unlock_shm(); } // Mark entry as not new - lock_shm(); - forwarded[forwardID].new = false; - unlock_shm(); + if(forwarded[forwardID].new) + { + lock_shm(); + forwarded[forwardID].new = false; + unlock_shm(); + } } } From cfa9120ccc9ec3ed821d0c9e44382f92e963ac22 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 21:22:24 +0200 Subject: [PATCH 27/30] Use newflag instead of .new booleans to avoid needing a further lock Signed-off-by: DL6ER --- resolve.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/resolve.c b/resolve.c index 80ad74f2..2754260d 100644 --- a/resolve.c +++ b/resolve.c @@ -113,14 +113,14 @@ void resolveClients(bool onlynew) // Memory access needs to get locked lock_shm(); - bool newlock = clients[clientID].new; + bool newflag = clients[clientID].new; size_t ippos = clients[clientID].ippos; size_t oldnamepos = clients[clientID].namepos; unlock_shm(); // If onlynew flag is set, we will only resolve new clients // If not, we will try to re-resolve all known clients - if(onlynew && !newlock) + if(onlynew && !newflag) continue; // Obtain/update hostname of this client @@ -132,11 +132,12 @@ void resolveClients(bool onlynew) lock_shm(); clients[clientID].namepos = newnamepos; clients[clientID].new = false; + newflag = false; unlock_shm(); } // Mark entry as not new even when we don't have a new host name - if(clients[clientID].new) + if(newflag) { lock_shm(); clients[clientID].new = false; @@ -178,11 +179,12 @@ void resolveForwardDestinations(bool onlynew) lock_shm(); forwarded[forwardID].namepos = newnamepos; forwarded[forwardID].new = false; + newflag = false; unlock_shm(); } // Mark entry as not new - if(forwarded[forwardID].new) + if(newflag) { lock_shm(); forwarded[forwardID].new = false; From 7a5eb9d715fdf2e793dddf4b5355dd727d9d5f3e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 14 Apr 2019 21:35:57 +0200 Subject: [PATCH 28/30] Simplify code further. This removes the if statements at the end of the client/forward loops Signed-off-by: DL6ER --- resolve.c | 43 +++++++++++-------------------------------- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/resolve.c b/resolve.c index 2754260d..5cfdab1b 100644 --- a/resolve.c +++ b/resolve.c @@ -126,23 +126,13 @@ void resolveClients(bool onlynew) // Obtain/update hostname of this client size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); - if(newnamepos != oldnamepos) - { - // Need lock when storing obtained hostname - lock_shm(); - clients[clientID].namepos = newnamepos; - clients[clientID].new = false; - newflag = false; - unlock_shm(); - } + lock_shm(); + // Store obtained host name (may be unchanged) + clients[clientID].namepos = newnamepos; - // Mark entry as not new even when we don't have a new host name - if(newflag) - { - lock_shm(); - clients[clientID].new = false; - unlock_shm(); - } + // Mark entry as not new + clients[clientID].new = false; + unlock_shm(); } } @@ -173,23 +163,12 @@ void resolveForwardDestinations(bool onlynew) // Obtain/update hostname of this client size_t newnamepos = resolveAndAddHostname(ippos, oldnamepos); - if(newnamepos != oldnamepos) - { - // Need lock when storing obtained hostname - lock_shm(); - forwarded[forwardID].namepos = newnamepos; - forwarded[forwardID].new = false; - newflag = false; - unlock_shm(); - } - + lock_shm(); + // Store obtained host name (may be unchanged) + forwarded[forwardID].namepos = newnamepos; // Mark entry as not new - if(newflag) - { - lock_shm(); - forwarded[forwardID].new = false; - unlock_shm(); - } + forwarded[forwardID].new = false; + unlock_shm(); } } From 6fcbce6c347354aa1d6823f9837de79bfac389b6 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 15 Apr 2019 21:29:50 +0200 Subject: [PATCH 29/30] Improve code based on a full static analysis of our code. There are no real bugs, nowever, we use the obtained knowledge to improve the code such as unify declarations and function definitions, reduce the scope of local variables, mark many function arguments and local variables const when they are, fix the type expected by %x and fix one if-condition Signed-off-by: DL6ER --- api.c | 111 ++++++++++++++--------------- api.h | 54 +++++++------- args.c | 2 +- capabilities.c | 2 +- config.c | 4 +- daemon.c | 19 +++-- database.c | 38 +++++----- datastructure.c | 10 +-- dnsmasq_interface.c | 166 +++++++++++++++++++++++--------------------- dnsmasq_interface.h | 18 ++--- gc.c | 12 ++-- grep.c | 2 +- log.c | 20 +++--- memory.c | 15 ++-- msgpack.c | 44 ++++++------ networktable.c | 6 +- overTime.c | 16 ++--- regex.c | 15 ++-- resolve.c | 4 +- routines.h | 57 ++++++++------- shmem.c | 37 +++++----- shmem.h | 4 +- socket.c | 20 +++--- 23 files changed, 334 insertions(+), 342 deletions(-) diff --git a/api.c b/api.c index d74d40c0..357870db 100644 --- a/api.c +++ b/api.c @@ -19,8 +19,8 @@ /* qsort comparision function (count field), sort ASC */ static int __attribute__((pure)) cmpasc(const void *a, const void *b) { - int *elem1 = (int*)a; - int *elem2 = (int*)b; + const int *elem1 = (int*)a; + const int *elem2 = (int*)b; if (elem1[1] < elem2[1]) return -1; @@ -33,8 +33,8 @@ static int __attribute__((pure)) cmpasc(const void *a, const void *b) // qsort subroutine, sort DESC static int __attribute__((pure)) cmpdesc(const void *a, const void *b) { - int *elem1 = (int*)a; - int *elem2 = (int*)b; + const int *elem1 = (int*)a; + const int *elem2 = (int*)b; if (elem1[1] > elem2[1]) return -1; @@ -44,10 +44,10 @@ static int __attribute__((pure)) cmpdesc(const void *a, const void *b) return 0; } -void getStats(int *sock) +void getStats(const int *sock) { - int blocked = counters->blocked; - int total = counters->queries; + const int blocked = counters->blocked; + const int total = counters->queries; float percentage = 0.0f; // Avoid 1/0 condition @@ -111,7 +111,7 @@ void getStats(int *sock) pack_uint8(*sock, blockingstatus); } -void getOverTime(int *sock) +void getOverTime(const int *sock) { int i, from = 0, until = OVERTIME_SLOTS; bool found = false; @@ -171,12 +171,12 @@ void getOverTime(int *sock) } } -void getTopDomains(const char *client_message, int *sock) +void getTopDomains(const char *client_message, const int *sock) { int i, temparray[counters->domains][2], count=10, num; - bool blocked, audit = false, asc = false; + bool audit = false, asc = false; - blocked = command(client_message, ">top-ads"); + const bool blocked = command(client_message, ">top-ads"); // Exit before processing any data if requested via config setting get_privacy_level(NULL); @@ -224,7 +224,7 @@ void getTopDomains(const char *client_message, int *sock) // Get filter - char * filter = read_setupVarsconf("API_QUERY_LOG_SHOW"); + const char* filter = read_setupVarsconf("API_QUERY_LOG_SHOW"); bool showpermitted = true, showblocked = true; if(filter != NULL) { @@ -264,7 +264,7 @@ void getTopDomains(const char *client_message, int *sock) for(i=0; i < counters->domains; i++) { // Get sorted indices - int j = temparray[i][0]; + const int j = temparray[i][0]; validate_access("domains", j, true, __LINE__, __FUNCTION__, __FILE__); // Skip this domain if there is a filter on it @@ -333,7 +333,7 @@ void getTopDomains(const char *client_message, int *sock) clearSetupVarsArray(); } -void getTopClients(const char *client_message, int *sock) +void getTopClients(const char *client_message, const int *sock) { int i, temparray[counters->clients][2], count=10, num; @@ -389,7 +389,7 @@ void getTopClients(const char *client_message, int *sock) qsort(temparray, counters->clients, sizeof(int[2]), cmpdesc); // Get clients which the user doesn't want to see - char * excludeclients = read_setupVarsconf("API_EXCLUDE_CLIENTS"); + const char* excludeclients = read_setupVarsconf("API_EXCLUDE_CLIENTS"); if(excludeclients != NULL) { getSetupVarsArray(excludeclients); @@ -405,8 +405,8 @@ void getTopClients(const char *client_message, int *sock) for(i=0; i < counters->clients; i++) { // Get sorted indices and counter values (may be either total or blocked count) - int j = temparray[i][0]; - int ccount = temparray[i][1]; + const int j = temparray[i][0]; + const int ccount = temparray[i][1]; validate_access("clients", j, true, __LINE__, __FUNCTION__, __FILE__); // Skip this client if there is a filter on it @@ -447,7 +447,7 @@ void getTopClients(const char *client_message, int *sock) } -void getForwardDestinations(const char *client_message, int *sock) +void getForwardDestinations(const char *client_message, const int *sock) { bool sort = true; int temparray[counters->forwarded][2], totalqueries = 0; @@ -538,17 +538,17 @@ void getForwardDestinations(const char *client_message, int *sock) } -void getQueryTypes(int *sock) +void getQueryTypes(const int *sock) { - int i,total = 0; - for(i=0; i < TYPE_MAX-1; i++) + int total = 0; + for(int i=0; i < TYPE_MAX-1; i++) total += counters->querytype[i]; float percentage[TYPE_MAX-1] = { 0.0 }; // Prevent floating point exceptions by checking if the divisor is != 0 if(total > 0) - for(i=0; i < TYPE_MAX-1; i++) + for(int i=0; i < TYPE_MAX-1; i++) percentage[i] = 1e2f*counters->querytype[i]/total; if(istelnet[*sock]) { @@ -576,7 +576,7 @@ void getQueryTypes(int *sock) const char *querytypes[8] = {"A","AAAA","ANY","SRV","SOA","PTR","TXT","UNKN"}; -void getAllQueries(const char *client_message, int *sock) +void getAllQueries(const char *client_message, const int *sock) { // Exit before processing any data if requested via config setting get_privacy_level(NULL); @@ -631,10 +631,9 @@ void getAllQueries(const char *client_message, int *sock) else { // Iterate through all known forward destinations - int i; validate_access("forwards", MAX(0,counters->forwarded-1), true, __LINE__, __FUNCTION__, __FILE__); forwarddestid = -3; - for(i = 0; i < counters->forwarded; i++) + for(int i = 0; i < counters->forwarded; i++) { // Try to match the requested string against their IP addresses and // (if available) their host names @@ -664,9 +663,8 @@ void getAllQueries(const char *client_message, int *sock) sscanf(client_message, ">getallqueries-domain %255s", domainname); filterdomainname = true; // Iterate through all known domains - int i; validate_access("domains", MAX(0,counters->domains-1), true, __LINE__, __FUNCTION__, __FILE__); - for(i = 0; i < counters->domains; i++) + for(int i = 0; i < counters->domains; i++) { // Try to match the requested string if(strcmp(getstr(domains[i].domainpos), domainname) == 0) @@ -692,9 +690,8 @@ void getAllQueries(const char *client_message, int *sock) sscanf(client_message, ">getallqueries-client %255s", clientname); filterclientname = true; // Iterate through all known clients - int i; validate_access("clients", MAX(0,counters->clients-1), true, __LINE__, __FUNCTION__, __FILE__); - for(i = 0; i < counters->clients; i++) + for(int i = 0; i < counters->clients; i++) { // Try to match the requested string if(strcmp(getstr(clients[i].ippos), clientname) == 0 || @@ -742,8 +739,8 @@ void getAllQueries(const char *client_message, int *sock) } clearSetupVarsArray(); - int i; - for(i=ibeg; i < counters->queries; i++) + // Main loop + for(int i=ibeg; i < counters->queries; i++) { validate_access("queries", i, true, __LINE__, __FUNCTION__, __FILE__); // Check if this query has been create while in maximum privacy mode @@ -845,7 +842,7 @@ void getAllQueries(const char *client_message, int *sock) free(forwarddest); } -void getRecentBlocked(const char *client_message, int *sock) +void getRecentBlocked(const char *client_message, const int *sock) { int num=1; @@ -883,7 +880,7 @@ void getRecentBlocked(const char *client_message, int *sock) } } -void getClientID(int *sock) +void getClientID(const int *sock) { if(istelnet[*sock]) ssend(*sock,"%i\n", *sock); @@ -891,11 +888,11 @@ void getClientID(int *sock) pack_int32(*sock, *sock); } -void getQueryTypesOverTime(int *sock) +void getQueryTypesOverTime(const int *sock) { - int i, from = -1, until = OVERTIME_SLOTS; + int from = -1, until = OVERTIME_SLOTS; time_t mintime = overTime[0].timestamp; - for(i = 0; i < OVERTIME_SLOTS; i++) + for(int i = 0; i < OVERTIME_SLOTS; i++) { if((overTime[i].total > 0 || overTime[i].blocked > 0) && overTime[i].timestamp >= mintime) { @@ -905,7 +902,7 @@ void getQueryTypesOverTime(int *sock) } // End with last non-empty overTime slot - for(i = 0; i < OVERTIME_SLOTS; i++) + for(int i = 0; i < OVERTIME_SLOTS; i++) { if(overTime[i].timestamp >= time(NULL)) { @@ -918,7 +915,7 @@ void getQueryTypesOverTime(int *sock) if(from < 0) return; - for(i = from; i < until; i++) + for(int i = from; i < until; i++) { float percentageIPv4 = 0.0, percentageIPv6 = 0.0; int sum = overTime[i].querytypedata[0] + overTime[i].querytypedata[1]; @@ -938,7 +935,7 @@ void getQueryTypesOverTime(int *sock) } } -void getVersion(int *sock) +void getVersion(const int *sock) { const char * commit = GIT_HASH; const char * tag = GIT_TAG; @@ -987,7 +984,7 @@ void getVersion(int *sock) } } -void getDBstats(int *sock) +void getDBstats(const int *sock) { // Get file details struct stat st; @@ -1014,9 +1011,9 @@ void getDBstats(int *sock) } } -void getClientsOverTime(int *sock) +void getClientsOverTime(const int *sock) { - int i, sendit = -1, until = OVERTIME_SLOTS; + int sendit = -1, until = OVERTIME_SLOTS; // Exit before processing any data if requested via config setting get_privacy_level(NULL); @@ -1024,7 +1021,7 @@ void getClientsOverTime(int *sock) return; // Find minimum ID to send - for(i = 0; i < OVERTIME_SLOTS; i++) + for(int i = 0; i < OVERTIME_SLOTS; i++) { if((overTime[i].total > 0 || overTime[i].blocked > 0) && overTime[i].timestamp >= overTime[0].timestamp) @@ -1037,7 +1034,7 @@ void getClientsOverTime(int *sock) return; // Find minimum ID to send - for(i = 0; i < OVERTIME_SLOTS; i++) + for(int i = 0; i < OVERTIME_SLOTS; i++) { if(overTime[i].timestamp >= time(NULL)) { @@ -1058,7 +1055,7 @@ void getClientsOverTime(int *sock) { getSetupVarsArray(excludeclients); - for(i=0; i < counters->clients; i++) + for(int i=0; i < counters->clients; i++) { validate_access("clients", i, true, __LINE__, __FUNCTION__, __FILE__); // Check if this client should be skipped @@ -1069,7 +1066,7 @@ void getClientsOverTime(int *sock) } // Main return loop - for(i = sendit; i < until; i++) + for(int i = sendit; i < until; i++) { if(istelnet[*sock]) ssend(*sock, "%li", overTime[i].timestamp); @@ -1082,7 +1079,7 @@ void getClientsOverTime(int *sock) if(skipclient[j]) continue; - int thisclient = clients[j].overTime[i]; + const int thisclient = clients[j].overTime[i]; if(istelnet[*sock]) ssend(*sock, " %i", thisclient); @@ -1100,10 +1097,8 @@ void getClientsOverTime(int *sock) clearSetupVarsArray(); } -void getClientNames(int *sock) +void getClientNames(const int *sock) { - int i; - // Exit before processing any data if requested via config setting get_privacy_level(NULL); if(config.privacylevel >= PRIVACY_HIDE_DOMAINS_CLIENTS) @@ -1121,7 +1116,7 @@ void getClientNames(int *sock) { getSetupVarsArray(excludeclients); - for(i=0; i < counters->clients; i++) + for(int i=0; i < counters->clients; i++) { validate_access("clients", i, true, __LINE__, __FUNCTION__, __FILE__); // Check if this client should be skipped @@ -1132,7 +1127,7 @@ void getClientNames(int *sock) } // Loop over clients to generate output to be sent to the client - for(i = 0; i < counters->clients; i++) + for(int i = 0; i < counters->clients; i++) { validate_access("clients", i, true, __LINE__, __FUNCTION__, __FILE__); if(skipclient[i]) @@ -1153,15 +1148,14 @@ void getClientNames(int *sock) clearSetupVarsArray(); } -void getUnknownQueries(int *sock) +void getUnknownQueries(const int *sock) { // Exit before processing any data if requested via config setting get_privacy_level(NULL); if(config.privacylevel >= PRIVACY_HIDE_DOMAINS) return; - int i; - for(i=0; i < counters->queries; i++) + for(int i=0; i < counters->queries; i++) { validate_access("queries", i, true, __LINE__, __FUNCTION__, __FILE__); if(queries[i].status != QUERY_UNKNOWN && queries[i].complete) continue; @@ -1202,7 +1196,7 @@ void getUnknownQueries(int *sock) } } -void getDomainDetails(const char *client_message, int *sock) +void getDomainDetails(const char *client_message, const int *sock) { // Get domain name char domain[128]; @@ -1212,8 +1206,7 @@ void getDomainDetails(const char *client_message, int *sock) return; } - int i; - for(i = 0; i < counters->domains; i++) + for(int i = 0; i < counters->domains; i++) { validate_access("domains", i, true, __LINE__, __FUNCTION__, __FILE__); if(strcmp(getstr(domains[i].domainpos), domain) == 0) @@ -1224,7 +1217,7 @@ void getDomainDetails(const char *client_message, int *sock) const char *regexstatus; if(domains[i].regexmatch == REGEX_BLOCKED) regexstatus = "blocked"; - if(domains[i].regexmatch == REGEX_NOTBLOCKED) + else if(domains[i].regexmatch == REGEX_NOTBLOCKED) regexstatus = "not blocked"; else regexstatus = "unknown"; diff --git a/api.h b/api.h index ed2203d6..c329342d 100644 --- a/api.h +++ b/api.h @@ -9,36 +9,36 @@ * Please see LICENSE file for your rights under this license. */ // Statistic methods -void getStats(int *sock); -void getOverTime(int *sock); -void getTopDomains(const char *client_message, int *sock); -void getTopClients(const char *client_message, int *sock); -void getForwardDestinations(const char *client_message, int *sock); -void getQueryTypes(int *sock); -void getAllQueries(const char *client_message, int *sock); -void getRecentBlocked(const char *client_message, int *sock); -void getQueryTypesOverTime(int *sock); -void getClientsOverTime(int *sock); -void getClientNames(int *sock); -void getDomainDetails(const char *client_message, int *sock); +void getStats(const int *sock); +void getOverTime(const int *sock); +void getTopDomains(const char *client_message, const int *sock); +void getTopClients(const char *client_message, const int *sock); +void getForwardDestinations(const char *client_message, const int *sock); +void getQueryTypes(const int *sock); +void getAllQueries(const char *client_message, const int *sock); +void getRecentBlocked(const char *client_message, const int *sock); +void getQueryTypesOverTime(const int *sock); +void getClientsOverTime(const int *sock); +void getClientNames(const int *sock); +void getDomainDetails(const char *client_message, const int *sock); // FTL methods -void getClientID(int *sock); -void getVersion(int *sock); -void getDBstats(int *sock); -void getUnknownQueries(int *sock); +void getClientID(const int *sock); +void getVersion(const int *sock); +void getDBstats(const int *sock); +void getUnknownQueries(const int *sock); // DNS resolver methods (dnsmasq_interface.c) -void getCacheInformation(int *sock); +void getCacheInformation(const int *sock); // MessagePack serialization helpers -void pack_eom(int sock); -void pack_bool(int sock, bool value); -void pack_uint8(int sock, uint8_t value); -void pack_uint64(int sock, uint64_t value); -void pack_int32(int sock, int32_t value); -void pack_int64(int sock, int64_t value); -void pack_float(int sock, float value); -bool pack_fixstr(int sock, const char *string); -bool pack_str32(int sock, const char *string); -void pack_map16_start(int sock, uint16_t length); +void pack_eom(const int sock); +void pack_bool(const int sock, const bool value); +void pack_uint8(const int sock, const uint8_t value); +void pack_uint64(const int sock, const uint64_t value); +void pack_int32(const int sock, const int32_t value); +void pack_int64(const int sock, const int64_t value); +void pack_float(const int sock, const float value); +bool pack_fixstr(const int sock, const char *string); +bool pack_str32(const int sock, const char *string); +void pack_map16_start(const int sock, const uint16_t length); diff --git a/args.c b/args.c index 8e2bdbf5..eb03049e 100644 --- a/args.c +++ b/args.c @@ -52,7 +52,6 @@ void parse_args(int argc, char* argv[]) strcmp(argv[i], "version") == 0 || strcmp(argv[i], "--version") == 0) { - const char * commit = GIT_HASH; const char * tag = GIT_TAG; if(strlen(tag) > 1) { @@ -60,6 +59,7 @@ void parse_args(int argc, char* argv[]) } else { + const char * commit = GIT_HASH; char hash[8]; // Extract first 7 characters of the hash strncpy(hash, commit, 7); hash[7] = 0; diff --git a/capabilities.c b/capabilities.c index 9290cc2c..3d3a8e4b 100644 --- a/capabilities.c +++ b/capabilities.c @@ -18,7 +18,7 @@ static const unsigned int capabilityIDs[] = { CAP_CHOWN , CAP_DAC_OVERRIDE , static const char* capabilityNames[] = {"CAP_CHOWN", "CAP_DAC_OVERRIDE", "CAP_DAC_READ_SEARCH", "CAP_FOWNER", "CAP_FSETID", "CAP_KILL", "CAP_SETGID", "CAP_SETUID", "CAP_SETPCAP", "CAP_LINUX_IMMUTABLE", "CAP_NET_BIND_SERVICE", "CAP_NET_BROADCAST", "CAP_NET_ADMIN", "CAP_NET_RAW", "CAP_IPC_LOCK", "CAP_IPC_OWNER", "CAP_SYS_MODULE", "CAP_SYS_RAWIO", "CAP_SYS_CHROOT", "CAP_SYS_PTRACE", "CAP_SYS_PACCT", "CAP_SYS_ADMIN", "CAP_SYS_BOOT", "CAP_SYS_NICE", "CAP_SYS_RESOURCE", "CAP_SYS_TIME", "CAP_SYS_TTY_CONFIG", "CAP_MKNOD", "CAP_LEASE", "CAP_AUDIT_WRITE", "CAP_AUDIT_CONTROL", "CAP_SETFCAP", "CAP_MAC_OVERRIDE", "CAP_MAC_ADMIN", "CAP_SYSLOG", "CAP_WAKE_ALARM", "CAP_BLOCK_SUSPEND", "CAP_AUDIT_READ"}; static const unsigned int numCaps = sizeof(capabilityIDs) / sizeof(const unsigned int); -bool check_capabilities() +bool check_capabilities(void) { // First assume header version 1 int capsize = 1; // VFS_CAP_U32_1 diff --git a/config.c b/config.c index c867a6a7..460d0297 100644 --- a/config.c +++ b/config.c @@ -13,7 +13,7 @@ ConfigStruct config; static char *parse_FTLconf(FILE *fp, const char * key); static void release_config_memory(void); -void getpath(FILE* fp, const char *option, const char *defaultloc, char **pointer); +static void getpath(FILE* fp, const char *option, const char *defaultloc, char **pointer); char *conflinebuffer = NULL; @@ -334,7 +334,7 @@ void read_FTLconf(void) fclose(fp); } -void getpath(FILE* fp, const char *option, const char *defaultloc, char **pointer) +static void getpath(FILE* fp, const char *option, const char *defaultloc, char **pointer) { // This subroutine is used to read paths from pihole-FTL.conf // fp: File pointer to opened and readable config file diff --git a/daemon.c b/daemon.c index 9236c36a..ef4b6176 100644 --- a/daemon.c +++ b/daemon.c @@ -14,11 +14,8 @@ struct timeval t0[NUMTIMERS]; void go_daemon(void) { - pid_t process_id = 0; - pid_t sid = 0; - // Create child process - process_id = fork(); + pid_t process_id = fork(); // Indication of fork() failure if (process_id < 0) @@ -41,7 +38,7 @@ void go_daemon(void) //set new session // creates a session and sets the process group ID - sid = setsid(); + const pid_t sid = setsid(); if(sid < 0) { // Return failure @@ -80,7 +77,7 @@ void go_daemon(void) // Closing stdin, stdout and stderr is handled by dnsmasq } -void timer_start(int i) +void timer_start(const int i) { if(i >= NUMTIMERS) { @@ -90,7 +87,7 @@ void timer_start(int i) gettimeofday(&t0[i], 0); } -double timer_elapsed_msec(int i) +double timer_elapsed_msec(const int i) { if(i >= NUMTIMERS) { @@ -102,7 +99,7 @@ double timer_elapsed_msec(int i) return (t1.tv_sec - t0[i].tv_sec) * 1000.0f + (t1.tv_usec - t0[i].tv_usec) / 1000.0f; } -void sleepms(int milliseconds) +void sleepms(const int milliseconds) { struct timeval tv; tv.tv_sec = milliseconds / 1000; @@ -113,7 +110,7 @@ void sleepms(int milliseconds) void savepid(void) { FILE *f; - pid_t pid = getpid(); + const pid_t pid = getpid(); if((f = fopen(FTLfiles.pid, "w+")) == NULL) { logg("WARNING: Unable to write PID to file."); @@ -143,8 +140,8 @@ char *getUserName(void) char * name; // the getpwuid() function shall search the user database for an entry with a matching uid // the geteuid() function shall return the effective user ID of the calling process - this is used as the search criteria for the getpwuid() function - uid_t euid = geteuid(); - struct passwd *pw = getpwuid(euid); + const uid_t euid = geteuid(); + const struct passwd *pw = getpwuid(euid); if(pw) { name = strdup(pw->pw_name); diff --git a/database.c b/database.c index ee9cfe09..03cdc9bf 100644 --- a/database.c +++ b/database.c @@ -19,8 +19,8 @@ long int lastdbindex = 0; static pthread_mutex_t dblock; -bool db_set_counter(unsigned int ID, int value); -int db_get_FTL_property(unsigned int ID); +static bool db_set_counter(const unsigned int ID, const int value); +static int db_get_FTL_property(const unsigned int ID); static void check_database(int rc) { @@ -265,7 +265,7 @@ void db_init(void) database = true; } -int db_get_FTL_property(unsigned int ID) +static int db_get_FTL_property(const unsigned int ID) { // Prepare SQL statement char* querystr = NULL; @@ -283,17 +283,17 @@ int db_get_FTL_property(unsigned int ID) return value; } -bool db_set_FTL_property(unsigned int ID, int value) +bool db_set_FTL_property(const unsigned int ID, const int value) { return dbquery("INSERT OR REPLACE INTO ftl (id, value) VALUES ( %u, %i );", ID, value); } -bool db_set_counter(unsigned int ID, int value) +static bool db_set_counter(const unsigned int ID, const int value) { return dbquery("INSERT OR REPLACE INTO counters (id, value) VALUES ( %u, %i );", ID, value); } -static bool db_update_counters(int total, int blocked) +static bool db_update_counters(const int total, const int blocked) { if(!dbquery("UPDATE counters SET value = value + %i WHERE id = %i;", total, DB_TOTALQUERIES)) return false; @@ -429,7 +429,7 @@ void save_to_DB(void) unsigned int saved = 0, saved_error = 0; long int i; - sqlite3_stmt* stmt; + sqlite3_stmt* stmt = NULL; // Get last ID stored in the database sqlite3_int64 lastID = last_ID_in_DB(); @@ -599,7 +599,7 @@ static void delete_old_queries_in_DB(void) } // Get how many rows have been affected (deleted) - int affected = sqlite3_changes(db); + const int affected = sqlite3_changes(db); // Print final message only if there is a difference if((config.debug & DEBUG_DATABASE) || affected) @@ -674,8 +674,8 @@ void read_data_from_DB(void) // Prepare request char *rstr = NULL; // Get time stamp 24 hours in the past - time_t now = time(NULL); - time_t mintime = now - config.maxlogage; + const time_t now = time(NULL); + const time_t mintime = now - config.maxlogage; int rc = asprintf(&rstr, "SELECT * FROM queries WHERE timestamp >= %li", mintime); if(rc < 1) { @@ -686,7 +686,7 @@ void read_data_from_DB(void) if(config.debug & DEBUG_DATABASE) logg("%s", rstr); // Prepare SQLite3 statement - sqlite3_stmt* stmt; + sqlite3_stmt* stmt = NULL; rc = sqlite3_prepare_v2(db, rstr, -1, &stmt, NULL); if( rc ){ logg("read_data_from_DB() - SQL error prepare (%i): %s", rc, sqlite3_errmsg(db)); @@ -698,8 +698,8 @@ void read_data_from_DB(void) // Loop through returned database rows while((rc = sqlite3_step(stmt)) == SQLITE_ROW) { - sqlite3_int64 dbid = sqlite3_column_int64(stmt, 0); - time_t queryTimeStamp = sqlite3_column_int(stmt, 1); + const sqlite3_int64 dbid = sqlite3_column_int64(stmt, 0); + const time_t queryTimeStamp = sqlite3_column_int(stmt, 1); // 1483228800 = 01/01/2017 @ 12:00am (UTC) if(queryTimeStamp < 1483228800) { @@ -712,7 +712,7 @@ void read_data_from_DB(void) continue; } - int type = sqlite3_column_int(stmt, 2); + const int type = sqlite3_column_int(stmt, 2); if(type < TYPE_A || type >= TYPE_MAX) { logg("DB warn: TYPE should not be %i", type); @@ -725,7 +725,7 @@ void read_data_from_DB(void) continue; } - int status = sqlite3_column_int(stmt, 3); + const int status = sqlite3_column_int(stmt, 3); if(status < QUERY_UNKNOWN || status > QUERY_EXTERNAL_BLOCKED_NXRA) { logg("DB warn: STATUS should be within [%i,%i] but is %i", QUERY_UNKNOWN, QUERY_EXTERNAL_BLOCKED_NXRA, status); @@ -768,15 +768,15 @@ void read_data_from_DB(void) } // Obtain IDs only after filtering which queries we want to keep - int timeidx = getOverTimeID(queryTimeStamp); - int domainID = findDomainID(domain); - int clientID = findClientID(client, true); + const int timeidx = getOverTimeID(queryTimeStamp); + const int domainID = findDomainID(domain); + const int clientID = findClientID(client, true); // Ensure we have enough space in the queries struct memory_check(QUERIES); // Set index for this query - int queryIndex = counters->queries; + const int queryIndex = counters->queries; // Store this query in memory validate_access("queries", queryIndex, false, __LINE__, __FUNCTION__, __FILE__); diff --git a/datastructure.c b/datastructure.c index 429fc3b3..1338d152 100644 --- a/datastructure.c +++ b/datastructure.c @@ -17,7 +17,7 @@ void strtolower(char *str) while(str[i]){ str[i] = tolower(str[i]); i++; } } -int findForwardID(const char * forward, bool count) +int findForwardID(const char * forward, const bool count) { int i, forwardID = -1; if(counters->forwarded > 0) @@ -106,7 +106,7 @@ int findDomainID(const char *domain) return domainID; } -int findClientID(const char *client, bool count) +int findClientID(const char *client, const bool count) { // Compare content of client against known client IP addresses if(counters->clients > 0) @@ -181,7 +181,7 @@ bool isValidIPv6(const char *addr) // Privacy-level sensitive subroutine that returns the domain name // only when appropriate for the requested query -const char *getDomainString(int queryID) +const char *getDomainString(const int queryID) { if(queries[queryID].privacylevel < PRIVACY_HIDE_DOMAINS) { @@ -194,7 +194,7 @@ const char *getDomainString(int queryID) // Privacy-level sensitive subroutine that returns the client IP // only when appropriate for the requested query -const char *getClientIPString(int queryID) +const char *getClientIPString(const int queryID) { if(queries[queryID].privacylevel < PRIVACY_HIDE_DOMAINS_CLIENTS) { @@ -207,7 +207,7 @@ const char *getClientIPString(int queryID) // Privacy-level sensitive subroutine that returns the client host name // only when appropriate for the requested query -const char *getClientNameString(int queryID) +const char *getClientNameString(const int queryID) { if(queries[queryID].privacylevel < PRIVACY_HIDE_DOMAINS_CLIENTS) { diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index a62945f3..e3e381cf 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -17,18 +17,20 @@ // Prototype of getCacheInformation() #include "api.h" -void print_flags(unsigned int flags); -void save_reply_type(unsigned int flags, int queryID, struct timeval response); -static unsigned long converttimeval(struct timeval time) __attribute__((const)); -static void block_single_domain_regex(char *domain); -static void detect_blocked_IP(unsigned short flags, const char* answer, int queryID); -static void query_externally_blocked(int i, unsigned char status); -static int findQueryID(int id); +static void print_flags(const unsigned int flags); +static void save_reply_type(const unsigned int flags, const int queryID, const struct timeval response); +static unsigned long converttimeval(const struct timeval time) __attribute__((const)); +static void block_single_domain_regex(const char *domain); +static void detect_blocked_IP(const unsigned short flags, const char* answer, const int queryID); +static void query_externally_blocked(const int i, const unsigned char status); +static int findQueryID(const int id); unsigned char* pihole_privacylevel = &config.privacylevel; -char flagnames[28][12] = {"F_IMMORTAL ", "F_NAMEP ", "F_REVERSE ", "F_FORWARD ", "F_DHCP ", "F_NEG ", "F_HOSTS ", "F_IPV4 ", "F_IPV6 ", "F_BIGNAME ", "F_NXDOMAIN ", "F_CNAME ", "F_DNSKEY ", "F_CONFIG ", "F_DS ", "F_DNSSECOK ", "F_UPSTREAM ", "F_RRNAME ", "F_SERVER ", "F_QUERY ", "F_NOERR ", "F_AUTH ", "F_DNSSEC ", "F_KEYTAG ", "F_SECSTAT ", "F_NO_RR ", "F_IPSET ", "F_NOEXTRA "}; +const char flagnames[28][12] = {"F_IMMORTAL ", "F_NAMEP ", "F_REVERSE ", "F_FORWARD ", "F_DHCP ", "F_NEG ", "F_HOSTS ", "F_IPV4 ", "F_IPV6 ", "F_BIGNAME ", "F_NXDOMAIN ", "F_CNAME ", "F_DNSKEY ", "F_CONFIG ", "F_DS ", "F_DNSSECOK ", "F_UPSTREAM ", "F_RRNAME ", "F_SERVER ", "F_QUERY ", "F_NOERR ", "F_AUTH ", "F_DNSSEC ", "F_KEYTAG ", "F_SECSTAT ", "F_NO_RR ", "F_IPSET ", "F_NOEXTRA "}; -void _FTL_new_query(unsigned int flags, char *name, struct all_addr *addr, char *types, int id, char type, const char* file, const int line) +void _FTL_new_query(const unsigned int flags, const char *name, const struct all_addr *addr, + const char *types, const int id, const char type, + const char* file, const int line) { // Don't analyze anything if in PRIVACY_NOSTATS mode if(config.privacylevel >= PRIVACY_NOSTATS) @@ -38,7 +40,7 @@ void _FTL_new_query(unsigned int flags, char *name, struct all_addr *addr, char lock_shm(); // Get timestamp - time_t querytimestamp = time(NULL); + const time_t querytimestamp = time(NULL); // Save request time struct timeval request; @@ -78,7 +80,7 @@ void _FTL_new_query(unsigned int flags, char *name, struct all_addr *addr, char // Ensure we have enough space in the queries struct memory_check(QUERIES); - int queryID = counters->queries; + const int queryID = counters->queries; // Convert domain to lower case char *domain = strdup(name); @@ -124,7 +126,7 @@ void _FTL_new_query(unsigned int flags, char *name, struct all_addr *addr, char counters->querytype[querytype-1]++; // Update overTime - unsigned int timeidx = getOverTimeID(querytimestamp); + const unsigned int timeidx = getOverTimeID(querytimestamp); overTime[timeidx].querytypedata[querytype-1]++; // Skip rest of the analysis if this query is not of type A or AAAA @@ -141,10 +143,10 @@ void _FTL_new_query(unsigned int flags, char *name, struct all_addr *addr, char } // Go through already knows domains and see if it is one of them - int domainID = findDomainID(domain); + const int domainID = findDomainID(domain); // Go through already knows clients and see if it is one of them - int clientID = findClientID(client, true); + const int clientID = findClientID(client, true); // Save everything validate_access("queries", queryID, false, __LINE__, __FUNCTION__, __FILE__); @@ -223,7 +225,7 @@ void _FTL_new_query(unsigned int flags, char *name, struct all_addr *addr, char unlock_shm(); } -static int findQueryID(int id) +static int findQueryID(const int id) { // Loop over all queries - we loop in reverse order (start from the most recent query and // continuously walk older queries while trying to find a match. Ideally, we should always @@ -234,8 +236,8 @@ static int findQueryID(int id) // MAX(0, a) is used to return 0 in case a is negative (negative array indices are harmful) // Validate access only once for the maximum index (all lower will work) - int until = MAX(0, counters->queries-MAXITER); - int start = MAX(0, counters->queries-1); + const int until = MAX(0, counters->queries-MAXITER); + const int start = MAX(0, counters->queries-1); validate_access("queries", until, false, __LINE__, __FUNCTION__, __FILE__); // Check UUIDs of queries @@ -247,7 +249,8 @@ static int findQueryID(int id) return -1; } -void _FTL_forwarded(unsigned int flags, char *name, struct all_addr *addr, int id, const char* file, const int line) +void _FTL_forwarded(const unsigned int flags, const char *name, const struct all_addr *addr, const int id, + const char* file, const int line) { // Don't analyze anything if in PRIVACY_NOSTATS mode if(config.privacylevel >= PRIVACY_NOSTATS) @@ -270,7 +273,7 @@ void _FTL_forwarded(unsigned int flags, char *name, struct all_addr *addr, int i if(config.debug & DEBUG_QUERIES) logg("**** forwarded %s to %s (ID %i, %s:%i)", name, forward, id, file, line); // Save status and forwardID in corresponding query identified by dnsmasq's ID - int i = findQueryID(id); + const int i = findQueryID(id); if(i < 0) { // This may happen e.g. if the original query was a PTR query or "pi.hole" @@ -295,10 +298,10 @@ void _FTL_forwarded(unsigned int flags, char *name, struct all_addr *addr, int i // Get ID of forward destination, create new forward destination record // if not found in current data structure - int forwardID = findForwardID(forward, true); + const int forwardID = findForwardID(forward, true); queries[i].forwardID = forwardID; - unsigned int timeidx = queries[i].timeidx; + const unsigned int timeidx = queries[i].timeidx; if(queries[i].status == QUERY_CACHE) { @@ -390,7 +393,8 @@ void FTL_dnsmasq_reload(void) check_capabilities(); } -void _FTL_reply(unsigned short flags, char *name, struct all_addr *addr, int id, const char* file, const int line) +void _FTL_reply(const unsigned short flags, const char *name, const struct all_addr *addr, const int id, + const char* file, const int line) { // Don't analyze anything if in PRIVACY_NOSTATS mode if(config.privacylevel >= PRIVACY_NOSTATS) @@ -426,7 +430,7 @@ void _FTL_reply(unsigned short flags, char *name, struct all_addr *addr, int id, gettimeofday(&response, 0); // Save status in corresponding query identified by dnsmasq's ID - int i = findQueryID(id); + const int i = findQueryID(id); if(i < 0) { // This may happen e.g. if the original query was "pi.hole" @@ -443,9 +447,9 @@ void _FTL_reply(unsigned short flags, char *name, struct all_addr *addr, int id, } // Determine if this reply is an exact match for the queried domain - int domainID = queries[i].domainID; + const int domainID = queries[i].domainID; validate_access("domains", domainID, true, __LINE__, __FUNCTION__, __FILE__); - bool isExactMatch = (name != NULL && strcmp(getstr(domains[domainID].domainpos), name) == 0); + const bool isExactMatch = (name != NULL && strcmp(getstr(domains[domainID].domainpos), name) == 0); if((flags & F_CONFIG) && isExactMatch && !queries[i].complete) { @@ -454,7 +458,7 @@ void _FTL_reply(unsigned short flags, char *name, struct all_addr *addr, int id, counters->unknown--; // Get time index - unsigned int timeidx = queries[i].timeidx; + const unsigned int timeidx = queries[i].timeidx; if(strcmp(answer, "(NXDOMAIN)") == 0 || strcmp(answer, "0.0.0.0") == 0 || @@ -525,7 +529,7 @@ void _FTL_reply(unsigned short flags, char *name, struct all_addr *addr, int id, unlock_shm(); } -static void detect_blocked_IP(unsigned short flags, const char* answer, int queryID) +static void detect_blocked_IP(const unsigned short flags, const char* answer, const int queryID) { if(flags & F_HOSTS) { @@ -619,39 +623,40 @@ static void detect_blocked_IP(unsigned short flags, const char* answer, int quer } } -static void query_externally_blocked(int i, unsigned char status) +static void query_externally_blocked(const int queryID, const unsigned char status) { // If query is already known to be externally blocked, // then we have nothing to do here - if(queries[i].status == QUERY_EXTERNAL_BLOCKED_IP || - queries[i].status == QUERY_EXTERNAL_BLOCKED_NULL || - queries[i].status == QUERY_EXTERNAL_BLOCKED_NXRA) + if(queries[queryID].status == QUERY_EXTERNAL_BLOCKED_IP || + queries[queryID].status == QUERY_EXTERNAL_BLOCKED_NULL || + queries[queryID].status == QUERY_EXTERNAL_BLOCKED_NXRA) return; // Get time index of this query - unsigned int timeidx = queries[i].timeidx; + const unsigned int timeidx = queries[queryID].timeidx; // Correct counters if necessary ... - if(queries[i].status == QUERY_FORWARDED) + if(queries[queryID].status == QUERY_FORWARDED) { counters->forwardedqueries--; overTime[timeidx].forwarded--; - validate_access("forwarded", queries[i].forwardID, true, __LINE__, __FUNCTION__, __FILE__); - forwarded[queries[i].forwardID].count--; + validate_access("forwarded", queries[queryID].forwardID, true, __LINE__, __FUNCTION__, __FILE__); + forwarded[queries[queryID].forwardID].count--; } // ... but as blocked counters->blocked++; overTime[timeidx].blocked++; - validate_access("domains", queries[i].domainID, true, __LINE__, __FUNCTION__, __FILE__); - domains[queries[i].domainID].blockedcount++; - validate_access("clients", queries[i].clientID, true, __LINE__, __FUNCTION__, __FILE__); - clients[queries[i].clientID].blockedcount++; + validate_access("domains", queries[queryID].domainID, true, __LINE__, __FUNCTION__, __FILE__); + domains[queries[queryID].domainID].blockedcount++; + validate_access("clients", queries[queryID].clientID, true, __LINE__, __FUNCTION__, __FILE__); + clients[queries[queryID].clientID].blockedcount++; // Update status - queries[i].status = status; + queries[queryID].status = status; } -void _FTL_cache(unsigned int flags, char *name, struct all_addr *addr, char *arg, int id, const char* file, const int line) +void _FTL_cache(const unsigned int flags, const char *name, const struct all_addr *addr, + const char *arg, const int id, const char* file, const int line) { // Don't analyze anything if in PRIVACY_NOSTATS mode if(config.privacylevel >= PRIVACY_NOSTATS) @@ -729,8 +734,8 @@ void _FTL_cache(unsigned int flags, char *name, struct all_addr *addr, char *arg print_flags(flags); } - int i = findQueryID(id); - if(i < 0 || queries[i].complete) + const int queryID = findQueryID(id); + if(queryID < 0 || queries[queryID].complete) { // This may happen e.g. if the original query was a PTR query or "pi.hole" // as we ignore them altogether or if the query is already complete @@ -742,25 +747,25 @@ void _FTL_cache(unsigned int flags, char *name, struct all_addr *addr, char *arg counters->unknown--; // Get time index - unsigned int timeidx = queries[i].timeidx; + const unsigned int timeidx = queries[queryID].timeidx; - int domainID = queries[i].domainID; + const int domainID = queries[queryID].domainID; validate_access("domains", domainID, true, __LINE__, __FUNCTION__, __FILE__); - int clientID = queries[i].clientID; + const int clientID = queries[queryID].clientID; validate_access("clients", clientID, true, __LINE__, __FUNCTION__, __FILE__); // Mark this query as blocked if domain was matched by a regex if(domains[domainID].regexmatch == REGEX_BLOCKED) requesttype = QUERY_WILDCARD; - queries[i].status = requesttype; + queries[queryID].status = requesttype; // Detect if returned IP indicates that this query was blocked - detect_blocked_IP(flags, dest, i); + detect_blocked_IP(flags, dest, queryID); // Re-read requesttype as detect_blocked_IP() might have changed it - requesttype = queries[i].status; + requesttype = queries[queryID].status; // Handle counters accordingly switch(requesttype) @@ -786,10 +791,10 @@ void _FTL_cache(unsigned int flags, char *name, struct all_addr *addr, char *arg } // Save reply type and update individual reply counters - save_reply_type(flags, i, response); + save_reply_type(flags, queryID, response); // Hereby, this query is now fully determined - queries[i].complete = true; + queries[queryID].complete = true; } else { @@ -799,7 +804,7 @@ void _FTL_cache(unsigned int flags, char *name, struct all_addr *addr, char *arg unlock_shm(); } -void _FTL_dnssec(int status, int id, const char* file, const int line) +void _FTL_dnssec(const int status, const int id, const char* file, const int line) { // Don't analyze anything if in PRIVACY_NOSTATS mode if(config.privacylevel >= PRIVACY_NOSTATS) @@ -808,8 +813,8 @@ void _FTL_dnssec(int status, int id, const char* file, const int line) // Process DNSSEC result for a domain lock_shm(); // Search for corresponding query identified by ID - int i = findQueryID(id); - if(i < 0) + const int queryID = findQueryID(id); + if(queryID < 0) { // This may happen e.g. if the original query was an unhandled query type unlock_shm(); @@ -819,23 +824,23 @@ void _FTL_dnssec(int status, int id, const char* file, const int line) // Debug logging if(config.debug & DEBUG_QUERIES) { - int domainID = queries[i].domainID; + const int domainID = queries[queryID].domainID; validate_access("domains", domainID, true, __LINE__, __FUNCTION__, __FILE__); logg("**** got DNSSEC details for %s: %i (ID %i, %s:%i)", getstr(domains[domainID].domainpos), status, id, file, line); } // Iterate through possible values if(status == STAT_SECURE) - queries[i].dnssec = DNSSEC_SECURE; + queries[queryID].dnssec = DNSSEC_SECURE; else if(status == STAT_INSECURE) - queries[i].dnssec = DNSSEC_INSECURE; + queries[queryID].dnssec = DNSSEC_INSECURE; else - queries[i].dnssec = DNSSEC_BOGUS; + queries[queryID].dnssec = DNSSEC_BOGUS; unlock_shm(); } -void _FTL_upstream_error(unsigned int rcode, int id, const char* file, const int line) +void _FTL_upstream_error(const unsigned int rcode, const int id, const char* file, const int line) { // Process upstream errors // Queries with error are those where the RCODE @@ -848,8 +853,8 @@ void _FTL_upstream_error(unsigned int rcode, int id, const char* file, const int // Process DNSSEC result for a domain lock_shm(); // Search for corresponding query identified by ID - int i = findQueryID(id); - if(i < 0) + const int queryID = findQueryID(id); + if(queryID < 0) { // This may happen e.g. if the original query was an unhandled query type unlock_shm(); @@ -861,29 +866,29 @@ void _FTL_upstream_error(unsigned int rcode, int id, const char* file, const int { case SERVFAIL: rcodestr = "SERVFAIL"; - queries[i].reply = REPLY_SERVFAIL; + queries[queryID].reply = REPLY_SERVFAIL; break; case REFUSED: rcodestr = "REFUSED"; - queries[i].reply = REPLY_REFUSED; + queries[queryID].reply = REPLY_REFUSED; break; case NOTIMP: rcodestr = "NOT IMPLEMENTED"; - queries[i].reply = REPLY_NOTIMP; + queries[queryID].reply = REPLY_NOTIMP; break; default: rcodestr = "UNKNOWN"; - queries[i].reply = REPLY_OTHER; + queries[queryID].reply = REPLY_OTHER; break; } // Debug logging if(config.debug & DEBUG_QUERIES) { - int domainID = queries[i].domainID; + const int domainID = queries[queryID].domainID; validate_access("domains", domainID, true, __LINE__, __FUNCTION__, __FILE__); logg("**** got error report for %s: %s (ID %i, %s:%i)", getstr(domains[domainID].domainpos), rcodestr, id, file, line); - if(queries[i].reply == REPLY_OTHER) + if(queries[queryID].reply == REPLY_OTHER) { logg("Unknown rcode = %i", rcode); } @@ -914,7 +919,7 @@ void _FTL_header_analysis(const unsigned char header4, const unsigned int rcode, lock_shm(); // Search for corresponding query identified by ID - int queryID = findQueryID(id); + const int queryID = findQueryID(id); if(queryID < 0) { // This may happen e.g. if the original query was an unhandled query type @@ -924,7 +929,7 @@ void _FTL_header_analysis(const unsigned char header4, const unsigned int rcode, if(config.debug & DEBUG_QUERIES) { - int domainID = queries[queryID].domainID; + const int domainID = queries[queryID].domainID; validate_access("domains", domainID, true, __LINE__, __FUNCTION__, __FILE__); logg("**** %s externally blocked (ID %i, FTL %i, %s:%i)", getstr(domains[domainID].domainpos), id, queryID, file, line); } @@ -943,7 +948,7 @@ void _FTL_header_analysis(const unsigned char header4, const unsigned int rcode, unlock_shm(); } -void print_flags(unsigned int flags) +void print_flags(const unsigned int flags) { // Debug function, listing resolver flags in clear text // e.g. "Flags: F_FORWARD F_NEG F_IPV6" @@ -952,16 +957,15 @@ void print_flags(unsigned int flags) if(!(config.debug & DEBUG_FLAGS)) return; - unsigned int i; char *flagstr = calloc(256,sizeof(char)); - for(i = 0; i < sizeof(flags)*8; i++) + for(unsigned int i = 0; i < sizeof(flags)*8; i++) if(flags & (1u << i)) strcat(flagstr, flagnames[i]); logg(" Flags: %s", flagstr); free(flagstr); } -void save_reply_type(unsigned int flags, int queryID, struct timeval response) +void save_reply_type(const unsigned int flags, const int queryID, const struct timeval response) { // Iterate through possible values validate_access("queries", queryID, false, __LINE__, __FUNCTION__, __FILE__); @@ -1088,7 +1092,7 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw) } // int cache_inserted, cache_live_freed are defined in dnsmasq/cache.c -void getCacheInformation(int *sock) +void getCacheInformation(const int *sock) { ssend(*sock,"cache-size: %i\ncache-live-freed: %i\ncache-inserted: %i\n", daemon->cachesize, @@ -1104,7 +1108,7 @@ void getCacheInformation(int *sock) // which hasn't been looked up for the longest time is evicted. } -void _FTL_forwarding_failed(struct server *server, const char* file, const int line) +void _FTL_forwarding_failed(const struct server *server, const char* file, const int line) { // Don't analyze anything if in PRIVACY_NOSTATS mode if(config.privacylevel >= PRIVACY_NOSTATS) @@ -1121,7 +1125,7 @@ void _FTL_forwarding_failed(struct server *server, const char* file, const int l // Convert forward to lower case char *forward = strdup(dest); strtolower(forward); - int forwardID = findForwardID(forward, false); + const int forwardID = findForwardID(forward, false); if(config.debug & DEBUG_QUERIES) logg("**** forwarding to %s (ID %i, %s:%i) failed", dest, forwardID, file, line); @@ -1132,7 +1136,7 @@ void _FTL_forwarding_failed(struct server *server, const char* file, const int l return; } -static unsigned long __attribute__((const)) converttimeval(struct timeval time) +static unsigned long __attribute__((const)) converttimeval(const struct timeval time) { // Convert time from struct timeval into units // of 10*milliseconds @@ -1196,8 +1200,8 @@ void rehash(int size); // This routine adds one domain to the resolver's cache. Depending on the configured blocking mode it may create // a single entry valid for IPv4 & IPv6 or two entries one for IPv4 and one for IPv6. // When IPv6 is not available on the machine, we do not add IPv6 cache entries (likewise for IPv4) -static int add_blocked_domain(struct all_addr *addr4, struct all_addr *addr6, bool has_IPv4, bool has_IPv6, - char *domain, int len, struct crec **rhash, int hashsz, unsigned int index) +static int add_blocked_domain(struct all_addr *addr4, struct all_addr *addr6, const bool has_IPv4, const bool has_IPv6, + const char *domain, const int len, struct crec **rhash, int hashsz, unsigned int index) { int name_count = 0; struct crec *cache4,*cache6; @@ -1251,7 +1255,7 @@ static int add_blocked_domain(struct all_addr *addr4, struct all_addr *addr6, bo // Add a single domain to resolver's cache. This respects the configured blocking mode // Note: This routine is meant for adding a single domain at a time. It should not be // invoked for batch processing -static void block_single_domain_regex(char *domain) +static void block_single_domain_regex(const char *domain) { struct all_addr addr4 = {{{ 0 }}}, addr6 = {{{ 0 }}}; bool has_IPv4 = false, has_IPv6 = false; @@ -1266,7 +1270,7 @@ static void block_single_domain_regex(char *domain) return; } -int FTL_listsfile(char* filename, unsigned int index, FILE *f, int cache_size, struct crec **rhash, int hashsz) +int FTL_listsfile(const char* filename, unsigned int index, FILE *f, int cache_size, struct crec **rhash, int hashsz) { int name_count = cache_size; int added = 0; diff --git a/dnsmasq_interface.h b/dnsmasq_interface.h index 51b76af9..87a65469 100644 --- a/dnsmasq_interface.h +++ b/dnsmasq_interface.h @@ -12,29 +12,29 @@ extern unsigned char* pihole_privacylevel; enum { TCP, UDP }; #define FTL_new_query(flags, name, addr, types, id, type) _FTL_new_query(flags, name, addr, types, id, type, __FILE__, __LINE__) -void _FTL_new_query(unsigned int flags, char *name, struct all_addr *addr, char *types, int id, char type, const char* file, const int line); +void _FTL_new_query(const unsigned int flags, const char *name, const struct all_addr *addr, const char *types, const int id, const char type, const char* file, const int line); #define FTL_forwarded(flags, name, addr, id) _FTL_forwarded(flags, name, addr, id, __FILE__, __LINE__) -void _FTL_forwarded(unsigned int flags, char *name, struct all_addr *addr, int id, const char* file, const int line); +void _FTL_forwarded(const unsigned int flags, const char *name, const struct all_addr *addr, const int id, const char* file, const int line); #define FTL_reply(flags, name, addr, id) _FTL_reply(flags, name, addr, id, __FILE__, __LINE__) -void _FTL_reply(unsigned short flags, char *name, struct all_addr *addr, int id, const char* file, const int line); +void _FTL_reply(const unsigned short flags, const char *name, const struct all_addr *addr, const int id, const char* file, const int line); #define FTL_cache(flags, name, addr, arg, id) _FTL_cache(flags, name, addr, arg, id, __FILE__, __LINE__) -void _FTL_cache(unsigned int flags, char *name, struct all_addr *addr, char * arg, int id, const char* file, const int line); +void _FTL_cache(const unsigned int flags, const char *name, const struct all_addr *addr, const char * arg, const int id, const char* file, const int line); #define FTL_dnssec(status, id) _FTL_dnssec(status, id, __FILE__, __LINE__) -void _FTL_dnssec(int status, int id, const char* file, const int line); +void _FTL_dnssec(const int status, const int id, const char* file, const int line); #define FTL_header_analysis(header4, rcode, id) _FTL_header_analysis(header4, rcode, id, __FILE__, __LINE__) -void _FTL_header_analysis(unsigned char header4, unsigned int rcode, int id, const char* file, const int line); +void _FTL_header_analysis(const unsigned char header4, const unsigned int rcode, const int id, const char* file, const int line); #define FTL_forwarding_failed(server) _FTL_forwarding_failed(server, __FILE__, __LINE__) -void _FTL_forwarding_failed(struct server *server, const char* file, const int line); +void _FTL_forwarding_failed(const struct server *server, const char* file, const int line); #define FTL_upstream_error(rcode, id) _FTL_upstream_error(rcode, id, __FILE__, __LINE__) -void _FTL_upstream_error(unsigned int rcode, int id, const char* file, const int line); +void _FTL_upstream_error(const unsigned int rcode, const int id, const char* file, const int line); void FTL_dnsmasq_reload(void); void FTL_fork_and_bind_sockets(struct passwd *ent_pw); -int FTL_listsfile(char* filename, unsigned int index, FILE *f, int cache_size, struct crec **rhash, int hashsz); +int FTL_listsfile(const char* filename, unsigned int index, FILE *f, int cache_size, struct crec **rhash, int hashsz); diff --git a/gc.c b/gc.c index 79dabbee..d5484655 100644 --- a/gc.c +++ b/gc.c @@ -44,12 +44,11 @@ void *GC_thread(void *val) if(config.debug & DEBUG_GC) timer_start(GC_TIMER); - long int i; int removed = 0; if(config.debug & DEBUG_GC) logg("GC starting, mintime: %lu %s", mintime, ctime(&mintime)); // Process all queries - for(i=0; i < counters->queries; i++) + for(long int i=0; i < counters->queries; i++) { validate_access("queries", i, true, __LINE__, __FUNCTION__, __FILE__); // Test if this query is too new @@ -57,18 +56,18 @@ void *GC_thread(void *val) break; // Adjust client counter - int clientID = queries[i].clientID; + const int clientID = queries[i].clientID; validate_access("clients", clientID, true, __LINE__, __FUNCTION__, __FILE__); clients[clientID].count--; // Adjust total counters and total over time data - int timeidx = queries[i].timeidx; + const int timeidx = queries[i].timeidx; overTime[timeidx].total--; // Adjust corresponding overTime counters clients[clientID].overTime[timeidx]--; // Adjust domain counter (no overTime information) - int domainID = queries[i].domainID; + const int domainID = queries[i].domainID; validate_access("domains", domainID, true, __LINE__, __FUNCTION__, __FILE__); domains[domainID].count--; @@ -164,7 +163,8 @@ void *GC_thread(void *val) // Determine if overTime memory needs to get moved moveOverTimeMemory(mintime); - if(config.debug & DEBUG_GC) logg("Notice: GC removed %i queries (took %.2f ms)", removed, timer_elapsed_msec(GC_TIMER)); + if(config.debug & DEBUG_GC) + logg("Notice: GC removed %i queries (took %.2f ms)", removed, timer_elapsed_msec(GC_TIMER)); // Release thread lock unlock_shm(); diff --git a/grep.c b/grep.c index 09cba3c8..628940a3 100644 --- a/grep.c +++ b/grep.c @@ -105,7 +105,7 @@ int countlineswith(const char* str, const char* fname) void check_blocking_status(void) { - char* blocking = read_setupVarsconf("BLOCKING_ENABLED"); + const char* blocking = read_setupVarsconf("BLOCKING_ENABLED"); const char* message; if(blocking == NULL || getSetupVarsBool(blocking)) diff --git a/log.c b/log.c index fe71c417..01a376b2 100644 --- a/log.c +++ b/log.c @@ -11,8 +11,8 @@ #include "FTL.h" #include "version.h" -pthread_mutex_t lock; -FILE *logfile = NULL; +static pthread_mutex_t lock; +static FILE *logfile = NULL; static void close_FTL_log(void) { @@ -20,7 +20,7 @@ static void close_FTL_log(void) fclose(logfile); } -void open_FTL_log(bool test) +void open_FTL_log(const bool test) { if(test) { @@ -52,11 +52,11 @@ void open_FTL_log(bool test) static void get_timestr(char *timestring) { - time_t t = time(NULL); - struct tm tm = *localtime(&t); + const time_t t = time(NULL); + const struct tm tm = *localtime(&t); struct timeval tv; gettimeofday(&tv, NULL); - int millisec = tv.tv_usec/1000; + const int millisec = tv.tv_usec/1000; sprintf(timestring,"%d-%02d-%02d %02d:%02d:%02d.%03i", tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec, millisec); } @@ -72,7 +72,7 @@ void __attribute__ ((format (gnu_printf, 1, 2))) logg(const char *format, ...) // Get and log PID of current process to avoid ambiguities when more than one // pihole-FTL instance is logging into the same file - long pid = (long)getpid(); + const long pid = (long)getpid(); // Print to stdout before writing to file if(!daemonmode) @@ -108,7 +108,7 @@ void __attribute__ ((format (gnu_printf, 1, 2))) logg(const char *format, ...) pthread_mutex_unlock(&lock); } -void format_memory_size(char *prefix, unsigned long int bytes, double *formated) +void format_memory_size(char *prefix, const unsigned long int bytes, double *formated) { int i; *formated = bytes; @@ -124,7 +124,7 @@ void format_memory_size(char *prefix, unsigned long int bytes, double *formated) strcpy(prefix, prefixes[i]); } -void logg_struct_resize(const char* str, int to, int step) +void logg_struct_resize(const char* str, const int to, const int step) { logg("Notice: Increasing %s struct size from %i to %i", str, (to-step), to); } @@ -141,7 +141,7 @@ void log_counter_info(void) logg(" -> Known forward destinations: %i", counters->forwarded); } -void log_FTL_version(bool crashreport) +void log_FTL_version(const bool crashreport) { logg("FTL branch: %s", GIT_BRANCH); logg("FTL version: %s", GIT_TAG); diff --git a/memory.c b/memory.c index 503e84d1..a0580dc5 100644 --- a/memory.c +++ b/memory.c @@ -44,7 +44,7 @@ clientsDataStruct *clients = NULL; domainsDataStruct *domains = NULL; overTimeDataStruct *overTime = NULL; -void memory_check(int which) +void memory_check(const int which) { switch(which) { @@ -104,7 +104,8 @@ void memory_check(int which) } } -void validate_access(const char * name, int pos, bool testmagic, int line, const char * function, const char * file) +void validate_access(const char * name, const int pos, const bool testmagic, + const int line, const char * function, const char * file) { int limit = 0; if(name[0] == 'c') limit = counters->clients_MAX; @@ -141,7 +142,7 @@ void validate_access(const char * name, int pos, bool testmagic, int line, const // not be protected by our (error logging) functions! #undef strdup -char* __attribute__((malloc)) FTLstrdup(const char *src, const char * file, const char * function, int line) +char* __attribute__((malloc)) FTLstrdup(const char *src, const char * file, const char * function, const int line) { // The FTLstrdup() function returns a pointer to a new string which is a // duplicate of the string s. Memory for the new string is obtained with @@ -151,7 +152,7 @@ char* __attribute__((malloc)) FTLstrdup(const char *src, const char * file, cons logg("WARN: Trying to copy a NULL string in %s() (%s:%i)", function, file, line); return NULL; } - size_t len = strlen(src); + const size_t len = strlen(src); char *dest = calloc(len+1, sizeof(char)); if(dest == NULL) { @@ -166,7 +167,7 @@ char* __attribute__((malloc)) FTLstrdup(const char *src, const char * file, cons } #undef calloc -void* __attribute__((malloc)) __attribute__((alloc_size(1,2))) FTLcalloc(size_t nmemb, size_t size, const char * file, const char * function, int line) +void* __attribute__((malloc)) __attribute__((alloc_size(1,2))) FTLcalloc(const size_t nmemb, const size_t size, const char * file, const char * function, const int line) { // The FTLcalloc() function allocates memory for an array of nmemb elements // of size bytes each and returns a pointer to the allocated memory. The @@ -182,7 +183,7 @@ void* __attribute__((malloc)) __attribute__((alloc_size(1,2))) FTLcalloc(size_t } #undef realloc -void __attribute__((alloc_size(2))) *FTLrealloc(void *ptr_in, size_t size, const char * file, const char * function, int line) +void __attribute__((alloc_size(2))) *FTLrealloc(void *ptr_in, const size_t size, const char * file, const char * function, const int line) { // The FTLrealloc() function changes the size of the memory block pointed to // by ptr to size bytes. The contents will be unchanged in the range from @@ -203,7 +204,7 @@ void __attribute__((alloc_size(2))) *FTLrealloc(void *ptr_in, size_t size, const } #undef free -void FTLfree(void *ptr, const char * file, const char * function, int line) +void FTLfree(void *ptr, const char * file, const char * function, const int line) { // The free() function frees the memory space pointed to by ptr, which // must have been returned by a previous call to malloc(), calloc(), or diff --git a/msgpack.c b/msgpack.c index 58cc7ee9..07b61341 100644 --- a/msgpack.c +++ b/msgpack.c @@ -11,19 +11,19 @@ #include "FTL.h" #include "api.h" -void pack_eom(int sock) { +void pack_eom(const int sock) { // This byte is explicitly never used in the MessagePack spec, so it is perfect to use as an EOM for this API. uint8_t eom = 0xc1; swrite(sock, &eom, sizeof(eom)); } -static void pack_basic(int sock, uint8_t format, void *value, size_t size) { +static void pack_basic(const int sock, const uint8_t format, const void *value, const size_t size) { swrite(sock, &format, sizeof(format)); swrite(sock, value, size); } -static uint64_t __attribute__((const)) leToBe64(uint64_t value) { - char *ptr = (char *) &value; +static uint64_t __attribute__((const)) leToBe64(const uint64_t value) { + const char *ptr = (char *) &value; uint32_t part1, part2; // Copy the two halves of the 64 bit input into uint32_t's so we can use htonl @@ -38,26 +38,26 @@ static uint64_t __attribute__((const)) leToBe64(uint64_t value) { return (uint64_t) part1 << 32 | part2; } -void pack_bool(int sock, bool value) { +void pack_bool(const int sock, const bool value) { uint8_t packed = (uint8_t) (value ? 0xc3 : 0xc2); swrite(sock, &packed, sizeof(packed)); } -void pack_uint8(int sock, uint8_t value) { +void pack_uint8(const int sock, const uint8_t value) { pack_basic(sock, 0xcc, &value, sizeof(value)); } -void pack_uint64(int sock, uint64_t value) { - uint64_t bigEValue = leToBe64(value); +void pack_uint64(const int sock, const uint64_t value) { + const uint64_t bigEValue = leToBe64(value); pack_basic(sock, 0xcf, &bigEValue, sizeof(bigEValue)); } -void pack_int32(int sock, int32_t value) { - uint32_t bigEValue = htonl((uint32_t) value); +void pack_int32(const int sock, const int32_t value) { + const uint32_t bigEValue = htonl((uint32_t) value); pack_basic(sock, 0xd2, &bigEValue, sizeof(bigEValue)); } -void pack_int64(int sock, int64_t value) { +void pack_int64(const int sock, const int64_t value) { // Need to use memcpy to do a direct copy without reinterpreting the bytes (making negatives into positives). // It should get optimized away. uint64_t bigEValue; @@ -66,7 +66,7 @@ void pack_int64(int sock, int64_t value) { pack_basic(sock, 0xd3, &bigEValue, sizeof(bigEValue)); } -void pack_float(int sock, float value) { +void pack_float(const int sock, const float value) { // Need to use memcpy to do a direct copy without reinterpreting the bytes. It should get optimized away. uint32_t bigEValue; memcpy(&bigEValue, &value, sizeof(bigEValue)); @@ -75,16 +75,16 @@ void pack_float(int sock, float value) { } // Return true if successful -bool pack_fixstr(int sock, const char *string) { +bool pack_fixstr(const int sock, const char *string) { // Make sure that the length is less than 32 - size_t length = strlen(string); + const size_t length = strlen(string); if(length >= 32) { logg("Tried to send a fixstr longer than 31 bytes!"); return false; } - uint8_t format = (uint8_t) (0xA0 | length); + const uint8_t format = (uint8_t) (0xA0 | length); swrite(sock, &format, sizeof(format)); swrite(sock, string, length); @@ -92,27 +92,27 @@ bool pack_fixstr(int sock, const char *string) { } // Return true if successful -bool pack_str32(int sock, const char *string) { +bool pack_str32(const int sock, const char *string) { // Make sure that the length is less than 4294967296 - size_t length = strlen(string); + const size_t length = strlen(string); if(length >= 2147483648u) { logg("Tried to send a str32 longer than 2147483647 bytes!"); return false; } - uint8_t format = 0xdb; + const uint8_t format = 0xdb; swrite(sock, &format, sizeof(format)); - uint32_t bigELength = htonl((uint32_t) length); + const uint32_t bigELength = htonl((uint32_t) length); swrite(sock, &bigELength, sizeof(bigELength)); swrite(sock, string, length); return true; } -void pack_map16_start(int sock, uint16_t length) { - uint8_t format = 0xde; +void pack_map16_start(const int sock, const uint16_t length) { + const uint8_t format = 0xde; swrite(sock, &format, sizeof(format)); - uint16_t bigELength = htons(length); + const uint16_t bigELength = htons(length); swrite(sock, &bigELength, sizeof(bigELength)); } diff --git a/networktable.c b/networktable.c index 5d47f4f0..bbba093f 100644 --- a/networktable.c +++ b/networktable.c @@ -65,7 +65,7 @@ void parse_arp_cache(void) char * linebuffer = NULL; size_t linebuffersize = 0; char ip[100], mask[100], hwaddr[100], iface[100]; - int type, flags, entries = 0; + unsigned int type, flags, entries = 0; time_t now = time(NULL); // Start collecting database commands @@ -101,7 +101,7 @@ void parse_arp_cache(void) } // Perform SQL query - int dbID = db_query_int(querystr); + const int dbID = db_query_int(querystr); free(querystr); if(dbID == DB_FAILED) @@ -120,7 +120,7 @@ void parse_arp_cache(void) // This client is known (by its IP address) to pihole-FTL if // findClientID() returned a non-negative index - bool clientKnown = clientID >= 0; + const bool clientKnown = clientID >= 0; // Get hostname of this client if the client is known const char *hostname = ""; diff --git a/overTime.c b/overTime.c index 207d0964..1da92e89 100644 --- a/overTime.c +++ b/overTime.c @@ -16,7 +16,7 @@ * @param index The overTime slot index * @param timestamp The timestamp of the slot */ -static void initSlot(unsigned int index, time_t timestamp) +static void initSlot(const unsigned int index, const time_t timestamp) { // Possible debug printing if(config.debug & DEBUG_OVERTIME) @@ -67,10 +67,10 @@ unsigned int getOverTimeID(time_t timestamp) timestamp += OVERTIME_INTERVAL/2; // Get timestamp of first interval - time_t firstTimestamp = overTime[0].timestamp; + const time_t firstTimestamp = overTime[0].timestamp; // Compute overTime ID - int id = (int) ((timestamp - firstTimestamp) / OVERTIME_INTERVAL); + const int id = (int) ((timestamp - firstTimestamp) / OVERTIME_INTERVAL); // Check bounds manually if(id < 0) @@ -93,9 +93,9 @@ unsigned int getOverTimeID(time_t timestamp) } // This routine is called by garbage collection to rearrange the overTime structure for the next hour -void moveOverTimeMemory(time_t mintime) +void moveOverTimeMemory(const time_t mintime) { - time_t oldestOverTimeIS = overTime[0].timestamp; + const time_t oldestOverTimeIS = overTime[0].timestamp; // Shift SHOULD timestemp into the future by the amount GC is running earlier time_t oldestOverTimeSHOULD = mintime; @@ -105,10 +105,10 @@ void moveOverTimeMemory(time_t mintime) // Calculate the number of slots to be garbage collected, which is also the // ID of the slot to move to the zero position - unsigned int moveOverTime = (unsigned int) ((oldestOverTimeSHOULD - oldestOverTimeIS) / OVERTIME_INTERVAL); + const unsigned int moveOverTime = (unsigned int) ((oldestOverTimeSHOULD - oldestOverTimeIS) / OVERTIME_INTERVAL); // The number of slots which will be moved (not garbage collected) - unsigned int remainingSlots = OVERTIME_SLOTS - moveOverTime; + const unsigned int remainingSlots = OVERTIME_SLOTS - moveOverTime; if(config.debug & DEBUG_OVERTIME) logg("moveOverTimeMemory(): IS: %lu, SHOULD: %lu, MOVING: %u", oldestOverTimeIS, oldestOverTimeSHOULD, moveOverTime); @@ -148,7 +148,7 @@ void moveOverTimeMemory(time_t mintime) for(unsigned int timeidx = remainingSlots; timeidx < OVERTIME_SLOTS ; timeidx++) { // This slot is OVERTIME_INTERVAL seconds after the previous slot - time_t timestamp = overTime[timeidx-1].timestamp + OVERTIME_INTERVAL; + const time_t timestamp = overTime[timeidx-1].timestamp + OVERTIME_INTERVAL; initSlot(timeidx, timestamp); } } diff --git a/regex.c b/regex.c index 68678bf6..8c8eb96a 100644 --- a/regex.c +++ b/regex.c @@ -17,22 +17,22 @@ static bool *regexconfigured = NULL; static char **regexbuffer = NULL; static whitelistStruct whitelist = { NULL, 0 }; -static void log_regex_error(const char *where, int errcode, int index) +static void log_regex_error(const char *where, const int errcode, const int index) { // Regex failed for some reason (probably user syntax error) // Get error string and log it - size_t length = regerror(errcode, ®ex[index], NULL, 0); + const size_t length = regerror(errcode, ®ex[index], NULL, 0); char *buffer = calloc(length,sizeof(char)); (void) regerror (errcode, ®ex[index], buffer, length); logg("ERROR %s regex on line %i: %s (%i)", where, index+1, buffer, errcode); free(buffer); } -static bool init_regex(const char *regexin, int index) +static bool init_regex(const char *regexin, const int index) { // compile regular expressions into data structures that // can be used with regexec to match against a string - int errcode = regcomp(®ex[index], regexin, REG_EXTENDED); + const int errcode = regcomp(®ex[index], regexin, REG_EXTENDED); if(errcode != 0) { log_regex_error("compiling", errcode, index); @@ -47,7 +47,7 @@ static bool init_regex(const char *regexin, int index) return true; } -bool __attribute__((pure)) in_whitelist(char *domain) +bool __attribute__((pure)) in_whitelist(const char *domain) { bool found = false; for(int i=0; i < whitelist.count; i++) @@ -77,14 +77,13 @@ static void free_whitelist_domains(void) } } -bool match_regex(char *input) +bool match_regex(const char *input) { - int index; bool matched = false; // Start matching timer timer_start(REGEX_TIMER); - for(index = 0; index < num_regex; index++) + for(int index = 0; index < num_regex; index++) { // Only check regex which have been successfully compiled if(!regexconfigured[index]) diff --git a/resolve.c b/resolve.c index 3bd7b0c3..93e27faf 100644 --- a/resolve.c +++ b/resolve.c @@ -72,7 +72,7 @@ static const char *resolveHostname(const char *addr) } // Resolve client host names -void resolveClients(bool onlynew) +void resolveClients(const bool onlynew) { int clientID; for(clientID = 0; clientID < counters->clients; clientID++) @@ -103,7 +103,7 @@ void resolveClients(bool onlynew) } // Resolve upstream destination host names -void resolveForwardDestinations(bool onlynew) +void resolveForwardDestinations(const bool onlynew) { int forwardID; for(forwardID = 0; forwardID < counters->forwarded; forwardID++) diff --git a/routines.h b/routines.h index fe990af7..4016637e 100644 --- a/routines.h +++ b/routines.h @@ -9,14 +9,14 @@ * Please see LICENSE file for your rights under this license. */ void go_daemon(void); -void timer_start(int i); -double timer_elapsed_msec(int i); -void sleepms(int milliseconds); +void timer_start(const int i); +double timer_elapsed_msec(const int i); +void sleepms(const int milliseconds); void savepid(void); char * getUserName(void); void removepid(void); -void open_FTL_log(bool test); +void open_FTL_log(const bool test); void logg(const char* format, ...) __attribute__ ((format (gnu_printf, 1, 2))); void logg_struct_resize(const char* str, int to, int step); void log_counter_info(void); @@ -25,20 +25,20 @@ void log_FTL_version(bool crashreport); // datastructure.c void strtolower(char *str); -int findForwardID(const char * forward, bool count); +int findForwardID(const char * forward, const bool count); int findDomainID(const char *domain); -int findClientID(const char *client, bool addNew); +int findClientID(const char *client, const bool count); bool isValidIPv4(const char *addr); bool isValidIPv6(const char *addr); -const char *getDomainString(int queryID); -const char *getClientIPString(int queryID); -const char *getClientNameString(int queryID); +const char *getDomainString(const int queryID); +const char *getClientIPString(const int queryID); +const char *getClientNameString(const int queryID); void close_telnet_socket(void); void close_unix_socket(void); -void seom(int sock); -void ssend(int sock, const char *format, ...) __attribute__ ((format (gnu_printf, 2, 3))); -void swrite(int sock, const void* value, size_t size); +void seom(const int sock); +void ssend(const int sock, const char *format, ...) __attribute__ ((format (gnu_printf, 2, 3))); +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); @@ -48,7 +48,6 @@ void bind_sockets(void); void process_request(const char *client_message, int *sock); bool command(const char *client_message, const char* cmd) __attribute__((pure)); -bool matchesEndpoint(char *client_message, const char *cmd); // grep.c int countlines(const char* fname); @@ -84,7 +83,7 @@ void *DB_thread(void *val); int get_number_of_queries_in_DB(void); void save_to_DB(void); void read_data_from_DB(void); -bool db_set_FTL_property(unsigned int ID, int value); +bool db_set_FTL_property(const unsigned int ID, const int value); bool dbquery(const char *format, ...); bool dbopen(void); void dbclose(void); @@ -92,12 +91,12 @@ int db_query_int(const char*); void SQLite3LogCallback(void *pArg, int iErrCode, const char *zMsg); // memory.c -void memory_check(int which); -char *FTLstrdup(const char *src, const char *file, const char *function, int line) __attribute__((malloc)); -void *FTLcalloc(size_t nmemb, size_t size, const char *file, const char *function, int line) __attribute__((malloc)) __attribute__((alloc_size(1,2))); -void *FTLrealloc(void *ptr_in, size_t size, const char *file, const char *function, int line) __attribute__((alloc_size(2))); -void FTLfree(void *ptr, const char* file, const char *function, int line); -void validate_access(const char * name, int pos, bool testmagic, int line, const char * function, const char * file); +void memory_check(const int which); +char *FTLstrdup(const char *src, const char *file, const char *function, const int line) __attribute__((malloc)); +void *FTLcalloc(size_t nmemb, size_t size, const char *file, const char *function, const int line) __attribute__((malloc)) __attribute__((alloc_size(1,2))); +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); +void validate_access(const char * name, int pos, bool testmagic, const int line, const char * function, const char * file); int main_dnsmasq(int argc, const char ** argv); @@ -106,27 +105,27 @@ void handle_signals(void); // resolve.c void *DNSclient_thread(void *val); -void resolveClients(bool onlynew); -void resolveForwardDestinations(bool onlynew); +void resolveClients(const bool onlynew); +void resolveForwardDestinations(const bool onlynew); // regex.c -bool match_regex(char *input); +bool match_regex(const char *input); void free_regex(void); void read_regex_from_file(void); -bool in_whitelist(char *domain) __attribute__((pure)); +bool in_whitelist(const char *domain) __attribute__((pure)); // shmem.c bool init_shmem(void); void destroy_shmem(void); size_t addstr(const char *str); -const char *getstr(size_t pos); -void *enlarge_shmem_struct(char type); +const char *getstr(const size_t pos); +void *enlarge_shmem_struct(const char type); /** * Create a new overTime client shared memory block. * This also updates `overTimeClientData`. */ -void newOverTimeClient(int clientID); +void newOverTimeClient(const int clientID); /** * Add a new overTime slot to each overTime client shared memory block. @@ -136,7 +135,7 @@ void addOverTimeClientSlot(void); // overTime.c void initOverTime(void); -unsigned int getOverTimeID(time_t timestamp); +unsigned int getOverTimeID(const time_t timestamp); /** * Move the overTime slots so the oldest interval starts with mintime. The time @@ -144,7 +143,7 @@ unsigned int getOverTimeID(time_t timestamp); * * @param mintime The start of the oldest interval */ -void moveOverTimeMemory(time_t mintime); +void moveOverTimeMemory(const time_t mintime); // capabilities.c bool check_capabilities(void); diff --git a/shmem.c b/shmem.c index dd868bad..8e40d1cc 100644 --- a/shmem.c +++ b/shmem.c @@ -46,7 +46,7 @@ static ShmSettings *shmSettings = NULL; static int pagesize; static unsigned int local_shm_counter = 0; -static size_t get_optimal_object_size(size_t objsize, size_t minsize); +static size_t get_optimal_object_size(const size_t objsize, const size_t minsize); size_t addstr(const char *str) { @@ -90,7 +90,7 @@ size_t addstr(const char *str) return (shmSettings->next_str_pos - (len + 1)); } -const char *getstr(size_t pos) +const char *getstr(const size_t pos) { // Only access the string memory if this memory region has already been set if(pos < shmSettings->next_str_pos) @@ -143,17 +143,17 @@ static void remap_shm(void) local_shm_counter = shmSettings->global_shm_counter; } -void _lock_shm(const char* function, const int line, const char * file) { +void _lock_shm(const char* func, const int line, const char * file) { // Signal that FTL is waiting for a lock shmLock->waitingForLock = true; if(config.debug & DEBUG_LOCKS) - logg("Waiting for lock in %s() (%s:%i)", function, file, line); + logg("Waiting for lock in %s() (%s:%i)", func, file, line); int result = pthread_mutex_lock(&shmLock->lock); if(config.debug & DEBUG_LOCKS) - logg("Obtained lock for %s() (%s:%i)", function, file, line); + logg("Obtained lock for %s() (%s:%i)", func, file, line); // Check if this process needs to remap the shared memory objects if(shmSettings != NULL && @@ -179,11 +179,11 @@ void _lock_shm(const char* function, const int line, const char * file) { logg("Failed to obtain SHM lock: %s", strerror(result)); } -void _unlock_shm(const char* function, const int line, const char * file) { +void _unlock_shm(const char* func, const int line, const char * file) { int result = pthread_mutex_unlock(&shmLock->lock); if(config.debug & DEBUG_LOCKS) - logg("Removed lock in %s() (%s:%i)", function, file, line); + logg("Removed lock in %s() (%s:%i)", func, file, line); if(result != 0) logg("Failed to unlock SHM lock: %s", strerror(result)); @@ -274,7 +274,7 @@ void destroy_shmem(void) delete_shm(&shm_settings); } -SharedMemory create_shm(const char *name, size_t size) +SharedMemory create_shm(const char *name, const size_t size) { if(config.debug & DEBUG_SHMEM) logg("Creating shared memory with name \"%s\" and size %zu", name, size); @@ -307,10 +307,10 @@ SharedMemory create_shm(const char *name, size_t size) } // Resize shared memory file - int result = ftruncate(fd, size); + ret = ftruncate(fd, size); // Check for `ftruncate` error - if(result == -1) + if(ret == -1) { logg("FATAL: create_shm(): ftruncate(%i, %zu): Failed to resize shared memory object \"%s\": %s", fd, size, sharedMemory.name, strerror(errno)); @@ -336,7 +336,7 @@ SharedMemory create_shm(const char *name, size_t size) return sharedMemory; } -void *enlarge_shmem_struct(char type) +void *enlarge_shmem_struct(const char type) { SharedMemory *sharedMemory = NULL; size_t sizeofobj, allocation_step; @@ -383,7 +383,7 @@ void *enlarge_shmem_struct(char type) return sharedMemory->ptr; } -bool realloc_shm(SharedMemory *sharedMemory, size_t size, bool resize) +bool realloc_shm(SharedMemory *sharedMemory, const size_t size, const bool resize) { // Check if we can skip this routine as nothing is to be done // when an object is not to be resized and its size didn't @@ -401,7 +401,7 @@ bool realloc_shm(SharedMemory *sharedMemory, size_t size, bool resize) if(resize) { // Open shared memory object - int fd = shm_open(sharedMemory->name, O_RDWR, S_IRUSR | S_IWUSR); + const int fd = shm_open(sharedMemory->name, O_RDWR, S_IRUSR | S_IWUSR); if(fd == -1) { logg("FATAL: realloc_shm(): Failed to open shared memory object \"%s\": %s", @@ -410,7 +410,7 @@ bool realloc_shm(SharedMemory *sharedMemory, size_t size, bool resize) } // Truncate shared memory object to specified size - int result = ftruncate(fd, size); + const int result = ftruncate(fd, size); if(result == -1) { logg("FATAL: realloc_shm(): ftruncate(%i, %zu): Failed to resize \"%s\": %s", fd, size, sharedMemory->name, strerror(errno)); @@ -444,8 +444,7 @@ bool realloc_shm(SharedMemory *sharedMemory, size_t size, bool resize) void delete_shm(SharedMemory *sharedMemory) { // Unmap shared memory - int ret; - ret = munmap(sharedMemory->ptr, sharedMemory->size); + int ret = munmap(sharedMemory->ptr, sharedMemory->size); if(ret != 0) logg("delete_shm(): munmap(%p, %zu) failed: %s", sharedMemory->ptr, sharedMemory->size, strerror(errno)); @@ -472,9 +471,9 @@ static size_t __attribute__((const)) gcd(size_t a, size_t b) // shared memory objects. This routine works by computing the LCM // of two numbers, the pagesize and the size of a single element // in the shared memory object -static size_t get_optimal_object_size(size_t objsize, size_t minsize) +static size_t get_optimal_object_size(const size_t objsize, const size_t minsize) { - size_t optsize = pagesize / gcd(pagesize, objsize); + const size_t optsize = pagesize / gcd(pagesize, objsize); if(optsize < minsize) { if(config.debug & DEBUG_SHMEM) @@ -490,7 +489,7 @@ static size_t get_optimal_object_size(size_t objsize, size_t minsize) // First part: Integer division, may cause clipping, e.g., 5/3 = 1 // Second part: Catch a possibly happened clipping event by adding // one to the number: (5 % 3 != 0) is 1 - size_t multiplier = (minsize/optsize) + ((minsize % optsize != 0) ? 1u : 0u); + const size_t multiplier = (minsize/optsize) + ((minsize % optsize != 0) ? 1u : 0u); if(config.debug & DEBUG_SHMEM) { logg("DEBUG: Using %zu*%zu == %zu >= %zu", diff --git a/shmem.h b/shmem.h index 83290262..15646328 100644 --- a/shmem.h +++ b/shmem.h @@ -27,7 +27,7 @@ typedef struct { /// \param size the size to allocate /// \return a structure with a pointer to the mounted shared memory. The pointer /// will always be valid, because if it failed FTL will have exited. -SharedMemory create_shm(const char *name, size_t size); +SharedMemory create_shm(const char *name, const size_t size); /// Reallocate shared memory /// @@ -35,7 +35,7 @@ SharedMemory create_shm(const char *name, size_t size); /// \param size the new size /// \param resize whether the object should be resized or only remapped /// \return if reallocation was successful -bool realloc_shm(SharedMemory *sharedMemory, size_t size, bool resize); +bool realloc_shm(SharedMemory *sharedMemory, const size_t size, const bool resize); /// Disconnect from shared memory. If there are no other connections to shared memory, it will be deleted. /// diff --git a/socket.c b/socket.c index 10adc411..8e779a74 100644 --- a/socket.c +++ b/socket.c @@ -201,7 +201,7 @@ static void removeport(void) fclose(f); } -void seom(int sock) +void seom(const int sock) { if(istelnet[sock]) ssend(sock, "---EOM---\n\n"); @@ -209,7 +209,7 @@ void seom(int sock) pack_eom(sock); } -void __attribute__ ((format (gnu_printf, 2, 3))) ssend(int sock, const char *format, ...) +void __attribute__ ((format (gnu_printf, 2, 3))) ssend(const int sock, const char *format, ...) { char *buffer; va_list args; @@ -224,12 +224,12 @@ void __attribute__ ((format (gnu_printf, 2, 3))) ssend(int sock, const char *for } } -void swrite(int sock, const void *value, size_t size) { +void swrite(const int sock, const void *value, size_t size) { if(write(sock, value, size) == -1) logg("WARNING: Socket write returned error code %i", errno); } -static inline int checkClientLimit(int socket) { +static inline int checkClientLimit(const int socket) { if(socket < MAXCONNS) { return socket; @@ -243,7 +243,7 @@ static inline int checkClientLimit(int socket) { } } -static int listener(int sockfd, char type) +static int listener(const int sockfd, const char type) { struct sockaddr_un un_addr; struct sockaddr_in in4_addr; @@ -430,7 +430,7 @@ void *telnet_listening_thread_IPv4(void *args) while(!killed) { // Look for new clients that want to connect - int csck = listener(telnetfd4, 4); + const int csck = listener(telnetfd4, 4); if(csck == -1) { logg("IPv4 telnet error: %s (%i)", strerror(errno), errno); @@ -471,7 +471,7 @@ void *telnet_listening_thread_IPv6(void *args) while(!killed) { // Look for new clients that want to connect - int csck = listener(telnetfd6, 6); + const int csck = listener(telnetfd6, 6); if(csck == -1) { logg("IPv6 telnet error: %s (%i)", strerror(errno), errno); @@ -512,7 +512,7 @@ void *socket_listening_thread(void *args) while(!killed) { // Look for new clients that want to connect - int csck = listener(socketfd, 0); + const int csck = listener(socketfd, 0); if(csck < 0) continue; // Allocate memory used to transport client socket ID to client listening thread @@ -544,8 +544,8 @@ bool ipv6_available(void) // Loop over interfaces for (interface = allInterfaces; interface != NULL; interface = interface->ifa_next) { - unsigned int flags = interface->ifa_flags; - struct sockaddr *addr = interface->ifa_addr; + const unsigned int flags = interface->ifa_flags; + const struct sockaddr *addr = interface->ifa_addr; // Check only for up and running IPv4, IPv6 interfaces if ((flags & (IFF_UP|IFF_RUNNING)) && addr != NULL) From 3a4cf9c6a47566244337e58a3a80a7d40654cf2d Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 15 Apr 2019 22:44:29 +0200 Subject: [PATCH 30/30] Remove obsolete logg_struct_resize() function Signed-off-by: DL6ER --- dnsmasq_interface.c | 2 +- log.c | 5 ----- routines.h | 1 - 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index e3e381cf..bf89d2f9 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -22,7 +22,7 @@ static void save_reply_type(const unsigned int flags, const int queryID, const s static unsigned long converttimeval(const struct timeval time) __attribute__((const)); static void block_single_domain_regex(const char *domain); static void detect_blocked_IP(const unsigned short flags, const char* answer, const int queryID); -static void query_externally_blocked(const int i, const unsigned char status); +static void query_externally_blocked(const int queryID, const unsigned char status); static int findQueryID(const int id); unsigned char* pihole_privacylevel = &config.privacylevel; diff --git a/log.c b/log.c index 01a376b2..71170537 100644 --- a/log.c +++ b/log.c @@ -124,11 +124,6 @@ void format_memory_size(char *prefix, const unsigned long int bytes, double *for strcpy(prefix, prefixes[i]); } -void logg_struct_resize(const char* str, const int to, const int step) -{ - logg("Notice: Increasing %s struct size from %i to %i", str, (to-step), to); -} - void log_counter_info(void) { logg(" -> Total DNS queries: %i", counters->queries); diff --git a/routines.h b/routines.h index 4016637e..4ecbe378 100644 --- a/routines.h +++ b/routines.h @@ -18,7 +18,6 @@ void removepid(void); void open_FTL_log(const bool test); void logg(const char* format, ...) __attribute__ ((format (gnu_printf, 1, 2))); -void logg_struct_resize(const char* str, int to, int step); void log_counter_info(void); void format_memory_size(char *prefix, unsigned long int bytes, double *formated); void log_FTL_version(bool crashreport);