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

Source upstream peer server / downstream peer client IP address from within Lua #2539

Closed
thebadpete opened this issue Feb 6, 2018 · 9 comments
Labels
enhancement Feature requests. Not bugs or questions. stale stalebot believes this issue/PR has not been touched recently

Comments

@thebadpete
Copy link

Description:

The current Envoy Lua plugin does not expose request info like upstream peer server or downstream peer client IP addresses. The only way we found is to set use_remote_address to true for http_connection_manager so that but the time lua plugin is invoked x-envoy-external-address is set, if the client is not considered internal. But there doesn't appear to have any way to do something similar for upstream server.

We have a bunch of logic in C-based ATS plugin code that hooks to different states of the ATS request/response state machine to check these IP addresses and add/remove custom request/response headers. We are porting the logic to run on Envoy, and we would like to take advantage of the Envoy Lua plugin + FFI to reuse some of them as much as possible. If we can source the peer upstream/downstream IP addresses, we can cross-check them with internally-maintained database (loaded into mdbm at runtime) to do our own whether-ip-is-internal checks, mandated by my company.

@mattklein123 mattklein123 added enhancement Feature requests. Not bugs or questions. help wanted Needs help! labels Feb 6, 2018
@mattklein123
Copy link
Member

We will need to Lua wrap RequestInfo and make it available to Lua. We can start with just a small portion of it since it's ultimately a big structure. Marking this as help wanted in case anyone out there would like to contribute. If so, I can provide guidance.

@thebadpete
Copy link
Author

I would like to contribute but certainly needs some time to ramp up. Would appreciate any guidance!

@mattklein123
Copy link
Member

@thebadpete here are some notes on how to implement this:

  1. The interface that you want to pull information from is here: https://github.com/envoyproxy/envoy/blob/master/include/envoy/request_info/request_info.h. Specifically, you will want information from upstreamHost().
  2. This is accessed from filters here: https://github.com/envoyproxy/envoy/blob/master/include/envoy/http/filter.h#L129.
  3. The Lua HTTP filter is here: https://github.com/envoyproxy/envoy/blob/master/source/common/http/filter/lua/lua_filter.h
  4. Common Lua wrappers are here: https://github.com/envoyproxy/envoy/blob/master/source/common/lua/wrappers.h. We will want a wrapper for request info. Potentially initially just exposing the information you need as a getter.
  5. You would likely end up exposing a new stream handler API called requestInfo from here: https://github.com/envoyproxy/envoy/blob/master/source/common/http/filter/lua/lua_filter.h#L97

Also note that if you already have C code you might be better off just writing a C++ filter directly.

@thebadpete
Copy link
Author

thebadpete commented Feb 15, 2018

@mattklein123 I took a stab at exposing request info. Looks like the StreamHandleWrapper is created at (1.5.0 tag)

https://github.com/envoyproxy/envoy/blob/v1.5.0/source/common/http/filter/lua/lua_filter.cc#L423

And the StreamDecoder/EncoderFilterCallbacks are set earlier via

https://github.com/envoyproxy/envoy/blob/v1.5.0/source/common/http/filter/lua/lua_filter.h#L249
https://github.com/envoyproxy/envoy/blob/v1.5.0/source/common/http/filter/lua/lua_filter.h#L264

What I found out is that with an envoy config where http lua filters run first, when I try to get a Upstream::HostDescriptionConstSharedPtr pointer from AccessLog::RequestInfo's upstreamHost(), I get a null pointer when requests come in, so I have no upstream remote IP that I can pass to lua scripts via StreamHandleWrapper. But later, when envoy_on_response is invoked from Lua, from the same doHeaders() callback, this time I get a valid Upstream::HostDescriptionConstSharedPtr and I get the IP, but it is too late, since I need that during envoy_on_request().

If I switch the filter order so that route filter goes 1st, then I notice that envoy_on_request is totally skipped. I am not sure if this is an issue or by design.

I may not be doing things correctly, but how do I get the remote upstream IP in C++ space and pass it to envoy_on_request()?

@mattklein123
Copy link
Member

@thebadpete the issue is that the upstream host is not selected until the router is run. What you might be looking for is actually some type of filtering functionality on the upstream requests themselves? This has been asked for several times and I think we will do this eventually but it's a really complicated feature. What are you trying to do exactly?

@thebadpete
Copy link
Author

Specifically we are porting some internal ATS plugin functionality over to Envoy as we want to adopt Envoy internally at my company. However our security folks enforce their own IP checks to determine if a proxied request is internal or not. And we are required to add the caller's IP for tracing purposes via custom request headers.

In ATS world, the request/response flow can be captured in a so-called transaction state diagram and folks can write plugins to plug into each state to do custom stuff:

https://docs.trafficserver.apache.org/en/7.1.x/developer-guide/plugins/hooks-and-transactions/index.en.html?highlight=state%20diagram#http-txn-state-diagram

The original ATS logic that I am porting plugs into so-called READ_REQUEST_* and SEND_REQUEST_HDR events to do custom checks of IP addresses to add/update/remove custom request headers. For the logic hooked to SEND_REQUEST_HDR, the logic looks up the peer upstream server IP address and call the custom check libraries provided by the security team to determine what header manipulations we should take. Note that at the SEND_REQUEST_HDR state, the request is actually not being made yet.

@shukitchan anything to add?

@mattklein123
Copy link
Member

@thebadpete unfortunately this is not currently possible and is basically tracked in this issue: #173. Ultimately we would like to add filter capabilities for upstream requests (essentially driven via the router), but this is not currently implemented and is a large/complicated feature.

You could trivially add this behavior by modifying the router filter but not sure if you want to run a custom patch or not.

Shikugawa pushed a commit to Shikugawa/envoy that referenced this issue Mar 28, 2020
Shikugawa pushed a commit to Shikugawa/envoy that referenced this issue Mar 28, 2020
jpsim pushed a commit that referenced this issue Nov 28, 2022
Description: Make it easy to run all of the example apps. Previously it was possible to use Android Studio to run Kotlin example app. This PR adds support for: Java example app, baseline test example app and experimental test example app.
Risk Level: None, touches example apps only
Testing: Manual
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this issue Nov 29, 2022
Description: Make it easy to run all of the example apps. Previously it was possible to use Android Studio to run Kotlin example app. This PR adds support for: Java example app, baseline test example app and experimental test example app.
Risk Level: None, touches example apps only
Testing: Manual
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
@alyssawilk alyssawilk removed the help wanted Needs help! label Oct 16, 2024
Copy link

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 Nov 15, 2024
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.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

3 participants