From a185996c829a159bb27446697329b01464ab3c03 Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Thu, 16 Jul 2020 16:49:06 -0700 Subject: Fix the issue provider can be wrong when requesting slice permission SlicePermissionActivity reads provider_pkg from intent, which can be modified at will. As a result user might see incorrect package name in the dialog granting slice permission. Bug: 159145361 Test: manual Merged-In: I8b66c02786df4096dad74b7e76255d5ddd1d609d Change-Id: I8b66c02786df4096dad74b7e76255d5ddd1d609d (cherry picked from commit 4344e632953b103910b48d43f4eb226b38ed5048) --- core/java/android/app/slice/SliceProvider.java | 1 + .../android/systemui/SlicePermissionActivity.java | 31 +++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/core/java/android/app/slice/SliceProvider.java b/core/java/android/app/slice/SliceProvider.java index bd1eea51f8af..46be54814dc9 100644 --- a/core/java/android/app/slice/SliceProvider.java +++ b/core/java/android/app/slice/SliceProvider.java @@ -153,6 +153,7 @@ public abstract class SliceProvider extends ContentProvider { */ public static final String EXTRA_PKG = "pkg"; /** + * @Deprecated provider pkg is now being extracted in SlicePermissionActivity * @hide */ public static final String EXTRA_PROVIDER_PKG = "provider_pkg"; diff --git a/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java b/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java index 449ed8c3bcdb..1b241b743242 100644 --- a/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java +++ b/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java @@ -16,6 +16,7 @@ package com.android.systemui; import static android.view.WindowManager.LayoutParams.SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS; +import android.annotation.Nullable; import android.app.Activity; import android.app.AlertDialog; import android.app.slice.SliceManager; @@ -29,6 +30,7 @@ import android.content.pm.PackageManager.NameNotFoundException; import android.net.Uri; import android.os.Bundle; import android.text.BidiFormatter; +import android.util.EventLog; import android.util.Log; import android.widget.CheckBox; import android.widget.TextView; @@ -50,10 +52,12 @@ public class SlicePermissionActivity extends Activity implements OnClickListener mUri = getIntent().getParcelableExtra(SliceProvider.EXTRA_BIND_URI); mCallingPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PKG); - mProviderPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PROVIDER_PKG); try { PackageManager pm = getPackageManager(); + mProviderPkg = pm.resolveContentProvider(mUri.getAuthority(), + PackageManager.GET_META_DATA).applicationInfo.packageName; + verifyCallingPkg(); CharSequence app1 = BidiFormatter.getInstance().unicodeWrap(pm.getApplicationInfo( mCallingPkg, 0).loadSafeLabel(pm, PackageItemInfo.DEFAULT_MAX_LABEL_SIZE_PX, PackageItemInfo.SAFE_LABEL_FLAG_TRIM @@ -97,4 +101,29 @@ public class SlicePermissionActivity extends Activity implements OnClickListener public void onDismiss(DialogInterface dialog) { finish(); } + + private void verifyCallingPkg() { + final String providerPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PROVIDER_PKG); + if (providerPkg == null || mProviderPkg.equals(providerPkg)) return; + final String callingPkg = getCallingPkg(); + EventLog.writeEvent(0x534e4554, "159145361", getUid(callingPkg), String.format( + "pkg %s (disguised as %s) attempted to request permission to show %s slices in %s", + callingPkg, providerPkg, mProviderPkg, mCallingPkg)); + } + + @Nullable + private String getCallingPkg() { + final Uri referrer = getReferrer(); + if (referrer == null) return null; + return referrer.getHost(); + } + + private int getUid(@Nullable final String pkg) { + if (pkg == null) return -1; + try { + return getPackageManager().getApplicationInfo(pkg, 0).uid; + } catch (NameNotFoundException e) { + } + return -1; + } } -- cgit v1.2.3-59-g8ed1b From 9ac71ec05aecd46d4569405a63f2033c06fe38ac Mon Sep 17 00:00:00 2001 From: Jing Ji Date: Fri, 28 Aug 2020 14:55:48 -0700 Subject: Enforce permission checks in getting app exit reasons Bug: 165595677 Test: atest CtsSecurityTestCases:ActivityManagerTest Change-Id: Ia758d32bce6b2ac4c7145a96eccf68a962f0748b (cherry picked from commit e9b1dd415fed5415bc21abbd8e2f653fda5cf30c) --- .../core/java/com/android/server/am/ActivityManagerService.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 3b8518004309..091c77ecaac3 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -10459,12 +10459,10 @@ public class ActivityManagerService extends IActivityManager.Stub } finally { Binder.restoreCallingIdentity(identity); } - if (uid == Process.INVALID_UID) { - return Process.INVALID_UID; - } + // If the uid is Process.INVALID_UID, the below 'if' check will be always true if (UserHandle.getAppId(uid) != UserHandle.getAppId(callingUid)) { // Requires the DUMP permission if the target package doesn't belong - // to the caller. + // to the caller or it doesn't exist. enforceCallingPermission(android.Manifest.permission.DUMP, function); } return uid; -- cgit v1.2.3-59-g8ed1b From 9588cd7de1f84a3ec8de273fb7d75921024189d8 Mon Sep 17 00:00:00 2001 From: Curtis Belmonte Date: Thu, 1 Oct 2020 17:57:27 -0700 Subject: DO NOT MERGE Check fingerprint client against top activity in auth callback Due to a race condition with activity task stack broadcasts, it's currently possible for fingerprint authentication to succeed for a non-top activity. This means, for example, that a malicious overlay could be drawn in order to mislead the user about what they are authenticating for. This commit addresses the issue by adding a check to the biometric authentication client interface that ensures the authenticating activity is on top at the time of authentication. Otherwise, the pending authentication will fail, as if an incorrect biometric had been presented. Test: Follow steps from b/159249069: 1. Install com.pro100svitlo.fingerprintauthdemo from the Play store. 2. Install the PoC attack app from b/159249069. 3. Start the PoC attack app and press the "Launch PoC attack" button. 4. Use fingerprint to authenticate while the overlay is showing. Before: Authentication succeeds, and a new activity is launched. After: Authentication fails, and no new activity is launched. Bug: 159249069 Change-Id: Ie5a0f8c3e9b92d348a78678a6ed192d440c45ffc Merged-In: I289d67e5c7055ed60f7a96725c523d07cd047b23 Merged-In: I3a810cd7e6b97333f648c978e44242662342ec57 (cherry picked from commit 09c1b8ebf9e58cd402ec6a7ae9b1948cf83982d1) --- .../server/biometrics/AuthenticationClient.java | 53 ++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/services/core/java/com/android/server/biometrics/AuthenticationClient.java b/services/core/java/com/android/server/biometrics/AuthenticationClient.java index edc8f15a9a03..8ca37b813b63 100644 --- a/services/core/java/com/android/server/biometrics/AuthenticationClient.java +++ b/services/core/java/com/android/server/biometrics/AuthenticationClient.java @@ -16,16 +16,22 @@ package com.android.server.biometrics; +import android.app.ActivityManager; +import android.app.ActivityTaskManager; +import android.content.ComponentName; import android.content.Context; +import android.content.pm.ApplicationInfo; import android.hardware.biometrics.BiometricAuthenticator; import android.hardware.biometrics.BiometricConstants; import android.hardware.biometrics.BiometricsProtoEnums; import android.os.IBinder; import android.os.RemoteException; import android.security.KeyStore; +import android.util.EventLog; import android.util.Slog; import java.util.ArrayList; +import java.util.List; /** * A class to keep track of the authentication state for a given client. @@ -148,7 +154,54 @@ public abstract class AuthenticationClient extends ClientMonitor { + ", requireConfirmation: " + mRequireConfirmation + ", user: " + getTargetUserId()); + // Ensure authentication only succeeds if the client activity is on top or is keyguard. + boolean isBackgroundAuth = false; + if (authenticated && !Utils.isKeyguard(getContext(), getOwnerString())) { + try { + final List tasks = + ActivityTaskManager.getService().getTasks(1); + if (tasks == null || tasks.isEmpty()) { + Slog.e(TAG, "No running tasks reported"); + isBackgroundAuth = true; + } else { + final ComponentName topActivity = tasks.get(0).topActivity; + if (topActivity == null) { + Slog.e(TAG, "Unable to get top activity"); + isBackgroundAuth = true; + } else { + final String topPackage = topActivity.getPackageName(); + if (!topPackage.contentEquals(getOwnerString())) { + Slog.e(TAG, "Background authentication detected, top: " + topPackage + + ", client: " + this); + isBackgroundAuth = true; + } + } + } + } catch (RemoteException e) { + Slog.e(TAG, "Unable to get running tasks", e); + isBackgroundAuth = true; + } + } + + // Fail authentication if we can't confirm the client activity is on top. + if (isBackgroundAuth) { + Slog.e(TAG, "Failing possible background authentication"); + authenticated = false; + + // SafetyNet logging for exploitation attempts of b/159249069. + final ApplicationInfo appInfo = getContext().getApplicationInfo(); + EventLog.writeEvent(0x534e4554, "159249069", appInfo != null ? appInfo.uid : -1, + "Attempted background authentication"); + } + if (authenticated) { + // SafetyNet logging for b/159249069 if constraint is violated. + if (isBackgroundAuth) { + final ApplicationInfo appInfo = getContext().getApplicationInfo(); + EventLog.writeEvent(0x534e4554, "159249069", appInfo != null ? appInfo.uid : -1, + "Successful background authentication!"); + } + mAlreadyDone = true; if (listener != null) { -- cgit v1.2.3-59-g8ed1b From e237a83f95767f669b83508bb1f594091cbd6bac Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Thu, 8 Oct 2020 10:14:05 -0700 Subject: remove sensitive pii from safetynet logging Bug: 159145361 Test: manual Change-Id: I8f1be55971672c7e8f5aa8848f65b1b9d9f40fb5 Merged-In: I8f1be55971672c7e8f5aa8848f65b1b9d9f40fb5 (cherry picked from commit 3b6905bf6a39de7789f93a7ce6ca5d65a3fe589e) (cherry picked from commit 1d50760050a7c4e819d5a25d074dfaca04e68fde) --- .../SystemUI/src/com/android/systemui/SlicePermissionActivity.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java b/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java index 1b241b743242..57e656827f1c 100644 --- a/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java +++ b/packages/SystemUI/src/com/android/systemui/SlicePermissionActivity.java @@ -106,9 +106,7 @@ public class SlicePermissionActivity extends Activity implements OnClickListener final String providerPkg = getIntent().getStringExtra(SliceProvider.EXTRA_PROVIDER_PKG); if (providerPkg == null || mProviderPkg.equals(providerPkg)) return; final String callingPkg = getCallingPkg(); - EventLog.writeEvent(0x534e4554, "159145361", getUid(callingPkg), String.format( - "pkg %s (disguised as %s) attempted to request permission to show %s slices in %s", - callingPkg, providerPkg, mProviderPkg, mCallingPkg)); + EventLog.writeEvent(0x534e4554, "159145361", getUid(callingPkg)); } @Nullable -- cgit v1.2.3-59-g8ed1b From 0c17049d39b5a8867f030f6f36433564140e124a Mon Sep 17 00:00:00 2001 From: Eugene Susla Date: Mon, 12 Oct 2020 16:23:15 -0700 Subject: RESTRICT AUTOMERGE Fix CDM package check CDM was using a pckage check that returns a value intead of throwing, resulting in failing to throw on querying other package's associations Test: ensure attached bug no longer reproduces Bug: 167244818 Change-Id: I21319b6f5495dcae681541c76b847aad0c00b8ab (cherry picked from commit 30b022a8d207573aeaf735a33439884afe60c684) --- .../com/android/server/companion/CompanionDeviceManagerService.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index d6759b3e2cca..29fc1674bab9 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -372,7 +372,10 @@ public class CompanionDeviceManagerService extends SystemService implements Bind checkArgument(getCallingUserId() == userId, "Must be called by either same user or system"); - mAppOpsManager.checkPackage(Binder.getCallingUid(), pkg); + int callingUid = Binder.getCallingUid(); + if (mAppOpsManager.checkPackage(callingUid, pkg) != AppOpsManager.MODE_ALLOWED) { + throw new SecurityException(pkg + " doesn't belong to uid " + callingUid); + } } @Override -- cgit v1.2.3-59-g8ed1b From 03da463b2a94c36e3b46f0a110ec43710b82d404 Mon Sep 17 00:00:00 2001 From: Nate Myren Date: Wed, 14 Oct 2020 15:14:04 -0700 Subject: Ensure permissions are revoked on state changes If a permission owner changes, or a permission level is upgraded, revoke the permission from all packages Test: Manual Bug: 154505240 Merged-In: I0dec9eb7c2fecd3147e33e04d3f79f6dffcf7721 Change-Id: I0dec9eb7c2fecd3147e33e04d3f79f6dffcf7721 (cherry picked from commit a28931a09814a89e1c55816c794c1e1f20dc0c91) (cherry picked from commit 84c1247a7e6d446822e5c72c9718db8373011313) --- .../android/server/pm/PackageManagerService.java | 24 +++++- .../server/pm/permission/BasePermission.java | 19 +++++ .../pm/permission/PermissionManagerService.java | 90 ++++++++++++++++++++-- .../PermissionManagerServiceInternal.java | 16 +++- 4 files changed, 139 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index c20a912152cd..99e1388dcd52 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -330,6 +330,7 @@ import com.android.internal.os.SomeArgs; import com.android.internal.os.Zygote; import com.android.internal.telephony.CarrierAppUtils; import com.android.internal.util.ArrayUtils; +import com.android.internal.util.CollectionUtils; import com.android.internal.util.ConcurrentUtils; import com.android.internal.util.DumpUtils; import com.android.internal.util.FastXmlSerializer; @@ -12441,12 +12442,17 @@ public class PackageManagerService extends IPackageManager.Stub mPermissionManager.addAllPermissionGroups(pkg, chatty); } + // If a permission has had its defining app changed, or it has had its protection + // upgraded, we need to revoke apps that hold it + final List permissionsWithChangedDefinition; // Don't allow ephemeral applications to define new permissions. if ((scanFlags & SCAN_AS_INSTANT_APP) != 0) { + permissionsWithChangedDefinition = null; Slog.w(TAG, "Permissions from package " + pkg.getPackageName() + " ignored: instant apps cannot define new permissions."); } else { - mPermissionManager.addAllPermissions(pkg, chatty); + permissionsWithChangedDefinition = + mPermissionManager.addAllPermissions(pkg, chatty); } int collectionSize = ArrayUtils.size(pkg.getInstrumentations()); @@ -12475,7 +12481,10 @@ public class PackageManagerService extends IPackageManager.Stub } } - if (oldPkg != null) { + boolean hasOldPkg = oldPkg != null; + boolean hasPermissionDefinitionChanges = + !CollectionUtils.isEmpty(permissionsWithChangedDefinition); + if (hasOldPkg || hasPermissionDefinitionChanges) { // We need to call revokeRuntimePermissionsIfGroupChanged async as permission // revoke callbacks from this method might need to kill apps which need the // mPackages lock on a different thread. This would dead lock. @@ -12486,9 +12495,16 @@ public class PackageManagerService extends IPackageManager.Stub // won't be granted yet, hence new packages are no problem. final ArrayList allPackageNames = new ArrayList<>(mPackages.keySet()); - AsyncTask.execute(() -> + AsyncTask.execute(() -> { + if (hasOldPkg) { mPermissionManager.revokeRuntimePermissionsIfGroupChanged(pkg, oldPkg, - allPackageNames)); + allPackageNames); + } + if (hasPermissionDefinitionChanges) { + mPermissionManager.revokeRuntimePermissionsIfPermissionDefinitionChanged( + permissionsWithChangedDefinition, allPackageNames); + } + }); } } diff --git a/services/core/java/com/android/server/pm/permission/BasePermission.java b/services/core/java/com/android/server/pm/permission/BasePermission.java index cfa0449aaf33..2d4b5012bf15 100644 --- a/services/core/java/com/android/server/pm/permission/BasePermission.java +++ b/services/core/java/com/android/server/pm/permission/BasePermission.java @@ -83,6 +83,8 @@ public final class BasePermission { final @PermissionType int type; + private boolean mPermissionDefinitionChanged; + String sourcePackageName; int protectionLevel; @@ -126,6 +128,11 @@ public final class BasePermission { public String getSourcePackageName() { return sourcePackageName; } + + public boolean isPermissionDefinitionChanged() { + return mPermissionDefinitionChanged; + } + public int getType() { return type; } @@ -140,6 +147,10 @@ public final class BasePermission { this.perm = perm; } + public void setPermissionDefinitionChanged(boolean shouldOverride) { + mPermissionDefinitionChanged = shouldOverride; + } + public int[] computeGids(int userId) { if (perUser) { final int[] userGids = new int[gids.length]; @@ -322,6 +333,7 @@ public final class BasePermission { final PackageSettingBase pkgSetting = (PackageSettingBase) packageManagerInternal.getPackageSetting(pkg.getPackageName()); // Allow system apps to redefine non-system permissions + boolean ownerChanged = false; if (bp != null && !Objects.equals(bp.sourcePackageName, p.getPackageName())) { final boolean currentOwnerIsSystem; if (bp.perm == null) { @@ -347,6 +359,7 @@ public final class BasePermission { String msg = "New decl " + pkg + " of permission " + p.getName() + " is system; overriding " + bp.sourcePackageName; PackageManagerService.reportSettingsProblem(Log.WARN, msg); + ownerChanged = true; bp = null; } } @@ -354,6 +367,7 @@ public final class BasePermission { if (bp == null) { bp = new BasePermission(p.getName(), p.getPackageName(), TYPE_NORMAL); } + boolean wasNormal = bp.isNormal(); StringBuilder r = null; if (bp.perm == null) { if (bp.sourcePackageName == null @@ -397,6 +411,11 @@ public final class BasePermission { && Objects.equals(bp.perm.getName(), p.getName())) { bp.protectionLevel = p.getProtectionLevel(); } + if (bp.isRuntime() && (ownerChanged || wasNormal)) { + // If this is a runtime permission and the owner has changed, or this was a normal + // permission, then permission state should be cleaned up + bp.mPermissionDefinitionChanged = true; + } if (PackageManagerService.DEBUG_PACKAGE_SCANNING && r != null) { Log.d(TAG, " Permissions: " + r); } diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index 66d8b5974261..3ffca028b1c0 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -2344,8 +2344,74 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } - private void addAllPermissions(AndroidPackage pkg, boolean chatty) { + /** + * If permissions are upgraded to runtime, or their owner changes to the system, then any + * granted permissions must be revoked. + * + * @param permissionsToRevoke A list of permission names to revoke + * @param allPackageNames All package names + * @param permissionCallback Callback for permission changed + */ + private void revokeRuntimePermissionsIfPermissionDefinitionChanged( + @NonNull List permissionsToRevoke, + @NonNull ArrayList allPackageNames, + @NonNull PermissionCallback permissionCallback) { + + final int[] userIds = mUserManagerInt.getUserIds(); + final int numPermissions = permissionsToRevoke.size(); + final int numUserIds = userIds.length; + final int numPackages = allPackageNames.size(); + final int callingUid = Binder.getCallingUid(); + + for (int permNum = 0; permNum < numPermissions; permNum++) { + String permName = permissionsToRevoke.get(permNum); + BasePermission bp = mSettings.getPermission(permName); + if (bp == null || !bp.isRuntime()) { + continue; + } + for (int userIdNum = 0; userIdNum < numUserIds; userIdNum++) { + final int userId = userIds[userIdNum]; + for (int packageNum = 0; packageNum < numPackages; packageNum++) { + final String packageName = allPackageNames.get(packageNum); + final int uid = mPackageManagerInt.getPackageUid(packageName, 0, userId); + if (uid < Process.FIRST_APPLICATION_UID) { + // do not revoke from system apps + continue; + } + final int permissionState = checkPermissionImpl(permName, packageName, + userId); + final int flags = getPermissionFlags(permName, packageName, userId); + final int flagMask = FLAG_PERMISSION_SYSTEM_FIXED + | FLAG_PERMISSION_POLICY_FIXED + | FLAG_PERMISSION_GRANTED_BY_DEFAULT + | FLAG_PERMISSION_GRANTED_BY_ROLE; + if (permissionState == PackageManager.PERMISSION_GRANTED + && (flags & flagMask) == 0) { + EventLog.writeEvent(0x534e4554, "154505240", uid, + "Revoking permission " + permName + " from package " + + packageName + " due to definition change"); + EventLog.writeEvent(0x534e4554, "168319670", uid, + "Revoking permission " + permName + " from package " + + packageName + " due to definition change"); + Slog.e(TAG, "Revoking permission " + permName + " from package " + + packageName + " due to definition change"); + try { + revokeRuntimePermissionInternal(permName, packageName, + false, callingUid, userId, null, permissionCallback); + } catch (Exception e) { + Slog.e(TAG, "Could not revoke " + permName + " from " + + packageName, e); + } + } + } + } + bp.setPermissionDefinitionChanged(false); + } + } + + private List addAllPermissions(AndroidPackage pkg, boolean chatty) { final int N = ArrayUtils.size(pkg.getPermissions()); + ArrayList definitionChangedPermissions = new ArrayList<>(); for (int i=0; i permissionsToRevoke, + @NonNull ArrayList allPackageNames) { + PermissionManagerService.this.revokeRuntimePermissionsIfPermissionDefinitionChanged( + permissionsToRevoke, allPackageNames, mDefaultPermissionCallback); + } + @Override - public void addAllPermissions(AndroidPackage pkg, boolean chatty) { - PermissionManagerService.this.addAllPermissions(pkg, chatty); + public List addAllPermissions(AndroidPackage pkg, boolean chatty) { + return PermissionManagerService.this.addAllPermissions(pkg, chatty); } @Override public void addAllPermissionGroups(AndroidPackage pkg, boolean chatty) { diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java index 2e83b23f57d8..31a65ba35f5e 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java @@ -253,13 +253,27 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager @NonNull AndroidPackage oldPackage, @NonNull ArrayList allPackageNames); + /** + * Some permissions might have been owned by a non-system package, and the system then defined + * said permission. Some other permissions may one have been install permissions, but are now + * runtime or higher. These permissions should be revoked. + * + * @param permissionsToRevoke A list of permission names to revoke + * @param allPackageNames All packages + */ + public abstract void revokeRuntimePermissionsIfPermissionDefinitionChanged( + @NonNull List permissionsToRevoke, + @NonNull ArrayList allPackageNames); + /** * Add all permissions in the given package. *

* NOTE: argument {@code groupTEMP} is temporary until mPermissionGroups is moved to * the permission settings. + * + * @return A list of BasePermissions that were updated, and need to be revoked from packages */ - public abstract void addAllPermissions(@NonNull AndroidPackage pkg, boolean chatty); + public abstract List addAllPermissions(@NonNull AndroidPackage pkg, boolean chatty); public abstract void addAllPermissionGroups(@NonNull AndroidPackage pkg, boolean chatty); public abstract void removeAllPermissions(@NonNull AndroidPackage pkg, boolean chatty); -- cgit v1.2.3-59-g8ed1b From c4ce178c261e6a2dc1aa4c1e1d570f0efd980e47 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Tue, 3 Nov 2020 15:12:44 -0800 Subject: Revoke permission on non-runtime -> runtime upgrade Not only on normal -> runtime. Test: atest PermissionEscalationTest Bug: 154505240, 168319670 Change-Id: If3b420067b4d7111dcf67ae6f98e42176158b679 Merged-In: If3b420067b4d7111dcf67ae6f98e42176158b679 (cherry picked from commit 33e24c883c5797fe7ba05468bde967fb0e46e66d) --- .../core/java/com/android/server/pm/permission/BasePermission.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/pm/permission/BasePermission.java b/services/core/java/com/android/server/pm/permission/BasePermission.java index 2d4b5012bf15..5e04171a3bca 100644 --- a/services/core/java/com/android/server/pm/permission/BasePermission.java +++ b/services/core/java/com/android/server/pm/permission/BasePermission.java @@ -367,7 +367,7 @@ public final class BasePermission { if (bp == null) { bp = new BasePermission(p.getName(), p.getPackageName(), TYPE_NORMAL); } - boolean wasNormal = bp.isNormal(); + boolean wasNonRuntime = !bp.isRuntime(); StringBuilder r = null; if (bp.perm == null) { if (bp.sourcePackageName == null @@ -411,7 +411,7 @@ public final class BasePermission { && Objects.equals(bp.perm.getName(), p.getName())) { bp.protectionLevel = p.getProtectionLevel(); } - if (bp.isRuntime() && (ownerChanged || wasNormal)) { + if (bp.isRuntime() && (ownerChanged || wasNonRuntime)) { // If this is a runtime permission and the owner has changed, or this was a normal // permission, then permission state should be cleaned up bp.mPermissionDefinitionChanged = true; -- cgit v1.2.3-59-g8ed1b From 90cfe17643aa4ecbe7cbfb1c787217456f764e08 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Wed, 28 Oct 2020 14:00:32 -0700 Subject: Hide overlays over uninstall confirm dialog Test: atest CtsPackageUninstallTestCases Fixes: 171221302 Change-Id: I38b6d85871064d76f2911e20acc74b4ab76406b3 (cherry picked from commit cdec871e660202ac20fea9805c35ca8e7fc66b20) --- .../src/com/android/packageinstaller/UninstallerActivity.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java b/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java index be778e92787e..94829b506f95 100755 --- a/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java +++ b/packages/PackageInstaller/src/com/android/packageinstaller/UninstallerActivity.java @@ -17,6 +17,7 @@ package com.android.packageinstaller; import static android.app.AppOpsManager.MODE_ALLOWED; +import static android.view.WindowManager.LayoutParams.SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS; import static com.android.packageinstaller.PackageUtil.getMaxTargetSdkVersionForUid; @@ -87,6 +88,8 @@ public class UninstallerActivity extends Activity { @Override public void onCreate(Bundle icicle) { + getWindow().addSystemFlags(SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS); + // Never restore any state, esp. never create any fragments. The data in the fragment might // be stale, if e.g. the app was uninstalled while the activity was destroyed. super.onCreate(null); -- cgit v1.2.3-59-g8ed1b From 828fe0b915f30e22fec03dc1ed2e66220ceebd3e Mon Sep 17 00:00:00 2001 From: Dmitry Dementyev Date: Mon, 9 Nov 2020 16:44:08 -0800 Subject: Protect GrantCredentialsPermissionActivity against overlay. Bug: 169763814 Test: manual Change-Id: I15dd22791fcc61ef02b06ad51d9e4409d11c0181 (cherry picked from commit f45dcfe1f5ed74dcec15799f2abd9e5ecbbed4b7) --- core/java/android/accounts/GrantCredentialsPermissionActivity.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/java/android/accounts/GrantCredentialsPermissionActivity.java b/core/java/android/accounts/GrantCredentialsPermissionActivity.java index af74b036a796..32b61b5ed8cc 100644 --- a/core/java/android/accounts/GrantCredentialsPermissionActivity.java +++ b/core/java/android/accounts/GrantCredentialsPermissionActivity.java @@ -47,6 +47,9 @@ public class GrantCredentialsPermissionActivity extends Activity implements View protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); + getWindow().addSystemFlags( + android.view.WindowManager.LayoutParams + .SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS); setContentView(R.layout.grant_credentials_permission); setTitle(R.string.grant_permissions_header_text); -- cgit v1.2.3-59-g8ed1b From 0b610a27ba60047842b9416dd0537c68f0dd22b2 Mon Sep 17 00:00:00 2001 From: Dmitry Dementyev Date: Tue, 10 Nov 2020 16:01:35 -0800 Subject: Ignore GrantCredentials call with unexpected calling uid. Activity can be used only in two cases. 1) Calling uid matches uid grantee. 2) Calling uid is is system. This flow is used by getToken methods with notifyAuthFailure=true. Test: Existing CTS tests Bug: 158480899 Change-Id: I1421c333b6cebb4f7cddcdd8766298f6872e933b (cherry picked from commit 10d8a114bbd91c07f2feb5009328b16203ff50b1) --- .../GrantCredentialsPermissionActivity.java | 34 ++++++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/core/java/android/accounts/GrantCredentialsPermissionActivity.java b/core/java/android/accounts/GrantCredentialsPermissionActivity.java index 32b61b5ed8cc..5dc6e602e5d6 100644 --- a/core/java/android/accounts/GrantCredentialsPermissionActivity.java +++ b/core/java/android/accounts/GrantCredentialsPermissionActivity.java @@ -16,16 +16,23 @@ package android.accounts; import android.app.Activity; -import android.content.res.Resources; -import android.os.Bundle; -import android.widget.TextView; -import android.widget.LinearLayout; -import android.view.View; -import android.view.LayoutInflater; +import android.app.ActivityTaskManager; import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; +import android.content.res.Resources; +import android.os.Bundle; +import android.os.IBinder; +import android.os.Process; +import android.os.RemoteException; +import android.os.UserHandle; import android.text.TextUtils; +import android.util.Log; +import android.view.LayoutInflater; +import android.view.View; +import android.widget.LinearLayout; +import android.widget.TextView; + import com.android.internal.R; import java.io.IOException; @@ -42,6 +49,7 @@ public class GrantCredentialsPermissionActivity extends Activity implements View private Account mAccount; private String mAuthTokenType; private int mUid; + private int mCallingUid; private Bundle mResultBundle = null; protected LayoutInflater mInflater; @@ -77,6 +85,20 @@ public class GrantCredentialsPermissionActivity extends Activity implements View return; } + try { + IBinder activityToken = getActivityToken(); + mCallingUid = ActivityTaskManager.getService().getLaunchedFromUid(activityToken); + } catch (RemoteException re) { + // Couldn't figure out caller details + Log.w(getClass().getSimpleName(), "Unable to get caller identity \n" + re); + } + + if (!UserHandle.isSameApp(mCallingUid, Process.SYSTEM_UID) && mCallingUid != mUid) { + setResult(Activity.RESULT_CANCELED); + finish(); + return; + } + String accountTypeLabel; try { accountTypeLabel = getAccountLabel(mAccount); -- cgit v1.2.3-59-g8ed1b