Re: JEP 411: Documentation on the new way to establish TLS connections

2021-06-03 Thread Peter Firmstone

Don't bother replying.

I found that it is actually on the TODO list.

https://bugs.openjdk.java.net/browse/JDK-8266592

I've had enough now anyway, there is no fixing this mess.

Sayonara.


On 4/06/2021 2:22 pm, Peter Firmstone wrote:
Could someone please advise the recommended way now to preserve the 
Subject in Executors to establish a TLS connection?


I am unable to find the documentation.

We use Executors and we preserve the calling Subject across them, to 
use for authentication our TLS endpoints.


This is now deprecated in Java 17, because it uses AccessController 
and AccessControlContext  methods.


I would like to do this in a way that's not deprecated?

Just wondering if anyone has any suggestions?

Ron mentioned on Reddit this morning that there are no new API's being 
developed.


https://bugs.openjdk.java.net/browse/JDK-8267108

Is the assumption that the JDK will be a single user process, so the 
subject just needs to be stored in a Static variable and accessed from 
there instead?


Just wondering what the use case scenario is?

This really sux for us, because we authenticate TLS connections, then 
we run the users with their calling Subjects.


Thank you.

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-jeri/src/main/java/net/jini/jeri/ssl/FilterX509TrustManager.java 

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-jeri/src/main/java/net/jini/jeri/ssl/SubjectCredentials.java 



I'm kinda getting the feeling that I'm no longer welcome here.

I recognize that I'm pushing back, and people don't like that, however 
I'm doing so because I am impacted by the recent decision, I can 
assure you I have no personal grudges against anyone.


I'm not looking for assurances that that isn't the case, I just want 
some guidance, I think our whole code base and how we use Java, just 
bit the dust.


--
Regards,
  
Peter.




JEP 411: Documentation on the new way to establish TLS connections

2021-06-03 Thread Peter Firmstone
Could someone please advise the recommended way now to preserve the 
Subject in Executors to establish a TLS connection?


I am unable to find the documentation.

We use Executors and we preserve the calling Subject across them, to use 
for authentication our TLS endpoints.


This is now deprecated in Java 17, because it uses AccessController and 
AccessControlContext  methods.


I would like to do this in a way that's not deprecated?

Just wondering if anyone has any suggestions?

Ron mentioned on Reddit this morning that there are no new API's being 
developed.


https://bugs.openjdk.java.net/browse/JDK-8267108

Is the assumption that the JDK will be a single user process, so the 
subject just needs to be stored in a Static variable and accessed from 
there instead?


Just wondering what the use case scenario is?

This really sux for us, because we authenticate TLS connections, then we 
run the users with their calling Subjects.


Thank you.

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-jeri/src/main/java/net/jini/jeri/ssl/FilterX509TrustManager.java 

https://github.com/pfirmstone/JGDMS/blob/trunk/JGDMS/jgdms-jeri/src/main/java/net/jini/jeri/ssl/SubjectCredentials.java 



I'm kinda getting the feeling that I'm no longer welcome here.

I recognize that I'm pushing back, and people don't like that, however 
I'm doing so because I am impacted by the recent decision, I can assure 
you I have no personal grudges against anyone.


I'm not looking for assurances that that isn't the case, I just want 
some guidance, I think our whole code base and how we use Java, just bit 
the dust.


--
Regards,
 
Peter.




Re: RFR: 8255557: Decouple GCM from CipherCore [v4]

2021-06-03 Thread Anthony Scarpino
On Thu, 3 Jun 2021 22:30:38 GMT, Valerie Peng  wrote:

>> A engine is a one time use, so setting originalOut to null isn't necessary.  
>> engineDoFinal() sets engine = null.
>
> engine is one time use per encryption/decryption. But 'originalOut' is for 
> overlap detection/protection which may be used multiple times during 
> multi-part encryption/decrypion. For each overlapDetection()/restoreOut() 
> pair, the 'originalOut' value should be cleared, otherwise there may be cases 
> where the old value of 'originalOut' gets used?

Ok. I see what you are saying.  I had not consider a situation where an update 
buffer overlapped and doFinal did not.  I'll set originalDst and originalOut to 
null on their restore methods.

-

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


Re: RFR: 8255557: Decouple GCM from CipherCore [v8]

2021-06-03 Thread Anthony Scarpino
On Thu, 3 Jun 2021 22:07:34 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - missed resultLen and undo decrypt heap hasarray check
>>  - code review comments
>>  - fix
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
> line 776:
> 
>> 774: if (dst != null) {
>> 775: dst.put(block, 0, len);
>> 776: }
> 
> Can this be "resultLen += op.doFinal(block, 0, len, dst)"?

doFinal doesn't have a (byte[], int, int, ByteBuffer) method.  While that's not 
a bad idea to have one, it would be a fair bit of code to do it because it's 
part of the GCM interface and I'd have to write methods for GCTRGHASH, GCTR, 
and GHASH.  I think that's  too much just for this one code segment that isn't 
broken.

-

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


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v4]

2021-06-03 Thread Mandy Chung
On Thu, 3 Jun 2021 22:27:16 GMT, Bradford Wetmore  wrote:

>> The JceSecurityManager is currently a subclass of 
>> java.security.SecurityManager.  Now that JEP 411 has been integrated, this 
>> class should be updated to no longer subclass SecurityManager.
>> 
>> The only reason for using SecurityManager to easily get the Class Context 
>> (call stack), but we can achieve the same effect by using the JDK 9 API 
>> java.lang.StackWalkeer.  None of the other SecurityManager API are used.
>> 
>> I have run mach5 tier1/tier2 plus --test 
>> jck:api/java_security,jck:api/javax_crypto,jck:api/javax_net,jck:api/javax_security,jck:api/org_ietf,jck:api/javax_xml/crypto
>>  with all green.
>
> Bradford Wetmore has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 14 commits:
> 
>  - More Codereview Comments
>  - Merge branch 'master' into JDK-8267485
>  - Minor typo
>  - Reduced SuppressWarnings scope
>  - Codereview Comments #2
>  - Merge branch 'master' into JDK-8267485
>  - Address codereview comments
>  - Merge branch 'master' into JDK-8267485
>  - Merge branch 'master' into JDK-8267485
>  - Merge branch 'master' into JDK-8267485
>  - ... and 4 more: 
> https://git.openjdk.java.net/jdk/compare/9f05c411...a441778b

src/java.base/share/classes/javax/crypto/JceSecurityManager.java line 109:

> 107: @SuppressWarnings("removal")
> 108: List stack =
> 109: AccessController.doPrivileged(pa).walk(Stream::toList);

You can replace line 108-125 with something like this: 

StackWalker walker = AccessController.doPrivileged(pa);
Optional callerCodeBase = walker.walk(s -> {
s.map(f -> JceSecurity.getCodeBase(f.getDeclaringClass()))
  .findFirst();
});

src/java.base/share/classes/javax/crypto/JceSecurityManager.java line 245:

> 243: @SuppressWarnings("removal")
> 244: Optional stackFrame = 
> AccessController.doPrivileged(pa)
> 245: .walk((s) -> s.skip(2).findFirst());

You can use the same `StackWalker` instance in multiple places. 

`StackWalker::getCallerClass` is the API to get the caller class.   You want to 
get the caller of the subclass of `Cipher` in this case.So `Cipher` 
constructor will call `walker.getCallerClass()` and then pass it to 
`isCallerTrusted` which will take an additional caller class parameter for 
validation.

-

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


Re: RFR: 8255557: Decouple GCM from CipherCore [v8]

2021-06-03 Thread Valerie Peng
On Thu, 3 Jun 2021 16:04:19 GMT, Anthony Scarpino  wrote:

>> Hi,
>> 
>> I need a review of this rather large change to GCM.  GCM will no longer use 
>> CipherCore, and AESCrypt  to handle it's buffers and other objects.  It is 
>> also a major code redesign limits the amount of data copies and make some 
>> performance-based decisions.
>> 
>> Thanks
>> 
>> Tony
>
> Anthony Scarpino has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - missed resultLen and undo decrypt heap hasarray check
>  - code review comments
>  - fix

Update changes look good. Just a nit and the issue with originalOut remains.
Thanks,
Valerie

-

Marked as reviewed by valeriep (Reviewer).

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


Re: RFR: 8255557: Decouple GCM from CipherCore [v4]

2021-06-03 Thread Valerie Peng
On Wed, 2 Jun 2021 03:18:49 GMT, Anthony Scarpino  wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
>> line 942:
>> 
>>> 940: 
>>> 941: System.arraycopy(out, originalOutOfs, originalOut, 
>>> originalOutOfs,
>>> 942: len);
>> 
>> Don't you need to do `originalOut = null;` after copying the bytes over? 
>> Otherwise, if you have two overlapping calls with the same engine, the 2nd 
>> restoreOut(...) call may lead to data corruption, i.e. it will duplicate the 
>> output bytes to the original output buffer (in the 1st overlapping call).
>> Same applies for the ByteBuffer case, i.e. restoreDst(...).
>> If time permits, please add a regression test to cover this.
>
> A engine is a one time use, so setting originalOut to null isn't necessary.  
> engineDoFinal() sets engine = null.

engine is one time use per encryption/decryption. But 'originalOut' is for 
overlap detection/protection which may be used multiple times during multi-part 
encryption/decrypion. For each overlapDetection()/restoreOut() pair, the 
'originalOut' value should be cleared, otherwise there may be cases where the 
old value of 'originalOut' gets used?

-

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


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v4]

2021-06-03 Thread Bradford Wetmore
> The JceSecurityManager is currently a subclass of 
> java.security.SecurityManager.  Now that JEP 411 has been integrated, this 
> class should be updated to no longer subclass SecurityManager.
> 
> The only reason for using SecurityManager to easily get the Class Context 
> (call stack), but we can achieve the same effect by using the JDK 9 API 
> java.lang.StackWalkeer.  None of the other SecurityManager API are used.
> 
> I have run mach5 tier1/tier2 plus --test 
> jck:api/java_security,jck:api/javax_crypto,jck:api/javax_net,jck:api/javax_security,jck:api/org_ietf,jck:api/javax_xml/crypto
>  with all green.

Bradford Wetmore has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 14 commits:

 - More Codereview Comments
 - Merge branch 'master' into JDK-8267485
 - Minor typo
 - Reduced SuppressWarnings scope
 - Codereview Comments #2
 - Merge branch 'master' into JDK-8267485
 - Address codereview comments
 - Merge branch 'master' into JDK-8267485
 - Merge branch 'master' into JDK-8267485
 - Merge branch 'master' into JDK-8267485
 - ... and 4 more: https://git.openjdk.java.net/jdk/compare/9f05c411...a441778b

-

Changes: https://git.openjdk.java.net/jdk/pull/4150/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4150=03
  Stats: 30 lines in 1 file changed: 15 ins; 5 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4150.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4150/head:pull/4150

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


Re: RFR: 8255557: Decouple GCM from CipherCore [v8]

2021-06-03 Thread Valerie Peng
On Thu, 3 Jun 2021 16:04:19 GMT, Anthony Scarpino  wrote:

>> Hi,
>> 
>> I need a review of this rather large change to GCM.  GCM will no longer use 
>> CipherCore, and AESCrypt  to handle it's buffers and other objects.  It is 
>> also a major code redesign limits the amount of data copies and make some 
>> performance-based decisions.
>> 
>> Thanks
>> 
>> Tony
>
> Anthony Scarpino has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - missed resultLen and undo decrypt heap hasarray check
>  - code review comments
>  - fix

src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 
776:

> 774: if (dst != null) {
> 775: dst.put(block, 0, len);
> 776: }

Can this be "resultLen += op.doFinal(block, 0, len, dst)"?

-

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


Re: RFR: 8255557: Decouple GCM from CipherCore [v8]

2021-06-03 Thread Valerie Peng
On Thu, 3 Jun 2021 20:45:41 GMT, Anthony Scarpino  wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
>> line 84:
>> 
>>> 82: private static final int MAX_BUF_SIZE = Integer.MAX_VALUE;
>>> 83: // data size when buffer is divided up to aid in intrinsics
>>> 84: private static final int TRIGGERLEN = 4096;  // 64k
>> 
>> nit: comment should be 4k?
>
> Ack.. I changed it to test something and forgot to change it back.. I'll put 
> it back to 64k with the merge-conflict

Sure.

-

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


Re: Authorization Layer post JEP 411

2021-06-03 Thread Peter Firmstone

Sean,

Also moving forward we currently preserve AccessControlContext across 
threads, and we do this to establish TLS connections for call backs.


Will there be a new way to preserve the calling Subject across threads, 
so we can perform callbacks over TLS?


Regards,

--
Regards,
 
Peter Firmstone


On 4/06/2021 7:39 am, Peter Firmstone wrote:

Hi Sean,

Developers are still going to need single points of control, where we 
can attach our agents to Java's API's.   We can't be playing a game of 
whack a mole trying to lock down the JDK.


It's fair enough that OpenJDK no longer wishes to maintain 
SecurityManager, however there are those of us who have to implement 
authorization layers and access controls and we don't have the luxury 
of choice.


So we've established that we need to use Agents and StackWalker now to 
implement our authorization layer.


It will be some years before we are able to keep up to date with Java 
releases again, but now we need to focus on how to achieve that.


Regarding your questions, the performance problems, were related to 
Java's FilePolicy implementation, I solved those issues by replacing 
it, but you're already aware of that, I was highlighting the struggle 
that developers have with Java security, but also that JAAS is a 
common foundation for user authorisation, so I hope that it will be 
improved, rather than removed.  I of course also use JAAS to establish 
TLS connections.


If there's anything else OpenJDK is thinking about, thinking about 
removing, then we need to know, so we don't use them in our new 
authorization layer.




Re: RFR: 8255557: Decouple GCM from CipherCore [v4]

2021-06-03 Thread Valerie Peng
On Wed, 2 Jun 2021 20:23:38 GMT, Anthony Scarpino  wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
>> line 1610:
>> 
>>> 1608: // update the input parameters for what was taken 
>>> out of 'in'
>>> 1609: inOfs += inUsed;
>>> 1610: inLen -= inUsed;
>> 
>> This merge block code won't be needed if inLen == 0, i.e. can just assign in 
>> to be buffer, inOfs to 0, and inLen to bufRemaining.
>
> You are correct, but it's not that simple to handle this case without adding 
> more if()'s which I've found can slow down overall performance.  I'm hesitant 
> change this code for this case

Ok, perhaps most often than not inLen != 0, so not worthwhile to check this.

-

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


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Weijun Wang
On Thu, 3 Jun 2021 21:50:32 GMT, Bradford Wetmore  wrote:

>> The latter is probably better so you don't have to move the code; @wangweij 
>> used this technique quite a bit for JEP 411 refactoring, see 
>> https://github.com/openjdk/jdk/commit/508cec7535cd0ad015d566389bc9e5f53ce4103b
>
> @seanjmullan , that's what I ended up doing.  I'll have a new revision out as 
> soon as the mach5 finishes.

In this case, I name the variable `tmp`. There are cases where the action is a 
`PrivlegedAction` and you still have to create a variable for the return 
value, and then I call it `dummy`.

-

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


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Bradford Wetmore
On Thu, 3 Jun 2021 21:41:02 GMT, Sean Mullan  wrote:

>> For the static initializer that needs updating:  I could move the code out 
>> of the initializer up to the declaration, or I could create a dummy 
>> declaration and then assign to INSTANCE.
>
> The latter is probably better so you don't have to move the code; @wangweij 
> used this technique quite a bit for JEP 411 refactoring, see 
> https://github.com/openjdk/jdk/commit/508cec7535cd0ad015d566389bc9e5f53ce4103b

@seanjmullan , that's what I ended up doing.  I'll have a new revision out as 
soon as the mach5 finishes.

-

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


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Sean Mullan
On Thu, 3 Jun 2021 20:49:27 GMT, Bradford Wetmore  wrote:

>> But if you follow my suggestion you can simply apply it to this line:
>> 
>> 
>> @SuppressWarnings("removal")
>> final List stack = 
>> AccessController.doPrivileged(pa).walk(Stream::toList);
>
> For the static initializer that needs updating:  I could move the code out of 
> the initializer up to the declaration, or I could create a dummy declaration 
> and then assign to INSTANCE.

The latter is probably better so you don't have to move the code; @wangweij 
used this technique quite a bit for JEP 411 refactoring, see 
https://github.com/openjdk/jdk/commit/508cec7535cd0ad015d566389bc9e5f53ce4103b

-

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


Re: Authorization Layer post JEP 411

2021-06-03 Thread Peter Firmstone

Hi Sean,

Developers are still going to need single points of control, where we 
can attach our agents to Java's API's.   We can't be playing a game of 
whack a mole trying to lock down the JDK.


It's fair enough that OpenJDK no longer wishes to maintain 
SecurityManager, however there are those of us who have to implement 
authorization layers and access controls and we don't have the luxury of 
choice.


So we've established that we need to use Agents and StackWalker now to 
implement our authorization layer.


It will be some years before we are able to keep up to date with Java 
releases again, but now we need to focus on how to achieve that.


Regarding your questions, the performance problems, were related to 
Java's FilePolicy implementation, I solved those issues by replacing it, 
but you're already aware of that, I was highlighting the struggle that 
developers have with Java security, but also that JAAS is a common 
foundation for user authorisation, so I hope that it will be improved, 
rather than removed.  I of course also use JAAS to establish TLS 
connections.


If there's anything else OpenJDK is thinking about, thinking about 
removing, then we need to know, so we don't use them in our new 
authorization layer.


--
Regards,
 
Peter Firmstone


On 4/06/2021 1:02 am, Sean Mullan wrote:



On 6/2/21 7:41 PM, Peter Firmstone wrote:
AccessController and AccessControlContext allow backward compatiblity 
for JAAS.   JAAS whether we like it or not, is the default 
authorisation layer framework.


http://word-bits.flurg.com/jaas-is-terrible-and-there-is-no-escape-from-it/ 





I'm not sure why you referenced this blog which is actually advocating 
that JAAS has *less* dependency on Security Manager APIs such as 
AccessControlContext, whereas you seem to be advocating the opposite.


In any case, I believe the first two issues in this blog will largely 
be addressed by the deprecation of the Security Manager and the JAAS 
related RFEs that we have filed as follow-on work to JEP 411 to remove 
the dependencies on the SM APIs:


https://bugs.openjdk.java.net/browse/JDK-8266592
https://bugs.openjdk.java.net/browse/JDK-8267108

As for the 3rd issue in the blog, it is not related to the Security 
Manager, but I would need more time to understand the issues that were 
described.


Also the blog was written by David Lloyd who has been participating in 
these discussions, so he may want to say more about it.


--Sean




Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Bradford Wetmore
On Thu, 3 Jun 2021 17:58:45 GMT, Daniel Fuchs  wrote:

>> Unfortunately, we are still calling AccessController, thus the annotation 
>> needs to remain.
>
> But if you follow my suggestion you can simply apply it to this line:
> 
> 
> @SuppressWarnings("removal")
> final List stack = 
> AccessController.doPrivileged(pa).walk(Stream::toList);

For the static initializer that needs updating:  I could move the code out of 
the initializer up to the declaration, or I could create a dummy declaration 
and then assign to INSTANCE.

-

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


Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v7]

2021-06-03 Thread Maurizio Cimadamore
> This patch overhauls the library loading mechanism used by the Foreign Linker 
> API. We realized that, while handy, the *default* lookup abstraction 
> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
> 
> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
> concern with library loading, only symbol lookup. For this reason, two 
> factories are added:
> 
> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
> lookup symbols in libraries loaded by current loader
> * `CLinker::systemLookup` - a more stable replacement for the *default* 
> lookup, which looks for symbols in libc.so (or its equivalent in other 
> platforms). The contents of this lookup are unspecified.
> 
> Both factories are *restricted*, so they can only be called when 
> `--enable-native-access` is set.

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

  Address review comments on shim lib makefile

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4316/files
  - new: https://git.openjdk.java.net/jdk/pull/4316/files/2545e2b6..23d66faf

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4316=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4316=05-06

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

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


Re: RFR: 8255557: Decouple GCM from CipherCore [v8]

2021-06-03 Thread Anthony Scarpino
On Thu, 3 Jun 2021 19:13:00 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with three 
>> additional commits since the last revision:
>> 
>>  - missed resultLen and undo decrypt heap hasarray check
>>  - code review comments
>>  - fix
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
> line 84:
> 
>> 82: private static final int MAX_BUF_SIZE = Integer.MAX_VALUE;
>> 83: // data size when buffer is divided up to aid in intrinsics
>> 84: private static final int TRIGGERLEN = 4096;  // 64k
> 
> nit: comment should be 4k?

Ack.. I changed it to test something and forgot to change it back.. I'll put it 
back to 64k with the merge-conflict

-

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


Re: RFR: 8255557: Decouple GCM from CipherCore [v8]

2021-06-03 Thread Valerie Peng
On Thu, 3 Jun 2021 16:04:19 GMT, Anthony Scarpino  wrote:

>> Hi,
>> 
>> I need a review of this rather large change to GCM.  GCM will no longer use 
>> CipherCore, and AESCrypt  to handle it's buffers and other objects.  It is 
>> also a major code redesign limits the amount of data copies and make some 
>> performance-based decisions.
>> 
>> Thanks
>> 
>> Tony
>
> Anthony Scarpino has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - missed resultLen and undo decrypt heap hasarray check
>  - code review comments
>  - fix

src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java line 
84:

> 82: private static final int MAX_BUF_SIZE = Integer.MAX_VALUE;
> 83: // data size when buffer is divided up to aid in intrinsics
> 84: private static final int TRIGGERLEN = 4096;  // 64k

nit: comment should be 4k?

-

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


Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v6]

2021-06-03 Thread Erik Joelsson
On Thu, 3 Jun 2021 16:43:51 GMT, Maurizio Cimadamore  
wrote:

>> This patch overhauls the library loading mechanism used by the Foreign 
>> Linker API. We realized that, while handy, the *default* lookup abstraction 
>> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
>> 
>> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
>> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
>> concern with library loading, only symbol lookup. For this reason, two 
>> factories are added:
>> 
>> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
>> lookup symbols in libraries loaded by current loader
>> * `CLinker::systemLookup` - a more stable replacement for the *default* 
>> lookup, which looks for symbols in libc.so (or its equivalent in other 
>> platforms). The contents of this lookup are unspecified.
>> 
>> Both factories are *restricted*, so they can only be called when 
>> `--enable-native-access` is set.
>
> Maurizio Cimadamore has updated the pull request with a new target base due 
> to a merge or a rebase. The pull request now contains 16 commits:
> 
>  - Merge branch 'master' into symbolLookup
>  - Forgot to add makefile for building shim library
>  - Address review comments
>  - Update test/jdk/java/foreign/TestIntrinsics.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/valist/VaListTest.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/TestVarArgs.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/TestUpcall.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/TestIllegalLink.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/TestSymbolLookup.java
>
>Co-authored-by: Paul Sandoz 
>  - Update test/jdk/java/foreign/TestDowncall.java
>
>Co-authored-by: Paul Sandoz 
>  - ... and 6 more: 
> https://git.openjdk.java.net/jdk/compare/52d8215a...2545e2b6

Looks pretty good, just a few comments.

make/modules/jdk.incubator.foreign/Lib.gmk line 28:

> 26: include LibCommon.gmk
> 27: 
> 28: ifeq ($(call isTargetOs, linux), true)

Please indent everything inside the ifeq-block 2 spaces. (See 
http://openjdk.java.net/groups/build/doc/code-conventions.html)

make/modules/jdk.incubator.foreign/Lib.gmk line 34:

> 32: CFLAGS := $(CFLAGS_JDKLIB), \
> 33: CXXFLAGS := $(CXXFLAGS_JDKLIB), \
> 34: LDFLAGS := -Wl$(COMMA)--no-as-needed -lc -lm -ldl $(LDFLAGS_JDKLIB) 
> $(call SET_SHARED_LIBRARY_ORIGIN), \

Unless you link with any other library in the JDK (typically libjava and/or 
libjvm), I don't think there is a need for adding SET_SHARED_LIBRARY_ORIGIN.

Please put all the -l* flags in LIBS rather than LDFLAGS.

I also recommend putting any additional flags after the general LDFLAGS_JDKLIB. 
That way you are guaranteed that your flag takes precedence over anything that 
may be added to LDFLAGS_JDKLIB in the future.

-

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


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Daniel Fuchs
On Thu, 3 Jun 2021 17:44:22 GMT, Bradford Wetmore  wrote:

>> src/java.base/share/classes/javax/crypto/JceSecurityManager.java line 111:
>> 
>>> 109: Option.RETAIN_CLASS_REFERENCE)
>>> 110: .walk((s) -> 
>>> s.collect(Collectors.toList(;
>>> 111: 
>> 
>> Note: StackWalker is a stateless capability object. It's not the walk() 
>> method that requires a permission, but the creation of the StackWalker 
>> itself (hence my suggestion to create it in the constructor, or in a static 
>> initializer). If you walk the stack from within a doPrivileged call then the 
>> doPrivileged frame will appear in the returned `List`; this may 
>> (or may not) be OK - depending on the logic that processes the stack.
>> 
>> You could consider simplifying:
>> 
>> 
>> PrivilegedAction pa = () -> 
>> StackWalker.getInstance(Option.RETAIN_CLASS_REFERENCE);
>> final List stack = 
>> AccessController.doPrivileged(pa).walk(Stream::toList);
>
> Thanks.  I was going to step through this code more thoroughly today, 
> hopefully I would have caught that.  
> 
> This code is only needed in certain deployment and Cipher creation 
> situations, so would rather not create a static CodeWalker that is not 
> normally used.

Fair enough.

-

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


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Daniel Fuchs
On Thu, 3 Jun 2021 17:44:42 GMT, Bradford Wetmore  wrote:

>> src/java.base/share/classes/javax/crypto/JceSecurityManager.java line 50:
>> 
>>> 48:  * @since 1.4
>>> 49:  */
>>> 50: @SuppressWarnings("removal")
>> 
>> You should remove this annotation now that the dependency on SecurityManager 
>> has been removed.
>
> Unfortunately, we are still calling AccessController, thus the annotation 
> needs to remain.

But if you follow my suggestion you can simply apply it to this line:


@SuppressWarnings("removal")
final List stack = 
AccessController.doPrivileged(pa).walk(Stream::toList);

-

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


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Bradford Wetmore
On Thu, 3 Jun 2021 08:27:14 GMT, Daniel Fuchs  wrote:

>> Bradford Wetmore has updated the pull request with a new target base due to 
>> a merge or a rebase. The pull request now contains eight commits:
>> 
>>  - Address codereview comments
>>  - Merge branch 'master' into JDK-8267485
>>  - Merge branch 'master' into JDK-8267485
>>  - Merge branch 'master' into JDK-8267485
>>  - Replace missing annotation
>>  - Merge branch 'master' into JDK-8267485
>>  - Updated copyright date.
>>  - 8267485: Remove the dependency on SecurityManager in 
>> JceSecurityManager.java
>
> src/java.base/share/classes/javax/crypto/JceSecurityManager.java line 111:
> 
>> 109: Option.RETAIN_CLASS_REFERENCE)
>> 110: .walk((s) -> 
>> s.collect(Collectors.toList(;
>> 111: 
> 
> Note: StackWalker is a stateless capability object. It's not the walk() 
> method that requires a permission, but the creation of the StackWalker itself 
> (hence my suggestion to create it in the constructor, or in a static 
> initializer). If you walk the stack from within a doPrivileged call then the 
> doPrivileged frame will appear in the returned `List`; this may 
> (or may not) be OK - depending on the logic that processes the stack.
> 
> You could consider simplifying:
> 
> 
> PrivilegedAction pa = () -> 
> StackWalker.getInstance(Option.RETAIN_CLASS_REFERENCE);
> final List stack = 
> AccessController.doPrivileged(pa).walk(Stream::toList);

Thanks.  I was going to step through this code more thoroughly today, hopefully 
I would have caught that.  

This code is only needed in certain deployment and Cipher creation situations, 
so would rather not create a static CodeWalker that is not normally used.

-

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


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Bradford Wetmore
On Thu, 3 Jun 2021 14:20:37 GMT, Sean Mullan  wrote:

>> Bradford Wetmore has updated the pull request with a new target base due to 
>> a merge or a rebase. The pull request now contains eight commits:
>> 
>>  - Address codereview comments
>>  - Merge branch 'master' into JDK-8267485
>>  - Merge branch 'master' into JDK-8267485
>>  - Merge branch 'master' into JDK-8267485
>>  - Replace missing annotation
>>  - Merge branch 'master' into JDK-8267485
>>  - Updated copyright date.
>>  - 8267485: Remove the dependency on SecurityManager in 
>> JceSecurityManager.java
>
> src/java.base/share/classes/javax/crypto/JceSecurityManager.java line 50:
> 
>> 48:  * @since 1.4
>> 49:  */
>> 50: @SuppressWarnings("removal")
> 
> You should remove this annotation now that the dependency on SecurityManager 
> has been removed.

Unfortunately, we are still calling AccessController, thus the annotation needs 
to remain.

-

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


Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v6]

2021-06-03 Thread Maurizio Cimadamore
> This patch overhauls the library loading mechanism used by the Foreign Linker 
> API. We realized that, while handy, the *default* lookup abstraction 
> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
> 
> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
> concern with library loading, only symbol lookup. For this reason, two 
> factories are added:
> 
> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
> lookup symbols in libraries loaded by current loader
> * `CLinker::systemLookup` - a more stable replacement for the *default* 
> lookup, which looks for symbols in libc.so (or its equivalent in other 
> platforms). The contents of this lookup are unspecified.
> 
> Both factories are *restricted*, so they can only be called when 
> `--enable-native-access` is set.

Maurizio Cimadamore has updated the pull request with a new target base due to 
a merge or a rebase. The pull request now contains 16 commits:

 - Merge branch 'master' into symbolLookup
 - Forgot to add makefile for building shim library
 - Address review comments
 - Update test/jdk/java/foreign/TestIntrinsics.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/valist/VaListTest.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/TestVarArgs.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/TestUpcall.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/TestIllegalLink.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/TestSymbolLookup.java
   
   Co-authored-by: Paul Sandoz 
 - Update test/jdk/java/foreign/TestDowncall.java
   
   Co-authored-by: Paul Sandoz 
 - ... and 6 more: https://git.openjdk.java.net/jdk/compare/52d8215a...2545e2b6

-

Changes: https://git.openjdk.java.net/jdk/pull/4316/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4316=05
  Stats: 1351 lines in 47 files changed: 626 ins; 621 del; 104 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4316.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4316/head:pull/4316

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


Re: RFR: 8268129: LibraryLookup::ofDefault leaks symbols from loaded libraries [v5]

2021-06-03 Thread Maurizio Cimadamore
> This patch overhauls the library loading mechanism used by the Foreign Linker 
> API. We realized that, while handy, the *default* lookup abstraction 
> (`LibraryLookup::ofDefault`) was behaving inconsistentlt across platforms.
> 
> This patch replaces `LibraryLookup` with a simpler `SymbolLookup` 
> abstraction, a functional interface. Crucially, `SymbolLookup` does not 
> concern with library loading, only symbol lookup. For this reason, two 
> factories are added:
> 
> * `SymbolLookup::loaderLookup` - which obtains a lookup that can be used to 
> lookup symbols in libraries loaded by current loader
> * `CLinker::systemLookup` - a more stable replacement for the *default* 
> lookup, which looks for symbols in libc.so (or its equivalent in other 
> platforms). The contents of this lookup are unspecified.
> 
> Both factories are *restricted*, so they can only be called when 
> `--enable-native-access` is set.

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

  Forgot to add makefile for building shim library

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4316/files
  - new: https://git.openjdk.java.net/jdk/pull/4316/files/7be87eae..6480332c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4316=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4316=03-04

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

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


Integrated: 8262409: sun/security/ssl/SSLSocketImpl/SSLSocketImplThrowsWrongExceptions. SSL test failures caused by java failed with "Server reported the wrong exception"

2021-06-03 Thread Fernando Guallini
On Wed, 2 Jun 2021 14:03:57 GMT, Fernando Guallini  
wrote:

> The test `SSLSocketImplThrowsWrongExceptions` is failing intermittently after 
> the change: [JDK-8259662: Don't wrap SocketExceptions into SSLExceptions in 
> SSLSocketImpl](https://bugs.openjdk.java.net/browse/JDK-8259662)
> 
> Since SocketExceptions are not wrapped into SSLException, also need to be 
> caught. Other tests that were expecting a SSLException to be thrown under 
> certain condition were updated with a similar fix in the change previously 
> mentioned.
> 
> In addition, thread.sleep was replaced with a CountDownLatch for 
> client/server synchronization

This pull request has now been integrated.

Changeset: 3aa7062c
Author:Fernando Guallini 
Committer: Rajan Halade 
URL:   
https://git.openjdk.java.net/jdk/commit/3aa7062c3dd41e06df67b46473ee2ef5a9671cf9
Stats: 14 lines in 2 files changed: 0 ins; 3 del; 11 mod

8262409: sun/security/ssl/SSLSocketImpl/SSLSocketImplThrowsWrongExceptions. SSL 
test failures caused by java failed with "Server reported the wrong exception"

Reviewed-by: rhalade, xuelei

-

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


Re: RFR: 8255557: Decouple GCM from CipherCore [v8]

2021-06-03 Thread Anthony Scarpino
> Hi,
> 
> I need a review of this rather large change to GCM.  GCM will no longer use 
> CipherCore, and AESCrypt  to handle it's buffers and other objects.  It is 
> also a major code redesign limits the amount of data copies and make some 
> performance-based decisions.
> 
> Thanks
> 
> Tony

Anthony Scarpino has updated the pull request incrementally with three 
additional commits since the last revision:

 - missed resultLen and undo decrypt heap hasarray check
 - code review comments
 - fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4072/files
  - new: https://git.openjdk.java.net/jdk/pull/4072/files/d230e665..c5e19250

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

  Stats: 199 lines in 3 files changed: 20 ins; 123 del; 56 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4072.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4072/head:pull/4072

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


Re: Authorization Layer post JEP 411

2021-06-03 Thread Sean Mullan




On 6/2/21 7:41 PM, Peter Firmstone wrote:
AccessController and AccessControlContext allow backward compatiblity 
for JAAS.   JAAS whether we like it or not, is the default authorisation 
layer framework.


http://word-bits.flurg.com/jaas-is-terrible-and-there-is-no-escape-from-it/



I'm not sure why you referenced this blog which is actually advocating 
that JAAS has *less* dependency on Security Manager APIs such as 
AccessControlContext, whereas you seem to be advocating the opposite.


In any case, I believe the first two issues in this blog will largely be 
addressed by the deprecation of the Security Manager and the JAAS 
related RFEs that we have filed as follow-on work to JEP 411 to remove 
the dependencies on the SM APIs:


https://bugs.openjdk.java.net/browse/JDK-8266592
https://bugs.openjdk.java.net/browse/JDK-8267108

As for the 3rd issue in the blog, it is not related to the Security 
Manager, but I would need more time to understand the issues that were 
described.


Also the blog was written by David Lloyd who has been participating in 
these discussions, so he may want to say more about it.


--Sean


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Sean Mullan
On Wed, 2 Jun 2021 23:15:46 GMT, Bradford Wetmore  wrote:

>> The JceSecurityManager is currently a subclass of 
>> java.security.SecurityManager.  Now that JEP 411 has been integrated, this 
>> class should be updated to no longer subclass SecurityManager.
>> 
>> The only reason for using SecurityManager to easily get the Class Context 
>> (call stack), but we can achieve the same effect by using the JDK 9 API 
>> java.lang.StackWalkeer.  None of the other SecurityManager API are used.
>> 
>> I have run mach5 tier1/tier2 plus --test 
>> jck:api/java_security,jck:api/javax_crypto,jck:api/javax_net,jck:api/javax_security,jck:api/org_ietf,jck:api/javax_xml/crypto
>>  with all green.
>
> Bradford Wetmore has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - Address codereview comments
>  - Merge branch 'master' into JDK-8267485
>  - Merge branch 'master' into JDK-8267485
>  - Merge branch 'master' into JDK-8267485
>  - Replace missing annotation
>  - Merge branch 'master' into JDK-8267485
>  - Updated copyright date.
>  - 8267485: Remove the dependency on SecurityManager in 
> JceSecurityManager.java

I would remove line 44 from the comments as it is no longer applicable:

* Note that this security manager is never installed, only instantiated.

Also, you may want to rename this class to something other than 
JceSecurityManager to avoid future confusion, maybe "JcePolicyManager".

-

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


Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Sean Mullan
On Wed, 2 Jun 2021 23:15:46 GMT, Bradford Wetmore  wrote:

>> The JceSecurityManager is currently a subclass of 
>> java.security.SecurityManager.  Now that JEP 411 has been integrated, this 
>> class should be updated to no longer subclass SecurityManager.
>> 
>> The only reason for using SecurityManager to easily get the Class Context 
>> (call stack), but we can achieve the same effect by using the JDK 9 API 
>> java.lang.StackWalkeer.  None of the other SecurityManager API are used.
>> 
>> I have run mach5 tier1/tier2 plus --test 
>> jck:api/java_security,jck:api/javax_crypto,jck:api/javax_net,jck:api/javax_security,jck:api/org_ietf,jck:api/javax_xml/crypto
>>  with all green.
>
> Bradford Wetmore has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - Address codereview comments
>  - Merge branch 'master' into JDK-8267485
>  - Merge branch 'master' into JDK-8267485
>  - Merge branch 'master' into JDK-8267485
>  - Replace missing annotation
>  - Merge branch 'master' into JDK-8267485
>  - Updated copyright date.
>  - 8267485: Remove the dependency on SecurityManager in 
> JceSecurityManager.java

src/java.base/share/classes/javax/crypto/JceSecurityManager.java line 50:

> 48:  * @since 1.4
> 49:  */
> 50: @SuppressWarnings("removal")

You should remove this annotation now that the dependency on SecurityManager 
has been removed.

-

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


Integrated: 8268167: MultipleLogins.java failure on macosx-aarch64

2021-06-03 Thread Sean Coffey
On Thu, 3 Jun 2021 11:10:10 GMT, Sean Coffey  wrote:

> MultipleLogins.java should skip test where NSS support is not present

This pull request has now been integrated.

Changeset: eb385c0d
Author:Sean Coffey 
URL:   
https://git.openjdk.java.net/jdk/commit/eb385c0de2026d6b184ce0c98ff421a4da95e1b1
Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod

8268167: MultipleLogins.java failure on macosx-aarch64

Reviewed-by: weijun

-

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


Re: RFR: 8268167: MultipleLogins.java failure on macosx-aarch64

2021-06-03 Thread Weijun Wang
On Thu, 3 Jun 2021 11:10:10 GMT, Sean Coffey  wrote:

> MultipleLogins.java should skip test where NSS support is not present

Marked as reviewed by weijun (Reviewer).

-

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


RFR: 8268167: MultipleLogins.java failure on macosx-aarch64

2021-06-03 Thread Sean Coffey
MultipleLogins.java should skip test where NSS support is not present

-

Commit messages:
 - 8268167: MultipleLogins.java failure on macosx-aarch64

Changes: https://git.openjdk.java.net/jdk/pull/4333/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4333=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8268167
  Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4333.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4333/head:pull/4333

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


Re: [External] : Authorization Layer post JEP 411

2021-06-03 Thread Peter Firmstone
Yes, I think so too.  However I will encourage developers to continue to 
take advantage of SM for improved security now, there's no need to rush 
to abandon it.


Maybe in future there will be better alternatives, but it's the best 
option for those who are security focused now.


With time no doubt hardening that will occur to the Java platform as 
OpenJDK responds to vulnerabilities, it will become the most secure 
option again, but I think that's a number of years away and I'd rather 
be conservative than get burned.


If SM deprecation doesn't impact your use case, then yes I would 
encourage you to upgrade, because that's the sensible thing to do.


I'll still test on later versions, but I won't be removing our 
authorization system until I'm satisfied there are sufficiently hardened 
alternative technologies available.


Thank you,

Peter.

On 3/06/2021 7:58 pm, Ron Pressler wrote:

It is certainly time to accept that JEP 411 has been accepted, and so that those
who use Security Manager will need to do some work to change their software.

The purpose of this and upcoming discussions is to find reasonable approaches
that might relieve some portion of the burden on those who use SM today while
not placing an undue (indirect) burden on those who do not.

— Ron


On 3 Jun 2021, at 10:43, Peter Firmstone  wrote:

Ok, thanks Ron,

I think we are confirming that Java, post version 17, will not meet the 
security needs our software.  It's time I accepted that and moved on.

Thanks for your time.

Have you seen my latest article on foojay?   Feel free to comment and let me 
know what you think.

https://urldefense.com/v3/__https://foojay.io/today/jep-411-what-it-means-for-javas-security-model/__;!!GqivPVa7Brio!MWpnS_ogZx24MskkZbSSrZ7ZbtCSyNeEswy1gegVSzGdDe4Qpmdy0ocIje9M4Wtv3A$
Cheers,

Peter.


On 3/06/2021 7:33 pm, Ron Pressler wrote:

On 3 Jun 2021, at 00:41, Peter Firmstone  wrote:


StackWalker doesn't work with compiled code, only bytecode.

If you’re referring to GraalVM’s Native Image, I don’t know about that problem 
and
there does seem to be a relevant patch 
(https://urldefense.com/v3/__https://github.com/oracle/graal/pull/734__;!!GqivPVa7Brio!MWpnS_ogZx24MskkZbSSrZ7ZbtCSyNeEswy1gegVSzGdDe4Qpmdy0ocIje-DV8ldZw$
 ), but
Native Image is a separate project from OpenJDK.


AccessController and AccessControlContext allow backward compatiblity for JAAS. 
  JAAS whether we like it or not, is the default authorisation layer framework.

https://urldefense.com/v3/__http://word-bits.flurg.com/jaas-is-terrible-and-there-is-no-escape-from-it/__;!!GqivPVa7Brio!MWpnS_ogZx24MskkZbSSrZ7ZbtCSyNeEswy1gegVSzGdDe4Qpmdy0ocIje-R7C-0Hg$

I don’t know how much a seven-year-old article, that predates Java 8 supports 
the use
of the present tense, but in any event, the JEP says that JAAS will be 
preserved.


With SecurityManager gone, people will no longer assume it has sole responsible 
for Security

People don’t assume that now, as secure software doesn’t employ it even today. 
People do,
however, assume that the mechanism, if used, is robust enough to be used for 
security
purposes.


OpenJDK devs won't carry a significant burden for it's maintenance.

While the number of places where the JDK *implements* some “protected 
operation”, like
opening a file or writing to a socket, is somewhat bounded — and so keeping 
some hooks
in those places *might* be reasonable — the number of places that *use* those 
operations
is not. Maintaining doPrivileged in that unbounded set of places is not an 
insignificant
burden.



Any security issues will be the responsibility of third party implementations, 
like mine.
The JDK won't provide an implementation, just the framework.

But the correct use of doPrivileged, if you’re proposing that it’s kept, must 
still be
tested against *some* implementation, and OpenJDK would still need to fix bugs 
related
to it.


Those of us using the Principle of Least Privilege can continue to do so

Perhaps you believe that the only software in the world that applies Least 
Privilege is
Java software that employs the Security Manager, but that is not how most 
people, including
the person who had framed it two decades prior to the invention of the Security 
Manager,
understand the principle.

The original statement of the principle was: "Every program and every 
privileged user of
the system should operate using the least amount of privilege necessary to 
complete the
job.” 
(https://urldefense.com/v3/__https://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.226.3939__;!!GqivPVa7Brio!MWpnS_ogZx24MskkZbSSrZ7ZbtCSyNeEswy1gegVSzGdDe4Qpmdy0ocIje-xd8krsA$
 )

You are talking about applying the principle at a granularity of code units 
that are
smaller than a program. It’s fine to believe that is worthwhile, but the 
principle
certainly doesn’t require that every effort be expended to afford least 
privilege at
any granularity.


and we can participate in OpenJDK to maintain Permission checks where we need 

Re: [External] : Authorization Layer post JEP 411

2021-06-03 Thread Ron Pressler
It is certainly time to accept that JEP 411 has been accepted, and so that those
who use Security Manager will need to do some work to change their software.

The purpose of this and upcoming discussions is to find reasonable approaches
that might relieve some portion of the burden on those who use SM today while
not placing an undue (indirect) burden on those who do not.

— Ron

> On 3 Jun 2021, at 10:43, Peter Firmstone  wrote:
> 
> Ok, thanks Ron,
> 
> I think we are confirming that Java, post version 17, will not meet the 
> security needs our software.  It's time I accepted that and moved on.
> 
> Thanks for your time.
> 
> Have you seen my latest article on foojay?   Feel free to comment and let me 
> know what you think.
> 
> https://urldefense.com/v3/__https://foojay.io/today/jep-411-what-it-means-for-javas-security-model/__;!!GqivPVa7Brio!MWpnS_ogZx24MskkZbSSrZ7ZbtCSyNeEswy1gegVSzGdDe4Qpmdy0ocIje9M4Wtv3A$
>  
> Cheers,
> 
> Peter.
> 
> 
> On 3/06/2021 7:33 pm, Ron Pressler wrote:
>> 
>>> On 3 Jun 2021, at 00:41, Peter Firmstone  
>>> wrote:
>>> 
>>> 
>>> StackWalker doesn't work with compiled code, only bytecode.
>> If you’re referring to GraalVM’s Native Image, I don’t know about that 
>> problem and
>> there does seem to be a relevant patch 
>> (https://urldefense.com/v3/__https://github.com/oracle/graal/pull/734__;!!GqivPVa7Brio!MWpnS_ogZx24MskkZbSSrZ7ZbtCSyNeEswy1gegVSzGdDe4Qpmdy0ocIje-DV8ldZw$
>>  ), but
>> Native Image is a separate project from OpenJDK.
>> 
>>> AccessController and AccessControlContext allow backward compatiblity for 
>>> JAAS.   JAAS whether we like it or not, is the default authorisation layer 
>>> framework.
>>> 
>>> https://urldefense.com/v3/__http://word-bits.flurg.com/jaas-is-terrible-and-there-is-no-escape-from-it/__;!!GqivPVa7Brio!MWpnS_ogZx24MskkZbSSrZ7ZbtCSyNeEswy1gegVSzGdDe4Qpmdy0ocIje-R7C-0Hg$
>>>  
>> I don’t know how much a seven-year-old article, that predates Java 8 
>> supports the use
>> of the present tense, but in any event, the JEP says that JAAS will be 
>> preserved.
>> 
>>> With SecurityManager gone, people will no longer assume it has sole 
>>> responsible for Security
>> People don’t assume that now, as secure software doesn’t employ it even 
>> today. People do,
>> however, assume that the mechanism, if used, is robust enough to be used for 
>> security
>> purposes.
>> 
>>> OpenJDK devs won't carry a significant burden for it's maintenance.
>> While the number of places where the JDK *implements* some “protected 
>> operation”, like
>> opening a file or writing to a socket, is somewhat bounded — and so keeping 
>> some hooks
>> in those places *might* be reasonable — the number of places that *use* 
>> those operations
>> is not. Maintaining doPrivileged in that unbounded set of places is not an 
>> insignificant
>> burden.
>> 
>> 
>>> Any security issues will be the responsibility of third party 
>>> implementations, like mine.
>>> The JDK won't provide an implementation, just the framework.
>> But the correct use of doPrivileged, if you’re proposing that it’s kept, 
>> must still be
>> tested against *some* implementation, and OpenJDK would still need to fix 
>> bugs related
>> to it.
>> 
>>> Those of us using the Principle of Least Privilege can continue to do so
>> Perhaps you believe that the only software in the world that applies Least 
>> Privilege is
>> Java software that employs the Security Manager, but that is not how most 
>> people, including
>> the person who had framed it two decades prior to the invention of the 
>> Security Manager,
>> understand the principle.
>> 
>> The original statement of the principle was: "Every program and every 
>> privileged user of
>> the system should operate using the least amount of privilege necessary to 
>> complete the
>> job.” 
>> (https://urldefense.com/v3/__https://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.226.3939__;!!GqivPVa7Brio!MWpnS_ogZx24MskkZbSSrZ7ZbtCSyNeEswy1gegVSzGdDe4Qpmdy0ocIje-xd8krsA$
>>  )
>> 
>> You are talking about applying the principle at a granularity of code units 
>> that are
>> smaller than a program. It’s fine to believe that is worthwhile, but the 
>> principle
>> certainly doesn’t require that every effort be expended to afford least 
>> privilege at
>> any granularity.
>> 
>>> and we can participate in OpenJDK to maintain Permission checks where we 
>>> need them and preserve context where appropriate.
>> I think you’re underestimating the magnitude of this work, which potentially 
>> interacts with
>> each and every change in the JDK (and in practice interacts with many of 
>> them, and today it’s
>> done by those who are responsible for the relevant change), which you’ll 
>> need to monitor,
>> not to mention that OpenJDK Reviewers, a role granted only to the most 
>> experienced contributors,
>> would still have to review that work.
>> 
>> However, if you think that is an amount of work you could manage, perhaps it 
>> could be done
>> outside the JDK 

Re: [External] : Authorization Layer post JEP 411

2021-06-03 Thread Peter Firmstone

Ok, thanks Ron,

I think we are confirming that Java, post version 17, will not meet the 
security needs our software.  It's time I accepted that and moved on.


Thanks for your time.

Have you seen my latest article on foojay?   Feel free to comment and 
let me know what you think.


https://foojay.io/today/jep-411-what-it-means-for-javas-security-model/

Cheers,

Peter.


On 3/06/2021 7:33 pm, Ron Pressler wrote:



On 3 Jun 2021, at 00:41, Peter Firmstone  wrote:


StackWalker doesn't work with compiled code, only bytecode.

If you’re referring to GraalVM’s Native Image, I don’t know about that problem 
and
there does seem to be a relevant patch 
(https://github.com/oracle/graal/pull/734), but
Native Image is a separate project from OpenJDK.


AccessController and AccessControlContext allow backward compatiblity for JAAS. 
  JAAS whether we like it or not, is the default authorisation layer framework.

http://word-bits.flurg.com/jaas-is-terrible-and-there-is-no-escape-from-it/

I don’t know how much a seven-year-old article, that predates Java 8 supports 
the use
of the present tense, but in any event, the JEP says that JAAS will be 
preserved.


With SecurityManager gone, people will no longer assume it has sole responsible 
for Security

People don’t assume that now, as secure software doesn’t employ it even today. 
People do,
however, assume that the mechanism, if used, is robust enough to be used for 
security
purposes.


OpenJDK devs won't carry a significant burden for it's maintenance.

While the number of places where the JDK *implements* some “protected 
operation”, like
opening a file or writing to a socket, is somewhat bounded — and so keeping 
some hooks
in those places *might* be reasonable — the number of places that *use* those 
operations
is not. Maintaining doPrivileged in that unbounded set of places is not an 
insignificant
burden.



Any security issues will be the responsibility of third party implementations, 
like mine.
The JDK won't provide an implementation, just the framework.

But the correct use of doPrivileged, if you’re proposing that it’s kept, must 
still be
tested against *some* implementation, and OpenJDK would still need to fix bugs 
related
to it.


Those of us using the Principle of Least Privilege can continue to do so

Perhaps you believe that the only software in the world that applies Least 
Privilege is
Java software that employs the Security Manager, but that is not how most 
people, including
the person who had framed it two decades prior to the invention of the Security 
Manager,
understand the principle.

The original statement of the principle was: "Every program and every 
privileged user of
the system should operate using the least amount of privilege necessary to 
complete the
job.” (https://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.226.3939)

You are talking about applying the principle at a granularity of code units 
that are
smaller than a program. It’s fine to believe that is worthwhile, but the 
principle
certainly doesn’t require that every effort be expended to afford least 
privilege at
any granularity.


and we can participate in OpenJDK to maintain Permission checks where we need 
them and preserve context where appropriate.

I think you’re underestimating the magnitude of this work, which potentially 
interacts with
each and every change in the JDK (and in practice interacts with many of them, 
and today it’s
done by those who are responsible for the relevant change), which you’ll need 
to monitor,
not to mention that OpenJDK Reviewers, a role granted only to the most 
experienced contributors,
would still have to review that work.

However, if you think that is an amount of work you could manage, perhaps it 
could be done
outside the JDK using Java Agents.


JAAS will continue to remain functional

The JEP already intends to keep JAAS functional, as far as I can tell.

— Ron




Re: [External] : Authorization Layer post JEP 411

2021-06-03 Thread Ron Pressler


> On 3 Jun 2021, at 00:41, Peter Firmstone  wrote:
> 
> 

> StackWalker doesn't work with compiled code, only bytecode.

If you’re referring to GraalVM’s Native Image, I don’t know about that problem 
and
there does seem to be a relevant patch 
(https://github.com/oracle/graal/pull/734), but
Native Image is a separate project from OpenJDK.

> 
> AccessController and AccessControlContext allow backward compatiblity for 
> JAAS.   JAAS whether we like it or not, is the default authorisation layer 
> framework.
> 
> http://word-bits.flurg.com/jaas-is-terrible-and-there-is-no-escape-from-it/

I don’t know how much a seven-year-old article, that predates Java 8 supports 
the use
of the present tense, but in any event, the JEP says that JAAS will be 
preserved.

> 
> With SecurityManager gone, people will no longer assume it has sole 
> responsible for Security

People don’t assume that now, as secure software doesn’t employ it even today. 
People do, 
however, assume that the mechanism, if used, is robust enough to be used for 
security 
purposes.

> OpenJDK devs won't carry a significant burden for it's maintenance.  

While the number of places where the JDK *implements* some “protected 
operation”, like
opening a file or writing to a socket, is somewhat bounded — and so keeping 
some hooks
in those places *might* be reasonable — the number of places that *use* those 
operations 
is not. Maintaining doPrivileged in that unbounded set of places is not an 
insignificant 
burden.


> Any security issues will be the responsibility of third party 
> implementations, like mine.
> The JDK won't provide an implementation, just the framework.

But the correct use of doPrivileged, if you’re proposing that it’s kept, must 
still be
tested against *some* implementation, and OpenJDK would still need to fix bugs 
related
to it.

> 
> Those of us using the Principle of Least Privilege can continue to do so

Perhaps you believe that the only software in the world that applies Least 
Privilege is 
Java software that employs the Security Manager, but that is not how most 
people, including 
the person who had framed it two decades prior to the invention of the Security 
Manager, 
understand the principle.

The original statement of the principle was: "Every program and every 
privileged user of 
the system should operate using the least amount of privilege necessary to 
complete the 
job.” (https://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.226.3939)

You are talking about applying the principle at a granularity of code units 
that are
smaller than a program. It’s fine to believe that is worthwhile, but the 
principle 
certainly doesn’t require that every effort be expended to afford least 
privilege at 
any granularity.

> and we can participate in OpenJDK to maintain Permission checks where we need 
> them and preserve context where appropriate.

I think you’re underestimating the magnitude of this work, which potentially 
interacts with 
each and every change in the JDK (and in practice interacts with many of them, 
and today it’s
done by those who are responsible for the relevant change), which you’ll need 
to monitor, 
not to mention that OpenJDK Reviewers, a role granted only to the most 
experienced contributors, 
would still have to review that work.

However, if you think that is an amount of work you could manage, perhaps it 
could be done 
outside the JDK using Java Agents.

> 
> JAAS will continue to remain functional 

The JEP already intends to keep JAAS functional, as far as I can tell.

— Ron

Re: Authorization Layer post JEP 411

2021-06-03 Thread Andrew Dinn

On 03/06/2021 08:28, Peter Firmstone wrote:
Apologies, I meant when compiled to native code, when you ship native 
binaries.


I'm not sure what you mean here. Are you talking about native binaries 
as generated by the GraalVM Native Image Generator? If you are 
suggesting there is a disparity in behaviour between any such image and 
the original app running on the JVM - whether specifically with respect 
to how the stack walk APIs operate or more generally  -- then I'd be 
very interested to know the full details.


Note however that were any such disparity to exist then there is no onus 
on the OpenJDK project to react to it. OpenJDK is based on a well 
defined standard and is not beholden to decisions made by other projects 
about how to translate Java code into a delivered executable.


Having said that, if it's necessary to use StackWalker behind 
AccessController.doPrivileged going forward then lets do so, and maybe 
the native binary is a later feature.


Hopefully my proposal is getting some consideration.
If you are proposing a change to Java then I think recommend that you 
propose it relative to the current reference implementation of the Java 
Language (and JVM) standards i.e. OpenJDK.


regards,


Andrew Dinn
---
Red Hat Distinguished Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill



Re: RFR: 8267485: Remove the dependency on SecurityManager in JceSecurityManager.java [v3]

2021-06-03 Thread Daniel Fuchs
On Wed, 2 Jun 2021 23:15:46 GMT, Bradford Wetmore  wrote:

>> The JceSecurityManager is currently a subclass of 
>> java.security.SecurityManager.  Now that JEP 411 has been integrated, this 
>> class should be updated to no longer subclass SecurityManager.
>> 
>> The only reason for using SecurityManager to easily get the Class Context 
>> (call stack), but we can achieve the same effect by using the JDK 9 API 
>> java.lang.StackWalkeer.  None of the other SecurityManager API are used.
>> 
>> I have run mach5 tier1/tier2 plus --test 
>> jck:api/java_security,jck:api/javax_crypto,jck:api/javax_net,jck:api/javax_security,jck:api/org_ietf,jck:api/javax_xml/crypto
>>  with all green.
>
> Bradford Wetmore has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - Address codereview comments
>  - Merge branch 'master' into JDK-8267485
>  - Merge branch 'master' into JDK-8267485
>  - Merge branch 'master' into JDK-8267485
>  - Replace missing annotation
>  - Merge branch 'master' into JDK-8267485
>  - Updated copyright date.
>  - 8267485: Remove the dependency on SecurityManager in 
> JceSecurityManager.java

src/java.base/share/classes/javax/crypto/JceSecurityManager.java line 111:

> 109: Option.RETAIN_CLASS_REFERENCE)
> 110: .walk((s) -> 
> s.collect(Collectors.toList(;
> 111: 

Note: StackWalker is a stateless capability object. It's not the walk() method 
that requires a permission, but the creation of the StackWalker itself (hence 
my suggestion to create it in the constructor, or in a static initializer). If 
you walk the stack from within a doPrivileged call then the doPrivileged frame 
will appear in the returned `List`; this may (or may not) be OK - 
depending on the logic that processes the stack.

-

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


Re: Authorization Layer post JEP 411

2021-06-03 Thread Peter Firmstone
Apologies, I meant when compiled to native code, when you ship native 
binaries.


Having said that, if it's necessary to use StackWalker behind 
AccessController.doPrivileged going forward then lets do so, and maybe 
the native binary is a later feature.


Hopefully my proposal is getting some consideration.

--
Regards,
 
Peter.


On 3/06/2021 5:18 pm, Alan Bateman wrote:



On 03/06/2021 01:04, Chapman Flack wrote:

On 06/02/21 19:41, Peter Firmstone wrote:
We need the power of AccessController's stack walk, StackWalker 
doesn't work

with compiled code, only bytecode.

Is this correct? I have not tried it, but the apidocs had led me to
understand it did not distinguish much between JITed and interpreted 
code.

Even getByteCodeIndex does not mention any limitation when the frame is
JITed Java code (though it does say it will return a negative number in
the distinct case of an actual native method).

There should be no issue here. I suspect Peter meant that the stack 
walker is about walking Java frames, it's transparent whether there 
are interpreter frames, compiled frame, or a mix on the stack.


-Alan




Re: Authorization Layer post JEP 411

2021-06-03 Thread Alan Bateman




On 03/06/2021 01:04, Chapman Flack wrote:

On 06/02/21 19:41, Peter Firmstone wrote:

We need the power of AccessController's stack walk, StackWalker doesn't work
with compiled code, only bytecode.

Is this correct? I have not tried it, but the apidocs had led me to
understand it did not distinguish much between JITed and interpreted code.
Even getByteCodeIndex does not mention any limitation when the frame is
JITed Java code (though it does say it will return a negative number in
the distinct case of an actual native method).

There should be no issue here. I suspect Peter meant that the stack 
walker is about walking Java frames, it's transparent whether there are 
interpreter frames, compiled frame, or a mix on the stack.


-Alan


Integrated: 8240256: Better resource cleaning for SunPKCS11 Provider

2021-06-03 Thread Sean Coffey
On Fri, 16 Apr 2021 11:24:57 GMT, Sean Coffey  wrote:

> 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

This pull request has now been integrated.

Changeset: bdeaeb47
Author:Sean Coffey 
URL:   
https://git.openjdk.java.net/jdk/commit/bdeaeb47d0155b9f233274cff90334e8dd761aae
Stats: 573 lines in 11 files changed: 467 ins; 68 del; 38 mod

8240256: Better resource cleaning for SunPKCS11 Provider

Reviewed-by: valeriep

-

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