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

Allow ignoring incoming headers that would be reported with extractClientIP #2924

Closed
3 tasks done
jrudolph opened this issue Feb 4, 2020 · 6 comments
Closed
3 tasks done
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:core Issues related to the akka-http-core module t:model Issues around the model classes and its functionality
Milestone

Comments

@jrudolph
Copy link
Member

jrudolph commented Feb 4, 2020

Depending from whom you receive these kinds of headers, they might not be trustworthy (you might trust your reverse proxy but you cannot trust headers from any kind of client). So, you cannot distinguish headers from the client from headers by the akka-http infrastructure.

An alternative could be to put remote address information into a message attribute in 10.2.x.

When done, review documentation (also wrt to changes added in #2922).

TODO list:

@jrudolph jrudolph added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:model Issues around the model classes and its functionality t:core Issues related to the akka-http-core module labels Feb 4, 2020
@jrudolph jrudolph added this to the 10.2.0 milestone Feb 4, 2020
@raboof
Copy link
Member

raboof commented Feb 26, 2020

extractClientIP gets its data from the possibly-untrustworthy X-Forwarded-For, Remote-Address, or X-Real-IP headers. When akka.http.server.remote-address-header is on, Remote-Address is filled with a trustworthy address. That is indeed confusing.

What we could do is:

  • deprecate akka.http.server.remote-address-header
  • introduce a akka.http.server.remote-address-attribute that works like akka.http.server.remote-address-header but sets an attribute (perhaps of type RemoteAddress?) instead of a header
  • document how to get that attribute, and how to use specific X-Forwarded-For/Remote-Address/X-Real-IP headers only if you are sure you are behind a trustworthy proxy that reliably sets those headers
  • update the extractClientIP docs to clarify that it may be used for debugging/diagnostics/convenience but not for security, and link to the new docs to reliably get the client IP

changvvb pushed a commit to changvvb/akka-http that referenced this issue Apr 10, 2020
changvvb added a commit to changvvb/akka-http that referenced this issue Apr 11, 2020
changvvb added a commit to changvvb/akka-http that referenced this issue Apr 11, 2020
@jrudolph
Copy link
Member Author

New remote-address-attribute feature added in #3051.

@jrudolph
Copy link
Member Author

#3083 (review):

When deprecating the remote-address-header setting we should touch extractClientIp again and change the priority to take the attribute with highest priority. Then we should update the documentation de-emphasizing the previous setting.

@raboof
Copy link
Member

raboof commented Apr 22, 2020

we should touch extractClientIp again and change the priority to take the attribute with highest priority

I'm not sure I agree. What is the purpose of extractClientIP?

AFAICS it's not to get a safe/reliable/secure client IP: if you want to get the client IP reliably, you should use the attribute, or (if you have proxies in between) you should first make sure the proxies correctly set a certain header (which might be one of the extractClientIP ones) and use that header specifically.

extractClientIP is only there to get a 'convenient approximation' of the client IP. For that use case I think the headers are better-suited than the attribute, since it will give a better answer if there are proxies in between.

So looking at it that way, looking at the headers first and the attribute last in extractClientIP might be the right thing?

@changvvb
Copy link
Contributor

Yes, I think the header provided by proxies may be more suitable than the attribute when use extractClientIP.

@jrudolph
Copy link
Member Author

I'm not sure I agree. What is the purpose of extractClientIP?

AFAICS it's not to get a safe/reliable/secure client IP: if you want to get the client IP reliably, you should use the attribute, or (if you have proxies in between) you should first make sure the proxies correctly set a certain header (which might be one of the extractClientIP ones) and use that header specifically.

extractClientIP is only there to get a 'convenient approximation' of the client IP. For that use case I think the headers are better-suited than the attribute, since it will give a better answer if there are proxies in between.

Yes, you are right, maybe we can/should just switch Remote-Address header and remote address attribute?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:core Issues related to the akka-http-core module t:model Issues around the model classes and its functionality
Projects
None yet
Development

No branches or pull requests

3 participants