Merge branch 'main' into next-v2

src/api/s3/Cargo.toml

@@ -1,12 +1,12 @@
 [package]
 name = "garage_api_s3"
-version = "1.0.1"
+version = "1.1.0"
 authors = ["Alex Auvolat <alex@adnab.me>"]
 edition = "2018"
 license = "AGPL-3.0"
 description = "S3 API server crate for the Garage object store"
 repository = "https://git.deuxfleurs.fr/Deuxfleurs/garage"
-readme = "../../README.md"
+readme = "../../../README.md"
 
 [lib]
 path = "lib.rs"

src/api/s3/api_server.rs

@@ -122,7 +122,9 @@ impl ApiHandler for S3ApiServer {
 			return Ok(options_res.map(|_empty_body: EmptyBody| empty_body()));
 		}
 
-		let (req, api_key, content_sha256) = verify_request(&garage, req, "s3").await?;
+		let verified_request = verify_request(&garage, req, "s3").await?;
+		let req = verified_request.request;
+		let api_key = verified_request.access_key;
 
 		let bucket_name = match bucket_name {
 			None => {
@@ -135,14 +137,7 @@ impl ApiHandler for S3ApiServer {
 
 		// Special code path for CreateBucket API endpoint
 		if let Endpoint::CreateBucket {} = endpoint {
-			return handle_create_bucket(
-				&garage,
-				req,
-				content_sha256,
-				&api_key.key_id,
-				bucket_name,
-			)
-			.await;
+			return handle_create_bucket(&garage, req, &api_key.key_id, bucket_name).await;
 		}
 
 		let bucket_id = garage
@@ -180,7 +175,7 @@ impl ApiHandler for S3ApiServer {
 		let resp = match endpoint {
 			Endpoint::HeadObject {
 				key, part_number, ..
-			} => handle_head(ctx, &req, &key, part_number).await,
+			} => handle_head(ctx, &req.map(|_| ()), &key, part_number).await,
 			Endpoint::GetObject {
 				key,
 				part_number,
@@ -200,20 +195,20 @@ impl ApiHandler for S3ApiServer {
 					response_content_type,
 					response_expires,
 				};
-				handle_get(ctx, &req, &key, part_number, overrides).await
+				handle_get(ctx, &req.map(|_| ()), &key, part_number, overrides).await
 			}
 			Endpoint::UploadPart {
 				key,
 				part_number,
 				upload_id,
-			} => handle_put_part(ctx, req, &key, part_number, &upload_id, content_sha256).await,
+			} => handle_put_part(ctx, req, &key, part_number, &upload_id).await,
 			Endpoint::CopyObject { key } => handle_copy(ctx, &req, &key).await,
 			Endpoint::UploadPartCopy {
 				key,
 				part_number,
 				upload_id,
 			} => handle_upload_part_copy(ctx, &req, &key, part_number, &upload_id).await,
-			Endpoint::PutObject { key } => handle_put(ctx, req, &key, content_sha256).await,
+			Endpoint::PutObject { key } => handle_put(ctx, req, &key).await,
 			Endpoint::AbortMultipartUpload { key, upload_id } => {
 				handle_abort_multipart_upload(ctx, &key, &upload_id).await
 			}
@@ -222,7 +217,7 @@ impl ApiHandler for S3ApiServer {
 				handle_create_multipart_upload(ctx, &req, &key).await
 			}
 			Endpoint::CompleteMultipartUpload { key, upload_id } => {
-				handle_complete_multipart_upload(ctx, req, &key, &upload_id, content_sha256).await
+				handle_complete_multipart_upload(ctx, req, &key, &upload_id).await
 			}
 			Endpoint::CreateBucket {} => unreachable!(),
 			Endpoint::HeadBucket {} => {
@@ -318,7 +313,6 @@ impl ApiHandler for S3ApiServer {
 			} => {
 				let query = ListPartsQuery {
 					bucket_name: ctx.bucket_name.clone(),
-					bucket_id,
 					key,
 					upload_id,
 					part_number_marker: part_number_marker.map(|p| p.min(10000)),
@@ -326,17 +320,15 @@ impl ApiHandler for S3ApiServer {
 				};
 				handle_list_parts(ctx, req, &query).await
 			}
-			Endpoint::DeleteObjects {} => handle_delete_objects(ctx, req, content_sha256).await,
+			Endpoint::DeleteObjects {} => handle_delete_objects(ctx, req).await,
 			Endpoint::GetBucketWebsite {} => handle_get_website(ctx).await,
-			Endpoint::PutBucketWebsite {} => handle_put_website(ctx, req, content_sha256).await,
+			Endpoint::PutBucketWebsite {} => handle_put_website(ctx, req).await,
 			Endpoint::DeleteBucketWebsite {} => handle_delete_website(ctx).await,
 			Endpoint::GetBucketCors {} => handle_get_cors(ctx).await,
-			Endpoint::PutBucketCors {} => handle_put_cors(ctx, req, content_sha256).await,
+			Endpoint::PutBucketCors {} => handle_put_cors(ctx, req).await,
 			Endpoint::DeleteBucketCors {} => handle_delete_cors(ctx).await,
 			Endpoint::GetBucketLifecycleConfiguration {} => handle_get_lifecycle(ctx).await,
-			Endpoint::PutBucketLifecycleConfiguration {} => {
-				handle_put_lifecycle(ctx, req, content_sha256).await
-			}
+			Endpoint::PutBucketLifecycleConfiguration {} => handle_put_lifecycle(ctx, req).await,
 			Endpoint::DeleteBucketLifecycle {} => handle_delete_lifecycle(ctx).await,
 			endpoint => Err(Error::NotImplemented(endpoint.name().to_owned())),
 		};

src/api/s3/bucket.rs

@@ -1,6 +1,5 @@
 use std::collections::HashMap;
 
-use http_body_util::BodyExt;
 use hyper::{Request, Response, StatusCode};
 
 use garage_model::bucket_alias_table::*;
@@ -10,12 +9,10 @@ use garage_model::key_table::Key;
 use garage_model::permission::BucketKeyPerm;
 use garage_table::util::*;
 use garage_util::crdt::*;
-use garage_util::data::*;
 use garage_util::time::*;
 
 use garage_api_common::common_error::CommonError;
 use garage_api_common::helpers::*;
-use garage_api_common::signature::verify_signed_content;
 
 use crate::api_server::{ReqBody, ResBody};
 use crate::error::*;
@@ -122,15 +119,10 @@ pub async fn handle_list_buckets(
 pub async fn handle_create_bucket(
 	garage: &Garage,
 	req: Request<ReqBody>,
-	content_sha256: Option<Hash>,
 	api_key_id: &String,
 	bucket_name: String,
 ) -> Result<Response<ResBody>, Error> {
-	let body = BodyExt::collect(req.into_body()).await?.to_bytes();
-
-	if let Some(content_sha256) = content_sha256 {
-		verify_signed_content(content_sha256, &body[..])?;
-	}
+	let body = req.into_body().collect().await?;
 
 	let cmd =
 		parse_create_bucket_xml(&body[..]).ok_or_bad_request("Invalid create bucket XML query")?;

src/api/s3/checksum.rs

@@ -1,406 +0,0 @@
-use std::convert::{TryFrom, TryInto};
-use std::hash::Hasher;
-
-use base64::prelude::*;
-use crc32c::Crc32cHasher as Crc32c;
-use crc32fast::Hasher as Crc32;
-use md5::{Digest, Md5};
-use sha1::Sha1;
-use sha2::Sha256;
-
-use http::{HeaderMap, HeaderName, HeaderValue};
-
-use garage_util::data::*;
-use garage_util::error::OkOrMessage;
-
-use garage_model::s3::object_table::*;
-
-use crate::error::*;
-
-pub const X_AMZ_CHECKSUM_ALGORITHM: HeaderName =
-	HeaderName::from_static("x-amz-checksum-algorithm");
-pub const X_AMZ_CHECKSUM_MODE: HeaderName = HeaderName::from_static("x-amz-checksum-mode");
-pub const X_AMZ_CHECKSUM_CRC32: HeaderName = HeaderName::from_static("x-amz-checksum-crc32");
-pub const X_AMZ_CHECKSUM_CRC32C: HeaderName = HeaderName::from_static("x-amz-checksum-crc32c");
-pub const X_AMZ_CHECKSUM_SHA1: HeaderName = HeaderName::from_static("x-amz-checksum-sha1");
-pub const X_AMZ_CHECKSUM_SHA256: HeaderName = HeaderName::from_static("x-amz-checksum-sha256");
-
-pub type Crc32Checksum = [u8; 4];
-pub type Crc32cChecksum = [u8; 4];
-pub type Md5Checksum = [u8; 16];
-pub type Sha1Checksum = [u8; 20];
-pub type Sha256Checksum = [u8; 32];
-
-#[derive(Debug, Default)]
-pub(crate) struct ExpectedChecksums {
-	// base64-encoded md5 (content-md5 header)
-	pub md5: Option<String>,
-	// content_sha256 (as a Hash / FixedBytes32)
-	pub sha256: Option<Hash>,
-	// extra x-amz-checksum-* header
-	pub extra: Option<ChecksumValue>,
-}
-
-pub(crate) struct Checksummer {
-	pub crc32: Option<Crc32>,
-	pub crc32c: Option<Crc32c>,
-	pub md5: Option<Md5>,
-	pub sha1: Option<Sha1>,
-	pub sha256: Option<Sha256>,
-}
-
-#[derive(Default)]
-pub(crate) struct Checksums {
-	pub crc32: Option<Crc32Checksum>,
-	pub crc32c: Option<Crc32cChecksum>,
-	pub md5: Option<Md5Checksum>,
-	pub sha1: Option<Sha1Checksum>,
-	pub sha256: Option<Sha256Checksum>,
-}
-
-impl Checksummer {
-	pub(crate) fn init(expected: &ExpectedChecksums, require_md5: bool) -> Self {
-		let mut ret = Self {
-			crc32: None,
-			crc32c: None,
-			md5: None,
-			sha1: None,
-			sha256: None,
-		};
-
-		if expected.md5.is_some() || require_md5 {
-			ret.md5 = Some(Md5::new());
-		}
-		if expected.sha256.is_some() || matches!(&expected.extra, Some(ChecksumValue::Sha256(_))) {
-			ret.sha256 = Some(Sha256::new());
-		}
-		if matches!(&expected.extra, Some(ChecksumValue::Crc32(_))) {
-			ret.crc32 = Some(Crc32::new());
-		}
-		if matches!(&expected.extra, Some(ChecksumValue::Crc32c(_))) {
-			ret.crc32c = Some(Crc32c::default());
-		}
-		if matches!(&expected.extra, Some(ChecksumValue::Sha1(_))) {
-			ret.sha1 = Some(Sha1::new());
-		}
-		ret
-	}
-
-	pub(crate) fn add(mut self, algo: Option<ChecksumAlgorithm>) -> Self {
-		match algo {
-			Some(ChecksumAlgorithm::Crc32) => {
-				self.crc32 = Some(Crc32::new());
-			}
-			Some(ChecksumAlgorithm::Crc32c) => {
-				self.crc32c = Some(Crc32c::default());
-			}
-			Some(ChecksumAlgorithm::Sha1) => {
-				self.sha1 = Some(Sha1::new());
-			}
-			Some(ChecksumAlgorithm::Sha256) => {
-				self.sha256 = Some(Sha256::new());
-			}
-			None => (),
-		}
-		self
-	}
-
-	pub(crate) fn update(&mut self, bytes: &[u8]) {
-		if let Some(crc32) = &mut self.crc32 {
-			crc32.update(bytes);
-		}
-		if let Some(crc32c) = &mut self.crc32c {
-			crc32c.write(bytes);
-		}
-		if let Some(md5) = &mut self.md5 {
-			md5.update(bytes);
-		}
-		if let Some(sha1) = &mut self.sha1 {
-			sha1.update(bytes);
-		}
-		if let Some(sha256) = &mut self.sha256 {
-			sha256.update(bytes);
-		}
-	}
-
-	pub(crate) fn finalize(self) -> Checksums {
-		Checksums {
-			crc32: self.crc32.map(|x| u32::to_be_bytes(x.finalize())),
-			crc32c: self
-				.crc32c
-				.map(|x| u32::to_be_bytes(u32::try_from(x.finish()).unwrap())),
-			md5: self.md5.map(|x| x.finalize()[..].try_into().unwrap()),
-			sha1: self.sha1.map(|x| x.finalize()[..].try_into().unwrap()),
-			sha256: self.sha256.map(|x| x.finalize()[..].try_into().unwrap()),
-		}
-	}
-}
-
-impl Checksums {
-	pub fn verify(&self, expected: &ExpectedChecksums) -> Result<(), Error> {
-		if let Some(expected_md5) = &expected.md5 {
-			match self.md5 {
-				Some(md5) if BASE64_STANDARD.encode(&md5) == expected_md5.trim_matches('"') => (),
-				_ => {
-					return Err(Error::InvalidDigest(
-						"MD5 checksum verification failed (from content-md5)".into(),
-					))
-				}
-			}
-		}
-		if let Some(expected_sha256) = &expected.sha256 {
-			match self.sha256 {
-				Some(sha256) if &sha256[..] == expected_sha256.as_slice() => (),
-				_ => {
-					return Err(Error::InvalidDigest(
-						"SHA256 checksum verification failed (from x-amz-content-sha256)".into(),
-					))
-				}
-			}
-		}
-		if let Some(extra) = expected.extra {
-			let algo = extra.algorithm();
-			if self.extract(Some(algo)) != Some(extra) {
-				return Err(Error::InvalidDigest(format!(
-					"Failed to validate checksum for algorithm {:?}",
-					algo
-				)));
-			}
-		}
-		Ok(())
-	}
-
-	pub fn extract(&self, algo: Option<ChecksumAlgorithm>) -> Option<ChecksumValue> {
-		match algo {
-			None => None,
-			Some(ChecksumAlgorithm::Crc32) => Some(ChecksumValue::Crc32(self.crc32.unwrap())),
-			Some(ChecksumAlgorithm::Crc32c) => Some(ChecksumValue::Crc32c(self.crc32c.unwrap())),
-			Some(ChecksumAlgorithm::Sha1) => Some(ChecksumValue::Sha1(self.sha1.unwrap())),
-			Some(ChecksumAlgorithm::Sha256) => Some(ChecksumValue::Sha256(self.sha256.unwrap())),
-		}
-	}
-}
-
-// ----
-
-#[derive(Default)]
-pub(crate) struct MultipartChecksummer {
-	pub md5: Md5,
-	pub extra: Option<MultipartExtraChecksummer>,
-}
-
-pub(crate) enum MultipartExtraChecksummer {
-	Crc32(Crc32),
-	Crc32c(Crc32c),
-	Sha1(Sha1),
-	Sha256(Sha256),
-}
-
-impl MultipartChecksummer {
-	pub(crate) fn init(algo: Option<ChecksumAlgorithm>) -> Self {
-		Self {
-			md5: Md5::new(),
-			extra: match algo {
-				None => None,
-				Some(ChecksumAlgorithm::Crc32) => {
-					Some(MultipartExtraChecksummer::Crc32(Crc32::new()))
-				}
-				Some(ChecksumAlgorithm::Crc32c) => {
-					Some(MultipartExtraChecksummer::Crc32c(Crc32c::default()))
-				}
-				Some(ChecksumAlgorithm::Sha1) => Some(MultipartExtraChecksummer::Sha1(Sha1::new())),
-				Some(ChecksumAlgorithm::Sha256) => {
-					Some(MultipartExtraChecksummer::Sha256(Sha256::new()))
-				}
-			},
-		}
-	}
-
-	pub(crate) fn update(
-		&mut self,
-		etag: &str,
-		checksum: Option<ChecksumValue>,
-	) -> Result<(), Error> {
-		self.md5
-			.update(&hex::decode(&etag).ok_or_message("invalid etag hex")?);
-		match (&mut self.extra, checksum) {
-			(None, _) => (),
-			(
-				Some(MultipartExtraChecksummer::Crc32(ref mut crc32)),
-				Some(ChecksumValue::Crc32(x)),
-			) => {
-				crc32.update(&x);
-			}
-			(
-				Some(MultipartExtraChecksummer::Crc32c(ref mut crc32c)),
-				Some(ChecksumValue::Crc32c(x)),
-			) => {
-				crc32c.write(&x);
-			}
-			(Some(MultipartExtraChecksummer::Sha1(ref mut sha1)), Some(ChecksumValue::Sha1(x))) => {
-				sha1.update(&x);
-			}
-			(
-				Some(MultipartExtraChecksummer::Sha256(ref mut sha256)),
-				Some(ChecksumValue::Sha256(x)),
-			) => {
-				sha256.update(&x);
-			}
-			(Some(_), b) => {
-				return Err(Error::internal_error(format!(
-					"part checksum was not computed correctly, got: {:?}",
-					b
-				)))
-			}
-		}
-		Ok(())
-	}
-
-	pub(crate) fn finalize(self) -> (Md5Checksum, Option<ChecksumValue>) {
-		let md5 = self.md5.finalize()[..].try_into().unwrap();
-		let extra = match self.extra {
-			None => None,
-			Some(MultipartExtraChecksummer::Crc32(crc32)) => {
-				Some(ChecksumValue::Crc32(u32::to_be_bytes(crc32.finalize())))
-			}
-			Some(MultipartExtraChecksummer::Crc32c(crc32c)) => Some(ChecksumValue::Crc32c(
-				u32::to_be_bytes(u32::try_from(crc32c.finish()).unwrap()),
-			)),
-			Some(MultipartExtraChecksummer::Sha1(sha1)) => {
-				Some(ChecksumValue::Sha1(sha1.finalize()[..].try_into().unwrap()))
-			}
-			Some(MultipartExtraChecksummer::Sha256(sha256)) => Some(ChecksumValue::Sha256(
-				sha256.finalize()[..].try_into().unwrap(),
-			)),
-		};
-		(md5, extra)
-	}
-}
-
-// ----
-
-/// Extract the value of the x-amz-checksum-algorithm header
-pub(crate) fn request_checksum_algorithm(
-	headers: &HeaderMap<HeaderValue>,
-) -> Result<Option<ChecksumAlgorithm>, Error> {
-	match headers.get(X_AMZ_CHECKSUM_ALGORITHM) {
-		None => Ok(None),
-		Some(x) if x == "CRC32" => Ok(Some(ChecksumAlgorithm::Crc32)),
-		Some(x) if x == "CRC32C" => Ok(Some(ChecksumAlgorithm::Crc32c)),
-		Some(x) if x == "SHA1" => Ok(Some(ChecksumAlgorithm::Sha1)),
-		Some(x) if x == "SHA256" => Ok(Some(ChecksumAlgorithm::Sha256)),
-		_ => Err(Error::bad_request("invalid checksum algorithm")),
-	}
-}
-
-/// Extract the value of any of the x-amz-checksum-* headers
-pub(crate) fn request_checksum_value(
-	headers: &HeaderMap<HeaderValue>,
-) -> Result<Option<ChecksumValue>, Error> {
-	let mut ret = vec![];
-
-	if let Some(crc32_str) = headers.get(X_AMZ_CHECKSUM_CRC32) {
-		let crc32 = BASE64_STANDARD
-			.decode(&crc32_str)
-			.ok()
-			.and_then(|x| x.try_into().ok())
-			.ok_or_bad_request("invalid x-amz-checksum-crc32 header")?;
-		ret.push(ChecksumValue::Crc32(crc32))
-	}
-	if let Some(crc32c_str) = headers.get(X_AMZ_CHECKSUM_CRC32C) {
-		let crc32c = BASE64_STANDARD
-			.decode(&crc32c_str)
-			.ok()
-			.and_then(|x| x.try_into().ok())
-			.ok_or_bad_request("invalid x-amz-checksum-crc32c header")?;
-		ret.push(ChecksumValue::Crc32c(crc32c))
-	}
-	if let Some(sha1_str) = headers.get(X_AMZ_CHECKSUM_SHA1) {
-		let sha1 = BASE64_STANDARD
-			.decode(&sha1_str)
-			.ok()
-			.and_then(|x| x.try_into().ok())
-			.ok_or_bad_request("invalid x-amz-checksum-sha1 header")?;
-		ret.push(ChecksumValue::Sha1(sha1))
-	}
-	if let Some(sha256_str) = headers.get(X_AMZ_CHECKSUM_SHA256) {
-		let sha256 = BASE64_STANDARD
-			.decode(&sha256_str)
-			.ok()
-			.and_then(|x| x.try_into().ok())
-			.ok_or_bad_request("invalid x-amz-checksum-sha256 header")?;
-		ret.push(ChecksumValue::Sha256(sha256))
-	}
-
-	if ret.len() > 1 {
-		return Err(Error::bad_request(
-			"multiple x-amz-checksum-* headers given",
-		));
-	}
-	Ok(ret.pop())
-}
-
-/// Checks for the presence of x-amz-checksum-algorithm
-/// if so extract the corresponding x-amz-checksum-* value
-pub(crate) fn request_checksum_algorithm_value(
-	headers: &HeaderMap<HeaderValue>,
-) -> Result<Option<ChecksumValue>, Error> {
-	match headers.get(X_AMZ_CHECKSUM_ALGORITHM) {
-		Some(x) if x == "CRC32" => {
-			let crc32 = headers
-				.get(X_AMZ_CHECKSUM_CRC32)
-				.and_then(|x| BASE64_STANDARD.decode(&x).ok())
-				.and_then(|x| x.try_into().ok())
-				.ok_or_bad_request("invalid x-amz-checksum-crc32 header")?;
-			Ok(Some(ChecksumValue::Crc32(crc32)))
-		}
-		Some(x) if x == "CRC32C" => {
-			let crc32c = headers
-				.get(X_AMZ_CHECKSUM_CRC32C)
-				.and_then(|x| BASE64_STANDARD.decode(&x).ok())
-				.and_then(|x| x.try_into().ok())
-				.ok_or_bad_request("invalid x-amz-checksum-crc32c header")?;
-			Ok(Some(ChecksumValue::Crc32c(crc32c)))
-		}
-		Some(x) if x == "SHA1" => {
-			let sha1 = headers
-				.get(X_AMZ_CHECKSUM_SHA1)
-				.and_then(|x| BASE64_STANDARD.decode(&x).ok())
-				.and_then(|x| x.try_into().ok())
-				.ok_or_bad_request("invalid x-amz-checksum-sha1 header")?;
-			Ok(Some(ChecksumValue::Sha1(sha1)))
-		}
-		Some(x) if x == "SHA256" => {
-			let sha256 = headers
-				.get(X_AMZ_CHECKSUM_SHA256)
-				.and_then(|x| BASE64_STANDARD.decode(&x).ok())
-				.and_then(|x| x.try_into().ok())
-				.ok_or_bad_request("invalid x-amz-checksum-sha256 header")?;
-			Ok(Some(ChecksumValue::Sha256(sha256)))
-		}
-		Some(_) => Err(Error::bad_request("invalid x-amz-checksum-algorithm")),
-		None => Ok(None),
-	}
-}
-
-pub(crate) fn add_checksum_response_headers(
-	checksum: &Option<ChecksumValue>,
-	mut resp: http::response::Builder,
-) -> http::response::Builder {
-	match checksum {
-		Some(ChecksumValue::Crc32(crc32)) => {
-			resp = resp.header(X_AMZ_CHECKSUM_CRC32, BASE64_STANDARD.encode(&crc32));
-		}
-		Some(ChecksumValue::Crc32c(crc32c)) => {
-			resp = resp.header(X_AMZ_CHECKSUM_CRC32C, BASE64_STANDARD.encode(&crc32c));
-		}
-		Some(ChecksumValue::Sha1(sha1)) => {
-			resp = resp.header(X_AMZ_CHECKSUM_SHA1, BASE64_STANDARD.encode(&sha1));
-		}
-		Some(ChecksumValue::Sha256(sha256)) => {
-			resp = resp.header(X_AMZ_CHECKSUM_SHA256, BASE64_STANDARD.encode(&sha256));
-		}
-		None => (),
-	}
-	resp
-}

src/api/s3/copy.rs

@@ -1,9 +1,9 @@
 use std::pin::Pin;
-use std::time::{Duration, SystemTime, UNIX_EPOCH};
 
 use futures::{stream, stream::Stream, StreamExt, TryStreamExt};
 
 use bytes::Bytes;
+use http::header::HeaderName;
 use hyper::{Request, Response};
 use serde::Serialize;
 
@@ -21,16 +21,25 @@ use garage_model::s3::object_table::*;
 use garage_model::s3::version_table::*;
 
 use garage_api_common::helpers::*;
+use garage_api_common::signature::checksum::*;
 
 use crate::api_server::{ReqBody, ResBody};
-use crate::checksum::*;
 use crate::encryption::EncryptionParams;
 use crate::error::*;
-use crate::get::full_object_byte_stream;
+use crate::get::{full_object_byte_stream, PreconditionHeaders};
 use crate::multipart;
-use crate::put::{get_headers, save_stream, ChecksumMode, SaveStreamResult};
+use crate::put::{extract_metadata_headers, save_stream, ChecksumMode, SaveStreamResult};
 use crate::xml::{self as s3_xml, xmlns_tag};
 
+pub const X_AMZ_COPY_SOURCE_IF_MATCH: HeaderName =
+	HeaderName::from_static("x-amz-copy-source-if-match");
+pub const X_AMZ_COPY_SOURCE_IF_NONE_MATCH: HeaderName =
+	HeaderName::from_static("x-amz-copy-source-if-none-match");
+pub const X_AMZ_COPY_SOURCE_IF_MODIFIED_SINCE: HeaderName =
+	HeaderName::from_static("x-amz-copy-source-if-modified-since");
+pub const X_AMZ_COPY_SOURCE_IF_UNMODIFIED_SINCE: HeaderName =
+	HeaderName::from_static("x-amz-copy-source-if-unmodified-since");
+
 // -------- CopyObject ---------
 
 pub async fn handle_copy(
@@ -38,7 +47,7 @@ pub async fn handle_copy(
 	req: &Request<ReqBody>,
 	dest_key: &str,
 ) -> Result<Response<ResBody>, Error> {
-	let copy_precondition = CopyPreconditionHeaders::parse(req)?;
+	let copy_precondition = PreconditionHeaders::parse_copy_source(req)?;
 
 	let checksum_algorithm = request_checksum_algorithm(req.headers())?;
 
@@ -48,7 +57,7 @@ pub async fn handle_copy(
 		extract_source_info(&source_object)?;
 
 	// Check precondition, e.g. x-amz-copy-source-if-match
-	copy_precondition.check(source_version, &source_version_meta.etag)?;
+	copy_precondition.check_copy_source(source_version, &source_version_meta.etag)?;
 
 	// Determine encryption parameters
 	let (source_encryption, source_object_meta_inner) =
@@ -73,7 +82,7 @@ pub async fn handle_copy(
 	let dest_object_meta = ObjectVersionMetaInner {
 		headers: match req.headers().get("x-amz-metadata-directive") {
 			Some(v) if v == hyper::header::HeaderValue::from_static("REPLACE") => {
-				get_headers(req.headers())?
+				extract_metadata_headers(req.headers())?
 			}
 			_ => source_object_meta_inner.into_owned().headers,
 		},
@@ -335,7 +344,7 @@ pub async fn handle_upload_part_copy(
 	part_number: u64,
 	upload_id: &str,
 ) -> Result<Response<ResBody>, Error> {
-	let copy_precondition = CopyPreconditionHeaders::parse(req)?;
+	let copy_precondition = PreconditionHeaders::parse_copy_source(req)?;
 
 	let dest_upload_id = multipart::decode_upload_id(upload_id)?;
 
@@ -351,7 +360,7 @@ pub async fn handle_upload_part_copy(
 		extract_source_info(&source_object)?;
 
 	// Check precondition on source, e.g. x-amz-copy-source-if-match
-	copy_precondition.check(source_object_version, &source_version_meta.etag)?;
+	copy_precondition.check_copy_source(source_object_version, &source_version_meta.etag)?;
 
 	// Determine encryption parameters
 	let (source_encryption, _) = EncryptionParams::check_decrypt_for_copy_source(
@@ -703,97 +712,6 @@ fn extract_source_info(
 	Ok((source_version, source_version_data, source_version_meta))
 }
 
-struct CopyPreconditionHeaders {
-	copy_source_if_match: Option<Vec<String>>,
-	copy_source_if_modified_since: Option<SystemTime>,
-	copy_source_if_none_match: Option<Vec<String>>,
-	copy_source_if_unmodified_since: Option<SystemTime>,
-}
-
-impl CopyPreconditionHeaders {
-	fn parse(req: &Request<ReqBody>) -> Result<Self, Error> {
-		Ok(Self {
-			copy_source_if_match: req
-				.headers()
-				.get("x-amz-copy-source-if-match")
-				.map(|x| x.to_str())
-				.transpose()?
-				.map(|x| {
-					x.split(',')
-						.map(|m| m.trim().trim_matches('"').to_string())
-						.collect::<Vec<_>>()
-				}),
-			copy_source_if_modified_since: req
-				.headers()
-				.get("x-amz-copy-source-if-modified-since")
-				.map(|x| x.to_str())
-				.transpose()?
-				.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
-				.headers()
-				.get("x-amz-copy-source-if-none-match")
-				.map(|x| x.to_str())
-				.transpose()?
-				.map(|x| {
-					x.split(',')
-						.map(|m| m.trim().trim_matches('"').to_string())
-						.collect::<Vec<_>>()
-				}),
-			copy_source_if_unmodified_since: req
-				.headers()
-				.get("x-amz-copy-source-if-unmodified-since")
-				.map(|x| x.to_str())
-				.transpose()?
-				.map(httpdate::parse_http_date)
-				.transpose()
-				.ok_or_bad_request("Invalid date in x-amz-copy-source-if-unmodified-since")?,
-		})
-	}
-
-	fn check(&self, v: &ObjectVersion, etag: &str) -> Result<(), Error> {
-		let v_date = UNIX_EPOCH + Duration::from_millis(v.timestamp);
-
-		let ok = match (
-			&self.copy_source_if_match,
-			&self.copy_source_if_unmodified_since,
-			&self.copy_source_if_none_match,
-			&self.copy_source_if_modified_since,
-		) {
-			// TODO I'm not sure all of the conditions are evaluated correctly here
-
-			// If we have both if-match and if-unmodified-since,
-			// basically we don't care about if-unmodified-since,
-			// because in the spec it says that if if-match evaluates to
-			// true but if-unmodified-since evaluates to false,
-			// the copy is still done.
-			(Some(im), _, None, None) => im.iter().any(|x| x == etag || x == "*"),
-			(None, Some(ius), None, None) => v_date <= *ius,
-
-			// If we have both if-none-match and if-modified-since,
-			// then both of the two conditions must evaluate to true
-			(None, None, Some(inm), Some(ims)) => {
-				!inm.iter().any(|x| x == etag || x == "*") && v_date > *ims
-			}
-			(None, None, Some(inm), None) => !inm.iter().any(|x| x == etag || x == "*"),
-			(None, None, None, Some(ims)) => v_date > *ims,
-			(None, None, None, None) => true,
-			_ => {
-				return Err(Error::bad_request(
-					"Invalid combination of x-amz-copy-source-if-xxxxx headers",
-				))
-			}
-		};
-
-		if ok {
-			Ok(())
-		} else {
-			Err(Error::PreconditionFailed)
-		}
-	}
-}
-
 type BlockStreamItemOk = (Bytes, Option<Hash>);
 type BlockStreamItem = Result<BlockStreamItemOk, garage_util::error::Error>;
 

src/api/s3/cors.rs

@@ -2,15 +2,11 @@ use quick_xml::de::from_reader;
 
 use hyper::{header::HeaderName, Method, Request, Response, StatusCode};
 
-use http_body_util::BodyExt;
-
 use serde::{Deserialize, Serialize};
 
 use garage_model::bucket_table::{Bucket, CorsRule as GarageCorsRule};
-use garage_util::data::*;
 
 use garage_api_common::helpers::*;
-use garage_api_common::signature::verify_signed_content;
 
 use crate::api_server::{ReqBody, ResBody};
 use crate::error::*;
@@ -59,7 +55,6 @@ pub async fn handle_delete_cors(ctx: ReqCtx) -> Result<Response<ResBody>, Error>
 pub async fn handle_put_cors(
 	ctx: ReqCtx,
 	req: Request<ReqBody>,
-	content_sha256: Option<Hash>,
 ) -> Result<Response<ResBody>, Error> {
 	let ReqCtx {
 		garage,
@@ -68,11 +63,7 @@ pub async fn handle_put_cors(
 		..
 	} = ctx;
 
-	let body = BodyExt::collect(req.into_body()).await?.to_bytes();
-
-	if let Some(content_sha256) = content_sha256 {
-		verify_signed_content(content_sha256, &body[..])?;
-	}
+	let body = req.into_body().collect().await?;
 
 	let conf: CorsConfiguration = from_reader(&body as &[u8])?;
 	conf.validate()?;

src/api/s3/delete.rs

@@ -1,4 +1,3 @@
-use http_body_util::BodyExt;
 use hyper::{Request, Response, StatusCode};
 
 use garage_util::data::*;
@@ -6,7 +5,6 @@ use garage_util::data::*;
 use garage_model::s3::object_table::*;
 
 use garage_api_common::helpers::*;
-use garage_api_common::signature::verify_signed_content;
 
 use crate::api_server::{ReqBody, ResBody};
 use crate::error::*;
@@ -68,13 +66,8 @@ pub async fn handle_delete(ctx: ReqCtx, key: &str) -> Result<Response<ResBody>,
 pub async fn handle_delete_objects(
 	ctx: ReqCtx,
 	req: Request<ReqBody>,
-	content_sha256: Option<Hash>,
 ) -> Result<Response<ResBody>, Error> {
-	let body = BodyExt::collect(req.into_body()).await?.to_bytes();
-
-	if let Some(content_sha256) = content_sha256 {
-		verify_signed_content(content_sha256, &body[..])?;
-	}
+	let body = req.into_body().collect().await?;
 
 	let cmd_xml = roxmltree::Document::parse(std::str::from_utf8(&body)?)?;
 	let cmd = parse_delete_objects_xml(&cmd_xml).ok_or_bad_request("Invalid delete XML query")?;

src/api/s3/encryption.rs

@@ -29,8 +29,8 @@ use garage_model::garage::Garage;
 use garage_model::s3::object_table::{ObjectVersionEncryption, ObjectVersionMetaInner};
 
 use garage_api_common::common_error::*;
+use garage_api_common::signature::checksum::Md5Checksum;
 
-use crate::checksum::Md5Checksum;
 use crate::error::Error;
 
 const X_AMZ_SERVER_SIDE_ENCRYPTION_CUSTOMER_ALGORITHM: HeaderName =

src/api/s3/error.rs

@@ -80,7 +80,7 @@ pub enum Error {
 	#[error(display = "Invalid encryption algorithm: {:?}, should be AES256", _0)]
 	InvalidEncryptionAlgorithm(String),
 
-	/// The client sent invalid XML data
+	/// The provided digest (checksum) value was invalid
 	#[error(display = "Invalid digest: {}", _0)]
 	InvalidDigest(String),
 
@@ -119,6 +119,7 @@ impl From<SignatureError> for Error {
 				Self::AuthorizationHeaderMalformed(c)
 			}
 			SignatureError::InvalidUtf8Str(i) => Self::InvalidUtf8Str(i),
+			SignatureError::InvalidDigest(d) => Self::InvalidDigest(d),
 		}
 	}
 }

src/api/s3/get.rs

@@ -2,17 +2,17 @@
 use std::collections::BTreeMap;
 use std::convert::TryInto;
 use std::sync::Arc;
-use std::time::{Duration, UNIX_EPOCH};
+use std::time::{Duration, SystemTime, UNIX_EPOCH};
 
 use bytes::Bytes;
 use futures::future;
 use futures::stream::{self, Stream, StreamExt};
 use http::header::{
-	ACCEPT_RANGES, CACHE_CONTROL, CONTENT_DISPOSITION, CONTENT_ENCODING, CONTENT_LANGUAGE,
-	CONTENT_LENGTH, CONTENT_RANGE, CONTENT_TYPE, ETAG, EXPIRES, IF_MODIFIED_SINCE, IF_NONE_MATCH,
-	LAST_MODIFIED, RANGE,
+	HeaderMap, HeaderName, ACCEPT_RANGES, CACHE_CONTROL, CONTENT_DISPOSITION, CONTENT_ENCODING,
+	CONTENT_LANGUAGE, CONTENT_LENGTH, CONTENT_RANGE, CONTENT_TYPE, ETAG, EXPIRES, IF_MATCH,
+	IF_MODIFIED_SINCE, IF_NONE_MATCH, IF_UNMODIFIED_SINCE, LAST_MODIFIED, RANGE,
 };
-use hyper::{body::Body, Request, Response, StatusCode};
+use hyper::{Request, Response, StatusCode};
 use tokio::sync::mpsc;
 
 use garage_net::stream::ByteStream;
@@ -26,13 +26,14 @@ use garage_model::s3::object_table::*;
 use garage_model::s3::version_table::*;
 
 use garage_api_common::helpers::*;
+use garage_api_common::signature::checksum::{add_checksum_response_headers, X_AMZ_CHECKSUM_MODE};
 
 use crate::api_server::ResBody;
-use crate::checksum::{add_checksum_response_headers, X_AMZ_CHECKSUM_MODE};
+use crate::copy::*;
 use crate::encryption::EncryptionParams;
 use crate::error::*;
 
-const X_AMZ_MP_PARTS_COUNT: &str = "x-amz-mp-parts-count";
+const X_AMZ_MP_PARTS_COUNT: HeaderName = HeaderName::from_static("x-amz-mp-parts-count");
 
 #[derive(Default)]
 pub struct GetObjectOverrides {
@@ -115,49 +116,29 @@ fn getobject_override_headers(
 	Ok(())
 }
 
-fn try_answer_cached(
+fn handle_http_precondition(
 	version: &ObjectVersion,
 	version_meta: &ObjectVersionMeta,
-	req: &Request<impl Body>,
-) -> Option<Response<ResBody>> {
-	// <trinity> It is possible, and is even usually the case, [that both If-None-Match and
-	// If-Modified-Since] are present in a request. In this situation If-None-Match takes
-	// precedence and If-Modified-Since is ignored (as per 6.Precedence from rfc7232). The rational
-	// being that etag based matching is more accurate, it has no issue with sub-second precision
-	// for instance (in case of very fast updates)
-	let cached = if let Some(none_match) = req.headers().get(IF_NONE_MATCH) {
-		let none_match = none_match.to_str().ok()?;
-		let expected = format!("\"{}\"", version_meta.etag);
-		let found = none_match
-			.split(',')
-			.map(str::trim)
-			.any(|etag| etag == expected || etag == "\"*\"");
-		found
-	} else if let Some(modified_since) = req.headers().get(IF_MODIFIED_SINCE) {
-		let modified_since = modified_since.to_str().ok()?;
-		let client_date = httpdate::parse_http_date(modified_since).ok()?;
-		let server_date = UNIX_EPOCH + Duration::from_millis(version.timestamp);
-		client_date >= server_date
-	} else {
-		false
-	};
+	req: &Request<()>,
+) -> Result<Option<Response<ResBody>>, Error> {
+	let precondition_headers = PreconditionHeaders::parse(req)?;
 
-	if cached {
-		Some(
+	if let Some(status_code) = precondition_headers.check(&version, &version_meta.etag)? {
+		Ok(Some(
 			Response::builder()
-				.status(StatusCode::NOT_MODIFIED)
+				.status(status_code)
 				.body(empty_body())
 				.unwrap(),
-		)
+		))
 	} else {
-		None
+		Ok(None)
 	}
 }
 
 /// Handle HEAD request
 pub async fn handle_head(
 	ctx: ReqCtx,
-	req: &Request<impl Body>,
+	req: &Request<()>,
 	key: &str,
 	part_number: Option<u64>,
 ) -> Result<Response<ResBody>, Error> {
@@ -167,7 +148,7 @@ pub async fn handle_head(
 /// Handle HEAD request for website
 pub async fn handle_head_without_ctx(
 	garage: Arc<Garage>,
-	req: &Request<impl Body>,
+	req: &Request<()>,
 	bucket_id: Uuid,
 	key: &str,
 	part_number: Option<u64>,
@@ -196,8 +177,8 @@ pub async fn handle_head_without_ctx(
 		_ => unreachable!(),
 	};
 
-	if let Some(cached) = try_answer_cached(object_version, version_meta, req) {
-		return Ok(cached);
+	if let Some(res) = handle_http_precondition(object_version, version_meta, req)? {
+		return Ok(res);
 	}
 
 	let (encryption, headers) =
@@ -278,7 +259,7 @@ pub async fn handle_head_without_ctx(
 /// Handle GET request
 pub async fn handle_get(
 	ctx: ReqCtx,
-	req: &Request<impl Body>,
+	req: &Request<()>,
 	key: &str,
 	part_number: Option<u64>,
 	overrides: GetObjectOverrides,
@@ -289,7 +270,7 @@ pub async fn handle_get(
 /// Handle GET request
 pub async fn handle_get_without_ctx(
 	garage: Arc<Garage>,
-	req: &Request<impl Body>,
+	req: &Request<()>,
 	bucket_id: Uuid,
 	key: &str,
 	part_number: Option<u64>,
@@ -318,8 +299,8 @@ pub async fn handle_get_without_ctx(
 		ObjectVersionData::FirstBlock(meta, _) => meta,
 	};
 
-	if let Some(cached) = try_answer_cached(last_v, last_v_meta, req) {
-		return Ok(cached);
+	if let Some(res) = handle_http_precondition(last_v, last_v_meta, req)? {
+		return Ok(res);
 	}
 
 	let (enc, headers) =
@@ -340,7 +321,12 @@ pub async fn handle_get_without_ctx(
 				enc,
 				&headers,
 				pn,
-				checksum_mode,
+				ChecksumMode {
+					// TODO: for multipart uploads, checksums of each part should be stored
+					// so that we can return the corresponding checksum here
+					// https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html
+					enabled: false,
+				},
 			)
 			.await
 		}
@@ -354,7 +340,12 @@ pub async fn handle_get_without_ctx(
 				&headers,
 				range.start,
 				range.start + range.length,
-				checksum_mode,
+				ChecksumMode {
+					// TODO: for range queries that align with part boundaries,
+					// we should return the saved checksum of the part
+					// https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html
+					enabled: false,
+				},
 			)
 			.await
 		}
@@ -577,7 +568,7 @@ async fn handle_get_part(
 }
 
 fn parse_range_header(
-	req: &Request<impl Body>,
+	req: &Request<()>,
 	total_size: u64,
 ) -> Result<Option<http_range::HttpRange>, Error> {
 	let range = match req.headers().get(RANGE) {
@@ -618,7 +609,7 @@ struct ChecksumMode {
 	enabled: bool,
 }
 
-fn checksum_mode(req: &Request<impl Body>) -> ChecksumMode {
+fn checksum_mode(req: &Request<()>) -> ChecksumMode {
 	ChecksumMode {
 		enabled: req
 			.headers()
@@ -751,3 +742,116 @@ fn std_error_from_read_error<E: std::fmt::Display>(e: E) -> std::io::Error {
 		format!("Error while reading object data: {}", e),
 	)
 }
+
+// ----
+
+pub struct PreconditionHeaders {
+	if_match: Option<Vec<String>>,
+	if_modified_since: Option<SystemTime>,
+	if_none_match: Option<Vec<String>>,
+	if_unmodified_since: Option<SystemTime>,
+}
+
+impl PreconditionHeaders {
+	fn parse<B>(req: &Request<B>) -> Result<Self, Error> {
+		Self::parse_with(
+			req.headers(),
+			&IF_MATCH,
+			&IF_NONE_MATCH,
+			&IF_MODIFIED_SINCE,
+			&IF_UNMODIFIED_SINCE,
+		)
+	}
+
+	pub(crate) fn parse_copy_source<B>(req: &Request<B>) -> Result<Self, Error> {
+		Self::parse_with(
+			req.headers(),
+			&X_AMZ_COPY_SOURCE_IF_MATCH,
+			&X_AMZ_COPY_SOURCE_IF_NONE_MATCH,
+			&X_AMZ_COPY_SOURCE_IF_MODIFIED_SINCE,
+			&X_AMZ_COPY_SOURCE_IF_UNMODIFIED_SINCE,
+		)
+	}
+
+	fn parse_with(
+		headers: &HeaderMap,
+		hdr_if_match: &HeaderName,
+		hdr_if_none_match: &HeaderName,
+		hdr_if_modified_since: &HeaderName,
+		hdr_if_unmodified_since: &HeaderName,
+	) -> Result<Self, Error> {
+		Ok(Self {
+			if_match: headers
+				.get(hdr_if_match)
+				.map(|x| x.to_str())
+				.transpose()?
+				.map(|x| {
+					x.split(',')
+						.map(|m| m.trim().trim_matches('"').to_string())
+						.collect::<Vec<_>>()
+				}),
+			if_none_match: headers
+				.get(hdr_if_none_match)
+				.map(|x| x.to_str())
+				.transpose()?
+				.map(|x| {
+					x.split(',')
+						.map(|m| m.trim().trim_matches('"').to_string())
+						.collect::<Vec<_>>()
+				}),
+			if_modified_since: headers
+				.get(hdr_if_modified_since)
+				.map(|x| x.to_str())
+				.transpose()?
+				.map(httpdate::parse_http_date)
+				.transpose()
+				.ok_or_bad_request("Invalid date in if-modified-since")?,
+			if_unmodified_since: headers
+				.get(hdr_if_unmodified_since)
+				.map(|x| x.to_str())
+				.transpose()?
+				.map(httpdate::parse_http_date)
+				.transpose()
+				.ok_or_bad_request("Invalid date in if-unmodified-since")?,
+		})
+	}
+
+	fn check(&self, v: &ObjectVersion, etag: &str) -> Result<Option<StatusCode>, Error> {
+		let v_date = UNIX_EPOCH + Duration::from_millis(v.timestamp);
+
+		// Implemented from https://datatracker.ietf.org/doc/html/rfc7232#section-6
+
+		if let Some(im) = &self.if_match {
+			// Step 1: if-match is present
+			if !im.iter().any(|x| x == etag || x == "*") {
+				return Ok(Some(StatusCode::PRECONDITION_FAILED));
+			}
+		} else if let Some(ius) = &self.if_unmodified_since {
+			// Step 2: if-unmodified-since is present, and if-match is absent
+			if v_date > *ius {
+				return Ok(Some(StatusCode::PRECONDITION_FAILED));
+			}
+		}
+
+		if let Some(inm) = &self.if_none_match {
+			// Step 3: if-none-match is present
+			if inm.iter().any(|x| x == etag || x == "*") {
+				return Ok(Some(StatusCode::NOT_MODIFIED));
+			}
+		} else if let Some(ims) = &self.if_modified_since {
+			// Step 4: if-modified-since is present, and if-none-match is absent
+			if v_date <= *ims {
+				return Ok(Some(StatusCode::NOT_MODIFIED));
+			}
+		}
+
+		Ok(None)
+	}
+
+	pub(crate) fn check_copy_source(&self, v: &ObjectVersion, etag: &str) -> Result<(), Error> {
+		match self.check(v, etag)? {
+			Some(_) => Err(Error::PreconditionFailed),
+			None => Ok(()),
+		}
+	}
+}

src/api/s3/lib.rs

@@ -14,9 +14,8 @@ mod list;
 mod multipart;
 mod post_object;
 mod put;
-mod website;
+pub mod website;
 
-mod checksum;
 mod encryption;
 mod router;
 pub mod xml;

src/api/s3/lifecycle.rs

@@ -1,12 +1,10 @@
 use quick_xml::de::from_reader;
 
-use http_body_util::BodyExt;
 use hyper::{Request, Response, StatusCode};
 
 use serde::{Deserialize, Serialize};
 
 use garage_api_common::helpers::*;
-use garage_api_common::signature::verify_signed_content;
 
 use crate::api_server::{ReqBody, ResBody};
 use crate::error::*;
@@ -16,7 +14,6 @@ use garage_model::bucket_table::{
 	parse_lifecycle_date, Bucket, LifecycleExpiration as GarageLifecycleExpiration,
 	LifecycleFilter as GarageLifecycleFilter, LifecycleRule as GarageLifecycleRule,
 };
-use garage_util::data::*;
 
 pub async fn handle_get_lifecycle(ctx: ReqCtx) -> Result<Response<ResBody>, Error> {
 	let ReqCtx { bucket_params, .. } = ctx;
@@ -56,7 +53,6 @@ pub async fn handle_delete_lifecycle(ctx: ReqCtx) -> Result<Response<ResBody>, E
 pub async fn handle_put_lifecycle(
 	ctx: ReqCtx,
 	req: Request<ReqBody>,
-	content_sha256: Option<Hash>,
 ) -> Result<Response<ResBody>, Error> {
 	let ReqCtx {
 		garage,
@@ -65,11 +61,7 @@ pub async fn handle_put_lifecycle(
 		..
 	} = ctx;
 
-	let body = BodyExt::collect(req.into_body()).await?.to_bytes();
-
-	if let Some(content_sha256) = content_sha256 {
-		verify_signed_content(content_sha256, &body[..])?;
-	}
+	let body = req.into_body().collect().await?;
 
 	let conf: LifecycleConfiguration = from_reader(&body as &[u8])?;
 	let config = conf

src/api/s3/list.rs

@@ -54,7 +54,6 @@ pub struct ListMultipartUploadsQuery {
 #[derive(Debug)]
 pub struct ListPartsQuery {
 	pub bucket_name: String,
-	pub bucket_id: Uuid,
 	pub key: String,
 	pub upload_id: String,
 	pub part_number_marker: Option<u64>,
@@ -1245,10 +1244,8 @@ mod tests {
 
 	#[test]
 	fn test_fetch_part_info() -> Result<(), Error> {
-		let uuid = Uuid::from([0x08; 32]);
 		let mut query = ListPartsQuery {
 			bucket_name: "a".to_string(),
-			bucket_id: uuid,
 			key: "a".to_string(),
 			upload_id: "xx".to_string(),
 			part_number_marker: None,

src/api/s3/multipart.rs

@@ -1,13 +1,20 @@
 use std::collections::HashMap;
-use std::convert::TryInto;
+use std::convert::{TryFrom, TryInto};
+use std::hash::Hasher;
 use std::sync::Arc;
 
 use base64::prelude::*;
+use crc32c::Crc32cHasher as Crc32c;
+use crc32fast::Hasher as Crc32;
 use futures::prelude::*;
 use hyper::{Request, Response};
+use md5::{Digest, Md5};
+use sha1::Sha1;
+use sha2::Sha256;
 
 use garage_table::*;
 use garage_util::data::*;
+use garage_util::error::OkOrMessage;
 
 use garage_model::garage::Garage;
 use garage_model::s3::block_ref_table::*;
@@ -16,10 +23,9 @@ use garage_model::s3::object_table::*;
 use garage_model::s3::version_table::*;
 
 use garage_api_common::helpers::*;
-use garage_api_common::signature::verify_signed_content;
+use garage_api_common::signature::checksum::*;
 
 use crate::api_server::{ReqBody, ResBody};
-use crate::checksum::*;
 use crate::encryption::EncryptionParams;
 use crate::error::*;
 use crate::put::*;
@@ -43,7 +49,7 @@ pub async fn handle_create_multipart_upload(
 	let upload_id = gen_uuid();
 	let timestamp = next_timestamp(existing_object.as_ref());
 
-	let headers = get_headers(req.headers())?;
+	let headers = extract_metadata_headers(req.headers())?;
 	let meta = ObjectVersionMetaInner {
 		headers,
 		checksum: None,
@@ -94,7 +100,6 @@ pub async fn handle_put_part(
 	key: &str,
 	part_number: u64,
 	upload_id: &str,
-	content_sha256: Option<Hash>,
 ) -> Result<Response<ResBody>, Error> {
 	let ReqCtx { garage, .. } = &ctx;
 
@@ -105,17 +110,30 @@ pub async fn handle_put_part(
 			Some(x) => Some(x.to_str()?.to_string()),
 			None => None,
 		},
-		sha256: content_sha256,
+		sha256: None,
 		extra: request_checksum_value(req.headers())?,
 	};
 
-	// Read first chuck, and at the same time try to get object to see if it exists
 	let key = key.to_string();
 
-	let (req_head, req_body) = req.into_parts();
-	let stream = body_stream(req_body);
+	let (req_head, mut req_body) = req.into_parts();
+
+	// Before we stream the body, configure the needed checksums.
+	req_body.add_expected_checksums(expected_checksums.clone());
+	// TODO: avoid parsing encryption headers twice...
+	if !EncryptionParams::new_from_headers(&garage, &req_head.headers)?.is_encrypted() {
+		// For non-encrypted objects, we need to compute the md5sum in all cases
+		// (even if content-md5 is not set), because it is used as an etag of the
+		// part, which is in turn used in the etag computation of the whole object
+		req_body.add_md5();
+	}
+
+	let (stream, stream_checksums) = req_body.streaming_with_checksums();
+	let stream = stream.map_err(Error::from);
+
 	let mut chunker = StreamChunker::new(stream, garage.config.block_size);
 
+	// Read first chuck, and at the same time try to get object to see if it exists
 	let ((_, object_version, mut mpu), first_block) =
 		futures::try_join!(get_upload(&ctx, &key, &upload_id), chunker.next(),)?;
 
@@ -172,21 +190,21 @@ pub async fn handle_put_part(
 	garage.version_table.insert(&version).await?;
 
 	// Copy data to version
-	let checksummer =
-		Checksummer::init(&expected_checksums, !encryption.is_encrypted()).add(checksum_algorithm);
-	let (total_size, checksums, _) = read_and_put_blocks(
+	let (total_size, _, _) = read_and_put_blocks(
 		&ctx,
 		&version,
 		encryption,
 		part_number,
 		first_block,
-		&mut chunker,
-		checksummer,
+		chunker,
+		Checksummer::new(),
 	)
 	.await?;
 
-	// Verify that checksums map
-	checksums.verify(&expected_checksums)?;
+	// Verify that checksums match
+	let checksums = stream_checksums
+		.await
+		.ok_or_internal_error("checksum calculation")??;
 
 	// Store part etag in version
 	let etag = encryption.etag_from_md5(&checksums.md5);
@@ -248,7 +266,6 @@ pub async fn handle_complete_multipart_upload(
 	req: Request<ReqBody>,
 	key: &str,
 	upload_id: &str,
-	content_sha256: Option<Hash>,
 ) -> Result<Response<ResBody>, Error> {
 	let ReqCtx {
 		garage,
@@ -260,11 +277,7 @@ pub async fn handle_complete_multipart_upload(
 
 	let expected_checksum = request_checksum_value(&req_head.headers)?;
 
-	let body = http_body_util::BodyExt::collect(req_body).await?.to_bytes();
-
-	if let Some(content_sha256) = content_sha256 {
-		verify_signed_content(content_sha256, &body[..])?;
-	}
+	let body = req_body.collect().await?;
 
 	let body_xml = roxmltree::Document::parse(std::str::from_utf8(&body)?)?;
 	let body_list_of_parts = parse_complete_multipart_upload_body(&body_xml)
@@ -430,7 +443,16 @@ pub async fn handle_complete_multipart_upload(
 	// Send response saying ok we're done
 	let result = s3_xml::CompleteMultipartUploadResult {
 		xmlns: (),
-		location: None,
+		// FIXME: the location returned is not always correct:
+		// - we always return https, but maybe some people do http
+		// - if root_domain is not specified, a full URL is not returned
+		location: garage
+			.config
+			.s3_api
+			.root_domain
+			.as_ref()
+			.map(|rd| s3_xml::Value(format!("https://{}.{}/{}", bucket_name, rd, key)))
+			.or(Some(s3_xml::Value(format!("/{}/{}", bucket_name, key)))),
 		bucket: s3_xml::Value(bucket_name.to_string()),
 		key: s3_xml::Value(key),
 		etag: s3_xml::Value(format!("\"{}\"", etag)),
@@ -593,3 +615,99 @@ fn parse_complete_multipart_upload_body(
 
 	Some(parts)
 }
+
+// ====== checksummer ====
+
+#[derive(Default)]
+pub(crate) struct MultipartChecksummer {
+	pub md5: Md5,
+	pub extra: Option<MultipartExtraChecksummer>,
+}
+
+pub(crate) enum MultipartExtraChecksummer {
+	Crc32(Crc32),
+	Crc32c(Crc32c),
+	Sha1(Sha1),
+	Sha256(Sha256),
+}
+
+impl MultipartChecksummer {
+	pub(crate) fn init(algo: Option<ChecksumAlgorithm>) -> Self {
+		Self {
+			md5: Md5::new(),
+			extra: match algo {
+				None => None,
+				Some(ChecksumAlgorithm::Crc32) => {
+					Some(MultipartExtraChecksummer::Crc32(Crc32::new()))
+				}
+				Some(ChecksumAlgorithm::Crc32c) => {
+					Some(MultipartExtraChecksummer::Crc32c(Crc32c::default()))
+				}
+				Some(ChecksumAlgorithm::Sha1) => Some(MultipartExtraChecksummer::Sha1(Sha1::new())),
+				Some(ChecksumAlgorithm::Sha256) => {
+					Some(MultipartExtraChecksummer::Sha256(Sha256::new()))
+				}
+			},
+		}
+	}
+
+	pub(crate) fn update(
+		&mut self,
+		etag: &str,
+		checksum: Option<ChecksumValue>,
+	) -> Result<(), Error> {
+		self.md5
+			.update(&hex::decode(&etag).ok_or_message("invalid etag hex")?);
+		match (&mut self.extra, checksum) {
+			(None, _) => (),
+			(
+				Some(MultipartExtraChecksummer::Crc32(ref mut crc32)),
+				Some(ChecksumValue::Crc32(x)),
+			) => {
+				crc32.update(&x);
+			}
+			(
+				Some(MultipartExtraChecksummer::Crc32c(ref mut crc32c)),
+				Some(ChecksumValue::Crc32c(x)),
+			) => {
+				crc32c.write(&x);
+			}
+			(Some(MultipartExtraChecksummer::Sha1(ref mut sha1)), Some(ChecksumValue::Sha1(x))) => {
+				sha1.update(&x);
+			}
+			(
+				Some(MultipartExtraChecksummer::Sha256(ref mut sha256)),
+				Some(ChecksumValue::Sha256(x)),
+			) => {
+				sha256.update(&x);
+			}
+			(Some(_), b) => {
+				return Err(Error::internal_error(format!(
+					"part checksum was not computed correctly, got: {:?}",
+					b
+				)))
+			}
+		}
+		Ok(())
+	}
+
+	pub(crate) fn finalize(self) -> (Md5Checksum, Option<ChecksumValue>) {
+		let md5 = self.md5.finalize()[..].try_into().unwrap();
+		let extra = match self.extra {
+			None => None,
+			Some(MultipartExtraChecksummer::Crc32(crc32)) => {
+				Some(ChecksumValue::Crc32(u32::to_be_bytes(crc32.finalize())))
+			}
+			Some(MultipartExtraChecksummer::Crc32c(crc32c)) => Some(ChecksumValue::Crc32c(
+				u32::to_be_bytes(u32::try_from(crc32c.finish()).unwrap()),
+			)),
+			Some(MultipartExtraChecksummer::Sha1(sha1)) => {
+				Some(ChecksumValue::Sha1(sha1.finalize()[..].try_into().unwrap()))
+			}
+			Some(MultipartExtraChecksummer::Sha256(sha256)) => Some(ChecksumValue::Sha256(
+				sha256.finalize()[..].try_into().unwrap(),
+			)),
+		};
+		(md5, extra)
+	}
+}

src/api/s3/post_object.rs

@@ -18,13 +18,13 @@ use garage_model::s3::object_table::*;
 
 use garage_api_common::cors::*;
 use garage_api_common::helpers::*;
+use garage_api_common::signature::checksum::*;
 use garage_api_common::signature::payload::{verify_v4, Authorization};
 
 use crate::api_server::ResBody;
-use crate::checksum::*;
 use crate::encryption::EncryptionParams;
 use crate::error::*;
-use crate::put::{get_headers, save_stream, ChecksumMode};
+use crate::put::{extract_metadata_headers, save_stream, ChecksumMode};
 use crate::xml as s3_xml;
 
 pub async fn handle_post_object(
@@ -216,8 +216,9 @@ pub async fn handle_post_object(
 
 	// if we ever start supporting ACLs, we likely want to map "acl" to x-amz-acl" somewhere
 	// around here to make sure the rest of the machinery takes our acl into account.
-	let headers = get_headers(&params)?;
+	let headers = extract_metadata_headers(&params)?;
 
+	let checksum_algorithm = request_checksum_algorithm(&params)?;
 	let expected_checksums = ExpectedChecksums {
 		md5: params
 			.get("content-md5")
@@ -225,7 +226,9 @@ pub async fn handle_post_object(
 			.transpose()?
 			.map(str::to_string),
 		sha256: None,
-		extra: request_checksum_algorithm_value(&params)?,
+		extra: checksum_algorithm
+			.map(|algo| extract_checksum_value(&params, algo))
+			.transpose()?,
 	};
 
 	let meta = ObjectVersionMetaInner {

src/api/s3/put.rs

@@ -31,11 +31,13 @@ use garage_model::s3::object_table::*;
 use garage_model::s3::version_table::*;
 
 use garage_api_common::helpers::*;
+use garage_api_common::signature::body::StreamingChecksumReceiver;
+use garage_api_common::signature::checksum::*;
 
 use crate::api_server::{ReqBody, ResBody};
-use crate::checksum::*;
 use crate::encryption::EncryptionParams;
 use crate::error::*;
+use crate::website::X_AMZ_WEBSITE_REDIRECT_LOCATION;
 
 const PUT_BLOCKS_MAX_PARALLEL: usize = 3;
 
@@ -48,6 +50,10 @@ pub(crate) struct SaveStreamResult {
 
 pub(crate) enum ChecksumMode<'a> {
 	Verify(&'a ExpectedChecksums),
+	VerifyFrom {
+		checksummer: StreamingChecksumReceiver,
+		trailer_algo: Option<ChecksumAlgorithm>,
+	},
 	Calculate(Option<ChecksumAlgorithm>),
 }
 
@@ -55,10 +61,9 @@ pub async fn handle_put(
 	ctx: ReqCtx,
 	req: Request<ReqBody>,
 	key: &String,
-	content_sha256: Option<Hash>,
 ) -> Result<Response<ResBody>, Error> {
 	// Retrieve interesting headers from request
-	let headers = get_headers(req.headers())?;
+	let headers = extract_metadata_headers(req.headers())?;
 	debug!("Object headers: {:?}", headers);
 
 	let expected_checksums = ExpectedChecksums {
@@ -66,9 +71,10 @@ pub async fn handle_put(
 			Some(x) => Some(x.to_str()?.to_string()),
 			None => None,
 		},
-		sha256: content_sha256,
+		sha256: None,
 		extra: request_checksum_value(req.headers())?,
 	};
+	let trailer_checksum_algorithm = request_trailer_checksum_algorithm(req.headers())?;
 
 	let meta = ObjectVersionMetaInner {
 		headers,
@@ -78,7 +84,19 @@ pub async fn handle_put(
 	// Determine whether object should be encrypted, and if so the key
 	let encryption = EncryptionParams::new_from_headers(&ctx.garage, req.headers())?;
 
-	let stream = body_stream(req.into_body());
+	// The request body is a special ReqBody object (see garage_api_common::signature::body)
+	// which supports calculating checksums while streaming the data.
+	// Before we start streaming, we configure it to calculate all the checksums we need.
+	let mut req_body = req.into_body();
+	req_body.add_expected_checksums(expected_checksums.clone());
+	if !encryption.is_encrypted() {
+		// For non-encrypted objects, we need to compute the md5sum in all cases
+		// (even if content-md5 is not set), because it is used as the object etag
+		req_body.add_md5();
+	}
+
+	let (stream, checksummer) = req_body.streaming_with_checksums();
+	let stream = stream.map_err(Error::from);
 
 	let res = save_stream(
 		&ctx,
@@ -86,7 +104,10 @@ pub async fn handle_put(
 		encryption,
 		stream,
 		key,
-		ChecksumMode::Verify(&expected_checksums),
+		ChecksumMode::VerifyFrom {
+			checksummer,
+			trailer_algo: trailer_checksum_algorithm,
+		},
 	)
 	.await?;
 
@@ -122,10 +143,15 @@ pub(crate) async fn save_stream<S: Stream<Item = Result<Bytes, Error>> + Unpin>(
 	let version_uuid = gen_uuid();
 	let version_timestamp = next_timestamp(existing_object.as_ref());
 
-	let mut checksummer = match checksum_mode {
+	let mut checksummer = match &checksum_mode {
 		ChecksumMode::Verify(expected) => Checksummer::init(expected, !encryption.is_encrypted()),
 		ChecksumMode::Calculate(algo) => {
-			Checksummer::init(&Default::default(), !encryption.is_encrypted()).add(algo)
+			Checksummer::init(&Default::default(), !encryption.is_encrypted()).add(*algo)
+		}
+		ChecksumMode::VerifyFrom { .. } => {
+			// Checksums are calculated by the garage_api_common::signature module
+			// so here we can just have an empty checksummer that does nothing
+			Checksummer::new()
 		}
 	};
 
@@ -133,7 +159,7 @@ pub(crate) async fn save_stream<S: Stream<Item = Result<Bytes, Error>> + Unpin>(
 	// as "inline data". We can then return immediately.
 	if first_block.len() < INLINE_THRESHOLD {
 		checksummer.update(&first_block);
-		let checksums = checksummer.finalize();
+		let mut checksums = checksummer.finalize();
 
 		match checksum_mode {
 			ChecksumMode::Verify(expected) => {
@@ -142,6 +168,18 @@ pub(crate) async fn save_stream<S: Stream<Item = Result<Bytes, Error>> + Unpin>(
 			ChecksumMode::Calculate(algo) => {
 				meta.checksum = checksums.extract(algo);
 			}
+			ChecksumMode::VerifyFrom {
+				checksummer,
+				trailer_algo,
+			} => {
+				drop(chunker);
+				checksums = checksummer
+					.await
+					.ok_or_internal_error("checksum calculation")??;
+				if let Some(algo) = trailer_algo {
+					meta.checksum = checksums.extract(Some(algo));
+				}
+			}
 		};
 
 		let size = first_block.len() as u64;
@@ -213,13 +251,13 @@ pub(crate) async fn save_stream<S: Stream<Item = Result<Bytes, Error>> + Unpin>(
 	garage.version_table.insert(&version).await?;
 
 	// Transfer data
-	let (total_size, checksums, first_block_hash) = read_and_put_blocks(
+	let (total_size, mut checksums, first_block_hash) = read_and_put_blocks(
 		ctx,
 		&version,
 		encryption,
 		1,
 		first_block,
-		&mut chunker,
+		chunker,
 		checksummer,
 	)
 	.await?;
@@ -232,6 +270,17 @@ pub(crate) async fn save_stream<S: Stream<Item = Result<Bytes, Error>> + Unpin>(
 		ChecksumMode::Calculate(algo) => {
 			meta.checksum = checksums.extract(algo);
 		}
+		ChecksumMode::VerifyFrom {
+			checksummer,
+			trailer_algo,
+		} => {
+			checksums = checksummer
+				.await
+				.ok_or_internal_error("checksum calculation")??;
+			if let Some(algo) = trailer_algo {
+				meta.checksum = checksums.extract(Some(algo));
+			}
+		}
 	};
 
 	// Verify quotas are respsected
@@ -332,7 +381,7 @@ pub(crate) async fn read_and_put_blocks<S: Stream<Item = Result<Bytes, Error>> +
 	encryption: EncryptionParams,
 	part_number: u64,
 	first_block: Bytes,
-	chunker: &mut StreamChunker<S>,
+	mut chunker: StreamChunker<S>,
 	checksummer: Checksummer,
 ) -> Result<(u64, Checksums, Hash), Error> {
 	let tracer = opentelemetry::global::tracer("garage");
@@ -601,7 +650,9 @@ impl Drop for InterruptedCleanup {
 
 // ============ helpers ============
 
-pub(crate) fn get_headers(headers: &HeaderMap<HeaderValue>) -> Result<HeaderList, Error> {
+pub(crate) fn extract_metadata_headers(
+	headers: &HeaderMap<HeaderValue>,
+) -> Result<HeaderList, Error> {
 	let mut ret = Vec::new();
 
 	// Preserve standard headers
@@ -627,6 +678,18 @@ pub(crate) fn get_headers(headers: &HeaderMap<HeaderValue>) -> Result<HeaderList
 				std::str::from_utf8(value.as_bytes())?.to_string(),
 			));
 		}
+		if name == X_AMZ_WEBSITE_REDIRECT_LOCATION {
+			let value = std::str::from_utf8(value.as_bytes())?.to_string();
+			if !(value.starts_with("/")
+				|| value.starts_with("http://")
+				|| value.starts_with("https://"))
+			{
+				return Err(Error::bad_request(format!(
+					"Invalid {X_AMZ_WEBSITE_REDIRECT_LOCATION} header",
+				)));
+			}
+			ret.push((X_AMZ_WEBSITE_REDIRECT_LOCATION.to_string(), value));
+		}
 	}
 
 	Ok(ret)

src/api/s3/router.rs

@@ -352,6 +352,18 @@ impl Endpoint {
 			_ => return Err(Error::bad_request("Unknown method")),
 		};
 
+		if let Some(x_id) = query.x_id.take() {
+			if x_id != res.name() {
+				// I think AWS ignores the x-id parameter.
+				// Let's make this at least be a warnin to help debugging.
+				warn!(
+					"x-id ({}) does not match parsed endpoint ({})",
+					x_id,
+					res.name()
+				);
+			}
+		}
+
 		if let Some(message) = query.nonempty_message() {
 			debug!("Unused query parameter: {}", message)
 		}
@@ -696,7 +708,8 @@ generateQueryParameters! {
 		"uploadId" => upload_id,
 		"upload-id-marker" => upload_id_marker,
 		"versionId" => version_id,
-		"version-id-marker" => version_id_marker
+		"version-id-marker" => version_id_marker,
+		"x-id" => x_id
 	]
 }
 

src/api/s3/website.rs

@@ -1,19 +1,19 @@
 use quick_xml::de::from_reader;
 
-use http_body_util::BodyExt;
-use hyper::{Request, Response, StatusCode};
+use hyper::{header::HeaderName, Request, Response, StatusCode};
 use serde::{Deserialize, Serialize};
 
 use garage_model::bucket_table::{self, *};
-use garage_util::data::*;
 
 use garage_api_common::helpers::*;
-use garage_api_common::signature::verify_signed_content;
 
 use crate::api_server::{ReqBody, ResBody};
 use crate::error::*;
 use crate::xml::{to_xml_with_header, xmlns_tag, IntValue, Value};
 
+pub const X_AMZ_WEBSITE_REDIRECT_LOCATION: HeaderName =
+	HeaderName::from_static("x-amz-website-redirect-location");
+
 pub async fn handle_get_website(ctx: ReqCtx) -> Result<Response<ResBody>, Error> {
 	let ReqCtx { bucket_params, .. } = ctx;
 	if let Some(website) = bucket_params.website_config.get() {
@@ -82,7 +82,6 @@ pub async fn handle_delete_website(ctx: ReqCtx) -> Result<Response<ResBody>, Err
 pub async fn handle_put_website(
 	ctx: ReqCtx,
 	req: Request<ReqBody>,
-	content_sha256: Option<Hash>,
 ) -> Result<Response<ResBody>, Error> {
 	let ReqCtx {
 		garage,
@@ -91,11 +90,7 @@ pub async fn handle_put_website(
 		..
 	} = ctx;
 
-	let body = BodyExt::collect(req.into_body()).await?.to_bytes();
-
-	if let Some(content_sha256) = content_sha256 {
-		verify_signed_content(content_sha256, &body[..])?;
-	}
+	let body = req.into_body().collect().await?;
 
 	let conf: WebsiteConfiguration = from_reader(&body as &[u8])?;
 	conf.validate()?;