Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v3]

2021-05-27 Thread Weijun Wang
On Thu, 27 May 2021 17:42:56 GMT, Phil Race wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update FtpClient.java > > src/java.desktop/share/classes/java/awt/Component.java line 63

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

2021-05-27 Thread Weijun Wang
On Thu, 27 May 2021 20:16:25 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. >

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

2021-05-27 Thread Weijun Wang
ions 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 c

Re: RFR: 8267543: Post JEP 411 refactoring: security [v2]

2021-05-25 Thread Weijun Wang
On Tue, 25 May 2021 21:25:54 GMT, Sean Mullan wrote: > Update copyright dates to 2021, if applicable. Will do. Since this PR can only be integrated after its parent PR #4073 is integrated. I'll delay the copyright update by then. - PR: https://git.openjdk.java.net/jdk/pull/4172

Re: RFR: 8267543: Post JEP 411 refactoring: security [v2]

2021-05-25 Thread Weijun Wang
pyright year update commit before integration. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: one more change - Changes: - all: https://git.openjdk.java.net/jdk/pull/4172/files - new: https://git.openjdk.java.net/jd

Re: RFR: 8267543: Post JEP 411 refactoring: security [v2]

2021-05-25 Thread Weijun Wang
On Tue, 25 May 2021 13:44:30 GMT, Sean Mullan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> one more change > > src/java.security.jgss/share/classes/sun/security/jgss/krb5/Krb5Conte

RFR: 8267543: Post JEP 411 refactoring: security

2021-05-24 Thread Weijun Wang
For all modified files in #4073 having "security", "crypto", or "ssl" in their names. Codes are refactored to bring a `@SuppressWarning` annotation nearer to the deprecated API usage and also shrink the size of its target. - Depends on: https://git.openjdk.java.net/jdk/pull/4073

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v3]

2021-05-24 Thread Weijun Wang
On Mon, 24 May 2021 09:00:07 GMT, Daniel Fuchs wrote: > Thanks for taking in my suggestions for FtpClient. I have also reviewed the > changes to java.util.logging and JMX. I wish there was a way to handle > doPrivileged returning void more nicely. Maybe component maintainers will be > able to

Integrated: 8267184: Add -Djava.security.manager=allow to tests calling System.setSecurityManager

2021-05-24 Thread Weijun Wang
On Mon, 17 May 2021 17:51:36 GMT, Weijun Wang wrote: > Please review the test changes for [JEP > 411](https://openjdk.java.net/jeps/411). > > With JEP 411 and the default value of `-Djava.security.manager` becoming > `disallow`, tests calling `System.setSecurityM

Integrated: 8267584: The java.security.krb5.realm system property only needs to be defined once

2021-05-24 Thread Weijun Wang
On Sun, 23 May 2021 20:42:24 GMT, Weijun Wang wrote: > A system property only needs to be put in a `{@systemProperty}` tag where > it's first defined. For this one, it's > https://github.com/openjdk/jdk/blob/cf21c5ef116c136f09ac5be0d68f02553d0c7a70/src/java.security.jgss/share/clas

Re: RFR: 8266400: importkeystore fails to a password less pkcs12 keystore

2021-05-24 Thread Weijun Wang
On Wed, 19 May 2021 19:01:21 GMT, Hai-May Chao wrote: > Please review the fix to address keytool -importkeystore failure when > importing to a password-less PKCS12 keystore. Marked as reviewed by weijun (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4119

Re: RFR: 8267184: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v4]

2021-05-24 Thread Weijun Wang
ts}/DriverManagerPermissionsTests.java (93%) > rename test/jdk/javax/sql/{testng/test/rowset/spi => > permissionTests}/SyncFactoryPermissionsTests.java (95%) > ``` > > The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073. Weijun Wang has updated the pull request with

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

2021-05-24 Thread Weijun Wang
On Sun, 23 May 2021 16:35:43 GMT, Sean Mullan wrote: >> src/java.base/share/classes/java/lang/SecurityManager.java line 104: >> >>> 102: * method will throw an {@code UnsupportedOperationException}). If the >>> 103: * {@systemProperty java.security.manager} system property is set to >>> the

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

2021-05-24 Thread Weijun Wang
ions 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:

RFR: 8267584: The java.security.krb5.realm system property only needs to be defined once

2021-05-23 Thread Weijun Wang
A system property only needs to be put in a `{@systemProperty}` tag where it's first defined. For this one, it's https://github.com/openjdk/jdk/blob/cf21c5ef116c136f09ac5be0d68f02553d0c7a70/src/java.security.jgss/share/classes/javax/security/auth/kerberos/package-info.java#L41. -

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

2021-05-21 Thread Weijun Wang
On Fri, 21 May 2021 20:43:05 GMT, Phil Race wrote: > I haven't seen any response to this comment I made a couple of days ago and I > suspect it got missed > > > Weijun Wang has updated the pull request incrementally with one additional > > commit since the last rev

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v3]

2021-05-21 Thread Weijun Wang
ated method call to a new local > variable and add annotation on the declaration, and then assign that value to > the original l-value. > 3. Put declaration and assignment into a single statement if possible. > 4. Rewrite code to achieve #3 above. Weijun Wang has updated the pull request i

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v2]

2021-05-21 Thread Weijun Wang
On Fri, 21 May 2021 15:37:48 GMT, Daniel Fuchs wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> typo on windows > > src/java.base/share/classes/sun/net/ftp/impl/FtpClient.java line

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v2]

2021-05-21 Thread Weijun Wang
On Fri, 21 May 2021 15:35:05 GMT, Daniel Fuchs wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> typo on windows > > src/java.base/share/classes/sun/net/ftp/impl/FtpC

Re: RFR: 8267184: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v3]

2021-05-21 Thread Weijun Wang
ts}/DriverManagerPermissionsTests.java (93%) > rename test/jdk/javax/sql/{testng/test/rowset/spi => > permissionTests}/SyncFactoryPermissionsTests.java (95%) > ``` > > The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073. Weijun Wang has updated the pull request i

Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v2]

2021-05-21 Thread Weijun Wang
On Fri, 21 May 2021 18:26:48 GMT, Phil Race wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> adjust order of VM options > > test/jdk/java/awt/FontClass/CreateFont/fileaccess/FontFile

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v2]

2021-05-21 Thread Weijun Wang
ated method call to a new local > variable and add annotation on the declaration, and then assign that value to > the original l-value. > 3. Put declaration and assignment into a single statement if possible. > 4. Rewrite code to achieve #3 above. Weijun Wang has updated the pull request i

RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K

2021-05-20 Thread Weijun Wang
The code change refactors classes that have a `SuppressWarnings("removal")` annotation that covers more than 50KB of code. The big annotation is often quite faraway from the actual deprecated API usage it is suppressing, and with the annotation covering too big a portion it's easy to call other

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

2021-05-19 Thread Weijun Wang
On Thu, 20 May 2021 02:06:46 GMT, Weijun Wang wrote: >> Well .. as an enhancement (P3 or otherwise) I can see it being dropped and >> definitely if it misses the fork, >> and I don't get why you can't do it here. And if it isn't done the JEP isn't >> really ready. >&

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

2021-05-19 Thread Weijun Wang
On Wed, 19 May 2021 23:50:04 GMT, Phil Race wrote: >> I just made it P3 (P4 was the default value), and I will target it to 17 >> once JEP 411 is targeted 17. But I think it's probably not a good idea to >> include it inside *this* PR. There are some middle ground where it's >> debatable if a

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

2021-05-19 Thread Weijun Wang
On Wed, 19 May 2021 22:04:57 GMT, Phil Race wrote: >> Correct, there are ways to modify the code to make it more >> annotation-friendly. We thought about whether it's good to do it before >> adding the annotations or after it. Our decision now is to do it after >> because it will be more easy

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

2021-05-19 Thread Weijun Wang
On Wed, 19 May 2021 19:31:24 GMT, Phil Race wrote: >> This happens when a deprecated method is called inside a static block. The >> annotation can only be added to a declaration and here it must be the whole >> class. The call in this file is >> >> s =

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

2021-05-19 Thread Weijun Wang
On Wed, 19 May 2021 18:44:06 GMT, Weijun Wang wrote: >> Similar as the one above, it's because of >> >> static { >> // Don't lazy-read because every app uses invalidate() >> isJavaAwtSmartInvalidate = AccessController.doPrivileged( >>

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

2021-05-19 Thread Weijun Wang
On Wed, 19 May 2021 18:39:10 GMT, Weijun Wang wrote: >> src/java.desktop/share/classes/java/awt/Container.java line 97: >> >>> 95: * @since 1.0 >>> 96: */ >>> 97: @SuppressWarnings("removal") >> >> Same issue as with Component.

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

2021-05-19 Thread Weijun Wang
On Wed, 19 May 2021 18:26:25 GMT, Phil Race wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java > > src/java.desktop/share/c

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

2021-05-19 Thread Weijun Wang
ions 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: fixing aw

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

2021-05-19 Thread Weijun Wang
On Tue, 18 May 2021 21:21:45 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. >

Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v2]

2021-05-18 Thread Weijun Wang
ts}/DriverManagerPermissionsTests.java (93%) > rename test/jdk/javax/sql/{testng/test/rowset/spi => > permissionTests}/SyncFactoryPermissionsTests.java (95%) > ``` > > The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073. Weijun Wang has updated the pull request

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

2021-05-18 Thread Weijun Wang
ystem > 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. Weijun Wang has update

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

2021-05-18 Thread Weijun Wang
On Tue, 18 May 2021 18:38:52 GMT, Weijun Wang wrote: >> src/java.base/share/classes/java/security/AccessController.java line 877: >> >>> 875: @CallerSensitive >>> 876: public static T doPrivileged(PrivilegedExcept

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

2021-05-18 Thread Weijun Wang
On Tue, 18 May 2021 17:36:55 GMT, Alan Bateman 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. >>

Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager

2021-05-18 Thread Weijun Wang
On Tue, 18 May 2021 05:48:56 GMT, Alan Bateman wrote: > The changes look okay but a bit inconsistent on where -Djava...=allow is > inserted for tests that already set other system properties or other > parameters. Not a correctness issue, just looks odd in several places, e.g. > >

Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager

2021-05-18 Thread Weijun Wang
On Tue, 18 May 2021 11:12:00 GMT, Daniel Fuchs wrote: >> Please review the test changes for [JEP >> 411](https://openjdk.java.net/jeps/411). >> >> With JEP 411 and the default value of `-Djava.security.manager` becoming >> `disallow`, tests calling `System.setSecurityManager()` need >>

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

2021-05-17 Thread Weijun Wang
On Mon, 17 May 2021 18:23:41 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.c

RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

2021-05-17 Thread 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

RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager

2021-05-17 Thread Weijun Wang
Please review the test changes for [JEP 411](https://openjdk.java.net/jeps/411). With JEP 411 and the default value of `-Djava.security.manager` becoming `disallow`, tests calling `System.setSecurityManager()` need `-Djava.security.manager=allow` when launched. This PR covers such changes for

Re: RFR: 8266225: jarsigner is using incorrect security property to show weakness of certs [v2]

2021-05-07 Thread Weijun Wang
On Thu, 6 May 2021 20:57:13 GMT, Hai-May Chao wrote: >> Please review the change to jarsigner so it uses certpath security property >> in order to properly display the weakness of the certificate algorithms. > > Hai-May Chao has updated the pull request incrementally with one additional >

Re: RFR: 8265426: Update java.security to use instanceof pattern variable [v5]

2021-05-06 Thread Weijun Wang
On Thu, 6 May 2021 14:42:20 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the >> `java.security` package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull

Re: RFR: 8266225: jarsigner is using incorrect security property to show weakness of certs

2021-05-06 Thread Weijun Wang
On Thu, 6 May 2021 16:49:33 GMT, Hai-May Chao wrote: > Please review the change to jarsigner so it uses certpath security property > in order to properly display the weakness of the certificate algorithms. test/jdk/sun/security/tools/jarsigner/CheckSignerCertChain.java line 90: > 88:

Integrated: 8266293: Key protection using PBEWithMD5AndDES fails with "java.security.InvalidAlgorithmParameterException: Salt must be 8 bytes long"

2021-05-06 Thread Weijun Wang
On Fri, 30 Apr 2021 17:35:46 GMT, Weijun Wang wrote: > `PKCS12KeyStore` always uses a 20-byte salt in encryption but > PBEWithMD5AndDES only accepts 8-byte salt. With this code change, the salt > used for this algorithm will be 8 bytes. > > RFC 2898 only requires the salt t

Re: RFR: 8266293: Key protection using PBEWithMD5AndDES fails with "java.security.InvalidAlgorithmParameterException: Salt must be 8 bytes long" [v2]

2021-05-06 Thread Weijun Wang
On Thu, 6 May 2021 01:23:40 GMT, Valerie Peng wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> better comment > > src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java lin

Re: RFR: 8266293: Key protection using PBEWithMD5AndDES fails with "java.security.InvalidAlgorithmParameterException: Salt must be 8 bytes long" [v2]

2021-05-06 Thread Weijun Wang
n a PKCS #⁠12 keystore as we have > not defined its Object Identifier anywhere. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: better comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/3822/files - new: https:

Re: RFR: 8265426: Update java.security to use instanceof pattern variable [v3]

2021-05-06 Thread Weijun Wang
On Thu, 6 May 2021 11:52:15 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the >> `java.security` package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull

RFR: 8266293: Key protection using PBEWithMD5AndDES fails with "java.security.InvalidAlgorithmParameterException: Salt must be 8 bytes long"

2021-04-30 Thread Weijun Wang
`PKCS12KeyStore` always uses a 20-byte salt in encryption but PBEWithMD5AndDES only accepts 8-byte salt. With this code change, the salt used for this algorithm will be 8 bytes. RFC 2898 only requires the salt to be at least 8 bytes, but I don't intend to modify the `PBES1Core.java` to accept

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

2021-04-30 Thread Weijun Wang
On Wed, 28 Apr 2021 15:07:14 GMT, Weijun Wang wrote: > It's awkward that for a password-less pkcs12 keystore, `keytool -list` does > not prompt for a password but `keytool -list -storetype pkcs12` does. This pull request has now been integrated. Changeset: 48bb996a Author:Weijun Wan

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

2021-04-29 Thread Weijun Wang
On Wed, 28 Apr 2021 15:07:14 GMT, Weijun Wang wrote: > It's awkward that for a password-less pkcs12 keystore, `keytool -list` does > not prompt for a password but `keytool -list -storetype pkcs12` does. New commit pushed. When the file is opened the second time, uses a local va

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

2021-04-29 Thread Weijun Wang
> It's awkward that for a password-less pkcs12 keystore, `keytool -list` does > not prompt for a password but `keytool -list -storetype pkcs12` does. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: close stream car

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

2021-04-29 Thread Weijun Wang
On Wed, 28 Apr 2021 15:07:14 GMT, Weijun Wang wrote: > It's awkward that for a password-less pkcs12 keystore, `keytool -list` does > not prompt for a password but `keytool -list -storetype pkcs12` does. Test `sun/security/tools/keytool/KeyToolTest.java` failed on Windows. Looks like

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

2021-04-28 Thread Weijun Wang
It's awkward that for a password-less pkcs12 keystore, `keytool -list` does not prompt for a password but `keytool -list -storetype pkcs12` does. - Commit messages: - 8266220: keytool still prompt for store password on a password-less pkcs12 file if -storetype pkcs12 is specified

Re: RFR: 8265426: Update java.security to use instanceof pattern variable

2021-04-26 Thread Weijun Wang
On Mon, 26 Apr 2021 08:50:36 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my code for updating the code in the > `java.security` package to make use of the `instanceof` pattern variable? > > Kind regards, > Patrick Two comments: 1. Why not reuse the existing variable

Re: RFR: 8265426: Update java.security to use instanceof pattern variable

2021-04-26 Thread Weijun Wang
On Mon, 26 Apr 2021 08:50:36 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my code for updating the code in the > `java.security` package to make use of the `instanceof` pattern variable? > > Kind regards, > Patrick Two comments: 1. Why not reuse the existing variable

Integrated: 8258915: Temporary buffer cleanup

2021-04-22 Thread Weijun Wang
On Wed, 13 Jan 2021 21:32:07 GMT, Weijun Wang wrote: > Clean up temporary byte array, char array, and keyspec around keys and > passwords. > > No new regression test. This pull request has now been integrated. Changeset: f834557a Author: Weijun Wang URL:

Re: RFR: 8258915: Temporary buffer cleanup [v10]

2021-04-22 Thread Weijun Wang
> Clean up temporary byte array, char array, and keyspec around keys and > passwords. > > No new regression test. Weijun Wang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits: - Merge branch 'master'

Integrated: 8264277: java.xml.crypto module should be granted FilePermission and SocketPermission

2021-04-19 Thread Weijun Wang
On Tue, 30 Mar 2021 13:02:33 GMT, Weijun Wang wrote: > These permissions are needed so that the URIDereferencer is able to read data > from a file system or a network. As the test shows, you still have to grant > the same type of permission to your application. This pull request has

Re: RFR: 8264277: java.xml.crypto module should be granted FilePermission and SocketPermission [v3]

2021-04-19 Thread Weijun Wang
> These permissions are needed so that the URIDereferencer is able to read data > from a file system or a network. As the test shows, you still have to grant > the same type of permission to your application. Weijun Wang has updated the pull request with a new target base due t

Integrated: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params

2021-04-19 Thread Weijun Wang
On Wed, 24 Mar 2021 21:36:21 GMT, Weijun Wang wrote: > This enhancement contains the following code changes: > > 1. Create a new public API `javax/xml/crypto/dsig/spec/RSAPSSParameterSpec` > and remove the internal one. > 2. Update marshaling and unmarshal

Re: RFR: 8264277: java.xml.crypto module should be granted FilePermission and SocketPermission [v2]

2021-04-19 Thread Weijun Wang
> These permissions are needed so that the URIDereferencer is able to read data > from a file system or a network. As the test shows, you still have to grant > the same type of permission to your application. Weijun Wang has updated the pull request with a new target base due t

Integrated: 8265227: Move Proc.java from security/testlibrary to test/lib

2021-04-15 Thread Weijun Wang
On Wed, 14 Apr 2021 18:12:57 GMT, Weijun Wang wrote: > I'd like to move this tool to test/lib inside a proper named package. This pull request has now been integrated. Changeset: c70589c6 Author: Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/c70589c6 Stats: 119 li

Re: RFR: 8265227: Move Proc.java from security/testlibrary to test/lib [v3]

2021-04-14 Thread Weijun Wang
> I'd like to move this tool to test/lib inside a proper named package. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: do not call internal method - Changes: - all: https://git.openjdk.java.net/jdk/pull/3496/fi

Re: RFR: 8265227: Move Proc.java from security/testlibrary to test/lib [v2]

2021-04-14 Thread Weijun Wang
On Wed, 14 Apr 2021 20:48:07 GMT, Weijun Wang wrote: >> I'd like to move this tool to test/lib inside a proper named package. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > move to process and a test f

Re: RFR: 8265227: Move Proc.java from security/testlibrary to test/lib [v2]

2021-04-14 Thread Weijun Wang
> I'd like to move this tool to test/lib inside a proper named package. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: move to process and a test for itself - Changes: - all: https://git.openjdk.java.net/jdk/pull/3

Re: RFR: 8265227: Move Proc.java from security/testlibrary to test/lib

2021-04-14 Thread Weijun Wang
On Wed, 14 Apr 2021 18:12:57 GMT, Weijun Wang wrote: > I'd like to move this tool to test/lib inside a proper named package. Well, we can certainly rename it. It's only used by several tests. Maybe `ChildProcess`? And we can certainly move it to a different package. I know it has s

RFR: 8265227: Move Proc.java from security/testlibrary to test/lib

2021-04-14 Thread Weijun Wang
I'd like to move this tool to test/lib inside a proper named package. - Commit messages: - 8265227: Move Proc.java from security/testlibrary to test/lib Changes: https://git.openjdk.java.net/jdk/pull/3496/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3496=00 Issue:

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v7]

2021-04-13 Thread Weijun Wang
tands extra fields in `PSSParameterSpec` and is aware of the > defaults in both directions. > 3. Update `DOMSignedInfo` so that secure validation can restrict > `DigestMethod` used inside `RSAPSSParameterSpec` > 4. Tests Weijun Wang has updated the pull request incrementally with one additional

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v6]

2021-04-13 Thread Weijun Wang
On Tue, 13 Apr 2021 17:07:19 GMT, Sean Mullan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> spec clarification > > src/java.xml.crypto/share/classes/javax/xml/crypto/dsig/Si

Integrated: 8265138: Simplify DerUtils::checkAlg

2021-04-13 Thread Weijun Wang
On Tue, 13 Apr 2021 14:36:46 GMT, Weijun Wang wrote: > This fix makes `DerUtils::checkAlg` simple and clear, no more raw `Object` > and ambiguous string arguments. This pull request has now been integrated. Changeset: 9cd5400d Author:Weijun Wang URL: https://git.openjdk.ja

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v6]

2021-04-13 Thread Weijun Wang
tands extra fields in `PSSParameterSpec` and is aware of the > defaults in both directions. > 3. Update `DOMSignedInfo` so that secure validation can restrict > `DigestMethod` used inside `RSAPSSParameterSpec` > 4. Tests Weijun Wang has updated the pull request incrementally with one additional

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v5]

2021-04-13 Thread Weijun Wang
On Mon, 12 Apr 2021 15:25:09 GMT, Weijun Wang wrote: >> This enhancement contains the following code changes: >> >> 1. Create a new public API `javax/xml/crypto/dsig/spec/RSAPSSParameterSpec` >> and remove the internal one. >> 2. Update marshalin

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-04-13 Thread Weijun Wang
On Tue, 13 Apr 2021 14:22:47 GMT, Sean Mullan wrote: >> You are right that the overridable methods are elsewhere >> (`XMLSignatureFactory::newSignatureMethod` and >> `SignatureMethod::getParameterSpec`), but I still feel it a little strange >> to move the default parameter of one particular

RFR: 8265138: Simplify DerUtils::checkAlg

2021-04-13 Thread Weijun Wang
This fix makes `DerUtils::checkAlg` simple and clear, no more raw `Object` and ambiguous string arguments. - Commit messages: - 8265138: Simplify DerUtils::checkAlg Changes: https://git.openjdk.java.net/jdk/pull/3467/files Webrev:

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-04-12 Thread Weijun Wang
On Mon, 12 Apr 2021 17:29:55 GMT, Sean Mullan wrote: >> I added the new lines as `@implNote` and kept the old `@implSpec` there >> (since it's still a requirement for implementations). New commit pushed. CSR >> updated as well. > > Ok, I understand now. I think `@implSpec` (and probably the

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v5]

2021-04-12 Thread Weijun Wang
tands extra fields in `PSSParameterSpec` and is aware of the > defaults in both directions. > 3. Update `DOMSignedInfo` so that secure validation can restrict > `DigestMethod` used inside `RSAPSSParameterSpec` > 4. Tests Weijun Wang has updated the pull request incrementally with one additional

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-04-12 Thread Weijun Wang
On Mon, 12 Apr 2021 12:42:23 GMT, Sean Mullan wrote: >> src/java.xml.crypto/share/classes/javax/xml/crypto/dsig/spec/RSAPSSParameterSpec.java >> line 139: >> >>> 137: >>> 138: @Override >>> 139: public String toString() { >> >> Add specification. > > Actually, on second thought, I

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v4]

2021-04-12 Thread Weijun Wang
On Mon, 12 Apr 2021 13:23:44 GMT, Sean Mullan wrote: > The `@implSpec` looks good. I view the `@implNote` more like an `@apiNote` > though. API notes are for "[commentary, rationale, or examples pertaining to the API](https://bugs.openjdk.java.net/browse/JDK-8068562)". I'm not sure if

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params

2021-04-09 Thread Weijun Wang
On Wed, 24 Mar 2021 21:36:21 GMT, Weijun Wang wrote: > This enhancement contains the following code changes: > > 1. Create a new public API `javax/xml/crypto/dsig/spec/RSAPSSParameterSpec` > and remove the internal one. > 2. Update marshaling and unmarshal

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v4]

2021-04-09 Thread Weijun Wang
tands extra fields in `PSSParameterSpec` and is aware of the > defaults in both directions. > 3. Update `DOMSignedInfo` so that secure validation can restrict > `DigestMethod` used inside `RSAPSSParameterSpec` > 4. Tests Weijun Wang has updated the pull request incrementally with one additional

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v3]

2021-04-09 Thread Weijun Wang
On Fri, 9 Apr 2021 17:23:05 GMT, Sean Mullan wrote: >> src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/algorithms/implementations/SignatureBaseRSA.java >> line 588: >> >>> 586: >>> 587: public enum DigestAlgorithm { >>> 588: //SHA1("SHA-1",

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v3]

2021-04-09 Thread Weijun Wang
On Fri, 9 Apr 2021 16:44:07 GMT, Sean Mullan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> spec word change, no hashCode and equals, test change > > src/java.xml.crypto/share/clas

Integrated: 8264864: Multiple byte tag not supported by ASN.1 encoding

2021-04-08 Thread Weijun Wang
On Thu, 8 Apr 2021 01:06:47 GMT, Weijun Wang wrote: > This code change does not intend to support multiple byte tags. Instead, it > aims to fail more gracefully when such a tag is encountered. For `DerValue` > constructors from an encoding (type I), an `IOException` will be thr

Re: RFR: 8264864: Multiple byte tag not supported by ASN.1 encoding [v5]

2021-04-08 Thread Weijun Wang
ructors from tag and value (type > II), an `IllegalArgumentException` will be thrown. All existing type II > callers inside JDK use tag numbers smaller than 31. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: add offset info in

Re: RFR: 8264864: Multiple byte tag not supported by ASN.1 encoding [v3]

2021-04-08 Thread Weijun Wang
On Thu, 8 Apr 2021 17:18:50 GMT, Jamil Nimeh wrote: >> I don't want to go on reading the following bytes to find out what the >> intended tag number is, because that somehow shows I do understand the >> encoding _a lot_ but still don't want to support it (well, actually I only >> understand

Re: RFR: 8264864: Multiple byte tag not supported by ASN.1 encoding [v3]

2021-04-08 Thread Weijun Wang
On Thu, 8 Apr 2021 16:58:24 GMT, Jamil Nimeh wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update exception wordings > > src/java.base/share/classes/sun/security/util/Der

Re: RFR: 8264864: Multiple byte tag not supported by ASN.1 encoding [v3]

2021-04-08 Thread Weijun Wang
On Thu, 8 Apr 2021 15:53:10 GMT, Xue-Lei Andrew Fan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update exception wordings > > src/java.base/share/classes/sun/security/util/Der

Re: RFR: 8264864: Multiple byte tag not supported by ASN.1 encoding [v4]

2021-04-08 Thread Weijun Wang
ructors from tag and value (type > II), an `IllegalArgumentException` will be thrown. All existing type II > callers inside JDK use tag numbers smaller than 31. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: change

Re: RFR: 8264864: Multiple byte tag not supported by ASN.1 encoding [v2]

2021-04-08 Thread Weijun Wang
On Thu, 8 Apr 2021 03:46:07 GMT, Xue-Lei Andrew Fan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> make sure test fails before code change > > src/java.base/share/classes/sun/securi

Re: RFR: 8264864: Multiple byte tag not supported by ASN.1 encoding [v3]

2021-04-08 Thread Weijun Wang
ructors from tag and value (type > II), an `IllegalArgumentException` will be thrown. All existing type II > callers inside JDK use tag numbers smaller than 31. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: update

Re: RFR: 8260693: Provide the support for specifying a signer in keytool -genkeypair [v6]

2021-04-07 Thread Weijun Wang
On Thu, 8 Apr 2021 01:48:20 GMT, Hai-May Chao wrote: >> Please review the changes that adds the -signer option to keytool >> -genkeypair command. As key agreement algorithms do not have a signing >> algorithm, the specified signer's private key will be used to sign and >> generate a key

Re: RFR: 8260693: Provide the support for specifying a signer in keytool -genkeypair [v5]

2021-04-07 Thread Weijun Wang
On Thu, 8 Apr 2021 01:42:16 GMT, Hai-May Chao wrote: >> test/jdk/sun/security/tools/keytool/GenKeyPairSigner.java line 96: >> >>> 94: >>> 95: Certificate[] certChain = kstore.getCertificateChain("e1"); >>> 96: if (certChain.length != 2) { >> >> Try using `Asserts` class in

Re: RFR: 8260693: Provide the support for specifying a signer in keytool -genkeypair [v5]

2021-04-07 Thread Weijun Wang
On Thu, 8 Apr 2021 01:48:20 GMT, Hai-May Chao wrote: >> In testSignerJKS() has made sure that the new entry created with new key >> password can *only* be accessed when a correct key password is provided in >> order to retrieve the corresponding signer’s private key. > > The new entry

Re: RFR: 8264864: Multiple byte tag not supported by ASN.1 encoding [v2]

2021-04-07 Thread Weijun Wang
ructors from tag and value (type > II), an `IllegalArgumentException` will be thrown. All existing type II > callers inside JDK use tag numbers smaller than 31. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: make sure test

RFR: 8264864: Multiple byte tag not supported by ASN.1 encoding

2021-04-07 Thread Weijun Wang
This code change does not intend to support multiple byte tags. Instead, it aims to fail more gracefully when such a tag is encountered. For `DerValue` constructors from an encoding (type I), an `IOException` will be thrown since it's already in the throws clause. For constructors from tag and

Re: RFR: 8260693: Provide the support for specifying a signer in keytool -genkeypair [v5]

2021-04-07 Thread Weijun Wang
On Wed, 7 Apr 2021 23:17:53 GMT, Hai-May Chao wrote: >> Please review the changes that adds the -signer option to keytool >> -genkeypair command. As key agreement algorithms do not have a signing >> algorithm, the specified signer's private key will be used to sign and >> generate a key

Integrated: 8262389: Use permitted_enctypes if default_tkt_enctypes or default_tgs_enctypes is not present

2021-04-06 Thread Weijun Wang
On Thu, 25 Feb 2021 20:56:42 GMT, Weijun Wang wrote: > When default_tkt_enctypes or default_tgs_enctypes, use the value of > permitted_enctypes if it exists. > > Please also review the CSR at > https://bugs.openjdk.java.net/browse/JDK-8262391 and release n

Withdrawn: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params

2021-04-02 Thread Weijun Wang
On Wed, 24 Mar 2021 21:36:21 GMT, Weijun Wang wrote: > This enhancement contains the following code changes: > > 1. Create a new public API `javax/xml/crypto/dsig/spec/RSAPSSParameterSpec` > and remove the internal one. > 2. Update marshaling and unmarshal

Re: RFR: 8260693: Provide the support for specifying a signer in keytool -genkeypair [v4]

2021-04-02 Thread Weijun Wang
On Fri, 2 Apr 2021 04:03:50 GMT, Xue-Lei Andrew Fan wrote: >> Maybe we don't need to resolve it in this code change. If we look carefully >> at RFC 8410 Sections 10.1 and 10.2, it shows the X25519 certificate in 10.2 >> is using the signer's SKID in 10.1 as its own SKID and it has no AKID. >>

<    2   3   4   5   6   7   8   9   10   11   >