On Thu, Nov 29, 2007 at 01:08:52PM -0500, Daniel Veillard wrote: > On Thu, Nov 29, 2007 at 05:16:34PM +0000, Daniel P. Berrange wrote: > > This patch hooks up the basic authentication RPC calls, and the specific > > SASL implementation. The SASL impl can be enabled/disable via the configurre > > script with --without-sasl / --with-sasl - it'll auto-enable it if it finds > > the headers & libs OK. > > > > The sample /etc/sasl2/libvirt.conf file enables the DIGEST-MD5 mechanism > > by default, since it is by far the easiest to setup for admins. No need for > > a Kerberos server, or certificates - it just uses username/password which > > can be set with 'saslpasswd2 -a libvirt [username]' and a list of all active > > users viewed with 'sasldblistusers2 -a libvirt' > > Hum, shouldn't the spec file then > Requires: cyrus-sasl-md5 > to make sure this works ? Yes & no. Technically we only require the base library. Anything else is an admin choice. I'm inclined to add a dep on cyrus-sasl-md5 though because it will be our default auth mechanism, so it'll safe plenty of BZ reports if we just always require it. > [...] > > diff -r 1c3780349e89 libvirt.spec.in > > --- a/libvirt.spec.in Wed Nov 28 12:02:28 2007 -0500 > > +++ b/libvirt.spec.in Thu Nov 29 09:24:10 2007 -0500 > > @@ -16,6 +16,8 @@ Requires: dnsmasq > > Requires: dnsmasq > > Requires: bridge-utils > > Requires: iptables > > +Requires: cyrus-sasl > > +Requires: cyrus-sasl-gssapi > > Requires: cyrus-sasl-md5 ? Yes, and I'm going to remove the gssapi one, since that's not eenabled by default in my latest patches & only a minority of people will use it. > > @@ -730,15 +735,28 @@ static struct qemud_server *qemudInitial > > > > virStateInitialize(); > > > > +#if HAVE_SASL > > + if ((err = sasl_server_init(NULL, "libvirt")) != SASL_OK) { > > + qemudLog(QEMUD_ERR, "Failed to initialize SASL authentication %s", > > + sasl_errstring(err, NULL, NULL)); > > + goto cleanup; > > + } > > +#endif > > + > > So if libvirt was configured/compiled with SASL but for some reason we fail > to initialize SASL we can't start the daemon at all ? Isn't that a bit > excessive ? Failure of the sasl_server_init function is a pretty serious problem - something is majorly messed up in your OS install. We shouldn't pretend we can safely continue. In any case, the 4th patch in this serieschanges this call so that sasl_server_init is only run, if the SASL auth is actually configured on one of the sockets. So if someone did have a problem with this call failing, they could simply edit the config file & turn off SASL auth (or fix the root problem). > > [...] > > if (listen_tls) { > > if (remoteInitializeGnuTLS () < 0) > > goto cleanup; > > > > - if (remoteListenTCP (server, tls_port, 1) < 0) > > + if (remoteListenTCP (server, tls_port, 1, REMOTE_AUTH_NONE) < 0) > > goto cleanup; > > } > > } > > Looks to me that as long as TLS can still work we should not fail on > sasl_server_init error, right ? The 4th patch will make it possible to run SASL on all the sockets - UNIX, TCP and TLS. > > @@ -325,6 +356,12 @@ doRemoteOpen (virConnectPtr conn, struct > > } else > > port = NULL; /* Port not used for unix, ext. */ > > > > + > > + priv->hostname = strdup (uri->server ? uri->server : "localhost"); > > + if (!priv->hostname) { > > + error (NULL, VIR_ERR_NO_MEMORY, "allocating priv->hostname"); > > + goto failed; > > + } > > if (uri->user) { > > username = strdup (uri->user); > > if (!username) goto out_of_memory; > > Actually there we should looks for a password and store it, that's very > common and convenient, e.g. use > xen://foo:bar@server/ > > as the connection URI, libxml2 will just return the user as 'foo:bar' > which could subsequently be split here to store the password (bar). The virConnectCredentialPtr struct which is populated for the auth callback function contains a 'defresult' field where the default value of the credential should go. I intended to populate this value with the username part of the URI for VIR_CRED_AUTHNAME credentials, but forgot. Will add that in.... Using passwords in URIs is seriously frowned upon. URIs get into log files, in the command line ARGV, into gconf, into bug reports. We absolutely do not want passwords visible in any of those places. RFC 2396 explicitly recommends against using passwords in URIs "Some URL schemes use the format "user:password" in the userinfo field. This practice is NOT RECOMMENDED, because the passing of authentication information in clear text (such as URI) has proven to be a security risk in almost every case where it has been used." Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list