Merge "Require .dm to install with .fsv_sig if supported" into sc-dev

3 files changed, 168 insertions, 73 deletions

diff --git a/core/java/android/content/pm/dex/DexMetadataHelper.java b/core/java/android/content/pm/dex/DexMetadataHelper.java
index bc5d14a533e2..0d5b33cd8672 100644
--- a/core/java/android/content/pm/dex/DexMetadataHelper.java
+++ b/core/java/android/content/pm/dex/DexMetadataHelper.java
@@ -24,16 +24,16 @@ import android.content.pm.parsing.ApkLiteParseUtils;
 import android.content.pm.parsing.PackageLite;
 import android.os.SystemProperties;
 import android.util.ArrayMap;
-import android.util.jar.StrictJarFile;
 import android.util.JsonReader;
 import android.util.Log;
+import android.util.jar.StrictJarFile;
 
 import com.android.internal.annotations.VisibleForTesting;
 
 import java.io.File;
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
-import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
@@ -53,7 +53,10 @@ public class DexMetadataHelper {
     /** $> adb shell 'setprop log.tag.DexMetadataHelper VERBOSE' */
     public static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);
     /** $> adb shell 'setprop pm.dexopt.dm.require_manifest true' */
-    private static String PROPERTY_DM_JSON_MANIFEST_REQUIRED = "pm.dexopt.dm.require_manifest";
+    private static final String PROPERTY_DM_JSON_MANIFEST_REQUIRED =
+            "pm.dexopt.dm.require_manifest";
+    /** $> adb shell 'setprop pm.dexopt.dm.require_fsverity true' */
+    private static final String PROPERTY_DM_FSVERITY_REQUIRED = "pm.dexopt.dm.require_fsverity";
 
     private static final String DEX_METADATA_FILE_EXTENSION = ".dm";
 
@@ -70,6 +73,13 @@ public class DexMetadataHelper {
     }
 
     /**
+     * Returns whether fs-verity is required to install a dex metadata
+     */
+    public static boolean isFsVerityRequired() {
+        return SystemProperties.getBoolean(PROPERTY_DM_FSVERITY_REQUIRED, false);
+    }
+
+    /**
      * Return the size (in bytes) of all dex metadata files associated with the given package.
      */
     public static long getPackageDexMetadataSize(PackageLite pkg) {
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 5cb9d8ff3f31..f09f33ea95ff 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -781,7 +781,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
     @GuardedBy("mLock")
     private File mInheritedFilesBase;
     @GuardedBy("mLock")
-    private boolean mVerityFound;
+    private boolean mVerityFoundForApks;
 
     /**
      * Both flags should be guarded with mLock whenever changes need to be in lockstep.
@@ -2717,8 +2717,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
             throw new PackageManagerException(INSTALL_FAILED_INVALID_APK,
                     "Missing existing base package");
         }
-        // Default to require only if existing base has fs-verity.
-        mVerityFound = PackageManagerServiceUtils.isApkVerityEnabled()
+        // Default to require only if existing base apk has fs-verity.
+        mVerityFoundForApks = PackageManagerServiceUtils.isApkVerityEnabled()
                 && params.mode == SessionParams.MODE_INHERIT_EXISTING
                 && VerityUtils.hasFsverity(pkgInfo.applicationInfo.getBaseCodePath());
 
@@ -3013,34 +3013,18 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
     }
 
     @GuardedBy("mLock")
-    private void maybeStageFsveritySignatureLocked(File origFile, File targetFile)
-            throws PackageManagerException {
+    private void maybeStageFsveritySignatureLocked(File origFile, File targetFile,
+            boolean fsVerityRequired) throws PackageManagerException {
         final File originalSignature = new File(
                 VerityUtils.getFsveritySignatureFilePath(origFile.getPath()));
-        // Make sure .fsv_sig exists when it should, then resolve and stage it.
         if (originalSignature.exists()) {
-            // mVerityFound can only change from false to true here during the staging loop. Since
-            // all or none of files should have .fsv_sig, this should only happen in the first time
-            // (or never), otherwise bail out.
-            if (!mVerityFound) {
-                mVerityFound = true;
-                if (mResolvedStagedFiles.size() > 1) {
-                    throw new PackageManagerException(INSTALL_FAILED_BAD_SIGNATURE,
-                            "Some file is missing fs-verity signature");
-                }
-            }
-        } else {
-            if (!mVerityFound) {
-                return;
-            }
+            final File stagedSignature = new File(
+                    VerityUtils.getFsveritySignatureFilePath(targetFile.getPath()));
+            stageFileLocked(originalSignature, stagedSignature);
+        } else if (fsVerityRequired) {
             throw new PackageManagerException(INSTALL_FAILED_BAD_SIGNATURE,
                     "Missing corresponding fs-verity signature to " + origFile);
         }
-
-        final File stagedSignature = new File(
-                VerityUtils.getFsveritySignatureFilePath(targetFile.getPath()));
-
-        stageFileLocked(originalSignature, stagedSignature);
     }
 
     @GuardedBy("mLock")
@@ -3059,7 +3043,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                 DexMetadataHelper.buildDexMetadataPathForApk(targetFile.getName()));
 
         stageFileLocked(dexMetadataFile, targetDexMetadataFile);
-        maybeStageFsveritySignatureLocked(dexMetadataFile, targetDexMetadataFile);
+
+        // Also stage .dm.fsv_sig. .dm may be required to install with fs-verity signature on
+        // supported on older devices.
+        maybeStageFsveritySignatureLocked(dexMetadataFile, targetDexMetadataFile,
+                VerityUtils.isFsVeritySupported() && DexMetadataHelper.isFsVerityRequired());
     }
 
     private void storeBytesToInstallationFile(final String localPath, final String absolutePath,
@@ -3121,13 +3109,45 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
     }
 
     @GuardedBy("mLock")
+    private boolean isFsVerityRequiredForApk(File origFile, File targetFile)
+            throws PackageManagerException {
+        if (mVerityFoundForApks) {
+            return true;
+        }
+
+        // We haven't seen .fsv_sig for any APKs. Treat it as not required until we see one.
+        final File originalSignature = new File(
+                VerityUtils.getFsveritySignatureFilePath(origFile.getPath()));
+        if (!originalSignature.exists()) {
+            return false;
+        }
+        mVerityFoundForApks = true;
+
+        // When a signature is found, also check any previous staged APKs since they also need to
+        // have fs-verity signature consistently.
+        for (File file : mResolvedStagedFiles) {
+            if (!file.getName().endsWith(".apk")) {
+                continue;
+            }
+            // Ignore the current targeting file.
+            if (targetFile.getName().equals(file.getName())) {
+                continue;
+            }
+            throw new PackageManagerException(INSTALL_FAILED_BAD_SIGNATURE,
+                    "Previously staged apk is missing fs-verity signature");
+        }
+        return true;
+    }
+
+    @GuardedBy("mLock")
     private void resolveAndStageFileLocked(File origFile, File targetFile, String splitName)
             throws PackageManagerException {
         stageFileLocked(origFile, targetFile);
 
-        // Stage fsverity signature if present.
-        maybeStageFsveritySignatureLocked(origFile, targetFile);
-        // Stage dex metadata (.dm) if present.
+        // Stage APK's fs-verity signature if present.
+        maybeStageFsveritySignatureLocked(origFile, targetFile,
+                isFsVerityRequiredForApk(origFile, targetFile));
+        // Stage dex metadata (.dm) and corresponding fs-verity signature if present.
         maybeStageDexMetadataLocked(origFile, targetFile);
         // Stage checksums (.digests) if present.
         maybeStageDigestsLocked(origFile, targetFile, splitName);
diff --git a/tests/ApkVerityTest/src/com/android/apkverity/ApkVerityTest.java b/tests/ApkVerityTest/src/com/android/apkverity/ApkVerityTest.java
index ab3572ba2173..d96005b8a71a 100644
--- a/tests/ApkVerityTest/src/com/android/apkverity/ApkVerityTest.java
+++ b/tests/ApkVerityTest/src/com/android/apkverity/ApkVerityTest.java
@@ -95,10 +95,13 @@ public class ApkVerityTest extends BaseHostJUnit4Test {
             new AddFsVerityCertRule(this, CERT_PATH);
 
     private ITestDevice mDevice;
+    private boolean mDmRequireFsVerity;
 
     @Before
     public void setUp() throws DeviceNotAvailableException {
         mDevice = getDevice();
+        mDmRequireFsVerity = "true".equals(
+                mDevice.getProperty("pm.dexopt.dm.require_fsverity"));
 
         uninstallPackage(TARGET_PACKAGE);
     }
@@ -124,7 +127,7 @@ public class ApkVerityTest extends BaseHostJUnit4Test {
         verifyInstalledFiles(
                 INSTALLED_BASE_APK,
                 INSTALLED_BASE_APK_FSV_SIG);
-        verifyInstalledFilesHaveFsverity();
+        verifyInstalledFilesHaveFsverity(INSTALLED_BASE_APK);
     }
 
     @Test
@@ -151,7 +154,9 @@ public class ApkVerityTest extends BaseHostJUnit4Test {
                 INSTALLED_BASE_APK_FSV_SIG,
                 INSTALLED_SPLIT_APK,
                 INSTALLED_SPLIT_APK_FSV_SIG);
-        verifyInstalledFilesHaveFsverity();
+        verifyInstalledFilesHaveFsverity(
+                INSTALLED_BASE_APK,
+                INSTALLED_SPLIT_APK);
     }
 
     @Test
@@ -167,7 +172,9 @@ public class ApkVerityTest extends BaseHostJUnit4Test {
                 INSTALLED_BASE_APK_FSV_SIG,
                 INSTALLED_BASE_DM,
                 INSTALLED_BASE_DM_FSV_SIG);
-        verifyInstalledFilesHaveFsverity();
+        verifyInstalledFilesHaveFsverity(
+                INSTALLED_BASE_APK,
+                INSTALLED_BASE_DM);
     }
 
     @Test
@@ -189,7 +196,11 @@ public class ApkVerityTest extends BaseHostJUnit4Test {
                 INSTALLED_SPLIT_APK_FSV_SIG,
                 INSTALLED_SPLIT_DM,
                 INSTALLED_SPLIT_DM_FSV_SIG);
-        verifyInstalledFilesHaveFsverity();
+        verifyInstalledFilesHaveFsverity(
+                INSTALLED_BASE_APK,
+                INSTALLED_BASE_DM,
+                INSTALLED_SPLIT_APK,
+                INSTALLED_SPLIT_DM);
     }
 
     @Test
@@ -213,7 +224,9 @@ public class ApkVerityTest extends BaseHostJUnit4Test {
                 INSTALLED_BASE_APK_FSV_SIG,
                 INSTALLED_SPLIT_APK,
                 INSTALLED_SPLIT_APK_FSV_SIG);
-        verifyInstalledFilesHaveFsverity();
+        verifyInstalledFilesHaveFsverity(
+                INSTALLED_BASE_APK,
+                INSTALLED_SPLIT_APK);
     }
 
     @Test
@@ -250,39 +263,92 @@ public class ApkVerityTest extends BaseHostJUnit4Test {
                 INSTALLED_BASE_APK,
                 INSTALLED_SPLIT_APK,
                 INSTALLED_SPLIT_APK_FSV_SIG);
-
     }
 
     @Test
-    public void testInstallOnlyBaseHasFsvSig()
+    public void testInstallOnlyDmHasFsvSig()
             throws DeviceNotAvailableException, FileNotFoundException {
         new InstallMultiple()
-                .addFileAndSignature(BASE_APK)
+                .addFile(BASE_APK)
+                .addFileAndSignature(BASE_APK_DM)
+                .addFile(SPLIT_APK)
+                .addFileAndSignature(SPLIT_APK_DM)
+                .run();
+        verifyInstalledFiles(
+                INSTALLED_BASE_APK,
+                INSTALLED_BASE_DM,
+                INSTALLED_BASE_DM_FSV_SIG,
+                INSTALLED_SPLIT_APK,
+                INSTALLED_SPLIT_DM,
+                INSTALLED_SPLIT_DM_FSV_SIG);
+        verifyInstalledFilesHaveFsverity(
+                INSTALLED_BASE_DM,
+                INSTALLED_SPLIT_DM);
+    }
+
+    @Test
+    public void testInstallDmWithoutFsvSig_Base()
+            throws DeviceNotAvailableException, FileNotFoundException {
+        InstallMultiple installer = new InstallMultiple()
+                .addFile(BASE_APK)
                 .addFile(BASE_APK_DM)
                 .addFile(SPLIT_APK)
-                .addFile(SPLIT_APK_DM)
-                .runExpectingFailure();
+                .addFileAndSignature(SPLIT_APK_DM);
+        if (mDmRequireFsVerity) {
+            installer.runExpectingFailure();
+        } else {
+            installer.run();
+            verifyInstalledFiles(
+                    INSTALLED_BASE_APK,
+                    INSTALLED_BASE_DM,
+                    INSTALLED_SPLIT_APK,
+                    INSTALLED_SPLIT_DM,
+                    INSTALLED_SPLIT_DM_FSV_SIG);
+            verifyInstalledFilesHaveFsverity(INSTALLED_SPLIT_DM);
+        }
     }
 
     @Test
-    public void testInstallOnlyDmHasFsvSig()
+    public void testInstallDmWithoutFsvSig_Split()
             throws DeviceNotAvailableException, FileNotFoundException {
-        new InstallMultiple()
+        InstallMultiple installer = new InstallMultiple()
                 .addFile(BASE_APK)
                 .addFileAndSignature(BASE_APK_DM)
                 .addFile(SPLIT_APK)
-                .addFile(SPLIT_APK_DM)
+                .addFile(SPLIT_APK_DM);
+        if (mDmRequireFsVerity) {
+            installer.runExpectingFailure();
+        } else {
+            installer.run();
+            verifyInstalledFiles(
+                    INSTALLED_BASE_APK,
+                    INSTALLED_BASE_DM,
+                    INSTALLED_BASE_DM_FSV_SIG,
+                    INSTALLED_SPLIT_APK,
+                    INSTALLED_SPLIT_DM);
+            verifyInstalledFilesHaveFsverity(INSTALLED_BASE_DM);
+        }
+    }
+
+    @Test
+    public void testInstallSomeApkIsMissingFsvSig_Base()
+            throws DeviceNotAvailableException, FileNotFoundException {
+        new InstallMultiple()
+                .addFileAndSignature(BASE_APK)
+                .addFileAndSignature(BASE_APK_DM)
+                .addFile(SPLIT_APK)
+                .addFileAndSignature(SPLIT_APK_DM)
                 .runExpectingFailure();
     }
 
     @Test
-    public void testInstallOnlySplitHasFsvSig()
+    public void testInstallSomeApkIsMissingFsvSig_Split()
             throws DeviceNotAvailableException, FileNotFoundException {
         new InstallMultiple()
                 .addFile(BASE_APK)
-                .addFile(BASE_APK_DM)
+                .addFileAndSignature(BASE_APK_DM)
                 .addFileAndSignature(SPLIT_APK)
-                .addFile(SPLIT_APK_DM)
+                .addFileAndSignature(SPLIT_APK_DM)
                 .runExpectingFailure();
     }
 
@@ -383,37 +449,36 @@ public class ApkVerityTest extends BaseHostJUnit4Test {
         }
     }
 
-    private void verifyInstalledFilesHaveFsverity() throws DeviceNotAvailableException {
+    private void verifyInstalledFilesHaveFsverity(String... filenames)
+            throws DeviceNotAvailableException {
         // Verify that all files are protected by fs-verity
         String apkPath = getApkPath(TARGET_PACKAGE);
         String appDir = apkPath.substring(0, apkPath.lastIndexOf("/"));
         long kTargetOffset = 0;
-        for (String basename : expectRemoteCommandToSucceed("ls " + appDir).split("\n")) {
-            if (basename.endsWith(".apk") || basename.endsWith(".dm")) {
-                String path = appDir + "/" + basename;
-                damageFileAgainstBlockDevice(path, kTargetOffset);
-
-                // Retry is sometimes needed to pass the test. Package manager may have FD leaks
-                // (see b/122744005 as example) that prevents the file in question to be evicted
-                // from filesystem cache. Forcing GC workarounds the problem.
-                int retry = 5;
-                for (; retry > 0; retry--) {
-                    BlockDeviceWriter.dropCaches(mDevice);
-                    if (!BlockDeviceWriter.canReadByte(mDevice, path, kTargetOffset)) {
-                        break;
-                    }
-                    try {
-                        CLog.d("lsof: " + expectRemoteCommandToSucceed("lsof " + apkPath));
-                        Thread.sleep(1000);
-                        String pid = expectRemoteCommandToSucceed("pidof system_server");
-                        mDevice.executeShellV2Command("kill -10 " + pid);  // force GC
-                    } catch (InterruptedException e) {
-                        Thread.currentThread().interrupt();
-                        return;
-                    }
+        for (String basename : filenames) {
+            String path = appDir + "/" + basename;
+            damageFileAgainstBlockDevice(path, kTargetOffset);
+
+            // Retry is sometimes needed to pass the test. Package manager may have FD leaks
+            // (see b/122744005 as example) that prevents the file in question to be evicted
+            // from filesystem cache. Forcing GC workarounds the problem.
+            int retry = 5;
+            for (; retry > 0; retry--) {
+                BlockDeviceWriter.dropCaches(mDevice);
+                if (!BlockDeviceWriter.canReadByte(mDevice, path, kTargetOffset)) {
+                    break;
+                }
+                try {
+                    CLog.d("lsof: " + expectRemoteCommandToSucceed("lsof " + apkPath));
+                    Thread.sleep(1000);
+                    String pid = expectRemoteCommandToSucceed("pidof system_server");
+                    mDevice.executeShellV2Command("kill -10 " + pid);  // force GC
+                } catch (InterruptedException e) {
+                    Thread.currentThread().interrupt();
+                    return;
                 }
-                assertTrue("Read from " + path + " should fail", retry > 0);
             }
+            assertTrue("Read from " + path + " should fail", retry > 0);
         }
     }