Author: schultz
Date: Fri Nov 23 21:18:48 2018
New Revision: 1847318
URL: http://svn.apache.org/viewvc?rev=1847318&view=rev
Log:
Fix EncryptInterceptor to be thread-safe.
Add multi-threaded unit test.
Modified:
tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/EncryptInterceptor.java
tomcat/trunk/test/org/apache/catalina/tribes/group/interceptors/TestEncryptInterceptor.java
tomcat/trunk/webapps/docs/changelog.xml
Modified:
tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/EncryptInterceptor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/EncryptInterceptor.java?rev=1847318&r1=1847317&r2=1847318&view=diff
==============================================================================
---
tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/EncryptInterceptor.java
(original)
+++
tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/EncryptInterceptor.java
Fri Nov 23 21:18:48 2018
@@ -17,13 +17,10 @@
package org.apache.catalina.tribes.group.interceptors;
import java.security.GeneralSecurityException;
-import java.security.InvalidAlgorithmParameterException;
-import java.security.InvalidKeyException;
import java.security.SecureRandom;
+import java.util.concurrent.ConcurrentLinkedQueue;
-import javax.crypto.BadPaddingException;
import javax.crypto.Cipher;
-import javax.crypto.IllegalBlockSizeException;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;
@@ -61,10 +58,14 @@ public class EncryptInterceptor extends
private String encryptionAlgorithm = DEFAULT_ENCRYPTION_ALGORITHM;
private byte[] encryptionKeyBytes;
private String encryptionKeyString;
- private SecretKeySpec secretKey;
- private Cipher encryptionCipher;
- private Cipher decryptionCipher;
+ /**
+ * This is the name of the core encryption algorithm e.g. AES.
+ */
+ private String algorithmName;
+ private SecretKeySpec secretKey;
+ private ConcurrentLinkedQueue<Cipher> cipherPool;
+ private ConcurrentLinkedQueue<SecureRandom> randomPool;
public EncryptInterceptor() {
}
@@ -84,6 +85,17 @@ public class EncryptInterceptor extends
}
@Override
+ public void stop(int svc) throws ChannelException {
+ if(Channel.SND_TX_SEQ == (svc & Channel.SND_TX_SEQ)) {
+ // Individual Cipher and SecureRandom objects need no explicit
teardown
+ cipherPool.clear();
+ randomPool.clear();
+ }
+
+ super.stop(svc);
+ }
+
+ @Override
public void sendMessage(Member[] destination, ChannelMessage msg,
InterceptorPayload payload)
throws ChannelException {
try {
@@ -101,18 +113,9 @@ public class EncryptInterceptor extends
super.sendMessage(destination, msg, payload);
- } catch (IllegalBlockSizeException ibse) {
- log.error(sm.getString("encryptInterceptor.encrypt.failed"));
- throw new ChannelException(ibse);
- } catch (BadPaddingException bpe) {
- log.error(sm.getString("encryptInterceptor.encrypt.failed"));
- throw new ChannelException(bpe);
- } catch (InvalidKeyException ike) {
+ } catch (GeneralSecurityException gse) {
log.error(sm.getString("encryptInterceptor.encrypt.failed"));
- throw new ChannelException(ike);
- } catch (InvalidAlgorithmParameterException iape) {
- log.error(sm.getString("encryptInterceptor.encrypt.failed"));
- throw new ChannelException(iape);
+ throw new ChannelException(gse);
}
}
@@ -130,14 +133,8 @@ public class EncryptInterceptor extends
xbb.append(data, 0, data.length);
super.messageReceived(msg);
- } catch (IllegalBlockSizeException ibse) {
- log.error(sm.getString("encryptInterceptor.decrypt.failed"), ibse);
- } catch (BadPaddingException bpe) {
- log.error(sm.getString("encryptInterceptor.decrypt.failed"), bpe);
- } catch (InvalidKeyException ike) {
- log.error(sm.getString("encryptInterceptor.decrypt.failed"), ike);
- } catch (InvalidAlgorithmParameterException iape) {
- log.error(sm.getString("encryptInterceptor.decrypt.failed"), iape);
+ } catch (GeneralSecurityException gse) {
+ log.error(sm.getString("encryptInterceptor.decrypt.failed"), gse);
}
}
@@ -261,6 +258,22 @@ public class EncryptInterceptor extends
return providerName;
}
+ private void setSecretKey(SecretKeySpec secretKey) {
+ this.secretKey = secretKey;
+ }
+
+ private SecretKeySpec getSecretKey() {
+ return secretKey;
+ }
+
+ private void setAlgorithmName(String algorithm) {
+ algorithmName = algorithm;
+ }
+
+ private String getAlgorithmName() {
+ return algorithmName;
+ }
+
private void initCiphers() throws GeneralSecurityException {
if(null == getEncryptionKey())
throw new
IllegalStateException(sm.getString("encryptInterceptor.key.required"));
@@ -294,32 +307,45 @@ public class EncryptInterceptor extends
|| "CFB".equalsIgnoreCase(algorithmMode)))
throw new
IllegalArgumentException(sm.getString("encryptInterceptor.algorithm.unsupported-mode",
algorithmMode));
+ setAlgorithmName(algorithm);
setSecretKey(new SecretKeySpec(getEncryptionKeyInternal(),
algorithmName));
- String providerName = getProviderName();
- if(null == providerName) {
- encryptionCipher = Cipher.getInstance(algorithm);
- decryptionCipher = Cipher.getInstance(algorithm);
- } else {
- encryptionCipher = Cipher.getInstance(algorithm,
getProviderName());
- decryptionCipher = Cipher.getInstance(algorithm,
getProviderName());
- }
+ cipherPool = new ConcurrentLinkedQueue<>();
+ randomPool = new ConcurrentLinkedQueue<>();
}
- private void setSecretKey(SecretKeySpec secretKey) {
- this.secretKey = secretKey;
+ private Cipher getCipher() throws GeneralSecurityException {
+ Cipher cipher = cipherPool.poll();
+
+ if(null == cipher) {
+ String providerName = getProviderName();
+
+ if(null == providerName) {
+ return Cipher.getInstance(getAlgorithmName());
+ } else {
+ return Cipher.getInstance(getAlgorithmName(), providerName);
+ }
+ }
+
+ return cipher;
}
- private SecretKeySpec getSecretKey() {
- return secretKey;
+ private void returnCipher(Cipher cipher) {
+ cipherPool.offer(cipher);
}
- private Cipher getEncryptionCipher() {
- return encryptionCipher;
+ private SecureRandom getRandom() throws GeneralSecurityException {
+ SecureRandom random = randomPool.poll();
+
+ if(null == random) {
+ random = new SecureRandom();
+ }
+
+ return random;
}
- private Cipher getDecryptionCipher() {
- return decryptionCipher;
+ private void returnRandom(SecureRandom random) {
+ randomPool.offer(random);
}
/**
@@ -335,33 +361,39 @@ public class EncryptInterceptor extends
*
* @return The IV in [0] and the encrypted data in [1].
*
- * @throws IllegalBlockSizeException If the input data is not a multiple of
- * the block size and no padding has been requested (for block
- * ciphers) or if the input data cannot be encrypted
- * @throws BadPaddingException Declared but should not occur during
encryption
- * @throws InvalidAlgorithmParameterException If the algorithm is invalid
- * @throws InvalidKeyException If the key is invalid
+ * @throws GeneralSecurityException If the input data cannot be encrypted.
*/
- private byte[][] encrypt(byte[] bytes) throws IllegalBlockSizeException,
BadPaddingException, InvalidKeyException, InvalidAlgorithmParameterException {
- Cipher cipher = getEncryptionCipher();
-
- byte[] iv = new byte[cipher.getBlockSize()];
-
- // Always use a random IV For cipher setup.
- // The recipient doesn't need the (matching) IV because we will always
- // pre-pad messages with the IV as a nonce.
- new SecureRandom().nextBytes(iv);
+ private byte[][] encrypt(byte[] bytes) throws GeneralSecurityException {
+ Cipher cipher = null;
+ SecureRandom random = null;
- IvParameterSpec IV = new IvParameterSpec(iv);
-
- cipher.init(Cipher.ENCRYPT_MODE, getSecretKey(), IV);
+ try {
+ cipher = getCipher();
+ random = getRandom();
- // Prepend the IV to the beginning of the encrypted data
- byte[][] data = new byte[2][];
- data[0] = iv;
- data[1] = cipher.doFinal(bytes);
+ byte[] iv = new byte[cipher.getBlockSize()];
- return data;
+ // Always use a random IV For cipher setup.
+ // The recipient doesn't need the (matching) IV because we will
always
+ // pre-pad messages with the IV as a nonce.
+ random.nextBytes(iv);
+
+ IvParameterSpec IV = new IvParameterSpec(iv);
+
+ cipher.init(Cipher.ENCRYPT_MODE, getSecretKey(), IV);
+
+ // Prepend the IV to the beginning of the encrypted data
+ byte[][] data = new byte[2][];
+ data[0] = iv;
+ data[1] = cipher.doFinal(bytes);
+
+ return data;
+ } finally {
+ if(null != cipher)
+ returnCipher(cipher);
+ if(null != random)
+ returnRandom(random);
+ }
}
/**
@@ -371,23 +403,26 @@ public class EncryptInterceptor extends
*
* @return The decrypted data.
*
- * @throws IllegalBlockSizeException If the input data cannot be encrypted
- * @throws BadPaddingException If the decrypted data does not include the
- * expected number of padding bytes
- * @throws InvalidAlgorithmParameterException If the algorithm is invalid
- * @throws InvalidKeyException If the key is invalid
+ * @throws GeneralSecurityException If the input data cannot be decrypted.
*/
- private byte[] decrypt(byte[] bytes) throws IllegalBlockSizeException,
BadPaddingException, InvalidKeyException, InvalidAlgorithmParameterException {
- Cipher cipher = getDecryptionCipher();
+ private byte[] decrypt(byte[] bytes) throws GeneralSecurityException {
+ Cipher cipher = null;
- int blockSize = cipher.getBlockSize();
+ try {
+ cipher = getCipher();
- // Use first-block of incoming data as IV
- IvParameterSpec IV = new IvParameterSpec(bytes, 0, blockSize);
- cipher.init(Cipher.DECRYPT_MODE, getSecretKey(), IV);
+ int blockSize = cipher.getBlockSize();
- // Decrypt remainder of the message.
- return cipher.doFinal(bytes, blockSize, bytes.length - blockSize);
+ // Use first-block of incoming data as IV
+ IvParameterSpec IV = new IvParameterSpec(bytes, 0, blockSize);
+ cipher.init(Cipher.DECRYPT_MODE, getSecretKey(), IV);
+
+ // Decrypt remainder of the message.
+ return cipher.doFinal(bytes, blockSize, bytes.length - blockSize);
+ } finally {
+ if(null != cipher)
+ returnCipher(cipher);
+ }
}
Modified:
tomcat/trunk/test/org/apache/catalina/tribes/group/interceptors/TestEncryptInterceptor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/tribes/group/interceptors/TestEncryptInterceptor.java?rev=1847318&r1=1847317&r2=1847318&view=diff
==============================================================================
---
tomcat/trunk/test/org/apache/catalina/tribes/group/interceptors/TestEncryptInterceptor.java
(original)
+++
tomcat/trunk/test/org/apache/catalina/tribes/group/interceptors/TestEncryptInterceptor.java
Fri Nov 23 21:18:48 2018
@@ -19,6 +19,8 @@ package org.apache.catalina.tribes.group
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
+import java.util.ArrayList;
+import java.util.Collection;
import org.hamcrest.core.IsEqual;
import org.hamcrest.core.IsNot;
@@ -349,6 +351,60 @@ public class TestEncryptInterceptor {
}
/**
+ * This test isn't guaranteed to catch any multithreaded issues, but it
+ * gives a good exercise.
+ */
+ @Test
+ public void testMultithreaded() throws Exception {
+ String inputValue = "A test string to fight over.";
+ final byte[] bytes = inputValue.getBytes("UTF-8");
+ int numThreads = 100;
+ final int messagesPerThread = 10;
+
+ dest.setPrevious(new ValuesCaptureInterceptor());
+
+ src.start(Channel.SND_TX_SEQ);
+ dest.start(Channel.SND_TX_SEQ);
+
+ Runnable job = new Runnable() {
+ public void run() {
+ try {
+ ChannelData msg = new ChannelData(false);
+ XByteBuffer xbb = new XByteBuffer(1024, false);
+ xbb.append(bytes, 0, bytes.length);
+ msg.setMessage(xbb);
+
+ for(int i=0; i<messagesPerThread; ++i)
+ src.sendMessage(null, msg, null);
+ } catch (ChannelException e) {
+ Assert.fail("Encountered exception sending messages: " +
e.getMessage());
+ }
+ }
+ };
+
+ Thread[] threads = new Thread[numThreads];
+ for(int i=0; i<numThreads; ++i) {
+ threads[i] = new Thread(job);
+ threads[i].setName("Message-Thread-" + i);
+ }
+
+ for(int i=0; i<numThreads; ++i)
+ threads[i].start();
+
+ for(int i=0; i<numThreads; ++i)
+ threads[i].join();
+
+ // Check all received messages to make sure they are not corrupted
+ Collection<byte[]> messages =
((ValuesCaptureInterceptor)dest.getPrevious()).getValues();
+
+ Assert.assertEquals("Did not receive all expected messages",
+ numThreads * messagesPerThread, messages.size());
+
+ for(byte[] message : messages)
+ Assert.assertArrayEquals("Message is corrupted", message, bytes);
+ }
+
+ /**
* Interceptor that delivers directly to a destination.
*/
private static class PipedInterceptor
@@ -393,4 +449,33 @@ public class TestEncryptInterceptor {
return value;
}
}
+
+ /**
+ * Interceptor that simply captures all messages sent to or received by it.
+ */
+ private static class ValuesCaptureInterceptor
+ extends ChannelInterceptorBase
+ {
+ private ArrayList<byte[]> messages = new ArrayList<byte[]>();
+
+ @Override
+ public void sendMessage(Member[] destination, ChannelMessage msg,
InterceptorPayload payload)
+ throws ChannelException {
+ synchronized(messages) {
+ messages.add(msg.getMessage().getBytes());
+ }
+ }
+
+ @Override
+ public void messageReceived(ChannelMessage msg) {
+ synchronized(messages) {
+ messages.add(msg.getMessage().getBytes());
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ public Collection<byte[]> getValues() {
+ return (Collection<byte[]>)messages.clone();
+ }
+ }
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1847318&r1=1847317&r2=1847318&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Nov 23 21:18:48 2018
@@ -159,6 +159,10 @@
executor from the Catalina Service. If running independently, the
Channel will provide the executor. (remm)
</update>
+ <fix>
+ Make EncryptInterceptor thread-safe. This makes this interceptor
+ actually usable. (schultz/markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Other">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
