laskoviymishka commented on code in PR #3082: URL: https://github.com/apache/iceberg-rust/pull/3082#discussion_r4169071923
########## crates/catalog/rest/src/auth/sigv4.rs: ########## @@ -0,0 +1,1296 @@ +// 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}; + +/// 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. +#[derive(Clone)] +pub struct SigV4Signer { + region: String, + service: String, + mode: PayloadHashMode, +} + +impl SigV4Signer { + /// Creates a new SigV4 signer. + pub fn new( Review Comment: This is the one round-1 point still open, and the one I'd really like closed before the first publish: `new` takes two adjacent `impl Into<String>`, so `new("execute-api", "us-east-1", mode)` compiles clean and only surfaces as a 403 from a live endpoint. Once this is on crates.io the positional shape is semver-locked, and the swap has no compile-time or runtime cue. `typed-builder` is already a workspace dep and used elsewhere in this crate, so `SigV4Signer::builder().region(…).service(…).mode(…)` is cheap and kills the swap outright; typed newtypes (`Region`/`ServiceName`) work too if we'd rather keep a free function. ########## crates/catalog/rest/src/auth/sigv4.rs: ########## @@ -0,0 +1,1296 @@ +// 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}; + +/// 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. +#[derive(Clone)] +pub struct SigV4Signer { + region: String, + service: String, + mode: PayloadHashMode, +} + +impl SigV4Signer { + /// Creates a new SigV4 signer. + pub fn new( + region: impl Into<String>, + service: impl Into<String>, + mode: PayloadHashMode, + ) -> Self { + Self { + region: region.into(), + service: service.into(), + mode, + } + } + + /// 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` marks tracing as in Review Comment: The branch is exercised now via `CapturedLog` — thanks, that's exactly what I was after. The comment's reasoning is still off, though: `with_default` is a scoped, thread-local override that restores the previous dispatcher on exit — it doesn't mark tracing in-use for good or disable the `log` bridge process-wide; only `set_global_default` does that. The guard is right, so I'd just correct the stated reason, otherwise someone later removes it thinking the documented risk doesn't apply and starts logging `Original-Authorization`. ########## crates/catalog/rest/src/auth/sigv4.rs: ########## @@ -0,0 +1,1296 @@ +// 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}; + +/// 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. +#[derive(Clone)] +pub struct SigV4Signer { + region: String, + service: String, + mode: PayloadHashMode, +} + +impl SigV4Signer { + /// Creates a new SigV4 signer. + pub fn new( + region: impl Into<String>, + service: impl Into<String>, + mode: PayloadHashMode, + ) -> Self { + Self { + region: region.into(), + service: service.into(), + mode, + } + } + + /// 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` marks tracing as in + // use for good, which turns off its `log` fallback process-wide. + 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) { + if !request.url().username().is_empty() || request.url().password().is_some() { + let url = request.url_mut(); + let _ = url.set_username(""); Review Comment: I'd propagate these instead of discarding them — `set_username`/`set_password` return `Result<(), ()>` and fail on schemes that don't carry userinfo, so on failure the credentials stay in the URL and we sign (and send) the host we meant to strip, which defeats the whole point of the function. Simplest fix is to have `rewrite_url_for_signing` return `Result<()>`, map the `Err(())` to a `DataInvalid`, and `?` it at the `sign_at` call site. -- 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]
