laskoviymishka commented on code in PR #3082: URL: https://github.com/apache/iceberg-rust/pull/3082#discussion_r4224607903
########## crates/catalog/rest/src/auth/sigv4.rs: ########## @@ -0,0 +1,1343 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! AWS SigV4 request signing for the REST catalog. + +use chrono::{DateTime, Utc}; +use iceberg::{Error, ErrorKind, Result}; +use sha2::{Digest, Sha256}; +use typed_builder::TypedBuilder; + +/// Hex SHA-256 of the empty string. +const EMPTY_BODY_HEX_SHA256: &str = + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + +/// How the payload hash is encoded in the `x-amz-content-sha256` header. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum PayloadHashMode { + /// Iceberg Java's style: a base64 header when there is a body, hex when + /// there is none, and hex in the canonical request. The base64 comes from + /// how Java configures the AWS SDK, not from SigV4, so a verifier that + /// trusts the header instead of hashing the body rejects it. + IcebergRest, + /// Standard SigV4: hex everywhere. + StandardAws, +} + +fn hex_sha256(data: &[u8]) -> String { + encode_hex(&Sha256::digest(data)) +} + +fn encode_hex(bytes: &[u8]) -> String { + bytes.iter().map(|byte| format!("{byte:02x}")).collect() +} + +fn base64_encode(bytes: &[u8]) -> String { + base64::engine::Engine::encode(&base64::engine::general_purpose::STANDARD, bytes) +} + +/// The `x-amz-content-sha256` value; `None` is no body at all. +fn content_sha256_header(body: Option<&[u8]>, mode: PayloadHashMode) -> String { + match mode { + PayloadHashMode::StandardAws => hex_sha256(body.unwrap_or_default()), + PayloadHashMode::IcebergRest => match body { + None => EMPTY_BODY_HEX_SHA256.to_string(), + Some(body) => base64_encode(&Sha256::digest(body)), + }, + } +} + +/// Signs REST catalog requests the way Iceberg Java's `RESTSigV4AuthSession` +/// does. Carries no credentials, so one signer serves every session. +/// +/// Built with [`SigV4Signer::builder`]. +#[derive(Clone, TypedBuilder)] +pub struct SigV4Signer { + /// The signing region, e.g. `us-east-1`. + #[builder(setter(into))] + region: String, + /// The signing name, e.g. `execute-api`. + #[builder(setter(into))] + service: String, + /// How the payload hash is encoded. + mode: PayloadHashMode, +} + +impl SigV4Signer { + /// Signs `request` in place. An existing `Authorization` moves to + /// `Original-Authorization`, and a caller's `x-amz-date`, + /// `x-amz-content-sha256` or `x-amz-security-token` that the signer + /// overwrites moves to `Original-<name>` (Java keeps a caller's content + /// hash when there is a body). Userinfo leaves the URL, and a `+` in the + /// query becomes `%20`, so write a literal plus as `%2B`. + /// + /// Uses `credentials` as given and never refreshes them: resolve temporary + /// ones from their provider before each call. + /// + /// Fails on a streaming body or a non-UTF-8 header, which cannot be + /// canonicalized faithfully. + /// + /// Send the result through a client that does not follow redirects: a + /// redirect replays the signature, and across hosts reqwest drops + /// `Authorization` but keeps `Original-Authorization`. + /// + /// `aws_sigv4` traces requests without redacting `Original-Authorization`. + /// A `tracing` subscriber is muted for the call, but the `log` bridge (no + /// subscriber, or `log-always`) still forwards those events, so keep + /// `aws_sigv4` below trace level there. + pub fn sign( + &self, + request: &mut crate::HttpRequest, + credentials: &aws_credential_types::Credentials, + ) -> Result<()> { + self.sign_at(request, credentials, Utc::now()) + } + + fn sign_at( + &self, + request: &mut crate::HttpRequest, + credentials: &aws_credential_types::Credentials, + now: DateTime<Utc>, + ) -> Result<()> { + use aws_sigv4::http_request::{SignableBody, SignableRequest, sign}; + use aws_sigv4::sign::v4; + use tracing::level_filters::LevelFilter; + use tracing::subscriber::NoSubscriber; + + let body = signable_body(request)?; + let content_header = content_sha256_header(body.as_deref(), self.mode); + + convert_headers(request); + + // Relocated after signing, so the `Original-` copy is not signed. + let displaced_content_hash: Vec<_> = request + .headers() + .get_all(CONTENT_SHA256) + .iter() + .filter(|v| v.as_bytes() != content_header.as_bytes()) + .cloned() + .collect(); + let content_value = content_header.parse().map_err(|e| { + Error::new(ErrorKind::Unexpected, "invalid x-amz-content-sha256 value").with_source(e) + })?; + request.headers_mut().insert(CONTENT_SHA256, content_value); + + rewrite_url_for_signing(request)?; + + let identity = credentials.clone().into(); + let params = v4::SigningParams::builder() + .identity(&identity) + .region(&self.region) + .name(&self.service) + .time(now.into()) + .settings(signing_settings()) + .build() + .map_err(|e| { + Error::new(ErrorKind::Unexpected, "failed to build SigV4 params").with_source(e) + })? + .into(); + + let headers = signable_headers(request)?; + let signable = SignableRequest::new( + request.method().as_str(), + request.url_str(), + headers.into_iter(), + SignableBody::Bytes(body.as_deref().unwrap_or_default()), + ) + .map_err(|e| { + Error::new(ErrorKind::DataInvalid, "request is not signable").with_source(e) + })?; + + // `aws_sigv4` traces the request, `Original-Authorization` included. + // Mute it only when a subscriber could record that (the max level is + // `OFF` until one is registered): `with_default` also sets + // tracing-core's `EXISTS` flag, which nothing clears, and with the + // `log` feature tracing then stops forwarding events to `log` for good. + let signed = if LevelFilter::current() == LevelFilter::TRACE { + tracing::subscriber::with_default(NoSubscriber::default(), || sign(signable, ¶ms)) + } else { + sign(signable, ¶ms) + }; + let (instructions, _signature) = signed + .map_err(|e| Error::new(ErrorKind::Unexpected, "SigV4 signing failed").with_source(e))? + .into_parts(); + + update_request_headers(request, instructions, displaced_content_hash) + } +} + +/// The body to sign; as in Java, an absent body and an empty one differ. +fn signable_body(request: &crate::HttpRequest) -> Result<Option<Vec<u8>>> { + match request.body() { + crate::HttpRequestBody::Empty => Ok(None), + crate::HttpRequestBody::Buffered(bytes) => Ok(Some(bytes.to_vec())), Review Comment: Optional: this `to_vec()` copies the whole body, and it then gets hashed twice — once for the content header, once inside `aws_sigv4` via `SignableBody::Bytes`. If we compute the hex digest once from the borrowed bytes and pass `SignableBody::Precomputed(hex)`, we drop both the copy and the second hash. Mostly matters for large commit payloads. ########## crates/catalog/rest/src/auth/sigv4.rs: ########## @@ -0,0 +1,1343 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! AWS SigV4 request signing for the REST catalog. + +use chrono::{DateTime, Utc}; +use iceberg::{Error, ErrorKind, Result}; +use sha2::{Digest, Sha256}; +use typed_builder::TypedBuilder; + +/// Hex SHA-256 of the empty string. +const EMPTY_BODY_HEX_SHA256: &str = + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + +/// How the payload hash is encoded in the `x-amz-content-sha256` header. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum PayloadHashMode { + /// Iceberg Java's style: a base64 header when there is a body, hex when + /// there is none, and hex in the canonical request. The base64 comes from + /// how Java configures the AWS SDK, not from SigV4, so a verifier that + /// trusts the header instead of hashing the body rejects it. + IcebergRest, + /// Standard SigV4: hex everywhere. + StandardAws, +} + +fn hex_sha256(data: &[u8]) -> String { + encode_hex(&Sha256::digest(data)) +} + +fn encode_hex(bytes: &[u8]) -> String { + bytes.iter().map(|byte| format!("{byte:02x}")).collect() +} + +fn base64_encode(bytes: &[u8]) -> String { + base64::engine::Engine::encode(&base64::engine::general_purpose::STANDARD, bytes) +} + +/// The `x-amz-content-sha256` value; `None` is no body at all. +fn content_sha256_header(body: Option<&[u8]>, mode: PayloadHashMode) -> String { + match mode { + PayloadHashMode::StandardAws => hex_sha256(body.unwrap_or_default()), + PayloadHashMode::IcebergRest => match body { + None => EMPTY_BODY_HEX_SHA256.to_string(), + Some(body) => base64_encode(&Sha256::digest(body)), + }, + } +} + +/// Signs REST catalog requests the way Iceberg Java's `RESTSigV4AuthSession` +/// does. Carries no credentials, so one signer serves every session. +/// +/// Built with [`SigV4Signer::builder`]. +#[derive(Clone, TypedBuilder)] +pub struct SigV4Signer { + /// The signing region, e.g. `us-east-1`. + #[builder(setter(into))] + region: String, + /// The signing name, e.g. `execute-api`. + #[builder(setter(into))] + service: String, + /// How the payload hash is encoded. + mode: PayloadHashMode, +} + +impl SigV4Signer { + /// Signs `request` in place. An existing `Authorization` moves to + /// `Original-Authorization`, and a caller's `x-amz-date`, + /// `x-amz-content-sha256` or `x-amz-security-token` that the signer + /// overwrites moves to `Original-<name>` (Java keeps a caller's content + /// hash when there is a body). Userinfo leaves the URL, and a `+` in the + /// query becomes `%20`, so write a literal plus as `%2B`. + /// + /// Uses `credentials` as given and never refreshes them: resolve temporary + /// ones from their provider before each call. + /// + /// Fails on a streaming body or a non-UTF-8 header, which cannot be + /// canonicalized faithfully. + /// + /// Send the result through a client that does not follow redirects: a + /// redirect replays the signature, and across hosts reqwest drops + /// `Authorization` but keeps `Original-Authorization`. + /// + /// `aws_sigv4` traces requests without redacting `Original-Authorization`. + /// A `tracing` subscriber is muted for the call, but the `log` bridge (no + /// subscriber, or `log-always`) still forwards those events, so keep + /// `aws_sigv4` below trace level there. + pub fn sign( + &self, + request: &mut crate::HttpRequest, + credentials: &aws_credential_types::Credentials, Review Comment: I'd re-export this type through the crate under the `sigv4` feature (`pub use aws_credential_types::Credentials;`) rather than naming it bare in the public `sign` signature. As it stands downstream has to add its own `aws-credential-types` dep on a compatible 1.x just to construct the argument, and the version coupling isn't visible from our API. Re-exporting makes the dependency explicit and leaves us a seam if we ever want to wrap it. ########## crates/catalog/rest/src/auth/sigv4.rs: ########## @@ -0,0 +1,1343 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! AWS SigV4 request signing for the REST catalog. + +use chrono::{DateTime, Utc}; +use iceberg::{Error, ErrorKind, Result}; +use sha2::{Digest, Sha256}; +use typed_builder::TypedBuilder; + +/// Hex SHA-256 of the empty string. +const EMPTY_BODY_HEX_SHA256: &str = + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + +/// How the payload hash is encoded in the `x-amz-content-sha256` header. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum PayloadHashMode { + /// Iceberg Java's style: a base64 header when there is a body, hex when + /// there is none, and hex in the canonical request. The base64 comes from + /// how Java configures the AWS SDK, not from SigV4, so a verifier that + /// trusts the header instead of hashing the body rejects it. + IcebergRest, + /// Standard SigV4: hex everywhere. + StandardAws, +} + +fn hex_sha256(data: &[u8]) -> String { + encode_hex(&Sha256::digest(data)) +} + +fn encode_hex(bytes: &[u8]) -> String { + bytes.iter().map(|byte| format!("{byte:02x}")).collect() +} + +fn base64_encode(bytes: &[u8]) -> String { + base64::engine::Engine::encode(&base64::engine::general_purpose::STANDARD, bytes) +} + +/// The `x-amz-content-sha256` value; `None` is no body at all. +fn content_sha256_header(body: Option<&[u8]>, mode: PayloadHashMode) -> String { + match mode { + PayloadHashMode::StandardAws => hex_sha256(body.unwrap_or_default()), + PayloadHashMode::IcebergRest => match body { + None => EMPTY_BODY_HEX_SHA256.to_string(), + Some(body) => base64_encode(&Sha256::digest(body)), + }, + } +} + +/// Signs REST catalog requests the way Iceberg Java's `RESTSigV4AuthSession` +/// does. Carries no credentials, so one signer serves every session. +/// +/// Built with [`SigV4Signer::builder`]. +#[derive(Clone, TypedBuilder)] +pub struct SigV4Signer { + /// The signing region, e.g. `us-east-1`. + #[builder(setter(into))] + region: String, + /// The signing name, e.g. `execute-api`. + #[builder(setter(into))] + service: String, + /// How the payload hash is encoded. + mode: PayloadHashMode, +} + +impl SigV4Signer { + /// Signs `request` in place. An existing `Authorization` moves to + /// `Original-Authorization`, and a caller's `x-amz-date`, + /// `x-amz-content-sha256` or `x-amz-security-token` that the signer + /// overwrites moves to `Original-<name>` (Java keeps a caller's content + /// hash when there is a body). Userinfo leaves the URL, and a `+` in the + /// query becomes `%20`, so write a literal plus as `%2B`. + /// + /// Uses `credentials` as given and never refreshes them: resolve temporary + /// ones from their provider before each call. + /// + /// Fails on a streaming body or a non-UTF-8 header, which cannot be + /// canonicalized faithfully. + /// + /// Send the result through a client that does not follow redirects: a + /// redirect replays the signature, and across hosts reqwest drops + /// `Authorization` but keeps `Original-Authorization`. + /// + /// `aws_sigv4` traces requests without redacting `Original-Authorization`. + /// A `tracing` subscriber is muted for the call, but the `log` bridge (no + /// subscriber, or `log-always`) still forwards those events, so keep + /// `aws_sigv4` below trace level there. + pub fn sign( + &self, + request: &mut crate::HttpRequest, + credentials: &aws_credential_types::Credentials, + ) -> Result<()> { + self.sign_at(request, credentials, Utc::now()) + } + + fn sign_at( + &self, + request: &mut crate::HttpRequest, + credentials: &aws_credential_types::Credentials, + now: DateTime<Utc>, + ) -> Result<()> { + use aws_sigv4::http_request::{SignableBody, SignableRequest, sign}; + use aws_sigv4::sign::v4; + use tracing::level_filters::LevelFilter; + use tracing::subscriber::NoSubscriber; + + let body = signable_body(request)?; + let content_header = content_sha256_header(body.as_deref(), self.mode); + + convert_headers(request); + + // Relocated after signing, so the `Original-` copy is not signed. + let displaced_content_hash: Vec<_> = request + .headers() + .get_all(CONTENT_SHA256) + .iter() + .filter(|v| v.as_bytes() != content_header.as_bytes()) + .cloned() + .collect(); + let content_value = content_header.parse().map_err(|e| { + Error::new(ErrorKind::Unexpected, "invalid x-amz-content-sha256 value").with_source(e) + })?; + request.headers_mut().insert(CONTENT_SHA256, content_value); + + rewrite_url_for_signing(request)?; + + let identity = credentials.clone().into(); + let params = v4::SigningParams::builder() + .identity(&identity) + .region(&self.region) + .name(&self.service) + .time(now.into()) + .settings(signing_settings()) + .build() + .map_err(|e| { + Error::new(ErrorKind::Unexpected, "failed to build SigV4 params").with_source(e) + })? + .into(); + + let headers = signable_headers(request)?; + let signable = SignableRequest::new( + request.method().as_str(), + request.url_str(), + headers.into_iter(), + SignableBody::Bytes(body.as_deref().unwrap_or_default()), + ) + .map_err(|e| { + Error::new(ErrorKind::DataInvalid, "request is not signable").with_source(e) + })?; + + // `aws_sigv4` traces the request, `Original-Authorization` included. + // Mute it only when a subscriber could record that (the max level is + // `OFF` until one is registered): `with_default` also sets + // tracing-core's `EXISTS` flag, which nothing clears, and with the + // `log` feature tracing then stops forwarding events to `log` for good. + let signed = if LevelFilter::current() == LevelFilter::TRACE { + tracing::subscriber::with_default(NoSubscriber::default(), || sign(signable, ¶ms)) + } else { + sign(signable, ¶ms) + }; + let (instructions, _signature) = signed + .map_err(|e| Error::new(ErrorKind::Unexpected, "SigV4 signing failed").with_source(e))? + .into_parts(); + + update_request_headers(request, instructions, displaced_content_hash) + } +} + +/// The body to sign; as in Java, an absent body and an empty one differ. +fn signable_body(request: &crate::HttpRequest) -> Result<Option<Vec<u8>>> { + match request.body() { + crate::HttpRequestBody::Empty => Ok(None), + crate::HttpRequestBody::Buffered(bytes) => Ok(Some(bytes.to_vec())), + crate::HttpRequestBody::Streaming => Err(Error::new( + ErrorKind::FeatureUnsupported, + "cannot sign a streaming request body", + )), + } +} + +/// The headers to sign. A non-UTF-8 one is an error: skipping it would send +/// it unsigned. +fn signable_headers(request: &crate::HttpRequest) -> Result<Vec<(&str, &str)>> { + request + .headers() + .iter() + .map(|(n, v)| { + let v = v.to_str().map_err(|e| { + Error::new( + ErrorKind::DataInvalid, + format!("cannot sign non-UTF-8 header value for `{n}`"), + ) + .with_source(e) + })?; + Ok((n.as_str(), v)) + }) + .collect() +} + +/// Drops userinfo, which the wire `Host` never carries, and rewrites `+` in the +/// query as `%20`: a space to AWS and Java either way, but unambiguous to any +/// verifier. +fn rewrite_url_for_signing(request: &mut crate::HttpRequest) -> Result<()> { + if !request.url().username().is_empty() || request.url().password().is_some() { + let url = request.url_mut(); + url.set_username("") + .and_then(|()| url.set_password(None)) + .map_err(|()| { + Error::new( + ErrorKind::DataInvalid, + "cannot strip userinfo from the request URL", + ) + })?; + } + if let Some(query) = request.url().query().filter(|q| q.contains('+')) { + let unambiguous = query.replace('+', "%20"); + request.url_mut().set_query(Some(&unambiguous)); + } + Ok(()) +} + +/// `Aws4Signer`'s settings: normalized double-encoded path, and Java's ignore +/// list. +fn signing_settings() -> aws_sigv4::http_request::SigningSettings { + use aws_sigv4::http_request::{ + PayloadChecksumKind, PercentEncodingMode, SigningSettings, UriPathNormalizationMode, + }; + + let mut settings = SigningSettings::default(); + settings.percent_encoding_mode = PercentEncodingMode::Double; + settings.uri_path_normalization_mode = UriPathNormalizationMode::Enabled; + // We set the header ourselves: base64 for IcebergRest. + settings.payload_checksum_kind = PayloadChecksumKind::NoHeader; + let mut excluded = settings.excluded_headers.take().unwrap_or_default(); + excluded.extend([ + // Java's list, spelled out rather than left to the crate's defaults. + "connection".into(), + "expect".into(), + "transfer-encoding".into(), + "user-agent".into(), + "x-amzn-trace-id".into(), + // Not Java's, but proxies append to it too. + "x-forwarded-for".into(), + // Relocation appends to these after signing. + "original-x-amz-date".into(), + "original-x-amz-content-sha256".into(), + "original-x-amz-security-token".into(), + ]); + settings.excluded_headers = Some(excluded); + settings +} + +/// Java's `convertHeaders`: moves `Authorization` aside before signing, so the +/// moved copy is signed. +fn convert_headers(request: &mut crate::HttpRequest) { + let displaced: Vec<_> = request + .headers() + .get_all(reqwest::header::AUTHORIZATION) + .iter() + .cloned() + .collect(); + if displaced.is_empty() { + return; + } + request.headers_mut().remove(reqwest::header::AUTHORIZATION); + for mut value in displaced { + value.set_sensitive(true); + request.headers_mut().append(RELOCATED_AUTHORIZATION, value); + } +} + +/// Java's `updateRequestHeaders`: installs the signed headers, moving +/// conflicting caller values aside. +fn update_request_headers( + request: &mut crate::HttpRequest, + instructions: aws_sigv4::http_request::SigningInstructions, + displaced_content_hash: Vec<reqwest::header::HeaderValue>, +) -> Result<()> { + let (signed_headers, _params) = instructions.into_parts(); + let h = request.headers_mut(); + for mut value in displaced_content_hash { + // The original may carry a credential. + value.set_sensitive(true); + h.append(RELOCATED_CONTENT_SHA256, value); + } + for header in signed_headers { + let name: reqwest::header::HeaderName = header.name().parse().map_err(|e| { + Error::new(ErrorKind::Unexpected, "invalid signed header name").with_source(e) + })?; + if let Some(relocated) = relocated_name(name.as_str()) { + relocate_conflicting(h, name.as_str(), header.value(), relocated); + } + let mut value: reqwest::header::HeaderValue = header.value().parse().map_err(|e| { + Error::new(ErrorKind::Unexpected, "invalid signed header value").with_source(e) + })?; + if name == reqwest::header::AUTHORIZATION || name == SECURITY_TOKEN { + value.set_sensitive(true); + } + h.insert(name, value); + } + Ok(()) +} + +const CONTENT_SHA256: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("x-amz-content-sha256"); +const AMZ_DATE: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("x-amz-date"); +const SECURITY_TOKEN: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("x-amz-security-token"); + +const RELOCATED_AUTHORIZATION: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("original-authorization"); +const RELOCATED_AMZ_DATE: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("original-x-amz-date"); +const RELOCATED_CONTENT_SHA256: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("original-x-amz-content-sha256"); +const RELOCATED_SECURITY_TOKEN: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("original-x-amz-security-token"); + +/// The `Original-<name>` counterpart of a header the signer generates. +fn relocated_name(name: &str) -> Option<reqwest::header::HeaderName> { + match name { + n if n == AMZ_DATE => Some(RELOCATED_AMZ_DATE), + n if n == CONTENT_SHA256 => Some(RELOCATED_CONTENT_SHA256), Review Comment: The `CONTENT_SHA256` arm can't fire as things stand: `signing_settings` uses `PayloadChecksumKind::NoHeader`, so `aws_sigv4` never emits `x-amz-content-sha256` among the signing instructions, and the caller's content hash is relocated separately through `displaced_content_hash`. I'd drop the arm with a comment, or at least note the invariant at `signing_settings` — otherwise a future flip of the checksum kind would silently double-relocate it. ########## crates/catalog/rest/src/auth/sigv4.rs: ########## @@ -0,0 +1,1343 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! AWS SigV4 request signing for the REST catalog. + +use chrono::{DateTime, Utc}; +use iceberg::{Error, ErrorKind, Result}; +use sha2::{Digest, Sha256}; +use typed_builder::TypedBuilder; + +/// Hex SHA-256 of the empty string. +const EMPTY_BODY_HEX_SHA256: &str = + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + +/// How the payload hash is encoded in the `x-amz-content-sha256` header. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum PayloadHashMode { + /// Iceberg Java's style: a base64 header when there is a body, hex when + /// there is none, and hex in the canonical request. The base64 comes from + /// how Java configures the AWS SDK, not from SigV4, so a verifier that + /// trusts the header instead of hashing the body rejects it. + IcebergRest, + /// Standard SigV4: hex everywhere. + StandardAws, +} + +fn hex_sha256(data: &[u8]) -> String { + encode_hex(&Sha256::digest(data)) +} + +fn encode_hex(bytes: &[u8]) -> String { + bytes.iter().map(|byte| format!("{byte:02x}")).collect() +} + +fn base64_encode(bytes: &[u8]) -> String { + base64::engine::Engine::encode(&base64::engine::general_purpose::STANDARD, bytes) +} + +/// The `x-amz-content-sha256` value; `None` is no body at all. +fn content_sha256_header(body: Option<&[u8]>, mode: PayloadHashMode) -> String { + match mode { + PayloadHashMode::StandardAws => hex_sha256(body.unwrap_or_default()), + PayloadHashMode::IcebergRest => match body { + None => EMPTY_BODY_HEX_SHA256.to_string(), + Some(body) => base64_encode(&Sha256::digest(body)), + }, + } +} + +/// Signs REST catalog requests the way Iceberg Java's `RESTSigV4AuthSession` +/// does. Carries no credentials, so one signer serves every session. +/// +/// Built with [`SigV4Signer::builder`]. +#[derive(Clone, TypedBuilder)] +pub struct SigV4Signer { + /// The signing region, e.g. `us-east-1`. + #[builder(setter(into))] + region: String, + /// The signing name, e.g. `execute-api`. + #[builder(setter(into))] + service: String, + /// How the payload hash is encoded. + mode: PayloadHashMode, +} + +impl SigV4Signer { + /// Signs `request` in place. An existing `Authorization` moves to + /// `Original-Authorization`, and a caller's `x-amz-date`, + /// `x-amz-content-sha256` or `x-amz-security-token` that the signer + /// overwrites moves to `Original-<name>` (Java keeps a caller's content + /// hash when there is a body). Userinfo leaves the URL, and a `+` in the + /// query becomes `%20`, so write a literal plus as `%2B`. + /// + /// Uses `credentials` as given and never refreshes them: resolve temporary + /// ones from their provider before each call. + /// + /// Fails on a streaming body or a non-UTF-8 header, which cannot be + /// canonicalized faithfully. + /// + /// Send the result through a client that does not follow redirects: a + /// redirect replays the signature, and across hosts reqwest drops + /// `Authorization` but keeps `Original-Authorization`. + /// + /// `aws_sigv4` traces requests without redacting `Original-Authorization`. + /// A `tracing` subscriber is muted for the call, but the `log` bridge (no + /// subscriber, or `log-always`) still forwards those events, so keep + /// `aws_sigv4` below trace level there. + pub fn sign( + &self, + request: &mut crate::HttpRequest, + credentials: &aws_credential_types::Credentials, + ) -> Result<()> { + self.sign_at(request, credentials, Utc::now()) + } + + fn sign_at( + &self, + request: &mut crate::HttpRequest, + credentials: &aws_credential_types::Credentials, + now: DateTime<Utc>, + ) -> Result<()> { + use aws_sigv4::http_request::{SignableBody, SignableRequest, sign}; + use aws_sigv4::sign::v4; + use tracing::level_filters::LevelFilter; + use tracing::subscriber::NoSubscriber; + + let body = signable_body(request)?; + let content_header = content_sha256_header(body.as_deref(), self.mode); + + convert_headers(request); Review Comment: `sign_at` mutates the request here — relocates `Authorization`, overwrites `x-amz-content-sha256`, strips userinfo, rewrites the query — before the fallible steps further down (the non-UTF-8 check in `signable_headers`, `SignableRequest::new`, `sign`). On an error the caller gets its request back half-signed: original `Authorization` gone, our content hash and `Original-*` headers in place, userinfo and `+` already stripped from the URL. A caller that retries or falls back on that error then sends something unauthenticated. The non-UTF-8 header is the only realistic trigger, and it's cheap to close: hoist `signable_headers(request)?` above `convert_headers` so we bail before touching anything. For the remaining near-unreachable failures, a line on the `sign()` doc saying the request is left partially modified on error would cover it. ########## crates/catalog/rest/src/auth/sigv4.rs: ########## @@ -0,0 +1,1343 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! AWS SigV4 request signing for the REST catalog. + +use chrono::{DateTime, Utc}; +use iceberg::{Error, ErrorKind, Result}; +use sha2::{Digest, Sha256}; +use typed_builder::TypedBuilder; + +/// Hex SHA-256 of the empty string. +const EMPTY_BODY_HEX_SHA256: &str = + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + +/// How the payload hash is encoded in the `x-amz-content-sha256` header. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum PayloadHashMode { + /// Iceberg Java's style: a base64 header when there is a body, hex when + /// there is none, and hex in the canonical request. The base64 comes from + /// how Java configures the AWS SDK, not from SigV4, so a verifier that + /// trusts the header instead of hashing the body rejects it. + IcebergRest, + /// Standard SigV4: hex everywhere. + StandardAws, +} + +fn hex_sha256(data: &[u8]) -> String { + encode_hex(&Sha256::digest(data)) +} + +fn encode_hex(bytes: &[u8]) -> String { + bytes.iter().map(|byte| format!("{byte:02x}")).collect() +} + +fn base64_encode(bytes: &[u8]) -> String { + base64::engine::Engine::encode(&base64::engine::general_purpose::STANDARD, bytes) +} + +/// The `x-amz-content-sha256` value; `None` is no body at all. +fn content_sha256_header(body: Option<&[u8]>, mode: PayloadHashMode) -> String { + match mode { + PayloadHashMode::StandardAws => hex_sha256(body.unwrap_or_default()), + PayloadHashMode::IcebergRest => match body { + None => EMPTY_BODY_HEX_SHA256.to_string(), + Some(body) => base64_encode(&Sha256::digest(body)), + }, + } +} + +/// Signs REST catalog requests the way Iceberg Java's `RESTSigV4AuthSession` +/// does. Carries no credentials, so one signer serves every session. +/// +/// Built with [`SigV4Signer::builder`]. +#[derive(Clone, TypedBuilder)] +pub struct SigV4Signer { + /// The signing region, e.g. `us-east-1`. + #[builder(setter(into))] + region: String, + /// The signing name, e.g. `execute-api`. + #[builder(setter(into))] + service: String, + /// How the payload hash is encoded. + mode: PayloadHashMode, +} + +impl SigV4Signer { + /// Signs `request` in place. An existing `Authorization` moves to + /// `Original-Authorization`, and a caller's `x-amz-date`, + /// `x-amz-content-sha256` or `x-amz-security-token` that the signer + /// overwrites moves to `Original-<name>` (Java keeps a caller's content + /// hash when there is a body). Userinfo leaves the URL, and a `+` in the + /// query becomes `%20`, so write a literal plus as `%2B`. + /// + /// Uses `credentials` as given and never refreshes them: resolve temporary + /// ones from their provider before each call. + /// + /// Fails on a streaming body or a non-UTF-8 header, which cannot be + /// canonicalized faithfully. + /// + /// Send the result through a client that does not follow redirects: a + /// redirect replays the signature, and across hosts reqwest drops + /// `Authorization` but keeps `Original-Authorization`. + /// + /// `aws_sigv4` traces requests without redacting `Original-Authorization`. + /// A `tracing` subscriber is muted for the call, but the `log` bridge (no + /// subscriber, or `log-always`) still forwards those events, so keep + /// `aws_sigv4` below trace level there. + pub fn sign( + &self, + request: &mut crate::HttpRequest, + credentials: &aws_credential_types::Credentials, + ) -> Result<()> { + self.sign_at(request, credentials, Utc::now()) + } + + fn sign_at( + &self, + request: &mut crate::HttpRequest, + credentials: &aws_credential_types::Credentials, + now: DateTime<Utc>, + ) -> Result<()> { + use aws_sigv4::http_request::{SignableBody, SignableRequest, sign}; + use aws_sigv4::sign::v4; + use tracing::level_filters::LevelFilter; + use tracing::subscriber::NoSubscriber; + + let body = signable_body(request)?; + let content_header = content_sha256_header(body.as_deref(), self.mode); + + convert_headers(request); + + // Relocated after signing, so the `Original-` copy is not signed. + let displaced_content_hash: Vec<_> = request + .headers() + .get_all(CONTENT_SHA256) + .iter() + .filter(|v| v.as_bytes() != content_header.as_bytes()) + .cloned() + .collect(); + let content_value = content_header.parse().map_err(|e| { + Error::new(ErrorKind::Unexpected, "invalid x-amz-content-sha256 value").with_source(e) + })?; + request.headers_mut().insert(CONTENT_SHA256, content_value); + + rewrite_url_for_signing(request)?; + + let identity = credentials.clone().into(); + let params = v4::SigningParams::builder() + .identity(&identity) + .region(&self.region) + .name(&self.service) + .time(now.into()) + .settings(signing_settings()) + .build() + .map_err(|e| { + Error::new(ErrorKind::Unexpected, "failed to build SigV4 params").with_source(e) + })? + .into(); + + let headers = signable_headers(request)?; + let signable = SignableRequest::new( + request.method().as_str(), + request.url_str(), + headers.into_iter(), + SignableBody::Bytes(body.as_deref().unwrap_or_default()), + ) + .map_err(|e| { + Error::new(ErrorKind::DataInvalid, "request is not signable").with_source(e) + })?; + + // `aws_sigv4` traces the request, `Original-Authorization` included. + // Mute it only when a subscriber could record that (the max level is + // `OFF` until one is registered): `with_default` also sets + // tracing-core's `EXISTS` flag, which nothing clears, and with the + // `log` feature tracing then stops forwarding events to `log` for good. + let signed = if LevelFilter::current() == LevelFilter::TRACE { + tracing::subscriber::with_default(NoSubscriber::default(), || sign(signable, ¶ms)) + } else { + sign(signable, ¶ms) + }; + let (instructions, _signature) = signed + .map_err(|e| Error::new(ErrorKind::Unexpected, "SigV4 signing failed").with_source(e))? + .into_parts(); + + update_request_headers(request, instructions, displaced_content_hash) + } +} + +/// The body to sign; as in Java, an absent body and an empty one differ. +fn signable_body(request: &crate::HttpRequest) -> Result<Option<Vec<u8>>> { + match request.body() { + crate::HttpRequestBody::Empty => Ok(None), + crate::HttpRequestBody::Buffered(bytes) => Ok(Some(bytes.to_vec())), + crate::HttpRequestBody::Streaming => Err(Error::new( + ErrorKind::FeatureUnsupported, + "cannot sign a streaming request body", + )), + } +} + +/// The headers to sign. A non-UTF-8 one is an error: skipping it would send +/// it unsigned. +fn signable_headers(request: &crate::HttpRequest) -> Result<Vec<(&str, &str)>> { + request + .headers() + .iter() + .map(|(n, v)| { + let v = v.to_str().map_err(|e| { + Error::new( + ErrorKind::DataInvalid, + format!("cannot sign non-UTF-8 header value for `{n}`"), + ) + .with_source(e) + })?; + Ok((n.as_str(), v)) + }) + .collect() +} + +/// Drops userinfo, which the wire `Host` never carries, and rewrites `+` in the +/// query as `%20`: a space to AWS and Java either way, but unambiguous to any +/// verifier. +fn rewrite_url_for_signing(request: &mut crate::HttpRequest) -> Result<()> { + if !request.url().username().is_empty() || request.url().password().is_some() { + let url = request.url_mut(); + url.set_username("") + .and_then(|()| url.set_password(None)) + .map_err(|()| { + Error::new( + ErrorKind::DataInvalid, + "cannot strip userinfo from the request URL", + ) + })?; + } + if let Some(query) = request.url().query().filter(|q| q.contains('+')) { + let unambiguous = query.replace('+', "%20"); + request.url_mut().set_query(Some(&unambiguous)); + } + Ok(()) +} + +/// `Aws4Signer`'s settings: normalized double-encoded path, and Java's ignore +/// list. +fn signing_settings() -> aws_sigv4::http_request::SigningSettings { + use aws_sigv4::http_request::{ + PayloadChecksumKind, PercentEncodingMode, SigningSettings, UriPathNormalizationMode, + }; + + let mut settings = SigningSettings::default(); + settings.percent_encoding_mode = PercentEncodingMode::Double; + settings.uri_path_normalization_mode = UriPathNormalizationMode::Enabled; + // We set the header ourselves: base64 for IcebergRest. + settings.payload_checksum_kind = PayloadChecksumKind::NoHeader; + let mut excluded = settings.excluded_headers.take().unwrap_or_default(); + excluded.extend([ + // Java's list, spelled out rather than left to the crate's defaults. + "connection".into(), + "expect".into(), + "transfer-encoding".into(), + "user-agent".into(), + "x-amzn-trace-id".into(), + // Not Java's, but proxies append to it too. + "x-forwarded-for".into(), + // Relocation appends to these after signing. + "original-x-amz-date".into(), + "original-x-amz-content-sha256".into(), + "original-x-amz-security-token".into(), + ]); + settings.excluded_headers = Some(excluded); + settings +} + +/// Java's `convertHeaders`: moves `Authorization` aside before signing, so the +/// moved copy is signed. +fn convert_headers(request: &mut crate::HttpRequest) { + let displaced: Vec<_> = request + .headers() + .get_all(reqwest::header::AUTHORIZATION) + .iter() + .cloned() + .collect(); + if displaced.is_empty() { + return; + } + request.headers_mut().remove(reqwest::header::AUTHORIZATION); + for mut value in displaced { + value.set_sensitive(true); + request.headers_mut().append(RELOCATED_AUTHORIZATION, value); + } +} + +/// Java's `updateRequestHeaders`: installs the signed headers, moving +/// conflicting caller values aside. +fn update_request_headers( + request: &mut crate::HttpRequest, + instructions: aws_sigv4::http_request::SigningInstructions, + displaced_content_hash: Vec<reqwest::header::HeaderValue>, +) -> Result<()> { + let (signed_headers, _params) = instructions.into_parts(); + let h = request.headers_mut(); + for mut value in displaced_content_hash { + // The original may carry a credential. + value.set_sensitive(true); + h.append(RELOCATED_CONTENT_SHA256, value); + } + for header in signed_headers { + let name: reqwest::header::HeaderName = header.name().parse().map_err(|e| { + Error::new(ErrorKind::Unexpected, "invalid signed header name").with_source(e) + })?; + if let Some(relocated) = relocated_name(name.as_str()) { + relocate_conflicting(h, name.as_str(), header.value(), relocated); + } + let mut value: reqwest::header::HeaderValue = header.value().parse().map_err(|e| { + Error::new(ErrorKind::Unexpected, "invalid signed header value").with_source(e) + })?; + if name == reqwest::header::AUTHORIZATION || name == SECURITY_TOKEN { + value.set_sensitive(true); + } + h.insert(name, value); + } + Ok(()) +} + +const CONTENT_SHA256: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("x-amz-content-sha256"); +const AMZ_DATE: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("x-amz-date"); +const SECURITY_TOKEN: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("x-amz-security-token"); + +const RELOCATED_AUTHORIZATION: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("original-authorization"); +const RELOCATED_AMZ_DATE: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("original-x-amz-date"); +const RELOCATED_CONTENT_SHA256: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("original-x-amz-content-sha256"); +const RELOCATED_SECURITY_TOKEN: reqwest::header::HeaderName = + reqwest::header::HeaderName::from_static("original-x-amz-security-token"); + +/// The `Original-<name>` counterpart of a header the signer generates. +fn relocated_name(name: &str) -> Option<reqwest::header::HeaderName> { + match name { + n if n == AMZ_DATE => Some(RELOCATED_AMZ_DATE), + n if n == CONTENT_SHA256 => Some(RELOCATED_CONTENT_SHA256), + n if n == SECURITY_TOKEN => Some(RELOCATED_SECURITY_TOKEN), + _ => None, + } +} + +/// Moves `name`'s values that differ from `signed` to `relocated`. +fn relocate_conflicting( + headers: &mut reqwest::header::HeaderMap, + name: &str, + signed: &str, + relocated: reqwest::header::HeaderName, +) { + let conflicting: Vec<_> = headers + .get_all(name) + .iter() + .filter(|value| value.as_bytes() != signed.as_bytes()) + .cloned() + .collect(); + for mut value in conflicting { + // It may carry a credential, e.g. a session token. + value.set_sensitive(true); + headers.append(relocated.clone(), value); + } +} + +impl std::fmt::Debug for SigV4Signer { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SigV4Signer") + .field("region", &self.region) + .field("service", &self.service) + .field("mode", &self.mode) + .finish_non_exhaustive() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::HttpRequest; + + const EMPTY_HEX: &str = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + + #[test] + fn signing_rewrites_an_ambiguous_plus_out_of_the_query() { + use chrono::TimeZone; + + // reqwest writes a space as `+`; signing makes it `%20`. + let mut request = HttpRequest::new( + reqwest::Client::new() + .get("https://rest.example.com/v1/namespaces";) + .query(&[("parent", "my ns")]) + .build() + .unwrap(), + ); + assert!(request.url().query().unwrap().contains("my+ns")); + + let signer = SigV4Signer::builder() + .region("us-east-1") + .service("execute-api") + .mode(PayloadHashMode::StandardAws) + .build(); + let now = Utc.with_ymd_and_hms(2015, 8, 30, 12, 36, 0).unwrap(); + signer + .sign_at(&mut request, &test_credentials(), now) + .unwrap(); + + let query = request.url().query().unwrap(); + assert!(!query.contains('+'), "{query}"); + assert!(query.contains("my%20ns"), "{query}"); + assert_signature_is( + &request, + "b7bb5a323a1ce0ace18454171084deef2dac44c933c3949771fc70179d3cce2b", + ); + } + + #[test] + fn content_sha256_header_iceberg_mode() { + let v = content_sha256_header(Some(b"hello"), PayloadHashMode::IcebergRest); + assert_eq!(v, "LPJNul+wow4m6DsqxbninhsWHlwfp0JecwQzYpOLmCQ="); + let e = content_sha256_header(None, PayloadHashMode::IcebergRest); + assert_eq!(e, EMPTY_HEX); + } + + /// As in Java, an empty body is hashed, unlike an absent one. + #[test] + fn content_sha256_header_separates_an_empty_body_from_an_absent_one() { + let empty = content_sha256_header(Some(b""), PayloadHashMode::IcebergRest); + assert_eq!(empty, "47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU="); + assert_ne!( + empty, + content_sha256_header(None, PayloadHashMode::IcebergRest) + ); + } + + /// The same, through `sign_at`. + #[test] + fn signing_separates_an_empty_body_from_an_absent_one() { + use chrono::TimeZone; + + let signer = test_signer(PayloadHashMode::IcebergRest); + let now = Utc.with_ymd_and_hms(2015, 8, 30, 12, 36, 0).unwrap(); + let hash_of = |builder: reqwest::RequestBuilder| { + let mut req = HttpRequest::new(builder.build().unwrap()); + signer.sign_at(&mut req, &test_credentials(), now).unwrap(); + req.headers() + .get("x-amz-content-sha256") + .unwrap() + .to_str() + .unwrap() + .to_string() + }; + + let client = reqwest::Client::new(); + let url = "https://rest.example.com/v1/namespaces";; + assert_eq!( + hash_of(client.post(url).body("")), + "47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=" + ); + assert_eq!(hash_of(client.post(url)), EMPTY_HEX); + } + + #[test] + fn content_sha256_header_standard_mode() { + let v = content_sha256_header(Some(b"hello"), PayloadHashMode::StandardAws); + assert_eq!( + v, + "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824" + ); + } + + /// Pins a signature this crate produced; `signatures_match_iceberg_java` + /// checks against Java. + fn assert_signature_is(req: &HttpRequest, expected: &str) { + let auth = req + .headers() + .get("authorization") + .unwrap() + .to_str() + .unwrap(); + assert!(auth.ends_with(&format!("Signature={expected}")), "{auth}"); + } + + fn test_signer(mode: PayloadHashMode) -> SigV4Signer { + SigV4Signer::builder() + .region("us-east-1") + .service("execute-api") + .mode(mode) + .build() + } + + fn test_credentials() -> aws_credential_types::Credentials { + aws_credential_types::Credentials::new("ak", "sk", None::<String>, None, "test") + } + + /// Records every event field. + #[derive(Clone, Default)] + struct CapturedLog(std::sync::Arc<std::sync::Mutex<String>>); + + impl tracing::field::Visit for CapturedLog { + fn record_debug(&mut self, _: &tracing::field::Field, value: &dyn std::fmt::Debug) { + self.0.lock().unwrap().push_str(&format!("{value:?}")); + } + } + + impl tracing::Subscriber for CapturedLog { + fn enabled(&self, _: &tracing::Metadata<'_>) -> bool { + true + } + fn new_span(&self, _: &tracing::span::Attributes<'_>) -> tracing::Id { + tracing::Id::from_u64(1) + } + fn record(&self, _: &tracing::Id, _: &tracing::span::Record<'_>) {} + fn record_follows_from(&self, _: &tracing::Id, _: &tracing::Id) {} + fn event(&self, event: &tracing::Event<'_>) { + event.record(&mut self.clone()); + } + fn enter(&self, _: &tracing::Id) {} + fn exit(&self, _: &tracing::Id) {} + } + + /// `aws_sigv4` does not redact `Original-Authorization` itself. + #[test] + fn signing_does_not_trace_a_relocated_bearer_token() { + use chrono::TimeZone; + + const TOKEN: &str = "Bearer topsecretdelegatetoken"; + let signer = test_signer(PayloadHashMode::IcebergRest); + let mut req = HttpRequest::new( + reqwest::Client::new() + .get("https://rest.example.com/v1/config";) + .header(reqwest::header::AUTHORIZATION, TOKEN) + .build() + .unwrap(), + ); + + let log = CapturedLog::default(); + tracing::subscriber::with_default(log.clone(), || { + // `sign_at` mutes only at this level, which is what is under test. + assert_eq!( + tracing::level_filters::LevelFilter::current(), + tracing::level_filters::LevelFilter::TRACE + ); + signer + .sign_at( + &mut req, + &test_credentials(), + Utc.with_ymd_and_hms(2015, 8, 30, 12, 36, 0).unwrap(), + ) + .unwrap(); + // Proves the capture works. + tracing::trace!(canary = "subscriber-is-live"); + }); + + let captured = log.0.lock().unwrap().clone(); + assert!(captured.contains("subscriber-is-live"), "captured nothing"); + assert!(!captured.contains(TOKEN), "{captured}"); + // The token still travels, it is just not logged. + assert_eq!(req.headers().get(RELOCATED_AUTHORIZATION).unwrap(), TOKEN); + } + + #[test] + fn a_non_utf8_header_value_is_rejected_rather_than_left_unsigned() { + use chrono::TimeZone; + + let signer = test_signer(PayloadHashMode::IcebergRest); + let mut req = HttpRequest::new( + reqwest::Client::new() + .get("https://rest.example.com/v1/config";) + .header( + "x-amz-meta-tenant", + reqwest::header::HeaderValue::from_bytes(b"acme\xfa").unwrap(), + ) + .build() + .unwrap(), + ); + + let err = signer + .sign_at( + &mut req, + &test_credentials(), + Utc.with_ymd_and_hms(2015, 8, 30, 12, 36, 0).unwrap(), + ) + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::DataInvalid); + assert!(err.message().contains("x-amz-meta-tenant"), "{err}"); + } + + /// Relocation appends to a caller's `Original-x-amz-*` after signing, so it + /// must stay unsigned. + #[test] + fn a_caller_supplied_relocation_header_is_not_signed() { + use chrono::TimeZone; + + let signer = test_signer(PayloadHashMode::StandardAws); + let mut req = HttpRequest::new( + reqwest::Client::new() + .get("https://rest.example.com/v1/config";) + .header("x-amz-content-sha256", "caller-hash") + .header("original-x-amz-content-sha256", "previous") + .build() + .unwrap(), + ); + + signer + .sign_at( + &mut req, + &test_credentials(), + Utc.with_ymd_and_hms(2015, 8, 30, 12, 36, 0).unwrap(), + ) + .unwrap(); + + let auth = req + .headers() + .get("authorization") + .unwrap() + .to_str() + .unwrap(); + let signed = auth + .split("SignedHeaders=") + .nth(1) + .unwrap() + .split(',') + .next() + .unwrap(); + assert!( + !signed + .split(';') + .any(|h| h == "original-x-amz-content-sha256"), + "{signed}" + ); + // Both values still travel, they are just outside the signature. + let relocated: Vec<_> = req + .headers() + .get_all("original-x-amz-content-sha256") + .iter() + .map(|v| v.to_str().unwrap()) + .collect(); + assert_eq!(relocated, ["previous", "caller-hash"]); + } + + #[test] + fn userinfo_is_stripped_before_signing() { + use chrono::TimeZone; + + // A hand-built request can carry userinfo; the wire `Host` never does. + let signer = test_signer(PayloadHashMode::StandardAws); + let mut req = HttpRequest::new(reqwest::Request::new( + reqwest::Method::GET, + "https://user:[email protected]/v1/config"; + .parse() + .unwrap(), + )); + assert_eq!(req.url().username(), "user"); + let now = Utc.with_ymd_and_hms(2015, 8, 30, 12, 36, 0).unwrap(); + + signer.sign_at(&mut req, &test_credentials(), now).unwrap(); + + assert_eq!(req.url().username(), ""); + assert_eq!(req.url().password(), None); + assert_signature_is( + &req, + "0f4a3487bcff9dd16bf0a42d06c24dc49b2366e8a928dc9666f8424cf5b306b3", + ); + } + + #[test] + fn a_doubled_slash_in_the_path_is_normalized() { + use chrono::TimeZone; + + // A trailing slash on the catalog URI gives `//v1/...`, which + // `Aws4Signer` collapses. + let signer = test_signer(PayloadHashMode::StandardAws); + let mut req = HttpRequest::new(reqwest::Request::new( + reqwest::Method::GET, + "https://rest.example.com//v1//config".parse().unwrap(), + )); + let now = Utc.with_ymd_and_hms(2015, 8, 30, 12, 36, 0).unwrap(); + + signer.sign_at(&mut req, &test_credentials(), now).unwrap(); + + assert_signature_is( + &req, + "0f4a3487bcff9dd16bf0a42d06c24dc49b2366e8a928dc9666f8424cf5b306b3", Review Comment: This literal and the one in `userinfo_is_stripped_before_signing` are both the signature of a plain `/v1/config` GET — stripping userinfo and collapsing `//` should land on exactly that request. That equivalence is the real property under test, but it's buried in a magic hex string: if canonicalization drifted and the vectors were regenerated, both would move together and still pass. I'd sign an unmodified `/v1/config` request in the same test and assert the two signatures are equal. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
