more wrap, less copy

wangweij · wangweij · commit b7216061921e · 2021-01-22T10:38:35.000-05:00
diff --git a/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2Core.java b/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2Core.java
@@ -158,9 +158,9 @@ protected SecretKey engineTranslateKey(SecretKey key)
                 } finally {
                     if (password != null) {
                         Arrays.fill(password, (char) 0);
+                        spec.clearPassword();
                     }
                     Arrays.fill(encoding, (byte)0);
-                    spec.clearPassword();
                 }
             }
         }
diff --git a/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2HmacSHA1Factory.java b/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2HmacSHA1Factory.java
@@ -158,9 +158,9 @@ protected SecretKey engineTranslateKey(SecretKey key)
                 } finally {
                     if (password != null) {
                         Arrays.fill(password, (char) 0);
+                        spec.clearPassword();
                     }
                     Arrays.fill(encoding, (byte)0);
-                    spec.clearPassword();
                 }
             }
         }
diff --git a/src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java b/src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java
@@ -218,20 +218,13 @@ public String getFormat() {
     private synchronized byte[] getEncodedInternal() {
         if (encodedKey == null) {
             try {
-                DerOutputStream out = new DerOutputStream ();
                 DerOutputStream tmp = new DerOutputStream();
                 tmp.putInteger(V1);
                 algid.encode(tmp);
-                // Write zeroes of correct length into intermediate
-                // DerOutputStream so that the real content will not
-                // appear everywhere. The actual bytes will be copied
-                // into the final encoding so they can be cleared
-                // effectively once required.
-                tmp.putOctetString(new byte[key.length]);
-                out.write(DerValue.tag_Sequence, tmp);
+                tmp.putOctetString(key);
+                DerValue out = DerValue.wrap(DerValue.tag_Sequence, tmp);
                 encodedKey = out.toByteArray();
-                // Copy the actual bytes
-                System.arraycopy(key, 0, encodedKey, encodedKey.length - key.length, key.length);
+                out.clear();
             } catch (IOException e) {
                 // encodedKey is still null
             }
diff --git a/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java b/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java
@@ -661,30 +661,23 @@ private void setKeyEntry(String alias, Key key,
                 keyEntry.date = new Date();
 
                 // Encode secret key in a PKCS#8
-                DerOutputStream pkcs8 = new DerOutputStream();
                 DerOutputStream secretKeyInfo = new DerOutputStream();
                 secretKeyInfo.putInteger(0);
                 AlgorithmId algId = AlgorithmId.get(key.getAlgorithm());
                 algId.encode(secretKeyInfo);
+
                 byte[] encoded = key.getEncoded();
-                // Write zeroes of correct length into intermediate
-                // DerOutputStream so that the real content will not
-                // appear everywhere. The actual bytes will be copied
-                // into the final encoding so they can be cleared
-                // effectively once required.
-                secretKeyInfo.putOctetString(new byte[encoded.length]);
-                pkcs8.write(DerValue.tag_Sequence, secretKeyInfo);
+                secretKeyInfo.putOctetString(encoded);
+                Arrays.fill(encoded, (byte)0);
 
+                DerValue pkcs8 = DerValue.wrap(DerValue.tag_Sequence, secretKeyInfo);
                 byte[] p8Array = pkcs8.toByteArray();
-                // Copy the actual bytes
-                System.arraycopy(encoded, 0, p8Array,
-                        p8Array.length - encoded.length, encoded.length);
+                pkcs8.clear();
                 try {
                     // Encrypt the secret key (using same PBE as for private keys)
                     keyEntry.protectedSecretKey =
                             encryptPrivateKey(p8Array, passwordProtection);
                 } finally {
-                    Arrays.fill(encoded, (byte)0);
                     Arrays.fill(p8Array, (byte)0);
                 }
 
diff --git a/src/java.base/share/classes/sun/security/rsa/RSAPrivateKeyImpl.java b/src/java.base/share/classes/sun/security/rsa/RSAPrivateKeyImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2021, 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
@@ -31,6 +31,7 @@
 import java.security.*;
 import java.security.spec.AlgorithmParameterSpec;
 import java.security.interfaces.*;
+import java.util.Arrays;
 
 import sun.security.util.*;
 import sun.security.pkcs.PKCS8Key;
@@ -90,19 +91,25 @@ public final class RSAPrivateKeyImpl extends PKCS8Key implements RSAPrivateKey {
 
         try {
             // generate the key encoding
-            DerOutputStream out = new DerOutputStream();
+            byte[] nbytes = n.toByteArray();
+            byte[] dbytes = d.toByteArray();
+            DerOutputStream out = new DerOutputStream(
+                    nbytes.length + dbytes.length + 50);
+                    // Enough for 7 zeroes (21) and 2 tag+length(4)
             out.putInteger(0); // version must be 0
-            out.putInteger(n);
+            out.putInteger(nbytes);
+            Arrays.fill(nbytes, (byte)0);
             out.putInteger(0);
-            out.putInteger(d);
+            out.putInteger(dbytes);
+            Arrays.fill(dbytes, (byte)0);
             out.putInteger(0);
             out.putInteger(0);
             out.putInteger(0);
             out.putInteger(0);
             out.putInteger(0);
-            DerValue val =
-                new DerValue(DerValue.tag_Sequence, out.toByteArray());
+            DerValue val = DerValue.wrap(DerValue.tag_Sequence, out);
             key = val.toByteArray();
+            val.clear();
         } catch (IOException exc) {
             // should never occur
             throw new InvalidKeyException(exc);
diff --git a/test/jdk/sun/security/pkcs11/rsa/TestKeyFactory.java b/test/jdk/sun/security/pkcs11/rsa/TestKeyFactory.java
@@ -98,8 +98,7 @@ public String getFormat() {
             }
             @Override
             public byte[] getEncoded() {
-                // skip cloning for testing key.
-                return encodedPriv;
+                return encodedPriv.clone();
             }
         };
         byte[] encodedPub = Base64.getDecoder().decode(PKCS1_PUB_STR);
@@ -114,8 +113,7 @@ public String getFormat() {
             }
             @Override
             public byte[] getEncoded() {
-                // skip cloning for testing key.
-                return encodedPub;
+                return encodedPub.clone();
             }
         };
     }
diff --git a/test/jdk/sun/security/rsa/TestKeyFactory.java b/test/jdk/sun/security/rsa/TestKeyFactory.java
@@ -97,8 +97,7 @@ public String getFormat() {
             }
             @Override
             public byte[] getEncoded() {
-                // skip cloning for testing key.
-                return encodedPriv;
+                return encodedPriv.clone();
             }
         };
         byte[] encodedPub = Base64.getDecoder().decode(PKCS1_PUB_STR);
@@ -113,8 +112,7 @@ public String getFormat() {
             }
             @Override
             public byte[] getEncoded() {
-                // skip cloning for testing key.
-                return encodedPub;
+                return encodedPub.clone();
             }
         };
     }

Original file line number	Diff line number	Diff line change
`@@ -158,9 +158,9 @@ protected SecretKey engineTranslateKey(SecretKey key)`
`158`	`158`	`} finally {`
`159`	`159`	`if (password != null) {`
`160`	`160`	`Arrays.fill(password, (char) 0);`
	`161`	`+ spec.clearPassword();`
`161`	`162`	`}`
`162`	`163`	`Arrays.fill(encoding, (byte)0);`
`163`		`- spec.clearPassword();`
`164`	`164`	`}`
`165`	`165`	`}`
`166`	`166`	`}`
Original file line number	Diff line number	Diff line change
`@@ -98,8 +98,7 @@ public String getFormat() {`
`98`	`98`	`}`
`99`	`99`	`@Override`
`100`	`100`	`public byte[] getEncoded() {`
`101`		`- // skip cloning for testing key.`
`102`		`- return encodedPriv;`
	`101`	`+ return encodedPriv.clone();`
`103`	`102`	`}`
`104`	`103`	`};`
`105`	`104`	`byte[] encodedPub = Base64.getDecoder().decode(PKCS1_PUB_STR);`
`@@ -114,8 +113,7 @@ public String getFormat() {`
`114`	`113`	`}`
`115`	`114`	`@Override`
`116`	`115`	`public byte[] getEncoded() {`
`117`		`- // skip cloning for testing key.`
`118`		`- return encodedPub;`
	`116`	`+ return encodedPub.clone();`
`119`	`117`	`}`
`120`	`118`	`};`
`121`	`119`	`}`
Original file line number	Diff line number	Diff line change
`@@ -97,8 +97,7 @@ public String getFormat() {`
`97`	`97`	`}`
`98`	`98`	`@Override`
`99`	`99`	`public byte[] getEncoded() {`
`100`		`- // skip cloning for testing key.`
`101`		`- return encodedPriv;`
	`100`	`+ return encodedPriv.clone();`
`102`	`101`	`}`
`103`	`102`	`};`
`104`	`103`	`byte[] encodedPub = Base64.getDecoder().decode(PKCS1_PUB_STR);`
`@@ -113,8 +112,7 @@ public String getFormat() {`
`113`	`112`	`}`
`114`	`113`	`@Override`
`115`	`114`	`public byte[] getEncoded() {`
`116`		`- // skip cloning for testing key.`
`117`		`- return encodedPub;`
	`115`	`+ return encodedPub.clone();`
`118`	`116`	`}`
`119`	`117`	`};`
`120`	`118`	`}`