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"; @@ -69,6 +72,13 @@ public class DexMetadataHelper { return path.endsWith(DEX_METADATA_FILE_EXTENSION); } + /** + * 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. */ 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, @@ -3120,14 +3108,46 @@ 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,18 +263,6 @@ public class ApkVerityTest extends BaseHostJUnit4Test { INSTALLED_BASE_APK, INSTALLED_SPLIT_APK, INSTALLED_SPLIT_APK_FSV_SIG); - - } - - @Test - public void testInstallOnlyBaseHasFsvSig() - throws DeviceNotAvailableException, FileNotFoundException { - new InstallMultiple() - .addFileAndSignature(BASE_APK) - .addFile(BASE_APK_DM) - .addFile(SPLIT_APK) - .addFile(SPLIT_APK_DM) - .runExpectingFailure(); } @Test @@ -271,18 +272,83 @@ public class ApkVerityTest extends BaseHostJUnit4Test { .addFile(BASE_APK) .addFileAndSignature(BASE_APK_DM) .addFile(SPLIT_APK) - .addFile(SPLIT_APK_DM) + .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) + .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 testInstallDmWithoutFsvSig_Split() + throws DeviceNotAvailableException, FileNotFoundException { + InstallMultiple installer = new InstallMultiple() + .addFile(BASE_APK) + .addFileAndSignature(BASE_APK_DM) + .addFile(SPLIT_APK) + .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); + 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; - } + // 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); } }