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

[Eric Blake]

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.

> +++ b/src/rpc/virnettlscontext.c
> @@ -0,0 +1,892 @@
> +/*
> + * virnettlscontext.c: TLS encryption/x509 handling
> + *
> + * Copyright (C) 2010 Red Hat, Inc.

2011

> +#include <config.h>
> +
> +#include <unistd.h>
> +#ifdef HAVE_FNMATCH_H
> +# include <fnmatch.h>
> +#endif

This should be unconditional inclusion, thanks to gnulib.

> +
> +static int virNetTLSContextLoadCredentials(virNetTLSContextPtr ctxt,
> +                                           bool isServer,
> +                                           const char *cacert,
> +                                           const char *cacrl,
> +                                           const char *cert,
> +                                           const char *key)
> +{
> +    int ret = -1;
> +    int err;
> +
> +    if (cacert && cacert[0] != '\0') {
> +        if (virNetTLSContextCheckCertFile("CA certificate", cacert, false) < 0)
> +            goto cleanup;
> +
> +        VIR_DEBUG("loading CA cert from %s", cacert);
> +        err = gnutls_certificate_set_x509_trust_file(ctxt->x509cred,
> +                                                     cacert,
> +                                                     GNUTLS_X509_FMT_PEM);
> +        if (err < 0) {
> +            virNetError(VIR_ERR_SYSTEM_ERROR,
> +                        _("Unable to set x509 CA certificate: %s: %s"),
> +                        cacert, gnutls_strerror (err));

Consistency on ' (' vs. '(' for function calls.

> +        } else {
> +            VIR_DEBUG("Skipping non-existant cert %s key %s on client", cert, key);

s/existant/existent/

> +
> +/* Check DN is on tls_allowed_dn_list. */
> +static int
> +virNetTLSContextCheckDN(virNetTLSContextPtr ctxt,
> +                        const char *dname)
> +{
> +    const char *const*wildcards;
> +
> +    /* If the list is not set, allow any DN. */
> +    wildcards = ctxt->x509dnWhitelist;
> +    if (!wildcards)
> +        return 1;
> +
> +    while (*wildcards) {
> +#ifdef HAVE_FNMATCH_H
> +        int ret = fnmatch (*wildcards, dname, 0);

Use this unconditionally.

> +
> +#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?

> --- /dev/null
> +++ b/src/rpc/virnettlscontext.h
> @@ -0,0 +1,100 @@
> +/*
> + * virnettlscontext.h: TLS encryption/x509 handling
> + *
> + * Copyright (C) 2010 Red Hat, Inc.

2011

> +#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.

> +
> +void virNetTLSSessionFree(virNetTLSSessionPtr sess);

Should cfg.mk list this as a free-like function?

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110315/5be42ac8/attachment-0001.sig>