wgtmac commented on code in PR #616: URL: https://github.com/apache/iceberg-cpp/pull/616#discussion_r3393578492
########## src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc: ########## @@ -0,0 +1,407 @@ +/* + * 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. + */ + +#include "iceberg/catalog/rest/auth/auth_manager_internal.h" +#include "iceberg/catalog/rest/auth/aws_sdk.h" +#include "iceberg/catalog/rest/auth/sigv4_auth_manager_internal.h" + +#ifdef ICEBERG_SIGV4 + +# include <atomic> +# include <mutex> +# include <sstream> + +# include <aws/core/Aws.h> +# include <aws/core/auth/AWSAuthSigner.h> +# include <aws/core/auth/AWSCredentialsProvider.h> +# include <aws/core/auth/AWSCredentialsProviderChain.h> +# include <aws/core/client/ClientConfiguration.h> +# include <aws/core/http/standard/StandardHttpRequest.h> +# include <aws/core/utils/HashingUtils.h> + +# include "iceberg/catalog/rest/auth/auth_managers.h" +# include "iceberg/catalog/rest/auth/auth_properties.h" +# include "iceberg/util/macros.h" +# include "iceberg/util/string_util.h" + +namespace iceberg::rest::auth { + +namespace { + +class AwsSdkLifecycle { + public: + static AwsSdkLifecycle& Instance() { + static AwsSdkLifecycle instance; + return instance; + } + + Status Initialize() { + std::lock_guard<std::mutex> lock(mutex_); + auto s = state_.load(); + if (s == State::kInitialized) return {}; + if (s == State::kFinalized) { + return InvalidArgument("AWS SDK has already been finalized; cannot reinitialize"); + } + Aws::InitAPI(options_); + state_.store(State::kInitialized); + return {}; + } + + Status Finalize() { + std::lock_guard<std::mutex> lock(mutex_); + if (state_.load() != State::kInitialized) return {}; + auto live = active_session_count_.load(); + if (live != 0) { + return Invalid( + "Cannot finalize AWS SDK while {} SigV4 auth session(s) are still alive", live); + } + Aws::ShutdownAPI(options_); + state_.store(State::kFinalized); + return {}; + } + + Status EnsureInitialized() { + if (state_.load() == State::kInitialized) return {}; + return Initialize(); + } + + bool IsInitialized() const { return state_.load() == State::kInitialized; } + bool IsFinalized() const { return state_.load() == State::kFinalized; } + + void IncrementSessionCount() { + active_session_count_.fetch_add(1, std::memory_order_relaxed); + } + void DecrementSessionCount() { + active_session_count_.fetch_sub(1, std::memory_order_relaxed); + } + + private: + enum class State : uint8_t { kUninitialized, kInitialized, kFinalized }; + + AwsSdkLifecycle() = default; + + std::atomic<State> state_{State::kUninitialized}; + std::mutex mutex_; + Aws::SDKOptions options_; + std::atomic<size_t> active_session_count_{0}; +}; + +Aws::Http::HttpMethod ToAwsMethod(HttpMethod method) { + switch (method) { + case HttpMethod::kGet: + return Aws::Http::HttpMethod::HTTP_GET; + case HttpMethod::kPost: + return Aws::Http::HttpMethod::HTTP_POST; + case HttpMethod::kPut: + return Aws::Http::HttpMethod::HTTP_PUT; + case HttpMethod::kDelete: + return Aws::Http::HttpMethod::HTTP_DELETE; + case HttpMethod::kHead: + return Aws::Http::HttpMethod::HTTP_HEAD; + } + return Aws::Http::HttpMethod::HTTP_GET; +} + +std::unordered_map<std::string, std::string> MergeProperties( + const std::unordered_map<std::string, std::string>& base, + const std::unordered_map<std::string, std::string>& overrides) { + auto merged = base; + for (const auto& [key, value] : overrides) { + merged.insert_or_assign(key, value); + } + return merged; +} + +/// Matches Java RESTSigV4AuthSession: canonical headers carry +/// Base64(SHA256(body)), canonical request trailer uses hex. +class RestSigV4Signer : public Aws::Client::AWSAuthV4Signer { + public: + RestSigV4Signer(const std::shared_ptr<Aws::Auth::AWSCredentialsProvider>& creds, + const char* service_name, const Aws::String& region) + : Aws::Client::AWSAuthV4Signer(creds, service_name, region, + PayloadSigningPolicy::Always, + /*urlEscapePath=*/false) { + // Skip the signer's hex overwrite of x-amz-content-sha256 so canonical + // headers see the caller's Base64; ComputePayloadHash still feeds hex + // into the canonical request trailer. + m_includeSha256HashHeader = false; + } +}; + +} // namespace + +// ---- SigV4AuthSession ---- + +SigV4AuthSession::SigV4AuthSession( + std::shared_ptr<AuthSession> delegate, std::string signing_region, + std::string signing_name, + std::shared_ptr<Aws::Auth::AWSCredentialsProvider> credentials_provider) + : delegate_(std::move(delegate)), + signing_region_(std::move(signing_region)), + signing_name_(std::move(signing_name)), + credentials_provider_(std::move(credentials_provider)), + signer_(std::make_unique<RestSigV4Signer>( + credentials_provider_, signing_name_.c_str(), signing_region_.c_str())) { + AwsSdkLifecycle::Instance().IncrementSessionCount(); +} + +SigV4AuthSession::~SigV4AuthSession() { + AwsSdkLifecycle::Instance().DecrementSessionCount(); +} + +Result<HttpRequest> SigV4AuthSession::Authenticate(const HttpRequest& request) { + ICEBERG_ASSIGN_OR_RAISE(auto delegate_request, delegate_->Authenticate(request)); + const auto& original_headers = delegate_request.headers; + + std::unordered_map<std::string, std::string> signing_headers; + for (const auto& [name, value] : original_headers) { + if (StringUtils::EqualsIgnoreCase(name, "Authorization")) { + signing_headers[std::string(kRelocatedHeaderPrefix) + name] = value; + } else { + signing_headers[name] = value; + } + } + + Aws::Http::URI aws_uri(delegate_request.url.c_str()); + auto aws_request = std::make_shared<Aws::Http::Standard::StandardHttpRequest>( + aws_uri, ToAwsMethod(delegate_request.method)); + for (const auto& [name, value] : signing_headers) { + aws_request->SetHeaderValue(Aws::String(name.c_str()), Aws::String(value.c_str())); + } + + // Empty body: hex EMPTY_BODY_SHA256 (Java parity workaround for the signer + // computing an invalid checksum on empty bodies). Non-empty: Base64. + if (delegate_request.body.empty()) { + aws_request->SetHeaderValue("x-amz-content-sha256", Aws::String(kEmptyBodySha256)); + } else { + auto body_stream = + Aws::MakeShared<std::stringstream>("SigV4Body", delegate_request.body); + aws_request->AddContentBody(body_stream); + auto sha256 = Aws::Utils::HashingUtils::CalculateSHA256( + Aws::String(delegate_request.body.data(), delegate_request.body.size())); + aws_request->SetHeaderValue("x-amz-content-sha256", + Aws::Utils::HashingUtils::Base64Encode(sha256)); Review Comment: This is still not fixed. The code still sets non-empty body `x-amz-content-sha256` with `Base64Encode`, and the test still checks for a 44-byte value. This should be hex SHA256, or the signed request will fail against a standard SigV4 verifier. ########## src/iceberg/test/sigv4_auth_test.cc: ########## @@ -0,0 +1,537 @@ +/* + * 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. + */ + +#ifdef ICEBERG_SIGV4 + +# include <string> +# include <unordered_map> + +# include <aws/core/auth/AWSCredentialsProvider.h> +# include <gtest/gtest.h> + +# include "iceberg/catalog/rest/auth/auth_managers.h" +# include "iceberg/catalog/rest/auth/auth_properties.h" +# include "iceberg/catalog/rest/auth/auth_session.h" +# include "iceberg/catalog/rest/auth/aws_sdk.h" +# include "iceberg/catalog/rest/auth/sigv4_auth_manager_internal.h" +# include "iceberg/catalog/rest/http_client.h" +# include "iceberg/table_identifier.h" +# include "iceberg/test/matchers.h" + +namespace iceberg::rest::auth { + +class SigV4AuthTest : public ::testing::Test { + protected: + static void SetUpTestSuite() { ASSERT_THAT(InitializeAwsSdk(), IsOk()); } + + HttpClient client_{{}}; + + std::unordered_map<std::string, std::string> MakeSigV4Properties() { + return { + {AuthProperties::kAuthType, "sigv4"}, + {AuthProperties::kSigV4SigningRegion, "us-east-1"}, + {AuthProperties::kSigV4SigningName, "execute-api"}, + {AuthProperties::kSigV4AccessKeyId, "AKIAIOSFODNN7EXAMPLE"}, + {AuthProperties::kSigV4SecretAccessKey, + "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"}, + }; + } +}; + +TEST_F(SigV4AuthTest, LifecycleInitializeIsIdempotent) { + EXPECT_THAT(InitializeAwsSdk(), IsOk()); + EXPECT_TRUE(IsAwsSdkInitialized()); + EXPECT_FALSE(IsAwsSdkFinalized()); +} + +TEST_F(SigV4AuthTest, LifecycleFinalizeRefusesWhileSessionsAlive) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + EXPECT_THAT(FinalizeAwsSdk(), IsError(ErrorKind::kInvalid)); + EXPECT_TRUE(IsAwsSdkInitialized()); +} + +TEST_F(SigV4AuthTest, LoadSigV4AuthManager) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); +} + +TEST_F(SigV4AuthTest, CatalogSessionProducesSession) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); +} + +TEST_F(SigV4AuthTest, AuthenticateAddsAuthorizationHeader) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + HttpRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + EXPECT_NE(headers.find("authorization"), headers.end()); + EXPECT_TRUE(headers.at("authorization").starts_with("AWS4-HMAC-SHA256")); + EXPECT_NE(headers.find("x-amz-date"), headers.end()); +} + +TEST_F(SigV4AuthTest, AuthenticateWithPostBody) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + HttpRequest request{.method = HttpMethod::kPost, + .url = "https://example.com/v1/namespaces";, + .headers = {{"Content-Type", "application/json"}}, + .body = R"({"namespace":["ns1"]})"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + EXPECT_NE(headers.find("authorization"), headers.end()); + EXPECT_TRUE(headers.at("authorization").starts_with("AWS4-HMAC-SHA256")); +} + +TEST_F(SigV4AuthTest, DelegateAuthorizationHeaderRelocated) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kToken.key()] = "my-oauth-token"; + properties[AuthProperties::kSigV4DelegateAuthType] = "oauth2"; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + HttpRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + EXPECT_NE(headers.find("authorization"), headers.end()); + EXPECT_TRUE(headers.at("authorization").starts_with("AWS4-HMAC-SHA256")); + EXPECT_NE(headers.find("original-authorization"), headers.end()); + EXPECT_EQ(headers.at("original-authorization"), "Bearer my-oauth-token"); +} + +TEST_F(SigV4AuthTest, AuthenticateWithSessionToken) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kSigV4SessionToken] = "FwoGZXIvYXdzEBYaDHqa0"; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + HttpRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + EXPECT_NE(headers.find("authorization"), headers.end()); + EXPECT_NE(headers.find("x-amz-security-token"), headers.end()); + EXPECT_EQ(headers.at("x-amz-security-token"), "FwoGZXIvYXdzEBYaDHqa0"); +} + +TEST_F(SigV4AuthTest, CustomSigningNameAndRegion) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kSigV4SigningRegion] = "eu-west-1"; + properties[AuthProperties::kSigV4SigningName] = "custom-service"; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + HttpRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + EXPECT_TRUE(auth_it->second.find("eu-west-1") != std::string::npos); + EXPECT_TRUE(auth_it->second.find("custom-service") != std::string::npos); +} + +TEST_F(SigV4AuthTest, AuthTypeCaseInsensitive) { + for (const auto& auth_type : {"SIGV4", "SigV4", "sigV4"}) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kAuthType] = auth_type; + EXPECT_THAT(AuthManagers::Load("test-catalog", properties), IsOk()) + << "Failed for auth type: " << auth_type; + } +} + +TEST_F(SigV4AuthTest, DelegateDefaultsToOAuth2NoAuth) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + HttpRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + EXPECT_EQ(headers.find("original-authorization"), headers.end()); +} + +TEST_F(SigV4AuthTest, TableSessionInheritsProperties) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto catalog_session = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(catalog_session, IsOk()); + + iceberg::TableIdentifier table_id{.ns = iceberg::Namespace{{"ns1"}}, .name = "table1"}; + std::unordered_map<std::string, std::string> table_props; + auto table_session = manager_result.value()->TableSession(table_id, table_props, + catalog_session.value()); + ASSERT_THAT(table_session, IsOk()); + + HttpRequest request{.method = HttpMethod::kGet, + .url = "https://example.com/v1/ns1/tables/table1"}; + auto auth_result = table_session.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + EXPECT_NE(auth_result.value().headers.find("authorization"), + auth_result.value().headers.end()); +} + +TEST_F(SigV4AuthTest, AuthenticateWithoutBodyDetailedHeaders) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + HttpRequest request{.method = HttpMethod::kGet, + .url = "http://localhost:8080/path";, + .headers = {{"Content-Type", "application/json"}}}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + // Original header preserved + EXPECT_EQ(headers.at("content-type"), "application/json"); + + // Host header generated by the signer + EXPECT_NE(headers.find("host"), headers.end()); + + // SigV4 headers + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + EXPECT_TRUE(auth_it->second.starts_with("AWS4-HMAC-SHA256 Credential=")); + + EXPECT_TRUE(auth_it->second.find("content-type") != std::string::npos); + EXPECT_TRUE(auth_it->second.find("host") != std::string::npos); + EXPECT_TRUE(auth_it->second.find("x-amz-content-sha256") != std::string::npos); + EXPECT_TRUE(auth_it->second.find("x-amz-date") != std::string::npos); + + // Empty body SHA256 hash + EXPECT_EQ(headers.at("x-amz-content-sha256"), SigV4AuthSession::kEmptyBodySha256); + + // X-Amz-Date present + EXPECT_NE(headers.find("x-amz-date"), headers.end()); +} + +TEST_F(SigV4AuthTest, AuthenticateWithBodyDetailedHeaders) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + HttpRequest request{.method = HttpMethod::kPost, + .url = "http://localhost:8080/path";, + .headers = {{"Content-Type", "application/json"}}, + .body = R"({"namespace":["ns1"]})"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + // SigV4 Authorization header + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + EXPECT_TRUE(auth_it->second.starts_with("AWS4-HMAC-SHA256 Credential=")); + + // x-amz-content-sha256 should be Base64-encoded body SHA256 (matching Java) + auto sha_it = headers.find("x-amz-content-sha256"); + ASSERT_NE(sha_it, headers.end()); + EXPECT_NE(sha_it->second, SigV4AuthSession::kEmptyBodySha256); + + EXPECT_EQ(sha_it->second.size(), 44) + << "Expected Base64 SHA256, got: " << sha_it->second; +} + +TEST_F(SigV4AuthTest, ConflictingAuthorizationHeaderIncludedInSignedHeaders) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kToken.key()] = "my-oauth-token"; + properties[AuthProperties::kSigV4DelegateAuthType] = "oauth2"; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + HttpRequest request{.method = HttpMethod::kGet, + .url = "http://localhost:8080/path";, + .headers = {{"Content-Type", "application/json"}}}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + // SigV4 Authorization header + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + EXPECT_TRUE(auth_it->second.starts_with("AWS4-HMAC-SHA256 Credential=")); + + // Relocated delegate header should be in SignedHeaders + EXPECT_TRUE(auth_it->second.find("original-authorization") != std::string::npos) + << "SignedHeaders should include 'original-authorization', got: " + << auth_it->second; + + // Relocated Authorization present + auto orig_it = headers.find("original-authorization"); + ASSERT_NE(orig_it, headers.end()); + EXPECT_EQ(orig_it->second, "Bearer my-oauth-token"); +} + +TEST_F(SigV4AuthTest, ConflictingSigV4HeadersRelocated) { + auto delegate = AuthSession::MakeDefault({ + {"x-amz-content-sha256", "fake-sha256"}, + {"X-Amz-Date", "fake-date"}, + {"Content-Type", "application/json"}, + }); + auto credentials = + std::make_shared<Aws::Auth::SimpleAWSCredentialsProvider>(Aws::Auth::AWSCredentials( + "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")); + auto session = std::make_shared<SigV4AuthSession>(delegate, "us-east-1", "execute-api", + credentials); + + HttpRequest request{.method = HttpMethod::kGet, .url = "http://localhost:8080/path"}; + auto auth_result = session->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + // The real x-amz-content-sha256 should be the empty body hash (signer overwrites fake) + EXPECT_EQ(headers.at("x-amz-content-sha256"), SigV4AuthSession::kEmptyBodySha256); + + // The fake values should be relocated since the signer produced different values + auto orig_sha_it = headers.find("Original-x-amz-content-sha256"); + ASSERT_NE(orig_sha_it, headers.end()); + EXPECT_EQ(orig_sha_it->second, "fake-sha256"); + + auto orig_date_it = headers.find("Original-X-Amz-Date"); + ASSERT_NE(orig_date_it, headers.end()); + EXPECT_EQ(orig_date_it->second, "fake-date"); + + // SigV4 Authorization present + EXPECT_NE(headers.find("authorization"), headers.end()); +} + +TEST_F(SigV4AuthTest, SessionCloseDelegatesToInner) { + auto delegate = AuthSession::MakeDefault({}); + auto credentials = std::make_shared<Aws::Auth::SimpleAWSCredentialsProvider>( + Aws::Auth::AWSCredentials("id", "secret")); + auto session = std::make_shared<SigV4AuthSession>(delegate, "us-east-1", "execute-api", + credentials); + + // Close should succeed without error + EXPECT_THAT(session->Close(), IsOk()); +} + +TEST_F(SigV4AuthTest, CreateCustomDelegateNone) { + std::unordered_map<std::string, std::string> properties = { + {AuthProperties::kAuthType, "sigv4"}, + {AuthProperties::kSigV4DelegateAuthType, "none"}, + {AuthProperties::kSigV4SigningRegion, "us-west-2"}, + {AuthProperties::kSigV4AccessKeyId, "id"}, + {AuthProperties::kSigV4SecretAccessKey, "secret"}, + }; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + // Authenticate should work with noop delegate + HttpRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + EXPECT_NE(headers.find("authorization"), headers.end()); + EXPECT_EQ(headers.find("original-authorization"), headers.end()); +} + +TEST_F(SigV4AuthTest, CreateInvalidCustomDelegateSigV4Circular) { + std::unordered_map<std::string, std::string> properties = { + {AuthProperties::kAuthType, "sigv4"}, + {AuthProperties::kSigV4DelegateAuthType, "sigv4"}, + {AuthProperties::kSigV4SigningRegion, "us-east-1"}, + {AuthProperties::kSigV4AccessKeyId, "id"}, + {AuthProperties::kSigV4SecretAccessKey, "secret"}, + }; + + auto result = AuthManagers::Load("test-catalog", properties); + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result, + HasErrorMessage("Cannot delegate a SigV4 auth manager to another SigV4")); +} + +TEST_F(SigV4AuthTest, ContextualSessionOverridesProperties) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kSigV4SigningRegion] = "us-west-2"; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto catalog_session = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(catalog_session, IsOk()); + + // Context overrides region and credentials + std::unordered_map<std::string, std::string> context = { + {AuthProperties::kSigV4AccessKeyId, "id2"}, + {AuthProperties::kSigV4SecretAccessKey, "secret2"}, + {AuthProperties::kSigV4SigningRegion, "eu-west-1"}, + }; + + auto ctx_session = + manager_result.value()->ContextualSession(context, catalog_session.value()); + ASSERT_THAT(ctx_session, IsOk()); + + HttpRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = ctx_session.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + + EXPECT_TRUE(auth_it->second.find("eu-west-1") != std::string::npos) + << "Expected eu-west-1 in Authorization, got: " << auth_it->second; +} + +TEST_F(SigV4AuthTest, TableSessionOverridesProperties) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kSigV4SigningRegion] = "us-west-2"; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto catalog_session = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(catalog_session, IsOk()); + + // Table properties override region and credentials + std::unordered_map<std::string, std::string> table_props = { + {AuthProperties::kSigV4AccessKeyId, "table-key-id"}, + {AuthProperties::kSigV4SecretAccessKey, "table-secret"}, + {AuthProperties::kSigV4SigningRegion, "ap-southeast-1"}, + }; + + iceberg::TableIdentifier table_id{.ns = iceberg::Namespace{{"db1"}}, .name = "table1"}; + auto table_session = manager_result.value()->TableSession(table_id, table_props, + catalog_session.value()); + ASSERT_THAT(table_session, IsOk()); + + HttpRequest request{.method = HttpMethod::kGet, + .url = "https://example.com/v1/db1/tables/table1"}; + auto auth_result = table_session.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; + + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + + EXPECT_TRUE(auth_it->second.find("ap-southeast-1") != std::string::npos) + << "Expected ap-southeast-1 in Authorization, got: " << auth_it->second; +} + +// Matches Java RESTSigV4AuthManager: a table session derived from a contextual +// parent does NOT inherit the contextual overrides; it merges catalog props +// with table props directly. Contextual and table are independent dimensions. +TEST_F(SigV4AuthTest, TableSessionIgnoresContextualOverrides) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kSigV4SigningRegion] = "us-west-2"; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto catalog_session = manager_result.value()->CatalogSession(client_, properties); Review Comment: This is still not fixed. The test still asserts a 44-byte Base64 hash. It should validate the 64-char lowercase hex SHA256 value. ########## src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc: ########## @@ -0,0 +1,474 @@ +/* + * 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. + */ + +#include "iceberg/catalog/rest/auth/auth_manager_internal.h" +#include "iceberg/catalog/rest/auth/aws_sdk.h" +#include "iceberg/catalog/rest/auth/sigv4_auth_manager_internal.h" + +#ifdef ICEBERG_SIGV4 + +# include <atomic> +# include <mutex> +# include <sstream> + +# include <aws/core/Aws.h> +# include <aws/core/auth/AWSAuthSigner.h> +# include <aws/core/auth/AWSCredentialsProvider.h> +# include <aws/core/auth/AWSCredentialsProviderChain.h> +# include <aws/core/client/ClientConfiguration.h> +# include <aws/core/http/standard/StandardHttpRequest.h> +# include <aws/core/utils/HashingUtils.h> + +# include "iceberg/catalog/rest/auth/auth_managers.h" +# include "iceberg/catalog/rest/auth/auth_properties.h" +# include "iceberg/catalog/rest/auth/oauth2_util.h" +# include "iceberg/util/macros.h" +# include "iceberg/util/string_util.h" + +namespace iceberg::rest::auth { + +namespace { + +class AwsSdkLifecycle { + public: + static AwsSdkLifecycle& Instance() { + static AwsSdkLifecycle instance; + return instance; + } + + Status Initialize() { + std::lock_guard<std::mutex> lock(mutex_); + auto s = state_.load(); + if (s == State::kInitialized) return {}; + if (s == State::kFinalized) { + return InvalidArgument("AWS SDK has already been finalized; cannot reinitialize"); + } + Aws::InitAPI(options_); + state_.store(State::kInitialized); + return {}; + } + + Status Finalize() { + std::lock_guard<std::mutex> lock(mutex_); + if (state_.load() != State::kInitialized) return {}; + if (active_session_count_ != 0) { + return Invalid( + "Cannot finalize AWS SDK while {} SigV4 auth session(s) are still alive", + active_session_count_); + } + Aws::ShutdownAPI(options_); + state_.store(State::kFinalized); + return {}; + } + + Status EnsureInitialized() { + if (state_.load() == State::kInitialized) return {}; + return Initialize(); + } + + bool IsInitialized() const { return state_.load() == State::kInitialized; } + bool IsFinalized() const { return state_.load() == State::kFinalized; } + + // Holds the mutex while incrementing, so Finalize() can never observe a + // stale 0 between its count check and Aws::ShutdownAPI. + Status RegisterSession() { + std::lock_guard<std::mutex> lock(mutex_); + if (state_.load() != State::kInitialized) { + return InvalidArgument( + "AWS SDK is not initialized; cannot create a SigV4AuthSession"); + } + ++active_session_count_; + return {}; + } + + void UnregisterSession() { + std::lock_guard<std::mutex> lock(mutex_); + --active_session_count_; + } + + private: + enum class State : uint8_t { kUninitialized, kInitialized, kFinalized }; + + AwsSdkLifecycle() = default; + + std::atomic<State> state_{State::kUninitialized}; + std::mutex mutex_; + Aws::SDKOptions options_; + size_t active_session_count_{0}; // guarded by mutex_ +}; + +Aws::Http::HttpMethod ToAwsMethod(HttpMethod method) { + switch (method) { + case HttpMethod::kGet: + return Aws::Http::HttpMethod::HTTP_GET; + case HttpMethod::kPost: + return Aws::Http::HttpMethod::HTTP_POST; + case HttpMethod::kPut: + return Aws::Http::HttpMethod::HTTP_PUT; + case HttpMethod::kDelete: + return Aws::Http::HttpMethod::HTTP_DELETE; + case HttpMethod::kHead: + return Aws::Http::HttpMethod::HTTP_HEAD; + } + return Aws::Http::HttpMethod::HTTP_GET; +} + +std::unordered_map<std::string, std::string> MergeProperties( + const std::unordered_map<std::string, std::string>& base, + const std::unordered_map<std::string, std::string>& overrides) { + auto merged = base; + for (const auto& [key, value] : overrides) { + merged.insert_or_assign(key, value); + } + return merged; +} + +/// Matches Java RESTSigV4AuthSession: canonical headers carry +/// Base64(SHA256(body)), canonical request trailer uses hex. +class RestSigV4Signer : public Aws::Client::AWSAuthV4Signer { + public: + RestSigV4Signer(const std::shared_ptr<Aws::Auth::AWSCredentialsProvider>& creds, + const char* service_name, const Aws::String& region) + : Aws::Client::AWSAuthV4Signer(creds, service_name, region, + PayloadSigningPolicy::Always, + /*urlEscapePath=*/false) { + // Skip the signer's hex overwrite of x-amz-content-sha256 so canonical + // headers see the caller's Base64; ComputePayloadHash still feeds hex + // into the canonical request trailer. + m_includeSha256HashHeader = false; + } +}; + +} // namespace + +// ---- SigV4AuthSession ---- + +SigV4AuthSession::SigV4AuthSession( + std::shared_ptr<AuthSession> delegate, std::string signing_region, + std::string signing_name, + std::shared_ptr<Aws::Auth::AWSCredentialsProvider> credentials_provider) + : delegate_(std::move(delegate)), + signing_region_(std::move(signing_region)), + signing_name_(std::move(signing_name)), + credentials_provider_(std::move(credentials_provider)), + signer_(std::make_unique<RestSigV4Signer>( + credentials_provider_, signing_name_.c_str(), signing_region_.c_str())) {} + +SigV4AuthSession::~SigV4AuthSession() { AwsSdkLifecycle::Instance().UnregisterSession(); } + +Result<HttpRequest> SigV4AuthSession::Authenticate(const HttpRequest& request) { + ICEBERG_ASSIGN_OR_RAISE(auto delegate_request, delegate_->Authenticate(request)); + const auto& original_headers = delegate_request.headers; + + std::unordered_map<std::string, std::string> signing_headers; + for (const auto& [name, value] : original_headers) { + if (StringUtils::EqualsIgnoreCase(name, kAuthorizationHeader)) { + signing_headers[std::string(kRelocatedHeaderPrefix) + name] = value; + } else { + signing_headers[name] = value; + } + } + + Aws::Http::URI aws_uri(delegate_request.url.c_str()); + auto aws_request = std::make_shared<Aws::Http::Standard::StandardHttpRequest>( + aws_uri, ToAwsMethod(delegate_request.method)); + for (const auto& [name, value] : signing_headers) { + aws_request->SetHeaderValue(Aws::String(name.c_str()), Aws::String(value.c_str())); + } + + // Empty body: hex EMPTY_BODY_SHA256 (Java parity workaround for the signer + // computing an invalid checksum on empty bodies). Non-empty: Base64. + if (delegate_request.body.empty()) { + aws_request->SetHeaderValue("x-amz-content-sha256", Aws::String(kEmptyBodySha256)); + } else { + auto body_stream = + Aws::MakeShared<std::stringstream>("SigV4Body", delegate_request.body); + aws_request->AddContentBody(body_stream); + auto sha256 = Aws::Utils::HashingUtils::CalculateSHA256( + Aws::String(delegate_request.body.data(), delegate_request.body.size())); + aws_request->SetHeaderValue("x-amz-content-sha256", + Aws::Utils::HashingUtils::Base64Encode(sha256)); + } + + if (!signer_->SignRequest(*aws_request)) { + return std::unexpected<Error>(Error{.kind = ErrorKind::kAuthenticationFailed, + .message = "SigV4 signing failed"}); + } + + // Build a case-insensitive index of original headers once so the outer + // loop over signed headers below is O(N + M) instead of O(N * M). + std::unordered_map<std::string, std::vector<const std::string*>> originals_by_name; + for (const auto& [orig_name, orig_value] : original_headers) { + originals_by_name[StringUtils::ToLower(orig_name)].push_back(&orig_value); + } + + HttpRequest signed_request{.method = delegate_request.method, + .url = std::move(delegate_request.url), + .headers = {}, + .body = std::move(delegate_request.body)}; + signed_request.headers.reserve(aws_request->GetHeaders().size() + + original_headers.size()); + for (const auto& [aws_name, aws_value] : aws_request->GetHeaders()) { + std::string name(aws_name.c_str(), aws_name.size()); + std::string value(aws_value.c_str(), aws_value.size()); + if (auto it = originals_by_name.find(StringUtils::ToLower(name)); + it != originals_by_name.end()) { + // Preserve every original entry with this name whose value the signer + // didn't produce, matching Java updateRequestHeaders. + for (const auto* orig_value : it->second) { + if (*orig_value != value) { + signed_request.headers.add(std::string(kRelocatedHeaderPrefix) + name, + *orig_value); + } + } + } + signed_request.headers.add(std::move(name), std::move(value)); + } + + return signed_request; +} + +Status SigV4AuthSession::Close() { return delegate_->Close(); } + +// ---- SigV4AuthManager ---- + +SigV4AuthManager::SigV4AuthManager(std::unique_ptr<AuthManager> delegate) + : delegate_(std::move(delegate)) {} + +SigV4AuthManager::~SigV4AuthManager() = default; + +Result<std::shared_ptr<AuthSession>> SigV4AuthManager::InitSession( + HttpClient& init_client, + const std::unordered_map<std::string, std::string>& properties) { + ICEBERG_RETURN_UNEXPECTED(AwsSdkLifecycle::Instance().EnsureInitialized()); + ICEBERG_ASSIGN_OR_RAISE(auto delegate_session, + delegate_->InitSession(init_client, properties)); + return WrapSession(std::move(delegate_session), properties); +} + +Result<std::shared_ptr<AuthSession>> SigV4AuthManager::CatalogSession( + HttpClient& shared_client, + const std::unordered_map<std::string, std::string>& properties) { + ICEBERG_RETURN_UNEXPECTED(AwsSdkLifecycle::Instance().EnsureInitialized()); + catalog_properties_ = properties; + ICEBERG_ASSIGN_OR_RAISE(auto delegate_session, + delegate_->CatalogSession(shared_client, properties)); + return WrapSession(std::move(delegate_session), properties); +} + +// Both derived sessions merge against the stored catalog_properties_, so +// contextual overrides do not propagate into child table sessions. + +Result<std::shared_ptr<AuthSession>> SigV4AuthManager::ContextualSession( + const SessionContext& context, std::shared_ptr<AuthSession> parent) { + auto sigv4_parent = std::dynamic_pointer_cast<SigV4AuthSession>(std::move(parent)); + ICEBERG_PRECHECK(sigv4_parent != nullptr, + "SigV4AuthManager parent must be a SigV4AuthSession"); + + ICEBERG_ASSIGN_OR_RAISE(auto delegate_session, delegate_->ContextualSession( + context, sigv4_parent->delegate())); + + // Merge context.credentials into properties so credential overrides aren't + // dropped. + auto merged = MergeProperties(catalog_properties_, + MergeProperties(context.properties, context.credentials)); + return WrapSession(std::move(delegate_session), merged, + sigv4_parent->credentials_provider()); +} + +Result<std::shared_ptr<AuthSession>> SigV4AuthManager::TableSession( + const TableIdentifier& table, + const std::unordered_map<std::string, std::string>& properties, + std::shared_ptr<AuthSession> parent) { + auto sigv4_parent = std::dynamic_pointer_cast<SigV4AuthSession>(std::move(parent)); + ICEBERG_PRECHECK(sigv4_parent != nullptr, + "SigV4AuthManager parent must be a SigV4AuthSession"); + + ICEBERG_ASSIGN_OR_RAISE( + auto delegate_session, + delegate_->TableSession(table, properties, sigv4_parent->delegate())); + + auto merged = MergeProperties(catalog_properties_, properties); + return WrapSession(std::move(delegate_session), merged, + sigv4_parent->credentials_provider()); +} + +Status SigV4AuthManager::Close() { return delegate_->Close(); } + +// TODO(sigv4): support loading a custom AWSCredentialsProvider via a class +// name property, matching Java's AwsProperties.restCredentialsProvider(). +Result<std::shared_ptr<Aws::Auth::AWSCredentialsProvider>> +SigV4AuthManager::MakeCredentialsProvider( + const std::unordered_map<std::string, std::string>& properties) { + auto access_key_it = properties.find(AuthProperties::kSigV4AccessKeyId); + auto secret_key_it = properties.find(AuthProperties::kSigV4SecretAccessKey); + bool has_ak = access_key_it != properties.end() && !access_key_it->second.empty(); + bool has_sk = secret_key_it != properties.end() && !secret_key_it->second.empty(); + + ICEBERG_PRECHECK( + has_ak == has_sk, "Both '{}' and '{}' must be set together, or neither", + AuthProperties::kSigV4AccessKeyId, AuthProperties::kSigV4SecretAccessKey); + + if (has_ak) { + Aws::Auth::AWSCredentials credentials(access_key_it->second.c_str(), + secret_key_it->second.c_str()); + auto session_token_it = properties.find(AuthProperties::kSigV4SessionToken); + if (session_token_it != properties.end() && !session_token_it->second.empty()) { + credentials.SetSessionToken(session_token_it->second.c_str()); + } + return std::make_shared<Aws::Auth::SimpleAWSCredentialsProvider>(credentials); + } + + return std::make_shared<Aws::Auth::DefaultAWSCredentialsProviderChain>(); +} + +std::string SigV4AuthManager::ResolveSigningRegion( + const std::unordered_map<std::string, std::string>& properties) { + if (auto it = properties.find(AuthProperties::kSigV4SigningRegion); + it != properties.end() && !it->second.empty()) { + return it->second; + } + // ClientConfiguration() walks env / profile / IMDS / us-east-1; the IMDS + // step can block for seconds on non-EC2 hosts. Resolve once per process + // (set AWS_EC2_METADATA_DISABLED=true to skip IMDS). + static const std::string kSdkResolvedRegion = Review Comment: This is still unresolved. `ResolveSigningRegion` still falls back through `ClientConfiguration().region`, which can silently become `us-east-1`. If `rest.signing-region` is absent and the AWS region chain cannot resolve a region, session setup should fail instead of signing for the wrong region. ########## cmake_modules/IcebergThirdpartyToolchain.cmake: ########## @@ -539,3 +539,26 @@ endif() if(ICEBERG_BUILD_REST) resolve_cpr_dependency() endif() + +# ---------------------------------------------------------------------- +# AWS SDK for C++ + +function(resolve_aws_sdk_dependency) + find_package(AWSSDK REQUIRED COMPONENTS core) Review Comment: This is still unresolved. The workflow now documents that S3 and SigV4 are split because linking both AWS SDK copies causes ODR conflicts, but CMake still allows `-DICEBERG_S3=ON -DICEBERG_SIGV4=ON`. Please either make both paths use one SDK instance or reject the combination at configure time. ########## src/iceberg/catalog/rest/meson.build: ########## @@ -40,17 +41,27 @@ cpr_needs_static = ( ) cpr_dep = dependency('cpr', static: cpr_needs_static) +iceberg_rest_sources += files('auth/sigv4_auth_manager.cc') iceberg_rest_build_deps = [iceberg_dep, cpr_dep] +iceberg_rest_compile_defs = [] + +sigv4_opt = get_option('sigv4') +aws_sdk_core_dep = dependency('aws-cpp-sdk-core', required: sigv4_opt) Review Comment: This is still unresolved. Meson now exposes `-Dsigv4=enabled` and registers `sigv4_auth_test`, but CI does not run that path and the dependency lookup is still system-only. Either add Meson SigV4 CI/support parity or leave SigV4 unsupported in Meson for now. ########## src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc: ########## @@ -0,0 +1,474 @@ +/* + * 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. + */ + +#include "iceberg/catalog/rest/auth/auth_manager_internal.h" +#include "iceberg/catalog/rest/auth/aws_sdk.h" +#include "iceberg/catalog/rest/auth/sigv4_auth_manager_internal.h" + +#ifdef ICEBERG_SIGV4 + +# include <atomic> +# include <mutex> +# include <sstream> + +# include <aws/core/Aws.h> +# include <aws/core/auth/AWSAuthSigner.h> +# include <aws/core/auth/AWSCredentialsProvider.h> +# include <aws/core/auth/AWSCredentialsProviderChain.h> +# include <aws/core/client/ClientConfiguration.h> +# include <aws/core/http/standard/StandardHttpRequest.h> +# include <aws/core/utils/HashingUtils.h> + +# include "iceberg/catalog/rest/auth/auth_managers.h" +# include "iceberg/catalog/rest/auth/auth_properties.h" +# include "iceberg/catalog/rest/auth/oauth2_util.h" +# include "iceberg/util/macros.h" +# include "iceberg/util/string_util.h" + +namespace iceberg::rest::auth { + +namespace { + +class AwsSdkLifecycle { + public: + static AwsSdkLifecycle& Instance() { + static AwsSdkLifecycle instance; + return instance; + } + + Status Initialize() { + std::lock_guard<std::mutex> lock(mutex_); + auto s = state_.load(); + if (s == State::kInitialized) return {}; + if (s == State::kFinalized) { + return InvalidArgument("AWS SDK has already been finalized; cannot reinitialize"); + } + Aws::InitAPI(options_); + state_.store(State::kInitialized); + return {}; + } + + Status Finalize() { + std::lock_guard<std::mutex> lock(mutex_); + if (state_.load() != State::kInitialized) return {}; + if (active_session_count_ != 0) { + return Invalid( + "Cannot finalize AWS SDK while {} SigV4 auth session(s) are still alive", + active_session_count_); + } + Aws::ShutdownAPI(options_); + state_.store(State::kFinalized); + return {}; + } + + Status EnsureInitialized() { + if (state_.load() == State::kInitialized) return {}; + return Initialize(); + } + + bool IsInitialized() const { return state_.load() == State::kInitialized; } + bool IsFinalized() const { return state_.load() == State::kFinalized; } + + // Holds the mutex while incrementing, so Finalize() can never observe a + // stale 0 between its count check and Aws::ShutdownAPI. + Status RegisterSession() { + std::lock_guard<std::mutex> lock(mutex_); + if (state_.load() != State::kInitialized) { + return InvalidArgument( + "AWS SDK is not initialized; cannot create a SigV4AuthSession"); + } + ++active_session_count_; + return {}; + } + + void UnregisterSession() { + std::lock_guard<std::mutex> lock(mutex_); + --active_session_count_; + } + + private: + enum class State : uint8_t { kUninitialized, kInitialized, kFinalized }; + + AwsSdkLifecycle() = default; + + std::atomic<State> state_{State::kUninitialized}; + std::mutex mutex_; + Aws::SDKOptions options_; + size_t active_session_count_{0}; // guarded by mutex_ +}; + +Aws::Http::HttpMethod ToAwsMethod(HttpMethod method) { + switch (method) { + case HttpMethod::kGet: + return Aws::Http::HttpMethod::HTTP_GET; + case HttpMethod::kPost: + return Aws::Http::HttpMethod::HTTP_POST; + case HttpMethod::kPut: + return Aws::Http::HttpMethod::HTTP_PUT; + case HttpMethod::kDelete: + return Aws::Http::HttpMethod::HTTP_DELETE; + case HttpMethod::kHead: + return Aws::Http::HttpMethod::HTTP_HEAD; + } + return Aws::Http::HttpMethod::HTTP_GET; +} + +std::unordered_map<std::string, std::string> MergeProperties( + const std::unordered_map<std::string, std::string>& base, + const std::unordered_map<std::string, std::string>& overrides) { + auto merged = base; + for (const auto& [key, value] : overrides) { + merged.insert_or_assign(key, value); + } + return merged; +} + +/// Matches Java RESTSigV4AuthSession: canonical headers carry +/// Base64(SHA256(body)), canonical request trailer uses hex. +class RestSigV4Signer : public Aws::Client::AWSAuthV4Signer { + public: + RestSigV4Signer(const std::shared_ptr<Aws::Auth::AWSCredentialsProvider>& creds, + const char* service_name, const Aws::String& region) + : Aws::Client::AWSAuthV4Signer(creds, service_name, region, + PayloadSigningPolicy::Always, + /*urlEscapePath=*/false) { + // Skip the signer's hex overwrite of x-amz-content-sha256 so canonical + // headers see the caller's Base64; ComputePayloadHash still feeds hex + // into the canonical request trailer. + m_includeSha256HashHeader = false; + } +}; + +} // namespace + +// ---- SigV4AuthSession ---- + +SigV4AuthSession::SigV4AuthSession( + std::shared_ptr<AuthSession> delegate, std::string signing_region, + std::string signing_name, + std::shared_ptr<Aws::Auth::AWSCredentialsProvider> credentials_provider) + : delegate_(std::move(delegate)), + signing_region_(std::move(signing_region)), + signing_name_(std::move(signing_name)), + credentials_provider_(std::move(credentials_provider)), + signer_(std::make_unique<RestSigV4Signer>( + credentials_provider_, signing_name_.c_str(), signing_region_.c_str())) {} + +SigV4AuthSession::~SigV4AuthSession() { AwsSdkLifecycle::Instance().UnregisterSession(); } Review Comment: This is still not fixed. `SigV4AuthSession` still unregisters in the destructor, while direct construction does not register a slot. The tests still directly construct it, so the active session count can underflow. The registration ownership should be paired inside the session or the constructor should not be public. -- 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]
