mcndt / obsidian-quickshare

📝 An Obsidian plugin for sharing encrypted Markdown notes on the web. Zero configuration required.
MIT License
249 stars 9 forks source link

[Security] Static IV during encryption/decryption #21

Closed stypr closed 1 year ago

stypr commented 1 year ago

Came from https://mcndt.dev/posts/how-to-e2e-encryption/ 👋🏻

Describe the bug

Currently the code sets the IV to static value of 0s, and this is considered to be insecure.. Consider randomizing your IVs.

https://github.com/mcndt/obsidian-quickshare/blob/73733c0292cb3f0d6775c69c734e80c690932777/src/crypto/crypto.ts#L45-L49

https://github.com/mcndt/noteshare.space/blob/f84ddba528b73c160a16b707ede064b752d3528a/webapp/src/lib/crypto/decrypt.ts#L54-L59

Also, please consider reading https://security.stackexchange.com/a/17046 with regards to secure usage of AES-CBC on your service. I honestly think it's better off to do something with GCM than with CBC mode.

mcndt commented 1 year ago

Hi!

I appreciate your work vetting the security of the quickshare plugin.

I chose to use a static IV of 0 because keys are never reused. Are there still vulnerabilities in that case? Would love to learn more.

I will consider making a v3 implementation of the crypto suite using these recommendations (switch to GCM, #20 random keys) soon.

stypr commented 1 year ago

I assume it could be reasonably safe since the keys are being reused only for once, at least for now.

However, if the plaintext of the first block is known, the attacker can still bruteforce to find out the value of the key. (by rainbow table or bruteforcing) Unfortunately, I also assume this kind of attack scenario to be not that practical since the key is in 256bit.

I think there's only encryption of markdown text for the current specification, so it's very unlikely that the attack will be successfully achieved. But if the project is planning to extend to contain files or images, the plaintext of first block could be known because of the image headers, which makes it possibly easier to find out the key of the encrypted content.

Wouldn't it be better off to share the randomized IV along with the decryption key?

Could be also better to read this one as well : https://crypto.stackexchange.com/questions/8600/why-should-i-use-an-initialization-vector-iv-when-i-have-unique-keys

mcndt commented 1 year ago

Yeah when I update the encryption I will do so.

mcndt commented 1 year ago

Shipped in version 1.0.2!

Feel free to have another look at the code if you want to check again for problems.

If I may ask, how did you stumble across my blog?

stypr commented 1 year ago

I will check later when I have time. I just checked your post from reddit. Thanks for updating!