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).
