From eac9a05321bf251dfe34891aa7c4be6d63cb3c34 Mon Sep 17 00:00:00 2001 From: Goetz Lindenmaier Date: Tue, 24 Jan 2023 08:54:41 +0000 Subject: [PATCH] 8292033: Move jdk.X509Certificate event logic to JCA layer Reviewed-by: mbaesken Backport-of: 102b2b32feec4727145be4814eb1a69ef462ff16 --- .../security/cert/CertificateFactory.java | 12 +-- .../internal/event/X509CertificateEvent.java | 11 ++- .../classes/sun/security/jca/JCAUtil.java | 46 ++++++++++++ .../sun/security/provider/X509Factory.java | 46 +----------- .../provider/certpath/OCSPResponse.java | 2 +- .../certpath/X509CertificatePair.java | 6 +- .../sun/security/x509/X509CertImpl.java | 8 ++ .../security/TestX509CertificateEvent.java | 53 +++++++++---- .../security/TestX509ValidationEvent.java | 2 +- .../logging/TestX509CertificateLog.java | 4 +- .../logging/TestX509ValidationLog.java | 3 +- .../test/lib/security/TestCertificate.java | 74 ++++++++++++++++--- 12 files changed, 183 insertions(+), 84 deletions(-) diff --git a/src/java.base/share/classes/java/security/cert/CertificateFactory.java b/src/java.base/share/classes/java/security/cert/CertificateFactory.java index 35b0f86168f..326ee7b4676 100644 --- a/src/java.base/share/classes/java/security/cert/CertificateFactory.java +++ b/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 final String getType() { public final Certificate generateCertificate(InputStream inStream) throws CertificateException { - return certFacSpi.engineGenerateCertificate(inStream); + Certificate c = certFacSpi.engineGenerateCertificate(inStream); + JCAUtil.tryCommitCertEvent(c); + return c; } /** diff --git a/src/java.base/share/classes/jdk/internal/event/X509CertificateEvent.java b/src/java.base/share/classes/jdk/internal/event/X509CertificateEvent.java index b145abceb69..5b668ca2a05 100644 --- a/src/java.base/share/classes/jdk/internal/event/X509CertificateEvent.java +++ b/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 @@ */ 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; diff --git a/src/java.base/share/classes/sun/security/jca/JCAUtil.java b/src/java.base/share/classes/sun/security/jca/JCAUtil.java index e4a38b37309..862a33530ed 100644 --- a/src/java.base/share/classes/sun/security/jca/JCAUtil.java +++ b/src/java.base/share/classes/sun/security/jca/JCAUtil.java @@ -27,6 +27,13 @@ import java.lang.ref.*; import java.security.*; +import java.security.PublicKey; +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. @@ -91,6 +98,45 @@ public static SecureRandom getDefSecureRandom() { } } 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); + } + } } } diff --git a/src/java.base/share/classes/sun/security/provider/X509Factory.java b/src/java.base/share/classes/sun/security/provider/X509Factory.java index 68780a2f485..f823f529629 100644 --- a/src/java.base/share/classes/sun/security/provider/X509Factory.java +++ b/src/java.base/share/classes/sun/security/provider/X509Factory.java @@ -26,12 +26,9 @@ package sun.security.provider; import java.io.*; -import java.security.PublicKey; import java.util.*; import java.security.cert.*; -import jdk.internal.event.EventHelper; -import jdk.internal.event.X509CertificateEvent; import sun.security.util.KeyUtil; import sun.security.util.Pem; import sun.security.x509.*; @@ -104,8 +101,6 @@ public Certificate engineGenerateCertificate(InputStream is) } cert = new X509CertImpl(encoding); addToCache(certCache, cert.getEncodedInternal(), cert); - // record cert details if necessary - commitEvent(cert); return cert; } else { throw new IOException("Empty input"); @@ -473,7 +468,7 @@ public Collection engineGenerateCRLs( } } catch (ParsingException e) { while (data != null) { - coll.add(new X509CertImpl(data)); + coll.add(X509CertImpl.newX509CertImpl(data)); data = readOneBlock(pbis); } } @@ -767,43 +762,4 @@ private static int readBERInternal(InputStream is, } return tag; } - - private 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); - } - } - } } diff --git a/src/java.base/share/classes/sun/security/provider/certpath/OCSPResponse.java b/src/java.base/share/classes/sun/security/provider/certpath/OCSPResponse.java index e6b9c2b4992..d31f206bf08 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/OCSPResponse.java +++ b/src/java.base/share/classes/sun/security/provider/certpath/OCSPResponse.java @@ -355,7 +355,7 @@ public OCSPResponse(byte[] bytes) throws IOException { 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) { diff --git a/src/java.base/share/classes/sun/security/provider/certpath/X509CertificatePair.java b/src/java.base/share/classes/sun/security/provider/certpath/X509CertificatePair.java index ea139dd061d..35c64147e88 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/X509CertificatePair.java +++ b/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 @@ private void parse(DerValue val) } opt = opt.data.getDerValue(); forward = X509Factory.intern - (new X509CertImpl(opt.toByteArray())); + (X509CertImpl.newX509CertImpl(opt.toByteArray())); } break; case TAG_REVERSE: @@ -251,7 +251,7 @@ private void parse(DerValue val) } opt = opt.data.getDerValue(); reverse = X509Factory.intern - (new X509CertImpl(opt.toByteArray())); + (X509CertImpl.newX509CertImpl(opt.toByteArray())); } break; default: diff --git a/src/java.base/share/classes/sun/security/x509/X509CertImpl.java b/src/java.base/share/classes/sun/security/x509/X509CertImpl.java index b658a94b8d6..254016d6a28 100644 --- a/src/java.base/share/classes/sun/security/x509/X509CertImpl.java +++ b/src/java.base/share/classes/sun/security/x509/X509CertImpl.java @@ -41,6 +41,7 @@ import javax.security.auth.x500.X500Principal; +import sun.security.jca.JCAUtil; import sun.security.util.*; import sun.security.provider.X509Factory; @@ -304,6 +305,13 @@ public X509CertImpl(DerValue derVal) throws CertificateException { } } + // 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. * diff --git a/test/jdk/jdk/jfr/event/security/TestX509CertificateEvent.java b/test/jdk/jdk/jfr/event/security/TestX509CertificateEvent.java index 02035ad2533..e7e905830a4 100644 --- a/test/jdk/jdk/jfr/event/security/TestX509CertificateEvent.java +++ b/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.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 { - try (Recording recording = new Recording()) { - recording.enable(EventNames.X509Certificate); - recording.start(); - + public static void main(String[] args) throws Throwable { + testCall(() -> { + // test regular cert construction TestCertificate.ONE.certificate(); TestCertificate.TWO.certificate(); - // Generate twice to make sure only one event per certificate is generated + // Generate twice to make sure we (now) capture all generate cert events TestCertificate.ONE.certificate(); TestCertificate.TWO.certificate(); + }, 4, true); - recording.stop(); + 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(); + recording.stop(); List events = Events.fromRecording(recording); - Asserts.assertEquals(events.size(), 2, "Incorrect number of X509Certificate events"); - assertEvent(events, TestCertificate.ONE); - assertEvent(events, TestCertificate.TWO); + if (expected >= 0) { + Asserts.assertEquals(events.size(), expected, "Incorrect number of events"); + } + if (runAsserts) { + assertEvent(events, TestCertificate.ONE); + assertEvent(events, TestCertificate.TWO); + } } } diff --git a/test/jdk/jdk/jfr/event/security/TestX509ValidationEvent.java b/test/jdk/jdk/jfr/event/security/TestX509ValidationEvent.java index 50f472301f0..816b8220ed7 100644 --- a/test/jdk/jdk/jfr/event/security/TestX509ValidationEvent.java +++ b/test/jdk/jdk/jfr/event/security/TestX509ValidationEvent.java @@ -39,7 +39,7 @@ * @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 { diff --git a/test/jdk/jdk/security/logging/TestX509CertificateLog.java b/test/jdk/jdk/security/logging/TestX509CertificateLog.java index cbeb03039d1..018ec60181d 100644 --- a/test/jdk/jdk/security/logging/TestX509CertificateLog.java +++ b/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 @@ * @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 */ diff --git a/test/jdk/jdk/security/logging/TestX509ValidationLog.java b/test/jdk/jdk/security/logging/TestX509ValidationLog.java index 1963fd83294..805cd546729 100644 --- a/test/jdk/jdk/security/logging/TestX509ValidationLog.java +++ b/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 @@ * @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 */ diff --git a/test/lib/jdk/test/lib/security/TestCertificate.java b/test/lib/jdk/test/lib/security/TestCertificate.java index 47fd9de0d06..914247e23f8 100644 --- a/test/lib/jdk/test/lib/security/TestCertificate.java +++ b/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.security.cert.CertPathValidator; -import java.security.cert.CertificateException; -import java.security.cert.CertificateFactory; -import java.security.cert.PKIXParameters; -import java.security.cert.TrustAnchor; -import java.security.cert.X509Certificate; -import java.util.Collections; -import java.util.Date; -import java.util.List; +import java.io.IOException; +import java.io.SequenceInputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.*; +import java.security.cert.*; +import java.security.cert.Certificate; +import java.util.*; + +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 X509Certificate certificate() throws CertificateException { return (X509Certificate) CERTIFICATE_FACTORY.generateCertificate(is); } + public static Collection 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 = """ + + + + true + true + + """; + 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. @@ -187,4 +237,4 @@ public static void generateChain(boolean selfSignedTest, boolean trustAnchorCert validator.validate(path, params); } } -} \ No newline at end of file +}