In Why Isn't My Encryption.. Encrypting?, many were up in arms about the flawed function I posted. And rightfully so, as there was a huge mistake in that code that just about invalidates any so-called "encryption" it performs. But there's one small problem: I didn't write that function.
“this Encrypt() function is like the village bicycle: everybody’s had a ride. It’s a shame this particular bicycle happens to have a crippling lack of brakes”
I would have gone with a herpes metaphor.
Stop it.
s/Mode = CipherMode.ECB/Mode = CipherMode.CBC/g
It might offer marginally more protection, but cryptographic soundness comes from understanding the technology being employed.
The concept CBC > ECB is a dangerous one. Successful implementation of a cryptosystem requires understanding of all requirements of the application, and of the underlying tools.
From the previous threads it appears that most people who consider themselves knowledgeable about cryptography have fundamental flaws in understanding of it.
Errors in a CBC stream are isolated to just two blocks, the block effected by the error, and the block following that. Since the cipher text of following block is error free, error’s do not propagate.
CBC mode also allows arbitrary injection of up to a block’s worth of data. This possible attack would not be available in ECB mode.
“s/Mode = CipherMode.ECB/Mode = CipherMode.CBC/g”
Then there’d just be a heap of “encrypted” material in CBC with no (or implementation fixed) IV. Better sure, but hardly good.
You could perhaps add some javascript call that catches mouse clicks, and then warns them that the code is unusable.
Jeff Atwood might be stuck in Visual Studio land, but at least he knows how to write a substitute expression. Respect++
Jeff:
Read on past OFB to the “Integrity protection and error propagation” section.
ECB, CBC, and OFB don’t enforce the integrity of the message. Using one of the modes that does prevents attackers from manipulating the message.
Also, I should have mentioned this last time, but the .NET calls create a random initialization vector for you already, if you don’t specify one:
http://msdn.microsoft.com/en-us/library/09d0kyb3.aspx
“If the current Key property is null, the GenerateKey method is called to create a new random Key. If the current IV property is null, the GenerateIV method is called to create a new random IV.”
which falls under the category of “provide sensible defaults”… although you need to pull out the IV and pass it back in to decrypt, in modes other than ECB.
Speaking of which, I just verified the TripleDESCryptoServiceProvider() picks CipherMode.CBC by default if you don’t specify the Mode.
So part of the WTF of this Encrypt() snippet is overriding the defaults for no good reason.
Then there’d just be a heap of “encrypted” material in CBC with no (or implementation fixed) IV. Better sure, but hardly good.
Actually, the decryption (as copied from the intarwebs) throws an exception if you switch to the default CipherMode; you have to pass back in the randomly generated IV – see above for MSDN ref.
You can always ask the “The Elders of the Internet” to apply your grep replace 
Okay, that was a lame reference of the IT Crowd episode but hey, I love my bathroom full of gravity 
If only there was a website that developers could post things like code and others could vote on responses and comment on that code.
“I recommend that you apply extra-strength peer review to any code snippets you absorb into your project from the bathroom wall of code”
It is very difficult to efficiently tag “bathroom wall” code, as contributors tend to hide embarassing things like copying code from the “bathroom wall”… you may detect some of the copy&paste, but most of the low quality contributions will be hidden by some minor changes or resegmentation of the code.
I would rather say: any mission-critical section of the code should undergo an extra-strength peer review.
And, if a project is using crypto for something more important than fun, the peer review should be conducted by someone who has some experience in the field, since the very nature of the encryption not only it is very easy to do mistakes, but they also are usually very good to hide themselves.
BTW, ECB mode HAS its fileds of practical application, but just not what was expected from who wrote the bathroom wall snippet.
Anyone thought perhaps Microsoft should just improve their documentation giving detailed descriptions of what modes to use, and include proper sample code with security implications outlined?
If you’re going to use cryptography then please read Bruce Schneiers “Applied Cryptography” first.
It is not about choosing the strongest encryption algorithm or choosing the right cipher mode. It is all about designing a cryptographic protocol that solves your security problem. Encryption algorithms, cipher modes, hash function, digest, random numbers etc. are just lower level primitives used to build your protocols.
If you get into trouble then it’s most probably not because of a weak encryption algorithm but because your protocol is insecure.
This also means that you can’t just copy and paste a line of code or just change ECB to CBC. You actually have to understand what you are doing.
Does that bathroom graffiti pic say “For a lousy lay, call Betty Ann”, followed by “For a lousy encryption algorithm, call Jeff Atwood” ?
There is a thing worse than posting bad code on your blog.
Posting bad code on your blog that someone else wrote.
But it can actually get worse.
Worse is not taking responsibility for posting bad code on your blog that someone else wrote.
What is worse than that? Not sure.
“I recommend that you apply extra-strength peer review to any code snippets you absorb into your project from the bathroom wall of code”
… except you’d only copy and paste code from the web if you didn’t know how to solve the problem yourself in the first place and, by Brian Kernighan’s maxim:
“Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it.”
So if you’ve got code from the net you’re almost certainly not qualified to debug it - because you presumably weren’t clever enough to write that code to start with by yourself (unless you temporarily forgot the exact API calls, for instance, but still knew exactly what you’re doing).
The only solutions, IMHO, are:
Jeff, why don’t you start to apply the sed to your previous post ?
You don’t think those “bathroom plagiarism idiots” are going to read both posts, do you ?
