From fa464c85186b7bbbcd1a3c3eb1dd6d18f8981811 Mon Sep 17 00:00:00 2001 From: John Esmet Date: Tue, 19 Jan 2021 21:21:43 +0000 Subject: [PATCH 1/5] router: support downstream cert formatting in the header formatter Signed-off-by: John Esmet --- .../http/http_conn_man/headers.rst | 6 ++ docs/root/version_history/current.rst | 1 + source/common/router/header_formatter.cc | 74 ++++++++----------- source/common/router/header_formatter.h | 11 ++- test/common/router/header_formatter_test.cc | 24 ++++++ 5 files changed, 72 insertions(+), 44 deletions(-) diff --git a/docs/root/configuration/http/http_conn_man/headers.rst b/docs/root/configuration/http/http_conn_man/headers.rst index 6763707f1aacf..23b9d05fd473e 100644 --- a/docs/root/configuration/http/http_conn_man/headers.rst +++ b/docs/root/configuration/http/http_conn_man/headers.rst @@ -605,12 +605,18 @@ Supported variable names are: TCP The validity start date of the client certificate used to establish the downstream TLS connection. + DOWNSTREAM_PEER_CERT_V_START can be customized with specifiers as specified in + :ref:`access log format rules`. + %DOWNSTREAM_PEER_CERT_V_END% HTTP The validity end date of the client certificate used to establish the downstream TLS connection. TCP The validity end date of the client certificate used to establish the downstream TLS connection. + DOWNSTREAM_PEER_CERT_V_END can be customized with specifiers as specified in + :ref:`access log format rules`. + %HOSTNAME% The system hostname. diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 7e6f71f7433dc..c3dd75435c390 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -19,6 +19,7 @@ Minor Behavior Changes revert to the legacy path canonicalizer, enable the runtime flag `envoy.reloadable_features.remove_forked_chromium_url`. * oauth filter: added the optional parameter :ref:`auth_scopes ` with default value of 'user' if not provided. Enables this value to be overridden in the Authorization request to the OAuth provider. +* router: extend custom date formatting to DOWNSTREAM_PEER_CERT_V_START and DOWNSTREAM_PEER_CERT_V_END when using :ref:`custom request/response header formats `. * tcp: setting NODELAY in the base connection class. This should have no effect for TCP or HTTP proxying, but may improve throughput in other areas. This behavior can be temporarily reverted by setting `envoy.reloadable_features.always_nodelay` to false. * upstream: host weight changes now cause a full load balancer rebuild as opposed to happening atomically inline. This change has been made to support load balancer pre-computation of data diff --git a/source/common/router/header_formatter.cc b/source/common/router/header_formatter.cc index f79c83b0de358..c2ef6c8ca7547 100644 --- a/source/common/router/header_formatter.cc +++ b/source/common/router/header_formatter.cc @@ -43,6 +43,33 @@ std::string formatPerRequestStateParseException(absl::string_view params) { params); } +// Parses a substitution format field and returns a function that formats it. +// Use the given empty_value if any of the underlying formatters yield no value. +// In practice, this lets the caller decide if empty string or "-" is desired. +std::function parseSubstitutionFormatField( + absl::string_view field_name, + StreamInfoHeaderFormatter::FormatterProviderListMap& formatter_providers, + const std::string& empty_value) { + const std::string pattern = fmt::format("%{}%", field_name); + if (formatter_providers.find(pattern) == formatter_providers.end()) { + formatter_providers.emplace( + std::make_pair(pattern, Formatter::SubstitutionFormatParser::parse(pattern))); + } + return [&formatter_providers, pattern, + empty_value](const Envoy::StreamInfo::StreamInfo& stream_info) { + const auto& formatters = formatter_providers.at(pattern); + std::string formatted; + for (const auto& formatter : formatters) { + const auto bit = formatter->format(*Http::StaticEmptyHeaders::get().request_headers, + *Http::StaticEmptyHeaders::get().response_headers, + *Http::StaticEmptyHeaders::get().response_trailers, + stream_info, absl::string_view()); + absl::StrAppend(&formatted, bit.value_or(empty_value)); + } + return formatted; + }; +} + // Parses the parameters for UPSTREAM_METADATA and returns a function suitable for accessing the // specified metadata from an StreamInfo::StreamInfo. Expects a string formatted as: // (["a", "b", "c"]) @@ -210,21 +237,6 @@ StreamInfoHeaderFormatter::FieldExtractor sslConnectionInfoStringHeaderExtractor }; } -// Helper that handles the case when the desired time field is empty. -StreamInfoHeaderFormatter::FieldExtractor sslConnectionInfoStringTimeHeaderExtractor( - std::function(const Ssl::ConnectionInfo& connection_info)> - time_extractor) { - return sslConnectionInfoStringHeaderExtractor( - [time_extractor](const Ssl::ConnectionInfo& connection_info) { - absl::optional time = time_extractor(connection_info); - if (!time.has_value()) { - return std::string(); - } - - return AccessLogDateTimeFormatter::fromTime(time.value()); - }); -} - } // namespace StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_name, bool append) @@ -313,16 +325,10 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam sslConnectionInfoStringHeaderExtractor([](const Ssl::ConnectionInfo& connection_info) { return connection_info.urlEncodedPemEncodedPeerCertificate(); }); - } else if (field_name == "DOWNSTREAM_PEER_CERT_V_START") { - field_extractor_ = - sslConnectionInfoStringTimeHeaderExtractor([](const Ssl::ConnectionInfo& connection_info) { - return connection_info.validFromPeerCertificate(); - }); - } else if (field_name == "DOWNSTREAM_PEER_CERT_V_END") { - field_extractor_ = - sslConnectionInfoStringTimeHeaderExtractor([](const Ssl::ConnectionInfo& connection_info) { - return connection_info.expirationPeerCertificate(); - }); + } else if (absl::StartsWith(field_name, "DOWNSTREAM_PEER_CERT_V_START")) { + field_extractor_ = parseSubstitutionFormatField(field_name, formatter_providers_, ""); + } else if (absl::StartsWith(field_name, "DOWNSTREAM_PEER_CERT_V_END")) { + field_extractor_ = parseSubstitutionFormatField(field_name, formatter_providers_, ""); } else if (field_name == "UPSTREAM_REMOTE_ADDRESS") { field_extractor_ = [](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string { if (stream_info.upstreamHost()) { @@ -331,23 +337,7 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam return ""; }; } else if (absl::StartsWith(field_name, "START_TIME")) { - const std::string pattern = fmt::format("%{}%", field_name); - if (start_time_formatters_.find(pattern) == start_time_formatters_.end()) { - start_time_formatters_.emplace( - std::make_pair(pattern, Formatter::SubstitutionFormatParser::parse(pattern))); - } - field_extractor_ = [this, pattern](const Envoy::StreamInfo::StreamInfo& stream_info) { - const auto& formatters = start_time_formatters_.at(pattern); - std::string formatted; - for (const auto& formatter : formatters) { - const auto bit = formatter->format(*Http::StaticEmptyHeaders::get().request_headers, - *Http::StaticEmptyHeaders::get().response_headers, - *Http::StaticEmptyHeaders::get().response_trailers, - stream_info, absl::string_view()); - absl::StrAppend(&formatted, bit.value_or("-")); - } - return formatted; - }; + field_extractor_ = parseSubstitutionFormatField(field_name, formatter_providers_, "-"); } else if (absl::StartsWith(field_name, "UPSTREAM_METADATA")) { field_extractor_ = parseMetadataField(field_name.substr(STATIC_STRLEN("UPSTREAM_METADATA"))); } else if (absl::StartsWith(field_name, "DYNAMIC_METADATA")) { diff --git a/source/common/router/header_formatter.h b/source/common/router/header_formatter.h index 65a996a5a9ebf..c0253ceae50d6 100644 --- a/source/common/router/header_formatter.h +++ b/source/common/router/header_formatter.h @@ -42,12 +42,19 @@ class StreamInfoHeaderFormatter : public HeaderFormatter { bool append() const override { return append_; } using FieldExtractor = std::function; + using FormatterProviderListMap = + absl::node_hash_map>; private: FieldExtractor field_extractor_; const bool append_; - absl::node_hash_map> - start_time_formatters_; + + // Maps a string format pattern (including field name and any command operators between + // parenthesis) to the list of FormatterProviderPtrs that are capable of formatting that pattern. + // We use a map here to make sure that we only create a single parser for a given format pattern + // even if it appears multiple times in the larger formatting context (e.g. it shows up multiple + // times in a format string). + FormatterProviderListMap formatter_providers_; }; /** diff --git a/test/common/router/header_formatter_test.cc b/test/common/router/header_formatter_test.cc index a99d7fbfe5587..2dbbd72de14da 100644 --- a/test/common/router/header_formatter_test.cc +++ b/test/common/router/header_formatter_test.cc @@ -463,6 +463,18 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVStart) { testFormatting(stream_info, "DOWNSTREAM_PEER_CERT_V_START", "2018-12-18T01:50:34.000Z"); } +TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVStartCustom) { + NiceMock stream_info; + auto connection_info = std::make_shared>(); + absl::Time abslStartTime = + TestUtility::parseTime("Dec 18 01:50:34 2018 GMT", "%b %e %H:%M:%S %Y GMT"); + SystemTime startTime = absl::ToChronoTime(abslStartTime); + ON_CALL(*connection_info, validFromPeerCertificate()).WillByDefault(Return(startTime)); + EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); + testFormatting(stream_info, "DOWNSTREAM_PEER_CERT_V_START(%b %e %H:%M:%S %Y %Z)", + "Dec 18 01:50:34 2018 UTC"); +} + TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVStartEmpty) { NiceMock stream_info; auto connection_info = std::make_shared>(); @@ -488,6 +500,18 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVEnd) { testFormatting(stream_info, "DOWNSTREAM_PEER_CERT_V_END", "2020-12-17T01:50:34.000Z"); } +TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVEndCustom) { + NiceMock stream_info; + auto connection_info = std::make_shared>(); + absl::Time abslStartTime = + TestUtility::parseTime("Dec 17 01:50:34 2020 GMT", "%b %e %H:%M:%S %Y GMT"); + SystemTime startTime = absl::ToChronoTime(abslStartTime); + ON_CALL(*connection_info, expirationPeerCertificate()).WillByDefault(Return(startTime)); + EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); + testFormatting(stream_info, "DOWNSTREAM_PEER_CERT_V_END(%b %e %H:%M:%S %Y %Z)", + "Dec 17 01:50:34 2020 UTC"); +} + TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVEndEmpty) { NiceMock stream_info; auto connection_info = std::make_shared>(); From bc135d6f1ba2505998949d620374b10ae5685c42 Mon Sep 17 00:00:00 2001 From: John Esmet Date: Tue, 26 Jan 2021 21:08:29 +0000 Subject: [PATCH 2/5] Try using the existing substitution formatter code Signed-off-by: John Esmet --- source/common/router/header_formatter.cc | 40 ++++++++++-------------- source/common/router/header_formatter.h | 5 ++- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/source/common/router/header_formatter.cc b/source/common/router/header_formatter.cc index c2ef6c8ca7547..81c3186bfb806 100644 --- a/source/common/router/header_formatter.cc +++ b/source/common/router/header_formatter.cc @@ -44,29 +44,21 @@ std::string formatPerRequestStateParseException(absl::string_view params) { } // Parses a substitution format field and returns a function that formats it. -// Use the given empty_value if any of the underlying formatters yield no value. -// In practice, this lets the caller decide if empty string or "-" is desired. -std::function parseSubstitutionFormatField( - absl::string_view field_name, - StreamInfoHeaderFormatter::FormatterProviderListMap& formatter_providers, - const std::string& empty_value) { +std::function +parseSubstitutionFormatField(absl::string_view field_name, + StreamInfoHeaderFormatter::FormatterPtrMap& formatter_map) { const std::string pattern = fmt::format("%{}%", field_name); - if (formatter_providers.find(pattern) == formatter_providers.end()) { - formatter_providers.emplace( - std::make_pair(pattern, Formatter::SubstitutionFormatParser::parse(pattern))); + if (formatter_map.find(pattern) == formatter_map.end()) { + formatter_map.emplace( + std::make_pair(pattern, Formatter::FormatterPtr(new Formatter::FormatterImpl( + pattern, /*omit_empty_values=*/true)))); } - return [&formatter_providers, pattern, - empty_value](const Envoy::StreamInfo::StreamInfo& stream_info) { - const auto& formatters = formatter_providers.at(pattern); - std::string formatted; - for (const auto& formatter : formatters) { - const auto bit = formatter->format(*Http::StaticEmptyHeaders::get().request_headers, - *Http::StaticEmptyHeaders::get().response_headers, - *Http::StaticEmptyHeaders::get().response_trailers, - stream_info, absl::string_view()); - absl::StrAppend(&formatted, bit.value_or(empty_value)); - } - return formatted; + return [&formatter_map, pattern](const Envoy::StreamInfo::StreamInfo& stream_info) { + const auto& formatter = formatter_map.at(pattern); + return formatter->format(*Http::StaticEmptyHeaders::get().request_headers, + *Http::StaticEmptyHeaders::get().response_headers, + *Http::StaticEmptyHeaders::get().response_trailers, stream_info, + absl::string_view()); }; } @@ -326,9 +318,9 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam return connection_info.urlEncodedPemEncodedPeerCertificate(); }); } else if (absl::StartsWith(field_name, "DOWNSTREAM_PEER_CERT_V_START")) { - field_extractor_ = parseSubstitutionFormatField(field_name, formatter_providers_, ""); + field_extractor_ = parseSubstitutionFormatField(field_name, formatter_map_); } else if (absl::StartsWith(field_name, "DOWNSTREAM_PEER_CERT_V_END")) { - field_extractor_ = parseSubstitutionFormatField(field_name, formatter_providers_, ""); + field_extractor_ = parseSubstitutionFormatField(field_name, formatter_map_); } else if (field_name == "UPSTREAM_REMOTE_ADDRESS") { field_extractor_ = [](const Envoy::StreamInfo::StreamInfo& stream_info) -> std::string { if (stream_info.upstreamHost()) { @@ -337,7 +329,7 @@ StreamInfoHeaderFormatter::StreamInfoHeaderFormatter(absl::string_view field_nam return ""; }; } else if (absl::StartsWith(field_name, "START_TIME")) { - field_extractor_ = parseSubstitutionFormatField(field_name, formatter_providers_, "-"); + field_extractor_ = parseSubstitutionFormatField(field_name, formatter_map_); } else if (absl::StartsWith(field_name, "UPSTREAM_METADATA")) { field_extractor_ = parseMetadataField(field_name.substr(STATIC_STRLEN("UPSTREAM_METADATA"))); } else if (absl::StartsWith(field_name, "DYNAMIC_METADATA")) { diff --git a/source/common/router/header_formatter.h b/source/common/router/header_formatter.h index c0253ceae50d6..0aace90b36537 100644 --- a/source/common/router/header_formatter.h +++ b/source/common/router/header_formatter.h @@ -42,8 +42,7 @@ class StreamInfoHeaderFormatter : public HeaderFormatter { bool append() const override { return append_; } using FieldExtractor = std::function; - using FormatterProviderListMap = - absl::node_hash_map>; + using FormatterPtrMap = absl::node_hash_map; private: FieldExtractor field_extractor_; @@ -54,7 +53,7 @@ class StreamInfoHeaderFormatter : public HeaderFormatter { // We use a map here to make sure that we only create a single parser for a given format pattern // even if it appears multiple times in the larger formatting context (e.g. it shows up multiple // times in a format string). - FormatterProviderListMap formatter_providers_; + FormatterPtrMap formatter_map_; }; /** From cff60931ec3a3955459887c47afd680bc76272ab Mon Sep 17 00:00:00 2001 From: John Esmet Date: Thu, 28 Jan 2021 11:14:02 -0500 Subject: [PATCH 3/5] Update current.rst Signed-off-by: John Esmet --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index c3dd75435c390..2f0d4618822f1 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -19,7 +19,7 @@ Minor Behavior Changes revert to the legacy path canonicalizer, enable the runtime flag `envoy.reloadable_features.remove_forked_chromium_url`. * oauth filter: added the optional parameter :ref:`auth_scopes ` with default value of 'user' if not provided. Enables this value to be overridden in the Authorization request to the OAuth provider. -* router: extend custom date formatting to DOWNSTREAM_PEER_CERT_V_START and DOWNSTREAM_PEER_CERT_V_END when using :ref:`custom request/response header formats `. +* router: extended custom date formatting to DOWNSTREAM_PEER_CERT_V_START and DOWNSTREAM_PEER_CERT_V_END when using :ref:`custom request/response header formats `. * tcp: setting NODELAY in the base connection class. This should have no effect for TCP or HTTP proxying, but may improve throughput in other areas. This behavior can be temporarily reverted by setting `envoy.reloadable_features.always_nodelay` to false. * upstream: host weight changes now cause a full load balancer rebuild as opposed to happening atomically inline. This change has been made to support load balancer pre-computation of data From 08f854f4398e463c67ebd6db1ffca2b80b913bee Mon Sep 17 00:00:00 2001 From: John Esmet Date: Fri, 29 Jan 2021 16:43:18 +0000 Subject: [PATCH 4/5] Format Signed-off-by: John Esmet --- test/common/router/header_formatter_test.cc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/common/router/header_formatter_test.cc b/test/common/router/header_formatter_test.cc index 2dbbd72de14da..f501a0d5ab356 100644 --- a/test/common/router/header_formatter_test.cc +++ b/test/common/router/header_formatter_test.cc @@ -526,6 +526,24 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerCertVEndNoTls) testFormatting(stream_info, "DOWNSTREAM_PEER_CERT_V_END", EMPTY_STRING); } +TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithStartTime) { + NiceMock stream_info; + absl::Time abslStartTime = + TestUtility::parseTime("Dec 17 01:50:34 2020 GMT", "%b %e %H:%M:%S %Y GMT"); + SystemTime startTime = absl::ToChronoTime(abslStartTime); + EXPECT_CALL(stream_info, startTime()).WillRepeatedly(Return(startTime)); + testFormatting(stream_info, "START_TIME", "2020-12-17T01:50:34.000Z"); +} + +TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithStartTimeCustom) { + NiceMock stream_info; + absl::Time abslStartTime = + TestUtility::parseTime("Dec 17 01:50:34 2020 GMT", "%b %e %H:%M:%S %Y GMT"); + SystemTime startTime = absl::ToChronoTime(abslStartTime); + EXPECT_CALL(stream_info, startTime()).WillRepeatedly(Return(startTime)); + testFormatting(stream_info, "START_TIME(%b %e %H:%M:%S %Y %Z)", "Dec 17 01:50:34 2020 UTC"); +} + TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithUpstreamMetadataVariable) { NiceMock stream_info; std::shared_ptr> host( From 49c83c793e2c0d77e05ce549da20fce24ef71c7f Mon Sep 17 00:00:00 2001 From: John Esmet Date: Tue, 2 Feb 2021 15:11:29 +0000 Subject: [PATCH 5/5] Reorder changelog entry Signed-off-by: John Esmet --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 59b91e744d465..7a2ff4f94fb79 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -34,8 +34,8 @@ Minor Behavior Changes revert to the legacy path canonicalizer, enable the runtime flag `envoy.reloadable_features.remove_forked_chromium_url`. * oauth filter: added the optional parameter :ref:`auth_scopes ` with default value of 'user' if not provided. Enables this value to be overridden in the Authorization request to the OAuth provider. -* router: extended custom date formatting to DOWNSTREAM_PEER_CERT_V_START and DOWNSTREAM_PEER_CERT_V_END when using :ref:`custom request/response header formats `. * perf: allow reading more bytes per operation from raw sockets to improve performance. +* router: extended custom date formatting to DOWNSTREAM_PEER_CERT_V_START and DOWNSTREAM_PEER_CERT_V_END when using :ref:`custom request/response header formats `. * tcp: setting NODELAY in the base connection class. This should have no effect for TCP or HTTP proxying, but may improve throughput in other areas. This behavior can be temporarily reverted by setting `envoy.reloadable_features.always_nodelay` to false. * upstream: host weight changes now cause a full load balancer rebuild as opposed to happening atomically inline. This change has been made to support load balancer pre-computation of data