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

2018-11-08 Thread Adam Petcher
I'm missing the motivation behind this question. Is the current set of 
aliases causing some problem? Are they incomplete? Why is it bad that 
"X9.62 prime256v1" works but "prime256v1" doesn't?


On 11/7/2018 10:05 PM, Weijun Wang wrote:

In CurveDB.java, we have

add("secp256r1 [NIST P-256, X9.62 prime256v1]", "1.2.840.10045.3.1.7", PD,
 "0001",
 "0001FFFC",
 "5AC635D8AA3A93E7B3EBBD55769886BC651D06B0CC53B0F63BCE3C3E27D2604B",
 "6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296",
 "4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5",
 "BCE6FAADA7179E84F3B9CAC2FC632551",
 1, nameSplitPattern);

So the aliases of secp256r1 are now "NIST P-256" and "X9.62 prime256v1". Do we 
really want to keep the organization name prefix after JDK-8208156? The alias can be used in 
ECGenParameterSpec and the proposed keytool -groupname option.

The following shows this behavior.


jshell> KeyPairGenerator.getInstance("EC")
$3 ==> java.security.KeyPairGenerator$Delegate@64bfbc86

jshell> $3.initialize(new ECGenParameterSpec("secp256r1"))

jshell> $3.initialize(new ECGenParameterSpec("prime256v1"))
|  Exception java.security.InvalidAlgorithmParameterException: Unknown curve 
name: prime256v1
|at ECKeyPairGenerator.initialize (ECKeyPairGenerator.java:103)
|at KeyPairGenerator$Delegate.initialize (KeyPairGenerator.java:699)
|at KeyPairGenerator.initialize (KeyPairGenerator.java:436)
|at (#6:1)

jshell> $3.initialize(new ECGenParameterSpec("X9.62 prime256v1"))

Thanks
Max


On Nov 7, 2018, at 11:48 PM, Weijun Wang  wrote:

CSR updated. With such a generalized option, I won't recommend -groupname over 
-keysize now, although I still intend to print some warning for EC.

Please take a review.

Thanks
Max





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

2018-11-08 Thread Adam Petcher

On 11/7/2018 8:53 PM, Weijun Wang wrote:


Oh, I didn't know that.

To make sure -keyalg matches KeyPairGenerator.getInstance(), I'd like to 
support it. If I read the impl correctly, you don't need to initialize it 
anymore and if you really want to initialize it the params must be the same. 
Currently keytool always calls initialize(). In this case, there will be no 
default -keysize, and initializa() will be not be called if user has not 
specified one. If user provides -groupname or -keysize just use it and keytool 
fails if API call fails.


This is correct. If you are using the "X25519" and "X448" algorithm 
name, then there is no need to initialize with parameters, because they 
are decided by the algorithm name. You can still initialize, and 
different providers may be more permissive than others if you try to use 
different parameters than what is specified by the algorithm name. For 
example: KeyPairGenerator.getInstance("X448").initialize(255). SunEC is 
very strict, and will not allow this sort of thing.


X25519/X448 keys can only be used for KeyAgreement, so they aren't 
supported in keytool anyway, right? If this is the case, then you don't 
need to worry about adding any code to keytool for them. Though I expect 
EdDSA to work in a similar way (algorithm names "Ed25519" and "Ed448"). 
Still, I don't know if it is worthwhile to add special code for it. For 
all algorithms, if no -keysize or -groupname is specified, then keytool 
could skip the initialize on the KPG, and the implementation defaults 
(if available) will be used. The hard part is deciding whether to emit a 
warning in this case, and that will probably be algorithm-specific.



On Nov 8, 2018, at 8:01 AM, Xuelei Fan  wrote:

On 11/7/2018 3:38 PM, Weijun Wang wrote:

This sounds a little misleading to me. Alg name and alg params are 2 different things. 
This is like asking user to call KeyPairGenerator.getInstance("secp256r1").

Well, KeyPairGenerator.getInstance("x25519") is a case that JDK 11 has 
supported now.

Otherwise, there is a need to check the conflict of alg name and group name.


This only works because "X25519" (and "X448") is both an algorithm name 
and a parameter spec name. This makes sense for X25519/X448, but not for 
all algorithms.


I don't think there is any need to check for algorithm/group conflicts 
in keytool, because it is checked in the crypto provider already. All 
keytool needs to do is pass down the algorithm name and group name (as a 
NamedParameterSpec/ECGenParameterSpec) and see if it works. If we want 
to support "secp256r1" in -keyalg, then we can accomplish that by adding 
it as an algorithm name for KeyPairGenerator. Though I'm not sure this 
is a good idea.


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

2018-11-07 Thread Xuelei Fan
I don't think the underlying provider is ready to support named curves. 
Additional RFEs may be required to standardize the names and improve the 
underlying provider.


Xuelei

On 11/7/2018 7:05 PM, Weijun Wang wrote:

In CurveDB.java, we have

add("secp256r1 [NIST P-256, X9.62 prime256v1]", "1.2.840.10045.3.1.7", PD,
 "0001",
 "0001FFFC",
 "5AC635D8AA3A93E7B3EBBD55769886BC651D06B0CC53B0F63BCE3C3E27D2604B",
 "6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296",
 "4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5",
 "BCE6FAADA7179E84F3B9CAC2FC632551",
 1, nameSplitPattern);

So the aliases of secp256r1 are now "NIST P-256" and "X9.62 prime256v1". Do we 
really want to keep the organization name prefix after JDK-8208156? The alias can be used in 
ECGenParameterSpec and the proposed keytool -groupname option.

The following shows this behavior.


jshell> KeyPairGenerator.getInstance("EC")
$3 ==> java.security.KeyPairGenerator$Delegate@64bfbc86

jshell> $3.initialize(new ECGenParameterSpec("secp256r1"))

jshell> $3.initialize(new ECGenParameterSpec("prime256v1"))
|  Exception java.security.InvalidAlgorithmParameterException: Unknown curve 
name: prime256v1
|at ECKeyPairGenerator.initialize (ECKeyPairGenerator.java:103)
|at KeyPairGenerator$Delegate.initialize (KeyPairGenerator.java:699)
|at KeyPairGenerator.initialize (KeyPairGenerator.java:436)
|at (#6:1)

jshell> $3.initialize(new ECGenParameterSpec("X9.62 prime256v1"))


Thanks
Max


On Nov 7, 2018, at 11:48 PM, Weijun Wang  wrote:

CSR updated. With such a generalized option, I won't recommend -groupname over 
-keysize now, although I still intend to print some warning for EC.

Please take a review.

Thanks
Max





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

2018-11-07 Thread Weijun Wang
In CurveDB.java, we have

add("secp256r1 [NIST P-256, X9.62 prime256v1]", "1.2.840.10045.3.1.7", PD,
"0001",
"0001FFFC",
"5AC635D8AA3A93E7B3EBBD55769886BC651D06B0CC53B0F63BCE3C3E27D2604B",
"6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296",
"4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5",
"BCE6FAADA7179E84F3B9CAC2FC632551",
1, nameSplitPattern);

So the aliases of secp256r1 are now "NIST P-256" and "X9.62 prime256v1". Do we 
really want to keep the organization name prefix after JDK-8208156? The alias 
can be used in ECGenParameterSpec and the proposed keytool -groupname option.

The following shows this behavior.

> jshell> KeyPairGenerator.getInstance("EC")
> $3 ==> java.security.KeyPairGenerator$Delegate@64bfbc86
> 
> jshell> $3.initialize(new ECGenParameterSpec("secp256r1"))
> 
> jshell> $3.initialize(new ECGenParameterSpec("prime256v1"))
> |  Exception java.security.InvalidAlgorithmParameterException: Unknown curve 
> name: prime256v1
> |at ECKeyPairGenerator.initialize (ECKeyPairGenerator.java:103)
> |at KeyPairGenerator$Delegate.initialize (KeyPairGenerator.java:699)
> |at KeyPairGenerator.initialize (KeyPairGenerator.java:436)
> |at (#6:1)
> 
> jshell> $3.initialize(new ECGenParameterSpec("X9.62 prime256v1"))

Thanks
Max

> On Nov 7, 2018, at 11:48 PM, Weijun Wang  wrote:
> 
> CSR updated. With such a generalized option, I won't recommend -groupname 
> over -keysize now, although I still intend to print some warning for EC.
> 
> Please take a review.
> 
> Thanks
> Max
> 



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

2018-11-07 Thread Weijun Wang
Oh, I didn't know that.

To make sure -keyalg matches KeyPairGenerator.getInstance(), I'd like to 
support it. If I read the impl correctly, you don't need to initialize it 
anymore and if you really want to initialize it the params must be the same. 
Currently keytool always calls initialize(). In this case, there will be no 
default -keysize, and initializa() will be not be called if user has not 
specified one. If user provides -groupname or -keysize just use it and keytool 
fails if API call fails.

Thanks
Max

> On Nov 8, 2018, at 8:01 AM, Xuelei Fan  wrote:
> 
> On 11/7/2018 3:38 PM, Weijun Wang wrote:
>> This sounds a little misleading to me. Alg name and alg params are 2 
>> different things. This is like asking user to call 
>> KeyPairGenerator.getInstance("secp256r1").
> Well, KeyPairGenerator.getInstance("x25519") is a case that JDK 11 has 
> supported now.
> 
> Otherwise, there is a need to check the conflict of alg name and group name.
> 
> Xuelei
> 
>> --Max
>>> On Nov 8, 2018, at 1:47 AM, Xuelei Fan  wrote:
>>> 
>>> Maybe, the -groupname/-curvename option can be replaced by extending the 
>>> existing -keyalg option:
>>>  -keyalg secp256r1
>>> 
>>> Then there is no conflict between the curve/group name and the key alg.
>>> 
>>> Xuelei
>>> 
>>> On 11/7/2018 7:48 AM, Weijun Wang wrote:
 CSR updated. With such a generalized option, I won't recommend -groupname 
 over -keysize now, although I still intend to print some warning for EC.
 Please take a review.
 Thanks
 Max
> On Nov 7, 2018, at 10:36 PM, Adam Petcher  wrote:
> 
> One issue that just came to me: How will this work for EdDSA? I think the 
> CSR could be generalized a bit:
> 
> 1) Make the first item in the "Solution" more general. Instead of 
> limiting it to "EC" allow any valid algorithm/curve combination.
> 2) (Optional) Use -groupname instead of -curvename and change "curve" to 
> "group" everywhere in the CSR. Then this mechanism can also be used for 
> DSA (with named groups) and other algorithms that use groups that aren't 
> curves.
> Also, see below for a comment about curve ambiguity.
> On 11/6/2018 7:59 PM, Weijun Wang wrote:
>>> Otherwise, there are may be more curve categories. As it is not the 
>>> recommended option, I may just remove this and the following one 
>>> sentence.
>>> 
>> I'll just leave it there as a FYI since it's not part of the spec.
> 
> I agree with Xuelei that this part should be removed. Unless you are 
> planning on implementing this curve selection logic in keytool, then we 
> can't control which curve is selected, and it wholly depends on the 
> behavior of the providers. We can't even guarantee that there is any 
> relationship between "key size" and the field size of the curve. Also, we 
> shouldn't use the word "random" here unless we plan to actually randomize 
> the selection of the curve at runtime (similar to random iteration order 
> for maps/sets). I suggest something more general and vague like:
> 
> If only -keysize is specified, an arbitrary curve of the specified size 
> is used
> 



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

2018-11-07 Thread Xuelei Fan

On 11/7/2018 3:38 PM, Weijun Wang wrote:

This sounds a little misleading to me. Alg name and alg params are 2 different things. 
This is like asking user to call KeyPairGenerator.getInstance("secp256r1").
Well, KeyPairGenerator.getInstance("x25519") is a case that JDK 11 has 
supported now.


Otherwise, there is a need to check the conflict of alg name and group name.

Xuelei



--Max


On Nov 8, 2018, at 1:47 AM, Xuelei Fan  wrote:

Maybe, the -groupname/-curvename option can be replaced by extending the 
existing -keyalg option:
  -keyalg secp256r1

Then there is no conflict between the curve/group name and the key alg.

Xuelei

On 11/7/2018 7:48 AM, Weijun Wang wrote:

CSR updated. With such a generalized option, I won't recommend -groupname over 
-keysize now, although I still intend to print some warning for EC.
Please take a review.
Thanks
Max

On Nov 7, 2018, at 10:36 PM, Adam Petcher  wrote:

One issue that just came to me: How will this work for EdDSA? I think the CSR 
could be generalized a bit:

1) Make the first item in the "Solution" more general. Instead of limiting it to 
"EC" allow any valid algorithm/curve combination.
2) (Optional) Use -groupname instead of -curvename and change "curve" to 
"group" everywhere in the CSR. Then this mechanism can also be used for DSA (with named 
groups) and other algorithms that use groups that aren't curves.
Also, see below for a comment about curve ambiguity.
On 11/6/2018 7:59 PM, Weijun Wang wrote:

Otherwise, there are may be more curve categories. As it is not the recommended 
option, I may just remove this and the following one sentence.


I'll just leave it there as a FYI since it's not part of the spec.


I agree with Xuelei that this part should be removed. Unless you are planning on implementing this 
curve selection logic in keytool, then we can't control which curve is selected, and it wholly 
depends on the behavior of the providers. We can't even guarantee that there is any relationship 
between "key size" and the field size of the curve. Also, we shouldn't use the word 
"random" here unless we plan to actually randomize the selection of the curve at runtime 
(similar to random iteration order for maps/sets). I suggest something more general and vague like:

If only -keysize is specified, an arbitrary curve of the specified size is used





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

2018-11-07 Thread Xuelei Fan
Maybe, the -groupname/-curvename option can be replaced by extending the 
existing -keyalg option:

  -keyalg secp256r1

Then there is no conflict between the curve/group name and the key alg.

Xuelei

On 11/7/2018 7:48 AM, Weijun Wang wrote:

CSR updated. With such a generalized option, I won't recommend -groupname over 
-keysize now, although I still intend to print some warning for EC.

Please take a review.

Thanks
Max



On Nov 7, 2018, at 10:36 PM, Adam Petcher  wrote:

One issue that just came to me: How will this work for EdDSA? I think the CSR 
could be generalized a bit:

1) Make the first item in the "Solution" more general. Instead of limiting it to 
"EC" allow any valid algorithm/curve combination.
2) (Optional) Use -groupname instead of -curvename and change "curve" to 
"group" everywhere in the CSR. Then this mechanism can also be used for DSA (with named 
groups) and other algorithms that use groups that aren't curves.
Also, see below for a comment about curve ambiguity.
On 11/6/2018 7:59 PM, Weijun Wang wrote:

Otherwise, there are may be more curve categories. As it is not the recommended 
option, I may just remove this and the following one sentence.


I'll just leave it there as a FYI since it's not part of the spec.


I agree with Xuelei that this part should be removed. Unless you are planning on implementing this 
curve selection logic in keytool, then we can't control which curve is selected, and it wholly 
depends on the behavior of the providers. We can't even guarantee that there is any relationship 
between "key size" and the field size of the curve. Also, we shouldn't use the word 
"random" here unless we plan to actually randomize the selection of the curve at runtime 
(similar to random iteration order for maps/sets). I suggest something more general and vague like:

If only -keysize is specified, an arbitrary curve of the specified size is used





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

2018-11-07 Thread Weijun Wang
CSR updated. With such a generalized option, I won't recommend -groupname over 
-keysize now, although I still intend to print some warning for EC.

Please take a review.

Thanks
Max


> On Nov 7, 2018, at 10:36 PM, Adam Petcher  wrote:
> 
> One issue that just came to me: How will this work for EdDSA? I think the CSR 
> could be generalized a bit:
> 
> 1) Make the first item in the "Solution" more general. Instead of limiting it 
> to "EC" allow any valid algorithm/curve combination.
> 2) (Optional) Use -groupname instead of -curvename and change "curve" to 
> "group" everywhere in the CSR. Then this mechanism can also be used for DSA 
> (with named groups) and other algorithms that use groups that aren't curves.
> Also, see below for a comment about curve ambiguity.
> On 11/6/2018 7:59 PM, Weijun Wang wrote:
>>> Otherwise, there are may be more curve categories. As it is not the 
>>> recommended option, I may just remove this and the following one sentence.
>>> 
>> I'll just leave it there as a FYI since it's not part of the spec.
> 
> I agree with Xuelei that this part should be removed. Unless you are planning 
> on implementing this curve selection logic in keytool, then we can't control 
> which curve is selected, and it wholly depends on the behavior of the 
> providers. We can't even guarantee that there is any relationship between 
> "key size" and the field size of the curve. Also, we shouldn't use the word 
> "random" here unless we plan to actually randomize the selection of the curve 
> at runtime (similar to random iteration order for maps/sets). I suggest 
> something more general and vague like:
> 
> If only -keysize is specified, an arbitrary curve of the specified size is 
> used
> 



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

2018-11-07 Thread Weijun Wang
I don't think there is any current AlgorithmParameterSpec that allow this for a 
KeyPairGenerator. When a curve name is used, keysize is calculated from the 
field size.

--Max

> On Nov 7, 2018, at 4:05 PM, Michael StJohns  wrote:
> 
> Inline below.
> 
> On 11/6/2018 2:18 AM, Weijun Wang wrote:
>> 
>>> On Nov 6, 2018, at 1:06 PM, Xuelei Fan  wrote:
>>> 
>>> On 11/5/2018 8:37 PM, Weijun Wang wrote:
> On Nov 6, 2018, at 12:12 PM, Xuelei Fan  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.
>> Well explained. I've updated the CSR and this will be an error.
> 
> Sorry to drop in late.
> 
> Basically, for EC private keys - either binary or prime curves, you will 
> reduce whatever initial random value you generate mod n of the curve to get 
> the final private key.  The generation logic should take care of this.You 
> could use key size as a way of controlling how many extra bits are 
> generated(see FIPS 186-4, section B.4.1) and error only if key size was less 
> than the size of the curve's n.
> 
> So 1) generate a random value of keysize length or if not specified the 
> length of the N of the curve plus 64, 2) reduce mod N.
> 
> Mime.
> 
>> 
>>> 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.
>> If people only want prime curves then -keysize still works. A warning is 
>> enough since in the CSR I've also said "we recommend".
>> 
>> Thanks
>> Max
>> 
>>> Xuelei



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

2018-11-07 Thread Adam Petcher
One issue that just came to me: How will this work for EdDSA? I think 
the CSR could be generalized a bit:


1) Make the first item in the "Solution" more general. Instead of 
limiting it to "EC" allow any valid algorithm/curve combination.
2) (Optional) Use -groupname instead of -curvename and change "curve" to 
"group" everywhere in the CSR. Then this mechanism can also be used for 
DSA (with named groups) and other algorithms that use groups that aren't 
curves.


Also, see below for a comment about curve ambiguity.

On 11/6/2018 7:59 PM, Weijun Wang wrote:


Otherwise, there are may be more curve categories. As it is not the recommended 
option, I may just remove this and the following one sentence.

I'll just leave it there as a FYI since it's not part of the spec.


I agree with Xuelei that this part should be removed. Unless you are 
planning on implementing this curve selection logic in keytool, then we 
can't control which curve is selected, and it wholly depends on the 
behavior of the providers. We can't even guarantee that there is any 
relationship between "key size" and the field size of the curve. Also, 
we shouldn't use the word "random" here unless we plan to actually 
randomize the selection of the curve at runtime (similar to random 
iteration order for maps/sets). I suggest something more general and 
vague like:


If only |-keysize| is specified, an arbitrary curve of the specified 
size is used




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

2018-11-07 Thread Michael StJohns

Inline below.

On 11/6/2018 2:18 AM, Weijun Wang wrote:



On Nov 6, 2018, at 1:06 PM, Xuelei Fan  wrote:

On 11/5/2018 8:37 PM, Weijun Wang wrote:

On Nov 6, 2018, at 12:12 PM, Xuelei Fan  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.

Well explained. I've updated the CSR and this will be an error.


Sorry to drop in late.

Basically, for EC private keys - either binary or prime curves, you will 
reduce whatever initial random value you generate mod n of the curve to 
get the final private key.  The generation logic should take care of 
this.    You could use key size as a way of controlling how many extra 
bits are generated(see FIPS 186-4, section B.4.1) and error only if key 
size was less than the size of the curve's n.


So 1) generate a random value of keysize length or if not specified the 
length of the N of the curve plus 64, 2) reduce mod N.


Mime.




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.

If people only want prime curves then -keysize still works. A warning is enough since in 
the CSR I've also said "we recommend".

Thanks
Max


Xuelei





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

2018-11-06 Thread Weijun Wang



> On Nov 7, 2018, at 12:47 AM, Xuelei Fan  wrote:
> 
> Some typos:
> 
> "When multiple curves have the same field size, and one of them is a prime 
> curve or a Koblitz curve, it will be used."
> 
> Which one will be used?  prime curve or Koblitz curve.  

I am not an ECC expert, but what I observed from FIPS 186-4 is that no prime 
curve and Koblitz curve use the same field size. The current impl chooses prime 
curve first, simply because they add added earlier.

> It will not be documented, right?  

No.

> Otherwise, there are may be more curve categories. As it is not the 
> recommended option, I may just remove this and the following one sentence.

I'll just leave it there as a FYI since it's not part of the spec.

Thanks
Max

> 
> Xuelei
> 
> 
> 
> On 11/6/2018 8:31 AM, Weijun Wang wrote:
>> 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  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  im Auftrag von 
>>>> Xuelei Fan 
>>>> *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  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 t

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

2018-11-06 Thread Xuelei Fan

Some typos:

"When multiple curves have the same field size, and one of them is a 
prime curve or a Koblitz curve, it will be used."


Which one will be used?  prime curve or Koblitz curve.  It will not be 
documented, right?  Otherwise, there are may be more curve categories. 
As it is not the recommended option, I may just remove this and the 
following one sentence.


Xuelei



On 11/6/2018 8:31 AM, Weijun Wang wrote:

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  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  im Auftrag von Xuelei Fan 

*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  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




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

2018-11-06 Thread 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  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  im Auftrag von 
>> Xuelei Fan 
>> *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  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



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

2018-11-06 Thread 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  im Auftrag 
von Xuelei Fan 

*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  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


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

2018-11-06 Thread Adam Petcher

On 11/6/2018 2:18 AM, Weijun Wang wrote:


On Nov 6, 2018, at 1:06 PM, Xuelei Fan  wrote:

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.

Well explained. I've updated the CSR and this will be an error.


This is a good improvement. If you like, you could even go one step 
further and error out any time -curvename and -keysize are used at the 
same time, even if the size is correct. This would simplify things and 
discourage use of -keysize for EC keys.



(curve ambiguity issue with -keysize)

Thanks
Max



I don't think it is worthwhile to add any code to choose some particular 
curve when only -keysize is used (or KeyPairGenerator.init(int)). 
Keeping the current behavior and choosing an arbitrary curve of the 
specified size is fine. Emitting a warning is a good idea, and if you 
plan to do this, you might want to emit a warning any time -keysize is 
used with an EC key, regardless of whether there are (currently) 
multiple curves of the specified size.


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

2018-11-05 Thread Bernd Eckenfels
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  im Auftrag von Xuelei 
Fan 
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  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


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

2018-11-05 Thread Weijun Wang



> On Nov 6, 2018, at 1:06 PM, Xuelei Fan  wrote:
> 
> On 11/5/2018 8:37 PM, Weijun Wang wrote:
>>> On Nov 6, 2018, at 12:12 PM, Xuelei Fan  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.

Well explained. I've updated the CSR and this will be an error.

> 
> 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.

If people only want prime curves then -keysize still works. A warning is enough 
since in the CSR I've also said "we recommend".

Thanks
Max

> 
> Xuelei



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

2018-11-05 Thread Xuelei Fan

On 11/5/2018 8:37 PM, Weijun Wang wrote:



On Nov 6, 2018, at 12:12 PM, Xuelei Fan  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


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

2018-11-05 Thread Weijun Wang



> On Nov 6, 2018, at 12:12 PM, Xuelei Fan  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?

> 
>> 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.

Thanks
Max

> 
> Xuelei



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

2018-11-05 Thread Xuelei Fan

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.



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.

Do you mean if no -curvename option, there is a need to choose a named 
curve?


Xuelei


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

2018-11-05 Thread Weijun Wang
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.

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?

Thanks
Max