-
Notifications
You must be signed in to change notification settings - Fork 629
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
Proposal: Add ForceMunlock
option
#760
Comments
I really think the upper applications (e.g. OpenBao in this case) should resolve this issue instead of bbolt.
One workaround proposal for Openbao:
Refer to https://man7.org/linux/man-pages/man2/mlock.2.html,
|
@ahrtr said (and thanks for the reply!!):
I'm not quite sure I follow how this'd work. The first mlockall would lock all presently memory, sure, but wouldn't the third one mlock bbolt as well? I think it'd need to be But if you ever closed+reopened bbolt afterwards, this new database would be covered under Or, if you open the database and then update it, with the above pattern, wouldn't the bbolt db get mlock'd under the first I think this means, if we'd want to do this properly, we'd have to push mlocking into every area of OpenBao, explicitly, since mlockall will chance locking too many things, and chance not locking others? I do agree in general that the mmap may fail before munlock can be called, so I agree it isn't an ideal solution... So perhaps the real solution is push bbolt into a separate process and mlockall only the main server process? |
Yes, my proposal also has significant limitations. You must open bbolt after calling
Yes, it's also a solution, but it may need big refactoring/change on upper applications. |
Maybe I need a quick reality check here with a very stupid set of questions:
are there really people that do run Vault-like software on unecrypted disk drives? How likely is that you store the bbolt file on an encrypted drive, but not your swap? Who still uses swap in 2024? 🤔 Just to also leave something constructive, what about we're exposing an mmap interface where you could implement your own locking/unblock around the actual mmap syscalls? |
Thanks @ahrtr. I agree that this is not the most elegant design, and that in principle separating lock from unlock like this shouldn't happen. I wanted to suggest it as an upstream option though because it might be an emergency escape valve for others. Vault was a professionally developed piece of software; if they can stumble into this design flaw, so might other developers. So add the option but put a big scary warning in the docs that using this is a code smell; it's there to help you while you refactor your app. But I understand if you don't want this in upstream at all. @cipherboy if this won't be added upstream, what do you think about patching a temporary fork of bbolt? I still think that getting munlock into
I'm a new contributor to the project so I don't think my opinion is authoritative, but I would think that, especially as the free community fork, the OpenBao project would want to have safe defaults for as many possible systems as could be thrown at it. I believe unencryped swap is still the default in several Linux distro; Arch for example. |
@ahrtr said:
Indeed, but I think it might be the only viable alternative at the minute. In the Raft backend, bbolt is used as the underlying storage and there tend to be lots of read/write operations concurrently (well, only a single writer obviously). We're working on pushing through transactional storage as well, which use bbolt read-only transactions, so operations will need to be concurrent with memory allocation of secrets. Databases can grow rather large as well (I'm aware of 100s+ GB in production IIRC). @tjungblu said:
Besides what @IohannesArnold has mentioned above, I think the other point is OpenBao encrypts entries into bbolt. The underlying disk has lower priority for being encrypted. Not that its bad to encrypt disks, just that perhaps in some scenarios it is hard to do securely or not done by default... How can you tell (e.g., as a pod in kubernetes) if your workload is running strictly on encrypted storage? My 2c., but I think overall security posture from using mlock benefits more users, regardless of whether or not their underlying disk is encrypted or not. (as an aside, the threat model of host compromise is outside the scope of OpenBao). |
Based on discussion above, can we close this ticket, since there is no any action from bbolt side? |
Yes, thanks for your time and consideration. |
Bbolt has its own
Mlock
option which will make call mlock on the mmapped db file. But mlock can also be active if bbolt is used in a process that had previously calledmlockall(MCL_FUTURE)
. If that command was previously called by the program that then opens a bbolt db, the entire mmapped db will be paged into memory immediately upon creation of the mmap, even if the bbolt-specificMlock
option was not used.This might not be desired. In my case, I'm writing on behalf of OpenBao, a community fork of HashiCorp Vault. Vault has an mlock setting which, if enabled, simply calls
mlockall(MCL_FUTURE)
early in the program execution. This then causes it to run into the above problem if it uses its embedded bbolt storage, so upstream Vault docs tell users to disable mlock in this case.We want to fix this better in OpenBao. The most correct fix would be to not call
mlockall
but only callmlock
on the specific regions of memory that contain sensitive data. That's too deep of an overhaul for this project right now though. Another route to fix this would be if bbolt could add an option to munlock the db mmap immediately upon creation. Then, while the rest of OpenBao memory would be mlocked, the pages of the bbolt db mmap would be free to be evicted back to disk.This should be little more that a +10 or so PR. The core of it would be adding to ~bbolt/bolt_unix.go:61:
I almost opened this as a PR, but ended up starting with an Issue in case someone wanted to discuss design or naming more. What do you think?
The text was updated successfully, but these errors were encountered: