OpenJDK / jdk / jdk
changeset 59804:80cbb6242147
8246077: Cloneable test in HmacCore seems questionable
Summary: Updated java.security.MessageDigest and Signature classes to return Cloneable obj for Cloneable spi obj
Reviewed-by: weijun, xuelei, coffeys
| author | valeriep |
|---|---|
| date | Mon, 15 Jun 2020 23:30:49 +0000 |
| parents | 7ee4492cb21b |
| children | b9ec913f0afd |
| files | src/java.base/share/classes/com/sun/crypto/provider/HmacCore.java src/java.base/share/classes/java/security/MessageDigest.java src/java.base/share/classes/java/security/Signature.java src/java.base/share/classes/sun/security/provider/DigestBase.java test/jdk/com/sun/crypto/provider/Mac/DigestCloneabilityTest.java test/jdk/java/security/MessageDigest/TestCloneable.java test/jdk/java/security/Signature/TestCloneable.java |
| diffstat | 7 files changed, 377 insertions(+), 32 deletions(-) [+] |
line wrap: on
line diff
--- a/src/java.base/share/classes/com/sun/crypto/provider/HmacCore.java Mon Jun 15 16:05:47 2020 -0700 +++ b/src/java.base/share/classes/com/sun/crypto/provider/HmacCore.java Mon Jun 15 23:30:49 2020 +0000 @@ -75,6 +75,7 @@ } else { String noCloneProv = md.getProvider().getName(); // if no Sun provider, use provider list + md = null; Provider[] provs = Security.getProviders(); for (Provider p : provs) { try { @@ -90,6 +91,10 @@ continue; } } + if (md == null) { + throw new NoSuchAlgorithmException + ("No Cloneable digest found for " + digestAlgo); + } } } this.md = md;
--- a/src/java.base/share/classes/java/security/MessageDigest.java Mon Jun 15 16:05:47 2020 -0700 +++ b/src/java.base/share/classes/java/security/MessageDigest.java Mon Jun 15 23:30:49 2020 +0000 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -131,6 +131,12 @@ this.algorithm = algorithm; } + // private constructor used only by Delegate class + private MessageDigest(String algorithm, Provider p) { + this.algorithm = algorithm; + this.provider = p; + } + /** * Returns a MessageDigest object that implements the specified digest * algorithm. @@ -178,10 +184,11 @@ (String)null); if (objs[0] instanceof MessageDigest) { md = (MessageDigest)objs[0]; + md.provider = (Provider)objs[1]; } else { - md = new Delegate((MessageDigestSpi)objs[0], algorithm); + md = Delegate.of((MessageDigestSpi)objs[0], algorithm, + (Provider) objs[1]); } - md.provider = (Provider)objs[1]; if (!skipDebug && pdebug != null) { pdebug.println("MessageDigest." + algorithm + @@ -245,8 +252,8 @@ return md; } else { MessageDigest delegate = - new Delegate((MessageDigestSpi)objs[0], algorithm); - delegate.provider = (Provider)objs[1]; + Delegate.of((MessageDigestSpi)objs[0], algorithm, + (Provider)objs[1]); return delegate; } } @@ -298,8 +305,8 @@ return md; } else { MessageDigest delegate = - new Delegate((MessageDigestSpi)objs[0], algorithm); - delegate.provider = (Provider)objs[1]; + Delegate.of((MessageDigestSpi)objs[0], algorithm, + (Provider)objs[1]); return delegate; } } @@ -547,8 +554,6 @@ } - - /* * The following class allows providers to extend from MessageDigestSpi * rather than from MessageDigest. It represents a MessageDigest with an @@ -563,14 +568,45 @@ * and its original parent (Object). */ - static class Delegate extends MessageDigest implements MessageDigestSpi2 { + private static class Delegate extends MessageDigest + implements MessageDigestSpi2 { + // use this class for spi objects which implements Cloneable + private static final class CloneableDelegate extends Delegate + implements Cloneable { + + private CloneableDelegate(MessageDigestSpi digestSpi, + String algorithm, Provider p) { + super(digestSpi, algorithm, p); + } + } // The provider implementation (delegate) - private MessageDigestSpi digestSpi; + private final MessageDigestSpi digestSpi; - // constructor - public Delegate(MessageDigestSpi digestSpi, String algorithm) { - super(algorithm); + // factory method used by MessageDigest class to create Delegate objs + static Delegate of(MessageDigestSpi digestSpi, String algo, + Provider p) { + Objects.requireNonNull(digestSpi); + boolean isCloneable = (digestSpi instanceof Cloneable); + // Spi impls from SunPKCS11 provider implement Cloneable but their + // clone() may throw CloneNotSupportException + if (isCloneable && p.getName().startsWith("SunPKCS11") && + p.getClass().getModule().getName().equals + ("jdk.crypto.cryptoki")) { + try { + digestSpi.clone(); + } catch (CloneNotSupportedException cnse) { + isCloneable = false; + } + } + return (isCloneable? new CloneableDelegate(digestSpi, algo, p) : + new Delegate(digestSpi, algo, p)); + } + + // private constructor + private Delegate(MessageDigestSpi digestSpi, String algorithm, + Provider p) { + super(algorithm, p); this.digestSpi = digestSpi; } @@ -582,17 +618,16 @@ * @throws CloneNotSupportedException if this is called on a * delegate that does not support {@code Cloneable}. */ + @Override public Object clone() throws CloneNotSupportedException { - if (digestSpi instanceof Cloneable) { - MessageDigestSpi digestSpiClone = - (MessageDigestSpi)digestSpi.clone(); + if (this instanceof Cloneable) { // Because 'algorithm', 'provider', and 'state' are private // members of our supertype, we must perform a cast to // access them. - MessageDigest that = - new Delegate(digestSpiClone, - ((MessageDigest)this).algorithm); - that.provider = ((MessageDigest)this).provider; + MessageDigest that = new CloneableDelegate( + (MessageDigestSpi)digestSpi.clone(), + ((MessageDigest)this).algorithm, + ((MessageDigest)this).provider); that.state = ((MessageDigest)this).state; return that; } else { @@ -600,22 +635,27 @@ } } + @Override protected int engineGetDigestLength() { return digestSpi.engineGetDigestLength(); } + @Override protected void engineUpdate(byte input) { digestSpi.engineUpdate(input); } + @Override protected void engineUpdate(byte[] input, int offset, int len) { digestSpi.engineUpdate(input, offset, len); } + @Override protected void engineUpdate(ByteBuffer input) { digestSpi.engineUpdate(input); } + @Override public void engineUpdate(SecretKey key) throws InvalidKeyException { if (digestSpi instanceof MessageDigestSpi2) { ((MessageDigestSpi2)digestSpi).engineUpdate(key); @@ -624,15 +664,19 @@ ("Digest does not support update of SecretKey object"); } } + + @Override protected byte[] engineDigest() { return digestSpi.engineDigest(); } + @Override protected int engineDigest(byte[] buf, int offset, int len) throws DigestException { return digestSpi.engineDigest(buf, offset, len); } + @Override protected void engineReset() { digestSpi.engineReset(); }
--- a/src/java.base/share/classes/java/security/Signature.java Mon Jun 15 16:05:47 2020 -0700 +++ b/src/java.base/share/classes/java/security/Signature.java Mon Jun 15 23:30:49 2020 +0000 @@ -272,7 +272,7 @@ NoSuchAlgorithmException failure; do { Service s = t.next(); - if (isSpi(s)) { + if (isSpi(s)) { // delayed provider selection return new Delegate(s, t, algorithm); } else { // must be a subclass of Signature, disable dynamic selection @@ -295,7 +295,7 @@ sig.algorithm = algorithm; } else { SignatureSpi spi = (SignatureSpi)instance.impl; - sig = new Delegate(spi, algorithm); + sig = Delegate.of(spi, algorithm); } sig.provider = instance.provider; return sig; @@ -464,7 +464,7 @@ // check Cipher try { Cipher c = Cipher.getInstance(RSA_CIPHER, p); - return new Delegate(new CipherAdapter(c), RSA_SIGNATURE); + return Delegate.of(new CipherAdapter(c), RSA_SIGNATURE); } catch (GeneralSecurityException e) { // throw Signature style exception message to avoid confusion, // but append Cipher exception as cause @@ -1092,6 +1092,14 @@ @SuppressWarnings("deprecation") private static class Delegate extends Signature { + // use this class for spi objects which implements Cloneable + private static final class CloneableDelegate extends Delegate + implements Cloneable { + private CloneableDelegate(SignatureSpi digestSpi, + String algorithm) { + super(digestSpi, algorithm); + } + } // The provider implementation (delegate) // filled in once the provider is selected @@ -1108,15 +1116,24 @@ // null once provider is selected private Iterator<Service> serviceIterator; - // constructor - Delegate(SignatureSpi sigSpi, String algorithm) { + // factory method used by Signature class to create Delegate objs + static Delegate of(SignatureSpi sigSpi, String algorithm) { + if (sigSpi instanceof Cloneable) { + return new CloneableDelegate(sigSpi, algorithm); + } else { + return new Delegate(sigSpi, algorithm); + } + } + + // private constructor + private Delegate(SignatureSpi sigSpi, String algorithm) { super(algorithm); this.sigSpi = sigSpi; this.lock = null; // no lock needed } - // used with delayed provider selection - Delegate(Service service, + // constructor used with delayed provider selection + private Delegate(Service service, Iterator<Service> iterator, String algorithm) { super(algorithm); this.firstService = service; @@ -1132,15 +1149,16 @@ * @throws CloneNotSupportedException if this is called on a * delegate that does not support {@code Cloneable}. */ + @Override public Object clone() throws CloneNotSupportedException { chooseFirstProvider(); if (sigSpi instanceof Cloneable) { - SignatureSpi sigSpiClone = (SignatureSpi)sigSpi.clone(); // Because 'algorithm' and 'provider' are private // members of our supertype, we must perform a cast to // access them. - Signature that = - new Delegate(sigSpiClone, ((Signature)this).algorithm); + Signature that = new CloneableDelegate( + (SignatureSpi)sigSpi.clone(), + ((Signature)this).algorithm); that.provider = ((Signature)this).provider; return that; } else {
--- a/src/java.base/share/classes/sun/security/provider/DigestBase.java Mon Jun 15 16:05:47 2020 -0700 +++ b/src/java.base/share/classes/sun/security/provider/DigestBase.java Mon Jun 15 23:30:49 2020 +0000 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -233,6 +233,7 @@ public Object clone() throws CloneNotSupportedException { DigestBase copy = (DigestBase) super.clone(); copy.buffer = copy.buffer.clone(); + copy.oneByte = null; return copy; }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/com/sun/crypto/provider/Mac/DigestCloneabilityTest.java Mon Jun 15 23:30:49 2020 +0000 @@ -0,0 +1,106 @@ +/* + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8246077 + * @summary Ensure a cloneable digest can be accepted/used by HmacCore + */ +import java.security.*; +import java.security.spec.AlgorithmParameterSpec; +import javax.crypto.*; +import javax.crypto.spec.*; + +public class DigestCloneabilityTest { + + private static String ALGO = "HmacSHA512"; + + public static void main(String[] args) throws Exception { + Provider p = new SampleProvider(); + // make SampleProvider the most preferred, so its digest impl is picked + int status = Security.insertProviderAt(p, 1); + try { + Mac mac = Mac.getInstance(ALGO, "SunJCE"); + // do a complete mac generation and check if the supplied + // digest is used + mac.init(new SecretKeySpec(new byte[512>>3], ALGO)); + mac.update((byte)0x12); + byte[] macBytes = mac.doFinal(); + if (!SampleProvider.CloneableDigest.isUsed) { + throw new RuntimeException("Expected Digest impl not used"); + } + } finally { + if (status != -1) { + Security.removeProvider(p.getName()); + } + } + System.out.println("Test Passed"); + } + + public static class SampleProvider extends Provider { + + public SampleProvider() { + super("Sample", "1.0", "test provider"); + putService(new Provider.Service(this, "MessageDigest", "SHA-512", + "DigestCloneabilityTest$SampleProvider$CloneableDigest", + null, null)); + } + public static class CloneableDigest extends MessageDigestSpi + implements Cloneable { + private MessageDigest md; + static boolean isUsed = false; + + public CloneableDigest() throws NoSuchAlgorithmException { + try { + md = MessageDigest.getInstance("SHA-512", "SUN"); + } catch (NoSuchProviderException nspe) { + // should never happen + } + } + + public byte[] engineDigest() { + isUsed = true; + return md.digest(); + } + + public void engineReset() { + isUsed = true; + md.reset(); + } + + public void engineUpdate(byte input) { + isUsed = true; + md.update(input); + } + + public void engineUpdate(byte[] b, int ofs, int len) { + isUsed = true; + md.update(b, ofs, len); + } + + public Object clone() throws CloneNotSupportedException { + return this; + } + } + } +}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/java/security/MessageDigest/TestCloneable.java Mon Jun 15 23:30:49 2020 +0000 @@ -0,0 +1,76 @@ +/* + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8246077 + * @summary Make sure that digest spi and the resulting digest impl are + * consistent in the impl of Cloneable interface + * @run testng TestCloneable + */ +import java.security.*; +import java.util.Objects; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import org.testng.Assert; + +public class TestCloneable { + + private static final Class<CloneNotSupportedException> CNSE = + CloneNotSupportedException.class; + + @DataProvider + public Object[][] testData() { + return new Object[][] { + { "MD2", "SUN" }, { "MD5", "SUN" }, { "SHA-1", "SUN" }, + { "SHA-224", "SUN" }, { "SHA-256", "SUN" }, + { "SHA-384", "SUN" }, { "SHA-512", "SUN" }, + { "SHA3-224", "SUN" }, { "SHA3-256", "SUN" }, + { "SHA3-384", "SUN" }, { "SHA3-512", "SUN" } + }; + } + + @Test(dataProvider = "testData") + public void test(String algo, String provName) + throws NoSuchProviderException, NoSuchAlgorithmException, + CloneNotSupportedException { + System.out.print("Testing " + algo + " impl from " + provName); + Provider p = Security.getProvider(provName); + Provider.Service s = p.getService("MessageDigest", algo); + Objects.requireNonNull(s); + MessageDigestSpi spi = (MessageDigestSpi) s.newInstance(null); + MessageDigest md = MessageDigest.getInstance(algo, provName); + if (spi instanceof Cloneable) { + System.out.println(": Cloneable"); + Assert.assertTrue(md instanceof Cloneable); + MessageDigest md2 = (MessageDigest) md.clone(); + Assert.assertEquals(md2.getAlgorithm(), algo); + Assert.assertEquals(md2.getProvider().getName(), provName); + Assert.assertTrue(md2 instanceof Cloneable); + } else { + System.out.println(": NOT Cloneable"); + Assert.assertThrows(CNSE, ()->md.clone()); + } + System.out.println("Test Passed"); + } +}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/java/security/Signature/TestCloneable.java Mon Jun 15 23:30:49 2020 +0000 @@ -0,0 +1,95 @@ +/* + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8246077 + * @summary Make sure that signature objects which are cloneable + * implement the Cloneable interface + * @run testng TestCloneable + */ +import java.security.NoSuchProviderException; +import java.security.NoSuchAlgorithmException; +import java.security.Signature; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import org.testng.Assert; + +public class TestCloneable { + + private static final Class<CloneNotSupportedException> CNSE = + CloneNotSupportedException.class; + + @DataProvider + public Object[][] testData() { + return new Object[][] { + { "SHA1withDSA", "SUN" }, { "NONEwithDSA", "SUN" }, + { "SHA224withDSA", "SUN" }, { "SHA256withDSA", "SUN" }, + { "EdDSA", "SunEC" }, { "Ed25519", "SunEC" }, { "Ed448", "SunEC" }, + { "SHA1withECDSA", "SunEC" }, { "SHA224withECDSA", "SunEC" }, + { "SHA256withECDSA", "SunEC" }, { "SHA384withECDSA", "SunEC" }, + { "SHA512withECDSA", "SunEC" }, { "NONEwithECDSA", "SunEC" }, + { "MD2withRSA", "SunRsaSign" }, { "MD5withRSA", "SunRsaSign" }, + { "SHA1withRSA", "SunRsaSign" }, { "SHA224withRSA", "SunRsaSign" }, + { "SHA256withRSA", "SunRsaSign" }, + { "SHA384withRSA", "SunRsaSign" }, + { "SHA512withRSA", "SunRsaSign" }, + { "SHA512/224withRSA", "SunRsaSign" }, + { "SHA512/256withRSA", "SunRsaSign" }, + { "RSASSA-PSS", "SunRsaSign" }, + { "NONEwithRSA", "SunMSCAPI" }, + { "SHA1withRSA", "SunMSCAPI" }, { "SHA256withRSA", "SunMSCAPI" }, + { "SHA384withRSA", "SunMSCAPI" }, { "SHA512withRSA", "SunMSCAPI" }, + { "RSASSA-PSS", "SunMSCAPI" }, + { "MD5withRSA", "SunMSCAPI" }, { "MD2withRSA", "SunMSCAPI" }, + { "SHA1withECDSA", "SunMSCAPI" }, + { "SHA224withECDSA", "SunMSCAPI" }, + { "SHA256withECDSA", "SunMSCAPI" }, + { "SHA384withECDSA", "SunMSCAPI" }, + { "SHA512withECDSA", "SunMSCAPI" } + }; + } + + @Test(dataProvider = "testData") + public void test(String algo, String provName) + throws NoSuchAlgorithmException, CloneNotSupportedException { + System.out.print("Testing " + algo + " impl from " + provName); + try { + Signature sig = Signature.getInstance(algo, provName); + if (sig instanceof Cloneable) { + System.out.println(": Cloneable"); + Signature sig2 = (Signature) sig.clone(); + Assert.assertEquals(sig2.getAlgorithm(), algo); + Assert.assertEquals(sig2.getProvider().getName(), provName); + Assert.assertTrue(sig2 instanceof Cloneable); + } else { + System.out.println(": NOT Cloneable"); + Assert.assertThrows(CNSE, ()->sig.clone()); + } + System.out.println("Test Passed"); + } catch (NoSuchProviderException nspe) { + // skip testing + System.out.println("Skip " + provName + " - not available"); + } + } +}