On 9/14/18 12:23 PM, Stuart Marks wrote:
On 9/14/18 4:52 AM, Daniel Fuchs wrote:
The name "jdk.allowSecurityManager" is actually fine.
I was also confused at first because I believed the
property, if set to false, would just prevent someone
to call System::setSecurityManager at runtime, whereas
it also prevents to set a security manager on the command
line.
Maybe emphasizing this would remove any confusion.
Yes, I was confused about this too -- I didn't realize that the
jdk.allowSecurityManager property also disallowed setting the security
manager via the java.security.manager property. I thought it just
applied to the System.setSecurityManager() method.
(Well, it turns out that setting the java.security.manager property on
the command line ends up calling System.setSecurityManager(), but this
is buried inside the implementation.)
So yes, the effect of setting jdk.allowSecurityManager needs to be
specified more explicitly.
Yes, agreed.
I wonder if the VM should fail to start if both
-Djdk.allowSecurityManager=false and -Djava.security.manager
are supplied?
Maybe, but I don't know that this is necessary. Again, if it's made
clear enough that the java.security.manager property is IGNORED if
jdk.allowSecurityManager=false, then that's OK with me.
The current implementation ignores the java.security.manager property if
jdk.allowSecurityManager=false if both are specified on the
command-line. However, that is mostly a side-effect of how the VM
currently handles exceptions in it's initialization phases.
System.setSecurityManager() does throw an UnsupportedOperationException,
but the VM ignores/swallows it.
From a usability standpoint, I think it makes more sense for the VM to
exit instead because it does not make sense to specify the properties as
above and this to me is clearly a configuration error. If the user
really wanted to enable a SecurityManager it may also lead to things
being deployed insecurely when they really should not be.
So, I am leaning towards catching the Exception and rethrowing it as an
Error.
--Sean
