diff options
| author | 2019-03-22 00:28:28 +0900 | |
|---|---|---|
| committer | 2019-05-10 14:30:54 +0900 | |
| commit | 80986d9a538ec48a7dba8cadc0010f8c5b26c9bc (patch) | |
| tree | b25533bcd5ae6883dca7d8ce39626083175b490c | |
| parent | 3423014cbea60f9454c1360891bc6ca7d247548f (diff) | |
Support strict mode private DNS on VPNs that provide Internet.
Currently, strict mode private DNS does not work on VPNs because
NetworkMonitor does not validate VPNs. When a VPN connects, it
immediately transitions to ValidatedState, skipping private DNS
hostname resolution.
This change makes NetworkMonitor perform private DNS hostname
resolution and evaluation even on VPNs.
In order to ensure that the system always immediately switches to
the VPN as soon as it connects, remove the unvalidated penalty
for VPN networks. This ensures that the VPN score is always 101
and the VPN always outscores other networks as soon as it
connects. Previously, it would only outscore other networks
when no-op validation completed.
Bug: 122652057
Test: atest FrameworksNetTests NetworkStackTests
Test: manually ran a VPN with private DNS in strict mode
atest android.net.cts.ConnectivityManagerTest com.android.cts.net.HostsideVpnTests
Change-Id: Iaa78a7edcf23755c89d7b354edbc28d37d74d891
6 files changed, 97 insertions, 17 deletions
diff --git a/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java b/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java index d6355bc111c2..669664dc3fcf 100644 --- a/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java +++ b/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java @@ -517,6 +517,9 @@ public class NetworkMonitor extends StateMachine { return NetworkMonitorUtils.isValidationRequired(mNetworkCapabilities); } + private boolean isPrivateDnsValidationRequired() { + return NetworkMonitorUtils.isPrivateDnsValidationRequired(mNetworkCapabilities); + } private void notifyNetworkTested(int result, @Nullable String redirectUrl) { try { @@ -604,7 +607,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 @@ -840,9 +843,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; } mEvaluateAttempts++; diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 63fd2fda1ce6..1ed025b762b4 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -41,7 +41,7 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED; import static android.net.NetworkCapabilities.TRANSPORT_VPN; import static android.net.NetworkPolicyManager.RULE_NONE; import static android.net.NetworkPolicyManager.uidRulesToString; -import static android.net.shared.NetworkMonitorUtils.isValidationRequired; +import static android.net.shared.NetworkMonitorUtils.isPrivateDnsValidationRequired; import static android.os.Process.INVALID_UID; import static android.system.OsConstants.IPPROTO_TCP; import static android.system.OsConstants.IPPROTO_UDP; @@ -2825,8 +2825,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private boolean networkRequiresValidation(NetworkAgentInfo nai) { - return isValidationRequired(nai.networkCapabilities); + private boolean networkRequiresPrivateDnsValidation(NetworkAgentInfo nai) { + return isPrivateDnsValidationRequired(nai.networkCapabilities); } private void handleFreshlyValidatedNetwork(NetworkAgentInfo nai) { @@ -2844,7 +2844,7 @@ 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)); } } @@ -2853,7 +2853,7 @@ public class ConnectivityService extends IConnectivityManager.Stub 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 NetworkAgentInfo/NetworkMonitor in case NetworkMonitor needs to cancel or // schedule DNS resolutions. If a DNS resolution is required the diff --git a/services/core/java/com/android/server/connectivity/ConnectivityConstants.java b/services/core/java/com/android/server/connectivity/ConnectivityConstants.java index 6fa98b8e8ad7..0fb6fecd4fe2 100644 --- a/services/core/java/com/android/server/connectivity/ConnectivityConstants.java +++ b/services/core/java/com/android/server/connectivity/ConnectivityConstants.java @@ -29,7 +29,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; diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index cfa91314f490..34772d062fd2 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -483,11 +483,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; diff --git a/services/net/java/android/net/shared/NetworkMonitorUtils.java b/services/net/java/android/net/shared/NetworkMonitorUtils.java index bb4a603ba421..46e9c7373f97 100644 --- a/services/net/java/android/net/shared/NetworkMonitorUtils.java +++ b/services/net/java/android/net/shared/NetworkMonitorUtils.java @@ -43,16 +43,23 @@ public class NetworkMonitorUtils { "android.permission.ACCESS_NETWORK_CONDITIONS"; /** - * Return whether validation is required for a network. - * @param dfltNetCap Default requested network capabilities. + * Return whether validation is required for private DNS in strict mode. * @param nc Network capabilities of the network to test. */ - public static boolean isValidationRequired(NetworkCapabilities nc) { + public static boolean isPrivateDnsValidationRequired(NetworkCapabilities nc) { // TODO: Consider requiring validation for DUN networks. return nc != null && nc.hasCapability(NET_CAPABILITY_INTERNET) && nc.hasCapability(NET_CAPABILITY_NOT_RESTRICTED) - && nc.hasCapability(NET_CAPABILITY_TRUSTED) - && nc.hasCapability(NET_CAPABILITY_NOT_VPN); + && nc.hasCapability(NET_CAPABILITY_TRUSTED); + } + + /** + * Return whether validation is required for a network. + * @param nc Network capabilities of the network to test. + */ + public static boolean isValidationRequired(NetworkCapabilities nc) { + // TODO: Consider requiring validation for DUN networks. + return isPrivateDnsValidationRequired(nc) && nc.hasCapability(NET_CAPABILITY_NOT_VPN); } } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 3c5bb6a0c64f..eb6c1d8be903 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -28,6 +28,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.INetworkMonitor.NETWORK_TEST_RESULT_INVALID; import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_PARTIAL_CONNECTIVITY; @@ -489,7 +490,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); @@ -619,6 +620,10 @@ public class ConnectivityServiceTest { mNetworkAgent.sendNetworkScore(mScore); } + public int getScore() { + return mScore; + } + public void explicitlySelected(boolean acceptUnvalidated) { mNetworkAgent.explicitlySelected(acceptUnvalidated); } @@ -1330,6 +1335,8 @@ public class ConnectivityServiceTest { return TYPE_WIFI; case TRANSPORT_CELLULAR: return TYPE_MOBILE; + case TRANSPORT_VPN: + return TYPE_VPN; default: return TYPE_NONE; } @@ -5393,6 +5400,58 @@ public class ConnectivityServiceTest { } @Test + public void testVpnUnvalidated() throws Exception { + final TestNetworkCallback callback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(callback); + + // Bring up Ethernet. + mEthernetNetworkAgent = new MockNetworkAgent(TRANSPORT_ETHERNET); + mEthernetNetworkAgent.connect(true); + callback.expectAvailableThenValidatedCallbacks(mEthernetNetworkAgent); + callback.assertNoCallback(); + + // Bring up a VPN that has the INTERNET capability, initially unvalidated. + final int uid = Process.myUid(); + final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); + 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(NetworkMonitorUtils.isValidationRequired(vpnNetworkAgent.mNetworkCapabilities)); + assertTrue(NetworkMonitorUtils.isPrivateDnsValidationRequired( + vpnNetworkAgent.mNetworkCapabilities)); + + // Pretend that the VPN network validates. + vpnNetworkAgent.setNetworkValid(); + vpnNetworkAgent.mNetworkMonitor.forceReevaluation(Process.myUid()); + // 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(); |