Re: [PATCH v2 1/1] maintenance: use systemd timers on Linux

["Lénaïc Huard" <lenaic@lhuard.fr>]

Hello Phillip,

Le lundi 10 mai 2021, 20:03:58 CEST Phillip Wood a écrit :
> > +	unit = "# This file was created and is maintained by Git.\n"
> > +	       "# Any edits made in this file might be replaced in the 
future\n"
> > +	       "# by a Git command.\n"
> > +	       "\n"
> > +	       "[Unit]\n"
> > +	       "Description=Optimize Git repositories data\n"
> > +	       "\n"
> > +	       "[Service]\n"
> > +	       "Type=oneshot\n"
> > +	       "ExecStart=\"%1$s/git\" --exec-path=\"%1$s\" for-each-repo
> > --config=maintenance.repo maintenance run --schedule=%%i\n" +	      
> > "LockPersonality=yes\n"
> > +	       "MemoryDenyWriteExecute=yes\n"
> > +	       "NoNewPrivileges=yes\n"
> > +	       "RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6\n"
> > +	       "RestrictNamespaces=yes\n"
> > +	       "RestrictRealtime=yes\n"
> > +	       "RestrictSUIDSGID=yes\n"
> 
> After a quick read of the systemd.exec man page it is unclear to me if
> these Restrict... lines are needed as we already have
> NoNewPrivileges=yes - maybe they have some effect if `git maintence` is
> run as root?

I think that the only thing that `NoNewPrivileges=yes` do is to set the no new 
privileges flag described in [1] on the process.

The `Restrict…` options are enabling some other sandboxing features by 
blocking some syscalls through a seccomp profile.

My understanding of the systemd.exec man page is the other way round, i.e.: as 
soon as there’s a `Restrict…` option, the `NoNewPrivileges=yes` is implied.

So, I would say that strictly speaking `NoNewPrivileges=yes` isn’t needed.
But `NoNewPrivileges=yes` doesn’t imply the `Restrict…` options.

But I thought that, from a security point of view, it’s better to set as many 
sandboxing options as possible and be as explicit as possible.

[1] https://www.kernel.org/doc/html/latest/userspace-api/no_new_privs.html