BouncyCastle Rijndael 256 Implementation

Question

I modified the implementation from this website to include a salt:

package de.xxx.yyy.main;

import static org.junit.Assert.*;

import org.bouncycastle.crypto.BlockCipher;
import org.bouncycastle.crypto.BufferedBlockCipher;
import org.bouncycastle.crypto.DataLengthException;
import org.bouncycastle.crypto.InvalidCipherTextException;
import org.bouncycastle.crypto.PBEParametersGenerator;
import org.bouncycastle.crypto.digests.SHA3Digest;
import org.bouncycastle.crypto.engines.RijndaelEngine;
import org.bouncycastle.crypto.generators.PKCS12ParametersGenerator;
import org.bouncycastle.crypto.modes.CBCBlockCipher;
import org.bouncycastle.crypto.paddings.PKCS7Padding;
import org.bouncycastle.crypto.paddings.PaddedBufferedBlockCipher;
import org.bouncycastle.crypto.params.ParametersWithIV;
import org.bouncycastle.util.encoders.Base64;
import org.junit.Test;

public class test {

    private char[] password = "0123456789abcdef0123456789abcdef".toCharArray();
    private byte[] salt = "0123456789".getBytes();
    private int iterationCount = 5;

    @Test
    public void testEncryptRijndael() throws DataLengthException, IllegalStateException, InvalidCipherTextException {

        PKCS12ParametersGenerator pGen = new PKCS12ParametersGenerator(new SHA3Digest(256));
        char[] passwordChars = password;

        final byte[] pkcs12PasswordBytes = PBEParametersGenerator.PKCS12PasswordToBytes(passwordChars);
        pGen.init(pkcs12PasswordBytes, salt , iterationCount );     

        BlockCipher engine = new RijndaelEngine(256);
        CBCBlockCipher cbc = new CBCBlockCipher(engine);
        BufferedBlockCipher cipher = new PaddedBufferedBlockCipher(cbc, new PKCS7Padding());

        ParametersWithIV aesCBCParams = (ParametersWithIV) pGen.generateDerivedParameters(256, 256);
        cipher.init(true, aesCBCParams);

        byte[] input = "Hallo ich bin ein Test".getBytes();
        byte[] cipherText = new byte[cipher.getOutputSize(input.length)];

        int cipherLength = cipher.processBytes(input, 0, input.length, cipherText, 0);
        cipher.doFinal(cipherText, cipherLength);

        String result = new String(Base64.encode(cipherText));
        System.out.println("testEncryptRijndael result : " + result);
        assertEquals("cMoMSNMNsikAkLjaheE6iD48Xkfvo7Y6gS8/zroGfHc=",result);
    }

    @Test
    public void testDecryptRijndael() throws DataLengthException, IllegalStateException, InvalidCipherTextException {
        PKCS12ParametersGenerator pGen = new PKCS12ParametersGenerator(new SHA3Digest(256));
        char[] passwordChars = password;

        final byte[] pkcs12PasswordBytes = PBEParametersGenerator.PKCS12PasswordToBytes(passwordChars);
        pGen.init(pkcs12PasswordBytes, salt , iterationCount );     

        BlockCipher engine = new RijndaelEngine(256);
        CBCBlockCipher cbc = new CBCBlockCipher(engine);
        BufferedBlockCipher cipher = new PaddedBufferedBlockCipher(cbc, new PKCS7Padding());

        ParametersWithIV aesCBCParams = (ParametersWithIV) pGen.generateDerivedParameters(256, 256);
        cipher.init(false, aesCBCParams);

        byte[] output = Base64.decode("cMoMSNMNsikAkLjaheE6iD48Xkfvo7Y6gS8/zroGfHc=".getBytes());
        byte[] cipherText = new byte[cipher.getOutputSize(output.length)];

        int cipherLength = cipher.processBytes(output, 0, output.length, cipherText, 0);
        int outputLength = cipher.doFinal(cipherText, cipherLength);
        outputLength += cipherLength;

        byte[] resultBytes = cipherText;
        if (outputLength != output.length) {
            resultBytes = new byte[outputLength];
            System.arraycopy(
                    cipherText, 0,
                    resultBytes, 0,
                    outputLength
                    );
        }

        String result = new String(resultBytes);
        System.out.println("testDecryptRijndael result : " + result);
        assertEquals("Hallo ich bin ein Test", result);
    }
}

Is this a good implementation, or are there some mistakes, logical errors or security risks?

You really shouldn't be using Rijndael-256. It's generally considered less secure than the much more heavily scrutinized AES (which is based on Rijndael-128). If you want to do AES-256, you use the Rijndael-128 cipher, with a 256 bit key. — ircmaxell
– ircmaxell, Commented Aug 28, 2014 at 12:55

janos · Accepted Answer · 2014-08-30 22:39:26Z

It's good that you used a more interesting test string instead of "value" and that in testEncryptRijndael you replaced assertNotNull with a more strict assertEquals.

Since your tests are different from the originals, it would be better to rename them to testEncryptRijndaelWithSalt and testDecryptRijndaelWithSalt, respectively. And keep the originals too.

In both the original tests and yours I don't see the point of the logging and printing statements. I think you can drop that. They do no harm, but they might invite the bad habit of reading test outputs instead of focusing on adding proper assertions.

There's a lot of duplication in the encryption and decryption cases. Perhaps you could move that logic to a common private helper method. That can reduce possible typing errors.

I don't see the point of this if block:

 if (outputLength != output.length) {
     // ...
 }

The thing is, in a unit test everything should be predictable: you should already know at this point the outcome of that condition. If outputLength should be equal to output.length, then drop the if but keep the block of code. Otherwise delete the whole thing. You can add an assertion for the condition as a sanity check.

Thanks for your answer. A private helper method is a good idea. I will do that. The if-block is there, because I want to generate random Strings, encrypt end decrypt them. So it's there, just for ongoing modifications. — schnawel007
– schnawel007, Commented Aug 28, 2014 at 7:13

Stack Exchange Network

BouncyCastle Rijndael 256 Implementation

1 Answer 1

You must log in to answer this question.

Hot Network Questions

BouncyCastle Rijndael 256 Implementation

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions