JEP411: Missing use-case: Monitoring / restricting libraries
One useful thing that you can do with SecurityManager, which would be
impossible if it is removed, and which isn't described in the 'specific
narrow use cases' section:
Monitoring and restriction of, specifically, third party libraries.
99 out of a 100 modern java projects have a rather long list of
dependencies, and most of those dependencies have a limited and specific
intent. "This library reads EXIM data from a JPG". "This library marshals
JSON into java POJOs". "This library makes QR code PNGs".
As an app programmer I want to monitor, and optionally restrict what these
libraries can do. I can have an app that does (and is intended to) make
network connections all the time, but as part of doing the job I wrote it
for, it may be generating some QR PNGs. If the _QR generator library_ is
making network calls? I want to know, and I probably want to stop it from
happening.
SecurityManager can do that. I don't know of a good way to take care of
this without it, and it is not (currently) described in JEP411. I can't use
OS-level monitoring, because the OS has no awareness of modules / packages
/ classnames, so I can't tell it to accept without log or warning any
network access done by the parts of my application that are supposed to do
this, but that I _do_ want it to log, warn, or halt any attempt by that QR
generator library to hit the network.
The original intent of SecurityManager was clearly to allow you to run
untrusted code on a VM (the 'applet' use case), but this is somewhat
different: It's not so much about attempting to secure presumably malicious
code in a library or applet, but instead about attempting to secure against
operations that ordinary java code may do, but which you simply aren't
expecting from some specific library.
Some real-world and/or highly plausible examples:
* An XML parser library may make network calls or open files on disk due to
e.g. XXE shenanigans: See
https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing
– this isn't just plausible, we have plenty of proof that this has caused
significant security breaches multiple times in XML's history. A
SecurityManager that monitors (or outright denies) specifically the network
and disk access from an XML parser library would have meant XXE attacks
could never have happened.
* Some twitter library may be invoking a relative-pathed `cmd.exe` in order
to retrieve some system info from windows that cannot be obtained with any
of the core java libraries. Perhaps to check if the twitter desktop client
is installed (the authors of the library may well be unaware of the new
ProcessInfo API). No doubt a scan of all java-tagged projects on github
finds rather a lot of libraries that Runtime.exec("cmd.exe") for some
unexpected, non-malicious purpose. Nevertheless, ProcessBuilder does apply
$PATH processing and a system operator may not be willing to accept invokes
to a relative path that can be trivially hijacked if some directory in the
PATH is compromised, especially if the programming team that uses the
library wasn't expecting it to do so. A SecurityManager can monitor this
and even stop it from happening.
* Any library could have the bright idea to 'phone home' and make a network
call simply to give the library author some idea of how widespread their
library is used. This could have an entirely innocuous purpose: The library
author thought it'd be a cool idea to have a live map of the planet on
their website, with a little animated blip every time their library is used
to, say, parse some JSON. SecurityManager is the simplest way to spot this
and stop it.
I don't think SecurityManager is necessarily fantastic at stopping
_intentionally malicious behaviour_ by a library written by untrustworthy
charlatans (even though that was its original intent). But, it does a great
job at stopping a misunderstanding between a library author and the user of
said library, such as the rather plausible scenarios I just described.
Modern security practices put a lot of focus on monitoring; SecurityManager
can do that too: A SecurityManager is not obligated to deal with e.g. a
notification that some code is attempting to open a file by throwing
SecurityException - they can also simply log or notify somebody that it is
happening and allow it. They could check if the caller is in a subset of
'blessed' code that has been checked by the dev team and has sign-off that
it is allowed to do it. They could simply do a quick echo in dev-mode only,
just so developers are aware whilst running tests that some library is
doing things that have potential security implications and open potential
surface area for a breach.
I'm not sure if the file-based configuration of the security manager
(policy files) needs to be kept around to enable this use case, but the
basic infrastructure, and almost all of the various `check` methods in
java.lang.SecurityManager have plausible scenarios where an application may
want to monitor or deny wha
Re: RFR: 8265237: String.join and StringJoiner can be improved further
On Wed, 14 Apr 2021 20:03:27 GMT, Claes Redestad wrote: > There's a StringJoinerBenchmark micro added by JDK-8148937 which could > perhaps be expanded with the scenarios you've experimented with here? I modified that micro benchmark and added a method to also measure String.join static method along with StringJoiner for same parameters and extended the range of parameters to cover more diversity. The results are here: https://jmh.morethan.io/?gist=c38cc13d63774ec505cc8d394c00d502 It is apparent that there is a huge speedup when strings are bigger. But even smaller strings get a substantial speedup. There's also substantial reduction of garbage per operation. Previously the garbage amounted to the internal array of String elements and the StringBuffer with its internal byte[] array of characters. Now only the array of elements is the garbage. - PR: https://git.openjdk.java.net/jdk/pull/3501
Re: RFR: 8265237: String.join and StringJoiner can be improved further [v2]
> While JDK-8148937 improved StringJoiner class by replacing internal use of > getChars that copies out characters from String elements into a char[] array > with StringBuilder which is somehow more optimal, the improvement was > marginal in speed (0% ... 10%) and mainly for smaller strings, while GC was > reduced by about 50% in average per operation. > Initial attempt to tackle that issue was more involved, but was later > discarded because it was apparently using too much internal String details in > code that lives outside String and outside java.lang package. > But there is another way to package such "intimate" code - we can put it into > String itself and just call it from StringJoiner. > This PR is an attempt at doing just that. It introduces new package-private > method in `java.lang.String` which is then used from both pubic static > `String.join` methods as well as from `java.util.StringJoiner` (via > SharedSecrets). The improvements can be seen by running the following JMH > benchmark: > > https://gist.github.com/plevart/86ac7fc6d4541dbc08256cde544019ce > > The comparative results are here: > > https://jmh.morethan.io/?gist=7eb421cf7982456a2962269137f71c15 > > The jmh-result.json files are here: > > https://gist.github.com/plevart/7eb421cf7982456a2962269137f71c15 > > Improvement in speed ranges from 8% (for small strings) to 200% (for long > strings), while creation of garbage has been further reduced to an almost > garbage-free operation. > > So WDYT? Peter Levart has updated the pull request incrementally with one additional commit since the last revision: Add String.join benchmark method to StringJoinerBenchmark and adjust some parameters to cover bigger range - Changes: - all: https://git.openjdk.java.net/jdk/pull/3501/files - new: https://git.openjdk.java.net/jdk/pull/3501/files/62b577fd..6160e5aa Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3501&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3501&range=00-01 Stats: 11 lines in 1 file changed: 8 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3501.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3501/head:pull/3501 PR: https://git.openjdk.java.net/jdk/pull/3501
Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v2]
On Wed, 14 Apr 2021 17:37:00 GMT, Sibabrata Sahoo wrote:
>> This is a simple Test to add few additional API coverage for all java
>> supported key types. The objective of this Test is to cover equals() and
>> hashcode() methods for each key types.
>
> Sibabrata Sahoo has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Comment changed.
>
> Comment changed.
test/jdk/javax/crypto/KeyGenerator/CompareKeys.java line 124:
> 122: public SecretKey genSecretKey() throws Exception {
> 123: KeyGenerator kg = KeyGenerator.getInstance(this.algoName);
> 124: return kg.generateKey();
Would be informative to print out which provider is tested, i.e. where this kg
is from. Same goes for KeyPairGenerator.
-
PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8264208: Console charset API [v8]
On Thu, 15 Apr 2021 14:17:11 GMT, Alan Bateman wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Added @see links.
>
> src/java.base/share/classes/java/io/Console.java line 397:
>
>> 395: /**
>> 396: * Returns the {@link java.nio.charset.Charset Charset} object used
>> in
>> 397: * the {@code Console}.
>
> What would you think about re-phrasing the first sentence to use "for the
> Console" rather than "in the Console".
Changed to "for the Console", as well as `@return`.
> src/java.base/share/classes/java/lang/System.java line 123:
>
>> 121: *
>> 122: * @see Console#charset()
>> 123: * @see Console#reader()
>
> What would you think about changing the example in InputStreamReader class
> description as part of this?
Replaced `System.in` with generic `anInputStream`, as changing `new
InputStreamReader` with `Console.reader()` would defy the purpose of the
example.
-
PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v2]
On Thu, 15 Apr 2021 18:18:07 GMT, Valerie Peng wrote:
>> Sibabrata Sahoo has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Comment changed.
>>
>> Comment changed.
>
> test/jdk/javax/crypto/KeyGenerator/CompareKeys.java line 73:
>
>> 71: && !Arrays.equals(origKey.getEncoded(),
>> copyKey.getEncoded())
>> 72: && origKey.hashCode() != copyKey.hashCode()) {
>> 73: throw new RuntimeException("Key inequality found");
>
> Check the format equality as well?
Should
be || instead of &&?
-
PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8264208: Console charset API [v9]
> Please review the changes for the subject issue. This has been suggested in > a recent discussion thread for the JEP 400 > [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)]. > A CSR has also been drafted, and comments are welcome > [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)]. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Modified javadocs per suggestions. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3419/files - new: https://git.openjdk.java.net/jdk/pull/3419/files/5988f600..083f6180 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3419&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3419&range=07-08 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3419.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3419/head:pull/3419 PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v2]
On Wed, 14 Apr 2021 17:37:00 GMT, Sibabrata Sahoo wrote:
>> This is a simple Test to add few additional API coverage for all java
>> supported key types. The objective of this Test is to cover equals() and
>> hashcode() methods for each key types.
>
> Sibabrata Sahoo has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Comment changed.
>
> Comment changed.
test/jdk/javax/crypto/KeyGenerator/CompareKeys.java line 114:
> 112: HmacSHA384("HmacSHA384"),
> 113: HmacSHA512("HmacSHA512"),
> 114: RC2("RC2");
Just curious, how are these decided? Should this be an exhaustive list or just
enough sampling for code coverage? If this is meant to be a general test, have
you tried to not hardcoding the algorithm names to be enum? Otherwise, new
algorithms will not be tested if not updating this test.
-
PR: https://git.openjdk.java.net/jdk/pull/3490
Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v2]
On Wed, 14 Apr 2021 17:37:00 GMT, Sibabrata Sahoo wrote:
>> This is a simple Test to add few additional API coverage for all java
>> supported key types. The objective of this Test is to cover equals() and
>> hashcode() methods for each key types.
>
> Sibabrata Sahoo has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Comment changed.
>
> Comment changed.
test/jdk/javax/crypto/KeyGenerator/CompareKeys.java line 73:
> 71: && !Arrays.equals(origKey.getEncoded(),
> copyKey.getEncoded())
> 72: && origKey.hashCode() != copyKey.hashCode()) {
> 73: throw new RuntimeException("Key inequality found");
Check the format equality as well?
-
PR: https://git.openjdk.java.net/jdk/pull/3490
New candidate JEP: 411: Deprecate the Security Manager for Removal
https://openjdk.java.net/jeps/411 Summary: Deprecate the Security Manager for removal in a future release. The Security Manager dates from Java 1.0. It has not been the primary means of securing client-side Java code for many years, and it has rarely been used to secure server-side code. To move Java forward, we intend to deprecate the Security Manager for removal in concert with the legacy Applet API (JEP 398). - Mark
Integrated: 8265227: Move Proc.java from security/testlibrary to test/lib
On Wed, 14 Apr 2021 18:12:57 GMT, Weijun Wang wrote: > I'd like to move this tool to test/lib inside a proper named package. This pull request has now been integrated. Changeset: c70589c6 Author:Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/c70589c6 Stats: 119 lines in 9 files changed: 96 ins; 9 del; 14 mod 8265227: Move Proc.java from security/testlibrary to test/lib Reviewed-by: rriggs, xuelei, rhalade, ssahoo - PR: https://git.openjdk.java.net/jdk/pull/3496
Re: RFR: 8255410: Add ChaCha20 and Poly1305 support to SunPKCS11 provider [v3]
On Wed, 14 Apr 2021 03:58:33 GMT, Valerie Peng wrote: >> Could someone (perhaps Jamil?) please help review this change? This enhances >> SunPKCS11 provider with ChaCha20-Poly1305 cipher and ChaCha20 key generation >> support. Majority of the regression tests are adapted from the existing ones >> for SunJCE provider's ChaCha20-Poly1305 cipher impl. When testing against >> NSS v3.57, it does not have support for ChaCha20 cipher, thus I did not add >> support for ChaCha20 cipher and the corresponding parameter. >> >> Thanks! >> Valerie > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed an tagLen issue, no key+iv reuse check for decryption, and add > regression test for ChaCha20 SKF. Marked as reviewed by jnimeh (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3420
Re: RFR: 8264208: Console charset API [v8]
On Wed, 14 Apr 2021 17:17:03 GMT, Naoto Sato wrote:
>> Please review the changes for the subject issue. This has been suggested in
>> a recent discussion thread for the JEP 400
>> [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)].
>> A CSR has also been drafted, and comments are welcome
>> [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)].
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Added @see links.
src/java.base/share/classes/java/io/Console.java line 397:
> 395: /**
> 396: * Returns the {@link java.nio.charset.Charset Charset} object used
> in
> 397: * the {@code Console}.
What would you think about re-phrasing the first sentence to use "for the
Console" rather than "in the Console".
src/java.base/share/classes/java/lang/System.java line 123:
> 121: *
> 122: * @see Console#charset()
> 123: * @see Console#reader()
What would you think about changing the example in InputStreamReader class
description as part of this?
-
PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8255410: Add ChaCha20 and Poly1305 support to SunPKCS11 provider [v3]
On Wed, 14 Apr 2021 03:58:33 GMT, Valerie Peng wrote: >> Could someone (perhaps Jamil?) please help review this change? This enhances >> SunPKCS11 provider with ChaCha20-Poly1305 cipher and ChaCha20 key generation >> support. Majority of the regression tests are adapted from the existing ones >> for SunJCE provider's ChaCha20-Poly1305 cipher impl. When testing against >> NSS v3.57, it does not have support for ChaCha20 cipher, thus I did not add >> support for ChaCha20 cipher and the corresponding parameter. >> >> Thanks! >> Valerie > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed an tagLen issue, no key+iv reuse check for decryption, and add > regression test for ChaCha20 SKF. src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11AEADCipher.java line 225: > 223: } > 224: apAlgo = "ChaCha20-Poly1305"; > 225: spec = new IvParameterSpec(iv); Are there protections further up the call stack that guarantee that iv will be non-null when encrypt == false? I assume there are but I figured I'd ask since a null iv could cause NPE. - PR: https://git.openjdk.java.net/jdk/pull/3420
Re: RFR: 8265227: Move Proc.java from security/testlibrary to test/lib [v3]
On Wed, 14 Apr 2021 22:57:55 GMT, Weijun Wang wrote: >> I'd like to move this tool to test/lib inside a proper named package. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > do not call internal method Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3496
Integrated: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI
On Fri, 9 Apr 2021 13:15:16 GMT, Conor Cleary wrote: > ### Description > This fix is part of a previous effort to both cleanup/modernise JNDI code, > the details of which can be seen in > [JDK-8048091](https://bugs.openjdk.java.net/browse/JDK-8048091). A number > JNDI methods under `java.naming` use Anonymous Inner Classes in cases where > only a single object unique to the requirements of the method is used. The > issues these occurrences of AICs cause are highlighted below. > > - AICs, in the cases concerned with this fix, are used where only one > operation is required. While AICs can be useful for more complex > implementations (using interfaces, multiple methods needed, local fields > etc.), Lambdas are better suited here as they result in a more readable and > concise solution. > > ### Fixes > - Where applicable, occurrences of AICs were replaced with lambdas to address > the issues above resulting primarily in more readable/concise code. This pull request has now been integrated. Changeset: 4e90d740 Author:Conor Cleary Committer: Aleksei Efimov URL: https://git.openjdk.java.net/jdk/commit/4e90d740 Stats: 84 lines in 5 files changed: 0 ins; 48 del; 36 mod 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI Reviewed-by: rriggs, dfuchs, aefimov, chegar - PR: https://git.openjdk.java.net/jdk/pull/3416
Re: RFR: 8265237: String.join and StringJoiner can be improved further
On Wed, 14 Apr 2021 22:23:57 GMT, Peter Levart wrote: >> src/java.base/share/classes/java/lang/String.java line 3230: >> >>> 3228: >>> 3229: /** >>> 3230: * Designated join routine. >> >> Did you mean "dedicated"? > > No, I meant designated. It is the routine that all other public API entry > points call at the end to do the job. Would some other word more accurately > describe that? I definitely didn't mean "dedicated". Oh then sorry, I thought it was a typo of some sort. I'd have said something like "Centralized join logic". But whatever works for you. - PR: https://git.openjdk.java.net/jdk/pull/3501
Re: RFR: 8255410: Add ChaCha20 and Poly1305 support to SunPKCS11 provider [v3]
On Wed, 14 Apr 2021 03:58:33 GMT, Valerie Peng wrote: >> Could someone (perhaps Jamil?) please help review this change? This enhances >> SunPKCS11 provider with ChaCha20-Poly1305 cipher and ChaCha20 key generation >> support. Majority of the regression tests are adapted from the existing ones >> for SunJCE provider's ChaCha20-Poly1305 cipher impl. When testing against >> NSS v3.57, it does not have support for ChaCha20 cipher, thus I did not add >> support for ChaCha20 cipher and the corresponding parameter. >> >> Thanks! >> Valerie > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > Fixed an tagLen issue, no key+iv reuse check for decryption, and add > regression test for ChaCha20 SKF. test/jdk/sun/security/pkcs11/Cipher/TestChaChaPoly.java line 1: > 1: /* There is no compatibility Test exist between SunJCE and SunPKCS11 providers. Do we need one here. - PR: https://git.openjdk.java.net/jdk/pull/3420
Re: RFR: 8265227: Move Proc.java from security/testlibrary to test/lib [v3]
On Wed, 14 Apr 2021 22:57:55 GMT, Weijun Wang wrote: >> I'd like to move this tool to test/lib inside a proper named package. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > do not call internal method Marked as reviewed by ssahoo (Committer). - PR: https://git.openjdk.java.net/jdk/pull/3496
