From 0f13f70655099543ba34eb8aeaa74b34a3993a3b Mon Sep 17 00:00:00 2001 From: Jason Chiu Date: Thu, 23 Mar 2023 15:30:19 +0800 Subject: [PATCH] Refine permission check process of 2-pane deep link - Check the deep link activity instance before redirecting to the internal activity for the managed profile invocation, so the caller can't bypass the permission check. - Get the referrer as the caller so that onNewIntent can recognize the new caller and check if it has a permission to open the target page. Test: robotest & manual Bug: 268193384 Change-Id: Ie69742983fb74ee2316b7aad16461db95ed927c2 Merged-In: Ie69742983fb74ee2316b7aad16461db95ed927c2 --- .../homepage/SettingsHomepageActivity.java | 94 +++++++++++++------ .../SettingsHomepageActivityTest.java | 92 +++++++++++++++--- 2 files changed, 145 insertions(+), 41 deletions(-) diff --git a/src/com/android/settings/homepage/SettingsHomepageActivity.java b/src/com/android/settings/homepage/SettingsHomepageActivity.java index a23b743aade..b3f84d68e00 100644 --- a/src/com/android/settings/homepage/SettingsHomepageActivity.java +++ b/src/com/android/settings/homepage/SettingsHomepageActivity.java @@ -29,11 +29,12 @@ import android.content.ComponentName; import android.content.Intent; import android.content.pm.ActivityInfo; import android.content.pm.PackageManager; +import android.content.pm.PackageManager.ApplicationInfoFlags; import android.content.pm.UserInfo; import android.content.res.Configuration; +import android.net.Uri; import android.os.Bundle; import android.os.Process; -import android.os.RemoteException; import android.os.UserHandle; import android.os.UserManager; import android.text.TextUtils; @@ -70,7 +71,6 @@ import com.android.settings.core.CategoryMixin; import com.android.settings.core.FeatureFlags; import com.android.settings.homepage.contextualcards.ContextualCardsFragment; import com.android.settings.overlay.FeatureFactory; -import com.android.settings.password.PasswordUtils; import com.android.settingslib.Utils; import com.android.settingslib.core.lifecycle.HideNonSystemOverlayMixin; @@ -92,6 +92,10 @@ public class SettingsHomepageActivity extends FragmentActivity implements public static final String EXTRA_SETTINGS_LARGE_SCREEN_DEEP_LINK_INTENT_DATA = "settings_large_screen_deep_link_intent_data"; + // The referrer who fires the initial intent to start the homepage + @VisibleForTesting + static final String EXTRA_INITIAL_REFERRER = "initial_referrer"; + static final int DEFAULT_HIGHLIGHT_MENU_KEY = R.string.menu_key_network; private static final long HOMEPAGE_LOADING_TIMEOUT_MS = 300; @@ -173,12 +177,16 @@ public class SettingsHomepageActivity extends FragmentActivity implements mIsEmbeddingActivityEnabled = ActivityEmbeddingUtils.isEmbeddingActivityEnabled(this); if (mIsEmbeddingActivityEnabled) { final UserManager um = getSystemService(UserManager.class); - final UserInfo userInfo = um.getUserInfo(getUser().getIdentifier()); + final UserInfo userInfo = um.getUserInfo(getUserId()); if (userInfo.isManagedProfile()) { final Intent intent = new Intent(getIntent()) - .setClass(this, DeepLinkHomepageActivityInternal.class) .addFlags(Intent.FLAG_ACTIVITY_FORWARD_RESULT) - .putExtra(EXTRA_USER_HANDLE, getUser()); + .putExtra(EXTRA_USER_HANDLE, getUser()) + .putExtra(EXTRA_INITIAL_REFERRER, getCurrentReferrer()); + if (TextUtils.equals(intent.getAction(), ACTION_SETTINGS_EMBED_DEEP_LINK_ACTIVITY) + && this instanceof DeepLinkHomepageActivity) { + intent.setClass(this, DeepLinkHomepageActivityInternal.class); + } intent.removeFlags(Intent.FLAG_ACTIVITY_NEW_TASK); startActivityAsUser(intent, um.getPrimaryUser().getUserHandle()); finish(); @@ -438,7 +446,7 @@ public class SettingsHomepageActivity extends FragmentActivity implements return; } - ActivityInfo targetActivityInfo = null; + ActivityInfo targetActivityInfo; try { targetActivityInfo = getPackageManager().getActivityInfo(targetComponentName, /* flags= */ 0); @@ -448,23 +456,29 @@ public class SettingsHomepageActivity extends FragmentActivity implements return; } - int callingUid = -1; - try { - callingUid = ActivityManager.getService().getLaunchedFromUid(getActivityToken()); - } catch (RemoteException re) { - Log.e(TAG, "Not able to get callingUid: " + re); - finish(); - return; + UserHandle user = intent.getParcelableExtra(EXTRA_USER_HANDLE, UserHandle.class); + String caller = getInitialReferrer(); + int callerUid = -1; + if (caller != null) { + try { + callerUid = getPackageManager().getApplicationInfoAsUser(caller, + ApplicationInfoFlags.of(/* flags= */ 0), + user != null ? user.getIdentifier() : getUserId()).uid; + } catch (PackageManager.NameNotFoundException e) { + Log.e(TAG, "Not able to get callerUid: " + e); + finish(); + return; + } } - if (!hasPrivilegedAccess(callingUid, targetActivityInfo)) { + if (!hasPrivilegedAccess(caller, callerUid, targetActivityInfo.packageName)) { if (!targetActivityInfo.exported) { Log.e(TAG, "Target Activity is not exported"); finish(); return; } - if (!isCallingAppPermitted(targetActivityInfo.permission)) { + if (!isCallingAppPermitted(targetActivityInfo.permission, callerUid)) { Log.e(TAG, "Calling app must have the permission of deep link Activity"); finish(); return; @@ -494,7 +508,7 @@ public class SettingsHomepageActivity extends FragmentActivity implements & (Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION); if (targetIntent.getData() != null && uriPermissionFlags != 0 - && checkUriPermission(targetIntent.getData(), /* pid= */ -1, callingUid, + && checkUriPermission(targetIntent.getData(), /* pid= */ -1, callerUid, uriPermissionFlags) == PackageManager.PERMISSION_DENIED) { Log.e(TAG, "Calling app must have the permission to access Uri and grant permission"); finish(); @@ -517,7 +531,6 @@ public class SettingsHomepageActivity extends FragmentActivity implements SplitRule.FINISH_ALWAYS, true /* clearTop */); - final UserHandle user = intent.getParcelableExtra(EXTRA_USER_HANDLE, UserHandle.class); if (user != null) { startActivityAsUser(targetIntent, user); } else { @@ -525,31 +538,30 @@ public class SettingsHomepageActivity extends FragmentActivity implements } } - // Check if calling app has privileged access to launch Activity of activityInfo. - private boolean hasPrivilegedAccess(int callingUid, ActivityInfo activityInfo) { - if (TextUtils.equals(PasswordUtils.getCallingAppPackageName(getActivityToken()), - getPackageName())) { + // Check if the caller has privileged access to launch the target page. + private boolean hasPrivilegedAccess(String callerPkg, int callerUid, String targetPackage) { + if (TextUtils.equals(callerPkg, getPackageName())) { return true; } int targetUid = -1; try { - targetUid = getPackageManager().getApplicationInfo(activityInfo.packageName, - /* flags= */ 0).uid; - } catch (PackageManager.NameNotFoundException nnfe) { - Log.e(TAG, "Not able to get targetUid: " + nnfe); + targetUid = getPackageManager().getApplicationInfo(targetPackage, + ApplicationInfoFlags.of(/* flags= */ 0)).uid; + } catch (PackageManager.NameNotFoundException e) { + Log.e(TAG, "Not able to get targetUid: " + e); return false; } // When activityInfo.exported is false, Activity still can be launched if applications have // the same user ID. - if (UserHandle.isSameApp(callingUid, targetUid)) { + if (UserHandle.isSameApp(callerUid, targetUid)) { return true; } // When activityInfo.exported is false, Activity still can be launched if calling app has // root or system privilege. - int callingAppId = UserHandle.getAppId(callingUid); + int callingAppId = UserHandle.getAppId(callerUid); if (callingAppId == Process.ROOT_UID || callingAppId == Process.SYSTEM_UID) { return true; } @@ -558,9 +570,31 @@ public class SettingsHomepageActivity extends FragmentActivity implements } @VisibleForTesting - boolean isCallingAppPermitted(String permission) { - return TextUtils.isEmpty(permission) || PasswordUtils.isCallingAppPermitted( - this, getActivityToken(), permission); + String getInitialReferrer() { + String referrer = getCurrentReferrer(); + if (!TextUtils.equals(referrer, getPackageName())) { + return referrer; + } + + String initialReferrer = getIntent().getStringExtra(EXTRA_INITIAL_REFERRER); + return TextUtils.isEmpty(initialReferrer) ? referrer : initialReferrer; + } + + @VisibleForTesting + String getCurrentReferrer() { + Intent intent = getIntent(); + // Clear extras to get the real referrer + intent.removeExtra(Intent.EXTRA_REFERRER); + intent.removeExtra(Intent.EXTRA_REFERRER_NAME); + Uri referrer = getReferrer(); + return referrer != null ? referrer.getHost() : null; + } + + @VisibleForTesting + boolean isCallingAppPermitted(String permission, int callerUid) { + return TextUtils.isEmpty(permission) + || checkPermission(permission, /* pid= */ -1, callerUid) + == PackageManager.PERMISSION_GRANTED; } private String getHighlightMenuKey() { diff --git a/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java b/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java index 4de8b005c3c..bf109c46c1a 100644 --- a/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java +++ b/tests/robotests/src/com/android/settings/homepage/SettingsHomepageActivityTest.java @@ -20,14 +20,24 @@ import static android.view.WindowManager.LayoutParams.SYSTEM_FLAG_HIDE_NON_SYSTE import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; + +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.ActivityManager; +import android.content.Intent; +import android.content.pm.PackageManager; +import android.net.Uri; import android.os.Build; import android.view.View; import android.view.Window; @@ -205,29 +215,89 @@ public class SettingsHomepageActivityTest { } @Test - @Config(shadows = {ShadowPasswordUtils.class}) + public void getInitialReferrer_differentPackage_returnCurrentReferrer() { + SettingsHomepageActivity activity = + spy(Robolectric.buildActivity(SettingsHomepageActivity.class).get()); + String referrer = "com.abc"; + doReturn(referrer).when(activity).getCurrentReferrer(); + + assertEquals(activity.getInitialReferrer(), referrer); + } + + @Test + public void getInitialReferrer_noReferrerExtra_returnCurrentReferrer() { + SettingsHomepageActivity activity = + spy(Robolectric.buildActivity(SettingsHomepageActivity.class).get()); + String referrer = activity.getPackageName(); + doReturn(referrer).when(activity).getCurrentReferrer(); + + assertEquals(activity.getInitialReferrer(), referrer); + } + + @Test + public void getInitialReferrer_hasReferrerExtra_returnGivenReferrer() { + SettingsHomepageActivity activity = + spy(Robolectric.buildActivity(SettingsHomepageActivity.class).get()); + doReturn(activity.getPackageName()).when(activity).getCurrentReferrer(); + String referrer = "com.abc"; + activity.setIntent(new Intent().putExtra(SettingsHomepageActivity.EXTRA_INITIAL_REFERRER, + referrer)); + + assertEquals(activity.getInitialReferrer(), referrer); + } + + @Test + public void getCurrentReferrer_hasReferrerExtra_shouldNotEqual() { + String referrer = "com.abc"; + Uri uri = new Uri.Builder().scheme("android-app").authority(referrer).build(); + SettingsHomepageActivity activity = + spy(Robolectric.buildActivity(SettingsHomepageActivity.class).get()); + activity.setIntent(new Intent().putExtra(Intent.EXTRA_REFERRER, uri)); + + assertNotEquals(activity.getCurrentReferrer(), referrer); + } + + @Test + public void getCurrentReferrer_hasReferrerNameExtra_shouldNotEqual() { + String referrer = "com.abc"; + SettingsHomepageActivity activity = + spy(Robolectric.buildActivity(SettingsHomepageActivity.class).get()); + activity.setIntent(new Intent().putExtra(Intent.EXTRA_REFERRER_NAME, referrer)); + + assertNotEquals(activity.getCurrentReferrer(), referrer); + } + + @Test public void isCallingAppPermitted_emptyPermission_returnTrue() { - SettingsHomepageActivity homepageActivity = spy(new SettingsHomepageActivity()); + SettingsHomepageActivity activity = + spy(Robolectric.buildActivity(SettingsHomepageActivity.class).get()); + doReturn(PackageManager.PERMISSION_DENIED).when(activity) + .checkPermission(anyString(), anyInt(), anyInt()); - assertTrue(homepageActivity.isCallingAppPermitted("")); + assertTrue(activity.isCallingAppPermitted("", 1000)); } @Test - @Config(shadows = {ShadowPasswordUtils.class}) - public void isCallingAppPermitted_noGrantedPermission_returnFalse() { - SettingsHomepageActivity homepageActivity = spy(new SettingsHomepageActivity()); + public void isCallingAppPermitted_notGrantedPermission_returnFalse() { + SettingsHomepageActivity activity = + spy(Robolectric.buildActivity(SettingsHomepageActivity.class).get()); + doReturn(PackageManager.PERMISSION_DENIED).when(activity) + .checkPermission(anyString(), anyInt(), anyInt()); - assertFalse(homepageActivity.isCallingAppPermitted("android.permission.TEST")); + assertFalse(activity.isCallingAppPermitted("android.permission.TEST", 1000)); } @Test - @Config(shadows = {ShadowPasswordUtils.class}) public void isCallingAppPermitted_grantedPermission_returnTrue() { - SettingsHomepageActivity homepageActivity = spy(new SettingsHomepageActivity()); + SettingsHomepageActivity activity = + spy(Robolectric.buildActivity(SettingsHomepageActivity.class).get()); String permission = "android.permission.TEST"; - ShadowPasswordUtils.addGrantedPermission(permission); + doReturn(PackageManager.PERMISSION_DENIED).when(activity) + .checkPermission(anyString(), anyInt(), anyInt()); + doReturn(PackageManager.PERMISSION_GRANTED).when(activity) + .checkPermission(eq(permission), anyInt(), eq(1000)); - assertTrue(homepageActivity.isCallingAppPermitted(permission)); + assertTrue(activity.isCallingAppPermitted(permission, 1000)); } @Implements(SuggestionFeatureProviderImpl.class)