resilar / sqleet

SQLite3 encryption that sucks less
The Unlicense
375 stars 55 forks source link

Leverage libsodium? #4

Closed tleyden closed 7 years ago

tleyden commented 7 years ago

Looks like a really nice library!

I forwarded this to a colleague who is an experienced user of sqlcipher, and his feedback was:

ChaCha20 and Poly1305 are both provided by the well-regarded libSodium library, so that could be used instead of the implementations here

resilar commented 7 years ago

We do not want external dependencies. They only make compiling for Android, iOS and embedded devices in general multiple times harder (the same goes for targeting Windows). sqleet's Poly1305 implementation is superior anyway because libsodium leaks secret key material to the stack:

https://github.com/jedisct1/libsodium/blob/d338ae951209aa85018cdd1d07fcc21c037f743c/src/libsodium/crypto_onetimeauth/poly1305/donna/poly1305_donna32.h#L32-L36 key material moved to heap @ st->r[0..4]

https://github.com/jedisct1/libsodium/blob/d338ae951209aa85018cdd1d07fcc21c037f743c/src/libsodium/crypto_onetimeauth/poly1305/donna/poly1305_donna32.h#L66-L75 and then copied to stack variables r0..r4 and s1..s4. The stack variables are never cleared. I have confirmed this experimentally and found out that libsodium leaks 2-4 halves of the secret key to the stack after a single poly1305 calculation.

sqleet/crypto.c:L180-184 shows how it should be done. Go tell the libsodium suckers to use sqleet's implementation instead. Btw, our ChaCha20 and PBKDF2-HMAC-SHA256 primitives are also rock solid ;)

snej commented 7 years ago

We do not want external dependencies. They only make compiling for Android, iOS and embedded devices in general multiple times harder

This is unfortunately true! On the other hand, I want brand-new homegrown crypto code even less than I want external dependencies. libSodium isn't the most mainstream crypto library, but it's got a solid multi-year track record behind it, and the original code is by djb.

libsodium leaks secret key material to the stack:

That's unpleasant. But it doesn't look as though you (or anyone) have reported this issue to libsodium, at least not in their Github issue tracker. Are you planning to, or should I do it?

Btw, our ChaCha20 and PBKDF2-HMAC-SHA256 primitives are also rock solid ;)

As I'm not a cryptographer and can't review your code, you're asking me to take this on trust. If I trusted everything people told me, I probably wouldn't be using encryption ;)

To be blunt, I have no idea who you are or what your experience level is. I have no idea whether that code is really just 3 days old, whether it's had any sort of peer review, whether it's even been unit tested.

This isn't an attack, I'm just being appropriately skeptical.

resilar commented 7 years ago

My post from reddit

I didn't bother to open issues on several bug trackers because I code & hack for fun and lulz. Reporting vulnerabilities takes time and effort, and it is very often met with "only leaks one half of the key", "attacker needs also a memory leak so this is not a problem", "give a real exploit for the latest or go away" kind of bullshit. Being edgy on Reddit probably gets bugs fixed faster anyway.

Lol @ jedisct1 refusing to fix the issue (just like I had predicted). This one is actually easy to fix, just zero out the memory allocated in the stack for the local variables. Register spilling (which I think he is mixing with the concept of local variables) is another problem but that is not what is happening here. Even that could be reasonably mitigated given a little thought.

The bug may disappear if optimizations are enabled and the compiler has enough registers to use so that all local variables can be held in registers. But that's not happening on any system I've tested (x86-32, x86-64, ARMv7) gcc optimizations enabled or not.

In the end, just remember that sqleet does not have the same secret key leak (you can all check this by setting a breakpoint in the end of poly1305 function and dumping the stack). Others can keep making excuses for why they do not have to fix it.

As I'm not a cryptographer and can't review your code, you're asking me to take this on trust. If I trusted everything people told me, I probably wouldn't be using encryption ;) To be blunt, I have no idea who you are or what your experience level is. I have no idea whether that code is really just 3 days old, whether it's had any sort of peer review, whether it's even been unit tested.

I totally get where you are coming from. I wouldn't trust me either. That's why I posted the code on Github - for the peer review. You can already establish trust by reading and auditing the code yourself.

snej commented 7 years ago

Um … OK. Have fun (I mean, "lulz") with your toy project, then.