Skip to content
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

Support dynamic host rewrite and Proxy-Authorization Header injection via SDS in TCPProxy tunneling #13809

Closed
larryrun opened this issue Oct 29, 2020 · 25 comments
Assignees
Labels
area/tcp_proxy enhancement Feature requests. Not bugs or questions. stale stalebot believes this issue/PR has not been touched recently

Comments

@larryrun
Copy link

Hi team,

We use the TCPProxy filter to tunneling TCP traffic to upstream services. The upstream service is also an Envoy, it does the authn/authz check and then forward the packets to the real port.

When a client calls a service, the traffic flow looks like :
[Client(call 2.2.2.2:80)] -> [Iptables redirect to :8000] -> [ClientSide Envoy TCPProxy (listens on 8000, Will do tunneling based on the client request: 2.2.2.2:80)] -> [Iptables redirect to :9000] -> [ServiceSide Envoy(listens on 9000, forwards to 2.2.2.2:80)] -> [Service(listens on 80)]

Problems we are facing:

  • The TCPProxy tunneling config only supports a hard coded host, but in our case the upstream endpoint is depending on the client request. so it will be helpful if this host field can be configured to a dynamic value, like %DOWNSTREAM_LOCAL_ADDRESS%
  • An additional Proxy-Authorization header need to be injected

To address the above difficulties. we configured a second listener(just like this example in the offical doc) in the client side Envoy with HTTP filters, and redirect the packets sent from TCPProxy filter to this listener. And use

  • request_headers_to_add=%DOWNSTREAM_LOCAL_ADDRESS% to add the Host(:authority) Header;
  • ext_authz filter to inject the Proxy-Authorization header (the value is a token).

So we are wondering, is this the right way to achieve our requirement in your opinion? We see there are some improvements can be made:

  • As the second listener is needed, extra unnecessary packets are generated, this seems to impact the TCP performance.
  • The token injection is implemented by using an ext_authz filter, not via an SDS which looks like a more appropriate way of handling token values

One option that came to my mind is that, allowing an HTTP filter chain to be configured for the TCPProxy Tunneling's HTTP CONNECT request, so that we can add more sophisticated HTTP logic.

For the token injection, I found another helpful feature request#6654 that was created long time ago. Is there any plan for it?

What do you guys think?
Thanks for your hard working and this great project!

@larryrun larryrun added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Oct 29, 2020
@yanavlasov yanavlasov added area/tcp_proxy and removed triage Issue requires triage labels Oct 29, 2020
@yanavlasov
Copy link
Contributor

I think this is a problem that @lambdai is trying to solve as well. @alyssawilk for any more comments as well.

@alyssawilk
Copy link
Contributor

Hm, I like the idea of allowing a filter chain for tcp tunneled over HTTP.
It smells more like upstream filters to me, which would imply using the work @snowp is doing there.
Alternately a simpler but hacker solution (which wouldn't go upstream but you could use in the interim) I think you could use pluggable upstreams (almost landed -see #13548) to put your own business logic in your own custom http-over-tcp handler.

@larryrun
Copy link
Author

@alyssawilk @yanavlasov thanks for your reply!
I'll check @snowp and @lambdai 's work, and I'll also see how the pluggable upstreams can help.
And for the token via SDS, any ideas for this feature request#6654 ?

@github-actions
Copy link

github-actions bot commented Dec 9, 2020

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 9, 2020
@mattklein123 mattklein123 removed the stale stalebot believes this issue/PR has not been touched recently label Dec 9, 2020
@github-actions
Copy link

github-actions bot commented Jan 8, 2021

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jan 8, 2021
@github-actions
Copy link

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@cortex93
Copy link

cortex93 commented Sep 7, 2021

Hi @larryrun, we are trying to build a dynamic upstream TLS proxy configuration. We have a working static solution here https://gist.github.com/cortex93/6d7cd9738b077d0447ab891cbacd0d5e but we need to get rid of host_rewrite_literal in HCM and hostname in tunneling_config.
Use case is Client --(http://dynamic.target/)--> envoy L1 --(TLS Upstream)--> envoy L2 --(TcpProxy passthrough to https://corporate.proxy)--> Corporate Proxy --(CONNECT dynamic.target)--> dynamic.target
It seems that you succeed in some way to bypass the limitation of TcpProxy. Do you think it can be adapted ?

@cortex93
Copy link

cortex93 commented Oct 3, 2021

@alyssawilk regarding my previous comment, would you mind reopen this ?

We should be able to do TLS upstream with a non statically configured hostname

@alyssawilk alyssawilk reopened this Oct 4, 2021
@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Oct 4, 2021
@larryrun
Copy link
Author

hi @cortex93 in your case, are you using the iptables to redirect the TCP packets? how do you pass the host params?

@cortex93
Copy link

hi @cortex93 in your case, are you using the iptables to redirect the TCP packets? how do you pass the host params?

Unfortunately, we are not able to route with iptables.
What we do, to make it as transparent as possible, is to define the target host as alias in /etc/hosts to 127.0.0.1 and envoy listen on localhost:80. So envoy receives the request as if it was the final target and do the TLS Upstream and forward through the corporate proxy (HTTP/HTTPS between envoy and the proxy depend on the target hostname).

@lambdai
Copy link
Contributor

lambdai commented Oct 13, 2021

I am working on a task that populates upstream header values. Described in this issue
#18128

It will be a simple PR following #18563

@larryrun
Copy link
Author

@cortex93 in your case, seems there's no way to send the destination info (dest ip or dest host).
As @lambdai 's helping here. Let's wait for his PR.

@scrocquesel
Copy link
Contributor

@lambdai Now that #18128 is closed, would it be possible to also evaluate hostname in

{Http::Headers::get().Host, config_.hostname()},

and
{Http::Headers::get().Host, config_.hostname()},

@github-actions
Copy link

github-actions bot commented Dec 3, 2021

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 3, 2021
@scrocquesel
Copy link
Contributor

There is an ongoing discussion at #18838

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Dec 3, 2021
@github-actions
Copy link

github-actions bot commented Jan 3, 2022

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jan 3, 2022
@scrocquesel
Copy link
Contributor

@lambdai any ongoing work around evaluating the hostname of tunneling_config ? Should another issue be created ?

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Jan 4, 2022
@github-actions
Copy link

github-actions bot commented Feb 3, 2022

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Feb 3, 2022
@scrocquesel
Copy link
Contributor

Still not fixed

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Feb 3, 2022
@github-actions
Copy link

github-actions bot commented Mar 5, 2022

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Mar 5, 2022
@lambdai
Copy link
Contributor

lambdai commented Mar 9, 2022

/assign lambdai

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Mar 9, 2022
@github-actions
Copy link

github-actions bot commented Apr 8, 2022

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Apr 8, 2022
@github-actions
Copy link

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@scrocquesel
Copy link
Contributor

@lambdai any progress on this one ?

@scrocquesel
Copy link
Contributor

dynamic hostname in tcpproxy is being adressed by #19612

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tcp_proxy enhancement Feature requests. Not bugs or questions. stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

No branches or pull requests

7 participants