Re: [PATCH v2 0/3] Add SystemD's sandboxing options

2018-03-01 Thread Willy Tarreau
Hi guys,

On Thu, Mar 01, 2018 at 03:53:25PM +0100, Tim Düsterhus wrote:
> Pavlos,
> 
> Am 27.02.2018 um 22:50 schrieb Pavlos Parissis:
> > BTW: The commit message is a bit misleading because If I don't read the 
> > code I will
> > think that those options are enabled, which isn't true. So, you may want to 
> > mention they aren't
> > enabled by default.
> > 
> I'm not sure how I could put that succinctly into the first line of the
> message. They are already too long.
> 
> While I'm a huge proponent of clear messages I think in this case it's
> easy to see in the diff that they are examples. I'll leave it up to
> Willy to adapt the commit messages, if he considers them unclear,
> instead of me sending even more patches to all the list subscriber's
> mailboxes :-)

Well, let's not bikeshed on the contents of commit messages for patches
that "only" contain extra documentation in config files in the end :-)

I've just merged them now.

Thanks!
Willy



Re: [PATCH v2 0/3] Add SystemD's sandboxing options

2018-03-01 Thread Tim Düsterhus
Pavlos,

Am 27.02.2018 um 22:50 schrieb Pavlos Parissis:
> BTW: The commit message is a bit misleading because If I don't read the code 
> I will
> think that those options are enabled, which isn't true. So, you may want to 
> mention they aren't
> enabled by default.
> 
I'm not sure how I could put that succinctly into the first line of the
message. They are already too long.

While I'm a huge proponent of clear messages I think in this case it's
easy to see in the diff that they are examples. I'll leave it up to
Willy to adapt the commit messages, if he considers them unclear,
instead of me sending even more patches to all the list subscriber's
mailboxes :-)

Thanks.
Tim Düsterhus



Re: [PATCH v2 0/3] Add SystemD's sandboxing options

2018-02-27 Thread Pavlos Parissis
On 27/02/2018 08:19 μμ, Tim Duesterhus wrote:
> Willy,
> 
> okay. I added an additional comment about the nature of those options in
> the first commit and then added the various settings in commented out
> versions. For reference, these are the settings I add on top of Debian's
> default unit file (haproxy 1.8.4 om Debian Stretch) for one of my production
> instances of haproxy:
> 
> # /lib/systemd/system/haproxy.service.d/config.conf
> [Service]
> Environment=CONFIG=/usr/share/haproxy/
> # /lib/systemd/system/haproxy.service.d/no-pidfile.conf
> [Service]
> ExecStart=
> ExecStart=/usr/sbin/haproxy -Ws -f $CONFIG
> # /lib/systemd/system/haproxy.service.d/security.conf
> [Service]
> ProtectSystem=strict
> ProtectHome=true
> ProtectKernelTunables=true
> ProtectKernelModules=true
> ProtectControlGroups=true
> SystemCallFilter=~@cpu-emulation @keyring @module @obsolete @raw-io
> NoNewPrivileges=true
> # /lib/systemd/system/haproxy.service.d/state.conf
> [Service]
> RuntimeDirectory=haproxy
> ExecReload=
> ExecReload=/usr/sbin/haproxy -f $CONFIG -c -q $EXTRAOPTS
> ExecReload=/bin/sh -c "echo show servers state |nc -U 
> /var/run/haproxy/admin.sock > /run/haproxy/global-state"
> ExecReload=/bin/kill -USR2 $MAINPID
> 
> I'm open for further feedback from the other participants in this thread
> as well!
> 
> Best regards
> 
> Tim Duesterhus (3):
>   MINOR: systemd: Add section for SystemD sandboxing to unit file
>   MINOR: systemd: Add SystemD's Protect*= options to the unit file
>   MINOR: systemd: Add SystemD's SystemCallFilter option to the unit file
> 

I am fine with adding the comments and thanks for accepting the feedback.
BTW: The commit message is a bit misleading because If I don't read the code I 
will
think that those options are enabled, which isn't true. So, you may want to 
mention they aren't
enabled by default.

Thanks,
Pavlos



signature.asc
Description: OpenPGP digital signature


[PATCH v2 0/3] Add SystemD's sandboxing options

2018-02-27 Thread Tim Duesterhus
Willy,

okay. I added an additional comment about the nature of those options in
the first commit and then added the various settings in commented out
versions. For reference, these are the settings I add on top of Debian's
default unit file (haproxy 1.8.4 om Debian Stretch) for one of my production
instances of haproxy:

# /lib/systemd/system/haproxy.service.d/config.conf
[Service]
Environment=CONFIG=/usr/share/haproxy/
# /lib/systemd/system/haproxy.service.d/no-pidfile.conf
[Service]
ExecStart=
ExecStart=/usr/sbin/haproxy -Ws -f $CONFIG
# /lib/systemd/system/haproxy.service.d/security.conf
[Service]
ProtectSystem=strict
ProtectHome=true
ProtectKernelTunables=true
ProtectKernelModules=true
ProtectControlGroups=true
SystemCallFilter=~@cpu-emulation @keyring @module @obsolete @raw-io
NoNewPrivileges=true
# /lib/systemd/system/haproxy.service.d/state.conf
[Service]
RuntimeDirectory=haproxy
ExecReload=
ExecReload=/usr/sbin/haproxy -f $CONFIG -c -q $EXTRAOPTS
ExecReload=/bin/sh -c "echo show servers state |nc -U 
/var/run/haproxy/admin.sock > /run/haproxy/global-state"
ExecReload=/bin/kill -USR2 $MAINPID

I'm open for further feedback from the other participants in this thread
as well!

Best regards

Tim Duesterhus (3):
  MINOR: systemd: Add section for SystemD sandboxing to unit file
  MINOR: systemd: Add SystemD's Protect*= options to the unit file
  MINOR: systemd: Add SystemD's SystemCallFilter option to the unit file

 contrib/systemd/haproxy.service.in | 18 ++
 1 file changed, 18 insertions(+)

-- 
2.16.2