From faac7005d173dd95bf654addfedbcfd39ee0a421 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 22 Nov 2019 09:42:06 +0100 Subject: [PATCH] Improve performance significantly. We store if (and if: why) a certain domain was blocked for any requesting client and can immediately reply similarly if the same client requests the same domain again. This reduces the O(N^3) problem (number of queries * number of domains * number of clients) to a O(N^2) problem (domains * clients). Note that this state of the code still lacks a possibility to reset when entries in the database have changed. For this, we still have to send either SIGHUP (drawback: clears the cache) or define a new signal for it. Signed-off-by: DL6ER --- src/api/api.c | 93 +++++++++++--------- src/datastructure.c | 11 ++- src/datastructure.h | 4 +- src/dnsmasq_interface.c | 185 +++++++++++++++++++++++++--------------- src/dnsmasq_interface.h | 2 + src/regex.c | 12 ++- src/regex_r.h | 5 +- src/vector.c | 132 +++++++++++++++++++++++++++- src/vector.h | 22 ++++- 9 files changed, 347 insertions(+), 119 deletions(-) diff --git a/src/api/api.c b/src/api/api.c index 3b3dc957..00e9c4bb 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -313,32 +313,13 @@ void getTopDomains(const char *client_message, const int *sock) if(blocked && showblocked && domain->blockedcount > 0) { - if(audit && domain->regexmatch == REGEX_BLOCKED) - { - if(istelnet[*sock]) - ssend(*sock, "%i %i %s wildcard\n", n, domain->blockedcount, getstr(domain->domainpos)); - else { - char *fancyWildcard = calloc(3 + strlen(getstr(domain->domainpos)), sizeof(char)); - if(fancyWildcard == NULL) return; - sprintf(fancyWildcard, "*.%s", getstr(domain->domainpos)); + if(istelnet[*sock]) + ssend(*sock, "%i %i %s\n", n, domain->blockedcount, getstr(domain->domainpos)); + else { + if(!pack_str32(*sock, getstr(domain->domainpos))) + return; - if(!pack_str32(*sock, fancyWildcard)) - return; - - pack_int32(*sock, domain->blockedcount); - free(fancyWildcard); - } - } - else - { - if(istelnet[*sock]) - ssend(*sock, "%i %i %s\n", n, domain->blockedcount, getstr(domain->domainpos)); - else { - if(!pack_str32(*sock, getstr(domain->domainpos))) - return; - - pack_int32(*sock, domain->blockedcount); - } + pack_int32(*sock, domain->blockedcount); } n++; } @@ -1296,11 +1277,12 @@ void getUnknownQueries(const int *sock) void getDomainDetails(const char *client_message, const int *sock) { // Get domain name + bool show_all = false; char domainString[128]; if(sscanf(client_message, "%*[^ ] %127s", domainString) < 1) { - ssend(*sock, "Need domain for this request\n"); - return; + ssend(*sock, "No domain specified, listing all known (%d)\n", counters->domains); + show_all = true; } for(int domainID = 0; domainID < counters->domains; domainID++) @@ -1310,23 +1292,56 @@ void getDomainDetails(const char *client_message, const int *sock) if(domain == NULL) continue; - if(strcmp(getstr(domain->domainpos), domainString) == 0) + if(show_all || strcmp(getstr(domain->domainpos), domainString) == 0) { - ssend(*sock,"Domain \"%s\", ID: %i\n", domainString, domainID); + ssend(*sock,"Domain \"%s\", ID: %i\n", getstr(domain->domainpos), domainID); ssend(*sock,"Total: %i\n", domain->count); ssend(*sock,"Blocked: %i\n", domain->blockedcount); - const char *regexstatus; - if(domain->regexmatch == REGEX_BLOCKED) - regexstatus = "blocked"; - else if(domain->regexmatch == REGEX_NOTBLOCKED) - regexstatus = "not blocked"; - else - regexstatus = "unknown"; - ssend(*sock,"Regex status: %s\n", regexstatus); - return; + ssend(*sock,"Client status:\n"); + for(int clientID = 0; clientID < counters->clients; clientID++) + { + clientsData *client = getClient(clientID, true); + if(client == NULL) + { + continue; + } + const char *str = "N/A"; + switch(domain->clientstatus->get(domain->clientstatus, clientID)) + { + case UNKNOWN_BLOCKED: + str = "unknown"; + break; + case BLACKLIST_BLOCKED: + str = "blacklisted"; + break; + case GRAVITY_BLOCKED: + str = "gravity"; + break; + case REGEX_BLOCKED: + str = "regex"; + break; + case NOT_BLOCKED: + str = "not blocked"; + break; + default: + str = "this cannot happen"; + break; + } + ssend(*sock, " %s (ID %d): %s\n", getstr(client->ippos), clientID, str); + } + ssend(*sock,"\n"); + + // Return early + if(!show_all) + { + return; + } } } // for loop finished without an exact match - ssend(*sock,"Domain \"%s\" is unknown\n", domainString); + if(!show_all) + { + ssend(*sock,"Domain \"%s\" is unknown\n", domainString); + } } diff --git a/src/datastructure.c b/src/datastructure.c index b72d7725..b1b2214f 100644 --- a/src/datastructure.c +++ b/src/datastructure.c @@ -126,8 +126,8 @@ int findDomainID(const char *domainString) domain->blockedcount = 0; // Store domain name - no need to check for NULL here as it doesn't harm domain->domainpos = addstr(domainString); - // RegEx needs to be evaluated for this new domain - domain->regexmatch = REGEX_UNKNOWN; + // Storage for individual client blocking status + domain->clientstatus = new_ucharvec(counters->clients); // Increase counter by one counters->domains++; @@ -201,6 +201,13 @@ int findClientID(const char *clientIP, const bool count) for(int i = 0; i < OVERTIME_SLOTS; i++) client->overTime[i] = 0; + // Initialize client-specific domain data + for(int domainID = 0; domainID < counters->domains; domainID++) + { + domainsData *domain = getDomain(domainID, true); + domain->clientstatus->append(domain->clientstatus, UNKNOWN_BLOCKED); + } + // Allocate regex substructure allocate_regex_client_enabled(client); diff --git a/src/datastructure.h b/src/datastructure.h index eae52a78..4cbce071 100644 --- a/src/datastructure.h +++ b/src/datastructure.h @@ -12,6 +12,8 @@ // Definition of sqlite3_stmt #include "database/sqlite3.h" +// struct ucharvec +#include "vector.h" void strtolower(char *str); int findForwardID(const char * forward, const bool count); @@ -68,10 +70,10 @@ typedef struct { typedef struct { unsigned char magic; - unsigned char regexmatch; size_t domainpos; int count; int blockedcount; + ucharvec *clientstatus; } domainsData; // Pointer getter functions diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 8f52fdda..079324a6 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -53,6 +53,119 @@ static struct all_addr blocking_addrp_v6 = {{{ 0 }}}; unsigned char* pihole_privacylevel = &config.privacylevel; 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 "}; + +static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const char **blockingreason, + const char* file, const int line) +{ + // Only check domains for blocking conditions when global blocking is enabled + if(blockingstatus == BLOCKING_DISABLED) + { + return false; + } + + // Get query, domain and client pointers + queriesData* query = getQuery(queryID, true); + domainsData* domain = getDomain(domainID, true); + clientsData* client = getClient(clientID, true); + if(query == NULL || domain == NULL || client == NULL) + { + // Encountered memory error, skip query + return false; + } + + // Skip the entire chain of tests if we already know the answer for this + // particular client + unsigned char blockingStatus = domain->clientstatus->get(domain->clientstatus, clientID); + switch(blockingStatus) + { + case UNKNOWN_BLOCKED: + // New domain/client combination. + // We have to go through all the tests below + break; + case BLACKLIST_BLOCKED: + query->status = QUERY_BLACKLIST; + *blockingreason = "exactly blacklisted"; + query_blocked(queryID, query, domain, client); + return true; + break; + case GRAVITY_BLOCKED: + query->status = QUERY_GRAVITY; + *blockingreason = "gravity blocked"; + query_blocked(queryID, query, domain, client); + return true; + break; + case REGEX_BLOCKED: + query->status = QUERY_WILDCARD; + *blockingreason = "regex blacklisted"; + query_blocked(queryID, query, domain, client); + return true; + break; + case NOT_BLOCKED: + return false; + break; + } + + // We check the user blacklist first as it is typically smaller than gravity + // If a domain is on the exact blacklist or gravity but also on the whitelist, + // we do NOT block it. + // in_whitelist() checks both the exact and the regex whitelist + bool blockDomain = false, black = false, gravity = false; + const char *domainString = getstr(domain->domainpos); + if(((black = in_blacklist(domainString, client)) || + (gravity = in_gravity(domainString, client))) && + !in_whitelist(domainString, client)) + { + blockDomain = true; + if(black) + { + // Mark domain as regex matched for this one client + domain->clientstatus->set(domain->clientstatus, clientID, BLACKLIST_BLOCKED); + query->status = QUERY_BLACKLIST; + *blockingreason = "exactly blacklisted"; + } + else if(gravity) + { + domain->clientstatus->set(domain->clientstatus, clientID, GRAVITY_BLOCKED); + query->status = QUERY_GRAVITY; + *blockingreason = "gravity blocked"; + } + + // Adjust counters + query_blocked(queryID, query, domain, client); + } + + // If a regex filter matched, we additionally compare the domain + // against all known whitelisted domains to possibly prevent blocking + // of a specific domain. The logic herein is: + // - Walk regex only if not already exactly matched above + // - If matched, then compare against whitelist + // - If in whitelist, negate matched so this function returns: not-to-be-blocked + if(!blockDomain && + match_regex(domainString, client, REGEX_BLACKLIST) && + !in_whitelist(domainString, client)) + { + // Mark domain as regex matched for this one client + domain->clientstatus->set(domain->clientstatus, clientID, REGEX_BLOCKED); + + // We have to block this domain + blockDomain = true; + query->status = QUERY_WILDCARD; + *blockingreason = "regex blacklisted"; + + // Adjust counters + query_blocked(queryID, query, domain, client); + } + // Explicitly mark as not blocked to skip the entire + // gravity/blacklist chain when the same client asks + // for the same domain in the future + domain->clientstatus->set(domain->clientstatus, clientID, NOT_BLOCKED); + + if(config.debug & DEBUG_QUERIES && blockDomain) + logg("Blocking %s as domain in %s", domainString, *blockingreason); + + return blockDomain; +} + bool _FTL_new_query(const unsigned int flags, const char *name, const char **blockingreason, const struct all_addr *addr, const char *types, const int id, const char type, @@ -237,77 +350,7 @@ bool _FTL_new_query(const unsigned int flags, const char *name, client->lastQuery = querytimestamp; client->numQueriesARP++; - // Get domain pointer - domainsData* domain = getDomain(domainID, true); - if(domain == NULL) - { - // Encountered memory error, skip query - // Free allocated memory - free(domainString); - free(clientIP); - // Release thread lock - unlock_shm(); - return false; - } - - // Only check domains for blocking conditions when global blocking is enabled - bool blockDomain = false; - if(blockingstatus != BLOCKING_DISABLED) - { - // We check the user blacklist first as it is typically smaller than gravity - // If a domain is on the exact blacklist or gravity but also on the whitelist, - // we do NOT block it. - bool black = false, gravity = false; - if(((black = in_blacklist(domainString, client)) || (gravity = in_gravity(domainString, client))) && - !in_whitelist(domainString, client)) - { - blockDomain = true; - if(black) - { - query->status = QUERY_BLACKLIST; - *blockingreason = "exactly blacklisted"; - } - else if(gravity) - { - query->status = QUERY_GRAVITY; - *blockingreason = "gravity blocked"; - } - - // Adjust counters - query_blocked(queryID, query, domain, client); - } - - // If a regex filter matched, we additionally compare the domain - // against all known whitelisted domains to possibly prevent blocking - // of a specific domain. The logic herein is: - // - Walk regex only if not already exactly matched above - // - If matched, then compare against whitelist - // - If in whitelist, negate matched so this function returns: not-to-be-blocked - if(!blockDomain && - match_regex(domainString, client, REGEX_BLACKLIST) && - !in_whitelist(domainString, client)) - { - // Mark domain as regex match (note that this might not apply for all clients!) - domain->regexmatch = REGEX_BLOCKED; - - // We have to block this domain - blockDomain = true; - *blockingreason = "regex blacklisted"; - query->status = QUERY_WILDCARD; - - // Adjust counters - query_blocked(queryID, query, domain, client); - } - else if(domain->regexmatch == REGEX_UNKNOWN && !blockDomain) - { - // Explicitly mark as not blocked to skip regex test - // next time we see this domain - domain->regexmatch = REGEX_NOTBLOCKED; - } - } - - if(config.debug & DEBUG_QUERIES && blockDomain) - logg("Blocking %s as domain in %s", domainString, *blockingreason); + bool blockDomain = FTL_check_blocking(queryID, domainID, clientID, blockingreason); // Free allocated memory free(domainString); diff --git a/src/dnsmasq_interface.h b/src/dnsmasq_interface.h index 35dd2f63..2c969541 100644 --- a/src/dnsmasq_interface.h +++ b/src/dnsmasq_interface.h @@ -44,6 +44,8 @@ void _FTL_upstream_error(const unsigned int rcode, const int id, const char* fil #define FTL_get_blocking_metadata(addrp, flags) _FTL_get_blocking_metadata(addrp, flags, __FILE__, __LINE__) void _FTL_get_blocking_metadata(struct all_addr **addrp, unsigned int *flags, const char* file, const int line); +#define FTL_check_blocking(queryID, domainID, clientID, blockingreason) _FTL_check_blocking(queryID, domainID, clientID, blockingreason, __FILE__, __LINE__) + void FTL_dnsmasq_reload(void); void FTL_fork_and_bind_sockets(struct passwd *ent_pw); diff --git a/src/regex.c b/src/regex.c index b1afd018..53e4c008 100644 --- a/src/regex.c +++ b/src/regex.c @@ -121,12 +121,18 @@ bool match_regex(const char *input, const clientsData *client, const unsigned ch static void free_regex(void) { // Reset cached regex results - for(int i = 0; i < counters->domains; i++) { + for(int i = 0u; i < counters->domains; i++) + { // Get domain pointer domainsData *domain = getDomain(i, true); + if(domain == NULL) + continue; - // Reset regexmatch to unknown - domain->regexmatch = REGEX_UNKNOWN; + // Reset blocking status of domain for all clients to unknown + for(int clientID = 0u; clientID < counters->clients; clientID++) + { + domain->clientstatus->set(domain->clientstatus, clientID, UNKNOWN_BLOCKED); + } } // Return early if we don't use any regex filters diff --git a/src/regex_r.h b/src/regex_r.h index 0733c465..75ddad97 100644 --- a/src/regex_r.h +++ b/src/regex_r.h @@ -17,7 +17,10 @@ bool match_regex(const char *input, const clientsData *client, const unsigned ch void allocate_regex_client_enabled(clientsData *client); void read_regex_from_database(void); -enum { REGEX_UNKNOWN, REGEX_BLOCKED, REGEX_NOTBLOCKED }; enum { REGEX_BLACKLIST, REGEX_WHITELIST }; +// Blocking status constants used by the domain->clientstatus vector +// We explicitly force UNKNOWN_BLOCKED to zero on all platforms as this is the +// default value set initially with calloc +enum { UNKNOWN_BLOCKED = 0, GRAVITY_BLOCKED, BLACKLIST_BLOCKED, REGEX_BLOCKED, NOT_BLOCKED }; #endif //REGEX_H diff --git a/src/vector.c b/src/vector.c index 46511ccd..acb10949 100644 --- a/src/vector.c +++ b/src/vector.c @@ -15,6 +15,135 @@ // memmove() #include + +/********************************* type ucharvec *********************************/ +ucharvec *new_ucharvec(unsigned int initial_size) +{ + ucharvec *v = calloc(1, sizeof(ucharvec)); + v->size = initial_size; + v->capacity = initial_size; + // Calloc ensures they are all set to zero which is the default state + v->items = calloc(initial_size, sizeof(unsigned char) * initial_size); + // Set correct subroutine pointers + v->append = append_ucharvec; + v->set = set_ucharvec; + v->get = get_ucharvec; + v->del = del_ucharvec; + v->free = free_ucharvec; + return v; +} + +static void resize_ucharvec(ucharvec *v, unsigned int capacity) +{ + printf("resize_ucharvec: Resizing %p from %u to %u\n", v, v->capacity, capacity); + + // If ptr is NULL, the call to realloc(ptr, size) is + // equivalent to malloc(size) so we can use it also for + // initializing a vector for the first time. + unsigned char *items = realloc(v->items, sizeof(unsigned char) * capacity); + if (items) + { + v->items = items; + v->capacity = capacity; + } + else + { + printf("ERROR: Memory allocation failed in resize_ucharvec(%p, %u)", + v, capacity); + } +} +void append_ucharvec(ucharvec *v, unsigned char item) +{ + if(v == NULL) + { + printf("ERROR: Passed NULL vector to append_ucharvec(%p, %u)", + v, item); + return; + } + + // Check if vector needs to be resized + if (v->capacity == v->size) + { + resize_ucharvec(v, v->capacity + VEC_ALLOC_STEP); + } + + // Append item + unsigned int index = v->size++; + v->items[index] = item; +} + +void set_ucharvec(ucharvec *v, unsigned int index, unsigned char item) +{ + if(v == NULL) + { + printf("ERROR: Passed NULL vector to set_ucharvec(%p, %u, %u)", + v, index, item); + return; + } + + if (index >= v->size) + { + printf("ERROR: Boundary violation in set_ucharvec(%p, %u, %u)", + v, index, item); + return; + } + + // Set item + v->items[index] = item; +} + +// This function has no effects except to return a value. It can +// be subject to data flow analysis and might be eliminated. +// Hence, we add the "pure" attribute to this function. +unsigned char __attribute__((pure)) get_ucharvec(ucharvec *v, unsigned int index) +{ + if(v == NULL) + { + printf("ERROR: Passed NULL vector to get_ucharvec(%p, %u)", + v, index); + return 0; + } + + if (index >= v->size) + { + printf("ERROR: Boundary violation in get_ucharvec(%p, %u)", + v, index); + return 0; + } + + return v->items[index]; +} + +void del_ucharvec(ucharvec *v, unsigned int index) +{ + if (index >= v->size) + return; + + // Use memmove to ensure there are no gaps in the vector + size_t move = v->size - index - 1u; + memmove(&v->items[index], &v->items[index + 1u], move * sizeof(v->items[index])); + + v->size--; + + // // Shorten vector to save some space + // if (v->size > 0u && v->size == v->capacity / 4) + // { + // vResize(v, v->capacity / 2); + // } +} + +void free_ucharvec(ucharvec *v) +{ + // Free elements of the vector... + free(v->items); + // ...and then then vector itself + free(v); + v = NULL; +} +/********************************* type ucharvec *********************************/ + + +/* vector *vNew(void) { vector *v = calloc(1, sizeof(vector)); @@ -93,7 +222,7 @@ void vSet(vector *v, unsigned int index, void *item, bool allocated) if (index >= v->size) { - printf("ERROR: Boundary violation vSet(%p, %u %p)", + printf("ERROR: Boundary violation in vSet(%p, %u, %p)", v, index, item); return; } @@ -160,3 +289,4 @@ void vFree(vector *v) free(v); v = NULL; } +*/ diff --git a/src/vector.h b/src/vector.h index 90f20c08..4b101222 100644 --- a/src/vector.h +++ b/src/vector.h @@ -15,13 +15,33 @@ #define VEC_ALLOC_STEP 2u +/* typedef struct vector { unsigned int size; unsigned int capacity; bool *alloc; void **items; } vector; +*/ +typedef struct ucharvec { + unsigned int size; + unsigned int capacity; + unsigned char *items; + unsigned char (*get)(struct ucharvec *, unsigned int); + void (*append)(struct ucharvec *, unsigned char); + void (*set)(struct ucharvec *, unsigned int, unsigned char); + void (*del)(struct ucharvec *, unsigned int); + void (*free)(struct ucharvec *); +} ucharvec; + +ucharvec *new_ucharvec(unsigned int initial_size); +void append_ucharvec(ucharvec *v, unsigned char item); +void set_ucharvec(ucharvec *v, unsigned int index, unsigned char item); +unsigned char get_ucharvec(ucharvec *v, unsigned int index) __attribute__((pure)); +void del_ucharvec(ucharvec *v, unsigned int index); +void free_ucharvec(ucharvec *v); +/* vector *vNew(void); unsigned int vSize(vector *v); void vAppend(vector *v, void *item, bool allocated); @@ -29,5 +49,5 @@ void vSet(vector *v, unsigned int index, void *item, bool allocated); void *vGet(vector *v, unsigned int index) __attribute__((pure)); void vRemove(vector *v, unsigned int index); void vFree(vector *v); - +*/ #endif //VECTOR_H