Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 48565ecf authored by Remi NGUYEN VAN's avatar Remi NGUYEN VAN
Browse files

Refine DNS private IP probes 

This addresses comments on the parent change that introduced the
optional feature.
This change also considers IPv6 ULAs as private addresses, corrects some
style and comments.

Bug: 136734947
Test: atest NetworkStackTests:NetworkMonitorTest \
          NetworkStackTests:NetworkStackUtilsTest
Change-Id: If4231804b77143b78f26a32c16db53fc4ac44cd8
parent 4915fafd

common/captiveportal/src/android/net/captiveportal/CaptivePortalProbeResult.java

+5 −4
Original line number Original line Diff line number Diff line
@@ -36,10 +36,11 @@ public final class CaptivePortalProbeResult { 
     */

     */

    public static final int PARTIAL_CODE = -1;

    public static final int PARTIAL_CODE = -1;





    // DNS response with private IP on the probe URL means the network, especially Wi-Fi network is

    // DNS response with private IP on the probe URL suggests that the network, especially Wi-Fi

    // not connected to the Internet. This code represents the result of a single probe, for correct

    // network is not connected to the Internet. This code represents the result of a single probe,

    // logging of the probe results. The result of the whole evaluation would typically be FAILED if

    // for correct logging of the probe results. The result of the whole evaluation would typically

    // one of the probes returns this status.

    // be FAILED if one of the probes returns this status.

    // This logic is only used if the config_force_dns_probe_private_ip_no_internet flag is set.

    public static final int DNS_PRIVATE_IP_RESPONSE_CODE = -2;

    public static final int DNS_PRIVATE_IP_RESPONSE_CODE = -2;





    @NonNull

    @NonNull

src/android/net/util/NetworkStackUtils.java

+8 −0
Original line number Original line Diff line number Diff line
@@ -385,4 +385,12 @@ public class NetworkStackUtils { 
                (address instanceof Inet6Address) ? "[%s]:%d" : "%s:%d",

                (address instanceof Inet6Address) ? "[%s]:%d" : "%s:%d",

                        address.getHostAddress(), port);

                        address.getHostAddress(), port);

    }

    }



    /**

     * Return true if the provided address is non-null and an IPv6 Unique Local Address (RFC4193).

     */

    public static boolean isIPv6ULA(@Nullable InetAddress addr) {

        return addr instanceof Inet6Address

                && ((addr.getAddress()[0] & 0xfe) == 0xfc);

    }

}
 
}

src/com/android/server/connectivity/NetworkMonitor.java

+13 −11
Original line number Original line Diff line number Diff line
@@ -72,6 +72,7 @@ import static android.net.util.NetworkStackUtils.DEFAULT_CAPTIVE_PORTAL_HTTP_URL 
import static android.net.util.NetworkStackUtils.DISMISS_PORTAL_IN_VALIDATED_NETWORK;

import static android.net.util.NetworkStackUtils.DISMISS_PORTAL_IN_VALIDATED_NETWORK;

import static android.net.util.NetworkStackUtils.DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION;

import static android.net.util.NetworkStackUtils.DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION;

import static android.net.util.NetworkStackUtils.isEmpty;

import static android.net.util.NetworkStackUtils.isEmpty;

import static android.net.util.NetworkStackUtils.isIPv6ULA;

import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;

import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;





import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_DNS_EVENTS;

import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_DNS_EVENTS;
@@ -424,7 +425,7 @@ public class NetworkMonitor extends StateMachine { 
    private boolean mAcceptPartialConnectivity = false;

    private boolean mAcceptPartialConnectivity = false;

    private final EvaluationState mEvaluationState = new EvaluationState();

    private final EvaluationState mEvaluationState = new EvaluationState();





    private final boolean mPrivateIpNotPortalEnabled;

    private final boolean mPrivateIpNoInternetEnabled;





    private int getCallbackVersion(INetworkMonitorCallbacks cb) {

    private int getCallbackVersion(INetworkMonitorCallbacks cb) {

        int version;

        int version;
@@ -487,7 +488,7 @@ public class NetworkMonitor extends StateMachine { 
        // CHECKSTYLE:ON IndentationCheck

        // CHECKSTYLE:ON IndentationCheck





        mIsCaptivePortalCheckEnabled = getIsCaptivePortalCheckEnabled();

        mIsCaptivePortalCheckEnabled = getIsCaptivePortalCheckEnabled();

        mPrivateIpNotPortalEnabled = getIsPrivateIpNotPortalEnabled();

        mPrivateIpNoInternetEnabled = getIsPrivateIpNoInternetEnabled();

        mUseHttps = getUseHttpsValidation();

        mUseHttps = getUseHttpsValidation();

        mCaptivePortalUserAgent = getCaptivePortalUserAgent();

        mCaptivePortalUserAgent = getCaptivePortalUserAgent();

        mCaptivePortalHttpsUrls = makeCaptivePortalHttpsUrls();

        mCaptivePortalHttpsUrls = makeCaptivePortalHttpsUrls();
@@ -1438,7 +1439,7 @@ public class NetworkMonitor extends StateMachine { 
        return mode != CAPTIVE_PORTAL_MODE_IGNORE;

        return mode != CAPTIVE_PORTAL_MODE_IGNORE;

    }

    }





    private boolean getIsPrivateIpNotPortalEnabled() {

    private boolean getIsPrivateIpNoInternetEnabled() {

        return mDependencies.isFeatureEnabled(mContext, DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION)

        return mDependencies.isFeatureEnabled(mContext, DNS_PROBE_PRIVATE_IP_NO_INTERNET_VERSION)

                || mContext.getResources().getBoolean(

                || mContext.getResources().getBoolean(

                        R.bool.config_force_dns_probe_private_ip_no_internet);

                        R.bool.config_force_dns_probe_private_ip_no_internet);
@@ -1888,9 +1889,9 @@ public class NetworkMonitor extends StateMachine { 
        // information to callers that does not make sense because the state machine has already

        // information to callers that does not make sense because the state machine has already

        // changed state.

        // changed state.

        final InetAddress[] resolvedAddr = sendDnsProbe(host);

        final InetAddress[] resolvedAddr = sendDnsProbe(host);

        // The private IP logic only applies to the HTTP probe, not the HTTPS probe (which would

        // The private IP logic only applies to captive portal detection (the HTTP probe), not

        // fail anyway) or the PAC probe.

        // network validation (the HTTPS probe, which would likely fail anyway) or the PAC probe.

        if (mPrivateIpNotPortalEnabled && probeType == ValidationProbeEvent.PROBE_HTTP

        if (mPrivateIpNoInternetEnabled && probeType == ValidationProbeEvent.PROBE_HTTP

                && (proxy == null) && hasPrivateIpAddress(resolvedAddr)) {

                && (proxy == null) && hasPrivateIpAddress(resolvedAddr)) {

            return CaptivePortalProbeResult.PRIVATE_IP;

            return CaptivePortalProbeResult.PRIVATE_IP;

        }

        }
@@ -1928,8 +1929,7 @@ public class NetworkMonitor extends StateMachine { 
    }

    }





    /**

    /**

     * Check if any of the provided IP addresses include a private IP, as defined by

     * Check if any of the provided IP addresses include a private IP.

     * {@link com.android.server.util.NetworkStackConstants#PRIVATE_IPV4_RANGES}.

     * @return true if an IP address is private.

     * @return true if an IP address is private.

     */

     */

    private static boolean hasPrivateIpAddress(@Nullable InetAddress[] addresses) {

    private static boolean hasPrivateIpAddress(@Nullable InetAddress[] addresses) {
@@ -1937,7 +1937,8 @@ public class NetworkMonitor extends StateMachine { 
            return false;

            return false;

        }

        }

        for (InetAddress address : addresses) {

        for (InetAddress address : addresses) {

            if (address.isLinkLocalAddress() || address.isSiteLocalAddress()) {

            if (address.isLinkLocalAddress() || address.isSiteLocalAddress()

                    || isIPv6ULA(address)) {

                return true;

                return true;

            }

            }

        }

        }
@@ -2266,10 +2267,11 @@ public class NetworkMonitor extends StateMachine { 
        }

        }

        // Consider a DNS response with a private IP address on the HTTP probe as an indication that

        // Consider a DNS response with a private IP address on the HTTP probe as an indication that

        // the network is not connected to the Internet, and have the whole evaluation fail in that

        // the network is not connected to the Internet, and have the whole evaluation fail in that

        // case.

        // case, instead of potentially detecting a captive portal. This logic only affects portal

        // detection, not network validation.

        // This only applies if the DNS probe completed within PROBE_TIMEOUT_MS, as the fallback

        // This only applies if the DNS probe completed within PROBE_TIMEOUT_MS, as the fallback

        // probe should not be delayed by this check.

        // probe should not be delayed by this check.

        if (mPrivateIpNotPortalEnabled && (httpResult.isDnsPrivateIpResponse())) {

        if (mPrivateIpNoInternetEnabled && (httpResult.isDnsPrivateIpResponse())) {

            validationLog("DNS response to the URL is private IP");

            validationLog("DNS response to the URL is private IP");

            return CaptivePortalProbeResult.FAILED;

            return CaptivePortalProbeResult.FAILED;

        }

        }

tests/unit/src/android/net/util/NetworkStackUtilsTest.java

+15 −1
Original line number Original line Diff line number Diff line
@@ -16,6 +16,9 @@ 




package android.net.util;

package android.net.util;





import static android.net.InetAddresses.parseNumericAddress;

import static android.net.util.NetworkStackUtils.isIPv6ULA;



import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession;
@@ -185,4 +188,15 @@ public class NetworkStackUtilsTest { 
        assertFalse(NetworkStackUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,

        assertFalse(NetworkStackUtils.isFeatureEnabled(mContext, TEST_NAME_SPACE,

                TEST_EXPERIMENT_FLAG));

                TEST_EXPERIMENT_FLAG));

    }

    }



    @Test

    public void testIsIPv6ULA() {

        assertTrue(isIPv6ULA(parseNumericAddress("fc00::")));

        assertTrue(isIPv6ULA(parseNumericAddress("fc00::1")));

        assertTrue(isIPv6ULA(parseNumericAddress("fc00:1234::5678")));

        assertTrue(isIPv6ULA(parseNumericAddress("fdff:ffff:ffff:ffff:ffff:ffff:ffff:ffff")));



        assertFalse(isIPv6ULA(parseNumericAddress("fe00::")));

        assertFalse(isIPv6ULA(parseNumericAddress("2480:1248::123:456")));

    }

}
 
}
 
 No newline at end of file