Feature/adjust qname aggregation (#242)

* Threat static sufix on DNS Qnames

* compare without casting

* add only_qname_suffix validation on unit tests

* replace string_view with size_t for suffix

* Fix aggregation tests

Author: leoparente

src/handlers/dns/DnsStreamHandler.cpp

@@ -163,7 +163,8 @@ void DnsStreamHandler::process_udp_packet_cb(pcpp::Packet &payload, PacketDirect
     if (metric_port) {
         DnsLayer dnsLayer(udpLayer, &payload);
         if (!_filtering(dnsLayer, dir, l3, pcpp::UDP, metric_port, stamp)) {
-            _metrics->process_dns_layer(dnsLayer, dir, l3, pcpp::UDP, flowkey, metric_port, stamp);
+            _metrics->process_dns_layer(dnsLayer, dir, l3, pcpp::UDP, flowkey, metric_port, _static_suffix_size, stamp);
+            _static_suffix_size = 0;
             // signal for chained stream handlers, if we have any
             udp_signal(payload, dir, l3, flowkey, stamp);
         }
@@ -243,7 +244,8 @@ void DnsStreamHandler::tcp_message_ready_cb(int8_t side, const pcpp::TcpStreamDa
         pcpp::Packet dummy_packet;
         DnsLayer dnsLayer(data.get(), size, nullptr, &dummy_packet);
         if (!_filtering(dnsLayer, dir, l3Type, pcpp::UDP, port, stamp)) {
-            _metrics->process_dns_layer(dnsLayer, dir, l3Type, pcpp::TCP, flowKey, port, stamp);
+            _metrics->process_dns_layer(dnsLayer, dir, l3Type, pcpp::TCP, flowKey, port, _static_suffix_size, stamp);
+            _static_suffix_size = 0;
         }
         // data is freed upon return
     };
@@ -319,9 +321,10 @@ bool DnsStreamHandler::_filtering(DnsLayer &payload, [[maybe_unused]] PacketDire
         std::string qname_ci{payload.getFirstQuery()->getName()};
         std::transform(qname_ci.begin(), qname_ci.end(), qname_ci.begin(),
             [](unsigned char c) { return std::tolower(c); });
-        for (auto fqn : _f_qnames) {
+        for (const auto &fqn : _f_qnames) {
             // if it matched, we know we are not filtering
             if (endsWith(qname_ci, fqn)) {
+                _static_suffix_size = fqn.size();
                 goto will_not_filter;
             }
         }
@@ -532,7 +535,7 @@ void DnsMetricsBucket::process_dnstap(bool deep, const dnstap::Dnstap &payload)
         process_dns_layer(deep, dpayload, l3, l4, port);
     }
 }
-void DnsMetricsBucket::process_dns_layer(bool deep, DnsLayer &payload, pcpp::ProtocolType l3, Protocol l4, uint16_t port)
+void DnsMetricsBucket::process_dns_layer(bool deep, DnsLayer &payload, pcpp::ProtocolType l3, Protocol l4, uint16_t port, size_t suffix_size)
 {
     std::unique_lock lock(_mutex);
 
@@ -626,7 +629,7 @@ void DnsMetricsBucket::process_dns_layer(bool deep, DnsLayer &payload, pcpp::Pro
                 }
             }
 
-            auto aggDomain = aggregateDomain(name);
+            auto aggDomain = aggregateDomain(name, suffix_size);
             _dns_topQname2.update(std::string(aggDomain.first));
             if (aggDomain.second.size()) {
                 _dns_topQname3.update(std::string(aggDomain.second));
@@ -788,12 +791,12 @@ void DnsMetricsBucket::process_filtered()
 }
 
 // the general metrics manager entry point (both UDP and TCP)
-void DnsMetricsManager::process_dns_layer(DnsLayer &payload, PacketDirection dir, pcpp::ProtocolType l3, pcpp::ProtocolType l4, uint32_t flowkey, uint16_t port, timespec stamp)
+void DnsMetricsManager::process_dns_layer(DnsLayer &payload, PacketDirection dir, pcpp::ProtocolType l3, pcpp::ProtocolType l4, uint32_t flowkey, uint16_t port, size_t suffix_size, timespec stamp)
 {
     // base event
     new_event(stamp);
     // process in the "live" bucket. this will parse the resources if we are deep sampling
-    live_bucket()->process_dns_layer(_deep_sampling_now, payload, l3, static_cast<Protocol>(l4), port);
+    live_bucket()->process_dns_layer(_deep_sampling_now, payload, l3, static_cast<Protocol>(l4), port, suffix_size);
 
     if (group_enabled(group::DnsMetrics::DnsTransactions)) {
         // handle dns transactions (query/response pairs)

src/handlers/dns/DnsStreamHandler.h

@@ -158,7 +158,7 @@ class DnsMetricsBucket final : public visor::AbstractMetricsBucket
     void to_prometheus(std::stringstream &out, Metric::LabelMap add_labels = {}) const override;
 
     void process_filtered();
-    void process_dns_layer(bool deep, DnsLayer &payload, pcpp::ProtocolType l3, Protocol l4, uint16_t port);
+    void process_dns_layer(bool deep, DnsLayer &payload, pcpp::ProtocolType l3, Protocol l4, uint16_t port, size_t suffix_size = 0);
     void process_dns_layer(pcpp::ProtocolType l3, Protocol l4, QR side, uint16_t port);
     void process_dnstap(bool deep, const dnstap::Dnstap &payload);
 
@@ -200,7 +200,7 @@ class DnsMetricsManager final : public visor::AbstractMetricsManager<DnsMetricsB
     }
 
     void process_filtered(timespec stamp);
-    void process_dns_layer(DnsLayer &payload, PacketDirection dir, pcpp::ProtocolType l3, pcpp::ProtocolType l4, uint32_t flowkey, uint16_t port, timespec stamp);
+    void process_dns_layer(DnsLayer &payload, PacketDirection dir, pcpp::ProtocolType l3, pcpp::ProtocolType l4, uint32_t flowkey, uint16_t port, size_t suffix_size, timespec stamp);
     void process_dnstap(const dnstap::Dnstap &payload, bool filtered);
 };
 
@@ -287,6 +287,7 @@ class DnsStreamHandler final : public visor::StreamMetricsHandler<DnsMetricsMana
     std::bitset<Filters::FiltersMAX> _f_enabled;
     uint16_t _f_rcode{0};
     std::vector<std::string> _f_qnames;
+    size_t _static_suffix_size{0};
     std::bitset<DNSTAP_TYPE_SIZE> _f_dnstap_types;
 
     static const inline StreamMetricsHandler::GroupDefType _group_defs = {

src/handlers/dns/dns.cpp

@@ -6,7 +6,7 @@
 
 namespace visor::handler::dns {
 
-AggDomainResult aggregateDomain(const std::string &domain)
+AggDomainResult aggregateDomain(const std::string &domain, size_t suffix_size)
 {
 
     std::string_view qname2(domain);
@@ -18,7 +18,9 @@ AggDomainResult aggregateDomain(const std::string &domain)
         return AggDomainResult(qname2, qname3);
     }
     std::size_t endDot = std::string::npos;
-    if (domain.back() == '.') {
+    if (suffix_size > 0 && domain.size() > suffix_size) {
+        endDot = domain.size() - suffix_size;
+    } else if (domain.back() == '.') {
         endDot = domain.size() - 2;
     }
     auto first_dot = domain.rfind('.', endDot);

src/handlers/dns/dns.h

@@ -13,7 +13,7 @@
 namespace visor::handler::dns {
 
 typedef std::pair<std::string_view, std::string_view> AggDomainResult;
-AggDomainResult aggregateDomain(const std::string &domain);
+AggDomainResult aggregateDomain(const std::string &domain, size_t suffix_size = 0);
 
 enum QR {
     query = 0,

src/handlers/dns/tests/test_dns.cpp

@@ -72,4 +72,47 @@ TEST_CASE("DNS Utilities", "[dns]")
         CHECK(result.first == ".b.c");
         CHECK(result.second == "");
     }
+
+    SECTION("aggregateDomain with static suffix")
+    {
+        AggDomainResult result;
+        std::string domain;
+        std::string static_suffix;
+
+        domain = "biz.foo.bar.com";
+        static_suffix = ".bar.com";
+        result = aggregateDomain(domain, static_suffix.size());
+        CHECK(result.first == ".foo.bar.com");
+        CHECK(result.second == "biz.foo.bar.com");
+
+        domain = "biz.foo.bar.com";
+        static_suffix = "bar.com";
+        result = aggregateDomain(domain, static_suffix.size());
+        CHECK(result.first == ".foo.bar.com");
+        CHECK(result.second == "biz.foo.bar.com");
+
+        domain = "biz.foo.bar.com";
+        static_suffix = "foo.bar.com";
+        result = aggregateDomain(domain, static_suffix.size());
+        CHECK(result.first == "biz.foo.bar.com");
+        CHECK(result.second == "");
+
+        domain = "foo.bar.com.";
+        static_suffix = "biz.foo.bar.com";
+        result = aggregateDomain(domain, static_suffix.size());
+        CHECK(result.first == ".bar.com.");
+        CHECK(result.second == "foo.bar.com.");
+
+        domain = "www.google.co.uk";
+        static_suffix = ".co.uk";
+        result = aggregateDomain(domain, static_suffix.size());
+        CHECK(result.first == ".google.co.uk");
+        CHECK(result.second == "www.google.co.uk");
+
+        domain = "www.google.co.uk";
+        static_suffix = "google.co.uk";
+        result = aggregateDomain(domain, static_suffix.size());
+        CHECK(result.first == "www.google.co.uk");
+        CHECK(result.second == "");
+    }
 }

src/handlers/dns/tests/test_dns_layer.cpp

@@ -374,6 +374,12 @@ TEST_CASE("DNS Filters: only_qname_suffix", "[pcap][dns]")
     CHECK(counters.REFUSED.value() == 0);
     CHECK(counters.NX.value() == 1);
     CHECK(counters.filtered.value() == 14);
+
+    nlohmann::json j;
+    dns_handler.metrics()->bucket(0)->to_json(j);
+
+    CHECK(j["top_qname2"][0]["name"].get<std::string>().find("google.com") != std::string::npos);
+    CHECK(j["top_qname3"][0]["name"] == nullptr);
 }
 
 TEST_CASE("DNS groups", "[pcap][dns]")