-
Notifications
You must be signed in to change notification settings - Fork 5.5k
tls: add functionality to override requested server name in the upstream cluster #4973
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 18 commits
2fa0949
9701ce6
ab66f42
5fe6e3e
b6f32d7
6aff2ea
0b2a296
6c9128d
dedd511
27557de
b019a99
09af682
8ff3b25
cfbb753
566570f
173849b
41cf2c7
995d15e
c8afa18
c9ed4a7
bdce6bb
895dafb
088f2d8
88b3d31
e9c0a87
0fa1bd3
7109894
b8f0d3c
dddd8e3
2532513
588fa98
2f23141
b31e6f6
d84b467
d6583b9
505cce2
58efa36
7080014
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| #include "common/network/transport_socket_options_impl.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Network { | ||
| TransportSocketOptionsImpl::TransportSocketOptionsImpl(std::string override_server_name) { | ||
| if (!override_server_name.empty()) { | ||
| override_server_name_ = override_server_name; | ||
| } | ||
| } | ||
|
|
||
| void TransportSocketOptionsImpl::hashKey(std::vector<uint8_t>& key) const { | ||
| if (!override_server_name_.has_value()) { | ||
| return; | ||
| } | ||
|
|
||
| std::hash<std::string> hash_function; | ||
| key.push_back(hash_function(override_server_name_.value())); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this does what is expected. The hash function will return a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
| } // namespace Network | ||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| #pragma once | ||
|
|
||
| #include "envoy/network/transport_socket.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Network { | ||
|
|
||
| class TransportSocketOptionsImpl : public TransportSocketOptions { | ||
| public: | ||
| TransportSocketOptionsImpl(std::string override_server_name = ""); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 895dafb. |
||
| const absl::optional<std::string>& overrideServerName() const override { | ||
| return override_server_name_; | ||
| } | ||
| void hashKey(std::vector<uint8_t>& key) const override; | ||
|
|
||
| private: | ||
| absl::optional<std::string> override_server_name_; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in bdce6bb. |
||
| }; | ||
|
|
||
| } // namespace Network | ||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| #include "common/network/upstream_server_name.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Network { | ||
|
|
||
| const std::string UpstreamServerName::Key = "envoy.network.upstream_server_name"; | ||
|
|
||
| } // namespace Network | ||
| } // namespace Envoy |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||
| #pragma once | ||||||
|
|
||||||
| #include "envoy/stream_info/filter_state.h" | ||||||
|
|
||||||
| #include "absl/strings/string_view.h" | ||||||
|
|
||||||
| namespace Envoy { | ||||||
| namespace Network { | ||||||
|
|
||||||
| /** | ||||||
| * Original Requested Server Name | ||||||
| */ | ||||||
| class UpstreamServerName : public StreamInfo::FilterState::Object { | ||||||
| public: | ||||||
| UpstreamServerName(absl::string_view server_name) : server_name_(server_name) {} | ||||||
| const std::string& value() const { return server_name_; } | ||||||
| static const std::string Key; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this for?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is needed to get an instance of
The pattern is similar to envoy/source/common/tcp_proxy/tcp_proxy.h Line 161 in 0b2e9f5
Code in tcp_proxy.cc that processes envoy/source/common/tcp_proxy/tcp_proxy.cc Line 114 in 0b2e9f5
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'm not sure how this snuck into tcp_proxy, but we don't allow static non-PoDs in Envoy due to static initializer fiasco (see https://github.com/envoyproxy/envoy/blob/master/STYLE.md). We should probably use something like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, let me fix it also in tcp_proxy. |
||||||
|
|
||||||
| private: | ||||||
| const std::string server_name_; | ||||||
| }; | ||||||
|
|
||||||
| } // namespace Network | ||||||
| } // namespace Envoy | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
|
|
||
| #include "common/ssl/context_manager_impl.h" | ||
|
|
||
| #include "absl/types/optional.h" | ||
| #include "openssl/ssl.h" | ||
|
|
||
| namespace Envoy { | ||
|
|
@@ -41,7 +42,8 @@ struct SslStats { | |
|
|
||
| class ContextImpl : public virtual Context { | ||
| public: | ||
| virtual bssl::UniquePtr<SSL> newSsl() const; | ||
| virtual bssl::UniquePtr<SSL> | ||
| newSsl(const absl::optional<std::string>& override_server_name) const; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: making this argument an
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 505cce2 |
||
|
|
||
| /** | ||
| * Logs successful TLS handshake and updates stats. | ||
|
|
@@ -142,7 +144,8 @@ class ClientContextImpl : public ContextImpl, public ClientContext { | |
| ClientContextImpl(Stats::Scope& scope, const ClientContextConfig& config, | ||
| TimeSource& time_source); | ||
|
|
||
| bssl::UniquePtr<SSL> newSsl() const override; | ||
| bssl::UniquePtr<SSL> | ||
| newSsl(const absl::optional<std::string>& override_server_name) const override; | ||
|
|
||
| private: | ||
| const std::string server_name_indication_; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment for each method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to
serverNameOverrideor just simplyserverName; otherwise, it sounds like a mutating method vs. an option.