From fcb3ed9511648a53a43da0de0ab054416dc70770 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 30 Jan 2019 22:12:53 +0100 Subject: [PATCH 1/4] Handle negative replies: SERVFAIL, REFUSED, and NOTIMPLEMENTED Signed-off-by: DL6ER --- FTL.h | 2 +- dnsmasq/forward.c | 1 + dnsmasq_interface.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ dnsmasq_interface.h | 3 +++ 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/FTL.h b/FTL.h index 9bcf7cba..5df5a07a 100644 --- a/FTL.h +++ b/FTL.h @@ -72,7 +72,7 @@ enum { QUERIES, FORWARDED, CLIENTS, DOMAINS, OVERTIME, WILDCARD }; enum { DNSSEC_UNSPECIFIED, DNSSEC_SECURE, DNSSEC_INSECURE, DNSSEC_BOGUS, DNSSEC_ABANDONED, DNSSEC_UNKNOWN }; enum { QUERY_UNKNOWN, QUERY_GRAVITY, QUERY_FORWARDED, QUERY_CACHE, QUERY_WILDCARD, QUERY_BLACKLIST, QUERY_EXTERNAL_BLOCKED }; enum { TYPE_A = 1, TYPE_AAAA, TYPE_ANY, TYPE_SRV, TYPE_SOA, TYPE_PTR, TYPE_TXT, TYPE_MAX }; -enum { REPLY_UNKNOWN, REPLY_NODATA, REPLY_NXDOMAIN, REPLY_CNAME, REPLY_IP, REPLY_DOMAIN, REPLY_RRNAME }; +enum { REPLY_UNKNOWN, REPLY_NODATA, REPLY_NXDOMAIN, REPLY_CNAME, REPLY_IP, REPLY_DOMAIN, REPLY_RRNAME, REPLY_SERVFAIL, REPLY_REFUSED, REPLY_NOTIMP, REPLY_OTHER }; enum { PRIVACY_SHOW_ALL = 0, PRIVACY_HIDE_DOMAINS, PRIVACY_HIDE_DOMAINS_CLIENTS, PRIVACY_MAXIMUM, PRIVACY_NOSTATS }; enum { MODE_IP, MODE_NX, MODE_NULL, MODE_IP_NODATA_AAAA, MODE_NODATA }; enum { REGEX_UNKNOWN, REGEX_BLOCKED, REGEX_NOTBLOCKED }; diff --git a/dnsmasq/forward.c b/dnsmasq/forward.c index 8d120786..9d998d29 100644 --- a/dnsmasq/forward.c +++ b/dnsmasq/forward.c @@ -700,6 +700,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server { struct all_addr a; a.addr.rcode.rcode = rcode; + FTL_query_error(rcode, daemon->log_display_id); log_query(F_UPSTREAM | F_RCODE, "error", &a, NULL); return resize_packet(header, n, pheader, plen); diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index d542b5c1..8478787f 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -775,6 +775,65 @@ void _FTL_dnssec(int status, int id, const char* file, const int line) unlock_shm(); } +void _FTL_query_error(unsigned int rcode, int id, const char* file, const int line) +{ + // Process upstream error messages + // Don't analyze anything if in PRIVACY_NOSTATS mode + if(config.privacylevel >= PRIVACY_NOSTATS) + return; + + // Process DNSSEC result for a domain + lock_shm(); + // Search for corresponding query identified by ID + int i = findQueryID(id); + if(i < 0) + { + // This may happen e.g. if the original query was an unhandled query type + unlock_shm(); + return; + } + // Translate dnsmasq's rcode into something we can use + char *rcodestr = NULL; + bool alloc = false; + unsigned char reply; + switch(rcode) + { + case SERVFAIL: + rcodestr = "SERVFAIL"; + reply = REPLY_SERVFAIL; + break; + case REFUSED: + rcodestr = "REFUSED"; + reply = REPLY_REFUSED; + break; + case NOTIMP: + rcodestr = "NOT IMPLEMENTED"; + reply = REPLY_NOTIMP; + break; + default: + if(asprintf(&rcodestr, "Unknown error type (%u)", rcode) > -1) + alloc = true; + reply = REPLY_OTHER; + break; + } + + // Set reply status + queries[i].reply = reply; + + // Debug logging + if(config.debug & DEBUG_QUERIES) + { + int domainID = queries[i].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 we allocated memory (due to an unknown error type), we need to free it here + if(alloc) + free(rcodestr); + + unlock_shm(); +} void _FTL_header_ADbit(unsigned char header4, unsigned int rcode, int id, const char* file, const int line) { diff --git a/dnsmasq_interface.h b/dnsmasq_interface.h index 0a5000ba..d90a5a54 100644 --- a/dnsmasq_interface.h +++ b/dnsmasq_interface.h @@ -32,6 +32,9 @@ void _FTL_header_ADbit(unsigned char header4, unsigned int rcode, int id, const #define FTL_forwarding_failed(server) _FTL_forwarding_failed(server, __FILE__, __LINE__) void _FTL_forwarding_failed(struct server *server, const char* file, const int line); +#define FTL_query_error(rcode, id) _FTL_query_error(rcode, id, __FILE__, __LINE__) +void _FTL_query_error(unsigned int rcode, 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); From 640a41be38f9c45f399752b1fdf3bd7f972ae5a9 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 30 Jan 2019 22:46:53 +0100 Subject: [PATCH 2/4] Remove unnecessary variable and clarify what an "error" is (added more comments) Signed-off-by: DL6ER --- dnsmasq_interface.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index 8478787f..dace5d2f 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -775,9 +775,13 @@ void _FTL_dnssec(int status, int id, const char* file, const int line) unlock_shm(); } + void _FTL_query_error(unsigned int rcode, int id, const char* file, const int line) { - // Process upstream error messages + // Process upstream errors + // Queries with error are those where the RCODE + // in the DNS header is neither NOERROR nor NXDOMAIN. + // Don't analyze anything if in PRIVACY_NOSTATS mode if(config.privacylevel >= PRIVACY_NOSTATS) return; @@ -795,31 +799,27 @@ void _FTL_query_error(unsigned int rcode, int id, const char* file, const int li // Translate dnsmasq's rcode into something we can use char *rcodestr = NULL; bool alloc = false; - unsigned char reply; switch(rcode) { case SERVFAIL: rcodestr = "SERVFAIL"; - reply = REPLY_SERVFAIL; + queries[i].reply = REPLY_SERVFAIL; break; case REFUSED: rcodestr = "REFUSED"; - reply = REPLY_REFUSED; + queries[i].reply = REPLY_REFUSED; break; case NOTIMP: rcodestr = "NOT IMPLEMENTED"; - reply = REPLY_NOTIMP; + queries[i].reply = REPLY_NOTIMP; break; default: if(asprintf(&rcodestr, "Unknown error type (%u)", rcode) > -1) alloc = true; - reply = REPLY_OTHER; + queries[i].reply = REPLY_OTHER; break; } - // Set reply status - queries[i].reply = reply; - // Debug logging if(config.debug & DEBUG_QUERIES) { From d20b0d6cdafba31092952bfe2138017eeb955c0d Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 31 Jan 2019 17:46:54 +0100 Subject: [PATCH 3/4] Bump shm version to version 2 Signed-off-by: DL6ER --- dnsmasq/forward.c | 2 +- dnsmasq_interface.c | 2 +- dnsmasq_interface.h | 4 ++-- shmem.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dnsmasq/forward.c b/dnsmasq/forward.c index 9d998d29..eb6bd989 100644 --- a/dnsmasq/forward.c +++ b/dnsmasq/forward.c @@ -700,7 +700,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server { struct all_addr a; a.addr.rcode.rcode = rcode; - FTL_query_error(rcode, daemon->log_display_id); + FTL_upstream_error(rcode, daemon->log_display_id); log_query(F_UPSTREAM | F_RCODE, "error", &a, NULL); return resize_packet(header, n, pheader, plen); diff --git a/dnsmasq_interface.c b/dnsmasq_interface.c index dace5d2f..f6dba204 100644 --- a/dnsmasq_interface.c +++ b/dnsmasq_interface.c @@ -776,7 +776,7 @@ void _FTL_dnssec(int status, int id, const char* file, const int line) unlock_shm(); } -void _FTL_query_error(unsigned int rcode, int id, const char* file, const int line) +void _FTL_upstream_error(unsigned int rcode, int id, const char* file, const int line) { // Process upstream errors // Queries with error are those where the RCODE diff --git a/dnsmasq_interface.h b/dnsmasq_interface.h index d90a5a54..30c9d414 100644 --- a/dnsmasq_interface.h +++ b/dnsmasq_interface.h @@ -32,8 +32,8 @@ void _FTL_header_ADbit(unsigned char header4, unsigned int rcode, int id, const #define FTL_forwarding_failed(server) _FTL_forwarding_failed(server, __FILE__, __LINE__) void _FTL_forwarding_failed(struct server *server, const char* file, const int line); -#define FTL_query_error(rcode, id) _FTL_query_error(rcode, id, __FILE__, __LINE__) -void _FTL_query_error(unsigned int rcode, int id, 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_dnsmasq_reload(void); void FTL_fork_and_bind_sockets(struct passwd *ent_pw); diff --git a/shmem.c b/shmem.c index 70d2d18b..18898385 100644 --- a/shmem.c +++ b/shmem.c @@ -12,7 +12,7 @@ #include "shmem.h" /// The version of shared memory used -#define SHARED_MEMORY_VERSION 1 +#define SHARED_MEMORY_VERSION 2 /// The name of the shared memory. Use this when connecting to the shared memory. #define SHARED_LOCK_NAME "/FTL-lock" From 52c6bdbf7c5dcef9e8f51518a5bf68a4064d0b0f Mon Sep 17 00:00:00 2001 From: Mcat12 Date: Sun, 3 Feb 2019 12:32:42 -0800 Subject: [PATCH 4/4] Panic (crash) if we are unable to create or resize shared memory If we are unable to create or resize shared memory then there is no point in continuing execution. Further interactions with shared memory may result in crashes which are hard to debug. As a result of this change, there is no need to check if the returned shared memory object pointer is null, because if the function succeeded, the operation was successful. Signed-off-by: Mcat12 --- shmem.c | 47 ++++++++++++----------------------------------- shmem.h | 3 ++- 2 files changed, 14 insertions(+), 36 deletions(-) diff --git a/shmem.c b/shmem.c index 18898385..9fb19b03 100644 --- a/shmem.c +++ b/shmem.c @@ -105,11 +105,6 @@ void newOverTimeClient(int clientID) { // Create the shared memory with enough space for the current overTime slots shm_unlink(name); SharedMemory shm = create_shm(name, (counters->overTime/pagesize + 1)*pagesize*sizeof(int)); - if(shm.ptr == NULL) { - free(shm.name); - logg("Failed to initialize new overTime client %d", clientID); - return; - } // Make space for the new shared memory shm_overTimeClients = realloc(shm_overTimeClients, sizeof(SharedMemory) * (clientID + 1)); @@ -207,8 +202,6 @@ bool init_shmem(void) /****************************** shared memory lock ******************************/ // Try to create shared memory object shm_lock = create_shm(SHARED_LOCK_NAME, sizeof(ShmLock)); - if(shm_lock.ptr == NULL) - return false; shmLock = (ShmLock*) shm_lock.ptr; shmLock->lock = create_mutex(); shmLock->waitingForLock = false; @@ -216,8 +209,6 @@ bool init_shmem(void) /****************************** shared strings buffer ******************************/ // Try to create shared memory object shm_strings = create_shm(SHARED_STRINGS_NAME, pagesize); - if(shm_strings.ptr == NULL) - return false; // Initialize shared string object with an empty string at position zero ((char*)shm_strings.ptr)[0] = '\0'; @@ -226,55 +217,41 @@ bool init_shmem(void) /****************************** shared counters struct ******************************/ // Try to create shared memory object shm_counters = create_shm(SHARED_COUNTERS_NAME, sizeof(countersStruct)); - if(shm_counters.ptr == NULL) - return false; counters = (countersStruct*)shm_counters.ptr; /****************************** shared domains struct ******************************/ // Try to create shared memory object shm_domains = create_shm(SHARED_DOMAINS_NAME, pagesize*sizeof(domainsDataStruct)); - if(shm_domains.ptr == NULL) - return false; domains = (domainsDataStruct*)shm_domains.ptr; counters->domains_MAX = pagesize; /****************************** shared clients struct ******************************/ // Try to create shared memory object shm_clients = create_shm(SHARED_CLIENTS_NAME, pagesize*sizeof(clientsDataStruct)); - if(shm_clients.ptr == NULL) - return false; clients = (clientsDataStruct*)shm_clients.ptr; counters->clients_MAX = pagesize; /****************************** shared forwarded struct ******************************/ // Try to create shared memory object shm_forwarded = create_shm(SHARED_FORWARDED_NAME, pagesize*sizeof(forwardedDataStruct)); - if(shm_forwarded.ptr == NULL) - return false; forwarded = (forwardedDataStruct*)shm_forwarded.ptr; counters->forwarded_MAX = pagesize; /****************************** shared queries struct ******************************/ // Try to create shared memory object shm_queries = create_shm(SHARED_QUERIES_NAME, pagesize*sizeof(queriesDataStruct)); - if(shm_queries.ptr == NULL) - return false; queries = (queriesDataStruct*)shm_queries.ptr; counters->queries_MAX = pagesize; /****************************** shared overTime struct ******************************/ // Try to create shared memory object shm_overTime = create_shm(SHARED_OVERTIME_NAME, pagesize*sizeof(overTimeDataStruct)); - if(shm_overTime.ptr == NULL) - return false; overTime = (overTimeDataStruct*)shm_overTime.ptr; counters->overTime_MAX = pagesize; /****************************** shared settings struct ******************************/ // Try to create shared memory object shm_settings = create_shm(SHARED_SETTINGS_NAME, sizeof(ShmSettings)); - if(shm_settings.ptr == NULL) - return false; ShmSettings *settings = (ShmSettings*)shm_settings.ptr; settings->version = SHARED_MEMORY_VERSION; @@ -333,9 +310,9 @@ SharedMemory create_shm(char *name, size_t size) // Check for `shm_open` error if(fd == -1) { - logg("create_shm(): Failed to create_shm shared memory object \"%s\": %s", + logg("FATAL: create_shm(): Failed to create_shm shared memory object \"%s\": %s", name, strerror(errno)); - return sharedMemory; + exit(EXIT_FAILURE); } // Resize shared memory file @@ -344,9 +321,9 @@ SharedMemory create_shm(char *name, size_t size) // Check for `ftruncate` error if(result == -1) { - logg("create_shm(): ftruncate(%i, %zu): Failed to resize shared memory object \"%s\": %s", + logg("FATAL: create_shm(): ftruncate(%i, %zu): Failed to resize shared memory object \"%s\": %s", fd, size, sharedMemory.name, strerror(errno)); - return sharedMemory; + exit(EXIT_FAILURE); } // Create shared memory mapping @@ -355,9 +332,9 @@ SharedMemory create_shm(char *name, size_t size) // Check for `mmap` error if(shm == MAP_FAILED) { - logg("create_shm(): Failed to map shared memory object \"%s\" (%i): %s", + logg("FATAL: create_shm(): Failed to map shared memory object \"%s\" (%i): %s", sharedMemory.name, fd, strerror(errno)); - return sharedMemory; + exit(EXIT_FAILURE); } // Close shared memory object file descriptor as it is no longer @@ -428,27 +405,27 @@ bool realloc_shm(SharedMemory *sharedMemory, size_t size) { int fd = shm_open(sharedMemory->name, O_RDWR, S_IRUSR | S_IWUSR); if(fd == -1) { - logg("realloc_shm(): Failed to open shared memory object \"%s\": %s", + logg("FATAL: realloc_shm(): Failed to open shared memory object \"%s\": %s", sharedMemory->name, strerror(errno)); - return false; + exit(EXIT_FAILURE); } // Resize shard memory object to requested size result = ftruncate(fd, size); if(result == -1) { - logg("realloc_shm(): ftruncate(%i, %zu): Failed to resize \"%s\": %s", + logg("FATAL: realloc_shm(): ftruncate(%i, %zu): Failed to resize \"%s\": %s", fd, size, sharedMemory->name, strerror(errno)); - return false; + exit(EXIT_FAILURE); } // void *new_ptr = mremap(sharedMemory->ptr, sharedMemory->size, size, MREMAP_MAYMOVE); void *new_ptr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if(new_ptr == MAP_FAILED) { - logg("realloc_shm(): mremap(%p, %zu, %zu, MREMAP_MAYMOVE): Failed to reallocate \"%s\" (%i): %s", + logg("FATAL: realloc_shm(): mremap(%p, %zu, %zu, MREMAP_MAYMOVE): Failed to reallocate \"%s\" (%i): %s", sharedMemory->ptr, sharedMemory->size, size, sharedMemory->name, fd, strerror(errno)); - return false; + exit(EXIT_FAILURE); } // Close shared memory object file descriptor as it is no longer diff --git a/shmem.h b/shmem.h index 377dab67..9f9b1f90 100644 --- a/shmem.h +++ b/shmem.h @@ -25,7 +25,8 @@ typedef struct { /// /// \param name the name of the shared memory /// \param size the size to allocate -/// \return a structure with a pointer to the mounted shared memory. The pointer will be NULL if it failed +/// \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(char *name, size_t size); /// Reallocate shared memory