From 4cb131b3a7b34b34f5225c29aec5e3d490e7dc65 Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Wed, 24 May 2023 09:29:01 +0000 Subject: [PATCH 1/4] debug: redact sensitive info from debug logs Redact potentially sensitive information from debug logs by masking - query string - authorization header value - cookie values Signed-off-by: Tero Saarni --- source/common/http/conn_manager_impl.cc | 2 +- source/common/http/utility.cc | 100 ++++++++++++++++------ source/common/http/utility.h | 43 ++++++++++ source/common/router/router.cc | 3 +- source/common/runtime/runtime_features.cc | 1 + 5 files changed, 122 insertions(+), 27 deletions(-) diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 80995aa8fd1a6..7223712eff442 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -1066,7 +1066,7 @@ void ConnectionManagerImpl::ActiveStream::maybeEndDecode(bool end_stream) { void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPtr&& headers, bool end_stream) { ENVOY_STREAM_LOG(debug, "request headers complete (end_stream={}):\n{}", *this, end_stream, - *headers); + Http::Utility::RedactSensitiveHeaderInfo(*headers)); ScopeTrackerScopeState scope(this, connection_manager_.read_callbacks_->connection().dispatcher()); request_headers_ = std::move(headers); diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index b805297d5e141..9df78db5b6108 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -317,37 +317,41 @@ bool maybeAdjustForIpv6(absl::string_view absolute_url, uint64_t& offset, uint64 return true; } +void forEachCookie( + absl::string_view cookie_header_value, + const std::function& cookie_consumer) { + // Split the cookie header into individual cookies. + for (const auto& s : StringUtil::splitToken(cookie_header_value, ";")) { + // Find the key part of the cookie (i.e. the name of the cookie). + size_t first_non_space = s.find_first_not_of(' '); + size_t equals_index = s.find('='); + if (equals_index == absl::string_view::npos) { + // The cookie is malformed if it does not have an `=`. Continue + // checking other cookies in this header. + continue; + } + absl::string_view k = s.substr(first_non_space, equals_index - first_non_space); + absl::string_view v = s.substr(equals_index + 1, s.size() - 1); + + // Cookie values may be wrapped in double quotes. + // https://tools.ietf.org/html/rfc6265#section-4.1.1 + if (v.size() >= 2 && v.back() == '"' && v[0] == '"') { + v = v.substr(1, v.size() - 2); + } + + if (!cookie_consumer(k, v)) { + return; + } + } +} + void forEachCookie( const HeaderMap& headers, const LowerCaseString& cookie_header, const std::function& cookie_consumer) { const Http::HeaderMap::GetResult cookie_headers = headers.get(cookie_header); for (size_t index = 0; index < cookie_headers.size(); index++) { - auto cookie_header_value = cookie_headers[index]->value().getStringView(); - - // Split the cookie header into individual cookies. - for (const auto& s : StringUtil::splitToken(cookie_header_value, ";")) { - // Find the key part of the cookie (i.e. the name of the cookie). - size_t first_non_space = s.find_first_not_of(' '); - size_t equals_index = s.find('='); - if (equals_index == absl::string_view::npos) { - // The cookie is malformed if it does not have an `=`. Continue - // checking other cookies in this header. - continue; - } - absl::string_view k = s.substr(first_non_space, equals_index - first_non_space); - absl::string_view v = s.substr(equals_index + 1, s.size() - 1); - - // Cookie values may be wrapped in double quotes. - // https://tools.ietf.org/html/rfc6265#section-4.1.1 - if (v.size() >= 2 && v.back() == '"' && v[0] == '"') { - v = v.substr(1, v.size() - 2); - } - - if (!cookie_consumer(k, v)) { - return; - } - } + forEachCookie(cookie_headers[index]->value().getStringView(), cookie_consumer); } } @@ -1494,5 +1498,51 @@ bool Utility::isValidRefererValue(absl::string_view value) { return true; } +void Utility::RedactSensitiveHeaderInfo::dumpState(std::ostream& os) const { + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.debug_redact_sensitive")) { + headers_.iterate([&os](const HeaderEntry& header) -> HeaderMap::Iterate { + os << "'" << header.key().getStringView() << "', '"; + if (header.key() == Http::Headers::get().Path) { + handlePath(os, header.value()); + } else if (header.key() == Http::Headers::get().Cookie) { + handleCookie(os, header.value()); + } else if (header.key() == Http::CustomHeaders::get().Authorization) { + os << "[redacted]"; + } else { + os << header.value().getStringView(); + } + os << "'\n"; + return HeaderMap::Iterate::Continue; + }); + } else { + os << headers_; + } +} + +void Utility::RedactSensitiveHeaderInfo::handlePath(std::ostream& os, const HeaderString& value) { + std::string stripped = Utility::stripQueryString(value); + if (stripped != value.getStringView()) { + os << stripped << "?[redacted]"; + } else { + os << value.getStringView(); + } +} + +void Utility::RedactSensitiveHeaderInfo::handleCookie(std::ostream& os, const HeaderString& value) { + bool first = true; + forEachCookie(value.getStringView(), + [&os, &first](absl::string_view k, absl::string_view) -> bool { + if (!first) { + os << ";"; + } + first = false; + os << k << "=" + << "[redacted]"; + + // continue iterating until all cookies are processed. + return true; + }); +} + } // namespace Http } // namespace Envoy diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 76a708b773cac..f3c7d44d70834 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -751,6 +751,49 @@ struct RedirectConfig { std::string newUri(::Envoy::OptRef redirect_config, const Http::RequestHeaderMap& headers); +/** + * Helper class for printing Http::HeaderMap while redacting sensitive information. + */ +class RedactSensitiveHeaderInfo { +public: + RedactSensitiveHeaderInfo(const Http::HeaderMap& headers) : headers_(headers) {} + friend std::ostream& operator<<(std::ostream& os, const RedactSensitiveHeaderInfo& redactor) { + redactor.dumpState(os); + return os; + } + +private: + /** + * Print key/value pairs. + * @param os supplies the ostream to print to. + * @param headers the headers to print. + */ + void dumpState(std::ostream& os) const; + + /** + * Redact query string from request path. + * @param os supplies the ostream to print to. + * @param value the value part of the pat. + */ + static void handlePath(std::ostream& os, const HeaderString& value); + + /** + * Redact cookie values from Cookie header. + * @param os supplies the ostream to print to. + * @param value value of the cookie header. + */ + static void handleCookie(std::ostream& os, const HeaderString& value); + + const Http::HeaderMap& headers_; +}; + } // namespace Utility } // namespace Http } // namespace Envoy + +// NOLINT(namespace-envoy) +namespace fmt { +// Allow fmtlib to format RedactSensitiveHeaderInfo +template <> +struct formatter : ostream_formatter {}; +} // namespace fmt diff --git a/source/common/router/router.cc b/source/common/router/router.cc index b03ea8b89a5aa..63a47740be143 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -687,7 +687,8 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } } - ENVOY_STREAM_LOG(debug, "router decoding headers:\n{}", *callbacks_, headers); + ENVOY_STREAM_LOG(debug, "router decoding headers:\n{}", *callbacks_, + Http::Utility::RedactSensitiveHeaderInfo(headers)); // Hang onto the modify_headers function for later use in handling upstream responses. modify_headers_ = modify_headers; diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 360998402fa38..f01155fa6b314 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -34,6 +34,7 @@ RUNTIME_GUARD(envoy_reloadable_features_append_query_parameters_path_rewriter); RUNTIME_GUARD(envoy_reloadable_features_append_xfh_idempotent); RUNTIME_GUARD(envoy_reloadable_features_conn_pool_delete_when_idle); RUNTIME_GUARD(envoy_reloadable_features_count_unused_mapped_pages_as_free); +RUNTIME_GUARD(envoy_reloadable_features_debug_redact_sensitive); RUNTIME_GUARD(envoy_reloadable_features_dfp_mixed_scheme); RUNTIME_GUARD(envoy_reloadable_features_enable_aws_credentials_file); RUNTIME_GUARD(envoy_reloadable_features_enable_compression_bomb_protection); From ebc501a839bfaf8f4127d658c901d6fe754c5e12 Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Fri, 26 May 2023 16:12:02 +0300 Subject: [PATCH 2/4] debug: negated the meaning of feature flag Signed-off-by: Tero Saarni --- source/common/http/utility.cc | 6 +++--- source/common/runtime/runtime_features.cc | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 9df78db5b6108..759def556d582 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -1499,7 +1499,9 @@ bool Utility::isValidRefererValue(absl::string_view value) { } void Utility::RedactSensitiveHeaderInfo::dumpState(std::ostream& os) const { - if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.debug_redact_sensitive")) { + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.debug_include_sensitive_data")) { + os << headers_; + } else { headers_.iterate([&os](const HeaderEntry& header) -> HeaderMap::Iterate { os << "'" << header.key().getStringView() << "', '"; if (header.key() == Http::Headers::get().Path) { @@ -1514,8 +1516,6 @@ void Utility::RedactSensitiveHeaderInfo::dumpState(std::ostream& os) const { os << "'\n"; return HeaderMap::Iterate::Continue; }); - } else { - os << headers_; } } diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index f01155fa6b314..fea59c12aee42 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -34,7 +34,7 @@ RUNTIME_GUARD(envoy_reloadable_features_append_query_parameters_path_rewriter); RUNTIME_GUARD(envoy_reloadable_features_append_xfh_idempotent); RUNTIME_GUARD(envoy_reloadable_features_conn_pool_delete_when_idle); RUNTIME_GUARD(envoy_reloadable_features_count_unused_mapped_pages_as_free); -RUNTIME_GUARD(envoy_reloadable_features_debug_redact_sensitive); +RUNTIME_GUARD(envoy_reloadable_features_debug_include_sensitive_data); RUNTIME_GUARD(envoy_reloadable_features_dfp_mixed_scheme); RUNTIME_GUARD(envoy_reloadable_features_enable_aws_credentials_file); RUNTIME_GUARD(envoy_reloadable_features_enable_compression_bomb_protection); From db53148e6d86b801cf9bfbf03d96f933788b6803 Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Mon, 29 May 2023 12:34:51 +0300 Subject: [PATCH 3/4] debug: move HeaderMap dumpState to Utilities Call Http::Utilities for dumping HeaderMap, to make it possible to redact potentially sensitive headers. Signed-off-by: Tero Saarni --- source/common/http/conn_manager_impl.cc | 2 +- source/common/http/header_map_impl.cc | 12 +++--- source/common/http/utility.cc | 53 ++++++++++++++----------- source/common/http/utility.h | 45 +++------------------ source/common/router/router.cc | 3 +- 5 files changed, 43 insertions(+), 72 deletions(-) diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 7223712eff442..80995aa8fd1a6 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -1066,7 +1066,7 @@ void ConnectionManagerImpl::ActiveStream::maybeEndDecode(bool end_stream) { void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapSharedPtr&& headers, bool end_stream) { ENVOY_STREAM_LOG(debug, "request headers complete (end_stream={}):\n{}", *this, end_stream, - Http::Utility::RedactSensitiveHeaderInfo(*headers)); + *headers); ScopeTrackerScopeState scope(this, connection_manager_.read_callbacks_->connection().dispatcher()); request_headers_ = std::move(headers); diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc index 43f7697cba04c..97d5a4b37c78e 100644 --- a/source/common/http/header_map_impl.cc +++ b/source/common/http/header_map_impl.cc @@ -441,13 +441,13 @@ size_t HeaderMapImpl::removePrefix(const LowerCaseString& prefix) { }); } +// Forward declaration. +namespace Utility { +void dumpHeaderMap(std::ostream& os, const HeaderMap& headers, int indent_level); +} + void HeaderMapImpl::dumpState(std::ostream& os, int indent_level) const { - iterate([&os, - spaces = spacesForLevel(indent_level)](const HeaderEntry& header) -> HeaderMap::Iterate { - os << spaces << "'" << header.key().getStringView() << "', '" << header.value().getStringView() - << "'\n"; - return HeaderMap::Iterate::Continue; - }); + Utility::dumpHeaderMap(os, dynamic_cast(*this), indent_level); } HeaderMapImpl::HeaderEntryImpl& HeaderMapImpl::maybeCreateInline(HeaderEntryImpl** entry, diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 759def556d582..5265f43d17c42 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -12,6 +12,7 @@ #include "source/common/buffer/buffer_impl.h" #include "source/common/common/assert.h" +#include "source/common/common/dump_state_utils.h" #include "source/common/common/empty_string.h" #include "source/common/common/enum_to_int.h" #include "source/common/common/fmt.h" @@ -1498,28 +1499,7 @@ bool Utility::isValidRefererValue(absl::string_view value) { return true; } -void Utility::RedactSensitiveHeaderInfo::dumpState(std::ostream& os) const { - if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.debug_include_sensitive_data")) { - os << headers_; - } else { - headers_.iterate([&os](const HeaderEntry& header) -> HeaderMap::Iterate { - os << "'" << header.key().getStringView() << "', '"; - if (header.key() == Http::Headers::get().Path) { - handlePath(os, header.value()); - } else if (header.key() == Http::Headers::get().Cookie) { - handleCookie(os, header.value()); - } else if (header.key() == Http::CustomHeaders::get().Authorization) { - os << "[redacted]"; - } else { - os << header.value().getStringView(); - } - os << "'\n"; - return HeaderMap::Iterate::Continue; - }); - } -} - -void Utility::RedactSensitiveHeaderInfo::handlePath(std::ostream& os, const HeaderString& value) { +void handlePath(std::ostream& os, const HeaderString& value) { std::string stripped = Utility::stripQueryString(value); if (stripped != value.getStringView()) { os << stripped << "?[redacted]"; @@ -1528,7 +1508,7 @@ void Utility::RedactSensitiveHeaderInfo::handlePath(std::ostream& os, const Head } } -void Utility::RedactSensitiveHeaderInfo::handleCookie(std::ostream& os, const HeaderString& value) { +void handleCookie(std::ostream& os, const HeaderString& value) { bool first = true; forEachCookie(value.getStringView(), [&os, &first](absl::string_view k, absl::string_view) -> bool { @@ -1544,5 +1524,32 @@ void Utility::RedactSensitiveHeaderInfo::handleCookie(std::ostream& os, const He }); } +void Utility::dumpHeaderMap(std::ostream& os, const HeaderMap& headers, int indent_level) { + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.debug_include_sensitive_data")) { + headers.iterate([&os, spaces = spacesForLevel(indent_level)]( + const HeaderEntry& header) -> HeaderMap::Iterate { + os << spaces << "'" << header.key().getStringView() << "', '" + << header.value().getStringView() << "'\n"; + return HeaderMap::Iterate::Continue; + }); + } else { + headers.iterate([&os, spaces = spacesForLevel(indent_level)]( + const HeaderEntry& header) -> HeaderMap::Iterate { + os << spaces << "'" << header.key().getStringView() << "', '"; + if (header.key() == Http::Headers::get().Path) { + handlePath(os, header.value()); + } else if (header.key() == Http::Headers::get().Cookie) { + handleCookie(os, header.value()); + } else if (header.key() == Http::CustomHeaders::get().Authorization) { + os << "[redacted]"; + } else { + os << header.value().getStringView(); + } + os << "'\n"; + return HeaderMap::Iterate::Continue; + }); + } +} + } // namespace Http } // namespace Envoy diff --git a/source/common/http/utility.h b/source/common/http/utility.h index f3c7d44d70834..2c7ed7eeb4a8a 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -752,48 +752,13 @@ std::string newUri(::Envoy::OptRef redirect_config, const Http::RequestHeaderMap& headers); /** - * Helper class for printing Http::HeaderMap while redacting sensitive information. + * Helper for printing Http::HeaderMap while redacting sensitive information. + * @param os supplies the ostream to print to. + * @param headers the headers to print. + * @param indent_level indentation level for the output. */ -class RedactSensitiveHeaderInfo { -public: - RedactSensitiveHeaderInfo(const Http::HeaderMap& headers) : headers_(headers) {} - friend std::ostream& operator<<(std::ostream& os, const RedactSensitiveHeaderInfo& redactor) { - redactor.dumpState(os); - return os; - } - -private: - /** - * Print key/value pairs. - * @param os supplies the ostream to print to. - * @param headers the headers to print. - */ - void dumpState(std::ostream& os) const; - - /** - * Redact query string from request path. - * @param os supplies the ostream to print to. - * @param value the value part of the pat. - */ - static void handlePath(std::ostream& os, const HeaderString& value); - - /** - * Redact cookie values from Cookie header. - * @param os supplies the ostream to print to. - * @param value value of the cookie header. - */ - static void handleCookie(std::ostream& os, const HeaderString& value); - - const Http::HeaderMap& headers_; -}; +void dumpHeaderMap(std::ostream& os, const HeaderMap& headers, int indent_level); } // namespace Utility } // namespace Http } // namespace Envoy - -// NOLINT(namespace-envoy) -namespace fmt { -// Allow fmtlib to format RedactSensitiveHeaderInfo -template <> -struct formatter : ostream_formatter {}; -} // namespace fmt diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 63a47740be143..b03ea8b89a5aa 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -687,8 +687,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, } } - ENVOY_STREAM_LOG(debug, "router decoding headers:\n{}", *callbacks_, - Http::Utility::RedactSensitiveHeaderInfo(headers)); + ENVOY_STREAM_LOG(debug, "router decoding headers:\n{}", *callbacks_, headers); // Hang onto the modify_headers function for later use in handling upstream responses. modify_headers_ = modify_headers; From 7043b24239bdee14968b6c5535e458192682b6af Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Mon, 29 May 2023 13:24:15 +0300 Subject: [PATCH 4/4] debug: added doc for masking debug logs Signed-off-by: Tero Saarni --- .../observability/application_logging.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/root/configuration/observability/application_logging.rst b/docs/root/configuration/observability/application_logging.rst index 7434d2cd5791c..05d08193d7706 100644 --- a/docs/root/configuration/observability/application_logging.rst +++ b/docs/root/configuration/observability/application_logging.rst @@ -23,3 +23,17 @@ with the following :ref:`command line options `: * The ``--log-level`` flag can be set to control the log severity logged to Stackdriver. `Reference documentation `_ for Stackdriver on GKE. + +Redacting sensitive information from debug logs +----------------------------------------------- + +Sometimes it may be necessary to enable ``debug`` level logs to troubleshoot issues in production environments. +This introduces a risk of exposing sensitive user information, such as: + +* Authorization header values with tokens or passwords. +* Cookie headers with sensitive cookies. +* Query parameters with sensitive information. + +By default header values are logged at in clear. +Envoy can be configured to redact the above header values by disabling runtime feature +``envoy.reloadable_features.debug_include_sensitive_data``.