RFR: 8292177: InitialSecurityProperty JFR event
New JFR event to record state of initial security properties. Debug output is also now added for these properties via -Djava.security.debug=properties ------------- Commit messages: - merge with master - Correct test bug ids - fix up imports - Add security debug test logic - Add JFR testcase - remove mirror event - 8292177 initial commit Changes: https://git.openjdk.org/jdk/pull/10394/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=10394&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8292177 Stats: 200 lines in 11 files changed: 196 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/10394.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10394/head:pull/10394 PR: https://git.openjdk.org/jdk/pull/10394
On Thu, 22 Sep 2022 15:57:56 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java line 312:
310: } 311: 312: private static void emitInitialSecurityProperties() {
Is the `Security` class loaded and have the properties always been populated at this point? ProtectionDomain` doesn't reference the `Security` class AFAICT. src/jdk.jfr/share/conf/jfr/default.jfc line 713:
711: 712: <event name="jdk.InitialSecurityProperty"> 713: <setting name="enabled">true</setting>
The other security related events are not enabled by default. Is this one enabled because it is only generated once? It seems it may still have some startup overhead because AFAIU it forces a load of security properties even if they are never accessed? Perhaps I don't fully understand how this event works though. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Tue, 27 Sep 2022 20:29:31 GMT, Sean Mullan <mullan@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java line 312:
310: } 311: 312: private static void emitInitialSecurityProperties() {
Is the `Security` class loaded and have the properties always been populated at this point? ProtectionDomain` doesn't reference the `Security` class AFAICT.
Good catch. JFR does load the Security class [1] via other dependencies but we shouldn't depend on that. I'll add a null check here. If Security class hasn't been loaded, then we shouldn't record any events. [1] java.lang.Throwable at java.base/java.security.Security.<clinit>(Security.java:73) at java.base/sun.security.util.SecurityProperties.getOverridableProperty(SecurityProperties.java:57) at java.base/sun.security.util.SecurityProperties.privilegedGetOverridable(SecurityProperties.java:48) at java.base/sun.security.util.SecurityProperties.includedInExceptions(SecurityProperties.java:72) at java.base/sun.security.util.SecurityProperties.<clinit>(SecurityProperties.java:36) at java.base/sun.security.util.FilePermCompat.<clinit>(FilePermCompat.java:43) at java.base/java.security.AccessControlContext.<init>(AccessControlContext.java:270) at java.base/java.security.AccessController.createWrapper(AccessController.java:649) at java.base/java.security.AccessController.doPrivileged(AccessController.java:461) at jdk.jfr/jdk.jfr.internal.SecuritySupport.doPrivilegedWithReturn(SecuritySupport.java:261) at jdk.jfr/jdk.jfr.internal.SecuritySupport.getPathInProperty(SecuritySupport.java:331) at jdk.jfr/jdk.jfr.internal.SecuritySupport.<clinit>(SecuritySupport.java:80) at jdk.jfr/jdk.jfr.internal.JVMSupport.checkAvailability(JVMSupport.java:46) at jdk.jfr/jdk.jfr.internal.JVMSupport.<clinit>(JVMSupport.java:41) at jdk.jfr/jdk.jfr.internal.Logger.<clinit>(Logger.java:41) at jdk.jfr/jdk.jfr.internal.dcmd.AbstractDCmd.execute(AbstractDCmd.java:75)
src/jdk.jfr/share/conf/jfr/default.jfc line 713:
711: 712: <event name="jdk.InitialSecurityProperty"> 713: <setting name="enabled">true</setting>
The other security related events are not enabled by default. Is this one enabled because it is only generated once? It seems it may still have some startup overhead because AFAIU it forces a load of security properties even if they are never accessed? Perhaps I don't fully understand how this event works though.
Yes - the thinking here is that since this is a one time event, we can have it enabled. It's similar to the InitialSystemProperty event. We won't force loading of Security properties/class. We shouldn't at least. If no security properties are read in at time of JFR event commit, then we should have no InitialSecurityProperty events. See below/next comment. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Wed, 28 Sep 2022 10:06:54 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
src/jdk.jfr/share/conf/jfr/default.jfc line 713:
711: 712: <event name="jdk.InitialSecurityProperty"> 713: <setting name="enabled">true</setting>
The other security related events are not enabled by default. Is this one enabled because it is only generated once? It seems it may still have some startup overhead because AFAIU it forces a load of security properties even if they are never accessed? Perhaps I don't fully understand how this event works though.
Yes - the thinking here is that since this is a one time event, we can have it enabled. It's similar to the InitialSystemProperty event.
We won't force loading of Security properties/class. We shouldn't at least. If no security properties are read in at time of JFR event commit, then we should have no InitialSecurityProperty events. See below/next comment.
Ok, just so I understand, you want to make sure that if JFR is started after the security properties have already been read, then they are still recorded, right? ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Wed, 28 Sep 2022 12:26:49 GMT, Sean Mullan <mullan@openjdk.org> wrote:
Yes - the thinking here is that since this is a one time event, we can have it enabled. It's similar to the InitialSystemProperty event.
We won't force loading of Security properties/class. We shouldn't at least. If no security properties are read in at time of JFR event commit, then we should have no InitialSecurityProperty events. See below/next comment.
Ok, just so I understand, you want to make sure that if JFR is started after the security properties have already been read, then they are still recorded, right?
Correct - this type of event period (beginChunk) will fire once when the JFR recording is begun. It should capture Security Properties (if java.security.Security is loaded) for any recording, no matter when it might begin or end. Similar to how InitialSystemProperty is captured (but that's implemented at native/VM level) ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Wed, 28 Sep 2022 12:52:02 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
Ok, just so I understand, you want to make sure that if JFR is started after the security properties have already been read, then they are still recorded, right?
Correct - this type of event period (beginChunk) will fire once when the JFR recording is begun. It should capture Security Properties (if java.security.Security is loaded) for any recording, no matter when it might begin or end. Similar to how InitialSystemProperty is captured (but that's implemented at native/VM level)
How does it capture the event if JFR was started before the security properties were read? I would think you still need some additional code in Security.java to record the properties if the event is enabled. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Wed, 28 Sep 2022 16:32:17 GMT, Sean Mullan <mullan@openjdk.org> wrote:
Correct - this type of event period (beginChunk) will fire once when the JFR recording is begun. It should capture Security Properties (if java.security.Security is loaded) for any recording, no matter when it might begin or end. Similar to how InitialSystemProperty is captured (but that's implemented at native/VM level)
How does it capture the event if JFR was started before the security properties were read? I would think you still need some additional code in Security.java to record the properties if the event is enabled.
As per yesterday's stack trace, JFR triggers loading of the Security class - so your scenario won't arise with current state. We could include the new Event with period of `endChunk `instead of `beingChunk `setting. That should ensure the properties are only captured when the JFR recording is exiting. @egahlin - would you have a preference on this ? ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Thu, 29 Sep 2022 10:02:05 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
How does it capture the event if JFR was started before the security properties were read? I would think you still need some additional code in Security.java to record the properties if the event is enabled.
As per yesterday's stack trace, JFR triggers loading of the Security class - so your scenario won't arise with current state. We could include the new Event with period of `endChunk `instead of `beingChunk `setting. That should ensure the properties are only captured when the JFR recording is exiting. @egahlin - would you have a preference on this ?
With event streaming, beginChunk is usually to prefer. Otherwise, a client that monitors the JVM must wait until the first chunk rotation to get the data. That said, we want startup to be quick. There should probably be a common parameter, i.e. security=off/normal/audit/trace, that handles enablement for all security events. I don't know how expensive this event is and where it would fit among those categories? If the event triggers class loading, it might make sense to check if the event is enabled first. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Thu, 29 Sep 2022 11:41:02 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
As per yesterday's stack trace, JFR triggers loading of the Security class - so your scenario won't arise with current state. We could include the new Event with period of `endChunk `instead of `beingChunk `setting. That should ensure the properties are only captured when the JFR recording is exiting. @egahlin - would you have a preference on this ?
With event streaming, beginChunk is usually to prefer. Otherwise, a client that monitors the JVM must wait until the first chunk rotation to get the data.
That said, we want startup to be quick. There should probably be a common parameter, i.e. security=off/normal/audit/trace, that handles enablement for all security events. I don't know how expensive this event is and where it would fit among those categories?
If the event triggers class loading, it might make sense to check if the event is enabled first.
Thanks @egahlin - maybe we can leave it at beginChunk setting then. I've been doing some testing to satisfy myself that the impact of this event on performance is minimal, Running the new `emitInitialSecurityProperties()` is showing a cost of ~ 1.6ms (1602998 ns). This new Event itself doesn't trigger extra class loading AFAICT. I went back to a jdk 20 binary without this patch and ran some tests. `ProtectionDomain ` is a very early class to initialize [1] (initPhase2) Without JFR, `java.security.Security` will initialize in a default JDK with a JMX `Agent.startLocalManagementAgent` call in a simple HelloWorld test which prints "Hello" and then sleeps [2] - the JMX thread starts after about 3 seconds of runtime. Without JFR and by using the `-XX:+DisableAttachMechanism` option, the Security class will not load in same test. If JFR is on, then Security class is already being loaded, even without this patch [3] [1] at java.base/java.security.ProtectionDomain.<clinit>(ProtectionDomain.java:64) at java.base/java.lang.ClassLoader.<init>(ClassLoader.java:316) at java.base/java.lang.ClassLoader.<init>(ClassLoader.java:431) at java.base/java.security.SecureClassLoader.<init>(SecureClassLoader.java:113) at java.base/jdk.internal.loader.BuiltinClassLoader.<init>(BuiltinClassLoader.java:194) at java.base/jdk.internal.loader.ClassLoaders$BootClassLoader.<init>(ClassLoaders.java:135) at java.base/jdk.internal.loader.ClassLoaders.<clinit>(ClassLoaders.java:79) at java.base/jdk.internal.loader.BootLoader.loadModule(BootLoader.java:120) at java.base/jdk.internal.module.ModuleBootstrap.boot2(ModuleBootstrap.java:266) at java.base/jdk.internal.module.ModuleBootstrap.boot(ModuleBootstrap.java:174) at java.base/java.lang.System.initPhase2(System.java:2214) [2] at java.base/java.security.Security.<clinit>(Security.java:73) at java.base/sun.net.InetAddressCachePolicy$1.run(InetAddressCachePolicy.java:93) at java.base/sun.net.InetAddressCachePolicy$1.run(InetAddressCachePolicy.java:90) at java.base/java.security.AccessController.doPrivileged(AccessController.java:319) at java.base/sun.net.InetAddressCachePolicy.<clinit>(InetAddressCachePolicy.java:89) at java.base/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:1005) at java.base/java.net.InetAddress.getAllByName0(InetAddress.java:1658) at java.base/java.net.InetAddress.getAllByName(InetAddress.java:1524) at java.base/java.net.InetAddress.getByName(InetAddress.java:1413) at jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startLocalConnectorServer(ConnectorBootstrap.java:531) at jdk.management.agent/jdk.internal.agent.Agent.startLocalManagementAgent(Agent.java:317) [3] at java.base/java.security.Security.<clinit>(Security.java:73) at java.base/sun.security.util.SecurityProperties.getOverridableProperty(SecurityProperties.java:57) at java.base/sun.security.util.SecurityProperties.privilegedGetOverridable(SecurityProperties.java:48) at java.base/sun.security.util.SecurityProperties.includedInExceptions(SecurityProperties.java:72) at java.base/sun.security.util.SecurityProperties.<clinit>(SecurityProperties.java:36) at java.base/sun.security.util.FilePermCompat.<clinit>(FilePermCompat.java:43) at java.base/java.security.AccessControlContext.<init>(AccessControlContext.java:270) at java.base/java.security.AccessController.createWrapper(AccessController.java:649) at java.base/java.security.AccessController.doPrivileged(AccessController.java:461) at jdk.jfr/jdk.jfr.internal.SecuritySupport.doPrivilegedWithReturn(SecuritySupport.java:261) at jdk.jfr/jdk.jfr.internal.SecuritySupport.getPathInProperty(SecuritySupport.java:331) at jdk.jfr/jdk.jfr.internal.SecuritySupport.<clinit>(SecuritySupport.java:80) at jdk.jfr/jdk.jfr.internal.JVMSupport.checkAvailability(JVMSupport.java:46) at jdk.jfr/jdk.jfr.internal.JVMSupport.<clinit>(JVMSupport.java:41) at jdk.jfr/jdk.jfr.internal.Logger.<clinit>(Logger.java:41) at jdk.jfr/jdk.jfr.internal.dcmd.AbstractDCmd.execute(AbstractDCmd.java:75) ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Thu, 29 Sep 2022 15:12:06 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
With event streaming, beginChunk is usually to prefer. Otherwise, a client that monitors the JVM must wait until the first chunk rotation to get the data.
That said, we want startup to be quick. There should probably be a common parameter, i.e. security=off/normal/audit/trace, that handles enablement for all security events. I don't know how expensive this event is and where it would fit among those categories?
If the event triggers class loading, it might make sense to check if the event is enabled first.
Thanks @egahlin - maybe we can leave it at beginChunk setting then.
I've been doing some testing to satisfy myself that the impact of this event on performance is minimal, Running the new `emitInitialSecurityProperties()` is showing a cost of ~ 1.6ms (1602998 ns).
This new Event itself doesn't trigger extra class loading AFAICT. I went back to a jdk 20 binary without this patch and ran some tests.
`ProtectionDomain ` is a very early class to initialize [1] (initPhase2)
Without JFR, `java.security.Security` will initialize in a default JDK with a JMX `Agent.startLocalManagementAgent` call in a simple HelloWorld test which prints "Hello" and then sleeps [2] - the JMX thread starts after about 3 seconds of runtime.
Without JFR and by using the `-XX:+DisableAttachMechanism` option, the Security class will not load in same test.
If JFR is on, then Security class is already being loaded, even without this patch [3]
[1]
at java.base/java.security.ProtectionDomain.<clinit>(ProtectionDomain.java:64) at java.base/java.lang.ClassLoader.<init>(ClassLoader.java:316) at java.base/java.lang.ClassLoader.<init>(ClassLoader.java:431) at java.base/java.security.SecureClassLoader.<init>(SecureClassLoader.java:113) at java.base/jdk.internal.loader.BuiltinClassLoader.<init>(BuiltinClassLoader.java:194) at java.base/jdk.internal.loader.ClassLoaders$BootClassLoader.<init>(ClassLoaders.java:135) at java.base/jdk.internal.loader.ClassLoaders.<clinit>(ClassLoaders.java:79) at java.base/jdk.internal.loader.BootLoader.loadModule(BootLoader.java:120) at java.base/jdk.internal.module.ModuleBootstrap.boot2(ModuleBootstrap.java:266) at java.base/jdk.internal.module.ModuleBootstrap.boot(ModuleBootstrap.java:174) at java.base/java.lang.System.initPhase2(System.java:2214)
[2]
at java.base/java.security.Security.<clinit>(Security.java:73) at java.base/sun.net.InetAddressCachePolicy$1.run(InetAddressCachePolicy.java:93) at java.base/sun.net.InetAddressCachePolicy$1.run(InetAddressCachePolicy.java:90) at java.base/java.security.AccessController.doPrivileged(AccessController.java:319) at java.base/sun.net.InetAddressCachePolicy.<clinit>(InetAddressCachePolicy.java:89) at java.base/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:1005) at java.base/java.net.InetAddress.getAllByName0(InetAddress.java:1658) at java.base/java.net.InetAddress.getAllByName(InetAddress.java:1524) at java.base/java.net.InetAddress.getByName(InetAddress.java:1413) at jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startLocalConnectorServer(ConnectorBootstrap.java:531) at jdk.management.agent/jdk.internal.agent.Agent.startLocalManagementAgent(Agent.java:317)
[3]
at java.base/java.security.Security.<clinit>(Security.java:73) at java.base/sun.security.util.SecurityProperties.getOverridableProperty(SecurityProperties.java:57) at java.base/sun.security.util.SecurityProperties.privilegedGetOverridable(SecurityProperties.java:48) at java.base/sun.security.util.SecurityProperties.includedInExceptions(SecurityProperties.java:72) at java.base/sun.security.util.SecurityProperties.<clinit>(SecurityProperties.java:36) at java.base/sun.security.util.FilePermCompat.<clinit>(FilePermCompat.java:43) at java.base/java.security.AccessControlContext.<init>(AccessControlContext.java:270) at java.base/java.security.AccessController.createWrapper(AccessController.java:649) at java.base/java.security.AccessController.doPrivileged(AccessController.java:461) at jdk.jfr/jdk.jfr.internal.SecuritySupport.doPrivilegedWithReturn(SecuritySupport.java:261) at jdk.jfr/jdk.jfr.internal.SecuritySupport.getPathInProperty(SecuritySupport.java:331) at jdk.jfr/jdk.jfr.internal.SecuritySupport.<clinit>(SecuritySupport.java:80) at jdk.jfr/jdk.jfr.internal.JVMSupport.checkAvailability(JVMSupport.java:46) at jdk.jfr/jdk.jfr.internal.JVMSupport.<clinit>(JVMSupport.java:41) at jdk.jfr/jdk.jfr.internal.Logger.<clinit>(Logger.java:41) at jdk.jfr/jdk.jfr.internal.dcmd.AbstractDCmd.execute(AbstractDCmd.java:75)
When support for the SM is removed, the dependency on `AccessController.doPrivileged` will be removed and there may no longer be a JFR dependency on loading the `Security` class. So, it is ok for now, but a solution that doesn't depend on this might be better in the long run. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Fri, 30 Sep 2022 19:32:29 GMT, Sean Mullan <mullan@openjdk.org> wrote:
Thanks @egahlin - maybe we can leave it at beginChunk setting then.
I've been doing some testing to satisfy myself that the impact of this event on performance is minimal, Running the new `emitInitialSecurityProperties()` is showing a cost of ~ 1.6ms (1602998 ns).
This new Event itself doesn't trigger extra class loading AFAICT. I went back to a jdk 20 binary without this patch and ran some tests.
`ProtectionDomain ` is a very early class to initialize [1] (initPhase2)
Without JFR, `java.security.Security` will initialize in a default JDK with a JMX `Agent.startLocalManagementAgent` call in a simple HelloWorld test which prints "Hello" and then sleeps [2] - the JMX thread starts after about 3 seconds of runtime.
Without JFR and by using the `-XX:+DisableAttachMechanism` option, the Security class will not load in same test.
If JFR is on, then Security class is already being loaded, even without this patch [3]
[1]
at java.base/java.security.ProtectionDomain.<clinit>(ProtectionDomain.java:64) at java.base/java.lang.ClassLoader.<init>(ClassLoader.java:316) at java.base/java.lang.ClassLoader.<init>(ClassLoader.java:431) at java.base/java.security.SecureClassLoader.<init>(SecureClassLoader.java:113) at java.base/jdk.internal.loader.BuiltinClassLoader.<init>(BuiltinClassLoader.java:194) at java.base/jdk.internal.loader.ClassLoaders$BootClassLoader.<init>(ClassLoaders.java:135) at java.base/jdk.internal.loader.ClassLoaders.<clinit>(ClassLoaders.java:79) at java.base/jdk.internal.loader.BootLoader.loadModule(BootLoader.java:120) at java.base/jdk.internal.module.ModuleBootstrap.boot2(ModuleBootstrap.java:266) at java.base/jdk.internal.module.ModuleBootstrap.boot(ModuleBootstrap.java:174) at java.base/java.lang.System.initPhase2(System.java:2214)
[2]
at java.base/java.security.Security.<clinit>(Security.java:73) at java.base/sun.net.InetAddressCachePolicy$1.run(InetAddressCachePolicy.java:93) at java.base/sun.net.InetAddressCachePolicy$1.run(InetAddressCachePolicy.java:90) at java.base/java.security.AccessController.doPrivileged(AccessController.java:319) at java.base/sun.net.InetAddressCachePolicy.<clinit>(InetAddressCachePolicy.java:89) at java.base/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:1005) at java.base/java.net.InetAddress.getAllByName0(InetAddress.java:1658) at java.base/java.net.InetAddress.getAllByName(InetAddress.java:1524) at java.base/java.net.InetAddress.getByName(InetAddress.java:1413) at jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startLocalConnectorServer(ConnectorBootstrap.java:531) at jdk.management.agent/jdk.internal.agent.Agent.startLocalManagementAgent(Agent.java:317)
[3]
at java.base/java.security.Security.<clinit>(Security.java:73) at java.base/sun.security.util.SecurityProperties.getOverridableProperty(SecurityProperties.java:57) at java.base/sun.security.util.SecurityProperties.privilegedGetOverridable(SecurityProperties.java:48) at java.base/sun.security.util.SecurityProperties.includedInExceptions(SecurityProperties.java:72) at java.base/sun.security.util.SecurityProperties.<clinit>(SecurityProperties.java:36) at java.base/sun.security.util.FilePermCompat.<clinit>(FilePermCompat.java:43) at java.base/java.security.AccessControlContext.<init>(AccessControlContext.java:270) at java.base/java.security.AccessController.createWrapper(AccessController.java:649) at java.base/java.security.AccessController.doPrivileged(AccessController.java:461) at jdk.jfr/jdk.jfr.internal.SecuritySupport.doPrivilegedWithReturn(SecuritySupport.java:261) at jdk.jfr/jdk.jfr.internal.SecuritySupport.getPathInProperty(SecuritySupport.java:331) at jdk.jfr/jdk.jfr.internal.SecuritySupport.<clinit>(SecuritySupport.java:80) at jdk.jfr/jdk.jfr.internal.JVMSupport.checkAvailability(JVMSupport.java:46) at jdk.jfr/jdk.jfr.internal.JVMSupport.<clinit>(JVMSupport.java:41) at jdk.jfr/jdk.jfr.internal.Logger.<clinit>(Logger.java:41) at jdk.jfr/jdk.jfr.internal.dcmd.AbstractDCmd.execute(AbstractDCmd.java:75)
When support for the SM is removed, the dependency on `AccessController.doPrivileged` will be removed and there may no longer be a JFR dependency on loading the `Security` class. So, it is ok for now, but a solution that doesn't depend on this might be better in the long run.
fair point. Worse case scenario is that we don't record security properties on start up. Not the case for now though. I've added some extra code to check for this in the test. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Thu, 22 Sep 2022 15:57:56 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Hello Sean,
Debug output is also now added for these properties via -Djava.security.debug=properties
Looking at the existing code upstream, it appears that the value `properties` is already recognized for `java.security.debug` system property. So it's not something this PR is introducing/proposing. However, the documentation for this system property doesn't seem to be listing that as a value https://docs.oracle.com/en/java/javase/19/security/troubleshooting-security..... Do you think that documentation would need to be addressed (as a separate issue)? ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Wed, 28 Sep 2022 07:06:30 GMT, Jaikiran Pai <jpai@openjdk.org> wrote:
Hello Sean,
Debug output is also now added for these properties via -Djava.security.debug=properties
Looking at the existing code upstream, it appears that the value `properties` is already recognized for `java.security.debug` system property. So it's not something this PR is introducing/proposing. However, the documentation for this system property doesn't seem to be listing that as a value https://docs.oracle.com/en/java/javase/19/security/troubleshooting-security..... Do you think that documentation would need to be addressed (as a separate issue)?
Good point Jai. It's a rarely used property value I guess, but it would be exposed via the "all" debug value option as per test. I suspect there may be other such properties also. I'll log a JBS issue to track this and to have security/docs team discuss. ------------- PR: https://git.openjdk.org/jdk/pull/10394
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision: Null check on Properties ------------- Changes: - all: https://git.openjdk.org/jdk/pull/10394/files - new: https://git.openjdk.org/jdk/pull/10394/files/5343a056..aed938d2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=10394&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10394&range=00-01 Stats: 7 lines in 1 file changed: 2 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/10394.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10394/head:pull/10394 PR: https://git.openjdk.org/jdk/pull/10394
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision: Check for 0 security events ------------- Changes: - all: https://git.openjdk.org/jdk/pull/10394/files - new: https://git.openjdk.org/jdk/pull/10394/files/aed938d2..f8faecf4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=10394&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10394&range=01-02 Stats: 7 lines in 1 file changed: 3 ins; 1 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/10394.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10394/head:pull/10394 PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 3 Oct 2022 10:30:54 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Check for 0 security events
Marked as reviewed by mullan (Reviewer). src/java.base/share/classes/jdk/internal/access/JavaSecurityAccess.java line 32:
30: import java.security.PrivilegedAction; 31: import java.security.ProtectionDomain; 32: import java.util.Properties;
Update copyright on this file. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 3 Oct 2022 10:30:54 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Check for 0 security events
src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java line 29:
27: 28: import jdk.jfr.*; 29: import jdk.jfr.internal.MirrorEvent;
Hello Sean, this `MirrorEvent` appears to be an unused import. Furthermore, as far as I know, in `core-libs` the `*` wildcard imports aren't typically used. I don't know if it's OK to use it here in the `jdk.jfr` module. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 10 Oct 2022 07:19:30 GMT, Jaikiran Pai <jpai@openjdk.org> wrote:
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Check for 0 security events
src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java line 29:
27: 28: import jdk.jfr.*; 29: import jdk.jfr.internal.MirrorEvent;
Hello Sean, this `MirrorEvent` appears to be an unused import. Furthermore, as far as I know, in `core-libs` the `*` wildcard imports aren't typically used. I don't know if it's OK to use it here in the `jdk.jfr` module.
Thanks Jai - I removed the unused import and reverted to non-wildcard import use
src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java line 318:
316: InitialSecurityPropertyEvent e = new InitialSecurityPropertyEvent(); 317: e.key = (String) entry.getKey(); 318: e.value = (String) entry.getValue();
To avoid any (odd/unexpected) `ClassCastException` here, perhaps this loop can be changed to something like:
for (Set<String> name : p.stringPropertyNames()) { InitialSecurityPropertyEvent e = new InitialSecurityPropertyEvent(); e.key = name; e.value = p.getProperty(name);
Since this code anyway wants to deal with string key/values, this wouldn't introduce any functional change and yet at the same time prevent any unexpected/theoretical `ClassCastException`s
nice - wasn't aware of that method in Properties. Code updated. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 3 Oct 2022 10:30:54 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Check for 0 security events
src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java line 318:
316: InitialSecurityPropertyEvent e = new InitialSecurityPropertyEvent(); 317: e.key = (String) entry.getKey(); 318: e.value = (String) entry.getValue();
To avoid any (odd/unexpected) `ClassCastException` here, perhaps this loop can be changed to something like: for (Set<String> name : p.stringPropertyNames()) { InitialSecurityPropertyEvent e = new InitialSecurityPropertyEvent(); e.key = name; e.value = p.getProperty(name); Since this code anyway wants to deal with string key/values, this wouldn't introduce any functional change and yet at the same time prevent any unexpected/theoretical `ClassCastException`s ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 3 Oct 2022 10:30:54 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Check for 0 security events
src/java.base/share/classes/java/security/ProtectionDomain.java line 76:
74: static class JavaSecurityAccessImpl implements JavaSecurityAccess { 75: /* cache a copy for recording purposes */ 76: static Properties initialSecurityProperties;
This doesn't look very clean. Could the Security class hold the initial security properties and provide an accessor method that JavaSecurityAccess:getIinitialProperties could use? ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 10 Oct 2022 07:31:29 GMT, Alan Bateman <alanb@openjdk.org> wrote:
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Check for 0 security events
src/java.base/share/classes/java/security/ProtectionDomain.java line 76:
74: static class JavaSecurityAccessImpl implements JavaSecurityAccess { 75: /* cache a copy for recording purposes */ 76: static Properties initialSecurityProperties;
This doesn't look very clean. Could the Security class hold the initial security properties and provide an accessor method that JavaSecurityAccess:getIinitialProperties could use?
Agree, and alternatively, it seems cleaner to add a new SharedSecrets class for `java.security.Security` and remove the dependency on PD. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 10 Oct 2022 14:19:18 GMT, Sean Mullan <mullan@openjdk.org> wrote:
src/java.base/share/classes/java/security/ProtectionDomain.java line 76:
74: static class JavaSecurityAccessImpl implements JavaSecurityAccess { 75: /* cache a copy for recording purposes */ 76: static Properties initialSecurityProperties;
This doesn't look very clean. Could the Security class hold the initial security properties and provide an accessor method that JavaSecurityAccess:getIinitialProperties could use?
Agree, and alternatively, it seems cleaner to add a new SharedSecrets class for `java.security.Security` and remove the dependency on PD.
modified code to have Security class hold the initial properties and provided an accessor method ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 10 Oct 2022 19:19:44 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
Agree, and alternatively, it seems cleaner to add a new SharedSecrets class for `java.security.Security` and remove the dependency on PD.
modified code to have Security class hold the initial properties and provided an accessor method
What about creating a new `JavaSecurityPropertiesAccess` class and moving the accessor method there? It seems it would be cleaner to remove the dependency on PD in the long run. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 10 Oct 2022 20:54:58 GMT, Sean Mullan <mullan@openjdk.org> wrote:
modified code to have Security class hold the initial properties and provided an accessor method
What about creating a new `JavaSecurityPropertiesAccess` class and moving the accessor method there? It seems it would be cleaner to remove the dependency on PD in the long run.
@seanjmullan - I looked at that approach. The `SharedSecrets.getJavaSecurityAccess().getInitialProperties();` call may trigger early initialization of the `java.security.Security` class - I'm not sure if we want that. ProtectionDomain class is currently loaded early in the JDK boot cycle. In fact the change suggested by @AlanBateman yesterday also has possibility to trigger unnecessary loading of the Security class. I might revert to the original design where we store the cached Properties in ProtectionDomain ? ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Tue, 11 Oct 2022 11:28:10 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
What about creating a new `JavaSecurityPropertiesAccess` class and moving the accessor method there? It seems it would be cleaner to remove the dependency on PD in the long run.
@seanjmullan - I looked at that approach. The `SharedSecrets.getJavaSecurityAccess().getInitialProperties();` call may trigger early initialization of the `java.security.Security` class - I'm not sure if we want that. ProtectionDomain class is currently loaded early in the JDK boot cycle.
In fact the change suggested by @AlanBateman yesterday also has possibility to trigger unnecessary loading of the Security class. I might revert to the original design where we store the cached Properties in ProtectionDomain ?
Maybe I am missing something. If this JFR event is enabled, and the properties have not yet been accessed, then it seems ok for JFR to load the `Security` class when JFR is started since the user is interested in this event. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Tue, 11 Oct 2022 12:39:14 GMT, Sean Mullan <mullan@openjdk.org> wrote:
@seanjmullan - I looked at that approach. The `SharedSecrets.getJavaSecurityAccess().getInitialProperties();` call may trigger early initialization of the `java.security.Security` class - I'm not sure if we want that. ProtectionDomain class is currently loaded early in the JDK boot cycle.
In fact the change suggested by @AlanBateman yesterday also has possibility to trigger unnecessary loading of the Security class. I might revert to the original design where we store the cached Properties in ProtectionDomain ?
Maybe I am missing something. If this JFR event is enabled, and the properties have not yet been accessed, then it seems ok for JFR to load the `Security` class when JFR is started since the user is interested in this event.
My own thoughts here would be that the JFR event system should try and avoid side effects in such areas. If the Security class hasn't been loaded at time of recording, then I'd argue that the number InitialSecurityProperty events should be zero. (which is not possible at the moment since JFR does trigger Security class loading itself anyway) ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Tue, 11 Oct 2022 12:39:14 GMT, Sean Mullan <mullan@openjdk.org> wrote:
@seanjmullan - I looked at that approach. The `SharedSecrets.getJavaSecurityAccess().getInitialProperties();` call may trigger early initialization of the `java.security.Security` class - I'm not sure if we want that. ProtectionDomain class is currently loaded early in the JDK boot cycle.
In fact the change suggested by @AlanBateman yesterday also has possibility to trigger unnecessary loading of the Security class. I might revert to the original design where we store the cached Properties in ProtectionDomain ?
Maybe I am missing something. If this JFR event is enabled, and the properties have not yet been accessed, then it seems ok for JFR to load the `Security` class when JFR is started since the user is interested in this event.
After further conversation with @seanjmullan , a sharedSecrets accessor for the Properties map should be ok. Edits pushed. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 3 Oct 2022 10:30:54 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Check for 0 security events
src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java line 33:
31: @Category({"Java Development Kit", "Security"}) 32: @Label("Initial Security Property") 33: @Name("jdk.InitialSecurityProperty")
Should we name this to `jdk.InitialSecurityProperties` and the label to `Initial Security Properties`, to be more accurate? src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java line 35:
33: @Name("jdk.InitialSecurityProperty") 34: @Description("Initial Security Properties") 35: public final class InitialSecurityPropertyEvent extends AbstractJDKEvent {
The event naming guidelines here https://docs.oracle.com/en/java/javase/17/jfapi/guidelines-naming-and-labeli... recommend leaving out `Event` from the class name. So, maybe we should call this `InitialSecurityProperties`? ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 10 Oct 2022 08:06:45 GMT, Jaikiran Pai <jpai@openjdk.org> wrote:
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Check for 0 security events
src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java line 35:
33: @Name("jdk.InitialSecurityProperty") 34: @Description("Initial Security Properties") 35: public final class InitialSecurityPropertyEvent extends AbstractJDKEvent {
The event naming guidelines here https://docs.oracle.com/en/java/javase/17/jfapi/guidelines-naming-and-labeli... recommend leaving out `Event` from the class name. So, maybe we should call this `InitialSecurityProperties`?
The documentation is somewhat misleading. If the event has a name annotation, I think it's fine to call the class Event, because name will override the class name. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 10 Oct 2022 08:04:28 GMT, Jaikiran Pai <jpai@openjdk.org> wrote:
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Check for 0 security events
src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java line 33:
31: @Category({"Java Development Kit", "Security"}) 32: @Label("Initial Security Property") 33: @Name("jdk.InitialSecurityProperty")
Should we name this to `jdk.InitialSecurityProperties` and the label to `Initial Security Properties`, to be more accurate?
There is one property per event, so it uses the same naming convention as jdk.InitialSystemProperty ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 10 Oct 2022 11:29:52 GMT, Erik Gahlin <egahlin@openjdk.org> wrote:
src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java line 33:
31: @Category({"Java Development Kit", "Security"}) 32: @Label("Initial Security Property") 33: @Name("jdk.InitialSecurityProperty")
Should we name this to `jdk.InitialSecurityProperties` and the label to `Initial Security Properties`, to be more accurate?
There is one property per event, so it uses the same naming convention as jdk.InitialSystemProperty
You are right indeed - I overlooked the part where this PR loops over these properties and creates one event per property.
src/jdk.jfr/share/classes/jdk/jfr/events/InitialSecurityPropertyEvent.java line 35:
33: @Name("jdk.InitialSecurityProperty") 34: @Description("Initial Security Properties") 35: public final class InitialSecurityPropertyEvent extends AbstractJDKEvent {
The event naming guidelines here https://docs.oracle.com/en/java/javase/17/jfapi/guidelines-naming-and-labeli... recommend leaving out `Event` from the class name. So, maybe we should call this `InitialSecurityProperties`?
The documentation is somewhat misleading. If the event has a name annotation, I think it's fine to call the class Event, because name will override the class name.
Thank you Erik for that detail. Looks fine to me then. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 3 Oct 2022 10:30:54 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Check for 0 security events
test/jdk/java/security/Security/ConfigFileTest.java line 132:
130: .shouldNotContain(EXPECTED_DEBUG_OUTPUT) 131: .shouldNotContain(UNEXPECTED_DEBUG_OUTPUT); 132: } else{
let's add one space after `else` ------------- PR: https://git.openjdk.org/jdk/pull/10394
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision: Address Oct 10 review comments ------------- Changes: - all: https://git.openjdk.org/jdk/pull/10394/files - new: https://git.openjdk.org/jdk/pull/10394/files/f8faecf4..323938d9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=10394&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10394&range=02-03 Stats: 22 lines in 6 files changed: 9 ins; 3 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/10394.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10394/head:pull/10394 PR: https://git.openjdk.org/jdk/pull/10394
On Mon, 10 Oct 2022 19:23:51 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with one additional commit since the last revision:
Address Oct 10 review comments
test/lib/jdk/test/lib/jfr/EventNames.java line 195:
193: public final static String X509Certificate = PREFIX + "X509Certificate"; 194: public final static String X509Validation = PREFIX + "X509Validation"; 195: public final static String InitialSecurityProperty = PREFIX + "InitialSecurityProperty";
Let's unify order of modifiers in this file. Some fields use `public final static` and some `public static final` ------------- PR: https://git.openjdk.org/jdk/pull/10394
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 17 commits: - test update - Merge branch 'master' into secEvent-8292177 - Use stringPropertyNames() - Refactor getter method name - Restore ProtectionDomain to original - Store properties cache in Security class - Remove unused import - Address Oct 10 review comments - Check for 0 security events - Null check on Properties - ... and 7 more: https://git.openjdk.org/jdk/compare/3644e26c...3a347dae ------------- Changes: https://git.openjdk.org/jdk/pull/10394/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=10394&range=04 Stats: 255 lines in 12 files changed: 252 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/10394.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10394/head:pull/10394 PR: https://git.openjdk.org/jdk/pull/10394
On Thu, 13 Oct 2022 16:06:13 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 17 commits:
- test update - Merge branch 'master' into secEvent-8292177 - Use stringPropertyNames() - Refactor getter method name - Restore ProtectionDomain to original - Store properties cache in Security class - Remove unused import - Address Oct 10 review comments - Check for 0 security events - Null check on Properties - ... and 7 more: https://git.openjdk.org/jdk/compare/3644e26c...3a347dae
Changes requested by mullan (Reviewer). src/java.base/share/classes/java/security/Security.java line 68:
66: 67: /* cache a copy for recording purposes */ 68: static Properties initialSecurityProperties;
This can be `private` now. src/java.base/share/classes/java/security/Security.java line 184:
182: } 183: 184: static Properties getInitialSecurityProperties() {
Don't think we need this method anymore. ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Thu, 13 Oct 2022 19:19:05 GMT, Sean Mullan <mullan@openjdk.org> wrote:
Sean Coffey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 17 commits:
- test update - Merge branch 'master' into secEvent-8292177 - Use stringPropertyNames() - Refactor getter method name - Restore ProtectionDomain to original - Store properties cache in Security class - Remove unused import - Address Oct 10 review comments - Check for 0 security events - Null check on Properties - ... and 7 more: https://git.openjdk.org/jdk/compare/3644e26c...3a347dae
src/java.base/share/classes/java/security/Security.java line 184:
182: } 183: 184: static Properties getInitialSecurityProperties() {
Don't think we need this method anymore.
Thanks - did another pass through the edits and caught these. Patch updated. [Use blessed modifier order in EventNames] [remove previous edit] ------------- PR: https://git.openjdk.org/jdk/pull/10394
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with two additional commits since the last revision: - remove previous edit - Use blessed modifier order in EventNames ------------- Changes: - all: https://git.openjdk.org/jdk/pull/10394/files - new: https://git.openjdk.org/jdk/pull/10394/files/3a347dae..138004ea Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=10394&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10394&range=04-05 Stats: 148 lines in 2 files changed: 0 ins; 4 del; 144 mod Patch: https://git.openjdk.org/jdk/pull/10394.diff Fetch: git fetch https://git.openjdk.org/jdk pull/10394/head:pull/10394 PR: https://git.openjdk.org/jdk/pull/10394
On Thu, 13 Oct 2022 19:53:27 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
Sean Coffey has updated the pull request incrementally with two additional commits since the last revision:
- remove previous edit - Use blessed modifier order in EventNames
Marked as reviewed by mullan (Reviewer). ------------- PR: https://git.openjdk.org/jdk/pull/10394
On Thu, 22 Sep 2022 15:57:56 GMT, Sean Coffey <coffeys@openjdk.org> wrote:
New JFR event to record state of initial security properties.
Debug output is also now added for these properties via -Djava.security.debug=properties
This pull request has now been integrated. Changeset: 8c40b7dc Author: Sean Coffey <coffeys@openjdk.org> URL: https://git.openjdk.org/jdk/commit/8c40b7dc8cd7b6a6d0c9349b991e0e01b69349c3 Stats: 393 lines in 12 files changed: 248 ins; 0 del; 145 mod 8292177: InitialSecurityProperty JFR event Reviewed-by: mullan ------------- PR: https://git.openjdk.org/jdk/pull/10394
participants (6)
-
Alan Bateman
-
Andrey Turbanov
-
Erik Gahlin
-
Jaikiran Pai
-
Sean Coffey
-
Sean Mullan