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

WE2-1132

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

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/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/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
         );
     }
 

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

@@ -24,7 +24,12 @@
 
 import eu.webeid.security.exceptions.AuthTokenException;
 import eu.webeid.security.exceptions.UserCertificateOCSPCheckFailedException;
-import eu.webeid.security.validator.ocsp.*;
+import eu.webeid.security.util.DateAndTime;
+import eu.webeid.security.validator.ocsp.DigestCalculatorImpl;
+import eu.webeid.security.validator.ocsp.OcspClient;
+import eu.webeid.security.validator.ocsp.OcspRequestBuilder;
+import eu.webeid.security.validator.ocsp.OcspResponseValidator;
+import eu.webeid.security.validator.ocsp.OcspServiceProvider;
 import eu.webeid.security.validator.ocsp.service.OcspService;
 import org.bouncycastle.asn1.ocsp.OCSPObjectIdentifiers;
 import org.bouncycastle.asn1.ocsp.OCSPResponseStatus;
@@ -41,39 +46,41 @@
 import org.bouncycastle.operator.OperatorCreationException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import eu.webeid.security.validator.ocsp.Digester;
-import eu.webeid.security.validator.ocsp.OcspClient;
-import eu.webeid.security.validator.ocsp.OcspRequestBuilder;
-import eu.webeid.security.validator.ocsp.OcspServiceProvider;
 
 import java.io.IOException;
 import java.math.BigInteger;
 import java.security.Security;
 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;
 
 public final class SubjectCertificateNotRevokedValidator {
 
     private static final Logger LOG = LoggerFactory.getLogger(SubjectCertificateNotRevokedValidator.class);
-    private static final DigestCalculator DIGEST_CALCULATOR = Digester.sha1();
 
     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;
     }
 
     /**
@@ -86,10 +93,6 @@ public void validateCertificateNotRevoked(X509Certificate subjectCertificate) th
         try {
             OcspService ocspService = ocspServiceProvider.getService(subjectCertificate);
 
-            if (!ocspService.doesSupportNonce()) {
-                LOG.debug("Disabling OCSP nonce extension");
-            }
-
             final CertificateID certificateId = getCertificateId(subjectCertificate,
                 Objects.requireNonNull(trustValidator.getSubjectCertificateIssuerCertificate()));
 
@@ -98,6 +101,10 @@ public void validateCertificateNotRevoked(X509Certificate subjectCertificate) th
                 .enableOcspNonce(ocspService.doesSupportNonce())
                 .build();
 
+            if (!ocspService.doesSupportNonce()) {
+                LOG.debug("Disabling OCSP nonce extension");
+            }
+
             LOG.debug("Sending OCSP request");
             final OCSPResp response = Objects.requireNonNull(ocspClient.request(ocspService.getAccessLocation(), request));
             if (response.getStatus() != OCSPResponseStatus.SUCCESSFUL) {
@@ -156,8 +163,9 @@ private void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspSer
         //   4. The signer is currently authorized to provide a response for the
         //      certificate in question.
 
-        final Date producedAt = basicResponse.getProducedAt();
-        ocspService.validateResponderCertificate(responderCert, producedAt);
+        // Use the clock instance so that the date can be mocked in tests.
+        final Date now = DateAndTime.DefaultClock.getInstance().now();
+        ocspService.validateResponderCertificate(responderCert, now);
 
         //   5. The time at which the status being indicated is known to be
         //      correct (thisUpdate) is sufficiently recent.
@@ -166,7 +174,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, producedAt);
+        OcspResponseValidator.validateCertificateStatusUpdateTime(certStatusResponse, allowedOcspResponseTimeSkew, maxOcspResponseThisUpdateAge);
 
         // Now we can accept the signed response as valid and validate the certificate status.
         OcspResponseValidator.validateSubjectCertificateStatus(certStatusResponse);
@@ -188,7 +196,8 @@ private static void checkNonce(OCSPReq request, BasicOCSPResp response) throws U
 
     private static CertificateID getCertificateId(X509Certificate subjectCertificate, X509Certificate issuerCertificate) throws CertificateEncodingException, IOException, OCSPException {
         final BigInteger serial = subjectCertificate.getSerialNumber();
-        return new CertificateID(DIGEST_CALCULATOR,
+        final DigestCalculator digestCalculator = DigestCalculatorImpl.sha1();
+        return new CertificateID(digestCalculator,
             new X509CertificateHolder(issuerCertificate.getEncoded()), serial);
     }