-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 All,
With this last patch, I'm ready for a back-port to tc8.5.x, but I'm waiting for a user who is trying to get this working on tc9.0 to be successful. If anyone else can confirm that this is all working in a real cluster (dev/test is okay) then I'll go ahead and back-port, assuming there is some kind of configuration error in that particular user's case. - -chris On 11/21/18 10:15, schu...@apache.org wrote: > Author: schultz Date: Wed Nov 21 15:15:34 2018 New Revision: > 1847118 > > URL: http://svn.apache.org/viewvc?rev=1847118&view=rev Log: Use > random IVs for encryption. Support cipher block modes other than > CBC. Expand unit tests. > > > Modified: > tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/Encryp tInterceptor.java > > tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/LocalStr ings.properties > tomcat/trunk/test/org/apache/catalina/tribes/group/interceptors/TestEn cryptInterceptor.java > > Modified: > tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/Encryp tInterceptor.java > > URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/tribe s/group/interceptors/EncryptInterceptor.java?rev=1847118&r1=1847117&r2=1 847118&view=diff > ====================================================================== ======== > > - --- tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/EncryptI nterceptor.java (original) > +++ > tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/Encryp tInterceptor.java > Wed Nov 21 15:15:34 2018 @@ -17,6 +17,8 @@ package > org.apache.catalina.tribes.group.interceptors; > > import java.security.GeneralSecurityException; +import > java.security.InvalidAlgorithmParameterException; +import > java.security.InvalidKeyException; import > java.security.SecureRandom; > > import javax.crypto.BadPaddingException; @@ -59,6 +61,7 @@ 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; > @@ -92,7 +95,7 @@ public class EncryptInterceptor extends > XByteBuffer xbb = msg.getMessage(); > > // Completely replace the message - xbb.setLength(0); + > xbb.clear(); xbb.append(bytes[0], 0, bytes[0].length); > xbb.append(bytes[1], 0, bytes[1].length); > > @@ -104,6 +107,12 @@ public class EncryptInterceptor extends } > catch (BadPaddingException bpe) { > log.error(sm.getString("encryptInterceptor.encrypt.failed")); throw > new ChannelException(bpe); + } catch (InvalidKeyException > ike) { + > 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); } } > > @@ -114,25 +123,21 @@ public class EncryptInterceptor extends > > data = decrypt(data); > > - // Remove the decrypted IV/nonce block from the front > of the message - int blockSize = > getDecryptionCipher().getBlockSize(); - int trimmedSize > = data.length - blockSize; - if(trimmedSize < 0) { - > log.error(sm.getString("encryptInterceptor.decrypt.error.short-message ")); > > - - throw new IllegalStateException(sm.getString("encryptInterceptor.decrypt.error.sho rt-message")); > - } - XByteBuffer xbb = msg.getMessage(); > > // Completely replace the message with the decrypted one - > xbb.setLength(0); - xbb.append(data, blockSize, > data.length - blockSize); + xbb.clear(); + > 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); } } > > @@ -262,55 +267,51 @@ public class EncryptInterceptor extends > > String algorithm = getEncryptionAlgorithm(); > > - String mode = getAlgorithmMode(algorithm); - - > if(!"CBC".equalsIgnoreCase(mode)) - throw new > IllegalArgumentException(sm.getString("encryptInterceptor.algorithm.re quires-cbc-mode", > mode)); - - Cipher cipher; - - String providerName = > getProviderName(); - if(null == providerName) { - > cipher = Cipher.getInstance(algorithm); - } else { - > cipher = Cipher.getInstance(algorithm, getProviderName()); - > } - - byte[] iv = new byte[cipher.getBlockSize()]; + > String algorithmName; + String algorithmMode; > > - // 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); - - IvParameterSpec IV = > new IvParameterSpec(iv); - - // If this is a cipher > transform of the form ALGO/MODE/PAD, + // We need to > break-apart the algorithm name e.g. AES/CBC/PKCS5Padding // take > just the algorithm part. int pos = algorithm.indexOf('/'); > > - String bareAlgorithm; if(pos >= 0) { - > bareAlgorithm = algorithm.substring(0, pos); + > algorithmName = algorithm.substring(0, pos); + int pos2 > = algorithm.indexOf('/', pos+1); + + if(pos2 >= 0) { + > algorithmMode = algorithm.substring(pos + 1, pos2); + } > else { + algorithmMode = "CBC"; + } } > else { - bareAlgorithm = algorithm; + > algorithmName = algorithm; + algorithmMode = "CBC"; } > > - SecretKeySpec encryptionKey = new > SecretKeySpec(getEncryptionKey(), bareAlgorithm); + // Note: > ECB is not an appropriate mode for secure communications. + > if(!("CBC".equalsIgnoreCase(algorithmMode) + || > "OFB".equalsIgnoreCase(algorithmMode) + || > "CFB".equalsIgnoreCase(algorithmMode))) + throw new > IllegalArgumentException(sm.getString("encryptInterceptor.algorithm.un supported-mode", > algorithmMode)); > > - cipher.init(Cipher.ENCRYPT_MODE, encryptionKey, IV); - - > encryptionCipher = cipher; + setSecretKey(new > SecretKeySpec(getEncryptionKeyInternal(), algorithmName)); > > + String providerName = getProviderName(); if(null == > providerName) { - cipher = > Cipher.getInstance(algorithm); + encryptionCipher = > Cipher.getInstance(algorithm); + decryptionCipher = > Cipher.getInstance(algorithm); } else { - cipher = > Cipher.getInstance(algorithm, getProviderName()); + > encryptionCipher = Cipher.getInstance(algorithm, > getProviderName()); + decryptionCipher = > Cipher.getInstance(algorithm, getProviderName()); } + } > > - cipher.init(Cipher.DECRYPT_MODE, encryptionKey, new > IvParameterSpec(iv)); + private void setSecretKey(SecretKeySpec > secretKey) { + this.secretKey = secretKey; + } > > - decryptionCipher = cipher; + private SecretKeySpec > getSecretKey() { + return secretKey; } > > private Cipher getEncryptionCipher() { @@ -321,20 +322,9 @@ public > class EncryptInterceptor extends return decryptionCipher; } > > - private static String getAlgorithmMode(String algorithm) { - > int start = algorithm.indexOf('/'); - if(start < 0) - > throw new > IllegalArgumentException(sm.getString("encryptInterceptor.algorithm.re quired")); > > - - int end = algorithm.indexOf('/', start + 1); > - if(end < 0) - throw new > IllegalArgumentException(sm.getString("encryptInterceptor.algorithm.re quired")); > > - - > - return algorithm.substring(start + 1, end); - } - /** * > Encrypts the input <code>bytes</code> into two separate byte > arrays: - * one for the initial block (which will be the > encrypted random IV) + * one for the random initialization > vector (IV) used for this message, * and the second one containing > the actual encrypted payload. * * This method returns a pair of > byte arrays instead of a single @@ -343,21 +333,32 @@ public class > EncryptInterceptor extends * * @param bytes The data to encrypt. * > - * @return The encrypted IV block in [0] and the encrypted > data in [1]. + * @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 */ - private > byte[][] encrypt(byte[] bytes) throws IllegalBlockSizeException, > BadPaddingException { + private byte[][] encrypt(byte[] bytes) > throws IllegalBlockSizeException, BadPaddingException, > InvalidKeyException, InvalidAlgorithmParameterException { Cipher > cipher = getEncryptionCipher(); > > - // Adding the IV to the beginning of the encrypted data - > byte[] iv = cipher.getIV(); + 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); > + + 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] = cipher.update(iv, 0, > iv.length); + data[0] = iv; data[1] = > cipher.doFinal(bytes); > > return data; @@ -373,10 +374,20 @@ public class EncryptInterceptor > extends * @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 */ - private byte[] decrypt(byte[] bytes) throws > IllegalBlockSizeException, BadPaddingException { - return > getDecryptionCipher().doFinal(bytes); + private byte[] > decrypt(byte[] bytes) throws IllegalBlockSizeException, > BadPaddingException, InvalidKeyException, > InvalidAlgorithmParameterException { + Cipher cipher = > getDecryptionCipher(); + + int blockSize = > cipher.getBlockSize(); + + // 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); } > > > > Modified: > tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/LocalS trings.properties > > URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/tribe s/group/interceptors/LocalStrings.properties?rev=1847118&r1=1847117&r2=1 847118&view=diff > ====================================================================== ======== > > - --- tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/LocalStr ings.properties [UTF-8] (original) > +++ > tomcat/trunk/java/org/apache/catalina/tribes/group/interceptors/LocalS trings.properties > [UTF-8] Wed Nov 21 15:15:34 2018 @@ -17,7 +17,7 @@ > domainFilterInterceptor.member.refused=M > domainFilterInterceptor.message.refused=Received message from > cluster[{0}] was refused. > > encryptInterceptor.algorithm.required=Encryption algorithm is > required, fully-specified e.g. AES/CBC/PKCS5Padding > -encryptInterceptor.algorithm.requires-cbc-mode=EncryptInterceptor > only supports CBC cipher modes, not [{0}] > +encryptInterceptor.algorithm.unsupported-mode=EncryptInterceptor > does not support block cipher mode [{0}] > encryptInterceptor.decrypt.error.short-message=Failed to decrypt > message: premature end-of-message > encryptInterceptor.decrypt.failed=Failed to decrypt message > encryptInterceptor.encrypt.failed=Failed to encrypt message > > Modified: > tomcat/trunk/test/org/apache/catalina/tribes/group/interceptors/TestEn cryptInterceptor.java > > URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/tribe s/group/interceptors/TestEncryptInterceptor.java?rev=1847118&r1=1847117& r2=1847118&view=diff > ====================================================================== ======== > > - --- tomcat/trunk/test/org/apache/catalina/tribes/group/interceptors/TestEncr yptInterceptor.java (original) > +++ > tomcat/trunk/test/org/apache/catalina/tribes/group/interceptors/TestEn cryptInterceptor.java > Wed Nov 21 15:15:34 2018 @@ -18,8 +18,14 @@ package > org.apache.catalina.tribes.group > > import org.junit.Assert; import org.junit.Before; +import > org.junit.FixMethodOrder; import org.junit.Ignore; import > org.junit.Test; +import org.junit.runners.MethodSorters; + +import > java.io.File; +import java.io.FileInputStream; +import > java.io.FileOutputStream; > > import org.apache.catalina.tribes.Channel; import > org.apache.catalina.tribes.ChannelException; @@ -38,8 +44,9 @@ > import org.apache.catalina.tribes.io.XBy * though the interceptor > actually operates on byte arrays. This is done * for readability > for the tests and their outputs. */ > +@FixMethodOrder(MethodSorters.NAME_ASCENDING) public class > TestEncryptInterceptor { - private static final String > encryptionKey128 = "cafebabedeadbeefcafebabedeadbeef"; + private > static final String encryptionKey128 = > "cafebabedeadbeefbeefcafecafebabe"; private static final String > encryptionKey192 = > "cafebabedeadbeefbeefcafecafebabedeadbeefbeefcafe"; private static > final String encryptionKey256 = > "cafebabedeadbeefcafebabedeadbeefcafebabedeadbeefcafebabedeadbeef"; > > @@ -95,7 +102,7 @@ public class TestEncryptInterceptor { } > > @Test - @Ignore // Too big for default settings. Breaks Gump, > Eclipse, ... + @Ignore("Too big for default settings. Breaks > Gump, Eclipse, ...") public void testHugePayload() throws Exception > { src.start(Channel.SND_TX_SEQ); dest.start(Channel.SND_TX_SEQ); @@ > -157,7 +164,7 @@ public class TestEncryptInterceptor { > > bytes = roundTrip(bytes, src, dest); > > - return new > String(((ValueCaptureInterceptor)dest.getPrevious()).getValue(), > "UTF-8"); + return new String(bytes, "UTF-8"); } > > /** @@ -171,6 +178,123 @@ public class TestEncryptInterceptor { > return ((ValueCaptureInterceptor)dest.getPrevious()).getValue(); } > > + @Test + @Ignore("ECB mode isn't because it's insecure") + > public void testECB() throws Exception { + > src.setEncryptionAlgorithm("AES/ECB/PKCS5Padding"); + > src.start(Channel.SND_TX_SEQ); + > dest.setEncryptionAlgorithm("AES/ECB/PKCS5Padding"); + > dest.start(Channel.SND_TX_SEQ); + + String testInput = "The > quick brown fox jumps over the lazy dog."; + + > Assert.assertEquals("Failed in ECB mode", + > testInput, + roundTrip(testInput, src, dest)); > + } + + @Test + public void testOFB() throws Exception { + > src.setEncryptionAlgorithm("AES/OFB/PKCS5Padding"); + > src.start(Channel.SND_TX_SEQ); + > dest.setEncryptionAlgorithm("AES/OFB/PKCS5Padding"); + > dest.start(Channel.SND_TX_SEQ); + + String testInput = "The > quick brown fox jumps over the lazy dog."; + + > Assert.assertEquals("Failed in OFB mode", + > testInput, + roundTrip(testInput, src, dest)); > + } + + @Test + public void testCFB() throws Exception { + > src.setEncryptionAlgorithm("AES/CFB/PKCS5Padding"); + > src.start(Channel.SND_TX_SEQ); + > dest.setEncryptionAlgorithm("AES/CFB/PKCS5Padding"); + > dest.start(Channel.SND_TX_SEQ); + + String testInput = "The > quick brown fox jumps over the lazy dog."; + + > Assert.assertEquals("Failed in CFB mode", + > testInput, + roundTrip(testInput, src, dest)); > + } + + @Test + @Ignore("GCM mode is unsupported because > it requires a custom initialization vector") + public void > testGCM() throws Exception { + > src.setEncryptionAlgorithm("AES/GCM/PKCS5Padding"); + > src.start(Channel.SND_TX_SEQ); + > dest.setEncryptionAlgorithm("AES/GCM/PKCS5Padding"); + > dest.start(Channel.SND_TX_SEQ); + + String testInput = "The > quick brown fox jumps over the lazy dog."; + + > Assert.assertEquals("Failed in GCM mode", + > testInput, + roundTrip(testInput, src, dest)); > + } + + @Test + public void testViaFile() throws Exception > { + src.start(Channel.SND_TX_SEQ); + src.setNext(new > ValueCaptureInterceptor()); + + String testInput = "The > quick brown fox jumps over the lazy dog."; + + ChannelData > msg = new ChannelData(false); + msg.setMessage(new > XByteBuffer(testInput.getBytes("UTF-8"), false)); + > src.sendMessage(null, msg, null); + + byte[] bytes = > ((ValueCaptureInterceptor)src.getNext()).getValue(); + + try > (FileOutputStream out = new FileOutputStream("message.bin")) { + > out.write(bytes); + } + + > dest.start(Channel.SND_TX_SEQ); + + bytes = new byte[8192]; > + int read; + + try (FileInputStream in = new > FileInputStream("message.bin")) { + read = > in.read(bytes); + } + + msg = new > ChannelData(false); + XByteBuffer xbb = new > XByteBuffer(read, false); + xbb.append(bytes, 0, read); + > msg.setMessage(xbb); + + dest.messageReceived(msg); + } > + + @Test + public void testPickup() throws Exception { + > File file = new File("message.bin"); + if(!file.exists()) { > + System.err.println("File message.bin does not exist. > Skipping test."); + return; + } + + > dest.start(Channel.SND_TX_SEQ); + + byte[] bytes = new > byte[8192]; + int read; + + try (FileInputStream in = > new FileInputStream("message.bin")) { + read = > in.read(bytes); + } + + ChannelData msg = new > ChannelData(false); + XByteBuffer xbb = new > XByteBuffer(read, false); + xbb.append(bytes, 0, read); + > msg.setMessage(xbb); + + dest.messageReceived(msg); + } > + /** * Interceptor that delivers directly to a destination. */ > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > -----BEGIN PGP SIGNATURE----- Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/ iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAlv1ed0ACgkQHPApP6U8 pFigtQ/8DqpXZ5lZrU/Wxxa7cXyFvJrzMUjnsMIkcl29e4pZtPeWT6OImyWud2FC VNtlGBrUhk2xOHLntS562mK0EJjXPWIjCsw2VRETZhKhV/SDpGEhJzgbkMLWdLHQ sh7egymxIJPyG3NswOyQa6EDSTFz0EA7n9AN3rZ1+Eko38C5CuQEdt2Ew3tlFKAM F/k/zv90/ZhhQHFPjFkLxokP5SOPdRIMN/cXSgZn4C8mUzsONPRr+Sscp+ztERBM 532fZo69pgyjXFHqTed+95xNMaRMsYpgjrKVxGtlVBjrrgsThSeEwvEVe+/Hshb/ 3hASRPLmMMXK6cuuqgxDUCtKNY0Z126GVuUYlqCah1PeJnvEY+fhH5hYDp2Nje0T xWM3NkJslokel7t25VdmSEbuqHhbqI3uHihaMQTQrFgfj7GDykTZgGb0AIGGvrB0 W795VDD73RKx/wQQxrbc6fQ0yqdx2qOmi0wsClr0Dge8a3xltNW9PB2PfGGGmf+Z 3iI/Z1pFAHM3HFa6fcnPqz4+avrasxGpRpbaRxmM5GVk0I4fqjmHxHLkisuwjrWj zrL1F3kyUOtV9kYLX0jKsHR5ZgVcosozwIG6ph2ezhSnUXO84BHTcE05IsSRApeT rfayfZeHo9ja//xgqGgrD8idf+BFeV3c7vQYBLDnPe+Njp4qOOg= =ewzI -----END PGP SIGNATURE----- --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
