gsadmin/rustfs
1
0
Fork 0
mirror of https://github.com/rustfs/rustfs.git synced 2026-07-26 08:18:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

docs: update security advisory lessons (#3447)

Browse Source
This commit is contained in:
安正超
2026-06-14 21:51:54 +08:00
committed by GitHub
parent b49057c8e3
commit b2e6cc520b
2 changed files with 19 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.agents/skills/security-advisory-lessons/SKILL.md
+11 -2
View File
@@ -25,8 +25,8 @@ For the full pattern map, read [advisory-patterns.md](references/advisory-patter
## Workflow
### 1. Scope the change
- Identify touched routes, handlers, storage paths, credentials, logs, browser surfaces, CI/release code, and policy checks.
- Treat these paths as security-sensitive by default: `rustfs/src/admin/`, `rustfs/src/storage/`, `rustfs/src/auth.rs`, `rustfs/src/server/layer.rs`, `crates/iam/`, `crates/policy/`, `crates/credentials/`, `crates/ecstore/src/rpc/`, `crates/rio/`, and console preview/auth code.
- Identify touched routes, protocol frontends, handlers, storage paths, credentials, logs, browser surfaces, CI/release code, and policy checks.
- Treat these paths as security-sensitive by default: `rustfs/src/admin/`, `rustfs/src/storage/`, `rustfs/src/auth.rs`, `rustfs/src/server/layer.rs`, `crates/iam/`, `crates/policy/`, `crates/credentials/`, `crates/ecstore/src/rpc/`, `crates/protocols/`, `crates/rio/`, and console preview/auth code.
### 2. Map to advisory classes
- Read [advisory-patterns.md](references/advisory-patterns.md) for matching GHSA lessons.
@@ -54,6 +54,7 @@ For the full pattern map, read [advisory-patterns.md](references/advisory-patter
- Match the admin action to the operation exactly. Copy-paste action constants are a known RustFS vulnerability class.
- Avoid authentication-only helpers for state-changing admin APIs; use `validate_admin_request` or the established equivalent with the right `AdminAction`.
- Read-only admin APIs such as metrics, server info, and diagnostics still require admin authorization; checking only that credentials exist is not enough.
- Replication admin reads can expose remote target credentials; list/get target endpoints require replication/admin authorization and must not return secrets to low-privilege callers.
- Do not assume admin-action `Resource` scoping constrains blast radius unless the policy engine actually enforces resources for that action.
### IAM and service accounts
@@ -67,6 +68,12 @@ For the full pattern map, read [advisory-patterns.md](references/advisory-patter
- Do not let `CreateMultipartUpload`, `UploadPartCopy`, `CompleteMultipartUpload`, or `AbortMultipartUpload` return success without authorization.
- Presigned POST policies are server-side contracts. Enforce `content-length-range`, key prefix, exact metadata/content-type, and all signed policy conditions.
### Protocol frontends and IAM parity
- FTP/FTPS, SFTP, gateway, and other protocol drivers must enforce IAM per operation before calling storage backends; authentication to a protocol listener is not authorization.
- Match protocol commands to the same S3 actions as HTTP, such as `RETR` to `GetObject`, `SIZE`/`MDTM` to `HeadObject`, `MKD` to `CreateBucket`, and bucket probes to `ListBucket` or `HeadBucket`.
- Review every handler in a protocol driver, not only the changed handler, because RustFS advisories show mixed guarded and unguarded siblings in the same driver.
- Regression tests for protocol frontends should deny the shared authorization hook and prove the backend is not reached for the denied command.
### Paths, object keys, and filesystem access
- Never join untrusted bucket/object/RPC path strings onto filesystem roots without normalization and boundary checks.
- Reject or safely handle `..`, absolute paths, URL-encoded traversal, platform separators, empty components, and paths that canonicalize outside the intended root.
@@ -116,8 +123,10 @@ For the full pattern map, read [advisory-patterns.md](references/advisory-patter
Use these prompts while reviewing a diff:
- Could a low-privileged authenticated user reach this path with the wrong action, parent, bucket, or source object?
- Does a non-HTTP protocol path call the same authorization boundary as the S3 API before touching storage?
- 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?
- 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?
- Does the test prove the exploit form is denied, or only that the intended form still works?
.agents/skills/security-advisory-lessons/references/advisory-patterns.md
+8
View File
@@ -21,6 +21,7 @@ Update this file only when an advisory adds or changes a reusable lesson, affect
- `GHSA-vcwh-pff9-64cc`: `ImportIam` checked `ExportIAMAction` for an import/write operation. Lesson: every admin handler must authorize the action it actually performs.
- `GHSA-jqmc-mg33-v45g` and `GHSA-8784-9m7f-c6p6`: `/profile/cpu` and `/profile/memory` were whitelisted from auth and allowed expensive diagnostics plus path disclosure. Lesson: profiling/debug endpoints need admin auth, opt-in, rate limits, and non-sensitive responses.
- `GHSA-f5cv-v44x-2xgf`: `/rustfs/admin/v3/metrics` accepted any authenticated IAM user and skipped admin authorization. Lesson: read-only metrics and diagnostic admin endpoints still require an operation-specific admin action check.
- `GHSA-796f-j7xp-hwf4`: `/rustfs/admin/v3/list-remote-targets` checked only that credentials existed and leaked replication target credentials. Lesson: replication target reads are privileged admin operations, and stored remote credentials require strict authz plus response redaction review.
- `GHSA-xp32-gxq2-3v52`: console license metadata endpoint was public and exposed subject and expiration fields. Lesson: management metadata endpoints should require admin auth or return only coarse public status.
### IAM import, service accounts, and privilege boundaries
@@ -35,6 +36,11 @@ Update this file only when an advisory adds or changes a reusable lesson, affect
- `GHSA-wfxj-ph3v-7mjf`: `UploadPartCopy` checked source and destination independently but missed destination copy-source policy constraints. Lesson: source read and destination write checks are not sufficient when policy constrains allowed copy sources.
- `GHSA-w5fh-f8xh-5x3p`: presigned POST accepted uploads without enforcing signed policy conditions. Lesson: parse and enforce all POST policy constraints server-side, including size, key prefix, and content type.
### Protocol frontends and IAM parity
- `GHSA-3g29-xff2-92vp`: FTP `RETR` and `SIZE`/`MDTM` read paths authenticated the user but skipped IAM before calling storage. Lesson: non-HTTP protocol frontends must enforce the same per-operation authorization as the S3 API before backend access.
- `GHSA-g3vq-vv42-f647`: FTPS `MKD` called `create_bucket` without checking `s3:CreateBucket`. Lesson: protocol command handlers need action-specific checks even when sibling handlers already authorize correctly.
### Filesystem paths and object key traversal
- `GHSA-pq29-69jg-9mxc`: RPC `read_file_stream` joined untrusted paths under a volume directory without canonical boundary checks. Lesson: `PathBuf::join` plus length checks are not path security.
@@ -85,6 +91,7 @@ Use these targeted searches when a diff touches security-sensitive code:
```bash
rg -n "validate_admin_request|check_permissions|AdminAction::|deny_only|is_allowed" rustfs crates
rg -n "authorize_operation|FtpsDriver|SftpDriver|RETR|MKD|SIZE|MDTM|CreateBucket|GetObject|HeadObject" crates/protocols rustfs
rg -n "UploadPartCopy|upload_part_copy|CompleteMultipart|PostObject|content-length-range|starts-with" rustfs crates
rg -n "normalize_extract_entry_key|Snowball|auto-extract|PathBuf::join|canonicalize|\\.\\.|x-forwarded-for|x-real-ip|SourceIp" rustfs crates
rg -n "DEFAULT_SECRET|DEFAULT_ACCESS|TEST_PRIVATE_KEY|rustfs rpc|RUSTFS_RPC_SECRET" rustfs crates
@@ -97,6 +104,7 @@ rg -n "deny_unknown_fields|serde.default|as u32|as usize|as i32" rustfs crates
## Minimum Regression Test Expectations
- Authz fixes: include unauthenticated, valid low-privilege, wrong-action, correct-action, owner, non-owner, and root/admin cases as applicable.
- Protocol frontend authz fixes: include denied `RETR`, `SIZE`/`MDTM`, `MKD`, bucket probe, and sibling allowed-operation cases, and assert denied paths do not reach the storage backend.
- IAM fixes: include import/update/list service-account cases with attacker-controlled parent, claims, access key, secret key, and policy.
- 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.