[Qemu-devel] [PATCH 0/2] crypto: add flexibility in TLS priority setup

[Qemu-devel] [PATCH 0/2] crypto: add flexibility in TLS priority setup

[Daniel P. Berrange]

This small series provides two new ways to customize the
default TLS priority QEMU uses, one at build time and one
at runtime.

This means QEMU is no longer forced to use the lowest
common denominator crypto choices from the global system
defaults.

Daniel P. Berrange (2):
  crypto: add support for TLS priority string override
  crypto: allow default TLS priority to be chosen at build time

 configure                 |  6 ++++++
 crypto/tlscreds.c         | 26 ++++++++++++++++++++++++++
 crypto/tlssession.c       | 26 +++++++++++++++++++-------
 include/crypto/tlscreds.h |  1 +
 4 files changed, 52 insertions(+), 7 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH 1/2] crypto: add support for TLS priority string override

[Daniel P. Berrange]

The gnutls default priority is either "NORMAL" (most historical
versions of gnutls) which is a built-in label in gnutls code,
or "@SYSTEM" (latest gnutls on Fedora at least) which refers
to an admin customizable entry in a gnutls config file.

Regardless of which default is used by a distro, they are both
global defaults applying to all applications using gnutls. If
a single application on the system needs to use a weaker set
of crypto priorities, this potentially forces the weakness onto
all applications. Or conversely if a single application wants a
strong default than all others, it can't do this via the global
config file.

This adds an extra parameter to the tls credential object which
allows the mgmt app / user to explicitly provide a priority
string to QEMU when configuring TLS.

For example, to use the "NORMAL" priority, but disable SSL 3.0
one can now configure QEMU thus:

  $QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
                priority="NORMAL:-VERS-SSL3.0" \
        ..other args...

If creating tls-creds-anon, whatever priority the user specifies
will always have "+ANON-DH" appended to it, since that's mandatory
to make the anonymous credentials work.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/tlscreds.c         | 26 ++++++++++++++++++++++++++
 crypto/tlssession.c       | 26 +++++++++++++++++++-------
 include/crypto/tlscreds.h |  1 +
 3 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
index 1620e12..a896553 100644
--- a/crypto/tlscreds.c
+++ b/crypto/tlscreds.c
@@ -179,6 +179,27 @@ qcrypto_tls_creds_prop_get_dir(Object *obj,
 
 
 static void
+qcrypto_tls_creds_prop_set_priority(Object *obj,
+                                    const char *value,
+                                    Error **errp G_GNUC_UNUSED)
+{
+    QCryptoTLSCreds *creds = QCRYPTO_TLS_CREDS(obj);
+
+    creds->priority = g_strdup(value);
+}
+
+
+static char *
+qcrypto_tls_creds_prop_get_priority(Object *obj,
+                                    Error **errp G_GNUC_UNUSED)
+{
+    QCryptoTLSCreds *creds = QCRYPTO_TLS_CREDS(obj);
+
+    return g_strdup(creds->priority);
+}
+
+
+static void
 qcrypto_tls_creds_prop_set_endpoint(Object *obj,
                                     int value,
                                     Error **errp G_GNUC_UNUSED)
@@ -216,6 +237,10 @@ qcrypto_tls_creds_class_init(ObjectClass *oc, void *data)
                                    qcrypto_tls_creds_prop_get_endpoint,
                                    qcrypto_tls_creds_prop_set_endpoint,
                                    NULL);
+    object_class_property_add_str(oc, "priority",
+                                  qcrypto_tls_creds_prop_get_priority,
+                                  qcrypto_tls_creds_prop_set_priority,
+                                  NULL);
 }
 
 
@@ -234,6 +259,7 @@ qcrypto_tls_creds_finalize(Object *obj)
     QCryptoTLSCreds *creds = QCRYPTO_TLS_CREDS(obj);
 
     g_free(creds->dir);
+    g_free(creds->priority);
 }
 
 
diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index a543e5a..2112d29 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -132,14 +132,22 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds,
     if (object_dynamic_cast(OBJECT(creds),
                             TYPE_QCRYPTO_TLS_CREDS_ANON)) {
         QCryptoTLSCredsAnon *acreds = QCRYPTO_TLS_CREDS_ANON(creds);
+        char *prio;
 
-        ret = gnutls_priority_set_direct(session->handle,
-                                         "NORMAL:+ANON-DH", NULL);
+        if (creds->priority != NULL) {
+            prio = g_strdup_printf("%s:+ANON-DH", creds->priority);
+        } else {
+            prio = g_strdup("NORMAL:+ANON-DH");
+        }
+
+        ret = gnutls_priority_set_direct(session->handle, prio, NULL);
         if (ret < 0) {
-            error_setg(errp, "Unable to set TLS session priority: %s",
-                       gnutls_strerror(ret));
+            error_setg(errp, "Unable to set TLS session priority %s: %s",
+                       prio, gnutls_strerror(ret));
+            g_free(prio);
             goto error;
         }
+        g_free(prio);
         if (creds->endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
             ret = gnutls_credentials_set(session->handle,
                                          GNUTLS_CRD_ANON,
@@ -157,11 +165,15 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds,
     } else if (object_dynamic_cast(OBJECT(creds),
                                    TYPE_QCRYPTO_TLS_CREDS_X509)) {
         QCryptoTLSCredsX509 *tcreds = QCRYPTO_TLS_CREDS_X509(creds);
+        const char *prio = creds->priority;
+        if (!prio) {
+            prio = "NORMAL";
+        }
 
-        ret = gnutls_set_default_priority(session->handle);
+        ret = gnutls_priority_set_direct(session->handle, prio, NULL);
         if (ret < 0) {
-            error_setg(errp, "Cannot set default TLS session priority: %s",
-                       gnutls_strerror(ret));
+            error_setg(errp, "Cannot set default TLS session priority %s: %s",
+                       prio, gnutls_strerror(ret));
             goto error;
         }
         ret = gnutls_credentials_set(session->handle,
diff --git a/include/crypto/tlscreds.h b/include/crypto/tlscreds.h
index 8e2babd..59e9187 100644
--- a/include/crypto/tlscreds.h
+++ b/include/crypto/tlscreds.h
@@ -54,6 +54,7 @@ struct QCryptoTLSCreds {
     gnutls_dh_params_t dh_params;
 #endif
     bool verifyPeer;
+    char *priority;
 };
 
 
-- 
2.5.5

[Qemu-devel] [PATCH 2/2] crypto: allow default TLS priority to be chosen at build time

[Daniel P. Berrange]

Modern gnutls can use a global config file to control the
crypto priority settings for TLS connections. For example
the priority string "@SYSTEM" instructs gnutls to find the
priority setting named "SYSTEM" in the global config file.

Latest gnutls GIT codebase gained the ability to reference
multiple priority strings in the config file, with the first
one that is found to existing winning. This means it is now
possible to configure QEMU out of the box with a default
priority of "@QEMU,SYSTEM", which says to look for the
settings "QEMU" first, and if not found, use the "SYSTEM"
settings.

To make use of this facility, we introduce the ability to
set the QEMU default priority at build time via a new
configure argument.  It is anticipated that distro vendors
will set this when building QEMU to a suitable value for
use with distro crypto policy setup. eg current Fedora
would run

 ./configure --tls-priority=@SYSTEM

while future Fedora would run

 ./configure --tls-priority=@QEMU,SYSTEM

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 configure           | 6 ++++++
 crypto/tlssession.c | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 8c2f90b..2afaf28 100755
--- a/configure
+++ b/configure
@@ -306,6 +306,7 @@ archipelago="no"
 gtk=""
 gtkabi=""
 gtk_gl="no"
+tls_priority="NORMAL"
 gnutls=""
 gnutls_hash=""
 gnutls_rnd=""
@@ -1098,6 +1099,8 @@ for opt do
   ;;
   --enable-gtk) gtk="yes"
   ;;
+  --tls-priority=*) tls_priority="$optarg"
+  ;;
   --disable-gnutls) gnutls="no"
   ;;
   --enable-gnutls) gnutls="yes"
@@ -1302,6 +1305,7 @@ Advanced options (experts only):
   --disable-blobs          disable installing provided firmware blobs
   --with-vss-sdk=SDK-path  enable Windows VSS support in QEMU Guest Agent
   --with-win-sdk=SDK-path  path to Windows Platform SDK (to build VSS .tlb)
+  --tls-priority           default TLS protocol/cipher priority string
 
 Optional features, enabled with --enable-FEATURE and
 disabled with --disable-FEATURE, default is enabled if available:
@@ -4815,6 +4819,7 @@ echo "SDL support       $sdl $(echo_version $sdl $sdlversion)"
 echo "GTK support       $gtk $(echo_version $gtk $gtk_version)"
 echo "GTK GL support    $gtk_gl"
 echo "VTE support       $vte $(echo_version $vte $vteversion)"
+echo "TLS priority      $tls_priority"
 echo "GNUTLS support    $gnutls"
 echo "GNUTLS hash       $gnutls_hash"
 echo "GNUTLS rnd        $gnutls_rnd"
@@ -5180,6 +5185,7 @@ if test "$gtk" = "yes" ; then
     echo "CONFIG_GTK_GL=y" >> $config_host_mak
   fi
 fi
+echo "CONFIG_TLS_PRIORITY=\"$tls_priority\"" >> $config_host_mak
 if test "$gnutls" = "yes" ; then
   echo "CONFIG_GNUTLS=y" >> $config_host_mak
 fi
diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index 2112d29..2de42c6 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -137,7 +137,7 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds,
         if (creds->priority != NULL) {
             prio = g_strdup_printf("%s:+ANON-DH", creds->priority);
         } else {
-            prio = g_strdup("NORMAL:+ANON-DH");
+            prio = g_strdup(CONFIG_TLS_PRIORITY ":+ANON-DH");
         }
 
         ret = gnutls_priority_set_direct(session->handle, prio, NULL);
@@ -167,7 +167,7 @@ qcrypto_tls_session_new(QCryptoTLSCreds *creds,
         QCryptoTLSCredsX509 *tcreds = QCRYPTO_TLS_CREDS_X509(creds);
         const char *prio = creds->priority;
         if (!prio) {
-            prio = "NORMAL";
+            prio = CONFIG_TLS_PRIORITY;
         }
 
         ret = gnutls_priority_set_direct(session->handle, prio, NULL);
-- 
2.5.5

Re: [Qemu-devel] [PATCH 0/2] crypto: add flexibility in TLS priority setup

[Daniel P. Berrange]

On Mon, Jun 13, 2016 at 01:13:51PM +0100, Daniel P. Berrange wrote:
> This small series provides two new ways to customize the
> default TLS priority QEMU uses, one at build time and one
> at runtime.
> 
> This means QEMU is no longer forced to use the lowest
> common denominator crypto choices from the global system
> defaults.
> 
> Daniel P. Berrange (2):
>   crypto: add support for TLS priority string override
>   crypto: allow default TLS priority to be chosen at build time

If anyone fancies reviewing these patches, I'd love to see
comments, otherwise I'll go ahead & submit a pull request
in a day or two.

Regards,
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 :|