From b37e1cd82fcaa7058e9fdf34749fcd19a7e2b2b4 Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Thu, 26 Apr 2018 18:24:01 -0700 Subject: [PATCH 1/2] Revert "Allow shared users to rotate signing certs in an OTA" This reverts commit ffd979d6b90a2780e8d625f7c65e5a8680ceb6ba. Reason for revert: Replacing with go/oag/673735 Bug: 74501739 Test: N/A Change-Id: I9e87b0f815081a196218744653542a29939c82bb --- .../android/server/pm/PackageManagerService.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index bb1f5c02f8643..74aabc20a7307 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -10198,10 +10198,20 @@ public class PackageManagerService extends IPackageManager.Stub // The signature has changed, but this package is in the system // image... let's recover! pkgSetting.signatures.mSigningDetails = pkg.mSigningDetails; - // If the system app is part of a shared user we allow that shared user to change - // signatures as well in part as part of an OTA. + // However... if this package is part of a shared user, but it + // doesn't match the signature of the shared user, let's fail. + // What this means is that you can't change the signatures + // associated with an overall shared user, which doesn't seem all + // that unreasonable. if (signatureCheckPs.sharedUser != null) { - signatureCheckPs.sharedUser.signatures.mSigningDetails = pkg.mSigningDetails; + if (compareSignatures( + signatureCheckPs.sharedUser.signatures.mSigningDetails.signatures, + pkg.mSigningDetails.signatures) != PackageManager.SIGNATURE_MATCH) { + throw new PackageManagerException( + INSTALL_PARSE_FAILED_INCONSISTENT_CERTIFICATES, + "Signature mismatch for shared user: " + + pkgSetting.sharedUser); + } } // File a report about this. String msg = "System package " + pkg.packageName From ad8a0da6a7b92e244953460068aebd4b40a5614e Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Sun, 6 May 2018 13:36:17 -0700 Subject: [PATCH 2/2] Allow changing signing cert for system apps that use shared users This allows changing the signing certificate of system apps that use shared users across an OTA, but still verifies that all signatures within a given boot are consistent for a given shared user. Bug: 72837107 Bug: 77973716 Bug: 74501739 Test: Flashed dev-keys build into slot b and (local) release-keys build into slot a, booted into b, then successfully booted into a without wiping userdata. Test: Same as above, but treated com.android.mtp (uses android.media UID) as PRESIGNED during release signing so it kept dev-keys. Verified it wasn't scanned first and that only that package was rejected. Test: Same as above, but treated com.android.providers.downloads (uses android.media UID) as PRESIGNED during release signing so it kept dev-keys. Verified it was scanned first (out of APKs using this UID) and that only this package was accepted. Verified other APKs using android.media UID that were scanned after were rejected. Change-Id: I2076e8358cae22e45a24aa4c32a1b8dd446679ca --- .../server/pm/PackageManagerService.java | 30 ++++++++++++------- .../android/server/pm/SharedUserSetting.java | 1 + 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 74aabc20a7307..9b4c8017c2c4f 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -10186,10 +10186,14 @@ public class PackageManagerService extends IPackageManager.Stub // if this is is a sharedUser, check to see if the new package is signed by a newer // signing certificate than the existing one, and if so, copy over the new details - if (signatureCheckPs.sharedUser != null - && pkg.mSigningDetails.hasAncestor( + if (signatureCheckPs.sharedUser != null) { + if (pkg.mSigningDetails.hasAncestor( signatureCheckPs.sharedUser.signatures.mSigningDetails)) { - signatureCheckPs.sharedUser.signatures.mSigningDetails = pkg.mSigningDetails; + signatureCheckPs.sharedUser.signatures.mSigningDetails = pkg.mSigningDetails; + } + if (signatureCheckPs.sharedUser.signaturesChanged == null) { + signatureCheckPs.sharedUser.signaturesChanged = Boolean.FALSE; + } } } catch (PackageManagerException e) { if ((parseFlags & PackageParser.PARSE_IS_SYSTEM_DIR) == 0) { @@ -10198,20 +10202,24 @@ public class PackageManagerService extends IPackageManager.Stub // The signature has changed, but this package is in the system // image... let's recover! pkgSetting.signatures.mSigningDetails = pkg.mSigningDetails; - // However... if this package is part of a shared user, but it - // doesn't match the signature of the shared user, let's fail. - // What this means is that you can't change the signatures - // associated with an overall shared user, which doesn't seem all - // that unreasonable. + + // If the system app is part of a shared user we allow that shared user to change + // signatures as well as part of an OTA. We still need to verify that the signatures + // are consistent within the shared user for a given boot, so only allow updating + // the signatures on the first package scanned for the shared user (i.e. if the + // signaturesChanged state hasn't been initialized yet in SharedUserSetting). if (signatureCheckPs.sharedUser != null) { - if (compareSignatures( + if (signatureCheckPs.sharedUser.signaturesChanged != null && + compareSignatures( signatureCheckPs.sharedUser.signatures.mSigningDetails.signatures, pkg.mSigningDetails.signatures) != PackageManager.SIGNATURE_MATCH) { throw new PackageManagerException( INSTALL_PARSE_FAILED_INCONSISTENT_CERTIFICATES, - "Signature mismatch for shared user: " - + pkgSetting.sharedUser); + "Signature mismatch for shared user: " + pkgSetting.sharedUser); } + + signatureCheckPs.sharedUser.signatures.mSigningDetails = pkg.mSigningDetails; + signatureCheckPs.sharedUser.signaturesChanged = Boolean.TRUE; } // File a report about this. String msg = "System package " + pkg.packageName diff --git a/services/core/java/com/android/server/pm/SharedUserSetting.java b/services/core/java/com/android/server/pm/SharedUserSetting.java index b6b94f5e3cab2..ca08415f7a777 100644 --- a/services/core/java/com/android/server/pm/SharedUserSetting.java +++ b/services/core/java/com/android/server/pm/SharedUserSetting.java @@ -46,6 +46,7 @@ public final class SharedUserSetting extends SettingBase { final ArraySet packages = new ArraySet(); final PackageSignatures signatures = new PackageSignatures(); + Boolean signaturesChanged; SharedUserSetting(String _name, int _pkgFlags, int _pkgPrivateFlags) { super(_pkgFlags, _pkgPrivateFlags);