diff --git a/src/java.base/share/classes/com/sun/crypto/provider/DESKey.java b/src/java.base/share/classes/com/sun/crypto/provider/DESKey.java index 2550556bbad..87aa1753eb5 100644 --- a/src/java.base/share/classes/com/sun/crypto/provider/DESKey.java +++ b/src/java.base/share/classes/com/sun/crypto/provider/DESKey.java @@ -89,12 +89,12 @@ final class DESKey implements SecretKey { public byte[] getEncoded() { // Return a copy of the key, rather than a reference, // so that the key data cannot be modified from outside - - // The key is zeroized by finalize() - // The reachability fence ensures finalize() isn't called early - byte[] result = key.clone(); - Reference.reachabilityFence(this); - return result; + try { + return key.clone(); + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } public String getAlgorithm() { @@ -111,25 +111,35 @@ final class DESKey implements SecretKey { */ @Override public int hashCode() { - return Arrays.hashCode(this.key) ^ "des".hashCode(); + try { + return Arrays.hashCode(this.key) ^ "des".hashCode(); + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } @Override public boolean equals(Object obj) { - if (this == obj) - return true; + try { + if (this == obj) + return true; - if (!(obj instanceof SecretKey that)) - return false; + if (!(obj instanceof SecretKey that)) + return false; - String thatAlg = that.getAlgorithm(); - if (!(thatAlg.equalsIgnoreCase("DES"))) - return false; + String thatAlg = that.getAlgorithm(); + if (!(thatAlg.equalsIgnoreCase("DES"))) + return false; - byte[] thatKey = that.getEncoded(); - boolean ret = MessageDigest.isEqual(this.key, thatKey); - java.util.Arrays.fill(thatKey, (byte)0x00); - return ret; + byte[] thatKey = that.getEncoded(); + boolean ret = MessageDigest.isEqual(this.key, thatKey); + java.util.Arrays.fill(thatKey, (byte)0x00); + return ret; + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } /** @@ -141,7 +151,13 @@ final class DESKey implements SecretKey { throws java.io.IOException, ClassNotFoundException { s.defaultReadObject(); - key = key.clone(); + byte[] temp = key; + key = temp.clone(); + Arrays.fill(temp, (byte)0x00); + // Use the cleaner to zero the key when no longer referenced + final byte[] k = this.key; + CleanerFactory.cleaner().register(this, + () -> java.util.Arrays.fill(k, (byte)0x00)); } /** @@ -154,9 +170,14 @@ final class DESKey implements SecretKey { */ @java.io.Serial private Object writeReplace() throws java.io.ObjectStreamException { - return new KeyRep(KeyRep.Type.SECRET, + try { + return new KeyRep(KeyRep.Type.SECRET, getAlgorithm(), getFormat(), key); + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } } diff --git a/src/java.base/share/classes/com/sun/crypto/provider/DESedeKey.java b/src/java.base/share/classes/com/sun/crypto/provider/DESedeKey.java index 7e075343abd..938fd217eee 100644 --- a/src/java.base/share/classes/com/sun/crypto/provider/DESedeKey.java +++ b/src/java.base/share/classes/com/sun/crypto/provider/DESedeKey.java @@ -89,11 +89,12 @@ final class DESedeKey implements SecretKey { } public byte[] getEncoded() { - // The key is zeroized by finalize() - // The reachability fence ensures finalize() isn't called early - byte[] result = key.clone(); - Reference.reachabilityFence(this); - return result; + try { + return key.clone(); + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } public String getAlgorithm() { @@ -110,26 +111,36 @@ final class DESedeKey implements SecretKey { */ @Override public int hashCode() { - return Arrays.hashCode(this.key) ^ "desede".hashCode(); + try { + return Arrays.hashCode(this.key) ^ "desede".hashCode(); + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } @Override public boolean equals(Object obj) { - if (this == obj) - return true; + try { + if (this == obj) + return true; - if (!(obj instanceof SecretKey that)) - return false; + if (!(obj instanceof SecretKey that)) + return false; - String thatAlg = that.getAlgorithm(); - if (!(thatAlg.equalsIgnoreCase("DESede")) - && !(thatAlg.equalsIgnoreCase("TripleDES"))) - return false; + String thatAlg = that.getAlgorithm(); + if (!(thatAlg.equalsIgnoreCase("DESede")) + && !(thatAlg.equalsIgnoreCase("TripleDES"))) + return false; - byte[] thatKey = that.getEncoded(); - boolean ret = MessageDigest.isEqual(this.key, thatKey); - java.util.Arrays.fill(thatKey, (byte)0x00); - return ret; + byte[] thatKey = that.getEncoded(); + boolean ret = MessageDigest.isEqual(this.key, thatKey); + java.util.Arrays.fill(thatKey, (byte)0x00); + return ret; + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } /** @@ -141,7 +152,13 @@ final class DESedeKey implements SecretKey { throws java.io.IOException, ClassNotFoundException { s.defaultReadObject(); - key = key.clone(); + byte[] temp = key; + this.key = temp.clone(); + java.util.Arrays.fill(temp, (byte)0x00); + // Use the cleaner to zero the key when no longer referenced + final byte[] k = this.key; + CleanerFactory.cleaner().register(this, + () -> java.util.Arrays.fill(k, (byte)0x00)); } /** @@ -154,9 +171,14 @@ final class DESedeKey implements SecretKey { */ @java.io.Serial private Object writeReplace() throws java.io.ObjectStreamException { - return new KeyRep(KeyRep.Type.SECRET, - getAlgorithm(), - getFormat(), - key); + try { + return new KeyRep(KeyRep.Type.SECRET, + getAlgorithm(), + getFormat(), + key); + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } } diff --git a/src/java.base/share/classes/com/sun/crypto/provider/KeyProtector.java b/src/java.base/share/classes/com/sun/crypto/provider/KeyProtector.java index e396f937a13..44c7a1f6432 100644 --- a/src/java.base/share/classes/com/sun/crypto/provider/KeyProtector.java +++ b/src/java.base/share/classes/com/sun/crypto/provider/KeyProtector.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2023, 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 @@ -129,7 +129,7 @@ final class KeyProtector { SecretKey sKey = null; PBEWithMD5AndTripleDESCipher cipher; try { - sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES", false); + sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES"); // encrypt private key cipher = new PBEWithMD5AndTripleDESCipher(); cipher.engineInit(Cipher.ENCRYPT_MODE, sKey, pbeSpec, null); @@ -193,7 +193,7 @@ final class KeyProtector { // create PBE key from password PBEKeySpec pbeKeySpec = new PBEKeySpec(this.password); - sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES", false); + sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES"); pbeKeySpec.clearPassword(); // decrypt private key @@ -339,7 +339,7 @@ final class KeyProtector { SecretKey sKey = null; Cipher cipher; try { - sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES", false); + sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES"); pbeKeySpec.clearPassword(); // seal key @@ -366,8 +366,7 @@ final class KeyProtector { try { // create PBE key from password PBEKeySpec pbeKeySpec = new PBEKeySpec(this.password); - sKey = new PBEKey(pbeKeySpec, - "PBEWithMD5AndTripleDES", false); + sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES"); pbeKeySpec.clearPassword(); SealedObjectForKeyProtector soForKeyProtector = null; diff --git a/src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java b/src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java index dba3cccf193..71efeb535aa 100644 --- a/src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java +++ b/src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java @@ -26,6 +26,7 @@ package com.sun.crypto.provider; import java.lang.ref.Reference; +import java.lang.ref.Cleaner.Cleanable; import java.security.MessageDigest; import java.security.KeyRep; import java.security.spec.InvalidKeySpecException; @@ -51,13 +52,14 @@ final class PBEKey implements SecretKey { private String type; + private transient Cleanable cleanable; + /** * Creates a PBE key from a given PBE key specification. * * @param keytype the given PBE key specification */ - PBEKey(PBEKeySpec keySpec, String keytype, boolean useCleaner) - throws InvalidKeySpecException { + PBEKey(PBEKeySpec keySpec, String keytype) throws InvalidKeySpecException { char[] passwd = keySpec.getPassword(); if (passwd == null) { // Should allow an empty password. @@ -78,19 +80,18 @@ final class PBEKey implements SecretKey { type = keytype; // Use the cleaner to zero the key when no longer referenced - if (useCleaner) { - final byte[] k = this.key; - CleanerFactory.cleaner().register(this, - () -> Arrays.fill(k, (byte) 0x00)); - } + final byte[] k = this.key; + cleanable = CleanerFactory.cleaner().register(this, + () -> java.util.Arrays.fill(k, (byte)0x00)); } public byte[] getEncoded() { - // The key is zeroized by finalize() - // The reachability fence ensures finalize() isn't called early - byte[] result = key.clone(); - Reference.reachabilityFence(this); - return result; + try { + return key.clone(); + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } public String getAlgorithm() { @@ -107,25 +108,40 @@ final class PBEKey implements SecretKey { */ @Override public int hashCode() { - return Arrays.hashCode(this.key) - ^ getAlgorithm().toLowerCase(Locale.ENGLISH).hashCode(); + try { + return Arrays.hashCode(this.key) + ^ getAlgorithm().toLowerCase(Locale.ENGLISH).hashCode(); + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } @Override public boolean equals(Object obj) { - if (obj == this) - return true; + try { + if (obj == this) + return true; - if (!(obj instanceof SecretKey that)) - return false; + if (!(obj instanceof SecretKey that)) + return false; - if (!(that.getAlgorithm().equalsIgnoreCase(type))) - return false; + // destroyed keys are considered different + if (isDestroyed() || that.isDestroyed()) { + return false; + } - byte[] thatEncoded = that.getEncoded(); - boolean ret = MessageDigest.isEqual(this.key, thatEncoded); - Arrays.fill(thatEncoded, (byte)0x00); - return ret; + if (!(that.getAlgorithm().equalsIgnoreCase(type))) + return false; + + byte[] thatEncoded = that.getEncoded(); + boolean ret = MessageDigest.isEqual(this.key, thatEncoded); + Arrays.fill(thatEncoded, (byte)0x00); + return ret; + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } /** @@ -134,12 +150,17 @@ final class PBEKey implements SecretKey { */ @Override public void destroy() { - if (key != null) { - Arrays.fill(key, (byte) 0x00); - key = null; + if (cleanable != null) { + cleanable.clean(); + cleanable = null; } } + @Override + public boolean isDestroyed() { + return (cleanable == null); + } + /** * readObject is called to restore the state of this key from * a stream. @@ -149,7 +170,13 @@ final class PBEKey implements SecretKey { throws java.io.IOException, ClassNotFoundException { s.defaultReadObject(); - key = key.clone(); + byte[] temp = key; + key = temp.clone(); + Arrays.fill(temp, (byte)0x00); + // Use cleaner to zero the key when no longer referenced + final byte[] k = this.key; + cleanable = CleanerFactory.cleaner().register(this, + () -> java.util.Arrays.fill(k, (byte)0x00)); } @@ -163,9 +190,14 @@ final class PBEKey implements SecretKey { */ @java.io.Serial private Object writeReplace() throws java.io.ObjectStreamException { - return new KeyRep(KeyRep.Type.SECRET, - getAlgorithm(), - getFormat(), - key); + try { + return new KeyRep(KeyRep.Type.SECRET, + getAlgorithm(), + getFormat(), + key); + } finally { + // prevent this from being cleaned for the above block + Reference.reachabilityFence(this); + } } } diff --git a/src/java.base/share/classes/com/sun/crypto/provider/PBEKeyFactory.java b/src/java.base/share/classes/com/sun/crypto/provider/PBEKeyFactory.java index 98f4e3923a8..66c0e6904c3 100644 --- a/src/java.base/share/classes/com/sun/crypto/provider/PBEKeyFactory.java +++ b/src/java.base/share/classes/com/sun/crypto/provider/PBEKeyFactory.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2023, 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 @@ -239,7 +239,7 @@ abstract class PBEKeyFactory extends SecretKeyFactorySpi { if (!(keySpec instanceof PBEKeySpec)) { throw new InvalidKeySpecException("Invalid key spec"); } - return new PBEKey((PBEKeySpec)keySpec, type, true); + return new PBEKey((PBEKeySpec)keySpec, type); } /** diff --git a/test/jdk/com/sun/crypto/provider/KeyFactory/PBEKeyDestroyTest.java b/test/jdk/com/sun/crypto/provider/KeyFactory/PBEKeyDestroyTest.java new file mode 100644 index 00000000000..da266c147fb --- /dev/null +++ b/test/jdk/com/sun/crypto/provider/KeyFactory/PBEKeyDestroyTest.java @@ -0,0 +1,71 @@ +/* + * Copyright (c) 2023, 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 8312306 + * @summary Check the destroy()/isDestroyed() of the PBEKey impl from SunJCE + * @library /test/lib + * @run testng/othervm PBEKeyDestroyTest + */ +import javax.crypto.*; +import javax.crypto.spec.*; +import java.nio.charset.StandardCharsets; +import org.testng.Assert; +import org.testng.annotations.Test; + +public class PBEKeyDestroyTest { + + @Test + public void test() throws Exception { + PBEKeySpec keySpec = new PBEKeySpec("12345678".toCharArray(), + "abcdefgh".getBytes(StandardCharsets.UTF_8), 100000, 128 >> 3); + + SecretKeyFactory skf = SecretKeyFactory.getInstance + ("PBEWithHmacSHA1AndAES_128", "SunJCE"); + + SecretKey key1 = skf.generateSecret(keySpec); + SecretKey key2 = skf.generateSecret(keySpec); + + // should be equal + Assert.assertFalse(key1.isDestroyed()); + Assert.assertFalse(key2.isDestroyed()); + Assert.assertTrue(key1.equals(key2)); + Assert.assertTrue(key2.equals(key1)); + + // destroy key1 + key1.destroy(); + Assert.assertTrue(key1.isDestroyed()); + Assert.assertFalse(key1.equals(key2)); + Assert.assertFalse(key2.equals(key1)); + + // also destroy key2 + key2.destroy(); + Assert.assertTrue(key2.isDestroyed()); + Assert.assertFalse(key1.equals(key2)); + Assert.assertFalse(key2.equals(key1)); + + // call destroy again to make sure no unexpected exceptions + key2.destroy(); + } +}