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

[Xuelei Fan]

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