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

Valerie Peng valeriep at openjdk.java.net
Fri Nov 19 00:23:43 UTC 2021


On Thu, 18 Nov 2021 19:27:30 GMT, Martin Balao <mbalao at openjdk.org> wrote:

> 
> 
> Hi @valeriepeng ,
> 
> Some comments and questions regarding Webrev.01:
> 
>     * P11Key.java
> 
>     * Would you consider replacing the 'Internal' suffix with 'Opaque'? I believe the term 'opaque' better reflects what these keys are: you cannot see their values -thus, opaque- but you can use them as-is. 'Internal' gives me the impression of something not supposed to be exposed; and this is not exactly the case.
> 
>     * Why are P11RSAPrivateKeyInternal::n, P11RSAPrivateKey::<e, d, p, q, pe, qe, coeff>, etc. now transient?
> 
>     * Now that P11RSAPrivateKey::n private field was removed, the extra PKCS#11 lib call I mentioned in Webrev.00 is not possible anymore. The override of ::getModulus looks good to me, though, for the reasons you said.
> 
>     * Can P11RSAPrivateNonCRTKey use the protected 'n' field from its parent instead of declaring a private one?
> 
>     * P11Signature.java
> 
>     * 'long errorCode = e.getErrorCode();' -> Looks to me that this change was included into the Webrev by mistake, and is part of JDK-8236512.
> 
> 
> Thanks, Martin.-

Either internal or opaque suffix works for me.  Fields aren't really being written out into the serialized bytes are "transient". IIRC, the serialized bytes are the "encoding" instead of the fields, we should mark these fields transient. As for the P11RSAPrivateNonCRTKey, yes, it should use the 'n' from parent instead of its own. I missed it when working on webrev.01. As for 'long errorCode = e.getErrorCode();' in P11Signature.java, yes, it should have been removed. I was manually merging the changes when refreshing the source tree and missed to remove this line.

Thanks,
Valerie

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

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



More information about the security-dev mailing list