Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled [v2]

2021-12-22 Thread Erik Joelsson
On Wed, 22 Dec 2021 01:18:58 GMT, Stuart Marks  wrote:

>> Enable the security manager in rmiregistry's launcher arguments.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Change java.security.manager to "allow"; filter warning lines from 
> VersionCheck

Build change looks good.

-

Marked as reviewed by erikj (Reviewer).

PR: https://git.openjdk.java.net/jdk18/pull/45


Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled [v2]

2021-12-22 Thread Alan Bateman
On Wed, 22 Dec 2021 01:18:58 GMT, Stuart Marks  wrote:

>> Enable the security manager in rmiregistry's launcher arguments.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Change java.security.manager to "allow"; filter warning lines from 
> VersionCheck

This version looks okay, avoids having an attempt to set the SM in 
createRegistry always be skipped.

-

Marked as reviewed by alanb (Reviewer).

PR: https://git.openjdk.java.net/jdk18/pull/45


Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled [v2]

2021-12-21 Thread Stuart Marks
On Wed, 22 Dec 2021 01:18:58 GMT, Stuart Marks  wrote:

>> Enable the security manager in rmiregistry's launcher arguments.
>
> Stuart Marks has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Change java.security.manager to "allow"; filter warning lines from 
> VersionCheck

Enabling the security manager using a property, versus setting the property to 
`allow` and enabling it in code, is mostly a distinction without a difference. 
I don't think there is really a need to have the different tools be consistent. 
Local tool considerations probably swamp the need for any cross-tool 
consistency.

In this case some of the RMI registry tests set the property to `allow` on the 
command line and then rely on the code to enable the security manager using the 
API, so it's much easier to switch the `rmiregistry` launcher to use the 
`allow` technique instead. This is sort-of the tail (the tests) wagging the doc 
(the tool) but there doesn't seem to any benefit to be gained from fiddling 
around with the tests and the RMI test library.

I've also updated VersionCheck.java to filter out the security manager warning 
messages.

-

PR: https://git.openjdk.java.net/jdk18/pull/45


Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled [v2]

2021-12-21 Thread Stuart Marks
> Enable the security manager in rmiregistry's launcher arguments.

Stuart Marks has updated the pull request incrementally with one additional 
commit since the last revision:

  Change java.security.manager to "allow"; filter warning lines from 
VersionCheck

-

Changes:
  - all: https://git.openjdk.java.net/jdk18/pull/45/files
  - new: https://git.openjdk.java.net/jdk18/pull/45/files/e899bd2d..f713e731

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk18=45=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk18=45=00-01

  Stats: 7 lines in 2 files changed: 3 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk18/pull/45.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk18 pull/45/head:pull/45

PR: https://git.openjdk.java.net/jdk18/pull/45


Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled

2021-12-21 Thread David Holmes
On Sun, 19 Dec 2021 07:37:19 GMT, Alan Bateman  wrote:

>> Enable the security manager in rmiregistry's launcher arguments.
>
> As things stand, `rmiregsitry -J-Djava.security.manager` and `rmiregistry 
> -J-Djava.security.manager=allow` are equivalent because rmiregistry sets the 
> default SM. Some difference may be observed if someone is running rmiregistry 
> with a custom system class loader set, or custom file system provider, or 
> running it with a JVM TI agent that is looking at events between VMStart and 
> VMInit but these seem somewhat obscure scenarios for a command line program 
> If rmiregstry worked with ToolProvider then I think there would be more to 
> discuss. If the final patch is to have the launcher set the default security 
> manager then we may want to change RegistryImpl.createRegistry to fail if not 
> set.
> 
> The warning that is emitted for both cases is expected. JEP 411 is very clear 
> that it there is no mechanism to suppress it. We may need to add a release 
> note to document that rmiregistry will emit a warning a startup, that will at 
> least be something to point at in the event that anyone asks or reports an 
> issue.

In the same kind of PR (https://github.com/openjdk/jdk18/pull/53) for `jstatd` 
@AlanBateman writes:
> This looks okay in that it is now worked exactly as it did in JDK 17.

And that PR uses `allow` as I have suggested here (to preserve existing 
behaviour). All the affected launchers should be fixed in the same consistent 
way.

-

PR: https://git.openjdk.java.net/jdk18/pull/45


Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled

2021-12-20 Thread Stuart Marks
On Fri, 17 Dec 2021 20:01:27 GMT, Stuart Marks  wrote:

> Enable the security manager in rmiregistry's launcher arguments.

I think there's little to worry about with custom configurations of the 
`rmiregistry` tool. What somebody might do is to enable a customized security 
manager using properties... of course they'd also have to ensure that the 
customized SM is somewhere in the classpath as well, by adding more command 
line options. That seems fairly unlikely to me. Anyone who wants to run an RMI 
registry service in some specialized configuration would probably just set 
things up themselves and then use the `LocateRegistry.createRegistry` API 
instead of trying to tinker with the `rmiregistry` command.

If `rmiregistry` enables the SM using properties, then yes we can probably 
change the code to assert that the SM is running instead of conditionally 
enabling it like it does now.

Next headache is that `tools/launcher/VersionCheck.jtr` fails because it sees 
the warning messages instead of the version string. (Argh!) Interestingly this 
test passes even though `rmiregistry` currently fails to operate normally, 
because it runs well enough to print its version string, but not well enough to 
start up a registry service. (Double argh!)

The warnings policy is a separate issue being discussed elsewhere.

-

PR: https://git.openjdk.java.net/jdk18/pull/45


Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled

2021-12-18 Thread Alan Bateman
On Fri, 17 Dec 2021 20:01:27 GMT, Stuart Marks  wrote:

> Enable the security manager in rmiregistry's launcher arguments.

As things stand, `rmiregsitry -J-Djava.security.manager` and `rmiregistry 
-J-Djava.security.manager=allow` are equivalent because rmiregistry sets the 
default SM. Some difference may be observed if someone is running rmiregistry 
with a custom system class loader set, or custom file system provider, or 
running it with a JVM TI agent that is looking at events between VMStart and 
VMInit but these seem somewhat obscure scenarios for a command line program If 
rmiregstry worked with ToolProvider then I think there would be more to 
discuss. If the final patch is to have the launcher set the default security 
manager then we may want to change RegistryImpl.createRegistry to fail if not 
set.

The warning that is emitted for both cases is expected. JEP 411 is very clear 
that it there is no mechanism to suppress it. We may need to add a release note 
to document that rmiregistry will emit a warning a startup, that will at least 
be something to point at in the event that anyone asks or reports an issue.

-

PR: https://git.openjdk.java.net/jdk18/pull/45