diff --git a/Cargo.lock b/Cargo.lock index a1be0ae2..015e1df3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1548,6 +1548,7 @@ dependencies = [ "md-5", "nom", "opentelemetry", + "percent-encoding", "pin-project", "quick-xml", "serde", diff --git a/src/api/common/Cargo.toml b/src/api/common/Cargo.toml index 8e9d074f..5e58577e 100644 --- a/src/api/common/Cargo.toml +++ b/src/api/common/Cargo.toml @@ -27,6 +27,7 @@ thiserror.workspace = true hex.workspace = true hmac.workspace = true md-5.workspace = true +percent-encoding.workspace = true tracing.workspace = true nom.workspace = true pin-project.workspace = true diff --git a/src/api/common/signature/error.rs b/src/api/common/signature/error.rs index a1b353e1..8d86f042 100644 --- a/src/api/common/signature/error.rs +++ b/src/api/common/signature/error.rs @@ -11,8 +11,13 @@ pub enum Error { Common(CommonError), /// Authorization Header Malformed - #[error("Authorization header malformed, unexpected scope: {0}")] - AuthorizationHeaderMalformed(String), + #[error( + "Authorization header malformed, unexpected scope: '{unexpected}', expected: '{expected}'" + )] + AuthorizationHeaderMalformed { + unexpected: String, + expected: String, + }, // Category: bad request /// The request contained an invalid UTF-8 sequence in its path or in other parameters diff --git a/src/api/common/signature/payload.rs b/src/api/common/signature/payload.rs index ec4558e6..532fa04b 100644 --- a/src/api/common/signature/payload.rs +++ b/src/api/common/signature/payload.rs @@ -340,7 +340,11 @@ pub fn canonical_request( let canonical_uri: std::borrow::Cow = if service != "s3" { uri_encode(canonical_uri, false).into() } else { - canonical_uri.into() + //TODO: decode is already do for construct Api::EndPoint, should be better to be able to keep it instead of compute it again. + let key = percent_encoding::percent_decode_str(canonical_uri) + .decode_utf8() + .unwrap(); + uri_encode(&key, false).into() }; // Canonical query string from passed HeaderMap @@ -400,7 +404,10 @@ pub fn verify_v4( ) -> Result { let scope_expected = compute_scope(&auth.date, &garage.config.s3_api.s3_region, service); if auth.scope != scope_expected { - return Err(Error::AuthorizationHeaderMalformed(auth.scope.to_string())); + return Err(Error::AuthorizationHeaderMalformed { + unexpected: auth.scope.to_string(), + expected: scope_expected, + }); } let key = garage diff --git a/src/api/k2v/error.rs b/src/api/k2v/error.rs index 7ce3b073..797eb868 100644 --- a/src/api/k2v/error.rs +++ b/src/api/k2v/error.rs @@ -20,8 +20,13 @@ pub enum Error { // Category: cannot process /// Authorization Header Malformed - #[error("Authorization header malformed, unexpected scope: {0}")] - AuthorizationHeaderMalformed(String), + #[error( + "Authorization header malformed, unexpected scope: '{unexpected}', expected: '{expected}'" + )] + AuthorizationHeaderMalformed { + unexpected: String, + expected: String, + }, /// The provided digest (checksum) value was invalid #[error("Invalid digest: {0}")] @@ -54,9 +59,13 @@ impl From for Error { fn from(err: SignatureError) -> Self { match err { SignatureError::Common(c) => Self::Common(c), - SignatureError::AuthorizationHeaderMalformed(c) => { - Self::AuthorizationHeaderMalformed(c) - } + SignatureError::AuthorizationHeaderMalformed { + unexpected, + expected, + } => Self::AuthorizationHeaderMalformed { + unexpected, + expected, + }, SignatureError::InvalidUtf8Str(i) => Self::InvalidUtf8Str(i), SignatureError::InvalidDigest(d) => Self::InvalidDigest(d), } @@ -72,7 +81,7 @@ impl Error { Error::Common(c) => c.aws_code(), Error::NoSuchKey => "NoSuchKey", Error::NotAcceptable(_) => "NotAcceptable", - Error::AuthorizationHeaderMalformed(_) => "AuthorizationHeaderMalformed", + Error::AuthorizationHeaderMalformed { .. } => "AuthorizationHeaderMalformed", Error::InvalidBase64(_) => "InvalidBase64", Error::InvalidUtf8Str(_) => "InvalidUtf8String", Error::InvalidCausalityToken => "CausalityToken", @@ -88,7 +97,7 @@ impl ApiError for Error { Error::Common(c) => c.http_status_code(), Error::NoSuchKey => StatusCode::NOT_FOUND, Error::NotAcceptable(_) => StatusCode::NOT_ACCEPTABLE, - Error::AuthorizationHeaderMalformed(_) + Error::AuthorizationHeaderMalformed { .. } | Error::InvalidBase64(_) | Error::InvalidUtf8Str(_) | Error::InvalidDigest(_) diff --git a/src/api/s3/error.rs b/src/api/s3/error.rs index 74ef6692..09aa0604 100644 --- a/src/api/s3/error.rs +++ b/src/api/s3/error.rs @@ -31,8 +31,13 @@ pub enum Error { // Category: cannot process /// Authorization Header Malformed - #[error("Authorization header malformed, unexpected scope: {0}")] - AuthorizationHeaderMalformed(String), + #[error( + "Authorization header malformed, unexpected scope: '{unexpected}', expected: '{expected}'" + )] + AuthorizationHeaderMalformed { + unexpected: String, + expected: String, + }, /// The object requested don't exists #[error("Key not found")] @@ -121,9 +126,13 @@ impl From for Error { fn from(err: SignatureError) -> Self { match err { SignatureError::Common(c) => Self::Common(c), - SignatureError::AuthorizationHeaderMalformed(c) => { - Self::AuthorizationHeaderMalformed(c) - } + SignatureError::AuthorizationHeaderMalformed { + unexpected, + expected, + } => Self::AuthorizationHeaderMalformed { + unexpected, + expected, + }, SignatureError::InvalidUtf8Str(i) => Self::InvalidUtf8Str(i), SignatureError::InvalidDigest(d) => Self::InvalidDigest(d), } @@ -146,7 +155,7 @@ impl Error { Error::InvalidPart => "InvalidPart", Error::InvalidPartOrder => "InvalidPartOrder", Error::EntityTooSmall => "EntityTooSmall", - Error::AuthorizationHeaderMalformed(_) => "AuthorizationHeaderMalformed", + Error::AuthorizationHeaderMalformed { .. } => "AuthorizationHeaderMalformed", Error::InvalidXml(_) => "MalformedXML", Error::InvalidXmlDe(_) => "MalformedXML", Error::InvalidXmlSe(_) => "InternalError", @@ -172,7 +181,7 @@ impl ApiError for Error { Error::PreconditionFailed => StatusCode::PRECONDITION_FAILED, Error::InvalidRange(_) => StatusCode::RANGE_NOT_SATISFIABLE, Error::InvalidXmlSe(_) => StatusCode::INTERNAL_SERVER_ERROR, - Error::AuthorizationHeaderMalformed(_) + Error::AuthorizationHeaderMalformed { .. } | Error::InvalidPart | Error::InvalidPartOrder | Error::EntityTooSmall diff --git a/src/garage/tests/s3/mod.rs b/src/garage/tests/s3/mod.rs index e75b1397..fa081389 100644 --- a/src/garage/tests/s3/mod.rs +++ b/src/garage/tests/s3/mod.rs @@ -2,6 +2,7 @@ mod list; mod multipart; mod objects; mod presigned; +mod signature_encoding; mod simple; mod ssec; mod streaming_signature; diff --git a/src/garage/tests/s3/signature_encoding.rs b/src/garage/tests/s3/signature_encoding.rs new file mode 100644 index 00000000..6fbb4078 --- /dev/null +++ b/src/garage/tests/s3/signature_encoding.rs @@ -0,0 +1,72 @@ +use crate::common; + +use aws_sdk_s3::presigning::PresigningConfig; +use bytes::Bytes; +use http::{Request, StatusCode}; +use http_body_util::Full; +use std::time::Duration; + +#[tokio::test] +async fn test_signature_encoding() { + let ctx = common::context(); + let bucket = ctx.create_bucket("signature-encoding"); + + let obj_key = "key@good~.txt"; + let obj_content = "hello world of special characters"; + + let _put_obj_info = ctx + .client + .put_object() + .bucket(&bucket) + .key(obj_key) + .body(obj_content.as_bytes().to_vec().into()) + .send() + .await + .expect("failed to put object"); + + let _get_obj = ctx + .client + .get_object() + .bucket(&bucket) + .key(obj_key) + .send() + .await + .expect("failed to get object"); + + let presign_config = PresigningConfig::builder() + .expires_in(Duration::from_secs(10)) + .build() + .expect("failed to build presigning config"); + let presigned_request = ctx + .client + .get_object() + .bucket(&bucket) + .key(obj_key) + .presigned(presign_config) + .await + .expect("failed to construct presigned request"); + + let altered_url = presigned_request + .uri() + .replace("%40", "@") + .replace("~", "%7E"); + + let client = ctx.custom_request.client(); + let req_builder = Request::builder() + .method(presigned_request.method()) + .uri(altered_url); + let req = presigned_request + .headers() + .fold(req_builder, |req_builder, (key, value)| { + req_builder.header(key, value) + }) + .body(Full::new(Bytes::new())) + .expect("failed to construct request from presigned_request"); + + let res = client + .request(req) + .await + .expect("failed to execute presigned request"); + + assert_eq!(res.status(), StatusCode::OK); +}