Re: [libvirt PATCH 35/42] systemd: Replace Requires with BindTo+After for sockets
On Tue, Sep 26, 2023 at 01:36:39PM +0100, Daniel P. Berrangé wrote:
> On Tue, Sep 26, 2023 at 04:09:17AM -0500, Andrea Bolognani wrote:
> > On Tue, Sep 26, 2023 at 09:44:52AM +0100, Daniel P. Berrangé wrote:
> > > On Mon, Sep 25, 2023 at 08:58:33PM +0200, Andrea Bolognani wrote:
> > > > This is the strongest relationship that can be declared between
> > > > two units, and causes the service to be terminated immediately
> > > > if any of its sockets disappear. This is the behavior we want.
> > >
> > > IIUC, this prevents running the service with /only/ the main
> > > socket, and ro/admin sockets disabled. Running without the
> > > ro socket in particular was something we wanted to allow to
> > > reduce exposure to unprivileged services (there have been
> > > a number of CVEs where the read-only socket was the way in)
> >
> > This doesn't work today either AFAICT, since the ro/admin sockets are
> > marked as Required by the various services.
>
> Doh, yes, I've confirmed. I'm sure it used to work, but we must have
> broken it at some point as we tweaked the deps countless times over
> to finese the setup.
>
> > If we want to support this configuration, then we need
> >
> > # foo.service
> > [Unit]
> > BindsTo=foo.socket
> > Wants=foo-ro.socket
> > Wants=foo-admin.socket
> > After=foo.socket
> >
> > In the default scenario, things will work just the same as they do
> > here, but it will also be possible to mask foo{-ro,-admin}.socket to
> > obtain the hardened setup you describe.
>
> Or we just decide to keep life simple, and if people want to harden
> things they can change permissions on the socket via a system unit
> override locally.
I don't think this is any more complicated than the version that uses
BindsTo/After for all sockets, and it shouldn't make things any worse
for people who stick with the defaults, so I don't mind trying to
integrate this requirement into v2.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 35/42] systemd: Replace Requires with BindTo+After for sockets
On Tue, Sep 26, 2023 at 04:09:17AM -0500, Andrea Bolognani wrote:
> On Tue, Sep 26, 2023 at 09:44:52AM +0100, Daniel P. Berrangé wrote:
> > On Mon, Sep 25, 2023 at 08:58:33PM +0200, Andrea Bolognani wrote:
> > > This is the strongest relationship that can be declared between
> > > two units, and causes the service to be terminated immediately
> > > if any of its sockets disappear. This is the behavior we want.
> >
> > IIUC, this prevents running the service with /only/ the main
> > socket, and ro/admin sockets disabled. Running without the
> > ro socket in particular was something we wanted to allow to
> > reduce exposure to unprivileged services (there have been
> > a number of CVEs where the read-only socket was the way in)
>
> This doesn't work today either AFAICT, since the ro/admin sockets are
> marked as Required by the various services.
Doh, yes, I've confirmed. I'm sure it used to work, but we must have
broken it at some point as we tweaked the deps countless times over
to finese the setup.
> If we want to support this configuration, then we need
>
> # foo.service
> [Unit]
> BindsTo=foo.socket
> Wants=foo-ro.socket
> Wants=foo-admin.socket
> After=foo.socket
>
> In the default scenario, things will work just the same as they do
> here, but it will also be possible to mask foo{-ro,-admin}.socket to
> obtain the hardened setup you describe.
Or we just decide to keep life simple, and if people want to harden
things they can change permissions on the socket via a system unit
override locally.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 35/42] systemd: Replace Requires with BindTo+After for sockets
On Tue, Sep 26, 2023 at 09:44:52AM +0100, Daniel P. Berrangé wrote:
> On Mon, Sep 25, 2023 at 08:58:33PM +0200, Andrea Bolognani wrote:
> > This is the strongest relationship that can be declared between
> > two units, and causes the service to be terminated immediately
> > if any of its sockets disappear. This is the behavior we want.
>
> IIUC, this prevents running the service with /only/ the main
> socket, and ro/admin sockets disabled. Running without the
> ro socket in particular was something we wanted to allow to
> reduce exposure to unprivileged services (there have been
> a number of CVEs where the read-only socket was the way in)
This doesn't work today either AFAICT, since the ro/admin sockets are
marked as Required by the various services.
If we want to support this configuration, then we need
# foo.service
[Unit]
BindsTo=foo.socket
Wants=foo-ro.socket
Wants=foo-admin.socket
After=foo.socket
In the default scenario, things will work just the same as they do
here, but it will also be possible to mask foo{-ro,-admin}.socket to
obtain the hardened setup you describe.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 35/42] systemd: Replace Requires with BindTo+After for sockets
On Mon, Sep 25, 2023 at 08:58:33PM +0200, Andrea Bolognani wrote: > This is the strongest relationship that can be declared between > two units, and causes the service to be terminated immediately > if any of its sockets disappear. This is the behavior we want. IIUC, this prevents running the service with /only/ the main socket, and ro/admin sockets disabled. Running without the ro socket in particular was something we wanted to allow to reduce exposure to unprivileged services (there have been a number of CVEs where the read-only socket was the way in) > > Signed-off-by: Andrea Bolognani > --- > src/locking/virtlockd.service.in | 6 -- > src/logging/virtlogd.service.in | 6 -- > src/virtd.service.in | 9 ++--- > 3 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/src/locking/virtlockd.service.in > b/src/locking/virtlockd.service.in > index 9e91fa3261..a21a2c2c19 100644 > --- a/src/locking/virtlockd.service.in > +++ b/src/locking/virtlockd.service.in > @@ -1,7 +1,9 @@ > [Unit] > Description=Virtual machine lock manager > -Requires=virtlockd.socket > -Requires=virtlockd-admin.socket > +BindsTo=virtlockd.socket > +BindsTo=virtlockd-admin.socket > +After=virtlockd.socket > +After=virtlockd-admin.socket > Before=libvirtd.service > Documentation=man:virtlockd(8) > Documentation=https://libvirt.org > diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in > index 97c942ffb0..f3bd576301 100644 > --- a/src/logging/virtlogd.service.in > +++ b/src/logging/virtlogd.service.in > @@ -1,7 +1,9 @@ > [Unit] > Description=Virtual machine log manager > -Requires=virtlogd.socket > -Requires=virtlogd-admin.socket > +BindsTo=virtlogd.socket > +BindsTo=virtlogd-admin.socket > +After=virtlogd.socket > +After=virtlogd-admin.socket > Before=libvirtd.service > Documentation=man:virtlogd(8) > Documentation=https://libvirt.org > diff --git a/src/virtd.service.in b/src/virtd.service.in > index 21391a65b0..b9e6345e8c 100644 > --- a/src/virtd.service.in > +++ b/src/virtd.service.in > @@ -1,8 +1,11 @@ > [Unit] > Description=@name@ daemon > -Requires=@[email protected] > -Requires=@[email protected] > -Requires=@[email protected] > +BindsTo=@[email protected] > +BindsTo=@[email protected] > +BindsTo=@[email protected] > +After=@[email protected] > +After=@[email protected] > +After=@[email protected] > Conflicts=libvirtd.service > After=libvirtd.service > After=network.target > -- > 2.41.0 > With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
