Merge pull request #153 from RIPE-NCC/fix/npe-on-invalid-cert

Wrap NPE when RpkiCaCertificateRequestParser fails

README.md

@@ -62,7 +62,9 @@ next (snapshot) release, e.g. `1.1-SNAPSHOT` after releasing `1.0`.
 
 ## Changelog
 
-## 2024-xx-yy 1.38
+## 2024-02-29 1.38
+  * Raise RpkiCaCertificateRequestParserException instead of NPE when an
+    invalid CSR is passed.
 
 ## 2024-02-28 1.37
   * Use bouncy castle 1.77 (and update API usage accordingly)

src/main/java/net/ripe/rpki/commons/provisioning/x509/pkcs10/RpkiCaCertificateRequestParser.java

@@ -42,8 +42,16 @@ public class RpkiCaCertificateRequestParser {
     private PublicKey publicKey;
 
     public RpkiCaCertificateRequestParser(PKCS10CertificationRequest pkcs10CertificationRequest) throws RpkiCaCertificateRequestParserException {
-        this.pkcs10CertificationRequest = new JcaPKCS10CertificationRequest(pkcs10CertificationRequest);
-        process();
+        // change in bouncy castle 1.74-1.74: PKCS10CertificationRequest constructor now throws NPE if object with
+        // null attributes is passed (e.g. a mock).
+        //
+        // Re-throw a checked exception on NPE.
+        try {
+            this.pkcs10CertificationRequest = new JcaPKCS10CertificationRequest(pkcs10CertificationRequest);
+            process();
+        } catch (NullPointerException e) {
+            throw new RpkiCaCertificateRequestParserException("Error while processing certification request.", e);
+        }
 
         if (caRepositoryUri == null) {
             throw new RpkiCaCertificateRequestParserException("No CA Repository URI included in SIA in request");

src/test/java/net/ripe/rpki/commons/provisioning/x509/pkcs10/RpkiCaCertificateRequestParserTest.java

@@ -0,0 +1,24 @@
+package net.ripe.rpki.commons.provisioning.x509.pkcs10;
+
+import org.bouncycastle.pkcs.PKCS10CertificationRequest;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+public class RpkiCaCertificateRequestParserTest {
+    @Test
+    void shouldThrowCaCertificateRequestParserException_on_null() {
+        // When passed in null
+        assertThatThrownBy(() -> new RpkiCaCertificateRequestParser(null))
+            .isInstanceOf(RpkiCaCertificateRequestParserException.class);
+
+        // Or when passed in object that has null properties (e.g. a mock)
+        var mock = Mockito.mock(PKCS10CertificationRequest.class);
+        assertThat(mock.getSubject()).isNull();
+
+        assertThatThrownBy(() -> new RpkiCaCertificateRequestParser(mock))
+            .isInstanceOf(RpkiCaCertificateRequestParserException.class);
+    }
+}