4 changed files with 22 additions and 111 deletions
--- a/src/api/signature/payload.rs
+++ b/src/api/signature/payload.rs
@ -31,13 +31,7 @@ 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 = HeaderMap<QueryValue>;
+pub type QueryMap = HashMap<String, String>;
 pub struct QueryValue {
 	/// Original key with potential uppercase characters,
 	/// for use in signature calculation
 	key: String,
 	value: String,
 }
 pub async fn check_payload_signature(
 	garage: &Garage,
@ -46,7 +40,7 @@ pub async fn check_payload_signature(
 ) -> Result<(Option<Key>, Option<Hash>), Error> {
 	let query = parse_query_map(request.uri())?;
-	if query.contains_key(&X_AMZ_ALGORITHM) {
+	if query.contains_key(X_AMZ_ALGORITHM.as_str()) {
 		// We check for presigned-URL-style authentification first, because
 		// the browser or someting else could inject an Authorization header
 		// that is totally unrelated to AWS signatures.
@ -127,8 +121,8 @@ async fn check_presigned_signature(
 	request: &mut Request<Body>,
 	mut query: QueryMap,
 ) -> Result<(Option<Key>, Option<Hash>), Error> {
-	let algorithm = query.get(&X_AMZ_ALGORITHM).unwrap();
+	let algorithm = query.get(X_AMZ_ALGORITHM.as_str()).unwrap();
-	let authorization = Authorization::parse_presigned(&algorithm.value, &query)?;
+	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:
@ -141,7 +135,7 @@ async fn check_presigned_signature(
 	// 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);
+	query.remove(X_AMZ_SIGNATURE.as_str());
 	let canonical_request = canonical_request(
 		service,
 		request.method(),
@ -167,8 +161,10 @@ async fn check_presigned_signature(
 	// then an InvalidRequest error is raised.
 	let headers_mut = request.headers_mut();
 	for (name, value) in query.iter() {
-		if let Some(existing) = headers_mut.get(name) {
+		let name =
-			if signed_headers.contains(&name) && existing.as_bytes() != value.value.as_bytes() {
+			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
@ -184,7 +180,7 @@ async fn check_presigned_signature(
 			// that are not signed, however there is not much reason that this would happen)
 			headers_mut.insert(
 				name,
-				HeaderValue::from_bytes(value.value.as_bytes())
+				HeaderValue::from_bytes(value.as_bytes())
 					.ok_or_bad_request("invalid query parameter value")?,
 			);
 		}
@ -196,19 +192,11 @@ async fn check_presigned_signature(
 }
 pub fn parse_query_map(uri: &http::uri::Uri) -> Result<QueryMap, Error> {
-	let mut query = QueryMap::with_capacity(0);
+	let mut query = QueryMap::new();
 	if let Some(query_str) = uri.query() {
 		let query_pairs = url::form_urlencoded::parse(query_str.as_bytes());
 		for (key, val) in query_pairs {
-			let name =
+			if query.insert(key.to_string(), val.into_owned()).is_some() {
 				HeaderName::from_bytes(key.as_bytes()).ok_or_bad_request("Invalid header name")?;
 			let value = QueryValue {
 				key: key.to_string(),
 				value: val.into_owned(),
 			};
 			if query.insert(name, value).is_some() {
 				return Err(Error::bad_request(format!(
 					"duplicate query parameter: `{}`",
 					key
@ -317,7 +305,7 @@ pub fn canonical_request(
 	// Canonical query string from passed HeaderMap
 	let canonical_query_string = {
 		let mut items = Vec::with_capacity(query.len());
-		for (_, QueryValue { key, value }) in query.iter() {
+		for (key, value) in query.iter() {
 			items.push(uri_encode(&key, true) + "=" + &uri_encode(&value, true));
 		}
 		items.sort();
@ -475,19 +463,18 @@ impl Authorization {
 		}
 		let cred = query
-			.get(&X_AMZ_CREDENTIAL)
+			.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)
+			.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)
+			.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)
+			.get(X_AMZ_EXPIRES.as_str())
 			.ok_or_bad_request("X-Amz-Expires not found in query parameters")?
 			.value
 			.parse()
 			.map_err(|_| Error::bad_request("X-Amz-Expires is not a number".to_string()))?;
@ -498,20 +485,20 @@ impl Authorization {
 		}
 		let date = query
-			.get(&X_AMZ_DATE)
+			.get(X_AMZ_DATE.as_str())
 			.ok_or_bad_request("Missing X-Amz-Date field")?;
-		let date = parse_date(&date.value)?;
+		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.value)?;
+		let (key_id, scope) = parse_credential(cred)?;
 		Ok(Authorization {
 			key_id,
 			scope,
-			signed_headers: signed_headers.value.clone(),
+			signed_headers: signed_headers.to_string(),
-			signature: signature.value.clone(),
+			signature: signature.to_string(),
 			content_sha256: UNSIGNED_PAYLOAD.to_string(),
 			date,
 		})
--- a/src/garage/tests/common/custom_requester.rs
+++ b/src/garage/tests/common/custom_requester.rs
@ -59,10 +59,6 @@ impl CustomRequester {
 			vhost_style: false,
 		}
 	}
 	pub fn client(&self) -> &Client<HttpConnector, Body> {
 		&self.client
 	}
 }
 pub struct RequestBuilder<'a> {
--- a/src/garage/tests/s3/mod.rs
+++ b/src/garage/tests/s3/mod.rs
@ -1,7 +1,6 @@
 mod list;
 mod multipart;
 mod objects;
 mod presigned;
 mod simple;
 mod streaming_signature;
 mod website;
--- a/src/garage/tests/s3/presigned.rs
+++ b/src/garage/tests/s3/presigned.rs
@ -1,71 +0,0 @@
 use std::time::{Duration, SystemTime};
 use crate::common;
 use aws_sdk_s3::presigning::PresigningConfig;
 use bytes::Bytes;
 use hyper::{Body, Request};
 const STD_KEY: &str = "hello world";
 const BODY: &[u8; 62] = b"0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
 #[tokio::test]
 async fn test_presigned_url() {
 	let ctx = common::context();
 	let bucket = ctx.create_bucket("presigned");
 	let etag = "\"46cf18a9b447991b450cad3facf5937e\"";
 	let body = Bytes::from(BODY.to_vec());
 	let psc = PresigningConfig::builder()
 		.start_time(SystemTime::now() - Duration::from_secs(60))
 		.expires_in(Duration::from_secs(3600))
 		.build()
 		.unwrap();
 	{
 		// PutObject
 		let req = ctx
 			.client
 			.put_object()
 			.bucket(&bucket)
 			.key(STD_KEY)
 			.presigned(psc.clone())
 			.await
 			.unwrap();
 		let client = ctx.custom_request.client();
 		let req = Request::builder()
 			.method("PUT")
 			.uri(req.uri())
 			.body(body.clone().into())
 			.unwrap();
 		let res = client.request(req).await.unwrap();
 		assert_eq!(res.status(), 200);
 		assert_eq!(res.headers().get("etag").unwrap(), etag);
 	}
 	{
 		// GetObject
 		let req = ctx
 			.client
 			.get_object()
 			.bucket(&bucket)
 			.key(STD_KEY)
 			.presigned(psc)
 			.await
 			.unwrap();
 		let client = ctx.custom_request.client();
 		let req = Request::builder()
 			.method("GET")
 			.uri(req.uri())
 			.body(Body::empty())
 			.unwrap();
 		let res = client.request(req).await.unwrap();
 		assert_eq!(res.status(), 200);
 		assert_eq!(res.headers().get("etag").unwrap(), etag);
 		let body2 = hyper::body::to_bytes(res.into_body()).await.unwrap();
 		assert_eq!(body, body2);
 	}
 }