eBPF: never inject on a stale tpinjector scratch buffer - #153
Open
paweljw wants to merge 1 commit into
Open
Conversation
fill_msg_buffers() deliberately bails for SSL connections (and on empty messages or allocation failure) without refreshing the per-CPU scratch buffer, but both call sites ignored its return value and ran protocol_detector() anyway. The detector reads only that scratch buffer, so a plaintext HTTP request left by a previous message on the same CPU could make it approve injection into a message that is actually TLS ciphertext. The resulting Traceparent splice corrupts the TLS record and the peer aborts the connection with a bad_record_mac alert. Patch 009 makes both call sites honor the fill result: no fresh buffer, no detection, no injection. H2 handling is unaffected; it reads the message directly and already has its own SSL guards.
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.
Problem
OBI's
tpinjectorsk_msg program can splice a 70-byteTraceparent:header into outbound TLS ciphertext, corrupting the record so the remote peer fails its MAC check and aborts with abad_record_macTLS alert (alert 20). Observed as a sustained, low-rate, probabilistic failure of outbound HTTPS from an nginx forward proxy on an instrumented host; retries of the identical request succeed.Root cause in OBI v0.12.2 (
bpf/tpinjector/tpinjector.c):fill_msg_buffers()is supposed to copy the outgoing bytes into the per-CPU scratch mapmsg_buffer_mem, but for known-SSL connections it early-returns without touching the buffer (:711-713).:1118,:1204).protocol_detector()never reads the actual message; it reads only the per-CPU scratch (:767-777). A stale plaintextPOST /... HTTP/1.1left by a previous message on the same CPU makes it classify a TLS message as an HTTP request.bpf_msg_push_datas the traceparent after the first0x0Abyte found in the ciphertext.The
valid=0shield entry written by the SSL uretprobe protects most sends, but is lost on a connection's first write (SSL binding happens after sk_msg), onSSL_writeretries, on LRU eviction of the host-wide 10k-entry map, and on egress-key collisions (the key is ports-only). Whenever a shield gap coincides with a poisoned CPU buffer, the splice fires. The H2 path already guards exactly this false-positive class (:1075-1078); the HTTP/1 path never did.Fix
Patch
009-never-inject-on-stale-msg-buffer.patch: honorfill_msg_buffers()' return at both call sites. No fresh buffer, no detection, no injection. Inhandle_existing_tp_pida failed fill now clears the map entry and terminates handling for the message, mirroring the existing detector-failure path. H2 handling is untouched.Verification
git apply --checkclean on v0.12.2 + patch 008.generate.sh+make compileof the patched tree inobi-generator:0.2.15passes (exit 0).Will also be proposed upstream separately.