[PM-29484] Add support for running rootless and with readonly filesystem #358
Conversation
|
|
|
Bump. @tangowithfoxtrot could you please approve the workflow runs? |
|
Maybe @mandreko-bitwarden or @vgrassia can help? |
|
Bumping again. Maybe @pixman20 can approve the workflows? |
|
How about @michalchecinski or @Eeebru ? |
mandreko-bitwarden
force-pushed
the
main
branch
from
aaccc4b to
d7566f2
Compare
|
|
Great job, no security vulnerabilities found in this Pull Request |
|
@kaysond / @tangowithfoxtrot: what's the current state of this PR? I'm deliberately waiting to run Bitwarden rootless 🤓 |
Looks like it needs to be rebased, which I'm happy to do, but doesn't seem clear that it's going to be merged... If the maintainers commit to merging it I'm happy to update it. Otherwise don't really want to spend more time. |
djsmith85
changed the title
djsmith85
left a comment
djsmith85
left a comment
There was a problem hiding this comment.
Hi @kaysond, thank you for your contribution 🎉 ❤️
With the recent announcement of bitwarden-unified being out of beta, it was also decided to change the name to bitwarden-lite. This unfortunately has caused a ton of conflicts with your changes.
As you mentioned previously you'd be up to rebase, if you hear back from Bitwarden. I spoke with someone from @bitwarden/dept-bre and product management, and they think this is a great addition. Once you have updated the PR, this is ready for review by @bitwarden/dept-bre and possibly also @bitwarden/team-platform-dev.
Please let us know if you are still interested in contribution to this PR.
Thanks again for taking the time to contribute and adding a highly requested feature!
Kind regards,
Daniel
Sure, I'll rebase it when I have a chance and let you know. |
|
Rebase completed by blindly re-applying the patches to the new dir and fixing some obvious issues. Still need to test. |
|
Not wanting to hijack this - but would you have plans to implement rootless & read-only to the 'classic' Bitwarden deployment method too? I prefer the 'original' approach as it fits better to the best-practises of how docker/containers were designed. |
No, sorry. That's a much more involved project covering many Dockerfile and entrypoint.sh files (see https://github.com/search?q=repo%3Abitwarden%2Fserver%20Dockerfile&type=code). You could apply the same strategy as is used here, though. |
|
@djsmith85 - this is ready for review. Tested with server v2025.12.0 I'd like to add a github action to run shellcheck on the entrypoint script. It's not clear to me where that should go, though, since it seems you're using azure. |
|
Hi @kaysond, thank you again for you contribution! We'll do some internal testing and get this reviewed.
I've also created a task to look at adding this. |
6 participants
🎟️ Tracking
Fixes #247 and bitwarden/server#2903
📔 Objective
Support running the unified container fully rootless and/or with a readonly root filesystem. See the commit message for a detailed listing of the changes.
Generally, the strategy is to move everything into
/etc/bitwardenin the container, which must be bind-mounted out to the host where the permissions can be managed (specifically, the user specified must have r/w). There are a few files created by the entry point that are needed in other locations. These are addressed with soft links in the image. supervisord and nginx pid/temp/etc files are moved to /tmp, following convention.Note that I left the PUID/PGID environment variables and behavior for the sake of not introducing breaking changes. However, I would strongly urge you to remove them (I can do that in this PR if you'd like). Though it can be convenient to start as root to set everything up then drop privileges for running the actual service, it needlessly increases the attack surface. Having a single directory to chmod/chown on the host is very simple. Given the importance of security for a password manager, I updated the example docker compose file and settings to be the most secure ("true" rootless, read only filesystem, no new privileges).
Also note that not every configuration will work. For example, if you run with PUID/PGID, but want a read only file system, the container fails trying to add the group because it can't write to /run. (Using
user:works fine, though, because group/user creation is skipped). This is a another argument for removing the env vars.Built and tested (briefly) using bitwarden/server@27606e2. I'd encourage further testing before merging.
📸 Screenshots
⏰ Reminders before review
🦮 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 confirmed issue 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