mirror of
https://github.com/rustfs/rustfs.git
synced 2026-09-08 21:25:59 +00:00
fix(s3): reject unsigned x-amz headers on presigned requests
A SigV4 presigned URL only binds the headers listed in X-Amz-SignedHeaders, but the handlers applied every x-amz-* request header regardless. The holder of a presigned PutObject URL signed with SignedHeaders=host could add x-amz-tagging, x-amz-storage-class, x-amz-website-redirect-location, ACL, metadata, Object Lock or SSE headers and have them applied (GHSA-g8w9-qw9q-fghr). Reject such requests at the S3 access boundary with 403 AccessDenied and the AWS message "There were headers present in the request which were not signed"; x-amz-cf-id stays tolerated for CloudFront. SigV2 and header-signed SigV4 requests are unchanged. Regression tests are named after the advisory (unit tests in rustfs/src/auth.rs, e2e in crates/e2e_test/src/presigned_negative_test.rs with a signed-tagging positive control); the security smoke floor rises to 20 and the e2e selection digests are refreshed for the two new cases.
This commit is contained in:
@@ -356,3 +356,119 @@ async fn tampered_presigned_put_returns_signature_does_not_match() -> Result<(),
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// GHSA-g8w9-qw9q-fghr: a presigned PUT signed with `SignedHeaders=host` must
|
||||
/// not honour `x-amz-*` headers the uploader adds afterwards. The presign
|
||||
/// authorised one plain upload; the extra headers would set tags, storage
|
||||
/// class and a website redirect the presigner never covered. AWS S3 rejects
|
||||
/// this with 403 `AccessDenied`, and so must RustFS — and the object must not
|
||||
/// be stored at all, not merely stored without the properties.
|
||||
#[tokio::test]
|
||||
async fn ghsa_g8w9_presigned_put_rejects_unsigned_x_amz_headers() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
|
||||
init_logging();
|
||||
let mut env = RustFSTestEnvironment::new().await?;
|
||||
setup(&mut env).await?;
|
||||
|
||||
let key = "presigned-put-unsigned-amz-headers.txt";
|
||||
let pr = env
|
||||
.create_s3_client()
|
||||
.put_object()
|
||||
.bucket(BUCKET)
|
||||
.key(key)
|
||||
.presigned(valid_config())
|
||||
.await?;
|
||||
assert!(
|
||||
!pr.headers().any(|(name, _)| name.eq_ignore_ascii_case("x-amz-tagging")),
|
||||
"fixture must presign a plain PutObject without tagging so the header below is unsigned"
|
||||
);
|
||||
|
||||
let unsigned: Vec<(&str, &str)> = vec![
|
||||
("x-amz-tagging", "owner=attacker&classification=public"),
|
||||
("x-amz-website-redirect-location", "https://attacker.example/phish"),
|
||||
("x-amz-storage-class", "REDUCED_REDUNDANCY"),
|
||||
];
|
||||
let headers = pr.headers().chain(unsigned.iter().copied());
|
||||
let resp = send_raw(pr.method(), pr.uri(), headers, Some(b"should-not-be-stored".to_vec())).await?;
|
||||
let status = resp.status();
|
||||
let body = resp.text().await?;
|
||||
assert_eq!(
|
||||
status.as_u16(),
|
||||
403,
|
||||
"presigned PUT with unsigned x-amz-* headers must be 403, body:\n{body}"
|
||||
);
|
||||
assert_error_code(&body, "AccessDenied");
|
||||
assert!(
|
||||
body.contains("were not signed"),
|
||||
"rejection must name unsigned headers as the cause, got:\n{body}"
|
||||
);
|
||||
|
||||
let error = env
|
||||
.create_s3_client()
|
||||
.head_object()
|
||||
.bucket(BUCKET)
|
||||
.key(key)
|
||||
.send()
|
||||
.await
|
||||
.expect_err("presigned PUT with unsigned x-amz-* headers must not store the object");
|
||||
assert_eq!(
|
||||
error.raw_response().map(|response| response.status().as_u16()),
|
||||
Some(404),
|
||||
"absence probe after the rejected upload must return HTTP 404, got {error:?}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// GHSA-g8w9-qw9q-fghr positive control: when the presigner itself sets the
|
||||
/// property, the SDK lists `x-amz-tagging` in `SignedHeaders`, the uploader
|
||||
/// replays it, and the upload succeeds with the tags applied. Without this the
|
||||
/// negative test above could pass because the server rejects every tagged
|
||||
/// presigned upload.
|
||||
#[tokio::test]
|
||||
async fn ghsa_g8w9_presigned_put_accepts_signed_x_amz_headers() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
|
||||
init_logging();
|
||||
let mut env = RustFSTestEnvironment::new().await?;
|
||||
setup(&mut env).await?;
|
||||
|
||||
let key = "presigned-put-signed-tagging.txt";
|
||||
let pr = env
|
||||
.create_s3_client()
|
||||
.put_object()
|
||||
.bucket(BUCKET)
|
||||
.key(key)
|
||||
.tagging("owner=app")
|
||||
.presigned(valid_config())
|
||||
.await?;
|
||||
assert!(
|
||||
pr.headers().any(|(name, _)| name.eq_ignore_ascii_case("x-amz-tagging")),
|
||||
"fixture must carry x-amz-tagging as a signed header"
|
||||
);
|
||||
assert!(
|
||||
pr.uri().contains("x-amz-tagging"),
|
||||
"X-Amz-SignedHeaders must list x-amz-tagging, uri: {}",
|
||||
pr.uri()
|
||||
);
|
||||
|
||||
let resp = send_presigned(&pr, Some(b"stored-with-signed-tagging".to_vec())).await?;
|
||||
let status = resp.status();
|
||||
let body = resp.text().await?;
|
||||
assert!(
|
||||
status.is_success(),
|
||||
"presigned PUT with signed x-amz-tagging must succeed, got {status}, body:\n{body}"
|
||||
);
|
||||
|
||||
let tags = env
|
||||
.create_s3_client()
|
||||
.get_object_tagging()
|
||||
.bucket(BUCKET)
|
||||
.key(key)
|
||||
.send()
|
||||
.await?;
|
||||
let tag_set: Vec<(String, String)> = tags
|
||||
.tag_set()
|
||||
.iter()
|
||||
.map(|tag| (tag.key().to_string(), tag.value().to_string()))
|
||||
.collect();
|
||||
assert_eq!(tag_set, vec![("owner".to_string(), "app".to_string())], "signed tagging must be applied");
|
||||
info!("signed presigned tagging control passed");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user