Add extractDirectClientIP for the client address that cannot be spoofed - #1219
Open
pjfanning wants to merge 1 commit into
Open
Add extractDirectClientIP for the client address that cannot be spoofed#1219pjfanning wants to merge 1 commit into
pjfanning wants to merge 1 commit into
Conversation
Motivation: `extractClientIP` prefers the X-Forwarded-For and X-Real-IP headers over the remote address attribute. Those headers are set by whoever sent the request, so unless a trusted proxy in front of the server overwrites them the client picks the address the application sees. That is the intended behaviour behind a proxy, but there is no equally convenient directive for the cases where the address must not be under the client's control, and the scaladoc and javadoc of `extractClientIP` do not mention the difference at all (only the docs page does). Modification: Add `extractDirectClientIP` to both DSLs, extracting the `AttributeKeys.remoteAddress` attribute alone, and point at it from the scaladoc and javadoc of `extractClientIP`. Document the new directive with its own page and examples, and cross-link it from the warning on the `extractClientIP` page. Result: Applications that need the address of the actual peer, for example for access control or rate limiting, can ask for it directly, and the distinction is visible where the directives are used rather than only on the documentation site. `extractClientIP` is unchanged. Tests: - sbt "http-tests/testOnly org.apache.pekko.http.scaladsl.server.directives.MiscDirectivesSpec" - pass, 4 new tests including one that asserts the forwarding headers are ignored - sbt http-tests/test - pass - sbt docs/test - pass, with new Scala and Java examples - sbt docs/paradox - pass - sbt +http/mimaReportBinaryIssues - pass - sbt http/scalafmt http-tests/Test/scalafmt docs/Test/scalafmt docs/Test/javafmt - clean References: None - makes the address that a client cannot spoof directly available
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
extractClientIPreadsX-Forwarded-For, thenX-Real-IP, and only then theAttributeKeys.remoteAddressattribute. Those headers are set by whoever sent the request, so unless a trusted proxy in front of the server overwrites them, the client decides which address the application sees. That is exactly what you want behind a proxy, but for uses where the address must not be under the client's control — access control, rate limiting, audit logs — there is no equally convenient directive, and reaching for the attribute means writing the extraction by hand.The documentation page for
extractClientIPdoes carry a warning about this, but the scaladoc and javadoc of the directive itself say nothing, which is where most people meet it.Modification
extractDirectClientIPto both DSLs. It extracts theAttributeKeys.remoteAddressattribute alone and ignores forwarding headers, so a client cannot choose the value; behind a proxy it is the address of the proxy rather than of the client.extractClientIPand point at the new directive.extractClientIPpage.extractClientIPitself is unchanged, and its fallback now simply reuses the new directive.Result
The address that a client cannot spoof is available as a one-liner, and the distinction between the two directives is visible in the API docs rather than only on the documentation site.
Naming note: I went with
extractDirectClientIPto say "the peer of this connection" without implying it is the end user's address. Happy to rename if you prefer something else.Tests
sbt "http-tests/testOnly org.apache.pekko.http.scaladsl.server.directives.MiscDirectivesSpec"- pass; 4 new tests: the attribute is extracted,X-Forwarded-ForandX-Real-IPsent by a client are ignored,Unknownwhen the attribute is not set, plus one forextractClientIPfalling back to the attributesbt http-tests/test- pass (1489 tests)sbt docs/test- pass, covering the new Scala and Java examplessbt docs/paradox- pass; the new page renders with its signature and both examplessbt +http/mimaReportBinaryIssues- pass on 2.13 and 3sbt http/scalafmt http-tests/Test/scalafmt docs/Test/scalafmt docs/Test/javafmt- cleanReferences
None - makes the address that a client cannot spoof directly available