v3#94
Open
Outfluencer wants to merge 9 commits into
Open
Conversation
* harden packet validation: drop fragments, SYN payload, SYN+FIN/RST - drop fragmented tcp packets: non-first fragments have no tcp header, so the port check ran on payload bytes and reassembled data could reach the backend without passing the inspection state machine - drop SYNs carrying payload (TCP fast open style), the data would bypass payload inspection - drop SYN+FIN and SYN+RST flag combinations in detect_tcp_bypass - bump conntrack_map to 16384 entries to reduce LRU eviction of legitimate in-flight handshakes during floods * unpin all maps and use proper prometheus counter semantics
The previous blanket drop of all fragmented TCP packets ran before the port check (non-first fragments carry no TCP header), so fragmented traffic for every other service on the same interface was dropped too. Split the check in two: - non-first fragments (offset != 0) are passed up the stack: their ports cannot be read, and they are harmless for the filtered range because reassembly can never complete once the first fragment is dropped; the kernel discards them after the frag timeout - first fragments (MF set, offset 0) keep the TCP header, so the port check runs as usual and only fragments aimed at the filtered range are dropped Protection for the filtered range is unchanged; fragmented packets can never reach the backend uninspected. Other services on the same NIC now receive their fragmented traffic again.
* Reorganize and clean up the entire codebase eBPF side (c/): - Split headers by topic: config.h (runtime config externs, removes the include-order dependency), varint.h (bounded VarInt reader), protocol.h (Minecraft packet limits + inspectors); minecraft_helper.h and minecraft_networking.h are gone - Deduplicate the bounds-check macros (SKIP_OR_RETURN and READ_VAL_OR_RETURN now build on CHECK_BOUNDS_OR_RETURN) and merge VARINT_OR_DIE/MAX_VARINT_OR_DIE into one READ_VARINT_OR_RETURN - Turn the connection-state defines into an enum, document the pseudo states, and unify inspector signatures (cursor, payload_end, data_end, ...) with const-correct payload cursors - Hoist the duplicated flow-key computation, inline the single-use remove_connection helper, drop the redundant switch_to_verified label, and fix the four const-qualifier warnings - Rewrite comments throughout and add section banners Verified equivalent: the disassembly differs from the previous object only by the deduplicated flow-key construction (8 fewer instructions); the kernel verifier walks both objects to the same state counts. Rust side (src/): - Split the 439-line main.rs into focused modules: logging, shutdown, ebpf, metrics; main.rs is now just CLI + wiring - Define the stats struct, per-cpu summing and Prometheus counters from a single field list (one macro) instead of five hand-maintained copies - Replace the per-call-site dummy mutexes around the shared Condvar with one Shutdown coordinator (the old setup paired two mutexes with one condvar) and drop the unnecessary Arc<Mutex<>> around the stats map - Replace lazy_static with std LazyLock and drop the dependency - Remove the redundant RUST_LOG set_var, fix typos (epbf, programm) Build: - build.rs: emit rerun-if-changed for the eBPF sources so the object is only recompiled when they change - README: document the project layout * Add native unit tests for the eBPF parsing code c/tests/protocol_test.c compiles the real headers (varint.h, protocol.h, common.h) for the host and exercises them in userspace: - VarInt reader: wiki.vg test vectors, encoder/decoder roundtrip, truncation, max-size limits, overlong encodings, and that parsing never runs past payload_end even when data_end leaves room - Bounds-check macros: in/out of bounds, dual-bounds behavior, unaligned reads - Packet inspectors: handshake (intentions incl. the 1.20.5 transfer intent, legacy ping, every truncated prefix, length/host limits, combined handshake+status and handshake+login segments with resume cursor), status request, ping request, and login across all protocol eras (pre-1.19, 1.19 key block, 1.19.1 uuid flag, 1.19.3, 1.20.2+) including the online/offline username rules Every inspector call runs on an exact-size heap copy so ASan catches any read past data_end; slack bytes behind payload_end are terminator-shaped so a parser that ignores payload_end produces a detectable wrong result (validated by mutation-testing the bounds checks). The harness is compiled with ASan/UBSan and run by the cargo integration test tests/c_unit_tests.rs, so plain running 7 tests test config::tests::partial_config_falls_back_to_defaults ... ok test config::tests::rejects_oversized_reset_window ... ok test config::tests::embedded_default_matches_struct_default ... ok test config::tests::rejects_inverted_port_range ... ok test config::tests::rejects_zero_reset_window ... ok test config::tests::rejects_zero_start_port ... ok test config::tests::unknown_keys_are_rejected ... ok test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s running 1 test test c_parser_unit_tests ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.96s covers it; CI now runs cargo test before the release build. UBSan found one real issue: the varint byte fold did a signed left shift that overflows __s32 for 5-byte values (e.g. 0x0F << 28), which is UB in C. The shift now happens in unsigned; the emitted BPF instructions are unchanged (verified via objdump diff).
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.