Max,
Please find comments in line...
On 06/12/13 18:27, Weijun Wang wrote:
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?
There is no spec saying the equality check should be based on encoded
bytes neither.
DH keys are different from other keys in that its parameters contains an
optional field. Thus, even for 2 DH public keys whose internal fields
are identical (including the optional one), they could still have
different ASN.1 encodings, i.e. one includes the optional value vs one
does not. I think the reasonable expectation would be that if all
non-optional fields match, then the 2 keys are equal. Otherwise, it's
hard to predict the behavior since DH keys can be constructed w/ or w/o
the optional field and there are mixed usages in our JDK code base.
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.
I will also apply the same change to P11DHPrivateKey/P11DHPublicKey
then. Equality check using ASN.1 encoding is fine for non-DH algorithms
but not for DH.
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.
Ok, I am fine with just using Objects.hash(Object... values) 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.
The change is for conforming to the description under section 7.1
"Private-value generation" of PKCS#3 DH Key Agreement Standard
ftp://ftp.rsasecurity.com/pub/pkcs/ascii/pkcs-3.asc , i.e.
An integer x, the private value, shall be generated
privately and randomly. This integer shall satisfy 0< x<
p-1, unless the central authority specifies a private-value
length l, in which case the integer shall satisfy 2^(l-1)<=
x< 2^l.
I will re-test everything and let you know once I have the webrev updated.
Thanks,
Valerie
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
