Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Do not allow to install S+ apps with cross cert permissions

Browse Source 
I.e. permissions and permission groups should stay inside a cert-group
so that there cannot be accidential security bugs in apps.

Test: atest CtsPermissionTestCases
            CtsPermission2TestCases
	    CtsAppSecurityHostTestCases
Fixes: 146211400 (No backport possible, all changes are for S+ apps
only)
Change-Id: I19c2f3e216ea57a9e25c65e276f87425aeb1c038
This commit is contained in:
Philip P. Moltmann
2020-10-14 20:17:36 -07:00
parent db6b4cae08
commit 22d2486c1f
4 changed files with 164 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
core/java/android/content/pm/PackageManager.java
View File

@@ -1577,6 +1577,26 @@ public abstract class PackageManager {
*/
public static final int INSTALL_PARSE_FAILED_SKIPPED = -125;
/**
* Installation failed return code: this is passed in the
* {@link PackageInstaller#EXTRA_LEGACY_STATUS} if the system failed to install the package
* because it is attempting to define a permission group that is already defined by some
* existing package.
*
* @hide
*/
public static final int INSTALL_FAILED_DUPLICATE_PERMISSION_GROUP = -126;
/**
* Installation failed return code: this is passed in the
* {@link PackageInstaller#EXTRA_LEGACY_STATUS} if the system failed to install the package
* because it is attempting to define a permission in a group that does not exists or that is
* defined by an packages with an incompatible certificate.
*
* @hide
*/
public static final int INSTALL_FAILED_BAD_PERMISSION_GROUP = -127;
/** @hide */
@IntDef(flag = true, prefix = { "DELETE_" }, value = {
DELETE_KEEP_DATA,

15
core/java/android/permission/PermissionManager.java
View File

@@ -16,6 +16,8 @@
package android.permission;
import static android.os.Build.VERSION_CODES.S;
import android.Manifest;
import android.annotation.CallbackExecutor;
import android.annotation.IntRange;
@@ -30,6 +32,8 @@ import android.app.ActivityManager;
import android.app.ActivityThread;
import android.app.IActivityManager;
import android.app.PropertyInvalidatedCache;
import android.compat.annotation.ChangeId;
import android.compat.annotation.EnabledAfter;
import android.content.Context;
import android.content.pm.IPackageManager;
import android.content.pm.PackageManager;
@@ -70,6 +74,17 @@ public final class PermissionManager {
public static final String KILL_APP_REASON_GIDS_CHANGED =
"permission grant or revoke changed gids";
/**
* Refuse to install package if groups of permissions are bad
* - Permission groups should only be shared between apps sharing a certificate
* - If a permission belongs to a group that group should be defined
*
* @hide
*/
@ChangeId
@EnabledAfter(targetSdkVersion = S)
public static final long CANNOT_INSTALL_WITH_BAD_PERMISSION_GROUPS = 146211400;
private final @NonNull Context mContext;
private final IPackageManager mPackageManager;

3
core/tests/coretests/apks/install_decl_perm/AndroidManifest.xml
View File

@@ -17,17 +17,14 @@
package="com.android.frameworks.coretests.install_decl_perm">
<permission android:name="com.android.frameworks.coretests.NORMAL"
android:permissionGroup="android.permission-group.COST_MONEY"
android:protectionLevel="normal"
android:label="test normal perm" />
<permission android:name="com.android.frameworks.coretests.DANGEROUS"
android:permissionGroup="android.permission-group.COST_MONEY"
android:protectionLevel="dangerous"
android:label="test dangerous perm" />
<permission android:name="com.android.frameworks.coretests.SIGNATURE"
android:permissionGroup="android.permission-group.COST_MONEY"
android:protectionLevel="signature"
android:label="test signature perm" />

169
services/core/java/com/android/server/pm/PackageManagerService.java
View File

@@ -1,6 +1,5 @@
/*
* Copyright (C) 2006 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
@@ -47,8 +46,10 @@ import static android.content.pm.PackageManager.FLAG_PERMISSION_USER_FIXED;
import static android.content.pm.PackageManager.FLAG_PERMISSION_USER_SET;
import static android.content.pm.PackageManager.FLAG_PERMISSION_WHITELIST_INSTALLER;
import static android.content.pm.PackageManager.INSTALL_FAILED_ALREADY_EXISTS;
import static android.content.pm.PackageManager.INSTALL_FAILED_BAD_PERMISSION_GROUP;
import static android.content.pm.PackageManager.INSTALL_FAILED_DUPLICATE_PACKAGE;
import static android.content.pm.PackageManager.INSTALL_FAILED_DUPLICATE_PERMISSION;
import static android.content.pm.PackageManager.INSTALL_FAILED_DUPLICATE_PERMISSION_GROUP;
import static android.content.pm.PackageManager.INSTALL_FAILED_INSUFFICIENT_STORAGE;
import static android.content.pm.PackageManager.INSTALL_FAILED_INTERNAL_ERROR;
import static android.content.pm.PackageManager.INSTALL_FAILED_INVALID_APK;
@@ -234,6 +235,7 @@ import android.content.pm.parsing.component.ParsedInstrumentation;
import android.content.pm.parsing.component.ParsedIntentInfo;
import android.content.pm.parsing.component.ParsedMainComponent;
import android.content.pm.parsing.component.ParsedPermission;
import android.content.pm.parsing.component.ParsedPermissionGroup;
import android.content.pm.parsing.component.ParsedProcess;
import android.content.pm.parsing.component.ParsedProvider;
import android.content.pm.parsing.component.ParsedService;
@@ -17527,6 +17529,49 @@ public class PackageManagerService extends IPackageManager.Stub
}
}
private boolean doesSignatureMatchForPermissions(@NonNull String sourcePackageName,
@NonNull ParsedPackage parsedPackage, int scanFlags) {
// If the defining package is signed with our cert, it's okay. This
// also includes the "updating the same package" case, of course.
// "updating same package" could also involve key-rotation.
final PackageSetting sourcePackageSetting;
synchronized (mLock) {
sourcePackageSetting = mSettings.getPackageLPr(sourcePackageName);
}
final SigningDetails sourceSigningDetails = (sourcePackageSetting == null
? SigningDetails.UNKNOWN : sourcePackageSetting.getSigningDetails());
final KeySetManagerService ksms = mSettings.mKeySetManagerService;
if (sourcePackageName.equals(parsedPackage.getPackageName())
&& (ksms.shouldCheckUpgradeKeySetLocked(
sourcePackageSetting, scanFlags))) {
return ksms.checkUpgradeKeySetLocked(sourcePackageSetting, parsedPackage);
} else {
// in the event of signing certificate rotation, we need to see if the
// package's certificate has rotated from the current one, or if it is an
// older certificate with which the current is ok with sharing permissions
if (sourceSigningDetails.checkCapability(
parsedPackage.getSigningDetails(),
PackageParser.SigningDetails.CertCapabilities.PERMISSION)) {
return true;
} else if (parsedPackage.getSigningDetails().checkCapability(
sourceSigningDetails,
PackageParser.SigningDetails.CertCapabilities.PERMISSION)) {
// the scanned package checks out, has signing certificate rotation
// history, and is newer; bring it over
synchronized (mLock) {
sourcePackageSetting.signatures.mSigningDetails =
parsedPackage.getSigningDetails();
}
return true;
} else {
return false;
}
}
}
@GuardedBy("mInstallLock")
private PrepareResult preparePackageLI(InstallArgs args, PackageInstalledInfo res)
throws PrepareFailure {
@@ -17685,6 +17730,15 @@ public class PackageManagerService extends IPackageManager.Stub
}
}
/*
* Cannot properly check CANNOT_INSTALL_WITH_BAD_PERMISSION_GROUPS using CompatChanges
* as this only works for packages that are installed
*
* TODO: Move logic for permission group compatibility into PermissionManagerService
*/
boolean cannotInstallWithBadPermissionGroups =
parsedPackage.getTargetSdkVersion() >= Build.VERSION_CODES.S;
PackageSetting ps = mSettings.mPackages.get(pkgName);
if (ps != null) {
if (DEBUG_INSTALL) Slog.d(TAG, "Existing package: " + ps);
@@ -17736,8 +17790,33 @@ public class PackageManagerService extends IPackageManager.Stub
res.origUsers = ps.queryInstalledUsers(mUserManager.getUserIds(), true);
}
final int numGroups = ArrayUtils.size(parsedPackage.getPermissionGroups());
for (int groupNum = 0; groupNum < numGroups; groupNum++) {
final ParsedPermissionGroup group =
parsedPackage.getPermissionGroups().get(groupNum);
final PermissionGroupInfo sourceGroup = getPermissionGroupInfo(group.getName(), 0);
int N = ArrayUtils.size(parsedPackage.getPermissions());
if (sourceGroup != null && cannotInstallWithBadPermissionGroups) {
final String sourcePackageName = sourceGroup.packageName;
if ((replace || !parsedPackage.getPackageName().equals(sourcePackageName))
&& !doesSignatureMatchForPermissions(sourcePackageName, parsedPackage,
scanFlags)) {
EventLog.writeEvent(0x534e4554, "146211400", -1,
parsedPackage.getPackageName());
throw new PrepareFailure(INSTALL_FAILED_DUPLICATE_PERMISSION_GROUP,
"Package "
+ parsedPackage.getPackageName()
+ " attempting to redeclare permission group "
+ group.getName() + " already owned by "
+ sourcePackageName);
}
}
}
// TODO: Move logic for checking permission compatibility into PermissionManagerService
final int N = ArrayUtils.size(parsedPackage.getPermissions());
for (int i = N - 1; i >= 0; i--) {
final ParsedPermission perm = parsedPackage.getPermissions().get(i);
final BasePermission bp = mPermissionManager.getPermissionTEMP(perm.getName());
@@ -17753,46 +17832,10 @@ public class PackageManagerService extends IPackageManager.Stub
// Check whether the newly-scanned package wants to define an already-defined perm
if (bp != null) {
// If the defining package is signed with our cert, it's okay. This
// also includes the "updating the same package" case, of course.
// "updating same package" could also involve key-rotation.
final boolean sigsOk;
final String sourcePackageName = bp.getPackageName();
final PackageSetting sourcePackageSetting;
synchronized (mLock) {
sourcePackageSetting = mSettings.getPackageLPr(sourcePackageName);
}
final SigningDetails sourceSigningDetails = (sourcePackageSetting == null
? SigningDetails.UNKNOWN : sourcePackageSetting.getSigningDetails());
final KeySetManagerService ksms = mSettings.mKeySetManagerService;
if (sourcePackageName.equals(parsedPackage.getPackageName())
&& (ksms.shouldCheckUpgradeKeySetLocked(
sourcePackageSetting, scanFlags))) {
sigsOk = ksms.checkUpgradeKeySetLocked(sourcePackageSetting, parsedPackage);
} else {
// in the event of signing certificate rotation, we need to see if the
// package's certificate has rotated from the current one, or if it is an
// older certificate with which the current is ok with sharing permissions
if (sourceSigningDetails.checkCapability(
parsedPackage.getSigningDetails(),
PackageParser.SigningDetails.CertCapabilities.PERMISSION)) {
sigsOk = true;
} else if (parsedPackage.getSigningDetails().checkCapability(
sourceSigningDetails,
PackageParser.SigningDetails.CertCapabilities.PERMISSION)) {
// the scanned package checks out, has signing certificate rotation
// history, and is newer; bring it over
synchronized (mLock) {
sourcePackageSetting.signatures.mSigningDetails =
parsedPackage.getSigningDetails();
}
sigsOk = true;
} else {
sigsOk = false;
}
}
if (!sigsOk) {
if (!doesSignatureMatchForPermissions(sourcePackageName, parsedPackage,
scanFlags)) {
// If the owning package is the system itself, we log but allow
// install to proceed; we fail the install on all other permission
// redefinitions.
@@ -17827,6 +17870,52 @@ public class PackageManagerService extends IPackageManager.Stub
}
}
}
if (perm.getGroup() != null && cannotInstallWithBadPermissionGroups) {
boolean isPermGroupDefinedByPackage = false;
for (int groupNum = 0; groupNum < numGroups; groupNum++) {
if (parsedPackage.getPermissionGroups().get(groupNum).getName()
.equals(perm.getGroup())) {
isPermGroupDefinedByPackage = true;
break;
}
}
if (!isPermGroupDefinedByPackage) {
final PermissionGroupInfo sourceGroup =
getPermissionGroupInfo(perm.getGroup(), 0);
if (sourceGroup == null) {
EventLog.writeEvent(0x534e4554, "146211400", -1,
parsedPackage.getPackageName());
throw new PrepareFailure(INSTALL_FAILED_BAD_PERMISSION_GROUP,
"Package "
+ parsedPackage.getPackageName()
+ " attempting to declare permission "
+ perm.getName() + " in non-existing group "
+ perm.getGroup());
} else {
String groupSourcePackageName = sourceGroup.packageName;
if (!PLATFORM_PACKAGE_NAME.equals(groupSourcePackageName)
&& !doesSignatureMatchForPermissions(groupSourcePackageName,
parsedPackage, scanFlags)) {
EventLog.writeEvent(0x534e4554, "146211400", -1,
parsedPackage.getPackageName());
throw new PrepareFailure(INSTALL_FAILED_BAD_PERMISSION_GROUP,
"Package "
+ parsedPackage.getPackageName()
+ " attempting to declare permission "
+ perm.getName() + " in group "
+ perm.getGroup() + " owned by package "
+ groupSourcePackageName
+ " with incompatible certificate");
}
}
}
}
}
}