Uninitialized nonce

[jedisct1]

Hi,

In the encryption function, it looks like the nonce is not initialized. So it uses whatever is left on the stack instead. And the security of AES-GCM is completely destroyed if a nonce is reused.

Also, AES-GCM can only safely be used to encrypt small messages. You need to use it within a construction such as CHAIN in order to safely encrypt long data streams such as arbitrary long files.

If everything fits in memory, I'd recommend just using crypto_secrebox() instead, along with a random nonce. For something that doesn't fit in memory, maybe consider blobcrypt.

[Alexis-D]

Thanks for the bug report, I just addressed the uninitialized nonce in e23ace0ea7ef434b0f87140857862d5303654938. As stated in the README.md this shouldn't be used for storing any real secrets, I was just playing with libsodium and trying to make the point that one should probably not write his own crypto (and you made my point with this issue!).

For the second part about AES-GCM message size, I'll link to this SO question for those curious Plain text size limits for AES-GCM mode just 64GB?.

[jedisct1]

You didn't invent your own crypto. You used an existing library, but still ended up with something insecure.

You didn't do anything wrong. What it actually means is that I need to improve the API, the documentation, or both so that this kind of mistake doesn't happen.

The main goal of libsodium is to make crypto easy and safe to use to everyone. Even if you're just quickly playing with it, it should build something secure. It's not the case yet, and the bug your code illustrates this very well.

So I have to go back to the blackboard.

Even though it doesn't include a password hashing function yet, it would be interesting if you could try playing with libhydrogen as well: https://github.com/jedisct1/libhydrogen

Libsodium is a bit too mature for its ABI to be radically changed. Libhydrogen is an opportunity to improve over the libsodium API in order to make it easier and safer to use. Among other things, there are no practical limits to the size of messages that can be encrypted, nonces are automatically generated, and even reusing nonces wouldn't have any security implications.

Feedback from developers who don't have much experience in cryptography is badly needed. I'd like this new API to be good, simple and useful to everyone. Eventually, this will serve as a basis for libsodium 2.0.

So feel free to play with it if you can. Your input would be very appreciated.

[Alexis-D]

Oh yeah, didn't mean to imply I invented my own crypto; just that it's safer/better for non-experts like me to reuse high level primitives rather than try to implement their own things using lower level functions (e.g. I use gpg/pass for my own secrets rather than the code in this repo).

The uninitialized nonce is definitely an oversight on my part, the documentation does initialize it. Though you're right, the API could probably be more fool-proof e.g. initializing the nonce in crypto_aead_aes256gcm_encrypt; I'm not sure what are the downsides of this approach, but I guess it's not too bad since it looks like that's what libhydrogen is doing.

I also went the manual route here, rather than using the crypto_secretbox_{,open_}easy pair of functions which I would have used if writing production code (this wouldn't fix the nonce issue, but would likely suffer less issues than trying get key derivation, encryption, etc right, on my own). Maybe the API shouldn't offer 'lower level' functions to begin with, as to avoid people shooting themselves in the foot? Another possible concern, should users even know about key derivation, e.g. a user might just expect to be able to do encrypt(ciphertext, m, mlen, pwd, pwdlen), maybe it's irrelevant if it's impossible to get key derivation wrong, I just don't know.

I'll try to take look at libhydrogen if I get some time.