nixos/postfix: systemd sandbox #93305

tnias · 2020-07-16T21:45:56Z

Motivation for this change

Add systemd service sandbox features.

Things done

tested on a mailserver based on simple-nixos-mailserver

cc: postfix maintainers @rickynils @globin

hmenke · 2020-07-16T23:02:46Z

nixos/modules/services/mail/postfix.nix

+    ln -sf /var/spool/mail /var/
+
+    #Finally delegate to postfix checking remain directories in /var/lib/postfix and set permissions on them
+    ${pkgs.postfix}/bin/postfix set-permissions config_directory=/var/lib/postfix/conf


The sandbox is incomplete here. This will still run postfix as root, although I don't know whether there is a way around it.

I assumed it is acceptable to have no sandbox for the prepare script, because it does not run as a network daemon.
Should put it in a sandbox as well?

To my knowledge the "main daemon" has to be run as root. It drops privileges for it's children, but there is no tight systemd integration. I am open for suggestions. :)

I see. In that case I'm not sure whether there is any benefit from running postfix set-permission in the sandbox.

alyssais · 2020-07-17T01:43:58Z

nixos/modules/services/mail/postfix.nix

+            PrivateTmp = true;
+            ProtectClock = true;
+            ProtectControlGroups = true;
+            ProtectHome = true;


Won’t this break .forward, or delivering mail to a local user’s home directory?

Yes. Replacing ProtectHome=true by ReadOnlyPaths=/home would allow to read .forward. For delivery to the home directory this has to be disabled entirely.

Should this be behind an option? If yes, how whould an option look like?

I would suggest you don't put this behind an option - don't include it at all. It isn't a reasonable default for a mail server. Add documentation so users who can and wish to will know how to add it.

hmenke · 2020-07-17T02:04:16Z

It's probably a good idea to also discuss this with the people from simple-nixos-mailserver @r-raymond
Same for your opendkim PR #93314, rspamd PR #93293, and redis PR #93277

r-raymond · 2020-07-17T05:06:33Z

@hmenke thanks for the heads up, I'll ping the current maintainers.

nlewo · 2020-07-17T07:53:31Z

Thanks for the heads up, and thanks for these patches! I really like this idea;)

Did you run the nixos-mailserver tests with your patches? I will do it this WE.

tnias · 2020-07-17T08:39:08Z

Did you run the nixos-mailserver tests with your patches?

I have not yet run any of the official tests. Just tested my usecase for now.

aanderse · 2020-07-17T10:32:20Z

nixos/modules/services/mail/postfix.nix

+        wantedBy = [ "multi-user.target" ];
+        before = [ "postifx.service" ];
+        serviceConfig = {
+          ExecStart = preparePostfixScript;


Can't you run this in the original service as an ExecStartPre with the + prefix?

Thanks, the + prefix looks good. I will fix my patches.

peti

How is this kind of sandboxing going to interact with non-trivial Postfix installations that run, say a mailing list server or reflector scripts or any other mail service that needs resources other than Postfix itself?

Running the daemon in a sandbox feels like a rather intrusive change. This should definitely be opt-in.

tnias · 2020-07-17T12:13:14Z

This should definitely be opt-in.

I will try to do something like services.nginx.enableSandbox.

aanderse · 2020-07-17T12:18:17Z

I will try to do something like services.nginx.enableSandbox.

@Mic92 given enableSandbox was meant as a temporary measure and enough time has passed with no negative reports can we please remove it now?

@tnias I'm not in favor of module randomly adding enableSandbox with no consistency at all. Please see my thoughts on the matter here. I think @flokli articulated the issue very well with this comment:

Sandboxing is not a boolean, but something very fuzzy.

tnias · 2020-07-17T13:43:47Z

The fixup changes loosen up rules to allow more general actions.

@peti:

mailing lists (mailman3) deliver mail by lmtp (tcp or unix socket). This should not be blocked by the current rules. Are other mailinglists using something not allowed yet?
are "reflector scripts" just forwarding mailservers? If yes, I don't see why this should not work.
"any other mail service that needs resources other than Postfix itself" can you provide examples? So that I can loosen up the ruleset accordingly

Izorkin · 2020-07-17T17:04:43Z

Rules systemd.tmpfiles can be taken from here - #76604
I running this variant on my server - it worked fine.

tnias · 2020-07-17T23:55:11Z

I used @Izorkin's tmpfiles patch and addded some (hopefully) reasonable sandboxing options.

tnias · 2020-07-20T00:05:38Z

There are some permission/ownership issues in the /var/lib/postfix directory.

nlewo · 2020-07-21T19:48:37Z

@tnias nixos-mailserver tests passed with this patchset.

tnias · 2020-07-21T20:02:01Z

@nlewo I just cleared my /var/lib/postfix directory and now it works just fine. I probably messed up the permissions at some point. ¯_(ツ)_/¯

tnias · 2020-07-21T20:33:30Z

I ran into the fatal: open lock file pid/inet.submission: cannot open file: Permission denied issue again. This widens the sandbox, so it hopefully won't happen again.

Mic92 · 2020-08-05T08:22:54Z

nixos/modules/services/mail/postfix.nix

+            ProtectKernelLogs = true;
+            ProtectKernelModules = true;
+            ProtectKernelTunables = true;
+            ProtectSystem = "full";


Does this still allow other milters? I know that our rspamd integration for postfix for example creates a socket in /run/rspamd/rspamd-milter.sock, opendkim might be similar.

IIUC, the full value still allows RW accesses to /run.
Also, I've run the nixos-mailserver tests (rspamd, opendkim, dovecot,...) on top of this patch and tests passed.

Mic92 · 2020-08-05T08:23:36Z

@GrahamcOfBorg test rspamd

nlewo · 2020-08-23T15:32:46Z

Still any blocker on this?
It would be nice to have it in 20.09;)

aanderse · 2020-08-23T15:40:01Z

And here I was thinking it would be really nice to not have this in 20.09 so it has more time to bake in unstable giving more people a chance to see if this breaks anything in various configurations... but systemd sandboxing on software that wasn't designed in the age of systemd dominance (and no support from upstream) always makes me nervous 🤷‍♂️

Mic92 · 2020-08-23T20:33:26Z

I would also say to have it tested in unstable for a bit longer and merge it after the branch-off. Postfix itself already does quite a bit of privilege separation so these sandboxes would not improve too much in terms of security.

ju1m · 2020-09-18T00:45:42Z

Due to the use of ExecStartPre= (as a str) this breaks configs using it as a str, or using preStart=. The error is:

The option `systemd.services.postfix.serviceConfig.ExecStartPre' has conflicting definitions, in `/nix/store/pb48ks5whk0sk5nn5602m6xw307vjbaj-nixpkgs-patched/nixos/modules/system/boot/systemd.nix' and `/nix/store/pb48ks5whk0sk5nn5602m6xw307vjbaj-nixpkgs-patched/nixos/modules/services/mail/postfix.nix'.

Such option merging conflict happens regularly to me and others #97371
So this does not concern only this PR (though at least it may be more user friendly to set services.postfix's ExecStartPre= as a listOf str). One general solution could be to modify the merging of systemd unit options to merge str and listOf str into listOf str. Or at least also make preStart= generate an ExecStartPre= which is a listOf str.

Mic92 · 2021-01-08T05:33:46Z

Was this closed on purpose?

tnias · 2021-01-08T09:13:35Z

@Mic92 There seemed to be little action around it and in the spirit of reducing the number of open PRs I closed it. We can also keep it open if you prefer.

Mic92 · 2021-01-08T09:47:53Z

@tnias I think it just take some time to make sure that postfix does not break.

Mic92 · 2021-01-08T09:48:23Z

/marvin opt-in
/status needs_reviewer

aanderse · 2021-01-17T13:35:28Z

We're roughly 4-5 months away from a new release. This change presents high enough risk that I would love to see it merged very soon. 4-5 months sounds like a long enough time to break and fix enough machines that we don't have to worry when this goes stable.

Just a thought...

Mic92 · 2021-01-17T17:00:29Z

nixos/modules/services/mail/postfix.nix

+
+            ReadWritePaths = [ "/var/lib/postfix" "/var/spool/mail" ];
+
+            CapabilityBoundingSet = [ "CAP_DAC_OVERRIDE" "CAP_NET_BIND_SERVICE" "CAP_SETGID" "CAP_SETUID" ];


@tnias Have you checked if postfix drops these capabilities for its less-privileged childs?
Otherwise it might make postfix less secure than it was before (those capabilities allow to read/write arbitrary files and change to different users/groups).

Before this update
Master process:

CapInh: 0000000000000000 CapPrm: 000001ffffffffff CapEff: 000001ffffffffff CapBnd: 000001ffffffffff CapAmb: 0000000000000000

Slave process:

CapInh: 0000000000000000 CapPrm: 0000000000000000 CapEff: 0000000000000000 CapBnd: 000001ffffffffff CapAmb: 0000000000000000

0x000001ffffffffff=cap_chown,cap_dac_override,cap_dac_read_search,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_linux_immutable,cap_net_bind_service,cap_net_broadcast,cap_net_admin,cap_net_raw,cap_ipc_lock,cap_ipc_owner,cap_sys_module,cap_sys_rawio,cap_sys_chroot,cap_sys_ptrace,cap_sys_pacct,cap_sys_admin,cap_sys_boot,cap_sys_nice,cap_sys_resource,cap_sys_time,cap_sys_tty_config,cap_mknod,cap_lease,cap_audit_write,cap_audit_control,cap_setfcap,cap_mac_override,cap_mac_admin,cap_syslog,cap_wake_alarm,cap_block_suspend,cap_audit_read,cap_perfmon,cap_bpf,cap_checkpoint_restore

After this update:
Master process:

CapInh: 0000000000000000 CapPrm: 00000000000004c2 CapEff: 00000000000004c2 CapBnd: 00000000000004c2 CapAmb: 0000000000000000

Slave process:

CapInh: 0000000000000000 CapPrm: 0000000000000000 CapEff: 0000000000000000 CapBnd: 00000000000004c2 CapAmb: 0000000000000000

0x00000000000004c2=cap_dac_override,cap_setgid,cap_setuid,cap_net_bind_service

stale · 2021-07-21T03:12:12Z

I marked this as stale due to inactivity. → More info

nixos-discourse · 2022-01-16T15:02:51Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/hardening-systemd-services/17147/7

tomfitzhenry · 2023-09-15T05:14:20Z

FWIW, I've proposed a smaller set of hardening directives in #255227, inspired by Gentoo's.

tnias requested a review from peti as a code owner July 16, 2020 21:45

ofborg bot added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Jul 16, 2020

hmenke reviewed Jul 16, 2020

View reviewed changes

alyssais reviewed Jul 17, 2020

View reviewed changes

aanderse reviewed Jul 17, 2020

View reviewed changes

peti reviewed Jul 17, 2020

View reviewed changes

Izorkin and others added 2 commits July 18, 2020 01:39

nixos/postfix: enable sandboxing

7692374

nixos/postfix: more sandboxing

a4f4dd2

tnias force-pushed the nixos_postfix_202016 branch from 3e7b9c7 to a4f4dd2 Compare July 17, 2020 23:42

nixos/postfix: fixup cosmetics

0bf216e

nlewo approved these changes Jul 21, 2020

View reviewed changes

nixos/postfix: fix permission issue

7b285fb

tnias force-pushed the nixos_postfix_202016 branch from 3276da3 to 7b285fb Compare July 21, 2020 22:17

Mic92 reviewed Aug 5, 2020

View reviewed changes

symphorien mentioned this pull request Aug 26, 2020

nixos/opendkim: systemd sandbox #93314

Merged

11 tasks

flokli mentioned this pull request Sep 25, 2020

nixos/systemd-sandbox: A generic sandboxing module #87661

Closed

10 tasks

tnias closed this Jan 8, 2021

Mic92 reopened this Jan 8, 2021

Mic92 reviewed Jan 17, 2021

View reviewed changes

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 21, 2021

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 16, 2022

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 31, 2022

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 15, 2023

wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict labels Mar 19, 2024

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024

wegank marked this pull request as draft March 25, 2024 16:17


		ReadWritePaths = [ "/var/lib/postfix" "/var/spool/mail" ];

		CapabilityBoundingSet = [ "CAP_DAC_OVERRIDE" "CAP_NET_BIND_SERVICE" "CAP_SETGID" "CAP_SETUID" ];

nixos/postfix: systemd sandbox #93305

Are you sure you want to change the base?

nixos/postfix: systemd sandbox #93305

Conversation

tnias commented Jul 16, 2020

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hmenke Jul 17, 2020 • edited

Choose a reason for hiding this comment

alyssais Jul 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hmenke commented Jul 17, 2020

r-raymond commented Jul 17, 2020

nlewo commented Jul 17, 2020

tnias commented Jul 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peti left a comment

Choose a reason for hiding this comment

tnias commented Jul 17, 2020

aanderse commented Jul 17, 2020

tnias commented Jul 17, 2020

Izorkin commented Jul 17, 2020

tnias commented Jul 17, 2020

tnias commented Jul 20, 2020

nlewo commented Jul 21, 2020

tnias commented Jul 21, 2020

tnias commented Jul 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 commented Aug 5, 2020

nlewo commented Aug 23, 2020

aanderse commented Aug 23, 2020

Mic92 commented Aug 23, 2020

ju1m commented Sep 18, 2020

Mic92 commented Jan 8, 2021

tnias commented Jan 8, 2021

Mic92 commented Jan 8, 2021 • edited

Mic92 commented Jan 8, 2021

aanderse commented Jan 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Jul 21, 2021

nixos-discourse commented Jan 16, 2022

tomfitzhenry commented Sep 15, 2023

hmenke Jul 17, 2020 •

edited

alyssais Jul 17, 2020 •

edited

Mic92 commented Jan 8, 2021 •

edited