Backport OCSP update time validation and thread-safety and CA certificate expiration fixes from main

WE2-1132

Signed-off-by: Mart Somermaa <mrts@users.noreply.github.com>

Author: mrts

pom.xml

@@ -15,7 +15,7 @@
         <maven-surefire-plugin.version>2.22.2</maven-surefire-plugin.version>
         <java.version>1.8</java.version>
         <jjwt.version>0.11.5</jjwt.version>
-        <jackson.version>2.13.4.2</jackson.version>
+        <jackson.version>2.18.5</jackson.version>
         <slf4j.version>1.7.36</slf4j.version>
         <bouncycastle.version>1.70</bouncycastle.version>
         <okhttp.version>4.10.0</okhttp.version>

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;
@@ -57,31 +57,32 @@ public static void certificateIsValidOnDate(X509Certificate cert, Date date, Str
         }
     }
 
-    public static void trustedCACertificatesAreValidOnDate(Set<TrustAnchor> trustedCACertificateAnchors, Date date) throws CertificateNotYetValidException, CertificateExpiredException {
-        for (TrustAnchor cert : trustedCACertificateAnchors) {
-            certificateIsValidOnDate(cert.getTrustedCert(), date, "Trusted CA");
-        }
-    }
-
     public static X509Certificate validateIsSignedByTrustedCA(X509Certificate certificate,
                                                               Set<TrustAnchor> trustedCACertificateAnchors,
                                                               CertStore trustedCACertificateCertStore,
-                                                              Date date) throws CertificateNotTrustedException, JceException {
+                                                              Date now) throws CertificateNotTrustedException, JceException, CertificateNotYetValidException, CertificateExpiredException {
+        certificateIsValidOnDate(certificate, now, "User");
+
         final X509CertSelector selector = new X509CertSelector();
         selector.setCertificate(certificate);
 
         try {
             final PKIXBuilderParameters pkixBuilderParameters = new PKIXBuilderParameters(trustedCACertificateAnchors, selector);
             // Certificate revocation check is intentionally disabled as we do the OCSP check with SubjectCertificateNotRevokedValidator ourselves.
             pkixBuilderParameters.setRevocationEnabled(false);
-            pkixBuilderParameters.setDate(date);
+            pkixBuilderParameters.setDate(now);
             pkixBuilderParameters.addCertStore(trustedCACertificateCertStore);
 
             // See the comment in buildCertStoreFromCertificates() below why we use the default JCE provider.
             final CertPathBuilder certPathBuilder = CertPathBuilder.getInstance(CertPathBuilder.getDefaultType());
             final PKIXCertPathBuilderResult result = (PKIXCertPathBuilderResult) certPathBuilder.build(pkixBuilderParameters);
 
-            return result.getTrustAnchor().getTrustedCert();
+            final X509Certificate trustedCACert = result.getTrustAnchor().getTrustedCert();
+
+            // Verify that the trusted CA cert is presently valid before returning the result.
+            certificateIsValidOnDate(trustedCACert, now, "Trusted CA");
+
+            return trustedCACert;
 
         } catch (InvalidAlgorithmParameterException | NoSuchAlgorithmException e) {
             throw new JceException(e);

src/main/java/eu/webeid/security/util/DateAndTime.java

@@ -45,8 +45,14 @@ public static void requirePositiveDuration(Duration duration, String fieldName)
 
     public static class DefaultClock implements Clock {
 
-        public static final Clock INSTANCE = new DefaultClock();
+        // Allows mocking of time-dependent behavior with Mockito.mockStatic() in tests.
+        private static final Clock instance = new DefaultClock();
 
+        public static Clock getInstance() {
+            return instance;
+        }
+
+        @Override
         public Date now() {
             return new Date();
         }

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

@@ -38,7 +38,6 @@
 
 import static eu.webeid.security.util.Collections.newHashSet;
 import static eu.webeid.security.util.DateAndTime.requirePositiveDuration;
-import static eu.webeid.security.validator.ocsp.OcspUrl.AIA_ESTEID_2015;
 
 /**
  * Stores configuration parameters for {@link AuthTokenValidatorImpl}.
@@ -49,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(
@@ -57,8 +58,7 @@ public final class AuthTokenValidationConfiguration {
         SubjectCertificatePolicies.ESTEID_SK_2015_MOBILE_ID_POLICY_V3,
         SubjectCertificatePolicies.ESTEID_SK_2015_MOBILE_ID_POLICY
     );
-    // Disable OCSP nonce extension for EstEID 2015 cards by default.
-    private Collection<URI> nonceDisabledOcspUrls = newHashSet(AIA_ESTEID_2015);
+    private Collection<URI> nonceDisabledOcspUrls = new HashSet<>();
 
     AuthTokenValidationConfiguration() {
     }
@@ -68,6 +68,8 @@ private AuthTokenValidationConfiguration(AuthTokenValidationConfiguration other)
         this.trustedCACertificates = Collections.unmodifiableSet(new HashSet<>(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));
@@ -101,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;
     }
@@ -130,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

@@ -77,7 +77,7 @@ public AuthTokenValidatorBuilder withTrustedCertificateAuthorities(X509Certifica
         if (LOG.isDebugEnabled()) {
             LOG.debug("Trusted intermediate certificate authorities set to {}",
                 configuration.getTrustedCACertificates().stream()
-                    .map(X509Certificate::getSubjectDN)
+                    .map(X509Certificate::getSubjectX500Principal)
                     .collect(Collectors.toList()));
         }
         return this;
@@ -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
+     * @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

@@ -23,13 +23,13 @@
 package eu.webeid.security.validator;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectReader;
 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.validator.certvalidators.SubjectCertificateExpiryValidator;
 import eu.webeid.security.validator.certvalidators.SubjectCertificateNotRevokedValidator;
 import eu.webeid.security.validator.certvalidators.SubjectCertificatePolicyValidator;
 import eu.webeid.security.validator.certvalidators.SubjectCertificatePurposeValidator;
@@ -57,7 +57,7 @@ final class AuthTokenValidatorImpl implements AuthTokenValidator {
     private static final int TOKEN_MAX_LENGTH = 10000;
     private static final Logger LOG = LoggerFactory.getLogger(AuthTokenValidatorImpl.class);
 
-    private static final ObjectMapper objectMapper = new ObjectMapper();
+    private static final ObjectReader OBJECT_READER = new ObjectMapper().readerFor(WebEidAuthToken.class);
 
     private final AuthTokenValidationConfiguration configuration;
     private final SubjectCertificateValidatorBatch simpleSubjectCertificateValidators;
@@ -84,7 +84,6 @@ final class AuthTokenValidatorImpl implements AuthTokenValidator {
         trustedCACertificateCertStore = CertificateValidator.buildCertStoreFromCertificates(configuration.getTrustedCACertificates());
 
         simpleSubjectCertificateValidators = SubjectCertificateValidatorBatch.createFrom(
-            new SubjectCertificateExpiryValidator(trustedCACertificateAnchors)::validateCertificateExpiry,
             SubjectCertificatePurposeValidator::validateCertificatePurpose,
             new SubjectCertificatePolicyValidator(configuration.getDisallowedSubjectCertificatePolicies())::validateCertificatePolicies
         );
@@ -109,7 +108,7 @@ public WebEidAuthToken parse(String authToken) throws AuthTokenException {
             validateTokenLength(authToken);
             return parseToken(authToken);
         } catch (Exception e) {
-            // Generally "log and rethrow" is an anti-pattern, but it fits with the surrounding logging style.
+            // Generally "log and rethrow" is an antipattern, but it fits with the surrounding logging style.
             LOG.warn("Token parsing was interrupted:", e);
             throw e;
         }
@@ -121,7 +120,7 @@ public X509Certificate validate(WebEidAuthToken authToken, String currentChallen
             LOG.info("Starting token validation");
             return validateToken(authToken, currentChallengeNonce);
         } catch (Exception e) {
-            // Generally "log and rethrow" is an anti-pattern, but it fits with the surrounding logging style.
+            // Generally "log and rethrow" is an antipattern, but it fits with the surrounding logging style.
             LOG.warn("Token validation was interrupted:", e);
             throw e;
         }
@@ -138,7 +137,7 @@ private void validateTokenLength(String authToken) throws AuthTokenParseExceptio
 
     private WebEidAuthToken parseToken(String authToken) throws AuthTokenParseException {
         try {
-            final WebEidAuthToken token = objectMapper.readValue(authToken, WebEidAuthToken.class);
+            final WebEidAuthToken token = OBJECT_READER.readValue(authToken);
             if (token == null) {
                 throw new AuthTokenParseException("Web eID authentication token is null");
             }
@@ -185,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
         );
     }