don't modify postobject request before validating policy

Reviewed-on: Deuxfleurs/garage#846
Reviewed-by: Quentin <quentin@dufour.io>

Cargo.lock

@@ -1527,6 +1527,7 @@ dependencies = [
  "garage_util",
  "gethostname",
  "hex",
+ "ipnet",
  "itertools 0.12.1",
  "k8s-openapi",
  "kube",

Cargo.nix

@@ -34,7 +34,7 @@ args@{
   ignoreLockHash,
 }:
 let
-  nixifiedLockHash = "1ccd5eb25a83962821e0e9da4ce6df31717b2b97a5b3a0c80c9e0e0759710143";
+  nixifiedLockHash = "fc41fb639a69d62c8c0fb3f9c227162162ebc8142c6fa5cd0599dc381dcd9ebb";
   workspaceSrc = if args.workspaceSrc == null then ./. else args.workspaceSrc;
   currentLockHash = builtins.hashFile "sha256" (workspaceSrc + /Cargo.lock);
   lockHashIgnored = if ignoreLockHash
@@ -2219,6 +2219,7 @@ in
       garage_util = (rustPackages."unknown".garage_util."1.0.0" { inherit profileName; }).out;
       gethostname = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".gethostname."0.4.3" { inherit profileName; }).out;
       hex = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".hex."0.4.3" { inherit profileName; }).out;
+      ipnet = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".ipnet."2.9.0" { inherit profileName; }).out;
       itertools = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".itertools."0.12.1" { inherit profileName; }).out;
       ${ if rootFeatures' ? "garage/kubernetes-discovery" || rootFeatures' ? "garage_rpc/k8s-openapi" || rootFeatures' ? "garage_rpc/kubernetes-discovery" then "k8s_openapi" else null } = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".k8s-openapi."0.21.0" { inherit profileName; }).out;
       ${ if rootFeatures' ? "garage/kubernetes-discovery" || rootFeatures' ? "garage_rpc/kube" || rootFeatures' ? "garage_rpc/kubernetes-discovery" then "kube" else null } = (rustPackages."registry+https://github.com/rust-lang/crates.io-index".kube."0.88.1" { inherit profileName; }).out;
@@ -3016,8 +3017,8 @@ in
     registry = "registry+https://github.com/rust-lang/crates.io-index";
     src = fetchCratesIo { inherit name version; sha256 = "8f518f335dce6725a761382244631d86cf0ccb2863413590b31338feb467f9c3"; };
     features = builtins.concatLists [
-      (lib.optional (rootFeatures' ? "garage/consul-discovery" || rootFeatures' ? "garage_rpc/consul-discovery" || rootFeatures' ? "garage_rpc/reqwest") "default")
-      (lib.optional (rootFeatures' ? "garage/consul-discovery" || rootFeatures' ? "garage_rpc/consul-discovery" || rootFeatures' ? "garage_rpc/reqwest") "std")
+      [ "default" ]
+      [ "std" ]
     ];
   });
   

Cargo.toml

@@ -55,6 +55,7 @@ hexdump = "0.1"
 hmac = "0.12"
 idna = "0.5"
 itertools = "0.12"
+ipnet = "2.9.0"
 lazy_static = "1.4"
 md-5 = "0.10"
 mktemp = "0.5"

doc/book/cookbook/real-world.md

@@ -152,6 +152,8 @@ Check the following for your configuration files:
 - Make sure `rpc_public_addr` contains the public IP address of the node you are configuring.
   This parameter is optional but recommended: if your nodes have trouble communicating with
   one another, consider adding it.
+  Alternatively, you can also set `rpc_public_addr_subnet`, which can filter
+  the addresses announced to other peers to a specific subnet.
 
 - Make sure `rpc_secret` is the same value on all nodes. It should be a 32-bytes hex-encoded secret key.
   You can generate such a key with `openssl rand -hex 32`.

doc/book/design/related-work.md

@@ -67,7 +67,7 @@ Pithos has been abandonned and should probably not used yet, in the following we
 Pithos was relying as a S3 proxy in front of Cassandra (and was working with Scylla DB too).
 From its designers' mouth, storing data in Cassandra has shown its limitations justifying the project abandonment.
 They built a closed-source version 2 that does not store blobs in the database (only metadata) but did not communicate further on it.
-We considered there v2's design but concluded that it does not fit both our *Self-contained & lightweight* and *Simple* properties. It makes the development, the deployment and the operations more complicated while reducing the flexibility.
+We considered their v2's design but concluded that it does not fit both our *Self-contained & lightweight* and *Simple* properties. It makes the development, the deployment and the operations more complicated while reducing the flexibility.
 
 **[Riak CS](https://docs.riak.com/riak/cs/2.1.1/index.html):**
 *Not written yet*

doc/book/reference-manual/configuration.md

@@ -31,6 +31,9 @@ rpc_secret = "4425f5c26c5e11581d3223904324dcb5b5d5dfb14e5e7f35e38c595424f5f1e6"
 rpc_bind_addr = "[::]:3901"
 rpc_bind_outgoing = false
 rpc_public_addr = "[fc00:1::1]:3901"
+# or set rpc_public_adr_subnet to filter down autodiscovery to a subnet:
+# rpc_public_addr_subnet = "2001:0db8:f00:b00:/64"
+
 
 allow_world_readable_secrets = false
 
@@ -105,6 +108,7 @@ Top-level configuration options:
 [`rpc_bind_addr`](#rpc_bind_addr),
 [`rpc_bind_outgoing`](#rpc_bind_outgoing),
 [`rpc_public_addr`](#rpc_public_addr),
+[`rpc_public_addr_subnet`](#rpc_public_addr_subnet)
 [`rpc_secret`/`rpc_secret_file`](#rpc_secret).
 
 The `[consul_discovery]` section:
@@ -295,7 +299,7 @@ Since `v0.8.0`, Garage can use alternative storage backends as follows:
 
 | DB engine | `db_engine` value | Database path |
 | --------- | ----------------- | ------------- |
-| [LMDB](https://www.lmdb.tech) (since `v0.8.0`, default since `v0.9.0`) | `"lmdb"` | `<metadata_dir>/db.lmdb/` |
+| [LMDB](https://www.symas.com/lmdb) (since `v0.8.0`, default since `v0.9.0`) | `"lmdb"` | `<metadata_dir>/db.lmdb/` |
 | [Sqlite](https://sqlite.org) (since `v0.8.0`) | `"sqlite"` | `<metadata_dir>/db.sqlite` |
 | [Sled](https://sled.rs) (old default, removed since `v1.0`) | `"sled"` | `<metadata_dir>/db/` |
 
@@ -543,6 +547,14 @@ RPC calls.  **This parameter is optional but recommended.** In case you have
 a NAT that binds the RPC port to a port that is different on your public IP,
 this field might help making it work.
 
+#### `rpc_public_addr_subnet` {#rpc_public_addr_subnet}
+In case `rpc_public_addr` is not set, but autodiscovery is used, this allows
+filtering the list of automatically discovered IPs to a specific subnet.
+
+For example, if nodes should pick *their* IP inside a specific subnet, but you
+don't want to explicitly write the IP down (as it's dynamic, or you want to
+share configs across nodes), you can use this option.
+
 #### `bootstrap_peers` {#bootstrap_peers}
 
 A list of peer identifiers on which to contact other Garage peers of this cluster.

script/helm/garage/templates/workload.yaml

@@ -64,6 +64,10 @@ spec:
               name: web-api
             - containerPort: 3903
               name: admin
+          {{- with .Values.environment }}
+          env:
+            {{- toYaml . | nindent 12 }}
+          {{- end }}
           volumeMounts:
             - name: meta
               mountPath: /mnt/meta

script/helm/garage/values.yaml

@@ -216,6 +216,8 @@ tolerations: []
 
 affinity: {}
 
+environment: {}
+
 monitoring:
   metrics:
     # If true, a service for monitoring is created with a prometheus.io/scrape annotation

src/api/s3/post_object.rs

@@ -71,21 +71,11 @@ pub async fn handle_post_object(
 		}
 
 		if let Ok(content) = HeaderValue::from_str(&field.text().await?) {
-			match name.as_str() {
-				"tag" => (/* tag need to be reencoded, but we don't support them yet anyway */),
-				"acl" => {
-					if params.insert("x-amz-acl", content).is_some() {
-						return Err(Error::bad_request("Field 'acl' provided more than once"));
-					}
-				}
-				_ => {
-					if params.insert(&name, content).is_some() {
-						return Err(Error::bad_request(format!(
-							"Field '{}' provided more than once",
-							name
-						)));
-					}
-				}
+			if params.insert(&name, content).is_some() {
+				return Err(Error::bad_request(format!(
+					"Field '{}' provided more than once",
+					name
+				)));
 			}
 		}
 	};
@@ -222,6 +212,8 @@ pub async fn handle_post_object(
 		)));
 	}
 
+	// if we ever start supporting ACLs, we likely want to map "acl" to x-amz-acl" somewhere
+	// arround here to make sure the rest of the machinery takes our acl into account.
 	let headers = get_headers(&params)?;
 
 	let expected_checksums = ExpectedChecksums {

src/model/garage.rs

@@ -141,7 +141,7 @@ impl Garage {
 		)?)
 		.ok()
 		.and_then(|x| NetworkKey::from_slice(&x))
-		.ok_or_message("Invalid RPC secret key")?;
+		.ok_or_message("Invalid RPC secret key: expected 32 bits of entropy, please check the documentation for requirements")?;
 
 		let (replication_factor, consistency_mode) = parse_replication_mode(&config)?;
 

src/rpc/Cargo.toml

@@ -24,6 +24,7 @@ bytes.workspace = true
 bytesize.workspace = true
 gethostname.workspace = true
 hex.workspace = true
+ipnet.workspace = true
 tracing.workspace = true
 rand.workspace = true
 itertools.workspace = true

src/rpc/system.rs

@@ -844,12 +844,20 @@ impl NodeStatus {
 	}
 }
 
-fn get_default_ip() -> Option<IpAddr> {
+/// Obtain the list of currently available IP addresses on all non-loopback
+/// interfaces, optionally filtering them to be inside a given IpNet.
+fn get_default_ip(filter_ipnet: Option<ipnet::IpNet>) -> Option<IpAddr> {
 	pnet_datalink::interfaces()
-		.iter()
-		.find(|e| e.is_up() && !e.is_loopback() && !e.ips.is_empty())
-		.and_then(|e| e.ips.first())
-		.map(|a| a.ip())
+		.into_iter()
+		// filter down and loopback interfaces
+		.filter(|i| i.is_up() && !i.is_loopback())
+		// get all IPs
+		.flat_map(|e| e.ips)
+		// optionally, filter to be inside filter_ipnet
+		.find(|ipn| {
+			filter_ipnet.is_some_and(|ipnet| ipnet.contains(&ipn.ip())) || filter_ipnet.is_none()
+		})
+		.map(|ipn| ipn.ip())
 }
 
 fn get_rpc_public_addr(config: &Config) -> Option<SocketAddr> {
@@ -877,7 +885,28 @@ fn get_rpc_public_addr(config: &Config) -> Option<SocketAddr> {
 			}
 		}
 		None => {
-			let addr = get_default_ip().map(|ip| SocketAddr::new(ip, config.rpc_bind_addr.port()));
+			// `No rpc_public_addr` specified, try to discover one, optionally filtering by `rpc_public_addr_subnet`.
+			let filter_subnet: Option<ipnet::IpNet> = config
+				.rpc_public_addr_subnet
+				.as_ref()
+				.and_then(|filter_subnet_str| match filter_subnet_str.parse::<ipnet::IpNet>() {
+					Ok(filter_subnet) => {
+						let filter_subnet_trunc = filter_subnet.trunc();
+						if filter_subnet_trunc != filter_subnet {
+							warn!("`rpc_public_addr_subnet` changed after applying netmask, continuing with {}", filter_subnet.trunc());
+						}
+						Some(filter_subnet_trunc)
+					}
+					Err(e) => {
+						panic!(
+							"Cannot parse rpc_public_addr_subnet {} from config file: {}. Bailing out.",
+							filter_subnet_str, e
+						);
+					}
+				});
+
+			let addr = get_default_ip(filter_subnet)
+				.map(|ip| SocketAddr::new(ip, config.rpc_bind_addr.port()));
 			if let Some(a) = addr {
 				warn!("Using autodetected rpc_public_addr: {}. Consider specifying it explicitly in configuration file if possible.", a);
 			}

src/util/config.rs

@@ -85,6 +85,10 @@ pub struct Config {
 	/// Public IP address of this node
 	pub rpc_public_addr: Option<String>,
 
+	/// In case `rpc_public_addr` was not set, this can filter
+	/// the addresses announced to other peers to a specific subnet.
+	pub rpc_public_addr_subnet: Option<String>,
+
 	/// Timeout for Netapp's ping messagess
 	pub rpc_ping_timeout_msec: Option<u64>,
 	/// Timeout for Netapp RPC calls