diff --git a/configs/policies/agent-default.yaml b/configs/policies/agent-default.yaml index 6b5e5dcf..b4470fce 100644 --- a/configs/policies/agent-default.yaml +++ b/configs/policies/agent-default.yaml @@ -843,6 +843,15 @@ package_rules: action: block reason: "Known malware detected" + # Every other finding is allowed. A finding that no rule matches blocks the + # install, and the local provider reports "license unknown" for every + # package, so without this rule every install was blocked. The daemon + # applies this last rule after its package_checks.block_on rules, so a + # stricter block_on still blocks. + - match: {} + action: allow + reason: "No critical vulnerability or malware" + # ============================================================================= # RESOURCE LIMITS # ============================================================================= diff --git a/internal/api/core.go b/internal/api/core.go index 39466259..268558e1 100644 --- a/internal/api/core.go +++ b/internal/api/core.go @@ -931,7 +931,14 @@ func (a *App) execInSessionCore(ctx context.Context, id string, req types.ExecRe // Package install check (after command policy check and approval, before event emission). if a.pkgChecker != nil && pre.EffectiveDecision != types.DecisionDeny { - verdict, pkgErr := a.pkgChecker.Check(ctx, req.Command, req.Args, req.WorkingDir) + // The session's own policy rules, as on the macOS exec path. + var verdict *pkgcheck.Verdict + var pkgErr error + if eng := s.PolicyEngine(); eng != nil { + verdict, pkgErr = a.pkgChecker.CheckForPolicy(ctx, eng.PackageRules(), req.Command, req.Args, req.WorkingDir) + } else { + verdict, pkgErr = a.pkgChecker.Check(ctx, req.Command, req.Args, req.WorkingDir) + } if pkgErr != nil { slog.Warn("package check error", "err", pkgErr) pre.EffectiveDecision = types.DecisionDeny diff --git a/internal/pkgcheck/checker.go b/internal/pkgcheck/checker.go index e014ea59..f9582fe1 100644 --- a/internal/pkgcheck/checker.go +++ b/internal/pkgcheck/checker.go @@ -3,7 +3,10 @@ package pkgcheck import ( "context" "fmt" + "path/filepath" "strings" + "sync" + "time" "github.com/diffsec/agentmon/internal/policy" ) @@ -13,7 +16,12 @@ type CheckerConfig struct { Scope string // "new_packages_only" | "all_installs" Resolvers []Resolver Providers map[string]ProviderEntry - Rules []policy.PackageRule + // Rules are the package rules of the daemon's default policy. Check uses + // them. CheckForPolicy takes a session's own rules instead. + Rules []policy.PackageRule + // BlockOn are the rules from package_checks.block_on. They apply after a + // policy's rules, except its final catch-all. See ComposeRules. + BlockOn []policy.PackageRule Allowlist *Allowlist // Privacy configures which packages are sent to external providers. // An empty PrivacyConfig (the zero value) means no filtering — all @@ -28,8 +36,24 @@ type Checker struct { cfg CheckerConfig orch *Orchestrator eval *Evaluator + + // recent holds the verdicts of the last recentTTL, by install. On macOS + // one `agentmon exec pip install x` is checked twice: by the exec API, + // and again when the system extension sees the exec. Without this, the + // second check resolves the same install again. + mu sync.Mutex + recent map[string]recentVerdict + now func() time.Time +} + +type recentVerdict struct { + verdict Verdict + at time.Time } +// recentTTL is how long Check reuses a verdict for the same install. +const recentTTL = 30 * time.Second + // NewChecker creates a new Checker wired to the given config. func NewChecker(cfg CheckerConfig) *Checker { pf := NewPrivacyFilter(cfg.Privacy) @@ -39,19 +63,83 @@ func NewChecker(cfg CheckerConfig) *Checker { Providers: cfg.Providers, PrivacyFilter: pf, }), - eval: NewEvaluator(cfg.Rules), + eval: NewEvaluator(ComposeRules(cfg.Rules, cfg.BlockOn)), + recent: make(map[string]recentVerdict), + now: time.Now, } } // Check evaluates a command. Returns a nil verdict if the command is not a // recognised package-install operation. +// +// A verdict of allow, warn or block for the same install in the same +// directory is reused for recentTTL. An approve verdict is not reused, +// because each one asks a person. func (c *Checker) Check(ctx context.Context, command string, args []string, workDir string) (*Verdict, error) { - // 1. Classify the command. + return c.check(ctx, c.eval, "default", command, args, workDir) +} + +// CheckForPolicy is Check with a session policy's package rules in place of +// the default policy's. The block_on rules still apply. Before, every check +// used the daemon's default policy, so an agent-default session never got +// agent-default's package rules. +func (c *Checker) CheckForPolicy(ctx context.Context, rules []policy.PackageRule, command string, args []string, workDir string) (*Verdict, error) { + eval := NewEvaluator(ComposeRules(rules, c.cfg.BlockOn)) + return c.check(ctx, eval, fmt.Sprintf("%+v", rules), command, args, workDir) +} + +// check classifies the command and returns a recent verdict for the same +// install under the same rules, or checks it. rulesKey names the rules. +func (c *Checker) check(ctx context.Context, eval *Evaluator, rulesKey string, command string, args []string, workDir string) (*Verdict, error) { + // `python -m pip install x` is a pip install. + command, args = UnwrapInstallCommand(command, args) intent := ClassifyInstallCommand(command, args, c.cfg.Scope) if intent == nil { return nil, nil } + key := rulesKey + "\x00" + recentKey(intent, workDir) + if v, ok := c.recentVerdict(key); ok { + return v, nil + } + v, err := c.resolveAndEvaluate(ctx, eval, command, args, workDir, intent) + if err == nil && v != nil && v.Action != VerdictApprove { + c.mu.Lock() + c.recent[key] = recentVerdict{verdict: *v, at: c.now()} + c.mu.Unlock() + } + return v, err +} +// recentKey names an install: the tool, the directory and the arguments. +// The directory is resolved, because the exec API and macOS ES can name the +// same one differently, for example /var and /private/var. +func recentKey(intent *InstallIntent, workDir string) string { + if dir, err := filepath.EvalSymlinks(workDir); err == nil { + workDir = dir + } + return intent.Tool + "\x00" + workDir + "\x00" + strings.Join(intent.OrigArgs, "\x00") +} + +// recentVerdict returns a copy of a verdict from the last recentTTL, and +// drops the ones that are older. +func (c *Checker) recentVerdict(key string) (*Verdict, bool) { + c.mu.Lock() + defer c.mu.Unlock() + now := c.now() + for k, r := range c.recent { + if now.Sub(r.at) > recentTTL { + delete(c.recent, k) + } + } + r, ok := c.recent[key] + if !ok { + return nil, false + } + v := r.verdict + return &v, true +} + +func (c *Checker) resolveAndEvaluate(ctx context.Context, eval *Evaluator, command string, args []string, workDir string, intent *InstallIntent) (*Verdict, error) { // 2. Find a resolver that can handle this tool. // Strip leading global flags so args[0] is the subcommand, // which is what resolvers' CanResolve expects. @@ -123,7 +211,7 @@ func (c *Checker) Check(ctx context.Context, command string, args []string, work } // 6. Evaluate findings against policy rules. - verdict := c.eval.EvaluateWithContext(EvalContext{ + verdict := eval.EvaluateWithContext(EvalContext{ Findings: findings, Ecosystem: plan.Ecosystem, ProviderErrors: providerErrs, @@ -193,3 +281,32 @@ func buildCheckerSummary(intent *InstallIntent, plan *InstallPlan, verdict *Verd } return sb.String() } + +// ComposeRules puts a policy's package rules first, then the block_on rules. +// A policy's last rule that matches every finding moves to the end. +// Otherwise it would shadow every block_on rule: agent-default ends with an +// allow for every finding, and block_on.vulnerability=high must still block +// a high vulnerability. A policy without a catch-all keeps the evaluator's +// default deny for a finding that no rule matches. +func ComposeRules(policyRules, blockOn []policy.PackageRule) []policy.PackageRule { + out := make([]policy.PackageRule, 0, len(policyRules)+len(blockOn)) + var catchAll *policy.PackageRule + if n := len(policyRules); n > 0 && IsCatchAllRule(policyRules[n-1]) { + catchAll = &policyRules[n-1] + policyRules = policyRules[:n-1] + } + out = append(out, policyRules...) + out = append(out, blockOn...) + if catchAll != nil { + out = append(out, *catchAll) + } + return out +} + +// IsCatchAllRule reports whether a package rule matches every finding. +func IsCatchAllRule(r policy.PackageRule) bool { + m := r.Match + return m.FindingType == "" && m.Severity == "" && len(m.Packages) == 0 && + len(m.NamePatterns) == 0 && len(m.Reasons) == 0 && m.Ecosystem == "" && + m.LicenseSPDX == nil && len(m.Options) == 0 +} diff --git a/internal/pkgcheck/checker_recent_test.go b/internal/pkgcheck/checker_recent_test.go new file mode 100644 index 00000000..bc0e6085 --- /dev/null +++ b/internal/pkgcheck/checker_recent_test.go @@ -0,0 +1,120 @@ +package pkgcheck + +import ( + "context" + "testing" + "time" + + "github.com/diffsec/agentmon/internal/policy" +) + +// countingResolver resolves every pip install to one package and counts the +// calls, so that a test can see whether Check resolved again. +type countingResolver struct{ calls int } + +func (r *countingResolver) Name() string { return "counting" } + +func (r *countingResolver) CanResolve(tool string, _ []string) bool { return tool == "pip" } + +func (r *countingResolver) Resolve(context.Context, string, []string) (*InstallPlan, error) { + r.calls++ + return &InstallPlan{ + Tool: "pip", + Ecosystem: EcosystemPyPI, + Direct: []PackageRef{{Name: "requests", Version: "2.0.0", Direct: true}}, + }, nil +} + +func TestCheckReusesRecentVerdict(t *testing.T) { + res := &countingResolver{} + c := NewChecker(CheckerConfig{Resolvers: []Resolver{res}}) + clock := time.Unix(1000, 0) + c.now = func() time.Time { return clock } + ctx := context.Background() + dir := t.TempDir() + + if _, err := c.Check(ctx, "pip3", []string{"install", "requests"}, dir); err != nil { + t.Fatal(err) + } + // The same install as macOS ES reports it: a versioned name. + if _, err := c.Check(ctx, "/opt/homebrew/bin/pip3.15", []string{"install", "requests"}, dir); err != nil { + t.Fatal(err) + } + // The same install as `python3 -m pip`. + if _, err := c.Check(ctx, "/opt/homebrew/bin/python3", []string{"-m", "pip", "install", "requests"}, dir); err != nil { + t.Fatal(err) + } + if res.calls != 1 { + t.Fatalf("resolved %d times within recentTTL, want 1", res.calls) + } + + clock = clock.Add(recentTTL + time.Second) + if _, err := c.Check(ctx, "pip3", []string{"install", "requests"}, dir); err != nil { + t.Fatal(err) + } + if res.calls != 2 { + t.Errorf("resolved %d times after recentTTL, want 2", res.calls) + } +} + +func TestUnwrapInstallCommand(t *testing.T) { + for _, c := range []struct { + cmd string + args []string + wantCmd string + wantArgs int + }{ + {"/opt/homebrew/bin/python3", []string{"-m", "pip", "install", "x"}, "pip", 2}, + {"python3.15", []string{"-I", "-m", "pip3", "install", "x"}, "pip", 2}, + {"python3", []string{"script.py", "-m", "pip"}, "python3", 3}, + {"python3", []string{"-m", "venv", "x"}, "python3", 3}, + {"node", []string{"-m", "pip"}, "node", 2}, + } { + cmd, args := UnwrapInstallCommand(c.cmd, c.args) + if cmd != c.wantCmd || len(args) != c.wantArgs { + t.Errorf("UnwrapInstallCommand(%s %v) = %s %v", c.cmd, c.args, cmd, args) + } + } + if intent := ClassifyInstallCommand("/opt/homebrew/bin/pip3.15", []string{"install", "x"}, ""); intent == nil || intent.Tool != "pip" { + t.Errorf("pip3.15 install x: intent = %+v, want a pip install", intent) + } +} + +// findingResolver resolves to one package, and findingProvider reports a +// "license unknown" finding for it, as the local provider does. +type findingProvider struct{} + +func (findingProvider) Name() string { return "fake" } +func (findingProvider) Capabilities() []FindingType { return []FindingType{FindingLicense} } +func (findingProvider) CheckBatch(_ context.Context, req CheckRequest) (*CheckResponse, error) { + var out []Finding + for _, p := range req.Packages { + out = append(out, Finding{Type: FindingLicense, Severity: SeverityInfo, Package: p, Provider: "fake"}) + } + return &CheckResponse{Provider: "fake", Findings: out}, nil +} + +// TestCheckForPolicyUsesSessionRules: the default policy has no package +// rules, so a license finding blocks. A session policy that allows every +// other finding, as agent-default does, must allow the same install. +func TestCheckForPolicyUsesSessionRules(t *testing.T) { + c := NewChecker(CheckerConfig{ + Resolvers: []Resolver{&countingResolver{}}, + Providers: map[string]ProviderEntry{"fake": {Provider: findingProvider{}}}, + }) + ctx := context.Background() + dir := t.TempDir() + + v, err := c.Check(ctx, "pip3", []string{"install", "requests"}, dir) + if err != nil || v == nil || v.Action != VerdictBlock { + t.Fatalf("Check with the default rules = %+v, %v; want block", v, err) + } + session := []policy.PackageRule{ + {Match: policy.PackageMatch{FindingType: "malware"}, Action: "block"}, + {Match: policy.PackageMatch{}, Action: "allow"}, + } + v, err = c.CheckForPolicy(ctx, session, "pip3", []string{"install", "requests"}, dir) + if err != nil || v == nil || v.Action != VerdictAllow { + t.Errorf("CheckForPolicy with a catch-all allow = %+v, %v; want allow", v, err) + } +} diff --git a/internal/pkgcheck/compose_rules_test.go b/internal/pkgcheck/compose_rules_test.go new file mode 100644 index 00000000..b9e011c5 --- /dev/null +++ b/internal/pkgcheck/compose_rules_test.go @@ -0,0 +1,49 @@ +package pkgcheck + +import ( + "testing" + + "github.com/diffsec/agentmon/internal/policy" +) + +// TestComposeRules_CatchAllLast: agent-default ends with an allow for +// every finding. A stricter block_on must still block, so the catch-all moves +// after the block_on rules. +func TestComposeRules_CatchAllLast(t *testing.T) { + policyRules := []policy.PackageRule{ + {Match: policy.PackageMatch{FindingType: "malware"}, Action: "block"}, + {Match: policy.PackageMatch{}, Action: "allow"}, + } + // What config.CompileBlockOnRules gives for block_on.vulnerability=high. + blockOn := []policy.PackageRule{ + {Match: policy.PackageMatch{FindingType: "vulnerability", Severity: "critical"}, Action: "deny"}, + {Match: policy.PackageMatch{FindingType: "vulnerability", Severity: "high"}, Action: "deny"}, + } + rules := ComposeRules(policyRules, blockOn) + if last := rules[len(rules)-1]; last.Action != "allow" || !IsCatchAllRule(last) { + t.Fatalf("last rule = %+v, want the policy's catch-all allow", last) + } + + e := NewEvaluator(rules) + high := Finding{Type: FindingVulnerability, Severity: SeverityHigh, + Package: PackageRef{Name: "x"}} + if v := e.Evaluate([]Finding{high}, EcosystemPyPI); v.Action != VerdictBlock { + t.Errorf("high vulnerability with block_on.vulnerability=high: %s, want block", v.Action) + } + license := Finding{Type: FindingLicense, Severity: SeverityInfo, + Package: PackageRef{Name: "x"}} + if v := e.Evaluate([]Finding{license}, EcosystemPyPI); v.Action != VerdictAllow { + t.Errorf("license unknown: %s, want allow", v.Action) + } +} + +// TestComposeRules_NoCatchAll keeps a policy without a catch-all in +// order, so the evaluator's default deny still applies to it. +func TestComposeRules_NoCatchAll(t *testing.T) { + policyRules := []policy.PackageRule{{Match: policy.PackageMatch{FindingType: "malware"}, Action: "block"}} + blockOn := []policy.PackageRule{{Match: policy.PackageMatch{FindingType: "license"}, Action: "deny"}} + rules := ComposeRules(policyRules, blockOn) + if len(rules) != 2 || rules[0].Match.FindingType != "malware" || rules[1].Match.FindingType != "license" { + t.Errorf("rules = %+v", rules) + } +} diff --git a/internal/pkgcheck/interceptor.go b/internal/pkgcheck/interceptor.go index d9f71897..caf2e895 100644 --- a/internal/pkgcheck/interceptor.go +++ b/internal/pkgcheck/interceptor.go @@ -3,6 +3,7 @@ package pkgcheck import ( "path/filepath" "strings" + "unicode" ) // InstallIntent captures the parsed intent of an install command. @@ -68,6 +69,9 @@ func ClassifyInstallCommand(command string, args []string, scope string) *Instal base = strings.TrimSuffix(base, ".exe") base = strings.TrimSuffix(base, ".cmd") base = strings.TrimSuffix(base, ".bat") + // A versioned name such as "pip3.15" is the same tool. Homebrew installs + // pip3.15 next to pip3, and on macOS ES reports resolved paths. + base = trimVersionSuffix(base) switch base { case "npm": @@ -87,6 +91,50 @@ func ClassifyInstallCommand(command string, args []string, scope string) *Instal } } +// trimVersionSuffix removes trailing "." parts: "pip3.15" gives +// "pip3". The part that stays must hold a letter. +func trimVersionSuffix(name string) string { + for { + dot := strings.LastIndexByte(name, '.') + if dot <= 0 || dot == len(name)-1 || !strings.ContainsFunc(name[:dot], unicode.IsLetter) { + return name + } + if strings.Trim(name[dot+1:], "0123456789") != "" { + return name + } + name = name[:dot] + } +} + +// UnwrapInstallCommand returns the package manager command inside a +// `python -m pip ...` command, as ("pip", the arguments after the module). +// Any other command comes back unchanged. +// +// The interpreter's own flags before -m are skipped. A flag that takes a +// value, such as -W or -X, is not expected before -m, so the first argument +// that does not start with "-" ends the search. +func UnwrapInstallCommand(command string, args []string) (string, []string) { + base := strings.ToLower(filepath.Base(command)) + if !strings.HasPrefix(trimVersionSuffix(base), "python") { + return command, args + } + for i, a := range args { + if a == "-m" { + if i+1 < len(args) { + switch trimVersionSuffix(strings.ToLower(args[i+1])) { + case "pip", "pip3": + return "pip", args[i+2:] + } + } + return command, args + } + if !strings.HasPrefix(a, "-") { + return command, args + } + } + return command, args +} + // classifyNPM handles npm install/i/add/ci commands. func classifyNPM(command string, args []string, scope string) *InstallIntent { if len(args) == 0 { diff --git a/internal/pkgcheck/provider/osv.go b/internal/pkgcheck/provider/osv.go index 01f989b3..4ddc1f70 100644 --- a/internal/pkgcheck/provider/osv.go +++ b/internal/pkgcheck/provider/osv.go @@ -7,8 +7,10 @@ import ( "fmt" "io" "net/http" + "net/url" "strconv" "strings" + "sync" "time" "github.com/diffsec/agentmon/internal/pkgcheck" @@ -29,8 +31,16 @@ type OSVConfig struct { type osvProvider struct { baseURL string client *http.Client + + // details caches full vulnerability records by ID. A record changes + // rarely, and one install can name the same IDs again and again. + mu sync.Mutex + details map[string]osvVuln } +// osvDetailFetches is how many vulnerability records are fetched at once. +const osvDetailFetches = 16 + // NewOSVProvider returns a CheckProvider that queries OSV.dev for vulnerabilities. func NewOSVProvider(cfg OSVConfig) pkgcheck.CheckProvider { baseURL := cfg.BaseURL @@ -44,6 +54,7 @@ func NewOSVProvider(cfg OSVConfig) pkgcheck.CheckProvider { return &osvProvider{ baseURL: strings.TrimRight(baseURL, "/"), client: &http.Client{Timeout: timeout}, + details: make(map[string]osvVuln), } } @@ -80,11 +91,11 @@ type osvResult struct { } type osvVuln struct { - ID string `json:"id"` - Summary string `json:"summary"` - Details string `json:"details"` - Severity []osvSeverity `json:"severity,omitempty"` - References []osvReference `json:"references,omitempty"` + ID string `json:"id"` + Summary string `json:"summary"` + Details string `json:"details"` + Severity []osvSeverity `json:"severity,omitempty"` + References []osvReference `json:"references,omitempty"` } type osvSeverity struct { @@ -152,6 +163,14 @@ func (p *osvProvider) CheckBatch(ctx context.Context, req pkgcheck.CheckRequest) iterCount = len(req.Packages) } + // /v1/querybatch returns only the ID and the modified time of each + // vulnerability, with no severity. Without the full records, every + // finding defaulted to medium, so a rule for critical vulnerabilities, + // such as agent-default's, never matched. + if !p.fillDetails(ctx, batchResp.Results[:iterCount]) { + partial = true + } + var findings []pkgcheck.Finding for i := 0; i < iterCount; i++ { result := batchResp.Results[i] @@ -190,6 +209,92 @@ func (p *osvProvider) CheckBatch(ctx context.Context, req pkgcheck.CheckRequest) }, nil } +// fillDetails replaces each vulnerability that has only an ID with its full +// record from /v1/vulns/{id}. It returns false when a record could not be +// fetched. That vulnerability then keeps no severity, and counts as medium. +func (p *osvProvider) fillDetails(ctx context.Context, results []osvResult) bool { + var missing []string + seen := map[string]bool{} + p.mu.Lock() + for _, r := range results { + for _, v := range r.Vulns { + if !needsDetails(v) || seen[v.ID] { + continue + } + seen[v.ID] = true + if _, ok := p.details[v.ID]; !ok { + missing = append(missing, v.ID) + } + } + } + p.mu.Unlock() + + ok := true + if len(missing) > 0 { + var wg sync.WaitGroup + var failMu sync.Mutex + sem := make(chan struct{}, osvDetailFetches) + for _, id := range missing { + wg.Add(1) + sem <- struct{}{} + go func(id string) { + defer wg.Done() + defer func() { <-sem }() + v, err := p.fetchVuln(ctx, id) + if err != nil { + failMu.Lock() + ok = false + failMu.Unlock() + return + } + p.mu.Lock() + p.details[id] = v + p.mu.Unlock() + }(id) + } + wg.Wait() + } + + p.mu.Lock() + defer p.mu.Unlock() + for i := range results { + for j, v := range results[i].Vulns { + if d, found := p.details[v.ID]; found && needsDetails(v) { + results[i].Vulns[j] = d + } + } + } + return ok +} + +// needsDetails reports whether a vulnerability holds only its ID, as the +// batch API returns it. +func needsDetails(v osvVuln) bool { + return v.ID != "" && len(v.Severity) == 0 && v.Summary == "" && v.Details == "" +} + +// fetchVuln gets one vulnerability record from /v1/vulns/{id}. +func (p *osvProvider) fetchVuln(ctx context.Context, id string) (osvVuln, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, p.baseURL+"/v1/vulns/"+url.PathEscape(id), nil) + if err != nil { + return osvVuln{}, err + } + resp, err := p.client.Do(req) + if err != nil { + return osvVuln{}, err + } + // The body is only read. A close error cannot change the record. + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + return osvVuln{}, fmt.Errorf("osv: vulnerability %s: status %d", id, resp.StatusCode) + } + var v osvVuln + if err := json.NewDecoder(resp.Body).Decode(&v); err != nil { + return osvVuln{}, fmt.Errorf("osv: decode vulnerability %s: %w", id, err) + } + return v, nil +} + // mapEcosystemOSV converts our Ecosystem type to the OSV ecosystem string. func mapEcosystemOSV(eco pkgcheck.Ecosystem) string { switch eco { diff --git a/internal/pkgcheck/provider/osv_test.go b/internal/pkgcheck/provider/osv_test.go index 38101f8c..f32920f6 100644 --- a/internal/pkgcheck/provider/osv_test.go +++ b/internal/pkgcheck/provider/osv_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "runtime" + "sync/atomic" "testing" "github.com/diffsec/agentmon/internal/pkgcheck" @@ -223,3 +224,42 @@ func TestParseCVSSBaseScore(t *testing.T) { }) } } + +// TestOSVProvider_FetchesSeverity: /v1/querybatch returns only the ID of each +// vulnerability. The provider must fetch the record to learn the severity. +// Before, every OSV finding was medium, so no critical rule ever matched. +func TestOSVProvider_FetchesSeverity(t *testing.T) { + var detailCalls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v1/querybatch": + _, _ = w.Write([]byte(`{"results":[{"vulns":[{"id":"GHSA-crit","modified":"2024-01-01T00:00:00Z"},{"id":"GHSA-gone","modified":"2024-01-01T00:00:00Z"}]}]}`)) + case "/v1/vulns/GHSA-crit": + detailCalls.Add(1) + _, _ = w.Write([]byte(`{"id":"GHSA-crit","summary":"SQL injection","severity":[{"type":"CVSS_V3","score":"CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H"}]}`)) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + p := NewOSVProvider(OSVConfig{BaseURL: server.URL}) + req := pkgcheck.CheckRequest{ + Ecosystem: pkgcheck.EcosystemPyPI, + Packages: []pkgcheck.PackageRef{{Name: "django", Version: "2.2"}}, + } + resp, err := p.CheckBatch(context.Background(), req) + require.NoError(t, err) + require.Len(t, resp.Findings, 2) + assert.Equal(t, pkgcheck.SeverityCritical, resp.Findings[0].Severity) + assert.Equal(t, "SQL injection", resp.Findings[0].Title) + // A record that could not be fetched keeps the medium default, and the + // response is marked partial. + assert.Equal(t, pkgcheck.SeverityMedium, resp.Findings[1].Severity) + assert.True(t, resp.Metadata.Partial) + + // The second check of the same package reuses the cached record. + _, err = p.CheckBatch(context.Background(), req) + require.NoError(t, err) + assert.Equal(t, int32(1), detailCalls.Load()) +} diff --git a/internal/pkgcheck/resolver/pip.go b/internal/pkgcheck/resolver/pip.go index 6b1832b3..f893b55a 100644 --- a/internal/pkgcheck/resolver/pip.go +++ b/internal/pkgcheck/resolver/pip.go @@ -69,11 +69,14 @@ func (r *pipResolver) Resolve(ctx context.Context, workDir string, command []str ctx, cancel := context.WithTimeout(ctx, r.cfg.Timeout) defer cancel() - // pip install --dry-run --report - [--index-url ] [--extra-index-url ] + // pip install --quiet --dry-run --report - [--index-url ] [--extra-index-url ] // Forward any --index-url / -i / --extra-index-url flags from the original // command so the dry-run resolves against the same index the actual install // will use. - cmdArgs := []string{"install", "--dry-run", "--report", "-"} + // + // --quiet is required. Without it, pip 26 writes "Collecting ..." lines + // to stdout before the JSON report, and the report never parses. + cmdArgs := []string{"install", "--quiet", "--dry-run", "--report", "-"} cmdArgs = append(cmdArgs, r.extractIndexURLFlags(command)...) cmdArgs = append(cmdArgs, packages...) allArgs := append(append([]string(nil), r.prefixArgs...), cmdArgs...) diff --git a/internal/platform/darwin/policysock/handler.go b/internal/platform/darwin/policysock/handler.go index 849a6817..62cf7da5 100644 --- a/internal/platform/darwin/policysock/handler.go +++ b/internal/platform/darwin/policysock/handler.go @@ -28,6 +28,7 @@ type PolicyAdapter struct { sessions SessionResolver sessionEngine func(sessionID string) *policy.Engine approvals *approvals.Manager + packages PackageChecker } // SetSessionEngineLookup gives the adapter each session's own policy engine. @@ -117,7 +118,7 @@ func (a *PolicyAdapter) ResolveSession(pid int32) string { // CheckExec evaluates a command through the exec pipeline, returning // the full decision and action for the ESF client to act on. -func (a *PolicyAdapter) CheckExec(executable string, args []string, pid int32, parentPID int32, sessionID string, _ ExecContext) ExecCheckResult { +func (a *PolicyAdapter) CheckExec(executable string, args []string, pid int32, parentPID int32, sessionID string, ec ExecContext) ExecCheckResult { engine := a.engineFor(sessionID) if engine == nil { slog.Error("policy socket: no policy engine installed, denying exec (fail closed)", @@ -174,12 +175,18 @@ func (a *PolicyAdapter) CheckExec(executable string, args []string, pid int32, p action = "deny" } - return ExecCheckResult{ + result := ExecCheckResult{ Decision: decision, Action: action, Rule: dec.Rule, Message: dec.Message, } + if action == "continue" { + if pkg, ok := a.checkPackageInstall(sessionID, executable, args, ec); ok { + return pkg + } + } + return result } // denyAllSnapshot returns a snapshot that denies everything for the session. diff --git a/internal/platform/darwin/policysock/handler_test.go b/internal/platform/darwin/policysock/handler_test.go index b2a06f0a..9e04048a 100644 --- a/internal/platform/darwin/policysock/handler_test.go +++ b/internal/platform/darwin/policysock/handler_test.go @@ -3,8 +3,12 @@ package policysock import ( + "context" + "errors" "testing" + "time" + "github.com/diffsec/agentmon/internal/pkgcheck" "github.com/diffsec/agentmon/internal/policy" ) @@ -541,3 +545,94 @@ func TestBuildPolicySnapshot_FailsClosed(t *testing.T) { }) } } + +// fakePackageChecker returns one verdict and records what it was asked. +type fakePackageChecker struct { + verdict *pkgcheck.Verdict + err error + command string + args []string + workDir string + deadline time.Duration +} + +func (f *fakePackageChecker) CheckForPolicy(ctx context.Context, _ []policy.PackageRule, command string, args []string, workDir string) (*pkgcheck.Verdict, error) { + f.command, f.args, f.workDir = command, args, workDir + if d, ok := ctx.Deadline(); ok { + f.deadline = time.Until(d) + } + return f.verdict, f.err +} + +// TestCheckExec_PackageInstall: an exec that the command rules allow is then +// package-checked. ES reports a script's interpreter as the executable, so +// `pip3 install x` must be classified from argv[1]. +func TestCheckExec_PackageInstall(t *testing.T) { + p := &policy.Policy{Version: 1, CommandRules: []policy.CommandRule{ + {Name: "allow-all", Commands: []string{"*"}, Decision: "allow"}, + }} + engine, err := policy.NewEngine(p, false, true) + if err != nil { + t.Fatal(err) + } + const py = "/opt/homebrew/Cellar/python@3.15/3.15.0/bin/python3.15" + argv := []string{py, "/opt/homebrew/bin/pip3", "install", "evil"} + ec := ExecContext{CWDPath: "/tmp/ws", Script: "/opt/homebrew/Cellar/python@3.15/3.15.0/bin/pip3", Deadline: 10 * time.Second} + + for _, c := range []struct { + name string + checker *fakePackageChecker + wantAction string + wantRule string + }{ + {"block", &fakePackageChecker{verdict: &pkgcheck.Verdict{Action: pkgcheck.VerdictBlock, Summary: "evil is malware"}}, "deny", "package-check"}, + {"allow", &fakePackageChecker{verdict: &pkgcheck.Verdict{Action: pkgcheck.VerdictAllow}}, "continue", "allow-all"}, + {"warn", &fakePackageChecker{verdict: &pkgcheck.Verdict{Action: pkgcheck.VerdictWarn}}, "continue", "allow-all"}, + {"not an install", &fakePackageChecker{}, "continue", "allow-all"}, + {"error", &fakePackageChecker{err: errors.New("pip dry-run failed")}, "deny", "package-check"}, + {"approve without approvals", &fakePackageChecker{verdict: &pkgcheck.Verdict{Action: pkgcheck.VerdictApprove}}, "deny", "package-check"}, + } { + t.Run(c.name, func(t *testing.T) { + a := NewPolicyAdapter(engine, nil) + a.SetPackageChecker(c.checker) + got := a.CheckExec(py, argv, 100, 1, "s1", ec) + if got.Action != c.wantAction || got.Rule != c.wantRule { + t.Errorf("CheckExec = %s by %q (%s), want %s by %q", got.Action, got.Rule, got.Message, c.wantAction, c.wantRule) + } + if c.checker.command != "/opt/homebrew/bin/pip3" || len(c.checker.args) != 2 || c.checker.workDir != "/tmp/ws" { + t.Errorf("checked %s %v in %s, want /opt/homebrew/bin/pip3 [install evil] in /tmp/ws", + c.checker.command, c.checker.args, c.checker.workDir) + } + if c.checker.deadline <= 0 || c.checker.deadline > 10*time.Second-packageCheckMargin { + t.Errorf("check deadline = %s, want under %s", c.checker.deadline, 10*time.Second-packageCheckMargin) + } + }) + } +} + +// TestCheckExec_PackageCheckSkippedOnDeny: an exec that the command rules +// deny is not package-checked. +func TestCheckExec_PackageCheckSkippedOnDeny(t *testing.T) { + engine, err := policy.NewEngine(&policy.Policy{Version: 1}, false, true) + if err != nil { + t.Fatal(err) + } + f := &fakePackageChecker{verdict: &pkgcheck.Verdict{Action: pkgcheck.VerdictAllow}} + a := NewPolicyAdapter(engine, nil) + a.SetPackageChecker(f) + got := a.CheckExec("/usr/bin/pip3", []string{"/usr/bin/pip3", "install", "x"}, 100, 1, "s1", ExecContext{}) + if got.Action != "deny" || f.command != "" { + t.Errorf("CheckExec = %s, checker asked for %q; want deny with no package check", got.Action, f.command) + } +} + +func TestInstallCommand(t *testing.T) { + cmd, args := installCommand("/usr/local/bin/uv", []string{"uv", "pip", "install", "x"}, "") + if cmd != "/usr/local/bin/uv" || len(args) != 3 { + t.Errorf("binary: %s %v", cmd, args) + } + cmd, args = installCommand("/usr/bin/python3", []string{"python3", "/usr/local/bin/pip3", "install", "x"}, "/usr/local/bin/pip3") + if cmd != "/usr/local/bin/pip3" || len(args) != 2 { + t.Errorf("script: %s %v", cmd, args) + } +} diff --git a/internal/platform/darwin/policysock/package_check.go b/internal/platform/darwin/policysock/package_check.go new file mode 100644 index 00000000..1e86b979 --- /dev/null +++ b/internal/platform/darwin/policysock/package_check.go @@ -0,0 +1,131 @@ +//go:build darwin + +package policysock + +import ( + "context" + "log/slog" + "path/filepath" + "strings" + "time" + + "github.com/diffsec/agentmon/internal/approvals" + "github.com/diffsec/agentmon/internal/pkgcheck" + "github.com/diffsec/agentmon/internal/policy" +) + +// PackageChecker checks a package install against a session policy's +// package rules. *pkgcheck.Checker implements it. It returns a nil verdict +// for a command that is not an install. +type PackageChecker interface { + CheckForPolicy(ctx context.Context, rules []policy.PackageRule, command string, args []string, workDir string) (*pkgcheck.Verdict, error) +} + +// SetPackageChecker makes CheckExec check package installs, as the exec API +// does for the command that it starts. With it, an install that an agent +// starts under `agentmon wrap`, or from inside a command, is checked too. +func (a *PolicyAdapter) SetPackageChecker(c PackageChecker) { + a.packages = c +} + +const ( + // packageCheckFallback is the time for a package check when the + // extension does not say how long it waits. Its own default wait is 2s. + packageCheckFallback = 1500 * time.Millisecond + + // packageCheckMargin is kept free before the extension stops waiting, + // so that the daemon answers before the extension denies on its own. + packageCheckMargin = 500 * time.Millisecond + + // packageRule names the rule in a deny that the package check caused. + packageRule = "package-check" +) + +// installCommand returns the command and arguments to classify as an +// install, from an exec as ES reports it. args holds argv, with argv[0]. +// +// For a script, ES reports the interpreter as the executable. The kernel +// puts the script path, as the caller named it, in argv[1]. So `pip3 +// install x` arrives as python3.15 with argv [python3.15, /opt/homebrew/bin/ +// pip3, install, x], and the install command is pip3 with [install, x]. +func installCommand(executable string, args []string, script string) (string, []string) { + if script != "" && len(args) >= 2 { + return args[1], args[2:] + } + if len(args) >= 1 { + return executable, args[1:] + } + return executable, nil +} + +// checkPackageInstall runs the package check on an exec that the command +// rules allow. ok is false when the exec is not an install, or when the +// check allows it. Then the command rules' verdict stands. +func (a *PolicyAdapter) checkPackageInstall(sessionID, executable string, args []string, ec ExecContext) (ExecCheckResult, bool) { + if a.packages == nil { + return ExecCheckResult{}, false + } + command, cmdArgs := installCommand(executable, args, ec.Script) + + budget := packageCheckFallback + if ec.Deadline > 0 { + budget = ec.Deadline - packageCheckMargin + } + if budget <= 0 { + return packageDeny("package check: no time left before the system extension stops waiting"), true + } + ctx, cancel := context.WithTimeout(context.Background(), budget) + defer cancel() + + var rules []policy.PackageRule + if engine := a.engineFor(sessionID); engine != nil { + rules = engine.PackageRules() + } + verdict, err := a.packages.CheckForPolicy(ctx, rules, command, cmdArgs, ec.CWDPath) + if err != nil { + // The exec API denies on the same error (api/core.go). + slog.Warn("policy socket: package check failed, denying the exec", + "command", command, "session_id", sessionID, "error", err) + return packageDeny("package check failed: " + err.Error()), true + } + if verdict == nil { + return ExecCheckResult{}, false + } + + switch verdict.Action { + case pkgcheck.VerdictBlock: + return packageDeny(verdict.Summary), true + case pkgcheck.VerdictApprove: + return a.packageApproval(sessionID, command, cmdArgs, verdict), true + case pkgcheck.VerdictWarn: + slog.Warn("policy socket: package install warning", "session_id", sessionID, "summary", verdict.Summary) + } + return ExecCheckResult{}, false +} + +func packageDeny(message string) ExecCheckResult { + return ExecCheckResult{Decision: "deny", Action: "deny", Rule: packageRule, Message: message} +} + +// packageApproval handles an approve verdict as execApproval handles an +// approve rule. The extension cannot hold an exec while a person reads a +// dialog, so the install is denied and a dialog opens. If the person +// approves, the same install succeeds when the agent tries it again. +func (a *PolicyAdapter) packageApproval(sessionID, command string, args []string, verdict *pkgcheck.Verdict) ExecCheckResult { + target := strings.Join(append([]string{filepath.Base(command)}, args...), " ") + if a.approvals == nil || sessionID == "" { + return packageDeny("package install needs approval, and no approval dialog is available: " + verdict.Summary) + } + if a.approvals.Granted(sessionID, "package", target, "install") { + return ExecCheckResult{Decision: "approve", Action: "continue", Rule: packageRule, Message: verdict.Summary} + } + a.approvals.Ask(approvals.Request{ + SessionID: sessionID, + Kind: "package", + Target: target, + Rule: packageRule, + Message: verdict.Summary, + Fields: map[string]any{"operation": "install"}, + }) + return packageDeny("package install needs approval: " + verdict.Summary) +} diff --git a/internal/platform/darwin/policysock/protocol.go b/internal/platform/darwin/policysock/protocol.go index 6e3ca947..49ccd5c0 100644 --- a/internal/platform/darwin/policysock/protocol.go +++ b/internal/platform/darwin/policysock/protocol.go @@ -2,6 +2,8 @@ package policysock +import "time" + // RequestType identifies the type of policy check. type RequestType string @@ -112,6 +114,13 @@ type PolicyRequest struct { // Exec context fields (from ESF event) TTYPath string `json:"tty_path,omitempty"` // Controlling terminal path CWDPath string `json:"cwd_path,omitempty"` // Working directory of the exec'ing process + // Script is the script that an interpreter runs, as ES reports it. For + // `pip3 install x`, the executable is python3.15 and args[1] is the + // pip3 script. Empty for a binary. + Script string `json:"script,omitempty"` + // DeadlineMS is the time left before the extension stops waiting for + // the verdict. 0 means the request does not say. + DeadlineMS int64 `json:"deadline_ms,omitempty"` // Exec depth tracking (matches Linux seccomp depth) Depth int `json:"depth,omitempty"` @@ -173,6 +182,11 @@ type PolicyResponse struct { type ExecContext struct { TTYPath string // Controlling terminal path (e.g. /dev/ttys001) CWDPath string // Working directory of the exec'ing process + // Script is the script that an interpreter runs. Empty for a binary. + Script string + // Deadline is how long the extension waits for the verdict. 0 means + // unknown. + Deadline time.Duration } // ApprovalResponse represents a pending approval request returned to the client. diff --git a/internal/platform/darwin/policysock/server.go b/internal/platform/darwin/policysock/server.go index e2f2f282..9ace4b9c 100644 --- a/internal/platform/darwin/policysock/server.go +++ b/internal/platform/darwin/policysock/server.go @@ -511,8 +511,10 @@ func (s *Server) handleExecCheck(req *PolicyRequest) PolicyResponse { } result := h.CheckExec(req.Path, req.Args, req.PID, req.ParentPID, req.SessionID, ExecContext{ - TTYPath: req.TTYPath, - CWDPath: req.CWDPath, + TTYPath: req.TTYPath, + CWDPath: req.CWDPath, + Script: req.Script, + Deadline: time.Duration(req.DeadlineMS) * time.Millisecond, }) return PolicyResponse{ Allow: result.Action == "continue", diff --git a/internal/policy/agent_policies_test.go b/internal/policy/agent_policies_test.go index 9cd33ffa..b8f4654f 100644 --- a/internal/policy/agent_policies_test.go +++ b/internal/policy/agent_policies_test.go @@ -201,12 +201,16 @@ func TestAgentPolicies_DefaultRuleDetails(t *testing.T) { assert.Contains(t, p.EnvPolicy.Deny, "*_SECRET*") // --- Package rules --- - require.Len(t, p.PackageRules, 2) + require.Len(t, p.PackageRules, 3) assert.Equal(t, "block", p.PackageRules[0].Action) assert.Equal(t, "vulnerability", p.PackageRules[0].Match.FindingType) assert.Equal(t, "critical", p.PackageRules[0].Match.Severity) assert.Equal(t, "block", p.PackageRules[1].Action) assert.Equal(t, "malware", p.PackageRules[1].Match.FindingType) + // Every other finding is allowed. Without this rule, the local + // provider's "license unknown" finding blocked every install. + assert.Equal(t, "allow", p.PackageRules[2].Action) + assert.Equal(t, PackageMatch{}, p.PackageRules[2].Match) // --- Resource limits --- assert.Equal(t, 8192, p.ResourceLimits.MaxMemoryMB) diff --git a/internal/server/policysock_darwin.go b/internal/server/policysock_darwin.go index 3de4f527..ca81d433 100644 --- a/internal/server/policysock_darwin.go +++ b/internal/server/policysock_darwin.go @@ -12,6 +12,7 @@ import ( "github.com/diffsec/agentmon/internal/config" "github.com/diffsec/agentmon/internal/events" "github.com/diffsec/agentmon/internal/netmonitor" + "github.com/diffsec/agentmon/internal/pkgcheck" "github.com/diffsec/agentmon/internal/platform/darwin" "github.com/diffsec/agentmon/internal/platform/darwin/policysock" "github.com/diffsec/agentmon/internal/policy" @@ -26,7 +27,7 @@ const approvalGrantTTL = 60 * time.Second // startPolicySocket creates and starts the policy socket server for macOS // system extension IPC. It sets the policySockCancel and policySockDone // fields on the Server so the socket is shut down when the server stops. -func (s *Server) startPolicySocket(cfg *config.Config, engine *policy.Engine, approvalsMgr *approvals.Manager) { +func (s *Server) startPolicySocket(cfg *config.Config, engine *policy.Engine, approvalsMgr *approvals.Manager, pkgChecker *pkgcheck.Checker) { sockPath := cfg.PolicySocket.Path if sockPath == "" { return @@ -85,6 +86,11 @@ func (s *Server) startPolicySocket(cfg *config.Config, engine *policy.Engine, ap adapter.SetApprovals(approvalsMgr) } psrv.SetApprovalAsker(adapter) + // A nil *pkgcheck.Checker in the interface would not be nil, so the + // adapter gets it only when package checks are on. + if pkgChecker != nil { + adapter.SetPackageChecker(pkgChecker) + } psrv.SetFlowHandler(flowHandler{ sessions: s.sessions, engine: engine, diff --git a/internal/server/policysock_other.go b/internal/server/policysock_other.go index ca2cdf78..3348f40d 100644 --- a/internal/server/policysock_other.go +++ b/internal/server/policysock_other.go @@ -5,9 +5,11 @@ package server import ( "github.com/diffsec/agentmon/internal/approvals" "github.com/diffsec/agentmon/internal/config" + "github.com/diffsec/agentmon/internal/pkgcheck" "github.com/diffsec/agentmon/internal/policy" ) // startPolicySocket is a no-op on non-darwin platforms. // The policy socket server is only available on macOS for system extension IPC. -func (s *Server) startPolicySocket(_ *config.Config, _ *policy.Engine, _ *approvals.Manager) {} +func (s *Server) startPolicySocket(_ *config.Config, _ *policy.Engine, _ *approvals.Manager, _ *pkgcheck.Checker) { +} diff --git a/internal/server/server.go b/internal/server/server.go index 0b1b8cf6..e0b5748e 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -650,6 +650,10 @@ func New(cfg *config.Config) (*Server, error) { // the proxy after compiling the session-local DB unavoidability bundle. // Initialize package checker (optional). + // pkgChecker stays nil when package checks are off. The policy socket + // uses the same checker, so the exec API and the macOS extension share + // one verdict for one install. + var pkgChecker *pkgcheck.Checker if cfg.PackageChecks.Enabled { // Resolve and apply fail mode so Snyk/Socket OnFailure reflects // the operator's policy before provider entries are constructed. @@ -699,18 +703,18 @@ func New(cfg *config.Config) (*Server, error) { return nil, fmt.Errorf("package_checks.enabled=true but no providers were initialized — at least one provider must be configured and have its API key (if required) set") } - // Compose rules: engine.PackageRules() takes precedence, then block_on - // shorthand fills in the remaining finding types. We use the no-catch-all - // variant here so that an operator's policy file can deliberately omit - // a catch-all rule and rely on the evaluator's default-deny. - rules := append([]policy.PackageRule(nil), engine.PackageRules()...) - rules = append(rules, config.CompileBlockOnRules(cfg.PackageChecks.BlockOn)...) - - pkgChecker := pkgcheck.NewChecker(pkgcheck.CheckerConfig{ + // Rules: the default policy's package rules, then the block_on + // shorthand (pkgcheck.ComposeRules). block_on uses the no-catch-all + // variant, so that an operator's policy file can deliberately omit a + // catch-all rule and rely on the evaluator's default-deny. A check + // for a session uses that session's policy rules instead + // (Checker.CheckForPolicy). + pkgChecker = pkgcheck.NewChecker(pkgcheck.CheckerConfig{ Scope: cfg.PackageChecks.Scope, Resolvers: resolvers, Providers: providerEntries, - Rules: rules, + Rules: engine.PackageRules(), + BlockOn: config.CompileBlockOnRules(cfg.PackageChecks.BlockOn), Allowlist: pkgcheck.NewAllowlist(30 * time.Second), Privacy: pkgcheck.PrivacyConfig{ ExternalScanRegistries: cfg.PackageChecks.Privacy.ExternalScanRegistries, @@ -867,7 +871,7 @@ func New(cfg *config.Config) (*Server, error) { } // Start the policy socket server (macOS only; no-op on other platforms). - srv.startPolicySocket(cfg, engine, approvalsMgr) + srv.startPolicySocket(cfg, engine, approvalsMgr, pkgChecker) // Wire cmdResolver and sessionTracker into the app (darwin only). if srv.cmdResolver != nil { app.SetCmdResolver(srv.cmdResolver) diff --git a/macos/AgentMon/ESFClient.swift b/macos/AgentMon/ESFClient.swift index a6953ab1..7bfb8b36 100644 --- a/macos/AgentMon/ESFClient.swift +++ b/macos/AgentMon/ESFClient.swift @@ -976,13 +976,50 @@ private func sendExecDeniedEvent(path: String, args: [String], pid: pid_t, paren ) } -/// How long to wait for the daemon's exec verdict before failing closed. +/// The shortest wait for the daemon's exec verdict before failing closed. /// -/// Comfortably inside the ES deadline for AUTH_EXEC, and enormous next to the -/// observed round trip, which is a single unix-socket request on the same -/// machine. +/// Enormous next to the observed round trip, which is a single unix-socket +/// request on the same machine. A package install needs longer, because the +/// daemon resolves it and asks the package providers. See execWaitTime. private let execDecisionTimeout: TimeInterval = 2.0 +/// The longest wait for the daemon's exec verdict. +private let execDecisionMaxWait: TimeInterval = 30.0 + +/// Time kept free before the ES deadline. The extension must answer before +/// it, or ES ends the extension's client. +private let execDeadlineMargin: TimeInterval = 1.0 + +/// Words that start a package install in the package managers that the +/// daemon checks: npm, pnpm, yarn, pip, uv and poetry. +private let packageInstallWords: Set = ["install", "i", "add", "ci", "sync"] + +/// Whether an exec can be a package install, from its arguments. A false +/// answer for a real install is safe: the daemon then has 1.5 s, and an +/// unfinished check denies the exec. +private func execMayBePackageInstall(_ args: [String]) -> Bool { + args.dropFirst().contains { packageInstallWords.contains($0) } +} + +/// How long to wait for the daemon's exec verdict: until execDeadlineMargin +/// before the message's ES deadline, but no longer than execDecisionMaxWait. +/// When the deadline leaves less than execDecisionTimeout, the wait ends +/// 0.2 s before the deadline. +private func execWaitTime(_ message: UnsafePointer) -> TimeInterval { + var timebase = mach_timebase_info_data_t() + mach_timebase_info(&timebase) + let now = mach_absolute_time() + let deadline = message.pointee.deadline + guard deadline > now, timebase.denom != 0 else { return execDecisionTimeout } + let ticks = Double(deadline - now) + let remaining = ticks * Double(timebase.numer) / Double(timebase.denom) / 1e9 + let wait = min(remaining - execDeadlineMargin, execDecisionMaxWait) + if wait >= execDecisionTimeout { + return wait + } + return max(min(execDecisionTimeout, remaining - 0.2), 0.1) +} + /// Queue for the fail-closed watchdog below. Concurrent: each exec's watchdog /// is independent and must not be delayed by another's. private let execWatchdogQueue = DispatchQueue( @@ -1074,6 +1111,19 @@ private func handleAuthExec(client: OpaquePointer, event: UnsafePointer= 2, let script = execPtr.pointee.script { + scriptPath = String(cString: script.pointee.path.data) + } + // An exec that can be a package install may need seconds: the daemon + // resolves the install and asks the package providers. Only such an + // exec waits longer, on a connection of its own. Every other exec keeps + // the 2 s limit. If this test misses an install, the daemon has 1.5 s + // for the check, and an unfinished check denies the exec. + let mayInstall = execMayBePackageInstall(args) + let waitTime = mayInstall ? execWaitTime(event) : execDecisionTimeout // Ask the daemon rather than deciding from the local cache. // @@ -1097,14 +1147,14 @@ private func handleAuthExec(client: OpaquePointer, event: UnsafePointer Void = { response in switch response?["action"] as? String { case "continue": responder.respond(ES_AUTH_RESULT_ALLOW) @@ -1148,6 +1201,11 @@ private func handleAuthExec(client: OpaquePointer, event: UnsafePointer Int32 { if requestFD >= 0 { return requestFD } + requestFD = try openConnection(timeout: timeout) + return requestFD + } + + /// Queue for requestOnce. Concurrent, so that one slow request does not + /// hold up another. + private let onceQueue = DispatchQueue(label: "dev.diffsec.agentmon.policysocket.once", + attributes: .concurrent) + + /// Send one request on a new connection of its own, and wait up to + /// `timeout` for the answer. + /// + /// request() sends every request on one kept connection, one at a time, + /// with a 5 s read timeout. A package install check can take longer than + /// that, and on the kept connection it would hold up every other exec + /// check. completion gets nil when the request fails. + func requestOnce(_ request: [String: Any], timeout: TimeInterval, + completion: @escaping ([String: Any]?) -> Void) { + onceQueue.async { + var fd: Int32 = -1 + defer { if fd >= 0 { close(fd) } } + do { + fd = try self.openConnection(timeout: timeout) + completion(try self.exchange(request, on: fd)) + } catch { + NSLog("PolicySocketClient: one-off request failed: \(error)") + completion(nil) + } + } + } + /// Open a connection to the daemon and check the daemon's signature. + private func openConnection(timeout: TimeInterval) throws -> Int32 { let fd = socket(AF_UNIX, SOCK_STREAM, 0) guard fd >= 0 else { throw SocketError.creation } @@ -510,7 +542,9 @@ class PolicySocketClient { } // Timeouts - var tv = timeval(tv_sec: Int(timeout), tv_usec: 0) + let seconds = Int(timeout) + var tv = timeval(tv_sec: seconds, + tv_usec: Int32((timeout - Double(seconds)) * 1_000_000)) setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, socklen_t(MemoryLayout.size)) setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &tv, socklen_t(MemoryLayout.size)) @@ -519,7 +553,6 @@ class PolicySocketClient { var nosigpipe: Int32 = 1 setsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, &nosigpipe, socklen_t(MemoryLayout.size)) - requestFD = fd return fd } diff --git a/scripts/test-macos-package-checks.sh b/scripts/test-macos-package-checks.sh new file mode 100755 index 00000000..5728249d --- /dev/null +++ b/scripts/test-macos-package-checks.sh @@ -0,0 +1,99 @@ +#!/usr/bin/env bash +# Hardware test: a package install inside a macOS session is checked, also +# when the agent starts it and not `agentmon exec`. +# +# Before build 1679, only the command that `agentmon exec` started was +# package-checked. An install that an agent ran under `agentmon wrap` was +# not. agent-default blocks a package with a critical vulnerability, and +# django 2.2 has CVE-2022-34265 (CVSS 9.8). +# +# Run it on a Mac where AgentMon.app is installed and its extension is +# enabled. It needs internet access to pypi.org and api.osv.dev. Run it +# before each release. The daemon configuration needs: +# +# package_checks: +# enabled: true +# resolvers: +# pip: +# dry_run_command: /path/to/a/venv/bin/pip +# +# launchd starts the daemon with no Homebrew directory in PATH, so the +# default resolver command "pip" is not found, and every check fails. +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; } + +START=$(date -u +%Y-%m-%dT%H:%M:%SZ) +WS=$(mktemp -d "${TMPDIR:-/tmp}/ampkg.XXXXXX") +WS=$(cd "$WS" && pwd -P) +status=0 +trap 'rm -rf "$WS"' EXIT + +ok() { echo "ok $*"; } +fail() { echo "FAIL $*"; status=1; } + +# A pip configuration on this Mac can name another index, which +# agent-default does not allow. --isolated ignores it, and every install +# names pypi.org. +INDEX=https://pypi.org/simple + +# The venv is made outside the session. Homebrew's python refuses a plain +# pip install (PEP 668), so every install goes into the venv. +"$PY" -m venv "$WS/.venv" + +# install.py runs one install and prints "installed" or "refused". A denied +# exec ends the child with SIGKILL. +cat > "$WS/install.py" <<'PY' +import subprocess, sys +r = subprocess.run(sys.argv[1:], capture_output=True) +print("installed" if r.returncode == 0 else "refused rc=%d" % r.returncode) +PY + +wrap() { + (cd "$WS" && "$A" wrap --policy agent-default --report=false -- "$@" 2>>"$WS/err") +} + +expect() { + local want=$1 got=$2 what=$3 + case "$got" in + "$want"*) ok "$what: $got" ;; + *) fail "$what: got '$got', want '$want'" ;; + esac +} + +# pip3 is a script. ES reports the python3.15 interpreter as the executable, +# and the daemon must find pip in argv[1]. +got=$(wrap "$PY" "$WS/install.py" "$WS/.venv/bin/pip" --isolated install --no-cache-dir --index-url "$INDEX" "django==2.2" | tail -n 1) || true +expect refused "$got" "pip install django==2.2 (critical CVE) under wrap" + +got=$(wrap "$PY" "$WS/install.py" "$WS/.venv/bin/python" -m pip --isolated install --no-cache-dir --index-url "$INDEX" "django==2.2" | tail -n 1) || true +expect refused "$got" "python -m pip install django==2.2 under wrap" + +got=$(wrap "$PY" "$WS/install.py" "$WS/.venv/bin/pip" --isolated install --no-cache-dir --index-url "$INDEX" "six==1.16.0" | tail -n 1) || true +expect installed "$got" "pip install six==1.16.0 (no findings) under wrap" + +# The events search runs on compact JSON, with no space after a colon. +# A failed check also denies, so the reason must name the vulnerability. +# The events of this run are the ones newer than the start of the script. +events=$("$A" events query --since "$START" --type execve --text-like '%django%' 2>/dev/null || true) +n=$(printf '%s\n' "$events" | grep -c '"rule": "package-check"' || true) +failed=$(printf '%s\n' "$events" | grep -c 'package check failed' || true) +if [ "$n" -ge 2 ] && [ "$failed" -eq 0 ]; then + ok "$n execve events from the package check, none from a failed check" +else + fail "$n execve events from the package check, $failed from a failed check; want 2 or more, and 0" + printf '%s\n' "$events" | grep '"message"' | sed 's/^/ /' +fi + +if [ $status -ne 0 ] && [ -s "$WS/err" ]; then + echo "wrap stderr:" + sed 's/^/ /' "$WS/err" +fi +exit $status