mbox series

[v7,0/9] Introduce akcipher service for virtio-crypto

Message ID 20220525090118.43403-1-helei.sig11@bytedance.com (mailing list archive)
Headers show
Series Introduce akcipher service for virtio-crypto | expand

Message

Lei He May 25, 2022, 9:01 a.m. UTC
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 <helei.sig11@bytedance.com>
   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 (6):
  qapi: crypto-akcipher: Introduce akcipher types to qapi
  crypto: add ASN.1 DER decoder
  crypto: Implement RSA algorithm by hogweed
  crypto: Implement RSA algorithm by gcrypt
  test/crypto: Add test suite for crypto akcipher
  tests/crypto: Add test suite for RSA keys

Zhenwei Pi (3):
  virtio-crypto: header update
  crypto: Introduce akcipher crypto class
  crypto: Introduce RSA algorithm

 backends/cryptodev-builtin.c                   | 272 ++++++-
 backends/cryptodev-vhost-user.c                |  34 +-
 backends/cryptodev.c                           |  32 +-
 crypto/akcipher-gcrypt.c.inc                   | 595 +++++++++++++++
 crypto/akcipher-nettle.c.inc                   | 451 +++++++++++
 crypto/akcipher.c                              | 108 +++
 crypto/akcipherpriv.h                          |  55 ++
 crypto/der.c                                   | 189 +++++
 crypto/der.h                                   |  81 ++
 crypto/meson.build                             |   6 +
 crypto/rsakey-builtin.c.inc                    | 200 +++++
 crypto/rsakey-nettle.c.inc                     | 158 ++++
 crypto/rsakey.c                                |  44 ++
 crypto/rsakey.h                                |  92 +++
 hw/virtio/virtio-crypto.c                      | 323 ++++++--
 include/crypto/akcipher.h                      | 158 ++++
 include/hw/virtio/virtio-crypto.h              |   5 +-
 include/standard-headers/linux/virtio_crypto.h |  82 +-
 include/sysemu/cryptodev.h                     |  83 ++-
 meson.build                                    |  11 +
 qapi/crypto.json                               |  64 ++
 tests/bench/benchmark-crypto-akcipher.c        | 137 ++++
 tests/bench/meson.build                        |   1 +
 tests/bench/test_akcipher_keys.inc             | 537 ++++++++++++++
 tests/unit/meson.build                         |   2 +
 tests/unit/test-crypto-akcipher.c              | 990 +++++++++++++++++++++++++
 tests/unit/test-crypto-der.c                   | 290 ++++++++
 27 files changed, 4854 insertions(+), 146 deletions(-)
 create mode 100644 crypto/akcipher-gcrypt.c.inc
 create mode 100644 crypto/akcipher-nettle.c.inc
 create mode 100644 crypto/akcipher.c
 create mode 100644 crypto/akcipherpriv.h
 create mode 100644 crypto/der.c
 create mode 100644 crypto/der.h
 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
 create mode 100644 include/crypto/akcipher.h
 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
 create mode 100644 tests/unit/test-crypto-der.c

Comments

Daniel P. Berrangé May 26, 2022, 10:48 a.m. UTC | #1
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 <helei.sig11@bytedance.com>
>    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 (6):
>   qapi: crypto-akcipher: Introduce akcipher types to qapi
>   crypto: add ASN.1 DER decoder
>   crypto: Implement RSA algorithm by hogweed
>   crypto: Implement RSA algorithm by gcrypt
>   test/crypto: Add test suite for crypto akcipher
>   tests/crypto: Add test suite for RSA keys
> 
> Zhenwei Pi (3):
>   virtio-crypto: header update
>   crypto: Introduce akcipher crypto class
>   crypto: Introduce RSA algorithm
> 
>  backends/cryptodev-builtin.c                   | 272 ++++++-
>  backends/cryptodev-vhost-user.c                |  34 +-
>  backends/cryptodev.c                           |  32 +-
>  crypto/akcipher-gcrypt.c.inc                   | 595 +++++++++++++++
>  crypto/akcipher-nettle.c.inc                   | 451 +++++++++++
>  crypto/akcipher.c                              | 108 +++
>  crypto/akcipherpriv.h                          |  55 ++
>  crypto/der.c                                   | 189 +++++
>  crypto/der.h                                   |  81 ++
>  crypto/meson.build                             |   6 +
>  crypto/rsakey-builtin.c.inc                    | 200 +++++
>  crypto/rsakey-nettle.c.inc                     | 158 ++++
>  crypto/rsakey.c                                |  44 ++
>  crypto/rsakey.h                                |  92 +++
>  hw/virtio/virtio-crypto.c                      | 323 ++++++--
>  include/crypto/akcipher.h                      | 158 ++++
>  include/hw/virtio/virtio-crypto.h              |   5 +-
>  include/standard-headers/linux/virtio_crypto.h |  82 +-
>  include/sysemu/cryptodev.h                     |  83 ++-
>  meson.build                                    |  11 +
>  qapi/crypto.json                               |  64 ++
>  tests/bench/benchmark-crypto-akcipher.c        | 137 ++++
>  tests/bench/meson.build                        |   1 +
>  tests/bench/test_akcipher_keys.inc             | 537 ++++++++++++++
>  tests/unit/meson.build                         |   2 +
>  tests/unit/test-crypto-akcipher.c              | 990 +++++++++++++++++++++++++
>  tests/unit/test-crypto-der.c                   | 290 ++++++++
>  27 files changed, 4854 insertions(+), 146 deletions(-)
>  create mode 100644 crypto/akcipher-gcrypt.c.inc
>  create mode 100644 crypto/akcipher-nettle.c.inc
>  create mode 100644 crypto/akcipher.c
>  create mode 100644 crypto/akcipherpriv.h
>  create mode 100644 crypto/der.c
>  create mode 100644 crypto/der.h
>  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
>  create mode 100644 include/crypto/akcipher.h
>  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
>  create mode 100644 tests/unit/test-crypto-der.c
> 
> -- 
> 2.11.0
> 

With regards,
Daniel
Zhijian Li (Fujitsu)" via May 26, 2022, 11:38 a.m. UTC | #2
> -----Original Message-----
> From: Daniel P. Berrangé [mailto:berrange@redhat.com]
> Sent: Thursday, May 26, 2022 6:48 PM
> To: Lei He <helei.sig11@bytedance.com>
> Cc: mst@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>;
> qemu-devel@nongnu.org; virtualization@lists.linux-foundation.org;
> linux-crypto@vger.kernel.org; jasowang@redhat.com; cohuck@redhat.com;
> pizhenwei@bytedance.com
> Subject: Re: [PATCH v7 0/9] Introduce akcipher service for virtio-crypto
> 
> 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.
> 

Thanks for your work, Daniel.

Regards,
-Gonglei

> 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 <helei.sig11@bytedance.com>
> >
> > 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 (6):
> >   qapi: crypto-akcipher: Introduce akcipher types to qapi
> >   crypto: add ASN.1 DER decoder
> >   crypto: Implement RSA algorithm by hogweed
> >   crypto: Implement RSA algorithm by gcrypt
> >   test/crypto: Add test suite for crypto akcipher
> >   tests/crypto: Add test suite for RSA keys
> >
> > Zhenwei Pi (3):
> >   virtio-crypto: header update
> >   crypto: Introduce akcipher crypto class
> >   crypto: Introduce RSA algorithm
> >
> >  backends/cryptodev-builtin.c                   | 272 ++++++-
> >  backends/cryptodev-vhost-user.c                |  34 +-
> >  backends/cryptodev.c                           |  32 +-
> >  crypto/akcipher-gcrypt.c.inc                   | 595
> +++++++++++++++
> >  crypto/akcipher-nettle.c.inc                   | 451 +++++++++++
> >  crypto/akcipher.c                              | 108 +++
> >  crypto/akcipherpriv.h                          |  55 ++
> >  crypto/der.c                                   | 189 +++++
> >  crypto/der.h                                   |  81 ++
> >  crypto/meson.build                             |   6 +
> >  crypto/rsakey-builtin.c.inc                    | 200 +++++
> >  crypto/rsakey-nettle.c.inc                     | 158 ++++
> >  crypto/rsakey.c                                |  44 ++
> >  crypto/rsakey.h                                |  92 +++
> >  hw/virtio/virtio-crypto.c                      | 323 ++++++--
> >  include/crypto/akcipher.h                      | 158 ++++
> >  include/hw/virtio/virtio-crypto.h              |   5 +-
> >  include/standard-headers/linux/virtio_crypto.h |  82 +-
> >  include/sysemu/cryptodev.h                     |  83 ++-
> >  meson.build                                    |  11 +
> >  qapi/crypto.json                               |  64 ++
> >  tests/bench/benchmark-crypto-akcipher.c        | 137 ++++
> >  tests/bench/meson.build                        |   1 +
> >  tests/bench/test_akcipher_keys.inc             | 537 ++++++++++++++
> >  tests/unit/meson.build                         |   2 +
> >  tests/unit/test-crypto-akcipher.c              | 990
> +++++++++++++++++++++++++
> >  tests/unit/test-crypto-der.c                   | 290 ++++++++
> >  27 files changed, 4854 insertions(+), 146 deletions(-)  create mode
> > 100644 crypto/akcipher-gcrypt.c.inc  create mode 100644
> > crypto/akcipher-nettle.c.inc  create mode 100644 crypto/akcipher.c
> > create mode 100644 crypto/akcipherpriv.h  create mode 100644
> > crypto/der.c  create mode 100644 crypto/der.h  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  create mode 100644
> > include/crypto/akcipher.h  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  create mode
> > 100644 tests/unit/test-crypto-der.c
> >
> > --
> > 2.11.0
> >
> 
> 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 :|
zhenwei pi May 26, 2022, 11:48 a.m. UTC | #3
Hi, Daniel

Thanks a lot for your review!

On 5/26/22 18:48, Daniel P. Berrangé wrote:
> 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 <helei.sig11@bytedance.com>
>>     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 (6):
>>    qapi: crypto-akcipher: Introduce akcipher types to qapi
>>    crypto: add ASN.1 DER decoder
>>    crypto: Implement RSA algorithm by hogweed
>>    crypto: Implement RSA algorithm by gcrypt
>>    test/crypto: Add test suite for crypto akcipher
>>    tests/crypto: Add test suite for RSA keys
>>
>> Zhenwei Pi (3):
>>    virtio-crypto: header update
>>    crypto: Introduce akcipher crypto class
>>    crypto: Introduce RSA algorithm
>>
>>   backends/cryptodev-builtin.c                   | 272 ++++++-
>>   backends/cryptodev-vhost-user.c                |  34 +-
>>   backends/cryptodev.c                           |  32 +-
>>   crypto/akcipher-gcrypt.c.inc                   | 595 +++++++++++++++
>>   crypto/akcipher-nettle.c.inc                   | 451 +++++++++++
>>   crypto/akcipher.c                              | 108 +++
>>   crypto/akcipherpriv.h                          |  55 ++
>>   crypto/der.c                                   | 189 +++++
>>   crypto/der.h                                   |  81 ++
>>   crypto/meson.build                             |   6 +
>>   crypto/rsakey-builtin.c.inc                    | 200 +++++
>>   crypto/rsakey-nettle.c.inc                     | 158 ++++
>>   crypto/rsakey.c                                |  44 ++
>>   crypto/rsakey.h                                |  92 +++
>>   hw/virtio/virtio-crypto.c                      | 323 ++++++--
>>   include/crypto/akcipher.h                      | 158 ++++
>>   include/hw/virtio/virtio-crypto.h              |   5 +-
>>   include/standard-headers/linux/virtio_crypto.h |  82 +-
>>   include/sysemu/cryptodev.h                     |  83 ++-
>>   meson.build                                    |  11 +
>>   qapi/crypto.json                               |  64 ++
>>   tests/bench/benchmark-crypto-akcipher.c        | 137 ++++
>>   tests/bench/meson.build                        |   1 +
>>   tests/bench/test_akcipher_keys.inc             | 537 ++++++++++++++
>>   tests/unit/meson.build                         |   2 +
>>   tests/unit/test-crypto-akcipher.c              | 990 +++++++++++++++++++++++++
>>   tests/unit/test-crypto-der.c                   | 290 ++++++++
>>   27 files changed, 4854 insertions(+), 146 deletions(-)
>>   create mode 100644 crypto/akcipher-gcrypt.c.inc
>>   create mode 100644 crypto/akcipher-nettle.c.inc
>>   create mode 100644 crypto/akcipher.c
>>   create mode 100644 crypto/akcipherpriv.h
>>   create mode 100644 crypto/der.c
>>   create mode 100644 crypto/der.h
>>   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
>>   create mode 100644 include/crypto/akcipher.h
>>   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
>>   create mode 100644 tests/unit/test-crypto-der.c
>>
>> -- 
>> 2.11.0
>>
> 
> With regards,
> Daniel