Re: [Libvir] PATCH: RFC supporting SASL authentication

On Thu, Oct 18, 2007 at 10:44:57AM -0400, Daniel Veillard wrote: > On Wed, Oct 17, 2007 at 10:21:56PM +0100, Daniel P. Berrange wrote: > > A legacy client getting back REMOTE_AUTH code will just quit the connection > > attempt since they don't support authentication. If the admin so desires > > they can still provide the TLS socket in a non-authenticated mode and only > > turn on SASL for the TCP socket. So the decision about whether to enable > > legacy clients is admin controlled. This is the best we can do. > > > > A new client getting back REMOTE_AUTH code will then read the remote_auth_type > > off the wire. If the requested type is one that the client supports then it > > can begin the authentication process, otherwise we virRaiseError and stop > > connecting. > > Yup the fallbacks are fine by me. > The only thing that could be improved in the global picture would be a way > for the client to export the authentication methods it supports on the first > step, unfortunately that would break wire compatibility (unless XDR has some > way to carry metadata payload, but I don't think taht's the case). Having the client export its auth types isn't really neccessary, since the server only supports a single primary auth type for any given socket. The SASL auth type, does have many sub-auth types - eg GSSAPI, PLAIN, OTP, etc and that's what the 'list mechanisms' called below provides info about. The client asks server for all SASL mechanisms that are allowed - it then chooss the best one it supports. If we need this for the primary libvirt auth types too, I'd change the wire protocol a little, so that when sending the REMOTE_AUTH code, we would send back a list of allowed primary auth types. The client can then invoke the RPC call for whichever one they support. This would let us support both PolicyKit & SASL on the same socket, which could be nice i guess.

> > So, to implement this if it neccessary to define 3 new RPC calls internal > > to the remote driver/daemon (aka not exposed to public API like the rest > > of the RPC calls). These are: > > > > REMOTE_PROC_AUTH_SASL_INIT = 66, > > REMOTE_PROC_AUTH_SASL_START = 67, > > REMOTE_PROC_AUTH_SASL_STEP = 68 > > > > These are basically just punting back & forth the data going in & out of > > the appropriate SASL apis. sasl_{server,client}_{new,start,step} See > > the man pages for more info. > > > > So on the server end, if a socket is configured to require SASL auth, the > > server will reject all RPC calls *except* for those three above with the > > REMOTE_AUTH code. Once SASL auth has completed, it will allow all the > > normal RPC calls. The effect is basically that the client is not able to > > I'm really wondering about the plural for 'normal RPC calls', i.e. once > the caller has been authentified are we sure we can keep the socket alive > on both ends for the duration of the virConnectPtr connection ? I hope > it's the case for such complex handshake and that at the client level > this stays true (for example remote monitoring via virt-manager effectively > keeps the same connection). I'm not sure what you mean here ? The whole remote session, takes place over a TCP socket. We are requiring authentication upfront, before allowing the virConnectOpen call. Once the client closes the socket, they have to open a new one & start the whole procss from scratch. Both the client & server cope with the other end dropping the connection in the middle of a handshake by just cleaning up their state.

> > The SASL implementation details > > ------------------------------- > > > > This is the bare minimum SASL integration. I have not attempted to hook > > up any callbacks for gathering credentials. This basically means that the > > only SASL mechanism which works is Kerberos GSSAPI - its credentials are > > fetched out-of-band & so don't require callbacks. We do need to consider > > callbacks later so we can do username/password auth, and all the various > > other methods SASL has. > > Designing an asynchronous API for generic authentication which could > be used by the multiple potential back-ends sounds a challenge to me, > first because callbacks in APIs always make life harder (think threading > and asynchronous operations), but also because when it comes to security > it's so easy to get things wrong without noticing ! Well if we registered a global function virConnectCallbacks(...) to register a set of callbacks that would be sufficient. The callbacks would be guarenteed to always execute in the same thread as the virConnectOpen() call itself. So we'd just have to document that you should make sure that the callbacks are able to run in parallel - ie multiple threads can be in virConnectOpen calls at once. Threading isn't really the thing to worry about here. The hard bit is actally

designing what data goes in & out of the callbacks. SASL has a pretty large list of possible callbacks. I listed a couple here http://www.redhat.com/archives/libvir-list/2007-January/msg00024.html but now I've looked at SASL more closely there's many more to consider & I think I can do a better job than my original proposal.

> > I have set the wire packet size for the SASL negotiation to 8192 bytes at > > a time. This has been sufficient so far, but I need to validate this > > before we commit, because this will be wire ABI sensitive. Or I could > > change the XDR spec to be variable length. Anyway needs re-visiting > > Hum, we will need to ask some security expert there, yup. Well its really a SASL implementation detail. There is a way you can specify a maximum network packet size you're prepared to accept. I need to check whether this appllies to the initial handshake, or just the post-handshake session encryption. > > This only deals with authentication. I have not attempted any authoriztion > > controls. So anyone who has a valid kerberos principle can connect to the > > server. We clearly need a local group list as we do for the x509 client > > certificates. Ultimately we could try LDAP lookups & other intersting > > suggestions. > > We definitely need an extensible framework in any case, there is just > too many possibilities, and it's usually corporate policies that could > be fairly rigid, we need to be able to adapt. Yep, not sure what todo here really, aisde from a local group file. > > diff -r 2ebd10b676a9 configure.in > > --- a/configure.in Wed Oct 17 15:03:04 2007 -0400 > > +++ b/configure.in Wed Oct 17 15:03:07 2007 -0400 > > @@ -329,6 +329,40 @@ AC_CHECK_TYPE(gnutls_session, > > [#include <gnutls/gnutls.h>]) > > CFLAGS="$old_cflags" > > LDFLAGS="$old_ldflags" > > + > > + > > +dnl Cyrus SASL > > +AC_ARG_WITH(sasl, > > + [ --with-sasl use cyrus SASL for authentication], > > + [], > > + [with_sasl=yes]) > > + > > +SASL_CFLAGS= > > +SASL_LIBS= > > +if test "$with_sasl" != "no"; then > > + if test "$with_sasl" != "yes"; then > > + SASL_CFLAGS="-I$with_sasl" > > + SASL_LIBS="-L$with_sasl" > > Hum, using the same flags for -I and -L looks suspicious to me Why, it just lets you do --with-sasl=/opt/sasl Yes, it assumes people install the libraries & headers into the same prefix, but frankly that's not unreasonable.

> > + err = sasl_server_new("libvirt", > > + NULL, /* XXX Server FQDN */ > > + NULL, /* XXX User realm */ > > + NULL, /* XXX IP local */ > > + NULL, /* XXX IP remote */ > > + NULL, /* XXX Callbacks */ > > > Hum, what kind of callback is allowed there ? *Many* different callbacks. Rather than listing them here, take a look at /usr/include/sasl/sasl.h - search for 'struct sasl_callback' - following the struct definition are many callback function signatures. Also look at the 'sasl_callbacks' many page.

This is specifically for server side calbacks. These deal with stuff like pluggable password checking API, and an authorization checking API. There is separate client side callbacks elsewhere, but I'd probably not use them. Instead I'd go for what SASL calls 'interactions'. Basically the 'step' API will fail & give you back a list of interactions that are needed. The advantage of this is that you can feed these back to the client app all in one go, rather than one-at-a-time. This makes is practical to build a UI prompting for all data at once.

> > + err = sasl_listmech(client->saslconn, > > + NULL, /* XXX username */ > > + "", /* Prefix */ > > + ",", /* Separator */ > > + "", /* Suffix */ > > + &mechlist, > > + NULL, > > + NULL); > > Also we start to see the identity problems showing up there. Maybe I'm > logged as veillard on the monitoring machine but the realm used to monitor > domain instances on the cluster is 'virtadmin' and I could have token for > both kind but we risk using the wrong one when doing the connection. This is server side & in fact the username field here is rarly used. The purpose here is to allow you to filter the list of authentication schemes given back to the client based on client's username. Trouble is you can't really trust the username at this point since we've done no auth, so it is of limited use. The docs say you rarely need to worry about this field, just leaving it NULL.

> > [...] > > +#if HAVE_SASL > > +/* Perform the SASL authentication process > > + * > > + * XXX negotiate a session encryption layer for non-TLS sockets > > + * XXX fetch credentials from a libvirt client app callback > > + * XXX fill in local/remote IP details > > + * XXX use the real hostname from the connect URI > > + * XXX max packet size spec > > + * XXX better mechanism negotiation ? Ask client app ? > > + */ > > +static int > > +remoteAuthSasl (virConnectPtr conn, struct private_data *priv, > > + int in_open /* if we are in virConnectOpen */) { [snip] > Hum, head hurts at this point ... There is alot of line noise there with the error handling, but bascialy it comes down to -> sasl_client_new -> Run the REMOTE_PROC_AUTH_SASL_INIT rpc call -> sasl_client_start -> Run the REMOTE_PROC_AUTH_SASL_START rpc call repeat until authenticated: -> sasl_client_step -> Run the REMOTE_PROC_AUTH_SASL_STEP rpc call Each of teh rpc calls basically ends up doing the equivalent SASL func on the server eg, sasl_server_new, sasl_server_start and sasl_server_step So it is pretty symmetric in sequence of calls.