diff --git a/crates/s3s/src/ops/mod.rs b/crates/s3s/src/ops/mod.rs index d002cbca..0eee44a5 100644 --- a/crates/s3s/src/ops/mod.rs +++ b/crates/s3s/src/ops/mod.rs @@ -364,7 +364,7 @@ async fn prepare(req: &mut Request, ccx: &CallContext<'_>) -> S3Result qs: req.s3ext.qs.as_ref(), hs, - decoded_uri_path, + raw_uri_path: req.uri.path(), vh_bucket, content_length, diff --git a/crates/s3s/src/ops/signature.rs b/crates/s3s/src/ops/signature.rs index 57009172..03872bed 100644 --- a/crates/s3s/src/ops/signature.rs +++ b/crates/s3s/src/ops/signature.rs @@ -80,7 +80,7 @@ pub struct SignatureContext<'a> { pub qs: Option<&'a OrderedQs>, pub hs: OrderedHeaders<'a>, - pub decoded_uri_path: String, + pub raw_uri_path: &'a str, pub vh_bucket: Option<&'a str>, pub content_length: Option, @@ -310,14 +310,20 @@ impl SignatureContext<'_> { }); let method = &self.req_method; - let uri_path = &self.decoded_uri_path; + let raw_uri_path = self.raw_uri_path; let payload = match headers.get_unique(crate::header::X_AMZ_CONTENT_SHA256) { Some(content_sha256) => sig_v4::Payload::SingleChunk(content_sha256), None => sig_v4::Payload::Unsigned, }; - let canonical_request = sig_v4::create_presigned_canonical_request(method, uri_path, qs.as_ref(), &headers, payload); + let canonical_request = sig_v4::create_presigned_canonical_request_with_raw_uri_path( + method, + raw_uri_path, + qs.as_ref(), + &headers, + payload, + ); let amz_date = &presigned_url.amz_date; let string_to_sign = sig_v4::create_string_to_sign(&canonical_request, amz_date, region, service); @@ -393,7 +399,7 @@ impl SignatureContext<'_> { let signature = { let method = &self.req_method; - let uri_path = &self.decoded_uri_path; + let raw_uri_path = self.raw_uri_path; let query_strings: &[(String, String)] = self.qs.as_ref().map_or(&[], AsRef::as_ref); // FIXME: throw error if any signed header is not in the request @@ -414,29 +420,39 @@ impl SignatureContext<'_> { }); let canonical_request = match amz_content_sha256 { - Some(AmzContentSha256::StreamingAws4HmacSha256Payload) => { - sig_v4::create_canonical_request(method, uri_path, query_strings, &headers, sig_v4::Payload::MultipleChunks) - } - Some(AmzContentSha256::StreamingAws4HmacSha256PayloadTrailer) => sig_v4::create_canonical_request( + Some(AmzContentSha256::StreamingAws4HmacSha256Payload) => sig_v4::create_canonical_request_with_raw_uri_path( method, - uri_path, + raw_uri_path, query_strings, &headers, - sig_v4::Payload::MultipleChunksWithTrailer, + sig_v4::Payload::MultipleChunks, ), - Some(AmzContentSha256::UnsignedPayload) => { - sig_v4::create_canonical_request(method, uri_path, query_strings, &headers, sig_v4::Payload::Unsigned) + Some(AmzContentSha256::StreamingAws4HmacSha256PayloadTrailer) => { + sig_v4::create_canonical_request_with_raw_uri_path( + method, + raw_uri_path, + query_strings, + &headers, + sig_v4::Payload::MultipleChunksWithTrailer, + ) } - Some(AmzContentSha256::StreamingUnsignedPayloadTrailer) => sig_v4::create_canonical_request( + Some(AmzContentSha256::UnsignedPayload) => sig_v4::create_canonical_request_with_raw_uri_path( method, - uri_path, + raw_uri_path, + query_strings, + &headers, + sig_v4::Payload::Unsigned, + ), + Some(AmzContentSha256::StreamingUnsignedPayloadTrailer) => sig_v4::create_canonical_request_with_raw_uri_path( + method, + raw_uri_path, query_strings, &headers, sig_v4::Payload::UnsignedMultipleChunksWithTrailer, ), - Some(AmzContentSha256::SingleChunk(payload_checksum)) => sig_v4::create_canonical_request( + Some(AmzContentSha256::SingleChunk(payload_checksum)) => sig_v4::create_canonical_request_with_raw_uri_path( method, - uri_path, + raw_uri_path, query_strings, &headers, sig_v4::Payload::SingleChunk(payload_checksum), @@ -463,9 +479,9 @@ impl SignatureContext<'_> { let hash = hex_sha256(&body_bytes, str::to_owned); // Create canonical request with the computed hash - sig_v4::create_canonical_request( + sig_v4::create_canonical_request_with_raw_uri_path( method, - uri_path, + raw_uri_path, query_strings, &headers, sig_v4::Payload::SingleChunk(&hash), @@ -752,7 +768,7 @@ file content\r\n\ req_body: &mut body, qs: None, hs: OrderedHeaders::from_slice_unchecked(&[]), - decoded_uri_path: "/test-bucket".to_owned(), + raw_uri_path: "/test-bucket", vh_bucket: None, content_length: None, mime: Some(mime), @@ -868,7 +884,7 @@ file content\r\n\ req_body: &mut body, qs: Some(&qs), hs: OrderedHeaders::from_slice_unchecked(&[]), - decoded_uri_path: "/test.txt".to_owned(), + raw_uri_path: "/test.txt", vh_bucket: None, content_length: None, mime: None, @@ -939,7 +955,7 @@ file content\r\n\ req_body: &mut body, qs: Some(&qs), hs: headers, - decoded_uri_path: "/test.txt".to_owned(), + raw_uri_path: "/test.txt", vh_bucket: None, content_length: Some(0), mime: None, @@ -957,6 +973,68 @@ file content\r\n\ assert_eq!(err.status_code(), Some(hyper::StatusCode::BAD_REQUEST)); } + #[tokio::test] + async fn v4_header_auth_uses_raw_uri_path_for_canonical_request() { + let access_key = "AKIAIOSFODNN7EXAMPLE"; + let secret_key: SecretKey = "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY".into(); + let auth = SimpleAuth::from_single(access_key, secret_key.clone()); + let config: Arc = Arc::new(StaticConfigProvider::default()); + + let method = Method::GET; + let uri = Uri::from_static("https://s3.amazonaws.com/test-bucket/path/sitemap.xmlage="); + let raw_uri_path = "/test-bucket/path/sitemap.xmlage="; + let amz_date = AmzDate::parse("20130524T000000Z").unwrap(); + let headers_for_signing = OrderedHeaders::from_slice_unchecked(&[ + ("host", "s3.amazonaws.com"), + ("x-amz-content-sha256", "UNSIGNED-PAYLOAD"), + ("x-amz-date", "20130524T000000Z"), + ]); + let canonical_request = sig_v4::create_canonical_request_with_raw_uri_path( + &method, + raw_uri_path, + &[] as &[(&str, &str)], + &headers_for_signing, + sig_v4::Payload::Unsigned, + ); + let string_to_sign = sig_v4::create_string_to_sign(&canonical_request, &amz_date, "us-east-1", "s3"); + let signature = sig_v4::calculate_signature(&string_to_sign, &secret_key, &amz_date, "us-east-1", "s3"); + let authorization = format!( + "AWS4-HMAC-SHA256 Credential={access_key}/20130524/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature={signature}" + ); + let headers = OrderedHeaders::from_slice_unchecked(&[ + ("authorization", authorization.as_str()), + ("host", "s3.amazonaws.com"), + ("x-amz-content-sha256", "UNSIGNED-PAYLOAD"), + ("x-amz-date", "20130524T000000Z"), + ]); + + let mut body = Body::empty(); + let mut cx = SignatureContext { + auth: Some(&auth), + config: &config, + req_version: ::http::Version::HTTP_11, + req_method: &method, + req_uri: &uri, + req_body: &mut body, + qs: None, + hs: headers, + raw_uri_path, + vh_bucket: None, + content_length: Some(0), + mime: None, + decoded_content_length: None, + transformed_body: None, + multipart: None, + trailing_headers: None, + }; + + let cred = cx + .v4_check_header_auth() + .await + .expect("valid SigV4 auth with a raw '=' URI path should succeed"); + assert_eq!(cred.access_key, access_key); + } + /// `SigV2` does not carry region in the credential scope, so `CredentialsExt.region` /// must always be `None` and `service` must always be `Some("s3")`. /// @@ -1004,7 +1082,7 @@ file content\r\n\ req_body: &mut body, qs: None, hs, - decoded_uri_path: "/test-bucket/test-key".to_owned(), + raw_uri_path: "/test-bucket/test-key", vh_bucket: None, content_length: None, mime: None, diff --git a/crates/s3s/src/ops/tests.rs b/crates/s3s/src/ops/tests.rs index 92a11de6..58d42d42 100644 --- a/crates/s3s/src/ops/tests.rs +++ b/crates/s3s/src/ops/tests.rs @@ -392,7 +392,7 @@ async fn presigned_url_expires_0_should_be_expired() { req_body: &mut body, qs: Some(&qs), hs: OrderedHeaders::from_slice_unchecked(&[]), - decoded_uri_path: "/test.txt".to_owned(), + raw_uri_path: "/test.txt", vh_bucket: None, content_length: None, mime: None, diff --git a/crates/s3s/src/sig_v4/methods.rs b/crates/s3s/src/sig_v4/methods.rs index 89a66868..2d137a0d 100644 --- a/crates/s3s/src/sig_v4/methods.rs +++ b/crates/s3s/src/sig_v4/methods.rs @@ -125,14 +125,13 @@ impl Payload<'_> { } } -/// create canonical request -#[must_use] -pub fn create_canonical_request( +fn create_canonical_request_with_canonical_uri( method: &Method, - uri_path: &str, + canonical_uri: &str, decoded_query_strings: &[(impl AsRef, impl AsRef)], signed_headers: &OrderedHeaders<'_>, payload: Payload<'_>, + skip_signature_query: bool, ) -> String { let mut ans = String::with_capacity(256); @@ -144,7 +143,7 @@ pub fn create_canonical_request( { // \n - uri_encode(&mut ans, uri_path, false); + ans.push_str(canonical_uri); ans.push('\n'); } @@ -153,6 +152,9 @@ pub fn create_canonical_request( let encoded_query_strings = { let mut qs = >::new(); for (n, v) in decoded_query_strings { + if skip_signature_query && is_skipped_query_string(n.as_ref()) { + continue; + } let name = uri_encode_string(n.as_ref(), true); let value = uri_encode_string(v.as_ref(), true); qs.push((name, value)); @@ -250,6 +252,30 @@ pub fn create_canonical_request( ans } +/// create canonical request +#[cfg(test)] +#[must_use] +pub fn create_canonical_request( + method: &Method, + uri_path: &str, + decoded_query_strings: &[(impl AsRef, impl AsRef)], + signed_headers: &OrderedHeaders<'_>, + payload: Payload<'_>, +) -> String { + let canonical_uri = uri_encode_string(uri_path, false); + create_canonical_request_with_canonical_uri(method, &canonical_uri, decoded_query_strings, signed_headers, payload, false) +} + +pub(crate) fn create_canonical_request_with_raw_uri_path( + method: &Method, + raw_uri_path: &str, + decoded_query_strings: &[(impl AsRef, impl AsRef)], + signed_headers: &OrderedHeaders<'_>, + payload: Payload<'_>, +) -> String { + create_canonical_request_with_canonical_uri(method, raw_uri_path, decoded_query_strings, signed_headers, payload, false) +} + /// create string to sign #[must_use] pub fn create_string_to_sign(canonical_request: &str, amz_date: &AmzDate, region: &str, service: &str) -> String { @@ -403,6 +429,7 @@ pub fn calculate_signature( } /// create presigned canonical request +#[cfg(test)] pub fn create_presigned_canonical_request( method: &Method, uri_path: &str, @@ -410,114 +437,18 @@ pub fn create_presigned_canonical_request( signed_headers: &OrderedHeaders<'_>, payload: Payload<'_>, ) -> String { - let mut ans = String::with_capacity(256); - { - // \n - ans.push_str(method.as_str()); - ans.push('\n'); - } - { - // \n - uri_encode(&mut ans, uri_path, false); - ans.push('\n'); - } - { - // \n - let encoded_query_strings = { - let mut qs = >::new(); - for (n, v) in decoded_query_strings { - if is_skipped_query_string(n.as_ref()) { - continue; - } - let name = uri_encode_string(n.as_ref(), true); - let value = uri_encode_string(v.as_ref(), true); - qs.push((name, value)); - } - stable_sort_by_first(&mut qs); - qs - }; - - if let Some((first, remain)) = encoded_query_strings.split_first() { - { - let (name, value) = first; - ans.push_str(name); - ans.push('='); - ans.push_str(value); - } - for (name, value) in remain { - ans.push('&'); - ans.push_str(name); - ans.push('='); - ans.push_str(value); - } - } - - ans.push('\n'); - } - { - // \n - // According to AWS SigV4 spec, multiple headers with the same name should be - // combined into a single header with values separated by commas. - - let headers_slice = signed_headers.as_ref(); - let mut i = 0; - while i < headers_slice.len() { - let (name, value) = headers_slice[i]; - if is_skipped_header(name) { - i += 1; - continue; - } - - ans.push_str(name); - ans.push(':'); - normalize_header_value(&mut ans, value); - - // Combine values for headers with the same name (comma-separated) - let mut j = i + 1; - while j < headers_slice.len() && headers_slice[j].0 == name { - ans.push(','); - normalize_header_value(&mut ans, headers_slice[j].1); - j += 1; - } - - ans.push('\n'); - i = j; - } - ans.push('\n'); - } - { - // \n - // Each header name should only appear once, even if the header has multiple values - let headers_slice = signed_headers.as_ref(); - let mut first_flag = true; - let mut i = 0; - while i < headers_slice.len() { - let (name, _) = headers_slice[i]; - if is_skipped_header(name) { - i += 1; - continue; - } - - if first_flag { - first_flag = false; - } else { - ans.push(';'); - } - ans.push_str(name); - - // Skip duplicate header names - while i < headers_slice.len() && headers_slice[i].0 == name { - i += 1; - } - } + let canonical_uri = uri_encode_string(uri_path, false); + create_canonical_request_with_canonical_uri(method, &canonical_uri, decoded_query_strings, signed_headers, payload, true) +} - ans.push('\n'); - } - { - // - append_payload(&mut ans, payload); - } - ans +pub(crate) fn create_presigned_canonical_request_with_raw_uri_path( + method: &Method, + raw_uri_path: &str, + decoded_query_strings: &[(impl AsRef, impl AsRef)], + signed_headers: &OrderedHeaders<'_>, + payload: Payload<'_>, +) -> String { + create_canonical_request_with_canonical_uri(method, raw_uri_path, decoded_query_strings, signed_headers, payload, true) } #[cfg(test)]