[libvirt] [PATCH] daemon: Don't initialize SASL context if not necessary

Peter Krempa posted 1 patch 7 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/0ff32d484958440c055b8af891df143501b7b166.1496405425.git.pkrempa@redhat.com
There is a newer version of this series
daemon/libvirtd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[libvirt] [PATCH] daemon: Don't initialize SASL context if not necessary
Posted by Peter Krempa 7 years, 11 months ago
SASL context would be initialized even if the corresponding TCP or TLS
sockets are not enabled.

fe772f24a68 attempted to fix the symptom by commenting out the settings,
but that did not fix the root cause. 3c647ee4bbb later reverted those
changes so that the more secure algorithm is used.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1450095
---
 daemon/libvirtd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 891238bcb..4a242e3e5 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -613,11 +613,11 @@ daemonSetupNetworking(virNetServerPtr srv,

 #if WITH_SASL
     if (config->auth_unix_rw == REMOTE_AUTH_SASL ||
-        config->auth_unix_ro == REMOTE_AUTH_SASL ||
+        (sock_path_ro && config->auth_unix_ro == REMOTE_AUTH_SASL) ||
 # if WITH_GNUTLS
-        config->auth_tls == REMOTE_AUTH_SASL ||
+        (config->listen_tls && config->auth_tls == REMOTE_AUTH_SASL) ||
 # endif
-        config->auth_tcp == REMOTE_AUTH_SASL) {
+        (config->listen_tcp && config->auth_tcp == REMOTE_AUTH_SASL)) {
         saslCtxt = virNetSASLContextNewServer(
             (const char *const*)config->sasl_allowed_username_list);
         if (!saslCtxt)
-- 
2.12.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] daemon: Don't initialize SASL context if not necessary
Posted by Daniel P. Berrange 7 years, 11 months ago
On Fri, Jun 02, 2017 at 02:10:25PM +0200, Peter Krempa wrote:
> SASL context would be initialized even if the corresponding TCP or TLS
> sockets are not enabled.
> 
> fe772f24a68 attempted to fix the symptom by commenting out the settings,
> but that did not fix the root cause. 3c647ee4bbb later reverted those
> changes so that the more secure algorithm is used.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1450095
> ---
>  daemon/libvirtd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 891238bcb..4a242e3e5 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -613,11 +613,11 @@ daemonSetupNetworking(virNetServerPtr srv,
> 
>  #if WITH_SASL
>      if (config->auth_unix_rw == REMOTE_AUTH_SASL ||
> -        config->auth_unix_ro == REMOTE_AUTH_SASL ||
> +        (sock_path_ro && config->auth_unix_ro == REMOTE_AUTH_SASL) ||
>  # if WITH_GNUTLS
> -        config->auth_tls == REMOTE_AUTH_SASL ||
> +        (config->listen_tls && config->auth_tls == REMOTE_AUTH_SASL) ||
>  # endif
> -        config->auth_tcp == REMOTE_AUTH_SASL) {
> +        (config->listen_tcp && config->auth_tcp == REMOTE_AUTH_SASL)) {
>          saslCtxt = virNetSASLContextNewServer(
>              (const char *const*)config->sasl_allowed_username_list);
>          if (!saslCtxt)

I think you need to check 'ipsock' too, since  listen_tls defaults
to 1, but is not used unless --listen is set.

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] daemon: Don't initialize SASL context if not necessary
Posted by Peter Krempa 7 years, 11 months ago
On Fri, Jun 02, 2017 at 13:28:31 +0100, Daniel Berrange wrote:
> On Fri, Jun 02, 2017 at 02:10:25PM +0200, Peter Krempa wrote:
> > SASL context would be initialized even if the corresponding TCP or TLS
> > sockets are not enabled.
> > 
> > fe772f24a68 attempted to fix the symptom by commenting out the settings,
> > but that did not fix the root cause. 3c647ee4bbb later reverted those
> > changes so that the more secure algorithm is used.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1450095
> > ---
> >  daemon/libvirtd.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> > index 891238bcb..4a242e3e5 100644
> > --- a/daemon/libvirtd.c
> > +++ b/daemon/libvirtd.c
> > @@ -613,11 +613,11 @@ daemonSetupNetworking(virNetServerPtr srv,
> > 
> >  #if WITH_SASL
> >      if (config->auth_unix_rw == REMOTE_AUTH_SASL ||
> > -        config->auth_unix_ro == REMOTE_AUTH_SASL ||
> > +        (sock_path_ro && config->auth_unix_ro == REMOTE_AUTH_SASL) ||
> >  # if WITH_GNUTLS
> > -        config->auth_tls == REMOTE_AUTH_SASL ||
> > +        (config->listen_tls && config->auth_tls == REMOTE_AUTH_SASL) ||
> >  # endif
> > -        config->auth_tcp == REMOTE_AUTH_SASL) {
> > +        (config->listen_tcp && config->auth_tcp == REMOTE_AUTH_SASL)) {
> >          saslCtxt = virNetSASLContextNewServer(
> >              (const char *const*)config->sasl_allowed_username_list);
> >          if (!saslCtxt)
> 
> I think you need to check 'ipsock' too, since  listen_tls defaults
> to 1, but is not used unless --listen is set.

Yes, I've just tested that option (after sending this obviously) and
came to the same conclusion.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list