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

[Adam Petcher]

On 12/16/2017 2:52 PM, Tobias Wagner wrote:

> -----Ursprüngliche Nachricht-----
>> Von:Adam Petcher <adam.petcher at oracle.com>
>> Gesendet: Fre 15 Dezember 2017 20:57
>> An: security-dev at openjdk.java.net
>> Betreff: Re: [PATCH]: Support for brainpool curves from CurveDB in SunEC
>> 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.
> Curves with less than 256 bits are not of special importance for us. In fact, our
> first approach did not include these curves, but this had some odd side effects,
> which came out when running the jtreg tests:
>
> These curves are already present in CurveDB, thus one can generally use their
> ASN.1 oids to request calculations on them. In SunEC there is a check on length
> of the oid's DER encoding, to decide wether the curve is supported or not:
>
> [ec.h, ecdecode.c: EC_FillParams()]
> 10: ANSI X9.62 curves
> 7: SECG curves
> (11: Brainpool curves)
>
> Using that mechanism leads to errors in the native part, as it tries to access the non-
> present domain parameters, if someone requests e.g. a ECDSA signature with brainpool160r1.
> For that reason I added the remaining parameters.
>
> One reason to add these curves, is to be able to support the verification of legacy signatures.
>
> If they should not be added, I see two options:
>
> * remove them from CurveDB as well, so they can't be addressed anymore.
>
> or
>
> * There should be a more explicit check than the length of the oid's DER encoding to decide wether the curve is supported or not.
>
> I am not sure, what option should be taken in that case. As far as I understand, the first one might affect other providers, which could not
> support these curves anymore as well.

Right. Removing them from CurveDB isn't a good option because of the 
compatibility impact. Also note that CurveDB is allowed to vary 
independently of the native implementation. So we always have to handle 
the case that curves exist in CurveDB, but are not supported in the 
native code. For example, someone could add the twisted Brainpool curves 
to CurveDB in the future.

I think the explicit check that you need is already there in the code 
you added to SECOID_FindOID. You just need to replace the elements in 
SECOidData BRAINPOOL_oids that you don't want to support with the 
"Unknown OID" block element that you are using for the twisted curves. 
Of course, there are probably other ways to do this check, but this 
seems to be the pattern that exists in the code already.

I'm not completely opposed to adding these small curves, but I don't 
think we should do it unless there is a compelling reason. So if anyone 
in the community has a need for these curves (or other thoughts on this 
issue), please let us know.


>
> Regards
> Tobias
>
>
>
>