-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
matchers: add input types for network streams #19493
Changes from 7 commits
aea58eb
f4ed5de
41e60d6
89eaf62
ebb0634
b8b15f8
7427533
35850bb
eba5305
2eeae31
eeef2bf
8da956e
93fd7fb
bc14d3e
a35a28a
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,87 @@ | ||
syntax = "proto3"; | ||
|
||
package envoy.type.matcher.v3; | ||
|
||
import "udpa/annotations/status.proto"; | ||
import "validate/validate.proto"; | ||
|
||
option java_package = "io.envoyproxy.envoy.type.matcher.v3"; | ||
option java_outer_classname = "NetworkInputsProto"; | ||
option java_multiple_files = true; | ||
option go_package = "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3;matcherv3"; | ||
option (udpa.annotations.file_status).package_version_status = ACTIVE; | ||
|
||
// [#protodoc-title: Common Network Matching Inputs] | ||
|
||
// Specifies that matching should be performed by the destination IP address. | ||
message DestinationIPInput { | ||
} | ||
|
||
// Specifies that matching should be performed by the destination port. | ||
message DestinationPortInput { | ||
} | ||
|
||
// Specifies that matching should be performed by the source IP address. | ||
message SourceIPInput { | ||
} | ||
|
||
// Specifies that matching should be performed by the source port. | ||
message SourcePortInput { | ||
} | ||
|
||
// Input that matches by the directly connected source IP address (this | ||
// will only be different from the source IP address when using a listener | ||
// filter that overrides the source address, such as the :ref:`Proxy Protocol | ||
// listener filter <config_listener_filters_proxy_protocol>`). | ||
message DirectSourceIPInput { | ||
} | ||
|
||
// Input that matches by the source IP type. | ||
// Specifies the source IP match type. The values include: | ||
// | ||
// * ``local`` - matches a connection originating from the same host, | ||
message SourceTypeInput { | ||
} | ||
|
||
// Input that matches by the requested server name (e.g. SNI in TLS), when detected by one of the listener filters. | ||
// | ||
// :ref:`TLS Inspector <config_listener_filters_tls_inspector>` provides the requested server name based on SNI. | ||
message ServerNameInput { | ||
} | ||
|
||
// Input that matches by the transport protocol, when | ||
// detected by one of the listener filters. | ||
// | ||
// Suggested values include: | ||
// | ||
// * ``raw_buffer`` - default, used when no transport protocol is detected, | ||
// * ``tls`` - set by :ref:`envoy.filters.listener.tls_inspector <config_listener_filters_tls_inspector>` | ||
// when TLS protocol is detected. | ||
message TransportProtocolInput { | ||
} | ||
|
||
// List of quoted and comma-separated requested application protocols, when detected by one of the listener filters. | ||
// Examples: | ||
// | ||
// * ``'h2','http/1.1'`` | ||
// * ``'h2c'``` | ||
// | ||
// Suggested values in the list include: | ||
// | ||
// * ``http/1.1`` - set by :ref:`envoy.filters.listener.tls_inspector | ||
// <config_listener_filters_tls_inspector>` and :ref:`envoy.filters.listener.http_inspector | ||
// <config_listener_filters_http_inspector>`, | ||
// * ``h2`` - set by :ref:`envoy.filters.listener.tls_inspector <config_listener_filters_tls_inspector>` | ||
// * ``h2c`` - set by :ref:`envoy.filters.listener.http_inspector <config_listener_filters_http_inspector>` | ||
// | ||
// .. attention:: | ||
// | ||
// Currently, :ref:`TLS Inspector <config_listener_filters_tls_inspector>` provides | ||
// application protocol detection based on the requested | ||
// `ALPN <https://en.wikipedia.org/wiki/Application-Layer_Protocol_Negotiation>`_ values. | ||
// | ||
// However, the use of ALPN is pretty much limited to the HTTP/2 traffic on the Internet, | ||
// and matching on values other than ``h2`` is going to lead to a lot of false negatives, | ||
// unless all connecting clients are known to use ALPN. | ||
message ApplicationProtocolInput { | ||
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. @snowp This field is problematic because it's a list of values. Do we have any support for "list" inputs? I think comma-separate join is fine, except it's hard to distinguish between "h2" and "h2c" with string matching. 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 made them quoted to make it easier to say "first element is h2, not h2c". 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. When this came up originally there was talks about an aggregate matcher that would allow applying a matcher against elements in the list of elements (ContainsMatching, AllMatching, etc.), though we never implemented that cc @markdroth 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. There is an implementation problem in that input is always a string when passed to matchers. I think we need either a dynamic value (e.g. something like CelValue variant) or typed inputs/matchers. Given the constraint, let's use a string serialization for now? We can always add another input once lists are supported. 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 recall that we talked about making it templatized so that the matcher framework could support types other than string. I'm not sure how much implementation work is involved in doing that, but it sounds like the right approach. In the short term, I don't have a conceptual problem with using a string serialization, but I think we're going to get into situations where the overhead of serializing and deserializing is way too high to be efficient. So I would like to see us move to a templatized approach before that happens. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
load( | ||
"//bazel:envoy_build_system.bzl", | ||
"envoy_cc_library", | ||
"envoy_package", | ||
) | ||
|
||
licenses(["notice"]) # Apache 2 | ||
|
||
envoy_package() | ||
|
||
envoy_cc_library( | ||
name = "data_impl_lib", | ||
hdrs = ["data_impl.h"], | ||
deps = [ | ||
"//envoy/network:filter_interface", | ||
], | ||
) | ||
|
||
envoy_cc_library( | ||
name = "inputs_lib", | ||
srcs = ["inputs.cc"], | ||
hdrs = ["inputs.h"], | ||
deps = [ | ||
"//envoy/matcher:matcher_interface", | ||
"//envoy/network:filter_interface", | ||
"//envoy/registry", | ||
"//source/common/network:utility_lib", | ||
"@envoy_api//envoy/type/matcher/v3:pkg_cc_proto", | ||
], | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
#pragma once | ||
|
||
#include "envoy/network/filter.h" | ||
|
||
namespace Envoy { | ||
namespace Network { | ||
namespace Matching { | ||
|
||
/** | ||
* Implementation of Network::MatchingData, providing connection-level data to | ||
* the match tree. | ||
*/ | ||
class MatchingDataImpl : public MatchingData { | ||
public: | ||
MatchingDataImpl(const ConnectionSocket& socket) : socket_(socket) {} | ||
kyessenov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
static absl::string_view name() { return "network"; } | ||
kyessenov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const ConnectionSocket& socket() const override { return socket_; } | ||
|
||
private: | ||
const ConnectionSocket& socket_; | ||
}; | ||
|
||
} // namespace Matching | ||
} // namespace Network | ||
} // namespace Envoy |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
#include "source/common/network/matching/inputs.h" | ||
|
||
#include "envoy/registry/registry.h" | ||
|
||
#include "source/common/network/utility.h" | ||
|
||
#include "absl/strings/str_cat.h" | ||
|
||
namespace Envoy { | ||
namespace Network { | ||
namespace Matching { | ||
|
||
Matcher::DataInputGetResult DestinationIPInput::get(const MatchingData& data) const { | ||
const auto& address = data.socket().connectionInfoProvider().localAddress(); | ||
if (address->type() != Network::Address::Type::Ip) { | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt}; | ||
} | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, | ||
address->ip()->addressAsString()}; | ||
} | ||
|
||
Matcher::DataInputGetResult DestinationPortInput::get(const MatchingData& data) const { | ||
const auto& address = data.socket().connectionInfoProvider().localAddress(); | ||
if (address->type() != Network::Address::Type::Ip) { | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt}; | ||
} | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, | ||
absl::StrCat(address->ip()->port())}; | ||
} | ||
|
||
Matcher::DataInputGetResult SourceIPInput::get(const MatchingData& data) const { | ||
const auto& address = data.socket().connectionInfoProvider().remoteAddress(); | ||
if (address->type() != Network::Address::Type::Ip) { | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt}; | ||
} | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, | ||
address->ip()->addressAsString()}; | ||
} | ||
|
||
Matcher::DataInputGetResult SourcePortInput::get(const MatchingData& data) const { | ||
const auto& address = data.socket().connectionInfoProvider().remoteAddress(); | ||
if (address->type() != Network::Address::Type::Ip) { | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt}; | ||
} | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, | ||
absl::StrCat(address->ip()->port())}; | ||
} | ||
|
||
Matcher::DataInputGetResult DirectSourceIPInput::get(const MatchingData& data) const { | ||
const auto& address = data.socket().connectionInfoProvider().directRemoteAddress(); | ||
if (address->type() != Network::Address::Type::Ip) { | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt}; | ||
} | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, | ||
address->ip()->addressAsString()}; | ||
} | ||
|
||
Matcher::DataInputGetResult SourceTypeInput::get(const MatchingData& data) const { | ||
const bool is_local_connection = Network::Utility::isSameIpOrLoopback(data.socket()); | ||
if (is_local_connection) { | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, "local"}; | ||
} | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt}; | ||
} | ||
|
||
Matcher::DataInputGetResult ServerNameInput::get(const MatchingData& data) const { | ||
const auto server_name = data.socket().requestedServerName(); | ||
if (!server_name.empty()) { | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, | ||
std::string(server_name)}; | ||
} | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt}; | ||
} | ||
|
||
Matcher::DataInputGetResult TransportProtocolInput::get(const MatchingData& data) const { | ||
const auto transport_protocol = data.socket().detectedTransportProtocol(); | ||
if (!transport_protocol.empty()) { | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, | ||
std::string(transport_protocol)}; | ||
} | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt}; | ||
} | ||
|
||
Matcher::DataInputGetResult ApplicationProtocolInput::get(const MatchingData& data) const { | ||
const auto& protocols = data.socket().requestedApplicationProtocols(); | ||
if (!protocols.empty()) { | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, | ||
absl::StrCat("'", absl::StrJoin(protocols, "','"), "'")}; | ||
} | ||
return {Matcher::DataInputGetResult::DataAvailability::AllDataAvailable, absl::nullopt}; | ||
} | ||
|
||
REGISTER_FACTORY(DestinationIPInputFactory, Matcher::DataInputFactory<MatchingData>); | ||
REGISTER_FACTORY(DestinationPortInputFactory, Matcher::DataInputFactory<MatchingData>); | ||
REGISTER_FACTORY(SourceIPInputFactory, Matcher::DataInputFactory<MatchingData>); | ||
REGISTER_FACTORY(SourcePortInputFactory, Matcher::DataInputFactory<MatchingData>); | ||
REGISTER_FACTORY(DirectSourceIPInputFactory, Matcher::DataInputFactory<MatchingData>); | ||
REGISTER_FACTORY(SourceTypeInputFactory, Matcher::DataInputFactory<MatchingData>); | ||
REGISTER_FACTORY(ServerNameInputFactory, Matcher::DataInputFactory<MatchingData>); | ||
REGISTER_FACTORY(TransportProtocolInputFactory, Matcher::DataInputFactory<MatchingData>); | ||
REGISTER_FACTORY(ApplicationProtocolInputFactory, Matcher::DataInputFactory<MatchingData>); | ||
|
||
} // namespace Matching | ||
} // namespace Network | ||
} // namespace Envoy |
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.
Are these matchers going to be used anywhere other than in
Listener
? If so, this is definitely the right place for them. But if not, I wonder if it would be more appropriate to put them somewhere in config/listener/v3.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.
I think so. RBAC uses port input, for example, which is not really an HTTP property. All of these could be read from HTTP matching data, and probable deserves a follow up. I should rephrase the comments to be less about listeners.
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.
Another option would be to express this as extensions (e.g. api/extensions), that might help avoid bloating envoy/type/matcher?
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.
I think extensions is fine, too. Is
envoy/extensions/matching/common_inputs/network
good enough?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.
That path sounds fine.
Hmm. Can we put this in the cncf/xds repo? The more new things we create there, the less we'll have to migrate over time. You can create an
extensions/matching/common_inputs/network
tree there. Is that going to cause any problems for Envoy extension documentation? CC @envoyproxy/api-shepherdsThere 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.
Moved to extensions. I am fine moving over to CNCF. I think some are somewhat envoy-specific.
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.
I think we need some guidance on which inputs should go to CNCF and which ones should reside in envoy. In particular, the ALPN list is awkward right now, and the "direct" addresses assume a capability to override addresses by a client.