Re: New candidate JEP: 411: Deprecate the Security Manager for Removal

[Peter Firmstone]

Which version of Java is this planned for?   Will the last version 
supporting the security manager be a long term support version, eg back 
ports of security patches and TLS technologies?


We have our own security manager implementation and policy provider 
implementations.  Both of these are high performance and non-blocking 
and we are able to dynamically grant and revoke some permissions.   
While I acknowledge the Java policy implementation has a significant 
performance impact, due to blocking permission checks, ours is less than 
1%.  Our software doesn't share PermissionCollection instances among 
threads, or even have a Permissions cache, PermissionCollection's are 
generated for each permission check and discarded for garbage 
collection, the Permission object themselves are cached (after 
initialization and safe publication), as are the results of repeated 
permission checks.  We also have our own Permission implementations.


We have tools that generate policy files with least privilege, although 
we will manually alter them with wildcards, for network connections for 
instance.


In our software, dynamic permissions are granted after authentication of 
TLS connections.


It is too early for me to tell if there are suitable replacement 
technologies available.  I can understand the motivation for reducing 
Java's software development burden, but I think this version of Java 
might be the last for us, it would certainly be good if a long term 
support version was available, perhaps indefinitely lol.


Regards,
 
Peter Firmstone

Zeus Project Services Pty Ltd.


On 16/04/2021 4:05 am, mark.reinh...@oracle.com wrote:

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v3]

[Mandy Chung]

On Wed, 28 Apr 2021 21:10:33 GMT, Maurizio Cimadamore  
wrote:

> I just did a test:
> 
> ```
> public class TestLookup {
> public static void main(String[] args) throws Throwable {
> MethodHandle handle = 
> MethodHandles.lookup().findVirtual(CLinker.class, "downcallHandle", 
> MethodType.methodType(MethodHandle.class, Addressable.class, 
> MethodType.class, FunctionDescriptor.class));
> CLinker linker = CLinker.getInstance();
> handle.invoke(linker, MemoryAddress.NULL, 
> MethodType.methodType(void.class), FunctionDescriptor.ofVoid());
> }
> }
> ```
> 
> this fails as expected when the handle is invoked. To test I had to disable 
> the check on CLinker.getInstance - otherwise that would have always throw 
> anyway.

My statement was overly simplified.   If `handle` is invoked in another module 
B and invoked by a class in module B,  which module (the `lookup`'s module or ) 
do you expect be the caller to check against for native access check?
`CLinker::downcallHandle` is not caller-sensitive but its implementation is.

The method handle of a caller-sensitive method behaves as if it were called 
from an instruction contained in the lookup class [1].   

[1] 
https://download.java.net/java/early_access/jdk17/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#callsens

> Also, on IllegalCaller vs. IllegalAccess - looking more, I think our impl 
> throws IllegalCaller - now that was done because IllegalAccess is a checked 
> exception and we don't want a checked exception here - but the option is 
> called "enableNativeAccess" - is that still ok?

Yes the implementation throws `IllegalCallerException` which is why I point out 
this.   Hmm... this seems more of `IllegalAccess` as the caller does not have 
access to this restricted method.  OTOH, `Module::addOpens` grants deep 
reflection access to the named module if the caller has access.  Otherwise, 
`IllegalCallerException` is thrown.  So I think it's okay to throw ICE.  Others 
may have a different opinion.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 18:44:18 GMT, Daniel Fuchs  wrote:

>> I think the method is called during module bootstrap - I don't think there 
>> is a race in practice. This method is also called in other parts of 
>> ModuleBootstrap. The code you allude to is called during initialization of 
>> the IllegalNativeAccessChecker singleton, which should happen only once, and 
>> only from one thread.
>
> I'll take your word for it - the use of a volatile variable to store the 
> singleton instance made this suspicious.

good point - I think that came in when I adapted the code from 
IllegalAccessLogger - which can indeed be accessed from multiple threads. In 
this case the initialization is effectively part of bootstrap, and volatile is 
not required.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v3]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 20:40:49 GMT, Mandy Chung  wrote:

> > To be clear - by aliasing you mean when the @CallerSensitive implementation 
> > is called with invokeinterface - so, e.g. doing 
> > `MethodHandles.lookup().findVirtual(CLinker.class, ...)` right?
> 
> Yes. The caller would be java.base if it's invoked via method handle.

I just did a test:


public class TestLookup {
public static void main(String[] args) throws Throwable {
MethodHandle handle = MethodHandles.lookup().findVirtual(CLinker.class, 
"downcallHandle", MethodType.methodType(MethodHandle.class, Addressable.class, 
MethodType.class, FunctionDescriptor.class));
CLinker linker = CLinker.getInstance();
handle.invoke(linker, MemoryAddress.NULL, 
MethodType.methodType(void.class), FunctionDescriptor.ofVoid());
}
}


this fails as expected when the handle is invoked. To test I had to disable the 
check on CLinker.getInstance - otherwise that would have always throw anyway.

Also, on IllegalCaller vs. IllegalAccess - looking more, I think our impl 
throws IllegalCaller - now that was done because IllegalAccess is a checked 
exception and we don't want a checked exception here - but the option is called 
"enableNativeAccess" - is that still ok?

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v3]

[Mandy Chung]

On Wed, 28 Apr 2021 20:38:47 GMT, Maurizio Cimadamore  
wrote:

> To be clear - by aliasing you mean when the @CallerSensitive implementation 
> is called with invokeinterface - so, e.g. doing 
> `MethodHandles.lookup().findVirtual(CLinker.class, ...)` right?

Yes.   The caller would be java.base if it's invoked via method handle.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v3]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 18:33:36 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address review comments:
>   * fix typos in javadoc
>   * document ISE being thrown in all methods accepting a scope; add more 
> tests for that

> > test/jdk/jdk/internal/reflect/CallerSensitive/CheckCSMs.java
> 
> Yes that's the test. That test misses to catch your case where aliasing may 
> be possible.



> Yes that's the test. That test misses to catch your case where aliasing may 
> be possible.

To be clear - by aliasing you mean when the @CallerSensitive implementation is 
called with invokeinterface - so, e.g. doing 
`MethodHandles.lookup().findVirtual(CLinker.class, ...)` right?

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 19:02:57 GMT, Daniel Fuchs  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address first batch of review comments
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java
>  line 209:
> 
>> 207: /**
>> 208:  * The native memory address instance modelling the {@code NULL} 
>> address, associated
>> 209:  * with the {@link ResourceScope#globalScope() global} resource 
>> scope.
> 
> `{@linkplain }` ?

thanks - I'll do a pass - I think I probably got all of them wrong

-

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

Re: RFR: 8241248: NullPointerException in sun.security.ssl.HKDF.extract(HKDF.java:93)

[Alexey Bakhtin]

On Tue, 27 Apr 2021 16:00:33 GMT, Jamil Nimeh  wrote:

> I think this looks good. Thank you.
> Since you've done all the work on this one, it seems fitting that you'd 
> become the owner of the issue in JBS. Also this might be a noreg-hard 
> candidate since the failure is intermittent and requires putting load on a 
> server in order to run into the issue, correct?

@jnimeh ,
Thank you for review. I've assigned it to myself and added "noreg-hard" label 
as you suggested. The issue is intermittent and I don't know how to reproduce 
it without third-party libs.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Daniel Fuchs]

On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address first batch of review comments

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/LibraryLookup.java
 line 52:

> 50:  * 
> 51:  * For {@link #lookup(String) memory addresses} obtained from a library 
> lookup object,
> 52:  * since {@link CLinker#downcallHandle(Addressable, MethodType, 
> FunctionDescriptor) native method handles}

These should be `{@linkplain }` since the text of the link is plain text (and 
not code)

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/LibraryLookup.java
 line 88:

> 86:  * @return the memory segment associated with the library symbol (if 
> any).
> 87:  * @throws IllegalArgumentException if the address associated with 
> the lookup symbol do not match the
> 88:  * {@link MemoryLayout#byteAlignment() alignment constraints} in 
> {@code layout}.

Same remark here (`{@linkplain }`)

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java
 line 43:

> 41:  * when performing memory dereference operations using a memory access 
> var handle (see {@link MemoryHandles}).
> 42:  * 
> 43:  * A memory address is associated with a {@link ResourceScope resource 
> scope}; the resource scope determines the

`{@linkplain }`

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java
 line 46:

> 44:  * lifecycle of the memory address, and whether the address can be used 
> from multiple threads. Memory addresses
> 45:  * obtained from {@link #ofLong(long) numeric values}, or from native 
> code, are associated with the
> 46:  * {@link ResourceScope#globalScope() global resource scope}. Memory 
> addresses obtained from segments

... and here to (`{@linkplain }`)

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java
 line 102:

> 100:  * @param segment the segment relative to which this address offset 
> should be computed
> 101:  * @throws IllegalArgumentException if {@code segment} is not 
> compatible with this address; this can happen, for instance,
> 102:  * when {@code segment} models an heap memory region, while this 
> address is a {@link #isNative() native} address.

`{@linkplain }`

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryAddress.java
 line 209:

> 207: /**
> 208:  * The native memory address instance modelling the {@code NULL} 
> address, associated
> 209:  * with the {@link ResourceScope#globalScope() global} resource 
> scope.

`{@linkplain }` ?

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Daniel Fuchs]

On Wed, 28 Apr 2021 18:19:14 GMT, Maurizio Cimadamore  
wrote:

>> src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java
>>  line 78:
>> 
>>> 76: int index = 0;
>>> 77: // the system property is removed after decoding
>>> 78: String value = getAndRemoveProperty(prefix + index);
>> 
>> I am not sure what is going on with the removal of the properties, but if 
>> I'm not mistaken this is racy: from the implementation of the checker() 
>> method above, it looks as if two different threads could trigger a call to 
>> the decode() function concurrently, which can result in a random 
>> partitioning of the properties against the two checkers being instantiated, 
>> with one of them being eventually set as the system-wide checker.
>
> I think the method is called during module bootstrap - I don't think there is 
> a race in practice. This method is also called in other parts of 
> ModuleBootstrap. The code you allude to is called during initialization of 
> the IllegalNativeAccessChecker singleton, which should happen only once, and 
> only from one thread.

I'll take your word for it - the use of a volatile variable to store the 
singleton instance made this suspicious.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 18:23:41 GMT, Mandy Chung  wrote:

> > I believe that we had to move @CallerSensitive out of interfaces because 
> > there was a test that was checking that @cs was not put on "virtual" 
> > methods.
> 
> Good if we do have such test. We need to re-examine this with the security 
> team. I suggest to create a JBS issue and follow up separately.

The test in question is:

test/jdk/jdk/internal/reflect/CallerSensitive/CheckCSMs.java

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v3]

[Maurizio Cimadamore]

> This PR contains the API and implementation changes for JEP-412 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/412

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  Address review comments:
  * fix typos in javadoc
  * document ISE being thrown in all methods accepting a scope; add more tests 
for that

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3699/files
  - new: https://git.openjdk.java.net/jdk/pull/3699/files/a80d8180..24e554c5

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3699=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3699=01-02

  Stats: 112 lines in 6 files changed: 50 ins; 0 del; 62 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3699.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3699/head:pull/3699

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Mandy Chung]

On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address first batch of review comments

> I believe that we had to move @CallerSensitive out of interfaces because 
> there was a test that was checking that @cs was not put on "virtual" methods.

Good if we do have such test.We need to re-examine this with the security 
team.   I suggest to create a JBS issue and follow up separately.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 17:53:44 GMT, Daniel Fuchs  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address first batch of review comments
>
> src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java
>  line 40:
> 
>> 38: 
>> 39: private IllegalNativeAccessChecker(Set allowedModuleNames, 
>> boolean allowAllUnnamedModules) {
>> 40: this.allowedModuleNames = 
>> Collections.unmodifiableSet(allowedModuleNames);
> 
> Should that be Set.copyOf() to take advantage of the immutability of `SetN` 
> (but at the expense of additional copying)...

Honestly, I'm not even sure why we should be concerned about mutation of the 
set here - since we create this with a bunch of values read from a system 
property... e.g. should we just store `allowedModuleNames` period?

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 18:07:32 GMT, Daniel Fuchs  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address first batch of review comments
>
> src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java
>  line 78:
> 
>> 76: int index = 0;
>> 77: // the system property is removed after decoding
>> 78: String value = getAndRemoveProperty(prefix + index);
> 
> I am not sure what is going on with the removal of the properties, but if I'm 
> not mistaken this is racy: from the implementation of the checker() method 
> above, it looks as if two different threads could trigger a call to the 
> decode() function concurrently, which can result in a random partitioning of 
> the properties against the two checkers being instantiated, with one of them 
> being eventually set as the system-wide checker.

I think the method is called during module bootstrap - I don't think there is a 
race in practice. This method is also called in other parts of ModuleBootstrap. 
The code you allude to is called during initialization of the 
IllegalNativeAccessChecker singleton, which should happen only once, and only 
from one thread.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 18:02:03 GMT, Mandy Chung  wrote:

> I reviewed the `--enable-native-access` related change that looks fine.
> 
> > Access to restricted methods from any other module not in the list is 
> > disallowed and will result in an IllegalAccessException.
> 
> I think you meant to say `IllegalCallerException` instead of 
> `IllegalAccessException`. Also do you intend to have javadoc to generate 
> `@throw IllegalCallerException` for the restricted methods automatically 
> besides the javadoc description?
> 

IllegalCalller is probably better yes - we started off with an access-like 
check, so things have evolved a bit. I'll also add the @throws.

> Making the restricted methods as `@CallerSensitive` in order to get the 
> caller class for native access check is the proper approach. However, some 
> interface methods are restricted methods such as `CLinker::downcallHandle` 
> whose the implementation method is `@CallerSensitive`. I concern with the 
> security issue with method handle and type aliasing. On the other hand, 
> `CLinker` is a sealed interface and only implemented by the platform and so 
> it's less of a concern. I think the interface method should also be 
> `@CallerSensitive` so that for example a method handle for 
> `CLinker::downcallHandle` will be produced with the proper caller-sensitive 
> context.

I believe that we had to move @CallerSensitive out of interfaces because there 
was a test that was checking that @CS was not put on "virtual" methods.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Daniel Fuchs]

On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address first batch of review comments

src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java 
line 40:

> 38: 
> 39: private IllegalNativeAccessChecker(Set allowedModuleNames, 
> boolean allowAllUnnamedModules) {
> 40: this.allowedModuleNames = 
> Collections.unmodifiableSet(allowedModuleNames);

Should that be Set.copyOf() to take advantage of the immutability of `SetN` 
(but at the expense of additional copying)...

src/java.base/share/classes/jdk/internal/module/IllegalNativeAccessChecker.java 
line 78:

> 76: int index = 0;
> 77: // the system property is removed after decoding
> 78: String value = getAndRemoveProperty(prefix + index);

I am not sure what is going on with the removal of the properties, but if I'm 
not mistaken this is racy: from the implementation of the checker() method 
above, it looks as if two different threads could trigger a call to the 
decode() function concurrently, which can result in a random partitioning of 
the properties against the two checkers being instantiated, with one of them 
being eventually set as the system-wide checker.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Mandy Chung]

On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address first batch of review comments

I reviewed the `--enable-native-access` related change that looks fine.

> Access to restricted methods from any other module not in the list is 
> disallowed and will result in an IllegalAccessException.

I think you meant to say `IllegalCallerException` instead of 
`IllegalAccessException`.  Also do you intend to have javadoc to generate 
`@throw IllegalCallerException` for  the restricted methods automatically 
besides the javadoc description?

Making the restricted methods as `@CallerSensitive` in order to get the caller 
class for native access check is the proper approach.   However, some interface 
methods are restricted methods such as `CLinker::downcallHandle` whose the 
implementation method is `@CallerSensitive`.I concern with the security 
issue with method handle and type aliasing.   On the other hand, `CLinker` is a 
sealed interface and only implemented by the platform and so it's less of a 
concern.   I think the interface method should also be `@CallerSensitive` so 
that for example a method handle for `CLinker::downcallHandle` will be produced 
with the proper caller-sensitive context.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Daniel Fuchs]

On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address first batch of review comments

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/package-info.java 
line 224:

> 222:  * Some methods in this package are considered restricted. 
> Restricted methods are typically used to bind native
> 223:  * foreign data and/or functions to first-class Java API elements which 
> can then be used directly by client. For instance
> 224:  * the restricted method {@link 
> jdk.incubator.foreign.MemoryAddress#asSegment(long, ResourceScope)} can be 
> used to create

typo: `used directly by client.` => `used directly by clients.` ?

-

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

Integrated: 8196415: Disable SHA-1 Signed JARs

[Sean Mullan]

On Mon, 26 Apr 2021 17:29:26 GMT, Sean Mullan  wrote:

> This change will restrict JARs signed with SHA-1 algorithms and treat them as 
> if they were unsigned. This applies to the algorithms used to digest, sign, 
> and optionally timestamp the JAR. It also applies to the signature and digest 
> algorithms of the certificates in the certificate chain of the code signer 
> and the Timestamp Authority, and any CRLs or OCSP responses that are used to 
> verify if those certificates have been revoked.
> 
> In order to reduce the compatibility risk for applications that have been 
> previously timestamped or use private CAs, there are two exceptions to this 
> policy:
> 
> - Any JAR signed with SHA-1 algorithms and timestamped prior to January 01, 
> 2019 will not be restricted.
> - Any JAR signed with a SHA-1 certificate that does not chain back to a Root 
> CA included by default in the JDK `cacerts` keystore will not be restricted.
> 
> These exceptions may be removed in a future JDK release.
> 
> All tests are in the closed repo for now.
> 
> CSR: https://bugs.openjdk.java.net/browse/JDK-8264362

This pull request has now been integrated.

Changeset: 27805775
Author:Sean Mullan 
URL:   
https://git.openjdk.java.net/jdk/commit/278057756a1a79a4b030750c48b821ba9735a0f9
Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod

8196415: Disable SHA-1 Signed JARs

Reviewed-by: coffeys

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Alan Bateman]

On Wed, 28 Apr 2021 13:47:43 GMT, Maurizio Cimadamore  
wrote:

>> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java 
>> line 270:
>> 
>>> 268: 
>>> 269: /**
>>> 270:  * Converts a Java string into a null-terminated C string, using 
>>> the platform's default charset,
>> 
>> Sorry if this has come up before, but, is the platform's default charset the 
>> right choice here? For other areas, we choose UTF-8 as the default. In fact, 
>> there is a draft JEP to move the default charset to UTF-8. So if there is an 
>> implicit need to match the underlying platform's charset then this may need 
>> to be revisited.  For now, I just want to check that this is not an 
>> accidental reliance on the platform's default charset, but a deliberate one.
>
> I believe here the goal is to be consistent with `String::getBytes`:
> 
> 
> /**
>  * Encodes this {@code String} into a sequence of bytes using the
>  * platform's default charset, storing the result into a new byte array.
>  *
>  *  The behavior of this method when this string cannot be encoded in
>  * the default charset is unspecified.  The {@link
>  * java.nio.charset.CharsetEncoder} class should be used when more control
>  * over the encoding process is required.
>  *
>  * @return  The resultant byte array
>  *
>  * @since  1.1
>  */
> public byte[] getBytes() {
> return encode(Charset.defaultCharset(), coder(), value);
> }
> 
> 
> So, you are right in that there's an element of platform-dependency here - 
> but I think it's a dependency that users learned to "ignore" mostly. If 
> developers want to be precise, and platform independent, they can always use 
> the Charset-accepting method. Of course this could be revisited, but I think 
> there is some consistency in what the API is trying to do. If, in the future, 
> defaultCharset will just be Utf8 - well, that's ok too - as long as the 
> method is specified to be "defaultCharset" dependent, what's behind 
> "defaultCharset" is an implementation detail that the user will have to be 
> aware of one way or another.

Naoto is working on a couple of changes in advance of JEP 400. One of these is 
to expose a system property with the host charset and I suspect that the 
CLinker method will want to use that instead of Charset.defaultCharset.

-

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

Integrated: 8228442: DHKeyExchange/LegacyDHEKeyExchange.java failed due to "SSLException: An established connection was aborted by the software in your host machine"

[Fernando Guallini]

On Thu, 22 Apr 2021 11:41:40 GMT, Fernando Guallini  
wrote:

> Test DHKeyExchange/LegacyDHEKeyExchange.java has been seen to fail 
> intermittently. There is a thread synchronisation issue that is fixed by:
> 
> - Using SSLSocketTemplate that handles client/server socket configuration and 
> synchronisation before connection
> - Making use of a CountDownLatch and socket linger to facilitate the socket 
> to close gracefully after both ends have finished the transmission

This pull request has now been integrated.

Changeset: 7e3bc4cb
Author:Fernando Guallini 
Committer: Xue-Lei Andrew Fan 
URL:   
https://git.openjdk.java.net/jdk/commit/7e3bc4cb444c7b7f3cd3e514b4ad7d7137799401
Stats: 287 lines in 1 file changed: 14 ins; 252 del; 21 mod

8228442: DHKeyExchange/LegacyDHEKeyExchange.java failed due to "SSLException: 
An established connection was aborted by the software in your host machine"

Reviewed-by: xuelei

-

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

RFR: 8266220: keytool still prompt for store password on a password-less pkcs12 file if -storetype pkcs12 is specified

[Weijun Wang]

It's awkward that for a password-less pkcs12 keystore, `keytool -list` does not 
prompt for a password but `keytool -list -storetype pkcs12` does.

-

Commit messages:
 - 8266220: keytool still prompt for store password on a password-less pkcs12 
file if -storetype pkcs12 is specified

Changes: https://git.openjdk.java.net/jdk/pull/3764/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3764=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8266220
  Stats: 35 lines in 2 files changed: 27 ins; 2 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3764.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3764/head:pull/3764

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

Re: JEP 411: methods in j.u.c.Executors

[Sean Mullan]

On 4/20/21 12:06 PM, Anthony Vanelverdinghe wrote:

The JEP should also address the following methods in 
java.util.concurrent.Executors: privilegedCallable, 
privilegedCallableUsingCurrentClassLoader​, privilegedThreadFactory
Since AccessController et al. will be terminally deprecated, I'd assume these 
methods should be as well.


Yes. I will add them to the Deprecated APIs section of the JEP.

--Sean



Kind regards,
Anthony

On Thursday, April 15, 2021 20:05 CEST, mark.reinh...@oracle.com wrote:
  

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 12:47:36 GMT, Chris Hegarty  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address first batch of review comments
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java 
> line 270:
> 
>> 268: 
>> 269: /**
>> 270:  * Converts a Java string into a null-terminated C string, using 
>> the platform's default charset,
> 
> Sorry if this has come up before, but, is the platform's default charset the 
> right choice here? For other areas, we choose UTF-8 as the default. In fact, 
> there is a draft JEP to move the default charset to UTF-8. So if there is an 
> implicit need to match the underlying platform's charset then this may need 
> to be revisited.  For now, I just want to check that this is not an 
> accidental reliance on the platform's default charset, but a deliberate one.

I believe here the goal is to be consistent with `String::getBytes`:


/**
 * Encodes this {@code String} into a sequence of bytes using the
 * platform's default charset, storing the result into a new byte array.
 *
 *  The behavior of this method when this string cannot be encoded in
 * the default charset is unspecified.  The {@link
 * java.nio.charset.CharsetEncoder} class should be used when more control
 * over the encoding process is required.
 *
 * @return  The resultant byte array
 *
 * @since  1.1
 */
public byte[] getBytes() {
return encode(Charset.defaultCharset(), coder(), value);
}


So, you are right in that there's an element of platform-dependency here - but 
I think it's a dependency that users learned to "ignore" mostly. If developers 
want to be precise, and platform independent, they can always use the 
Charset-accepting method. Of course this could be revisited, but I think there 
is some consistency in what the API is trying to do. If, in the future, 
defaultCharset will just be Utf8 - well, that's ok too - as long as the method 
is specified to be "defaultCharset" dependent, what's behind "defaultCharset" 
is an implementation detail that the user will have to be aware of one way or 
another.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 13:08:26 GMT, Chris Hegarty  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address first batch of review comments
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java
>  line 693:
> 
>> 691:  */
>> 692: static MemorySegment allocateNative(MemoryLayout layout, 
>> ResourceScope scope) {
>> 693: Objects.requireNonNull(scope);
> 
> Should the allocateNative methods declare that they throw ISE, if the given 
> ResourceScope is not alive?   ( I found myself asking this q, then 
> considering the behaviour of a SegmentAllocator that is asked to allocate 
> after a RS has been closed )

Good point, yes it should

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Chris Hegarty]

On Wed, 28 Apr 2021 10:42:54 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Address first batch of review comments

Like Paul, I tracked the changes to this API in the Panama repo. Most of my 
remaining comments are small and come from re-reading the API docs.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 
270:

> 268: 
> 269: /**
> 270:  * Converts a Java string into a null-terminated C string, using the 
> platform's default charset,

Sorry if this has come up before, but, is the platform's default charset the 
right choice here? For other areas, we choose UTF-8 as the default. In fact, 
there is a draft JEP to move the default charset to UTF-8. So if there is an 
implicit need to match the underlying platform's charset then this may need to 
be revisited.  For now, I just want to check that this is not an accidental 
reliance on the platform's default charset, but a deliberate one.

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java
 line 101:

> 99:  * Lifecycle and confinement
> 100:  *
> 101:  * Memory segments are associated to a resource scope (see {@link 
> ResourceScope}), which can be accessed using

Typo?? "Memory segments are associated *with* a resource scope"

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java
 line 112:

> 110: MemoryAccess.getLong(segment); // already closed!
> 111:  * }
> 112:  * Additionally, access to a memory segment in subject to the 
> thread-confinement checks enforced by the owning scope; that is,

Typo?? "Additionally, access to a memory segment *is* subject to ..."

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java
 line 114:

> 112:  * Additionally, access to a memory segment in subject to the 
> thread-confinement checks enforced by the owning scope; that is,
> 113:  * if the segment is associated with a shared scope, it can be accessed 
> by multiple threads; if it is associated with a confined
> 114:  * scope, it can only be accessed by the thread which own the scope.

Typo?? "it can only be accessed by the thread which ownS the scope."

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java
 line 693:

> 691:  */
> 692: static MemorySegment allocateNative(MemoryLayout layout, 
> ResourceScope scope) {
> 693: Objects.requireNonNull(scope);

Should the allocateNative methods declare that they throw ISE, if the given 
ResourceScope is not alive?   ( I found myself asking this q, then considering 
the behaviour of a SegmentAllocator that is asked to allocate after a RS has 
been closed )

-

Marked as reviewed by chegar (Reviewer).

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

Re: RFR: 8236671: NullPointerException in JKS keystore [v2]

[Sean Coffey]

> Trivial enough change. Improved the exception thrown from JceKeyStore also.

Sean Coffey has updated the pull request with a new target base due to a merge 
or a rebase. The incremental webrev excludes the unrelated changes brought in 
by the merge/rebase. The pull request contains four additional commits since 
the last revision:

 - Check for null before try block
 - Merge branch 'master' of https://github.com/openjdk/jdk into JDK-8236671-NPE
 - Fix white space
 - 8236671: NullPointerException in JKS keystore

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3588/files
  - new: https://git.openjdk.java.net/jdk/pull/3588/files/836ea7e7..54baaad7

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

  Stats: 42439 lines in 1504 files changed: 9588 ins; 28041 del; 4810 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3588.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3588/head:pull/3588

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

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

[Markus Gronlund]

Thanks Chris,

I like that approach very much.

Thanks again
Markus

From: Chris Hegarty 
Sent: den 28 april 2021 12:51
To: Markus Gronlund 
Cc: Lim ; Ron Pressler 
; security-dev@openjdk.java.net
Subject: Re: JEP411: Missing use-case: Monitoring / restricting libraries




On 28 Apr 2021, at 11:38, Markus Gronlund 
mailto:markus.gronl...@oracle.com>> wrote:

Hi Lim,

JFR specific feedback can be posted to: 
hotspot-jfr-...@openjdk.java.net

Thanks Markus. That is the appropriate list to send JFR feedback.

Just to add, I filed an Enhancement Request in JIRA, 8265962: "Evaluate adding 
Networking JFR events” [1], to track the possibility of adding JFR events to 
the JDK libraries that perform low-level networking activity (which is mostly 
in the purview of the networking and libraries area). If we had such, then it 
would be possible to monitor *all* low-level network activity performed by the 
platform, regardless of which higher-level library is performing the activity. 
Clearly such would not capture URLs, but rather the network activity that would 
be triggered by, say, an HTTP Client library. This seems like a more fruitful 
and uniform approach, rather than trying to add JFR events to, say, every HTTP 
library.

-Chris.

[1] https://bugs.openjdk.java.net/browse/JDK-8265962



Thanks
Markus

-Original Message-
From: Lim 
mailto:lim.chainz11+mail...@gmail.com>>
Sent: den 28 april 2021 12:18
To: Markus Gronlund 
mailto:markus.gronl...@oracle.com>>
Cc: Ron Pressler mailto:ron.press...@oracle.com>>; 
security-dev@openjdk.java.net
Subject: Re: JEP411: Missing use-case: Monitoring / restricting libraries

Hi Markus, thank you for giving me the guidance for performing the JFR 
programmatically.
I am able to test if my use case is suitable. Where do I provide my 
feedback/issue of using the streamed JFR?

On Wed, Apr 21, 2021 at 10:32 PM Markus Gronlund 
mailto:markus.gronl...@oracle.com>> wrote:

If the existing event probes in the JDK does not give you the information you 
need, like the name of URL's, it can be a reached by building your own "custom 
events" via the Events API [3]. It can be harder to add events to unknown code 
dynamically, but it can be done and you can use java.lang.Instrument to build 
an agent to inject the custom event.

I understand that new events can be added in code that I have control of using 
the Events API but in this case, which is the name of URLs is not feasible.

Firstly, using a Java agent to instrument bytecode cannot be scaled because 
there are a lot of HTTP libraries, including the built in Java APIs and 3rd 
parties such as Apache HTTP, OkHttp. They can also roll their own "HTTP 
wrapper" if the author doesn't want dependency. In addition, these 3rd party 
libraries can be shaded and relocated, making it harder to target via 
instrumentation.

Obfuscation can also have an impact on reliability of instrumentation, since 
obfuscation can be changed in every version and what if the obfuscation has 
"anti-tamper/anti-debug" features? This is not scalable if we need to monitor 
for each library that might call URLs.


If there is a general problem area and provides a good scaling factor, and the 
URL information might just be such a case, it can make sense to investigate if 
this information can be provided directly by the JDK, by extending existing or 
new JFR events.

I believe that the majority of the HTTP libraries, and code that roll their own 
are using the built in Java APIs, thus monitoring the built in API that is used 
for making URLs calls make sense. Then, it can be scaled to most of the 
libraries compared to instrumenting each one which has its own problem stated 
above.

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

[Chris Hegarty]

On 28 Apr 2021, at 11:38, Markus Gronlund 
mailto:markus.gronl...@oracle.com>> wrote:

Hi Lim,

JFR specific feedback can be posted to: 
hotspot-jfr-...@openjdk.java.net

Thanks Markus. That is the appropriate list to send JFR feedback.

Just to add, I filed an Enhancement Request in JIRA, 8265962: "Evaluate adding 
Networking JFR events” [1], to track the possibility of adding JFR events to 
the JDK libraries that perform low-level networking activity (which is mostly 
in the purview of the networking and libraries area). If we had such, then it 
would be possible to monitor *all* low-level network activity performed by the 
platform, regardless of which higher-level library is performing the activity. 
Clearly such would not capture URLs, but rather the network activity that would 
be triggered by, say, an HTTP Client library. This seems like a more fruitful 
and uniform approach, rather than trying to add JFR events to, say, every HTTP 
library.

-Chris.

[1] https://bugs.openjdk.java.net/browse/JDK-8265962


Thanks
Markus

-Original Message-
From: Lim 
mailto:lim.chainz11+mail...@gmail.com>>
Sent: den 28 april 2021 12:18
To: Markus Gronlund 
mailto:markus.gronl...@oracle.com>>
Cc: Ron Pressler mailto:ron.press...@oracle.com>>; 
security-dev@openjdk.java.net
Subject: Re: JEP411: Missing use-case: Monitoring / restricting libraries

Hi Markus, thank you for giving me the guidance for performing the JFR 
programmatically.
I am able to test if my use case is suitable. Where do I provide my 
feedback/issue of using the streamed JFR?

On Wed, Apr 21, 2021 at 10:32 PM Markus Gronlund 
mailto:markus.gronl...@oracle.com>> wrote:
If the existing event probes in the JDK does not give you the information you 
need, like the name of URL's, it can be a reached by building your own "custom 
events" via the Events API [3]. It can be harder to add events to unknown code 
dynamically, but it can be done and you can use java.lang.Instrument to build 
an agent to inject the custom event.

I understand that new events can be added in code that I have control of using 
the Events API but in this case, which is the name of URLs is not feasible.

Firstly, using a Java agent to instrument bytecode cannot be scaled because 
there are a lot of HTTP libraries, including the built in Java APIs and 3rd 
parties such as Apache HTTP, OkHttp. They can also roll their own "HTTP 
wrapper" if the author doesn't want dependency. In addition, these 3rd party 
libraries can be shaded and relocated, making it harder to target via 
instrumentation.

Obfuscation can also have an impact on reliability of instrumentation, since 
obfuscation can be changed in every version and what if the obfuscation has 
"anti-tamper/anti-debug" features? This is not scalable if we need to monitor 
for each library that might call URLs.

If there is a general problem area and provides a good scaling factor, and the 
URL information might just be such a case, it can make sense to investigate if 
this information can be provided directly by the JDK, by extending existing or 
new JFR events.

I believe that the majority of the HTTP libraries, and code that roll their own 
are using the built in Java APIs, thus monitoring the built in API that is used 
for making URLs calls make sense. Then, it can be scaled to most of the 
libraries compared to instrumenting each one which has its own problem stated 
above.

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Chris Hegarty]

On Wed, 28 Apr 2021 09:06:29 GMT, Chris Hegarty  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address first batch of review comments
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java
>  line 205:
> 
>> 203:  * until all the resource scope handles acquired from it have been 
>> {@link #release(Handle)} released}.
>> 204:  * @return a resource scope handle.
>> 205:  */
> 
> Given recent changes, it might be good to generalise the opening sentence 
> here. Maybe :
>  " Acquires a resource scope handle associated with this resource scope. If 
> the resource scope is explicit ... "   Or some such.

The spec for the acquire method talks only of explicit resource scopes. The 
comment is suggesting that the spec could be generalised a little, since it is 
possible to acquire a resource scope handle associated with implicit scopes.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Maurizio Cimadamore]

> This PR contains the API and implementation changes for JEP-412 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/412

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  Address first batch of review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3699/files
  - new: https://git.openjdk.java.net/jdk/pull/3699/files/7545f71f..a80d8180

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

  Stats: 42 lines in 4 files changed: 25 ins; 11 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3699.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3699/head:pull/3699

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v2]

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 09:10:37 GMT, Chris Hegarty  wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Address first batch of review comments
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java
>  line 135:
> 
>> 133: } finally {
>> 134:segment.scope().release(segmentHandle);
>> 135: }
> 
> I do like the symmetry in this example, but just to add an alternative idiom:
> `segmentHandle.scope().release(segmentHandle)`
> , which guarantees to avoid release throwing and IAE

I see what you mean - I don't think I want to encourage that style too much by 
giving it prominence in the javadoc. It's true you can go back to the scope 
from the handle, so that you are guaranteed to release the right thing, but I 
think that should be unnecessary in most idiomatic uses.

-

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

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

[Markus Gronlund]

Hi Lim,

JFR specific feedback can be posted to: hotspot-jfr-...@openjdk.java.net

Thanks
Markus

-Original Message-
From: Lim  
Sent: den 28 april 2021 12:18
To: Markus Gronlund 
Cc: Ron Pressler ; security-dev@openjdk.java.net
Subject: Re: JEP411: Missing use-case: Monitoring / restricting libraries

Hi Markus, thank you for giving me the guidance for performing the JFR 
programmatically.
I am able to test if my use case is suitable. Where do I provide my 
feedback/issue of using the streamed JFR?

On Wed, Apr 21, 2021 at 10:32 PM Markus Gronlund  
wrote:
> If the existing event probes in the JDK does not give you the information you 
> need, like the name of URL's, it can be a reached by building your own 
> "custom events" via the Events API [3]. It can be harder to add events to 
> unknown code dynamically, but it can be done and you can use 
> java.lang.Instrument to build an agent to inject the custom event.

I understand that new events can be added in code that I have control of using 
the Events API but in this case, which is the name of URLs is not feasible.

Firstly, using a Java agent to instrument bytecode cannot be scaled because 
there are a lot of HTTP libraries, including the built in Java APIs and 3rd 
parties such as Apache HTTP, OkHttp. They can also roll their own "HTTP 
wrapper" if the author doesn't want dependency. In addition, these 3rd party 
libraries can be shaded and relocated, making it harder to target via 
instrumentation.

Obfuscation can also have an impact on reliability of instrumentation, since 
obfuscation can be changed in every version and what if the obfuscation has 
"anti-tamper/anti-debug" features? This is not scalable if we need to monitor 
for each library that might call URLs.

> If there is a general problem area and provides a good scaling factor, and 
> the URL information might just be such a case, it can make sense to 
> investigate if this information can be provided directly by the JDK, by 
> extending existing or new JFR events.

I believe that the majority of the HTTP libraries, and code that roll their own 
are using the built in Java APIs, thus monitoring the built in API that is used 
for making URLs calls make sense. Then, it can be scaled to most of the 
libraries compared to instrumenting each one which has its own problem stated 
above.

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator)

[Maurizio Cimadamore]

On Wed, 28 Apr 2021 09:06:29 GMT, Chris Hegarty  wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java
>  line 205:
> 
>> 203:  * until all the resource scope handles acquired from it have been 
>> {@link #release(Handle)} released}.
>> 204:  * @return a resource scope handle.
>> 205:  */
> 
> Given recent changes, it might be good to generalise the opening sentence 
> here. Maybe :
>  " Acquires a resource scope handle associated with this resource scope. If 
> the resource scope is explicit ... "   Or some such.

I'm afraid I don't get this comment ;-)

-

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

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

[Lim]

On Wed, Apr 21, 2021 at 8:38 PM Ron Pressler  wrote:
> Its current events might be not have everything you want, but will be 
> expanded, in
> part to address the functionality that will be lost with the removal of 
> Security Manager.

I agree that monitoring needs to be improved since there is a lack of
monitoring APIs except for JFR. Until those monitoring APIs are on par
with the usage of SM "monitoring", it makes no sense to remove without
providing alternatives.

> Libraries that can disable the Security Manager aren’t able to circumvent 
> OS-level
> sandboxing. If you’re not afraid of that, then they’re trusted and JFR is 
> superior;
> if they’re untrusted, then configuring the Security Manager correctly for 
> untrusted rich
> libraries is very difficult.

Since you said that it cannot circumvent OS-level sandboxing, what
will prevent those
libraries to monitor if there is JFR active and become dormant until
there is no JFR
present, then it will execute the malicious behavior; Or, the library
attempts to hide
or render the JFR useless so that it will not be recorded and noticed?

On Wed, Apr 21, 2021 at 8:55 PM Ron Pressler  wrote:
> For rich libraries and applications, your best bet is an OS-level sandbox. 
> The Security Manager
> might give you a false sense of security.

Yes, OS-level sandbox is good but can it be scalable for many types of end
users that have different OS and hardware environments? In addition, how many
end users out there have used sandbox to isolate their desktop applications
except if the program has built-in sandbox such as web browsers? Programs
such as Docker does not count.

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

[Lim]

Hi Markus, thank you for giving me the guidance for performing the JFR
programmatically.
I am able to test if my use case is suitable. Where do I provide my
feedback/issue of using the streamed JFR?

On Wed, Apr 21, 2021 at 10:32 PM Markus Gronlund
 wrote:
> If the existing event probes in the JDK does not give you the information you 
> need, like the name of URL's, it can be a reached by building your own 
> "custom events" via the Events API [3]. It can be harder to add events to 
> unknown code dynamically, but it can be done and you can use 
> java.lang.Instrument to build an agent to inject the custom event.

I understand that new events can be added in code that I have control of
using the Events API but in this case, which is the name of URLs is not
feasible.

Firstly, using a Java agent to instrument bytecode cannot be scaled because
there are a lot of HTTP libraries, including the built in Java APIs and 3rd
parties such as Apache HTTP, OkHttp. They can also roll their own "HTTP
wrapper" if the author doesn't want dependency. In addition, these 3rd party
libraries can be shaded and relocated, making it harder to target via
instrumentation.

Obfuscation can also have an impact on reliability of instrumentation,
since obfuscation can be changed in every version and what if the
obfuscation has "anti-tamper/anti-debug" features? This is not
scalable if we need to monitor for each library that might call URLs.

> If there is a general problem area and provides a good scaling factor, and 
> the URL information might just be such a case, it can make sense to 
> investigate if this information can be provided directly by the JDK, by 
> extending existing or new JFR events.

I believe that the majority of the HTTP libraries, and code that roll
their own are using the built in Java APIs, thus monitoring the built
in API that is used for making URLs calls make sense. Then, it can be
scaled to most of the libraries compared to instrumenting each one
which has its own problem stated above.

Integrated: 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 now been integrated.

Changeset: 343a4a76
Author:Sibabrata Sahoo 
URL:   
https://git.openjdk.java.net/jdk/commit/343a4a76f273078f78897e8fb7e695bc2c111536
Stats: 149 lines in 1 file changed: 149 ins; 0 del; 0 mod

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

Reviewed-by: valeriep

-

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

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

[Sibabrata Sahoo]

On Wed, 28 Apr 2021 06:56:21 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:
> 
>   Typo error in comment section updated
>   
>   Typo error in comment section updated

Pushing the change after addressing minor typo issue in the review comment.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator)

[Chris Hegarty]

On Mon, 26 Apr 2021 17:10:13 GMT, Maurizio Cimadamore  
wrote:

> This PR contains the API and implementation changes for JEP-412 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/412

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java
 line 135:

> 133: } finally {
> 134:segment.scope().release(segmentHandle);
> 135: }

I do like the symmetry in this example, but just to add an alternative idiom:
`segmentHandle.scope().release(segmentHandle)`
, which guarantees to avoid release throwing and IAE

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java
 line 186:

> 184:  * this resource scope is confined, and this method is 
> called from a thread other than the thread owning this resource scope
> 185:  * this resource scope is shared and a resource associated 
> with this scope is accessed while this method is called
> 186:  * one or more handles (see {@link #acquire()}) associated 
> with this resource scope have not been closed

Minor spec suggestion: "... associated with this resource scope have not been 
*released*"

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/ResourceScope.java
 line 205:

> 203:  * until all the resource scope handles acquired from it have been 
> {@link #release(Handle)} released}.
> 204:  * @return a resource scope handle.
> 205:  */

Given recent changes, it might be good to generalise the opening sentence here. 
Maybe :
 " Acquires a resource scope handle associated with this resource scope. If the 
resource scope is explicit ... "   Or some such.

-

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

Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator)

[Chris Hegarty]

On Tue, 27 Apr 2021 18:40:24 GMT, Alan Bateman  wrote:

>> This PR contains the API and implementation changes for JEP-412 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/412
>
> src/java.base/share/classes/sun/nio/ch/IOUtil.java line 466:
> 
>> 464: }
>> 465: 
>> 466: private static final JavaNioAccess NIO_ACCESS = 
>> SharedSecrets.getJavaNioAccess();
> 
> It might be cleaner to move to acquire/release methods to their own 
> supporting class as it's not really IOUtil.

I went back and forth on this a number of times already. I think where we 
landed is a reasonable place, given the current shape of the code.

Scope is a private property of Buffer, and as such should be consulted anywhere 
where a buffer's address is being accessed. In fact, a prior prototype would 
not allow access to the underlying address value without the caller passing a 
valid handle for the buffer view's scope. It's hard to find the sweet-spot here 
between code reuse and safety, but the high-order bit is that the code 
accessing the address is auditable and testable to avoid accessing memory 
unsafely. Maybe there is a better alternative implementation code structure (at 
the cost of some duplication), but it is not obvious to me what that is (and I 
have given it some thought). Suggestions welcome.

Note, there is a little more follow-on work to be done in this area, if we are 
to expand support to other non-TCP channel implementations. Maybe investigation 
into possible code refactorings could be done as part of that?

-

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

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

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

  Typo error in comment section updated
  
  Typo error in comment section updated

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3490/files
  - new: https://git.openjdk.java.net/jdk/pull/3490/files/b990d100..09fe0dc5

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3490=08
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3490=07-08

  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 [v6]

[Sibabrata Sahoo]

On Wed, 21 Apr 2021 00:16:59 GMT, Valerie Peng  wrote:

>> Sibabrata Sahoo has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - Update CompareKeys.java
>>  - Update CompareKeys.java
>
> test/jdk/javax/crypto/KeyGenerator/CompareKeys.java line 85:
> 
>> 83: || origKey.hashCode() == copyKey.hashCode())
>> 84: && !Arrays.equals(origKey.getEncoded(), 
>> copyKey.getEncoded())
>> 85: && !origKey.getFormat().equals(copyKey.getFormat())) {
> 
> So, all of these 3 checks have to fail in order to be considered key 
> inequality? Could you spell out clearly what is expected here? I am not sure 
> if this compound condition is correct. As it is now, the copy must have 
> different format(2nd condition) AND different encoding(3rd condition) AND 
> (not equals AND not same hash code)(1st condition), in order to trigger the 
> RuntimeException.

I have updated the condition to check for equality, hashCode, Format, Encoded 
bytes of original key against another copy of same. As the keys are same and 
the copy of key is a replica of original key, i am expecting all condition to 
match in same time. If there is any condition check fails then it will be 
treated as a mismatch. Please correct me if i am wrong here.

-

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