views:

2495

answers:

3

I'm trying to encrypt and decrypt a file stream over a socket using RijndaelManaged, but I keep bumping into the exception

CryptographicException: Length of the data to decrypt is invalid.
    at System.Security.Cryptography.RijndaelManagedTransform.TransformFinalBlock(Byte[] inputBuffer, Int32 inputOffset, Int32 inputCount)
    at System.Security.Cryptography.CryptoStream.FlushFinalBlock()
    at System.Security.Cryptography.CryptoStream.Dispose(Boolean disposing)

The exception is thrown at the end of the using statement in receiveFile, when the whole file has been transferred.

I tried searching the web but only found answers to problems that arise when using Encoding when encrypting and decrypting a single string. I use a FileStream, so I don't specify any Encoding to be used, so that should not be the problem. These are my methods:

private void transferFile(FileInfo file, long position, long readBytes) {
    // transfer on socket stream
    Stream stream = new FileStream(file.FullName, FileMode.Open);
    if (position > 0) {
        stream.Seek(position, SeekOrigin.Begin);
    }
    // if this should be encrypted, wrap the encryptor stream
    if (UseCipher) {
        stream = new CryptoStream(stream, streamEncryptor, CryptoStreamMode.Read);
    }
    using (stream) {
        int read;
        byte[] array = new byte[8096];
        while ((read = stream.Read(array, 0, array.Length)) > 0) {
            streamSocket.Send(array, 0, read, SocketFlags.None);
            position += read;
        }
    }
}
private void receiveFile(FileInfo transferFile) {
    byte[] array = new byte[8096];
    // receive file
    Stream stream = new FileStream(transferFile.FullName, FileMode.Append);
    if (UseCipher) {
        stream = new CryptoStream(stream, streamDecryptor, CryptoStreamMode.Write);
    }
    using (stream) {
        long position = new FileInfo(transferFile.Path).Length;
        while (position < transferFile.Length) {
            int maxRead = Math.Min(array.Length, (int)(transferFile.Length - position));
            int read = position < array.Length
                       ? streamSocket.Receive(array, maxRead, SocketFlags.None)
                       : streamSocket.Receive(array, SocketFlags.None);
            stream.Write(array, 0, read);
            position += read;
        }
    }
}

This is the method I use to set up the ciphers. byte[] init is a generated byte array.

private void setupStreamCipher(byte[] init) {
    RijndaelManaged cipher = new RijndaelManaged();
    cipher.KeySize = cipher.BlockSize = 256; // bit size
    cipher.Mode = CipherMode.ECB;
    cipher.Padding = PaddingMode.ISO10126;
    byte[] keyBytes = new byte[32];
    byte[] ivBytes = new byte[32];

    Array.Copy(init, keyBytes, 32);
    Array.Copy(init, 32, ivBytes, 0, 32);

    streamEncryptor = cipher.CreateEncryptor(keyBytes, ivBytes);
    streamDecryptor = cipher.CreateDecryptor(keyBytes, ivBytes);
}

Anyone have an idea in what I might be doing wrong?

Regards, Patrick

+1  A: 

It looks to me like you're not properly sending the final block. You need to at least FlushFinalBlock() the sending CryptoStream in order to ensure that the final block (which the receiving stream is looking for) is sent.

By the way, CipherMode.ECB is more than likely an epic fail in terms of security for what you're doing. At least use CipherMode.CBC (cipher-block chaining) which actually uses the IV and makes each block dependent on the previous one.

EDIT: Whoops, the enciphering stream is in read mode. In that case you need to make sure you read to EOF so that the CryptoStream can deal with the final block, rather than stopping after readBytes. It's probably easier to control if you run the enciphering stream in write mode.

One more note: You cannot assume that bytes in equals bytes out. Block ciphers have a fixed block size they process, and unless you are using a cipher mode that converts the block cipher to a stream cipher, there will be padding that makes the ciphertext longer than the plaintext.

Jeffrey Hantin
The FlushFinalBlock() method is called in the "closing section" of the using statement<pre>using(stream) { // } // calls Close() -> FlushFinalBlock()</pre>I will change the CipherMode, I just entered it as an example so you know that I don't initialize my cipher in any "weird" way.The readBytes in sendFile() is not used yet, I forgot to remove it. I read to the end of the file, so this should not be the issue here.I thought <pre>cipher.Padding = PaddingMode.ISO10126;</pre> was taking care of the padding? What can I change to make it work?
Patrick
If the enciphering stream is in read mode, the final block will be lost if you Dispose it; it has to actually read end of file from its underlying source stream to produce the final block.
Jeffrey Hantin
In response to Jeffrey: If I try to call stream.FlushFinalBlock() it says NonSupportedException: FlushFinalBlock can not be called twice on the same stream. Doesn't this mean that an end of file has been read (and sent)?
Patrick
Not necessarily. Since the stream is in read mode, you can't call FlushFinalBlock yourself or you'll have a problem: you need to ensure that the EOF condition "reads through" the CryptoStream.On the receive side, your code seems to make the assumption that there is a byte-for-byte correspondence between bytes-on-the-wire and bytes-in-the-file, which is actually more likely the cause of your exception. You will have to read more bytes from the socket than the size of the file permits in order to read the padding.
Jeffrey Hantin
Ah, of course, I'm not reading from the cryptostream, but from a socket! Silly me...
Patrick
A: 
cipher.Mode = CipherMode.ECB;

Argh! Rolling your own security code is almost always a bad idea.

Alun Harford
????? huh? He is using Rijndael? this is not "roll your own." Though, there is a good point to be made, that developers need to be careful about how they use encryption.
Cheeso
ECB fails here because each block is independently encrypted.
Jeffrey Hantin
It doesn't matter which CipherMode I use, I still get the "Length of data..." exception...
Patrick
@Cheeso: You don't have to be making up your own crypto algorithm (or even implementation) to be rolling your own security.In this case, it looks like Patrick was trying to send a file over the wire - there are lots of good ways to do that.By rolling his own way, he very nearly put a huge security hole in his application that was found not by good analysis (as would be the case with a decent library) but by posted an unrelated question on StackOverflow.
Alun Harford
+1  A: 

After the comment made by Jeffrey Hantin, I changed some lines in receiveFile to

using (stream) {
    FileInfo finfo = new FileInfo(transferFile.Path);
    long position = finfo.Length;
    while (position < transferFile.Length) {
        int maxRead = Math.Min(array.Length, (int)(transferFile.Length - position));
        int read = position < array.Length
                   ? streamSocket.Receive(array, maxRead, SocketFlags.None)
                   : streamSocket.Receive(array, SocketFlags.None);
        stream.Write(array, 0, read);
        position += read;
    }
}

->

using (stream) {
    int read = array.Length;
    while ((read = streamSocket.Receive(array, read, SocketFlags.None)) > 0) {
        stream.Write(array, 0, read);
        if ((read = streamSocket.Available) == 0) {
            break;
        }
    }
}

And voila, she works (because of the ever so kind padding that I didn't care to bother about earlier). I'm not sure what happens if Available returns 0 even though all data hasn't been transferred, but I'll tend to that later in that case. Thanks for your help Jeffrey!

Regards.

Patrick