fix(pcap): don't widen explicit host_spec with the interface subnet (Refs #758)

cmd/pktvisor-reader/main.cpp

@@ -46,7 +46,9 @@ static const char USAGE[] =
       --geo-asn FILE        GeoLite2 ASN database to use for IP to ASN mapping (if enabled)
       -H HOSTSPEC           Specify subnets (comma separated) to consider HOST, in CIDR form. In live capture this /may/ be detected automatically
                             from capture device but /must/ be specified for pcaps. Example: "10.0.1.0/24,10.0.2.1/32,2001:db8::/64"
-                            Specifying this for live capture will append to any automatic detection.
+                            For live capture, specifying this defines the host set explicitly and
+                            disables automatic detection from the capture device; list every address
+                            family (IPv4 and IPv6) you need.
 )";
 
 namespace {

cmd/pktvisord/main.cpp

@@ -111,7 +111,9 @@ static const char USAGE[] =
       -H HOSTSPEC                           Specify subnets (comma separated) to consider HOST, in CIDR form. In live capture this
                                             /may/ be detected automatically from capture device but /must/ be specified for pcaps.
                                             Example: "10.0.1.0/24,10.0.2.1/32,2001:db8::/64"
-                                            Specifying this for live capture will append to any automatic detection.
+                                            For live capture, specifying this defines the host set explicitly and
+                                            disables automatic detection from the capture device; list every address
+                                            family (IPv4 and IPv6) you need.
 )";
 
 namespace {

libs/visor_utils/test_utils.cpp

@@ -103,3 +103,50 @@ TEST_CASE("parseHostSpec", "[utils]")
     }
 }
 
+// `str` is the canonical inet_ntop rendering of the binary address, not an echo of
+// any input string; the inputs below are already canonical so they round-trip. A
+// future non-canonical input (e.g. "2001:db8:0:0::1") would need its canonical form.
+// (std::pair brace-init resolves via utils.h, which gains #include <utility> in this task.)
+TEST_CASE("appendInterfaceHostSubnets", "[utils]")
+{
+    in_addr a4{};
+    REQUIRE(inet_pton(AF_INET, "192.168.50.23", &a4) == 1);
+    in6_addr a6{};
+    REQUIRE(inet_pton(AF_INET6, "2001:db8::1", &a6) == 1);
+
+    SECTION("skips interface hosts when a host set is already provided")
+    {
+        IPv4subnetList hostIPv4;
+        IPv6subnetList hostIPv6;
+        append_interface_host_subnets(true, {{a4, 24}}, {{a6, 64}}, hostIPv4, hostIPv6);
+        CHECK(hostIPv4.empty());
+        CHECK(hostIPv6.empty());
+    }
+
+    SECTION("appends interface hosts at their prefix when no host set provided")
+    {
+        IPv4subnetList hostIPv4;
+        IPv6subnetList hostIPv6;
+        append_interface_host_subnets(false, {{a4, 24}}, {{a6, 64}}, hostIPv4, hostIPv6);
+        REQUIRE(hostIPv4.size() == 1);
+        CHECK(hostIPv4[0].cidr == 24);
+        CHECK(hostIPv4[0].str == "192.168.50.23/24");
+        REQUIRE(hostIPv6.size() == 1);
+        CHECK(hostIPv6[0].cidr == 64);
+        CHECK(hostIPv6[0].str == "2001:db8::1/64");
+    }
+
+    SECTION("appends multiple addresses of the same family in order")
+    {
+        in_addr b4{};
+        REQUIRE(inet_pton(AF_INET, "10.0.0.1", &b4) == 1);
+        IPv4subnetList hostIPv4;
+        IPv6subnetList hostIPv6;
+        append_interface_host_subnets(false, {{a4, 24}, {b4, 32}}, {}, hostIPv4, hostIPv6);
+        REQUIRE(hostIPv4.size() == 2);
+        CHECK(hostIPv4[0].str == "192.168.50.23/24");
+        CHECK(hostIPv4[1].str == "10.0.0.1/32");
+        CHECK(hostIPv6.empty());
+    }
+}
+

libs/visor_utils/utils.cpp

@@ -4,6 +4,12 @@
 #include <pcapplusplus/IpUtils.h>
 #include <fmt/format.h>
 #include <sstream>
+#include <utility> // std::pair
+#ifdef _WIN32
+#include <ws2tcpip.h>
+#else
+#include <arpa/inet.h>
+#endif
 
 namespace visor::lib::utils {
 
@@ -162,6 +168,34 @@ void parse_host_specs(const std::vector<std::string> &host_list, IPv4subnetList
     }
 }
 
+void append_interface_host_subnets(
+    bool host_set_provided,
+    const std::vector<std::pair<in_addr, uint8_t>> &v4_addrs,
+    const std::vector<std::pair<in6_addr, uint8_t>> &v6_addrs,
+    IPv4subnetList &host_ipv4,
+    IPv6subnetList &host_ipv6)
+{
+    // An explicit host_spec is authoritative: do not widen it with the capture
+    // interface's own address/subnet. (When no host_spec is given, the interface
+    // subnet is added on purpose, to distinguish inside vs. outside traffic.)
+    if (host_set_provided) {
+        return;
+    }
+    char buf[INET6_ADDRSTRLEN];
+    for (const auto &[addr, prefix] : v4_addrs) {
+        if (inet_ntop(AF_INET, &addr, buf, sizeof(buf)) == nullptr) {
+            continue;
+        }
+        host_ipv4.push_back({addr, prefix, std::string(buf) + "/" + std::to_string(prefix)});
+    }
+    for (const auto &[addr, prefix] : v6_addrs) {
+        if (inet_ntop(AF_INET6, &addr, buf, sizeof(buf)) == nullptr) {
+            continue;
+        }
+        host_ipv6.push_back({addr, prefix, std::string(buf) + "/" + std::to_string(prefix)});
+    }
+}
+
 std::vector<std::string> split_str_to_vec_str(const std::string &spec, const char &delimiter)
 {
     std::vector<std::string> elems;

libs/visor_utils/utils.h

@@ -23,6 +23,7 @@
 #include <optional>
 #include <stdexcept>
 #include <string>
+#include <utility>
 #include <vector>
 
 namespace visor::lib::utils {
@@ -60,6 +61,7 @@ bool ipv6_to_sockaddr(const pcpp::IPv6Address &ip, struct sockaddr_in6 *sa);
 
 std::vector<std::string> split_str_to_vec_str(const std::string &spec, const char &delimiter);
 void parse_host_specs(const std::vector<std::string> &host_list, IPv4subnetList &ipv4_list, IPv6subnetList &ipv6_list);
+void append_interface_host_subnets(bool host_set_provided, const std::vector<std::pair<in_addr, uint8_t>> &v4_addrs, const std::vector<std::pair<in6_addr, uint8_t>> &v6_addrs, IPv4subnetList &host_ipv4, IPv6subnetList &host_ipv6);
 std::optional<IPv4subnetList::const_iterator> match_subnet(IPv4subnetList &ipv4_list, uint32_t ipv4_val);
 std::optional<IPv6subnetList::const_iterator> match_subnet(IPv6subnetList &ipv6_list, const uint8_t *ipv6_val);
 bool match_subnet(IPv4subnetList &ipv4_list, IPv6subnetList &ipv6_list, const std::string &ip_val);

src/inputs/pcap/PcapInputStream.cpp

@@ -30,6 +30,8 @@
 #include <cstdint>
 #include <pcapplusplus/IpUtils.h>
 #include <sstream>
+#include <utility>
+#include <vector>
 
 using namespace std::chrono;
 
@@ -577,29 +579,36 @@ void PcapInputStream::_open_libpcap_iface(const std::string &bpfFilter)
 void PcapInputStream::_get_hosts_from_libpcap_iface()
 {
 #ifndef _WIN32
+    // An explicit host_spec is authoritative; skip interface auto-detection
+    // entirely (don't even call getifaddrs) when parse_host_spec() — run earlier
+    // in start() — already populated the host set. We gate on the parsed lists,
+    // NOT config_exists("host_spec"): the CLI default tap always sets host_spec
+    // (empty string when -H is omitted), so config_exists would be true for a
+    // plain `pktvisord <iface>` and wrongly skip auto-detection.
+    if (!_hostIPv4.empty() || !_hostIPv6.empty()) {
+        return;
+    }
     ifaddrs *ifap = nullptr;
     if (getifaddrs(&ifap) != 0 || ifap == nullptr) {
         return;
     }
     const std::string devName = _pcapDevice->getName();
+    std::vector<std::pair<in_addr, uint8_t>> v4_addrs;
+    std::vector<std::pair<in6_addr, uint8_t>> v6_addrs;
     for (ifaddrs *i = ifap; i != nullptr; i = i->ifa_next) {
         if (i->ifa_addr == nullptr || i->ifa_name == nullptr || devName != i->ifa_name) {
             continue;
         }
-        char buf[INET6_ADDRSTRLEN];
         if (i->ifa_addr->sa_family == AF_INET) {
             auto ip4 = reinterpret_cast<sockaddr_in *>(i->ifa_addr);
-            inet_ntop(AF_INET, &ip4->sin_addr, buf, sizeof(buf));
             uint8_t prefix = 32;
             if (i->ifa_netmask) {
                 auto nm = reinterpret_cast<sockaddr_in *>(i->ifa_netmask);
                 prefix = static_cast<uint8_t>(__builtin_popcount(ntohl(nm->sin_addr.s_addr)));
             }
-            std::string cidr = std::string(buf) + "/" + std::to_string(prefix);
-            _hostIPv4.push_back({ip4->sin_addr, prefix, cidr});
+            v4_addrs.emplace_back(ip4->sin_addr, prefix);
         } else if (i->ifa_addr->sa_family == AF_INET6) {
             auto ip6 = reinterpret_cast<sockaddr_in6 *>(i->ifa_addr);
-            inet_ntop(AF_INET6, &ip6->sin6_addr, buf, sizeof(buf));
             uint8_t prefix = 128;
             if (i->ifa_netmask) {
                 auto nm6 = reinterpret_cast<sockaddr_in6 *>(i->ifa_netmask);
@@ -608,11 +617,18 @@ void PcapInputStream::_get_hosts_from_libpcap_iface()
                     prefix += static_cast<uint8_t>(__builtin_popcount(nm6->sin6_addr.s6_addr[b]));
                 }
             }
-            std::string cidr = std::string(buf) + "/" + std::to_string(prefix);
-            _hostIPv6.push_back({ip6->sin6_addr, prefix, cidr});
+            v6_addrs.emplace_back(ip6->sin6_addr, prefix);
         }
     }
     freeifaddrs(ifap);
+
+    // The early return above already skipped getifaddrs when a host set was
+    // present, so this point is only reached with no explicit host_spec. Pass the
+    // computed flag (not a hard-coded false) so the helper enforces the same skip
+    // independently — defense-in-depth that keeps an explicit host_spec
+    // authoritative even if the early return is ever changed.
+    const bool host_set_provided = !_hostIPv4.empty() || !_hostIPv6.empty();
+    lib::utils::append_interface_host_subnets(host_set_provided, v4_addrs, v6_addrs, _hostIPv4, _hostIPv6);
 #endif
 }
 
@@ -656,6 +672,14 @@ std::unique_ptr<InputEventProxy> PcapInputStream::create_event_proxy(const Confi
 
 void PcapInputStream::parse_host_spec()
 {
+    // Recompute the host set from scratch on every call. start() calls this each
+    // time, and stop() does not clear these lists, so without this a stopped/
+    // restarted stream would accumulate stale entries: a no-host_spec restart
+    // would keep the previous start's auto-detected interface subnets (and the
+    // _get_hosts_from_libpcap_iface() early return would then skip refreshing
+    // them via getifaddrs), and a host_spec restart would duplicate entries.
+    _hostIPv4.clear();
+    _hostIPv6.clear();
     if (config_exists("host_spec")) {
         lib::utils::parse_host_specs(lib::utils::split_str_to_vec_str(config_get<std::string>("host_spec"), ','),
             _hostIPv4, _hostIPv6);