[libvirt PATCH] remote: use SocketMode=0600 when polkit is not compiled

Daniel P. Berrangé posted 1 patch 3 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200902175436.3547280-1-berrange@redhat.com
Test syntax-check failed
src/meson.build               | 11 ++++++++++
src/remote/libvirtd.conf.in   | 40 ++++++++++++++++++++++++++---------
src/remote/libvirtd.socket.in |  2 +-
3 files changed, 42 insertions(+), 11 deletions(-)
[libvirt PATCH] remote: use SocketMode=0600 when polkit is not compiled
Posted by Daniel P. Berrangé 3 years, 7 months ago
The systemd .socket unit files we ship for libvirt daemons use
SocketMode=0666 on the assumption that libvirt is built with
polkit which provides access control.

Some people, however, may have explicitly turned off polkit at
build time and not realize that leaves them insecure unless
they also change the SocketMode.  This addresses that problem
by making the SocketMode default to 0600 when polkit is
disabled at compile time.

Note we cannot automatically fix the case where the user
compiles polkit, but then overrides the libvirtd.conf defaults
to disable polkit. This is what lead to CVE-2020-15708 in
Ubuntu 20.10.  We can at least improve the inline comments
in the config file to give a clearer warning though, which
may have helped avoid the mistaken config.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/meson.build               | 11 ++++++++++
 src/remote/libvirtd.conf.in   | 40 ++++++++++++++++++++++++++---------
 src/remote/libvirtd.socket.in |  2 +-
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/src/meson.build b/src/meson.build
index 5d8deaf548..897b5ecbca 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -713,6 +713,12 @@ foreach data : virt_daemon_confs
   daemon_conf.set('DAEMON_NAME_UC', name_uc)
   # to silence meson warning about missing 'CONFIG' in the configuration_data
   daemon_conf.set('CONFIG', '@CONFIG@')
+  if conf.has('WITH_POLKIT')
+    daemon_conf.set('default_auth', 'polkit')
+  else
+    daemon_conf.set('default_auth', 'none')
+  endif
+
 
   if data.get('with_ip', false)
     conf_in = libvirtd_conf_tmp
@@ -792,6 +798,11 @@ if conf.has('WITH_LIBVIRTD')
       unit_conf.set('service', unit['service'])
       unit_conf.set('sockprefix', unit['sockprefix'])
       unit_conf.set('deps', unit.get('deps', ''))
+      if conf.has('WITH_POLKIT')
+        unit_conf.set('mode', '0666')
+      else
+        unit_conf.set('mode', '0600')
+      endif
 
       configure_file(
         input: unit['service_in'],
diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in
index 2607fbad86..ae6207bf54 100644
--- a/src/remote/libvirtd.conf.in
+++ b/src/remote/libvirtd.conf.in
@@ -127,6 +127,8 @@
 #
 # Authentication.
 #
+# There are the following choices available:
+#
 #  - none: do not perform auth checks. If you can connect to the
 #          socket you are allowed. This is suitable if there are
 #          restrictions on connecting to the socket (eg, UNIX
@@ -144,21 +146,39 @@
 #            full read/write access (aka sudo like), while anyone
 #            is allowed read/only access.
 #
+
 # Set an authentication scheme for UNIX read-only sockets
+#
 # By default socket permissions allow anyone to connect
 #
-# To restrict monitoring of domains you may wish to enable
-# an authentication mechanism here
-#auth_unix_ro = "none"
+# If libvirt was compiled without support for 'polkit', then
+# no access control checks are done, but libvirt still only
+# allows execution of APIs which don't change state.
+#
+# If libvirt was compiled with support for 'polkit', then
+# the libvirt socket will perform a check with polkit after
+# connections. The default policy still allows any local
+# user access.
+#
+# To restrict monitoring of domains you may wish to either
+# enable 'sasl' here, or change the polkit policy definition.
+#auth_unix_ro = "@default_auth@"
 
-# Set an authentication scheme for UNIX read-write sockets
-# By default socket permissions only allow root. If PolicyKit
-# support was compiled into libvirt, the default will be to
-# use 'polkit' auth.
+# Set an authentication scheme for UNIX read-write sockets.
+#
+# If libvirt was compiled without support for 'polkit', then
+# the systemd .socket files will use SocketMode=0600 by default
+# thus only allowing root user to connect, and 'auth_unix_rw'
+# will default to 'none'.
+#
+# If libvirt was compiled with support for 'polkit', then
+# the systemd .socket files will use SocketMode=0666 which
+# allows any user to connect and 'auth_iunix_rw' will default
+# to 'polkit'. If you disable use of 'polkit' here, then it
+# is essential to change the systemd SocketMode parameter
+# back to 0600, to avoid an insecure configuration.
 #
-# If the unix_sock_rw_perms are changed you may wish to enable
-# an authentication mechanism here
-#auth_unix_rw = "none"
+#auth_unix_rw = "@default_auth@"
 @CUT_ENABLE_IP@
 
 # Change the authentication scheme for TCP sockets.
diff --git a/src/remote/libvirtd.socket.in b/src/remote/libvirtd.socket.in
index df36df2125..85b4aa800a 100644
--- a/src/remote/libvirtd.socket.in
+++ b/src/remote/libvirtd.socket.in
@@ -8,7 +8,7 @@ Before=@service@.service
 # when using systemd version < 227
 ListenStream=@runstatedir@/libvirt/@sockprefix@-sock
 Service=@service@.service
-SocketMode=0666
+SocketMode=@mode@
 
 [Install]
 WantedBy=sockets.target
-- 
2.25.4

Re: [libvirt PATCH] remote: use SocketMode=0600 when polkit is not compiled
Posted by Jiri Denemark 3 years, 7 months ago
On Wed, Sep 02, 2020 at 18:54:36 +0100, Daniel P. Berrangé wrote:
> The systemd .socket unit files we ship for libvirt daemons use
> SocketMode=0666 on the assumption that libvirt is built with
> polkit which provides access control.
> 
> Some people, however, may have explicitly turned off polkit at
> build time and not realize that leaves them insecure unless
> they also change the SocketMode.  This addresses that problem
> by making the SocketMode default to 0600 when polkit is
> disabled at compile time.
> 
> Note we cannot automatically fix the case where the user
> compiles polkit, but then overrides the libvirtd.conf defaults
> to disable polkit. This is what lead to CVE-2020-15708 in
> Ubuntu 20.10.  We can at least improve the inline comments
> in the config file to give a clearer warning though, which
> may have helped avoid the mistaken config.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/meson.build               | 11 ++++++++++
>  src/remote/libvirtd.conf.in   | 40 ++++++++++++++++++++++++++---------
>  src/remote/libvirtd.socket.in |  2 +-
>  3 files changed, 42 insertions(+), 11 deletions(-)

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Re: [libvirt PATCH] remote: use SocketMode=0600 when polkit is not compiled
Posted by Ján Tomko 3 years, 7 months ago
On a Wednesday in 2020, Daniel P. Berrangé wrote:
>The systemd .socket unit files we ship for libvirt daemons use
>SocketMode=0666 on the assumption that libvirt is built with
>polkit which provides access control.
>
>Some people, however, may have explicitly turned off polkit at
>build time and not realize that leaves them insecure unless
>they also change the SocketMode.  This addresses that problem
>by making the SocketMode default to 0600 when polkit is
>disabled at compile time.
>
>Note we cannot automatically fix the case where the user
>compiles polkit, but then overrides the libvirtd.conf defaults
>to disable polkit. This is what lead to CVE-2020-15708 in
>Ubuntu 20.10.  We can at least improve the inline comments
>in the config file to give a clearer warning though, which
>may have helped avoid the mistaken config.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> src/meson.build               | 11 ++++++++++
> src/remote/libvirtd.conf.in   | 40 ++++++++++++++++++++++++++---------
> src/remote/libvirtd.socket.in |  2 +-
> 3 files changed, 42 insertions(+), 11 deletions(-)
>
>diff --git a/src/meson.build b/src/meson.build
>index 5d8deaf548..897b5ecbca 100644
>--- a/src/meson.build
>+++ b/src/meson.build
>@@ -713,6 +713,12 @@ foreach data : virt_daemon_confs
>   daemon_conf.set('DAEMON_NAME_UC', name_uc)
>   # to silence meson warning about missing 'CONFIG' in the configuration_data
>   daemon_conf.set('CONFIG', '@CONFIG@')
>+  if conf.has('WITH_POLKIT')
>+    daemon_conf.set('default_auth', 'polkit')
>+  else
>+    daemon_conf.set('default_auth', 'none')
>+  endif
>+
>
>   if data.get('with_ip', false)
>     conf_in = libvirtd_conf_tmp
>@@ -792,6 +798,11 @@ if conf.has('WITH_LIBVIRTD')
>       unit_conf.set('service', unit['service'])
>       unit_conf.set('sockprefix', unit['sockprefix'])
>       unit_conf.set('deps', unit.get('deps', ''))
>+      if conf.has('WITH_POLKIT')
>+        unit_conf.set('mode', '0666')
>+      else
>+        unit_conf.set('mode', '0600')
>+      endif
>
>       configure_file(
>         input: unit['service_in'],
>diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in
>index 2607fbad86..ae6207bf54 100644
>--- a/src/remote/libvirtd.conf.in
>+++ b/src/remote/libvirtd.conf.in
>@@ -127,6 +127,8 @@
> #
> # Authentication.
> #
>+# There are the following choices available:
>+#
> #  - none: do not perform auth checks. If you can connect to the
> #          socket you are allowed. This is suitable if there are
> #          restrictions on connecting to the socket (eg, UNIX
>@@ -144,21 +146,39 @@
> #            full read/write access (aka sudo like), while anyone
> #            is allowed read/only access.
> #
>+
> # Set an authentication scheme for UNIX read-only sockets
>+#
> # By default socket permissions allow anyone to connect
> #
>-# To restrict monitoring of domains you may wish to enable
>-# an authentication mechanism here
>-#auth_unix_ro = "none"
>+# If libvirt was compiled without support for 'polkit', then
>+# no access control checks are done, but libvirt still only
>+# allows execution of APIs which don't change state.
>+#
>+# If libvirt was compiled with support for 'polkit', then
>+# the libvirt socket will perform a check with polkit after
>+# connections. The default policy still allows any local
>+# user access.
>+#
>+# To restrict monitoring of domains you may wish to either
>+# enable 'sasl' here, or change the polkit policy definition.
>+#auth_unix_ro = "@default_auth@"
>

This change affects the augeas tests which will need some special
treatment:
https://gitlab.com/libvirt/libvirt/-/jobs/717784534#L2306

Jano

>-# Set an authentication scheme for UNIX read-write sockets
>-# By default socket permissions only allow root. If PolicyKit
>-# support was compiled into libvirt, the default will be to
>-# use 'polkit' auth.
>+# Set an authentication scheme for UNIX read-write sockets.
>+#
>+# If libvirt was compiled without support for 'polkit', then
>+# the systemd .socket files will use SocketMode=0600 by default
>+# thus only allowing root user to connect, and 'auth_unix_rw'
>+# will default to 'none'.
>+#
>+# If libvirt was compiled with support for 'polkit', then
>+# the systemd .socket files will use SocketMode=0666 which
>+# allows any user to connect and 'auth_iunix_rw' will default
>+# to 'polkit'. If you disable use of 'polkit' here, then it
>+# is essential to change the systemd SocketMode parameter
>+# back to 0600, to avoid an insecure configuration.
> #
>-# If the unix_sock_rw_perms are changed you may wish to enable
>-# an authentication mechanism here
>-#auth_unix_rw = "none"
>+#auth_unix_rw = "@default_auth@"
> @CUT_ENABLE_IP@
>
> # Change the authentication scheme for TCP sockets.
Re: [libvirt PATCH] remote: use SocketMode=0600 when polkit is not compiled
Posted by Daniel P. Berrangé 3 years, 7 months ago
On Thu, Sep 03, 2020 at 04:22:39PM +0200, Ján Tomko wrote:
> On a Wednesday in 2020, Daniel P. Berrangé wrote:
> > The systemd .socket unit files we ship for libvirt daemons use
> > SocketMode=0666 on the assumption that libvirt is built with
> > polkit which provides access control.
> > 
> > Some people, however, may have explicitly turned off polkit at
> > build time and not realize that leaves them insecure unless
> > they also change the SocketMode.  This addresses that problem
> > by making the SocketMode default to 0600 when polkit is
> > disabled at compile time.
> > 
> > Note we cannot automatically fix the case where the user
> > compiles polkit, but then overrides the libvirtd.conf defaults
> > to disable polkit. This is what lead to CVE-2020-15708 in
> > Ubuntu 20.10.  We can at least improve the inline comments
> > in the config file to give a clearer warning though, which
> > may have helped avoid the mistaken config.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > src/meson.build               | 11 ++++++++++
> > src/remote/libvirtd.conf.in   | 40 ++++++++++++++++++++++++++---------
> > src/remote/libvirtd.socket.in |  2 +-
> > 3 files changed, 42 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/meson.build b/src/meson.build
> > index 5d8deaf548..897b5ecbca 100644
> > --- a/src/meson.build
> > +++ b/src/meson.build
> > @@ -713,6 +713,12 @@ foreach data : virt_daemon_confs
> >   daemon_conf.set('DAEMON_NAME_UC', name_uc)
> >   # to silence meson warning about missing 'CONFIG' in the configuration_data
> >   daemon_conf.set('CONFIG', '@CONFIG@')
> > +  if conf.has('WITH_POLKIT')
> > +    daemon_conf.set('default_auth', 'polkit')
> > +  else
> > +    daemon_conf.set('default_auth', 'none')
> > +  endif
> > +
> > 
> >   if data.get('with_ip', false)
> >     conf_in = libvirtd_conf_tmp
> > @@ -792,6 +798,11 @@ if conf.has('WITH_LIBVIRTD')
> >       unit_conf.set('service', unit['service'])
> >       unit_conf.set('sockprefix', unit['sockprefix'])
> >       unit_conf.set('deps', unit.get('deps', ''))
> > +      if conf.has('WITH_POLKIT')
> > +        unit_conf.set('mode', '0666')
> > +      else
> > +        unit_conf.set('mode', '0600')
> > +      endif
> > 
> >       configure_file(
> >         input: unit['service_in'],
> > diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in
> > index 2607fbad86..ae6207bf54 100644
> > --- a/src/remote/libvirtd.conf.in
> > +++ b/src/remote/libvirtd.conf.in
> > @@ -127,6 +127,8 @@
> > #
> > # Authentication.
> > #
> > +# There are the following choices available:
> > +#
> > #  - none: do not perform auth checks. If you can connect to the
> > #          socket you are allowed. This is suitable if there are
> > #          restrictions on connecting to the socket (eg, UNIX
> > @@ -144,21 +146,39 @@
> > #            full read/write access (aka sudo like), while anyone
> > #            is allowed read/only access.
> > #
> > +
> > # Set an authentication scheme for UNIX read-only sockets
> > +#
> > # By default socket permissions allow anyone to connect
> > #
> > -# To restrict monitoring of domains you may wish to enable
> > -# an authentication mechanism here
> > -#auth_unix_ro = "none"
> > +# If libvirt was compiled without support for 'polkit', then
> > +# no access control checks are done, but libvirt still only
> > +# allows execution of APIs which don't change state.
> > +#
> > +# If libvirt was compiled with support for 'polkit', then
> > +# the libvirt socket will perform a check with polkit after
> > +# connections. The default policy still allows any local
> > +# user access.
> > +#
> > +# To restrict monitoring of domains you may wish to either
> > +# enable 'sasl' here, or change the polkit policy definition.
> > +#auth_unix_ro = "@default_auth@"
> > 
> 
> This change affects the augeas tests which will need some special
> treatment:
> https://gitlab.com/libvirt/libvirt/-/jobs/717784534#L2306

Urgh, I wasn't paying attention to CI since it was already broken
for other jobs.  I'll post a fix soon.



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 :|