From 21dcb3d9744c83dcf2ff8fcee06dbca7bfa4ef35 Mon Sep 17 00:00:00 2001 From: David Hook Date: Sat, 27 Aug 2016 13:20:05 +1000 Subject: [PATCH] modified IESEngine so that MAC check is the primary one added general BadBlockException class for asymmetric ciphers. --- .../crypto/engines/IESEngine.java | 32 ++++++++++++------- .../provider/asymmetric/dh/IESCipher.java | 7 ++-- .../provider/asymmetric/ec/IESCipher.java | 10 +++--- .../provider/asymmetric/rsa/CipherSpi.java | 11 ++----- .../provider/util/BadBlockException.java | 21 ++++++++++++ .../jce/provider/test/DHIESTest.java | 22 +++++++++++++ .../jce/provider/test/ECIESTest.java | 21 ++++++++++++ 7 files changed, 96 insertions(+), 28 deletions(-) create mode 100644 prov/src/main/java/org/bouncycastle/jcajce/provider/util/BadBlockException.java diff --git a/core/src/main/java/org/bouncycastle/crypto/engines/IESEngine.java b/core/src/main/java/org/bouncycastle/crypto/engines/IESEngine.java index 7c32011917..3e7ade8eae 100755 --- a/core/src/main/java/org/bouncycastle/crypto/engines/IESEngine.java +++ b/core/src/main/java/org/bouncycastle/crypto/engines/IESEngine.java @@ -67,8 +67,8 @@ public IESEngine( /** - * set up for use in conjunction with a block cipher to handle the - * message. + * Set up for use in conjunction with a block cipher to handle the + * message.It is strongly recommended that the cipher is not in ECB mode. * * @param agree the key agreement used as the basis for the encryption * @param kdf the key derivation function used for byte generation @@ -269,8 +269,8 @@ private byte[] decryptBlock( int inLen) throws InvalidCipherTextException { - byte[] M = null, K = null, K1 = null, K2 = null; - int len; + byte[] M, K, K1, K2; + int len = 0; // Ensure that the length of the input is greater than the MAC in bytes if (inLen < V.length + mac.getMacSize()) @@ -278,6 +278,7 @@ private byte[] decryptBlock( throw new InvalidCipherTextException("Length of input must be greater than the MAC and V combined"); } + // note order is important: set up keys, do simple encryptions, check mac, do final encryption. if (cipher == null) { // Streaming mode. @@ -298,14 +299,13 @@ private byte[] decryptBlock( System.arraycopy(K, K1.length, K2, 0, K2.length); } + // process the message M = new byte[K1.length]; for (int i = 0; i != K1.length; i++) { M[i] = (byte)(in_enc[inOff + V.length + i] ^ K1[i]); } - - len = K1.length; } else { @@ -325,15 +325,15 @@ private byte[] decryptBlock( } else { - cipher.init(false, new KeyParameter(K1)); + cipher.init(false, new KeyParameter(K1)); } M = new byte[cipher.getOutputSize(inLen - V.length - mac.getMacSize())]; + + // do initial processing len = cipher.processBytes(in_enc, inOff + V.length, inLen - V.length - mac.getMacSize(), M, 0); - len += cipher.doFinal(M, len); } - // Convert the length of the encoding vector into a byte array. byte[] P2 = param.getEncodingV(); byte[] L2 = null; @@ -362,11 +362,19 @@ private byte[] decryptBlock( if (!Arrays.constantTimeAreEqual(T1, T2)) { - throw new InvalidCipherTextException("Invalid MAC."); + throw new InvalidCipherTextException("invalid MAC"); } - // Output the message. - return Arrays.copyOfRange(M, 0, len); + if (cipher == null) + { + return M; + } + else + { + len += cipher.doFinal(M, len); + + return Arrays.copyOfRange(M, 0, len); + } } diff --git a/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/dh/IESCipher.java b/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/dh/IESCipher.java index a721cc0fa3..7a99419f51 100644 --- a/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/dh/IESCipher.java +++ b/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/dh/IESCipher.java @@ -45,6 +45,7 @@ import org.bouncycastle.crypto.parsers.DHIESPublicKeyParser; import org.bouncycastle.jcajce.provider.asymmetric.util.DHUtil; import org.bouncycastle.jcajce.provider.asymmetric.util.IESUtil; +import org.bouncycastle.jcajce.provider.util.BadBlockException; import org.bouncycastle.jcajce.util.BCJcaJceHelper; import org.bouncycastle.jcajce.util.JcaJceHelper; import org.bouncycastle.jce.interfaces.IESKey; @@ -425,7 +426,7 @@ public byte[] engineDoFinal( } catch (Exception e) { - throw new BadPaddingException(e.getMessage()); + throw new BadBlockException("unable to process block", e); } } @@ -464,7 +465,7 @@ public byte[] getEncoded(AsymmetricKeyParameter keyParameter) } catch (Exception e) { - throw new BadPaddingException(e.getMessage()); + throw new BadBlockException("unable to process block", e); } } else if (state == Cipher.DECRYPT_MODE || state == Cipher.UNWRAP_MODE) @@ -478,7 +479,7 @@ else if (state == Cipher.DECRYPT_MODE || state == Cipher.UNWRAP_MODE) } catch (InvalidCipherTextException e) { - throw new BadPaddingException(e.getMessage()); + throw new BadBlockException("unable to process block", e); } } else diff --git a/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/IESCipher.java b/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/IESCipher.java index 3c29b2b74e..b14bde3058 100644 --- a/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/IESCipher.java +++ b/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/IESCipher.java @@ -43,6 +43,7 @@ import org.bouncycastle.crypto.parsers.ECIESPublicKeyParser; import org.bouncycastle.jcajce.provider.asymmetric.util.ECUtil; import org.bouncycastle.jcajce.provider.asymmetric.util.IESUtil; +import org.bouncycastle.jcajce.provider.util.BadBlockException; import org.bouncycastle.jcajce.util.BCJcaJceHelper; import org.bouncycastle.jcajce.util.JcaJceHelper; import org.bouncycastle.jce.interfaces.ECKey; @@ -430,7 +431,7 @@ public byte[] engineDoFinal( } catch (Exception e) { - throw new BadPaddingException(e.getMessage()); + throw new BadBlockException("unable to process block", e); } } @@ -456,11 +457,10 @@ public byte[] getEncoded(AsymmetricKeyParameter keyParameter) return engine.processBlock(in, 0, in.length); } - catch (Exception e) + catch (final Exception e) { - throw new BadPaddingException(e.getMessage()); + throw new BadBlockException("unable to process block", e); } - } else if (state == Cipher.DECRYPT_MODE || state == Cipher.UNWRAP_MODE) { @@ -473,7 +473,7 @@ else if (state == Cipher.DECRYPT_MODE || state == Cipher.UNWRAP_MODE) } catch (InvalidCipherTextException e) { - throw new BadPaddingException(e.getMessage()); + throw new BadBlockException("unable to process block", e); } } else diff --git a/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/rsa/CipherSpi.java b/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/rsa/CipherSpi.java index 3bfcbe5437..7c40baa1cd 100644 --- a/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/rsa/CipherSpi.java +++ b/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/rsa/CipherSpi.java @@ -32,6 +32,7 @@ import org.bouncycastle.crypto.engines.RSABlindedEngine; import org.bouncycastle.crypto.params.ParametersWithRandom; import org.bouncycastle.jcajce.provider.asymmetric.util.BaseCipherSpi; +import org.bouncycastle.jcajce.provider.util.BadBlockException; import org.bouncycastle.jcajce.provider.util.DigestFactory; import org.bouncycastle.jcajce.util.BCJcaJceHelper; import org.bouncycastle.jcajce.util.JcaJceHelper; @@ -528,15 +529,9 @@ private byte[] getOutput() return cipher.processBlock(bytes, 0, bytes.length); } - catch (final InvalidCipherTextException e) + catch (InvalidCipherTextException e) { - throw new BadPaddingException("unable to decrypt block") - { - public synchronized Throwable getCause() - { - return e; - } - }; + throw new BadBlockException("unable to decrypt block", e); } finally { diff --git a/prov/src/main/java/org/bouncycastle/jcajce/provider/util/BadBlockException.java b/prov/src/main/java/org/bouncycastle/jcajce/provider/util/BadBlockException.java new file mode 100644 index 0000000000..e2a8d63d15 --- /dev/null +++ b/prov/src/main/java/org/bouncycastle/jcajce/provider/util/BadBlockException.java @@ -0,0 +1,21 @@ +package org.bouncycastle.jcajce.provider.util; + +import javax.crypto.BadPaddingException; + +public class BadBlockException + extends BadPaddingException +{ + private final Throwable cause; + + public BadBlockException(String msg, Throwable cause) + { + super(msg); + + this.cause = cause; + } + + public Throwable getCause() + { + return cause; + } +} diff --git a/prov/src/test/java/org/bouncycastle/jce/provider/test/DHIESTest.java b/prov/src/test/java/org/bouncycastle/jce/provider/test/DHIESTest.java index 4cea8a920a..a21bbc2e32 100755 --- a/prov/src/test/java/org/bouncycastle/jce/provider/test/DHIESTest.java +++ b/prov/src/test/java/org/bouncycastle/jce/provider/test/DHIESTest.java @@ -7,6 +7,7 @@ import java.security.SecureRandom; import java.security.Security; +import javax.crypto.BadPaddingException; import javax.crypto.Cipher; import javax.crypto.interfaces.DHPrivateKey; import javax.crypto.interfaces.DHPublicKey; @@ -222,6 +223,27 @@ public void doTest( out2 = c2.doFinal(out1, 0, out1.length); if (!areEqual(out2, message)) fail(testname + " test failed with non-null parameters, DHAES mode true."); + + // + // corrupted data test + // + byte[] tmp = new byte[out1.length]; + for (int i = 0; i != out1.length; i++) + { + System.arraycopy(out1, 0, tmp, 0, tmp.length); + tmp[i] = (byte)~tmp[i]; + + try + { + c2.doFinal(tmp, 0, tmp.length); + + fail("decrypted corrupted data"); + } + catch (BadPaddingException e) + { + isTrue("wrong message: " + e.getMessage(), "unable to process block".equals(e.getMessage())); + } + } } public static void main( diff --git a/prov/src/test/java/org/bouncycastle/jce/provider/test/ECIESTest.java b/prov/src/test/java/org/bouncycastle/jce/provider/test/ECIESTest.java index 9a5e0128cf..059fa19a50 100755 --- a/prov/src/test/java/org/bouncycastle/jce/provider/test/ECIESTest.java +++ b/prov/src/test/java/org/bouncycastle/jce/provider/test/ECIESTest.java @@ -7,6 +7,7 @@ import java.security.Security; import java.security.spec.ECGenParameterSpec; +import javax.crypto.BadPaddingException; import javax.crypto.Cipher; import javax.crypto.SealedObject; @@ -241,7 +242,27 @@ public void doTest( if (!areEqual(out2, message)) fail(testname + " test failed with non-null parameters, DHAES mode false."); + // + // corrupted data test + // + int offset = out1.length - (message.length + 8); + byte[] tmp = new byte[out1.length]; + for (int i = offset; i != out1.length; i++) + { + System.arraycopy(out1, 0, tmp, 0, tmp.length); + tmp[i] = (byte)~tmp[i]; + + try + { + c2.doFinal(tmp, 0, tmp.length); + fail("decrypted corrupted data"); + } + catch (BadPaddingException e) + { + isTrue("wrong message: " + e.getMessage(), "unable to process block".equals(e.getMessage())); + } + } // TODO: DHAES mode is not currently implemented, perhaps it shouldn't be... // c1 = Cipher.getInstance(cipher + "/DHAES/PKCS7Padding","BC"); // c2 = Cipher.getInstance(cipher + "/DHAES/PKCS7Padding","BC");