You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by co...@apache.org on 2018/12/19 13:36:10 UTC
[cxf] branch master updated: Clearing up more keys after they're
used
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/master by this push:
new 731d72b Clearing up more keys after they're used
731d72b is described below
commit 731d72b5fc39bd1da6245102ae9e1105325c3612
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Wed Dec 19 13:35:52 2018 +0000
Clearing up more keys after they're used
---
.../java/org/apache/cxf/io/CachedOutputStream.java | 3 +++
core/src/main/java/org/apache/cxf/io/CachedWriter.java | 3 +++
core/src/main/java/org/apache/cxf/io/CipherPair.java | 18 ++++++++++++++++--
.../jose/jwe/AbstractContentEncryptionAlgorithm.java | 16 +++++++++++++---
.../rs/security/jose/jwe/AbstractJweDecryption.java | 14 ++++++++++++--
.../rs/security/jose/jwe/AbstractJweEncryption.java | 12 +++++++++++-
.../org/apache/cxf/rt/security/crypto/CryptoUtils.java | 7 +------
.../org/apache/cxf/rt/security/crypto/HmacUtils.java | 18 ++++++++++++++++--
8 files changed, 75 insertions(+), 16 deletions(-)
diff --git a/core/src/main/java/org/apache/cxf/io/CachedOutputStream.java b/core/src/main/java/org/apache/cxf/io/CachedOutputStream.java
index d4a7bf0..e396937 100644
--- a/core/src/main/java/org/apache/cxf/io/CachedOutputStream.java
+++ b/core/src/main/java/org/apache/cxf/io/CachedOutputStream.java
@@ -221,6 +221,9 @@ public class CachedOutputStream extends OutputStream {
}
doClose();
currentStream.close();
+ if (ciphers != null) {
+ ciphers.clean();
+ }
if (!maybeDeleteTempFile(currentStream)) {
postClose();
}
diff --git a/core/src/main/java/org/apache/cxf/io/CachedWriter.java b/core/src/main/java/org/apache/cxf/io/CachedWriter.java
index 019f6da..3223f4d 100644
--- a/core/src/main/java/org/apache/cxf/io/CachedWriter.java
+++ b/core/src/main/java/org/apache/cxf/io/CachedWriter.java
@@ -245,6 +245,9 @@ public class CachedWriter extends Writer {
doClose();
currentStream.close();
maybeDeleteTempFile(currentStream);
+ if (ciphers != null) {
+ ciphers.clean();
+ }
postClose();
}
diff --git a/core/src/main/java/org/apache/cxf/io/CipherPair.java b/core/src/main/java/org/apache/cxf/io/CipherPair.java
index 525c1fe..bb6cf6a 100644
--- a/core/src/main/java/org/apache/cxf/io/CipherPair.java
+++ b/core/src/main/java/org/apache/cxf/io/CipherPair.java
@@ -20,12 +20,14 @@
package org.apache.cxf.io;
import java.security.GeneralSecurityException;
-import java.security.Key;
import java.security.SecureRandom;
+import java.util.Arrays;
import javax.crypto.Cipher;
import javax.crypto.KeyGenerator;
+import javax.crypto.SecretKey;
import javax.crypto.spec.IvParameterSpec;
+import javax.security.auth.DestroyFailedException;
/**
* A class to hold a pair of encryption and decryption ciphers.
@@ -33,7 +35,7 @@ import javax.crypto.spec.IvParameterSpec;
public class CipherPair {
private String transformation;
private Cipher enccipher;
- private Key key;
+ private SecretKey key;
private byte[] ivp;
public CipherPair(String transformation) throws GeneralSecurityException {
@@ -77,4 +79,16 @@ public class CipherPair {
}
return deccipher;
}
+
+ public void clean() {
+ if (ivp != null) {
+ Arrays.fill(ivp, (byte) 0);
+ }
+ // Clean the key after we're done with it
+ try {
+ key.destroy();
+ } catch (DestroyFailedException e) {
+ // ignore
+ }
+ }
}
diff --git a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractContentEncryptionAlgorithm.java b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractContentEncryptionAlgorithm.java
index 11a76f3..a0ad9fa 100644
--- a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractContentEncryptionAlgorithm.java
+++ b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractContentEncryptionAlgorithm.java
@@ -20,6 +20,9 @@ package org.apache.cxf.rs.security.jose.jwe;
import java.util.concurrent.atomic.AtomicInteger;
+import javax.crypto.SecretKey;
+import javax.security.auth.DestroyFailedException;
+
import org.apache.cxf.rs.security.jose.jwa.AlgorithmUtils;
import org.apache.cxf.rs.security.jose.jwa.ContentAlgorithm;
import org.apache.cxf.rt.security.crypto.CryptoUtils;
@@ -32,7 +35,7 @@ public abstract class AbstractContentEncryptionAlgorithm extends AbstractContent
private byte[] iv;
private AtomicInteger providedIvUsageCount;
private boolean generateCekOnce;
-
+
protected AbstractContentEncryptionAlgorithm(ContentAlgorithm algo, boolean generateCekOnce) {
super(algo);
this.generateCekOnce = generateCekOnce;
@@ -50,13 +53,20 @@ public abstract class AbstractContentEncryptionAlgorithm extends AbstractContent
byte[] theCek = null;
if (cek == null) {
String algoJava = getAlgorithm().getJavaName();
- theCek = CryptoUtils.getSecretKey(AlgorithmUtils.stripAlgoProperties(algoJava),
- getContentEncryptionKeySize(headers)).getEncoded();
+ SecretKey secretKey = CryptoUtils.getSecretKey(AlgorithmUtils.stripAlgoProperties(algoJava),
+ getContentEncryptionKeySize(headers));
+ theCek = secretKey.getEncoded();
if (generateCekOnce) {
synchronized (this) {
cek = theCek;
}
}
+ // Clean the key after we're done with it
+ try {
+ secretKey.destroy();
+ } catch (DestroyFailedException e) {
+ // ignore
+ }
} else {
theCek = cek;
}
diff --git a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractJweDecryption.java b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractJweDecryption.java
index 7d34fad..d383849 100644
--- a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractJweDecryption.java
+++ b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractJweDecryption.java
@@ -18,10 +18,12 @@
*/
package org.apache.cxf.rs.security.jose.jwe;
-import java.security.Key;
import java.security.spec.AlgorithmParameterSpec;
import java.util.logging.Logger;
+import javax.crypto.SecretKey;
+import javax.security.auth.DestroyFailedException;
+
import org.apache.cxf.common.logging.LogUtils;
import org.apache.cxf.rs.security.jose.common.JoseConstants;
import org.apache.cxf.rs.security.jose.jwa.AlgorithmUtils;
@@ -65,9 +67,17 @@ public abstract class AbstractJweDecryption implements JweDecryptionProvider {
keyProperties.setCompressionSupported(compressionSupported);
byte[] actualCek = getActualCek(cek,
jweDecryptionInput.getJweHeaders().getContentEncryptionAlgorithm().getJwaName());
- Key secretKey = CryptoUtils.createSecretKeySpec(actualCek, keyProperties.getKeyAlgo());
+ SecretKey secretKey = CryptoUtils.createSecretKeySpec(actualCek, keyProperties.getKeyAlgo());
byte[] bytes =
CryptoUtils.decryptBytes(getEncryptedContentWithAuthTag(jweDecryptionInput), secretKey, keyProperties);
+
+ // Here we're finished with the SecretKey we created, so we can destroy it
+ try {
+ secretKey.destroy();
+ } catch (DestroyFailedException e) {
+ // ignore
+ }
+
return new JweDecryptionOutput(jweDecryptionInput.getJweHeaders(), bytes);
}
protected byte[] getEncryptedContentEncryptionKey(JweCompactConsumer consumer) {
diff --git a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractJweEncryption.java b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractJweEncryption.java
index 361d2a7..da15f7d 100644
--- a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractJweEncryption.java
+++ b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AbstractJweEncryption.java
@@ -25,6 +25,7 @@ import java.util.logging.Logger;
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
+import javax.security.auth.DestroyFailedException;
import org.apache.cxf.common.logging.LogUtils;
import org.apache.cxf.jaxrs.json.basic.JsonMapObjectReaderWriter;
@@ -114,7 +115,16 @@ public abstract class AbstractJweEncryption implements JweEncryptionProvider {
}
protected byte[] encryptInternal(JweEncryptionInternal state, byte[] content) {
try {
- return CryptoUtils.encryptBytes(content, createCekSecretKey(state), state.keyProps);
+ SecretKey createCekSecretKey = createCekSecretKey(state);
+ byte[] encryptedBytes = CryptoUtils.encryptBytes(content, createCekSecretKey, state.keyProps);
+
+ // Here we're finished with the SecretKey we created, so we can destroy it
+ try {
+ createCekSecretKey.destroy();
+ } catch (DestroyFailedException e) {
+ // ignore
+ }
+ return encryptedBytes;
} catch (SecurityException ex) {
LOG.fine(ex.getMessage());
if (ex.getCause() instanceof NoSuchAlgorithmException) {
diff --git a/rt/security/src/main/java/org/apache/cxf/rt/security/crypto/CryptoUtils.java b/rt/security/src/main/java/org/apache/cxf/rt/security/crypto/CryptoUtils.java
index a5edd62..ce32031 100644
--- a/rt/security/src/main/java/org/apache/cxf/rt/security/crypto/CryptoUtils.java
+++ b/rt/security/src/main/java/org/apache/cxf/rt/security/crypto/CryptoUtils.java
@@ -49,8 +49,6 @@ import java.security.spec.ECPublicKeySpec;
import java.security.spec.RSAPrivateCrtKeySpec;
import java.security.spec.RSAPrivateKeySpec;
import java.security.spec.RSAPublicKeySpec;
-import java.util.logging.Level;
-import java.util.logging.Logger;
import javax.crypto.Cipher;
import javax.crypto.KeyGenerator;
@@ -61,7 +59,6 @@ import javax.crypto.spec.SecretKeySpec;
import javax.security.auth.DestroyFailedException;
import org.apache.cxf.common.classloader.ClassLoaderUtils;
-import org.apache.cxf.common.logging.LogUtils;
import org.apache.cxf.common.util.Base64UrlUtility;
import org.apache.cxf.common.util.Base64Utility;
import org.apache.cxf.common.util.CompressionUtils;
@@ -74,8 +71,6 @@ import org.apache.cxf.helpers.JavaUtils;
*/
public final class CryptoUtils {
- private static final Logger LOG = LogUtils.getL7dLogger(CryptoUtils.class);
-
private CryptoUtils() {
}
@@ -500,7 +495,7 @@ public final class CryptoUtils {
try {
secretKey.destroy();
} catch (DestroyFailedException e) {
- LOG.log(Level.FINE, "Error destroying key: {}", e.getMessage());
+ // ignore
}
return encryptedKey;
}
diff --git a/rt/security/src/main/java/org/apache/cxf/rt/security/crypto/HmacUtils.java b/rt/security/src/main/java/org/apache/cxf/rt/security/crypto/HmacUtils.java
index 43fc3eb..8c4317e 100644
--- a/rt/security/src/main/java/org/apache/cxf/rt/security/crypto/HmacUtils.java
+++ b/rt/security/src/main/java/org/apache/cxf/rt/security/crypto/HmacUtils.java
@@ -26,11 +26,13 @@ import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
import java.security.Provider;
import java.security.spec.AlgorithmParameterSpec;
+import java.util.Arrays;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.crypto.KeyGenerator;
import javax.crypto.Mac;
+import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;
import javax.security.auth.DestroyFailedException;
@@ -110,7 +112,7 @@ public final class HmacUtils {
try {
secretKey.destroy();
} catch (DestroyFailedException e) {
- LOG.log(Level.FINE, "Error destroying key: {}", e.getMessage());
+ // ignore
}
return digest;
}
@@ -149,7 +151,19 @@ public final class HmacUtils {
public static String generateKey(String algo) {
try {
KeyGenerator keyGen = KeyGenerator.getInstance(algo);
- return Base64Utility.encode(keyGen.generateKey().getEncoded());
+ SecretKey secretKey = keyGen.generateKey();
+ byte[] encodedSecretKey = secretKey.getEncoded();
+ String encodedKey = Base64Utility.encode(encodedSecretKey);
+
+ // Clean the key after we're done with it
+ Arrays.fill(encodedSecretKey, (byte) 0);
+ try {
+ secretKey.destroy();
+ } catch (DestroyFailedException e) {
+ // ignore
+ }
+
+ return encodedKey;
} catch (NoSuchAlgorithmException e) {
throw new SecurityException(e);
}