mirror of
https://github.com/laurent22/joplin.git
synced 2024-12-30 10:36:35 +02:00
43 lines
5.1 KiB
Markdown
43 lines
5.1 KiB
Markdown
---
|
|
url: https://www.patreon.com/posts/joplin-informal-35719724
|
|
published_at: 2020-04-06T21:42:54.000+00:00
|
|
---
|
|
|
|
# Joplin informal encryption and security audit results
|
|
|
|
Joplin encryption, and in particular the E2EE system used during synchronisation, was recently audited by Isaac Potoczny-Jones, CEO of [Tozny](https://tozny.com) and this is what he had to say:
|
|
|
|
> I was looking through your encryption implementation for Joplin and I have a few comments and concerns. I don't see anything that I \*know\* is a critical issue, but there are a number of choices and weaknesses that I'd like to lend you some advice about.
|
|
|
|
### OBC2
|
|
|
|
> OCB2, the chosen multi-block cipher mode has had some weaknesses identified in the last few years. I don't know this mode well since it's not a NIST-approved mode, but here's a paper on the topic. I get the impression it's not considered a good choice anymore. [Source](https://pdfs.semanticscholar.org/bb95/0d82fd390e732f71d8320530994bfb6d2529.pdf)
|
|
|
|
We indeed had been notified about this issue by another cryptographer and had been preparing migration to the more secure CCM mode. Migration for this is now complete in all the Joplin clients and a migration tool has been added to the Encryption config screen of the desktop application. In particular you can perform two operations:
|
|
|
|
- Upgrade the master key: This will convert the master key encryption to CCM
|
|
- Re-encryption: With this tool, you can re-encrypt all your data using the new encryption method based on CCM. Please follow the instructions on this screen and note that this process can take quite a bit of time so it's better to plan for it and run it over night. It is not entirely clear how the OBC2 flaw can be exploited but it is best to upgrade your data as soon as possible.
|
|
|
|
### Unnecessary key expansions
|
|
|
|
> Running key expansion on a random key: Your encrypt function uses either 1k or 10k rounds of key derivation. The goal of this is to reduce brute-force attacks against user-chosen passwords. This function appears to me to be used for both password-based key derivation (at 10k rounds) \*and\* bulk encryption of data from a randomly-generated "master key" (at 1k rounds). The bulk encryption does not need the password expansion since the key is randomly generated (presumably with a cryptographically strong generator). I suspect this could be a major performance issue on the bulk encryption of raw data, so if you're finding encryption slow, this is maybe why.
|
|
|
|
This is more a performance than a security issue. Indeed, the previous encryption method was using 1,000 key expansion iterations every time a note was encrypted, which is unnecessary since the master key is already secured with 10,000 iterations. As a result the encryption algorithm has been changed to perform only 100 iterations when encrypting notes, which should result in faster encryption and decryption on the desktop, mobile and CLI applications.
|
|
|
|
### Unnecessary and potentially insecure master key checksum
|
|
|
|
> You make and store a checksum of the master password with SHA256 in addition to encrypting it. I expect this is because you need a way to tell if the user's password is correct. I've never seen this done before, and it has me concerned, but I don't know for sure that it's an issue. Thought I'd mention it anyway. [Source](https://crypto.stackexchange.com/questions/61915/can-i-hash-a-secret-key-and-used-the-hash-as-key-id). At least with CCM mode (and I think with OCB2) it shouldn't successfully decrypt if you have the wrong password.
|
|
|
|
A checksum was previously stored with the master key to verify that it is valid. This could potentially weaken the security of the mater key since, as mentioned in Cryptography StackExchange link, "in the standard model of hash functions there isn't a requirement that hash outputs not have properties that leak information about the input". It was also unnecessary since the decryption algorithm in use would fail if the key is invalid, so the additional checksum was not needed.
|
|
|
|
This has also been addressed by the new master key upgrading tool. If you have performed the upgrade, the checksum will be gone from your master key.
|
|
|
|
### Encrypting local secrets with a keychain service
|
|
|
|
> Now I did notice that you cache the plain text password in the database, which is a bit concerning, but I guess the security model of your encryption approach is that it happens during sync, not locally. The generally accepted approach \[to store secrets\] is to use a keychain service, which is available pretty much on all modern platforms.
|
|
|
|
Passwords are indeed cached locally, so that you don't have to input it again every time a note needs to be encrypted or decrypted for synchronisation. It is assumed that your local device is secure, which is why for now passwords were cached locally.
|
|
|
|
To improve security however, future versions of Joplin will use the system keychain whenever it is available. A [pull request](https://github.com/laurent22/joplin/pull/2861) is in progress to add this feature.
|
|
|
|
To conclude I'd like to thank Isaac Potoczny-Jones for conducting this audit and revealing these potential security issues. Joplin is now much safer as a result. |