8292033: Move jdk.X509Certificate event logic to JCA layer

Reviewed-by: mullan

src/java.base/share/classes/java/security/cert/CertificateFactory.java

@@ -26,14 +26,14 @@
 package java.security.cert;
 
 import java.io.InputStream;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Objects;
 import java.security.Provider;
 import java.security.Security;
 import java.security.NoSuchAlgorithmException;
 import java.security.NoSuchProviderException;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Objects;
 
 import sun.security.jca.*;
 import sun.security.jca.GetInstance.Instance;
@@ -352,7 +352,9 @@ public class CertificateFactory {
     public final Certificate generateCertificate(InputStream inStream)
         throws CertificateException
     {
-        return certFacSpi.engineGenerateCertificate(inStream);
+        Certificate c = certFacSpi.engineGenerateCertificate(inStream);
+        JCAUtil.tryCommitCertEvent(c);
+        return c;
     }
 
     /**

src/java.base/share/classes/jdk/internal/event/X509CertificateEvent.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -31,6 +31,15 @@ package jdk.internal.event;
  */
 
 public final class X509CertificateEvent extends Event {
+    private static final X509CertificateEvent EVENT = new X509CertificateEvent();
+
+    /**
+     * Returns {@code true} if event is enabled, {@code false} otherwise.
+     */
+    public static boolean isTurnedOn() {
+        return EVENT.isEnabled();
+    }
+
     public String algorithm;
     public String serialNumber;
     public String subject;

src/java.base/share/classes/sun/security/jca/JCAUtil.java

@@ -25,7 +25,14 @@
 
 package sun.security.jca;
 
+import java.security.PublicKey;
 import java.security.SecureRandom;
+import java.security.cert.Certificate;
+import java.security.cert.X509Certificate;
+
+import jdk.internal.event.EventHelper;
+import jdk.internal.event.X509CertificateEvent;
+import sun.security.util.KeyUtil;
 
 /**
  * Collection of static utility methods used by the security framework.
@@ -90,6 +97,45 @@ public final class JCAUtil {
             }
         }
         return result;
+    }
 
+    public static void tryCommitCertEvent(Certificate cert) {
+        if ((X509CertificateEvent.isTurnedOn() || EventHelper.isLoggingSecurity()) &&
+                (cert instanceof X509Certificate x509)) {
+            PublicKey pKey = x509.getPublicKey();
+            String algId = x509.getSigAlgName();
+            String serNum = x509.getSerialNumber().toString(16);
+            String subject = x509.getSubjectX500Principal().toString();
+            String issuer = x509.getIssuerX500Principal().toString();
+            String keyType = pKey.getAlgorithm();
+            int length = KeyUtil.getKeySize(pKey);
+            int hashCode = x509.hashCode();
+            long beginDate = x509.getNotBefore().getTime();
+            long endDate = x509.getNotAfter().getTime();
+            if (X509CertificateEvent.isTurnedOn()) {
+                X509CertificateEvent xce = new X509CertificateEvent();
+                xce.algorithm = algId;
+                xce.serialNumber = serNum;
+                xce.subject = subject;
+                xce.issuer = issuer;
+                xce.keyType = keyType;
+                xce.keyLength = length;
+                xce.certificateId = hashCode;
+                xce.validFrom = beginDate;
+                xce.validUntil = endDate;
+                xce.commit();
+            }
+            if (EventHelper.isLoggingSecurity()) {
+                EventHelper.logX509CertificateEvent(algId,
+                        serNum,
+                        subject,
+                        issuer,
+                        keyType,
+                        length,
+                        hashCode,
+                        beginDate,
+                        endDate);
+            }
+        }
     }
 }

src/java.base/share/classes/sun/security/provider/X509Factory.java

@@ -26,20 +26,16 @@
 package sun.security.provider;
 
 import java.io.*;
-import java.security.PublicKey;
+
 import java.security.cert.*;
 import java.util.*;
 
-import jdk.internal.event.EventHelper;
-import jdk.internal.event.X509CertificateEvent;
-
 import sun.security.pkcs.PKCS7;
 import sun.security.pkcs.ParsingException;
 import sun.security.provider.certpath.X509CertPath;
 import sun.security.provider.certpath.X509CertificatePair;
 import sun.security.util.Cache;
 import sun.security.util.DerValue;
-import sun.security.util.KeyUtil;
 import sun.security.x509.X509CRLImpl;
 import sun.security.x509.X509CertImpl;
 
@@ -116,8 +112,6 @@ public class X509Factory extends CertificateFactorySpi {
         }
         cert = new X509CertImpl(encoding);
         addToCache(certCache, cert.getEncodedInternal(), cert);
-        // record cert details if necessary
-        commitEvent(cert);
         return cert;
     }
 
@@ -478,7 +472,7 @@ public class X509Factory extends CertificateFactorySpi {
             }
         } catch (ParsingException e) {
             while (data != null) {
-                coll.add(new X509CertImpl(data));
+                coll.add(X509CertImpl.newX509CertImpl(data));
                 data = readOneBlock(pbis);
             }
         }
@@ -772,43 +766,4 @@ public class X509Factory extends CertificateFactorySpi {
         }
         return tag;
     }
-
-    private static void commitEvent(X509CertImpl info) {
-        X509CertificateEvent xce = new X509CertificateEvent();
-        if (xce.shouldCommit() || EventHelper.isLoggingSecurity()) {
-            PublicKey pKey = info.getPublicKey();
-            String algId = info.getSigAlgName();
-            String serNum = info.getSerialNumber().toString(16);
-            String subject = info.getSubjectDN().getName();
-            String issuer = info.getIssuerDN().getName();
-            String keyType = pKey.getAlgorithm();
-            int length = KeyUtil.getKeySize(pKey);
-            int hashCode = info.hashCode();
-            long beginDate = info.getNotBefore().getTime();
-            long endDate = info.getNotAfter().getTime();
-            if (xce.shouldCommit()) {
-                xce.algorithm = algId;
-                xce.serialNumber = serNum;
-                xce.subject = subject;
-                xce.issuer = issuer;
-                xce.keyType = keyType;
-                xce.keyLength = length;
-                xce.certificateId = hashCode;
-                xce.validFrom = beginDate;
-                xce.validUntil = endDate;
-                xce.commit();
-            }
-            if (EventHelper.isLoggingSecurity()) {
-                EventHelper.logX509CertificateEvent(algId,
-                        serNum,
-                        subject,
-                        issuer,
-                        keyType,
-                        length,
-                        hashCode,
-                        beginDate,
-                        endDate);
-            }
-        }
-    }
 }

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

@@ -356,7 +356,7 @@ public final class OCSPResponse {
             try {
                 for (int i = 0; i < derCerts.length; i++) {
                     X509CertImpl cert =
-                        new X509CertImpl(derCerts[i].toByteArray());
+                        X509CertImpl.newX509CertImpl(derCerts[i].toByteArray());
                     certs.add(cert);
 
                     if (debug != null) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -240,7 +240,7 @@ public class X509CertificatePair {
                         }
                         opt = opt.data.getDerValue();
                         forward = X509Factory.intern
-                                        (new X509CertImpl(opt.toByteArray()));
+                                        (X509CertImpl.newX509CertImpl(opt.toByteArray()));
                     }
                     break;
                 case TAG_REVERSE:
@@ -251,7 +251,7 @@ public class X509CertificatePair {
                         }
                         opt = opt.data.getDerValue();
                         reverse = X509Factory.intern
-                                        (new X509CertImpl(opt.toByteArray()));
+                                        (X509CertImpl.newX509CertImpl(opt.toByteArray()));
                     }
                     break;
                 default:

src/java.base/share/classes/sun/security/x509/X509CertImpl.java

@@ -41,6 +41,7 @@ import java.util.concurrent.ConcurrentHashMap;
 
 import javax.security.auth.x500.X500Principal;
 
+import sun.security.jca.JCAUtil;
 import sun.security.util.*;
 import sun.security.provider.X509Factory;
 
@@ -266,6 +267,13 @@ public class X509CertImpl extends X509Certificate implements DerEncoder {
         }
     }
 
+    // helper method to record certificate, if necessary, after construction
+    public static X509CertImpl newX509CertImpl(byte[] certData) throws CertificateException {
+        var cert = new X509CertImpl(certData);
+        JCAUtil.tryCommitCertEvent(cert);
+        return cert;
+    }
+
     /**
      * Appends the certificate to an output stream.
      *

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,6 @@
 
 package jdk.jfr.event.security;
 
-import java.security.cert.CertificateFactory;
 import java.util.List;
 
 import jdk.jfr.Recording;
@@ -31,35 +30,63 @@ import jdk.jfr.consumer.RecordedEvent;
 import jdk.test.lib.Asserts;
 import jdk.test.lib.jfr.EventNames;
 import jdk.test.lib.jfr.Events;
+import jdk.test.lib.jfr.VoidFunction;
 import jdk.test.lib.security.TestCertificate;
 
 /*
  * @test
- * @bug 8148188
+ * @bug 8148188 8292033
  * @summary Enhance the security libraries to record events of interest
  * @key jfr
  * @requires vm.hasJFR
+ * @modules java.base/sun.security.x509 java.base/sun.security.tools.keytool
  * @library /test/lib
  * @run main/othervm jdk.jfr.event.security.TestX509CertificateEvent
  */
 public class TestX509CertificateEvent {
-    public static void main(String[] args) throws Exception {
+    public static void main(String[] args) throws Throwable {
+        testCall(() -> {
+            // test regular cert construction
+            TestCertificate.ONE.certificate();
+            TestCertificate.TWO.certificate();
+            // Generate twice to make sure we (now) capture all generate cert events
+            TestCertificate.ONE.certificate();
+            TestCertificate.TWO.certificate();
+        }, 4, true);
+
+        testCall(() -> {
+            // test generateCertificates method
+            TestCertificate.certificates();
+        }, 2, true);
+
+        testCall(() -> {
+            // test generateCertPath method
+            TestCertificate.certPath();
+        }, 4, true);
+
+        testCall(() -> {
+            // test keytool cert generation with JFR enabled
+            // The keytool test will load the dedicated keystore
+            // and call CertificateFactory.generateCertificate
+            // cacerts
+            TestCertificate.keyToolTest();
+        }, -1, false);
+    }
+
+    private static void testCall(VoidFunction f, int expected, boolean runAsserts) throws Throwable {
         try (Recording recording = new Recording()) {
             recording.enable(EventNames.X509Certificate);
             recording.start();
-
+            f.run();
-            TestCertificate.ONE.certificate();
-            TestCertificate.TWO.certificate();
-            // Generate twice to make sure only one event per certificate is generated
-            TestCertificate.ONE.certificate();
-            TestCertificate.TWO.certificate();
-
             recording.stop();
-
             List<RecordedEvent> events = Events.fromRecording(recording);
-            Asserts.assertEquals(events.size(), 2, "Incorrect number of X509Certificate events");
+            if (expected >= 0) {
-            assertEvent(events, TestCertificate.ONE);
+                Asserts.assertEquals(events.size(), expected, "Incorrect number of events");
-            assertEvent(events, TestCertificate.TWO);
+            }
+            if (runAsserts) {
+                assertEvent(events, TestCertificate.ONE);
+                assertEvent(events, TestCertificate.TWO);
+            }
         }
     }
 

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

@@ -39,7 +39,7 @@ import jdk.test.lib.security.TestCertificate;
  * @key jfr
  * @requires vm.hasJFR
  * @library /test/lib
- * @modules jdk.jfr/jdk.jfr.events
+ * @modules jdk.jfr/jdk.jfr.events java.base/sun.security.x509 java.base/sun.security.tools.keytool
  * @run main/othervm jdk.jfr.event.security.TestX509ValidationEvent
  */
 public class TestX509ValidationEvent {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,6 @@
 
 package jdk.security.logging;
 
-import java.security.cert.CertificateFactory;
 import jdk.test.lib.security.TestCertificate;
 
 /*
@@ -31,6 +30,7 @@ import jdk.test.lib.security.TestCertificate;
  * @bug 8148188
  * @summary Enhance the security libraries to record events of interest
  * @library /test/lib /test/jdk
+ * @modules java.base/sun.security.x509 java.base/sun.security.tools.keytool
  * @run main/othervm jdk.security.logging.TestX509CertificateLog LOGGING_ENABLED
  * @run main/othervm jdk.security.logging.TestX509CertificateLog LOGGING_DISABLED
  */

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -30,6 +30,7 @@ import jdk.test.lib.security.TestCertificate;
  * @bug 8148188
  * @summary Enhance the security libraries to record events of interest
  * @library /test/lib /test/jdk
+ * @modules java.base/sun.security.x509 java.base/sun.security.tools.keytool
  * @run main/othervm jdk.security.logging.TestX509ValidationLog LOGGING_ENABLED
  * @run main/othervm jdk.security.logging.TestX509ValidationLog LOGGING_DISABLED
  */

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,16 +24,21 @@
 package jdk.test.lib.security;
 
 import java.io.ByteArrayInputStream;
-import java.security.cert.CertPath;
+import java.io.IOException;
-import java.security.cert.CertPathValidator;
+import java.io.SequenceInputStream;
-import java.security.cert.CertificateException;
+import java.nio.file.Files;
-import java.security.cert.CertificateFactory;
+import java.nio.file.Path;
-import java.security.cert.PKIXParameters;
+import java.security.*;
-import java.security.cert.TrustAnchor;
+import java.security.cert.*;
-import java.security.cert.X509Certificate;
+import java.security.cert.Certificate;
-import java.util.Collections;
+import java.util.*;
-import java.util.Date;
+
-import java.util.List;
+import sun.security.tools.keytool.CertAndKeyGen;
+import sun.security.x509.X500Name;
+
+import jdk.test.lib.JDKToolFinder;
+import jdk.test.lib.SecurityTools;
+import jdk.test.lib.process.OutputAnalyzer;
 
 // Certificates taken from old ValWithAnchorByName testcase ***
 public enum TestCertificate {
@@ -158,6 +163,51 @@ public enum TestCertificate {
         return (X509Certificate) CERTIFICATE_FACTORY.generateCertificate(is);
     }
 
+    public static Collection<? extends Certificate> certificates() throws CertificateException {
+        ByteArrayInputStream is1 = new ByteArrayInputStream((TestCertificate.ONE.encoded + "\n").getBytes());
+        ByteArrayInputStream is2 = new ByteArrayInputStream(TestCertificate.TWO.encoded.getBytes());
+        return CERTIFICATE_FACTORY.generateCertificates(new SequenceInputStream(is1, is2));
+    }
+
+    public static void certPath() throws CertificateException {
+        CertPath cp = CERTIFICATE_FACTORY.generateCertPath(List.of(TestCertificate.ONE.certificate(),
+                TestCertificate.TWO.certificate()));
+
+        // Get the encoded form of the CertPath we made
+        byte[] encoded = cp.getEncoded("PKCS7");
+        CERTIFICATE_FACTORY.generateCertPath(new ByteArrayInputStream(encoded), "PKCS7");
+    }
+
+    public static void keyToolTest() throws Exception {
+        String config = """
+                <?xml version="1.0" encoding="UTF-8"?>
+                <configuration version="2.0" description="test">
+                    <event name="jdk.X509Certificate">
+                       <setting name="enabled">true</setting>
+                       <setting name="stackTrace">true</setting>
+                    </event>
+                </configuration>""";
+        Files.writeString(Path.of("config.jfc"), config);
+
+        SecurityTools.keytool("-J-XX:StartFlightRecording=filename=keytool.jfr,settings=config.jfc",
+            "-genkeypair", "-alias", "testkey", "-keyalg", "RSA", "-keysize", "2048", "-dname",
+            "CN=8292033.oracle.com,OU=JPG,C=US", "-keypass", "changeit",
+            "-validity", "365", "-keystore", "keystore.pkcs12", "-storepass", "changeit")
+            .shouldHaveExitValue(0);
+        // The keytool command will load the keystore and call CertificateFactory.generateCertificate
+        jfrTool("keytool.jfr")
+            .shouldContain("8292033.oracle.com") // should record our new cert
+            .shouldNotContain("algorithm = N/A") // shouldn't record cert under construction
+            .shouldHaveExitValue(0);
+    }
+
+    private static OutputAnalyzer jfrTool(String jfrFile) throws Exception {
+        ProcessBuilder pb = new ProcessBuilder();
+        pb.command(new String[] { JDKToolFinder.getJDKTool("jfr"), "print", "--events",
+                "jdk.X509Certificate", jfrFile});
+        return new OutputAnalyzer(pb.start());
+    }
+
     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.