Hacker News Reader

Comment by sweis | Hacker News Reader

This appears to be using AES-CBC without any authentication:

https://textshredder.codeplex.com/SourceControl/latest

(HN breaks the link directly to the code, but add this fragment to go straight to the AES.cs class: "#Code/Release 1.0/ClientLibrary/CryptoProviders/AES.cs")

This is not secure and should include authentication. As far as I know, .NET does not include authenticated cipher modes by default:

http://msdn.microsoft.com/en-us/library/system.security.cryp...

I recommend using a second HMAC key and computing a HMAC over both the IV and ciphertext. Keyczar does something similar:

https://code.google.com/p/keyczar/wiki/CiphertextFormat

View on HN

I will update the code to do a hmac of the message and then alert the user if the cipher text has been tampered with. Not that hard to add in.

Make sure to encrypt-then-mac instead of mac-then-encrypt[1]

[1] http://crypto.stackexchange.com/questions/202/should-we-mac-...

Yeah I will do it in that order. Thanks for the help :-)

Would you mind to elaborate in which way the use of AES-CBC makes the encryption insecure?

CBC is okay if it is used to encrypt before MACing. The issue is that TextShredder used CBC without any authentication, i.e. a MAC. That means someone can modify the ciphertext in transit and it won't be detected. This opens up several types of attacks. In theoretical terms, unauthenticated block ciphers are not secure against adaptive chosen ciphertext attacks (CCA2). In practical terms, one attack that comes to mind is a padding oracle attack. Essentially, an attacker given a real message and can create modified ciphertexts that will cause a decrypting party to potentially leak information in a side channel.

Serge Vaudenay talked about this 12 years ago:

http://www.iacr.org/cryptodb/archive/2002/EUROCRYPT/2850/285...

The recent Lucky 13 attack was a form of this type of padding oracle attack:

https://www.imperialviolet.org/2013/02/04/luckythirteen.html

I understand the point in the general case but I don't think you could exploit it in this case. Nonetheless it would still be nicer if the application would tell me that someone tampered with the message instead of making me infer it from the fact that it partially decoded into garbage. Thanks!

I will add in a hmac if the ciphertext and IV and alert the user if the message has been tampered with. I will have it updated within the next week.

Thanks for the constructive feedback. This is why I posted it here and made it open source so I could get peer review, make the utility better and learn a few things along the way.

One thing I would really suggest is to get rid of all this conversions between strings and byte arrays, this adds unnecessarily complexity to the code. Convert messages and passwords to byte arrays as soon as you get them, preferably using a Unicode encoding to support foreign languages, and then only work with byte arrays for everything until you finally want to output the encrypted message where you probably want to Base64 encode it. Especially I did not understand why you have these ugly methods in ByteHelpers when you already have Encoding.GetString() and Encoding.GetBytes() and you are using them in some places.

It's not that AES-CBC in itself is insecure, but without message authentication you are vulnerable to chosen ciphertext attacks.

I will fix this issue and release a new version.

Reply on news.ycombinator.com