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