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);
         }

Reply via email to