From 94348831bc52dd708ef9be289f894d908d6246c4 Mon Sep 17 00:00:00 2001 From: Trinity Pointard Date: Mon, 17 Jan 2022 19:39:50 +0100 Subject: [PATCH 1/3] verify date of presigned urls properly --- src/api/signature/payload.rs | 54 ++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/src/api/signature/payload.rs b/src/api/signature/payload.rs index b13819a8..4b13387a 100644 --- a/src/api/signature/payload.rs +++ b/src/api/signature/payload.rs @@ -38,20 +38,9 @@ pub async fn check_payload_signature( parse_query_authorization(&headers)? }; - let date = headers - .get("x-amz-date") - .ok_or_bad_request("Missing X-Amz-Date field")?; - let date: NaiveDateTime = - NaiveDateTime::parse_from_str(date, LONG_DATETIME).ok_or_bad_request("Invalid date")?; - let date: DateTime = DateTime::from_utc(date, Utc); - - if Utc::now() - date > Duration::hours(24) { - return Err(Error::BadRequest("Date is too old".to_string())); - } - let scope = format!( "{}/{}/s3/aws4_request", - date.format(SHORT_DATE), + authorization.date.format(SHORT_DATE), garage.config.s3_api.s3_region ); if authorization.scope != scope { @@ -74,10 +63,10 @@ pub async fn check_payload_signature( &authorization.signed_headers, &authorization.content_sha256, ); - let string_to_sign = string_to_sign(&date, &scope, &canonical_request); + let string_to_sign = string_to_sign(&authorization.date, &scope, &canonical_request); let mut hmac = signing_hmac( - &date, + &authorization.date, &key_p.secret_key, &garage.config.s3_api.s3_region, "s3", @@ -113,6 +102,7 @@ struct Authorization { signed_headers: String, signature: String, content_sha256: String, + date: DateTime, } fn parse_authorization( @@ -147,6 +137,17 @@ fn parse_authorization( .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")?; + let date: NaiveDateTime = + NaiveDateTime::parse_from_str(date, LONG_DATETIME).ok_or_bad_request("Invalid date")?; + let date: DateTime = DateTime::from_utc(date, Utc); + + if Utc::now() - date > Duration::hours(24) { + return Err(Error::BadRequest("Date is too old".to_string())); + } + let auth = Authorization { key_id, scope, @@ -159,6 +160,7 @@ fn parse_authorization( .ok_or_bad_request("Could not find Signature in Authorization field")? .to_string(), content_sha256: content_sha256.to_string(), + date, }; Ok(auth) } @@ -188,12 +190,36 @@ fn parse_query_authorization(headers: &HashMap) -> Result 7 * 24 * 3600 { + return Err(Error::BadRequest( + "X-Amz-Exprires may not exceed a week".to_string(), + )); + } + + let date = headers + .get("x-amz-date") + .ok_or_bad_request("Missing X-Amz-Date field")?; + let date: NaiveDateTime = + NaiveDateTime::parse_from_str(date, LONG_DATETIME).ok_or_bad_request("Invalid date")?; + let date: DateTime = DateTime::from_utc(date, Utc); + + if Utc::now() - date > Duration::seconds(duration) { + return Err(Error::BadRequest("Date is too old".to_string())); + } + Ok(Authorization { key_id, scope, signed_headers: signed_headers.to_string(), signature: signature.to_string(), content_sha256: content_sha256.to_string(), + date, }) } -- 2.45.2 From 2735dfae46041009228108c012c1d737484c1514 Mon Sep 17 00:00:00 2001 From: Trinity Pointard Date: Mon, 17 Jan 2022 19:41:34 +0100 Subject: [PATCH 2/3] clippy --- src/api/error.rs | 6 +----- src/api/s3_copy.rs | 4 ++-- src/api/s3_list.rs | 2 +- src/model/helper/error.rs | 6 +----- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/api/error.rs b/src/api/error.rs index 955faed1..8602cfdc 100644 --- a/src/api/error.rs +++ b/src/api/error.rs @@ -206,11 +206,7 @@ where fn ok_or_bad_request>(self, reason: M) -> Result { match self { Ok(x) => Ok(x), - Err(e) => Err(Error::BadRequest(format!( - "{}: {}", - reason.as_ref(), - e.to_string() - ))), + Err(e) => Err(Error::BadRequest(format!("{}: {}", reason.as_ref(), e))), } } } diff --git a/src/api/s3_copy.rs b/src/api/s3_copy.rs index f8b3550e..93947b78 100644 --- a/src/api/s3_copy.rs +++ b/src/api/s3_copy.rs @@ -487,7 +487,7 @@ impl CopyPreconditionHeaders { .get("x-amz-copy-source-if-modified-since") .map(|x| x.to_str()) .transpose()? - .map(|x| httpdate::parse_http_date(x)) + .map(httpdate::parse_http_date) .transpose() .ok_or_bad_request("Invalid date in x-amz-copy-source-if-modified-since")?, copy_source_if_none_match: req @@ -505,7 +505,7 @@ impl CopyPreconditionHeaders { .get("x-amz-copy-source-if-unmodified-since") .map(|x| x.to_str()) .transpose()? - .map(|x| httpdate::parse_http_date(x)) + .map(httpdate::parse_http_date) .transpose() .ok_or_bad_request("Invalid date in x-amz-copy-source-if-unmodified-since")?, }) diff --git a/src/api/s3_list.rs b/src/api/s3_list.rs index f991b07d..c3bc6938 100644 --- a/src/api/s3_list.rs +++ b/src/api/s3_list.rs @@ -121,7 +121,7 @@ pub async fn handle_list( key: uriencode_maybe(key, query.common.urlencode_resp), last_modified: s3_xml::Value(msec_to_rfc3339(info.last_modified)), size: s3_xml::IntValue(info.size as i64), - etag: s3_xml::Value(format!("\"{}\"", info.etag.to_string())), + etag: s3_xml::Value(format!("\"{}\"", info.etag)), storage_class: s3_xml::Value("STANDARD".to_string()), }) .collect(), diff --git a/src/model/helper/error.rs b/src/model/helper/error.rs index b9b515f3..30b2ba32 100644 --- a/src/model/helper/error.rs +++ b/src/model/helper/error.rs @@ -31,11 +31,7 @@ where fn ok_or_bad_request>(self, reason: M) -> Result { match self { Ok(x) => Ok(x), - Err(e) => Err(Error::BadRequest(format!( - "{}: {}", - reason.as_ref(), - e.to_string() - ))), + Err(e) => Err(Error::BadRequest(format!("{}: {}", reason.as_ref(), e))), } } } -- 2.45.2 From e3e80396ed4d9c6004aa8b17d354447b959e1dae Mon Sep 17 00:00:00 2001 From: Trinity Pointard Date: Mon, 17 Jan 2022 20:10:59 +0100 Subject: [PATCH 3/3] return 403 on anonymous query --- src/api/api_server.rs | 3 +++ src/api/signature/payload.rs | 27 +++++++++++++++++++-------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/api/api_server.rs b/src/api/api_server.rs index b34030fd..b064ac24 100644 --- a/src/api/api_server.rs +++ b/src/api/api_server.rs @@ -91,6 +91,9 @@ async fn handler( async fn handler_inner(garage: Arc, req: Request) -> Result, Error> { let (api_key, content_sha256) = check_payload_signature(&garage, &req).await?; + let api_key = api_key.ok_or_else(|| { + Error::Forbidden("Garage does not support anonymous access yet".to_string()) + })?; let authority = req .headers() diff --git a/src/api/signature/payload.rs b/src/api/signature/payload.rs index 4b13387a..fe6120d3 100644 --- a/src/api/signature/payload.rs +++ b/src/api/signature/payload.rs @@ -20,7 +20,7 @@ use crate::error::*; pub async fn check_payload_signature( garage: &Garage, request: &Request, -) -> Result<(Key, Option), Error> { +) -> Result<(Option, Option), Error> { let mut headers = HashMap::new(); for (key, val) in request.headers() { headers.insert(key.to_string(), val.to_str()?.to_string()); @@ -34,8 +34,19 @@ pub async fn check_payload_signature( let authorization = if let Some(authorization) = headers.get("authorization") { parse_authorization(authorization, &headers)? + } else if let Some(algorithm) = headers.get("x-amz-algorithm") { + parse_query_authorization(algorithm, &headers)? } else { - parse_query_authorization(&headers)? + let content_sha256 = headers.get("x-amz-content-sha256"); + if let Some(content_sha256) = content_sha256.filter(|c| "UNSIGNED-PAYLOAD" != c.as_str()) { + 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))); + } else { + return Ok((None, None)); + } }; let scope = format!( @@ -93,7 +104,7 @@ pub async fn check_payload_signature( Some(Hash::try_from(&bytes).ok_or_bad_request("Invalid content sha256 hash")?) }; - Ok((key, content_sha256)) + Ok((Some(key), content_sha256)) } struct Authorization { @@ -165,11 +176,11 @@ fn parse_authorization( Ok(auth) } -fn parse_query_authorization(headers: &HashMap) -> Result { - let algo = headers - .get("x-amz-algorithm") - .ok_or_bad_request("X-Amz-Algorithm not found in query parameters")?; - if algo != "AWS4-HMAC-SHA256" { +fn parse_query_authorization( + algorithm: &str, + headers: &HashMap, +) -> Result { + if algorithm != "AWS4-HMAC-SHA256" { return Err(Error::BadRequest( "Unsupported authorization method".to_string(), )); -- 2.45.2