[PULL 0/5] Misc crypto subsystem fixes

[PULL 0/5] Misc crypto subsystem fixes

[Daniel P. Berrangé]

The following changes since commit 609dd53df540edd72faee705205aceca9c42fea5:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20200506' into stag=
ing (2020-05-07 09:45:54 +0100)

are available in the Git repository at:

  https://github.com/berrange/qemu tags/qcrypto-next-pull-request

for you to fetch changes up to 6022e15d146d8bba9610a9d134afa4bc6e5d9275:

  crypto: extend hash benchmark to cover more algorithms (2020-05-07 12:52:51=
 +0100)

----------------------------------------------------------------
Misc crypto subsystem fixes

* Improve error message for large files when creating LUKS volumes
* Expand crypto hash benchmark coverage
* Misc code refactoring with no functional change

----------------------------------------------------------------

Alexey Krasikov (1):
  crypto/secret: fix inconsequential errors.

Chen Qun (1):
  crypto: Redundant type conversion for AES_KEY pointer

Daniel P. Berrang=C3=A9 (1):
  crypto: extend hash benchmark to cover more algorithms

Maxim Levitsky (1):
  block: luks: better error message when creating too large files

Tong Ho (1):
  crypto: fix getter of a QCryptoSecret's property

 block/crypto.c                | 25 ++++++++++--
 crypto/cipher-builtin.c       | 10 ++---
 crypto/secret.c               |  5 ++-
 tests/benchmark-crypto-hash.c | 73 ++++++++++++++++++++++++++++-------
 4 files changed, 87 insertions(+), 26 deletions(-)

--=20
2.26.2

[PULL 1/5] crypto: fix getter of a QCryptoSecret's property

[Daniel P. Berrangé]

From: Tong Ho <tong.ho@xilinx.com>

This fixes the condition-check done by the "loaded" property
getter, such that the property returns true even when the
secret is loaded by the 'file' option.

Signed-off-by: Tong Ho <tong.ho@xilinx.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/secret.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/crypto/secret.c b/crypto/secret.c
index 1cf0ad0ce8..5fb6bbe59c 100644
--- a/crypto/secret.c
+++ b/crypto/secret.c
@@ -221,6 +221,7 @@ qcrypto_secret_prop_set_loaded(Object *obj,
         secret->rawlen = inputlen;
     } else {
         g_free(secret->rawdata);
+        secret->rawdata = NULL;
         secret->rawlen = 0;
     }
 }
@@ -231,7 +232,7 @@ qcrypto_secret_prop_get_loaded(Object *obj,
                                Error **errp G_GNUC_UNUSED)
 {
     QCryptoSecret *secret = QCRYPTO_SECRET(obj);
-    return secret->data != NULL;
+    return secret->rawdata != NULL;
 }
 
 
-- 
2.26.2

[PULL 2/5] crypto/secret: fix inconsequential errors.

[Daniel P. Berrangé]

From: Alexey Krasikov <alex-krasikov@yandex-team.ru>

Change condition from QCRYPTO_SECRET_FORMAT_RAW
to QCRYPTO_SECRET_FORMAT_BASE64 in if-operator, because
this is potential error if you add another format value.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alexey Krasikov <alex-krasikov@yandex-team.ru>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/secret.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/secret.c b/crypto/secret.c
index 5fb6bbe59c..a846a3c87c 100644
--- a/crypto/secret.c
+++ b/crypto/secret.c
@@ -204,7 +204,7 @@ qcrypto_secret_prop_set_loaded(Object *obj,
             input = output;
             inputlen = outputlen;
         } else {
-            if (secret->format != QCRYPTO_SECRET_FORMAT_RAW) {
+            if (secret->format == QCRYPTO_SECRET_FORMAT_BASE64) {
                 qcrypto_secret_decode(input, inputlen,
                                       &output, &outputlen, &local_err);
                 g_free(input);
-- 
2.26.2

[PULL 3/5] crypto: Redundant type conversion for AES_KEY pointer

[Daniel P. Berrangé]

From: Chen Qun <kuhn.chenqun@huawei.com>

We can delete the redundant type conversion if
we set the the AES_KEY parameter with 'const' in
qcrypto_cipher_aes_ecb_(en|de)crypt() function.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/cipher-builtin.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/crypto/cipher-builtin.c b/crypto/cipher-builtin.c
index bf8413e71a..35cf7820d9 100644
--- a/crypto/cipher-builtin.c
+++ b/crypto/cipher-builtin.c
@@ -74,7 +74,7 @@ static void qcrypto_cipher_free_aes(QCryptoCipher *cipher)
 }
 
 
-static void qcrypto_cipher_aes_ecb_encrypt(AES_KEY *key,
+static void qcrypto_cipher_aes_ecb_encrypt(const AES_KEY *key,
                                            const void *in,
                                            void *out,
                                            size_t len)
@@ -100,7 +100,7 @@ static void qcrypto_cipher_aes_ecb_encrypt(AES_KEY *key,
 }
 
 
-static void qcrypto_cipher_aes_ecb_decrypt(AES_KEY *key,
+static void qcrypto_cipher_aes_ecb_decrypt(const AES_KEY *key,
                                            const void *in,
                                            void *out,
                                            size_t len)
@@ -133,8 +133,7 @@ static void qcrypto_cipher_aes_xts_encrypt(const void *ctx,
 {
     const QCryptoCipherBuiltinAESContext *aesctx = ctx;
 
-    qcrypto_cipher_aes_ecb_encrypt((AES_KEY *)&aesctx->enc,
-                                   src, dst, length);
+    qcrypto_cipher_aes_ecb_encrypt(&aesctx->enc, src, dst, length);
 }
 
 
@@ -145,8 +144,7 @@ static void qcrypto_cipher_aes_xts_decrypt(const void *ctx,
 {
     const QCryptoCipherBuiltinAESContext *aesctx = ctx;
 
-    qcrypto_cipher_aes_ecb_decrypt((AES_KEY *)&aesctx->dec,
-                                   src, dst, length);
+    qcrypto_cipher_aes_ecb_decrypt(&aesctx->dec, src, dst, length);
 }
 
 
-- 
2.26.2

[PULL 4/5] block: luks: better error message when creating too large files

[Daniel P. Berrangé]

From: Maxim Levitsky <mlevitsk@redhat.com>

Currently if you attampt to create too large file with luks you
get the following error message:

Formatting 'test.luks', fmt=luks size=17592186044416 key-secret=sec0
qemu-img: test.luks: Could not resize file: File too large

While for raw format the error message is
qemu-img: test.img: The image size is too large for file format 'raw'

The reason for this is that qemu-img checks for errono of the failure,
and presents the later error when it is -EFBIG

However crypto generic code 'swallows' the errno and replaces it
with -EIO.

As an attempt to make it better, we can make luks driver,
detect -EFBIG and in this case present a better error message,
which is what this patch does

The new error message is:

qemu-img: error creating test.luks: The requested file size is too large

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534898
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 block/crypto.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index ca44dae4f7..6b21d6bf6c 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -104,18 +104,35 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
                                       Error **errp)
 {
     struct BlockCryptoCreateData *data = opaque;
+    Error *local_error = NULL;
+    int ret;
 
     if (data->size > INT64_MAX || headerlen > INT64_MAX - data->size) {
-        error_setg(errp, "The requested file size is too large");
-        return -EFBIG;
+        ret = -EFBIG;
+        goto error;
     }
 
     /* User provided size should reflect amount of space made
      * available to the guest, so we must take account of that
      * which will be used by the crypto header
      */
-    return blk_truncate(data->blk, data->size + headerlen, false,
-                        data->prealloc, 0, errp);
+    ret = blk_truncate(data->blk, data->size + headerlen, false,
+                       data->prealloc, 0, &local_error);
+
+    if (ret >= 0) {
+        return ret;
+    }
+
+error:
+    if (ret == -EFBIG) {
+        /* Replace the error message with a better one */
+        error_free(local_error);
+        error_setg(errp, "The requested file size is too large");
+    } else {
+        error_propagate(errp, local_error);
+    }
+
+    return ret;
 }
 
 
-- 
2.26.2

[PULL 5/5] crypto: extend hash benchmark to cover more algorithms

[Daniel P. Berrangé]

Extend the hash benchmark so that it can validate all algorithms
supported by QEMU instead of being limited to sha256.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/benchmark-crypto-hash.c | 73 ++++++++++++++++++++++++++++-------
 1 file changed, 59 insertions(+), 14 deletions(-)

diff --git a/tests/benchmark-crypto-hash.c b/tests/benchmark-crypto-hash.c
index 7f659f7323..d16837d00a 100644
--- a/tests/benchmark-crypto-hash.c
+++ b/tests/benchmark-crypto-hash.c
@@ -15,9 +15,14 @@
 #include "crypto/init.h"
 #include "crypto/hash.h"
 
+typedef struct QCryptoHashOpts {
+    size_t chunk_size;
+    QCryptoHashAlgorithm alg;
+} QCryptoHashOpts;
+
 static void test_hash_speed(const void *opaque)
 {
-    size_t chunk_size = (size_t)opaque;
+    const QCryptoHashOpts *opts = opaque;
     uint8_t *in = NULL, *out = NULL;
     size_t out_len = 0;
     const size_t total = 2 * GiB;
@@ -25,26 +30,24 @@ static void test_hash_speed(const void *opaque)
     struct iovec iov;
     int ret;
 
-    in = g_new0(uint8_t, chunk_size);
-    memset(in, g_test_rand_int(), chunk_size);
+    in = g_new0(uint8_t, opts->chunk_size);
+    memset(in, g_test_rand_int(), opts->chunk_size);
 
     iov.iov_base = (char *)in;
-    iov.iov_len = chunk_size;
+    iov.iov_len = opts->chunk_size;
 
     g_test_timer_start();
     remain = total;
     while (remain) {
-        ret = qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256,
+        ret = qcrypto_hash_bytesv(opts->alg,
                                   &iov, 1, &out, &out_len,
                                   NULL);
         g_assert(ret == 0);
 
-        remain -= chunk_size;
+        remain -= opts->chunk_size;
     }
     g_test_timer_elapsed();
 
-    g_print("sha256: ");
-    g_print("Hash %zu GB chunk size %zu bytes ", total / GiB, chunk_size);
     g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last());
 
     g_free(out);
@@ -53,17 +56,59 @@ static void test_hash_speed(const void *opaque)
 
 int main(int argc, char **argv)
 {
-    size_t i;
     char name[64];
 
     g_test_init(&argc, &argv, NULL);
     g_assert(qcrypto_init(NULL) == 0);
 
-    for (i = 512; i <= 64 * KiB; i *= 2) {
-        memset(name, 0 , sizeof(name));
-        snprintf(name, sizeof(name), "/crypto/hash/speed-%zu", i);
-        g_test_add_data_func(name, (void *)i, test_hash_speed);
-    }
+#define TEST_ONE(a, c)                                          \
+    QCryptoHashOpts opts ## a ## c = {                          \
+        .alg = QCRYPTO_HASH_ALG_ ## a, .chunk_size = c,         \
+    };                                                          \
+    memset(name, 0 , sizeof(name));                             \
+    snprintf(name, sizeof(name),                                \
+             "/crypto/benchmark/hash/%s/bufsize-%d",            \
+             QCryptoHashAlgorithm_str(QCRYPTO_HASH_ALG_ ## a),  \
+             c);                                                \
+    if (qcrypto_hash_supports(QCRYPTO_HASH_ALG_ ## a))          \
+        g_test_add_data_func(name,                              \
+                             &opts ## a ## c,                   \
+                             test_hash_speed);
+
+    TEST_ONE(MD5, 512);
+    TEST_ONE(MD5, 1024);
+    TEST_ONE(MD5, 4096);
+    TEST_ONE(MD5, 16384);
+
+    TEST_ONE(SHA1, 512);
+    TEST_ONE(SHA1, 1024);
+    TEST_ONE(SHA1, 4096);
+    TEST_ONE(SHA1, 16384);
+
+    TEST_ONE(SHA224, 512);
+    TEST_ONE(SHA224, 1024);
+    TEST_ONE(SHA224, 4096);
+    TEST_ONE(SHA224, 16384);
+
+    TEST_ONE(SHA384, 512);
+    TEST_ONE(SHA384, 1024);
+    TEST_ONE(SHA384, 4096);
+    TEST_ONE(SHA384, 16384);
+
+    TEST_ONE(SHA256, 512);
+    TEST_ONE(SHA256, 1024);
+    TEST_ONE(SHA256, 4096);
+    TEST_ONE(SHA256, 16384);
+
+    TEST_ONE(SHA512, 512);
+    TEST_ONE(SHA512, 1024);
+    TEST_ONE(SHA512, 4096);
+    TEST_ONE(SHA512, 16384);
+
+    TEST_ONE(RIPEMD160, 512);
+    TEST_ONE(RIPEMD160, 1024);
+    TEST_ONE(RIPEMD160, 4096);
+    TEST_ONE(RIPEMD160, 16384);
 
     return g_test_run();
 }
-- 
2.26.2

Re: [PULL 0/5] Misc crypto subsystem fixes

[Peter Maydell]

On Thu, 7 May 2020 at 12:59, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The following changes since commit 609dd53df540edd72faee705205aceca9c42fea5:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20200506' into stag=
> ing (2020-05-07 09:45:54 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/berrange/qemu tags/qcrypto-next-pull-request
>
> for you to fetch changes up to 6022e15d146d8bba9610a9d134afa4bc6e5d9275:
>
>   crypto: extend hash benchmark to cover more algorithms (2020-05-07 12:52:51=
>  +0100)
>
> ----------------------------------------------------------------
> Misc crypto subsystem fixes
>
> * Improve error message for large files when creating LUKS volumes
> * Expand crypto hash benchmark coverage
> * Misc code refactoring with no functional change
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM