Code review request, JDK-6956398, make ephemeral DH key match the length of the certificate key

[Xuelei Fan]

new webrev: http://cr.openjdk.java.net/~xuelei/6956398/webrev.01/

On 9/24/2013 5:08 PM, Weijun Wang wrote:
> Hi Xuelei
> 
> Comments below.
> 
> On 9/22/13 11:15 AM, Xuelei Fan wrote:
>> Hi Weijun,
>>
>> Are you available to review this update?
>>
>> webrev: http://cr.openjdk.java.net/~xuelei/6956398/webrev.00/
>>
>> This is an enhancement to support stronger ephemeral DH keys during TLS
>> handshaking.  A new system property is defined
>> "jdk.tls.ephemeralDHKeySize".  By default, the value of this system
>> property is not defined.  This system property won't impact DH key size
>> in ServerKeyExchange message of exportable cipher suites.
>>
>> If this system property is defined as "legacy", no actually behavior
>> change within this update.
>>
>> If this system property is defined as "smart", for non-exportable
>> anonymous cipher suites, the DH key size in ServerKeyExchange message is
>> updated from 768 bits to 1024 bits; and for X.509 certificate based
>> authentication (of non-exportable cipher suites), the DH key size
>> matching the corresponding authentication key is used except that the
>> size should be between 1024 bits and 2048 bits.  For example, if the
>> public key size of an authentication certificate is 2048 bits, then the
>> ephemeral DH key size should be 2048 bits accordingly unless the cipher
>> suite is exportable.
> 
> How about "matched"?
> 
OK to me.

>>
>> If this system property is defined as a valid integer between 1024 and
>> 2048 inclusive, a fixed ephemeral DH key size as the specified integer
>> value will be used for non-exportable cipher suites.
>>
>> If this system property is not defined, or the value is other than
>> "legacy", "smart" and a valid integer, 1024 bits DH key is always used
>> for non-exportable cipher suites.
> 
> Why not throw an error when it's an illegal value?
If it is a invalid integer (an integer between 1024 and 2048),
IllegalArgumentException is thrown.

If it is not a integer, I wanted to treat it as "false" or "normal"
case.  Looks like throw an error is a more clear solution.  Updated.

> Do you expect more values in future versions?
Properly.  The DHE cipher suite may not retire in a short term.  So we
may need to support 4096 on demand.  But current, SunJCE only support
1024 and 2048 bits.

Because providers may only accept 2048 bits DH keys bigger than 1024
bits, we cannot support key size other than 1024 and 2048 in matched
mode.  We may improve the behavior in JCE and JSSE providers in the future.

> If user specify 2048 you now choose 2048.
Yes.

> Do you mean if user specify 4096 and you will only choose 1024?
> 
IllegalArgumentException is thrown.

>>
>> Note that with this fix, the biggest acceptable key size is 2048 bits
>> because DH keys bigger than 2048 bits may be not supported by underlying
>> JCE providers (for example, SunJCE provider).
> 
> But this will be changed later, right?
Maybe.

> I think you can say something
> like "Due to the limitation of underlying JCE providers, the actual
> keysize of the ephemeral DH key generated could be smaller. The maximum
> keysize for JDK 8 is 2048".
Cool!

> BTW, could it be bigger?
> 
Yes, DH can have bigger keys.

> Also, you don't allow someone set it to 768 directly? Must he use "legacy"?
> 
768-bits should be deprecated.  So I don't want to provide an
alternative option any more.

Thanks,
Xuelei

> Thanks
> Max
> 
>>
>> We may update the default ephemeral DH key size (which is 1024 bits with
>> this fix) again in the future if the industry needs to use stronger
>> strength.
>>
>> This update only impact DHE_RSA, DHE_DSS and DH_anon based cipher suites
>> in Oracle provider.
>>
>> Here is a recap of the behaviors:
>>
>> jdk.tls.ephemeralDHKeySize | legacy | smart |  integer | other
>>                             |        |       |  (fixed) |
>> ---------------------------+--------+-------+----------+------
>> exportable DH key size     |  512   |  512  |      512 |   512
>> ---------------------------+--------+-------+----------+------
>> anonymous                  |  768   | 1024  | fixed[+] |  1024
>> ---------------------------+--------+-------+----------+------
>> authentication cert        |  768   |  [*]  | fixed[+] |  1024
>>
>>
>> [*]: the key size the same as the authentication certificate, but should
>> be between 1024-bits and 2048-bits, inclusive.
>> [+]: the fixed key size is specified by a valid integer property value,
>> which should be between 1024-bits and 2048-bits, inclusive.
>>
>>
>> Thanks,
>> Xuelei
>>