Code Review Request, JDK-8140436, Support the FFDHE TLS extension

[Jamil Nimeh]

Hi Xuelei, one question and one nit, the rest looks pretty good.

  * SupportedGroupsExtension
      o Line 267 and 278: From what I can see historically and with TLS
        1.3 draft 19 the underlying list for elliptic_curves and/or
        supported_groups extension data cannot be empty. Should there be
        guards in the two constructors to prevent use of an empty array
        or an incoming empty vector?
  * NamedGroup
      o This is a nit, but I'm curious why you didn't just overload
        valueOf as valueOf(int) and valueOf(String) similar to how
        Integer does it and others like it.  I'm fine with it as-is if
        you prefer to keep it that way.

--Jamil


On 4/14/2017 10:48 AM, Xuelei Fan wrote:
> Hi,
>
> Adam Petcher have some good comments in a private mail to me.  The 
> webrev is updated accordingly:
>
>    http://cr.openjdk.java.net/~xuelei/8140436/webrev.01/
>
> Major update: No customization is allowed for the FFDHE parameters.
>
> Thanks,
> Xuelei
>
> On 4/13/2017 11:15 AM, Xuelei Fan wrote:
>> Hi,
>>
>> Please review the enhancement to support Finite Field Diffie-Hellman
>> Ephemeral (FFDHE) Parameters negotiation in SSL/TLS/DTLS implementation.
>>
>>    http://cr.openjdk.java.net/~xuelei/8140436/webrev.00/
>>
>> Updates:
>> 1. Support predefined FFDHE parameters.
>> JDK will support the following FFDHE parameters defined in RFC 7919, in
>> preference order:
>>       name        |    key size (bits)
>>    ---------------+-------------------
>>     ffdhe2048     |    2048
>>    ---------------+-------------------
>>     ffdhe3072     |    3072
>>    ---------------+-------------------
>>     ffdhe4096     |    4096
>>    ---------------+-------------------
>>     ffdhe6144     |    6144
>>    ---------------+-------------------
>>     ffdhe8192     |    8192
>>    ---------------+-------------------
>>
>>
>> 2. Define a new System Property so as to disable the FFDHE mechanism
>> For RFC 7919 compatible client, the predefined FFDHE parameter names are
>> present in the "supported_groups" TLS extension.  Some server may not be
>> able to handle this extension or the FFDHE groups in the extension.   If
>> there is an interop issue, the new defined System Property,
>> "jsse.enableFFDHE", can be used to dismiss the predefined FFDHE
>> parameters for DHE cipher suites.
>>
>> 3. Redefine the jdk.tls.ephemeralDHKeySize System Property.
>> For connection request from RFC 7919 compatible clients, the server
>> would prefer to use FFDHE mechanism at first unless
>> "jdk.tls.ephemeralDHKeySize" is defined to use "legacy" mode for
>> compatibility reason.
>>
>> jdk.tls.ephemeralDHKeySize | FFDHE                | Server behavior
>> ---------------------------+----------------------+---------------------- 
>>
>>   "legacy"                 | in any case          | Use legacy mode.
>> ---------------------------+----------------------+---------------------- 
>>
>>   not "legacy"             | Not present in the   | Use DHE parameters
>>                            | ClientHello message  | compatible to the
>>                            |                      | System Property.
>> ---------------------------+----------------------+---------------------- 
>>
>>   not "legacy"             | Present in the       | Use the FFDHE
>>                            | ClientHello message  | defined parameters.
>>
>> Note: Exportable cipher suites do not use the FFDHE mechanism.
>>
>> 4. Extend the "jdk.tls.namedGroups" System Property
>> Extend the "jdk.tls.namedGroups" System Property to support customized
>> FFDHE groups.  The following names are now supported by the System
>> Property.
>>
>>     Names for named group  | For EC or DH  | Is it new in the update?
>> ------------------------+---------------+-------------------------
>>     secp256r1              |  ECDHE        | No
>> ------------------------+---------------+-------------------------
>>     secp384r1              |  ECDHE        | No
>> ------------------------+---------------+-------------------------
>>     secp521r1              |  ECDHE        | No
>> ------------------------+---------------+-------------------------
>>     sect283k1              |  ECDHE        | No
>> ------------------------+---------------+-------------------------
>>     sect283r1              |  ECDHE        | No
>> ------------------------+---------------+-------------------------
>>     sect409k1              |  ECDHE        | No
>> ------------------------+---------------+-------------------------
>>     sect409r1              |  ECDHE        | No
>> ------------------------+---------------+-------------------------
>>     sect571k1              |  ECDHE        | No
>> ------------------------+---------------+-------------------------
>>     sect571r1              |  ECDHE        | No
>> ------------------------+---------------+-------------------------
>>     ffdhe2048              |  FFDHE        | Yes
>> ------------------------+---------------+-------------------------
>>     ffdhe3072              |  FFDHE        | Yes
>> ------------------------+---------------+-------------------------
>>     ffdhe4096              |  FFDHE        | Yes
>> ------------------------+---------------+-------------------------
>>     ffdhe6144              |  FFDHE        | Yes
>> ------------------------+---------------+-------------------------
>>     ffdhe8192              |  FFDHE        | Yes
>> ------------------------+---------------+-------------------------
>>
>> Thanks,
>> Xuelei
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20170417/1a34f604/attachment.htm>