Refactor OcspResponseValidator.validateCertificateStatusUpdateTime(),…

… make OCSP response time validation parameters configurable

WE2-868

Signed-off-by: Mart Somermaa <[email protected]>

README.md

@@ -302,6 +302,8 @@ The following additional configuration options are available in `AuthTokenValida
 - `withOcspRequestTimeout(Duration ocspRequestTimeout)` – sets both the connection and response timeout of user certificate revocation check OCSP requests. Default is 5 seconds.
 - `withDisallowedCertificatePolicies(ASN1ObjectIdentifier... policies)` – adds the given policies to the list of disallowed user certificate policies. In order for the user certificate to be considered valid, it must not contain any policies present in this list. Contains the Estonian Mobile-ID policies by default as it must not be possible to authenticate with a Mobile-ID certificate when an eID smart card is expected.
 - `withNonceDisabledOcspUrls(URI... urls)` – adds the given URLs to the list of OCSP responder access location URLs for which the nonce protocol extension will be disabled. Some OCSP responders don't support the nonce extension.
+- `withAllowedOcspResponseTimeSkew(Duration allowedTimeSkew)` – sets the allowed time skew for OCSP response's `thisUpdate` and `nextUpdate` times to allow discrepancies between the system clock and the OCSP responder's clock or revocation updates that are not published in real time. The default allowed time skew is 15 minutes. The relatively long default is specifically chosen to account for one particular OCSP responder that used CRLs for authoritative revocation info, these CRLs were updated every 15 minutes.
+- `withMaxOcspResponseThisUpdateAge(Duration maxThisUpdateAge)` – sets the maximum age for the OCSP response's `thisUpdate` time before it is considered too old to rely on. The default maximum age is 2 minutes.
 
 Extended configuration example:  
 
@@ -312,6 +314,8 @@ AuthTokenValidator validator = new AuthTokenValidatorBuilder()
     .withoutUserCertificateRevocationCheckWithOcsp()
     .withDisallowedCertificatePolicies(new ASN1ObjectIdentifier("1.2.3"))
     .withNonceDisabledOcspUrls(URI.create("http://aia.example.org/cert"))
+    .withAllowedOcspResponseTimeSkew(Duration.ofMinutes(10))
+    .withMaxOcspResponseThisUpdateAge(Duration.ofMinutes(5))
     .build();
 ```
 

src/main/java/eu/webeid/security/certificate/CertificateValidator.java

@@ -23,9 +23,9 @@
 package eu.webeid.security.certificate;
 
 import eu.webeid.security.exceptions.CertificateExpiredException;
+import eu.webeid.security.exceptions.CertificateNotTrustedException;
 import eu.webeid.security.exceptions.CertificateNotYetValidException;
 import eu.webeid.security.exceptions.JceException;
-import eu.webeid.security.exceptions.CertificateNotTrustedException;
 
 import java.security.GeneralSecurityException;
 import java.security.InvalidAlgorithmParameterException;
@@ -40,7 +40,6 @@
 import java.security.cert.X509CertSelector;
 import java.security.cert.X509Certificate;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Date;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -91,9 +90,8 @@ public static X509Certificate validateIsSignedByTrustedCA(X509Certificate certif
     }
 
     public static Set<TrustAnchor> buildTrustAnchorsFromCertificates(Collection<X509Certificate> certificates) {
-        return Collections.unmodifiableSet(certificates.stream()
-            .map(cert -> new TrustAnchor(cert, null))
-            .collect(Collectors.toSet()));
+        return certificates.stream()
+            .map(cert -> new TrustAnchor(cert, null)).collect(Collectors.toUnmodifiableSet());
     }
 
     public static CertStore buildCertStoreFromCertificates(Collection<X509Certificate> certificates) throws JceException {

src/main/java/eu/webeid/security/validator/AuthTokenValidationConfiguration.java

@@ -32,9 +32,9 @@
 import java.security.cert.X509Certificate;
 import java.time.Duration;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.Objects;
+import java.util.Set;
 
 import static eu.webeid.security.util.Collections.newHashSet;
 import static eu.webeid.security.util.DateAndTime.requirePositiveDuration;
@@ -48,6 +48,8 @@ public final class AuthTokenValidationConfiguration {
     private Collection<X509Certificate> trustedCACertificates = new HashSet<>();
     private boolean isUserCertificateRevocationCheckWithOcspEnabled = true;
     private Duration ocspRequestTimeout = Duration.ofSeconds(5);
+    private Duration allowedOcspResponseTimeSkew = Duration.ofMinutes(15);
+    private Duration maxOcspResponseThisUpdateAge = Duration.ofMinutes(2);
     private DesignatedOcspServiceConfiguration designatedOcspServiceConfiguration;
     // Don't allow Estonian Mobile-ID policy by default.
     private Collection<ASN1ObjectIdentifier> disallowedSubjectCertificatePolicies = newHashSet(
@@ -63,12 +65,14 @@ public final class AuthTokenValidationConfiguration {
 
     private AuthTokenValidationConfiguration(AuthTokenValidationConfiguration other) {
         this.siteOrigin = other.siteOrigin;
-        this.trustedCACertificates = Collections.unmodifiableSet(new HashSet<>(other.trustedCACertificates));
+        this.trustedCACertificates = Set.copyOf(other.trustedCACertificates);
         this.isUserCertificateRevocationCheckWithOcspEnabled = other.isUserCertificateRevocationCheckWithOcspEnabled;
         this.ocspRequestTimeout = other.ocspRequestTimeout;
+        this.allowedOcspResponseTimeSkew = other.allowedOcspResponseTimeSkew;
+        this.maxOcspResponseThisUpdateAge = other.maxOcspResponseThisUpdateAge;
         this.designatedOcspServiceConfiguration = other.designatedOcspServiceConfiguration;
-        this.disallowedSubjectCertificatePolicies = Collections.unmodifiableSet(new HashSet<>(other.disallowedSubjectCertificatePolicies));
-        this.nonceDisabledOcspUrls = Collections.unmodifiableSet(new HashSet<>(other.nonceDisabledOcspUrls));
+        this.disallowedSubjectCertificatePolicies = Set.copyOf(other.disallowedSubjectCertificatePolicies);
+        this.nonceDisabledOcspUrls = Set.copyOf(other.nonceDisabledOcspUrls);
     }
 
     void setSiteOrigin(URI siteOrigin) {
@@ -99,6 +103,22 @@ void setOcspRequestTimeout(Duration ocspRequestTimeout) {
         this.ocspRequestTimeout = ocspRequestTimeout;
     }
 
+    public Duration getAllowedOcspResponseTimeSkew() {
+        return allowedOcspResponseTimeSkew;
+    }
+
+    public void setAllowedOcspResponseTimeSkew(Duration allowedOcspResponseTimeSkew) {
+        this.allowedOcspResponseTimeSkew = allowedOcspResponseTimeSkew;
+    }
+
+    public Duration getMaxOcspResponseThisUpdateAge() {
+        return maxOcspResponseThisUpdateAge;
+    }
+
+    public void setMaxOcspResponseThisUpdateAge(Duration maxOcspResponseThisUpdateAge) {
+        this.maxOcspResponseThisUpdateAge = maxOcspResponseThisUpdateAge;
+    }
+
     public DesignatedOcspServiceConfiguration getDesignatedOcspServiceConfiguration() {
         return designatedOcspServiceConfiguration;
     }
@@ -128,6 +148,8 @@ void validate() {
             throw new IllegalArgumentException("At least one trusted certificate authority must be provided");
         }
         requirePositiveDuration(ocspRequestTimeout, "OCSP request timeout");
+        requirePositiveDuration(allowedOcspResponseTimeSkew, "Allowed OCSP response time-skew");
+        requirePositiveDuration(maxOcspResponseThisUpdateAge, "Max OCSP response thisUpdate age");
     }
 
     AuthTokenValidationConfiguration copy() {

src/main/java/eu/webeid/security/validator/AuthTokenValidatorBuilder.java

@@ -127,6 +127,38 @@ public AuthTokenValidatorBuilder withOcspRequestTimeout(Duration ocspRequestTime
         return this;
     }
 
+    /**
+     * Sets the allowed time skew for OCSP response's thisUpdate and nextUpdate times.
+     * This parameter is used to allow discrepancies between the system clock and the OCSP responder's clock,
+     * which may occur due to clock drift, network delays or revocation updates that are not published in real time.
+     * <p>
+     * This is an optional configuration parameter, the default is 15 minutes.
+     * The relatively long default is specifically chosen to account for one particular OCSP responder that used
+     * CRLs for authoritative revocation info, these CRLs were updated every 15 minutes.
+     *
+     * @param allowedTimeSkew the allowed time skew
+     * @return the builder instance for method chaining.
+     */
+    public AuthTokenValidatorBuilder withAllowedOcspResponseTimeSkew(Duration allowedTimeSkew) {
+        configuration.setAllowedOcspResponseTimeSkew(allowedTimeSkew);
+        LOG.debug("Allowed OCSP response time skew set to {}", allowedTimeSkew);
+        return this;
+    }
+
+    /**
+     * Sets the maximum age of the OCSP response's thisUpdate time before it is considered too old.
+     * <p>
+     * This is an optional configuration parameter, the default is 2 minutes.
+     *
+     * @param maxThisUpdateAge the maximum age of the OCSP response's thisUpdate time in milliseconds
+     * @return the builder instance for method chaining.
+     */
+    public AuthTokenValidatorBuilder withMaxOcspResponseThisUpdateAge(Duration maxThisUpdateAge) {
+        configuration.setMaxOcspResponseThisUpdateAge(maxThisUpdateAge);
+        LOG.debug("Maximum OCSP response thisUpdate age set to {}", maxThisUpdateAge);
+        return this;
+    }
+
     /**
      * Adds the given URLs to the list of OCSP URLs for which the nonce protocol extension will be disabled.
      * The OCSP URL is extracted from the user certificate and some OCSP services don't support the nonce extension.

src/main/java/eu/webeid/security/validator/AuthTokenValidatorImpl.java

@@ -27,9 +27,9 @@
 import eu.webeid.security.authtoken.WebEidAuthToken;
 import eu.webeid.security.certificate.CertificateLoader;
 import eu.webeid.security.certificate.CertificateValidator;
-import eu.webeid.security.exceptions.JceException;
-import eu.webeid.security.exceptions.AuthTokenParseException;
 import eu.webeid.security.exceptions.AuthTokenException;
+import eu.webeid.security.exceptions.AuthTokenParseException;
+import eu.webeid.security.exceptions.JceException;
 import eu.webeid.security.validator.certvalidators.SubjectCertificateExpiryValidator;
 import eu.webeid.security.validator.certvalidators.SubjectCertificateNotRevokedValidator;
 import eu.webeid.security.validator.certvalidators.SubjectCertificatePolicyValidator;
@@ -64,10 +64,8 @@ final class AuthTokenValidatorImpl implements AuthTokenValidator {
     private final SubjectCertificateValidatorBatch simpleSubjectCertificateValidators;
     private final Set<TrustAnchor> trustedCACertificateAnchors;
     private final CertStore trustedCACertificateCertStore;
-    // OcspClient uses OkHttp internally.
-    // OkHttp performs best when a single OkHttpClient instance is created and reused for all HTTP calls.
-    // This is because each client holds its own connection pool and thread pools.
-    // Reusing connections and threads reduces latency and saves memory.
+    // OcspClient uses built-in HttpClient internally by default.
+    // A single HttpClient instance is reused for all HTTP calls to utilize connection and thread pools.
     private OcspClient ocspClient;
     private OcspServiceProvider ocspServiceProvider;
     private final AuthTokenSignatureValidator authTokenSignatureValidator;
@@ -186,7 +184,11 @@ private SubjectCertificateValidatorBatch getCertTrustValidators() {
         return SubjectCertificateValidatorBatch.createFrom(
             certTrustedValidator::validateCertificateTrusted
         ).addOptional(configuration.isUserCertificateRevocationCheckWithOcspEnabled(),
-            new SubjectCertificateNotRevokedValidator(certTrustedValidator, ocspClient, ocspServiceProvider)::validateCertificateNotRevoked
+            new SubjectCertificateNotRevokedValidator(certTrustedValidator,
+                ocspClient, ocspServiceProvider,
+                configuration.getAllowedOcspResponseTimeSkew(),
+                configuration.getMaxOcspResponseThisUpdateAge()
+            )::validateCertificateNotRevoked
         );
     }
 

src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificateNotRevokedValidator.java

@@ -52,6 +52,7 @@
 import java.security.cert.CertificateEncodingException;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
+import java.time.Duration;
 import java.util.Date;
 import java.util.Objects;
 
@@ -63,17 +64,23 @@ public final class SubjectCertificateNotRevokedValidator {
     private final SubjectCertificateTrustedValidator trustValidator;
     private final OcspClient ocspClient;
     private final OcspServiceProvider ocspServiceProvider;
+    private final Duration allowedOcspResponseTimeSkew;
+    private final Duration maxOcspResponseThisUpdateAge;
 
     static {
         Security.addProvider(new BouncyCastleProvider());
     }
 
     public SubjectCertificateNotRevokedValidator(SubjectCertificateTrustedValidator trustValidator,
                                                  OcspClient ocspClient,
-                                                 OcspServiceProvider ocspServiceProvider) {
+                                                 OcspServiceProvider ocspServiceProvider,
+                                                 Duration allowedOcspResponseTimeSkew,
+                                                 Duration maxOcspResponseThisUpdateAge) {
         this.trustValidator = trustValidator;
         this.ocspClient = ocspClient;
         this.ocspServiceProvider = ocspServiceProvider;
+        this.allowedOcspResponseTimeSkew = allowedOcspResponseTimeSkew;
+        this.maxOcspResponseThisUpdateAge = maxOcspResponseThisUpdateAge;
     }
 
     /**
@@ -166,7 +173,7 @@ private void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspSer
         //      be available about the status of the certificate (nextUpdate) is
         //      greater than the current time.
 
-        OcspResponseValidator.validateCertificateStatusUpdateTime(certStatusResponse);
+        OcspResponseValidator.validateCertificateStatusUpdateTime(certStatusResponse, allowedOcspResponseTimeSkew, maxOcspResponseThisUpdateAge);
 
         // Now we can accept the signed response as valid and validate the certificate status.
         OcspResponseValidator.validateSubjectCertificateStatus(certStatusResponse);

src/main/java/eu/webeid/security/validator/ocsp/OcspResponseValidator.java

@@ -40,11 +40,10 @@
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateParsingException;
 import java.security.cert.X509Certificate;
-import java.text.SimpleDateFormat;
+import java.time.Duration;
+import java.time.Instant;
 import java.util.Date;
 import java.util.Objects;
-import java.util.TimeZone;
-import java.util.concurrent.TimeUnit;
 
 public final class OcspResponseValidator {
 
@@ -54,8 +53,7 @@ public final class OcspResponseValidator {
      * https://oidref.com/1.3.6.1.5.5.7.3.9
      */
     private static final String OID_OCSP_SIGNING = "1.3.6.1.5.5.7.3.9";
-
-    static final long ALLOWED_TIME_SKEW_MILLIS = TimeUnit.MINUTES.toMillis(15);
+    private static final String ERROR_PREFIX = "Certificate status update time check failed: ";
 
     public static void validateHasSigningExtension(X509Certificate certificate) throws OCSPCertificateException {
         Objects.requireNonNull(certificate, "certificate");
@@ -78,7 +76,7 @@ public static void validateResponseSignature(BasicOCSPResp basicResponse, X509Ce
         }
     }
 
-    public static void validateCertificateStatusUpdateTime(SingleResp certStatusResponse) throws UserCertificateOCSPCheckFailedException {
+    public static void validateCertificateStatusUpdateTime(SingleResp certStatusResponse, Duration allowedTimeSkewMillis, Duration maxThisupdateAgeMillis) throws UserCertificateOCSPCheckFailedException {
         // From RFC 2560, https://www.ietf.org/rfc/rfc2560.txt:
         // 4.2.2.  Notes on OCSP Responses
         // 4.2.2.1.  Time
@@ -88,18 +86,34 @@ public static void validateCertificateStatusUpdateTime(SingleResp certStatusResp
         //   SHOULD be considered unreliable.
         //   If nextUpdate is not set, the responder is indicating that newer
         //   revocation information is available all the time.
-        final Date now = DateAndTime.DefaultClock.getInstance().now();
-        final Date notAllowedBefore = new Date(now.getTime() - ALLOWED_TIME_SKEW_MILLIS);
-        final Date notAllowedAfter = new Date(now.getTime() + ALLOWED_TIME_SKEW_MILLIS);
-        final Date thisUpdate = certStatusResponse.getThisUpdate();
-        final Date nextUpdate = certStatusResponse.getNextUpdate() != null ? certStatusResponse.getNextUpdate() : thisUpdate;
-        if (notAllowedAfter.before(thisUpdate) ||
-            notAllowedBefore.after(nextUpdate)) {
-            throw new UserCertificateOCSPCheckFailedException("Certificate status update time check failed: " +
-                "notAllowedBefore: " + toUtcString(notAllowedBefore) +
-                ", notAllowedAfter: " + toUtcString(notAllowedAfter) +
-                ", thisUpdate: " + toUtcString(thisUpdate) +
-                ", nextUpdate: " + toUtcString(certStatusResponse.getNextUpdate()));
+        final Instant now = DateAndTime.DefaultClock.getInstance().now().toInstant();
+        final Instant earliestAcceptableTimeSkew = now.minus(allowedTimeSkewMillis);
+        final Instant latestAcceptableTimeSkew = now.plus(allowedTimeSkewMillis);
+        final Instant earliestAcceptableThisUpdateTime = now.minus(maxThisupdateAgeMillis);
+
+        final Instant thisUpdate = certStatusResponse.getThisUpdate().toInstant();
+        if (thisUpdate.isAfter(latestAcceptableTimeSkew)) {
+            throw new UserCertificateOCSPCheckFailedException(ERROR_PREFIX +
+                "thisUpdate '" + thisUpdate + "' is too far in the future, " +
+                "latest allowed: '" + latestAcceptableTimeSkew + "'");
+        }
+        if (thisUpdate.isBefore(earliestAcceptableThisUpdateTime)) {
+            throw new UserCertificateOCSPCheckFailedException(ERROR_PREFIX +
+                "thisUpdate '" + thisUpdate + "' is too old, " +
+                "earliest allowed: '" + earliestAcceptableThisUpdateTime + "'");
+        }
+
+        if (certStatusResponse.getNextUpdate() == null) {
+            return;
+        }
+        final Instant nextUpdate = certStatusResponse.getNextUpdate().toInstant();
+        if (nextUpdate.isBefore(earliestAcceptableTimeSkew)) {
+            throw new UserCertificateOCSPCheckFailedException(ERROR_PREFIX +
+                "nextUpdate '" + nextUpdate + "' is in the past");
+        }
+        if (nextUpdate.isBefore(thisUpdate)) {
+            throw new UserCertificateOCSPCheckFailedException(ERROR_PREFIX +
+                "nextUpdate '" + nextUpdate + "' is before thisUpdate '" + thisUpdate + "'");
         }
     }
 
@@ -120,15 +134,6 @@ public static void validateSubjectCertificateStatus(SingleResp certStatusRespons
         }
     }
 
-    static String toUtcString(Date date) {
-        if (date == null) {
-            return String.valueOf((Object) null);
-        }
-        final SimpleDateFormat dateFormatter = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss z");
-        dateFormatter.setTimeZone(TimeZone.getTimeZone("UTC"));
-        return dateFormatter.format(date);
-    }
-
     private OcspResponseValidator() {
         throw new IllegalStateException("Utility class");
     }

src/test/java/eu/webeid/security/testutil/AuthTokenValidators.java

@@ -89,6 +89,10 @@ public static AuthTokenValidator getAuthTokenValidatorWithDisallowedESTEIDPolicy
             .build();
     }
 
+    public static AuthTokenValidatorBuilder getDefaultAuthTokenValidatorBuilder() throws CertificateException, IOException {
+        return getAuthTokenValidatorBuilder(TOKEN_ORIGIN_URL, getCACertificates());
+    }
+
     private static AuthTokenValidatorBuilder getAuthTokenValidatorBuilder(String uri, X509Certificate[] certificates) {
         return new AuthTokenValidatorBuilder()
             .withSiteOrigin(URI.create(uri))