From f1744af7eda9c1a17ac1fdafe6b397d211ed3682 Mon Sep 17 00:00:00 2001 From: Michael Groover Date: Fri, 15 May 2020 19:05:48 -0700 Subject: [PATCH] Address edge cases for signing certificate lineages in sharedUids Currently when a package is installed / updated in a sharedUid the signatures for the sharedUid are not updated unless the new package adds a new signer to the lineage; in this case the new lineage is assigned to the sharedUid without consideration for the existing lineage. This leads to the following problems: 1. If the current sharedUid lineage is A -> B and the new package has lineage B -> C then this is used for the sharedUid and A is lost from the lineage. 2. If the new lineage revokes one or more capabilities from a previous signer in the lineage these updated capabilities are ignored unless the lineage added a new signer as well. 3. If the new lineage revokes the sharedUid capability from a previous signing key in the lineage and another app is installed as part of the sharedUid and signed with that key the new app's installation is allowed to proceed. 4. If only a single app is installed as part of a sharedUid, and that app is updated with a rotated key and a lineage that revokes the previous signing key's sharedUid capability the update is blocked. 5. If an app is installed as part of the sharedUid and has a diverged signer in the lineage (ie sharedUid lineage is Y -> A -> B and new app lineage is Z -> A -> B -> C) the installation is allowed and Y is lost from the lineage. Problems 1 and 2 are addressed with the new SigningDetails mergeLineageWith method that merges common signers between two lineages and also updates their capabilities to the most restrictive between the two lineages (capabilities are anded together). Problems 3 is addressed by checking the signatures of each of the packages in the sharedUid for any signed with an ancestor for which the sharedUid capability may have been revoked. Problem 4 is addressed by checking if the package being updated is the only one in the sharedUid; if so the update to the new lineage is allowed to proceed. Problem 5 is addressed by verifying the new app's lineage is the same, a subset, or a superset of the other. Bug: 152046935 Test: atest PkgInstallSignatureVerificationTest Test: atest SigningDetailsTest Test: atest PackageManagerTests Test: atest PackageManagerTest Change-Id: I420c309f522bb47b65ca40ee848024c85cd5804d --- .../android/content/pm/PackageParser.java | 208 ++++++- core/java/android/content/pm/Signature.java | 15 + .../content/pm/SigningDetailsTest.java | 525 +++++++++++++++++- .../server/pm/PackageManagerService.java | 14 +- .../server/pm/PackageManagerServiceUtils.java | 44 +- 5 files changed, 783 insertions(+), 23 deletions(-) diff --git a/core/java/android/content/pm/PackageParser.java b/core/java/android/content/pm/PackageParser.java index 312e98e776362..a6b024ede18f8 100644 --- a/core/java/android/content/pm/PackageParser.java +++ b/core/java/android/content/pm/PackageParser.java @@ -5974,6 +5974,206 @@ public class PackageParser { } } + /** + * Merges the signing lineage of this instance with the lineage in the provided {@code + * otherSigningDetails} when one has the same or an ancestor signer of the other. + * + *

Merging two signing lineages will result in a new {@code SigningDetails} instance + * containing the longest common lineage with the most restrictive capabilities. If the two + * lineages contain the same signers with the same capabilities then the instance on which + * this was invoked is returned without any changes. Similarly if neither instance has a + * lineage, or if neither has the same or an ancestor signer then this instance is returned. + * + * Following are some example results of this method for lineages with signers A, B, C, D: + * - lineage B merged with lineage A -> B returns lineage A -> B. + * - lineage A -> B merged with lineage B -> C returns lineage A -> B -> C + * - lineage A -> B with the {@code PERMISSION} capability revoked for A merged with + * lineage A -> B with the {@code SHARED_USER_ID} capability revoked for A returns + * lineage A -> B with both capabilities revoked for A. + * - lineage A -> B -> C merged with lineage A -> B -> D would return the original lineage + * A -> B -> C since the current signer of both instances is not the same or in the + * lineage of the other. + */ + public SigningDetails mergeLineageWith(SigningDetails otherSigningDetails) { + if (!hasPastSigningCertificates()) { + return otherSigningDetails.hasPastSigningCertificates() + && otherSigningDetails.hasAncestorOrSelf(this) ? otherSigningDetails : this; + } + if (!otherSigningDetails.hasPastSigningCertificates()) { + return this; + } + // Use the utility method to determine which SigningDetails instance is the descendant + // and to confirm that the signing lineage does not diverge. + SigningDetails descendantSigningDetails = getDescendantOrSelf(otherSigningDetails); + if (descendantSigningDetails == null) { + return this; + } + return descendantSigningDetails == this ? mergeLineageWithAncestorOrSelf( + otherSigningDetails) : otherSigningDetails.mergeLineageWithAncestorOrSelf(this); + } + + /** + * Merges the signing lineage of this instance with the lineage of the ancestor (or same) + * signer in the provided {@code otherSigningDetails}. + */ + private SigningDetails mergeLineageWithAncestorOrSelf(SigningDetails otherSigningDetails) { + // This method should only be called with instances that contain lineages. + int index = pastSigningCertificates.length - 1; + int otherIndex = otherSigningDetails.pastSigningCertificates.length - 1; + if (index < 0 || otherIndex < 0) { + return this; + } + + List mergedSignatures = new ArrayList<>(); + boolean capabilitiesModified = false; + // If this is a descendant lineage then add all of the descendant signer(s) to the + // merged lineage until the ancestor signer is reached. + while (index >= 0 && !pastSigningCertificates[index].equals( + otherSigningDetails.pastSigningCertificates[otherIndex])) { + mergedSignatures.add(new Signature(pastSigningCertificates[index--])); + } + // If the signing lineage was exhausted then the provided ancestor is not actually an + // ancestor of this lineage. + if (index < 0) { + return this; + } + + do { + // Add the common signer to the merged lineage with the most restrictive + // capabilities of the two lineages. + Signature signature = pastSigningCertificates[index--]; + Signature ancestorSignature = + otherSigningDetails.pastSigningCertificates[otherIndex--]; + Signature mergedSignature = new Signature(signature); + int mergedCapabilities = signature.getFlags() & ancestorSignature.getFlags(); + if (signature.getFlags() != mergedCapabilities) { + capabilitiesModified = true; + mergedSignature.setFlags(mergedCapabilities); + } + mergedSignatures.add(mergedSignature); + } while (index >= 0 && otherIndex >= 0 && pastSigningCertificates[index].equals( + otherSigningDetails.pastSigningCertificates[otherIndex])); + + // If both lineages still have elements then their lineages have diverged; since this is + // not supported return the invoking instance. + if (index >= 0 && otherIndex >= 0) { + return this; + } + + // Add any remaining elements from either lineage that is not yet exhausted to the + // the merged lineage. + while (otherIndex >= 0) { + mergedSignatures.add(new Signature( + otherSigningDetails.pastSigningCertificates[otherIndex--])); + } + while (index >= 0) { + mergedSignatures.add(new Signature(pastSigningCertificates[index--])); + } + + // if this lineage already contains all the elements in the ancestor and none of the + // capabilities were changed then just return this instance. + if (mergedSignatures.size() == pastSigningCertificates.length + && !capabilitiesModified) { + return this; + } + // Since the signatures were added to the merged lineage from newest to oldest reverse + // the list to ensure the oldest signer is at index 0. + Collections.reverse(mergedSignatures); + try { + return new SigningDetails(new Signature[]{new Signature(signatures[0])}, + signatureSchemeVersion, mergedSignatures.toArray(new Signature[0])); + } catch (CertificateException e) { + Slog.e(TAG, "Caught an exception creating the merged lineage: ", e); + return this; + } + } + + /** + * Returns whether this and the provided {@code otherSigningDetails} share a common + * ancestor. + * + *

The two SigningDetails have a common ancestor if any of the following conditions are + * met: + * - If neither has a lineage and their current signer(s) are equal. + * - If only one has a lineage and the signer of the other is the same or in the lineage. + * - If both have a lineage and their current signers are the same or one is in the lineage + * of the other, and their lineages do not diverge to different signers. + */ + public boolean hasCommonAncestor(SigningDetails otherSigningDetails) { + if (!hasPastSigningCertificates()) { + // If this instance does not have a lineage then it must either be in the ancestry + // of or the same signer of the otherSigningDetails. + return otherSigningDetails.hasAncestorOrSelf(this); + } + if (!otherSigningDetails.hasPastSigningCertificates()) { + return hasAncestorOrSelf(otherSigningDetails); + } + // If both have a lineage then use getDescendantOrSelf to obtain the descendant signing + // details; a null return from that method indicates there is no common lineage between + // the two or that they diverge at a point in the lineage. + return getDescendantOrSelf(otherSigningDetails) != null; + } + + /** + * Returns the SigningDetails with a descendant (or same) signer after verifying the + * descendant has the same, a superset, or a subset of the lineage of the ancestor. + * + *

If this instance and the provided {@code otherSigningDetails} do not share an + * ancestry, or if their lineages diverge then null is returned to indicate there is no + * valid descendant SigningDetails. + */ + private SigningDetails getDescendantOrSelf(SigningDetails otherSigningDetails) { + SigningDetails descendantSigningDetails; + SigningDetails ancestorSigningDetails; + if (hasAncestorOrSelf(otherSigningDetails)) { + // If the otherSigningDetails has the same signer or a signer in the lineage of this + // instance then treat this instance as the descendant. + descendantSigningDetails = this; + ancestorSigningDetails = otherSigningDetails; + } else if (otherSigningDetails.hasAncestor(this)) { + // The above check confirmed that the two instances do not have the same signer and + // the signer of otherSigningDetails is not in this instance's lineage; if this + // signer is in the otherSigningDetails lineage then treat this as the ancestor. + descendantSigningDetails = otherSigningDetails; + ancestorSigningDetails = this; + } else { + // The signers are not the same and neither has the current signer of the other in + // its lineage; return null to indicate there is no descendant signer. + return null; + } + // Once the descent (or same) signer is identified iterate through the ancestry until + // the current signer of the ancestor is found. + int descendantIndex = descendantSigningDetails.pastSigningCertificates.length - 1; + int ancestorIndex = ancestorSigningDetails.pastSigningCertificates.length - 1; + while (descendantIndex >= 0 + && !descendantSigningDetails.pastSigningCertificates[descendantIndex].equals( + ancestorSigningDetails.pastSigningCertificates[ancestorIndex])) { + descendantIndex--; + } + // Since the ancestry was verified above the descendant lineage should never be + // exhausted, but if for some reason the ancestor signer is not found then return null. + if (descendantIndex < 0) { + return null; + } + // Once the common ancestor (or same) signer is found iterate over the lineage of both + // to ensure that they are either the same or one is a subset of the other. + do { + descendantIndex--; + ancestorIndex--; + } while (descendantIndex >= 0 && ancestorIndex >= 0 + && descendantSigningDetails.pastSigningCertificates[descendantIndex].equals( + ancestorSigningDetails.pastSigningCertificates[ancestorIndex])); + + // If both lineages still have elements then they diverge and cannot be considered a + // valid common lineage. + if (descendantIndex >= 0 && ancestorIndex >= 0) { + return null; + } + // Since one or both of the lineages was exhausted they are either the same or one is a + // subset of the other; return the valid descendant. + return descendantSigningDetails; + } + /** Returns true if the signing details have one or more signatures. */ public boolean hasSignatures() { return signatures != null && signatures.length > 0; @@ -6290,7 +6490,13 @@ public class PackageParser { if (!Arrays.equals(pastSigningCertificates, that.pastSigningCertificates)) { return false; } - + // The capabilities for the past signing certs must match as well. + for (int i = 0; i < pastSigningCertificates.length; i++) { + if (pastSigningCertificates[i].getFlags() + != that.pastSigningCertificates[i].getFlags()) { + return false; + } + } return true; } diff --git a/core/java/android/content/pm/Signature.java b/core/java/android/content/pm/Signature.java index 3b84ae775ad10..b2875e91f80a4 100644 --- a/core/java/android/content/pm/Signature.java +++ b/core/java/android/content/pm/Signature.java @@ -122,6 +122,21 @@ public class Signature implements Parcelable { mSignature = sig; } + /** + * Copy constructor that creates a new instance from the provided {@code other} Signature. + * + * @hide + */ + public Signature(Signature other) { + mSignature = other.mSignature.clone(); + Certificate[] otherCertificateChain = other.mCertificateChain; + if (otherCertificateChain != null && otherCertificateChain.length > 1) { + mCertificateChain = Arrays.copyOfRange(otherCertificateChain, 1, + otherCertificateChain.length); + } + mFlags = other.mFlags; + } + /** * Sets the flags representing the capabilities of the past signing certificate. * @hide diff --git a/core/tests/coretests/src/android/content/pm/SigningDetailsTest.java b/core/tests/coretests/src/android/content/pm/SigningDetailsTest.java index 51af048bcae88..24f45a57dabc6 100644 --- a/core/tests/coretests/src/android/content/pm/SigningDetailsTest.java +++ b/core/tests/coretests/src/android/content/pm/SigningDetailsTest.java @@ -15,13 +15,19 @@ */ package android.content.pm; +import static android.content.pm.PackageParser.SigningDetails.CertCapabilities.AUTH; +import static android.content.pm.PackageParser.SigningDetails.CertCapabilities.INSTALLED_DATA; +import static android.content.pm.PackageParser.SigningDetails.CertCapabilities.PERMISSION; +import static android.content.pm.PackageParser.SigningDetails.CertCapabilities.ROLLBACK; +import static android.content.pm.PackageParser.SigningDetails.CertCapabilities.SHARED_USER_ID; import static android.content.pm.PackageParser.SigningDetails.SignatureSchemeVersion.SIGNING_BLOCK_V3; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import android.content.pm.PackageParser.SigningDetails; -import android.util.ArraySet; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.SmallTest; @@ -29,14 +35,70 @@ import androidx.test.filters.SmallTest; import org.junit.Test; import org.junit.runner.RunWith; -import java.security.PublicKey; - @RunWith(AndroidJUnit4.class) @SmallTest public class SigningDetailsTest { - private static final String FIRST_SIGNATURE = "1234"; - private static final String SECOND_SIGNATURE = "5678"; - private static final String THIRD_SIGNATURE = "9abc"; + private static final int DEFAULT_CAPABILITIES = + INSTALLED_DATA | SHARED_USER_ID | PERMISSION | AUTH; + + // Some of the tests in this class require valid certificate encodings from which to pull the + // public key for the SigningDetails; the following are all DER encoded EC X.509 certificates. + private static final String FIRST_SIGNATURE = + "3082016c30820111a003020102020900ca0fb64dfb66e772300a06082a86" + + "48ce3d04030230123110300e06035504030c0765632d70323536301e170d" + + "3136303333313134353830365a170d3433303831373134353830365a3012" + + "3110300e06035504030c0765632d703235363059301306072a8648ce3d02" + + "0106082a8648ce3d03010703420004a65f113d22cb4913908307ac31ee2b" + + "a0e9138b785fac6536d14ea2ce90d2b4bfe194b50cdc8e169f54a73a991e" + + "f0fa76329825be078cc782740703da44b4d7eba350304e301d0603551d0e" + + "04160414d4133568b95b30158b322071ea8c43ff5b05ccc8301f0603551d" + + "23041830168014d4133568b95b30158b322071ea8c43ff5b05ccc8300c06" + + "03551d13040530030101ff300a06082a8648ce3d04030203490030460221" + + "00f504a0866caef029f417142c5cb71354c79ffcd1d640618dfca4f19e16" + + "db78d6022100f8eea4829799c06cad08c6d3d2d2ec05e0574154e747ea0f" + + "dbb8042cb655aadd"; + private static final String SECOND_SIGNATURE = + "3082016d30820113a0030201020209008855bd1dd2b2b225300a06082a86" + + "48ce3d04030230123110300e06035504030c0765632d70323536301e170d" + + "3138303731333137343135315a170d3238303731303137343135315a3014" + + "3112301006035504030c0965632d703235365f323059301306072a8648ce" + + "3d020106082a8648ce3d030107034200041d4cca0472ad97ee3cecef0da9" + + "3d62b450c6788333b36e7553cde9f74ab5df00bbba6ba950e68461d70bbc" + + "271b62151dad2de2bf6203cd2076801c7a9d4422e1a350304e301d060355" + + "1d0e041604147991d92b0208fc448bf506d4efc9fff428cb5e5f301f0603" + + "551d23041830168014d4133568b95b30158b322071ea8c43ff5b05ccc830" + + "0c0603551d13040530030101ff300a06082a8648ce3d0403020348003045" + + "02202769abb1b49fc2f53479c4ae92a6631dabfd522c9acb0bba2b43ebeb" + + "99c63011022100d260fb1d1f176cf9b7fa60098bfd24319f4905a3e5fda1" + + "00a6fe1a2ab19ff09e"; + private static final String THIRD_SIGNATURE = + "3082016e30820115a0030201020209008394f5cad16a89a7300a06082a86" + + "48ce3d04030230143112301006035504030c0965632d703235365f32301e" + + "170d3138303731343030303532365a170d3238303731313030303532365a" + + "30143112301006035504030c0965632d703235365f333059301306072a86" + + "48ce3d020106082a8648ce3d03010703420004f31e62430e9db6fc5928d9" + + "75fc4e47419bacfcb2e07c89299e6cd7e344dd21adfd308d58cb49a1a2a3" + + "fecacceea4862069f30be1643bcc255040d8089dfb3743a350304e301d06" + + "03551d0e041604146f8d0828b13efaf577fc86b0e99fa3e54bcbcff0301f" + + "0603551d230418301680147991d92b0208fc448bf506d4efc9fff428cb5e" + + "5f300c0603551d13040530030101ff300a06082a8648ce3d040302034700" + + "30440220256bdaa2784c273e4cc291a595a46779dee9de9044dc9f7ab820" + + "309567df9fe902201a4ad8c69891b5a8c47434fe9540ed1f4979b5fad348" + + "3f3fa04d5677355a579e"; + private static final String FOURTH_SIGNATURE = + "3082017b30820120a00302010202146c8cb8a818433c1e6431fb16fb3ae0" + + "fb5ad60aa7300a06082a8648ce3d04030230143112301006035504030c09" + + "65632d703235365f33301e170d3230303531333139313532385a170d3330" + + "303531313139313532385a30143112301006035504030c0965632d703235" + + "365f343059301306072a8648ce3d020106082a8648ce3d03010703420004" + + "db4a60031e79ad49cb759007d6855d4469b91c8bab065434f2fba971ade7" + + "e4d19599a0f67b5e708cfda7543e5630c3769d37e093640d7c768a15144c" + + "d0e5dcf4a350304e301d0603551d0e041604146e78970332554336b6ee89" + + "24eaa70230e393f678301f0603551d230418301680146f8d0828b13efaf5" + + "77fc86b0e99fa3e54bcbcff0300c0603551d13040530030101ff300a0608" + + "2a8648ce3d0403020349003046022100ce786e79ec7547446082e9caf910" + + "614ff80758f9819fb0f148695067abe0fcd4022100a4881e332ddec2116a" + + "d2b59cf891d0f331ff7e27e77b7c6206c7988d9b539330"; @Test public void hasAncestor_multipleSignersInLineageWithAncestor_returnsTrue() throws Exception { @@ -121,27 +183,456 @@ public class SigningDetailsTest { assertFalse(result2); } - private SigningDetails createSigningDetailsWithLineage(String... signers) { + @Test + public void mergeLineageWith_neitherHasLineage_returnsOriginal() throws Exception { + // When attempting to merge two instances of SigningDetails that do not have a lineage the + // initial object should be returned to indicate no changes were made. + SigningDetails noLineageDetails = createSigningDetails(FIRST_SIGNATURE); + SigningDetails otherNoLineageDetails = createSigningDetails(FIRST_SIGNATURE); + + SigningDetails result1 = noLineageDetails.mergeLineageWith(otherNoLineageDetails); + SigningDetails result2 = otherNoLineageDetails.mergeLineageWith(noLineageDetails); + + assertTrue(result1 == noLineageDetails); + assertTrue(result2 == otherNoLineageDetails); + } + + @Test + public void mergeLineageWith_oneHasNoLineage_returnsOther() throws Exception { + // When attempting to merge a SigningDetails with no lineage with another that has a + // lineage and is a descendant the descendant SigningDetails with lineage should be returned + SigningDetails noLineageDetails = createSigningDetails(FIRST_SIGNATURE); + SigningDetails lineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + + SigningDetails result1 = noLineageDetails.mergeLineageWith(lineageDetails); + SigningDetails result2 = lineageDetails.mergeLineageWith(noLineageDetails); + + assertTrue(result1 == lineageDetails); + assertTrue(result2 == lineageDetails); + } + + @Test + public void mergeLineageWith_bothHaveSameLineage_returnsOriginal() throws Exception { + // If twoSigningDetails instances have the exact same lineage with the same capabilities + // then the original instance should be returned without modification. + SigningDetails firstLineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + SigningDetails secondLineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + + SigningDetails result1 = firstLineageDetails.mergeLineageWith(secondLineageDetails); + SigningDetails result2 = secondLineageDetails.mergeLineageWith(firstLineageDetails); + + assertTrue(result1 == firstLineageDetails); + assertTrue(result2 == secondLineageDetails); + } + + @Test + public void mergeLineageWith_oneIsAncestorWithoutLineage_returnsDescendant() throws Exception { + // If one instance without a lineage is an ancestor of the other then the descendant should + // be returned. + SigningDetails ancestorDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE); + SigningDetails descendantDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + + SigningDetails result1 = ancestorDetails.mergeLineageWith(descendantDetails); + SigningDetails result2 = descendantDetails.mergeLineageWith(ancestorDetails); + + assertEquals(descendantDetails, result1); + assertTrue(result2 == descendantDetails); + } + + @Test + public void mergeLineageWith_oneIsAncestorWithLineage_returnsDescendant() throws Exception { + // Similar to the above test if one instance with a lineage is an ancestor of the other then + // the descendant should be returned. + SigningDetails ancestorDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + SigningDetails descendantDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE, THIRD_SIGNATURE); + + SigningDetails result1 = ancestorDetails.mergeLineageWith(descendantDetails); + SigningDetails result2 = descendantDetails.mergeLineageWith(ancestorDetails); + + assertEquals(descendantDetails, result1); + assertTrue(result2 == descendantDetails); + } + + @Test + public void mergeLineageWith_singleSignerInMiddleOfLineage_returnsFullLineage() + throws Exception { + // If one instance without a lineage is an ancestor in the middle of the lineage for the + // descendant the descendant should be returned. + SigningDetails singleSignerDetails = createSigningDetails(SECOND_SIGNATURE); + SigningDetails fullLineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE, THIRD_SIGNATURE); + + SigningDetails result1 = singleSignerDetails.mergeLineageWith(fullLineageDetails); + SigningDetails result2 = fullLineageDetails.mergeLineageWith(singleSignerDetails); + + assertTrue(result1 == fullLineageDetails); + assertTrue(result2 == fullLineageDetails); + } + + @Test + public void mergeLineageWith_noCommonLineage_returnsOriginal() throws Exception { + // While a call should never be made to merge two lineages without a common ancestor if it + // is attempted the original lineage should be returned. + SigningDetails firstLineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + SigningDetails secondLineageDetails = createSigningDetailsWithLineage(THIRD_SIGNATURE, + FOURTH_SIGNATURE); + + SigningDetails result1 = firstLineageDetails.mergeLineageWith(secondLineageDetails); + SigningDetails result2 = secondLineageDetails.mergeLineageWith(firstLineageDetails); + + assertTrue(result1 == firstLineageDetails); + assertTrue(result2 == secondLineageDetails); + } + + @Test + public void mergeLineageWith_bothPartialLineages_returnsFullLineage() throws Exception { + // This test verifies the following scenario: + // - One package is signed with a rotated key B and linage A -> B + // - The other package is signed with a rotated key C and lineage B -> C + // Merging the lineage of these two should return the full lineage A -> B -> C + SigningDetails firstLineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + SigningDetails secondLineageDetails = createSigningDetailsWithLineage(SECOND_SIGNATURE, + THIRD_SIGNATURE); + SigningDetails expectedDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE, THIRD_SIGNATURE); + + SigningDetails result1 = firstLineageDetails.mergeLineageWith(secondLineageDetails); + SigningDetails result2 = secondLineageDetails.mergeLineageWith(firstLineageDetails); + + assertEquals(expectedDetails, result1); + assertEquals(expectedDetails, result2); + } + + @Test + public void mergeLineageWith_oneSubsetLineage_returnsFullLineage() throws Exception { + // This test verifies when one lineage is a subset of the other the full lineage is + // returned. + SigningDetails subsetLineageDetails = createSigningDetailsWithLineage(SECOND_SIGNATURE, + THIRD_SIGNATURE); + SigningDetails fullLineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE, THIRD_SIGNATURE, FOURTH_SIGNATURE); + + SigningDetails result1 = subsetLineageDetails.mergeLineageWith(fullLineageDetails); + SigningDetails result2 = fullLineageDetails.mergeLineageWith(subsetLineageDetails); + + assertEquals(fullLineageDetails, result1); + assertTrue(result2 == fullLineageDetails); + } + + @Test + public void mergeLineageWith_differentRootsOfTrust_returnsOriginal() throws Exception { + // If two SigningDetails share a common lineage but diverge at one of the ancestors then the + // merge should return the invoking instance since this is not supported. + SigningDetails firstLineageDetails = createSigningDetailsWithLineage("1234", + FIRST_SIGNATURE, SECOND_SIGNATURE); + SigningDetails secondLineageDetails = createSigningDetailsWithLineage("5678", + FIRST_SIGNATURE, SECOND_SIGNATURE, THIRD_SIGNATURE); + + SigningDetails result1 = firstLineageDetails.mergeLineageWith(secondLineageDetails); + SigningDetails result2 = secondLineageDetails.mergeLineageWith(firstLineageDetails); + + assertTrue(result1 == firstLineageDetails); + assertTrue(result2 == secondLineageDetails); + } + + @Test + public void mergeLineageWith_divergedSignerInLineage_returnsOriginal() throws Exception { + // Similar to the test above if two lineages diverge at any point then the merge should + // return the original since the signers in a sharedUserId must always be either the same, + // a subset, or a superset of the existing lineage. + SigningDetails firstLineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + "1234", SECOND_SIGNATURE, THIRD_SIGNATURE); + SigningDetails secondLineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + "5678", SECOND_SIGNATURE, THIRD_SIGNATURE); + + SigningDetails result1 = firstLineageDetails.mergeLineageWith(secondLineageDetails); + SigningDetails result2 = secondLineageDetails.mergeLineageWith(firstLineageDetails); + + assertTrue(result1 == firstLineageDetails); + assertTrue(result2 == secondLineageDetails); + } + + @Test + public void mergeLineageWith_sameLineageDifferentCaps_returnsLineageWithModifiedCaps() + throws Exception { + // This test verifies when two lineages consist of the same signers but have different + // capabilities the more restrictive capabilities are returned. + SigningDetails defaultCapabilitiesDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE, THIRD_SIGNATURE); + SigningDetails modifiedCapabilitiesDetails = createSigningDetailsWithLineageAndCapabilities( + new String[]{FIRST_SIGNATURE, SECOND_SIGNATURE, THIRD_SIGNATURE}, + new int[]{INSTALLED_DATA, INSTALLED_DATA, INSTALLED_DATA}); + + SigningDetails result1 = defaultCapabilitiesDetails.mergeLineageWith( + modifiedCapabilitiesDetails); + SigningDetails result2 = modifiedCapabilitiesDetails.mergeLineageWith( + defaultCapabilitiesDetails); + + assertEquals(modifiedCapabilitiesDetails, result1); + assertTrue(result2 == modifiedCapabilitiesDetails); + } + + @Test + public void mergeLineageWith_overlappingLineageDiffCaps_returnsFullLineageWithModifiedCaps() + throws Exception { + // This test verifies the following scenario: + // - First lineage has signers A -> B with modified capabilities for A and B + // - Second lineage has signers B -> C with modified capabilities for B and C + // The merged lineage should be A -> B -> C with the most restrictive capabilities for B + // since it is in both lineages. + int[] firstCapabilities = + new int[]{INSTALLED_DATA | AUTH, INSTALLED_DATA | SHARED_USER_ID | PERMISSION}; + int[] secondCapabilities = new int[]{INSTALLED_DATA | SHARED_USER_ID | AUTH, + INSTALLED_DATA | SHARED_USER_ID | AUTH}; + int[] expectedCapabilities = + new int[]{firstCapabilities[0], firstCapabilities[1] & secondCapabilities[0], + secondCapabilities[1]}; + SigningDetails firstDetails = createSigningDetailsWithLineageAndCapabilities( + new String[]{FIRST_SIGNATURE, SECOND_SIGNATURE}, firstCapabilities); + SigningDetails secondDetails = createSigningDetailsWithLineageAndCapabilities( + new String[]{SECOND_SIGNATURE, THIRD_SIGNATURE}, secondCapabilities); + SigningDetails expectedDetails = createSigningDetailsWithLineageAndCapabilities( + new String[]{FIRST_SIGNATURE, SECOND_SIGNATURE, THIRD_SIGNATURE}, + expectedCapabilities); + + SigningDetails result1 = firstDetails.mergeLineageWith(secondDetails); + SigningDetails result2 = secondDetails.mergeLineageWith(firstDetails); + + assertEquals(expectedDetails, result1); + assertEquals(expectedDetails, result2); + } + + @Test + public void mergeLineageWith_subLineageModifiedCaps_returnsFullLineageWithModifiedCaps() + throws Exception { + // This test verifies the following scenario: + // - First lineage has signers B -> C with modified capabilities + // - Second lineage has signers A -> B -> C -> D with modified capabilities + // The merged lineage should be A -> B -> C -> D with the most restrictive capabilities for + // B and C since they are in both lineages. + int[] subCapabilities = new int[]{INSTALLED_DATA | SHARED_USER_ID | PERMISSION, + DEFAULT_CAPABILITIES | ROLLBACK}; + int[] fullCapabilities = + new int[]{0, SHARED_USER_ID, DEFAULT_CAPABILITIES, DEFAULT_CAPABILITIES}; + int[] expectedCapabilities = + new int[]{fullCapabilities[0], subCapabilities[0] & fullCapabilities[1], + subCapabilities[1] & fullCapabilities[2], fullCapabilities[3]}; + SigningDetails subLineageDetails = createSigningDetailsWithLineageAndCapabilities( + new String[]{SECOND_SIGNATURE, THIRD_SIGNATURE}, subCapabilities); + SigningDetails fullLineageDetails = createSigningDetailsWithLineageAndCapabilities( + new String[]{FIRST_SIGNATURE, SECOND_SIGNATURE, THIRD_SIGNATURE, FOURTH_SIGNATURE}, + fullCapabilities); + SigningDetails expectedDetails = createSigningDetailsWithLineageAndCapabilities( + new String[]{FIRST_SIGNATURE, SECOND_SIGNATURE, THIRD_SIGNATURE, FOURTH_SIGNATURE}, + expectedCapabilities); + + SigningDetails result1 = subLineageDetails.mergeLineageWith(fullLineageDetails); + SigningDetails result2 = fullLineageDetails.mergeLineageWith(subLineageDetails); + + assertEquals(expectedDetails, result1); + assertEquals(expectedDetails, result2); + } + + @Test + public void mergeLineageWith_commonLineageDivergedSigners_returnsOriginal() throws Exception { + // When mergeWithLineage is invoked with SigningDetails instances that have a common lineage + // but diverged signers the calling instance should be returned since the current signer + // is not in the ancestry of the other's lineage. + SigningDetails firstLineageDetails = createSigningDetails(FIRST_SIGNATURE, SECOND_SIGNATURE, + THIRD_SIGNATURE); + SigningDetails secondLineageDetails = createSigningDetails(FIRST_SIGNATURE, + SECOND_SIGNATURE, FOURTH_SIGNATURE); + + SigningDetails result1 = firstLineageDetails.mergeLineageWith(secondLineageDetails); + SigningDetails result2 = secondLineageDetails.mergeLineageWith(firstLineageDetails); + + assertTrue(result1 == firstLineageDetails); + assertTrue(result2 == secondLineageDetails); + } + + @Test + public void hasCommonAncestor_noLineageSameSingleSigner_returnsTrue() throws Exception { + // If neither SigningDetails have a lineage but they have the same single signer then + // hasCommonAncestor should return true. + SigningDetails firstDetails = createSigningDetails(FIRST_SIGNATURE); + SigningDetails secondDetails = createSigningDetails(FIRST_SIGNATURE); + + assertTrue(firstDetails.hasCommonAncestor(secondDetails)); + assertTrue(secondDetails.hasCommonAncestor(firstDetails)); + } + + @Test + public void hasCommonAncestor_noLineageSameMultipleSigners_returnsTrue() throws Exception { + // Similar to above if neither SigningDetails have a lineage but they have the same multiple + // signers then hasCommonAncestor should return true. + SigningDetails firstDetails = createSigningDetails(FIRST_SIGNATURE, SECOND_SIGNATURE); + SigningDetails secondDetails = createSigningDetails(SECOND_SIGNATURE, FIRST_SIGNATURE); + + assertTrue(firstDetails.hasCommonAncestor(secondDetails)); + assertTrue(secondDetails.hasCommonAncestor(firstDetails)); + } + + @Test + public void hasCommonAncestor_noLineageDifferentSigners_returnsFalse() throws Exception { + // If neither SigningDetails have a lineage and they have different signers then + // hasCommonAncestor should return false. + SigningDetails firstDetails = createSigningDetails(FIRST_SIGNATURE); + SigningDetails secondDetails = createSigningDetails(SECOND_SIGNATURE); + SigningDetails thirdDetails = createSigningDetails(FIRST_SIGNATURE, SECOND_SIGNATURE); + SigningDetails fourthDetails = createSigningDetails(SECOND_SIGNATURE, THIRD_SIGNATURE); + + assertFalse(firstDetails.hasCommonAncestor(secondDetails)); + assertFalse(firstDetails.hasCommonAncestor(thirdDetails)); + assertFalse(firstDetails.hasCommonAncestor(fourthDetails)); + assertFalse(secondDetails.hasCommonAncestor(firstDetails)); + assertFalse(secondDetails.hasCommonAncestor(thirdDetails)); + assertFalse(secondDetails.hasCommonAncestor(fourthDetails)); + assertFalse(thirdDetails.hasCommonAncestor(firstDetails)); + assertFalse(thirdDetails.hasCommonAncestor(secondDetails)); + assertFalse(thirdDetails.hasCommonAncestor(fourthDetails)); + assertFalse(fourthDetails.hasCommonAncestor(firstDetails)); + assertFalse(fourthDetails.hasCommonAncestor(secondDetails)); + assertFalse(fourthDetails.hasCommonAncestor(thirdDetails)); + } + + @Test + public void hasCommonAncestor_oneWithOthersSignerInLineage_returnsTrue() throws Exception { + // If only one of the SigningDetails has a lineage and the current signer of the other is in + // the lineage then hasCommonAncestor should return true. + SigningDetails noLineageDetails = createSigningDetails(FIRST_SIGNATURE); + SigningDetails lineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + + assertTrue(noLineageDetails.hasCommonAncestor(lineageDetails)); + assertTrue(lineageDetails.hasCommonAncestor(noLineageDetails)); + } + + @Test + public void hasCommonAncestor_oneWithSameSignerWithoutLineage_returnsTrue() throws Exception { + // If only one of the SigningDetails has a lineage and both have the same current signer + // then hasCommonAncestor should return true. + SigningDetails noLineageDetails = createSigningDetails(SECOND_SIGNATURE); + SigningDetails lineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + + assertTrue(noLineageDetails.hasCommonAncestor(lineageDetails)); + assertTrue(lineageDetails.hasCommonAncestor(noLineageDetails)); + } + + @Test + public void hasCommonAncestor_bothHaveSameLineage_returnsTrue() throws Exception { + // If both SigningDetails have the exact same lineage then hasCommonAncestor should return + // true. + SigningDetails firstDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + SigningDetails secondDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + + assertTrue(firstDetails.hasCommonAncestor(secondDetails)); + assertTrue(secondDetails.hasCommonAncestor(firstDetails)); + } + + @Test + public void hasCommonAncestor_oneLineageIsAncestor_returnsTrue() throws Exception { + // If one SigningDetails has a lineage that is an ancestor of the other then + // hasCommonAncestor should return true. + SigningDetails ancestorDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + SigningDetails descendantDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE, THIRD_SIGNATURE); + + assertTrue(ancestorDetails.hasCommonAncestor(descendantDetails)); + assertTrue(descendantDetails.hasCommonAncestor(ancestorDetails)); + } + + @Test + public void hasCommonAncestor_oneLineageIsSubset_returnsTrue() throws Exception { + // If one SigningDetails has a lineage that is a subset of the other then hasCommonAncestor + // should return true. + SigningDetails subsetDetails = createSigningDetailsWithLineage(SECOND_SIGNATURE, + THIRD_SIGNATURE); + SigningDetails fullDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE, THIRD_SIGNATURE, FOURTH_SIGNATURE); + + assertTrue(subsetDetails.hasCommonAncestor(fullDetails)); + assertTrue(fullDetails.hasCommonAncestor(subsetDetails)); + } + + @Test + public void hasCommonAncestor_differentRootOfTrustInLineage_returnsFalse() throws Exception { + // if the two SigningDetails have a different root of trust then hasCommonAncestor should + // return false. + SigningDetails firstDetails = createSigningDetailsWithLineage(THIRD_SIGNATURE, + FIRST_SIGNATURE, SECOND_SIGNATURE); + SigningDetails secondDetails = createSigningDetailsWithLineage(FOURTH_SIGNATURE, + FIRST_SIGNATURE, SECOND_SIGNATURE); + + assertFalse(firstDetails.hasCommonAncestor(secondDetails)); + assertFalse(secondDetails.hasCommonAncestor(firstDetails)); + } + + @Test + public void hasCommonAncestor_differentSignerInMiddleOfLineage_returnsFalse() throws Exception { + // if the two SigningDetails have a different signer in the middle of a common lineage then + // hasCommonAncestor should return false. + SigningDetails firstDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, "1234", + SECOND_SIGNATURE, THIRD_SIGNATURE); + SigningDetails secondDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, "5678", + SECOND_SIGNATURE, THIRD_SIGNATURE); + + assertFalse(firstDetails.hasCommonAncestor(secondDetails)); + assertFalse(secondDetails.hasCommonAncestor(firstDetails)); + } + + @Test + public void hasCommonAncestor_overlappingLineages_returnsTrue() throws Exception { + // if the two SigningDetails have overlapping lineages then hasCommonAncestor should return + // true. + SigningDetails firstLineageDetails = createSigningDetailsWithLineage(FIRST_SIGNATURE, + SECOND_SIGNATURE); + SigningDetails secondLineageDetails = createSigningDetailsWithLineage(SECOND_SIGNATURE, + THIRD_SIGNATURE); + + assertTrue(firstLineageDetails.hasCommonAncestor(secondLineageDetails)); + assertTrue(secondLineageDetails.hasCommonAncestor(firstLineageDetails)); + } + + private SigningDetails createSigningDetailsWithLineage(String... signers) throws Exception { + int[] capabilities = new int[signers.length]; + for (int i = 0; i < capabilities.length; i++) { + capabilities[i] = DEFAULT_CAPABILITIES; + } + return createSigningDetailsWithLineageAndCapabilities(signers, capabilities); + } + + private SigningDetails createSigningDetailsWithLineageAndCapabilities(String[] signers, + int[] capabilities) throws Exception { + if (capabilities.length != signers.length) { + fail("The capabilities array must contain the same number of elements as the signers " + + "array"); + } Signature[] signingHistory = new Signature[signers.length]; for (int i = 0; i < signers.length; i++) { signingHistory[i] = new Signature(signers[i]); + signingHistory[i].setFlags(capabilities[i]); } Signature[] currentSignature = new Signature[]{signingHistory[signers.length - 1]}; - // TODO: Since the PublicKey ArraySet is not used by any of the tests a generic empty Set - // works for now, but if this is needed in the future consider creating mock PublicKeys that - // can respond as required for the method under test. - ArraySet publicKeys = new ArraySet<>(); - return new SigningDetails(currentSignature, SIGNING_BLOCK_V3, publicKeys, signingHistory); + return new SigningDetails(currentSignature, SIGNING_BLOCK_V3, signingHistory); } - private SigningDetails createSigningDetails(String... signers) { + private SigningDetails createSigningDetails(String... signers) throws Exception { Signature[] currentSignatures = new Signature[signers.length]; for (int i = 0; i < signers.length; i++) { currentSignatures[i] = new Signature(signers[i]); } - // TODO: Similar to above when tests are added that require this it should be updated to use - // mocked PublicKeys. - ArraySet publicKeys = new ArraySet<>(); - return new SigningDetails(currentSignatures, SIGNING_BLOCK_V3, publicKeys, null); + return new SigningDetails(currentSignatures, SIGNING_BLOCK_V3, null); } } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 3e587bf01521f..eb9f599cf9222 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -16265,10 +16265,16 @@ public class PackageManagerService extends IPackageManager.Stub // signing certificate than the existing one, and if so, copy over the new // details if (signatureCheckPs.sharedUser != null) { - if (parsedPackage.getSigningDetails().hasAncestor( - signatureCheckPs.sharedUser.signatures.mSigningDetails)) { - signatureCheckPs.sharedUser.signatures.mSigningDetails = - parsedPackage.getSigningDetails(); + // Attempt to merge the existing lineage for the shared SigningDetails with + // the lineage of the new package; if the shared SigningDetails are not + // returned this indicates the new package added new signers to the lineage + // and/or changed the capabilities of existing signers in the lineage. + SigningDetails sharedSigningDetails = + signatureCheckPs.sharedUser.signatures.mSigningDetails; + SigningDetails mergedDetails = sharedSigningDetails.mergeLineageWith( + signingDetails); + if (mergedDetails != sharedSigningDetails) { + signatureCheckPs.sharedUser.signatures.mSigningDetails = mergedDetails; } if (signatureCheckPs.sharedUser.signaturesChanged == null) { signatureCheckPs.sharedUser.signaturesChanged = Boolean.FALSE; diff --git a/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java b/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java index 5c175a6ef8475..1fce07b0cbf13 100644 --- a/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java +++ b/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java @@ -611,7 +611,6 @@ public class PackageManagerServiceUtils { final String packageName = pkgSetting.name; boolean compatMatch = false; if (pkgSetting.signatures.mSigningDetails.signatures != null) { - // Already existing package. Make sure signatures match boolean match = parsedSignatures.checkCapability( pkgSetting.signatures.mSigningDetails, @@ -664,6 +663,13 @@ public class PackageManagerServiceUtils { || pkgSetting.getSharedUser().signatures.mSigningDetails.checkCapability( parsedSignatures, PackageParser.SigningDetails.CertCapabilities.SHARED_USER_ID); + // Special case: if the sharedUserId capability check failed it could be due to this + // being the only package in the sharedUserId so far and the lineage being updated to + // deny the sharedUserId capability of the previous key in the lineage. + if (!match && pkgSetting.getSharedUser().packages.size() == 1 + && pkgSetting.getSharedUser().packages.valueAt(0).name.equals(packageName)) { + match = true; + } if (!match && compareCompat) { match = matchSignaturesCompat( packageName, pkgSetting.getSharedUser().signatures, parsedSignatures); @@ -686,6 +692,42 @@ public class PackageManagerServiceUtils { + " has no signatures that match those in shared user " + pkgSetting.getSharedUser().name + "; ignoring!"); } + // It is possible that this package contains a lineage that blocks sharedUserId access + // to an already installed package in the sharedUserId signed with a previous key. + // Iterate over all of the packages in the sharedUserId and ensure any that are signed + // with a key in this package's lineage have the SHARED_USER_ID capability granted. + if (parsedSignatures.hasPastSigningCertificates()) { + for (PackageSetting shUidPkgSetting : pkgSetting.getSharedUser().packages) { + // if the current package in the sharedUserId is the package being updated then + // skip this check as the update may revoke the sharedUserId capability from + // the key with which this app was previously signed. + if (packageName.equals(shUidPkgSetting.name)) { + continue; + } + PackageParser.SigningDetails shUidSigningDetails = + shUidPkgSetting.getSigningDetails(); + // The capability check only needs to be performed against the package if it is + // signed with a key that is in the lineage of the package being installed. + if (parsedSignatures.hasAncestor(shUidSigningDetails)) { + if (!parsedSignatures.checkCapability(shUidSigningDetails, + PackageParser.SigningDetails.CertCapabilities.SHARED_USER_ID)) { + throw new PackageManagerException( + INSTALL_FAILED_SHARED_USER_INCOMPATIBLE, + "Package " + packageName + + " revoked the sharedUserId capability from the " + + "signing key used to sign " + shUidPkgSetting.name); + } + } + } + } + // If the lineage of this package diverges from the lineage of the sharedUserId then + // do not allow the installation to proceed. + if (!parsedSignatures.hasCommonAncestor( + pkgSetting.getSharedUser().signatures.mSigningDetails)) { + throw new PackageManagerException(INSTALL_FAILED_SHARED_USER_INCOMPATIBLE, + "Package " + packageName + " has a signing lineage " + + "that diverges from the lineage of the sharedUserId"); + } } return compatMatch; }