From 2ea91aee424a68e209bb487f245e8f2964213fdd Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Tue, 3 Apr 2018 20:30:54 -0700 Subject: [PATCH 1/3] Update IpSecManager to use InetAddress and prefixLen LinkAddress constructors are currently @hide; this change updates IpSecManager to use InetAddress and prefixLen, and then construct a LinkAddress internally. LinkAddress is used over the binder interface to IpSecService to ensure validity. Bug: 77528639 Test: CTS, Java unit tests ran on walleye Change-Id: I19e124adef6d9f4992d8293db3190bcf74c95848 --- tests/net/java/android/net/IpSecManagerTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/net/java/android/net/IpSecManagerTest.java b/tests/net/java/android/net/IpSecManagerTest.java index a946e50585..13210e8972 100644 --- a/tests/net/java/android/net/IpSecManagerTest.java +++ b/tests/net/java/android/net/IpSecManagerTest.java @@ -260,12 +260,14 @@ public class IpSecManagerTest { IpSecManager.IpSecTunnelInterface tunnelIntf = createAndValidateVti(DUMMY_RESOURCE_ID, VTI_INTF_NAME); - tunnelIntf.addAddress(VTI_INNER_ADDRESS); + tunnelIntf.addAddress(VTI_INNER_ADDRESS.getAddress(), + VTI_INNER_ADDRESS.getPrefixLength()); verify(mMockIpSecService) .addAddressToTunnelInterface( eq(DUMMY_RESOURCE_ID), eq(VTI_INNER_ADDRESS), anyString()); - tunnelIntf.removeAddress(VTI_INNER_ADDRESS); + tunnelIntf.removeAddress(VTI_INNER_ADDRESS.getAddress(), + VTI_INNER_ADDRESS.getPrefixLength()); verify(mMockIpSecService) .addAddressToTunnelInterface( eq(DUMMY_RESOURCE_ID), eq(VTI_INNER_ADDRESS), anyString()); From 471ce709e86a900823ba99a932f914b3681c2061 Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Wed, 21 Mar 2018 07:18:33 -0700 Subject: [PATCH 2/3] Move the logic of (re)evaluation of Private DNS Moves this out of ConnectivityService and into each NetworkMonitor (where it's more self-contained). Test: as follows - builds, flashes, boots - runtest frameworks-net passes - manual testing with working and non-working hostnames behaves somewhat (but not entirely) as expected, and not always quickly Bug: 64133961 Bug: 72345192 Bug: 73872000 Bug: 77140445 Merged-In: I5dc90ecfe6f6f10967b7501645ad8e030cb38982 Merged-In: Ida4967d22f0781524f0f269e30e653b8ec867258 Change-Id: Ic4322af3cb49149f2d975cb31f54b2ac7927f907 (cherry picked from commit 736353a584aa89a29e737e21e29c49fad0d38a63) --- .../android/server/ConnectivityService.java | 143 ++++++------------ .../server/connectivity/DnsManager.java | 45 ++++++ .../server/ConnectivityServiceTest.java | 134 +++++++++++++--- 3 files changed, 210 insertions(+), 112 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index a1ef1ed9e9..59cb135627 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -934,7 +934,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Used only for testing. // TODO: Delete this and either: - // 1. Give Fake SettingsProvider the ability to send settings change notifications (requires + // 1. Give FakeSettingsProvider the ability to send settings change notifications (requires // changing ContentResolver to make registerContentObserver non-final). // 2. Give FakeSettingsProvider an alternative notification mechanism and have the test use it // by subclassing SettingsObserver. @@ -943,6 +943,12 @@ public class ConnectivityService extends IConnectivityManager.Stub mHandler.sendEmptyMessage(EVENT_CONFIGURE_MOBILE_DATA_ALWAYS_ON); } + // See FakeSettingsProvider comment above. + @VisibleForTesting + void updatePrivateDnsSettings() { + mHandler.sendEmptyMessage(EVENT_PRIVATE_DNS_SETTINGS_CHANGED); + } + private void handleMobileDataAlwaysOn() { final boolean enable = toBool(Settings.Global.getInt( mContext.getContentResolver(), Settings.Global.MOBILE_DATA_ALWAYS_ON, 1)); @@ -972,8 +978,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void registerPrivateDnsSettingsCallbacks() { - for (Uri u : DnsManager.getPrivateDnsSettingsUris()) { - mSettingsObserver.observe(u, EVENT_PRIVATE_DNS_SETTINGS_CHANGED); + for (Uri uri : DnsManager.getPrivateDnsSettingsUris()) { + mSettingsObserver.observe(uri, EVENT_PRIVATE_DNS_SETTINGS_CHANGED); } } @@ -1026,8 +1032,12 @@ public class ConnectivityService extends IConnectivityManager.Stub if (network == null) { return null; } + return getNetworkAgentInfoForNetId(network.netId); + } + + private NetworkAgentInfo getNetworkAgentInfoForNetId(int netId) { synchronized (mNetworkForNetId) { - return mNetworkForNetId.get(network.netId); + return mNetworkForNetId.get(netId); } } @@ -1167,9 +1177,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } NetworkAgentInfo nai; if (vpnNetId != NETID_UNSET) { - synchronized (mNetworkForNetId) { - nai = mNetworkForNetId.get(vpnNetId); - } + nai = getNetworkAgentInfoForNetId(vpnNetId); if (nai != null) return nai.network; } nai = getDefaultNetwork(); @@ -2155,41 +2163,21 @@ public class ConnectivityService extends IConnectivityManager.Stub default: return false; case NetworkMonitor.EVENT_NETWORK_TESTED: { - final NetworkAgentInfo nai; - synchronized (mNetworkForNetId) { - nai = mNetworkForNetId.get(msg.arg2); - } + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(msg.arg2); if (nai == null) break; final boolean valid = (msg.arg1 == NetworkMonitor.NETWORK_TEST_RESULT_VALID); final boolean wasValidated = nai.lastValidated; final boolean wasDefault = isDefaultNetwork(nai); - final PrivateDnsConfig privateDnsCfg = (msg.obj instanceof PrivateDnsConfig) - ? (PrivateDnsConfig) msg.obj : null; final String redirectUrl = (msg.obj instanceof String) ? (String) msg.obj : ""; - final boolean reevaluationRequired; - final String logMsg; - if (valid) { - reevaluationRequired = updatePrivateDns(nai, privateDnsCfg); - logMsg = (DBG && (privateDnsCfg != null)) - ? " with " + privateDnsCfg.toString() : ""; - } else { - reevaluationRequired = false; - logMsg = (DBG && !TextUtils.isEmpty(redirectUrl)) - ? " with redirect to " + redirectUrl : ""; - } if (DBG) { + final String logMsg = !TextUtils.isEmpty(redirectUrl) + ? " with redirect to " + redirectUrl + : ""; log(nai.name() + " validation " + (valid ? "passed" : "failed") + logMsg); } - // If there is a change in Private DNS configuration, - // trigger reevaluation of the network to test it. - if (reevaluationRequired) { - nai.networkMonitor.sendMessage( - NetworkMonitor.CMD_FORCE_REEVALUATION, Process.SYSTEM_UID); - break; - } if (valid != nai.lastValidated) { if (wasDefault) { metricsLogger().defaultNetworkMetrics().logDefaultNetworkValidity( @@ -2218,10 +2206,7 @@ public class ConnectivityService extends IConnectivityManager.Stub case NetworkMonitor.EVENT_PROVISIONING_NOTIFICATION: { final int netId = msg.arg2; final boolean visible = toBool(msg.arg1); - final NetworkAgentInfo nai; - synchronized (mNetworkForNetId) { - nai = mNetworkForNetId.get(netId); - } + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(netId); // If captive portal status has changed, update capabilities or disconnect. if (nai != null && (visible != nai.lastCaptivePortalDetected)) { final int oldScore = nai.getCurrentScore(); @@ -2252,18 +2237,10 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case NetworkMonitor.EVENT_PRIVATE_DNS_CONFIG_RESOLVED: { - final NetworkAgentInfo nai; - synchronized (mNetworkForNetId) { - nai = mNetworkForNetId.get(msg.arg2); - } + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(msg.arg2); if (nai == null) break; - final PrivateDnsConfig cfg = (PrivateDnsConfig) msg.obj; - final boolean reevaluationRequired = updatePrivateDns(nai, cfg); - if (nai.lastValidated && reevaluationRequired) { - nai.networkMonitor.sendMessage( - NetworkMonitor.CMD_FORCE_REEVALUATION, Process.SYSTEM_UID); - } + updatePrivateDns(nai, (PrivateDnsConfig) msg.obj); break; } } @@ -2301,61 +2278,38 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private boolean networkRequiresValidation(NetworkAgentInfo nai) { + return NetworkMonitor.isValidationRequired( + mDefaultRequest.networkCapabilities, nai.networkCapabilities); + } + private void handlePrivateDnsSettingsChanged() { final PrivateDnsConfig cfg = mDnsManager.getPrivateDnsConfig(); for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { - // Private DNS only ever applies to networks that might provide - // Internet access and therefore also require validation. - if (!NetworkMonitor.isValidationRequired( - mDefaultRequest.networkCapabilities, nai.networkCapabilities)) { - continue; - } - - // Notify the NetworkMonitor thread in case it needs to cancel or - // schedule DNS resolutions. If a DNS resolution is required the - // result will be sent back to us. - nai.networkMonitor.notifyPrivateDnsSettingsChanged(cfg); - - if (!cfg.inStrictMode()) { - // No strict mode hostname DNS resolution needed, so just update - // DNS settings directly. In opportunistic and "off" modes this - // just reprograms netd with the network-supplied DNS servers - // (and of course the boolean of whether or not to attempt TLS). - // - // TODO: Consider code flow parity with strict mode, i.e. having - // NetworkMonitor relay the PrivateDnsConfig back to us and then - // performing this call at that time. - updatePrivateDns(nai, cfg); - } + handlePerNetworkPrivateDnsConfig(nai, cfg); } } - private boolean updatePrivateDns(NetworkAgentInfo nai, PrivateDnsConfig newCfg) { - final boolean reevaluationRequired = true; - final boolean dontReevaluate = false; + private void handlePerNetworkPrivateDnsConfig(NetworkAgentInfo nai, PrivateDnsConfig cfg) { + // Private DNS only ever applies to networks that might provide + // Internet access and therefore also require validation. + if (!networkRequiresValidation(nai)) return; - final PrivateDnsConfig oldCfg = mDnsManager.updatePrivateDns(nai.network, newCfg); + // Notify the NetworkMonitor thread in case it needs to cancel or + // schedule DNS resolutions. If a DNS resolution is required the + // result will be sent back to us. + nai.networkMonitor.notifyPrivateDnsSettingsChanged(cfg); + + // With Private DNS bypass support, we can proceed to update the + // Private DNS config immediately, even if we're in strict mode + // and have not yet resolved the provider name into a set of IPs. + updatePrivateDns(nai, cfg); + } + + private void updatePrivateDns(NetworkAgentInfo nai, PrivateDnsConfig newCfg) { + mDnsManager.updatePrivateDns(nai.network, newCfg); updateDnses(nai.linkProperties, null, nai.network.netId); - - if (newCfg == null) { - if (oldCfg == null) return dontReevaluate; - return oldCfg.useTls ? reevaluationRequired : dontReevaluate; - } - - if (oldCfg == null) { - return newCfg.useTls ? reevaluationRequired : dontReevaluate; - } - - if (oldCfg.useTls != newCfg.useTls) { - return reevaluationRequired; - } - - if (newCfg.inStrictMode() && !Objects.equals(oldCfg.hostname, newCfg.hostname)) { - return reevaluationRequired; - } - - return dontReevaluate; } private void updateLingerState(NetworkAgentInfo nai, long now) { @@ -3300,7 +3254,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (isNetworkWithLinkPropertiesBlocked(lp, uid, false)) { return; } - nai.networkMonitor.sendMessage(NetworkMonitor.CMD_FORCE_REEVALUATION, uid); + nai.networkMonitor.forceReevaluation(uid); } private ProxyInfo getDefaultProxy() { @@ -4919,7 +4873,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } public void handleUpdateLinkProperties(NetworkAgentInfo nai, LinkProperties newLp) { - if (mNetworkForNetId.get(nai.network.netId) != nai) { + if (getNetworkAgentInfoForNetId(nai.network.netId) != nai) { // Ignore updates for disconnected networks return; } @@ -5495,6 +5449,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (!networkAgent.everConnected && state == NetworkInfo.State.CONNECTED) { networkAgent.everConnected = true; + handlePerNetworkPrivateDnsConfig(networkAgent, mDnsManager.getPrivateDnsConfig()); updateLinkProperties(networkAgent, null); notifyIfacesChangedForNetworkStats(); @@ -5911,4 +5866,4 @@ public class ConnectivityService extends IConnectivityManager.Stub pw.println(" Get airplane mode."); } } -} \ No newline at end of file +} diff --git a/services/core/java/com/android/server/connectivity/DnsManager.java b/services/core/java/com/android/server/connectivity/DnsManager.java index 55798491cd..2a361a02d7 100644 --- a/services/core/java/com/android/server/connectivity/DnsManager.java +++ b/services/core/java/com/android/server/connectivity/DnsManager.java @@ -61,6 +61,51 @@ import java.util.StringJoiner; * This class it NOT designed for concurrent access. Furthermore, all non-static * methods MUST be called from ConnectivityService's thread. * + * [ Private DNS ] + * The code handling Private DNS is spread across several components, but this + * seems like the least bad place to collect all the observations. + * + * Private DNS handling and updating occurs in response to several different + * events. Each is described here with its corresponding intended handling. + * + * [A] Event: A new network comes up. + * Mechanics: + * [1] ConnectivityService gets notifications from NetworkAgents. + * [2] in updateNetworkInfo(), the first time the NetworkAgent goes into + * into CONNECTED state, the Private DNS configuration is retrieved, + * programmed, and strict mode hostname resolution (if applicable) is + * enqueued in NetworkAgent's NetworkMonitor, via a call to + * handlePerNetworkPrivateDnsConfig(). + * [3] Re-resolution of strict mode hostnames that fail to return any + * IP addresses happens inside NetworkMonitor; it sends itself a + * delayed CMD_EVALUATE_PRIVATE_DNS message in a simple backoff + * schedule. + * [4] Successfully resolved hostnames are sent to ConnectivityService + * inside an EVENT_PRIVATE_DNS_CONFIG_RESOLVED message. The resolved + * IP addresses are programmed into netd via: + * + * updatePrivateDns() -> updateDnses() + * + * both of which make calls into DnsManager. + * [5] Upon a successful hostname resolution NetworkMonitor initiates a + * validation attempt in the form of a lookup for a one-time hostname + * that uses Private DNS. + * + * [B] Event: Private DNS settings are changed. + * Mechanics: + * [1] ConnectivityService gets notifications from its SettingsObserver. + * [2] handlePrivateDnsSettingsChanged() is called, which calls + * handlePerNetworkPrivateDnsConfig() and the process proceeds + * as if from A.3 above. + * + * [C] Event: An application calls ConnectivityManager#reportBadNetwork(). + * Mechanics: + * [1] NetworkMonitor is notified and initiates a reevaluation, which + * always bypasses Private DNS. + * [2] Once completed, NetworkMonitor checks if strict mode is in operation + * and if so enqueues another evaluation of Private DNS, as if from + * step A.5 above. + * * @hide */ public class DnsManager { diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 163dd2a7e9..b0e11c4e8b 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -17,6 +17,9 @@ package com.android.server; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; +import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OFF; +import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OPPORTUNISTIC; +import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_PROVIDER_HOSTNAME; import static android.net.ConnectivityManager.TYPE_ETHERNET; import static android.net.ConnectivityManager.TYPE_MOBILE; import static android.net.ConnectivityManager.TYPE_MOBILE_FOTA; @@ -70,6 +73,7 @@ import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -181,6 +185,9 @@ public class ConnectivityServiceTest { private static final int TIMEOUT_MS = 500; private static final int TEST_LINGER_DELAY_MS = 120; + private static final String MOBILE_IFNAME = "test_rmnet_data0"; + private static final String WIFI_IFNAME = "test_wlan0"; + private MockContext mServiceContext; private WrappedConnectivityService mService; private WrappedConnectivityManager mCm; @@ -751,7 +758,7 @@ public class ConnectivityServiceTest { // NetworkMonitor implementation allowing overriding of Internet connectivity probe result. private class WrappedNetworkMonitor extends NetworkMonitor { - public Handler connectivityHandler; + public final Handler connectivityHandler; // HTTP response code fed back to NetworkMonitor for Internet connectivity probe. public int gen204ProbeResult = 500; public String gen204ProbeRedirectUrl = null; @@ -928,6 +935,7 @@ public class ConnectivityServiceTest { // Ensure that the default setting for Captive Portals is used for most tests setCaptivePortalMode(Settings.Global.CAPTIVE_PORTAL_MODE_PROMPT); setMobileDataAlwaysOn(false); + setPrivateDnsSettings(PRIVATE_DNS_MODE_OFF, "ignored.example.com"); } @After @@ -2582,6 +2590,14 @@ public class ConnectivityServiceTest { waitForIdle(); } + private void setPrivateDnsSettings(String mode, String specifier) { + final ContentResolver cr = mServiceContext.getContentResolver(); + Settings.Global.putString(cr, Settings.Global.PRIVATE_DNS_MODE, mode); + Settings.Global.putString(cr, Settings.Global.PRIVATE_DNS_SPECIFIER, specifier); + mService.updatePrivateDnsSettings(); + waitForIdle(); + } + private boolean isForegroundNetwork(MockNetworkAgent network) { NetworkCapabilities nc = mCm.getNetworkCapabilities(network.getNetwork()); assertNotNull(nc); @@ -3583,7 +3599,7 @@ public class ConnectivityServiceTest { mCm.registerNetworkCallback(networkRequest, networkCallback); LinkProperties lp = new LinkProperties(); - lp.setInterfaceName("wlan0"); + lp.setInterfaceName(WIFI_IFNAME); LinkAddress myIpv4Address = new LinkAddress("192.168.12.3/24"); RouteInfo myIpv4DefaultRoute = new RouteInfo((IpPrefix) null, NetworkUtils.numericToInetAddress("192.168.12.1"), lp.getInterfaceName()); @@ -3672,52 +3688,63 @@ public class ConnectivityServiceTest { @Test public void testBasicDnsConfigurationPushed() throws Exception { - final String IFNAME = "test_rmnet_data0"; - final String[] EMPTY_TLS_SERVERS = new String[0]; + setPrivateDnsSettings(PRIVATE_DNS_MODE_OPPORTUNISTIC, "ignored.example.com"); + ArgumentCaptor tlsServers = ArgumentCaptor.forClass(String[].class); + + // Clear any interactions that occur as a result of CS starting up. + reset(mNetworkManagementService); + + final String[] EMPTY_STRING_ARRAY = new String[0]; mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); waitForIdle(); verify(mNetworkManagementService, never()).setDnsConfigurationForNetwork( - anyInt(), any(), any(), any(), anyString(), eq(EMPTY_TLS_SERVERS)); + anyInt(), eq(EMPTY_STRING_ARRAY), any(), any(), eq(""), eq(EMPTY_STRING_ARRAY)); + verifyNoMoreInteractions(mNetworkManagementService); final LinkProperties cellLp = new LinkProperties(); - cellLp.setInterfaceName(IFNAME); + cellLp.setInterfaceName(MOBILE_IFNAME); // Add IPv4 and IPv6 default routes, because DNS-over-TLS code does // "is-reachable" testing in order to not program netd with unreachable // nameservers that it might try repeated to validate. cellLp.addLinkAddress(new LinkAddress("192.0.2.4/24")); - cellLp.addRoute(new RouteInfo((IpPrefix) null, InetAddress.getByName("192.0.2.4"), IFNAME)); + cellLp.addRoute(new RouteInfo((IpPrefix) null, InetAddress.getByName("192.0.2.4"), + MOBILE_IFNAME)); cellLp.addLinkAddress(new LinkAddress("2001:db8:1::1/64")); - cellLp.addRoute( - new RouteInfo((IpPrefix) null, InetAddress.getByName("2001:db8:1::1"), IFNAME)); + cellLp.addRoute(new RouteInfo((IpPrefix) null, InetAddress.getByName("2001:db8:1::1"), + MOBILE_IFNAME)); mCellNetworkAgent.sendLinkProperties(cellLp); mCellNetworkAgent.connect(false); waitForIdle(); - verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( - anyInt(), mStringArrayCaptor.capture(), any(), any(), - anyString(), eq(EMPTY_TLS_SERVERS)); // CS tells netd about the empty DNS config for this network. - assertEmpty(mStringArrayCaptor.getValue()); + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( + anyInt(), eq(EMPTY_STRING_ARRAY), any(), any(), eq(""), eq(EMPTY_STRING_ARRAY)); reset(mNetworkManagementService); cellLp.addDnsServer(InetAddress.getByName("2001:db8::1")); mCellNetworkAgent.sendLinkProperties(cellLp); waitForIdle(); - verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( anyInt(), mStringArrayCaptor.capture(), any(), any(), - anyString(), eq(EMPTY_TLS_SERVERS)); + eq(""), tlsServers.capture()); assertEquals(1, mStringArrayCaptor.getValue().length); assertTrue(ArrayUtils.contains(mStringArrayCaptor.getValue(), "2001:db8::1")); + // Opportunistic mode. + assertTrue(ArrayUtils.contains(tlsServers.getValue(), "2001:db8::1")); reset(mNetworkManagementService); cellLp.addDnsServer(InetAddress.getByName("192.0.2.1")); mCellNetworkAgent.sendLinkProperties(cellLp); waitForIdle(); - verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( anyInt(), mStringArrayCaptor.capture(), any(), any(), - anyString(), eq(EMPTY_TLS_SERVERS)); + eq(""), tlsServers.capture()); assertEquals(2, mStringArrayCaptor.getValue().length); assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), new String[]{"2001:db8::1", "192.0.2.1"})); + // Opportunistic mode. + assertEquals(2, tlsServers.getValue().length); + assertTrue(ArrayUtils.containsAll(tlsServers.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); reset(mNetworkManagementService); final String TLS_SPECIFIER = "tls.example.com"; @@ -3730,7 +3757,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent.getNetwork().netId, new DnsManager.PrivateDnsConfig(TLS_SPECIFIER, TLS_IPS))); waitForIdle(); - verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( anyInt(), mStringArrayCaptor.capture(), any(), any(), eq(TLS_SPECIFIER), eq(TLS_SERVERS)); assertEquals(2, mStringArrayCaptor.getValue().length); @@ -3739,6 +3766,77 @@ public class ConnectivityServiceTest { reset(mNetworkManagementService); } + @Test + public void testPrivateDnsSettingsChange() throws Exception { + final String[] EMPTY_STRING_ARRAY = new String[0]; + ArgumentCaptor tlsServers = ArgumentCaptor.forClass(String[].class); + + // Clear any interactions that occur as a result of CS starting up. + reset(mNetworkManagementService); + + // The default on Android is opportunistic mode ("Automatic"). + setPrivateDnsSettings(PRIVATE_DNS_MODE_OPPORTUNISTIC, "ignored.example.com"); + + mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); + waitForIdle(); + // CS tells netd about the empty DNS config for this network. + verify(mNetworkManagementService, never()).setDnsConfigurationForNetwork( + anyInt(), eq(EMPTY_STRING_ARRAY), any(), any(), eq(""), eq(EMPTY_STRING_ARRAY)); + verifyNoMoreInteractions(mNetworkManagementService); + + final LinkProperties cellLp = new LinkProperties(); + cellLp.setInterfaceName(MOBILE_IFNAME); + // Add IPv4 and IPv6 default routes, because DNS-over-TLS code does + // "is-reachable" testing in order to not program netd with unreachable + // nameservers that it might try repeated to validate. + cellLp.addLinkAddress(new LinkAddress("192.0.2.4/24")); + cellLp.addRoute(new RouteInfo((IpPrefix) null, InetAddress.getByName("192.0.2.4"), + MOBILE_IFNAME)); + cellLp.addLinkAddress(new LinkAddress("2001:db8:1::1/64")); + cellLp.addRoute(new RouteInfo((IpPrefix) null, InetAddress.getByName("2001:db8:1::1"), + MOBILE_IFNAME)); + cellLp.addDnsServer(InetAddress.getByName("2001:db8::1")); + cellLp.addDnsServer(InetAddress.getByName("192.0.2.1")); + + mCellNetworkAgent.sendLinkProperties(cellLp); + mCellNetworkAgent.connect(false); + waitForIdle(); + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( + anyInt(), mStringArrayCaptor.capture(), any(), any(), + eq(""), tlsServers.capture()); + assertEquals(2, mStringArrayCaptor.getValue().length); + assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + // Opportunistic mode. + assertEquals(2, tlsServers.getValue().length); + assertTrue(ArrayUtils.containsAll(tlsServers.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + reset(mNetworkManagementService); + + setPrivateDnsSettings(PRIVATE_DNS_MODE_OFF, "ignored.example.com"); + verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( + anyInt(), mStringArrayCaptor.capture(), any(), any(), + eq(""), eq(EMPTY_STRING_ARRAY)); + assertEquals(2, mStringArrayCaptor.getValue().length); + assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + reset(mNetworkManagementService); + + setPrivateDnsSettings(PRIVATE_DNS_MODE_OPPORTUNISTIC, "ignored.example.com"); + verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( + anyInt(), mStringArrayCaptor.capture(), any(), any(), + eq(""), tlsServers.capture()); + assertEquals(2, mStringArrayCaptor.getValue().length); + assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + assertEquals(2, tlsServers.getValue().length); + assertTrue(ArrayUtils.containsAll(tlsServers.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + reset(mNetworkManagementService); + + // Can't test strict mode without properly mocking out the DNS lookups. + } + private void checkDirectlyConnectedRoutes(Object callbackObj, Collection linkAddresses, Collection otherRoutes) { assertTrue(callbackObj instanceof LinkProperties); From f4bdf74cd620230f99eccc9ce291dfb9d257c1f4 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 6 Apr 2018 17:35:33 +0900 Subject: [PATCH 3/3] Remove unwanted capability code per API council feedback. The addition of hasUnwantedCapability was late in the release cycle and does not simplify the API enough to be worth it. The recommendation is, in a future release, to do something more complete: not just add something like addUnwantedCapability, but also deprecate all the NET_CAPABILITY_NOT_xxx constants and add opposite NET_CAPABILITY_xxx constants for use with it. Fix: 77601789 Test: builds, boots Test: atest android.net.cts.NetworkRequestTest Test: atest android.net.cts.ConnectivityManagerTest Change-Id: Ib98fb01da4a4a0bae464787b589ad88f45002eb8 --- core/java/android/net/NetworkRequest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index 82af5d3c13..6f812ac38e 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -233,6 +233,8 @@ public class NetworkRequest implements Parcelable { * * @param capability The capability to add to unwanted capability list. * @return The builder to facilitate chaining. + * + * @removed */ public Builder addUnwantedCapability(@NetworkCapabilities.NetCapability int capability) { mNetworkCapabilities.addUnwantedCapability(capability); @@ -439,6 +441,8 @@ public class NetworkRequest implements Parcelable { /** * @see Builder#addUnwantedCapability(int) + * + * @removed */ public boolean hasUnwantedCapability(@NetCapability int capability) { return networkCapabilities.hasUnwantedCapability(capability);