From dff383399769b74141b1d5870db8006f307ae3fe Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 27 Jul 2026 11:40:45 -0700 Subject: [PATCH 1/3] fix(copy): resolve CopyObject sources on path-mapped and OIDC deployments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Server-side CopyObject was unusable on a proxy that maps client paths onto internal bucket names (Source Cooperative's `/{account}/{product}/{key}` → `account:product`), for two independent reasons. Every SDK client hit the first one, so the feature was 100% broken there: 1. `x-amz-copy-source` names the source in a *header*, which no URL rewrite touches. The gateway parsed the raw client-facing value, so the source resolved as `alukach` — an account, not a bucket — and the copy died with `404 NoSuchBucket`. The client signs that header, so a proxy cannot fix it by mutating the request. 2. Even given a resolvable source, `same_backing_store` compared credentials between the two ends. A credential-injecting middleware (`AwsBackendAuth`) only runs against the *destination* config in the dispatch context, so a source resolved from the same `auth_type=oidc` connection still carried its unresolved form and never matched — every copy, including a copy within one product, was rejected as a `501` cross-store copy. - `route_handler.rs`: `RequestInfo::copy_source` + `with_copy_source`, carrying the copy-source rewritten into the gateway's bucket namespace. Mirrors `signing_path`: the signed header stays as sent for SigV4 verification while source resolution uses the mapped value. - `api/request.rs`: `parse_s3_request` takes `copy_source_override`, preferring it over the header when the request is a copy. A PUT without the header stays a `PutObject` regardless of the override. - `path-mapping`: `PathMapping::rewrite_copy_source` folds leading segments into the bucket name, preserving the key's percent-encoding and any `versionId`. Returns `None` for values too short to map, so the caller falls through to the header and the gateway reports the real error. - `proxy.rs`: `same_backing_store` → `same_s3_endpoint`, comparing S3-ness, endpoint, and region only. Credentials are dropped from the comparison deliberately: the copy is signed with the destination's credentials and the source contributes only its bucket name and prefix, so the source's own credentials are never used. Whether the destination may read the source is an IAM question S3 answers with `AccessDenied`. Caller authorization is unaffected — the source is still separately authorized as a read. - Cargo.lock: incidental catch-up to the 0.7.0 version bump. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 20 +- crates/core/src/api/request.rs | 76 +++++++- crates/core/src/proxy.rs | 311 +++++++++++++++++++++++++++--- crates/core/src/route_handler.rs | 24 +++ crates/core/src/router.rs | 1 + crates/path-mapping/src/lib.rs | 88 +++++++++ docs/architecture/crate-layout.md | 1 + docs/reference/operations.md | 22 ++- 8 files changed, 496 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ebd149e..765cbd5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1314,7 +1314,7 @@ dependencies = [ [[package]] name = "multistore" -version = "0.6.4" +version = "0.7.0" dependencies = [ "async-trait", "base64", @@ -1341,7 +1341,7 @@ dependencies = [ [[package]] name = "multistore-cf-workers" -version = "0.6.4" +version = "0.7.0" dependencies = [ "async-trait", "bytes", @@ -1366,7 +1366,7 @@ dependencies = [ [[package]] name = "multistore-cf-workers-example" -version = "0.6.4" +version = "0.7.0" dependencies = [ "bytes", "console_error_panic_hook", @@ -1391,7 +1391,7 @@ dependencies = [ [[package]] name = "multistore-lambda" -version = "0.6.4" +version = "0.7.0" dependencies = [ "bytes", "http", @@ -1411,7 +1411,7 @@ dependencies = [ [[package]] name = "multistore-metering" -version = "0.6.4" +version = "0.7.0" dependencies = [ "bytes", "futures", @@ -1423,7 +1423,7 @@ dependencies = [ [[package]] name = "multistore-oidc-provider" -version = "0.6.4" +version = "0.7.0" dependencies = [ "base64", "chrono", @@ -1443,7 +1443,7 @@ dependencies = [ [[package]] name = "multistore-path-mapping" -version = "0.6.4" +version = "0.7.0" dependencies = [ "multistore", "percent-encoding", @@ -1452,7 +1452,7 @@ dependencies = [ [[package]] name = "multistore-server" -version = "0.6.4" +version = "0.7.0" dependencies = [ "axum", "bytes", @@ -1474,7 +1474,7 @@ dependencies = [ [[package]] name = "multistore-static-config" -version = "0.6.4" +version = "0.7.0" dependencies = [ "chrono", "multistore", @@ -1486,7 +1486,7 @@ dependencies = [ [[package]] name = "multistore-sts" -version = "0.6.4" +version = "0.7.0" dependencies = [ "aes-gcm", "base64", diff --git a/crates/core/src/api/request.rs b/crates/core/src/api/request.rs index 7481147..a8133b2 100644 --- a/crates/core/src/api/request.rs +++ b/crates/core/src/api/request.rs @@ -8,12 +8,19 @@ use http::Method; /// /// Path-style: `/{bucket}/{key}` /// Virtual-hosted-style: Host header `{bucket}.s3.example.com` with path `/{key}` +/// +/// `copy_source_override` replaces the `x-amz-copy-source` header when +/// resolving a `CopyObject` source — see [`RequestInfo::copy_source`]. Pass +/// `None` to use the header as the client sent it. +/// +/// [`RequestInfo::copy_source`]: crate::route_handler::RequestInfo::copy_source pub fn parse_s3_request( method: &Method, uri_path: &str, query: Option<&str>, headers: &http::HeaderMap, host_style: HostStyle, + copy_source_override: Option<&str>, ) -> Result { // GET / with path-style → ListBuckets (no bucket in path) if matches!(host_style, HostStyle::Path) && uri_path.trim_start_matches('/').is_empty() { @@ -38,13 +45,16 @@ pub fn parse_s3_request( // into its own operation handled by `execute_copy`. Callers that invoke // `build_s3_operation` directly (custom resolvers) must replicate this // check — `build_s3_operation` has no access to headers. - if *method == Method::PUT { - if let Some(copy_source) = headers.get("x-amz-copy-source") { - let copy_source = copy_source.to_str().map_err(|_| { + if *method == Method::PUT && headers.contains_key("x-amz-copy-source") { + // The override (a namespace-mapped copy-source) wins when present; the + // header itself stays untouched because the client signed it. + let copy_source = match copy_source_override { + Some(mapped) => mapped, + None => headers["x-amz-copy-source"].to_str().map_err(|_| { ProxyError::InvalidRequest("x-amz-copy-source is not valid UTF-8".into()) - })?; - return build_copy_object(bucket, key, copy_source, query); - } + })?, + }; + return build_copy_object(bucket, key, copy_source, query); } build_s3_operation(method, bucket, key, query) @@ -295,7 +305,7 @@ mod tests { query: Option<&str>, headers: &http::HeaderMap, ) -> Result { - parse_s3_request(&method, path, query, headers, HostStyle::Path) + parse_s3_request(&method, path, query, headers, HostStyle::Path, None) } #[test] @@ -348,6 +358,58 @@ mod tests { ); } + /// A path-mapping proxy hands the gateway a copy-source rewritten into the + /// internal bucket namespace; the signed header keeps its client-facing + /// form, so the override — not the header — must resolve the source. + #[test] + fn copy_source_override_replaces_the_signed_header() { + let mut headers = http::HeaderMap::new(); + headers.insert( + "x-amz-copy-source", + "/account/product/README.md".parse().unwrap(), + ); + let op = parse_s3_request( + &Method::PUT, + "/account:product/dst-key", + None, + &headers, + HostStyle::Path, + Some("/account:product/README.md"), + ) + .unwrap(); + match op { + S3Operation::CopyObject { + src_bucket, + src_key, + .. + } => { + assert_eq!(src_bucket, "account:product"); + assert_eq!(src_key, "README.md"); + } + other => panic!("expected CopyObject, got {other:?}"), + } + } + + /// The override applies only to a request that is already a copy: a PUT + /// with no `x-amz-copy-source` header stays a plain `PutObject`. + #[test] + fn copy_source_override_without_the_header_is_ignored() { + let headers = http::HeaderMap::new(); + let op = parse_s3_request( + &Method::PUT, + "/dst-bucket/dst-key", + None, + &headers, + HostStyle::Path, + Some("/account:product/README.md"), + ) + .unwrap(); + assert!( + matches!(op, S3Operation::PutObject { .. }), + "expected PutObject, got {op:?}" + ); + } + #[test] fn copy_source_without_leading_slash_and_with_version_parses() { let mut headers = http::HeaderMap::new(); diff --git a/crates/core/src/proxy.rs b/crates/core/src/proxy.rs index b6d1b08..b31dee0 100644 --- a/crates/core/src/proxy.rs +++ b/crates/core/src/proxy.rs @@ -339,7 +339,14 @@ where fn op_needs_buffered_body(&self, req: &RequestInfo<'_>) -> bool { let host_style = determine_host_style(req.headers, self.virtual_host_domain.as_deref()); matches!( - request::parse_s3_request(req.method, req.path, req.query, req.headers, host_style), + request::parse_s3_request( + req.method, + req.path, + req.query, + req.headers, + host_style, + req.copy_source, + ), Ok(S3Operation::CreateMultipartUpload { .. } | S3Operation::CompleteMultipartUpload { .. } | S3Operation::AbortMultipartUpload { .. } @@ -689,6 +696,7 @@ where req.query, req.headers, host_style, + req.copy_source, ) { Ok(op) => op, Err(err) => return self.error_result(err, req.path, &request_id, req.source_ip), @@ -1567,9 +1575,9 @@ where /// /// The copy is delegated to the backend S3: a signed `PUT` to the /// destination key carries `x-amz-copy-source` pointing at the source's - /// backend bucket/key. Only same-store copies (source and destination on - /// the same S3 endpoint + credentials) are supported — a native S3 copy - /// cannot reach across two distinct backends. + /// backend bucket/key. Only same-endpoint copies are supported (see + /// [`same_s3_endpoint`]) — a native S3 copy cannot reach across two + /// distinct backends. /// /// The backend's `CopyObjectResult` XML (and any 4xx/5xx, including the /// "error embedded in a 200" case) is passed straight through: it carries @@ -1588,7 +1596,7 @@ where // destination. Upgrade path: proxy-side read-then-write streaming // (with >5 GB handling and 200-embedded-error detection) when a real // cross-store use case appears. - if !same_backing_store(src_config, dest_config) { + if !same_s3_endpoint(src_config, dest_config) { return Err(ProxyError::NotImplemented( "cross-store copy (source and destination on different backends) is not supported" .into(), @@ -1640,23 +1648,30 @@ where } } -/// Whether two bucket configs point at the same S3 backing store, such that a -/// native server-side copy from one to the other is possible. +/// Whether two bucket configs sit on the same S3 endpoint, such that a native +/// server-side copy from one to the other is possible. +/// +/// Requires both to be S3 backends sharing an endpoint and region — then a +/// `PUT` signed for the destination endpoint can name the source bucket in +/// `x-amz-copy-source` and S3 performs the copy internally. The bucket names +/// may differ (cross-bucket copy within one account). /// -/// Requires both to be S3 backends sharing the same endpoint, region, and -/// credentials — then a `PUT` signed for the destination endpoint can name the -/// source bucket in `x-amz-copy-source` and S3 performs the copy internally. -/// The bucket names may differ (cross-bucket copy within one account). This is -/// deliberately conservative: two configs that resolve to the same physical -/// store via different credentials are treated as distinct. -fn same_backing_store(a: &BucketConfig, b: &BucketConfig) -> bool { +/// Credentials are deliberately *not* compared. The copy is signed with the +/// destination's credentials and the source config contributes only its bucket +/// name and prefix, so the source's own credentials are never used. Comparing +/// them is also unobservable here: a credential-injecting middleware (e.g. +/// `AwsBackendAuth`, which swaps `auth_type=oidc` for minted STS keys) only +/// runs against the destination config in the dispatch context, so the source +/// still carries its unresolved form and the two could never match. Whether +/// the destination's credentials may read the source is an IAM question, and +/// S3 answers it with `AccessDenied` — which passes through to the client. +/// Authorization of the caller is unaffected: the source is separately +/// authorized as a read against the caller's identity before we get here. +fn same_s3_endpoint(a: &BucketConfig, b: &BucketConfig) -> bool { a.is_s3_backend() && b.is_s3_backend() && a.option("endpoint") == b.option("endpoint") && a.option("region") == b.option("region") - && a.option("access_key_id") == b.option("access_key_id") - && a.option("secret_access_key") == b.option("secret_access_key") - && a.option("token") == b.option("token") } /// Build the backend `x-amz-copy-source` header value for a same-store copy: @@ -2954,30 +2969,59 @@ mod tests { } #[test] - fn same_backing_store_matches_shared_endpoint_and_creds() { - // Two virtual buckets on the same endpoint/creds but different backend - // buckets are the same store — a cross-bucket copy is native. + fn same_s3_endpoint_matches_shared_endpoint_and_region() { + // Two virtual buckets on the same endpoint but different backend + // buckets are copy-compatible — a cross-bucket copy is native. let mut a = test_bucket_config("src"); let mut b = test_bucket_config("dst"); b.backend_options .insert("bucket_name".into(), "other-backend-bucket".into()); - assert!(same_backing_store(&a, &b)); + assert!(same_s3_endpoint(&a, &b)); - // Different endpoint → different store. + // Different endpoint → a copy can't reach across. b.backend_options .insert("endpoint".into(), "https://minio.example.com".into()); - assert!(!same_backing_store(&a, &b)); + assert!(!same_s3_endpoint(&a, &b)); - // Different credentials → different store (conservative). + // Different region → likewise. let mut c = test_bucket_config("dst2"); c.backend_options - .insert("access_key_id".into(), "AKIADIFFERENT".into()); - assert!(!same_backing_store(&a, &c)); + .insert("region".into(), "eu-west-1".into()); + assert!(!same_s3_endpoint(&a, &c)); // A non-S3 backend is never a copy-compatible store. a.backend_type = "azure".into(); let d = test_bucket_config("dst3"); - assert!(!same_backing_store(&a, &d)); + assert!(!same_s3_endpoint(&a, &d)); + } + + /// A credential-injecting middleware (`AwsBackendAuth`) resolves + /// `auth_type=oidc` into minted STS keys, but only on the *destination* + /// config in the dispatch context — the copy source is resolved afterwards + /// and still carries its unresolved form. Comparing credentials therefore + /// rejected every copy on such a deployment; the endpoint is what matters. + #[test] + fn middleware_resolved_destination_still_matches_unresolved_source() { + let mut src = test_bucket_config("src"); + src.backend_options.remove("access_key_id"); + src.backend_options.remove("secret_access_key"); + src.backend_options + .insert("auth_type".into(), "oidc".into()); + src.backend_options.insert( + "oidc_role_arn".into(), + "arn:aws:iam::123:role/Reader".into(), + ); + + // Post-middleware destination: `auth_type` swapped for minted keys. + let mut dst = test_bucket_config("dst"); + dst.backend_options + .insert("access_key_id".into(), "ASIAMINTEDBYSTS".into()); + dst.backend_options + .insert("secret_access_key".into(), "minted-secret".into()); + dst.backend_options + .insert("token".into(), "sts-session-token".into()); + + assert!(same_s3_endpoint(&src, &dst)); } #[test] @@ -3141,4 +3185,217 @@ mod tests { ); }); } + + /// A registry that only knows internal, already-mapped bucket names — the + /// shape a path-mapping proxy presents (`/{account}/{product}/{key}` is + /// rewritten to the bucket `account:product` before dispatch). + #[derive(Clone)] + struct MappedMockRegistry; + + impl BucketRegistry for MappedMockRegistry { + async fn get_bucket( + &self, + name: &str, + _identity: &ResolvedIdentity, + _operation: &S3Operation, + ) -> Result { + if !name.contains(':') { + return Err(ProxyError::BucketNotFound(name.to_string())); + } + Ok(ResolvedBucket { + config: test_bucket_config(name), + list_rewrite: None, + display_name: None, + }) + } + + async fn list_buckets( + &self, + _identity: &ResolvedIdentity, + ) -> Result, ProxyError> { + Ok(vec![]) + } + } + + /// `x-amz-copy-source` carries a *client-facing* path, which the URL + /// rewrite never touches. Without a mapped override the gateway resolves + /// the raw first segment (an account, not a bucket) and the copy dies with + /// `NoSuchBucket` — the failure every SDK client hits, since they all send + /// the client-facing form. + #[test] + fn unmapped_copy_source_fails_on_a_path_mapping_registry() { + run(async { + let captured = Arc::new(std::sync::Mutex::new(None)); + let gw = ProxyGateway::new( + CaptureHeadersBackend { + captured: captured.clone(), + }, + MappedMockRegistry, + MockCreds, + None, + ); + let mut headers = HeaderMap::new(); + headers.insert( + "x-amz-copy-source", + "/account/product/README.md".parse().unwrap(), + ); + let action = gw + .resolve_request( + Method::PUT, + "/account:product/README.md.copy", + None, + &headers, + None, + ) + .await; + match action { + HandlerAction::Response(resp) => assert_eq!(resp.status, 404), + other => panic!( + "expected 404 Response, got {:?}", + std::mem::discriminant(&other) + ), + } + assert!(captured.lock().unwrap().is_none()); + }); + } + + /// The same request succeeds once the proxy supplies the copy-source + /// mapped into the gateway's bucket namespace. The signed header is left + /// alone; only source resolution uses the override. + #[test] + fn mapped_copy_source_override_resolves_the_source() { + run(async { + let captured = Arc::new(std::sync::Mutex::new(None)); + let gw = ProxyGateway::new( + CaptureHeadersBackend { + captured: captured.clone(), + }, + MappedMockRegistry, + MockCreds, + None, + ); + let mut headers = HeaderMap::new(); + headers.insert( + "x-amz-copy-source", + "/account/product/README.md".parse().unwrap(), + ); + let method = Method::PUT; + let req = RequestInfo::new( + &method, + "/account:product/README.md.copy", + None, + &headers, + None, + ) + .with_copy_source(Some("/account:product/README.md")); + + let (action, _) = gw.resolve_request_with_metadata(&req).await; + match action { + HandlerAction::Response(resp) => assert_eq!(resp.status, 200), + other => panic!( + "expected 200 Response, got {:?}", + std::mem::discriminant(&other) + ), + } + let sent = captured + .lock() + .unwrap() + .clone() + .expect("backend was called"); + assert_eq!( + sent.get("x-amz-copy-source").unwrap(), + "/backend-bucket/README.md" + ); + }); + } + + /// A registry whose configs carry `auth_type=oidc` — credentials are minted + /// by middleware, which only ever sees the destination config. + #[derive(Clone)] + struct OidcRegistry; + + impl BucketRegistry for OidcRegistry { + async fn get_bucket( + &self, + name: &str, + _identity: &ResolvedIdentity, + operation: &S3Operation, + ) -> Result { + let mut config = test_bucket_config(name); + // The destination has been through the credential middleware; the + // copy source (resolved later, as a GetObject) has not. + if matches!(operation, S3Operation::GetObject { .. }) { + config.backend_options.remove("access_key_id"); + config.backend_options.remove("secret_access_key"); + config + .backend_options + .insert("auth_type".into(), "oidc".into()); + } else { + config + .backend_options + .insert("token".into(), "sts-session-token".into()); + } + Ok(ResolvedBucket { + config, + list_rewrite: None, + display_name: None, + }) + } + + async fn list_buckets( + &self, + _identity: &ResolvedIdentity, + ) -> Result, ProxyError> { + Ok(vec![]) + } + } + + /// Source and destination are the same product on the same endpoint, so the + /// copy is native — even though only the destination carries the STS + /// credentials the middleware minted. + #[test] + fn copy_succeeds_when_only_the_destination_has_minted_credentials() { + run(async { + let captured = Arc::new(std::sync::Mutex::new(None)); + let gw = ProxyGateway::new( + CaptureHeadersBackend { + captured: captured.clone(), + }, + OidcRegistry, + MockCreds, + None, + ); + let mut headers = HeaderMap::new(); + headers.insert("x-amz-copy-source", "/src-bucket/obj.txt".parse().unwrap()); + let action = gw + .resolve_request( + Method::PUT, + "/dst-bucket/obj.txt.copy", + None, + &headers, + None, + ) + .await; + match action { + HandlerAction::Response(resp) => assert_eq!( + resp.status, 200, + "asymmetric credential materialization must not read as a cross-store copy" + ), + other => panic!( + "expected 200 Response, got {:?}", + std::mem::discriminant(&other) + ), + } + assert_eq!( + captured + .lock() + .unwrap() + .clone() + .expect("backend was called") + .get("x-amz-copy-source") + .unwrap(), + "/backend-bucket/obj.txt" + ); + }); + } } diff --git a/crates/core/src/route_handler.rs b/crates/core/src/route_handler.rs index ba62e9a..a1d0acb 100644 --- a/crates/core/src/route_handler.rs +++ b/crates/core/src/route_handler.rs @@ -263,6 +263,22 @@ pub struct RequestInfo<'a> { /// When `None`, `query` is used for both operation parsing and signature /// verification. pub signing_query: Option<&'a str>, + /// The `x-amz-copy-source` value rewritten into the gateway's bucket + /// namespace, for proxies that rewrite paths before dispatch. + /// + /// `CopyObject` names its source in a header, not the URL, so a proxy that + /// maps client paths onto internal bucket names (see + /// `multistore-path-mapping`) must map the copy-source too — otherwise the + /// source resolves as a client-facing name the registry has never heard of + /// and the copy fails with `NoSuchBucket`. The header itself is signed by + /// the client and must not be mutated, so the mapped value is passed + /// alongside it here: signature verification keeps using the original + /// header, while source resolution uses this value. + /// + /// Expects the same wire format as the header + /// (`[/]bucket/percent-encoded-key[?versionId=id]`). When `None`, the + /// `x-amz-copy-source` header is used as sent. + pub copy_source: Option<&'a str>, /// The form-encoded request body, for handlers that accept parameters in /// the body as well as the query string. /// @@ -301,6 +317,7 @@ impl<'a> RequestInfo<'a> { params: Params::default(), signing_path: None, signing_query: None, + copy_source: None, form_body: None, } } @@ -380,6 +397,13 @@ impl<'a> RequestInfo<'a> { self.signing_query = signing_query; self } + + /// Set the namespace-rewritten `x-amz-copy-source` value for `CopyObject`. + /// See [`RequestInfo::copy_source`]. + pub fn with_copy_source(mut self, copy_source: Option<&'a str>) -> Self { + self.copy_source = copy_source; + self + } } /// A pluggable handler that can intercept requests before proxy dispatch. diff --git a/crates/core/src/router.rs b/crates/core/src/router.rs index 0bc0939..970b644 100644 --- a/crates/core/src/router.rs +++ b/crates/core/src/router.rs @@ -68,6 +68,7 @@ impl Router { source_ip: req.source_ip, signing_path: req.signing_path, signing_query: req.signing_query, + copy_source: req.copy_source, form_body: req.form_body, }; matched diff --git a/crates/path-mapping/src/lib.rs b/crates/path-mapping/src/lib.rs index 2705ac1..682a498 100644 --- a/crates/path-mapping/src/lib.rs +++ b/crates/path-mapping/src/lib.rs @@ -273,6 +273,40 @@ impl PathMapping { } } + /// Rewrite an `x-amz-copy-source` header value into the gateway's bucket + /// namespace. + /// + /// `CopyObject` names its source in a header rather than the URL, so + /// [`rewrite_request`](Self::rewrite_request) never sees it and the gateway + /// would otherwise resolve a client-facing name (`alukach`) that the + /// registry has never heard of. Feed the result to + /// [`RequestInfo::with_copy_source`] alongside the untouched header, which + /// stays as sent for SigV4 verification. + /// + /// `/{a}/{b}/{key}[?versionId=id]` → `/{a:b}/{key}[?versionId=id]`. The key + /// keeps its percent-encoding (only leading segments are rewritten), and + /// the `versionId` suffix rides through. Returns `None` when the value has + /// too few segments to map, leaving the caller to pass the header through + /// unchanged. + /// + /// [`RequestInfo::with_copy_source`]: multistore::route_handler::RequestInfo::with_copy_source + pub fn rewrite_copy_source(&self, copy_source: &str) -> Option { + let value = copy_source.strip_prefix('/').unwrap_or(copy_source); + let (path, version) = match value.split_once("?versionId=") { + Some((p, v)) => (p, Some(v)), + None => (value, None), + }; + + let mapped = self.parse(&format!("/{path}"))?; + let key = mapped.key?; + let mut out = format!("/{}/{}", mapped.bucket, key); + if let Some(version) = version { + out.push_str("?versionId="); + out.push_str(version); + } + Some(out) + } + /// Fold the first prefix component into the bucket name. /// /// `/{account}?prefix=product/sub/` → `/{account:product}?prefix=sub/` @@ -343,6 +377,60 @@ fn rewrite_prefix_in_query(query: &str, new_prefix: &str) -> String { mod tests { use super::*; + fn mapping() -> PathMapping { + PathMapping { + bucket_segments: 2, + bucket_separator: ":".into(), + display_bucket_segments: 1, + } + } + + /// The copy-source header is a client-facing path and must be folded into + /// the internal bucket name, or the gateway resolves `alukach` (an account, + /// not a bucket) and the copy fails with `NoSuchBucket`. + #[test] + fn rewrite_copy_source_folds_leading_segments_into_the_bucket() { + let m = mapping(); + assert_eq!( + m.rewrite_copy_source("/alukach/experimentation/README.md") + .as_deref(), + Some("/alukach:experimentation/README.md") + ); + // A leading slash is optional on the wire. + assert_eq!( + m.rewrite_copy_source("alukach/experimentation/README.md") + .as_deref(), + Some("/alukach:experimentation/README.md") + ); + // Nested keys keep every segment past the bucket. + assert_eq!( + m.rewrite_copy_source("/alukach/experimentation/a/b/c.txt") + .as_deref(), + Some("/alukach:experimentation/a/b/c.txt") + ); + } + + /// The key stays percent-encoded (the gateway decodes it) and `versionId` + /// rides through untouched. + #[test] + fn rewrite_copy_source_preserves_encoding_and_version() { + assert_eq!( + mapping() + .rewrite_copy_source("/acme/data/a%20b.txt?versionId=v42") + .as_deref(), + Some("/acme:data/a%20b.txt?versionId=v42") + ); + } + + /// Too few segments to map (no key, or no product) → `None`, so the caller + /// passes the header through and the gateway reports the real error. + #[test] + fn rewrite_copy_source_declines_unmappable_values() { + let m = mapping(); + assert_eq!(m.rewrite_copy_source("/acme/data"), None); + assert_eq!(m.rewrite_copy_source("/acme"), None); + } + #[test] fn is_list_request_detects_list_type() { assert!(is_list_request("list-type=2")); diff --git a/docs/architecture/crate-layout.md b/docs/architecture/crate-layout.md index 05e0889..ef1c79f 100644 --- a/docs/architecture/crate-layout.md +++ b/docs/architecture/crate-layout.md @@ -79,6 +79,7 @@ The built-in configuration provider (`StaticProvider`) — the only config provi Hierarchical path-based backend resolution: - Maps request paths of the form `/{account}/{product}/{key}` to a per-(account, product) backend - Resolves the backend for each account/product pair from the configured mapping +- Maps the `x-amz-copy-source` header into the same namespace (`PathMapping::rewrite_copy_source`), since `CopyObject` names its source in a header rather than the URL ### `multistore-server` diff --git a/docs/reference/operations.md b/docs/reference/operations.md index a21274d..8aad60e 100644 --- a/docs/reference/operations.md +++ b/docs/reference/operations.md @@ -7,7 +7,7 @@ | GetObject | `GET /{bucket}/{key}` | Forward | Download a file | | HeadObject | `HEAD /{bucket}/{key}` | Forward | Get file metadata | | PutObject | `PUT /{bucket}/{key}` | Forward | Upload a file | -| CopyObject | `PUT /{bucket}/{key}` + `x-amz-copy-source` | Response | Server-side copy within one backing store | +| CopyObject | `PUT /{bucket}/{key}` + `x-amz-copy-source` | Response | Server-side copy within one S3 endpoint | | DeleteObject | `DELETE /{bucket}/{key}` | Forward | Delete a file | | DeleteObjects | `POST /{bucket}?delete` | NeedsBody | Batch-delete up to 1000 keys (`aws s3 rm --recursive`, `delete_objects`) | | ListBucket | `GET /{bucket}` | Response | List objects in a bucket (ListObjectsV1 and V2) | @@ -31,7 +31,22 @@ `CopyObject` is a `PUT /{bucket}/{key}` carrying an `x-amz-copy-source: /{src-bucket}/{src-key}[?versionId=…]` header. The proxy resolves and authorizes **both** ends — the source as a read (`GetObject`) and the destination as a write (`PutObject`) — then delegates the copy to the backend: a signed `PUT` to the destination key carries `x-amz-copy-source` rewritten into the source's backend bucket/key space. The backend's `CopyObjectResult` XML (and any error, including S3's "error embedded in a `200 OK`" case) is passed straight through. Copy-relevant client headers are forwarded and signed: `x-amz-metadata-directive`, `x-amz-tagging-directive`, `x-amz-tagging`, `x-amz-acl`, `x-amz-storage-class`, `x-amz-website-redirect-location`, `x-amz-meta-*`, `x-amz-server-side-encryption*`, and the `x-amz-copy-source-if-*` preconditions. -**Same-store only.** A native S3 copy needs one endpoint that can read the source and write the destination, so source and destination must resolve to the same S3 backend — same endpoint, region, and credentials (the backend bucket names may differ, so cross-bucket copies within one account work). A cross-store copy, or a copy from a backend without a `bucket_name`, is rejected with `501 NotImplemented`. `UploadPartCopy` (a copy-source `PUT` with `uploadId`/`partNumber`) is likewise not supported. +**Same-endpoint only.** A native S3 copy needs one endpoint that can read the source and write the destination, so source and destination must resolve to the same S3 endpoint and region (the backend bucket names may differ, so cross-bucket copies work). A copy across endpoints, or from a backend without a `bucket_name`, is rejected with `501 NotImplemented`. `UploadPartCopy` (a copy-source `PUT` with `uploadId`/`partNumber`) is likewise not supported. + +Credentials are *not* compared between the two ends. The copy is signed with the destination's credentials and the source contributes only its bucket name and prefix, so the source's own credentials are never used; whether the destination's credentials may read the source is an IAM question S3 answers with `AccessDenied`. (Comparing them would also be meaningless: a credential-injecting middleware such as `AwsBackendAuth` runs only against the destination config, so a source resolved from the same `auth_type=oidc` connection could never match.) The caller's own authorization is unaffected — the source is separately authorized as a read. + +**Path-mapped deployments must map the copy-source.** `CopyObject` names its source in a header, so a proxy that rewrites client paths onto internal bucket names must rewrite the copy-source too — otherwise the source resolves as a client-facing name the registry has never heard of and every copy fails with `404 NoSuchBucket`. The client signed that header, so it must not be mutated; pass the mapped value alongside it via `RequestInfo::with_copy_source`, and signature verification keeps using the header as sent. `PathMapping::rewrite_copy_source` produces the mapped value: + +```rust +let mapped = headers + .get("x-amz-copy-source") + .and_then(|v| v.to_str().ok()) + .and_then(|v| mapping.rewrite_copy_source(v)); + +let req = RequestInfo::new(&method, &rewrite.path, query, &headers, None) + .with_signing_path(&rewrite.signing_path) + .with_copy_source(mapped.as_deref()); +``` ### Writes and request headers @@ -67,7 +82,8 @@ Handled by OIDC discovery closures (registered on the `Router` via `OidcRouterEx > [!WARNING] > - **Multipart and batch delete are S3 only** — Both use raw HTTP with `S3RequestSigner` and are gated to `backend_type = "s3"`. Non-S3 backends should use single PUT/DELETE requests. > - **DeleteObject does not return confirmation** — The proxy forwards the DELETE and returns the backend's response status. -> - **Server-side copy is same-store only** — `CopyObject` works when source and destination resolve to the same S3 backend (see "Server-side copy" above). A cross-store copy, or `UploadPartCopy`, is rejected with `501 NotImplemented`. +> - **Server-side copy is same-endpoint only** — `CopyObject` works when source and destination resolve to the same S3 endpoint and region (see "Server-side copy" above). A copy across endpoints, or `UploadPartCopy`, is rejected with `501 NotImplemented`. +> - **Path-mapped proxies must map the copy-source** — `CopyObject` names its source in a header, which no URL rewrite touches. Pass the mapped value via `RequestInfo::with_copy_source` or every copy fails with `404 NoSuchBucket` (see "Server-side copy" above). > - **`x-amz-*` write headers are dropped** — Object metadata, storage class, tagging, ACLs, SSE, and checksum headers on writes are not forwarded (see "Writes and request headers" above). > - **Versioned/MFA delete is not handled** — A `?versionId=` on a delete is ignored; the current object version is deleted. > - **Degenerate object keys are rejected** — Keys containing empty, `.`, or `..` path segments (including leading/trailing slashes, e.g. `dir/` folder markers), or ASCII control characters, return `400 InvalidRequest` on every keyed operation. Real S3 accepts such keys; the proxy is deliberately stricter because they cannot be addressed consistently across its presigned and raw-signed backend paths. Batch-delete body keys are exempt, as the remediation route for legacy keys already stored under such names. From aa3414b434a74a09d499c2cb7d43abcbfd72ee05 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 27 Jul 2026 11:49:02 -0700 Subject: [PATCH 2/3] test(copy): pin CopyObject's two-sided authorization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A copy is two permissions, not one, but nothing pinned that: the existing CopyObject tests run against a registry that authorizes everything, so a regression that dropped either check — or passed the wrong key to it — would have gone unnoticed. `RecordingRegistry` captures every (bucket, action, key) the gateway asks the registry to authorize, and can deny a chosen action: - `copy_authorizes_destination_write_and_source_read` — asserts exactly two authorizations, the destination as `PutObject` on the destination key and the source as `GetObject` on the source key. The keys matter: a registry that scopes by prefix can only enforce that if the key that end touches reaches it. - `copy_denied_when_the_caller_cannot_read_the_source` — write access to the destination does not let a caller launder an unreadable source into a bucket they control. 403, backend never contacted. - `copy_denied_when_the_caller_cannot_write_the_destination` — the mirror case. Co-Authored-By: Claude Opus 5 (1M context) --- crates/core/src/proxy.rs | 164 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/crates/core/src/proxy.rs b/crates/core/src/proxy.rs index b31dee0..93eaff3 100644 --- a/crates/core/src/proxy.rs +++ b/crates/core/src/proxy.rs @@ -3398,4 +3398,168 @@ mod tests { ); }); } + + /// Records every authorization the gateway asks for, and optionally denies + /// one action so a half-authorized caller can be simulated. + #[derive(Clone)] + struct RecordingRegistry { + seen: Arc>>, + deny: Option, + } + + impl BucketRegistry for RecordingRegistry { + async fn get_bucket( + &self, + name: &str, + _identity: &ResolvedIdentity, + operation: &S3Operation, + ) -> Result { + self.seen.lock().unwrap().push(( + name.to_string(), + operation.action(), + operation.key().to_string(), + )); + if self.deny == Some(operation.action()) { + return Err(ProxyError::AccessDenied); + } + Ok(ResolvedBucket { + config: test_bucket_config(name), + list_rewrite: None, + display_name: None, + }) + } + + async fn list_buckets( + &self, + _identity: &ResolvedIdentity, + ) -> Result, ProxyError> { + Ok(vec![]) + } + } + + /// A copy is two permissions, not one. The registry — the authorization + /// seam — must be asked to authorize the destination as a write *and* the + /// source as a read, each against the key that end actually touches. A + /// registry that scopes by key prefix can only enforce that if the right + /// key reaches it. + #[test] + fn copy_authorizes_destination_write_and_source_read() { + run(async { + let seen = Arc::new(std::sync::Mutex::new(Vec::new())); + let gw = ProxyGateway::new( + CaptureHeadersBackend { + captured: Arc::new(std::sync::Mutex::new(None)), + }, + RecordingRegistry { + seen: seen.clone(), + deny: None, + }, + MockCreds, + None, + ); + let mut headers = HeaderMap::new(); + headers.insert( + "x-amz-copy-source", + "/src-bucket/secret/obj.txt".parse().unwrap(), + ); + let action = gw + .resolve_request( + Method::PUT, + "/dst-bucket/public/obj.txt", + None, + &headers, + None, + ) + .await; + assert!(matches!(action, HandlerAction::Response(_))); + + let seen = seen.lock().unwrap().clone(); + assert_eq!( + seen, + vec![ + ( + "dst-bucket".to_string(), + Action::PutObject, + "public/obj.txt".to_string() + ), + ( + "src-bucket".to_string(), + Action::GetObject, + "secret/obj.txt".to_string() + ), + ], + "a copy must authorize the destination write and the source read" + ); + }); + } + + /// Write access to the destination is not enough: a caller who may not read + /// the source cannot launder it into a bucket they control. The denial + /// lands before the backend is contacted, so no bytes move. + #[test] + fn copy_denied_when_the_caller_cannot_read_the_source() { + run(async { + let captured = Arc::new(std::sync::Mutex::new(None)); + let gw = ProxyGateway::new( + CaptureHeadersBackend { + captured: captured.clone(), + }, + RecordingRegistry { + seen: Arc::new(std::sync::Mutex::new(Vec::new())), + deny: Some(Action::GetObject), + }, + MockCreds, + None, + ); + let mut headers = HeaderMap::new(); + headers.insert("x-amz-copy-source", "/src-bucket/obj.txt".parse().unwrap()); + let action = gw + .resolve_request(Method::PUT, "/dst-bucket/obj.txt", None, &headers, None) + .await; + match action { + HandlerAction::Response(resp) => assert_eq!(resp.status, 403), + other => panic!( + "expected 403 Response, got {:?}", + std::mem::discriminant(&other) + ), + } + assert!( + captured.lock().unwrap().is_none(), + "an unauthorized source read must never reach the backend" + ); + }); + } + + /// The mirror case: read access to the source does not let a caller write + /// the destination. Denied before the source is even resolved. + #[test] + fn copy_denied_when_the_caller_cannot_write_the_destination() { + run(async { + let captured = Arc::new(std::sync::Mutex::new(None)); + let gw = ProxyGateway::new( + CaptureHeadersBackend { + captured: captured.clone(), + }, + RecordingRegistry { + seen: Arc::new(std::sync::Mutex::new(Vec::new())), + deny: Some(Action::PutObject), + }, + MockCreds, + None, + ); + let mut headers = HeaderMap::new(); + headers.insert("x-amz-copy-source", "/src-bucket/obj.txt".parse().unwrap()); + let action = gw + .resolve_request(Method::PUT, "/dst-bucket/obj.txt", None, &headers, None) + .await; + match action { + HandlerAction::Response(resp) => assert_eq!(resp.status, 403), + other => panic!( + "expected 403 Response, got {:?}", + std::mem::discriminant(&other) + ), + } + assert!(captured.lock().unwrap().is_none()); + }); + } } From be6fcf87b2a623240aded3094761ec8b975c6c55 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 27 Jul 2026 11:54:57 -0700 Subject: [PATCH 3/3] test(authz): cover CopyObject in the authorization policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `authorize` was only exercised for `DeleteObjects`, so the policy half of a copy's authorization was untested — the gateway tests in aa3414b prove the registry is *asked* about both ends, but nothing proved the default policy answers correctly. - `copy_object_enforces_prefix_on_the_destination_key` — the operation's key must read as the destination key. Mutating `authorize` to key off `src_key` instead makes this fail; without it a prefix-scoped caller could write anywhere in the bucket. - `copy_object_requires_the_put_action` — `CopyObject` maps to `PutObject`, so read access is not copy access. - `copy_object_denied_for_anonymous` — a copy writes, so it is denied even on an anonymous-readable bucket. - `copy_source_read_enforces_prefix_via_get_object` — the source half: the synthetic `GetObject` the gateway builds is prefix-checked as a read. Co-Authored-By: Claude Opus 5 (1M context) --- crates/core/src/auth/authorize.rs | 68 +++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/crates/core/src/auth/authorize.rs b/crates/core/src/auth/authorize.rs index 97021e0..5767dc9 100644 --- a/crates/core/src/auth/authorize.rs +++ b/crates/core/src/auth/authorize.rs @@ -241,4 +241,72 @@ mod tests { // Even on an anonymous-readable bucket, batch delete is a write. assert!(authorize(&ResolvedIdentity::Anonymous, &op, &bucket("b", true)).is_err()); } + + fn copy(dest_key: &str) -> S3Operation { + S3Operation::CopyObject { + bucket: "b".into(), + key: dest_key.into(), + src_bucket: "b".into(), + src_key: "any/source.txt".into(), + src_version: None, + } + } + + /// A copy is a write of its *destination* key, so a prefix-scoped caller may + /// only copy into the prefixes they hold. Were the operation's key to read + /// as anything else (the source key, or empty), a scoped caller could write + /// anywhere in the bucket. + #[test] + fn copy_object_enforces_prefix_on_the_destination_key() { + let id = identity_with(AccessScope { + bucket: "b".into(), + prefixes: vec!["public/".into()], + actions: vec![Action::PutObject], + }); + assert!(authorize(&id, ©("public/x.txt"), &bucket("b", false)).is_ok()); + assert!(authorize(&id, ©("private/x.txt"), &bucket("b", false)).is_err()); + } + + /// Read access is not copy access: `CopyObject` maps to `PutObject`, so a + /// caller holding only `GetObject` cannot copy even within their prefix. + #[test] + fn copy_object_requires_the_put_action() { + let id = identity_with(AccessScope { + bucket: "b".into(), + prefixes: vec![], + actions: vec![Action::GetObject], + }); + assert!(authorize(&id, ©("anywhere.txt"), &bucket("b", false)).is_err()); + } + + /// A copy writes, so an anonymous caller is denied even on a bucket that + /// allows anonymous reads. + #[test] + fn copy_object_denied_for_anonymous() { + assert!(authorize( + &ResolvedIdentity::Anonymous, + ©("x.txt"), + &bucket("b", true) + ) + .is_err()); + } + + /// The other half of a copy's authorization: the gateway resolves the copy + /// source through a synthetic `GetObject`, so the source key is prefix- + /// checked as a read. A caller scoped to `public/` cannot name a source + /// outside it. + #[test] + fn copy_source_read_enforces_prefix_via_get_object() { + let id = identity_with(AccessScope { + bucket: "b".into(), + prefixes: vec!["public/".into()], + actions: vec![Action::GetObject, Action::PutObject], + }); + let read = |key: &str| S3Operation::GetObject { + bucket: "b".into(), + key: key.into(), + }; + assert!(authorize(&id, &read("public/src.txt"), &bucket("b", false)).is_ok()); + assert!(authorize(&id, &read("private/src.txt"), &bucket("b", false)).is_err()); + } }