feat(adapters): read the audit lane back, attributed by filter id - #333
Merged
Conversation
Step 3b of #303. Reads which destinations the kernel actually refused, and converts them into the same EgressEvent the Linux supervisor produces. Still nothing wired into a run. On Windows this lane is the only witness. UDP send_to returns SUCCESS on a datagram the kernel drops, so assert_no_egress cannot be built on client-side errors the way a faithful port would have been — what the kernel recorded is the evidence, and there is no second source to check it against. Attribution is by FILTER ID rather than by SID, which is where this departs from the spike. FwpmNetEventEnum returns the entire host's event buffer: every user, every process. Filtering by the run identity's SID works in a controlled probe and is wrong here, because a drop record whose USER_ID_SET flag is absent cannot be attributed at all — and neither answer is safe. Counting it fails the run over somebody else's traffic; ignoring it risks passing over our own. Our own filter ids have no such middle case: a drop carrying one is ours by construction. The SID is kept as corroboration and never as the test, and install() now returns the block ids so the attribution exists to be made. A timestamp lower bound goes with it. Filter ids are allocated by the kernel and can be reused once a session closes, so a stale record from an earlier run could otherwise be counted as this one's. Two runs on one host is the CI case, not an exotic one. Two decisions about missing fields, both in the same direction. A record with no address is still a denial, so it is reported as "?:443" rather than discarded — discarding it would turn a real refusal into silence, which is the direction that produces a false green. And a protocol the kernel did not set is "?", never guessed: "we do not know" and "it was TCP" are different findings, and inventing the second would put something in the evidence that never happened. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XdXrbksFKirm7yW6EDunur
There was a problem hiding this comment.
Pull request overview
Adds Windows “audit lane” readback support so WFP net-event drops can be attributed to this run (by filter id + timestamp lower bound) and converted into the cross-platform flowproof_trace::egress::EgressEvent shape. This is a key building block for making assert_no_egress meaningful on Windows, even though containment is still not wired into runs yet.
Changes:
- Change
egress_windows::filters::installto return the BLOCK filter IDs needed for attribution. - Add
egress_windows::auditmodule to enumerate WFP net events and decode attributable classify-drop records intoEgressEvents. - Enable the Windows binding feature needed to obtain a FILETIME lower bound (
GetSystemTimeAsFileTime).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/flowproof-adapters/src/egress_windows/filters.rs | Return block filter IDs from install() for later audit attribution. |
| crates/flowproof-adapters/src/egress_windows/audit.rs | New audit readback implementation: enumerate/deduce drops, render destination/protocol, and convert to EgressEvent. |
| crates/flowproof-adapters/src/egress_windows.rs | Export the new audit module. |
| crates/flowproof-adapters/Cargo.toml | Add Win32_System_SystemInformation feature for FILETIME lower bound. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+99
to
+106
| loop { | ||
| let mut entries: *mut *mut FWPM_NET_EVENT5 = std::ptr::null_mut(); | ||
| let mut num = 0u32; | ||
| let rc = unsafe { FwpmNetEventEnum5(engine, enum_handle, 128, &mut entries, &mut num) }; | ||
| if rc != 0 || num == 0 || entries.is_null() { | ||
| break; | ||
| } | ||
| for i in 0..num as usize { |
Comment on lines
+185
to
+192
| pub fn destination_of(addr: Option<IpAddr>, port: Option<u16>) -> String { | ||
| match (addr, port) { | ||
| (Some(a), Some(p)) => format!("{a}:{p}"), | ||
| (Some(a), None) => format!("{a}:?"), | ||
| (None, Some(p)) => format!("?:{p}"), | ||
| (None, None) => "?".to_string(), | ||
| } | ||
| } |
Comment on lines
+40
to
+44
| /// One drop the kernel attributed to one of our filters. | ||
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| pub struct Drop_ { | ||
| pub destination: String, | ||
| pub protocol: String, |
Comment on lines
+262
to
+266
| let v4 = IpAddr::V4(Ipv4Addr::new(198, 51, 100, 9)); | ||
| assert_eq!(destination_of(Some(v4), Some(443)), "198.51.100.9:443"); | ||
| assert_eq!(destination_of(Some(v4), None), "198.51.100.9:?"); | ||
| assert_eq!(destination_of(None, Some(443)), "?:443"); | ||
| assert_eq!(destination_of(None, None), "?"); |
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
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.
Step 3b of #303. Based on
main(#332 merged).Reads which destinations the kernel actually refused and converts them into the same
EgressEventthe Linux supervisor produces. Still nothing wired into a run —Containment::command_flowremains "not implemented".Needs the
full-cilabel (see the note at the bottom — this matters more than usual).Why the lane is the only witness
UDP
send_toreturns SUCCESS on a datagram the kernel drops. Soassert_no_egresscannot be built on client-side errors the way a faithful port would have been. What the kernel recorded is the evidence, and there is no second source to check it against.The departure from the spike: attribute by filter id, not SID
FwpmNetEventEnumreturns the entire host's event buffer — every user, every process. The spike filtered by the run identity's SID, which is fine in a controlled probe and wrong here:Our own filter ids have no such middle case — a drop carrying one is ours by construction, no inference required. So
install()now returns the block filter ids, and the SID is kept as corroboration rather than as the test.A timestamp lower bound goes with it. Filter ids are allocated by the kernel and can be reused once a session closes, so a stale record from an earlier run could otherwise be counted as this one's. Two runs on one host is the CI case, not an exotic one.
Two decisions about missing fields, both in the same direction
?:443rather than discarded — discarding it would turn a real refusal into silence, and silence is the direction that produces a false green.?, never guessed. "We do not know" and "it was TCP" are different findings; inventing the second would put something in the evidence that never happened.Verification
cargo check … --target x86_64-pc-windows-msvccargo clippy … --target x86_64-pc-windows-msvc --no-deps -D warningsegress_windows/cargo test --workspace(Linux)cargo clippy --workspace --all-targets --all-features -- -D warnings(CI command)cargo fmt --checkscripts/gate/ratchets.shThe cross-target check caught a real API mismatch again:
GetSystemTimeAsFileTimereturns aFILETIMEinwindows0.62 rather than taking an out-pointer.Worth stating plainly now that the port is substantial. On #332 — and, as far as I can tell, on every Windows PR in this series — the
windows build + E2Ejob reportedskipped. It is gated ongithub.event_name == 'schedule' || contains(labels, 'full-ci'), and the label has not been applied.So the whole
egress_windows/tree is merged on the strength ofcargo check --target x86_64-pc-windows-msvc, which proves signatures and types and nothing about behaviour. The nightly schedule is the backstop and should pick it up, but it has not run since this code landed.I'd suggest labelling this one
full-cibefore merging, since it is the first module whose logic could be wrong in a way types cannot catch.Generated by Claude Code