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

Commit 4656ed9d authored by Lorenzo Colitti's avatar Lorenzo Colitti
Browse files

Add a timeout for the DNS probe. 

Bug: 129312219
Test: atest NetworkStackTests
Test: atest --generate-new-metrics 50 NetworkStackTests:com.android.server.connectivity.NetworkMonitorTest
Change-Id: Ib3ab9105d7ae39f551b51e8d5a04b9ec5e549655
Merged-In: Ib3ab9105d7ae39f551b51e8d5a04b9ec5e549655
(cherry picked from commit c7204f689dc58c3ac84818cb1f8fb9add307d90c)
parent 4e445514

res/values/config.xml

+4 −0
Original line number Diff line number Diff line
@@ -7,6 +7,9 @@ 
    values are meant to be the default when no other configuration is specified.

    -->



    <!-- DNS probe timeout for network validation. Enough for 3 DNS queries 5 seconds apart. -->

    <integer name="default_captive_portal_dns_probe_timeout">12500</integer>



    <!-- HTTP URL for network validation, to use for detecting captive portals. -->

    <string name="default_captive_portal_http_url" translatable="false">http://connectivitycheck.gstatic.com/generate_204</string>
@@ -27,6 +30,7 @@ 


    <!-- Configuration hooks for the above settings.

         Empty by default but may be overridden by RROs. -->

    <integer name="config_captive_portal_dns_probe_timeout"></integer>

    <!--suppress CheckTagEmptyBody: overlayable resource to use as configuration hook -->

    <string name="config_captive_portal_http_url" translatable="false"></string>

    <!--suppress CheckTagEmptyBody: overlayable resource to use as configuration hook -->

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

+81 −1
Original line number Diff line number Diff line
@@ -59,6 +59,7 @@ import android.content.Intent; 
import android.content.IntentFilter;

import android.content.res.Resources;

import android.net.ConnectivityManager;

import android.net.DnsResolver;

import android.net.INetworkMonitor;

import android.net.INetworkMonitorCallbacks;

import android.net.LinkProperties;
@@ -122,6 +123,7 @@ import java.util.Random; 
import java.util.UUID;

import java.util.concurrent.CountDownLatch;

import java.util.concurrent.TimeUnit;

import java.util.concurrent.atomic.AtomicReference;

import java.util.function.Function;



/**
@@ -136,8 +138,16 @@ public class NetworkMonitor extends StateMachine { 
                                                      + "AppleWebKit/537.36 (KHTML, like Gecko) "

                                                      + "Chrome/60.0.3112.32 Safari/537.36";



    @VisibleForTesting

    static final String CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT =

            "captive_portal_dns_probe_timeout";



    private static final int SOCKET_TIMEOUT_MS = 10000;

    private static final int PROBE_TIMEOUT_MS  = 3000;

    // Enough for 3 DNS queries 5 seconds apart.

    // TODO: get this from resources and DeviceConfig instead.

    private static final int DNS_TIMEOUT_MS = 12500;



    enum EvaluationResult {

        VALIDATED(true),

        CAPTIVE_PORTAL(false);
@@ -1185,6 +1195,33 @@ public class NetworkMonitor extends StateMachine { 
                Settings.Global.CAPTIVE_PORTAL_HTTPS_URL);

    }



    private int getDnsProbeTimeout() {

        return getIntSetting(mContext, R.integer.config_captive_portal_dns_probe_timeout,

                CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT,

                R.integer.default_captive_portal_dns_probe_timeout);

    }



    /**

     * Gets an integer setting from resources or device config

     *

     * configResource is used if set, followed by device config if set, followed by defaultResource.

     * If none of these are set then an exception is thrown.

     *

     * TODO: move to a common location such as a ConfigUtils class.

     * TODO(b/130324939): test that the resources can be overlayed by an RRO package.

     */

    @VisibleForTesting

    int getIntSetting(@NonNull final Context context, @StringRes int configResource,

            @NonNull String symbol, @StringRes int defaultResource) {

        final Resources res = context.getResources();

        try {

            return res.getInteger(configResource);

        } catch (Resources.NotFoundException e) {

            return mDependencies.getDeviceConfigPropertyInt(NAMESPACE_CONNECTIVITY,

                    symbol, res.getInteger(defaultResource));

        }

    }



    /**

     * Get the captive portal server HTTP URL that is configured on the device.

     *
@@ -1446,6 +1483,45 @@ public class NetworkMonitor extends StateMachine { 
        return sendHttpProbe(url, probeType, null);

    }



    /** Do a DNS lookup for the given server, or throw UnknownHostException after timeoutMs */

    @VisibleForTesting

    protected InetAddress[] sendDnsProbeWithTimeout(String host, int timeoutMs)

                throws UnknownHostException {

        final CountDownLatch latch = new CountDownLatch(1);

        final AtomicReference<List<InetAddress>> resultRef = new AtomicReference<>();

        final DnsResolver.Callback<List<InetAddress>> callback =

                    new DnsResolver.Callback<List<InetAddress>>() {

            public void onAnswer(List<InetAddress> answer, int rcode) {

                if (rcode == 0) {

                    resultRef.set(answer);

                }

                latch.countDown();

            }

            public void onError(@NonNull DnsResolver.DnsException e) {

                validationLog("DNS error resolving " + host + ": " + e.getMessage());

                latch.countDown();

            }

        };



        final int oldTag = TrafficStats.getAndSetThreadStatsTag(

                TrafficStatsConstants.TAG_SYSTEM_PROBE);

        mDependencies.getDnsResolver().query(mNetwork, host, DnsResolver.FLAG_EMPTY,

                r -> r.run() /* executor */, null /* cancellationSignal */, callback);

        TrafficStats.setThreadStatsTag(oldTag);



        try {

            latch.await(timeoutMs, TimeUnit.MILLISECONDS);

        } catch (InterruptedException e) {

        }



        List<InetAddress> result = resultRef.get();

        if (result == null || result.size() == 0) {

            throw new UnknownHostException(host);

        }



        return result.toArray(new InetAddress[0]);

    }



    /** Do a DNS resolution of the given server. */

    private void sendDnsProbe(String host) {

        if (TextUtils.isEmpty(host)) {
@@ -1457,7 +1533,7 @@ public class NetworkMonitor extends StateMachine { 
        int result;

        String connectInfo;

        try {

            InetAddress[] addresses = mNetwork.getAllByName(host);

            InetAddress[] addresses = sendDnsProbeWithTimeout(host, getDnsProbeTimeout());

            StringBuffer buffer = new StringBuffer();

            for (InetAddress address : addresses) {

                buffer.append(',').append(address.getHostAddress());
@@ -1782,6 +1858,10 @@ public class NetworkMonitor extends StateMachine { 
            return new OneAddressPerFamilyNetwork(network);

        }



        public DnsResolver getDnsResolver() {

            return DnsResolver.getInstance();

        }



        public Random getRandom() {

            return new Random();

        }

tests/src/com/android/server/connectivity/NetworkMonitorTest.java

+115 −3
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_USE_HTTPS; 
import static junit.framework.Assert.assertEquals;

import static junit.framework.Assert.assertFalse;



import static org.junit.Assert.assertArrayEquals;

import static org.junit.Assert.assertNotNull;

import static org.junit.Assert.assertNull;

import static org.junit.Assert.assertTrue;
@@ -41,6 +42,7 @@ import static org.mockito.ArgumentMatchers.eq; 
import static org.mockito.Mockito.any;

import static org.mockito.Mockito.anyInt;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.doNothing;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.doThrow;

import static org.mockito.Mockito.never;
@@ -55,6 +57,7 @@ import android.content.Context; 
import android.content.Intent;

import android.content.res.Resources;

import android.net.ConnectivityManager;

import android.net.DnsResolver;

import android.net.INetworkMonitorCallbacks;

import android.net.InetAddresses;

import android.net.LinkProperties;
@@ -69,6 +72,7 @@ import android.net.wifi.WifiManager; 
import android.os.Bundle;

import android.os.ConditionVariable;

import android.os.Handler;

import android.os.Looper;

import android.os.RemoteException;

import android.os.SystemClock;

import android.provider.Settings;
@@ -79,6 +83,7 @@ import android.util.ArrayMap; 
import androidx.test.filters.SmallTest;

import androidx.test.runner.AndroidJUnit4;



import com.android.networkstack.R;

import com.android.networkstack.metrics.DataStallDetectionStats;

import com.android.networkstack.metrics.DataStallStatsUtils;
@@ -96,8 +101,12 @@ import java.io.IOException; 
import java.net.HttpURLConnection;

import java.net.InetAddress;

import java.net.URL;

import java.net.UnknownHostException;

import java.util.ArrayList;

import java.util.HashSet;

import java.util.List;

import java.util.Random;

import java.util.concurrent.Executor;



import javax.net.ssl.SSLHandshakeException;
@@ -111,6 +120,7 @@ public class NetworkMonitorTest { 
    private @Mock IpConnectivityLog mLogger;

    private @Mock SharedLog mValidationLogger;

    private @Mock NetworkInfo mNetworkInfo;

    private @Mock DnsResolver mDnsResolver;

    private @Mock ConnectivityManager mCm;

    private @Mock TelephonyManager mTelephony;

    private @Mock WifiManager mWifi;
@@ -156,10 +166,36 @@ public class NetworkMonitorTest { 
    private static final NetworkCapabilities NO_INTERNET_CAPABILITIES = new NetworkCapabilities()

            .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR);



    private void setDnsAnswers(String[] answers) throws UnknownHostException {

        if (answers == null) {

            doThrow(new UnknownHostException()).when(mNetwork).getAllByName(any());

            doNothing().when(mDnsResolver).query(any(), any(), anyInt(), any(), any(), any());

            return;

        }



        List<InetAddress> answerList = new ArrayList<>();

        for (String answer : answers) {

            answerList.add(InetAddresses.parseNumericAddress(answer));

        }

        InetAddress[] answerArray = answerList.toArray(new InetAddress[0]);



        doReturn(answerArray).when(mNetwork).getAllByName(any());



        doAnswer((invocation) -> {

            Executor executor = (Executor) invocation.getArgument(3);

            DnsResolver.Callback<List<InetAddress>> callback = invocation.getArgument(5);

            new Handler(Looper.getMainLooper()).post(() -> {

                executor.execute(() -> callback.onAnswer(answerList, 0));

            });

            return null;

        }).when(mDnsResolver).query(eq(mNetwork), any(), anyInt(), any(), any(), any());

    }



    @Before

    public void setUp() throws IOException {

        MockitoAnnotations.initMocks(this);

        when(mDependencies.getPrivateDnsBypassNetwork(any())).thenReturn(mNetwork);

        when(mDependencies.getDnsResolver()).thenReturn(mDnsResolver);

        when(mDependencies.getRandom()).thenReturn(mRandom);

        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_MODE), anyInt()))

                .thenReturn(Settings.Global.CAPTIVE_PORTAL_MODE_PROMPT);
@@ -204,9 +240,8 @@ public class NetworkMonitorTest { 
        }).when(mNetwork).openConnection(any());

        when(mHttpConnection.getRequestProperties()).thenReturn(new ArrayMap<>());

        when(mHttpsConnection.getRequestProperties()).thenReturn(new ArrayMap<>());

        doReturn(new InetAddress[] {

                InetAddresses.parseNumericAddress("192.168.0.0")

        }).when(mNetwork).getAllByName(any());



        setDnsAnswers(new String[]{"2001:db8::1", "192.0.2.2"});



        when(mContext.registerReceiver(any(BroadcastReceiver.class), any())).then((invocation) -> {

            mRegisteredReceivers.add(invocation.getArgument(0));
@@ -312,6 +347,44 @@ public class NetworkMonitorTest { 
        }

    }



    @Test

    public void testGetIntSetting() throws Exception {

        WrappedNetworkMonitor wnm = makeNotMeteredNetworkMonitor();



        // No config resource, no device config. Expect to get default resource.

        doThrow(new Resources.NotFoundException())

                .when(mResources).getInteger(eq(R.integer.config_captive_portal_dns_probe_timeout));

        doAnswer(invocation -> {

            int defaultValue = invocation.getArgument(2);

            return defaultValue;

        }).when(mDependencies).getDeviceConfigPropertyInt(any(),

                eq(NetworkMonitor.CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT),

                anyInt());

        when(mResources.getInteger(eq(R.integer.default_captive_portal_dns_probe_timeout)))

                .thenReturn(42);

        assertEquals(42, wnm.getIntSetting(mContext,

                R.integer.config_captive_portal_dns_probe_timeout,

                NetworkMonitor.CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT,

                R.integer.default_captive_portal_dns_probe_timeout));



        // Set device config. Expect to get device config.

        when(mDependencies.getDeviceConfigPropertyInt(any(),

                eq(NetworkMonitor.CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT), anyInt()))

                        .thenReturn(1234);

        assertEquals(1234, wnm.getIntSetting(mContext,

                R.integer.config_captive_portal_dns_probe_timeout,

                NetworkMonitor.CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT,

                R.integer.default_captive_portal_dns_probe_timeout));



        // Set config resource. Expect to get config resource.

        when(mResources.getInteger(eq(R.integer.config_captive_portal_dns_probe_timeout)))

                .thenReturn(5678);

        assertEquals(5678, wnm.getIntSetting(mContext,

                R.integer.config_captive_portal_dns_probe_timeout,

                NetworkMonitor.CONFIG_CAPTIVE_PORTAL_DNS_PROBE_TIMEOUT,

                R.integer.default_captive_portal_dns_probe_timeout));

    }



    @Test

    public void testIsCaptivePortal_HttpProbeIsPortal() throws IOException {

        setSslException(mHttpsConnection);
@@ -642,6 +715,45 @@ public class NetworkMonitorTest { 
        runPartialConnectivityNetworkTest();

    }



    private void assertIpAddressArrayEquals(String[] expected, InetAddress[] actual) {

        String[] actualStrings = new String[actual.length];

        for (int i = 0; i < actual.length; i++) {

            actualStrings[i] = actual[i].getHostAddress();

        }

        assertArrayEquals("Array of IP addresses differs", expected, actualStrings);

    }



    @Test

    public void testSendDnsProbeWithTimeout() throws Exception {

        WrappedNetworkMonitor wnm = makeNotMeteredNetworkMonitor();

        final int shortTimeoutMs = 200;



        String[] expected = new String[]{"2001:db8::"};

        setDnsAnswers(expected);

        InetAddress[] actual = wnm.sendDnsProbeWithTimeout("www.google.com", shortTimeoutMs);

        assertIpAddressArrayEquals(expected, actual);



        expected = new String[]{"2001:db8::", "192.0.2.1"};

        setDnsAnswers(expected);

        actual = wnm.sendDnsProbeWithTimeout("www.google.com", shortTimeoutMs);

        assertIpAddressArrayEquals(expected, actual);



        expected = new String[0];

        setDnsAnswers(expected);

        try {

            wnm.sendDnsProbeWithTimeout("www.google.com", shortTimeoutMs);

            fail("No DNS results, expected UnknownHostException");

        } catch (UnknownHostException e) {

        }



        setDnsAnswers(null);

        try {

            wnm.sendDnsProbeWithTimeout("www.google.com", shortTimeoutMs);

            fail("DNS query timed out, expected UnknownHostException");

        } catch (UnknownHostException e) {

        }

    }



    private void makeDnsTimeoutEvent(WrappedNetworkMonitor wrappedMonitor, int count) {

        for (int i = 0; i < count; i++) {

            wrappedMonitor.getDnsStallDetector().accumulateConsecutiveDnsTimeoutCount(