diff --git a/core/res/res/values/config.xml b/core/res/res/values/config.xml index 7698d271cfac9..f1e9d659e0fd1 100644 --- a/core/res/res/values/config.xml +++ b/core/res/res/values/config.xml @@ -302,14 +302,15 @@ Settings.Global.NETWORK_AVOID_BAD_WIFI. This is the default value of that setting. --> 1 - - http://connectivitycheck.gstatic.com/generate_204 + + diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index 05303c96ea808..e98a98487f365 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -1945,7 +1945,7 @@ - + diff --git a/packages/NetworkStack/Android.bp b/packages/NetworkStack/Android.bp index 262e6f633917a..5817118bc8619 100644 --- a/packages/NetworkStack/Android.bp +++ b/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", diff --git a/packages/NetworkStack/res/values/config.xml b/packages/NetworkStack/res/values/config.xml index 52425e57ba3ae..90f96e0f7a186 100644 --- a/packages/NetworkStack/res/values/config.xml +++ b/packages/NetworkStack/res/values/config.xml @@ -1,5 +1,38 @@ - - http://connectivitycheck.gstatic.com/generate_204 + + + + http://connectivitycheck.gstatic.com/generate_204 + + + https://www.google.com/generate_204 + + + + http://www.google.com/gen_204 + http://play.googleapis.com/generate_204 + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java b/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java index 8fbbccb8178f7..cceb3be237627 100644 --- a/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java +++ b/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, - 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 urls = new ArrayList<>(); - for (String s : joinedUrls.split(separator)) { - URL u = makeURL(s); - if (u == null) { - continue; - } - urls.add(u); + 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]; } - if (urls.isEmpty()) { - Log.e(TAG, String.format("could not create any url from %s", joinedUrls)); - } - 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 CaptivePortalProbeSpec[] emptySpecs = new CaptivePortalProbeSpec[0]; + final CaptivePortalProbeSpec[] providerValue = TextUtils.isEmpty(settingsValue) + ? emptySpecs + : parseCaptivePortalProbeSpecs(settingsValue).toArray(emptySpecs); - final Collection specs = - CaptivePortalProbeSpec.parseCaptivePortalProbeSpecs(settingsValue); - final CaptivePortalProbeSpec[] specsArray = new CaptivePortalProbeSpec[specs.size()]; - return specs.toArray(specsArray); + 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. + * + *

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. + * + *

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[] getArrayConfig(@NonNull T[] providerValue, @ArrayRes int configResId, + @ArrayRes int defaultResId, @NonNull Function 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. + * + *

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[] convertStrings( + @NonNull String[] strings, Function converter, T[] emptyArray) { + final ArrayList 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 diff --git a/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java b/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java index 6665aae65d90d..1c11586ce59a9 100644 --- a/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java +++ b/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_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); diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 9d2965afdbca3..e3382c311cb97 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/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( - R.string.config_networkDefaultCaptivePortalServerUrl); - return NetworkMonitorUtils.getCaptivePortalServerHttpUrl(mContext, defaultUrl); + String settingUrl = mContext.getResources().getString( + R.string.config_networkCaptivePortalServerUrl); + + 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 diff --git a/services/net/java/android/net/shared/NetworkMonitorUtils.java b/services/net/java/android/net/shared/NetworkMonitorUtils.java index a17cb46471586..bb4a603ba4217 100644 --- a/services/net/java/android/net/shared/NetworkMonitorUtils.java +++ b/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.