diff --git a/services/core/java/com/android/server/pm/InstallPackageHelper.java b/services/core/java/com/android/server/pm/InstallPackageHelper.java index 70bd24ce20a22..aedebbaa565ba 100644 --- a/services/core/java/com/android/server/pm/InstallPackageHelper.java +++ b/services/core/java/com/android/server/pm/InstallPackageHelper.java @@ -174,6 +174,7 @@ import com.android.server.pm.pkg.component.ParsedPermission; import com.android.server.pm.pkg.component.ParsedPermissionGroup; import com.android.server.pm.pkg.parsing.ParsingPackageUtils; import com.android.server.rollback.RollbackManagerInternal; +import com.android.server.security.FileIntegrityService; import com.android.server.utils.WatchedArrayMap; import com.android.server.utils.WatchedLongSparseArray; @@ -1836,6 +1837,7 @@ final class InstallPackageHelper { } } + var fis = FileIntegrityService.getService(); for (Map.Entry entry : fsverityCandidates.entrySet()) { try { final String filePath = entry.getKey(); @@ -1843,13 +1845,31 @@ final class InstallPackageHelper { continue; } - // Set up fs-verity with optional signature. final String signaturePath = entry.getValue(); - String optionalSignaturePath = null; if (new File(signaturePath).exists()) { - optionalSignaturePath = signaturePath; + // If signature is provided, enable fs-verity first so that the file can be + // measured for signature check below. + VerityUtils.setUpFsverity(filePath, (byte[]) null); + + if (!fis.verifyPkcs7DetachedSignature(signaturePath, filePath)) { + throw new PrepareFailure(PackageManager.INSTALL_FAILED_BAD_SIGNATURE, + "fs-verity signature does not verify against a known key"); + } + } else { + // Without signature, we don't need to access the digest right away and can + // enable fs-verity in background (since this is a blocking call). + new Thread("fsverity-setup") { + @Override public void run() { + try { + VerityUtils.setUpFsverity(filePath, (byte[]) null); + } catch (IOException e) { + // There's nothing we can do if the setup failed. Since fs-verity is + // optional, just ignore the error for now. + Slog.e(TAG, "Failed to enable fs-verity to " + filePath); + } + } + }.start(); } - VerityUtils.setUpFsverity(filePath, optionalSignaturePath); } catch (IOException e) { throw new PrepareFailure(PackageManager.INSTALL_FAILED_BAD_SIGNATURE, "Failed to enable fs-verity: " + e); diff --git a/services/core/java/com/android/server/security/FileIntegrityService.java b/services/core/java/com/android/server/security/FileIntegrityService.java index 466ac74a83225..5ae697315ed1b 100644 --- a/services/core/java/com/android/server/security/FileIntegrityService.java +++ b/services/core/java/com/android/server/security/FileIntegrityService.java @@ -23,27 +23,37 @@ import android.content.Context; import android.content.pm.PackageManager; import android.content.pm.PackageManagerInternal; import android.os.Binder; +import android.os.Build; import android.os.Environment; import android.os.IBinder; +import android.os.ParcelFileDescriptor; +import android.os.ResultReceiver; +import android.os.ShellCallback; +import android.os.ShellCommand; import android.os.UserHandle; import android.security.IFileIntegrityService; import android.util.Slog; +import com.android.internal.annotations.GuardedBy; import com.android.internal.security.VerityUtils; import com.android.server.LocalServices; import com.android.server.SystemService; import java.io.ByteArrayInputStream; import java.io.File; +import java.io.FileDescriptor; import java.io.IOException; +import java.io.InputStream; +import java.io.PrintWriter; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; 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.ArrayList; -import java.util.Collection; /** * A {@link SystemService} that provides file integrity related operations. @@ -52,9 +62,19 @@ import java.util.Collection; public class FileIntegrityService extends SystemService { private static final String TAG = "FileIntegrityService"; + /** The maximum size of signature file. This is just to avoid potential abuse. */ + private static final int MAX_SIGNATURE_FILE_SIZE_BYTES = 8192; + private static CertificateFactory sCertFactory; - private Collection mTrustedCertificates = new ArrayList(); + @GuardedBy("mTrustedCertificates") + private final ArrayList mTrustedCertificates = + new ArrayList(); + + /** Gets the instance of the service */ + public static FileIntegrityService getService() { + return LocalServices.getService(FileIntegrityService.class); + } private final IBinder mService = new IFileIntegrityService.Stub() { @Override @@ -75,13 +95,23 @@ public class FileIntegrityService extends SystemService { Slog.w(TAG, "Received a null certificate"); return false; } - return mTrustedCertificates.contains(toCertificate(certificateBytes)); + synchronized (mTrustedCertificates) { + return mTrustedCertificates.contains(toCertificate(certificateBytes)); + } } catch (CertificateException e) { Slog.e(TAG, "Failed to convert the certificate: " + e); return false; } } + @Override + public void onShellCommand(FileDescriptor in, FileDescriptor out, + FileDescriptor err, String[] args, ShellCallback callback, + ResultReceiver resultReceiver) { + new FileIntegrityServiceShellCommand() + .exec(this, in, out, err, args, callback, resultReceiver); + } + private void checkCallerPermission(String packageName) { final int callingUid = Binder.getCallingUid(); final int callingUserId = UserHandle.getUserId(callingUid); @@ -116,6 +146,7 @@ public class FileIntegrityService extends SystemService { } catch (CertificateException e) { Slog.wtf(TAG, "Cannot get an instance of X.509 certificate factory"); } + LocalServices.addService(FileIntegrityService.class, this); } @Override @@ -124,6 +155,34 @@ public class FileIntegrityService extends SystemService { publishBinderService(Context.FILE_INTEGRITY_SERVICE, mService); } + /** + * Returns whether the signature over the file's fs-verity digest can be verified by one of the + * known certiticates. + */ + public boolean verifyPkcs7DetachedSignature(String signaturePath, String filePath) + throws IOException { + if (Files.size(Paths.get(signaturePath)) > MAX_SIGNATURE_FILE_SIZE_BYTES) { + throw new SecurityException("Signature file is unexpectedly large: " + + signaturePath); + } + byte[] signatureBytes = Files.readAllBytes(Paths.get(signaturePath)); + byte[] digest = VerityUtils.getFsverityDigest(filePath); + synchronized (mTrustedCertificates) { + for (var cert : mTrustedCertificates) { + try { + byte[] derEncoded = cert.getEncoded(); + if (VerityUtils.verifyPkcs7DetachedSignature(signatureBytes, digest, + new ByteArrayInputStream(derEncoded))) { + return true; + } + } catch (CertificateEncodingException e) { + Slog.w(TAG, "Ignoring ill-formed certificate: " + e); + } + } + } + return false; + } + private void loadAllCertificates() { // A better alternative to load certificates would be to read from .fs-verity kernel // keyring, which fsverity_init loads to during earlier boot time from the same sources @@ -148,10 +207,6 @@ public class FileIntegrityService extends SystemService { for (File cert : files) { byte[] certificateBytes = Files.readAllBytes(cert.toPath()); - if (certificateBytes == null) { - Slog.w(TAG, "The certificate file is empty, ignoring " + cert); - continue; - } collectCertificate(certificateBytes); } } catch (IOException e) { @@ -165,7 +220,9 @@ public class FileIntegrityService extends SystemService { */ private void collectCertificate(@NonNull byte[] bytes) { try { - mTrustedCertificates.add(toCertificate(bytes)); + synchronized (mTrustedCertificates) { + mTrustedCertificates.add(toCertificate(bytes)); + } } catch (CertificateException e) { Slog.e(TAG, "Invalid certificate, ignored: " + e); } @@ -184,4 +241,71 @@ public class FileIntegrityService extends SystemService { } return (X509Certificate) certificate; } + + + private class FileIntegrityServiceShellCommand extends ShellCommand { + @Override + public int onCommand(String cmd) { + if (!Build.IS_DEBUGGABLE) { + return -1; + } + if (cmd == null) { + return handleDefaultCommands(cmd); + } + final PrintWriter pw = getOutPrintWriter(); + switch (cmd) { + case "append-cert": + String nextArg = getNextArg(); + if (nextArg == null) { + pw.println("Invalid argument"); + pw.println(""); + onHelp(); + return -1; + } + ParcelFileDescriptor pfd = openFileForSystem(nextArg, "r"); + if (pfd == null) { + pw.println("Cannot open the file"); + return -1; + } + InputStream is = new ParcelFileDescriptor.AutoCloseInputStream(pfd); + try { + collectCertificate(is.readAllBytes()); + } catch (IOException e) { + pw.println("Failed to add certificate: " + e); + return -1; + } + pw.println("Certificate is added successfully"); + return 0; + + case "remove-last-cert": + synchronized (mTrustedCertificates) { + if (mTrustedCertificates.size() == 0) { + pw.println("Certificate list is already empty"); + return -1; + } + mTrustedCertificates.remove(mTrustedCertificates.size() - 1); + } + pw.println("Certificate is removed successfully"); + return 0; + default: + pw.println("Unknown action"); + pw.println(""); + onHelp(); + } + return -1; + } + + @Override + public void onHelp() { + final PrintWriter pw = getOutPrintWriter(); + pw.println("File integrity service commands:"); + pw.println(" help"); + pw.println(" Print this help text."); + pw.println(" append-cert path/to/cert.der"); + pw.println(" Add the DER-encoded certificate (only in debug builds)"); + pw.println(" remove-last-cert"); + pw.println(" Remove the last certificate in the key list (only in debug builds)"); + pw.println(""); + } + } } diff --git a/tests/ApkVerityTest/src/com/android/apkverity/ApkVerityTest.java b/tests/ApkVerityTest/src/com/android/apkverity/ApkVerityTest.java index d96005b8a71a2..16f005f288564 100644 --- a/tests/ApkVerityTest/src/com/android/apkverity/ApkVerityTest.java +++ b/tests/ApkVerityTest/src/com/android/apkverity/ApkVerityTest.java @@ -25,7 +25,6 @@ import static org.junit.Assert.fail; import android.platform.test.annotations.RootPermissionTest; import com.android.blockdevicewriter.BlockDeviceWriter; -import com.android.fsverity.AddFsVerityCertRule; import com.android.tradefed.device.DeviceNotAvailableException; import com.android.tradefed.device.ITestDevice; import com.android.tradefed.log.LogUtil.CLog; @@ -36,7 +35,6 @@ import com.android.tradefed.util.CommandStatus; import org.junit.After; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -90,10 +88,6 @@ public class ApkVerityTest extends BaseHostJUnit4Test { /** Only 4K page is supported by fs-verity currently. */ private static final int FSVERITY_PAGE_SIZE = 4096; - @Rule - public final AddFsVerityCertRule mAddFsVerityCertRule = - new AddFsVerityCertRule(this, CERT_PATH); - private ITestDevice mDevice; private boolean mDmRequireFsVerity; @@ -103,11 +97,13 @@ public class ApkVerityTest extends BaseHostJUnit4Test { mDmRequireFsVerity = "true".equals( mDevice.getProperty("pm.dexopt.dm.require_fsverity")); + expectRemoteCommandToSucceed("cmd file_integrity append-cert " + CERT_PATH); uninstallPackage(TARGET_PACKAGE); } @After public void tearDown() throws DeviceNotAvailableException { + expectRemoteCommandToSucceed("cmd file_integrity remove-last-cert"); uninstallPackage(TARGET_PACKAGE); } diff --git a/tests/utils/hostutils/src/com/android/fsverity/AddFsVerityCertRule.java b/tests/utils/hostutils/src/com/android/fsverity/AddFsVerityCertRule.java deleted file mode 100644 index 5ab4dc60e2ac3..0000000000000 --- a/tests/utils/hostutils/src/com/android/fsverity/AddFsVerityCertRule.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright (C) 2021 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 com.android.fsverity; - -import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; - -import static org.junit.Assume.assumeTrue; - -import com.android.tradefed.device.DeviceNotAvailableException; -import com.android.tradefed.device.ITestDevice; -import com.android.tradefed.log.LogUtil; -import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test; -import com.android.tradefed.util.CommandResult; -import com.android.tradefed.util.CommandStatus; - -import org.junit.rules.ExternalResource; - -public final class AddFsVerityCertRule extends ExternalResource { - - private static final String APK_VERITY_STANDARD_MODE = "2"; - - private final BaseHostJUnit4Test mHost; - private final String mCertPath; - private String mKeyId; - - public AddFsVerityCertRule(BaseHostJUnit4Test host, String certPath) { - mHost = host; - mCertPath = certPath; - } - - @Override - protected void before() throws Throwable { - ITestDevice device = mHost.getDevice(); - String apkVerityMode = device.getProperty("ro.apk_verity.mode"); - assumeTrue(device.getLaunchApiLevel() >= 30 - || APK_VERITY_STANDARD_MODE.equals(apkVerityMode)); - - String keyId = executeCommand( - "mini-keyctl padd asymmetric fsv_test .fs-verity < " + mCertPath).trim(); - assertThat(keyId).matches("^\\d+$"); - mKeyId = keyId; - } - - @Override - protected void after() { - if (mKeyId == null) return; - try { - executeCommand("mini-keyctl unlink " + mKeyId + " .fs-verity"); - } catch (DeviceNotAvailableException e) { - LogUtil.CLog.e(e); - } - mKeyId = null; - } - - private String executeCommand(String cmd) throws DeviceNotAvailableException { - CommandResult result = mHost.getDevice().executeShellV2Command(cmd); - assertWithMessage("`" + cmd + "` failed: " + result.getStderr()) - .that(result.getStatus()) - .isEqualTo(CommandStatus.SUCCESS); - return result.getStdout(); - } -}