Code Review Request for 7196805: DH Key interoperability testing between SunJCE and JsafeJCE not successful

[Weijun Wang]

Hi Valerie

Not sure if we really need to fix the equals() method here. Is there a 
spec saying the equality is based on internal fields instead of encoded 
bytes?

I am saying this because it's not easy to get equals() correct here. In 
DHPrivateKey.java, the new equals() requires the other object to be 
instanceof javax.crypto.interfaces.DHPrivateKey. I see that another 
class P11DHPrivateKey also implements this interface but has not 
overridden equals, so it looks like a 
com/sun/crypto/provider/DHPrivateKey could equals() to a 
P11DHPrivateKey, but a P11DHPrivateKey will not equals() to a 
com/sun/crypto/provider/DHPrivateKey.

Same with DHPublicKey.java.

BTW, the hashCode() methods in these 2 classes use <<2 and <<4 which 
looks not as cool as the normal "*31+". I would simply use 
Objects.hash(....) method.

For DHKeyPairGenerator.java, it looks like you don't want the first 
octet being zero. Is this related to this bug? Is that required in the 
"Handbook of Applied Cryptography" book? I understand it could be 
necessary for interop.

Thanks
Max

On 5/29/13 9:25 AM, Valerie (Yu-Ching) Peng wrote:
> Vinnie,
>
> Can you help reviewing my fix for 7196805 "DH Key interoperability
> testing between SunJCE and JsafeJCE not successful"?
>
> In SunJCE provider, the equality check for DH private/public keys is
> based on DER encoding which may not be correct all the time due to the
> optional L value defined in the DER syntax. In addition, JsafeJCE
> provider sometimes encode the optional L value incorrectly which leads
> to unexpected IOException when parsing the DER bytes.
> I have changed the comparison to based on component values rather than
> DER encodings which may vary due to the presence or missing of optional
> values. In addition, I made the changes to DHKeyPairGenerator to ensure
> that generated private value has the requested length/size.
>
> Webrev: http://cr.openjdk.java.net/~valeriep/7196805/webrev.00/
>
> Thanks,
> Valerie