Merge changes Iaa78a7ed,I6497b7ef into pi-dev
* changes: Support strict mode private DNS on VPNs that provide Internet. Add test coverage for strict mode private DNS.
This commit is contained in:
committed by
Android (Google) Code Review
commit
6e79d000ab
@@ -2349,9 +2349,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
}
|
||||
}
|
||||
|
||||
private boolean networkRequiresValidation(NetworkAgentInfo nai) {
|
||||
return NetworkMonitor.isValidationRequired(
|
||||
mDefaultRequest.networkCapabilities, nai.networkCapabilities);
|
||||
private boolean networkRequiresPrivateDnsValidation(NetworkAgentInfo nai) {
|
||||
return nai.networkMonitor.isPrivateDnsValidationRequired();
|
||||
}
|
||||
|
||||
private void handleFreshlyValidatedNetwork(NetworkAgentInfo nai) {
|
||||
@@ -2369,16 +2368,14 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
||||
|
||||
for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) {
|
||||
handlePerNetworkPrivateDnsConfig(nai, cfg);
|
||||
if (networkRequiresValidation(nai)) {
|
||||
if (networkRequiresPrivateDnsValidation(nai)) {
|
||||
handleUpdateLinkProperties(nai, new LinkProperties(nai.linkProperties));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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;
|
||||
if (!networkRequiresPrivateDnsValidation(nai)) return;
|
||||
|
||||
// Notify the NetworkMonitor thread in case it needs to cancel or
|
||||
// schedule DNS resolutions. If a DNS resolution is required the
|
||||
|
||||
@@ -45,7 +45,7 @@ public class ConnectivityConstants {
|
||||
//
|
||||
// This ensures that a) the explicitly selected network is never trumped by anything else, and
|
||||
// b) the explicitly selected network is never torn down.
|
||||
public static final int MAXIMUM_NETWORK_SCORE = 100;
|
||||
public static final int EXPLICITLY_SELECTED_NETWORK_SCORE = 100;
|
||||
// VPNs typically have priority over other networks. Give them a score that will
|
||||
// let them win every single time.
|
||||
public static final int VPN_DEFAULT_SCORE = 101;
|
||||
|
||||
@@ -432,11 +432,11 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
|
||||
// down an explicitly selected network before the user gets a chance to prefer it when
|
||||
// a higher-scoring network (e.g., Ethernet) is available.
|
||||
if (networkMisc.explicitlySelected && (networkMisc.acceptUnvalidated || pretendValidated)) {
|
||||
return ConnectivityConstants.MAXIMUM_NETWORK_SCORE;
|
||||
return ConnectivityConstants.EXPLICITLY_SELECTED_NETWORK_SCORE;
|
||||
}
|
||||
|
||||
int score = currentScore;
|
||||
if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty()) {
|
||||
if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) {
|
||||
score -= ConnectivityConstants.UNVALIDATED_SCORE_PENALTY;
|
||||
}
|
||||
if (score < 0) score = 0;
|
||||
|
||||
@@ -243,12 +243,6 @@ public class NetworkMonitor extends StateMachine {
|
||||
|
||||
private String mPrivateDnsProviderHostname = "";
|
||||
|
||||
public static boolean isValidationRequired(
|
||||
NetworkCapabilities dfltNetCap, NetworkCapabilities nc) {
|
||||
// TODO: Consider requiring validation for DUN networks.
|
||||
return dfltNetCap.satisfiedByNetworkCapabilities(nc);
|
||||
}
|
||||
|
||||
private final Context mContext;
|
||||
private final Handler mConnectivityServiceHandler;
|
||||
private final NetworkAgentInfo mNetworkAgentInfo;
|
||||
@@ -381,11 +375,19 @@ public class NetworkMonitor extends StateMachine {
|
||||
return 0 == mValidations ? ValidationStage.FIRST_VALIDATION : ValidationStage.REVALIDATION;
|
||||
}
|
||||
|
||||
private boolean isValidationRequired() {
|
||||
return isValidationRequired(
|
||||
mDefaultRequest.networkCapabilities, mNetworkAgentInfo.networkCapabilities);
|
||||
@VisibleForTesting
|
||||
public boolean isValidationRequired() {
|
||||
// TODO: Consider requiring validation for DUN networks.
|
||||
return mDefaultRequest.networkCapabilities.satisfiedByNetworkCapabilities(
|
||||
mNetworkAgentInfo.networkCapabilities);
|
||||
}
|
||||
|
||||
public boolean isPrivateDnsValidationRequired() {
|
||||
// VPNs become the default network for applications even if they do not provide the INTERNET
|
||||
// capability (e.g., split tunnels; See b/119216095).
|
||||
// Ensure private DNS works on such VPNs as well.
|
||||
return isValidationRequired() || mNetworkAgentInfo.isVPN();
|
||||
}
|
||||
|
||||
private void notifyNetworkTestResultInvalid(Object obj) {
|
||||
mConnectivityServiceHandler.sendMessage(obtainMessage(
|
||||
@@ -455,7 +457,7 @@ public class NetworkMonitor extends StateMachine {
|
||||
return HANDLED;
|
||||
case CMD_PRIVATE_DNS_SETTINGS_CHANGED: {
|
||||
final PrivateDnsConfig cfg = (PrivateDnsConfig) message.obj;
|
||||
if (!isValidationRequired() || cfg == null || !cfg.inStrictMode()) {
|
||||
if (!isPrivateDnsValidationRequired() || cfg == null || !cfg.inStrictMode()) {
|
||||
// No DNS resolution required.
|
||||
//
|
||||
// We don't force any validation in opportunistic mode
|
||||
@@ -621,9 +623,20 @@ public class NetworkMonitor extends StateMachine {
|
||||
// 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.
|
||||
//
|
||||
// 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()) {
|
||||
validationLog("Network would not satisfy default request, not validating");
|
||||
transitionTo(mValidatedState);
|
||||
if (isPrivateDnsValidationRequired()) {
|
||||
validationLog("Network would not satisfy default request, "
|
||||
+ "resolving private DNS");
|
||||
transitionTo(mEvaluatingPrivateDnsState);
|
||||
} else {
|
||||
validationLog("Network would not satisfy default request, "
|
||||
+ "not validating");
|
||||
transitionTo(mValidatedState);
|
||||
}
|
||||
return HANDLED;
|
||||
}
|
||||
mAttempts++;
|
||||
@@ -796,7 +809,7 @@ public class NetworkMonitor extends StateMachine {
|
||||
try {
|
||||
// Do a blocking DNS resolution using the network-assigned nameservers.
|
||||
// Do not set AI_ADDRCONFIG in ai_flags so we get all address families in advance.
|
||||
final InetAddress[] ips = ResolvUtil.blockingResolveAllLocally(
|
||||
final InetAddress[] ips = resolveAllLocally(
|
||||
mNetwork, mPrivateDnsProviderHostname, 0 /* aiFlags */);
|
||||
mPrivateDnsConfig = new PrivateDnsConfig(mPrivateDnsProviderHostname, ips);
|
||||
} catch (UnknownHostException uhe) {
|
||||
@@ -830,7 +843,7 @@ public class NetworkMonitor extends StateMachine {
|
||||
final String host = UUID.randomUUID().toString().substring(0, 8) +
|
||||
ONE_TIME_HOSTNAME_SUFFIX;
|
||||
try {
|
||||
final InetAddress[] ips = mNetworkAgentInfo.network().getAllByName(host);
|
||||
final InetAddress[] ips = getAllByName(mNetworkAgentInfo.network(), host);
|
||||
return (ips != null && ips.length > 0);
|
||||
} catch (UnknownHostException uhe) {}
|
||||
return false;
|
||||
@@ -1046,7 +1059,7 @@ public class NetworkMonitor extends StateMachine {
|
||||
int result;
|
||||
String connectInfo;
|
||||
try {
|
||||
InetAddress[] addresses = mNetwork.getAllByName(host);
|
||||
InetAddress[] addresses = getAllByName(mNetwork, host);
|
||||
StringBuffer buffer = new StringBuffer();
|
||||
for (InetAddress address : addresses) {
|
||||
buffer.append(',').append(address.getHostAddress());
|
||||
@@ -1233,6 +1246,18 @@ public class NetworkMonitor extends StateMachine {
|
||||
}
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
protected InetAddress[] getAllByName(Network network, String host) throws UnknownHostException {
|
||||
return network.getAllByName(host);
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
protected InetAddress[] resolveAllLocally(Network network, String hostname, int flags)
|
||||
throws UnknownHostException {
|
||||
// We cannot use this in OneAddressPerFamilyNetwork#getAllByName because that's static.
|
||||
return ResolvUtil.blockingResolveAllLocally(network, hostname, flags);
|
||||
}
|
||||
|
||||
private URL makeURL(String url) {
|
||||
if (url != null) {
|
||||
try {
|
||||
|
||||
@@ -26,6 +26,7 @@ import static android.net.ConnectivityManager.TYPE_MOBILE;
|
||||
import static android.net.ConnectivityManager.TYPE_MOBILE_FOTA;
|
||||
import static android.net.ConnectivityManager.TYPE_MOBILE_MMS;
|
||||
import static android.net.ConnectivityManager.TYPE_NONE;
|
||||
import static android.net.ConnectivityManager.TYPE_VPN;
|
||||
import static android.net.ConnectivityManager.TYPE_WIFI;
|
||||
import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL;
|
||||
import static android.net.NetworkCapabilities.NET_CAPABILITY_CBS;
|
||||
@@ -165,6 +166,7 @@ import org.mockito.MockitoAnnotations;
|
||||
import org.mockito.Spy;
|
||||
|
||||
import java.net.InetAddress;
|
||||
import java.net.UnknownHostException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collection;
|
||||
@@ -387,7 +389,7 @@ public class ConnectivityServiceTest {
|
||||
|
||||
MockNetworkAgent(int transport, LinkProperties linkProperties) {
|
||||
final int type = transportToLegacyType(transport);
|
||||
final String typeName = ConnectivityManager.getNetworkTypeName(transport);
|
||||
final String typeName = ConnectivityManager.getNetworkTypeName(type);
|
||||
mNetworkInfo = new NetworkInfo(type, 0, typeName, "Mock");
|
||||
mNetworkCapabilities = new NetworkCapabilities();
|
||||
mNetworkCapabilities.addTransportType(transport);
|
||||
@@ -455,6 +457,10 @@ public class ConnectivityServiceTest {
|
||||
mNetworkAgent.sendNetworkScore(mScore);
|
||||
}
|
||||
|
||||
public int getScore() {
|
||||
return mScore;
|
||||
}
|
||||
|
||||
public void explicitlySelected(boolean acceptUnvalidated) {
|
||||
mNetworkAgent.explicitlySelected(acceptUnvalidated);
|
||||
}
|
||||
@@ -869,6 +875,7 @@ public class ConnectivityServiceTest {
|
||||
// HTTP response code fed back to NetworkMonitor for Internet connectivity probe.
|
||||
public int gen204ProbeResult = 500;
|
||||
public String gen204ProbeRedirectUrl = null;
|
||||
public volatile InetAddress[] dnsLookupResults = null;
|
||||
|
||||
public WrappedNetworkMonitor(Context context, Handler handler,
|
||||
NetworkAgentInfo networkAgentInfo, NetworkRequest defaultRequest,
|
||||
@@ -883,6 +890,25 @@ public class ConnectivityServiceTest {
|
||||
if (!mIsCaptivePortalCheckEnabled) { return new CaptivePortalProbeResult(204); }
|
||||
return new CaptivePortalProbeResult(gen204ProbeResult, gen204ProbeRedirectUrl, null);
|
||||
}
|
||||
|
||||
private InetAddress[] fakeDnsLookup() throws UnknownHostException {
|
||||
if (dnsLookupResults == null) {
|
||||
throw new UnknownHostException();
|
||||
}
|
||||
return dnsLookupResults;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected InetAddress[] getAllByName(Network network, String hostname)
|
||||
throws UnknownHostException {
|
||||
return fakeDnsLookup();
|
||||
}
|
||||
|
||||
@Override
|
||||
protected InetAddress[] resolveAllLocally(Network network, String hostname, int flags)
|
||||
throws UnknownHostException {
|
||||
return fakeDnsLookup();
|
||||
}
|
||||
}
|
||||
|
||||
private class WrappedMultinetworkPolicyTracker extends MultinetworkPolicyTracker {
|
||||
@@ -1086,6 +1112,8 @@ public class ConnectivityServiceTest {
|
||||
return TYPE_WIFI;
|
||||
case TRANSPORT_CELLULAR:
|
||||
return TYPE_MOBILE;
|
||||
case TRANSPORT_VPN:
|
||||
return TYPE_VPN;
|
||||
default:
|
||||
return TYPE_NONE;
|
||||
}
|
||||
@@ -4021,7 +4049,7 @@ public class ConnectivityServiceTest {
|
||||
cellLp.addDnsServer(InetAddress.getByName("192.0.2.1"));
|
||||
|
||||
mCellNetworkAgent.sendLinkProperties(cellLp);
|
||||
mCellNetworkAgent.connect(false);
|
||||
mCellNetworkAgent.connect(true);
|
||||
waitForIdle();
|
||||
verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork(
|
||||
anyInt(), mStringArrayCaptor.capture(), any(), any(),
|
||||
@@ -4039,9 +4067,10 @@ public class ConnectivityServiceTest {
|
||||
mCellNetworkAgent);
|
||||
CallbackInfo cbi = cellNetworkCallback.expectCallback(
|
||||
CallbackState.LINK_PROPERTIES, mCellNetworkAgent);
|
||||
cellNetworkCallback.assertNoCallback();
|
||||
assertFalse(((LinkProperties)cbi.arg).isPrivateDnsActive());
|
||||
assertNull(((LinkProperties)cbi.arg).getPrivateDnsServerName());
|
||||
cellNetworkCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, mCellNetworkAgent);
|
||||
cellNetworkCallback.assertNoCallback();
|
||||
|
||||
setPrivateDnsSettings(PRIVATE_DNS_MODE_OFF, "ignored.example.com");
|
||||
verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork(
|
||||
@@ -4066,14 +4095,45 @@ public class ConnectivityServiceTest {
|
||||
reset(mNetworkManagementService);
|
||||
cellNetworkCallback.assertNoCallback();
|
||||
|
||||
// Strict mode.
|
||||
mCellNetworkAgent.getWrappedNetworkMonitor().dnsLookupResults = new InetAddress[] {
|
||||
InetAddress.getByName("2001:db8::66"),
|
||||
InetAddress.getByName("192.0.2.44")
|
||||
};
|
||||
setPrivateDnsSettings(PRIVATE_DNS_MODE_PROVIDER_HOSTNAME, "strict.example.com");
|
||||
// Can't test dns configuration for strict mode without properly mocking
|
||||
// out the DNS lookups, but can test that LinkProperties is updated.
|
||||
|
||||
// Expect a callback saying that private DNS is now in strict mode.
|
||||
cbi = cellNetworkCallback.expectCallback(CallbackState.LINK_PROPERTIES,
|
||||
mCellNetworkAgent);
|
||||
LinkProperties lp = (LinkProperties) cbi.arg;
|
||||
assertTrue(lp.isPrivateDnsActive());
|
||||
assertEquals("strict.example.com", lp.getPrivateDnsServerName());
|
||||
cellNetworkCallback.assertNoCallback();
|
||||
assertTrue(((LinkProperties)cbi.arg).isPrivateDnsActive());
|
||||
assertEquals("strict.example.com", ((LinkProperties)cbi.arg).getPrivateDnsServerName());
|
||||
|
||||
// When the validation callback arrives, LinkProperties are updated.
|
||||
// We need to wait for this callback because the test thread races with the NetworkMonitor
|
||||
// thread, and if the test thread wins the race, then the times(2) verify call below will
|
||||
// fail.
|
||||
mService.mNetdEventCallback.onPrivateDnsValidationEvent(
|
||||
mCellNetworkAgent.getNetwork().netId, "2001:db8::66", "strict.example.com", true);
|
||||
cbi = cellNetworkCallback.expectCallback(CallbackState.LINK_PROPERTIES,
|
||||
mCellNetworkAgent);
|
||||
lp = (LinkProperties) cbi.arg;
|
||||
assertTrue(lp.isPrivateDnsActive());
|
||||
assertEquals(1, lp.getValidatedPrivateDnsServers().size());
|
||||
|
||||
// setDnsConfigurationForNetwork is called twice: once when private DNS is set to strict
|
||||
// mode and once when the hostname resolves.
|
||||
verify(mNetworkManagementService, times(2)).setDnsConfigurationForNetwork(
|
||||
anyInt(), mStringArrayCaptor.capture(), any(), any(),
|
||||
eq("strict.example.com"), 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::66", "192.0.2.44"}));
|
||||
reset(mNetworkManagementService);
|
||||
|
||||
// Send the same LinkProperties and expect getting the same result including private dns.
|
||||
// b/118518971
|
||||
@@ -4395,6 +4455,69 @@ public class ConnectivityServiceTest {
|
||||
mCm.unregisterNetworkCallback(defaultCallback);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testVpnUnvalidated() throws Exception {
|
||||
final TestNetworkCallback callback = new TestNetworkCallback();
|
||||
mCm.registerDefaultNetworkCallback(callback);
|
||||
|
||||
// Enable private DNS.
|
||||
setPrivateDnsSettings(PRIVATE_DNS_MODE_PROVIDER_HOSTNAME, "strict.example.com");
|
||||
|
||||
// Bring up Ethernet.
|
||||
mEthernetNetworkAgent = new MockNetworkAgent(TRANSPORT_ETHERNET);
|
||||
mEthernetNetworkAgent.getWrappedNetworkMonitor().dnsLookupResults =
|
||||
new InetAddress[]{ InetAddress.getByName("2001:db8::1") };
|
||||
mEthernetNetworkAgent.connect(true);
|
||||
callback.expectAvailableThenValidatedCallbacks(mEthernetNetworkAgent);
|
||||
callback.assertNoCallback();
|
||||
|
||||
// Bring up a VPN that has the INTERNET capability but does not validate.
|
||||
final int uid = Process.myUid();
|
||||
final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);
|
||||
vpnNetworkAgent.getWrappedNetworkMonitor().gen204ProbeResult = 500;
|
||||
vpnNetworkAgent.getWrappedNetworkMonitor().dnsLookupResults = null;
|
||||
|
||||
final ArraySet<UidRange> ranges = new ArraySet<>();
|
||||
ranges.add(new UidRange(uid, uid));
|
||||
mMockVpn.setNetworkAgent(vpnNetworkAgent);
|
||||
mMockVpn.setUids(ranges);
|
||||
vpnNetworkAgent.connect(false /* validated */, true /* hasInternet */);
|
||||
mMockVpn.connect();
|
||||
|
||||
// Even though the VPN is unvalidated, it becomes the default network for our app.
|
||||
callback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent);
|
||||
// TODO: this looks like a spurious callback.
|
||||
callback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent);
|
||||
callback.assertNoCallback();
|
||||
|
||||
assertTrue(vpnNetworkAgent.getScore() > mEthernetNetworkAgent.getScore());
|
||||
assertEquals(ConnectivityConstants.VPN_DEFAULT_SCORE, vpnNetworkAgent.getScore());
|
||||
assertEquals(vpnNetworkAgent.getNetwork(), mCm.getActiveNetwork());
|
||||
|
||||
NetworkCapabilities nc = mCm.getNetworkCapabilities(vpnNetworkAgent.getNetwork());
|
||||
assertFalse(nc.hasCapability(NET_CAPABILITY_VALIDATED));
|
||||
assertTrue(nc.hasCapability(NET_CAPABILITY_INTERNET));
|
||||
|
||||
assertFalse(vpnNetworkAgent.getWrappedNetworkMonitor().isValidationRequired());
|
||||
assertTrue(vpnNetworkAgent.getWrappedNetworkMonitor().isPrivateDnsValidationRequired());
|
||||
|
||||
// Pretend that the strict mode private DNS hostname now resolves. Even though the
|
||||
// connectivity probe still returns 500, the network validates because the connectivity
|
||||
// probe is not used on VPNs.
|
||||
vpnNetworkAgent.getWrappedNetworkMonitor().dnsLookupResults =
|
||||
new InetAddress[]{ InetAddress.getByName("2001:db8::1") };
|
||||
mCm.reportNetworkConnectivity(vpnNetworkAgent.getNetwork(), true);
|
||||
|
||||
// Expect to see the validated capability, but no other changes, because the VPN is already
|
||||
// the default network for the app.
|
||||
callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, vpnNetworkAgent);
|
||||
callback.assertNoCallback();
|
||||
|
||||
vpnNetworkAgent.disconnect();
|
||||
callback.expectCallback(CallbackState.LOST, vpnNetworkAgent);
|
||||
callback.expectAvailableCallbacksValidated(mEthernetNetworkAgent);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testVpnSetUnderlyingNetworks() {
|
||||
final int uid = Process.myUid();
|
||||
|
||||
Reference in New Issue
Block a user