On 21/11/2018 15:37, Mark Thomas wrote: > On 21/11/2018 15:29, Christopher Schultz wrote: >> 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. > > I'll fire up my 4 node test cluster and let you know. It may take me a > while - there are usually a bunch of OS updates waiting for me when I > start it up.
I'm seeing lots of errors. I think the problem is that the interceptor is using one Cipher for all members but nodes don't send the same messages to every member so the members get out of sync and decryption starts failing. Mark > > Mark > > >> >> -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 >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
