2

I'm sorry about the confusing title, I honestly had no clue what to call it! I was tossing up whether this belong on one of the security/crypto exchange sites but as this is predominantly a programming question I will post it here. Feel free to move it!

I have an AES crypto stream, and as AES pads the original data with blocks, the resulting encrypted data is almost always a different size to the original unencrypted data. When decrypting, you need to know how many bytes to read from the crypto stream (how many unencrypted bytes there are). I was originally planning on sending the original, unencrypted data length in the packet but then I thought of another way. If I just read 4096 bytes from the Crypto Stream and store how many actual bytes were read, I can then copy the correct amount of bytes to a new array and use that.

Is it safe to do that? My code is the following:

using (ICryptoTransform crypt = AES.CreateDecryptor())
{
    using (MemoryStream memStrm = new MemoryStream(data))
    {
        using (CryptoStream cryptStrm = new CryptoStream(memStrm, crypt, CryptoStreamMode.Read))
        {
            byte[] bytes = new byte[size];
            int read = cryptStrm.Read(bytes, 0, 4096);
            byte[] temp = new byte[read];
            Array.Copy(bytes, temp, read);
            return temp;
        }
    }
}

By safe I mean, will it always produce correct decrypted data?

jduncanator
  • 2,154
  • 1
  • 22
  • 39
  • Rereading your post, it's not clear to me whether you've found that the CryptoStream *will* return extra data (and you're asking whether just a single Read will prevent that) or whether you're checking whether CryptoStream *already* removes padding. Either way, using a single call to `Read` is a bad idea - there's no guarantee that it'll return all the data in one go. – Jon Skeet Feb 14 '13 at 07:15
  • I did a test with the string "Hello!" and it decrypted ok, but I cannot afford for it giving unreliable results. – jduncanator Feb 14 '13 at 07:23
  • I suspect this question degenerates into "How do I read correctly from streams?". The `CryptoStream` should transparently handle any padding on the final block, so you can treat it like any other input stream. – Duncan Jones Feb 14 '13 at 07:53
  • However, how do I communicate, across the Internet, the original length of the input data without sending it with the rest of the packet. – jduncanator Feb 14 '13 at 07:57
  • 1
    Your code will fail if the data is larger than 4096 bytes or larger than `size`. – CodesInChaos Feb 14 '13 at 08:09
  • @jduncanator: If Duncan's right and it handles the padding itself, then do you still *need* to send the length? And if you *do* need to send the length, why wouldn't you put it with the rest of the data? – Jon Skeet Feb 14 '13 at 08:13
  • @JonSkeet I should have made it clearer that I was *assuming* that to be the case, based on how similar classes operate in Java [and a cursory scan of the CryptoStream API] :-) – Duncan Jones Feb 14 '13 at 08:20
  • @CodesInChaos I'm aware of that, any packets larger than 4096 bytes are split in two, and have a bit-flag set to allow for concatenation (don't heckle me over this, it was a requirement of the protocol). The `size` isn't meant to be there, sorry, just a left over from copy-paste. – jduncanator Feb 14 '13 at 08:34
  • 1
    Since you're talking about packets, this sounds a bit like you're encrypting a network connection. In that case, use SSL, not such a homebrew scheme. – CodesInChaos Feb 14 '13 at 08:41
  • I'd prefer to use my own key-exchange (Diffie-Hellman) and encryption algorithms (AES) as SSL has been proven untrustworthy. I think I remember seeing Wireshark decrypting SSL traffic? – jduncanator Feb 14 '13 at 08:46
  • 1
    @jduncanator Only if you give wireshark additional information, such as the server's private key. SSL has its flaws, but if you use it correctly, they'll be much smaller than the flaws in your protocol. – CodesInChaos Feb 14 '13 at 08:53
  • @CodesInChaos But SSL is easily defeated by a man-in-the-middle attack is it not? The purpose for my encryption is to prevent a user of my program from "emulating" my authentication servers. By encrypting everything, it makes it next to impossible! With Diffie-Hellman, I can at-least hide the two Primes/Bases in my code which is heavily protected and it provides full forward secrecy. With SSL I could simply plop a client in the middle and negotiate the key pairs to decrypt data from both parties without even decompiling my program! – jduncanator Feb 14 '13 at 09:05
  • 1
    SSL is not vulnerable to MitM if the client verifies the server key. But since you don't want security, just obfuscation a custom scheme might be preferable. I'd still run it inside SSL. – CodesInChaos Feb 14 '13 at 09:07

1 Answers1

2

Why are you jumping through so many hoops? MemoryStream, CryptoStream, temporary arrays...

return crypt.TransformFinalBlock(data, 0, data.Length);

To make your crypto secure, you should also use a random IV for each encryption, stored alongside the ciphertext. And adding a MAC (such as HMAC-SHA-256) in an encrypt-then-mac construction prevents a number of active attacks, including padding oracles.

CodesInChaos
  • 106,488
  • 23
  • 218
  • 262
  • 1
    Everywhere I go I see people using MemoryStreams etc. Even the Microsoft documentation does it this way! You suggest adding a MAC, do you have anywhere you could point me to find a guide or some information on it? – jduncanator Feb 14 '13 at 08:28
  • However, this method seems to work flawlessly!! THANKS! I wonder why Microsoft and others decide to implement streams and the lot! Regarding the IV, is the IV basically a Salt? Wouldn't sending the IV in plain text remove the purpose for it? – jduncanator Feb 14 '13 at 08:38
  • 1
    [HMACSHA256 class](http://msdn.microsoft.com/en-us/library/system.security.cryptography.hmacsha256.aspx) and for how encrypt-then-mac works: http://security.stackexchange.com/questions/26289/strongest-cryptographic-algorithm-available-in-php-5-3/26317#26317 – CodesInChaos Feb 14 '13 at 08:38
  • IV is something similar to a salt. It's not secret, but must be different for each encryption, and in the case of CBC mode it should be unpredictable. – CodesInChaos Feb 14 '13 at 08:39
  • Brute-forcing AES keys is hopeless with current technology, because there are too many (and with AES-256 it's hopeless even with future technology). The idea of an IV is that if you encrypt the same message twice, you'll get completely independent ciphertext. In some cases it also prevents multi-target attacks. – CodesInChaos Feb 14 '13 at 08:51
  • Thanks for clarifying that! :) Sorry I deleted my comment, after reviewing it, I determined it was stupid ;) What is the point of a MAC? To me it seems as if its just layering as much extra encryption as possible without doing much. How is anyone going to obtain the original key to the encryption to "modify" the message contents?! To me MAC looks as if it assumes the key for the encryption is weak and simply uses another key to "beef up" the encryption a little. – jduncanator Feb 14 '13 at 09:00
  • A MAC allows you to reject messages that an attacker modified. If you decrypt modified messages without verifying a MAC first, an attacker can get the recipient to decrypt a message for him. The attacker doesn't learn the encryption key, but he learns the plaintext, and can feed your recipient forged data. – CodesInChaos Feb 14 '13 at 09:00
  • How would the attacker encrypt a message in the first place for the recipient to decrypt without knowing the AES Key? – jduncanator Feb 14 '13 at 09:02
  • @jduncanator there are various side channel attacks that work that way. I wrote a public domain code snippet for [Encrypt then Mac](http://stackoverflow.com/a/10366194/637783) that I try and keep up to date and reviewed. – jbtule Feb 14 '13 at 13:30