ipnlocal: support automatic exit node disablement when captive portal detected#13430
Open
ipnlocal: support automatic exit node disablement when captive portal detected#13430
Conversation
agottardo
force-pushed
the
angott/captive-exit-node-disablement
branch
from
f75dfe4 to
72c9f0f
Compare
andrew-d
reviewed
Sep 17, 2024
| b.mu.Lock() | ||
| defer b.mu.Unlock() | ||
| res := b.ControlKnobs().DisableExitNodeBehindCaptivePortal.Load() | ||
| b.logf("wantsExitNodeDisablementBehindCaptivePortal = %v", res) |
Member
There was a problem hiding this comment.
This could be pretty noisy in the false sense; do we want to log this at [v1], or maybe just in the true case? Especially since we log inside the if b.wantsExitNodeDisablementBehindCaptivePortal above.
| } | ||
| } | ||
|
|
||
| b.logf("routerConfig: b.captiveDetected is %v", b.captiveDetected) |
Member
There was a problem hiding this comment.
This log line is also quite verbose, IMO, since it'll get logged every time any part of the routerConfig changes.
| b.logf("captive portal detected, dropping zero routes") | ||
| // If a captive portal is present, remove the zero routes (ipv4Default and ipv6Default) | ||
| // to allow the user to authenticate with the captive portal. | ||
| rs.Routes = slices.DeleteFunc(rs.Routes, isZeroRouteFunc) |
Member
There was a problem hiding this comment.
I need to think a bit more about whether this is the right place for this, vs further up the stack; let me get back to you on that.
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
2 participants
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.
Introduces a new capability
DisableExitNodeBehindCaptivePortal. When enabled and a captive portal is detected by the detection machinery introduced in v1.72, any zero route due to an active exit node is dropped by the backend until connectivity is re-established. This allows the user to authenticate with the captive portal web UI.We're going to need another change (later) to additionally disable CorpDNS when a captive portal is detected, as in some configurations it might be impossible to reach the DNS resolvers behind the captive portal.