* fix(admin): bound IAM import archive expansion
MAX_IAM_IMPORT_SIZE caps the compressed upload at 10 MB, but every member of the
archive was then read with read_to_end into an unbounded Vec. Deflate ratios well
above 100:1 are easy to construct, so a small authorized upload could expand
without limit across the seven members ImportIam reads.
Add a shared expansion budget (MAX_IAM_IMPORT_EXPANDED_SIZE, 10x the compressed
cap) drawn down by every member, and route all seven reads through one helper
that reads a byte past the remaining budget to detect overrun. Sharing the budget
bounds the archive as a whole rather than letting each member spend the full
limit independently.
Covers R03-CAN-024 through R03-CAN-030 plus R04-CAN-077 (backlog #1471) — one
fix rather than seven, since all seven call sites were byte-identical.
* fix(kms): confine local key paths and refuse silent key replacement
Local KMS key identifiers arrive from request input — the `name` tag on CreateKey,
the `keyId` body field or query parameter on DeleteKey — and were joined onto
`key_dir` with no validation. An identifier such as `../../tmp/evil` escaped the
configured directory, making key creation a constrained arbitrary-file write and
`DeleteKey` with `force_immediate` a cross-directory delete.
Validate in `master_key_path` and make it fallible, so every filesystem path in
this backend inherits the guard: decode_stored_key, load_master_key,
save_master_key, create_key and delete_key all derive their paths there. The rule
is containment rather than a character allowlist, so identifiers already in use
keep resolving; only separators, NUL, absolute paths and non-single-component
forms are refused. Note `.` and `..` are contained rather than refused — the
`.key` suffix turns them into the ordinary filenames `..key` and `...key`.
Separately, `LocalKmsBackend::create_key` had no existence check, while the
sibling `KmsClient::create_key` has always had one. Since `save_master_key`
renames over its destination, creating a key under an existing name silently
replaced its material and destroyed the ability to decrypt everything wrapped
under it — and the backend path is the one the admin API uses. It now returns
KeyAlreadyExists, matching StaticKmsBackend.
Covers R03-CAN-072, R03-CAN-073 and R07-CAN-103 (backlog #1475). R03-CAN-073
needed no separate change: delete_key routes both its load and its remove_file
through master_key_path.
* fix(swift): bound SLO manifest reads to the 2 MiB manifest limit
The three Swift SLO handlers that load a stored manifest (handle_slo_get,
handle_slo_get_manifest, handle_slo_delete) read the `<object>.slo-manifest`
object to EOF with AsyncReadExt::read_to_end. That key is predictable and
writable through the ordinary object PUT path, so a tenant can replace the
manifest with an arbitrarily large object and then make the server allocate
its full size on every SLO GET, multipart-manifest=get, or
multipart-manifest=delete request - a memory amplification bounded only by
the stored object size (CWE-400 / CWE-770). The 2 MiB manifest limit that
handle_slo_put enforces was not applied on the read side.
Introduce MAX_SLO_MANIFEST_SIZE (the existing 2 MiB PUT limit, now a named
constant) and a shared read_manifest_bytes helper that reads through a
`take(limit + 1)` and rejects anything larger, so an oversized manifest is
refused instead of being buffered first. All three call sites go through the
helper. handle_slo_put now checks the size before parsing the JSON.
Regression tests: test_read_manifest_bytes_rejects_oversized_manifest and
test_read_manifest_bytes_stops_reading_oversized_manifest (which asserts the
reader is not consumed past the limit), plus a boundary test that a manifest
at exactly 2 MiB is still accepted.
* fix(protocols): authorize every object in FTPS/WebDAV recursive deletes
The FTPS and WebDAV gateways authorized only the container before a
recursive delete and then destroyed everything inside it without a
further check:
- FTPS RMD (and DELE on a bucket path ending in '/') cleared
s3:DeleteBucket, then delete_bucket_recursively listed the bucket and
deleted every object.
- WebDAV DELETE on a bucket did the same via its own
delete_bucket_recursively.
- WebDAV DELETE on a directory cleared s3:DeleteObject for the directory
marker key ("dir/") only, then listed that prefix and deleted every
child under it.
A principal holding s3:DeleteBucket (or s3:DeleteObject on a single
marker key) could therefore erase objects it had no s3:DeleteObject
permission for, and the operation reported success.
Deletion stays recursive - that is the expected behaviour for these
protocols - but each object now clears s3:DeleteObject on its own key
before it is removed, and the enumeration clears s3:ListBucket. A denial
aborts the whole operation with access denied rather than being skipped,
so the caller can never be told the delete succeeded while objects were
left behind or removed without authorization.
The test double gained shared-state cloning, delete_object/delete_bucket
call logs, and list/delete queue helpers so the regression tests can
observe that nothing is deleted once a deny lands.
* fix(server,ecstore): bound TLS handshakes and remote volume RPC waits
Three call sites let an unauthenticated client or a misbehaving peer hold
server resources with no deadline.
TLS listener (R03-CAN-035): process_connection awaited
`acceptor.accept(socket)` with no bound. A client that opens a TCP
connection and never finishes the handshake parks a Tokio task and a socket
forever, and the connection cap (RUSTFS_API_MAX_CONNECTIONS) is unlimited by
default, so nothing else sheds it. The handshake now runs under
accept_tls_with_deadline(), reusing the existing HTTP/1 header-read budget —
the established slow-client bound for the pre-request phase — and the
expiry is recorded through the same log/metric path as a handshake error,
under a new TIMEOUT failure kind.
Remote disk RPCs (R03-CAN-049, R03-CAN-050): list_volumes and delete_volume
passed Duration::ZERO, which execute_with_timeout treats as "no deadline",
so a peer that accepts the request and never answers stalls the coordinator
(and, for delete_volume, the bucket-deletion workflow). Both now pass
get_max_timeout_duration(), matching every sibling method in the file.
Regression tests: a silent TLS peer must be shed by the handshake deadline;
list_volumes/delete_volume against a peer that completes the TCP connect and
then goes silent must fail with DiskError::Timeout instead of hanging.
* fix(security): stop leaking signed headers and bound OIDC/KMS credentials
Three independent hygiene fixes found by the security review.
R03-CAN-018 (crates/signer): try_get_canonical_headers and get_signed_headers
logged the complete header map at DEBUG before signing. Runtime callers pass
session credentials and SSE-C key material through these headers, so anyone
able to raise the log level (or read DEBUG logs) recovered
X-Amz-Security-Token and SSE-C keys verbatim. The statements were debugging
leftovers with no operational value and are deleted rather than redacted.
R03-CAN-014 (crates/iam): the OIDC HTTP adapter buffered provider responses
with an unbounded Response::bytes(), so a configured, compromised or
attacker-pointed IdP endpoint could stream an arbitrarily large or endless
body into memory (the ValidateOidcConfig admin handler lets a ServerInfo
caller choose the endpoint). Responses are now read incrementally and fail
closed past MAX_OIDC_RESPONSE_SIZE, and the already SSRF-hardened client
builder gains request and connect timeouts so a stalled provider cannot pin
the calling task indefinitely.
R07-CAN-105 (helm): the Vault KMS token was serialized into the chart
ConfigMap, exposing it to every subject allowed to get ConfigMaps in the
namespace. It now renders into a dedicated Secret that the Deployment and
StatefulSet consume via envFrom; the Secret is separate from the main
credentials Secret so it also works when secret.existingSecret is set.
Regression tests:
- rustfs-signer: signing_never_logs_signed_header_material
- rustfs-iam: oidc_response_body_past_the_limit_is_rejected,
oidc_response_body_at_the_limit_is_accepted
- scripts/test_helm_templates.sh: KMS token must never render in plaintext
* fix(webdav): enforce body limit, request timeout and connection cap
The configured WebDAV maximum body size was enforced from Content-Length, so a
chunked request declared no length and bypassed it entirely. The configured
request timeout was never applied to the connection at all, and the accept loop
spawned a task per connection with no bound, so an unauthenticated client could
hold resources indefinitely and in unbounded number.
Enforce the limit on bytes actually read rather than the declared length, apply
the configured timeout to the request, and bound accepted connections with a new
RUSTFS_WEBDAV_MAX_CONNECTIONS (default 1024) surfaced in the config report.
Covers R03-CAN-051, R03-CAN-052, R03-CAN-067, R04-CAN-089, R05-CAN-094 and
R05-CAN-097 (backlog #1471, #1474).
* fix(security): stop STS credentials from crossing the parent trust boundary
Two related credential-boundary holes let a short-lived STS credential act
with the full, unrestricted authority of the long-term user it was minted
from.
AddUser (R03-CAN-021, CWE-269/863): should_check_deny_only relaxes the admin
policy check to deny-only when a Console/STS session targets the IAM user it
represents. Nothing then stopped that session from calling AddUser with its
own parent's access key, so the handler wrote an attacker-chosen secret key
and status over the parent's stored Credentials via create_user ->
save_user_identity. A session that expires in minutes became permanent
control of the account. AddUser now rejects any temp or service-account
requester whose resolved parent equals the target access key, resolving the
parent the same way should_check_deny_only does (parent_user field, else the
JWT `parent` claim, since some stores persist the parent only in the token).
FTPS/SFTP/WebDAV password auth (R04-CAN-086, CWE-287/862): these protocols
looked the access key up with check_key, which falls back to the STS account
cache, and then compared only the stored secret. An STS access key plus
secret therefore authenticated with no session token presented and no
session-policy claims applied - the holder got the parent's full permissions.
Password authentication now rejects temporary credentials before the secret
comparison. The discriminator is is_temp() && !is_service_account(), the same
one IamCache::update_user_with_claims uses to route an identity into the STS
cache, so service accounts - which resolve policy from stored IAM state
rather than a client-presented token - keep working over these protocols.
Regression tests cover both predicates and pin the guards to their call
sites so neither can be dropped without a test failure.
Stop the OIDC subsystem from writing credential-grade secrets into logs and returned errors.
- crates/iam/src/oidc.rs: redact sensitive header values (authorization, proxy-authorization, cookie, set-cookie) in format_http_headers, emitting only name and length; drop the raw request/response body from the DEBUG events (keep byte length); stop logging and stop splicing the raw token response body into the error returned on token_response_parse_failed (the TokenResponseBodyShape summary and length are retained); remove the now-unused format_http_body helper.
- rustfs/src/admin/handlers/oidc.rs: stop logging the raw authorization code and state on the code-exchange error path (code_len/state_len are kept).
This is the OIDC log/error redaction pre-work (batch 0B) from the OIDC review in rustfs/backlog#1437. It changes diagnostic content only; HTTP/STS status codes and legitimate request results are unchanged.
Restore the workspace reqwest default feature stack for RustFS outbound HTTPS clients, while keeping per-crate extra APIs such as json, stream, and multipart explicit. Lazily initialize the notification runtime from admin target access when RUSTFS_NOTIFY_ENABLE=true is already effective, and add regression coverage for HTTPS webhook custom CA handling and target-list visibility.
Fixes#5052.
Co-authored-by: heihutu <heihutu@gmail.com>
* chore(deps): remove redundant dependency features
Remove manifest feature entries that are implied by other requested features in the same dependency declaration.
Verified that the resolved Cargo feature graph is unchanged after the cleanup.
Co-Authored-By: heihutu <heihutu@gmail.com>
* chore(deps): narrow tokio and reqwest features
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
Move workspace-level dependency feature lists into the member crates that consume each dependency while keeping required default-features flags at the workspace root.
Also refresh starshard to 2.2.2 via cargo update and cargo upgrade --exclude ratelimit.
Co-authored-by: heihutu <heihutu@gmail.com>
* test(utils): add rustfs-test-utils crate, absorb heal/iam ECStore bootstrap
backlog#1153 infra-1. The ~50-line "build a real temp-disk ECStore"
bootstrap was copy-pasted (and drifting) across the heal and iam
integration tests. This adds crates/test-utils (rustfs-test-utils, a
dev-dependency-only crate) owning that bootstrap and converts the four
copies into thin wrappers:
- TestECStoreEnvBuilder: disk_count (default 4), prefix (uuid-suffixed
/tmp dir), base_dir (caller-owned dir, e.g. tempfile::TempDir),
init_bucket_metadata (default true; the iam bootstrap test opts out
to preserve its historical semantics). TestECStoreEnv exposes
temp_root/disk_paths/ecstore plus a versioned-bucket helper, and
init_tracing() replaces the per-file Once blocks.
- All rustfs_ecstore imports stay behind src/ecstore_test_compat.rs,
the sanctioned test-compat boundary pattern (mirrors
crates/iam/tests/ecstore_test_compat).
- heal: heal_integration_test / heal_b5_versioned_regression_test /
heal_b920_subquorum_union_test drop their setup_test_env{,_n} copies
for heal_env{,_n} wrappers; the tests/storage_api.rs integration
surface shrinks to what test bodies still touch.
- iam: iam_bootstrap_no_lock_test drops build_local_ecstore; its
ecstore_test_compat fixture shrinks to SetupType +
update_erasure_type.
rg 'async fn setup_test_env' crates/heal crates/iam now returns 0.
Scanner's lifecycle tests are deliberately NOT absorbed (gated on
ilm-1; 14 of 15 are #[ignore]d today). Net -230 lines.
* fix(heal): drop tokio::fs import orphaned by the b920 bootstrap move
* fix(heal): drop tokio::fs import orphaned by the b5 bootstrap move
test(security): pin GHSA-m77q STS root-secret token signing (sec-7)
GHSA-m77q-r63m-pj89 (intentionally UNFIXED) is that STS session tokens are
signed with the shared root secret: crates/iam/src/root_credentials.rs
token_signing_key() returns the root secret_key, so anyone holding the root
secret can forge STS session tokens. No test named the advisory, and the
existing test_created_sts_credentials_authorize_with_session_token_claims uses
token_signing_key() for both signing and verifying, so it pins "same key signs
and verifies" but not the m77q-specific "signing key IS the root secret" — a
future fix that decouples the STS key from the root secret would pass it
silently.
Add a flow-level pin, test_ghsa_m77q_sts_session_token_signed_with_root_secret,
that captures the advisory's exact signature: (1) token_signing_key() == the
root secret; (2) an AssumeRole-style session token issued with that key decodes
with the root secret and NOT with any other secret; (3) it authorizes through
the STS path. All three assert CURRENT (by-design-vulnerable) behavior, so a
real m77q fix (a dedicated STS signing key) turns them red and forces a
red -> green regression update.
Also GHSA-name the existing characterization test and token_signing_key() with
doc comments and the advisory URL, and update the m77q row in
docs/testing/security-regressions.md. No production behavior changes.
Refs: backlog#1151 (sec-7)
Co-authored-by: houseme <housemecn@gmail.com>
* Change Rust toolchain channel to stable
Signed-off-by: houseme <housemecn@gmail.com>
* style: apply clippy --fix and cargo fix lint suggestions
Run `cargo clippy --fix --all-targets --all-features` and
`cargo fix --lib --all-targets` across the workspace, then resolve the
remaining warnings by hand:
- collapse needless borrows in `format!` args, prefer `?` over explicit
early returns, and use `.values()` / `.flatten()` iterator adapters
- rewrite the `Md5` scan loop via `manual_flatten` and re-indent the
`select!` macro body (rustfmt skips macro interiors)
- annotate the intentional dead-code `Md5` inherent methods (constructed
only by the test factory) with `#[allow(dead_code)]`
Behavior is unchanged.
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Signed-off-by: houseme <housemecn@gmail.com>
Co-authored-by: heihutu <heihutu@gmail.com>
backlog#1052 S3, fourth slice. The AppContext's IamHandle already owned an
Arc<IamSys>, but the value it held was read back from the process
singleton after init — so a future second server's context would silently
bind to the first server's IAM domain (credentials, policies, users all
belong to a specific store's .rustfs.sys).
- rustfs_iam gains build_iam_sys(store): construct an IAM system bound to
a store without touching the singleton. init_iam_sys wraps it (build,
publish first-wins, return the handle — previously Result<()>).
- Startup threads the built handle: the inline bootstrap path passes the
Arc it just created into ensure_startup_after_iam, which constructs the
AppContext from the passed value instead of re-resolving the global.
The deferred-recovery path resolves the freshly published global
directly (context-first resolution cannot be used there: the AppContext
does not exist yet — creating it is the finalizer's job; caught by the
embedded deferred-IAM e2e).
- IamHandle::is_ready() reports the held system's readiness instead of
consulting the process singleton; the dead global re-resolver is
removed. token_signing_key stays on the credentials global until S4.
157 iam tests + embedded e2e (basic + deferred recovery) green.
* refactor(app): give each context's server-config handle its own copy
backlog#1052 S3, first slice: establish the per-context state pattern on
the smallest subsystem. ServerConfigHandle was a unit struct forwarding
both get and set to the process-global GLOBAL_SERVER_CONFIG, so every
AppContext shared one config regardless of which context a caller held.
The handle now keeps its own copy: a set lands on the owning context and
on the process global (ambient readers — the config loader path, the
scanner — keep working), and a get prefers the owned copy, falling back
to the global while the initial load still publishes there. Single
instance: the owned copy and the global are written together, so reads
are unchanged. Two contexts that both published stay isolated even though
the shared global only remembers the last write (covered by a new test).
The global write in set() is the transitional bridge for ambient readers;
it goes away when those readers migrate and multi-instance flips on
(backlog#1052 S5).
* refactor(notify): hand out the notification system as an owned Arc
backlog#1052 S3, second slice. GLOBAL_NOTIFICATION_SYS stored the
NotificationSys by value and every accessor returned
Option<&'static NotificationSys> — a process-lifetime borrow that a
per-server AppContext can never hold. The global now stores an Arc and
the accessor chain (ecstore runtime sources, ECStore::notification_system,
the iam forwarders, the rustfs shims, NotificationSystemInterface and the
AppContext resolvers) hands out owned Arcs instead.
Call sites are unchanged apart from two borrow adjustments in the
rebalance admin handler (pass &Arc where a reference is expected; borrow
with as_ref() so the handle survives to the second use). Behavior is
identical — same single global instance, first init still wins — but the
type now permits a future per-server context to own its notification
system (backlog#1052 S3 follow-up).
load_all fetched policies/users/user-policies by passing the FULL remaining
list to load_*_concurrent every iteration and then split_off(32), so each item
was re-fetched once per preceding chunk — O(n^2) redundant loads on startup for
>32 items. Replace the split_off loops with chunks(32) so each item is fetched
exactly once. Behavior-preserving (cache inserts were already idempotent).
* fix(scanner): scope long walk timeouts
* fix(scanner): bound IAM config walks
---------
Co-authored-by: Henry Guo <marshawcoco@users.noreply.github.com>
* fix(iam): load IAM bootstrap snapshot without namespace locks
IAM bootstrap (init_iam_sys -> load_all) read every config object with
default ObjectOptions (no_lock=false), so each read acquired a
distributed namespace read lock. Lock quorum is counted over cluster
nodes and unreachable peers are hard failures, so during a sequential
restart the very first read failed with
"Quorum not reached: required 2, achieved 0" and IAM could not come up
until enough peers' lock RPC surfaces converged - even when the storage
read quorum was already satisfiable (rustfs#4304).
Extend the startup contract from rustfs#4056 ("startup metadata I/O must
not require namespace locks") to the IAM bootstrap path:
- Introduce LoadMode {Locked, BootstrapNoLock} and plumb it through the
load_all chain (groups, users, policies, mapped policies and their
concurrent variants) down to the storage read options.
- load_all now performs all reads with no_lock=true; on-line
single-object loads via the Store trait keep locked semantics.
- Fail-closed behavior is unchanged: any loader error still aborts the
whole snapshot load.
Safety: config objects are atomic whole-object writes, so a lock-free
read only observes an old or a new value; staleness is bounded by the
existing periodic IAM reload. Listing (walk) never took namespace locks,
and maybe_schedule_lazy_rewrite stays a best-effort background task.
Verification:
- cargo test -p rustfs-iam --lib (153 passed, incl. new LoadMode tests)
- cargo clippy -p rustfs-iam --all-targets
- cargo check -p rustfs
- make pre-commit
Ref: rustfs#4304; tracking rustfs/backlog#884, rustfs/backlog#885
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(iam): sequential-restart regression test for lock-free bootstrap
Add an integration test reproducing the rustfs#4304 failure mode against
a real 4-disk temp-dir ECStore:
- Seed IAM group data in single-node mode, then flip the runtime into
distributed-erasure mode. new_ns_lock now builds a distributed lock
over the set's (empty) lock-client list, so every namespace-locked
read fails exactly like a sequential restart with unreachable peers
(lock quorum unavailable, storage read quorum healthy).
- Assert the locked load_group path fails in that state, while the
bulk snapshot load_all (no_lock plumbing from the previous commit)
succeeds, and the data survives intact once single-node mode is
restored.
Reverse-verified: temporarily switching load_all back to the locked
mode makes the test fail, so it genuinely guards the contract.
Verification:
- cargo test -p rustfs-iam --test iam_bootstrap_no_lock_test
- cargo test -p rustfs-iam --lib
Ref: rustfs#4304; tracking rustfs/backlog#886
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(iam): route test ECStore imports through ecstore_test_compat boundary
The new integration test imported rustfs_ecstore facade paths directly,
tripping three architecture migration rules. Move every ECStore import
behind crates/iam/tests/ecstore_test_compat/mod.rs (the sanctioned
test-compat pattern), and register that module as a reviewed test-only
global-facade boundary in check_architecture_migration_rules.sh: the
sequential-restart regression test needs api::global::update_erasure_type
to flip into distributed-erasure mode for lock-quorum fault injection.
Verification:
- ./scripts/check_architecture_migration_rules.sh
- cargo test -p rustfs-iam --test iam_bootstrap_no_lock_test
- make pre-commit
Co-Authored-By: heihutu <heihutu@gmail.com>
* feat(server): expose readiness blocking reason + rolling-restart runbook
Operators hitting the rustfs#4304 sequential cold start could not tell
from the outside why a node stayed unavailable. Three additions:
- The readiness gate's 503 now names the blocking dependency in both the
body ("Service not ready: waiting for storage_quorum") and a new
x-rustfs-readiness-pending header (storage_quorum | iam |
startup_finalization), derived from the current startup stage.
/health/ready already returned details + degradedReasons; this covers
the plain S3 requests that hit the gate.
- IAM bootstrap retry logs now carry an actionable `hint` field that
classifies the failure (storage read quorum vs lock quorum vs
uninitialized metadata) instead of only echoing the storage error.
- New docs/operations/rolling-restart.md runbook: correct rolling
restart procedure, sequential cold-start expectations (degraded ->
auto-recovery), readiness signal reference, and
RUSTFS_STARTUP_READINESS_MAX_WAIT_SECS guidance.
Verification:
- cargo test -p rustfs --lib -- hint_tests service_not_ready readiness_pending
- make pre-commit
Ref: rustfs#4304; tracking rustfs/backlog#887
Co-Authored-By: heihutu <heihutu@gmail.com>
* upgrade deps version and improve import
* feat(iam): notification-path cache refreshes read without namespace locks (#4368)
P3 step 1 of rustfs/backlog#884 (scoped down from full MinIO readConfig
alignment after review): cross-node notification handlers
(group/policy/policy-mapping/user) refresh the local IAM cache with
single-object reads that previously took distributed namespace read
locks. These refreshes are asynchronous, best-effort, and already
stale-tolerant (the periodic reload converges them), so a node-counted
lock quorum failure or lock RPC hiccup on a peer must not fail them —
the same rationale as the lock-free bootstrap load_all (rustfs#4304).
- Store trait: add load_user_no_lock / load_group_no_lock /
load_policy_doc_no_lock / load_mapped_policy_no_lock with defaults
forwarding to the locked variants, so existing implementations and
test mocks keep their behavior.
- ObjectStore overrides them via the existing LoadMode::BootstrapNoLock
plumbing. Deletions triggered by the handlers keep locked writes.
- manager.rs: the four *_notification_handler paths (8 call sites)
switch to the lock-free variants.
- Integration test: while the lock quorum is unavailable (DistErasure
with empty lockers), load_group_no_lock must succeed exactly where
the locked load_group fails.
Request-path loads (check_key, verify_temp_user_persistence) and admin
write-then-reload paths intentionally stay locked: load_user_identity
embeds expiry deletions, so those need the side-effect extraction
tracked in rustfs/backlog#884 before going lock-free.
Verification:
- cargo test -p rustfs-iam --lib (156 passed)
- cargo test -p rustfs-iam --test iam_bootstrap_no_lock_test
- make pre-commit
Ref: rustfs/backlog#884, rustfs#4304
Co-authored-by: heihutu <heihutu@gmail.com>
* fix(server): drop unused iam_bootstrap_failure_hint import in tests
The hint tests live in their own hint_tests module with a local import;
the stale re-import in mod tests failed clippy's -D warnings on the
Test and Lint CI variants.
Verification:
- cargo clippy -p rustfs --all-targets
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix
---------
Co-authored-by: heihutu <heihutu@gmail.com>
* fix(migration): decrypt MinIO IAM & server config on drop-in migration
MinIO encrypts IAM identity/service-account files and the server config at
rest with a key derived from the root credentials. The drop-in migration
paths read those blobs from the legacy `.minio.sys` bucket and parsed them
as plaintext JSON, so any encrypted blob failed to parse and was silently
skipped with "incompatible format". This is why users migrating from MinIO
kept their buckets/objects/policies but lost users and access keys (#2212).
The IAM load path already knows how to decrypt these blobs (RustFS master
keys plus MinIO-compatible legacy keys derived from the root credentials),
but that logic lived behind a private method and was never used by the
migration paths. Expose it as `rustfs_iam::try_decrypt_iam_blob` and inject
it into both migration paths via a `LegacyBlobDecryptFn` callback (ecstore
cannot depend on the IAM crate, so the closure is wired in the binary crate).
When a blob cannot be decrypted the raw bytes are used as-is, preserving the
previous plaintext-only behavior with no regression.
Also improve object-layer migration observability without changing control
flow: `try_migrate_format` now distinguishes "no legacy format" (a normal
fresh install) from "legacy format present but incompatible", and the caller
logs a loud error before initializing a fresh format that would leave the
existing MinIO objects unreadable. Topology/version skip reasons are promoted
from debug to warn.
Fixes a pre-existing test isolation race by marking
`test_recovery_falls_back_to_default_config_when_blob_stays_corrupt` serial,
since it reads a process-wide env var toggled by a sibling test.
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(migration): box FormatV3 in LegacyFormatOutcome to satisfy clippy
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(ecstore): stabilize concurrent multipart resend lock timeout
concurrent_resend_same_part_commits_one_generation spawns 6 same-part
resends whose cross-disk commits serialize on the per-uploadId commit
lock. Under the full nextest suite the parallel disk load pushes those
serialized commits past the small default lock-acquire timeout (5s),
producing a spurious `Lock(Timeout ...)` unrelated to the property under
test (observed on CI at 5.775s vs ~0.5s in isolation).
Raise RUSTFS_OBJECT_LOCK_ACQUIRE_TIMEOUT to the production default (30s)
for the concurrent-commit section via temp_env, so the regression guard
reflects correctness (exactly one intact generation) rather than disk
latency under CI load. The meaningful assertions are unchanged, and
#[serial] keeps the process-wide env override isolated.
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(lock): bound fast-lock notification wait to prevent lost-wakeup stall
The real cause of the concurrent_resend_same_part_commits_one_generation
failures was a lost wakeup in the fast-lock slow path, not disk latency:
raising the acquire timeout to 30s only delayed the failure (it then timed
out at 30s), proving a genuine stall rather than overload.
In acquire_lock_slow_path a waiter that reaches the notification phase did a
single `timeout(remaining, wait_for_write())` spanning the whole acquire
budget, and treated that wait's elapse as a hard `Timeout`. But the release
path only notifies when `writer_waiters > 0`, so if the holder releases in
the gap after the waiter's `try_acquire` fails and before it registers as a
waiter, no notification (and no stored permit, since the pooled `Notify` is
gated) is produced. The waiter then blocks until the deadline even though the
lock is free and stays free — a spurious lock-acquire timeout. The shared
process-wide notify pool makes it worse: a wakeup can be consumed by a waiter
of a different lock hashing to the same slot.
Bound each notification wait (NOTIFY_WAIT_CAP = 50ms) and, on elapse, loop
back and re-`try_acquire` instead of returning `Timeout`; the deadline check
at the top of the loop is the single source of truth for timing out. A
lost/stolen wakeup now degrades to bounded re-polling (acquire within ~50ms
of the lock becoming free) instead of stalling for the whole timeout.
Correctness (mutual exclusion) is unchanged — acquisition still only happens
via `try_acquire_*`.
Add a regression test that reproduces the stall (holder + late waiter across
many keys): it times out without the fix and passes in ~1s with it. Revert
the earlier acquire-timeout workaround in the multipart test now that the
underlying stall is fixed, so it runs under the default timeout again.
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
feat(admin): add MinIO-compatible IAM/IDP admin endpoints
Register and implement MinIO admin API compatibility for IAM/IDP:
- PUT /v3/import-iam-v2 and POST /v3/revoke-tokens/{userProvider}
- generic /v3/idp-config/{type}[/{name}] CRUD mapped onto existing config
- LDAP/OpenID service-account, policy-entities, and list-access-keys flows
Adds IamSys::delete_temp_account primitive to back STS token revocation.
revoke-tokens requires the broader ListUsers admin capability for
cross-user revocation (self-revocation only needs RemoveServiceAccount),
mirroring the cross-user guard used by the service-account handlers.
Registers admin route-policy inventory entries for every new route.
Unsupported LDAP/OpenID backends return honest compatibility errors.
Refs rustfs/backlog#609 #610#616
Second TODO-convergence round over the current tree (backlog#646). All
line numbers in the old inventory had gone stale after the set_disk /
diagnostics / cluster refactors, so this re-scans and reduces the marker
count from 144 to 99.
STALE removals (comment describes already-implemented behavior, or dead
commented-out blocks) across ecstore (set_disk ops/core, store,
cluster/rpc, bucket/metadata_sys, services), iam, filemeta, s3select and
rustfs auth/object_usecase. No behavior change.
Safe fills, each verified:
- filemeta: replication_info_equals now also compares
replication_state_internal (function currently has no callers; adds a
regression test).
- bitrot: drop the confirmed-unused `_want` parameter from bitrot_verify
and the now-unused `sum` on LocalDisk::bitrot_verify, removing a
Bytes::copy_from_slice allocation. Streaming verify uses the file's
embedded per-shard hash, never the passed sum.
- signer: rename v4_ignored_headers -> V4_IGNORED_HEADERS and drop the
non_upper_case_globals allow.
- admin/heal: test_decode was #[ignore]d and used serde_urlencoded on a
JSON body (would panic); rewire to serde_json::from_slice to match the
production decode path, add assertions, un-ignore.
Verified: cargo fmt; cargo check on touched crates; tests pass
(filemeta, signer, bitrot, heal::test_decode); arch guardrail scripts
pass.
* fix(rio): reject corrupted short compressed/encrypted blocks instead of panicking
DecompressReader::poll_read and DecryptReader::poll_read sliced the block
body with a fixed `[0..16]` index to read the length varint. The body length
comes from an untrusted 24-bit header field, so a corrupted/truncated block
shorter than 16 bytes made the slice panic and crash the request task — a
read-path DoS on GET of tiered/corrupted data.
Pass the whole (arbitrary-length-safe) slice to uvarint and reject a
non-positive or out-of-range length prefix with InvalidData. Adds a repro
test for each reader; all existing round-trip tests still pass.
Refs rustfs/backlog#812
* fix(utils): close SSRF bypass via IPv4-mapped IPv6 addresses
validate_outbound_ip branched on the IpAddr variant, and the V6 branch's
is_loopback/is_unicast_link_local/is_unique_local checks never inspect the
embedded IPv4 of an IPv4-mapped address (::ffff:a.b.c.d). The metadata guard
also only matched the plain V4 169.254.169.254. So ::ffff:127.0.0.1,
::ffff:10.0.0.5 and ::ffff:169.254.169.254 all passed the outbound guard,
letting an attacker reach loopback/private/metadata endpoints.
Normalize IPv4-mapped IPv6 to its embedded IPv4 (via to_ipv4_mapped, which
matches only the true mapped form) before classification. Adds reject tests
for mapped loopback/private/metadata and an allow test for public IPv6.
Refs rustfs/backlog#813
* fix(ecstore): streaming last-part loss, GCS tier Range/remove, stat_all_dirs alignment
Four confirmed data-reliability defects:
- put_object_multipart_stream: the CompleteMultipartUpload part-collection loop
used exclusive `1..total_parts_count`, dropping the final part (and collecting
zero parts for a single-part object) — silently truncating the completed object.
Extracted collect_complete_parts (1..=total_parts_count) with unit tests.
- GCS warm backend get() ignored the requested byte range, returning the whole
object for a Range GET; now applies ReadRange::segment like the other backends.
- GCS warm backend remove() was an empty stub, so deleting a tiered object left
it on GCS forever; now deletes via StorageControl (added a control-plane client),
and in_use() actually lists (prefix-scoped) instead of always returning false.
- stat_all_dirs skipped None disk slots and dropped JoinErrors, returning a
compressed, misaligned error vector; heal_object_dir then zipped it against the
full disks array and could make_volume on the WRONG disk. Now returns one
index-aligned entry per slot (None -> DiskNotFound), and heal no longer
pre-fills the drive report (which would double it). Added an alignment test.
Refs rustfs/backlog#807
* fix(kms): stop Vault backend from destroying/reviving keys on failure
Two confirmed key-safety defects in the Vault KV2 backend:
- get_key_material() 'self-healed' a decrypt or wrong-length failure by minting a
fresh random master key and overwriting the stored value. That destroys the
original key material, making every DEK ever wrapped by it permanently
undecryptable. Decryption must never mutate the stored key: both branches now
return a cryptographic_error instead. (The empty-material bootstrap path, which
only fills a never-initialized key, is intentionally left intact.)
- cancel_key_deletion() reset key_state to Enabled only in the returned response
and never persisted it, so the key stayed PendingDeletion in storage and would
still be reaped. It now writes the state back via update_key_metadata_in_storage
and fails the request if the write fails.
Adds ignored (Vault-requiring) integration tests documenting both behaviours.
The third item (VaultTransit key state only in memory -> revived as Enabled after
restart) is deferred: a fail-closed guard would break restart availability for all
transit keys; the correct fix needs a persistent metadata store + Vault integration
testing. Tracked in rustfs/backlog#808.
Refs rustfs/backlog#808
* fix(admin): clamp STS AssumeRole duration; persist ImportBucketMetadata to disk
Two confirmed admin-API defects:
- Standard AssumeRole used the raw client-supplied DurationSeconds with no upper
bound, so a caller could mint near-permanent temporary credentials. Clamp it to
the AWS/MinIO STS window [900, 43200] (with 0 -> default 3600) via a shared
clamp_assume_role_duration helper, and build the exp claim with saturating_add.
This matches the existing AssumeRoleWithWebIdentity path.
- ImportBucketMetadata only mutated an in-memory map and returned 200, silently
dropping every imported config. It now persists each non-empty config via
metadata_sys::update (which merges onto existing on-disk metadata) and returns
InternalError if a write fails. Mapping extracted to imported_configs_to_persist
with unit tests.
Refs rustfs/backlog#809
* fix(heal): enqueue displacing request in release builds
push_displacing_lower_priority folded the real enqueue call into
debug_assert_eq!(self.push(request), Accepted). In release builds
(debug_assertions off) the whole macro — including its argument — is compiled
out, so after evicting a lower-priority queued item the new high-priority
request was silently dropped and never healed. Hoist self.push(request) out of
the assertion so the side effect runs in all builds. Adds a --release regression
test.
Refs rustfs/backlog#811
* fix(iam): propagate real delete_policy backend errors instead of swallowing them
delete_policy's is_from_notify path had its error handling inverted: a real
backend failure (disk IO / insufficient quorum) evicted the cache and returned
Ok(()), reporting a phantom success while policy.json survived on disk (to be
reloaded on the next full IAM reload); NoSuchPolicy — which should be idempotent
success — returned Err. Propagate real errors and let NoSuchPolicy fall through
to the idempotent cache-evict + Ok, matching delete_user / the notification
handler in the same file. Adds a backend-error-injection regression test.
Refs rustfs/backlog#810
* fix(utils): also normalize IPv4-compatible IPv6 in the SSRF guard
The initial fix only unwrapped IPv4-mapped (::ffff:a.b.c.d) addresses; the
deprecated IPv4-compatible form (::a.b.c.d, e.g. ::127.0.0.1 / ::169.254.169.254)
still bypassed the guard. Reject pure-IPv6 specials (::, ::1, fe80::, fc00::)
first, then normalize BOTH embedded-IPv4 forms before the IPv4 rules. Adds tests
for compatible-form loopback/metadata and confirms ::1 / :: stay rejected.
Found by adversarial review of the initial fix. Refs rustfs/backlog#813
* fix(ecstore): fix the same last-part loss in the parallel streaming path
put_object_multipart_stream_parallel had the identical off-by-one
(1..total_parts_count) that truncated the last part / produced zero parts for a
single-part upload — reachable when concurrent stream parts are enabled. Reuse
collect_complete_parts, which now returns an error instead of panicking on a gap
in the parts map. Adds a missing-part error test.
Found by adversarial review of the initial fix. Refs rustfs/backlog#807
* fix(kms): local backend must preserve key material on status change
LocalKmsClient (the default KMS backend) regenerated the master key material on
enable_key/disable_key/schedule_key_deletion/cancel_key_deletion — a pure status
change. A single disable+enable cycle therefore destroyed the original key,
making every DEK ever wrapped by it permanently undecryptable (silent data loss,
no network needed). Preserve the existing material via get_key_material and
re-save with only the status changed. Adds a hermetic regression test that wraps
a DEK, cycles all four status methods, and asserts the DEK still decrypts.
Found by adversarial review of the Vault fix. Refs rustfs/backlog#808
* test(rio): cover the length-prefix guard; correct its comment
Add a DecompressReader test that feeds an unterminated length varint so uvarint
returns 0 and the new guard (not the downstream codec) produces the InvalidData
error, and reword the guard comment which overclaimed that the > len bound
prevents a reachable panic (it is belt-and-suspenders). No behavior change.
Found by adversarial review. Refs rustfs/backlog#812
* test(rio): build test block headers via vec! to satisfy clippy
The new corrupted-block tests built the header with Vec::new() + repeated push,
tripping clippy::vec_init_then_push (-D warnings in CI). Construct the fixed
header bytes with vec![] instead. No behavior change.
---------
Co-authored-by: houseme <housemecn@gmail.com>
Remove or consolidate 57 test cases that cannot catch regressions
(literal-constant asserts, construct-then-assert, derived-serde
round-trips, near-duplicate env/getter matrices) in common, config,
iam, madmin, and object-capacity, keeping all wire-format and
error-path guards. Add 13 tests for previously uncovered high-risk
behavior: filemeta version-sort determinism and merge resilience to
garbage headers, zip extraction path-traversal rejection and exact
limit boundaries, JWT tampered-signature rejection, and the bytes
variant of dual-key (rustfs/minio) metadata fallback and precedence.
Test-only change; no production code touched.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Zhengchao An