Skip to content

fix(sigv4): harden replay window and compares#15

Closed
houseme wants to merge 9 commits into
mainfrom
houseme/issue-3555-sigv4-hardening
Closed

fix(sigv4): harden replay window and compares#15
houseme wants to merge 9 commits into
mainfrom
houseme/issue-3555-sigv4-hardening

Conversation

@houseme

@houseme houseme commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • harden SigV4 comparisons by switching header, presigned URL, and POST signature equality checks to constant-time comparison
  • enforce the configured SigV4 clock-skew window for header-authenticated requests and signed POST uploads, not only presigned URLs
  • refresh the existing SigV4 and POST policy regression fixtures to sign with current request timestamps, and add focused replay-window tests for stale header and POST signatures

Related Tracking

  • Tracks rustfs/backlog#694
  • Parent context: rustfs/rustfs#3520
  • Original deleted tracker: rustfs/rustfs#3555

Verification

  • cargo fmt --all
  • cargo fmt --all --check
  • cargo test -p s3s v4_header_auth_rejects_stale_request_time -- --nocapture
  • cargo test -p s3s v4_post_signature_rejects_stale_request_time -- --nocapture
  • cargo test -p s3s sig_v4_signatures_match_reports_match_and_mismatch -- --nocapture
  • cargo test -p s3s
  • just ci-rust

Notes

  • this PR stays scoped to the fork-level SigV4 verifier and its regression tests
  • no repository-local RustFS integration changes are included here

@houseme houseme force-pushed the houseme/issue-3555-sigv4-hardening branch from aa2f397 to c5018e0 Compare June 20, 2026 00:33
Nugine added 3 commits June 21, 2026 17:34
…via config

Restore deterministic timestamps in existing SigV4 and POST policy
tests, using presigned_url_max_skew_time_secs: u32::MAX to bypass
clock-skew enforcement instead of switching to OffsetDateTime::now_utc().

- Derive stale-request offsets from config in new regression tests
- Remove now-unused fmt_current_amz_date from tests.rs
- Apply clippy field_reassign_with_default fix
…skew

Expose the `now` instant and `S3Config` snapshot as explicit
parameters so callers control timing and the caller resolves the
config snapshot. Removes the hidden `now_utc()` call and
`.snapshot()` resolution inside the function.
Per AWS SigV4 spec, the date in the credential scope must match
the date in the x-amz-date header/form-field. Previously the
verifier ignored the credential date; add explicit checks in all
three SigV4 paths (header auth, POST signature, presigned URL).
@Nugine Nugine force-pushed the houseme/issue-3555-sigv4-hardening branch from 17fbaa3 to 095afbd Compare June 21, 2026 13:15
Per AWS SigV4 spec, the signed POST policy must contain eq conditions
for x-amz-date, x-amz-credential, and x-amz-algorithm that match
the submitted form fields exactly. This closes a replay-window bypass
where the clock-skew check trusted the unsigned form field x-amz-date
while the signing key only bound YYYYMMDD.

- Add PostPolicy::eq_condition_value helper
- Validate policy conditions in v4_check_post_signature, before
  clock-skew check and signature verification
- Update test fixtures to include required eq conditions
@Nugine Nugine force-pushed the houseme/issue-3555-sigv4-hardening branch from 095afbd to 43e4f3b Compare June 21, 2026 13:22
…TODO

- Move x-amz-date extraction, credential-date validation, and
  clock-skew check before the secret key lookup in v4_check_header_auth
  so stale requests are rejected before I/O.

- Replace remaining S3Error::new + set_message patterns in the new
  credential-date checks with the s3_error! macro for consistency.

- Add TODO in v4_check_post_signature noting the double policy parse
  (signature verification + later prepare stage) as an optimization
  target.
@Nugine Nugine force-pushed the houseme/issue-3555-sigv4-hardening branch from 6ef5e4b to 70090fe Compare June 21, 2026 13:56
@houseme houseme closed this Jun 22, 2026
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.

2 participants