RFR (RFE-13): JDK-8224520: Support X25519 and X448 in TLS

[Bradford Wetmore]

Updated webrev:

     http://cr.openjdk.java.net/~wetmore/8171279/webrev.01/

Xuelei wrote:

 > ECDHClientKeyExchange.java
 > --------------------------
 > The enum builtin method valueOf(String) is used.  There is not problem
 > here. But as requires the enum name in NamedGroup is exactly the same as
 > the name defined in NamedParameterSpec.  It might be a potential risk
 > for future update of the names.

Unfortunately, all I have available is the key.getAlgorithm() and the 
key.getParams() String from the NamedParameterSpec at this point, so 
I've switched to NamedGroup.nameOf(String) which is what other places in 
the code do.

 > ECDHKeyExchange.java
 > --------------------
 >   471                         break search;
 >
 > The use of break label branching makes me a little bit nervous.  I'm
 > fine if you like it.

This is one of the rare circumstances where this construct is 
appropriate, as the alternative is harder to read.  Will leave as is for 
now.

Sean Mullan wrote:
> Just noticed a couple things so far:

Is there more coming?  I plan to putback tomorrow to make RDP1.  We can 
do an RFE during RDP1->RDP2 if you find something.

> * CipherSuite.java
> 
> 1139                 this.allowed = JsseJce.ALLOW_ECC;
> 
> Any reason why you don't just pass this in as the value of the allowed 
> parameter? Then you don't need to iterate over the array.

Done.

> * XECParameters.java
> 
> 2  * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
> 
> Should retain 2018.

Missed this, it now says 2018, 2019,

Brad


> --Sean
> 
> On 6/6/19 4:27 PM, Bradford Wetmore wrote:
>> Good morning/afternoon/evening/night,
>>
>> This RFE adds TLS protocol versions 1->1.3 support for the x25519/x448 
>> curves in the SunJSSE provider.  These algorithms are preferred by 
>> many of the major browsers for their efficiency and security properties.
>>
>> This work is the natural extension of JDK-8181595/JEP 324[2] which 
>> added the RFC 7748 [1] Curve25519 and Curve448 (ECDH) to the Java 
>> Crypto Mechanism in JDK 11.
>>
>> RFE:     https://bugs.openjdk.java.net/browse/JDK-8171279
>> CSR:     https://bugs.openjdk.java.net/browse/JDK-8224520
>>
>> Webrev:  http://cr.openjdk.java.net/~wetmore/8171279/webrev.00/
>>
>> There is a fair amount of change, so the webrevs are hard to follow in 
>> spots.  I'll introduce many of the implementation changes here:
>>
>> 1.  The JCE design choice of using a parallel set of APIs and Names 
>> ECPublicKey/XECPublicKey, KeyExchange.getInstance("EC"/"XDH"), etc. 
>> for these curves made things more challenging than expected.
>>
>> 2.  The key agreement key derivation code was identically replicated 3 
>> times (FFDHE/ECDHE/XDH), so this was consolidated into KAKeyDerivation.
>>
>> 3.  NamedGroups were broken out of SupportedGroupsExtension.
>>
>> 3a.  Added a function table to do NamedGroup operations in an 
>> algorithm-independent and consistent fashion for FFDHE/ECDHE/XDH. 
>> Things like checking Constraints, getting parameters, 
>> encoding/decoding Possession keys, creating possession, and whether 
>> the algorithms are available.
>>
>> For example, given a negotiated group:
>>
>>      namedGroup->encodePossessionPublicKey->Functions->
>>          ((XDHKeyExchange.XDHEPossession)poss).encode()
>>
>> Both the ECDHE/XDH functions call into the existing ECDHE code, as 
>> they use some of the same underlying structures/messages, which needed 
>> to be updated a bit to be algorithm independent.  This is for TLSv1->1.3.
>>
>> The FFDHE functions for TLSv1.3 also call into the existing DH code, 
>> but I did not update the existing FFDHE code for TLSv1-1.2 to use the 
>> function tables since it was not necessary.  If reviewers feel this is 
>> desired, I can do this as an enhancement later.
>>
>> 4.  Added NamedGroupPossession/NamedGroupCredentials interfaces to 
>> make code cleaner/less error prone.
>>
>> 5.  The ciphersuite KeyExchanges (K_EC*_*) were updated to reflect 
>> that either ECDHE/XDH can be used.
>>
>> 6.  Create a new ECDHKeyExchange.ECDHEXDHKAGenerator class to handle 
>> creating TLSv1-1.2 KeyDerivations, based on whether it was ECDHE/XDH.
>>
>> 7. RFC 8422 (Appendix B) deprecated/removed the TLS_ECDH_* 
>> ciphersuites.   Our KeyManager APIs currently do not allow for 
>> selecting specific curve entries.  I've made a best effort for 
>> supporting client-side ECDH, but we won't support server-side ECDH at 
>> this point.  TBD if we'll add support as API changes will be 
>> necessary, and not be worth the time if no one should/will be using ECDH.
>>
>> 8.  Is there a common byte array reverse function?  Seems so simple, 
>> but seems to be duplicated in different modules/packages.
>>
>> 9.  A few minor tweaks that helped code readability (typos/comments).  ;)
>>
>> Thanks to Adam Petcher, who started work on this project last year.  I 
>> took several of his ideas in my final implementation.
>>
>> Thanks,
>>
>> Brad
>>
>> [1] https://tools.ietf.org/html/rfc7748
>> [2] https://bugs.openjdk.java.net/browse/JDK-8181595
>>