From f164e4221301d6251167ec9f479c60e7ffc234ba Mon Sep 17 00:00:00 2001 From: Leo Parente <23251360+leoparente@users.noreply.github.com> Date: Fri, 26 Jun 2026 13:52:03 -0300 Subject: [PATCH 1/6] feat(utils): add append_interface_host_subnets() helper for pcap host detection (Refs #758) --- libs/visor_utils/test_utils.cpp | 47 +++++++++++++++++++++++++++++++++ libs/visor_utils/utils.cpp | 30 +++++++++++++++++++++ libs/visor_utils/utils.h | 2 ++ 3 files changed, 79 insertions(+) diff --git a/libs/visor_utils/test_utils.cpp b/libs/visor_utils/test_utils.cpp index 7b5dce792..0d02257b2 100644 --- a/libs/visor_utils/test_utils.cpp +++ b/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 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()); + } +} + diff --git a/libs/visor_utils/utils.cpp b/libs/visor_utils/utils.cpp index da4e38cfe..ca3dba699 100644 --- a/libs/visor_utils/utils.cpp +++ b/libs/visor_utils/utils.cpp @@ -4,6 +4,12 @@ #include #include #include +#include // std::pair +#ifdef _WIN32 +#include +#else +#include +#endif namespace visor::lib::utils { @@ -162,6 +168,30 @@ void parse_host_specs(const std::vector &host_list, IPv4subnetList } } +void append_interface_host_subnets( + bool host_set_provided, + const std::vector> &v4_addrs, + const std::vector> &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) { + inet_ntop(AF_INET, &addr, buf, sizeof(buf)); + host_ipv4.push_back({addr, prefix, std::string(buf) + "/" + std::to_string(prefix)}); + } + for (const auto &[addr, prefix] : v6_addrs) { + inet_ntop(AF_INET6, &addr, buf, sizeof(buf)); + host_ipv6.push_back({addr, prefix, std::string(buf) + "/" + std::to_string(prefix)}); + } +} + std::vector split_str_to_vec_str(const std::string &spec, const char &delimiter) { std::vector elems; diff --git a/libs/visor_utils/utils.h b/libs/visor_utils/utils.h index 28c6053b9..68101302e 100644 --- a/libs/visor_utils/utils.h +++ b/libs/visor_utils/utils.h @@ -23,6 +23,7 @@ #include #include #include +#include #include namespace visor::lib::utils { @@ -60,6 +61,7 @@ bool ipv6_to_sockaddr(const pcpp::IPv6Address &ip, struct sockaddr_in6 *sa); std::vector split_str_to_vec_str(const std::string &spec, const char &delimiter); void parse_host_specs(const std::vector &host_list, IPv4subnetList &ipv4_list, IPv6subnetList &ipv6_list); +void append_interface_host_subnets(bool host_set_provided, const std::vector> &v4_addrs, const std::vector> &v6_addrs, IPv4subnetList &host_ipv4, IPv6subnetList &host_ipv6); std::optional match_subnet(IPv4subnetList &ipv4_list, uint32_t ipv4_val); std::optional match_subnet(IPv6subnetList &ipv6_list, const uint8_t *ipv6_val); bool match_subnet(IPv4subnetList &ipv4_list, IPv6subnetList &ipv6_list, const std::string &ip_val); From 1c74d32a9cca0cda73491330c61b8f44537de266 Mon Sep 17 00:00:00 2001 From: Leo Parente <23251360+leoparente@users.noreply.github.com> Date: Fri, 26 Jun 2026 13:52:03 -0300 Subject: [PATCH 2/6] fix(pcap): don't widen explicit host_spec with the interface subnet (Refs #758) --- src/inputs/pcap/PcapInputStream.cpp | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/inputs/pcap/PcapInputStream.cpp b/src/inputs/pcap/PcapInputStream.cpp index 1ca906c1b..94aa5b7a2 100644 --- a/src/inputs/pcap/PcapInputStream.cpp +++ b/src/inputs/pcap/PcapInputStream.cpp @@ -30,6 +30,8 @@ #include #include #include +#include +#include using namespace std::chrono; @@ -582,24 +584,22 @@ void PcapInputStream::_get_hosts_from_libpcap_iface() return; } const std::string devName = _pcapDevice->getName(); + std::vector> v4_addrs; + std::vector> 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(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(i->ifa_netmask); prefix = static_cast(__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(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(i->ifa_netmask); @@ -608,11 +608,19 @@ void PcapInputStream::_get_hosts_from_libpcap_iface() prefix += static_cast(__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); + + // An explicit host_spec is authoritative; only auto-add interface hosts when + // parse_host_spec() (run earlier in start()) did NOT already populate a host + // set. Do NOT gate on 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 ` and wrongly skip auto-detection, + // leaving the host set empty and classifying every packet as unknown. + 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 } From 67669d6969f484a248d07b312a705ca53eb7f352 Mon Sep 17 00:00:00 2001 From: Leo Parente <23251360+leoparente@users.noreply.github.com> Date: Fri, 26 Jun 2026 13:52:03 -0300 Subject: [PATCH 3/6] docs(cli): -H replaces interface host auto-detection for live capture (Refs #758) --- cmd/pktvisor-reader/main.cpp | 4 +++- cmd/pktvisord/main.cpp | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/pktvisor-reader/main.cpp b/cmd/pktvisor-reader/main.cpp index 3c4948a9a..c4a1f1bd6 100644 --- a/cmd/pktvisor-reader/main.cpp +++ b/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 { diff --git a/cmd/pktvisord/main.cpp b/cmd/pktvisord/main.cpp index 0b451b90d..845eb944f 100644 --- a/cmd/pktvisord/main.cpp +++ b/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 { From 20281296936f241c7b97a1150f1c238a3ffb7dc5 Mon Sep 17 00:00:00 2001 From: Leo Parente <23251360+leoparente@users.noreply.github.com> Date: Fri, 26 Jun 2026 14:24:49 -0300 Subject: [PATCH 4/6] fix(pcap): early-return before getifaddrs when host_spec set; guard inet_ntop (Refs #758) --- libs/visor_utils/utils.cpp | 8 ++++++-- src/inputs/pcap/PcapInputStream.cpp | 21 +++++++++++++-------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/libs/visor_utils/utils.cpp b/libs/visor_utils/utils.cpp index ca3dba699..6af552b09 100644 --- a/libs/visor_utils/utils.cpp +++ b/libs/visor_utils/utils.cpp @@ -183,11 +183,15 @@ void append_interface_host_subnets( } char buf[INET6_ADDRSTRLEN]; for (const auto &[addr, prefix] : v4_addrs) { - inet_ntop(AF_INET, &addr, buf, sizeof(buf)); + 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) { - inet_ntop(AF_INET6, &addr, buf, sizeof(buf)); + if (inet_ntop(AF_INET6, &addr, buf, sizeof(buf)) == nullptr) { + continue; + } host_ipv6.push_back({addr, prefix, std::string(buf) + "/" + std::to_string(prefix)}); } } diff --git a/src/inputs/pcap/PcapInputStream.cpp b/src/inputs/pcap/PcapInputStream.cpp index 94aa5b7a2..3c5ae3c8c 100644 --- a/src/inputs/pcap/PcapInputStream.cpp +++ b/src/inputs/pcap/PcapInputStream.cpp @@ -579,6 +579,15 @@ 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 ` and wrongly skip auto-detection. + if (!_hostIPv4.empty() || !_hostIPv6.empty()) { + return; + } ifaddrs *ifap = nullptr; if (getifaddrs(&ifap) != 0 || ifap == nullptr) { return; @@ -613,14 +622,10 @@ void PcapInputStream::_get_hosts_from_libpcap_iface() } freeifaddrs(ifap); - // An explicit host_spec is authoritative; only auto-add interface hosts when - // parse_host_spec() (run earlier in start()) did NOT already populate a host - // set. Do NOT gate on 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 ` and wrongly skip auto-detection, - // leaving the host set empty and classifying every packet as unknown. - const bool host_set_provided = !_hostIPv4.empty() || !_hostIPv6.empty(); - lib::utils::append_interface_host_subnets(host_set_provided, v4_addrs, v6_addrs, _hostIPv4, _hostIPv6); + // No explicit host_spec (guaranteed by the early return above): auto-add the + // interface's own addresses at their subnet prefix so direction classification + // works out of the box. The helper also self-guards on this flag. + lib::utils::append_interface_host_subnets(false, v4_addrs, v6_addrs, _hostIPv4, _hostIPv6); #endif } From df50edac73767538a495a00d1c128a9df2bf172c Mon Sep 17 00:00:00 2001 From: Leo Parente <23251360+leoparente@users.noreply.github.com> Date: Fri, 26 Jun 2026 15:48:27 -0300 Subject: [PATCH 5/6] fix(pcap): recompute host set on each parse_host_spec to avoid stale hosts on restart (Refs #758) --- src/inputs/pcap/PcapInputStream.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/inputs/pcap/PcapInputStream.cpp b/src/inputs/pcap/PcapInputStream.cpp index 3c5ae3c8c..ecd3991b5 100644 --- a/src/inputs/pcap/PcapInputStream.cpp +++ b/src/inputs/pcap/PcapInputStream.cpp @@ -669,6 +669,14 @@ std::unique_ptr 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("host_spec"), ','), _hostIPv4, _hostIPv6); From e9be5cbcf956d768b5447a9e8069e7b566a2d757 Mon Sep 17 00:00:00 2001 From: Leo Parente <23251360+leoparente@users.noreply.github.com> Date: Fri, 26 Jun 2026 16:12:23 -0300 Subject: [PATCH 6/6] fix(pcap): pass computed host_set_provided to helper for defense-in-depth (Refs #758) --- src/inputs/pcap/PcapInputStream.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/inputs/pcap/PcapInputStream.cpp b/src/inputs/pcap/PcapInputStream.cpp index ecd3991b5..5deb2f40b 100644 --- a/src/inputs/pcap/PcapInputStream.cpp +++ b/src/inputs/pcap/PcapInputStream.cpp @@ -622,10 +622,13 @@ void PcapInputStream::_get_hosts_from_libpcap_iface() } freeifaddrs(ifap); - // No explicit host_spec (guaranteed by the early return above): auto-add the - // interface's own addresses at their subnet prefix so direction classification - // works out of the box. The helper also self-guards on this flag. - lib::utils::append_interface_host_subnets(false, v4_addrs, v6_addrs, _hostIPv4, _hostIPv6); + // 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 }