-
Notifications
You must be signed in to change notification settings - Fork 4.9k
proxy protocol: add UDP listener filter #38873
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Alek Underwood <[email protected]>
Signed-off-by: Alek Underwood <[email protected]>
Signed-off-by: Alek Underwood <[email protected]>
…d some tests. Signed-off-by: Alek Underwood <[email protected]>
… equal to Udp Signed-off-by: Alek Underwood <[email protected]>
Signed-off-by: Alek Underwood <[email protected]> Signed-off-by: Alek Underwood <>
Signed-off-by: Alek Underwood <[email protected]>
Signed-off-by: Alek Underwood <[email protected]>
Signed-off-by: Alek Underwood <[email protected]>
Signed-off-by: Alek Underwood <[email protected]> Signed-off-by: Alek Underwood <[email protected]>
Hi @aunderwood, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: Alek Underwood <[email protected]>
Drive-by review comment: please add an integration test to validate that it works as expected in a complete filter chain, and include a test where you send it a datagram without a PP header (which should get dropped). |
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.
Left minor comment regarding the API.
This filter will need a sponsor if it will be added as a non-contrib (see https://github.com/envoyproxy/envoy/blob/main/EXTENSION_POLICY.md).
Signed-off-by: Alek Underwood <[email protected]>
Added the link to the spec, and I will work on adding some integration tests. @ggreenway , since you're the maintainer for the TCP filter, are you interested in sponsoring this extension? I don't think we have the resources to contribute a full maintainer for this at the moment. I'd be happy to iterate on the design/implementation if necessary. If not I can work on moving this to contrib. |
Yes, I'll sponsor this. |
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.
You'll need to add more unit test coverage for various corner cases, and add an integration test, as well as get CI passing.
/wait
envoy.filters.udp.proxy_protocol: | ||
categories: | ||
- envoy.filters.udp_listener | ||
security_posture: robust_to_untrusted_downstream |
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.
Since this is new and unused so far, it should be unknown
. After it's had some use and is no longer alpha
it can be changed to robust_to_untrusted_downstream
@@ -43,13 +45,42 @@ class ProxyProtocolConfigFactory : public Server::Configuration::NamedListenerFi | |||
std::string name() const override { return "envoy.filters.listener.proxy_protocol"; } | |||
}; | |||
|
|||
class UdpProxyProtocolConfigFactory |
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.
All code from the new filter should be in extensions/filters/udp/proxy_protocol
. That may mean that some code needs to move to extensions/common/proxy_protocol
for things that are shared.
Commit Message: Add PROXY protocol UDP listener filter.
Additional Description: The filter decapsulates the PROXY header from each UDP datagram and rewrites the source and destination addresses of the datagram using the PROXY header. TLVs are ignored. The filter only supports PROXY protocol V2. Currently there are no configuration options for this filter. If this filter is active, non-PROXY protocol datagrams are dropped, as well as those that do not correctly specify UDP in the header's protocol field.
Risk Level: Medium
Testing: Unit tests.
Docs Changes: A doc page for the new filter.
Release Notes: N/A
Platform Specific Features: N/A
Fixes #38560