diff mbox

[07/10] block: convert quorum blockdrv to use crypto APIs

Message ID 1432205817-16414-8-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé May 21, 2015, 10:56 a.m. UTC
Get rid of direct use of gnutls APIs in quorum blockdrv in
favour of using the crypto APIs. This avoids the need to
do conditional compilation of the quorum driver. It can
simply report an error at file open file instead if the
required hash algorithm isn't supported by QEMU.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 block/Makefile.objs |  2 +-
 block/quorum.c      | 38 +++++++++++++++++++-------------------
 configure           | 39 ---------------------------------------
 3 files changed, 20 insertions(+), 59 deletions(-)

Comments

Gonglei (Arei) May 29, 2015, 6:49 a.m. UTC | #1
On 2015/5/21 18:56, Daniel P. Berrange wrote:
> Get rid of direct use of gnutls APIs in quorum blockdrv in
> favour of using the crypto APIs. This avoids the need to
> do conditional compilation of the quorum driver. It can
> simply report an error at file open file instead if the
> required hash algorithm isn't supported by QEMU.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  block/Makefile.objs |  2 +-
>  block/quorum.c      | 38 +++++++++++++++++++-------------------
>  configure           | 39 ---------------------------------------
>  3 files changed, 20 insertions(+), 59 deletions(-)
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 0d8c2a4..8f908d9 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -3,7 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c
>  block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
>  block-obj-y += qed-check.o
>  block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o
> -block-obj-$(CONFIG_QUORUM) += quorum.o
> +block-obj-y += quorum.o
>  block-obj-y += parallels.o blkdebug.o blkverify.o
>  block-obj-y += block-backend.o snapshot.o qapi.o
>  block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
> diff --git a/block/quorum.c b/block/quorum.c
> index f91ef75..4193f30 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -13,8 +13,6 @@
>   * See the COPYING file in the top-level directory.
>   */
>  
> -#include <gnutls/gnutls.h>
> -#include <gnutls/crypto.h>
>  #include "block/block_int.h"
>  #include "qapi/qmp/qbool.h"
>  #include "qapi/qmp/qdict.h"
> @@ -23,6 +21,9 @@
>  #include "qapi/qmp/qlist.h"
>  #include "qapi/qmp/qstring.h"
>  #include "qapi-event.h"
> +#include "crypto/hash.h"
> +
> +#include <glib/gi18n.h>
>  
>  #define HASH_LENGTH 32
>  
> @@ -33,7 +34,7 @@
>  
>  /* This union holds a vote hash value */
>  typedef union QuorumVoteValue {
> -    char h[HASH_LENGTH];       /* SHA-256 hash */
> +    uint8_t h[HASH_LENGTH];    /* SHA-256 hash */
>      int64_t l;                 /* simpler 64 bits hash */
>  } QuorumVoteValue;
>  
> @@ -427,25 +428,18 @@ static void quorum_free_vote_list(QuorumVotes *votes)
>  
>  static int quorum_compute_hash(QuorumAIOCB *acb, int i, QuorumVoteValue *hash)
>  {
> -    int j, ret;
> -    gnutls_hash_hd_t dig;
>      QEMUIOVector *qiov = &acb->qcrs[i].qiov;
> -
> -    ret = gnutls_hash_init(&dig, GNUTLS_DIG_SHA256);
> -
> -    if (ret < 0) {
> -        return ret;
> +    size_t len = sizeof(hash->h);
> +    uint8_t *data = hash->h;
> +
> +    if (qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256,
> +                            qiov->iov, qiov->niov,
> +                            &data, &len,
> +                            NULL) < 0) {

The last argument should not be NULL, maybe miss error message.

Regards,
-Gonglei

> +        return -EINVAL;
>      }
>  
> -    for (j = 0; j < qiov->niov; j++) {
> -        ret = gnutls_hash(dig, qiov->iov[j].iov_base, qiov->iov[j].iov_len);
> -        if (ret < 0) {
> -            break;
> -        }
> -    }
> -
> -    gnutls_hash_deinit(dig, (void *) hash);
> -    return ret;
> +    return 0;
>  }
>  
>  static QuorumVoteVersion *quorum_get_vote_winner(QuorumVotes *votes)
> @@ -872,6 +866,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, "%s",
> +                   _("SHA256 hash support is required for quorum device"));
> +        return -EINVAL;
> +    }
> +
>      qdict_flatten(options);
>      qdict_extract_subqdict(options, &sub, "children.");
>      qdict_array_split(sub, &list);
> diff --git a/configure b/configure
> index a48aa90..cc60f0b 100755
> --- a/configure
> +++ b/configure
> @@ -336,7 +336,6 @@ vte=""
>  tpm="yes"
>  libssh2=""
>  vhdx=""
> -quorum=""
>  numa=""
>  tcmalloc="no"
>  
> @@ -1133,10 +1132,6 @@ for opt do
>    ;;
>    --disable-vhdx) vhdx="no"
>    ;;
> -  --disable-quorum) quorum="no"
> -  ;;
> -  --enable-quorum) quorum="yes"
> -  ;;
>    --disable-numa) numa="no"
>    ;;
>    --enable-numa) numa="yes"
> @@ -1416,8 +1411,6 @@ Advanced options (experts only):
>    --enable-libssh2         enable ssh block device support
>    --disable-vhdx           disable support for the Microsoft VHDX image format
>    --enable-vhdx            enable support for the Microsoft VHDX image format
> -  --disable-quorum         disable quorum block filter support
> -  --enable-quorum          enable quorum block filter support
>    --disable-numa           disable libnuma support
>    --enable-numa            enable libnuma support
>    --disable-tcmalloc       disable tcmalloc support
> @@ -2414,33 +2407,6 @@ EOF
>  fi
>  
>  ##########################################
> -# Quorum probe (check for gnutls)
> -if test "$quorum" != "no" ; then
> -cat > $TMPC <<EOF
> -#include <gnutls/gnutls.h>
> -#include <gnutls/crypto.h>
> -int main(void) {char data[4096], digest[32];
> -gnutls_hash_fast(GNUTLS_DIG_SHA256, data, 4096, digest);
> -return 0;
> -}
> -EOF
> -quorum_tls_cflags=`$pkg_config --cflags gnutls 2> /dev/null`
> -quorum_tls_libs=`$pkg_config --libs gnutls 2> /dev/null`
> -if compile_prog "$quorum_tls_cflags" "$quorum_tls_libs" ; then
> -  qcow_tls=yes
> -  libs_softmmu="$quorum_tls_libs $libs_softmmu"
> -  libs_tools="$quorum_tls_libs $libs_softmmu"
> -  QEMU_CFLAGS="$QEMU_CFLAGS $quorum_tls_cflags"
> -  quorum="yes"
> -else
> -  if test "$quorum" = "yes"; then
> -    feature_not_found "gnutls" "gnutls > 2.10.0 required to compile Quorum"
> -  fi
> -  quorum="no"
> -fi
> -fi
> -
> -##########################################
>  # VNC SASL detection
>  if test "$vnc" = "yes" -a "$vnc_sasl" != "no" ; then
>    cat > $TMPC <<EOF
> @@ -4562,7 +4528,6 @@ echo "libssh2 support   $libssh2"
>  echo "TPM passthrough   $tpm_passthrough"
>  echo "QOM debugging     $qom_cast_debug"
>  echo "vhdx              $vhdx"
> -echo "Quorum            $quorum"
>  echo "lzo support       $lzo"
>  echo "snappy support    $snappy"
>  echo "bzip2 support     $bzip2"
> @@ -5038,10 +5003,6 @@ if test "$libssh2" = "yes" ; then
>    echo "LIBSSH2_LIBS=$libssh2_libs" >> $config_host_mak
>  fi
>  
> -if test "$quorum" = "yes" ; then
> -  echo "CONFIG_QUORUM=y" >> $config_host_mak
> -fi
> -
>  if test "$vhdx" = "yes" ; then
>    echo "CONFIG_VHDX=y" >> $config_host_mak
>  fi
>
Daniel P. Berrangé June 1, 2015, 4:56 p.m. UTC | #2
On Fri, May 29, 2015 at 02:49:46PM +0800, Gonglei wrote:
> On 2015/5/21 18:56, Daniel P. Berrange wrote:
> > Get rid of direct use of gnutls APIs in quorum blockdrv in
> > favour of using the crypto APIs. This avoids the need to
> > do conditional compilation of the quorum driver. It can
> > simply report an error at file open file instead if the
> > required hash algorithm isn't supported by QEMU.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  block/Makefile.objs |  2 +-
> >  block/quorum.c      | 38 +++++++++++++++++++-------------------
> >  configure           | 39 ---------------------------------------
> >  3 files changed, 20 insertions(+), 59 deletions(-)
> > 
> > diff --git a/block/Makefile.objs b/block/Makefile.objs
> > index 0d8c2a4..8f908d9 100644
> > --- a/block/Makefile.objs
> > +++ b/block/Makefile.objs
> > @@ -3,7 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c
> >  block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
> >  block-obj-y += qed-check.o
> >  block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o
> > -block-obj-$(CONFIG_QUORUM) += quorum.o
> > +block-obj-y += quorum.o
> >  block-obj-y += parallels.o blkdebug.o blkverify.o
> >  block-obj-y += block-backend.o snapshot.o qapi.o
> >  block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
> > diff --git a/block/quorum.c b/block/quorum.c
> > index f91ef75..4193f30 100644
> > --- a/block/quorum.c
> > +++ b/block/quorum.c
> > @@ -13,8 +13,6 @@
> >   * See the COPYING file in the top-level directory.
> >   */
> >  
> > -#include <gnutls/gnutls.h>
> > -#include <gnutls/crypto.h>
> >  #include "block/block_int.h"
> >  #include "qapi/qmp/qbool.h"
> >  #include "qapi/qmp/qdict.h"
> > @@ -23,6 +21,9 @@
> >  #include "qapi/qmp/qlist.h"
> >  #include "qapi/qmp/qstring.h"
> >  #include "qapi-event.h"
> > +#include "crypto/hash.h"
> > +
> > +#include <glib/gi18n.h>
> >  
> >  #define HASH_LENGTH 32
> >  
> > @@ -33,7 +34,7 @@
> >  
> >  /* This union holds a vote hash value */
> >  typedef union QuorumVoteValue {
> > -    char h[HASH_LENGTH];       /* SHA-256 hash */
> > +    uint8_t h[HASH_LENGTH];    /* SHA-256 hash */
> >      int64_t l;                 /* simpler 64 bits hash */
> >  } QuorumVoteValue;
> >  
> > @@ -427,25 +428,18 @@ static void quorum_free_vote_list(QuorumVotes *votes)
> >  
> >  static int quorum_compute_hash(QuorumAIOCB *acb, int i, QuorumVoteValue *hash)
> >  {
> > -    int j, ret;
> > -    gnutls_hash_hd_t dig;
> >      QEMUIOVector *qiov = &acb->qcrs[i].qiov;
> > -
> > -    ret = gnutls_hash_init(&dig, GNUTLS_DIG_SHA256);
> > -
> > -    if (ret < 0) {
> > -        return ret;
> > +    size_t len = sizeof(hash->h);
> > +    uint8_t *data = hash->h;
> > +
> > +    if (qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256,
> > +                            qiov->iov, qiov->niov,
> > +                            &data, &len,
> > +                            NULL) < 0) {
> 
> The last argument should not be NULL, maybe miss error message.

Yes, it would be desirable to pass in an Error ** object, but this
quorum_compute_hash method doesn't currently have any error reporting
framework except for returning errno values. The quorum block driver
would need some refactoring to be able to propagate Error* objects
back up before we can make use of that here, so I just stuck with
the generic EINVAL.

> > +        return -EINVAL;
> >      }
> >  
> > -    for (j = 0; j < qiov->niov; j++) {
> > -        ret = gnutls_hash(dig, qiov->iov[j].iov_base, qiov->iov[j].iov_len);
> > -        if (ret < 0) {
> > -            break;
> > -        }
> > -    }
> > -
> > -    gnutls_hash_deinit(dig, (void *) hash);
> > -    return ret;
> > +    return 0;
> >  }

Regards,
Daniel
diff mbox

Patch

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 0d8c2a4..8f908d9 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -3,7 +3,7 @@  block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c
 block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-obj-y += qed-check.o
 block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o
-block-obj-$(CONFIG_QUORUM) += quorum.o
+block-obj-y += quorum.o
 block-obj-y += parallels.o blkdebug.o blkverify.o
 block-obj-y += block-backend.o snapshot.o qapi.o
 block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
diff --git a/block/quorum.c b/block/quorum.c
index f91ef75..4193f30 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -13,8 +13,6 @@ 
  * See the COPYING file in the top-level directory.
  */
 
-#include <gnutls/gnutls.h>
-#include <gnutls/crypto.h>
 #include "block/block_int.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qdict.h"
@@ -23,6 +21,9 @@ 
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi-event.h"
+#include "crypto/hash.h"
+
+#include <glib/gi18n.h>
 
 #define HASH_LENGTH 32
 
@@ -33,7 +34,7 @@ 
 
 /* This union holds a vote hash value */
 typedef union QuorumVoteValue {
-    char h[HASH_LENGTH];       /* SHA-256 hash */
+    uint8_t h[HASH_LENGTH];    /* SHA-256 hash */
     int64_t l;                 /* simpler 64 bits hash */
 } QuorumVoteValue;
 
@@ -427,25 +428,18 @@  static void quorum_free_vote_list(QuorumVotes *votes)
 
 static int quorum_compute_hash(QuorumAIOCB *acb, int i, QuorumVoteValue *hash)
 {
-    int j, ret;
-    gnutls_hash_hd_t dig;
     QEMUIOVector *qiov = &acb->qcrs[i].qiov;
-
-    ret = gnutls_hash_init(&dig, GNUTLS_DIG_SHA256);
-
-    if (ret < 0) {
-        return ret;
+    size_t len = sizeof(hash->h);
+    uint8_t *data = hash->h;
+
+    if (qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256,
+                            qiov->iov, qiov->niov,
+                            &data, &len,
+                            NULL) < 0) {
+        return -EINVAL;
     }
 
-    for (j = 0; j < qiov->niov; j++) {
-        ret = gnutls_hash(dig, qiov->iov[j].iov_base, qiov->iov[j].iov_len);
-        if (ret < 0) {
-            break;
-        }
-    }
-
-    gnutls_hash_deinit(dig, (void *) hash);
-    return ret;
+    return 0;
 }
 
 static QuorumVoteVersion *quorum_get_vote_winner(QuorumVotes *votes)
@@ -872,6 +866,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, "%s",
+                   _("SHA256 hash support is required for quorum device"));
+        return -EINVAL;
+    }
+
     qdict_flatten(options);
     qdict_extract_subqdict(options, &sub, "children.");
     qdict_array_split(sub, &list);
diff --git a/configure b/configure
index a48aa90..cc60f0b 100755
--- a/configure
+++ b/configure
@@ -336,7 +336,6 @@  vte=""
 tpm="yes"
 libssh2=""
 vhdx=""
-quorum=""
 numa=""
 tcmalloc="no"
 
@@ -1133,10 +1132,6 @@  for opt do
   ;;
   --disable-vhdx) vhdx="no"
   ;;
-  --disable-quorum) quorum="no"
-  ;;
-  --enable-quorum) quorum="yes"
-  ;;
   --disable-numa) numa="no"
   ;;
   --enable-numa) numa="yes"
@@ -1416,8 +1411,6 @@  Advanced options (experts only):
   --enable-libssh2         enable ssh block device support
   --disable-vhdx           disable support for the Microsoft VHDX image format
   --enable-vhdx            enable support for the Microsoft VHDX image format
-  --disable-quorum         disable quorum block filter support
-  --enable-quorum          enable quorum block filter support
   --disable-numa           disable libnuma support
   --enable-numa            enable libnuma support
   --disable-tcmalloc       disable tcmalloc support
@@ -2414,33 +2407,6 @@  EOF
 fi
 
 ##########################################
-# Quorum probe (check for gnutls)
-if test "$quorum" != "no" ; then
-cat > $TMPC <<EOF
-#include <gnutls/gnutls.h>
-#include <gnutls/crypto.h>
-int main(void) {char data[4096], digest[32];
-gnutls_hash_fast(GNUTLS_DIG_SHA256, data, 4096, digest);
-return 0;
-}
-EOF
-quorum_tls_cflags=`$pkg_config --cflags gnutls 2> /dev/null`
-quorum_tls_libs=`$pkg_config --libs gnutls 2> /dev/null`
-if compile_prog "$quorum_tls_cflags" "$quorum_tls_libs" ; then
-  qcow_tls=yes
-  libs_softmmu="$quorum_tls_libs $libs_softmmu"
-  libs_tools="$quorum_tls_libs $libs_softmmu"
-  QEMU_CFLAGS="$QEMU_CFLAGS $quorum_tls_cflags"
-  quorum="yes"
-else
-  if test "$quorum" = "yes"; then
-    feature_not_found "gnutls" "gnutls > 2.10.0 required to compile Quorum"
-  fi
-  quorum="no"
-fi
-fi
-
-##########################################
 # VNC SASL detection
 if test "$vnc" = "yes" -a "$vnc_sasl" != "no" ; then
   cat > $TMPC <<EOF
@@ -4562,7 +4528,6 @@  echo "libssh2 support   $libssh2"
 echo "TPM passthrough   $tpm_passthrough"
 echo "QOM debugging     $qom_cast_debug"
 echo "vhdx              $vhdx"
-echo "Quorum            $quorum"
 echo "lzo support       $lzo"
 echo "snappy support    $snappy"
 echo "bzip2 support     $bzip2"
@@ -5038,10 +5003,6 @@  if test "$libssh2" = "yes" ; then
   echo "LIBSSH2_LIBS=$libssh2_libs" >> $config_host_mak
 fi
 
-if test "$quorum" = "yes" ; then
-  echo "CONFIG_QUORUM=y" >> $config_host_mak
-fi
-
 if test "$vhdx" = "yes" ; then
   echo "CONFIG_VHDX=y" >> $config_host_mak
 fi