Merge "Expose captive portal urls for configuration"

4 files changed, 171 insertions, 58 deletions

diff --git a/Android.bp b/Android.bp
index 262e6f6..5817118 100644
--- a/Android.bp
+++ b/Android.bp
@@ -39,6 +39,7 @@ android_library {
         ":services-networkstack-shared-srcs",
     ],
     static_libs: [
+        "androidx.annotation_annotation",
         "ipmemorystore-client",
         "netd_aidl_interface-java",
         "networkstack-aidl-interfaces-java",
diff --git a/res/values/config.xml b/res/values/config.xml
index 52425e5..90f96e0 100644
--- a/res/values/config.xml
+++ b/res/values/config.xml
@@ -1,5 +1,38 @@
 <?xml version="1.0" encoding="utf-8"?>
 <resources>
-    <!-- Captive portal http url -->
-    <string name="config_captive_portal_http_url" translatable="false">http://connectivitycheck.gstatic.com/generate_204</string>
+    <!--
+    OEMs that wish to change the below settings must do so via a runtime resource overlay package
+    and *NOT* by changing this file. This file is part of the NetworkStack mainline module.
+    The overlays must apply to the config_* values, not the default_* values. The default_*
+    values are meant to be the default when no other configuration is specified.
+    -->
+
+    <!-- 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>
+
+    <!-- HTTPS URL for network validation, to use for confirming internet connectivity. -->
+    <string name="default_captive_portal_https_url" translatable="false">https://www.google.com/generate_204</string>
+
+    <!-- List of fallback URLs to use for detecting captive portals. -->
+    <string-array name="default_captive_portal_fallback_urls" translatable="false">
+        <item>http://www.google.com/gen_204</item>
+        <item>http://play.googleapis.com/generate_204</item>
+    </string-array>
+
+    <!-- List of fallback probe specs to use for detecting captive portals.
+         This is an alternative to fallback URLs that provides more flexibility on detection rules.
+         Empty, so unused by default. -->
+    <string-array name="default_captive_portal_fallback_probe_specs" translatable="false">
+    </string-array>
+
+    <!-- Configuration hooks for the above settings.
+         Empty by default but may be overridden by RROs. -->
+    <!--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 -->
+    <string name="config_captive_portal_https_url" translatable="false"></string>
+    <string-array name="config_captive_portal_fallback_urls" translatable="false">
+    </string-array>
+    <string-array name="config_captive_portal_fallback_probe_specs" translatable="false">
+    </string-array>
 </resources>
\ No newline at end of file
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index 8fbbccb..cceb3be 100644
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -28,6 +28,7 @@ import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_PARTIAL_CONNECTIVI
 import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED;
 import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
 import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
+import static android.net.captiveportal.CaptivePortalProbeSpec.parseCaptivePortalProbeSpecs;
 import static android.net.metrics.ValidationProbeEvent.DNS_FAILURE;
 import static android.net.metrics.ValidationProbeEvent.DNS_SUCCESS;
 import static android.net.metrics.ValidationProbeEvent.PROBE_FALLBACK;
@@ -42,6 +43,7 @@ import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
+import android.content.res.Resources;
 import android.net.ConnectivityManager;
 import android.net.INetworkMonitor;
 import android.net.INetworkMonitorCallbacks;
@@ -80,6 +82,9 @@ import android.text.TextUtils;
 import android.util.Log;
 import android.util.Pair;
 
+import androidx.annotation.ArrayRes;
+import androidx.annotation.StringRes;
+
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.RingBufferIndices;
 import com.android.internal.util.State;
@@ -94,7 +99,6 @@ import java.net.URL;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -102,6 +106,7 @@ import java.util.Random;
 import java.util.UUID;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
 
 /**
  * {@hide}
@@ -111,15 +116,6 @@ public class NetworkMonitor extends StateMachine {
     private static final boolean DBG  = true;
     private static final boolean VDBG = false;
     private static final boolean VDBG_STALL = Log.isLoggable(TAG, Log.DEBUG);
-    // TODO: use another permission for CaptivePortalLoginActivity once it has its own certificate
-    private static final String PERMISSION_NETWORK_SETTINGS = "android.permission.NETWORK_SETTINGS";
-    // Default configuration values for captive portal detection probes.
-    // TODO: append a random length parameter to the default HTTPS url.
-    // TODO: randomize browser version ids in the default User-Agent String.
-    private static final String DEFAULT_HTTPS_URL = "https://www.google.com/generate_204";
-    private static final String DEFAULT_FALLBACK_URL  = "http://www.google.com/gen_204";
-    private static final String DEFAULT_OTHER_FALLBACK_URLS =
-            "http://play.googleapis.com/generate_204";
     private static final String DEFAULT_USER_AGENT    = "Mozilla/5.0 (X11; Linux x86_64) "
                                                       + "AppleWebKit/537.36 (KHTML, like Gecko) "
                                                       + "Chrome/60.0.3112.32 Safari/537.36";
@@ -379,7 +375,7 @@ public class NetworkMonitor extends StateMachine {
         mUseHttps = getUseHttpsValidation();
         mCaptivePortalUserAgent = getCaptivePortalUserAgent();
         mCaptivePortalHttpsUrl = makeURL(getCaptivePortalServerHttpsUrl());
-        mCaptivePortalHttpUrl = makeURL(deps.getCaptivePortalServerHttpUrl(context));
+        mCaptivePortalHttpUrl = makeURL(getCaptivePortalServerHttpUrl());
         mCaptivePortalFallbackUrls = makeCaptivePortalFallbackUrls();
         mCaptivePortalFallbackSpecs = makeCaptivePortalFallbackProbeSpecs();
         mRandom = deps.getRandom();
@@ -1179,8 +1175,22 @@ public class NetworkMonitor extends StateMachine {
     }
 
     private String getCaptivePortalServerHttpsUrl() {
-        return mDependencies.getSetting(mContext,
-                Settings.Global.CAPTIVE_PORTAL_HTTPS_URL, DEFAULT_HTTPS_URL);
+        return getSettingFromResource(mContext, R.string.config_captive_portal_https_url,
+                R.string.default_captive_portal_https_url,
+                Settings.Global.CAPTIVE_PORTAL_HTTPS_URL);
+    }
+
+    /**
+     * Get the captive portal server HTTP URL that is configured on the device.
+     *
+     * NetworkMonitor does not use {@link ConnectivityManager#getCaptivePortalServerUrl()} as
+     * it has its own updatable strategies to detect captive portals. The framework only advises
+     * on one URL that can be used, while NetworkMonitor may implement more complex logic.
+     */
+    public String getCaptivePortalServerHttpUrl() {
+        return getSettingFromResource(mContext, R.string.config_captive_portal_http_url,
+                R.string.default_captive_portal_http_url,
+                Settings.Global.CAPTIVE_PORTAL_HTTP_URL);
     }
 
     private int getConsecutiveDnsTimeoutThreshold() {
@@ -1208,24 +1218,23 @@ public class NetworkMonitor extends StateMachine {
 
     private URL[] makeCaptivePortalFallbackUrls() {
         try {
-            String separator = ",";
-            String firstUrl = mDependencies.getSetting(mContext,
-                    Settings.Global.CAPTIVE_PORTAL_FALLBACK_URL, DEFAULT_FALLBACK_URL);
-            String joinedUrls = firstUrl + separator + mDependencies.getSetting(mContext,
-                    Settings.Global.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS,
-                    DEFAULT_OTHER_FALLBACK_URLS);
-            List<URL> urls = new ArrayList<>();
-            for (String s : joinedUrls.split(separator)) {
-                URL u = makeURL(s);
-                if (u == null) {
-                    continue;
-                }
-                urls.add(u);
-            }
-            if (urls.isEmpty()) {
-                Log.e(TAG, String.format("could not create any url from %s", joinedUrls));
+            final String firstUrl = mDependencies.getSetting(mContext,
+                    Settings.Global.CAPTIVE_PORTAL_FALLBACK_URL, null);
+
+            final URL[] settingProviderUrls;
+            if (!TextUtils.isEmpty(firstUrl)) {
+                final String otherUrls = mDependencies.getSetting(mContext,
+                        Settings.Global.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS, "");
+                // otherUrls may be empty, but .split() ignores trailing empty strings
+                final String separator = ",";
+                final String[] urls = (firstUrl + separator + otherUrls).split(separator);
+                settingProviderUrls = convertStrings(urls, this::makeURL, new URL[0]);
+            } else {
+                settingProviderUrls = new URL[0];
             }
-            return urls.toArray(new URL[urls.size()]);
+
+            return getArrayConfig(settingProviderUrls, R.array.config_captive_portal_fallback_urls,
+                    R.array.default_captive_portal_fallback_urls, this::makeURL);
         } catch (Exception e) {
             // Don't let a misconfiguration bootloop the system.
             Log.e(TAG, "Error parsing configured fallback URLs", e);
@@ -1237,15 +1246,14 @@ public class NetworkMonitor extends StateMachine {
         try {
             final String settingsValue = mDependencies.getSetting(
                     mContext, Settings.Global.CAPTIVE_PORTAL_FALLBACK_PROBE_SPECS, null);
-            // Probe specs only used if configured in settings
-            if (TextUtils.isEmpty(settingsValue)) {
-                return null;
-            }
-
-            final Collection<CaptivePortalProbeSpec> specs =
-                    CaptivePortalProbeSpec.parseCaptivePortalProbeSpecs(settingsValue);
-            final CaptivePortalProbeSpec[] specsArray = new CaptivePortalProbeSpec[specs.size()];
-            return specs.toArray(specsArray);
+            final CaptivePortalProbeSpec[] emptySpecs = new CaptivePortalProbeSpec[0];
+            final CaptivePortalProbeSpec[] providerValue = TextUtils.isEmpty(settingsValue)
+                    ? emptySpecs
+                    : parseCaptivePortalProbeSpecs(settingsValue).toArray(emptySpecs);
+
+            return getArrayConfig(providerValue, R.array.config_captive_portal_fallback_probe_specs,
+                    R.array.default_captive_portal_fallback_probe_specs,
+                    CaptivePortalProbeSpec::parseSpecOrNull);
         } catch (Exception e) {
             // Don't let a misconfiguration bootloop the system.
             Log.e(TAG, "Error parsing configured fallback probe specs", e);
@@ -1253,6 +1261,83 @@ public class NetworkMonitor extends StateMachine {
         }
     }
 
+    /**
+     * Read a setting from a resource or the settings provider.
+     *
+     * <p>The configuration resource is prioritized, then the provider value, then the default
+     * resource value.
+     * @param context The context
+     * @param configResource The resource id for the configuration parameter
+     * @param defaultResource The resource id for the default value
+     * @param symbol The symbol in the settings provider
+     * @return The best available value
+     */
+    @NonNull
+    private String getSettingFromResource(@NonNull final Context context,
+            @StringRes int configResource, @StringRes int defaultResource,
+            @NonNull String symbol) {
+        final Resources res = context.getResources();
+        String setting = res.getString(configResource);
+
+        if (!TextUtils.isEmpty(setting)) return setting;
+
+        setting = mDependencies.getSetting(context, symbol, null);
+        if (!TextUtils.isEmpty(setting)) return setting;
+
+        return res.getString(defaultResource);
+    }
+
+    /**
+     * Get an array configuration from resources or the settings provider.
+     *
+     * <p>The configuration resource is prioritized, then the provider values, then the default
+     * resource values.
+     * @param providerValue Values obtained from the setting provider.
+     * @param configResId ID of the configuration resource.
+     * @param defaultResId ID of the default resource.
+     * @param resourceConverter Converter from the resource strings to stored setting class. Null
+     *                          return values are ignored.
+     */
+    private <T> T[] getArrayConfig(@NonNull T[] providerValue, @ArrayRes int configResId,
+            @ArrayRes int defaultResId, @NonNull Function<String, T> resourceConverter) {
+        final Resources res = mContext.getResources();
+        String[] configValue = res.getStringArray(configResId);
+
+        if (configValue.length == 0) {
+            if (providerValue.length > 0) {
+                return providerValue;
+            }
+
+            configValue = res.getStringArray(defaultResId);
+        }
+
+        return convertStrings(configValue, resourceConverter, Arrays.copyOf(providerValue, 0));
+    }
+
+    /**
+     * Convert a String array to an array of some other type using the specified converter.
+     *
+     * <p>Any null value, or value for which the converter throws a {@link RuntimeException}, will
+     * not be added to the output array, so the output array may be smaller than the input.
+     */
+    private <T> T[] convertStrings(
+            @NonNull String[] strings, Function<String, T> converter, T[] emptyArray) {
+        final ArrayList<T> convertedValues = new ArrayList<>(strings.length);
+        for (String configString : strings) {
+            T convertedValue = null;
+            try {
+                convertedValue = converter.apply(configString);
+            } catch (Exception e) {
+                Log.e(TAG, "Error parsing configuration", e);
+                // Fall through
+            }
+            if (convertedValue != null) {
+                convertedValues.add(convertedValue);
+            }
+        }
+        return convertedValues.toArray(emptyArray);
+    }
+
     private String getCaptivePortalUserAgent() {
         return mDependencies.getSetting(mContext,
                 Settings.Global.CAPTIVE_PORTAL_USER_AGENT, DEFAULT_USER_AGENT);
@@ -1694,19 +1779,6 @@ public class NetworkMonitor extends StateMachine {
         }
 
         /**
-         * Get the captive portal server HTTP URL that is configured on the device.
-         *
-         * NetworkMonitor does not use {@link ConnectivityManager#getCaptivePortalServerUrl()} as
-         * it has its own updatable strategies to detect captive portals. The framework only advises
-         * on one URL that can be used, while  NetworkMonitor may implement more complex logic.
-         */
-        public String getCaptivePortalServerHttpUrl(Context context) {
-            final String defaultUrl =
-                    context.getResources().getString(R.string.config_captive_portal_http_url);
-            return NetworkMonitorUtils.getCaptivePortalServerHttpUrl(context, defaultUrl);
-        }
-
-        /**
          * Get the value of a global integer setting.
          * @param symbol Name of the setting
          * @param defaultValue Value to return if the setting is not defined.
diff --git a/tests/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/src/com/android/server/connectivity/NetworkMonitorTest.java
index 6665aae..1c11586 100644
--- a/tests/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/tests/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -30,7 +30,6 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
-import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyInt;
@@ -45,6 +44,7 @@ import static org.mockito.Mockito.when;
 
 import android.annotation.NonNull;
 import android.content.Context;
+import android.content.res.Resources;
 import android.net.ConnectivityManager;
 import android.net.INetworkMonitorCallbacks;
 import android.net.InetAddresses;
@@ -95,6 +95,7 @@ public class NetworkMonitorTest {
     private static final String LOCATION_HEADER = "location";
 
     private @Mock Context mContext;
+    private @Mock Resources mResources;
     private @Mock IpConnectivityLog mLogger;
     private @Mock SharedLog mValidationLogger;
     private @Mock NetworkInfo mNetworkInfo;
@@ -150,14 +151,20 @@ public class NetworkMonitorTest {
                 .thenReturn(Settings.Global.CAPTIVE_PORTAL_MODE_PROMPT);
         when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_USE_HTTPS),
                 anyInt())).thenReturn(1);
-        when(mDependencies.getCaptivePortalServerHttpUrl(any())).thenReturn(TEST_HTTP_URL);
-        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTPS_URL),
-                anyString())).thenReturn(TEST_HTTPS_URL);
+        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTP_URL), any()))
+                .thenReturn(TEST_HTTP_URL);
+        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTPS_URL), any()))
+                .thenReturn(TEST_HTTPS_URL);
+
         doReturn(mNetwork).when(mNetwork).getPrivateDnsBypassingCopy();
 
         when(mContext.getSystemService(Context.CONNECTIVITY_SERVICE)).thenReturn(mCm);
         when(mContext.getSystemService(Context.TELEPHONY_SERVICE)).thenReturn(mTelephony);
         when(mContext.getSystemService(Context.WIFI_SERVICE)).thenReturn(mWifi);
+        when(mContext.getResources()).thenReturn(mResources);
+
+        when(mResources.getString(anyInt())).thenReturn("");
+        when(mResources.getStringArray(anyInt())).thenReturn(new String[0]);
 
         when(mNetworkInfo.getType()).thenReturn(ConnectivityManager.TYPE_WIFI);
         setFallbackUrl(TEST_FALLBACK_URL);