[libvirt] [PATCH v2] daemon: initialize GnuTLS

[Daniel Veillard]

On Thu, Aug 18, 2011 at 10:48:35AM +0200, Michal Privoznik wrote:
> When spice_tls is set but listen_tls is not, we don't initialize
> GnuTLS library. So any later gnutls call (e.g. during migration,
> where we initialize a certificate) will access uninitialized GnuTLS
> internal structs and throws an error.
> 
> Although, we might now initialize GnuTLS twice, it is safe according
> to the documentation:
> 
>     This function can be called many times,
>     but will only do something the first time.
> 
> This patch creates 2 functions: virNetTLSInit and virNetTLSDeinit
> with respect to written above.
> ---
> diff to v1:
> - moved from qemu to daemon
> - created special init function
> 
>  daemon/libvirtd.c          |    2 ++
>  src/rpc/virnettlscontext.c |   25 ++++++++++++++++++++++---
>  src/rpc/virnettlscontext.h |    3 +++
>  3 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index b99c637..0530ba5 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -1516,6 +1516,7 @@ int main(int argc, char **argv) {
>      virHookCall(VIR_HOOK_DRIVER_DAEMON, "-", VIR_HOOK_DAEMON_OP_START,
>                  0, "start", NULL);
>  
> +    virNetTLSInit();
>      if (daemonSetupNetworking(srv, config,
>                                sock_file, sock_file_ro,
>                                ipsock, privileged) < 0) {
> @@ -1554,6 +1555,7 @@ cleanup:
>      virNetServerProgramFree(qemuProgram);
>      virNetServerClose(srv);
>      virNetServerFree(srv);
> +    virNetTLSDeinit();
>      if (statuswrite != -1) {
>          if (ret != 0) {
>              /* Tell parent of daemon what failed */
> diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> index 19a9b25..8482eaf 100644
> --- a/src/rpc/virnettlscontext.c
> +++ b/src/rpc/virnettlscontext.c
> @@ -679,9 +679,6 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
>  
>      ctxt->refs = 1;
>  
> -    /* Initialise GnuTLS. */
> -    gnutls_global_init();
> -
>      if ((gnutlsdebug = getenv("LIBVIRT_GNUTLS_DEBUG")) != NULL) {
>          int val;
>          if (virStrToLong_i(gnutlsdebug, NULL, 10, &val) < 0)
> @@ -1399,3 +1396,25 @@ void virNetTLSSessionFree(virNetTLSSessionPtr sess)
>      virMutexDestroy(&sess->lock);
>      VIR_FREE(sess);
>  }
> +
> +/*
> + * This function MUST be called before any
> + * virNetTLS* because it initializes
> + * underlying GnuTLS library. According to
> + * it's documentation, it's safe to be called
> + * many times, but is not thread safe. Each
> + * call SHOULD be later followed by
> + * virNetTLSContextDeinit.
> + */
> +void virNetTLSInit(void)
> +{
> +    gnutls_global_init();
> +}
> +
> +/*
> + * See virNetTLSInit
> + */
> +void virNetTLSDeinit(void)
> +{
> +    gnutls_global_deinit();
> +}
> diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h
> index 641d67e..99f31b9 100644
> --- a/src/rpc/virnettlscontext.h
> +++ b/src/rpc/virnettlscontext.h
> @@ -30,6 +30,9 @@ typedef struct _virNetTLSSession virNetTLSSession;
>  typedef virNetTLSSession *virNetTLSSessionPtr;
>  
>  
> +void virNetTLSInit(void);
> +void virNetTLSDeinit(void);
> +
>  virNetTLSContextPtr virNetTLSContextNewServerPath(const char *pkipath,
>                                                    bool tryUserPkiPath,
>                                                    const char *const*x509dnWhitelist,

  I wonder about virNetTLSDeinit(), it basically call gnutls and tell
the library that we don't use it anymore. While gnutls_global_init()
might be safe, if they don't do reference counting
gnutls_global_deinit() may free up data still used by another library
under the hood.
  The comment seems to imply that gnutls_global_(de)init
do reference counting and ACK in this case but if not I would
just drop the virNetTLSDeinit() part, we were not calling
gnutls_global_deinit before anyway.

  ACK if ref counting in gnutls confirmed

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/