On Thu, 15 Sep 2022 05:21:52 GMT, Daniel Jeliński wrote:
>> Speaking of MessageDigest.isEqual, we don't need constant time comparison
>> here. We could use Arrays.equals for some extra performance.
>
> Actually, never mind that. We need constant time comparison to avoid leaking
> information ab
On Wed, 14 Sep 2022 18:13:40 GMT, Xue-Lei Andrew Fan wrote:
>> Hi,
>>
>> Please review this simple code cleanup.
>>
>> The following checking for key in the makeSessionKey() method is redundant
>> as it the same checking has been performance before calling the method.
>>
>>
>> if (k
On Thu, 15 Sep 2022 05:09:06 GMT, Daniel Jeliński wrote:
>>> > Actually, NM, init still has to call MessageDigest.isEqual so eliminating
>>> > keys of invalid length before that is probably more efficient.
>>>
>>> The key should be valid for common cases. For valid key, it is more
>>> efficien
On Wed, 14 Sep 2022 21:04:58 GMT, Sean Mullan wrote:
>>> If the key is null, the following condition could bypass the checking, and
>>> result in NPE.
>>>
>>> ` if (!MessageDigest.isEqual(key, lastKey)) {`
>>>
>>> Although it is unlikely to happen as the caller should has already been
>>> che
On Wed, 7 Sep 2022 19:49:53 GMT, Mark Powers wrote:
> https://bugs.openjdk.org/browse/JDK-8291974
In the problem section part of the CSR, add a sentence on what's wrong with the
field.
In the spec section, please only keep the removal of the `testing` field. All
others are implementation chan
On Wed, 14 Sep 2022 23:15:29 GMT, Mark Powers wrote:
>> https://bugs.openjdk.org/browse/JDK-8291509
>
> Mark Powers has updated the pull request incrementally with one additional
> commit since the last revision:
>
> final comments from Max
Marked as reviewed by weijun (Reviewer).
-
On Wed, 14 Sep 2022 17:03:41 GMT, zzambers wrote:
> During testing I have found messages such as this are printed to stdout:
>
> sun.security.pkcs11.P11PSSSignature@6e1567f1: Calling C_SignUpdate
>
>
> **Problem:**
> Problem is in P11PSSSignature.engineUpdate method [1], which unconditionally
On Wed, 14 Sep 2022 18:41:42 GMT, Valerie Peng wrote:
>> During testing I have found messages such as this are printed to stdout:
>>
>> sun.security.pkcs11.P11PSSSignature@6e1567f1: Calling C_SignUpdate
>>
>>
>> **Problem:**
>> Problem is in P11PSSSignature.engineUpdate method [1], which uncon
> https://bugs.openjdk.org/browse/JDK-8291509
Mark Powers has updated the pull request incrementally with one additional
commit since the last revision:
final comments from Max
-
Changes:
- all: https://git.openjdk.org/jdk/pull/9972/files
- new: https://git.openjdk.org/jdk/pu
On Wed, 14 Sep 2022 19:44:52 GMT, Weijun Wang wrote:
>> Fixed. Throwing exception sooner rather than later.
>
> Some final comments.
>
> 1. Move the definition of `namedParams` inside the if block it is used and
> merge with the assignment line.
> 2. Change `if (ecParams == null)` to `else`, an
On Wed, 14 Sep 2022 19:52:34 GMT, Weijun Wang wrote:
>> Mark Powers has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> final comments from Max
>
> src/java.base/share/classes/sun/security/provider/certpath/ForwardBuilder.java
> line 572:
>
On Wed, 14 Sep 2022 20:49:34 GMT, Sean Mullan wrote:
>>> Actually, NM, init still has to call MessageDigest.isEqual so eliminating
>>> keys of invalid length before that is probably more efficient.
>>
>> The key should be valid for common cases. For valid key, it is more
>> efficient to have
On Wed, 14 Sep 2022 18:23:51 GMT, Xue-Lei Andrew Fan wrote:
>> Good point. Modified to use less checking.
>>
>> If the key is null, the following condition could bypass the checking, and
>> result in NPE.
>>
>> `if (!MessageDigest.isEqual(key, lastKey)) {
>> `
>>
>> Although it is un
On Wed, 14 Sep 2022 13:48:19 GMT, Mark Powers wrote:
>> https://bugs.openjdk.org/browse/JDK-8291509
>
> Mark Powers has updated the pull request incrementally with one additional
> commit since the last revision:
>
> final comments from Max
Almost done. I only have 2 comments on `ssl/ECDHCli
On Wed, 14 Sep 2022 13:50:55 GMT, Mark Powers wrote:
>> I did as you suggested and did a bit more with the indentation.
>
> I reworked it again. Take a look.
Looks good. Thanks.
-
PR: https://git.openjdk.org/jdk/pull/9972
On Wed, 14 Sep 2022 03:14:23 GMT, Mark Powers wrote:
>> src/java.base/share/classes/sun/security/ssl/SSLExtensions.java line 307:
>>
>>> 305: // extension_data
>>> length(2)
>>> 306: }
>>> 307: encodedLength +=
On Wed, 14 Sep 2022 03:14:33 GMT, Mark Powers wrote:
>> src/java.base/share/classes/sun/security/ssl/ECDHClientKeyExchange.java line
>> 289:
>>
>>> 287: }
>>> 288:
>>> 289: // Can't figure this out, bail.
>>
>> Maybe we should instead check if `namedParams` is null on
On Wed, 14 Sep 2022 17:03:41 GMT, zzambers wrote:
> During testing I have found messages such as this are printed to stdout:
>
> sun.security.pkcs11.P11PSSSignature@6e1567f1: Calling C_SignUpdate
>
>
> **Problem:**
> Problem is in P11PSSSignature.engineUpdate method [1], which unconditionally
On Wed, 14 Sep 2022 18:14:21 GMT, Xue-Lei Andrew Fan wrote:
>> Actually, NM, init still has to call MessageDigest.isEqual so eliminating
>> keys of invalid length before that is probably more efficient.
>
> Good point. Modified to use less checking.
>
> If the key is null, the following condit
On Wed, 14 Sep 2022 17:48:22 GMT, Sean Mullan wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/AESCrypt.java line 605:
>>
>>> 603: */
>>> 604: private void makeSessionKey(byte[] k) throws InvalidKeyException {
>>> 605: int ROUNDS = getRounds(k.length);
>>
> Hi,
>
> Please review this simple code cleanup.
>
> The following checking for key in the makeSessionKey() method is redundant as
> it the same checking has been performance before calling the method.
>
>
> if (k == null) {
> throw new InvalidKeyException("Empty key");
>
On Wed, 14 Sep 2022 16:03:48 GMT, Xue-Lei Andrew Fan wrote:
>> Hi,
>>
>> Please review this simple code cleanup.
>>
>> The following checking for key in the makeSessionKey() method is redundant
>> as it the same checking has been performance before calling the method.
>>
>>
>> if (k
On Wed, 14 Sep 2022 17:36:50 GMT, Sean Mullan wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - remove unused throws in comment
>> - remove unused throws
>
> src/java.base/share/classes/com/sun/crypto/provid
On Wed, 14 Sep 2022 16:03:48 GMT, Xue-Lei Andrew Fan wrote:
>> Hi,
>>
>> Please review this simple code cleanup.
>>
>> The following checking for key in the makeSessionKey() method is redundant
>> as it the same checking has been performance before calling the method.
>>
>>
>> if (k
During testing I have found messages such as this are printed to stdout:
sun.security.pkcs11.P11PSSSignature@6e1567f1: Calling C_SignUpdate
**Problem:**
Problem is in P11PSSSignature.engineUpdate method [1], which unconditionally
prints some debug information to stdout. Other prints in that cla
On Wed, 27 Jul 2022 13:14:39 GMT, Sean Coffey wrote:
> Add a JFR Event for `java.security.Provider.getService(String type, String
> algorithm)` calls.
Reviewer request: @seanjmullan @egahlin
-
PR: https://git.openjdk.org/jdk/pull/9657
Add a JFR Event for `java.security.Provider.getService(String type, String
algorithm)` calls.
-
Commit messages:
- Track successful getInstance calls only
- Merge branch 'master' into 8254711-jfr-jca
- Refactor and use of static Event on
- Merge branch 'master' into 8254711-jfr-j
On Wed, 14 Sep 2022 15:53:41 GMT, Weijun Wang wrote:
> I have no more comment.
Thank you Max for your time and input
-
PR: https://git.openjdk.org/jdk/pull/10045
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Wed, 14 Sep 2022 12:53:12 GMT, Alan Bateman wrote:
> I think you can insert a comma after "when it is the first entry in the
> stream"? I think that would make it a bit clearer that there are two cases.
Done
>
> Also I'm wondering if the paragraph should be split into two, meaning "When
>
On Wed, 14 Sep 2022 16:03:48 GMT, Xue-Lei Andrew Fan wrote:
>> Hi,
>>
>> Please review this simple code cleanup.
>>
>> The following checking for key in the makeSessionKey() method is redundant
>> as it the same checking has been performance before calling the method.
>>
>>
>> if (k
On Wed, 14 Sep 2022 13:25:41 GMT, Sean Mullan wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - remove unused throws in comment
>> - remove unused throws
>
> src/java.base/share/classes/com/sun/crypto/provid
> Hi,
>
> Please review this simple code cleanup.
>
> The following checking for key in the makeSessionKey() method is redundant as
> it the same checking has been performance before calling the method.
>
>
> if (k == null) {
> throw new InvalidKeyException("Empty key");
>
On Tue, 13 Sep 2022 14:58:11 GMT, Sean Coffey wrote:
> Ensure that security properties are only overridden if the override security
> properties file exists.
> Refactored some of the code in Security class initialization also.
> Extra test coverage for security properties file options.
Looks g
On Wed, 14 Sep 2022 10:31:41 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Wed, 14 Sep 2022 03:14:29 GMT, Mark Powers wrote:
>> src/java.base/share/classes/sun/security/ssl/SSLCipher.java line 70:
>>
>>> 68: @SuppressWarnings({"unchecked", "rawtypes"})
>>> 69: B_NULL("NULL", NULL_CIPHER, 0, 0, 0, 0, true, true,
>>> 70: new Map.Entry[] {
>>
>> It
> https://bugs.openjdk.org/browse/JDK-8291509
Mark Powers has updated the pull request incrementally with one additional
commit since the last revision:
final comments from Max
-
Changes:
- all: https://git.openjdk.org/jdk/pull/9972/files
- new: https://git.openjdk.org/jdk/pu
On Wed, 14 Sep 2022 05:58:10 GMT, Xue-Lei Andrew Fan wrote:
> Hi,
>
> Please review this simple code cleanup.
>
> The following checking for key in the makeSessionKey() method is redundant as
> it the same checking has been performance before calling the method.
>
>
> if (k == null)
On Wed, 14 Sep 2022 10:31:41 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Wed, 14 Sep 2022 02:22:17 GMT, Weijun Wang wrote:
> Only tiny comments for the last paragraph.
Thank you Max, I addressed the above
>
> That said, I have some questions on the other parts of this file:
>
> 1. In `getNextEntry`, the method spec says "If verification has been
> enabled,
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Tue, 13 Sep 2022 16:47:09 GMT, Xue-Lei Andrew Fan wrote:
> Is it a behavioral change that requires CSR approval?
I considered the CSR impact before logging this PR. This issue corrects a bug
where security properties had potential to be lost if an incorrect URL/resource
was specified for th
On Wed, 14 Sep 2022 05:58:10 GMT, Xue-Lei Andrew Fan wrote:
> Hi,
>
> Please review this simple code cleanup.
>
> The following checking for key in the makeSessionKey() method is redundant as
> it the same checking has been performance before calling the method.
>
>
> if (k == null)
43 matches
Mail list logo
