Skip to content

fix(inputs/pcap): look up iface by name unless it is an address - #802

Merged
leoparente merged 1 commit into
developfrom
fix/pcap-iface-name
Oct 28, 2026
Merged

leoparente merged 1 commit into
developfrom
fix/pcap-iface-name

Conversation

@leoparente

Copy link
Copy Markdown
Contributor

Summary

Live capture has failed for every iface value since the PcapPlusPlus upgrade in #763 (23.09 to 24.09, 2026-04-08):

  • a name such as eth0, lo0, or auto once it resolves to a name fails with Not a valid IPv4 address: eth0;
  • an IPv4 literal fails with Not a valid IPv6 address: 192.0.2.1;
  • an IPv6 literal fails with Not a valid IPv4 address: 2001:db8::1.

Cause. PcapInputStream::start() assigns the iface string to both an IPv4Address and an IPv6Address. PcapPlusPlus 23.09 turned an invalid string into the zero address, which sent names to the name lookup. From 24.09 on, both constructors throw std::invalid_argument instead. The af_packet source goes through the same assignments before it opens the interface by name, so it was broken too.

Fix. Parse the value as an address only when isValidIPv4Address or isValidIPv6Address says it is one; anything else is looked up by name. The flow handler, the netprobe input and match_subnet already guard this way. No string is valid as both an IPv4 and an IPv6 address, so this picks the same lookup as 23.09 for every input:

  • 0.0.0.0 and :: still go to the name lookup;
  • a zone-scoped IPv6 such as fe80::1%en0 is handled as before (it was never supported).

Who is affected. No pktvisor release tag contains #763. orb-agent images copy pktvisord from the develop build, though, so they are affected. On the released orb-agent 2.17.0 image (pktvisor 4.5.0-develop-ca3f081), pktvisord eth0 exits with Not a valid IPv4 address: eth0.

Tests

  • New src/inputs/pcap/tests/test_iface.cpp: it checks that pktvisor's own lookup error comes back, not PcapPlusPlus's. It needs no capture privileges, because each lookup fails before a capture opens. The four cases:

    • an unknown name;
    • an IPv4 address and an IPv6 address reserved for documentation (RFC 5737, RFC 3849);
    • 0.0.0.0, which is looked up as a name.

    The first three failed on develop with the PcapPlusPlus messages.

  • Suite: all 25 ctest targets pass on macOS (Release).

  • pktvisord on macOS:

    • with lo0, develop's binary stops at Not a valid IPv4 address: lo0;
    • this branch resolves the device and gets as far as opening it, with lo0 and with 127.0.0.1. The open then fails with Cannot open interface for packet capture, because BPF is root-only on that machine.

PolicyManager::load catches only std::runtime_error, so this std::invalid_argument also skipped the policy rollback, and a retry got 409. With this fix, a bad iface no longer throws it. #801 makes Policy::start stop its handlers whatever the exception; load still rolls back only on a std::runtime_error.

🤖 Generated with Claude Code

PcapInputStream::start assigned the iface string to both an IPv4Address
and an IPv6Address. Since PcapPlusPlus 24.09 those constructors throw
std::invalid_argument on a string that is not an address, so live capture
failed for every value: "eth0", "auto" and an IPv4 literal with "Not a
valid IPv4/IPv6 address". Parse the value as an address only when it is
one, as the flow and netprobe inputs already do; anything else is looked
up by name, as with PcapPlusPlus 23.09.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nbl-ai-review

nbl-ai-review Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

AI Code Review — risk tier: Lite · dimensions run: correctness, security

Advisory only. A human owns the merge decision.

Summary — Net-positive, targeted fix: iface is now classified as IPv4/IPv6 only when isValidIPv4Address/isValidIPv6Address confirms it, matching the existing guard pattern used elsewhere (flow handler, netprobe input, match_subnet) and backed by new regression tests. No correctness or security issues found.

🤖 AI Code Review · run · prompts: code-review/v1 · models: claude-sonnet-5/medium · context: description · 0 comment(s) · 0 prior AI review(s) · usage: $0.58 · 961k in (93% cached) · 4.5k out

@github-actions

Copy link
Copy Markdown

LCOV of commit a181f2f during Debug Builds #251

  lines......: 85.7% (17941 of 20923 lines)
  functions..: 75.5% (1810 of 2396 functions)
  branches...: no data found

Files changed coverage rate: n/a

Full coverage report

@leoparente

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T17:43:05.702081Z a181f2f Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: a181f2f6c9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@leoparente
leoparente marked this pull request as ready for review October 28, 2026 17:48
@leoparente
leoparente merged commit 019b6e1 into develop Oct 28, 2026
23 checks passed
@leoparente
leoparente deleted the fix/pcap-iface-name branch October 28, 2026 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants