RFR CSR for 8213400: Support choosing curve name in keytool keypair generation

[Weijun Wang]

Thanks everyone. CSR updated, and I describe the behavior in the Solution part. If you are all happy I'll start coding.

And yes, KeyPairGenerator::init(int) needs some clarification, but I don't know in which class/method we should add it. Maybe the JDK Provider Doc?

--Max

> On Nov 7, 2018, at 12:00 AM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
> 
> As -curvename is a new option, I would second the comments that don't allow mixing curve names and keysize at the same time.
> 
> Xuelei
> 
> On 11/5/2018 11:41 PM, Bernd Eckenfels wrote:
>> Hello,
>> I would agree ignoring an (conflicting) option adds confusion. When specifying a curve is a new feature we don’t need to worry about beeing compatible, therefore I would  forbid mixing curve names and keysize at all (even when the size matches).
>> I guess we cannot remove the option to only pass the keysize (to have the generator pick a curve) to stay compatible. But the chosen curve should be printed, and I would also deprecate this usage.
>> I guess clarifying the keysize-only init() method would be a different topic, but something like deprecating it and restricting the selection to „SunEC only selects NIST prime curves“ would be a good thing.
>> Gruss
>> Bernd
>> Gruss
>> Bernd
>> -- 
>> http://bernd.eckenfels.net
>> ------------------------------------------------------------------------
>> *Von:* security-dev <security-dev-bounces at openjdk.java.net> im Auftrag von Xuelei Fan <xuelei.fan at oracle.com>
>> *Gesendet:* Dienstag, November 6, 2018 7:38 AM
>> *An:* Weijun Wang
>> *Cc:* security-dev at openjdk.java.net
>> *Betreff:* Re: RFR CSR for 8213400: Support choosing curve name in keytool keypair generation
>> On 11/5/2018 8:37 PM, Weijun Wang wrote:
>> >
>> >> On Nov 6, 2018, at 12:12 PM, Xuelei Fan <xuelei.fan at oracle.com> wrote:
>> >>
>> >> On 11/5/2018 7:13 PM, Weijun Wang wrote:
>> >>> Please take a review at the CSR at
>> >>> https://bugs.openjdk.java.net/browse/JDK-8213401
>> >>> As for implementation, I intend to report an error when -keyalg is not EC but -curvename is provided. If both -curvename and -keysize are provided, I intend to ignore -keysize no matter if they match or not.
>> >> Why not use a strict mode: fail if not match. It might be misleading if ignoring unmatched options.
>> >
>> > We can do that, but misleading to what? That we treat -curvename and -keysize the same important?
>> >
>> If the option "-keysize 256 -curvename sect163k1" work, I may think that
>> the key size if 256 bits. I want to create a 256 bits sect163k1 EC key,
>> and the tool allows this behavior, so I should get a 256 bits sect163k1
>> EC key. Sure, that's incorrect, but I don't know it is incorrect as the
>> tool ignore the key size. What's the problem of the command, I don't
>> know either unless I clearly understand sect163k1 is not 256 bits. The
>> next question to me, what's the key size actually is? 256 bits or 163
>> bits? which option are used? It adds more confusing to me.
>> We can ignore the -keysize option, but it is complicated to me to use
>> the tool.
>> >>
>> >>> Another question: in sun.security.util.CurveDB, we have
>> >>> // Return EC parameters for the specified field size. If there are known
>> >>> // NIST recommended parameters for the given length, they are returned.
>> >>> // Otherwise, if there are multiple matches for the given size, an
>> >>> // arbitrary one is returns.
>> >>> // If no parameters are known, the method returns null.
>> >>> // NOTE that this method returns both prime and binary curves.
>> >>> static NamedCurve lookup(int length) {
>> >>> return lengthMap.get(length);
>> >>> }
>> >>> FIPS 186-4 has 2 recommendations (K- and B-) for a binary curve field size. Do we have a choice?
>> >>> In fact, CurveDB.java seems to have a bug when adding the curves:
>> >>> add("sect163k1 [NIST K-163]", "1.3.132.0.1", BD,...
>> >>> add("sect163r2 [NIST B-163]", "1.3.132.0.15", BD,... // Another default?
>> >>> add("sect233k1 [NIST K-233]", "1.3.132.0.26", BD,...
>> >>> add("sect233r1 [NIST B-233]", "1.3.132.0.27", B,...
>> >>> and now 163 is sect163r2 and 233 is sect233k1.
>> >>> I assume we should always prefer the K- one?
>> >> TLS 1.3 uses secp256r1/secp384r1/secp521r1, no K- curves.
>> >
>> > There is no ambiguity for prime curves.
>> >
>> >>
>> >> Do you mean if no -curvename option, there is a need to choose a named curve?
>> >
>> > ECKeyPairGenerator::initialize(int) will choose one and keytool will use it. I just meant if we have a bug here and if we should be more public on what curve is chosen.
>> >
>> I see your concerns.
>> It might be a potential issue if we use a named curve if no curvename
>> specified. If the compatibility is not serious, I may suggest supported
>> named curves only, or use arbitrary curves but with a warning.
>> Xuelei