aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authortrinity-1686a <trinity.pointard@gmail.com>2022-01-18 12:22:31 +0100
committerAlex <alex@adnab.me>2022-01-18 12:22:31 +0100
commite55fa38c9995294edcdf0f7f4f95dc767b343fb5 (patch)
treed2a43ac455f87bee797a8f1caf083ab807b0d942
parent178e35f868d3102342838f5669da44b4eb0fc4f3 (diff)
downloadgarage-e55fa38c9995294edcdf0f7f4f95dc767b343fb5.tar.gz
garage-e55fa38c9995294edcdf0f7f4f95dc767b343fb5.zip
Add date verification to presigned urls (#196)
fix #96 fix #162 by returning Forbidden instead Bad Request Co-authored-by: Trinity Pointard <trinity.pointard@gmail.com> Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/196 Co-authored-by: trinity-1686a <trinity.pointard@gmail.com> Co-committed-by: trinity-1686a <trinity.pointard@gmail.com>
-rw-r--r--src/api/api_server.rs3
-rw-r--r--src/api/error.rs6
-rw-r--r--src/api/s3_copy.rs4
-rw-r--r--src/api/s3_list.rs2
-rw-r--r--src/api/signature/payload.rs81
-rw-r--r--src/model/helper/error.rs6
6 files changed, 67 insertions, 35 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<Garage>, req: Request<Body>) -> Result<Response<Body>, 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/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<M: AsRef<str>>(self, reason: M) -> Result<T, Error> {
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/api/signature/payload.rs b/src/api/signature/payload.rs
index b13819a8..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<Body>,
-) -> Result<(Key, Option<Hash>), Error> {
+) -> Result<(Option<Key>, Option<Hash>), Error> {
let mut headers = HashMap::new();
for (key, val) in request.headers() {
headers.insert(key.to_string(), val.to_str()?.to_string());
@@ -34,24 +34,24 @@ 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 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<Utc> = 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 +74,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",
@@ -104,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 {
@@ -113,6 +113,7 @@ struct Authorization {
signed_headers: String,
signature: String,
content_sha256: String,
+ date: DateTime<Utc>,
}
fn parse_authorization(
@@ -147,6 +148,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<Utc> = 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,15 +171,16 @@ 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)
}
-fn parse_query_authorization(headers: &HashMap<String, String>) -> Result<Authorization, Error> {
- 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<String, String>,
+) -> Result<Authorization, Error> {
+ if algorithm != "AWS4-HMAC-SHA256" {
return Err(Error::BadRequest(
"Unsupported authorization method".to_string(),
));
@@ -188,12 +201,36 @@ fn parse_query_authorization(headers: &HashMap<String, String>) -> Result<Author
.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::BadRequest("X-Amz-Expires is not a number".to_string()))?;
+
+ if duration > 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<Utc> = 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,
})
}
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<M: AsRef<str>>(self, reason: M) -> Result<T, Error> {
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))),
}
}
}