diff --git a/internal/api/ptrace_handlers.go b/internal/api/ptrace_handlers.go index 7c1fa4d5..edb3e138 100644 --- a/internal/api/ptrace_handlers.go +++ b/internal/api/ptrace_handlers.go @@ -69,7 +69,35 @@ func (r *ptraceHandlerRouter) emitDBBypassAttempt(ctx context.Context, engine *p func (r *ptraceHandlerRouter) HandleExecve(ctx context.Context, ec ptrace.ExecContext) ptrace.ExecResult { s, ok := r.sessions.Get(ec.SessionID) if !ok { - slog.Warn("ptrace: unknown session for execve", "session_id", ec.SessionID, "pid", ec.PID) + // Two cases that look the same (no session for this tracee) + // but mean very different things: + // + // (a) Sessionless pid-attach. initPtraceTracer calls + // tr.AttachPID(pid) without WithSessionID for the + // attach_mode=pid path, so the attached root and its + // descendants are sessionless by design -- the wrapper + // / session layer governs enforcement above the tracer. + // Pass through (no policy engine to consult here). + // + // (b) Non-empty SessionID that the session manager does not + // know about. This is a real session-accounting bug: + // something registered a session id with the tracer but + // the session is gone (or never existed) by the time + // execve fires. Fail closed, loud log. + // + // The earlier version of this branch flipped deny->allow + // unconditionally to avoid crashing tracees on a session race; + // per maintainer review (PR #312), the race itself is now + // closed by seedChildStateFromParent in the tracer minimal- + // state fallbacks, and the conflated case (b) must remain + // fail-closed rather than silently allowed. + if ec.SessionlessPIDAttach { + slog.Debug("ptrace: sessionless pid-attach execve, allowing pass-through", + "pid", ec.PID, "filename", ec.Filename) + return ptrace.ExecResult{Allow: true, Action: "allow", Rule: "sessionless_pid_attach"} + } + slog.Warn("ptrace: unknown session for execve, denying", + "session_id", ec.SessionID, "pid", ec.PID, "filename", ec.Filename) return ptrace.ExecResult{Allow: false, Action: "deny", Errno: int32(syscall.EACCES), Rule: "unknown_session"} } diff --git a/internal/api/ptrace_handlers_test.go b/internal/api/ptrace_handlers_test.go index e82f7fc2..1649e57f 100644 --- a/internal/api/ptrace_handlers_test.go +++ b/internal/api/ptrace_handlers_test.go @@ -197,6 +197,63 @@ func TestHandleExecve_DBUnavoidabilityDenyEmitsBypassAttempt(t *testing.T) { } } +// TestHandleExecve_SessionlessPIDAttachAllows verifies the legitimate +// attach_mode=pid sessionless path (PR #312 review concern B): +// initPtraceTracer calls tr.AttachPID(pid) without WithSessionID, so +// the attached root and its descendants are sessionless by design -- +// the wrapper / session layer governs enforcement above the tracer. +// HandleExecve must let those execve's pass through with rule +// "sessionless_pid_attach". +func TestHandleExecve_SessionlessPIDAttachAllows(t *testing.T) { + router, _ := newTestRouter(t, "") + res := router.HandleExecve(context.Background(), ptrace.ExecContext{ + PID: 4242, + Filename: "/bin/true", + SessionID: "", + SessionlessPIDAttach: true, + }) + if !res.Allow { + t.Fatalf("SessionlessPIDAttach execve must allow; got Allow=false") + } + if res.Action != "allow" { + t.Fatalf("Action=%q; want %q", res.Action, "allow") + } + if res.Rule != "sessionless_pid_attach" { + t.Fatalf("Rule=%q; want %q", res.Rule, "sessionless_pid_attach") + } + if res.Errno != 0 { + t.Fatalf("Errno=%d; want 0 (no errno on allow)", res.Errno) + } +} + +// TestHandleExecve_NonEmptyUnknownSessionDenies covers the other half +// of the PR #312 review split: a non-empty SessionID that the session +// manager does not know about is a real session-accounting bug, not +// the legitimate sessionless-pid-attach case. Must fail closed with +// rule "unknown_session" so the bug is visible rather than silently +// allowed. +func TestHandleExecve_NonEmptyUnknownSessionDenies(t *testing.T) { + router, _ := newTestRouter(t, "") + res := router.HandleExecve(context.Background(), ptrace.ExecContext{ + PID: 4242, + Filename: "/bin/true", + SessionID: "session-that-does-not-exist", + SessionlessPIDAttach: false, + }) + if res.Allow { + t.Fatalf("non-empty unknown SessionID must deny; got Allow=true") + } + if res.Action != "deny" { + t.Fatalf("Action=%q; want %q", res.Action, "deny") + } + if res.Rule != "unknown_session" { + t.Fatalf("Rule=%q; want %q", res.Rule, "unknown_session") + } + if res.Errno != int32(syscall.EACCES) { + t.Fatalf("Errno=%d; want EACCES (%d)", res.Errno, syscall.EACCES) + } +} + func TestHandleFile_SoftDelete(t *testing.T) { workspace := t.TempDir() diff --git a/internal/ptrace/attach.go b/internal/ptrace/attach.go index 4e5acc1f..9d6482a4 100644 --- a/internal/ptrace/attach.go +++ b/internal/ptrace/attach.go @@ -129,6 +129,13 @@ func (t *Tracer) attachThread(tid int, opts attachOpts) error { MemFD: memFD, PendingExecStubFD: -1, PendingExecSavedFD: -1, + // Mark the legitimate "attach_mode=pid without a SessionID" + // case (initPtraceTracer's tr.AttachPID(pid) path). Children + // inherit this flag via seedChildStateFromParent so + // HandleExecve can distinguish an intentionally sessionless + // pid-attach descendant from a real "non-empty unknown + // SessionID" session-accounting bug. + SessionlessPIDAttach: opts.sessionID == "", } if opts.keepStopped { t.parkedTracees[tid] = struct{}{} diff --git a/internal/ptrace/tracer.go b/internal/ptrace/tracer.go index dd094cc1..41f97429 100644 --- a/internal/ptrace/tracer.go +++ b/internal/ptrace/tracer.go @@ -29,6 +29,15 @@ type ExecContext struct { Truncated bool SessionID string Depth int + // SessionlessPIDAttach is true when this tracee descends from a root + // that was attached via AttachPID without a SessionID (the + // attach_mode=pid path in app_ptrace_linux.go's initPtraceTracer). + // In that mode the wrapper/session layer governs enforcement above + // the tracer, so an empty SessionID at HandleExecve time is + // intentional, not a session-accounting bug. Handlers use this to + // distinguish "intentionally sessionless" from "non-empty but + // unknown SessionID" (which is a real bug and must fail closed). + SessionlessPIDAttach bool } // ExecResult carries the policy decision. @@ -183,6 +192,13 @@ type TraceeState struct { PendingExecStubFD int // fd injected for exec redirect; cleaned up on exec failure (-1 = none) PendingExecSavedFD int // fd that was displaced by stub fd; restored on exec failure (-1 = none) MemFD int + // SessionlessPIDAttach marks a tracee that descends from a root + // attached via AttachPID without a SessionID (the attach_mode=pid + // path). Propagated to children via seedChildStateFromParent so + // HandleExecve can distinguish "intentionally sessionless" + // (allow + pass-through) from "non-empty unknown SessionID" + // (fail-closed bug). + SessionlessPIDAttach bool } type resumeRequest struct { @@ -314,6 +330,77 @@ func (t *Tracer) ResolveSessionID(pid int32) (string, bool) { return "", false } +// findParentByTGID returns the first tracee whose TGID matches +// parentTGID, or nil if none. Caller must hold t.mu. +func findParentByTGID(tracees map[int]*TraceeState, parentTGID int) *TraceeState { + if parentTGID <= 0 { + return nil + } + for _, st := range tracees { + if st.TGID == parentTGID { + return st + } + } + return nil +} + +// seedChildStateFromParent builds a fully-seeded child TraceeState by +// copying enforcement metadata from its parent. Mirrors what +// handleNewChild's create-from-scratch branch does, so a child created +// through this helper (the child-stop-before-fork-event fallback path) +// is byte-identical in enforcement state to a child created through +// handleNewChild. +// +// Used by: +// - handleNewChild's else branch (the normal create path). +// - The two run()/PTRACE_EVENT_STOP minimal-state fallbacks, where a +// child stop arrives before the parent's PTRACE_EVENT_FORK and we +// must create state immediately. Without inheritance, the child's +// SessionID stays "" until the fork event fires; if it execve's in +// that window, HandleExecve previously denied with EACCES, which +// raced ld.so on the new ELF and crashed the tracee. +// +// Copied (in addition to bookkeeping): SessionID, HasPrefilter, +// PendingPrefilter (skipped when parent already has the filter installed +// since children inherit it via fork), TGID-level escalation flags, +// thread escalation flags, and SessionlessPIDAttach. Per-thread runtime +// state (LastNr, MemFD, Pending*, etc.) is initialized to defaults. +// +// If parent is nil (parent not yet in t.tracees, e.g. attaching root +// before any tracee exists), only the per-thread defaults are +// populated. +// +// Caller must hold t.mu. +func seedChildStateFromParent(parent *TraceeState, childTID, childTGID int) *TraceeState { + st := &TraceeState{ + TID: childTID, + TGID: childTGID, + Attached: time.Now(), + LastNr: -1, + MemFD: -1, + PendingExecStubFD: -1, + PendingExecSavedFD: -1, + SuppressInitialStop: true, + } + if parent == nil { + return st + } + pendingPrefilter := false + if !parent.HasPrefilter { + pendingPrefilter = parent.PendingPrefilter + } + st.ParentPID = parent.TGID + st.SessionID = parent.SessionID + st.HasPrefilter = parent.HasPrefilter + st.PendingPrefilter = pendingPrefilter + st.NeedsReadEscalation = parent.NeedsReadEscalation + st.NeedsWriteEscalation = parent.NeedsWriteEscalation + st.ThreadHasReadEscalation = parent.ThreadHasReadEscalation + st.ThreadHasWriteEscalation = parent.ThreadHasWriteEscalation + st.SessionlessPIDAttach = parent.SessionlessPIDAttach + return st +} + // writeReadyFile writes the sentinel file if configured and not yet written. // Retries up to 3 times on failure before giving up. func (t *Tracer) writeReadyFile() { @@ -723,23 +810,24 @@ func (t *Tracer) handleStop(ctx context.Context, tid int, status unix.WaitStatus t.mu.Unlock() // Auto-attached children may receive this stop before - // handleNewChild creates their state. Create minimal - // state and resume to avoid leaving them stuck. + // handleNewChild creates their state. Seed full + // enforcement state from the parent immediately so a + // child that execve's in this window has the same + // SessionID / prefilter / escalation flags it would + // have had via handleNewChild -- otherwise HandleExecve + // previously saw session_id="" and denied with EACCES, + // which raced the new ELF's startup in ld.so and + // crashed the tracee mid-injection. if !hasState { childTGID, _ := readTGID(tid) if childTGID == 0 { childTGID = tid } + parentPID, _ := readPPID(tid) t.mu.Lock() if _, exists := t.tracees[tid]; !exists { - t.tracees[tid] = &TraceeState{ - TID: tid, - TGID: childTGID, - LastNr: -1, - MemFD: -1, - PendingExecStubFD: -1, - PendingExecSavedFD: -1, - } + parent := findParentByTGID(t.tracees, parentPID) + t.tracees[tid] = seedChildStateFromParent(parent, tid, childTGID) t.metrics.SetTraceeCount(len(t.tracees)) } t.mu.Unlock() @@ -1378,28 +1466,10 @@ func (t *Tracer) handleNewChild(parentTID int, event int) { existing.ThreadHasWriteEscalation = parent.ThreadHasWriteEscalation existing.Attached = time.Now() } else { - pendingPrefilter := false - if !parent.HasPrefilter { - pendingPrefilter = parent.PendingPrefilter - } - t.tracees[tid] = &TraceeState{ - TID: tid, - TGID: childTGID, - ParentPID: parent.TGID, - SessionID: parent.SessionID, - HasPrefilter: parent.HasPrefilter, - PendingPrefilter: pendingPrefilter, - NeedsReadEscalation: parent.NeedsReadEscalation, - NeedsWriteEscalation: parent.NeedsWriteEscalation, - ThreadHasReadEscalation: parent.ThreadHasReadEscalation, - ThreadHasWriteEscalation: parent.ThreadHasWriteEscalation, - Attached: time.Now(), - LastNr: -1, - MemFD: -1, - PendingExecStubFD: -1, - PendingExecSavedFD: -1, - SuppressInitialStop: true, - } + // Shared with the two minimal-state fallback paths in + // run()/handleEventStop() so a child created via either path + // is byte-identical in enforcement state. + t.tracees[tid] = seedChildStateFromParent(parent, tid, childTGID) } t.metrics.SetTraceeCount(len(t.tracees)) t.mu.Unlock() @@ -1650,21 +1720,19 @@ func (t *Tracer) handleEventStop(tid int) { // Both are correctly resumed with PtraceSyscall/PtraceCont; PTRACE_LISTEN // is not needed here. if !hasState { - // Create minimal state so the child doesn't get lost. + // Create minimal state so the child doesn't get lost. Seed full + // enforcement state from the parent via seedChildStateFromParent + // (see the matching block higher up in run() and the helper + // doc for the full rationale). childTGID, _ := readTGID(tid) if childTGID == 0 { childTGID = tid } + parentPID, _ := readPPID(tid) t.mu.Lock() if _, exists := t.tracees[tid]; !exists { - t.tracees[tid] = &TraceeState{ - TID: tid, - TGID: childTGID, - LastNr: -1, - MemFD: -1, - PendingExecStubFD: -1, - PendingExecSavedFD: -1, - } + parent := findParentByTGID(t.tracees, parentPID) + t.tracees[tid] = seedChildStateFromParent(parent, tid, childTGID) t.metrics.SetTraceeCount(len(t.tracees)) } t.mu.Unlock() @@ -1713,10 +1781,12 @@ func (t *Tracer) handleExecve(ctx context.Context, tid int, sc *SyscallContext) state := t.tracees[tid] var tgid, parentPID int var sessionID string + var sessionlessPIDAttach bool if state != nil { tgid = state.TGID parentPID = state.ParentPID sessionID = state.SessionID + sessionlessPIDAttach = state.SessionlessPIDAttach } t.mu.Unlock() @@ -1726,13 +1796,14 @@ func (t *Tracer) handleExecve(ctx context.Context, tid int, sc *SyscallContext) depth := t.processTree.Depth(tgid) result := t.cfg.ExecHandler.HandleExecve(ctx, ExecContext{ - PID: tgid, - ParentPID: parentPID, - Filename: filename, - Argv: argv, - Truncated: truncated, - SessionID: sessionID, - Depth: depth, + PID: tgid, + ParentPID: parentPID, + Filename: filename, + Argv: argv, + Truncated: truncated, + SessionID: sessionID, + Depth: depth, + SessionlessPIDAttach: sessionlessPIDAttach, }) // Dispatch based on Action field (preferred) or Allow field (legacy fallback). diff --git a/internal/ptrace/tracer_inherit_test.go b/internal/ptrace/tracer_inherit_test.go new file mode 100644 index 00000000..61b71704 --- /dev/null +++ b/internal/ptrace/tracer_inherit_test.go @@ -0,0 +1,142 @@ +//go:build linux + +package ptrace + +import ( + "testing" +) + +// TestFindParentByTGID covers the helper used by the minimal-state +// fallback callers to look up a parent by TGID. +func TestFindParentByTGID(t *testing.T) { + tracees := map[int]*TraceeState{ + 100: {TID: 100, TGID: 100, SessionID: "sess-A"}, + 101: {TID: 101, TGID: 100, SessionID: "sess-A"}, // sibling thread shares TGID + 200: {TID: 200, TGID: 200, SessionID: "sess-B"}, + } + + if p := findParentByTGID(tracees, 100); p == nil || p.SessionID != "sess-A" { + t.Fatalf("findParentByTGID(100) returned %+v", p) + } + if p := findParentByTGID(tracees, 200); p == nil || p.SessionID != "sess-B" { + t.Fatalf("findParentByTGID(200) returned %+v", p) + } + if p := findParentByTGID(tracees, 999); p != nil { + t.Fatalf("findParentByTGID(unknown) returned %+v, want nil", p) + } + for _, ppid := range []int{0, -1} { + if p := findParentByTGID(tracees, ppid); p != nil { + t.Fatalf("findParentByTGID(%d) returned %+v, want nil", ppid, p) + } + } +} + +// TestSeedChildStateFromParent_CopiesAllEnforcementFields verifies the +// helper mirrors handleNewChild's create-from-scratch branch byte-for- +// byte (per PR #312 review). A child created via the minimal-state +// fallback path must end up indistinguishable from one created via the +// normal fork-event path in every enforcement-relevant field: +// SessionID, HasPrefilter, PendingPrefilter (skipped when parent +// already has filter), TGID-level escalation, thread-level escalation, +// and the SessionlessPIDAttach marker. +func TestSeedChildStateFromParent_CopiesAllEnforcementFields(t *testing.T) { + parent := &TraceeState{ + TID: 100, + TGID: 100, + SessionID: "sess-A", + HasPrefilter: true, + PendingPrefilter: true, // should be skipped: parent already has filter + NeedsReadEscalation: true, + NeedsWriteEscalation: true, + ThreadHasReadEscalation: true, + ThreadHasWriteEscalation: true, + SessionlessPIDAttach: true, + } + + child := seedChildStateFromParent(parent, 200, 200) + if child == nil { + t.Fatal("seedChildStateFromParent returned nil") + } + + // Identity / bookkeeping + if child.TID != 200 || child.TGID != 200 { + t.Fatalf("child TID/TGID = %d/%d, want 200/200", child.TID, child.TGID) + } + if child.ParentPID != parent.TGID { + t.Errorf("ParentPID = %d, want %d", child.ParentPID, parent.TGID) + } + if !child.SuppressInitialStop { + t.Error("SuppressInitialStop must be true on a freshly-seeded child") + } + + // Enforcement state copied + if child.SessionID != parent.SessionID { + t.Errorf("SessionID = %q, want %q", child.SessionID, parent.SessionID) + } + if !child.HasPrefilter { + t.Error("HasPrefilter must be inherited (parent had it)") + } + // Parent already had the filter installed -> child inherits it via + // fork, so PendingPrefilter must NOT be propagated. + if child.PendingPrefilter { + t.Error("PendingPrefilter must be false when parent already has filter installed") + } + if !child.NeedsReadEscalation || !child.NeedsWriteEscalation { + t.Error("TGID-level escalation flags must be inherited") + } + if !child.ThreadHasReadEscalation || !child.ThreadHasWriteEscalation { + t.Error("thread-level escalation flags must be inherited") + } + if !child.SessionlessPIDAttach { + t.Error("SessionlessPIDAttach must be inherited so descendants of an attach_mode=pid root keep the marker") + } + + // Per-thread runtime defaults + if child.LastNr != -1 || child.MemFD != -1 || + child.PendingExecStubFD != -1 || child.PendingExecSavedFD != -1 { + t.Errorf("runtime defaults wrong: LastNr=%d MemFD=%d PendingExecStubFD=%d PendingExecSavedFD=%d", + child.LastNr, child.MemFD, child.PendingExecStubFD, child.PendingExecSavedFD) + } +} + +// TestSeedChildStateFromParent_PendingPrefilterPropagatedWhenParentLacksFilter +// verifies the conditional: when the parent does NOT have the prefilter +// installed yet (e.g. PendingPrefilter is true but HasPrefilter is +// false), the child must also be marked PendingPrefilter so the tracer +// installs it on the child's next syscall stop. +func TestSeedChildStateFromParent_PendingPrefilterPropagatedWhenParentLacksFilter(t *testing.T) { + parent := &TraceeState{ + TID: 100, TGID: 100, + SessionID: "sess-A", HasPrefilter: false, PendingPrefilter: true, + } + child := seedChildStateFromParent(parent, 200, 200) + if child.HasPrefilter { + t.Error("child HasPrefilter must mirror parent (false here)") + } + if !child.PendingPrefilter { + t.Error("child PendingPrefilter must be true when parent has pending-but-not-installed filter") + } +} + +// TestSeedChildStateFromParent_NilParent verifies the helper returns a +// state with defaults only (no enforcement inheritance) when the parent +// is not known to the tracer. This path is hit on the very first +// fork-attach race before any tracee state exists. +func TestSeedChildStateFromParent_NilParent(t *testing.T) { + child := seedChildStateFromParent(nil, 200, 200) + if child == nil { + t.Fatal("seedChildStateFromParent(nil, ...) returned nil") + } + if child.TID != 200 || child.TGID != 200 { + t.Errorf("child TID/TGID = %d/%d, want 200/200", child.TID, child.TGID) + } + if child.SessionID != "" || child.HasPrefilter || child.PendingPrefilter || + child.NeedsReadEscalation || child.NeedsWriteEscalation || + child.ThreadHasReadEscalation || child.ThreadHasWriteEscalation || + child.SessionlessPIDAttach || child.ParentPID != 0 { + t.Errorf("nil-parent child must have zero enforcement fields; got %+v", child) + } + if child.LastNr != -1 || child.MemFD != -1 { + t.Error("nil-parent child must still have per-thread defaults") + } +}