From c783194e8b8c3263fad579a85ea07d62e63b16be Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Tue, 9 May 2023 20:49:34 +0100 Subject: *: apply clippy recommendations. --- src/api/admin/api_server.rs | 2 +- src/api/admin/bucket.rs | 4 ++-- src/api/admin/key.rs | 4 ++-- src/api/generic_server.rs | 2 +- src/api/k2v/batch.rs | 4 ++-- src/api/s3/get.rs | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) (limited to 'src/api') diff --git a/src/api/admin/api_server.rs b/src/api/admin/api_server.rs index 58dd38d8..b0dfdfb7 100644 --- a/src/api/admin/api_server.rs +++ b/src/api/admin/api_server.rs @@ -105,7 +105,7 @@ impl AdminApiServer { let bucket_id = self .garage .bucket_helper() - .resolve_global_bucket_name(&domain) + .resolve_global_bucket_name(domain) .await? .ok_or(HelperError::NoSuchBucket(domain.to_string()))?; diff --git a/src/api/admin/bucket.rs b/src/api/admin/bucket.rs index e60f07ca..f0a4a9e7 100644 --- a/src/api/admin/bucket.rs +++ b/src/api/admin/bucket.rs @@ -183,8 +183,8 @@ async fn bucket_info_results( } }), keys: relevant_keys - .into_iter() - .map(|(_, key)| { + .into_values() + .map(|key| { let p = key.state.as_option().unwrap(); GetBucketInfoKey { access_key_id: key.key_id, diff --git a/src/api/admin/key.rs b/src/api/admin/key.rs index 2bbabb7b..d74ca361 100644 --- a/src/api/admin/key.rs +++ b/src/api/admin/key.rs @@ -183,8 +183,8 @@ async fn key_info_results(garage: &Arc, key: Key) -> Result ApiServer { let uri = req.uri().clone(); if let Ok(forwarded_for_ip_addr) = - forwarded_headers::handle_forwarded_for_headers(&req.headers()) + forwarded_headers::handle_forwarded_for_headers(req.headers()) { info!( "{} (via {}) {} {}", diff --git a/src/api/k2v/batch.rs b/src/api/k2v/batch.rs index 26d678da..294380ea 100644 --- a/src/api/k2v/batch.rs +++ b/src/api/k2v/batch.rs @@ -282,8 +282,8 @@ pub(crate) async fn handle_poll_range( if let Some((items, seen_marker)) = resp { let resp = PollRangeResponse { items: items - .into_iter() - .map(|(_k, i)| ReadBatchResponseItem::from(i)) + .into_values() + .map(ReadBatchResponseItem::from) .collect::>(), seen_marker, }; diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs index 2a99551a..cde7b461 100644 --- a/src/api/s3/get.rs +++ b/src/api/s3/get.rs @@ -443,7 +443,7 @@ fn body_from_blocks_range( // block.part_number, which is not the same in the case of a multipart upload) let mut blocks: Vec<(VersionBlock, u64)> = Vec::with_capacity(std::cmp::min( all_blocks.len(), - 4 + ((end - begin) / std::cmp::max(all_blocks[0].1.size as u64, 1024)) as usize, + 4 + ((end - begin) / std::cmp::max(all_blocks[0].1.size, 1024)) as usize, )); let mut block_offset: u64 = 0; for (_, b) in all_blocks.iter() { @@ -454,7 +454,7 @@ fn body_from_blocks_range( if block_offset < end && block_offset + b.size > begin { blocks.push((*b, block_offset)); } - block_offset += b.size as u64; + block_offset += b.size; } let order_stream = OrderTag::stream(); -- cgit v1.2.3 From 746b0090e4ad02e98b08aa1cfefd77a22d9490fe Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Thu, 18 May 2023 00:06:03 +0200 Subject: k2v signature verification: double urlencoding (see comment in source code) --- src/api/signature/payload.rs | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) (limited to 'src/api') diff --git a/src/api/signature/payload.rs b/src/api/signature/payload.rs index 4c7934e5..c945ab11 100644 --- a/src/api/signature/payload.rs +++ b/src/api/signature/payload.rs @@ -19,7 +19,7 @@ use crate::signature::error::*; pub async fn check_payload_signature( garage: &Garage, - service: &str, + service: &'static str, request: &Request, ) -> Result<(Option, Option), Error> { let mut headers = HashMap::new(); @@ -51,6 +51,7 @@ pub async fn check_payload_signature( }; let canonical_request = canonical_request( + service, request.method(), request.uri(), &headers, @@ -231,15 +232,50 @@ pub fn string_to_sign(datetime: &DateTime, scope_string: &str, canonical_re } pub fn canonical_request( + service: &'static str, method: &Method, uri: &hyper::Uri, headers: &HashMap, signed_headers: &str, content_sha256: &str, ) -> String { + // There seems to be evidence that in AWSv4 signatures, the path component is url-encoded + // a second time when building the canonical request, as specified in this documentation page: + // -> https://docs.aws.amazon.com/rolesanywhere/latest/userguide/authentication-sign-process.html + // However this documentation page is for a specific service ("roles anywhere"), and + // in the S3 service we know for a fact that there is no double-urlencoding, because all of + // the tests we made with external software work without it. + // + // The theory is that double-urlencoding occurs for all services except S3, + // which is what is implemented in rusoto_signature: + // -> https://docs.rs/rusoto_signature/latest/src/rusoto_signature/signature.rs.html#464 + // + // Digging into the code of the official AWS Rust SDK, we learn that double-URI-encoding can + // be set or unset on a per-request basis (the signature crates, aws-sigv4 and aws-sig-auth, + // are agnostic to this). Grepping the codebase confirms that S3 is the only API for which + // double_uri_encode is set to false, meaning it is true (its default value) for all other + // AWS services. We will therefore implement this behavior in Garage as well. + // + // Note that this documentation page, which is touted as the "authoritative reference" on + // AWSv4 signatures, makes no mention of either single- or double-urlencoding: + // -> https://docs.aws.amazon.com/IAM/latest/UserGuide/create-signed-request.html + // This page of the S3 documentation does also not mention anything specific: + // -> https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html + // + // Note that there is also the issue of path normalization, which I hope is unrelated to the + // one of URI-encoding. At least in aws-sigv4 both parameters can be set independently, + // and rusoto_signature does not seem to do any effective path normalization, even though + // it mentions it in the comments (same link to the souce code as above). + // We make the explicit choice of NOT normalizing paths in the K2V API because doing so + // would make non-normalized paths invalid K2V partition keys, and we don't want that. + let path: std::borrow::Cow = if service != "s3" { + uri_encode(uri.path(), false).into() + } else { + uri.path().into() + }; [ method.as_str(), - uri.path(), + &path, &canonical_query_string(uri), &canonical_header_string(headers, signed_headers), "", -- cgit v1.2.3 From 180992d0f1b8d5731b0bf8844129f5686541bc86 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Tue, 6 Jun 2023 16:25:29 +0100 Subject: payload.rs: Fixed typo in error message. --- src/api/signature/payload.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/api') diff --git a/src/api/signature/payload.rs b/src/api/signature/payload.rs index c945ab11..1e3fa207 100644 --- a/src/api/signature/payload.rs +++ b/src/api/signature/payload.rs @@ -185,7 +185,7 @@ fn parse_query_authorization( if duration > 7 * 24 * 3600 { return Err(Error::bad_request( - "X-Amz-Exprires may not exceed a week".to_string(), + "X-Amz-Expires may not exceed a week".to_string(), )); } -- cgit v1.2.3 From 8a945ee9963021594378095c4b949bf15e816439 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Tue, 6 Jun 2023 16:26:06 +0100 Subject: payload.rs: Surround / in inverted commas. --- src/api/signature/payload.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/api') diff --git a/src/api/signature/payload.rs b/src/api/signature/payload.rs index 1e3fa207..b50fb3bb 100644 --- a/src/api/signature/payload.rs +++ b/src/api/signature/payload.rs @@ -211,7 +211,7 @@ fn parse_query_authorization( fn parse_credential(cred: &str) -> Result<(String, String), Error> { let first_slash = cred .find('/') - .ok_or_bad_request("Credentials does not contain / in authorization field")?; + .ok_or_bad_request("Credentials does not contain '/' in authorization field")?; let (key_id, scope) = cred.split_at(first_slash); Ok(( key_id.to_string(), -- cgit v1.2.3