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
fix: Drop special bits from Postfix maildrop/
and public/
directory permissions
#3625
Conversation
Because `allowPrivilegeEscalation` controls SUID/SGID, we require it when postdrop is invoked.
The reason our permissions previously worked out as that in setups where SUID/SGID worked, the binaries used to place files in these directories already have SGID set; the current set of permissions makes less sense (as explained in this comment: #3619 (comment)) Since the binaries used to place files inside these directories alredy have SUID/SGID set, we do not require these bits (or the sticky bit) to be set on the directories.
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set. | ||
# Ref: https://github.com/docker-mailserver/docker-mailserver/issues/3619 | ||
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | ||
chmod 710 "${STATEDIR}/spool-postfix/public" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If maintainers think we should remove this altogether and just use the Debian defaults, I do not mind it either. But remember: these files are most likely in a persisted volume, hence, if we removed these lines, users would be stuck with the approach on master
. Keeping these lines here is safer for now; we could add a TODO comment about removing this in future versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actual review feedback comment (read + respond to this one)
Outdated
Why is my comment ref stripped away? It was more direct at conveying the information vs a long issue discussion?
If you want to replace the ref rather than add an additional one, perhaps link to a similar comment that communicates the intent to the maintainer/reader for context?
UPDATE: This series of review comments itself now provides a rather comprehensive overview of the topic. It may serve as a better reference link?
Question: What changed since the original #3149 PR to motivate this for you?:
- You commented recently there that
allowPrivilegeEscalation: false
caused the issue for you.- But that PR itself fixed the same problem you encountered correct?
- What is different now with the report in docs: Kubernetes requies
allowPrivilegeEscalation: true
#3619 ? Why did the existing approach not work for that k8s user like it had for you?
If maintainers think we should remove this altogether and just use the Debian defaults, I do not mind it either.
Keeping these lines here is safer for now; we could add a TODO comment about removing this in future versions.
You can't drop this to rely on Debian defaults.
They're not carried over into a fresh volume mount for these locations IIRC. That was the whole point we explicitly added them 😅
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set.
- Doesn't look like there is any SUID bit involved at all?
- Only
setgid
/ SGID on thepostdrop
binary. - Possibly also
postqueue
(it has SGID set too), but I'm not sure about it's involvement with either dir.
- Only
- I agree that these extra special bits (Sticky bit on
maildrop/
and SGID onpublic/
) don't seem to be necessary, at least for DMS (dropping them will however diverge from Debian and be inconsistent vs no volume mount for state).
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set. | |
# Ref: https://github.com/docker-mailserver/docker-mailserver/issues/3619 | |
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | |
chmod 710 "${STATEDIR}/spool-postfix/public" | |
# These permissions rely on the `postdrop` binary having the SGID bit set. | |
# Ref: https://github.com/docker-mailserver/docker-mailserver/pull/3625 | |
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | |
chmod 710 "${STATEDIR}/spool-postfix/public" |
NOTE: The sticky-bit on maildrop/
was likely historical from the previous PR referenced Postfix release notes. Debian just may never have adjusted to that on their end, or it may be for supporting third-party software like mailx
writing to maildrop/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference
All comments here below are a collection of references / notes.
I tried to splitting it out into several comments (easier to link to specific parts for future reference), and to make it easier to scan over by collapsing a fair amount of it.
Relevance of permission special bits
Relevancy of SGID and sticky bits for these locations based on prior PR comments:
Reference (click to expand)
- My comment from the original PR references Postfix's own SGID check script.
- UPDATE:
find
predicates breakdown from thispostfix check
script is covered in follow-up comment.
- UPDATE:
- The permissions we set in the original PR was also to mirror what Debian 11
post-install
scripts configured AFAIK?- Both @casperklein and myself confirmed that when a volume is not mounted to
/var/mail-state
(which previously lacked preserving the special bits during our startup script here), the permissions in the container for these locations were1730
+2710
. - The linked PR comment by @casperklein also mentions special bits are missing if these locations recreated during Postfix startup if they were deleted prior.
- While in another comment I reference
postfix
via Alpine installs:- Without a sticky bit (
T
/o1000
) onmaildrop/
. - I did not mention if SGID (
S
/o2000
) was omitted forpublic/
, but it was presumably missing too.
- Without a sticky bit (
- UPDATE: I think one benefit of the original PR was to be consistent regardless if a volume is mounted to
/var/mail-state
?
- Both @casperklein and myself confirmed that when a volume is not mounted to
- Additionally my earlier review comment on that PR cites Postfix source again:
Where SGID is implied (for.maildrop
+public
)- Similar for the
postdrop
executable (explicitly setso2755
). - UPDATE: Appears to be in regards to the intended group for SGID (
setgid_group = postdrop
) assigned to thepostdrop
+postqueue
executables.
setgid_group
setgid_group
is also used in post-install
with documented description:
Quoted snippets from referenced docs
setgid_group
- The group for mail submission and for queue management commands.
- Its numerical group ID must not be used by any other accounts on the system, not even by the
mail_owner
account
post-install
also referenced setgid_group
in docs for create-missing
, set-permissions
and upgrade-permissions
args:
create-missing
- Create missing queue directories with ownerships and permissions according to the contents of
$meta_directory/postfix-files
and optionally in$meta_directory/postfix-files.d/*
, using themail_owner
andsetgid_group
parameter settings from the command line, process environment or from the installedmain.cf
file.- This is required at Postfix start-up time.
set-permissions
- Set all file/directory ownerships and permissions according to the contents of
$meta_directory/postfix-files
and optionally in$meta_directory/postfix-files.d/*
, using themail_owner
andsetgid_group
parameter settings from the command line, process environment or from the installedmain.cf
file.- Implies
create-missing
.- This is required when installing Postfix from a pre-built package, or when changing the
mail_owner
orsetgid_group
installation parameter settings after Postfix is already installed.
upgrade-permissions
- Update ownership and permission of existing files/directories as specified in
$meta_directory/postfix-files
and optionally in$meta_directory/postfix-files.d/*
, using themail_owner
andsetgid_group
parameter settings from the command line, process environment or from the installedmain.cf
file.- Implies
create-missing
.- This is required when upgrading an existing Postfix instance.
EDIT: setgid_group
is also a main.cf
parameter, which sates postdrop
is the default and changing it would require running postfix set-permissions
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Postfix docs on maildrop/
dir and postdrop
command
I also referenced the Postfix docs from my prior PR comment:
Reference (click to expand)
-
Maildrop
master.cf
service +virtual_transport
(we dropped themaster.cf
config for this, so technically shouldn't be relevant to DMS?):Collapsed as probably not relevant
- The
vmail
userid as used below is the user thatmaildrop
should run as. This would be the owner of the virtual mailboxes if they all have the same owner. - If
maildrop
issuid
(see maildrop documentation), thenmaildrop
will change to the appropriate owner to deliver the mail. - Note: Do not use the
postfix
user as themaildrop
user.
- Configured via:
master.cf
service (directly run command)mailbox_command
(indirectly runs command) inmain.cf
(runs the command with UID + GID of intended recipient).
maildrop
process (appears to be Couriermaildrop
?) will usesetuid
/ SUID (Couriermaildrop
docs, security section references aborting delivery if sticky bit is set on recipient mail storage dir).- Debian
maildrop
package page implies that it uses SGID instead of SUID (which seems to be a fallback when sticky bit is not used on the mail directory, introduced in 2010 (v2.4
)?). v0.99.2
removed defaultg+rw
permissions on mbox files created when sticky bit was not set as it apparently was a potential exploit source.v0.76
notesmaildirmake
should not be installed with SUID or GUID bits set. By late 2001 withv1.3.5
,setuid
install was dropped?- The repo has a
README.postfix
, which I thought the Postfix docs might have been referencing formaildrop
, but this Debian patch states that file is quite outdated.
- Debian
- The
-
- Messages that have been submitted via the Postfix
sendmail(1)
command, but not yet brought into the main Postfix queue by thepickup(8)
service, await processing in themaildrop
queue. - Messages can be added to the
maildrop
queue even when the Postfix system is not running. They will begin to be processed once Postfix is started. - The
maildrop
queue is drained by the single threadedpickup(8)
service scanning the queue directory periodically or when notified of new message arrival by thepostdrop(1)
program. - The
postdrop(1)
program is asetgid
helper that allows the unprivileged Postfixsendmail(1)
program to inject mail into themaildrop
queue and to notify thepickup(8)
service of its arrival.
- Messages that have been submitted via the Postfix
-
SECURITY
The command is designed to run with set-group ID privileges, so that it can write to themaildrop
queue directory and so that it can connect to Postfix daemon processes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
postfix check
and find
predicates
Reference (click to expand)
find
command predicates check the following via Postfix postfix-script
(logic for postfix check
command):
- All files that are not of named pipe or socket type within the Postfix queue directory should have the same UID (
! \( -type p -o -type s \) ! -user $mail_owner
. maildrop
andpublic
dirs in the Postfix queue directory should have the SGID intended grouppostdrop
assigned (! -group $setgid_group
).postqueue
andpostdrop
commands should have world executable (chmod ugo+x
/o111
) and have SGID bit (chmod g+s
/o2000
) permissions (! -perm -02111
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference since I always forget this stuff 😅
# Permissions special bit example:
$ touch user group other
# SUID:
$ chmod u+s user
# SGID:
$ chmod g+s group
# Sticky-bit:
$ chmod o+t other
# Default 644 base permissions (`chmod u=rwx,go=r`)
$ eza --octal-permissions --long
2644 .rw-r-Sr-- 0 root 8 Nov 01:01 group
1644 .rw-r--r-T 0 root 8 Nov 01:01 other
4644 .rwSr--r-- 0 root 8 Nov 01:01 user
# Make world executable:
$ chmod ugo+x user group other
$ eza --octal-permissions --long
2755 .rwxr-sr-x 0 root 8 Nov 01:01 group
1755 .rwxr-xr-t 0 root 8 Nov 01:01 other
4755 .rwsr-xr-x 0 root 8 Nov 01:01 user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Permissions - Historical context
In the original PR, I referenced this mailing list link:
Reference (click to expand)
-
The Postfix maintainer states that
postfix set-permissions
may not be sufficient for Debian due to their downstream packaging? But emphasizes thatpostfix-files
conveys all permissions that Postfix requires to work correctly. -
An earlier comment at that link notes that
maildrop/
is leveraging SGID for access by related processes that may be run from a user that would otherwise lacks access into the directory. -
The original author message at that link also uses
postfix check
to verify permissionswhich are caught as incorrect(EDIT: My bad, it said everything was ok, issue was with third-party softwaremailx
). -
In the same original PR comment that I shared this mailing list link, I referenced Postfix release notes (many years older than mailing list discussion):
[snapshot-20020106]
This release modifies the existingmaster.cf
file. The local pickup service is now unprivileged, and the cleanup
and flush service are now "public".
Should you have to back out to a previous release, then you must
...
2)chmod 755 /var/spool/postfix/public
.
To revert to a world-writable mail submission directory,chmod 1733 /var/spool/postfix/maildrop
.So
chmod 755 public
(rwx-r-x-r-x
) +chmod 1733 maildrop
(rwx-wx-wx
) was the previous permissions quite a while back.pickup
andcleanup
services in our ownmaster.cf
are unprivileged 👍
Now we have these directory permissions:
public/
with710
only permits the group (g=x
) to enter the directory to access content inside. The group beingpostdrop
that leverages SGID bit on it's binary.maildrop/
with730
, additionally allows the group to write (g=wx
).- Programs like Postfix
sendmail
will usepostdrop
command (with its SGID permission bit) to write intomaildrop/
. - Additionally adding sticky-bit (
T
) tomaildrop/
would only be useful if users were part of thepostdrop
group to protect against deletion/edits of files that they have ownership of inmaildrop/
from other members of the shared group. But this is not the case; users temporarily are a member ofpostdrop
group via thepostdrop
executable viasetgid
(SGID). So no benefit within DMS setups?
- Programs like Postfix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conclusion
The original PR commit message notes that we chose to match what Debian was using, which would be consistent regardless of a volume mount (which this script requires to run).
maildrop/
doesn't need sticky bit (T
)
# Send mail as a different user (eg: As user `nobody`):
$ su -s /bin/bash -c 'sendmail -f from@example.test to@example.test' nobody <<< 'this is a test'
postdrop: warning: unable to look up public/pickup: No such file or directory
# Stored mail at `maildrop/` with permissions 644:
$ ls -l /var/spool/postfix/maildrop/
drwx-wx--T 1 postfix postdrop 4.0K Nov 8 05:15 .
drwxr-xr-x 1 root root 4.0K May 3 2023 ..
-rwxr--r-- 1 nobody postdrop 108 Nov 8 05:15 3B8FAB6DF
It's perhaps relevant if a third-party software was writing to this location, which is also when SGID is may be relevant (not a concern for DMS):
Reference (click to expand)
-
As the earlier referenced 2015 mailing discussion notes with
mailx
trying to write to this location.While
postfix check
shows no errors, when I runmailx
from the command line I get warnings about an inability to write to/var/spool/postfix/maildrop
.Postfix uses a very interesting trick of having the executables set the
GroupID when being run as the user, and this allows them to get into
directories when the user they are running as normally cannot. -
On the Debian mailing list in 2001, it was noted that
mailx
had a security hole allowing users use themail
group, so the packagers removedsetgid
bit (SGID).- It also mentions that Solaris used a world writable maildrop location, which required sticky bit (which apparently elimitates the need for
setgid
mail programs). - In reference to
/var/mail
, they also encourage it to have sticky bit assigned to significantly reduce the mentionedgid=mail
exploit. They suggest for Postfix with/var/mail
to use3775
, advising against2755
(SGID only).
- It also mentions that Solaris used a world writable maildrop location, which required sticky bit (which apparently elimitates the need for
public/
doesn't need SGID set (s
)
Postfix docs for master.cf
explain what private/
and public/
are for:
Private (default:
y
)
- Whether a service is internal to Postfix (pathname starts with
private/
), or exposed through Postfix command-line tools (pathname starts withpublic/
).- Internet (type
inet
) services can't be private.
Nothing about that seems to justify needing SGID then either? Especially since group permissions is execute only for the directory 🤷♂️
Additional context
In the prior PR, I noted that this location only contained unix sockets.
It should only be services defined in our master.cf
with private
column as n
, where most type
are unix
/ inet
+ 1 fifo
+ 1 unix-dgram
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: What changed since the original #3149 PR to motivate this for you?:
You commented recently there that
allowPrivilegeEscalation: false
caused the issue for you.
- But that PR itself fixed the same problem you encountered correct?
That's a good point, indeed.
- What is different now with the report in docs: Kubernetes requies
allowPrivilegeEscalation: true
#3619 ? Why did the existing approach not work for that k8s user like it had for you?
I cannot answer this question TBH, because I don't know the answer 😆 I can only state that the setup with this PR makes way more sense. I do not know why I stopped seeing this error, I can only guess: at some point, I started using a completely custom Postfix configuration - maybe this is the culprit.
If maintainers think we should remove this altogether and just use the Debian defaults, I do not mind it either.
Keeping these lines here is safer for now; we could add a TODO comment about removing this in future versions.You can't drop this to rely on Debian defaults.
They're not carried over into a fresh volume mount for these locations IIRC. That was the whole point we explicitly added them 😅
👍🏼
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set.
Doesn't look like there is any SUID bit involved at all?
- Only
setgid
/ SGID on thepostdrop
binary.- Possibly also
postqueue
(it has SGID set too), but I'm not sure about it's involvement with either dir.
Indeed, it's only SGID.
- I agree that these extra special bits (Sticky bit on
maildrop/
and SGID onpublic/
) don't seem to be necessary, at least for DMS (dropping them will however diverge from Debian and be inconsistent vs no volume mount for state).NOTE: The sticky-bit on
maildrop/
was likely historical from the previous PR referenced Postfix release notes. Debian just may never have adjusted to that on their end, or it may be for supporting third-party software likemailx
writing tomaildrop/
I am pretty sure too that the sticky bit is not required :)
PS: I am now running this on my production machine to see whether there are any issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 2 suggestions
- 1 question: What changed since the original bugfix: special bits for maildrop and public directory #3149 PR to motivate this for you?
I've heavily documented the topic in review comments for my own benefit while recapping on the prior PR. Should be helpful towards addressing: #3557 (comment)
@@ -190,7 +190,7 @@ spec: | |||
imagePullPolicy: IfNotPresent | |||
|
|||
securityContext: | |||
allowPrivilegeEscalation: false | |||
allowPrivilegeEscalation: true # required for SUID/SGID to work |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you look after the k8s docs, is this enough context to the user?
Would it be helpful to add extra context?:
allowPrivilegeEscalation: true # required for SUID/SGID to work | |
# Required to support SGID via `postdrop` executable | |
# in `/var/mail-state` for Postfix (maildrop + public dirs): | |
# https://github.com/docker-mailserver/docker-mailserver/pull/3625 | |
allowPrivilegeEscalation: true |
git blame
will handle that implicitly, so up to you.
Original feedback exploring possibility of improvement
I think this setting might be a bit of a red flag to some users as a workaround?
The equivalent for Docker appears to be opt-in via --security-opt=no-new-privileges
, although on a Docker host with SELinux enabled, that or similar restriction may already be default? 🤷♂️
- On non-root user currently it seems to clear capabilities by default.
- Recently a change was made for upcoming v25 release of Docker, and backported to bugfix releases of v23 and v24: Do not drop effective&permitted set moby/moby#45511 (review)
Is this config of any help?:
-
fsGroup
(NOTE: Support depends on volume driver)- By default, Kubernetes recursively changes ownership and permissions for the contents of each volume to match the
fsGroup
specified in a Pod'ssecurityContext
when that volume is mounted. - For large volumes, checking and changing ownership and permissions can take a lot of time, slowing Pod startup.
- You can use the
fsGroupChangePolicy
field inside asecurityContext
to control the way that Kubernetes checks and manages ownership and permissions for a volume.
- By default, Kubernetes recursively changes ownership and permissions for the contents of each volume to match the
-
Paired with a Volume Subpath:
Sometimes, it is useful to share one volume for multiple uses in a single pod.
ThevolumeMounts.subPath
property specifies a sub-path inside the referenced volume instead of its root.
fsGroup: set as group ID/SGID on the mounted volume & added as supplemental group ID
Note: ThefsGroup
option must be supported by the storage provider for the volume. All files and directories created in this directory inherit thefsGroup
ID through the special SGID bit.supplementalGroups: (additional) supplemental groups for shared group access; must be set in the
securityContext
on pod levelA note on fsGroup:
- The behavior of applying
fsGroup
by setting thefsGroup
and SGID bit (drwxrwsrwx
) on the mounted directory depends on the type of volume and its support of ownership management.- It differs for volumes managed by Kubernetes like
EmptyDir {}
and CSI volumes managed by a CSI driver.- By default, Kubernetes recursively changes ownership and permissions for the contents of each volume to match the
fsGroup
specified in a pod'ssecurityContext
when the volume is mounted.
If the two features make any sense to use together, this seems to provide a config example if helpful.
EDIT: I guess there is no granular "capability" that covers this? 😅
Use Case #3: Special Permissions (SUID, SGID)
- Set User ID (
setuid
) and Set Group ID (sgid
) are special permissions for executable files.- When these permissions are assigned to a file, the file to be executed assumes the privileges of the file's owner or group.
setuid
bit changes a program effectiveuid
(euid
) upon execution.
Actually, you seem to already have applied that via SETGID
? capability below?
setgid()
sets the effective group ID of the calling process.
If the calling process is privileged (more precisely: has theCAP_SETGID
capability in its user namespace), the real GID and savedset-group-ID
are also set.
So was allowPrivilegeEscalation: false
overriding allowing that capability? Or was it due to another process running instead of the root user / entrypoint? (EDIT: Overriding SETGID
capability)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up adjustment comment regarding vol subpath + fsGroup
Oh I see below that a volume subpath is already used. I was wondering if it'd be good for the maildrop
+ public
dirs if the fsGroup
feature caters to that with allowPrivilegeEscalation: false
?
Only issue then is the GID won't be static (deterministic) unless we create it before installing the postfix
package (like with ClamAV) I think?
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set. | ||
# Ref: https://github.com/docker-mailserver/docker-mailserver/issues/3619 | ||
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | ||
chmod 710 "${STATEDIR}/spool-postfix/public" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actual review feedback comment (read + respond to this one)
Outdated
Why is my comment ref stripped away? It was more direct at conveying the information vs a long issue discussion?
If you want to replace the ref rather than add an additional one, perhaps link to a similar comment that communicates the intent to the maintainer/reader for context?
UPDATE: This series of review comments itself now provides a rather comprehensive overview of the topic. It may serve as a better reference link?
Question: What changed since the original #3149 PR to motivate this for you?:
- You commented recently there that
allowPrivilegeEscalation: false
caused the issue for you.- But that PR itself fixed the same problem you encountered correct?
- What is different now with the report in docs: Kubernetes requies
allowPrivilegeEscalation: true
#3619 ? Why did the existing approach not work for that k8s user like it had for you?
If maintainers think we should remove this altogether and just use the Debian defaults, I do not mind it either.
Keeping these lines here is safer for now; we could add a TODO comment about removing this in future versions.
You can't drop this to rely on Debian defaults.
They're not carried over into a fresh volume mount for these locations IIRC. That was the whole point we explicitly added them 😅
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set.
- Doesn't look like there is any SUID bit involved at all?
- Only
setgid
/ SGID on thepostdrop
binary. - Possibly also
postqueue
(it has SGID set too), but I'm not sure about it's involvement with either dir.
- Only
- I agree that these extra special bits (Sticky bit on
maildrop/
and SGID onpublic/
) don't seem to be necessary, at least for DMS (dropping them will however diverge from Debian and be inconsistent vs no volume mount for state).
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set. | |
# Ref: https://github.com/docker-mailserver/docker-mailserver/issues/3619 | |
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | |
chmod 710 "${STATEDIR}/spool-postfix/public" | |
# These permissions rely on the `postdrop` binary having the SGID bit set. | |
# Ref: https://github.com/docker-mailserver/docker-mailserver/pull/3625 | |
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | |
chmod 710 "${STATEDIR}/spool-postfix/public" |
NOTE: The sticky-bit on maildrop/
was likely historical from the previous PR referenced Postfix release notes. Debian just may never have adjusted to that on their end, or it may be for supporting third-party software like mailx
writing to maildrop/
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set. | ||
# Ref: https://github.com/docker-mailserver/docker-mailserver/issues/3619 | ||
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | ||
chmod 710 "${STATEDIR}/spool-postfix/public" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference
All comments here below are a collection of references / notes.
I tried to splitting it out into several comments (easier to link to specific parts for future reference), and to make it easier to scan over by collapsing a fair amount of it.
Relevance of permission special bits
Relevancy of SGID and sticky bits for these locations based on prior PR comments:
Reference (click to expand)
- My comment from the original PR references Postfix's own SGID check script.
- UPDATE:
find
predicates breakdown from thispostfix check
script is covered in follow-up comment.
- UPDATE:
- The permissions we set in the original PR was also to mirror what Debian 11
post-install
scripts configured AFAIK?- Both @casperklein and myself confirmed that when a volume is not mounted to
/var/mail-state
(which previously lacked preserving the special bits during our startup script here), the permissions in the container for these locations were1730
+2710
. - The linked PR comment by @casperklein also mentions special bits are missing if these locations recreated during Postfix startup if they were deleted prior.
- While in another comment I reference
postfix
via Alpine installs:- Without a sticky bit (
T
/o1000
) onmaildrop/
. - I did not mention if SGID (
S
/o2000
) was omitted forpublic/
, but it was presumably missing too.
- Without a sticky bit (
- UPDATE: I think one benefit of the original PR was to be consistent regardless if a volume is mounted to
/var/mail-state
?
- Both @casperklein and myself confirmed that when a volume is not mounted to
- Additionally my earlier review comment on that PR cites Postfix source again:
Where SGID is implied (for.maildrop
+public
)- Similar for the
postdrop
executable (explicitly setso2755
). - UPDATE: Appears to be in regards to the intended group for SGID (
setgid_group = postdrop
) assigned to thepostdrop
+postqueue
executables.
setgid_group
setgid_group
is also used in post-install
with documented description:
Quoted snippets from referenced docs
setgid_group
- The group for mail submission and for queue management commands.
- Its numerical group ID must not be used by any other accounts on the system, not even by the
mail_owner
account
post-install
also referenced setgid_group
in docs for create-missing
, set-permissions
and upgrade-permissions
args:
create-missing
- Create missing queue directories with ownerships and permissions according to the contents of
$meta_directory/postfix-files
and optionally in$meta_directory/postfix-files.d/*
, using themail_owner
andsetgid_group
parameter settings from the command line, process environment or from the installedmain.cf
file.- This is required at Postfix start-up time.
set-permissions
- Set all file/directory ownerships and permissions according to the contents of
$meta_directory/postfix-files
and optionally in$meta_directory/postfix-files.d/*
, using themail_owner
andsetgid_group
parameter settings from the command line, process environment or from the installedmain.cf
file.- Implies
create-missing
.- This is required when installing Postfix from a pre-built package, or when changing the
mail_owner
orsetgid_group
installation parameter settings after Postfix is already installed.
upgrade-permissions
- Update ownership and permission of existing files/directories as specified in
$meta_directory/postfix-files
and optionally in$meta_directory/postfix-files.d/*
, using themail_owner
andsetgid_group
parameter settings from the command line, process environment or from the installedmain.cf
file.- Implies
create-missing
.- This is required when upgrading an existing Postfix instance.
EDIT: setgid_group
is also a main.cf
parameter, which sates postdrop
is the default and changing it would require running postfix set-permissions
.
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set. | ||
# Ref: https://github.com/docker-mailserver/docker-mailserver/issues/3619 | ||
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | ||
chmod 710 "${STATEDIR}/spool-postfix/public" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Postfix docs on maildrop/
dir and postdrop
command
I also referenced the Postfix docs from my prior PR comment:
Reference (click to expand)
-
Maildrop
master.cf
service +virtual_transport
(we dropped themaster.cf
config for this, so technically shouldn't be relevant to DMS?):Collapsed as probably not relevant
- The
vmail
userid as used below is the user thatmaildrop
should run as. This would be the owner of the virtual mailboxes if they all have the same owner. - If
maildrop
issuid
(see maildrop documentation), thenmaildrop
will change to the appropriate owner to deliver the mail. - Note: Do not use the
postfix
user as themaildrop
user.
- Configured via:
master.cf
service (directly run command)mailbox_command
(indirectly runs command) inmain.cf
(runs the command with UID + GID of intended recipient).
maildrop
process (appears to be Couriermaildrop
?) will usesetuid
/ SUID (Couriermaildrop
docs, security section references aborting delivery if sticky bit is set on recipient mail storage dir).- Debian
maildrop
package page implies that it uses SGID instead of SUID (which seems to be a fallback when sticky bit is not used on the mail directory, introduced in 2010 (v2.4
)?). v0.99.2
removed defaultg+rw
permissions on mbox files created when sticky bit was not set as it apparently was a potential exploit source.v0.76
notesmaildirmake
should not be installed with SUID or GUID bits set. By late 2001 withv1.3.5
,setuid
install was dropped?- The repo has a
README.postfix
, which I thought the Postfix docs might have been referencing formaildrop
, but this Debian patch states that file is quite outdated.
- Debian
- The
-
- Messages that have been submitted via the Postfix
sendmail(1)
command, but not yet brought into the main Postfix queue by thepickup(8)
service, await processing in themaildrop
queue. - Messages can be added to the
maildrop
queue even when the Postfix system is not running. They will begin to be processed once Postfix is started. - The
maildrop
queue is drained by the single threadedpickup(8)
service scanning the queue directory periodically or when notified of new message arrival by thepostdrop(1)
program. - The
postdrop(1)
program is asetgid
helper that allows the unprivileged Postfixsendmail(1)
program to inject mail into themaildrop
queue and to notify thepickup(8)
service of its arrival.
- Messages that have been submitted via the Postfix
-
SECURITY
The command is designed to run with set-group ID privileges, so that it can write to themaildrop
queue directory and so that it can connect to Postfix daemon processes.
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set. | ||
# Ref: https://github.com/docker-mailserver/docker-mailserver/issues/3619 | ||
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | ||
chmod 710 "${STATEDIR}/spool-postfix/public" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
postfix check
and find
predicates
Reference (click to expand)
find
command predicates check the following via Postfix postfix-script
(logic for postfix check
command):
- All files that are not of named pipe or socket type within the Postfix queue directory should have the same UID (
! \( -type p -o -type s \) ! -user $mail_owner
. maildrop
andpublic
dirs in the Postfix queue directory should have the SGID intended grouppostdrop
assigned (! -group $setgid_group
).postqueue
andpostdrop
commands should have world executable (chmod ugo+x
/o111
) and have SGID bit (chmod g+s
/o2000
) permissions (! -perm -02111
).
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set. | ||
# Ref: https://github.com/docker-mailserver/docker-mailserver/issues/3619 | ||
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | ||
chmod 710 "${STATEDIR}/spool-postfix/public" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference since I always forget this stuff 😅
# Permissions special bit example:
$ touch user group other
# SUID:
$ chmod u+s user
# SGID:
$ chmod g+s group
# Sticky-bit:
$ chmod o+t other
# Default 644 base permissions (`chmod u=rwx,go=r`)
$ eza --octal-permissions --long
2644 .rw-r-Sr-- 0 root 8 Nov 01:01 group
1644 .rw-r--r-T 0 root 8 Nov 01:01 other
4644 .rwSr--r-- 0 root 8 Nov 01:01 user
# Make world executable:
$ chmod ugo+x user group other
$ eza --octal-permissions --long
2755 .rwxr-sr-x 0 root 8 Nov 01:01 group
1755 .rwxr-xr-t 0 root 8 Nov 01:01 other
4755 .rwsr-xr-x 0 root 8 Nov 01:01 user
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set. | ||
# Ref: https://github.com/docker-mailserver/docker-mailserver/issues/3619 | ||
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | ||
chmod 710 "${STATEDIR}/spool-postfix/public" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Permissions - Historical context
In the original PR, I referenced this mailing list link:
Reference (click to expand)
-
The Postfix maintainer states that
postfix set-permissions
may not be sufficient for Debian due to their downstream packaging? But emphasizes thatpostfix-files
conveys all permissions that Postfix requires to work correctly. -
An earlier comment at that link notes that
maildrop/
is leveraging SGID for access by related processes that may be run from a user that would otherwise lacks access into the directory. -
The original author message at that link also uses
postfix check
to verify permissionswhich are caught as incorrect(EDIT: My bad, it said everything was ok, issue was with third-party softwaremailx
). -
In the same original PR comment that I shared this mailing list link, I referenced Postfix release notes (many years older than mailing list discussion):
[snapshot-20020106]
This release modifies the existingmaster.cf
file. The local pickup service is now unprivileged, and the cleanup
and flush service are now "public".
Should you have to back out to a previous release, then you must
...
2)chmod 755 /var/spool/postfix/public
.
To revert to a world-writable mail submission directory,chmod 1733 /var/spool/postfix/maildrop
.So
chmod 755 public
(rwx-r-x-r-x
) +chmod 1733 maildrop
(rwx-wx-wx
) was the previous permissions quite a while back.pickup
andcleanup
services in our ownmaster.cf
are unprivileged 👍
Now we have these directory permissions:
public/
with710
only permits the group (g=x
) to enter the directory to access content inside. The group beingpostdrop
that leverages SGID bit on it's binary.maildrop/
with730
, additionally allows the group to write (g=wx
).- Programs like Postfix
sendmail
will usepostdrop
command (with its SGID permission bit) to write intomaildrop/
. - Additionally adding sticky-bit (
T
) tomaildrop/
would only be useful if users were part of thepostdrop
group to protect against deletion/edits of files that they have ownership of inmaildrop/
from other members of the shared group. But this is not the case; users temporarily are a member ofpostdrop
group via thepostdrop
executable viasetgid
(SGID). So no benefit within DMS setups?
- Programs like Postfix
# These permissions rely on the `postdrop` binary (and alike) having proper SUID/SGID bits set. | ||
# Ref: https://github.com/docker-mailserver/docker-mailserver/issues/3619 | ||
chmod 730 "${STATEDIR}/spool-postfix/maildrop" | ||
chmod 710 "${STATEDIR}/spool-postfix/public" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conclusion
The original PR commit message notes that we chose to match what Debian was using, which would be consistent regardless of a volume mount (which this script requires to run).
maildrop/
doesn't need sticky bit (T
)
# Send mail as a different user (eg: As user `nobody`):
$ su -s /bin/bash -c 'sendmail -f from@example.test to@example.test' nobody <<< 'this is a test'
postdrop: warning: unable to look up public/pickup: No such file or directory
# Stored mail at `maildrop/` with permissions 644:
$ ls -l /var/spool/postfix/maildrop/
drwx-wx--T 1 postfix postdrop 4.0K Nov 8 05:15 .
drwxr-xr-x 1 root root 4.0K May 3 2023 ..
-rwxr--r-- 1 nobody postdrop 108 Nov 8 05:15 3B8FAB6DF
It's perhaps relevant if a third-party software was writing to this location, which is also when SGID is may be relevant (not a concern for DMS):
Reference (click to expand)
-
As the earlier referenced 2015 mailing discussion notes with
mailx
trying to write to this location.While
postfix check
shows no errors, when I runmailx
from the command line I get warnings about an inability to write to/var/spool/postfix/maildrop
.Postfix uses a very interesting trick of having the executables set the
GroupID when being run as the user, and this allows them to get into
directories when the user they are running as normally cannot. -
On the Debian mailing list in 2001, it was noted that
mailx
had a security hole allowing users use themail
group, so the packagers removedsetgid
bit (SGID).- It also mentions that Solaris used a world writable maildrop location, which required sticky bit (which apparently elimitates the need for
setgid
mail programs). - In reference to
/var/mail
, they also encourage it to have sticky bit assigned to significantly reduce the mentionedgid=mail
exploit. They suggest for Postfix with/var/mail
to use3775
, advising against2755
(SGID only).
- It also mentions that Solaris used a world writable maildrop location, which required sticky bit (which apparently elimitates the need for
public/
doesn't need SGID set (s
)
Postfix docs for master.cf
explain what private/
and public/
are for:
Private (default:
y
)
- Whether a service is internal to Postfix (pathname starts with
private/
), or exposed through Postfix command-line tools (pathname starts withpublic/
).- Internet (type
inet
) services can't be private.
Nothing about that seems to justify needing SGID then either? Especially since group permissions is execute only for the directory 🤷♂️
Additional context
In the prior PR, I noted that this location only contained unix sockets.
It should only be services defined in our master.cf
with private
column as n
, where most type
are unix
/ inet
+ 1 fifo
+ 1 unix-dgram
.
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
maildrop
and public
directories (Postfix)maildrop/
and public/
directories
maildrop/
and public/
directoriesmaildrop/
and public/
directory permissions
Not a single issue with these changes on my production machine so far. |
Documentation preview for this PR is ready! 🎉 Built with commit: 7008f66 |
All good on my machine; I will go ahead and merge this. 2 PRs remaining for v13.0.0 then. |
…ry permissions (docker-mailserver#3625) * update K8s deployment Because `allowPrivilegeEscalation` controls SUID/SGID, we require it when postdrop is invoked. * correct permissions for maildrop/public The reason our permissions previously worked out as that in setups where SUID/SGID worked, the binaries used to place files in these directories already have SGID set; the current set of permissions makes less sense (as explained in this comment: docker-mailserver#3619 (comment)) Since the binaries used to place files inside these directories alredy have SUID/SGID set, we do not require these bits (or the sticky bit) to be set on the directories. * Apply suggestions from code review --------- Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Description
allowPrivilegeEscalation: true
as it turns out/var/spool/postfix/{maildrop,public}
. This is more tricky, but docs: Kubernetes requiesallowPrivilegeEscalation: true
#3619 provides good insights. In short, we do not seem to require SUID/SGID/sticky bits on these directories as the binaries that place files in these directories already have the proper SGID bits set on them.Fixes #3619
CC @innotecsol
Type of change
Checklist:
docs/
)