From 28417f4982159d579888d5e2aacf0e101c05fb89 Mon Sep 17 00:00:00 2001 From: Wayne Ma Date: Wed, 17 Apr 2019 08:03:59 -0700 Subject: [PATCH] Backwards-incompatible resolv module API change for making setResolverConfiguration take a parcelable. Test: built, flashed, booted atest FrameworksNetTests Bug: 130788363 Change-Id: I3b4e8672f5273c3baa9378025bfaef2e6514df64 Merged-In: I6dc9029af0df0d3b391210bd315516bdf1b5e4c9 (cherry picked from commit 9e9fda7558a924feb86869fca7dc7fd7dd01a78c) --- .../android/server/connectivity/DnsManager.java | 39 +++++---- .../android/server/ConnectivityServiceTest.java | 93 ++++++++++------------ 2 files changed, 66 insertions(+), 66 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/DnsManager.java b/services/core/java/com/android/server/connectivity/DnsManager.java index 1913635f80e2..e33392d359dd 100644 --- a/services/core/java/com/android/server/connectivity/DnsManager.java +++ b/services/core/java/com/android/server/connectivity/DnsManager.java @@ -34,6 +34,7 @@ import android.net.IDnsResolver; import android.net.LinkProperties; import android.net.Network; import android.net.NetworkUtils; +import android.net.ResolverParamsParcel; import android.net.Uri; import android.net.shared.PrivateDnsConfig; import android.os.Binder; @@ -311,11 +312,9 @@ public class DnsManager { public void setDnsConfigurationForNetwork( int netId, LinkProperties lp, boolean isDefaultNetwork) { - final String[] assignedServers = NetworkUtils.makeStrings(lp.getDnsServers()); - final String[] domainStrs = getDomainStrings(lp.getDomains()); updateParametersSettings(); - final int[] params = { mSampleValidity, mSuccessThreshold, mMinSamples, mMaxSamples }; + final ResolverParamsParcel paramsParcel = new ResolverParamsParcel(); // We only use the PrivateDnsConfig data pushed to this class instance // from ConnectivityService because it works in coordination with @@ -329,34 +328,44 @@ public class DnsManager { final boolean useTls = privateDnsCfg.useTls; final boolean strictMode = privateDnsCfg.inStrictMode(); - final String tlsHostname = strictMode ? privateDnsCfg.hostname : ""; - final String[] tlsServers = + paramsParcel.netId = netId; + paramsParcel.sampleValiditySeconds = mSampleValidity; + paramsParcel.successThreshold = mSuccessThreshold; + paramsParcel.minSamples = mMinSamples; + paramsParcel.maxSamples = mMaxSamples; + paramsParcel.servers = NetworkUtils.makeStrings(lp.getDnsServers()); + paramsParcel.domains = getDomainStrings(lp.getDomains()); + paramsParcel.tlsName = strictMode ? privateDnsCfg.hostname : ""; + paramsParcel.tlsServers = strictMode ? NetworkUtils.makeStrings( Arrays.stream(privateDnsCfg.ips) .filter((ip) -> lp.isReachable(ip)) .collect(Collectors.toList())) - : useTls ? assignedServers // Opportunistic + : useTls ? paramsParcel.servers // Opportunistic : new String[0]; // Off - + paramsParcel.tlsFingerprints = new String[0]; // Prepare to track the validation status of the DNS servers in the // resolver config when private DNS is in opportunistic or strict mode. if (useTls) { if (!mPrivateDnsValidationMap.containsKey(netId)) { mPrivateDnsValidationMap.put(netId, new PrivateDnsValidationStatuses()); } - mPrivateDnsValidationMap.get(netId).updateTrackedDnses(tlsServers, tlsHostname); + mPrivateDnsValidationMap.get(netId).updateTrackedDnses(paramsParcel.tlsServers, + paramsParcel.tlsName); } else { mPrivateDnsValidationMap.remove(netId); } - Slog.d(TAG, String.format("setDnsConfigurationForNetwork(%d, %s, %s, %s, %s, %s)", - netId, Arrays.toString(assignedServers), Arrays.toString(domainStrs), - Arrays.toString(params), tlsHostname, Arrays.toString(tlsServers))); - final String[] tlsFingerprints = new String[0]; + Slog.d(TAG, String.format("setDnsConfigurationForNetwork(%d, %s, %s, %d, %d, %d, %d, " + + "%d, %d, %s, %s)", paramsParcel.netId, Arrays.toString(paramsParcel.servers), + Arrays.toString(paramsParcel.domains), paramsParcel.sampleValiditySeconds, + paramsParcel.successThreshold, paramsParcel.minSamples, + paramsParcel.maxSamples, paramsParcel.baseTimeoutMsec, + paramsParcel.retryCount, paramsParcel.tlsName, + Arrays.toString(paramsParcel.tlsServers))); + try { - mDnsResolver.setResolverConfiguration( - netId, assignedServers, domainStrs, params, - tlsHostname, tlsServers, tlsFingerprints); + mDnsResolver.setResolverConfiguration(paramsParcel); } catch (RemoteException | ServiceSpecificException e) { Slog.e(TAG, "Error setting DNS configuration: " + e); return; diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 44380cce1c77..5f08a347eefd 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -131,6 +131,7 @@ import android.net.NetworkStackClient; import android.net.NetworkState; import android.net.NetworkUtils; import android.net.ProxyInfo; +import android.net.ResolverParamsParcel; import android.net.RouteInfo; import android.net.SocketKeepalive; import android.net.UidRange; @@ -262,7 +263,8 @@ public class ConnectivityServiceTest { @Mock INetd mMockNetd; @Mock NetworkStackClient mNetworkStack; - private ArgumentCaptor mStringArrayCaptor = ArgumentCaptor.forClass(String[].class); + private ArgumentCaptor mResolverParamsParcelCaptor = + ArgumentCaptor.forClass(ResolverParamsParcel.class); // This class exists to test bindProcessToNetwork and getBoundNetworkForProcess. These methods // do not go through ConnectivityService but talk to netd directly, so they don't automatically @@ -4819,16 +4821,13 @@ public class ConnectivityServiceTest { @Test public void testBasicDnsConfigurationPushed() throws Exception { 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(mMockDnsResolver); mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); waitForIdle(); - verify(mMockDnsResolver, never()).setResolverConfiguration( - anyInt(), eq(EMPTY_STRING_ARRAY), any(), any(), eq(""), - eq(EMPTY_STRING_ARRAY), eq(EMPTY_STRING_ARRAY)); + verify(mMockDnsResolver, never()).setResolverConfiguration(any()); verifyNoMoreInteractions(mMockDnsResolver); final LinkProperties cellLp = new LinkProperties(); @@ -4846,35 +4845,33 @@ public class ConnectivityServiceTest { mCellNetworkAgent.connect(false); waitForIdle(); // CS tells netd about the empty DNS config for this network. - verify(mMockDnsResolver, atLeastOnce()).setResolverConfiguration( - anyInt(), eq(EMPTY_STRING_ARRAY), any(), any(), eq(""), - eq(EMPTY_STRING_ARRAY), eq(EMPTY_STRING_ARRAY)); + verify(mMockDnsResolver, atLeastOnce()).setResolverConfiguration(any()); reset(mMockDnsResolver); cellLp.addDnsServer(InetAddress.getByName("2001:db8::1")); mCellNetworkAgent.sendLinkProperties(cellLp); waitForIdle(); verify(mMockDnsResolver, atLeastOnce()).setResolverConfiguration( - anyInt(), mStringArrayCaptor.capture(), any(), any(), - eq(""), tlsServers.capture(), eq(EMPTY_STRING_ARRAY)); - assertEquals(1, mStringArrayCaptor.getValue().length); - assertTrue(ArrayUtils.contains(mStringArrayCaptor.getValue(), "2001:db8::1")); + mResolverParamsParcelCaptor.capture()); + ResolverParamsParcel resolvrParams = mResolverParamsParcelCaptor.getValue(); + assertEquals(1, resolvrParams.servers.length); + assertTrue(ArrayUtils.contains(resolvrParams.servers, "2001:db8::1")); // Opportunistic mode. - assertTrue(ArrayUtils.contains(tlsServers.getValue(), "2001:db8::1")); + assertTrue(ArrayUtils.contains(resolvrParams.tlsServers, "2001:db8::1")); reset(mMockDnsResolver); cellLp.addDnsServer(InetAddress.getByName("192.0.2.1")); mCellNetworkAgent.sendLinkProperties(cellLp); waitForIdle(); verify(mMockDnsResolver, atLeastOnce()).setResolverConfiguration( - anyInt(), mStringArrayCaptor.capture(), any(), any(), - eq(""), tlsServers.capture(), eq(EMPTY_STRING_ARRAY)); - assertEquals(2, mStringArrayCaptor.getValue().length); - assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + mResolverParamsParcelCaptor.capture()); + resolvrParams = mResolverParamsParcelCaptor.getValue(); + assertEquals(2, resolvrParams.servers.length); + assertTrue(ArrayUtils.containsAll(resolvrParams.servers, new String[]{"2001:db8::1", "192.0.2.1"})); // Opportunistic mode. - assertEquals(2, tlsServers.getValue().length); - assertTrue(ArrayUtils.containsAll(tlsServers.getValue(), + assertEquals(2, resolvrParams.tlsServers.length); + assertTrue(ArrayUtils.containsAll(resolvrParams.tlsServers, new String[]{"2001:db8::1", "192.0.2.1"})); reset(mMockDnsResolver); @@ -4887,18 +4884,16 @@ public class ConnectivityServiceTest { waitForIdle(); verify(mMockDnsResolver, atLeastOnce()).setResolverConfiguration( - anyInt(), mStringArrayCaptor.capture(), any(), any(), - eq(TLS_SPECIFIER), eq(TLS_SERVERS), eq(EMPTY_STRING_ARRAY)); - assertEquals(2, mStringArrayCaptor.getValue().length); - assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + mResolverParamsParcelCaptor.capture()); + resolvrParams = mResolverParamsParcelCaptor.getValue(); + assertEquals(2, resolvrParams.servers.length); + assertTrue(ArrayUtils.containsAll(resolvrParams.servers, new String[]{"2001:db8::1", "192.0.2.1"})); reset(mMockDnsResolver); } @Test public void testPrivateDnsSettingsChange() throws Exception { - ArgumentCaptor tlsServers = ArgumentCaptor.forClass(String[].class); - // Clear any interactions that occur as a result of CS starting up. reset(mMockDnsResolver); @@ -4913,9 +4908,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); waitForIdle(); // CS tells netd about the empty DNS config for this network. - verify(mMockDnsResolver, never()).setResolverConfiguration( - anyInt(), eq(EMPTY_STRING_ARRAY), any(), any(), eq(""), - eq(EMPTY_STRING_ARRAY), eq(EMPTY_STRING_ARRAY)); + verify(mMockDnsResolver, never()).setResolverConfiguration(any()); verifyNoMoreInteractions(mMockDnsResolver); final LinkProperties cellLp = new LinkProperties(); @@ -4936,14 +4929,14 @@ public class ConnectivityServiceTest { mCellNetworkAgent.connect(false); waitForIdle(); verify(mMockDnsResolver, atLeastOnce()).setResolverConfiguration( - anyInt(), mStringArrayCaptor.capture(), any(), any(), - eq(""), tlsServers.capture(), eq(EMPTY_STRING_ARRAY)); - assertEquals(2, mStringArrayCaptor.getValue().length); - assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + mResolverParamsParcelCaptor.capture()); + ResolverParamsParcel resolvrParams = mResolverParamsParcelCaptor.getValue(); + assertEquals(2, resolvrParams.tlsServers.length); + assertTrue(ArrayUtils.containsAll(resolvrParams.tlsServers, new String[]{"2001:db8::1", "192.0.2.1"})); // Opportunistic mode. - assertEquals(2, tlsServers.getValue().length); - assertTrue(ArrayUtils.containsAll(tlsServers.getValue(), + assertEquals(2, resolvrParams.tlsServers.length); + assertTrue(ArrayUtils.containsAll(resolvrParams.tlsServers, new String[]{"2001:db8::1", "192.0.2.1"})); reset(mMockDnsResolver); cellNetworkCallback.expectCallback(CallbackState.AVAILABLE, mCellNetworkAgent); @@ -4958,23 +4951,23 @@ public class ConnectivityServiceTest { setPrivateDnsSettings(PRIVATE_DNS_MODE_OFF, "ignored.example.com"); verify(mMockDnsResolver, times(1)).setResolverConfiguration( - anyInt(), mStringArrayCaptor.capture(), any(), any(), - eq(""), eq(EMPTY_STRING_ARRAY), eq(EMPTY_STRING_ARRAY)); - assertEquals(2, mStringArrayCaptor.getValue().length); - assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + mResolverParamsParcelCaptor.capture()); + resolvrParams = mResolverParamsParcelCaptor.getValue(); + assertEquals(2, resolvrParams.servers.length); + assertTrue(ArrayUtils.containsAll(resolvrParams.servers, new String[]{"2001:db8::1", "192.0.2.1"})); reset(mMockDnsResolver); cellNetworkCallback.assertNoCallback(); setPrivateDnsSettings(PRIVATE_DNS_MODE_OPPORTUNISTIC, "ignored.example.com"); verify(mMockDnsResolver, atLeastOnce()).setResolverConfiguration( - anyInt(), mStringArrayCaptor.capture(), any(), any(), - eq(""), tlsServers.capture(), eq(EMPTY_STRING_ARRAY)); - assertEquals(2, mStringArrayCaptor.getValue().length); - assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + mResolverParamsParcelCaptor.capture()); + resolvrParams = mResolverParamsParcelCaptor.getValue(); + assertEquals(2, resolvrParams.servers.length); + assertTrue(ArrayUtils.containsAll(resolvrParams.servers, new String[]{"2001:db8::1", "192.0.2.1"})); - assertEquals(2, tlsServers.getValue().length); - assertTrue(ArrayUtils.containsAll(tlsServers.getValue(), + assertEquals(2, resolvrParams.tlsServers.length); + assertTrue(ArrayUtils.containsAll(resolvrParams.tlsServers, new String[]{"2001:db8::1", "192.0.2.1"})); reset(mMockDnsResolver); cellNetworkCallback.assertNoCallback(); @@ -5826,9 +5819,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent.sendLinkProperties(cellLp); networkCallback.expectCallback(CallbackState.LINK_PROPERTIES, mCellNetworkAgent); verify(mMockDnsResolver, times(1)).stopPrefix64Discovery(cellNetId); - verify(mMockDnsResolver, atLeastOnce()).setResolverConfiguration( - eq(cellNetId), eq(EMPTY_STRING_ARRAY), any(), any(), - eq(""), eq(EMPTY_STRING_ARRAY), eq(EMPTY_STRING_ARRAY)); + verify(mMockDnsResolver, atLeastOnce()).setResolverConfiguration(any()); verifyNoMoreInteractions(mMockNetd); verifyNoMoreInteractions(mMockDnsResolver); @@ -5871,10 +5862,10 @@ public class ConnectivityServiceTest { assertEquals(makeClatLinkProperties(myIpv4), stackedLpsAfterChange.get(0)); verify(mMockDnsResolver, times(1)).setResolverConfiguration( - eq(cellNetId), mStringArrayCaptor.capture(), any(), any(), - eq(""), eq(EMPTY_STRING_ARRAY), eq(EMPTY_STRING_ARRAY)); - assertEquals(1, mStringArrayCaptor.getValue().length); - assertTrue(ArrayUtils.contains(mStringArrayCaptor.getValue(), "8.8.8.8")); + mResolverParamsParcelCaptor.capture()); + ResolverParamsParcel resolvrParams = mResolverParamsParcelCaptor.getValue(); + assertEquals(1, resolvrParams.servers.length); + assertTrue(ArrayUtils.contains(resolvrParams.servers, "8.8.8.8")); // Add ipv4 address, expect that clatd and prefix discovery are stopped and stacked // linkproperties are cleaned up. -- 2.11.0