Skip to content

Commit 2bc89ea

Browse files
mrtsmrts
committed
fix: use system time in OcspResponseValidator.validateCertificateStatusUpdateTime()
WE2-868 Signed-off-by: Mart Somermaa <mrts@users.noreply.github.com>
1 parent 04723bb commit 2bc89ea

File tree

6 files changed

+101
-65
lines changed

6 files changed

+101
-65
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ private void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspSer
166166
// be available about the status of the certificate (nextUpdate) is
167167
// greater than the current time.
168168

169-
OcspResponseValidator.validateCertificateStatusUpdateTime(certStatusResponse, producedAt);
169+
OcspResponseValidator.validateCertificateStatusUpdateTime(certStatusResponse);
170170

171171
// Now we can accept the signed response as valid and validate the certificate status.
172172
OcspResponseValidator.validateSubjectCertificateStatus(certStatusResponse);

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import eu.webeid.security.exceptions.OCSPCertificateException;
2626
import eu.webeid.security.exceptions.UserCertificateOCSPCheckFailedException;
2727
import eu.webeid.security.exceptions.UserCertificateRevokedException;
28+
import eu.webeid.security.util.DateAndTime;
2829
import org.bouncycastle.cert.X509CertificateHolder;
2930
import org.bouncycastle.cert.ocsp.BasicOCSPResp;
3031
import org.bouncycastle.cert.ocsp.CertificateStatus;
@@ -54,7 +55,7 @@ public final class OcspResponseValidator {
5455
*/
5556
private static final String OID_OCSP_SIGNING = "1.3.6.1.5.5.7.3.9";
5657

57-
private static final long ALLOWED_TIME_SKEW = TimeUnit.MINUTES.toMillis(15);
58+
static final long ALLOWED_TIME_SKEW_MILLIS = TimeUnit.MINUTES.toMillis(15);
5859

5960
public static void validateHasSigningExtension(X509Certificate certificate) throws OCSPCertificateException {
6061
Objects.requireNonNull(certificate, "certificate");
@@ -77,7 +78,7 @@ public static void validateResponseSignature(BasicOCSPResp basicResponse, X509Ce
7778
}
7879
}
7980

80-
public static void validateCertificateStatusUpdateTime(SingleResp certStatusResponse, Date producedAt) throws UserCertificateOCSPCheckFailedException {
81+
public static void validateCertificateStatusUpdateTime(SingleResp certStatusResponse) throws UserCertificateOCSPCheckFailedException {
8182
// From RFC 2560, https://www.ietf.org/rfc/rfc2560.txt:
8283
// 4.2.2. Notes on OCSP Responses
8384
// 4.2.2.1. Time
@@ -87,8 +88,9 @@ public static void validateCertificateStatusUpdateTime(SingleResp certStatusResp
8788
// SHOULD be considered unreliable.
8889
// If nextUpdate is not set, the responder is indicating that newer
8990
// revocation information is available all the time.
90-
final Date notAllowedBefore = new Date(producedAt.getTime() - ALLOWED_TIME_SKEW);
91-
final Date notAllowedAfter = new Date(producedAt.getTime() + ALLOWED_TIME_SKEW);
91+
final Date now = DateAndTime.DefaultClock.getInstance().now();
92+
final Date notAllowedBefore = new Date(now.getTime() - ALLOWED_TIME_SKEW_MILLIS);
93+
final Date notAllowedAfter = new Date(now.getTime() + ALLOWED_TIME_SKEW_MILLIS);
9294
final Date thisUpdate = certStatusResponse.getThisUpdate();
9395
final Date nextUpdate = certStatusResponse.getNextUpdate() != null ? certStatusResponse.getNextUpdate() : thisUpdate;
9496
if (notAllowedAfter.before(thisUpdate) ||
@@ -118,7 +120,7 @@ public static void validateSubjectCertificateStatus(SingleResp certStatusRespons
118120
}
119121
}
120122

121-
private static String toUtcString(Date date) {
123+
static String toUtcString(Date date) {
122124
if (date == null) {
123125
return String.valueOf((Object) null);
124126
}

src/test/java/eu/webeid/security/testutil/Dates.java renamed to src/test/java/eu/webeid/security/testutil/DateMocker.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,23 @@
2323
package eu.webeid.security.testutil;
2424

2525
import com.fasterxml.jackson.databind.util.StdDateFormat;
26+
import eu.webeid.security.util.DateAndTime;
27+
import io.jsonwebtoken.Clock;
28+
import org.mockito.MockedStatic;
2629

2730
import java.text.ParseException;
2831
import java.util.Date;
2932

30-
public final class Dates {
33+
public class DateMocker {
34+
3135
private static final StdDateFormat STD_DATE_FORMAT = new StdDateFormat();
3236

33-
public static Date create(String iso8601Date) {
37+
public static void mockDate(String iso8601Date, MockedStatic<DateAndTime.DefaultClock> mockedClock) {
3438
try {
35-
return STD_DATE_FORMAT.parse(iso8601Date);
39+
final Date theDate = STD_DATE_FORMAT.parse(iso8601Date);
40+
mockedClock.when(DateAndTime.DefaultClock::getInstance).thenReturn((Clock) () -> theDate);
3641
} catch (ParseException e) {
3742
throw new RuntimeException(e);
3843
}
3944
}
40-
4145
}

‎src/test/java/eu/webeid/security/validator/AuthTokenCertificateTest.java‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,16 @@
3636
import eu.webeid.security.exceptions.UserCertificateWrongPurposeException;
3737
import eu.webeid.security.testutil.AbstractTestWithValidator;
3838
import eu.webeid.security.testutil.AuthTokenValidators;
39-
import eu.webeid.security.testutil.Dates;
4039
import eu.webeid.security.util.DateAndTime;
41-
import io.jsonwebtoken.Clock;
4240
import org.junit.jupiter.api.AfterEach;
4341
import org.junit.jupiter.api.BeforeEach;
4442
import org.junit.jupiter.api.Disabled;
4543
import org.junit.jupiter.api.Test;
4644
import org.mockito.MockedStatic;
4745

4846
import java.security.cert.CertificateException;
49-
import java.util.Date;
5047

48+
import static eu.webeid.security.testutil.DateMocker.mockDate;
5149
import static org.assertj.core.api.Assertions.assertThatThrownBy;
5250
import static org.mockito.Mockito.mockStatic;
5351

@@ -71,13 +69,14 @@ class AuthTokenCertificateTest extends AbstractTestWithValidator {
7169

7270
private MockedStatic<DateAndTime.DefaultClock> mockedClock;
7371

72+
7473
@Override
7574
@BeforeEach
7675
protected void setup() {
7776
super.setup();
7877
mockedClock = mockStatic(DateAndTime.DefaultClock.class);
7978
// Ensure that the certificates do not expire.
80-
mockDate("2021-08-01");
79+
mockDate("2021-08-01", mockedClock);
8180
}
8281

8382
@AfterEach
@@ -180,7 +179,7 @@ void whenCertificatePolicyIsDisallowed_thenValidationFails() throws Exception {
180179

181180
@Test
182181
void whenUsingOldMobileIdCertificate_thenValidationFails() throws AuthTokenException {
183-
mockDate("2021-03-01");
182+
mockDate("2021-03-01", mockedClock);
184183
final WebEidAuthToken token = replaceTokenField(AUTH_TOKEN, "X5C", OLD_MOBILE_ID_CERT);
185184
assertThatThrownBy(() -> validator
186185
.validate(token, VALID_CHALLENGE_NONCE))
@@ -222,7 +221,7 @@ void whenCertificateIsExpiredEcdsa_thenValidationFails() throws AuthTokenExcepti
222221

223222
@Test
224223
void whenUserCertificateIsNotYetValid_thenValidationFails() {
225-
mockDate("2018-10-17");
224+
mockDate("2018-10-17", mockedClock);
226225
assertThatThrownBy(() -> validator
227226
.validate(validAuthToken, VALID_CHALLENGE_NONCE))
228227
.isInstanceOf(CertificateNotYetValidException.class)
@@ -231,7 +230,7 @@ void whenUserCertificateIsNotYetValid_thenValidationFails() {
231230

232231
@Test
233232
void whenTrustedCACertificateIsNotYetValid_thenValidationFails() {
234-
mockDate("2018-08-17");
233+
mockDate("2018-08-17", mockedClock);
235234
assertThatThrownBy(() -> validator
236235
.validate(validAuthToken, VALID_CHALLENGE_NONCE))
237236
.isInstanceOf(CertificateNotYetValidException.class)
@@ -240,7 +239,7 @@ void whenTrustedCACertificateIsNotYetValid_thenValidationFails() {
240239

241240
@Test
242241
void whenUserCertificateIsNoLongerValid_thenValidationFails() {
243-
mockDate("2026-10-19");
242+
mockDate("2026-10-19", mockedClock);
244243
assertThatThrownBy(() -> validator
245244
.validate(validAuthToken, VALID_CHALLENGE_NONCE))
246245
.isInstanceOf(CertificateExpiredException.class)
@@ -249,7 +248,7 @@ void whenUserCertificateIsNoLongerValid_thenValidationFails() {
249248

250249
@Test
251250
void whenTrustedCACertificateIsNoLongerValid_thenValidationFails() {
252-
mockDate("2033-10-19");
251+
mockDate("2033-10-19", mockedClock);
253252
assertThatThrownBy(() -> validator
254253
.validate(validAuthToken, VALID_CHALLENGE_NONCE))
255254
.isInstanceOf(CertificateExpiredException.class)
@@ -259,7 +258,7 @@ void whenTrustedCACertificateIsNoLongerValid_thenValidationFails() {
259258
@Test
260259
@Disabled("A new designated test OCSP responder certificate was issued whose validity period no longer overlaps with the revoked certificate")
261260
void whenCertificateIsRevoked_thenOcspCheckFails() throws Exception {
262-
mockDate("2020-01-01");
261+
mockDate("2020-01-01", mockedClock);
263262
final AuthTokenValidator validatorWithOcspCheck = AuthTokenValidators.getAuthTokenValidatorWithOcspCheck();
264263
final WebEidAuthToken token = replaceTokenField(AUTH_TOKEN, "X5C", REVOKED_CERT);
265264
assertThatThrownBy(() -> validatorWithOcspCheck
@@ -270,7 +269,7 @@ void whenCertificateIsRevoked_thenOcspCheckFails() throws Exception {
270269
@Test
271270
@Disabled("A new designated test OCSP responder certificate was issued whose validity period no longer overlaps with the revoked certificate")
272271
void whenCertificateIsRevoked_thenOcspCheckWithDesignatedOcspServiceFails() throws Exception {
273-
mockDate("2020-01-01");
272+
mockDate("2020-01-01", mockedClock);
274273
final AuthTokenValidator validatorWithOcspCheck = AuthTokenValidators.getAuthTokenValidatorWithDesignatedOcspCheck();
275274
final WebEidAuthToken token = replaceTokenField(AUTH_TOKEN, "X5C", REVOKED_CERT);
276275
assertThatThrownBy(() -> validatorWithOcspCheck
@@ -286,9 +285,4 @@ void whenCertificateCaIsNotPartOfTrustChain_thenValidationFails() throws Excepti
286285
.isInstanceOf(CertificateNotTrustedException.class);
287286
}
288287

289-
private void mockDate(String date) {
290-
final Date theDate = Dates.create(date);
291-
mockedClock.when(DateAndTime.DefaultClock::getInstance).thenReturn((Clock) () -> theDate);
292-
}
293-
294288
}

‎src/test/java/eu/webeid/security/validator/certvalidators/SubjectCertificateNotRevokedValidatorTest.java‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import eu.webeid.security.exceptions.JceException;
2727
import eu.webeid.security.exceptions.UserCertificateOCSPCheckFailedException;
2828
import eu.webeid.security.exceptions.UserCertificateRevokedException;
29+
import eu.webeid.security.util.DateAndTime;
2930
import eu.webeid.security.validator.ocsp.OcspClient;
3031
import eu.webeid.security.validator.ocsp.OcspClientImpl;
3132
import eu.webeid.security.validator.ocsp.OcspServiceProvider;
@@ -53,11 +54,13 @@
5354

5455
import static eu.webeid.security.testutil.Certificates.getJaakKristjanEsteid2018Cert;
5556
import static eu.webeid.security.testutil.Certificates.getTestEsteid2018CA;
57+
import static eu.webeid.security.testutil.DateMocker.mockDate;
5658
import static eu.webeid.security.testutil.OcspServiceMaker.getAiaOcspServiceProvider;
5759
import static eu.webeid.security.testutil.OcspServiceMaker.getDesignatedOcspServiceProvider;
5860
import static org.assertj.core.api.Assertions.assertThatCode;
5961
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
6062
import static org.mockito.Mockito.mock;
63+
import static org.mockito.Mockito.mockStatic;
6164
import static org.mockito.Mockito.when;
6265

6366
class SubjectCertificateNotRevokedValidatorTest {
@@ -223,21 +226,27 @@ void whenOcspResponseRevoked_thenThrows() throws Exception {
223226
final SubjectCertificateNotRevokedValidator validator = getSubjectCertificateNotRevokedValidatorWithAiaOcsp(
224227
getMockedResponse(getOcspResponseBytesFromResources("ocsp_response_revoked.der"))
225228
);
226-
assertThatExceptionOfType(UserCertificateRevokedException.class)
227-
.isThrownBy(() ->
228-
validator.validateCertificateNotRevoked(estEid2018Cert))
229-
.withMessage("User certificate has been revoked: Revocation reason: 0");
229+
try (var mockedClock = mockStatic(DateAndTime.DefaultClock.class)) {
230+
mockDate("2021-09-18", mockedClock);
231+
assertThatExceptionOfType(UserCertificateRevokedException.class)
232+
.isThrownBy(() ->
233+
validator.validateCertificateNotRevoked(estEid2018Cert))
234+
.withMessage("User certificate has been revoked: Revocation reason: 0");
235+
}
230236
}
231237

232238
@Test
233239
void whenOcspResponseUnknown_thenThrows() throws Exception {
234240
final OcspServiceProvider ocspServiceProvider = getDesignatedOcspServiceProvider("https://web-eid-test.free.beeceptor.com");
235241
final HttpResponse<byte[]> response = getMockedResponse(getOcspResponseBytesFromResources("ocsp_response_unknown.der"));
236242
final SubjectCertificateNotRevokedValidator validator = getSubjectCertificateNotRevokedValidator(getMockClient(response), ocspServiceProvider);
237-
assertThatExceptionOfType(UserCertificateRevokedException.class)
238-
.isThrownBy(() ->
239-
validator.validateCertificateNotRevoked(estEid2018Cert))
240-
.withMessage("User certificate has been revoked: Unknown status");
243+
try (var mockedClock = mockStatic(DateAndTime.DefaultClock.class)) {
244+
mockDate("2021-09-18T00:16:25", mockedClock);
245+
assertThatExceptionOfType(UserCertificateRevokedException.class)
246+
.isThrownBy(() ->
247+
validator.validateCertificateNotRevoked(estEid2018Cert))
248+
.withMessage("User certificate has been revoked: Unknown status");
249+
}
241250
}
242251

243252
@Test
@@ -256,10 +265,13 @@ void whenNonceDiffers_thenThrows() throws Exception {
256265
final SubjectCertificateNotRevokedValidator validator = getSubjectCertificateNotRevokedValidatorWithAiaOcsp(
257266
getMockedResponse(getOcspResponseBytesFromResources())
258267
);
259-
assertThatExceptionOfType(UserCertificateOCSPCheckFailedException.class)
260-
.isThrownBy(() ->
261-
validator.validateCertificateNotRevoked(estEid2018Cert))
262-
.withMessage("User certificate revocation check has failed: OCSP request and response nonces differ, possible replay attack");
268+
try (var mockedClock = mockStatic(DateAndTime.DefaultClock.class)) {
269+
mockDate("2021-09-17T18:25:24", mockedClock);
270+
assertThatExceptionOfType(UserCertificateOCSPCheckFailedException.class)
271+
.isThrownBy(() ->
272+
validator.validateCertificateNotRevoked(estEid2018Cert))
273+
.withMessage("User certificate revocation check has failed: OCSP request and response nonces differ, possible replay attack");
274+
}
263275
}
264276

265277
private static byte[] buildOcspResponseBodyWithInternalErrorStatus() throws IOException {

0 commit comments

Comments
 (0)