8209129: Further improvements to cipher buffer management
Reviewed-by: weijun, igerasim
--- 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);
}
--- 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();
}
--- 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
--- 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;
--- 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);
--- 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;
}
--- 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");
--- 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');
}
}
--- 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.
--- 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;
--- 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;
--- 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;
--- 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);
}
--- 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);
}
--- 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"; }