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

[Anthony Scarpino]

On Thu, 20 May 2021 18:05:48 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix perf problem by reorganizing doLastBlock()
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
> line 1014:
> 
>> 1012: len += gctrghash.update(buffer, 0, bLen, out, 
>> outOfs);
>> 1013: outOfs += bLen;
>> 1014: }
> 
> For encryption, would the ibuffer contain more than one blocksize of data? 
> Isn't the existing impl only put the remaining input (less than a block) into 
> it? Line 1013: the `outOfs += bLen;`, shouldn't 'bLen' be 'len'?

Actually this is related to one of your code review comments from the previous 
change that went into jdk16 for the code to be safe.  It sounds like you are 
comfortable removing this check?  I will remove it.
Yes, it should be 'len', proving the code never gets run

-

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

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

[Anthony Scarpino]

On Thu, 20 May 2021 01:17:14 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix perf problem by reorganizing doLastBlock()
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
> line 975:
> 
>> 973: doUpdate(in, inOff, inLen, output, 0);
>> 974: } catch (ShortBufferException e) {
>> 975: // update decryption has no output
> 
> The comment does not seems to make sense? This is GCMEncrypt class. The right 
> reason should be that the output array is allocated by the provider and 
> should have the right size. However, it seems safer to throw 
> AssertionException() instead of swallowing the SBE...

Yeah the comment isn't right.  I think it's excessive to throw an exception 
that should never happen, but I'll add a ProviderException.

-

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

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

[Anthony Scarpino]

On Thu, 20 May 2021 00:37:44 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix perf problem by reorganizing doLastBlock()
>
> 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.

-

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

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

[Anthony Scarpino]

On Tue, 1 Jun 2021 19:28:44 GMT, Valerie Peng  wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
>> line 874:
>> 
>>> 872: } else if (!src.isDirect() && !dst.isDirect()) {
>>> 873: // if src is read only, then we need a copy
>>> 874: if (!src.isReadOnly()) {
>> 
>> Do you mean if (src.hasArray() && dst.hasArray())? Even if src is not read 
>> only, but array()/arrayOffset() methods are optional and can only be safely 
>> invoked after the hasArray() call.
>
> I assume this is again the case of !isDirect()&&!isReadOnly => hasArray()?

hasArray() would not be appropriate here.  The code needs to get into this if, 
being a heap bytebuffer.  If I use hasArray() it will skip to the ending else 
and cause a test failure.  Having the isReadOnly checked inside the 
if(isDirect) causes it to create the necessary copy

-

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

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

[Anthony Scarpino]

On Wed, 19 May 2021 23:59:36 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix perf problem by reorganizing doLastBlock()
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
> line 805:
> 
>> 803: /**
>> 804:  * This segments large data into smaller chunks so hotspot will 
>> start
>> 805:  * using CTR and GHASH intrinsics sooner.  This is a problem 
>> for app
> 
> nit: typo: CTR->GCTR? This is to improve performance rather than a problem, 
> no? Same comment applies to the other throttleData() method above.

Yes, this is for apps or perf tests that send large input data and cause the 
hotspot compiler to be slower to start using the intrinsics.. This is just a 
new version of what was in the old code in doLastBlock() around line 400.

-

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

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

[Anthony Scarpino]

On Wed, 19 May 2021 23:10:07 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix perf problem by reorganizing doLastBlock()
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
> line 757:
> 
>> 755: if (dst != null) {
>> 756: dst.put(block, 0, 
>> Math.min(block.length, len));
>> 757: }
> 
> Would this method work correctly if dst is null? Shouldn't this be checked in 
> the beginning of this method?

Because this is a general purpose methods, the check for dst == null is when op 
is only ghash.  Null is not an exception situation.

-

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

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

[Anthony Scarpino]

On Wed, 19 May 2021 22:05:16 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix perf problem by reorganizing doLastBlock()
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
> line 741:
> 
>> 739: } else {
>> 740: // If the remaining in buffer + data does 
>> not fill a
>> 741: // block, complete the gctr operation
> 
> This comment seems more suited for the else block below at line 753. In 
> addition, the criteria for completing the gctr operation should be whether 
> src still has remaining bytes left. It's possible that the (src.remaining() 
> == blockSize - over) and happen to fill up the block, right? The current flow 
> for this particular case would be an op.update(...) then continue down to 
> line 770-778, maybe you can adjust the if-check on line748 so this would go 
> through line 754-759 and return, i.e. the else block?

I changed the comment, but I also changed the code which will be in the next 
update

> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
> line 752:
> 
>> 750: if (dst != null) {
>> 751: dst.put(block, 0, blockSize);
>> 752: }
> 
> not counting this blockSize value into "processed"?

code is now changed

-

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

Re: RFR: 8248268: Support KWP in addition to KW [v9]

[Xue-Lei Andrew Fan]

On Sun, 30 May 2021 07:25:54 GMT, Valerie Peng  wrote:

>> This change updates SunJCE provider as below:
>> - updated existing AESWrap support with AES/KW/NoPadding cipher 
>> transformation. 
>> - added support for AES/KWP/NoPadding and AES/KW/PKCS5Padding.
>> 
>> Existing AESWrap impl, i.e. AESWrapCipher class, is re-factored and renamed 
>> to KeyWrapCipher class. The W and W_inverse functions are moved to KWUtil 
>> class. The KW and KWP support are in the new AESKeyWrap and AESKeyWrapPadded 
>> classes which extend FeedbackCipher and used in KeyWrapCipher class. To 
>> minimize data copying, AESKeyWrap and AESKeyWrapPadded will do the crypto 
>> operation over the same input buffer which is allocated and managed by 
>> KeyWrapCipher class. 
>> 
>> Also note that existing AESWrap impl does not take IV. However, the 
>> corresponding PKCS#11 mechanisms do, so I added support for accepting IVs to 
>> both KW and KWP.
>> 
>> Thanks,
>> Valerie
>
> Valerie Peng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update to address review feedbacks and minor optimizations.

Marked as reviewed by xuelei (Reviewer).

-

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

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

[Anthony Scarpino]

On Tue, 1 Jun 2021 18:54:19 GMT, Valerie Peng  wrote:

>> Yeah, I changed this code long ago, i suspect I didn't realize 
>> engineGetParameters() used it
>
> It seems that you still have not saved the specified 'random' into the 
> instance 'random' field? Did I miss something?

yeah.. i think i got confused with the "random" vs "this.random"

-

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

Re: RFR: 8248268: Support KWP in addition to KW [v9]

[Valerie Peng]

On Sun, 30 May 2021 07:25:54 GMT, Valerie Peng  wrote:

>> This change updates SunJCE provider as below:
>> - updated existing AESWrap support with AES/KW/NoPadding cipher 
>> transformation. 
>> - added support for AES/KWP/NoPadding and AES/KW/PKCS5Padding.
>> 
>> Existing AESWrap impl, i.e. AESWrapCipher class, is re-factored and renamed 
>> to KeyWrapCipher class. The W and W_inverse functions are moved to KWUtil 
>> class. The KW and KWP support are in the new AESKeyWrap and AESKeyWrapPadded 
>> classes which extend FeedbackCipher and used in KeyWrapCipher class. To 
>> minimize data copying, AESKeyWrap and AESKeyWrapPadded will do the crypto 
>> operation over the same input buffer which is allocated and managed by 
>> KeyWrapCipher class. 
>> 
>> Also note that existing AESWrap impl does not take IV. However, the 
>> corresponding PKCS#11 mechanisms do, so I added support for accepting IVs to 
>> both KW and KWP.
>> 
>> Thanks,
>> Valerie
>
> Valerie Peng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update to address review feedbacks and minor optimizations.

In the latest commit, I have made some minor optimizations regrading the 
in-place encryption as suggested in Mike's earlier comments. However, the 
optimizations are limited to when the output offset == 0 case.

-

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

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

[Anthony Scarpino]

On Tue, 1 Jun 2021 16:34:44 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Remove GCTR reset() calls because GCTR is released after the operation
>>   some variable name consistency
>>   other small cleanup
>
> src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java line 156:
> 
>> 154: }
>> 155: }
>> 156: */
> 
> Remove?

Yeah.. probably part of testing and never deleted it

-

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

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

[Valerie Peng]

On Mon, 24 May 2021 22:36:49 GMT, Anthony Scarpino  
wrote:

>> test/jdk/com/sun/crypto/provider/Cipher/AEAD/GCMShortBuffer.java line 34:
>> 
>>> 32: /*
>>> 33:  * @test
>>> 34:  * @summary Call decrypt doFinal() with different output values to see 
>>> if the
>> 
>> Missing bug id?
>
> Here is my logic
> It's not a regression test to verify a specific bug id, it's just a general 
> functionality test.  Therefore the bug id does not point to a previous 
> failure.

Hmm, alright.

>> test/jdk/com/sun/crypto/provider/Cipher/AES/TestAESCipher.java line 90:
>> 
>>> 88: i++;
>>> 89: }
>>> 90: return b;
>> 
>> The generated data seems too similar, all starts with 0 and increment. Maybe 
>> use i = len to start with? Or take some additional argument for starting 
>> value?
>
> I wanted consistent data because it's harder to solve crypto failures when 
> you cannot see consistent pattern in the data.  For example, the GCM case 
> where the ciphertext is correct, but the tag is wrong.  It is easier to know 
> that the problem is in GHASH and not GCTR.
> I don't see any negatives to having similar data.

Might as well just hardcode the data then. Saves time to re-generate the 
similar bytes over and over. Not seeing much benefit of this method. Not a big 
deal if you intentionally use similar data.

-

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

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

[Valerie Peng]

On Thu, 20 May 2021 00:57:42 GMT, Valerie Peng  wrote:

>> Anthony Scarpino has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix perf problem by reorganizing doLastBlock()
>
> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
> line 874:
> 
>> 872: } else if (!src.isDirect() && !dst.isDirect()) {
>> 873: // if src is read only, then we need a copy
>> 874: if (!src.isReadOnly()) {
> 
> Do you mean if (src.hasArray() && dst.hasArray())? Even if src is not read 
> only, but array()/arrayOffset() methods are optional and can only be safely 
> invoked after the hasArray() call.

I assume this is again the case of !isDirect()&&!isReadOnly => hasArray()?

-

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

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

[Valerie Peng]

On Fri, 21 May 2021 04:15:44 GMT, Anthony Scarpino  
wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
>> line 628:
>> 
>>> 626: }
>>> 627: 
>>> 628: int mergeBlock(byte[] buffer, int bufOfs, byte[] in, int inOfs,
>> 
>> Can be made 'static'? No javadoc for this one, maybe move the javadoc for 
>> the other one up and use "Methods" in the javadoc to cover both?
>
> I just copied it and made a slight change to explain the difference

Sure.

-

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

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

[Valerie Peng]

On Fri, 21 May 2021 03:07:15 GMT, Anthony Scarpino  
wrote:

>> yeah these checks are a bit all over the place.. I'll rework them
>
> So I think I only need to add a check to the engineDoFinal() that did not 
> have a check before.

for engineUpdate(...) impl, the ArrayUtil.nullAndBoundsCheck() calls are 
deferred to GCMEngine.doUpdate(...). In the case of engineUpdate(byte[] input, 
int inputOffset, int inputLen, byte[] output, int outputOffset) method, the 
inputLen is already used to calculate the needed output size before the check 
takes place. For consistency and correctness, shouldn't the check be done 
before the values are used, as in engineDoFinal() case?

-

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

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

[Valerie Peng]

On Mon, 24 May 2021 16:37:05 GMT, Anthony Scarpino  
wrote:

>> src/java.base/share/classes/com/sun/crypto/provider/GHASH.java line 189:
>> 
>>> 187: ct.position(ct.position());
>>> 188: return processed;
>>> 189: } else if (!ct.isReadOnly()) {
>> 
>> Maybe you meant "ct.hasArray()" instead of "!ct.isReadOnly()"?
>
> hasArray() checks both isReadonly() and isDirect().  Since I already did 
> isDirect() in the previous if, I just need to check readonly here

I see.

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
>> line 259:
>> 
>>> 257: @Override
>>> 258: protected void engineInit(int opmode, Key key,
>>> 259: AlgorithmParameterSpec params, SecureRandom random)
>> 
>> The specified "random" is not saved to internal "random". Perhaps an 
>> oversight?
>
> Yeah, I changed this code long ago, i suspect I didn't realize 
> engineGetParameters() used it

It seems that you still have not saved the specified 'random' into the instance 
'random' field? Did I miss something?

>> src/java.base/share/classes/com/sun/crypto/provider/GaloisCounterMode.java 
>> line 279:
>> 
>>> 277: if (iv.length == 0) {
>>> 278: throw new InvalidAlgorithmParameterException("IV is 
>>> empty");
>>> 279: }
>> 
>> Why separating the validation of iv and tag length in separate methods, i.e. 
>> engineInit() vs init()? Consider consolidating them?
>
> I think I had a reason for this where I didn't know what the exact error was. 
>  I'd rather keep them separate so it throws the right message for the error.  
> It's not a performance critical area.

Ok, it's more for code robustness, not for performance.

-

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

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

[Valerie Peng]

On Wed, 26 May 2021 21:13:56 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 one 
> additional commit since the last revision:
> 
>   Remove GCTR reset() calls because GCTR is released after the operation
>   some variable name consistency
>   other small cleanup

Most of the updates look fine, but some of my review comments in 
GaloisCounterMode didn't get a reply, just want to be sure that you saw them?

Thanks,
Valerie

src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java line 156:

> 154: }
> 155: }
> 156: */

Remove?

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

> 94: 
> 95: // Default value is 128bits, this is in bytes.
> 96: int tagLenBytes = 16;

nit: use DEFAULT_TAG_LEN instead of 16

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

> 742: int resultLen = 0;
> 743: 
> 744: int bLen = getBufferedLength();

Well, it seems strange to calculate bLen based on 'ibuffer' instead of the 
specified 'buffer'. For code clarity, it seems better to just use 'buffer'?

-

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

Integrated: 8180571: Refactor sun/security/pkcs11 shell tests to plain java tests and fix failures

[Fernando Guallini]

On Tue, 18 May 2021 13:19:53 GMT, Fernando Guallini  
wrote:

> Refactor the following shell tests to Java:
> - security/pkcs11/KeyStore/Basic.sh
> - security/pkcs11/KeyStore/ClientAuth.sh
> - security/pkcs11/KeyStore/SecretKeysBasic.sh
> - security/pkcs11/Provider/ConfigQuotedString.sh
> - security/pkcs11/Provider/Login.sh
> - security/pkcs11/Config/ReadConfInUTF16Env.sh
> 
> Currently, most of the shell tests in the list may be ignored during 
> execution time in most platforms since they are incorrectly filtered out by 
> the OS name they are run on. For example, ClientAuth.sh is only run if the OS 
> name is equal to ‘Linux’, but OS name may also include the architecture such 
> as ‘Linux x86_64’. Those platform constraints are removed in this PR.
> 
> Additionally, further changes are introduced in the following test:
> 
> - ClientAuth: it was failing intermittently because the server side was 
> binding to the wildcard address. The issue is fixed by binding to loopback 
> address instead. Also, Thread.sleep is replaced with CountdownLatch to 
> facilitate synchronization between client and server. Finally, a new ‘user1’ 
> certificate was generated since the current one has expired.
> 
> - Basic: Remove redundant X509Certificate casting 
> 
> - SecretKeysBasic: it was already in the problem list since it reproduces the 
> open bug JDK-8209398 and fails. Test is refactored to java and still 
> reproduces the issue.
> 
> All the mentioned tests were run many times in multiple platforms to ensure 
> stability

This pull request has now been integrated.

Changeset: ccfcd926
Author:Fernando Guallini 
Committer: Xue-Lei Andrew Fan 
URL:   
https://git.openjdk.java.net/jdk/commit/ccfcd926674ee0bd88f34b16b489abe008169b11
Stats: 1076 lines in 18 files changed: 195 ins; 820 del; 61 mod

8180571: Refactor sun/security/pkcs11 shell tests to plain java tests and fix 
failures

Reviewed-by: xuelei

-

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

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

[Sean Mullan]

On 6/1/21 1:30 PM, Chapman Flack wrote:

On 06/01/21 12:32, Sean Mullan wrote:

On 6/1/21 2:11 AM, Peter Firmstone wrote:

Would have really liked to have known about that.


It was announced on jdk-dev at the time it was launched, with a follow-up
reminder one week later:

https://mail.openjdk.java.net/pipermail/jdk-dev/2018-February/000649.html


Thanks for that link. I see the announcement said

   The results of the survey will be made public after the survey closes.

That email thread seems to end with the week-later reminder, but I think
I found the followup [1].

It links to the results on SurveyMonkey [2], which seem to be viewable
if one whitelists 
https://urldefense.com/v3/__http://www.surveymonkey.com__;!!GqivPVa7Brio!PQjV64chVEWfRM5VPM45uJ1dPiIsGzyukEkPp1I8jCoJWUg65dwS9cshHXnCziOu$
 , cdnjs.cloudflare.com, and
prod.smassets.net.


Those results are very incomplete (it says there were only 16 responses 
when there were more). I don't know what happened to the rest of the 
responses, perhaps SurveyMonkey deleted many of them over time for some 
reason. The page should probably be deleted as it is no longer accurate. 
I do have a PDF of all the results that I saved locally, but I have no 
plans to go over that again or post that right now.



It also refers to a blog post [3], no longer a good link; also the only
occurrence of that URL in the Wayback Machine is a November 2020 archive
of its "The content that you're looking [sic] is no longer on this page"
page [4].


It was automatically deleted, probably because I didn't post enough ...


Is it possible the blogs.oracle.com URL structure just got rearranged?
Is there a currently usable link? (My search there for "security manager
survey" produces 742 results, six at a time; I haven't found it yet.)

Ironically, I easily found a March 2021 blog post [5] "Quiz yourself:
Using the SecurityManager class in Java", from which a reader might
have come away confident the thing was a going concern.


This is just a quiz about how to use some of the SecurityManager APIs by 
some people who write about Java technology.


--Sean



Regards,
-Chap



[1] https://mail.openjdk.java.net/pipermail/jdk-dev/2018-March/000929.html
[2] 
https://urldefense.com/v3/__https://www.surveymonkey.com/results/SM-PSJ6ZNMZ8/__;!!GqivPVa7Brio!PQjV64chVEWfRM5VPM45uJ1dPiIsGzyukEkPp1I8jCoJWUg65dwS9cshHeuKhBql$
[3] https://blogs.oracle.com/mullan/securitymanager-survey-results
[4]
https://urldefense.com/v3/__https://web.archive.org/web/20201121023702/https:/*blogs.oracle.com/mullan/securitymanager-survey-results__;Lw!!GqivPVa7Brio!PQjV64chVEWfRM5VPM45uJ1dPiIsGzyukEkPp1I8jCoJWUg65dwS9cshHUl1Q8D8$
[5]
https://blogs.oracle.com/javamagazine/quiz-yourself-using-the-securitymanager-class-in-java

Re: JEP 411: Missing use-case: user functions in an RDBMS

[Geertjan Wielenga]

We have updated the JEP with a few changes to the "Issue Warnings"
section [1], summarized as follows:

If the Java runtime is started without setting the system property
'java.security.manager' then a custom Security Manager can be installed
dynamically by calling System::setSecurityManager, just as in Java 16.
No UnsupportedOperationException will be thrown. This call will,
however, issue a warning message explaining that the Security Manager is
deprecated and will be removed in a future release.

We plan to change the default value of the 'java.security.manager'
system property to "disallow" in the next release, i.e., Java 18. That
will cause System::setSecurityManager to throw an
UnsupportedOperationException in Java 18.

With these changes, the process of deprecating and eventually removing
the Security Manager will be consistent with our treatment of past
breaking changes such as, e.g., the strong encapsulation of internal
APIs. Maintainers of libraries and applications will be given fair
warning before any existing code is broken.

https://mail.openjdk.java.net/pipermail/jdk-dev/2021-May/005616.html



On Sat, May 29, 2021 at 12:04 AM Peter Firmstone <
peter.firmst...@zeus.net.au> wrote:

> While I accept that my particular use case will no longer be supported
> in future, it's difficult to see the value of a sandbox, because
> developers will always want to relax it in some way and people fall into
> the trap of thinking that trust is black and white; this is trusted,
> that is not.
>
> Nowadays, there's a lot more interest in the Principle of Least
> Privilege, especially in health care regulations, now I'm going to use
> the meaning implied in "Inside Java 2 Platform Security, Second
> Edition", so that we aren't arguing whether that applies with the JVM or
> external to it.
>
> The reason POLP is simpler, is that it can be automated with tooling,
> then the development / deployment team may choose to relax it, they can
> certainly test it and validate it.   Then we are simply focused on what
> we need to do, rather than what we might want to restrict, which is
> always unknown and subject to change.
>
> The problem is that this is not how developers have been taught to use
> SecurityManager, instead they are told that trusted code can be given
> AllPermission because it's trusted and that's an approach which has
> proven largely ineffective and the OpenJDK development team carry's the
> cost as a result, with little benefit.
>
> I just thought I'd provide you with a different perspective, so
> hopefully the mistake isn't repeated.
>
> Peter.
>
> On 28/05/2021 8:09 pm, Ron Pressler wrote:
> >
> > Deep sandboxes, simple or stack-dependent, are useful for very “rich”
> code,
> > that is potentially very big and possibly contains arbitrary third-party
> > libraries, while shallow sandboxes are more suitable to limited plugins.
> >
> > While a complex, stack-dependent, deep sandbox *could* be used for
> plugins,
> > permissions that don’t specify what is forbidden but what is allowed
> > effectively also severely limit the use of third-party libraries, that
> > for example, might want to do benign operations with their own files,
> > and so effectively only allow very limited plugins. The result is a
> costly
> > mechanism that is overkill for what it’s used for.
> >
> > — Ron
>
>

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

[joshua spies]

RE:no

Re: Updates to JEP 411: Deprecate the Security Manager for Removal

[Alex Buckley]

A further point is that `-Djava.security.manager=allow` has been 
special-cased since JDK 12:


- RFE:
  https://bugs.openjdk.java.net/browse/JDK-8191053

- Compatibility review:
  https://bugs.openjdk.java.net/browse/JDK-8203316

- Release note:

https://www.oracle.com/java/technologies/javase/12-relnote-issues.html#JDK-8191053 



(I saw that Netbeans has run into trouble under JEP 411 because some 
code does not recognize `allow`, and treats it as a class name: 
https://issues.apache.org/jira/browse/NETBEANS-5703)


Alex

On 5/19/2021 6:48 AM, Sean Mullan wrote:
Thanks for those that have taken the time to review JEP 411 [1]. The JEP 
has been updated today with a few changes, the most significant which is 
the addition of a new section titled "Future Work" [2] which lists 
related potential enhancements and works in progress.


Other smaller changes have been made throughout the JEP, including:

  - The "Alternate JAAS APIs" section has been removed and replaced with 
a smaller section in the Description section with a pointer to a JBS 
issue with more details.


  - Changes have been made to the Motivation section to improve some of 
the wording and add more details.


  - A new item named "Preserve the Security Manager API for extension by 
custom Security Managers that wish to intercept, log, and veto access to 
resources" has been added to the Alternatives section, as this has been 
a topic of discussion since the JEP has been published.


  - RMISecurityException has been removed from the list of APIs to be 
deprecated for removal, and 
java.util.concurrent.Executors::{privilegedCallable, 
privilegedCallableUsingCurrentClassLoader, privilegedThreadFactory} have 
been added.


Thanks,
Sean

[1] https://openjdk.java.net/jeps/411
[2] https://openjdk.java.net/jeps/411#Future-Work

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

[Chapman Flack]

On 06/01/21 12:32, Sean Mullan wrote:
> On 6/1/21 2:11 AM, Peter Firmstone wrote:
>> Would have really liked to have known about that.
> 
> It was announced on jdk-dev at the time it was launched, with a follow-up
> reminder one week later:
> 
> https://mail.openjdk.java.net/pipermail/jdk-dev/2018-February/000649.html

Thanks for that link. I see the announcement said

  The results of the survey will be made public after the survey closes.

That email thread seems to end with the week-later reminder, but I think
I found the followup [1].

It links to the results on SurveyMonkey [2], which seem to be viewable
if one whitelists www.surveymonkey.com, cdnjs.cloudflare.com, and
prod.smassets.net.

It also refers to a blog post [3], no longer a good link; also the only
occurrence of that URL in the Wayback Machine is a November 2020 archive
of its "The content that you're looking [sic] is no longer on this page"
page [4].

Is it possible the blogs.oracle.com URL structure just got rearranged?
Is there a currently usable link? (My search there for "security manager
survey" produces 742 results, six at a time; I haven't found it yet.)

Ironically, I easily found a March 2021 blog post [5] "Quiz yourself:
Using the SecurityManager class in Java", from which a reader might
have come away confident the thing was a going concern.

Regards,
-Chap



[1] https://mail.openjdk.java.net/pipermail/jdk-dev/2018-March/000929.html
[2] https://www.surveymonkey.com/results/SM-PSJ6ZNMZ8/
[3] https://blogs.oracle.com/mullan/securitymanager-survey-results
[4]
https://web.archive.org/web/20201121023702/https://blogs.oracle.com/mullan/securitymanager-survey-results
[5]
https://blogs.oracle.com/javamagazine/quiz-yourself-using-the-securitymanager-class-in-java

Integrated: 8267990: Revisit some uses of `synchronized` in the HttpClient API

[Daniel Fuchs]

On Mon, 31 May 2021 16:21:29 GMT, Daniel Fuchs  wrote:

> The Utils.remaining(List list) method assumes that it can and 
> should synchronize on the given list to prevent concurrent modification. In 
> 99% of the cases this assumption is wrong. There's only one such list (the 
> SSLFlowDelegate writeList) that requires this synchronization. 
> 
> Also the `SequentialScheduler.synchronizedScheduler` uses `synchronized`, it 
> could use a Lock instead and this would make it possible to assert that there 
> is no contention (since the logic of the SequentialScheduler is supposed to 
> prevent contention from occurring at this place).

This pull request has now been integrated.

Changeset: 9d8ad2ed
Author:Daniel Fuchs 
URL:   
https://git.openjdk.java.net/jdk/commit/9d8ad2ed62325bd8d813974d5aa1e031ed8bf8da
Stats: 105 lines in 13 files changed: 62 ins; 6 del; 37 mod

8267990: Revisit some uses of `synchronized` in the HttpClient API

Reviewed-by: chegar

-

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

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

[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 with a new target base due to 
a merge or a rebase. The pull request now contains 38 commits:

 - Merge branch 'master' into JEP-412
 - Merge branch 'master' into JEP-412
 - * Add missing `final` in some static fields
   * Downgrade native methods in ProgrammableUpcallHandler to package-private
 - Add sealed modifiers in morally sealed API interfaces
 - Merge branch 'master' into JEP-412
 - Fix VaList test
   Remove unused code in Utils
 - Merge pull request #11 from JornVernee/JEP-412-MXCSR
   
   Add MXCSR save and restore to upcall stubs for non-windows platforms
 - Add MXCSR save and restore to upcall stubs for non-windows platforms
 - Merge branch 'master' into JEP-412
 - Fix issue with bounded arena allocator
 - ... and 28 more: https://git.openjdk.java.net/jdk/compare/36dc268a...10767bc0

-

Changes: https://git.openjdk.java.net/jdk/pull/3699/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3699&range=26
  Stats: 14501 lines in 219 files changed: 8847 ins; 3642 del; 2012 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: JEP411: Missing use-case: Monitoring / restricting libraries

[Dalibor Topic]

The survey was also shared by @java to its many followers, fwiw:

https://twitter.com/java/status/961296752732135424?s=20

and through the OpenJDK Quality Outreach to the many projects 
participating there:


https://mail.openjdk.java.net/pipermail/quality-discuss/2018-February/000749.html

If you'd like to get regular information about upcoming JEPs, bug fixes, 
surveys etc. please consider joining the Quality Outreach for your open 
source project by joining the mailing list quality-discuss, and asking 
there to be added to the list.


Please see 
https://wiki.openjdk.java.net/display/quality/Quality+Outreach for 
details on participating projects, reports, etc.


cheers,
dalibor topic

On 01.06.2021 18:32, Sean Mullan wrote:



On 6/1/21 2:11 AM, Peter Firmstone wrote:

Would have really liked to have known about that.


It was announced on jdk-dev at the time it was launched, with a 
follow-up reminder one week later:


https://mail.openjdk.java.net/pipermail/jdk-dev/2018-February/000649.html

--Sean



Any chance Java 17 LTS (hopefully with sorted security manager
property), can be supported as long as Java 8, 16 years?

The new security API's don't exist yet, I'd prefer to work with a
version that has a fully functional SecurityManager rather than one
that's non functional, because it's the functionality that's important
to us.

It's going to take a long time to migrate, once the new API's exist,
they need to receive acceptance, then we also need to rewrite our
software to suit, assuming it takes 4 years to write and implement the
new security api, there's not much time left then before 17's EOL if it
only has 8 years of support.

I figure there's no harm in asking.

Regards,

Peter.

On 1/06/2021 3:51 pm, Alan Bateman wrote:

On 01/06/2021 01:11, Chapman Flack wrote:

On 05/31/21 03:59, Alan Bateman wrote:

The SM survey in 2018 showed that there was some usage too.

Out of curiosity, where can I learn more about this survey?

Was it the kind of survey that, if I had been hanging around in the
right
places in 2018, I might have been solicited to respond to?

The survey that has been referenced a few times here was in Feb 2018
[1]. Sean linked to the results at the time [2] but the link no longer
works. The main goal was to gather data on usages beyond applets and
JNLP.

-Alan

[1] https://twitter.com/seanjmullan/status/960553938553630721
[2] https://twitter.com/seanjmullan/status/976466563997028354


--
 Dalibor Topic
Consulting Product Manager
Phone: +494089091214 , Mobile: +491737185961
, Video: dalibor.to...@oracle.com


Oracle Global Services Germany GmbH
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRB 246209
Geschäftsführer: Ralf Herrmann

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

[Sean Mullan]

On 6/1/21 2:11 AM, Peter Firmstone wrote:

Would have really liked to have known about that.


It was announced on jdk-dev at the time it was launched, with a 
follow-up reminder one week later:


https://mail.openjdk.java.net/pipermail/jdk-dev/2018-February/000649.html

--Sean



Any chance Java 17 LTS (hopefully with sorted security manager
property), can be supported as long as Java 8, 16 years?

The new security API's don't exist yet, I'd prefer to work with a
version that has a fully functional SecurityManager rather than one
that's non functional, because it's the functionality that's important
to us.

It's going to take a long time to migrate, once the new API's exist,
they need to receive acceptance, then we also need to rewrite our
software to suit, assuming it takes 4 years to write and implement the
new security api, there's not much time left then before 17's EOL if it
only has 8 years of support.

I figure there's no harm in asking.

Regards,

Peter.

On 1/06/2021 3:51 pm, Alan Bateman wrote:

On 01/06/2021 01:11, Chapman Flack wrote:

On 05/31/21 03:59, Alan Bateman wrote:

The SM survey in 2018 showed that there was some usage too.

Out of curiosity, where can I learn more about this survey?

Was it the kind of survey that, if I had been hanging around in the
right
places in 2018, I might have been solicited to respond to?

The survey that has been referenced a few times here was in Feb 2018
[1]. Sean linked to the results at the time [2] but the link no longer
works. The main goal was to gather data on usages beyond applets and
JNLP.

-Alan

[1] https://twitter.com/seanjmullan/status/960553938553630721
[2] https://twitter.com/seanjmullan/status/976466563997028354

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v8]

[Joe Wang]

On Tue, 1 Jun 2021 15:21:33 GMT, Weijun Wang  wrote:

>> Please review this implementation of [JEP 
>> 411](https://openjdk.java.net/jeps/411).
>> 
>> The code change is divided into 3 commits. Please review them one by one.
>> 
>> 1. 
>> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>>  The essential change for this JEP, including the `@Deprecate` annotations 
>> and spec change. It also update the default value of the 
>> `java.security.manager` system property to "disallow", and necessary test 
>> change following this update.
>> 2. 
>> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>>  Manual changes to several files so that the next commit can be generated 
>> programatically.
>> 3. 
>> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>>  Automatic changes to other source files to avoid javac warnings on 
>> deprecation for removal
>> 
>> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
>> generated programmatically, see the comment below for more details. If you 
>> are only interested in a portion of the 3rd commit and would like to review 
>> it as a separate file, please comment here and I'll generate an individual 
>> webrev.
>> 
>> Due to the size of this PR, no attempt is made to update copyright years for 
>> any file to minimize unnecessary merge conflict.
>> 
>> Furthermore, since the default value of `java.security.manager` system 
>> property is now "disallow", most of the tests calling 
>> `System.setSecurityManager()` need to launched with 
>> `-Djava.security.manager=allow`. This is covered in a different PR at 
>> https://github.com/openjdk/jdk/pull/4071.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 11 commits:
> 
>  - merge from master
>  - rename setSecurityManagerDirect to implSetSecurityManager
>  - default behavior reverted to allow
>  - move one annotation to new method
>  - merge from master, two files removed, one needs merge
>  - keep only one systemProperty tag
>  - fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java
>  - feedback from Sean, Phil and Alan
>  - add supresswarnings annotations automatically
>  - manual change before automatic annotating
>  - ... and 1 more: 
> https://git.openjdk.java.net/jdk/compare/74b70a56...ea2c4b48

Marked as reviewed by joehw (Reviewer).

-

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

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v8]

[Alan Bateman]

On Tue, 1 Jun 2021 15:21:33 GMT, Weijun Wang  wrote:

>> Please review this implementation of [JEP 
>> 411](https://openjdk.java.net/jeps/411).
>> 
>> The code change is divided into 3 commits. Please review them one by one.
>> 
>> 1. 
>> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>>  The essential change for this JEP, including the `@Deprecate` annotations 
>> and spec change. It also update the default value of the 
>> `java.security.manager` system property to "disallow", and necessary test 
>> change following this update.
>> 2. 
>> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>>  Manual changes to several files so that the next commit can be generated 
>> programatically.
>> 3. 
>> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>>  Automatic changes to other source files to avoid javac warnings on 
>> deprecation for removal
>> 
>> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
>> generated programmatically, see the comment below for more details. If you 
>> are only interested in a portion of the 3rd commit and would like to review 
>> it as a separate file, please comment here and I'll generate an individual 
>> webrev.
>> 
>> Due to the size of this PR, no attempt is made to update copyright years for 
>> any file to minimize unnecessary merge conflict.
>> 
>> Furthermore, since the default value of `java.security.manager` system 
>> property is now "disallow", most of the tests calling 
>> `System.setSecurityManager()` need to launched with 
>> `-Djava.security.manager=allow`. This is covered in a different PR at 
>> https://github.com/openjdk/jdk/pull/4071.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 11 commits:
> 
>  - merge from master
>  - rename setSecurityManagerDirect to implSetSecurityManager
>  - default behavior reverted to allow
>  - move one annotation to new method
>  - merge from master, two files removed, one needs merge
>  - keep only one systemProperty tag
>  - fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java
>  - feedback from Sean, Phil and Alan
>  - add supresswarnings annotations automatically
>  - manual change before automatic annotating
>  - ... and 1 more: 
> https://git.openjdk.java.net/jdk/compare/74b70a56...ea2c4b48

Marked as reviewed by alanb (Reviewer).

-

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

Re: RFR: 8180571: Refactor sun/security/pkcs11 shell tests to plain java tests and fix failures [v2]

[Xue-Lei Andrew Fan]

On Fri, 28 May 2021 15:50:23 GMT, Fernando Guallini  
wrote:

>> Refactor the following shell tests to Java:
>> - security/pkcs11/KeyStore/Basic.sh
>> - security/pkcs11/KeyStore/ClientAuth.sh
>> - security/pkcs11/KeyStore/SecretKeysBasic.sh
>> - security/pkcs11/Provider/ConfigQuotedString.sh
>> - security/pkcs11/Provider/Login.sh
>> - security/pkcs11/Config/ReadConfInUTF16Env.sh
>> 
>> Currently, most of the shell tests in the list may be ignored during 
>> execution time in most platforms since they are incorrectly filtered out by 
>> the OS name they are run on. For example, ClientAuth.sh is only run if the 
>> OS name is equal to ‘Linux’, but OS name may also include the architecture 
>> such as ‘Linux x86_64’. Those platform constraints are removed in this PR.
>> 
>> Additionally, further changes are introduced in the following test:
>> 
>> - ClientAuth: it was failing intermittently because the server side was 
>> binding to the wildcard address. The issue is fixed by binding to loopback 
>> address instead. Also, Thread.sleep is replaced with CountdownLatch to 
>> facilitate synchronization between client and server. Finally, a new ‘user1’ 
>> certificate was generated since the current one has expired.
>> 
>> - Basic: Remove redundant X509Certificate casting 
>> 
>> - SecretKeysBasic: it was already in the problem list since it reproduces 
>> the open bug JDK-8209398 and fails. Test is refactored to java and still 
>> reproduces the issue.
>> 
>> All the mentioned tests were run many times in multiple platforms to ensure 
>> stability
>
> Fernando Guallini has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains six commits:
> 
>  - Merge branch 'master' into 8180571
>  - removed tab in comment
>  - Merge branch 'master' into 8180571
>  - fixed summary in ClientAuth
>  - refactor tests
>  - refactored shell tests to java

Nice update.  Look good to me.

-

Marked as reviewed by xuelei (Reviewer).

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

Re: RFR: 8180571: Refactor sun/security/pkcs11 shell tests to plain java tests and fix failures [v2]

[Fernando Guallini]

On Fri, 28 May 2021 15:50:23 GMT, Fernando Guallini  
wrote:

>> Refactor the following shell tests to Java:
>> - security/pkcs11/KeyStore/Basic.sh
>> - security/pkcs11/KeyStore/ClientAuth.sh
>> - security/pkcs11/KeyStore/SecretKeysBasic.sh
>> - security/pkcs11/Provider/ConfigQuotedString.sh
>> - security/pkcs11/Provider/Login.sh
>> - security/pkcs11/Config/ReadConfInUTF16Env.sh
>> 
>> Currently, most of the shell tests in the list may be ignored during 
>> execution time in most platforms since they are incorrectly filtered out by 
>> the OS name they are run on. For example, ClientAuth.sh is only run if the 
>> OS name is equal to ‘Linux’, but OS name may also include the architecture 
>> such as ‘Linux x86_64’. Those platform constraints are removed in this PR.
>> 
>> Additionally, further changes are introduced in the following test:
>> 
>> - ClientAuth: it was failing intermittently because the server side was 
>> binding to the wildcard address. The issue is fixed by binding to loopback 
>> address instead. Also, Thread.sleep is replaced with CountdownLatch to 
>> facilitate synchronization between client and server. Finally, a new ‘user1’ 
>> certificate was generated since the current one has expired.
>> 
>> - Basic: Remove redundant X509Certificate casting 
>> 
>> - SecretKeysBasic: it was already in the problem list since it reproduces 
>> the open bug JDK-8209398 and fails. Test is refactored to java and still 
>> reproduces the issue.
>> 
>> All the mentioned tests were run many times in multiple platforms to ensure 
>> stability
>
> Fernando Guallini has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains six commits:
> 
>  - Merge branch 'master' into 8180571
>  - removed tab in comment
>  - Merge branch 'master' into 8180571
>  - fixed summary in ClientAuth
>  - refactor tests
>  - refactored shell tests to java

Please if someone could review and sponsor this PR. Thanks

-

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

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v8]

[Weijun Wang]

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.
> 
> Update: the deprecation annotations and javadoc tags, build, compiler, 
> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
> reviewed. Rest are 2d, awt, beans, sound, and swing.

Weijun Wang has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 11 commits:

 - merge from master
 - rename setSecurityManagerDirect to implSetSecurityManager
 - default behavior reverted to allow
 - move one annotation to new method
 - merge from master, two files removed, one needs merge
 - keep only one systemProperty tag
 - fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java
 - feedback from Sean, Phil and Alan
 - add supresswarnings annotations automatically
 - manual change before automatic annotating
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/74b70a56...ea2c4b48

-

Changes: https://git.openjdk.java.net/jdk/pull/4073/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4073&range=07
  Stats: 2132 lines in 826 files changed: 1997 ins; 20 del; 115 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4073.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4073/head:pull/4073

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

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v7]

[Weijun Wang]

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.
> 
> Update: the deprecation annotations and javadoc tags, build, compiler, 
> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
> reviewed. Rest are 2d, awt, beans, sound, and swing.

Weijun Wang has updated the pull request incrementally with one additional 
commit since the last revision:

  rename setSecurityManagerDirect to implSetSecurityManager

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4073/files
  - new: https://git.openjdk.java.net/jdk/pull/4073/files/8fd09c39..926e4b9a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4073&range=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4073&range=05-06

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

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

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

[Dalibor Topic]

On 01.06.2021 08:11, Peter Firmstone wrote:
Any chance Java 17 LTS (hopefully with sorted security manager 
property), can be supported as long as Java 8, 16 years?


According to the Oracle Java SE Support Roadmap at 
https://www.oracle.com/java/technologies/java-se-support-roadmap.html 
extended support for Java SE 17 is planned until September 2029.


That's longer than Java SE 8 was initially planned to be supported.

It's too early to speculate about extensions at this point in time 
before the ecosystem has had a chance to adopt the release.


cheers,
dalibor topic
--
 Dalibor Topic
Consulting Product Manager
Phone: +494089091214 , Mobile: +491737185961
, Video: dalibor.to...@oracle.com


Oracle Global Services Germany GmbH
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRB 246209
Geschäftsführer: Ralf Herrmann

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v6]

[Alan Bateman]

On Mon, 31 May 2021 15:02:57 GMT, Weijun Wang  wrote:

>> Please review this implementation of [JEP 
>> 411](https://openjdk.java.net/jeps/411).
>> 
>> The code change is divided into 3 commits. Please review them one by one.
>> 
>> 1. 
>> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>>  The essential change for this JEP, including the `@Deprecate` annotations 
>> and spec change. It also update the default value of the 
>> `java.security.manager` system property to "disallow", and necessary test 
>> change following this update.
>> 2. 
>> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>>  Manual changes to several files so that the next commit can be generated 
>> programatically.
>> 3. 
>> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>>  Automatic changes to other source files to avoid javac warnings on 
>> deprecation for removal
>> 
>> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
>> generated programmatically, see the comment below for more details. If you 
>> are only interested in a portion of the 3rd commit and would like to review 
>> it as a separate file, please comment here and I'll generate an individual 
>> webrev.
>> 
>> Due to the size of this PR, no attempt is made to update copyright years for 
>> any file to minimize unnecessary merge conflict.
>> 
>> Furthermore, since the default value of `java.security.manager` system 
>> property is now "disallow", most of the tests calling 
>> `System.setSecurityManager()` need to launched with 
>> `-Djava.security.manager=allow`. This is covered in a different PR at 
>> https://github.com/openjdk/jdk/pull/4071.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   default behavior reverted to allow

System.setSecurityManagerDirect looks a bit ugly now. Can this be renamed to 
implSetSecurityManager and avoid the line break in the  middle of the 
declaration?

The usage of System.err usage in setSecurityManager also needs to be 
re-examined as this will run arbitrary code when System.err can be changed. To 
fix this will require capturing the stream at startup (as was done with the 
illegal access logger). It's okay to integrate with what you have for the first 
push and we can fix this issue with System.err when the warning message is 
changed to the intended message.

-

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

Re: JEP 411 - Secure Java Distribution

[Ron Pressler]

That depends what you mean by “acceptable.” You can maintain a version of the 
Java SE spec that
contains the Security Manager forever. If you want to add the SecurityManager 
to a version of 
the spec that doesn’t contain it (or contains it but behaves in a degraded 
manner) then that
will not pass the JCK even when no Security Manager is installed, as the spec 
requires that
“spec-owned” APIs are neither removed *nor added*. 

You could create a JDK targeting such a version that still passes the JCK with 
an API *similar* to 
SecurityManager but that isn’t in any of the spec’s namespaces.

Of course, OpenJDK has an open-source license, and you can do what you want 
with that code in accordance
with the license even if you don’t pass the JCK. Adding *new* files that are 
not currently covered by
the license might have other issues if they interfere with the spec in some 
way, and you’ll need to
consult with the appropriate people.

— Ron


> On 1 Jun 2021, at 10:06, Peter Firmstone  wrote:
> 
> If a vendor were to continue supporting SecurityManager and was backporting 
> from OpenJDK, if it passes the JCK with SecurityManager disabled, that's 
> still acceptable right?
> 
> -- 
> Regards,
> Peter Firmstone
> 

JEP 411 - Secure Java Distribution

[Peter Firmstone]

If a vendor were to continue supporting SecurityManager and was 
backporting from OpenJDK, if it passes the JCK with SecurityManager 
disabled, that's still acceptable right?


--
Regards,
 
Peter Firmstone

Re: [External] : Re: JEP 411: Disable warning message with flag?

[Ron Pressler]

> On 31 May 2021, at 22:53, Chapman Flack  wrote:
> 
> 
> I am not sure what you are getting at with goal 3. Will the warning
> phone home?

No, but it will make users aware, and that awareness can be measured or
estimated.

> 
> I am also sort of wondering what's to become of some of the familiar
> known rules in the post-SM world. Will getClassLoader() become always
> allowed, whether the loader is an ancestor or not? Will
> setContextClassLoader() be a free-for-all? checkGuard()? ...?
> 
> Is it confidently believed that the JPMS encapsulation suffices for
> integrity enforcement and non-circumvention with all of those former
> familiar rules relaxed?
> 

Someone on the security team would be better placed to answer those questions
but it is not our goal to provide a sandbox in the JDK or replace the current
functionality of the Security Manager. We believe the module system, once 
encapsulation is fully made airtight, will significantly reduce the JDK’s 
attack surface area (and that of any other component the application chooses 
to encapsulate).

— Ron

Re: JEP 411: Disable warning message with flag?

[Alan Bateman]

On 31/05/2021 22:53, Chapman Flack wrote:

:
Thank you for that. One question I have been wondering about: do you have
any internally-tracked metrics that you can share about how many uses
of doPrivileged there are in the JDK codebase, and perhaps even a histogram
of the stack depth from the doPrivileged down to the affected
checked operation(s)?
The #usages of doPrivileged in the JDK is around 1200. The code is in 
the openjdk/jdk repo so you can get other stats if you want. If you do 
analysis to create a histogram on the number of frames walked when doing 
privileged operations then it would be useful to share.


-Alan