All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v1 0/2] Use GChecksum as fallback hash impl
@ 2016-07-05 10:49 Daniel P. Berrange
  2016-07-05 10:49 ` [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm Daniel P. Berrange
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2016-07-05 10:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Alberto Garcia, Kevin Wolf, Fam Zheng, Max Reitz,
	Daniel P. Berrange

This uses the GChecksum APIs as final hash impl, instead of
a no-op stub. This lets us remove conditional registration
of the quorum driver.

Daniel P. Berrange (2):
  crypto: use glib as fallback for hash algorithm
  Revert "block: don't register quorum driver if SHA256 support is
    unavailable"

 block/quorum.c       | 10 +++---
 crypto/Makefile.objs |  2 +-
 crypto/hash-glib.c   | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 crypto/hash-stub.c   | 41 -----------------------
 4 files changed, 101 insertions(+), 46 deletions(-)
 create mode 100644 crypto/hash-glib.c
 delete mode 100644 crypto/hash-stub.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm
  2016-07-05 10:49 [Qemu-devel] [PATCH v1 0/2] Use GChecksum as fallback hash impl Daniel P. Berrange
@ 2016-07-05 10:49 ` Daniel P. Berrange
  2016-07-05 15:03   ` Eric Blake
                     ` (2 more replies)
  2016-07-05 10:50 ` [Qemu-devel] [PATCH v1 2/2] Revert "block: don't register quorum driver if SHA256 support is unavailable" Daniel P. Berrange
  2016-07-05 16:43 ` [Qemu-devel] [PATCH v1 3/2] crypto: don't open-code qcrypto_hash_supports Daniel P. Berrange
  2 siblings, 3 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2016-07-05 10:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Alberto Garcia, Kevin Wolf, Fam Zheng, Max Reitz,
	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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v1 2/2] Revert "block: don't register quorum driver if SHA256 support is unavailable"
  2016-07-05 10:49 [Qemu-devel] [PATCH v1 0/2] Use GChecksum as fallback hash impl Daniel P. Berrange
  2016-07-05 10:49 ` [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm Daniel P. Berrange
@ 2016-07-05 10:50 ` Daniel P. Berrange
  2016-07-05 15:04   ` Eric Blake
  2016-07-07  8:53   ` Alberto Garcia
  2016-07-05 16:43 ` [Qemu-devel] [PATCH v1 3/2] crypto: don't open-code qcrypto_hash_supports Daniel P. Berrange
  2 siblings, 2 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2016-07-05 10:50 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Alberto Garcia, Kevin Wolf, Fam Zheng, Max Reitz,
	Daniel P. Berrange

The qcrypto hash APIs now guarantee that sha256 is available at
compile time, so skipping registration is rarely needed. A check
at time of open is kept to ensure good error reporting in the
(unlikely) case sha256 is runtime disabled.

This reverts commit e94867ed5f241008d0f53142b2704a075f9ed505.
---
 block/quorum.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 331b726..ed02cce 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -878,6 +878,12 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
     int i;
     int ret = 0;
 
+    if (!qcrypto_hash_supports(QCRYPTO_HASH_ALG_SHA256)) {
+        error_setg(errp,
+                   "SHA256 hash support is required for quorum device");
+        return -EINVAL;
+    }
+
     qdict_flatten(options);
 
     /* count how many different children are present */
@@ -1113,10 +1119,6 @@ static BlockDriver bdrv_quorum = {
 
 static void bdrv_quorum_init(void)
 {
-    if (!qcrypto_hash_supports(QCRYPTO_HASH_ALG_SHA256)) {
-        /* SHA256 hash support is required for quorum device */
-        return;
-    }
     bdrv_register(&bdrv_quorum);
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm
  2016-07-05 10:49 ` [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm Daniel P. Berrange
@ 2016-07-05 15:03   ` Eric Blake
  2016-07-05 15:32     ` Daniel P. Berrange
  2016-07-06 11:58   ` Alberto Garcia
  2016-07-07  8:52   ` Alberto Garcia
  2 siblings, 1 reply; 14+ messages in thread
From: Eric Blake @ 2016-07-05 15:03 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, qemu-block, Max Reitz, Alberto Garcia

[-- Attachment #1: Type: text/plain, Size: 1791 bytes --]

On 07/05/2016 04:49 AM, Daniel P. Berrange wrote:
> 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
> 

> +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) {

Worth writing this as 'if (!gcrypto_hash_supports(alg)) {' ?

Otherwise,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v1 2/2] Revert "block: don't register quorum driver if SHA256 support is unavailable"
  2016-07-05 10:50 ` [Qemu-devel] [PATCH v1 2/2] Revert "block: don't register quorum driver if SHA256 support is unavailable" Daniel P. Berrange
@ 2016-07-05 15:04   ` Eric Blake
  2016-07-07  8:53   ` Alberto Garcia
  1 sibling, 0 replies; 14+ messages in thread
From: Eric Blake @ 2016-07-05 15:04 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, qemu-block, Max Reitz, Alberto Garcia

[-- Attachment #1: Type: text/plain, Size: 623 bytes --]

On 07/05/2016 04:50 AM, Daniel P. Berrange wrote:
> The qcrypto hash APIs now guarantee that sha256 is available at
> compile time, so skipping registration is rarely needed. A check
> at time of open is kept to ensure good error reporting in the
> (unlikely) case sha256 is runtime disabled.
> 
> This reverts commit e94867ed5f241008d0f53142b2704a075f9ed505.
> ---
>  block/quorum.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm
  2016-07-05 15:03   ` Eric Blake
@ 2016-07-05 15:32     ` Daniel P. Berrange
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2016-07-05 15:32 UTC (permalink / raw)
  To: Eric Blake
  Cc: qemu-devel, Kevin Wolf, Fam Zheng, qemu-block, Max Reitz, Alberto Garcia

On Tue, Jul 05, 2016 at 09:03:26AM -0600, Eric Blake wrote:
> On 07/05/2016 04:49 AM, Daniel P. Berrange wrote:
> > 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
> > 
> 
> > +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) {
> 
> Worth writing this as 'if (!gcrypto_hash_supports(alg)) {' ?

This pattern is used in the nettle + gcrypt impls too. I'd be happy to
switch to what you suggest in all impls separately.

> Otherwise,
> Reviewed-by: Eric Blake <eblake@redhat.com>

Oh, and BTW the pre-existing test-crypto-hash unit tests will already
provide coverage for this implementation & i've checked it passes
when --disable-nettle --disable-gcrypt are given to confnigure.



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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v1 3/2] crypto: don't open-code qcrypto_hash_supports
  2016-07-05 10:49 [Qemu-devel] [PATCH v1 0/2] Use GChecksum as fallback hash impl Daniel P. Berrange
  2016-07-05 10:49 ` [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm Daniel P. Berrange
  2016-07-05 10:50 ` [Qemu-devel] [PATCH v1 2/2] Revert "block: don't register quorum driver if SHA256 support is unavailable" Daniel P. Berrange
@ 2016-07-05 16:43 ` Daniel P. Berrange
  2016-07-05 22:26   ` Eric Blake
  2016-07-07  8:55   ` Alberto Garcia
  2 siblings, 2 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2016-07-05 16:43 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Alberto Garcia, Kevin Wolf, Fam Zheng, Max Reitz,
	Eric Blake, 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 8ea5aff..0dad13d 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v1 3/2] crypto: don't open-code qcrypto_hash_supports
  2016-07-05 16:43 ` [Qemu-devel] [PATCH v1 3/2] crypto: don't open-code qcrypto_hash_supports Daniel P. Berrange
@ 2016-07-05 22:26   ` Eric Blake
  2016-07-07  8:55   ` Alberto Garcia
  1 sibling, 0 replies; 14+ messages in thread
From: Eric Blake @ 2016-07-05 22:26 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel
  Cc: qemu-block, Alberto Garcia, Kevin Wolf, Fam Zheng, Max Reitz

[-- Attachment #1: Type: text/plain, Size: 546 bytes --]

On 07/05/2016 10:43 AM, Daniel P. Berrange wrote:
> 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(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm
  2016-07-05 10:49 ` [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm Daniel P. Berrange
  2016-07-05 15:03   ` Eric Blake
@ 2016-07-06 11:58   ` Alberto Garcia
  2016-07-06 14:53     ` Eric Blake
  2016-07-07  8:52   ` Alberto Garcia
  2 siblings, 1 reply; 14+ messages in thread
From: Alberto Garcia @ 2016-07-06 11:58 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel
  Cc: qemu-block, Kevin Wolf, Fam Zheng, Max Reitz

On Tue 05 Jul 2016 12:49:59 PM CEST, "Daniel P. Berrange" <berrange@redhat.com> wrote:

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

Sorry if I missed the explanation, but how do you disable them at
runtime ?

Berto

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm
  2016-07-06 11:58   ` Alberto Garcia
@ 2016-07-06 14:53     ` Eric Blake
  2016-07-07  9:18       ` Daniel P. Berrange
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Blake @ 2016-07-06 14:53 UTC (permalink / raw)
  To: Alberto Garcia, Daniel P. Berrange, qemu-devel
  Cc: Kevin Wolf, Fam Zheng, qemu-block, Max Reitz

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

On 07/06/2016 05:58 AM, Alberto Garcia wrote:
> On Tue 05 Jul 2016 12:49:59 PM CEST, "Daniel P. Berrange" <berrange@redhat.com> wrote:
> 
>> 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.
> 
> Sorry if I missed the explanation, but how do you disable them at
> runtime ?

FIPS is a common case where portions of a crypto lib are disabled at
runtime based on whether the system is running in FIPS mode or not.  I
don't think any of the hashes in the glib fallback are necessarily
covered by FIPS disabling, so much as the qcrypto interface being
interested in generically catering to this behavior across the various
implementations.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm
  2016-07-05 10:49 ` [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm Daniel P. Berrange
  2016-07-05 15:03   ` Eric Blake
  2016-07-06 11:58   ` Alberto Garcia
@ 2016-07-07  8:52   ` Alberto Garcia
  2 siblings, 0 replies; 14+ messages in thread
From: Alberto Garcia @ 2016-07-07  8:52 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel
  Cc: qemu-block, Kevin Wolf, Fam Zheng, Max Reitz

On Tue 05 Jul 2016 12:49:59 PM CEST, "Daniel P. Berrange" <berrange@redhat.com> wrote:
> +    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);
> +    }

Not too important, but you could do this after checking the hash length
and the buffer size. You don't need to create or feed the GChecksum
first for those, and that way you would also get rid of the 'goto
error'.

Either way,

Reviewed-by: Alberto Garcia <berto@igalia.com>

> +    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;
> +}

Berto

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v1 2/2] Revert "block: don't register quorum driver if SHA256 support is unavailable"
  2016-07-05 10:50 ` [Qemu-devel] [PATCH v1 2/2] Revert "block: don't register quorum driver if SHA256 support is unavailable" Daniel P. Berrange
  2016-07-05 15:04   ` Eric Blake
@ 2016-07-07  8:53   ` Alberto Garcia
  1 sibling, 0 replies; 14+ messages in thread
From: Alberto Garcia @ 2016-07-07  8:53 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel
  Cc: qemu-block, Kevin Wolf, Fam Zheng, Max Reitz

On Tue 05 Jul 2016 12:50:00 PM CEST, "Daniel P. Berrange" <berrange@redhat.com> wrote:
> The qcrypto hash APIs now guarantee that sha256 is available at
> compile time, so skipping registration is rarely needed. A check
> at time of open is kept to ensure good error reporting in the
> (unlikely) case sha256 is runtime disabled.
>
> This reverts commit e94867ed5f241008d0f53142b2704a075f9ed505.

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v1 3/2] crypto: don't open-code qcrypto_hash_supports
  2016-07-05 16:43 ` [Qemu-devel] [PATCH v1 3/2] crypto: don't open-code qcrypto_hash_supports Daniel P. Berrange
  2016-07-05 22:26   ` Eric Blake
@ 2016-07-07  8:55   ` Alberto Garcia
  1 sibling, 0 replies; 14+ messages in thread
From: Alberto Garcia @ 2016-07-07  8:55 UTC (permalink / raw)
  To: Daniel P. Berrange, qemu-devel
  Cc: qemu-block, Kevin Wolf, Fam Zheng, Max Reitz, Eric Blake

On Tue 05 Jul 2016 06:43:13 PM CEST, "Daniel P. Berrange" <berrange@redhat.com> wrote:
> 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>

"PATCH 3/2" ? :-)

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm
  2016-07-06 14:53     ` Eric Blake
@ 2016-07-07  9:18       ` Daniel P. Berrange
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrange @ 2016-07-07  9:18 UTC (permalink / raw)
  To: Eric Blake
  Cc: Alberto Garcia, qemu-devel, Kevin Wolf, Fam Zheng, qemu-block, Max Reitz

On Wed, Jul 06, 2016 at 08:53:56AM -0600, Eric Blake wrote:
> On 07/06/2016 05:58 AM, Alberto Garcia wrote:
> > On Tue 05 Jul 2016 12:49:59 PM CEST, "Daniel P. Berrange" <berrange@redhat.com> wrote:
> > 
> >> 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.
> > 
> > Sorry if I missed the explanation, but how do you disable them at
> > runtime ?
> 
> FIPS is a common case where portions of a crypto lib are disabled at
> runtime based on whether the system is running in FIPS mode or not.  I
> don't think any of the hashes in the glib fallback are necessarily
> covered by FIPS disabling, so much as the qcrypto interface being
> interested in generically catering to this behavior across the various
> implementations.

Yep, currently none of the hashes are disabled by FIPS, but the QEMU
crypto API is designed to allow for that in the future, without us
needing to change the rest of QEMU code using those APIs

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-07-07  9:18 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-05 10:49 [Qemu-devel] [PATCH v1 0/2] Use GChecksum as fallback hash impl Daniel P. Berrange
2016-07-05 10:49 ` [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm Daniel P. Berrange
2016-07-05 15:03   ` Eric Blake
2016-07-05 15:32     ` Daniel P. Berrange
2016-07-06 11:58   ` Alberto Garcia
2016-07-06 14:53     ` Eric Blake
2016-07-07  9:18       ` Daniel P. Berrange
2016-07-07  8:52   ` Alberto Garcia
2016-07-05 10:50 ` [Qemu-devel] [PATCH v1 2/2] Revert "block: don't register quorum driver if SHA256 support is unavailable" Daniel P. Berrange
2016-07-05 15:04   ` Eric Blake
2016-07-07  8:53   ` Alberto Garcia
2016-07-05 16:43 ` [Qemu-devel] [PATCH v1 3/2] crypto: don't open-code qcrypto_hash_supports Daniel P. Berrange
2016-07-05 22:26   ` Eric Blake
2016-07-07  8:55   ` Alberto Garcia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.