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....@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-boun...@openjdk.java.net> im Auftrag von >> Xuelei Fan <xuelei....@oracle.com> >> *Gesendet:* Dienstag, November 6, 2018 7:38 AM >> *An:* Weijun Wang >> *Cc:* security-dev@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....@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
