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

Commit 8ba8fbd0 authored by Tom Chan's avatar Tom Chan
Browse files

Do not fetch remote CRL if all certificates are recently checked 

This change limits remote fetches to at most once every 24 hours and
reduces potentially metered network use for repeated attestations
from the same device.

Test: Manually, also atest AttestationVerificationTest:com.android.server.security.CertificateRevocationStatusManagerTest
Bug: 389088384
Flag: EXEMPT bug fix
Change-Id: Ie68b7c904b5a75145e75654f173f44655dae6d31
parent 0d5933f1

services/core/java/com/android/server/security/CertificateRevocationStatusManager.java

+33 −12
Original line number Diff line number Diff line
@@ -42,6 +42,7 @@ import java.net.MalformedURLException; 
import java.net.URL;

import java.security.cert.CertPathValidatorException;

import java.security.cert.X509Certificate;

import java.time.Duration;

import java.time.LocalDate;

import java.time.LocalDateTime;

import java.time.ZoneId;
@@ -67,6 +68,8 @@ class CertificateRevocationStatusManager { 
     */

    @VisibleForTesting static final int MAX_DAYS_SINCE_LAST_CHECK = 30;



    @VisibleForTesting static final int NUM_HOURS_BEFORE_NEXT_CHECK = 24;



    /**

     * The number of days since issue date for an intermediary certificate to be considered fresh

     * and not require a revocation list check.
@@ -126,6 +129,17 @@ class CertificateRevocationStatusManager { 
            }

            serialNumbers.add(serialNumber);

        }

        try {

            if (isLastCheckedWithin(Duration.ofHours(NUM_HOURS_BEFORE_NEXT_CHECK), serialNumbers)) {

                Slog.d(

                        TAG,

                        "All certificates have been checked for revocation recently. No need to"

                                + " check this time.");

                return;

            }

        } catch (IOException ignored) {

            // Proceed to check the revocation status

        }

        try {

            JSONObject revocationList = fetchRemoteRevocationList();

            Map<String, Boolean> areCertificatesRevoked = new HashMap<>();
@@ -151,25 +165,32 @@ class CertificateRevocationStatusManager { 
                    serialNumbers.remove(serialNumber);

                }

            }

            Map<String, LocalDateTime> lastRevocationCheckData;

            try {

                lastRevocationCheckData = getLastRevocationCheckData();

                if (!isLastCheckedWithin(

                        Duration.ofDays(MAX_DAYS_SINCE_LAST_CHECK), serialNumbers)) {

                    throw new CertPathValidatorException(

                            "Unable to verify the revocation status of one of the certificates "

                                    + serialNumbers);

                }

            } catch (IOException ex2) {

                throw new CertPathValidatorException(

                        "Unable to load stored revocation status", ex2);

            }

        }

    }



    private boolean isLastCheckedWithin(Duration lastCheckedWithin, List<String> serialNumbers)

            throws IOException {

        Map<String, LocalDateTime> lastRevocationCheckData = getLastRevocationCheckData();

        for (String serialNumber : serialNumbers) {

            if (!lastRevocationCheckData.containsKey(serialNumber)

                    || lastRevocationCheckData

                            .get(serialNumber)

                                .isBefore(

                                        LocalDateTime.now().minusDays(MAX_DAYS_SINCE_LAST_CHECK))) {

                    throw new CertPathValidatorException(

                            "Unable to verify the revocation status of certificate "

                                    + serialNumber);

                }

                            .isBefore(LocalDateTime.now().minus(lastCheckedWithin))) {

                return false;

            }

        }

        return true;

    }



    private static boolean needToCheckRevocationStatus(

tests/AttestationVerificationTest/src/com/android/server/security/CertificateRevocationStatusManagerTest.java

+66 −0
Original line number Diff line number Diff line
@@ -277,6 +277,72 @@ public class CertificateRevocationStatusManagerTest { 
        }

    }



    @Test

    public void checkRevocationStatus_allCertificatesRecentlyChecked_doesNotFetchRemoteCrl()

            throws Exception {

        copyFromAssetToFile(

                REVOCATION_LIST_WITHOUT_CERTIFICATES_USED_IN_THIS_TEST, mRevocationListFile);

        mCertificateRevocationStatusManager =

                new CertificateRevocationStatusManager(

                        mContext, mRevocationListUrl, mRevocationStatusFile, false);

        mCertificateRevocationStatusManager.checkRevocationStatus(mCertificates1);

        // indirectly verifies the remote list is not fetched by simulating a remote revocation

        copyFromAssetToFile(

                REVOCATION_LIST_WITH_CERTIFICATES_USED_IN_THIS_TEST, mRevocationListFile);



        // no exception

        mCertificateRevocationStatusManager.checkRevocationStatus(mCertificates1);

    }



    @Test

    public void checkRevocationStatus_allCertificatesBarelyRecentlyChecked_doesNotFetchRemoteCrl()

            throws Exception {

        copyFromAssetToFile(

                REVOCATION_LIST_WITH_CERTIFICATES_USED_IN_THIS_TEST, mRevocationListFile);

        mCertificateRevocationStatusManager =

                new CertificateRevocationStatusManager(

                        mContext, mRevocationListUrl, mRevocationStatusFile, false);

        Map<String, LocalDateTime> lastCheckedDates = new HashMap<>();

        LocalDateTime barelyRecently =

                LocalDateTime.now()

                        .minusHours(

                                CertificateRevocationStatusManager.NUM_HOURS_BEFORE_NEXT_CHECK - 1);

        for (X509Certificate certificate : mCertificates1) {

            lastCheckedDates.put(getSerialNumber(certificate), barelyRecently);

        }

        mCertificateRevocationStatusManager.storeLastRevocationCheckData(lastCheckedDates);



        // Indirectly verify the remote CRL is not checked by checking there is no exception despite

        // a certificate being revoked. This test differs from the next only in the lastCheckedDate,

        // one before the NUM_HOURS_BEFORE_NEXT_CHECK cutoff and one after

        mCertificateRevocationStatusManager.checkRevocationStatus(mCertificates1);

    }



    @Test

    public void checkRevocationStatus_certificatesRevokedAfterCheck_throwsException()

            throws Exception {

        copyFromAssetToFile(

                REVOCATION_LIST_WITH_CERTIFICATES_USED_IN_THIS_TEST, mRevocationListFile);

        mCertificateRevocationStatusManager =

                new CertificateRevocationStatusManager(

                        mContext, mRevocationListUrl, mRevocationStatusFile, false);

        Map<String, LocalDateTime> lastCheckedDates = new HashMap<>();

        // To save network use, we do not check the remote CRL if all the certificates are recently

        // checked, so we set the lastCheckDate to some time not recent.

        LocalDateTime notRecently =

                LocalDateTime.now()

                        .minusHours(

                                CertificateRevocationStatusManager.NUM_HOURS_BEFORE_NEXT_CHECK + 1);

        for (X509Certificate certificate : mCertificates1) {

            lastCheckedDates.put(getSerialNumber(certificate), notRecently);

        }

        mCertificateRevocationStatusManager.storeLastRevocationCheckData(lastCheckedDates);



        assertThrows(

                CertPathValidatorException.class,

                () -> mCertificateRevocationStatusManager.checkRevocationStatus(mCertificates1));

    }



    private List<X509Certificate> getCertificateChain(String fileName) throws Exception {

        Collection<? extends Certificate> certificates =

                mFactory.generateCertificates(mContext.getResources().getAssets().open(fileName));