From deae66ba58a54f009316691ea172016f60f6ed03 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 17 Jan 2021 11:39:12 +0100 Subject: [PATCH 01/21] Optimize datastructures using bitfields and item re-arrangement (to minimize padding). This reduces the size of query, client, and regex records by 8 bytes per item. Note that this optimization was done on x86_64 and may not apply for other architectures (32bit architectures already used less padding). Signed-off-by: DL6ER --- src/FTL.h | 1 + src/api/api.c | 14 ++++++------- src/config.h | 24 +++++++++++++--------- src/database/aliasclients.c | 16 +++++++-------- src/database/gravity-db.c | 18 ++++++++-------- src/database/gravity-db.h | 4 ++-- src/database/network-table.c | 2 +- src/database/query-table.c | 4 ++-- src/datastructure.c | 8 ++++---- src/datastructure.h | 26 +++++++++++++++-------- src/dnsmasq/forward.c | 4 ++-- src/dnsmasq_interface.c | 40 ++++++++++++++++++------------------ src/dnsmasq_interface.h | 2 +- src/edns0.c | 2 +- src/edns0.h | 8 ++++---- src/overTime.h | 2 +- src/regex.c | 26 +++++++++++------------ src/regex_r.h | 12 ++++------- src/resolve.c | 6 +++--- 19 files changed, 115 insertions(+), 104 deletions(-) diff --git a/src/FTL.h b/src/FTL.h index 879e1e9b0..3ed6ce466 100644 --- a/src/FTL.h +++ b/src/FTL.h @@ -143,6 +143,7 @@ // Preprocessor help functions #define str(x) # x #define xstr(x) str(x) +#define STATIC_ASSERT(COND,MSG) typedef char static_assertion_##MSG[(COND)?1:-1] extern pthread_t telnet_listenthreadv4; extern pthread_t telnet_listenthreadv6; diff --git a/src/api/api.c b/src/api/api.c index 3874f4421..86a5a0179 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -397,7 +397,7 @@ void getTopClients(const char *client_message, const int *sock) // Get client pointer const clientsData* client = getClient(clientID, true); // Skip invalid clients and also those managed by alias clients - if(client == NULL || (!client->aliasclient && client->aliasclient_id >= 0)) + if(client == NULL || (!client->flags.aliasclient && client->aliasclient_id >= 0)) { temparray[clientID][0] = -1; continue; @@ -823,7 +823,7 @@ void getAllQueries(const char *client_message, const int *sock) clientid = i; // Is this a alias-client? - if(client->aliasclient) + if(client->flags.aliasclient) clientid_list = get_aliasclient_list(i); break; @@ -1312,7 +1312,7 @@ void getClientsOverTime(const int *sock) // Check if this client should be skipped if(insetupVarsArray(getstr(client->ippos)) || insetupVarsArray(getstr(client->namepos)) || - (!client->aliasclient && client->aliasclient_id > -1)) + (!client->flags.aliasclient && client->aliasclient_id > -1)) skipclient[clientID] = true; } } @@ -1387,7 +1387,7 @@ void getClientNames(const int *sock) // Check if this client should be skipped if(insetupVarsArray(getstr(client->ippos)) || insetupVarsArray(getstr(client->namepos)) || - (!client->aliasclient && client->aliasclient_id > -1)) + (!client->flags.aliasclient && client->aliasclient_id > -1)) skipclient[clientID] = true; } } @@ -1434,7 +1434,7 @@ void getUnknownQueries(const int *sock) const queriesData* query = getQuery(queryID, true); if(query == NULL || - (query->status != QUERY_UNKNOWN && query->complete)) + (query->status != QUERY_UNKNOWN && query->flags.complete)) continue; char type[5]; @@ -1459,7 +1459,7 @@ void getUnknownQueries(const int *sock) const char *clientIP = getstr(client->ippos); if(istelnet[*sock]) - ssend(*sock, "%lli %i %i %s %s %s %i %s\n", (long long)query->timestamp, queryID, query->id, type, getstr(domain->domainpos), clientIP, query->status, query->complete ? "true" : "false"); + ssend(*sock, "%lli %i %i %s %s %s %i %s\n", (long long)query->timestamp, queryID, query->id, type, getstr(domain->domainpos), clientIP, query->status, query->flags.complete ? "true" : "false"); else { pack_int32(*sock, (int32_t)query->timestamp); pack_int32(*sock, query->id); @@ -1473,7 +1473,7 @@ void getUnknownQueries(const int *sock) return; pack_uint8(*sock, query->status); - pack_bool(*sock, query->complete); + pack_bool(*sock, query->flags.complete); } } } diff --git a/src/config.h b/src/config.h index 36c297146..f3b5f323b 100644 --- a/src/config.h +++ b/src/config.h @@ -24,17 +24,11 @@ void get_privacy_level(FILE *fp); void get_blocking_mode(FILE *fp); void read_debuging_settings(FILE *fp); +// We do not use bitfields in here as this struct exists only once in memory. +// Accessing bitfields may produce slightly more inefficient code on some +// architectures (such as ARM) and savng a few bit of RAM but bloating up the +// rest of the application each time these fields are accessed is bad. typedef struct { - int maxDBdays; - int port; - int maxlogage; - int dns_port; - unsigned int delay_startup; - enum debug_flags debug; - unsigned int network_expire; - enum privacy_level privacylevel; - enum blocking_mode blockingmode; - enum refresh_hostnames refresh_hostnames; bool socket_listenlocal; bool analyze_AAAA; bool resolveIPv6; @@ -48,6 +42,16 @@ typedef struct { bool block_esni; bool names_from_netdb; bool edns0_ecs; + enum privacy_level privacylevel; + enum blocking_mode blockingmode; + enum refresh_hostnames refresh_hostnames; + int maxDBdays; + int port; + int maxlogage; + int dns_port; + unsigned int delay_startup; + unsigned int network_expire; + enum debug_flags debug; time_t DBinterval; } ConfigStruct; diff --git a/src/database/aliasclients.c b/src/database/aliasclients.c index 023c402a6..43b9308b2 100644 --- a/src/database/aliasclients.c +++ b/src/database/aliasclients.c @@ -63,7 +63,7 @@ static void recompute_aliasclient(const int aliasclientID) // Get pointer to client candidate const clientsData *client = getClient(clientID, true); // Skip invalid clients and alias-clients - if(client == NULL || client->aliasclient) + if(client == NULL || client->flags.aliasclient) continue; // Skip clients that are not managed by this aliasclient @@ -131,7 +131,7 @@ bool import_aliasclients(void) const int clientID = findClientID(aliasclient_str, false, true); clientsData *client = getClient(clientID, true); - client->new = false; + client->flags.new = false; // Reset counter client->count = 0; @@ -141,7 +141,7 @@ bool import_aliasclients(void) client->namepos = addstr(name); // This is a aliasclient - client->aliasclient = true; + client->flags.aliasclient = true; client->aliasclient_id = aliasclient_id; // Debug logging @@ -169,7 +169,7 @@ bool import_aliasclients(void) static int get_aliasclient_ID(const clientsData *client) { // Skip alias-clients themselves - if(client->aliasclient) + if(client->flags.aliasclient) return -1; const char *clientIP = getstr(client->ippos); @@ -190,7 +190,7 @@ static int get_aliasclient_ID(const clientsData *client) const clientsData *alias_client = getClient(aliasclientID, true); // Skip clients that are not alias-clients - if(!alias_client->aliasclient) + if(!alias_client->flags.aliasclient) continue; // Compare MAC address of the current client to the @@ -220,7 +220,7 @@ static int get_aliasclient_ID(const clientsData *client) void reset_aliasclient(clientsData *client) { // Skip alias-clients themselves - if(client->aliasclient) + if(client->flags.aliasclient) return; // Find corresponding alias-client (if any) @@ -288,7 +288,7 @@ void reimport_aliasclients(void) // Get pointer to client candidate clientsData *client = getClient(clientID, true); // Skip invalid and non-alias-clients - if(client == NULL || !client->aliasclient) + if(client == NULL || !client->flags.aliasclient) continue; // Reset this alias-client @@ -309,7 +309,7 @@ void reimport_aliasclients(void) // Get pointer to client candidate clientsData *client = getClient(clientID, true); // Skip invalid and alias-clients - if(client == NULL || client->aliasclient) + if(client == NULL || client->flags.aliasclient) continue; reset_aliasclient(client); diff --git a/src/database/gravity-db.c b/src/database/gravity-db.c index 633abc0f9..c067d1dc1 100644 --- a/src/database/gravity-db.c +++ b/src/database/gravity-db.c @@ -30,7 +30,7 @@ // reset_aliasclient() #include "aliasclients.h" -// Definition of struct regex_data +// Definition of struct regexData #include "../regex_r.h" // Prefix of interface names in the client table @@ -219,7 +219,7 @@ static inline const char *show_client_string(const char *hwaddr, const char *hos static bool get_client_groupids(clientsData* client) { const char *ip = getstr(client->ippos); - client->found_group = false; + client->flags.found_group = false; client->groupspos = 0u; // Do not proceed when database is not available @@ -619,7 +619,7 @@ static bool get_client_groupids(clientsData* client) show_client_string(hwaddr, hostname, ip)); client->groupspos = addstr("0"); - client->found_group = true; + client->flags.found_group = true; if(hwaddr != NULL) { @@ -682,7 +682,7 @@ static bool get_client_groupids(clientsData* client) if(result != NULL) { client->groupspos = addstr(result); - client->found_group = true; + client->flags.found_group = true; } } else if(rc == SQLITE_DONE) @@ -690,7 +690,7 @@ static bool get_client_groupids(clientsData* client) // Found no record for this client in the database // -> No associated groups client->groupspos = addstr(""); - client->found_group = true; + client->flags.found_group = true; } else { @@ -805,7 +805,7 @@ bool gravityDB_prepare_client_statements(clientsData *client) // Get associated groups for this client (if defined) char *querystr = NULL; - if(!client->found_group && !get_client_groupids(client)) + if(!client->flags.found_group && !get_client_groupids(client)) return false; // Prepare whitelist statement @@ -888,7 +888,7 @@ static inline void gravityDB_finalize_client_statements(clientsData *client) // client sends a query if(client != NULL) { - client->found_group = false; + client->flags.found_group = false; } } @@ -1304,14 +1304,14 @@ bool in_auditlist(const char *domain) return domain_in_list(domain, auditlist_stmt, "auditlist"); } -bool gravityDB_get_regex_client_groups(clientsData* client, const unsigned int numregex, const regex_data *regex, +bool gravityDB_get_regex_client_groups(clientsData* client, const unsigned int numregex, const regexData *regex, const unsigned char type, const char* table) { if(config.debug & DEBUG_REGEX) logg("Getting regex client groups for client with ID %i", client->id); char *querystr = NULL; - if(!client->found_group && !get_client_groupids(client)) + if(!client->flags.found_group && !get_client_groupids(client)) return false; // Group filtering diff --git a/src/database/gravity-db.h b/src/database/gravity-db.h index 5aa379473..5b8198fca 100644 --- a/src/database/gravity-db.h +++ b/src/database/gravity-db.h @@ -12,7 +12,7 @@ // clientsData #include "../datastructure.h" -// regex_data +// regexData #include "../regex_r.h" // Table indices @@ -35,7 +35,7 @@ bool in_gravity(const char *domain, clientsData* client); bool in_blacklist(const char *domain, clientsData* client); bool in_whitelist(const char *domain, const DNSCacheData *dns_cache, clientsData* client); -bool gravityDB_get_regex_client_groups(clientsData* client, const unsigned int numregex, const regex_data *regex, +bool gravityDB_get_regex_client_groups(clientsData* client, const unsigned int numregex, const regexData *regex, const unsigned char type, const char* table); #endif //GRAVITY_H diff --git a/src/database/network-table.c b/src/database/network-table.c index 40c9f9ab9..458db2b11 100644 --- a/src/database/network-table.c +++ b/src/database/network-table.c @@ -689,7 +689,7 @@ static bool add_FTL_clients_to_network_table(enum arp_status *client_status, tim } // Silently skip alias-clients - they do not really exist - if(client->aliasclient) + if(client->flags.aliasclient) continue; // Get hostname and IP address of this client diff --git a/src/database/query-table.c b/src/database/query-table.c index 169ccdad2..4034703ca 100644 --- a/src/database/query-table.c +++ b/src/database/query-table.c @@ -107,7 +107,7 @@ void DB_save_queries(void) continue; } - if(!query->complete && query->timestamp > currenttimestamp-2) + if(!query->flags.complete && query->timestamp > currenttimestamp-2) { // Break if a brand new query (age < 2 seconds) is not yet completed // giving it a chance to be stored next time @@ -453,7 +453,7 @@ void DB_read_queries(void) query->timeidx = timeidx; query->db = dbid; query->id = 0; - query->complete = true; // Mark as all information is available + query->flags.complete = true; // Mark as all information is available query->response = 0; query->dnssec = DNSSEC_UNSPECIFIED; query->reply = REPLY_UNKNOWN; diff --git a/src/datastructure.c b/src/datastructure.c index c800fa268..cd0093738 100644 --- a/src/datastructure.c +++ b/src/datastructure.c @@ -245,14 +245,14 @@ int findClientID(const char *clientIP, const bool count, const bool aliasclient) // Due to the nature of us being the resolver, // the actual resolving of the host name has // to be done separately to be non-blocking - client->new = true; + client->flags.new = true; client->namepos = 0; set_event(RESOLVE_NEW_HOSTNAMES); // No query seen so far client->lastQuery = 0; client->numQueriesARP = client->count; // Configured groups are yet unknown - client->found_group = false; + client->flags.found_group = false; client->groupspos = 0u; // Store time this client was added, we re-read group settings // some time after adding a client to ensure we pick up possible @@ -265,7 +265,7 @@ int findClientID(const char *clientIP, const bool count, const bool aliasclient) client->hwlen = -1; memset(client->hwaddr, 0, sizeof(client->hwaddr)); // This may be a alias-client, the ID is set elsewhere - client->aliasclient = aliasclient; + client->flags.aliasclient = aliasclient; client->aliasclient_id = -1; // Initialize client-specific overTime data @@ -303,7 +303,7 @@ void change_clientcount(clientsData *client, int total, int blocked, int overTim client->overTime[overTimeIdx] += overTimeMod; // Also add counts to the conencted alias-client (if any) - if(client->aliasclient) + if(client->flags.aliasclient) { logg("WARN: Should not add to alias-client directly (client \"%s\" (%s))!", getstr(client->namepos), getstr(client->ippos)); diff --git a/src/datastructure.h b/src/datastructure.h index c48a4a2f8..6db105bb3 100644 --- a/src/datastructure.h +++ b/src/datastructure.h @@ -35,16 +35,24 @@ typedef struct { unsigned long response; // saved in units of 1/10 milliseconds (1 = 0.1ms, 2 = 0.2ms, 2500 = 250.0ms, etc.) time_t timestamp; int64_t db; - bool whitelisted; - bool complete; + // Adjacent bit field members in the struct flags may be packed to share + // and straddle the individual bytes. It is useful to pack the memory as + // tightly as possible as there may be dozens of thousands of these + // objects in memory (one per query). + // C99 guarentees that bit-fields will be packed as tightly as possible, + // provided they don’t cross storageau unit boundaries (6.7.2.1 #10). + struct query_flags { + bool whitelisted :1; + bool complete :1; + } flags; } queriesData; typedef struct { unsigned char magic; bool new; - in_addr_t port; int count; int failed; + in_addr_t port; size_t ippos; size_t namepos; time_t lastQuery; @@ -55,15 +63,17 @@ typedef struct { unsigned char reread_groups; char hwlen; unsigned char hwaddr[16]; // See DHCP_CHADDR_MAX in dnsmasq/dhcp-protocol.h - bool new; - bool found_group; - bool aliasclient; + struct client_flags { + bool new:1; + bool found_group:1; + bool aliasclient:1; + } flags; int count; int blockedcount; int aliasclient_id; - int overTime[OVERTIME_SLOTS]; unsigned int id; unsigned int numQueriesARP; + int overTime[OVERTIME_SLOTS]; size_t groupspos; size_t ippos; size_t namepos; @@ -74,9 +84,9 @@ typedef struct { typedef struct { unsigned char magic; - size_t domainpos; int count; int blockedcount; + size_t domainpos; } domainsData; typedef struct { diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index 782963af2..33646911d 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -1541,7 +1541,7 @@ void receive_query(struct listener *listen, time_t now) #endif //********************** Pi-hole modification **********************// - struct edns_data edns = { 0 }; + ednsData edns = { 0 }; if (find_pseudoheader(header, (size_t)n, NULL, &pheader, NULL, NULL)) FTL_parse_pseudoheaders(header, n, &source_addr, &edns); //******************************************************************// @@ -1945,7 +1945,7 @@ unsigned char *tcp_request(int confd, time_t now, no_cache_dnssec = 1; //********************** Pi-hole modification **********************// - struct edns_data edns = { 0 }; + ednsData edns = { 0 }; if (find_pseudoheader(header, (size_t)size, NULL, &pheader, NULL, NULL)) FTL_parse_pseudoheaders(header, size, &peer_addr, &edns); //******************************************************************// diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index d8a62d94e..228dc98d0 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -104,7 +104,7 @@ static bool check_domain_blocked(const char *domain, const int clientID, // Check domains against gravity domains // Skipped when the domain is whitelisted or blocked by exact blacklist - if(!query->whitelisted && !blockDomain && + if(!query->flags.whitelisted && !blockDomain && in_gravity(domain, client)) { // We block this domain @@ -120,7 +120,7 @@ static bool check_domain_blocked(const char *domain, const int clientID, // Check domain against blacklist regex filters // Skipped when the domain is whitelisted or blocked by exact blacklist or gravity int regex_idx = 0; - if(!query->whitelisted && !blockDomain && + if(!query->flags.whitelisted && !blockDomain && (regex_idx = match_regex(domain, dns_cache, client->id, REGEX_BLACKLIST, false)) > -1) { // We block this domain @@ -188,7 +188,7 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c // Do not block if the entire query is to be permitted // as something along the CNAME path hit the whitelist - if(!query->whitelisted) + if(!query->flags.whitelisted) { query_blocked(query, domain, client, QUERY_BLACKLIST); force_next_DNS_reply = dns_cache->force_reply; @@ -208,7 +208,7 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c // Do not block if the entire query is to be permitted // as sometving along the CNAME path hit the whitelist - if(!query->whitelisted) + if(!query->flags.whitelisted) { query_blocked(query, domain, client, QUERY_GRAVITY); force_next_DNS_reply = dns_cache->force_reply; @@ -229,7 +229,7 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c // Do not block if the entire query is to be permitted // as sometving along the CNAME path hit the whitelist - if(!query->whitelisted) + if(!query->flags.whitelisted) { query_blocked(query, domain, client, QUERY_REGEX); return true; @@ -245,7 +245,7 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c logg("%s is known as not to be blocked (whitelisted)", domainstr); } - query->whitelisted = true; + query->flags.whitelisted = true; return false; break; @@ -264,7 +264,7 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c } // Skip all checks and continue if we hit already at least one whitelist in the chain - if(query->whitelisted) + if(query->flags.whitelisted) { if(config.debug & DEBUG_QUERIES) { @@ -280,19 +280,19 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c const char *blockedDomain = domainstr; // Check whitelist (exact + regex) for match - query->whitelisted = in_whitelist(domainstr, dns_cache, client); + query->flags.whitelisted = in_whitelist(domainstr, dns_cache, client); bool blockDomain = false; unsigned char new_status = QUERY_UNKNOWN; // Check blacklist (exact + regex) and gravity for queried domain - if(!query->whitelisted) + if(!query->flags.whitelisted) { blockDomain = check_domain_blocked(domainstr, clientID, client, query, dns_cache, blockingreason, &new_status); } // Check blacklist (exact + regex) and gravity for _esni.domain if enabled (defaulting to true) - if(config.block_esni && !query->whitelisted && !blockDomain && strncasecmp(domainstr, "_esni.", 6u) == 0) + if(config.block_esni && !query->flags.whitelisted && !blockDomain && strncasecmp(domainstr, "_esni.", 6u) == 0) { blockDomain = check_domain_blocked(domainstr + 6u, clientID, client, query, dns_cache, blockingreason, &new_status); @@ -322,7 +322,7 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c // gravity/blacklist chain when the same client asks // for the same domain in the future. Explicitly store // domain as whitelisted if this is the case - dns_cache->blocking_status = query->whitelisted ? WHITELISTED : NOT_BLOCKED; + dns_cache->blocking_status = query->flags.whitelisted ? WHITELISTED : NOT_BLOCKED; } free(domainstr); @@ -458,7 +458,7 @@ bool _FTL_CNAME(const char *domain, const struct crec *cpp, const int id, const bool _FTL_new_query(const unsigned int flags, const char *name, const char **blockingreason, const union all_addr *addr, const char *types, const unsigned short qtype, const int id, - const struct edns_data *edns, const enum protocol proto, + const ednsData *edns, const enum protocol proto, const char* file, const int line) { // Create new query in data structure @@ -634,7 +634,7 @@ bool _FTL_new_query(const unsigned int flags, const char *name, // Initialize database rowID with zero, will be set when the query is stored in the long-term DB query->db = 0; query->id = id; - query->complete = false; + query->flags.complete = false; query->response = converttimeval(request); // Initialize reply type query->reply = REPLY_UNKNOWN; @@ -852,7 +852,7 @@ void _FTL_forwarded(const unsigned int flags, const char *name, const struct ser // - the query was formally known as cached but had to be forwarded // (this is a special case further described below) // Use short-circuit evaluation to check if query is NULL - if(query == NULL || (query->complete && query->status != QUERY_CACHE)) + if(query == NULL || (query->flags.complete && query->status != QUERY_CACHE)) { free(upstreamIP); unlock_shm(); @@ -902,7 +902,7 @@ void _FTL_forwarded(const unsigned int flags, const char *name, const struct ser // Query is no longer unknown counters->unknown--; // Hereby, this query is now fully determined - query->complete = true; + query->flags.complete = true; } // Set query status to forwarded only after the @@ -1055,7 +1055,7 @@ void _FTL_reply(const unsigned int flags, const char *name, const union all_addr // Check if this domain matches exactly const bool isExactMatch = strcmp_escaped(name, getstr(domain->domainpos)); - if((flags & F_CONFIG) && isExactMatch && !query->complete) + if((flags & F_CONFIG) && isExactMatch && !query->flags.complete) { // Answered from local configuration, might be a wildcard or user-provided // This query is no longer unknown @@ -1075,7 +1075,7 @@ void _FTL_reply(const unsigned int flags, const char *name, const union all_addr save_reply_type(flags, addr, query, response); // Hereby, this query is now fully determined - query->complete = true; + query->flags.complete = true; } else if((flags & F_FORWARD) && isExactMatch) { @@ -1106,7 +1106,7 @@ void _FTL_reply(const unsigned int flags, const char *name, const union all_addr // Save reply type and update individual reply counters save_reply_type(flags, addr, query, response); } - else if(isExactMatch && !query->complete) + else if(isExactMatch && !query->flags.complete) { logg("*************************** unknown REPLY ***************************"); print_flags(flags); @@ -1357,7 +1357,7 @@ void _FTL_cache(const unsigned int flags, const char *name, const union all_addr // Skip this query if already marked as complete // Use short-circuit evaluation to check query if query is NULL - if(query == NULL || query->complete) + if(query == NULL || query->flags.complete) { unlock_shm(); return; @@ -1396,7 +1396,7 @@ void _FTL_cache(const unsigned int flags, const char *name, const union all_addr save_reply_type(flags, addr, query, response); // Hereby, this query is now fully determined - query->complete = true; + query->flags.complete = true; } else { diff --git a/src/dnsmasq_interface.h b/src/dnsmasq_interface.h index e80de13c9..7f7d34bfc 100644 --- a/src/dnsmasq_interface.h +++ b/src/dnsmasq_interface.h @@ -22,7 +22,7 @@ enum protocol { TCP, UDP }; void FTL_next_iface(const char *newiface); #define FTL_new_query(flags, name, blockingreason, addr, types, qtype, id, edns, proto) _FTL_new_query(flags, name, blockingreason, addr, types, qtype, id, edns, proto, __FILE__, __LINE__) -bool _FTL_new_query(const unsigned int flags, const char *name, const char** blockingreason, const union all_addr *addr, const char *types, const unsigned short qtype, const int id, const struct edns_data *edns, enum protocol proto, const char* file, const int line); +bool _FTL_new_query(const unsigned int flags, const char *name, const char** blockingreason, const union all_addr *addr, const char *types, const unsigned short qtype, const int id, const ednsData *edns, enum protocol proto, const char* file, const int line); #define FTL_forwarded(flags, name, serv, id) _FTL_forwarded(flags, name, serv, id, __FILE__, __LINE__) void _FTL_forwarded(const unsigned int flags, const char *name, const struct server *serv, const int id, const char* file, const int line); diff --git a/src/edns0.c b/src/edns0.c index 32c4033b0..7e4170cab 100644 --- a/src/edns0.c +++ b/src/edns0.c @@ -41,7 +41,7 @@ // dnsmasq option: --add-cpe-id=... #define EDNS0_CPE_ID EDNS0_OPTION_NOMCPEID -void FTL_parse_pseudoheaders(struct dns_header *header, size_t n, union mysockaddr *peer, struct edns_data *edns) +void FTL_parse_pseudoheaders(struct dns_header *header, size_t n, union mysockaddr *peer, ednsData *edns) { int is_sign; size_t plen; diff --git a/src/edns0.h b/src/edns0.h index 3f1cc74fc..31261c52a 100644 --- a/src/edns0.h +++ b/src/edns0.h @@ -12,14 +12,14 @@ #include "edns0.h" -struct edns_data { +typedef struct { bool client_set; - char client[ADDRSTRLEN]; bool mac_set; + char client[ADDRSTRLEN]; char mac_byte[6]; char mac_text[18]; -}; +} ednsData; -void FTL_parse_pseudoheaders(struct dns_header *header, size_t n, union mysockaddr *peer, struct edns_data *edns); +void FTL_parse_pseudoheaders(struct dns_header *header, size_t n, union mysockaddr *peer, ednsData *edns); #endif // EDNS0_HEADER diff --git a/src/overTime.h b/src/overTime.h index e02480605..2bea5cb17 100644 --- a/src/overTime.h +++ b/src/overTime.h @@ -27,11 +27,11 @@ void moveOverTimeMemory(const time_t mintime); typedef struct { unsigned char magic; - time_t timestamp; int total; int blocked; int cached; int forwarded; + time_t timestamp; int querytypedata[TYPE_MAX-1]; } overTimeData; diff --git a/src/regex.c b/src/regex.c index d4f8622b9..1ec309c5e 100644 --- a/src/regex.c +++ b/src/regex.c @@ -28,13 +28,13 @@ const char *regextype[REGEX_MAX] = { "blacklist", "whitelist", "CLI" }; -static regex_data *white_regex = NULL; -static regex_data *black_regex = NULL; -static regex_data *cli_regex = NULL; +static regexData *white_regex = NULL; +static regexData *black_regex = NULL; +static regexData *cli_regex = NULL; static unsigned int num_regex[REGEX_MAX] = { 0 }; unsigned int regex_change = 0; -static inline regex_data *get_regex_ptr(const enum regex_type regexid) +static inline regexData *get_regex_ptr(const enum regex_type regexid) { switch (regexid) { @@ -52,7 +52,7 @@ static inline regex_data *get_regex_ptr(const enum regex_type regexid) static inline void free_regex_ptr(const enum regex_type regexid) { - regex_data **regex; + regexData **regex; switch (regexid) { case REGEX_BLACKLIST: @@ -97,7 +97,7 @@ unsigned int __attribute__((pure)) get_num_regex(const enum regex_type regexid) regexec() to match against a string */ static bool compile_regex(const char *regexin, const enum regex_type regexid) { - regex_data *regex = get_regex_ptr(regexid); + regexData *regex = get_regex_ptr(regexid); int index = num_regex[regexid]++; // Extract possible Pi-hole extensions @@ -205,7 +205,7 @@ int match_regex(const char *input, const DNSCacheData* dns_cache, const int clie const enum regex_type regexid, const bool regextest) { int match_idx = -1; - regex_data *regex = get_regex_ptr(regexid); + regexData *regex = get_regex_ptr(regexid); #ifdef USE_TRE_REGEX regmatch_t match = { 0 }; // This also disables any sub-matching #endif @@ -370,7 +370,7 @@ static void free_regex(void) // Loop over regex types for(enum regex_type regexid = REGEX_BLACKLIST; regexid < REGEX_MAX; regexid++) { - regex_data *regex = get_regex_ptr(regexid); + regexData *regex = get_regex_ptr(regexid); // Reset counter for number of regex const unsigned int oldcount = num_regex[regexid]; @@ -460,15 +460,15 @@ static void read_regex_table(const enum regex_type regexid) } // Allocate memory for regex - regex_data *regex = NULL; + regexData *regex = NULL; if(regexid == REGEX_BLACKLIST) { - black_regex = calloc(count, sizeof(regex_data)); + black_regex = calloc(count, sizeof(regexData)); regex = black_regex; } else { - white_regex = calloc(count, sizeof(regex_data)); + white_regex = calloc(count, sizeof(regexData)); regex = white_regex; } @@ -554,7 +554,7 @@ void read_regex_from_database(void) // Get client pointer clientsData *client = getClient(clientID, true); // Skip invalid and alias-clients - if(client == NULL || client->aliasclient) + if(client == NULL || client->flags.aliasclient) continue; reload_per_client_regex(client); @@ -614,7 +614,7 @@ int regex_test(const bool debug_mode, const bool quiet, const char *domainin, co { // Compile CLI regex logg("%s Compiling regex filter...", cli_info()); - cli_regex = calloc(1, sizeof(regex_data)); + cli_regex = calloc(1, sizeof(regexData)); // Compile CLI regex timer_start(REGEX_TIMER); diff --git a/src/regex_r.h b/src/regex_r.h index 3627b5983..1b4a4fbc0 100644 --- a/src/regex_r.h +++ b/src/regex_r.h @@ -24,19 +24,15 @@ extern const char *regextype[]; #include #endif -typedef struct regex_data { +typedef struct { bool available; bool inverted; bool query_type_inverted; - char *string; - int database_id; enum query_types query_type; + int database_id; + char *string; regex_t regex; -} regex_data; - -struct query_details { - enum query_types query_type; -}; +} regexData; unsigned int get_num_regex(const enum regex_type regexid) __attribute__((pure)); int match_regex(const char *input, const DNSCacheData* dns_cache, const int clientID, diff --git a/src/resolve.c b/src/resolve.c index d06b0108c..0365c5931 100644 --- a/src/resolve.c +++ b/src/resolve.c @@ -373,13 +373,13 @@ static void resolveClients(const bool onlynew, const bool force_refreshing) } // Skip alias-clients - if(client->aliasclient) + if(client->flags.aliasclient) { unlock_shm(); continue; } - bool newflag = client->new; + bool newflag = client->flags.new; size_t ippos = client->ippos; size_t oldnamepos = client->namepos; @@ -470,7 +470,7 @@ static void resolveClients(const bool onlynew, const bool force_refreshing) // Store obtained host name (may be unchanged) client->namepos = newnamepos; // Mark entry as not new - client->new = false; + client->flags.new = false; if(config.debug & DEBUG_RESOLVER) logg("Client %s -> \"%s\" is new", getstr(ippos), getstr(newnamepos)); From 50d182950a1091dbdd08d015118b05fe4ef16f39 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 17 Jan 2021 14:00:53 +0100 Subject: [PATCH 02/21] Statically assert struct sizes are what we expect. This prevents us from increasing the memory needs unintentionally (e.g. due to sub-optimal padding) Signed-off-by: DL6ER --- src/CMakeLists.txt | 1 + src/FTL.h | 1 - src/config.h | 3 +++ src/datastructure.h | 9 +++++++++ src/edns0.h | 4 +++- src/overTime.h | 4 ++++ src/regex_r.h | 4 ++++ src/shmem.h | 5 +++++ src/static_assert.h | 27 +++++++++++++++++++++++++++ src/vector.h | 3 +++ 10 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 src/static_assert.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 462ca24ad..e2d6facc8 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -143,6 +143,7 @@ set(sources shmem.h signals.c signals.h + static_assert.h timers.c timers.h vector.c diff --git a/src/FTL.h b/src/FTL.h index 3ed6ce466..879e1e9b0 100644 --- a/src/FTL.h +++ b/src/FTL.h @@ -143,7 +143,6 @@ // Preprocessor help functions #define str(x) # x #define xstr(x) str(x) -#define STATIC_ASSERT(COND,MSG) typedef char static_assertion_##MSG[(COND)?1:-1] extern pthread_t telnet_listenthreadv4; extern pthread_t telnet_listenthreadv6; diff --git a/src/config.h b/src/config.h index f3b5f323b..fb767dede 100644 --- a/src/config.h +++ b/src/config.h @@ -17,6 +17,8 @@ #include // typedef uni32_t #include +// assert_sizeof +#include "static_assert.h" void getLogFilePath(void); void read_FTLconf(void); @@ -54,6 +56,7 @@ typedef struct { enum debug_flags debug; time_t DBinterval; } ConfigStruct; +ASSERT_SIZEOF(ConfigStruct, 56, 48, 48); typedef struct { const char* conf; diff --git a/src/datastructure.h b/src/datastructure.h index 6db105bb3..6a2454e98 100644 --- a/src/datastructure.h +++ b/src/datastructure.h @@ -15,6 +15,8 @@ // enum privacy_level #include "enums.h" +// assert_sizeof +#include "static_assert.h" extern const char *querytypes[TYPE_MAX]; @@ -47,6 +49,9 @@ typedef struct { } flags; } queriesData; +// ARM needs extra padding at the end +ASSERT_SIZEOF(queriesData, 64, 52, 56); + typedef struct { unsigned char magic; bool new; @@ -57,6 +62,7 @@ typedef struct { size_t namepos; time_t lastQuery; } upstreamsData; +ASSERT_SIZEOF(upstreamsData, 40, 28, 28); typedef struct { unsigned char magic; @@ -81,6 +87,7 @@ typedef struct { time_t lastQuery; time_t firstSeen; } clientsData; +ASSERT_SIZEOF(clientsData, 688, 664, 664); typedef struct { unsigned char magic; @@ -88,6 +95,7 @@ typedef struct { int blockedcount; size_t domainpos; } domainsData; +ASSERT_SIZEOF(domainsData, 24, 16, 16); typedef struct { unsigned char magic; @@ -98,6 +106,7 @@ typedef struct { int clientID; int black_regex_idx; } DNSCacheData; +ASSERT_SIZEOF(DNSCacheData, 16, 16, 16); void strtolower(char *str); int findQueryID(const int id); diff --git a/src/edns0.h b/src/edns0.h index 31261c52a..e5eafd299 100644 --- a/src/edns0.h +++ b/src/edns0.h @@ -10,7 +10,8 @@ #ifndef EDNS0_HEADER #define EDNS0_HEADER -#include "edns0.h" +// assert_sizeof +#include "static_assert.h" typedef struct { bool client_set; @@ -19,6 +20,7 @@ typedef struct { char mac_byte[6]; char mac_text[18]; } ednsData; +ASSERT_SIZEOF(ednsData, 72, 72, 72); void FTL_parse_pseudoheaders(struct dns_header *header, size_t n, union mysockaddr *peer, ednsData *edns); diff --git a/src/overTime.h b/src/overTime.h index 2bea5cb17..709419219 100644 --- a/src/overTime.h +++ b/src/overTime.h @@ -14,6 +14,9 @@ // TYPE_MAX #include "datastructure.h" +// assert_sizeof +#include "static_assert.h" + void initOverTime(void); unsigned int getOverTimeID(const time_t timestamp); @@ -34,6 +37,7 @@ typedef struct { time_t timestamp; int querytypedata[TYPE_MAX-1]; } overTimeData; +ASSERT_SIZEOF(overTimeData, 96, 88, 88); extern overTimeData *overTime; diff --git a/src/regex_r.h b/src/regex_r.h index 1b4a4fbc0..45b56303c 100644 --- a/src/regex_r.h +++ b/src/regex_r.h @@ -24,6 +24,9 @@ extern const char *regextype[]; #include #endif +// assert_sizeof +#include "static_assert.h" + typedef struct { bool available; bool inverted; @@ -33,6 +36,7 @@ typedef struct { char *string; regex_t regex; } regexData; +ASSERT_SIZEOF(regexData, 32, 20, 20); unsigned int get_num_regex(const enum regex_type regexid) __attribute__((pure)); int match_regex(const char *input, const DNSCacheData* dns_cache, const int clientID, diff --git a/src/shmem.h b/src/shmem.h index 7d7eb9fe0..601cb9cea 100644 --- a/src/shmem.h +++ b/src/shmem.h @@ -18,17 +18,22 @@ // TYPE_MAX #include "datastructure.h" +// assert_sizeof +#include "static_assert.h" + typedef struct { const char *name; size_t size; void *ptr; } SharedMemory; +ASSERT_SIZEOF(SharedMemory, 24, 12, 12); typedef struct { int version; unsigned int global_shm_counter; unsigned int next_str_pos; } ShmSettings; +ASSERT_SIZEOF(ShmSettings, 12, 12, 12); typedef struct { int queries; diff --git a/src/static_assert.h b/src/static_assert.h new file mode 100644 index 000000000..4df9ecb20 --- /dev/null +++ b/src/static_assert.h @@ -0,0 +1,27 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2021 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* Struct size assertion tool +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ + +#include + + +#define STATIC_ASSERT(OBJECT, EXPECTED) \ + static_assert(sizeof(OBJECT) == EXPECTED , "Expected size of " #OBJECT " is " #EXPECTED " on this architecture."); + +// Check based on detected architecture +#if defined(__x86_64__) || defined(__aarch64__) +#define ASSERT_SIZEOF(OBJECT, SIZE64, SIZE32, SIZEARM) \ + STATIC_ASSERT(OBJECT, SIZE64) +#elif defined(__i386__) +#define ASSERT_SIZEOF(OBJECT, SIZE64, SIZE32, SIZEARM) \ + STATIC_ASSERT(OBJECT, SIZE32) +#elif defined(__arm__) +#define ASSERT_SIZEOF(OBJECT, SIZE64, SIZE32, SIZEARM) \ + STATIC_ASSERT(OBJECT, SIZEARM) +#endif \ No newline at end of file diff --git a/src/vector.h b/src/vector.h index f39aa9875..48a159cec 100644 --- a/src/vector.h +++ b/src/vector.h @@ -18,6 +18,8 @@ #include // type sqlite3_stmt #include "database/sqlite3.h" +// assert_sizeof +#include "static_assert.h" #define VEC_ALLOC_STEP 10u @@ -28,6 +30,7 @@ typedef struct sqlite3_stmt_vec { void (*set)(struct sqlite3_stmt_vec *, unsigned int, sqlite3_stmt*); void (*free)(struct sqlite3_stmt_vec *); } sqlite3_stmt_vec; +ASSERT_SIZEOF(sqlite3_stmt_vec, 40, 20, 20); sqlite3_stmt_vec *new_sqlite3_stmt_vec(unsigned int initial_size); void set_sqlite3_stmt_vec(sqlite3_stmt_vec *v, unsigned int index, sqlite3_stmt* item); From c227fc1598fcf211b0d0c223b7668cdc69210e9e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Jan 2021 14:57:20 +0100 Subject: [PATCH 03/21] Store blocked property in query flags. Signed-off-by: DL6ER --- src/database/query-table.c | 20 +++++++++----------- src/datastructure.h | 2 ++ src/dnsmasq_interface.c | 6 ++++++ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/database/query-table.c b/src/database/query-table.c index 4034703ca..4f450998e 100644 --- a/src/database/query-table.c +++ b/src/database/query-table.c @@ -148,7 +148,7 @@ void DB_save_queries(void) sqlite3_bind_text(stmt, 5, client, -1, SQLITE_STATIC); // FORWARD - if(query->status == QUERY_FORWARDED && query->upstreamID > -1) + if(query->flags.forwarded && query->upstreamID > -1) { // Get forward pointer const upstreamsData* upstream = getUpstream(query->upstreamID, true); @@ -209,15 +209,7 @@ void DB_save_queries(void) // Total counter information (delta computation) total++; - if(query->status == QUERY_GRAVITY || - query->status == QUERY_BLACKLIST || - query->status == QUERY_REGEX || - query->status == QUERY_EXTERNAL_BLOCKED_IP || - query->status == QUERY_EXTERNAL_BLOCKED_NULL || - query->status == QUERY_EXTERNAL_BLOCKED_NXRA || - query->status == QUERY_GRAVITY_CNAME || - query->status == QUERY_REGEX_CNAME || - query->status == QUERY_BLACKLIST_CNAME) + if(query->flags.blocked) blocked++; // Update lasttimestamp variable with timestamp of the latest stored query @@ -453,11 +445,15 @@ void DB_read_queries(void) query->timeidx = timeidx; query->db = dbid; query->id = 0; - query->flags.complete = true; // Mark as all information is available query->response = 0; query->dnssec = DNSSEC_UNSPECIFIED; query->reply = REPLY_UNKNOWN; query->CNAME_domainID = -1; + // Initialize flags + query->flags.complete = true; // Mark as all information is available + query->flags.blocked = false; + query->flags.forwarded = false; + query->flags.whitelisted = false; // Set lastQuery timer for network table clientsData* client = getClient(clientID, true); @@ -523,6 +519,7 @@ void DB_read_queries(void) case QUERY_REGEX_CNAME: // Blocked by regex blacklist (inside CNAME path) case QUERY_BLACKLIST_CNAME: // Blocked by exact blacklist (inside CNAME path) counters->blocked++; + query->flags.blocked = true; // Get domain pointer domainsData* domain = getDomain(domainID, true); domain->blockedcount++; @@ -533,6 +530,7 @@ void DB_read_queries(void) case QUERY_FORWARDED: // Forwarded counters->forwarded++; + query->flags.forwarded = true; // Update overTime data structure overTime[timeidx].forwarded++; break; diff --git a/src/datastructure.h b/src/datastructure.h index 6a2454e98..295267a7f 100644 --- a/src/datastructure.h +++ b/src/datastructure.h @@ -46,6 +46,8 @@ typedef struct { struct query_flags { bool whitelisted :1; bool complete :1; + bool blocked :1; + bool forwarded :1; } flags; } queriesData; diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 228dc98d0..8c99e2d72 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -641,6 +641,10 @@ bool _FTL_new_query(const unsigned int flags, const char *name, // Store DNSSEC result for this domain query->dnssec = DNSSEC_UNSPECIFIED; query->CNAME_domainID = -1; + // This query is not yet known ad forwarded or blocked + query->flags.blocked = false; + query->flags.forwarded = false; + query->flags.whitelisted = false; // Check and apply possible privacy level rules // The currently set privacy level (at the time the query is @@ -910,6 +914,7 @@ void _FTL_forwarded(const unsigned int flags, const char *name, const struct ser // from above as otherwise this check will always // be negative query->status = QUERY_FORWARDED; + query->flags.forwarded = true; // Update overTime data overTime[timeidx].forwarded++; @@ -1442,6 +1447,7 @@ static void query_blocked(queriesData* query, domainsData* domain, clientsData* // Update status query->status = new_status; + query->flags.blocked = true; } void _FTL_dnssec(const int status, const int id, const char* file, const int line) From 471fbf7b629586c6633646a27de8ed7c67b0581b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Jan 2021 14:57:42 +0100 Subject: [PATCH 04/21] Use blocked property in API code. Make query->upstreamID = -1 the new default to differentiate easily what was forwarded (ID will be >= 0) and what not (ID == -1). Store the upstream server also for other query types that were forwarded (like queries blocked during CNAME inspection). Signed-off-by: DL6ER --- src/api/api.c | 45 +++++++++++--------------------------- src/database/query-table.c | 20 +++++------------ src/datastructure.h | 1 - src/dnsmasq_interface.c | 5 +++-- 4 files changed, 22 insertions(+), 49 deletions(-) diff --git a/src/api/api.c b/src/api/api.c index 86a5a0179..a86442aeb 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -891,19 +891,12 @@ void getAllQueries(const char *client_message, const int *sock) if(query->status == QUERY_UNKNOWN && !(showpermitted && showblocked)) continue; - // 1 = gravity.list, 4 = wildcard, 5 = black.list - if((query->status == QUERY_GRAVITY || - query->status == QUERY_REGEX || - query->status == QUERY_BLACKLIST || - query->status == QUERY_GRAVITY_CNAME || - query->status == QUERY_REGEX_CNAME || - query->status == QUERY_BLACKLIST_CNAME) && !showblocked) + // Skip blocked queries when asked to + if(query->flags.blocked && !showblocked) continue; - // 2 = forwarded, 3 = cached - if((query->status == QUERY_FORWARDED || - query->status == QUERY_CACHE || - query->status == QUERY_RETRIED || - query->status == QUERY_RETRIED_DNSSEC) && !showpermitted) + + // Skip permitted queries when asked to + if(!query->flags.blocked && !showpermitted) continue; // Skip those entries which so not meet the requested timeframe @@ -921,10 +914,7 @@ void getAllQueries(const char *client_message, const int *sock) // If the domain of this query did not match, the CNAME // domain may still match - we have to check it in // addition if this query is of CNAME blocked type - else if((query->status == QUERY_GRAVITY_CNAME || - query->status == QUERY_BLACKLIST_CNAME || - query->status == QUERY_REGEX_CNAME) && - query->CNAME_domainID == domainid) + else if(query->CNAME_domainID > -1) { // Get this query } @@ -959,13 +949,8 @@ void getAllQueries(const char *client_message, const int *sock) if(filterforwarddest) { - // Does the user want to see queries answered from blocking lists? - if(forwarddestid == -2 && query->status != QUERY_GRAVITY - && query->status != QUERY_REGEX - && query->status != QUERY_BLACKLIST - && query->status != QUERY_GRAVITY_CNAME - && query->status != QUERY_REGEX_CNAME - && query->status != QUERY_BLACKLIST_CNAME) + // Skip if not from the virtual blocking "upstream" server + if(forwarddestid == -2 && !query->flags.blocked) continue; // Does the user want to see queries answered from local cache? else if(forwarddestid == -1 && query->status != QUERY_CACHE) @@ -1017,7 +1002,7 @@ void getAllQueries(const char *client_message, const int *sock) // Get IP of upstream destination, if applicable in_port_t upstream_port = 0; const char *upstream_name = "N/A"; - if(query->status == QUERY_FORWARDED) + if(query->upstreamID > -1) { const upstreamsData *upstream = getUpstream(query->upstreamID, true); if(upstream != NULL) @@ -1104,15 +1089,8 @@ void getRecentBlocked(const char *client_message, const int *sock) if(query == NULL) continue; - if(query->status == QUERY_GRAVITY || - query->status == QUERY_REGEX || - query->status == QUERY_BLACKLIST || - query->status == QUERY_GRAVITY_CNAME || - query->status == QUERY_REGEX_CNAME || - query->status == QUERY_BLACKLIST_CNAME) + if(query->flags.blocked) { - found++; - // Ask subroutine for domain. It may return "hidden" depending on // the privacy settings at the time the query was made const char *domain = getDomainString(query); @@ -1123,6 +1101,9 @@ void getRecentBlocked(const char *client_message, const int *sock) ssend(*sock,"%s\n", domain); else if(!pack_str32(*sock, domain)) return; + + // Only count when sent succesfully + found++; } if(found >= num) diff --git a/src/database/query-table.c b/src/database/query-table.c index 4f450998e..c15cf23c7 100644 --- a/src/database/query-table.c +++ b/src/database/query-table.c @@ -148,7 +148,7 @@ void DB_save_queries(void) sqlite3_bind_text(stmt, 5, client, -1, SQLITE_STATIC); // FORWARD - if(query->flags.forwarded && query->upstreamID > -1) + if(query->upstreamID > -1) { // Get forward pointer const upstreamsData* upstream = getUpstream(query->upstreamID, true); @@ -387,19 +387,13 @@ void DB_read_queries(void) continue; } - const char *upstream = (const char *)sqlite3_column_text(stmt, 6); - int upstreamID = 0; + const char *upstream = NULL; + int upstreamID = -1; // Default if not forwarded // Determine upstreamID only when status == 2 (forwarded) as the // field need not to be filled for other query status types - if(status == QUERY_FORWARDED) + if(sqlite3_column_bytes(stmt, 6) > 0 && + (upstream = (const char *)sqlite3_column_text(stmt, 6)) != NULL) { - if(upstream == NULL) - { - logg("WARN (during database import): FORWARD should not be NULL with status QUERY_FORWARDED (timestamp: %lli), skipping entry", - (long long)queryTimeStamp); - continue; - } - // Get IP address and port of upstream destination char serv_addr[INET6_ADDRSTRLEN] = { 0 }; unsigned int serv_port = 53; @@ -452,7 +446,6 @@ void DB_read_queries(void) // Initialize flags query->flags.complete = true; // Mark as all information is available query->flags.blocked = false; - query->flags.forwarded = false; query->flags.whitelisted = false; // Set lastQuery timer for network table @@ -479,7 +472,7 @@ void DB_read_queries(void) status == QUERY_REGEX_CNAME || status == QUERY_BLACKLIST_CNAME) { - // QUERY_*_CNAME: Getdomain causing the blocking + // QUERY_*_CNAME: Get domain causing the blocking const char *CNAMEdomain = (const char *)sqlite3_column_text(stmt, 7); if(CNAMEdomain != NULL && strlen(CNAMEdomain) > 0) { @@ -530,7 +523,6 @@ void DB_read_queries(void) case QUERY_FORWARDED: // Forwarded counters->forwarded++; - query->flags.forwarded = true; // Update overTime data structure overTime[timeidx].forwarded++; break; diff --git a/src/datastructure.h b/src/datastructure.h index 295267a7f..e8e77f68a 100644 --- a/src/datastructure.h +++ b/src/datastructure.h @@ -47,7 +47,6 @@ typedef struct { bool whitelisted :1; bool complete :1; bool blocked :1; - bool forwarded :1; } flags; } queriesData; diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 8c99e2d72..f4faac65b 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -643,9 +643,11 @@ bool _FTL_new_query(const unsigned int flags, const char *name, query->CNAME_domainID = -1; // This query is not yet known ad forwarded or blocked query->flags.blocked = false; - query->flags.forwarded = false; query->flags.whitelisted = false; + // Indicator that this query was not forwarded so far + query->upstreamID = -1; + // Check and apply possible privacy level rules // The currently set privacy level (at the time the query is // generated) is stored in the queries structure @@ -914,7 +916,6 @@ void _FTL_forwarded(const unsigned int flags, const char *name, const struct ser // from above as otherwise this check will always // be negative query->status = QUERY_FORWARDED; - query->flags.forwarded = true; // Update overTime data overTime[timeidx].forwarded++; From ca36f2de8d656e48bee7f80978e391e161a66674 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 18 Jan 2021 19:40:24 +0100 Subject: [PATCH 05/21] Add MAXDBDAYS=-1 to disable auto-cleaning and ensure overflow cannot happen (we just enforce the maximum in this case) Signed-off-by: DL6ER --- src/config.c | 15 ++++++++++++++- src/database/database-thread.c | 2 +- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/config.c b/src/config.c index fcb5c1b74..713600cc8 100644 --- a/src/config.c +++ b/src/config.c @@ -17,6 +17,9 @@ // saveport() #include "api/socket.h" +// INT_MAX +#include + ConfigStruct config; FTLFileNamesStruct FTLfiles = { // Default path for config file (regular installations) @@ -132,12 +135,22 @@ void read_FTLconf(void) buffer = parse_FTLconf(fp, "MAXDBDAYS"); int value = 0; + const int maxdbdays_max = INT_MAX / 24 / 60 / 60; if(buffer != NULL && sscanf(buffer, "%i", &value)) - if(value >= 0) + { + // Prevent possible overflow + if(value > maxdbdays_max) + value = maxdbdays_max; + + // Only use valid values + if(value == -1 || value >= 0) config.maxDBdays = value; + } if(config.maxDBdays == 0) logg(" MAXDBDAYS: --- (DB disabled)"); + else if(config.maxDBdays == -1) + logg(" MAXDBDAYS: --- (cleaning disabled)"); else logg(" MAXDBDAYS: max age for stored queries is %i days", config.maxDBdays); diff --git a/src/database/database-thread.c b/src/database/database-thread.c index 320ec3334..bfb84509a 100644 --- a/src/database/database-thread.c +++ b/src/database/database-thread.c @@ -54,7 +54,7 @@ void *DB_thread(void *val) unlock_shm(); // Check if GC should be done on the database - if(DBdeleteoldqueries) + if(DBdeleteoldqueries && config.maxDBdays != -1) { // No thread locks needed delete_old_queries_in_DB(); From 76b610ee39930348c799d34e30eab9960438f3ea Mon Sep 17 00:00:00 2001 From: Miao Wang Date: Fri, 4 Dec 2020 09:59:37 +0800 Subject: [PATCH 06/21] pxe: support pxe clients with custom vendor-class According to UEFI[1] and PXE[2] specs, PXE clients are required to have `PXEClient` identfier in the vendor-class field of DHCP requests, and PXE servers should also include that identifier in their responses. However, the firmware of servers from a few vendors[3] are customized to include a different identifier. This patch adds an option named `dhcp-pxe-vendor` to provide a list of such identifiers. The identifier used in responses sent from dnsmasq is identical to that in the coresponding request. [1]: https://uefi.org/sites/default/files/resources/UEFI%20Spec%202.8B%20May%202020.pdf [2]: http://www.pix.net/software/pxeboot/archive/pxespec.pdf [3]: For instance, TaiShan servers from Huawei, which are Arm64-based, send `HW-Client` in PXE requests up to now. Signed-off-by: Miao Wang Signed-off-by: DL6ER --- src/dnsmasq/dnsmasq.h | 9 ++++ src/dnsmasq/option.c | 27 +++++++++++- src/dnsmasq/rfc2131.c | 97 +++++++++++++++++++++++++++++++------------ 3 files changed, 105 insertions(+), 28 deletions(-) diff --git a/src/dnsmasq/dnsmasq.h b/src/dnsmasq/dnsmasq.h index 24ecb5689..71a9137b2 100644 --- a/src/dnsmasq/dnsmasq.h +++ b/src/dnsmasq/dnsmasq.h @@ -831,6 +831,7 @@ struct dhcp_opt { #define DHOPT_RFC3925 2048 #define DHOPT_TAGOK 4096 #define DHOPT_ADDR6 8192 +#define DHOPT_VENDOR_PXE 16384 struct dhcp_boot { char *file, *sname, *tftp_sname; @@ -854,6 +855,8 @@ struct pxe_service { struct pxe_service *next; }; +#define DHCP_PXE_DEF_VENDOR "PXEClient" + #define MATCH_VENDOR 1 #define MATCH_USER 2 #define MATCH_CIRCUIT 3 @@ -869,6 +872,11 @@ struct dhcp_vendor { struct dhcp_vendor *next; }; +struct dhcp_pxe_vendor { + char *data; + struct dhcp_pxe_vendor *next; +}; + struct dhcp_mac { unsigned int mask; int hwaddr_len, hwaddr_type; @@ -1042,6 +1050,7 @@ extern struct daemon { struct dhcp_config *dhcp_conf; struct dhcp_opt *dhcp_opts, *dhcp_match, *dhcp_opts6, *dhcp_match6; struct dhcp_match_name *dhcp_name_match; + struct dhcp_pxe_vendor *dhcp_pxe_vendors; struct dhcp_vendor *dhcp_vendors; struct dhcp_mac *dhcp_macs; struct dhcp_boot *boot_config; diff --git a/src/dnsmasq/option.c b/src/dnsmasq/option.c index d0e1add2f..2e61f4fb6 100644 --- a/src/dnsmasq/option.c +++ b/src/dnsmasq/option.c @@ -171,6 +171,7 @@ struct myoption { #define LOPT_IGNORE_CLID 358 #define LOPT_SINGLE_PORT 359 #define LOPT_SCRIPT_TIME 360 +#define LOPT_PXE_VENDOR 361 #ifdef HAVE_GETOPT_LONG static const struct option opts[] = @@ -274,6 +275,7 @@ static const struct myoption opts[] = { "dhcp-circuitid", 1, 0, LOPT_CIRCUIT }, { "dhcp-remoteid", 1, 0, LOPT_REMOTE }, { "dhcp-subscrid", 1, 0, LOPT_SUBSCR }, + { "dhcp-pxe-vendor", 1, 0, LOPT_PXE_VENDOR }, { "interface-name", 1, 0, LOPT_INTNAME }, { "dhcp-hostsfile", 1, 0, LOPT_DHCP_HOST }, { "dhcp-optsfile", 1, 0, LOPT_DHCP_OPTS }, @@ -387,6 +389,7 @@ static struct { { LOPT_CIRCUIT, ARG_DUP, "set:,", gettext_noop("Map RFC3046 circuit-id to tag."), NULL }, { LOPT_REMOTE, ARG_DUP, "set:,", gettext_noop("Map RFC3046 remote-id to tag."), NULL }, { LOPT_SUBSCR, ARG_DUP, "set:,", gettext_noop("Map RFC3993 subscriber-id to tag."), NULL }, + { LOPT_PXE_VENDOR, ARG_DUP, "[,...]", gettext_noop("Specify vendor class to match for PXE requests."), NULL }, { 'J', ARG_DUP, "tag:...", gettext_noop("Don't do DHCP for hosts with tag set."), NULL }, { LOPT_BROADCAST, ARG_DUP, "[=tag:...]", gettext_noop("Force broadcast replies for hosts with tag set."), NULL }, { 'k', OPT_NO_FORK, NULL, gettext_noop("Do NOT fork into the background, do NOT run in debug mode."), NULL }, @@ -3676,8 +3679,8 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma new->val = opt_malloc(new->len); memcpy(new->val + 1, arg, new->len - 1); - new->u.vendor_class = (unsigned char *)"PXEClient"; - new->flags = DHOPT_VENDOR; + new->u.vendor_class = NULL; + new->flags = DHOPT_VENDOR | DHOPT_VENDOR_PXE; if (comma && atoi_check(comma, &timeout)) *(new->val) = timeout; @@ -3939,6 +3942,19 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma new->next = daemon->override_relays; daemon->override_relays = new; arg = comma; + } + break; + + case LOPT_PXE_VENDOR: /* --dhcp-pxe-vendor */ + { + while (arg) { + struct dhcp_pxe_vendor *new = opt_malloc(sizeof(struct dhcp_pxe_vendor)); + comma = split(arg); + new->data = opt_string_alloc(arg); + new->next = daemon->dhcp_pxe_vendors; + daemon->dhcp_pxe_vendors = new; + arg = comma; + } } break; @@ -5222,6 +5238,13 @@ void read_opts(int argc, char **argv, char *compile_opts) strcat(buff, daemon->authserver); daemon->hostmaster = opt_string_alloc(buff); } + + if (!daemon->dhcp_pxe_vendors) + { + daemon->dhcp_pxe_vendors = opt_malloc(sizeof(struct dhcp_pxe_vendor)); + daemon->dhcp_pxe_vendors->data = opt_string_alloc(DHCP_PXE_DEF_VENDOR); + daemon->dhcp_pxe_vendors->next = NULL; + } /* only one of these need be specified: the other defaults to the host-name */ if (option_bool(OPT_LOCALMX) || daemon->mxnames || daemon->mxtarget) diff --git a/src/dnsmasq/rfc2131.c b/src/dnsmasq/rfc2131.c index fc54aab4b..d6780685e 100644 --- a/src/dnsmasq/rfc2131.c +++ b/src/dnsmasq/rfc2131.c @@ -30,7 +30,7 @@ static struct in_addr server_id(struct dhcp_context *context, struct in_addr ove static unsigned int calc_time(struct dhcp_context *context, struct dhcp_config *config, unsigned char *opt); static void option_put(struct dhcp_packet *mess, unsigned char *end, int opt, int len, unsigned int val); static void option_put_string(struct dhcp_packet *mess, unsigned char *end, - int opt, char *string, int null_term); + int opt, const char *string, int null_term); static struct in_addr option_addr(unsigned char *opt); static unsigned int option_uint(unsigned char *opt, int offset, int size); static void log_packet(char *type, void *addr, unsigned char *ext_mac, @@ -54,17 +54,19 @@ static void do_options(struct dhcp_context *context, int vendor_class_len, time_t now, unsigned int lease_time, - unsigned short fuzz); + unsigned short fuzz, + const char *pxevendor); static void match_vendor_opts(unsigned char *opt, struct dhcp_opt *dopt); static int do_encap_opts(struct dhcp_opt *opt, int encap, int flag, struct dhcp_packet *mess, unsigned char *end, int null_term); -static void pxe_misc(struct dhcp_packet *mess, unsigned char *end, unsigned char *uuid); +static void pxe_misc(struct dhcp_packet *mess, unsigned char *end, unsigned char *uuid, const char *pxevendor); static int prune_vendor_opts(struct dhcp_netid *netid); static struct dhcp_opt *pxe_opts(int pxe_arch, struct dhcp_netid *netid, struct in_addr local, time_t now); struct dhcp_boot *find_boot(struct dhcp_netid *netid); static int pxe_uefi_workaround(int pxe_arch, struct dhcp_netid *netid, struct dhcp_packet *mess, struct in_addr local, time_t now, int pxe); static void apply_delay(u32 xid, time_t recvtime, struct dhcp_netid *netid); +static int is_pxe_client(struct dhcp_packet *mess, size_t sz, const char **pxe_vendor); size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, size_t sz, time_t now, int unicast_dest, int loopback, @@ -76,6 +78,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, struct dhcp_mac *mac; struct dhcp_netid_list *id_list; int clid_len = 0, ignore = 0, do_classes = 0, rapid_commit = 0, selecting = 0, pxearch = -1; + const char *pxevendor = NULL; struct dhcp_packet *mess = (struct dhcp_packet *)daemon->dhcp_packet.iov_base; unsigned char *end = (unsigned char *)(mess + 1); unsigned char *real_end = (unsigned char *)(mess + 1); @@ -647,7 +650,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, clear_packet(mess, end); do_options(context, mess, end, NULL, hostname, get_domain(mess->yiaddr), - netid, subnet_addr, 0, 0, -1, NULL, vendor_class_len, now, 0xffffffff, 0); + netid, subnet_addr, 0, 0, -1, NULL, vendor_class_len, now, 0xffffffff, 0, NULL); } } @@ -835,9 +838,8 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, clid = NULL; /* Check if client is PXE client. */ - if (daemon->enable_pxe && - (opt = option_find(mess, sz, OPTION_VENDOR_ID, 9)) && - strncmp(option_ptr(opt, 0), "PXEClient", 9) == 0) + if (daemon->enable_pxe && + is_pxe_client(mess, sz, &pxevendor)) { if ((opt = option_find(mess, sz, OPTION_PXE_UUID, 17))) { @@ -899,7 +901,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, option_put(mess, end, OPTION_MESSAGE_TYPE, 1, DHCPACK); option_put(mess, end, OPTION_SERVER_IDENTIFIER, INADDRSZ, htonl(context->local.s_addr)); - pxe_misc(mess, end, uuid); + pxe_misc(mess, end, uuid, pxevendor); prune_vendor_opts(tagif_netid); opt71.val = save71; @@ -979,7 +981,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, option_put(mess, end, OPTION_MESSAGE_TYPE, 1, mess_type == DHCPDISCOVER ? DHCPOFFER : DHCPACK); option_put(mess, end, OPTION_SERVER_IDENTIFIER, INADDRSZ, htonl(tmp->local.s_addr)); - pxe_misc(mess, end, uuid); + pxe_misc(mess, end, uuid, pxevendor); prune_vendor_opts(tagif_netid); if ((pxe && !workaround) || !redirect4011) do_encap_opts(pxe_opts(pxearch, tagif_netid, tmp->local, now), OPTION_VENDOR_CLASS_OPT, DHOPT_VENDOR_MATCH, mess, end, 0); @@ -1150,7 +1152,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, option_put(mess, end, OPTION_LEASE_TIME, 4, time); /* T1 and T2 are required in DHCPOFFER by HP's wacky Jetdirect client. */ do_options(context, mess, end, req_options, offer_hostname, get_domain(mess->yiaddr), - netid, subnet_addr, fqdn_flags, borken_opt, pxearch, uuid, vendor_class_len, now, time, fuzz); + netid, subnet_addr, fqdn_flags, borken_opt, pxearch, uuid, vendor_class_len, now, time, fuzz, pxevendor); return dhcp_packet_size(mess, agent_id, real_end); @@ -1499,7 +1501,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, if (rapid_commit) option_put(mess, end, OPTION_RAPID_COMMIT, 0, 0); do_options(context, mess, end, req_options, hostname, get_domain(mess->yiaddr), - netid, subnet_addr, fqdn_flags, borken_opt, pxearch, uuid, vendor_class_len, now, time, fuzz); + netid, subnet_addr, fqdn_flags, borken_opt, pxearch, uuid, vendor_class_len, now, time, fuzz, pxevendor); } return dhcp_packet_size(mess, agent_id, real_end); @@ -1566,7 +1568,7 @@ size_t dhcp_reply(struct dhcp_context *context, char *iface_name, int int_index, } do_options(context, mess, end, req_options, hostname, get_domain(mess->ciaddr), - netid, subnet_addr, fqdn_flags, borken_opt, pxearch, uuid, vendor_class_len, now, 0xffffffff, 0); + netid, subnet_addr, fqdn_flags, borken_opt, pxearch, uuid, vendor_class_len, now, 0xffffffff, 0, pxevendor); *is_inform = 1; /* handle reply differently */ return dhcp_packet_size(mess, agent_id, real_end); @@ -1948,7 +1950,7 @@ static void option_put(struct dhcp_packet *mess, unsigned char *end, int opt, in } static void option_put_string(struct dhcp_packet *mess, unsigned char *end, int opt, - char *string, int null_term) + const char *string, int null_term) { unsigned char *p; size_t len = strlen(string); @@ -2026,15 +2028,32 @@ static void match_vendor_opts(unsigned char *opt, struct dhcp_opt *dopt) dopt->flags &= ~DHOPT_VENDOR_MATCH; if (opt && (dopt->flags & DHOPT_VENDOR)) { - int i, len = 0; - if (dopt->u.vendor_class) - len = strlen((char *)dopt->u.vendor_class); - for (i = 0; i <= (option_len(opt) - len); i++) - if (len == 0 || memcmp(dopt->u.vendor_class, option_ptr(opt, i), len) == 0) - { - dopt->flags |= DHOPT_VENDOR_MATCH; - break; - } + const struct dhcp_pxe_vendor *pv; + struct dhcp_pxe_vendor dummy_vendor = { + .data = (char *)dopt->u.vendor_class, + .next = NULL, + }; + if (dopt->flags & DHOPT_VENDOR_PXE) + pv = daemon->dhcp_pxe_vendors; + else + pv = &dummy_vendor; + for (; pv; pv = pv->next) + { + int i, len = 0, matched = 0; + if (pv->data) + len = strlen(pv->data); + for (i = 0; i <= (option_len(opt) - len); i++) + if (len == 0 || memcmp(pv->data, option_ptr(opt, i), len) == 0) + { + matched = 1; + break; + } + if (matched) + { + dopt->flags |= DHOPT_VENDOR_MATCH; + break; + } + } } } } @@ -2087,11 +2106,13 @@ static int do_encap_opts(struct dhcp_opt *opt, int encap, int flag, return ret; } -static void pxe_misc(struct dhcp_packet *mess, unsigned char *end, unsigned char *uuid) +static void pxe_misc(struct dhcp_packet *mess, unsigned char *end, unsigned char *uuid, const char *pxevendor) { unsigned char *p; - option_put_string(mess, end, OPTION_VENDOR_ID, "PXEClient", 0); + if (!pxevendor) + pxevendor="PXEClient"; + option_put_string(mess, end, OPTION_VENDOR_ID, pxevendor, 0); if (uuid && (p = free_space(mess, end, OPTION_PXE_UUID, 17))) memcpy(p, uuid, 17); } @@ -2308,6 +2329,29 @@ struct dhcp_boot *find_boot(struct dhcp_netid *netid) return boot; } +static int is_pxe_client(struct dhcp_packet *mess, size_t sz, const char **pxe_vendor) +{ + const unsigned char *opt = NULL; + ssize_t conf_len = 0; + const struct dhcp_pxe_vendor *conf = daemon->dhcp_pxe_vendors; + opt = option_find(mess, sz, OPTION_VENDOR_ID, 0); + if (!opt) + return 0; + for (; conf; conf = conf->next) + { + conf_len = strlen(conf->data); + if (option_len(opt) < conf_len) + continue; + if (strncmp(option_ptr(opt, 0), conf->data, conf_len) == 0) + { + if (pxe_vendor) + *pxe_vendor = conf->data; + return 1; + } + } + return 0; +} + static void do_options(struct dhcp_context *context, struct dhcp_packet *mess, unsigned char *end, @@ -2322,7 +2366,8 @@ static void do_options(struct dhcp_context *context, int vendor_class_len, time_t now, unsigned int lease_time, - unsigned short fuzz) + unsigned short fuzz, + const char *pxevendor) { struct dhcp_opt *opt, *config_opts = daemon->dhcp_opts; struct dhcp_boot *boot; @@ -2696,7 +2741,7 @@ static void do_options(struct dhcp_context *context, if (context && pxe_arch != -1) { - pxe_misc(mess, end, uuid); + pxe_misc(mess, end, uuid, pxevendor); if (!pxe_uefi_workaround(pxe_arch, tagif, mess, context->local, now, 0)) config_opts = pxe_opts(pxe_arch, tagif, context->local, now); } From 5deb35b24fe8adf597d1e9d85debd601d326d9dd Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sat, 12 Dec 2020 23:26:45 +0000 Subject: [PATCH 07/21] Use the values of --min-port and --max-port in TCP connections. Rather that letting the kernel pick source ports, do it ourselves so that the --min-port and --max-port parameters are be obeyed. Signed-off-by: DL6ER --- src/dnsmasq/network.c | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/dnsmasq/network.c b/src/dnsmasq/network.c index b0f790110..323d446c7 100644 --- a/src/dnsmasq/network.c +++ b/src/dnsmasq/network.c @@ -1263,17 +1263,46 @@ int random_sock(int family) int local_bind(int fd, union mysockaddr *addr, char *intname, unsigned int ifindex, int is_tcp) { union mysockaddr addr_copy = *addr; + unsigned short port; + int tries = 1, done = 0; + unsigned int ports_avail = ((unsigned short)daemon->max_port - (unsigned short)daemon->min_port) + 1; + + if (addr_copy.sa.sa_family == AF_INET) + port = addr_copy.in.sin_port; + else + port = addr_copy.in6.sin6_port; /* cannot set source _port_ for TCP connections. */ if (is_tcp) + port = 0; + + /* Bind a random port within the range given by min-port and max-port */ + if (port == 0) + { + tries = ports_avail < 30 ? 3 * ports_avail : 100; + port = htons(daemon->min_port + (rand16() % ((unsigned short)ports_avail))); + } + + while (tries--) { if (addr_copy.sa.sa_family == AF_INET) - addr_copy.in.sin_port = 0; + addr_copy.in.sin_port = port; else - addr_copy.in6.sin6_port = 0; + addr_copy.in6.sin6_port = port; + + if (bind(fd, (struct sockaddr *)&addr_copy, sa_len(&addr_copy)) != -1) + { + done = 1; + break; + } + + if (errno != EADDRINUSE && errno != EACCES) + return 0; + + port = htons(daemon->min_port + (rand16() % ((unsigned short)ports_avail))); } - - if (bind(fd, (struct sockaddr *)&addr_copy, sa_len(&addr_copy)) == -1) + + if (!done) return 0; if (!is_tcp && ifindex > 0) From e68acd8e6c5e7973f665d7f0e98f81b7624daba5 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Wed, 11 Nov 2020 23:25:04 +0000 Subject: [PATCH 08/21] Fix remote buffer overflow CERT VU#434904 The problem is in the sort_rrset() function and allows a remote attacker to overwrite memory. Any dnsmasq instance with DNSSEC enabled is vulnerable. Signed-off-by: DL6ER --- src/dnsmasq/dnssec.c | 273 ++++++++++++++++++++++++------------------- 1 file changed, 152 insertions(+), 121 deletions(-) diff --git a/src/dnsmasq/dnssec.c b/src/dnsmasq/dnssec.c index db5c2d11c..e95aa3442 100644 --- a/src/dnsmasq/dnssec.c +++ b/src/dnsmasq/dnssec.c @@ -223,138 +223,147 @@ static int check_date_range(unsigned long curtime, u32 date_start, u32 date_end) && serial_compare_32(curtime, date_end) == SERIAL_LT; } -/* Return bytes of canonicalised rdata, when the return value is zero, the remaining - data, pointed to by *p, should be used raw. */ -static int get_rdata(struct dns_header *header, size_t plen, unsigned char *end, char *buff, int bufflen, - unsigned char **p, u16 **desc) +/* Return bytes of canonicalised rrdata one by one. + Init state->ip with the RR, and state->end with the end of same. + Init state->op to NULL. + Init state->desc to RR descriptor. + Init state->buff with a MAXDNAME * 2 buffer. + + After each call which returns 1, state->op points to the next byte of data. + On returning 0, the end has been reached. +*/ +struct rdata_state { + u16 *desc; + size_t c; + unsigned char *end, *ip, *op; + char *buff; +}; + +static int get_rdata(struct dns_header *header, size_t plen, struct rdata_state *state) { - int d = **desc; + int d; - /* No more data needs mangling */ - if (d == (u16)-1) + if (state->op && state->c != 1) { - /* If there's more data than we have space for, just return what fits, - we'll get called again for more chunks */ - if (end - *p > bufflen) - { - memcpy(buff, *p, bufflen); - *p += bufflen; - return bufflen; - } - - return 0; + state->op++; + state->c--; + return 1; } - - (*desc)++; - - if (d == 0 && extract_name(header, plen, p, buff, 1, 0)) - /* domain-name, canonicalise */ - return to_wire(buff); - else - { - /* plain data preceding a domain-name, don't run off the end of the data */ - if ((end - *p) < d) - d = end - *p; + + while (1) + { + d = *(state->desc); - if (d != 0) + if (d == (u16)-1) { - memcpy(buff, *p, d); - *p += d; + /* all the bytes to the end. */ + if ((state->c = state->end - state->ip) != 0) + { + state->op = state->ip; + state->ip = state->end;; + } + else + return 0; + } + else + { + state->desc++; + + if (d == (u16)0) + { + /* domain-name, canonicalise */ + int len; + + if (!extract_name(header, plen, &state->ip, state->buff, 1, 0) || + (len = to_wire(state->buff)) == 0) + continue; + + state->c = len; + state->op = (unsigned char *)state->buff; + } + else + { + /* plain data preceding a domain-name, don't run off the end of the data */ + if ((state->end - state->ip) < d) + d = state->end - state->ip; + + if (d == 0) + continue; + + state->op = state->ip; + state->c = d; + state->ip += d; + } } - return d; + return 1; } } -/* Bubble sort the RRset into the canonical order. - Note that the byte-streams from two RRs may get unsynced: consider - RRs which have two domain-names at the start and then other data. - The domain-names may have different lengths in each RR, but sort equal - - ------------ - |abcde|fghi| - ------------ - |abcd|efghi| - ------------ - - leaving the following bytes as deciding the order. Hence the nasty left1 and left2 variables. -*/ +/* Bubble sort the RRset into the canonical order. */ static int sort_rrset(struct dns_header *header, size_t plen, u16 *rr_desc, int rrsetidx, unsigned char **rrset, char *buff1, char *buff2) { - int swap, quit, i, j; + int swap, i, j; do { for (swap = 0, i = 0; i < rrsetidx-1; i++) { - int rdlen1, rdlen2, left1, left2, len1, len2, len, rc; - u16 *dp1, *dp2; - unsigned char *end1, *end2; + int rdlen1, rdlen2; + struct rdata_state state1, state2; + /* Note that these have been determined to be OK previously, so we don't need to check for NULL return here. */ - unsigned char *p1 = skip_name(rrset[i], header, plen, 10); - unsigned char *p2 = skip_name(rrset[i+1], header, plen, 10); - - p1 += 8; /* skip class, type, ttl */ - GETSHORT(rdlen1, p1); - end1 = p1 + rdlen1; - - p2 += 8; /* skip class, type, ttl */ - GETSHORT(rdlen2, p2); - end2 = p2 + rdlen2; + state1.ip = skip_name(rrset[i], header, plen, 10); + state2.ip = skip_name(rrset[i+1], header, plen, 10); + state1.op = state2.op = NULL; + state1.buff = buff1; + state2.buff = buff2; + state1.desc = state2.desc = rr_desc; - dp1 = dp2 = rr_desc; + state1.ip += 8; /* skip class, type, ttl */ + GETSHORT(rdlen1, state1.ip); + if (!CHECK_LEN(header, state1.ip, plen, rdlen1)) + return rrsetidx; /* short packet */ + state1.end = state1.ip + rdlen1; - for (quit = 0, left1 = 0, left2 = 0, len1 = 0, len2 = 0; !quit;) + state2.ip += 8; /* skip class, type, ttl */ + GETSHORT(rdlen2, state2.ip); + if (!CHECK_LEN(header, state2.ip, plen, rdlen2)) + return rrsetidx; /* short packet */ + state2.end = state2.ip + rdlen2; + + while (1) { - if (left1 != 0) - memmove(buff1, buff1 + len1 - left1, left1); + int ok1, ok2; - if ((len1 = get_rdata(header, plen, end1, buff1 + left1, (MAXDNAME * 2) - left1, &p1, &dp1)) == 0) - { - quit = 1; - len1 = end1 - p1; - memcpy(buff1 + left1, p1, len1); - } - len1 += left1; - - if (left2 != 0) - memmove(buff2, buff2 + len2 - left2, left2); - - if ((len2 = get_rdata(header, plen, end2, buff2 + left2, (MAXDNAME *2) - left2, &p2, &dp2)) == 0) - { - quit = 1; - len2 = end2 - p2; - memcpy(buff2 + left2, p2, len2); - } - len2 += left2; - - if (len1 > len2) - left1 = len1 - len2, left2 = 0, len = len2; - else - left2 = len2 - len1, left1 = 0, len = len1; - - rc = (len == 0) ? 0 : memcmp(buff1, buff2, len); - - if (rc > 0 || (rc == 0 && quit && len1 > len2)) - { - unsigned char *tmp = rrset[i+1]; - rrset[i+1] = rrset[i]; - rrset[i] = tmp; - swap = quit = 1; - } - else if (rc == 0 && quit && len1 == len2) + ok1 = get_rdata(header, plen, &state1); + ok2 = get_rdata(header, plen, &state2); + + if (!ok1 && !ok2) { /* Two RRs are equal, remove one copy. RFC 4034, para 6.3 */ for (j = i+1; j < rrsetidx-1; j++) rrset[j] = rrset[j+1]; rrsetidx--; i--; + break; + } + else if (ok1 && (!ok2 || *state1.op > *state2.op)) + { + unsigned char *tmp = rrset[i+1]; + rrset[i+1] = rrset[i]; + rrset[i] = tmp; + swap = 1; + break; } - else if (rc < 0) - quit = 1; + else if (ok2 && (!ok1 || *state2.op > *state1.op)) + break; + + /* arrive here when bytes are equal, go round the loop again + and compare the next ones. */ } } } while (swap); @@ -569,15 +578,18 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in wire_len = to_wire(keyname); hash->update(ctx, (unsigned int)wire_len, (unsigned char*)keyname); from_wire(keyname); + +#define RRBUFLEN 300 /* Most RRs are smaller than this. */ for (i = 0; i < rrsetidx; ++i) { - int seg; - unsigned char *end, *cp; - u16 len, *dp; + int j; + struct rdata_state state; + u16 len; + unsigned char rrbuf[RRBUFLEN]; p = rrset[i]; - + if (!extract_name(header, plen, &p, name, 1, 10)) return STAT_BOGUS; @@ -586,12 +598,11 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in /* if more labels than in RRsig name, hash *. 4035 5.3.2 */ if (labels < name_labels) { - int k; - for (k = name_labels - labels; k != 0; k--) + for (j = name_labels - labels; j != 0; j--) { while (*name_start != '.' && *name_start != 0) name_start++; - if (k != 1 && *name_start == '.') + if (j != 1 && *name_start == '.') name_start++; } @@ -612,24 +623,44 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in if (!CHECK_LEN(header, p, plen, rdlen)) return STAT_BOGUS; - end = p + rdlen; + /* canonicalise rdata and calculate length of same, use + name buffer as workspace for get_rdata. */ + state.ip = p; + state.op = NULL; + state.desc = rr_desc; + state.buff = name; + state.end = p + rdlen; - /* canonicalise rdata and calculate length of same, use name buffer as workspace. - Note that name buffer is twice MAXDNAME long in DNSSEC mode. */ - cp = p; - dp = rr_desc; - for (len = 0; (seg = get_rdata(header, plen, end, name, MAXDNAME * 2, &cp, &dp)) != 0; len += seg); - len += end - cp; - len = htons(len); + for (j = 0; get_rdata(header, plen, &state); j++) + if (j < RRBUFLEN) + rrbuf[j] = *state.op; + + len = htons((u16)j); hash->update(ctx, 2, (unsigned char *)&len); + + /* If the RR is shorter than RRBUFLEN (most of them, in practice) + then we can just digest it now. If it exceeds RRBUFLEN we have to + go back to the start and do it in chunks. */ + if (j >= RRBUFLEN) + { + state.ip = p; + state.op = NULL; + state.desc = rr_desc; + + for (j = 0; get_rdata(header, plen, &state); j++) + { + rrbuf[j] = *state.op; + + if (j == RRBUFLEN - 1) + { + hash->update(ctx, RRBUFLEN, rrbuf); + j = -1; + } + } + } - /* Now canonicalise again and digest. */ - cp = p; - dp = rr_desc; - while ((seg = get_rdata(header, plen, end, name, MAXDNAME * 2, &cp, &dp))) - hash->update(ctx, seg, (unsigned char *)name); - if (cp != end) - hash->update(ctx, end - cp, cp); + if (j != 0) + hash->update(ctx, j, rrbuf); } hash->digest(ctx, hash->digest_size, digest); From 9002e99cd8a454fb4d4be5108dd5dea439a8e9eb Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Thu, 12 Nov 2020 18:49:23 +0000 Subject: [PATCH 09/21] Check destination of DNS UDP query replies. At any time, dnsmasq will have a set of sockets open, bound to random ports, on which it sends queries to upstream nameservers. This patch fixes the existing problem that a reply for ANY in-flight query would be accepted via ANY open port, which increases the chances of an attacker flooding answers "in the blind" in an attempt to poison the DNS cache. CERT VU#434904 refers. Signed-off-by: DL6ER --- src/dnsmasq/forward.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index 33646911d..a7bb4e60a 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -17,7 +17,7 @@ #include "dnsmasq.h" #include "../dnsmasq_interface.h" -static struct frec *lookup_frec(unsigned short id, void *hash); +static struct frec *lookup_frec(unsigned short id, int fd, int family, void *hash); static struct frec *lookup_frec_by_sender(unsigned short id, union mysockaddr *addr, void *hash); @@ -841,7 +841,7 @@ void reply_query(int fd, int family, time_t now) crc = questions_crc(header, n, daemon->namebuff); #endif - if (!(forward = lookup_frec(ntohs(header->id), hash))) + if (!(forward = lookup_frec(ntohs(header->id), fd, family, hash))) return; #ifdef HAVE_DUMPFILE @@ -2458,14 +2458,25 @@ struct frec *get_new_frec(time_t now, int *wait, struct frec *force) } /* crc is all-ones if not known. */ -static struct frec *lookup_frec(unsigned short id, void *hash) +static struct frec *lookup_frec(unsigned short id, int fd, int family, void *hash) { struct frec *f; for(f = daemon->frec_list; f; f = f->next) if (f->sentto && f->new_id == id && (!hash || memcmp(hash, f->hash, HASH_SIZE) == 0)) - return f; + { + /* sent from random port */ + if (family == AF_INET && f->rfd4 && f->rfd4->fd == fd) + return f; + + if (family == AF_INET6 && f->rfd6 && f->rfd6->fd == fd) + return f; + + /* sent to upstream from bound socket. */ + if (f->sentto->sfd && f->sentto->sfd->fd == fd) + return f; + } return NULL; } @@ -2526,12 +2537,20 @@ void server_gone(struct server *server) static unsigned short get_id(void) { unsigned short ret = 0; + struct frec *f; - do - ret = rand16(); - while (lookup_frec(ret, NULL)); - - return ret; + while (1) + { + ret = rand16(); + + /* ensure id is unique. */ + for (f = daemon->frec_list; f; f = f->next) + if (f->sentto && f->new_id == ret) + break; + + if (!f) + return ret; + } } From afff889e82e922dec8ee5dc53c77229dfdc2c197 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Thu, 12 Nov 2020 22:06:07 +0000 Subject: [PATCH 10/21] Use SHA-256 to provide security against DNS cache poisoning. Use the SHA-256 hash function to verify that DNS answers received are for the questions originally asked. This replaces the slightly insecure SHA-1 (when compiled with DNSSEC) or the very insecure CRC32 (otherwise). Refer: CERT VU#434904. Signed-off-by: DL6ER --- src/dnsmasq/dnsmasq.h | 11 +- src/dnsmasq/dnssec.c | 31 ---- src/dnsmasq/forward.c | 43 +----- src/dnsmasq/hash_questions.c | 281 +++++++++++++++++++++++++++++++++++ src/dnsmasq/rfc1035.c | 49 ------ 5 files changed, 293 insertions(+), 122 deletions(-) create mode 100644 src/dnsmasq/hash_questions.c diff --git a/src/dnsmasq/dnsmasq.h b/src/dnsmasq/dnsmasq.h index 71a9137b2..36946668b 100644 --- a/src/dnsmasq/dnsmasq.h +++ b/src/dnsmasq/dnsmasq.h @@ -657,11 +657,7 @@ struct hostsfile { #define FREC_TEST_PKTSZ 256 #define FREC_HAS_EXTRADATA 512 -#ifdef HAVE_DNSSEC -#define HASH_SIZE 20 /* SHA-1 digest size */ -#else -#define HASH_SIZE sizeof(int) -#endif +#define HASH_SIZE 32 /* SHA-256 digest size */ struct frec { union mysockaddr source; @@ -1234,7 +1230,6 @@ int check_for_bogus_wildcard(struct dns_header *header, size_t qlen, char *name, struct bogus_addr *baddr, time_t now); int check_for_ignored_address(struct dns_header *header, size_t qlen, struct bogus_addr *baddr); int check_for_local_domain(char *name, time_t now); -unsigned int questions_crc(struct dns_header *header, size_t plen, char *name); size_t resize_packet(struct dns_header *header, size_t plen, unsigned char *pheader, size_t hlen); int add_resource_record(struct dns_header *header, char *limit, int *truncp, @@ -1259,9 +1254,11 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch int check_unsigned, int *neganswer, int *nons, int *nsec_ttl); int dnskey_keytag(int alg, int flags, unsigned char *key, int keylen); size_t filter_rrsigs(struct dns_header *header, size_t plen); -unsigned char* hash_questions(struct dns_header *header, size_t plen, char *name); int setup_timestamp(void); +/* hash_questions.c */ +unsigned char *hash_questions(struct dns_header *header, size_t plen, char *name); + /* crypto.c */ const struct nettle_hash *hash_find(char *name); int hash_init(const struct nettle_hash *hash, void **ctxp, unsigned char **digestp); diff --git a/src/dnsmasq/dnssec.c b/src/dnsmasq/dnssec.c index e95aa3442..9410a7e0d 100644 --- a/src/dnsmasq/dnssec.c +++ b/src/dnsmasq/dnssec.c @@ -2087,35 +2087,4 @@ size_t dnssec_generate_query(struct dns_header *header, unsigned char *end, char return ret; } -unsigned char* hash_questions(struct dns_header *header, size_t plen, char *name) -{ - int q; - unsigned int len; - unsigned char *p = (unsigned char *)(header+1); - const struct nettle_hash *hash; - void *ctx; - unsigned char *digest; - - if (!(hash = hash_find("sha1")) || !hash_init(hash, &ctx, &digest)) - return NULL; - - for (q = ntohs(header->qdcount); q != 0; q--) - { - if (!extract_name(header, plen, &p, name, 1, 4)) - break; /* bad packet */ - - len = to_wire(name); - hash->update(ctx, len, (unsigned char *)name); - /* CRC the class and type as well */ - hash->update(ctx, 4, p); - - p += 4; - if (!CHECK_LEN(header, p, plen, 0)) - break; /* bad packet */ - } - - hash->digest(ctx, hash->digest_size, digest); - return digest; -} - #endif /* HAVE_DNSSEC */ diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index a7bb4e60a..246612f38 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -266,19 +266,16 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, union all_addr *addrp = NULL; unsigned int flags = 0; struct server *start = NULL; -#ifdef HAVE_DNSSEC void *hash = hash_questions(header, plen, daemon->namebuff); +#ifdef HAVE_DNSSEC int do_dnssec = 0; -#else - unsigned int crc = questions_crc(header, plen, daemon->namebuff); - void *hash = &crc; #endif unsigned int gotname = extract_request(header, plen, daemon->namebuff, NULL); unsigned char *oph = find_pseudoheader(header, plen, NULL, NULL, NULL, NULL); (void)do_bit; /* may be no servers available. */ - if (forward || (hash && (forward = lookup_frec_by_sender(ntohs(header->id), udpaddr, hash)))) + if (forward || (forward = lookup_frec_by_sender(ntohs(header->id), udpaddr, hash))) { /* If we didn't get an answer advertising a maximal packet in EDNS, fall back to 1280, which should work everywhere on IPv6. @@ -805,9 +802,6 @@ void reply_query(int fd, int family, time_t now) size_t nn; struct server *server; void *hash; -#ifndef HAVE_DNSSEC - unsigned int crc; -#endif /* packet buffer overwritten */ daemon->srv_save = NULL; @@ -834,12 +828,7 @@ void reply_query(int fd, int family, time_t now) if (difftime(now, server->pktsz_reduced) > UDP_TEST_TIME) server->edns_pktsz = daemon->edns_pktsz; -#ifdef HAVE_DNSSEC hash = hash_questions(header, n, daemon->namebuff); -#else - hash = &crc; - crc = questions_crc(header, n, daemon->namebuff); -#endif if (!(forward = lookup_frec(ntohs(header->id), fd, family, hash))) return; @@ -1151,8 +1140,7 @@ void reply_query(int fd, int family, time_t now) log_query(F_NOEXTRA | F_DNSSEC | F_IPV6, daemon->keyname, (union all_addr *)&(server->addr.in6.sin6_addr), querystr("dnssec-query", querytype)); - if ((hash = hash_questions(header, nn, daemon->namebuff))) - memcpy(new->hash, hash, HASH_SIZE); + memcpy(new->hash, hash_questions(header, nn, daemon->namebuff), HASH_SIZE); new->new_id = get_id(); header->id = htons(new->new_id); /* Save query for retransmission */ @@ -2077,15 +2065,9 @@ unsigned char *tcp_request(int confd, time_t now, if (!flags && last_server) { struct server *firstsendto = NULL; -#ifdef HAVE_DNSSEC - unsigned char *newhash, hash[HASH_SIZE]; - if ((newhash = hash_questions(header, (unsigned int)size, daemon->namebuff))) - memcpy(hash, newhash, HASH_SIZE); - else - memset(hash, 0, HASH_SIZE); -#else - unsigned int crc = questions_crc(header, (unsigned int)size, daemon->namebuff); -#endif + unsigned char hash[HASH_SIZE]; + memcpy(hash, hash_questions(header, (unsigned int)size, daemon->namebuff), HASH_SIZE); + /* Loop round available servers until we succeed in connecting to one. Note that this code subtly ensures that consecutive queries on this connection which can go to the same server, do so. */ @@ -2233,20 +2215,11 @@ unsigned char *tcp_request(int confd, time_t now, /* If the crc of the question section doesn't match the crc we sent, then someone might be attempting to insert bogus values into the cache by sending replies containing questions and bogus answers. */ -#ifdef HAVE_DNSSEC - newhash = hash_questions(header, (unsigned int)m, daemon->namebuff); - if (!newhash || memcmp(hash, newhash, HASH_SIZE) != 0) + if (memcmp(hash, hash_questions(header, (unsigned int)m, daemon->namebuff), HASH_SIZE) != 0) { m = 0; break; } -#else - if (crc != questions_crc(header, (unsigned int)m, daemon->namebuff)) - { - m = 0; - break; - } -#endif m = process_reply(header, now, last_server, (unsigned int)m, option_bool(OPT_NO_REBIND) && !norebind, no_cache_dnssec, cache_secure, bogusanswer, @@ -2464,7 +2437,7 @@ static struct frec *lookup_frec(unsigned short id, int fd, int family, void *has for(f = daemon->frec_list; f; f = f->next) if (f->sentto && f->new_id == id && - (!hash || memcmp(hash, f->hash, HASH_SIZE) == 0)) + (memcmp(hash, f->hash, HASH_SIZE) == 0)) { /* sent from random port */ if (family == AF_INET && f->rfd4 && f->rfd4->fd == fd) diff --git a/src/dnsmasq/hash_questions.c b/src/dnsmasq/hash_questions.c new file mode 100644 index 000000000..ae112ace4 --- /dev/null +++ b/src/dnsmasq/hash_questions.c @@ -0,0 +1,281 @@ +/* Copyright (c) 2012-2020 Simon Kelley + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; version 2 dated June, 1991, or + (at your option) version 3 dated 29 June, 2007. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . +*/ + + +/* Hash the question section. This is used to safely detect query + retransmission and to detect answers to questions we didn't ask, which + might be poisoning attacks. Note that we decode the name rather + than CRC the raw bytes, since replies might be compressed differently. + We ignore case in the names for the same reason. + + The hash used is SHA-256. If we're building with DNSSEC support, + we use the Nettle cypto library. If not, we prefer not to + add a dependency on Nettle, and use a stand-alone implementaion. +*/ + +#include "dnsmasq.h" + +#ifdef HAVE_DNSSEC +unsigned char *hash_questions(struct dns_header *header, size_t plen, char *name) +{ + int q; + unsigned char *p = (unsigned char *)(header+1); + const struct nettle_hash *hash; + void *ctx; + unsigned char *digest; + + if (!(hash = hash_find("sha256")) || !hash_init(hash, &ctx, &digest)) + { + /* don't think this can ever happen. */ + static unsigned char dummy[HASH_SIZE]; + static int warned = 0; + + if (warned) + my_syslog(LOG_ERR, _("Failed to create SHA-256 hash object")); + warned = 1; + + return dummy; + } + + for (q = ntohs(header->qdcount); q != 0; q--) + { + char *cp, c; + + if (!extract_name(header, plen, &p, name, 1, 4)) + break; /* bad packet */ + + for (cp = name; (c = *cp); cp++) + if (c >= 'A' && c <= 'Z') + *cp += 'a' - 'A'; + + hash->update(ctx, cp - name, (unsigned char *)name); + /* CRC the class and type as well */ + hash->update(ctx, 4, p); + + p += 4; + if (!CHECK_LEN(header, p, plen, 0)) + break; /* bad packet */ + } + + hash->digest(ctx, hash->digest_size, digest); + return digest; +} + +#else /* HAVE_DNSSEC */ + +#define SHA256_BLOCK_SIZE 32 // SHA256 outputs a 32 byte digest +typedef unsigned char BYTE; // 8-bit byte +typedef unsigned int WORD; // 32-bit word, change to "long" for 16-bit machines + +typedef struct { + BYTE data[64]; + WORD datalen; + unsigned long long bitlen; + WORD state[8]; +} SHA256_CTX; + +static void sha256_init(SHA256_CTX *ctx); +static void sha256_update(SHA256_CTX *ctx, const BYTE data[], size_t len); +static void sha256_final(SHA256_CTX *ctx, BYTE hash[]); + + +unsigned char *hash_questions(struct dns_header *header, size_t plen, char *name) +{ + int q; + unsigned char *p = (unsigned char *)(header+1); + SHA256_CTX ctx; + static BYTE digest[SHA256_BLOCK_SIZE]; + + sha256_init(&ctx); + + for (q = ntohs(header->qdcount); q != 0; q--) + { + char *cp, c; + + if (!extract_name(header, plen, &p, name, 1, 4)) + break; /* bad packet */ + + for (cp = name; (c = *cp); cp++) + if (c >= 'A' && c <= 'Z') + *cp += 'a' - 'A'; + + sha256_update(&ctx, (BYTE *)name, cp - name); + /* CRC the class and type as well */ + sha256_update(&ctx, (BYTE *)p, 4); + + p += 4; + if (!CHECK_LEN(header, p, plen, 0)) + break; /* bad packet */ + } + + sha256_final(&ctx, digest); + return (unsigned char *)digest; +} + +/* Code from here onwards comes from https://github.com/B-Con/crypto-algorithms + and was written by Brad Conte (brad@bradconte.com), to whom all credit is given. + + This code is in the public domain, and the copyright notice at the head of this + file does not apply to it. +*/ + + +/****************************** MACROS ******************************/ +#define ROTLEFT(a,b) (((a) << (b)) | ((a) >> (32-(b)))) +#define ROTRIGHT(a,b) (((a) >> (b)) | ((a) << (32-(b)))) + +#define CH(x,y,z) (((x) & (y)) ^ (~(x) & (z))) +#define MAJ(x,y,z) (((x) & (y)) ^ ((x) & (z)) ^ ((y) & (z))) +#define EP0(x) (ROTRIGHT(x,2) ^ ROTRIGHT(x,13) ^ ROTRIGHT(x,22)) +#define EP1(x) (ROTRIGHT(x,6) ^ ROTRIGHT(x,11) ^ ROTRIGHT(x,25)) +#define SIG0(x) (ROTRIGHT(x,7) ^ ROTRIGHT(x,18) ^ ((x) >> 3)) +#define SIG1(x) (ROTRIGHT(x,17) ^ ROTRIGHT(x,19) ^ ((x) >> 10)) + +/**************************** VARIABLES *****************************/ +static const WORD k[64] = { + 0x428a2f98,0x71374491,0xb5c0fbcf,0xe9b5dba5,0x3956c25b,0x59f111f1,0x923f82a4,0xab1c5ed5, + 0xd807aa98,0x12835b01,0x243185be,0x550c7dc3,0x72be5d74,0x80deb1fe,0x9bdc06a7,0xc19bf174, + 0xe49b69c1,0xefbe4786,0x0fc19dc6,0x240ca1cc,0x2de92c6f,0x4a7484aa,0x5cb0a9dc,0x76f988da, + 0x983e5152,0xa831c66d,0xb00327c8,0xbf597fc7,0xc6e00bf3,0xd5a79147,0x06ca6351,0x14292967, + 0x27b70a85,0x2e1b2138,0x4d2c6dfc,0x53380d13,0x650a7354,0x766a0abb,0x81c2c92e,0x92722c85, + 0xa2bfe8a1,0xa81a664b,0xc24b8b70,0xc76c51a3,0xd192e819,0xd6990624,0xf40e3585,0x106aa070, + 0x19a4c116,0x1e376c08,0x2748774c,0x34b0bcb5,0x391c0cb3,0x4ed8aa4a,0x5b9cca4f,0x682e6ff3, + 0x748f82ee,0x78a5636f,0x84c87814,0x8cc70208,0x90befffa,0xa4506ceb,0xbef9a3f7,0xc67178f2 +}; + +/*********************** FUNCTION DEFINITIONS ***********************/ +static void sha256_transform(SHA256_CTX *ctx, const BYTE data[]) +{ + WORD a, b, c, d, e, f, g, h, i, j, t1, t2, m[64]; + + for (i = 0, j = 0; i < 16; ++i, j += 4) + m[i] = (data[j] << 24) | (data[j + 1] << 16) | (data[j + 2] << 8) | (data[j + 3]); + for ( ; i < 64; ++i) + m[i] = SIG1(m[i - 2]) + m[i - 7] + SIG0(m[i - 15]) + m[i - 16]; + + a = ctx->state[0]; + b = ctx->state[1]; + c = ctx->state[2]; + d = ctx->state[3]; + e = ctx->state[4]; + f = ctx->state[5]; + g = ctx->state[6]; + h = ctx->state[7]; + + for (i = 0; i < 64; ++i) + { + t1 = h + EP1(e) + CH(e,f,g) + k[i] + m[i]; + t2 = EP0(a) + MAJ(a,b,c); + h = g; + g = f; + f = e; + e = d + t1; + d = c; + c = b; + b = a; + a = t1 + t2; + } + + ctx->state[0] += a; + ctx->state[1] += b; + ctx->state[2] += c; + ctx->state[3] += d; + ctx->state[4] += e; + ctx->state[5] += f; + ctx->state[6] += g; + ctx->state[7] += h; +} + +static void sha256_init(SHA256_CTX *ctx) +{ + ctx->datalen = 0; + ctx->bitlen = 0; + ctx->state[0] = 0x6a09e667; + ctx->state[1] = 0xbb67ae85; + ctx->state[2] = 0x3c6ef372; + ctx->state[3] = 0xa54ff53a; + ctx->state[4] = 0x510e527f; + ctx->state[5] = 0x9b05688c; + ctx->state[6] = 0x1f83d9ab; + ctx->state[7] = 0x5be0cd19; +} + +static void sha256_update(SHA256_CTX *ctx, const BYTE data[], size_t len) +{ + WORD i; + + for (i = 0; i < len; ++i) + { + ctx->data[ctx->datalen] = data[i]; + ctx->datalen++; + if (ctx->datalen == 64) { + sha256_transform(ctx, ctx->data); + ctx->bitlen += 512; + ctx->datalen = 0; + } + } +} + +static void sha256_final(SHA256_CTX *ctx, BYTE hash[]) +{ + WORD i; + + i = ctx->datalen; + + // Pad whatever data is left in the buffer. + if (ctx->datalen < 56) + { + ctx->data[i++] = 0x80; + while (i < 56) + ctx->data[i++] = 0x00; + } + else + { + ctx->data[i++] = 0x80; + while (i < 64) + ctx->data[i++] = 0x00; + sha256_transform(ctx, ctx->data); + memset(ctx->data, 0, 56); + } + + // Append to the padding the total message's length in bits and transform. + ctx->bitlen += ctx->datalen * 8; + ctx->data[63] = ctx->bitlen; + ctx->data[62] = ctx->bitlen >> 8; + ctx->data[61] = ctx->bitlen >> 16; + ctx->data[60] = ctx->bitlen >> 24; + ctx->data[59] = ctx->bitlen >> 32; + ctx->data[58] = ctx->bitlen >> 40; + ctx->data[57] = ctx->bitlen >> 48; + ctx->data[56] = ctx->bitlen >> 56; + sha256_transform(ctx, ctx->data); + + // Since this implementation uses little endian byte ordering and SHA uses big endian, + // reverse all the bytes when copying the final state to the output hash. + for (i = 0; i < 4; ++i) + { + hash[i] = (ctx->state[0] >> (24 - i * 8)) & 0x000000ff; + hash[i + 4] = (ctx->state[1] >> (24 - i * 8)) & 0x000000ff; + hash[i + 8] = (ctx->state[2] >> (24 - i * 8)) & 0x000000ff; + hash[i + 12] = (ctx->state[3] >> (24 - i * 8)) & 0x000000ff; + hash[i + 16] = (ctx->state[4] >> (24 - i * 8)) & 0x000000ff; + hash[i + 20] = (ctx->state[5] >> (24 - i * 8)) & 0x000000ff; + hash[i + 24] = (ctx->state[6] >> (24 - i * 8)) & 0x000000ff; + hash[i + 28] = (ctx->state[7] >> (24 - i * 8)) & 0x000000ff; + } +} + +#endif diff --git a/src/dnsmasq/rfc1035.c b/src/dnsmasq/rfc1035.c index 4a7994db5..aaacc5c27 100644 --- a/src/dnsmasq/rfc1035.c +++ b/src/dnsmasq/rfc1035.c @@ -334,55 +334,6 @@ unsigned char *skip_section(unsigned char *ansp, int count, struct dns_header *h return ansp; } -/* CRC the question section. This is used to safely detect query - retransmission and to detect answers to questions we didn't ask, which - might be poisoning attacks. Note that we decode the name rather - than CRC the raw bytes, since replies might be compressed differently. - We ignore case in the names for the same reason. Return all-ones - if there is not question section. */ -#ifndef HAVE_DNSSEC -unsigned int questions_crc(struct dns_header *header, size_t plen, char *name) -{ - int q; - unsigned int crc = 0xffffffff; - unsigned char *p1, *p = (unsigned char *)(header+1); - - for (q = ntohs(header->qdcount); q != 0; q--) - { - if (!extract_name(header, plen, &p, name, 1, 4)) - return crc; /* bad packet */ - - for (p1 = (unsigned char *)name; *p1; p1++) - { - int i = 8; - char c = *p1; - - if (c >= 'A' && c <= 'Z') - c += 'a' - 'A'; - - crc ^= c << 24; - while (i--) - crc = crc & 0x80000000 ? (crc << 1) ^ 0x04c11db7 : crc << 1; - } - - /* CRC the class and type as well */ - for (p1 = p; p1 < p+4; p1++) - { - int i = 8; - crc ^= *p1 << 24; - while (i--) - crc = crc & 0x80000000 ? (crc << 1) ^ 0x04c11db7 : crc << 1; - } - - p += 4; - if (!CHECK_LEN(header, p, plen, 0)) - return crc; /* bad packet */ - } - - return crc; -} -#endif - size_t resize_packet(struct dns_header *header, size_t plen, unsigned char *pheader, size_t hlen) { unsigned char *ansp = skip_questions(header, plen); From 58c6b8d56e375701d415d9b3fa7f3ee93fd6f09f Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Thu, 12 Nov 2020 23:09:15 +0000 Subject: [PATCH 11/21] Optimse RR digest calculation in DNSSEC. If an RR is of a type which doesn't need canonicalisation, bypass the relatively slow canonicalisation code, and insert it direct into the digest. Signed-off-by: DL6ER --- src/dnsmasq/dnssec.c | 82 ++++++++++++++++++++++++++------------------ 1 file changed, 49 insertions(+), 33 deletions(-) diff --git a/src/dnsmasq/dnssec.c b/src/dnsmasq/dnssec.c index 9410a7e0d..93cc7bf1e 100644 --- a/src/dnsmasq/dnssec.c +++ b/src/dnsmasq/dnssec.c @@ -579,7 +579,7 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in hash->update(ctx, (unsigned int)wire_len, (unsigned char*)keyname); from_wire(keyname); -#define RRBUFLEN 300 /* Most RRs are smaller than this. */ +#define RRBUFLEN 128 /* Most RRs are smaller than this. */ for (i = 0; i < rrsetidx; ++i) { @@ -617,50 +617,66 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in hash->update(ctx, (unsigned int)wire_len, (unsigned char *)name_start); hash->update(ctx, 4, p); /* class and type */ hash->update(ctx, 4, (unsigned char *)&nsigttl); - - p += 8; /* skip class, type, ttl */ + + p += 8; /* skip type, class, ttl */ GETSHORT(rdlen, p); if (!CHECK_LEN(header, p, plen, rdlen)) return STAT_BOGUS; - - /* canonicalise rdata and calculate length of same, use - name buffer as workspace for get_rdata. */ - state.ip = p; - state.op = NULL; - state.desc = rr_desc; - state.buff = name; - state.end = p + rdlen; - - for (j = 0; get_rdata(header, plen, &state); j++) - if (j < RRBUFLEN) - rrbuf[j] = *state.op; - - len = htons((u16)j); - hash->update(ctx, 2, (unsigned char *)&len); - /* If the RR is shorter than RRBUFLEN (most of them, in practice) - then we can just digest it now. If it exceeds RRBUFLEN we have to - go back to the start and do it in chunks. */ - if (j >= RRBUFLEN) + /* Optimisation for RR types which need no cannonicalisation. + This includes DNSKEY DS NSEC and NSEC3, which are also long, so + it saves lots of calls to get_rdata, and avoids the pessimal + segmented insertion, even with a small rrbuf[]. + + If canonicalisation is not needed, a simple insertion into the hash works. + */ + if (*rr_desc == (u16)-1) { + len = htons(rdlen); + hash->update(ctx, 2, (unsigned char *)&len); + hash->update(ctx, rdlen, p); + } + else + { + /* canonicalise rdata and calculate length of same, use + name buffer as workspace for get_rdata. */ state.ip = p; state.op = NULL; state.desc = rr_desc; - + state.buff = name; + state.end = p + rdlen; + for (j = 0; get_rdata(header, plen, &state); j++) + if (j < RRBUFLEN) + rrbuf[j] = *state.op; + + len = htons((u16)j); + hash->update(ctx, 2, (unsigned char *)&len); + + /* If the RR is shorter than RRBUFLEN (most of them, in practice) + then we can just digest it now. If it exceeds RRBUFLEN we have to + go back to the start and do it in chunks. */ + if (j >= RRBUFLEN) { - rrbuf[j] = *state.op; - - if (j == RRBUFLEN - 1) - { - hash->update(ctx, RRBUFLEN, rrbuf); - j = -1; - } + state.ip = p; + state.op = NULL; + state.desc = rr_desc; + + for (j = 0; get_rdata(header, plen, &state); j++) + { + rrbuf[j] = *state.op; + + if (j == RRBUFLEN - 1) + { + hash->update(ctx, RRBUFLEN, rrbuf); + j = -1; + } + } } + + if (j != 0) + hash->update(ctx, j, rrbuf); } - - if (j != 0) - hash->update(ctx, j, rrbuf); } hash->digest(ctx, hash->digest_size, digest); From 17cb5d4d8134381f29e1528882652880e4641677 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sat, 14 Nov 2020 15:29:34 +0000 Subject: [PATCH 12/21] Fix DNS reply when asking for DNSSEC and a validated CNAME is already cached. Signed-off-by: DL6ER --- src/dnsmasq/rfc1035.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/dnsmasq/rfc1035.c b/src/dnsmasq/rfc1035.c index aaacc5c27..013fbf143 100644 --- a/src/dnsmasq/rfc1035.c +++ b/src/dnsmasq/rfc1035.c @@ -1373,6 +1373,8 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen, } } + else + return 0; /* give up if any cached CNAME in chain can't be used for DNSSEC reasons. */ strcpy(name, cname_target); } From a5743a1643726eeff1cea366cd20b2065adcbf5e Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Sun, 15 Nov 2020 22:13:25 +0000 Subject: [PATCH 13/21] Add missing check for NULL return from allocate_rfd(). Signed-off-by: DL6ER --- src/dnsmasq/forward.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index 246612f38..5ab817b18 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -859,7 +859,6 @@ void reply_query(int fd, int family, time_t now) int is_sign; #ifdef HAVE_DNSSEC - /* For DNSSEC originated queries, just retry the query to the same server. */ if (forward->flags & (FREC_DNSKEY_QUERY | FREC_DS_QUERY)) { struct server *start; @@ -885,6 +884,8 @@ void reply_query(int fd, int family, time_t now) } + fd = -1; + if (start->sfd) fd = start->sfd->fd; else @@ -892,19 +893,21 @@ void reply_query(int fd, int family, time_t now) if (start->addr.sa.sa_family == AF_INET6) { /* may have changed family */ - if (!forward->rfd6) - forward->rfd6 = allocate_rfd(AF_INET6); - fd = forward->rfd6->fd; + if (forward->rfd6 || (forward->rfd6 = allocate_rfd(AF_INET6))) + fd = forward->rfd6->fd; } else { /* may have changed family */ - if (!forward->rfd4) - forward->rfd4 = allocate_rfd(AF_INET); - fd = forward->rfd4->fd; + if (forward->rfd4 || (forward->rfd4 = allocate_rfd(AF_INET))) + fd = forward->rfd4->fd; } } + /* Can't get socket. */ + if (fd == -1) + return; + #ifdef HAVE_DUMPFILE dump_packet(DUMP_SEC_QUERY, (void *)header, (size_t)plen, NULL, &start->addr); #endif @@ -2430,7 +2433,6 @@ struct frec *get_new_frec(time_t now, int *wait, struct frec *force) return f; /* OK if malloc fails and this is NULL */ } -/* crc is all-ones if not known. */ static struct frec *lookup_frec(unsigned short id, int fd, int family, void *hash) { struct frec *f; From 3774edeba8fad5d8080530e805e33a28ab444fde Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Wed, 18 Nov 2020 18:34:55 +0000 Subject: [PATCH 14/21] Handle multiple identical near simultaneous DNS queries better. Previously, such queries would all be forwarded independently. This is, in theory, inefficent but in practise not a problem, _except_ that is means that an answer for any of the forwarded queries will be accepted and cached. An attacker can send a query multiple times, and for each repeat, another {port, ID} becomes capable of accepting the answer he is sending in the blind, to random IDs and ports. The chance of a succesful attack is therefore multiplied by the number of repeats of the query. The new behaviour detects repeated queries and merely stores the clients sending repeats so that when the first query completes, the answer can be sent to all the clients who asked. Refer: CERT VU#434904. Signed-off-by: DL6ER --- src/dnsmasq/dnsmasq.h | 19 ++++-- src/dnsmasq/forward.c | 142 +++++++++++++++++++++++++++++++++++------- 2 files changed, 132 insertions(+), 29 deletions(-) diff --git a/src/dnsmasq/dnsmasq.h b/src/dnsmasq/dnsmasq.h index 36946668b..dddb615a5 100644 --- a/src/dnsmasq/dnsmasq.h +++ b/src/dnsmasq/dnsmasq.h @@ -655,19 +655,24 @@ struct hostsfile { #define FREC_DO_QUESTION 64 #define FREC_ADDED_PHEADER 128 #define FREC_TEST_PKTSZ 256 -#define FREC_HAS_EXTRADATA 512 +#define FREC_HAS_EXTRADATA 512 +#define FREC_HAS_PHEADER 1024 #define HASH_SIZE 32 /* SHA-256 digest size */ struct frec { - union mysockaddr source; - union all_addr dest; + struct frec_src { + union mysockaddr source; + union all_addr dest; + unsigned int iface, log_id; + unsigned short orig_id; + struct frec_src *next; + } frec_src; struct server *sentto; /* NULL means free */ struct randfd *rfd4; struct randfd *rfd6; - unsigned int iface; - unsigned short orig_id, new_id; - int log_id, fd, forwardall, flags; + unsigned short new_id; + int fd, forwardall, flags; time_t time; unsigned char *hash[HASH_SIZE]; #ifdef HAVE_DNSSEC @@ -1095,6 +1100,8 @@ extern struct daemon { int back_to_the_future; #endif struct frec *frec_list; + struct frec_src *free_frec_src; + int frec_src_count; struct serverfd *sfds; struct irec *interfaces; struct listener *listeners; diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index 5ab817b18..6810d0d35 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -21,6 +21,8 @@ static struct frec *lookup_frec(unsigned short id, int fd, int family, void *has static struct frec *lookup_frec_by_sender(unsigned short id, union mysockaddr *addr, void *hash); +static struct frec *lookup_frec_by_query(void *hash, unsigned int flags); + static unsigned short get_id(void); static void free_frec(struct frec *f); @@ -265,6 +267,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, int type = SERV_DO_DNSSEC, norebind = 0; union all_addr *addrp = NULL; unsigned int flags = 0; + unsigned int fwd_flags = 0; struct server *start = NULL; void *hash = hash_questions(header, plen, daemon->namebuff); #ifdef HAVE_DNSSEC @@ -273,7 +276,18 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, unsigned int gotname = extract_request(header, plen, daemon->namebuff, NULL); unsigned char *oph = find_pseudoheader(header, plen, NULL, NULL, NULL, NULL); (void)do_bit; - + + if (header->hb4 & HB4_CD) + fwd_flags |= FREC_CHECKING_DISABLED; + if (ad_reqd) + fwd_flags |= FREC_AD_QUESTION; + if (oph) + fwd_flags |= FREC_HAS_PHEADER; +#ifdef HAVE_DNSSEC + if (do_bit) + fwd_flags |= FREC_DO_QUESTION; +#endif + /* may be no servers available. */ if (forward || (forward = lookup_frec_by_sender(ntohs(header->id), udpaddr, hash))) { @@ -349,6 +363,39 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, } else { + /* Query from new source, but the same query may be in progress + from another source. If so, just add this client to the + list that will get the reply. + + Note that is the EDNS client subnet option is in use, we can't do this, + as the clients (and therefore query EDNS options) will be different + for each query. The EDNS subnet code has checks to avoid + attacks in this case. */ + if (!option_bool(OPT_CLIENT_SUBNET) && (forward = lookup_frec_by_query(hash, fwd_flags))) + { + /* Note whine_malloc() zeros memory. */ + if (!daemon->free_frec_src && + daemon->frec_src_count < daemon->ftabsize && + (daemon->free_frec_src = whine_malloc(sizeof(struct frec_src)))) + daemon->frec_src_count++; + + /* If we've been spammed with many duplicates, just drop the query. */ + if (daemon->free_frec_src) + { + struct frec_src *new = daemon->free_frec_src; + daemon->free_frec_src = new->next; + new->next = forward->frec_src.next; + forward->frec_src.next = new; + new->orig_id = ntohs(header->id); + new->source = *udpaddr; + new->dest = *dst_addr; + new->log_id = daemon->log_id; + new->iface = dst_iface; + } + + return 1; + } + if (gotname) flags = search_servers(now, &addrp, gotname, daemon->namebuff, &type, &domain, &norebind); @@ -356,22 +403,22 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, do_dnssec = type & SERV_DO_DNSSEC; #endif type &= ~SERV_DO_DNSSEC; - + if (daemon->servers && !flags) forward = get_new_frec(now, NULL, NULL); /* table full - flags == 0, return REFUSED */ if (forward) { - forward->source = *udpaddr; - forward->dest = *dst_addr; - forward->iface = dst_iface; - forward->orig_id = ntohs(header->id); + forward->frec_src.source = *udpaddr; + forward->frec_src.orig_id = ntohs(header->id); + forward->frec_src.dest = *dst_addr; + forward->frec_src.iface = dst_iface; forward->new_id = get_id(); forward->fd = udpfd; memcpy(forward->hash, hash, HASH_SIZE); forward->forwardall = 0; - forward->flags = 0; + forward->flags = fwd_flags; if (norebind) forward->flags |= FREC_NOREBIND; if (header->hb4 & HB4_CD) @@ -426,9 +473,9 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, unsigned char *pheader; /* If a query is retried, use the log_id for the retry when logging the answer. */ - forward->log_id = daemon->log_id; + forward->frec_src.log_id = daemon->log_id; - plen = add_edns0_config(header, plen, ((unsigned char *)header) + PACKETSZ, &forward->source, now, &subnet); + plen = add_edns0_config(header, plen, ((unsigned char *)header) + PACKETSZ, &forward->frec_src.source, now, &subnet); if (subnet) forward->flags |= FREC_HAS_SUBNET; @@ -573,7 +620,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, return 1; /* could not send on, prepare to return */ - header->id = htons(forward->orig_id); + header->id = htons(forward->frec_src.orig_id); free_frec(forward); /* cancel */ } @@ -840,8 +887,8 @@ void reply_query(int fd, int family, time_t now) /* log_query gets called indirectly all over the place, so pass these in global variables - sorry. */ - daemon->log_display_id = forward->log_id; - daemon->log_source_addr = &forward->source; + daemon->log_display_id = forward->frec_src.log_id; + daemon->log_source_addr = &forward->frec_src.source; if (daemon->ignore_addr && RCODE(header) == NOERROR && check_for_ignored_address(header, n, daemon->ignore_addr)) @@ -1116,6 +1163,7 @@ void reply_query(int fd, int family, time_t now) new->sentto = server; new->rfd4 = NULL; new->rfd6 = NULL; + new->frec_src.next = NULL; new->flags &= ~(FREC_DNSKEY_QUERY | FREC_DS_QUERY | FREC_HAS_EXTRADATA); new->forwardall = 0; @@ -1252,9 +1300,11 @@ void reply_query(int fd, int family, time_t now) if ((nn = process_reply(header, now, forward->sentto, (size_t)n, check_rebind, no_cache_dnssec, cache_secure, bogusanswer, forward->flags & FREC_AD_QUESTION, forward->flags & FREC_DO_QUESTION, - forward->flags & FREC_ADDED_PHEADER, forward->flags & FREC_HAS_SUBNET, &forward->source))) + forward->flags & FREC_ADDED_PHEADER, forward->flags & FREC_HAS_SUBNET, &forward->frec_src.source))) { - header->id = htons(forward->orig_id); + struct frec_src *src; + + header->id = htons(forward->frec_src.orig_id); header->hb4 |= HB4_RA; /* recursion if available */ #ifdef HAVE_DNSSEC /* We added an EDNSO header for the purpose of getting DNSSEC RRs, and set the value of the UDP payload size @@ -1270,13 +1320,26 @@ void reply_query(int fd, int family, time_t now) } #endif + for (src = &forward->frec_src; src; src = src->next) + { + header->id = htons(src->orig_id); + #ifdef HAVE_DUMPFILE - dump_packet(DUMP_REPLY, daemon->packet, (size_t)nn, NULL, &forward->source); + dump_packet(DUMP_REPLY, daemon->packet, (size_t)nn, NULL, &src->source); #endif - - send_from(forward->fd, option_bool(OPT_NOWILD) || option_bool (OPT_CLEVERBIND), daemon->packet, nn, - &forward->source, &forward->dest, forward->iface); + + send_from(forward->fd, option_bool(OPT_NOWILD) || option_bool (OPT_CLEVERBIND), daemon->packet, nn, + &src->source, &src->dest, src->iface); + + if (option_bool(OPT_EXTRALOG) && src != &forward->frec_src) + { + daemon->log_display_id = src->log_id; + daemon->log_source_addr = &src->source; + log_query(F_UPSTREAM, "query", NULL, "duplicate"); + } + } } + free_frec(forward); /* cancel */ } } @@ -2321,6 +2384,17 @@ void free_rfd(struct randfd *rfd) static void free_frec(struct frec *f) { + struct frec_src *src, *tmp; + + /* add back to freelist of not the record builtin to every frec. */ + for (src = f->frec_src.next; src; src = tmp) + { + tmp = src->next; + src->next = daemon->free_frec_src; + daemon->free_frec_src = src; + } + + f->frec_src.next = NULL; free_rfd(f->rfd4); f->rfd4 = NULL; f->sentto = NULL; @@ -2461,17 +2535,39 @@ static struct frec *lookup_frec_by_sender(unsigned short id, void *hash) { struct frec *f; + struct frec_src *src; + + for (f = daemon->frec_list; f; f = f->next) + if (f->sentto && + !(f->flags & (FREC_DNSKEY_QUERY | FREC_DS_QUERY)) && + memcmp(hash, f->hash, HASH_SIZE) == 0) + for (src = &f->frec_src; src; src = src->next) + if (src->orig_id == id && + sockaddr_isequal(&src->source, addr)) + return f; + + return NULL; +} + +static struct frec *lookup_frec_by_query(void *hash, unsigned int flags) +{ + struct frec *f; + + /* FREC_DNSKEY and FREC_DS_QUERY are never set in flags, so the test below + ensures that no frec created for internal DNSSEC query can be returned here. */ + +#define FLAGMASK (FREC_CHECKING_DISABLED | FREC_AD_QUESTION | FREC_DO_QUESTION \ + | FREC_HAS_PHEADER | FREC_DNSKEY_QUERY | FREC_DS_QUERY) for(f = daemon->frec_list; f; f = f->next) if (f->sentto && - f->orig_id == id && - memcmp(hash, f->hash, HASH_SIZE) == 0 && - sockaddr_isequal(&f->source, addr)) + (f->flags & FLAGMASK) == flags && + memcmp(hash, f->hash, HASH_SIZE) == 0) return f; - + return NULL; } - + /* Send query packet again, if we can. */ void resend_query() { From a4864cf5dd73f45fe2bc1b8d160942cc03a7f0ec Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Wed, 25 Nov 2020 21:17:52 +0000 Subject: [PATCH 15/21] Handle caching with EDNS options better. If we add the EDNS client subnet option, or the client's MAC address, then the reply we get back may very depending on that. Since the cache is ignorant of such things, it's not safe to cache such replies. This patch determines when a dangerous EDNS option is being added and disables caching. Note that for much the same reason, we can't combine multiple queries for the same question when dangerous EDNS options are being added, and the code now handles that in the same way. This query combining is required for security against cache poisoning, so disabling the cache has a security function as well as a correctness one. Signed-off-by: DL6ER --- src/dnsmasq/dnsmasq.h | 3 +- src/dnsmasq/edns0.c | 75 +++++++++++++++++++++++++++---------------- src/dnsmasq/forward.c | 41 +++++++++++++++-------- 3 files changed, 76 insertions(+), 43 deletions(-) diff --git a/src/dnsmasq/dnsmasq.h b/src/dnsmasq/dnsmasq.h index dddb615a5..e0a5e3871 100644 --- a/src/dnsmasq/dnsmasq.h +++ b/src/dnsmasq/dnsmasq.h @@ -657,6 +657,7 @@ struct hostsfile { #define FREC_TEST_PKTSZ 256 #define FREC_HAS_EXTRADATA 512 #define FREC_HAS_PHEADER 1024 +#define FREC_NO_CACHE 2048 #define HASH_SIZE 32 /* SHA-256 digest size */ @@ -1674,7 +1675,7 @@ size_t add_pseudoheader(struct dns_header *header, size_t plen, unsigned char *l unsigned short udp_sz, int optno, unsigned char *opt, size_t optlen, int set_do, int replace); size_t add_do_bit(struct dns_header *header, size_t plen, unsigned char *limit); size_t add_edns0_config(struct dns_header *header, size_t plen, unsigned char *limit, - union mysockaddr *source, time_t now, int *check_subnet); + union mysockaddr *source, time_t now, int *check_subnet, int *cacheable); int check_source(struct dns_header *header, size_t plen, unsigned char *pseudoheader, union mysockaddr *peer); /* arp.c */ diff --git a/src/dnsmasq/edns0.c b/src/dnsmasq/edns0.c index d75d3cc1f..53cfe2401 100644 --- a/src/dnsmasq/edns0.c +++ b/src/dnsmasq/edns0.c @@ -264,7 +264,8 @@ static void encoder(unsigned char *in, char *out) out[3] = char64(in[2]); } -static size_t add_dns_client(struct dns_header *header, size_t plen, unsigned char *limit, union mysockaddr *l3, time_t now) +static size_t add_dns_client(struct dns_header *header, size_t plen, unsigned char *limit, + union mysockaddr *l3, time_t now, int *cacheablep) { int maclen, replace = 2; /* can't get mac address, just delete any incoming. */ unsigned char mac[DHCP_CHADDR_MAX]; @@ -273,6 +274,7 @@ static size_t add_dns_client(struct dns_header *header, size_t plen, unsigned ch if ((maclen = find_mac(l3, mac, 1, now)) == 6) { replace = 1; + *cacheablep = 0; if (option_bool(OPT_MAC_HEX)) print_mac(encode, mac, maclen); @@ -288,14 +290,18 @@ static size_t add_dns_client(struct dns_header *header, size_t plen, unsigned ch } -static size_t add_mac(struct dns_header *header, size_t plen, unsigned char *limit, union mysockaddr *l3, time_t now) +static size_t add_mac(struct dns_header *header, size_t plen, unsigned char *limit, + union mysockaddr *l3, time_t now, int *cacheablep) { int maclen; unsigned char mac[DHCP_CHADDR_MAX]; if ((maclen = find_mac(l3, mac, 1, now)) != 0) - plen = add_pseudoheader(header, plen, limit, PACKETSZ, EDNS0_OPTION_MAC, mac, maclen, 0, 0); - + { + *cacheablep = 0; + plen = add_pseudoheader(header, plen, limit, PACKETSZ, EDNS0_OPTION_MAC, mac, maclen, 0, 0); + } + return plen; } @@ -313,17 +319,18 @@ static void *get_addrp(union mysockaddr *addr, const short family) return &addr->in.sin_addr; } -static size_t calc_subnet_opt(struct subnet_opt *opt, union mysockaddr *source) +static size_t calc_subnet_opt(struct subnet_opt *opt, union mysockaddr *source, int *cacheablep) { /* http://tools.ietf.org/html/draft-vandergaast-edns-client-subnet-02 */ int len; void *addrp = NULL; int sa_family = source->sa.sa_family; - + int cacheable = 0; + opt->source_netmask = 0; opt->scope_netmask = 0; - + if (source->sa.sa_family == AF_INET6 && daemon->add_subnet6) { opt->source_netmask = daemon->add_subnet6->mask; @@ -331,6 +338,7 @@ static size_t calc_subnet_opt(struct subnet_opt *opt, union mysockaddr *source) { sa_family = daemon->add_subnet6->addr.sa.sa_family; addrp = get_addrp(&daemon->add_subnet6->addr, sa_family); + cacheable = 1; } else addrp = &source->in6.sin6_addr; @@ -343,6 +351,7 @@ static size_t calc_subnet_opt(struct subnet_opt *opt, union mysockaddr *source) { sa_family = daemon->add_subnet4->addr.sa.sa_family; addrp = get_addrp(&daemon->add_subnet4->addr, sa_family); + cacheable = 1; /* Address is constant */ } else addrp = &source->in.sin_addr; @@ -350,8 +359,6 @@ static size_t calc_subnet_opt(struct subnet_opt *opt, union mysockaddr *source) opt->family = htons(sa_family == AF_INET6 ? 2 : 1); - len = 0; - if (addrp && opt->source_netmask != 0) { len = ((opt->source_netmask - 1) >> 3) + 1; @@ -359,18 +366,26 @@ static size_t calc_subnet_opt(struct subnet_opt *opt, union mysockaddr *source) if (opt->source_netmask & 7) opt->addr[len-1] &= 0xff << (8 - (opt->source_netmask & 7)); } + else + { + cacheable = 1; /* No address ever supplied. */ + len = 0; + } + + if (cacheablep) + *cacheablep = cacheable; return len + 4; } -static size_t add_source_addr(struct dns_header *header, size_t plen, unsigned char *limit, union mysockaddr *source) +static size_t add_source_addr(struct dns_header *header, size_t plen, unsigned char *limit, union mysockaddr *source, int *cacheable) { /* http://tools.ietf.org/html/draft-vandergaast-edns-client-subnet-02 */ int len; struct subnet_opt opt; - len = calc_subnet_opt(&opt, source); + len = calc_subnet_opt(&opt, source, cacheable); return add_pseudoheader(header, plen, (unsigned char *)limit, PACKETSZ, EDNS0_OPTION_CLIENT_SUBNET, (unsigned char *)&opt, len, 0, 0); } @@ -383,18 +398,18 @@ int check_source(struct dns_header *header, size_t plen, unsigned char *pseudohe unsigned char *p; int code, i, rdlen; - calc_len = calc_subnet_opt(&opt, peer); - - if (!(p = skip_name(pseudoheader, header, plen, 10))) - return 1; - - p += 8; /* skip UDP length and RCODE */ + calc_len = calc_subnet_opt(&opt, peer, NULL); - GETSHORT(rdlen, p); - if (!CHECK_LEN(header, p, plen, rdlen)) - return 1; /* bad packet */ - - /* check if option there */ + if (!(p = skip_name(pseudoheader, header, plen, 10))) + return 1; + + p += 8; /* skip UDP length and RCODE */ + + GETSHORT(rdlen, p); + if (!CHECK_LEN(header, p, plen, rdlen)) + return 1; /* bad packet */ + + /* check if option there */ for (i = 0; i + 4 < rdlen; i += len + 4) { GETSHORT(code, p); @@ -412,24 +427,28 @@ int check_source(struct dns_header *header, size_t plen, unsigned char *pseudohe return 1; } +/* Set *check_subnet if we add a client subnet option, which needs to checked + in the reply. Set *cacheable to zero if we add an option which the answer + may depend on. */ size_t add_edns0_config(struct dns_header *header, size_t plen, unsigned char *limit, - union mysockaddr *source, time_t now, int *check_subnet) + union mysockaddr *source, time_t now, int *check_subnet, int *cacheable) { *check_subnet = 0; - + *cacheable = 1; + if (option_bool(OPT_ADD_MAC)) - plen = add_mac(header, plen, limit, source, now); + plen = add_mac(header, plen, limit, source, now, cacheable); if (option_bool(OPT_MAC_B64) || option_bool(OPT_MAC_HEX)) - plen = add_dns_client(header, plen, limit, source, now); - + plen = add_dns_client(header, plen, limit, source, now, cacheable); + if (daemon->dns_client_id) plen = add_pseudoheader(header, plen, limit, PACKETSZ, EDNS0_OPTION_NOMCPEID, (unsigned char *)daemon->dns_client_id, strlen(daemon->dns_client_id), 0, 1); if (option_bool(OPT_CLIENT_SUBNET)) { - plen = add_source_addr(header, plen, limit, source); + plen = add_source_addr(header, plen, limit, source, cacheable); *check_subnet = 1; } diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index 6810d0d35..7ac35ec8e 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -365,13 +365,10 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, { /* Query from new source, but the same query may be in progress from another source. If so, just add this client to the - list that will get the reply. + list that will get the reply.*/ - Note that is the EDNS client subnet option is in use, we can't do this, - as the clients (and therefore query EDNS options) will be different - for each query. The EDNS subnet code has checks to avoid - attacks in this case. */ - if (!option_bool(OPT_CLIENT_SUBNET) && (forward = lookup_frec_by_query(hash, fwd_flags))) + if (!option_bool(OPT_ADD_MAC) && !option_bool(OPT_MAC_B64) && + (forward = lookup_frec_by_query(hash, fwd_flags))) { /* Note whine_malloc() zeros memory. */ if (!daemon->free_frec_src && @@ -468,18 +465,21 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, if (!flags && forward) { struct server *firstsentto = start; - int subnet, forwarded = 0; + int subnet, cacheable, forwarded = 0; size_t edns0_len; unsigned char *pheader; /* If a query is retried, use the log_id for the retry when logging the answer. */ forward->frec_src.log_id = daemon->log_id; - plen = add_edns0_config(header, plen, ((unsigned char *)header) + PACKETSZ, &forward->frec_src.source, now, &subnet); + plen = add_edns0_config(header, plen, ((unsigned char *)header) + PACKETSZ, &forward->frec_src.source, now, &subnet, &cacheable); if (subnet) forward->flags |= FREC_HAS_SUBNET; - + + if (!cacheable) + forward->flags |= FREC_NO_CACHE; + #ifdef HAVE_DNSSEC if (option_bool(OPT_DNSSEC_VALID) && do_dnssec) { @@ -671,7 +671,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server } } #endif - + if ((pheader = find_pseudoheader(header, n, &plen, &sizep, &is_sign, NULL))) { /* Get extended RCODE. */ @@ -1297,6 +1297,11 @@ void reply_query(int fd, int family, time_t now) header->hb4 |= HB4_CD; else header->hb4 &= ~HB4_CD; + + /* Never cache answers which are contingent on the source or MAC address EDSN0 option, + since the cache is ignorant of such things. */ + if (forward->flags & FREC_NO_CACHE) + no_cache_dnssec = 1; if ((nn = process_reply(header, now, forward->sentto, (size_t)n, check_rebind, no_cache_dnssec, cache_secure, bogusanswer, forward->flags & FREC_AD_QUESTION, forward->flags & FREC_DO_QUESTION, @@ -1895,7 +1900,7 @@ unsigned char *tcp_request(int confd, time_t now, int local_auth = 0; #endif int checking_disabled, do_bit, added_pheader = 0, have_pseudoheader = 0; - int check_subnet, no_cache_dnssec = 0, cache_secure = 0, bogusanswer = 0; + int check_subnet, cacheable, no_cache_dnssec = 0, cache_secure = 0, bogusanswer = 0; size_t m; unsigned short qtype; unsigned int gotname; @@ -2099,7 +2104,7 @@ unsigned char *tcp_request(int confd, time_t now, char *domain = NULL; unsigned char *oph = find_pseudoheader(header, size, NULL, NULL, NULL, NULL); - size = add_edns0_config(header, size, ((unsigned char *) header) + 65536, &peer_addr, now, &check_subnet); + size = add_edns0_config(header, size, ((unsigned char *) header) + 65536, &peer_addr, now, &check_subnet, &cacheable); if (gotname) flags = search_servers(now, &addrp, gotname, daemon->namebuff, &type, &domain, &norebind); @@ -2287,6 +2292,11 @@ unsigned char *tcp_request(int confd, time_t now, break; } + /* Never cache answers which are contingent on the source or MAC address EDSN0 option, + since the cache is ignorant of such things. */ + if (!cacheable) + no_cache_dnssec = 1; + m = process_reply(header, now, last_server, (unsigned int)m, option_bool(OPT_NO_REBIND) && !norebind, no_cache_dnssec, cache_secure, bogusanswer, ad_reqd, do_bit, added_pheader, check_subnet, &peer_addr); @@ -2554,10 +2564,13 @@ static struct frec *lookup_frec_by_query(void *hash, unsigned int flags) struct frec *f; /* FREC_DNSKEY and FREC_DS_QUERY are never set in flags, so the test below - ensures that no frec created for internal DNSSEC query can be returned here. */ + ensures that no frec created for internal DNSSEC query can be returned here. + + Similarly FREC_NO_CACHE is never set in flags, so a query which is + contigent on a particular source address EDNS0 option will never be matched. */ #define FLAGMASK (FREC_CHECKING_DISABLED | FREC_AD_QUESTION | FREC_DO_QUESTION \ - | FREC_HAS_PHEADER | FREC_DNSKEY_QUERY | FREC_DS_QUERY) + | FREC_HAS_PHEADER | FREC_DNSKEY_QUERY | FREC_DS_QUERY | FREC_NO_CACHE) for(f = daemon->frec_list; f; f = f->next) if (f->sentto && From dad7959f2b6fb77f1b571ffd2f2cbf80b81dae33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Wed, 25 Nov 2020 17:18:55 +0100 Subject: [PATCH 16/21] Support hash function from nettle (only) Unlike COPTS=-DHAVE_DNSSEC, allow usage of just sha256 function from nettle, but keep DNSSEC disabled at build time. Skips use of internal hash implementation without support for validation built-in. Signed-off-by: DL6ER --- src/dnsmasq/config.h | 8 ++++++++ src/dnsmasq/crypto.c | 7 +++++++ src/dnsmasq/dnsmasq.h | 2 +- src/dnsmasq/hash_questions.c | 2 +- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/dnsmasq/config.h b/src/dnsmasq/config.h index 719eff23b..2c4eb3a4d 100644 --- a/src/dnsmasq/config.h +++ b/src/dnsmasq/config.h @@ -122,6 +122,9 @@ HAVE_AUTH define this to include the facility to act as an authoritative DNS server for one or more zones. +HAVE_NETTLEHASH + include just hash function from nettle, but no DNSSEC. + HAVE_DNSSEC include DNSSEC validator. @@ -189,6 +192,7 @@ RESOLVFILE /* #define HAVE_IDN */ /* #define HAVE_LIBIDN2 */ /* #define HAVE_CONNTRACK */ +/* #define HAVE_NETTLEHASH */ /* #define HAVE_DNSSEC */ @@ -422,6 +426,10 @@ static char *compile_opts = "no-" #endif "auth " +#if !defined(HAVE_NETTLEHASH) && !defined(HAVE_DNSSEC) +"no-" +#endif +"nettlehash " #ifndef HAVE_DNSSEC "no-" #endif diff --git a/src/dnsmasq/crypto.c b/src/dnsmasq/crypto.c index ca6311122..09525d2f1 100644 --- a/src/dnsmasq/crypto.c +++ b/src/dnsmasq/crypto.c @@ -25,6 +25,9 @@ #if NETTLE_VERSION_MAJOR == 3 && NETTLE_VERSION_MINOR >= 6 # include #endif +#endif + +#if defined(HAVE_DNSSEC) || defined(HAVE_NETTLEHASH) #include #include @@ -167,6 +170,10 @@ int hash_init(const struct nettle_hash *hash, void **ctxp, unsigned char **diges return 1; } + +#endif + +#ifdef HAVE_DNSSEC static int dnsmasq_rsa_verify(struct blockdata *key_data, unsigned int key_len, unsigned char *sig, size_t sig_len, unsigned char *digest, size_t digest_len, int algo) diff --git a/src/dnsmasq/dnsmasq.h b/src/dnsmasq/dnsmasq.h index e0a5e3871..2522a4063 100644 --- a/src/dnsmasq/dnsmasq.h +++ b/src/dnsmasq/dnsmasq.h @@ -157,7 +157,7 @@ extern int capget(cap_user_header_t header, cap_user_data_t data); #include #endif -#ifdef HAVE_DNSSEC +#if defined(HAVE_DNSSEC) || defined(HAVE_NETTLEHASH) # include #endif diff --git a/src/dnsmasq/hash_questions.c b/src/dnsmasq/hash_questions.c index ae112ace4..917c18e66 100644 --- a/src/dnsmasq/hash_questions.c +++ b/src/dnsmasq/hash_questions.c @@ -28,7 +28,7 @@ #include "dnsmasq.h" -#ifdef HAVE_DNSSEC +#if defined(HAVE_DNSSEC) || defined(HAVE_NETTLEHASH) unsigned char *hash_questions(struct dns_header *header, size_t plen, char *name) { int q; From fab1f64db1f0344a83f6278aaf7a98a0c688483f Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Fri, 4 Dec 2020 18:35:11 +0000 Subject: [PATCH 17/21] Small cleanups in frec_src datastucture handling. Signed-off-by: DL6ER --- src/dnsmasq/forward.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index 7ac35ec8e..5a301b2fb 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -374,7 +374,10 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, if (!daemon->free_frec_src && daemon->frec_src_count < daemon->ftabsize && (daemon->free_frec_src = whine_malloc(sizeof(struct frec_src)))) - daemon->frec_src_count++; + { + daemon->frec_src_count++; + daemon->free_frec_src->next = NULL; + } /* If we've been spammed with many duplicates, just drop the query. */ if (daemon->free_frec_src) @@ -411,6 +414,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, forward->frec_src.orig_id = ntohs(header->id); forward->frec_src.dest = *dst_addr; forward->frec_src.iface = dst_iface; + forward->frec_src.next = NULL; forward->new_id = get_id(); forward->fd = udpfd; memcpy(forward->hash, hash, HASH_SIZE); @@ -2394,16 +2398,16 @@ void free_rfd(struct randfd *rfd) static void free_frec(struct frec *f) { - struct frec_src *src, *tmp; - - /* add back to freelist of not the record builtin to every frec. */ - for (src = f->frec_src.next; src; src = tmp) + struct frec_src *last; + + /* add back to freelist if not the record builtin to every frec. */ + for (last = f->frec_src.next; last && last->next; last = last->next) ; + if (last) { - tmp = src->next; - src->next = daemon->free_frec_src; - daemon->free_frec_src = src; + last->next = daemon->free_frec_src; + daemon->free_frec_src = f->frec_src.next; } - + f->frec_src.next = NULL; free_rfd(f->rfd4); f->rfd4 = NULL; From df2df56c5155b7730bde8a9859165740c82e0d62 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 9 Jan 2021 09:43:16 +0100 Subject: [PATCH 18/21] Adapt for change in struct forward to forward->frec_src Signed-off-by: DL6ER --- src/dnsmasq/CMakeLists.txt | 1 + src/dnsmasq/forward.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/dnsmasq/CMakeLists.txt b/src/dnsmasq/CMakeLists.txt index 43a410805..be1081f71 100644 --- a/src/dnsmasq/CMakeLists.txt +++ b/src/dnsmasq/CMakeLists.txt @@ -21,6 +21,7 @@ set(sources dump.c edns0.c forward.c + hash_questions.c helper.c inotify.c ip6addr.h diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index 5a301b2fb..4e144498d 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -322,7 +322,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, else log_query(F_NOEXTRA | F_DNSSEC | F_IPV6, "retry", (union all_addr *)&forward->sentto->addr.in6.sin6_addr, "dnssec"); - FTL_forwarding_retried(forward->sentto, forward->log_id, daemon->log_id, true); + FTL_forwarding_retried(forward->sentto, forward->frec_src.log_id, daemon->log_id, true); if (forward->sentto->sfd) fd = forward->sentto->sfd->fd; @@ -359,7 +359,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, start = daemon->servers; /* at end of list, recycle */ header->id = htons(forward->new_id); - FTL_forwarding_retried(forward->sentto, forward->log_id, daemon->log_id, false); + FTL_forwarding_retried(forward->sentto, forward->frec_src.log_id, daemon->log_id, false); } else { From 845890fea1a4de0e427c7fb1a4d9f8f8c4252b12 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 19 Jan 2021 10:34:11 +0100 Subject: [PATCH 19/21] Update dnsmasq version string Signed-off-by: DL6ER --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 066847517..99b5ffe20 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,6 @@ cmake_minimum_required(VERSION 2.8.12) project(PIHOLE_FTL C) -set(DNSMASQ_VERSION pi-hole-2.82) +set(DNSMASQ_VERSION pi-hole-2.83) add_subdirectory(src) From 8e84016f541ee05b0973c23d710809f758b49e66 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Fri, 15 Jan 2021 21:53:29 +0000 Subject: [PATCH 20/21] Fix warning message logic. Signed-off-by: DL6ER --- src/dnsmasq/hash_questions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dnsmasq/hash_questions.c b/src/dnsmasq/hash_questions.c index 917c18e66..51d88c28d 100644 --- a/src/dnsmasq/hash_questions.c +++ b/src/dnsmasq/hash_questions.c @@ -43,7 +43,7 @@ unsigned char *hash_questions(struct dns_header *header, size_t plen, char *name static unsigned char dummy[HASH_SIZE]; static int warned = 0; - if (warned) + if (!warned) my_syslog(LOG_ERR, _("Failed to create SHA-256 hash object")); warned = 1; From 1ecb3ff96e7c8f62b0c5e6527d03681b889b5925 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Fri, 15 Jan 2021 22:21:52 +0000 Subject: [PATCH 21/21] Update to new struct frec fields in conntrack code. Signed-off-by: DL6ER --- src/dnsmasq/forward.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index 4e144498d..5deb386b8 100644 --- a/src/dnsmasq/forward.c +++ b/src/dnsmasq/forward.c @@ -551,7 +551,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr, if (option_bool(OPT_CONNTRACK)) { unsigned int mark; - if (get_incoming_mark(&forward->source, &forward->dest, 0, &mark)) + if (get_incoming_mark(&forward->frec_src.source, &forward->frec_src.dest, 0, &mark)) setsockopt(fd, SOL_SOCKET, SO_MARK, &mark, sizeof(unsigned int)); } #endif @@ -1229,7 +1229,7 @@ void reply_query(int fd, int family, time_t now) if (option_bool(OPT_CONNTRACK)) { unsigned int mark; - if (get_incoming_mark(&orig->source, &orig->dest, 0, &mark)) + if (get_incoming_mark(&orig->frec_src.source, &orig->frec_src.dest, 0, &mark)) setsockopt(fd, SOL_SOCKET, SO_MARK, &mark, sizeof(unsigned int)); } #endif