mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-29 00:17:11 +00:00
fix(s3): reject tampered multipart payloads cleanly (#6578)
This commit is contained in:
@@ -9,4 +9,4 @@
|
|||||||
# if the selected count drops below this number, so a rename or removal that
|
# if the selected count drops below this number, so a rename or removal that
|
||||||
# thins the security smoke gate must update this file in the same PR.
|
# thins the security smoke gate must update this file in the same PR.
|
||||||
# Adding tests does not require a bump, but bumping keeps the guard tight.
|
# Adding tests does not require a bump, but bumping keeps the guard tight.
|
||||||
16
|
18
|
||||||
|
|||||||
@@ -325,6 +325,57 @@ async fn tampered_payload_is_rejected() -> Result<(), Box<dyn std::error::Error
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A signed UploadPart body must pass the same payload-hash gate as PutObject.
|
||||||
|
/// Rejection must happen before the part is published into the multipart upload.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn tampered_upload_part_payload_is_rejected() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
|
||||||
|
init_logging();
|
||||||
|
let mut env = RustFSTestEnvironment::new().await?;
|
||||||
|
setup(&mut env).await?;
|
||||||
|
|
||||||
|
let key = "tampered-upload-part.bin";
|
||||||
|
let client = env.create_s3_client();
|
||||||
|
let upload = client.create_multipart_upload().bucket(BUCKET).key(key).send().await?;
|
||||||
|
let upload_id = upload.upload_id().ok_or("create multipart upload omitted upload_id")?;
|
||||||
|
|
||||||
|
let path = format!("/{BUCKET}/{key}");
|
||||||
|
let canonical_query = format!("partNumber=1&uploadId={}", urlencoding::encode(upload_id));
|
||||||
|
let request_target = format!("{path}?{canonical_query}");
|
||||||
|
let claimed_body = b"the-part-i-claim-to-send";
|
||||||
|
let actual_body = b"the-part-i-really-send!!";
|
||||||
|
assert_eq!(claimed_body.len(), actual_body.len(), "keep content-length stable for the mismatch");
|
||||||
|
|
||||||
|
let signer = SigV4::new(&env);
|
||||||
|
let headers = signer.sign("PUT", &path, &canonical_query, &sha256_hex(claimed_body));
|
||||||
|
let resp = send_signed(&env, reqwest::Method::PUT, &request_target, &headers, Some(actual_body.to_vec())).await?;
|
||||||
|
let status = resp.status();
|
||||||
|
let body = resp.text().await.unwrap_or_default();
|
||||||
|
assert_eq!(
|
||||||
|
status,
|
||||||
|
reqwest::StatusCode::BAD_REQUEST,
|
||||||
|
"multipart payload mismatch must be rejected as BadDigest, body:\n{body}"
|
||||||
|
);
|
||||||
|
assert_error_code(&body, "BadDigest");
|
||||||
|
|
||||||
|
let parts = client
|
||||||
|
.list_parts()
|
||||||
|
.bucket(BUCKET)
|
||||||
|
.key(key)
|
||||||
|
.upload_id(upload_id)
|
||||||
|
.send()
|
||||||
|
.await?;
|
||||||
|
assert!(parts.parts().is_empty(), "a tampered UploadPart must not publish a part");
|
||||||
|
|
||||||
|
client
|
||||||
|
.abort_multipart_upload()
|
||||||
|
.bucket(BUCKET)
|
||||||
|
.key(key)
|
||||||
|
.upload_id(upload_id)
|
||||||
|
.send()
|
||||||
|
.await?;
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
/// (e) A request whose `x-amz-date` is skewed beyond the server's tolerance
|
/// (e) A request whose `x-amz-date` is skewed beyond the server's tolerance
|
||||||
/// (s3s default 900s / 15 min) must be rejected with RequestTimeTooSkewed /
|
/// (s3s default 900s / 15 min) must be rejected with RequestTimeTooSkewed /
|
||||||
/// 403. The signature is otherwise valid: the credential-scope date and
|
/// 403. The signature is otherwise valid: the credential-scope date and
|
||||||
|
|||||||
@@ -71,7 +71,7 @@ use crate::app::object_data_cache::{
|
|||||||
};
|
};
|
||||||
use crate::app::object_usecase::{
|
use crate::app::object_usecase::{
|
||||||
acquire_copy_bucket_lifecycle_locks, apply_quota_admission, build_put_like_object_lock_metadata, map_quota_check_outcome,
|
acquire_copy_bucket_lifecycle_locks, apply_quota_admission, build_put_like_object_lock_metadata, map_quota_check_outcome,
|
||||||
validate_existing_object_lock_for_write,
|
s3s_body_error_to_io, validate_existing_object_lock_for_write,
|
||||||
};
|
};
|
||||||
use crate::app::runtime_sources::{
|
use crate::app::runtime_sources::{
|
||||||
AppContext, current_app_context, current_object_data_cache_for_context, current_object_store_handle_for_context,
|
AppContext, current_app_context, current_object_data_cache_for_context, current_object_store_handle_for_context,
|
||||||
@@ -988,7 +988,7 @@ impl DefaultMultipartUsecase {
|
|||||||
let mut total = 0i64;
|
let mut total = 0i64;
|
||||||
let mut buffer = bytes::BytesMut::new();
|
let mut buffer = bytes::BytesMut::new();
|
||||||
while let Some(chunk) = body_stream.next().await {
|
while let Some(chunk) = body_stream.next().await {
|
||||||
let chunk = chunk.map_err(|e| ApiError::from(StorageError::other(e.to_string())))?;
|
let chunk = chunk.map_err(|e| ApiError::from(s3s_body_error_to_io(e)))?;
|
||||||
total += chunk.len() as i64;
|
total += chunk.len() as i64;
|
||||||
buffer.extend_from_slice(&chunk);
|
buffer.extend_from_slice(&chunk);
|
||||||
}
|
}
|
||||||
@@ -1022,7 +1022,7 @@ impl DefaultMultipartUsecase {
|
|||||||
let buffer_size = get_buffer_size_opt_in(size);
|
let buffer_size = get_buffer_size_opt_in(size);
|
||||||
let body = tokio::io::BufReader::with_capacity(
|
let body = tokio::io::BufReader::with_capacity(
|
||||||
buffer_size,
|
buffer_size,
|
||||||
StreamReader::new(body_stream.map(|f| f.map_err(|e| std::io::Error::other(e.to_string())))),
|
StreamReader::new(body_stream.map(|f| f.map_err(s3s_body_error_to_io))),
|
||||||
);
|
);
|
||||||
|
|
||||||
let is_disk_compressed = rustfs_utils::http::contains_key_str(&fi.user_defined, rustfs_utils::http::SUFFIX_COMPRESSION);
|
let is_disk_compressed = rustfs_utils::http::contains_key_str(&fi.user_defined, rustfs_utils::http::SUFFIX_COMPRESSION);
|
||||||
|
|||||||
@@ -234,7 +234,7 @@ use crate::app::object_traffic_health::ObjectTrafficHealth;
|
|||||||
|
|
||||||
type S3StdError = Box<dyn std::error::Error + Send + Sync + 'static>;
|
type S3StdError = Box<dyn std::error::Error + Send + Sync + 'static>;
|
||||||
|
|
||||||
fn s3s_body_error_to_io(err: StdError) -> io::Error {
|
pub(crate) fn s3s_body_error_to_io(err: StdError) -> io::Error {
|
||||||
io::Error::other(err)
|
io::Error::other(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user