Skip to content

Commit b3acfde

Browse files
committed
Validate only the relevant CA in certificate chain
WE2-913 Modify the certificate validation process to only check the expiration of CA certificate that is directly part of the user's certificate chain. This prevents service interruptions caused by expired but unrelated CA certificates. Signed-off-by: Mart Somermaa <[email protected]>
1 parent 0470d72 commit b3acfde

File tree

8 files changed

+50
-85
lines changed

8 files changed

+50
-85
lines changed

README.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,7 @@ import eu.webeid.security.certificate.CertificateLoader;
108108

109109
...
110110
private X509Certificate[] trustedIntermediateCACertificates() {
111-
return CertificateLoader.loadCertificatesFromResources(
112-
"cacerts/ESTEID2018.cer");
111+
return CertificateLoader.loadCertificatesFromResources("cacerts/ESTEID2018.cer");
113112
}
114113
...
115114
```
@@ -128,7 +127,7 @@ import eu.webeid.security.validator.AuthTokenValidatorBuilder;
128127
public AuthTokenValidator tokenValidator() throws JceException {
129128
return new AuthTokenValidatorBuilder()
130129
.withSiteOrigin("https://example.org")
131-
.withTrustedCertificateAuthorities(trustedCertificateAuthorities())
130+
.withTrustedCertificateAuthorities(trustedIntermediateCACertificates())
132131
.build();
133132
}
134133
...

pom.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@
118118
<groupId>org.apache.maven.plugins</groupId>
119119
<artifactId>maven-javadoc-plugin</artifactId>
120120
<version>${maven-javadoc-plugin.version}</version>
121+
<configuration>
122+
<doclint>all,-missing</doclint>
123+
</configuration>
121124
<executions>
122125
<execution>
123126
<id>attach-javadocs</id>

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,31 +56,32 @@ public static void certificateIsValidOnDate(X509Certificate cert, Date date, Str
5656
}
5757
}
5858

59-
public static void trustedCACertificatesAreValidOnDate(Set<TrustAnchor> trustedCACertificateAnchors, Date date) throws CertificateNotYetValidException, CertificateExpiredException {
60-
for (TrustAnchor cert : trustedCACertificateAnchors) {
61-
certificateIsValidOnDate(cert.getTrustedCert(), date, "Trusted CA");
62-
}
63-
}
64-
6559
public static X509Certificate validateIsSignedByTrustedCA(X509Certificate certificate,
6660
Set<TrustAnchor> trustedCACertificateAnchors,
6761
CertStore trustedCACertificateCertStore,
68-
Date date) throws CertificateNotTrustedException, JceException {
62+
Date now) throws CertificateNotTrustedException, JceException, CertificateNotYetValidException, CertificateExpiredException {
63+
certificateIsValidOnDate(certificate, now, "User");
64+
6965
final X509CertSelector selector = new X509CertSelector();
7066
selector.setCertificate(certificate);
7167

7268
try {
7369
final PKIXBuilderParameters pkixBuilderParameters = new PKIXBuilderParameters(trustedCACertificateAnchors, selector);
7470
// Certificate revocation check is intentionally disabled as we do the OCSP check with SubjectCertificateNotRevokedValidator ourselves.
7571
pkixBuilderParameters.setRevocationEnabled(false);
76-
pkixBuilderParameters.setDate(date);
72+
pkixBuilderParameters.setDate(now);
7773
pkixBuilderParameters.addCertStore(trustedCACertificateCertStore);
7874

7975
// See the comment in buildCertStoreFromCertificates() below why we use the default JCE provider.
8076
final CertPathBuilder certPathBuilder = CertPathBuilder.getInstance(CertPathBuilder.getDefaultType());
8177
final PKIXCertPathBuilderResult result = (PKIXCertPathBuilderResult) certPathBuilder.build(pkixBuilderParameters);
8278

83-
return result.getTrustAnchor().getTrustedCert();
79+
final X509Certificate trustedCACert = result.getTrustAnchor().getTrustedCert();
80+
81+
// Verify that the trusted CA cert is presently valid before returning the result.
82+
certificateIsValidOnDate(trustedCACert, now, "Trusted CA");
83+
84+
return trustedCACert;
8485

8586
} catch (InvalidAlgorithmParameterException | NoSuchAlgorithmException e) {
8687
throw new JceException(e);

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import eu.webeid.security.exceptions.AuthTokenException;
3131
import eu.webeid.security.exceptions.AuthTokenParseException;
3232
import eu.webeid.security.exceptions.JceException;
33-
import eu.webeid.security.validator.certvalidators.SubjectCertificateExpiryValidator;
3433
import eu.webeid.security.validator.certvalidators.SubjectCertificateNotRevokedValidator;
3534
import eu.webeid.security.validator.certvalidators.SubjectCertificatePolicyValidator;
3635
import eu.webeid.security.validator.certvalidators.SubjectCertificatePurposeValidator;
@@ -83,7 +82,6 @@ final class AuthTokenValidatorImpl implements AuthTokenValidator {
8382
trustedCACertificateCertStore = CertificateValidator.buildCertStoreFromCertificates(configuration.getTrustedCACertificates());
8483

8584
simpleSubjectCertificateValidators = SubjectCertificateValidatorBatch.createFrom(
86-
new SubjectCertificateExpiryValidator(trustedCACertificateAnchors)::validateCertificateExpiry,
8785
SubjectCertificatePurposeValidator::validateCertificatePurpose,
8886
new SubjectCertificatePolicyValidator(configuration.getDisallowedSubjectCertificatePolicies())::validateCertificatePolicies
8987
);

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

Lines changed: 0 additions & 62 deletions
This file was deleted.

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@
2424

2525
import eu.webeid.security.certificate.CertificateValidator;
2626
import eu.webeid.security.exceptions.AuthTokenException;
27-
import org.slf4j.Logger;
28-
import org.slf4j.LoggerFactory;
27+
import eu.webeid.security.exceptions.CertificateExpiredException;
2928
import eu.webeid.security.exceptions.CertificateNotTrustedException;
29+
import eu.webeid.security.exceptions.CertificateNotYetValidException;
3030
import eu.webeid.security.util.DateAndTime;
31+
import org.slf4j.Logger;
32+
import org.slf4j.LoggerFactory;
3133

3234
import java.security.cert.CertStore;
3335
import java.security.cert.TrustAnchor;
@@ -49,10 +51,14 @@ public SubjectCertificateTrustedValidator(Set<TrustAnchor> trustedCACertificateA
4951
}
5052

5153
/**
52-
* Validates that the user certificate from the authentication token is signed by a trusted certificate authority.
54+
* Checks that the user certificate from the authentication token is valid and signed by
55+
* a trusted certificate authority. Also checks the validity of the user certificate's
56+
* trusted CA certificate.
5357
*
5458
* @param subjectCertificate user certificate to be validated
55-
* @throws CertificateNotTrustedException when user certificate is not signed by a trusted CA.
59+
* @throws CertificateNotTrustedException when user certificate is not signed by a trusted CA
60+
* @throws CertificateNotYetValidException when a CA certificate in the chain or the user certificate is not yet valid
61+
* @throws CertificateExpiredException when a CA certificate in the chain or the user certificate is expired
5662
*/
5763
public void validateCertificateTrusted(X509Certificate subjectCertificate) throws AuthTokenException {
5864
// Use the clock instance so that the date can be mocked in tests.
@@ -63,7 +69,7 @@ public void validateCertificateTrusted(X509Certificate subjectCertificate) throw
6369
trustedCACertificateCertStore,
6470
now
6571
);
66-
LOG.debug("Subject certificate is signed by a trusted CA");
72+
LOG.debug("Subject certificate is valid and signed by a trusted CA");
6773
}
6874

6975
public X509Certificate getSubjectCertificateIssuerCertificate() {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ public static AuthTokenValidator getAuthTokenValidatorWithWrongTrustedCA() throw
8282
CertificateLoader.loadCertificatesFromResources("ESTEID2018.cer"));
8383
}
8484

85+
public static AuthTokenValidator getAuthTokenValidatorWithJuly2024ExpiredUnrelatedTrustedCA() throws CertificateException, JceException, IOException {
86+
return getAuthTokenValidator(TOKEN_ORIGIN_URL,
87+
CertificateLoader.loadCertificatesFromResources("TEST_of_ESTEID2018.cer", "TEST_of_SK_OCSP_RESPONDER_2020.cer"));
88+
}
89+
8590
public static AuthTokenValidator getAuthTokenValidatorWithDisallowedESTEIDPolicy() throws CertificateException, JceException, IOException {
8691
return getAuthTokenValidatorBuilder(TOKEN_ORIGIN_URL, getCACertificates())
8792
.withDisallowedCertificatePolicies(EST_IDEMIA_POLICY)

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import java.security.cert.CertificateException;
4747

4848
import static eu.webeid.security.testutil.DateMocker.mockDate;
49+
import static org.assertj.core.api.Assertions.assertThatCode;
4950
import static org.assertj.core.api.Assertions.assertThatThrownBy;
5051
import static org.mockito.Mockito.mockStatic;
5152

@@ -228,13 +229,14 @@ void whenUserCertificateIsNotYetValid_thenValidationFails() {
228229
.hasMessage("User certificate is not yet valid");
229230
}
230231

232+
// In this case both CA and user certificate are not yet valid, we expect the user certificate to be checked first.
231233
@Test
232-
void whenTrustedCACertificateIsNotYetValid_thenValidationFails() {
234+
void whenTrustedCACertificateIsNotYetValid_thenUserCertValidationFails() {
233235
mockDate("2018-08-17", mockedClock);
234236
assertThatThrownBy(() -> validator
235237
.validate(validAuthToken, VALID_CHALLENGE_NONCE))
236238
.isInstanceOf(CertificateNotYetValidException.class)
237-
.hasMessage("Trusted CA certificate is not yet valid");
239+
.hasMessage("User certificate is not yet valid");
238240
}
239241

240242
@Test
@@ -246,13 +248,26 @@ void whenUserCertificateIsNoLongerValid_thenValidationFails() {
246248
.hasMessage("User certificate has expired");
247249
}
248250

251+
// In this case both CA and user certificate have expired, we expect the user certificate to be checked first.
249252
@Test
250-
void whenTrustedCACertificateIsNoLongerValid_thenValidationFails() {
253+
void whenTrustedCACertificateIsNoLongerValid_thenUserCertValidationFails() {
251254
mockDate("2033-10-19", mockedClock);
252255
assertThatThrownBy(() -> validator
253256
.validate(validAuthToken, VALID_CHALLENGE_NONCE))
254257
.isInstanceOf(CertificateExpiredException.class)
255-
.hasMessage("Trusted CA certificate has expired");
258+
.hasMessage("User certificate has expired");
259+
}
260+
261+
// The certificate validation process must only check the expiration of the CA certificate that is directly part of
262+
// the user's certificate chain. Expired but unrelated CA certificates must not cause exceptions.
263+
@Test
264+
void whenUnrelatedCACertificateIsExpired_thenValidationSucceeds() throws Exception {
265+
mockDate("2024-07-01", mockedClock);
266+
final AuthTokenValidator validatorWithExpiredUnrelatedTrustedCA = AuthTokenValidators.getAuthTokenValidatorWithJuly2024ExpiredUnrelatedTrustedCA();
267+
268+
assertThatCode(() -> validatorWithExpiredUnrelatedTrustedCA
269+
.validate(validAuthToken, VALID_CHALLENGE_NONCE))
270+
.doesNotThrowAnyException();
256271
}
257272

258273
@Test

0 commit comments

Comments
 (0)