Adding initial TLS attach/detach mechanism#358
Draft
P1llus wants to merge 23 commits into
Draft
Conversation
Add the EBPF_EVENT_TLS_CONN and EBPF_EVENT_TLS_CHUNK event types and the EBPF_VL_FIELD_TLS_DATA variable-length field used to carry captured plaintext. Define ebpf_tls_new_event (emitted when a new SSL object is created) and ebpf_tls_chunk_event (one slice of a single SSL_read/SSL_write call). The chunk direction is a fixed-width uint8_t on the wire rather than an enum with implementation-defined width. Add the per-task entry/return state arms (ssl_new, ssl_read, ssl_write) so the return probes can recover the arguments captured at entry.
SSL_new mints a globally unique, monotonic connection id from a single array cell and records (tgid, ssl_ptr) -> conn_id in an LRU map. The key includes tgid because an SSL pointer is only unique within one address space; LRU lets stale entries age out on their own since there is no SSL_free hook to prune them, and a reused pointer is corrected by the next SSL_new overwriting its entry. SSL_read and SSL_write both capture at return so the length reflects the bytes actually transferred (min of the requested and returned counts), which is correct for partial and non-blocking calls. The buffer is split into fixed-size chunks emitted with bpf_loop, each carrying a per (conn_id, direction) call sequence number. A tracked_tgids allow-list gates every probe: nothing is captured for a tgid that has not been explicitly added. max_tls_call caps the bytes captured per call, with 0 meaning unlimited.
Add the RAW_TLS_CONN and RAW_TLS_CHUNK raw types, the public quark_tls_conn and quark_tls_call structs, the QUARK_EV_TLS_CONN_ESTABLISHED and QUARK_EV_TLS_CALL event flags, the QQ_TLS queue flag and the max_tls_call queue attribute (0 = unlimited). Aggregate the RAW_TLS_CHUNK events belonging to one call into a single quark_tls_call chain, following the same pattern as TTY aggregation, and track per-call truncation (a clean trailing cut) separately from a gap (a missing or unreadable non-trailing chunk, so the buffer is not a safe contiguous prefix). Convert the ebpf TLS events into raw quark events, wire the aggregation into the queue and free the chains on event teardown. Add quark_queue_track_tgid and quark_queue_untrack_tgid to manage the capture allow-list, and quark_queue_tls_attach to attach the uprobes system-wide at caller-provided file offsets. The TLS programs are autoloaded and max_tls_call is pushed to the bpf object only when QQ_TLS is set.
Add the TlsConn and TlsCall types, the QQ_TLS, QUARK_EV_TLS_* and QUARK_TLS_DIR_* constants and the MaxTlsCall queue attribute, and decode both new event kinds in GetEvent (TlsCall reassembles its chunk chain into a slice of byte slices). Add TrackTgid, UntrackTgid and TlsAttach wrappers over the C tracking/attach API. Surface the event timestamp as Event.Time (wall-clock nanoseconds: boot wall time plus the kernel monotonic timestamp).
Add OpenSSL-backed tests covering probe load, connection-id minting, single- and multi-chunk call reassembly, per-call truncation, and concurrent capture from two tracked processes (asserting distinct conn_ids and no cross-attribution of payloads). Detect OpenSSL via pkg-config and link it into the dynamic test binary only; OpenSSL 3.x loads providers at runtime so static linking is not viable. The tests compile out when OpenSSL is absent. Exclude the TLS tests from the valgrind target: uprobes and valgrind's ptrace-based tracing conflict at the kernel breakpoint level, so a traced child dies with SIGTRAP the moment it hits an attached uprobe.
Drive the TLS connection lifecycle from SSL_free and make the read/write
path resilient to connections whose SSL_new was never observed.
Connection state moves into a single plain hash tls_conns
(BPF_F_NO_PREALLOC) keyed by {tgid, ssl}, holding the conn_id, the
per-direction call_seq, and a flags word. The key must include tgid
because an SSL* is only unique within one address space. The entry is
created by SSL_new's return probe (gated on the tracked allow-list and
the trusted deny-list) and the same shape the network probe's sk_to_tgid
uses.
SSL_free adds an uprobe that emits a close event
(EBPF_EVENT_TLS_CONN_CLOSE -> RAW_TLS_CONN_CLOSE ->
QUARK_EV_TLS_CONN_CLOSED) and deletes the entry, so a reused SSL* address
can never alias a stale conn_id. SSL_free is userspace and does not run on
SIGKILL, so entries for a process that dies without it are reclaimed by an
exit-driven sweep in bpf_queue (also invoked by untrack_tgid). TlsAttach
now requires the SSL_free offset alongside new/read/write.
On the read/write return probe a tls_conns hit is captured as before. A
miss is no longer dropped silently: for a tracked, non-trusted tgid the
connection is adopted in place -- a conn_id is minted, recorded and
announced with EBPF_TLS_CONN_F_PREFIX_UNKNOWN, and the payload delivered
-- so attaching (or re-hooking) after a connection is already open still
surfaces its traffic instead of losing it. The flag marks that the
captured stream does not start at the connection's first byte, letting a
consumer decide per protocol whether the missing prefix is recoverable.
Untracked callers stay silent; the tracked/trusted checks are paid only on
the miss, so the capture hot path remains a single lookup.
The flag is surfaced through quark_tls_conn.flags and the Go
TlsConn.PrefixUnknown. Tests assert the close event and its conn_id, that
connections seen from SSL_new carry no flag, and add a late-adoption test
that tracks a client only after its handshake and checks the first write
is adopted with PREFIX_UNKNOWN and still delivered.
Three fixes to the TLS uprobe path found in review. Gate the SSL_read/SSL_write entry probes on the same trusted deny-list and tracked allow-list as the capture path. The entry probes stash the caller's buffer into the per-task state map, a bounded LRU shared with the file and network probes. A caller that is not tracked can never be captured at the return probe (a tls_conns miss is only adopted for a tracked tgid), so stashing its arguments only churns that LRU and can evict in-flight state that would have been captured. Gating keeps the map populated only by callers that may actually be captured; adoption is unaffected, since a tracked tgid whose SSL_new was missed still passes the gate and is adopted at return. Own the uprobe links in a per-attachment record on the bpf_queue instead of the skeleton's single link slot per program. Attach is system-wide per binary, so a process may be asked to instrument several distinct binaries; a second quark_queue_tls_attach reusing the same program would overwrite the first path's link and leak it, with no way to detach it. The links now live on a tls_attachments list, torn down in bpf_queue_close; a half-finished attach rolls its own record back. Set the default max_tls_call to 0xffffffff (~4 GiB) rather than 0. The attribute is a size_t but the BPF rodata mirror is a u32, so a multiple of 4 GiB truncates to 0, which the probe reads as "unlimited". Staying just under the u32 cap keeps the effective limit unbounded in practice without the truncation footgun. Still TEMP; revert to 1 << 20 (1 MiB).
The TLS uprobe series was much more heavily commented than the rest of the codebase. Cut the narrative/explanatory paragraphs down to the non-obvious bits (verifier footguns, the tgid-in-key invariant, gap-vs-truncated semantics, the SIGKILL exit sweep), removing whole redundant sentences rather than rewording so nothing changes meaning. Comment-only changes; no behavior change.
Add entry/return probes for the SSL_read_ex/SSL_write_ex ABI, which report the transferred byte count through an out-parameter, and factor the common capture path out of the SSL_read/SSL_write return probes into a shared helper.
…tach quark_queue_tls_attach now takes a pid: -1 keeps the system-wide behavior gated by the tracked_tgids allow-list, while pid >= 0 scopes the uprobes to that process and tracks it automatically. Add quark_queue_tls_attach_sym to attach by symbol name in a shared library (SSL_new/SSL_free/SSL_read/ SSL_write required, SSL_read_ex/SSL_write_ex optional). Both return an opaque quark_tls_attachment handle that quark_queue_tls_detach releases, so multiple distinct attachments can coexist and be torn down individually.
libbpf's attach-by-symbol resolver walks a versioned .dynsym and skips every entry whose gelf_getversym() fails; the local elftoolchain build stubbed it to return NULL, so symbol lookups missed all libc/libssl symbols. Provide a real implementation modeled on gelf_getsym; the underlying SHT_GNU_versym translation was already present.
TlsAttach now takes a pid and returns an opaque TlsAttachment; add TlsAttachSym for symbol-based attach and TlsDetach to release a single attachment.
Add t_tls_attach_sym, which attaches to libssl by symbol and verifies capture, and t_tls_attach_sym_missing, which checks graceful failure for a missing symbol and a nonexistent path. Exclude the uprobe-based TLS tests from the valgrind run.
…index Capture is no longer gated by a caller-managed tracked_tgids allow-list; it is gated only by the existing trusted_pids deny-list. The allow-list added no value for pid-scoped probes and bloated every capture path. Both attach modes stay: offset attach (pid or -1) and symbol attach (pid >= 0). The quark_queue_track_tgid/untrack_tgid API (C and Go) is removed. Connection reclamation now uses tls_conn_tgids, a probe-maintained GC index of tgids that have opened a TLS connection (written on SSL_new and on mid-stream adopt; it does not gate capture). The process-exit handler consults it with an O(1) lookup and only sweeps tls_conns for a tgid that actually had connections, so unrelated exits don't scan the map. SSL_free still deletes on clean close; detach just removes the attachment and lets exit reclaim any leftovers.
Drop the temporary 4 GiB debug default and set the per-call capture ceiling to 1 MiB. Callers that want more (or unlimited, 0) can override via quark_queue_attr.max_tls_call.
Two safety gaps in the caller-driven TLS attach/detach lifecycle, found while auditing the detach and teardown paths. quark_queue_tls_detach removed and freed the handle after only a NULL check, so a repeated detach or a handle belonging to another queue was a use-after-free or a foreign-list removal. It now confirms the handle is linked in this queue by walking the list and comparing addresses (never dereferencing it), and is a no-op otherwise. A tls_conns entry could also outlive both queue drain and process exit: tls_mark_conn_tgid was unchecked, so under tls_conn_tgids saturation a connection was inserted whose tgid the process-exit sweep never reclaims, stranding it until the queue closed. The mark now returns its result and both uretprobe__ssl_new and tls_adopt_conn record the tgid before inserting the connection, dropping the connection when the mark fails. This enforces the invariant that a tls_conns entry always has a recorded tgid, so the exit sweep can always reclaim it.
The bison-generated parse.tab.c sets a variable it never reads, which trips -Werror under the strict warning set. Scope the suppression to that one generated object rather than relaxing it project-wide.
Contributor
|
Looking more myself, but the bot seems to have found some things worth investigating PR Review: #358 — Adding initial TLS attach/detach mechanismAuthor: P1llus SummaryThis PR adds TLS uprobe attachment APIs, TLS event wire formats, userspace aggregation, Go bindings, and a useful set of e2e tests. The core design looks coherent, but I found two correctness issues that should be addressed before merge: one can emit impossible connection lifecycle state under map pressure, and the public attach API can mis-handle queues that fell back away from the eBPF backend. FindingsSignificant
VerdictREQUEST_CHANGES. The feature is promising and well-tested for the happy paths, but the two lifecycle/backend correctness issues above can produce misleading events or unsafe API behavior in realistic failure/fallback scenarios. |
Member
Author
|
Thanks @nicholasberlin . Yeah the last one is a good gap to cover, it's only meant to work with the EBPF backend either way. The first one is also correct, its a good shoutout though its not really a "significant" finding it's something worth resolving. I have kind of put this on hold as I am awaiting some answers on the open questions in my PR description, once that is done I'l fix those and do one last review. We are also doing some of the discussions outside the github issue which is not always good in terms of sharing context. |
Simplifies and hardens the TLS uprobe capture based on PR review: - Drop symbol-based attach; offset attach is the only path. Reverts the elftoolchain gelf_getversym implementation back to its upstream stub and removes quark_queue_tls_attach_sym, the Go TlsAttachSym binding, and the symbol-attach tests. - Remove the SSL_read_ex/SSL_write_ex uprobes and their entry/return state (State.h ops and count_ptr); only the plain SSL_read/SSL_write are hooked. - Remove the max_tls_call per-call cap from the BPF rodata, quark_queue_attr / quark_queue, and the Go binding. Chunking alone bounds per-record size and userspace reassembles the call, so the cap added no value. - Reclaim the connections of a process that exits without SSL_free entirely in-kernel, via a second disassociate_ctty hook (fentry or kprobe) that sweeps tls_conns with bpf_for_each_map_elem, gated by an O(1) tls_conn_tgids lookup. Removes the userspace tls_reclaim_conns sweep and its per-exit syscalls from the drain path. - uretprobe__ssl_new now confirms the tls_conns insert before emitting ESTABLISHED, matching the mid-stream adopt path, so a full map never yields an establish with no backing state. - Require the eBPF backend for QQ_TLS: quark_get_bpf_probes gates on the active backend rather than the requested flags, and quark_queue_open fails with ENOTSUP if QQ_TLS is set but eBPF is not the backend that opened. Prevents a QQ_ALL_BACKENDS fallback to kprobe from letting tls_attach dereference a kprobe queue as a bpf queue. - TLS tests skip themselves under valgrind (its ptrace use breaks uprobes) instead of relying on a Makefile exclusion; drop the max_tls_call truncation test.
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
2 participants
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.
This PR adds the possibility to attach uprobes properly to both the systems OpenSSL shared library (libssl*.so) through symbols and to binaries which includes their own compiled static linked OpenSSL/BoringSSL but has their symbols stripped (through offsets).
This represents a draft/opinion on an implementation rather than a required feature request. A few important notes below:
Notes:
I won't be including any direction on "how" to retrieve correct offsets, or when/how to trigger this in the PR description but would be happy to discuss it elsewhere.
Summary
Answered questions:
Q: Currently the default ringbuffer is 4MB, the "best" scenario would be to give this a separate ringbuffer so that it won't have the possibility to block other more important events.
A: Currently we can increase the existing ringbuffer and test first, a second ringbuffer could work though it will still only be 1 queue. A second queue was not preferred.
Q: Providing the symbol resolution in quark adds a fair bit of extra code, what to do?.
A: Removed all the specific code for doing symbol resolution, you can still use the tls_attach function with offsets and resolve symbols in userspace. The symbol related functions is easier to add in separate PR.
Q: How do we handle lingering map entries for all the connection ID's for a process that for some reason was not freed (like if the process crashes).
A: Moved the code for cleaning up on process exit into the TLS ebpf program itself, mirroring the way that Process/Probe.bpf.c does it, on last thread exiting.
Q: Where should I exclude tests from Valgrind, as Uprobes causes SIGTRAP?
A: Removed the Makefile entries and added them to the tests themself as other tests currently do.
How to use it:
Now to explain a bit about how it works, there is 2 usecases that the public API exposes, the logic to determine WHEN to do this is up to either a future quark addition (a rule action as a pure example) or determined by the program that imports quark, as these are attached on-demand, then drained exactly like other events through
GetEventTlsAttach
When a binary you want to attach to compiles its own static version of
OpenSSL/BoringSSLyou provide thepid,pathand theoffsetsto use (new, read, write, free). It is recommended that you use-1as thepid, and track this by(dev, inode). This is because you do not need to apply a new tls attachment each time someone starts the same binary or use the same inode object, so when your logic triggers a second time indicating possible new tls attachment, you check if the inode object is already attached and skips, but it's left up to the user on what they want to do. Multiple uprobes on the same inode objecct (but scoped to their specific pid) is refcounted, so it will still only create 1 int3 breakpoints rather than multiple.It's the same with preventing TOCTOU and similar is up to the user through various common techniques.
TlsAttachSym (Removed from this PR, will be opened separately, resolve symbols yourself at first)
Mainly meant as a helper for attaching to system shared libraries like
libssl.so. Currently it does this by already defining the required symbol names.All you provide is the pid and the inode object path and restricts you from passing
-1as the PID to prevent mistakes, this is because the overhead is not in filtering out the events but the fact that it adds aint3breakpoint for ANY process usinglibssl.Draining events
Draining the collected events is done through the same mechanism as before, your
GetEventnow returns one of three new types that you branch on:Closing down
Both
TlsAttachandTlsAttachSymreturns a handler, whenever you want to close it again you callTlsDetachwith the provided handler.When shutting down, quark will automatically close any handler still open as part of its normal graceful shutdown in case something didn't go wrong, but preferably this would already be empty.
Currently a process with remaining references to open connections are automatically cleaned up on process exit (as mentioned in the open questions at the top).
Event Flow Chart
flowchart TD A["quark_queue_tls_attach / _attach_sym (userspace)"] -->|"one bpf_link per program, stored in ta->links[]"| B["uprobes now live on libssl in the target"] B --> C{"target calls an SSL_* function"} C -->|SSL_new returns| N["uretprobe__ssl_new"] C -->|SSL_write entry+return| W["uprobe/uretprobe__ssl_write"] C -->|SSL_read entry+return| R["uprobe/uretprobe__ssl_read"] C -->|SSL_free entry| F["uprobe__ssl_free"] N -->|"reserve+submit (fixed size)"| RB[("shared ringbuf")] W -->|"state set → get → capture"| CAP["tls_capture_io"] R -->|"state set → get → capture"| CAP CAP -->|"tls_emit_call → bpf_loop → tls_emit_chunk"| PCPU["per-CPU event_buffer_map (scratch)"] PCPU -->|"ebpf_ringbuf_write = copy in"| RB F -->|reserve+submit| RB RB -->|"ring_buffer__consume_n, called INSIDE GetEvent"| CB["bpf_ringbuf_cb → ebpf_events_to_raw"] CB -->|raw_event_insert| T["by_time + by_pidtime RB-trees"] T -->|"held ~hold_time, then oldest (RB_MIN)"| AGG["quark_queue_aggregate: chains chunks of one call"] AGG -->|raw_event_tls_conn / raw_event_tls_call| ES["event_storage (one reused quark_event)"] ES -->|"GetEvent() returns it"| GO["your Go Event → your drain"]