Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check that unauthenticated plaintext is not leaked from OpenSSL #70

Open
Demi-Marie opened this issue Jul 5, 2020 · 4 comments
Open
Assignees
Labels
security Pull requests that address a security vulnerability

Comments

@Demi-Marie
Copy link

If it is leaked, it is a security vulnerability. The entire plaintext must be buffered (either in memory, or in an anonymous and/or inaccessible temporary file) until the authentication tag can be checked.

@timvisee
Copy link
Owner

timvisee commented Jul 7, 2020

Thanks for the heads up. I'm unsure what you're targeting here though, my knowledge on this subject is somewhat lacking. Is this also applicable for crypto usage in ffsend?

Are you saying that the full plaintext (in ffsend's case, a file that is uploaded) must be encrypted in one shot? Because otherwise the plaintext may be retrieved in some way by a 3rd party while uploading it in a (crypto-) streaming manner?

@timvisee timvisee self-assigned this Jul 7, 2020
@timvisee timvisee added the security Pull requests that address a security vulnerability label Jul 7, 2020
@Demi-Marie
Copy link
Author

Streaming encryption is fine. The problem arises when decrypting. If the plaintext is revealed before the authentication tag is checked, the system is vulnerable to a chosen-ciphertext attack, in which an attacker sends forged messages and observes the decrypted plaintext. Without authentication, an attacker can then infer the plaintext of a message of their choice, even though the victim will not reveal it willingly.

To prevent this attack, the plaintext must be kept secret until the authentication tag has been checked. If the authentication check fails, the forged plaintext must be securely erased. This is very difficult with standard APIs, in which decryption and authentication are done in parallel, unless the entire data is stored in memory.

There are a few solutions. One is to compute the authentication tag as the data is being streamed to disk. If the tag does not match, the file (which is still safely encrypted) is simply deleted. If the tag does match, the file can then be safely decrypted, since it cannot be a forgery.

However, this runs into another problem: many AEAD ciphers have limits on how much data they can encrypt at once. For AES-GCM, I believe that this limit is 64GiB - 16B (but don’t quote me on it!). libsodium’s crypto_secretstream API solves this by breaking the file into chunks, each with its own authentication tag. Each chunk is small enough that it can trivially be buffered in memory. It also does further checks to ensure that an attacker cannot reorder or delete chunks without being detected. And it works for any reasonable message size.

@timvisee
Copy link
Owner

timvisee commented Jul 8, 2020

Thanks a lot for the thorough explanation!

For Send V3 we actually use ECE, which basically does decrypt the ciphertext in AES-GCM chunks. If I recall correctly parts of these chunks aren't streamed and are authenticated in full before written to the disk. So I don't think there's a problem here, but I should confirm that.

For Send V2 however, the ciphertext is streamed through AES-GCM and stored before it's authenticated. So this is where the problem lies that you're mentioning. This is also what I'd like to have streaming AES-GCM support for in ring. V2 is, as far as I know, never really used though. It's the old protocol Send used back in the day before it was even released. I kept support for this in ffsend for anyone that might host an old instance themselves. I probably want to deprecate support for this in the near future because of it. In fact, current ffsend builds use the ring crypto backend by default, and this has V2 disabled already.

I understand libsodium might provide better crypto solutions. I'm however limited to what crypto methods Mozilla chooses in their Send platform.

So to summarize, I think depreciating Send V2 support is the right solution here.

@Demi-Marie
Copy link
Author

Removing v2 support seems like the best option. I used libsodium as an example of how to do things right, not because it is the only right way to do things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Pull requests that address a security vulnerability
Projects
None yet
Development

No branches or pull requests

2 participants