Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main' into davidpz/split-runti…
Browse files Browse the repository at this point in the history
…meerror-and-requestrejection-into-protocol-specific-structs
  • Loading branch information
david-perez committed Mar 31, 2023
2 parents 3ed02d1 + 637333c commit c271761
Show file tree
Hide file tree
Showing 63 changed files with 1,860 additions and 894 deletions.
22 changes: 22 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,25 @@ message = "The `enableNewCrateOrganizationScheme` codegen flag has been removed.
references = ["smithy-rs#2507"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" }
author = "jdisanti"

[[aws-sdk-rust]]
message = """
S3's `GetObject` will no longer panic when checksum validation is enabled and the target object was uploaded as a multi-part upload.
However, these objects cannot be checksum validated by the SDK due to the way checksums are calculated for multipart uploads.
For more information, see [this page](https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html#large-object-checksums).
"""
references = ["aws-sdk-rust#764"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "Velfi"

[[aws-sdk-rust]]
message = "`AppName` is now configurable from within `ConfigLoader`."
references = ["smithy-rs#2513"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "ysaito1001"

[[aws-sdk-rust]]
message = "Add support for omitting session token in canonical requests for SigV4 signing."
references = ["smithy-rs#2473"]
meta = { "breaking" = false, "tada" = false, "bug" = false }
author = "martinjlowm"
40 changes: 33 additions & 7 deletions aws/rust-runtime/aws-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,25 @@ mod loader {
self
}

/// Override the name of the app used to build [`SdkConfig`](aws_types::SdkConfig).
///
/// This _optional_ name is used to identify the application in the user agent that
/// gets sent along with requests.
///
/// # Examples
/// ```no_run
/// # async fn create_config() {
/// use aws_config::AppName;
/// let config = aws_config::from_env()
/// .app_name(AppName::new("my-app-name").expect("valid app name"))
/// .load().await;
/// # }
/// ```
pub fn app_name(mut self, app_name: AppName) -> Self {
self.app_name = Some(app_name);
self
}

/// Provides the ability to programmatically override the profile files that get loaded by the SDK.
///
/// The [`Default`] for `ProfileFiles` includes the default SDK config and credential files located in
Expand Down Expand Up @@ -628,19 +647,19 @@ mod loader {
)
.load()
.await;
assert_eq!(loader.retry_config().unwrap().max_attempts(), 10);
assert_eq!(loader.region().unwrap().as_ref(), "us-west-4");
assert_eq!(10, loader.retry_config().unwrap().max_attempts());
assert_eq!("us-west-4", loader.region().unwrap().as_ref());
assert_eq!(
"akid",
loader
.credentials_provider()
.unwrap()
.provide_credentials()
.await
.unwrap()
.access_key_id(),
"akid"
);
assert_eq!(loader.app_name(), Some(&AppName::new("correct").unwrap()));
assert_eq!(Some(&AppName::new("correct").unwrap()), loader.app_name());
logs_assert(|lines| {
let num_config_loader_logs = lines
.iter()
Expand Down Expand Up @@ -669,16 +688,23 @@ mod loader {
#[tokio::test]
async fn load_fips() {
let conf = base_conf().use_fips(true).load().await;
assert_eq!(conf.use_fips(), Some(true));
assert_eq!(Some(true), conf.use_fips());
}

#[tokio::test]
async fn load_dual_stack() {
let conf = base_conf().use_dual_stack(false).load().await;
assert_eq!(conf.use_dual_stack(), Some(false));
assert_eq!(Some(false), conf.use_dual_stack());

let conf = base_conf().load().await;
assert_eq!(conf.use_dual_stack(), None);
assert_eq!(None, conf.use_dual_stack());
}

#[tokio::test]
async fn app_name() {
let app_name = AppName::new("my-app-name").unwrap();
let conf = base_conf().app_name(app_name.clone()).load().await;
assert_eq!(Some(&app_name), conf.app_name());
}
}
}
2 changes: 1 addition & 1 deletion aws/rust-runtime/aws-inlineable/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ aws-types = { path = "../aws-types" }
bytes = "1"
bytes-utils = "0.1.1"
hex = "0.4.3"
http = "0.2.4"
http = "0.2.9"
http-body = "0.4.5"
md-5 = "0.10.1"
ring = "0.16"
Expand Down
80 changes: 74 additions & 6 deletions aws/rust-runtime/aws-inlineable/src/http_body_checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
//! Functions for modifying requests and responses for the purposes of checksum validation
use aws_smithy_http::operation::error::BuildError;
use http::header::HeaderName;

/// Errors related to constructing checksum-validated HTTP requests
#[derive(Debug)]
Expand Down Expand Up @@ -185,15 +184,32 @@ pub(crate) fn check_headers_for_precalculated_checksum(
let checksum_algorithm: aws_smithy_checksums::ChecksumAlgorithm = checksum_algorithm.parse().expect(
"CHECKSUM_ALGORITHMS_IN_PRIORITY_ORDER only contains valid checksum algorithm names",
);
if let Some(precalculated_checksum) = headers.get(HeaderName::from(checksum_algorithm)) {
if let Some(precalculated_checksum) =
headers.get(http::HeaderName::from(checksum_algorithm))
{
let base64_encoded_precalculated_checksum = precalculated_checksum
.to_str()
.expect("base64 uses ASCII characters");

let precalculated_checksum: bytes::Bytes =
aws_smithy_types::base64::decode(base64_encoded_precalculated_checksum)
.expect("services will always base64 encode the checksum value per the spec")
.into();
// S3 needs special handling for checksums of objects uploaded with `MultiPartUpload`.
if is_part_level_checksum(base64_encoded_precalculated_checksum) {
tracing::warn!(
more_info = "See https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html#large-object-checksums for more information.",
"This checksum is a part-level checksum which can't be validated by the Rust SDK. Disable checksum validation for this request to fix this warning.",
);

return None;
}

let precalculated_checksum = match aws_smithy_types::base64::decode(
base64_encoded_precalculated_checksum,
) {
Ok(decoded_checksum) => decoded_checksum.into(),
Err(_) => {
tracing::error!("Checksum received from server could not be base64 decoded. No checksum validation will be performed.");
return None;
}
};

return Some((checksum_algorithm, precalculated_checksum));
}
Expand All @@ -202,9 +218,38 @@ pub(crate) fn check_headers_for_precalculated_checksum(
None
}

fn is_part_level_checksum(checksum: &str) -> bool {
let mut found_number = false;
let mut found_dash = false;

for ch in checksum.chars().rev() {
// this could be bad
if ch.is_ascii_digit() {
found_number = true;
continue;
}

// Yup, it's a part-level checksum
if ch == '-' {
if found_dash {
// Found a second dash?? This isn't a part-level checksum.
return false;
}

found_dash = true;
continue;
}

break;
}

found_number && found_dash
}

#[cfg(test)]
mod tests {
use super::wrap_body_with_checksum_validator;
use crate::http_body_checksum::is_part_level_checksum;
use aws_smithy_checksums::ChecksumAlgorithm;
use aws_smithy_http::body::SdkBody;
use aws_smithy_http::byte_stream::ByteStream;
Expand Down Expand Up @@ -347,4 +392,27 @@ mod tests {

assert_eq!(input_text, body);
}

#[test]
fn test_is_multipart_object_checksum() {
// These ARE NOT part-level checksums
assert!(!is_part_level_checksum("abcd"));
assert!(!is_part_level_checksum("abcd="));
assert!(!is_part_level_checksum("abcd=="));
assert!(!is_part_level_checksum("1234"));
assert!(!is_part_level_checksum("1234="));
assert!(!is_part_level_checksum("1234=="));
// These ARE part-level checksums
assert!(is_part_level_checksum("abcd-1"));
assert!(is_part_level_checksum("abcd=-12"));
assert!(is_part_level_checksum("abcd12-134"));
assert!(is_part_level_checksum("abcd==-10000"));
// These are gibberish and shouldn't be regarded as a part-level checksum
assert!(!is_part_level_checksum(""));
assert!(!is_part_level_checksum("Spaces? In my header values?"));
assert!(!is_part_level_checksum("abcd==-134!#{!#"));
assert!(!is_part_level_checksum("abcd==-"));
assert!(!is_part_level_checksum("abcd==--11"));
assert!(!is_part_level_checksum("abcd==-AA"));
}
}
13 changes: 8 additions & 5 deletions aws/rust-runtime/aws-sig-auth/src/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
use crate::middleware::Signature;
use aws_credential_types::Credentials;
use aws_sigv4::http_request::{
sign, PayloadChecksumKind, PercentEncodingMode, SignableRequest, SignatureLocation,
SigningParams, SigningSettings, UriPathNormalizationMode,
sign, PayloadChecksumKind, PercentEncodingMode, SessionTokenMode, SignableRequest,
SignatureLocation, SigningParams, SigningSettings, UriPathNormalizationMode,
};
use aws_smithy_http::body::SdkBody;
use aws_types::region::SigningRegion;
Expand Down Expand Up @@ -63,6 +63,7 @@ impl OperationSigningConfig {
double_uri_encode: true,
content_sha256_header: false,
normalize_uri_path: true,
omit_session_token: false,
},
signing_requirements: SigningRequirements::Required,
expires_in: None,
Expand Down Expand Up @@ -90,10 +91,7 @@ pub struct SigningOptions {
pub double_uri_encode: bool,
pub content_sha256_header: bool,
pub normalize_uri_path: bool,
/*
Currently unsupported:
pub omit_session_token: bool,
*/
}

/// Signing Configuration for an individual Request
Expand Down Expand Up @@ -145,6 +143,11 @@ impl SigV4Signer {
} else {
UriPathNormalizationMode::Disabled
};
settings.session_token_mode = if operation_config.signing_options.omit_session_token {
SessionTokenMode::Exclude
} else {
SessionTokenMode::Include
};
settings.signature_location = match operation_config.signature_type {
HttpSignatureType::HttpRequestHeaders => SignatureLocation::Headers,
HttpSignatureType::HttpRequestQueryParams => SignatureLocation::QueryParams,
Expand Down
54 changes: 51 additions & 3 deletions aws/rust-runtime/aws-sigv4/src/http_request/canonical_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use crate::date_time::{format_date, format_date_time};
use crate::http_request::error::CanonicalRequestError;
use crate::http_request::settings::SessionTokenMode;
use crate::http_request::settings::UriPathNormalizationMode;
use crate::http_request::sign::SignableRequest;
use crate::http_request::uri_path_normalization::normalize_uri_path;
Expand Down Expand Up @@ -122,6 +123,8 @@ impl<'a> CanonicalRequest<'a> {
/// - If `settings.percent_encoding_mode` specifies double encoding, `%` in the URL will be re-encoded as `%25`
/// - If `settings.payload_checksum_kind` is XAmzSha256, add a x-amz-content-sha256 with the body
/// checksum. This is the same checksum used as the "payload_hash" in the canonical request
/// - If `settings.session_token_mode` specifies X-Amz-Security-Token to be
/// included before calculating the signature, add it, otherwise omit it.
/// - `settings.signature_location` determines where the signature will be placed in a request,
/// and also alters the kinds of signing values that go along with it in the request.
pub(super) fn from<'b>(
Expand All @@ -146,11 +149,17 @@ impl<'a> CanonicalRequest<'a> {
let (signed_headers, canonical_headers) =
Self::headers(req, params, &payload_hash, &date_time)?;
let signed_headers = SignedHeaders::new(signed_headers);

let security_token = match params.settings.session_token_mode {
SessionTokenMode::Include => params.security_token,
SessionTokenMode::Exclude => None,
};

let values = match params.settings.signature_location {
SignatureLocation::Headers => SignatureValues::Headers(HeaderValues {
content_sha256: payload_hash,
date_time,
security_token: params.security_token,
security_token,
signed_headers,
}),
SignatureLocation::QueryParams => SignatureValues::QueryParams(QueryParamValues {
Expand All @@ -170,10 +179,11 @@ impl<'a> CanonicalRequest<'a> {
.expect("presigning requires expires_in")
.as_secs()
.to_string(),
security_token: params.security_token,
security_token,
signed_headers,
}),
};

let creq = CanonicalRequest {
method: req.method(),
path,
Expand Down Expand Up @@ -232,6 +242,12 @@ impl<'a> CanonicalRequest<'a> {
}
}

if params.settings.session_token_mode == SessionTokenMode::Exclude
&& name == HeaderName::from_static(header::X_AMZ_SECURITY_TOKEN)
{
continue;
}

if params.settings.signature_location == SignatureLocation::QueryParams {
// The X-Amz-User-Agent header should not be signed if this is for a presigned URL
if name == HeaderName::from_static(header::X_AMZ_USER_AGENT) {
Expand All @@ -240,6 +256,7 @@ impl<'a> CanonicalRequest<'a> {
}
signed_headers.push(CanonicalHeaderName(name.clone()));
}

Ok((signed_headers, canonical_headers))
}

Expand Down Expand Up @@ -280,6 +297,7 @@ impl<'a> CanonicalRequest<'a> {
param::X_AMZ_SIGNED_HEADERS,
values.signed_headers.as_str(),
);

if let Some(security_token) = values.security_token {
add_param(&mut params, param::X_AMZ_SECURITY_TOKEN, security_token);
}
Expand Down Expand Up @@ -521,7 +539,7 @@ mod tests {
};
use crate::http_request::test::{test_canonical_request, test_request, test_sts};
use crate::http_request::{
PayloadChecksumKind, SignableBody, SignableRequest, SigningSettings,
PayloadChecksumKind, SessionTokenMode, SignableBody, SignableRequest, SigningSettings,
};
use crate::http_request::{SignatureLocation, SigningParams};
use crate::sign::sha256_hex_string;
Expand Down Expand Up @@ -726,6 +744,36 @@ mod tests {
assert_eq!(expected, actual);
}

#[test]
fn test_omit_session_token() {
let req = test_request("get-vanilla-query-order-key-case");
let req = SignableRequest::from(&req);
let settings = SigningSettings {
session_token_mode: SessionTokenMode::Include,
..Default::default()
};
let mut signing_params = signing_params(settings);
signing_params.security_token = Some("notarealsessiontoken");

let creq = CanonicalRequest::from(&req, &signing_params).unwrap();
assert_eq!(
creq.values.signed_headers().as_str(),
"host;x-amz-date;x-amz-security-token"
);
assert_eq!(
creq.headers.get("x-amz-security-token").unwrap(),
"notarealsessiontoken"
);

signing_params.settings.session_token_mode = SessionTokenMode::Exclude;
let creq = CanonicalRequest::from(&req, &signing_params).unwrap();
assert_eq!(
creq.headers.get("x-amz-security-token").unwrap(),
"notarealsessiontoken"
);
assert_eq!(creq.values.signed_headers().as_str(), "host;x-amz-date");
}

// It should exclude user-agent and x-amz-user-agent headers from presigning
#[test]
fn presigning_header_exclusion() {
Expand Down
Loading

0 comments on commit c271761

Please sign in to comment.