[15] RFR JDK-8246077: Cloneable test in HmacCore seems questionable

[Weijun Wang]

HmacCore.java:

  78                 md = null;
  79                 String noCloneProv = md.getProvider().getName();

NPE on line 79. Should reverse.


> On Jun 4, 2020, at 8:09 AM, Valerie Peng <valerie.peng at 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?

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.

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
>