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 <erko@risthein.ee>

Author: ErkoRisthein

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

@@ -32,13 +32,18 @@ public SubjectCertificatePolicyValidator(Collection<ASN1ObjectIdentifier> 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.
      * @throws UserCertificateParseException when user certificate policy is invalid.
      */
     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)

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

@@ -15,13 +15,15 @@ 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;
 
     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;
+    }
+
 }

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();

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();
+    }
+
+}