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
