EIDA / mediatorws

EIDA NG Mediator/Federator web services
GNU General Public License v3.0
6 stars 6 forks source link

Add x forwarded header #73

Closed Jollyfant closed 3 years ago

Jollyfant commented 5 years ago

Maybe something like this? The request.access_route returns a list of all the IP addresses and proxies in between with the last one being the original address. Since our Federator is behind a proxy it was just showing our own IP when I used request.remote_addr.

Jollyfant commented 5 years ago

Or maybe the user IP address is at index 0. I'm not sure 🤷‍♂️

kaestli commented 5 years ago

Hi, isn't x-forwarded-for a header that should be added by the proxy itself? otherwise if sb. is using the federator without proxy, it is looking as having forwarded the message to itself, and if it is run behind a NAT, the header would contain the wrong (local) address...

damb commented 5 years ago

@Jollyfant,

When performing PR I'd prefer merging back into next. I'm planning to use the git-flow branching model which should make it much easier to keep the history clean. Especially when working collaboratively.

Though, I should fix this fact within some still to be done contribution guidelines ...

Jollyfant commented 5 years ago

isn't x-forwarded-for a header that should be added by the proxy itself? otherwise if sb. is using the federator without proxy, it is looking as having forwarded the message to itself, and if it is run behind a NAT, the header would contain the wrong (local) address...

Hmm.. using request.access_route I think it always includes any previous proxy IPs including itself so that should be fine. But the Federator must pass the information to the endpoints through this header in any case right?

Jollyfant commented 5 years ago

When performing PR I'd prefer merging back into next. I'm planning to use the git-flow branching model which should make it much easier to keep the history clean. Especially when working collaboratively.

Yeah sorry good point.

kaestli commented 5 years ago

I am not sure whether i have got your point.

my understanding: request.remote_addr does not return header content, but the address of the other end of the tcp connection. request.access_route returns: [any x-accessed-for header], request.remote_addr()

Thus: if a request is travelling client -> proxy -> federator, and the response federator -> proxy -> server, and if we are looking at the response:

  1. the proxy, applying these methods would get request.remote_addr(): [federator ip] request.access_route(): [federator ip]
  2. if the proxy is polite, it would add an x-accessed-for header to its response to the customer, containing the ip address of the federator
  3. the client, applying these methods to the response he gets from the proxy, would get: request.remote_addr(): [proxy_ip] request.access_route(): [proxy_ip, federator_ip from (2)] (not sure about the ordering) However, the client cannot be sure where the response actually came from, because proxies may hide their nature, and do this for good reasons: Imagine you are using a load balancer (which is nothing else than a proxy) for two service (such as federator) instances at localhost:8000 and localhost:8001, it would not make sense to pass the information back to the client that the response is initially coming from "localhost".
Jollyfant commented 5 years ago

I'm a little confused too about the discussion. The point is that the X-Forwarded-For header is not for the client. If it for the EIDA node that the Federator makes the request to so that their logging shows the original client, and not just the Federator IP.

Client -> Proxy -> Federator -> Endpoint

If the Proxy and Federator both respect the X-Forwared-For the result should be that the original client IP should be passed to the endpoint.

kaestli commented 5 years ago

Ok. i got it. it is kind of cheating as, in most cases, the request coming from the federator to the end point is different from the one coming from the client to the federator. but it is probably a quite pragmatic idea...