RE: [RFC v1] virtio-crypto specification

["Gonglei (Arei)" <arei.gonglei@huawei.com>]

Hi Michael,

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Friday, November 20, 2015 5:40 PM
> 
> Thanks, looks good overall.
> You might want to join the TC if you maintain a device.

Yes, IIRC Jani is a member of the TC, right? Jani ?

> 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?
> 
Yeah, because I just use the controlq to control session operations at present. 

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

Okay, will fix them.

> > };
> > 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?
> 

CAN is more accurate.

> > enum {
> >     VIRTIO_CRYPTO_ABLKCIPHER,
> >     VIRTIO_CRYPTO_AEAD,
> >     VIRTIO_CRYPTO_HASH,
> > };
> 
> Specify values here too pls.
> 
What do you mean here?

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

Yes.

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

I mean SHOULD support.
> > Even then the performance is poor in synchronous operation because
> frequent context switching and virtualization overhead.
> 
> So driver SHOULD by preference use asynchronous encryption?
> 
Yes, that's true ;)

> > 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 .... ?
> 
Correct.

Great thanks for your repaid feedback, Michael.

Regards,
-Gonglei