Merge remote-tracking branch 'danpb/tags/pull-qcrypto-2017-05-09-1' into staging

Merge qcrypto 2017/05/09 v1

# gpg: Signature made Tue 09 May 2017 09:43:47 AM EDT
# gpg:                using RSA key 0xBE86EBB415104FDF
# gpg: Good signature from "Daniel P. Berrange <dan@berrange.com>"
# gpg:                 aka "Daniel P. Berrange <berrange@redhat.com>"
# Primary key fingerprint: DAF3 A6FD B26B 6291 2D0E  8E3F BE86 EBB4 1510 4FDF

* danpb/tags/pull-qcrypto-2017-05-09-1:
  crypto: qcrypto_random_bytes() now works on windows w/o any other crypto libs
  crypto: move 'opaque' parameter to (nearly) the end of parameter list
  List SASL config file under the cryptography maintainer's realm
  Default to GSSAPI (Kerberos) instead of DIGEST-MD5 for SASL
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

MAINTAINERS

@@ -1487,6 +1487,7 @@ S: Maintained
 F: crypto/
 F: include/crypto/
 F: tests/test-crypto-*
+F: qemu.sasl
 
 Coroutines
 M: Stefan Hajnoczi <stefanha@redhat.com>

block/crypto.c

@@ -56,10 +56,10 @@ static int block_crypto_probe_generic(QCryptoBlockFormat format,
 
 
 static ssize_t block_crypto_read_func(QCryptoBlock *block,
-                                      void *opaque,
                                       size_t offset,
                                       uint8_t *buf,
                                       size_t buflen,
+                                      void *opaque,
                                       Error **errp)
 {
     BlockDriverState *bs = opaque;
@@ -83,10 +83,10 @@ struct BlockCryptoCreateData {
 
 
 static ssize_t block_crypto_write_func(QCryptoBlock *block,
-                                       void *opaque,
                                        size_t offset,
                                        const uint8_t *buf,
                                        size_t buflen,
+                                       void *opaque,
                                        Error **errp)
 {
     struct BlockCryptoCreateData *data = opaque;
@@ -102,8 +102,8 @@ static ssize_t block_crypto_write_func(QCryptoBlock *block,
 
 
 static ssize_t block_crypto_init_func(QCryptoBlock *block,
-                                      void *opaque,
                                       size_t headerlen,
+                                      void *opaque,
                                       Error **errp)
 {
     struct BlockCryptoCreateData *data = opaque;

crypto/block-luks.c

@@ -473,9 +473,9 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
      * then encrypted.
      */
     rv = readfunc(block,
-                  opaque,
                   slot->key_offset * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
                   splitkey, splitkeylen,
+                  opaque,
                   errp);
     if (rv < 0) {
         goto cleanup;
@@ -676,9 +676,10 @@ qcrypto_block_luks_open(QCryptoBlock *block,
 
     /* Read the entire LUKS header, minus the key material from
      * the underlying device */
-    rv = readfunc(block, opaque, 0,
+    rv = readfunc(block, 0,
                   (uint8_t *)&luks->header,
                   sizeof(luks->header),
+                  opaque,
                   errp);
     if (rv < 0) {
         ret = rv;
@@ -1245,7 +1246,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE;
 
     /* Reserve header space to match payload offset */
-    initfunc(block, opaque, block->payload_offset, &local_err);
+    initfunc(block, block->payload_offset, opaque, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto error;
@@ -1267,9 +1268,10 @@ qcrypto_block_luks_create(QCryptoBlock *block,
 
 
     /* Write out the partition header and key slot headers */
-    writefunc(block, opaque, 0,
+    writefunc(block, 0,
               (const uint8_t *)&luks->header,
               sizeof(luks->header),
+              opaque,
               &local_err);
 
     /* Delay checking local_err until we've byte-swapped */
@@ -1295,10 +1297,11 @@ qcrypto_block_luks_create(QCryptoBlock *block,
 
     /* Write out the master key material, starting at the
      * sector immediately following the partition header. */
-    if (writefunc(block, opaque,
+    if (writefunc(block,
                   luks->header.key_slots[0].key_offset *
                   QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
                   splitkey, splitkeylen,
+                  opaque,
                   errp) != splitkeylen) {
         goto error;
     }

crypto/init.c

@@ -32,6 +32,8 @@
 #include <gcrypt.h>
 #endif
 
+#include "crypto/random.h"
+
 /* #define DEBUG_GNUTLS */
 
 /*
@@ -146,5 +148,9 @@ int qcrypto_init(Error **errp)
     gcry_control(GCRYCTL_INITIALIZATION_FINISHED, 0);
 #endif
 
+    if (qcrypto_random_init(errp) < 0) {
+        return -1;
+    }
+
     return 0;
 }

crypto/random-gcrypt.c

@@ -31,3 +31,5 @@ int qcrypto_random_bytes(uint8_t *buf,
     gcry_randomize(buf, buflen, GCRY_STRONG_RANDOM);
     return 0;
 }
+
+int qcrypto_random_init(Error **errp G_GNUC_UNUSED) { return 0; }

crypto/random-gnutls.c

@@ -41,3 +41,6 @@ int qcrypto_random_bytes(uint8_t *buf,
 
     return 0;
 }
+
+
+int qcrypto_random_init(Error **errp G_GNUC_UNUSED) { return 0; }

crypto/random-platform.c

@@ -22,14 +22,16 @@
 
 #include "crypto/random.h"
 
-int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
-                         size_t buflen G_GNUC_UNUSED,
-                         Error **errp)
-{
-    int fd;
-    int ret = -1;
-    int got;
+#ifdef _WIN32
+#include <Wincrypt.h>
+static HCRYPTPROV hCryptProv;
+#else
+static int fd; /* a file handle to either /dev/urandom or /dev/random */
+#endif
 
+int qcrypto_random_init(Error **errp)
+{
+#ifndef _WIN32
     /* TBD perhaps also add support for BSD getentropy / Linux
      * getrandom syscalls directly */
     fd = open("/dev/urandom", O_RDONLY);
@@ -41,6 +43,25 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
         error_setg(errp, "No /dev/urandom or /dev/random found");
         return -1;
     }
+#else
+    if (!CryptAcquireContext(&hCryptProv, NULL, NULL, PROV_RSA_FULL,
+                             CRYPT_SILENT | CRYPT_VERIFYCONTEXT)) {
+        error_setg_win32(errp, GetLastError(),
+                         "Unable to create cryptographic provider");
+        return -1;
+    }
+#endif
+
+    return 0;
+}
+
+int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
+                         size_t buflen G_GNUC_UNUSED,
+                         Error **errp)
+{
+#ifndef _WIN32
+    int ret = -1;
+    int got;
 
     while (buflen > 0) {
         got = read(fd, buf, buflen);
@@ -59,6 +80,14 @@ int qcrypto_random_bytes(uint8_t *buf G_GNUC_UNUSED,
 
     ret = 0;
  cleanup:
-    close(fd);
     return ret;
+#else
+    if (!CryptGenRandom(hCryptProv, buflen, buf)) {
+        error_setg_win32(errp, GetLastError(),
+                         "Unable to read random bytes");
+        return -1;
+    }
+
+    return 0;
+#endif
 }

include/crypto/block.h

@@ -30,22 +30,22 @@ typedef struct QCryptoBlock QCryptoBlock;
  * and QCryptoBlockOpenOptions in qapi/crypto.json */
 
 typedef ssize_t (*QCryptoBlockReadFunc)(QCryptoBlock *block,
-                                        void *opaque,
                                         size_t offset,
                                         uint8_t *buf,
                                         size_t buflen,
+                                        void *opaque,
                                         Error **errp);
 
 typedef ssize_t (*QCryptoBlockInitFunc)(QCryptoBlock *block,
-                                        void *opaque,
                                         size_t headerlen,
+                                        void *opaque,
                                         Error **errp);
 
 typedef ssize_t (*QCryptoBlockWriteFunc)(QCryptoBlock *block,
-                                         void *opaque,
                                          size_t offset,
                                          const uint8_t *buf,
                                          size_t buflen,
+                                         void *opaque,
                                          Error **errp);
 
 /**

include/crypto/random.h

@@ -40,5 +40,14 @@ int qcrypto_random_bytes(uint8_t *buf,
                          size_t buflen,
                          Error **errp);
 
+/**
+ * qcrypto_random_init:
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Initializes the handles used by qcrypto_random_bytes
+ *
+ * Returns 0 on success, -1 on error
+ */
+int qcrypto_random_init(Error **errp);
 
 #endif /* QCRYPTO_RANDOM_H */

qemu-doc.texi

@@ -1732,37 +1732,45 @@ SASL service config /etc/sasl2/qemu.conf. If running QEMU as an
 unprivileged user, an environment variable SASL_CONF_PATH can be used
 to make it search alternate locations for the service config.
 
-The default configuration might contain
+If the TLS option is enabled for VNC, then it will provide session encryption,
+otherwise the SASL mechanism will have to provide encryption. In the latter
+case the list of possible plugins that can be used is drastically reduced. In
+fact only the GSSAPI SASL mechanism provides an acceptable level of security
+by modern standards. Previous versions of QEMU referred to the DIGEST-MD5
+mechanism, however, it has multiple serious flaws described in detail in
+RFC 6331 and thus should never be used any more. The SCRAM-SHA-1 mechanism
+provides a simple username/password auth facility similar to DIGEST-MD5, but
+does not support session encryption, so can only be used in combination with
+TLS.
+
+When not using TLS the recommended configuration is
 
 @example
-mech_list: digest-md5
-sasldb_path: /etc/qemu/passwd.db
+mech_list: gssapi
+keytab: /etc/qemu/krb5.tab
 @end example
 
-This says to use the 'Digest MD5' mechanism, which is similar to the HTTP
-Digest-MD5 mechanism. The list of valid usernames & passwords is maintained
-in the /etc/qemu/passwd.db file, and can be updated using the saslpasswd2
-command. While this mechanism is easy to configure and use, it is not
-considered secure by modern standards, so only suitable for developers /
-ad-hoc testing.
+This says to use the 'GSSAPI' mechanism with the Kerberos v5 protocol, with
+the server principal stored in /etc/qemu/krb5.tab. For this to work the
+administrator of your KDC must generate a Kerberos principal for the server,
+with a name of 'qemu/somehost.example.com@@EXAMPLE.COM' replacing
+'somehost.example.com' with the fully qualified host name of the machine
+running QEMU, and 'EXAMPLE.COM' with the Kerberos Realm.
 
-A more serious deployment might use Kerberos, which is done with the 'gssapi'
-mechanism
+When using TLS, if username+password authentication is desired, then a
+reasonable configuration is
 
 @example
-mech_list: gssapi
-keytab: /etc/qemu/krb5.tab
+mech_list: scram-sha-1
+sasldb_path: /etc/qemu/passwd.db
 @end example
 
-For this to work the administrator of your KDC must generate a Kerberos
-principal for the server, with a name of  'qemu/somehost.example.com@@EXAMPLE.COM'
-replacing 'somehost.example.com' with the fully qualified host name of the
-machine running QEMU, and 'EXAMPLE.COM' with the Kerberos Realm.
+The saslpasswd2 program can be used to populate the passwd.db file with
+accounts.
 
-Other configurations will be left as an exercise for the reader. It should
-be noted that only Digest-MD5 and GSSAPI provides a SSF layer for data
-encryption. For all other mechanisms, VNC should always be configured to
-use TLS and x509 certificates to protect security credentials from snooping.
+Other SASL configurations will be left as an exercise for the reader. Note that
+all mechanisms except GSSAPI, should be combined with use of TLS to ensure a
+secure data channel.
 
 @node gdb_usage
 @section GDB usage

qemu.sasl

-# If you want to use the non-TLS socket, then you *must* include
-# the GSSAPI or DIGEST-MD5 mechanisms, because they are the only
-# ones that can offer session encryption as well as authentication.
+# If you want to use VNC remotely without TLS, then you *must*
+# pick a mechanism which provides session encryption as well
+# as authentication.
 #
-# If you're only using TLS, then you can turn on any mechanisms
+# If you are only using TLS, then you can turn on any mechanisms
 # you like for authentication, because TLS provides the encryption
 #
-# Default to a simple username+password mechanism
-# NB digest-md5 is no longer considered secure by current standards
-mech_list: digest-md5
+# If you are only using UNIX sockets then encryption is not
+# required at all.
+#
+# NB, previously DIGEST-MD5 was set as the default mechanism for
+# QEMU VNC. Per RFC 6331 this is vulnerable to many serious security
+# flaws as should no longer be used. Thus GSSAPI is now the default.
+#
+# To use GSSAPI requires that a QEMU service principal is
+# added to the Kerberos server for each host running QEMU.
+# This principal needs to be exported to the keytab file listed below
+mech_list: gssapi
 
-# Before you can use GSSAPI, you need a service principle on the
-# KDC server for libvirt, and that to be exported to the keytab
-# file listed below
-#mech_list: gssapi
+# If using TLS with VNC, or a UNIX socket only, it is possible to
+# enable plugins which don't provide session encryption. The
+# 'scram-sha-1' plugin allows plain username/password authentication
+# to be performed
 #
-# You can also list many mechanisms at once, then the user can choose
-# by adding  '?auth=sasl.gssapi' to their libvirt URI, eg
-#   qemu+tcp://hostname/system?auth=sasl.gssapi
-#mech_list: digest-md5 gssapi
+#mech_list: scram-sha-1
+
+# You can also list many mechanisms at once, and the VNC server will
+# negotiate which to use by considering the list enabled on the VNC
+# client.
+#mech_list: scram-sha-1 gssapi
 
 # Some older builds of MIT kerberos on Linux ignore this option &
 # instead need KRB5_KTNAME env var.
 # For modern Linux, and other OS, this should be sufficient
 #
-# There is no default value here, uncomment if you need this
-#keytab: /etc/qemu/krb5.tab
+# This file needs to be populated with the service principal that
+# was created on the Kerberos v5 server. If switching to a non-gssapi
+# mechanism this can be commented out.
+keytab: /etc/qemu/krb5.tab
 
-# If using digest-md5 for username/passwds, then this is the file
+# If using scram-sha-1 for username/passwds, then this is the file
 # containing the passwds. Use 'saslpasswd2 -a qemu [username]'
 # to add entries, and 'sasldblistusers2 -f [sasldb_path]' to browse it
-sasldb_path: /etc/qemu/passwd.db
-
-
-auxprop_plugin: sasldb
-
+#sasldb_path: /etc/qemu/passwd.db

tests/test-crypto-block.c

@@ -187,10 +187,10 @@ static struct QCryptoBlockTestData {
 
 
 static ssize_t test_block_read_func(QCryptoBlock *block,
-                                    void *opaque,
                                     size_t offset,
                                     uint8_t *buf,
                                     size_t buflen,
+                                    void *opaque,
                                     Error **errp)
 {
     Buffer *header = opaque;
@@ -204,8 +204,8 @@ static ssize_t test_block_read_func(QCryptoBlock *block,
 
 
 static ssize_t test_block_init_func(QCryptoBlock *block,
-                                    void *opaque,
                                     size_t headerlen,
+                                    void *opaque,
                                     Error **errp)
 {
     Buffer *header = opaque;
@@ -219,10 +219,10 @@ static ssize_t test_block_init_func(QCryptoBlock *block,
 
 
 static ssize_t test_block_write_func(QCryptoBlock *block,
-                                     void *opaque,
                                      size_t offset,
                                      const uint8_t *buf,
                                      size_t buflen,
+                                     void *opaque,
                                      Error **errp)
 {
     Buffer *header = opaque;