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