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 <cohei...@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); }