Skip to content

fix(admin): preserve Heal target identity in SigV4 signing - #374

Merged
cxymds merged 1 commit into
mainfrom
cxymds/fix-rc-heal-path-signing
Sep 10, 2026
Merged

fix(admin): preserve Heal target identity in SigV4 signing#374
cxymds merged 1 commit into
mainfrom
cxymds/fix-rc-heal-path-signing

Conversation

@cxymds

@cxymds cxymds commented Sep 10, 2026

Copy link
Copy Markdown
Member

Related work

No dedicated rc issue. Companion server fix: rustfs/rustfs#7644. Validation harness: https://github.com/rustfs/rustfs-release-validation/pull/14.

Problem and root cause

Heal prefixes are encoded as one URL component, so the object dir/key is sent as dir%2Fkey, while the distinct object dir%2Fkey is sent as dir%252Fkey. The Admin signer used the generic AWS SigV4 defaults: double percent encoding and path normalization. Its canonical URI therefore disagreed with MinIO-compatible Admin verification, causing SignatureDoesNotMatch even after the server's object-route fix.

MinIO canonicalizes the decoded URL path with S3 path encoding, retaining slashes (upstream signer). Changing only the encoding setting is insufficient: an encoded slash must become a slash in the signing URI, without rewriting the transport URL or decoding a literal %2F twice.

Solution

  • Build an Admin-only signing URI by decoding the wire path once and re-encoding non-S3-safe bytes while preserving slashes. Preserve the query and authority.
  • Select single percent encoding and disable canonical path normalization. The actual HTTP request still uses its original URL.
  • Reject a target before sending if the HTTP URL parser would normalize its path, such as a standalone . or .. segment, to avoid selecting a different target.
  • Leave authentication, ordinary S3 signing, CLI commands, output/config contracts, dependencies, and protected files unchanged.

Regression coverage

Four new tests cover 15 canonical path vectors, encoded/literal slashes, mixed-case escapes, Unicode, spaces, plus/percent characters, repeated separators, encoded dot segments, byte preservation, query signing, payload hashes, and IPv6 authority. The canonical-request oracle is constructed independently; only the HMAC primitives are shared with the SDK.

A real HTTP capture test checks start/status/stop for three distinct prefixes (nine requests), including the exact transport target and signature. Another regression verifies rejection of five transport-normalized paths. All new regressions failed against the original signer before the fix.

Validation

Based on freshly fetched main at fbf5e9f447d9d0eca7c850c3898343a59a74ff0e. The submitted source passed these checks before committing (local builds used six jobs, disabled debug info/incremental compilation, and localhost proxy bypass):

  • cargo fmt --all --check
  • cargo clippy --locked --workspace --all-targets -- -D warnings
  • cargo test --locked --workspace: 2,673 passed, zero failed, two existing ignored tests (the opt-in real RustFS ILM probe and a retry doctest).
  • cargo test --locked -p rc-s3 admin::tests -- --nocapture: 174 passed.
  • cargo build --locked -p rustfs-cli --bin rc
  • ./scripts/check-protected-files.sh origin/main and git diff --check.

The actual built rc executable also passed a local, isolated-process API check against the server patch in rustfs/rustfs#7644:

  • Start, status, and stop succeeded for objects/0000.bin, the distinct objects%2F0000.bin, and 中文/space +percent%/object.bin using --dry-run --scan-mode deep.
  • Raw server canonical receipts proved exactly one intended object per task, completed coverage, dry_run_observed, zero repaired/failed/unknown items, and no counter overflow.
  • Original object bodies, metadata, tags, and the exact object-name set remained unchanged.
  • Cluster info succeeded; a deliberately wrong secret was rejected with SignatureDoesNotMatch; a standalone dot target was rejected before transport.
  • The owned server process and disposable test data were cleaned up.

Tested rc binary SHA-256: fd08e9e0636ecffd7af8e1bfaf257101fa7592007189ad769b980735f55d7885. The current main workspace reports rc 0.1.24; this is a source build, not a newly published release package. Server binary SHA-256: e9c15073f20804d833808089a3750c541ec7341df482de00d71f9fd96ce1e11b (release ancestor 97c7b451d plus patch 8346a3d35).

Boundaries and observations

This is not a jump-host/four-disk run, a fault-repair proof, active-task cancellation, or an 84-case matrix PASS. Stop was checked against completed tasks. rc does not send pool/set selectors, and the observed topology/incarnation fields were null; no scoped repair claim is made. Existing scoped physical-validation assertions were not changed.

The local probe also observed the server's legacy status metadata echoing default settings (scanMode=1, dryRun=false) after a deep dry-run, while the canonical disposition correctly reported dry_run_observed. That server status-reporting discrepancy is outside this signing-only change. rustfs/rustfs#7644's separate s3s-footprint CI failure is also not changed here.

@cxymds
cxymds merged commit 1b212cb into main Sep 10, 2026
17 checks passed
@cxymds
cxymds deleted the cxymds/fix-rc-heal-path-signing branch September 10, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant