-
Notifications
You must be signed in to change notification settings - Fork 10
[PM-18100] Add mlock
and memfd_secret
implementations
#125
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
base: main
Are you sure you want to change the base?
Conversation
Great job, no security vulnerabilities found in this Pull Request |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #125 +/- ##
==========================================
+ Coverage 68.98% 69.19% +0.20%
==========================================
Files 211 214 +3
Lines 16112 16319 +207
==========================================
+ Hits 11115 11292 +177
- Misses 4997 5027 +30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e4906dc
to
0aa0b89
Compare
0aa0b89
to
c52afa4
Compare
mlock
and memfd_secret
implementationsmlock
and memfd_secret
implementations
This comment was marked as resolved.
This comment was marked as resolved.
|
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-18100
📔 Objective
Implement some new backends for the KeyStore that use the OS memory protections:
memfd_secret
: On newer Linux only, but should protect any content in the store from being read from external programs, even if running as root. It also provides some minor protection against malicious kernel modules, but that can be bypassed.mlock
: Supported on Unix. Doesn't provide any real protection from reads, but ensures that the keys aren't swapped to disk. Note that Windows also supports a similarVirtualLock
API, but I've had some trouble with the crates depending on an olderwindows
crate, so it's not enabled for Windows yet.Note that for this implementation to be useful, we need to revert the boxing of keys so they are stack allocated. This would mean either not exposing them publicly, or having a separate internal key type.
This requires a MSRV bump to 1.80 for the use of LazyLock, but #256 requires 1.82 anyway.
For this implementation we're using the
hashbrown
crate, which allows us to create a HashMap that uses a custom allocator. I've implemented a custom allocator for the two protections mentioned above.To ensure that this works correctly, I've implemented a simple fuzzing test that executes thousands of random operations and compares the results and contents of these new backeds against the basic rust hashmap that we're using now.
⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes