From d9d438ac4e851275abb4ddc6671f74701e07b4fc Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Wed, 6 Apr 2016 14:08:14 -0700 Subject: [PATCH] Only parse an APK once During package installation, we were parsing the APK twice; once in the context of the PackageInstaller and once in the context of the PackageManager. Instead, the installer should just pass the certificates to be used further in the process. If the PackageManager doesn't receive certificates [or, if there's an error using them], it will fallback to re-parsing the APK. Bug: 27502465 Change-Id: I94ce551af54eaa9916228e933134debe50867d21 --- .../android/content/pm/PackageParser.java | 56 ++++++++++++++++--- .../server/pm/PackageInstallerSession.java | 5 +- .../server/pm/PackageManagerService.java | 50 +++++++++++------ 3 files changed, 87 insertions(+), 24 deletions(-) diff --git a/core/java/android/content/pm/PackageParser.java b/core/java/android/content/pm/PackageParser.java index fe8db9f6ad37d..aa1e372fe30de 100644 --- a/core/java/android/content/pm/PackageParser.java +++ b/core/java/android/content/pm/PackageParser.java @@ -385,6 +385,7 @@ public class PackageParser { public final int installLocation; public final VerifierInfo[] verifiers; public final Signature[] signatures; + public final Certificate[][] certificates; public final boolean coreApp; public final boolean multiArch; public final boolean use32bitAbi; @@ -392,8 +393,8 @@ public class PackageParser { public ApkLite(String codePath, String packageName, String splitName, int versionCode, int revisionCode, int installLocation, List verifiers, - Signature[] signatures, boolean coreApp, boolean multiArch, boolean use32bitAbi, - boolean extractNativeLibs) { + Signature[] signatures, Certificate[][] certificates, boolean coreApp, + boolean multiArch, boolean use32bitAbi, boolean extractNativeLibs) { this.codePath = codePath; this.packageName = packageName; this.splitName = splitName; @@ -402,6 +403,7 @@ public class PackageParser { this.installLocation = installLocation; this.verifiers = verifiers.toArray(new VerifierInfo[verifiers.size()]); this.signatures = signatures; + this.certificates = certificates; this.coreApp = coreApp; this.multiArch = multiArch; this.use32bitAbi = use32bitAbi; @@ -1073,6 +1075,43 @@ public class PackageParser { return APK_SIGNING_UNKNOWN; } + /** + * Populates the correct packages fields with the given certificates. + *

+ * This is useful when we've already processed the certificates [such as during package + * installation through an installer session]. We don't re-process the archive and + * simply populate the correct fields. + */ + public static void populateCertificates(Package pkg, Certificate[][] certificates) + throws PackageParserException { + pkg.mCertificates = null; + pkg.mSignatures = null; + pkg.mSigningKeys = null; + + pkg.mCertificates = certificates; + try { + pkg.mSignatures = convertToSignatures(certificates); + } catch (CertificateEncodingException e) { + // certificates weren't encoded properly; something went wrong + throw new PackageParserException(INSTALL_PARSE_FAILED_NO_CERTIFICATES, + "Failed to collect certificates from " + pkg.baseCodePath, e); + } + pkg.mSigningKeys = new ArraySet<>(certificates.length); + for (int i = 0; i < certificates.length; i++) { + Certificate[] signerCerts = certificates[i]; + Certificate signerCert = signerCerts[0]; + pkg.mSigningKeys.add(signerCert.getPublicKey()); + } + // add signatures to child packages + final int childCount = (pkg.childPackages != null) ? pkg.childPackages.size() : 0; + for (int i = 0; i < childCount; i++) { + Package childPkg = pkg.childPackages.get(i); + childPkg.mCertificates = pkg.mCertificates; + childPkg.mSignatures = pkg.mSignatures; + childPkg.mSigningKeys = pkg.mSigningKeys; + } + } + /** * Collect certificates from all the APKs described in the given package, * populating {@link Package#mSignatures}. Also asserts that all APK @@ -1304,6 +1343,7 @@ public class PackageParser { parser = assets.openXmlResourceParser(cookie, ANDROID_MANIFEST_FILENAME); final Signature[] signatures; + final Certificate[][] certificates; if ((flags & PARSE_COLLECT_CERTIFICATES) != 0) { // TODO: factor signature related items out of Package object final Package tempPkg = new Package(null); @@ -1314,12 +1354,14 @@ public class PackageParser { Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER); } signatures = tempPkg.mSignatures; + certificates = tempPkg.mCertificates; } else { signatures = null; + certificates = null; } final AttributeSet attrs = parser; - return parseApkLite(apkPath, res, parser, attrs, flags, signatures); + return parseApkLite(apkPath, res, parser, attrs, flags, signatures, certificates); } catch (XmlPullParserException | IOException | RuntimeException e) { throw new PackageParserException(INSTALL_PARSE_FAILED_UNEXPECTED_EXCEPTION, @@ -1405,8 +1447,8 @@ public class PackageParser { } private static ApkLite parseApkLite(String codePath, Resources res, XmlPullParser parser, - AttributeSet attrs, int flags, Signature[] signatures) throws IOException, - XmlPullParserException, PackageParserException { + AttributeSet attrs, int flags, Signature[] signatures, Certificate[][] certificates) + throws IOException, XmlPullParserException, PackageParserException { final Pair packageSplit = parsePackageSplitNames(parser, attrs); int installLocation = PARSE_DEFAULT_INSTALL_LOCATION; @@ -1466,8 +1508,8 @@ public class PackageParser { } return new ApkLite(codePath, packageSplit.first, packageSplit.second, versionCode, - revisionCode, installLocation, verifiers, signatures, coreApp, multiArch, - use32bitAbi, extractNativeLibs); + revisionCode, installLocation, verifiers, signatures, certificates, coreApp, + multiArch, use32bitAbi, extractNativeLibs); } /** diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index ef53905d55bba..6cdc40f7a81cc 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -81,6 +81,7 @@ import java.io.File; import java.io.FileDescriptor; import java.io.FileFilter; import java.io.IOException; +import java.security.cert.Certificate; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -151,6 +152,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private String mPackageName; private int mVersionCode; private Signature[] mSignatures; + private Certificate[][] mCertificates; /** * Path to the validated base APK for this session, which may point at an @@ -633,7 +635,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { mRelinquished = true; mPm.installStage(mPackageName, stageDir, stageCid, localObserver, params, - installerPackageName, installerUid, user); + installerPackageName, installerUid, user, mCertificates); } /** @@ -695,6 +697,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } if (mSignatures == null) { mSignatures = apk.signatures; + mCertificates = apk.certificates; } assertApkConsistent(String.valueOf(addedFile), apk); diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index fbf4c0cb9cc0d..19a1e7bb39a79 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -277,6 +277,7 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.PublicKey; +import java.security.cert.Certificate; import java.security.cert.CertificateEncodingException; import java.security.cert.CertificateException; import java.text.SimpleDateFormat; @@ -10964,7 +10965,8 @@ public class PackageManagerService extends IPackageManager.Stub { null /*originatingUri*/, null /*referrer*/, -1 /*originatingUid*/, callingUid); final InstallParams params = new InstallParams(origin, null /*moveInfo*/, observer, installFlags, installerPackageName, null /*volumeUuid*/, verificationInfo, user, - null /*packageAbiOverride*/, null /*grantedPermissions*/); + null /*packageAbiOverride*/, null /*grantedPermissions*/, + null /*certificates*/); params.setTraceMethod("installAsUser").setTraceCookie(System.identityHashCode(params)); msg.obj = params; @@ -10978,7 +10980,8 @@ public class PackageManagerService extends IPackageManager.Stub { void installStage(String packageName, File stagedDir, String stagedCid, IPackageInstallObserver2 observer, PackageInstaller.SessionParams sessionParams, - String installerPackageName, int installerUid, UserHandle user) { + String installerPackageName, int installerUid, UserHandle user, + Certificate[][] certificates) { if (DEBUG_EPHEMERAL) { if ((sessionParams.installFlags & PackageManager.INSTALL_EPHEMERAL) != 0) { Slog.d(TAG, "Ephemeral install of " + packageName); @@ -10999,7 +11002,7 @@ public class PackageManagerService extends IPackageManager.Stub { final InstallParams params = new InstallParams(origin, null, observer, sessionParams.installFlags, installerPackageName, sessionParams.volumeUuid, verificationInfo, user, sessionParams.abiOverride, - sessionParams.grantedRuntimePermissions); + sessionParams.grantedRuntimePermissions, certificates); params.setTraceMethod("installStage").setTraceCookie(System.identityHashCode(params)); msg.obj = params; @@ -12094,11 +12097,12 @@ public class PackageManagerService extends IPackageManager.Stub { final String packageAbiOverride; final String[] grantedRuntimePermissions; final VerificationInfo verificationInfo; + final Certificate[][] certificates; InstallParams(OriginInfo origin, MoveInfo move, IPackageInstallObserver2 observer, int installFlags, String installerPackageName, String volumeUuid, VerificationInfo verificationInfo, UserHandle user, String packageAbiOverride, - String[] grantedPermissions) { + String[] grantedPermissions, Certificate[][] certificates) { super(user); this.origin = origin; this.move = move; @@ -12109,6 +12113,7 @@ public class PackageManagerService extends IPackageManager.Stub { this.verificationInfo = verificationInfo; this.packageAbiOverride = packageAbiOverride; this.grantedRuntimePermissions = grantedPermissions; + this.certificates = certificates; } @Override @@ -12577,6 +12582,7 @@ public class PackageManagerService extends IPackageManager.Stub { /** If non-null, drop an async trace when the install completes */ final String traceMethod; final int traceCookie; + final Certificate[][] certificates; // The list of instruction sets supported by this app. This is currently // only used during the rmdex() phase to clean up resources. We can get rid of this @@ -12587,7 +12593,7 @@ public class PackageManagerService extends IPackageManager.Stub { int installFlags, String installerPackageName, String volumeUuid, UserHandle user, String[] instructionSets, String abiOverride, String[] installGrantPermissions, - String traceMethod, int traceCookie) { + String traceMethod, int traceCookie, Certificate[][] certificates) { this.origin = origin; this.move = move; this.installFlags = installFlags; @@ -12600,6 +12606,7 @@ public class PackageManagerService extends IPackageManager.Stub { this.installGrantPermissions = installGrantPermissions; this.traceMethod = traceMethod; this.traceCookie = traceCookie; + this.certificates = certificates; } abstract int copyApk(IMediaContainerService imcs, boolean temp) throws RemoteException; @@ -12692,9 +12699,9 @@ public class PackageManagerService extends IPackageManager.Stub { FileInstallArgs(InstallParams params) { super(params.origin, params.move, params.observer, params.installFlags, params.installerPackageName, params.volumeUuid, - params.getUser(), null /* instruction sets */, params.packageAbiOverride, + params.getUser(), null /*instructionSets*/, params.packageAbiOverride, params.grantedRuntimePermissions, - params.traceMethod, params.traceCookie); + params.traceMethod, params.traceCookie, params.certificates); if (isFwdLocked()) { throw new IllegalArgumentException("Forward locking only supported in ASEC"); } @@ -12703,7 +12710,7 @@ public class PackageManagerService extends IPackageManager.Stub { /** Existing install */ FileInstallArgs(String codePath, String resourcePath, String[] instructionSets) { super(OriginInfo.fromNothing(), null, null, 0, null, null, null, instructionSets, - null, null, null, 0); + null, null, null, 0, null /*certificates*/); this.codeFile = (codePath != null) ? new File(codePath) : null; this.resourceFile = (resourcePath != null) ? new File(resourcePath) : null; } @@ -12928,15 +12935,15 @@ public class PackageManagerService extends IPackageManager.Stub { params.installerPackageName, params.volumeUuid, params.getUser(), null /* instruction sets */, params.packageAbiOverride, params.grantedRuntimePermissions, - params.traceMethod, params.traceCookie); + params.traceMethod, params.traceCookie, params.certificates); } /** Existing install */ AsecInstallArgs(String fullCodePath, String[] instructionSets, boolean isExternal, boolean isForwardLocked) { super(OriginInfo.fromNothing(), null, null, (isExternal ? INSTALL_EXTERNAL : 0) - | (isForwardLocked ? INSTALL_FORWARD_LOCK : 0), null, null, null, - instructionSets, null, null, null, 0); + | (isForwardLocked ? INSTALL_FORWARD_LOCK : 0), null, null, null, + instructionSets, null, null, null, 0, null /*certificates*/); // Hackily pretend we're still looking at a full code path if (!fullCodePath.endsWith(RES_FILE_NAME)) { fullCodePath = new File(fullCodePath, RES_FILE_NAME).getAbsolutePath(); @@ -12952,8 +12959,8 @@ public class PackageManagerService extends IPackageManager.Stub { AsecInstallArgs(String cid, String[] instructionSets, boolean isForwardLocked) { super(OriginInfo.fromNothing(), null, null, (isAsecExternal(cid) ? INSTALL_EXTERNAL : 0) - | (isForwardLocked ? INSTALL_FORWARD_LOCK : 0), null, null, null, - instructionSets, null, null, null, 0); + | (isForwardLocked ? INSTALL_FORWARD_LOCK : 0), null, null, null, + instructionSets, null, null, null, 0, null /*certificates*/); this.cid = cid; setMountPath(PackageHelper.getSdDir(cid)); } @@ -13222,7 +13229,7 @@ public class PackageManagerService extends IPackageManager.Stub { params.installerPackageName, params.volumeUuid, params.getUser(), null /* instruction sets */, params.packageAbiOverride, params.grantedRuntimePermissions, - params.traceMethod, params.traceCookie); + params.traceMethod, params.traceCookie, params.certificates); } int copyApk(IMediaContainerService imcs, boolean temp) { @@ -14261,7 +14268,18 @@ public class PackageManagerService extends IPackageManager.Stub { } try { - PackageParser.collectCertificates(pkg, parseFlags); + // either use what we've been given or parse directly from the APK + if (args.certificates != null) { + try { + PackageParser.populateCertificates(pkg, args.certificates); + } catch (PackageParserException e) { + // there was something wrong with the certificates we were given; + // try to pull them from the APK + PackageParser.collectCertificates(pkg, parseFlags); + } + } else { + PackageParser.collectCertificates(pkg, parseFlags); + } } catch (PackageParserException e) { res.setError("Failed collect during installPackageLI", e); return; @@ -19146,7 +19164,7 @@ Slog.v(TAG, ":: stepped forward, applying functor at tag " + parser.getName()); final OriginInfo origin = OriginInfo.fromExistingFile(codeFile); final InstallParams params = new InstallParams(origin, move, installObserver, installFlags, installerPackageName, volumeUuid, null /*verificationInfo*/, user, - packageAbiOverride, null); + packageAbiOverride, null /*grantedPermissions*/, null /*certificates*/); params.setTraceMethod("movePackage").setTraceCookie(System.identityHashCode(params)); msg.obj = params;