Deuxfleurs/garage
44
56
Fork 73
Code Issues 100 Pull requests 14 Releases 30 Wiki Activity

Split presigned signature verification + fix conditions #735

Merged
lx merged 6 commits from fix-presigned into main 2024-02-28 11:38:01 +00:00
Conversation 0 Commits 6 Files changed 8 +87 -83
4 changed files with 87 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 90cab5b8f2 - Show all commits

18
src/api/k2v/api_server.rs
View file

@ -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::streaming::*;
use crate::signature::verify_request;
use crate::helpers::*;
use crate::k2v::batch::*;
@ -69,7 +68,7 @@ impl ApiHandler for K2VApiServer {
async fn handle(
&self,
mut req: Request<IncomingBody>,
req: Request<IncomingBody>,
endpoint: K2VApiEndpoint,
) -> Result<Response<ResBody>, Error> {
let K2VApiEndpoint {
@ -86,18 +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", &mut req).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 (req, api_key, _content_sha256) = verify_request(&garage, req, "k2v").await?;
let bucket_id = garage
.bucket_helper()

18
src/api/s3/api_server.rs
View file

@ -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::streaming::*;
use crate::signature::verify_request;
use crate::helpers::*;
use crate::s3::bucket::*;
@ -107,7 +106,7 @@ impl ApiHandler for S3ApiServer {
async fn handle(
&self,
mut req: Request<IncomingBody>,
req: Request<IncomingBody>,
endpoint: S3ApiEndpoint,
) -> Result<Response<ResBody>, Error> {
let S3ApiEndpoint {
@ -125,18 +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", &mut req).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 (req, api_key, content_sha256) = verify_request(&garage, req, "s3").await?;
let bucket_name = match bucket_name {
None => {

29
src/api/signature/mod.rs
View file

@ -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(

105
src/api/signature/payload.rs
View file

@ -35,15 +35,18 @@ pub type QueryMap = HashMap<String, String>;
pub async fn check_payload_signature(
garage: &Garage,
service: &'static str,
request: &mut Request<IncomingBody>,
service: &'static str,
) -> Result<(Option<Key>, Option<Hash>), Error> {
let query = parse_query_map(request.uri())?;
if request.headers().contains_key(AUTHORIZATION) {
check_standard_signature(garage, service, request, query).await
} else if query.contains_key(X_AMZ_ALGORITHM.as_str()) {
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.
check_presigned_signature(garage, service, request, query).await
} else if request.headers().contains_key(AUTHORIZATION) {
check_standard_signature(garage, service, request, query).await
} else {
// Unsigned (anonymous) request
let content_sha256 = request
@ -68,27 +71,15 @@ async fn check_standard_signature(
request: &Request<IncomingBody>,
query: QueryMap,
) -> Result<(Option<Key>, Option<Hash>), Error> {
let authorization = Authorization::parse(request.headers())?;
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 incldued:
// - the Host header (in all cases)
// 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)?;
if !signed_headers.contains(&HOST) {
return Err(Error::bad_request("Header `Host` should be signed"));
}
for (name, _) in request.headers().iter() {
if name == CONTENT_TYPE || name.as_str().starts_with("x-amz-") {
if !signed_headers.contains(name) {
return Err(Error::bad_request(format!(
"Header `{}` should be signed",
name
)));
}
}
}
verify_signed_headers(request.headers(), &signed_headers, &[CONTENT_TYPE])?;
let canonical_request = canonical_request(
service,
@ -135,22 +126,10 @@ async fn check_presigned_signature(
// Verify that all necessary request headers are included in signed_headers
// For AWSv4 pre-signed URLs, the following must be incldued:
// - the Host header (in all cases)
// - the Host header (mandatory)
// - all x-amz-* headers used in the request
let signed_headers = split_signed_headers(&authorization)?;
if !signed_headers.contains(&HOST) {
return Err(Error::bad_request("Header `Host` should be signed"));
}
for (name, _) in request.headers().iter() {
if name.as_str().starts_with("x-amz-") {
if !signed_headers.contains(name) {
return Err(Error::bad_request(format!(
"Header `{}` should be signed",
name
)));
}
}
}
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.
@ -172,13 +151,14 @@ async fn check_presigned_signature(
&canonical_request,
);
trace!("canonical request:\n{}", canonical_request);
trace!("string to sign:\n{}", string_to_sign);
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?;
// 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.
// 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 =
@ -197,7 +177,7 @@ async fn check_presigned_signature(
// 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)
// that are not signed, however there is not much reason that this would happen)
headers_mut.insert(
name,
HeaderValue::from_bytes(value.as_bytes())
@ -206,6 +186,8 @@ async fn check_presigned_signature(
}
}
// Presigned URLs always use UNSIGNED-PAYLOAD,
// so there is no sha256 hash to return.
Ok((Some(key), None))
}
@ -225,6 +207,17 @@ pub fn parse_query_map(uri: &http::uri::Uri) -> Result<QueryMap, Error> {
Ok(query)
}
fn parse_credential(cred: &str) -> Result<(String, String), Error> {
let first_slash = cred
.find('/')
.ok_or_bad_request("Credentials does not contain '/' in authorization field")?;
let (key_id, scope) = cred.split_at(first_slash);
Ok((
key_id.to_string(),
scope.trim_start_matches('/').to_string(),
))
}
fn split_signed_headers(authorization: &Authorization) -> Result<Vec<HeaderName>, Error> {
let mut signed_headers = authorization
.signed_headers
@ -236,6 +229,27 @@ fn split_signed_headers(authorization: &Authorization) -> Result<Vec<HeaderName>
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());
@ -381,7 +395,7 @@ pub struct Authorization {
}
impl Authorization {
fn parse(headers: &HeaderMap) -> Result<Self, Error> {
fn parse_header(headers: &HeaderMap) -> Result<Self, Error> {
let authorization = headers
.get(AUTHORIZATION)
.ok_or_bad_request("Missing authorization header")?
@ -535,14 +549,3 @@ impl Authorization {
Ok(auth)
}
}
fn parse_credential(cred: &str) -> Result<(String, String), Error> {
let first_slash = cred
.find('/')
.ok_or_bad_request("Credentials does not contain '/' in authorization field")?;
let (key_id, scope) = cred.split_at(first_slash);
Ok((
key_id.to_string(),
scope.trim_start_matches('/').to_string(),
))
}