Re: [libvirt] [PATCH 03/10] Generic module for handling TLS encryption and x509 certs
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 :|