RFR: 8148188: Enhance the security libraries to record events of interest

Erik Gahlin erik.gahlin at oracle.com
Sun Jun 24 13:21:33 UTC 2018


Hi Sean,

Some of the changes in the webrev belongs to JDK-8203629 and should be 
removed for clarity.

Some initial comments:

default.jfc, profile.jfr:
The events should not have control="enable-exceptions". The purpose of 
the control attribute is so to provide parameterized configuration of 
events for JMC.  As it is now, the security events will be enabled when 
a user turns on the exception events.

X509CertEvent:
You should use milliseconds since epoch to represent a date instead of a 
string value, i.e.

     @Label("Valid From")
     @Timestamp(Timestamp.MILLISECONDS_SINCE_EPOCH)
     public long validFrom;

     @Label("Valid Until")
     @Timestamp(Timestamp.MILLISECONDS_SINCE_EPOCH)
     public long validUntil;

This following annotation adds little value

@Description("Details of Security Property")

I would either remove the annotation, or provide information that helps 
a user understand the event. For instance, what is X509, and what kind 
of certificates are we talking about?

@Category("Java Application")

I'm a bit worried that we will pollute the "Java Application" namespace 
if we add lots of JDK events in that category. We may want to add a new 
top level category "Java Development Kit", analogous to the "Java 
Virtual Machine" category, and put all security related events in 
subcategory, perhaps called "Security".

@Label("X509Cert")

The label should be human readable name, with spaces, title cased etc. I 
would recommend "X.509 Certificate". In general, avoid abbreviations 
like "certs" and instead use labels such as "Certificate Chain". The 
label should be short and not a full sentence.

For details see,
https://docs.oracle.com/javase/10/docs/api/jdk/jfr/Label.html

I think it would be good to separate testing of JFR and logging into 
different files / tests. I would prefer that the test name is the same 
as the event prefixed with "Test", i.e TestX509CertificateEvent, as this 
is the pattern used by other JFR tests.

I reworked one of the tests to how I like to see it:

/*
  * @test
  * @key jfr
  * @library /test/lib
  * @run main/othervm jdk.jfr.event.security.TestX509CertificateEvent
  */
public class TestX509CertificateEvent {

     private static final String CERTIFICATE_1 = "...";
     private static final String CERTIFICATE_2 = "...";

     public static void main(String... args) throws CertificateException {

          Recording r = new Recording();
          r.enable(EventNames.X509Certificate).withoutStackTrace();
          r.start();

          CertificateFactory cf = CertificateFactory.getInstance("X.509");
          cf.generateCertificate(new 
ByteArrayInputStream(CERTIFICATE_1.getBytes()));
          cf.generateCertificate(new 
ByteArrayInputStream(CERTIFICATE_2.getBytes()));

          // Make sure only one event per certificate
          cf.generateCertificate(new 
ByteArrayInputStream(CERTIFICATE_1.getBytes()));
          cf.generateCertificate(new 
ByteArrayInputStream(CERTIFICATE_2.getBytes()));

          r.stop();

          List<RecordedEvent> events = Events.fromRecording(r);
          Asserts.assertEquals(events.size(), 2, "Expected two X.509 
Certificate events");

          assertEvent(events, "1000", "SHA256withRSA",
                     "CN=SSLCertificate, O=SomeCompany",
                     "CN=Intermediate CA Cert, O=SomeCompany",
                      "RSA", 2048);
          assertEvent(events, "1000", "SHA256withRSA",
                     "CN=SSLCertificate, O=SomeCompany",
                     "CN=Intermediate CA Cert, O=SomeCompany",
                      "RSA", 2048);
     }

     private static void assertEvent(List<RecordedEvent> events, String 
certID, String algId, String subject,
             String issuer, String keyType, int length) throws Exception {

         for (RecordedEvent e : events) {
             if (e.getString("serialNumber").equals(certID)) {
                 Events.assertField(e, "algId").equal(algId);
                 ...
                 return;
             }
         }
         System.out.println(events);
         throw new Exception("Could not find event with Cert ID: " + 
certID);
     }
}

The reworked example uses the Events.assertField method, which will give 
context if the assertion fails. Keeping the test simple, means it can be 
analyzed quickly if it fails in testing. There is no new test framework 
to learn, or methods to search for, and it is usually not hard to 
determine if the failure is product, test or infrastructure related, and 
what component (team) should be assigned the issue. The use of 
EventNames.X509Certificate means all occurrences of the event can be 
tracked done in an IDE using find by reference.

Thanks
Erik

> Following on from the recent JDK-8203629 code review, I'd like to 
> propose enhancements on how we can record events in security libs. The 
> introduction of the JFR libraries can give us much better ways of 
> examining JDK actions. For the initial phase, I'm looking to enhance 
> some key security library events in JDK 11 so that they can be either 
> recorded to JFR, logged to a traditional logger, or both.
>
> Examples of how useful JFR recordings could be can be seen here :
>
> http://cr.openjdk.java.net/~coffeys/event_snaps/X509Event_1.png
> http://cr.openjdk.java.net/~coffeys/event_snaps/securityProp_1.png
> http://cr.openjdk.java.net/~coffeys/event_snaps/securityProp_2.png
> http://cr.openjdk.java.net/~coffeys/event_snaps/TLSEvent_1.png
>
> securityProp_2.png gives an example of how the JFR recording can be 
> queried to quickly locate events of interest (in this case, code 
> setting the jdk.tls.* Security properties). I still need to clean up 
> the TLSEvents testcase to improve test coverage and hope to do that in 
> coming days.
>
> JBS record :
>  * https://bugs.openjdk.java.net/browse/JDK-8148188
>
> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8148188.v1/webrev/
>



More information about the security-dev mailing list