From 00aa2977b41a0e57dd34bebe0b917df35a4a058e Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 29 Jan 2021 20:34:38 +0900 Subject: [PATCH 1/2] Allow passing the underlying network to startLegacyVpn. This will be used by a future change that makes the legacy lockdown VPN pass the underlying network. Bug: 173331190 Test: tests in subsequent CLs in stack Change-Id: I09366a7f872ef3d4538962a75b0114a2ecb536e6 --- .../com/android/server/ConnectivityService.java | 2 +- .../com/android/server/connectivity/Vpn.java | 16 ++++++++++------ .../android/server/net/LockdownVpnTracker.java | 2 +- .../com/android/server/connectivity/VpnTest.java | 3 ++- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 96c3e573a8f5e..d6c7dda02b7f4 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4838,7 +4838,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } synchronized (mVpns) { throwIfLockdownEnabled(); - mVpns.get(user).startLegacyVpn(profile, mKeyStore, egress); + mVpns.get(user).startLegacyVpn(profile, mKeyStore, null /* underlying */, egress); } } diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index b455a3f4169fd..d956ba375ba1e 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -1982,27 +1982,28 @@ public class Vpn { * secondary thread to perform connection work, returning quickly. * * Should only be called to respond to Binder requests as this enforces caller permission. Use - * {@link #startLegacyVpnPrivileged(VpnProfile, KeyStore, LinkProperties)} to skip the + * {@link #startLegacyVpnPrivileged(VpnProfile, KeyStore, Network, LinkProperties)} to skip the * permission check only when the caller is trusted (or the call is initiated by the system). */ - public void startLegacyVpn(VpnProfile profile, KeyStore keyStore, LinkProperties egress) { + public void startLegacyVpn(VpnProfile profile, KeyStore keyStore, @Nullable Network underlying, + LinkProperties egress) { enforceControlPermission(); final long token = Binder.clearCallingIdentity(); try { - startLegacyVpnPrivileged(profile, keyStore, egress); + startLegacyVpnPrivileged(profile, keyStore, underlying, egress); } finally { Binder.restoreCallingIdentity(token); } } /** - * Like {@link #startLegacyVpn(VpnProfile, KeyStore, LinkProperties)}, but does not check - * permissions under the assumption that the caller is the system. + * Like {@link #startLegacyVpn(VpnProfile, KeyStore, Network, LinkProperties)}, but does not + * check permissions under the assumption that the caller is the system. * * Callers are responsible for checking permissions if needed. */ public void startLegacyVpnPrivileged(VpnProfile profile, KeyStore keyStore, - LinkProperties egress) { + @Nullable Network underlying, @NonNull LinkProperties egress) { UserManager mgr = UserManager.get(mContext); UserInfo user = mgr.getUserInfo(mUserId); if (user.isRestricted() || mgr.hasUserRestriction(UserManager.DISALLOW_CONFIG_VPN, @@ -2128,6 +2129,9 @@ public class Vpn { config.session = profile.name; config.isMetered = false; config.proxyInfo = profile.proxy; + if (underlying != null) { + config.underlyingNetworks = new Network[] { underlying }; + } config.addLegacyRoutes(profile.routes); if (!profile.dnsServers.isEmpty()) { diff --git a/services/core/java/com/android/server/net/LockdownVpnTracker.java b/services/core/java/com/android/server/net/LockdownVpnTracker.java index ea2788c0c3d80..6d1c68039ee51 100644 --- a/services/core/java/com/android/server/net/LockdownVpnTracker.java +++ b/services/core/java/com/android/server/net/LockdownVpnTracker.java @@ -155,7 +155,7 @@ public class LockdownVpnTracker { try { // Use the privileged method because Lockdown VPN is initiated by the system, so // no additional permission checks are necessary. - mVpn.startLegacyVpnPrivileged(mProfile, mKeyStore, egressProp); + mVpn.startLegacyVpnPrivileged(mProfile, mKeyStore, null, egressProp); } catch (IllegalStateException e) { mAcceptedEgressIface = null; Log.e(TAG, "Failed to start VPN", e); diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 68aaaeda1b12b..f4782829cff7c 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -148,6 +148,7 @@ public class VpnTest { managedProfileA.profileGroupId = primaryUser.id; } + static final Network EGRESS_NETWORK = new Network(101); static final String EGRESS_IFACE = "wlan0"; static final String TEST_VPN_PKG = "com.testvpn.vpn"; private static final String TEST_VPN_SERVER = "1.2.3.4"; @@ -963,7 +964,7 @@ public class VpnTest { InetAddresses.parseNumericAddress("192.0.2.0"), EGRESS_IFACE); lp.addRoute(defaultRoute); - vpn.startLegacyVpn(vpnProfile, mKeyStore, lp); + vpn.startLegacyVpn(vpnProfile, mKeyStore, EGRESS_NETWORK, lp); return vpn; } From 559ba25636d476a77c679b4b4f9890526e0073e1 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 29 Jan 2021 21:03:01 +0900 Subject: [PATCH 2/2] Allow setting underlying networks when legacy lockdown enabled. Currently, if a legacy lockdown VPN is up, no VPN can set underlying networks. This does not make much sense. When legacy lockdown VPN is enabled, no other VPN is allowed to call prepare() or establish(), so no other VPN can connect, and if no VPN can connect, then no VPN can set underlying networks. Therefore, disabling the ability to set underlying networks only affects the legacy lockdown VPN itself. This change is necessary because in a future CL, the legacy lockdown VPN will start to inform ConnectivityService of its underlying network. Bug: 173331190 Test: tests in subsequent CLs in stack Change-Id: Ifa2aa3351c2c8324571f96fda151864ed987ed5a --- services/core/java/com/android/server/ConnectivityService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d6c7dda02b7f4..f2e192065ed11 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -8068,7 +8068,6 @@ public class ConnectivityService extends IConnectivityManager.Stub int user = UserHandle.getUserId(mDeps.getCallingUid()); final boolean success; synchronized (mVpns) { - throwIfLockdownEnabled(); success = mVpns.get(user).setUnderlyingNetworks(networks); } return success;