Skip to content

Commit c8e2c85

Browse files
SanderKondratjevNortalSanderKondratjevNortal
committed
NFC-46 Delegate to super.validate first and fix tests. Centralize common parse logic in AuthTokenValidator and remove duplicates. Move null/empty check into validateSupportedSignatureAlgorithms. Signing cert authority and key check.
1 parent 13c1cbf commit c8e2c85

File tree

6 files changed

+170
-76
lines changed

6 files changed

+170
-76
lines changed

‎src/main/java/eu/webeid/security/validator/AuthTokenV11Validator.java‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
import eu.webeid.security.validator.certvalidators.SubjectCertificateValidatorBatch;
99
import eu.webeid.security.validator.ocsp.OcspClient;
1010
import eu.webeid.security.validator.ocsp.OcspServiceProvider;
11+
import org.bouncycastle.asn1.x509.Extension;
1112

1213
import java.security.cert.CertStore;
1314
import java.security.cert.TrustAnchor;
1415
import java.security.cert.X509Certificate;
16+
import java.util.Arrays;
1517
import java.util.List;
1618
import java.util.Set;
1719

@@ -26,6 +28,7 @@ class AuthTokenV11Validator extends AuthTokenV1Validator {
2628
"SHA-224", "SHA-256", "SHA-384", "SHA-512",
2729
"SHA3-224", "SHA3-256", "SHA3-384", "SHA3-512"
2830
);
31+
private static final int KEY_USAGE_NON_REPUDIATION = 1;
2932

3033
public AuthTokenV11Validator(
3134
SubjectCertificateValidatorBatch simpleSubjectCertificateValidators,
@@ -62,23 +65,34 @@ public X509Certificate validate(WebEidAuthToken token, String currentChallengeNo
6265
throw new AuthTokenParseException("'unverifiedSigningCertificate' field is missing, null or empty for format 'web-eid:1.1'");
6366
}
6467

65-
if (token.getSupportedSignatureAlgorithms() == null || token.getSupportedSignatureAlgorithms().isEmpty()) {
66-
throw new AuthTokenParseException("'supportedSignatureAlgorithms' field is missing");
67-
}
68-
6968
validateSupportedSignatureAlgorithms(token.getSupportedSignatureAlgorithms());
7069

71-
final X509Certificate subjectCertificate = CertificateLoader.decodeCertificateFromBase64(token.getUnverifiedCertificate());
70+
final X509Certificate subjectCertificate = validateV1(token, currentChallengeNonce);
7271
final X509Certificate signingCertificate = CertificateLoader.decodeCertificateFromBase64(token.getUnverifiedSigningCertificate());
7372

7473
if (!subjectCertificate.getSubjectX500Principal().equals(signingCertificate.getSubjectX500Principal())) {
7574
throw new AuthTokenParseException("Signing certificate subject does not match authentication certificate subject");
7675
}
7776

78-
return super.validate(token, currentChallengeNonce);
77+
byte[] subjectSki = subjectCertificate.getExtensionValue(Extension.subjectKeyIdentifier.getId());
78+
byte[] signingAki = signingCertificate.getExtensionValue(Extension.authorityKeyIdentifier.getId());
79+
if (subjectSki != null && signingAki != null && !Arrays.equals(subjectSki, signingAki)) {
80+
throw new AuthTokenParseException("Signing certificate not issued by same authority as authentication certificate");
81+
}
82+
83+
boolean[] keyUsage = signingCertificate.getKeyUsage();
84+
if (keyUsage == null || keyUsage.length <= KEY_USAGE_NON_REPUDIATION || !keyUsage[KEY_USAGE_NON_REPUDIATION]) {
85+
throw new AuthTokenParseException("Signing certificate does not have nonRepudiation key usage");
86+
}
87+
88+
return subjectCertificate;
7989
}
8090

8191
private static void validateSupportedSignatureAlgorithms(List<SupportedSignatureAlgorithm> algorithms) throws AuthTokenParseException {
92+
if (algorithms == null || algorithms.isEmpty()) {
93+
throw new AuthTokenParseException("'supportedSignatureAlgorithms' field is missing");
94+
}
95+
8296
boolean hasInvalid = algorithms.stream().anyMatch(supportedSignatureAlgorithm ->
8397
!SUPPORTED_CRYPTO_ALGORITHMS.contains(supportedSignatureAlgorithm.getCryptoAlgorithm()) ||
8498
!SUPPORTED_HASH_FUNCTIONS.contains(supportedSignatureAlgorithm.getHashFunction()) ||
@@ -89,4 +103,8 @@ private static void validateSupportedSignatureAlgorithms(List<SupportedSignature
89103
throw new AuthTokenParseException("Unsupported signature algorithm");
90104
}
91105
}
106+
107+
protected X509Certificate validateV1(WebEidAuthToken token, String currentChallengeNonce) throws AuthTokenException {
108+
return super.validate(token, currentChallengeNonce);
109+
}
92110
}

‎src/main/java/eu/webeid/security/validator/AuthTokenV1Validator.java‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package eu.webeid.security.validator;
22

3-
import com.fasterxml.jackson.databind.ObjectMapper;
43
import eu.webeid.security.authtoken.WebEidAuthToken;
54
import eu.webeid.security.certificate.CertificateLoader;
65
import eu.webeid.security.exceptions.AuthTokenException;
@@ -11,7 +10,6 @@
1110
import eu.webeid.security.validator.ocsp.OcspClient;
1211
import eu.webeid.security.validator.ocsp.OcspServiceProvider;
1312

14-
import java.io.IOException;
1513
import java.security.cert.CertStore;
1614
import java.security.cert.TrustAnchor;
1715
import java.security.cert.X509Certificate;
@@ -52,19 +50,6 @@ public boolean supports(String format) {
5250
return format != null && format.startsWith(SUPPORTED_TOKEN_FORMAT_VERSION);
5351
}
5452

55-
@Override
56-
public WebEidAuthToken parse(String authToken) throws AuthTokenException {
57-
try {
58-
final WebEidAuthToken token = new ObjectMapper().readValue(authToken, WebEidAuthToken.class);
59-
if (token == null) {
60-
throw new AuthTokenParseException("Web eID authentication token is null");
61-
}
62-
return token;
63-
} catch (IOException e) {
64-
throw new AuthTokenParseException("Error parsing Web eID authentication token", e);
65-
}
66-
}
67-
6853
@Override
6954
public X509Certificate validate(WebEidAuthToken token, String currentChallengeNonce) throws AuthTokenException {
7055
if (token.getFormat() == null || token.getFormat().isBlank()) {

‎src/main/java/eu/webeid/security/validator/AuthTokenValidator.java‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,15 @@
2222

2323
package eu.webeid.security.validator;
2424

25+
import com.fasterxml.jackson.databind.ObjectMapper;
26+
import com.fasterxml.jackson.databind.ObjectReader;
2527
import eu.webeid.security.authtoken.WebEidAuthToken;
2628
import eu.webeid.security.certificate.CertificateData;
2729
import eu.webeid.security.exceptions.AuthTokenException;
30+
import eu.webeid.security.exceptions.AuthTokenParseException;
2831
import eu.webeid.security.util.Strings;
2932

33+
import java.io.IOException;
3034
import java.security.cert.X509Certificate;
3135

3236
/**
@@ -49,7 +53,25 @@ public interface AuthTokenValidator {
4953
* @return the Web eID authentication token
5054
* @throws AuthTokenException when parsing fails
5155
*/
52-
WebEidAuthToken parse(String authToken) throws AuthTokenException;
56+
default WebEidAuthToken parse(String authToken) throws AuthTokenException {
57+
try {
58+
if (authToken == null || authToken.length() < 100) {
59+
throw new AuthTokenParseException("Auth token is null or too short");
60+
}
61+
if (authToken.length() > 10000) {
62+
throw new AuthTokenParseException("Auth token is too long");
63+
}
64+
65+
ObjectReader reader = new ObjectMapper().readerFor(WebEidAuthToken.class);
66+
WebEidAuthToken token = reader.readValue(authToken);
67+
if (token == null) {
68+
throw new AuthTokenParseException("Web eID authentication token is null");
69+
}
70+
return token;
71+
} catch (IOException e) {
72+
throw new AuthTokenParseException("Error parsing Web eID authentication token", e);
73+
}
74+
}
5375

5476
/**
5577
* Validates the Web eID authentication token signed by the subject and returns

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

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,9 @@
2222

2323
package eu.webeid.security.validator;
2424

25-
import com.fasterxml.jackson.databind.ObjectMapper;
26-
import com.fasterxml.jackson.databind.ObjectReader;
2725
import eu.webeid.security.authtoken.WebEidAuthToken;
2826
import eu.webeid.security.certificate.CertificateValidator;
2927
import eu.webeid.security.exceptions.AuthTokenException;
30-
import eu.webeid.security.exceptions.AuthTokenParseException;
3128
import eu.webeid.security.exceptions.JceException;
3229
import eu.webeid.security.validator.certvalidators.SubjectCertificatePolicyValidator;
3330
import eu.webeid.security.validator.certvalidators.SubjectCertificatePurposeValidator;
@@ -38,7 +35,6 @@
3835
import org.slf4j.Logger;
3936
import org.slf4j.LoggerFactory;
4037

41-
import java.io.IOException;
4238
import java.security.cert.CertStore;
4339
import java.security.cert.TrustAnchor;
4440
import java.security.cert.X509Certificate;
@@ -51,11 +47,7 @@
5147
*/
5248
final class AuthTokenValidatorImpl implements AuthTokenValidator {
5349

54-
private static final int TOKEN_MIN_LENGTH = 100;
55-
private static final int TOKEN_MAX_LENGTH = 10000;
5650
private static final Logger LOG = LoggerFactory.getLogger(AuthTokenValidatorImpl.class);
57-
58-
private static final ObjectReader OBJECT_READER = new ObjectMapper().readerFor(WebEidAuthToken.class);
5951
private OcspServiceProvider ocspServiceProvider;
6052
private final AuthTokenValidatorFactory tokenValidatorFactory;
6153

@@ -112,20 +104,6 @@ public boolean supports(String format) {
112104
return tokenValidatorFactory.supports(format);
113105
}
114106

115-
@Override
116-
public WebEidAuthToken parse(String authToken) throws AuthTokenException {
117-
try {
118-
LOG.info("Starting token parsing");
119-
validateTokenLength(authToken);
120-
WebEidAuthToken token = parseToken(authToken);
121-
return tokenValidatorFactory.requireFor(token.getFormat()).parse(authToken);
122-
} catch (Exception e) {
123-
// Generally "log and rethrow" is an anti-pattern, but it fits with the surrounding logging style.
124-
LOG.warn("Token parsing was interrupted:", e);
125-
throw e;
126-
}
127-
}
128-
129107
@Override
130108
public X509Certificate validate(WebEidAuthToken authToken, String currentChallengeNonce) throws AuthTokenException {
131109
try {
@@ -137,25 +115,4 @@ public X509Certificate validate(WebEidAuthToken authToken, String currentChallen
137115
throw e;
138116
}
139117
}
140-
141-
private void validateTokenLength(String authToken) throws AuthTokenParseException {
142-
if (authToken == null || authToken.length() < TOKEN_MIN_LENGTH) {
143-
throw new AuthTokenParseException("Auth token is null or too short");
144-
}
145-
if (authToken.length() > TOKEN_MAX_LENGTH) {
146-
throw new AuthTokenParseException("Auth token is too long");
147-
}
148-
}
149-
150-
private WebEidAuthToken parseToken(String authToken) throws AuthTokenParseException {
151-
try {
152-
final WebEidAuthToken token = OBJECT_READER.readValue(authToken);
153-
if (token == null) {
154-
throw new AuthTokenParseException("Web eID authentication token is null");
155-
}
156-
return token;
157-
} catch (IOException e) {
158-
throw new AuthTokenParseException("Error parsing Web eID authentication token", e);
159-
}
160-
}
161118
}

0 commit comments

Comments
 (0)