RFR: 8271566: DSA signature length value is not accurate in P11Signature [v2]

Martin Balao mbalao at openjdk.java.net
Sat Dec 4 22:14:22 UTC 2021


On Tue, 30 Nov 2021 19:48:19 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> Hmm, thinking more about "internal"/"opaque", given this is naming for the parent, maybe "internal" is more correct. The non-sensitive keys are encapsulated by the children classes and is still an instance of the parent. If you name the parent class w/ "opaque" suffix, it looks misleading/confusing. The opaqueness is implied by the implementation instead of the properties of the objects.
>
>> Hi @valeriepeng ,
>> 
>> Yes, I just verified how serialization works for P11Keys and your 'transient' change makes sense to me now.
>> 
>> I see your point about Internal/Opaque. Keep 'Internal' then if you prefer. The whole inheritance relationship between these classes sounds a bit weird to me, beyond the names we call them. I wouldn't suggest any additional changes there now, though.
>> 
>> It looks to me that the only 2 changes expected for Webrev.02 are: 1) P11RSAPrivateNonCRTKey to use the parent's protected 'n'; and 2) removal of 'long errorCode = e.getErrorCode();'.
>> 
>> Now that we almost have the changeset ready, I'm not sure of how to proceed with the push. Do you want me to commit Webrev.02 in my own branch and use the 'Co-authored-by:' header? If we do that, do we still need an additional Reviewer? Do you prefer to have your own branch? Please let me know of how to move forward.
>> 
>> Martin.-
> 
> You can just incorporate the changes on your branch and proceeds with me being the reviewer. The webrev that I sent u is just an easier way to express the comments/feedbacks.
> 
> Thanks,
> Valerie

Hi @valeriepeng ,

Please take a look at the latest change based on your comments.

No regressions found in 'jdk/sun/security/pkcs11'.

Thanks,
Martin.-

-------------

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



More information about the security-dev mailing list