Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
Thanks Max and Sean for the review and suggestions. I have updated the
webrev accordingly and finalized CSR.
Webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.04/
CSR: https://bugs.openjdk.java.net/browse/JDK-8206864
Valerie
On 7/19/2018 3:13 PM, Wang Weijun wrote:
在 2018年7月20日，05:18，Valerie Peng 写道：
Hi Sean,
Thanks for the suggestion, I like it.
Me too.
Max, any objection or concern before I update the webrev and CSR?
No.
Thanks
Max
Valerie
On 7/19/2018 7:28 AM, Sean Mullan wrote:
Hi Valerie, Max -
I took a fresh look at this issue this morning. I think we are getting bogged
down by trying to adjust within the current wording, which is somewhat awkward
and hard to understand. So, I think it might be better to break up the wording
into multiple sentences. Here's a cut at the rewording:
/**
* Returns the parameters used with this signature object.
*
* If this signature has been previously initialized with parameters
* (by calling the {@code setParameter} method), this method returns
* the same parameters. If this signature has not been initialized with
* parameters, this method may return a combination of default and
* randomly generated parameter values if the underlying
* signature implementation supports it and can successfully generate
* them. Otherwise, {@code null} is returned.
*
* @return the parameters used with this signature, or {@code null}
*/
In the first sentence of the 2nd paragraph above, I wanted to first list the case where
the signature is initialized with parameters, which is the most clear-cut case of what
the behavior will be. Then I described the case where an implementation may return
default/generated parameters -- and this is clearly marked "may" since it is
optional. All other cases other than those two always return null, so I think this makes
it easier to understand.
--Sean
On 7/18/18 1:29 PM, Valerie Peng wrote:
Sean,
Where do you think that we should add the part about "null must be returned
..." paragraph? At the end of first or second paragraph?
I will go with majority.
Valerie
On 7/17/2018 8:38 PM, Weijun Wang wrote:
Is it better to append the new lines to the 2nd paragraph?
Thanks
Max
On Jul 18, 2018, at 9:46 AM, Valerie Peng wrote:
Ok, let's use "must" then. I have also added the part about default parameters.
Hope that all is clear now.
Latest webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.03/
Latest CSR: https://bugs.openjdk.java.net/browse/JDK-8206864
Thanks,
Valerie
On 7/17/2018 5:50 PM, Weijun Wang wrote:
On Jul 18, 2018, at 8:23 AM, Valerie Peng wrote:
Hi Max,
Thanks for the suggestions. Please find comments inline.
On 7/16/2018 7:38 PM, Weijun Wang wrote:
CSR at https://bugs.openjdk.java.net/browse/JDK-8206864.
- At the end of the 1st paragraph, you have now
However, for signature algorithm such as "RSASSA-PSS", it requires
parameters and the one used for signing must be supplied for verification to succeed. It
may be better to return null instead of returning provider-specific default parameters.
I suggest we don't talk about the reason (params must be the same for signing and
verification), we can just say something like "If there is no provider-specific
default parameters, this method should return null before user sets one".
Alright, I initially didn't put down the reason. But since Brad asked about it,
I add it to the CSR in case Joe raise the same question. I will update the CSR
per your suggestion.
- null may be returned
How about "{@code null} must be returned"?
How about "should"? Is there a guideline on when to use "may/should/must"?
Anyone knows?
Even if there were guidelines on this for Java, I think we should just stick to
https://tools.ietf.org/html/rfc2119, except that the capitalization is not
necessary.
"Must" is precise here.
I thought must is mostly used in mandating input arguments must not be null.
But don't recall it being used much for return values.
"must return" appears 39 times in java/ and "should return" 19 (simple grep, no
line break).
--Max
Thanks,
Valerie
Everything else looks fine.
Thanks
Max
On Jul 17, 2018, at 9:46 AM, Valerie Peng wrote:
Hi Max,
Good catch on the SignatureSpi class and SunMSCAPI provider, I will include
them.
As for the part about "randomly generated", I am leaning toward not having it
as I don't see a value of specifying this.
Webrev and CSR has been updated.
New webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.02/
Thanks,
Valerie
On 7/16/2018 4:29 PM, Weijun Wang wrote:
Valerie
Would you like to retain the word "randomly generated" somewhere?
Also, the SignatureSpi class needs to be updated in the same way.
Can you also update the implementation in the MSCAPI Signature object?
Thanks
Max
On Jul 17, 2018, at 6:16 AM, Valerie Peng wrote:
No issues found and it seems ok to return null if no parameters is set. Thus, I
have updated the webrev and
Re: RFR(s): 8204196: integer cleanup
Looks fine to me. Thanks, Xuelei On 7/19/2018 5:02 PM, Anthony Scarpino wrote: I need a review of some integer check cleanups. http://cr.openjdk.java.net/~ascarpino/int/webrev/ thanks Tony
RFR(s): 8204196: integer cleanup
I need a review of some integer check cleanups. http://cr.openjdk.java.net/~ascarpino/int/webrev/ thanks Tony
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
> 在 2018年7月20日，05:18，Valerie Peng 写道：
>
> Hi Sean,
>
> Thanks for the suggestion, I like it.
Me too.
>
> Max, any objection or concern before I update the webrev and CSR?
No.
Thanks
Max
>
> Valerie
>
>
>> On 7/19/2018 7:28 AM, Sean Mullan wrote:
>> Hi Valerie, Max -
>>
>> I took a fresh look at this issue this morning. I think we are getting
>> bogged down by trying to adjust within the current wording, which is
>> somewhat awkward and hard to understand. So, I think it might be better to
>> break up the wording into multiple sentences. Here's a cut at the rewording:
>>
>> /**
>> * Returns the parameters used with this signature object.
>> *
>> * If this signature has been previously initialized with parameters
>> * (by calling the {@code setParameter} method), this method returns
>> * the same parameters. If this signature has not been initialized with
>> * parameters, this method may return a combination of default and
>> * randomly generated parameter values if the underlying
>> * signature implementation supports it and can successfully generate
>> * them. Otherwise, {@code null} is returned.
>> *
>> * @return the parameters used with this signature, or {@code null}
>> */
>>
>> In the first sentence of the 2nd paragraph above, I wanted to first list the
>> case where the signature is initialized with parameters, which is the most
>> clear-cut case of what the behavior will be. Then I described the case where
>> an implementation may return default/generated parameters -- and this is
>> clearly marked "may" since it is optional. All other cases other than those
>> two always return null, so I think this makes it easier to understand.
>>
>> --Sean
>>
>>> On 7/18/18 1:29 PM, Valerie Peng wrote:
>>> Sean,
>>>
>>> Where do you think that we should add the part about "null must be returned
>>> ..." paragraph? At the end of first or second paragraph?
>>>
>>> I will go with majority.
>>>
>>> Valerie
>>>
On 7/17/2018 8:38 PM, Weijun Wang wrote:
Is it better to append the new lines to the 2nd paragraph?
Thanks
Max
> On Jul 18, 2018, at 9:46 AM, Valerie Peng wrote:
>
>
> Ok, let's use "must" then. I have also added the part about default
> parameters.
> Hope that all is clear now.
>
> Latest webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.03/
> Latest CSR: https://bugs.openjdk.java.net/browse/JDK-8206864
>
> Thanks,
> Valerie
>
> On 7/17/2018 5:50 PM, Weijun Wang wrote:
>>> On Jul 18, 2018, at 8:23 AM, Valerie Peng
>>> wrote:
>>>
>>> Hi Max,
>>>
>>> Thanks for the suggestions. Please find comments inline.
>>>
On 7/16/2018 7:38 PM, Weijun Wang wrote:
CSR at https://bugs.openjdk.java.net/browse/JDK-8206864.
- At the end of the 1st paragraph, you have now
>However, for signature algorithm such as "RSASSA-PSS", it requires
> parameters and the one used for signing must be supplied for
> verification to succeed. It may be better to return null instead of
> returning provider-specific default parameters.
I suggest we don't talk about the reason (params must be the same for
signing and verification), we can just say something like "If there is
no provider-specific default parameters, this method should return
null before user sets one".
>>> Alright, I initially didn't put down the reason. But since Brad asked
>>> about it, I add it to the CSR in case Joe raise the same question. I
>>> will update the CSR per your suggestion.
- null may be returned
How about "{@code null} must be returned"?
>>> How about "should"? Is there a guideline on when to use
>>> "may/should/must"? Anyone knows?
>> Even if there were guidelines on this for Java, I think we should just
>> stick to https://tools.ietf.org/html/rfc2119, except that the
>> capitalization is not necessary.
>>
>> "Must" is precise here.
>>
>>> I thought must is mostly used in mandating input arguments must not be
>>> null. But don't recall it being used much for return values.
>> "must return" appears 39 times in java/ and "should return" 19 (simple
>> grep, no line break).
>>
>> --Max
>>
>>> Thanks,
>>> Valerie
>>>
Everything else looks fine.
Thanks
Max
> On Jul 17, 2018, at 9:46 AM, Valerie Peng
> wrote:
>
> Hi Max,
>
> Good catch on the SignatureSpi class and SunMSCAPI provider, I will
> include them.
>
> As for the part about "randomly generated", I am leaning toward not
> having it as I don't see a value of specifying this.
>
> Webrev and CSR has been updated.
>
> New
Re: RFR[11] JDK-8206171: Signature#getParameters for RSASSA-PSS throws ProviderException when not initialized
Hi Sean,
Thanks for the suggestion, I like it.
Max, any objection or concern before I update the webrev and CSR?
Valerie
On 7/19/2018 7:28 AM, Sean Mullan wrote:
Hi Valerie, Max -
I took a fresh look at this issue this morning. I think we are getting
bogged down by trying to adjust within the current wording, which is
somewhat awkward and hard to understand. So, I think it might be
better to break up the wording into multiple sentences. Here's a cut
at the rewording:
/**
* Returns the parameters used with this signature object.
*
* If this signature has been previously initialized with parameters
* (by calling the {@code setParameter} method), this method returns
* the same parameters. If this signature has not been initialized with
* parameters, this method may return a combination of default and
* randomly generated parameter values if the underlying
* signature implementation supports it and can successfully generate
* them. Otherwise, {@code null} is returned.
*
* @return the parameters used with this signature, or {@code null}
*/
In the first sentence of the 2nd paragraph above, I wanted to first
list the case where the signature is initialized with parameters,
which is the most clear-cut case of what the behavior will be. Then I
described the case where an implementation may return
default/generated parameters -- and this is clearly marked "may" since
it is optional. All other cases other than those two always return
null, so I think this makes it easier to understand.
--Sean
On 7/18/18 1:29 PM, Valerie Peng wrote:
Sean,
Where do you think that we should add the part about "null must be
returned ..." paragraph? At the end of first or second paragraph?
I will go with majority.
Valerie
On 7/17/2018 8:38 PM, Weijun Wang wrote:
Is it better to append the new lines to the 2nd paragraph?
Thanks
Max
On Jul 18, 2018, at 9:46 AM, Valerie Peng
wrote:
Ok, let's use "must" then. I have also added the part about default
parameters.
Hope that all is clear now.
Latest webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.03/
Latest CSR: https://bugs.openjdk.java.net/browse/JDK-8206864
Thanks,
Valerie
On 7/17/2018 5:50 PM, Weijun Wang wrote:
On Jul 18, 2018, at 8:23 AM, Valerie Peng
wrote:
Hi Max,
Thanks for the suggestions. Please find comments inline.
On 7/16/2018 7:38 PM, Weijun Wang wrote:
CSR at https://bugs.openjdk.java.net/browse/JDK-8206864.
- At the end of the 1st paragraph, you have now
However, for signature algorithm such as "RSASSA-PSS", it
requires parameters and the one used for signing must be
supplied for verification to succeed. It may be better to
return null instead of returning provider-specific default
parameters.
I suggest we don't talk about the reason (params must be the
same for signing and verification), we can just say something
like "If there is no provider-specific default parameters, this
method should return null before user sets one".
Alright, I initially didn't put down the reason. But since Brad
asked about it, I add it to the CSR in case Joe raise the same
question. I will update the CSR per your suggestion.
- null may be returned
How about "{@code null} must be returned"?
How about "should"? Is there a guideline on when to use
"may/should/must"? Anyone knows?
Even if there were guidelines on this for Java, I think we should
just stick to https://tools.ietf.org/html/rfc2119, except that the
capitalization is not necessary.
"Must" is precise here.
I thought must is mostly used in mandating input arguments must
not be null. But don't recall it being used much for return values.
"must return" appears 39 times in java/ and "should return" 19
(simple grep, no line break).
--Max
Thanks,
Valerie
Everything else looks fine.
Thanks
Max
On Jul 17, 2018, at 9:46 AM, Valerie Peng
wrote:
Hi Max,
Good catch on the SignatureSpi class and SunMSCAPI provider, I
will include them.
As for the part about "randomly generated", I am leaning toward
not having it as I don't see a value of specifying this.
Webrev and CSR has been updated.
New webrev:
http://cr.openjdk.java.net/~valeriep/8206171/webrev.02/
Thanks,
Valerie
On 7/16/2018 4:29 PM, Weijun Wang wrote:
Valerie
Would you like to retain the word "randomly generated" somewhere?
Also, the SignatureSpi class needs to be updated in the same way.
Can you also update the implementation in the MSCAPI Signature
object?
Thanks
Max
On Jul 17, 2018, at 6:16 AM, Valerie Peng
wrote:
No issues found and it seems ok to return null if no
parameters is set. Thus, I have updated the webrev and CSR
accordingly.
Bug: https://bugs.openjdk.java.net/browse/JDK-8206171
Webrev: http://cr.openjdk.java.net/~valeriep/8206171/webrev.01/
CSR: https://bugs.openjdk.java.net/browse/JDK-8206864
Thanks,
Valerie
On 7/13/2018 11:05 AM, Valerie Peng wrote:
Hmm, I like the idea of expanding null to cover both cases.
I will explore it more and see.
Re: EC weirdness
On 7/16/2018 4:42 PM, Adam Petcher wrote: I think the reason for the hard-coded curves is largely historical, and I don't know of a security-related reason for it (other than the prevention of insecure parameters). Though it has the additional benefit that it simplifies the interface a lot, because the implementation for a curve is not completely determined by the domain parameters. Actually - they are. An ECGenParameterSpec (basically a named curve) is turned into an ECParameterSpec (the actual curve and G stuff) which is turned into an AlgorithmSpecification of type (SunEC, EC) which strangely tries to do a reverse lookup of the parameter spec to get a name for the curve. (That's not exactly it, but its close in spirit). The implementation may also need to know which optimized finite field implementation, precomputed tables, etc to use. Looking up all of these things at once can be a lot simpler than trying to specify them over an interface. Nope - that's not it. The underlying C code mostly uses generic bignum operations on the curve parameters rather than pre-computed things. My guess is that there were some dependencies on named curves in the EC library and the EC related stuff in the PKCS11 library. I'm trying to figure out if your problem motivates support for arbitrary base points, or if there is another way to get what you want. Is the base point a secret in your scheme? Either way, can you implement your scheme using KeyAgreement (e.g. using the "base point" as the public key and a random integer as the private key)? What if ECDH KeyAgreement supported three-party Diffie-Hellman, or you extract a point from generateSecret()? Would that help? In the scheme, G' is secret. This is basically the EC version of https://en.wikipedia.org/wiki/SPEKE. Two key pairs generated on P-256' (e.g. same curve params, but with G'), can use ECDH to get to the same shared secret. If your key pair wasn't generated with G', then you can't get to the same secret with someone who's key pair was. This is sort of an IOT experiment - I don't want to have to do an explicit authorization via a certificate binding a public key. I can generate a group G', and have all the various devices generate a key pair using that G'. The ability to get a pairwise shared secret with another device proves the membership in the group to that other device and vice versa. The problem with using ECDH is that generic ECDH operations throw away the Y coordinate. I can do xG' -> X' using the SunEC ECDH, but what I actually get as a result is the scalar X'.x. I've been trying to avoid using external libraries, but I'll probably end up using the BC point math stuff to make this work. I may do a bit more archeology and see whether there are actual dependencies on the build in curve data (e.g. PKCS11). If not, I may try and provide a patch refactoring out that limitation and doing a more thorough separation of the math from the packaging. Later, Mike On 7/13/2018 9:30 PM, Michael StJohns wrote: Hi - Every so often I run into some rather strange things in the way the Sun EC classes were built. Most recently, I was trying to use the SunEC provider to do a PACE like protocol. Basically, the idea was to be able to generate public key points on the P-256 curve, but with a different base point G (knowledge of G' or having a public key generated from G' would allow you to do a valid ECDH operation, keys with disjoint points would not). I was able to generate a normal key pair using ECGenParameterSpec with a name, so I grabbed the ECParameterSpec from the public key, made a copy of most of the stuff (curve, cofactor), but substituted the public point W from the key pair I'd just generated, and passed that in as G to the new parameter spec. I tried to initialize the KPG with my *almost* P-256 spec, and it failed with "unsupported curve". Looking into the code and tracing through sun.crypto.ec.ECKeyPairGenerator to the native code, I'm once again surprised that all of the curves are hard coded into the C native code, rather than being passed in as data from the Java code. Is there a good security reason for hard coding the curves at the C level that I'm missing? This comes under the heading of unexpected behavior rather than a bug per se I think. Although - I'd *really* like to be able to pass a valid ECParameterSpec in to the generation process and get whatever keys are appropriate for that curve and base point. Later, Mike
