Re: [Qemu-devel] [RFC v1] virtio-crypto specification

[Michael S. Tsirkin]

Thanks, looks good overall.
You might want to join the TC if you maintain a device.
Generally, I think this needs a bit more formal conformance
statements.
Some examples below.

On Fri, Nov 20, 2015 at 03:27:51AM +0000, Gonglei (Arei) wrote:
> Hi guys,
> 
> After initial discussion at this year's KVM forum, I post the RFC version of 
> virtio-crypto
> device specification now. 
> 
> If you have any comments, please let me know, thanks.
> 
> Regards,
> -Gonglei
> 
> 
> 1     Crypto Device
> 
> The virtio crypto device is a virtual crypto device (ie. hardware crypto 
> accelerator card). Encrypt and decrypt requests are placed in the data queue, 
> and handled by the real hardware crypto accelerators finally. A second queue 
> is the controlling queue, which is used to create/destroy session or some 
> other advanced filtering features.
> 
> 1.1   Device ID
> 
>       65535 (experimental)
> 
> 1.2   Virtqueues
> 
> 0 
>       controlq
> 1
>       dataq
> 
> 1.3   Feature bits
> 
> VIRTIO_CRYPTO_F_REQ_SIZE_MAX (0)      
> Maximum size of any single request is in “size_max”. 
> VIRTIO_CRYPTO_F_SYM (1)
> Device supports the symmetric cryptography API.
> VIRTIO_CRYPTO_F_DH (2)
> Device supports the Diffie Hellman API.
> VIRTIO_CRYPTO_F_DSA (3)
> Device supports the DSA API.
> VIRTIO_CRYPTO_F_RSA (4)
> Device supports the RSA API.
> VIRTIO_CRYPTO_F_EC (5)
> Device supports the Elliptic Curve API.
> VIRTIO_CRYPTO_F_ECDH (6)
> Device supports the Elliptic Curve Diffie Hellman API.
> VIRTIO_CRYPTO_F_ECDSA (7)
> Device supports the Elliptic Curve DSA API.
> VIRTIO_CRYPTO_F _KEY (8)
> Device supports the Key Generation API.
> VIRTIO_CRYPTO_F_LN (9)
> Device supports the Large Number API.
> VIRTIO_CRYPTO_F_PRIME (10)
> Device supports the prime number testing API.
> VIRTIO_CRYPTO_F_DRGB (11)
> Device supports the DRGB API.
> VIRTIO_CRYPTO_F_NRGB (12)
> Device supports the NRGB API.
> VIRTIO_CRYPTO_F_RAND (13)
> Device supports the random bit/number generation API.
> 
> 1.4   Device configuration layout
> 
> struct virtio_crypto_config {
>       le32 size_max; /* Maximum size of any single request */
> }
> 
> 1.5   Device Initialization
> 
> 1. The initialization routine should identify the data and control virtqueues.
> 2. If the VIRTIO_CRYPTO_F_SYM feature bit is negotiated, identify the device 
> supports the symmetric cryptography API, which as the same as other features.
> 
> 1.6   Device Operation
> 
> The controlq is used to control session operations, such as create or
> destroy. Meanwhile, some other features or functions can also be
> handled by controlq.

In future versions of the specification?

> The control request is preceded by a header:
> struct virtio_crypto_ctx_outhdr {
>     /* cipher algorithm type (ie. aes-cbc ) */
>     __virtio32 alg;
>     /* length of key */
>     __virtio32 keylen;
>     /* reserved */
>     __virtio32 flags;
>     /* control type  */
>     uint8_t type;
>     /* encrypt or decrypt */
>     uint8_t op;
>     /* mode of hash operation, including authenticated/plain/nested hash */
>     uint8_t hash_mode;
>     /* authenticate hash/cipher ordering  */
>     uint8_t alg_chain_order;
>     /* length of authenticated key */
>     __virtio32 auth_key_len;
>     /* hash algorithm type */
>     __virtio32 hash_alg;

You can make this all le too: I don't think we need to
support legacy devices of this type.
Spec also does not use uint8_t.

> };
> The encrypt/decrypt requests and the corresponding results are transmitted by 
> placing them in dataq. The request itself is preceded by a header:
> struct virtio_crypto_req_outhdr {
>     /* algorithm type (ie. aes-128-cbc ) */
>     __virtio32 mode;
>     /* length of iv */
>     __virtio32 ivlen;
>     /* length of source data */
>     __virtio32 len;
>     /* length of auth data */
>     __virtio32 auth_len;
>     /* the backend session id */
>     __virtio64 session_id;
>     /* reserved */
>     __virtio32 flags;
> };
> 
> Both ctx and data requests end by a status byte. The final status byte is 
> written by the device: either VIRTIO_CRYPTO_S_OK for success, 
> VIRTIO_BLK_S_IOERR for device or driver error or VIRTIO_BLK_S_UNSUPP for a 
> request unsupported by device, VIRTIO_CRYPTO_S_BADMSG for verification failed 
> when decrypt AEAD algorithms:
> 
> #define VIRTIO_CRYPTO_S_OK    0
> #define VIRTIO_CRYPTO_S_ERR    1
> #define VIRTIO_CRYPTO_S_UNSUPP    2
> #define VIRTIO_CRYPTO_S_BADMSG    3
> 
> For symmetric cryptography, three types algorithms are supported:

What does this "are supported" mean?
Device SHOULD support 3 types of algorithms?
Or CAN? MUST?

> enum {
>     VIRTIO_CRYPTO_ABLKCIPHER,
>     VIRTIO_CRYPTO_AEAD,
>     VIRTIO_CRYPTO_HASH,
> };

Specify values here too pls.

> VIRTIO_CRYPTO_ABLKCIPHER: Asynchronous Block Cipher.
> VIRTIO_CRYPTO_AEAD: Authenticated Encryption With Associated Data (AEAD) 
> Cipher.
> VIRTIO_CRYPTO_HASH: Hash and MAC (Message Authentication Code) cipher.
> 
> 1.6.1 Encryption Operation
> 
> Bothe ctrlq and dataq virtqueue are bidirectional.
> Step1: Create a session:
> 1.    The front-end driver fill out the context message, include algorithm 
> name, key, keylen etc;
> 2.    The front-end driver send a context message to the backend device by 
> controlq;
> 3.    The backend driver create a session using the message transmitted by 
> controlq;
> 4.    Return a session id to the driver. 
> Step 2: Execute the detail encryption operation:
> 1.    The front-end driver fill out the encrypt requests;
> 2.    Put the requests into dataq and kick the virtqueue;
> 3.    The backend driver execute the encryption operation according the 
> requests’ arguments;
> 4.    Return the encryption result to the front-end driver by dataq.
> 5.    The front-end driver callback handle the result and over
> 

I'm guessing backend driver is the device and front-end the driver?


> Note: the front-end driver needs to support both synchronous and asynchronous 
> encryption.

So Driver  MUST support .... ?
Or does this in fact a requirement from device to support
both types.

> Even then the performance is poor in synchronous operation because frequent 
> context switching and virtualization overhead.

So driver SHOULD by preference use asynchronous encryption?

> 1.6.2 Decryption Operation
> 
> The decryption process is the same with encryption, except that AEAD 
> algorithm needs to be verified before decryption, if the verify result is not 
> correct, the device will directly return VIRTIO_CRYPTO_S_BADMSG (bad message) 
> to front-end driver.
> 
> 

needs to be verified -> device MUST verify and MUST return .... ?