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 ? |
* Use supervisord's built in support for reading env vars * Move all log files to /etc/bitwarden/logs * Move all pid files and sockets to /tmp/bitwarden * Move nginx temp files to /tmp/bitwarden * Clean up some shellcheck errors in the entrypoint * Only chown files that need it (when running root; report the list otherwise) * Change the default PUID/PGID variables to 911 (using 1000 is a security risk as it's the default user for many distros and often isn't unprivileged) * Use softlinks in the image to point to files generated in the entrypoint * Update docker compose and settings examples to be more secure * Add EXPOSE to Dockerfile
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. |
3 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