Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AbstractStreamCipher leads to CBC with zeroed IV #28

Open
GoogleCodeExporter opened this issue Mar 12, 2015 · 1 comment
Open

AbstractStreamCipher leads to CBC with zeroed IV #28

GoogleCodeExporter opened this issue Mar 12, 2015 · 1 comment

Comments

@GoogleCodeExporter
Copy link

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant