8303613: EdECPrivateKey should support Destroyable.destroy()

src/java.base/share/classes/sun/security/ec/ed/EdDSAPrivateKeyImpl.java

@@ -25,17 +25,22 @@
 
 package sun.security.ec.ed;
 
+import jdk.internal.ref.CleanerFactory;
+import sun.security.pkcs.PKCS8Key;
+import sun.security.util.DerInputStream;
+import sun.security.util.DerValue;
+import sun.security.x509.AlgorithmId;
+
 import java.io.IOException;
 import java.io.InvalidObjectException;
 import java.io.ObjectInputStream;
+import java.lang.ref.Cleaner.Cleanable;
+import java.lang.ref.Reference;
 import java.security.InvalidKeyException;
 import java.security.interfaces.EdECPrivateKey;
-import java.util.Optional;
 import java.security.spec.NamedParameterSpec;
-
-import sun.security.pkcs.PKCS8Key;
-import sun.security.x509.AlgorithmId;
-import sun.security.util.*;
+import java.util.Arrays;
+import java.util.Optional;
 
 public final class EdDSAPrivateKeyImpl
         extends PKCS8Key implements EdECPrivateKey {
@@ -47,6 +52,8 @@ public final class EdDSAPrivateKeyImpl
     private final NamedParameterSpec paramSpec;
     private byte[] h;
 
+    private transient Cleanable cleanable;
+
     EdDSAPrivateKeyImpl(EdDSAParameters params, byte[] h)
             throws InvalidKeyException {
 
@@ -61,6 +68,11 @@ public final class EdDSAPrivateKeyImpl
             val.clear();
         }
         checkLength(params);
+
+        final byte[] lh = this.h;
+        cleanable = CleanerFactory.cleaner().register(this,
+                                                      () -> Arrays.fill(lh,
+                                                                        (byte) 0x00));
     }
 
     EdDSAPrivateKeyImpl(byte[] encoded) throws InvalidKeyException {
@@ -77,6 +89,29 @@ public final class EdDSAPrivateKeyImpl
             throw new InvalidKeyException(ex);
         }
         checkLength(params);
+
+        final byte[] lh = this.h;
+        cleanable = CleanerFactory.cleaner().register(this,
+                                                      () -> Arrays.fill(lh,
+                                                                        (byte) 0x00));
+    }
+
+    /**
+     * Clears the internal copy of the key.
+     *
+     */
+    @Override
+    public void destroy() {
+        super.destroy();
+        if (cleanable != null) {
+            cleanable.clean();
+            cleanable = null;
+        }
+    }
+
+    @Override
+    public boolean isDestroyed() {
+        return (cleanable == null);
     }
 
     void checkLength(EdDSAParameters params) throws InvalidKeyException {
@@ -88,7 +123,15 @@ void checkLength(EdDSAParameters params) throws InvalidKeyException {
     }
 
     public byte[] getKey() {
-        return h.clone();
+        try {
+            if (isDestroyed()) {
+                throw new IllegalStateException("Key is destroyed");
+            }
+            return h.clone();
+        } finally {
+            // prevent this from being cleaned for the above block
+            Reference.reachabilityFence(this);
+        }
     }
 
     @Override

src/java.base/share/classes/sun/security/pkcs/NamedPKCS8Key.java

@@ -127,7 +127,7 @@ private void readObject(ObjectInputStream stream)
     }
 
     @Override
-    public void destroy() throws DestroyFailedException {
+    public void destroy() {
         Arrays.fill(rawBytes, (byte)0);
         Arrays.fill(privKeyMaterial, (byte)0);
         if (encodedKey != null) {

src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java

@@ -26,13 +26,26 @@
 package sun.security.pkcs;
 
 import jdk.internal.access.SharedSecrets;
-import sun.security.util.*;
+import jdk.internal.ref.CleanerFactory;
+import sun.security.util.DerOutputStream;
+import sun.security.util.DerValue;
+import sun.security.util.InternalPrivateKey;
 import sun.security.x509.AlgorithmId;
 import sun.security.x509.X509Key;
 
+import javax.security.auth.Destroyable;
 import java.io.IOException;
 import java.io.ObjectInputStream;
-import java.security.*;
+import java.lang.ref.Cleaner.Cleanable;
+import java.lang.ref.Reference;
+import java.security.InvalidKeyException;
+import java.security.Key;
+import java.security.KeyFactory;
+import java.security.KeyRep;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.security.PrivateKey;
+import java.security.Provider;
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.PKCS8EncodedKeySpec;
 import java.util.Arrays;
@@ -82,6 +95,8 @@ public class PKCS8Key implements PrivateKey, InternalPrivateKey {
     public static final int V1 = 0;
     public static final int V2 = 1;
 
+    private transient Cleanable cleanable;
+
     /**
      * Default constructor. Constructors in subclasses that create a new key
      * from its components require this. These constructors must initialize
@@ -96,12 +111,22 @@ protected PKCS8Key() { }
      *
      * This method is also used by {@link #parseKey} to create a raw key.
      */
+    @SuppressWarnings("this-escape")
     public PKCS8Key(byte[] input) throws InvalidKeyException {
         try {
             decode(new DerValue(input));
         } catch (IOException e) {
             throw new InvalidKeyException("Unable to decode key", e);
         }
+        final byte[] eK = this.encodedKey;
+        final byte[] pK = this.privKeyMaterial;
+        // this-escape is suppressed because the cleaner needs to hold a reference to the object
+        cleanable = CleanerFactory.cleaner().register(this,
+                                                      () -> {
+            if(eK != null)
+                Arrays.fill(eK, (byte) 0x00);
+            Arrays.fill(pK, (byte) 0x00);
+        });
     }
 
     private PKCS8Key(byte[] privEncoding, byte[] pubEncoding)
@@ -111,6 +136,23 @@ private PKCS8Key(byte[] privEncoding, byte[] pubEncoding)
         version = V2;
     }
 
+    /**
+     * Clears the internal copy of the key.
+     *
+     */
+    @Override
+    public void destroy() {
+        if (cleanable != null) {
+            cleanable.clean();
+            cleanable = null;
+        }
+    }
+
+    @Override
+    public boolean isDestroyed() {
+        return (cleanable == null);
+    }
+
     public int getVersion() {
         return version;
     }
@@ -256,8 +298,16 @@ public AlgorithmId getAlgorithmId () {
      * or {@code null} if an encoding error occurs.
      */
     public byte[] getEncoded() {
-        byte[] b = getEncodedInternal();
-        return (b != null) ? b.clone() : null;
+        try {
+            if (isDestroyed()) {
+                throw new IllegalStateException("Key is destroyed");
+            }
+            byte[] b = getEncodedInternal();
+            return (b != null) ? b.clone() : null;
+        } finally {
+            // prevent this from being cleaned for the above block
+            Reference.reachabilityFence(this);
+        }
     }
 
     /**
@@ -373,28 +423,45 @@ private void readObject(ObjectInputStream stream) throws IOException {
      */
     @Override
     public boolean equals(Object object) {
-        if (this == object) {
-            return true;
-        }
-        if (object instanceof PKCS8Key) {
-            // time-constant comparison
-            return MessageDigest.isEqual(
-                    getEncodedInternal(),
-                    ((PKCS8Key)object).getEncodedInternal());
-        } else if (object instanceof Key) {
-            // time-constant comparison
-            byte[] otherEncoded = ((Key)object).getEncoded();
-            try {
+        try {
+            if (this == object) {
+                return true;
+            }
+            if (object instanceof PKCS8Key pkcs8Key) {
+                // destroyed keys are considered different
+                if (isDestroyed() || pkcs8Key.isDestroyed()) {
+                    return false;
+                }
+                // time-constant comparison
                 return MessageDigest.isEqual(
                         getEncodedInternal(),
-                        otherEncoded);
-            } finally {
-                if (otherEncoded != null) {
-                    Arrays.fill(otherEncoded, (byte) 0);
+                        pkcs8Key.getEncodedInternal());
+            } else if (object instanceof Key keyHandle) {
+
+                if (keyHandle instanceof Destroyable destKeyHandle) {
+                    // destroyed keys are considered different
+                    if (isDestroyed() || destKeyHandle.isDestroyed()) {
+                        return false;
+                    }
+                }
+
+                // time-constant comparison
+                byte[] otherEncoded = ((Key) object).getEncoded();
+                try {
+                    return MessageDigest.isEqual(
+                            getEncodedInternal(),
+                            otherEncoded);
+                } finally {
+                    if (otherEncoded != null) {
+                        Arrays.fill(otherEncoded, (byte) 0);
+                    }
                 }
             }
+            return false;
+        } finally {
+            // prevent this from being cleaned for the above block
+            Reference.reachabilityFence(this);
         }
-        return false;
     }
 
     /**
@@ -403,13 +470,15 @@ public boolean equals(Object object) {
      */
     @Override
     public int hashCode() {
-        return Arrays.hashCode(getEncodedInternal());
+        try {
+            return Arrays.hashCode(getEncodedInternal());
+        } finally {
+            // prevent this from being cleaned for the above block
+            Reference.reachabilityFence(this);
+        }
     }
 
     public void clear() {
-        if (encodedKey != null) {
-            Arrays.fill(encodedKey, (byte)0);
-        }
-        Arrays.fill(privKeyMaterial, (byte)0);
+        destroy();
     }
 }

test/jdk/com/sun/crypto/provider/KeyFactory/EdDSAPrivateKeyDestroyTest.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 2025, 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 8303613
+ * @summary Check the destroy()/isDestroyed() of the EdDSA impl from SunEC
+ * @library /test/lib
+ * @run testng/othervm EdDSAPrivateKeyDestroyTest
+ */
+
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import java.security.KeyFactory;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.PrivateKey;
+import java.security.spec.KeySpec;
+import java.security.spec.PKCS8EncodedKeySpec;
+
+public class EdDSAPrivateKeyDestroyTest {
+    @Test
+    public void test() throws Exception {
+        KeyPairGenerator edDSAGen =
+                KeyPairGenerator.getInstance("EdDSA", "SunEC");
+
+        KeyPair kp = edDSAGen.generateKeyPair();
+
+        PrivateKey priv1 = kp.getPrivate();
+
+        KeySpec priv2Spec = new PKCS8EncodedKeySpec(priv1.getEncoded());
+        KeyFactory edDSAFac2 = KeyFactory.getInstance("EdDSA", "SunEC");
+        PrivateKey priv2 = edDSAFac2.generatePrivate(priv2Spec);
+
+
+        // should be equal
+        Assert.assertFalse(priv1.isDestroyed());
+        Assert.assertFalse(priv2.isDestroyed());
+        Assert.assertEquals(priv2, priv1);
+        Assert.assertEquals(priv1, priv2);
+        Assert.assertEquals(priv2.hashCode(), priv1.hashCode());
+
+        System.out.println("Past sanity checks");
+
+        // destroy key1
+        priv1.destroy();
+        Assert.assertTrue(priv1.isDestroyed());
+        Assert.assertNotEquals(priv2, priv1);
+        Assert.assertNotEquals(priv1, priv2);
+
+        System.out.println("Past destroy priv1");
+
+        // also destroy key2
+        priv2.destroy();
+        Assert.assertTrue(priv2.isDestroyed());
+        Assert.assertNotEquals(priv2, priv1);
+        Assert.assertNotEquals(priv1, priv2);
+
+        System.out.println("Past destroy priv2");
+
+        // call destroy again to make sure no unexpected exceptions
+        priv2.destroy();
+
+    }
+}