Feat/core extraction and cli#1
Merged
Merged
Conversation
Implementation plans consumed by AI agents are local working documents, not part of the shipped repo.
Capture, parsing, and analysis now live in a NetProbeCore static library with no ImGui/GLFW dependency, so the GUI, the test suite, and (next) a headless CLI can all build on the same code. - Replace file(GLOB_RECURSE) with explicit source lists. The glob made it too easy for a new core file to silently miss the test and fuzz targets, which re-list core sources by hand. - Split platform libraries by consumer: ws2_32/iphlpapi back the core (ProcessResolver needs them), while dwmapi/opengl32 stay GUI-only. - NetProbeTests drops its hand-maintained copy of the core source list and links NetProbeCore instead. - NetProbeParserFuzzer keeps its explicit list on purpose: libFuzzer needs every analyzed TU built with coverage instrumentation, which NetProbeCore is not. Documented in place so the duplication is intentional, not stale.
The per-packet pipeline — protocol decode, cross-packet SNI recovery, DNS answer harvesting, hostname attribution, process lookup, and flow aggregation — lived inside GuiLayer::processQueue(), which meant the only way to analyse a capture was to open a window. It now lives in core::AnalysisSession, and GuiLayer keeps just the presentation state layered on top. The pipeline logic moved verbatim; the analysis-owning members (FlowAggregator, HostnameCache, TlsReassembler, QuicTracker, ProcessResolver, DNS counters) moved with it. GeoIPResolver stays in the GUI since it is display enrichment, not analysis. AnalysisSession can skip process resolution, which is pure overhead when replaying a capture file whose sockets are long gone. Adds tests covering the cross-packet behaviour the split risked breaking: a DNS answer naming a later flow, SNI recovered only once a split ClientHello's second segment arrives, and clear() resetting every piece of state. 63/63 tests pass; GUI verified to start and render.
Flow export moves out of the GUI so the headless CLI can share it, and gains a JSON format for pipelines (Elastic, Splunk, jq) that cannot read the spreadsheet-oriented CSV. The CSV column set and ordering are unchanged; GuiLayer::exportFlowsCsv is now a one-line delegation, so the file dialog flow is untouched. JSON strings are written by hand rather than via a JSON library. The one real correctness concern is that hostnames and SNI arrive as raw bytes off the wire and are not guaranteed to be valid UTF-8, so the writer decodes each multi-byte sequence and substitutes U+FFFD for anything malformed — truncated sequences, overlong encodings, surrogate halves, out-of-range code points, stray continuation bytes. Tests cover each of those, since a malformed SNI silently producing an unparseable document is exactly the failure a network tool must not have. organizationLabel() moves to core so the flow table and the exported files cannot disagree about how a network is named. 69/69 tests pass.
Runs the full capture and analysis pipeline with no window, so NetProbe can be used over SSH, on servers, and in CI. Links NetProbeCore only — no ImGui, GLFW, or OpenGL. netprobe-cli --list-devices netprobe-cli -r capture.pcap -o flows.json netprobe-cli -i eth0 -f "tcp port 443" --duration 60 -o flows.csv netprobe-cli -r capture.pcap -o - | jq '.flows[]' Adds CaptureEngine::replayFile(), which streams an offline capture packet by packet to a callback instead of routing it through the bounded PacketQueue. openFile() pushes the entire file into a 50k-entry queue that evicts its oldest entries when full, so any capture larger than the queue silently loses its beginning. That is survivable for a live UI showing recent traffic; it is not survivable when the output is an exported flow table that is supposed to describe the whole file. Live capture keeps using the queue, and reports queue drops on stderr rather than quietly undercounting. Ctrl+C stops a capture and still writes the output. Exit codes separate usage errors (1) from runtime failures (2) so scripts can branch on them, and all diagnostics go to stderr so `-o -` stays pipeable. Adds four CTest cases driving the real binary over the generated sample capture — the only coverage of the complete chain from pcap file through parsing and attribution to an exported file. Both failure paths of the check script were verified to fail as intended. 73/73 tests pass.
The flow table reported how much a connection moved and how fast, but not whether it was healthy. Each direction of a TCP flow now tracks the highest sequence number seen and classifies data-bearing segments against it: segments landing behind it are retransmissions, segments skipping ahead are out of order. All sequence comparisons use signed 32-bit differences (RFC 1982 serial arithmetic). A plain integer comparison passes every ordinary test and then reports a flood of phantom loss the moment a long-lived stream wraps past 2^32; two tests cover the wrap in both directions. Bare ACKs are excluded — they carry no bytes and repeat sequence numbers by design, so counting them would report constant duplicates. A flow whose first segment arrives mid-stream only establishes a baseline, and an RST clears tracking so a reused port pair does not read as a huge backwards jump. Surfaced as a sortable, color-coded Loss column in the flows table (a rate, since 20 retransmits out of 50 packets is a broken connection and out of 500,000 is noise), with absolute per-direction counts in the detail pane and four new fields in both exports. Also fixes the bundled sample capture, which this feature exposed: all three ClientHellos were addressed to the same server with the same sequence number regardless of the hostname they advertised, contradicting the DNS answers in the same file. That collapsed them into one flow whose repeated sequence numbers correctly read as retransmissions. Each site now gets its own address, client port, and full three-way handshake, so the sample shows four flows with matching hostnames, real RTTs, and no loss. 83/83 tests pass; GUI verified to render the widened table.
35 named fixtures of the shapes that crash capture tools in production: headers claiming lengths the buffer does not contain, IPv4 IHL and total-length lies, TCP data offsets past the payload, UDP lengths below their own header, IPv6 extension chains that overrun or point at themselves, DNS compression-pointer loops and record counts larger than the payload, truncated TLS records, and QUIC connection-ID and token lengths beyond the packet. Each is checked against the parsers, the stateful reassemblers, and the full AnalysisSession. The assertions are about degradation rather than success: a payload window must never point outside the captured bytes, and recovered hostnames must stay bounded — a parser that walks off the end of a packet and returns a plausible-looking hostname is worse than one that crashes, because the wrong answer is shown to the user as fact. Also checks every prefix of a valid frame exhaustively, since any driver or capture file can hand us a truncated packet, and pins the termination of the IPv6 extension walk, which is an unbounded `while (true)` that is safe only because each step advances at least eight bytes. No defects were found: the existing parsers already handle all of these. The value is the regression barrier — these run under the ASan/UBSan CI job, where an out-of-bounds read fails the build instead of passing silently. The same fixtures now seed the fuzz corpus, taking it from 5 entries to 40, so libFuzzer starts from these shapes instead of rediscovering them byte by byte. 89/89 tests pass.
Static analysis was the missing third leg next to the existing ASan/UBSan and fuzzing jobs. The check list is curated for signal rather than coverage. Checks that fire constantly on idiomatic code are disabled in .clang-tidy with the reasoning recorded there, rather than silenced case by case at each site — a warning nobody acts on trains everyone to ignore the tool. Notably disabled: enum-size (saves bytes that do not matter, invites surprising conversions) and the padding analyzer (advises reordering ~70 members of a class with exactly one instance, trading a readable header for 48 bytes). Every first-party source was then brought clean, so WarningsAsErrors is on and a new finding fails the build instead of scrolling past in a log. This was verified by introducing a deliberate defect and confirming the gate fails, then confirming it passes once reverted. Fixes worth calling out: - ~GuiLayer could let an exception from saving preferences escape a destructor, which terminates the process during shutdown. Now caught, and reported via fputs rather than iostreams, since a handler that can itself throw defeats the purpose. - Both main() functions now catch on a non-throwing path; previously an escaping exception gave a bare failure code with no diagnostic, and for the GUI meant the window simply vanished. - Explicit widening on the multiplications feeding pointer offsets in ProtocolParser. All four are provably bounded today (header lengths cap at 60 bytes), so this fixes no live defect — but it keeps the warning class meaningful for a future site that is not bounded. - sscanf replaced with istringstream extraction: type-safe and portable, without reaching for the Windows-only sscanf_s. - std::endl to '\n' throughout, nullptr for NULL, and the value-parameter and reserve fixes clang-tidy identified. The CI job drives clang-tidy directly rather than run-clang-tidy, which is packaged inconsistently across distributions, and configures with Ninja since compile_commands.json is what the tool needs. README gains the CI badge and a one-line summary of what every push runs, plus a marked TODO for the screenshots that need a human at a screen. 89/89 tests pass; GUI and CLI both verified to run.
Corrections after an audit found several places where the implementation diverged from the plan without justification. - JSON now exports initial_rtt_us as the plan specified, not initial_rtt_ms. Integer microseconds is the better contract anyway: the JSON is for machines, and it avoids handing consumers a rounded float to convert back. CSV keeps milliseconds for spreadsheets. - The flows column is "Retrans" showing retransmissions only, as specified, rather than a combined "Loss" figure. Merging retransmissions with reordering produced a number that cannot be acted on: the two have different causes — loss versus multipath or queueing. Out-of-order counts remain in the tooltip and the detail pane. - Adds the TCP option fixtures the plan asked for and the first pass missed: option length zero (the classic infinite-loop bait), an option overrunning the header, a data offset claiming absent option bytes, and a well-formed option area as the control. - Malformed fixtures now pin their expected degraded outcome instead of only asserting generic sanity. This immediately earned its keep by catching four wrong assumptions of mine — the parser reports "Truncated" rather than "Unknown" for sub-Ethernet frames, distinguishes a zero-length capture from a truncated one, and correctly clamps a lying IPv4 total-length field to the real buffer end, which means recovering the SNI behind it is right rather than invented. Every mismatch was my expectation being wrong; no parser defect was found. - README architecture section no longer claims the UI owns the reassemblers and name cache; that moved to AnalysisSession. Also makes offline replay interruptible. Ctrl+C during a long replay previously did nothing until the whole file had been read, because replayFile never checked for cancellation. SIGBREAK is handled too, since Windows delivers Ctrl+Break that way. Verified by interrupting a real live capture: 13,772 packets on a Wi-Fi adapter, Ctrl+Break, clean exit 0, valid JSON with 39 flows. That capture also validates the loss statistics against real traffic — 4 of 29 TCP flows showed retransmissions, and the busiest showed 152 retransmitted and 133 out of order in 6,492 packets, the near-equal counts being the expected signature of reordering producing one classification of each. 89/89 tests pass; clang-tidy clean; fuzz corpus now 44 seeds.
Phase 6's technical writing deliverable: a walk through the full path from a long-header UDP packet to a recovered server name — HKDF-Extract and Expand-Label, header protection, AES-128-GCM, CRYPTO reassembly, and the ClientHello parse — using this repository's implementation as the worked example. Written around the things that actually cost time rather than a summary of the RFCs: the "tls13 " label prefix, QUIC v2's different salt AND labels AND Initial type bits (a v1-only parser fails silently on v2), the chicken-and-egg that header protection exists to solve, the nonce being iv XOR packet number rather than the iv, and a failed GCM tag being the normal answer for a packet that was never QUIC rather than an error. The section on CRYPTO reassembly is the one that justifies the post: post-quantum key shares have pushed real ClientHellos past a single Initial, so a parser that reads only the first CRYPTO frame does not break loudly — it quietly stops finding SNI on a growing share of exactly the traffic you most want to see. Draft, not published. Needs a human read-through before it goes anywhere.
The clang-tidy gate was reported clean and was not. Running it over all of src/ exits 1: Mbed TLS, GLFW, and NFD headers produce macro-parentheses, reserved-identifier, and use-nullptr findings which WarningsAsErrors turns into failures. QuicParser.cpp, Dashboard.cpp, FlowsView.cpp, and GuiLayer.cpp all fail — precisely the files that include those headers. Two compounding mistakes: The filter. HeaderFilterRegex was '.*[/\](src|include)[/\].*', which also matches _deps/mbedtls-src/include/mbedtls/aes.h — that path contains both "src" and "include". Combined with WarningsAsErrors, every dependency header became a build failure. Fixed by naming the first-party module directories explicitly: src/(capture|cli|core|ui)/. There are no headers directly in src/ and the repository's include/ directory is empty, so this covers everything of ours and cannot match a dependency — verified no _deps path shadows those four names. The verification. I checked this by grepping stdout for first-party paths rather than by looking at the exit code, so failures in dependency headers were invisible to the check. The deliberate-defect test that appeared to prove the gate worked ran on FlowExporter.cpp, which happens to include no third-party header, and so passed for the wrong reason. Re-verified properly: exit code 0 across every file in src/, and the gate still fails on a defect injected into a first-party .cpp and, separately, into a first-party .hpp — in QuicParser, which does include Mbed TLS. That proves dependency headers are excluded while ours are still analysed. The CI job now prints the clang-tidy version and names the failing files, and both .clang-tidy and CONTRIBUTING.md state that this check is judged by exit code, not by reading the log. Also completes the parts of Phase 6 that do not need a human: - The QUIC deep-dive gains an ASCII diagram for every stage — the pipeline overview, the HKDF key schedule, the HkdfLabel byte layout, the header-protection sample offset and mask usage, the AEAD nonce and AAD layout, and CRYPTO reassembly across two Initials — plus line-specific source links throughout and a source index table mapping each stage to its function and line. - CONTRIBUTING.md: per-platform build instructions, how to run each check CI runs, and what review looks for in a tool that parses hostile input (never trust a wire length, degrade visibly rather than plausibly, bound anything holding cross-packet state), plus four good first issues. 89/89 tests pass; clang-tidy exits 0.
Audit follow-ups, all documentation. README claimed 35 malformed fixtures; there are 40 (39 of which seed the fuzz corpus — the empty-capture case has no bytes to write, taking the corpus to 44 entries). The description also predated the TCP option fixtures and the pinned degraded outcomes, so it now mentions both. The plan's status header said 10 commits, carried <pending> placeholders for commits that had already landed, and listed CONTRIBUTING.md as both done and not started. Corrected, and the outstanding list now separates "CONTRIBUTING.md written" from "good first issue labels created on GitHub" — the four candidates exist only in the file; the repository still shows zero labelled issues, and creating them is an outward-facing action. The blog claimed a diagram at every stage and Stage 5 had none. Added the ClientHello field walk: the fixed-size prefix, the three variable-length skips, the extension list, and the nested server_name structure down to host_name. The nesting is the point — host_name_len sits inside the server_name entry, inside ext_len, inside extensions_len, inside the reassembled buffer, so a bounds check at only the innermost level passes for a packet that lied at the outermost one. Verified: build clean, 89/89 tests, clang-tidy exit 0, every one of the blog's source anchors still within its file.
YehiaGewily
force-pushed
the
feat/core-extraction-and-cli
branch
from
July 19, 2026 12:09
d00c0d4 to
f916109
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.