diff options
2 files changed, 99 insertions, 12 deletions
diff --git a/services/core/java/com/android/server/security/CertificateRevocationStatusManager.java b/services/core/java/com/android/server/security/CertificateRevocationStatusManager.java index d36d9f5f6636..4cd4b3b84910 100644 --- a/services/core/java/com/android/server/security/CertificateRevocationStatusManager.java +++ b/services/core/java/com/android/server/security/CertificateRevocationStatusManager.java @@ -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. @@ -127,6 +130,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<>(); for (String serialNumber : serialNumbers) { @@ -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); } - 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); - } + } + } + + 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().minus(lastCheckedWithin))) { + return false; } } + return true; } private static boolean needToCheckRevocationStatus( diff --git a/tests/AttestationVerificationTest/src/com/android/server/security/CertificateRevocationStatusManagerTest.java b/tests/AttestationVerificationTest/src/com/android/server/security/CertificateRevocationStatusManagerTest.java index c38517ace5e6..586bb76388f6 100644 --- a/tests/AttestationVerificationTest/src/com/android/server/security/CertificateRevocationStatusManagerTest.java +++ b/tests/AttestationVerificationTest/src/com/android/server/security/CertificateRevocationStatusManagerTest.java @@ -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)); |