All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash
@ 2026-11-02 12:23 Daniel P. Berrangé
  2026-11-02 12:23 ` [PATCH v2 1/4] crypto: set default initializer hmac/hash arrays to -1 Daniel P. Berrangé
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2026-11-02 12:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Daniel P. Berrangé

Note that patch 4 is best reviewed by applying to git and then using
'git show -b' to hide the huge whitespace indentation change.

Changed in v2:

 * Use array range initializer for setting default value
 * Drop redundant countermeasures now we can gurantee a
   correct default initializer value
 * Drop SM3 conditions in tests
 * Refactor tests to register one test per algorithm

Daniel P. Berrangé (4):
  crypto: set default initializer hmac/hash arrays to -1
  crypto: unconditionally report SM3 hash length
  tests/unit: drop conditionals for SM3
  tests/unit: register one test per hash/hmac algorithm

 crypto/hash-glib.c            |   3 +-
 crypto/hash.c                 |   4 +-
 crypto/hmac-glib.c            |   4 +-
 tests/unit/test-crypto-hash.c | 325 +++++++++++++++-------------------
 tests/unit/test-crypto-hmac.c | 272 +++++++++++++---------------
 5 files changed, 275 insertions(+), 333 deletions(-)

-- 
2.56.0



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

* [PATCH v2 1/4] crypto: set default initializer hmac/hash arrays to -1
  2026-11-02 12:23 [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash Daniel P. Berrangé
@ 2026-11-02 12:23 ` Daniel P. Berrangé
  2026-11-02 12:23 ` [PATCH v2 2/4] crypto: unconditionally report SM3 hash length Daniel P. Berrangé
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2026-11-02 12:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Daniel P. Berrangé

The default static array initializer of 0 is not suitable for
the glib hash/hmac backends, as G_CHECKSUM_MD5 has a value of
0. Instead elements must be initialized to -1, but remembering
todo that as new algorithms are defined is errorprone. The
SM3 algorithm initializer is missing, leading to it being
mapped to MD5. This could potentially be a security flaw through
use of the wrong (much weaker) hashing algorithm, if a further
bug elsewhere didn't mask the issue.

Set a default array initializer so unlisted algorithms are
always set to -1 instead of 0.

Fixes: https://gitlab.com/qemu-project/qemu/-/work_items/4786
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/hash-glib.c | 3 +--
 crypto/hmac-glib.c | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/crypto/hash-glib.c b/crypto/hash-glib.c
index 809cef98aee..d915430f459 100644
--- a/crypto/hash-glib.c
+++ b/crypto/hash-glib.c
@@ -26,13 +26,12 @@
 
 
 static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
+    [0 ... QCRYPTO_HASH_ALGO__MAX - 1] = -1,
     [QCRYPTO_HASH_ALGO_MD5] = G_CHECKSUM_MD5,
     [QCRYPTO_HASH_ALGO_SHA1] = G_CHECKSUM_SHA1,
-    [QCRYPTO_HASH_ALGO_SHA224] = -1,
     [QCRYPTO_HASH_ALGO_SHA256] = G_CHECKSUM_SHA256,
     [QCRYPTO_HASH_ALGO_SHA384] = G_CHECKSUM_SHA384,
     [QCRYPTO_HASH_ALGO_SHA512] = G_CHECKSUM_SHA512,
-    [QCRYPTO_HASH_ALGO_RIPEMD160] = -1,
 };
 
 gboolean qcrypto_hash_supports(QCryptoHashAlgo alg)
diff --git a/crypto/hmac-glib.c b/crypto/hmac-glib.c
index 1f17769c1c7..4453e0d5f06 100644
--- a/crypto/hmac-glib.c
+++ b/crypto/hmac-glib.c
@@ -18,13 +18,11 @@
 #include "hmacpriv.h"
 
 static int qcrypto_hmac_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
+    [0 ... QCRYPTO_HASH_ALGO__MAX - 1] = -1,
     [QCRYPTO_HASH_ALGO_MD5] = G_CHECKSUM_MD5,
     [QCRYPTO_HASH_ALGO_SHA1] = G_CHECKSUM_SHA1,
     [QCRYPTO_HASH_ALGO_SHA256] = G_CHECKSUM_SHA256,
     [QCRYPTO_HASH_ALGO_SHA512] = G_CHECKSUM_SHA512,
-    [QCRYPTO_HASH_ALGO_SHA224] = -1,
-    [QCRYPTO_HASH_ALGO_SHA384] = -1,
-    [QCRYPTO_HASH_ALGO_RIPEMD160] = -1,
 };
 
 typedef struct QCryptoHmacGlib QCryptoHmacGlib;
-- 
2.56.0



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

* [PATCH v2 2/4] crypto: unconditionally report SM3 hash length
  2026-11-02 12:23 [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash Daniel P. Berrangé
  2026-11-02 12:23 ` [PATCH v2 1/4] crypto: set default initializer hmac/hash arrays to -1 Daniel P. Berrangé
@ 2026-11-02 12:23 ` Daniel P. Berrangé
  2026-11-02 12:23 ` [PATCH v2 3/4] tests/unit: drop conditionals for SM3 Daniel P. Berrangé
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2026-11-02 12:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Daniel P. Berrangé

Callers may query the hash digest length before getting to the
code path that performs hashing, which is where the lack of
support for SM3 in the backend will be reported.

Reporting a hash length of 0 is liable to cause divide by zero
errors in callers.

Fixes: https://gitlab.com/qemu-project/qemu/-/work_items/4786
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/hash.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/crypto/hash.c b/crypto/hash.c
index 6ffb88bf541..42a0a8ec56f 100644
--- a/crypto/hash.c
+++ b/crypto/hash.c
@@ -33,9 +33,7 @@ static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALGO__MAX] = {
     [QCRYPTO_HASH_ALGO_SHA384]    = QCRYPTO_HASH_DIGEST_LEN_SHA384,
     [QCRYPTO_HASH_ALGO_SHA512]    = QCRYPTO_HASH_DIGEST_LEN_SHA512,
     [QCRYPTO_HASH_ALGO_RIPEMD160] = QCRYPTO_HASH_DIGEST_LEN_RIPEMD160,
-#ifdef CONFIG_CRYPTO_SM3
-    [QCRYPTO_HASH_ALGO_SM3] = QCRYPTO_HASH_DIGEST_LEN_SM3,
-#endif
+    [QCRYPTO_HASH_ALGO_SM3]       = QCRYPTO_HASH_DIGEST_LEN_SM3,
 };
 
 size_t qcrypto_hash_digest_len(QCryptoHashAlgo alg)
-- 
2.56.0



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

* [PATCH v2 3/4] tests/unit: drop conditionals for SM3
  2026-11-02 12:23 [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash Daniel P. Berrangé
  2026-11-02 12:23 ` [PATCH v2 1/4] crypto: set default initializer hmac/hash arrays to -1 Daniel P. Berrangé
  2026-11-02 12:23 ` [PATCH v2 2/4] crypto: unconditionally report SM3 hash length Daniel P. Berrangé
@ 2026-11-02 12:23 ` Daniel P. Berrangé
  2026-11-02 12:23 ` [PATCH v2 4/4] tests/unit: register one test per hash/hmac algorithm Daniel P. Berrangé
  2026-11-12 18:00 ` [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash Daniel P. Berrangé
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2026-11-02 12:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Daniel P. Berrangé

There is no need to conditionalize test data for SM3, since the
test explicitly checks whether each algorithm is supported.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/unit/test-crypto-hash.c | 10 ----------
 tests/unit/test-crypto-hmac.c |  4 +---
 2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/tests/unit/test-crypto-hash.c b/tests/unit/test-crypto-hash.c
index 8fee1593f94..1813e77b5a5 100644
--- a/tests/unit/test-crypto-hash.c
+++ b/tests/unit/test-crypto-hash.c
@@ -43,9 +43,7 @@
                       "63b54e4cb2d2032b393994aa263c0dbb" \
                       "e00a9f2fe9ef6037352232a1eec55ee7"
 #define OUTPUT_RIPEMD160 "f3d658fad3fdfb2b52c9369cf0d441249ddfa8a0"
-#ifdef CONFIG_CRYPTO_SM3
 #define OUTPUT_SM3 "d4a97db105b477b84c4f20ec9c31a6c814e2705a0b83a5a89748d75f0ef456a1"
-#endif
 
 #define OUTPUT_MD5_B64 "Yo0gY3FWMDWrjvYvSSveyQ=="
 #define OUTPUT_SHA1_B64 "sudPJnWKOkIeUJzuBFJEt4dTzAI="
@@ -58,9 +56,7 @@
                           "7sVe5w=="
 #define OUTPUT_RIPEMD160_B64 "89ZY+tP9+ytSyTac8NRBJJ3fqKA="
 
-#ifdef CONFIG_CRYPTO_SM3
 #define OUTPUT_SM3_B64 "1Kl9sQW0d7hMTyDsnDGmyBTicFoLg6Wol0jXXw70VqE="
-#endif
 
 static const char *expected_outputs[] = {
     [QCRYPTO_HASH_ALGO_MD5] = OUTPUT_MD5,
@@ -70,9 +66,7 @@ static const char *expected_outputs[] = {
     [QCRYPTO_HASH_ALGO_SHA384] = OUTPUT_SHA384,
     [QCRYPTO_HASH_ALGO_SHA512] = OUTPUT_SHA512,
     [QCRYPTO_HASH_ALGO_RIPEMD160] = OUTPUT_RIPEMD160,
-#ifdef CONFIG_CRYPTO_SM3
     [QCRYPTO_HASH_ALGO_SM3] = OUTPUT_SM3,
-#endif
 };
 static const char *expected_outputs_b64[] = {
     [QCRYPTO_HASH_ALGO_MD5] = OUTPUT_MD5_B64,
@@ -82,9 +76,7 @@ static const char *expected_outputs_b64[] = {
     [QCRYPTO_HASH_ALGO_SHA384] = OUTPUT_SHA384_B64,
     [QCRYPTO_HASH_ALGO_SHA512] = OUTPUT_SHA512_B64,
     [QCRYPTO_HASH_ALGO_RIPEMD160] = OUTPUT_RIPEMD160_B64,
-#ifdef CONFIG_CRYPTO_SM3
     [QCRYPTO_HASH_ALGO_SM3] = OUTPUT_SM3_B64,
-#endif
 };
 static const int expected_lens[] = {
     [QCRYPTO_HASH_ALGO_MD5] = 16,
@@ -94,9 +86,7 @@ static const int expected_lens[] = {
     [QCRYPTO_HASH_ALGO_SHA384] = 48,
     [QCRYPTO_HASH_ALGO_SHA512] = 64,
     [QCRYPTO_HASH_ALGO_RIPEMD160] = 20,
-#ifdef CONFIG_CRYPTO_SM3
     [QCRYPTO_HASH_ALGO_SM3] = 32,
-#endif
 };
 
 static const char hex[] = "0123456789abcdef";
diff --git a/tests/unit/test-crypto-hmac.c b/tests/unit/test-crypto-hmac.c
index 20c60eb9d8b..f7158621908 100644
--- a/tests/unit/test-crypto-hmac.c
+++ b/tests/unit/test-crypto-hmac.c
@@ -76,14 +76,12 @@ static QCryptoHmacTestData test_data[] = {
             "94964ed4c1155b62b668c241d67279e5"
             "8a711676",
     },
-#ifdef CONFIG_CRYPTO_SM3
     {
         .alg = QCRYPTO_HASH_ALGO_SM3,
         .hex_digest =
             "760e3799332bc913819b930085360ddb"
-    "c05529261313d5b15b75bab4fd7ae91e",
+            "c05529261313d5b15b75bab4fd7ae91e",
     },
-#endif
 };
 
 static const char hex[] = "0123456789abcdef";
-- 
2.56.0



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

* [PATCH v2 4/4] tests/unit: register one test per hash/hmac algorithm
  2026-11-02 12:23 [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash Daniel P. Berrangé
                   ` (2 preceding siblings ...)
  2026-11-02 12:23 ` [PATCH v2 3/4] tests/unit: drop conditionals for SM3 Daniel P. Berrangé
@ 2026-11-02 12:23 ` Daniel P. Berrangé
  2026-11-12 18:00 ` [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash Daniel P. Berrangé
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2026-11-02 12:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé, Daniel P. Berrangé

The hash/hmac tests iterate over all support algorithms testing
each one in turn, but this is not visible from the test output.
Register one test per algorithm so the test result directly
reflect what algorithms were tested. This is useful information
since different crypto backends support different subsets of
algorithms.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/unit/test-crypto-hash.c | 315 ++++++++++++++++------------------
 tests/unit/test-crypto-hmac.c | 268 ++++++++++++++---------------
 2 files changed, 271 insertions(+), 312 deletions(-)

diff --git a/tests/unit/test-crypto-hash.c b/tests/unit/test-crypto-hash.c
index 1813e77b5a5..fbdae46fd64 100644
--- a/tests/unit/test-crypto-hash.c
+++ b/tests/unit/test-crypto-hash.c
@@ -92,218 +92,191 @@ static const int expected_lens[] = {
 static const char hex[] = "0123456789abcdef";
 
 /* Test with dynamic allocation */
-static void test_hash_alloc(void)
+static void test_hash_alloc(const void *opaque)
 {
-    size_t i;
-
-    for (i = 0; i < G_N_ELEMENTS(expected_outputs) ; i++) {
-        uint8_t *result = NULL;
-        size_t resultlen = 0;
-        int ret;
-        size_t j;
-
-        if (!qcrypto_hash_supports(i)) {
-            continue;
-        }
-
-        ret = qcrypto_hash_bytes(i,
-                                 INPUT_TEXT,
-                                 strlen(INPUT_TEXT),
-                                 &result,
-                                 &resultlen,
-                                 &error_fatal);
-        g_assert(ret == 0);
-        g_assert(resultlen == expected_lens[i]);
+    QCryptoHashAlgo alg = GPOINTER_TO_INT(opaque);
+    uint8_t *result = NULL;
+    size_t resultlen = 0;
+    int ret;
+    size_t j;
+
+    ret = qcrypto_hash_bytes(alg,
+                             INPUT_TEXT,
+                             strlen(INPUT_TEXT),
+                             &result,
+                             &resultlen,
+                             &error_fatal);
+    g_assert(ret == 0);
+    g_assert(resultlen == expected_lens[alg]);
 
-        for (j = 0; j < resultlen; j++) {
-            g_assert(expected_outputs[i][j * 2] == hex[(result[j] >> 4) & 0xf]);
-            g_assert(expected_outputs[i][j * 2 + 1] == hex[result[j] & 0xf]);
-        }
-        g_free(result);
+    for (j = 0; j < resultlen; j++) {
+        g_assert(expected_outputs[alg][j * 2] == hex[(result[j] >> 4) & 0xf]);
+        g_assert(expected_outputs[alg][j * 2 + 1] == hex[result[j] & 0xf]);
     }
+    g_free(result);
 }
 
 /* Test with caller preallocating */
-static void test_hash_prealloc(void)
+static void test_hash_prealloc(const void *opaque)
 {
-    size_t i;
-
-    for (i = 0; i < G_N_ELEMENTS(expected_outputs) ; i++) {
-        uint8_t *result, *origresult;
-        size_t resultlen;
-        int ret;
-        size_t j;
-
-        if (!qcrypto_hash_supports(i)) {
-            continue;
-        }
-
-        resultlen = expected_lens[i];
-        origresult = result = g_new0(uint8_t, resultlen);
-
-        ret = qcrypto_hash_bytes(i,
-                                 INPUT_TEXT,
-                                 strlen(INPUT_TEXT),
-                                 &result,
-                                 &resultlen,
-                                 &error_fatal);
-        g_assert(ret == 0);
-        /* Validate that our pre-allocated pointer was not replaced */
-        g_assert(result == origresult);
-        g_assert(resultlen == expected_lens[i]);
-        for (j = 0; j < resultlen; j++) {
-            g_assert(expected_outputs[i][j * 2] == hex[(result[j] >> 4) & 0xf]);
-            g_assert(expected_outputs[i][j * 2 + 1] == hex[result[j] & 0xf]);
-        }
-        g_free(result);
+    QCryptoHashAlgo alg = GPOINTER_TO_INT(opaque);
+    uint8_t *result, *origresult;
+    size_t resultlen;
+    int ret;
+    size_t j;
+
+    resultlen = expected_lens[alg];
+    origresult = result = g_new0(uint8_t, resultlen);
+
+    ret = qcrypto_hash_bytes(alg,
+                             INPUT_TEXT,
+                             strlen(INPUT_TEXT),
+                             &result,
+                             &resultlen,
+                             &error_fatal);
+    g_assert(ret == 0);
+    /* Validate that our pre-allocated pointer was not replaced */
+    g_assert(result == origresult);
+    g_assert(resultlen == expected_lens[alg]);
+    for (j = 0; j < resultlen; j++) {
+        g_assert(expected_outputs[alg][j * 2] == hex[(result[j] >> 4) & 0xf]);
+        g_assert(expected_outputs[alg][j * 2 + 1] == hex[result[j] & 0xf]);
     }
+    g_free(result);
 }
 
 
 /* Test with dynamic allocation */
-static void test_hash_iov(void)
+static void test_hash_iov(const void *opaque)
 {
-    size_t i;
-
-    for (i = 0; i < G_N_ELEMENTS(expected_outputs) ; i++) {
-        struct iovec iov[3] = {
-            { .iov_base = (char *)INPUT_TEXT1, .iov_len = strlen(INPUT_TEXT1) },
-            { .iov_base = (char *)INPUT_TEXT2, .iov_len = strlen(INPUT_TEXT2) },
-            { .iov_base = (char *)INPUT_TEXT3, .iov_len = strlen(INPUT_TEXT3) },
-        };
-        uint8_t *result = NULL;
-        size_t resultlen = 0;
-        int ret;
-        size_t j;
-
-        if (!qcrypto_hash_supports(i)) {
-            continue;
-        }
-
-        ret = qcrypto_hash_bytesv(i,
-                                  iov, 3,
-                                  &result,
-                                  &resultlen,
-                                  &error_fatal);
-        g_assert(ret == 0);
-        g_assert(resultlen == expected_lens[i]);
-        for (j = 0; j < resultlen; j++) {
-            g_assert(expected_outputs[i][j * 2] == hex[(result[j] >> 4) & 0xf]);
-            g_assert(expected_outputs[i][j * 2 + 1] == hex[result[j] & 0xf]);
-        }
-        g_free(result);
+    QCryptoHashAlgo alg = GPOINTER_TO_INT(opaque);
+    struct iovec iov[3] = {
+        { .iov_base = (char *)INPUT_TEXT1, .iov_len = strlen(INPUT_TEXT1) },
+        { .iov_base = (char *)INPUT_TEXT2, .iov_len = strlen(INPUT_TEXT2) },
+        { .iov_base = (char *)INPUT_TEXT3, .iov_len = strlen(INPUT_TEXT3) },
+    };
+    uint8_t *result = NULL;
+    size_t resultlen = 0;
+    int ret;
+    size_t j;
+
+    ret = qcrypto_hash_bytesv(alg,
+                              iov, 3,
+                              &result,
+                              &resultlen,
+                              &error_fatal);
+    g_assert(ret == 0);
+    g_assert(resultlen == expected_lens[alg]);
+    for (j = 0; j < resultlen; j++) {
+        g_assert(expected_outputs[alg][j * 2] == hex[(result[j] >> 4) & 0xf]);
+        g_assert(expected_outputs[alg][j * 2 + 1] == hex[result[j] & 0xf]);
     }
+    g_free(result);
 }
 
 
 /* Test with printable hashing */
-static void test_hash_digest(void)
+static void test_hash_digest(const void *opaque)
 {
-    size_t i;
-
-    for (i = 0; i < G_N_ELEMENTS(expected_outputs) ; i++) {
-        int ret;
-        char *digest;
-        size_t digestsize;
-
-        if (!qcrypto_hash_supports(i)) {
-            continue;
-        }
+    QCryptoHashAlgo alg = GPOINTER_TO_INT(opaque);
+    int ret;
+    char *digest;
+    size_t digestsize;
 
-        digestsize = qcrypto_hash_digest_len(i);
+    digestsize = qcrypto_hash_digest_len(alg);
 
-        g_assert_cmpint(digestsize * 2, ==, strlen(expected_outputs[i]));
+    g_assert_cmpint(digestsize * 2, ==, strlen(expected_outputs[alg]));
 
-        ret = qcrypto_hash_digest(i,
-                                  INPUT_TEXT,
-                                  strlen(INPUT_TEXT),
-                                  &digest,
-                                  &error_fatal);
-        g_assert(ret == 0);
-        g_assert_cmpstr(digest, ==, expected_outputs[i]);
-        g_free(digest);
-    }
+    ret = qcrypto_hash_digest(alg,
+                              INPUT_TEXT,
+                              strlen(INPUT_TEXT),
+                              &digest,
+                              &error_fatal);
+    g_assert(ret == 0);
+    g_assert_cmpstr(digest, ==, expected_outputs[alg]);
+    g_free(digest);
 }
 
 /* Test with base64 encoding */
-static void test_hash_base64(void)
+static void test_hash_base64(const void *opaque)
 {
-    size_t i;
-
-    for (i = 0; i < G_N_ELEMENTS(expected_outputs) ; i++) {
-        int ret;
-        char *digest;
-
-        if (!qcrypto_hash_supports(i)) {
-            continue;
-        }
-
-        ret = qcrypto_hash_base64(i,
-                                  INPUT_TEXT,
-                                  strlen(INPUT_TEXT),
-                                  &digest,
-                                  &error_fatal);
-        g_assert(ret == 0);
-        g_assert_cmpstr(digest, ==, expected_outputs_b64[i]);
-        g_free(digest);
-    }
+    QCryptoHashAlgo alg = GPOINTER_TO_INT(opaque);
+    int ret;
+    char *digest;
+
+    ret = qcrypto_hash_base64(alg,
+                              INPUT_TEXT,
+                              strlen(INPUT_TEXT),
+                              &digest,
+                              &error_fatal);
+    g_assert(ret == 0);
+    g_assert_cmpstr(digest, ==, expected_outputs_b64[alg]);
+    g_free(digest);
 }
 
-static void test_hash_accumulate(void)
+static void test_hash_accumulate(const void *opaque)
 {
-    size_t i;
-
-    for (i = 0; i < G_N_ELEMENTS(expected_outputs) ; i++) {
-        g_autoptr(QCryptoHash) hash = NULL;
-        struct iovec iov[] = {
-            { .iov_base = (char *)INPUT_TEXT1, .iov_len = strlen(INPUT_TEXT1) },
-            { .iov_base = (char *)INPUT_TEXT2, .iov_len = strlen(INPUT_TEXT2) },
-            { .iov_base = (char *)INPUT_TEXT3, .iov_len = strlen(INPUT_TEXT3) },
-        };
-        g_autofree uint8_t *result = NULL;
-        size_t resultlen = 0;
-        int ret;
-        size_t j;
-
-        if (!qcrypto_hash_supports(i)) {
-            continue;
-        }
+    QCryptoHashAlgo alg = GPOINTER_TO_INT(opaque);
+    g_autoptr(QCryptoHash) hash = NULL;
+    struct iovec iov[] = {
+        { .iov_base = (char *)INPUT_TEXT1, .iov_len = strlen(INPUT_TEXT1) },
+        { .iov_base = (char *)INPUT_TEXT2, .iov_len = strlen(INPUT_TEXT2) },
+        { .iov_base = (char *)INPUT_TEXT3, .iov_len = strlen(INPUT_TEXT3) },
+    };
+    g_autofree uint8_t *result = NULL;
+    size_t resultlen = 0;
+    int ret;
+    size_t j;
+
+    hash = qcrypto_hash_new(alg, &error_fatal);
+    g_assert(hash != NULL);
+
+    /* Add each iovec to the hash context separately */
+    for (j = 0; j < G_N_ELEMENTS(iov); j++) {
+        ret = qcrypto_hash_updatev(hash,
+                                   &iov[j], 1,
+                                   &error_fatal);
 
-        hash = qcrypto_hash_new(i, &error_fatal);
-        g_assert(hash != NULL);
+        g_assert(ret == 0);
+    }
 
-        /* Add each iovec to the hash context separately */
-        for (j = 0; j < G_N_ELEMENTS(iov); j++) {
-            ret = qcrypto_hash_updatev(hash,
-                                      &iov[j], 1,
+    ret = qcrypto_hash_finalize_bytes(hash, &result, &resultlen,
                                       &error_fatal);
 
-            g_assert(ret == 0);
-        }
-
-        ret = qcrypto_hash_finalize_bytes(hash, &result, &resultlen,
-                                          &error_fatal);
-
-        g_assert(ret == 0);
-        g_assert(resultlen == expected_lens[i]);
-        for (j = 0; j < resultlen; j++) {
-            g_assert(expected_outputs[i][j * 2] == hex[(result[j] >> 4) & 0xf]);
-            g_assert(expected_outputs[i][j * 2 + 1] == hex[result[j] & 0xf]);
-        }
+    g_assert(ret == 0);
+    g_assert(resultlen == expected_lens[alg]);
+    for (j = 0; j < resultlen; j++) {
+        g_assert(expected_outputs[alg][j * 2] == hex[(result[j] >> 4) & 0xf]);
+        g_assert(expected_outputs[alg][j * 2 + 1] == hex[result[j] & 0xf]);
     }
 }
 
 int main(int argc, char **argv)
 {
+    size_t i;
     int ret = qcrypto_init(&error_fatal);
     g_assert(ret == 0);
 
     g_test_init(&argc, &argv, NULL);
-    g_test_add_func("/crypto/hash/iov", test_hash_iov);
-    g_test_add_func("/crypto/hash/alloc", test_hash_alloc);
-    g_test_add_func("/crypto/hash/prealloc", test_hash_prealloc);
-    g_test_add_func("/crypto/hash/digest", test_hash_digest);
-    g_test_add_func("/crypto/hash/base64", test_hash_base64);
-    g_test_add_func("/crypto/hash/accumulate", test_hash_accumulate);
+
+#define ADD_TEST(alg, prefix, meth) \
+    { \
+        g_autofree char *name = g_strdup_printf( \
+            "%s/%s", prefix, QCryptoHashAlgo_str(alg)); \
+        g_test_add_data_func(name, GINT_TO_POINTER(alg), meth); \
+    }
+
+    for (i = 0; i < QCRYPTO_HASH_ALGO__MAX ; i++) {
+        if (!qcrypto_hash_supports(i)) {
+            continue;
+        }
+
+        ADD_TEST(i, "/crypto/hash/iov", test_hash_iov);
+        ADD_TEST(i, "/crypto/hash/alloc", test_hash_alloc);
+        ADD_TEST(i, "/crypto/hash/prealloc", test_hash_prealloc);
+        ADD_TEST(i, "/crypto/hash/digest", test_hash_digest);
+        ADD_TEST(i, "/crypto/hash/base64", test_hash_base64);
+        ADD_TEST(i, "/crypto/hash/accumulate", test_hash_accumulate);
+    }
     return g_test_run();
 }
diff --git a/tests/unit/test-crypto-hmac.c b/tests/unit/test-crypto-hmac.c
index f7158621908..9c2ee8c17f8 100644
--- a/tests/unit/test-crypto-hmac.c
+++ b/tests/unit/test-crypto-hmac.c
@@ -86,177 +86,163 @@ static QCryptoHmacTestData test_data[] = {
 
 static const char hex[] = "0123456789abcdef";
 
-static void test_hmac_alloc(void)
+static void test_hmac_alloc(const void *opaque)
 {
-    size_t i;
-
-    for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
-        QCryptoHmacTestData *data = &test_data[i];
-        QCryptoHmac *hmac = NULL;
-        uint8_t *result = NULL;
-        size_t resultlen = 0;
-        const char *exp_output = NULL;
-        int ret;
-        size_t j;
-
-        if (!qcrypto_hmac_supports(data->alg)) {
-            return;
-        }
-
-        exp_output = data->hex_digest;
-
-        hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
-                                strlen(KEY), &error_fatal);
-        g_assert(hmac != NULL);
-
-        ret = qcrypto_hmac_bytes(hmac, (const char *)INPUT_TEXT,
-                                 strlen(INPUT_TEXT), &result,
-                                 &resultlen, &error_fatal);
-        g_assert(ret == 0);
-
-        for (j = 0; j < resultlen; j++) {
-            g_assert(exp_output[j * 2] == hex[(result[j] >> 4) & 0xf]);
-            g_assert(exp_output[j * 2 + 1] == hex[result[j] & 0xf]);
-        }
+    QCryptoHashAlgo alg = GPOINTER_TO_INT(opaque);
+    QCryptoHmacTestData *data = &test_data[alg];
+    QCryptoHmac *hmac = NULL;
+    uint8_t *result = NULL;
+    size_t resultlen = 0;
+    const char *exp_output = NULL;
+    int ret;
+    size_t j;
+
+    exp_output = data->hex_digest;
+
+    hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
+                            strlen(KEY), &error_fatal);
+    g_assert(hmac != NULL);
+
+    ret = qcrypto_hmac_bytes(hmac, (const char *)INPUT_TEXT,
+                             strlen(INPUT_TEXT), &result,
+                             &resultlen, &error_fatal);
+    g_assert(ret == 0);
+
+    for (j = 0; j < resultlen; j++) {
+        g_assert(exp_output[j * 2] == hex[(result[j] >> 4) & 0xf]);
+        g_assert(exp_output[j * 2 + 1] == hex[result[j] & 0xf]);
+    }
 
-        qcrypto_hmac_free(hmac);
+    qcrypto_hmac_free(hmac);
 
-        g_free(result);
-    }
+    g_free(result);
 }
 
-static void test_hmac_prealloc(void)
+static void test_hmac_prealloc(const void *opaque)
 {
-    size_t i;
-
-    for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
-        QCryptoHmacTestData *data = &test_data[i];
-        QCryptoHmac *hmac = NULL;
-        uint8_t *result = NULL, *origresult = NULL;
-        size_t resultlen = 0;
-        const char *exp_output = NULL;
-        int ret;
-        size_t j;
-
-        if (!qcrypto_hmac_supports(data->alg)) {
-            return;
-        }
-
-        exp_output = data->hex_digest;
-
-        resultlen = strlen(exp_output) / 2;
-        origresult = result = g_new0(uint8_t, resultlen);
-
-        hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
-                                strlen(KEY), &error_fatal);
-        g_assert(hmac != NULL);
-
-        ret = qcrypto_hmac_bytes(hmac, (const char *)INPUT_TEXT,
-                                 strlen(INPUT_TEXT), &result,
-                                 &resultlen, &error_fatal);
-        g_assert(ret == 0);
-        /* Validate that our pre-allocated pointer was not replaced */
-        g_assert(result == origresult);
-
-        exp_output = data->hex_digest;
-        for (j = 0; j < resultlen; j++) {
-            g_assert(exp_output[j * 2] == hex[(result[j] >> 4) & 0xf]);
-            g_assert(exp_output[j * 2 + 1] == hex[result[j] & 0xf]);
-        }
+    QCryptoHashAlgo alg = GPOINTER_TO_INT(opaque);
+    QCryptoHmacTestData *data = &test_data[alg];
+    QCryptoHmac *hmac = NULL;
+    uint8_t *result = NULL, *origresult = NULL;
+    size_t resultlen = 0;
+    const char *exp_output = NULL;
+    int ret;
+    size_t j;
+
+    exp_output = data->hex_digest;
+
+    resultlen = strlen(exp_output) / 2;
+    origresult = result = g_new0(uint8_t, resultlen);
+
+    hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
+                            strlen(KEY), &error_fatal);
+    g_assert(hmac != NULL);
+
+    ret = qcrypto_hmac_bytes(hmac, (const char *)INPUT_TEXT,
+                             strlen(INPUT_TEXT), &result,
+                             &resultlen, &error_fatal);
+    g_assert(ret == 0);
+    /* Validate that our pre-allocated pointer was not replaced */
+    g_assert(result == origresult);
+
+    exp_output = data->hex_digest;
+    for (j = 0; j < resultlen; j++) {
+        g_assert(exp_output[j * 2] == hex[(result[j] >> 4) & 0xf]);
+        g_assert(exp_output[j * 2 + 1] == hex[result[j] & 0xf]);
+    }
 
-        qcrypto_hmac_free(hmac);
+    qcrypto_hmac_free(hmac);
 
-        g_free(result);
-    }
+    g_free(result);
 }
 
-static void test_hmac_iov(void)
+static void test_hmac_iov(const void *opaque)
 {
-    size_t i;
-
-    for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
-        QCryptoHmacTestData *data = &test_data[i];
-        QCryptoHmac *hmac = NULL;
-        uint8_t *result = NULL;
-        size_t resultlen = 0;
-        const char *exp_output = NULL;
-        int ret;
-        size_t j;
-        struct iovec iov[3] = {
-            { .iov_base = (char *)INPUT_TEXT1, .iov_len = strlen(INPUT_TEXT1) },
-            { .iov_base = (char *)INPUT_TEXT2, .iov_len = strlen(INPUT_TEXT2) },
-            { .iov_base = (char *)INPUT_TEXT3, .iov_len = strlen(INPUT_TEXT3) },
-        };
-
-        if (!qcrypto_hmac_supports(data->alg)) {
-            return;
-        }
-
-        exp_output = data->hex_digest;
-
-        hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
-                                strlen(KEY), &error_fatal);
-        g_assert(hmac != NULL);
-
-        ret = qcrypto_hmac_bytesv(hmac, iov, 3, &result,
-                                  &resultlen, &error_fatal);
-        g_assert(ret == 0);
-
-        for (j = 0; j < resultlen; j++) {
-            g_assert(exp_output[j * 2] == hex[(result[j] >> 4) & 0xf]);
-            g_assert(exp_output[j * 2 + 1] == hex[result[j] & 0xf]);
-        }
+    QCryptoHashAlgo alg = GPOINTER_TO_INT(opaque);
+    QCryptoHmacTestData *data = &test_data[alg];
+    QCryptoHmac *hmac = NULL;
+    uint8_t *result = NULL;
+    size_t resultlen = 0;
+    const char *exp_output = NULL;
+    int ret;
+    size_t j;
+    struct iovec iov[3] = {
+        { .iov_base = (char *)INPUT_TEXT1, .iov_len = strlen(INPUT_TEXT1) },
+        { .iov_base = (char *)INPUT_TEXT2, .iov_len = strlen(INPUT_TEXT2) },
+        { .iov_base = (char *)INPUT_TEXT3, .iov_len = strlen(INPUT_TEXT3) },
+    };
+
+    exp_output = data->hex_digest;
+
+    hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
+                            strlen(KEY), &error_fatal);
+    g_assert(hmac != NULL);
+
+    ret = qcrypto_hmac_bytesv(hmac, iov, 3, &result,
+                              &resultlen, &error_fatal);
+    g_assert(ret == 0);
+
+    for (j = 0; j < resultlen; j++) {
+        g_assert(exp_output[j * 2] == hex[(result[j] >> 4) & 0xf]);
+        g_assert(exp_output[j * 2 + 1] == hex[result[j] & 0xf]);
+    }
 
-        qcrypto_hmac_free(hmac);
+    qcrypto_hmac_free(hmac);
 
-        g_free(result);
-    }
+    g_free(result);
 }
 
-static void test_hmac_digest(void)
+static void test_hmac_digest(const void *opaque)
 {
-    size_t i;
+    QCryptoHashAlgo alg = GPOINTER_TO_INT(opaque);
+    QCryptoHmacTestData *data = &test_data[alg];
+    QCryptoHmac *hmac = NULL;
+    uint8_t *result = NULL;
+    const char *exp_output = NULL;
+    int ret;
 
-    for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
-        QCryptoHmacTestData *data = &test_data[i];
-        QCryptoHmac *hmac = NULL;
-        uint8_t *result = NULL;
-        const char *exp_output = NULL;
-        int ret;
+    exp_output = data->hex_digest;
 
-        if (!qcrypto_hmac_supports(data->alg)) {
-            return;
-        }
-
-        exp_output = data->hex_digest;
-
-        hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
-                                strlen(KEY), &error_fatal);
-        g_assert(hmac != NULL);
+    hmac = qcrypto_hmac_new(data->alg, (const uint8_t *)KEY,
+                            strlen(KEY), &error_fatal);
+    g_assert(hmac != NULL);
 
-        ret = qcrypto_hmac_digest(hmac, (const char *)INPUT_TEXT,
-                                  strlen(INPUT_TEXT), (char **)&result,
-                                  &error_fatal);
-        g_assert(ret == 0);
+    ret = qcrypto_hmac_digest(hmac, (const char *)INPUT_TEXT,
+                              strlen(INPUT_TEXT), (char **)&result,
+                              &error_fatal);
+    g_assert(ret == 0);
 
-        g_assert_cmpstr((const char *)result, ==, exp_output);
+    g_assert_cmpstr((const char *)result, ==, exp_output);
 
-        qcrypto_hmac_free(hmac);
+    qcrypto_hmac_free(hmac);
 
-        g_free(result);
-    }
+    g_free(result);
 }
 
 int main(int argc, char **argv)
 {
+    size_t i;
     g_test_init(&argc, &argv, NULL);
 
     g_assert(qcrypto_init(NULL) == 0);
 
-    g_test_add_func("/crypto/hmac/iov", test_hmac_iov);
-    g_test_add_func("/crypto/hmac/alloc", test_hmac_alloc);
-    g_test_add_func("/crypto/hmac/prealloc", test_hmac_prealloc);
-    g_test_add_func("/crypto/hmac/digest", test_hmac_digest);
+#define ADD_TEST(alg, prefix, meth) \
+    { \
+        g_autofree char *name = g_strdup_printf( \
+            "%s/%s", prefix, QCryptoHashAlgo_str(alg)); \
+        g_test_add_data_func(name, GINT_TO_POINTER(alg), meth); \
+    }
+
+    for (i = 0; i < QCRYPTO_HASH_ALGO__MAX ; i++) {
+        if (!qcrypto_hmac_supports(i)) {
+            continue;
+        }
+
+        ADD_TEST(i, "/crypto/hmac/iov", test_hmac_iov);
+        ADD_TEST(i, "/crypto/hmac/alloc", test_hmac_alloc);
+        ADD_TEST(i, "/crypto/hmac/prealloc", test_hmac_prealloc);
+        ADD_TEST(i, "/crypto/hmac/digest", test_hmac_digest);
+    }
 
     return g_test_run();
 }
-- 
2.56.0



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

* Re: [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash
  2026-11-02 12:23 [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash Daniel P. Berrangé
                   ` (3 preceding siblings ...)
  2026-11-02 12:23 ` [PATCH v2 4/4] tests/unit: register one test per hash/hmac algorithm Daniel P. Berrangé
@ 2026-11-12 18:00 ` Daniel P. Berrangé
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2026-11-12 18:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

ping: i'd like to get this fix into 11.2

On Mon, Nov 02, 2026 at 12:23:39PM +0000, Daniel P. Berrangé wrote:
> Note that patch 4 is best reviewed by applying to git and then using
> 'git show -b' to hide the huge whitespace indentation change.
> 
> Changed in v2:
> 
>  * Use array range initializer for setting default value
>  * Drop redundant countermeasures now we can gurantee a
>    correct default initializer value
>  * Drop SM3 conditions in tests
>  * Refactor tests to register one test per algorithm
> 
> Daniel P. Berrangé (4):
>   crypto: set default initializer hmac/hash arrays to -1
>   crypto: unconditionally report SM3 hash length
>   tests/unit: drop conditionals for SM3
>   tests/unit: register one test per hash/hmac algorithm
> 
>  crypto/hash-glib.c            |   3 +-
>  crypto/hash.c                 |   4 +-
>  crypto/hmac-glib.c            |   4 +-
>  tests/unit/test-crypto-hash.c | 325 +++++++++++++++-------------------
>  tests/unit/test-crypto-hmac.c | 272 +++++++++++++---------------
>  5 files changed, 275 insertions(+), 333 deletions(-)
> 
> -- 
> 2.56.0
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|



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

end of thread, other threads:[~2026-11-12 18:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-02 12:23 [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash Daniel P. Berrangé
2026-11-02 12:23 ` [PATCH v2 1/4] crypto: set default initializer hmac/hash arrays to -1 Daniel P. Berrangé
2026-11-02 12:23 ` [PATCH v2 2/4] crypto: unconditionally report SM3 hash length Daniel P. Berrangé
2026-11-02 12:23 ` [PATCH v2 3/4] tests/unit: drop conditionals for SM3 Daniel P. Berrangé
2026-11-02 12:23 ` [PATCH v2 4/4] tests/unit: register one test per hash/hmac algorithm Daniel P. Berrangé
2026-11-12 18:00 ` [PATCH v2 0/4] crypto: robustness fixes to handling of SM3 hash Daniel P. Berrangé

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.