Re: Code Review Request, JDK-8213577, Update the default SSL session cache size to 20480

I made the update accordingly:   http://cr.openjdk.java.net/~xuelei/8210985/webrev.04/ Thanks, Xuelei On 11/19/2018 7:39 AM, Sean Mullan wrote: On 11/16/18 3:19 PM, Xuelei Fan wrote: Thanks for the review, Jmail & Sean. New webrev: http://cr.openjdk.java.net/~xuelei/8210985/webrev.03/

Code Review Request, JDK-8214321: Misleading code in SSLCipher

Hi, Please review this code cleanup in SSLCipher.java: http://cr.openjdk.java.net/~xuelei/8214321/webrev.00/ The code should be fine as readCipherGenerators.length and writeCipherGenerators.length are the same value in the implementation. However, it is misleading to use readCipherGenerator

Re: RFR 8210476: sun/security/mscapi/PrngSlow.java fails with Still too slow

Do you know, is there any other way except Cleaner and finalize() to clean up the allocated resources? I'm not very sure of the use of static Cleaner: 1. a daemon thread will run underlying. 2. the number of registered actions could be huge in some circumstances. I'm not very sure if it could b

Re: RFR [12]: 8214140: Remove TLS v1 and v1.1 from SSLContext required algorithms

Looks fine to me. I added myself as Reviewer of the CSR. Xuelei On 11/28/2018 11:30 AM, Sean Mullan wrote: Please review this change to remove TLS v1 and v1.1 from the SE required algorithms for SSLContext. These TLS protocols have various weaknesses and are no longer recommended and are bein

Re: RFR 8210476: sun/security/mscapi/PrngSlow.java fails with Still too slow

. For my concerns, if you go with Cleaner, we can open a new RFE for the tracking if we have a good idea in the future. Thanks, Xuelei On 11/30/2018 3:47 AM, Weijun Wang wrote: Hi Xuelei, and Ivan who replied in another mail, On Nov 29, 2018, at 4:31 AM, Xue-Lei Fan wrote: Do you know, is

Re: RFR 8210476: sun/security/mscapi/PrngSlow.java fails with Still too slow

on is now implemented as a named inner class. Thanks Max On Dec 1, 2018, at 2:03 AM, Xue-Lei Fan wrote: Hi Weijun, I think you made a significant improvement of the performance, with less timing and resources. I don't think my concerns are strong enough to prevent this fix from moving for

Re: 8214098: sun.security.ssl.HandshakeHash.T12HandshakeHash constructor check backwards.

Looks fine to me. Thanks, Xuelei On 12/4/2018 12:06 PM, Anthony Scarpino wrote: Hi, I need a code review for the TLS 1.2 check being backwards for HandshakeHash, which showed up with a clonable MessageDigest object. Since TLS 1.3 does not use the same scheme as 1.2, I removed the unnecessar

Re: RFR JDK-8214129: SSL session resumption/SNI with TLS1.2 causes StackOverflowError

Hi Jamil, For a defense in depth fix, as you are already there, I may suggest update two more places. ServerNameExtension.java: private CHServerNamesSpec(List serverNames) { this.serverNames = Collections.unmodifiableList(serverNames); } SSLSession

Re: RFR JDK-8214129: SSL session resumption/SNI with TLS1.2 causes StackOverflowError

Looks fine to me. Thanks, Xuelei On 12/6/2018 1:09 PM, Jamil Nimeh wrote: Hello everyone, Updates from Xuelei's comments have been folded into an updated webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8214129/webrev.02/ Thank you, --Jamil On 12/5/18 3:59 PM, Jamil Nimeh wrote: Hello a

Code Review Request, JDK-8209333 Socket reset issue for TLS 1.3 socket close

Hi, Please review the TLS 1.3 half-close issue in JDK. http://cr.openjdk.java.net/~xuelei/8209333/webrev.00/ While trying to duplex close a TLS connection upon the half-close policy, there might be pending receiving data in the closing side, and result in a TCP RST during closing. The TC

Re: RFR[12] JDK-8214520: [TEST_BUG] sun/security/mscapi/nonUniqueAliases/NonUniqueAliases.java failed with incorrect jtreg tags order

looks fine to me. thanks, xuelei On 12/11/2018 11:35 PM, sha.ji...@oracle.com wrote: Hi, When run this test on Windows, jtreg complains that '@library' must appear before first action tag. So, just adjust the tag positions. Issue: https://bugs.openjdk.java.net/browse/JDK-8214520 diff -r a61

Code Review Request, JDK-8214339 : SSLSocketImpl erroneously wraps SocketException

Hi, Please review the update: http://cr.openjdk.java.net/~xuelei/8214339/webrev.00/ In some cases, the SSLProtocolException or SSLHandshakeException may be thrown if the underlying socket run into problems. An application may depends on the exception class for further action, for example r

Code Review Request, JDK-8213782: NullPointerException in sun.security.ssl.OutputRecord.changeWriteCiphers

Hi, Could I have the fix reviewed? http://cr.openjdk.java.net/~xuelei/8213782/webrev.00/ The SSLCipher.createReadCipher() and createWriteCipher() could return null if the cipher is not supported or the cipher is not available for a certain protocol version. The caller should check the null

Re: Code Review Request, JDK-8213782: NullPointerException in sun.security.ssl.OutputRecord.changeWriteCiphers

, Xue-Lei Fan wrote: Hi, Could I have the fix reviewed? http://cr.openjdk.java.net/~xuelei/8213782/webrev.00/ The SSLCipher.createReadCipher() and createWriteCipher() could return null if the cipher is not supported or the cipher is not available for a certain protocol version. The c

Re: Code Review Request, JDK-8214339 : SSLSocketImpl erroneously wraps SocketException

that this review doesn't touch. --Jamil On 12/14/2018 8:14 AM, Xue-Lei Fan wrote: Hi, Please review the update:    http://cr.openjdk.java.net/~xuelei/8214339/webrev.00/ In some cases, the SSLProtocolException or SSLHandshakeException may be thrown if the underlying socket run into pro

Re: Code Review Request, JDK-8214339 : SSLSocketImpl erroneously wraps SocketException

On 12/14/2018 1:46 PM, Xue-Lei Fan wrote: Hi, The purpose of combination of the two lines together:     shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, "...");     return null;    // make the complier happy is actually for the reading of the code.  If one don't know the fatal(

Re: Code Review Request, JDK-8214339 : SSLSocketImpl erroneously wraps SocketException

ei, thanks for the clarification.  If you want to keep the structure as-is, then the comment "fatal() always throws, make the compiler happy" is fine.  I think that's a little more helpful to a future maintainer who might be new to the code. Thanks, --Jamil On 12/14/2018 1:56 P

Code Review Request JDK-8215443: The use of TransportContext.fatal() leads to bad coding style

Hi, Could I have the update reviewed? http://cr.openjdk.java.net/~xuelei/8215443/webrev.00/ The TransportContext.fatal() methods always throw exception. While the compiler does not aware of it, and may not happy without following a return statement. Currently, a lot never executable return

Re: Code Review Request, JDK-8209333 Socket reset issue for TLS 1.3 socket close

ping ... On 12/10/2018 1:14 PM, Xue-Lei Fan wrote: Hi, Please review the TLS 1.3 half-close issue in JDK.     http://cr.openjdk.java.net/~xuelei/8209333/webrev.00/ While trying to duplex close a TLS connection upon the half-close policy, there might be pending receiving data in the closing

Re: Code Review Request, JDK-8209333 Socket reset issue for TLS 1.3 socket close

oint! It's a really nice enhancement! I had pushed this template in another bug fix. Let's do it separately. I filed a new bug for tracking this enhancement. https://bugs.openjdk.java.net/browse/JDK-8215509 Thanks, Xuelei The rest looks OK to me. --Jamil On 12/17/2018 9:52 AM, X

Re: Code Review Request JDK-8215443: The use of TransportContext.fatal() leads to bad coding style

here is a case that now exception get thrown, there will be a compiling issue. Are you OK with it? Thanks, Xuelei Tony On 12/15/18 7:51 AM, Xue-Lei Fan wrote: Hi, Could I have the update reviewed?     http://cr.openjdk.java.net/~xuelei/8215443/webrev.00/ The TransportContext.fatal() metho

Code Review Request, JDK-8214418 HttpClient falls in running with 100% cpu usage after an error signalled on channel

Hi, Could I get the update reviewed? http://cr.openjdk.java.net/~xuelei/8214418/webrev.00/ The reproducing testing case passed with the update. The issue is caused by the handshake status "NEED_WRAP" while the connection is half-closed. An application may just call wrap() when the handshak

Re: [12] RFR 8215694: keytool cannot generate RSASSA-PSS certificates

sigAlg.equalsIgnoreCase("RSASSA-PSS"): Do you really want to ignore the case? I used to think that an algorithm name is case sensitive. Main.java:1445 minor, 4 more indent? AlgorithmId.java:1073-1091: I may prefer to use cached parameters (for both AlgorithmParameters and AlgorithmParameterS

Re: [12] RFR 8215694: keytool cannot generate RSASSA-PSS certificates

On 1/3/2019 2:10 AM, Weijun Wang wrote: On Jan 2, 2019, at 11:56 PM, Xue-Lei Fan wrote: sigAlg.equalsIgnoreCase("RSASSA-PSS"): Do you really want to ignore the case? I used to think that an algorithm name is case sensitive. getInstance(alg) is always case-insensitive. Hm, I

Re: Not possible to disable new TLS extensions for TLS 1.2 connections

Hi Amir, What's the certificate used in the connection? It looks like a certificate issue per the debug log: "unable to find valid certification path to requested target" Please feel free file a bug if the certificate is not restricted by the signature_algorithms and signature_algorithms_

Re: Code Review Request, JDK-8214418 HttpClient falls in running with 100% cpu usage after an error signalled on channel

ping ... Xuelei On 12/22/2018 9:20 AM, Xue-Lei Fan wrote: Hi, Could I get the update reviewed?    http://cr.openjdk.java.net/~xuelei/8214418/webrev.00/ The reproducing testing case passed with the update. The issue is caused by the handshake status "NEED_WRAP" while the connecti

Re: Code Review Request, JDK-8214418 half-closed SSLEnigne status may cause application dead loop

? There was an test update that covered the fix. Thanks, Xuelei -Chris. On 08/01/2019 23:00, Xue-Lei Fan wrote: ping ... Xuelei On 12/22/2018 9:20 AM, Xue-Lei Fan wrote: Hi, Could I get the update reviewed?     http://cr.openjdk.java.net/~xuelei/8214418/webrev.00/ The reproducing testing

Re: Code Review Request, JDK-8214418 HttpClient falls in running with 100% cpu usage after an error signalled on channel

On 1/9/2019 10:58 AM, Daniel Fuchs wrote: Hi Xuelei, On 22/12/2018 17:20, Xue-Lei Fan wrote: The issue is caused by the handshake status "NEED_WRAP" while the connection is half-closed. An application may just call wrap() when the handshake status is "NEED_WRAP". For comp

Code Review Request, JDK-8216045 The size of key_exchange may be wrong on FFDHE

Hi, Could I have the update reviewed? http://cr.openjdk.java.net/~xuelei/8216045/webrev.00/ While getting the encoded public key for DH key exchange, the leading zeros of the key are not trimmed and the key bit size is used for byte size. John helped to verify the fix with the infra testi

Re: Is TLS1.3 support missing the "certificate_authorities" extension?

Hi Andrew, If one point have more than one certificates, and one of the cert cannot be verified by the peer, there is an impact. Normally, I think the peer is configured to be able to validate the certificate. RFC 8466 marked this extension as a "MAY" level feature. Did you know any requir

Re: Is TLS1.3 support missing the "certificate_authorities" extension?

Hi Martin, Yes, we re-orged the code a lot for TLS 1.3. As you were already there, did you want to resume the work? I can sponsor for the code review. Thanks, Xuelei On 1/15/2019 6:16 AM, Martin Balao wrote: Hi, I was working on an implementation of the Certificate Authorities TLS extensi

Re: RFR: 8036608: Key.getAlgorithm link to standard algorithm names needs to be updated

Looks good to me. Xuelei - Original Message - From: sean.mul...@oracle.com To: security-dev@openjdk.java.net Sent: Monday, April 29, 2019 10:34:58 AM GMT -08:00 US/Canada Pacific Subject: Re: RFR: 8036608: Key.getAlgorithm link to standard algorithm names needs to be updated Ping ... do

Re: [RFR] JDK-8224997: ChaCha20-Poly1305 TLS cipher suite decryption throws ShortBufferException

Good catch! For the engineGetOutputSize() implementation, as the inputLen parameter is an external input, we may want to ensure it is not too big to integer overflow. The Math.addExact() was used to check the overflow issues. What do you think we keep using the Math.addExact() method, at line

Re: Request for comment: the session ticket protection scheme for distributed TLS sessions.

The PNG may be too large to open from some mail system. Here is the PDF version. BTW, I also made an update on the use of AEAD algorithm with additional data. Thanks, Xuelei distributed_credential_protection.pdf Description: Adobe PDF document > On Oct 23, 2020, at 8:58 AM, Xuelei Fan wr

Re: ECC Key Usage ignored

Hi Daniel, Would you mind file a bug for the tracking? Xuelei > On Oct 31, 2020, at 5:45 AM, Daniel Jeliński wrote: > > Verified that this behavior is still observed with JDK 16 EA 22. > > Client side code responsible for server certificate validation is located in > EndEntityChecker.checkTL

Re: Code Review Request JDK-8170329 New SSLSocket testing template

On 11/29/2016 5:22 AM, Sean Mullan wrote: On 11/27/16 7:43 AM, Xuelei Fan wrote: On 11/27/2016 6:04 PM, Wang Weijun wrote: This is not only a test update. No, I happened to find an implementation issue with the new test, so fix it altogether. The issue is that the simple validator (SimpleVal

Re: [9] RFR: JDK-8170245 [TEST_BUG] Cipher tests fail when running with unlimited policy

The update looks fine to me. Xuelei On 11/29/2016 4:58 PM, Valerie Peng wrote: Anyone has cycles to review this fix? Some cipher tests fail when running against unlimited crypto policy due to hardcoded checks and values. Changes are straight-forward. However, given the dependency between the

Re: Code Review Request JDK-8170329 New SSLSocket testing template

, but it may be worthy to have a sub-template for HTTPS client testing. What do you think if we address this enhancement in a new bug? Thanks, Xuelei Artem On 12/02/2016 11:23 AM, Xue-Lei Fan wrote: On 11/29/2016 5:22 AM, Sean Mullan wrote: On 11/27/16 7:43 AM, Xuelei Fan wrote: On 11/27/20

Re: Code Review Request JDK-8170329 New SSLSocket testing template

On 12/2/2016 4:34 PM, Artem Smotrakov wrote: Hi Xuelei, Please see inline. On 12/02/2016 03:53 PM, Xue-Lei Fan wrote: Let's whether Sean or Weijun can have free cycle for the review of this part. Yeah, that would be great. - Why did you remove Peer and Application interfaces? I

Re: Code Review Request JDK-8170329 New SSLSocket testing template

On 12/2/2016 5:53 PM, Artem Smotrakov wrote: Please see inline On 12/02/2016 05:25 PM, Xue-Lei Fan wrote: - Why did you remove Peer and Application interfaces? I think those interfaces make SSLSocketTemplate more flexible since it allows override doServerSide/doClientSide logic if necessary

Re: RFR 8209416: Refactoring GetPropertyAction calls in JGSS

Looks fine to me. Did you notice other places that might need similar update as well in security components? Thanks, Xuelei On 8/13/2018 3:24 AM, Weijun Wang wrote: Please take a review at http://cr.openjdk.java.net/~weijun/8209416/webrev.00/ This is all about refactoring "AccessContr

Re: Code Review Request, JDK-8207009 SSLEngine#closeInbound mentions SSLException when no close_notify is received

One more update: http://cr.openjdk.java.net/~xuelei/8207009/webrev.05/ It is desired to make a note in SSLSocket and SSLEngine specification, so that users have a good sense that an application should close the input and output stream always. Updated for SSLEngine.java and SSLSocket.java on

Re: Code Review Request, JDK-8207009 SSLEngine#closeInbound mentions SSLException when no close_notify is received

Let's use two to emphasize the behaviors: 1. both input and output streams should be closed in each side, and 2. both client and server should perform #1. Thanks, Xuelei Looks good to me otherwise. --Jamil On 8/13/2018 11:31 AM, Xue-Lei Fan wrote: One more update: http://cr.openjdk.j

Re: RFR 8209416: Refactoring GetPropertyAction calls in JGSS

18, at 11:33 PM, Weijun Wang wrote: On Aug 13, 2018, at 10:00 PM, Xue-Lei Fan wrote: Looks fine to me. Did you notice other places that might need similar update as well in security components? One "new GetPropertyAction()" at String property = AccessController.doPr

Re: Inconsistent SSLEngine behavior for closing outbound while in handshake in 11ea22

On 8/13/2018 2:12 PM, Bradford Wetmore wrote: An update. 1.  OK vs. BUFFER_UNDERFLOW may still be an issue.  It's not a big one, but potentially confusing as to when you will get either. Did you figure out when OK is returned, and when BUFFER_UNDERFLOW is returned? I think we have an improv

Re: Inconsistent SSLEngine behavior for closing outbound while in handshake in 11ea22

OK, I will check it. Thanks! Xuelei On 8/13/2018 3:15 PM, Bradford Wetmore wrote: No, I haven't had the time to determine when OK vs BU. Brad On 8/13/2018 2:32 PM, Xue-Lei Fan wrote: On 8/13/2018 2:12 PM, Bradford Wetmore wrote: An update. 1.  OK vs. BUFFER_UNDERFLOW may still

Re: Code Review Request, JDK-8207009 SSLEngine#closeInbound mentions SSLException when no close_notify is received

state is desired, the output stream should generally be closed before the input stream. Once an {@code SSLSocket} is closed, it is not reusable: a new {@code SSLSocket} must be created. Brad On 8/13/2018 11:50 AM, Xue-Lei Fan wrote: Hi Jamil, Thanks for review.  One more step close to the in

Re: RFR 8209416: Refactoring GetPropertyAction calls in JGSS

more review from me. Thanks, Xuelei Thanks Max [1] http://cr.openjdk.java.net/~weijun/8209416/webrev.01/interdiff.patch.html On Aug 14, 2018, at 3:24 AM, Xue-Lei Fan wrote: It's nice to fix them if you'd like to. Thanks, Xuelei On 8/13/2018 8:36 AM, Weijun Wang wrot

Re: RFR: 8206176: Remove the temporary tls13VN

Looks fine to me. Thanks! Xuelei On 8/14/2018 4:10 PM, Rajan Halade wrote: Please review this fix to remove "jdk.tsl13.version" system property we had as interim solution for testing TLSv1.3 implementation. Webrev: http://cr.openjdk.java.net/~rhalade/8206176/webrev.00/ Thanks, Rajan

Re: RFR: Disable all DES cipher suites

NoDesRC4CiphSuite.java -- Please move line 30-31 out of the test comment block. The two lines will be parsed as part of the run parameters. I would prefer to use othervm mode. Otherwise, once there is a test case does not run with othervm and changes the context, this tes

Re: RFR: Disable all DES cipher suites

ogger. http://cr.openjdk.java.net/~jnimeh/reviews/8208350/webrev.02 Thanks, --Jamil On 08/20/2018 09:01 AM, Xue-Lei Fan wrote: NoDesRC4CiphSuite.java -- Please move line 30-31 out of the test comment block.  The two lines will be parsed as part of the run parameters. I

Re: RFR11(s): 8207317: SSLEngine negotiation fail exception behavior changed from fail-fast to fail-lazy

SSLTransport.java - line 173, nice to have 4 more indent white spaces. SSLEngineImpl.java -- 942 // If it wasn't a RuntimeException/SSLException, need to wrap it. 943 if (!(exc instanceof RuntimeException) && 944 !(exc instanceof SSLException)) {

Re: CFV: New JDK Committer: Hai-May Chao

Vote: yes. Xuelei > On Jan 21, 2021, at 10:36 AM, sean.mul...@oracle.com wrote: > > I hereby nominate Hai-May Chao to JDK Committer. > > Hai-May is a member of the Java Security Libraries team at Oracle. She has > contributed 13 fixes [0][1] to the JDK which span a wide range of security > fu

Re: 8259886 : Improve SSL session cache performance and scalability

Hi Daniel, I would like to help you out. Did you have some numbers about the performance improvement of your evaluation? I had a draft re-write cache by using ConcurrentH

RFR [17]: JDK-8260310 Release Note: Configurable extensions with system properties

Hi, May I have the following release note reviewed: https://bugs.openjdk.java.net/browse/JDK-8260310 Thanks, Xuelei

Re: RFR [17]: JDK-8260310 Release Note: Configurable extensions with system properties

tted I think it reads a little more smoothly, at least to my eyes. * I'm not sure if we have any style guide rules concerning contractions in release notes, but I think changing "don't" -> "do not" also reads better for these kinds of things. --Jamil On

Re: fix(truststore): log about truststore not found prints every time the same value

Hi Kevin, I think you are done with this PR. I created the bug for you, and you linked the bug with the PR. The PR now is integrated. Xuelei On Feb 2, 2021, at 1:24 PM, Kevin Davin wrote:  Hi everyone, I'm contacting you because I've opened a PR to fix a bug, but it seems to be required

Re: Bug with TLSv1.3 session resumption (most likely caused by concurrency bug)

Forward to security-dev. Xuelei On Mar 4, 2021, at 6:11 AM, Norman Maurer mailto:norman.mau...@googlemail.com>> wrote: Hi there, I think I found a bug in the TLSv1.3 session cache implementation which sometimes can cause failures during session resumption. The cause of this sometimes show up

Re: TLS 1.3 Post-handshake authentication

Hi Arjan, Did you have a chance to read RFC 8740? Post-Handshake authentication in HTTP/2 is not allowed for TLS 1.3. Is there a concern for the use case you mentioned? Xuelei From: jdk-dev on behalf of arjan tijms Sent: Thursday, March 4, 2021 12:57 PM

Re: [External] : Re: TLS 1.3 Post-handshake authentication

Sent: Thursday, March 4, 2021 2:08 PM To: Xue-Lei Fan Cc: security-dev@openjdk.java.net Subject: [External] : Re: TLS 1.3 Post-handshake authentication Hi, On Thu, Mar 4, 2021 at 10:48 PM Xue-Lei Fan mailto:xuelei@oracle.com>> wrote: Hi Arjan, Did you have a chance to read RFC

Re: [External] : Re: TLS 1.3 Post-handshake authentication

wrote: Hi, On Fri, Mar 5, 2021 at 2:05 AM Xue-Lei Fan mailto:xuelei@oracle.com>> wrote: Does it mean that when switch to HTTP/2, the concern is not valid any longer? Or there is an alternative solution? Sorry for the questions, I know little about servlet. I'm trying