Fix bad signature for PutObject, and subsequent GetObject after it

We were not setting up correclty the Payload x-amz header, we now do.
We also add a custom sorting function for our std::map of headers so that it always sorts them alphabetically.

Author: ggcr

src/s3cpp/auth.cpp

@@ -17,6 +17,16 @@ void AWSSigV4Signer::sign(HttpRequestBase<T>& request) {
     // Autorization
     const std::string hash_algo = "AWS4-HMAC-SHA256";
 
+    // Compute payload hash and set header ONLY for body requests
+    std::string payload_hash;
+    if constexpr (std::is_same_v<T, HttpBodyRequest>) {
+        const std::string& body = static_cast<HttpBodyRequest&>(request).getBody();
+        payload_hash = hex(sha256(body));
+        request.header("x-amz-content-sha256", payload_hash);
+    } else {
+        payload_hash = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855";
+    }
+
     // Compute date and add it as a header
     const std::string timestamp = this->getTimestamp();
     request.header("X-Amz-Date", timestamp);
@@ -39,7 +49,7 @@ void AWSSigV4Signer::sign(HttpRequestBase<T>& request) {
     }
 
     // Cannonical request
-    std::string hex_cannonical_request = hex(sha256(createCannonicalRequest(request)));
+    std::string hex_cannonical_request = hex(sha256(createCannonicalRequest(request, payload_hash)));
 
     // To sign
     std::string string_to_sign = std::format("{}\n{}\n{}\n{}", hash_algo, timestamp, credential_scope, hex_cannonical_request);
@@ -50,7 +60,7 @@ void AWSSigV4Signer::sign(HttpRequestBase<T>& request) {
 }
 
 template <typename T>
-std::string AWSSigV4Signer::createCannonicalRequest(HttpRequestBase<T>& request) {
+std::string AWSSigV4Signer::createCannonicalRequest(HttpRequestBase<T>& request, const std::string& payload_hash) {
     const std::string http_verb = request.getHttpMethodStr(request.getHttpMethod());
     std::string url = request.getURL();
 
@@ -93,34 +103,29 @@ std::string AWSSigV4Signer::createCannonicalRequest(HttpRequestBase<T>& request)
     }
 
     // Canonical Headers + SignedHeaders
-    const std::map<std::string, std::string> headers = request.getHeaders();
+    const std::map<std::string, std::string, LowerCaseCompare> headers = request.getHeaders();
     std::string cheaders = "";
     std::string signed_headers = "";
     uint_fast16_t i = 0;
     for (const auto& header : headers) {
         std::string kHeader = header.first;
-        std::transform(kHeader.begin(), kHeader.end(), kHeader.begin(),
-            ::tolower);
+        std::transform(kHeader.begin(), kHeader.end(), kHeader.begin(), ::tolower);
         if (i > 0)
             signed_headers += ";";
         signed_headers += kHeader;
         cheaders += kHeader + ":" + header.second + "\n";
         i++;
     }
 
-    std::string payload_hash;
-    if constexpr (std::is_same_v<T, HttpBodyRequest>) {
-        const std::string& body = static_cast<HttpBodyRequest&>(request).getBody();
-        payload_hash = hex(sha256(body));
-    } else {
-        payload_hash = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855";
-    }
-
-    // The x-amz-content-sha256 header is required for Amazon S3 AWS requests. It provides a hash of the request payload. If there is no payload, you must provide the hash of an empty string.
-    request.header("x-amz-content-sha256", payload_hash);
+    std::string canonical_request = std::format("{}\n{}\n{}\n{}\n{}\n{}",
+        http_verb,
+        cannonical_uri,
+        query_str,
+        cheaders,
+        signed_headers,
+        payload_hash);
 
-    return std::format("{}\n{}\n{}\n{}\n{}\n{}", http_verb, cannonical_uri, query_str,
-        cheaders, signed_headers, payload_hash);
+    return canonical_request;
 }
 
 const unsigned char* AWSSigV4Signer::sha256(const std::string& str) {
@@ -194,5 +199,5 @@ const unsigned char* AWSSigV4Signer::deriveSigningKey(const std::string request_
 // Why are we still here? Just to suffer?
 template void AWSSigV4Signer::sign<HttpRequest>(HttpRequestBase<HttpRequest>&);
 template void AWSSigV4Signer::sign<HttpBodyRequest>(HttpRequestBase<HttpBodyRequest>&);
-template std::string AWSSigV4Signer::createCannonicalRequest<HttpRequest>(HttpRequestBase<HttpRequest>&);
-template std::string AWSSigV4Signer::createCannonicalRequest<HttpBodyRequest>(HttpRequestBase<HttpBodyRequest>&);
+template std::string AWSSigV4Signer::createCannonicalRequest<HttpRequest>(HttpRequestBase<HttpRequest>&, const std::string&);
+template std::string AWSSigV4Signer::createCannonicalRequest<HttpBodyRequest>(HttpRequestBase<HttpBodyRequest>&, const std::string&);

src/s3cpp/auth.h

@@ -20,7 +20,7 @@ class AWSSigV4Signer {
     void sign(HttpRequestBase<T>& request);
 
     template <typename T>
-    std::string createCannonicalRequest(HttpRequestBase<T>& request);
+    std::string createCannonicalRequest(HttpRequestBase<T>& request, const std::string& payload_hash = "");
 
     const unsigned char* sha256(const std::string& str);
     const unsigned char* HMAC_SHA256(const unsigned char* key, size_t key_len, const std::string& data);

src/s3cpp/httpclient.cpp

@@ -38,7 +38,7 @@ HttpResponse HttpClient::execute_get(HttpRequest& request) {
             "cURL handle is invalid");
     }
     std::string body_buf;
-    std::map<std::string, std::string> headers_buf;
+    std::map<std::string, std::string, LowerCaseCompare> headers_buf;
     std::string error_buf;
 
     // TODO(cristian): from libcurl docs, they state that each curl handle has
@@ -48,6 +48,9 @@ HttpResponse HttpClient::execute_get(HttpRequest& request) {
     //
     // curl_easy_reset(curl_handle);
 
+    curl_easy_setopt(curl_handle, CURLOPT_HTTPGET, 1L);
+    curl_easy_setopt(curl_handle, CURLOPT_CUSTOMREQUEST, NULL);
+    curl_easy_setopt(curl_handle, CURLOPT_NOBODY, 0L);
     curl_easy_setopt(curl_handle, CURLOPT_URL, request.getURL().c_str());
     // body callback
     curl_easy_setopt(curl_handle, CURLOPT_WRITEFUNCTION, write_callback);
@@ -90,9 +93,10 @@ HttpResponse HttpClient::execute_head(HttpRequest& request) {
             // is invalidated in the HTTPClient copy constructor
             "cURL handle is invalid");
     }
-    std::map<std::string, std::string> headers_buf;
+    std::map<std::string, std::string, LowerCaseCompare> headers_buf;
     std::string error_buf;
 
+    curl_easy_setopt(curl_handle, CURLOPT_CUSTOMREQUEST, NULL);
     curl_easy_setopt(curl_handle, CURLOPT_URL, request.getURL().c_str());
     curl_easy_setopt(curl_handle, CURLOPT_HEADERFUNCTION, header_callback);
     curl_easy_setopt(curl_handle, CURLOPT_HEADERDATA, &headers_buf);
@@ -132,7 +136,7 @@ HttpResponse HttpClient::execute_post(HttpBodyRequest& request) {
             "cURL handle is invalid");
     }
     std::string body_buf;
-    std::map<std::string, std::string> headers_buf;
+    std::map<std::string, std::string, LowerCaseCompare> headers_buf;
     std::string error_buf;
 
     // TODO(cristian): from libcurl docs, they state that each curl handle has
@@ -142,6 +146,7 @@ HttpResponse HttpClient::execute_post(HttpBodyRequest& request) {
     //
     // curl_easy_reset(curl_handle);
 
+    curl_easy_setopt(curl_handle, CURLOPT_NOBODY, 0L);
     curl_easy_setopt(curl_handle, CURLOPT_URL, request.getURL().c_str());
     // body callback
     curl_easy_setopt(curl_handle, CURLOPT_WRITEFUNCTION, write_callback);
@@ -193,7 +198,7 @@ HttpResponse HttpClient::execute_delete(HttpBodyRequest& request) {
             "cURL handle is invalid");
     }
     std::string body_buf;
-    std::map<std::string, std::string> headers_buf;
+    std::map<std::string, std::string, LowerCaseCompare> headers_buf;
     std::string error_buf;
 
     // TODO(cristian): from libcurl docs, they state that each curl handle has

src/s3cpp/httpclient.h

@@ -21,25 +21,35 @@ enum class HttpMethod {
     Delete
 };
 
+struct LowerCaseCompare { // A custom lambda to sort keys alphabetically
+    bool operator()(const std::string& a, const std::string& b) const {
+        std::string sa = a;
+        std::string sb = b;
+        std::transform(sa.begin(), sa.end(), sa.begin(), ::tolower);
+        std::transform(sb.begin(), sb.end(), sb.begin(), ::tolower);
+        return sa < sb;
+    }
+};
+
 class HttpResponse {
 public:
     HttpResponse(int c)
         : code_(c) { };
     HttpResponse(int c, std::string b)
         : code_(c)
         , body_(std::move(b)) { };
-    HttpResponse(int c, std::map<std::string, std::string> h)
+    HttpResponse(int c, std::map<std::string, std::string, LowerCaseCompare> h)
         : code_(c)
         , headers_(std::move(h)) { };
-    HttpResponse(int c, std::string b, std::map<std::string, std::string> h)
+    HttpResponse(int c, std::string b, std::map<std::string, std::string, LowerCaseCompare> h)
         : code_(c)
         , body_(std::move(b))
         , headers_(std::move(h)) { };
 
     // Getters
     int status() const { return code_; }
     const std::string& body() const { return body_; }
-    const std::map<std::string, std::string>& headers() const { return headers_; }
+    const auto& headers() const { return headers_; }
 
     // Status via code
     bool is_ok() const { return code_ >= 200 && code_ < 300; }
@@ -54,7 +64,7 @@ class HttpResponse {
 private:
     int code_;
     std::string body_;
-    std::map<std::string, std::string> headers_;
+    std::map<std::string, std::string, LowerCaseCompare> headers_;
 };
 
 // HttpRequest will handle all the headers and request params
@@ -93,11 +103,11 @@ class HttpRequestBase {
     const std::string& getURL() const { return URL_; }
     const HttpMethod& getHttpMethod() const { return http_method_; }
     const long long getTimeout() const { return timeout_.count(); }
-    const std::map<std::string, std::string>& getHeaders() const {
+    const std::map<std::string, std::string, LowerCaseCompare>& getHeaders() const {
         return headers_;
     }
 
-		// Cannonicalize HTTP verb from the request
+    // Cannonicalize HTTP verb from the request
     const std::string getHttpMethodStr(const HttpMethod& http_method) const {
         switch (http_method) {
         case HttpMethod::Get:
@@ -115,12 +125,10 @@ class HttpRequestBase {
         }
     }
 
-
-
 protected:
     HttpClient& client_;
     std::string URL_;
-    std::map<std::string, std::string> headers_;
+    std::map<std::string, std::string, LowerCaseCompare> headers_;
     std::chrono::seconds timeout_;
     HttpMethod http_method_;
 };