[PATCH]: Support for brainpool curves from CurveDB in SunEC

[Adam Petcher]

On 12/15/2017 11:31 AM, Tobias Wagner wrote:

> Hi,
>
> in our current project, we have the requirement to support brainpool curves for TLS connections (RFC 7027).
>
> As part of this requirement, we introduced the brainpoolP*r1 curves to SunEC, as they are already known in sun.security.util.CurveDB. It does not introduce the twisted curves from RFC 5639. We want to share this patch, hoping it might be useful for others. Especially for public funded projects (e.g. health care or eID) in Europe, the use or at least support for these curves is often mandatory.

Great! You are going to make it a happy Christmas for a lot of people. I 
would be happy to sponsor this change, and I have a few initial 
requests/comments/questions before I look at the patch in more detail:

1) Please include a test which checks the new curves against some test 
vectors. Ideally, these vectors come from a standard (e.g. RFC 7027 has 
some).
2) The existing tests will check for some form of correctness for all 
enabled curves, but this doesn't ensure that the Brainpool curves are 
enabled. So you should also add a test that ensures that the new curves 
are enabled. This can be combined with the previously mentioned test 
against the test vectors.
3) We can't push this change to the repo without also fixing 
JDK-8189594. So I think you have a couple of options. Either submit a 
separate patch for JDK-8189594 first (which may be tricky because you 
will need to find a way to write a test for it) or include the fix for 
JDK-8189594 in this patch.
4) How important are the 224-bit and smaller curves? We can't use them 
in TLS, and I don't think we should add curves that are already obsolete 
unless there is a good reason.