[PATCH v10 1/1] crypto: Introduce RSA algorithm
There are two parts in this patch: 1, support akcipher service by cryptodev-builtin driver 2, virtio-crypto driver supports akcipher service In principle, we should separate this into two patches, to avoid compiling error, merge them into one. Then virtio-crypto gets request from guest side, and forwards the request to builtin driver to handle it. Test with a guest linux: 1, The self-test framework of crypto layer works fine in guest kernel 2, Test with Linux guest(with asym support), the following script test(note that pkey_XXX is supported only in a newer version of keyutils): - both public key & private key - create/close session - encrypt/decrypt/sign/verify basic driver operation - also test with kernel crypto layer(pkey add/query) All the cases work fine. Run script in guest: rm -rf *.der *.pem *.pfx modprobe pkcs8_key_parser # if CONFIG_PKCS8_PRIVATE_KEY_PARSER=m rm -rf /tmp/data dd if=/dev/random of=/tmp/data count=1 bs=20 openssl req -nodes -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -subj "/C=CN/ST=BJ/L=HD/O=qemu/OU=dev/CN=qemu/emailAddress=q...@qemu.org" openssl pkcs8 -in key.pem -topk8 -nocrypt -outform DER -out key.der openssl x509 -in cert.pem -inform PEM -outform DER -out cert.der PRIV_KEY_ID=`cat key.der | keyctl padd asymmetric test_priv_key @s` echo "priv key id = "$PRIV_KEY_ID PUB_KEY_ID=`cat cert.der | keyctl padd asymmetric test_pub_key @s` echo "pub key id = "$PUB_KEY_ID keyctl pkey_query $PRIV_KEY_ID 0 keyctl pkey_query $PUB_KEY_ID 0 echo "Enc with priv key..." keyctl pkey_encrypt $PRIV_KEY_ID 0 /tmp/data enc=pkcs1 >/tmp/enc.priv echo "Dec with pub key..." keyctl pkey_decrypt $PRIV_KEY_ID 0 /tmp/enc.priv enc=pkcs1 >/tmp/dec cmp /tmp/data /tmp/dec echo "Sign with priv key..." keyctl pkey_sign $PRIV_KEY_ID 0 /tmp/data enc=pkcs1 hash=sha1 > /tmp/sig echo "Verify with pub key..." keyctl pkey_verify $PRIV_KEY_ID 0 /tmp/data /tmp/sig enc=pkcs1 hash=sha1 echo "Enc with pub key..." keyctl pkey_encrypt $PUB_KEY_ID 0 /tmp/data enc=pkcs1 >/tmp/enc.pub echo "Dec with priv key..." keyctl pkey_decrypt $PRIV_KEY_ID 0 /tmp/enc.pub enc=pkcs1 >/tmp/dec cmp /tmp/data /tmp/dec echo "Verify with pub key..." keyctl pkey_verify $PUB_KEY_ID 0 /tmp/data /tmp/sig enc=pkcs1 hash=sha1 Reviewed-by: Gonglei Signed-off-by: lei he --- backends/cryptodev-builtin.c | 276 + backends/cryptodev-vhost-user.c | 34 +++- backends/cryptodev.c | 32 ++- hw/virtio/virtio-crypto.c | 323 -- include/hw/virtio/virtio-crypto.h | 5 +- include/sysemu/cryptodev.h| 83 ++-- 6 files changed, 609 insertions(+), 144 deletions(-) diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c index 0671bf9f3e..125cbad1d3 100644 --- a/backends/cryptodev-builtin.c +++ b/backends/cryptodev-builtin.c @@ -26,6 +26,7 @@ #include "qapi/error.h" #include "standard-headers/linux/virtio_crypto.h" #include "crypto/cipher.h" +#include "crypto/akcipher.h" #include "qom/object.h" @@ -42,10 +43,11 @@ typedef struct CryptoDevBackendBuiltinSession { QCryptoCipher *cipher; uint8_t direction; /* encryption or decryption */ uint8_t type; /* cipher? hash? aead? */ +QCryptoAkCipher *akcipher; QTAILQ_ENTRY(CryptoDevBackendBuiltinSession) next; } CryptoDevBackendBuiltinSession; -/* Max number of symmetric sessions */ +/* Max number of symmetric/asymmetric sessions */ #define MAX_NUM_SESSIONS 256 #define CRYPTODEV_BUITLIN_MAX_AUTH_KEY_LEN512 @@ -80,15 +82,17 @@ static void cryptodev_builtin_init( backend->conf.crypto_services = 1u << VIRTIO_CRYPTO_SERVICE_CIPHER | 1u << VIRTIO_CRYPTO_SERVICE_HASH | - 1u << VIRTIO_CRYPTO_SERVICE_MAC; + 1u << VIRTIO_CRYPTO_SERVICE_MAC | + 1u << VIRTIO_CRYPTO_SERVICE_AKCIPHER; backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC; backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1; +backend->conf.akcipher_algo = 1u << VIRTIO_CRYPTO_AKCIPHER_RSA; /* * Set the Maximum length of crypto request. * Why this value? Just avoid to overflow when * memory allocation for each crypto request. */ -backend->conf.max_size = LONG_MAX - sizeof(CryptoDevBackendSymOpInfo); +backend->conf.max_size = LONG_MAX - sizeof(CryptoDevBackendOpInfo); backend->conf.max_cipher_key_len = CRYPTODEV_BUITLIN_MAX_CIPHER_KEY_LEN; backend->conf.max_auth_key_len = CRYPTODEV_BUITLIN_MAX_AUTH_KEY_LEN; @@ -148,6 +152,55 @@ err: return -1; } +static int cryptodev_builtin_get_rsa_hash_algo( +int virtio_rsa_hash, Error **errp) +{ +switch (virtio_rsa_hash) { +case VIRTIO_CRYPTO_RSA_MD5: +return QCRYPTO_HASH_ALG_MD5; + +case VIRTIO_CRYPTO_RSA_SHA1: +return QCRYPTO_HASH_ALG_SHA1; + +case VIRTIO_CRYPTO_RSA_SHA256: +return QCRYPTO_HASH_ALG_SHA2
[PATCH v10 0/1] Introduce akcipher service for virtio-crypto
v9 - v10: - Minor fix of coding style by v9. v8 - v9: - Fix compiling error reported by clang-13/14: opt->hash_alg = cryptodev_builtin_get_rsa_hash_algo(); this leads implicit convertion from 'int' to 'uint32'. 'if (opt->hash_alg < 0)' is always false. Thanks to Philippe Mathieu-Daudé. v7 - v8: - The changes of QEMU crypto has been reviewed & merged by Daniel, remove this part from this series. Thanks to Daniel! - virtio_crypto.h is updated by e4082063e47e ("linux-headers: Update to v5.18-rc6"), remove from this series. - Minor fixes reviewed by Gonglei. Thanks to Gonglei! 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 akci
Re: [PATCH V6 8/9] virtio: harden vring IRQ
On Fri, May 27, 2022 at 02:01:19PM +0800, Jason Wang wrote: > This is a rework on the previous IRQ hardening that is done for > virtio-pci where several drawbacks were found and were reverted: > > 1) try to use IRQF_NO_AUTOEN which is not friendly to affinity managed IRQ >that is used by some device such as virtio-blk > 2) done only for PCI transport > > The vq->broken is re-used in this patch for implementing the IRQ > hardening. The vq->broken is set to true during both initialization > and reset. And the vq->broken is set to false in > virtio_device_ready(). Then vring_interrupt() can check and return > when vq->broken is true. And in this case, switch to return IRQ_NONE > to let the interrupt core aware of such invalid interrupt to prevent > IRQ storm. > > The reason of using a per queue variable instead of a per device one > is that we may need it for per queue reset hardening in the future. > > Note that the hardening is only done for vring interrupt since the > config interrupt hardening is already done in commit 22b7050a024d7 > ("virtio: defer config changed notifications"). But the method that is > used by config interrupt can't be reused by the vring interrupt > handler because it uses spinlock to do the synchronization which is > expensive. > > Cc: Thomas Gleixner > Cc: Peter Zijlstra > Cc: "Paul E. McKenney" > Cc: Marc Zyngier > Cc: Halil Pasic > Cc: Cornelia Huck > Cc: Vineeth Vijayan > Cc: Peter Oberparleiter > Cc: linux-s...@vger.kernel.org > Signed-off-by: Jason Wang Jason, I am really concerned by all the fallout. I propose adding a flag to suppress the hardening - this will be a debugging aid and a work around for users if we find more buggy drivers. suppress_interrupt_hardening ? > --- > drivers/s390/virtio/virtio_ccw.c | 4 > drivers/virtio/virtio.c| 15 --- > drivers/virtio/virtio_mmio.c | 5 + > drivers/virtio/virtio_pci_modern_dev.c | 5 + > drivers/virtio/virtio_ring.c | 11 +++ > include/linux/virtio_config.h | 20 > 6 files changed, 53 insertions(+), 7 deletions(-) > > diff --git a/drivers/s390/virtio/virtio_ccw.c > b/drivers/s390/virtio/virtio_ccw.c > index c188e4f20ca3..97e51c34e6cf 100644 > --- a/drivers/s390/virtio/virtio_ccw.c > +++ b/drivers/s390/virtio/virtio_ccw.c > @@ -971,6 +971,10 @@ static void virtio_ccw_set_status(struct virtio_device > *vdev, u8 status) > ccw->flags = 0; > ccw->count = sizeof(status); > ccw->cda = (__u32)(unsigned long)&vcdev->dma_area->status; > + /* We use ssch for setting the status which is a serializing > + * instruction that guarantees the memory writes have > + * completed before ssch. > + */ > ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_STATUS); > /* Write failed? We assume status is unchanged. */ > if (ret) > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c > index aa1eb5132767..95fac4c97c8b 100644 > --- a/drivers/virtio/virtio.c > +++ b/drivers/virtio/virtio.c > @@ -220,6 +220,15 @@ static int virtio_features_ok(struct virtio_device *dev) > * */ > void virtio_reset_device(struct virtio_device *dev) > { > + /* > + * The below virtio_synchronize_cbs() guarantees that any > + * interrupt for this line arriving after > + * virtio_synchronize_vqs() has completed is guaranteed to see > + * vq->broken as true. > + */ > + virtio_break_device(dev); So make this conditional > + virtio_synchronize_cbs(dev); > + > dev->config->reset(dev); > } > EXPORT_SYMBOL_GPL(virtio_reset_device); > @@ -428,6 +437,9 @@ int register_virtio_device(struct virtio_device *dev) > dev->config_enabled = false; > dev->config_change_pending = false; > > + INIT_LIST_HEAD(&dev->vqs); > + spin_lock_init(&dev->vqs_list_lock); > + > /* We always start by resetting the device, in case a previous >* driver messed it up. This also tests that code path a little. */ > virtio_reset_device(dev); > @@ -435,9 +447,6 @@ int register_virtio_device(struct virtio_device *dev) > /* Acknowledge that we've seen the device. */ > virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE); > > - INIT_LIST_HEAD(&dev->vqs); > - spin_lock_init(&dev->vqs_list_lock); > - > /* >* device_add() causes the bus infrastructure to look for a matching >* driver. > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c > index c9699a59f93c..f9a36bc7ac27 100644 > --- a/drivers/virtio/virtio_mmio.c > +++ b/drivers/virtio/virtio_mmio.c > @@ -253,6 +253,11 @@ static void vm_set_status(struct virtio_device *vdev, u8 > status) > /* We should never be setting status to 0. */ > BUG_ON(status == 0); > > + /* > + * Per memory-barriers.txt, wmb() is not needed to guarantee > + * that the the cache coherent memory writes have completed >
Re: [PATCH v9 1/1] crypto: Introduce RSA algorithm
On Sat, Jun 11, 2022 at 11:36:58AM +0800, zhenwei pi wrote: > There are two parts in this patch: > 1, support akcipher service by cryptodev-builtin driver > 2, virtio-crypto driver supports akcipher service > > In principle, we should separate this into two patches, to avoid > compiling error, merge them into one. > > Then virtio-crypto gets request from guest side, and forwards the > request to builtin driver to handle it. > > Test with a guest linux: > 1, The self-test framework of crypto layer works fine in guest kernel > 2, Test with Linux guest(with asym support), the following script > test(note that pkey_XXX is supported only in a newer version of keyutils): > - both public key & private key > - create/close session > - encrypt/decrypt/sign/verify basic driver operation > - also test with kernel crypto layer(pkey add/query) > > All the cases work fine. > > Run script in guest: > rm -rf *.der *.pem *.pfx > modprobe pkcs8_key_parser # if CONFIG_PKCS8_PRIVATE_KEY_PARSER=m > rm -rf /tmp/data > dd if=/dev/random of=/tmp/data count=1 bs=20 > > openssl req -nodes -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -subj > "/C=CN/ST=BJ/L=HD/O=qemu/OU=dev/CN=qemu/emailAddress=q...@qemu.org" > openssl pkcs8 -in key.pem -topk8 -nocrypt -outform DER -out key.der > openssl x509 -in cert.pem -inform PEM -outform DER -out cert.der > > PRIV_KEY_ID=`cat key.der | keyctl padd asymmetric test_priv_key @s` > echo "priv key id = "$PRIV_KEY_ID > PUB_KEY_ID=`cat cert.der | keyctl padd asymmetric test_pub_key @s` > echo "pub key id = "$PUB_KEY_ID > > keyctl pkey_query $PRIV_KEY_ID 0 > keyctl pkey_query $PUB_KEY_ID 0 > > echo "Enc with priv key..." > keyctl pkey_encrypt $PRIV_KEY_ID 0 /tmp/data enc=pkcs1 >/tmp/enc.priv > echo "Dec with pub key..." > keyctl pkey_decrypt $PRIV_KEY_ID 0 /tmp/enc.priv enc=pkcs1 >/tmp/dec > cmp /tmp/data /tmp/dec > > echo "Sign with priv key..." > keyctl pkey_sign $PRIV_KEY_ID 0 /tmp/data enc=pkcs1 hash=sha1 > /tmp/sig > echo "Verify with pub key..." > keyctl pkey_verify $PRIV_KEY_ID 0 /tmp/data /tmp/sig enc=pkcs1 hash=sha1 > > echo "Enc with pub key..." > keyctl pkey_encrypt $PUB_KEY_ID 0 /tmp/data enc=pkcs1 >/tmp/enc.pub > echo "Dec with priv key..." > keyctl pkey_decrypt $PRIV_KEY_ID 0 /tmp/enc.pub enc=pkcs1 >/tmp/dec > cmp /tmp/data /tmp/dec > > echo "Verify with pub key..." > keyctl pkey_verify $PUB_KEY_ID 0 /tmp/data /tmp/sig enc=pkcs1 hash=sha1 > > Reviewed-by: Gonglei > Signed-off-by: lei he Signed-off-by: zhenwei pi > --- > backends/cryptodev-builtin.c | 275 + > backends/cryptodev-vhost-user.c | 34 +++- > backends/cryptodev.c | 32 ++- > hw/virtio/virtio-crypto.c | 323 -- > include/hw/virtio/virtio-crypto.h | 5 +- > include/sysemu/cryptodev.h| 83 ++-- > 6 files changed, 608 insertions(+), 144 deletions(-) > > diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c > index 0671bf9f3e..ed73ea789b 100644 > --- a/backends/cryptodev-builtin.c > +++ b/backends/cryptodev-builtin.c > @@ -26,6 +26,7 @@ > #include "qapi/error.h" > #include "standard-headers/linux/virtio_crypto.h" > #include "crypto/cipher.h" > +#include "crypto/akcipher.h" > #include "qom/object.h" > > > @@ -42,10 +43,11 @@ typedef struct CryptoDevBackendBuiltinSession { > QCryptoCipher *cipher; > uint8_t direction; /* encryption or decryption */ > uint8_t type; /* cipher? hash? aead? */ > +QCryptoAkCipher *akcipher; > QTAILQ_ENTRY(CryptoDevBackendBuiltinSession) next; > } CryptoDevBackendBuiltinSession; > > -/* Max number of symmetric sessions */ > +/* Max number of symmetric/asymmetric sessions */ > #define MAX_NUM_SESSIONS 256 > > #define CRYPTODEV_BUITLIN_MAX_AUTH_KEY_LEN512 > @@ -80,15 +82,17 @@ static void cryptodev_builtin_init( > backend->conf.crypto_services = > 1u << VIRTIO_CRYPTO_SERVICE_CIPHER | > 1u << VIRTIO_CRYPTO_SERVICE_HASH | > - 1u << VIRTIO_CRYPTO_SERVICE_MAC; > + 1u << VIRTIO_CRYPTO_SERVICE_MAC | > + 1u << VIRTIO_CRYPTO_SERVICE_AKCIPHER; > backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC; > backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1; > +backend->conf.akcipher_algo = 1u << VIRTIO_CRYPTO_AKCIPHER_RSA; > /* > * Set the Maximum length of crypto request. > * Why this value? Just avoid to overflow when > * memory allocation for each crypto request. > */ > -backend->conf.max_size = LONG_MAX - sizeof(CryptoDevBackendSymOpInfo); > +backend->conf.max_size = LONG_MAX - sizeof(CryptoDevBackendOpInfo); > backend->conf.max_cipher_key_len = CRYPTODEV_BUITLIN_MAX_CIPHER_KEY_LEN; > backend->conf.max_auth_key_len = CRYPTODEV_BUITLIN_MAX_AUTH_KEY_LEN; > > @@ -148,6 +152,54 @@ err: > return -1; > } > > +static int cryp
[PATCH v9 1/1] crypto: Introduce RSA algorithm
There are two parts in this patch: 1, support akcipher service by cryptodev-builtin driver 2, virtio-crypto driver supports akcipher service In principle, we should separate this into two patches, to avoid compiling error, merge them into one. Then virtio-crypto gets request from guest side, and forwards the request to builtin driver to handle it. Test with a guest linux: 1, The self-test framework of crypto layer works fine in guest kernel 2, Test with Linux guest(with asym support), the following script test(note that pkey_XXX is supported only in a newer version of keyutils): - both public key & private key - create/close session - encrypt/decrypt/sign/verify basic driver operation - also test with kernel crypto layer(pkey add/query) All the cases work fine. Run script in guest: rm -rf *.der *.pem *.pfx modprobe pkcs8_key_parser # if CONFIG_PKCS8_PRIVATE_KEY_PARSER=m rm -rf /tmp/data dd if=/dev/random of=/tmp/data count=1 bs=20 openssl req -nodes -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -subj "/C=CN/ST=BJ/L=HD/O=qemu/OU=dev/CN=qemu/emailAddress=q...@qemu.org" openssl pkcs8 -in key.pem -topk8 -nocrypt -outform DER -out key.der openssl x509 -in cert.pem -inform PEM -outform DER -out cert.der PRIV_KEY_ID=`cat key.der | keyctl padd asymmetric test_priv_key @s` echo "priv key id = "$PRIV_KEY_ID PUB_KEY_ID=`cat cert.der | keyctl padd asymmetric test_pub_key @s` echo "pub key id = "$PUB_KEY_ID keyctl pkey_query $PRIV_KEY_ID 0 keyctl pkey_query $PUB_KEY_ID 0 echo "Enc with priv key..." keyctl pkey_encrypt $PRIV_KEY_ID 0 /tmp/data enc=pkcs1 >/tmp/enc.priv echo "Dec with pub key..." keyctl pkey_decrypt $PRIV_KEY_ID 0 /tmp/enc.priv enc=pkcs1 >/tmp/dec cmp /tmp/data /tmp/dec echo "Sign with priv key..." keyctl pkey_sign $PRIV_KEY_ID 0 /tmp/data enc=pkcs1 hash=sha1 > /tmp/sig echo "Verify with pub key..." keyctl pkey_verify $PRIV_KEY_ID 0 /tmp/data /tmp/sig enc=pkcs1 hash=sha1 echo "Enc with pub key..." keyctl pkey_encrypt $PUB_KEY_ID 0 /tmp/data enc=pkcs1 >/tmp/enc.pub echo "Dec with priv key..." keyctl pkey_decrypt $PRIV_KEY_ID 0 /tmp/enc.pub enc=pkcs1 >/tmp/dec cmp /tmp/data /tmp/dec echo "Verify with pub key..." keyctl pkey_verify $PUB_KEY_ID 0 /tmp/data /tmp/sig enc=pkcs1 hash=sha1 Reviewed-by: Gonglei Signed-off-by: lei he --- backends/cryptodev-builtin.c | 275 + backends/cryptodev-vhost-user.c | 34 +++- backends/cryptodev.c | 32 ++- hw/virtio/virtio-crypto.c | 323 -- include/hw/virtio/virtio-crypto.h | 5 +- include/sysemu/cryptodev.h| 83 ++-- 6 files changed, 608 insertions(+), 144 deletions(-) diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c index 0671bf9f3e..ed73ea789b 100644 --- a/backends/cryptodev-builtin.c +++ b/backends/cryptodev-builtin.c @@ -26,6 +26,7 @@ #include "qapi/error.h" #include "standard-headers/linux/virtio_crypto.h" #include "crypto/cipher.h" +#include "crypto/akcipher.h" #include "qom/object.h" @@ -42,10 +43,11 @@ typedef struct CryptoDevBackendBuiltinSession { QCryptoCipher *cipher; uint8_t direction; /* encryption or decryption */ uint8_t type; /* cipher? hash? aead? */ +QCryptoAkCipher *akcipher; QTAILQ_ENTRY(CryptoDevBackendBuiltinSession) next; } CryptoDevBackendBuiltinSession; -/* Max number of symmetric sessions */ +/* Max number of symmetric/asymmetric sessions */ #define MAX_NUM_SESSIONS 256 #define CRYPTODEV_BUITLIN_MAX_AUTH_KEY_LEN512 @@ -80,15 +82,17 @@ static void cryptodev_builtin_init( backend->conf.crypto_services = 1u << VIRTIO_CRYPTO_SERVICE_CIPHER | 1u << VIRTIO_CRYPTO_SERVICE_HASH | - 1u << VIRTIO_CRYPTO_SERVICE_MAC; + 1u << VIRTIO_CRYPTO_SERVICE_MAC | + 1u << VIRTIO_CRYPTO_SERVICE_AKCIPHER; backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC; backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1; +backend->conf.akcipher_algo = 1u << VIRTIO_CRYPTO_AKCIPHER_RSA; /* * Set the Maximum length of crypto request. * Why this value? Just avoid to overflow when * memory allocation for each crypto request. */ -backend->conf.max_size = LONG_MAX - sizeof(CryptoDevBackendSymOpInfo); +backend->conf.max_size = LONG_MAX - sizeof(CryptoDevBackendOpInfo); backend->conf.max_cipher_key_len = CRYPTODEV_BUITLIN_MAX_CIPHER_KEY_LEN; backend->conf.max_auth_key_len = CRYPTODEV_BUITLIN_MAX_AUTH_KEY_LEN; @@ -148,6 +152,54 @@ err: return -1; } +static int cryptodev_builtin_get_rsa_hash_algo( +int virtio_rsa_hash, Error **errp) +{ +switch (virtio_rsa_hash) { +case VIRTIO_CRYPTO_RSA_MD5: +return QCRYPTO_HASH_ALG_MD5; + +case VIRTIO_CRYPTO_RSA_SHA1: +return QCRYPTO_HASH_ALG_SHA1; + +case VIRTIO_CRYPTO_RSA_SHA256: +return QCRYPTO_HASH_ALG_SHA2
[PATCH v9 0/1] Introduce akcipher service for virtio-crypto
v8 - v9: - Fix compiling error reported by clang-13/14: opt->hash_alg = cryptodev_builtin_get_rsa_hash_algo(); this leads implicit convertion from 'int' to 'uint32'. 'if (opt->hash_alg < 0)' is always false. Thanks to Philippe Mathieu-Daudé. v7 - v8: - The changes of QEMU crypto has been reviewed & merged by Daniel, remove this part from this series. Thanks to Daniel! - virtio_crypto.h is updated by e4082063e47e ("linux-headers: Update to v5.18-rc6"), remove from this series. - Minor fixes reviewed by Gonglei. Thanks to Gonglei! 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
Re: [External] Re: [PATCH] virtio_ring : fix vring_packed_desc memory out of bounds bug
On Sat, Jun 11, 2022 at 12:38:10AM +0800, 黄杰 wrote: > > This pattern was always iffy, but I don't think the patch > > improves the situation much. last_used_idx and vq->packed.used_wrap_counter > > can still get out of sync. > > Yes, You are absolutely correct, thanks for pointing out this issue, I > didn't take that into consideration, > how about disabling interrupts before this code below: > > > vq->last_used_idx += vq->packed.desc_state[id].num; > > if (unlikely(vq->last_used_idx >= vq->packed.vring.num)) { > > vq->last_used_idx -= vq->packed.vring.num; > > vq->packed.used_wrap_counter ^= 1; > > } > > it seems to be fine to just turn off the interrupts of the current vring. > > BR That would make datapath significantly slower. > > Michael S. Tsirkin 于2022年6月10日周五 22:50写道: > > > > On Fri, Jun 10, 2022 at 06:33:14PM +0800, huangjie.albert wrote: > > > ksoftirqd may consume the packet and it will call: > > > virtnet_poll > > > -->virtnet_receive > > > -->virtqueue_get_buf_ctx > > > -->virtqueue_get_buf_ctx_packed > > > and in virtqueue_get_buf_ctx_packed: > > > > > > vq->last_used_idx += vq->packed.desc_state[id].num; > > > if (unlikely(vq->last_used_idx >= vq->packed.vring.num)) { > > > vq->last_used_idx -= vq->packed.vring.num; > > > vq->packed.used_wrap_counter ^= 1; > > > } > > > > > > if at the same time, there comes a vring interrupt,in vring_interrupt: > > > we will call: > > > vring_interrupt > > > -->more_used > > > -->more_used_packed > > > -->is_used_desc_packed > > > in is_used_desc_packed, the last_used_idx maybe >= vq->packed.vring.num. > > > so this could case a memory out of bounds bug. > > > > > > this patch is to fix this. > > > > > > Signed-off-by: huangjie.albert > > > > > > This pattern was always iffy, but I don't think the patch > > improves the situation much. last_used_idx and vq->packed.used_wrap_counter > > can still get out of sync. > > > > Maybe refactor code to keep everything in vq->last_used_idx? > > > > Jason what is your take? > > > > > > > > > > > > > --- > > > drivers/virtio/virtio_ring.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > > > index 13a7348cedff..d2abbb3a8187 100644 > > > --- a/drivers/virtio/virtio_ring.c > > > +++ b/drivers/virtio/virtio_ring.c > > > @@ -1397,6 +1397,9 @@ static inline bool is_used_desc_packed(const struct > > > vring_virtqueue *vq, > > > bool avail, used; > > > u16 flags; > > > > > > + if (idx >= vq->packed.vring.num) > > > + return false; > > > + > > > flags = le16_to_cpu(vq->packed.vring.desc[idx].flags); > > > avail = !!(flags & (1 << VRING_PACKED_DESC_F_AVAIL)); > > > used = !!(flags & (1 << VRING_PACKED_DESC_F_USED)); > > > -- > > > 2.27.0 > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC v1 4/7] swiotlb: to implement io_tlb_high_mem
Hi Christoph, On 6/8/22 10:05 PM, Christoph Hellwig wrote: > All this really needs to be hidden under the hood. > Since this patch file has 200+ lines, would you please help clarify what does 'this' indicate? The idea of this patch: 1. Convert the functions to initialize swiotlb into callee. The callee accepts 'true' or 'false' as arguments to indicate whether it is for default or new swiotlb buffer, e.g., swiotlb_init_remap() into __swiotlb_init_remap(). 2. At the caller side to decide if we are going to call the callee to create the extra buffer. Do you mean the callee if still *too high level* and all the decision/allocation/initialization should be down at *lower level functions*? E.g., if I re-work the "struct io_tlb_mem" to make the 64-bit buffer as the 2nd array of io_tlb_mem->slots[32_or_64][index], the user will only notice it is the default 'io_tlb_default_mem', but will not be able to know if it is allocated from 32-bit or 64-bit buffer. Thank you very much for the feedback. Dongli Zhang ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio_ring : fix vring_packed_desc memory out of bounds bug
On Fri, Jun 10, 2022 at 06:33:14PM +0800, huangjie.albert wrote: > ksoftirqd may consume the packet and it will call: > virtnet_poll > -->virtnet_receive > -->virtqueue_get_buf_ctx > -->virtqueue_get_buf_ctx_packed > and in virtqueue_get_buf_ctx_packed: > > vq->last_used_idx += vq->packed.desc_state[id].num; > if (unlikely(vq->last_used_idx >= vq->packed.vring.num)) { > vq->last_used_idx -= vq->packed.vring.num; > vq->packed.used_wrap_counter ^= 1; > } > > if at the same time, there comes a vring interrupt,in vring_interrupt: > we will call: > vring_interrupt > -->more_used > -->more_used_packed > -->is_used_desc_packed > in is_used_desc_packed, the last_used_idx maybe >= vq->packed.vring.num. > so this could case a memory out of bounds bug. > > this patch is to fix this. > > Signed-off-by: huangjie.albert This pattern was always iffy, but I don't think the patch improves the situation much. last_used_idx and vq->packed.used_wrap_counter can still get out of sync. Maybe refactor code to keep everything in vq->last_used_idx? Jason what is your take? > --- > drivers/virtio/virtio_ring.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index 13a7348cedff..d2abbb3a8187 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -1397,6 +1397,9 @@ static inline bool is_used_desc_packed(const struct > vring_virtqueue *vq, > bool avail, used; > u16 flags; > > + if (idx >= vq->packed.vring.num) > + return false; > + > flags = le16_to_cpu(vq->packed.vring.desc[idx].flags); > avail = !!(flags & (1 << VRING_PACKED_DESC_F_AVAIL)); > used = !!(flags & (1 << VRING_PACKED_DESC_F_USED)); > -- > 2.27.0 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 03/19] fs: Add aops->migrate_folio
On 09.06.22 16:35, Matthew Wilcox wrote: > On Thu, Jun 09, 2022 at 02:50:20PM +0200, David Hildenbrand wrote: >> On 08.06.22 17:02, Matthew Wilcox (Oracle) wrote: >>> diff --git a/Documentation/filesystems/locking.rst >>> b/Documentation/filesystems/locking.rst >>> index c0fe711f14d3..3d28b23676bd 100644 >>> --- a/Documentation/filesystems/locking.rst >>> +++ b/Documentation/filesystems/locking.rst >>> @@ -253,7 +253,8 @@ prototypes:: >>> void (*free_folio)(struct folio *); >>> int (*direct_IO)(struct kiocb *, struct iov_iter *iter); >>> bool (*isolate_page) (struct page *, isolate_mode_t); >>> - int (*migratepage)(struct address_space *, struct page *, struct page >>> *); >>> + int (*migrate_folio)(struct address_space *, struct folio *dst, >>> + struct folio *src, enum migrate_mode); >>> void (*putback_page) (struct page *); >> >> isolate_page/putback_page are leftovers from the previous patch, no? > > Argh, right, I completely forgot I needed to update the documentation in > that patch. > >>> +++ b/Documentation/vm/page_migration.rst >>> @@ -181,22 +181,23 @@ which are function pointers of struct >>> address_space_operations. >>> Once page is successfully isolated, VM uses page.lru fields so driver >>> shouldn't expect to preserve values in those fields. >>> >>> -2. ``int (*migratepage) (struct address_space *mapping,`` >>> -| ``struct page *newpage, struct page *oldpage, enum migrate_mode);`` >>> - >>> - After isolation, VM calls migratepage() of driver with the isolated >>> page. >>> - The function of migratepage() is to move the contents of the old page >>> to the >>> - new page >>> - and set up fields of struct page newpage. Keep in mind that you should >>> - indicate to the VM the oldpage is no longer movable via >>> __ClearPageMovable() >>> - under page_lock if you migrated the oldpage successfully and returned >>> - MIGRATEPAGE_SUCCESS. If driver cannot migrate the page at the moment, >>> driver >>> - can return -EAGAIN. On -EAGAIN, VM will retry page migration in a short >>> time >>> - because VM interprets -EAGAIN as "temporary migration failure". On >>> returning >>> - any error except -EAGAIN, VM will give up the page migration without >>> - retrying. >>> - >>> - Driver shouldn't touch the page.lru field while in the migratepage() >>> function. >>> +2. ``int (*migrate_folio) (struct address_space *mapping,`` >>> +| ``struct folio *dst, struct folio *src, enum migrate_mode);`` >>> + >>> + After isolation, VM calls the driver's migrate_folio() with the >>> + isolated folio. The purpose of migrate_folio() is to move the contents >>> + of the source folio to the destination folio and set up the fields >>> + of destination folio. Keep in mind that you should indicate to the >>> + VM the source folio is no longer movable via __ClearPageMovable() >>> + under folio if you migrated the source successfully and returned >>> + MIGRATEPAGE_SUCCESS. If driver cannot migrate the folio at the >>> + moment, driver can return -EAGAIN. On -EAGAIN, VM will retry folio >>> + migration in a short time because VM interprets -EAGAIN as "temporary >>> + migration failure". On returning any error except -EAGAIN, VM will >>> + give up the folio migration without retrying. >>> + >>> + Driver shouldn't touch the folio.lru field while in the migrate_folio() >>> + function. >>> >>> 3. ``void (*putback_page)(struct page *);`` >> >> Hmm, here it's a bit more complicated now, because we essentially have >> two paths: LRU+migrate_folio or !LRU+movable_ops >> (isolate/migrate/putback page) > > Oh ... actually, this is just documenting the driver side of things. > I don't really like how it's written. Here, have some rewritten > documentation (which is now part of the previous patch): > LGTM, thanks. -- Thanks, David / dhildenb ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v1] drivers/virtio: Clarify CONFIG_VIRTIO_MEM for unsupported architectures
Let's make it clearer that simply unlocking CONFIG_VIRTIO_MEM on an architecture is most probably not sufficient to have it working as expected. Cc: "Michael S. Tsirkin" Cc: Jason Wang Cc: Gavin Shan Signed-off-by: David Hildenbrand --- drivers/virtio/Kconfig | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index b5adf6abd241..f86b6a988b63 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -115,9 +115,11 @@ config VIRTIO_MEM This driver provides access to virtio-mem paravirtualized memory devices, allowing to hotplug and hotunplug memory. -This driver was only tested under x86-64 and arm64, but should -theoretically work on all architectures that support memory hotplug -and hotremove. +This driver currently only supports x86-64 and arm64. Although it +should compile on other architectures that implement memory +hot(un)plug, architecture-specific and/or common +code changes may be required for virtio-mem, kdump and kexec to work as +expected. If unsure, say M. -- 2.35.3 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization