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.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?

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
> 

Reply via email to