8255348: NPE in PKIXCertPathValidator event logging code

Reviewed-by: mullan

src/java.base/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java

@@ -241,13 +241,13 @@ public final class PKIXCertPathValidator extends CertPathValidatorSpi {
         X509ValidationEvent xve = new X509ValidationEvent();
         if (xve.shouldCommit() || EventHelper.isLoggingSecurity()) {
             int[] certIds = params.certificates().stream()
-                    .mapToInt(x -> x.hashCode())
+                    .mapToInt(Certificate::hashCode)
                     .toArray();
-            int anchorCertId =
+            int anchorCertId = (anchorCert != null) ?
-                    anchor.getTrustedCert().hashCode();
+                anchorCert.hashCode() : anchor.getCAPublicKey().hashCode();
             if (xve.shouldCommit()) {
                 xve.certificateId = anchorCertId;
-                int certificatePos = 1; //anchor cert
+                int certificatePos = 1; // most trusted CA
                 xve.certificatePosition = certificatePos;
                 xve.validationCounter = validationCounter.incrementAndGet();
                 xve.commit();

test/jdk/jdk/jfr/event/security/TestX509CertificateEvent.java

@@ -48,12 +48,11 @@ public class TestX509CertificateEvent {
             recording.enable(EventNames.X509Certificate);
             recording.start();
 
-            CertificateFactory cf = CertificateFactory.getInstance("X.509");
+            TestCertificate.ONE.certificate();
-            TestCertificate.ONE.generate(cf);
+            TestCertificate.TWO.certificate();
-            TestCertificate.TWO.generate(cf);
             // Generate twice to make sure only one event per certificate is generated
-            TestCertificate.ONE.generate(cf);
+            TestCertificate.ONE.certificate();
-            TestCertificate.TWO.generate(cf);
+            TestCertificate.TWO.certificate();
 
             recording.stop();
 

test/jdk/jdk/jfr/event/security/TestX509ValidationEvent.java

@@ -47,8 +47,8 @@ public class TestX509ValidationEvent {
         try (Recording recording = new Recording()) {
             recording.enable(EventNames.X509Validation);
             recording.start();
-            // intermeditate certificate test
+            // intermediate certificate test
-            TestCertificate.generateChain(false);
+            TestCertificate.generateChain(false, true);
             recording.stop();
             List<RecordedEvent> events = Events.fromRecording(recording);
             Asserts.assertEquals(events.size(), 3, "Incorrect number of events");
@@ -59,12 +59,23 @@ public class TestX509ValidationEvent {
             recording.enable(EventNames.X509Validation);
             recording.start();
             // self signed certificate test
-            TestCertificate.generateChain(true);
+            TestCertificate.generateChain(true, true);
             recording.stop();
             List<RecordedEvent> events = Events.fromRecording(recording);
             Asserts.assertEquals(events.size(), 2, "Incorrect number of events");
             assertEvent2(events);
         }
+
+        try (Recording recording = new Recording()) {
+            recording.enable(EventNames.X509Validation);
+            recording.start();
+            // intermediate certificate test, with no Cert for trust anchor
+            TestCertificate.generateChain(true, false);
+            recording.stop();
+            List<RecordedEvent> events = Events.fromRecording(recording);
+            Asserts.assertEquals(events.size(), 2, "Incorrect number of events");
+            assertEvent3(events);
+        }
     }
 
     private static void assertEvent1(List<RecordedEvent> events) throws Exception {
@@ -108,4 +119,26 @@ public class TestX509ValidationEvent {
             }
         }
     }
+    /*
+     * Self signed certificate test
+     */
+    private static void assertEvent3(List<RecordedEvent> events) throws Exception {
+        for (RecordedEvent e : events) {
+            int pos = e.getInt("certificatePosition");
+            switch (pos) {
+                // use public key of cert provided in TrustAnchor
+                case 1:
+                    Asserts.assertEquals(e.getLong("certificateId"),
+                        Long.valueOf(TestCertificate.ROOT_CA.certificate().getPublicKey().hashCode()));
+                    break;
+                case 2:
+                    Events.assertField(e, "certificateId")
+                            .equal(TestCertificate.ROOT_CA.certId);
+                    break;
+                default:
+                    System.out.println(events);
+                    throw new Exception("Unexpected position:" + pos);
+            }
+        }
+    }
 }

test/jdk/jdk/security/logging/TestX509CertificateLog.java

@@ -58,9 +58,8 @@ public class TestX509CertificateLog {
 
     public static class GenerateX509Certicate {
         public static void main(String[] args) throws Exception {
-            CertificateFactory cf = CertificateFactory.getInstance("X.509");
+            TestCertificate.ONE.certificate();
-            TestCertificate.ONE.generate(cf);
+            TestCertificate.TWO.certificate();
-            TestCertificate.TWO.generate(cf);
         }
     }
 }

test/jdk/jdk/security/logging/TestX509ValidationLog.java

@@ -43,14 +43,19 @@ public class TestX509ValidationLog {
         l.addExpected("FINE: ValidationChain: " +
                 TestCertificate.ROOT_CA.certId + ", " +
                 TestCertificate.ROOT_CA.certId);
+        l.addExpected("FINE: ValidationChain: " +
+                TestCertificate.ROOT_CA.certificate().getPublicKey().hashCode() +
+                ", " + TestCertificate.ROOT_CA.certId);
         l.testExpected();
     }
 
     public static class GenerateCertificateChain {
         public static void main(String[] args) throws Exception {
-            TestCertificate.generateChain(false);
+            TestCertificate.generateChain(false, true);
             // self signed test
-            TestCertificate.generateChain(true);
+            TestCertificate.generateChain(true, true);
+            // no cert for trust anchor
+            TestCertificate.generateChain(true, false);
         }
     }
 }

test/lib/jdk/test/lib/security/TestCertificate.java

@@ -122,6 +122,8 @@ public enum TestCertificate {
         "J2GyCaJINsyaI/I2\n" +
         "-----END CERTIFICATE-----");
 
+    private static final CertificateFactory CERTIFICATE_FACTORY = getCertificateFactory();
+
     public String serialNumber;
     public String algorithm;
     public String subject;
@@ -143,22 +145,35 @@ public enum TestCertificate {
         this.keyLength = 2048;
     }
 
-    public X509Certificate generate(CertificateFactory cf) throws CertificateException {
+    private static CertificateFactory getCertificateFactory() {
-        ByteArrayInputStream is = new ByteArrayInputStream(encoded.getBytes());
+        try {
-        return (X509Certificate) cf.generateCertificate(is);
+            return CertificateFactory.getInstance("X.509");
+        } catch (CertificateException e) {
+            throw new RuntimeException(e);
+        }
     }
 
-    public static void generateChain(boolean selfSignedTest) throws Exception {
+    public X509Certificate certificate() throws CertificateException {
+        ByteArrayInputStream is = new ByteArrayInputStream(encoded.getBytes());
+        return (X509Certificate) CERTIFICATE_FACTORY.generateCertificate(is);
+    }
+
+    public static void generateChain(boolean selfSignedTest, boolean trustAnchorCert) throws Exception {
         // Do path validation as if it is always Tue, 06 Sep 2016 22:12:21 GMT
         // This value is within the lifetimes of all certificates.
         Date testDate = new Date(1473199941000L);
 
         CertificateFactory cf = CertificateFactory.getInstance("X.509");
-        X509Certificate c1 = TestCertificate.ONE.generate(cf);
+        X509Certificate c1 = TestCertificate.ONE.certificate();
-        X509Certificate c2 = TestCertificate.TWO.generate(cf);
+        X509Certificate c2 = TestCertificate.TWO.certificate();
-        X509Certificate ca = TestCertificate.ROOT_CA.generate(cf);
+        X509Certificate ca = TestCertificate.ROOT_CA.certificate();
 
-        TrustAnchor ta = new TrustAnchor(ca, null);
+        TrustAnchor ta;
+        if (trustAnchorCert) {
+            ta = new TrustAnchor(ca, null);
+        } else {
+            ta = new TrustAnchor(ca.getIssuerX500Principal(), ca.getPublicKey(), null);
+        }
         CertPathValidator validator = CertPathValidator.getInstance("PKIX");
 
         PKIXParameters params = new PKIXParameters(Collections.singleton(ta));