Code Review Request: TLS 1.3 full handshake (JDK-8196584)

Jamil Nimeh jamil.j.nimeh at oracle.com
Mon Mar 12 15:37:50 UTC 2018


This approach is only necessary when running a test that exercises a 
class with package-private visibility.  The test itself has to run from 
within sun/security/ssl in this case.  Originally some of my OCSP 
stapling tests for internal classes did this, but didn't use this exact 
approach and then a few months after committing it to JDK 9 another 
engineer modified those tests to use this new format.  I have to dig up 
the old emails to get the reasons why (maybe because of module support?  
I don't recall exactly), but ever since then I've followed this approach 
when I have to test class that only has package-private visibility.

I made the internal dir in case there were other package-private 
sun.security.ssl classes that we might want to exercise (OCSP stapling 
will have some, for instance), and this could be a catch-all for those 
tests.  If you prefer not to do it that way I'm happy to shuffle things 
around.

I'm not sure why you're having the problem creating the HKDF object in 
the test.  I don't have that problem when I run the test.  I thought at 
first that maybe I hadn't done an hg add on the HKDF.java file and then 
the commit would've skipped it, but I just checked the commit log this 
morning and it went back with everything else. Let's look at it together 
today.

--Jamil

On 03/11/2018 12:18 PM, Xuelei Fan wrote:
> test/jdk/sun/security/ssl/internal/TEST.properties
> test/jdk/sun/security/ssl/internal/TestRun.java
> test/jdk/sun/security/ssl/internal/java.base/sun/security/ssl/TestHkdf.java 
>
> ----------------------
> Why put the test in a internal/java.base/.. sub-directory?  I did not 
> get the point why it cannot be in the general 
> test/jdk/sun/security/ssl/HKDF directory.  Using the 
> "sun.security.ssl" in a test does not sound right to me, too.
>
>
> I also run into testing failure:
>
> TestHkdf.java:162: error: cannot find symbol
>         HKDF kdfHkdf;
>         ^
>   symbol:   class HKDF
>   location: class TestHkdf
>
>
> Xuelei
>
> On 2/22/2018 12:29 PM, Xuelei Fan wrote:
>> Updated to use package private HKDF implementation.
>>
>> webrev (based on JDK-8185576):
>>        http://cr.openjdk.java.net/~xuelei/8196584/webrev-step.01
>>
>> webrev (including JDK-8185576):
>>        http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.01
>>
>> Thanks,
>> Xuelei
>>
>> On 2/20/2018 11:57 AM, Xuelei Fan wrote:
>>> Hi,
>>>
>>> I'd like to invite you to review the TLS 1.3 full handshake 
>>> implementation.  I appreciate it if I could have feedback before 
>>> March 9, 2018.
>>>
>>> In the "JDK-8185576: New handshake implementation" [1] code review 
>>> around, I was trying to re-org the TLS handshaking implementation in 
>>> the
>>> SunJSSE provider.  If you had reviewed that part, you can start from 
>>> the following webrev that based on the update of JDK-8185576:
>>>      http://cr.openjdk.java.net/~xuelei/8196584/webrev-step.00
>>>
>>> If you would like start from earlier, here is the webrev that 
>>> contains the handshaking implementation re-org in JDK-8185576:
>>>      http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.00
>>>
>>>
>>> This changeset only implements the full handshake of TLS 1.3, rather 
>>> then a fully implementation of the latest TLS 1.3 draft [2].
>>>
>>> In this implementation, I removed:
>>> 1. the KRB5 cipher suite implementation.
>>> Please let me know if you are still using KRB5 cipher suite. I may 
>>> not add them back if no objections.
>>>
>>> 2. OCSP stapling.
>>> This feature will be added back later.
>>>
>>> Resumption and key update, and more features may be added later.
>>>
>>> Thanks & Regards,
>>> Xuelei
>>>
>>> [1]: 
>>> http://mail.openjdk.java.net/pipermail/security-dev/2017-December/016642.html 
>>>
>>> [2]: https://tools.ietf.org/html/draft-ietf-tls-tls13-24



More information about the security-dev mailing list