diff --git a/.agents/skills/security-advisory-lessons/SKILL.md b/.agents/skills/security-advisory-lessons/SKILL.md index 639b67bce..25d7456da 100644 --- a/.agents/skills/security-advisory-lessons/SKILL.md +++ b/.agents/skills/security-advisory-lessons/SKILL.md @@ -60,12 +60,14 @@ For the full pattern map, read [advisory-patterns.md](references/advisory-patter ### IAM and service accounts - Treat imported IAM payload fields as attacker-controlled: `parent`, `claims`, `accessKey`, `secretKey`, status, policy names, and groups. - For service account create/update/import, prove parent ownership or root/admin authority before writing credentials or claims; an action permission alone must not allow choosing root or another user as `target_user`. +- Treat IAM export packages as credential disclosure surfaces; never include plaintext user or service-account secret keys unless the caller is allowed to recover those secrets and the export format is intentionally sealed. - Do not let `deny_only` or "no explicit deny" become an allow decision that skips required allow checks. - Test cross-user list/update/import flows with wrong, correct, self, parent, and root identities. ### STS, OIDC, and federation flows - Every STS endpoint must have an explicit authentication story: SigV4 where required, OIDC token verification for web identity, and role/session policy validation before issuing credentials. - JWT session tokens must be signed and verified by a trusted issuer/key path, not by service-account-controlled material or a reused root secret. +- JWT verification must enforce required claims and expiration for every bearer token path; "allow missing exp" is never acceptable for user-presented credentials. - Public OIDC bootstrap and callback routes must treat `Host`, `X-Forwarded-Proto`, redirect targets, `state`, and callback parameters as untrusted; credential-bearing redirects require a configured, allowlisted origin. - OIDC discovery and validation URLs are SSRF sinks. Resolve and classify hostnames at connection time, reject rebinding to loopback/private/link-local ranges, and do not rely on literal string checks. @@ -137,7 +139,9 @@ Use these prompts while reviewing a diff: - Does a public/default/empty config change security behavior from fail-closed to fail-open? - Is any attacker-controlled value later used as a path, policy condition, credential identity, log field, URL, Origin, or response body? - Does this response contain stored replication, remote target, or service credentials that need redaction or stricter authorization? +- Does an IAM export/import path expose or trust plaintext credential secrets beyond the caller's intended authority? - Can this STS/OIDC path issue credentials without SigV4, trusted issuer validation, allowlisted redirects, or trusted-proxy host/scheme handling? +- Can a service-account or STS token omit `exp`, forge `sessionPolicy`, or use a principal-controlled key as signing authority? - Does this outbound validation path resolve attacker-supplied hostnames and reject private, loopback, link-local, and rebound addresses at the actual connection boundary? - Is an archive entry, object key, or policy resource normalized differently between authorization and storage? - Is the same operation implemented in multiple paths, such as `CopyObject` vs `UploadPartCopy`, and do all paths enforce the same security contract? diff --git a/.agents/skills/security-advisory-lessons/references/advisory-patterns.md b/.agents/skills/security-advisory-lessons/references/advisory-patterns.md index f99f6d701..afa586346 100644 --- a/.agents/skills/security-advisory-lessons/references/advisory-patterns.md +++ b/.agents/skills/security-advisory-lessons/references/advisory-patterns.md @@ -27,14 +27,15 @@ Update this file only when an advisory adds or changes a reusable lesson, affect ### IAM import, service accounts, and privilege boundaries - `GHSA-566f-q62r-wcr8`: `ImportIam` accepted attacker-controlled service account `parent`, `claims`, `accessKey`, and `secretKey`, enabling persistent backdoor accounts under root. Lesson: imported IAM payloads are untrusted data and must be validated against privilege boundaries. +- `GHSA-3495-h8r9-gfqg`: `ExportIAM` wrote regular-user and service-account secret keys into exported ZIP data. Lesson: IAM export is a credential-disclosure boundary; redact, seal, or strictly justify every exported secret before treating export permission as safe. - `GHSA-5354-r3w2-34m8`: `AddServiceAccount` checked `CreateServiceAccountAdminAction` but trusted caller-supplied `target_user`, allowing service accounts under the root parent. Lesson: service-account create paths must validate parent ownership or root/admin authority, not only the create action. - `GHSA-xgr5-qc6w-vcg9`: `deny_only=true` skipped allow checks and let restricted service accounts mint unrestricted children. Lesson: deny-only logic must never become implicit allow for privilege creation. - `GHSA-mm2q-qcmx-gw4w`: leaked service account access keys plus update-without-ownership formed an escalation chain. Lesson: service-account identifiers are security-sensitive because update APIs consume them. ### STS, OIDC, and federation flows -- `GHSA-5qfg-mf7r-jp3w`: `AssumeRoleWithWebIdentity` was reachable without the required request authentication and could issue temporary credentials from crafted web identity input. Lesson: every STS route needs explicit SigV4 or trusted identity-provider validation before role assumption. -- `GHSA-ccrv-v8v9-ch9q`: service-account-controlled material could self-sign JWT session tokens with forged policy claims. Lesson: session tokens must be signed by a trusted issuer/key path and validation must reject self-signed or principal-controlled tokens. +- `GHSA-5qfg-mf7r-jp3w` and `GHSA-3473-5353-xhwh`: `AssumeRoleWithWebIdentity` was reachable through unauthenticated `POST /` routing and could issue temporary credentials from crafted web identity input. Lesson: every STS route needs explicit SigV4 or trusted identity-provider validation before role assumption, and unauthenticated exemptions must be narrowed to the exact action with uniform failure responses. +- `GHSA-ccrv-v8v9-ch9q` and `GHSA-48rf-7j3q-3hfv`: service-account-controlled material could self-sign JWT session tokens with forged policy claims, and missing `exp` was accepted for service-account tokens. Lesson: session tokens must be signed by a trusted issuer/key path, enforce required claims and expiration, and reject self-signed or principal-controlled tokens. - `GHSA-9pjf-w3c2-m32r`, `GHSA-4x2q-cpx9-9h26`, and `GHSA-xvpm-p3f7-34c3`: public OIDC authorize/callback flows trusted request `Host` or forwarded scheme when building credential-bearing redirects. Lesson: OIDC redirects must use configured allowlisted origins and trusted-proxy handling; never derive the post-login credential destination from direct client headers. - `GHSA-m479-9x88-94w6`, `GHSA-frwq-mfqx-83p8`, `GHSA-q9q8-rf9r-fg9f`, and `GHSA-j5c2-hhf7-6gf5`: OIDC validation accepted attacker-controlled discovery URLs because hostname checks rejected only literal forbidden IPs, allowing DNS rebinding SSRF. Lesson: outbound federation URL validation must resolve and classify hostnames at the connection boundary and reject loopback, private, link-local, and rebound addresses. @@ -58,7 +59,7 @@ Update this file only when an advisory adds or changes a reusable lesson, affect ### Secrets, defaults, and cryptographic misuse -- `GHSA-j59h-h7q5-q348`, `GHSA-3wm5-wpm5-hmfm`, `GHSA-6wc8-xm48-qhmx`, and `GHSA-9gf3-jx4p-4xxf`: RustFS shipped known default root credentials that could authenticate to S3, admin APIs, IAM, KMS, console, and token-signing surfaces. Lesson: root credentials must be operator-provided or generated per install; known defaults and warnings are not acceptable for network-reachable deployments. +- `GHSA-j59h-h7q5-q348`, `GHSA-3wm5-wpm5-hmfm`, `GHSA-6wc8-xm48-qhmx`, `GHSA-9gf3-jx4p-4xxf`, and `GHSA-63xc-c3w3-m2cf`: RustFS shipped known default root credentials that could authenticate to S3, admin APIs, IAM, KMS, console, and token-signing surfaces. Lesson: root credentials must be operator-provided or generated per install; known defaults and warnings are not acceptable for network-reachable deployments. - `GHSA-h956-rh7x-ppgj`: gRPC used the hard-coded token `rustfs rpc` on both client and server. Lesson: source-visible shared tokens are authentication bypasses. - `GHSA-r5qv-rc46-hv8q`: internode RPC HMAC secret fell back to the public default `rustfsadmin`. Lesson: RPC/internode auth must fail closed instead of silently using public defaults. - `GHSA-75fx-qg6f-8rm7` and `GHSA-68cw-96m3-h2cf`: internode RPC secrets were derivable from known root credentials, making raw storage RPC signatures forgeable when explicit RPC secrets were unset. Lesson: RPC auth keys must be independent random secrets, never derived from S3 root credentials, and raw storage RPC should not share the public S3 listener without an internode-only boundary. @@ -122,6 +123,7 @@ rg -n "deny_unknown_fields|serde.default|as u32|as usize|as i32" rustfs crates - Copy/upload fixes: include cross-bucket, cross-user, source-denied, destination-denied, copy-source-condition, and multipart completion cases. - Path fixes: include encoded traversal, absolute path, nested traversal, archive entries with `..`, valid object keys that resemble traversal text but should be rejected, and canonical bucket/prefix boundary checks. - Logging fixes: assert redacted output for structs and response bodies that may contain credentials. +- IAM export fixes: assert exported archives omit plaintext user and service-account secrets unless the format deliberately encrypts or seals them. - RPC auth fixes: include captured metadata replay across two concrete methods, stale timestamps, wrong path, wrong method surrogate, wrong secret, and valid same-method calls. - Browser/CORS fixes: assert no credentials on reflected/default origins, correct behavior for explicit allowlists, and no same-origin script execution for previewed object content. - SSE fixes: inspect stored bytes and verify API metadata, read-back behavior, and on-disk ciphertext together.