Got rid of S3AuthSigner

src/IO/S3/PocoHTTPClient.cpp

@@ -5,18 +5,20 @@
 #include "PocoHTTPClient.h"
 
 #include <utility>
+
+#include <Common/logger_useful.h>
+#include <Common/Stopwatch.h>
 #include <IO/HTTPCommon.h>
 #include <IO/WriteBufferFromString.h>
 #include <IO/Operators.h>
-#include <Common/Stopwatch.h>
+
 #include <aws/core/http/HttpRequest.h>
 #include <aws/core/http/HttpResponse.h>
 #include <aws/core/monitoring/HttpClientMetrics.h>
 #include <aws/core/utils/ratelimiter/RateLimiterInterface.h>
 #include "Poco/StreamCopier.h"
 #include <Poco/Net/HTTPRequest.h>
 #include <Poco/Net/HTTPResponse.h>
-#include <Common/logger_useful.h>
 #include <re2/re2.h>
 
 #include <boost/algorithm/string.hpp>
@@ -105,6 +107,7 @@ PocoHTTPClient::PocoHTTPClient(const PocoHTTPClientConfiguration & client_config
     , remote_host_filter(client_configuration.remote_host_filter)
     , s3_max_redirects(client_configuration.s3_max_redirects)
     , enable_s3_requests_logging(client_configuration.enable_s3_requests_logging)
+    , extra_headers(client_configuration.extra_headers)
 {
 }
 
@@ -206,7 +209,7 @@ void PocoHTTPClient::makeRequestInternal(
               * Effectively, `Poco::URI` chooses smaller subset of characters to encode,
               * whereas Amazon S3 and Google Cloud Storage expects another one.
               * In order to successfully execute a request, a path must be exact representation
-              * of decoded path used by `S3AuthSigner`.
+              * of decoded path used by `AWSAuthSigner`.
               * Therefore we shall encode some symbols "manually" to fit the signatures.
               */
 
@@ -243,8 +246,11 @@ void PocoHTTPClient::makeRequestInternal(
                     break;
             }
 
+            /// Headers coming from SDK are lower-cased.
             for (const auto & [header_name, header_value] : request.GetHeaders())
                 poco_request.set(header_name, header_value);
+            for (const auto & [header_name, header_value] : extra_headers)
+                poco_request.set(boost::algorithm::to_lower_copy(header_name), header_value);
 
             Poco::Net::HTTPResponse poco_response;
 

src/IO/S3/PocoHTTPClient.h

@@ -8,11 +8,14 @@
 #include <IO/ConnectionTimeouts.h>
 #include <IO/HTTPCommon.h>
 #include <IO/S3/SessionAwareIOStream.h>
+#include <Storages/StorageS3Settings.h>
+
 #include <aws/core/client/ClientConfiguration.h>
 #include <aws/core/http/HttpClient.h>
 #include <aws/core/http/HttpRequest.h>
 #include <aws/core/http/standard/StandardHttpResponse.h>
 
+
 namespace Aws::Http::Standard
 {
 class StandardHttpResponse;
@@ -41,6 +44,7 @@ struct PocoHTTPClientConfiguration : public Aws::Client::ClientConfiguration
     const RemoteHostFilter & remote_host_filter;
     unsigned int s3_max_redirects;
     bool enable_s3_requests_logging;
+    HeaderCollection extra_headers;
 
     void updateSchemeAndRegion();
 
@@ -114,6 +118,7 @@ class PocoHTTPClient : public Aws::Http::HttpClient
     const RemoteHostFilter & remote_host_filter;
     unsigned int s3_max_redirects;
     bool enable_s3_requests_logging;
+    const HeaderCollection extra_headers;
 };
 
 }

src/IO/S3/tests/TestPocoHTTPServer.h

@@ -0,0 +1,53 @@
+#pragma once
+
+#include <cstddef>
+#include <memory>
+#include <string>
+
+#include <Poco/Net/HTTPRequestHandler.h>
+#include <Poco/Net/HTTPRequestHandlerFactory.h>
+#include <Poco/Net/HTTPServer.h>
+#include <Poco/Net/HTTPServerParams.h>
+#include <Poco/Net/NetException.h>
+#include <Poco/Net/ServerSocket.h>
+#include <Poco/AutoPtr.h>
+#include <Poco/SharedPtr.h>
+
+
+template <typename RequestHandler>
+class HTTPRequestHandlerFactory : public Poco::Net::HTTPRequestHandlerFactory
+{
+    virtual Poco::Net::HTTPRequestHandler * createRequestHandler(const Poco::Net::HTTPServerRequest &) override
+    {
+        return new RequestHandler();
+    }
+
+public:
+    virtual ~HTTPRequestHandlerFactory() override
+    {
+    }
+};
+
+template <typename RequestHandler>
+class TestPocoHTTPServer
+{
+    std::unique_ptr<Poco::Net::ServerSocket> server_socket;
+    Poco::SharedPtr<HTTPRequestHandlerFactory<RequestHandler>> handler_factory;
+    Poco::AutoPtr<Poco::Net::HTTPServerParams> server_params;
+    std::unique_ptr<Poco::Net::HTTPServer> server;
+
+public:
+    TestPocoHTTPServer():
+        server_socket(std::make_unique<Poco::Net::ServerSocket>(0)),
+        handler_factory(new HTTPRequestHandlerFactory<RequestHandler>()),
+        server_params(new Poco::Net::HTTPServerParams()),
+        server(std::make_unique<Poco::Net::HTTPServer>(handler_factory, *server_socket, server_params))
+    {
+        server->start();
+    }
+
+    std::string getUrl()
+    {
+        return "http://" + server_socket->address().toString();
+    }
+};

src/IO/S3/tests/gtest_aws_s3_client.cpp

@@ -0,0 +1,129 @@
+#include <gtest/gtest.h>
+
+#include <Common/config.h>
+
+
+#if USE_AWS_S3
+
+#include <memory>
+#include <ostream>
+
+#include <boost/algorithm/string.hpp>
+
+#include <Poco/Net/HTTPRequestHandler.h>
+#include <Poco/Net/HTTPServerRequest.h>
+#include <Poco/Net/HTTPServerResponse.h>
+#include <Poco/URI.h>
+
+#include <aws/core/client/AWSError.h>
+#include <aws/core/client/CoreErrors.h>
+#include <aws/core/client/RetryStrategy.h>
+#include <aws/core/http/URI.h>
+#include <aws/s3/S3Client.h>
+
+#include <Common/RemoteHostFilter.h>
+#include <IO/ReadBufferFromS3.h>
+#include <IO/ReadHelpers.h>
+#include <IO/ReadSettings.h>
+#include <IO/S3Common.h>
+#include <Storages/StorageS3Settings.h>
+
+#include "TestPocoHTTPServer.h"
+
+
+class NoRetryStrategy : public Aws::Client::StandardRetryStrategy
+{
+    bool ShouldRetry(const Aws::Client::AWSError<Aws::Client::CoreErrors> &, long /* NOLINT */) const override { return false; }
+
+public:
+    ~NoRetryStrategy() override = default;
+};
+
+
+TEST(IOTestAwsS3Client, AppendExtraSSECHeaders)
+{
+    /// See https://github.com/ClickHouse/ClickHouse/pull/19748
+
+    class MyRequestHandler : public Poco::Net::HTTPRequestHandler
+    {
+    public:
+        void handleRequest(Poco::Net::HTTPServerRequest & request, Poco::Net::HTTPServerResponse & response) override
+        {
+            response.setStatus(Poco::Net::HTTPResponse::HTTP_OK);
+            std::ostream & out = response.send();
+            for (const auto & [header_name, header_value] : request)
+            {
+                if (boost::algorithm::starts_with(header_name, "x-amz-server-side-encryption-customer-"))
+                {
+                    out << header_name << ": " << header_value << "\n";
+                }
+                else if (header_name == "authorization")
+                {
+                    std::vector<String> parts;
+                    boost::split(parts, header_value, [](char c){ return c == ' '; });
+                    for (const auto & part : parts)
+                    {
+                        if (boost::algorithm::starts_with(part, "SignedHeaders="))
+                            out << header_name << ": ... " << part << " ...\n";
+                    }
+                }
+            }
+            out.flush();
+        }
+    };
+
+    TestPocoHTTPServer<MyRequestHandler> http;
+
+    DB::RemoteHostFilter remote_host_filter;
+    unsigned int s3_max_redirects = 100;
+    DB::S3::URI uri(Poco::URI(http.getUrl() + "/IOTestAwsS3ClientAppendExtraHeaders/test.txt"));
+    String access_key_id = "ACCESS_KEY_ID";
+    String secret_access_key = "SECRET_ACCESS_KEY";
+    String region = "us-east-1";
+    String version_id;
+    UInt64 max_single_read_retries = 1;
+    bool enable_s3_requests_logging = false;
+    DB::S3::PocoHTTPClientConfiguration client_configuration = DB::S3::ClientFactory::instance().createClientConfiguration(
+        region,
+        remote_host_filter,
+        s3_max_redirects,
+        enable_s3_requests_logging
+    );
+
+    client_configuration.endpointOverride = uri.endpoint;
+    client_configuration.retryStrategy = std::make_shared<NoRetryStrategy>();
+
+    String server_side_encryption_customer_key_base64 = "Kv/gDqdWVGIT4iDqg+btQvV3lc1idlm4WI+MMOyHOAw=";
+    DB::HeaderCollection headers;
+    bool use_environment_credentials = false;
+    bool use_insecure_imds_request = false;
+
+    std::shared_ptr<Aws::S3::S3Client> client = DB::S3::ClientFactory::instance().create(
+        client_configuration,
+        uri.is_virtual_hosted_style,
+        access_key_id,
+        secret_access_key,
+        server_side_encryption_customer_key_base64,
+        headers,
+        use_environment_credentials,
+        use_insecure_imds_request
+    );
+
+    ASSERT_TRUE(client);
+
+    DB::ReadSettings read_settings;
+    DB::ReadBufferFromS3 read_buffer(
+        client,
+        uri.bucket,
+        uri.key,
+        version_id,
+        max_single_read_retries,
+        read_settings
+    );
+
+    String content;
+    DB::readStringUntilEOF(content, read_buffer);
+    EXPECT_EQ(content, "authorization: ... SignedHeaders=amz-sdk-invocation-id;amz-sdk-request;content-type;host;x-amz-api-version;x-amz-content-sha256;x-amz-date, ...\nx-amz-server-side-encryption-customer-algorithm: AES256\nx-amz-server-side-encryption-customer-key: Kv/gDqdWVGIT4iDqg+btQvV3lc1idlm4WI+MMOyHOAw=\nx-amz-server-side-encryption-customer-key-md5: fMNuOw6OLU5GG2vc6RTA+g==\n");
+}
+
+#endif

src/IO/S3Common.cpp

@@ -617,72 +617,6 @@ class S3CredentialsProviderChain : public Aws::Auth::AWSCredentialsProviderChain
     }
 };
 
-class S3AuthSigner : public Aws::Client::AWSAuthV4Signer
-{
-public:
-    S3AuthSigner(
-        const Aws::Client::ClientConfiguration & client_configuration,
-        const Aws::Auth::AWSCredentials & credentials,
-        const DB::HeaderCollection & headers_,
-        bool use_environment_credentials,
-        bool use_insecure_imds_request)
-        : Aws::Client::AWSAuthV4Signer(
-            std::make_shared<S3CredentialsProviderChain>(
-                static_cast<const DB::S3::PocoHTTPClientConfiguration &>(client_configuration),
-                credentials,
-                use_environment_credentials,
-                use_insecure_imds_request),
-            "s3",
-            client_configuration.region,
-            Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never,
-            false)
-        , headers(headers_)
-    {
-    }
-
-    bool SignRequest(Aws::Http::HttpRequest & request, const char * region, bool sign_body) const override
-    {
-        auto result = Aws::Client::AWSAuthV4Signer::SignRequest(request, region, sign_body);
-        for (const auto & header : headers)
-            request.SetHeaderValue(header.name, header.value);
-        return result;
-    }
-
-    bool SignRequest(Aws::Http::HttpRequest & request, const char * region, const char * service_name, bool sign_body) const override
-    {
-        auto result = Aws::Client::AWSAuthV4Signer::SignRequest(request, region, service_name, sign_body);
-        for (const auto & header : headers)
-            request.SetHeaderValue(header.name, header.value);
-        return result;
-    }
-
-    bool PresignRequest(
-        Aws::Http::HttpRequest & request,
-        const char * region,
-        long long expiration_time_sec) const override // NOLINT
-    {
-        auto result = Aws::Client::AWSAuthV4Signer::PresignRequest(request, region, expiration_time_sec);
-        for (const auto & header : headers)
-            request.SetHeaderValue(header.name, header.value);
-        return result;
-    }
-
-    bool PresignRequest(
-        Aws::Http::HttpRequest & request,
-        const char * region,
-        const char * service_name,
-        long long expiration_time_sec) const override // NOLINT
-    {
-        auto result = Aws::Client::AWSAuthV4Signer::PresignRequest(request, region, service_name, expiration_time_sec);
-        for (const auto & header : headers)
-            request.SetHeaderValue(header.name, header.value);
-        return result;
-    }
-
-private:
-    const DB::HeaderCollection headers;
-};
-
 }
 
 
@@ -729,8 +663,6 @@ namespace S3
         PocoHTTPClientConfiguration client_configuration = cfg_;
         client_configuration.updateSchemeAndRegion();
 
-        Aws::Auth::AWSCredentials credentials(access_key_id, secret_access_key);
-
         if (!server_side_encryption_customer_key_base64.empty())
         {
             /// See S3Client::GeneratePresignedUrlWithSSEC().
@@ -747,17 +679,20 @@ namespace S3
                 Aws::Utils::HashingUtils::Base64Encode(Aws::Utils::HashingUtils::CalculateMD5(str_buffer))});
         }
 
-        auto auth_signer = std::make_shared<S3AuthSigner>(
-            client_configuration,
-            std::move(credentials),
-            std::move(headers),
-            use_environment_credentials,
-            use_insecure_imds_request);
+        client_configuration.extra_headers = std::move(headers);
+
+        Aws::Auth::AWSCredentials credentials(access_key_id, secret_access_key);
+        auto credentials_provider = std::make_shared<S3CredentialsProviderChain>(
+                client_configuration,
+                std::move(credentials),
+                use_environment_credentials,
+                use_insecure_imds_request);
 
         return std::make_unique<Aws::S3::S3Client>(
-            std::move(auth_signer),
+            std::move(credentials_provider),
             std::move(client_configuration), // Client configuration.
-            is_virtual_hosted_style || client_configuration.endpointOverride.empty() // Use virtual addressing only if endpoint is not specified.
+            Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never,
+            is_virtual_hosted_style || client_configuration.endpointOverride.empty() /// Use virtual addressing if endpoint is not specified.
         );
     }