Looks good to me.
--Jamil
On 8/19/19 9:02 AM, Xuelei Fan wrote:
Hi,
Could I have the following code cleanup reviewed?
http://cr.openjdk.java.net/~xuelei/8228757/webrev.00/
It is trying to fail fast if unknown handshake type get requested.
Simple fix and hard to capture the fail point, no
Hi Jamil,
Thank you for the review.
On 5/10/2019 9:22 AM, Jamil Nimeh wrote:
This looks good to me. One question, more for my curiosity than
anything else: Is the way you loaded the appData array in the test code
done for any specific reason? Or did you just want to make sure you had
printa
This looks good to me. One question, more for my curiosity than
anything else: Is the way you loaded the appData array in the test code
done for any specific reason? Or did you just want to make sure you had
printable ASCII that wasn't all just the same character, so it looked
"random-ish"?
Hi Xuelei,
looks good to me as well.
best regards,
-- daniel
On 05/05/2019 16:18, Xuelei Fan wrote:
All good catches!
I made the update accordingly. Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.03/
Thanks,
Xuelei
On 5/3/2019 11:27 PM, Alan Bateman wrote:
O
On 05/05/2019 16:18, Xuelei Fan wrote:
All good catches!
I made the update accordingly. Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.03/
This update looks okay to me (an aternative for read is a nested
try/finally but what you have is okay).
-Alan
All good catches!
I made the update accordingly. Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.03/
Thanks,
Xuelei
On 5/3/2019 11:27 PM, Alan Bateman wrote:
On 04/05/2019 02:23, Xuelei Fan wrote:
Hi,
There is a surprise in the regression test. I made the update
On 04/05/2019 02:23, Xuelei Fan wrote:
Hi,
There is a surprise in the regression test. I made the update, and
now the testing passed. Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.01/
I assume you can save two volatile writes by not initializing isClosing
and
Oops, forgot to update the link of the webrev. It should be:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.02/
Xuelei
On 5/3/2019 6:23 PM, Xuelei Fan wrote:
Hi,
There is a surprise in the regression test. I made the update, and now
the testing passed. Here is the new webrev:
h
Hi,
There is a surprise in the regression test. I made the update, and now
the testing passed. Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8219991/webrev.01/
Thanks,
Xuelei
On 5/3/2019 6:45 AM, Daniel Fuchs wrote:
Hi Xuelei,
I agree this should fix the issue I was speak
Hi Xuelei,
I agree this should fix the issue I was speaking of.
Looks good to me - but maybe you'll want a second
reviewer from the security-dev team :-)
best regards,
-- daniel
On 03/05/2019 14:03, Xuelei Fan wrote:
Hi Daniel,
Good catch!
Here is a new webrev that's trying to address the p
Hi Daniel,
Good catch!
Here is a new webrev that's trying to address the problem.
http://cr.openjdk.java.net/~xuelei/8219991/webrev.01/
The isClosing field update is moving ahead, and a new filed hasDepleted
was added for threads competition.
The external test described in JDK-8219658 pass
Hi Xuelei,
I believe there is still a small window of opportunity
for which `readLockedDeplete();` will never be called.
The issue is in `deplete()`:
If tryLock() fails to lock at line
1046 if (readLock.tryLock()) {
then there's no guarantee that the reading
thread will not release
Looks good, pretty straightforward cleanup.
--Sean
On 2/5/19 1:44 PM, Xuelei Fan wrote:
Hi,
Could I have the update reviewed?
http://cr.openjdk.java.net/~xuelei/8217835/webrev.00/
With this update, the experimental FIPS 140 compliant mode is removed
from the SunJSSE provider. As the SunJ
Looks fine to me.
--Jamil
On 2/8/2019 8:16 AM, Xuelei Fan wrote:
Hi,
Please review this update:
http://cr.openjdk.java.net/~xuelei/8218580/webrev.00/
Objects.equals is case-sensitive, and should not be used to recognize
case-insensitive objects.
Trivial update, no new regression test.
You sure can! Looks good.
--Jamil
On 1/25/2019 12:02 PM, Xuelei Fan wrote:
Hi,
Can I have a code review for a trivial code cleanup?
https://bugs.openjdk.java.net/browse/JDK-8217820
Thanks,
Xuelei
diff -r 1262a93634c2
src/java.base/share/classes/sun/security/util/ECUtil.java
--- a/src/ja
Hi Xuelei, this looks good to me.
--Jamil
On 1/15/2019 7:45 AM, Xue-Lei Fan wrote:
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 ke
Hi Xuelei,
This is not my area of expertise - so I'm going to rephrase
what I understand (which may be wrong):
SSLEngineImpl.java:
This change makes sure that the SSLEngineResult::getStatus()
returns Status.CLOSED when closure has been initiated, even
if the engine is only half-closed at t
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 compatibility, I changed t
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 compatibility, I changed the
handshake status from NEED_WRAP back to
Code changes look good to me.
--Jamil
On 1/8/2019 3:00 PM, 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 case passed with the update.
The i
On 1/9/2019 6:10 AM, Chris Hegarty wrote:
Xuelei,
Is it possible to update the synopsis of this bug to better
reflect the underlying issue ( rather than one particular
symptom )?
Updated.
Also, it is possible to construct a small, non-HTTP related,
targeted test that verifies the fix?
T
Xuelei,
Is it possible to update the synopsis of this bug to better
reflect the underlying issue ( rather than one particular
symptom )?
Also, it is possible to construct a small, non-HTTP related,
targeted test that verifies the fix?
-Chris.
On 08/01/2019 23:00, Xue-Lei Fan wrote:
ping ...
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
connection is half-
On 12/17/18 5:26 PM, Xue-Lei Fan wrote:
On 12/17/2018 1:17 PM, Anthony Scarpino wrote:
It looks like in TransportContext.java:68, you had a mistype that
added "fa" to the end of a comment.
Oops, I will update it.
Also in fatal():267, did you plan to return the exception and have the
calling
On 12/17/2018 1:17 PM, Anthony Scarpino wrote:
It looks like in TransportContext.java:68, you had a mistype that added
"fa" to the end of a comment.
Oops, I will update it.
Also in fatal():267, did you plan to return the exception and have the
calling method throw the exception? As is, the
Hi Xuelei, comments in-line.
On 12/17/2018 12:11 PM, Xue-Lei Fan wrote:
On 12/17/2018 11:28 AM, Jamil Nimeh wrote:
Hi Xuelei, just a couple questions:
* SSLSocketImpl
o 611: Are you sure conContext.inputRecord should go into a
try-with-resources? As far as I can tell, the inhe
It looks like in TransportContext.java:68, you had a mistype that added
"fa" to the end of a comment.
Also in fatal():267, did you plan to return the exception and have the
calling method throw the exception? As is, the exception is never
return and fatal() continues to throw the exceptions.
On 12/17/2018 11:28 AM, Jamil Nimeh wrote:
Hi Xuelei, just a couple questions:
* SSLSocketImpl
o 611: Are you sure conContext.inputRecord should go into a
try-with-resources? As far as I can tell, the inheritence chain
is SSLSocketInputRecord->InputRecord and that direct
Hi Xuelei, just a couple questions:
* SSLSocketImpl
o 611: Are you sure conContext.inputRecord should go into a
try-with-resources? As far as I can tell, the inheritence chain
is SSLSocketInputRecord->InputRecord and that directly or by
extension implements the SSLReco
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
Just the complete the thread. Says "fatal() throws exception" is fine.
If this all only trying to make the compiler happy, you could just have
one "return null" at the bottom of the method. It is not necessary to
put a return after each fatal(). I will admit it could be less readable.
Or f
Hi Jamil,
Thanks for the code review.
Okay, I will go ahead with the comment "fatal() always throws, make the
compiler happy" for this update, and file a new bug and see if we could
make an improvement in the future.
Thanks,
Xuelei
On 12/14/2018 4:16 PM, Jamil Nimeh wrote:
Hi Xuelei, thanks
Hi Xuelei, 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 PM, Xue-L
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()
always throw
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()
always throw an exception (compiler is one of them), he ma
On 12/14/2018 12:26 PM, Anthony Scarpino wrote:
Other than my nit about the “make the compiler happy”, this all looks fine.
It makes sense to me. I will remove the comment while pushing.
For KeyUpdate, shouldn’t it never be null given the suite and protocol are
already known good? I have n
Other than my nit about the “make the compiler happy”, this all looks fine.
For KeyUpdate, shouldn’t it never be null given the suite and protocol are
already known good? I have not problem with the check to be cautious even if
it should never happen.
Tony
> On Dec 14, 2018, at 9:00 AM, Xue
My nit is I’d remove the “make compiler happy” comment. If this was something
for Xcomp, I’d understand, but this is standard return value requirement.
Otherwise I’m fine with this
Tony
> On Dec 14, 2018, at 11:49 AM, Jamil Nimeh wrote:
>
> Looks pretty good. I did have one question about a
Looks pretty good. I did have one question about a few of the methods
in KeyShareExtension and PreSharedKeyExtension, specifically where you
return nulls on error branches with the make-compiler-happy comments.
In those cases would it be a bit cleaner to set a byte[] variable to
null at the b
Looks good. My only question is whether the apiNote should be an
implNote instead since it refers to what the JDK Implementation does.
But either way seems ok.
--Sean
On 11/26/18 1:14 PM, Xue-Lei Fan wrote:
I made the update accordingly:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.0
On 11/26/18 4:14 PM, Xue-Lei Fan wrote:
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, i
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/
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/
I will update CSR when we come to an agreement.
On 11/16/2018 11:33 AM, Sean Mullan wrote:
122 * @apiNote Both the session timeout and cache
Thanks for the review, Jmail & Sean.
New webrev:
http://cr.openjdk.java.net/~xuelei/8210985/webrev.03/
I will update CSR when we come to an agreement.
On 11/16/2018 11:33 AM, Sean Mullan wrote:
122 * @apiNote Both the session timeout and cache size impact
performance
123 *
122 * @apiNote Both the session timeout and cache size impact
performance
123 * of future connections. It is not recommended to
use too big
124 * or too small timeout or cache size limit.
Applications should
125 * carefully weigh the limits and
Hi Xuelei,
A little wordsmithing, nit picky stuff (sorry for not seeing this earlier):
* @apiNote for setSessionCacheSize: The sentence beginning, "It is not
recommended to use too big..." needs a slight grammatical change
o Suggestion: "It is recommended that applications tune their
It's time to use the systemProperty tag as it is ready.
As we are already there, I also update the setSessionCacheSize() for
more clarification.
Please review both CSR and webrev:
https://bugs.openjdk.java.net/browse/JDK-8213577
http://cr.openjdk.java.net/~xuelei/8210985/webrev.02/
Th
On 11/15/18 3:37 PM, Xuelei Fan wrote:
Hi Sean,
Are you OK if we do it later? I'm waiting for the @systemProperty tag,
proposed within JDK-5076751. I will file a bug to use the tag for more
cleanup.
JDK-5076751 is completed and pushed to JDK 12, so you can use the new
tag now.
I think i
Hi Sean,
Are you OK if we do it later? I'm waiting for the @systemProperty tag,
proposed within JDK-5076751. I will file a bug to use the tag for more
cleanup.
Thanks,
Xuelei
On 11/15/2018 11:55 AM, Sean Mullan wrote:
This is a good opportunity to document the
javax.net.ssl.sessionCacheSi
This is a good opportunity to document the
javax.net.ssl.sessionCacheSize system property in the SSLSessionContext
API (and use the new @systemProperty tag) in an @implNote, for example:
/**
* Returns the size of the cache used for storing
* SSLSession objects grouped under this
On 11/14/2018 9:16 AM, Jamil Nimeh wrote:
Hi Xuelei,
The fix looks fine to me. I think it would be good to have an else
branch off of the check on line 205 so any < 0 value has a warning log
entry stating that an invalid value was detected and the cache is
getting set to DEFAULT_MAX_CACHE_SI
Hi Xuelei,
The fix looks fine to me. I think it would be good to have an else
branch off of the check on line 205 so any < 0 value has a warning log
entry stating that an invalid value was detected and the cache is
getting set to DEFAULT_MAX_CACHE_SIZE.
--Jamil
On 11/14/2018 8:59 AM, Xuele
Looks fine to me.
On 11/9/2018 8:43 AM, Xuelei Fan wrote:
Hi,
Could I get the following small change reviewed please?
http://cr.openjdk.java.net/~xuelei/8213694/webrev.00/
The test SSLSessionContextImpl/Timeout.java is running in the default
mode. As the test initializes the SSLContext wit
Looks good to me
Tony
On 10/29/2018 10:41 AM, Xuelei Fan wrote:
Hi,
Please review the update:
http://cr.openjdk.java.net/~xuelei/8212738/webrev.00/
The signature algorithm name should be ""ecdsa_secp521r1_sha512",
instead of "ecdsa_secp512r1_sha512".
No new regression test. Trivial
Ditto.
Brad
On 9/20/2018 1:03 PM, Jamil Nimeh wrote:
Looks good.
On 9/20/2018 1:02 PM, Xuelei Fan wrote:
Hi,
Please review this simple fix for SunJSSE debug log:
http://cr.openjdk.java.net/~xuelei/8210974/webrev.00/
The debug log for ClientHello message does not appear in JDK 12.
Trivia
Looks good.
On 9/20/2018 1:02 PM, Xuelei Fan wrote:
Hi,
Please review this simple fix for SunJSSE debug log:
http://cr.openjdk.java.net/~xuelei/8210974/webrev.00/
The debug log for ClientHello message does not appear in JDK 12.
Trivial update, no new regression test.
Thanks,
Xuelei
Looks ok to me too.
Brad
On 9/11/2018 8:43 PM, Jamil Nimeh wrote:
Looks good to me.
Thanks,
--Jamil
On 9/11/2018 7:22 PM, Xuelei Fan wrote:
Hi Jamil,
Would you please review the fix for the NPE issue:
http://cr.openjdk.java.net/~xuelei/8209916/webrev.00/
The issue may happen if the cli
Hi,
does the fix work if there is only one unknown named group ?
Not that the connection fails than with an better error text instead of
skiping the unknown group.
Gruß Thomas
On 12.09.2018 04:22:49, Xuelei Fan wrote:
Hi Jamil,
Would you please review the fix for the NPE issue:
http://cr
Looks good to me.
Thanks,
--Jamil
On 9/11/2018 7:22 PM, Xuelei Fan wrote:
Hi Jamil,
Would you please review the fix for the NPE issue:
http://cr.openjdk.java.net/~xuelei/8209916/webrev.00/
The issue may happen if the client supports a SunJSSE provider known
but not supported named group.
Yes I will take a look at this tonight.
--Jamil
Original message From: Xuelei Fan
Date: 9/11/18 7:22 PM (GMT-08:00) To: [email protected], Jamil
Nimeh Subject: Code Review Request, JDK-8209916 :
NPE in SupportedGroupsExtension
Hi Jamil,
Would you please revie
Looks good.
Brad
On 9/5/2018 11:01 AM, Xuelei Fan wrote:
Hi,
Please review:
http://cr.openjdk.java.net/~xuelei/8210334/webrev.00/
Simple update, no new regression test.
Thanks,
Xuelei
Looks fine
> On Sep 5, 2018, at 11:01 AM, Xuelei Fan wrote:
>
> Hi,
>
> Please review:
>http://cr.openjdk.java.net/~xuelei/8210334/webrev.00/
>
> Simple update, no new regression test.
>
> Thanks,
> Xuelei
Thanks for considering the idea
Tony
> On Aug 27, 2018, at 7:12 AM, Xuelei Fan wrote:
>
> Hi Tony,
>
> I thought about to limit the workaround to TLS 1.2 and prior version.
> However, just as you noticed that the implementation is not effective as it
> is needed to wait and check for the su
Hi Tony,
I thought about to limit the workaround to TLS 1.2 and prior version.
However, just as you noticed that the implementation is not effective as
it is needed to wait and check for the supported_versions extension if
it presents. As may make the workaround a lot complicated. I would
p
The change looks fine but I have a question about restricting it.
This sounds like a problem with servers using 1.2 or before, does it make sense
to throw an error for 1.3? I don’t like allowing buggy implementation to
continue because we will never be able to undo this workaround. It would be
Hi Brad,
Good points! Here is the updated webrev:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.06/
Please let me know if you have more comments by 11:30AM today.
Thanks,
Xuelei
On 8/13/2018 4:43 PM, Bradford Wetmore wrote:
Hi Xuelei,
> Let's use two to emphasize the behaviors:
> 1
Hi Xuelei,
> 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.
SSLEngine.java
--
159: Both sides (i.e. the peer) may not be a SSLEngine:
both the client and server applicati
Hi Jamil,
Thanks for review. One more step close to the integration.
On 8/13/2018 11:45 AM, Jamil Nimeh wrote:
Hi Xuelei,
* SSLSocket.java
o 134: Nit - You can remove the first "both" in this sentence
since you use it later with the input/output stream closure.
Let's use two
Hi Xuelei,
* SSLSocket.java
o 134: Nit - You can remove the first "both" in this sentence
since you use it later with the input/output stream closure.
Looks good to me otherwise.
--Jamil
On 8/13/2018 11:31 AM, Xue-Lei Fan wrote:
One more update:
http://cr.openjdk.java.net/~xuel
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
I'm good with the changes.
--Jamil
On 8/7/2018 5:24 PM, Xuelei Fan wrote:
Hi Jamil,
Thanks for comments. Here is the updated webrev:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.04/
Thanks,
Xuelei
On 8/7/2018 3:12 PM, Jamil Nimeh wrote:
Hi Xuelei, mostly small stuff:
* SSLEngine
I'm waiting for the CSR approval for JDK 11:
https://bugs.openjdk.java.net/browse/JDK-8208526
Thanks,
Xuelei
On 8/9/2018 6:50 AM, Norman Maurer wrote:
I there any idea when the patch will be merged and a release will be cut so we
can enable testing with JDK11 again for netty ?
On 31. Jul 20
I there any idea when the patch will be merged and a release will be cut so we
can enable testing with JDK11 again for netty ?
> On 31. Jul 2018, at 07:19, Norman Maurer wrote:
>
> After digging more this morning I noticed the test code did made some wrong
> assumptions which just worked out
Hi Jamil,
Thanks for comments. Here is the updated webrev:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.04/
Thanks,
Xuelei
On 8/7/2018 3:12 PM, Jamil Nimeh wrote:
Hi Xuelei, mostly small stuff:
* SSLEngineImpl.java
o 717: Nit, inbout --> inbound
* SSLEngineOutputRecord.java
Hi Xuelei, mostly small stuff:
* SSLEngineImpl.java
o 717: Nit, inbout --> inbound
* SSLEngineOutputRecord.java
o 162, 169: Nit, applicatoin --> application
o Same section: It looks like the "if" and "else if" clauses take
the same actions with the same message. Maybe jus
New webrev:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.03/
Thanks for a find of Tim Brooks, that the SSLEngine inbound/outbound
status is incorrect if closing during handshake. The above webrev is
trying to fix the problems. See more in the OpenJDK thread:
http://mail.openjdk.java.net
Update: http://cr.openjdk.java.net/~xuelei/8207009/webrev.02/
In webrev.01, the socket close may be blocked by super class close
synchronization. Updated the SSLSocketImpl.java to use handshake only
lock in the startHandshake() implementation.
Thanks,
Xuelei
On 8/1/2018 7:27 PM, Xuelei Fan
Update: http://cr.openjdk.java.net/~xuelei/8207009/webrev.01/
Integrated the fix for JDK-8208642, "Server initiated TLSv1.2
renegotiation fails if Java client allows TLSv1.3". SSLHandshake.java
is updated to use negotiated version so that TLS 1.2 HelloRequest is
acceptable in TLS 1.3 client s
After digging more this morning I noticed the test code did made some wrong
assumptions which just worked out of luck before. After fixing the test
everything passes now.
So +1 from me on the patch :)
Also sorry for the false-alarm.
Niorman
> On 30. Jul 2018, at 22:23, Xuelei Fan wrote:
>
Would you mind send me the debug log (-Djavax.net.debug=all) and the
exception stacks? The "renegotiation" in TLS 1.3 is different from TLS
1.2 and prior specifications. It would be helpful to me to find the
cause of the test failure.
Thanks,
Xuelei
On 7/30/2018 1:11 PM, Norman Maurer wrote
Hi Norman,
Thank you very much for the great help! Glad to know it works for you.
For the SSLEngine.setUSeClientMode() issues, the
SSLEngine.beginHandshake() spec is expected to throw
IllegalStateException if the client/server mode has not yet been set.
https://docs.oracle.com/javase/10/docs
Sorry but I just noticed we still have a another integration test failing which
tests that client SSL renegotiation is failing. This seems to be not the case
anymore with java11 + your patch (it was in ea20 tho).
https://github.com/netty/netty/blob/netty-4.1.28.Final/testsuite/src/main/java/io/n
Hey Xuelei,
I just re-ran our testsuite with your patch and everything pass except two
tests. After digging a bit I found that we needed to add explicit calls to
`SSLEngine.setUSeClientMode(false)` now in these test where we did not need to
do this before.
The tests in question are:
https://g
Will do and report back as soon as possible.
Thanks
Norman
> On 30. Jul 2018, at 19:57, Xuelei Fan wrote:
>
> Hi Norman,
>
> Would you mind look at the code I posted in the following thread:
> http://mail.openjdk.java.net/pipermail/security-dev/2018-July/017708.html
>
> I appreciate if you c
Hi Norman,
Would you mind look at the code I posted in the following thread:
http://mail.openjdk.java.net/pipermail/security-dev/2018-July/017708.html
I appreciate if you could have a test by the end of this week.
Note that with this update, a complete TLS connection should close both
inbound
Please let me know your concerns by the end of August 1st, 2018.
Thanks,
Xuelei
On 7/30/2018 9:59 AM, Xuelei Fan wrote:
Hi,
Please review the update for the TLS 1.3 half-close and synchronization
implementation:
http://cr.openjdk.java.net/~xuelei/8207009/webrev.00/
Unlike TLS 1.2 and
Just FYI… I tested this patch via the netty ssl tests and we no longer see the
class-cast-exception problems I reported before dso I think this solves the
issue.
That said we still encounter a few test-failures for tests that test behaviour
of closing outbound of the SSLEngine but I think thes
On 7/12/2018 5:06 PM, Bradford Wetmore wrote:
The code itself looks good, but I didn't see a regression tests. Is
this what JDK-8207174 will be for?
No, I did not have a regression test in this update. I asked the bug
reporter helped for the testing, and passed. However, we definitely
need
The code itself looks good, but I didn't see a regression tests. Is
this what JDK-8207174 will be for?
On 7/11/2018 11:53 PM, Alan Bateman wrote:
On 12/07/2018 05:47, Xuelei Fan wrote:
It's an interesting user case of the TrustManagerFactory and
KeyManagerFactory. The KeyManager or TrustMa
Hi Alan,
Yes, it is likely to introduce the issue again. We should consider a
regression test. I just filed a follow-up bug in the JBS:
https://bugs.openjdk.java.net/browse/JDK-8207174
Thanks,
Xuelei
On 7/11/2018 11:53 PM, Alan Bateman wrote:
On 12/07/2018 05:47, Xuelei Fan wrote:
Hi,
On 12/07/2018 05:47, Xuelei Fan wrote:
Hi,
Please review the update:
http://cr.openjdk.java.net/~xuelei/8207029/webrev.00/
It's an interesting user case of the TrustManagerFactory and
KeyManagerFactory. The KeyManager or TrustManager implementation may
be not implemented in the same prov
Hi Xuelei,
Thanks for your notification.
This is on my backlog again but don't have an ETA yet.
Kind regards,
Martin.-
On Thu, Jun 28, 2018 at 1:14 PM, Xuelei Fan wrote:
> Hi Martin,
>
> The TLS 1.3 implementation was integrated into the mainline.
>
> I know you have multiple contributions we
Hi Martin,
The TLS 1.3 implementation was integrated into the mainline.
I know you have multiple contributions were pending because of the
re-org of the JSSE implementation. Would you mind to check if your
design and implementation need some adjustment?
I may not reply for your contribution
Update: http://hg.openjdk.java.net/jdk/sandbox/rev/32a737f51e37
Xuelei
On 6/18/2018 8:48 AM, Xuelei Fan wrote:
CertificateMessage.java
X509TrustManagerImpl.java
X509KeyManagerImpl.java
-
These implementation has not consider the impact of RSASSA-PSS key type.
Xuelei
O
Update: http://hg.openjdk.java.net/jdk/sandbox/rev/63ab0dfe3dbb
Except the issues in this thread, this update also include changes to
set signature parameters after key initialization, so that the provider
of Signature instance is the same as its key provider if needed.
We have fixed the SunM
All good catches! I will push the changeset soon after the testing.
On 6/22/2018 11:18 AM, Jamil Nimeh wrote:
* DHKeyExchange.java
o 177-192: Am I missing something or does isRecovering get defined
as false and never gets set to true within the lifetime of the
variable?
* DHKeyExchange.java
o 177-192: Am I missing something or does isRecovering get defined
as false and never gets set to true within the lifetime of the
variable? Do we still need this?
o 178: Nit/typo: "recove" --> "recover"
o 207-210: Catching Exception seems really
Update: http://hg.openjdk.java.net/jdk/sandbox/rev/76025c6c6e29
On 6/21/2018 1:10 PM, Valerie Peng wrote:
> - line 175, the exception message seems not right, clientAlias is a
> private key entry with no cert chain stored.
Good catch!
> - line 56, instead of returning null, I wonder if we should
- line 175, the exception message seems not right, clientAlias is a
private key entry with no cert chain stored.
- line 56, instead of returning null, I wonder if we should throw
exception to catch unsupported type early.
That's all.
Valerie
On 5/25/2018 4:45 PM, Xuelei Fan wrote:
Hi,
I
Hi Xuelei,
- line 2, why is the copyright year changed from 2015 only to 2003 and
2018? Don't we normally preserve the first year and only update/add the
second year?
- line 110, instead of erroring out, I wonder if it's better to call
createPossessions(handshakeContext) and only error out if
1 - 100 of 1741 matches
Mail list logo
