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. 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
