Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-05-05 Thread Valerie Peng
On Wed, 4 May 2022 04:16:42 GMT, Xue-Lei Andrew Fan  wrote:

>> How about the case when no parameters are given? Say A is the user-supplied 
>> values, B is the provider specific default or random values, your suggestion 
>> has A, A+B, and null. Isn't the sentence about B needed (no A and provider 
>> can generate the parameters)?
>
> I think this sentence covers case B, "... or may contain additional default 
> or random parameter
> values used by the underlying signature implementation."

Sean's comment on the other PR regarding Cipher.getParameters implies 
otherwise, i.e. `A few comments:

I don't think this new text covers the case where the cipher is not 
initialized with any parameters, but the provider returns parameters. I tried 
to think of how to say that all in one sentence, but I think it is best to have 
two sentences covering each case, ex: "If this cipher was not initialized with 
parameters, the returned parameters may be null or may be ..." "If this cipher 
was initialized with parameters, the returned parameters may be the same or may 
be ..."`

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-05-05 Thread Valerie Peng
On Thu, 5 May 2022 22:03:16 GMT, Valerie Peng  wrote:

>> I think this sentence covers case B, "... or may contain additional default 
>> or random parameter
>> values used by the underlying signature implementation."
>
> Sean's comment on the other PR regarding Cipher.getParameters implies 
> otherwise, i.e. `A few comments:
> 
> I don't think this new text covers the case where the cipher is not 
> initialized with any parameters, but the provider returns parameters. I tried 
> to think of how to say that all in one sentence, but I think it is best to 
> have two sentences covering each case, ex: "If this cipher was not 
> initialized with parameters, the returned parameters may be null or may be 
> ..." "If this cipher was initialized with parameters, the returned parameters 
> may be the same or may be ..."`

He later agrees that two sentences are probably too lengthy, so we ended up 
exploring other wording choices.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-05-04 Thread Valerie Peng
On Tue, 3 May 2022 00:17:11 GMT, Weijun Wang  wrote:

>> An example is RSASSA-PSS, i.e. it requires the caller to explicitly state 
>> which message digest to use, etc.
>
> You listed 2 cases when null is returned: 1) not supplied. 2) cannot 
> generate. My understanding is that the RSASSA-PSS case is 1), and the EdDSA 
> case is 2). This is why I suggested an "or" relation between them.

For EdDSA, it's really the case of the signature impl cannot convert the 
supplied parameter values to "AlgorithmParameters" object.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-05-03 Thread Xue-Lei Andrew Fan
On Mon, 2 May 2022 21:42:28 GMT, Valerie Peng  wrote:

>>> What kind of additional sentence do you have in mind?
>> 
>> It may be fine to put it into the state for 'null" returned value.  For 
>> example:
>> 
>> 
>> The returned parameters may be the same that were used to initialize
>> this signature, or may contain additional default or random parameter
>> values used by the underlying signature implementation, or null if the
>> underlying signature implementation does not support returning the
>> parameters as {@code AlgorithmParameters}.
>> 
>> 
>> 
>> The null return conditional in the following sentence may be able to combine 
>> together.
>> 
>> 
>> The returned parameters may be the same that were used to initialize
>> this signature, or may contain additional default or random parameter
>> values used by the underlying signature implementation.  {@code null}
>> may be returned if the underlying signature implementation does not
>> support returning the parameters as {@code AlgorithmParameters}, or > conditions>
>
> How about the case when no parameters are given? Say A is the user-supplied 
> values, B is the provider specific default or random values, your suggestion 
> has A, A+B, and null. Isn't the sentence about B needed (no A and provider 
> can generate the parameters)?

I think this sentence covers case B, "... or may contain additional default or 
random parameter
values used by the underlying signature implementation."

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-05-02 Thread Weijun Wang
On Mon, 2 May 2022 21:14:21 GMT, Valerie Peng  wrote:

>> Then what does "cannot generate parameter values" mean? Any example?
>
> An example is RSASSA-PSS, i.e. it requires the caller to explicitly state 
> which message digest to use, etc.

You listed 2 cases when null is returned: 1) not supplied. 2) cannot generate. 
My understanding is that the RSASSA-PSS case is 1), and the EdDSA case is 2). 
This is why I suggested an "or" relation between them.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-05-02 Thread Valerie Peng
On Fri, 29 Apr 2022 04:27:36 GMT, Xue-Lei Andrew Fan  wrote:

>> What kind of additional sentence do you have in mind?
>
>> What kind of additional sentence do you have in mind?
> 
> It may be fine to put it into the state for 'null" returned value.  For 
> example:
> 
> 
> The returned parameters may be the same that were used to initialize
> this signature, or may contain additional default or random parameter
> values used by the underlying signature implementation, or null if the
> underlying signature implementation does not support returning the
> parameters as {@code AlgorithmParameters}.
> 
> 
> 
> The null return conditional in the following sentence may be able to combine 
> together.
> 
> 
> The returned parameters may be the same that were used to initialize
> this signature, or may contain additional default or random parameter
> values used by the underlying signature implementation.  {@code null}
> may be returned if the underlying signature implementation does not
> support returning the parameters as {@code AlgorithmParameters}, or  conditions>

How about the case when no parameters are given? Say A is the user-supplied 
values, B is the provider specific default or random values, your suggestion 
has A, A+B, and null. Isn't the sentence about B needed (no A and provider can 
generate the parameters)?

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-05-02 Thread Valerie Peng
On Thu, 28 Apr 2022 23:28:39 GMT, Weijun Wang  wrote:

>> The impl does not need to generate parameter values, but rather cannot 
>> convert the supplied parameter values into AlgorithmParameter objects. By 
>> parameter values, I mean the components of the parameters.
>
> Then what does "cannot generate parameter values" mean? Any example?

An example is RSASSA-PSS, i.e. it requires the caller to explicitly state which 
message digest to use, etc.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-28 Thread Xue-Lei Andrew Fan
On Thu, 28 Apr 2022 23:09:00 GMT, Valerie Peng  wrote:

> What kind of additional sentence do you have in mind?

It may be fine to put it into the state for 'null" returned value.  For example:

The returned parameters may be the same that were used to initialize
this signature, or may contain additional default or random parameter
values used by the underlying signature implementation, **or null if the
underlying signature implementation does not support returning the
parameters as {@code AlgorithmParameters}.**

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-28 Thread Weijun Wang
On Thu, 28 Apr 2022 23:22:30 GMT, Valerie Peng  wrote:

>> I suggest the last sentence to be "null is returned if the required 
>> parameters were not supplied **or** the underlying signature implementation 
>> cannot generate the parameter values." I used "or" because for EdDSA 
>> parameters are supplied but the impl cannot generate parameter values.
>
> The impl does not need to generate parameter values, but rather cannot 
> convert the supplied parameter values into AlgorithmParameter objects. By 
> parameter values, I mean the components of the parameters.

Then what does "cannot generate parameter values" mean? Any example?

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-28 Thread Valerie Peng
On Thu, 28 Apr 2022 23:14:56 GMT, Weijun Wang  wrote:

>> I assume you were suggesting this? `"The returned parameters may be the same 
>> that were used to initialize this signature, or may contain additional 
>> default or random parameter values used by the underlying signature 
>> implementation. {https://github.com/code null} is returned if the required 
>> parameters were not supplied and the underlying signature implementation 
>> cannot generate the parameter values."`
>> But the "the underlying signature implementation supports returning the 
>> parameters as {https://github.com/code AlgorithmParameters}" is necessary. 
>> Strictly speaking, this is somewhat different than the "cannot generate 
>> parameter values" though. Perhaps we should go a bit broader for the last 
>> sentence regarding null return value?
>
> I suggest the last sentence to be "null is returned if the required 
> parameters were not supplied **or** the underlying signature implementation 
> cannot generate the parameter values." I used "or" because for EdDSA 
> parameters are supplied but the impl cannot generate parameter values.

The impl does not need to generate parameter values, but rather cannot convert 
the supplied parameter values into AlgorithmParameter objects. By parameter 
values, I mean the components of the parameters.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-28 Thread Weijun Wang
On Thu, 28 Apr 2022 23:08:17 GMT, Valerie Peng  wrote:

>> So, "the underlying signature implementation supports returning the 
>> parameters as {@code AlgorithmParameters}" is quite necessary. Xuelei's 
>> suggestion is quite good, just change the last "and" to "or".
>
> I assume you were suggesting this? `"The returned parameters may be the same 
> that were used to initialize this signature, or may contain additional 
> default or random parameter values used by the underlying signature 
> implementation. {https://github.com/code null} is returned if the required 
> parameters were not supplied and the underlying signature implementation 
> cannot generate the parameter values."`
> But the "the underlying signature implementation supports returning the 
> parameters as {https://github.com/code AlgorithmParameters}" is necessary. 
> Strictly speaking, this is somewhat different than the "cannot generate 
> parameter values" though. Perhaps we should go a bit broader for the last 
> sentence regarding null return value?

I suggest the last sentence to be "null is returned if the required parameters 
were not supplied **or** the underlying signature implementation cannot 
generate the parameter values." I used "or" because for EdDSA parameters are 
supplied but the impl cannot generate parameter values.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-28 Thread Valerie Peng
On Wed, 27 Apr 2022 23:02:28 GMT, Weijun Wang  wrote:

>> Right, the user-supplied values takes precedence and provider-specific 
>> default/random values should just be supplemental.
>> 
>> As for EdDSA, looks like the prehash and context are only in RFC 8032 and 
>> NOT RFC 8410.  caller has to pass these settings/values to the other entity 
>> through some other means since the getParameters() return null as there is 
>> no ASN.1 definition for the parameters. Looks like these values are packaged 
>> into a parameter spec and passed into the underlying EdDSA signature 
>> implementation in order to fit into existing API without adding new 
>> algorithm specific APIs.
>
> So, "the underlying signature implementation supports returning the 
> parameters as {@code AlgorithmParameters}" is quite necessary. Xuelei's 
> suggestion is quite good, just change the last "and" to "or".

I assume you were suggesting this? `"The returned parameters may be the same 
that were used to initialize this signature, or may contain additional default 
or random parameter values used by the underlying signature implementation. 
{https://github.com/code null} is returned if the required parameters were not 
supplied and the underlying signature implementation cannot generate the 
parameter values."`
But the "the underlying signature implementation supports returning the 
parameters as {https://github.com/code AlgorithmParameters}" is necessary. 
Strictly speaking, this is somewhat different than the "cannot generate 
parameter values" though. Perhaps we should go a bit broader for the last 
sentence regarding null return value?

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-28 Thread Valerie Peng
On Thu, 28 Apr 2022 04:56:47 GMT, Xue-Lei Andrew Fan  wrote:

>>> Can you clarify what is the A and B that you are referring to?
>> 
>> The sentence is, “If the required parameters were not supplied and the 
>> underlying signature implementation can generate the parameter values, it 
>> will be returned. Otherwise, {https://github.com/code null} is returned."
>> 
>> I read "the required parameters were not supplied" as condition A; and "the 
>> underlying signature implementation can generate the parameter values" as 
>> condition B.
>
>> With Signature class, there is a caveat for EdDSA, the supplied parameters 
>> are set but null is being returned when getParameters() is called. This is 
>> currently covered by the condition `if the underlying signature 
>> implementation supports returning the parameters as {@code 
>> AlgorithmParameters}` as the underlying signature does not support 
>> AlgorithmParameters for the supplied EdDSAParameterSpec object due to lack 
>> of ASN.1 definition.
> 
> I see now.  My 1st read of the condition, I did not get the point and thought 
> it is not necessary as the method is trying to return "AlgorithmParameters".  
> Could it be more clear if describe this case in an additional sentence?

What kind of additional sentence do you have in mind?

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Xue-Lei Andrew Fan
On Thu, 28 Apr 2022 04:44:43 GMT, Xue-Lei Andrew Fan  wrote:

>>> > What does it refer to with 'it'? Is 'it' refer to the implementation 
>>> > generated parameter values?
>>> 
>>> 'It' refers to the parameters containing all of the parameter values 
>>> including the supplied ones and provider-generated ones if any.
>> 
>> The full sentence is, "If the required parameters were not supplied and the 
>> underlying signature implementation can generate the parameter values, it 
>> will be returned."   As there is no supplied value, I think 'it' refer to 
>> the provider-generated ones if any.  As the previous noun is "the parameters 
>> values", I'm not sure if the use of 'it' here is properly.
>
>> Can you clarify what is the A and B that you are referring to?
> 
> The sentence is, “If the required parameters were not supplied and the 
> underlying signature implementation can generate the parameter values, it 
> will be returned. Otherwise, {https://github.com/code null} is returned."
> 
> I read "the required parameters were not supplied" as condition A; and "the 
> underlying signature implementation can generate the parameter values" as 
> condition B.

> With Signature class, there is a caveat for EdDSA, the supplied parameters 
> are set but null is being returned when getParameters() is called. This is 
> currently covered by the condition `if the underlying signature 
> implementation supports returning the parameters as {@code 
> AlgorithmParameters}` as the underlying signature does not support 
> AlgorithmParameters for the supplied EdDSAParameterSpec object due to lack of 
> ASN.1 definition.

I see now.  My 1st read of the condition, I did not get the point and thought 
it is not necessary as the method is trying to return "AlgorithmParameters".  
Could it be more clear if describe this case in an additional sentence?

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Xue-Lei Andrew Fan
On Wed, 27 Apr 2022 23:35:19 GMT, Valerie Peng  wrote:

>> With Signature class, there is a caveat for EdDSA, the supplied parameters 
>> are set but null is being returned when getParameters() is called. This is 
>> currently covered by the condition `if the underlying signature 
>> implementation supports returning the parameters as {@code 
>> AlgorithmParameters}` as the underlying signature does not support 
>> AlgorithmParameters for the supplied EdDSAParameterSpec object due to lack 
>> of ASN.1 definition.
>
> Besides this Signature-specific condition, there is the common condition 
> where provider cannot (or do not) generate default parameter values. {@code 
> null} is used as the catch-all result, but as you said, describe various 
> conditions tersely and correctly is key.

> > What does it refer to with 'it'? Is 'it' refer to the implementation 
> > generated parameter values?
> 
> 'It' refers to the parameters containing all of the parameter values 
> including the supplied ones and provider-generated ones if any.

The full sentence is, "If the required parameters were not supplied and the 
underlying signature implementation can generate the parameter values, it will 
be returned."   As there is no supplied value, I think 'it' refer to the 
provider-generated ones if any.  As the previous noun is "the parameters 
values", I'm not sure if the use of 'it' here is properly.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Xue-Lei Andrew Fan
On Thu, 28 Apr 2022 04:41:20 GMT, Xue-Lei Andrew Fan  wrote:

>> Besides this Signature-specific condition, there is the common condition 
>> where provider cannot (or do not) generate default parameter values. {@code 
>> null} is used as the catch-all result, but as you said, describe various 
>> conditions tersely and correctly is key.
>
>> > What does it refer to with 'it'? Is 'it' refer to the implementation 
>> > generated parameter values?
>> 
>> 'It' refers to the parameters containing all of the parameter values 
>> including the supplied ones and provider-generated ones if any.
> 
> The full sentence is, "If the required parameters were not supplied and the 
> underlying signature implementation can generate the parameter values, it 
> will be returned."   As there is no supplied value, I think 'it' refer to the 
> provider-generated ones if any.  As the previous noun is "the parameters 
> values", I'm not sure if the use of 'it' here is properly.

> Can you clarify what is the A and B that you are referring to?

The sentence is, “If the required parameters were not supplied and the 
underlying signature implementation can generate the parameter values, it will 
be returned. Otherwise, {https://github.com/code null} is returned."

I read "the required parameters were not supplied" as condition A; and "the 
underlying signature implementation can generate the parameter values" as 
condition B.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Valerie Peng
On Wed, 27 Apr 2022 23:30:22 GMT, Valerie Peng  wrote:

>> Can you clarify what is the A and B that you are referring to? The way I 
>> read it, it has more than 2 conditions... So, best to clarify the conditions 
>> first. 
>> I see your point with the wording suggestion at the end. Was a bit lost when 
>> trying to go through the various if-else logic and figure out what you 
>> mean...
>
> With Signature class, there is a caveat for EdDSA, the supplied parameters 
> are set but null is being returned when getParameters() is called. This is 
> currently covered by the condition `if the underlying signature 
> implementation supports returning the parameters as {@code 
> AlgorithmParameters}` as the underlying signature does not support 
> AlgorithmParameters for the supplied EdDSAParameterSpec object due to lack of 
> ASN.1 definition.

Besides this Signature-specific condition, there is the common condition where 
provider cannot (or do not) generate default parameter values. {@code null} is 
used as the catch-all result, but as you said, describe various conditions 
tersely and correctly is key.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Valerie Peng
On Wed, 27 Apr 2022 23:19:56 GMT, Valerie Peng  wrote:

>>> What does it refer to with 'it'? Is 'it' refer to the implementation 
>>> generated parameter values?
>> 
>> 'It' refers to the parameters containing all of the parameter values 
>> including the supplied ones and provider-generated ones if any.
>
> Can you clarify what is the A and B that you are referring to? The way I read 
> it, it has more than 2 conditions... So, best to clarify the conditions 
> first. 
> I see your point with the wording suggestion at the end. Was a bit lost when 
> trying to go through the various if-else logic and figure out what you mean...

With Signature class, there is a caveat for EdDSA, the supplied parameters are 
set but null is being returned when getParameters() is called. This is 
currently covered by the condition `if the underlying signature implementation 
supports returning the parameters as {@code AlgorithmParameters}` as the 
underlying signature does not support AlgorithmParameters for the supplied 
EdDSAParameterSpec object due to lack of ASN.1 definition.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Valerie Peng
On Wed, 27 Apr 2022 23:15:41 GMT, Valerie Peng  wrote:

>> src/java.base/share/classes/java/security/Signature.java line 1014:
>> 
>>> 1012:  * {@code AlgorithmParameters}. If the required
>>> 1013:  * parameters were not supplied and the underlying signature 
>>> implementation
>>> 1014:  * can generate the parameter values, it will be returned. 
>>> Otherwise,
>> 
>>> If the required parameters were not supplied and the underlying signature 
>>> implementation can generate the parameter values, it will be returned.
>> 
>> What does it refer to with 'it'? Is 'it' refer to the implementation 
>> generated parameter values?
>> 
>>> If the required parameters were not supplied and the underlying signature 
>>> implementation can generate the parameter values, it will be returned. 
>>> Otherwise, {@code null} is returned.
>> 
>> The logic looks like
>> 
>> if (A & B) {
>> // it will be returned
>> } else {
>> // {@code null} is returned.
>> }
>> 
>> If I read it correctly, the behavior may look like:
>> 1. If the required parameters were supplied, {@code null} is returned; (if 
>> !A)
>> 2. if the underlying signature implementation cannot generate the parameter 
>> values, {@code null} is returned; (if !B)
>> 3. if not 1 and 2, ... (if A & B)
>> 
>> It does not look like right.  The expected behavior may be:
>> 
>> if (A) {
>> if (B) {
>> // it will be returned
>> } else {
>> // {@code null} is returned.
>> }
>> }
>> 
>> 
>> Maybe, the confusion could be mitigated by re-org the logic:
>> 
>>  if (A & !B) {
>> // {@code null} is returned.
>>  } // Otherwise, refer to 1st sentence.
>> 
>> 
>> "The returned parameters may be the same that were used to initialize this 
>> signature, or may contain additional default or random parameter values used 
>> by the underlying signature implementation.   {@code null} is returned if 
>> the required parameters were not supplied and the underlying signature 
>> implementation cannot generate the parameter values."
>> 
>> Similar comment to [PR 8117](https://github.com/openjdk/jdk/pull/8117), if 
>> you want to use similar description there as well.
>
>> What does it refer to with 'it'? Is 'it' refer to the implementation 
>> generated parameter values?
> 
> 'It' refers to the parameters containing all of the parameter values 
> including the supplied ones and provider-generated ones if any.

Can you clarify what is the A and B that you are referring to? The way I read 
it, it has more than 2 conditions... So, best to clarify the conditions first.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Valerie Peng
On Wed, 27 Apr 2022 05:25:42 GMT, Xue-Lei Andrew Fan  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Undo un-intentional changes.
>
> src/java.base/share/classes/java/security/Signature.java line 1012:
> 
>> 1010:  * values used by the underlying signature implementation if the 
>> underlying
>> 1011:  * signature implementation supports returning the parameters as
>> 1012:  * {@code AlgorithmParameters}. If the required
> 
> "... if the underlying signature implementation supports returning the 
> parameters a {@code AlgorithmParameters}", it looks this part is duplicated 
> and may be removed.

I am not sure which part duplicates the text that you quoted? The original text 
only mentions the supplied values. The proposed changes add that there may be 
additional parameters. Are you saying that these additional parameters are 
"implied" by the part that you quoted?

> What does it refer to with 'it'? Is 'it' refer to the implementation 
> generated parameter values?

'It' refers to the parameters containing all of the parameter values including 
the supplied ones and provider-generated ones if any.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Weijun Wang
On Wed, 27 Apr 2022 22:54:47 GMT, Valerie Peng  wrote:

>> RSASSA-PSS always requires a user-provided params.
>> 
>> I think one thing we can guarantee is that the default/random values 
>> generated by the impl will never overwrite the user-provided ones, they will 
>> only be supplemented. Also, I think the real usage of this method is that 
>> the result -- after converting to a spec -- can be set on the verify side 
>> and the verification should succeed.
>> 
>> I don't quite understand EdDSA. Looks like the params is not stored along 
>> with the signature in the algorithm identifier. I also haven't found 
>> different OIDs defined when prehash or context is used. How does the 
>> verifier know this kind of flavor settings?
>
> Right, the user-supplied values takes precedence and provider-specific 
> default/random values should just be supplemental.
> 
> As for EdDSA, looks like the prehash and context are only in RFC 8032 and NOT 
> RFC 8410.  caller has to pass these settings/values to the other entity 
> through some other means since the getParameters() return null as there is no 
> ASN.1 definition for the parameters. Looks like these values are packaged 
> into a parameter spec and passed into the underlying EdDSA signature 
> implementation in order to fit into existing API without adding new algorithm 
> specific APIs.

So, "the underlying signature implementation supports returning the parameters 
as {@code AlgorithmParameters}" is quite necessary. Xuelei's suggestion is 
quite good, just change the last "and" to "or".

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Valerie Peng
On Wed, 27 Apr 2022 15:10:42 GMT, Weijun Wang  wrote:

>> I searched about "and/or" and it is said that "or" covers "and". So, 
>> "and/or" should just be "or".
>> 
>> I am on the fence for requiring provider to generate default parameters 
>> (using provider-specific or random values). Could there be scenarios where 
>> users are required to supply parameters? 
>> That aside, this javadoc is lastly updated by JDK-8243424. It seems that 
>> JDK's EdDSA signature impl has some interesting usage of 
>> AlgorithmParameterSpec which this javadoc has to cover/allow.
>
> RSASSA-PSS always requires a user-provided params.
> 
> I think one thing we can guarantee is that the default/random values 
> generated by the impl will never overwrite the user-provided ones, they will 
> only be supplemented. Also, I think the real usage of this method is that the 
> result -- after converting to a spec -- can be set on the verify side and the 
> verification should succeed.
> 
> I don't quite understand EdDSA. Looks like the params is not stored along 
> with the signature in the algorithm identifier. I also haven't found 
> different OIDs defined when prehash or context is used. How does the verifier 
> know this kind of flavor settings?

Right, the user-supplied values takes precedence and provider-specific 
default/random values should just be supplemental.

As for EdDSA, looks like the prehash and context are only in RFC 8032 and NOT 
RFC 8410.  caller has to pass these settings/values to the other entity through 
some other means since the getParameters() return null as there is no ASN.1 
definition for the parameters. Looks like these values are packaged into a 
parameter spec and passed into the underlying EdDSA signature implementation in 
order to fit into existing API without adding new algorithm specific APIs.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Weijun Wang
On Wed, 27 Apr 2022 00:35:49 GMT, Valerie Peng  wrote:

>> src/java.base/share/classes/java/security/Signature.java line 1015:
>> 
>>> 1013:  * parameters were not supplied and the underlying signature 
>>> implementation
>>> 1014:  * can generate the parameter values, it will be returned. 
>>> Otherwise,
>>> 1015:  * {@code null} is returned.
>> 
>> If we cannot guarantee anything but just want to clarify. how about just say 
>> "The returned parameters is a combination of user-provided values, 
>> implementation default values, and/or random values used by the underlying 
>> signature implementation. It can also be {@code null} if there's none."
>> 
>> I assume that if a value is necessary but user has not provided one, then 
>> there will be a default or a random one.
>
> I searched about "and/or" and it is said that "or" covers "and". So, "and/or" 
> should just be "or".
> 
> I am on the fence for requiring provider to generate default parameters 
> (using provider-specific or random values). Could there be scenarios where 
> users are required to supply parameters? 
> That aside, this javadoc is lastly updated by JDK-8243424. It seems that 
> JDK's EdDSA signature impl has some interesting usage of 
> AlgorithmParameterSpec which this javadoc has to cover/allow.

RSASSA-PSS always requires a user-provided params.

I think one thing we can guarantee is that the default/random values generated 
by the impl will never overwrite the user-provided ones, they will only be 
supplemented. Also, I think the real usage of this method is that the result -- 
after converting to a spec -- can be set on the verify side and the 
verification should succeed.

I don't quite understand EdDSA. Looks like the params is not stored along with 
the signature in the algorithm identifier. I also haven't found different OIDs 
defined when prehash or context is used. How does the verifier know this kind 
of flavor settings?

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-27 Thread Xue-Lei Andrew Fan
On Tue, 26 Apr 2022 17:51:45 GMT, Valerie Peng  wrote:

>> This is to update the method javadoc of 
>> java.security.Signature.getParameters() with the missing `@throws 
>> UnsupportedOperationException`. In addition, the wording on the returned 
>> parameters are updated to match those in Cipher and CipherSpi classes. 
>> 
>> CSR will be filed later.
>> 
>> Thanks,
>> Valerie
>
> Valerie Peng has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Undo un-intentional changes.

src/java.base/share/classes/java/security/Signature.java line 1012:

> 1010:  * values used by the underlying signature implementation if the 
> underlying
> 1011:  * signature implementation supports returning the parameters as
> 1012:  * {@code AlgorithmParameters}. If the required

"... if the underlying signature implementation supports returning the 
parameters a {@code AlgorithmParameters}", it looks this part is duplicated and 
may be removed.

src/java.base/share/classes/java/security/Signature.java line 1014:

> 1012:  * {@code AlgorithmParameters}. If the required
> 1013:  * parameters were not supplied and the underlying signature 
> implementation
> 1014:  * can generate the parameter values, it will be returned. 
> Otherwise,

> If the required parameters were not supplied and the underlying signature 
> implementation can generate the parameter values, it will be returned.

What does it refer to with 'it'? Is 'it' refer to the implementation generated 
parameter values?

> If the required parameters were not supplied and the underlying signature 
> implementation can generate the parameter values, it will be returned. 
> Otherwise, {@code null} is returned.

The logic looks like

if (A & B) {
// it will be returned
} else {
// {@code null} is returned.
}

If I read it correctly, the behavior may look like:
1. If the required parameters were supplied, {@code null} is returned; (if !A)
2. if the underlying signature implementation cannot generate the parameter 
values, {@code null} is returned; (if !B)
3. if not 1 and 2, ... (if A & B)

It does not look like right.  The expected behavior may be:

if (A) {
if (B) {
// it will be returned
} else {
// {@code null} is returned.
}
}


Maybe, the confusion could be mitigated by re-org the logic:

 if (A & !B) {
// {@code null} is returned.
 } // Otherwise, refer to 1st sentence.


"The returned parameters may be the same that were used to initialize this 
signature, or may contain additional default or random parameter values used by 
the underlying signature implementation.   {@code null} is returned if the 
required parameters were not supplied and the underlying signature 
implementation cannot generate the parameter values."

Similar comment to [PR 8117](https://github.com/openjdk/jdk/pull/8117), if you 
want to use similar description there as well.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-26 Thread Valerie Peng
On Tue, 26 Apr 2022 14:59:35 GMT, Weijun Wang  wrote:

>> Valerie Peng has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Undo un-intentional changes.
>
> src/java.base/share/classes/java/security/Signature.java line 1015:
> 
>> 1013:  * parameters were not supplied and the underlying signature 
>> implementation
>> 1014:  * can generate the parameter values, it will be returned. 
>> Otherwise,
>> 1015:  * {@code null} is returned.
> 
> If we cannot guarantee anything but just want to clarify. how about just say 
> "The returned parameters is a combination of user-provided values, 
> implementation default values, and/or random values used by the underlying 
> signature implementation. It can also be {@code null} if there's none."
> 
> I assume that if a value is necessary but user has not provided one, then 
> there will be a default or a random one.

I searched about "and/or" and it is said that "or" covers "and". So, "and/or" 
should just be "or".

I am on the fence for requiring provider to generate default parameters (using 
provider-specific or random values). Could there be scenarios where users are 
required to supply parameters? 
That aside, this javadoc is lastly updated by JDK-8243424. It seems that JDK's 
EdDSA signature impl has some interesting usage of AlgorithmParameterSpec which 
this javadoc has to cover/allow.

-

PR: https://git.openjdk.java.net/jdk/pull/8396


Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v2]

2022-04-26 Thread Valerie Peng
> This is to update the method javadoc of 
> java.security.Signature.getParameters() with the missing `@throws 
> UnsupportedOperationException`. In addition, the wording on the returned 
> parameters are updated to match those in Cipher and CipherSpi classes. 
> 
> CSR will be filed later.
> 
> Thanks,
> Valerie

Valerie Peng has updated the pull request incrementally with one additional 
commit since the last revision:

  Undo un-intentional changes.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8396/files
  - new: https://git.openjdk.java.net/jdk/pull/8396/files/033d4d85..9be3644d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8396=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8396=00-01

  Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8396.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8396/head:pull/8396

PR: https://git.openjdk.java.net/jdk/pull/8396