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

Protect InternetChecker from being tricked by a captive portal #461

Merged
merged 1 commit into from
May 15, 2023

Conversation

fhunleth
Copy link
Member

This is a partial fix for the InternetChecker to keep it from being
tricked by a captive portal that resolves DNS queries to itself. What
was happening was that the remote host that was being checked got
redirected to the captive portal. Since the ping test is simplistic, it
only makes a TCP connection to the specified host and port, it was
successful due to the port being 443. This made the InternetChecker
think that it had connected to a remote host and it hadn't. The updated
behavior is to verify that the IP address to check is not on the LAN
(off subnet). This, of course, isn't perfect since the captive portal
can make up off-LAN IP addresses just as easy as local ones, but the
hope is that it is a low risk way of reducing false positives.

This is a partial fix for the InternetChecker to keep it from being
tricked by a captive portal that resolves DNS queries to itself. What
was happening was that the remote host that was being checked got
redirected to the captive portal. Since the ping test is simplistic, it
only makes a TCP connection to the specified host and port, it was
successful due to the port being 443. This made the InternetChecker
think that it had connected to a remote host and it hadn't. The updated
behavior is to verify that the IP address to check is not on the LAN
(off subnet). This, of course, isn't perfect since the captive portal
can make up off-LAN IP addresses just as easy as local ones, but the
hope is that it is a low risk way of reducing false positives.
@fhunleth fhunleth requested a review from jjcarstens May 12, 2023 20:34
@fhunleth fhunleth merged commit c5808c6 into main May 15, 2023
@fhunleth fhunleth deleted the captive-portal-fix branch May 15, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants