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

Why is AES-128 used instead of 256 by default? Is that changeable? #7686

Closed
TheQuantumPhysicist opened this issue Jan 29, 2023 · 20 comments
Closed
Labels
bug It's a bug desktop All desktop platforms high High priority issues

Comments

@TheQuantumPhysicist
Copy link

Hello :)

Let me start by saying I tried to post this as a discussion in discourse, but links are not allowed there. This discussion is pointless without pointing to the code. So, here I am. So, I think this qualifies to be called a bug.

It's well-known that AES-128 is not "secure enough", because key-size is not equal to bit security level. It's quite common to find attacks that have O(sqrt(N)) complexity over time (or by spreading complexity over disk-space and time), which result in close to 64-bit security, which is not secure enough. No, it's not secure enough, which seems to be the default encryption.

Instead of going into these pointless discussions where nuance is ignored, people usually just use 256-bit AES or Chacha20, because it's always better to have higher security than argue about the bare minimum. There's virtually no harm in increasing key sizes of AES because hardware support of encryption is given, and Chacha20 is heavily hardware friendly. So why are we being stingey? I failed to find the option to do that in Joplin desktop.

Now AES-256-CCM seems to be supported. However, it doesn't seem to be the default. I tested syncing and it seems that the data uploaded to the sync server has "ks":128.

I would really appreciate telling me how to change the encryption algorithm to use SJCL4 from the code. Is that possible? All the code is there and ready to do it already.

If not, let's talk about changing key sizes of the default (changing to SJCL4 encryption method). It seems the software is well-prepared to solve this problem and encrypt keys with 256-bit keys. That's really wonderful and I call it very good design. Can that be done, anyway?

Thank you for the great software. Let's make it the best :D

@TheQuantumPhysicist TheQuantumPhysicist added the bug It's a bug label Jan 29, 2023
@laurent22
Copy link
Owner

I guess we could create a new config set for this. The 128 bits size is based on SJCL own documentation, but perhaps that doc also needs to be updated

@wh201906
Copy link
Contributor

There's virtually no harm in increasing key sizes of AES because hardware support of encryption is given, and Chacha20 is heavily hardware friendly.

I'm sorry to interrupt, but I wonder if SJCL supports hardware acceleration.

@TheQuantumPhysicist
Copy link
Author

TheQuantumPhysicist commented Jan 30, 2023

I'm sorry to interrupt, but I wonder if SJCL supports hardware acceleration.

Good point.

From the looks of it, it seems not. It has a custom software implementation.

But that doesn't really matter. Because:

  1. I'm quite sure most people are OK with a little more CPU usage for guaranteed security
  2. The only time I ever heard cryptography people discuss this problem is in legacy systems or embedded systems. No one argues whether AES-128 should be used for saving power on retail devices.
  3. We're encrypting very small files on smartphones, and even more restrictively, only when syncing new/edited files. This is the level of chat software. No one asks this question there. We shouldn't either.
  4. Smartphones are very powerful nowadays. Even those from 5-8 years ago are very powerful and can do all this in no time.

So, I don't think hardware support should have bearing on the decision to move to good encryption based on the usage of Joplin.

@wh201906
Copy link
Contributor

  • I'm quite sure most people are OK with a little more CPU usage for guaranteed security

  • The only time I ever heard cryptography people discuss this problem is in legacy systems or embedded systems. No one argues whether AES-128 should be used for saving power on retail devices.

I support your idea. We should use better cryptography method like AES-256 with little performance loss compared with AES-128. People who enable E2EE are tend to have better security.
I'm just doubting that if SJCL is still a good choice now.

  • We're encrypting very small files on smartphones, and even more restrictively, only when syncing new/edited files. This is the level of chat software. No one asks this question there. We shouldn't either.

  • Smartphones are very powerful nowadays. Even those from 5-8 years ago are very powerful and can do all this in no time.

You can feel the slow syncing process after attaching multiple or big files to a note. I guess the current encryption process is handled by a single thread with no hardware acceleration and the slowness is notable.

@TheQuantumPhysicist
Copy link
Author

If performance is an issue, and there's will to use another library, I would recommend using Chacha20poly1305 as an encryption algorithm. It's a stream cipher algorithm, very famous, and extremely CPU and software friendly. Just rotations and bit-shifts. Even without hardware implementation it's faster than AES.

The problem with AES is that it uses the ugly Galois fields, which can be though of as polynomials being operated on... just imagine multiplying (and even dividing) polynomials in every step... very CPU unfriendly and it even has too many issues that relate to timing attacks because of this (unrelated to Joplin as an application since Joplin encrypts offline), which is why hardware-based CPU implementations are good, because they're both fast and work at constant time, guaranteeing security in addition to speed. While software implementations are very slow because they need to artificially run in constant time to avoid timing attacks.

I'm not so familiar with SJCL library since I'm a Rust/C++ guy. But I can say with confidence that if performance is an issue, moving to Chacha20 should be on the table.

However

considering this is open-source, I don't want to burden @laurent22 or anyone with any additional work. Optimum performance is the least of my worries, personally. If we can just make this 256-bit by default by tweaking a few variables, I'll be happy and satisfied.

@tessus
Copy link
Collaborator

tessus commented Feb 2, 2023

Wrt perf, there might be WASM implementations available. They should be faster than JS code.

@wh201906
Copy link
Contributor

wh201906 commented Feb 2, 2023

libsodium.js?

@TheQuantumPhysicist
Copy link
Author

TheQuantumPhysicist commented Feb 2, 2023

libsodium.js?

Hello guys.

From the looks of it, libsodium.js seems like a great option for performance.

However, I wanna mention that this post is going in the wrong direction in my opinion. Not because I don't want chacha20 with libsodium... that would actually be amazing!

But because these are two completely different problems.

Problem No. 1: We only use 128-bit keys, and using 256-bit is the golden standard that we should follow.

Problem No. 2: We use a software implementation of AES. We should probably phase it out and move to something much, much more efficient because speed is becoming an issue, which seems to be chach20.

The reason I wouldn't want to mix these two missions is that these two have different requirements and execution schemes.

  • Going from 128-bit to 256-bit is very easy and will take an hour, at most, of the time of anyone familiar with the source code. Just add another encryption type, make it 256-bit (and please, oh, please, increase iterations to 500k, I'll explain later in this text), and make it the default. Then users reencrypt, and voila, we're done
  • Moving from AES to chacha20 is much messier. The current source code seems to heavily depend on the parametrization of SJCL, which can create a problem for backwards compatibility and can't be mapped to chacha20.

Allow me to elaborate on the complexity of the second plan.

It seems that, currently, the parameters from SJCL are taken directly from SJCL and written into the encrypted md files as json. Some parameters have nothing to do with stream ciphers, like "mode". The deserialization happens by directly deserializing that json into data, which may need to be upgraded to support new parameters. So we have to create our own abstraction layer.

Another thing people seem to be forgetting: KDF. I.e., moving from passwords to cryptographic keys.

Using an easy and nice library just means that you just put a password, and it'll spit out cipher text + parameters that you store and don't worry about. To decrypt, you just throw the parameters back with the cipher text into a function, and it'll decrypt. Easy.

If you're gonna use something so low-level like libsodium, it isn't even close to this. It'll need much more work. Here are a few things to consider that should be done.

  • Let's not forget the parametrization/abstraction layer that has to be updated to work with both SJCL for backwards compatibility and whatever new thing we'll come up with manually
  • KDF: Key Derivation Functions. Currently SJCL seems to use PBKDF2 with only 101 iterations... which is a disaster from a security point of view... literally. People are criticizing Bitwarden for having only 100k iterations... So we should be using something like Argon2 with good parameters where we stretch the password into a 32-byte key.
  • We have to do the IV ourselves. So we have to generate an initialization vector that's truly random, and use it in chacha20. Probably easiest thing in this story to be done.
  • Maybe we have to implement CTR block cipher-mode ourselves. Chacha20 is said to not be tested for more than 4096 bytes data as per its documentation. It may be a good security practice to implement CTR to encrypt further. This is important if we wanna test our encryption against pre-prepared test-vectors. This depends on how "certified", per se, the desired encryption is.

This is just the tip of the iceberg... and I would've happily implemented all this if Joplin was in Rust or C++ (I did implement this myself years ago in C++)... but it's not Rust or C++, so hopefully I can help if you wanna go that path.

Conclusion

  • Moving from 128-bit to 256-bit and moving to Chacha20 are completely different tasks with different plans and execution.
  • Moving to Chacha20 isn't as simple as it looks and there's tons of extra work in it that'll probably take weeks or months (?)
  • If we don't want to go through the manual work I mentioned, we should look into some library that does everything that SJCL does, which may or may not be easy to find, if we're looking for something good, trust-worthy, and with good performance.

Sorry for the long text. I hope I explained the situation well.

@tessus
Copy link
Collaborator

tessus commented Feb 2, 2023

I just added argon2id to the API of vaultwarden and the bitwarden clients use an argon2 wasm implementation.
I am pretty sure that there are more high level crypto libs available, so that we do not have to manually implement these high level funtions ourselves. Because this would certainly take a lot of effort. Not such much due to the level complexity, which is rather low I believe, but it's just a huge endeavor in general, because it touches many moving parts and the test cases alone will be plenty.

I agree, the first step should be to switch to AES-256 + increasing the KDF iterations closer to the OWASP recommendation.

@wh201906
Copy link
Contributor

wh201906 commented Feb 3, 2023

I agree, the first step should be to switch to AES-256 + increasing the KDF iterations closer to the OWASP recommendation.

I second that.
And thanks @TheQuantumPhysicist for providing so much useful information!

@laurent22
Copy link
Owner

laurent22 commented Feb 13, 2023

Regarding the number of iterations see here: https://github.com/laurent22/joplin/blob/e1db70d45e3ae48c59be89fb891f8e287f301206/readme/news/20200406-224254.md

I don't think we'll change this. But changing to 512 that might be fine

@TheQuantumPhysicist
Copy link
Author

TheQuantumPhysicist commented Feb 14, 2023

Regarding the number of iterations see here: https://github.com/laurent22/joplin/blob/e1db70d45e3ae48c59be89fb891f8e287f301206/readme/news/20200406-224254.md

I don't think we'll change this. But changing to 512 that might be fine

Even though it's against OWASP recommendation, I wouldn't complain too much about PBKDF2 iterations for two reasons:

  1. because I use very long passwords, and password lengths add exponential strength instead of linear, as in iterations
  2. It's a battle that we'll never win, because the number will just keep increasing. The right answer is Argon2 with configurable parameters. I'm not in hurry for that.

If we get 256-bit AES, I'm more than happy (I'm assuming you meant that with that 512 and it's just a typo).

@laurent22 laurent22 added enhancement Feature requests and code enhancements desktop All desktop platforms medium Medium priority issues and removed bug It's a bug labels Feb 15, 2023
@laurent22 laurent22 added bug It's a bug high High priority issues and removed enhancement Feature requests and code enhancements medium Medium priority issues labels May 31, 2023
@laurent22
Copy link
Owner

Let's bumped up the priority of this, and switch to AES-256. Normally the current algorithm should handle that transition without any issues

@TheQuantumPhysicist
Copy link
Author

TheQuantumPhysicist commented Jun 27, 2023

May I ask, which release version of Joplin will/does have this new 256-bit key scheme included in it? And how do I activate the new encryption? Do I simply un-encrypt then re-encrypt?

Edit: I disabled encryption, then enabled it again, and that did the trick.

@gavinr-maps
Copy link

Is there a way to check which version is being used?

@qua3k
Copy link

qua3k commented Jul 1, 2023

I think making cryptography changes like this are fraught with misunderstandings, especially when the issue author is not familiar with the subject. Some corrections:

The problem with AES is that it uses the ugly Galois fields, which can be though of as polynomials being operated on... just imagine multiplying (and even dividing) polynomials in every step... very CPU unfriendly and it even has too many issues that relate to timing attacks because of this

Finite field arithmetic in GF(2^N) is actually really attractive for a number of reasons—addition is an xor and multiplication is fairly trivial as well. None of this is "CPU unfriendly"; the issues you're referring to are in table-based AES implementations where you leak information while doing a table lookup. Contrary to your statement, lookups are fast, and it's why most software AES implementations use one.

Maybe we have to implement CTR block cipher-mode ourselves. Chacha20 is said to not be tested for more than 4096 bytes data as per its documentation.

This makes no sense? You're conflating some documentation in djb's NaCl with how much data you can encrypt at once? You're linking to libsodium which is distinct from NaCl, and you're also talking about CTR for ChaCha20 for some reason? It's a stream cipher with a block counter...


To your original point: out of the points in the link of your initial post, only one of them is valid—a multi-target attack where an attacker cares about breaking any one plaintext out of many. This may or may not be relevant to Joplin.

AES-128 still offers substantial security against quantum computers; it's difficult (read: nigh improbable) to have such a long running serial computation such that you can reduce the security of AES-128 by half.

In the end, I think it's best if you consulted a cryptographer when you're dealing with cryptography, rather than strangers on the internet such as I who have no such background...

@TheQuantumPhysicist
Copy link
Author

In the end, I think it's best if you consulted a cryptographer when you're dealing with cryptography, rather than strangers on the internet such as I who have no such background...

Even though I'm not a cryptographer myself, I assure you that my opinions come from cryptographers in every point I mentioned, and I've been dealing with cryptography for over a decade. I even work with some cryptographers, daily, for creating secure protocols in everything I do. Everything you said is debatable at some level, and you're talking as if you just learned about cryptography two weeks ago (not saying this to offend you), as many of these discussions are nuanced, and your hostile attitude, by assuming the worst, doesn't show good faith towards understanding why these answers came up and from where, to get the point of "ah, yes, I see what you meant there and why this is a good idea in that case". I've spent long years in academia and software learning that people with bombastic and hostile attitudes towards others, assuming the worst, are never worth a discussion. Therefore, thank you for your opinion. The discussion is over.

After all, @laurent22 implemented the upgrade to 256-bit AES, thanks to him. Nothing else matters, and I'm not gonna create a debate stage on Github.

Have a great one.

@qua3k
Copy link

qua3k commented Jul 1, 2023

Everything you said is debatable at some level

How are any of the points I said debatable? Finite field arithmetic in GF(2^N) is easy to implement in both software in hardware, and I’m completely failing to understand your comment about CTR and ChaCha20, as well as how you seemingly dragged NaCl documentation into this and made it seem like you couldn’t encrypt more than 4096 bytes at a time. Furthermore, my comments on AES are not at all unsubstantiated; multi-target attacks are the only issue that would be slightly pertinent to Joplin, and the cryptographers I know all (as well as the ones at NIST running the PQC contest) hold the opinion that Grover’s algorithm is not going to drastically reduce the security of AES-128.

I never meant to come off as hostile, and I’m sorry if you took it that way, but I’m just surprised at your reasoning on what are seemingly well-known facts. I’d like it if you wrote meaningful responses pointing out inaccuracies in my arguments rather than belittling me and making hand-wavy claims that my points are “debatable at some level”.

@b-mc
Copy link

b-mc commented Jul 1, 2023

Is there a way to check which version is being used?

The only way I've found so far is to look it up per note. Open an MD file of your choice with a text editor and check the ks value from encryption_cipher_text. It will be either 128 or 256.

encryption_cipher_text: xyz{"iv":"xyz","v":1,"iter":101,"ks":128,"ts":64,"mode":"ccm","adata":"","cipher":"aes","salt":"xyz", "ct":"xyz"}

The existing notes which I haven't modified yet with the newest app version still use AES-128. Most likely a full re-encryption will change all to AES-256.

@wh201906
Copy link
Contributor

multi-target attacks are the only issue that would be slightly pertinent to Joplin

@qua3k Hi. Would you mind telling me why multi-target attacks might be a concern for Joplin?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It's a bug desktop All desktop platforms high High priority issues
Projects
None yet
Development

No branches or pull requests

7 participants