From 72fe97ea1c340828ea4476ad86a37ccca77080c4 Mon Sep 17 00:00:00 2001 From: chiachangwang Date: Wed, 22 Jun 2022 03:24:46 +0000 Subject: [PATCH 1/4] Add tests to verify getProvisionedVpnProfileState Add tests to verify that calling getProvisionedVpnProfileState works normally from the same package but gets a security exception from a differnet package. Bug: 230548427 Test: atest FrameworksNetTests Change-Id: I0132c42dce1b8269859fca8d0143304142162838 --- .../android/server/VpnManagerServiceTest.java | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/unit/java/com/android/server/VpnManagerServiceTest.java b/tests/unit/java/com/android/server/VpnManagerServiceTest.java index ece13b3361..eb9a752225 100644 --- a/tests/unit/java/com/android/server/VpnManagerServiceTest.java +++ b/tests/unit/java/com/android/server/VpnManagerServiceTest.java @@ -121,14 +121,14 @@ public class VpnManagerServiceTest extends VpnTestBase { eq(mHandler) /* scheduler */); doReturn(SYSTEM_USER).when(mUserManager).getUserInfo(eq(SYSTEM_USER_ID)); mService = new VpnManagerService(mContext, mDeps); + + // Add user to create vpn in mVpn + mService.onUserStarted(SYSTEM_USER_ID); + assertNotNull(mService.mVpns.get(SYSTEM_USER_ID)); } @Test public void testUpdateAppExclusionList() { - // Add user to create vpn in mVpn - mService.onUserStarted(SYSTEM_USER_ID); - assertNotNull(mService.mVpns.get(SYSTEM_USER_ID)); - // Start vpn mService.startVpnProfile(TEST_VPN_PKG); verify(mVpn).startVpnProfile(eq(TEST_VPN_PKG)); @@ -160,4 +160,16 @@ public class VpnManagerServiceTest extends VpnTestBase { public void testStopVpnProfileFromDiffPackage() { assertThrows(SecurityException.class, () -> mService.stopVpnProfile(mNotMyVpnPkg)); } + + @Test + public void testGetProvisionedVpnProfileStateFromDiffPackage() { + assertThrows(SecurityException.class, () -> + mService.getProvisionedVpnProfileState(mNotMyVpnPkg)); + } + + @Test + public void testGetProvisionedVpnProfileState() { + mService.getProvisionedVpnProfileState(TEST_VPN_PKG); + verify(mVpn).getProvisionedVpnProfileState(TEST_VPN_PKG); + } } From 9460f45c047e49aff7e83ea3496476dd21752e0e Mon Sep 17 00:00:00 2001 From: chiachangwang Date: Thu, 30 Jun 2022 01:38:26 +0000 Subject: [PATCH 2/4] Update the prefix of VPN_APP_EXCLUDED in the test Update the string according to the change in Vpn.java Bug: 237345836 Test: atest FrameworksNetTests Change-Id: I9bfe0a1fad29ba1435eeb8b8b2cffa9d385b1cb5 --- tests/unit/java/com/android/server/connectivity/VpnTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/java/com/android/server/connectivity/VpnTest.java b/tests/unit/java/com/android/server/connectivity/VpnTest.java index 0891ee36d9..b93bac0111 100644 --- a/tests/unit/java/com/android/server/connectivity/VpnTest.java +++ b/tests/unit/java/com/android/server/connectivity/VpnTest.java @@ -232,7 +232,7 @@ public class VpnTest extends VpnTestBase { private static final int TEST_TUNNEL_RESOURCE_ID = 0x2345; private static final long TEST_TIMEOUT_MS = 500L; private static final String PRIMARY_USER_APP_EXCLUDE_KEY = - "VPN_APP_EXCLUDED_27_com.testvpn.vpn"; + "VPNAPPEXCLUDED_27_com.testvpn.vpn"; static final String PKGS_BYTES = getPackageByteString(List.of(PKGS)); private static final Range PRIMARY_USER_RANGE = uidRangeForUser(PRIMARY_USER.id); From a412026c6bc42219671c9dd27f0be2ccd971ed7a Mon Sep 17 00:00:00 2001 From: chiachangwang Date: Wed, 29 Jun 2022 02:46:45 +0000 Subject: [PATCH 3/4] Test reconnect VpnManager VPN with always on enabled Verify reconnect using startVpnProfile with the same package with always-on package should work without exception. Bug: 235322391 Test: atest FrameworksNetTests Change-Id: I9a780d970208032e2468e9c5b3334373627c6a20 --- .../android/server/connectivity/VpnTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/unit/java/com/android/server/connectivity/VpnTest.java b/tests/unit/java/com/android/server/connectivity/VpnTest.java index b93bac0111..5c1992d9d0 100644 --- a/tests/unit/java/com/android/server/connectivity/VpnTest.java +++ b/tests/unit/java/com/android/server/connectivity/VpnTest.java @@ -1359,6 +1359,31 @@ public class VpnTest extends VpnTestBase { null /* sessionKey */, true /* alwaysOn */, false /* lockdown */)); } + @Test + public void testReconnectVpnManagerVpnWithAlwaysOnEnabled() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(AppOpsManager.OPSTR_ACTIVATE_PLATFORM_VPN); + when(mVpnProfileStore.get(vpn.getProfileNameForPackage(TEST_VPN_PKG))) + .thenReturn(mVpnProfile.encode()); + vpn.startVpnProfile(TEST_VPN_PKG); + verifyPlatformVpnIsActivated(TEST_VPN_PKG); + + // Enable VPN always-on for TEST_VPN_PKG. + assertTrue(vpn.setAlwaysOnPackage(TEST_VPN_PKG, false /* lockdown */, + null /* lockdownAllowlist */)); + + // Reset to verify next startVpnProfile. + reset(mAppOps); + + vpn.stopVpnProfile(TEST_VPN_PKG); + + // Reconnect the vpn with different package will cause exception. + assertThrows(SecurityException.class, () -> vpn.startVpnProfile(PKGS[0])); + + // Reconnect the vpn again with the vpn always on package w/o exception. + vpn.startVpnProfile(TEST_VPN_PKG); + verifyPlatformVpnIsActivated(TEST_VPN_PKG); + } + @Test public void testSetPackageAuthorizationVpnService() throws Exception { final Vpn vpn = createVpnAndSetupUidChecks(); From 26de58ea790041b5c1c2d38471622c7376ee648d Mon Sep 17 00:00:00 2001 From: chiachangwang Date: Tue, 5 Jul 2022 08:42:43 +0000 Subject: [PATCH 4/4] Refactor to mock package add and remove event Use mock intent receiver to handle intent related events instead of exposing the private methods even for testing only. The visibility for onPackageAdded, onPackageRemoved and onUserStarted were exposed for testing only in former commit. The test coverage for intent receiver part may be missed. The other intent related methods will be verified in the follow up commits. This refactor will also help to not to expose further private methods. Bug: 230548427 Test: atest VpnManagerServiceTest Change-Id: If2aa88d8d5c1643e8bd4937673aa792186047fd8 --- .../android/server/VpnManagerServiceTest.java | 93 +++++++++++++++---- 1 file changed, 76 insertions(+), 17 deletions(-) diff --git a/tests/unit/java/com/android/server/VpnManagerServiceTest.java b/tests/unit/java/com/android/server/VpnManagerServiceTest.java index eb9a752225..164f8251d7 100644 --- a/tests/unit/java/com/android/server/VpnManagerServiceTest.java +++ b/tests/unit/java/com/android/server/VpnManagerServiceTest.java @@ -24,7 +24,6 @@ import static com.android.testutils.MiscAsserts.assertThrows; import static org.junit.Assert.assertNotNull; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; @@ -32,15 +31,18 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import android.annotation.UserIdInt; +import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; import android.net.ConnectivityManager; import android.net.INetd; +import android.net.Uri; import android.os.Handler; import android.os.HandlerThread; import android.os.INetworkManagementService; import android.os.Looper; +import android.os.UserHandle; import android.os.UserManager; import androidx.test.filters.SmallTest; @@ -48,14 +50,15 @@ import androidx.test.filters.SmallTest; import com.android.server.connectivity.Vpn; import com.android.testutils.DevSdkIgnoreRule; import com.android.testutils.DevSdkIgnoreRunner; +import com.android.testutils.HandlerUtils; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import org.mockito.Spy; @RunWith(DevSdkIgnoreRunner.class) @IgnoreUpTo(R) // VpnManagerService is not available before R @@ -64,18 +67,23 @@ public class VpnManagerServiceTest extends VpnTestBase { @Rule public final DevSdkIgnoreRule mIgnoreRule = new DevSdkIgnoreRule(); - @Spy Context mContext; + private static final int TIMEOUT_MS = 2_000; + + @Mock Context mContext; + @Mock Context mSystemContext; + @Mock Context mUserAllContext; private HandlerThread mHandlerThread; - @Mock private Handler mHandler; @Mock private Vpn mVpn; @Mock private INetworkManagementService mNms; @Mock private ConnectivityManager mCm; @Mock private UserManager mUserManager; @Mock private INetd mNetd; @Mock private PackageManager mPackageManager; + private VpnManagerServiceDependencies mDeps; private VpnManagerService mService; - + private BroadcastReceiver mUserPresentReceiver; + private BroadcastReceiver mIntentReceiver; private final String mNotMyVpnPkg = "com.not.my.vpn"; class VpnManagerServiceDependencies extends VpnManagerService.Dependencies { @@ -107,23 +115,31 @@ public class VpnManagerServiceTest extends VpnTestBase { mHandlerThread = new HandlerThread("TestVpnManagerService"); mDeps = new VpnManagerServiceDependencies(); - doReturn(mContext).when(mContext).createContextAsUser(any(), anyInt()); + doReturn(mUserAllContext).when(mContext).createContextAsUser(UserHandle.ALL, 0); + doReturn(mSystemContext).when(mContext).createContextAsUser(UserHandle.SYSTEM, 0); doReturn(mPackageManager).when(mContext).getPackageManager(); setMockedPackages(mPackageManager, sPackages); mockService(mContext, ConnectivityManager.class, Context.CONNECTIVITY_SERVICE, mCm); mockService(mContext, UserManager.class, Context.USER_SERVICE, mUserManager); - - doReturn(new Intent()).when(mContext).registerReceiver( - any() /* receiver */, - any() /* intentFilter */, - any() /* broadcastPermission */, - eq(mHandler) /* scheduler */); doReturn(SYSTEM_USER).when(mUserManager).getUserInfo(eq(SYSTEM_USER_ID)); + mService = new VpnManagerService(mContext, mDeps); + mService.systemReady(); + + final ArgumentCaptor intentReceiverCaptor = + ArgumentCaptor.forClass(BroadcastReceiver.class); + final ArgumentCaptor userPresentReceiverCaptor = + ArgumentCaptor.forClass(BroadcastReceiver.class); + verify(mSystemContext).registerReceiver( + userPresentReceiverCaptor.capture(), any(), any(), any()); + verify(mUserAllContext, times(2)).registerReceiver( + intentReceiverCaptor.capture(), any(), any(), any()); + mUserPresentReceiver = userPresentReceiverCaptor.getValue(); + mIntentReceiver = intentReceiverCaptor.getValue(); // Add user to create vpn in mVpn - mService.onUserStarted(SYSTEM_USER_ID); + onUserStarted(SYSTEM_USER_ID); assertNotNull(mService.mVpns.get(SYSTEM_USER_ID)); } @@ -134,19 +150,19 @@ public class VpnManagerServiceTest extends VpnTestBase { verify(mVpn).startVpnProfile(eq(TEST_VPN_PKG)); // Remove package due to package replaced. - mService.onPackageRemoved(PKGS[0], PKG_UIDS[0], true /* isReplacing */); + onPackageRemoved(PKGS[0], PKG_UIDS[0], true /* isReplacing */); verify(mVpn, never()).refreshPlatformVpnAppExclusionList(); // Add package due to package replaced. - mService.onPackageAdded(PKGS[0], PKG_UIDS[0], true /* isReplacing */); + onPackageAdded(PKGS[0], PKG_UIDS[0], true /* isReplacing */); verify(mVpn, never()).refreshPlatformVpnAppExclusionList(); // Remove package - mService.onPackageRemoved(PKGS[0], PKG_UIDS[0], false /* isReplacing */); + onPackageRemoved(PKGS[0], PKG_UIDS[0], false /* isReplacing */); verify(mVpn).refreshPlatformVpnAppExclusionList(); // Add the package back - mService.onPackageAdded(PKGS[0], PKG_UIDS[0], false /* isReplacing */); + onPackageAdded(PKGS[0], PKG_UIDS[0], false /* isReplacing */); verify(mVpn, times(2)).refreshPlatformVpnAppExclusionList(); } @@ -172,4 +188,47 @@ public class VpnManagerServiceTest extends VpnTestBase { mService.getProvisionedVpnProfileState(TEST_VPN_PKG); verify(mVpn).getProvisionedVpnProfileState(TEST_VPN_PKG); } + + private Intent buildIntent(String action, String packageName, int userId, int uid, + boolean isReplacing) { + final Intent intent = new Intent(action); + intent.putExtra(Intent.EXTRA_USER_HANDLE, userId); + intent.putExtra(Intent.EXTRA_UID, uid); + intent.putExtra(Intent.EXTRA_REPLACING, isReplacing); + if (packageName != null) { + intent.setData(Uri.fromParts("package" /* scheme */, packageName, null /* fragment */)); + } + + return intent; + } + + private void sendIntent(Intent intent) { + final Handler h = mHandlerThread.getThreadHandler(); + + // Send in handler thread. + h.post(() -> mIntentReceiver.onReceive(mContext, intent)); + HandlerUtils.waitForIdle(mHandlerThread, TIMEOUT_MS); + } + + private void onUserStarted(int userId) { + sendIntent(buildIntent(Intent.ACTION_USER_STARTED, + null /* packageName */, userId, -1 /* uid */, false /* isReplacing */)); + } + + private void onPackageAdded(String packageName, int userId, int uid, boolean isReplacing) { + sendIntent(buildIntent(Intent.ACTION_PACKAGE_ADDED, packageName, userId, uid, isReplacing)); + } + + private void onPackageAdded(String packageName, int uid, boolean isReplacing) { + onPackageAdded(packageName, UserHandle.USER_SYSTEM, uid, isReplacing); + } + + private void onPackageRemoved(String packageName, int userId, int uid, boolean isReplacing) { + sendIntent(buildIntent(Intent.ACTION_PACKAGE_REMOVED, packageName, userId, uid, + isReplacing)); + } + + private void onPackageRemoved(String packageName, int uid, boolean isReplacing) { + onPackageRemoved(packageName, UserHandle.USER_SYSTEM, uid, isReplacing); + } }