diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 94eda01410dae..c003773639de2 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -4711,19 +4711,19 @@ public class ConnectivityManager { /** * Returns the {@code uid} of the owner of a network connection. * - * @param protocol The protocol of the connection. Only {@code IPPROTO_TCP} and - * {@code IPPROTO_UDP} currently supported. + * @param protocol The protocol of the connection. Only {@code IPPROTO_TCP} and {@code + * IPPROTO_UDP} currently supported. * @param local The local {@link InetSocketAddress} of a connection. * @param remote The remote {@link InetSocketAddress} of a connection. - * * @return {@code uid} if the connection is found and the app has permission to observe it - * (e.g., if it is associated with the calling VPN app's tunnel) or - * {@link android.os.Process#INVALID_UID} if the connection is not found. - * Throws {@link SecurityException} if the caller is not the active VPN for the current user. - * Throws {@link IllegalArgumentException} if an unsupported protocol is requested. + * (e.g., if it is associated with the calling VPN app's VpnService tunnel) or {@link + * android.os.Process#INVALID_UID} if the connection is not found. + * @throws {@link SecurityException} if the caller is not the active VpnService for the current + * user. + * @throws {@link IllegalArgumentException} if an unsupported protocol is requested. */ - public int getConnectionOwnerUid(int protocol, @NonNull InetSocketAddress local, - @NonNull InetSocketAddress remote) { + public int getConnectionOwnerUid( + int protocol, @NonNull InetSocketAddress local, @NonNull InetSocketAddress remote) { ConnectionInfo connectionInfo = new ConnectionInfo(protocol, local, remote); try { return mService.getConnectionOwnerUid(connectionInfo); diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 954c36cc938ce..f06d500212889 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -7519,6 +7519,13 @@ public class ConnectivityService extends IConnectivityManager.Stub */ public int getConnectionOwnerUid(ConnectionInfo connectionInfo) { final Vpn vpn = enforceActiveVpnOrNetworkStackPermission(); + + // Only VpnService based VPNs should be able to get this information. + if (vpn != null && vpn.getActiveAppVpnType() != VpnManager.TYPE_VPN_SERVICE) { + throw new SecurityException( + "getConnectionOwnerUid() not allowed for non-VpnService VPNs"); + } + if (connectionInfo.protocol != IPPROTO_TCP && connectionInfo.protocol != IPPROTO_UDP) { throw new IllegalArgumentException("Unsupported protocol " + connectionInfo.protocol); } diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java index 7f6dc55a369d1..8692436f92e4b 100644 --- a/services/core/java/com/android/server/connectivity/Vpn.java +++ b/services/core/java/com/android/server/connectivity/Vpn.java @@ -794,10 +794,10 @@ public class Vpn { // ignore } mContext.unbindService(mConnection); - mConnection = null; + cleanupVpnStateLocked(); } else if (mVpnRunner != null) { + // cleanupVpnStateLocked() is called from mVpnRunner.exit() mVpnRunner.exit(); - mVpnRunner = null; } try { @@ -1104,7 +1104,6 @@ public class Vpn { */ public synchronized ParcelFileDescriptor establish(VpnConfig config) { // Check if the caller is already prepared. - UserManager mgr = UserManager.get(mContext); if (Binder.getCallingUid() != mOwnerUID) { return null; } @@ -1118,10 +1117,7 @@ public class Vpn { long token = Binder.clearCallingIdentity(); try { // Restricted users are not allowed to create VPNs, they are tied to Owner - UserInfo user = mgr.getUserInfo(mUserHandle); - if (user.isRestricted()) { - throw new SecurityException("Restricted users cannot establish VPNs"); - } + enforceNotRestrictedUser(); ResolveInfo info = AppGlobals.getPackageManager().resolveService(intent, null, 0, mUserHandle); @@ -1543,24 +1539,30 @@ public class Vpn { public void interfaceRemoved(String interfaze) { synchronized (Vpn.this) { if (interfaze.equals(mInterface) && jniCheck(interfaze) == 0) { - mStatusIntent = null; - mNetworkCapabilities.setUids(null); - mConfig = null; - mInterface = null; if (mConnection != null) { mContext.unbindService(mConnection); - mConnection = null; - agentDisconnect(); + cleanupVpnStateLocked(); } else if (mVpnRunner != null) { - // agentDisconnect must be called from mVpnRunner.exit() + // cleanupVpnStateLocked() is called from mVpnRunner.exit() mVpnRunner.exit(); - mVpnRunner = null; } } } } }; + private void cleanupVpnStateLocked() { + mStatusIntent = null; + mNetworkCapabilities.setUids(null); + mConfig = null; + mInterface = null; + + // Unconditionally clear both VpnService and VpnRunner fields. + mVpnRunner = null; + mConnection = null; + agentDisconnect(); + } + private void enforceControlPermission() { mContext.enforceCallingPermission(Manifest.permission.CONTROL_VPN, "Unauthorized Caller"); } @@ -1672,6 +1674,25 @@ public class Vpn { return mNetworkCapabilities.appliesToUid(uid); } + /** + * Gets the currently running App-based VPN type + * + * @return the {@link VpnManager.VpnType}. {@link VpnManager.TYPE_VPN_NONE} if not running an + * app-based VPN. While VpnService-based VPNs are always app VPNs and LegacyVpn is always + * Settings-based, the Platform VPNs can be initiated by both apps and Settings. + */ + public synchronized int getActiveAppVpnType() { + if (VpnConfig.LEGACY_VPN.equals(mPackage)) { + return VpnManager.TYPE_VPN_NONE; + } + + if (mVpnRunner != null && mVpnRunner instanceof IkeV2VpnRunner) { + return VpnManager.TYPE_VPN_PLATFORM; + } else { + return VpnManager.TYPE_VPN_SERVICE; + } + } + /** * @param uid The target uid. * @@ -1800,6 +1821,17 @@ public class Vpn { throw new IllegalStateException("Unable to find IPv4 default gateway"); } + private void enforceNotRestrictedUser() { + Binder.withCleanCallingIdentity(() -> { + final UserManager mgr = UserManager.get(mContext); + final UserInfo user = mgr.getUserInfo(mUserHandle); + + if (user.isRestricted()) { + throw new SecurityException("Restricted users cannot configure VPNs"); + } + }); + } + /** * Start legacy VPN, controlling native daemons as needed. Creates a * secondary thread to perform connection work, returning quickly. @@ -2020,7 +2052,25 @@ public class Vpn { public abstract void run(); - protected abstract void exit(); + /** + * Disconnects the NetworkAgent and cleans up all state related to the VpnRunner. + * + *

All outer Vpn instance state is cleaned up in cleanupVpnStateLocked() + */ + protected abstract void exitVpnRunner(); + + /** + * Triggers the cleanup of the VpnRunner, and additionally cleans up Vpn instance-wide state + * + *

This method ensures that simple calls to exit() will always clean up global state + * properly. + */ + protected final void exit() { + synchronized (Vpn.this) { + exitVpnRunner(); + cleanupVpnStateLocked(); + } + } } interface IkeV2VpnRunnerCallback { @@ -2349,17 +2399,6 @@ public class Vpn { } } - /** - * Triggers cleanup of outer class' state - * - *

Can be called from any thread, as it does not mutate state in the Ikev2VpnRunner. - */ - private void cleanupVpnState() { - synchronized (Vpn.this) { - agentDisconnect(); - } - } - /** * Cleans up all Ikev2VpnRunner internal state * @@ -2379,10 +2418,7 @@ public class Vpn { } @Override - public void exit() { - // Cleanup outer class' state immediately, otherwise race conditions may ensue. - cleanupVpnState(); - + public void exitVpnRunner() { mExecutor.execute(() -> { shutdownVpnRunner(); }); @@ -2481,10 +2517,9 @@ public class Vpn { /** Tears down this LegacyVpn connection */ @Override - public void exit() { + public void exitVpnRunner() { // We assume that everything is reset after stopping the daemons. interrupt(); - agentDisconnect(); try { mContext.unregisterReceiver(mBroadcastReceiver); } catch (IllegalArgumentException e) {} @@ -2757,6 +2792,7 @@ public class Vpn { checkNotNull(keyStore, "KeyStore missing"); verifyCallingUidAndPackage(packageName); + enforceNotRestrictedUser(); final byte[] encodedProfile = profile.encode(); if (encodedProfile.length > MAX_VPN_PROFILE_SIZE_BYTES) { @@ -2792,6 +2828,7 @@ public class Vpn { checkNotNull(keyStore, "KeyStore missing"); verifyCallingUidAndPackage(packageName); + enforceNotRestrictedUser(); Binder.withCleanCallingIdentity( () -> { @@ -2834,6 +2871,8 @@ public class Vpn { checkNotNull(packageName, "No package name provided"); checkNotNull(keyStore, "KeyStore missing"); + enforceNotRestrictedUser(); + // Prepare VPN for startup if (!prepare(packageName, null /* newPackage */, VpnManager.TYPE_VPN_PLATFORM)) { throw new SecurityException("User consent not granted for package " + packageName); @@ -2899,6 +2938,8 @@ public class Vpn { public synchronized void stopVpnProfile(@NonNull String packageName) { checkNotNull(packageName, "No package name provided"); + enforceNotRestrictedUser(); + // To stop the VPN profile, the caller must be the current prepared package and must be // running an Ikev2VpnProfile. if (!isCurrentPreparedPackage(packageName) && mVpnRunner instanceof IkeV2VpnRunner) { diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 1a1556f4ae0be..ff5a411b8f95a 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -78,6 +78,7 @@ import static android.net.NetworkPolicyManager.RULE_NONE; import static android.net.NetworkPolicyManager.RULE_REJECT_ALL; import static android.net.NetworkPolicyManager.RULE_REJECT_METERED; import static android.net.RouteInfo.RTN_UNREACHABLE; +import static android.system.OsConstants.IPPROTO_TCP; import static com.android.server.ConnectivityServiceTestUtilsKt.transportToLegacyType; import static com.android.testutils.ConcurrentUtilsKt.await; @@ -137,6 +138,7 @@ import android.content.pm.UserInfo; import android.content.res.Resources; import android.location.LocationManager; import android.net.CaptivePortalData; +import android.net.ConnectionInfo; import android.net.ConnectivityManager; import android.net.ConnectivityManager.NetworkCallback; import android.net.ConnectivityManager.PacketKeepalive; @@ -152,6 +154,7 @@ import android.net.INetworkMonitorCallbacks; import android.net.INetworkPolicyListener; import android.net.INetworkPolicyManager; import android.net.INetworkStatsService; +import android.net.InetAddresses; import android.net.InterfaceConfiguration; import android.net.IpPrefix; import android.net.IpSecManager; @@ -175,6 +178,7 @@ import android.net.RouteInfo; import android.net.SocketKeepalive; import android.net.UidRange; import android.net.Uri; +import android.net.VpnManager; import android.net.metrics.IpConnectivityLog; import android.net.shared.NetworkMonitorUtils; import android.net.shared.PrivateDnsConfig; @@ -271,6 +275,7 @@ import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Predicate; +import java.util.function.Supplier; import kotlin.reflect.KClass; @@ -443,15 +448,21 @@ public class ConnectivityServiceTest { return mPackageManager; } + private int checkMockedPermission(String permission, Supplier ifAbsent) { + final Integer granted = mMockedPermissions.get(permission); + return granted != null ? granted : ifAbsent.get(); + } + @Override public int checkPermission(String permission, int pid, int uid) { - final Integer granted = mMockedPermissions.get(permission); - if (granted == null) { - // All non-mocked permissions should be held by the test or unnecessary: check as - // normal to make sure the code does not rely on unexpected permissions. - return super.checkPermission(permission, pid, uid); - } - return granted; + return checkMockedPermission( + permission, () -> super.checkPermission(permission, pid, uid)); + } + + @Override + public int checkCallingOrSelfPermission(String permission) { + return checkMockedPermission( + permission, () -> super.checkCallingOrSelfPermission(permission)); } @Override @@ -1000,6 +1011,7 @@ public class ConnectivityServiceTest { // Careful ! This is different from mNetworkAgent, because MockNetworkAgent does // not inherit from NetworkAgent. private TestNetworkAgentWrapper mMockNetworkAgent; + private int mVpnType = VpnManager.TYPE_VPN_SERVICE; private VpnInfo mVpnInfo; @@ -1020,6 +1032,10 @@ public class ConnectivityServiceTest { updateCapabilities(null /* defaultNetwork */); } + public void setVpnType(int vpnType) { + mVpnType = vpnType; + } + @Override public int getNetId() { if (mMockNetworkAgent == null) { @@ -1038,6 +1054,11 @@ public class ConnectivityServiceTest { return mConnected; // Similar trickery } + @Override + public int getActiveAppVpnType() { + return mVpnType; + } + private void connect(boolean isAlwaysMetered) { mNetworkCapabilities.set(mMockNetworkAgent.getNetworkCapabilities()); mConnected = true; @@ -6446,6 +6467,90 @@ public class ConnectivityServiceTest { assertEquals(Process.INVALID_UID, newNc.getOwnerUid()); } + private void setupConnectionOwnerUid(int vpnOwnerUid, @VpnManager.VpnType int vpnType) + throws Exception { + final Set vpnRange = Collections.singleton(UidRange.createForUser(VPN_USER)); + establishVpn(new LinkProperties(), vpnOwnerUid, vpnRange); + mMockVpn.setVpnType(vpnType); + + final VpnInfo vpnInfo = new VpnInfo(); + vpnInfo.ownerUid = vpnOwnerUid; + mMockVpn.setVpnInfo(vpnInfo); + } + + private void setupConnectionOwnerUidAsVpnApp(int vpnOwnerUid, @VpnManager.VpnType int vpnType) + throws Exception { + setupConnectionOwnerUid(vpnOwnerUid, vpnType); + + // Test as VPN app + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + mServiceContext.setPermission( + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, PERMISSION_DENIED); + } + + private ConnectionInfo getTestConnectionInfo() throws Exception { + return new ConnectionInfo( + IPPROTO_TCP, + new InetSocketAddress(InetAddresses.parseNumericAddress("1.2.3.4"), 1234), + new InetSocketAddress(InetAddresses.parseNumericAddress("2.3.4.5"), 2345)); + } + + @Test + public void testGetConnectionOwnerUidPlatformVpn() throws Exception { + final int myUid = Process.myUid(); + setupConnectionOwnerUidAsVpnApp(myUid, VpnManager.TYPE_VPN_PLATFORM); + + try { + mService.getConnectionOwnerUid(getTestConnectionInfo()); + fail("Expected SecurityException for non-VpnService app"); + } catch (SecurityException expected) { + } + } + + @Test + public void testGetConnectionOwnerUidVpnServiceWrongUser() throws Exception { + final int myUid = Process.myUid(); + setupConnectionOwnerUidAsVpnApp(myUid + 1, VpnManager.TYPE_VPN_SERVICE); + + try { + mService.getConnectionOwnerUid(getTestConnectionInfo()); + fail("Expected SecurityException for non-VpnService app"); + } catch (SecurityException expected) { + } + } + + @Test + public void testGetConnectionOwnerUidVpnServiceDoesNotThrow() throws Exception { + final int myUid = Process.myUid(); + setupConnectionOwnerUidAsVpnApp(myUid, VpnManager.TYPE_VPN_SERVICE); + + // TODO: Test the returned UID + mService.getConnectionOwnerUid(getTestConnectionInfo()); + } + + @Test + public void testGetConnectionOwnerUidVpnServiceNetworkStackDoesNotThrow() throws Exception { + final int myUid = Process.myUid(); + setupConnectionOwnerUid(myUid, VpnManager.TYPE_VPN_SERVICE); + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); + + // TODO: Test the returned UID + mService.getConnectionOwnerUid(getTestConnectionInfo()); + } + + @Test + public void testGetConnectionOwnerUidVpnServiceMainlineNetworkStackDoesNotThrow() + throws Exception { + final int myUid = Process.myUid(); + setupConnectionOwnerUid(myUid, VpnManager.TYPE_VPN_SERVICE); + mServiceContext.setPermission( + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, PERMISSION_GRANTED); + + // TODO: Test the returned UID + mService.getConnectionOwnerUid(getTestConnectionInfo()); + } + private TestNetworkAgentWrapper establishVpn( LinkProperties lp, int ownerUid, Set vpnRange) throws Exception { final TestNetworkAgentWrapper diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index eb78529e87158..ac1c51837e933 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -656,8 +656,12 @@ public class VpnTest { } private Vpn createVpnAndSetupUidChecks(int... grantedOps) throws Exception { - final Vpn vpn = createVpn(primaryUser.id); - setMockedUsers(primaryUser); + return createVpnAndSetupUidChecks(primaryUser, grantedOps); + } + + private Vpn createVpnAndSetupUidChecks(UserInfo user, int... grantedOps) throws Exception { + final Vpn vpn = createVpn(user.id); + setMockedUsers(user); when(mPackageManager.getPackageUidAsUser(eq(TEST_VPN_PKG), anyInt())) .thenReturn(Process.myUid()); @@ -725,6 +729,19 @@ public class VpnTest { } } + @Test + public void testProvisionVpnProfileRestrictedUser() throws Exception { + final Vpn vpn = + createVpnAndSetupUidChecks( + restrictedProfileA, AppOpsManager.OP_ACTIVATE_PLATFORM_VPN); + + try { + vpn.provisionVpnProfile(TEST_VPN_PKG, mVpnProfile, mKeyStore); + fail("Expected SecurityException due to restricted user"); + } catch (SecurityException expected) { + } + } + @Test public void testDeleteVpnProfile() throws Exception { final Vpn vpn = createVpnAndSetupUidChecks(); @@ -735,6 +752,19 @@ public class VpnTest { .delete(eq(vpn.getProfileNameForPackage(TEST_VPN_PKG)), eq(Process.SYSTEM_UID)); } + @Test + public void testDeleteVpnProfileRestrictedUser() throws Exception { + final Vpn vpn = + createVpnAndSetupUidChecks( + restrictedProfileA, AppOpsManager.OP_ACTIVATE_PLATFORM_VPN); + + try { + vpn.deleteVpnProfile(TEST_VPN_PKG, mKeyStore); + fail("Expected SecurityException due to restricted user"); + } catch (SecurityException expected) { + } + } + @Test public void testGetVpnProfilePrivileged() throws Exception { final Vpn vpn = createVpnAndSetupUidChecks(); @@ -819,6 +849,32 @@ public class VpnTest { eq(TEST_VPN_PKG)); } + @Test + public void testStartVpnProfileRestrictedUser() throws Exception { + final Vpn vpn = + createVpnAndSetupUidChecks( + restrictedProfileA, AppOpsManager.OP_ACTIVATE_PLATFORM_VPN); + + try { + vpn.startVpnProfile(TEST_VPN_PKG, mKeyStore); + fail("Expected SecurityException due to restricted user"); + } catch (SecurityException expected) { + } + } + + @Test + public void testStopVpnProfileRestrictedUser() throws Exception { + final Vpn vpn = + createVpnAndSetupUidChecks( + restrictedProfileA, AppOpsManager.OP_ACTIVATE_PLATFORM_VPN); + + try { + vpn.stopVpnProfile(TEST_VPN_PKG); + fail("Expected SecurityException due to restricted user"); + } catch (SecurityException expected) { + } + } + @Test public void testSetPackageAuthorizationVpnService() throws Exception { final Vpn vpn = createVpnAndSetupUidChecks();