From 93e6641cb7d988fb501383edfd1d9313de719afb Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 08:55:20 +0300 Subject: [PATCH 01/18] add AttributeName::kConnectionRequestedServerName --- include/istio/utils/attribute_names.h | 1 + src/istio/utils/attribute_names.cc | 2 ++ 2 files changed, 3 insertions(+) diff --git a/include/istio/utils/attribute_names.h b/include/istio/utils/attribute_names.h index d7c00fd3bb2..9dad877a20a 100644 --- a/include/istio/utils/attribute_names.h +++ b/include/istio/utils/attribute_names.h @@ -65,6 +65,7 @@ struct AttributeName { static const char kConnectionSendTotalBytes[]; static const char kConnectionDuration[]; static const char kConnectionMtls[]; + static const char kConnectionRequestedServerName[]; static const char kConnectionId[]; // Record TCP connection status: open, continue, close static const char kConnectionEvent[]; diff --git a/src/istio/utils/attribute_names.cc b/src/istio/utils/attribute_names.cc index 5b50d411c44..30f9eb249f5 100644 --- a/src/istio/utils/attribute_names.cc +++ b/src/istio/utils/attribute_names.cc @@ -58,6 +58,8 @@ const char AttributeName::kConnectionSendTotalBytes[] = "connection.sent.bytes_total"; const char AttributeName::kConnectionDuration[] = "connection.duration"; const char AttributeName::kConnectionMtls[] = "connection.mtls"; +const char AttributeName::kConnectionRequestedServerName[] = "connection.requested_server_name"; + // Downstream TCP connection id. const char AttributeName::kConnectionId[] = "connection.id"; const char AttributeName::kConnectionEvent[] = "connection.event"; From 777b38f46c113d866fd3ca57b5bddc6c08ab7d95 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 10:21:13 +0300 Subject: [PATCH 02/18] fix format --- src/istio/utils/attribute_names.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/istio/utils/attribute_names.cc b/src/istio/utils/attribute_names.cc index 30f9eb249f5..759cd49b985 100644 --- a/src/istio/utils/attribute_names.cc +++ b/src/istio/utils/attribute_names.cc @@ -58,7 +58,8 @@ const char AttributeName::kConnectionSendTotalBytes[] = "connection.sent.bytes_total"; const char AttributeName::kConnectionDuration[] = "connection.duration"; const char AttributeName::kConnectionMtls[] = "connection.mtls"; -const char AttributeName::kConnectionRequestedServerName[] = "connection.requested_server_name"; +const char AttributeName::kConnectionRequestedServerName[] = + "connection.requested_server_name"; // Downstream TCP connection id. const char AttributeName::kConnectionId[] = "connection.id"; From cb543c1da7903579ecb06d632c85539adf5ac66b Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 10:22:10 +0300 Subject: [PATCH 03/18] add GetRequestedServerName() to TCP CheckData --- include/istio/control/tcp/check_data.h | 3 +++ src/istio/control/tcp/mock_check_data.h | 1 + 2 files changed, 4 insertions(+) diff --git a/include/istio/control/tcp/check_data.h b/include/istio/control/tcp/check_data.h index 991b6f8d421..6bb5edf2210 100644 --- a/include/istio/control/tcp/check_data.h +++ b/include/istio/control/tcp/check_data.h @@ -37,6 +37,9 @@ class CheckData { // Returns true if connection is mutual TLS enabled. virtual bool IsMutualTLS() const = 0; + // Get requested server name, SNI in case of TLS + virtual std::string GetRequestedServerName() const = 0; + // Get downstream tcp connection id. virtual std::string GetConnectionId() const = 0; }; diff --git a/src/istio/control/tcp/mock_check_data.h b/src/istio/control/tcp/mock_check_data.h index 170e766b500..4c10d3ee309 100644 --- a/src/istio/control/tcp/mock_check_data.h +++ b/src/istio/control/tcp/mock_check_data.h @@ -29,6 +29,7 @@ class MockCheckData : public CheckData { MOCK_CONST_METHOD2(GetSourceIpPort, bool(std::string* ip, int* port)); MOCK_CONST_METHOD1(GetSourceUser, bool(std::string* user)); MOCK_CONST_METHOD0(IsMutualTLS, bool()); + MOCK_CONST_METHOD0(GetRequestedServerName, std::string()); MOCK_CONST_METHOD0(GetConnectionId, std::string()); }; From 2ead91911f8d39013d7065344e1ab7f2f41ed587 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 10:57:05 +0300 Subject: [PATCH 04/18] add building attribute ConnectionRequestedServerName --- src/istio/control/tcp/attributes_builder.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/istio/control/tcp/attributes_builder.cc b/src/istio/control/tcp/attributes_builder.cc index 64fa59ed473..f299b62dba0 100644 --- a/src/istio/control/tcp/attributes_builder.cc +++ b/src/istio/control/tcp/attributes_builder.cc @@ -51,6 +51,9 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { builder.AddBool(utils::AttributeName::kConnectionMtls, check_data->IsMutualTLS()); + builder.AddString(utils::AttributeName::kConnectionRequestedServerName, + check_data->GetRequestedServerName()); + builder.AddTimestamp(utils::AttributeName::kContextTime, std::chrono::system_clock::now()); builder.AddString(utils::AttributeName::kContextProtocol, "tcp"); From b45b25d1c3ab3b6715568aaa91c4928c546c0e80 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 11:06:17 +0300 Subject: [PATCH 05/18] test building attribute ConnectionRequestedServerName --- src/istio/control/tcp/attributes_builder_test.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/istio/control/tcp/attributes_builder_test.cc b/src/istio/control/tcp/attributes_builder_test.cc index a9d14c9b3b1..d93c923f31f 100644 --- a/src/istio/control/tcp/attributes_builder_test.cc +++ b/src/istio/control/tcp/attributes_builder_test.cc @@ -67,6 +67,12 @@ attributes { bool_value: true } } +attributes { + key: "connection.requested_server_name" + value { + string_value: "www.google.com" + } +} attributes { key: "source.principal" value { @@ -305,6 +311,8 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { return true; })); EXPECT_CALL(mock_data, GetConnectionId()).WillOnce(Return("1234-5")); + EXPECT_CALL(mock_data, GetRequestedServerName()) + .WillOnce(Return("www.google.com")); RequestContext request; AttributesBuilder builder(&request); From 83dd6a049f4f34c9788c57d6286beb811c92efb9 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 14:09:16 +0300 Subject: [PATCH 06/18] add GetRequestedServerName() to tcp mixer filter --- src/envoy/tcp/mixer/filter.cc | 4 ++++ src/envoy/tcp/mixer/filter.h | 1 + 2 files changed, 5 insertions(+) diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc index 128e06ef715..0b7ae1cecc7 100644 --- a/src/envoy/tcp/mixer/filter.cc +++ b/src/envoy/tcp/mixer/filter.cc @@ -157,6 +157,10 @@ bool Filter::IsMutualTLS() const { return Utils::IsMutualTLS(&filter_callbacks_->connection()); } +std::string Filter::GetRequestedServerName() const { + return filter_callbacks->connection().requestedServerName() +} + bool Filter::GetDestinationIpPort(std::string* str_ip, int* port) const { if (filter_callbacks_->upstreamHost() && filter_callbacks_->upstreamHost()->address()) { diff --git a/src/envoy/tcp/mixer/filter.h b/src/envoy/tcp/mixer/filter.h index 2fa8b8fe382..ed1f9685714 100644 --- a/src/envoy/tcp/mixer/filter.h +++ b/src/envoy/tcp/mixer/filter.h @@ -53,6 +53,7 @@ class Filter : public Network::Filter, bool GetSourceIpPort(std::string* str_ip, int* port) const override; bool GetSourceUser(std::string* user) const override; bool IsMutualTLS() const override; + std::string GetRequestedServerName() const override; // ReportData virtual functions. bool GetDestinationIpPort(std::string* str_ip, int* port) const override; From 0091e10d6f45bdde4f788ae0b97f96723ebf7ed0 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 14:29:52 +0300 Subject: [PATCH 07/18] fix compilation errors --- src/envoy/tcp/mixer/filter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc index 0b7ae1cecc7..ff46e2ef6c9 100644 --- a/src/envoy/tcp/mixer/filter.cc +++ b/src/envoy/tcp/mixer/filter.cc @@ -158,7 +158,7 @@ bool Filter::IsMutualTLS() const { } std::string Filter::GetRequestedServerName() const { - return filter_callbacks->connection().requestedServerName() + return filter_callbacks_->connection().requestedServerName(); } bool Filter::GetDestinationIpPort(std::string* str_ip, int* port) const { From 79fa89ffba1b56e8245719c07e8a5614981e448a Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 14:35:54 +0300 Subject: [PATCH 08/18] use explicit conversion from absl::string_view to std::string --- src/envoy/tcp/mixer/filter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/envoy/tcp/mixer/filter.cc b/src/envoy/tcp/mixer/filter.cc index ff46e2ef6c9..944e2b1ce6f 100644 --- a/src/envoy/tcp/mixer/filter.cc +++ b/src/envoy/tcp/mixer/filter.cc @@ -158,7 +158,7 @@ bool Filter::IsMutualTLS() const { } std::string Filter::GetRequestedServerName() const { - return filter_callbacks_->connection().requestedServerName(); + return std::string(filter_callbacks_->connection().requestedServerName()); } bool Filter::GetDestinationIpPort(std::string* str_ip, int* port) const { From 336a0f05283e8ef85ed9a4b84f05de026c1351b5 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 20:00:56 +0300 Subject: [PATCH 09/18] check that the requested server name is not emtpy in attributes builder --- src/istio/control/tcp/attributes_builder.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/istio/control/tcp/attributes_builder.cc b/src/istio/control/tcp/attributes_builder.cc index f299b62dba0..2a54a5c35ac 100644 --- a/src/istio/control/tcp/attributes_builder.cc +++ b/src/istio/control/tcp/attributes_builder.cc @@ -51,8 +51,11 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { builder.AddBool(utils::AttributeName::kConnectionMtls, check_data->IsMutualTLS()); - builder.AddString(utils::AttributeName::kConnectionRequestedServerName, - check_data->GetRequestedServerName()); + std::string requested_server_name = check_data->GetRequestedServerName()); + if (!requested_server_name.empty()) { + builder.AddString(utils::AttributeName::kConnectionRequestedServerName, + requested_server_name); + } builder.AddTimestamp(utils::AttributeName::kContextTime, std::chrono::system_clock::now()); From 13896816bb3472ee2a08cebf6ecca6a87b56b386 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 20:14:39 +0300 Subject: [PATCH 10/18] fixed a compilation error --- src/istio/control/tcp/attributes_builder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/istio/control/tcp/attributes_builder.cc b/src/istio/control/tcp/attributes_builder.cc index 2a54a5c35ac..329702283cf 100644 --- a/src/istio/control/tcp/attributes_builder.cc +++ b/src/istio/control/tcp/attributes_builder.cc @@ -51,7 +51,7 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData* check_data) { builder.AddBool(utils::AttributeName::kConnectionMtls, check_data->IsMutualTLS()); - std::string requested_server_name = check_data->GetRequestedServerName()); + std::string requested_server_name = check_data->GetRequestedServerName(); if (!requested_server_name.empty()) { builder.AddString(utils::AttributeName::kConnectionRequestedServerName, requested_server_name); From a129dbf5b320b1d4e8508a549f28e0db160f1eb1 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 20:25:45 +0300 Subject: [PATCH 11/18] add GetRequestedServerName to http mixer filters (check_data) --- include/istio/control/http/check_data.h | 3 +++ src/envoy/http/mixer/check_data.cc | 4 ++++ src/envoy/http/mixer/check_data.h | 1 + 3 files changed, 8 insertions(+) diff --git a/include/istio/control/http/check_data.h b/include/istio/control/http/check_data.h index fc8d204aa88..01f7d2d4c0a 100644 --- a/include/istio/control/http/check_data.h +++ b/include/istio/control/http/check_data.h @@ -47,6 +47,9 @@ class CheckData { // Returns true if connection is mutual TLS enabled. virtual bool IsMutualTLS() const = 0; + // Get requested server name, SNI in case of TLS + virtual std::string GetRequestedServerName() const = 0; + // These headers are extracted into top level attributes. // This is for standard HTTP headers. It supports both HTTP/1.1 and HTTP2 // They can be retrieved at O(1) speed by environment (Envoy). diff --git a/src/envoy/http/mixer/check_data.cc b/src/envoy/http/mixer/check_data.cc index 4726a4be7ae..5c111633dbf 100644 --- a/src/envoy/http/mixer/check_data.cc +++ b/src/envoy/http/mixer/check_data.cc @@ -77,6 +77,10 @@ std::map CheckData::GetRequestHeaders() const { bool CheckData::IsMutualTLS() const { return Utils::IsMutualTLS(connection_); } +std::string GetRequestedServerName() const { + return connection_.requestedServerName(); +} + bool CheckData::FindHeaderByType(HttpCheckData::HeaderType header_type, std::string* value) const { switch (header_type) { diff --git a/src/envoy/http/mixer/check_data.h b/src/envoy/http/mixer/check_data.h index b03c3a1e7f2..b41d05547f7 100644 --- a/src/envoy/http/mixer/check_data.h +++ b/src/envoy/http/mixer/check_data.h @@ -41,6 +41,7 @@ class CheckData : public ::istio::control::http::CheckData, std::map GetRequestHeaders() const override; bool IsMutualTLS() const override; + std::string GetRequestedServerName() const override; bool FindHeaderByType( ::istio::control::http::CheckData::HeaderType header_type, From b1967a2e832c2aebd1db2d72f2a6f1fa43b99f23 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 20:35:35 +0300 Subject: [PATCH 12/18] add GetRequestedServerName to http MockCheckData --- src/istio/control/http/mock_check_data.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/istio/control/http/mock_check_data.h b/src/istio/control/http/mock_check_data.h index 9fc44f20d71..43066567b84 100644 --- a/src/istio/control/http/mock_check_data.h +++ b/src/istio/control/http/mock_check_data.h @@ -44,6 +44,7 @@ class MockCheckData : public CheckData { MOCK_CONST_METHOD1(GetAuthenticationResult, bool(istio::authn::Result *result)); MOCK_CONST_METHOD0(IsMutualTLS, bool()); + MOCK_CONST_METHOD0(GetRequestedServerName, std::string()); }; // The mock object for HeaderUpdate interface. From b874c59eb216a74ed62a66b775e3f6894c4acf65 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 20:38:08 +0300 Subject: [PATCH 13/18] specify the class of a method --- src/envoy/http/mixer/check_data.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/envoy/http/mixer/check_data.cc b/src/envoy/http/mixer/check_data.cc index 5c111633dbf..983f9e393ef 100644 --- a/src/envoy/http/mixer/check_data.cc +++ b/src/envoy/http/mixer/check_data.cc @@ -77,7 +77,7 @@ std::map CheckData::GetRequestHeaders() const { bool CheckData::IsMutualTLS() const { return Utils::IsMutualTLS(connection_); } -std::string GetRequestedServerName() const { +std::string CheckData::GetRequestedServerName() const { return connection_.requestedServerName(); } From 43e4beaba5cddb4bd6778e608a32ac532a298c6a Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 20:44:06 +0300 Subject: [PATCH 14/18] add setting connection.requested_server_name to the http attributes --- src/istio/control/http/attributes_builder.cc | 6 ++++++ src/istio/control/http/attributes_builder_test.cc | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index 7acdff379cd..e39537c696a 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -157,6 +157,12 @@ void AttributesBuilder::ExtractCheckAttributes(CheckData *check_data) { builder.AddBool(utils::AttributeName::kConnectionMtls, check_data->IsMutualTLS()); + std::string requested_server_name = check_data->GetRequestedServerName(); + if (!requested_server_name.empty()) { + builder.AddString(utils::AttributeName::kConnectionRequestedServerName, + requested_server_name); + } + builder.AddTimestamp(utils::AttributeName::kRequestTime, std::chrono::system_clock::now()); diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index d1475f734b9..24a652958ac 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -89,6 +89,12 @@ attributes { bool_value: true } } +attributes { + key: "connection.requested_server_name" + value { + string_value: "www.google.com" + } +} attributes { key: "source.principal" value { @@ -286,6 +292,8 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { EXPECT_CALL(mock_data, IsMutualTLS()).WillOnce(Invoke([]() -> bool { return true; })); + EXPECT_CALL(mock_data, GetRequestedServerName()) + .WillOnce(Return("www.google.com")); EXPECT_CALL(mock_data, GetRequestHeaders()) .WillOnce(Invoke([]() -> std::map { std::map map; From 715ce489cf7f1903a8a48002ded3f465dfb3f2a3 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 20:48:37 +0300 Subject: [PATCH 15/18] qualify Return by testing:: --- src/istio/control/http/attributes_builder_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 24a652958ac..7ea673e5930 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -293,7 +293,7 @@ TEST(AttributesBuilderTest, TestCheckAttributes) { return true; })); EXPECT_CALL(mock_data, GetRequestedServerName()) - .WillOnce(Return("www.google.com")); + .WillOnce(testing::Return("www.google.com")); EXPECT_CALL(mock_data, GetRequestHeaders()) .WillOnce(Invoke([]() -> std::map { std::map map; From c3ab69c6b254d23cb37526a1a59762ad12e0a5da Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 20:56:10 +0300 Subject: [PATCH 16/18] use connection_ as a pointer --- src/envoy/http/mixer/check_data.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/envoy/http/mixer/check_data.cc b/src/envoy/http/mixer/check_data.cc index 983f9e393ef..0a79135bddf 100644 --- a/src/envoy/http/mixer/check_data.cc +++ b/src/envoy/http/mixer/check_data.cc @@ -78,7 +78,11 @@ std::map CheckData::GetRequestHeaders() const { bool CheckData::IsMutualTLS() const { return Utils::IsMutualTLS(connection_); } std::string CheckData::GetRequestedServerName() const { - return connection_.requestedServerName(); + if (connection_) { + return connection_->requestedServerName(); + } + + return ""; } bool CheckData::FindHeaderByType(HttpCheckData::HeaderType header_type, From 76fe1d88186147963d804ffc28d7e8b27a22b5d6 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 21:08:41 +0300 Subject: [PATCH 17/18] add explicit conversion from absl::string_view to std::string --- src/envoy/http/mixer/check_data.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/envoy/http/mixer/check_data.cc b/src/envoy/http/mixer/check_data.cc index 0a79135bddf..64cc1fcd972 100644 --- a/src/envoy/http/mixer/check_data.cc +++ b/src/envoy/http/mixer/check_data.cc @@ -79,7 +79,7 @@ bool CheckData::IsMutualTLS() const { return Utils::IsMutualTLS(connection_); } std::string CheckData::GetRequestedServerName() const { if (connection_) { - return connection_->requestedServerName(); + return std::string(connection_->requestedServerName()); } return ""; From 3da532768be66dcfaa02a7cad5903a02a870c4d4 Mon Sep 17 00:00:00 2001 From: Vadim Eisenberg Date: Tue, 10 Jul 2018 21:28:47 +0300 Subject: [PATCH 18/18] add missing mock call --- src/istio/control/http/attributes_builder_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 7ea673e5930..26a319440ff 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -349,6 +349,8 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) { EXPECT_CALL(mock_data, IsMutualTLS()).WillOnce(Invoke([]() -> bool { return true; })); + EXPECT_CALL(mock_data, GetRequestedServerName()) + .WillOnce(testing::Return("www.google.com")); EXPECT_CALL(mock_data, GetRequestHeaders()) .WillOnce(Invoke([]() -> std::map { std::map map;