Code Review Request 8072452 Support DHE sizes up to 8192-bits
xuelei.fan at oracle.com
Tue Apr 5 00:48:56 UTC 2016
On 4/2/2016 5:36 AM, Bernd Eckenfels wrote:
> Hello Xuelei,
> glad to see this. :)
> Does the comment "// FEDHE" stand for FFDHE (JDK-8140436)?
Good catch! Updated to "FFDHE".
> maybe name the variables ffdheXXXX instead of pXXXX? (they might be
> directly used clients to verify well known parameters)
'p' in "pXXX" means the prime modulus p of DH parameters. If using
ffdheXXX, there are more than 80 characters in the line. I would prefer
to use pXXX instead.
> Unrelated to the change: What about reomving p512 and p768 from the
> cache. In normal scenarios they are disabled, but if somebody enables
> the weak sizes he would benefit from dynamic generation (and not much
> time is needed).
In general, the dynamic generation is still slow for a server. 512 and
768 should be used for corner cases only at present.
> Line 154: just an observation, the other places in this file request
> 'JsseJce.get*("DiffieHellmann"' but the KeyFactory is requested by name
"DH" is an alias of "DiffieHellmann". Nice to use the formal name.
> For my taste it would be good to document why generation of
> modulus primes >1024 bit is not supported. Is this a performance issue
> or a problem with the security of the generator?)
Performance issue, very slow. Updated the comment for the reason.
> Line 64+70: Does not support 8k?
Not yet because the performance issue.
> Observation unrelated to the patch: Would it make sense in DHCrypt to
> reference the parametes from ParamterCache and not have 2 distinguished
> places where same (if any) constants are defined? (the differente
> caches can still exist)
Better to use different groups for different protocols. The groups used
in DHCrypt are for SSL/TLS connections only, which would better be
different from the groups defined in ParameterCache. For the same
constants (defined by 2409/3526), it would be nice to update to use
different safe primes later.
> Line 68+94+168 - the 3072bit case is provided, should also be supporte?
> Line 291+333 - in case you need to have another revision there is a
> nit: blank lines are used in all other places before the xxxCache.put
> Another observation not related to your change:
> DHParameterGenerator:141+157+DSAParameterGenerator:152 is it
> intentional that two use provider SUN, SunJCE and one does not specify
> a provider?
I think so. Algorithm parameters normally can be provider independent.
Thanks a lot for the review.
> Am Fri, 1 Apr 2016 08:53:05 -0700
> schrieb Xuelei Fan <Xuelei.Fan at Oracle.COM>:
>> Please review this improvement update to support DHE sizes up to
>> 8192-bits and DSA sizes up to 3072-bits:
>> This updated extends to support 3072-bits DH and DSA parameters
>> generation, and pre-computed DH parameters up to 8192 bits and
>> pre-computed DSA parameters up to 3072-bits.
More information about the security-dev