mooreds / gwt-crypto

Automatically exported from code.google.com/p/gwt-crypto
10 stars 1 forks source link

AbstractStreamCipher leads to CBC with zeroed IV #28

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
See code below. AESCipher is not directly unit tested. Your API seems to imply 
that AESCipher & AESCipher.setKey(...) are safe to use. However, they're not, 
since without specifically calling setParameters with a ParametersWithIV 
object, the CBC mode will be initialized with zeroes. This potentially leaks 
information if the same key is used. Please clean this up; per my other bug 
report, it seems CTR would be better, but at the least your API / tests should 
show a more secure way of using this.

public class AbstractStreamCipher implements StreamCipher{
    private PaddedBufferedBlockCipher cipher = null;
    private KeyParameter params = null;

    protected AbstractStreamCipher(BlockCipher cipher) {
        this.cipher = new PaddedBufferedBlockCipher(new CBCBlockCipher(cipher));
    }

    public byte[] getKey() {
        return params.getKey();
    }

    /**
     * @param key must be between 16 and 24 bytes.  
     */
    public void setKey(byte[] key) {
        this.params = new KeyParameter(key);
    }

...

public class CBCBlockCipher {
...
    /**
     * Initialise the cipher and, possibly, the initialisation vector (IV).
     * If an IV isn't passed as part of the parameter, the IV will be all zeros.
     *
     * @param encrypting if true the cipher is initialised for
     *  encryption, if false for decryption.
     * @param params the key and other data required by the cipher.
     * @exception IllegalArgumentException if the params argument is
     * inappropriate.
     */
    @Override
    public void init(
        boolean             encrypting,
        CipherParameters    params)
        throws IllegalArgumentException
    {
        this.encrypting = encrypting;

        if (params instanceof ParametersWithIV)
        {

Original issue reported on code.google.com by quickte...@gmail.com on 14 Feb 2014 at 12:34

GoogleCodeExporter commented 9 years ago
Also, AbstractStreamCipher doesn't event allow ParametersWithIV to be passed 
in; that means that CBCBlockCipher can never be initialized with a non-zero IV 
using the AESCipher API.

    public void setParameters(CipherParameters params) throws IllegalArgumentException
    {
        if (params == null) this.params = null;
        else if (! (params instanceof KeyParameter))
            throw new IllegalArgumentException();
        else this.params = (KeyParameter)params;
    }

Original comment by quickte...@gmail.com on 14 Feb 2014 at 6:34