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

[DO NOT MERGE] Don't drop restricted permissions on upgrade

Browse Source 
Restricted permissions cannot be held until whitelisted. In
a P -> Q upgrade we grandfather all restricted permissions.
However, the whitelisting code runs after the internal update
of permission happens for the first time resulting in a
revocation of the restricted permissions we were about to
grandfather.

The fix is to not deal with restricted permission when updating
the permissions state until the permission controller has run
the grandfathering logic and once the latter happens we do run
the permission update logic again to properly handle the
restricted permissions.

Bug: 138263882

Test: atest CtsPermissionTestCases
      atest CtsPermission2TestCases
      atest CtsAppSecurityHostTestCases:android.appsecurity.cts.PermissionsHostTest
      P -> Q upgrade preserves grandfathered restricted permissions
      P -> Bad Q build -> Q fixes up broken fixed restricted permissions

Change-Id: Iaef80426bf50181df93d1380af1d0855340def8e
This commit is contained in:
Svet Ganov
2019-07-26 17:45:56 -07:00
committed by Svetoslav Ganov
parent 000415be6e
commit 0b41c8940a
6 changed files with 136 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -319,6 +319,8 @@ import com.android.server.pm.permission.PermissionManagerService;
import com.android.server.pm.permission.PermissionManagerServiceInternal;
import com.android.server.pm.permission.PermissionManagerServiceInternal.PermissionCallback;
import com.android.server.pm.permission.PermissionsState;
import com.android.server.policy.PermissionPolicyInternal;
import com.android.server.policy.PermissionPolicyInternal.OnInitializedCallback;
import com.android.server.security.VerityUtils;
import com.android.server.storage.DeviceStorageMonitorInternal;
import com.android.server.wm.ActivityTaskManagerInternal;
@@ -21633,6 +21635,17 @@ public class PackageManagerService extends IPackageManager.Stub
mPermissionManager.updateAllPermissions(
StorageManager.UUID_PRIVATE_INTERNAL, false, mPackages.values(),
mPermissionCallback);
final PermissionPolicyInternal permissionPolicyInternal =
LocalServices.getService(PermissionPolicyInternal.class);
permissionPolicyInternal.setOnInitializedCallback(userId -> {
// The SDK updated case is already handled when we run during the ctor.
synchronized (mPackages) {
mPermissionManager.updateAllPermissions(
StorageManager.UUID_PRIVATE_INTERNAL, false /*sdkUpdated*/,
mPackages.values(), mPermissionCallback);
}
});
}
// Watch for external volumes that come and go over time

16
services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java
View File

@@ -1170,6 +1170,11 @@ public final class DefaultPermissionGrantPolicy {
final int flags = mContext.getPackageManager().getPermissionFlags(
permission, pkg.packageName, user);
// If we are trying to grant as system fixed and already system fixed
// then the system can change the system fixed grant state.
final boolean changingGrantForSystemFixed = systemFixed
&& (flags & PackageManager.FLAG_PERMISSION_SYSTEM_FIXED) != 0;
// Certain flags imply that the permission's current state by the system or
// device/profile owner or the user. In these cases we do not want to clobber the
// current state.
@@ -1177,7 +1182,8 @@ public final class DefaultPermissionGrantPolicy {
// Unless the caller wants to override user choices. The override is
// to make sure we can grant the needed permission to the default
// sms and phone apps after the user chooses this in the UI.
if (!isFixedOrUserSet(flags) || ignoreSystemPackage) {
if (!isFixedOrUserSet(flags) || ignoreSystemPackage
|| changingGrantForSystemFixed) {
// Never clobber policy fixed permissions.
// We must allow the grant of a system-fixed permission because
// system-fixed is sticky, but the permission itself may be revoked.
@@ -1196,6 +1202,14 @@ public final class DefaultPermissionGrantPolicy {
PackageManager.FLAG_PERMISSION_RESTRICTION_SYSTEM_EXEMPT, user);
}
// If the system tries to change a system fixed permission from one fixed
// state to another we need to drop the fixed flag to allow the grant.
if (changingGrantForSystemFixed) {
mContext.getPackageManager().updatePermissionFlags(permission,
pkg.packageName, flags,
flags & ~PackageManager.FLAG_PERMISSION_SYSTEM_FIXED, user);
}
if (pm.checkPermission(permission, pkg.packageName)
!= PackageManager.PERMISSION_GRANTED) {
mContext.getPackageManager()

83
services/core/java/com/android/server/pm/permission/PermissionManagerService.java
View File

@@ -97,6 +97,7 @@ import com.android.server.pm.SharedUserSetting;
import com.android.server.pm.UserManagerService;
import com.android.server.pm.permission.PermissionManagerServiceInternal.PermissionCallback;
import com.android.server.pm.permission.PermissionsState.PermissionState;
import com.android.server.policy.PermissionPolicyInternal;
import com.android.server.policy.SoftRestrictedPermissionPolicy;
import libcore.util.EmptyArray;
@@ -197,6 +198,9 @@ public class PermissionManagerService {
@GuardedBy("mLock")
private boolean mSystemReady;
@GuardedBy("mLock")
private PermissionPolicyInternal mPermissionPolicyInternal;
/**
* For each foreground/background permission the mapping:
* Background permission -> foreground permissions
@@ -1080,6 +1084,13 @@ public class PermissionManagerService {
boolean softRestricted = bp.isSoftRestricted();
for (int userId : currentUserIds) {
// If permission policy is not ready we don't deal with restricted
// permissions as the policy may whitelist some permissions. Once
// the policy is initialized we would re-evaluate permissions.
final boolean permissionPolicyInitialized =
mPermissionPolicyInternal != null
&& mPermissionPolicyInternal.isInitialized(userId);
PermissionState permState = origPermissions
.getRuntimePermissionState(perm, userId);
int flags = permState != null ? permState.getFlags() : 0;
@@ -1094,7 +1105,7 @@ public class PermissionManagerService {
if (appSupportsRuntimePermissions) {
// If hard restricted we don't allow holding it
if (hardRestricted) {
if (permissionPolicyInitialized && hardRestricted) {
if (!restrictionExempt) {
if (permState != null && permState.isGranted()
&& permissionsState.revokeRuntimePermission(
@@ -1107,7 +1118,7 @@ public class PermissionManagerService {
}
}
// If soft restricted we allow holding in a restricted form
} else if (softRestricted) {
} else if (permissionPolicyInitialized && softRestricted) {
// Regardless if granted set the restriction flag as it
// may affect app treatment based on this permission.
if (!restrictionExempt && !restrictionApplied) {
@@ -1126,7 +1137,8 @@ public class PermissionManagerService {
flags &= ~FLAG_PERMISSION_REVOKE_ON_UPGRADE;
wasChanged = true;
// Hard restricted permissions cannot be held.
} else if (!hardRestricted || restrictionExempt) {
} else if (!permissionPolicyInitialized
|| (!hardRestricted || restrictionExempt)) {
if (permState != null && permState.isGranted()) {
if (permissionsState.grantRuntimePermission(bp, userId)
== PERMISSION_OPERATION_FAILURE) {
@@ -1155,33 +1167,28 @@ public class PermissionManagerService {
// If legacy app always grant the permission but if restricted
// and not exempt take a note a restriction should be applied.
if ((hardRestricted || softRestricted)
&& !restrictionExempt && !restrictionApplied) {
if (permissionPolicyInitialized
&& (hardRestricted || softRestricted)
&& !restrictionExempt && !restrictionApplied) {
flags |= FLAG_PERMISSION_APPLY_RESTRICTION;
wasChanged = true;
}
}
// If unrestricted or restriction exempt, don't apply restriction.
if (!(hardRestricted || softRestricted) || restrictionExempt) {
if (restrictionApplied) {
flags &= ~FLAG_PERMISSION_APPLY_RESTRICTION;
// Dropping restriction on a legacy app requires a review.
if (!appSupportsRuntimePermissions) {
flags |= FLAG_PERMISSION_REVIEW_REQUIRED;
if (permissionPolicyInitialized) {
if (!(hardRestricted || softRestricted) || restrictionExempt) {
if (restrictionApplied) {
flags &= ~FLAG_PERMISSION_APPLY_RESTRICTION;
// Dropping restriction on a legacy app implies a review
if (!appSupportsRuntimePermissions) {
flags |= FLAG_PERMISSION_REVIEW_REQUIRED;
}
wasChanged = true;
}
wasChanged = true;
}
}
if (hardRestricted && !restrictionExempt
&& (flags & FLAG_PERMISSION_SYSTEM_FIXED) != 0) {
// Applying a hard restriction implies revoking it. This might
// lead to a system-fixed, revoked permission.
flags &= ~FLAG_PERMISSION_SYSTEM_FIXED;
wasChanged = true;
}
if (wasChanged) {
updatedUserIds = ArrayUtils.appendInt(updatedUserIds, userId);
}
@@ -1216,6 +1223,13 @@ public class PermissionManagerService {
boolean softRestricted = bp.isSoftRestricted();
for (int userId : currentUserIds) {
// If permission policy is not ready we don't deal with restricted
// permissions as the policy may whitelist some permissions. Once
// the policy is initialized we would re-evaluate permissions.
final boolean permissionPolicyInitialized =
mPermissionPolicyInternal != null
&& mPermissionPolicyInternal.isInitialized(userId);
boolean wasChanged = false;
boolean restrictionExempt =
@@ -1226,7 +1240,7 @@ public class PermissionManagerService {
if (appSupportsRuntimePermissions) {
// If hard restricted we don't allow holding it
if (hardRestricted) {
if (permissionPolicyInitialized && hardRestricted) {
if (!restrictionExempt) {
if (permState != null && permState.isGranted()
&& permissionsState.revokeRuntimePermission(
@@ -1239,7 +1253,7 @@ public class PermissionManagerService {
}
}
// If soft restricted we allow holding in a restricted form
} else if (softRestricted) {
} else if (permissionPolicyInitialized && softRestricted) {
// Regardless if granted set the restriction flag as it
// may affect app treatment based on this permission.
if (!restrictionExempt && !restrictionApplied) {
@@ -1258,7 +1272,8 @@ public class PermissionManagerService {
flags &= ~FLAG_PERMISSION_REVOKE_ON_UPGRADE;
wasChanged = true;
// Hard restricted permissions cannot be held.
} else if (!hardRestricted || restrictionExempt) {
} else if (!permissionPolicyInitialized ||
(!hardRestricted || restrictionExempt)) {
if (permissionsState.grantRuntimePermission(bp, userId) !=
PERMISSION_OPERATION_FAILURE) {
wasChanged = true;
@@ -1274,22 +1289,25 @@ public class PermissionManagerService {
// If legacy app always grant the permission but if restricted
// and not exempt take a note a restriction should be applied.
if ((hardRestricted || softRestricted)
&& !restrictionExempt && !restrictionApplied) {
if (permissionPolicyInitialized
&& (hardRestricted || softRestricted)
&& !restrictionExempt && !restrictionApplied) {
flags |= FLAG_PERMISSION_APPLY_RESTRICTION;
wasChanged = true;
}
}
// If unrestricted or restriction exempt, don't apply restriction.
if (!(hardRestricted || softRestricted) || restrictionExempt) {
if (restrictionApplied) {
flags &= ~FLAG_PERMISSION_APPLY_RESTRICTION;
// Dropping restriction on a legacy app requires a review.
if (!appSupportsRuntimePermissions) {
flags |= FLAG_PERMISSION_REVIEW_REQUIRED;
if (permissionPolicyInitialized) {
if (!(hardRestricted || softRestricted) || restrictionExempt) {
if (restrictionApplied) {
flags &= ~FLAG_PERMISSION_APPLY_RESTRICTION;
// Dropping restriction on a legacy app implies a review
if (!appSupportsRuntimePermissions) {
flags |= FLAG_PERMISSION_REVIEW_REQUIRED;
}
wasChanged = true;
}
wasChanged = true;
}
}
@@ -2900,6 +2918,7 @@ public class PermissionManagerService {
}
mPermissionControllerManager = mContext.getSystemService(PermissionControllerManager.class);
mPermissionPolicyInternal = LocalServices.getService(PermissionPolicyInternal.class);
}
private static String getVolumeUuidForPackage(PackageParser.Package pkg) {

27
services/core/java/com/android/server/policy/PermissionPolicyInternal.java
View File

@@ -18,6 +18,7 @@ package com.android.server.policy;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.annotation.UserIdInt;
import android.content.Intent;
/**
@@ -25,6 +26,19 @@ import android.content.Intent;
*/
public abstract class PermissionPolicyInternal {
/**
* Callback for initializing the permission policy service.
*/
public interface OnInitializedCallback {
/**
* Called when initialized for the given user.
*
* @param userId The initialized user.
*/
void onInitialized(@UserIdInt int userId);
}
/**
* Check whether an activity should be started.
*
@@ -36,4 +50,17 @@ public abstract class PermissionPolicyInternal {
*/
public abstract boolean checkStartActivity(@NonNull Intent intent, int callingUid,
@Nullable String callingPackage);
/**
* @return Whether the policy is initialized for a user.
*/
public abstract boolean isInitialized(@UserIdInt int userId);
/**
* Set a callback for users being initialized. If the user is already
* initialized the callback will not be invoked.
*
* @param callback The callback to register.
*/
public abstract void setOnInitializedCallback(@NonNull OnInitializedCallback callback);
}

25
services/core/java/com/android/server/policy/PermissionPolicyService.java
View File

@@ -66,6 +66,7 @@ import com.android.server.LocalServices;
import com.android.server.SystemService;
import com.android.server.pm.permission.PermissionManagerServiceInternal;
import com.android.server.policy.PermissionPolicyInternal.OnInitializedCallback;
import java.util.ArrayList;
import java.util.concurrent.CountDownLatch;
@@ -86,6 +87,10 @@ public final class PermissionPolicyService extends SystemService {
@GuardedBy("mLock")
private final SparseBooleanArray mIsStarted = new SparseBooleanArray();
/** Callbacks for when a user is initialized */
@GuardedBy("mLock")
private OnInitializedCallback mOnInitializedCallback;
/**
* Whether an async {@link #synchronizePackagePermissionsAndAppOpsForUser} is currently
* scheduled for a package/user.
@@ -240,12 +245,20 @@ public final class PermissionPolicyService extends SystemService {
grantOrUpgradeDefaultRuntimePermissionsIfNeeded(userId);
final OnInitializedCallback callback;
synchronized (mLock) {
mIsStarted.put(userId, true);
callback = mOnInitializedCallback;
}
// Force synchronization as permissions might have changed
synchronizePermissionsAndAppOpsForUser(userId);
// Tell observers we are initialized for this user.
if (callback != null) {
callback.onInitialized(userId);
}
}
@Override
@@ -807,6 +820,18 @@ public final class PermissionPolicyService extends SystemService {
return true;
}
@Override
public boolean isInitialized(int userId) {
return isStarted(userId);
}
@Override
public void setOnInitializedCallback(@NonNull OnInitializedCallback callback) {
synchronized (mLock) {
mOnInitializedCallback = callback;
}
}
/**
* Check if the intent action is removed for the calling package (often based on target SDK
* version). If the action is removed, we'll silently cancel the activity launch.

10
services/java/com/android/server/SystemServer.java
View File

@@ -1973,6 +1973,11 @@ public final class SystemServer {
}
traceEnd();
// Permission policy service
traceBeginAndSlog("StartPermissionPolicyService");
mSystemServiceManager.startService(PermissionPolicyService.class);
traceEnd();
traceBeginAndSlog("MakePackageManagerServiceReady");
mPackageManagerService.systemReady();
traceEnd();
@@ -2007,11 +2012,6 @@ public final class SystemServer {
mSystemServiceManager.startBootPhase(SystemService.PHASE_DEVICE_SPECIFIC_SERVICES_READY);
traceEnd();
// Permission policy service
traceBeginAndSlog("StartPermissionPolicyService");
mSystemServiceManager.startService(PermissionPolicyService.class);
traceEnd();
// These are needed to propagate to the runnable below.
final NetworkManagementService networkManagementF = networkManagement;
final NetworkStatsService networkStatsF = networkStats;