From a24e1874869f66bf60027a7711f266aa76fd6ba5 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 26 Mar 2022 07:33:28 +0100 Subject: [PATCH 1/2] Add individual reply counters as well as their sum to the API + ensure we subtract from the previous reply counter when setting a new status Signed-off-by: DL6ER --- src/api/api.c | 10 +++++-- src/dnsmasq_interface.c | 65 ++++++++++++++++++----------------------- test/test_suite.bats | 25 +++++++++++----- 3 files changed, 53 insertions(+), 47 deletions(-) diff --git a/src/api/api.c b/src/api/api.c index 8f3d575ce..56739ae99 100644 --- a/src/api/api.c +++ b/src/api/api.c @@ -121,9 +121,13 @@ void getStats(const int *sock) ssend(*sock, "dns_queries_all_types %i\n", sumalltypes); // Send individual reply type counters - ssend(*sock, "reply_NODATA %i\nreply_NXDOMAIN %i\nreply_CNAME %i\nreply_IP %i\n", - counters->reply[REPLY_NODATA], counters->reply[REPLY_NXDOMAIN], - counters->reply[REPLY_CNAME], counters->reply[REPLY_IP]); + int sumallreplies = 0; + for(enum reply_type reply = REPLY_UNKNOWN; reply < QUERY_REPLY_MAX; reply++) + { + ssend(*sock, "reply_%s %i\n", get_query_reply_str(reply), counters->reply[reply]); + sumallreplies += counters->reply[reply]; + } + ssend(*sock, "dns_queries_all_replies %i\n", sumallreplies); ssend(*sock, "privacy_level %i\n", config.privacylevel); } else diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 5881020d1..9f58acbe6 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -50,7 +50,6 @@ #include "database/message-table.h" // Private prototypes -static const char *reply_status_str[QUERY_REPLY_MAX+1]; static void print_flags(const unsigned int flags); #define query_set_reply(flags, type, addr, query, response) _query_set_reply(flags, type, addr, query, response, __FILE__, __LINE__) static void _query_set_reply(const unsigned int flags, const enum reply_type reply, const union all_addr *addr, queriesData* query, @@ -720,6 +719,7 @@ bool _FTL_new_query(const unsigned int flags, const char *name, query->flags.response_calculated = false; // Initialize reply type query->reply = REPLY_UNKNOWN; + counters->reply[REPLY_UNKNOWN]++; // Store DNSSEC result for this domain query->dnssec = DNSSEC_UNSPECIFIED; // Every domain is insecure in the beginning. It can get secure or bogus @@ -1449,7 +1449,7 @@ static bool _FTL_check_blocking(int queryID, int domainID, int clientID, const c logg("Blocking %s as %s is %s", domainstr, blockedDomain, blockingreason); if(force_next_DNS_reply != 0) - logg("Forcing next reply to %s", reply_status_str[force_next_DNS_reply]); + logg("Forcing next reply to %s", get_query_reply_str(force_next_DNS_reply)); } else if(db_okay) { @@ -2570,95 +2570,86 @@ void print_flags(const unsigned int flags) free(flagstr); } -static const char *reply_status_str[QUERY_REPLY_MAX+1] = { - "UNKNOWN", - "NODATA", - "NXDOMAIN", - "CNAME", - "IP", - "DOMAIN", - "RRNAME", - "SERVFAIL", - "REFUSED", - "NOTIMP", - "OTHER", - "DNSSEC", - "NONE", - "BLOB", - "MAX" -}; - static void _query_set_reply(const unsigned int flags, const enum reply_type reply, const union all_addr *addr, queriesData *query, const struct timeval response, const char *file, const int line) { + enum reply_type new_reply = REPLY_UNKNOWN; // If reply is set, we use it directly instead of interpreting the flags if(reply != 0) { - query->reply = reply; + new_reply = reply; } // else: Iterate through possible values by analyzing both the flags and the addr bits else if(flags & F_NEG || - (flags & F_NOERR && !(flags & (F_IPV4 | F_IPV6))) || // <-- FTL_make_answer() when no A or AAAA is added - force_next_DNS_reply == REPLY_NXDOMAIN || - force_next_DNS_reply == REPLY_NODATA) + (flags & F_NOERR && !(flags & (F_IPV4 | F_IPV6))) || // <-- FTL_make_answer() when no A or AAAA is added + force_next_DNS_reply == REPLY_NXDOMAIN || + force_next_DNS_reply == REPLY_NODATA) { if(flags & F_NXDOMAIN || force_next_DNS_reply == REPLY_NXDOMAIN) // NXDOMAIN - query->reply = REPLY_NXDOMAIN; + new_reply = REPLY_NXDOMAIN; else // NODATA(-IPv6) - query->reply = REPLY_NODATA; + new_reply = REPLY_NODATA; } else if(flags & F_CNAME) // - query->reply = REPLY_CNAME; + new_reply = REPLY_CNAME; else if(flags & F_REVERSE) // reserve lookup - query->reply = REPLY_DOMAIN; + new_reply = REPLY_DOMAIN; else if(flags & F_RRNAME) // TXT query - query->reply = REPLY_RRNAME; + new_reply = REPLY_RRNAME; else if((flags & F_RCODE && addr != NULL) || force_next_DNS_reply == REPLY_REFUSED) { if((addr != NULL && addr->log.rcode == REFUSED) || force_next_DNS_reply == REPLY_REFUSED ) { // REFUSED query - query->reply = REPLY_REFUSED; + new_reply = REPLY_REFUSED; } else if(addr != NULL && addr->log.rcode == SERVFAIL) { // SERVFAIL query - query->reply = REPLY_SERVFAIL; + new_reply = REPLY_SERVFAIL; } } else if(flags & F_KEYTAG) - query->reply = REPLY_DNSSEC; + new_reply = REPLY_DNSSEC; else if(force_next_DNS_reply == REPLY_NONE) { - query->reply = REPLY_NONE; + new_reply = REPLY_NONE; } else if(flags & (F_IPV4 | F_IPV6)) { // IP address - query->reply = REPLY_IP; + new_reply = REPLY_IP; } else { // Other binary, possibly proprietry, data - query->reply = REPLY_BLOB; + new_reply = REPLY_BLOB; } if(config.debug & DEBUG_QUERIES) { const char *path = short_path(file); - logg("Set reply to %s (%d) in %s:%d", reply_status_str[query->reply], query->reply, + logg("Set reply to %s (%d) in %s:%d", get_query_reply_str(query->reply), query->reply, path, line); + if(query->reply != REPLY_UNKNOWN && query->reply != new_reply) + logg("Reply of query %i was %s now changing to %s", query->id, + get_query_reply_str(query->reply), get_query_reply_str(new_reply)); } - counters->reply[query->reply]++; + // Subtract from old reply counter + counters->reply[query->reply]--; + // Add to new reply counter + counters->reply[new_reply]++; + // Store reply type + query->reply = new_reply; // Save response time // Skipped internally if already computed diff --git a/test/test_suite.bats b/test/test_suite.bats index 096e61a58..2a20a90d7 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -412,13 +412,24 @@ #[[ ${lines[8]} == "clients_ever_seen 8" ]] #[[ ${lines[9]} == "unique_clients 8" ]] [[ ${lines[10]} == "dns_queries_all_types 47" ]] - [[ ${lines[11]} == "reply_NODATA 0" ]] - [[ ${lines[12]} == "reply_NXDOMAIN 1" ]] - [[ ${lines[13]} == "reply_CNAME 5" ]] - [[ ${lines[14]} == "reply_IP 23" ]] - [[ ${lines[15]} == "privacy_level 0" ]] - [[ ${lines[16]} == "status enabled" ]] - [[ ${lines[17]} == "" ]] + [[ ${lines[11]} == "reply_UNKNOWN 0" ]] + [[ ${lines[12]} == "reply_NODATA 0" ]] + [[ ${lines[13]} == "reply_NXDOMAIN 1" ]] + [[ ${lines[14]} == "reply_CNAME 3" ]] + [[ ${lines[15]} == "reply_IP 23" ]] + [[ ${lines[16]} == "reply_DOMAIN 0" ]] + [[ ${lines[17]} == "reply_RRNAME 5" ]] + [[ ${lines[18]} == "reply_SERVFAIL 0" ]] + [[ ${lines[19]} == "reply_REFUSED 0" ]] + [[ ${lines[20]} == "reply_NOTIMP 0" ]] + [[ ${lines[21]} == "reply_OTHER 0" ]] + [[ ${lines[22]} == "reply_DNSSEC 5" ]] + [[ ${lines[23]} == "reply_NONE 0" ]] + [[ ${lines[24]} == "reply_BLOB 10" ]] + [[ ${lines[25]} == "dns_queries_all_replies 47" ]] + [[ ${lines[26]} == "privacy_level 0" ]] + [[ ${lines[27]} == "status enabled" ]] + [[ ${lines[28]} == "" ]] } # Here and below: It is not meaningful to assume a particular order From 8734b60a53b0ac3854dee5d6eaa21c41d8e36269 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 26 Mar 2022 13:05:30 +0100 Subject: [PATCH 2/2] Ensure we subtract from the old reply counter when reply type changes Signed-off-by: DL6ER --- src/database/query-table.c | 12 +++++++++--- src/dnsmasq_interface.c | 6 +++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/database/query-table.c b/src/database/query-table.c index 05d47857c..f7b025891 100644 --- a/src/database/query-table.c +++ b/src/database/query-table.c @@ -867,7 +867,14 @@ void DB_read_queries(void) if(type < 100) { // Mapped query type - query->type = type; + if(type >= TYPE_A && type < TYPE_MAX) + query->type = type; + else + { + // Invalid query type + logg("DB warn: Query type %d is invalid.", type); + continue; + } } else { @@ -900,8 +907,7 @@ void DB_read_queries(void) client->lastQuery = queryTimeStamp; // Handle type counters - if(type >= TYPE_A && type < TYPE_MAX) - counters->querytype[type-1]++; + counters->querytype[query->type-1]++; // Update overTime data overTime[timeidx].total++; diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 9f58acbe6..d973d68a4 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -663,9 +663,6 @@ bool _FTL_new_query(const unsigned int flags, const char *name, id, queryID, short_path(file), line); } - // Update counters - counters->querytype[querytype-1]++; - // Update overTime const unsigned int timeidx = getOverTimeID(querytimestamp); @@ -755,6 +752,9 @@ bool _FTL_new_query(const unsigned int flags, const char *name, client->lastQuery = querytimestamp; client->numQueriesARP++; + // Update counters + counters->querytype[querytype-1]++; + // Process interface information of client (if available) // Skip interface name length 1 to skip "-". No real interface should // have a name with a length of 1...