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/21 15:46:34 UTC
[cxf] 03/03: More work on cleaning keys
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
commit fc89f35c1f80863d8500eef359ba6259fac49e44
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Fri Dec 21 15:46:12 2018 +0000
More work on cleaning keys
---
.../security/jose/jaxrs/JweWriterInterceptor.java | 8 ++++----
.../security/jose/jwe/AbstractJweDecryption.java | 15 ++++++++------
.../jose/jwe/AesGcmWrapKeyEncryptionAlgorithm.java | 3 +++
.../jose/jwe/DirectKeyDecryptionAlgorithm.java | 2 +-
.../rs/security/jose/jwe/JweEncryptionOutput.java | 10 +++++-----
.../cxf/rs/security/jose/jwe/JweJsonProducer.java | 4 ++--
.../rs/security/xml/AbstractXmlEncInHandler.java | 23 ++++++++++++++++++++--
7 files changed, 45 insertions(+), 20 deletions(-)
diff --git a/rt/rs/security/jose-parent/jose-jaxrs/src/main/java/org/apache/cxf/rs/security/jose/jaxrs/JweWriterInterceptor.java b/rt/rs/security/jose-parent/jose-jaxrs/src/main/java/org/apache/cxf/rs/security/jose/jaxrs/JweWriterInterceptor.java
index 68648f4..b51d4a3 100644
--- a/rt/rs/security/jose-parent/jose-jaxrs/src/main/java/org/apache/cxf/rs/security/jose/jaxrs/JweWriterInterceptor.java
+++ b/rt/rs/security/jose-parent/jose-jaxrs/src/main/java/org/apache/cxf/rs/security/jose/jaxrs/JweWriterInterceptor.java
@@ -85,7 +85,7 @@ public class JweWriterInterceptor implements WriterInterceptor {
try {
JweCompactBuilder.startJweContent(actualOs,
encryption.getHeaders(),
- encryption.getContentEncryptionKey(),
+ encryption.getEncryptedContentEncryptionKey(),
encryption.getIv());
} catch (IOException ex) {
LOG.warning("JWE encryption error");
@@ -138,11 +138,11 @@ public class JweWriterInterceptor implements WriterInterceptor {
protected void protectHttpHeadersIfNeeded(WriterInterceptorContext ctx, JweHeaders jweHeaders) {
if (protectHttpHeaders) {
- JoseJaxrsUtils.protectHttpHeaders(ctx.getHeaders(),
- jweHeaders,
+ JoseJaxrsUtils.protectHttpHeaders(ctx.getHeaders(),
+ jweHeaders,
protectedHttpHeaders);
}
-
+
}
public void setProtectHttpHeaders(boolean protectHttpHeaders) {
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 d383849..cd4faf9 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
@@ -19,6 +19,7 @@
package org.apache.cxf.rs.security.jose.jwe;
import java.security.spec.AlgorithmParameterSpec;
+import java.util.Arrays;
import java.util.logging.Logger;
import javax.crypto.SecretKey;
@@ -44,19 +45,17 @@ public abstract class AbstractJweDecryption implements JweDecryptionProvider {
this.contentDecryptionAlgo = contentDecryptionAlgo;
}
- protected byte[] getContentEncryptionKey(JweDecryptionInput jweDecryptionInput) {
- return keyDecryptionAlgo.getDecryptedContentEncryptionKey(jweDecryptionInput);
- }
-
public JweDecryptionOutput decrypt(String content) {
JweCompactConsumer consumer = new JweCompactConsumer(content);
- byte[] cek = getContentEncryptionKey(consumer.getJweDecryptionInput());
+ byte[] cek = keyDecryptionAlgo.getDecryptedContentEncryptionKey(consumer.getJweDecryptionInput());
return doDecrypt(consumer.getJweDecryptionInput(), cek);
}
+
public byte[] decrypt(JweDecryptionInput jweDecryptionInput) {
- byte[] cek = getContentEncryptionKey(jweDecryptionInput);
+ byte[] cek = keyDecryptionAlgo.getDecryptedContentEncryptionKey(jweDecryptionInput);
return doDecrypt(jweDecryptionInput, cek).getContent();
}
+
protected JweDecryptionOutput doDecrypt(JweDecryptionInput jweDecryptionInput, byte[] cek) {
KeyProperties keyProperties = new KeyProperties(getContentEncryptionAlgorithm(jweDecryptionInput));
keyProperties.setAdditionalData(getContentEncryptionCipherAAD(jweDecryptionInput));
@@ -77,6 +76,10 @@ public abstract class AbstractJweDecryption implements JweDecryptionProvider {
} catch (DestroyFailedException e) {
// ignore
}
+ Arrays.fill(cek, (byte) 0);
+ if (actualCek != cek) {
+ Arrays.fill(actualCek, (byte) 0);
+ }
return new JweDecryptionOutput(jweDecryptionInput.getJweHeaders(), bytes);
}
diff --git a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AesGcmWrapKeyEncryptionAlgorithm.java b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AesGcmWrapKeyEncryptionAlgorithm.java
index baa4e56..26f83cc 100644
--- a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AesGcmWrapKeyEncryptionAlgorithm.java
+++ b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/AesGcmWrapKeyEncryptionAlgorithm.java
@@ -54,6 +54,9 @@ public class AesGcmWrapKeyEncryptionAlgorithm extends AbstractWrapKeyEncryptionA
String encodedTag = Base64UrlUtility.encodeChunk(wrappedKeyAndTag,
wrappedKeyAndTag.length - 128 / 8, 128 / 8);
headers.setHeader("tag", encodedTag);
+
+ // Cleanup
+ Arrays.fill(wrappedKeyAndTag, (byte) 0);
return wrappedKey;
}
protected AlgorithmParameterSpec getAlgorithmParameterSpec(JweHeaders headers) {
diff --git a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/DirectKeyDecryptionAlgorithm.java b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/DirectKeyDecryptionAlgorithm.java
index 92bc469..a9159a0 100644
--- a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/DirectKeyDecryptionAlgorithm.java
+++ b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/DirectKeyDecryptionAlgorithm.java
@@ -40,7 +40,7 @@ public class DirectKeyDecryptionAlgorithm implements KeyDecryptionProvider {
@Override
public byte[] getDecryptedContentEncryptionKey(JweDecryptionInput jweDecryptionInput) {
validateKeyEncryptionKey(jweDecryptionInput);
- return contentDecryptionKey;
+ return contentDecryptionKey.clone();
}
@Override
public KeyAlgorithm getAlgorithm() {
diff --git a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweEncryptionOutput.java b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweEncryptionOutput.java
index 2275222..b28a416 100644
--- a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweEncryptionOutput.java
+++ b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweEncryptionOutput.java
@@ -25,7 +25,7 @@ import org.apache.cxf.rt.security.crypto.KeyProperties;
public class JweEncryptionOutput {
private Cipher cipher;
private JweHeaders headers;
- private byte[] contentEncryptionKey;
+ private byte[] encryptedContentEncryptionKey;
private byte[] iv;
private AuthenticationTagProducer authTagProducer;
private byte[] encryptedContent;
@@ -35,7 +35,7 @@ public class JweEncryptionOutput {
//CHECKSTYLE:OFF
public JweEncryptionOutput(Cipher cipher,
JweHeaders headers,
- byte[] contentEncryptionKey,
+ byte[] encryptedContentEncryptionKey,
byte[] iv,
AuthenticationTagProducer authTagProducer,
KeyProperties keyProps,
@@ -44,7 +44,7 @@ public class JweEncryptionOutput {
//CHECKSTYLE:ON
this.cipher = cipher;
this.headers = headers;
- this.contentEncryptionKey = contentEncryptionKey;
+ this.encryptedContentEncryptionKey = encryptedContentEncryptionKey;
this.iv = iv;
this.authTagProducer = authTagProducer;
this.keyProps = keyProps;
@@ -57,8 +57,8 @@ public class JweEncryptionOutput {
public JweHeaders getHeaders() {
return headers;
}
- public byte[] getContentEncryptionKey() {
- return contentEncryptionKey;
+ public byte[] getEncryptedContentEncryptionKey() {
+ return encryptedContentEncryptionKey;
}
public byte[] getIv() {
return iv;
diff --git a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweJsonProducer.java b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweJsonProducer.java
index 312717a..146529a 100644
--- a/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweJsonProducer.java
+++ b/rt/rs/security/jose-parent/jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweJsonProducer.java
@@ -140,8 +140,8 @@ public class JweJsonProducer {
iv = currentIv;
}
- byte[] encryptedCek = state.getContentEncryptionKey();
- if (encryptedCek.length == 0
+ byte[] encryptedCek = state.getEncryptedContentEncryptionKey();
+ if (encryptedCek.length == 0
&& encryptor.getKeyAlgorithm() != null
&& !KeyAlgorithm.DIRECT.equals(encryptor.getKeyAlgorithm())) {
LOG.warning("Unexpected key encryption algorithm");
diff --git a/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/AbstractXmlEncInHandler.java b/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/AbstractXmlEncInHandler.java
index e9ca746..68ee82d 100644
--- a/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/AbstractXmlEncInHandler.java
+++ b/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/xml/AbstractXmlEncInHandler.java
@@ -29,6 +29,7 @@ import java.util.Arrays;
import javax.crypto.Cipher;
import javax.crypto.SecretKey;
+import javax.security.auth.DestroyFailedException;
import javax.security.auth.callback.CallbackHandler;
import javax.xml.stream.XMLStreamReader;
@@ -248,7 +249,16 @@ public abstract class AbstractXmlEncInHandler extends AbstractXmlSecInHandler {
EncryptionUtils.initCipherWithKey(keyEncAlgo, digestAlgo, Cipher.DECRYPT_MODE, key);
try {
byte[] encryptedBytes = Base64Utility.decode(base64EncodedKey);
- return cipher.doFinal(encryptedBytes);
+ byte[] decryptedKey = cipher.doFinal(encryptedBytes);
+
+ // Clean the private key from memory now that we're finished with it
+ try {
+ key.destroy();
+ } catch (DestroyFailedException ex) {
+ // ignore
+ }
+
+ return decryptedKey;
} catch (Base64Exception ex) {
throwFault("Base64 decoding has failed", ex);
} catch (Exception ex) {
@@ -265,7 +275,16 @@ public abstract class AbstractXmlEncInHandler extends AbstractXmlSecInHandler {
try {
XMLCipher xmlCipher =
EncryptionUtils.initXMLCipher(symEncAlgo, XMLCipher.DECRYPT_MODE, key);
- return xmlCipher.decryptToByteArray(root);
+ byte[] decryptedContent = xmlCipher.decryptToByteArray(root);
+
+ // Clean the private key from memory now that we're finished with it
+ try {
+ key.destroy();
+ } catch (DestroyFailedException ex) {
+ // ignore
+ }
+
+ return decryptedContent;
} catch (XMLEncryptionException ex) {
throw new WSSecurityException(WSSecurityException.ErrorCode.UNSUPPORTED_ALGORITHM, ex);
}