Improve resilient OCSP certificate revocation checker

pom.xml

@@ -22,6 +22,7 @@
         <junit-jupiter.version>5.14.4</junit-jupiter.version>
         <assertj.version>3.27.7</assertj.version>
         <mockito.version>5.23.0</mockito.version>
+        <awaitility.version>4.3.0</awaitility.version>
         <maven-surefire-plugin.version>3.6.0</maven-surefire-plugin.version>
         <maven-compiler-plugin.version>3.15.0</maven-compiler-plugin.version>
         <maven-source-plugin.version>3.4.0</maven-source-plugin.version>
@@ -115,6 +116,12 @@
             <version>${mockito.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.awaitility</groupId>
+            <artifactId>awaitility</artifactId>
+            <version>${awaitility.version}</version>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.slf4j</groupId>
             <artifactId>slf4j-simple</artifactId>

src/main/java/eu/webeid/ocsp/OcspCertificateRevocationChecker.java

@@ -10,6 +10,7 @@
 import eu.webeid.ocsp.protocol.OcspResponseValidator;
 import eu.webeid.security.exceptions.AuthTokenException;
 import eu.webeid.ocsp.exceptions.UserCertificateOCSPCheckFailedException;
+import eu.webeid.ocsp.exceptions.UserCertificateOCSPException;
 import eu.webeid.security.util.DateAndTime;
 import eu.webeid.ocsp.service.OcspServiceProvider;
 import eu.webeid.ocsp.service.OcspService;
@@ -50,13 +51,13 @@ public class OcspCertificateRevocationChecker implements CertificateRevocationCh
 
     public static final Duration DEFAULT_TIME_SKEW = Duration.ofMinutes(15);
     public static final Duration DEFAULT_THIS_UPDATE_AGE = Duration.ofMinutes(2);
+    public static final Duration DEFAULT_NEXT_UPDATE_AGE = Duration.ofMinutes(15);
 
     private static final Logger LOG = LoggerFactory.getLogger(OcspCertificateRevocationChecker.class);
 
     private final OcspClient ocspClient;
     private final OcspServiceProvider ocspServiceProvider;
     private final Duration allowedOcspResponseTimeSkew;
-    private final Duration maxOcspResponseThisUpdateAge;
 
     static {
         if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) {
@@ -66,12 +67,10 @@ public class OcspCertificateRevocationChecker implements CertificateRevocationCh
 
     public OcspCertificateRevocationChecker(OcspClient ocspClient,
                                             OcspServiceProvider ocspServiceProvider,
-                                            Duration allowedOcspResponseTimeSkew,
-                                            Duration maxOcspResponseThisUpdateAge) {
+                                            Duration allowedOcspResponseTimeSkew) {
         this.ocspClient = requireNonNull(ocspClient, "ocspClient");
         this.ocspServiceProvider = requireNonNull(ocspServiceProvider, "ocspServiceProvider");
         this.allowedOcspResponseTimeSkew = requirePositiveDuration(allowedOcspResponseTimeSkew, "allowedOcspResponseTimeSkew");
-        this.maxOcspResponseThisUpdateAge = requirePositiveDuration(maxOcspResponseThisUpdateAge, "maxOcspResponseThisUpdateAge");
     }
 
     /**
@@ -86,22 +85,16 @@ public List<RevocationInfo> validateCertificateNotRevoked(X509Certificate subjec
         requireNonNull(subjectCertificate, "subjectCertificate");
         requireNonNull(issuerCertificate, "issuerCertificate");
 
-        URI ocspResponderUri = null;
-        try {
-            OcspService ocspService = ocspServiceProvider.getService(subjectCertificate, issuerCertificate);
-            ocspResponderUri = requireNonNull(ocspService.getAccessLocation(), "ocspResponderUri");
-
-            final CertificateID certificateId = getCertificateId(subjectCertificate, issuerCertificate);
-
-            final OCSPReq request = new OcspRequestBuilder()
-                .withCertificateId(certificateId)
-                .enableOcspNonce(ocspService.doesSupportNonce())
-                .build();
+        final OcspService ocspService = ocspServiceProvider.getService(subjectCertificate, issuerCertificate);
+        final CertificateID certificateId = getCertificateId(subjectCertificate, issuerCertificate);
+        final URI ocspResponderUri = ocspService.getAccessLocation();
+        final OCSPReq request = getOcspRequest(certificateId, ocspService);
 
-            if (!ocspService.doesSupportNonce()) {
-                LOG.debug("Disabling OCSP nonce extension");
-            }
+        if (!ocspService.doesSupportNonce()) {
+            LOG.debug("Disabling OCSP nonce extension");
+        }
 
+        try {
             LOG.debug("Sending OCSP request");
             final OCSPResp response = requireNonNull(ocspClient.request(ocspResponderUri, request), "OCSPResp");
             if (response.getStatus() != OCSPResponseStatus.SUCCESSFUL) {
@@ -113,20 +106,33 @@ public List<RevocationInfo> validateCertificateNotRevoked(X509Certificate subjec
             }
             LOG.debug("OCSP response received successfully");
 
-            verifyOcspResponse(basicResponse, ocspService, certificateId, issuerCertificate, maxOcspResponseThisUpdateAge);
+            verifyOcspResponse(basicResponse, ocspService, certificateId, issuerCertificate);
             if (ocspService.doesSupportNonce()) {
                 checkNonce(request, basicResponse, ocspResponderUri);
             }
             LOG.debug("OCSP response verified successfully");
 
             return List.of(new RevocationInfo(ocspResponderUri, Map.of(RevocationInfo.KEY_OCSP_RESPONSE, response)));
 
-        } catch (OCSPException | CertificateException | OperatorCreationException | IOException | OCSPClientException e) {
+        } catch (OCSPException | CertificateException | OperatorCreationException | OCSPClientException e) {
             throw new UserCertificateOCSPCheckFailedException(e, ocspResponderUri);
         }
     }
 
-    protected void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspService, CertificateID requestCertificateId, X509Certificate issuerCertificate, Duration maxOcspResponseThisUpdateAge) throws AuthTokenException, OCSPException, CertificateException, OperatorCreationException {
+    protected static OCSPReq getOcspRequest(CertificateID certificateId, OcspService ocspService) throws UserCertificateOCSPException {
+        final OCSPReq request;
+        try {
+            request = new OcspRequestBuilder()
+                .withCertificateId(certificateId)
+                .enableOcspNonce(ocspService.doesSupportNonce())
+                .build();
+        } catch (OCSPException e) {
+            throw new UserCertificateOCSPException("Unable to create OCSP request", e);
+        }
+        return request;
+    }
+
+    protected void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspService, CertificateID requestCertificateId, X509Certificate issuerCertificate) throws AuthTokenException, OCSPException, CertificateException, OperatorCreationException {
         // The verification algorithm follows RFC 2560, https://www.ietf.org/rfc/rfc2560.txt.
         //
         // 3.2.  Signed Response Acceptance Requirements
@@ -142,12 +148,18 @@ protected void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspS
                 + "received " + basicResponse.getResponses().length + " responses instead", ocspService.getAccessLocation());
         }
         final SingleResp certStatusResponse = basicResponse.getResponses()[0];
-        if (!requestCertificateId.equals(certStatusResponse.getCertID())) {
-            throw new UserCertificateOCSPCheckFailedException("OCSP responded with certificate ID that differs from the requested ID",
-                    ocspService.getAccessLocation());
-        }
+        OcspResponseValidator.validateCertificateId(requestCertificateId, certStatusResponse.getCertID(), ocspService.getAccessLocation());
 
-        //   2. The signature on the response is valid.
+        // Steps 3 and 4 are performed BEFORE step 2. The responder certificate is
+        // attacker-controlled until validated; verifying the response signature first lets
+        // an untrusted key drive expensive or unsafe crypto (e.g., a huge RSA public exponent)
+        // that later rejection can't undo.
+
+        //   3. The identity of the signer matches the intended recipient of the
+        //      request.
+        //
+        //   4. The signer is currently authorized to provide a response for the
+        //      certificate in question.
 
         // We assume that the responder includes its certificate in the certs field of the response
         // that helps us to verify it. According to RFC 2560 this field is optional, but including it
@@ -158,13 +170,6 @@ protected void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspS
         }
         // The first certificate is the responder certificate, other certificates, if given, are the certificate's chain.
         final X509CertificateHolder responderCert = basicResponse.getCerts()[0];
-        OcspResponseValidator.validateResponseSignature(basicResponse, responderCert, ocspService.getAccessLocation());
-
-        //   3. The identity of the signer matches the intended recipient of the
-        //      request.
-        //
-        //   4. The signer is currently authorized to provide a response for the
-        //      certificate in question.
 
         // Use the clock instance so that the date can be mocked in tests.
         final Date now = DateAndTime.DefaultClock.getInstance().now();
@@ -175,14 +180,18 @@ protected void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspS
             throw new UserCertificateOCSPCheckFailedException(e, ocspService.getAccessLocation());
         }
 
+        //   2. The signature on the response is valid.
+
+        OcspResponseValidator.validateResponseSignature(basicResponse, responderCert, ocspService.getAccessLocation());
+
         //   5. The time at which the status being indicated is known to be
         //      correct (thisUpdate) is sufficiently recent.
         //
         //   6. When available, the time at or before which newer information will
         //      be available about the status of the certificate (nextUpdate) is
         //      greater than the current time.
 
-        OcspResponseValidator.validateCertificateStatusUpdateTime(certStatusResponse, allowedOcspResponseTimeSkew, maxOcspResponseThisUpdateAge, ocspService.getAccessLocation());
+        OcspResponseValidator.validateCertificateStatusUpdateTime(certStatusResponse, allowedOcspResponseTimeSkew, ocspService.getMaxThisUpdateAge(), ocspService.getMaxNextUpdateAge(), ocspService.getAccessLocation());
 
         // Now we can accept the signed response as valid and validate the certificate status.
         OcspResponseValidator.validateSubjectCertificateStatus(certStatusResponse, ocspService.getAccessLocation());
@@ -202,11 +211,15 @@ protected static void checkNonce(OCSPReq request, BasicOCSPResp response, URI oc
         }
     }
 
-    protected static CertificateID getCertificateId(X509Certificate subjectCertificate, X509Certificate issuerCertificate) throws CertificateEncodingException, IOException, OCSPException {
-        final BigInteger serial = subjectCertificate.getSerialNumber();
-        final DigestCalculator digestCalculator = DigestCalculatorImpl.sha1();
-        return new CertificateID(digestCalculator,
-            new X509CertificateHolder(issuerCertificate.getEncoded()), serial);
+    protected static CertificateID getCertificateId(X509Certificate subjectCertificate, X509Certificate issuerCertificate) throws UserCertificateOCSPException {
+        try {
+            final BigInteger serial = subjectCertificate.getSerialNumber();
+            final DigestCalculator digestCalculator = DigestCalculatorImpl.sha1();
+            return new CertificateID(digestCalculator,
+                new X509CertificateHolder(issuerCertificate.getEncoded()), serial);
+        } catch (CertificateEncodingException | IOException | OCSPException e) {
+            throw new UserCertificateOCSPException("Unable to compute certificateId for subject certificate", e);
+        }
     }
 
     protected static String ocspStatusToString(int status) {
@@ -227,8 +240,4 @@ protected OcspClient getOcspClient() {
     protected OcspServiceProvider getOcspServiceProvider() {
         return ocspServiceProvider;
     }
-
-    protected Duration getMaxOcspResponseThisUpdateAge() {
-        return maxOcspResponseThisUpdateAge;
-    }
 }