From f97549861f8c5ea5fb08cb1a89c9ebb17cec1bab Mon Sep 17 00:00:00 2001 From: Ben Gittins <5601410+ihavespoons@users.noreply.github.com> Date: Mon, 2 Nov 2026 22:11:33 +1000 Subject: [PATCH] Run every wrap into a macOS session inside that session A second `agentmon wrap --session` into a session ran its agent outside the session, with no policy (SECENG-312). The daemon sent the extension only the first root PID. wrap-init waited for any snapshot of the session, so it returned at once when the extension already held one. The tracker now keeps every root of a session, and the snapshot sends them as root_pids. wrap-init waits for a snapshot that names its own PID. For a session that is already registered, it also posts the policy-update notification, so that the extension fetches the session again. If that wait times out, the session stays, because another agent runs in it. The extension maps each root in root_pids to the session, and drops its "outside" answers when a new root arrives. scripts/test-macos-wrap-session-reuse.sh failed 2 of 4 on build 1676 and passes 4 of 4 on build 1677. Co-Authored-By: Claude Opus 5.5 --- internal/api/app.go | 10 ++ internal/api/wrap_esf_darwin.go | 25 +++- internal/api/wrap_esf_darwin_test.go | 88 ++++++++++++++ .../platform/darwin/policysock/handler.go | 8 +- .../darwin/policysock/handler_test.go | 9 +- .../policysock/network_enforcement_test.go | 45 ++++++- .../platform/darwin/policysock/protocol.go | 7 +- .../platform/darwin/policysock/sessions.go | 84 ++++++++++++- .../platform/darwin/policysock/snapshot.go | 1 + .../darwin/policysock/snapshot_test.go | 21 ++++ internal/server/server.go | 8 ++ macos/AgentMon/SessionPolicyCache.swift | 37 +++++- scripts/test-macos-wrap-session-reuse.sh | 113 ++++++++++++++++++ 13 files changed, 444 insertions(+), 12 deletions(-) create mode 100755 scripts/test-macos-wrap-session-reuse.sh diff --git a/internal/api/app.go b/internal/api/app.go index d8d86181..a1054450 100644 --- a/internal/api/app.go +++ b/internal/api/app.go @@ -124,6 +124,14 @@ type App struct { // a notification, and the extension fetches asynchronously. Both // wrap-init and exec block on this before letting a process start. AwaitSnapshot(sessionID string, timeout time.Duration) bool + // AwaitRootDelivered blocks until the extension holds a snapshot of + // the session that names pid as a root. wrap-init needs it, because + // a session that the extension already holds passes AwaitSnapshot + // before the extension knows the new wrap process. + AwaitRootDelivered(sessionID string, pid int32, timeout time.Duration) bool + // RootPIDsForSession lists the session's roots. A non-empty list + // means that the session is already registered. + RootPIDsForSession(sessionID string) []int32 } // acceptNotifyFDForTest, if non-nil, wraps the goroutine launch for @@ -308,6 +316,8 @@ func (a *App) SetSessionTracker(t interface { RegisterProcess(sessionID string, pid, ppid int32) EndSession(sessionID string) AwaitSnapshot(sessionID string, timeout time.Duration) bool + AwaitRootDelivered(sessionID string, pid int32, timeout time.Duration) bool + RootPIDsForSession(sessionID string) []int32 }) { a.sessionTracker = t } diff --git a/internal/api/wrap_esf_darwin.go b/internal/api/wrap_esf_darwin.go index 287e4cc5..a753d76d 100644 --- a/internal/api/wrap_esf_darwin.go +++ b/internal/api/wrap_esf_darwin.go @@ -54,6 +54,10 @@ func platformWrapInit(a *App, sessionID string, req types.WrapInitRequest) (type fmt.Errorf("wrap: the agentmon system extension is not enforcing, so nothing would constrain this session (%s)", live.Detail) } + // A second `agentmon wrap --session` joins a session that is already + // registered, and that the extension may already hold. + reused := len(a.sessionTracker.RootPIDsForSession(sessionID)) > 0 + // ppid 0 marks this as a session root rather than a tracked child. a.sessionTracker.RegisterProcess(sessionID, int32(req.CallerPID), 0) @@ -71,6 +75,13 @@ func platformWrapInit(a *App, sessionID string, req types.WrapInitRequest) (type // exec sessions then enforce nothing at all. That is a separate, known // defect; this line is what wrap needs once it is fixed. notifySessionRegistered() + // The session-registered notification makes the extension fetch only the + // sessions that it does not hold. For a reused session it must fetch + // again, to learn the new root. The policy-update notification makes it + // fetch every session it holds. + if reused { + notifyPolicyUpdated() + } // Wait for the extension to actually hold the policy before letting the // agent start. @@ -82,12 +93,22 @@ func platformWrapInit(a *App, sessionID string, req types.WrapInitRequest) (type // unenforced -- a `whoami` the policy denies succeeded, and so did a read of // a denied file, while a curl a few hundred milliseconds later was blocked // correctly. An agent's first actions are exactly the ones worth policing. - if !a.sessionTracker.AwaitSnapshot(sessionID, interceptionReadyTimeout) { + // + // The wait is for a snapshot that names this caller as a root. Any + // snapshot of the session is not enough: before, a second wrap into a + // session that the extension held returned at once, and its agent ran + // outside the session with no policy (SECENG-312). + if !a.sessionTracker.AwaitRootDelivered(sessionID, int32(req.CallerPID), interceptionReadyTimeout) { // Undo the registration. A refused wrap-init must leave nothing behind: // the server is telling the caller this session is not wrapped, and a // session root left registered would have the extension attributing // processes to a session the server considers unwrapped. - a.sessionTracker.EndSession(sessionID) + // + // A reused session stays. Another agent runs in it, and ending the + // session would stop the enforcement of that agent. + if !reused { + a.sessionTracker.EndSession(sessionID) + } return types.WrapInitResponse{}, http.StatusServiceUnavailable, fmt.Errorf("wrap: the system extension did not pick up this session's policy within %s, so the agent would start unconstrained", interceptionReadyTimeout) } diff --git a/internal/api/wrap_esf_darwin_test.go b/internal/api/wrap_esf_darwin_test.go index 4d4f3492..ccba9d30 100644 --- a/internal/api/wrap_esf_darwin_test.go +++ b/internal/api/wrap_esf_darwin_test.go @@ -25,6 +25,9 @@ type recordingTracker struct { // picked up the policy. Zero value is "delivered", so tests about other // things do not sit through the wait. notDelivered bool + + // awaited records the PIDs that AwaitRootDelivered waited for. + awaited []int32 } func (r *recordingTracker) RegisterProcess(sessionID string, pid, ppid int32) { @@ -42,6 +45,25 @@ func (r *recordingTracker) AwaitSnapshot(string, time.Duration) bool { return !r.notDelivered } +func (r *recordingTracker) AwaitRootDelivered(_ string, pid int32, _ time.Duration) bool { + r.mu.Lock() + defer r.mu.Unlock() + r.awaited = append(r.awaited, pid) + return !r.notDelivered +} + +func (r *recordingTracker) RootPIDsForSession(sessionID string) []int32 { + r.mu.Lock() + defer r.mu.Unlock() + var out []int32 + for _, reg := range r.registered { + if reg.sessionID == sessionID && reg.ppid == 0 { + out = append(out, reg.pid) + } + } + return out +} + func (r *recordingTracker) EndSession(sessionID string) { r.mu.Lock() defer r.mu.Unlock() @@ -226,3 +248,69 @@ func TestPlatformWrapInit_RefusesWhenSnapshotNeverArrives(t *testing.T) { t.Errorf("refused wrap-init did not unregister the session; ended = %v", tr.ended) } } + +// TestPlatformWrapInit_ReusedSessionWaitsForItsOwnRoot covers SECENG-312. A +// second `agentmon wrap --session` joined a session that the extension +// already held, so the wait for "a snapshot of the session" ended at once. +// That snapshot did not name the new wrap process, and its agent ran outside +// the session. wrap-init must wait for a snapshot that names the caller. +func TestPlatformWrapInit_ReusedSessionWaitsForItsOwnRoot(t *testing.T) { + live := darwin.CheckSysExtLiveness() + if !live.Running { + t.Skip("no running system extension on this machine") + } + darwin.SetExtensionPresenceProbe(func() (bool, time.Time) { return true, time.Now() }) + t.Cleanup(func() { darwin.SetExtensionPresenceProbe(nil) }) + + tr := &recordingTracker{} + a := &App{} + a.SetSessionTracker(tr) + + for _, pid := range []int{4242, 5353} { + if _, _, err := platformWrapInit(a, "sess-1", types.WrapInitRequest{CallerPID: pid}); err != nil { + t.Fatalf("wrap-init for pid %d: %v", pid, err) + } + } + + tr.mu.Lock() + defer tr.mu.Unlock() + if len(tr.awaited) != 2 || tr.awaited[0] != 4242 || tr.awaited[1] != 5353 { + t.Errorf("wrap-init waited for roots %v, want [4242 5353]", tr.awaited) + } +} + +// TestPlatformWrapInit_ReusedSessionTimeoutKeepsSession: when the second +// wrap into a session times out, the session must stay. Another agent runs +// in it, and ending the session would stop the enforcement of that agent. +func TestPlatformWrapInit_ReusedSessionTimeoutKeepsSession(t *testing.T) { + live := darwin.CheckSysExtLiveness() + if !live.Running { + t.Skip("no running system extension on this machine") + } + darwin.SetExtensionPresenceProbe(func() (bool, time.Time) { return true, time.Now() }) + t.Cleanup(func() { darwin.SetExtensionPresenceProbe(nil) }) + + tr := &recordingTracker{} + a := &App{} + a.SetSessionTracker(tr) + if _, _, err := platformWrapInit(a, "sess-1", types.WrapInitRequest{CallerPID: 4242}); err != nil { + t.Fatalf("first wrap-init: %v", err) + } + + tr.mu.Lock() + tr.notDelivered = true + tr.mu.Unlock() + _, code, err := platformWrapInit(a, "sess-1", types.WrapInitRequest{CallerPID: 5353}) + if err == nil { + t.Fatal("second wrap-init succeeded although the extension never named its root") + } + if code != http.StatusServiceUnavailable { + t.Errorf("status = %d, want %d", code, http.StatusServiceUnavailable) + } + + tr.mu.Lock() + defer tr.mu.Unlock() + if len(tr.ended) != 0 { + t.Errorf("a refused second wrap-init ended the session; ended = %v", tr.ended) + } +} diff --git a/internal/platform/darwin/policysock/handler.go b/internal/platform/darwin/policysock/handler.go index 038d4d7c..849a6817 100644 --- a/internal/platform/darwin/policysock/handler.go +++ b/internal/platform/darwin/policysock/handler.go @@ -17,8 +17,9 @@ type SessionResolver interface { SessionForPID(pid int32) string LatestSession() (sessionID string, rootPID int32) RootPIDForSession(sessionID string) int32 + RootPIDsForSession(sessionID string) []int32 ActiveSessions() []string - NoteSnapshotDelivered(sessionID string) + NoteSnapshotDelivered(sessionID string, roots []int32) } // PolicyAdapter adapts the policy.Engine to the PolicyHandler interface. @@ -330,8 +331,10 @@ func (a *PolicyAdapter) BuildPolicySnapshot(sessionID string, clientVersion uint // two registrations; it fetches the latest and would never learn about the // other. Handing back the full list lets it fetch what it is missing. var activeSessions []string + var rootPIDs []int32 if a.sessions != nil { activeSessions = a.sessions.ActiveSessions() + rootPIDs = a.sessions.RootPIDsForSession(sessionID) } // The extension asked for this session's policy and is about to receive it, @@ -339,7 +342,7 @@ func (a *PolicyAdapter) BuildPolicySnapshot(sessionID string, clientVersion uint // letting the agent start; without it the agent's first commands run before // the extension has any policy at all. if a.sessions != nil { - a.sessions.NoteSnapshotDelivered(sessionID) + a.sessions.NoteSnapshotDelivered(sessionID, rootPIDs) } enforcement := networkEnforcement(p) @@ -355,6 +358,7 @@ func (a *PolicyAdapter) BuildPolicySnapshot(sessionID string, clientVersion uint Allow: true, SessionID: sessionID, RootPID: rootPID, + RootPIDs: rootPIDs, SnapshotVersion: nextSnapshotVersion(), FileRules: fileRules, NetworkRules: networkRules, diff --git a/internal/platform/darwin/policysock/handler_test.go b/internal/platform/darwin/policysock/handler_test.go index c98d21e8..b2a06f0a 100644 --- a/internal/platform/darwin/policysock/handler_test.go +++ b/internal/platform/darwin/policysock/handler_test.go @@ -387,7 +387,14 @@ func (m *mockSessionResolver) RootPIDForSession(sessionID string) int32 { return 0 } -func (m *mockSessionResolver) NoteSnapshotDelivered(string) {} +func (m *mockSessionResolver) RootPIDsForSession(sessionID string) []int32 { + if pid := m.RootPIDForSession(sessionID); pid != 0 { + return []int32{pid} + } + return nil +} + +func (m *mockSessionResolver) NoteSnapshotDelivered(string, []int32) {} func (m *mockSessionResolver) ActiveSessions() []string { seen := make(map[string]struct{}, len(m.sessions)) diff --git a/internal/platform/darwin/policysock/network_enforcement_test.go b/internal/platform/darwin/policysock/network_enforcement_test.go index b7085138..f93340bb 100644 --- a/internal/platform/darwin/policysock/network_enforcement_test.go +++ b/internal/platform/darwin/policysock/network_enforcement_test.go @@ -222,7 +222,7 @@ func TestAwaitSnapshot(t *testing.T) { t.Run("returns immediately once delivered", func(t *testing.T) { tracker := NewSessionTracker() tracker.RegisterProcess("session-a", 100, 0) - tracker.NoteSnapshotDelivered("session-a") + tracker.NoteSnapshotDelivered("session-a", nil) start := time.Now() if !tracker.AwaitSnapshot("session-a", time.Minute) { @@ -240,7 +240,7 @@ func TestAwaitSnapshot(t *testing.T) { go func() { time.Sleep(50 * time.Millisecond) - tracker.NoteSnapshotDelivered("session-a") + tracker.NoteSnapshotDelivered("session-a", nil) }() if !tracker.AwaitSnapshot("session-a", 5*time.Second) { @@ -279,7 +279,7 @@ func TestAwaitSnapshot(t *testing.T) { } // Giving up must not latch the session off: the extension may still // arrive, and later commands should see it. - tracker.NoteSnapshotDelivered("session-a") + tracker.NoteSnapshotDelivered("session-a", nil) if !tracker.AwaitSnapshot("session-a", time.Second) { t.Error("a snapshot delivered after a timeout was ignored") } @@ -288,7 +288,7 @@ func TestAwaitSnapshot(t *testing.T) { t.Run("ending a session clears both flags", func(t *testing.T) { tracker := NewSessionTracker() tracker.RegisterProcess("session-a", 100, 0) - tracker.NoteSnapshotDelivered("session-a") + tracker.NoteSnapshotDelivered("session-a", nil) tracker.EndSession("session-a") // A reused ID must not inherit the old session's readiness, or its @@ -340,3 +340,40 @@ func TestNoSessionSnapshotShape(t *testing.T) { t.Errorf("rule = %v, want no-policy-fail-closed", decoded["rule"]) } } + +// TestAwaitRootDelivered covers SECENG-312: a second wrap into a session +// must wait for a snapshot that names its own process, not for any snapshot +// of the session. +func TestAwaitRootDelivered(t *testing.T) { + tracker := NewSessionTracker() + tracker.RegisterProcess("session-a", 100, 0) + tracker.NoteSnapshotDelivered("session-a", tracker.RootPIDsForSession("session-a")) + + tracker.RegisterProcess("session-a", 200, 0) + tracker.RegisterProcess("session-a", 300, 100) // a child, not a root + if got := tracker.RootPIDsForSession("session-a"); len(got) != 2 || got[0] != 100 || got[1] != 200 { + t.Fatalf("RootPIDsForSession = %v, want [100 200]", got) + } + if !tracker.AwaitSnapshot("session-a", time.Millisecond) { + t.Fatal("AwaitSnapshot: the session has a delivered snapshot") + } + if tracker.AwaitRootDelivered("session-a", 200, 20*time.Millisecond) { + t.Fatal("AwaitRootDelivered reported pid 200 delivered; no snapshot named it") + } + + go func() { + time.Sleep(20 * time.Millisecond) + tracker.NoteSnapshotDelivered("session-a", tracker.RootPIDsForSession("session-a")) + }() + if !tracker.AwaitRootDelivered("session-a", 200, 5*time.Second) { + t.Fatal("AwaitRootDelivered gave up on a snapshot that named pid 200") + } + + tracker.EndSession("session-a") + if got := tracker.RootPIDsForSession("session-a"); got != nil { + t.Errorf("RootPIDsForSession after EndSession = %v, want nil", got) + } + if tracker.AwaitRootDelivered("session-a", 200, time.Millisecond) { + t.Error("AwaitRootDelivered reported a root of an ended session") + } +} diff --git a/internal/platform/darwin/policysock/protocol.go b/internal/platform/darwin/policysock/protocol.go index 2be15754..6e3ca947 100644 --- a/internal/platform/darwin/policysock/protocol.go +++ b/internal/platform/darwin/policysock/protocol.go @@ -140,9 +140,14 @@ type PolicyResponse struct { // PendingApprovals answers list_approvals. PendingApprovals []PendingApproval `json:"pending_approvals,omitempty"` - // Policy snapshot fields (returned by fetch_policy_snapshot) + // Policy snapshot fields (returned by fetch_policy_snapshot). + // + // RootPIDs lists every root of the session: root_pid first, then the + // process of each later `agentmon wrap --session` into it. The extension + // maps each one to the session. SnapshotVersion uint64 `json:"version,omitempty"` RootPID int32 `json:"root_pid,omitempty"` + RootPIDs []int32 `json:"root_pids,omitempty"` FileRules []SnapshotFileRule `json:"file_rules,omitempty"` NetworkRules []SnapshotNetworkRule `json:"network_rules,omitempty"` DNSRules []SnapshotDNSRule `json:"dns_rules,omitempty"` diff --git a/internal/platform/darwin/policysock/sessions.go b/internal/platform/darwin/policysock/sessions.go index f8b954e6..e24a73a3 100644 --- a/internal/platform/darwin/policysock/sessions.go +++ b/internal/platform/darwin/policysock/sessions.go @@ -26,6 +26,17 @@ type SessionTracker struct { // sessionID -> root PID (first PID registered for the session) sessionRootPID map[string]int32 + // sessionID -> every PID registered with no parent, in order. The first + // is sessionRootPID. Each later `agentmon wrap --session` into the same + // session adds its own wrap process here. The snapshot sends them all, so + // that the extension maps each one to the session. + sessionRoots map[string][]int32 + + // sessionID -> the roots that a delivered snapshot listed. A wrap into a + // session that the extension already holds must wait for a snapshot that + // names its own PID, not for any snapshot of the session. + rootsDelivered map[string]map[int32]struct{} + // sessionIDs in registration order, most recent last. Go map iteration is // randomised, so "latest" cannot be derived from sessionRootPID -- see // LatestSession. @@ -48,6 +59,8 @@ func NewSessionTracker() *SessionTracker { pidToParent: make(map[int32]int32), sessionToPids: make(map[string]map[int32]struct{}), sessionRootPID: make(map[string]int32), + sessionRoots: make(map[string][]int32), + rootsDelivered: make(map[string]map[int32]struct{}), snapshotDelivered: make(map[string]struct{}), snapshotAbandoned: make(map[string]struct{}), @@ -65,13 +78,54 @@ func NewSessionTracker() *SessionTracker { // `whoami` the policy denies succeeded, and so did a read of a denied file. // wrap-init waits on this rather than returning as soon as the notification is // posted. -func (t *SessionTracker) NoteSnapshotDelivered(sessionID string) { +// +// roots are the root PIDs that the delivered snapshot listed. +func (t *SessionTracker) NoteSnapshotDelivered(sessionID string, roots []int32) { if sessionID == "" { return } t.mu.Lock() defer t.mu.Unlock() t.snapshotDelivered[sessionID] = struct{}{} + if t.rootsDelivered[sessionID] == nil { + t.rootsDelivered[sessionID] = make(map[int32]struct{}) + } + for _, pid := range roots { + t.rootsDelivered[sessionID][pid] = struct{}{} + } +} + +// rootDelivered reports whether a delivered snapshot of the session listed +// pid as a root. +func (t *SessionTracker) rootDelivered(sessionID string, pid int32) bool { + t.mu.RLock() + defer t.mu.RUnlock() + _, ok := t.rootsDelivered[sessionID][pid] + return ok +} + +// AwaitRootDelivered blocks until the extension has fetched a snapshot of +// the session that lists pid as a root, or until timeout. It reports +// whether that happened. +// +// wrap-init uses this, and not AwaitSnapshot. When a second wrap joins a +// session, the extension already holds a snapshot of it, so AwaitSnapshot +// returns at once. But that snapshot does not name the new wrap process, so +// the extension treats the wrapped agent as outside every session. +func (t *SessionTracker) AwaitRootDelivered(sessionID string, pid int32, timeout time.Duration) bool { + if sessionID == "" || pid <= 0 { + return false + } + deadline := time.Now().Add(timeout) + for { + if t.rootDelivered(sessionID, pid) { + return true + } + if !time.Now().Before(deadline) { + return false + } + time.Sleep(snapshotPollInterval) + } } // SnapshotDelivered reports whether the extension has fetched this session's @@ -161,6 +215,18 @@ func (t *SessionTracker) RegisterProcess(sessionID string, pid, ppid int32) { t.sessionRootPID[sessionID] = pid t.sessionOrder = append(t.sessionOrder, sessionID) } + if ppid <= 0 && !containsPID(t.sessionRoots[sessionID], pid) { + t.sessionRoots[sessionID] = append(t.sessionRoots[sessionID], pid) + } +} + +func containsPID(pids []int32, pid int32) bool { + for _, p := range pids { + if p == pid { + return true + } + } + return false } // SetParent records a parent-child relationship (from fork events). @@ -200,6 +266,8 @@ func (t *SessionTracker) EndSession(sessionID string) { // accumulated forever and LatestSession could hand back a terminated // session's root PID (AUDIT M30). delete(t.sessionRootPID, sessionID) + delete(t.sessionRoots, sessionID) + delete(t.rootsDelivered, sessionID) // Same reason: an ended session's delivery flag must not outlive it, or a // later session reusing the ID would be reported as already enforced. delete(t.snapshotDelivered, sessionID) @@ -307,6 +375,20 @@ func (t *SessionTracker) RootPIDForSession(sessionID string) int32 { return t.sessionRootPID[sessionID] } +// RootPIDsForSession returns every root PID of a session, first registered +// first. It returns a copy. +func (t *SessionTracker) RootPIDsForSession(sessionID string) []int32 { + t.mu.RLock() + defer t.mu.RUnlock() + roots := t.sessionRoots[sessionID] + if len(roots) == 0 { + return nil + } + out := make([]int32, len(roots)) + copy(out, roots) + return out +} + // RegisterSession implements SessionRegistrar by registering the root PID // for a session. This is called when the system extension registers a session. func (t *SessionTracker) RegisterSession(rootPID int32, sessionID string) { diff --git a/internal/platform/darwin/policysock/snapshot.go b/internal/platform/darwin/policysock/snapshot.go index eb9f6214..83ece5e5 100644 --- a/internal/platform/darwin/policysock/snapshot.go +++ b/internal/platform/darwin/policysock/snapshot.go @@ -10,6 +10,7 @@ type PolicySnapshotResponse struct { Version uint64 `json:"version"` SessionID string `json:"session_id"` RootPID int32 `json:"root_pid"` + RootPIDs []int32 `json:"root_pids,omitempty"` FileRules []SnapshotFileRule `json:"file_rules"` NetworkRules []SnapshotNetworkRule `json:"network_rules"` DNSRules []SnapshotDNSRule `json:"dns_rules"` diff --git a/internal/platform/darwin/policysock/snapshot_test.go b/internal/platform/darwin/policysock/snapshot_test.go index cbdf0b03..52616306 100644 --- a/internal/platform/darwin/policysock/snapshot_test.go +++ b/internal/platform/darwin/policysock/snapshot_test.go @@ -7,6 +7,7 @@ import ( "slices" "sort" "testing" + "time" "github.com/diffsec/agentmon/internal/policy" "golang.org/x/sys/unix" @@ -156,3 +157,23 @@ func TestMacOSSignalAction(t *testing.T) { } } } + +// TestBuildPolicySnapshot_RootPIDs: the snapshot names every root of the +// session, and the tracker records that the extension received each one. +func TestBuildPolicySnapshot_RootPIDs(t *testing.T) { + engine, err := policy.NewEngine(&policy.Policy{Version: 1}, false, true) + if err != nil { + t.Fatal(err) + } + tracker := NewSessionTracker() + tracker.RegisterProcess("s1", 100, 0) + tracker.RegisterProcess("s1", 200, 0) + + resp := NewPolicyAdapter(engine, tracker).BuildPolicySnapshot("s1", 0) + if resp.RootPID != 100 || !slices.Equal(resp.RootPIDs, []int32{100, 200}) { + t.Errorf("root_pid = %d, root_pids = %v, want 100 and [100 200]", resp.RootPID, resp.RootPIDs) + } + if !tracker.AwaitRootDelivered("s1", 200, time.Millisecond) { + t.Error("the tracker did not record that the snapshot named pid 200") + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 00c46aee..0b1b8cf6 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -110,6 +110,14 @@ type Server struct { // session's policy, so neither a wrapped agent nor an exec'd command // can start before anything can enforce against it. AwaitSnapshot(sessionID string, timeout time.Duration) bool + // AwaitRootDelivered blocks until the extension holds a snapshot of + // the session that names pid as a root. wrap-init needs it, because + // a session that the extension already holds passes AwaitSnapshot + // before the extension knows the new wrap process. + AwaitRootDelivered(sessionID string, pid int32, timeout time.Duration) bool + // RootPIDsForSession lists the session's roots. A non-empty list + // means that the session is already registered. + RootPIDsForSession(sessionID string) []int32 } } diff --git a/macos/AgentMon/SessionPolicyCache.swift b/macos/AgentMon/SessionPolicyCache.swift index b00ba702..6eba3aab 100644 --- a/macos/AgentMon/SessionPolicyCache.swift +++ b/macos/AgentMon/SessionPolicyCache.swift @@ -93,6 +93,8 @@ class SessionCache { let rootPID: pid_t var version: UInt64 var sessionPIDs: Set + /// Every root of the session, root_pid first. See parseRootPIDs. + var rootPIDs: [pid_t] var fileRules: [FileRule] var networkRules: [NetworkRule] var dnsRules: [DNSRule] @@ -129,6 +131,7 @@ class SessionCache { self.rootPID = rootPID self.version = version self.sessionPIDs = [rootPID] + self.rootPIDs = [rootPID] self.fileRules = fileRules self.networkRules = networkRules self.dnsRules = dnsRules @@ -167,7 +170,9 @@ class SessionPolicyCache { invalidateOutsideSessions() queue.async(flags: .barrier) { self.sessions[sessionID] = snapshot - self.pidToSession[rootPID] = sessionID + for pid in snapshot.rootPIDs { + self.pidToSession[pid] = sessionID + } } } @@ -880,7 +885,22 @@ class SessionPolicyCache { if snapshot.version <= existing.version { return } // Preserve sessionPIDs — they're maintained by fork/exit, not snapshot snapshot.sessionPIDs = existing.sessionPIDs + // A later `agentmon wrap --session` adds a root. Before, only + // the first root was ever mapped, so the second wrap's agent ran + // outside the session with no policy (SECENG-312). + var added = false + for pid in snapshot.rootPIDs where self.pidToSession[pid] != sessionID { + self.pidToSession[pid] = sessionID + snapshot.sessionPIDs.insert(pid) + added = true + } self.sessions[sessionID] = snapshot + // The new root, or a process below it, can hold an "outside" + // answer from before. Drop those answers after the mapping is in + // place, so that no lookup caches "outside" again in between. + if added { + self.invalidateOutsideSessions() + } } } @@ -1210,9 +1230,24 @@ extension SessionCache { ) cache.unixSocketRules = parseUnixSocketRules(json["unix_socket_rules"]) cache.signalRules = parseSignalRules(json["signal_rules"]) + cache.rootPIDs = parseRootPIDs(json["root_pids"], rootPID: rootPID) + cache.sessionPIDs.formUnion(cache.rootPIDs) return cache } + /// The session's roots: root_pid, then the process of each later + /// `agentmon wrap --session` into the session. A daemon that does not + /// send root_pids gives root_pid alone. + static func parseRootPIDs(_ raw: Any?, rootPID: pid_t) -> [pid_t] { + var roots = [rootPID] + for value in raw as? [Any] ?? [] { + guard let pid = value as? Int32 ?? (value as? Int).map({ Int32($0) }), + pid > 0, !roots.contains(pid) else { continue } + roots.append(pid) + } + return roots + } + static func parseUnixSocketRules(_ raw: Any?) -> [UnixSocketRule] { guard let rules = raw as? [[String: Any]] else { return [] } return rules.compactMap { r in diff --git a/scripts/test-macos-wrap-session-reuse.sh b/scripts/test-macos-wrap-session-reuse.sh new file mode 100755 index 00000000..bccd7eab --- /dev/null +++ b/scripts/test-macos-wrap-session-reuse.sh @@ -0,0 +1,113 @@ +#!/usr/bin/env bash +# Hardware test: every `agentmon wrap --session` into one session runs inside +# that session. +# +# Before build 1677 the daemon sent the extension only the session's first +# root PID. A wrap into a session that the extension already held returned at +# once, and its agent ran outside the session with no policy (SECENG-312). +# This script starts two wraps into one session, the first still running +# while the second starts, and expects agent-default to deny a connect to +# /var/run/usbmuxd in both. +# +# Run it on a Mac where AgentMon.app is installed, its extension is enabled, +# and the daemon is running. Run it before each release. +set -euo pipefail + +A=${AGENTMON:-/Applications/AgentMon.app/Contents/MacOS/agentmon} +PY=${PYTHON:-} +if [ -z "$PY" ]; then + for p in /opt/homebrew/bin/python3 /usr/local/bin/python3; do + if [ -x "$p" ]; then PY=$p; break; fi + done +fi +[ -n "$PY" ] || { echo "no python3 found; set PYTHON" >&2; exit 2; } +# Homebrew's python3 is a launcher that execs Python.app. Under wrap, that +# second exec dies with no event (SECENG-311), so wrap runs Python.app itself. +PYAPP=$("$PY" -c 'import os, sys; print(os.path.join(sys.base_prefix, "Resources/Python.app/Contents/MacOS/Python"))') +[ -x "$PYAPP" ] || { echo "$PYAPP is missing" >&2; exit 2; } + +SYS_SOCK=/var/run/usbmuxd +[ -S "$SYS_SOCK" ] || { echo "$SYS_SOCK is missing" >&2; exit 2; } + +WS=$(mktemp -d "${TMPDIR:-/tmp}/amreuse.XXXXXX") +WS=$(cd "$WS" && pwd -P) +SID="" +FIRST_PID="" +status=0 + +cleanup() { + [ -n "$FIRST_PID" ] && kill "$FIRST_PID" 2>/dev/null || true + [ -n "$SID" ] && "$A" session destroy "$SID" >/dev/null 2>&1 || true + rm -rf "$WS" +} +trap 'cleanup' EXIT + +ok() { echo "ok $*"; } +fail() { echo "FAIL $*"; status=1; } + +# probe.py prints "insession" when the connect is denied, which agent-default +# does only to a session process. With a file name, it checks once, then waits +# for that file and checks again. +cat > "$WS/probe.py" <<'PY' +import os, socket, sys, time + +def check(): + s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + try: + s.connect("/var/run/usbmuxd") + return "outside" + except PermissionError: + return "insession" + finally: + s.close() + +print(check(), flush=True) +if len(sys.argv) > 1: + for _ in range(600): + if os.path.exists(sys.argv[1]): + break + time.sleep(0.1) + print(check(), flush=True) +PY + +SID=$("$A" session create --workspace "$WS" --policy agent-default | awk '/^Session/{print $2}') +[ -n "$SID" ] || { echo "could not create a session" >&2; exit 2; } + +wrap() { + (cd "$WS" && "$A" wrap --session "$SID" --report=false -- "$PYAPP" "$WS/probe.py" "$@" 2>>"$WS/err") +} + +# The first wrap checks, then waits until the second wrap finished. +wrap "$WS/go" > "$WS/first.out" & +FIRST_PID=$! +for _ in $(seq 1 100); do + [ -s "$WS/first.out" ] && break + sleep 0.1 +done +got=$(head -n 1 "$WS/first.out") +if [ "$got" = insession ]; then ok "first wrap: in the session"; else fail "first wrap: got '$got', want insession"; fi + +got=$(wrap | tail -n 1) +if [ "$got" = insession ]; then ok "second wrap: in the session"; else fail "second wrap: got '$got', want insession"; fi + +touch "$WS/go" +wait "$FIRST_PID" || true +FIRST_PID="" +got=$(sed -n 2p "$WS/first.out") +if [ "$got" = insession ]; then + ok "first wrap after the second started: still in the session" +else + fail "first wrap after the second started: got '$got', want insession" +fi + +got=$(wrap | tail -n 1) +if [ "$got" = insession ]; then ok "third wrap: in the session"; else fail "third wrap: got '$got', want insession"; fi + +if [ $status -ne 0 ] && [ -s "$WS/err" ]; then + echo "wrap stderr:" + sed 's/^/ /' "$WS/err" +fi + +trap - EXIT +cleanup +exit $status