Skip to content

Commit ec1cc76

Browse files
SanderKondratjevNortalSanderKondratjevNortal
committed
NFC-115 Review findings
Signed-off-by: Sander Kondratjev <sander.kondratjev@nortal.com>
1 parent 9ae267c commit ec1cc76

11 files changed

Lines changed: 73 additions & 43 deletions

File tree

‎example/src/main/java/eu/webeid/example/security/WebEidAuthentication.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
package eu.webeid.example.security;
2424

2525
import eu.webeid.security.authtoken.SupportedSignatureAlgorithm;
26+
import eu.webeid.security.authtoken.UnverifiedSigningCertificate;
2627
import eu.webeid.security.certificate.CertificateData;
2728
import org.springframework.lang.Nullable;
2829
import org.springframework.security.core.Authentication;
@@ -48,11 +49,17 @@ private WebEidAuthentication(String principalName, String idCode, String signing
4849
this.supportedSignatureAlgorithms = supportedSignatureAlgorithms;
4950
}
5051

51-
public static Authentication fromCertificate(X509Certificate userCertificate, @Nullable String signingCertificate, @Nullable List<SupportedSignatureAlgorithm> supportedSignatureAlgorithms, List<GrantedAuthority> authorities) throws CertificateEncodingException {
52+
public static Authentication fromCertificate(X509Certificate userCertificate, @Nullable UnverifiedSigningCertificate signingCertificate, List<GrantedAuthority> authorities) throws CertificateEncodingException {
5253
final String principalName = getPrincipalNameFromCertificate(userCertificate);
5354
final String idCode = CertificateData.getSubjectIdCode(userCertificate)
5455
.orElseThrow(() -> new CertificateEncodingException("Certificate does not contain subject ID code"));
55-
return new WebEidAuthentication(principalName, idCode, signingCertificate, supportedSignatureAlgorithms, authorities);
56+
return new WebEidAuthentication(
57+
principalName,
58+
idCode,
59+
signingCertificate != null ? signingCertificate.getUnverifiedSigningCertificate() : null,
60+
signingCertificate != null ? signingCertificate.getSupportedSignatureAlgorithms() : null,
61+
authorities
62+
);
5663
}
5764

5865
private static String getPrincipalNameFromCertificate(X509Certificate userCertificate) throws CertificateEncodingException {

‎example/src/main/java/eu/webeid/example/security/WebEidAuthenticationProvider.java‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,10 @@ public Authentication authenticate(Authentication auth) throws AuthenticationExc
7676
try {
7777
final String nonce = challengeNonceStore.getAndRemove().getBase64EncodedNonce();
7878
final X509Certificate userCertificate = tokenValidator.validate(authToken, nonce);
79-
boolean isV11 = authToken.getFormat() != null && authToken.getFormat().startsWith("web-eid:1.1");
80-
final String signingCertificate = (requireSigningCert && isV11)
81-
? authToken.getUnverifiedSigningCertificates().get(0).getUnverifiedSigningCertificate()
79+
final var signingCertificate = requireSigningCert
80+
? authToken.getUnverifiedSigningCertificates().getFirst() // NOTE: Handling multiple signing certificates is out of scope of this example.
8281
: null;
83-
final List<SupportedSignatureAlgorithm> supportedSignatureAlgorithms = (requireSigningCert && isV11)
84-
? authToken.getUnverifiedSigningCertificates().get(0).getSupportedSignatureAlgorithms()
85-
: null;
86-
87-
return WebEidAuthentication.fromCertificate(userCertificate, signingCertificate, supportedSignatureAlgorithms, authorities);
82+
return WebEidAuthentication.fromCertificate(userCertificate, signingCertificate, authorities);
8883
} catch (AuthTokenException e) {
8984
throw new AuthenticationServiceException("Web eID token validation failed", e);
9085
} catch (CertificateEncodingException e) {

‎example/src/main/java/eu/webeid/example/service/SigningService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public DigestDTO prepareContainer(CertificateDTO certificateDTO, WebEidAuthentic
116116

117117
final DataToSign dataToSign = SignatureBuilder
118118
.aSignature(containerToSign)
119-
.withSignatureProfile(SignatureProfile.T) // AIA OCSP is supported for signatures with LT or LTA profile.
119+
.withSignatureProfile(SignatureProfile.LT) // AIA OCSP is supported for signatures with LT or LTA profile.
120120
.withSigningCertificate(certificate)
121121
.withSignatureDigestAlgorithm(signatureDigestAlgorithm)
122122
.buildDataToSign();

‎example/src/test/java/eu/webeid/example/security/WebEidAuthenticationTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class WebEidAuthenticationTest {
3838
@Test
3939
void whenOrganizationCertificate_thenSucceeds() throws Exception {
4040
final X509Certificate certificate = CertificateLoader.decodeCertificateFromBase64(ORGANIZATION_CERT);
41-
final Authentication authentication = WebEidAuthentication.fromCertificate(certificate, null, null, Collections.emptyList());
41+
final Authentication authentication = WebEidAuthentication.fromCertificate(certificate, null, Collections.emptyList());
4242
assertThat(authentication.getPrincipal()).isEqualTo("Testijad.ee isikutuvastus");
4343
}
4444

‎example/src/test/java/eu/webeid/example/testutil/ObjectMother.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public class ObjectMother {
7272
"\"unverifiedSigningCertificate\":\"MIID6zCCA02gAwIBAgIQT7j6zk6pmVRcyspLo5SqejAKBggqhkjOPQQDBDBgMQswCQYDVQQGEwJFRTEbMBkGA1UECgwSU0sgSUQgU29sdXRpb25zIEFTMRcwFQYDVQRhDA5OVFJFRS0xMDc0NzAxMzEbMBkGA1UEAwwSVEVTVCBvZiBFU1RFSUQyMDE4MB4XDTE5MDUwMjEwNDUzMVoXDTI5MDUwMjEwNDUzMVowfzELMAkGA1UEBhMCRUUxFjAUBgNVBCoMDUpBQUstS1JJU1RKQU4xEDAOBgNVBAQMB0rDlUVPUkcxKjAoBgNVBAMMIUrDlUVPUkcsSkFBSy1LUklTVEpBTiwzODAwMTA4NTcxODEaMBgGA1UEBRMRUE5PRUUtMzgwMDEwODU3MTgwdjAQBgcqhkjOPQIBBgUrgQQAIgNiAASkwENR8GmCpEs6OshDWDfIiKvGuyNMOD2rjIQW321AnZD3oIsqD0svBMNEJJj9Dlvq/47TYDObIa12KAU5IuOBfJs2lrFdSXZjaM+a5TWT3O2JTM36YDH2GcMe/eisepejggGrMIIBpzAJBgNVHRMEAjAAMA4GA1UdDwEB/wQEAwIGQDBIBgNVHSAEQTA/MDIGCysGAQQBg5EhAQIBMCMwIQYIKwYBBQUHAgEWFWh0dHBzOi8vd3d3LnNrLmVlL0NQUzAJBgcEAIvsQAECMB0GA1UdDgQWBBTVX3s48Spy/Es2TcXgkRvwUn2YcjCBigYIKwYBBQUHAQMEfjB8MAgGBgQAjkYBATAIBgYEAI5GAQQwEwYGBACORgEGMAkGBwQAjkYBBgEwUQYGBACORgEFMEcwRRY/aHR0cHM6Ly9zay5lZS9lbi9yZXBvc2l0b3J5L2NvbmRpdGlvbnMtZm9yLXVzZS1vZi1jZXJ0aWZpY2F0ZXMvEwJFTjAfBgNVHSMEGDAWgBTAhJkpxE6fOwI09pnhClYACCk+ezBzBggrBgEFBQcBAQRnMGUwLAYIKwYBBQUHMAGGIGh0dHA6Ly9haWEuZGVtby5zay5lZS9lc3RlaWQyMDE4MDUGCCsGAQUFBzAChilodHRwOi8vYy5zay5lZS9UZXN0X29mX0VTVEVJRDIwMTguZGVyLmNydDAKBggqhkjOPQQDBAOBiwAwgYcCQgGBr+Jbo1GeqgWdIwgMo7SA29AP38JxNm2HWq2Qb+kIHpusAK574Co1K5D4+Mk7/ITTuXQaET5WphHoN7tdAciTaQJBAn0zBigYyVPYSTO68HM6hmlwTwi/KlJDdXW/2NsMjSqofFFJXpGvpxk2CTqSRCjcavxLPnkasTbNROYSJcmM8Xc=\"," +
7373
"\"supportedSignatureAlgorithms\":[{\"cryptoAlgorithm\":\"RSA\",\"hashFunction\":\"SHA-256\",\"paddingScheme\":\"PKCS1.5\"}]" +
7474
"}]," +
75-
"\"issuerApp\":\"https://web-eid.eu/web-eid-mobile-app/releases/v1.0.0\"," +
75+
"\"appVersion\":\"https://web-eid.eu/web-eid-mobile-app/releases/v1.0.0\"," +
7676
"\"signature\":\"0Ov7ME6pTY1K2GXMj8Wxov/o2fGIMEds8OMY5dKdkB0nrqQX7fG1E5mnsbvyHpMDecMUH6Yg+p1HXdgB/lLqOcFZjt/OVXPjAAApC5d1YgRYATDcxsR1zqQwiNcHdmWn\"," +
7777
"\"format\":\"web-eid:1.1\"}",
7878
WebEidAuthToken.class);

src/main/java/eu/webeid/security/certificate/UnverifiedSigningCertificate.java renamed to src/main/java/eu/webeid/security/authtoken/UnverifiedSigningCertificate.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,16 @@
2020
* SOFTWARE.
2121
*/
2222

23-
package eu.webeid.security.certificate;
23+
package eu.webeid.security.authtoken;
2424

25-
import eu.webeid.security.authtoken.SupportedSignatureAlgorithm;
25+
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
2626

2727
import java.util.List;
2828

29+
@JsonIgnoreProperties(ignoreUnknown = true)
2930
public class UnverifiedSigningCertificate {
3031

32+
3133
private String unverifiedSigningCertificate;
3234
private List<SupportedSignatureAlgorithm> supportedSignatureAlgorithms;
3335

‎src/main/java/eu/webeid/security/authtoken/WebEidAuthToken.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
package eu.webeid.security.authtoken;
2424

2525
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
26-
import eu.webeid.security.certificate.UnverifiedSigningCertificate;
2726

2827
import java.util.List;
2928

‎src/main/java/eu/webeid/security/validator/versionvalidators/AuthTokenVersion11Validator.java‎

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import eu.webeid.security.authtoken.SupportedSignatureAlgorithm;
2626
import eu.webeid.security.authtoken.WebEidAuthToken;
2727
import eu.webeid.security.certificate.CertificateLoader;
28-
import eu.webeid.security.certificate.UnverifiedSigningCertificate;
28+
import eu.webeid.security.authtoken.UnverifiedSigningCertificate;
2929
import eu.webeid.security.exceptions.AuthTokenException;
3030
import eu.webeid.security.exceptions.AuthTokenParseException;
3131
import eu.webeid.security.exceptions.CertificateDecodingException;
@@ -46,7 +46,9 @@
4646
import java.security.cert.CertificateNotYetValidException;
4747
import java.security.cert.TrustAnchor;
4848
import java.security.cert.X509Certificate;
49+
import java.util.ArrayList;
4950
import java.util.Arrays;
51+
import java.util.Iterator;
5052
import java.util.List;
5153
import java.util.Set;
5254

@@ -91,50 +93,57 @@ protected String getSupportedFormatPrefix() {
9193
@Override
9294
public X509Certificate validate(WebEidAuthToken token, String currentChallengeNonce) throws AuthTokenException {
9395
final X509Certificate subjectCertificate = validateV1(token, currentChallengeNonce);
94-
final X509Certificate signingCertificate = validateSigningCertificatesExist(token);
95-
validateSupportedSignatureAlgorithms(token.getUnverifiedSigningCertificates());
96-
validateSameSubject(subjectCertificate, signingCertificate);
97-
validateSameIssuer(subjectCertificate, signingCertificate);
98-
validateSigningCertificateValidity(signingCertificate);
99-
validateKeyUsage(signingCertificate);
96+
final List<X509Certificate> signingCertificates = validateSigningCertificatesExist(token);
97+
final List<UnverifiedSigningCertificate> unverifiedSigningCertificates = token.getUnverifiedSigningCertificates();
98+
Iterator<UnverifiedSigningCertificate> unverifiedIterator = unverifiedSigningCertificates.iterator();
99+
for(X509Certificate signingCertificate : signingCertificates) {
100+
UnverifiedSigningCertificate unverifiedSigningCertificate = unverifiedIterator.next();
101+
validateSupportedSignatureAlgorithms(unverifiedSigningCertificate);
102+
validateSameSubject(subjectCertificate, signingCertificate);
103+
validateSameIssuer(subjectCertificate, signingCertificate);
104+
validateSigningCertificateValidity(signingCertificate);
105+
validateKeyUsage(signingCertificate);
106+
}
100107

101108
return subjectCertificate;
102109
}
103110

104-
private static void validateSupportedSignatureAlgorithms(List<UnverifiedSigningCertificate> unverifiedSigningCertificates) throws AuthTokenParseException {
105-
for (UnverifiedSigningCertificate cert : unverifiedSigningCertificates) {
106-
List<SupportedSignatureAlgorithm> algorithms = cert.getSupportedSignatureAlgorithms();
111+
private static void validateSupportedSignatureAlgorithms(UnverifiedSigningCertificate cert) throws AuthTokenParseException {
112+
List<SupportedSignatureAlgorithm> algorithms = cert.getSupportedSignatureAlgorithms();
107113

108-
if (algorithms == null || algorithms.isEmpty()) {
109-
throw new AuthTokenParseException("'supportedSignatureAlgorithms' field is missing");
110-
}
114+
if (algorithms == null || algorithms.isEmpty()) {
115+
throw new AuthTokenParseException("'supportedSignatureAlgorithms' field is missing");
116+
}
111117

112-
boolean hasInvalid = algorithms.stream().anyMatch(algorithm ->
113-
!SUPPORTED_SIGNING_CRYPTO_ALGORITHMS.contains(algorithm.getCryptoAlgorithm()) ||
114-
!SUPPORTED_SIGNING_HASH_FUNCTIONS.contains(algorithm.getHashFunction()) ||
115-
!SUPPORTED_SIGNING_PADDING_SCHEMES.contains(algorithm.getPaddingScheme())
116-
);
118+
boolean hasInvalid = algorithms.stream().anyMatch(algorithm ->
119+
!SUPPORTED_SIGNING_CRYPTO_ALGORITHMS.contains(algorithm.getCryptoAlgorithm()) ||
120+
!SUPPORTED_SIGNING_HASH_FUNCTIONS.contains(algorithm.getHashFunction()) ||
121+
!SUPPORTED_SIGNING_PADDING_SCHEMES.contains(algorithm.getPaddingScheme())
122+
);
117123

118-
if (hasInvalid) {
119-
throw new AuthTokenParseException("Unsupported signature algorithm");
120-
}
124+
if (hasInvalid) {
125+
throw new AuthTokenParseException("Unsupported signature algorithm");
121126
}
122127
}
123128

124-
private static X509Certificate validateSigningCertificatesExist(WebEidAuthToken token) throws AuthTokenParseException, CertificateDecodingException {
129+
private static List<X509Certificate> validateSigningCertificatesExist(WebEidAuthToken token) throws AuthTokenParseException, CertificateDecodingException {
125130
List<UnverifiedSigningCertificate> signingCertificates = token.getUnverifiedSigningCertificates();
126131

127132
if (signingCertificates == null || signingCertificates.isEmpty()) {
128133
throw new AuthTokenParseException("'unverifiedSigningCertificates' field is missing, null or empty for format 'web-eid:1.1'");
129134
}
130135

131-
UnverifiedSigningCertificate signingCertificate = signingCertificates.get(0);
136+
List<X509Certificate> result = new ArrayList<>();
132137

133-
if (signingCertificate == null || isNullOrEmpty(signingCertificate.getUnverifiedSigningCertificate())) {
134-
throw new AuthTokenParseException("'unverifiedSigningCertificates' field is missing, null or empty for format 'web-eid:1.1'");
138+
for (UnverifiedSigningCertificate certificate : signingCertificates) {
139+
if (certificate == null || isNullOrEmpty(certificate.getUnverifiedSigningCertificate())) {
140+
throw new AuthTokenParseException("'unverifiedSigningCertificates' field is missing, null or empty for format 'web-eid:1.1'");
141+
}
142+
143+
result.add(CertificateLoader.decodeCertificateFromBase64(certificate.getUnverifiedSigningCertificate()));
135144
}
136145

137-
return CertificateLoader.decodeCertificateFromBase64(signingCertificate.getUnverifiedSigningCertificate());
146+
return result;
138147
}
139148

140149
private static void validateSameSubject(X509Certificate subjectCertificate, X509Certificate signingCertificate)

‎src/main/java/eu/webeid/security/validator/versionvalidators/AuthTokenVersion1Validator.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ protected String getSupportedFormatPrefix() {
7878

7979
@Override
8080
public X509Certificate validate(WebEidAuthToken token, String currentChallengeNonce) throws AuthTokenException {
81+
if (token.getUnverifiedSigningCertificates() != null) {
82+
throw new AuthTokenParseException("'unverifiedSigningCertificates' field is not allowed for format '" + token.getFormat() + "'");
83+
}
84+
8185
if (token.getUnverifiedCertificate() == null || token.getUnverifiedCertificate().isEmpty()) {
8286
throw new AuthTokenParseException("'unverifiedCertificate' field is missing, null or empty");
8387
}

‎src/test/java/eu/webeid/security/validator/versionvalidators/AuthTokenVersion11ValidatorTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424

2525
import eu.webeid.security.authtoken.WebEidAuthToken;
2626
import eu.webeid.security.certificate.CertificateLoader;
27-
import eu.webeid.security.certificate.UnverifiedSigningCertificate;
27+
import eu.webeid.security.authtoken.UnverifiedSigningCertificate;
2828
import eu.webeid.security.exceptions.AuthTokenParseException;
2929
import eu.webeid.security.validator.AuthTokenSignatureValidator;
3030
import eu.webeid.security.validator.AuthTokenValidationConfiguration;

0 commit comments

Comments
 (0)