From df936cf1a706a3a798dbf74e164060f1063fe662 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Tue, 5 Nov 2019 12:56:25 -0800 Subject: [PATCH 1/2] Add basic logic for profile-based VPNs This change adds stubs for the Platform built-in VPNs, along with implementing some basic permissions checks. Bug: 144246837 Test: FrameworksNetTests passing, new tests added Change-Id: I68d2293fc1468544f0d9f64d02ea7e1c80c8d18c --- .../android/net/IConnectivityManager.aidl | 8 + .../android/server/ConnectivityService.java | 81 ++++++- .../android/server/connectivity/VpnTest.java | 206 +++++++++++++++++- 3 files changed, 284 insertions(+), 11 deletions(-) diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index 3e9e7faccb..b050e479d2 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -120,6 +120,14 @@ interface IConnectivityManager ParcelFileDescriptor establishVpn(in VpnConfig config); + boolean provisionVpnProfile(in VpnProfile profile, String packageName); + + void deleteVpnProfile(String packageName); + + void startVpnProfile(String packageName); + + void stopVpnProfile(String packageName); + VpnConfig getVpnConfig(int userId); @UnsupportedAppUsage diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 478b87c010..caf185145f 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4293,7 +4293,7 @@ public class ConnectivityService extends IConnectivityManager.Stub throwIfLockdownEnabled(); Vpn vpn = mVpns.get(userId); if (vpn != null) { - return vpn.prepare(oldPackage, newPackage); + return vpn.prepare(oldPackage, newPackage, false); } else { return false; } @@ -4341,6 +4341,78 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Stores the given VPN profile based on the provisioning package name. + * + *

If there is already a VPN profile stored for the provisioning package, this call will + * overwrite the profile. + * + *

This is designed to serve the VpnManager only; settings-based VPN profiles are managed + * exclusively by the Settings app, and passed into the platform at startup time. + * + * @return {@code true} if user consent has already been granted, {@code false} otherwise. + * @hide + */ + @Override + public boolean provisionVpnProfile(@NonNull VpnProfile profile, @NonNull String packageName) { + final int user = UserHandle.getUserId(Binder.getCallingUid()); + synchronized (mVpns) { + return mVpns.get(user).provisionVpnProfile(packageName, profile, mKeyStore); + } + } + + /** + * Deletes the stored VPN profile for the provisioning package + * + *

If there are no profiles for the given package, this method will silently succeed. + * + *

This is designed to serve the VpnManager only; settings-based VPN profiles are managed + * exclusively by the Settings app, and passed into the platform at startup time. + * + * @hide + */ + @Override + public void deleteVpnProfile(@NonNull String packageName) { + final int user = UserHandle.getUserId(Binder.getCallingUid()); + synchronized (mVpns) { + mVpns.get(user).deleteVpnProfile(packageName, mKeyStore); + } + } + + /** + * Starts the VPN based on the stored profile for the given package + * + *

This is designed to serve the VpnManager only; settings-based VPN profiles are managed + * exclusively by the Settings app, and passed into the platform at startup time. + * + * @throws IllegalArgumentException if no profile was found for the given package name. + * @hide + */ + @Override + public void startVpnProfile(@NonNull String packageName) { + final int user = UserHandle.getUserId(Binder.getCallingUid()); + synchronized (mVpns) { + throwIfLockdownEnabled(); + mVpns.get(user).startVpnProfile(packageName, mKeyStore); + } + } + + /** + * Stops the Platform VPN if the provided package is running one. + * + *

This is designed to serve the VpnManager only; settings-based VPN profiles are managed + * exclusively by the Settings app, and passed into the platform at startup time. + * + * @hide + */ + @Override + public void stopVpnProfile(@NonNull String packageName) { + final int user = UserHandle.getUserId(Binder.getCallingUid()); + synchronized (mVpns) { + mVpns.get(user).stopVpnProfile(packageName); + } + } + /** * Start legacy VPN, controlling native daemons as needed. Creates a * secondary thread to perform connection work, returning quickly. @@ -4544,6 +4616,13 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Throws if there is any currently running, always-on Legacy VPN. + * + *

The LockdownVpnTracker and mLockdownEnabled both track whether an always-on Legacy VPN is + * running across the entire system. Tracking for app-based VPNs is done on a per-user, + * per-package basis in Vpn.java + */ @GuardedBy("mVpns") private void throwIfLockdownEnabled() { if (mLockdownEnabled) { diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index ce50bef53d..084ec73302 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -28,11 +28,11 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_VPN; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; -import static android.net.RouteInfo.RTN_UNREACHABLE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -43,6 +43,7 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -58,21 +59,20 @@ import android.content.pm.ServiceInfo; import android.content.pm.UserInfo; import android.content.res.Resources; import android.net.ConnectivityManager; -import android.net.IpPrefix; -import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkInfo.DetailedState; -import android.net.RouteInfo; import android.net.UidRange; import android.net.VpnService; import android.os.Build.VERSION_CODES; import android.os.Bundle; import android.os.INetworkManagementService; import android.os.Looper; -import android.os.SystemClock; +import android.os.Process; import android.os.UserHandle; import android.os.UserManager; +import android.security.Credentials; +import android.security.KeyStore; import android.util.ArrayMap; import android.util.ArraySet; @@ -81,6 +81,7 @@ import androidx.test.runner.AndroidJUnit4; import com.android.internal.R; import com.android.internal.net.VpnConfig; +import com.android.internal.net.VpnProfile; import org.junit.Before; import org.junit.Test; @@ -90,9 +91,6 @@ import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import java.net.Inet4Address; -import java.net.Inet6Address; -import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -124,6 +122,8 @@ public class VpnTest { managedProfileA.profileGroupId = primaryUser.id; } + static final String TEST_VPN_PKG = "com.dummy.vpn"; + /** * Names and UIDs for some fake packages. Important points: * - UID is ordered increasing. @@ -148,6 +148,8 @@ public class VpnTest { @Mock private NotificationManager mNotificationManager; @Mock private Vpn.SystemServices mSystemServices; @Mock private ConnectivityManager mConnectivityManager; + @Mock private KeyStore mKeyStore; + private final VpnProfile mVpnProfile = new VpnProfile("key"); @Before public void setUp() throws Exception { @@ -166,6 +168,7 @@ public class VpnTest { when(mContext.getString(R.string.config_customVpnAlwaysOnDisconnectedDialogComponent)) .thenReturn(Resources.getSystem().getString( R.string.config_customVpnAlwaysOnDisconnectedDialogComponent)); + when(mSystemServices.isCallerSystem()).thenReturn(true); // Used by {@link Notification.Builder} ApplicationInfo applicationInfo = new ApplicationInfo(); @@ -175,6 +178,10 @@ public class VpnTest { .thenReturn(applicationInfo); doNothing().when(mNetService).registerObserver(any()); + + // Deny all appops by default. + when(mAppOps.noteOpNoThrow(anyInt(), anyInt(), anyString())) + .thenReturn(AppOpsManager.MODE_IGNORED); } @Test @@ -464,12 +471,12 @@ public class VpnTest { order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(entireUser)); // When a new VPN package is set the rules should change to cover that package. - vpn.prepare(null, PKGS[0]); + vpn.prepare(null, PKGS[0], false /* isPlatformVpn */); order.verify(mNetService).setAllowOnlyVpnForUids(eq(false), aryEq(entireUser)); order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(exceptPkg0)); // When that VPN package is unset, everything should be undone again in reverse. - vpn.prepare(null, VpnConfig.LEGACY_VPN); + vpn.prepare(null, VpnConfig.LEGACY_VPN, false /* isPlatformVpn */); order.verify(mNetService).setAllowOnlyVpnForUids(eq(false), aryEq(exceptPkg0)); order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(entireUser)); } @@ -631,6 +638,185 @@ public class VpnTest { assertTrue(caps.hasCapability(NET_CAPABILITY_NOT_CONGESTED)); } + /** + * The profile name should NOT change between releases for backwards compatibility + * + *

If this is changed between releases, the {@link Vpn#getVpnProfilePrivileged()} method MUST + * be updated to ensure backward compatibility. + */ + @Test + public void testGetProfileNameForPackage() throws Exception { + final Vpn vpn = createVpn(primaryUser.id); + setMockedUsers(primaryUser); + + final String expected = Credentials.PLATFORM_VPN + primaryUser.id + "_" + TEST_VPN_PKG; + assertEquals(expected, vpn.getProfileNameForPackage(TEST_VPN_PKG)); + } + + private Vpn createVpnAndSetupUidChecks(int... grantedOps) throws Exception { + final Vpn vpn = createVpn(primaryUser.id); + setMockedUsers(primaryUser); + + when(mPackageManager.getPackageUidAsUser(eq(TEST_VPN_PKG), anyInt())) + .thenReturn(Process.myUid()); + + for (final int op : grantedOps) { + when(mAppOps.noteOpNoThrow(op, Process.myUid(), TEST_VPN_PKG)) + .thenReturn(AppOpsManager.MODE_ALLOWED); + } + + return vpn; + } + + private void checkProvisionVpnProfile(Vpn vpn, boolean expectedResult, int... checkedOps) { + assertEquals(expectedResult, vpn.provisionVpnProfile(TEST_VPN_PKG, mVpnProfile, mKeyStore)); + + // The profile should always be stored, whether or not consent has been previously granted. + verify(mKeyStore) + .put( + eq(vpn.getProfileNameForPackage(TEST_VPN_PKG)), + eq(mVpnProfile.encode()), + eq(Process.SYSTEM_UID), + eq(0)); + + for (final int checkedOp : checkedOps) { + verify(mAppOps).noteOpNoThrow(checkedOp, Process.myUid(), TEST_VPN_PKG); + } + } + + @Test + public void testProvisionVpnProfilePreconsented() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN); + + checkProvisionVpnProfile( + vpn, true /* expectedResult */, AppOpsManager.OP_ACTIVATE_PLATFORM_VPN); + } + + @Test + public void testProvisionVpnProfileNotPreconsented() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(); + + // Expect that both the ACTIVATE_VPN and ACTIVATE_PLATFORM_VPN were tried, but the caller + // had neither. + checkProvisionVpnProfile(vpn, false /* expectedResult */, + AppOpsManager.OP_ACTIVATE_PLATFORM_VPN, AppOpsManager.OP_ACTIVATE_VPN); + } + + @Test + public void testProvisionVpnProfileVpnServicePreconsented() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(AppOpsManager.OP_ACTIVATE_VPN); + + checkProvisionVpnProfile(vpn, true /* expectedResult */, AppOpsManager.OP_ACTIVATE_VPN); + } + + @Test + public void testProvisionVpnProfileTooLarge() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN); + + final VpnProfile bigProfile = new VpnProfile(""); + bigProfile.name = new String(new byte[Vpn.MAX_VPN_PROFILE_SIZE_BYTES + 1]); + + try { + vpn.provisionVpnProfile(TEST_VPN_PKG, bigProfile, mKeyStore); + fail("Expected IAE due to profile size"); + } catch (IllegalArgumentException expected) { + } + } + + @Test + public void testDeleteVpnProfile() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(); + + vpn.deleteVpnProfile(TEST_VPN_PKG, mKeyStore); + + verify(mKeyStore) + .delete(eq(vpn.getProfileNameForPackage(TEST_VPN_PKG)), eq(Process.SYSTEM_UID)); + } + + @Test + public void testGetVpnProfilePrivileged() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(); + + when(mKeyStore.get(vpn.getProfileNameForPackage(TEST_VPN_PKG))) + .thenReturn(new VpnProfile("").encode()); + + vpn.getVpnProfilePrivileged(TEST_VPN_PKG, mKeyStore); + + verify(mKeyStore).get(eq(vpn.getProfileNameForPackage(TEST_VPN_PKG))); + } + + @Test + public void testStartVpnProfile() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN); + + when(mKeyStore.get(vpn.getProfileNameForPackage(TEST_VPN_PKG))) + .thenReturn(mVpnProfile.encode()); + + vpn.startVpnProfile(TEST_VPN_PKG, mKeyStore); + + verify(mKeyStore).get(eq(vpn.getProfileNameForPackage(TEST_VPN_PKG))); + verify(mAppOps) + .noteOpNoThrow( + eq(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN), + eq(Process.myUid()), + eq(TEST_VPN_PKG)); + } + + @Test + public void testStartVpnProfileVpnServicePreconsented() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(AppOpsManager.OP_ACTIVATE_VPN); + + when(mKeyStore.get(vpn.getProfileNameForPackage(TEST_VPN_PKG))) + .thenReturn(mVpnProfile.encode()); + + vpn.startVpnProfile(TEST_VPN_PKG, mKeyStore); + + // Verify that the the ACTIVATE_VPN appop was checked, but no error was thrown. + verify(mAppOps).noteOpNoThrow(AppOpsManager.OP_ACTIVATE_VPN, Process.myUid(), TEST_VPN_PKG); + } + + @Test + public void testStartVpnProfileNotConsented() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(); + + try { + vpn.startVpnProfile(TEST_VPN_PKG, mKeyStore); + fail("Expected failure due to no user consent"); + } catch (SecurityException expected) { + } + + // Verify both appops were checked. + verify(mAppOps) + .noteOpNoThrow( + eq(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN), + eq(Process.myUid()), + eq(TEST_VPN_PKG)); + verify(mAppOps).noteOpNoThrow(AppOpsManager.OP_ACTIVATE_VPN, Process.myUid(), TEST_VPN_PKG); + + // Keystore should never have been accessed. + verify(mKeyStore, never()).get(any()); + } + + @Test + public void testStartVpnProfileMissingProfile() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN); + + when(mKeyStore.get(vpn.getProfileNameForPackage(TEST_VPN_PKG))).thenReturn(null); + + try { + vpn.startVpnProfile(TEST_VPN_PKG, mKeyStore); + fail("Expected failure due to missing profile"); + } catch (IllegalArgumentException expected) { + } + + verify(mKeyStore).get(vpn.getProfileNameForPackage(TEST_VPN_PKG)); + verify(mAppOps) + .noteOpNoThrow( + eq(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN), + eq(Process.myUid()), + eq(TEST_VPN_PKG)); + } + /** * Mock some methods of vpn object. */ From 79ea64f963ad0f7ef8331acadc5cd2f526f74b8c Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 12 Dec 2019 23:38:36 -0800 Subject: [PATCH 2/2] Add VpnManager calls to ConnectivityService This commit adds the relevant calls to ConnectivityService for the VpnManager API to be functional Bug: 144246837 Test: VpnManagerTest updated, FrameworksNetTests passing Change-Id: I446a8595e3583a842a7f89c4f8d74526a85e311c --- .../net/java/android/net/VpnManagerTest.java | 66 +++++++++++++------ 1 file changed, 47 insertions(+), 19 deletions(-) diff --git a/tests/net/java/android/net/VpnManagerTest.java b/tests/net/java/android/net/VpnManagerTest.java index 655c4d1185..97551c94e2 100644 --- a/tests/net/java/android/net/VpnManagerTest.java +++ b/tests/net/java/android/net/VpnManagerTest.java @@ -16,13 +16,21 @@ package android.net; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.test.mock.MockContext; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.internal.net.VpnProfile; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -31,7 +39,12 @@ import org.junit.runner.RunWith; @SmallTest @RunWith(AndroidJUnit4.class) public class VpnManagerTest { - private static final String VPN_PROFILE_KEY = "KEY"; + private static final String PKG_NAME = "fooPackage"; + + private static final String SESSION_NAME_STRING = "testSession"; + private static final String SERVER_ADDR_STRING = "1.2.3.4"; + private static final String IDENTITY_STRING = "Identity"; + private static final byte[] PSK_BYTES = "preSharedKey".getBytes(); private IConnectivityManager mMockCs; private VpnManager mVpnManager; @@ -39,7 +52,7 @@ public class VpnManagerTest { new MockContext() { @Override public String getOpPackageName() { - return "fooPackage"; + return PKG_NAME; } }; @@ -50,34 +63,49 @@ public class VpnManagerTest { } @Test - public void testProvisionVpnProfile() throws Exception { - try { - mVpnManager.provisionVpnProfile(mock(PlatformVpnProfile.class)); - } catch (UnsupportedOperationException expected) { - } + public void testProvisionVpnProfilePreconsented() throws Exception { + final PlatformVpnProfile profile = getPlatformVpnProfile(); + when(mMockCs.provisionVpnProfile(any(VpnProfile.class), eq(PKG_NAME))).thenReturn(true); + + // Expect there to be no intent returned, as consent has already been granted. + assertNull(mVpnManager.provisionVpnProfile(profile)); + verify(mMockCs).provisionVpnProfile(eq(profile.toVpnProfile()), eq(PKG_NAME)); + } + + @Test + public void testProvisionVpnProfileNeedsConsent() throws Exception { + final PlatformVpnProfile profile = getPlatformVpnProfile(); + when(mMockCs.provisionVpnProfile(any(VpnProfile.class), eq(PKG_NAME))).thenReturn(false); + + // Expect intent to be returned, as consent has not already been granted. + assertNotNull(mVpnManager.provisionVpnProfile(profile)); + verify(mMockCs).provisionVpnProfile(eq(profile.toVpnProfile()), eq(PKG_NAME)); } @Test public void testDeleteProvisionedVpnProfile() throws Exception { - try { - mVpnManager.deleteProvisionedVpnProfile(); - } catch (UnsupportedOperationException expected) { - } + mVpnManager.deleteProvisionedVpnProfile(); + verify(mMockCs).deleteVpnProfile(eq(PKG_NAME)); } @Test public void testStartProvisionedVpnProfile() throws Exception { - try { - mVpnManager.startProvisionedVpnProfile(); - } catch (UnsupportedOperationException expected) { - } + mVpnManager.startProvisionedVpnProfile(); + verify(mMockCs).startVpnProfile(eq(PKG_NAME)); } @Test public void testStopProvisionedVpnProfile() throws Exception { - try { - mVpnManager.stopProvisionedVpnProfile(); - } catch (UnsupportedOperationException expected) { - } + mVpnManager.stopProvisionedVpnProfile(); + verify(mMockCs).stopVpnProfile(eq(PKG_NAME)); + } + + private Ikev2VpnProfile getPlatformVpnProfile() throws Exception { + return new Ikev2VpnProfile.Builder(SERVER_ADDR_STRING, IDENTITY_STRING) + .setBypassable(true) + .setMaxMtu(1300) + .setMetered(true) + .setAuthPsk(PSK_BYTES) + .build(); } }