aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorAlex Auvolat <alex@adnab.me>2024-02-28 00:27:54 +0100
committerAlex Auvolat <alex@adnab.me>2024-02-28 12:24:21 +0100
commite9f759d4cb9be28584ab511a0a2dc78b579475c8 (patch)
tree471ad303d34e992ee151227b40dd5261b85711b1 /src
parenta5e4bfeae9d705e0c8a56dfd8268e1309999c5cd (diff)
downloadgarage-e9f759d4cb9be28584ab511a0a2dc78b579475c8.tar.gz
garage-e9f759d4cb9be28584ab511a0a2dc78b579475c8.zip
[fix-presigned] presigned requests: allow x-amz-* query parameters to stand in for equivalent headers
Diffstat (limited to 'src')
-rw-r--r--src/api/k2v/api_server.rs5
-rw-r--r--src/api/s3/api_server.rs5
-rw-r--r--src/api/signature/payload.rs45
-rw-r--r--src/garage/tests/common/custom_requester.rs2
4 files changed, 44 insertions, 13 deletions
diff --git a/src/api/k2v/api_server.rs b/src/api/k2v/api_server.rs
index e97da2af..5ed7e286 100644
--- a/src/api/k2v/api_server.rs
+++ b/src/api/k2v/api_server.rs
@@ -69,7 +69,7 @@ impl ApiHandler for K2VApiServer {
async fn handle(
&self,
- req: Request<IncomingBody>,
+ mut req: Request<IncomingBody>,
endpoint: K2VApiEndpoint,
) -> Result<Response<ResBody>, Error> {
let K2VApiEndpoint {
@@ -86,7 +86,8 @@ impl ApiHandler for K2VApiServer {
return Ok(options_res.map(|_empty_body: EmptyBody| empty_body()));
}
- let (api_key, mut content_sha256) = check_payload_signature(&garage, "k2v", &req).await?;
+ let (api_key, mut content_sha256) =
+ check_payload_signature(&garage, "k2v", &mut req).await?;
let api_key = api_key
.ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?;
diff --git a/src/api/s3/api_server.rs b/src/api/s3/api_server.rs
index 08405923..fdfaf0a4 100644
--- a/src/api/s3/api_server.rs
+++ b/src/api/s3/api_server.rs
@@ -107,7 +107,7 @@ impl ApiHandler for S3ApiServer {
async fn handle(
&self,
- req: Request<IncomingBody>,
+ mut req: Request<IncomingBody>,
endpoint: S3ApiEndpoint,
) -> Result<Response<ResBody>, Error> {
let S3ApiEndpoint {
@@ -125,7 +125,8 @@ impl ApiHandler for S3ApiServer {
return Ok(options_res.map(|_empty_body: EmptyBody| empty_body()));
}
- let (api_key, mut content_sha256) = check_payload_signature(&garage, "s3", &req).await?;
+ let (api_key, mut content_sha256) =
+ check_payload_signature(&garage, "s3", &mut req).await?;
let api_key = api_key
.ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?;
diff --git a/src/api/signature/payload.rs b/src/api/signature/payload.rs
index 8a7a4341..435b4206 100644
--- a/src/api/signature/payload.rs
+++ b/src/api/signature/payload.rs
@@ -3,7 +3,7 @@ use std::convert::TryFrom;
use chrono::{DateTime, Duration, NaiveDateTime, TimeZone, Utc};
use hmac::Mac;
-use hyper::header::{HeaderMap, HeaderName, AUTHORIZATION, CONTENT_TYPE, HOST};
+use hyper::header::{HeaderMap, HeaderName, HeaderValue, AUTHORIZATION, CONTENT_TYPE, HOST};
use hyper::{body::Incoming as IncomingBody, Method, Request};
use sha2::{Digest, Sha256};
@@ -36,7 +36,7 @@ pub type QueryMap = HashMap<String, String>;
pub async fn check_payload_signature(
garage: &Garage,
service: &'static str,
- request: &Request<IncomingBody>,
+ request: &mut Request<IncomingBody>,
) -> Result<(Option<Key>, Option<Hash>), Error> {
let query = parse_query_map(request.uri())?;
@@ -96,7 +96,7 @@ async fn check_standard_signature(
request.uri().path(),
&query,
request.headers(),
- signed_headers,
+ &signed_headers,
&authorization.content_sha256,
)?;
let string_to_sign = string_to_sign(
@@ -127,7 +127,7 @@ async fn check_standard_signature(
async fn check_presigned_signature(
garage: &Garage,
service: &'static str,
- request: &Request<IncomingBody>,
+ request: &mut Request<IncomingBody>,
mut query: QueryMap,
) -> Result<(Option<Key>, Option<Hash>), Error> {
let algorithm = query.get(X_AMZ_ALGORITHM.as_str()).unwrap();
@@ -163,7 +163,7 @@ async fn check_presigned_signature(
request.uri().path(),
&query,
request.headers(),
- signed_headers,
+ &signed_headers,
&authorization.content_sha256,
)?;
let string_to_sign = string_to_sign(
@@ -177,6 +177,35 @@ async fn check_presigned_signature(
let key = verify_v4(garage, service, &authorization, string_to_sign.as_bytes()).await?;
+ // AWS specifies that if a signed query parameter and a signed header of the same name
+ // have different values, then an InvalidRequest error is raised.
+ let headers_mut = request.headers_mut();
+ for (name, value) in query.iter() {
+ let name =
+ HeaderName::from_bytes(name.as_bytes()).ok_or_bad_request("Invalid header name")?;
+ if let Some(existing) = headers_mut.get(&name) {
+ if signed_headers.contains(&name) && existing.as_bytes() != value.as_bytes() {
+ return Err(Error::bad_request(format!(
+ "Conflicting values for `{}` in query parameters and request headers",
+ name
+ )));
+ }
+ }
+ if name.as_str().starts_with("x-amz-") {
+ // Query parameters that start by x-amz- are actually intended to stand in for
+ // headers that can't be added at the time the request is made.
+ // What we do is just add them to the Request object as regular headers,
+ // that will be handled downstream as if they were included like in a normal request.
+ // (Here we allow such query parameters to override headers with the same name
+ // if they are not signed, however there is not much reason that this would happen)
+ headers_mut.insert(
+ name,
+ HeaderValue::from_bytes(value.as_bytes())
+ .ok_or_bad_request("invalid query parameter value")?,
+ );
+ }
+ }
+
Ok((Some(key), None))
}
@@ -197,12 +226,13 @@ pub fn parse_query_map(uri: &http::uri::Uri) -> Result<QueryMap, Error> {
}
fn split_signed_headers(authorization: &Authorization) -> Result<Vec<HeaderName>, Error> {
- let signed_headers = authorization
+ let mut signed_headers = authorization
.signed_headers
.split(';')
.map(HeaderName::try_from)
.collect::<Result<Vec<HeaderName>, _>>()
.ok_or_bad_request("invalid header name")?;
+ signed_headers.sort_by(|h1, h2| h1.as_str().cmp(h2.as_str()));
Ok(signed_headers)
}
@@ -224,7 +254,7 @@ pub fn canonical_request(
canonical_uri: &str,
query: &QueryMap,
headers: &HeaderMap,
- mut signed_headers: Vec<HeaderName>,
+ signed_headers: &[HeaderName],
content_sha256: &str,
) -> Result<String, Error> {
// There seems to be evidence that in AWSv4 signatures, the path component is url-encoded
@@ -273,7 +303,6 @@ pub fn canonical_request(
};
// Canonical header string calculated from signed headers
- signed_headers.sort_by(|h1, h2| h1.as_str().cmp(h2.as_str()));
let canonical_header_string = signed_headers
.iter()
.map(|name| {
diff --git a/src/garage/tests/common/custom_requester.rs b/src/garage/tests/common/custom_requester.rs
index f311418c..2cac5cd5 100644
--- a/src/garage/tests/common/custom_requester.rs
+++ b/src/garage/tests/common/custom_requester.rs
@@ -251,7 +251,7 @@ impl<'a> RequestBuilder<'a> {
uri.path(),
&query,
&all_headers,
- signed_headers,
+ &signed_headers,
&body_sha,
)
.unwrap();