Repository navigation
bug report: docker-data/dms/config/postfix-accounts.cf is world-readable聽#4436
Description
馃摑 Preliminary Checks
- I tried searching for an existing issue and followed the debugging docs advice, but still need assistance.
馃憖 What Happened?
I recently installed DMS and created one account for myself using setup add email. (v15.0.1 at time of deployment, since upgraded to v15.0.2). I took a peek at my docker-data/dms/config/ and noticed that the password hash file (postfix-accounts.cf) was world-readable:
$ stat config/postfix-accounts.cf
[...]
Access: (0644/-rw-r--r--) Uid: ( 0/ root) Gid: ( 0/ root)
[...]
Since this is the (hashed) password file, I would have expected it to have restrictive permissions. I have other (unprivileged but not containerized) services running on this machine that shouldn't be allowed to read password hashes.
I manually chmoded it 0640 and restarted DMS with docker compose up --force-recreate and it seems to work still, so I assume it doesn't actually need to be world-readable. If that's true, please consider tweaking the setup tool to create this file root-only (e.g. umask 0027) because that would be a safer default. Thanks!
馃憻 Reproduction Steps
- Install DMS as per https://docker-mailserver.github.io/docker-mailserver/latest/usage/
docker exec -it mailserver setup add email user@example.comstat ./docker-data/dms/config/postfix-accounts.cf
馃悑 DMS Version
v15.0.1
馃捇 Operating System and Architecture
Debian 12 (bookworm) arm64
鈿欙笍 Container configuration files
馃摐 Relevant log output
Metadata
Metadata
Assignees
Labels
Type
Projects
- Status
Activity
georglauterbach commentedon Apr 1, 2025
@casperklein could you take a look please?
casperklein commentedon Apr 4, 2025
I can confirm what OP says. If we want to change that behaviour, we could add
umask 027to thesetupscript. That would effect anysetupcommand. Or we could add the newumaskjust toaddmailuser.BTW: Other files like
postfix-virtual.cfare also world readable. Someone might consider available aliases also as confidential.georglauterbach commentedon Apr 5, 2025
I'd probably start with adding
umaskto individual scripts 馃憤馃徏 And including the aliases is a good idea.polarathene commentedon Apr 5, 2025
How's that differ vs chmod?
umaskmakes more sense when you're certain the default022isn't desirable. It could otherwise potentially break things? (/var/mailwith LDAP comes to mind maybe?)I suppose it's rare that we need non-owner/group users to have access to files, so are we changing this more globally with
chmodcorrections after if needed?Might as well just set
umaskfor the entiresetupCLI then, and once instart-mailserver.shandcheck-for-changes.sh? (might need to consider any relevant cron jobs/services too?) Or justchmodon a case-by-case basis as we have been 馃casperklein commentedon Apr 6, 2025
If we add
umaskonly toaddmailuser, there should be no side-effects. The only file that is touched is/tmp/postfix-accounts?polarathene commentedon Apr 6, 2025
Yeah presently. If I ever get around to reviving the encrypted mailbox feature PR I have buried somewhere that'd come into play too I think.
Only concern with
umaskis our usage of many bash scripts and how there isn't a proper import system for that, so if we add one for this, and then do it again somewhere else in future, we might be more prone to a caveat where we're troubleshooting something and eventually backtrack to discover it was becauseumaskwas adjusted (or having to think about that).I'm generally iffy about global state lingering like that, but I can see why you might prefer that approach since it can be slipped into the
addmailuserscript directly, whereaspostfix-accounts.cfcreation is abstracted away into the database management scripts I wrote (which coverspostfix-virtual.cffor aliases and other configs with secrets like one for relay hosts).I've not checked if it'd work, but
umask 027could be set here I guess?docker-mailserver/target/bin/setup
Line 92 in 23bb1c8
The reason I also suggested for
start-mailserver.sh/check-for-changes.shis you'll find those will call stuff like this:docker-mailserver/target/scripts/helpers/aliases.sh
Lines 9 to 12 in 23bb1c8
Which while not the user config volume location, is the equivalent real config we map that content to, which should equally be locked down?
But like I said, at a broader scope using
umaskinstead ofchmodon individual files will be less obvious on impact and may lead to some subtle bugs. I don't think our test suite presently has wide enough coverage to be confident in it passing as not introducing breaking changes 馃casperklein commentedon Apr 7, 2025
Same here. To address the security concerns, one could remove the group rwx permission from the parent config folder (not tested if that breaks anything else). That way, all config files would be "protected".