[libvirt] [PATCH 03/10] Generic module for handling TLS encryption and x509 certs

[Daniel P. Berrange]

On Tue, Mar 15, 2011 at 04:34:33PM -0600, Eric Blake wrote:
> On 03/15/2011 11:51 AM, Daniel P. Berrange wrote:
> > This provides two modules for handling TLS
> > 
> >  * virNetTLSContext provides the process-wide state, in particular
> >    all the x509 credentials, DH params and x509 whitelists
> >  * virNetTLSSession provides the per-connection state, ie the
> >    TLS session itself.
> > 
> > The virNetTLSContext provides APIs for validating a TLS session's
> > x509 credentials. The virNetTLSSession includes APIs for performing
> > the initial TLS handshake and sending/recving encrypted data
> > 
> > * src/Makefile.am: Add to libvirt-net-rpc.la
> > * src/rpc/virnettlscontext.c, src/rpc/virnettlscontext.h: Generic
> >   TLS handling code
> > ---
> >  configure.ac               |    2 +-
> >  po/POTFILES.in             |    1 +
> >  src/Makefile.am            |    5 +-
> >  src/rpc/virnettlscontext.c |  892 ++++++++++++++++++++++++++++++++++++++++++++
> >  src/rpc/virnettlscontext.h |  100 +++++
> >  5 files changed, 998 insertions(+), 2 deletions(-)
> >  create mode 100644 src/rpc/virnettlscontext.c
> >  create mode 100644 src/rpc/virnettlscontext.h
> 
> No src/libvirt_private.syms entries?
> 
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 49403dd..81bad91 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -134,7 +134,7 @@ LIBS=$old_libs
> >  dnl Availability of various common headers (non-fatal if missing).
> >  AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/syslimits.h sys/un.h \
> >    sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
> > -  sys/un.h sys/syscall.h netinet/tcp.h])
> > +  sys/un.h sys/syscall.h netinet/tcp.h fnmatch.h])
> 
> Gnulib provides fnmatch.  We shouldn't be adding this check, but modify
> bootstrap.conf instead.

Ah, I didn't know this. We already use fnmatch in libvirtd, but
hadn't added gnulib module for it.

> > +
> > +#if 0
> > +    PROBE(CLIENT_TLS_ALLOW, "fd=%d, name=%s",
> > +          virNetServerClientGetFD(client), name);
> > +#endif
> > +    return 0;
> 
> Are these PROBE() statements worth keeping?  Are they for debug, for
> systemtap probe points, or something else?

They're an item I need to fix before I finally convert
libvirtd. I will address that as a followup patch though
once the generic code is committed.

> > +#ifndef __VIR_NET_TLS_CONTEXT_H__
> > +# define __VIR_NET_TLS_CONTEXT_H__
> > +
> > +# include <stdbool.h>
> 
> Is this redundant, now that "internal.h" guarantees this and all .c
> files should be including "internal.h"?  I don't see any other headers
> that include <stdbool.h> since commit 3541672.

Yes, I forgot to remove this one


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|