Snap for 7732814 from 8b8a2155e8eb380c32072188d6ca9d59435afa41 to mainline-cellbroadcast-release Change-Id: I080bc961ae2343f7223542c739c7bea38c454c13
diff --git a/AndroidManifest.xml b/AndroidManifest.xml index 6a11b2c..8750795 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml
@@ -21,6 +21,7 @@ android:sharedUserId="android.uid.networkstack" android:versionCode="319999900" android:versionName="s_aml_319999900" + coreApp="true" > <!-- Permissions must be defined here, and not in the base manifest, as the network stack running in the system server process does not need any permission, and having privileged
diff --git a/AndroidManifest_InProcess.xml b/AndroidManifest_InProcess.xml index 2cb146a..6c64d87 100644 --- a/AndroidManifest_InProcess.xml +++ b/AndroidManifest_InProcess.xml
@@ -19,7 +19,8 @@ <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="com.android.networkstack.inprocess" android:sharedUserId="android.uid.system" - android:process="system"> + android:process="system" + coreApp="true"> <application> <service android:name="com.android.server.NetworkStackService" android:process="system"
diff --git a/AndroidManifest_Next.xml b/AndroidManifest_Next.xml index 9ad69ae..244d465 100644 --- a/AndroidManifest_Next.xml +++ b/AndroidManifest_Next.xml
@@ -18,5 +18,6 @@ package="com.android.networkstack" android:sharedUserId="android.uid.networkstack" android:versionCode="320000000" - android:versionName="T-next"> + android:versionName="T-next" + coreApp="true"> </manifest>
diff --git a/TEST_MAPPING b/TEST_MAPPING index 2bb0e61..ed6a1a8 100644 --- a/TEST_MAPPING +++ b/TEST_MAPPING
@@ -40,8 +40,7 @@ // We must specify at least one module here or the tests won't run. Use the same set as CTS // so in theory the infra would not need to reinstall/reboot devices to run both. { - // TODO: add back tethering when it is updatable in this branch - "name": "NetworkStackTests[CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex]" + "name": "NetworkStackTests[CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex]" } ], "mainline-postsubmit": [
diff --git a/res/values-da/strings.xml b/res/values-da/strings.xml index e54f11c..706f174 100644 --- a/res/values-da/strings.xml +++ b/res/values-da/strings.xml
@@ -19,7 +19,7 @@ <string name="notification_channel_name_connected" msgid="1795068343200033922">"Godkendelse til loginportal"</string> <string name="notification_channel_description_connected" msgid="7239184168268014518">"De notifikationer, der vises, når enheden er blevet godkendt til et netværk via en loginportal"</string> <string name="notification_channel_name_network_venue_info" msgid="6526543187249265733">"Oplysninger om netværksplacering"</string> - <string name="notification_channel_description_network_venue_info" msgid="5131499595382733605">"Notifikationer, der vises for at indikere, at netværket har en side med oplysninger om placeringen"</string> + <string name="notification_channel_description_network_venue_info" msgid="5131499595382733605">"Notifikationer, der vises for at indikere, at netværket har en side med oplysninger om lokationen"</string> <string name="connected" msgid="4563643884927480998">"Der er oprettet forbindelse"</string> <string name="tap_for_info" msgid="6849746325626883711">"Der er oprettet forbindelse/tryk for at se website"</string> <string name="application_label" msgid="1322847171305285454">"Netværksadministrator"</string>
diff --git a/src/android/net/ip/IpClient.java b/src/android/net/ip/IpClient.java index 4de9393..6746680 100644 --- a/src/android/net/ip/IpClient.java +++ b/src/android/net/ip/IpClient.java
@@ -715,7 +715,7 @@ (ifaceUp) -> sendMessage(EVENT_NETLINK_LINKPROPERTIES_CHANGED, ifaceUp ? ARG_LINKPROP_CHANGED_LINKSTATE_UP : ARG_LINKPROP_CHANGED_LINKSTATE_DOWN), - config, mLog) { + config, mLog, mDependencies) { @Override public void onInterfaceAdded(String iface) { super.onInterfaceAdded(iface);
diff --git a/src/android/net/ip/IpClientLinkObserver.java b/src/android/net/ip/IpClientLinkObserver.java index 3702674..ff0aafe 100644 --- a/src/android/net/ip/IpClientLinkObserver.java +++ b/src/android/net/ip/IpClientLinkObserver.java
@@ -16,6 +16,7 @@ package android.net.ip; +import static android.net.util.NetworkStackUtils.IPCLIENT_PARSE_NETLINK_EVENTS_VERSION; import static android.system.OsConstants.AF_INET6; import static com.android.net.module.util.NetworkStackConstants.ICMPV6_ROUTER_ADVERTISEMENT; @@ -37,6 +38,7 @@ import com.android.net.module.util.netlink.NetlinkConstants; import com.android.net.module.util.netlink.NetlinkMessage; import com.android.net.module.util.netlink.StructNdOptPref64; +import com.android.net.module.util.netlink.StructNdOptRdnss; import com.android.networkstack.apishim.NetworkInformationShimImpl; import com.android.networkstack.apishim.common.NetworkInformationShim; import com.android.server.NetworkObserver; @@ -107,6 +109,7 @@ } } + private final Context mContext; private final String mInterfaceName; private final Callback mCallback; private final LinkProperties mLinkProperties; @@ -115,13 +118,15 @@ private final AlarmManager mAlarmManager; private final Configuration mConfig; private final Handler mHandler; + private final IpClient.Dependencies mDependencies; private final MyNetlinkMonitor mNetlinkMonitor; private static final boolean DBG = false; public IpClientLinkObserver(Context context, Handler h, String iface, Callback callback, - Configuration config, SharedLog log) { + Configuration config, SharedLog log, IpClient.Dependencies deps) { + mContext = context; mInterfaceName = iface; mTag = "NetlinkTracker/" + mInterfaceName; mCallback = callback; @@ -134,6 +139,7 @@ mAlarmManager = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE); mNetlinkMonitor = new MyNetlinkMonitor(h, log, mTag); mHandler.post(mNetlinkMonitor::start); + mDependencies = deps; } public void shutdown() { @@ -153,6 +159,11 @@ } } + private boolean isNetlinkEventParsingEnabled() { + return mDependencies.isFeatureEnabled(mContext, IPCLIENT_PARSE_NETLINK_EVENTS_VERSION, + false /* default value */); + } + @Override public void onInterfaceRemoved(String iface) { maybeLog("interfaceRemoved", iface); @@ -246,17 +257,21 @@ @Override public void onInterfaceDnsServerInfo(String iface, long lifetime, String[] addresses) { - if (mInterfaceName.equals(iface)) { - maybeLog("interfaceDnsServerInfo", Arrays.toString(addresses)); - final boolean changed = mDnsServerRepository.addServers(lifetime, addresses); - final boolean linkState; - if (changed) { - synchronized (this) { - mDnsServerRepository.setDnsServersOn(mLinkProperties); - linkState = getInterfaceLinkStateLocked(); - } - mCallback.update(linkState); + if (isNetlinkEventParsingEnabled()) return; + if (!mInterfaceName.equals(iface)) return; + maybeLog("interfaceDnsServerInfo", Arrays.toString(addresses)); + updateInterfaceDnsServerInfo(lifetime, addresses); + } + + private void updateInterfaceDnsServerInfo(long lifetime, final String[] addresses) { + final boolean changed = mDnsServerRepository.addServers(lifetime, addresses); + final boolean linkState; + if (changed) { + synchronized (this) { + mDnsServerRepository.setDnsServersOn(mLinkProperties); + linkState = getInterfaceLinkStateLocked(); } + mCallback.update(linkState); } } @@ -408,6 +423,15 @@ updatePref64(opt.prefix, now, expiry); } + private void processRdnssOption(StructNdOptRdnss opt) { + if (!isNetlinkEventParsingEnabled()) return; + final String[] addresses = new String[opt.servers.length]; + for (int i = 0; i < opt.servers.length; i++) { + addresses[i] = opt.servers[i].getHostAddress(); + } + updateInterfaceDnsServerInfo(opt.header.lifetime, addresses); + } + private void processNduseroptMessage(NduseroptMessage msg, final long whenMs) { if (msg.family != AF_INET6 || msg.option == null || msg.ifindex != mIfindex) return; if (msg.icmp_type != (byte) ICMPV6_ROUTER_ADVERTISEMENT) return; @@ -417,8 +441,12 @@ processPref64Option((StructNdOptPref64) msg.option, whenMs); break; + case StructNdOptRdnss.TYPE: + processRdnssOption((StructNdOptRdnss) msg.option); + break; + default: - // TODO: implement RDNSS and DNSSL. + // TODO: implement DNSSL. break; } }
diff --git a/src/android/net/ip/IpReachabilityMonitor.java b/src/android/net/ip/IpReachabilityMonitor.java index 070925b..0f199e2 100644 --- a/src/android/net/ip/IpReachabilityMonitor.java +++ b/src/android/net/ip/IpReachabilityMonitor.java
@@ -156,6 +156,8 @@ protected static final int NUD_MCAST_RESOLICIT_NUM = 3; private static final int INVALID_NUD_MCAST_RESOLICIT_NUM = -1; + private static final int INVALID_LEGACY_NUD_FAILURE_TYPE = -1; + public interface Callback { /** * This callback function must execute as quickly as possible as it is @@ -408,6 +410,7 @@ + " to: " + event.macAddr; mLog.w(logMsg); mCallback.notifyLost(event.ip, logMsg); + logNudFailed(event, NudEventType.NUD_MAC_ADDRESS_CHANGED); return; } maybeRestoreNeighborParameters(); @@ -446,6 +449,8 @@ final boolean lostProvisioning = (mLinkProperties.isIpv4Provisioned() && !whatIfLp.isIpv4Provisioned()) || (mLinkProperties.isIpv6Provisioned() && !whatIfLp.isIpv6Provisioned()); + final NudEventType type = getNudFailureEventType(isFromProbe(), + isNudFailureDueToRoam(), lostProvisioning); if (lostProvisioning) { final String logMsg = "FAILURE: LOST_PROVISIONING, " + event; @@ -454,7 +459,7 @@ // an InetAddress argument. mCallback.notifyLost(ip, logMsg); } - logNudFailed(event, lostProvisioning); + logNudFailed(event, type); } private void maybeRestoreNeighborParameters() { @@ -574,14 +579,16 @@ return duration < getProbeWakeLockDuration(); } - private boolean isProbedNudFailureDueToRoam() { + private boolean isNudFailureDueToRoam() { + if (!isFromProbe()) return false; + // Check to which probe expiry the curren timestamp gets close when NUD failure event // happens, theoretically that indicates which probe event(due to roam or CMD_CONFIRM) // was triggered eariler. // // Note that this would be incorrect if the probe or confirm was so long ago that the // probe duration has already expired. That cannot happen because isFromProbe would return - // false, and this method is only called on NUD failures due to probes. + // false. final long probeExpiryAfterRoam = mLastProbeDueToRoamMs + getProbeWakeLockDuration(); final long probeExpiryAfterConfirm = mLastProbeDueToConfirmMs + getProbeWakeLockDuration(); @@ -595,10 +602,15 @@ mMetricsLog.log(mInterfaceParams.name, new IpReachabilityEvent(eventType)); } - private void logNudFailed(final NeighborEvent event, boolean lostProvisioning) { - final int eventType = nudFailureEventType(isFromProbe(), lostProvisioning); + private void logNudFailed(final NeighborEvent event, final NudEventType type) { + logNeighborLostEvent(event, type); + + // The legacy metrics only record whether the failure came from a probe and whether + // the network is still provisioned. They do not record provisioning failures due to + // multicast resolicits finding that the MAC address has changed. + final int eventType = legacyNudFailureType(type); + if (eventType == INVALID_LEGACY_NUD_FAILURE_TYPE) return; mMetricsLog.log(mInterfaceParams.name, new IpReachabilityEvent(eventType)); - logNeighborLostEvent(event, lostProvisioning); } /** @@ -637,23 +649,32 @@ * Log NUD failure metrics with new Westworld APIs while the function using mMetricsLog API * still sends the legacy metrics, @see #logNudFailed. */ - private void logNeighborLostEvent(final NeighborEvent event, boolean isProvisioningLost) { + private void logNeighborLostEvent(final NeighborEvent event, final NudEventType type) { final IpType ipType = (event.ip instanceof Inet6Address) ? IpType.IPV6 : IpType.IPV4; mIpReachabilityMetrics.setNudIpType(ipType); mIpReachabilityMetrics.setNudNeighborType(getNeighborType(event)); - mIpReachabilityMetrics.setNudEventType(getNudFailureEventType(isFromProbe(), - isProbedNudFailureDueToRoam(), isProvisioningLost)); + mIpReachabilityMetrics.setNudEventType(type); mIpReachabilityMetrics.statsWrite(); } /** * Returns the NUD failure event type code corresponding to the given conditions. */ - private static int nudFailureEventType(boolean isFromProbe, boolean isProvisioningLost) { - if (isFromProbe) { - return isProvisioningLost ? PROVISIONING_LOST : NUD_FAILED; - } else { - return isProvisioningLost ? PROVISIONING_LOST_ORGANIC : NUD_FAILED_ORGANIC; + private static int legacyNudFailureType(final NudEventType type) { + switch (type) { + case NUD_POST_ROAMING_FAILED: + case NUD_CONFIRM_FAILED: + return NUD_FAILED; + case NUD_POST_ROAMING_FAILED_CRITICAL: + case NUD_CONFIRM_FAILED_CRITICAL: + return PROVISIONING_LOST; + case NUD_ORGANIC_FAILED: + return NUD_FAILED_ORGANIC; + case NUD_ORGANIC_FAILED_CRITICAL: + return PROVISIONING_LOST_ORGANIC; + default: + // Do not log legacy event + return INVALID_LEGACY_NUD_FAILURE_TYPE; } } }
diff --git a/src/android/net/util/NetworkStackUtils.java b/src/android/net/util/NetworkStackUtils.java index e06cdca..6dc2a5b 100755 --- a/src/android/net/util/NetworkStackUtils.java +++ b/src/android/net/util/NetworkStackUtils.java
@@ -250,6 +250,13 @@ "ipclient_garp_na_roaming_version"; /** + * Experiment flag to enable parsing netlink events from kernel directly instead from netd aidl + * interface. + */ + public static final String IPCLIENT_PARSE_NETLINK_EVENTS_VERSION = + "ipclient_parse_netlink_events_version"; + + /** * Experiment flag to disable accept_ra parameter when IPv6 provisioning loss happens due to * the default route has gone. */
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java index 3d2d804..cb0ee18 100755 --- a/src/com/android/server/connectivity/NetworkMonitor.java +++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -388,7 +388,7 @@ private static final int CMD_BANDWIDTH_CHECK_COMPLETE = 23; /** - * Message to self to know the bandwidth check is timeouted. + * Message to self to know the bandwidth check has timed out. */ private static final int CMD_BANDWIDTH_CHECK_TIMEOUT = 24; @@ -623,6 +623,10 @@ // even before notifyNetworkConnected. mLinkProperties = new LinkProperties(); mNetworkCapabilities = new NetworkCapabilities(null); + + Log.d(TAG, "Starting on network " + mNetwork + + " with capport HTTPS URL " + Arrays.toString(mCaptivePortalHttpsUrls) + + " and HTTP URL " + Arrays.toString(mCaptivePortalHttpUrls)); } /** @@ -636,6 +640,13 @@ /** * Request the NetworkMonitor to reevaluate the network. + * + * TODO : refactor reevaluation to introduce rate limiting. If the system finds a network is + * validated but some app can't access their server, or the network is behind a captive portal + * that only lets the validation URL through, apps may be calling reportNetworkConnectivity + * often, causing many revalidation attempts. Meanwhile, reevaluation attempts that result + * from actions that may affect the validation status (e.g. the user just logged in through + * the captive portal app) should never be skipped because of the rate limitation. */ public void forceReevaluation(int responsibleUid) { sendMessage(CMD_FORCE_REEVALUATION, responsibleUid, 0); @@ -916,6 +927,18 @@ // If the user wants to use this network anyway, there is no need to // perform the bandwidth check even if configured. mIsBandwidthCheckPassedOrIgnored = true; + // If the user wants to use this network anyway, it should always + // be reported as validated, but other checks still need to be + // done. For example, it should still validate strict private DNS and + // show a notification if not available, because the network will + // be unusable for this additional reason. + mEvaluationState.setCaptivePortalWantedAsIs(); + // A successful evaluation result should be reported immediately, so + // that the network stack may immediately use the validation in ranking + // without waiting for a possibly long private DNS or bandwidth eval + // step. + mEvaluationState.reportEvaluationResult(NETWORK_VALIDATION_RESULT_VALID, + null); // TODO: Distinguish this from a network that actually validates. // Displaying the "x" on the system UI icon may still be a good idea. transitionTo(mEvaluatingPrivateDnsState); @@ -1290,11 +1313,19 @@ // the network so don't bother validating here. Furthermore sending HTTP // packets over the network may be undesirable, for example an extremely // expensive metered network, or unwanted leaking of the User Agent string. + // Also don't bother validating networks that the user already said they + // wanted as-is. // // On networks that need to support private DNS in strict mode (e.g., VPNs, but // not networks that don't provide Internet access), we still need to perform // private DNS server resolution. - if (!isValidationRequired()) { + if (mEvaluationState.isCaptivePortalWantedAsIs() + && isPrivateDnsValidationRequired()) { + // Captive portals can only be detected on networks that validate both + // validation and private DNS validation. + validationLog("Captive portal is used as is, resolving private DNS"); + transitionTo(mEvaluatingPrivateDnsState); + } else if (!isValidationRequired()) { if (isPrivateDnsValidationRequired()) { validationLog("Network would not satisfy default request, " + "resolving private DNS"); @@ -1955,11 +1986,17 @@ } final long now = System.currentTimeMillis(); - if (expTime < now || (expTime - now) > TEST_URL_EXPIRATION_MS) return null; + if (expTime < now || (expTime - now) > TEST_URL_EXPIRATION_MS) { + logw("Skipping test URL with expiration " + expTime + ", now " + now); + return null; + } final String strUrl = mDependencies.getDeviceConfigProperty(NAMESPACE_CONNECTIVITY, key, null /* defaultValue */); - if (!isValidTestUrl(strUrl)) return null; + if (!isValidTestUrl(strUrl)) { + logw("Skipping invalid test URL " + strUrl); + return null; + } return makeURL(strUrl); } @@ -3390,18 +3427,28 @@ // NETWORK_VALIDATION_RESULT_VALID. But with this scheme, the first two or three validation // reports are all failures, because they are "HTTP succeeded but validation not yet passed", // "HTTP and HTTPS succeeded but validation not yet passed", etc. + // TODO : rename EvaluationState to not contain "State" in the name, as it makes this class + // sound like one of the states of the state machine, which it's not. @VisibleForTesting protected class EvaluationState { // The latest validation result for this network. This is a bitmask of // INetworkMonitor.NETWORK_VALIDATION_RESULT_* constants. private int mEvaluationResult = NETWORK_VALIDATION_RESULT_INVALID; + + + // Set when the captive portal app said this network should be used as is as a result + // of user interaction. The valid bit represents the user's decision to override automatic + // determination of whether the network has access to Internet, so in this case the + // network is always reported as validated. + // TODO : Make ConnectivityService aware of this state, so that it can use the network as + // the default without setting the VALIDATED bit, as it's a bit of a lie. This can't be + // done on Android <= R where CS can't be updated, but it is doable on S+. + private boolean mCaptivePortalWantedAsIs = false; // Indicates which probes have succeeded since clearProbeResults was called. // This is a bitmask of INetworkMonitor.NETWORK_VALIDATION_PROBE_* constants. private int mProbeResults = 0; // A bitmask to record which probes are completed. private int mProbeCompleted = 0; - // The latest redirect URL. - private String mRedirectUrl; protected void clearProbeResults() { mProbeResults = 0; @@ -3435,17 +3482,26 @@ }); } + protected void setCaptivePortalWantedAsIs() { + mCaptivePortalWantedAsIs = true; + } + + protected boolean isCaptivePortalWantedAsIs() { + return mCaptivePortalWantedAsIs; + } + protected void reportEvaluationResult(int result, @Nullable String redirectUrl) { - if (!isValidationRequired() && mProbeCompleted == 0 && ShimUtils.isAtLeastS()) { + if (mCaptivePortalWantedAsIs) { + result = NETWORK_VALIDATION_RESULT_VALID; + } else if (!isValidationRequired() && mProbeCompleted == 0 && mCallbackVersion >= 11) { // If validation is not required AND no probes were attempted, the validation was // skipped. Report this to ConnectivityService for ConnectivityDiagnostics, but only - // if the platform is Android S+, as ConnectivityService must also know how to - // understand this bit. + // if the platform has callback version 11+, as ConnectivityService must also know + // how to understand this bit. result |= NETWORK_VALIDATION_RESULT_SKIPPED; } mEvaluationResult = result; - mRedirectUrl = redirectUrl; final NetworkTestResultParcelable p = new NetworkTestResultParcelable(); p.result = result; p.probesSucceeded = mProbeResults;
diff --git a/tests/unit/Android.bp b/tests/unit/Android.bp index a74d018..fa054fe 100644 --- a/tests/unit/Android.bp +++ b/tests/unit/Android.bp
@@ -56,10 +56,7 @@ min_sdk_version: "29", srcs: [], // TODO: tests that only apply to the current, non-stable API can be added here test_suites: ["general-tests"], - test_mainline_modules: [ - "CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex", - "CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex" - ], + test_mainline_modules: ["CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex"], defaults: ["NetworkStackTestsDefaults"], static_libs: ["NetworkStackApiCurrentLib"], compile_multilib: "both", // Workaround for b/147785146 for mainline-presubmit @@ -86,10 +83,7 @@ min_sdk_version: "29", target_sdk_version: "30", test_suites: ["general-tests", "mts"], - test_mainline_modules: [ - "CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex", - "CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex" - ], + test_mainline_modules: ["CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex"], defaults: ["NetworkStackTestsDefaults"], static_libs: ["NetworkStackApiStableLib"], compile_multilib: "both",
diff --git a/tests/unit/src/android/net/ip/IpReachabilityMonitorTest.kt b/tests/unit/src/android/net/ip/IpReachabilityMonitorTest.kt index 3f0ee2f..ea8f1da 100644 --- a/tests/unit/src/android/net/ip/IpReachabilityMonitorTest.kt +++ b/tests/unit/src/android/net/ip/IpReachabilityMonitorTest.kt
@@ -25,6 +25,7 @@ import android.net.RouteInfo import android.net.metrics.IpConnectivityLog import android.net.util.InterfaceParams +import android.net.util.NetworkStackUtils.IP_REACHABILITY_MCAST_RESOLICIT_VERSION import android.net.util.SharedLog import android.os.Handler import android.os.HandlerThread @@ -36,6 +37,7 @@ import android.stats.connectivity.NudEventType import android.stats.connectivity.NudEventType.NUD_CONFIRM_FAILED import android.stats.connectivity.NudEventType.NUD_CONFIRM_FAILED_CRITICAL +import android.stats.connectivity.NudEventType.NUD_MAC_ADDRESS_CHANGED import android.stats.connectivity.NudEventType.NUD_POST_ROAMING_FAILED import android.stats.connectivity.NudEventType.NUD_POST_ROAMING_FAILED_CRITICAL import android.stats.connectivity.NudEventType.NUD_ORGANIC_FAILED @@ -50,6 +52,7 @@ import androidx.test.runner.AndroidJUnit4 import com.android.networkstack.metrics.IpReachabilityMonitorMetrics import com.android.net.module.util.netlink.StructNdMsg.NUD_FAILED +import com.android.net.module.util.netlink.StructNdMsg.NUD_REACHABLE import com.android.net.module.util.netlink.StructNdMsg.NUD_STALE import com.android.testutils.makeNewNeighMessage import com.android.testutils.waitForIdle @@ -59,7 +62,9 @@ import org.junit.runner.RunWith import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.anyObject import org.mockito.ArgumentMatchers.anyString import org.mockito.ArgumentMatchers.eq import org.mockito.Mockito.doAnswer @@ -324,6 +329,29 @@ verifyNudFailureMetrics(eventType, ipType, lostNeighborType) } + private fun runNeighborReachableButMacAddrChangedTest( + newLp: LinkProperties, + neighbor: InetAddress, + ipType: IpType + ) { + doReturn(true).`when`(dependencies).isFeatureEnabled(anyObject(), + eq(IP_REACHABILITY_MCAST_RESOLICIT_VERSION), anyBoolean()) + + reachabilityMonitor.updateLinkProperties(newLp) + + neighborMonitor.enqueuePacket(makeNewNeighMessage(neighbor, NUD_REACHABLE, + "001122334455" /* oldMac */)) + handlerThread.waitForIdle(TEST_TIMEOUT_MS) + verify(callback, never()).notifyLost(eq(neighbor), anyString()) + + reachabilityMonitor.probeAll(true /* dueToRoam */) + + neighborMonitor.enqueuePacket(makeNewNeighMessage(neighbor, NUD_REACHABLE, + "1122334455aa" /* newMac */)) + verify(callback, timeout(TEST_TIMEOUT_MS)).notifyLost(eq(neighbor), anyString()) + verifyNudFailureMetrics(NUD_MAC_ADDRESS_CHANGED, ipType, NUD_NEIGHBOR_GATEWAY) + } + @Test fun testLoseProvisioning_Ipv4DnsLost() { runLoseProvisioningTest(TEST_LINK_PROPERTIES, TEST_IPV4_DNS) @@ -519,4 +547,14 @@ verifyNudFailureMetrics(NUD_CONFIRM_FAILED_CRITICAL, IPV6, NUD_NEIGHBOR_GATEWAY) } + + @Test + fun testNudProbeFailedMetrics_defaultIPv6GatewayMacAddrChanged() { + runNeighborReachableButMacAddrChangedTest(TEST_LINK_PROPERTIES, TEST_IPV6_GATEWAY, IPV6) + } + + @Test + fun testNudProbeFailedMetrics_defaultIPv4GatewayMacAddrChanged() { + runNeighborReachableButMacAddrChangedTest(TEST_LINK_PROPERTIES, TEST_IPV4_GATEWAY, IPV4) + } }
diff --git a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java index 606d243..9d392eb 100644 --- a/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java +++ b/tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -17,6 +17,7 @@ package com.android.server.connectivity; import static android.net.CaptivePortal.APP_RETURN_DISMISSED; +import static android.net.CaptivePortal.APP_RETURN_WANTED_AS_IS; import static android.net.DnsResolver.TYPE_A; import static android.net.DnsResolver.TYPE_AAAA; import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_DNS; @@ -588,7 +589,7 @@ } private void resetCallbacks() { - resetCallbacks(6); + resetCallbacks(11); } private void resetCallbacks(int interfaceVersion) { @@ -1795,12 +1796,8 @@ runFailedNetworkTest(); } - private void doValidationSkippedTest(NetworkCapabilities nc) throws Exception { - // For S+, the RESULT_SKIPPED bit will be included on networks that both do not require - // validation and for which validation is not performed. - final int validationResult = ShimUtils.isAtLeastS() - ? NETWORK_VALIDATION_RESULT_VALID | NETWORK_VALIDATION_RESULT_SKIPPED - : NETWORK_VALIDATION_RESULT_VALID; + private void doValidationSkippedTest(NetworkCapabilities nc, int validationResult) + throws Exception { runNetworkTest(TEST_LINK_PROPERTIES, nc, validationResult, 0 /* probesSucceeded */, null /* redirectUrl */); verify(mCleartextDnsNetwork, never()).openConnection(any()); @@ -1808,7 +1805,15 @@ @Test public void testNoInternetCapabilityValidated() throws Exception { - doValidationSkippedTest(CELL_NO_INTERNET_CAPABILITIES); + doValidationSkippedTest(CELL_NO_INTERNET_CAPABILITIES, + NETWORK_VALIDATION_RESULT_VALID | NETWORK_VALIDATION_RESULT_SKIPPED); + } + + @Test + public void testNoInternetCapabilityValidated_OlderPlatform() throws Exception { + // Before callbacks version 11, NETWORK_VALIDATION_RESULT_SKIPPED is not sent + resetCallbacks(10); + doValidationSkippedTest(CELL_NO_INTERNET_CAPABILITIES, NETWORK_VALIDATION_RESULT_VALID); } @Test @@ -1821,7 +1826,8 @@ if (ShimUtils.isAtLeastS()) { nc.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); } - doValidationSkippedTest(nc); + doValidationSkippedTest(nc, + NETWORK_VALIDATION_RESULT_VALID | NETWORK_VALIDATION_RESULT_SKIPPED); } @Test @@ -1834,7 +1840,8 @@ if (ShimUtils.isAtLeastS()) { nc.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED); } - doValidationSkippedTest(nc); + doValidationSkippedTest(nc, + NETWORK_VALIDATION_RESULT_VALID | NETWORK_VALIDATION_RESULT_SKIPPED); } private NetworkCapabilities getVcnUnderlyingCarrierWifiCaps() { @@ -1878,12 +1885,10 @@ nm.getEvaluationState().getEvaluationResult()); } - @Test - public void testLaunchCaptivePortalApp() throws Exception { + public void setupAndLaunchCaptivePortalApp(final NetworkMonitor nm) throws Exception { setSslException(mHttpsConnection); setPortal302(mHttpConnection); when(mHttpConnection.getHeaderField(eq("location"))).thenReturn(TEST_LOGIN_URL); - final NetworkMonitor nm = makeMonitor(CELL_METERED_CAPABILITIES); notifyNetworkConnected(nm, CELL_METERED_CAPABILITIES); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1)) @@ -1910,11 +1915,18 @@ final String redirectUrl = bundle.getString(ConnectivityManager.EXTRA_CAPTIVE_PORTAL_URL); assertEquals(TEST_HTTP_URL, redirectUrl); + resetCallbacks(); + } + + @Test + public void testCaptivePortalLogin() throws Exception { + final NetworkMonitor nm = makeMonitor(CELL_METERED_CAPABILITIES); + setupAndLaunchCaptivePortalApp(nm); + // Have the app report that the captive portal is dismissed, and check that we revalidate. setStatus(mHttpsConnection, 204); setStatus(mHttpConnection, 204); - resetCallbacks(); nm.notifyCaptivePortalAppFinished(APP_RETURN_DISMISSED); verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce()) .notifyNetworkTestedWithExtras(matchNetworkTestResultParcelable( @@ -1924,6 +1936,31 @@ } @Test + public void testCaptivePortalUseAsIs() throws Exception { + final NetworkMonitor nm = makeMonitor(CELL_METERED_CAPABILITIES); + setupAndLaunchCaptivePortalApp(nm); + + // The user decides this network is wanted as is, either by encountering an SSL error or + // encountering an unknown scheme and then deciding to continue through the browser, or by + // selecting this option through the options menu. + nm.notifyCaptivePortalAppFinished(APP_RETURN_WANTED_AS_IS); + // The captive portal is still closed, but the network validates since the user said so. + verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce()) + .notifyNetworkTestedWithExtras(matchNetworkTestResultParcelable( + NETWORK_VALIDATION_RESULT_VALID, 0 /* probesSucceeded */)); + resetCallbacks(); + + // Revalidate. + nm.forceReevaluation(0 /* responsibleUid */); + + // The network should still be valid. + verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).atLeastOnce()) + .notifyNetworkTestedWithExtras(matchNetworkTestResultParcelable( + NETWORK_VALIDATION_RESULT_VALID, 0 /* probesSucceeded */, + TEST_LOGIN_URL)); + } + + @Test public void testPrivateDnsSuccess() throws Exception { setStatus(mHttpsConnection, 204); setStatus(mHttpConnection, 204); @@ -2758,9 +2795,8 @@ new NetworkCapabilities(WIFI_OEM_PAID_CAPABILITIES); networkCapabilities.removeCapability(NET_CAPABILITY_INTERNET); - final int validationResult = ShimUtils.isAtLeastS() - ? NETWORK_VALIDATION_RESULT_VALID | NETWORK_VALIDATION_RESULT_SKIPPED - : NETWORK_VALIDATION_RESULT_VALID; + final int validationResult = + NETWORK_VALIDATION_RESULT_VALID | NETWORK_VALIDATION_RESULT_SKIPPED; runNetworkTest(TEST_LINK_PROPERTIES, networkCapabilities, validationResult, 0 /* probesSucceeded */, null /* redirectUrl */);