RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice

2022-04-12 Thread Daniel Jeliński
During TLS handshake, hundreds of constraints are evaluated to determine which 
cipher suites are usable. Most of the evaluations are performed using 
`HandshakeContext#algorithmConstraints` object. By default that object contains 
a `SSLAlgorithmConstraints` instance wrapping another `SSLAlgorithmConstraints` 
instance. As a result the constraints defined in `SSLAlgorithmConstraints` are 
evaluated twice.

This PR improves the default case; if the user-specified constraints are left 
at defaults, we use a single `SSLAlgorithmConstraints` instance, and avoid 
duplicate checks.

-

Commit messages:
 - Fix file attributes
 - Fix file attributes
 - Avoid wrapping default constraints
 - Add handshake bench

Changes: https://git.openjdk.java.net/jdk/pull/8199/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8199=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8284694
  Stats: 337 lines in 4 files changed: 336 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8199.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8199/head:pull/8199

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


Re: RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice

2022-04-12 Thread Daniel Jeliński
On Tue, 12 Apr 2022 11:28:12 GMT, Daniel Jeliński  wrote:

> During TLS handshake, hundreds of constraints are evaluated to determine 
> which cipher suites are usable. Most of the evaluations are performed using 
> `HandshakeContext#algorithmConstraints` object. By default that object 
> contains a `SSLAlgorithmConstraints` instance wrapping another 
> `SSLAlgorithmConstraints` instance. As a result the constraints defined in 
> `SSLAlgorithmConstraints` are evaluated twice.
> 
> This PR improves the default case; if the user-specified constraints are left 
> at defaults, we use a single `SSLAlgorithmConstraints` instance, and avoid 
> duplicate checks.

Results of the attached benchmark:
Before:

Benchmark (resume)  (tlsVersion)   Mode  Cnt Score 
Error  Units
SSLHandshake.doHandshake  true   TLSv1.2  thrpt5  1407.320 ± 
302.562  ops/s
SSLHandshake.doHandshake  true   TLS  thrpt5   391.037 ±  
13.014  ops/s
SSLHandshake.doHandshake false   TLSv1.2  thrpt5   280.003 ±  
69.273  ops/s
SSLHandshake.doHandshake false   TLS  thrpt5   233.401 ±   
9.371  ops/s


After:

Benchmark (resume)  (tlsVersion)   Mode  Cnt Score 
Error  Units
SSLHandshake.doHandshake  true   TLSv1.2  thrpt5  2267.325 ± 
119.800  ops/s
SSLHandshake.doHandshake  true   TLS  thrpt5   490.465 ±  
24.698  ops/s
SSLHandshake.doHandshake false   TLSv1.2  thrpt5   340.275 ±  
72.833  ops/s
SSLHandshake.doHandshake false   TLS  thrpt5   271.656 ±   
5.444  ops/s


The results show a nice double-digit improvement across the board.

-

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


Re: RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice

2022-04-12 Thread Claes Redestad
On Tue, 12 Apr 2022 11:28:12 GMT, Daniel Jeliński  wrote:

> During TLS handshake, hundreds of constraints are evaluated to determine 
> which cipher suites are usable. Most of the evaluations are performed using 
> `HandshakeContext#algorithmConstraints` object. By default that object 
> contains a `SSLAlgorithmConstraints` instance wrapping another 
> `SSLAlgorithmConstraints` instance. As a result the constraints defined in 
> `SSLAlgorithmConstraints` are evaluated twice.
> 
> This PR improves the default case; if the user-specified constraints are left 
> at defaults, we use a single `SSLAlgorithmConstraints` instance, and avoid 
> duplicate checks.

src/java.base/share/classes/sun/security/ssl/SSLAlgorithmConstraints.java line 
73:

> 71: 
> 72: static AlgorithmConstraints wrap(AlgorithmConstraints 
> userSpecifiedConstraints) {
> 73: if (userSpecifiedConstraints == DEFAULT) {

Just thinking out loud: It seems all this does when `userSpecifiedConstraints` 
is a `SSLAlgorithmConstraints` is force the `enableX509..` flag to `true`. So 
in addition to the obvious thing for `DEFAULT`, you could also return `DEFAULT` 
for `DEFAULT_SSL_ONLY`. Or more generally: if `userSpecifiedConstraints 
instanceof SSLAlgorithmConstraints` then you could either return 
`userSpecifiedConstraints` as-is if `enabledX509DisabledAlgConstraints` is 
`true` or else return a clone of it with `enabledX509DisabledAlgConstraints` 
set to `true`.

-

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


Java Cryptographic Extension Survey

2022-04-12 Thread Anthony Scarpino

Hello,

Java Cryptographic Extension (JCE) has been in Java SE for a long time 
and has made incremental changes over the years.  Looking forward, we 
would like to know more about how projects are using JCE and what 
changes, features, and API enhancements would be helpful for your projects.


Below is a survey that we hope you can spend 5 minutes to fill out.  If 
you have written or maintain code that uses the JCE API, then we would 
appreciate if you would complete this survey:


https://www.questionpro.com/t/AUzP7ZrFWv

The survey will be open through April 29.

Thank you,
Anthony Scarpino
OpenJDK Security Group
https://openjdk.java.net/groups/security/


Re: RFR: 8284553: Deprecate the DEFAULT static field of OAEPParameterSpec

2022-04-12 Thread Sean Mullan
On Tue, 12 Apr 2022 02:48:51 GMT, Michael StJohns  wrote:

> I think deprecating DEFAULT? is wrong.? RFC8017 still has this definition:
> 
> > RSAES-OAEP-params ::= SEQUENCE {
> > hashAlgorithm  [0] HashAlgorithm DEFAULT sha1,
> > maskGenAlgorithm   [1] MaskGenAlgorithm  DEFAULT mgf1SHA1,
> > pSourceAlgorithm   [2] PSourceAlgorithm  DEFAULT pSpecifiedEmpty
> > }
> 
> and DEFAULT is what you should be getting if you see an empty sequence in the 
> param field.? It's DEFAULT in ASN1 terms, not DEFAULT in terms of what you 
> should use going forward? to create signatures, and the ASN1 DEFAULT won't 
> change.
> 
> In any event, I can't find where RFC8017 says anything about deprecating the 
> defaults.? AFAICT, although there's general guidance to go away from SHA1,? 
> the math suggests that SHA1 is still sufficient for OAEP,? and there's no 
> guidance specific to OAEP's use of SHA1 that I can find other than the 
> requirement in NIST SP800-56B rev 2 to use "Approved Hash Functions" for 
> OAEP. If there's a section in 8017 that you're looking at for this guidance 
> that I missed, you may want to update your comment to point to it.

[https://www.rfc-editor.org/rfc/rfc8017#appendix-A.2.1](https://www.rfc-editor.org/rfc/rfc8017#appendix-A.2.1)
 (which defines the OAEP-params structure) says:

> o  hashAlgorithm identifies the hash function.  It SHALL be an
>   algorithm ID with an OID in the set OAEP-PSSDigestAlgorithms.  For
>   a discussion of supported hash functions, see [Appendix 
> B.1](https://www.rfc-editor.org/rfc/rfc8017#appendix-B.1).

[https://www.rfc-editor.org/rfc/rfc8017#appendix-B.1](https://www.rfc-editor.org/rfc/rfc8017#appendix-B.1)
 is a long section discussing hash functions, but I think this text (last two 
paragraphs) is most relevant:

>   There have also been advances in the cryptanalysis of SHA-1.
>Particularly, the results of Wang et al.  
> [[SHA1CRYPT](https://www.rfc-editor.org/rfc/rfc8017#ref-SHA1CRYPT)] (which 
> have
>been independently verified by M.  Cochran in his analysis 
> [[COCHRAN](https://www.rfc-editor.org/rfc/rfc8017#ref-COCHRAN)])
>on using a differential path to find collisions in SHA-1, which
>conclude that the security strength of the SHA-1 hashing algorithm is
>significantly reduced.  However, this reduction is not significant
>enough to warrant the removal of SHA-1 from existing applications,
>but its usage is only recommended for backwards-compatibility
>reasons.
> 
>To address these concerns, only SHA-224, SHA-256, SHA-384, SHA-512,
>SHA-512/224, and SHA-512/256 are RECOMMENDED for new applications.
>As of today, the best (known) collision attacks against these hash
>functions are generic attacks with complexity 2L/2, where L is the
>bit length of the hash output.  For the signature schemes in this
>document, a collision attack is easily translated into a signature
>forgery.  Therefore, the value L / 2 should be at least equal to the
>desired security level in bits of the signature scheme (a security
>level of B bits means that the best attack has complexity 2B).  The
>same rule of thumb can be applied to RSAES-OAEP; it is RECOMMENDED
>that the bit length of the seed (which is equal to the bit length of
>the hash output) be twice the desired security level in bits.

Also, this is a proposed deprecation ... there is no plan to remove this field. 
The main intention of this deprecation is to alert users that SHA-1 is being 
used as it may not be apparent from the "DEFAULT" name, and to make sure that 
is ok or to think about using something stronger.

@valeriepeng I recommend making the link more specific in the deprecation text 
to point to https://www.rfc-editor.org/rfc/rfc8017#appendix-B.1

-

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


Re: RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice

2022-04-12 Thread Claes Redestad
On Tue, 12 Apr 2022 11:28:12 GMT, Daniel Jeliński  wrote:

> During TLS handshake, hundreds of constraints are evaluated to determine 
> which cipher suites are usable. Most of the evaluations are performed using 
> `HandshakeContext#algorithmConstraints` object. By default that object 
> contains a `SSLAlgorithmConstraints` instance wrapping another 
> `SSLAlgorithmConstraints` instance. As a result the constraints defined in 
> `SSLAlgorithmConstraints` are evaluated twice.
> 
> This PR improves the default case; if the user-specified constraints are left 
> at defaults, we use a single `SSLAlgorithmConstraints` instance, and avoid 
> duplicate checks.

Marked as reviewed by redestad (Reviewer).

-

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


Re: RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice

2022-04-12 Thread Claes Redestad
On Tue, 12 Apr 2022 15:19:41 GMT, Daniel Jeliński  wrote:

>> src/java.base/share/classes/sun/security/ssl/SSLAlgorithmConstraints.java 
>> line 73:
>> 
>>> 71: 
>>> 72: static AlgorithmConstraints wrap(AlgorithmConstraints 
>>> userSpecifiedConstraints) {
>>> 73: if (userSpecifiedConstraints == DEFAULT) {
>> 
>> Just thinking out loud: It seems all this does when 
>> `userSpecifiedConstraints` is a `SSLAlgorithmConstraints` is force the 
>> `enableX509..` flag to `true`. So in addition to the obvious thing for 
>> `DEFAULT`, you could also return `DEFAULT` for `DEFAULT_SSL_ONLY`. Or more 
>> generally: if `userSpecifiedConstraints instanceof SSLAlgorithmConstraints` 
>> then you could either return `userSpecifiedConstraints` as-is if 
>> `enabledX509DisabledAlgConstraints` is `true` or else return a clone of it 
>> with `enabledX509DisabledAlgConstraints` set to `true`.
>
> While this is technically true, `SSLAlgorithmConstraints` is an internal 
> class, so it's very unlikely that we will ever get `SSLAlgorithmConstraints` 
> other than `DEFAULT` here.

Right, I see even `DEFAULT_SSL_ONLY` is only used statically in one place. 

So the patch is probably good enough. Out of scope here, but if these 
permits-calls are (somewhat) performance-sensitive and the `DEFAULT` object is 
likely the only instance of `SSLAlgorithmConstraints` we'll ever see then 
perhaps it should be a specialized implementation that avoid the always-null 
`userSpecifiedConstraints != null` and `peerSpecifiedConstraints != null` 
checks.

-

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


Re: RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice

2022-04-12 Thread Bradford Wetmore
On Tue, 12 Apr 2022 11:28:12 GMT, Daniel Jeliński  wrote:

> During TLS handshake, hundreds of constraints are evaluated to determine 
> which cipher suites are usable. Most of the evaluations are performed using 
> `HandshakeContext#algorithmConstraints` object. By default that object 
> contains a `SSLAlgorithmConstraints` instance wrapping another 
> `SSLAlgorithmConstraints` instance. As a result the constraints defined in 
> `SSLAlgorithmConstraints` are evaluated twice.
> 
> This PR improves the default case; if the user-specified constraints are left 
> at defaults, we use a single `SSLAlgorithmConstraints` instance, and avoid 
> duplicate checks.

Please make sure @XueleiFan has a chance to look at this before integrating.

-

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


RFR: 8284796: sun.security.ssl.Finished::toString misses a line feed in the message format pattern

2022-04-12 Thread John Jiang
The log for Finished message looks like the below,

"Finished": {
  "verify data": {
: ... ...
  }'}  // looks weird

because a line feed is missing in the format pattern.

""Finished": '{'\n" +
"  "verify data": '{'\n" +
"{0}\n" +
"  '}'" +  // a line feed is needed
"'}'",

-

Commit messages:
 - 8284796: sun.security.ssl.Finished::toString misses a line feed in the 
message format pattern

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

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


Re: RFR: JDK-8284112 Minor cleanup could be done in javax.crypto

2022-04-12 Thread Bradford Wetmore
On Tue, 12 Apr 2022 21:59:09 GMT, Mark Powers  wrote:

> JDK-8284112 Minor cleanup could be done in javax.crypto

src/java.base/share/classes/javax/crypto/CryptoPermission.java line 437:

> 435: // may be the best try.
> 436: return this.algParamSpec.equals(algParamSpec);
> 437: } else return !this.checkParam;

Please use the standard coding format.

} else { 
return !this.checkParam;
}

or

} 

return !this.checkParam;

-

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


Re: RFR: JDK-8284112 Minor cleanup could be done in javax.crypto

2022-04-12 Thread Bradford Wetmore
On Tue, 12 Apr 2022 21:59:09 GMT, Mark Powers  wrote:

> JDK-8284112 Minor cleanup could be done in javax.crypto

Can you file a bug to update the javax.crypto files to use proper javadoc for 
mentioned classes, e.g. < code> tags.

src/java.base/share/classes/javax/crypto/CryptoPermissions.java line 158:

> 156: 
> 157: /**
> 158:  * Checks if this object's PermissionCollection for permissions

Just FYI and not for this review, but this class should really be updated to 
use proper javadoc comment style, which is to tag all objects with < code 
>PermissionCollection< /code >

-

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


Re: RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice

2022-04-12 Thread Daniel Jeliński
On Tue, 12 Apr 2022 15:40:46 GMT, Claes Redestad  wrote:

>> While this is technically true, `SSLAlgorithmConstraints` is an internal 
>> class, so it's very unlikely that we will ever get `SSLAlgorithmConstraints` 
>> other than `DEFAULT` here.
>
> Right, I see even `DEFAULT_SSL_ONLY` is only used statically in one place. 
> 
> So the patch is probably good enough. Out of scope here, but if these 
> permits-calls are (somewhat) performance-sensitive and the `DEFAULT` object 
> is likely the only instance of `SSLAlgorithmConstraints` we'll ever see then 
> perhaps it should be a specialized implementation that avoid the always-null 
> `userSpecifiedConstraints != null` and `peerSpecifiedConstraints != null` 
> checks.

Thanks for reviewing!
These permits calls are generally performing sufficiently well; the problem is 
that they in turn end up calling 
[AlgorithmDecomposer#decomposeImpl](https://github.com/openjdk/jdk/blob/36b59f3814fdaa44c9c04a0e8d63d0ba56929326/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java#L53),
 which performs string splitting using a [pattern with negative 
look-behind](https://github.com/openjdk/jdk/blob/36b59f3814fdaa44c9c04a0e8d63d0ba56929326/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java#L43),
 which is pretty slow. But that's another story.

-

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


Withdrawn: JDK-6782021: It is not possible to read local computer certificates with the SunMSCAPI provider

2022-04-12 Thread Mat Carter
On Tue, 12 Apr 2022 16:55:28 GMT, Mat Carter  wrote:

> On Windows you can now access the local machine keystores using the strings 
> "Windows-MY-LOCALMACHINE" and "Windows-ROOT-LOCALMACHINE"; note the 
> application requires admin privileges.
> 
> "Windows-MY" and "Windows-ROOT" remain unchanged, however given these 
> original keystore strings mapped to the current user, I added  
> "Windows-MY-CURRENTUSER" and "Windows-ROOT-CURRENTUSER" so that a developer 
> can explicitly specify the current user location.  These two new strings 
> simply map to the original two strings, i.e. no duplication of code paths etc
> 
> No new tests added, keystore functionality and API remains unchanged, the 
> local machine keystore types would require the tests to run in admin mode
> 
> Tested on windows, passes tier1 and tier2 tests

This pull request has been closed without being integrated.

-

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


RFR: 6782021: add support for localmachine keystores on windows

2022-04-12 Thread Mat Carter
On Windows you can now access the local machine keystores using the strings 
"Windows-MY-LOCALMACHINE" and "Windows-ROOT-LOCALMACHINE"; note the application 
requires admin privileges.

"Windows-MY" and "Windows-ROOT" remain unchanged, however given these original 
keystore strings mapped to the current user, I added  "Windows-MY-CURRENTUSER" 
and "Windows-ROOT-CURRENTUSER" so that a developer can explicitly specify the 
current user location.  These two new strings simply map to the original two 
strings, i.e. no duplication of code paths etc

No new tests added, keystore functionality and API remains unchanged, the local 
machine keystore types would require the tests to run in admin mode

Tested on windows, passes tier1 and tier2 tests

-

Commit messages:
 - 6782021: add support for localmachine keystores on windows

Changes: https://git.openjdk.java.net/jdk/pull/8210/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8210=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-6782021
  Stats: 79 lines in 4 files changed: 66 ins; 0 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8210.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8210/head:pull/8210

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


Re: RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice

2022-04-12 Thread Daniel Jeliński
On Tue, 12 Apr 2022 13:38:17 GMT, Claes Redestad  wrote:

>> During TLS handshake, hundreds of constraints are evaluated to determine 
>> which cipher suites are usable. Most of the evaluations are performed using 
>> `HandshakeContext#algorithmConstraints` object. By default that object 
>> contains a `SSLAlgorithmConstraints` instance wrapping another 
>> `SSLAlgorithmConstraints` instance. As a result the constraints defined in 
>> `SSLAlgorithmConstraints` are evaluated twice.
>> 
>> This PR improves the default case; if the user-specified constraints are 
>> left at defaults, we use a single `SSLAlgorithmConstraints` instance, and 
>> avoid duplicate checks.
>
> src/java.base/share/classes/sun/security/ssl/SSLAlgorithmConstraints.java 
> line 73:
> 
>> 71: 
>> 72: static AlgorithmConstraints wrap(AlgorithmConstraints 
>> userSpecifiedConstraints) {
>> 73: if (userSpecifiedConstraints == DEFAULT) {
> 
> Just thinking out loud: It seems all this does when 
> `userSpecifiedConstraints` is a `SSLAlgorithmConstraints` is force the 
> `enableX509..` flag to `true`. So in addition to the obvious thing for 
> `DEFAULT`, you could also return `DEFAULT` for `DEFAULT_SSL_ONLY`. Or more 
> generally: if `userSpecifiedConstraints instanceof SSLAlgorithmConstraints` 
> then you could either return `userSpecifiedConstraints` as-is if 
> `enabledX509DisabledAlgConstraints` is `true` or else return a clone of it 
> with `enabledX509DisabledAlgConstraints` set to `true`.

While this is technically true, `SSLAlgorithmConstraints` is an internal class, 
so it's very unlikely that we will ever get `SSLAlgorithmConstraints` other 
than `DEFAULT` here.

-

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


RFR: JDK-6782021: It is not possible to read local computer certificates with the SunMSCAPI provider

2022-04-12 Thread Mat Carter
On Windows you can now access the local machine keystores using the strings 
"Windows-MY-LOCALMACHINE" and "Windows-ROOT-LOCALMACHINE"; note the application 
requires admin privileges.

"Windows-MY" and "Windows-ROOT" remain unchanged, however given these original 
keystore strings mapped to the current user, I added "Windows-MY-CURRENTUSER" 
and "Windows-ROOT-CURRENTUSER" so that a developer can explicitly specify the 
current user location. These two new strings simply map to the original two 
strings, i.e. no duplication of code paths etc

No new tests added, keystore functionality and API remains unchanged, the local 
machine keystore types would require the tests to run in admin mode

Tested on windows, passes tier1 and tier2 tests

-

Commit messages:
 - 6782021: add support for localmachine keystores on windows

Changes: https://git.openjdk.java.net/jdk/pull/8211/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8211=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-6782021
  Stats: 79 lines in 4 files changed: 66 ins; 0 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8211.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8211/head:pull/8211

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


Re: RFR: JDK-6782021: It is not possible to read local computer certificates with the SunMSCAPI provider [v2]

2022-04-12 Thread Mat Carter
> On Windows you can now access the local machine keystores using the strings 
> "Windows-MY-LOCALMACHINE" and "Windows-ROOT-LOCALMACHINE"; note the 
> application requires admin privileges.
> 
> "Windows-MY" and "Windows-ROOT" remain unchanged, however given these 
> original keystore strings mapped to the current user, I added  
> "Windows-MY-CURRENTUSER" and "Windows-ROOT-CURRENTUSER" so that a developer 
> can explicitly specify the current user location.  These two new strings 
> simply map to the original two strings, i.e. no duplication of code paths etc
> 
> No new tests added, keystore functionality and API remains unchanged, the 
> local machine keystore types would require the tests to run in admin mode
> 
> Tested on windows, passes tier1 and tier2 tests

Mat Carter has refreshed the contents of this pull request, and previous 
commits have been removed. Incremental views are not available.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8210/files
  - new: https://git.openjdk.java.net/jdk/pull/8210/files/91d58f38..4e165f66

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

  Stats: 79 lines in 4 files changed: 0 ins; 66 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8210.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8210/head:pull/8210

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


Re: Proposal: Extend Windows KeyStore support to include access to the local machine location

2022-04-12 Thread Mat Carter
Weijun

Here's a PR [1] if you would like to review and consider sponsoring

[1] https://github.com/openjdk/jdk/pull/8211


Cheers
Mat


Sent from Outlook


From: Wei-Jun Wang 
Sent: Monday, April 11, 2022 3:33 PM
To: Mat Carter 
Cc: Bernd Eckenfels ; security-dev@openjdk.java.net 

Subject: Re: Proposal: Extend Windows KeyStore support to include access to the 
local machine location

Added a comment and assigned the enhancement to you. Thanks.

--Weijun

> On Apr 11, 2022, at 5:02 PM, Mat Carter  wrote:
>
> Thanks, Weijun
>
> Let's move ahead with the two new strings while we consider read-only access. 
>  As the current assignee can you update the JBS issue [1] with what we've 
> agreed here.
>
> I have an implementation that I've been testing in 11u which can easily move 
> to tip; if you are happy for me to make the changes then please ack here and 
> re-assign the issue to me
>
> [1] 
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-6782021data=05%7C01%7CMatthew.Carter%40microsoft.com%7Cf46a1e0bd128483c980f08da1c0b40e6%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637853131938100906%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=%2FcdLRJAf8TyNNQjc7Pw9vDBnLNBZ%2BhIjQ17MDMPUqIg%3Dreserved=0
>
>
> Thanks
> Mat



Re: Proposal: Extend Windows KeyStore support to include access to the local machine location

2022-04-12 Thread Wei-Jun Wang
No problem. The code change looks fine to me but you will need to create a CSR. 
I'll add a comment in the PR.

Thanks,
Weijun

> On Apr 12, 2022, at 5:37 PM, Mat Carter  wrote:
> 
> Weijun
> 
> Here's a PR [1] if you would like to review and consider sponsoring
> 
> [1] https://github.com/openjdk/jdk/pull/8211
> 
> 
> Cheers
> Mat
> 
> Sent from Outlook
> From: Wei-Jun Wang 
> Sent: Monday, April 11, 2022 3:33 PM
> To: Mat Carter 
> Cc: Bernd Eckenfels ; security-dev@openjdk.java.net 
> 
> Subject: Re: Proposal: Extend Windows KeyStore support to include access to 
> the local machine location
>  
> Added a comment and assigned the enhancement to you. Thanks.
> 
> --Weijun
> 
> > On Apr 11, 2022, at 5:02 PM, Mat Carter  
> > wrote:
> > 
> > Thanks, Weijun
> > 
> > Let's move ahead with the two new strings while we consider read-only 
> > access.  As the current assignee can you update the JBS issue [1] with what 
> > we've agreed here.
> > 
> > I have an implementation that I've been testing in 11u which can easily 
> > move to tip; if you are happy for me to make the changes then please ack 
> > here and re-assign the issue to me
> > 
> > [1] 
> > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-6782021data=05%7C01%7CMatthew.Carter%40microsoft.com%7Cf46a1e0bd128483c980f08da1c0b40e6%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637853131938100906%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=%2FcdLRJAf8TyNNQjc7Pw9vDBnLNBZ%2BhIjQ17MDMPUqIg%3Dreserved=0
> > 
> > 
> > Thanks
> > Mat
> 



RFR: JDK-8284112 Minor cleanup could be done in javax.crypto

2022-04-12 Thread Mark Powers
JDK-8284112 Minor cleanup could be done in javax.crypto

-

Commit messages:
 - second iteration
 - Merge
 - first iteration

Changes: https://git.openjdk.java.net/jdk/pull/8214/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8214=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8284112
  Stats: 300 lines in 37 files changed: 9 ins; 79 del; 212 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8214.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8214/head:pull/8214

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


Re: RFR: JDK-6782021: It is not possible to read local computer certificates with the SunMSCAPI provider

2022-04-12 Thread Weijun Wang
On Tue, 12 Apr 2022 19:03:40 GMT, Mat Carter  wrote:

> On Windows you can now access the local machine keystores using the strings 
> "Windows-MY-LOCALMACHINE" and "Windows-ROOT-LOCALMACHINE"; note the 
> application requires admin privileges.
> 
> "Windows-MY" and "Windows-ROOT" remain unchanged, however given these 
> original keystore strings mapped to the current user, I added 
> "Windows-MY-CURRENTUSER" and "Windows-ROOT-CURRENTUSER" so that a developer 
> can explicitly specify the current user location. These two new strings 
> simply map to the original two strings, i.e. no duplication of code paths etc
> 
> No new tests added, keystore functionality and API remains unchanged, the 
> local machine keystore types would require the tests to run in admin mode
> 
> Tested on windows, passes tier1 and tier2 tests

Thanks a lot for the quick contribution! I'll definitely be happy to sponsor it.

Since new keystore types are created, this needs a CSR. Go to 
https://bugs.openjdk.java.net/browse/JDK-6782021, click "More" and at the end 
there is a "Create CSR". Basically you talk about what these new keystores are 
and why they are useful. The scope is JDK and I assume the compatibility risk 
is low. There is no spec change but you can suggest new entries in the "JDK 
Providers Documentation" 
(https://docs.oracle.com/en/java/javase/18/security/oracle-providers.html#GUID-4F1737D6-1569-4340-B140-678C70E63CD5).
 You can also add a label like `noreg-other` to the bug and add a comment 
explaining what manual tests can be added, how to run it, and what the expected 
output is.

-

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