[Qemu-devel] [PULL v1 0/2] Merge qcrypto-next 2016/07/20

[Qemu-devel] [PULL v1 0/2] Merge qcrypto-next 2016/07/20

[Daniel P. Berrange]

The following changes since commit a3b343772114c8c98986508f3352a631164f913c:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2016-07-19 15:08:05 +0100)

are available in the git repository at:

  git://github.com/berrange/qemu tags/pull-qcrypto-2016-07-20-1

for you to fetch changes up to 718083de8051736bba02f10dc4a88bd14ff6b79b:

  crypto: don't open-code qcrypto_hash_supports (2016-07-19 16:46:23 +0100)

----------------------------------------------------------------
Merge qcrypto-next 2016/07/20 v1

----------------------------------------------------------------
Daniel P. Berrange (2):
  crypto: use glib as fallback for hash algorithm
  crypto: don't open-code qcrypto_hash_supports

 crypto/Makefile.objs |  2 +-
 crypto/hash-gcrypt.c |  3 +-
 crypto/hash-glib.c   | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 crypto/hash-nettle.c |  3 +-
 crypto/hash-stub.c   | 41 -----------------------
 5 files changed, 96 insertions(+), 46 deletions(-)
 create mode 100644 crypto/hash-glib.c
 delete mode 100644 crypto/hash-stub.c

-- 
2.7.4

[Qemu-devel] [PULL v1 1/2] crypto: use glib as fallback for hash algorithm

[Daniel P. Berrange]

GLib >= 2.16 provides GChecksum API which is good enough
for md5, sha1, sha256 and sha512. Use this as a final
fallback if neither nettle or gcrypt are available. This
lets us remove the stub hash impl, and so callers can
be sure those 4 algs are always available at compile
time. They may still be disabled at runtime, so a check
for qcrypto_hash_supports() is still best practice to
report good error messages.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/Makefile.objs |  2 +-
 crypto/hash-glib.c   | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 crypto/hash-stub.c   | 41 -----------------------
 3 files changed, 95 insertions(+), 42 deletions(-)
 create mode 100644 crypto/hash-glib.c
 delete mode 100644 crypto/hash-stub.c

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 1f86f4f..e409b89 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -2,6 +2,7 @@ crypto-obj-y = init.o
 crypto-obj-y += hash.o
 crypto-obj-$(CONFIG_NETTLE) += hash-nettle.o
 crypto-obj-$(if $(CONFIG_NETTLE),n,$(CONFIG_GCRYPT)) += hash-gcrypt.o
+crypto-obj-$(if $(CONFIG_NETTLE),n,$(if $(CONFIG_GCRYPT),n,y)) += hash-glib.o
 crypto-obj-y += aes.o
 crypto-obj-y += desrfb.o
 crypto-obj-y += cipher.o
@@ -30,4 +31,3 @@ crypto-aes-obj-y = aes.o
 
 stub-obj-y += random-stub.o
 stub-obj-y += pbkdf-stub.o
-stub-obj-y += hash-stub.o
diff --git a/crypto/hash-glib.c b/crypto/hash-glib.c
new file mode 100644
index 0000000..81ef7ca
--- /dev/null
+++ b/crypto/hash-glib.c
@@ -0,0 +1,94 @@
+/*
+ * QEMU Crypto hash algorithms
+ *
+ * Copyright (c) 2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "crypto/hash.h"
+
+
+static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALG__MAX] = {
+    [QCRYPTO_HASH_ALG_MD5] = G_CHECKSUM_MD5,
+    [QCRYPTO_HASH_ALG_SHA1] = G_CHECKSUM_SHA1,
+    [QCRYPTO_HASH_ALG_SHA224] = -1,
+    [QCRYPTO_HASH_ALG_SHA256] = G_CHECKSUM_SHA256,
+    [QCRYPTO_HASH_ALG_SHA384] = -1,
+    [QCRYPTO_HASH_ALG_SHA512] = G_CHECKSUM_SHA512,
+    [QCRYPTO_HASH_ALG_RIPEMD160] = -1,
+};
+
+gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg)
+{
+    if (alg < G_N_ELEMENTS(qcrypto_hash_alg_map) &&
+        qcrypto_hash_alg_map[alg] != -1) {
+        return true;
+    }
+    return false;
+}
+
+
+int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
+                        const struct iovec *iov,
+                        size_t niov,
+                        uint8_t **result,
+                        size_t *resultlen,
+                        Error **errp)
+{
+    int i, ret;
+    GChecksum *cs;
+
+    if (alg >= G_N_ELEMENTS(qcrypto_hash_alg_map) ||
+        qcrypto_hash_alg_map[alg] == -1) {
+        error_setg(errp,
+                   "Unknown hash algorithm %d",
+                   alg);
+        return -1;
+    }
+
+    cs = g_checksum_new(qcrypto_hash_alg_map[alg]);
+
+    for (i = 0; i < niov; i++) {
+        g_checksum_update(cs, iov[i].iov_base, iov[i].iov_len);
+    }
+
+    ret = g_checksum_type_get_length(qcrypto_hash_alg_map[alg]);
+    if (ret < 0) {
+        error_setg(errp, "%s",
+                   "Unable to get hash length");
+        goto error;
+    }
+    if (*resultlen == 0) {
+        *resultlen = ret;
+        *result = g_new0(uint8_t, *resultlen);
+    } else if (*resultlen != ret) {
+        error_setg(errp,
+                   "Result buffer size %zu is smaller than hash %d",
+                   *resultlen, ret);
+        goto error;
+    }
+
+    g_checksum_get_digest(cs, *result, resultlen);
+
+    g_checksum_free(cs);
+    return 0;
+
+ error:
+    g_checksum_free(cs);
+    return -1;
+}
diff --git a/crypto/hash-stub.c b/crypto/hash-stub.c
deleted file mode 100644
index 8a9b8d4..0000000
--- a/crypto/hash-stub.c
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * QEMU Crypto hash algorithms
- *
- * Copyright (c) 2016 Red Hat, Inc.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see <http://www.gnu.org/licenses/>.
- *
- */
-
-#include "qemu/osdep.h"
-#include "qapi/error.h"
-#include "crypto/hash.h"
-
-gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg G_GNUC_UNUSED)
-{
-    return false;
-}
-
-int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
-                        const struct iovec *iov G_GNUC_UNUSED,
-                        size_t niov G_GNUC_UNUSED,
-                        uint8_t **result G_GNUC_UNUSED,
-                        size_t *resultlen G_GNUC_UNUSED,
-                        Error **errp)
-{
-    error_setg(errp,
-               "Hash algorithm %d not supported without GNUTLS",
-               alg);
-    return -1;
-}
-- 
2.7.4

[Qemu-devel] [PULL v1 2/2] crypto: don't open-code qcrypto_hash_supports

[Daniel P. Berrange]

Call the existing qcrypto_hash_supports method from
qcrypto_hash_bytesv instead of open-coding it again.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 crypto/hash-gcrypt.c | 3 +--
 crypto/hash-glib.c   | 3 +--
 crypto/hash-nettle.c | 3 +--
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/crypto/hash-gcrypt.c b/crypto/hash-gcrypt.c
index ed6f842..7690690 100644
--- a/crypto/hash-gcrypt.c
+++ b/crypto/hash-gcrypt.c
@@ -55,8 +55,7 @@ int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
     gcry_md_hd_t md;
     unsigned char *digest;
 
-    if (alg >= G_N_ELEMENTS(qcrypto_hash_alg_map) ||
-        qcrypto_hash_alg_map[alg] == GCRY_MD_NONE) {
+    if (!qcrypto_hash_supports(alg)) {
         error_setg(errp,
                    "Unknown hash algorithm %d",
                    alg);
diff --git a/crypto/hash-glib.c b/crypto/hash-glib.c
index 81ef7ca..ce54a4b 100644
--- a/crypto/hash-glib.c
+++ b/crypto/hash-glib.c
@@ -53,8 +53,7 @@ int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
     int i, ret;
     GChecksum *cs;
 
-    if (alg >= G_N_ELEMENTS(qcrypto_hash_alg_map) ||
-        qcrypto_hash_alg_map[alg] == -1) {
+    if (!qcrypto_hash_supports(alg)) {
         error_setg(errp,
                    "Unknown hash algorithm %d",
                    alg);
diff --git a/crypto/hash-nettle.c b/crypto/hash-nettle.c
index 4c6f50b..6a206dc 100644
--- a/crypto/hash-nettle.c
+++ b/crypto/hash-nettle.c
@@ -113,8 +113,7 @@ int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
     int i;
     union qcrypto_hash_ctx ctx;
 
-    if (alg >= G_N_ELEMENTS(qcrypto_hash_alg_map) ||
-        qcrypto_hash_alg_map[alg].init == NULL) {
+    if (!qcrypto_hash_supports(alg)) {
         error_setg(errp,
                    "Unknown hash algorithm %d",
                    alg);
-- 
2.7.4

Re: [Qemu-devel] [PULL v1 0/2] Merge qcrypto-next 2016/07/20

[Peter Maydell]

On 20 July 2016 at 12:32, Daniel P. Berrange <berrange@redhat.com> wrote:
> The following changes since commit a3b343772114c8c98986508f3352a631164f913c:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2016-07-19 15:08:05 +0100)
>
> are available in the git repository at:
>
>   git://github.com/berrange/qemu tags/pull-qcrypto-2016-07-20-1
>
> for you to fetch changes up to 718083de8051736bba02f10dc4a88bd14ff6b79b:
>
>   crypto: don't open-code qcrypto_hash_supports (2016-07-19 16:46:23 +0100)
>
> ----------------------------------------------------------------
> Merge qcrypto-next 2016/07/20 v1
>
> ----------------------------------------------------------------
> Daniel P. Berrange (2):
>   crypto: use glib as fallback for hash algorithm
>   crypto: don't open-code qcrypto_hash_supports
>

Fails to build with minimum-glib-version:

/Users/pm215/src/qemu-for-merges/crypto/hash-glib.c:32:33: error: use
of undeclared identifier 'G_CHECKSUM_SHA512'; did you mean
'G_CHECKSUM_SHA1'?
    [QCRYPTO_HASH_ALG_SHA512] = G_CHECKSUM_SHA512,
                                ^~~~~~~~~~~~~~~~~
                                G_CHECKSUM_SHA1
/sw/include/glib-2.0/glib/gchecksum.h:48:3: note: 'G_CHECKSUM_SHA1'
declared here
  G_CHECKSUM_SHA1,

SHA512 doesn't exist in our minimum-supported glib version,
I think (which has just MD5, SHA1 and SHA256).

I also get test failures on the ppc64 build:

TEST: tests/test-crypto-afsplit... (pid=36787)
  /crypto/afsplit/sha256/5:
Unexpected error in qcrypto_random_bytes() at
/home/pm215/qemu/crypto/random-stub.c:29:
No random byte source provided in this build
FAIL
GTester: last random seed: R02S49a560b2cb6ee4cfc024d35cc33004f6
(pid=36789)
  /crypto/afsplit/sha256/5000:
Unexpected error in qcrypto_random_bytes() at
/home/pm215/qemu/crypto/random-stub.c:29:
No random byte source provided in this build
FAIL
GTester: last random seed: R02S7e066aa40979ffe0de63a18e2e6a4bfa
(pid=36790)
  /crypto/afsplit/sha256/big:
Unexpected error in qcrypto_random_bytes() at
/home/pm215/qemu/crypto/random-stub.c:29:
No random byte source provided in this build
FAIL
GTester: last random seed: R02S91b927f7808472d3ae4672d177078877
(pid=36791)
  /crypto/afsplit/sha1/1000:
Unexpected error in qcrypto_random_bytes() at
/home/pm215/qemu/crypto/random-stub.c:29:
No random byte source provided in this build
FAIL
GTester: last random seed: R02S0a50082cedeb926cabdf8df1028d4296
(pid=36792)
FAIL: tests/test-crypto-afsplit


thanks
-- PMM

Re: [Qemu-devel] [PULL v1 0/2] Merge qcrypto-next 2016/07/20

[Daniel P. Berrange]

On Wed, Jul 20, 2016 at 08:31:09PM +0100, Peter Maydell wrote:
> On 20 July 2016 at 12:32, Daniel P. Berrange <berrange@redhat.com> wrote:
> > The following changes since commit a3b343772114c8c98986508f3352a631164f913c:
> >
> >   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2016-07-19 15:08:05 +0100)
> >
> > are available in the git repository at:
> >
> >   git://github.com/berrange/qemu tags/pull-qcrypto-2016-07-20-1
> >
> > for you to fetch changes up to 718083de8051736bba02f10dc4a88bd14ff6b79b:
> >
> >   crypto: don't open-code qcrypto_hash_supports (2016-07-19 16:46:23 +0100)
> >
> > ----------------------------------------------------------------
> > Merge qcrypto-next 2016/07/20 v1
> >
> > ----------------------------------------------------------------
> > Daniel P. Berrange (2):
> >   crypto: use glib as fallback for hash algorithm
> >   crypto: don't open-code qcrypto_hash_supports
> >
> 
> Fails to build with minimum-glib-version:
> 
> /Users/pm215/src/qemu-for-merges/crypto/hash-glib.c:32:33: error: use
> of undeclared identifier 'G_CHECKSUM_SHA512'; did you mean
> 'G_CHECKSUM_SHA1'?
>     [QCRYPTO_HASH_ALG_SHA512] = G_CHECKSUM_SHA512,
>                                 ^~~~~~~~~~~~~~~~~
>                                 G_CHECKSUM_SHA1
> /sw/include/glib-2.0/glib/gchecksum.h:48:3: note: 'G_CHECKSUM_SHA1'
> declared here
>   G_CHECKSUM_SHA1,
> 
> SHA512 doesn't exist in our minimum-supported glib version,
> I think (which has just MD5, SHA1 and SHA256).

Yay, glib docs simply say since 2.16, never mentioning that
SHA512 was not added until 2.36. Filed a bug against glib
to fix this

  https://bugzilla.gnome.org/show_bug.cgi?id=769027

It is easy enough to workaround in QEMU, so will do that.

> I also get test failures on the ppc64 build:
> 
> TEST: tests/test-crypto-afsplit... (pid=36787)
>   /crypto/afsplit/sha256/5:
> Unexpected error in qcrypto_random_bytes() at
> /home/pm215/qemu/crypto/random-stub.c:29:
> No random byte source provided in this build
> FAIL
> GTester: last random seed: R02S49a560b2cb6ee4cfc024d35cc33004f6
> (pid=36789)
>   /crypto/afsplit/sha256/5000:
> Unexpected error in qcrypto_random_bytes() at
> /home/pm215/qemu/crypto/random-stub.c:29:
> No random byte source provided in this build
> FAIL
> GTester: last random seed: R02S7e066aa40979ffe0de63a18e2e6a4bfa
> (pid=36790)
>   /crypto/afsplit/sha256/big:
> Unexpected error in qcrypto_random_bytes() at
> /home/pm215/qemu/crypto/random-stub.c:29:
> No random byte source provided in this build
> FAIL
> GTester: last random seed: R02S91b927f7808472d3ae4672d177078877
> (pid=36791)
>   /crypto/afsplit/sha1/1000:
> Unexpected error in qcrypto_random_bytes() at
> /home/pm215/qemu/crypto/random-stub.c:29:
> No random byte source provided in this build
> FAIL
> GTester: last random seed: R02S0a50082cedeb926cabdf8df1028d4296
> (pid=36792)
> FAIL: tests/test-crypto-afsplit

Ah fun, so afsplit used to automatically skip it tests if
no checksum impl was provided. It never bothered to check
if random byte impl was provided, since the check condition
for that matched that for checksums. Now we're always providing
a checksum impl, the lack of checking for random byte impl rears
its head.  Rather than fixing the test, I'm jst going to add a
general fallback random byte source using /dev/[u]random.

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