transport_socket(http_11_proxy): add Proxy-Authorization header support - #46675
transport_socket(http_11_proxy): add Proxy-Authorization header support#46675glennc24 wants to merge 4 commits into
Conversation
This patch adds support in HTTP/1.1 Proxy for HTTP proxy authorization. The HTTP/1.1 Proxy looks up the encoded credentials in its host's typed filter metadata and sends it with the Proxy-Authorization HTTP header in the CONNECT request to the proxy. Proxy authorization is still unsupported when filter state metadata is used. Signed-off-by: Glenn Chen <glenn.chen@nutanix.com>
Signed-off-by: Glenn Chen <glenn.chen@nutanix.com>
Signed-off-by: Glenn Chen <glenn.chen@nutanix.com>
|
/coverage |
|
Coverage for this Pull Request will be rendered here: https://storage.googleapis.com/envoy-cncf-pr/46675/coverage/index.html For comparison, current coverage on https://storage.googleapis.com/envoy-cncf-postsubmit/main/coverage/index.html The coverage results are (re-)rendered each time the CI |
tonya11en
left a comment
tonya11en
left a comment
There was a problem hiding this comment.
Thanks for the contribution. A few small comments.
| if (authentication.empty()) { | ||
| header_buffer_.add( | ||
| absl::StrCat("CONNECT ", host->address()->asStringView(), " HTTP/1.1\r\n\r\n")); | ||
| } else { | ||
| header_buffer_.add(absl::StrCat("CONNECT ", host->address()->asStringView(), " HTTP/1.1\r\n", | ||
| "Proxy-Authorization: ", authentication, "\r\n\r\n")); | ||
| } |
There was a problem hiding this comment.
I think this nesting is getting out of hand. Can you pull out this and similar string building logic above into a common helper function?
| // Proxy-Authorization header value for HTTP/1.1 proxy transport sockets. | ||
| // When present, the value (a google.protobuf.StringValue) is added as a | ||
| // "Proxy-Authorization" header in the HTTP/1.1 CONNECT request. | ||
| const std::string ENVOY_HTTP11_PROXY_TRANSPORT_SOCKET_AUTH = | ||
| "envoy.http11_proxy_transport_socket.proxy_authorization"; |
There was a problem hiding this comment.
Can you please update the documentation with this.
There was a problem hiding this comment.
Added a comment in upstream_http11_connect.proto describing the proxy_authorization key. Please let me know if you had another location in mind.
| Config::MetadataFilters::get().ENVOY_HTTP11_PROXY_TRANSPORT_SOCKET_AUTH); | ||
| if (auth_it != host->metadata()->typed_filter_metadata().end()) { | ||
| Protobuf::StringValue auth_value; | ||
| if (MessageUtil::unpackTo(auth_it->second, auth_value).ok()) { |
There was a problem hiding this comment.
If this fails, we should emit some kind of trace log.
rework formatConnectRequest() Signed-off-by: Glenn Chen <glenn.chen@nutanix.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
3 participants
Commit Message: This patch adds support in HTTP/1.1 Proxy for HTTP proxy authorization.
The HTTP/1.1 Proxy looks up the encoded credentials in its host's
typed filter metadata and sends it with the Proxy-Authorization HTTP
header in the CONNECT request to the proxy.
Proxy authorization is still unsupported when filter state metadata is
used.
Additional Description: AI used to write tests
Risk Level: Low
Testing: Unit and integration test added
Docs Changes: No
Release Notes:
Platform Specific Features: No