From 6b955f6c9f9cb4e612c8d7587280ef1461c8ee85 Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Mon, 10 Jul 2023 09:49:47 +0000 Subject: [PATCH] Revert "Revert "Update signature permissions when package signing is changed."" This reverts commit 2eaf8488392807b00fa7e492661e94a1acc2b20c. Reason for revert: Fixed SigningDetails.equals() NPE in b/290558495 Fixes: 288515966 Test: KeySetHostTest#testUpgradeDefinerSigPerm{Gained,Lost} Test: Reproduction steps in b/290558495 Change-Id: I694a565957576633ba7e1424c49a77d97fcdff26 --- core/java/android/content/pm/SigningDetails.java | 10 ++++++---- .../access/permission/AppIdPermissionPolicy.kt | 11 +++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/core/java/android/content/pm/SigningDetails.java b/core/java/android/content/pm/SigningDetails.java index 1e659b74db77b..af2649f3e4dfc 100644 --- a/core/java/android/content/pm/SigningDetails.java +++ b/core/java/android/content/pm/SigningDetails.java @@ -867,10 +867,12 @@ public final class SigningDetails implements Parcelable { return false; } // The capabilities for the past signing certs must match as well. - for (int i = 0; i < mPastSigningCertificates.length; i++) { - if (mPastSigningCertificates[i].getFlags() - != that.mPastSigningCertificates[i].getFlags()) { - return false; + if (mPastSigningCertificates != null) { + for (int i = 0; i < mPastSigningCertificates.length; i++) { + if (mPastSigningCertificates[i].getFlags() + != that.mPastSigningCertificates[i].getFlags()) { + return false; + } } } return true; diff --git a/services/permission/java/com/android/server/permission/access/permission/AppIdPermissionPolicy.kt b/services/permission/java/com/android/server/permission/access/permission/AppIdPermissionPolicy.kt index d39c8a0a11020..aa86cd6323b90 100644 --- a/services/permission/java/com/android/server/permission/access/permission/AppIdPermissionPolicy.kt +++ b/services/permission/java/com/android/server/permission/access/permission/AppIdPermissionPolicy.kt @@ -390,7 +390,14 @@ class AppIdPermissionPolicy : SchemePolicy() { packageState: PackageState, changedPermissionNames: MutableIndexedSet ) { - packageState.androidPackage!!.permissions.forEachIndexed { _, parsedPermission -> + val androidPackage = packageState.androidPackage!! + // This may not be the same package as the old permission because the old permission owner + // can be different, hence using this somewhat strange name to prevent misuse. + val oldNewPackage = oldState.externalState.packageStates[packageState.packageName] + ?.androidPackage + val isPackageSigningChanged = oldNewPackage != null && + androidPackage.signingDetails != oldNewPackage.signingDetails + androidPackage.permissions.forEachIndexed { _, parsedPermission -> val newPermissionInfo = PackageInfoUtils.generatePermissionInfo( parsedPermission, PackageManager.GET_META_DATA.toLong() )!! @@ -520,7 +527,7 @@ class AppIdPermissionPolicy : SchemePolicy() { newPackageName != oldPermission.packageName || newPermission.protectionLevel != oldPermission.protectionLevel || ( oldPermission.isReconciled && ( - ( + (newPermission.isSignature && isPackageSigningChanged) || ( newPermission.isKnownSigner && newPermission.knownCerts != oldPermission.knownCerts ) || (