This repository was archived by the owner on Apr 30, 2025. It is now read-only.
Merged
Conversation
b375f40 to
b59864f
Compare
b59864f to
c4ff757
Compare
Merged
5 tasks
domdom82
suggested changes
Aug 11, 2022
0bfdcf2 to
8135ee8
Compare
8135ee8 to
90c3e73
Compare
domdom82
suggested changes
Aug 12, 2022
Co-authored-by: Dominik Froehlich <Dominik.Frolic@gmail.com>
domdom82
reviewed
Aug 12, 2022
domdom82
approved these changes
Aug 12, 2022
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Gorouter route service hairpinning / internal resolution had caused CVE-2019-3789. The main issue was that external domains can be hijacked by declaring an external domain as 'private domain' and mapping it to a route service, which would mask the request to the external resource.
In #281, we proposed do add an allowlist for route service host names that are allowed to be resolved internally via Gorouter's route registry.
This PR implements the new property and handling of allowlists.
This PR allows enabling Gorouter internal route serice resolution / hairpinning for a subset of safe domains. This subset is defined in the allowlist and is under the operator's control.
The functionality is covered via unit tests.
Otherwise, a route service can be declared and the route's host added to the hairpinning allowlist. An app configured with the route service will result in a call to the route service resolved internally instead of externally. This should be visible, e.g. in the LB logs: i.e. the request issued by gorouter will not be shown on the LB.
When hairpinning is disabled, the behaviour remains the same as before, i.e. route service requests are carried out via external request. This is the default.
When hairpinning is enabled and an allowlist is set, only route service URLs, whose host name matches one of the entries on the allowlist will be resolved internally. Other requests will be requested externally in order to avoid CVE-2019-3789.
When hairpinning is enabled and no allowlist is set, the allowlist is ignored and all route service requests that can be resolved internally via the route registrar are resolved internally. The Gorouter is subject to CVE-2019-3789.
When hairpinning is disabled, all route service requests lead to external requests. This incurs another hop on the ingress and potential added delay of the overall request.
When hairpinning is enabled, all route service requests that can be resolved internally via the route registrar are resolved internally. The Gorouter is subject to CVE-2019-3789.
Links to any other associated PRs
I have viewed signed and have submitted the Contributor License Agreement
I have made this pull request to the
mainbranchI have run all the unit tests using
scripts/run-unit-tests-in-dockerfrom routing-release.(Optional) I have run Routing Acceptance Tests and Routing Smoke Tests on bosh lite
(Optional) I have run CF Acceptance Tests on bosh lite