Hi Max,
Please find replies in line.
On 6/4/2020 3:54 AM, Weijun Wang wrote:
HmacCore.java:
78 md = null;
79 String noCloneProv = md.getProvider().getName();
NPE on line 79. Should reverse.
Good catch, fixed.
On Jun 4, 2020, at 8:09 AM, Valerie Peng <valerie.p...@oracle.com> wrote:
Hi,
Anyone can help review this fix? I changed com.sun.crypto.provider.HmacCore
class and sun.security.ssl.HandshakeHash class to check for cloneability based
on the clone() call instead of the Cloneable interface.
Can you give an example when these 2 rules have different results? Is this only
true for those implementation that directly subclass MessageDigest?
Before this fix, even the Spi impl implements Cloneable the instanceof
check will always fail because the wrapper class, i.e.
MessageDigest.Delegate does not. However, if you call the clone() (made
public by the MessageDigest class), it will succeed because
Delegate.clone() checks to see if the spi object implements the
Cloneable interface, if yes, it will proceed to call the spi clone().
So, for this scenario, the results are different, e.g. instanceof
returns false, but clone() succeeds. This is just one example. Is this
what you are asking?
The test is not complete. There should be non-cloneable hash. Or even better, a
hash which is not cloneable from the preferred provider but cloneable from
another. Hopefully you can simply reuse an existing implementation with a
different algorithm name and override its clone() to throw CNSE.
Right, I will expand the tests. Just want to gauge on people's
preference of matching the Cloneable interface (the last part of changes
in my earlier email) first, while I explore the existing tests.
Thanks,
Valerie
Thanks,
Max
Noticed a bug in sun.security.provider.DigestBase class which misses to reset
the temporary buffer when cloning and fixed it here as well.
Lastly, I also made changes to java.security.MessageDigest and
java.security.Signature classes and attempt to match the Delegate wrapper with
the underlying spi object, i.e. if the spi implements Cloneable and then
Delegate wrapper also implements Cloneable. This part is mostly for non-JDK
callers which rely on the instanceof Cloneable check for cloneability. However,
for Signature class, if the object is requested using
Signature.getInstance(String), then we can't do matching here since the
underlying spi is not yet determined at this time. The 3 TestCloneable.java
tests are for testing this last part and is NOT for the HmacCore and
HandshakeHash changes. I am on the fence about this part and am open to leave
this out if minimum fix is preferred.
Bug: https://bugs.openjdk.java.net/browse/JDK-8246077
Webrev: http://cr.openjdk.java.net/~valeriep/8246077/webrev.00/
Thanks,
Valerie
