From d68b87cdd402d46013170d9316a31c82be4e4816 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 12 Nov 2014 12:18:11 -0800 Subject: [PATCH] Recover apps with malformed certificates. There was a window of time in Lollipop where we persisted certificates after they had passed through a decode/encode cycle. The well-written OpenSSL library was liberal when decoding (allowing slightly malformed certs to be parsed), but then strict when encoding, giving us different bytes for effectively the same certificate. A related libcore change (0c990ab4a90b8a5492a67b2b728ac9a4a1ccfa1b) now returns the original bytes verbatim, fixing both pre-Lollipop installs and installs after that change. This change recovers any apps that had been installed during the window of time described above by doing a one-time check to see if the certs are effectively equal. Bug: 18228011 Change-Id: Ib82bd6db718d0490d7a26c9c1014b7c8457a7f2d --- core/java/android/content/pm/Signature.java | 51 +++++++++++++++++++ .../src/android/content/pm/SignatureTest.java | 57 ++++++++++++++++++++++ .../android/server/pm/PackageManagerService.java | 49 ++++++++++++++++++- .../core/java/com/android/server/pm/Settings.java | 10 +++- 4 files changed, 164 insertions(+), 3 deletions(-) create mode 100644 core/tests/coretests/src/android/content/pm/SignatureTest.java diff --git a/core/java/android/content/pm/Signature.java b/core/java/android/content/pm/Signature.java index 7edf4b984f9e..fdc54aedac96 100644 --- a/core/java/android/content/pm/Signature.java +++ b/core/java/android/content/pm/Signature.java @@ -22,12 +22,14 @@ import android.os.Parcelable; import com.android.internal.util.ArrayUtils; import java.io.ByteArrayInputStream; +import java.io.InputStream; import java.lang.ref.SoftReference; import java.security.PublicKey; import java.security.cert.Certificate; import java.security.cert.CertificateEncodingException; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; +import java.security.cert.X509Certificate; import java.util.Arrays; /** @@ -252,4 +254,53 @@ public class Signature implements Parcelable { return (a.length == b.length) && ArrayUtils.containsAll(a, b) && ArrayUtils.containsAll(b, a); } + + /** + * Test if given {@link Signature} sets are effectively equal. In rare + * cases, certificates can have slightly malformed encoding which causes + * exact-byte checks to fail. + *

+ * To identify effective equality, we bounce the certificates through an + * decode/encode pass before doing the exact-byte check. To reduce attack + * surface area, we only allow a byte size delta of a few bytes. + * + * @throws CertificateException if the before/after length differs + * substantially, usually a signal of something fishy going on. + * @hide + */ + public static boolean areEffectiveMatch(Signature[] a, Signature[] b) + throws CertificateException { + final CertificateFactory cf = CertificateFactory.getInstance("X.509"); + + final Signature[] aPrime = new Signature[a.length]; + for (int i = 0; i < a.length; i++) { + aPrime[i] = bounce(cf, a[i]); + } + final Signature[] bPrime = new Signature[b.length]; + for (int i = 0; i < b.length; i++) { + bPrime[i] = bounce(cf, b[i]); + } + + return areExactMatch(aPrime, bPrime); + } + + /** + * Bounce the given {@link Signature} through a decode/encode cycle. + * + * @throws CertificateException if the before/after length differs + * substantially, usually a signal of something fishy going on. + * @hide + */ + public static Signature bounce(CertificateFactory cf, Signature s) throws CertificateException { + final InputStream is = new ByteArrayInputStream(s.mSignature); + final X509Certificate cert = (X509Certificate) cf.generateCertificate(is); + final Signature sPrime = new Signature(cert.getEncoded()); + + if (Math.abs(sPrime.mSignature.length - s.mSignature.length) > 2) { + throw new CertificateException("Bounced cert length looks fishy; before " + + s.mSignature.length + ", after " + sPrime.mSignature.length); + } + + return sPrime; + } } diff --git a/core/tests/coretests/src/android/content/pm/SignatureTest.java b/core/tests/coretests/src/android/content/pm/SignatureTest.java new file mode 100644 index 000000000000..89d599742e90 --- /dev/null +++ b/core/tests/coretests/src/android/content/pm/SignatureTest.java @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2014 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.content.pm; + +import junit.framework.TestCase; + +public class SignatureTest extends TestCase { + + /** Cert A with valid syntax */ + private static final Signature A = new Signature("308201D33082013CA0030201020219373565373461363A31336534333439623635343A2D38303030300D06092A864886F70D01010505003017311530130603550403130C6269736F6E416E64726F6964301E170D3133303432343232323134345A170D3338303432353232323134345A3017311530130603550403130C6269736F6E416E64726F696430819F300D06092A864886F70D010101050003818D00308189028181009214CE08563B77FF3128D3A303254287301263A842D19D5D4EAF024EBEDF864F3802C215B2F3EA85432F3EFF1DB8F591B0854FA7C1C6E4A8A85132FA762CC2D12A8EBD34D8B15C241A91716577F03BB3D2AFFC24367AB1E5E03C387891E34E646E47FAD75B178C1FD077B9199B3ABA6D48E2464801F6592E98245124046E51A90203010001A317301530130603551D25040C300A06082B06010505070303300D06092A864886F70D0101050500038181000B71581EDDC20E8C18C1C140BEE72501A97E04CA12030C51D4C38767B6A9FB5155CF4858C565EF77E5E2C22687C1AAB04BBA2B81C9A73CFB8DE118B624094AAE43D8FC2D585D90839DAFA5033AF7B8C0DE27E6ADAE44C40508CE493E9C80F1F5DA9EC87ECA1844BAB12C83CC8EB5937E1BE36A42CD22086A826E00FB763CD577"); + /** Cert A with malformed syntax */ + private static final Signature M = new Signature("308201D43082013CA0030201020219373565373461363A31336534333439623635343A2D38303030300D06092A864886F70D01010505003017311530130603550403130C6269736F6E416E64726F6964301E170D3133303432343232323134345A170D3338303432353232323134345A3017311530130603550403130C6269736F6E416E64726F696430819F300D06092A864886F70D010101050003818D00308189028181009214CE08563B77FF3128D3A303254287301263A842D19D5D4EAF024EBEDF864F3802C215B2F3EA85432F3EFF1DB8F591B0854FA7C1C6E4A8A85132FA762CC2D12A8EBD34D8B15C241A91716577F03BB3D2AFFC24367AB1E5E03C387891E34E646E47FAD75B178C1FD077B9199B3ABA6D48E2464801F6592E98245124046E51A90203010001A317301530130603551D25040C300A06082B06010505070303300D06092A864886F70D010105050003820081000B71581EDDC20E8C18C1C140BEE72501A97E04CA12030C51D4C38767B6A9FB5155CF4858C565EF77E5E2C22687C1AAB04BBA2B81C9A73CFB8DE118B624094AAE43D8FC2D585D90839DAFA5033AF7B8C0DE27E6ADAE44C40508CE493E9C80F1F5DA9EC87ECA1844BAB12C83CC8EB5937E1BE36A42CD22086A826E00FB763CD577"); + /** Cert B with valid syntax */ + private static final Signature B = new Signature("308204a830820390a003020102020900a1573d0f45bea193300d06092a864886f70d0101050500308194310b3009060355040613025553311330110603550408130a43616c69666f726e6961311630140603550407130d4d6f756e7461696e20566965773110300e060355040a1307416e64726f69643110300e060355040b1307416e64726f69643110300e06035504031307416e64726f69643122302006092a864886f70d0109011613616e64726f696440616e64726f69642e636f6d301e170d3131303931393138343232355a170d3339303230343138343232355a308194310b3009060355040613025553311330110603550408130a43616c69666f726e6961311630140603550407130d4d6f756e7461696e20566965773110300e060355040a1307416e64726f69643110300e060355040b1307416e64726f69643110300e06035504031307416e64726f69643122302006092a864886f70d0109011613616e64726f696440616e64726f69642e636f6d30820120300d06092a864886f70d01010105000382010d00308201080282010100de1b51336afc909d8bcca5920fcdc8940578ec5c253898930e985481cfdea75ba6fc54b1f7bb492a03d98db471ab4200103a8314e60ee25fef6c8b83bc1b2b45b084874cffef148fa2001bb25c672b6beba50b7ac026b546da762ea223829a22b80ef286131f059d2c9b4ca71d54e515a8a3fd6bf5f12a2493dfc2619b337b032a7cf8bbd34b833f2b93aeab3d325549a93272093943bb59dfc0197ae4861ff514e019b73f5cf10023ad1a032adb4b9bbaeb4debecb4941d6a02381f1165e1ac884c1fca9525c5854dce2ad8ec839b8ce78442c16367efc07778a337d3ca2cdf9792ac722b95d67c345f1c00976ec372f02bfcbef0262cc512a6845e71cfea0d020103a381fc3081f9301d0603551d0e0416041478a0fc4517fb70ff52210df33c8d32290a44b2bb3081c90603551d230481c13081be801478a0fc4517fb70ff52210df33c8d32290a44b2bba1819aa48197308194310b3009060355040613025553311330110603550408130a43616c69666f726e6961311630140603550407130d4d6f756e7461696e20566965773110300e060355040a1307416e64726f69643110300e060355040b1307416e64726f69643110300e06035504031307416e64726f69643122302006092a864886f70d0109011613616e64726f696440616e64726f69642e636f6d820900a1573d0f45bea193300c0603551d13040530030101ff300d06092a864886f70d01010505000382010100977302dfbf668d7c61841c9c78d2563bcda1b199e95e6275a799939981416909722713531157f3cdcfea94eea7bb79ca3ca972bd8058a36ad1919291df42d7190678d4ea47a4b9552c9dfb260e6d0d9129b44615cd641c1080580e8a990dd768c6ab500c3b964e185874e4105109d94c5bd8c405deb3cf0f7960a563bfab58169a956372167a7e2674a04c4f80015d8f7869a7a4139aecbbdca2abc294144ee01e4109f0e47a518363cf6e9bf41f7560e94bdd4a5d085234796b05c7a1389adfd489feec2a107955129d7991daa49afb3d327dc0dc4fe959789372b093a89c8dbfa41554f771c18015a6cb242a17e04d19d55d3b4664eae12caf2a11cd2b836e"); + + public void testExactlyEqual() throws Exception { + assertTrue(Signature.areExactMatch(asArray(A), asArray(A))); + assertTrue(Signature.areExactMatch(asArray(M), asArray(M))); + + assertFalse(Signature.areExactMatch(asArray(A), asArray(B))); + assertFalse(Signature.areExactMatch(asArray(A), asArray(M))); + assertFalse(Signature.areExactMatch(asArray(M), asArray(A))); + + assertTrue(Signature.areExactMatch(asArray(A, M), asArray(M, A))); + } + + public void testEffectiveMatch() throws Exception { + assertTrue(Signature.areEffectiveMatch(asArray(A), asArray(A))); + assertTrue(Signature.areEffectiveMatch(asArray(M), asArray(M))); + + assertFalse(Signature.areEffectiveMatch(asArray(A), asArray(B))); + assertTrue(Signature.areEffectiveMatch(asArray(A), asArray(M))); + assertTrue(Signature.areEffectiveMatch(asArray(M), asArray(A))); + + assertTrue(Signature.areEffectiveMatch(asArray(A, M), asArray(M, A))); + assertTrue(Signature.areEffectiveMatch(asArray(A, B), asArray(M, B))); + assertFalse(Signature.areEffectiveMatch(asArray(A, M), asArray(A, B))); + } + + private static Signature[] asArray(Signature... s) { + return s; + } +} diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index b79e15756a11..3e1647ea7949 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -2860,6 +2860,38 @@ public class PackageManagerService extends IPackageManager.Stub { return PackageManager.SIGNATURE_NO_MATCH; } + private boolean isRecoverSignatureUpdateNeeded(PackageParser.Package scannedPkg) { + if (isExternal(scannedPkg)) { + return mSettings.isExternalDatabaseVersionOlderThan( + DatabaseVersion.SIGNATURE_MALFORMED_RECOVER); + } else { + return mSettings.isInternalDatabaseVersionOlderThan( + DatabaseVersion.SIGNATURE_MALFORMED_RECOVER); + } + } + + private int compareSignaturesRecover(PackageSignatures existingSigs, + PackageParser.Package scannedPkg) { + if (!isRecoverSignatureUpdateNeeded(scannedPkg)) { + return PackageManager.SIGNATURE_NO_MATCH; + } + + String msg = null; + try { + if (Signature.areEffectiveMatch(existingSigs.mSignatures, scannedPkg.mSignatures)) { + logCriticalInfo(Log.INFO, "Recovered effectively matching certificates for " + + scannedPkg.packageName); + return PackageManager.SIGNATURE_MATCH; + } + } catch (CertificateException e) { + msg = e.getMessage(); + } + + logCriticalInfo(Log.INFO, + "Failed to recover certificates for " + scannedPkg.packageName + ": " + msg); + return PackageManager.SIGNATURE_NO_MATCH; + } + @Override public String[] getPackagesForUid(int uid) { uid = UserHandle.getAppId(uid); @@ -4148,7 +4180,8 @@ public class PackageManagerService extends IPackageManager.Stub { if (ps != null && ps.codePath.equals(srcFile) && ps.timeStamp == srcFile.lastModified() - && !isCompatSignatureUpdateNeeded(pkg)) { + && !isCompatSignatureUpdateNeeded(pkg) + && !isRecoverSignatureUpdateNeeded(pkg)) { long mSigningKeySetId = ps.keySetData.getProperSigningKeySet(); if (ps.signatures.mSignatures != null && ps.signatures.mSignatures.length != 0 @@ -4423,6 +4456,10 @@ public class PackageManagerService extends IPackageManager.Stub { == PackageManager.SIGNATURE_MATCH; } if (!match) { + match = compareSignaturesRecover(pkgSetting.signatures, pkg) + == PackageManager.SIGNATURE_MATCH; + } + if (!match) { throw new PackageManagerException(INSTALL_FAILED_UPDATE_INCOMPATIBLE, "Package " + pkg.packageName + " signatures do not match the " + "previously installed version; ignoring!"); @@ -4439,6 +4476,10 @@ public class PackageManagerService extends IPackageManager.Stub { == PackageManager.SIGNATURE_MATCH; } if (!match) { + match = compareSignaturesRecover(pkgSetting.sharedUser.signatures, pkg) + == PackageManager.SIGNATURE_MATCH; + } + if (!match) { throw new PackageManagerException(INSTALL_FAILED_SHARED_USER_INCOMPATIBLE, "Package " + pkg.packageName + " has no signatures that match those in shared user " @@ -5386,6 +5427,9 @@ public class PackageManagerService extends IPackageManager.Stub { if (!pkgSetting.keySetData.isUsingUpgradeKeySets() || pkgSetting.sharedUser != null) { try { verifySignaturesLP(pkgSetting, pkg); + // We just determined the app is signed correctly, so bring + // over the latest parsed certs. + pkgSetting.signatures.mSignatures = pkg.mSignatures; } catch (PackageManagerException e) { if ((parseFlags & PackageParser.PARSE_IS_SYSTEM_DIR) == 0) { throw e; @@ -5418,7 +5462,8 @@ public class PackageManagerService extends IPackageManager.Stub { + pkg.packageName + " upgrade keys do not match the " + "previously installed version"); } else { - // signatures may have changed as result of upgrade + // We just determined the app is signed correctly, so bring + // over the latest parsed certs. pkgSetting.signatures.mSignatures = pkg.mSignatures; } } diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index 7de56c890f6b..699adeff1870 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -103,7 +103,7 @@ final class Settings { * Note that care should be taken to make sure all database upgrades are * idempotent. */ - private static final int CURRENT_DATABASE_VERSION = DatabaseVersion.SIGNATURE_END_ENTITY; + private static final int CURRENT_DATABASE_VERSION = DatabaseVersion.SIGNATURE_MALFORMED_RECOVER; /** * This class contains constants that can be referred to from upgrade code. @@ -121,6 +121,14 @@ final class Settings { * just the signing certificate. */ public static final int SIGNATURE_END_ENTITY = 2; + + /** + * There was a window of time in + * {@link android.os.Build.VERSION_CODES#LOLLIPOP} where we persisted + * certificates after potentially mutating them. To switch back to the + * original untouched certificates, we need to force a collection pass. + */ + public static final int SIGNATURE_MALFORMED_RECOVER = 3; } private static final boolean DEBUG_STOPPED = false; -- 2.11.0