| Message ID | be41ccf0-8b00-63fd-cce4-559ceea6f5e5@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | wpa_supplicant: harden systemd service | expand |
On Thu, Apr 04, 2019 at 02:22:59PM +0300, Topi Miettinen wrote: > Subject: [PATCH] Harden systemd service > > Signed-off-by: Topi Miettinen <toiwoton@gmail.com> I have not seen any comments on this so far and since I'm not very familiar with systemd, I'm not sure what exactly this does and how this work with various different distributions. How has this been tested? Is it clear that these capabilities do not result in regressions? It would be helpful if the commit message were to address such details. > diff --git a/wpa_supplicant/systemd/wpa_supplicant.service.in b/wpa_supplicant/systemd/wpa_supplicant.service.in > @@ -4,9 +4,28 @@ Before=network.target > Wants=network.target > > [Service] > -Type=dbus Why is this Type=dbus line being moved? > BusName=fi.w1.wpa_supplicant1 > +CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW > ExecStart=@BINDIR@/wpa_supplicant -u > +IPAddressDeny=any Does that prevent TCP or UDP communication in some manner? If so, why? Wouldn't that break WPS ER? (Or a more recent addition, DPP Controller?) > +LimitMEMLOCK=0 > +LockPersonality=yes > +MemoryDenyWriteExecute=yes > +NoNewPrivileges=yes > +PrivateTmp=yes > +ProtectControlGroups=yes > +ProtectHome=yes > +ProtectKernelModules=yes > +ProtectKernelTunables=yes > +ProtectSystem=strict > +RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 AF_NETLINK AF_PACKET Would this break use of AF_ALG? Or AF_BRIDGE? Or AF_LINK? > +RestrictNamespaces=yes > +RestrictRealtime=yes > +SystemCallArchitectures=native > +SystemCallFilter=@system-service > +TasksMax=1 > +Type=dbus > +UMask=0077 Everything else here looks like an addition, but the Type line is removed and added, i.e., moved, which makes this patch more complex than (hopefully) needed.
On 30.12.2019 18.02, Jouni Malinen wrote: > On Thu, Apr 04, 2019 at 02:22:59PM +0300, Topi Miettinen wrote: >> Subject: [PATCH] Harden systemd service >> >> Signed-off-by: Topi Miettinen <toiwoton@gmail.com> > > I have not seen any comments on this so far and since I'm not very > familiar with systemd, I'm not sure what exactly this does and how this > work with various different distributions. How has this been tested? Is > it clear that these capabilities do not result in regressions? It would > be helpful if the commit message were to address such details. I've tested this with my configuration, which of course did not exercise all possible features. The choice of distro shouldn't matter as the sandboxing features are rather generic. >> diff --git a/wpa_supplicant/systemd/wpa_supplicant.service.in b/wpa_supplicant/systemd/wpa_supplicant.service.in >> @@ -4,9 +4,28 @@ Before=network.target >> Wants=network.target >> >> [Service] >> -Type=dbus > > Why is this Type=dbus line being moved? It's good practice to keep the lines sorted alphabetically to help finding a specific directive quick visually. >> BusName=fi.w1.wpa_supplicant1 >> +CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW >> ExecStart=@BINDIR@/wpa_supplicant -u >> +IPAddressDeny=any > > Does that prevent TCP or UDP communication in some manner? If so, why? > Wouldn't that break WPS ER? (Or a more recent addition, DPP Controller?) Yes, it blocks TCP and UDP as I did not see them ever used in my straightforward configuration. I don't even know what WPS ER or DPP Controller are and certainly I didn't test those. >> +LimitMEMLOCK=0 >> +LockPersonality=yes >> +MemoryDenyWriteExecute=yes >> +NoNewPrivileges=yes >> +PrivateTmp=yes >> +ProtectControlGroups=yes >> +ProtectHome=yes After I posted this, systemd has introduced new sandboxing features, which probably would be safe to enable: ProtectHostname=yes ProtectKernelLogs=yes >> +ProtectKernelModules=yes >> +ProtectKernelTunables=yes >> +ProtectSystem=strict >> +RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 AF_NETLINK AF_PACKET > > Would this break use of AF_ALG? Or AF_BRIDGE? Or AF_LINK? Yes, if those address families are used, they need to be added to avoid breakage. Alternatively, address families which are known not to be ever used could be blacklisted with ~, if that's easier. >> +RestrictNamespaces=yes >> +RestrictRealtime=yes This new directive could be added: RestrictSUIDSGID=yes >> +SystemCallArchitectures=native >> +SystemCallFilter=@system-service >> +TasksMax=1 >> +Type=dbus >> +UMask=0077 > > Everything else here looks like an addition, but the Type line is > removed and added, i.e., moved, which makes this patch more complex than > (hopefully) needed. > It's just to sort the lines. Unfortunately I've removed wpa_supplicant and switched to iwd a few months ago. -Topi
From f3f8511b6e23076f9b2fcdca00d5b19b4343bc29 Mon Sep 17 00:00:00 2001 From: Topi Miettinen <toiwoton@gmail.com> Date: Thu, 4 Apr 2019 14:18:08 +0300 Subject: [PATCH] Harden systemd service Signed-off-by: Topi Miettinen <toiwoton@gmail.com> --- .../systemd/wpa_supplicant.service.in | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/wpa_supplicant/systemd/wpa_supplicant.service.in b/wpa_supplicant/systemd/wpa_supplicant.service.in index 75a37a8cd..d70e0bc36 100644 --- a/wpa_supplicant/systemd/wpa_supplicant.service.in +++ b/wpa_supplicant/systemd/wpa_supplicant.service.in @@ -4,9 +4,28 @@ Before=network.target Wants=network.target [Service] -Type=dbus BusName=fi.w1.wpa_supplicant1 +CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW ExecStart=@BINDIR@/wpa_supplicant -u +IPAddressDeny=any +LimitMEMLOCK=0 +LockPersonality=yes +MemoryDenyWriteExecute=yes +NoNewPrivileges=yes +PrivateTmp=yes +ProtectControlGroups=yes +ProtectHome=yes +ProtectKernelModules=yes +ProtectKernelTunables=yes +ProtectSystem=strict +RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 AF_NETLINK AF_PACKET +RestrictNamespaces=yes +RestrictRealtime=yes +SystemCallArchitectures=native +SystemCallFilter=@system-service +TasksMax=1 +Type=dbus +UMask=0077 [Install] WantedBy=multi-user.target -- 2.20.1