# HG changeset patch
# User coffeys
# Date 1535020634 -3600
# Node ID c9a3e3cac9c733a434dbb7cc53a808f99c532c65
# Parent  0265a70ea2a5ee10e761e8b8652edaf3d4606d44
8209129: Further improvements to cipher buffer management
Reviewed-by: weijun, igerasim

diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/com/sun/crypto/provider/HmacPKCS12PBESHA1.java
--- a/src/java.base/share/classes/com/sun/crypto/provider/HmacPKCS12PBESHA1.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/com/sun/crypto/provider/HmacPKCS12PBESHA1.java	Thu Aug 23 11:37:14 2018 +0100
@@ -73,62 +73,69 @@
             salt = pbeKey.getSalt(); // maybe null if unspecified
             iCount = pbeKey.getIterationCount(); // maybe 0 if unspecified
         } else if (key instanceof SecretKey) {
-            byte[] passwdBytes = key.getEncoded();
-            if ((passwdBytes == null) ||
-                !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
+            byte[] passwdBytes;
+            if (!(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3)) ||
+                    (passwdBytes = key.getEncoded()) == null) {
                 throw new InvalidKeyException("Missing password");
             }
             passwdChars = new char[passwdBytes.length];
             for (int i=0; i<passwdChars.length; i++) {
                 passwdChars[i] = (char) (passwdBytes[i] & 0x7f);
             }
+            Arrays.fill(passwdBytes, (byte)0x00);
         } else {
             throw new InvalidKeyException("SecretKey of PBE type required");
         }
-        if (params == null) {
-            // should not auto-generate default values since current
-            // javax.crypto.Mac api does not have any method for caller to
-            // retrieve the generated defaults.
-            if ((salt == null) || (iCount == 0)) {
+
+        byte[] derivedKey;
+        try {
+            if (params == null) {
+                // should not auto-generate default values since current
+                // javax.crypto.Mac api does not have any method for caller to
+                // retrieve the generated defaults.
+                if ((salt == null) || (iCount == 0)) {
+                    throw new InvalidAlgorithmParameterException
+                            ("PBEParameterSpec required for salt and iteration count");
+                }
+            } else if (!(params instanceof PBEParameterSpec)) {
                 throw new InvalidAlgorithmParameterException
-                    ("PBEParameterSpec required for salt and iteration count");
-            }
-        } else if (!(params instanceof PBEParameterSpec)) {
-            throw new InvalidAlgorithmParameterException
-                ("PBEParameterSpec type required");
-        } else {
-            PBEParameterSpec pbeParams = (PBEParameterSpec) params;
-            // make sure the parameter values are consistent
-            if (salt != null) {
-                if (!Arrays.equals(salt, pbeParams.getSalt())) {
-                    throw new InvalidAlgorithmParameterException
-                        ("Inconsistent value of salt between key and params");
+                        ("PBEParameterSpec type required");
+            } else {
+                PBEParameterSpec pbeParams = (PBEParameterSpec) params;
+                // make sure the parameter values are consistent
+                if (salt != null) {
+                    if (!Arrays.equals(salt, pbeParams.getSalt())) {
+                        throw new InvalidAlgorithmParameterException
+                                ("Inconsistent value of salt between key and params");
+                    }
+                } else {
+                    salt = pbeParams.getSalt();
                 }
-            } else {
-                salt = pbeParams.getSalt();
+                if (iCount != 0) {
+                    if (iCount != pbeParams.getIterationCount()) {
+                        throw new InvalidAlgorithmParameterException
+                                ("Different iteration count between key and params");
+                    }
+                } else {
+                    iCount = pbeParams.getIterationCount();
+                }
             }
-            if (iCount != 0) {
-                if (iCount != pbeParams.getIterationCount()) {
-                    throw new InvalidAlgorithmParameterException
-                        ("Different iteration count between key and params");
-                }
-            } else {
-                iCount = pbeParams.getIterationCount();
+            // For security purpose, we need to enforce a minimum length
+            // for salt; just require the minimum salt length to be 8-byte
+            // which is what PKCS#5 recommends and openssl does.
+            if (salt.length < 8) {
+                throw new InvalidAlgorithmParameterException
+                        ("Salt must be at least 8 bytes long");
             }
+            if (iCount <= 0) {
+                throw new InvalidAlgorithmParameterException
+                        ("IterationCount must be a positive number");
+            }
+            derivedKey = PKCS12PBECipherCore.derive(passwdChars, salt,
+                    iCount, engineGetMacLength(), PKCS12PBECipherCore.MAC_KEY);
+        } finally {
+            Arrays.fill(passwdChars, '\0');
         }
-        // For security purpose, we need to enforce a minimum length
-        // for salt; just require the minimum salt length to be 8-byte
-        // which is what PKCS#5 recommends and openssl does.
-        if (salt.length < 8) {
-            throw new InvalidAlgorithmParameterException
-                ("Salt must be at least 8 bytes long");
-        }
-        if (iCount <= 0) {
-            throw new InvalidAlgorithmParameterException
-                ("IterationCount must be a positive number");
-        }
-        byte[] derivedKey = PKCS12PBECipherCore.derive(passwdChars, salt,
-            iCount, engineGetMacLength(), PKCS12PBECipherCore.MAC_KEY);
         SecretKey cipherKey = new SecretKeySpec(derivedKey, "HmacSHA1");
         super.engineInit(cipherKey, null);
     }
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/com/sun/crypto/provider/KeyProtector.java
--- a/src/java.base/share/classes/com/sun/crypto/provider/KeyProtector.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/com/sun/crypto/provider/KeyProtector.java	Thu Aug 23 11:37:14 2018 +0100
@@ -310,14 +310,14 @@
         Cipher cipher;
         try {
             sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES", false);
-        pbeKeySpec.clearPassword();
+            pbeKeySpec.clearPassword();
 
-        // seal key
-        PBEWithMD5AndTripleDESCipher cipherSpi;
-        cipherSpi = new PBEWithMD5AndTripleDESCipher();
-        cipher = new CipherForKeyProtector(cipherSpi, SunJCE.getInstance(),
-                                           "PBEWithMD5AndTripleDES");
-        cipher.init(Cipher.ENCRYPT_MODE, sKey, pbeSpec);
+            // seal key
+            PBEWithMD5AndTripleDESCipher cipherSpi;
+            cipherSpi = new PBEWithMD5AndTripleDESCipher();
+            cipher = new CipherForKeyProtector(cipherSpi, SunJCE.getInstance(),
+                                               "PBEWithMD5AndTripleDES");
+            cipher.init(Cipher.ENCRYPT_MODE, sKey, pbeSpec);
         } finally {
             if (sKey != null) sKey.destroy();
         }
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java
--- a/src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java	Thu Aug 23 11:37:14 2018 +0100
@@ -73,7 +73,7 @@
         this.key = new byte[passwd.length];
         for (int i=0; i<passwd.length; i++)
             this.key[i] = (byte) (passwd[i] & 0x7f);
-        Arrays.fill(passwd, ' ');
+        Arrays.fill(passwd, '\0');
         type = keytype;
 
         // Use the cleaner to zero the key when no longer referenced
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/com/sun/crypto/provider/PBES1Core.java
--- a/src/java.base/share/classes/com/sun/crypto/provider/PBES1Core.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/com/sun/crypto/provider/PBES1Core.java	Thu Aug 23 11:37:14 2018 +0100
@@ -27,6 +27,7 @@
 
 import java.security.*;
 import java.security.spec.*;
+import java.util.Arrays;
 import javax.crypto.*;
 import javax.crypto.spec.*;
 
@@ -213,35 +214,43 @@
             throw new InvalidAlgorithmParameterException("Parameters "
                                                          + "missing");
         }
-        if ((key == null) ||
-            (key.getEncoded() == null) ||
-            !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
-            throw new InvalidKeyException("Missing password");
+        if (key == null) {
+            throw new InvalidKeyException("Null key");
         }
 
-        if (params == null) {
-            // create random salt and use default iteration count
-            salt = new byte[8];
-            random.nextBytes(salt);
-        } else {
-            if (!(params instanceof PBEParameterSpec)) {
-                throw new InvalidAlgorithmParameterException
-                    ("Wrong parameter type: PBE expected");
+        byte[] derivedKey;
+        byte[] passwdBytes = key.getEncoded();
+        try {
+            if ((passwdBytes == null) ||
+                    !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
+                throw new InvalidKeyException("Missing password");
             }
-            salt = ((PBEParameterSpec) params).getSalt();
-            // salt must be 8 bytes long (by definition)
-            if (salt.length != 8) {
-                throw new InvalidAlgorithmParameterException
-                    ("Salt must be 8 bytes long");
+
+            if (params == null) {
+                // create random salt and use default iteration count
+                salt = new byte[8];
+                random.nextBytes(salt);
+            } else {
+                if (!(params instanceof PBEParameterSpec)) {
+                    throw new InvalidAlgorithmParameterException
+                            ("Wrong parameter type: PBE expected");
+                }
+                salt = ((PBEParameterSpec) params).getSalt();
+                // salt must be 8 bytes long (by definition)
+                if (salt.length != 8) {
+                    throw new InvalidAlgorithmParameterException
+                            ("Salt must be 8 bytes long");
+                }
+                iCount = ((PBEParameterSpec) params).getIterationCount();
+                if (iCount <= 0) {
+                    throw new InvalidAlgorithmParameterException
+                            ("IterationCount must be a positive number");
+                }
             }
-            iCount = ((PBEParameterSpec) params).getIterationCount();
-            if (iCount <= 0) {
-                throw new InvalidAlgorithmParameterException
-                    ("IterationCount must be a positive number");
-            }
+            derivedKey = deriveCipherKey(passwdBytes);
+        } finally {
+            if (passwdBytes != null) Arrays.fill(passwdBytes, (byte) 0x00);
         }
-
-        byte[] derivedKey = deriveCipherKey(key);
         // use all but the last 8 bytes as the key value
         SecretKeySpec cipherKey = new SecretKeySpec(derivedKey, 0,
                                                     derivedKey.length-8, algo);
@@ -253,16 +262,14 @@
         cipher.init(opmode, cipherKey, ivSpec, random);
     }
 
-    private byte[] deriveCipherKey(Key key) {
+    private byte[] deriveCipherKey(byte[] passwdBytes) {
 
         byte[] result = null;
-        byte[] passwdBytes = key.getEncoded();
 
         if (algo.equals("DES")) {
             // P || S (password concatenated with salt)
             byte[] concat = new byte[Math.addExact(passwdBytes.length, salt.length)];
             System.arraycopy(passwdBytes, 0, concat, 0, passwdBytes.length);
-            java.util.Arrays.fill(passwdBytes, (byte)0x00);
             System.arraycopy(salt, 0, concat, passwdBytes.length, salt.length);
 
             // digest P || S with c iterations
@@ -271,7 +278,7 @@
                 md.update(toBeHashed);
                 toBeHashed = md.digest(); // this resets the digest
             }
-            java.util.Arrays.fill(concat, (byte)0x00);
+            Arrays.fill(concat, (byte)0x00);
             result = toBeHashed;
         } else if (algo.equals("DESede")) {
             // if the 2 salt halves are the same, invert one of them
@@ -294,8 +301,6 @@
             // Concatenate the output from each digest round with the
             // password, and use the result as the input to the next digest
             // operation.
-            byte[] kBytes = null;
-            IvParameterSpec iv = null;
             byte[] toBeHashed = null;
             result = new byte[DESedeKeySpec.DES_EDE_KEY_LEN +
                               DESConstants.DES_BLOCK_SIZE];
@@ -306,12 +311,14 @@
                 for (int j=0; j < iCount; j++) {
                     md.update(toBeHashed);
                     md.update(passwdBytes);
-                    toBeHashed = md.digest(); // this resets the digest
+                    toBeHashed = md.digest();
                 }
                 System.arraycopy(toBeHashed, 0, result, i*16,
                                  toBeHashed.length);
             }
         }
+        // clear data used in message
+        md.reset();
         return result;
     }
 
@@ -478,9 +485,9 @@
     byte[] wrap(Key key)
         throws IllegalBlockSizeException, InvalidKeyException {
         byte[] result = null;
-
+        byte[] encodedKey = null;
         try {
-            byte[] encodedKey = key.getEncoded();
+            encodedKey = key.getEncoded();
             if ((encodedKey == null) || (encodedKey.length == 0)) {
                 throw new InvalidKeyException("Cannot get an encoding of " +
                                               "the key to be wrapped");
@@ -489,6 +496,8 @@
             result = doFinal(encodedKey, 0, encodedKey.length);
         } catch (BadPaddingException e) {
             // Should never happen
+        } finally {
+            if (encodedKey != null) Arrays.fill(encodedKey, (byte)0x00);
         }
 
         return result;
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java
--- a/src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java	Thu Aug 23 11:37:14 2018 +0100
@@ -27,6 +27,7 @@
 
 import java.security.*;
 import java.security.spec.*;
+import java.util.Arrays;
 import javax.crypto.*;
 import javax.crypto.spec.*;
 
@@ -173,101 +174,105 @@
                               SecureRandom random)
         throws InvalidKeyException, InvalidAlgorithmParameterException {
 
-        if ((key == null) ||
-            (key.getEncoded() == null) ||
-            !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
-            throw new InvalidKeyException("Missing password");
-        }
-
-        // TBD: consolidate the salt, ic and IV parameter checks below
-
-        // Extract salt and iteration count from the key, if present
-        if (key instanceof javax.crypto.interfaces.PBEKey) {
-            salt = ((javax.crypto.interfaces.PBEKey)key).getSalt();
-            if (salt != null && salt.length < 8) {
-                throw new InvalidAlgorithmParameterException(
-                    "Salt must be at least 8 bytes long");
-            }
-            iCount = ((javax.crypto.interfaces.PBEKey)key).getIterationCount();
-            if (iCount == 0) {
-                iCount = DEFAULT_COUNT;
-            } else if (iCount < 0) {
-                throw new InvalidAlgorithmParameterException(
-                    "Iteration count must be a positive number");
-            }
+        if (key == null) {
+            throw new InvalidKeyException("Null key");
         }
 
-        // Extract salt, iteration count and IV from the params, if present
-        if (params == null) {
-            if (salt == null) {
-                // generate random salt and use default iteration count
-                salt = new byte[DEFAULT_SALT_LENGTH];
-                random.nextBytes(salt);
-                iCount = DEFAULT_COUNT;
+        byte[] passwdBytes = key.getEncoded();
+        char[] passwdChars = null;
+        PBEKeySpec pbeSpec;
+        try {
+            if ((passwdBytes == null) ||
+                    !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
+                throw new InvalidKeyException("Missing password");
             }
-            if ((opmode == Cipher.ENCRYPT_MODE) ||
-                        (opmode == Cipher.WRAP_MODE)) {
-                // generate random IV
-                byte[] ivBytes = new byte[blkSize];
-                random.nextBytes(ivBytes);
-                ivSpec = new IvParameterSpec(ivBytes);
-            }
-        } else {
-            if (!(params instanceof PBEParameterSpec)) {
-                throw new InvalidAlgorithmParameterException
-                    ("Wrong parameter type: PBE expected");
+
+            // TBD: consolidate the salt, ic and IV parameter checks below
+
+            // Extract salt and iteration count from the key, if present
+            if (key instanceof javax.crypto.interfaces.PBEKey) {
+                salt = ((javax.crypto.interfaces.PBEKey)key).getSalt();
+                if (salt != null && salt.length < 8) {
+                    throw new InvalidAlgorithmParameterException(
+                            "Salt must be at least 8 bytes long");
+                }
+                iCount = ((javax.crypto.interfaces.PBEKey)key).getIterationCount();
+                if (iCount == 0) {
+                    iCount = DEFAULT_COUNT;
+                } else if (iCount < 0) {
+                    throw new InvalidAlgorithmParameterException(
+                            "Iteration count must be a positive number");
+                }
             }
-            // salt and iteration count from the params take precedence
-            byte[] specSalt = ((PBEParameterSpec) params).getSalt();
-            if (specSalt != null && specSalt.length < 8) {
-                throw new InvalidAlgorithmParameterException(
-                    "Salt must be at least 8 bytes long");
-            }
-            salt = specSalt;
-            int specICount = ((PBEParameterSpec) params).getIterationCount();
-            if (specICount == 0) {
-                specICount = DEFAULT_COUNT;
-            } else if (specICount < 0) {
-                throw new InvalidAlgorithmParameterException(
-                    "Iteration count must be a positive number");
-            }
-            iCount = specICount;
 
-            AlgorithmParameterSpec specParams =
-                ((PBEParameterSpec) params).getParameterSpec();
-            if (specParams != null) {
-                if (specParams instanceof IvParameterSpec) {
-                    ivSpec = (IvParameterSpec)specParams;
+            // Extract salt, iteration count and IV from the params, if present
+            if (params == null) {
+                if (salt == null) {
+                    // generate random salt and use default iteration count
+                    salt = new byte[DEFAULT_SALT_LENGTH];
+                    random.nextBytes(salt);
+                    iCount = DEFAULT_COUNT;
+                }
+                if ((opmode == Cipher.ENCRYPT_MODE) ||
+                        (opmode == Cipher.WRAP_MODE)) {
+                    // generate random IV
+                    byte[] ivBytes = new byte[blkSize];
+                    random.nextBytes(ivBytes);
+                    ivSpec = new IvParameterSpec(ivBytes);
+                }
+            } else {
+                if (!(params instanceof PBEParameterSpec)) {
+                    throw new InvalidAlgorithmParameterException
+                            ("Wrong parameter type: PBE expected");
+                }
+                // salt and iteration count from the params take precedence
+                byte[] specSalt = ((PBEParameterSpec) params).getSalt();
+                if (specSalt != null && specSalt.length < 8) {
+                    throw new InvalidAlgorithmParameterException(
+                            "Salt must be at least 8 bytes long");
+                }
+                salt = specSalt;
+                int specICount = ((PBEParameterSpec) params).getIterationCount();
+                if (specICount == 0) {
+                    specICount = DEFAULT_COUNT;
+                } else if (specICount < 0) {
+                    throw new InvalidAlgorithmParameterException(
+                            "Iteration count must be a positive number");
+                }
+                iCount = specICount;
+
+                AlgorithmParameterSpec specParams =
+                        ((PBEParameterSpec) params).getParameterSpec();
+                if (specParams != null) {
+                    if (specParams instanceof IvParameterSpec) {
+                        ivSpec = (IvParameterSpec)specParams;
+                    } else {
+                        throw new InvalidAlgorithmParameterException(
+                                "Wrong parameter type: IV expected");
+                    }
+                } else if ((opmode == Cipher.ENCRYPT_MODE) ||
+                        (opmode == Cipher.WRAP_MODE)) {
+                    // generate random IV
+                    byte[] ivBytes = new byte[blkSize];
+                    random.nextBytes(ivBytes);
+                    ivSpec = new IvParameterSpec(ivBytes);
                 } else {
                     throw new InvalidAlgorithmParameterException(
-                        "Wrong parameter type: IV expected");
+                            "Missing parameter type: IV expected");
                 }
-            } else if ((opmode == Cipher.ENCRYPT_MODE) ||
-                        (opmode == Cipher.WRAP_MODE)) {
-                // generate random IV
-                byte[] ivBytes = new byte[blkSize];
-                random.nextBytes(ivBytes);
-                ivSpec = new IvParameterSpec(ivBytes);
-            } else {
-                throw new InvalidAlgorithmParameterException(
-                    "Missing parameter type: IV expected");
             }
-        }
 
-        SecretKeySpec cipherKey = null;
-        byte[] derivedKey = null;
-        byte[] passwdBytes = key.getEncoded();
-        char[] passwdChars = new char[passwdBytes.length];
+            passwdChars = new char[passwdBytes.length];
+            for (int i = 0; i < passwdChars.length; i++)
+                passwdChars[i] = (char) (passwdBytes[i] & 0x7f);
 
-        for (int i=0; i<passwdChars.length; i++)
-            passwdChars[i] = (char) (passwdBytes[i] & 0x7f);
-
-        PBEKeySpec pbeSpec =
-            new PBEKeySpec(passwdChars, salt, iCount, keyLength);
+            pbeSpec = new PBEKeySpec(passwdChars, salt, iCount, keyLength);
             // password char[] was cloned in PBEKeySpec constructor,
             // so we can zero it out here
-        java.util.Arrays.fill(passwdChars, ' ');
-        java.util.Arrays.fill(passwdBytes, (byte)0x00);
+        } finally {
+            if (passwdChars != null) Arrays.fill(passwdChars, '\0');
+            if (passwdBytes != null) Arrays.fill(passwdBytes, (byte)0x00);
+        }
 
         SecretKey s = null;
 
@@ -280,8 +285,8 @@
             ike.initCause(ikse);
             throw ike;
         }
-        derivedKey = s.getEncoded();
-        cipherKey = new SecretKeySpec(derivedKey, cipherAlgo);
+        byte[] derivedKey = s.getEncoded();
+        SecretKeySpec cipherKey = new SecretKeySpec(derivedKey, cipherAlgo);
 
         // initialize the underlying cipher
         cipher.init(opmode, cipherKey, ivSpec, random);
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/com/sun/crypto/provider/PBKDF2KeyImpl.java
--- a/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2KeyImpl.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2KeyImpl.java	Thu Aug 23 11:37:14 2018 +0100
@@ -93,46 +93,50 @@
         }
         // Convert the password from char[] to byte[]
         byte[] passwdBytes = getPasswordBytes(this.passwd);
+        // remove local copy
+        if (passwd != null) Arrays.fill(passwd, '\0');
 
-        this.salt = keySpec.getSalt();
-        if (salt == null) {
-            throw new InvalidKeySpecException("Salt not found");
-        }
-        this.iterCount = keySpec.getIterationCount();
-        if (iterCount == 0) {
-            throw new InvalidKeySpecException("Iteration count not found");
-        } else if (iterCount < 0) {
-            throw new InvalidKeySpecException("Iteration count is negative");
-        }
-        int keyLength = keySpec.getKeyLength();
-        if (keyLength == 0) {
-            throw new InvalidKeySpecException("Key length not found");
-        } else if (keyLength < 0) {
-            throw new InvalidKeySpecException("Key length is negative");
-        }
         try {
+            this.salt = keySpec.getSalt();
+            if (salt == null) {
+                throw new InvalidKeySpecException("Salt not found");
+            }
+            this.iterCount = keySpec.getIterationCount();
+            if (iterCount == 0) {
+                throw new InvalidKeySpecException("Iteration count not found");
+            } else if (iterCount < 0) {
+                throw new InvalidKeySpecException("Iteration count is negative");
+            }
+            int keyLength = keySpec.getKeyLength();
+            if (keyLength == 0) {
+                throw new InvalidKeySpecException("Key length not found");
+            } else if (keyLength < 0) {
+                throw new InvalidKeySpecException("Key length is negative");
+            }
             this.prf = Mac.getInstance(prfAlgo);
             // SunPKCS11 requires a non-empty PBE password
             if (passwdBytes.length == 0 &&
-                this.prf.getProvider().getName().startsWith("SunPKCS11")) {
+                    this.prf.getProvider().getName().startsWith("SunPKCS11")) {
                 this.prf = Mac.getInstance(prfAlgo, SunJCE.getInstance());
             }
+            this.key = deriveKey(prf, passwdBytes, salt, iterCount, keyLength);
         } catch (NoSuchAlgorithmException nsae) {
             // not gonna happen; re-throw just in case
             InvalidKeySpecException ike = new InvalidKeySpecException();
             ike.initCause(nsae);
             throw ike;
-        }
-        this.key = deriveKey(prf, passwdBytes, salt, iterCount, keyLength);
+        } finally {
+            Arrays.fill(passwdBytes, (byte) 0x00);
 
-        // Use the cleaner to zero the key when no longer referenced
-        final byte[] k = this.key;
-        final char[] p = this.passwd;
-        CleanerFactory.cleaner().register(this,
-                () -> {
-                    java.util.Arrays.fill(k, (byte)0x00);
-                    java.util.Arrays.fill(p, '0');
-                });
+            // Use the cleaner to zero the key when no longer referenced
+            final byte[] k = this.key;
+            final char[] p = this.passwd;
+            CleanerFactory.cleaner().register(this,
+                    () -> {
+                        Arrays.fill(k, (byte) 0x00);
+                        Arrays.fill(p, '\0');
+                    });
+        }
     }
 
     private static byte[] deriveKey(final Mac prf, final byte[] password,
@@ -266,8 +270,8 @@
         if (!(that.getFormat().equalsIgnoreCase("RAW")))
             return false;
         byte[] thatEncoded = that.getEncoded();
-        boolean ret = MessageDigest.isEqual(key, that.getEncoded());
-        java.util.Arrays.fill(thatEncoded, (byte)0x00);
+        boolean ret = MessageDigest.isEqual(key, thatEncoded);
+        Arrays.fill(thatEncoded, (byte)0x00);
         return ret;
     }
 
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Core.java
--- a/src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Core.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Core.java	Thu Aug 23 11:37:14 2018 +0100
@@ -108,72 +108,76 @@
             salt = pbeKey.getSalt(); // maybe null if unspecified
             iCount = pbeKey.getIterationCount(); // maybe 0 if unspecified
         } else if (key instanceof SecretKey) {
-            byte[] passwdBytes = key.getEncoded();
-            if ((passwdBytes == null) ||
-                !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
+            byte[] passwdBytes;
+            if (!(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3)) ||
+                    (passwdBytes = key.getEncoded()) == null) {
                 throw new InvalidKeyException("Missing password");
             }
             passwdChars = new char[passwdBytes.length];
             for (int i=0; i<passwdChars.length; i++) {
                 passwdChars[i] = (char) (passwdBytes[i] & 0x7f);
             }
+            Arrays.fill(passwdBytes, (byte)0x00);
         } else {
             throw new InvalidKeyException("SecretKey of PBE type required");
         }
-        if (params == null) {
-            // should not auto-generate default values since current
-            // javax.crypto.Mac api does not have any method for caller to
-            // retrieve the generated defaults.
-            if ((salt == null) || (iCount == 0)) {
+
+        PBEKeySpec pbeSpec;
+        try {
+            if (params == null) {
+                // should not auto-generate default values since current
+                // javax.crypto.Mac api does not have any method for caller to
+                // retrieve the generated defaults.
+                if ((salt == null) || (iCount == 0)) {
+                    throw new InvalidAlgorithmParameterException
+                            ("PBEParameterSpec required for salt and iteration count");
+                }
+            } else if (!(params instanceof PBEParameterSpec)) {
                 throw new InvalidAlgorithmParameterException
-                    ("PBEParameterSpec required for salt and iteration count");
-            }
-        } else if (!(params instanceof PBEParameterSpec)) {
-            throw new InvalidAlgorithmParameterException
-                ("PBEParameterSpec type required");
-        } else {
-            PBEParameterSpec pbeParams = (PBEParameterSpec) params;
-            // make sure the parameter values are consistent
-            if (salt != null) {
-                if (!Arrays.equals(salt, pbeParams.getSalt())) {
-                    throw new InvalidAlgorithmParameterException
-                        ("Inconsistent value of salt between key and params");
-                }
+                        ("PBEParameterSpec type required");
             } else {
-                salt = pbeParams.getSalt();
-            }
-            if (iCount != 0) {
-                if (iCount != pbeParams.getIterationCount()) {
-                    throw new InvalidAlgorithmParameterException
-                        ("Different iteration count between key and params");
+                PBEParameterSpec pbeParams = (PBEParameterSpec) params;
+                // make sure the parameter values are consistent
+                if (salt != null) {
+                    if (!Arrays.equals(salt, pbeParams.getSalt())) {
+                        throw new InvalidAlgorithmParameterException
+                                ("Inconsistent value of salt between key and params");
+                    }
+                } else {
+                    salt = pbeParams.getSalt();
                 }
-            } else {
-                iCount = pbeParams.getIterationCount();
+                if (iCount != 0) {
+                    if (iCount != pbeParams.getIterationCount()) {
+                        throw new InvalidAlgorithmParameterException
+                                ("Different iteration count between key and params");
+                    }
+                } else {
+                    iCount = pbeParams.getIterationCount();
+                }
             }
-        }
-        // For security purpose, we need to enforce a minimum length
-        // for salt; just require the minimum salt length to be 8-byte
-        // which is what PKCS#5 recommends and openssl does.
-        if (salt.length < 8) {
-            throw new InvalidAlgorithmParameterException
-                ("Salt must be at least 8 bytes long");
-        }
-        if (iCount <= 0) {
-            throw new InvalidAlgorithmParameterException
-                ("IterationCount must be a positive number");
+            // For security purpose, we need to enforce a minimum length
+            // for salt; just require the minimum salt length to be 8-byte
+            // which is what PKCS#5 recommends and openssl does.
+            if (salt.length < 8) {
+                throw new InvalidAlgorithmParameterException
+                        ("Salt must be at least 8 bytes long");
+            }
+            if (iCount <= 0) {
+                throw new InvalidAlgorithmParameterException
+                        ("IterationCount must be a positive number");
+            }
+
+            pbeSpec = new PBEKeySpec(passwdChars, salt, iCount, blockLength);
+            // password char[] was cloned in PBEKeySpec constructor,
+            // so we can zero it out here
+        } finally {
+            Arrays.fill(passwdChars, '\0');
         }
 
-        PBEKeySpec pbeSpec =
-            new PBEKeySpec(passwdChars, salt, iCount, blockLength);
-            // password char[] was cloned in PBEKeySpec constructor,
-            // so we can zero it out here
-        java.util.Arrays.fill(passwdChars, ' ');
-
-        SecretKey s = null;
+        SecretKey s;
         PBKDF2Core kdf = getKDFImpl(kdfAlgo);
         try {
             s = kdf.engineGenerateSecret(pbeSpec);
-
         } catch (InvalidKeySpecException ikse) {
             InvalidKeyException ike =
                 new InvalidKeyException("Cannot construct PBE key");
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/com/sun/crypto/provider/PKCS12PBECipherCore.java
--- a/src/java.base/share/classes/com/sun/crypto/provider/PKCS12PBECipherCore.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/com/sun/crypto/provider/PKCS12PBECipherCore.java	Thu Aug 23 11:37:14 2018 +0100
@@ -104,6 +104,7 @@
             Arrays.fill(D, (byte)type);
             concat(salt, I, 0, s);
             concat(passwd, I, s, p);
+            Arrays.fill(passwd, (byte) 0x00);
 
             byte[] Ai;
             byte[] B = new byte[v];
@@ -268,87 +269,92 @@
             salt = pbeKey.getSalt(); // maybe null if unspecified
             iCount = pbeKey.getIterationCount(); // maybe 0 if unspecified
         } else if (key instanceof SecretKey) {
-            byte[] passwdBytes = key.getEncoded();
-            if ((passwdBytes == null) ||
-                !(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3))) {
+            byte[] passwdBytes;
+            if (!(key.getAlgorithm().regionMatches(true, 0, "PBE", 0, 3)) ||
+                    (passwdBytes = key.getEncoded()) == null) {
                 throw new InvalidKeyException("Missing password");
             }
             passwdChars = new char[passwdBytes.length];
             for (int i=0; i<passwdChars.length; i++) {
                 passwdChars[i] = (char) (passwdBytes[i] & 0x7f);
             }
+            Arrays.fill(passwdBytes, (byte)0x00);
         } else {
             throw new InvalidKeyException("SecretKey of PBE type required");
         }
 
-        if (((opmode == Cipher.DECRYPT_MODE) ||
-             (opmode == Cipher.UNWRAP_MODE)) &&
-            ((params == null) && ((salt == null) || (iCount == 0)))) {
-            throw new InvalidAlgorithmParameterException
-                ("Parameters missing");
-        }
+        try {
+            if (((opmode == Cipher.DECRYPT_MODE) ||
+                    (opmode == Cipher.UNWRAP_MODE)) &&
+                    ((params == null) && ((salt == null) || (iCount == 0)))) {
+                throw new InvalidAlgorithmParameterException
+                        ("Parameters missing");
+            }
 
-        if (params == null) {
-            // generate default for salt and iteration count if necessary
-            if (salt == null) {
-                salt = new byte[DEFAULT_SALT_LENGTH];
-                if (random != null) {
-                    random.nextBytes(salt);
+            if (params == null) {
+                // generate default for salt and iteration count if necessary
+                if (salt == null) {
+                    salt = new byte[DEFAULT_SALT_LENGTH];
+                    if (random != null) {
+                        random.nextBytes(salt);
+                    } else {
+                        SunJCE.getRandom().nextBytes(salt);
+                    }
+                }
+                if (iCount == 0) iCount = DEFAULT_COUNT;
+            } else if (!(params instanceof PBEParameterSpec)) {
+                throw new InvalidAlgorithmParameterException
+                        ("PBEParameterSpec type required");
+            } else {
+                PBEParameterSpec pbeParams = (PBEParameterSpec) params;
+                // make sure the parameter values are consistent
+                if (salt != null) {
+                    if (!Arrays.equals(salt, pbeParams.getSalt())) {
+                        throw new InvalidAlgorithmParameterException
+                                ("Inconsistent value of salt between key and params");
+                    }
                 } else {
-                    SunJCE.getRandom().nextBytes(salt);
+                    salt = pbeParams.getSalt();
+                }
+                if (iCount != 0) {
+                    if (iCount != pbeParams.getIterationCount()) {
+                        throw new InvalidAlgorithmParameterException
+                                ("Different iteration count between key and params");
+                    }
+                } else {
+                    iCount = pbeParams.getIterationCount();
                 }
             }
-            if (iCount == 0) iCount = DEFAULT_COUNT;
-        } else if (!(params instanceof PBEParameterSpec)) {
-            throw new InvalidAlgorithmParameterException
-                ("PBEParameterSpec type required");
-        } else {
-            PBEParameterSpec pbeParams = (PBEParameterSpec) params;
-            // make sure the parameter values are consistent
-            if (salt != null) {
-                if (!Arrays.equals(salt, pbeParams.getSalt())) {
-                    throw new InvalidAlgorithmParameterException
-                        ("Inconsistent value of salt between key and params");
-                }
-            } else {
-                salt = pbeParams.getSalt();
+            // salt is recommended to be ideally as long as the output
+            // of the hash function. However, it may be too strict to
+            // force this; so instead, we'll just require the minimum
+            // salt length to be 8-byte which is what PKCS#5 recommends
+            // and openssl does.
+            if (salt.length < 8) {
+                throw new InvalidAlgorithmParameterException
+                        ("Salt must be at least 8 bytes long");
             }
-            if (iCount != 0) {
-                if (iCount != pbeParams.getIterationCount()) {
-                    throw new InvalidAlgorithmParameterException
-                        ("Different iteration count between key and params");
-                }
-            } else {
-                iCount = pbeParams.getIterationCount();
+            if (iCount <= 0) {
+                throw new InvalidAlgorithmParameterException
+                        ("IterationCount must be a positive number");
             }
-        }
-        // salt is recommended to be ideally as long as the output
-        // of the hash function. However, it may be too strict to
-        // force this; so instead, we'll just require the minimum
-        // salt length to be 8-byte which is what PKCS#5 recommends
-        // and openssl does.
-        if (salt.length < 8) {
-            throw new InvalidAlgorithmParameterException
-                ("Salt must be at least 8 bytes long");
-        }
-        if (iCount <= 0) {
-            throw new InvalidAlgorithmParameterException
-                ("IterationCount must be a positive number");
-        }
-        byte[] derivedKey = derive(passwdChars, salt, iCount,
-                                   keySize, CIPHER_KEY);
-        SecretKey cipherKey = new SecretKeySpec(derivedKey, algo);
+            byte[] derivedKey = derive(passwdChars, salt, iCount,
+                    keySize, CIPHER_KEY);
+            SecretKey cipherKey = new SecretKeySpec(derivedKey, algo);
+
+            if (cipherImpl != null && cipherImpl instanceof ARCFOURCipher) {
+                ((ARCFOURCipher)cipherImpl).engineInit(opmode, cipherKey, random);
 
-        if (cipherImpl != null && cipherImpl instanceof ARCFOURCipher) {
-            ((ARCFOURCipher)cipherImpl).engineInit(opmode, cipherKey, random);
+            } else {
+                byte[] derivedIv = derive(passwdChars, salt, iCount, 8,
+                        CIPHER_IV);
+                IvParameterSpec ivSpec = new IvParameterSpec(derivedIv, 0, 8);
 
-        } else {
-            byte[] derivedIv = derive(passwdChars, salt, iCount, 8,
-                                  CIPHER_IV);
-            IvParameterSpec ivSpec = new IvParameterSpec(derivedIv, 0, 8);
-
-            // initialize the underlying cipher
-            cipher.init(opmode, cipherKey, ivSpec, random);
+                // initialize the underlying cipher
+                cipher.init(opmode, cipherKey, ivSpec, random);
+            }
+        } finally {
+           Arrays.fill(passwdChars, '\0');
         }
     }
 
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/sun/security/provider/DigestBase.java
--- a/src/java.base/share/classes/sun/security/provider/DigestBase.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/sun/security/provider/DigestBase.java	Thu Aug 23 11:37:14 2018 +0100
@@ -28,6 +28,7 @@
 import java.security.MessageDigestSpi;
 import java.security.DigestException;
 import java.security.ProviderException;
+import java.util.Arrays;
 import java.util.Objects;
 
 import jdk.internal.HotSpotIntrinsicCandidate;
@@ -178,6 +179,7 @@
         implReset();
         bufOfs = 0;
         bytesProcessed = 0;
+        Arrays.fill(buffer, (byte) 0x00);
     }
 
     // return the digest. See JCA doc.
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/sun/security/provider/MD4.java
--- a/src/java.base/share/classes/sun/security/provider/MD4.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/sun/security/provider/MD4.java	Thu Aug 23 11:37:14 2018 +0100
@@ -26,6 +26,7 @@
 package sun.security.provider;
 
 import java.security.*;
+import java.util.Arrays;
 
 import static sun.security.provider.ByteArrayAccess.*;
 import static sun.security.util.SecurityConstants.PROVIDER_VER;
@@ -92,7 +93,7 @@
         super("MD4", 16, 64);
         state = new int[4];
         x = new int[16];
-        implReset();
+        resetHashes();
     }
 
     // clone this object
@@ -108,6 +109,12 @@
      */
     void implReset() {
         // Load magic initialization constants.
+        resetHashes();
+        // clear out old data
+        Arrays.fill(x, 0);
+    }
+
+    private void resetHashes() {
         state[0] = 0x67452301;
         state[1] = 0xefcdab89;
         state[2] = 0x98badcfe;
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/sun/security/provider/MD5.java
--- a/src/java.base/share/classes/sun/security/provider/MD5.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/sun/security/provider/MD5.java	Thu Aug 23 11:37:14 2018 +0100
@@ -25,6 +25,8 @@
 
 package sun.security.provider;
 
+import java.util.Arrays;
+
 import static sun.security.provider.ByteArrayAccess.*;
 
 /**
@@ -66,7 +68,7 @@
         super("MD5", 16, 64);
         state = new int[4];
         x = new int[16];
-        implReset();
+        resetHashes();
     }
 
     // clone this object
@@ -82,6 +84,12 @@
      */
     void implReset() {
         // Load magic initialization constants.
+        resetHashes();
+        // clear out old data
+        Arrays.fill(x, 0);
+    }
+
+    private void resetHashes() {
         state[0] = 0x67452301;
         state[1] = 0xefcdab89;
         state[2] = 0x98badcfe;
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/sun/security/provider/SHA.java
--- a/src/java.base/share/classes/sun/security/provider/SHA.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/sun/security/provider/SHA.java	Thu Aug 23 11:37:14 2018 +0100
@@ -25,6 +25,7 @@
 
 package sun.security.provider;
 
+import java.util.Arrays;
 import java.util.Objects;
 
 import static sun.security.provider.ByteArrayAccess.*;
@@ -62,7 +63,7 @@
         super("SHA-1", 20, 64);
         state = new int[5];
         W = new int[80];
-        implReset();
+        resetHashes();
     }
 
     /*
@@ -79,6 +80,13 @@
      * Resets the buffers and hash value to start a new hash.
      */
     void implReset() {
+        // Load magic initialization constants.
+        resetHashes();
+        // clear out old data
+        Arrays.fill(W, 0);
+    }
+
+    private void resetHashes() {
         state[0] = 0x67452301;
         state[1] = 0xefcdab89;
         state[2] = 0x98badcfe;
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/sun/security/provider/SHA2.java
--- a/src/java.base/share/classes/sun/security/provider/SHA2.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/sun/security/provider/SHA2.java	Thu Aug 23 11:37:14 2018 +0100
@@ -25,6 +25,7 @@
 
 package sun.security.provider;
 
+import java.util.Arrays;
 import java.util.Objects;
 
 import jdk.internal.HotSpotIntrinsicCandidate;
@@ -83,13 +84,18 @@
         this.initialHashes = initialHashes;
         state = new int[8];
         W = new int[64];
-        implReset();
+        resetHashes();
     }
 
     /**
      * Resets the buffers and hash value to start a new hash.
      */
     void implReset() {
+        resetHashes();
+        Arrays.fill(W, 0);
+    }
+
+    private void resetHashes() {
         System.arraycopy(initialHashes, 0, state, 0, state.length);
     }
 
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 src/java.base/share/classes/sun/security/provider/SHA5.java
--- a/src/java.base/share/classes/sun/security/provider/SHA5.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/src/java.base/share/classes/sun/security/provider/SHA5.java	Thu Aug 23 11:37:14 2018 +0100
@@ -25,6 +25,7 @@
 
 package sun.security.provider;
 
+import java.util.Arrays;
 import java.util.Objects;
 
 import jdk.internal.HotSpotIntrinsicCandidate;
@@ -98,10 +99,15 @@
         this.initialHashes = initialHashes;
         state = new long[8];
         W = new long[80];
-        implReset();
+        resetHashes();
     }
 
     final void implReset() {
+        resetHashes();
+        Arrays.fill(W, 0L);
+    }
+
+    private void resetHashes() {
         System.arraycopy(initialHashes, 0, state, 0, state.length);
     }
 
diff -r 0265a70ea2a5 -r c9a3e3cac9c7 test/jdk/com/sun/crypto/provider/Cipher/PBE/PKCS12Cipher.java
--- a/test/jdk/com/sun/crypto/provider/Cipher/PBE/PKCS12Cipher.java	Thu Aug 23 10:52:27 2018 +0200
+++ b/test/jdk/com/sun/crypto/provider/Cipher/PBE/PKCS12Cipher.java	Thu Aug 23 11:37:14 2018 +0100
@@ -106,7 +106,7 @@
         this.salt = salt;
         this.iCount = iCount;
     }
-    public char[] getPassword() { return passwd; }
+    public char[] getPassword() { return passwd.clone(); }
     public byte[] getSalt() { return salt; }
     public int getIterationCount() { return iCount; }
     public String getAlgorithm() { return "PBE"; }