Merge pull request 'Fix checksum handling' (#21) from bug/checksums into master

Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/21

3 files changed, 75 insertions, 45 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 330b50fc..a7cf8b56 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -60,6 +60,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b41b7ea54a0c9d92199de89e20e58d49f02f8e699814ef3fdf266f6f748d15c7"
 
 [[package]]
+name = "base64"
+version = "0.13.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "904dfeac50f3cdaba28fc6f57fdcddb75f49ed61346676a78c4ffe55877802fd"
+
+[[package]]
 name = "bitflags"
 version = "1.2.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -436,6 +442,7 @@ dependencies = [
 name = "garage_api"
 version = "0.1.1"
 dependencies = [
+ "base64 0.13.0",
  "bytes 0.4.12",
  "chrono",
  "crypto-mac",
@@ -1378,7 +1385,7 @@ version = "0.17.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "c0d4a31f5d68413404705d6982529b0e11a9aacd4839d1d6222ee3b8cb4015e1"
 dependencies = [
- "base64",
+ "base64 0.11.0",
  "log",
  "ring",
  "sct",
diff --git a/src/api/Cargo.toml b/src/api/Cargo.toml
index 578cb9d5..a366f9b8 100644
--- a/src/api/Cargo.toml
+++ b/src/api/Cargo.toml
@@ -20,6 +20,7 @@ garage_model = { version = "0.1.1", path = "../model" }
 err-derive = "0.2.3"
 bytes = "0.4"
 hex = "0.3"
+base64 = "0.13"
 log = "0.4"
 chrono = "0.4"
 md-5 = "0.9.1"
diff --git a/src/api/s3_put.rs b/src/api/s3_put.rs
index a1681d77..a528720d 100644
--- a/src/api/s3_put.rs
+++ b/src/api/s3_put.rs
@@ -4,7 +4,7 @@ use std::sync::Arc;
 
 use futures::stream::*;
 use hyper::{Body, Request, Response};
-use md5::{Digest as Md5Digest, Md5};
+use md5::{digest::generic_array::*, Digest as Md5Digest, Md5};
 use sha2::{Digest as Sha256Digest, Sha256};
 
 use garage_table::*;
@@ -48,20 +48,35 @@ pub async fn handle_put(
 	if first_block.len() < INLINE_THRESHOLD {
 		let mut md5sum = Md5::new();
 		md5sum.update(&first_block[..]);
-		let etag = hex::encode(md5sum.finalize());
+		let md5sum_arr = md5sum.finalize();
+		let md5sum_hex = hex::encode(md5sum_arr);
+
+		let mut sha256sum = Sha256::new();
+		sha256sum.input(&first_block[..]);
+		let sha256sum_arr = sha256sum.result();
+		let mut hash = [0u8; 32];
+		hash.copy_from_slice(&sha256sum_arr[..]);
+		let sha256sum_hash = Hash::from(hash);
+
+		ensure_checksum_matches(
+			md5sum_arr.as_slice(),
+			sha256sum_hash,
+			content_md5.as_deref(),
+			content_sha256,
+		)?;
 
 		object_version.state = ObjectVersionState::Complete(ObjectVersionData::Inline(
 			ObjectVersionMeta {
 				headers,
 				size: first_block.len() as u64,
-				etag: etag.clone(),
+				etag: md5sum_hex.clone(),
 			},
 			first_block,
 		));
 
 		let object = Object::new(bucket.into(), key.into(), vec![object_version]);
 		garage.object_table.insert(&object).await?;
-		return Ok(put_response(version_uuid, etag));
+		return Ok(put_response(version_uuid, md5sum_hex));
 	}
 
 	let version = Version::new(version_uuid, bucket.into(), key.into(), false, vec![]);
@@ -70,7 +85,7 @@ pub async fn handle_put(
 	let object = Object::new(bucket.into(), key.into(), vec![object_version.clone()]);
 	garage.object_table.insert(&object).await?;
 
-	let (total_size, md5sum, sha256sum) = read_and_put_blocks(
+	let (total_size, md5sum_arr, sha256sum) = read_and_put_blocks(
 		&garage,
 		version,
 		1,
@@ -80,7 +95,40 @@ pub async fn handle_put(
 	)
 	.await?;
 
-	// Validate MD5 sum against content-md5 header and sha256sum against signed content-sha256
+	ensure_checksum_matches(
+		md5sum_arr.as_slice(),
+		sha256sum,
+		content_md5.as_deref(),
+		content_sha256,
+	)?;
+
+	// TODO: if at any step we have an error, we should undo everything we did
+
+	let md5sum_hex = hex::encode(md5sum_arr);
+
+	object_version.state = ObjectVersionState::Complete(ObjectVersionData::FirstBlock(
+		ObjectVersionMeta {
+			headers,
+			size: total_size,
+			etag: md5sum_hex.clone(),
+		},
+		first_block_hash,
+	));
+
+	let object = Object::new(bucket.into(), key.into(), vec![object_version]);
+	garage.object_table.insert(&object).await?;
+
+	Ok(put_response(version_uuid, md5sum_hex))
+}
+
+/// Validate MD5 sum against content-md5 header
+/// and sha256sum against signed content-sha256
+fn ensure_checksum_matches(
+	md5sum: &[u8],
+	sha256sum: garage_util::data::FixedBytes32,
+	content_md5: Option<&str>,
+	content_sha256: Option<garage_util::data::FixedBytes32>,
+) -> Result<(), Error> {
 	if let Some(expected_sha256) = content_sha256 {
 		if expected_sha256 != sha256sum {
 			return Err(Error::BadRequest(format!(
@@ -91,28 +139,13 @@ pub async fn handle_put(
 		}
 	}
 	if let Some(expected_md5) = content_md5 {
-		if expected_md5.trim_matches('"') != md5sum {
+		if expected_md5.trim_matches('"') != base64::encode(md5sum) {
 			return Err(Error::BadRequest(format!("Unable to validate content-md5")));
 		} else {
 			trace!("Successfully validated content-md5");
 		}
 	}
-
-	// TODO: if at any step we have an error, we should undo everything we did
-
-	object_version.state = ObjectVersionState::Complete(ObjectVersionData::FirstBlock(
-		ObjectVersionMeta {
-			headers,
-			size: total_size,
-			etag: md5sum.clone(),
-		},
-		first_block_hash,
-	));
-
-	let object = Object::new(bucket.into(), key.into(), vec![object_version]);
-	garage.object_table.insert(&object).await?;
-
-	Ok(put_response(version_uuid, md5sum))
+	Ok(())
 }
 
 async fn read_and_put_blocks(
@@ -122,7 +155,7 @@ async fn read_and_put_blocks(
 	first_block: Vec<u8>,
 	first_block_hash: Hash,
 	chunker: &mut BodyChunker,
-) -> Result<(u64, String, Hash), Error> {
+) -> Result<(u64, GenericArray<u8, typenum::U16>, Hash), Error> {
 	let mut md5sum = Md5::new();
 	let mut sha256sum = Sha256::new();
 	md5sum.update(&first_block[..]);
@@ -165,14 +198,14 @@ async fn read_and_put_blocks(
 	}
 
 	let total_size = next_offset as u64;
-	let md5sum = hex::encode(md5sum.finalize());
+	let md5sum_arr = md5sum.finalize();
 
 	let sha256sum = sha256sum.result();
 	let mut hash = [0u8; 32];
 	hash.copy_from_slice(&sha256sum[..]);
 	let sha256sum = Hash::from(hash);
 
-	Ok((total_size, md5sum, sha256sum))
+	Ok((total_size, md5sum_arr, sha256sum))
 }
 
 async fn put_block_meta(
@@ -338,7 +371,7 @@ pub async fn handle_put_part(
 	// Copy block to store
 	let version = Version::new(version_uuid, bucket.into(), key.into(), false, vec![]);
 	let first_block_hash = hash(&first_block[..]);
-	let (_, md5sum, sha256sum) = read_and_put_blocks(
+	let (_, md5sum_arr, sha256sum) = read_and_put_blocks(
 		&garage,
 		version,
 		part_number,
@@ -348,23 +381,12 @@ pub async fn handle_put_part(
 	)
 	.await?;
 
-	// Validate MD5 sum against content-md5 header and sha256sum against signed content-sha256
-	if let Some(expected_sha256) = content_sha256 {
-		if expected_sha256 != sha256sum {
-			return Err(Error::BadRequest(format!(
-				"Unable to validate x-amz-content-sha256"
-			)));
-		} else {
-			trace!("Successfully validated x-amz-content-sha256");
-		}
-	}
-	if let Some(expected_md5) = content_md5 {
-		if expected_md5.trim_matches('"') != md5sum {
-			return Err(Error::BadRequest(format!("Unable to validate content-md5")));
-		} else {
-			trace!("Successfully validated content-md5");
-		}
-	}
+	ensure_checksum_matches(
+		md5sum_arr.as_slice(),
+		sha256sum,
+		content_md5.as_deref(),
+		content_sha256,
+	)?;
 
 	Ok(Response::new(Body::from(vec![])))
 }