Re: [PATCH v7 0/9] Introduce akcipher service for virtio-crypto

[Daniel P . Berrangé]

I've sent a pull request containing all the crypto/ changes,
as that covers stuff I maintain. ie patches 2-8

Patches 1 and 9, I'll leave for MST to review & queue since the
virtual hardware is not my area of knowledge.

On Wed, May 25, 2022 at 05:01:09PM +0800, Lei He wrote:
> v6 -> v7:
> - Fix serval build errors for some specific platforms/configurations.
> - Use '%zu' instead of '%lu' for size_t parameters.
> - AkCipher-gcrypt: avoid setting wrong error messages when parsing RSA
>   keys.
> - AkCipher-benchmark: process constant amount of sign/verify instead
>  of running sign/verify for a constant duration.
> 
> v5 -> v6:
> - Fix build errors and codestyles.
> - Add parameter 'Error **errp' for qcrypto_akcipher_rsakey_parse.
> - Report more detailed errors.
> - Fix buffer length check and return values of akcipher-nettle, allows caller 
> to
>  pass a buffer with larger size than actual needed.
> 
> A million thanks to Daniel!
> 
> v4 -> v5:
> - Move QCryptoAkCipher into akcipherpriv.h, and modify the related comments.
> - Rename asn1_decoder.c to der.c.
> - Code style fix: use 'cleanup' & 'error' lables.
> - Allow autoptr type to auto-free.
> - Add test cases for rsakey to handle DER error.
> - Other minor fixes.
> 
> v3 -> v4:
> - Coding style fix: Akcipher -> AkCipher, struct XXX -> XXX, Rsa -> RSA,
> XXX-alg -> XXX-algo.
> - Change version info in qapi/crypto.json, from 7.0 -> 7.1.
> - Remove ecdsa from qapi/crypto.json, it would be introduced with the 
> implemetion later.
> - Use QCryptoHashAlgothrim instead of QCryptoRSAHashAlgorithm(removed) in 
> qapi/crypto.json.
> - Rename arguments of qcrypto_akcipher_XXX to keep aligned with 
> qcrypto_cipher_XXX(dec/enc/sign/vefiry -> in/out/in2), and add 
> qcrypto_akcipher_max_XXX APIs.
> - Add new API: qcrypto_akcipher_supports.
> - Change the return value of qcrypto_akcipher_enc/dec/sign, these functions 
> return the actual length of result.
> - Separate ASN.1 source code and test case clean.
> - Disable RSA raw encoding for akcipher-nettle.
> - Separate RSA key parser into rsakey.{hc}, and implememts it with 
> builtin-asn1-decoder and nettle respectivly.
> - Implement RSA(pkcs1 and raw encoding) algorithm by gcrypt. This has higher 
> priority than nettle.
> - For some akcipher operations(eg, decryption of pkcs1pad(rsa)), the length 
> of returned result maybe less than the dst buffer size, return the actual 
> length of result instead of the buffer length to the guest side. (in function 
> virtio_crypto_akcipher_input_data_helper)
> - Other minor changes.
> 
> Thanks to Daniel!
> 
> Eric pointed out this missing part of use case, send it here again.
> 
> In our plan, the feature is designed for HTTPS offloading case and other 
> applications which use kernel RSA/ecdsa by keyctl syscall. The full picture 
> shows bellow:
> 
> 
>  Nginx/openssl[1] ... Apps
> Guest   -
>   virtio-crypto driver[2]
> -
>   virtio-crypto backend[3]
> Host-
>  /  |  \
>  builtin[4]   vhost keyctl[5] ...
> 
> 
> [1] User applications can offload RSA calculation to kernel by keyctl 
> syscall. There is no keyctl engine in openssl currently, we developed a 
> engine and tried to contribute it to openssl upstream, but openssl 1.x does 
> not accept new feature. Link:
>https://github.com/openssl/openssl/pull/16689
> 
> This branch is available and maintained by Lei 
>https://github.com/TousakaRin/openssl/tree/OpenSSL_1_1_1-kctl_engine
> 
> We tested nginx(change config file only) with openssl keyctl engine, it works 
> fine.
> 
> [2] virtio-crypto driver is used to communicate with host side, send requests 
> to host side to do asymmetric calculation.
>https://lkml.org/lkml/2022/3/1/1425
> 
> [3] virtio-crypto backend handles requests from guest side, and forwards 
> request to crypto backend driver of QEMU.
> 
> [4] Currently RSA is supported only in builtin driver. This driver is 
> supposed to test the full feature without other software(Ex vhost process) 
> and hardware dependence. ecdsa is introduced into qapi type without 
> implementation, this may be implemented in Q3-2022 or later. If ecdsa type 
> definition should be added with the implementation together, I'll remove this 
> in next version.
> 
> [5] keyctl backend is in development, we will post this feature in Q2-2022. 
> keyctl backend can use hardware acceleration(Ex, Intel QAT).
> 
> Setup the full environment, tested with Intel QAT on host side, the QPS of 
> HTTPS increase to ~200% in a guest.
> 
> VS PCI passthrough: the most important benefit of this solution makes the VM 
> migratable.
> 
> v2 -> v3:
> - Introduce akcipher types to qapi
> - Add test/benchmark suite for akcipher class
> - Seperate 'virtio_crypto: Support virtio crypto asym operation' into:
>  - 

Re: [PATCH v6 4/9] crypto: add ASN.1 DER decoder

[Daniel P . Berrangé]

On Sat, May 14, 2022 at 08:54:59AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Add an ANS.1 DER decoder which is used to parse asymmetric
> cipher keys
> 
> Signed-off-by: zhenwei pi 
> Signed-off-by: lei he 
> ---
>  crypto/der.c | 189 +++
>  crypto/der.h |  81 ++
>  crypto/meson.build   |   1 +
>  tests/unit/meson.build   |   1 +
>  tests/unit/test-crypto-der.c | 290 +++
>  5 files changed, 562 insertions(+)
>  create mode 100644 crypto/der.c
>  create mode 100644 crypto/der.h
>  create mode 100644 tests/unit/test-crypto-der.c
> 

> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index 264f2bc0c8..a8af85128d 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -47,6 +47,7 @@ tests = {
>'ptimer-test': ['ptimer-test-stubs.c', meson.project_source_root() / 
> 'hw/core/ptimer.c'],
>'test-qapi-util': [],
>'test-smp-parse': [qom, meson.project_source_root() / 
> 'hw/core/machine-smp.c'],
> +  'test-crypto-der': [crypto],
>  }

This needs to be moved to later in this file where the other
test-crypto- rules are, otherwise it fails to build
on a configuration  --without-system --without-tools.


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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v6 7/9] test/crypto: Add test suite for crypto akcipher

[Daniel P . Berrangé]

On Sat, May 14, 2022 at 08:55:02AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Add unit test and benchmark test for crypto akcipher.
> 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> Reviewed-by: Daniel P. Berrangé 
> ---
>  tests/bench/benchmark-crypto-akcipher.c | 157 ++
>  tests/bench/meson.build |   1 +
>  tests/bench/test_akcipher_keys.inc  | 537 ++
>  tests/unit/meson.build  |   1 +
>  tests/unit/test-crypto-akcipher.c   | 711 
>  5 files changed, 1407 insertions(+)
>  create mode 100644 tests/bench/benchmark-crypto-akcipher.c
>  create mode 100644 tests/bench/test_akcipher_keys.inc
>  create mode 100644 tests/unit/test-crypto-akcipher.c
> 
> diff --git a/tests/bench/benchmark-crypto-akcipher.c 
> b/tests/bench/benchmark-crypto-akcipher.c
> new file mode 100644
> index 00..c6c80c0be1
> --- /dev/null
> +++ b/tests/bench/benchmark-crypto-akcipher.c
> @@ -0,0 +1,157 @@
> +/*
> + * QEMU Crypto akcipher speed benchmark
> + *
> + * Copyright (c) 2022 Bytedance
> + *
> + * Authors:
> + *lei he 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version.  See the COPYING file in the
> + * top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "crypto/init.h"
> +#include "crypto/akcipher.h"
> +#include "standard-headers/linux/virtio_crypto.h"
> +
> +#include "test_akcipher_keys.inc"
> +
> +static bool keep_running;
> +
> +static void alarm_handler(int sig)
> +{
> +keep_running = false;
> +}
> +
> +static QCryptoAkCipher *create_rsa_akcipher(const uint8_t *priv_key,
> +size_t keylen,
> +QCryptoRSAPaddingAlgorithm 
> padding,
> +QCryptoHashAlgorithm hash)
> +{
> +QCryptoAkCipherOptions opt;
> +QCryptoAkCipher *rsa;
> +
> +opt.alg = QCRYPTO_AKCIPHER_ALG_RSA;
> +opt.u.rsa.padding_alg = padding;
> +opt.u.rsa.hash_alg = hash;
> +rsa = qcrypto_akcipher_new(, QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE,
> +   priv_key, keylen, _abort);
> +return rsa;
> +}
> +
> +static void test_rsa_speed(const uint8_t *priv_key, size_t keylen,
> +   size_t key_size)
> +{
> +#define BYTE 8
> +#define SHA1_DGST_LEN 20
> +#define DURATION_SECONDS 10
> +#define PADDING QCRYPTO_RSA_PADDING_ALG_PKCS1
> +#define HASH QCRYPTO_HASH_ALG_SHA1
> +
> +g_autoptr(QCryptoAkCipher) rsa =
> +create_rsa_akcipher(priv_key, keylen, PADDING, HASH);
> +g_autofree uint8_t *dgst = NULL;
> +g_autofree uint8_t *signature = NULL;
> +size_t count;
> +
> +dgst = g_new0(uint8_t, SHA1_DGST_LEN);
> +memset(dgst, g_test_rand_int(), SHA1_DGST_LEN);
> +signature = g_new0(uint8_t, key_size / BYTE);
> +
> +g_test_message("benchmark rsa%lu (%s-%s) sign in %d seconds", key_size,
> +   QCryptoRSAPaddingAlgorithm_str(PADDING),
> +   QCryptoHashAlgorithm_str(HASH),
> +   DURATION_SECONDS);

Needs to be '%zu' here and several other places in this file for any
parameter which is 'size_t'.

> +alarm(DURATION_SECONDS);
> +g_test_timer_start();
> +for (keep_running = true, count = 0; keep_running; ++count) {
> +g_assert(qcrypto_akcipher_sign(rsa, dgst, SHA1_DGST_LEN,
> +   signature, key_size / BYTE,
> +   _abort) > 0);
> +}
> +g_test_timer_elapsed();
> +g_test_message("rsa%lu (%s-%s) sign %lu times in %.2f seconds,"
> +   " %.2f times/sec ",
> +   key_size,  QCryptoRSAPaddingAlgorithm_str(PADDING),
> +   QCryptoHashAlgorithm_str(HASH),
> +   count, g_test_timer_last(),
> +   (double)count / g_test_timer_last());
> +
> +g_test_message("benchmark rsa%lu (%s-%s) verify in %d seconds", key_size,
> +   QCryptoRSAPaddingAlgorithm_str(PADDING),
> +   QCryptoHashAlgorithm_str(HASH),
> +   DURATION_SECONDS);
> +alarm(DURATION_SECONDS);
> +g_test_timer_start();
> +for (keep_running = true, count = 0; keep_running; ++count) {
> +g_assert(qcrypto_akcipher_verify(rsa, signature, key_size / BYTE,
> + dgst, SHA1_DGST_LEN,
> + _abort) == 0);
> +

Re: [PATCH v6 5/9] crypto: Implement RSA algorithm by hogweed

[Daniel P . Berrangé]

On Sat, May 14, 2022 at 08:55:00AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Implement RSA algorithm by hogweed from nettle. Thus QEMU supports
> a 'real' RSA backend to handle request from guest side. It's
> important to test RSA offload case without OS & hardware requirement.
> 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  crypto/akcipher-nettle.c.inc | 451 +++
>  crypto/akcipher.c|   4 +
>  crypto/meson.build   |   4 +
>  crypto/rsakey-builtin.c.inc  | 200 
>  crypto/rsakey-nettle.c.inc   | 158 
>  crypto/rsakey.c  |  44 
>  crypto/rsakey.h  |  94 
>  meson.build  |  11 +
>  8 files changed, 966 insertions(+)
>  create mode 100644 crypto/akcipher-nettle.c.inc
>  create mode 100644 crypto/rsakey-builtin.c.inc
>  create mode 100644 crypto/rsakey-nettle.c.inc
>  create mode 100644 crypto/rsakey.c
>  create mode 100644 crypto/rsakey.h


> diff --git a/crypto/rsakey.h b/crypto/rsakey.h
> new file mode 100644
> index 00..17bb22333d
> --- /dev/null
> +++ b/crypto/rsakey.h
> @@ -0,0 +1,94 @@
> +
> +#ifndef QCRYPTO_RSAKEY_H
> +#define QCRYPTO_RSAKEY_H
> +
> +#include 

This should be removed -it isn't needed and breaks build on
without-nettle build configurations.

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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v6 5/9] crypto: Implement RSA algorithm by hogweed

[Daniel P . Berrangé]

On Sat, May 14, 2022 at 08:55:00AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Implement RSA algorithm by hogweed from nettle. Thus QEMU supports
> a 'real' RSA backend to handle request from guest side. It's
> important to test RSA offload case without OS & hardware requirement.
> 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  crypto/akcipher-nettle.c.inc | 451 +++
>  crypto/akcipher.c|   4 +
>  crypto/meson.build   |   4 +
>  crypto/rsakey-builtin.c.inc  | 200 
>  crypto/rsakey-nettle.c.inc   | 158 
>  crypto/rsakey.c  |  44 
>  crypto/rsakey.h  |  94 
>  meson.build  |  11 +
>  8 files changed, 966 insertions(+)
>  create mode 100644 crypto/akcipher-nettle.c.inc
>  create mode 100644 crypto/rsakey-builtin.c.inc
>  create mode 100644 crypto/rsakey-nettle.c.inc
>  create mode 100644 crypto/rsakey.c
>  create mode 100644 crypto/rsakey.h
> 
> diff --git a/crypto/akcipher-nettle.c.inc b/crypto/akcipher-nettle.c.inc
> new file mode 100644
> index 00..0796bddcaa
> --- /dev/null
> +++ b/crypto/akcipher-nettle.c.inc

> +static int qcrypto_nettle_rsa_encrypt(QCryptoAkCipher *akcipher,
> +  const void *data, size_t data_len,
> +  void *enc, size_t enc_len,
> +  Error **errp)
> +{
> +
> +QCryptoNettleRSA *rsa = (QCryptoNettleRSA *)akcipher;
> +mpz_t c;
> +int ret = -1;
> +
> +if (data_len > rsa->pub.size) {
> +error_setg(errp, "Plaintext length should be less than key size: 
> %lu",
> +   rsa->pub.size);
> +return ret;
> +}

This needs to include both the good & bad values. I'm going to make
the following changes to error messages:

ie

+error_setg(errp, "Plaintext length %zu is greater than key size: %lu"
+   data_len, rsa->pub.size);
 return ret;
 }


But also the '%lu' needs to change to '%zu' because the rsa->pub.size
parameter is 'size_t'.  %lu doesn't match size_t on 32-bit hosts.

The same issues appear in several other error messages through this
file

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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v6 6/9] crypto: Implement RSA algorithm by gcrypt

[Daniel P . Berrangé]

On Sat, May 14, 2022 at 08:55:01AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Added gcryt implementation of RSA algorithm, RSA algorithm
> implemented by gcrypt has a higher priority than nettle because
> it supports raw padding.
> 
> Signed-off-by: zhenwei pi 
> Signed-off-by: lei he 
> ---
>  crypto/akcipher-gcrypt.c.inc | 597 +++
>  crypto/akcipher.c|   4 +-
>  2 files changed, 600 insertions(+), 1 deletion(-)
>  create mode 100644 crypto/akcipher-gcrypt.c.inc
> 
> diff --git a/crypto/akcipher-gcrypt.c.inc b/crypto/akcipher-gcrypt.c.inc
> new file mode 100644
> index 00..6c5daa301e
> --- /dev/null
> +++ b/crypto/akcipher-gcrypt.c.inc
> @@ -0,0 +1,597 @@
> +/*
> + * QEMU Crypto akcipher algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: lei he 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#include 
> +static QCryptoGcryptRSA *qcrypto_gcrypt_rsa_new(
> +const QCryptoAkCipherOptionsRSA *opt,
> +QCryptoAkCipherKeyType type,
> +const uint8_t *key, size_t keylen,
> +Error **errp)
> +{
> +QCryptoGcryptRSA *rsa = g_new0(QCryptoGcryptRSA, 1);
> +rsa->padding_alg = opt->padding_alg;
> +rsa->hash_alg = opt->hash_alg;
> +rsa->akcipher.driver = _rsa;
> +
> +switch (type) {
> +case QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE:
> +if (qcrypto_gcrypt_parse_rsa_private_key(rsa, key, keylen, errp) != 
> 0) {
> +error_setg(errp, "Failed to parse rsa private key");

Not need now, since qcrypto_gcrypt_parse_rsa_private_key reports the
real error message.

> +goto error;
> +}
> +break;
> +
> +case QCRYPTO_AKCIPHER_KEY_TYPE_PUBLIC:
> +if (qcrypto_gcrypt_parse_rsa_public_key(rsa, key, keylen, errp) != 
> 0) {
> +error_setg(errp, "Failed to parse rsa public rsa key");

Likewise not needed.

> +goto error;
> +}
> +break;
> +
> +default:
> +error_setg(errp, "Unknown akcipher key type %d", type);
> +goto error;
> +}
> +
> +return rsa;
> +
> +error:
> +qcrypto_gcrypt_rsa_free((QCryptoAkCipher *)rsa);
> +return NULL;
> +}

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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [External] [PATCH v5 5/9] crypto: Implement RSA algorithm by hogweed

[Daniel P . Berrangé]

On Fri, May 13, 2022 at 08:26:14PM +0800, 何磊 wrote:
> 
> 
> > On May 13, 2022, at 6:55 PM, Daniel P. Berrangé  wrote:
> > 
> > On Thu, Apr 28, 2022 at 09:59:39PM +0800, zhenwei pi wrote:
> >> From: Lei He 
> >> 
> >> Implement RSA algorithm by hogweed from nettle. Thus QEMU supports
> >> a 'real' RSA backend to handle request from guest side. It's
> >> important to test RSA offload case without OS & hardware requirement.
> >> 
> >> Signed-off-by: lei he 
> >> Signed-off-by: zhenwei pi 
> >> ---
> >> crypto/akcipher-nettle.c.inc | 432 +++
> >> crypto/akcipher.c|   4 +
> >> crypto/meson.build   |   4 +
> >> crypto/rsakey-builtin.c.inc  | 209 +
> >> crypto/rsakey-nettle.c.inc   | 154 +
> >> crypto/rsakey.c  |  44 
> >> crypto/rsakey.h  |  94 
> >> meson.build  |  11 +
> >> 8 files changed, 952 insertions(+)
> >> create mode 100644 crypto/akcipher-nettle.c.inc
> >> create mode 100644 crypto/rsakey-builtin.c.inc
> >> create mode 100644 crypto/rsakey-nettle.c.inc
> >> create mode 100644 crypto/rsakey.c
> >> create mode 100644 crypto/rsakey.h


> >> +static int qcrypto_nettle_rsa_decrypt(QCryptoAkCipher *akcipher,
> >> +  const void *enc, size_t enc_len,
> >> +  void *data, size_t data_len,
> >> +  Error **errp)
> >> +{
> >> +QCryptoNettleRSA *rsa = (QCryptoNettleRSA *)akcipher;
> >> +mpz_t c;
> >> +int ret = -1;
> >> +if (enc_len > rsa->priv.size) {
> >> +error_setg(errp, "Invalid buffer size");
> >> +return ret;
> >> +}
> > 
> > Again please report the invalid & expected sizes in the message
> > 
> > We don't need to validate 'data_len' in the decrypt case,
> > as you did in encrypt ?
> 
> In the decrypt case, it is difficult (and unnecessary) to check 'data_len' 
> before 
> we completing the decryption action. If the plaintext buffer is too small, 
> following ‘rsa_decrypt’ will return an error, and it should be valid to pass 
> a very 
> large buffer.
> 
> According to the pkcs#1 stardard, the length of ciphertext should always equal
> to key size, and the length of plaintext can be any value in range [1, 
> key_size - 11]:
> 
> https://datatracker.ietf.org/doc/html/rfc2437#section-7.2

Ok, thanks for explaining.


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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v5 6/9] crypto: Implement RSA algorithm by gcrypt

[Daniel P . Berrangé]

On Thu, Apr 28, 2022 at 09:59:40PM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Added gcryt implementation of RSA algorithm, RSA algorithm
> implemented by gcrypt has a higher priority than nettle because
> it supports raw padding.
> 
> Signed-off-by: lei he 
> ---
>  crypto/akcipher-gcrypt.c.inc | 520 +++
>  crypto/akcipher.c|   4 +-
>  2 files changed, 523 insertions(+), 1 deletion(-)
>  create mode 100644 crypto/akcipher-gcrypt.c.inc
> 
> diff --git a/crypto/akcipher-gcrypt.c.inc b/crypto/akcipher-gcrypt.c.inc
> new file mode 100644
> index 00..32ff502f71
> --- /dev/null
> +++ b/crypto/akcipher-gcrypt.c.inc
> @@ -0,0 +1,520 @@
> +/*
> + * QEMU Crypto akcipher algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: lei he 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#include 
> +
> +#include "qemu/osdep.h"
> +#include "qemu/host-utils.h"
> +#include "crypto/akcipher.h"
> +#include "crypto/random.h"
> +#include "qapi/error.h"
> +#include "sysemu/cryptodev.h"
> +#include "rsakey.h"
> +
> +typedef struct QCryptoGcryptRSA {
> +QCryptoAkCipher akcipher;
> +gcry_sexp_t key;
> +QCryptoRSAPaddingAlgorithm padding_alg;
> +QCryptoHashAlgorithm hash_alg;
> +} QCryptoGcryptRSA;
> +
> +static void qcrypto_gcrypt_rsa_free(QCryptoAkCipher *akcipher)
> +{
> +QCryptoGcryptRSA *rsa = (QCryptoGcryptRSA *)akcipher;
> +if (!rsa) {
> +return;
> +}
> +
> +gcry_sexp_release(rsa->key);
> +g_free(rsa);
> +}
> +
> +static QCryptoGcryptRSA *qcrypto_gcrypt_rsa_new(
> +const QCryptoAkCipherOptionsRSA *opt,
> +QCryptoAkCipherKeyType type,
> +const uint8_t *key,  size_t keylen,
> +Error **errp);
> +
> +QCryptoAkCipher *qcrypto_akcipher_new(const QCryptoAkCipherOptions *opts,
> +  QCryptoAkCipherKeyType type,
> +  const uint8_t *key, size_t keylen,
> +  Error **errp)
> +{
> +switch (opts->alg) {
> +case QCRYPTO_AKCIPHER_ALG_RSA:
> +return (QCryptoAkCipher *)qcrypto_gcrypt_rsa_new(
> +>u.rsa, type, key, keylen, errp);
> +
> +default:
> +error_setg(errp, "Unsupported algorithm: %u", opts->alg);
> +return NULL;
> +}
> +
> +return NULL;
> +}
> +
> +static void qcrypto_gcrypt_set_rsa_size(QCryptoAkCipher *akcipher, 
> gcry_mpi_t n)
> +{
> +size_t key_size = (gcry_mpi_get_nbits(n) + 7) / 8;
> +akcipher->max_plaintext_len = key_size;
> +akcipher->max_ciphertext_len = key_size;
> +akcipher->max_dgst_len = key_size;
> +akcipher->max_signature_len = key_size;
> +}
> +
> +static int qcrypto_gcrypt_parse_rsa_private_key(
> +QCryptoGcryptRSA *rsa,
> +const uint8_t *key, size_t keylen)
> +{
> +g_autoptr(QCryptoAkCipherRSAKey) rsa_key = qcrypto_akcipher_rsakey_parse(
> +QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE, key, keylen);
> +gcry_mpi_t n = NULL, e = NULL, d = NULL, p = NULL, q = NULL, u = NULL;
> +bool compute_mul_inv = false;
> +int ret = -1;
> +gcry_error_t err;
> +
> +if (!rsa_key) {
> +return ret;
> +}

If qcrypto_akcipher_rsakey_parse can fail, we need to get a
'Error **errp' in/out of it

> +
> +err = gcry_mpi_scan(, GCRYMPI_FMT_STD,
> +rsa_key->n.data, rsa_key->n.len, NULL);
> +if (gcry_err_code(err) != 0) {
> +goto cleanup;
> +}

Please add an 'Error **errp' parameter to this method, and
populate it with an error message that includes the output
of gcry_

> +
> +err = gcry_mpi_scan(, GCRYMPI_FMT_STD,
> +rsa_key->e.data, rsa_key->e.len, NULL);
> +if (gcry_err_code(err) != 0) {
> +goto cleanup;
> +}
> +
> +err = gcry_mpi_scan(, GCRYMPI_FMT_STD,
> +rsa_key->d.data, rsa_key->d.len, NULL);
> +if (gcry_err_code(err) != 0) {
> +goto cleanup;
> +}
> +
> +err = gcry_mpi_scan(, GCRYMPI_FMT_STD,
> +rsa_key->p.data, rsa_key->p.len, NULL);
> +if (gcry_err_code(err) != 0) {
> +goto cleanup;
> +}
> +
> +err = gcry_mpi_scan(, GCRYMPI_FMT_STD,
> +rsa_key->q.data, rsa_key->q.len, NULL);
> +if (gcry_err_code(err) != 0) {
> +

Re: [PATCH v5 5/9] crypto: Implement RSA algorithm by hogweed

[Daniel P . Berrangé]

On Thu, Apr 28, 2022 at 09:59:39PM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Implement RSA algorithm by hogweed from nettle. Thus QEMU supports
> a 'real' RSA backend to handle request from guest side. It's
> important to test RSA offload case without OS & hardware requirement.
> 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  crypto/akcipher-nettle.c.inc | 432 +++
>  crypto/akcipher.c|   4 +
>  crypto/meson.build   |   4 +
>  crypto/rsakey-builtin.c.inc  | 209 +
>  crypto/rsakey-nettle.c.inc   | 154 +
>  crypto/rsakey.c  |  44 
>  crypto/rsakey.h  |  94 
>  meson.build  |  11 +
>  8 files changed, 952 insertions(+)
>  create mode 100644 crypto/akcipher-nettle.c.inc
>  create mode 100644 crypto/rsakey-builtin.c.inc
>  create mode 100644 crypto/rsakey-nettle.c.inc
>  create mode 100644 crypto/rsakey.c
>  create mode 100644 crypto/rsakey.h
> 


> +
> +static void wrap_nettle_random_func(void *ctx, size_t len, uint8_t *out)
> +{
> +/* TODO: check result */
> +qcrypto_random_bytes(out, len, NULL);
> +}

Unfortunate meson requires this function to be void.

Since we've no way to report errors, then our only option is assume
qcrypto_random_bytes will never fail, and enforce that assumptiomn
by passing '_abort' for the last parameter.

> +
> +static int qcrypto_nettle_rsa_encrypt(QCryptoAkCipher *akcipher,
> +  const void *data, size_t data_len,
> +  void *enc, size_t enc_len,
> +  Error **errp)
> +{
> +
> +QCryptoNettleRSA *rsa = (QCryptoNettleRSA *)akcipher;
> +mpz_t c;
> +int ret = -1;
> +
> +if (data_len > rsa->pub.size || enc_len != rsa->pub.size) {
> +error_setg(errp, "Invalid buffer size");
> +return ret;
> +}

Can you report the invalid & expect buffer sizes, as it'll
make debugging much easier. You'll need a separate check
and error reporting for enc_len and data_len.

> +
> +/* Nettle do not support RSA encryption without any padding */
> +switch (rsa->padding_alg) {
> +case QCRYPTO_RSA_PADDING_ALG_RAW:
> +error_setg(errp, "RSA with raw padding is not supported");
> +break;
> +
> +case QCRYPTO_RSA_PADDING_ALG_PKCS1:
> +mpz_init(c);
> +if (rsa_encrypt(>pub, NULL, wrap_nettle_random_func,
> +  data_len, (uint8_t *)data, c) != 1) {
> +error_setg(errp, "Failed to encrypt");
> +} else {
> +nettle_mpz_get_str_256(enc_len, (uint8_t *)enc, c);
> +ret = enc_len;
> +}
> +mpz_clear(c);
> +break;
> +
> +default:
> +error_setg(errp, "Unknown padding");
> +}
> +
> +return ret;
> +}
> +
> +static int qcrypto_nettle_rsa_decrypt(QCryptoAkCipher *akcipher,
> +  const void *enc, size_t enc_len,
> +  void *data, size_t data_len,
> +  Error **errp)
> +{
> +QCryptoNettleRSA *rsa = (QCryptoNettleRSA *)akcipher;
> +mpz_t c;
> +int ret = -1;
> +if (enc_len > rsa->priv.size) {
> +error_setg(errp, "Invalid buffer size");
> +return ret;
> +}

Again please report the invalid & expected sizes in the message

We don't need to validate 'data_len' in the decrypt case,
as you did in encrypt ?

> +
> +switch (rsa->padding_alg) {
> +case QCRYPTO_RSA_PADDING_ALG_RAW:
> +error_setg(errp, "RSA with raw padding is not supported");
> +break;
> +
> +case QCRYPTO_RSA_PADDING_ALG_PKCS1:
> +nettle_mpz_init_set_str_256_u(c, enc_len, enc);
> +if (!rsa_decrypt(>priv, _len, (uint8_t *)data, c)) {
> +error_setg(errp, "Failed to decrypt");
> +} else {
> +ret = data_len;
> +}
> +
> +mpz_clear(c);
> +break;
> +
> +default:
> +ret = -1;

'ret' was initialized to '-1' at time of declaration

> +error_setg(errp, "Unknown padding");
> +}
> +
> +return ret;
> +}
> +
> +static int qcrypto_nettle_rsa_sign(QCryptoAkCipher *akcipher,
> +   const void *data, size_t data_len,
> +   void *sig, size_t sig_len, Error **errp)
> +{
> +QCryptoNettleRSA *rsa = (QCryptoNettleRSA *)akcipher;
> +int ret;

For consistency with the earlier methods, initialize this to -1

> +mpz_t s;
> +
> +/**
> + * The RSA algorithm cannot be used for signature/verification
> + * without padding.
> + */
> +if (rsa->padding_alg == QCRYPTO_RSA_PADDING_ALG_RAW) {
> +error_setg(errp, "Try to make signature without padding");
> +return -1;
> +}
> +
> +if (data_len > rsa->priv.size || sig_len != rsa->priv.size) {
> +error_setg(errp, "Invalid buffer 

Re: [PATCH v5 8/9] tests/crypto: Add test suite for RSA keys

[Daniel P . Berrangé]

On Thu, Apr 28, 2022 at 09:59:42PM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> As Daniel suggested, Add tests suite for rsakey, as a way to prove
> that we can handle DER errors correctly.
> 
> Signed-off-by: lei he 
> ---
>  tests/unit/test-crypto-akcipher.c | 285 +-
>  1 file changed, 282 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v5 3/9] crypto: Introduce akcipher crypto class

[Daniel P . Berrangé]

On Thu, Apr 28, 2022 at 09:59:37PM +0800, zhenwei pi wrote:
> Introduce new akcipher crypto class 'QCryptoAkCIpher', which supports
> basic asymmetric operations: encrypt, decrypt, sign and verify.
> 
> Suggested by Daniel P. Berrangé, also add autoptr cleanup for the new
> class. Thanks to Daniel!
> 
> Co-developed-by: lei he 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  crypto/akcipher.c | 102 
>  crypto/akcipherpriv.h |  55 +
>  crypto/meson.build|   1 +
>  include/crypto/akcipher.h | 158 ++
>  4 files changed, 316 insertions(+)
>  create mode 100644 crypto/akcipher.c
>  create mode 100644 crypto/akcipherpriv.h
>  create mode 100644 include/crypto/akcipher.h

Reviewed-by: Daniel P. Berrangé 


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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v5 1/9] virtio-crypto: header update

[Daniel P . Berrangé]

On Thu, Apr 28, 2022 at 09:59:35PM +0800, zhenwei pi wrote:
> Update header from linux, support akcipher service.
> 
> Reviewed-by: Gonglei 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  .../standard-headers/linux/virtio_crypto.h| 82 ++-
>  1 file changed, 81 insertions(+), 1 deletion(-)

I see these changes were now merged in linux.git with

  commit 24e19590628b58578748eeaec8140bf9c9dc00d9
  Author: zhenwei pi 
  AuthorDate: Wed Mar 2 11:39:15 2022 +0800
  Commit: Michael S. Tsirkin 
  CommitDate: Mon Mar 28 16:52:58 2022 -0400

virtio-crypto: introduce akcipher service

Introduce asymmetric service definition, asymmetric operations and
several well known algorithms.

Co-developed-by: lei he 
Signed-off-by: lei he 
Signed-off-by: zhenwei pi 
Link: 
https://lore.kernel.org/r/20220302033917.1295334-3-pizhen...@bytedance.com
Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Gonglei 


And the changes proposed here match that, so

  Reviewed-by: Daniel P. Berrangé 


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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v5 4/9] crypto: add ASN.1 DER decoder

[Daniel P . Berrangé]

On Thu, Apr 28, 2022 at 09:59:38PM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Add an ANS.1 DER decoder which is used to parse asymmetric
> cipher keys
> 
> Signed-off-by: zhenwei pi 
> Signed-off-by: lei he 
> ---
>  crypto/der.c | 190 +++
>  crypto/der.h |  82 ++
>  crypto/meson.build   |   1 +
>  tests/unit/meson.build   |   1 +
>  tests/unit/test-crypto-der.c | 290 +++
>  5 files changed, 564 insertions(+)
>  create mode 100644 crypto/der.c
>  create mode 100644 crypto/der.h
>  create mode 100644 tests/unit/test-crypto-der.c
> 
> diff --git a/crypto/der.c b/crypto/der.c
> new file mode 100644
> index 00..7907bcfd51
> --- /dev/null
> +++ b/crypto/der.c
> @@ -0,0 +1,190 @@
> +/*
> + * QEMU Crypto ASN.1 DER decoder
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: lei he 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#include 
> +#include 

These should both be replaced by

  #include "qemu/osdep.h"

otherwise this fails to build for Mingw targets


> +static int qcrypto_der_invoke_callback(DERDecodeCb cb, void *ctx,
> +   const uint8_t *value, size_t vlen,
> +   Error **errp)
> +{
> +if (!cb) {
> +return 0;
> +}
> +
> +return cb(ctx, value, vlen, errp);
> +}
> +
> +static int qcrypto_der_extract_definite_data(const uint8_t **data, size_t 
> *dlen,
> + DERDecodeCb cb, void *ctx,
> + Error **errp)
> +{
> +const uint8_t *value;
> +size_t vlen = 0;
> +uint8_t byte_count = qcrypto_der_cut_byte(data, dlen);
> +
> +/* short format of definite-length */
> +if (!(byte_count & QCRYPTO_DER_SHORT_LEN_MASK)) {
> +if (byte_count > *dlen) {
> +error_setg(errp, "Invalid content length: %u", byte_count);
> +return -1;
> +}
> +
> +value = *data;
> +vlen = byte_count;
> +qcrypto_der_cut_nbytes(data, dlen, vlen);
> +
> +if (qcrypto_der_invoke_callback(cb, ctx, value, vlen, errp) != 0) {
> +return -1;
> +}
> +return vlen;
> +}
> +
> +/* Ignore highest bit */
> +byte_count &= ~QCRYPTO_DER_SHORT_LEN_MASK;
> +
> +/*
> + * size_t is enough to store the value of length, although the DER
> + * encoding standard supports larger length.
> + */
> +if (byte_count > sizeof(size_t)) {
> +error_setg(errp, "Invalid byte count of content length: %u",
> +   byte_count);
> +return -1;
> +}

> +
> +if (*dlen < byte_count) {

Can you flip this to   'byte_count > *dlen' so that the ordering
is consistent with the rest of the checks in this method.


> +error_setg(errp, "Invalid content length: %u", byte_count);
> +return -1;
> +}
> +while (byte_count--) {
> +vlen <<= 8;
> +vlen += qcrypto_der_cut_byte(data, dlen);
> +}
> +
> +if (vlen > *dlen) {
> +error_setg(errp, "Invalid content length: %lu", vlen);
> +return -1;
> +}
> +
> +value = *data;
> +qcrypto_der_cut_nbytes(data, dlen, vlen);
> +
> +if (qcrypto_der_invoke_callback(cb, ctx, value, vlen, errp) != 0) {
> +return -1;
> +}
> +return vlen;
> +}



> diff --git a/crypto/der.h b/crypto/der.h
> new file mode 100644
> index 00..aaa0e01969
> --- /dev/null
> +++ b/crypto/der.h
> @@ -0,0 +1,82 @@

> +#ifndef QCRYPTO_ASN1_DECODER_H
> +#define QCRYPTO_ASN1_DECODER_H
> +
> +#include "qemu/osdep.h"

osdep.h should always be in the .c file

> +#include "qapi/error.h"
> +
> +/* Simple decoder used to parse DER encoded rsa keys. */
> +
> +/**
> + *  @opaque: user context.
> + *  @value: the starting address of |value| part of 'Tag-Length-Value' 
> pattern.
> + *  @vlen: length of the |value|.
> + *  Returns: 0 for success, any other value is considered an error.
> + */
> +typedef int (*DERDecodeCb) (void *opaque, const uint8_t *value,
> +size_t vlen, Error **errp);

Could you call this one   'QCryptoDERDecodeCb)'

> +
> +/**
> + * der_decode_int:

Needs updating for the new func name

> + * @data: pointer to address 

Re: [PATCH v5 7/9] test/crypto: Add test suite for crypto akcipher

[Daniel P . Berrangé]

On Thu, Apr 28, 2022 at 09:59:41PM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Add unit test and benchmark test for crypto akcipher.
> 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> Reviewed-by: Daniel P. Berrangé 
> ---
>  tests/bench/benchmark-crypto-akcipher.c | 157 ++
>  tests/bench/meson.build |   4 +
>  tests/bench/test_akcipher_keys.inc  | 537 ++
>  tests/unit/meson.build  |   1 +
>  tests/unit/test-crypto-akcipher.c   | 711 
>  5 files changed, 1410 insertions(+)
>  create mode 100644 tests/bench/benchmark-crypto-akcipher.c
>  create mode 100644 tests/bench/test_akcipher_keys.inc
>  create mode 100644 tests/unit/test-crypto-akcipher.c


> diff --git a/tests/bench/meson.build b/tests/bench/meson.build
> index 00b3c209dc..f793d972b6 100644
> --- a/tests/bench/meson.build
> +++ b/tests/bench/meson.build
> @@ -23,6 +23,10 @@ if have_block
>}
>  endif
>  
> +benchs += {
> +'benchmark-crypto-akcipher': [crypto],
> +}

This needs to moved above a bit to be include the 'if have_block'
section above, otherwise it breaks the build when using --disable-system


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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v4 7/8] tests/crypto: Add test suite for crypto akcipher

[Daniel P . Berrangé]

On Mon, Apr 11, 2022 at 06:43:26PM +0800, zhenwei pi wrote:
> From: lei he 
> 
> Add unit test and benchmark test for crypto akcipher.
> 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  tests/bench/benchmark-crypto-akcipher.c | 161 ++
>  tests/bench/meson.build |   4 +
>  tests/bench/test_akcipher_keys.inc  | 537 ++
>  tests/unit/meson.build  |   1 +
>  tests/unit/test-crypto-akcipher.c   | 708 
>  5 files changed, 1411 insertions(+)
>  create mode 100644 tests/bench/benchmark-crypto-akcipher.c
>  create mode 100644 tests/bench/test_akcipher_keys.inc
>  create mode 100644 tests/unit/test-crypto-akcipher.c
> 

> diff --git a/tests/bench/test_akcipher_keys.inc 
> b/tests/bench/test_akcipher_keys.inc
> new file mode 100644
> index 00..7adf218135
> --- /dev/null
> +++ b/tests/bench/test_akcipher_keys.inc
> @@ -0,0 +1,537 @@
> +/*
> + * Copyright (c) 2022 Bytedance, and/or its affiliates
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + * Author: lei he 
> + */
> +
> +/* RSA test keys, generated by OpenSSL */
> +static const uint8_t rsa1024_priv_key[] = {
> +0x30, 0x82, 0x02, 0x5c, 0x02, 0x01, 0x00, 0x02,
> +     0x81, 0x81, 0x00, 0xe6, 0x4d, 0x76, 0x4f, 0xb2,

snip

For the patch as is:

 Reviewed-by: Daniel P. Berrangé 


It could be nice to add another test with some intentionally corrupt
RSA keys with bad DER encoding, as a way to prove that we're handling
errors in DER decoding correctly when faced with malicous data from a
bad guest.

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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v4 6/8] crypto: Implement RSA algorithm by gcrypt

[Daniel P . Berrangé]

On Mon, Apr 11, 2022 at 06:43:25PM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Added gcryt implementation of RSA algorithm, RSA algorithm
> implemented by gcrypt has a higher priority than nettle because
> it supports raw padding.
> 
> Signed-off-by: Lei He 
> ---
>  crypto/akcipher-gcrypt.c.inc | 531 +++
>  crypto/akcipher.c|   4 +-
>  2 files changed, 534 insertions(+), 1 deletion(-)
>  create mode 100644 crypto/akcipher-gcrypt.c.inc
> 
> diff --git a/crypto/akcipher-gcrypt.c.inc b/crypto/akcipher-gcrypt.c.inc
> new file mode 100644
> index 00..c109bf0566
> --- /dev/null
> +++ b/crypto/akcipher-gcrypt.c.inc
> @@ -0,0 +1,531 @@
> +/*
> + * QEMU Crypto akcipher algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: lei he 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#include 
> +
> +#include 
> +
> +#include "qemu/osdep.h"
> +#include "qemu/host-utils.h"
> +#include "asn1_decoder.h"
> +#include "crypto/akcipher.h"
> +#include "crypto/random.h"
> +#include "qapi/error.h"
> +#include "sysemu/cryptodev.h"
> +#include "rsakey.h"
> +
> +typedef struct QCryptoGcryptRSA {
> +QCryptoAkCipher akcipher;
> +gcry_sexp_t key;
> +QCryptoRSAPaddingAlgorithm padding_alg;
> +QCryptoHashAlgorithm hash_alg;
> +} QCryptoGcryptRSA;
> +
> +static void qcrypto_gcrypt_rsa_destroy(QCryptoGcryptRSA *rsa)
> +{
> +if (!rsa) {
> +return;
> +}
> +
> +gcry_sexp_release(rsa->key);
> +g_free(rsa);
> +}
> +
> +static QCryptoGcryptRSA *qcrypto_gcrypt_rsa_new(
> +const QCryptoAkCipherOptionsRSA *opt,
> +QCryptoAkCipherKeyType type,
> +const uint8_t *key,  size_t keylen,
> +Error **errp);
> +
> +QCryptoAkCipher *qcrypto_akcipher_new(const QCryptoAkCipherOptions *opts,
> +  QCryptoAkCipherKeyType type,
> +  const uint8_t *key, size_t keylen,
> +  Error **errp)
> +{
> +switch (opts->algorithm) {
> +case QCRYPTO_AKCIPHER_ALG_RSA:
> +return (QCryptoAkCipher *)qcrypto_gcrypt_rsa_new(
> +>u.rsa, type, key, keylen, errp);
> +
> +default:
> +error_setg(errp, "Unsupported algorithm: %u", opts->algorithm);
> +return NULL;
> +}
> +
> +return NULL;
> +}
> +
> +static void qcrypto_gcrypt_set_rsa_size(QCryptoAkCipher *akcipher, 
> gcry_mpi_t n)
> +{
> +size_t key_size = (gcry_mpi_get_nbits(n) + 7) / 8;
> +akcipher->max_plaintext_len = key_size;
> +akcipher->max_ciphertext_len = key_size;
> +akcipher->max_dgst_len = key_size;
> +akcipher->max_signature_len = key_size;
> +}
> +
> +static int qcrypto_gcrypt_parse_rsa_private_key(
> +QCryptoGcryptRSA *rsa,
> +const uint8_t *key, size_t keylen)
> +{
> +QCryptoAkCipherRSAKey *rsa_key =
> +qcrypto_akcipher_parse_rsa_private_key(key, keylen);

With my earlier suggestion, you can use

  g_autoptr(QCryptoAkCipherRSAKey) rsa_key = ...

and avoid need for the later  qcrypto_akcipher_free_rsa_key calls.

> +gcry_mpi_t n = NULL, e = NULL, d = NULL, p = NULL, q = NULL, u = NULL;
> +int ret = -1;
> +bool compute_mul_inv = false;
> +gcry_error_t err;
> +if (!rsa_key) {
> +return ret;
> +}
> +
> +err = gcry_mpi_scan(, GCRYMPI_FMT_STD,
> +rsa_key->n.data, rsa_key->n.len, NULL);
> +if (gcry_err_code(err) != 0) {
> +goto clear;
> +}
> +
> +err = gcry_mpi_scan(, GCRYMPI_FMT_STD,
> +rsa_key->e.data, rsa_key->e.len, NULL);
> +if (gcry_err_code(err) != 0) {
> +goto clear;
> +}
> +
> +err = gcry_mpi_scan(, GCRYMPI_FMT_STD,
> +rsa_key->d.data, rsa_key->d.len, NULL);
> +if (gcry_err_code(err) != 0) {
> +goto clear;
> +}
> +
> +err = gcry_mpi_scan(, GCRYMPI_FMT_STD,
> +rsa_key->p.data, rsa_key->p.len, NULL);
> +if (gcry_err_code(err) != 0) {
> +goto clear;
> +}
> +
> +err = gcry_mpi_scan(, GCRYMPI_FMT_STD,
> +rsa_key->q.data, rsa_key->q.len, NULL);
> +if (gcry_err_code(err) != 0) {
> +goto clear;
> +}
> +
> +if (gcry_mpi_cmp_ui(p, 0) > 0 && gcry_mpi_cmp_ui(q, 0) > 0) {
> +

Re: [PATCH v4 5/8] crypto: Implement RSA algorithm by hogweed

[Daniel P . Berrangé]

On Mon, Apr 11, 2022 at 06:43:24PM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Implement RSA algorithm by hogweed from nettle. Thus QEMU supports
> a 'real' RSA backend to handle request from guest side. It's
> important to test RSA offload case without OS & hardware requirement.
> 
> Signed-off-by: Lei He 
> Signed-off-by: zhenwei pi 
> ---
>  crypto/akcipher-nettle.c.inc | 448 +++
>  crypto/akcipher.c|   4 +
>  crypto/meson.build   |   4 +
>  crypto/rsakey-builtin.c.inc  | 150 
>  crypto/rsakey-nettle.c.inc   | 141 +++
>  crypto/rsakey.c  |  43 
>  crypto/rsakey.h  |  96 
>  meson.build  |  11 +
>  8 files changed, 897 insertions(+)
>  create mode 100644 crypto/akcipher-nettle.c.inc
>  create mode 100644 crypto/rsakey-builtin.c.inc
>  create mode 100644 crypto/rsakey-nettle.c.inc
>  create mode 100644 crypto/rsakey.c
>  create mode 100644 crypto/rsakey.h
> 
> diff --git a/crypto/akcipher-nettle.c.inc b/crypto/akcipher-nettle.c.inc
> new file mode 100644
> index 00..de163cd89e
> --- /dev/null
> +++ b/crypto/akcipher-nettle.c.inc
> @@ -0,0 +1,448 @@
> +/*
> + * QEMU Crypto akcipher algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: lei he 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#include 
> +
> +#include 
> +
> +#include "qemu/osdep.h"
> +#include "qemu/host-utils.h"
> +#include "asn1_decoder.h"
> +#include "crypto/akcipher.h"
> +#include "crypto/random.h"
> +#include "qapi/error.h"
> +#include "sysemu/cryptodev.h"
> +#include "rsakey.h"
> +
> +typedef struct QCryptoNettleRSA {
> +QCryptoAkCipher akcipher;
> +struct rsa_public_key pub;
> +struct rsa_private_key priv;
> +QCryptoRSAPaddingAlgorithm padding_alg;
> +QCryptoHashAlgorithm hash_alg;
> +} QCryptoNettleRSA;
> +
> +static void qcrypto_nettle_rsa_destroy(void *ptr)
> +{
> +QCryptoNettleRSA *rsa = (QCryptoNettleRSA *)ptr;
> +if (!rsa) {
> +return;
> +}
> +
> +rsa_public_key_clear(>pub);
> +rsa_private_key_clear(>priv);
> +g_free(rsa);
> +}
> +
> +static QCryptoAkCipher *qcrypto_nettle_rsa_new(
> +const QCryptoAkCipherOptionsRSA *opt,
> +QCryptoAkCipherKeyType type,
> +const uint8_t *key,  size_t keylen,
> +Error **errp);
> +
> +QCryptoAkCipher *qcrypto_akcipher_new(const QCryptoAkCipherOptions *opts,
> +  QCryptoAkCipherKeyType type,
> +  const uint8_t *key, size_t keylen,
> +  Error **errp)
> +{
> +switch (opts->algorithm) {
> +case QCRYPTO_AKCIPHER_ALG_RSA:
> +return qcrypto_nettle_rsa_new(>u.rsa, type, key, keylen, errp);
> +
> +default:
> +error_setg(errp, "Unsupported algorithm: %u", opts->algorithm);
> +return NULL;
> +}
> +
> +return NULL;
> +}
> +
> +static void qcrypto_nettle_rsa_set_akcipher_size(QCryptoAkCipher *akcipher,
> + int key_size)
> +{
> +akcipher->max_plaintext_len = key_size;
> +akcipher->max_ciphertext_len = key_size;
> +akcipher->max_signature_len = key_size;
> +akcipher->max_dgst_len = key_size;
> +}
> +
> +static int qcrypt_nettle_parse_rsa_private_key(QCryptoNettleRSA *rsa,
> +   const uint8_t *key,
> +   size_t keylen)
> +{
> +QCryptoAkCipherRSAKey *rsa_key =
> +qcrypto_akcipher_parse_rsa_private_key(key, keylen);
> +int ret = -1;
> +if (!rsa_key) {
> +return ret;
> +}
> +
> +nettle_mpz_init_set_str_256_u(rsa->pub.n, rsa_key->n.len, 
> rsa_key->n.data);
> +nettle_mpz_init_set_str_256_u(rsa->pub.e, rsa_key->e.len, 
> rsa_key->e.data);
> +nettle_mpz_init_set_str_256_u(rsa->priv.d, rsa_key->d.len, 
> rsa_key->d.data);
> +nettle_mpz_init_set_str_256_u(rsa->priv.p, rsa_key->p.len, 
> rsa_key->p.data);
> +nettle_mpz_init_set_str_256_u(rsa->priv.q, rsa_key->q.len, 
> rsa_key->q.data);
> +nettle_mpz_init_set_str_256_u(rsa->priv.a, rsa_key->dp.len,
> +  rsa_key->dp.data);
> +nettle_mpz_init_set_str_256_u(rsa->priv.b, rsa_key->dq.len,
> +   

Re: [PATCH v4 4/8] crypto: add ASN.1 decoder

[Daniel P . Berrangé]

On Mon, Apr 11, 2022 at 06:43:23PM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Add an ANS.1 decoder which is used to parse asymmetric
> cipher keys
> 
> Signed-off-by: zhenwei pi 
> Signed-off-by: Lei He 
> ---
>  crypto/asn1_decoder.c | 161 ++
>  crypto/asn1_decoder.h |  75 +++
>  crypto/meson.build|   1 +
>  tests/unit/meson.build|   1 +
>  tests/unit/test-crypto-asn1-decoder.c | 289 ++
>  5 files changed, 527 insertions(+)
>  create mode 100644 crypto/asn1_decoder.c
>  create mode 100644 crypto/asn1_decoder.h
>  create mode 100644 tests/unit/test-crypto-asn1-decoder.c
> 
> diff --git a/crypto/asn1_decoder.c b/crypto/asn1_decoder.c
> new file mode 100644
> index 00..506487f713
> --- /dev/null
> +++ b/crypto/asn1_decoder.c

Lets rename this to simply 'der.c' since the DER format is just one
way ASN1 can be encoded, and we only care about DER.

> @@ -0,0 +1,161 @@
> +/*
> + * QEMU Crypto ASN.1 decoder
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: lei he 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#include 
> +#include 
> +
> +#include "crypto/asn1_decoder.h"
> +
> +enum der_type_tag {

QCryptoDERTypeTag

> +der_type_tag_bool = 0x1,

QCRYPTO_DER_TYPE_TAG_  for each constant

> +der_type_tag_int = 0x2,
> +der_type_tag_bit_str = 0x3,
> +der_type_tag_oct_str = 0x4,
> +der_type_tag_oct_null = 0x5,
> +der_type_tag_oct_oid = 0x6,
> +der_type_tag_seq = 0x10,
> +der_type_tag_set = 0x11,
> +};
> +
> +#define DER_CONSTRUCTED_MASK 0x20
> +#define DER_SHORT_LEN_MASK 0x80

QCRYPTO_DER_ as the name prefix for constants.

> +
> +static uint8_t der_peek_byte(const uint8_t **data, size_t *dlen)

'qcrypto_der_' as the name prefix for all methods


> +{
> +return **data;
> +}
> +
> +static void der_cut_nbytes(const uint8_t **data, size_t *dlen,
> +   size_t nbytes)
> +{
> +*data += nbytes;
> +*dlen -= nbytes;
> +}
> +
> +static uint8_t der_cut_byte(const uint8_t **data, size_t *dlen)
> +{
> +uint8_t val = der_peek_byte(data, dlen);
> +
> +der_cut_nbytes(data, dlen, 1);
> +
> +return val;
> +}
> +
> +static int der_invoke_callback(DERDecodeCb cb, void *ctx,
> +   const uint8_t *value, size_t vlen)

Make sure the 'const uint8_t' is vertically aligned with
the 'DERDecodeCb'

> +{
> +if (!cb) {
> +return 0;
> +}
> +
> +return cb(ctx, value, vlen);
> +}
> +
> +static int der_extract_definite_data(const uint8_t **data, size_t *dlen,
> + DERDecodeCb cb, void *ctx)
> +{
> +const uint8_t *value;
> +size_t vlen = 0;
> +uint8_t byte_count = der_cut_byte(data, dlen);
> +
> +/* short format of definite-length */
> +if (!(byte_count & DER_SHORT_LEN_MASK)) {
> +if (byte_count > *dlen) {
> +return -1;
> +}
> +
> +value = *data;
> +vlen = byte_count;
> +der_cut_nbytes(data, dlen, vlen);
> +
> +if (der_invoke_callback(cb, ctx, value, vlen)) {
> +return -1;
> +}
> +return vlen;
> +}
> +
> +/* Ignore highest bit */
> +byte_count &= ~DER_SHORT_LEN_MASK;
> +
> +/*
> + * size_t is enough to express the length, although the der encoding
> + * standard supports larger length.
> + */
> +if (byte_count > sizeof(size_t)) {
> +return -1;
> +}
> +
> +while (byte_count--) {
> +vlen <<= 8;
> +vlen += der_cut_byte(data, dlen);
> +}
> +
> +if (vlen > *dlen) {
> +return -1;
> +}
> +
> +value = *data;
> +der_cut_nbytes(data, dlen, vlen);
> +
> +if (der_invoke_callback(cb, ctx, value, vlen) != 0) {
> +return -1;
> +}
> +return vlen;
> +}
> +
> +static int der_extract_data(const uint8_t **data, size_t *dlen,
> +DERDecodeCb cb, void *ctx)
> +{
> +uint8_t val = der_peek_byte(data, dlen);
> +
> +/* must use definite length format */
> +if (val == DER_SHORT_LEN_MASK) {
> +return -1;
> +}
> +
> +return der_extract_definite_data(data, dlen, cb, ctx);
> +}
> +
> +int der_decode_int(const uint8_t **data, size_t *dlen,
> + 

Re: [PATCH v4 3/8] crypto: Introduce akcipher crypto class

[Daniel P . Berrangé]

On Mon, Apr 11, 2022 at 06:43:22PM +0800, zhenwei pi wrote:
> Support basic asymmetric operations: encrypt, decrypt, sign and
> verify.
> 
> Co-developed-by: lei he 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  crypto/akcipher.c | 102 +
>  crypto/akcipherpriv.h |  43 +++
>  crypto/meson.build|   1 +
>  include/crypto/akcipher.h | 151 ++
>  4 files changed, 297 insertions(+)
>  create mode 100644 crypto/akcipher.c
>  create mode 100644 crypto/akcipherpriv.h
>  create mode 100644 include/crypto/akcipher.h


> diff --git a/crypto/akcipherpriv.h b/crypto/akcipherpriv.h
> new file mode 100644
> index 00..da9e54a796
> --- /dev/null
> +++ b/crypto/akcipherpriv.h
> @@ -0,0 +1,43 @@
> +/*
> + * QEMU Crypto asymmetric algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: zhenwei pi 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#ifndef QCRYPTO_AKCIPHERPRIV_H
> +#define QCRYPTO_AKCIPHERPRIV_H
> +
> +#include "qapi/qapi-types-crypto.h"
> +
> +struct QCryptoAkCipherDriver {
> +int (*encrypt)(QCryptoAkCipher *akcipher,
> +   const void *in, size_t in_len,
> +   void *out, size_t out_len, Error **errp);
> +int (*decrypt)(QCryptoAkCipher *akcipher,
> +   const void *out, size_t out_len,
> +   void *in, size_t in_len, Error **errp);
> +int (*sign)(QCryptoAkCipher *akcipher,
> +const void *in, size_t in_len,
> +void *out, size_t out_len, Error **errp);
> +int (*verify)(QCryptoAkCipher *akcipher,
> +  const void *in, size_t in_len,
> +  const void *in2, size_t in2_len, Error **errp);
> +int (*free)(QCryptoAkCipher *akcipher, Error **errp);
> +};
> +
> +#endif /* QCRYPTO_AKCIPHER_H */


> diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
> new file mode 100644
> index 00..c1970b3b3b
> --- /dev/null
> +++ b/include/crypto/akcipher.h
> @@ -0,0 +1,151 @@
> +/*
> + * QEMU Crypto asymmetric algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: zhenwei pi 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#ifndef QCRYPTO_AKCIPHER_H
> +#define QCRYPTO_AKCIPHER_H
> +
> +#include "qapi/qapi-types-crypto.h"
> +
> +typedef struct QCryptoAkCipher QCryptoAkCipher;

This belongs here.

> +typedef struct QCryptoAkCipherDriver QCryptoAkCipherDriver;

This and...

> +
> +struct QCryptoAkCipher {
> +QCryptoAkCipherAlgorithm alg;
> +QCryptoAkCipherKeyType type;
> +int max_plaintext_len;
> +int max_ciphertext_len;
> +int max_signature_len;
> +int max_dgst_len;
> +QCryptoAkCipherDriver *driver;
> +};

...this should be in the akcipherpriv.h file though, since
they're only for internal usage.



> +/**
> + * qcrypto_akcipher_encrypt:
> + * @akcipher: akcipher context
> + * @in: plaintext pending to be encrypted
> + * @in_len: length of the plaintext, MUST less or equal to max_plaintext_len
> + * @out: buffer to store the ciphertext
> + * @out_len: the length of ciphertext buffer, usually equals to
> + *   max_ciphertext_len
> + * @errp: error pointer
> + *
> + * Encrypt data and write ciphertext into out
> + *
> + * Returns: length of ciphertext if encrypt succeed, otherwise -1 is returned
> + */
> +int qcrypto_akcipher_encrypt(QCryptoAkCipher *akcipher,
> + const void *in, size_t in_len,
> + void *out, size_t out_len, Error **errp);
> +
> +/**
> + * qcrypto_akcipher_decrypt:
> + * @akcipher: akcipher 

Re: [PATCH v4 2/8] crypto-akcipher: Introduce akcipher types to qapi

[Daniel P . Berrangé]

On Mon, Apr 11, 2022 at 06:43:21PM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Introduce akcipher types, also include RSA related types.
> 
> Signed-off-by: Lei He 
> Signed-off-by: zhenwei pi 
> ---
>  qapi/crypto.json | 64 
>  1 file changed, 64 insertions(+)

snip

> +##
> +# @QCryptoAkCipherOptions:
> +#
> +# The options that are available for all asymmetric key algorithms
> +# when creating a new QCryptoAkCipher.
> +#
> +# Since: 7.1
> +##
> +{ 'union': 'QCryptoAkCipherOptions',
> +  'base': { 'algorithm': 'QCryptoAkCipherAlgorithm' },
> +  'discriminator': 'algorithm',
> +  'data': { 'rsa': 'QCryptoAkCipherOptionsRSA' }}

I mistakenly suggested 'algorithm' here, but for consistency
with other fields, I should have said just 'alg'.

With that change

  Reviewed-by: Daniel P. Berrangé 


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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: PING: [PATCH v4 0/8] Introduce akcipher service for virtio-crypto

[Daniel P . Berrangé]

On Thu, Apr 21, 2022 at 09:41:40AM +0800, zhenwei pi wrote:
> Hi Daniel,
> Could you please review this series?

Yes, its on my to do. I've been on holiday for 2 weeks, so still catching
up on the backlog of reviews.

> On 4/11/22 18:43, zhenwei pi wrote:
> > v3 -> v4:
> > - Coding style fix: Akcipher -> AkCipher, struct XXX -> XXX, Rsa -> RSA,
> > XXX-alg -> XXX-algo.
> > - Change version info in qapi/crypto.json, from 7.0 -> 7.1.
> > - Remove ecdsa from qapi/crypto.json, it would be introduced with the 
> > implemetion later.
> > - Use QCryptoHashAlgothrim instead of QCryptoRSAHashAlgorithm(removed) in 
> > qapi/crypto.json.
> > - Rename arguments of qcrypto_akcipher_XXX to keep aligned with 
> > qcrypto_cipher_XXX(dec/enc/sign/vefiry -> in/out/in2), and add 
> > qcrypto_akcipher_max_XXX APIs.
> > - Add new API: qcrypto_akcipher_supports.
> > - Change the return value of qcrypto_akcipher_enc/dec/sign, these functions 
> > return the actual length of result.
> > - Separate ASN.1 source code and test case clean.
> > - Disable RSA raw encoding for akcipher-nettle.
> > - Separate RSA key parser into rsakey.{hc}, and implememts it with 
> > builtin-asn1-decoder and nettle respectivly.
> > - Implement RSA(pkcs1 and raw encoding) algorithm by gcrypt. This has 
> > higher priority than nettle.
> > - For some akcipher operations(eg, decryption of pkcs1pad(rsa)), the length 
> > of returned result maybe less than the dst buffer size, return the actual 
> > length of result instead of the buffer length to the guest side. (in 
> > function virtio_crypto_akcipher_input_data_helper)
> > - Other minor changes.
> > 
> > Thanks to Daniel!
> > 
> > Eric pointed out this missing part of use case, send it here again.
> > 
> > In our plan, the feature is designed for HTTPS offloading case and other 
> > applications which use kernel RSA/ecdsa by keyctl syscall. The full picture 
> > shows bellow:
> > 
> > 
> >Nginx/openssl[1] ... Apps
> > Guest   -
> > virtio-crypto driver[2]
> > -
> > virtio-crypto backend[3]
> > Host-
> >/  |  \
> >builtin[4]   vhost keyctl[5] ...
> > 
> > 
> > [1] User applications can offload RSA calculation to kernel by keyctl 
> > syscall. There is no keyctl engine in openssl currently, we developed a 
> > engine and tried to contribute it to openssl upstream, but openssl 1.x does 
> > not accept new feature. Link:
> >  https://github.com/openssl/openssl/pull/16689
> > 
> > This branch is available and maintained by Lei 
> >  https://github.com/TousakaRin/openssl/tree/OpenSSL_1_1_1-kctl_engine
> > 
> > We tested nginx(change config file only) with openssl keyctl engine, it 
> > works fine.
> > 
> > [2] virtio-crypto driver is used to communicate with host side, send 
> > requests to host side to do asymmetric calculation.
> >  https://lkml.org/lkml/2022/3/1/1425
> > 
> > [3] virtio-crypto backend handles requests from guest side, and forwards 
> > request to crypto backend driver of QEMU.
> > 
> > [4] Currently RSA is supported only in builtin driver. This driver is 
> > supposed to test the full feature without other software(Ex vhost process) 
> > and hardware dependence. ecdsa is introduced into qapi type without 
> > implementation, this may be implemented in Q3-2022 or later. If ecdsa type 
> > definition should be added with the implementation together, I'll remove 
> > this in next version.
> > 
> > [5] keyctl backend is in development, we will post this feature in Q2-2022. 
> > keyctl backend can use hardware acceleration(Ex, Intel QAT).
> > 
> > Setup the full environment, tested with Intel QAT on host side, the QPS of 
> > HTTPS increase to ~200% in a guest.
> > 
> > VS PCI passthrough: the most important benefit of this solution makes the 
> > VM migratable.
> > 
> > v2 -> v3:
> > - Introduce akcipher types to qapi
> > - Add test/benchmark suite for akcipher class
> > - Seperate 'virtio_crypto: Support virtio crypto asym operation' into:
> >- crypto: Introduce akcipher crypto class
> >- virtio-crypto: Introduce RSA algorithm
> > 
> > v1 -> v2:
> > - Update virtio_crypto.h from v2 version of related kernel patch.
> > 
> > v1:
> > - Support akcipher for virtio-crypto.
> > - Introduce akcipher class.
> > - Introduce ASN1 decoder into QEMU.
> > - Implement RSA backend by nettle/hogweed.
> > 
> > Lei He (4):
> >crypto-akcipher: Introduce akcipher types to qapi
> >crypto: add ASN.1 decoder
> >crypto: Implement RSA algorithm by hogweed
> >crypto: Implement RSA algorithm by gcrypt
> > 
> > Zhenwei Pi (3):
> >virtio-crypto: header update
> >crypto: Introduce akcipher crypto class
> >crypto: Introduce RSA algorithm
> > 
> > lei he (1):
> >tests/crypto: Add test suite for crypto akcipher
> > 
> >   

Re: [Spice-devel] 回复: Re: 回复: Re: 回复: Re: 回复: Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64

[Daniel P . Berrangé]

On Thu, Mar 24, 2022 at 02:21:09PM +0100, Gerd Hoffmann wrote:
> On Thu, Mar 24, 2022 at 06:34:02PM +0800, liuco...@kylinos.cn wrote:
> >ok, thanks, a lot of our customer use qxl on x86 before, so it still need
> >to supoort qxl on arm64.
> 
> Well, qxl isn't the best choice even on x86.  The main advantage it
> offers (2d acceleration) is basically useless today because pretty much
> everything moved on to use 3d acceleration instead.  So qxl ends up
> being used as dumb framebuffer with software 3d rendering.
> 
> So, I'm still recommending to just use virtio-gpu ...

Also bear in mind that while almost no one uses the 2d acceleration
in QXL, the QEMU device still implements all the 2d functionality.
This exposes an attack surface to the guest VM, from code that is
largely ignored by maintainers today, as attention is focused on
virtio-gpu instead. 

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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v3 1/6] virtio-crypto: header update

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:07AM +0800, zhenwei pi wrote:
> Update header from linux, support akcipher service.

I'm assuming this is updated for *non-merged* Linux headers, since
I don't see these changes present in current linux.git 

> 
> Reviewed-by: Gonglei 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  .../standard-headers/linux/virtio_crypto.h| 82 ++-
>  1 file changed, 81 insertions(+), 1 deletion(-)
> 
> diff --git a/include/standard-headers/linux/virtio_crypto.h 
> b/include/standard-headers/linux/virtio_crypto.h
> index 5ff0b4ee59..68066dafb6 100644
> --- a/include/standard-headers/linux/virtio_crypto.h
> +++ b/include/standard-headers/linux/virtio_crypto.h
> @@ -37,6 +37,7 @@
>  #define VIRTIO_CRYPTO_SERVICE_HASH   1
>  #define VIRTIO_CRYPTO_SERVICE_MAC2
>  #define VIRTIO_CRYPTO_SERVICE_AEAD   3
> +#define VIRTIO_CRYPTO_SERVICE_AKCIPHER 4
>  
>  #define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
>  
> @@ -57,6 +58,10 @@ struct virtio_crypto_ctrl_header {
>  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
>  #define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
>  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
> +#define VIRTIO_CRYPTO_AKCIPHER_CREATE_SESSION \
> +VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x04)
> +#define VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION \
> +VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x05)
>   uint32_t opcode;
>   uint32_t algo;
>   uint32_t flag;
> @@ -180,6 +185,58 @@ struct virtio_crypto_aead_create_session_req {
>   uint8_t padding[32];
>  };
>  
> +struct virtio_crypto_rsa_session_para {
> +#define VIRTIO_CRYPTO_RSA_RAW_PADDING   0
> +#define VIRTIO_CRYPTO_RSA_PKCS1_PADDING 1
> + uint32_t padding_algo;
> +
> +#define VIRTIO_CRYPTO_RSA_NO_HASH   0
> +#define VIRTIO_CRYPTO_RSA_MD2   1
> +#define VIRTIO_CRYPTO_RSA_MD3   2
> +#define VIRTIO_CRYPTO_RSA_MD4   3
> +#define VIRTIO_CRYPTO_RSA_MD5   4
> +#define VIRTIO_CRYPTO_RSA_SHA1  5

Do we really need to be adding support for all these obsolete
hash functions. Maybe SHA1 is borderline acceptable, but all
those obsolete MD* functions too ??

> +#define VIRTIO_CRYPTO_RSA_SHA2566
> +#define VIRTIO_CRYPTO_RSA_SHA3847
> +#define VIRTIO_CRYPTO_RSA_SHA5128
> +#define VIRTIO_CRYPTO_RSA_SHA2249
> + uint32_t hash_algo;
> +};
> +
> +struct virtio_crypto_ecdsa_session_para {
> +#define VIRTIO_CRYPTO_CURVE_UNKNOWN   0
> +#define VIRTIO_CRYPTO_CURVE_NIST_P192 1
> +#define VIRTIO_CRYPTO_CURVE_NIST_P224 2
> +#define VIRTIO_CRYPTO_CURVE_NIST_P256 3
> +#define VIRTIO_CRYPTO_CURVE_NIST_P384 4
> +#define VIRTIO_CRYPTO_CURVE_NIST_P521 5
> + uint32_t curve_id;
> + uint32_t padding;
> +};
> +
> +struct virtio_crypto_akcipher_session_para {
> +#define VIRTIO_CRYPTO_NO_AKCIPHER0
> +#define VIRTIO_CRYPTO_AKCIPHER_RSA   1
> +#define VIRTIO_CRYPTO_AKCIPHER_DSA   2
> +#define VIRTIO_CRYPTO_AKCIPHER_ECDSA 3

Here we have RSA, DSA and ECDSA, but the corresponding QEMU
qapi/crypto.json doesn't define DSA at all. Is that a mistake
on the QEMU side, or is the DSA support redundant ?

> + uint32_t algo;
> +
> +#define VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PUBLIC  1
> +#define VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PRIVATE 2
> + uint32_t keytype;
> + uint32_t keylen;
> +
> + union {
> + struct virtio_crypto_rsa_session_para rsa;
> + struct virtio_crypto_ecdsa_session_para ecdsa;
> + } u;
> +};


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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v3 5/6] tests/crypto: Add test suite for crypto akcipher

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:11AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Add unit test and benchmark test for crypto akcipher.
> 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  tests/bench/benchmark-crypto-akcipher.c | 163 ++
>  tests/bench/meson.build |   6 +
>  tests/bench/test_akcipher_keys.inc  | 277 +
>  tests/unit/meson.build  |   1 +
>  tests/unit/test-crypto-akcipher.c   | 715 
>  5 files changed, 1162 insertions(+)
>  create mode 100644 tests/bench/benchmark-crypto-akcipher.c
>  create mode 100644 tests/bench/test_akcipher_keys.inc
>  create mode 100644 tests/unit/test-crypto-akcipher.c
> 
> diff --git a/tests/bench/benchmark-crypto-akcipher.c 
> b/tests/bench/benchmark-crypto-akcipher.c
> new file mode 100644
> index 00..152fed8d73
> --- /dev/null
> +++ b/tests/bench/benchmark-crypto-akcipher.c
> @@ -0,0 +1,163 @@
> +/*
> + * QEMU Crypto cipher speed benchmark
> + *
> + * Copyright (c) 2022 Bytedance
> + *
> + * Authors:
> + *lei he 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version.  See the COPYING file in the
> + * top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "crypto/init.h"
> +#include "crypto/akcipher.h"
> +#include "standard-headers/linux/virtio_crypto.h"
> +
> +#include "test_akcipher_keys.inc"
> +
> +static bool keep_running;
> +
> +static void alarm_handler(int sig)
> +{
> +keep_running = false;
> +}
> +
> +static QCryptoAkcipher *create_rsa_akcipher(const uint8_t *priv_key,
> +size_t keylen,
> +QCryptoRsaPaddingAlgorithm 
> padding,
> +QCryptoRsaHashAlgorithm hash)
> +{
> +QCryptoRsaOptions opt;
> +QCryptoAkcipher *rsa;
> +Error *err = NULL;
> +
> +opt.padding_algo = padding;
> +opt.hash_algo = hash;
> +rsa = qcrypto_akcipher_new(QCRYPTO_AKCIPHER_ALG_RSA,
> +   QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE,
> +   priv_key, keylen, , );
> +
> +g_assert(rsa != NULL);
> +return rsa;
> +}
> +
> +static void test_rsa_speed(const uint8_t *priv_key, size_t keylen,
> +   size_t key_size)
> +{
> +#define Byte 8
> +#define SHA1_DGST_LEN 40
> +#define DURATION_SECONDS 10
> +#define padding QCRYPTO_RSA_PADDING_ALG_PKCS1
> +#define hash QCRYPTO_RSA_HASH_ALG_SHA1

'Byte' 'padding' and 'duration' are 

> +
> +Error *err = NULL;
> +QCryptoAkcipher *rsa;
> +uint8_t *dgst, *signature;
> +size_t count;
> +
> +rsa = create_rsa_akcipher(priv_key, keylen, padding, hash);
> +
> +dgst = g_new0(uint8_t, SHA1_DGST_LEN);
> +memset(dgst, g_test_rand_int(), SHA1_DGST_LEN);
> +signature = g_new0(uint8_t, key_size / Byte);
> +
> +g_test_message("benchmark rsa%lu (%s-%s) sign in %d seconds", key_size,
> +   QCryptoRsaPaddingAlgorithm_str(padding),
> +   QCryptoRsaHashAlgorithm_str(hash),
> +   DURATION_SECONDS);
> +alarm(DURATION_SECONDS);
> +g_test_timer_start();
> +for (keep_running = true, count = 0; keep_running; ++count) {
> +g_assert(qcrypto_akcipher_sign(rsa, dgst, SHA1_DGST_LEN,
> +   signature, key_size / Byte, ) > 
> 0);
> +}
> +g_test_timer_elapsed();



> +g_test_message("rsa%lu (%s-%s) sign %lu times in %.2f seconds,"
> +   " %.2f times/sec ",
> +   key_size,  QCryptoRsaPaddingAlgorithm_str(padding),
> +   QCryptoRsaHashAlgorithm_str(hash),
> +   count, g_test_timer_last(),
> +   (double)count / g_test_timer_last());
> +
> +g_test_message("benchmark rsa%lu (%s-%s) verify in %d seconds", key_size,
> +   QCryptoRsaPaddingAlgorithm_str(padding),
> +   QCryptoRsaHashAlgorithm_str(hash),
> +   DURATION_SECONDS);
> +alarm(DURATION_SECONDS);
> +g_test_timer_start();
> +for (keep_running = true, count = 0; keep_running; ++count) {
> +g_assert(qcrypto_akcipher_verify(rsa, signature, key_size / Byte,
> + dgst, SHA1_DGST_LEN, ) == 0);
> +}
> +g_test_timer_elapsed();
> +g_test_message("rsa%lu (%s-%s) verify %lu times in %.2f seconds,"
> +   " %.2f times/sec ",
> +   key_size, QCryptoRsaPaddingAlgorithm_str(padding),
> +   QCryptoRsaHashAlgorithm_str(hash),
> +   count, g_test_timer_last(),
> +   (double)count / g_test_timer_last());
> +
> +g_assert(qcrypto_akcipher_free(rsa, ) == 0);
> +g_free(dgst);
> +g_free(signature);
> +}
> +
> +static void test_rsa_1024_speed(const void *opaque)
> +{
> +size_t key_size = (size_t)opaque;
> +

Re: [PATCH v3 2/6] crypto-akcipher: Introduce akcipher types to qapi

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:08AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Introduce akcipher types, also include RSA & ECDSA related types.
> 
> Signed-off-by: Lei He 
> Signed-off-by: zhenwei pi 
> ---
>  qapi/crypto.json | 86 
>  1 file changed, 86 insertions(+)
> 
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 1ec54c15ca..d44c38e3b1 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -540,3 +540,89 @@
>'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
>  '*sanity-check': 'bool',
>  '*passwordid': 'str' } }
> +##
> +# @QCryptoAkcipherAlgorithm:
> +#
> +# The supported algorithms for asymmetric encryption ciphers
> +#
> +# @rsa: RSA algorithm
> +# @ecdsa: ECDSA algorithm
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoAkcipherAlgorithm',
> +  'prefix': 'QCRYPTO_AKCIPHER_ALG',
> +  'data': ['rsa', 'ecdsa']}

What were your intentions wrt  ecdsa - the nettle impl in this patch
series doesn't appear to actually support ecdsa. Are you intending to
add this in later versions of this patch series, or do it as separate
work at a later date ?


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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v3 4/6] crypto: Implement RSA algorithm by hogweed

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:10AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Introduce ASN.1 decoder, and implement RSA algorithm by hogweed
> from nettle. Thus QEMU supports a 'real' RSA backend to handle
> request from guest side. It's important to test RSA offload case
> without OS & hardware requirement.
> 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  crypto/akcipher-nettle.c  | 523 ++
>  crypto/akcipher.c |   3 +
>  crypto/asn1_decoder.c | 185 ++
>  crypto/asn1_decoder.h |  42 +++

Please introduce the asn1 files in a separate commit, and also
provide a unit test to validate them in the same commit.

> diff --git a/crypto/akcipher-nettle.c b/crypto/akcipher-nettle.c
> new file mode 100644
> index 00..45b93af772
> --- /dev/null
> +++ b/crypto/akcipher-nettle.c
> @@ -0,0 +1,523 @@
> +/*
> + * QEMU Crypto akcipher algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: lei he 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#include 
> +
> +#include 
> +
> +#include "qemu/osdep.h"
> +#include "qemu/host-utils.h"
> +#include "asn1_decoder.h"
> +#include "crypto/akcipher.h"
> +#include "crypto/random.h"
> +#include "qapi/error.h"
> +#include "sysemu/cryptodev.h"
> +
> +typedef struct QCryptoNettleRsa {
> +QCryptoAkcipher akcipher;
> +struct rsa_public_key pub;
> +struct rsa_private_key priv;
> +QCryptoRsaPaddingAlgorithm padding_algo;
> +QCryptoRsaHashAlgorithm hash_algo;
> +} QCryptoNettleRsa;

Call this QCryptoAkCipherNettleRSA

> +
> +struct asn1_parse_ctx {
> +const uint8_t *data;
> +size_t dlen;
> +};
> +
> +#define Octet 8
> +
> +static int extract_value(void *p, const uint8_t *data, size_t dlen)
> +{
> +struct asn1_parse_ctx *ctx = (struct asn1_parse_ctx *)p;
> +ctx->data = (uint8_t *)data;
> +ctx->dlen = dlen;
> +
> +return 0;
> +}
> +
> +static int extract_mpi(void *p, const uint8_t *data, size_t dlen)
> +{
> +mpz_t *target = (mpz_t *)p;
> +nettle_mpz_set_str_256_u(*target, dlen, data);
> +
> +return 0;
> +}
> +
> +static QCryptoNettleRsa *qcrypto_nettle_rsa_malloc(void);
> +
> +static void qcrypto_nettle_rsa_destroy(void *ptr)
> +{
> +QCryptoNettleRsa *rsa = (QCryptoNettleRsa *)ptr;
> +if (!rsa) {
> +return;
> +}
> +
> +rsa_public_key_clear(>pub);
> +rsa_private_key_clear(>priv);
> +g_free(rsa);
> +}
> +
> +static QCryptoAkcipher *qcrypto_nettle_new_rsa(
> +QCryptoAkcipherKeyType type,
> +const uint8_t *key,  size_t keylen,
> +QCryptoRsaOptions *opt, Error **errp);
> +
> +QCryptoAkcipher *qcrypto_akcipher_nettle_new(QCryptoAkcipherAlgorithm alg,
> + QCryptoAkcipherKeyType type,
> + const uint8_t *key,
> + size_t keylen, void *para,
> + Error **errp)
> +{
> +switch (alg) {
> +case QCRYPTO_AKCIPHER_ALG_RSA:
> +return qcrypto_nettle_new_rsa(type, key, keylen,
> +  (QCryptoRsaOptions *)para, errp);
> +default:
> +error_setg(errp, "Unsupported algorithm: %u", alg);
> +return NULL;
> +}
> +
> +return NULL;
> +}
> +
> +/**
> + * Parse ber encoded rsa private key, asn1 schema:
> + *RsaPrivKey ::= SEQUENCE {
> + * version INTEGER
> + * n   INTEGER
> + * e   INTEGER
> + * d   INTEGER
> + * p   INTEGER
> + * q   INTEGER
> + * e1  INTEGER
> + * e2  INTEGER
> + * u   INTEGER
> + * }
> + */
> +static int parse_rsa_private_key(QCryptoNettleRsa *rsa,
> + const uint8_t *key, size_t keylen)
> +{
> +struct asn1_parse_ctx ctx;
> +
> +if (ber_decode_seq(, , extract_value, ) != 0 ||
> +keylen != 0) {
> +return -1;
> +}
> +
> +if (ber_decode_int(, , NULL, NULL) != 0 ||
> +ber_decode_int(, , extract_mpi, >pub.n) != 0 
> ||
> +ber_decode_int(, , extract_mpi, >pub.e) != 0 
> ||
> +ber_decode_int(, , extract_mpi, >priv.d) != 

Re: [PATCH v3 3/6] crypto: Introduce akcipher crypto class

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:09AM +0800, zhenwei pi wrote:
> Support basic asymmetric operations: encrypt, decrypt, sign and
> verify.
> 
> Co-developed-by: lei he 
> Signed-off-by: lei he 
> Signed-off-by: zhenwei pi 
> ---
>  crypto/akcipher.c |  78 +
>  crypto/meson.build|   1 +
>  include/crypto/akcipher.h | 139 ++
>  3 files changed, 218 insertions(+)
>  create mode 100644 crypto/akcipher.c
>  create mode 100644 include/crypto/akcipher.h
> 
> diff --git a/crypto/akcipher.c b/crypto/akcipher.c
> new file mode 100644
> index 00..1e52f2fd76
> --- /dev/null
> +++ b/crypto/akcipher.c
> @@ -0,0 +1,78 @@
> +/*
> + * QEMU Crypto akcipher algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: zhenwei pi 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/host-utils.h"
> +#include "qapi/error.h"
> +#include "crypto/akcipher.h"
> +
> +QCryptoAkcipher *qcrypto_akcipher_new(QCryptoAkcipherAlgorithm alg,
> +  QCryptoAkcipherKeyType type,
> +  const uint8_t *key, size_t keylen,
> +  void *para, Error **errp)
> +{
> +QCryptoAkcipher *akcipher = NULL;
> +
> +return akcipher;
> +}
> +
> +int qcrypto_akcipher_encrypt(QCryptoAkcipher *akcipher,
> + const void *data, size_t data_len,
> + void *enc, size_t enc_len, Error **errp)
> +{
> +const QCryptoAkcipherDriver *drv = akcipher->driver;
> +
> +return drv->encrypt(akcipher, data, data_len, enc, enc_len, errp);
> +}
> +
> +int qcrypto_akcipher_decrypt(struct QCryptoAkcipher *akcipher,
> + const void *enc, size_t enc_len,
> + void *data, size_t data_len, Error **errp)
> +{
> +const QCryptoAkcipherDriver *drv = akcipher->driver;
> +
> +return drv->decrypt(akcipher, enc, enc_len, data, data_len, errp);
> +}
> +
> +int qcrypto_akcipher_sign(struct QCryptoAkcipher *akcipher,
> +  const void *data, size_t data_len,
> +  void *sig, size_t sig_len, Error **errp)
> +{
> +const QCryptoAkcipherDriver *drv = akcipher->driver;
> +
> +return drv->sign(akcipher, data, data_len, sig, sig_len, errp);
> +}
> +
> +int qcrypto_akcipher_verify(struct QCryptoAkcipher *akcipher,
> +const void *sig, size_t sig_len,
> +const void *data, size_t data_len, Error **errp)
> +{
> +const QCryptoAkcipherDriver *drv = akcipher->driver;
> +
> +return drv->verify(akcipher, sig, sig_len, data, data_len, errp);
> +}
> +
> +int qcrypto_akcipher_free(struct QCryptoAkcipher *akcipher, Error **errp)
> +{
> +const QCryptoAkcipherDriver *drv = akcipher->driver;
> +
> +return drv->free(akcipher, errp);
> +}
> diff --git a/crypto/meson.build b/crypto/meson.build
> index 19c44bea89..c32b57aeda 100644
> --- a/crypto/meson.build
> +++ b/crypto/meson.build
> @@ -19,6 +19,7 @@ crypto_ss.add(files(
>'tlscredspsk.c',
>'tlscredsx509.c',
>'tlssession.c',
> +  'akcipher.c',
>  ))
>  
>  if nettle.found()
> diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
> new file mode 100644
> index 00..03cc3bf46b
> --- /dev/null
> +++ b/include/crypto/akcipher.h
> @@ -0,0 +1,139 @@
> +/*
> + * QEMU Crypto asymmetric algorithms
> + *
> + * Copyright (c) 2022 Bytedance
> + * Author: zhenwei pi 
> + *
> + * 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.1 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 
> .
> + *
> + */
> +
> +#ifndef QCRYPTO_AKCIPHER_H

Re: [PATCH v3 2/6] crypto-akcipher: Introduce akcipher types to qapi

[Daniel P . Berrangé]

On Wed, Mar 23, 2022 at 10:49:08AM +0800, zhenwei pi wrote:
> From: Lei He 
> 
> Introduce akcipher types, also include RSA & ECDSA related types.
> 
> Signed-off-by: Lei He 
> Signed-off-by: zhenwei pi 
> ---
>  qapi/crypto.json | 86 
>  1 file changed, 86 insertions(+)
> 
> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index 1ec54c15ca..d44c38e3b1 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -540,3 +540,89 @@
>'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
>  '*sanity-check': 'bool',
>  '*passwordid': 'str' } }
> +##
> +# @QCryptoAkcipherAlgorithm:

Should be named  QCryptoAkCipherAlgorithm

> +#
> +# The supported algorithms for asymmetric encryption ciphers
> +#
> +# @rsa: RSA algorithm
> +# @ecdsa: ECDSA algorithm
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoAkcipherAlgorithm',
> +  'prefix': 'QCRYPTO_AKCIPHER_ALG',
> +  'data': ['rsa', 'ecdsa']}
> +
> +##
> +# @QCryptoAkcipherKeyType:

Should be named  QCryptoAkCipherKeyType

> +#
> +# The type of asymmetric keys.
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoAkcipherKeyType',
> +  'prefix': 'QCRYPTO_AKCIPHER_KEY_TYPE',
> +  'data': ['public', 'private']}
> +
> +##
> +# @QCryptoRsaHashAlgorithm:
> +#
> +# The hash algorithm for RSA pkcs1 padding algothrim
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoRsaHashAlgorithm',
> +  'prefix': 'QCRYPTO_RSA_HASH_ALG',
> +  'data': [ 'md2', 'md3', 'md4', 'md5', 'sha1', 'sha256', 'sha384', 
> 'sha512', 'sha224' ]}

We already have QCryptoHashAlgorithm and I don't see the
benefit in duplicating it here.

We don't have md2, md3, and md4 in QCryptoHashAlgorithm, but
that doesn't look like a real negative as I can't imagine
those should be used today.

> +##
> +# @QCryptoRsaPaddingAlgorithm:
> +#
> +# The padding algorithm for RSA.
> +#
> +# @raw: no padding used
> +# @pkcs1: pkcs1#v1.5
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoRsaPaddingAlgorithm',
> +  'prefix': 'QCRYPTO_RSA_PADDING_ALG',
> +  'data': ['raw', 'pkcs1']}
> +
> +##
> +# @QCryptoCurveId:

Should be named  QCryptoCurveID

> +#
> +# The well-known curves, referenced from 
> https://csrc.nist.gov/csrc/media/publications/fips/186/3/archive/2009-06-25/documents/fips_186-3.pdf
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'QCryptoCurveId',
> +  'prefix': 'QCRYPTO_CURVE_ID',
> +  'data': ['nist-p192', 'nist-p224', 'nist-p256', 'nist-p384', 'nist-p521']}


> +
> +##
> +# @QCryptoRsaOptions:

This should be named  QCryptoAkCipherOptionsRSA

> +#
> +# Specific parameters for RSA algorithm.
> +#
> +# @hash-algo: QCryptoRsaHashAlgorithm
> +# @padding-algo: QCryptoRsaPaddingAlgorithm
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'QCryptoRsaOptions',
> +  'data': { 'hash-algo':'QCryptoRsaHashAlgorithm',
> +'padding-algo': 'QCryptoRsaPaddingAlgorithm'}}

Our naming convention is  'XXX-alg' rather than 'XXX-algo'.

> +
> +##
> +# @QCryptoEcdsaOptions:

This should be named  QCryptoAkCipherOptionsECDSA

> +#
> +# Specific parameter for ECDSA algorithm.
> +#
> +# @curve-id: QCryptoCurveId
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'QCryptoEcdsaOptions',
> +  'data': { 'curve-id': 'QCryptoCurveId' }}

Having these two structs standalone looks wrong to me. I suspect that
callers will need to be able to conditionally pass in either one, and
so require the API to use a discriminated union

  { 'union': 'QCryptoAkCipherOptions'
'base': { 'algorithm': 'QCryptoAkCipherAlgorithm' },
'discriminator': 'algorithm',
'data': { 'rsa': 'QCryptoAkCipherOptionsRSA' ,
  'ecdsa': 'QCryptoAkCipherOptionsECDSA' } }


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

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization