Merge "Expose captive portal urls for configuration"

core/res/res/values/config.xml

@@ -302,14 +302,15 @@
          Settings.Global.NETWORK_AVOID_BAD_WIFI. This is the default value of that setting. -->
     <integer translatable="false" name="config_networkAvoidBadWifi">1</integer>
 
-    <!-- The URL returned by ConnectivityManager#getCaptivePortalServerUrl. The actual returned
+    <!-- Configuration hook for the URL returned by ConnectivityManager#getCaptivePortalServerUrl.
-         value is controlled by Settings.Global.CAPTIVE_PORTAL_HTTP_URL. This is the default value
+         If empty, the returned value is controlled by Settings.Global.CAPTIVE_PORTAL_HTTP_URL,
-         used if that setting is unset.
+         and if that value is empty, the framework will use a hard-coded default.
          This is *NOT* a URL that will always be used by the system network validation to detect
          captive portals: NetworkMonitor may use different strategies and will not necessarily use
          this URL. NetworkMonitor behaviour should be configured with NetworkStack resource overlays
          instead. -->
-    <string translatable="false" name="config_networkDefaultCaptivePortalServerUrl">http://connectivitycheck.gstatic.com/generate_204</string>
+    <!--suppress CheckTagEmptyBody -->
+    <string translatable="false" name="config_networkCaptivePortalServerUrl"></string>
 
     <!-- If the hardware supports specially marking packets that caused a wakeup of the
          main CPU, set this value to the mark used. -->

core/res/res/values/symbols.xml

@@ -1945,7 +1945,7 @@
   <java-symbol type="integer" name="config_networkNotifySwitchType" />
   <java-symbol type="array" name="config_networkNotifySwitches" />
   <java-symbol type="integer" name="config_networkAvoidBadWifi" />
-  <java-symbol type="string" name="config_networkDefaultCaptivePortalServerUrl" />
+  <java-symbol type="string" name="config_networkCaptivePortalServerUrl" />
   <java-symbol type="integer" name="config_networkWakeupPacketMark" />
   <java-symbol type="integer" name="config_networkWakeupPacketMask" />
   <java-symbol type="bool" name="config_apfDrop802_3Frames" />

packages/NetworkStack/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",

packages/NetworkStack/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>

packages/NetworkStack/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,
+        return getSettingFromResource(mContext, R.string.config_captive_portal_https_url,
-                Settings.Global.CAPTIVE_PORTAL_HTTPS_URL, DEFAULT_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 = ",";
+            final String firstUrl = mDependencies.getSetting(mContext,
-            String firstUrl = mDependencies.getSetting(mContext,
+                    Settings.Global.CAPTIVE_PORTAL_FALLBACK_URL, null);
-                    Settings.Global.CAPTIVE_PORTAL_FALLBACK_URL, DEFAULT_FALLBACK_URL);
+
-            String joinedUrls = firstUrl + separator + mDependencies.getSetting(mContext,
+            final URL[] settingProviderUrls;
-                    Settings.Global.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS,
+            if (!TextUtils.isEmpty(firstUrl)) {
-                    DEFAULT_OTHER_FALLBACK_URLS);
+                final String otherUrls = mDependencies.getSetting(mContext,
-            List<URL> urls = new ArrayList<>();
+                        Settings.Global.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS, "");
-            for (String s : joinedUrls.split(separator)) {
+                // otherUrls may be empty, but .split() ignores trailing empty strings
-                URL u = makeURL(s);
+                final String separator = ",";
-                if (u == null) {
+                final String[] urls = (firstUrl + separator + otherUrls).split(separator);
-                    continue;
+                settingProviderUrls = convertStrings(urls, this::makeURL, new URL[0]);
+            } else {
+                settingProviderUrls = new URL[0];
             }
-                urls.add(u);
+
-            }
+            return getArrayConfig(settingProviderUrls, R.array.config_captive_portal_fallback_urls,
-            if (urls.isEmpty()) {
+                    R.array.default_captive_portal_fallback_urls, this::makeURL);
-                Log.e(TAG, String.format("could not create any url from %s", joinedUrls));
-            }
-            return urls.toArray(new URL[urls.size()]);
         } 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
+            final CaptivePortalProbeSpec[] emptySpecs = new CaptivePortalProbeSpec[0];
-            if (TextUtils.isEmpty(settingsValue)) {
+            final CaptivePortalProbeSpec[] providerValue = TextUtils.isEmpty(settingsValue)
-                return null;
+                    ? emptySpecs
-            }
+                    : parseCaptivePortalProbeSpecs(settingsValue).toArray(emptySpecs);
 
-            final Collection<CaptivePortalProbeSpec> specs =
+            return getArrayConfig(providerValue, R.array.config_captive_portal_fallback_probe_specs,
-                    CaptivePortalProbeSpec.parseCaptivePortalProbeSpecs(settingsValue);
+                    R.array.default_captive_portal_fallback_probe_specs,
-            final CaptivePortalProbeSpec[] specsArray = new CaptivePortalProbeSpec[specs.size()];
+                    CaptivePortalProbeSpec::parseSpecOrNull);
-            return specs.toArray(specsArray);
         } 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);
@@ -1693,19 +1778,6 @@ public class NetworkMonitor extends StateMachine {
             return new Random();
         }
 
-        /**
-         * 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

packages/NetworkStack/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_HTTP_URL), any()))
-        when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTPS_URL),
+                .thenReturn(TEST_HTTP_URL);
-                anyString())).thenReturn(TEST_HTTPS_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);

services/core/java/com/android/server/ConnectivityService.java

@@ -108,7 +108,6 @@ import android.net.VpnService;
 import android.net.metrics.IpConnectivityLog;
 import android.net.metrics.NetworkEvent;
 import android.net.netlink.InetDiagMessage;
-import android.net.shared.NetworkMonitorUtils;
 import android.net.shared.PrivateDnsConfig;
 import android.net.util.MultinetworkPolicyTracker;
 import android.net.util.NetdService;
@@ -238,6 +237,16 @@ public class ConnectivityService extends IConnectivityManager.Stub
 
     private static final boolean LOGD_BLOCKED_NETWORKINFO = true;
 
+    /**
+     * Default URL to use for {@link #getCaptivePortalServerUrl()}. This should not be changed
+     * by OEMs for configuration purposes, as this value is overridden by
+     * Settings.Global.CAPTIVE_PORTAL_HTTP_URL.
+     * R.string.config_networkCaptivePortalServerUrl should be overridden instead for this purpose
+     * (preferably via runtime resource overlays).
+     */
+    private static final String DEFAULT_CAPTIVE_PORTAL_HTTP_URL =
+            "http://connectivitycheck.gstatic.com/generate_204";
+
     // TODO: create better separation between radio types and network types
 
     // how long to wait before switching back to a radio's default network
@@ -6712,9 +6721,20 @@ public class ConnectivityService extends IConnectivityManager.Stub
     @Override
     public String getCaptivePortalServerUrl() {
         enforceConnectivityInternalPermission();
-        final String defaultUrl = mContext.getResources().getString(
+        String settingUrl = mContext.getResources().getString(
-                R.string.config_networkDefaultCaptivePortalServerUrl);
+                R.string.config_networkCaptivePortalServerUrl);
-        return NetworkMonitorUtils.getCaptivePortalServerHttpUrl(mContext, defaultUrl);
+
+        if (!TextUtils.isEmpty(settingUrl)) {
+            return settingUrl;
+        }
+
+        settingUrl = Settings.Global.getString(mContext.getContentResolver(),
+                Settings.Global.CAPTIVE_PORTAL_HTTP_URL);
+        if (!TextUtils.isEmpty(settingUrl)) {
+            return settingUrl;
+        }
+
+        return DEFAULT_CAPTIVE_PORTAL_HTTP_URL;
     }
 
     @Override

services/net/java/android/net/shared/NetworkMonitorUtils.java

@@ -21,9 +21,7 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VPN;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_TRUSTED;
 
-import android.content.Context;
 import android.net.NetworkCapabilities;
-import android.provider.Settings;
 
 /** @hide */
 public class NetworkMonitorUtils {
@@ -44,16 +42,6 @@ public class NetworkMonitorUtils {
     public static final String PERMISSION_ACCESS_NETWORK_CONDITIONS =
             "android.permission.ACCESS_NETWORK_CONDITIONS";
 
-    /**
-     * Get the captive portal server HTTP URL that is configured on the device.
-     */
-    public static String getCaptivePortalServerHttpUrl(Context context, String defaultUrl) {
-        final String settingUrl = Settings.Global.getString(
-                context.getContentResolver(),
-                Settings.Global.CAPTIVE_PORTAL_HTTP_URL);
-        return settingUrl != null ? settingUrl : defaultUrl;
-    }
-
     /**
      * Return whether validation is required for a network.
      * @param dfltNetCap Default requested network capabilities.