Merge pull request 'Split presigned signature verification + fix conditions' (#735) from fix-presigned into main

Reviewed-on: Deuxfleurs/garage#735

src/api/k2v/api_server.rs

@@ -15,8 +15,7 @@ use garage_model::garage::Garage;
 use crate::generic_server::*;
 use crate::k2v::error::*;
 
-use crate::signature::payload::check_payload_signature;
+use crate::signature::verify_request;
-use crate::signature::streaming::*;
 
 use crate::helpers::*;
 use crate::k2v::batch::*;
@@ -86,17 +85,7 @@ 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 (req, api_key, _content_sha256) = verify_request(&garage, req, "k2v").await?;
-		let api_key = api_key
-			.ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?;
-
-		let req = parse_streaming_body(
-			&api_key,
-			req,
-			&mut content_sha256,
-			&garage.config.s3_api.s3_region,
-			"k2v",
-		)?;
 
 		let bucket_id = garage
 			.bucket_helper()

src/api/s3/api_server.rs

@@ -17,8 +17,7 @@ use garage_model::key_table::Key;
 use crate::generic_server::*;
 use crate::s3::error::*;
 
-use crate::signature::payload::check_payload_signature;
+use crate::signature::verify_request;
-use crate::signature::streaming::*;
 
 use crate::helpers::*;
 use crate::s3::bucket::*;
@@ -125,17 +124,7 @@ 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 (req, api_key, content_sha256) = verify_request(&garage, req, "s3").await?;
-		let api_key = api_key
-			.ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?;
-
-		let req = parse_streaming_body(
-			&api_key,
-			req,
-			&mut content_sha256,
-			&garage.config.s3_api.s3_region,
-			"s3",
-		)?;
 
 		let bucket_name = match bucket_name {
 			None => {

src/api/s3/post_object.rs

@@ -21,7 +21,7 @@ use crate::s3::cors::*;
 use crate::s3::error::*;
 use crate::s3::put::{get_headers, save_stream};
 use crate::s3::xml as s3_xml;
-use crate::signature::payload::{parse_date, verify_v4};
+use crate::signature::payload::{verify_v4, Authorization};
 
 pub async fn handle_post_object(
 	garage: Arc<Garage>,
@@ -88,22 +88,11 @@ pub async fn handle_post_object(
 		.get("key")
 		.ok_or_bad_request("No key was provided")?
 		.to_str()?;
-	let credential = params
-		.get("x-amz-credential")
-		.ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?
-		.to_str()?;
 	let policy = params
 		.get("policy")
 		.ok_or_bad_request("No policy was provided")?
 		.to_str()?;
-	let signature = params
+	let authorization = Authorization::parse_form(&params)?;
-		.get("x-amz-signature")
-		.ok_or_bad_request("No signature was provided")?
-		.to_str()?;
-	let date = params
-		.get("x-amz-date")
-		.ok_or_bad_request("No date was provided")?
-		.to_str()?;
 
 	let key = if key.contains("${filename}") {
 		// if no filename is provided, don't replace. This matches the behavior of AWS.
@@ -116,16 +105,7 @@ pub async fn handle_post_object(
 		key.to_owned()
 	};
 
-	let date = parse_date(date)?;
+	let api_key = verify_v4(&garage, "s3", &authorization, policy.as_bytes()).await?;
-	let api_key = verify_v4(
-		&garage,
-		"s3",
-		credential,
-		&date,
-		signature,
-		policy.as_bytes(),
-	)
-	.await?;
 
 	let bucket_id = garage
 		.bucket_helper()

src/api/signature/mod.rs

@@ -2,19 +2,44 @@ use chrono::{DateTime, Utc};
 use hmac::{Hmac, Mac};
 use sha2::Sha256;
 
+use hyper::{body::Incoming as IncomingBody, Request};
+
+use garage_model::garage::Garage;
+use garage_model::key_table::Key;
 use garage_util::data::{sha256sum, Hash};
 
+use error::*;
+
 pub mod error;
 pub mod payload;
 pub mod streaming;
 
-use error::*;
-
 pub const SHORT_DATE: &str = "%Y%m%d";
 pub const LONG_DATETIME: &str = "%Y%m%dT%H%M%SZ";
 
 type HmacSha256 = Hmac<Sha256>;
 
+pub async fn verify_request(
+	garage: &Garage,
+	mut req: Request<IncomingBody>,
+	service: &'static str,
+) -> Result<(Request<streaming::ReqBody>, Key, Option<Hash>), Error> {
+	let (api_key, mut content_sha256) =
+		payload::check_payload_signature(&garage, &mut req, service).await?;
+	let api_key =
+		api_key.ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?;
+
+	let req = streaming::parse_streaming_body(
+		&api_key,
+		req,
+		&mut content_sha256,
+		&garage.config.s3_api.s3_region,
+		service,
+	)?;
+
+	Ok((req, api_key, content_sha256))
+}
+
 pub fn verify_signed_content(expected_sha256: Hash, body: &[u8]) -> Result<(), Error> {
 	if expected_sha256 != sha256sum(body) {
 		return Err(Error::bad_request(

src/api/signature/payload.rs

@@ -1,7 +1,9 @@
 use std::collections::HashMap;
+use std::convert::TryFrom;
 
 use chrono::{DateTime, Duration, NaiveDateTime, TimeZone, Utc};
 use hmac::Mac;
+use hyper::header::{HeaderMap, HeaderName, HeaderValue, AUTHORIZATION, CONTENT_TYPE, HOST};
 use hyper::{body::Incoming as IncomingBody, Method, Request};
 use sha2::{Digest, Sha256};
 
@@ -17,66 +19,91 @@ use super::{compute_scope, signing_hmac};
 use crate::encoding::uri_encode;
 use crate::signature::error::*;
 
+pub const X_AMZ_ALGORITHM: HeaderName = HeaderName::from_static("x-amz-algorithm");
+pub const X_AMZ_CREDENTIAL: HeaderName = HeaderName::from_static("x-amz-credential");
+pub const X_AMZ_DATE: HeaderName = HeaderName::from_static("x-amz-date");
+pub const X_AMZ_EXPIRES: HeaderName = HeaderName::from_static("x-amz-expires");
+pub const X_AMZ_SIGNEDHEADERS: HeaderName = HeaderName::from_static("x-amz-signedheaders");
+pub const X_AMZ_SIGNATURE: HeaderName = HeaderName::from_static("x-amz-signature");
+pub const X_AMZ_CONTENT_SH256: HeaderName = HeaderName::from_static("x-amz-content-sha256");
+
+pub const AWS4_HMAC_SHA256: &str = "AWS4-HMAC-SHA256";
+pub const UNSIGNED_PAYLOAD: &str = "UNSIGNED-PAYLOAD";
+pub const STREAMING_AWS4_HMAC_SHA256_PAYLOAD: &str = "STREAMING-AWS4-HMAC-SHA256-PAYLOAD";
+
+pub type QueryMap = HashMap<String, String>;
+
 pub async fn check_payload_signature(
 	garage: &Garage,
+	request: &mut Request<IncomingBody>,
 	service: &'static str,
-	request: &Request<IncomingBody>,
 ) -> Result<(Option<Key>, Option<Hash>), Error> {
-	let mut headers = HashMap::new();
+	let query = parse_query_map(request.uri())?;
-	for (key, val) in request.headers() {
-		headers.insert(key.to_string(), val.to_str()?.to_string());
-	}
-	if let Some(query) = request.uri().query() {
-		let query_pairs = url::form_urlencoded::parse(query.as_bytes());
-		for (key, val) in query_pairs {
-			headers.insert(key.to_lowercase(), val.to_string());
-		}
-	}
 
-	let authorization = if let Some(authorization) = headers.get("authorization") {
+	if query.contains_key(X_AMZ_ALGORITHM.as_str()) {
-		parse_authorization(authorization, &headers)?
+		// We check for presigned-URL-style authentification first, because
-	} else if let Some(algorithm) = headers.get("x-amz-algorithm") {
+		// the browser or someting else could inject an Authorization header
-		parse_query_authorization(algorithm, &headers)?
+		// that is totally unrelated to AWS signatures.
+		check_presigned_signature(garage, service, request, query).await
+	} else if request.headers().contains_key(AUTHORIZATION) {
+		check_standard_signature(garage, service, request, query).await
 	} else {
-		let content_sha256 = headers.get("x-amz-content-sha256");
+		// Unsigned (anonymous) request
-		if let Some(content_sha256) = content_sha256.filter(|c| "UNSIGNED-PAYLOAD" != c.as_str()) {
+		let content_sha256 = request
+			.headers()
+			.get("x-amz-content-sha256")
+			.filter(|c| c.as_bytes() != UNSIGNED_PAYLOAD.as_bytes());
+		if let Some(content_sha256) = content_sha256 {
 			let sha256 = hex::decode(content_sha256)
 				.ok()
 				.and_then(|bytes| Hash::try_from(&bytes))
 				.ok_or_bad_request("Invalid content sha256 hash")?;
-			return Ok((None, Some(sha256)));
+			Ok((None, Some(sha256)))
 		} else {
-			return Ok((None, None));
+			Ok((None, None))
 		}
-	};
+	}
+}
+
+async fn check_standard_signature(
+	garage: &Garage,
+	service: &'static str,
+	request: &Request<IncomingBody>,
+	query: QueryMap,
+) -> Result<(Option<Key>, Option<Hash>), Error> {
+	let authorization = Authorization::parse_header(request.headers())?;
+
+	// Verify that all necessary request headers are included in signed_headers
+	// For standard AWSv4 signatures, the following must be included:
+	// - the Host header (mandatory)
+	// - the Content-Type header, if it is used in the request
+	// - all x-amz-* headers used in the request
+	let signed_headers = split_signed_headers(&authorization)?;
+	verify_signed_headers(request.headers(), &signed_headers, &[CONTENT_TYPE])?;
 
 	let canonical_request = canonical_request(
 		service,
 		request.method(),
-		request.uri(),
+		request.uri().path(),
-		&headers,
+		&query,
-		&authorization.signed_headers,
+		request.headers(),
+		&signed_headers,
 		&authorization.content_sha256,
+	)?;
+	let string_to_sign = string_to_sign(
+		&authorization.date,
+		&authorization.scope,
+		&canonical_request,
 	);
-	let (_, scope) = parse_credential(&authorization.credential)?;
-	let string_to_sign = string_to_sign(&authorization.date, &scope, &canonical_request);
 
 	trace!("canonical request:\n{}", canonical_request);
 	trace!("string to sign:\n{}", string_to_sign);
 
-	let key = verify_v4(
+	let key = verify_v4(garage, service, &authorization, string_to_sign.as_bytes()).await?;
-		garage,
-		service,
-		&authorization.credential,
-		&authorization.date,
-		&authorization.signature,
-		string_to_sign.as_bytes(),
-	)
-	.await?;
 
-	let content_sha256 = if authorization.content_sha256 == "UNSIGNED-PAYLOAD" {
+	let content_sha256 = if authorization.content_sha256 == UNSIGNED_PAYLOAD {
 		None
-	} else if authorization.content_sha256 == "STREAMING-AWS4-HMAC-SHA256-PAYLOAD" {
+	} else if authorization.content_sha256 == STREAMING_AWS4_HMAC_SHA256_PAYLOAD {
 		let bytes = hex::decode(authorization.signature).ok_or_bad_request("Invalid signature")?;
 		Some(Hash::try_from(&bytes).ok_or_bad_request("Invalid signature")?)
 	} else {
@@ -88,124 +115,96 @@ pub async fn check_payload_signature(
 	Ok((Some(key), content_sha256))
 }
 
-struct Authorization {
+async fn check_presigned_signature(
-	credential: String,
+	garage: &Garage,
-	signed_headers: String,
+	service: &'static str,
-	signature: String,
+	request: &mut Request<IncomingBody>,
-	content_sha256: String,
+	mut query: QueryMap,
-	date: DateTime<Utc>,
+) -> Result<(Option<Key>, Option<Hash>), Error> {
+	let algorithm = query.get(X_AMZ_ALGORITHM.as_str()).unwrap();
+	let authorization = Authorization::parse_presigned(algorithm, &query)?;
+
+	// Verify that all necessary request headers are included in signed_headers
+	// For AWSv4 pre-signed URLs, the following must be incldued:
+	// - the Host header (mandatory)
+	// - all x-amz-* headers used in the request
+	let signed_headers = split_signed_headers(&authorization)?;
+	verify_signed_headers(request.headers(), &signed_headers, &[])?;
+
+	// The X-Amz-Signature value is passed as a query parameter,
+	// but the signature cannot be computed from a string that contains itself.
+	// AWS specifies that all query params except X-Amz-Signature are included
+	// in the canonical request.
+	query.remove(X_AMZ_SIGNATURE.as_str());
+	let canonical_request = canonical_request(
+		service,
+		request.method(),
+		request.uri().path(),
+		&query,
+		request.headers(),
+		&signed_headers,
+		&authorization.content_sha256,
+	)?;
+	let string_to_sign = string_to_sign(
+		&authorization.date,
+		&authorization.scope,
+		&canonical_request,
+	);
+
+	trace!("canonical request (presigned url):\n{}", canonical_request);
+	trace!("string to sign (presigned url):\n{}", string_to_sign);
+
+	let key = verify_v4(garage, service, &authorization, string_to_sign.as_bytes()).await?;
+
+	// In the page on presigned URLs, 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
+			// that 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")?,
+			);
+		}
+	}
+
+	// Presigned URLs always use UNSIGNED-PAYLOAD,
+	// so there is no sha256 hash to return.
+	Ok((Some(key), None))
 }
 
-fn parse_authorization(
+pub fn parse_query_map(uri: &http::uri::Uri) -> Result<QueryMap, Error> {
-	authorization: &str,
+	let mut query = QueryMap::new();
-	headers: &HashMap<String, String>,
+	if let Some(query_str) = uri.query() {
-) -> Result<Authorization, Error> {
+		let query_pairs = url::form_urlencoded::parse(query_str.as_bytes());
-	let first_space = authorization
+		for (key, val) in query_pairs {
-		.find(' ')
+			if query.insert(key.to_string(), val.into_owned()).is_some() {
-		.ok_or_bad_request("Authorization field to short")?;
+				return Err(Error::bad_request(format!(
-	let (auth_kind, rest) = authorization.split_at(first_space);
+					"duplicate query parameter: `{}`",
-
+					key
-	if auth_kind != "AWS4-HMAC-SHA256" {
+				)));
-		return Err(Error::bad_request("Unsupported authorization method"));
+			}
+		}
 	}
-
+	Ok(query)
-	let mut auth_params = HashMap::new();
-	for auth_part in rest.split(',') {
-		let auth_part = auth_part.trim();
-		let eq = auth_part
-			.find('=')
-			.ok_or_bad_request("Field without value in authorization header")?;
-		let (key, value) = auth_part.split_at(eq);
-		auth_params.insert(key.to_string(), value.trim_start_matches('=').to_string());
-	}
-
-	let cred = auth_params
-		.get("Credential")
-		.ok_or_bad_request("Could not find Credential in Authorization field")?;
-
-	let content_sha256 = headers
-		.get("x-amz-content-sha256")
-		.ok_or_bad_request("Missing X-Amz-Content-Sha256 field")?;
-
-	let date = headers
-		.get("x-amz-date")
-		.ok_or_bad_request("Missing X-Amz-Date field")
-		.map_err(Error::from)
-		.and_then(|d| parse_date(d))?;
-
-	if Utc::now() - date > Duration::hours(24) {
-		return Err(Error::bad_request("Date is too old".to_string()));
-	}
-
-	let auth = Authorization {
-		credential: cred.to_string(),
-		signed_headers: auth_params
-			.get("SignedHeaders")
-			.ok_or_bad_request("Could not find SignedHeaders in Authorization field")?
-			.to_string(),
-		signature: auth_params
-			.get("Signature")
-			.ok_or_bad_request("Could not find Signature in Authorization field")?
-			.to_string(),
-		content_sha256: content_sha256.to_string(),
-		date,
-	};
-	Ok(auth)
-}
-
-fn parse_query_authorization(
-	algorithm: &str,
-	headers: &HashMap<String, String>,
-) -> Result<Authorization, Error> {
-	if algorithm != "AWS4-HMAC-SHA256" {
-		return Err(Error::bad_request(
-			"Unsupported authorization method".to_string(),
-		));
-	}
-
-	let cred = headers
-		.get("x-amz-credential")
-		.ok_or_bad_request("X-Amz-Credential not found in query parameters")?;
-	let signed_headers = headers
-		.get("x-amz-signedheaders")
-		.ok_or_bad_request("X-Amz-SignedHeaders not found in query parameters")?;
-	let signature = headers
-		.get("x-amz-signature")
-		.ok_or_bad_request("X-Amz-Signature not found in query parameters")?;
-	let content_sha256 = headers
-		.get("x-amz-content-sha256")
-		.map(|x| x.as_str())
-		.unwrap_or("UNSIGNED-PAYLOAD");
-
-	let duration = headers
-		.get("x-amz-expires")
-		.ok_or_bad_request("X-Amz-Expires not found in query parameters")?
-		.parse()
-		.map_err(|_| Error::bad_request("X-Amz-Expires is not a number".to_string()))?;
-
-	if duration > 7 * 24 * 3600 {
-		return Err(Error::bad_request(
-			"X-Amz-Expires may not exceed a week".to_string(),
-		));
-	}
-
-	let date = headers
-		.get("x-amz-date")
-		.ok_or_bad_request("Missing X-Amz-Date field")
-		.map_err(Error::from)
-		.and_then(|d| parse_date(d))?;
-
-	if Utc::now() - date > Duration::seconds(duration) {
-		return Err(Error::bad_request("Date is too old".to_string()));
-	}
-
-	Ok(Authorization {
-		credential: cred.to_string(),
-		signed_headers: signed_headers.to_string(),
-		signature: signature.to_string(),
-		content_sha256: content_sha256.to_string(),
-		date,
-	})
 }
 
 fn parse_credential(cred: &str) -> Result<(String, String), Error> {
@@ -219,11 +218,43 @@ fn parse_credential(cred: &str) -> Result<(String, String), Error> {
 	))
 }
 
+fn split_signed_headers(authorization: &Authorization) -> Result<Vec<HeaderName>, Error> {
+	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)
+}
+
+fn verify_signed_headers(
+	headers: &HeaderMap,
+	signed_headers: &[HeaderName],
+	extra_headers: &[HeaderName],
+) -> Result<(), Error> {
+	if !signed_headers.contains(&HOST) {
+		return Err(Error::bad_request("Header `Host` should be signed"));
+	}
+	for (name, _) in headers.iter() {
+		if name.as_str().starts_with("x-amz-") || extra_headers.contains(name) {
+			if !signed_headers.contains(name) {
+				return Err(Error::bad_request(format!(
+					"Header `{}` should be signed",
+					name
+				)));
+			}
+		}
+	}
+	Ok(())
+}
+
 pub fn string_to_sign(datetime: &DateTime<Utc>, scope_string: &str, canonical_req: &str) -> String {
 	let mut hasher = Sha256::default();
 	hasher.update(canonical_req.as_bytes());
 	[
-		"AWS4-HMAC-SHA256",
+		AWS4_HMAC_SHA256,
 		&datetime.format(LONG_DATETIME).to_string(),
 		scope_string,
 		&hex::encode(hasher.finalize().as_slice()),
@@ -234,11 +265,12 @@ pub fn string_to_sign(datetime: &DateTime<Utc>, scope_string: &str, canonical_re
 pub fn canonical_request(
 	service: &'static str,
 	method: &Method,
-	uri: &hyper::Uri,
+	canonical_uri: &str,
-	headers: &HashMap<String, String>,
+	query: &QueryMap,
-	signed_headers: &str,
+	headers: &HeaderMap,
+	signed_headers: &[HeaderName],
 	content_sha256: &str,
-) -> String {
+) -> Result<String, Error> {
 	// 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
@@ -268,49 +300,46 @@ pub fn canonical_request(
 	// 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<str> = if service != "s3" {
+	let canonical_uri: std::borrow::Cow<str> = if service != "s3" {
-		uri_encode(uri.path(), false).into()
+		uri_encode(canonical_uri, false).into()
 	} else {
-		uri.path().into()
+		canonical_uri.into()
 	};
-	[
-		method.as_str(),
-		&path,
-		&canonical_query_string(uri),
-		&canonical_header_string(headers, signed_headers),
-		"",
-		signed_headers,
-		content_sha256,
-	]
-	.join("\n")
-}
 
-fn canonical_header_string(headers: &HashMap<String, String>, signed_headers: &str) -> String {
+	// Canonical query string from passed HeaderMap
-	let signed_headers_vec = signed_headers.split(';').collect::<Vec<_>>();
+	let canonical_query_string = {
-	let mut items = headers
+		let mut items = Vec::with_capacity(query.len());
-		.iter()
+		for (key, value) in query.iter() {
-		.filter(|(key, _)| signed_headers_vec.contains(&key.as_str()))
+			items.push(uri_encode(&key, true) + "=" + &uri_encode(&value, true));
-		.collect::<Vec<_>>();
+		}
-	items.sort_by(|(k1, _), (k2, _)| k1.cmp(k2));
-	items
-		.iter()
-		.map(|(key, value)| key.to_lowercase() + ":" + value.trim())
-		.collect::<Vec<_>>()
-		.join("\n")
-}
-
-fn canonical_query_string(uri: &hyper::Uri) -> String {
-	if let Some(query) = uri.query() {
-		let query_pairs = url::form_urlencoded::parse(query.as_bytes());
-		let mut items = query_pairs
-			.filter(|(key, _)| key != "X-Amz-Signature")
-			.map(|(key, value)| uri_encode(&key, true) + "=" + &uri_encode(&value, true))
-			.collect::<Vec<_>>();
 		items.sort();
 		items.join("&")
-	} else {
+	};
-		"".to_string()
+
-	}
+	// Canonical header string calculated from signed headers
+	let canonical_header_string = signed_headers
+		.iter()
+		.map(|name| {
+			let value = headers
+				.get(name)
+				.ok_or_bad_request(format!("signed header `{}` is not present", name))?
+				.to_str()?;
+			Ok(format!("{}:{}", name.as_str(), value.trim()))
+		})
+		.collect::<Result<Vec<String>, Error>>()?
+		.join("\n");
+	let signed_headers = signed_headers.join(";");
+
+	let list = [
+		method.as_str(),
+		&canonical_uri,
+		&canonical_query_string,
+		&canonical_header_string,
+		"",
+		&signed_headers,
+		content_sha256,
+	];
+	Ok(list.join("\n"))
 }
 
 pub fn parse_date(date: &str) -> Result<DateTime<Utc>, Error> {
@@ -322,28 +351,24 @@ pub fn parse_date(date: &str) -> Result<DateTime<Utc>, Error> {
 pub async fn verify_v4(
 	garage: &Garage,
 	service: &str,
-	credential: &str,
+	auth: &Authorization,
-	date: &DateTime<Utc>,
-	signature: &str,
 	payload: &[u8],
 ) -> Result<Key, Error> {
-	let (key_id, scope) = parse_credential(credential)?;
+	let scope_expected = compute_scope(&auth.date, &garage.config.s3_api.s3_region, service);
-
+	if auth.scope != scope_expected {
-	let scope_expected = compute_scope(date, &garage.config.s3_api.s3_region, service);
+		return Err(Error::AuthorizationHeaderMalformed(auth.scope.to_string()));
-	if scope != scope_expected {
-		return Err(Error::AuthorizationHeaderMalformed(scope.to_string()));
 	}
 
 	let key = garage
 		.key_table
-		.get(&EmptyKey, &key_id)
+		.get(&EmptyKey, &auth.key_id)
 		.await?
 		.filter(|k| !k.state.is_deleted())
-		.ok_or_else(|| Error::forbidden(format!("No such key: {}", &key_id)))?;
+		.ok_or_else(|| Error::forbidden(format!("No such key: {}", &auth.key_id)))?;
 	let key_p = key.params().unwrap();
 
 	let mut hmac = signing_hmac(
-		date,
+		&auth.date,
 		&key_p.secret_key,
 		&garage.config.s3_api.s3_region,
 		service,
@@ -351,9 +376,176 @@ pub async fn verify_v4(
 	.ok_or_internal_error("Unable to build signing HMAC")?;
 	hmac.update(payload);
 	let our_signature = hex::encode(hmac.finalize().into_bytes());
-	if signature != our_signature {
+	if auth.signature != our_signature {
 		return Err(Error::forbidden("Invalid signature".to_string()));
 	}
 
 	Ok(key)
 }
+
+// ============ Authorization header, or X-Amz-* query params =========
+
+pub struct Authorization {
+	key_id: String,
+	scope: String,
+	signed_headers: String,
+	signature: String,
+	content_sha256: String,
+	date: DateTime<Utc>,
+}
+
+impl Authorization {
+	fn parse_header(headers: &HeaderMap) -> Result<Self, Error> {
+		let authorization = headers
+			.get(AUTHORIZATION)
+			.ok_or_bad_request("Missing authorization header")?
+			.to_str()?;
+
+		let (auth_kind, rest) = authorization
+			.split_once(' ')
+			.ok_or_bad_request("Authorization field to short")?;
+
+		if auth_kind != AWS4_HMAC_SHA256 {
+			return Err(Error::bad_request("Unsupported authorization method"));
+		}
+
+		let mut auth_params = HashMap::new();
+		for auth_part in rest.split(',') {
+			let auth_part = auth_part.trim();
+			let eq = auth_part
+				.find('=')
+				.ok_or_bad_request("Field without value in authorization header")?;
+			let (key, value) = auth_part.split_at(eq);
+			auth_params.insert(key.to_string(), value.trim_start_matches('=').to_string());
+		}
+
+		let cred = auth_params
+			.get("Credential")
+			.ok_or_bad_request("Could not find Credential in Authorization field")?;
+		let signed_headers = auth_params
+			.get("SignedHeaders")
+			.ok_or_bad_request("Could not find SignedHeaders in Authorization field")?
+			.to_string();
+		let signature = auth_params
+			.get("Signature")
+			.ok_or_bad_request("Could not find Signature in Authorization field")?
+			.to_string();
+
+		let content_sha256 = headers
+			.get(X_AMZ_CONTENT_SH256)
+			.ok_or_bad_request("Missing X-Amz-Content-Sha256 field")?;
+
+		let date = headers
+			.get(X_AMZ_DATE)
+			.ok_or_bad_request("Missing X-Amz-Date field")
+			.map_err(Error::from)?
+			.to_str()?;
+		let date = parse_date(date)?;
+
+		if Utc::now() - date > Duration::hours(24) {
+			return Err(Error::bad_request("Date is too old".to_string()));
+		}
+
+		let (key_id, scope) = parse_credential(cred)?;
+		let auth = Authorization {
+			key_id,
+			scope,
+			signed_headers,
+			signature,
+			content_sha256: content_sha256.to_str()?.to_string(),
+			date,
+		};
+		Ok(auth)
+	}
+
+	fn parse_presigned(algorithm: &str, query: &QueryMap) -> Result<Self, Error> {
+		if algorithm != AWS4_HMAC_SHA256 {
+			return Err(Error::bad_request(
+				"Unsupported authorization method".to_string(),
+			));
+		}
+
+		let cred = query
+			.get(X_AMZ_CREDENTIAL.as_str())
+			.ok_or_bad_request("X-Amz-Credential not found in query parameters")?;
+		let signed_headers = query
+			.get(X_AMZ_SIGNEDHEADERS.as_str())
+			.ok_or_bad_request("X-Amz-SignedHeaders not found in query parameters")?;
+		let signature = query
+			.get(X_AMZ_SIGNATURE.as_str())
+			.ok_or_bad_request("X-Amz-Signature not found in query parameters")?;
+
+		let duration = query
+			.get(X_AMZ_EXPIRES.as_str())
+			.ok_or_bad_request("X-Amz-Expires not found in query parameters")?
+			.parse()
+			.map_err(|_| Error::bad_request("X-Amz-Expires is not a number".to_string()))?;
+
+		if duration > 7 * 24 * 3600 {
+			return Err(Error::bad_request(
+				"X-Amz-Expires may not exceed a week".to_string(),
+			));
+		}
+
+		let date = query
+			.get(X_AMZ_DATE.as_str())
+			.ok_or_bad_request("Missing X-Amz-Date field")?;
+		let date = parse_date(date)?;
+
+		if Utc::now() - date > Duration::seconds(duration) {
+			return Err(Error::bad_request("Date is too old".to_string()));
+		}
+
+		let (key_id, scope) = parse_credential(cred)?;
+		Ok(Authorization {
+			key_id,
+			scope,
+			signed_headers: signed_headers.to_string(),
+			signature: signature.to_string(),
+			content_sha256: UNSIGNED_PAYLOAD.to_string(),
+			date,
+		})
+	}
+
+	pub(crate) fn parse_form(params: &HeaderMap) -> Result<Self, Error> {
+		let algorithm = params
+			.get(X_AMZ_ALGORITHM)
+			.ok_or_bad_request("Missing X-Amz-Algorithm header")?
+			.to_str()?;
+		if algorithm != AWS4_HMAC_SHA256 {
+			return Err(Error::bad_request(
+				"Unsupported authorization method".to_string(),
+			));
+		}
+
+		let credential = params
+			.get(X_AMZ_CREDENTIAL)
+			.ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?
+			.to_str()?;
+		let signature = params
+			.get(X_AMZ_SIGNATURE)
+			.ok_or_bad_request("No signature was provided")?
+			.to_str()?
+			.to_string();
+		let date = params
+			.get(X_AMZ_DATE)
+			.ok_or_bad_request("No date was provided")?
+			.to_str()?;
+		let date = parse_date(date)?;
+
+		if Utc::now() - date > Duration::hours(24) {
+			return Err(Error::bad_request("Date is too old".to_string()));
+		}
+
+		let (key_id, scope) = parse_credential(credential)?;
+		let auth = Authorization {
+			key_id,
+			scope,
+			signed_headers: "".to_string(),
+			signature,
+			content_sha256: UNSIGNED_PAYLOAD.to_string(),
+			date,
+		};
+		Ok(auth)
+	}
+}

src/api/signature/streaming.rs

@@ -15,6 +15,11 @@ use super::{compute_scope, sha256sum, HmacSha256, LONG_DATETIME};
 
 use crate::helpers::*;
 use crate::signature::error::*;
+use crate::signature::payload::{
+	STREAMING_AWS4_HMAC_SHA256_PAYLOAD, X_AMZ_CONTENT_SH256, X_AMZ_DATE,
+};
+
+pub const AWS4_HMAC_SHA256_PAYLOAD: &str = "AWS4-HMAC-SHA256-PAYLOAD";
 
 pub type ReqBody = BoxBody<Error>;
 
@@ -25,8 +30,8 @@ pub fn parse_streaming_body(
 	region: &str,
 	service: &str,
 ) -> Result<Request<ReqBody>, Error> {
-	match req.headers().get("x-amz-content-sha256") {
+	match req.headers().get(X_AMZ_CONTENT_SH256) {
-		Some(header) if header == "STREAMING-AWS4-HMAC-SHA256-PAYLOAD" => {
+		Some(header) if header == STREAMING_AWS4_HMAC_SHA256_PAYLOAD => {
 			let signature = content_sha256
 				.take()
 				.ok_or_bad_request("No signature provided")?;
@@ -39,7 +44,7 @@ pub fn parse_streaming_body(
 
 			let date = req
 				.headers()
-				.get("x-amz-date")
+				.get(X_AMZ_DATE)
 				.ok_or_bad_request("Missing X-Amz-Date field")?
 				.to_str()?;
 			let date: NaiveDateTime = NaiveDateTime::parse_from_str(date, LONG_DATETIME)
@@ -75,7 +80,7 @@ fn compute_streaming_payload_signature(
 	content_sha256: Hash,
 ) -> Result<Hash, Error> {
 	let string_to_sign = [
-		"AWS4-HMAC-SHA256-PAYLOAD",
+		AWS4_HMAC_SHA256_PAYLOAD,
 		&date.format(LONG_DATETIME).to_string(),
 		scope,
 		&hex::encode(previous_signature),

src/garage/tests/common/custom_requester.rs

@@ -1,12 +1,15 @@
 #![allow(dead_code)]
 
 use std::collections::HashMap;
-use std::convert::TryFrom;
+use std::convert::{TryFrom, TryInto};
 
 use chrono::{offset::Utc, DateTime};
 use hmac::{Hmac, Mac};
 use http_body_util::BodyExt;
 use http_body_util::Full as FullBody;
+use hyper::header::{
+	HeaderMap, HeaderName, HeaderValue, AUTHORIZATION, CONTENT_ENCODING, CONTENT_LENGTH, HOST,
+};
 use hyper::{Method, Request, Response, Uri};
 use hyper_util::client::legacy::{connect::HttpConnector, Client};
 use hyper_util::rt::TokioExecutor;
@@ -173,54 +176,85 @@ impl<'a> RequestBuilder<'a> {
 		.unwrap();
 		let streaming_signer = signer.clone();
 
-		let mut all_headers = self.signed_headers.clone();
+		let mut all_headers = self
+			.signed_headers
+			.iter()
+			.map(|(k, v)| {
+				(
+					HeaderName::try_from(k).expect("invalid header name"),
+					HeaderValue::try_from(v).expect("invalid header value"),
+				)
+			})
+			.collect::<HeaderMap>();
 
 		let date = now.format(signature::LONG_DATETIME).to_string();
-		all_headers.insert("x-amz-date".to_owned(), date);
+		all_headers.insert(
-		all_headers.insert("host".to_owned(), host);
+			signature::payload::X_AMZ_DATE,
+			HeaderValue::from_str(&date).unwrap(),
+		);
+		all_headers.insert(HOST, HeaderValue::from_str(&host).unwrap());
 
 		let body_sha = match self.body_signature {
 			BodySignature::Unsigned => "UNSIGNED-PAYLOAD".to_owned(),
 			BodySignature::Classic => hex::encode(garage_util::data::sha256sum(&self.body)),
 			BodySignature::Streaming(size) => {
-				all_headers.insert("content-encoding".to_owned(), "aws-chunked".to_owned());
 				all_headers.insert(
-					"x-amz-decoded-content-length".to_owned(),
+					CONTENT_ENCODING,
-					self.body.len().to_string(),
+					HeaderValue::from_str("aws-chunked").unwrap(),
+				);
+				all_headers.insert(
+					HeaderName::from_static("x-amz-decoded-content-length"),
+					HeaderValue::from_str(&self.body.len().to_string()).unwrap(),
 				);
 				// Get lenght of body by doing the conversion to a streaming body with an
 				// invalid signature (we don't know the seed) just to get its length. This
 				// is a pretty lazy and inefficient way to do it, but it's enought for test
 				// code.
 				all_headers.insert(
-					"content-length".to_owned(),
+					CONTENT_LENGTH,
 					to_streaming_body(&self.body, size, String::new(), signer.clone(), now, "")
 						.len()
-						.to_string(),
+						.to_string()
+						.try_into()
+						.unwrap(),
 				);
 
 				"STREAMING-AWS4-HMAC-SHA256-PAYLOAD".to_owned()
 			}
 		};
-		all_headers.insert("x-amz-content-sha256".to_owned(), body_sha.clone());
+		all_headers.insert(
+			signature::payload::X_AMZ_CONTENT_SH256,
+			HeaderValue::from_str(&body_sha).unwrap(),
+		);
 
-		let mut signed_headers = all_headers
+		let mut signed_headers = all_headers.keys().cloned().collect::<Vec<_>>();
-			.keys()
+		signed_headers.sort_by(|h1, h2| h1.as_str().cmp(h2.as_str()));
-			.map(|k| k.as_ref())
+		let signed_headers_str = signed_headers
-			.collect::<Vec<&str>>();
+			.iter()
-		signed_headers.sort();
+			.map(ToString::to_string)
-		let signed_headers = signed_headers.join(";");
+			.collect::<Vec<_>>()
+			.join(";");
 
-		all_headers.extend(self.unsigned_headers.clone());
+		all_headers.extend(self.unsigned_headers.iter().map(|(k, v)| {
+			(
+				HeaderName::try_from(k).expect("invalid header name"),
+				HeaderValue::try_from(v).expect("invalid header value"),
+			)
+		}));
+
+		let uri = Uri::try_from(&uri).unwrap();
+		let query = signature::payload::parse_query_map(&uri).unwrap();
 
 		let canonical_request = signature::payload::canonical_request(
 			self.service,
 			&self.method,
-			&Uri::try_from(&uri).unwrap(),
+			uri.path(),
+			&query,
 			&all_headers,
 			&signed_headers,
 			&body_sha,
-		);
+		)
+		.unwrap();
 
 		let string_to_sign = signature::payload::string_to_sign(&now, &scope, &canonical_request);
 
@@ -228,14 +262,15 @@ impl<'a> RequestBuilder<'a> {
 		let signature = hex::encode(signer.finalize().into_bytes());
 		let authorization = format!(
 			"AWS4-HMAC-SHA256 Credential={}/{},SignedHeaders={},Signature={}",
-			self.requester.key.id, scope, signed_headers, signature
+			self.requester.key.id, scope, signed_headers_str, signature
+		);
+		all_headers.insert(
+			AUTHORIZATION,
+			HeaderValue::from_str(&authorization).unwrap(),
 		);
-		all_headers.insert("authorization".to_owned(), authorization);
 
 		let mut request = Request::builder();
-		for (k, v) in all_headers {
+		*request.headers_mut().unwrap() = all_headers;
-			request = request.header(k, v);
-		}
 
 		let body = if let BodySignature::Streaming(size) = self.body_signature {
 			to_streaming_body(&self.body, size, signature, streaming_signer, now, &scope)

src/garage/tests/s3/streaming_signature.rs

@@ -26,7 +26,7 @@ async fn test_putobject_streaming() {
 			.builder(bucket.clone())
 			.method(Method::PUT)
 			.path(STD_KEY.to_owned())
-			.unsigned_headers(headers)
+			.signed_headers(headers)
 			.vhost_style(true)
 			.body(vec![])
 			.body_signature(BodySignature::Streaming(10))