From 12a88fcac9bc81e86c5e3d32f919d650d82544b9 Mon Sep 17 00:00:00 2001 From: Abhijat Malviya Date: Thu, 19 Oct 2023 14:27:17 +0530 Subject: [PATCH 1/3] http: Move redacted fields to a public method This allows other units of code to call it. (cherry picked from commit 2d7dcf47dabe29a4a0c75e995cd59e774500ce96) --- src/v/http/client.cc | 19 +++++++++---------- src/v/http/client.h | 3 +++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/v/http/client.cc b/src/v/http/client.cc index 29c4f61f403c..855847f07964 100644 --- a/src/v/http/client.cc +++ b/src/v/http/client.cc @@ -39,17 +39,16 @@ #include #include -namespace { -using field_type = std::variant; - -const std::unordered_set redacted_fields{ - boost::beast::http::field::authorization, - "x-amz-content-sha256", - "x-amz-security-token"}; -} // namespace - namespace http { +const std::unordered_set> +redacted_fields() { + return { + boost::beast::http::field::authorization, + "x-amz-content-sha256", + "x-amz-security-token"}; +} + // client implementation // static constexpr ss::lowres_clock::duration default_max_idle_time = 1s; @@ -627,7 +626,7 @@ ss::input_stream client::response_stream::as_input_stream() { client::request_header redacted_header(client::request_header original) { auto h{std::move(original)}; - for (const auto& field : redacted_fields) { + for (const auto& field : redacted_fields()) { std::visit( [&h](const auto& f) { if (h.find(f) != h.end()) { diff --git a/src/v/http/client.h b/src/v/http/client.h index 5d162bb46dd3..15f5f2049def 100644 --- a/src/v/http/client.h +++ b/src/v/http/client.h @@ -267,6 +267,9 @@ auto with_client(client&& cl, Func func) { }); } +const std::unordered_set> +redacted_fields(); + } // namespace http template<> From 55aa789ee6ab9e896908f1a6ae39edcb28db21df Mon Sep 17 00:00:00 2001 From: Abhijat Malviya Date: Thu, 19 Oct 2023 14:28:16 +0530 Subject: [PATCH 2/3] cloud_roles: Redact fields when printing request When printing a request the fields inside the string can leak secrets. A replacement is done inside the string for sensitive headers. (cherry picked from commit 044ec1e798904f370d35b88ceae4a10e827dd1f4) --- src/v/cloud_roles/signature.cc | 39 ++++++++++++++++++++++- src/v/cloud_roles/signature.h | 2 ++ src/v/cloud_roles/tests/signature_test.cc | 26 +++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/v/cloud_roles/signature.cc b/src/v/cloud_roles/signature.cc index 48224a46ba6d..1911d87668fb 100644 --- a/src/v/cloud_roles/signature.cc +++ b/src/v/cloud_roles/signature.cc @@ -12,6 +12,7 @@ #include "bytes/bytes.h" #include "cloud_roles/logger.h" +#include "config/base_property.h" #include "hashing/secure.h" #include "ssx/sformat.h" #include "utils/base64.h" @@ -20,6 +21,8 @@ #include #include +#include +#include #include #include #include @@ -372,6 +375,33 @@ ss::sstring signature_v4::sha256_hexdigest(std::string_view payload) { return sha_256(payload); } +ss::sstring redact_headers_from_string(const std::string_view original) { + std::set redacted{}; + const auto redacted_fields = http::redacted_fields(); + for (const auto& rf : redacted_fields) { + redacted.insert(ss::visit( + rf, + [](const boost::beast::http::field& f) { return to_string(f); }, + [](const std::string& s) { return boost::core::string_view{s}; })); + } + + const auto lines = absl::StrSplit(original, "\n"); + std::vector result{}; + for (const auto& line : lines) { + if (line.find(':') != std::string_view::npos) { + const auto tokens = absl::StrSplit(line, ":"); + const auto key = *tokens.begin(); + if (redacted.contains(key)) { + result.emplace_back(fmt::format( + "{}:{}", *tokens.begin(), config::secret_placeholder)); + continue; + } + } + result.emplace_back(line.data(), line.size()); + } + return absl::StrJoin(result, "\n"); +} + std::error_code signature_v4::sign_header( http::client::request_header& header, std::string_view sha256) const { ss::sstring date_str = _sig_time.format_date(); @@ -392,7 +422,14 @@ std::error_code signature_v4::sign_header( if (!canonical_req) { return canonical_req.error(); } - vlog(clrl_log.trace, "\n[canonical-request]\n{}\n", canonical_req.value()); + + if (clrl_log.is_enabled(seastar::log_level::trace)) { + vlog( + clrl_log.trace, + "\n[canonical-request]\n{}\n", + redact_headers_from_string(canonical_req.value())); + } + auto str_to_sign = get_string_to_sign( amz_date, cred_scope, canonical_req.value()); auto digest = hmac(sign_key, str_to_sign); diff --git a/src/v/cloud_roles/signature.h b/src/v/cloud_roles/signature.h index c0998a0f4bab..8d570d8cb271 100644 --- a/src/v/cloud_roles/signature.h +++ b/src/v/cloud_roles/signature.h @@ -180,4 +180,6 @@ time_source::time_source(Fn&& fn, int) ss::sstring uri_encode(const ss::sstring& input, bool encode_slash); +ss::sstring redact_headers_from_string(const std::string_view original); + } // namespace cloud_roles diff --git a/src/v/cloud_roles/tests/signature_test.cc b/src/v/cloud_roles/tests/signature_test.cc index db1734646ca8..fe53ce9fc167 100644 --- a/src/v/cloud_roles/tests/signature_test.cc +++ b/src/v/cloud_roles/tests/signature_test.cc @@ -14,6 +14,8 @@ #include #include +#include +#include #include #include @@ -237,3 +239,27 @@ SEASTAR_THREAD_TEST_CASE(test_gnutls) { to_hex(bytes_view{result.data(), 32}), "c4afb1cc5771d871763a393e44b703571b55cc28424d1a5e86da6ed3c154a4b9"); } + +SEASTAR_THREAD_TEST_CASE(test_redact_headers_from_string) { + std::vector lines{}; + lines.emplace_back(fmt::format("{}:{}", "x-amz-security-token", "abcd")); + lines.emplace_back(fmt::format("{}:{}", "x-amz-content-sha256", "secret")); + lines.emplace_back( + fmt::format("{}:{}", "x-amz-security-token111", "abcd111")); + lines.emplace_back( + fmt::format("{}:{}", "x-amz-security-token222", "abcd222")); + lines.emplace_back( + fmt::format("{}:{}", "x-amz-security-token", "abcdabcd")); + + const std::string redacted = cloud_roles::redact_headers_from_string( + absl::StrJoin(lines, "\n")); + + const auto redacted_lines = absl::StrSplit(redacted, "\n"); + auto it = redacted_lines.begin(); + + BOOST_REQUIRE_EQUAL(*(it++), "x-amz-security-token:[secret]"); + BOOST_REQUIRE_EQUAL(*(it++), "x-amz-content-sha256:[secret]"); + BOOST_REQUIRE_EQUAL(*(it++), "x-amz-security-token111:abcd111"); + BOOST_REQUIRE_EQUAL(*(it++), "x-amz-security-token222:abcd222"); + BOOST_REQUIRE_EQUAL(*(it++), "x-amz-security-token:[secret]"); +} From 25f55ee4bfcaca968c70f93f272f6cce0d61ee49 Mon Sep 17 00:00:00 2001 From: Abhijat Malviya Date: Fri, 20 Oct 2023 20:24:14 +0530 Subject: [PATCH 3/3] cloud_roles: replace boost::string_view with std::string_view The boost::core::string_view returned by boost::beast helpers seems to break OSS build. --- src/v/cloud_roles/signature.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/v/cloud_roles/signature.cc b/src/v/cloud_roles/signature.cc index 1911d87668fb..312e6787a76b 100644 --- a/src/v/cloud_roles/signature.cc +++ b/src/v/cloud_roles/signature.cc @@ -376,13 +376,16 @@ ss::sstring signature_v4::sha256_hexdigest(std::string_view payload) { } ss::sstring redact_headers_from_string(const std::string_view original) { - std::set redacted{}; + std::set redacted{}; const auto redacted_fields = http::redacted_fields(); for (const auto& rf : redacted_fields) { redacted.insert(ss::visit( rf, - [](const boost::beast::http::field& f) { return to_string(f); }, - [](const std::string& s) { return boost::core::string_view{s}; })); + [](const boost::beast::http::field& f) { + const auto view = to_string(f); + return std::string_view{view.data(), view.size()}; + }, + [](const std::string& s) { return std::string_view{s}; })); } const auto lines = absl::StrSplit(original, "\n");