From 6d798c640f523c0eb1653be9c7d89114a75b3fc3 Mon Sep 17 00:00:00 2001 From: Baptiste Jonglez Date: Sun, 26 Jan 2025 20:40:02 +0100 Subject: [PATCH 01/43] WIP: fix crash in layout computation when changing all nodes of a zone to gateway mode This change is probably not a proper fix, somebody with more expertise on this code should look at it. Here is how to reproduce the crash: - start with a layout with two zones - move all nodes of a zone to gateway mode: `garage layout assign fea54bcc081f318 -g` - `garage layout show` will panic with a backtrace Fortunately, the crash is only on the RPC client side, not on the Garage server itself, and `garage layout revert` still works to go back to the previous state. As far as I can tell, this bug is present since Garage 0.9.0 which includes the new layout assignation algorithm: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/296 --- src/rpc/layout/version.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/rpc/layout/version.rs b/src/rpc/layout/version.rs index a569c7c6..b7902898 100644 --- a/src/rpc/layout/version.rs +++ b/src/rpc/layout/version.rs @@ -650,8 +650,11 @@ impl LayoutVersion { let mut cost = CostFunction::new(); for (p, assoc_p) in prev_assign.iter().enumerate() { for n in assoc_p.iter() { - let node_zone = zone_to_id[self.expect_get_node_zone(&self.node_id_vec[*n])]; - cost.insert((Vertex::PZ(p, node_zone), Vertex::N(*n)), -1); + if let Some(&node_zone) = + zone_to_id.get(self.expect_get_node_zone(&self.node_id_vec[*n])) + { + cost.insert((Vertex::PZ(p, node_zone), Vertex::N(*n)), -1); + } } } @@ -751,8 +754,11 @@ impl LayoutVersion { if let Some(prev_assign) = prev_assign_opt { let mut old_zones_of_p = Vec::::new(); for n in prev_assign[p].iter() { - old_zones_of_p - .push(zone_to_id[self.expect_get_node_zone(&self.node_id_vec[*n])]); + if let Some(&zone_id) = + zone_to_id.get(self.expect_get_node_zone(&self.node_id_vec[*n])) + { + old_zones_of_p.push(zone_id); + } } if !old_zones_of_p.contains(&z) { new_partitions_zone[z] += 1; From 06aa4b604fe0a9b5230bc0626d883e5b37953dec Mon Sep 17 00:00:00 2001 From: Baptiste Jonglez Date: Fri, 24 Jan 2025 19:24:09 +0100 Subject: [PATCH 02/43] db-snapshot: Fix error reporting when using "garage meta snapshot --all" Snapshot errors on remote nodes were not reported at all. We now get proper error output such as: 0fa0f35be69528ab error: Internal error: DB error: LMDB: No space left on device (os error 28) 88d92e2971d14bae ok Fix #920 --- src/garage/admin/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/garage/admin/mod.rs b/src/garage/admin/mod.rs index ea414b56..176911fb 100644 --- a/src/garage/admin/mod.rs +++ b/src/garage/admin/mod.rs @@ -484,7 +484,7 @@ impl AdminRpcHandler { AdminRpc::MetaOperation(MetaOperation::Snapshot { all: false }), PRIO_NORMAL, ) - .await + .await? })) .await; From a2e134f036a5bdeca55ae0ce6d731d1ec37a454c Mon Sep 17 00:00:00 2001 From: Baptiste Jonglez Date: Fri, 24 Jan 2025 19:21:08 +0100 Subject: [PATCH 03/43] db-snapshot: propagate any node snapshot error through RPC call In particular, it means that "garage meta snapshot --all" will get an exit code of 1 if any node fails to snapshot. This makes sure that any external tool trying to snapshot nodes (e.g. from cron) will be aware of the failure. Fix #920 --- src/garage/admin/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/garage/admin/mod.rs b/src/garage/admin/mod.rs index 176911fb..1a4ff853 100644 --- a/src/garage/admin/mod.rs +++ b/src/garage/admin/mod.rs @@ -497,7 +497,11 @@ impl AdminRpcHandler { ret.push(format!("{:?}\t{}", to, res_str)); } - Ok(AdminRpc::Ok(format_table_to_string(ret))) + if resps.iter().any(Result::is_err) { + Err(GarageError::Message(format_table_to_string(ret)).into()) + } else { + Ok(AdminRpc::Ok(format_table_to_string(ret))) + } } MetaOperation::Snapshot { all: false } => { garage_model::snapshot::async_snapshot_metadata(&self.garage).await?; From d84308c413ce3e7d84678170f51384e25341031e Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 14:11:23 +0100 Subject: [PATCH 04/43] cli: return info of all nodes when doing garage stats -a (fix #814) --- src/garage/admin/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/garage/admin/mod.rs b/src/garage/admin/mod.rs index 1a4ff853..3bbc2b86 100644 --- a/src/garage/admin/mod.rs +++ b/src/garage/admin/mod.rs @@ -146,7 +146,12 @@ impl AdminRpcHandler { async fn handle_stats(&self, opt: StatsOpt) -> Result { if opt.all_nodes { let mut ret = String::new(); - let all_nodes = self.garage.system.cluster_layout().all_nodes().to_vec(); + let mut all_nodes = self.garage.system.cluster_layout().all_nodes().to_vec(); + for node in self.garage.system.get_known_nodes().iter() { + if node.is_up && !all_nodes.contains(&node.id) { + all_nodes.push(node.id); + } + } for node in all_nodes.iter() { let mut opt = opt.clone(); From c1b39d9ba15ffef5ba4cb1d3e5eac89c670acb05 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 14:30:46 +0100 Subject: [PATCH 05/43] s3 api: parse x-id query parameter and warn of any inconsistency (fix #822) --- src/api/s3/router.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/api/s3/router.rs b/src/api/s3/router.rs index 9de84b2b..e3f58490 100644 --- a/src/api/s3/router.rs +++ b/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 ] } From 8b9cc5ca3fea35ee54e309bde5a0a651c3b31322 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Wed, 2 Aug 2023 14:30:04 +0100 Subject: [PATCH 06/43] web_server.rs: Added bucket domain to observability. --- src/web/web_server.rs | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/src/web/web_server.rs b/src/web/web_server.rs index 48dcb5b1..2ab3f53b 100644 --- a/src/web/web_server.rs +++ b/src/web/web_server.rs @@ -120,18 +120,34 @@ impl WebServer { req: Request, addr: String, ) -> Result>, http::Error> { + let request_host_bucket = req + .headers() + .get(HOST) + .expect("No host header found") + .to_str() + .expect("Error converting host header to string") + .to_string(); + if let Ok(forwarded_for_ip_addr) = forwarded_headers::handle_forwarded_for_headers(req.headers()) { + // uri() below has a preceding '/', so no space with host info!( - "{} (via {}) {} {}", + "{} (via {}) {} {}{}", forwarded_for_ip_addr, addr, req.method(), + request_host_bucket, req.uri() ); } else { - info!("{} {} {}", addr, req.method(), req.uri()); + info!( + "{} {} {}{}", + addr, + req.method(), + request_host_bucket, + req.uri() + ); } // Lots of instrumentation @@ -140,12 +156,16 @@ impl WebServer { .span_builder(format!("Web {} request", req.method())) .with_trace_id(gen_trace_id()) .with_attributes(vec![ + KeyValue::new("domain", format!("{}", request_host_bucket.to_string())), KeyValue::new("method", format!("{}", req.method())), KeyValue::new("uri", req.uri().to_string()), ]) .start(&tracer); - let metrics_tags = &[KeyValue::new("method", req.method().to_string())]; + let metrics_tags = &[ + KeyValue::new("domain", request_host_bucket.to_string()), + KeyValue::new("method", req.method().to_string()), + ]; // The actual handler let res = self @@ -160,22 +180,30 @@ impl WebServer { // Returning the result match res { Ok(res) => { - debug!("{} {} {}", req.method(), res.status(), req.uri()); + debug!( + "{} {} {}{}", + req.method(), + res.status(), + request_host_bucket, + req.uri() + ); Ok(res .map(|body| BoxBody::new(http_body_util::BodyExt::map_err(body, Error::from)))) } Err(error) => { info!( - "{} {} {} {}", + "{} {} {}{} {}", req.method(), error.http_status_code(), + request_host_bucket, req.uri(), error ); self.metrics.error_counter.add( 1, &[ - metrics_tags[0].clone(), + KeyValue::new("domain", request_host_bucket.to_string()), + KeyValue::new("method", req.method().to_string()), KeyValue::new("status_code", error.http_status_code().to_string()), ], ); From 2f558898354a668b73579d7e09a0ecc922e9dcaf Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 14:54:06 +0100 Subject: [PATCH 07/43] add configuration option to enable/disable monitoring bucket in web metrics --- doc/book/reference-manual/configuration.md | 9 ++++ src/garage/server.rs | 2 +- src/util/config.rs | 3 ++ src/web/web_server.rs | 51 ++++++++++------------ 4 files changed, 35 insertions(+), 30 deletions(-) diff --git a/doc/book/reference-manual/configuration.md b/doc/book/reference-manual/configuration.md index f0a3b438..f545de29 100644 --- a/doc/book/reference-manual/configuration.md +++ b/doc/book/reference-manual/configuration.md @@ -75,6 +75,7 @@ root_domain = ".s3.garage" [s3_web] bind_addr = "[::]:3902" root_domain = ".web.garage" +add_host_to_metrics = true [admin] api_bind_addr = "0.0.0.0:3903" @@ -138,6 +139,7 @@ The `[s3_api]` section: [`s3_region`](#s3_region). The `[s3_web]` section: +[`add_host_to_metrics`](#web_add_host_to_metrics), [`bind_addr`](#web_bind_addr), [`root_domain`](#web_root_domain). @@ -744,6 +746,13 @@ For instance, if `root_domain` is `web.garage.eu`, a bucket called `deuxfleurs.f will be accessible either with hostname `deuxfleurs.fr.web.garage.eu` or with hostname `deuxfleurs.fr`. +#### `add_host_to_metrics` {#web_add_host_to_metrics} + +Whether to include the requested domain name (HTTP `Host` header) in the +Prometheus metrics of the web endpoint. This is disabled by default as the +number of possible values is not bounded and can be a source of cardinality +explosion in the exported metrics. + ### The `[admin]` section diff --git a/src/garage/server.rs b/src/garage/server.rs index 9e58fa6d..1dc86fd3 100644 --- a/src/garage/server.rs +++ b/src/garage/server.rs @@ -113,7 +113,7 @@ pub async fn run_server(config_file: PathBuf, secrets: Secrets) -> Result<(), Er if let Some(web_config) = &config.s3_web { info!("Initializing web server..."); - let web_server = WebServer::new(garage.clone(), web_config.root_domain.clone()); + let web_server = WebServer::new(garage.clone(), &web_config); servers.push(( "Web", tokio::spawn(web_server.run(web_config.bind_addr.clone(), watch_cancel.clone())), diff --git a/src/util/config.rs b/src/util/config.rs index b4e2b008..73fc4ff4 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -183,6 +183,9 @@ pub struct WebConfig { pub bind_addr: UnixOrTCPSocketAddress, /// Suffix to remove from domain name to find bucket pub root_domain: String, + /// Whether to add the requested domain to exported Prometheus metrics + #[serde(default)] + pub add_host_to_metrics: bool, } /// Configuration for the admin and monitoring HTTP API diff --git a/src/web/web_server.rs b/src/web/web_server.rs index 2ab3f53b..e73dab48 100644 --- a/src/web/web_server.rs +++ b/src/web/web_server.rs @@ -33,6 +33,7 @@ use garage_api_s3::get::{handle_get_without_ctx, handle_head_without_ctx}; use garage_model::garage::Garage; use garage_table::*; +use garage_util::config::WebConfig; use garage_util::data::Uuid; use garage_util::error::Error as GarageError; use garage_util::forwarded_headers; @@ -69,16 +70,18 @@ pub struct WebServer { garage: Arc, metrics: Arc, root_domain: String, + add_host_to_metrics: bool, } impl WebServer { /// Run a web server - pub fn new(garage: Arc, root_domain: String) -> Arc { + pub fn new(garage: Arc, config: &WebConfig) -> Arc { let metrics = Arc::new(WebMetrics::new()); Arc::new(WebServer { garage, metrics, - root_domain, + root_domain: config.root_domain.clone(), + add_host_to_metrics: config.add_host_to_metrics, }) } @@ -120,12 +123,11 @@ impl WebServer { req: Request, addr: String, ) -> Result>, http::Error> { - let request_host_bucket = req + let host_header = req .headers() .get(HOST) - .expect("No host header found") - .to_str() - .expect("Error converting host header to string") + .and_then(|x| x.to_str().ok()) + .unwrap_or("") .to_string(); if let Ok(forwarded_for_ip_addr) = @@ -137,17 +139,11 @@ impl WebServer { forwarded_for_ip_addr, addr, req.method(), - request_host_bucket, + host_header, req.uri() ); } else { - info!( - "{} {} {}{}", - addr, - req.method(), - request_host_bucket, - req.uri() - ); + info!("{} {} {}{}", addr, req.method(), host_header, req.uri()); } // Lots of instrumentation @@ -156,16 +152,16 @@ impl WebServer { .span_builder(format!("Web {} request", req.method())) .with_trace_id(gen_trace_id()) .with_attributes(vec![ - KeyValue::new("domain", format!("{}", request_host_bucket.to_string())), + KeyValue::new("host", format!("{}", host_header.clone())), KeyValue::new("method", format!("{}", req.method())), KeyValue::new("uri", req.uri().to_string()), ]) .start(&tracer); - let metrics_tags = &[ - KeyValue::new("domain", request_host_bucket.to_string()), - KeyValue::new("method", req.method().to_string()), - ]; + let mut metrics_tags = vec![KeyValue::new("method", req.method().to_string())]; + if self.add_host_to_metrics { + metrics_tags.push(KeyValue::new("host", host_header.clone())); + } // The actual handler let res = self @@ -184,7 +180,7 @@ impl WebServer { "{} {} {}{}", req.method(), res.status(), - request_host_bucket, + host_header, req.uri() ); Ok(res @@ -195,18 +191,15 @@ impl WebServer { "{} {} {}{} {}", req.method(), error.http_status_code(), - request_host_bucket, + host_header, req.uri(), error ); - self.metrics.error_counter.add( - 1, - &[ - KeyValue::new("domain", request_host_bucket.to_string()), - KeyValue::new("method", req.method().to_string()), - KeyValue::new("status_code", error.http_status_code().to_string()), - ], - ); + metrics_tags.push(KeyValue::new( + "status_code", + error.http_status_code().to_string(), + )); + self.metrics.error_counter.add(1, &metrics_tags); Ok(error_to_res(error)) } } From d0104b9f9bd96e008fe3fbe8e5658cae605525b1 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 16:14:59 +0100 Subject: [PATCH 08/43] block manager: write blocks only to currently active layout version (fix #815) avoid wastefully writing blocks to nodes that will discard them as soon as the layout migration is finished --- src/block/manager.rs | 4 ++-- src/rpc/layout/helper.rs | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/block/manager.rs b/src/block/manager.rs index 537e1fc1..572bdadd 100644 --- a/src/block/manager.rs +++ b/src/block/manager.rs @@ -370,7 +370,7 @@ impl BlockManager { prevent_compression: bool, order_tag: Option, ) -> Result<(), Error> { - let who = self.replication.write_sets(&hash); + let who = self.system.cluster_layout().current_storage_nodes_of(&hash); let compression_level = self.compression_level.filter(|_| !prevent_compression); let (header, bytes) = DataBlock::from_buffer(data, compression_level) @@ -396,7 +396,7 @@ impl BlockManager { .rpc_helper() .try_write_many_sets( &self.endpoint, - who.as_ref(), + &[who], put_block_rpc, RequestStrategy::with_priority(PRIO_NORMAL | PRIO_SECONDARY) .with_drop_on_completion(permit) diff --git a/src/rpc/layout/helper.rs b/src/rpc/layout/helper.rs index 44c826f9..c08a5629 100644 --- a/src/rpc/layout/helper.rs +++ b/src/rpc/layout/helper.rs @@ -219,6 +219,11 @@ impl LayoutHelper { ret } + pub fn current_storage_nodes_of(&self, position: &Hash) -> Vec { + let ver = self.current(); + ver.nodes_of(position, ver.replication_factor).collect() + } + pub fn trackers_hash(&self) -> Hash { self.trackers_hash } From 6820b69f305728cab10065e403afd94a240c7c46 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 16:15:44 +0100 Subject: [PATCH 09/43] block manager: improve read strategy to find blocks faster --- src/rpc/rpc_helper.rs | 73 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 9 deletions(-) diff --git a/src/rpc/rpc_helper.rs b/src/rpc/rpc_helper.rs index b8ca8120..2505c2ce 100644 --- a/src/rpc/rpc_helper.rs +++ b/src/rpc/rpc_helper.rs @@ -540,19 +540,73 @@ impl RpcHelper { // ---- functions not related to MAKING RPCs, but just determining to what nodes // they should be made and in which order ---- + /// Determine to what nodes, and in what order, requests to read a data block + /// should be sent. All nodes in the Vec returned by this function are tried + /// one by one until there is one that returns the block (in block/manager.rs). + /// + /// We want to have the best chance of finding the block in as few requests + /// as possible, and we want to avoid nodes that answer slowly. + /// + /// Note that when there are several active layout versions, the block might + /// be stored only by nodes of the latest version (in case of a block that was + /// written after the layout change), or only by nodes of the oldest active + /// version (for all blocks that were written before). So we have to try nodes + /// of all layout versions. We also want to try nodes of all layout versions + /// fast, so as to optimize the chance of finding the block fast. + /// + /// Therefore, the strategy is the following: + /// + /// 1. ask first all nodes of all currently active layout versions + /// -> ask the preferred node in all layout versions (older to newer), + /// then the second preferred onde in all verions, etc. + /// -> we start by the oldest active layout version first, because a majority + /// of blocks will have been saved before the layout change + /// 2. ask all nodes of historical layout versions, for blocks which have not + /// yet been transferred to their new storage nodes + /// + /// The preference order, for each layout version, is given by `request_order`, + /// based on factors such as nodes being in the same datacenter, + /// having low ping, etc. pub fn block_read_nodes_of(&self, position: &Hash, rpc_helper: &RpcHelper) -> Vec { let layout = self.0.layout.read().unwrap(); - let mut ret = Vec::with_capacity(12); - let ver_iter = layout - .versions() - .iter() - .rev() - .chain(layout.inner().old_versions.iter().rev()); - for ver in ver_iter { - if ver.version > layout.sync_map_min() { - continue; + // Compute, for each layout version, the set of nodes that might store + // the block, and put them in their preferred order as of `request_order`. + let mut vernodes = layout.versions().iter().map(|ver| { + let nodes = ver.nodes_of(position, ver.replication_factor); + rpc_helper.request_order(layout.current(), nodes) + }); + + let mut ret = if layout.versions().len() == 1 { + // If we have only one active layout version, then these are the + // only nodes we ask in step 1 + vernodes.next().unwrap() + } else { + let vernodes = vernodes.collect::>(); + + let mut nodes = Vec::::with_capacity(12); + for i in 0..layout.current().replication_factor { + for vn in vernodes.iter() { + if let Some(n) = vn.get(i) { + if !nodes.contains(&n) { + if *n == self.0.our_node_id { + // it's always fast (almost free) to ask locally, + // so always put that as first choice + nodes.insert(0, *n); + } else { + nodes.push(*n); + } + } + } + } } + + nodes + }; + + // Second step: add nodes of older layout versions + let old_ver_iter = layout.inner().old_versions.iter().rev(); + for ver in old_ver_iter { let nodes = ver.nodes_of(position, ver.replication_factor); for node in rpc_helper.request_order(layout.current(), nodes) { if !ret.contains(&node) { @@ -560,6 +614,7 @@ impl RpcHelper { } } } + ret } From fdf4dad72833264be6bc4bae70f4e743e656d2df Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 16:27:12 +0100 Subject: [PATCH 10/43] block resync: avoid saving blocks to draining nodes --- src/block/resync.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/block/resync.rs b/src/block/resync.rs index 947c68de..b476a0b8 100644 --- a/src/block/resync.rs +++ b/src/block/resync.rs @@ -377,7 +377,10 @@ impl BlockResyncManager { info!("Resync block {:?}: offloading and deleting", hash); let existing_path = existing_path.unwrap(); - let mut who = manager.replication.storage_nodes(hash); + let mut who = manager + .system + .cluster_layout() + .current_storage_nodes_of(hash); if who.len() < manager.replication.write_quorum() { return Err(Error::Message("Not trying to offload block because we don't have a quorum of nodes to write to".to_string())); } @@ -455,6 +458,25 @@ impl BlockResyncManager { } if rc.is_nonzero() && !exists { + // The refcount is > 0, and the block is not present locally. + // We might need to fetch it from another node. + + // First, check whether we are still supposed to store that + // block in the latest cluster layout version. + let storage_nodes = manager + .system + .cluster_layout() + .current_storage_nodes_of(&hash); + + if !storage_nodes.contains(&manager.system.id) { + info!( + "Resync block {:?}: block is absent with refcount > 0, but it will drop to zero after all metadata is synced. Not fetching the block.", + hash + ); + return Ok(()); + } + + // We know we need the block. Fetch it. info!( "Resync block {:?}: fetching absent but needed block (refcount > 0)", hash From e4c9a8cd5391ced427cb69ebd5f96e4482cc1536 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 17:41:50 +0100 Subject: [PATCH 11/43] block manager: avoid deadlock in fix_block_location (fix #845) --- src/block/manager.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/block/manager.rs b/src/block/manager.rs index 572bdadd..41b2f02a 100644 --- a/src/block/manager.rs +++ b/src/block/manager.rs @@ -668,10 +668,12 @@ impl BlockManager { hash: &Hash, wrong_path: DataBlockPath, ) -> Result { + let data = self.read_block_from(hash, &wrong_path).await?; self.lock_mutate(hash) .await - .fix_block_location(hash, wrong_path, self) - .await + .write_block_inner(hash, &data, self, Some(wrong_path)) + .await?; + Ok(data.as_parts_ref().1.len()) } async fn lock_mutate(&self, hash: &Hash) -> MutexGuard<'_, BlockManagerLocked> { @@ -827,18 +829,6 @@ impl BlockManagerLocked { } Ok(()) } - - async fn fix_block_location( - &self, - hash: &Hash, - wrong_path: DataBlockPath, - mgr: &BlockManager, - ) -> Result { - let data = mgr.read_block_from(hash, &wrong_path).await?; - self.write_block_inner(hash, &data, mgr, Some(wrong_path)) - .await?; - Ok(data.as_parts_ref().1.len()) - } } struct DeleteOnDrop(Option); From 165f9316e2c1414f27d04fd7c86bf628dc6a096d Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 18:02:49 +0100 Subject: [PATCH 12/43] s3api: return Location in CompleteMultipartUpload (fix #852) NB. The location returned is not guaranteed to work in all cases. This already fixes the parse issue in #852. --- src/api/s3/multipart.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index fe39fc93..fa053df2 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -430,7 +430,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)), From 9c7e3c7bde954faace17b8c60fa8cfcce3a34b08 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 18:05:57 +0100 Subject: [PATCH 13/43] remove cargo build options in makefile to avoid mistakes --- Makefile | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 22e1f548..35c3f22c 100644 --- a/Makefile +++ b/Makefile @@ -2,9 +2,7 @@ all: clear - cargo build \ - --config 'target.x86_64-unknown-linux-gnu.linker="clang"' \ - --config 'target.x86_64-unknown-linux-gnu.rustflags=["-C", "link-arg=-fuse-ld=mold"]' \ + cargo build # ---- From 5b26545abf1837e084cc8bbb80373809fe6f3949 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 18:08:23 +0100 Subject: [PATCH 14/43] fix deprecated uses of chrono in lifecycle worker --- src/model/s3/lifecycle_worker.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/model/s3/lifecycle_worker.rs b/src/model/s3/lifecycle_worker.rs index 38212a1c..bb10ba48 100644 --- a/src/model/s3/lifecycle_worker.rs +++ b/src/model/s3/lifecycle_worker.rs @@ -395,13 +395,13 @@ fn midnight_ts(date: NaiveDate, use_local_tz: bool) -> u64 { .expect("bad local midnight") .timestamp_millis() as u64; } - midnight.timestamp_millis() as u64 + midnight.and_utc().timestamp_millis() as u64 } fn next_date(ts: u64) -> NaiveDate { - NaiveDateTime::from_timestamp_millis(ts as i64) + DateTime::::from_timestamp_millis(ts as i64) .expect("bad timestamp") - .date() + .date_naive() .succ_opt() .expect("no next day") } From 24470377c93bba8edcfa0339779e3b911852bab4 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 18:11:44 +0100 Subject: [PATCH 15/43] garage_model: fix warning about dead code --- src/model/garage.rs | 2 +- src/model/helper/locked.rs | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/model/garage.rs b/src/model/garage.rs index 29e0bddd..11c0d90f 100644 --- a/src/model/garage.rs +++ b/src/model/garage.rs @@ -329,7 +329,7 @@ impl Garage { pub async fn locked_helper(&self) -> helper::locked::LockedHelper { let lock = self.bucket_lock.lock().await; - helper::locked::LockedHelper(self, lock) + helper::locked::LockedHelper(self, Some(lock)) } } diff --git a/src/model/helper/locked.rs b/src/model/helper/locked.rs index 43f4f363..482e91b0 100644 --- a/src/model/helper/locked.rs +++ b/src/model/helper/locked.rs @@ -27,9 +27,16 @@ use crate::permission::BucketKeyPerm; /// See issues: #649, #723 pub struct LockedHelper<'a>( pub(crate) &'a Garage, - pub(crate) tokio::sync::MutexGuard<'a, ()>, + pub(crate) Option>, ); +impl<'a> Drop for LockedHelper<'a> { + fn drop(&mut self) { + // make it explicit that the mutexguard lives until here + drop(self.1.take()) + } +} + #[allow(clippy::ptr_arg)] impl<'a> LockedHelper<'a> { pub fn bucket(&self) -> BucketHelper<'a> { From c9d00f5f7be307443d2f345398dd650c716fa04a Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 18:17:11 +0100 Subject: [PATCH 16/43] garage_api_s3: remove unused field in ListPartsQuery --- src/api/s3/api_server.rs | 1 - src/api/s3/list.rs | 3 --- 2 files changed, 4 deletions(-) diff --git a/src/api/s3/api_server.rs b/src/api/s3/api_server.rs index bf48bba1..14fd03c3 100644 --- a/src/api/s3/api_server.rs +++ b/src/api/s3/api_server.rs @@ -317,7 +317,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)), diff --git a/src/api/s3/list.rs b/src/api/s3/list.rs index a5cc03b0..94c2c895 100644 --- a/src/api/s3/list.rs +++ b/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, @@ -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, From 2729a71d9dc55f142a7f89236b1c868a1bdb1c0f Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 14 Feb 2025 18:27:00 +0100 Subject: [PATCH 17/43] fix warning in garage test --- src/garage/tests/common/garage.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/garage/tests/common/garage.rs b/src/garage/tests/common/garage.rs index db23d316..da6c624b 100644 --- a/src/garage/tests/common/garage.rs +++ b/src/garage/tests/common/garage.rs @@ -13,7 +13,6 @@ static GARAGE_TEST_SECRET: &str = #[derive(Debug, Default, Clone)] pub struct Key { - pub name: Option, pub id: String, pub secret: String, } @@ -213,10 +212,7 @@ api_bind_addr = "127.0.0.1:{admin_port}" assert!(!key.id.is_empty(), "Invalid key: Key ID is empty"); assert!(!key.secret.is_empty(), "Invalid key: Key secret is empty"); - Key { - name: maybe_name.map(String::from), - ..key - } + key } } From 2f0c5ca220d73b6c621f21816b666f939839dd49 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Sun, 16 Feb 2025 16:34:18 +0100 Subject: [PATCH 18/43] signature: refactor: move constant defs to mod.rs --- src/api/common/signature/mod.rs | 48 +++++++++++++++++++++++++++ src/api/common/signature/payload.rs | 16 +-------- src/api/common/signature/streaming.rs | 12 +------ 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/src/api/common/signature/mod.rs b/src/api/common/signature/mod.rs index 6514da43..27082168 100644 --- a/src/api/common/signature/mod.rs +++ b/src/api/common/signature/mod.rs @@ -2,6 +2,7 @@ use chrono::{DateTime, Utc}; use hmac::{Hmac, Mac}; use sha2::Sha256; +use hyper::header::HeaderName; use hyper::{body::Incoming as IncomingBody, Request}; use garage_model::garage::Garage; @@ -17,8 +18,55 @@ pub mod streaming; pub const SHORT_DATE: &str = "%Y%m%d"; pub const LONG_DATETIME: &str = "%Y%m%dT%H%M%SZ"; +// ---- Constants used in AWSv4 signatures ---- + +pub const X_AMZ_ALGORITHM: HeaderName = HeaderName::from_static("x-amz-algorithm"); +pub const X_AMZ_CREDENTIAL: HeaderName = HeaderName::from_static("x-amz-credential"); +pub const X_AMZ_DATE: HeaderName = HeaderName::from_static("x-amz-date"); +pub const X_AMZ_EXPIRES: HeaderName = HeaderName::from_static("x-amz-expires"); +pub const X_AMZ_SIGNEDHEADERS: HeaderName = HeaderName::from_static("x-amz-signedheaders"); +pub const X_AMZ_SIGNATURE: HeaderName = HeaderName::from_static("x-amz-signature"); +pub const X_AMZ_CONTENT_SH256: HeaderName = HeaderName::from_static("x-amz-content-sha256"); +pub const X_AMZ_TRAILER: HeaderName = HeaderName::from_static("x-amz-trailer"); + +/// Result of `sha256("")` +pub(crate) const EMPTY_STRING_HEX_DIGEST: &str = + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + +// Signature calculation algorithm +pub const AWS4_HMAC_SHA256: &str = "AWS4-HMAC-SHA256"; type HmacSha256 = Hmac; +// Possible values for x-amz-content-sha256, in addition to the actual sha256 +pub const UNSIGNED_PAYLOAD: &str = "UNSIGNED-PAYLOAD"; +pub const STREAMING_AWS4_HMAC_SHA256_PAYLOAD: &str = "STREAMING-AWS4-HMAC-SHA256-PAYLOAD"; + +// Used in the computation of StringToSign +pub const AWS4_HMAC_SHA256_PAYLOAD: &str = "AWS4-HMAC-SHA256-PAYLOAD"; + +// ---- enums to describe stuff going on in signature calculation ---- + +pub enum ContentSha256Header { + UnsignedPayload, + Sha256Hash(String), + StreamingPayload { + trailer: Option, + algorithm: Option, + }, +} + +pub enum SigningAlgorithm { + AwsHmacSha256, +} + +pub enum TrailerHeader { + XAmzChecksumCrc32, + XAmzChecksumCrc32c, + XAmzChecksumCrc64Nvme, +} + +// ---- top-level functions ---- + pub async fn verify_request( garage: &Garage, mut req: Request, diff --git a/src/api/common/signature/payload.rs b/src/api/common/signature/payload.rs index 81541e4a..0b501853 100644 --- a/src/api/common/signature/payload.rs +++ b/src/api/common/signature/payload.rs @@ -13,23 +13,9 @@ use garage_util::data::Hash; use garage_model::garage::Garage; use garage_model::key_table::*; -use super::LONG_DATETIME; -use super::{compute_scope, signing_hmac}; +use super::*; use crate::encoding::uri_encode; -use crate::signature::error::*; - -pub const X_AMZ_ALGORITHM: HeaderName = HeaderName::from_static("x-amz-algorithm"); -pub const X_AMZ_CREDENTIAL: HeaderName = HeaderName::from_static("x-amz-credential"); -pub const X_AMZ_DATE: HeaderName = HeaderName::from_static("x-amz-date"); -pub const X_AMZ_EXPIRES: HeaderName = HeaderName::from_static("x-amz-expires"); -pub const X_AMZ_SIGNEDHEADERS: HeaderName = HeaderName::from_static("x-amz-signedheaders"); -pub const X_AMZ_SIGNATURE: HeaderName = HeaderName::from_static("x-amz-signature"); -pub const X_AMZ_CONTENT_SH256: HeaderName = HeaderName::from_static("x-amz-content-sha256"); - -pub const AWS4_HMAC_SHA256: &str = "AWS4-HMAC-SHA256"; -pub const UNSIGNED_PAYLOAD: &str = "UNSIGNED-PAYLOAD"; -pub const STREAMING_AWS4_HMAC_SHA256_PAYLOAD: &str = "STREAMING-AWS4-HMAC-SHA256-PAYLOAD"; pub type QueryMap = HeaderMap; pub struct QueryValue { diff --git a/src/api/common/signature/streaming.rs b/src/api/common/signature/streaming.rs index e223d1b1..e08a4750 100644 --- a/src/api/common/signature/streaming.rs +++ b/src/api/common/signature/streaming.rs @@ -11,15 +11,9 @@ use hyper::Request; use garage_util::data::Hash; -use super::{compute_scope, sha256sum, HmacSha256, LONG_DATETIME}; +use super::*; use crate::helpers::*; -use crate::signature::error::*; -use crate::signature::payload::{ - STREAMING_AWS4_HMAC_SHA256_PAYLOAD, X_AMZ_CONTENT_SH256, X_AMZ_DATE, -}; - -pub const AWS4_HMAC_SHA256_PAYLOAD: &str = "AWS4-HMAC-SHA256-PAYLOAD"; pub type ReqBody = BoxBody; @@ -68,10 +62,6 @@ pub fn parse_streaming_body( } } -/// Result of `sha256("")` -const EMPTY_STRING_HEX_DIGEST: &str = - "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; - fn compute_streaming_payload_signature( signing_hmac: &HmacSha256, date: DateTime, From cee7560fc1c3e885dc80dfee233211f54ac9db7d Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Sun, 16 Feb 2025 16:44:34 +0100 Subject: [PATCH 19/43] api: refactor: move checksum algorithms to common --- Cargo.lock | 5 + src/api/common/Cargo.toml | 5 + src/api/common/signature/checksum.rs | 181 +++++++++++++++++++++++++++ src/api/common/signature/error.rs | 4 + src/api/common/signature/mod.rs | 1 + src/api/k2v/error.rs | 7 ++ src/api/s3/checksum.rs | 171 +------------------------ src/api/s3/copy.rs | 1 + src/api/s3/encryption.rs | 2 +- src/api/s3/error.rs | 3 +- src/api/s3/get.rs | 3 +- src/api/s3/multipart.rs | 1 + src/api/s3/post_object.rs | 1 + src/api/s3/put.rs | 1 + 14 files changed, 215 insertions(+), 171 deletions(-) create mode 100644 src/api/common/signature/checksum.rs diff --git a/Cargo.lock b/Cargo.lock index ad5d098d..26f6ea1d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1301,8 +1301,11 @@ dependencies = [ name = "garage_api_common" version = "1.0.1" dependencies = [ + "base64 0.21.7", "bytes", "chrono", + "crc32c", + "crc32fast", "crypto-common", "err-derive", "futures", @@ -1316,11 +1319,13 @@ dependencies = [ "hyper 1.6.0", "hyper-util", "idna 0.5.0", + "md-5", "nom", "opentelemetry", "pin-project", "serde", "serde_json", + "sha1", "sha2", "tokio", "tracing", diff --git a/src/api/common/Cargo.toml b/src/api/common/Cargo.toml index 5b9cf479..c33d585d 100644 --- a/src/api/common/Cargo.toml +++ b/src/api/common/Cargo.toml @@ -18,16 +18,21 @@ garage_model.workspace = true garage_table.workspace = true garage_util.workspace = true +base64.workspace = true bytes.workspace = true chrono.workspace = true +crc32fast.workspace = true +crc32c.workspace = true crypto-common.workspace = true err-derive.workspace = true hex.workspace = true hmac.workspace = true +md-5.workspace = true idna.workspace = true tracing.workspace = true nom.workspace = true pin-project.workspace = true +sha1.workspace = true sha2.workspace = true futures.workspace = true diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs new file mode 100644 index 00000000..c6beb33f --- /dev/null +++ b/src/api/common/signature/checksum.rs @@ -0,0 +1,181 @@ +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::HeaderName; + +use garage_util::data::*; + +use garage_model::s3::object_table::*; + +use super::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 struct ExpectedChecksums { + // base64-encoded md5 (content-md5 header) + pub md5: Option, + // content_sha256 (as a Hash / FixedBytes32) + pub sha256: Option, + // extra x-amz-checksum-* header + pub extra: Option, +} + +pub struct Checksummer { + pub crc32: Option, + pub crc32c: Option, + pub md5: Option, + pub sha1: Option, + pub sha256: Option, +} + +#[derive(Default)] +pub struct Checksums { + pub crc32: Option, + pub crc32c: Option, + pub md5: Option, + pub sha1: Option, + pub sha256: Option, +} + +impl Checksummer { + pub 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 fn add(mut self, algo: Option) -> 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 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 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) -> Option { + 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())), + } + } +} diff --git a/src/api/common/signature/error.rs b/src/api/common/signature/error.rs index 2d92a072..b2f396b5 100644 --- a/src/api/common/signature/error.rs +++ b/src/api/common/signature/error.rs @@ -18,6 +18,10 @@ pub enum Error { /// The request contained an invalid UTF-8 sequence in its path or in other parameters #[error(display = "Invalid UTF-8: {}", _0)] InvalidUtf8Str(#[error(source)] std::str::Utf8Error), + + /// The provided digest (checksum) value was invalid + #[error(display = "Invalid digest: {}", _0)] + InvalidDigest(String), } impl From for Error diff --git a/src/api/common/signature/mod.rs b/src/api/common/signature/mod.rs index 27082168..08b0aa7e 100644 --- a/src/api/common/signature/mod.rs +++ b/src/api/common/signature/mod.rs @@ -11,6 +11,7 @@ use garage_util::data::{sha256sum, Hash}; use error::*; +pub mod checksum; pub mod error; pub mod payload; pub mod streaming; diff --git a/src/api/k2v/error.rs b/src/api/k2v/error.rs index 3cd0e6f7..b7ca5aa4 100644 --- a/src/api/k2v/error.rs +++ b/src/api/k2v/error.rs @@ -23,6 +23,10 @@ pub enum Error { #[error(display = "Authorization header malformed, unexpected scope: {}", _0)] AuthorizationHeaderMalformed(String), + /// The provided digest (checksum) value was invalid + #[error(display = "Invalid digest: {}", _0)] + InvalidDigest(String), + /// The object requested don't exists #[error(display = "Key not found")] NoSuchKey, @@ -54,6 +58,7 @@ impl From for Error { Self::AuthorizationHeaderMalformed(c) } SignatureError::InvalidUtf8Str(i) => Self::InvalidUtf8Str(i), + SignatureError::InvalidDigest(d) => Self::InvalidDigest(d), } } } @@ -71,6 +76,7 @@ impl Error { Error::InvalidBase64(_) => "InvalidBase64", Error::InvalidUtf8Str(_) => "InvalidUtf8String", Error::InvalidCausalityToken => "CausalityToken", + Error::InvalidDigest(_) => "InvalidDigest", } } } @@ -85,6 +91,7 @@ impl ApiError for Error { Error::AuthorizationHeaderMalformed(_) | Error::InvalidBase64(_) | Error::InvalidUtf8Str(_) + | Error::InvalidDigest(_) | Error::InvalidCausalityToken => StatusCode::BAD_REQUEST, } } diff --git a/src/api/s3/checksum.rs b/src/api/s3/checksum.rs index 02fb55ec..a720a82f 100644 --- a/src/api/s3/checksum.rs +++ b/src/api/s3/checksum.rs @@ -8,181 +8,16 @@ use md5::{Digest, Md5}; use sha1::Sha1; use sha2::Sha256; -use http::{HeaderMap, HeaderName, HeaderValue}; +use http::{HeaderMap, HeaderValue}; -use garage_util::data::*; use garage_util::error::OkOrMessage; use garage_model::s3::object_table::*; +use garage_api_common::signature::checksum::*; + 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, - // content_sha256 (as a Hash / FixedBytes32) - pub sha256: Option, - // extra x-amz-checksum-* header - pub extra: Option, -} - -pub(crate) struct Checksummer { - pub crc32: Option, - pub crc32c: Option, - pub md5: Option, - pub sha1: Option, - pub sha256: Option, -} - -#[derive(Default)] -pub(crate) struct Checksums { - pub crc32: Option, - pub crc32c: Option, - pub md5: Option, - pub sha1: Option, - pub sha256: Option, -} - -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) -> 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) -> Option { - 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, diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs index 07d50ea5..4bf68406 100644 --- a/src/api/s3/copy.rs +++ b/src/api/s3/copy.rs @@ -21,6 +21,7 @@ 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::*; diff --git a/src/api/s3/encryption.rs b/src/api/s3/encryption.rs index b38d7792..fa7285ca 100644 --- a/src/api/s3/encryption.rs +++ b/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 = diff --git a/src/api/s3/error.rs b/src/api/s3/error.rs index 1bb8909c..6d4b7a11 100644 --- a/src/api/s3/error.rs +++ b/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 for Error { Self::AuthorizationHeaderMalformed(c) } SignatureError::InvalidUtf8Str(i) => Self::InvalidUtf8Str(i), + SignatureError::InvalidDigest(d) => Self::InvalidDigest(d), } } } diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs index c2393a51..6627cf4a 100644 --- a/src/api/s3/get.rs +++ b/src/api/s3/get.rs @@ -26,9 +26,10 @@ use garage_model::s3::object_table::*; use garage_model::s3::version_table::*; use garage_api_common::helpers::*; +use garage_api_common::signature::checksum::X_AMZ_CHECKSUM_MODE; use crate::api_server::ResBody; -use crate::checksum::{add_checksum_response_headers, X_AMZ_CHECKSUM_MODE}; +use crate::checksum::add_checksum_response_headers; use crate::encryption::EncryptionParams; use crate::error::*; diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index fa053df2..7f8d6440 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -16,6 +16,7 @@ use garage_model::s3::object_table::*; use garage_model::s3::version_table::*; use garage_api_common::helpers::*; +use garage_api_common::signature::checksum::*; use garage_api_common::signature::verify_signed_content; use crate::api_server::{ReqBody, ResBody}; diff --git a/src/api/s3/post_object.rs b/src/api/s3/post_object.rs index 6c0e73d4..908ee9f3 100644 --- a/src/api/s3/post_object.rs +++ b/src/api/s3/post_object.rs @@ -18,6 +18,7 @@ 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; diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 530b4e7b..834be6f1 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -31,6 +31,7 @@ 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::*; From 44a896f9b50680a1fdaeb6aaad18a6bf07e9f3c3 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Sun, 16 Feb 2025 18:25:35 +0100 Subject: [PATCH 20/43] api: add logic to parse x-amz-content-sha256 --- src/api/common/signature/checksum.rs | 2 +- src/api/common/signature/mod.rs | 51 ++++++------ src/api/common/signature/payload.rs | 90 +++++++++++++++------ src/api/common/signature/streaming.rs | 41 +++++++--- src/api/k2v/api_server.rs | 4 +- src/api/k2v/error.rs | 4 +- src/api/s3/api_server.rs | 11 ++- src/garage/tests/common/custom_requester.rs | 7 +- 8 files changed, 138 insertions(+), 72 deletions(-) diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs index c6beb33f..432ed44d 100644 --- a/src/api/common/signature/checksum.rs +++ b/src/api/common/signature/checksum.rs @@ -12,7 +12,7 @@ use http::HeaderName; use garage_util::data::*; -use garage_model::s3::object_table::*; +use garage_model::s3::object_table::{ChecksumAlgorithm, ChecksumValue}; use super::error::*; diff --git a/src/api/common/signature/mod.rs b/src/api/common/signature/mod.rs index 08b0aa7e..2421d696 100644 --- a/src/api/common/signature/mod.rs +++ b/src/api/common/signature/mod.rs @@ -27,7 +27,7 @@ pub const X_AMZ_DATE: HeaderName = HeaderName::from_static("x-amz-date"); pub const X_AMZ_EXPIRES: HeaderName = HeaderName::from_static("x-amz-expires"); pub const X_AMZ_SIGNEDHEADERS: HeaderName = HeaderName::from_static("x-amz-signedheaders"); pub const X_AMZ_SIGNATURE: HeaderName = HeaderName::from_static("x-amz-signature"); -pub const X_AMZ_CONTENT_SH256: HeaderName = HeaderName::from_static("x-amz-content-sha256"); +pub const X_AMZ_CONTENT_SHA256: HeaderName = HeaderName::from_static("x-amz-content-sha256"); pub const X_AMZ_TRAILER: HeaderName = HeaderName::from_static("x-amz-trailer"); /// Result of `sha256("")` @@ -40,6 +40,7 @@ type HmacSha256 = Hmac; // Possible values for x-amz-content-sha256, in addition to the actual sha256 pub const UNSIGNED_PAYLOAD: &str = "UNSIGNED-PAYLOAD"; +pub const STREAMING_UNSIGNED_PAYLOAD_TRAILER: &str = "STREAMING-UNSIGNED-PAYLOAD-TRAILER"; pub const STREAMING_AWS4_HMAC_SHA256_PAYLOAD: &str = "STREAMING-AWS4-HMAC-SHA256-PAYLOAD"; // Used in the computation of StringToSign @@ -47,46 +48,46 @@ pub const AWS4_HMAC_SHA256_PAYLOAD: &str = "AWS4-HMAC-SHA256-PAYLOAD"; // ---- enums to describe stuff going on in signature calculation ---- +#[derive(Debug)] pub enum ContentSha256Header { UnsignedPayload, - Sha256Hash(String), - StreamingPayload { - trailer: Option, - algorithm: Option, - }, -} - -pub enum SigningAlgorithm { - AwsHmacSha256, -} - -pub enum TrailerHeader { - XAmzChecksumCrc32, - XAmzChecksumCrc32c, - XAmzChecksumCrc64Nvme, + Sha256Hash(Hash), + StreamingPayload { trailer: bool, signed: bool }, } // ---- top-level functions ---- +pub struct VerifiedRequest { + pub request: Request, + pub access_key: Key, + pub content_sha256_header: ContentSha256Header, + // TODO: oneshot chans to retrieve hashes after reading all body +} + pub async fn verify_request( garage: &Garage, mut req: Request, service: &'static str, -) -> Result<(Request, Key, Option), 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"))?; +) -> Result { + let checked_signature = payload::check_payload_signature(&garage, &mut req, service).await?; + eprintln!("checked signature: {:?}", checked_signature); - let req = streaming::parse_streaming_body( - &api_key, + let request = streaming::parse_streaming_body( req, - &mut content_sha256, + &checked_signature, &garage.config.s3_api.s3_region, service, )?; - Ok((req, api_key, content_sha256)) + let access_key = checked_signature + .key + .ok_or_else(|| Error::forbidden("Garage does not support anonymous access yet"))?; + + Ok(VerifiedRequest { + request, + access_key, + content_sha256_header: checked_signature.content_sha256_header, + }) } pub fn verify_signed_content(expected_sha256: Hash, body: &[u8]) -> Result<(), Error> { diff --git a/src/api/common/signature/payload.rs b/src/api/common/signature/payload.rs index 0b501853..ccc55c90 100644 --- a/src/api/common/signature/payload.rs +++ b/src/api/common/signature/payload.rs @@ -25,11 +25,18 @@ pub struct QueryValue { value: String, } +#[derive(Debug)] +pub struct CheckedSignature { + pub key: Option, + pub content_sha256_header: ContentSha256Header, + pub signature_header: Option, +} + pub async fn check_payload_signature( garage: &Garage, request: &mut Request, service: &'static str, -) -> Result<(Option, Option), Error> { +) -> Result { let query = parse_query_map(request.uri())?; if query.contains_key(&X_AMZ_ALGORITHM) { @@ -43,17 +50,51 @@ pub async fn check_payload_signature( // Unsigned (anonymous) request let content_sha256 = request .headers() - .get("x-amz-content-sha256") - .filter(|c| c.as_bytes() != UNSIGNED_PAYLOAD.as_bytes()); - if let Some(content_sha256) = content_sha256 { - let sha256 = hex::decode(content_sha256) - .ok() - .and_then(|bytes| Hash::try_from(&bytes)) - .ok_or_bad_request("Invalid content sha256 hash")?; - Ok((None, Some(sha256))) + .get(X_AMZ_CONTENT_SHA256) + .map(|x| x.to_str()) + .transpose()?; + Ok(CheckedSignature { + key: None, + content_sha256_header: parse_x_amz_content_sha256(content_sha256)?, + signature_header: None, + }) + } +} + +fn parse_x_amz_content_sha256(header: Option<&str>) -> Result { + let header = match header { + Some(x) => x, + None => return Ok(ContentSha256Header::UnsignedPayload), + }; + if header == UNSIGNED_PAYLOAD { + Ok(ContentSha256Header::UnsignedPayload) + } else if let Some(rest) = header.strip_prefix("STREAMING-") { + let (trailer, algo) = if let Some(rest2) = rest.strip_suffix("-TRAILER") { + (true, rest2) } else { - Ok((None, None)) + (false, rest) + }; + if algo == AWS4_HMAC_SHA256_PAYLOAD { + Ok(ContentSha256Header::StreamingPayload { + trailer, + signed: true, + }) + } else if algo == UNSIGNED_PAYLOAD { + Ok(ContentSha256Header::StreamingPayload { + trailer, + signed: false, + }) + } else { + Err(Error::bad_request( + "invalid or unsupported x-amz-content-sha256", + )) } + } else { + let sha256 = hex::decode(header) + .ok() + .and_then(|bytes| Hash::try_from(&bytes)) + .ok_or_bad_request("Invalid content sha256 hash")?; + Ok(ContentSha256Header::Sha256Hash(sha256)) } } @@ -62,7 +103,7 @@ async fn check_standard_signature( service: &'static str, request: &Request, query: QueryMap, -) -> Result<(Option, Option), Error> { +) -> Result { let authorization = Authorization::parse_header(request.headers())?; // Verify that all necessary request headers are included in signed_headers @@ -94,18 +135,13 @@ async fn check_standard_signature( let key = verify_v4(garage, service, &authorization, string_to_sign.as_bytes()).await?; - let content_sha256 = if authorization.content_sha256 == UNSIGNED_PAYLOAD { - None - } else if authorization.content_sha256 == STREAMING_AWS4_HMAC_SHA256_PAYLOAD { - let bytes = hex::decode(authorization.signature).ok_or_bad_request("Invalid signature")?; - Some(Hash::try_from(&bytes).ok_or_bad_request("Invalid signature")?) - } else { - let bytes = hex::decode(authorization.content_sha256) - .ok_or_bad_request("Invalid content sha256 hash")?; - Some(Hash::try_from(&bytes).ok_or_bad_request("Invalid content sha256 hash")?) - }; + let content_sha256_header = parse_x_amz_content_sha256(Some(&authorization.content_sha256))?; - Ok((Some(key), content_sha256)) + Ok(CheckedSignature { + key: Some(key), + content_sha256_header, + signature_header: Some(authorization.signature), + }) } async fn check_presigned_signature( @@ -113,7 +149,7 @@ async fn check_presigned_signature( service: &'static str, request: &mut Request, mut query: QueryMap, -) -> Result<(Option, Option), Error> { +) -> Result { let algorithm = query.get(&X_AMZ_ALGORITHM).unwrap(); let authorization = Authorization::parse_presigned(&algorithm.value, &query)?; @@ -179,7 +215,11 @@ async fn check_presigned_signature( // Presigned URLs always use UNSIGNED-PAYLOAD, // so there is no sha256 hash to return. - Ok((Some(key), None)) + Ok(CheckedSignature { + key: Some(key), + content_sha256_header: ContentSha256Header::UnsignedPayload, + signature_header: Some(authorization.signature), + }) } pub fn parse_query_map(uri: &http::uri::Uri) -> Result { @@ -428,7 +468,7 @@ impl Authorization { .to_string(); let content_sha256 = headers - .get(X_AMZ_CONTENT_SH256) + .get(X_AMZ_CONTENT_SHA256) .ok_or_bad_request("Missing X-Amz-Content-Sha256 field")?; let date = headers diff --git a/src/api/common/signature/streaming.rs b/src/api/common/signature/streaming.rs index e08a4750..98079ffb 100644 --- a/src/api/common/signature/streaming.rs +++ b/src/api/common/signature/streaming.rs @@ -3,7 +3,6 @@ use std::pin::Pin; use chrono::{DateTime, NaiveDateTime, TimeZone, Utc}; use futures::prelude::*; use futures::task; -use garage_model::key_table::Key; use hmac::Mac; use http_body_util::StreamBody; use hyper::body::{Bytes, Incoming as IncomingBody}; @@ -14,27 +13,47 @@ use garage_util::data::Hash; use super::*; use crate::helpers::*; +use crate::signature::payload::CheckedSignature; pub type ReqBody = BoxBody; pub fn parse_streaming_body( - api_key: &Key, req: Request, - content_sha256: &mut Option, + checked_signature: &CheckedSignature, region: &str, service: &str, ) -> Result, Error> { - match req.headers().get(X_AMZ_CONTENT_SH256) { - Some(header) if header == STREAMING_AWS4_HMAC_SHA256_PAYLOAD => { - let signature = content_sha256 - .take() - .ok_or_bad_request("No signature provided")?; + match checked_signature.content_sha256_header { + ContentSha256Header::StreamingPayload { signed, trailer } => { + if trailer { + return Err(Error::bad_request( + "STREAMING-*-TRAILER is not supported by Garage", + )); + } + if !signed { + return Err(Error::bad_request( + "STREAMING-UNSIGNED-PAYLOAD-* is not supported by Garage", + )); + } - let secret_key = &api_key + let signature = checked_signature + .signature_header + .clone() + .ok_or_bad_request("No signature provided")?; + let signature = hex::decode(signature) + .ok() + .and_then(|bytes| Hash::try_from(&bytes)) + .ok_or_bad_request("Invalid signature")?; + + let secret_key = checked_signature + .key + .as_ref() + .ok_or_bad_request("Cannot sign streaming payload without signing key")? .state .as_option() .ok_or_internal_error("Deleted key state")? - .secret_key; + .secret_key + .to_string(); let date = req .headers() @@ -46,7 +65,7 @@ pub fn parse_streaming_body( let date: DateTime = Utc.from_utc_datetime(&date); let scope = compute_scope(&date, region, service); - let signing_hmac = crate::signature::signing_hmac(&date, secret_key, region, service) + let signing_hmac = crate::signature::signing_hmac(&date, &secret_key, region, service) .ok_or_internal_error("Unable to build signing HMAC")?; Ok(req.map(move |body| { diff --git a/src/api/k2v/api_server.rs b/src/api/k2v/api_server.rs index eb276f5b..de5775da 100644 --- a/src/api/k2v/api_server.rs +++ b/src/api/k2v/api_server.rs @@ -81,7 +81,9 @@ impl ApiHandler for K2VApiServer { return Ok(options_res.map(|_empty_body: EmptyBody| empty_body())); } - let (req, api_key, _content_sha256) = verify_request(&garage, req, "k2v").await?; + let verified_request = verify_request(&garage, req, "k2v").await?; + let req = verified_request.request; + let api_key = verified_request.access_key; let bucket_id = garage .bucket_helper() diff --git a/src/api/k2v/error.rs b/src/api/k2v/error.rs index b7ca5aa4..257ff893 100644 --- a/src/api/k2v/error.rs +++ b/src/api/k2v/error.rs @@ -76,7 +76,7 @@ impl Error { Error::InvalidBase64(_) => "InvalidBase64", Error::InvalidUtf8Str(_) => "InvalidUtf8String", Error::InvalidCausalityToken => "CausalityToken", - Error::InvalidDigest(_) => "InvalidDigest", + Error::InvalidDigest(_) => "InvalidDigest", } } } @@ -91,7 +91,7 @@ impl ApiError for Error { Error::AuthorizationHeaderMalformed(_) | Error::InvalidBase64(_) | Error::InvalidUtf8Str(_) - | Error::InvalidDigest(_) + | Error::InvalidDigest(_) | Error::InvalidCausalityToken => StatusCode::BAD_REQUEST, } } diff --git a/src/api/s3/api_server.rs b/src/api/s3/api_server.rs index 14fd03c3..0fdaab70 100644 --- a/src/api/s3/api_server.rs +++ b/src/api/s3/api_server.rs @@ -15,7 +15,7 @@ use garage_model::key_table::Key; use garage_api_common::cors::*; use garage_api_common::generic_server::*; use garage_api_common::helpers::*; -use garage_api_common::signature::verify_request; +use garage_api_common::signature::{verify_request, ContentSha256Header}; use crate::bucket::*; use crate::copy::*; @@ -121,7 +121,14 @@ 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 content_sha256 = match verified_request.content_sha256_header { + ContentSha256Header::Sha256Hash(h) => Some(h), + // TODO take into account streaming/trailer checksums, etc. + _ => None, + }; let bucket_name = match bucket_name { None => { diff --git a/src/garage/tests/common/custom_requester.rs b/src/garage/tests/common/custom_requester.rs index 2db72e9f..99fd4385 100644 --- a/src/garage/tests/common/custom_requester.rs +++ b/src/garage/tests/common/custom_requester.rs @@ -192,10 +192,7 @@ impl<'a> RequestBuilder<'a> { .collect::(); let date = now.format(signature::LONG_DATETIME).to_string(); - all_headers.insert( - signature::payload::X_AMZ_DATE, - HeaderValue::from_str(&date).unwrap(), - ); + all_headers.insert(signature::X_AMZ_DATE, HeaderValue::from_str(&date).unwrap()); all_headers.insert(HOST, HeaderValue::from_str(&host).unwrap()); let body_sha = match self.body_signature { @@ -227,7 +224,7 @@ impl<'a> RequestBuilder<'a> { } }; all_headers.insert( - signature::payload::X_AMZ_CONTENT_SH256, + signature::X_AMZ_CONTENT_SHA256, HeaderValue::from_str(&body_sha).unwrap(), ); From a04d6cd5b8a3acffb8daeee00aed744fb1a78ea3 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Sun, 16 Feb 2025 19:12:53 +0100 Subject: [PATCH 21/43] api: streaming: parse unsigned streaming bodies and payload trailers --- src/api/common/signature/streaming.rs | 454 +++++++++++++++++--------- 1 file changed, 306 insertions(+), 148 deletions(-) diff --git a/src/api/common/signature/streaming.rs b/src/api/common/signature/streaming.rs index 98079ffb..b8a5e66d 100644 --- a/src/api/common/signature/streaming.rs +++ b/src/api/common/signature/streaming.rs @@ -25,53 +25,60 @@ pub fn parse_streaming_body( ) -> Result, Error> { match checked_signature.content_sha256_header { ContentSha256Header::StreamingPayload { signed, trailer } => { - if trailer { + if !signed && !trailer { return Err(Error::bad_request( - "STREAMING-*-TRAILER is not supported by Garage", - )); - } - if !signed { - return Err(Error::bad_request( - "STREAMING-UNSIGNED-PAYLOAD-* is not supported by Garage", + "STREAMING-UNSIGNED-PAYLOAD is not a valid combination", )); } - let signature = checked_signature - .signature_header - .clone() - .ok_or_bad_request("No signature provided")?; - let signature = hex::decode(signature) - .ok() - .and_then(|bytes| Hash::try_from(&bytes)) - .ok_or_bad_request("Invalid signature")?; + let sign_params = if signed { + let signature = checked_signature + .signature_header + .clone() + .ok_or_bad_request("No signature provided")?; + let signature = hex::decode(signature) + .ok() + .and_then(|bytes| Hash::try_from(&bytes)) + .ok_or_bad_request("Invalid signature")?; - let secret_key = checked_signature - .key - .as_ref() - .ok_or_bad_request("Cannot sign streaming payload without signing key")? - .state - .as_option() - .ok_or_internal_error("Deleted key state")? - .secret_key - .to_string(); + let secret_key = checked_signature + .key + .as_ref() + .ok_or_bad_request("Cannot sign streaming payload without signing key")? + .state + .as_option() + .ok_or_internal_error("Deleted key state")? + .secret_key + .to_string(); - let date = req - .headers() - .get(X_AMZ_DATE) - .ok_or_bad_request("Missing X-Amz-Date field")? - .to_str()?; - let date: NaiveDateTime = NaiveDateTime::parse_from_str(date, LONG_DATETIME) - .ok_or_bad_request("Invalid date")?; - let date: DateTime = Utc.from_utc_datetime(&date); + let date = req + .headers() + .get(X_AMZ_DATE) + .ok_or_bad_request("Missing X-Amz-Date field")? + .to_str()?; + let date: NaiveDateTime = NaiveDateTime::parse_from_str(date, LONG_DATETIME) + .ok_or_bad_request("Invalid date")?; + let date: DateTime = Utc.from_utc_datetime(&date); - let scope = compute_scope(&date, region, service); - let signing_hmac = crate::signature::signing_hmac(&date, &secret_key, region, service) - .ok_or_internal_error("Unable to build signing HMAC")?; + let scope = compute_scope(&date, region, service); + let signing_hmac = + crate::signature::signing_hmac(&date, &secret_key, region, service) + .ok_or_internal_error("Unable to build signing HMAC")?; + + Some(SignParams { + datetime: date, + scope, + signing_hmac, + previous_signature: signature, + }) + } else { + None + }; Ok(req.map(move |body| { let stream = body_stream::<_, Error>(body); let signed_payload_stream = - SignedPayloadStream::new(stream, signing_hmac, date, &scope, signature) + StreamingPayloadStream::new(stream, sign_params, trailer) .map(|x| x.map(hyper::body::Frame::data)) .map_err(Error::from); ReqBody::new(StreamBody::new(signed_payload_stream)) @@ -87,7 +94,7 @@ fn compute_streaming_payload_signature( scope: &str, previous_signature: Hash, content_sha256: Hash, -) -> Result { +) -> Result { let string_to_sign = [ AWS4_HMAC_SHA256_PAYLOAD, &date.format(LONG_DATETIME).to_string(), @@ -101,12 +108,47 @@ fn compute_streaming_payload_signature( let mut hmac = signing_hmac.clone(); hmac.update(string_to_sign.as_bytes()); - Ok(Hash::try_from(&hmac.finalize().into_bytes()).ok_or_internal_error("Invalid signature")?) + Hash::try_from(&hmac.finalize().into_bytes()) + .ok_or_else(|| StreamingPayloadError::Message("Could not build signature".into())) +} + +fn compute_streaming_trailer_signature( + signing_hmac: &HmacSha256, + date: DateTime, + scope: &str, + previous_signature: Hash, + trailer_sha256: Hash, +) -> Result { + let string_to_sign = [ + AWS4_HMAC_SHA256_PAYLOAD, + &date.format(LONG_DATETIME).to_string(), + scope, + &hex::encode(previous_signature), + &hex::encode(trailer_sha256), + ] + .join("\n"); + + let mut hmac = signing_hmac.clone(); + hmac.update(string_to_sign.as_bytes()); + + Hash::try_from(&hmac.finalize().into_bytes()) + .ok_or_else(|| StreamingPayloadError::Message("Could not build signature".into())) } mod payload { use garage_util::data::Hash; + use nom::bytes::streaming::{tag, take_while}; + use nom::character::streaming::hex_digit1; + use nom::combinator::map_res; + use nom::number::streaming::hex_u32; + + macro_rules! try_parse { + ($expr:expr) => { + $expr.map_err(|e| e.map(Error::Parser))? + }; + } + pub enum Error { Parser(nom::error::Error), BadSignature, @@ -122,24 +164,13 @@ mod payload { } #[derive(Debug, Clone)] - pub struct Header { + pub struct ChunkHeader { pub size: usize, - pub signature: Hash, + pub signature: Option, } - impl Header { - pub fn parse(input: &[u8]) -> nom::IResult<&[u8], Self, Error<&[u8]>> { - use nom::bytes::streaming::tag; - use nom::character::streaming::hex_digit1; - use nom::combinator::map_res; - use nom::number::streaming::hex_u32; - - macro_rules! try_parse { - ($expr:expr) => { - $expr.map_err(|e| e.map(Error::Parser))? - }; - } - + impl ChunkHeader { + pub fn parse_signed(input: &[u8]) -> nom::IResult<&[u8], Self, Error<&[u8]>> { let (input, size) = try_parse!(hex_u32(input)); let (input, _) = try_parse!(tag(";")(input)); @@ -149,96 +180,165 @@ mod payload { let (input, _) = try_parse!(tag("\r\n")(input)); - let header = Header { + let header = ChunkHeader { size: size as usize, - signature, + signature: Some(signature), + }; + + Ok((input, header)) + } + + pub fn parse_unsigned(input: &[u8]) -> nom::IResult<&[u8], Self, Error<&[u8]>> { + let (input, size) = try_parse!(hex_u32(input)); + let (input, _) = try_parse!(tag("\r\n")(input)); + + let header = ChunkHeader { + size: size as usize, + signature: None, }; Ok((input, header)) } } + + #[derive(Debug, Clone)] + pub struct TrailerChunk { + pub header_name: Vec, + pub header_value: Vec, + pub signature: Option, + } + + impl TrailerChunk { + fn parse_content(input: &[u8]) -> nom::IResult<&[u8], Self, Error<&[u8]>> { + let (input, header_name) = try_parse!(take_while( + |c: u8| c.is_ascii_alphanumeric() || c == b'-' + )(input)); + let (input, _) = try_parse!(tag(b":")(input)); + let (input, header_value) = try_parse!(take_while( + |c: u8| c.is_ascii_alphanumeric() || b"+/=".contains(&c) + )(input)); + let (input, _) = try_parse!(tag(b"\n")(input)); + + Ok(( + input, + TrailerChunk { + header_name: header_name.to_vec(), + header_value: header_value.to_vec(), + signature: None, + }, + )) + } + pub fn parse_signed(input: &[u8]) -> nom::IResult<&[u8], Self, Error<&[u8]>> { + let (input, trailer) = Self::parse_content(input)?; + + let (input, _) = try_parse!(tag(b"\r\n\r\n")(input)); + + Ok((input, trailer)) + } + pub fn parse_unsigned(input: &[u8]) -> nom::IResult<&[u8], Self, Error<&[u8]>> { + let (input, trailer) = Self::parse_content(input)?; + + let (input, _) = try_parse!(tag(b"\r\n")(input)); + + let (input, data) = try_parse!(map_res(hex_digit1, hex::decode)(input)); + let signature = Hash::try_from(&data).ok_or(nom::Err::Failure(Error::BadSignature))?; + let (input, _) = try_parse!(tag(b"\r\n")(input)); + + Ok(( + input, + TrailerChunk { + signature: Some(signature), + ..trailer + }, + )) + } + } } #[derive(Debug)] -pub enum SignedPayloadStreamError { +pub enum StreamingPayloadError { Stream(Error), InvalidSignature, Message(String), } -impl SignedPayloadStreamError { +impl StreamingPayloadError { fn message(msg: &str) -> Self { - SignedPayloadStreamError::Message(msg.into()) + StreamingPayloadError::Message(msg.into()) } } -impl From for Error { - fn from(err: SignedPayloadStreamError) -> Self { +impl From for Error { + fn from(err: StreamingPayloadError) -> Self { match err { - SignedPayloadStreamError::Stream(e) => e, - SignedPayloadStreamError::InvalidSignature => { + StreamingPayloadError::Stream(e) => e, + StreamingPayloadError::InvalidSignature => { Error::bad_request("Invalid payload signature") } - SignedPayloadStreamError::Message(e) => { + StreamingPayloadError::Message(e) => { Error::bad_request(format!("Chunk format error: {}", e)) } } } } -impl From> for SignedPayloadStreamError { +impl From> for StreamingPayloadError { fn from(err: payload::Error) -> Self { Self::message(err.description()) } } -impl From> for SignedPayloadStreamError { +impl From> for StreamingPayloadError { fn from(err: nom::error::Error) -> Self { Self::message(err.code.description()) } } -struct SignedPayload { - header: payload::Header, - data: Bytes, +enum StreamingPayloadChunk { + Chunk { + header: payload::ChunkHeader, + data: Bytes, + }, + Trailer(payload::TrailerChunk), } -#[pin_project::pin_project] -pub struct SignedPayloadStream -where - S: Stream>, -{ - #[pin] - stream: S, - buf: bytes::BytesMut, +struct SignParams { datetime: DateTime, scope: String, signing_hmac: HmacSha256, previous_signature: Hash, } -impl SignedPayloadStream +#[pin_project::pin_project] +pub struct StreamingPayloadStream where S: Stream>, { - pub fn new( - stream: S, - signing_hmac: HmacSha256, - datetime: DateTime, - scope: &str, - seed_signature: Hash, - ) -> Self { + #[pin] + stream: S, + buf: bytes::BytesMut, + signing: Option, + has_trailer: bool, +} + +impl StreamingPayloadStream +where + S: Stream>, +{ + fn new(stream: S, signing: Option, has_trailer: bool) -> Self { Self { stream, buf: bytes::BytesMut::new(), - datetime, - scope: scope.into(), - signing_hmac, - previous_signature: seed_signature, + signing, + has_trailer, } } - fn parse_next(input: &[u8]) -> nom::IResult<&[u8], SignedPayload, SignedPayloadStreamError> { + fn parse_next( + input: &[u8], + is_signed: bool, + has_trailer: bool, + ) -> nom::IResult<&[u8], StreamingPayloadChunk, StreamingPayloadError> { use nom::bytes::streaming::{tag, take}; macro_rules! try_parse { @@ -247,17 +347,30 @@ where }; } - let (input, header) = try_parse!(payload::Header::parse(input)); + let (input, header) = if is_signed { + try_parse!(payload::ChunkHeader::parse_signed(input)) + } else { + try_parse!(payload::ChunkHeader::parse_unsigned(input)) + }; // 0-sized chunk is the last if header.size == 0 { - return Ok(( - input, - SignedPayload { - header, - data: Bytes::new(), - }, - )); + if has_trailer { + let (input, trailer) = if is_signed { + try_parse!(payload::TrailerChunk::parse_signed(input)) + } else { + try_parse!(payload::TrailerChunk::parse_unsigned(input)) + }; + return Ok((input, StreamingPayloadChunk::Trailer(trailer))); + } else { + return Ok(( + input, + StreamingPayloadChunk::Chunk { + header, + data: Bytes::new(), + }, + )); + } } let (input, data) = try_parse!(take::<_, _, nom::error::Error<_>>(header.size)(input)); @@ -265,15 +378,15 @@ where let data = Bytes::from(data.to_vec()); - Ok((input, SignedPayload { header, data })) + Ok((input, StreamingPayloadChunk::Chunk { header, data })) } } -impl Stream for SignedPayloadStream +impl Stream for StreamingPayloadStream where S: Stream> + Unpin, { - type Item = Result; + type Item = Result; fn poll_next( self: Pin<&mut Self>, @@ -284,55 +397,92 @@ where let mut this = self.project(); loop { - let (input, payload) = match Self::parse_next(this.buf) { - Ok(res) => res, - Err(nom::Err::Incomplete(_)) => { - match futures::ready!(this.stream.as_mut().poll_next(cx)) { - Some(Ok(bytes)) => { - this.buf.extend(bytes); - continue; - } - Some(Err(e)) => { - return Poll::Ready(Some(Err(SignedPayloadStreamError::Stream(e)))) - } - None => { - return Poll::Ready(Some(Err(SignedPayloadStreamError::message( - "Unexpected EOF", - )))); + let (input, payload) = + match Self::parse_next(this.buf, this.signing.is_some(), *this.has_trailer) { + Ok(res) => res, + Err(nom::Err::Incomplete(_)) => { + match futures::ready!(this.stream.as_mut().poll_next(cx)) { + Some(Ok(bytes)) => { + this.buf.extend(bytes); + continue; + } + Some(Err(e)) => { + return Poll::Ready(Some(Err(StreamingPayloadError::Stream(e)))) + } + None => { + return Poll::Ready(Some(Err(StreamingPayloadError::message( + "Unexpected EOF", + )))); + } } } + Err(nom::Err::Error(e)) | Err(nom::Err::Failure(e)) => { + return Poll::Ready(Some(Err(e))) + } + }; + + match payload { + StreamingPayloadChunk::Chunk { data, header } => { + if let Some(signing) = this.signing.as_mut() { + let data_sha256sum = sha256sum(&data); + + let expected_signature = compute_streaming_payload_signature( + &signing.signing_hmac, + signing.datetime, + &signing.scope, + signing.previous_signature, + data_sha256sum, + )?; + + if header.signature.unwrap() != expected_signature { + return Poll::Ready(Some(Err(StreamingPayloadError::InvalidSignature))); + } + + signing.previous_signature = header.signature.unwrap(); + } + + *this.buf = input.into(); + + // 0-sized chunk is the last + if data.is_empty() { + // if there was a trailer, it would have been returned by the parser + assert!(!*this.has_trailer); + return Poll::Ready(None); + } + + return Poll::Ready(Some(Ok(data))); } - Err(nom::Err::Error(e)) | Err(nom::Err::Failure(e)) => { - return Poll::Ready(Some(Err(e))) + StreamingPayloadChunk::Trailer(trailer) => { + if let Some(signing) = this.signing.as_mut() { + let data = [ + &trailer.header_name[..], + &b":"[..], + &trailer.header_value[..], + &b"\n"[..], + ] + .concat(); + let trailer_sha256sum = sha256sum(&data); + + let expected_signature = compute_streaming_trailer_signature( + &signing.signing_hmac, + signing.datetime, + &signing.scope, + signing.previous_signature, + trailer_sha256sum, + )?; + + if trailer.signature.unwrap() != expected_signature { + return Poll::Ready(Some(Err(StreamingPayloadError::InvalidSignature))); + } + } + + *this.buf = input.into(); + + // TODO: handle trailer + + return Poll::Ready(None); } - }; - - // 0-sized chunk is the last - if payload.data.is_empty() { - return Poll::Ready(None); } - - let data_sha256sum = sha256sum(&payload.data); - - let expected_signature = compute_streaming_payload_signature( - this.signing_hmac, - *this.datetime, - this.scope, - *this.previous_signature, - data_sha256sum, - ) - .map_err(|e| { - SignedPayloadStreamError::Message(format!("Could not build signature: {}", e)) - })?; - - if payload.header.signature != expected_signature { - return Poll::Ready(Some(Err(SignedPayloadStreamError::InvalidSignature))); - } - - *this.buf = input.into(); - *this.previous_signature = payload.header.signature; - - return Poll::Ready(Some(Ok(payload.data))); } } @@ -345,7 +495,7 @@ where mod tests { use futures::prelude::*; - use super::{SignedPayloadStream, SignedPayloadStreamError}; + use super::{SignParams, StreamingPayloadError, StreamingPayloadStream}; #[tokio::test] async fn test_interrupted_signed_payload_stream() { @@ -367,12 +517,20 @@ mod tests { let seed_signature = Hash::default(); - let mut stream = - SignedPayloadStream::new(body, signing_hmac, datetime, &scope, seed_signature); + let mut stream = StreamingPayloadStream::new( + body, + Some(SignParams { + signing_hmac, + datetime, + scope, + previous_signature: seed_signature, + }), + false, + ); assert!(stream.try_next().await.is_err()); match stream.try_next().await { - Err(SignedPayloadStreamError::Message(msg)) if msg == "Unexpected EOF" => {} + Err(StreamingPayloadError::Message(msg)) if msg == "Unexpected EOF" => {} item => panic!( "Unexpected result, expected early EOF error, got {:?}", item From c5df820e2c2b4bff5e239b8e99f07178b98b3f5a Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 17 Feb 2025 18:47:06 +0100 Subject: [PATCH 22/43] api: start refactor of signature to calculate checksums earlier --- src/api/common/cors.rs | 8 +- src/api/common/signature/body.rs | 69 +++++++++++++ src/api/common/signature/checksum.rs | 135 +++++++++++++++++++++++++- src/api/common/signature/mod.rs | 12 +-- src/api/common/signature/payload.rs | 2 +- src/api/common/signature/streaming.rs | 52 +++++++--- src/api/k2v/batch.rs | 8 +- src/api/k2v/item.rs | 4 +- src/api/s3/api_server.rs | 27 ++---- src/api/s3/bucket.rs | 10 +- src/api/s3/checksum.rs | 108 --------------------- src/api/s3/copy.rs | 1 - src/api/s3/cors.rs | 11 +-- src/api/s3/delete.rs | 9 +- src/api/s3/get.rs | 16 +-- src/api/s3/lifecycle.rs | 10 +- src/api/s3/multipart.rs | 14 ++- src/api/s3/post_object.rs | 1 - src/api/s3/put.rs | 4 +- src/api/s3/website.rs | 10 +- src/web/web_server.rs | 8 +- 21 files changed, 288 insertions(+), 231 deletions(-) create mode 100644 src/api/common/signature/body.rs diff --git a/src/api/common/cors.rs b/src/api/common/cors.rs index 14369b56..09b55c13 100644 --- a/src/api/common/cors.rs +++ b/src/api/common/cors.rs @@ -14,9 +14,9 @@ use crate::common_error::{ }; use crate::helpers::*; -pub fn find_matching_cors_rule<'a>( +pub fn find_matching_cors_rule<'a, B>( bucket_params: &'a BucketParams, - req: &Request, + req: &Request, ) -> Result, CommonError> { if let Some(cors_config) = bucket_params.cors_config.get() { if let Some(origin) = req.headers().get("Origin") { @@ -132,8 +132,8 @@ pub async fn handle_options_api( } } -pub fn handle_options_for_bucket( - req: &Request, +pub fn handle_options_for_bucket( + req: &Request, bucket_params: &BucketParams, ) -> Result, CommonError> { let origin = req diff --git a/src/api/common/signature/body.rs b/src/api/common/signature/body.rs new file mode 100644 index 00000000..877d8d85 --- /dev/null +++ b/src/api/common/signature/body.rs @@ -0,0 +1,69 @@ +use std::sync::Mutex; + +use futures::prelude::*; +use futures::stream::BoxStream; +use http_body_util::{BodyExt, StreamBody}; +use hyper::body::{Bytes, Frame}; +use serde::Deserialize; +use tokio::sync::{mpsc, oneshot}; + +use super::*; + +use crate::signature::checksum::*; + +pub struct ReqBody { + // why need mutex to be sync?? + pub stream: Mutex, Error>>>, + pub checksummer: Checksummer, + pub expected_checksums: ExpectedChecksums, +} + +pub type StreamingChecksumReceiver = oneshot::Receiver>; + +impl ReqBody { + pub async fn json Deserialize<'a>>(self) -> Result { + let body = self.collect().await?; + let resp: T = serde_json::from_slice(&body).ok_or_bad_request("Invalid JSON")?; + Ok(resp) + } + + pub async fn collect(self) -> Result { + self.collect_with_checksums().await.map(|(b, _)| b) + } + + pub async fn collect_with_checksums(mut self) -> Result<(Bytes, Checksums), Error> { + let stream: BoxStream<_> = self.stream.into_inner().unwrap(); + let bytes = BodyExt::collect(StreamBody::new(stream)).await?.to_bytes(); + + self.checksummer.update(&bytes); + let checksums = self.checksummer.finalize(); + checksums.verify(&self.expected_checksums)?; + + Ok((bytes, checksums)) + } + + pub fn streaming(self) -> impl Stream> { + self.streaming_with_checksums(false).0 + } + + pub fn streaming_with_checksums( + self, + add_md5: bool, + ) -> ( + impl Stream>, + StreamingChecksumReceiver, + ) { + let (tx, rx) = oneshot::channel(); + // TODO: actually calculate checksums!! + let stream: BoxStream<_> = self.stream.into_inner().unwrap(); + ( + stream.map(|x| { + x.and_then(|f| { + f.into_data() + .map_err(|_| Error::bad_request("non-data frame")) + }) + }), + rx, + ) + } +} diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs index 432ed44d..b184fc65 100644 --- a/src/api/common/signature/checksum.rs +++ b/src/api/common/signature/checksum.rs @@ -8,13 +8,15 @@ use md5::{Digest, Md5}; use sha1::Sha1; use sha2::Sha256; -use http::HeaderName; +use http::{HeaderMap, HeaderName, HeaderValue}; use garage_util::data::*; use garage_model::s3::object_table::{ChecksumAlgorithm, ChecksumValue}; -use super::error::*; +use super::*; + +pub const CONTENT_MD5: HeaderName = HeaderName::from_static("content-md5"); pub const X_AMZ_CHECKSUM_ALGORITHM: HeaderName = HeaderName::from_static("x-amz-checksum-algorithm"); @@ -58,14 +60,18 @@ pub struct Checksums { } impl Checksummer { - pub fn init(expected: &ExpectedChecksums, require_md5: bool) -> Self { - let mut ret = Self { + pub fn new() -> Self { + Self { crc32: None, crc32c: None, md5: None, sha1: None, sha256: None, - }; + } + } + + pub fn init(expected: &ExpectedChecksums, require_md5: bool) -> Self { + let mut ret = Self::new(); if expected.md5.is_some() || require_md5 { ret.md5 = Some(Md5::new()); @@ -179,3 +185,122 @@ impl Checksums { } } } + +// ---- + +/// Extract the value of the x-amz-checksum-algorithm header +pub fn request_checksum_algorithm( + headers: &HeaderMap, +) -> Result, 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")), + } +} + +pub fn request_trailer_checksum_algorithm( + headers: &HeaderMap, +) -> Result, Error> { + match headers.get(X_AMZ_TRAILER).map(|x| x.to_str()).transpose()? { + None => Ok(None), + Some(x) if x == X_AMZ_CHECKSUM_CRC32 => Ok(Some(ChecksumAlgorithm::Crc32)), + Some(x) if x == X_AMZ_CHECKSUM_CRC32C => Ok(Some(ChecksumAlgorithm::Crc32c)), + Some(x) if x == X_AMZ_CHECKSUM_SHA1 => Ok(Some(ChecksumAlgorithm::Sha1)), + Some(x) if x == X_AMZ_CHECKSUM_SHA256 => Ok(Some(ChecksumAlgorithm::Sha256)), + _ => Err(Error::bad_request("invalid checksum algorithm")), + } +} + +/// Extract the value of any of the x-amz-checksum-* headers +pub fn request_checksum_value( + headers: &HeaderMap, +) -> Result, 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 fn request_checksum_algorithm_value( + headers: &HeaderMap, +) -> Result, 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), + } +} diff --git a/src/api/common/signature/mod.rs b/src/api/common/signature/mod.rs index 2421d696..e93ca85a 100644 --- a/src/api/common/signature/mod.rs +++ b/src/api/common/signature/mod.rs @@ -11,6 +11,7 @@ use garage_util::data::{sha256sum, Hash}; use error::*; +pub mod body; pub mod checksum; pub mod error; pub mod payload; @@ -51,7 +52,7 @@ pub const AWS4_HMAC_SHA256_PAYLOAD: &str = "AWS4-HMAC-SHA256-PAYLOAD"; #[derive(Debug)] pub enum ContentSha256Header { UnsignedPayload, - Sha256Hash(Hash), + Sha256Checksum(Hash), StreamingPayload { trailer: bool, signed: bool }, } @@ -90,15 +91,6 @@ pub async fn verify_request( }) } -pub fn verify_signed_content(expected_sha256: Hash, body: &[u8]) -> Result<(), Error> { - if expected_sha256 != sha256sum(body) { - return Err(Error::bad_request( - "Request content hash does not match signed hash".to_string(), - )); - } - Ok(()) -} - pub fn signing_hmac( datetime: &DateTime, secret_key: &str, diff --git a/src/api/common/signature/payload.rs b/src/api/common/signature/payload.rs index ccc55c90..4ca0153f 100644 --- a/src/api/common/signature/payload.rs +++ b/src/api/common/signature/payload.rs @@ -94,7 +94,7 @@ fn parse_x_amz_content_sha256(header: Option<&str>) -> Result; +pub use crate::signature::body::ReqBody; pub fn parse_streaming_body( req: Request, @@ -23,6 +24,20 @@ pub fn parse_streaming_body( region: &str, service: &str, ) -> Result, Error> { + let expected_checksums = ExpectedChecksums { + md5: match req.headers().get("content-md5") { + Some(x) => Some(x.to_str()?.to_string()), + None => None, + }, + sha256: match &checked_signature.content_sha256_header { + ContentSha256Header::Sha256Checksum(sha256) => Some(*sha256), + _ => None, + }, + extra: None, + }; + + let mut checksummer = Checksummer::init(&expected_checksums, false); + match checked_signature.content_sha256_header { ContentSha256Header::StreamingPayload { signed, trailer } => { if !signed && !trailer { @@ -31,6 +46,11 @@ pub fn parse_streaming_body( )); } + if trailer { + let algo = request_trailer_checksum_algorithm(req.headers())?; + checksummer = checksummer.add(algo); + } + let sign_params = if signed { let signature = checked_signature .signature_header @@ -77,14 +97,24 @@ pub fn parse_streaming_body( Ok(req.map(move |body| { let stream = body_stream::<_, Error>(body); + let signed_payload_stream = - StreamingPayloadStream::new(stream, sign_params, trailer) - .map(|x| x.map(hyper::body::Frame::data)) - .map_err(Error::from); - ReqBody::new(StreamBody::new(signed_payload_stream)) + StreamingPayloadStream::new(stream, sign_params, trailer).map_err(Error::from); + ReqBody { + stream: Mutex::new(signed_payload_stream.boxed()), + checksummer, + expected_checksums, + } })) } - _ => Ok(req.map(|body| ReqBody::new(http_body_util::BodyExt::map_err(body, Error::from)))), + _ => Ok(req.map(|body| { + let stream = http_body_util::BodyStream::new(body).map_err(Error::from); + ReqBody { + stream: Mutex::new(stream.boxed()), + checksummer, + expected_checksums, + } + })), } } @@ -386,7 +416,7 @@ impl Stream for StreamingPayloadStream where S: Stream> + Unpin, { - type Item = Result; + type Item = Result, StreamingPayloadError>; fn poll_next( self: Pin<&mut Self>, @@ -450,7 +480,7 @@ where return Poll::Ready(None); } - return Poll::Ready(Some(Ok(data))); + return Poll::Ready(Some(Ok(Frame::data(data)))); } StreamingPayloadChunk::Trailer(trailer) => { if let Some(signing) = this.signing.as_mut() { diff --git a/src/api/k2v/batch.rs b/src/api/k2v/batch.rs index c284dbd4..7a03d836 100644 --- a/src/api/k2v/batch.rs +++ b/src/api/k2v/batch.rs @@ -20,7 +20,7 @@ pub async fn handle_insert_batch( let ReqCtx { garage, bucket_id, .. } = &ctx; - let items = parse_json_body::, _, Error>(req).await?; + let items = req.into_body().json::>().await?; let mut items2 = vec![]; for it in items { @@ -47,7 +47,7 @@ pub async fn handle_read_batch( ctx: ReqCtx, req: Request, ) -> Result, Error> { - let queries = parse_json_body::, _, Error>(req).await?; + let queries = req.into_body().json::>().await?; let resp_results = futures::future::join_all( queries @@ -141,7 +141,7 @@ pub async fn handle_delete_batch( ctx: ReqCtx, req: Request, ) -> Result, Error> { - let queries = parse_json_body::, _, Error>(req).await?; + let queries = req.into_body().json::>().await?; let resp_results = futures::future::join_all( queries @@ -262,7 +262,7 @@ pub(crate) async fn handle_poll_range( } = ctx; use garage_model::k2v::sub::PollRange; - let query = parse_json_body::(req).await?; + let query = req.into_body().json::().await?; let timeout_msec = query.timeout.unwrap_or(300).clamp(1, 600) * 1000; diff --git a/src/api/k2v/item.rs b/src/api/k2v/item.rs index 4e28b499..0fb945d2 100644 --- a/src/api/k2v/item.rs +++ b/src/api/k2v/item.rs @@ -144,9 +144,7 @@ pub async fn handle_insert_item( .map(parse_causality_token) .transpose()?; - let body = http_body_util::BodyExt::collect(req.into_body()) - .await? - .to_bytes(); + let body = req.into_body().collect().await?; let value = DvvsValue::Value(body.to_vec()); diff --git a/src/api/s3/api_server.rs b/src/api/s3/api_server.rs index 0fdaab70..fe6545cc 100644 --- a/src/api/s3/api_server.rs +++ b/src/api/s3/api_server.rs @@ -125,7 +125,7 @@ impl ApiHandler for S3ApiServer { let req = verified_request.request; let api_key = verified_request.access_key; let content_sha256 = match verified_request.content_sha256_header { - ContentSha256Header::Sha256Hash(h) => Some(h), + ContentSha256Header::Sha256Checksum(h) => Some(h), // TODO take into account streaming/trailer checksums, etc. _ => None, }; @@ -141,14 +141,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 @@ -186,7 +179,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, @@ -206,7 +199,7 @@ 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, @@ -228,7 +221,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 {} => { @@ -331,17 +324,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())), }; diff --git a/src/api/s3/bucket.rs b/src/api/s3/bucket.rs index 0a192ba6..3a09e769 100644 --- a/src/api/s3/bucket.rs +++ b/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, - content_sha256: Option, api_key_id: &String, bucket_name: String, ) -> Result, 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")?; diff --git a/src/api/s3/checksum.rs b/src/api/s3/checksum.rs index a720a82f..8e6096b6 100644 --- a/src/api/s3/checksum.rs +++ b/src/api/s3/checksum.rs @@ -8,8 +8,6 @@ use md5::{Digest, Md5}; use sha1::Sha1; use sha2::Sha256; -use http::{HeaderMap, HeaderValue}; - use garage_util::error::OkOrMessage; use garage_model::s3::object_table::*; @@ -112,112 +110,6 @@ impl MultipartChecksummer { } } -// ---- - -/// Extract the value of the x-amz-checksum-algorithm header -pub(crate) fn request_checksum_algorithm( - headers: &HeaderMap, -) -> Result, 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, -) -> Result, 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, -) -> Result, 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, mut resp: http::response::Builder, diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs index 4bf68406..9ae48807 100644 --- a/src/api/s3/copy.rs +++ b/src/api/s3/copy.rs @@ -24,7 +24,6 @@ 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; diff --git a/src/api/s3/cors.rs b/src/api/s3/cors.rs index 625b84db..fcfdb934 100644 --- a/src/api/s3/cors.rs +++ b/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, Error> pub async fn handle_put_cors( ctx: ReqCtx, req: Request, - content_sha256: Option, ) -> Result, 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()?; diff --git a/src/api/s3/delete.rs b/src/api/s3/delete.rs index b799e67a..d785b9d8 100644 --- a/src/api/s3/delete.rs +++ b/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, pub async fn handle_delete_objects( ctx: ReqCtx, req: Request, - content_sha256: Option, ) -> Result, 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")?; diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs index 6627cf4a..16e2b935 100644 --- a/src/api/s3/get.rs +++ b/src/api/s3/get.rs @@ -12,7 +12,7 @@ use http::header::{ CONTENT_LENGTH, CONTENT_RANGE, CONTENT_TYPE, ETAG, EXPIRES, IF_MODIFIED_SINCE, IF_NONE_MATCH, LAST_MODIFIED, RANGE, }; -use hyper::{body::Body, Request, Response, StatusCode}; +use hyper::{Request, Response, StatusCode}; use tokio::sync::mpsc; use garage_net::stream::ByteStream; @@ -119,7 +119,7 @@ fn getobject_override_headers( fn try_answer_cached( version: &ObjectVersion, version_meta: &ObjectVersionMeta, - req: &Request, + req: &Request<()>, ) -> Option> { // 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 @@ -158,7 +158,7 @@ fn try_answer_cached( /// Handle HEAD request pub async fn handle_head( ctx: ReqCtx, - req: &Request, + req: &Request<()>, key: &str, part_number: Option, ) -> Result, Error> { @@ -168,7 +168,7 @@ pub async fn handle_head( /// Handle HEAD request for website pub async fn handle_head_without_ctx( garage: Arc, - req: &Request, + req: &Request<()>, bucket_id: Uuid, key: &str, part_number: Option, @@ -279,7 +279,7 @@ pub async fn handle_head_without_ctx( /// Handle GET request pub async fn handle_get( ctx: ReqCtx, - req: &Request, + req: &Request<()>, key: &str, part_number: Option, overrides: GetObjectOverrides, @@ -290,7 +290,7 @@ pub async fn handle_get( /// Handle GET request pub async fn handle_get_without_ctx( garage: Arc, - req: &Request, + req: &Request<()>, bucket_id: Uuid, key: &str, part_number: Option, @@ -578,7 +578,7 @@ async fn handle_get_part( } fn parse_range_header( - req: &Request, + req: &Request<()>, total_size: u64, ) -> Result, Error> { let range = match req.headers().get(RANGE) { @@ -619,7 +619,7 @@ struct ChecksumMode { enabled: bool, } -fn checksum_mode(req: &Request) -> ChecksumMode { +fn checksum_mode(req: &Request<()>) -> ChecksumMode { ChecksumMode { enabled: req .headers() diff --git a/src/api/s3/lifecycle.rs b/src/api/s3/lifecycle.rs index c35047ed..c140494e 100644 --- a/src/api/s3/lifecycle.rs +++ b/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, Error> { let ReqCtx { bucket_params, .. } = ctx; @@ -56,7 +53,6 @@ pub async fn handle_delete_lifecycle(ctx: ReqCtx) -> Result, E pub async fn handle_put_lifecycle( ctx: ReqCtx, req: Request, - content_sha256: Option, ) -> Result, 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 diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index 7f8d6440..f381d670 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -17,7 +17,6 @@ use garage_model::s3::version_table::*; use garage_api_common::helpers::*; use garage_api_common::signature::checksum::*; -use garage_api_common::signature::verify_signed_content; use crate::api_server::{ReqBody, ResBody}; use crate::checksum::*; @@ -114,7 +113,11 @@ pub async fn handle_put_part( let key = key.to_string(); let (req_head, req_body) = req.into_parts(); - let stream = body_stream(req_body); + + let (stream, checksums) = req_body.streaming_with_checksums(true); + let stream = stream.map_err(Error::from); + // TODO checksums + let mut chunker = StreamChunker::new(stream, garage.config.block_size); let ((_, object_version, mut mpu), first_block) = @@ -249,7 +252,6 @@ pub async fn handle_complete_multipart_upload( req: Request, key: &str, upload_id: &str, - content_sha256: Option, ) -> Result, Error> { let ReqCtx { garage, @@ -261,11 +263,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) diff --git a/src/api/s3/post_object.rs b/src/api/s3/post_object.rs index 908ee9f3..6c1b7453 100644 --- a/src/api/s3/post_object.rs +++ b/src/api/s3/post_object.rs @@ -22,7 +22,6 @@ 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}; diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 834be6f1..551c3b76 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -79,7 +79,9 @@ 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()); + let (stream, checksums) = req.into_body().streaming_with_checksums(true); + let stream = stream.map_err(Error::from); + // TODO checksums let res = save_stream( &ctx, diff --git a/src/api/s3/website.rs b/src/api/s3/website.rs index b55bb345..7553bef7 100644 --- a/src/api/s3/website.rs +++ b/src/api/s3/website.rs @@ -1,14 +1,11 @@ use quick_xml::de::from_reader; -use http_body_util::BodyExt; use hyper::{Request, Response, StatusCode}; use serde::{Deserialize, Serialize}; use garage_model::bucket_table::*; -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::*; @@ -61,7 +58,6 @@ pub async fn handle_delete_website(ctx: ReqCtx) -> Result, Err pub async fn handle_put_website( ctx: ReqCtx, req: Request, - content_sha256: Option, ) -> Result, Error> { let ReqCtx { garage, @@ -70,11 +66,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()?; diff --git a/src/web/web_server.rs b/src/web/web_server.rs index e73dab48..34ba834c 100644 --- a/src/web/web_server.rs +++ b/src/web/web_server.rs @@ -1,6 +1,6 @@ use std::fs::{self, Permissions}; use std::os::unix::prelude::PermissionsExt; -use std::{convert::Infallible, sync::Arc}; +use std::sync::Arc; use tokio::net::{TcpListener, UnixListener}; use tokio::sync::watch; @@ -163,6 +163,8 @@ impl WebServer { metrics_tags.push(KeyValue::new("host", host_header.clone())); } + let req = req.map(|_| ()); + // The actual handler let res = self .serve_file(&req) @@ -218,7 +220,7 @@ impl WebServer { async fn serve_file( self: &Arc, - req: &Request, + req: &Request<()>, ) -> Result>, Error> { // Get http authority string (eg. [::1]:3902 or garage.tld:80) let authority = req @@ -322,7 +324,7 @@ impl WebServer { // Create a fake HTTP request with path = the error document let req2 = Request::builder() .uri(format!("http://{}/{}", host, &error_document)) - .body(empty_body::()) + .body(()) .unwrap(); match handle_get_without_ctx( From 658541d812103662be88ad6d3d1c0fdf1a948862 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 17 Feb 2025 19:54:25 +0100 Subject: [PATCH 23/43] api: use checksumming in api_common::signature for put/putpart --- src/api/common/signature/body.rs | 112 +++++++++++++++++++++----- src/api/common/signature/checksum.rs | 44 ++++++---- src/api/common/signature/streaming.rs | 6 +- src/api/s3/api_server.rs | 11 +-- src/api/s3/multipart.rs | 27 ++++--- src/api/s3/put.rs | 44 +++++++--- 6 files changed, 170 insertions(+), 74 deletions(-) diff --git a/src/api/common/signature/body.rs b/src/api/common/signature/body.rs index 877d8d85..d8c15ee5 100644 --- a/src/api/common/signature/body.rs +++ b/src/api/common/signature/body.rs @@ -5,7 +5,13 @@ use futures::stream::BoxStream; use http_body_util::{BodyExt, StreamBody}; use hyper::body::{Bytes, Frame}; use serde::Deserialize; -use tokio::sync::{mpsc, oneshot}; +use tokio::sync::mpsc; +use tokio::task; + +use opentelemetry::{ + trace::{FutureExt as OtelFutureExt, TraceContextExt, Tracer}, + Context, +}; use super::*; @@ -13,14 +19,33 @@ use crate::signature::checksum::*; pub struct ReqBody { // why need mutex to be sync?? - pub stream: Mutex, Error>>>, - pub checksummer: Checksummer, - pub expected_checksums: ExpectedChecksums, + pub(crate) stream: Mutex, Error>>>, + pub(crate) checksummer: Checksummer, + pub(crate) expected_checksums: ExpectedChecksums, } -pub type StreamingChecksumReceiver = oneshot::Receiver>; +pub type StreamingChecksumReceiver = task::JoinHandle>; impl ReqBody { + pub fn add_expected_checksums(&mut self, more: ExpectedChecksums) { + if more.md5.is_some() { + self.expected_checksums.md5 = more.md5; + } + if more.sha256.is_some() { + self.expected_checksums.sha256 = more.sha256; + } + if more.extra.is_some() { + self.expected_checksums.extra = more.extra; + } + self.checksummer.add_expected(&self.expected_checksums); + } + + pub fn add_md5(&mut self) { + self.checksummer.add_md5(); + } + + // ============ non-streaming ============= + pub async fn json Deserialize<'a>>(self) -> Result { let body = self.collect().await?; let resp: T = serde_json::from_slice(&body).ok_or_bad_request("Invalid JSON")?; @@ -42,28 +67,71 @@ impl ReqBody { Ok((bytes, checksums)) } - pub fn streaming(self) -> impl Stream> { - self.streaming_with_checksums(false).0 - } + // ============ streaming ============= pub fn streaming_with_checksums( self, - add_md5: bool, ) -> ( - impl Stream>, + BoxStream<'static, Result>, StreamingChecksumReceiver, ) { - let (tx, rx) = oneshot::channel(); - // TODO: actually calculate checksums!! - let stream: BoxStream<_> = self.stream.into_inner().unwrap(); - ( - stream.map(|x| { - x.and_then(|f| { - f.into_data() - .map_err(|_| Error::bad_request("non-data frame")) - }) - }), - rx, - ) + let Self { + stream, + mut checksummer, + mut expected_checksums, + } = self; + + let (frame_tx, mut frame_rx) = mpsc::channel::>(1); + + let join_checksums = tokio::spawn(async move { + let tracer = opentelemetry::global::tracer("garage"); + + while let Some(frame) = frame_rx.recv().await { + match frame.into_data() { + Ok(data) => { + checksummer = tokio::task::spawn_blocking(move || { + checksummer.update(&data); + checksummer + }) + .await + .unwrap() + } + Err(frame) => { + let trailers = frame.into_trailers().unwrap(); + if let Some(cv) = request_checksum_value(&trailers)? { + expected_checksums.extra = Some(cv); + } + break; + } + } + } + + let checksums = checksummer.finalize(); + checksums.verify(&expected_checksums)?; + + return Ok(checksums); + }); + + let stream: BoxStream<_> = stream.into_inner().unwrap(); + let stream = stream.filter_map(move |x| { + let frame_tx = frame_tx.clone(); + async move { + match x { + Err(e) => Some(Err(e)), + Ok(frame) => { + if frame.is_data() { + let data = frame.data_ref().unwrap().clone(); + let _ = frame_tx.send(frame).await; + Some(Ok(data)) + } else { + let _ = frame_tx.send(frame).await; + None + } + } + } + } + }); + + (stream.boxed(), join_checksums) } } diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs index b184fc65..a9f00423 100644 --- a/src/api/common/signature/checksum.rs +++ b/src/api/common/signature/checksum.rs @@ -32,7 +32,7 @@ pub type Md5Checksum = [u8; 16]; pub type Sha1Checksum = [u8; 20]; pub type Sha256Checksum = [u8; 32]; -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub struct ExpectedChecksums { // base64-encoded md5 (content-md5 header) pub md5: Option, @@ -70,27 +70,37 @@ impl Checksummer { } } - pub fn init(expected: &ExpectedChecksums, require_md5: bool) -> Self { + pub fn init(expected: &ExpectedChecksums, add_md5: bool) -> Self { let mut ret = Self::new(); - - 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.add_expected(expected); + if add_md5 { + ret.add_md5(); } ret } + pub fn add_md5(&mut self) { + self.md5 = Some(Md5::new()); + } + + pub fn add_expected(&mut self, expected: &ExpectedChecksums) { + if expected.md5.is_some() { + self.md5 = Some(Md5::new()); + } + if expected.sha256.is_some() || matches!(&expected.extra, Some(ChecksumValue::Sha256(_))) { + self.sha256 = Some(Sha256::new()); + } + if matches!(&expected.extra, Some(ChecksumValue::Crc32(_))) { + self.crc32 = Some(Crc32::new()); + } + if matches!(&expected.extra, Some(ChecksumValue::Crc32c(_))) { + self.crc32c = Some(Crc32c::default()); + } + if matches!(&expected.extra, Some(ChecksumValue::Sha1(_))) { + self.sha1 = Some(Sha1::new()); + } + } + pub fn add(mut self, algo: Option) -> Self { match algo { Some(ChecksumAlgorithm::Crc32) => { diff --git a/src/api/common/signature/streaming.rs b/src/api/common/signature/streaming.rs index e8f9b3d7..3ffc5b2f 100644 --- a/src/api/common/signature/streaming.rs +++ b/src/api/common/signature/streaming.rs @@ -25,15 +25,11 @@ pub fn parse_streaming_body( service: &str, ) -> Result, Error> { let expected_checksums = ExpectedChecksums { - md5: match req.headers().get("content-md5") { - Some(x) => Some(x.to_str()?.to_string()), - None => None, - }, sha256: match &checked_signature.content_sha256_header { ContentSha256Header::Sha256Checksum(sha256) => Some(*sha256), _ => None, }, - extra: None, + ..Default::default() }; let mut checksummer = Checksummer::init(&expected_checksums, false); diff --git a/src/api/s3/api_server.rs b/src/api/s3/api_server.rs index fe6545cc..e26c2b65 100644 --- a/src/api/s3/api_server.rs +++ b/src/api/s3/api_server.rs @@ -15,7 +15,7 @@ use garage_model::key_table::Key; use garage_api_common::cors::*; use garage_api_common::generic_server::*; use garage_api_common::helpers::*; -use garage_api_common::signature::{verify_request, ContentSha256Header}; +use garage_api_common::signature::verify_request; use crate::bucket::*; use crate::copy::*; @@ -124,11 +124,6 @@ impl ApiHandler for S3ApiServer { let verified_request = verify_request(&garage, req, "s3").await?; let req = verified_request.request; let api_key = verified_request.access_key; - let content_sha256 = match verified_request.content_sha256_header { - ContentSha256Header::Sha256Checksum(h) => Some(h), - // TODO take into account streaming/trailer checksums, etc. - _ => None, - }; let bucket_name = match bucket_name { None => { @@ -205,14 +200,14 @@ impl ApiHandler for S3ApiServer { 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 } diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index f381d670..59a469d1 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -94,7 +94,6 @@ pub async fn handle_put_part( key: &str, part_number: u64, upload_id: &str, - content_sha256: Option, ) -> Result, Error> { let ReqCtx { garage, .. } = &ctx; @@ -105,18 +104,23 @@ 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 (req_head, mut req_body) = req.into_parts(); - let (stream, checksums) = req_body.streaming_with_checksums(true); + 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() { + req_body.add_md5(); + } + + let (stream, stream_checksums) = req_body.streaming_with_checksums(); let stream = stream.map_err(Error::from); - // TODO checksums let mut chunker = StreamChunker::new(stream, garage.config.block_size); @@ -176,21 +180,22 @@ 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( + // TODO don't duplicate checksums + 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)?; + let checksums = stream_checksums + .await + .ok_or_internal_error("checksum calculation")??; // Store part etag in version let etag = encryption.etag_from_md5(&checksums.md5); diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 551c3b76..24f888bc 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -31,6 +31,7 @@ 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}; @@ -49,6 +50,7 @@ pub(crate) struct SaveStreamResult { pub(crate) enum ChecksumMode<'a> { Verify(&'a ExpectedChecksums), + VerifyFrom(StreamingChecksumReceiver), Calculate(Option), } @@ -56,7 +58,6 @@ pub async fn handle_put( ctx: ReqCtx, req: Request, key: &String, - content_sha256: Option, ) -> Result, Error> { // Retrieve interesting headers from request let headers = get_headers(req.headers())?; @@ -67,7 +68,7 @@ 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())?, }; @@ -79,9 +80,14 @@ 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, checksums) = req.into_body().streaming_with_checksums(true); + let mut req_body = req.into_body(); + req_body.add_expected_checksums(expected_checksums.clone()); + if !encryption.is_encrypted() { + req_body.add_md5(); + } + + let (stream, checksums) = req_body.streaming_with_checksums(); let stream = stream.map_err(Error::from); - // TODO checksums let res = save_stream( &ctx, @@ -89,7 +95,7 @@ pub async fn handle_put( encryption, stream, key, - ChecksumMode::Verify(&expected_checksums), + ChecksumMode::VerifyFrom(checksums), ) .await?; @@ -125,10 +131,15 @@ pub(crate) async fn save_stream> + 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() } }; @@ -136,7 +147,7 @@ pub(crate) async fn save_stream> + 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) => { @@ -145,6 +156,12 @@ pub(crate) async fn save_stream> + Unpin>( ChecksumMode::Calculate(algo) => { meta.checksum = checksums.extract(algo); } + ChecksumMode::VerifyFrom(checksummer) => { + drop(chunker); + checksums = checksummer + .await + .ok_or_internal_error("checksum calculation")??; + } }; let size = first_block.len() as u64; @@ -216,13 +233,13 @@ pub(crate) async fn save_stream> + 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?; @@ -235,6 +252,11 @@ pub(crate) async fn save_stream> + Unpin>( ChecksumMode::Calculate(algo) => { meta.checksum = checksums.extract(algo); } + ChecksumMode::VerifyFrom(checksummer) => { + checksums = checksummer + .await + .ok_or_internal_error("checksum calculation")??; + } }; // Verify quotas are respsected @@ -335,7 +357,7 @@ pub(crate) async fn read_and_put_blocks> + encryption: EncryptionParams, part_number: u64, first_block: Bytes, - chunker: &mut StreamChunker, + mut chunker: StreamChunker, checksummer: Checksummer, ) -> Result<(u64, Checksums, Hash), Error> { let tracer = opentelemetry::global::tracer("garage"); From 21c0dda16a9a97412cfd5c0232c382388d25ec56 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 17 Feb 2025 20:11:06 +0100 Subject: [PATCH 24/43] api: refactor: move checksumming code around again --- src/api/common/signature/body.rs | 7 -- src/api/common/signature/checksum.rs | 22 +++++ src/api/common/signature/mod.rs | 1 - src/api/s3/checksum.rs | 133 --------------------------- src/api/s3/get.rs | 3 +- src/api/s3/lib.rs | 1 - src/api/s3/multipart.rs | 106 ++++++++++++++++++++- src/api/s3/put.rs | 1 - 8 files changed, 127 insertions(+), 147 deletions(-) delete mode 100644 src/api/s3/checksum.rs diff --git a/src/api/common/signature/body.rs b/src/api/common/signature/body.rs index d8c15ee5..512d02b3 100644 --- a/src/api/common/signature/body.rs +++ b/src/api/common/signature/body.rs @@ -8,11 +8,6 @@ use serde::Deserialize; use tokio::sync::mpsc; use tokio::task; -use opentelemetry::{ - trace::{FutureExt as OtelFutureExt, TraceContextExt, Tracer}, - Context, -}; - use super::*; use crate::signature::checksum::*; @@ -84,8 +79,6 @@ impl ReqBody { let (frame_tx, mut frame_rx) = mpsc::channel::>(1); let join_checksums = tokio::spawn(async move { - let tracer = opentelemetry::global::tracer("garage"); - while let Some(frame) = frame_rx.recv().await { match frame.into_data() { Ok(data) => { diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs index a9f00423..890c0452 100644 --- a/src/api/common/signature/checksum.rs +++ b/src/api/common/signature/checksum.rs @@ -314,3 +314,25 @@ pub fn request_checksum_algorithm_value( None => Ok(None), } } + +pub fn add_checksum_response_headers( + checksum: &Option, + 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 +} diff --git a/src/api/common/signature/mod.rs b/src/api/common/signature/mod.rs index e93ca85a..78518436 100644 --- a/src/api/common/signature/mod.rs +++ b/src/api/common/signature/mod.rs @@ -71,7 +71,6 @@ pub async fn verify_request( service: &'static str, ) -> Result { let checked_signature = payload::check_payload_signature(&garage, &mut req, service).await?; - eprintln!("checked signature: {:?}", checked_signature); let request = streaming::parse_streaming_body( req, diff --git a/src/api/s3/checksum.rs b/src/api/s3/checksum.rs deleted file mode 100644 index 8e6096b6..00000000 --- a/src/api/s3/checksum.rs +++ /dev/null @@ -1,133 +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 garage_util::error::OkOrMessage; - -use garage_model::s3::object_table::*; - -use garage_api_common::signature::checksum::*; - -use crate::error::*; - -#[derive(Default)] -pub(crate) struct MultipartChecksummer { - pub md5: Md5, - pub extra: Option, -} - -pub(crate) enum MultipartExtraChecksummer { - Crc32(Crc32), - Crc32c(Crc32c), - Sha1(Sha1), - Sha256(Sha256), -} - -impl MultipartChecksummer { - pub(crate) fn init(algo: Option) -> 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, - ) -> 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) { - 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) - } -} - -pub(crate) fn add_checksum_response_headers( - checksum: &Option, - 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 -} diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs index 16e2b935..15929cd1 100644 --- a/src/api/s3/get.rs +++ b/src/api/s3/get.rs @@ -26,10 +26,9 @@ use garage_model::s3::object_table::*; use garage_model::s3::version_table::*; use garage_api_common::helpers::*; -use garage_api_common::signature::checksum::X_AMZ_CHECKSUM_MODE; +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; use crate::encryption::EncryptionParams; use crate::error::*; diff --git a/src/api/s3/lib.rs b/src/api/s3/lib.rs index fd99b443..4d1d3ef5 100644 --- a/src/api/s3/lib.rs +++ b/src/api/s3/lib.rs @@ -16,7 +16,6 @@ mod post_object; mod put; mod website; -mod checksum; mod encryption; mod router; pub mod xml; diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index 59a469d1..53eff6ad 100644 --- a/src/api/s3/multipart.rs +++ b/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::*; @@ -19,7 +26,6 @@ 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::put::*; @@ -606,3 +612,99 @@ fn parse_complete_multipart_upload_body( Some(parts) } + +// ====== checksummer ==== + +#[derive(Default)] +pub(crate) struct MultipartChecksummer { + pub md5: Md5, + pub extra: Option, +} + +pub(crate) enum MultipartExtraChecksummer { + Crc32(Crc32), + Crc32c(Crc32c), + Sha1(Sha1), + Sha256(Sha256), +} + +impl MultipartChecksummer { + pub(crate) fn init(algo: Option) -> 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, + ) -> 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) { + 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) + } +} diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 24f888bc..6fcf33cb 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -35,7 +35,6 @@ 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::*; From f8b0817ddcfea9c537cb4b8e3a4d62bf394db3a0 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 18 Feb 2025 12:00:41 +0100 Subject: [PATCH 25/43] api: streaming signature: fix trailer parsing --- script/dev-cluster.sh | 2 +- src/api/common/signature/streaming.rs | 23 +++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/script/dev-cluster.sh b/script/dev-cluster.sh index 6b39255a..998ffdb9 100755 --- a/script/dev-cluster.sh +++ b/script/dev-cluster.sh @@ -11,7 +11,7 @@ PATH="${GARAGE_DEBUG}:${GARAGE_RELEASE}:${NIX_RELEASE}:$PATH" FANCYCOLORS=("41m" "42m" "44m" "45m" "100m" "104m") export RUST_BACKTRACE=1 -export RUST_LOG=garage=info,garage_api=debug +export RUST_LOG=garage=info,garage_api_common=debug,garage_api_s3=debug MAIN_LABEL="\e[${FANCYCOLORS[0]}[main]\e[49m" if [ -z "$GARAGE_BIN" ]; then diff --git a/src/api/common/signature/streaming.rs b/src/api/common/signature/streaming.rs index 3ffc5b2f..6afc2621 100644 --- a/src/api/common/signature/streaming.rs +++ b/src/api/common/signature/streaming.rs @@ -24,6 +24,11 @@ pub fn parse_streaming_body( region: &str, service: &str, ) -> Result, Error> { + debug!( + "Content signature mode: {:?}", + checked_signature.content_sha256_header + ); + let expected_checksums = ExpectedChecksums { sha256: match &checked_signature.content_sha256_header { ContentSha256Header::Sha256Checksum(sha256) => Some(*sha256), @@ -243,7 +248,7 @@ mod payload { let (input, header_value) = try_parse!(take_while( |c: u8| c.is_ascii_alphanumeric() || b"+/=".contains(&c) )(input)); - let (input, _) = try_parse!(tag(b"\n")(input)); + let (input, _) = try_parse!(tag(b"\r\n")(input)); Ok(( input, @@ -257,15 +262,7 @@ mod payload { pub fn parse_signed(input: &[u8]) -> nom::IResult<&[u8], Self, Error<&[u8]>> { let (input, trailer) = Self::parse_content(input)?; - let (input, _) = try_parse!(tag(b"\r\n\r\n")(input)); - - Ok((input, trailer)) - } - pub fn parse_unsigned(input: &[u8]) -> nom::IResult<&[u8], Self, Error<&[u8]>> { - let (input, trailer) = Self::parse_content(input)?; - - let (input, _) = try_parse!(tag(b"\r\n")(input)); - + let (input, _) = try_parse!(tag(b"x-amz-trailer-signature:")(input)); let (input, data) = try_parse!(map_res(hex_digit1, hex::decode)(input)); let signature = Hash::try_from(&data).ok_or(nom::Err::Failure(Error::BadSignature))?; let (input, _) = try_parse!(tag(b"\r\n")(input)); @@ -278,6 +275,12 @@ mod payload { }, )) } + pub fn parse_unsigned(input: &[u8]) -> nom::IResult<&[u8], Self, Error<&[u8]>> { + let (input, trailer) = Self::parse_content(input)?; + let (input, _) = try_parse!(tag(b"\r\n")(input)); + + Ok((input, trailer)) + } } } From abb60dcf7e97b19ab7b82015cc12d9006cbe3dda Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 18 Feb 2025 12:27:53 +0100 Subject: [PATCH 26/43] api: remove content-encoding: aws-chunked for streaming payload --- src/api/common/signature/streaming.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/api/common/signature/streaming.rs b/src/api/common/signature/streaming.rs index 6afc2621..17d3a802 100644 --- a/src/api/common/signature/streaming.rs +++ b/src/api/common/signature/streaming.rs @@ -5,6 +5,7 @@ use chrono::{DateTime, NaiveDateTime, TimeZone, Utc}; use futures::prelude::*; use futures::task; use hmac::Mac; +use http::header::{HeaderValue, CONTENT_ENCODING}; use hyper::body::{Bytes, Frame, Incoming as IncomingBody}; use hyper::Request; @@ -19,7 +20,7 @@ use crate::signature::payload::CheckedSignature; pub use crate::signature::body::ReqBody; pub fn parse_streaming_body( - req: Request, + mut req: Request, checked_signature: &CheckedSignature, region: &str, service: &str, @@ -41,17 +42,34 @@ pub fn parse_streaming_body( match checked_signature.content_sha256_header { ContentSha256Header::StreamingPayload { signed, trailer } => { + // Sanity checks if !signed && !trailer { return Err(Error::bad_request( "STREAMING-UNSIGNED-PAYLOAD is not a valid combination", )); } + // Remove the aws-chunked component in the content-encoding: header + // Note: this header is not properly sent by minio client, so don't fail + // if it is absent from the request. + if let Some(content_encoding) = req.headers_mut().remove(CONTENT_ENCODING) { + if let Some(rest) = content_encoding.as_bytes().strip_prefix(b"aws-chunked,") { + req.headers_mut() + .insert(CONTENT_ENCODING, HeaderValue::from_bytes(rest).unwrap()); + } else if content_encoding != "aws-chunked" { + return Err(Error::bad_request( + "content-encoding does not contain aws-chunked for STREAMING-*-PAYLOAD", + )); + } + } + + // If trailer header is announced, add the calculation of the requested checksum if trailer { let algo = request_trailer_checksum_algorithm(req.headers())?; checksummer = checksummer.add(algo); } + // For signed variants, determine signing parameters let sign_params = if signed { let signature = checked_signature .signature_header From ccab0e4ae5c083d0a06ad2b3ef5fe62481da3c2c Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 18 Feb 2025 12:55:45 +0100 Subject: [PATCH 27/43] api: fix optional \n after trailer checksum header --- src/api/common/signature/streaming.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/api/common/signature/streaming.rs b/src/api/common/signature/streaming.rs index 17d3a802..75f3bf80 100644 --- a/src/api/common/signature/streaming.rs +++ b/src/api/common/signature/streaming.rs @@ -189,7 +189,7 @@ mod payload { use nom::bytes::streaming::{tag, take_while}; use nom::character::streaming::hex_digit1; - use nom::combinator::map_res; + use nom::combinator::{map_res, opt}; use nom::number::streaming::hex_u32; macro_rules! try_parse { @@ -266,6 +266,11 @@ mod payload { let (input, header_value) = try_parse!(take_while( |c: u8| c.is_ascii_alphanumeric() || b"+/=".contains(&c) )(input)); + + // Possible '\n' after the header value, depends on clients + // https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html + let (input, _) = try_parse!(opt(tag(b"\n"))(input)); + let (input, _) = try_parse!(tag(b"\r\n")(input)); Ok(( From 730bfee753c4f22cd0595d9195222de334ec36f9 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 18 Feb 2025 13:59:43 +0100 Subject: [PATCH 28/43] api: validate trailing checksum + add test for unsigned-paylad-trailer --- Cargo.lock | 1 + src/api/common/signature/body.rs | 13 +- src/api/common/signature/checksum.rs | 81 +++++------ src/api/common/signature/streaming.rs | 58 +++++--- src/api/s3/post_object.rs | 5 +- src/garage/Cargo.toml | 1 + src/garage/tests/common/custom_requester.rs | 111 +++++++++++++-- src/garage/tests/common/garage.rs | 5 +- src/garage/tests/s3/streaming_signature.rs | 150 +++++++++++++++++++- 9 files changed, 337 insertions(+), 88 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 26f6ea1d..477e4456 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1230,6 +1230,7 @@ dependencies = [ "bytes", "bytesize", "chrono", + "crc32fast", "format_table", "futures", "garage_api_admin", diff --git a/src/api/common/signature/body.rs b/src/api/common/signature/body.rs index 512d02b3..4279d7b5 100644 --- a/src/api/common/signature/body.rs +++ b/src/api/common/signature/body.rs @@ -17,6 +17,7 @@ pub struct ReqBody { pub(crate) stream: Mutex, Error>>>, pub(crate) checksummer: Checksummer, pub(crate) expected_checksums: ExpectedChecksums, + pub(crate) trailer_algorithm: Option, } pub type StreamingChecksumReceiver = task::JoinHandle>; @@ -74,6 +75,7 @@ impl ReqBody { stream, mut checksummer, mut expected_checksums, + trailer_algorithm, } = self; let (frame_tx, mut frame_rx) = mpsc::channel::>(1); @@ -91,18 +93,21 @@ impl ReqBody { } Err(frame) => { let trailers = frame.into_trailers().unwrap(); - if let Some(cv) = request_checksum_value(&trailers)? { - expected_checksums.extra = Some(cv); - } + let algo = trailer_algorithm.unwrap(); + expected_checksums.extra = Some(extract_checksum_value(&trailers, algo)?); break; } } } + if trailer_algorithm.is_some() && expected_checksums.extra.is_none() { + return Err(Error::bad_request("trailing checksum was not sent")); + } + let checksums = checksummer.finalize(); checksums.verify(&expected_checksums)?; - return Ok(checksums); + Ok(checksums) }); let stream: BoxStream<_> = stream.into_inner().unwrap(); diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs index 890c0452..3c5e7c53 100644 --- a/src/api/common/signature/checksum.rs +++ b/src/api/common/signature/checksum.rs @@ -12,10 +12,10 @@ use http::{HeaderMap, HeaderName, HeaderValue}; use garage_util::data::*; -use garage_model::s3::object_table::{ChecksumAlgorithm, ChecksumValue}; - use super::*; +pub use garage_model::s3::object_table::{ChecksumAlgorithm, ChecksumValue}; + pub const CONTENT_MD5: HeaderName = HeaderName::from_static("content-md5"); pub const X_AMZ_CHECKSUM_ALGORITHM: HeaderName = @@ -198,17 +198,23 @@ impl Checksums { // ---- +pub fn parse_checksum_algorithm(algo: &str) -> Result { + match algo { + "CRC32" => Ok(ChecksumAlgorithm::Crc32), + "CRC32C" => Ok(ChecksumAlgorithm::Crc32c), + "SHA1" => Ok(ChecksumAlgorithm::Sha1), + "SHA256" => Ok(ChecksumAlgorithm::Sha256), + _ => Err(Error::bad_request("invalid checksum algorithm")), + } +} + /// Extract the value of the x-amz-checksum-algorithm header pub fn request_checksum_algorithm( headers: &HeaderMap, ) -> Result, 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")), + Some(x) => parse_checksum_algorithm(x.to_str()?).map(Some), } } @@ -231,37 +237,17 @@ pub fn request_checksum_value( ) -> Result, 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 headers.contains_key(X_AMZ_CHECKSUM_CRC32) { + ret.push(extract_checksum_value(headers, ChecksumAlgorithm::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 headers.contains_key(X_AMZ_CHECKSUM_CRC32C) { + ret.push(extract_checksum_value(headers, ChecksumAlgorithm::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 headers.contains_key(X_AMZ_CHECKSUM_SHA1) { + ret.push(extract_checksum_value(headers, ChecksumAlgorithm::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 headers.contains_key(X_AMZ_CHECKSUM_SHA256) { + ret.push(extract_checksum_value(headers, ChecksumAlgorithm::Sha256)?); } if ret.len() > 1 { @@ -274,44 +260,43 @@ pub fn request_checksum_value( /// Checks for the presence of x-amz-checksum-algorithm /// if so extract the corresponding x-amz-checksum-* value -pub fn request_checksum_algorithm_value( +pub fn extract_checksum_value( headers: &HeaderMap, -) -> Result, Error> { - match headers.get(X_AMZ_CHECKSUM_ALGORITHM) { - Some(x) if x == "CRC32" => { + algo: ChecksumAlgorithm, +) -> Result { + match algo { + ChecksumAlgorithm::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))) + Ok(ChecksumValue::Crc32(crc32)) } - Some(x) if x == "CRC32C" => { + ChecksumAlgorithm::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))) + Ok(ChecksumValue::Crc32c(crc32c)) } - Some(x) if x == "SHA1" => { + ChecksumAlgorithm::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))) + Ok(ChecksumValue::Sha1(sha1)) } - Some(x) if x == "SHA256" => { + ChecksumAlgorithm::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))) + Ok(ChecksumValue::Sha256(sha256)) } - Some(_) => Err(Error::bad_request("invalid x-amz-checksum-algorithm")), - None => Ok(None), } } diff --git a/src/api/common/signature/streaming.rs b/src/api/common/signature/streaming.rs index 75f3bf80..70b6e004 100644 --- a/src/api/common/signature/streaming.rs +++ b/src/api/common/signature/streaming.rs @@ -5,7 +5,7 @@ use chrono::{DateTime, NaiveDateTime, TimeZone, Utc}; use futures::prelude::*; use futures::task; use hmac::Mac; -use http::header::{HeaderValue, CONTENT_ENCODING}; +use http::header::{HeaderMap, HeaderValue, CONTENT_ENCODING}; use hyper::body::{Bytes, Frame, Incoming as IncomingBody}; use hyper::Request; @@ -64,10 +64,16 @@ pub fn parse_streaming_body( } // If trailer header is announced, add the calculation of the requested checksum - if trailer { - let algo = request_trailer_checksum_algorithm(req.headers())?; + let trailer_algorithm = if trailer { + let algo = Some( + request_trailer_checksum_algorithm(req.headers())? + .ok_or_bad_request("Missing x-amz-trailer header")?, + ); checksummer = checksummer.add(algo); - } + algo + } else { + None + }; // For signed variants, determine signing parameters let sign_params = if signed { @@ -123,6 +129,7 @@ pub fn parse_streaming_body( stream: Mutex::new(signed_payload_stream.boxed()), checksummer, expected_checksums, + trailer_algorithm, } })) } @@ -132,6 +139,7 @@ pub fn parse_streaming_body( stream: Mutex::new(stream.boxed()), checksummer, expected_checksums, + trailer_algorithm: None, } })), } @@ -185,6 +193,8 @@ fn compute_streaming_trailer_signature( } mod payload { + use http::{HeaderName, HeaderValue}; + use garage_util::data::Hash; use nom::bytes::streaming::{tag, take_while}; @@ -252,19 +262,21 @@ mod payload { #[derive(Debug, Clone)] pub struct TrailerChunk { - pub header_name: Vec, - pub header_value: Vec, + pub header_name: HeaderName, + pub header_value: HeaderValue, pub signature: Option, } impl TrailerChunk { fn parse_content(input: &[u8]) -> nom::IResult<&[u8], Self, Error<&[u8]>> { - let (input, header_name) = try_parse!(take_while( - |c: u8| c.is_ascii_alphanumeric() || c == b'-' + let (input, header_name) = try_parse!(map_res( + take_while(|c: u8| c.is_ascii_alphanumeric() || c == b'-'), + HeaderName::from_bytes )(input)); let (input, _) = try_parse!(tag(b":")(input)); - let (input, header_value) = try_parse!(take_while( - |c: u8| c.is_ascii_alphanumeric() || b"+/=".contains(&c) + let (input, header_value) = try_parse!(map_res( + take_while(|c: u8| c.is_ascii_alphanumeric() || b"+/=".contains(&c)), + HeaderValue::from_bytes )(input)); // Possible '\n' after the header value, depends on clients @@ -276,8 +288,8 @@ mod payload { Ok(( input, TrailerChunk { - header_name: header_name.to_vec(), - header_value: header_value.to_vec(), + header_name, + header_value, signature: None, }, )) @@ -371,6 +383,7 @@ where buf: bytes::BytesMut, signing: Option, has_trailer: bool, + done: bool, } impl StreamingPayloadStream @@ -383,6 +396,7 @@ where buf: bytes::BytesMut::new(), signing, has_trailer, + done: false, } } @@ -448,6 +462,10 @@ where let mut this = self.project(); + if *this.done { + return Poll::Ready(None); + } + loop { let (input, payload) = match Self::parse_next(this.buf, this.signing.is_some(), *this.has_trailer) { @@ -499,17 +517,23 @@ where if data.is_empty() { // if there was a trailer, it would have been returned by the parser assert!(!*this.has_trailer); + *this.done = true; return Poll::Ready(None); } return Poll::Ready(Some(Ok(Frame::data(data)))); } StreamingPayloadChunk::Trailer(trailer) => { + trace!( + "In StreamingPayloadStream::poll_next: got trailer {:?}", + trailer + ); + if let Some(signing) = this.signing.as_mut() { let data = [ - &trailer.header_name[..], + trailer.header_name.as_ref(), &b":"[..], - &trailer.header_value[..], + trailer.header_value.as_ref(), &b"\n"[..], ] .concat(); @@ -529,10 +553,12 @@ where } *this.buf = input.into(); + *this.done = true; - // TODO: handle trailer + let mut trailers_map = HeaderMap::new(); + trailers_map.insert(trailer.header_name, trailer.header_value); - return Poll::Ready(None); + return Poll::Ready(Some(Ok(Frame::trailers(trailers_map)))); } } } diff --git a/src/api/s3/post_object.rs b/src/api/s3/post_object.rs index 6c1b7453..350684da 100644 --- a/src/api/s3/post_object.rs +++ b/src/api/s3/post_object.rs @@ -218,6 +218,7 @@ pub async fn handle_post_object( // around here to make sure the rest of the machinery takes our acl into account. let headers = get_headers(¶ms)?; + let checksum_algorithm = request_checksum_algorithm(¶ms)?; 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(¶ms)?, + extra: checksum_algorithm + .map(|algo| extract_checksum_value(¶ms, algo)) + .transpose()?, }; let meta = ObjectVersionMetaInner { diff --git a/src/garage/Cargo.toml b/src/garage/Cargo.toml index c036f000..5860cf97 100644 --- a/src/garage/Cargo.toml +++ b/src/garage/Cargo.toml @@ -75,6 +75,7 @@ static_init.workspace = true assert-json-diff.workspace = true serde_json.workspace = true base64.workspace = true +crc32fast.workspace = true k2v-client.workspace = true diff --git a/src/garage/tests/common/custom_requester.rs b/src/garage/tests/common/custom_requester.rs index 99fd4385..6a8eed38 100644 --- a/src/garage/tests/common/custom_requester.rs +++ b/src/garage/tests/common/custom_requester.rs @@ -195,10 +195,10 @@ impl<'a> RequestBuilder<'a> { all_headers.insert(signature::X_AMZ_DATE, HeaderValue::from_str(&date).unwrap()); all_headers.insert(HOST, HeaderValue::from_str(&host).unwrap()); - let body_sha = match self.body_signature { + let body_sha = match &self.body_signature { BodySignature::Unsigned => "UNSIGNED-PAYLOAD".to_owned(), BodySignature::Classic => hex::encode(garage_util::data::sha256sum(&self.body)), - BodySignature::Streaming(size) => { + BodySignature::Streaming { chunk_size } => { all_headers.insert( CONTENT_ENCODING, HeaderValue::from_str("aws-chunked").unwrap(), @@ -213,15 +213,56 @@ impl<'a> RequestBuilder<'a> { // code. all_headers.insert( CONTENT_LENGTH, - to_streaming_body(&self.body, size, String::new(), signer.clone(), now, "") - .len() - .to_string() - .try_into() - .unwrap(), + to_streaming_body( + &self.body, + *chunk_size, + String::new(), + signer.clone(), + now, + "", + ) + .len() + .to_string() + .try_into() + .unwrap(), ); "STREAMING-AWS4-HMAC-SHA256-PAYLOAD".to_owned() } + BodySignature::StreamingUnsignedTrailer { + chunk_size, + trailer_algorithm, + trailer_value, + } => { + all_headers.insert( + CONTENT_ENCODING, + HeaderValue::from_str("aws-chunked").unwrap(), + ); + all_headers.insert( + HeaderName::from_static("x-amz-decoded-content-length"), + HeaderValue::from_str(&self.body.len().to_string()).unwrap(), + ); + all_headers.insert( + HeaderName::from_static("x-amz-trailer"), + HeaderValue::from_str(&trailer_algorithm).unwrap(), + ); + + all_headers.insert( + CONTENT_LENGTH, + to_streaming_unsigned_trailer_body( + &self.body, + *chunk_size, + &trailer_algorithm, + &trailer_value, + ) + .len() + .to_string() + .try_into() + .unwrap(), + ); + + "STREAMING-UNSIGNED-PAYLOAD-TRAILER".to_owned() + } }; all_headers.insert( signature::X_AMZ_CONTENT_SHA256, @@ -273,10 +314,26 @@ impl<'a> RequestBuilder<'a> { let mut request = Request::builder(); *request.headers_mut().unwrap() = all_headers; - let body = if let BodySignature::Streaming(size) = self.body_signature { - to_streaming_body(&self.body, size, signature, streaming_signer, now, &scope) - } else { - self.body.clone() + let body = match &self.body_signature { + BodySignature::Streaming { chunk_size } => to_streaming_body( + &self.body, + *chunk_size, + signature, + streaming_signer, + now, + &scope, + ), + BodySignature::StreamingUnsignedTrailer { + chunk_size, + trailer_algorithm, + trailer_value, + } => to_streaming_unsigned_trailer_body( + &self.body, + *chunk_size, + &trailer_algorithm, + &trailer_value, + ), + _ => self.body.clone(), }; let request = request .uri(uri) @@ -305,7 +362,14 @@ impl<'a> RequestBuilder<'a> { pub enum BodySignature { Unsigned, Classic, - Streaming(usize), + Streaming { + chunk_size: usize, + }, + StreamingUnsignedTrailer { + chunk_size: usize, + trailer_algorithm: String, + trailer_value: String, + }, } fn query_param_to_string(params: &HashMap>) -> String { @@ -360,3 +424,26 @@ fn to_streaming_body( res } + +fn to_streaming_unsigned_trailer_body( + body: &[u8], + chunk_size: usize, + trailer_algorithm: &str, + trailer_value: &str, +) -> Vec { + let mut res = Vec::with_capacity(body.len()); + for chunk in body.chunks(chunk_size) { + let header = format!("{:x}\r\n", chunk.len()); + res.extend_from_slice(header.as_bytes()); + res.extend_from_slice(chunk); + res.extend_from_slice(b"\r\n"); + } + + res.extend_from_slice(b"0\r\n"); + res.extend_from_slice(trailer_algorithm.as_bytes()); + res.extend_from_slice(b":"); + res.extend_from_slice(trailer_value.as_bytes()); + res.extend_from_slice(b"\n\r\n\r\n"); + + res +} diff --git a/src/garage/tests/common/garage.rs b/src/garage/tests/common/garage.rs index da6c624b..8d71504f 100644 --- a/src/garage/tests/common/garage.rs +++ b/src/garage/tests/common/garage.rs @@ -99,7 +99,10 @@ api_bind_addr = "127.0.0.1:{admin_port}" .arg("server") .stdout(stdout) .stderr(stderr) - .env("RUST_LOG", "garage=debug,garage_api=trace") + .env( + "RUST_LOG", + "garage=debug,garage_api_common=trace,garage_api_s3=trace", + ) .spawn() .expect("Could not start garage"); diff --git a/src/garage/tests/s3/streaming_signature.rs b/src/garage/tests/s3/streaming_signature.rs index 351aa422..e83d2355 100644 --- a/src/garage/tests/s3/streaming_signature.rs +++ b/src/garage/tests/s3/streaming_signature.rs @@ -1,5 +1,8 @@ use std::collections::HashMap; +use base64::prelude::*; +use crc32fast::Hasher as Crc32; + use crate::common; use crate::common::ext::CommandExt; use common::custom_requester::BodySignature; @@ -21,7 +24,7 @@ async fn test_putobject_streaming() { let content_type = "text/csv"; let mut headers = HashMap::new(); headers.insert("content-type".to_owned(), content_type.to_owned()); - let _ = ctx + let res = ctx .custom_request .builder(bucket.clone()) .method(Method::PUT) @@ -29,10 +32,11 @@ async fn test_putobject_streaming() { .signed_headers(headers) .vhost_style(true) .body(vec![]) - .body_signature(BodySignature::Streaming(10)) + .body_signature(BodySignature::Streaming { chunk_size: 10 }) .send() .await .unwrap(); + assert!(res.status().is_success(), "got response: {:?}", res); // assert_eq!(r.e_tag.unwrap().as_str(), etag); // We return a version ID here @@ -65,7 +69,7 @@ async fn test_putobject_streaming() { { let etag = "\"46cf18a9b447991b450cad3facf5937e\""; - let _ = ctx + let res = ctx .custom_request .builder(bucket.clone()) .method(Method::PUT) @@ -74,10 +78,144 @@ async fn test_putobject_streaming() { .path("abc".to_owned()) .vhost_style(true) .body(BODY.to_vec()) - .body_signature(BodySignature::Streaming(16)) + .body_signature(BodySignature::Streaming { chunk_size: 16 }) .send() .await .unwrap(); + assert!(res.status().is_success(), "got response: {:?}", res); + + // assert_eq!(r.e_tag.unwrap().as_str(), etag); + // assert!(r.version_id.is_some()); + + let o = ctx + .client + .get_object() + .bucket(&bucket) + //.key(CTRL_KEY) + .key("abc") + .send() + .await + .unwrap(); + + assert_bytes_eq!(o.body, BODY); + assert_eq!(o.e_tag.unwrap(), etag); + assert!(o.last_modified.is_some()); + assert_eq!(o.content_length.unwrap(), 62); + assert_eq!(o.parts_count, None); + assert_eq!(o.tag_count, None); + } +} + +#[tokio::test] +async fn test_putobject_streaming_unsigned_trailer() { + let ctx = common::context(); + let bucket = ctx.create_bucket("putobject-streaming-unsigned-trailer"); + + { + // Send an empty object (can serve as a directory marker) + // with a content type + let etag = "\"d41d8cd98f00b204e9800998ecf8427e\""; + let content_type = "text/csv"; + let mut headers = HashMap::new(); + headers.insert("content-type".to_owned(), content_type.to_owned()); + + let empty_crc32 = BASE64_STANDARD.encode(&u32::to_be_bytes(Crc32::new().finalize())[..]); + + let res = ctx + .custom_request + .builder(bucket.clone()) + .method(Method::PUT) + .path(STD_KEY.to_owned()) + .signed_headers(headers) + .vhost_style(true) + .body(vec![]) + .body_signature(BodySignature::StreamingUnsignedTrailer { + chunk_size: 10, + trailer_algorithm: "x-amz-checksum-crc32".into(), + trailer_value: empty_crc32, + }) + .send() + .await + .unwrap(); + assert!(res.status().is_success(), "got response: {:?}", res); + + // assert_eq!(r.e_tag.unwrap().as_str(), etag); + // We return a version ID here + // We should check if Amazon is returning one when versioning is not enabled + // assert!(r.version_id.is_some()); + + //let _version = r.version_id.unwrap(); + + let o = ctx + .client + .get_object() + .bucket(&bucket) + .key(STD_KEY) + .send() + .await + .unwrap(); + + assert_bytes_eq!(o.body, b""); + assert_eq!(o.e_tag.unwrap(), etag); + // We do not return version ID + // We should check if Amazon is returning one when versioning is not enabled + // assert_eq!(o.version_id.unwrap(), _version); + assert_eq!(o.content_type.unwrap(), content_type); + assert!(o.last_modified.is_some()); + assert_eq!(o.content_length.unwrap(), 0); + assert_eq!(o.parts_count, None); + assert_eq!(o.tag_count, None); + } + + { + let etag = "\"46cf18a9b447991b450cad3facf5937e\""; + + let mut crc32 = Crc32::new(); + crc32.update(&BODY[..]); + let crc32 = BASE64_STANDARD.encode(&u32::to_be_bytes(crc32.finalize())[..]); + + // try sending with wrong crc32, check that it fails + let err_res = ctx + .custom_request + .builder(bucket.clone()) + .method(Method::PUT) + //.path(CTRL_KEY.to_owned()) at the moment custom_request does not encode url so this + //fail + .path("abc".to_owned()) + .vhost_style(true) + .body(BODY.to_vec()) + .body_signature(BodySignature::StreamingUnsignedTrailer { + chunk_size: 16, + trailer_algorithm: "x-amz-checksum-crc32".into(), + trailer_value: "2Yp9Yw==".into(), + }) + .send() + .await + .unwrap(); + assert!( + err_res.status().is_client_error(), + "got response: {:?}", + err_res + ); + + let res = ctx + .custom_request + .builder(bucket.clone()) + .method(Method::PUT) + //.path(CTRL_KEY.to_owned()) at the moment custom_request does not encode url so this + //fail + .path("abc".to_owned()) + .vhost_style(true) + .body(BODY.to_vec()) + .body_signature(BodySignature::StreamingUnsignedTrailer { + chunk_size: 16, + trailer_algorithm: "x-amz-checksum-crc32".into(), + trailer_value: crc32, + }) + .send() + .await + .unwrap(); + assert!(res.status().is_success(), "got response: {:?}", res); // assert_eq!(r.e_tag.unwrap().as_str(), etag); // assert!(r.version_id.is_some()); @@ -119,7 +257,7 @@ async fn test_create_bucket_streaming() { .custom_request .builder(bucket.to_owned()) .method(Method::PUT) - .body_signature(BodySignature::Streaming(10)) + .body_signature(BodySignature::Streaming { chunk_size: 10 }) .send() .await .unwrap(); @@ -174,7 +312,7 @@ async fn test_put_website_streaming() { .method(Method::PUT) .query_params(query) .body(website_config.as_bytes().to_vec()) - .body_signature(BodySignature::Streaming(10)) + .body_signature(BodySignature::Streaming { chunk_size: 10 }) .send() .await .unwrap(); From 45e10e55f9761d79aa8d5b7dbaeb56d4d535629e Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 18 Feb 2025 12:56:09 +0100 Subject: [PATCH 29/43] update aws-sdk-s3 in tests and fix wrong checksumming behavior in GetObject --- Cargo.lock | 97 +++++++++++++++++++++---------- Cargo.toml | 4 +- src/api/s3/get.rs | 14 ++++- src/garage/Cargo.toml | 1 + src/garage/tests/common/client.rs | 2 +- src/garage/tests/s3/objects.rs | 23 ++++++-- 6 files changed, 98 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 477e4456..558dde1c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -238,9 +238,9 @@ checksum = "ace50bade8e6234aa140d9a2f552bbee1db4d353f69b8217bc503490fc1a9f26" [[package]] name = "aws-credential-types" -version = "1.1.4" +version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "33cc49dcdd31c8b6e79850a179af4c367669150c7ac0135f176c61bec81a70f7" +checksum = "60e8f6b615cb5fc60a98132268508ad104310f0cfb25a1c22eee76efdf9154da" dependencies = [ "aws-smithy-async", "aws-smithy-runtime-api", @@ -250,15 +250,16 @@ dependencies = [ [[package]] name = "aws-runtime" -version = "1.1.4" +version = "1.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eb031bff99877c26c28895766f7bb8484a05e24547e370768d6cc9db514662aa" +checksum = "76dd04d39cc12844c0994f2c9c5a6f5184c22e9188ec1ff723de41910a21dcad" dependencies = [ "aws-credential-types", "aws-sigv4", "aws-smithy-async", "aws-smithy-eventstream", "aws-smithy-http", + "aws-smithy-runtime", "aws-smithy-runtime-api", "aws-smithy-types", "aws-types", @@ -266,6 +267,7 @@ dependencies = [ "fastrand", "http 0.2.12", "http-body 0.4.6", + "once_cell", "percent-encoding", "pin-project-lite", "tracing", @@ -274,9 +276,9 @@ dependencies = [ [[package]] name = "aws-sdk-config" -version = "1.13.0" +version = "1.62.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4af4f5b0f64563ada272e009cc95027effb546110ed85d014611420ac0d97858" +checksum = "0f94d79b8eef608af51b5415d13f5c670dec177880c6f78cd27bea968e5c9b76" dependencies = [ "aws-credential-types", "aws-runtime", @@ -296,9 +298,9 @@ dependencies = [ [[package]] name = "aws-sdk-s3" -version = "1.14.0" +version = "1.68.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "951f7730f51a2155c711c85c79f337fbc02a577fa99d2a0a8059acfce5392113" +checksum = "bc5ddf1dc70287dc9a2f953766a1fe15e3e74aef02fd1335f2afa475c9b4f4fc" dependencies = [ "aws-credential-types", "aws-runtime", @@ -314,20 +316,25 @@ dependencies = [ "aws-smithy-xml", "aws-types", "bytes", + "fastrand", + "hex", + "hmac", "http 0.2.12", "http-body 0.4.6", + "lru", "once_cell", "percent-encoding", "regex-lite", + "sha2", "tracing", "url", ] [[package]] name = "aws-sigv4" -version = "1.1.4" +version = "1.2.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c371c6b0ac54d4605eb6f016624fb5c7c2925d315fdf600ac1bf21b19d5f1742" +checksum = "9bfe75fad52793ce6dec0dc3d4b1f388f038b5eb866c8d4d7f3a8e21b5ea5051" dependencies = [ "aws-credential-types", "aws-smithy-eventstream", @@ -354,9 +361,9 @@ dependencies = [ [[package]] name = "aws-smithy-async" -version = "1.1.4" +version = "1.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72ee2d09cce0ef3ae526679b522835d63e75fb427aca5413cd371e490d52dcc6" +checksum = "fa59d1327d8b5053c54bf2eaae63bf629ba9e904434d0835a28ed3c0ed0a614e" dependencies = [ "futures-util", "pin-project-lite", @@ -365,9 +372,9 @@ dependencies = [ [[package]] name = "aws-smithy-checksums" -version = "0.60.4" +version = "0.60.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be2acd1b9c6ae5859999250ed5a62423aedc5cf69045b844432de15fa2f31f2b" +checksum = "ba1a71073fca26775c8b5189175ea8863afb1c9ea2cceb02a5de5ad9dfbaa795" dependencies = [ "aws-smithy-http", "aws-smithy-types", @@ -386,9 +393,9 @@ dependencies = [ [[package]] name = "aws-smithy-eventstream" -version = "0.60.4" +version = "0.60.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6363078f927f612b970edf9d1903ef5cef9a64d1e8423525ebb1f0a1633c858" +checksum = "8b18559a41e0c909b77625adf2b8c50de480a8041e5e4a3f5f7d177db70abc5a" dependencies = [ "aws-smithy-types", "bytes", @@ -397,9 +404,9 @@ dependencies = [ [[package]] name = "aws-smithy-http" -version = "0.60.4" +version = "0.60.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dab56aea3cd9e1101a0a999447fb346afb680ab1406cebc44b32346e25b4117d" +checksum = "7809c27ad8da6a6a68c454e651d4962479e81472aa19ae99e59f9aba1f9713cc" dependencies = [ "aws-smithy-eventstream", "aws-smithy-runtime-api", @@ -418,18 +425,18 @@ dependencies = [ [[package]] name = "aws-smithy-json" -version = "0.60.4" +version = "0.61.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd3898ca6518f9215f62678870064398f00031912390efd03f1f6ef56d83aa8e" +checksum = "623a51127f24c30776c8b374295f2df78d92517386f77ba30773f15a30ce1422" dependencies = [ "aws-smithy-types", ] [[package]] name = "aws-smithy-runtime" -version = "1.1.4" +version = "1.7.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fafdab38f40ad7816e7da5dec279400dd505160780083759f01441af1bbb10ea" +checksum = "d526a12d9ed61fadefda24abe2e682892ba288c2018bcb38b1b4c111d13f6d92" dependencies = [ "aws-smithy-async", "aws-smithy-http", @@ -440,6 +447,8 @@ dependencies = [ "h2 0.3.24", "http 0.2.12", "http-body 0.4.6", + "http-body 1.0.1", + "httparse", "hyper 0.14.32", "hyper-rustls 0.24.2", "once_cell", @@ -452,31 +461,36 @@ dependencies = [ [[package]] name = "aws-smithy-runtime-api" -version = "1.1.4" +version = "1.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c18276dd28852f34b3bf501f4f3719781f4999a51c7bff1a5c6dc8c4529adc29" +checksum = "92165296a47a812b267b4f41032ff8069ab7ff783696d217f0994a0d7ab585cd" dependencies = [ "aws-smithy-async", "aws-smithy-types", "bytes", "http 0.2.12", + "http 1.2.0", "pin-project-lite", "tokio", "tracing", + "zeroize", ] [[package]] name = "aws-smithy-types" -version = "1.1.4" +version = "1.2.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb3e134004170d3303718baa2a4eb4ca64ee0a1c0a7041dca31b38be0fb414f3" +checksum = "c7b8a53819e42f10d0821f56da995e1470b199686a1809168db6ca485665f042" dependencies = [ "base64-simd", "bytes", "bytes-utils", "futures-core", "http 0.2.12", + "http 1.2.0", "http-body 0.4.6", + "http-body 1.0.1", + "http-body-util", "itoa", "num-integer", "pin-project-lite", @@ -490,24 +504,23 @@ dependencies = [ [[package]] name = "aws-smithy-xml" -version = "0.60.4" +version = "0.60.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8604a11b25e9ecaf32f9aa56b9fe253c5e2f606a3477f0071e96d3155a5ed218" +checksum = "ab0b0166827aa700d3dc519f72f8b3a91c35d0b8d042dc5d643a91e6f80648fc" dependencies = [ "xmlparser", ] [[package]] name = "aws-types" -version = "1.1.4" +version = "1.3.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "789bbe008e65636fe1b6dbbb374c40c8960d1232b96af5ff4aec349f9c4accf4" +checksum = "dfbd0a668309ec1f66c0f6bda4840dd6d4796ae26d699ebc266d7cc95c6d040f" dependencies = [ "aws-credential-types", "aws-smithy-async", "aws-smithy-runtime-api", "aws-smithy-types", - "http 0.2.12", "rustc_version", "tracing", ] @@ -1116,6 +1129,12 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foldhash" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a0d2fde1f7b3d48b8395d5f2de76c18a528bd6a9cdde438df747bfcba3e05d6f" + [[package]] name = "form_urlencoded" version = "1.2.1" @@ -1744,6 +1763,11 @@ name = "hashbrown" version = "0.15.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf151400ff0baff5465007dd2f3e717f3fe502074ca563069ce3a6629d07b289" +dependencies = [ + "allocator-api2", + "equivalent", + "foldhash", +] [[package]] name = "hashlink" @@ -2600,6 +2624,15 @@ version = "0.4.25" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04cbf5b083de1c7e0222a7a51dbfdba1cbe1c6ab0b15e29fff3f6c077fd9cd9f" +[[package]] +name = "lru" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "234cf4f4a04dc1f57e24b96cc0cd600cf2af460d4161ac5ecdd0af8e1f3b2a38" +dependencies = [ + "hashbrown 0.15.2", +] + [[package]] name = "matchers" version = "0.1.0" @@ -4915,7 +4948,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.52.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index fa8f0be0..0e156358 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -141,8 +141,8 @@ thiserror = "1.0" assert-json-diff = "2.0" rustc_version = "0.4.0" static_init = "1.0" -aws-sdk-config = "1.13" -aws-sdk-s3 = "1.14" +aws-sdk-config = "1.62" +aws-sdk-s3 = "=1.68" [profile.dev] #lto = "thin" # disabled for now, adds 2-4 min to each CI build diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs index 15929cd1..bcb72cc3 100644 --- a/src/api/s3/get.rs +++ b/src/api/s3/get.rs @@ -340,7 +340,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 +359,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 } diff --git a/src/garage/Cargo.toml b/src/garage/Cargo.toml index 5860cf97..966c8ac5 100644 --- a/src/garage/Cargo.toml +++ b/src/garage/Cargo.toml @@ -71,6 +71,7 @@ hyper-util.workspace = true mktemp.workspace = true sha2.workspace = true + static_init.workspace = true assert-json-diff.workspace = true serde_json.workspace = true diff --git a/src/garage/tests/common/client.rs b/src/garage/tests/common/client.rs index ffa4cae8..7a6612cb 100644 --- a/src/garage/tests/common/client.rs +++ b/src/garage/tests/common/client.rs @@ -12,7 +12,7 @@ pub fn build_client(key: &Key) -> Client { .endpoint_url(format!("http://127.0.0.1:{}", DEFAULT_PORT)) .region(super::REGION) .credentials_provider(credentials) - .behavior_version(BehaviorVersion::v2023_11_09()) + .behavior_version(BehaviorVersion::v2024_03_28()) .build(); Client::from_conf(config) diff --git a/src/garage/tests/s3/objects.rs b/src/garage/tests/s3/objects.rs index 77eca2b1..dfc5253d 100644 --- a/src/garage/tests/s3/objects.rs +++ b/src/garage/tests/s3/objects.rs @@ -189,12 +189,14 @@ async fn test_getobject() { #[tokio::test] async fn test_metadata() { + use aws_sdk_s3::primitives::{DateTime, DateTimeFormat}; + let ctx = common::context(); let bucket = ctx.create_bucket("testmetadata"); let etag = "\"46cf18a9b447991b450cad3facf5937e\""; - let exp = aws_sdk_s3::primitives::DateTime::from_secs(10000000000); - let exp2 = aws_sdk_s3::primitives::DateTime::from_secs(10000500000); + let exp = DateTime::from_secs(10000000000); + let exp2 = DateTime::from_secs(10000500000); { // Note. The AWS client SDK adds a Content-Type header @@ -227,7 +229,7 @@ async fn test_metadata() { assert_eq!(o.content_disposition, None); assert_eq!(o.content_encoding, None); assert_eq!(o.content_language, None); - assert_eq!(o.expires, None); + assert_eq!(o.expires_string, None); assert_eq!(o.metadata.unwrap_or_default().len(), 0); let o = ctx @@ -250,7 +252,10 @@ async fn test_metadata() { assert_eq!(o.content_disposition.unwrap().as_str(), "cddummy"); assert_eq!(o.content_encoding.unwrap().as_str(), "cedummy"); assert_eq!(o.content_language.unwrap().as_str(), "cldummy"); - assert_eq!(o.expires.unwrap(), exp); + assert_eq!( + o.expires_string.unwrap(), + exp.fmt(DateTimeFormat::HttpDate).unwrap() + ); } { @@ -288,7 +293,10 @@ async fn test_metadata() { assert_eq!(o.content_disposition.unwrap().as_str(), "cdtest"); assert_eq!(o.content_encoding.unwrap().as_str(), "cetest"); assert_eq!(o.content_language.unwrap().as_str(), "cltest"); - assert_eq!(o.expires.unwrap(), exp2); + assert_eq!( + o.expires_string.unwrap(), + exp2.fmt(DateTimeFormat::HttpDate).unwrap() + ); let mut meta = o.metadata.unwrap(); assert_eq!(meta.remove("testmeta").unwrap(), "hello people"); assert_eq!(meta.remove("nice-unicode-meta").unwrap(), "宅配便"); @@ -314,7 +322,10 @@ async fn test_metadata() { assert_eq!(o.content_disposition.unwrap().as_str(), "cddummy"); assert_eq!(o.content_encoding.unwrap().as_str(), "cedummy"); assert_eq!(o.content_language.unwrap().as_str(), "cldummy"); - assert_eq!(o.expires.unwrap(), exp); + assert_eq!( + o.expires_string.unwrap(), + exp.fmt(DateTimeFormat::HttpDate).unwrap() + ); } } From f6e805e7db7b35e9fc6baaad6ec953e30a443437 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 18 Feb 2025 18:57:50 +0100 Subject: [PATCH 30/43] api: various fixes --- src/api/common/signature/body.rs | 2 +- src/api/common/signature/mod.rs | 1 - src/api/common/signature/payload.rs | 25 ++++++++++--------------- src/api/common/signature/streaming.rs | 24 ++++++++++++------------ src/api/s3/multipart.rs | 9 ++++++--- src/api/s3/put.rs | 5 +++++ 6 files changed, 34 insertions(+), 32 deletions(-) diff --git a/src/api/common/signature/body.rs b/src/api/common/signature/body.rs index 4279d7b5..96be0d5b 100644 --- a/src/api/common/signature/body.rs +++ b/src/api/common/signature/body.rs @@ -78,7 +78,7 @@ impl ReqBody { trailer_algorithm, } = self; - let (frame_tx, mut frame_rx) = mpsc::channel::>(1); + let (frame_tx, mut frame_rx) = mpsc::channel::>(5); let join_checksums = tokio::spawn(async move { while let Some(frame) = frame_rx.recv().await { diff --git a/src/api/common/signature/mod.rs b/src/api/common/signature/mod.rs index 78518436..50fbd304 100644 --- a/src/api/common/signature/mod.rs +++ b/src/api/common/signature/mod.rs @@ -62,7 +62,6 @@ pub struct VerifiedRequest { pub request: Request, pub access_key: Key, pub content_sha256_header: ContentSha256Header, - // TODO: oneshot chans to retrieve hashes after reading all body } pub async fn verify_request( diff --git a/src/api/common/signature/payload.rs b/src/api/common/signature/payload.rs index 4ca0153f..2d5f8603 100644 --- a/src/api/common/signature/payload.rs +++ b/src/api/common/signature/payload.rs @@ -74,21 +74,16 @@ fn parse_x_amz_content_sha256(header: Option<&str>) -> Result true, + UNSIGNED_PAYLOAD => false, + _ => { + return Err(Error::bad_request( + "invalid or unsupported x-amz-content-sha256", + )) + } + }; + Ok(ContentSha256Header::StreamingPayload { trailer, signed }) } else { let sha256 = hex::decode(header) .ok() diff --git a/src/api/common/signature/streaming.rs b/src/api/common/signature/streaming.rs index 70b6e004..64362727 100644 --- a/src/api/common/signature/streaming.rs +++ b/src/api/common/signature/streaming.rs @@ -30,22 +30,12 @@ pub fn parse_streaming_body( checked_signature.content_sha256_header ); - let expected_checksums = ExpectedChecksums { - sha256: match &checked_signature.content_sha256_header { - ContentSha256Header::Sha256Checksum(sha256) => Some(*sha256), - _ => None, - }, - ..Default::default() - }; - - let mut checksummer = Checksummer::init(&expected_checksums, false); - match checked_signature.content_sha256_header { ContentSha256Header::StreamingPayload { signed, trailer } => { // Sanity checks if !signed && !trailer { return Err(Error::bad_request( - "STREAMING-UNSIGNED-PAYLOAD is not a valid combination", + "STREAMING-UNSIGNED-PAYLOAD without trailer is not a valid combination", )); } @@ -64,6 +54,7 @@ pub fn parse_streaming_body( } // If trailer header is announced, add the calculation of the requested checksum + let mut checksummer = Checksummer::init(&Default::default(), false); let trailer_algorithm = if trailer { let algo = Some( request_trailer_checksum_algorithm(req.headers())? @@ -128,12 +119,21 @@ pub fn parse_streaming_body( ReqBody { stream: Mutex::new(signed_payload_stream.boxed()), checksummer, - expected_checksums, + expected_checksums: Default::default(), trailer_algorithm, } })) } _ => Ok(req.map(|body| { + let expected_checksums = ExpectedChecksums { + sha256: match &checked_signature.content_sha256_header { + ContentSha256Header::Sha256Checksum(sha256) => Some(*sha256), + _ => None, + }, + ..Default::default() + }; + let checksummer = Checksummer::init(&expected_checksums, false); + let stream = http_body_util::BodyStream::new(body).map_err(Error::from); ReqBody { stream: Mutex::new(stream.boxed()), diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index 53eff6ad..1ee04bc1 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -114,14 +114,17 @@ pub async fn handle_put_part( 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, 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(); } @@ -130,6 +133,7 @@ pub async fn handle_put_part( 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(),)?; @@ -186,7 +190,6 @@ pub async fn handle_put_part( garage.version_table.insert(&version).await?; // Copy data to version - // TODO don't duplicate checksums let (total_size, _, _) = read_and_put_blocks( &ctx, &version, @@ -198,7 +201,7 @@ pub async fn handle_put_part( ) .await?; - // Verify that checksums map + // Verify that checksums match let checksums = stream_checksums .await .ok_or_internal_error("checksum calculation")??; diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 6fcf33cb..496d80f3 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -79,9 +79,14 @@ 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())?; + // 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(); } From cfe8e8d45cf54a28911525d1cc7bb01c8ecb6bea Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 18 Feb 2025 21:55:48 +0100 Subject: [PATCH 31/43] api: PutObject: save trailer checksum in metadata --- src/api/s3/put.rs | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 496d80f3..4d866a06 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -49,7 +49,10 @@ pub(crate) struct SaveStreamResult { pub(crate) enum ChecksumMode<'a> { Verify(&'a ExpectedChecksums), - VerifyFrom(StreamingChecksumReceiver), + VerifyFrom { + checksummer: StreamingChecksumReceiver, + trailer_algo: Option, + }, Calculate(Option), } @@ -70,6 +73,7 @@ pub async fn handle_put( sha256: None, extra: request_checksum_value(req.headers())?, }; + let trailer_checksum_algorithm = request_trailer_checksum_algorithm(req.headers())?; let meta = ObjectVersionMetaInner { headers, @@ -90,7 +94,7 @@ pub async fn handle_put( req_body.add_md5(); } - let (stream, checksums) = req_body.streaming_with_checksums(); + let (stream, checksummer) = req_body.streaming_with_checksums(); let stream = stream.map_err(Error::from); let res = save_stream( @@ -99,7 +103,10 @@ pub async fn handle_put( encryption, stream, key, - ChecksumMode::VerifyFrom(checksums), + ChecksumMode::VerifyFrom { + checksummer, + trailer_algo: trailer_checksum_algorithm, + }, ) .await?; @@ -140,7 +147,7 @@ pub(crate) async fn save_stream> + Unpin>( ChecksumMode::Calculate(algo) => { Checksummer::init(&Default::default(), !encryption.is_encrypted()).add(*algo) } - ChecksumMode::VerifyFrom(_) => { + 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() @@ -160,11 +167,17 @@ pub(crate) async fn save_stream> + Unpin>( ChecksumMode::Calculate(algo) => { meta.checksum = checksums.extract(algo); } - ChecksumMode::VerifyFrom(checksummer) => { + 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)); + } } }; @@ -256,10 +269,16 @@ pub(crate) async fn save_stream> + Unpin>( ChecksumMode::Calculate(algo) => { meta.checksum = checksums.extract(algo); } - ChecksumMode::VerifyFrom(checksummer) => { + 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)); + } } }; From 6d38907dac2872a43e5bbaa108c14e8877dd818e Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 18 Feb 2025 22:02:04 +0100 Subject: [PATCH 32/43] test: verify saved checksums in streaming putobject tests --- src/garage/tests/s3/streaming_signature.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/garage/tests/s3/streaming_signature.rs b/src/garage/tests/s3/streaming_signature.rs index e83d2355..a86feefc 100644 --- a/src/garage/tests/s3/streaming_signature.rs +++ b/src/garage/tests/s3/streaming_signature.rs @@ -69,6 +69,13 @@ async fn test_putobject_streaming() { { let etag = "\"46cf18a9b447991b450cad3facf5937e\""; + let mut crc32 = Crc32::new(); + crc32.update(&BODY[..]); + let crc32 = BASE64_STANDARD.encode(&u32::to_be_bytes(crc32.finalize())[..]); + + let mut headers = HashMap::new(); + headers.insert("x-amz-checksum-crc32".to_owned(), crc32.clone()); + let res = ctx .custom_request .builder(bucket.clone()) @@ -77,6 +84,7 @@ async fn test_putobject_streaming() { //fail .path("abc".to_owned()) .vhost_style(true) + .signed_headers(headers) .body(BODY.to_vec()) .body_signature(BodySignature::Streaming { chunk_size: 16 }) .send() @@ -93,6 +101,7 @@ async fn test_putobject_streaming() { .bucket(&bucket) //.key(CTRL_KEY) .key("abc") + .checksum_mode(aws_sdk_s3::types::ChecksumMode::Enabled) .send() .await .unwrap(); @@ -103,6 +112,7 @@ async fn test_putobject_streaming() { assert_eq!(o.content_length.unwrap(), 62); assert_eq!(o.parts_count, None); assert_eq!(o.tag_count, None); + assert_eq!(o.checksum_crc32.unwrap(), crc32); } } @@ -210,7 +220,7 @@ async fn test_putobject_streaming_unsigned_trailer() { .body_signature(BodySignature::StreamingUnsignedTrailer { chunk_size: 16, trailer_algorithm: "x-amz-checksum-crc32".into(), - trailer_value: crc32, + trailer_value: crc32.clone(), }) .send() .await @@ -226,6 +236,7 @@ async fn test_putobject_streaming_unsigned_trailer() { .bucket(&bucket) //.key(CTRL_KEY) .key("abc") + .checksum_mode(aws_sdk_s3::types::ChecksumMode::Enabled) .send() .await .unwrap(); @@ -236,6 +247,7 @@ async fn test_putobject_streaming_unsigned_trailer() { assert_eq!(o.content_length.unwrap(), 62); assert_eq!(o.parts_count, None); assert_eq!(o.tag_count, None); + assert_eq!(o.checksum_crc32.unwrap(), crc32); } } From bf27a3ec9844cf86f2a7ca67b94e7fb8db3873df Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 19 Feb 2025 17:04:10 +0100 Subject: [PATCH 33/43] web: implement x-amz-website-redirect-location --- src/api/s3/copy.rs | 4 ++-- src/api/s3/lib.rs | 2 +- src/api/s3/multipart.rs | 2 +- src/api/s3/post_object.rs | 4 ++-- src/api/s3/put.rs | 19 +++++++++++++++++-- src/api/s3/website.rs | 5 ++++- src/garage/tests/s3/website.rs | 28 ++++++++++++++++++++++++++++ src/web/web_server.rs | 13 +++++++++++-- 8 files changed, 66 insertions(+), 11 deletions(-) diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs index 9ae48807..ff8019e6 100644 --- a/src/api/s3/copy.rs +++ b/src/api/s3/copy.rs @@ -28,7 +28,7 @@ use crate::encryption::EncryptionParams; use crate::error::*; use crate::get::full_object_byte_stream; 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}; // -------- CopyObject --------- @@ -73,7 +73,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, }, diff --git a/src/api/s3/lib.rs b/src/api/s3/lib.rs index 4d1d3ef5..83f684f8 100644 --- a/src/api/s3/lib.rs +++ b/src/api/s3/lib.rs @@ -14,7 +14,7 @@ mod list; mod multipart; mod post_object; mod put; -mod website; +pub mod website; mod encryption; mod router; diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index 1ee04bc1..d6eb26cb 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -49,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, diff --git a/src/api/s3/post_object.rs b/src/api/s3/post_object.rs index 350684da..b9bccae6 100644 --- a/src/api/s3/post_object.rs +++ b/src/api/s3/post_object.rs @@ -24,7 +24,7 @@ use garage_api_common::signature::payload::{verify_v4, Authorization}; use crate::api_server::ResBody; 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,7 +216,7 @@ 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(¶ms)?; + let headers = extract_metadata_headers(¶ms)?; let checksum_algorithm = request_checksum_algorithm(¶ms)?; let expected_checksums = ExpectedChecksums { diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 4d866a06..830a7998 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -37,6 +37,7 @@ use garage_api_common::signature::checksum::*; use crate::api_server::{ReqBody, ResBody}; use crate::encryption::EncryptionParams; use crate::error::*; +use crate::website::X_AMZ_WEBSITE_REDIRECT_LOCATION; const PUT_BLOCKS_MAX_PARALLEL: usize = 3; @@ -62,7 +63,7 @@ pub async fn handle_put( key: &String, ) -> Result, 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 { @@ -649,7 +650,9 @@ impl Drop for InterruptedCleanup { // ============ helpers ============ -pub(crate) fn get_headers(headers: &HeaderMap) -> Result { +pub(crate) fn extract_metadata_headers( + headers: &HeaderMap, +) -> Result { let mut ret = Vec::new(); // Preserve standard headers @@ -675,6 +678,18 @@ pub(crate) fn get_headers(headers: &HeaderMap) -> Result Result, Error> { let ReqCtx { bucket_params, .. } = ctx; if let Some(website) = bucket_params.website_config.get() { diff --git a/src/garage/tests/s3/website.rs b/src/garage/tests/s3/website.rs index 0cadc388..9a9e29f2 100644 --- a/src/garage/tests/s3/website.rs +++ b/src/garage/tests/s3/website.rs @@ -11,6 +11,7 @@ use http::{Request, StatusCode}; use http_body_util::BodyExt; use http_body_util::Full as FullBody; use hyper::body::Bytes; +use hyper::header::LOCATION; use hyper_util::client::legacy::Client; use hyper_util::rt::TokioExecutor; use serde_json::json; @@ -295,6 +296,33 @@ async fn test_website_s3_api() { ); } + // Test x-amz-website-redirect-location + { + ctx.client + .put_object() + .bucket(&bucket) + .key("test-redirect.html") + .website_redirect_location("https://perdu.com") + .send() + .await + .unwrap(); + + let req = Request::builder() + .method("GET") + .uri(format!( + "http://127.0.0.1:{}/test-redirect.html", + ctx.garage.web_port + )) + .header("Host", format!("{}.web.garage", BCKT_NAME)) + .body(Body::new(Bytes::new())) + .unwrap(); + + let resp = client.request(req).await.unwrap(); + + assert_eq!(resp.status(), StatusCode::MOVED_PERMANENTLY); + assert_eq!(resp.headers().get(LOCATION).unwrap(), "https://perdu.com"); + } + // Test CORS with an allowed preflight request { let req = Request::builder() diff --git a/src/web/web_server.rs b/src/web/web_server.rs index 34ba834c..242f7801 100644 --- a/src/web/web_server.rs +++ b/src/web/web_server.rs @@ -7,7 +7,7 @@ use tokio::sync::watch; use hyper::{ body::Incoming as IncomingBody, - header::{HeaderValue, HOST}, + header::{HeaderValue, HOST, LOCATION}, Method, Request, Response, StatusCode, }; @@ -29,6 +29,7 @@ use garage_api_s3::error::{ CommonErrorDerivative, Error as ApiError, OkOrBadRequest, OkOrInternalError, }; use garage_api_s3::get::{handle_get_without_ctx, handle_head_without_ctx}; +use garage_api_s3::website::X_AMZ_WEBSITE_REDIRECT_LOCATION; use garage_model::garage::Garage; @@ -294,7 +295,15 @@ impl WebServer { { Ok(Response::builder() .status(StatusCode::FOUND) - .header("Location", url) + .header(LOCATION, url) + .body(empty_body()) + .unwrap()) + } + (Ok(ret), _) if ret.headers().contains_key(X_AMZ_WEBSITE_REDIRECT_LOCATION) => { + let redirect_location = ret.headers().get(X_AMZ_WEBSITE_REDIRECT_LOCATION).unwrap(); + Ok(Response::builder() + .status(StatusCode::MOVED_PERMANENTLY) + .header(LOCATION, redirect_location) .body(empty_body()) .unwrap()) } From 1a8f74fc94495e1a6b60138f634bb8018ed2078b Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 19 Feb 2025 17:26:29 +0100 Subject: [PATCH 34/43] api: GetObject: implement if-match and if-unmodified-since --- src/api/s3/get.rs | 57 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs index bcb72cc3..65be220f 100644 --- a/src/api/s3/get.rs +++ b/src/api/s3/get.rs @@ -9,8 +9,8 @@ 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, + CONTENT_LENGTH, CONTENT_RANGE, CONTENT_TYPE, ETAG, EXPIRES, IF_MATCH, IF_MODIFIED_SINCE, + IF_NONE_MATCH, IF_UNMODIFIED_SINCE, LAST_MODIFIED, RANGE, }; use hyper::{Request, Response, StatusCode}; use tokio::sync::mpsc; @@ -115,42 +115,67 @@ fn getobject_override_headers( Ok(()) } -fn try_answer_cached( +fn handle_http_precondition( version: &ObjectVersion, version_meta: &ObjectVersionMeta, req: &Request<()>, -) -> Option> { +) -> Result>, Error> { + if let Some(if_match) = req.headers().get(IF_MATCH) { + let if_match = if_match.to_str()?; + let expected = format!("\"{}\"", version_meta.etag); + let found = if_match + .split(',') + .map(str::trim) + .any(|etag| etag == expected || etag == "\"*\""); + if !found { + return Ok(Some( + Response::builder() + .status(StatusCode::PRECONDITION_FAILED) + .body(empty_body()) + .unwrap(), + )); + } + } + // 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 object_date = UNIX_EPOCH + Duration::from_millis(version.timestamp); + let cached = if let Some(none_match) = req.headers().get(IF_NONE_MATCH) { - let none_match = none_match.to_str().ok()?; + let none_match = none_match.to_str()?; let expected = format!("\"{}\"", version_meta.etag); let found = none_match .split(',') .map(str::trim) .any(|etag| etag == expected || etag == "\"*\""); found + } else if let Some(unmodified_since) = req.headers().get(IF_UNMODIFIED_SINCE) { + let unmodified_since = unmodified_since.to_str()?; + let unmodified_since = + httpdate::parse_http_date(unmodified_since).ok_or_bad_request("invalid http date")?; + object_date <= unmodified_since } 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 + let modified_since = modified_since.to_str()?; + let modified_since = + httpdate::parse_http_date(modified_since).ok_or_bad_request("invalid http date")?; + let object_date = UNIX_EPOCH + Duration::from_millis(version.timestamp); + object_date > modified_since } else { false }; if cached { - Some( + Ok(Some( Response::builder() .status(StatusCode::NOT_MODIFIED) .body(empty_body()) .unwrap(), - ) + )) } else { - None + Ok(None) } } @@ -196,8 +221,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) = @@ -318,8 +343,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) = From c0846c56fe360c859e573e645fead4a0978b9b9a Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 19 Feb 2025 17:54:58 +0100 Subject: [PATCH 35/43] api: unify http precondition handling --- src/api/s3/copy.rs | 112 ++++------------------------- src/api/s3/get.rs | 175 +++++++++++++++++++++++++++++++-------------- 2 files changed, 137 insertions(+), 150 deletions(-) diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs index ff8019e6..a5b2d706 100644 --- a/src/api/s3/copy.rs +++ b/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; @@ -26,11 +26,20 @@ use garage_api_common::signature::checksum::*; use crate::api_server::{ReqBody, ResBody}; 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::{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, dest_key: &str, ) -> Result, 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) = @@ -335,7 +344,7 @@ pub async fn handle_upload_part_copy( part_number: u64, upload_id: &str, ) -> Result, 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>, - copy_source_if_modified_since: Option, - copy_source_if_none_match: Option>, - copy_source_if_unmodified_since: Option, -} - -impl CopyPreconditionHeaders { - fn parse(req: &Request) -> Result { - 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::>() - }), - 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::>() - }), - 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); type BlockStreamItem = Result; diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs index 65be220f..22076603 100644 --- a/src/api/s3/get.rs +++ b/src/api/s3/get.rs @@ -2,15 +2,15 @@ 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_MATCH, IF_MODIFIED_SINCE, - IF_NONE_MATCH, IF_UNMODIFIED_SINCE, 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::{Request, Response, StatusCode}; use tokio::sync::mpsc; @@ -29,10 +29,11 @@ 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::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 { @@ -120,57 +121,12 @@ fn handle_http_precondition( version_meta: &ObjectVersionMeta, req: &Request<()>, ) -> Result>, Error> { - if let Some(if_match) = req.headers().get(IF_MATCH) { - let if_match = if_match.to_str()?; - let expected = format!("\"{}\"", version_meta.etag); - let found = if_match - .split(',') - .map(str::trim) - .any(|etag| etag == expected || etag == "\"*\""); - if !found { - return Ok(Some( - Response::builder() - .status(StatusCode::PRECONDITION_FAILED) - .body(empty_body()) - .unwrap(), - )); - } - } + let precondition_headers = PreconditionHeaders::parse(req)?; - // 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 object_date = UNIX_EPOCH + Duration::from_millis(version.timestamp); - - let cached = if let Some(none_match) = req.headers().get(IF_NONE_MATCH) { - let none_match = none_match.to_str()?; - let expected = format!("\"{}\"", version_meta.etag); - let found = none_match - .split(',') - .map(str::trim) - .any(|etag| etag == expected || etag == "\"*\""); - found - } else if let Some(unmodified_since) = req.headers().get(IF_UNMODIFIED_SINCE) { - let unmodified_since = unmodified_since.to_str()?; - let unmodified_since = - httpdate::parse_http_date(unmodified_since).ok_or_bad_request("invalid http date")?; - object_date <= unmodified_since - } else if let Some(modified_since) = req.headers().get(IF_MODIFIED_SINCE) { - let modified_since = modified_since.to_str()?; - let modified_since = - httpdate::parse_http_date(modified_since).ok_or_bad_request("invalid http date")?; - let object_date = UNIX_EPOCH + Duration::from_millis(version.timestamp); - object_date > modified_since - } else { - false - }; - - if cached { + 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(), )) @@ -786,3 +742,116 @@ fn std_error_from_read_error(e: E) -> std::io::Error { format!("Error while reading object data: {}", e), ) } + +// ---- + +pub struct PreconditionHeaders { + if_match: Option>, + if_modified_since: Option, + if_none_match: Option>, + if_unmodified_since: Option, +} + +impl PreconditionHeaders { + fn parse(req: &Request) -> Result { + Self::parse_with( + req.headers(), + &IF_MATCH, + &IF_NONE_MATCH, + &IF_MODIFIED_SINCE, + &IF_UNMODIFIED_SINCE, + ) + } + + pub(crate) fn parse_copy_source(req: &Request) -> Result { + 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 { + 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::>() + }), + 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::>() + }), + 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, 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(()), + } + } +} From f87943a39d81efcd250ff7ddfa3bf562a2e61ece Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 19 Feb 2025 18:26:03 +0100 Subject: [PATCH 36/43] tests: add test for http preconditions --- src/garage/tests/s3/objects.rs | 126 ++++++++++++++++++++++++++++++++- 1 file changed, 125 insertions(+), 1 deletion(-) diff --git a/src/garage/tests/s3/objects.rs b/src/garage/tests/s3/objects.rs index dfc5253d..d63ac000 100644 --- a/src/garage/tests/s3/objects.rs +++ b/src/garage/tests/s3/objects.rs @@ -1,5 +1,6 @@ use crate::common; -use aws_sdk_s3::primitives::ByteStream; +use aws_sdk_s3::error::SdkError; +use aws_sdk_s3::primitives::{ByteStream, DateTime}; use aws_sdk_s3::types::{Delete, ObjectIdentifier}; const STD_KEY: &str = "hello world"; @@ -125,6 +126,129 @@ async fn test_putobject() { } } +#[tokio::test] +async fn test_precondition() { + let ctx = common::context(); + let bucket = ctx.create_bucket("precondition"); + + let etag = "\"46cf18a9b447991b450cad3facf5937e\""; + let etag2 = "\"ae4984b984cd984fe98d4efa954dce98\""; + let data = ByteStream::from_static(BODY); + + let r = ctx + .client + .put_object() + .bucket(&bucket) + .key(STD_KEY) + .body(data) + .send() + .await + .unwrap(); + + assert_eq!(r.e_tag.unwrap().as_str(), etag); + + let last_modified; + { + let o = ctx + .client + .get_object() + .bucket(&bucket) + .key(STD_KEY) + .if_match(etag) + .send() + .await + .unwrap(); + assert_eq!(o.e_tag.as_ref().unwrap().as_str(), etag); + last_modified = o.last_modified.unwrap(); + + let err = ctx + .client + .get_object() + .bucket(&bucket) + .key(STD_KEY) + .if_match(etag2) + .send() + .await; + assert!( + matches!(err, Err(SdkError::ServiceError(se)) if se.raw().status().as_u16() == 412) + ); + } + { + let o = ctx + .client + .get_object() + .bucket(&bucket) + .key(STD_KEY) + .if_none_match(etag2) + .send() + .await + .unwrap(); + assert_eq!(o.e_tag.as_ref().unwrap().as_str(), etag); + + let err = ctx + .client + .get_object() + .bucket(&bucket) + .key(STD_KEY) + .if_none_match(etag) + .send() + .await; + assert!( + matches!(err, Err(SdkError::ServiceError(se)) if se.raw().status().as_u16() == 304) + ); + } + let older_date = DateTime::from_secs_f64(last_modified.as_secs_f64() - 10.0); + let newer_date = DateTime::from_secs_f64(last_modified.as_secs_f64() + 10.0); + { + let err = ctx + .client + .get_object() + .bucket(&bucket) + .key(STD_KEY) + .if_modified_since(newer_date) + .send() + .await; + assert!( + matches!(err, Err(SdkError::ServiceError(se)) if se.raw().status().as_u16() == 304) + ); + + let o = ctx + .client + .get_object() + .bucket(&bucket) + .key(STD_KEY) + .if_modified_since(older_date) + .send() + .await + .unwrap(); + assert_eq!(o.e_tag.as_ref().unwrap().as_str(), etag); + } + { + let err = ctx + .client + .get_object() + .bucket(&bucket) + .key(STD_KEY) + .if_unmodified_since(older_date) + .send() + .await; + assert!( + matches!(err, Err(SdkError::ServiceError(se)) if se.raw().status().as_u16() == 412) + ); + + let o = ctx + .client + .get_object() + .bucket(&bucket) + .key(STD_KEY) + .if_unmodified_since(newer_date) + .send() + .await + .unwrap(); + assert_eq!(o.e_tag.as_ref().unwrap().as_str(), etag); + } +} + #[tokio::test] async fn test_getobject() { let ctx = common::context(); From a0ea28b0da2d148e7190ade3c81fbc87aa33641d Mon Sep 17 00:00:00 2001 From: babykart Date: Sun, 23 Feb 2025 15:45:55 +0100 Subject: [PATCH 37/43] Add headless service for statefulSet serviceName Signed-off-by: babykart --- .../garage/templates/service-headless.yaml | 21 +++++++++++++++++++ script/helm/garage/templates/workload.yaml | 3 +-- 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 script/helm/garage/templates/service-headless.yaml diff --git a/script/helm/garage/templates/service-headless.yaml b/script/helm/garage/templates/service-headless.yaml new file mode 100644 index 00000000..43d8dfa2 --- /dev/null +++ b/script/helm/garage/templates/service-headless.yaml @@ -0,0 +1,21 @@ +{{- if eq .Values.deployment.kind "StatefulSet" -}} +apiVersion: v1 +kind: Service +metadata: + name: {{ include "garage.fullname" . }}-headless + labels: + {{- include "garage.labels" . | nindent 4 }} +spec: + type: {{ .Values.service.type }} + ports: + - port: {{ .Values.service.s3.api.port }} + targetPort: 3900 + protocol: TCP + name: s3-api + - port: {{ .Values.service.s3.web.port }} + targetPort: 3902 + protocol: TCP + name: s3-web + selector: + {{- include "garage.selectorLabels" . | nindent 4 }} +{{- end }} \ No newline at end of file diff --git a/script/helm/garage/templates/workload.yaml b/script/helm/garage/templates/workload.yaml index 251813e4..cb9e76a2 100644 --- a/script/helm/garage/templates/workload.yaml +++ b/script/helm/garage/templates/workload.yaml @@ -10,12 +10,11 @@ spec: {{- include "garage.selectorLabels" . | nindent 6 }} {{- if eq .Values.deployment.kind "StatefulSet" }} replicas: {{ .Values.deployment.replicaCount }} - serviceName: {{ include "garage.fullname" . }} + serviceName: {{ include "garage.fullname" . }}-headless podManagementPolicy: {{ .Values.deployment.podManagementPolicy }} {{- end }} template: metadata: - annotations: checksum/config: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }} {{- with .Values.podAnnotations }} From 09ed5ab8ccccc3cfe423aca93e334a4a6a8ae02a Mon Sep 17 00:00:00 2001 From: babykart Date: Sun, 23 Feb 2025 15:55:01 +0100 Subject: [PATCH 38/43] Fix documentation link Signed-off-by: babykart --- script/helm/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/helm/README.md b/script/helm/README.md index 5f919a23..1b22e604 100644 --- a/script/helm/README.md +++ b/script/helm/README.md @@ -1,3 +1,3 @@ # Garage helm3 chart -Documentation is located [here](/doc/book/cookbook/kubernetes.md). +Documentation is located [here](https://garagehq.deuxfleurs.fr/documentation/cookbook/kubernetes/). From 8647ebf0037dff7216c7f29b5645243bedcd7434 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 5 Mar 2025 10:15:28 +0100 Subject: [PATCH 39/43] admin api definition: fix globalAlias query parameter name (related: #971) --- doc/api/garage-admin-v1.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/garage-admin-v1.yml b/doc/api/garage-admin-v1.yml index 1ea77b2e..a70dc97b 100644 --- a/doc/api/garage-admin-v1.yml +++ b/doc/api/garage-admin-v1.yml @@ -687,7 +687,7 @@ paths: operationId: "GetBucketInfo" summary: "Get a bucket" description: | - Given a bucket identifier (`id`) or a global alias (`alias`), get its information. + Given a bucket identifier (`id`) or a global alias (`globalAlias`), get its information. It includes its aliases, its web configuration, keys that have some permissions on it, some statistics (number of objects, size), number of dangling multipart uploads, and its quotas (if any). @@ -701,7 +701,7 @@ paths: example: "b4018dc61b27ccb5c64ec1b24f53454bbbd180697c758c4d47a22a8921864a87" schema: type: string - - name: alias + - name: globalAlias in: query description: | The exact global alias of one of the existing buckets. From 4689b1044878dc08d81d0d961112febd8f85af20 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Thu, 20 Feb 2025 19:09:35 +0100 Subject: [PATCH 40/43] bump version to v1.1.0 --- Cargo.lock | 26 +++++++++++++------------- Cargo.toml | 24 ++++++++++++------------ doc/book/cookbook/real-world.md | 10 +++++----- doc/drafts/admin-api.md | 2 +- script/helm/garage/Chart.yaml | 4 ++-- src/api/admin/Cargo.toml | 2 +- src/api/common/Cargo.toml | 2 +- src/api/k2v/Cargo.toml | 2 +- src/api/s3/Cargo.toml | 2 +- src/block/Cargo.toml | 2 +- src/db/Cargo.toml | 2 +- src/garage/Cargo.toml | 2 +- src/model/Cargo.toml | 2 +- src/net/Cargo.toml | 2 +- src/rpc/Cargo.toml | 2 +- src/table/Cargo.toml | 2 +- src/util/Cargo.toml | 2 +- src/web/Cargo.toml | 2 +- 18 files changed, 46 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 558dde1c..b56b72bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1239,7 +1239,7 @@ dependencies = [ [[package]] name = "garage" -version = "1.0.1" +version = "1.1.0" dependencies = [ "assert-json-diff", "async-trait", @@ -1293,7 +1293,7 @@ dependencies = [ [[package]] name = "garage_api_admin" -version = "1.0.1" +version = "1.1.0" dependencies = [ "argon2", "async-trait", @@ -1319,7 +1319,7 @@ dependencies = [ [[package]] name = "garage_api_common" -version = "1.0.1" +version = "1.1.0" dependencies = [ "base64 0.21.7", "bytes", @@ -1354,7 +1354,7 @@ dependencies = [ [[package]] name = "garage_api_k2v" -version = "1.0.1" +version = "1.1.0" dependencies = [ "base64 0.21.7", "err-derive", @@ -1377,7 +1377,7 @@ dependencies = [ [[package]] name = "garage_api_s3" -version = "1.0.1" +version = "1.1.0" dependencies = [ "aes-gcm", "async-compression", @@ -1422,7 +1422,7 @@ dependencies = [ [[package]] name = "garage_block" -version = "1.0.1" +version = "1.1.0" dependencies = [ "arc-swap", "async-compression", @@ -1447,7 +1447,7 @@ dependencies = [ [[package]] name = "garage_db" -version = "1.0.1" +version = "1.1.0" dependencies = [ "err-derive", "heed", @@ -1460,7 +1460,7 @@ dependencies = [ [[package]] name = "garage_model" -version = "1.0.1" +version = "1.1.0" dependencies = [ "async-trait", "base64 0.21.7", @@ -1487,7 +1487,7 @@ dependencies = [ [[package]] name = "garage_net" -version = "1.0.1" +version = "1.1.0" dependencies = [ "arc-swap", "bytes", @@ -1512,7 +1512,7 @@ dependencies = [ [[package]] name = "garage_rpc" -version = "1.0.1" +version = "1.1.0" dependencies = [ "arc-swap", "async-trait", @@ -1544,7 +1544,7 @@ dependencies = [ [[package]] name = "garage_table" -version = "1.0.1" +version = "1.1.0" dependencies = [ "arc-swap", "async-trait", @@ -1565,7 +1565,7 @@ dependencies = [ [[package]] name = "garage_util" -version = "1.0.1" +version = "1.1.0" dependencies = [ "arc-swap", "async-trait", @@ -1597,7 +1597,7 @@ dependencies = [ [[package]] name = "garage_web" -version = "1.0.1" +version = "1.1.0" dependencies = [ "err-derive", "garage_api_common", diff --git a/Cargo.toml b/Cargo.toml index 0e156358..41f91228 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,18 +24,18 @@ default-members = ["src/garage"] # Internal Garage crates format_table = { version = "0.1.1", path = "src/format-table" } -garage_api_common = { version = "1.0.1", path = "src/api/common" } -garage_api_admin = { version = "1.0.1", path = "src/api/admin" } -garage_api_s3 = { version = "1.0.1", path = "src/api/s3" } -garage_api_k2v = { version = "1.0.1", path = "src/api/k2v" } -garage_block = { version = "1.0.1", path = "src/block" } -garage_db = { version = "1.0.1", path = "src/db", default-features = false } -garage_model = { version = "1.0.1", path = "src/model", default-features = false } -garage_net = { version = "1.0.1", path = "src/net" } -garage_rpc = { version = "1.0.1", path = "src/rpc" } -garage_table = { version = "1.0.1", path = "src/table" } -garage_util = { version = "1.0.1", path = "src/util" } -garage_web = { version = "1.0.1", path = "src/web" } +garage_api_common = { version = "1.1.0", path = "src/api/common" } +garage_api_admin = { version = "1.1.0", path = "src/api/admin" } +garage_api_s3 = { version = "1.1.0", path = "src/api/s3" } +garage_api_k2v = { version = "1.1.0", path = "src/api/k2v" } +garage_block = { version = "1.1.0", path = "src/block" } +garage_db = { version = "1.1.0", path = "src/db", default-features = false } +garage_model = { version = "1.1.0", path = "src/model", default-features = false } +garage_net = { version = "1.1.0", path = "src/net" } +garage_rpc = { version = "1.1.0", path = "src/rpc" } +garage_table = { version = "1.1.0", path = "src/table" } +garage_util = { version = "1.1.0", path = "src/util" } +garage_web = { version = "1.1.0", path = "src/web" } k2v-client = { version = "0.0.4", path = "src/k2v-client" } # External crates from crates.io diff --git a/doc/book/cookbook/real-world.md b/doc/book/cookbook/real-world.md index 751609db..594f1905 100644 --- a/doc/book/cookbook/real-world.md +++ b/doc/book/cookbook/real-world.md @@ -96,14 +96,14 @@ to store 2 TB of data in total. ## Get a Docker image Our docker image is currently named `dxflrs/garage` and is stored on the [Docker Hub](https://hub.docker.com/r/dxflrs/garage/tags?page=1&ordering=last_updated). -We encourage you to use a fixed tag (eg. `v1.0.1`) and not the `latest` tag. -For this example, we will use the latest published version at the time of the writing which is `v1.0.1` but it's up to you +We encourage you to use a fixed tag (eg. `v1.1.0`) and not the `latest` tag. +For this example, we will use the latest published version at the time of the writing which is `v1.1.0` but it's up to you to check [the most recent versions on the Docker Hub](https://hub.docker.com/r/dxflrs/garage/tags?page=1&ordering=last_updated). For example: ``` -sudo docker pull dxflrs/garage:v1.0.1 +sudo docker pull dxflrs/garage:v1.1.0 ``` ## Deploying and configuring Garage @@ -171,7 +171,7 @@ docker run \ -v /etc/garage.toml:/etc/garage.toml \ -v /var/lib/garage/meta:/var/lib/garage/meta \ -v /var/lib/garage/data:/var/lib/garage/data \ - dxflrs/garage:v1.0.1 + dxflrs/garage:v1.1.0 ``` With this command line, Garage should be started automatically at each boot. @@ -185,7 +185,7 @@ If you want to use `docker-compose`, you may use the following `docker-compose.y version: "3" services: garage: - image: dxflrs/garage:v1.0.1 + image: dxflrs/garage:v1.1.0 network_mode: "host" restart: unless-stopped volumes: diff --git a/doc/drafts/admin-api.md b/doc/drafts/admin-api.md index a614af58..acceefab 100644 --- a/doc/drafts/admin-api.md +++ b/doc/drafts/admin-api.md @@ -70,7 +70,7 @@ Example response body: ```json { "node": "b10c110e4e854e5aa3f4637681befac755154b20059ec163254ddbfae86b09df", - "garageVersion": "v1.0.1", + "garageVersion": "v1.1.0", "garageFeatures": [ "k2v", "lmdb", diff --git a/script/helm/garage/Chart.yaml b/script/helm/garage/Chart.yaml index fca569cc..1a3e27e0 100644 --- a/script/helm/garage/Chart.yaml +++ b/script/helm/garage/Chart.yaml @@ -15,10 +15,10 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.6.0 +version: 0.7.0 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. # It is recommended to use it with quotes. -appVersion: "v1.0.1" +appVersion: "v1.1.0" diff --git a/src/api/admin/Cargo.toml b/src/api/admin/Cargo.toml index adddf306..a61d13e5 100644 --- a/src/api/admin/Cargo.toml +++ b/src/api/admin/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_api_admin" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/api/common/Cargo.toml b/src/api/common/Cargo.toml index c33d585d..8fab93ee 100644 --- a/src/api/common/Cargo.toml +++ b/src/api/common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_api_common" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/api/k2v/Cargo.toml b/src/api/k2v/Cargo.toml index e3ebedca..c8c7c8ef 100644 --- a/src/api/k2v/Cargo.toml +++ b/src/api/k2v/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_api_k2v" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/api/s3/Cargo.toml b/src/api/s3/Cargo.toml index 387e45db..2507ee07 100644 --- a/src/api/s3/Cargo.toml +++ b/src/api/s3/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_api_s3" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/block/Cargo.toml b/src/block/Cargo.toml index 3358a3e7..1f5558c5 100644 --- a/src/block/Cargo.toml +++ b/src/block/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_block" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/db/Cargo.toml b/src/db/Cargo.toml index 3ef51fae..bfc9029c 100644 --- a/src/db/Cargo.toml +++ b/src/db/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_db" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/garage/Cargo.toml b/src/garage/Cargo.toml index 966c8ac5..53a5a447 100644 --- a/src/garage/Cargo.toml +++ b/src/garage/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/model/Cargo.toml b/src/model/Cargo.toml index b58ad43b..42ec8537 100644 --- a/src/model/Cargo.toml +++ b/src/model/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_model" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/net/Cargo.toml b/src/net/Cargo.toml index c0b47a6e..b48eb153 100644 --- a/src/net/Cargo.toml +++ b/src/net/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_net" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/rpc/Cargo.toml b/src/rpc/Cargo.toml index fcc1c304..e6466001 100644 --- a/src/rpc/Cargo.toml +++ b/src/rpc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_rpc" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/table/Cargo.toml b/src/table/Cargo.toml index fad6ea08..ef7b44e4 100644 --- a/src/table/Cargo.toml +++ b/src/table/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_table" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/util/Cargo.toml b/src/util/Cargo.toml index fec5b1ed..123406db 100644 --- a/src/util/Cargo.toml +++ b/src/util/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_util" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/web/Cargo.toml b/src/web/Cargo.toml index a0a3e566..c4fdbc0e 100644 --- a/src/web/Cargo.toml +++ b/src/web/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_web" -version = "1.0.1" +version = "1.1.0" authors = ["Alex Auvolat ", "Quentin Dufour "] edition = "2018" license = "AGPL-3.0" From 42c5d02cdfa1c4aabb741fd00046cbbeef612b23 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Thu, 20 Feb 2025 19:39:53 +0100 Subject: [PATCH 41/43] doc: fix "since vX.X.X" in multiple places --- doc/book/reference-manual/configuration.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/book/reference-manual/configuration.md b/doc/book/reference-manual/configuration.md index f545de29..e0fc17bc 100644 --- a/doc/book/reference-manual/configuration.md +++ b/doc/book/reference-manual/configuration.md @@ -154,7 +154,7 @@ The `[admin]` section: The following configuration parameter must be specified as an environment variable, it does not exist in the configuration file: -- `GARAGE_LOG_TO_SYSLOG` (since v0.9.4): set this to `1` or `true` to make the +- `GARAGE_LOG_TO_SYSLOG` (since `v0.9.4`): set this to `1` or `true` to make the Garage daemon send its logs to `syslog` (using the libc `syslog` function) instead of printing to stderr. @@ -300,7 +300,7 @@ data_dir = [ See [the dedicated documentation page](@/documentation/operations/multi-hdd.md) on how to operate Garage in such a setup. -#### `metadata_snapshots_dir` (since Garage `v1.0.2`) {#metadata_snapshots_dir} +#### `metadata_snapshots_dir` (since `v1.1.0`) {#metadata_snapshots_dir} The directory in which Garage will store metadata snapshots when it performs a snapshot of the metadata database, either when instructed to do @@ -416,7 +416,7 @@ at the cost of a moderate drop in write performance. Similarly to `metatada_fsync`, this is likely not necessary if geographical replication is used. -#### `metadata_auto_snapshot_interval` (since Garage v0.9.4) {#metadata_auto_snapshot_interval} +#### `metadata_auto_snapshot_interval` (since `v0.9.4`) {#metadata_auto_snapshot_interval} If this value is set, Garage will automatically take a snapshot of the metadata DB file at a regular interval and save it in the metadata directory. @@ -453,7 +453,7 @@ you should delete it from the data directory and then call `garage repair blocks` on the node to ensure that it re-obtains a copy from another node on the network. -#### `use_local_tz` {#use_local_tz} +#### `use_local_tz` (since `v1.1.0`) {#use_local_tz} By default, Garage runs the lifecycle worker every day at midnight in UTC. Set the `use_local_tz` configuration value to `true` if you want Garage to run the @@ -475,7 +475,7 @@ files will remain available. This however means that chunks from existing files will not be deduplicated with chunks from newly uploaded files, meaning you might use more storage space that is optimally possible. -#### `block_ram_buffer_max` (since v0.9.4) {#block_ram_buffer_max} +#### `block_ram_buffer_max` (since `v0.9.4`) {#block_ram_buffer_max} A limit on the total size of data blocks kept in RAM by S3 API nodes awaiting to be sent to storage nodes asynchronously. @@ -562,7 +562,7 @@ the node, even in the case of a NAT: the NAT should be configured to forward the port number to the same internal port nubmer. This means that if you have several nodes running behind a NAT, they should each use a different RPC port number. -#### `rpc_bind_outgoing`(since v0.9.2) {#rpc_bind_outgoing} +#### `rpc_bind_outgoing` (since `v0.9.2`) {#rpc_bind_outgoing} If enabled, pre-bind all sockets for outgoing connections to the same IP address used for listening (the IP address specified in `rpc_bind_addr`) before From 12f15c4c2b40c52e481b8bd31ab68aa07c615d41 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 5 Mar 2025 11:00:19 +0100 Subject: [PATCH 42/43] fix readme paths in cargo.toml for new crates --- src/api/admin/Cargo.toml | 2 +- src/api/common/Cargo.toml | 2 +- src/api/k2v/Cargo.toml | 2 +- src/api/s3/Cargo.toml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/api/admin/Cargo.toml b/src/api/admin/Cargo.toml index a61d13e5..7b1d65e1 100644 --- a/src/api/admin/Cargo.toml +++ b/src/api/admin/Cargo.toml @@ -6,7 +6,7 @@ edition = "2018" license = "AGPL-3.0" description = "Admin 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" diff --git a/src/api/common/Cargo.toml b/src/api/common/Cargo.toml index 8fab93ee..6d906423 100644 --- a/src/api/common/Cargo.toml +++ b/src/api/common/Cargo.toml @@ -6,7 +6,7 @@ edition = "2018" license = "AGPL-3.0" description = "Common functions for the API server crates for the Garage object store" repository = "https://git.deuxfleurs.fr/Deuxfleurs/garage" -readme = "../../README.md" +readme = "../../../README.md" [lib] path = "lib.rs" diff --git a/src/api/k2v/Cargo.toml b/src/api/k2v/Cargo.toml index c8c7c8ef..385aef3b 100644 --- a/src/api/k2v/Cargo.toml +++ b/src/api/k2v/Cargo.toml @@ -6,7 +6,7 @@ edition = "2018" license = "AGPL-3.0" description = "K2V 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" diff --git a/src/api/s3/Cargo.toml b/src/api/s3/Cargo.toml index 2507ee07..7b0cac94 100644 --- a/src/api/s3/Cargo.toml +++ b/src/api/s3/Cargo.toml @@ -6,7 +6,7 @@ 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" From f3b05ff7716bda30d251bc090923b7e6513c1ce4 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Wed, 5 Mar 2025 12:06:05 +0100 Subject: [PATCH 43/43] doc: fix version number in quick start --- doc/book/quick-start/_index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/book/quick-start/_index.md b/doc/book/quick-start/_index.md index b74500d5..41867b19 100644 --- a/doc/book/quick-start/_index.md +++ b/doc/book/quick-start/_index.md @@ -132,7 +132,7 @@ docker run \ -v /etc/garage.toml:/path/to/garage.toml \ -v /var/lib/garage/meta:/path/to/garage/meta \ -v /var/lib/garage/data:/path/to/garage/data \ - dxflrs/garage:v0.9.4 + dxflrs/garage:v1.1.0 ``` Under Linux, you can substitute `--network host` for `-p 3900:3900 -p 3901:3901 -p 3902:3902 -p 3903:3903`