Re: RFR: 8264208: Console charset API [v10]

[Naoto Sato]

> 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 two additional 
commits since the last revision:

 - Changed shell based test into java based
 - Added link to Charset#defaultChaset() in InputStreamReader.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3419/files
  - new: https://git.openjdk.java.net/jdk/pull/3419/files/083f6180..238dbb42

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3419&range=09
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3419&range=08-09

  Stats: 88 lines in 3 files changed: 27 ins; 51 del; 10 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: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v7]

[Sean Mullan]

On Tue, 13 Apr 2021 18:16:30 GMT, Weijun Wang  wrote:

>> This enhancement contains the following code changes:
>> 
>> 1. Create a new public API `javax/xml/crypto/dsig/spec/RSAPSSParameterSpec` 
>> and remove the internal one.
>> 2. Update marshaling and unmarshaling code inside `DOMRSAPSSSignatureMethod` 
>> so it understands extra fields in `PSSParameterSpec` and is aware of the 
>> defaults in both directions.
>> 3. Update `DOMSignedInfo` so that secure validation can restrict 
>> `DigestMethod` used inside `RSAPSSParameterSpec`
>> 4. Tests
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   more spec clarification

Marked as reviewed by mullan (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3181

Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Mark Raynsford]

On 2021-04-16T17:02:06 -0400
Sean Mullan  wrote:
>
> That said, I think it is worth exploring (in this JEP) or another JEP 
> ways that we might think about that could help provide DiD protection 
> for network and file access. This is an opportunity to look at the 
> problem with a fresh set of eyes, w/o the existing complicated 
> infrastructure and APIs that encompass the Security Manager.

This is something that has interested me in the past. Although I'm not
working on anything currently that would need it, I've often come up
against this sort of thing in application plugin systems. That is,
users have an application that they do trust and they want to load
plugins into it that weren't written by the application author and that
they do not necessarily trust.

Languages such as Lua handle this fairly well by having programmers
create lightweight scripting contexts for running scripts inside a
host program. The guest scripts:

  * Can't call I/O methods if they aren't given access to a
a table of I/O methods. This actually extends to not being
able to call foreign code at all if access isn't provided; 
scripts are limited to objects within the provided table.

  * Can't use unbounded heap space if a custom allocator is
handed to the script context.

  * Can't go into an infinite loop if instruction count limits
are enabled (the interpreter is pre-empted or halted if it
reaches N instructions, where N is some value configured
by the host).

  * Can't create new threads.
 
  * Are probably memory-safe, assuming a lack of bugs in the
Lua interpreter. :)

Under those constraints, it's pretty tough to do anything disruptive
even if you're trying to. Without access to I/O functions and other
foreign code in the global table, you're pretty much limited to doing
arithmetic. Quietly. And not too much of it.

Similar constraints are available for code running under GraalJS [0]
and that's certainly achieved without a security manager.

I'm more inclined to think something that is rather blunt and brute
force can be made to work well than something extremely fine-grained
like the security manager. The blunt and brute force method says
"put all this small piece of untrusted code in this box, and it's
not allowed to do anything other than the very few things I say it can,
and the code outside of the box is allowed to do whatever it could
normally do". The security manager more or less has to have a large
manually-maintained policy for the entire application and everything in
it, and I think that's where it falls over.

[0]:https://www.graalvm.org/sdk/javadoc/org/graalvm/polyglot/Context.Builder.html

-- 
Mark Raynsford | https://www.io7m.com



pgpfVUgPu8JpE.pgp
Description: OpenPGP digital signature

Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Ron Pressler]

I think it’s worth adding that treating libraries as untrusted code is 
unworkable over the long run, as their set of permissions possibly 
needs to be reexamined with every update. On the other hand, JFR can serve as a 
mechanism for tracing application behaviour, which,
when streamed, can serve to raise alerts.

— Ron

> On 16 Apr 2021, at 22:02, Sean Mullan  wrote:
> 
> Hello Reinier,
> 
> Thanks for the feedback on the JEP. If I read your message correctly, you 
> seem to be primarily concerned with logging and/or restricting access to file 
> and network operations.
> 
> In my personal view, some of the examples you present that do somewhat 
> sketchy things are probably not a good idea to put in production. There 
> should be additional audits and reviews of code, and code that is not widely 
> used should be eyed with extra suspicion. It may not be quite the same as 
> running untrusted code, but it seems quite near it.
> 
> It doesn't seem realistic to keep the SecurityManager API and possibly a lot 
> more of related APIs supported for this limited use case. Also if you remove 
> the access control and policy infrastructure from the JDK, then you are 
> responsible for the policy, determining what code is making the check, 
> whether it is trusted or not, etc. That is difficult to implement safely. My 
> guess is that it would become too hard to use, or too hard to implement 
> safely, and would become rarely used (like the Security Manager).
> 
> That said, I think it is worth exploring (in this JEP) or another JEP ways 
> that we might think about that could help provide DiD protection for network 
> and file access. This is an opportunity to look at the problem with a fresh 
> set of eyes, w/o the existing complicated infrastructure and APIs that 
> encompass the Security Manager.
> 
> Thanks,
> Sean
> 
> On 4/15/21 9:29 PM, Reinier Zwitserloot wrote:
>> 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 pr

Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Sean Mullan]

Hello Reinier,

Thanks for the feedback on the JEP. If I read your message correctly, 
you seem to be primarily concerned with logging and/or restricting 
access to file and network operations.


In my personal view, some of the examples you present that do somewhat 
sketchy things are probably not a good idea to put in production. There 
should be additional audits and reviews of code, and code that is not 
widely used should be eyed with extra suspicion. It may not be quite the 
same as running untrusted code, but it seems quite near it.


It doesn't seem realistic to keep the SecurityManager API and possibly a 
lot more of related APIs supported for this limited use case. Also if 
you remove the access control and policy infrastructure from the JDK, 
then you are responsible for the policy, determining what code is making 
the check, whether it is trusted or not, etc. That is difficult to 
implement safely. My guess is that it would become too hard to use, or 
too hard to implement safely, and would become rarely used (like the 
Security Manager).


That said, I think it is worth exploring (in this JEP) or another JEP 
ways that we might think about that could help provide DiD protection 
for network and file access. This is an opportunity to look at the 
problem with a fresh set of eyes, w/o the existing complicated 
infrastructure and APIs that encompass the Security Manager.


Thanks,
Sean

On 4/15/21 9:29 PM, Reinier Zwitserloot wrote:
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

Re: RFR: 8264208: Console charset API [v9]

[Naoto Sato]

On Fri, 16 Apr 2021 18:15:41 GMT, Roger Riggs  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Modified javadocs per suggestions.
>
> src/java.base/share/classes/java/io/InputStreamReader.java line 48:
> 
>> 46:  *  For top efficiency, consider wrapping an InputStreamReader within 
>> a
>> 47:  * BufferedReader.  For example:
>> 48:  *
> 
> Oddly, none of the reference in this class to the default charset are links 
> to Charset.defaultCharset().
> That would be a useful addition, either in the class javadoc or in the 1-arg 
> constructor that uses the default charset.

Thanks, Roger. Both are good suggestions. Will incorporate them into the next 
iteration.

-

PR: https://git.openjdk.java.net/jdk/pull/3419

Re: RFR: 8264208: Console charset API [v9]

[Roger Riggs]

On Thu, 15 Apr 2021 18:29:17 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:
> 
>   Modified javadocs per suggestions.

For the test, can it be re-written in Java.  
The direction has been to avoid creating new shell tests as they are fragile.
There are test utilities in ProcessTool to make launching and checking for 
output very easy.

src/java.base/share/classes/java/io/InputStreamReader.java line 48:

> 46:  *  For top efficiency, consider wrapping an InputStreamReader within a
> 47:  * BufferedReader.  For example:
> 48:  *

Oddly, none of the reference in this class to the default charset are links to 
Charset.defaultCharset().
That would be a useful addition, either in the class javadoc or in the 1-arg 
constructor that uses the default charset.

-

PR: https://git.openjdk.java.net/jdk/pull/3419

Integrated: 8264152: javax/net/ssl/DTLS/RespondToRetransmit.java timed out

[Fernando Guallini]

On Tue, 13 Apr 2021 13:19:17 GMT, Fernando Guallini  
wrote:

> test/jdk/javax/net/ssl/DTLS/RespondToRetransmit.java has been seen to fail 
> intermittently. 
> The server side is binding to the wildcard/localhost address which has been a 
> source of instability in many tests. Binding to loopback address fixes the 
> intermittent failures.
> 
> In addition, other changes were introduced in the tests to improve code 
> readability:
> - Reduce duplication by reusing code
> - Replace if statements with Switch expressions
> - Make fields final when appropriate
> - Convert ServerCallable and ClientCallable to records
> - Replace Byte.valueOf with Byte.parseByte to avoid redundant boxing

This pull request has now been integrated.

Changeset: 79adc16f
Author:Fernando Guallini 
Committer: Xue-Lei Andrew Fan 
URL:   https://git.openjdk.java.net/jdk/commit/79adc16f
Stats: 300 lines in 3 files changed: 78 ins; 130 del; 92 mod

8264152: javax/net/ssl/DTLS/RespondToRetransmit.java timed out

Reviewed-by: xuelei

-

PR: https://git.openjdk.java.net/jdk/pull/3466

RFR: 8240256: Better resource cleaning for SunPKCS11 Provider

[Sean Coffey]

Added capability to allow the PKCS11 Token to be destroyed once a session is 
logged out from. New configuration properties via pkcs11 config file. Cleaned 
up the native resource poller also.

New unit test case to test behaviour. Some PKCS11 tests refactored to allow 
pkcs11 provider to be configured (and tested) with a config file of choice.

Reviewer request @valeriepeng

-

Commit messages:
 - 8240256: Better resource cleaning for SunPKCS11 Provider

Changes: https://git.openjdk.java.net/jdk/pull/3544/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3544&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8240256
  Stats: 604 lines in 13 files changed: 493 ins; 63 del; 48 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3544.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3544/head:pull/3544

PR: https://git.openjdk.java.net/jdk/pull/3544

Withdrawn: 8185127: Add tests to cover hashCode() method for java supported crypto key types

[Sibabrata Sahoo]

On Wed, 14 Apr 2021 13:38:06 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.

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk/pull/3490

Re: RFR: 8185127: Add tests to cover hashCode() method for java supported crypto key types [v4]

[Sibabrata Sahoo]

On Fri, 16 Apr 2021 08:34:11 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:
> 
>   Update CompareKeys.java

Getting java.io.NotSerializableException: 
sun.security.mscapi.CKey$NativeHandles for MSCAPI RSA algorithm. Need to change 
the Test with alternate implementation. For now Withdrawing the pull request.

-

PR: https://git.openjdk.java.net/jdk/pull/3490

Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v4]

[Sibabrata Sahoo]

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

  Update CompareKeys.java

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3490/files
  - new: https://git.openjdk.java.net/jdk/pull/3490/files/1fc51e07..be5d9a62

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3490.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490

PR: https://git.openjdk.java.net/jdk/pull/3490

Re: RFR: 8185127: Add Tests to cover hashCode() method for java supported crypto key types. [v3]

[Sibabrata Sahoo]

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

  Addressed Initial comments.
  
  Addressed Initial comments.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3490/files
  - new: https://git.openjdk.java.net/jdk/pull/3490/files/ca459d0c..1fc51e07

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3490&range=01-02

  Stats: 85 lines in 1 file changed: 30 ins; 34 del; 21 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3490.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3490/head:pull/3490

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]

[Sibabrata Sahoo]

On Thu, 15 Apr 2021 18:32:26 GMT, Valerie Peng  wrote:

>> 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 &&? Would also be nice to know which one(s) failed.

Considering equality and hashCode check may not be same for all KeyTypes, i 
have relaxed the verification with || operator. Also added print statement for 
possible failure cause. Added verification for format too.

-

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]

[Sibabrata Sahoo]

On Thu, 15 Apr 2021 18:23:09 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 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.

Done. Key(pair)Generator list will be collected dynamically. In fact it will 
now test even the same algorithm names supported by different providers too 
instead finding the 1st one.

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

Added print statement for KeyGenerator and corresponding Provider list.

-

PR: https://git.openjdk.java.net/jdk/pull/3490