[PULL 08/17] crypto/hash: Implement and use new hash API

Daniel P. Berrangé posted 17 patches 1 month, 2 weeks ago
There is a newer version of this series
[PULL 08/17] crypto/hash: Implement and use new hash API
Posted by Daniel P. Berrangé 1 month, 2 weeks ago
From: Alejandro Zeise <alejandro.zeise@seagate.com>

Changes the public hash API implementation to support accumulative hashing.

Implementations for the public functions are added to call the new
driver functions that implement context creation, updating,
finalization, and destruction.

Additionally changes the "shortcut" functions to use these 4 new core
functions.

Signed-off-by: Alejandro Zeise <alejandro.zeise@seagate.com>
[ clg: - Reworked qcrypto_hash_bytesv() error handling
       - Used hash->driver int qcrypto_hash_new(), qcrypto_hash_free()
         qcrypto_hash_updatev()
       - Introduced qcrypto_hash_supports() check in
         qcrypto_hash_new()
       - Introduced g_autofree variables in qcrypto_hash_finalize_digest()
         and qcrypto_hash_finalize_base64()
       - Re-arrranged code in qcrypto_hash_digestv() and
         qcrypto_hash_digest()
       - Checkpatch fixes ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/hash.c | 161 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 131 insertions(+), 30 deletions(-)

diff --git a/crypto/hash.c b/crypto/hash.c
index 4a265582b8..0c8548c568 100644
--- a/crypto/hash.c
+++ b/crypto/hash.c
@@ -1,6 +1,7 @@
 /*
  * QEMU Crypto hash algorithms
  *
+ * Copyright (c) 2024 Seagate Technology LLC and/or its Affiliates
  * Copyright (c) 2015 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
@@ -19,6 +20,8 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi-types-crypto.h"
 #include "crypto/hash.h"
 #include "hashpriv.h"
 
@@ -45,23 +48,18 @@ int qcrypto_hash_bytesv(QCryptoHashAlgo alg,
                         size_t *resultlen,
                         Error **errp)
 {
-#ifdef CONFIG_AF_ALG
-    int ret;
-    /*
-     * TODO:
-     * Maybe we should treat some afalg errors as fatal
-     */
-    ret = qcrypto_hash_afalg_driver.hash_bytesv(alg, iov, niov,
-                                                result, resultlen,
-                                                NULL);
-    if (ret == 0) {
-        return ret;
+    g_autoptr(QCryptoHash) ctx = qcrypto_hash_new(alg, errp);
+
+    if (!ctx) {
+        return -1;
+    }
+
+    if (qcrypto_hash_updatev(ctx, iov, niov, errp) < 0 ||
+        qcrypto_hash_finalize_bytes(ctx, result, resultlen, errp) < 0) {
+        return -1;
     }
-#endif
 
-    return qcrypto_hash_lib_driver.hash_bytesv(alg, iov, niov,
-                                               result, resultlen,
-                                               errp);
+    return 0;
 }
 
 
@@ -77,29 +75,130 @@ int qcrypto_hash_bytes(QCryptoHashAlgo alg,
     return qcrypto_hash_bytesv(alg, &iov, 1, result, resultlen, errp);
 }
 
+int qcrypto_hash_updatev(QCryptoHash *hash,
+                         const struct iovec *iov,
+                         size_t niov,
+                         Error **errp)
+{
+    QCryptoHashDriver *drv = hash->driver;
+
+    return drv->hash_update(hash, iov, niov, errp);
+}
+
+int qcrypto_hash_update(QCryptoHash *hash,
+                        const char *buf,
+                        size_t len,
+                        Error **errp)
+{
+    struct iovec iov = { .iov_base = (char *)buf, .iov_len = len };
+
+    return qcrypto_hash_updatev(hash, &iov, 1, errp);
+}
+
+QCryptoHash *qcrypto_hash_new(QCryptoHashAlgo alg, Error **errp)
+{
+    QCryptoHash *hash = NULL;
+
+    if (!qcrypto_hash_supports(alg)) {
+        error_setg(errp, "Unsupported hash algorithm %s",
+                   QCryptoHashAlgo_str(alg));
+        return NULL;
+   }
+
+#ifdef CONFIG_AF_ALG
+    hash = qcrypto_hash_afalg_driver.hash_new(alg, NULL);
+    if (hash) {
+        hash->driver = &qcrypto_hash_afalg_driver;
+        return hash;
+    }
+#endif
+
+    hash = qcrypto_hash_lib_driver.hash_new(alg, errp);
+    if (!hash) {
+        return NULL;
+    }
+
+    hash->driver = &qcrypto_hash_lib_driver;
+    return hash;
+}
+
+void qcrypto_hash_free(QCryptoHash *hash)
+{
+   QCryptoHashDriver *drv;
+
+    if (hash) {
+        drv = hash->driver;
+        drv->hash_free(hash);
+    }
+}
+
+int qcrypto_hash_finalize_bytes(QCryptoHash *hash,
+                                uint8_t **result,
+                                size_t *result_len,
+                                Error **errp)
+{
+    QCryptoHashDriver *drv = hash->driver;
+
+    return drv->hash_finalize(hash, result, result_len, errp);
+}
+
 static const char hex[] = "0123456789abcdef";
 
+int qcrypto_hash_finalize_digest(QCryptoHash *hash,
+                                 char **digest,
+                                 Error **errp)
+{
+    int ret;
+    g_autofree uint8_t *result = NULL;
+    size_t resultlen = 0;
+    size_t i;
+
+    ret = qcrypto_hash_finalize_bytes(hash, &result, &resultlen, errp);
+    if (ret == 0) {
+        *digest = g_new0(char, (resultlen * 2) + 1);
+        for (i = 0 ; i < resultlen ; i++) {
+            (*digest)[(i * 2)] = hex[(result[i] >> 4) & 0xf];
+            (*digest)[(i * 2) + 1] = hex[result[i] & 0xf];
+        }
+        (*digest)[resultlen * 2] = '\0';
+    }
+
+    return ret;
+}
+
+int qcrypto_hash_finalize_base64(QCryptoHash *hash,
+                                 char **base64,
+                                 Error **errp)
+{
+    int ret;
+    g_autofree uint8_t *result = NULL;
+    size_t resultlen = 0;
+
+    ret = qcrypto_hash_finalize_bytes(hash, &result, &resultlen, errp);
+    if (ret == 0) {
+        *base64 = g_base64_encode(result, resultlen);
+    }
+
+    return ret;
+}
+
 int qcrypto_hash_digestv(QCryptoHashAlgo alg,
                          const struct iovec *iov,
                          size_t niov,
                          char **digest,
                          Error **errp)
 {
-    uint8_t *result = NULL;
-    size_t resultlen = 0;
-    size_t i;
+    g_autoptr(QCryptoHash) ctx = qcrypto_hash_new(alg, errp);
 
-    if (qcrypto_hash_bytesv(alg, iov, niov, &result, &resultlen, errp) < 0) {
+    if (!ctx) {
         return -1;
     }
 
-    *digest = g_new0(char, (resultlen * 2) + 1);
-    for (i = 0 ; i < resultlen ; i++) {
-        (*digest)[(i * 2)] = hex[(result[i] >> 4) & 0xf];
-        (*digest)[(i * 2) + 1] = hex[result[i] & 0xf];
+    if (qcrypto_hash_updatev(ctx, iov, niov, errp) < 0 ||
+        qcrypto_hash_finalize_digest(ctx, digest, errp) < 0) {
+        return -1;
     }
-    (*digest)[resultlen * 2] = '\0';
-    g_free(result);
+
     return 0;
 }
 
@@ -120,15 +219,17 @@ int qcrypto_hash_base64v(QCryptoHashAlgo alg,
                          char **base64,
                          Error **errp)
 {
-    uint8_t *result = NULL;
-    size_t resultlen = 0;
+    g_autoptr(QCryptoHash) ctx = qcrypto_hash_new(alg, errp);
+
+    if (!ctx) {
+        return -1;
+    }
 
-    if (qcrypto_hash_bytesv(alg, iov, niov, &result, &resultlen, errp) < 0) {
+    if (qcrypto_hash_updatev(ctx, iov, niov, errp) < 0 ||
+        qcrypto_hash_finalize_base64(ctx, base64, errp) < 0) {
         return -1;
     }
 
-    *base64 = g_base64_encode(result, resultlen);
-    g_free(result);
     return 0;
 }
 
-- 
2.46.0


Re: [PULL 08/17] crypto/hash: Implement and use new hash API
Posted by Thomas Huth 1 month ago
On 10/10/2024 18.20, Daniel P. Berrangé wrote:
> From: Alejandro Zeise <alejandro.zeise@seagate.com>
> 
> Changes the public hash API implementation to support accumulative hashing.
> 
> Implementations for the public functions are added to call the new
> driver functions that implement context creation, updating,
> finalization, and destruction.
> 
> Additionally changes the "shortcut" functions to use these 4 new core
> functions.
> 
> Signed-off-by: Alejandro Zeise <alejandro.zeise@seagate.com>
> [ clg: - Reworked qcrypto_hash_bytesv() error handling
>         - Used hash->driver int qcrypto_hash_new(), qcrypto_hash_free()
>           qcrypto_hash_updatev()
>         - Introduced qcrypto_hash_supports() check in
>           qcrypto_hash_new()
>         - Introduced g_autofree variables in qcrypto_hash_finalize_digest()
>           and qcrypto_hash_finalize_base64()
>         - Re-arrranged code in qcrypto_hash_digestv() and
>           qcrypto_hash_digest()
>         - Checkpatch fixes ]
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   crypto/hash.c | 161 ++++++++++++++++++++++++++++++++++++++++----------
>   1 file changed, 131 insertions(+), 30 deletions(-)

  Hi,

something recently broke qemu-iotest 081 in raw mode and bisecting it 
pointed me to this commit here.

cd tests/qemu-iotests/ ; ./check -raw 081 ; cd ../../
[...]
081   fail       [21:07:40] [21:07:42]   1.4s   (last: 1.4s)  output 
mismatch (see 
/home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/raw-file-081/081.out.bad)
--- /home/thuth/devel/qemu/tests/qemu-iotests/081.out
+++ 
/home/thuth/tmp/qemu-build/tests/qemu-iotests/scratch/raw-file-081/081.out.bad
@@ -31,7 +31,6 @@
  {"return": {}}
  {"return": {}}
  {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"QUORUM_REPORT_BAD", "data": {"node-name": "drive2", "sectors-count": 20480, 
"sector-num": 0, "type": "read"}}
  read 10485760/10485760 bytes at offset 0
  10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
  {"return": ""}
@@ -44,6 +43,7 @@
  10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

  == checking that quorum has corrected the corrupted file ==
+Pattern verification failed at offset 0, 10485760 bytes
  read 10485760/10485760 bytes at offset 0
  10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

@@ -63,7 +63,6 @@
      } -device virtio-scsi,id=scsi -device 
scsi-hd,id=quorum-drive,bus=scsi.0,drive=quorum
  QMP_VERSION
  {"return": {}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"QUORUM_REPORT_BAD", "data": {"node-name": "file2", "sectors-count": 20480, 
"sector-num": 0, "type": "read"}}
  read 10485760/10485760 bytes at offset 0
  10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
  {"return": ""}
@@ -71,6 +70,7 @@
  {"return": {}}

  -- checking that the image has been corrected --
+Pattern verification failed at offset 0, 10485760 bytes
  read 10485760/10485760 bytes at offset 0
  10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

@@ -81,7 +81,9 @@
  10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

  == checking that quorum is broken ==
-read failed: Input/output error
+Pattern verification failed at offset 0, 10485760 bytes
+read 10485760/10485760 bytes at offset 0
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

  == checking the blkverify mode with broken content ==
  quorum: offset=0 bytes=10485760 contents mismatch at offset 0
Failures: 081
Failed 1 of 1 iotests

Could you please have a look?

  Thanks,
   Thomas


Re: [PULL 08/17] crypto/hash: Implement and use new hash API
Posted by Daniel P. Berrangé 1 month ago
On Tue, Oct 22, 2024 at 09:10:50PM +0200, Thomas Huth wrote:
> On 10/10/2024 18.20, Daniel P. Berrangé wrote:
> > From: Alejandro Zeise <alejandro.zeise@seagate.com>
> > 
> > Changes the public hash API implementation to support accumulative hashing.
> > 
> > Implementations for the public functions are added to call the new
> > driver functions that implement context creation, updating,
> > finalization, and destruction.
> > 
> > Additionally changes the "shortcut" functions to use these 4 new core
> > functions.
> > 
> > Signed-off-by: Alejandro Zeise <alejandro.zeise@seagate.com>
> > [ clg: - Reworked qcrypto_hash_bytesv() error handling
> >         - Used hash->driver int qcrypto_hash_new(), qcrypto_hash_free()
> >           qcrypto_hash_updatev()
> >         - Introduced qcrypto_hash_supports() check in
> >           qcrypto_hash_new()
> >         - Introduced g_autofree variables in qcrypto_hash_finalize_digest()
> >           and qcrypto_hash_finalize_base64()
> >         - Re-arrranged code in qcrypto_hash_digestv() and
> >           qcrypto_hash_digest()
> >         - Checkpatch fixes ]
> > Signed-off-by: Cédric Le Goater <clg@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   crypto/hash.c | 161 ++++++++++++++++++++++++++++++++++++++++----------
> >   1 file changed, 131 insertions(+), 30 deletions(-)
> 
>  Hi,
> 
> something recently broke qemu-iotest 081 in raw mode and bisecting it
> pointed me to this commit here.

snip

> Could you please have a look?

This is fixed by my current pending pull request


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|