From e232d91af63f66eaddc492186fcdac5cbcdf3961 Mon Sep 17 00:00:00 2001 From: Erko Risthein Date: Fri, 4 Sep 2026 07:34:21 +0300 Subject: [PATCH] fix: handle user certificate without certificate policies extension SubjectCertificatePolicyValidator passed the extension value straight to JcaX509ExtensionUtils.parseExtensionValue(), which throws NullPointerException when the certificate has no certificate policies extension. As the policy check runs before the trust and signature checks, any client could trigger it by presenting a self-signed certificate without the extension, and the NullPointerException escaped the AuthTokenException hierarchy that callers handle. A certificate without the extension does not contain disallowed policies, so validation now continues to the trust check, matching the behaviour of the .NET validation library. Signed-off-by: Erko Risthein --- .../SubjectCertificatePolicyValidator.java | 5 +++ .../security/testutil/Certificates.java | 9 ++++ .../validator/AuthTokenCertificateTest.java | 12 ++++++ ...SubjectCertificatePolicyValidatorTest.java | 43 +++++++++++++++++++ 4 files changed, 69 insertions(+) create mode 100644 src/test/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidatorTest.java diff --git a/src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidator.java b/src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidator.java index a7521c96..90430c54 100644 --- a/src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidator.java +++ b/src/main/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidator.java @@ -32,6 +32,7 @@ public SubjectCertificatePolicyValidator(Collection disall /** * Validates that the user certificate policies match the configured policies. + * A certificate without the certificate policies extension does not contain disallowed policies and passes validation. * * @param subjectCertificate user certificate to be validated * @throws UserCertificateDisallowedPolicyException when user certificate policy does not match the configured policies. @@ -39,6 +40,10 @@ public SubjectCertificatePolicyValidator(Collection disall */ public void validateCertificatePolicies(X509Certificate subjectCertificate) throws AuthTokenException { final byte[] extensionValue = subjectCertificate.getExtensionValue(Extension.certificatePolicies.getId()); + if (extensionValue == null) { + LOG.debug("User certificate does not contain the certificate policies extension, hence it does not contain disallowed policies."); + return; + } try { final CertificatePolicies policies = CertificatePolicies.getInstance( JcaX509ExtensionUtils.parseExtensionValue(extensionValue) diff --git a/src/test/java/eu/webeid/security/testutil/Certificates.java b/src/test/java/eu/webeid/security/testutil/Certificates.java index ec3dd235..737a3ae1 100644 --- a/src/test/java/eu/webeid/security/testutil/Certificates.java +++ b/src/test/java/eu/webeid/security/testutil/Certificates.java @@ -15,6 +15,7 @@ public class Certificates { private static final String JAAK_KRISTJAN_ESTEID2018_CERT = "MIIEAzCCA2WgAwIBAgIQOWkBWXNDJm1byFd3XsWkvjAKBggqhkjOPQQDBDBgMQswCQYDVQQGEwJFRTEbMBkGA1UECgwSU0sgSUQgU29sdXRpb25zIEFTMRcwFQYDVQRhDA5OVFJFRS0xMDc0NzAxMzEbMBkGA1UEAwwSVEVTVCBvZiBFU1RFSUQyMDE4MB4XDTE4MTAxODA5NTA0N1oXDTIzMTAxNzIxNTk1OVowfzELMAkGA1UEBhMCRUUxKjAoBgNVBAMMIUrDlUVPUkcsSkFBSy1LUklTVEpBTiwzODAwMTA4NTcxODEQMA4GA1UEBAwHSsOVRU9SRzEWMBQGA1UEKgwNSkFBSy1LUklTVEpBTjEaMBgGA1UEBRMRUE5PRUUtMzgwMDEwODU3MTgwdjAQBgcqhkjOPQIBBgUrgQQAIgNiAAR5k1lXzvSeI9O/1s1pZvjhEW8nItJoG0EBFxmLEY6S7ki1vF2Q3TEDx6dNztI1Xtx96cs8r4zYTwdiQoDg7k3diUuR9nTWGxQEMO1FDo4Y9fAmiPGWT++GuOVoZQY3XxijggHDMIIBvzAJBgNVHRMEAjAAMA4GA1UdDwEB/wQEAwIDiDBHBgNVHSAEQDA+MDIGCysGAQQBg5EhAQIBMCMwIQYIKwYBBQUHAgEWFWh0dHBzOi8vd3d3LnNrLmVlL0NQUzAIBgYEAI96AQIwHwYDVR0RBBgwFoEUMzgwMDEwODU3MThAZWVzdGkuZWUwHQYDVR0OBBYEFOQsvTQJEBVMMSmhyZX5bibYJubAMGEGCCsGAQUFBwEDBFUwUzBRBgYEAI5GAQUwRzBFFj9odHRwczovL3NrLmVlL2VuL3JlcG9zaXRvcnkvY29uZGl0aW9ucy1mb3ItdXNlLW9mLWNlcnRpZmljYXRlcy8TAkVOMCAGA1UdJQEB/wQWMBQGCCsGAQUFBwMCBggrBgEFBQcDBDAfBgNVHSMEGDAWgBTAhJkpxE6fOwI09pnhClYACCk+ezBzBggrBgEFBQcBAQRnMGUwLAYIKwYBBQUHMAGGIGh0dHA6Ly9haWEuZGVtby5zay5lZS9lc3RlaWQyMDE4MDUGCCsGAQUFBzAChilodHRwOi8vYy5zay5lZS9UZXN0X29mX0VTVEVJRDIwMTguZGVyLmNydDAKBggqhkjOPQQDBAOBiwAwgYcCQgH1UsmMdtLZti51Fq2QR4wUkAwpsnhsBV2HQqUXFYBJ7EXnLCkaXjdZKkHpABfM0QEx7UUhaI4i53jiJ7E1Y7WOAAJBDX4z61pniHJapI1bkMIiJQ/ti7ha8fdJSMSpAds5CyHIyHkQzWlVy86f9mA7Eu3oRO/1q+eFUzDbNN3Vvy7gQWQ="; private static final String MARILIIS_ESTEID2015_CERT = "MIIFwjCCA6qgAwIBAgIQY+LgQ6n0BURZ048wIEiYHjANBgkqhkiG9w0BAQsFADBrMQswCQYDVQQGEwJFRTEiMCAGA1UECgwZQVMgU2VydGlmaXRzZWVyaW1pc2tlc2t1czEXMBUGA1UEYQwOTlRSRUUtMTA3NDcwMTMxHzAdBgNVBAMMFlRFU1Qgb2YgRVNURUlELVNLIDIwMTUwHhcNMTcxMDAzMTMyMjU2WhcNMjIxMDAyMjA1OTU5WjCBnjELMAkGA1UEBhMCRUUxDzANBgNVBAoMBkVTVEVJRDEaMBgGA1UECwwRZGlnaXRhbCBzaWduYXR1cmUxJjAkBgNVBAMMHU3DhE5OSUssTUFSSS1MSUlTLDYxNzEwMDMwMTYzMRAwDgYDVQQEDAdNw4ROTklLMRIwEAYDVQQqDAlNQVJJLUxJSVMxFDASBgNVBAUTCzYxNzEwMDMwMTYzMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE+nNdtmZ2Ve3XXtjBEGwpvVrDIg7slPfLlyHbCBFMXevfqW5KsXIOy6E2A+Yof+/cqRlY4IhsX2Ka9SsJSo8/EekasFasLFPw9ZBE3MG0nn5zaatg45VSjnPinMmrzFzxo4IB2jCCAdYwCQYDVR0TBAIwADAOBgNVHQ8BAf8EBAMCBkAwgYsGA1UdIASBgzCBgDBzBgkrBgEEAc4fAwEwZjAvBggrBgEFBQcCARYjaHR0cHM6Ly93d3cuc2suZWUvcmVwb3NpdG9vcml1bS9DUFMwMwYIKwYBBQUHAgIwJwwlQWludWx0IHRlc3RpbWlzZWtzLiBPbmx5IGZvciB0ZXN0aW5nLjAJBgcEAIvsQAECMB0GA1UdDgQWBBTiw6M0uow+u6sfhgJAWCSvtkB/ejAiBggrBgEFBQcBAwQWMBQwCAYGBACORgEBMAgGBgQAjkYBBDAfBgNVHSMEGDAWgBRJwPJEOWXVm0Y7DThgg7HWLSiGpjCBgwYIKwYBBQUHAQEEdzB1MCwGCCsGAQUFBzABhiBodHRwOi8vYWlhLmRlbW8uc2suZWUvZXN0ZWlkMjAxNTBFBggrBgEFBQcwAoY5aHR0cHM6Ly9zay5lZS91cGxvYWQvZmlsZXMvVEVTVF9vZl9FU1RFSUQtU0tfMjAxNS5kZXIuY3J0MEEGA1UdHwQ6MDgwNqA0oDKGMGh0dHA6Ly93d3cuc2suZWUvY3Jscy9lc3RlaWQvdGVzdF9lc3RlaWQyMDE1LmNybDANBgkqhkiG9w0BAQsFAAOCAgEAEWBdwmzo/yRncJXKvrE+A1G6yQaBNarKectI5uk18BewYEA4QkhmIwOCwD83jBDB9JF+kuODMHsnvz2mfhwaB/uJIPwfBDQ5JCMBdHPsxLN9nzW/UUzqv2UDMwFkibHCcfV5lTBcmOd7FagUHTUm+8gRlWbDiVl5yPochdJgGYPV+fs/jc5ttHaBvBon0z9LbI4qi0VXdRmV0iogErh8JF5yfGkbfGRaMkWkNYQtQ68i/hPe6MaUxL2/MMt4YTyXtVghmc3ZKZIyp4j0+jlK4vL+d4gaE+TvoQvh6HrmP145FqlMDurATWdB069+hdDLO5fI6AYkc79D5XPKwQ/f1MBufLtBYtOJmtpLT+tdBt/EqOEIO/0FeHcXZlFioNMuxBBeTE/QcDtJ2jxTcg8jNOoepS0wjuxBon9iI1710SR53DLGSWdL52lPoBFacnyPQI1htXVUkJ8icMQKYe3BLt1Ha2cvsA4n4IpjqVROX4mzoPL1hg/aJlD+W2uI2ppYRUNY5FX7C0R+AYzMpOahQ7STQfUxtEnKW98e1I33LWwpjJW9q4htsZeXs4Zatf9ssfUW0VA49tnI28kkN2D8aw1NgWfzVlnJKkEj0qa3ewLZK577j8MexAetT/7leH6mqewr9ewC/tKbYjhufieXx6RPcRC4OZsxtii7ih8TqRg="; private static final String ORGANIZATION_CERT = "MIIF2zCCA8OgAwIBAgIQJs4xyGoNzixjYmV9gUjYljANBgkqhkiG9w0BAQsFADCBjjELMAkGA1UEBhMCRUUxIjAgBgNVBAoMGUFTIFNlcnRpZml0c2VlcmltaXNrZXNrdXMxITAfBgNVBAsMGFNlcnRpZml0c2VlcmltaXN0ZWVudXNlZDEXMBUGA1UEYQwOTlRSRUUtMTA3NDcwMTMxHzAdBgNVBAMMFlRFU1Qgb2YgS0xBU1MzLVNLIDIwMTYwHhcNMjIxMTAyMTI0MTA0WhcNMjUxMjAxMTI0MTA0WjB7MREwDwYDVQQFEwgxMjI3NjI3OTERMA8GA1UECAwISGFyanVtYWExEDAOBgNVBAcMB1RhbGxpbm4xCzAJBgNVBAYTAkVFMRAwDgYDVQQKDAdUVFQgT8OcMSIwIAYDVQQDDBlUZXN0aWphZC5lZSBpc2lrdXR1dmFzdHVzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAzSV4zydk5WY2AuUJ50lNpH3q2C+WH0dE/wqq4nFqpNYkyzFNHecFDFlU0YcpPrhFKDZfJtaAP/drvmdqaVdAcCGIPnXhZ+01pCvmlebe7//kQXaZ6ZHS3EAtwy0EBsVVOMapw1kC58YYymlJhTrdzDFrqjdgv1t1Ph9Gkg/PhaHvqGtKp3IY+v33EwxEV3nPIhZHHC/d0YnzVaN5QiSHbU+mRt8+d2vHPNPNY3qVDh8MPOrJIDeIHp9oSS1+FF4crnvfxmg99d7zemsSstR8/SXedYuvWZb6iSybAjhucp21uF0tcqJ2k6+ZH/976AEy0IC8r4tgf7r70hhYu6KOOQIDAQABo4IBRTCCAUEwCQYDVR0TBAIwADBUBgNVHSAETTBLMDIGCysGAQQBzh8HAQIGMCMwIQYIKwYBBQUHAgEWFWh0dHBzOi8vd3d3LnNrLmVlL2NwczAIBgYEAI96AQEwCwYJKwYBBAHOHwkDMBMGA1UdJQQMMAoGCCsGAQUFBwMCMB8GA1UdIwQYMBaAFC4bj7sBLzT42jAEi1zB8lwl49j3MA4GA1UdDwEB/wQEAwIEsDAdBgNVHQ4EFgQUbNSRZSddDUofhxlpoSVEunofez8weQYIKwYBBQUHAQEEbTBrMC0GCCsGAQUFBzABhiFodHRwOi8vYWlhLmRlbW8uc2suZWUva2xhc3MzLTIwMTYwOgYIKwYBBQUHMAKGLmh0dHBzOi8vYy5zay5lZS9URVNUX29mX0tMQVNTMy1TS18yMDE2LmRlci5jcnQwDQYJKoZIhvcNAQELBQADggIBAE8Z/GIEfPWGMe1fHYqCQ2v3zSOuIzyeEId595wrknl7IcLY8ogG10oDUw6rDWQ6jMBS5PINUG+WpH6Wo8qxkPY5Dz4WQvBB2qnuJTH3Bvm/PFpsD1Jk7dOF35P4kfX63NnsCkccRxwlhjFE56WdxDOwhC+neF5FP4hvYvbIIK73DVxRg6yBe4i/Y/g5MOXKrzpHvRzMTURqR3lF0dAgIwMNluik4so/B2DIXMYHi6jZVJlwdQriyL7HI4/Ub3QwyTrbfJtXkwWINsMaCFG+Ccjae3TVRFDJvIIE/gQd4wEh+PK0RJBYfOnAypFEKyH+giID7LIAnO90MY6mNl1QSLQWrdlqMxv+fDdEi/JwGLZyHzEOxKs9C4S8zngwCiDFBHMtJcL9A1vq512yBz5aXYwlqcmjcQDegLT6s6otu+AXO8ZOdqsA+/ak7BEl0FUWlsc8yLKa4cuLiV68iArfl+VFVIZ+jgdMplwUuf5c2QN5f0gPZZxkiAXQ8D8qssW1yI+dLCuPXPwyMENGxWTzyodcSdkpZsdIyOg7/o+WK3RczvMjjT8X8F4XKo8JPjZBYyGBx5XkqhwVrX3SjEmRPFdcvy+glYRoTslgM2fsj5fSNxCIsq1fQN8yVjYnxk8/X53AsorcpWpLMHxtoxT+YvNZzryY00QjS5kgUQBNmFaU"; + private static final String MISSING_CERTIFICATE_POLICIES_CERT = "MIIB9zCCAX2gAwIBAgIUUISmjq5PuDw3omUL4J5/bpSmcN4wCgYIKoZIzj0EAwIwFTETMBEGA1UEAwwKd2ViLWVpZC5ldTAeFw0yMDA5MjQxMjI0MzNaFw0zMDA5MjIxMjI0MzNaMBUxEzARBgNVBAMMCndlYi1laWQuZXUwdjAQBgcqhkjOPQIBBgUrgQQAIgNiAATYHS7A+I0B1kzD1F7xMjTDxFXj67/ej5We8OWEFdbS0H9n/fdsUx/NuHo905BdosipwbLnLPi9QHCjhMc8Q6D4d9CPskq5FcnvuRF5UHOTJpwToLOeGVOQ5N8WZXDyzZijgY0wgYowHQYDVR0OBBYEFBV9jVSvepV1VGIK6D55S4+QEaxnMB8GA1UdIwQYMBaAFBV9jVSvepV1VGIK6D55S4+QEaxnMAwGA1UdEwEB/wQCMAAwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoGCCsGAQUFBwMCMBUGA1UdEQQOMAyCCndlYi1laWQuZXUwCgYIKoZIzj0EAwIDaAAwZQIwdyRDN4DkuTKCDIfWrrIQSXP0DEf6u/5IkJU4BE7H8ugQB7rabKEHVPzxJZvLih8JAjEAuDoeJmrQJoC1QQXFh58IPW3x2bs6AuKVN6O2l/1n4bWJgD2yjEjHiIxU91IAAHEE"; private static X509Certificate testEsteid2018CA; private static X509Certificate testEsteid2015CA; @@ -22,6 +23,7 @@ public class Certificates { private static X509Certificate jaakKristjanEsteid2018Cert; private static X509Certificate mariliisEsteid2015Cert; private static X509Certificate organizationCert; + private static X509Certificate certificateWithoutCertificatePolicies; private static X509Certificate testSkOcspResponder2020; static void loadCertificates() throws CertificateException, IOException { @@ -73,4 +75,11 @@ public static X509Certificate getOrganizationCert() throws CertificateDecodingEx return organizationCert; } + public static X509Certificate getCertificateWithoutCertificatePolicies() throws CertificateDecodingException { + if (certificateWithoutCertificatePolicies == null) { + certificateWithoutCertificatePolicies = CertificateLoader.decodeCertificateFromBase64(MISSING_CERTIFICATE_POLICIES_CERT); + } + return certificateWithoutCertificatePolicies; + } + } diff --git a/src/test/java/eu/webeid/security/validator/AuthTokenCertificateTest.java b/src/test/java/eu/webeid/security/validator/AuthTokenCertificateTest.java index a13d0e3d..2bffe91b 100644 --- a/src/test/java/eu/webeid/security/validator/AuthTokenCertificateTest.java +++ b/src/test/java/eu/webeid/security/validator/AuthTokenCertificateTest.java @@ -25,7 +25,9 @@ import org.mockito.MockedStatic; import java.security.cert.CertificateException; +import java.util.Base64; +import static eu.webeid.security.testutil.Certificates.getCertificateWithoutCertificatePolicies; import static eu.webeid.security.testutil.DateMocker.mockDate; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -151,6 +153,16 @@ void whenCertificatePolicyIsWrong_thenValidationFails() throws AuthTokenExceptio .isInstanceOf(UserCertificateDisallowedPolicyException.class); } + @Test + void whenCertificatePoliciesExtensionIsMissing_thenValidationContinuesToTrustCheck() throws Exception { + final String certificateWithoutPolicies = Base64.getEncoder() + .encodeToString(getCertificateWithoutCertificatePolicies().getEncoded()); + final WebEidAuthToken token = replaceTokenField(AUTH_TOKEN, "X5C", certificateWithoutPolicies); + assertThatThrownBy(() -> validator + .validate(token, VALID_CHALLENGE_NONCE)) + .isInstanceOf(CertificateNotTrustedException.class); + } + @Test void whenCertificatePolicyIsDisallowed_thenValidationFails() throws Exception { final AuthTokenValidator validatorWithDisallowedESTEIDPolicy = AuthTokenValidators.getAuthTokenValidatorWithDisallowedESTEIDPolicy(); diff --git a/src/test/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidatorTest.java b/src/test/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidatorTest.java new file mode 100644 index 00000000..8dd5dfdc --- /dev/null +++ b/src/test/java/eu/webeid/security/validator/certvalidators/SubjectCertificatePolicyValidatorTest.java @@ -0,0 +1,43 @@ +// SPDX-FileCopyrightText: Estonian Information System Authority +// SPDX-License-Identifier: MIT + +package eu.webeid.security.validator.certvalidators; + +import eu.webeid.security.exceptions.UserCertificateDisallowedPolicyException; +import org.bouncycastle.asn1.ASN1ObjectIdentifier; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static eu.webeid.security.testutil.Certificates.getCertificateWithoutCertificatePolicies; +import static eu.webeid.security.testutil.Certificates.getJaakKristjanEsteid2018Cert; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +class SubjectCertificatePolicyValidatorTest { + + private static final ASN1ObjectIdentifier ESTEID2018_POLICY = new ASN1ObjectIdentifier("1.3.6.1.4.1.51361.1.2.1"); + private static final ASN1ObjectIdentifier UNRELATED_POLICY = new ASN1ObjectIdentifier("1.3.6.1.4.1.51361.1.2.2"); + + @Test + void whenCertificateContainsDisallowedPolicy_thenValidationFails() throws Exception { + final SubjectCertificatePolicyValidator validator = new SubjectCertificatePolicyValidator(List.of(ESTEID2018_POLICY)); + assertThatExceptionOfType(UserCertificateDisallowedPolicyException.class) + .isThrownBy(() -> validator.validateCertificatePolicies(getJaakKristjanEsteid2018Cert())); + } + + @Test + void whenCertificateDoesNotContainDisallowedPolicies_thenValidationSucceeds() throws Exception { + final SubjectCertificatePolicyValidator validator = new SubjectCertificatePolicyValidator(List.of(UNRELATED_POLICY)); + assertThatCode(() -> validator.validateCertificatePolicies(getJaakKristjanEsteid2018Cert())) + .doesNotThrowAnyException(); + } + + @Test + void whenCertificateDoesNotContainCertificatePoliciesExtension_thenValidationSucceeds() throws Exception { + final SubjectCertificatePolicyValidator validator = new SubjectCertificatePolicyValidator(List.of(ESTEID2018_POLICY)); + assertThatCode(() -> validator.validateCertificatePolicies(getCertificateWithoutCertificatePolicies())) + .doesNotThrowAnyException(); + } + +}