RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

classic Classic list List threaded Threaded
55 messages Options
123
Reply | Threaded
Open this post in threaded view
|

RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Sean Mullan
This is a SecurityManager related change which warrants some additional
details for its motivation.

The current System.setSecurityManager() API allows a SecurityManager to
be set at run-time. However, because of this mutability, it incurs a
performance overhead even for applications that never call it and do not
enable a SecurityManager dynamically, which is probably the majority of
applications.

For example, there are lots of "SecurityManager sm =
System.getSecurityManager(); if (sm != null) ..." checks in the JDK. If
it was known that a SecurityManager could never be set at run-time,
these checks could be optimized using constant-folding.

There are essentially two main parts to this change:

1. Deprecation of System.securityManager()

Going forward, we want to discourage applications from calling
System.setSecurityManager(). Instead they should enable a
SecurityManager using the java.security.manager system property on the
command-line.

2. A new JDK-specific system property to disallow the setting of the
security manager at run-time: jdk.allowSecurityManager

If set to false, it allows the run-time to optimize the code and improve
performance when it is known that an application will never run with a
SecurityManager. To support this behavior, the
System.setSecurityManager() API has been updated such that it can throw
an UnsupportedOperationException if it does not allow a security manager
to be set dynamically.

webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8203316
JBS: https://bugs.openjdk.java.net/browse/JDK-8191053

(I will likely also send this to core-libs for additional review later)

--Sean
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Bernd Eckenfels-4
Hallo Sean,

The change looks fine to me, but if you have to roll another version maybe you could add a comment on this line to explain its purpose. Since this line is changed in the patch it would be a good time:

System.java:350
   sm.checkPackageAccess("java.lang");

Is that some kind of warm-up? (It cant be a sanity or security check as its result is ignored.

I am curious, did you verify that the compiler will actually optimize the getSecurityManager null check in case allow=never? Is that happening in C1?

Gruss
Bernd

Gruss
Bernd
--
http://bernd.eckenfels.net
 

Von: -995614032m Auftrag von
Gesendet: Donnerstag, September 13, 2018 10:14 PM
An: security Dev OpenJDK
Betreff: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable
 
This is a SecurityManager related change which warrants some additional
details for its motivation.

The current System.setSecurityManager() API allows a SecurityManager to
be set at run-time. However, because of this mutability, it incurs a
performance overhead even for applications that never call it and do not
enable a SecurityManager dynamically, which is probably the majority of
applications.

For example, there are lots of "SecurityManager sm =
System.getSecurityManager(); if (sm != null) ..." checks in the JDK. If
it was known that a SecurityManager could never be set at run-time,
these checks could be optimized using constant-folding.

There are essentially two main parts to this change:

1. Deprecation of System.securityManager()

Going forward, we want to discourage applications from calling
System.setSecurityManager(). Instead they should enable a
SecurityManager using the java.security.manager system property on the
command-line.

2. A new JDK-specific system property to disallow the setting of the
security manager at run-time: jdk.allowSecurityManager

If set to false, it allows the run-time to optimize the code and improve
performance when it is known that an application will never run with a
SecurityManager. To support this behavior, the
System.setSecurityManager() API has been updated such that it can throw
an UnsupportedOperationException if it does not allow a security manager
to be set dynamically.

webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8203316
JBS: https://bugs.openjdk.java.net/browse/JDK-8191053

(I will likely also send this to core-libs for additional review later)

--Sean
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Stuart Marks
In reply to this post by Sean Mullan
Hi Sean,

Looks sensible to me.

On 9/13/18 1:02 PM, Sean Mullan wrote:
> 2. A new JDK-specific system property to disallow the setting of the security
> manager at run-time: jdk.allowSecurityManager
>
> If set to false, it allows the run-time to optimize the code and improve
> performance when it is known that an application will never run with a
> SecurityManager. To support this behavior, the System.setSecurityManager() API
> has been updated such that it can throw an UnsupportedOperationException if it
> does not allow a security manager to be set dynamically.

I guess the default value is true?

The behavior makes sense, though the name I think is misleading. It seems not to
disallow a security manager, but to disallow the capability to *set* the
security manager. Maybe "jdk.allowSetSecurityManager" ?

s'marks
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Mandy Chung
In reply to this post by Sean Mullan


On 9/13/18 1:02 PM, Sean Mullan wrote:
This is a SecurityManager related change which warrants some additional details for its motivation.

The current System.setSecurityManager() API allows a SecurityManager to be set at run-time. However, because of this mutability, it incurs a performance overhead even for applications that never call it and do not enable a SecurityManager dynamically, which is probably the majority of applications.

:
webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8203316
JBS: https://bugs.openjdk.java.net/browse/JDK-8191053

This is a welcoming change as many applications run without security manager and they will benefit the performance improvement.   This patch makes the private System::security field @Stable hich is the first installment of perf optimization.   I hope to see further optimization can be done for example speed up of doPrivileged block in the no-security manager case.

This patch looks good.  It may be good to add one test case to launch with -Djava.security.manager and -Djdk.allowSecurityManager=false to show that no security manager is installed; essential -Djava.security.manager is ignored.  Maybe @implNote should mention such case where -Djava.security.manager is ignored if -Djdk.allowSecurityManager=false.

Mandy
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Mandy Chung
In reply to this post by Stuart Marks


On 9/13/18 4:50 PM, Stuart Marks wrote:
Hi Sean,

Looks sensible to me.

On 9/13/18 1:02 PM, Sean Mullan wrote:
2. A new JDK-specific system property to disallow the setting of the security manager at run-time: jdk.allowSecurityManager

If set to false, it allows the run-time to optimize the code and improve performance when it is known that an application will never run with a SecurityManager. To support this behavior, the System.setSecurityManager() API has been updated such that it can throw an UnsupportedOperationException if it does not allow a security manager to be set dynamically.

I guess the default value is true?

The behavior makes sense, though the name I think is misleading. It seems not to disallow a security manager, but to disallow the capability to *set* the security manager. Maybe "jdk.allowSetSecurityManager" ?


When -Djdk.allowSecurityManager is set at startup, no security manager is allowed.  Most cases a security manager is started via -Djava.security.manager on the command-line. 

This name also prepares for the future to potentially flip the default (no security manager by default) and allow a security manager at runtime.  

Mandy

Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Weijun Wang
In reply to this post by Sean Mullan


> On Sep 14, 2018, at 4:02 AM, Sean Mullan <[hidden email]> wrote:
>
> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.00/

     public static SecurityManager getSecurityManager() {
+        if (allowSecurityManager()) {
         return security;
+        } else {
+            return null;
+        }
     }

Is this change really necessary? You have to call a method here.

> CSR: https://bugs.openjdk.java.net/browse/JDK-8203316

Still having @Deprecated(since="11").

Thanks
Max


Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Alan Bateman
In reply to this post by Bernd Eckenfels-4


On 14/09/2018 00:19, Bernd Eckenfels wrote:
:

I am curious, did you verify that the compiler will actually optimize the getSecurityManager null check in case allow=never?
Yes, and the reason the allowSecurityManager field is an int rather than a boolean is so that it get sets to a value other than its default.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Alan Bateman
In reply to this post by Sean Mullan
On 13/09/2018 21:02, Sean Mullan wrote:
> :
>
> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.00/
> CSR: https://bugs.openjdk.java.net/browse/JDK-8203316
> JBS: https://bugs.openjdk.java.net/browse/JDK-8191053
>
This is inline with the discussion we had on this mailing list last year
about deprecating the ability to enable a SM in a running VM and then
profiting in several areas of the JDK. The changes look good and I'm
really happy to see this moving.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Daniel Fuchs
In reply to this post by Mandy Chung
On 14/09/2018 03:45, mandy chung wrote:
>> The behavior makes sense, though the name I think is misleading. It
>> seems not to disallow a security manager, but to disallow the
>> capability to *set* the security manager. Maybe
>> "jdk.allowSetSecurityManager" ?
>>
>
> When -Djdk.allowSecurityManager is set at startup, no security manager
> is allowed.  Most cases a security manager is started via
> -Djava.security.manager on the command-line.

Maybe it would be less confusing if the property was named
-Djdk.disableSecurityManager, because AFAICT, it's what it does?

best regards,

-- daniel
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Daniel Fuchs
On 14/09/2018 12:31, Daniel Fuchs wrote:
> Maybe it would be less confusing if the property was named
> -Djdk.disableSecurityManager, because AFAICT, it's what it does?

Forget I said that ;-)

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.

I wonder if the VM should fail to start if both
-Djdk.allowSecurityManager=false and -Djava.security.manager
are supplied?

best regards and apologies for the noise of my previous mail...


-- daniel
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

David Lloyd-2
In reply to this post by Sean Mullan
On Thu, Sep 13, 2018 at 3:03 PM Sean Mullan <[hidden email]> wrote:
>
> This is a SecurityManager related change which warrants some additional
> details for its motivation.
>
> The current System.setSecurityManager() API allows a SecurityManager to
> be set at run-time. However, because of this mutability, it incurs a
> performance overhead even for applications that never call it and do not
> enable a SecurityManager dynamically, which is probably the majority of
> applications.

What kind of performance overhead?

> For example, there are lots of "SecurityManager sm =
> System.getSecurityManager(); if (sm != null) ..." checks in the JDK. If
> it was known that a SecurityManager could never be set at run-time,
> these checks could be optimized using constant-folding.

I think they could be optimized using constant-folding either way, if
something like SwitchPoint were used instead of a plain field; am I
incorrect in my understanding?  The reason I ask is... (see below)

> There are essentially two main parts to this change:
>
> 1. Deprecation of System.s[etS]ecurityManager()

We (JBoss) use this method to configure some things at run time (like
customizing the Policy) before installing our security manager, so
this would be not so great for us.

--
- DML
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Alan Bateman
On 14/09/2018 13:46, David Lloyd wrote:
> :
>> There are essentially two main parts to this change:
>>
>> 1. Deprecation of System.s[etS]ecurityManager()
> We (JBoss) use this method to configure some things at run time (like
> customizing the Policy) before installing our security manager, so
> this would be not so great for us.
The security manager is legacy these days and I think we need to figure
out a plan how to deprecate and eventually bury it. I have no idea how
many releases or years that might take but the proposal here is a
profitable first step. It's easy to imagine follow on steps where the
default changes to not support the security manager without some opt-in.
Yes, this will be disruptive for a number of usages but there is lots of
time to look for solutions to the issues that people are using the
security manager for today - for example we've seen several cases where
people set a security manager because they lack hooks to prevent plugins
from doing things (plugins or tests calling System.exit comes up
periodically for example).

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

David Lloyd-2
On Fri, Sep 14, 2018 at 8:22 AM Alan Bateman <[hidden email]> wrote:

>
> On 14/09/2018 13:46, David Lloyd wrote:
> > :
> >> There are essentially two main parts to this change:
> >>
> >> 1. Deprecation of System.s[etS]ecurityManager()
> > We (JBoss) use this method to configure some things at run time (like
> > customizing the Policy) before installing our security manager, so
> > this would be not so great for us.
> The security manager is legacy these days and I think we need to figure
> out a plan how to deprecate and eventually bury it. I have no idea how
> many releases or years that might take but the proposal here is a
> profitable first step. It's easy to imagine follow on steps where the
> default changes to not support the security manager without some opt-in.
> Yes, this will be disruptive for a number of usages but there is lots of
> time to look for solutions to the issues that people are using the
> security manager for today - for example we've seen several cases where
> people set a security manager because they lack hooks to prevent plugins
> from doing things (plugins or tests calling System.exit comes up
> periodically for example).

I can say that this explanation would be more palatable by far if the
security manager as a whole could be deprecated all at once.  I for
one would certainly be happy to remove support for it in the software
that I maintain (it's a considerable amount of code and other
gymnastics to be sure).  However, I'm not sure that our users and
customers will be so easily convinced.  My understanding is that there
are plenty of corporate and perhaps government security standards that
dictate security manager usage.

If the security manager itself is not yet to be deprecated, then I
have a much harder time accepting this argument.  It's not really a
stepping stone in any practical sense, at least not from our
perspective, unless that step is "break JBoss first, and then break
everyone else later"; to be fair though I'm approximately 200% more
cynical in the morning. :)

--
- DML
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Alan Bateman
On 14/09/2018 14:28, David Lloyd wrote:

> :
> I can say that this explanation would be more palatable by far if the
> security manager as a whole could be deprecated all at once.  I for
> one would certainly be happy to remove support for it in the software
> that I maintain (it's a considerable amount of code and other
> gymnastics to be sure).  However, I'm not sure that our users and
> customers will be so easily convinced.  My understanding is that there
> are plenty of corporate and perhaps government security standards that
> dictate security manager usage.
>
> If the security manager itself is not yet to be deprecated, then I
> have a much harder time accepting this argument.  It's not really a
> stepping stone in any practical sense, at least not from our
> perspective, unless that step is "break JBoss first, and then break
> everyone else later"; to be fair though I'm approximately 200% more
> cynical in the morning. :)
If the proposed patch goes into jdk/jdk then it just means that code
using System.setSecurityManager gets a deprecation warning at
compile-time, nothing more. I agree it would be nice to able to go
further and deprecate SecurityManager, also emit a warning if
-Djava.security.manager is set on the command line. These seem like
possible future disruptive steps in a multi-release plan.

-Alan


Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Stuart Marks
In reply to this post by Daniel Fuchs


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.

> 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.

s'marks
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Sean Mullan
In reply to this post by Weijun Wang
On 9/13/18 11:01 PM, Weijun Wang wrote:

>
>
>> On Sep 14, 2018, at 4:02 AM, Sean Mullan <[hidden email]> wrote:
>>
>> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.00/
>
>       public static SecurityManager getSecurityManager() {
> +        if (allowSecurityManager()) {
>           return security;
> +        } else {
> +            return null;
> +        }
>       }
>
> Is this change really necessary? You have to call a method here.

I'll defer to a compiler expert, but I believe this is necessary to
cause the constant-folding of this method since the allowSecurityManager
field has an @Stable annotation.

>> CSR: https://bugs.openjdk.java.net/browse/JDK-8203316
>
> Still having @Deprecated(since="11").

Fixed. Good catch.

--Sean
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Sean Mullan
In reply to this post by Bernd Eckenfels-4
On 9/13/18 7:19 PM, Bernd Eckenfels wrote:

> Hallo Sean,
>
> The change looks fine to me, but if you have to roll another version
> maybe you could add a comment on this line to explain its purpose. Since
> this line is changed in the patch it would be a good time:
>
> System.java:350
>     sm.checkPackageAccess("java.lang");
>
> Is that some kind of warm-up? (It cant be a sanity or security check as
> its result is ignored.

Yes. If I recall, that forces the Policy implementation to be loaded
early and avoids potential class loading and/or stack recursion issues
if done later on. I played around with removing this a while back and
all sorts of things broke, so I'll do that again but this time add some
meaningful comments as to why it is needed.

> I am curious, did you verify that the compiler will actually optimize
> the getSecurityManager null check in case allow=never? Is that happening
> in C1?

Yes, Claes did some initial testing and confirmed that the
constant-folded is occurring. See the comment in JDK-8191053 for more
info and some results:

https://bugs.openjdk.java.net/browse/JDK-8191053?focusedCommentId=14186619&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14186619

--Sean

>
> Gruss
> Bernd
>
> Gruss
> Bernd
> --
> http://bernd.eckenfels.net
> ------------------------------------------------------------------------
> *Von:* -995614032m Auftrag von
> *Gesendet:* Donnerstag, September 13, 2018 10:14 PM
> *An:* security Dev OpenJDK
> *Betreff:* RFR (12): 8191053: Provide a mechanism to make system's
> security manager immutable
> This is a SecurityManager related change which warrants some additional
> details for its motivation.
>
> The current System.setSecurityManager() API allows a SecurityManager to
> be set at run-time. However, because of this mutability, it incurs a
> performance overhead even for applications that never call it and do not
> enable a SecurityManager dynamically, which is probably the majority of
> applications.
>
> For example, there are lots of "SecurityManager sm =
> System.getSecurityManager(); if (sm != null) ..." checks in the JDK. If
> it was known that a SecurityManager could never be set at run-time,
> these checks could be optimized using constant-folding.
>
> There are essentially two main parts to this change:
>
> 1. Deprecation of System.securityManager()
>
> Going forward, we want to discourage applications from calling
> System.setSecurityManager(). Instead they should enable a
> SecurityManager using the java.security.manager system property on the
> command-line.
>
> 2. A new JDK-specific system property to disallow the setting of the
> security manager at run-time: jdk.allowSecurityManager
>
> If set to false, it allows the run-time to optimize the code and improve
> performance when it is known that an application will never run with a
> SecurityManager. To support this behavior, the
> System.setSecurityManager() API has been updated such that it can throw
> an UnsupportedOperationException if it does not allow a security manager
> to be set dynamically.
>
> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.00/
> CSR: https://bugs.openjdk.java.net/browse/JDK-8203316
> JBS: https://bugs.openjdk.java.net/browse/JDK-8191053
>
> (I will likely also send this to core-libs for additional review later)
>
> --Sean
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

roger riggs
In reply to this post by Sean Mullan
Hi Sean,

Looks good.

I would quibble with System.java:335-6
"Java virtual machine does not allow a security manager"

I would say only "a security manager is not allowed to be set dynamically."

There is no need to identify the Java virtual machine.  The VM itself has nothing to do with it.
Also remove the same phrase in SecurityManager class javadoc.

Regards, Roger


On 9/13/18 4:02 PM, Sean Mullan wrote:
This is a SecurityManager related change which warrants some additional details for its motivation.

The current System.setSecurityManager() API allows a SecurityManager to be set at run-time. However, because of this mutability, it incurs a performance overhead even for applications that never call it and do not enable a SecurityManager dynamically, which is probably the majority of applications.

For example, there are lots of "SecurityManager sm = System.getSecurityManager(); if (sm != null) ..." checks in the JDK. If it was known that a SecurityManager could never be set at run-time, these checks could be optimized using constant-folding.

There are essentially two main parts to this change:

1. Deprecation of System.securityManager()

Going forward, we want to discourage applications from calling System.setSecurityManager(). Instead they should enable a SecurityManager using the java.security.manager system property on the command-line.

2. A new JDK-specific system property to disallow the setting of the security manager at run-time: jdk.allowSecurityManager

If set to false, it allows the run-time to optimize the code and improve performance when it is known that an application will never run with a SecurityManager. To support this behavior, the System.setSecurityManager() API has been updated such that it can throw an UnsupportedOperationException if it does not allow a security manager to be set dynamically.

webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8203316
JBS: https://bugs.openjdk.java.net/browse/JDK-8191053

(I will likely also send this to core-libs for additional review later)

--Sean

Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Bernd Eckenfels-4
In reply to this post by Stuart Marks
There is another way, by reusing the existing security manager property with a new keyword („default“ is already a well known value) one could implement the stable suppression of the SM without actually needing a new property. It also avoids unclear meaning of denied but specified SM:

java.security.manager=disabled // or „denied“ or „forbidden“ or simply „false“

This would also allow an „ignored“ (setSM does nothing).

Gruss
Bernd
--
http://bernd.eckenfels.net
 

Von: -1056361920m Auftrag von
Gesendet: Freitag, September 14, 2018 8:23 PM
An: Daniel Fuchs
Cc: security Dev OpenJDK
Betreff: Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable
 


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.

> 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.

s'marks
Reply | Threaded
Open this post in threaded view
|

Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

Sean Mullan
In reply to this post by Stuart Marks
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
123