8208583: Better management of internal KeyStore buffers
8207775: Better management of CipherCore buffers
Reviewed-by: weijun, ascarpino
diff --git a/jdk/src/share/classes/com/sun/crypto/provider/CipherCore.java b/jdk/src/share/classes/com/sun/crypto/provider/CipherCore.java
index 87f4772..81027aa 100644
--- a/jdk/src/share/classes/com/sun/crypto/provider/CipherCore.java
+++ b/jdk/src/share/classes/com/sun/crypto/provider/CipherCore.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2002, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2018, 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
@@ -673,7 +673,12 @@
if (len == output.length) {
return output;
} else {
- return Arrays.copyOf(output, len);
+ byte[] copy = Arrays.copyOf(output, len);
+ if (decrypting) {
+ // Zero out internal buffer which is no longer required
+ Arrays.fill(output, (byte) 0x00);
+ }
+ return copy;
}
} catch (ShortBufferException e) {
// should never happen
@@ -767,11 +772,14 @@
inputLen -= temp;
buffered = Math.addExact(buffered, temp);
}
- // process 'buffer'
+ // process 'buffer'. When finished we can null out 'buffer'
+ // Only necessary to null out if buffer holds data for encryption
if (decrypting) {
outLen = cipher.decrypt(buffer, 0, buffered, output, outputOffset);
} else {
outLen = cipher.encrypt(buffer, 0, buffered, output, outputOffset);
+ //encrypt mode. Zero out internal (input) buffer
+ Arrays.fill(buffer, (byte) 0x00);
}
outputOffset = Math.addExact(outputOffset, outLen);
buffered = 0;
@@ -846,7 +854,12 @@
output = new byte[getOutputSizeByOperation(inputLen, true)];
int len = doFinal(input, inputOffset, inputLen, output, 0);
if (len < output.length) {
- return Arrays.copyOf(output, len);
+ byte[] copy = Arrays.copyOf(output, len);
+ if (decrypting) {
+ // Zero out internal (ouput) array
+ Arrays.fill(output, (byte) 0x00);
+ }
+ return copy;
} else {
return output;
}
@@ -959,6 +972,11 @@
finalOffset = 0;
if (buffered != 0) {
System.arraycopy(buffer, 0, finalBuf, 0, buffered);
+ if (!decrypting) {
+ // done with input buffer. We should zero out the
+ // data if we're in encrypt mode.
+ Arrays.fill(buffer, (byte) 0x00);
+ }
}
if (inputLen != 0) {
System.arraycopy(input, inputOffset, finalBuf,
@@ -1005,6 +1023,8 @@
}
// copy the result into user-supplied output buffer
System.arraycopy(outWithPadding, 0, output, outputOffset, outLen);
+ // decrypt mode. Zero out output data that's not required
+ Arrays.fill(outWithPadding, (byte) 0x00);
} else { // encrypting
try {
outLen = finalNoPadding(finalBuf, finalOffset, output,
@@ -1012,6 +1032,10 @@
} finally {
// reset after doFinal() for GCM encryption
requireReinit = (cipherMode == GCM_MODE);
+ if (finalBuf != input) {
+ // done with internal finalBuf array. Copied to output
+ Arrays.fill(finalBuf, (byte) 0x00);
+ }
}
}
diff --git a/jdk/src/share/classes/com/sun/crypto/provider/KeyProtector.java b/jdk/src/share/classes/com/sun/crypto/provider/KeyProtector.java
index 7f9d0b2..09d12a7 100644
--- a/jdk/src/share/classes/com/sun/crypto/provider/KeyProtector.java
+++ b/jdk/src/share/classes/com/sun/crypto/provider/KeyProtector.java
@@ -37,12 +37,15 @@
import java.security.AlgorithmParameters;
import java.security.spec.InvalidParameterSpecException;
import java.security.spec.PKCS8EncodedKeySpec;
+import java.util.Arrays;
import javax.crypto.Cipher;
import javax.crypto.CipherSpi;
import javax.crypto.SecretKey;
import javax.crypto.SealedObject;
import javax.crypto.spec.*;
+import javax.security.auth.DestroyFailedException;
+
import sun.security.x509.AlgorithmId;
import sun.security.util.ObjectIdentifier;
@@ -103,15 +106,20 @@
// create PBE key from password
PBEKeySpec pbeKeySpec = new PBEKeySpec(this.password);
- SecretKey sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
- pbeKeySpec.clearPassword();
-
- // encrypt private key
+ SecretKey sKey = null;
PBEWithMD5AndTripleDESCipher cipher;
- cipher = new PBEWithMD5AndTripleDESCipher();
- cipher.engineInit(Cipher.ENCRYPT_MODE, sKey, pbeSpec, null);
+ try {
+ sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
+ // encrypt private key
+ cipher = new PBEWithMD5AndTripleDESCipher();
+ cipher.engineInit(Cipher.ENCRYPT_MODE, sKey, pbeSpec, null);
+ } finally {
+ pbeKeySpec.clearPassword();
+ if (sKey != null) sKey.destroy();
+ }
byte[] plain = key.getEncoded();
byte[] encrKey = cipher.engineDoFinal(plain, 0, plain.length);
+ Arrays.fill(plain, (byte) 0x00);
// wrap encrypted private key in EncryptedPrivateKeyInfo
// (as defined in PKCS#8)
@@ -131,8 +139,8 @@
Key recover(EncryptedPrivateKeyInfo encrInfo)
throws UnrecoverableKeyException, NoSuchAlgorithmException
{
- byte[] plain;
-
+ byte[] plain = null;
+ SecretKey sKey = null;
try {
String encrAlg = encrInfo.getAlgorithm().getOID().toString();
if (!encrAlg.equals(PBE_WITH_MD5_AND_DES3_CBC_OID)
@@ -160,8 +168,7 @@
// create PBE key from password
PBEKeySpec pbeKeySpec = new PBEKeySpec(this.password);
- SecretKey sKey =
- new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
+ sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
pbeKeySpec.clearPassword();
// decrypt private key
@@ -178,7 +185,6 @@
(new PrivateKeyInfo(plain).getAlgorithm().getOID()).getName();
KeyFactory kFac = KeyFactory.getInstance(oidName);
return kFac.generatePrivate(new PKCS8EncodedKeySpec(plain));
-
} catch (NoSuchAlgorithmException ex) {
// Note: this catch needed to be here because of the
// later catch of GeneralSecurityException
@@ -187,6 +193,15 @@
throw new UnrecoverableKeyException(ioe.getMessage());
} catch (GeneralSecurityException gse) {
throw new UnrecoverableKeyException(gse.getMessage());
+ } finally {
+ if (plain != null) Arrays.fill(plain, (byte) 0x00);
+ if (sKey != null) {
+ try {
+ sKey.destroy();
+ } catch (DestroyFailedException e) {
+ //shouldn't happen
+ }
+ }
}
}
@@ -262,7 +277,7 @@
// of <code>protectedKey</code>. If the two digest values are
// different, throw an exception.
md.update(passwdBytes);
- java.util.Arrays.fill(passwdBytes, (byte)0x00);
+ Arrays.fill(passwdBytes, (byte)0x00);
passwdBytes = null;
md.update(plainKey);
digest = md.digest();
@@ -291,17 +306,21 @@
// create PBE key from password
PBEKeySpec pbeKeySpec = new PBEKeySpec(this.password);
- SecretKey sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
- pbeKeySpec.clearPassword();
-
- // seal key
+ SecretKey sKey = null;
Cipher cipher;
+ try {
+ sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
+ pbeKeySpec.clearPassword();
- PBEWithMD5AndTripleDESCipher cipherSpi;
- cipherSpi = new PBEWithMD5AndTripleDESCipher();
- cipher = new CipherForKeyProtector(cipherSpi, SunJCE.getInstance(),
+ // seal key
+ PBEWithMD5AndTripleDESCipher cipherSpi;
+ cipherSpi = new PBEWithMD5AndTripleDESCipher();
+ cipher = new CipherForKeyProtector(cipherSpi, SunJCE.getInstance(),
"PBEWithMD5AndTripleDES");
- cipher.init(Cipher.ENCRYPT_MODE, sKey, pbeSpec);
+ cipher.init(Cipher.ENCRYPT_MODE, sKey, pbeSpec);
+ } finally {
+ if (sKey != null) sKey.destroy();
+ }
return new SealedObjectForKeyProtector(key, cipher);
}
@@ -309,12 +328,12 @@
* Unseals the sealed key.
*/
Key unseal(SealedObject so)
- throws NoSuchAlgorithmException, UnrecoverableKeyException
- {
+ throws NoSuchAlgorithmException, UnrecoverableKeyException {
+ SecretKey sKey = null;
try {
// create PBE key from password
PBEKeySpec pbeKeySpec = new PBEKeySpec(this.password);
- SecretKey skey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
+ sKey = new PBEKey(pbeKeySpec, "PBEWithMD5AndTripleDES");
pbeKeySpec.clearPassword();
SealedObjectForKeyProtector soForKeyProtector = null;
@@ -342,7 +361,7 @@
Cipher cipher = new CipherForKeyProtector(cipherSpi,
SunJCE.getInstance(),
"PBEWithMD5AndTripleDES");
- cipher.init(Cipher.DECRYPT_MODE, skey, params);
+ cipher.init(Cipher.DECRYPT_MODE, sKey, params);
return soForKeyProtector.getKey(cipher);
} catch (NoSuchAlgorithmException ex) {
// Note: this catch needed to be here because of the
@@ -354,6 +373,14 @@
throw new UnrecoverableKeyException(cnfe.getMessage());
} catch (GeneralSecurityException gse) {
throw new UnrecoverableKeyException(gse.getMessage());
+ } finally {
+ if (sKey != null) {
+ try {
+ sKey.destroy();
+ } catch (DestroyFailedException e) {
+ //shouldn't happen
+ }
+ }
}
}
}
diff --git a/jdk/src/share/classes/com/sun/crypto/provider/PBEKey.java b/jdk/src/share/classes/com/sun/crypto/provider/PBEKey.java
index b063eec..103ff96 100644
--- a/jdk/src/share/classes/com/sun/crypto/provider/PBEKey.java
+++ b/jdk/src/share/classes/com/sun/crypto/provider/PBEKey.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -28,6 +28,7 @@
import java.security.MessageDigest;
import java.security.KeyRep;
import java.security.spec.InvalidKeySpecException;
+import java.util.Arrays;
import java.util.Locale;
import javax.crypto.SecretKey;
import javax.crypto.spec.PBEKeySpec;
@@ -68,7 +69,7 @@
this.key = new byte[passwd.length];
for (int i=0; i<passwd.length; i++)
this.key[i] = (byte) (passwd[i] & 0x7f);
- java.util.Arrays.fill(passwd, ' ');
+ Arrays.fill(passwd, ' ');
type = keytype;
}
@@ -110,11 +111,23 @@
byte[] thatEncoded = that.getEncoded();
boolean ret = MessageDigest.isEqual(this.key, thatEncoded);
- java.util.Arrays.fill(thatEncoded, (byte)0x00);
+ Arrays.fill(thatEncoded, (byte)0x00);
return ret;
}
/**
+ * Clears the internal copy of the key.
+ *
+ */
+ @Override
+ public void destroy() {
+ if (key != null) {
+ Arrays.fill(key, (byte) 0x00);
+ key = null;
+ }
+ }
+
+ /**
* readObject is called to restore the state of this key from
* a stream.
*/
diff --git a/jdk/src/share/classes/javax/crypto/spec/PBEKeySpec.java b/jdk/src/share/classes/javax/crypto/spec/PBEKeySpec.java
index 239231d..8f8d141 100644
--- a/jdk/src/share/classes/javax/crypto/spec/PBEKeySpec.java
+++ b/jdk/src/share/classes/javax/crypto/spec/PBEKeySpec.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -26,6 +26,7 @@
package javax.crypto.spec;
import java.security.spec.KeySpec;
+import java.util.Arrays;
/**
* A user-chosen password that can be used with password-based encryption
@@ -174,9 +175,7 @@
*/
public final void clearPassword() {
if (password != null) {
- for (int i = 0; i < password.length; i++) {
- password[i] = ' ';
- }
+ Arrays.fill(password, ' ');
password = null;
}
}
diff --git a/jdk/src/share/classes/sun/security/provider/JavaKeyStore.java b/jdk/src/share/classes/sun/security/provider/JavaKeyStore.java
index 6145666..409af47 100644
--- a/jdk/src/share/classes/sun/security/provider/JavaKeyStore.java
+++ b/jdk/src/share/classes/sun/security/provider/JavaKeyStore.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -132,18 +132,20 @@
throw new UnrecoverableKeyException("Password must not be null");
}
- KeyProtector keyProtector = new KeyProtector(password);
+ byte[] passwordBytes = convertToBytes(password);
+ KeyProtector keyProtector = new KeyProtector(passwordBytes);
byte[] encrBytes = ((KeyEntry)entry).protectedPrivKey;
EncryptedPrivateKeyInfo encrInfo;
- byte[] plain;
try {
encrInfo = new EncryptedPrivateKeyInfo(encrBytes);
+ return keyProtector.recover(encrInfo);
} catch (IOException ioe) {
throw new UnrecoverableKeyException("Private key not stored as "
+ "PKCS #8 "
+ "EncryptedPrivateKeyInfo");
+ } finally {
+ Arrays.fill(passwordBytes, (byte) 0x00);
}
- return keyProtector.recover(encrInfo);
}
/**
@@ -252,7 +254,8 @@
Certificate[] chain)
throws KeyStoreException
{
- KeyProtector keyProtector = null;
+ KeyProtector keyProtector;
+ byte[] passwordBytes = null;
if (!(key instanceof java.security.PrivateKey)) {
throw new KeyStoreException("Cannot store non-PrivateKeys");
@@ -263,7 +266,8 @@
entry.date = new Date();
// Protect the encoding of the key
- keyProtector = new KeyProtector(password);
+ passwordBytes = convertToBytes(password);
+ keyProtector = new KeyProtector(passwordBytes);
entry.protectedPrivKey = keyProtector.protect(key);
// clone the chain
@@ -279,7 +283,8 @@
} catch (NoSuchAlgorithmException nsae) {
throw new KeyStoreException("Key protection algorithm not found");
} finally {
- keyProtector = null;
+ if (passwordBytes != null)
+ Arrays.fill(passwordBytes, (byte) 0x00);
}
}
@@ -793,18 +798,26 @@
private MessageDigest getPreKeyedHash(char[] password)
throws NoSuchAlgorithmException, UnsupportedEncodingException
{
- int i, j;
MessageDigest md = MessageDigest.getInstance("SHA");
+ byte[] passwdBytes = convertToBytes(password);
+ md.update(passwdBytes);
+ Arrays.fill(passwdBytes, (byte) 0x00);
+ md.update("Mighty Aphrodite".getBytes("UTF8"));
+ return md;
+ }
+
+ /**
+ * Helper method to convert char[] to byte[]
+ */
+
+ private byte[] convertToBytes(char[] password) {
+ int i, j;
byte[] passwdBytes = new byte[password.length * 2];
for (i=0, j=0; i<password.length; i++) {
passwdBytes[j++] = (byte)(password[i] >> 8);
passwdBytes[j++] = (byte)password[i];
}
- md.update(passwdBytes);
- for (i=0; i<passwdBytes.length; i++)
- passwdBytes[i] = 0;
- md.update("Mighty Aphrodite".getBytes("UTF8"));
- return md;
+ return passwdBytes;
}
}
diff --git a/jdk/src/share/classes/sun/security/provider/KeyProtector.java b/jdk/src/share/classes/sun/security/provider/KeyProtector.java
index ef7a1f4..b0d798a 100644
--- a/jdk/src/share/classes/sun/security/provider/KeyProtector.java
+++ b/jdk/src/share/classes/sun/security/provider/KeyProtector.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -119,28 +119,15 @@
/**
* Creates an instance of this class, and initializes it with the given
* password.
- *
- * <p>The password is expected to be in printable ASCII.
- * Normal rules for good password selection apply: at least
- * seven characters, mixed case, with punctuation encouraged.
- * Phrases or words which are easily guessed, for example by
- * being found in dictionaries, are bad.
*/
- public KeyProtector(char[] password)
+ public KeyProtector(byte[] passwordBytes)
throws NoSuchAlgorithmException
{
- int i, j;
-
- if (password == null) {
+ if (passwordBytes == null) {
throw new IllegalArgumentException("password can't be null");
}
md = MessageDigest.getInstance(DIGEST_ALG);
- // Convert password to byte array, so that it can be digested
- passwdBytes = new byte[password.length * 2];
- for (i=0, j=0; i<password.length; i++) {
- passwdBytes[j++] = (byte)(password[i] >> 8);
- passwdBytes[j++] = (byte)password[i];
- }
+ this.passwdBytes = passwordBytes;
}
/**
