RFR 8171279: Support X25519 and X448 in TLS 1.3

Adam Petcher adam.petcher at oracle.com
Wed Sep 5 19:49:24 UTC 2018


New webrev: http://cr.openjdk.java.net/~apetcher/8171279/webrev.02/

On 9/5/2018 1:35 PM, Xuelei Fan wrote:

> On 9/5/2018 10:09 AM, Adam Petcher wrote:
>
>> Is there some place in the code where JSSE is doing something too 
>> complicated related to these parameters?
>>
> Yes, the algorithm name is sufficient, I'm not sure the necessary to 
> use XECParameters in sun.security.util.

The algorithm name is not quite sufficient. See the new methods that 
were added to ECUtil that encode/decode public keys. We also need to 
know the key length (which is in XECParameters) in order to 
encode/decode public keys.

>
>>
>> I simply continued the pattern that was used in DH and ECDH. We can 
>> use a different string here, but I worry that people will be 
>> surprised if DH and ECDH support "TlsPreMasterSecret" but XDH doesn't.
> It could support "TlsPreMasterSecret", right?  My concern is about not 
> limited to this one only.

Yes, it supports "TlsPreMasterSecret" in the webrev now. Perhaps I'm not 
sure what you are suggesting here.

>
>>>
>>> 3. NamedGroupFunctions
>>> It might be more straightforward to define these functions in 
>>> NamedGroup.  If comparing 
>>> nameGroup.getFunctions().getParameterSpec() and 
>>> namedGroup.getParameterSpec(), I'd prefer the latter.
>>
>> A simple way to accomplish this is to leave the structure alone and 
>> add methods in NamedGroup that pass through to the corresponding 
>> methods in NamedGroupFunctions. I made this change in the updated 
>> webrev. Let me know what you think.
>>
> It looks like a problem to me to use this before it constructed.
>     this.functions = new ECDHFunctions(this);
>
> and the use of new object for each named group is not effective. The 
> current NamedGroupFunctions abstract class does not sound good enough 
> to me, considering the numbers of the named groups.
>
> I have no idea so far, but I think you can improve it.  Probably use 
> static methods?

In the latest webrev, I changed it so there is a single static 
NamedGroupFunctions of each type, and the NamedGroup is passed in as the 
first argument to each method that requires it (rather than being a 
member of NamedGroupFunctions).




More information about the security-dev mailing list