Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4bdb6e5e authored by Victor Hsieh's avatar Victor Hsieh
Browse files

Move fs-verity signature check from kernel to install

This unblocks the deprecation of fs-verity kernel keyring.

With an APK is installed with .fsv_sig, the signature was installed to
the filesystem to be verified with a kernel keyring. Due to the current
threat model of root or system server, checking the signature in kernel
is not superior since the attacker can simply strip fs-verity protection
from the file. The userspace check should be done anyway if the keyring
contains multiple keys.

Without regressing any security guarantee, this change moves the
signature check to (only) install time. FileIntegrityService already
holds the certificates and facilitates the signature check.

In order to keep the test passing, FileIntegrityService now supports
(debug only) cmd for adding and removing a debug cert.

Bug: 258708453
Test: ApkVerityTest
Test: CtsAppSecurityHostTestCases:android.appsecurity.cts.ApkVerityInstallTest
Test: ChecksumsTest

Change-Id: I737f058229928f1b242612631a13c62709b06d33
parent cc6f6e08
Loading
Loading
Loading
Loading
+9 −4
Original line number Diff line number Diff line
@@ -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<String, String> entry : fsverityCandidates.entrySet()) {
            try {
                final String filePath = entry.getKey();
@@ -1843,13 +1845,16 @@ final class InstallPackageHelper {
                    continue;
                }

                // Set up fs-verity with optional signature.
                VerityUtils.setUpFsverity(filePath, (byte[]) null);

                // Verify fs-verity signature if exists.
                final String signaturePath = entry.getValue();
                String optionalSignaturePath = null;
                if (new File(signaturePath).exists()) {
                    optionalSignaturePath = signaturePath;
                    if (!fis.verifyPkcs7DetachedSignature(signaturePath, filePath)) {
                        throw new PrepareFailure(PackageManager.INSTALL_FAILED_BAD_SIGNATURE,
                                "fs-verity signature does not verify against a known key");
                    }
                }
                VerityUtils.setUpFsverity(filePath, optionalSignaturePath);
            } catch (IOException e) {
                throw new PrepareFailure(PackageManager.INSTALL_FAILED_BAD_SIGNATURE,
                        "Failed to enable fs-verity: " + e);
+132 −8
Original line number Diff line number Diff line
@@ -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<X509Certificate> mTrustedCertificates = new ArrayList<X509Certificate>();
    @GuardedBy("mTrustedCertificates")
    private final ArrayList<X509Certificate> mTrustedCertificates =
            new ArrayList<X509Certificate>();

    /** 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;
                }
                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 {
            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("");
        }
    }
}
+2 −6
Original line number Diff line number Diff line
@@ -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);
    }