mbox series

[v4,0/8] Introduce akcipher service for virtio-crypto

Message ID 20220411104327.197048-1-pizhenwei@bytedance.com
Headers show
Series Introduce akcipher service for virtio-crypto | expand

Message

zhenwei pi April 11, 2022, 10:43 a.m. UTC
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 (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

 backends/cryptodev-builtin.c                  | 261 ++++++-
 backends/cryptodev-vhost-user.c               |  34 +-
 backends/cryptodev.c                          |  32 +-
 crypto/akcipher-gcrypt.c.inc                  | 531 +++++++++++++
 crypto/akcipher-nettle.c.inc                  | 448 +++++++++++
 crypto/akcipher.c                             | 108 +++
 crypto/akcipherpriv.h                         |  43 ++
 crypto/asn1_decoder.c                         | 161 ++++
 crypto/asn1_decoder.h                         |  75 ++
 crypto/meson.build                            |   6 +
 crypto/rsakey-builtin.c.inc                   | 150 ++++
 crypto/rsakey-nettle.c.inc                    | 141 ++++
 crypto/rsakey.c                               |  43 ++
 crypto/rsakey.h                               |  96 +++
 hw/virtio/virtio-crypto.c                     | 323 ++++++--
 include/crypto/akcipher.h                     | 151 ++++
 include/hw/virtio/virtio-crypto.h             |   5 +-
 .../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       | 161 ++++
 tests/bench/meson.build                       |   4 +
 tests/bench/test_akcipher_keys.inc            | 537 +++++++++++++
 tests/unit/meson.build                        |   2 +
 tests/unit/test-crypto-akcipher.c             | 708 ++++++++++++++++++
 tests/unit/test-crypto-asn1-decoder.c         | 289 +++++++
 27 files changed, 4404 insertions(+), 145 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/asn1_decoder.c
 create mode 100644 crypto/asn1_decoder.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-asn1-decoder.c

Comments

Paolo Bonzini April 12, 2022, 9:47 a.m. UTC | #1
> In our plan, the feature is designed for HTTPS offloading case and
> other applications which use kernel RSA/ecdsa by keyctl syscall.

Hi Zhenwei,

what is the % of time spent doing asymmetric key operations in your
benchmark?  I am not very familiar with crypto acceleration but my
understanding has always been that most time is spent doing either
hashing (for signing) or symmetric key operations (for encryption).

If I understand correctly, without support for acceleration these 
patches are more of a demonstration of virtio-crypto, or usable for 
testing purposes.

Would it be possible to extend virtio-crypto to use keys already in the
host keyctl, or in a PKCS#11 smartcard, so that virtio-crypto could also
provide the functionality of an HSM?  Or does the standard require that
the keys are provided by the guest?

Paolo
zhenwei pi April 12, 2022, 10:21 a.m. UTC | #2
On 4/12/22 17:47, Paolo Bonzini wrote:
> 
>> In our plan, the feature is designed for HTTPS offloading case and
>> other applications which use kernel RSA/ecdsa by keyctl syscall.
> 
> Hi Zhenwei,
> 
> what is the % of time spent doing asymmetric key operations in your
> benchmark?  I am not very familiar with crypto acceleration but my
> understanding has always been that most time is spent doing either
> hashing (for signing) or symmetric key operations (for encryption).
> 
> If I understand correctly, without support for acceleration these 
> patches are more of a demonstration of virtio-crypto, or usable for 
> testing purposes.
> 

Hi, Paolo

This is the perf result of nginx+openssl CPU calculation, the heavy load 
from openssl uses the most time(as same as you mentioned).
27.37%    26.00%  nginx            libcrypto.so.1.1          [.] 
__bn_sqrx8x_reduction
20.58%    19.52%  nginx            libcrypto.so.1.1          [.] 
mulx4x_internal
16.73%    15.89%  nginx            libcrypto.so.1.1          [.] 
bn_sqrx8x_internal
  8.79%     0.00%  nginx            [unknown]                 [k] 
0000000000000000
  7.26%     0.00%  nginx            [unknown]                 [.] 
0x89388669992a0cbc
  7.00%     0.00%  nginx            [unknown]                 [k] 
0x45f0e480d5f2a58e
  6.76%     0.02%  nginx            [kernel.kallsyms]         [k] 
entry_SYSCALL_64_after_hwframe
  6.74%     0.02%  nginx            [kernel.kallsyms]         [k] 
do_syscall_64
  6.61%     0.00%  nginx            [unknown]                 [.] 
0xa75a60d7820f9ffb
  6.47%     0.00%  nginx            [unknown]                 [k] 
0xe91223f6da36254c
  5.51%     0.01%  nginx            [kernel.kallsyms]         [k] 
asm_common_interrupt
  5.46%     0.01%  nginx            [kernel.kallsyms]         [k] 
common_interrupt
  5.16%     0.04%  nginx            [kernel.kallsyms]         [k] 
__softirqentry_text_start
  4.92%     0.01%  nginx            [kernel.kallsyms]         [k] 
irq_exit_rcu
  4.91%     0.04%  nginx            [kernel.kallsyms]         [k] 
net_rx_action


This is the result of nginx+openssl keyctl offload(virtio crypto + host 
keyctl + Intel QAT):
30.38%     0.08%  nginx            [kernel.kallsyms]         [k] 
entry_SYSCALL_64_after_hwframe
30.29%     0.07%  nginx            [kernel.kallsyms]         [k] 
do_syscall_64
23.84%     0.00%  nginx            [unknown]                 [k] 
0000000000000000
14.24%     0.03%  nginx            [kernel.kallsyms]         [k] 
asm_common_interrupt
14.06%     0.05%  nginx            [kernel.kallsyms]         [k] 
common_interrupt
12.99%     0.11%  nginx            [kernel.kallsyms]         [k] 
__softirqentry_text_start
12.27%     0.12%  nginx            [kernel.kallsyms]         [k] 
net_rx_action
12.13%     0.03%  nginx            [kernel.kallsyms]         [k] __napi_poll
12.06%     0.06%  nginx            [kernel.kallsyms]         [k] 
irq_exit_rcu
10.49%     0.14%  nginx            libssl.so.1.1             [.] 
tls_process_client_key_exchange
10.21%     0.12%  nginx            [virtio_net]              [k] 
virtnet_poll
10.13%     0.04%  nginx            libc-2.28.so              [.] syscall
10.12%     0.03%  nginx            kctl-engine.so            [.] 
kctl_rsa_priv_dec
10.02%     0.02%  nginx            kctl-engine.so            [.] 
kctl_hw_rsa_priv_func
  9.98%     0.01%  nginx            libkeyutils.so.1.10       [.] 
keyctl_pkey_decrypt
  9.95%     0.02%  nginx            libkeyutils.so.1.10       [.] keyctl
  9.77%     0.03%  nginx            [kernel.kallsyms]         [k] 
keyctl_pkey_e_d_s
  8.97%     0.00%  nginx            [unknown]                 [k] 
0x00007f4adbb81f0b
  8.78%     0.08%  nginx            libpthread-2.28.so        [.] 
__libc_write
  8.49%     0.05%  nginx            [kernel.kallsyms]         [k] 
netif_receive_skb_list_internal

The RSA part gets reduced, and the QPS of https improves to ~200%.

Something may be ignored in this cover letter:
[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.
-> Yes, this patch is a demonstration of virtio-crypto.

[5] keyctl backend is in development, we will post this feature in 
Q2-2022. keyctl backend can use hardware acceleration(Ex, Intel QAT).
-> This is our plan. Currently it's still in developing.


> Would it be possible to extend virtio-crypto to use keys already in the
> host keyctl, or in a PKCS#11 smartcard, so that virtio-crypto could also
> provide the functionality of an HSM?  Or does the standard require that
> the keys are provided by the guest?
> 
> Paolo

I'm very interested in this, I'll try in Q3-2022 or later.
Daniel P. Berrangé April 21, 2022, 8:10 a.m. UTC | #3
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 <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 (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
> > 
> >   backends/cryptodev-builtin.c                  | 261 ++++++-
> >   backends/cryptodev-vhost-user.c               |  34 +-
> >   backends/cryptodev.c                          |  32 +-
> >   crypto/akcipher-gcrypt.c.inc                  | 531 +++++++++++++
> >   crypto/akcipher-nettle.c.inc                  | 448 +++++++++++
> >   crypto/akcipher.c                             | 108 +++
> >   crypto/akcipherpriv.h                         |  43 ++
> >   crypto/asn1_decoder.c                         | 161 ++++
> >   crypto/asn1_decoder.h                         |  75 ++
> >   crypto/meson.build                            |   6 +
> >   crypto/rsakey-builtin.c.inc                   | 150 ++++
> >   crypto/rsakey-nettle.c.inc                    | 141 ++++
> >   crypto/rsakey.c                               |  43 ++
> >   crypto/rsakey.h                               |  96 +++
> >   hw/virtio/virtio-crypto.c                     | 323 ++++++--
> >   include/crypto/akcipher.h                     | 151 ++++
> >   include/hw/virtio/virtio-crypto.h             |   5 +-
> >   .../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       | 161 ++++
> >   tests/bench/meson.build                       |   4 +
> >   tests/bench/test_akcipher_keys.inc            | 537 +++++++++++++
> >   tests/unit/meson.build                        |   2 +
> >   tests/unit/test-crypto-akcipher.c             | 708 ++++++++++++++++++
> >   tests/unit/test-crypto-asn1-decoder.c         | 289 +++++++
> >   27 files changed, 4404 insertions(+), 145 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/asn1_decoder.c
> >   create mode 100644 crypto/asn1_decoder.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-asn1-decoder.c
> > 
> 
> -- 
> zhenwei pi
> 

With regards,
Daniel