Deuxfleurs/garage
48
56
Fork 73
Code Issues 100 Pull requests 13 Releases 30 Wiki Activity

Add verification of part numbers in CompleteMultipartUpload (WIP #30)
All checks were successful
continuous-integration/drone/push Build is passing
Details

Browse source
This commit is contained in:
Alex 2021-02-20 00:13:07 +01:00
parent 1de96248e0
commit 10b983b8e7
4 changed files with 77 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/api/api_server.rs
View file

@ -157,7 +157,7 @@ async fn handler_inner(garage: Arc<Garage>, req: Request<Body>) -> Result<Respon
// CompleteMultipartUpload call // CompleteMultipartUpload call
let upload_id = params.get("uploadid").unwrap(); let upload_id = params.get("uploadid").unwrap();
Ok( Ok(
handle_complete_multipart_upload(garage, req, &bucket, &key, upload_id) handle_complete_multipart_upload(garage, req, &bucket, &key, upload_id, content_sha256)
.await?, .await?,
) )
} else { } else {
@ -205,7 +205,7 @@ async fn handler_inner(garage: Arc<Garage>, req: Request<Body>) -> Result<Respon
&Method::POST => { &Method::POST => {
if params.contains_key(&"delete".to_string()) { if params.contains_key(&"delete".to_string()) {
// DeleteObjects // DeleteObjects
Ok(handle_delete_objects(garage, bucket, req).await?) Ok(handle_delete_objects(garage, bucket, req, content_sha256).await?)
} else { } else {
debug!( debug!(
"Body: {}", "Body: {}",

30
src/api/s3_delete.rs
View file

@ -10,6 +10,7 @@ use garage_model::object_table::*;
use crate::encoding::*; use crate::encoding::*;
use crate::error::*; use crate::error::*;
use crate::signature::verify_signed_content;
async fn handle_delete_internal( async fn handle_delete_internal(
garage: &Garage, garage: &Garage,
@ -73,8 +74,11 @@ pub async fn handle_delete_objects(
garage: Arc<Garage>, garage: Arc<Garage>,
bucket: &str, bucket: &str,
req: Request<Body>, req: Request<Body>,
content_sha256: Option<Hash>,
) -> Result<Response<Body>, Error> { ) -> Result<Response<Body>, Error> {
let body = hyper::body::to_bytes(req.into_body()).await?; let body = hyper::body::to_bytes(req.into_body()).await?;
verify_signed_content(content_sha256, &body[..])?;
let cmd_xml = roxmltree::Document::parse(&std::str::from_utf8(&body)?)?; 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")?; let cmd = parse_delete_objects_xml(&cmd_xml).ok_or_bad_request("Invalid delete XML query")?;
@ -131,33 +135,27 @@ struct DeleteObject {
key: String, key: String,
} }
fn parse_delete_objects_xml(xml: &roxmltree::Document) -> Result<DeleteRequest, String> { fn parse_delete_objects_xml(xml: &roxmltree::Document) -> Option<DeleteRequest> {
let mut ret = DeleteRequest { objects: vec![] }; let mut ret = DeleteRequest { objects: vec![] };
let root = xml.root(); let root = xml.root();
let delete = root.first_child().ok_or(format!("Delete tag not found"))?; let delete = root.first_child()?;
if !delete.has_tag_name("Delete") { if !delete.has_tag_name("Delete") {
return Err(format!("Invalid root tag: {:?}", root)); return None;
} }
for item in delete.children() { for item in delete.children() {
if item.has_tag_name("Object") { if item.has_tag_name("Object") {
if let Some(key) = item.children().find(|e| e.has_tag_name("Key")) { let key = item.children().find(|e| e.has_tag_name("Key"))?;
if let Some(key_str) = key.text() { let key_str = key.text()?;
ret.objects.push(DeleteObject { ret.objects.push(DeleteObject {
key: key_str.to_string(), key: key_str.to_string(),
}); });
} else {
return Err(format!("No text for key: {:?}", key));
}
} else {
return Err(format!("No delete key for item: {:?}", item));
}
} else { } else {
return Err(format!("Invalid delete item: {:?}", item)); return None;
} }
} }
Ok(ret) Some(ret)
} }

59
src/api/s3_put.rs
View file

@ -11,14 +11,15 @@ use garage_table::*;
use garage_util::data::*; use garage_util::data::*;
use garage_util::error::Error as GarageError; use garage_util::error::Error as GarageError;
use crate::error::*;
use garage_model::block::INLINE_THRESHOLD; use garage_model::block::INLINE_THRESHOLD;
use garage_model::block_ref_table::*; use garage_model::block_ref_table::*;
use garage_model::garage::Garage; use garage_model::garage::Garage;
use garage_model::object_table::*; use garage_model::object_table::*;
use garage_model::version_table::*; use garage_model::version_table::*;
use crate::error::*;
use crate::encoding::*; use crate::encoding::*;
use crate::signature::verify_signed_content;
pub async fn handle_put( pub async fn handle_put(
garage: Arc<Garage>, garage: Arc<Garage>,
@ -416,11 +417,19 @@ pub async fn handle_put_part(
pub async fn handle_complete_multipart_upload( pub async fn handle_complete_multipart_upload(
garage: Arc<Garage>, garage: Arc<Garage>,
_req: Request<Body>, req: Request<Body>,
bucket: &str, bucket: &str,
key: &str, key: &str,
upload_id: &str, upload_id: &str,
content_sha256: Option<Hash>,
) -> Result<Response<Body>, Error> { ) -> Result<Response<Body>, Error> {
let body = hyper::body::to_bytes(req.into_body()).await?;
verify_signed_content(content_sha256, &body[..])?;
let body_xml = roxmltree::Document::parse(&std::str::from_utf8(&body)?)?;
let body_list_of_parts = parse_complete_multpart_upload_body(&body_xml).ok_or_bad_request("Invalid CompleteMultipartUpload XML")?;
debug!("CompleteMultipartUpload list of parts: {:?}", body_list_of_parts);
let version_uuid = decode_upload_id(upload_id)?; let version_uuid = decode_upload_id(upload_id)?;
let bucket = bucket.to_string(); let bucket = bucket.to_string();
@ -450,6 +459,16 @@ pub async fn handle_complete_multipart_upload(
_ => unreachable!(), _ => unreachable!(),
}; };
// Check that the list of parts they gave us corresponds to the parts we have here
// TODO: check MD5 sum of all uploaded parts? but that would mean we have to store them somewhere...
let mut parts = version.blocks().iter().map(|x| x.part_number)
.collect::<Vec<_>>();
parts.dedup();
let same_parts = body_list_of_parts.iter().map(|x| &x.part_number).eq(parts.iter());
if !same_parts {
return Err(Error::BadRequest(format!("We don't have the same parts")));
}
// ETag calculation: we produce ETags that have the same form as // ETag calculation: we produce ETags that have the same form as
// those of S3 multipart uploads, but we don't use their actual // those of S3 multipart uploads, but we don't use their actual
// calculation for the first part (we use random bytes). This // calculation for the first part (we use random bytes). This
@ -465,11 +484,6 @@ pub async fn handle_complete_multipart_upload(
num_parts num_parts
); );
// TODO: check that all the parts that they pretend they gave us are indeed there
// TODO: when we read the XML from _req, remember to check the sha256 sum of the payload
// against the signed x-amz-content-sha256
// TODO: check MD5 sum of all uploaded parts? but that would mean we have to store them somewhere...
let total_size = version let total_size = version
.blocks() .blocks()
.iter() .iter()
@ -583,3 +597,34 @@ fn decode_upload_id(id: &str) -> Result<UUID, Error> {
uuid.copy_from_slice(&id_bin[..]); uuid.copy_from_slice(&id_bin[..]);
Ok(UUID::from(uuid)) Ok(UUID::from(uuid))
} }
#[derive(Debug)]
struct CompleteMultipartUploadPart {
etag: String,
part_number: u64,
}
fn parse_complete_multpart_upload_body(xml: &roxmltree::Document) -> Option<Vec<CompleteMultipartUploadPart>> {
let mut parts = vec![];
let root = xml.root();
let cmu = root.first_child()?;
if !cmu.has_tag_name("CompleteMultipartUpload") {
return None;
}
for item in cmu.children() {
if item.has_tag_name("Part") {
let etag = item.children().find(|e| e.has_tag_name("ETag"))?.text()?;
let part_number = item.children().find(|e| e.has_tag_name("PartNumber"))?.text()?;
parts.push(CompleteMultipartUploadPart{
etag: etag.trim_matches('"').to_string(),
part_number: part_number.parse().ok()?,
});
} else {
return None;
}
}
Some(parts)
}

10
src/api/signature.rs
View file

@ -6,7 +6,7 @@ use hyper::{Body, Method, Request};
use sha2::{Digest, Sha256}; use sha2::{Digest, Sha256};
use garage_table::*; use garage_table::*;
use garage_util::data::Hash; use garage_util::data::{hash, Hash};
use garage_model::garage::Garage; use garage_model::garage::Garage;
use garage_model::key_table::*; use garage_model::key_table::*;
@ -293,3 +293,11 @@ fn canonical_query_string(uri: &hyper::Uri) -> String {
"".to_string() "".to_string()
} }
} }
pub fn verify_signed_content(content_sha256: Option<Hash>, body: &[u8]) -> Result<(), Error> {
let expected_sha256 = content_sha256.ok_or_bad_request("Request content hash not signed, aborting.")?;
if expected_sha256 != hash(body) {
return Err(Error::BadRequest(format!("Request content hash does not match signed hash")));
}
Ok(())
}