Skip to content

Commit

Permalink
Fix s3 checksum-mode header in presigned requests
Browse files Browse the repository at this point in the history
Also update our presigned canary tests to do both PUT and GET and have
extra headers included that aren't in the query string
  • Loading branch information
landonxjames committed Feb 6, 2025
1 parent 5ed776f commit dea54b8
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,17 @@ class HttpResponseChecksumCustomization(
.load::<#{ResponseChecksumValidation}>()
.unwrap_or(&#{ResponseChecksumValidation}::WhenSupported);
let is_presigned_req = cfg.load::<#{PresigningMarker}>().is_some();
// For presigned requests we do not enable the checksum mode header.
// If validation setting is WhenSupported (or unknown) we enable response checksum
// validation. If it is WhenRequired we do not enable (since there is no way to
// indicate that a response checksum is required).
##[allow(clippy::wildcard_in_or_patterns)]
match response_checksum_validation {
#{ResponseChecksumValidation}::WhenRequired => {}
#{ResponseChecksumValidation}::WhenSupported | _ => {
match (is_presigned_req, response_checksum_validation) {
(true, _) => {}
(false, #{ResponseChecksumValidation}::WhenRequired) => {}
(false, #{ResponseChecksumValidation}::WhenSupported) | _ => {
input.$requestValidationModeName = Some(#{ValidationModeShape}::Enabled);
}
}
Expand Down Expand Up @@ -206,6 +210,7 @@ class HttpResponseChecksumCustomization(
CargoDependency.smithyTypes(codegenContext.runtimeConfig).toType()
.resolve("checksum_config::ResponseChecksumValidation"),
"ConfigBag" to RuntimeType.configBag(codegenContext.runtimeConfig),
"PresigningMarker" to AwsRuntimeType.presigning().resolve("PresigningMarker"),
)
}
}
Expand Down
38 changes: 31 additions & 7 deletions tools/ci-cdk/canary-lambda/src/latest/s3_canary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ use crate::{mk_canary, CanaryEnv};
use anyhow::{Context, Error};
use aws_config::SdkConfig;
use aws_sdk_s3 as s3;
use aws_sdk_s3::types::{CompletedMultipartUpload, CompletedPart, Delete, ObjectIdentifier};
use aws_sdk_s3::types::{
CompletedMultipartUpload, CompletedPart, Delete, ObjectIdentifier, RequestPayer,
};
use s3::config::Region;
use s3::presigning::PresigningConfig;
use s3::primitives::ByteStream;
Expand All @@ -31,6 +33,7 @@ mk_canary!("s3", |sdk_config: &SdkConfig, env: &CanaryEnv| {
/// Runs canary exercising S3 APIs against a regular bucket
pub async fn s3_canary(client: s3::Client, s3_bucket_name: String) -> anyhow::Result<()> {
let test_key = Uuid::new_v4().as_u128().to_string();
let presigned_test_key = test_key.clone().push_str("_presigned");

// Look for the test object and expect that it doesn't exist
match client
Expand Down Expand Up @@ -74,21 +77,42 @@ pub async fn s3_canary(client: s3::Client, s3_bucket_name: String) -> anyhow::Re
.await
.context("s3::GetObject[2]")?;

// repeat the test with a presigned url
let uri = client
// Repeat the GET/PUT tests with a presigned url
let reqwest_client = reqwest::Client::new();

let presigned_put = client
.put_object()
.bucket(&s3_bucket_name)
.key(&presigned_test_key)
.presigned(PresigningConfig::expires_in(Duration::from_secs(120)).unwrap())
.await
.unwrap();
let http_put = presigned_request.make_http_1x_request("presigned_test");
let reqwest_put = reqwest::Request::try_from(http_put).unwrap();
let put_resp = reqwest_client.execute(reqwest_put).await?;
if put_resp.is_err() {
return Err(CanaryError(format!("presigned put failed: {:?}", put_resp)).into());
}

let presigned_get = client
.get_object()
.bucket(&s3_bucket_name)
.key(&test_key)
.key(&presigned_test_key)
// Ensure a header is included that isn't in the query string
.request_payer(RequestPayer::Requester)
.presigned(PresigningConfig::expires_in(Duration::from_secs(120)).unwrap())
.await
.unwrap();
let response = reqwest::get(uri.uri().to_string())
let headers = presigned_get.make_http_1x_request("").headers().clone();
let get_resp = reqwest_client
.get(presigned_get.uri().to_string())
.headers(headers)
.await
.context("s3::presigned")?
.text()
.await?;
if response != "test" {
return Err(CanaryError(format!("presigned URL returned bad data: {:?}", response)).into());
if get_resp != "presigned_test" {
return Err(CanaryError(format!("presigned URL returned bad data: {:?}", get_resp)).into());
}

let metadata_value = output
Expand Down
2 changes: 1 addition & 1 deletion tools/ci-cdk/canary-runner/src/build_bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ arbitrary = "=1.3.2"
lazy_static! {
static ref REQUIRED_SDK_CRATES: Vec<RequiredDependency> = vec![
RequiredDependency::new("aws-config").with_features(["behavior-version-latest"]),
RequiredDependency::new("aws-sdk-s3"),
RequiredDependency::new("aws-sdk-s3").with_features(["http-1x"]),
RequiredDependency::new("aws-sdk-ec2"),
RequiredDependency::new("aws-sdk-transcribestreaming"),
RequiredDependency::new("aws-smithy-wasm"),
Expand Down

0 comments on commit dea54b8

Please sign in to comment.