From 0f72bc39849bcb2ff1cdca5494e3c00edac0f654 Mon Sep 17 00:00:00 2001
From: Song Pan <songpan@google.com>
Date: Fri, 28 Feb 2020 18:28:06 +0000
Subject: [PATCH] Fix some bugs for robustness.

1. Catch all exceptions when we are trying to read an APK to find the base apk.
We noticed in some error cases, package parser will throw an exception instead
of returning null. We should continue inspecting other apks.
2. Make invalid apk default to allow.

Bug: 145674131
Test: atest AppIntegrityMangaerServiceImpl
Change-Id: I11f2b7bad84c3ee697e01da68f901935db0c4f44
---
 .../AppIntegrityManagerServiceImpl.java       | 43 +++++++++++--------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java b/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java
index b4ec359f04666..54dd69d706854 100644
--- a/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java
+++ b/services/core/java/com/android/server/integrity/AppIntegrityManagerServiceImpl.java
@@ -239,17 +239,6 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
             Slog.i(TAG, "Received integrity verification intent " + intent.toString());
             Slog.i(TAG, "Extras " + intent.getExtras());
 
-            String packageName = intent.getStringExtra(EXTRA_PACKAGE_NAME);
-
-            PackageInfo packageInfo = getPackageArchiveInfo(intent.getData());
-            if (packageInfo == null) {
-                Slog.w(TAG, "Cannot parse package " + packageName);
-                // We can't parse the package.
-                mPackageManagerInternal.setIntegrityVerificationResult(
-                        verificationId, PackageManagerInternal.INTEGRITY_VERIFICATION_ALLOW);
-                return;
-            }
-
             String installerPackageName = getInstallerPackageName(intent);
 
             // Skip integrity verification if the verifier is doing the install.
@@ -261,6 +250,17 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
                 return;
             }
 
+            String packageName = intent.getStringExtra(EXTRA_PACKAGE_NAME);
+
+            PackageInfo packageInfo = getPackageArchiveInfo(intent.getData());
+            if (packageInfo == null) {
+                Slog.w(TAG, "Cannot parse package " + packageName);
+                // We can't parse the package.
+                mPackageManagerInternal.setIntegrityVerificationResult(
+                        verificationId, PackageManagerInternal.INTEGRITY_VERIFICATION_ALLOW);
+                return;
+            }
+
             List<String> appCertificates = getCertificateFingerprint(packageInfo);
             List<String> installerCertificates =
                     getInstallerCertificateFingerprint(installerPackageName);
@@ -508,7 +508,8 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
             return PackageInfoUtils.generate(pkg, null, flags, 0, 0, null, new PackageUserState(),
                     UserHandle.getCallingUserId(), null);
         } catch (Exception e) {
-            throw new IllegalArgumentException("Exception reading " + dataUri, e);
+            Slog.w(TAG, "Exception reading " + dataUri, e);
+            return null;
         }
     }
 
@@ -530,12 +531,18 @@ public class AppIntegrityManagerServiceImpl extends IAppIntegrityManager.Stub {
 
                 // If we didn't find a base.apk, then try to parse each apk until we find the one
                 // that succeeds.
-                basePackageInfo =
-                        mContext.getPackageManager()
-                                .getPackageArchiveInfo(
-                                        apkFile.getAbsolutePath(),
-                                        PackageManager.GET_SIGNING_CERTIFICATES
-                                                | PackageManager.GET_META_DATA);
+                try {
+                    basePackageInfo =
+                            mContext.getPackageManager()
+                                    .getPackageArchiveInfo(
+                                            apkFile.getAbsolutePath(),
+                                            PackageManager.GET_SIGNING_CERTIFICATES
+                                                    | PackageManager.GET_META_DATA);
+                } catch (Exception e) {
+                    // Some of the splits may not contain a valid android manifest. It is an
+                    // expected exception. We still log it nonetheless but we should keep looking.
+                    Slog.w(TAG, "Exception reading " + apkFile, e);
+                }
                 if (basePackageInfo != null) {
                     Slog.i(TAG, "Found package info from " + apkFile);
                     break;