Skip to content

Commit 748d97a

Browse files
aarmammadislm
authored andcommitted
Improve resilient OCSP certificate revocation checker
Signed-off-by: Madis Jaagup Laurson <madisjaagup.laurson@nortal.com>
1 parent d298a7f commit 748d97a

42 files changed

Lines changed: 2964 additions & 387 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎pom.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
<junit-jupiter.version>5.14.4</junit-jupiter.version>
2323
<assertj.version>3.27.7</assertj.version>
2424
<mockito.version>5.23.0</mockito.version>
25+
<awaitility.version>4.3.0</awaitility.version>
2526
<maven-surefire-plugin.version>3.6.0</maven-surefire-plugin.version>
2627
<maven-compiler-plugin.version>3.15.0</maven-compiler-plugin.version>
2728
<maven-source-plugin.version>3.4.0</maven-source-plugin.version>
@@ -115,6 +116,12 @@
115116
<version>${mockito.version}</version>
116117
<scope>test</scope>
117118
</dependency>
119+
<dependency>
120+
<groupId>org.awaitility</groupId>
121+
<artifactId>awaitility</artifactId>
122+
<version>${awaitility.version}</version>
123+
<scope>test</scope>
124+
</dependency>
118125
<dependency>
119126
<groupId>org.slf4j</groupId>
120127
<artifactId>slf4j-simple</artifactId>

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

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import eu.webeid.ocsp.protocol.OcspResponseValidator;
1111
import eu.webeid.security.exceptions.AuthTokenException;
1212
import eu.webeid.ocsp.exceptions.UserCertificateOCSPCheckFailedException;
13+
import eu.webeid.ocsp.exceptions.UserCertificateOCSPException;
1314
import eu.webeid.security.util.DateAndTime;
1415
import eu.webeid.ocsp.service.OcspServiceProvider;
1516
import eu.webeid.ocsp.service.OcspService;
@@ -50,13 +51,13 @@ public class OcspCertificateRevocationChecker implements CertificateRevocationCh
5051

5152
public static final Duration DEFAULT_TIME_SKEW = Duration.ofMinutes(15);
5253
public static final Duration DEFAULT_THIS_UPDATE_AGE = Duration.ofMinutes(2);
54+
public static final Duration DEFAULT_NEXT_UPDATE_AGE = Duration.ofMinutes(15);
5355

5456
private static final Logger LOG = LoggerFactory.getLogger(OcspCertificateRevocationChecker.class);
5557

5658
private final OcspClient ocspClient;
5759
private final OcspServiceProvider ocspServiceProvider;
5860
private final Duration allowedOcspResponseTimeSkew;
59-
private final Duration maxOcspResponseThisUpdateAge;
6061

6162
static {
6263
if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) {
@@ -66,12 +67,10 @@ public class OcspCertificateRevocationChecker implements CertificateRevocationCh
6667

6768
public OcspCertificateRevocationChecker(OcspClient ocspClient,
6869
OcspServiceProvider ocspServiceProvider,
69-
Duration allowedOcspResponseTimeSkew,
70-
Duration maxOcspResponseThisUpdateAge) {
70+
Duration allowedOcspResponseTimeSkew) {
7171
this.ocspClient = requireNonNull(ocspClient, "ocspClient");
7272
this.ocspServiceProvider = requireNonNull(ocspServiceProvider, "ocspServiceProvider");
7373
this.allowedOcspResponseTimeSkew = requirePositiveDuration(allowedOcspResponseTimeSkew, "allowedOcspResponseTimeSkew");
74-
this.maxOcspResponseThisUpdateAge = requirePositiveDuration(maxOcspResponseThisUpdateAge, "maxOcspResponseThisUpdateAge");
7574
}
7675

7776
/**
@@ -86,22 +85,16 @@ public List<RevocationInfo> validateCertificateNotRevoked(X509Certificate subjec
8685
requireNonNull(subjectCertificate, "subjectCertificate");
8786
requireNonNull(issuerCertificate, "issuerCertificate");
8887

89-
URI ocspResponderUri = null;
90-
try {
91-
OcspService ocspService = ocspServiceProvider.getService(subjectCertificate, issuerCertificate);
92-
ocspResponderUri = requireNonNull(ocspService.getAccessLocation(), "ocspResponderUri");
93-
94-
final CertificateID certificateId = getCertificateId(subjectCertificate, issuerCertificate);
95-
96-
final OCSPReq request = new OcspRequestBuilder()
97-
.withCertificateId(certificateId)
98-
.enableOcspNonce(ocspService.doesSupportNonce())
99-
.build();
88+
final OcspService ocspService = ocspServiceProvider.getService(subjectCertificate, issuerCertificate);
89+
final CertificateID certificateId = getCertificateId(subjectCertificate, issuerCertificate);
90+
final URI ocspResponderUri = ocspService.getAccessLocation();
91+
final OCSPReq request = getOcspRequest(certificateId, ocspService);
10092

101-
if (!ocspService.doesSupportNonce()) {
102-
LOG.debug("Disabling OCSP nonce extension");
103-
}
93+
if (!ocspService.doesSupportNonce()) {
94+
LOG.debug("Disabling OCSP nonce extension");
95+
}
10496

97+
try {
10598
LOG.debug("Sending OCSP request");
10699
final OCSPResp response = requireNonNull(ocspClient.request(ocspResponderUri, request), "OCSPResp");
107100
if (response.getStatus() != OCSPResponseStatus.SUCCESSFUL) {
@@ -113,20 +106,33 @@ public List<RevocationInfo> validateCertificateNotRevoked(X509Certificate subjec
113106
}
114107
LOG.debug("OCSP response received successfully");
115108

116-
verifyOcspResponse(basicResponse, ocspService, certificateId, issuerCertificate, maxOcspResponseThisUpdateAge);
109+
verifyOcspResponse(basicResponse, ocspService, certificateId, issuerCertificate);
117110
if (ocspService.doesSupportNonce()) {
118111
checkNonce(request, basicResponse, ocspResponderUri);
119112
}
120113
LOG.debug("OCSP response verified successfully");
121114

122115
return List.of(new RevocationInfo(ocspResponderUri, Map.of(RevocationInfo.KEY_OCSP_RESPONSE, response)));
123116

124-
} catch (OCSPException | CertificateException | OperatorCreationException | IOException | OCSPClientException e) {
117+
} catch (OCSPException | CertificateException | OperatorCreationException | OCSPClientException e) {
125118
throw new UserCertificateOCSPCheckFailedException(e, ocspResponderUri);
126119
}
127120
}
128121

129-
protected void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspService, CertificateID requestCertificateId, X509Certificate issuerCertificate, Duration maxOcspResponseThisUpdateAge) throws AuthTokenException, OCSPException, CertificateException, OperatorCreationException {
122+
protected static OCSPReq getOcspRequest(CertificateID certificateId, OcspService ocspService) throws UserCertificateOCSPException {
123+
final OCSPReq request;
124+
try {
125+
request = new OcspRequestBuilder()
126+
.withCertificateId(certificateId)
127+
.enableOcspNonce(ocspService.doesSupportNonce())
128+
.build();
129+
} catch (OCSPException e) {
130+
throw new UserCertificateOCSPException("Unable to create OCSP request", e);
131+
}
132+
return request;
133+
}
134+
135+
protected void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspService, CertificateID requestCertificateId, X509Certificate issuerCertificate) throws AuthTokenException, OCSPException, CertificateException, OperatorCreationException {
130136
// The verification algorithm follows RFC 2560, https://www.ietf.org/rfc/rfc2560.txt.
131137
//
132138
// 3.2. Signed Response Acceptance Requirements
@@ -182,7 +188,7 @@ protected void verifyOcspResponse(BasicOCSPResp basicResponse, OcspService ocspS
182188
// be available about the status of the certificate (nextUpdate) is
183189
// greater than the current time.
184190

185-
OcspResponseValidator.validateCertificateStatusUpdateTime(certStatusResponse, allowedOcspResponseTimeSkew, maxOcspResponseThisUpdateAge, ocspService.getAccessLocation());
191+
OcspResponseValidator.validateCertificateStatusUpdateTime(certStatusResponse, allowedOcspResponseTimeSkew, ocspService.getMaxThisUpdateAge(), ocspService.getMaxNextUpdateAge(), ocspService.getAccessLocation());
186192

187193
// Now we can accept the signed response as valid and validate the certificate status.
188194
OcspResponseValidator.validateSubjectCertificateStatus(certStatusResponse, ocspService.getAccessLocation());
@@ -202,11 +208,15 @@ protected static void checkNonce(OCSPReq request, BasicOCSPResp response, URI oc
202208
}
203209
}
204210

205-
protected static CertificateID getCertificateId(X509Certificate subjectCertificate, X509Certificate issuerCertificate) throws CertificateEncodingException, IOException, OCSPException {
206-
final BigInteger serial = subjectCertificate.getSerialNumber();
207-
final DigestCalculator digestCalculator = DigestCalculatorImpl.sha1();
208-
return new CertificateID(digestCalculator,
209-
new X509CertificateHolder(issuerCertificate.getEncoded()), serial);
211+
protected static CertificateID getCertificateId(X509Certificate subjectCertificate, X509Certificate issuerCertificate) throws UserCertificateOCSPException {
212+
try {
213+
final BigInteger serial = subjectCertificate.getSerialNumber();
214+
final DigestCalculator digestCalculator = DigestCalculatorImpl.sha1();
215+
return new CertificateID(digestCalculator,
216+
new X509CertificateHolder(issuerCertificate.getEncoded()), serial);
217+
} catch (CertificateEncodingException | IOException | OCSPException e) {
218+
throw new UserCertificateOCSPException("Unable to compute certificateId for subject certificate", e);
219+
}
210220
}
211221

212222
protected static String ocspStatusToString(int status) {
@@ -227,8 +237,4 @@ protected OcspClient getOcspClient() {
227237
protected OcspServiceProvider getOcspServiceProvider() {
228238
return ocspServiceProvider;
229239
}
230-
231-
protected Duration getMaxOcspResponseThisUpdateAge() {
232-
return maxOcspResponseThisUpdateAge;
233-
}
234240
}

‎src/main/java/eu/webeid/ocsp/README.md‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,15 @@ List<X509Certificate> trustedCAs = List.of(trustedIntermediateCACertificates());
7575
AiaOcspServiceConfiguration aiaConfiguration = new AiaOcspServiceConfiguration(
7676
Set.of(), // AIA responder URLs for which request and response nonce checks are disabled.
7777
CertificateValidator.buildTrustAnchorsFromCertificates(trustedCAs),
78-
CertificateValidator.buildCertStoreFromCertificates(trustedCAs)
78+
CertificateValidator.buildCertStoreFromCertificates(trustedCAs),
79+
OcspCertificateRevocationChecker.DEFAULT_THIS_UPDATE_AGE,
80+
OcspCertificateRevocationChecker.DEFAULT_NEXT_UPDATE_AGE
7981
);
8082
OcspServiceProvider services = new OcspServiceProvider(null, aiaConfiguration);
8183
OcspCertificateRevocationChecker checker = new OcspCertificateRevocationChecker(
8284
OcspClientImpl.build(Duration.ofSeconds(5)),
8385
services,
84-
OcspCertificateRevocationChecker.DEFAULT_TIME_SKEW,
85-
OcspCertificateRevocationChecker.DEFAULT_THIS_UPDATE_AGE
86+
OcspCertificateRevocationChecker.DEFAULT_TIME_SKEW
8687
);
8788

8889
AuthTokenValidator validator = new AuthTokenValidatorBuilder()
@@ -94,7 +95,14 @@ AuthTokenValidator validator = new AuthTokenValidatorBuilder()
9495

9596
The five-second connection and response timeout above is an explicit example setting. For a custom Java `HttpClient`, use `new OcspClientImpl(httpClient, responseTimeout)` and configure the connection timeout on that client. Alternatively, supply your own `OcspClient` implementation. See [OcspClientOverrideTest](../../../../../test/java/eu/webeid/ocsp/client/OcspClientOverrideTest.java).
9697

97-
The custom checker's suggested constants are 15 minutes for clock/update skew and 2 minutes for maximum `thisUpdate` age; pass different positive durations to its constructor to change them. These checks are implemented by [OcspResponseValidator](protocol/OcspResponseValidator.java).
98+
The custom checker's suggested constant for clock/update skew is 15 minutes; pass a different positive duration to its constructor to change it.
99+
100+
The maximum ages of the OCSP response's `thisUpdate` and `nextUpdate` times are configured per OCSP service, via the constructor of `AiaOcspServiceConfiguration`, `DesignatedOcspServiceConfiguration` or `FallbackOcspServiceConfiguration`:
101+
102+
- `maxThisUpdateAge` – the maximum age of the OCSP response's `thisUpdate` time before the response is too old to rely on. `OcspCertificateRevocationChecker.DEFAULT_THIS_UPDATE_AGE` is 2 minutes.
103+
- `maxNextUpdateAge` – the maximum age of the OCSP response's `nextUpdate` time before the response is too old to rely on. `OcspCertificateRevocationChecker.DEFAULT_NEXT_UPDATE_AGE` is 15 minutes, which is equal to the default allowed time skew.
104+
105+
These checks are implemented by [OcspResponseValidator](protocol/OcspResponseValidator.java).
98106

99107
For a designated responder, replace the `services` definition above with the following configuration. `responderCertificate` must be the service's trusted signing certificate and `supportedIssuers` the collection of issuer certificates served by it:
100108
@@ -106,7 +114,9 @@ DesignatedOcspServiceConfiguration designated = new DesignatedOcspServiceConfigu
106114
URI.create("https://ocsp.example.org"),
107115
responderCertificate,
108116
supportedIssuers,
109-
true // This service supports nonces.
117+
true, // This service supports nonces.
118+
OcspCertificateRevocationChecker.DEFAULT_THIS_UPDATE_AGE,
119+
OcspCertificateRevocationChecker.DEFAULT_NEXT_UPDATE_AGE
110120
);
111121
OcspServiceProvider services = new OcspServiceProvider(designated, aiaConfiguration);
112122
```
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// SPDX-FileCopyrightText: Estonian Information System Authority
2+
// SPDX-License-Identifier: MIT
3+
4+
package eu.webeid.ocsp.exceptions;
5+
6+
import eu.webeid.security.exceptions.AuthTokenException;
7+
8+
public class UserCertificateOCSPException extends AuthTokenException {
9+
10+
public UserCertificateOCSPException(String message) {
11+
super(message);
12+
}
13+
14+
public UserCertificateOCSPException(String message, Throwable exception) {
15+
super(message, exception);
16+
}
17+
18+
}

‎src/main/java/eu/webeid/ocsp/protocol/OcspResponseValidator.java‎

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.security.cert.X509Certificate;
2626
import java.time.Duration;
2727
import java.time.Instant;
28+
import java.util.List;
2829
import java.util.Objects;
2930

3031
public final class OcspResponseValidator {
@@ -35,14 +36,57 @@ public final class OcspResponseValidator {
3536
* https://oidref.com/1.3.6.1.5.5.7.3.9
3637
*/
3738
private static final String OID_OCSP_SIGNING = "1.3.6.1.5.5.7.3.9";
39+
private static final int KEY_USAGE_DIGITAL_SIGNATURE_BIT_INDEX = 0;
40+
private static final int KEY_USAGE_KEY_CERT_SIGN_BIT_INDEX = 5;
3841
private static final String ERROR_PREFIX = "Certificate status update time check failed: ";
3942

40-
public static void validateHasSigningExtension(X509Certificate certificate) throws OCSPCertificateException {
43+
public static void validateBasicConstraintsNotCA(X509Certificate certificate) throws OCSPCertificateException {
44+
Objects.requireNonNull(certificate, "certificate");
45+
// X509Certificate.getBasicConstraints() returns -1 when the Basic Constraints extension is absent
46+
// or when cA=FALSE, and a non-negative value only when cA=TRUE (the pathLenConstraint, or
47+
// Integer.MAX_VALUE when cA=TRUE without pathLenConstraint).
48+
if (certificate.getBasicConstraints() >= 0) {
49+
throw new OCSPCertificateException("Certificate " + certificate.getSubjectX500Principal() +
50+
" must not be a CA certificate (Basic Constraints CA:TRUE is not allowed for OCSP responder)");
51+
}
52+
}
53+
54+
public static void validateKeyUsageDigitalSignature(X509Certificate certificate) throws OCSPCertificateException {
55+
Objects.requireNonNull(certificate, "certificate");
56+
final boolean[] keyUsage = certificate.getKeyUsage();
57+
if (keyUsage == null) {
58+
throw new OCSPCertificateException("Certificate " + certificate.getSubjectX500Principal() +
59+
" does not contain the Key Usage extension required for OCSP response signing");
60+
}
61+
if (keyUsage.length <= KEY_USAGE_DIGITAL_SIGNATURE_BIT_INDEX || !keyUsage[KEY_USAGE_DIGITAL_SIGNATURE_BIT_INDEX]) {
62+
throw new OCSPCertificateException("Certificate " + certificate.getSubjectX500Principal() +
63+
" Key Usage extension does not contain Digital Signature, which is required for OCSP response signing");
64+
}
65+
}
66+
67+
public static void validateKeyUsageNotCertificateSigning(X509Certificate certificate) throws OCSPCertificateException {
68+
Objects.requireNonNull(certificate, "certificate");
69+
final boolean[] keyUsage = certificate.getKeyUsage();
70+
if (keyUsage == null) {
71+
return;
72+
}
73+
if (keyUsage.length > KEY_USAGE_KEY_CERT_SIGN_BIT_INDEX && keyUsage[KEY_USAGE_KEY_CERT_SIGN_BIT_INDEX]) {
74+
throw new OCSPCertificateException("Certificate " + certificate.getSubjectX500Principal() +
75+
" Key Usage extension contains Certificate Signing, which is not allowed for OCSP responder");
76+
}
77+
}
78+
79+
public static void validateExtendedKeyUsageOcspSigning(X509Certificate certificate) throws OCSPCertificateException {
4180
Objects.requireNonNull(certificate, "certificate");
4281
try {
43-
if (certificate.getExtendedKeyUsage() == null || !certificate.getExtendedKeyUsage().contains(OID_OCSP_SIGNING)) {
82+
final List<String> extendedKeyUsage = certificate.getExtendedKeyUsage();
83+
if (extendedKeyUsage == null) {
84+
throw new OCSPCertificateException("Certificate " + certificate.getSubjectX500Principal() +
85+
" does not contain the Extended Key Usage extension required for OCSP response signing");
86+
}
87+
if (!extendedKeyUsage.contains(OID_OCSP_SIGNING)) {
4488
throw new OCSPCertificateException("Certificate " + certificate.getSubjectX500Principal() +
45-
" does not contain the key usage extension for OCSP response signing");
89+
" Extended Key Usage extension does not contain OCSP Signing, which is required for OCSP response signing");
4690
}
4791
} catch (CertificateParsingException e) {
4892
throw new OCSPCertificateException("Certificate parsing failed:", e);
@@ -58,7 +102,7 @@ public static void validateResponseSignature(BasicOCSPResp basicResponse, X509Ce
58102
}
59103
}
60104

61-
public static void validateCertificateStatusUpdateTime(SingleResp certStatusResponse, Duration allowedTimeSkew, Duration maxThisupdateAge, URI ocspResponderUri) throws UserCertificateOCSPCheckFailedException {
105+
public static void validateCertificateStatusUpdateTime(SingleResp certStatusResponse, Duration allowedTimeSkew, Duration maxThisUpdateAge, Duration maxNextUpdateAge, URI ocspResponderUri) throws UserCertificateOCSPCheckFailedException {
62106
// From RFC 2560, https://www.ietf.org/rfc/rfc2560.txt:
63107
// 4.2.2. Notes on OCSP Responses
64108
// 4.2.2.1. Time
@@ -69,9 +113,9 @@ public static void validateCertificateStatusUpdateTime(SingleResp certStatusResp
69113
// If nextUpdate is not set, the responder is indicating that newer
70114
// revocation information is available all the time.
71115
final Instant now = DateAndTime.DefaultClock.getInstance().now().toInstant();
72-
final Instant earliestAcceptableTimeSkew = now.minus(allowedTimeSkew);
73116
final Instant latestAcceptableTimeSkew = now.plus(allowedTimeSkew);
74-
final Instant minimumValidThisUpdateTime = now.minus(maxThisupdateAge);
117+
final Instant minimumValidThisUpdateTime = now.minus(maxThisUpdateAge);
118+
final Instant minimumValidNextUpdateTime = now.minus(maxNextUpdateAge);
75119

76120
final Instant thisUpdate = certStatusResponse.getThisUpdate().toInstant();
77121
if (thisUpdate.isAfter(latestAcceptableTimeSkew)) {
@@ -89,9 +133,10 @@ public static void validateCertificateStatusUpdateTime(SingleResp certStatusResp
89133
return;
90134
}
91135
final Instant nextUpdate = certStatusResponse.getNextUpdate().toInstant();
92-
if (nextUpdate.isBefore(earliestAcceptableTimeSkew)) {
136+
if (nextUpdate.isBefore(minimumValidNextUpdateTime)) {
93137
throw new UserCertificateOCSPCheckFailedException(ERROR_PREFIX +
94-
"nextUpdate '" + nextUpdate + "' is in the past", ocspResponderUri);
138+
"nextUpdate '" + nextUpdate + "' is too old, " +
139+
"minimum time allowed: '" + minimumValidNextUpdateTime + "'", ocspResponderUri);
95140
}
96141
if (nextUpdate.isBefore(thisUpdate)) {
97142
throw new UserCertificateOCSPCheckFailedException(ERROR_PREFIX +

0 commit comments

Comments
 (0)