WIP: Implement in-memory encryption for reducing risk of swapping out sensitive information #3055
Conversation
phoerious
force-pushed
the
feature/memory-protection-v3
branch
4 times, most recently
from
1834b55
to
ad74e24
Compare
|
Just a question, is it possible to add in-memory encryption to notes as well? People usually store some sensitive information there as well like backup TOTP codes, account recovery codes, secret question answers, account related information etc. |
|
Theoretically, we could encrypt everything. It's just a matter of performance. Be aware though, that this kind of encryption is really only to reduce the risk of stuff being swapped out in plaintext. |
|
Is there some way you're able to check if performance is still good enough if say you do encrypt everything? I assume for most people that use KeepassXC they would be willing to give up some performance for this kind of security defense in depth. |
|
It could certainly be an option under the security settings. |
|
KDBX has metadata fields that define which fields to protect by default. We respect those settings, but have no GUI for modifying them. |
This reduces the risk of sensitive information being swapped to disk. It is NOT a guarantee that no sensitive information can be extracted from the process memory (we have other permission-based countermeasures for that in place). Passwords and other data still need to be accessed and shown in plaintext at some point and there is no way to do that without having a decrypted copy in memory. However, this patch reduces the risk of long-lived objects being swapped to disk in plain text from where they may be recoverable by an attacker. Encryption only applies to fields marked as "Protected" (which is the default for the password field) and attachments. Attachments are always encrypted, independently of whether they are marked as "Protected", since there is no GUI option to set this flag and we can safely assume that attachments very often contain private keys and other highly critical information.
droidmonkey
force-pushed
the
feature/memory-protection-v3
branch
from
ad74e24
to
1c2b8cb
Compare
|
This seems to me like a bogus security measure: you can't avoid swapping sensitive data completely but you will complicate code base which can lead to bugs. Shouldn't OSes have some mechanisms to avoid swapping sensitive memory pages? At least Linux has mlock(2). |
|
We would have to mlock the whole process memory. Encryption reduces the risk that swapped parts are recoverable in plain text. OpenSSH has implemented similar measures against Spectre side-channel attacks. The added complexity is very minimal. This patch just needs to be updated to reflect the current develop branch. |
Why? At least you can mlock only pages which contain data (not code). On the other hand, keepassxc eats not so much memory — mlock-ing even whole process wouldn't cause many troubles. |
|
We have no control over Qt's memory allocation. |
|
I am closing this until its ready. I think 2.6.0 is big enough refactor as it stands, lets not introduce more entropy to our changes with this. |
This PR is work in progress and not ready for production yet.
I am targeting this at 2.5, but it won't merge cleanly until release/2.4.2 has been merged back into develop.
Type of change
Testing strategy
Tests are failing at the moment. New tests will be added.
Checklist:
-DWITH_ASAN=ON. [REQUIRED]