qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Zeng, Xin
Subject: Re: [Qemu-devel] [RFC v4] virtio-crypto specification
Date: Fri, 22 Jul 2016 05:31:29 +0000

On Friday, July 22, 2016 10:53 AM Gonglei (Arei) wrote:
> 
> Hi Xin,
> 
> Thank you so much for your great comments.
> I agree with you almostly except some trivial detals.
> Please see my below replies.
> 
> And I'll submit V5 next week, and you can finish the asym algos parts if you
> like.
> Let's co-work to finish the virtio-crypto spec, shall we?
> 

That's great. 

> Regards,
> -Gonglei
> 
> 
> > -----Original Message-----
> > From: Zeng, Xin [mailto:address@hidden
> > Sent: Friday, July 22, 2016 8:48 AM
> > To: Gonglei (Arei); address@hidden; qemu-
> address@hidden
> > Cc: Hanweidong (Randy); Stefan Hajnoczi; Cornelia Huck; address@hidden;
> > Lingli Deng; Jani Kokkonen; Luonengjun; Huangpeng (Peter); Zhoujian (jay,
> > Euler); chenshanxi 00222737; 'Ola address@hidden'; Varun Sethi
> > Subject: RE: [RFC v4] virtio-crypto specification
> >
> > On Sunday, June 26, 2016 5:35 PM, Gonglei (Arei) Wrote:
> > > Hi all,
> > >
> > > This is the specification (version 4) about a new virtio crypto device.
> > >
> >
> > In general, our comments around this proposal are listed below:
> > 1. Suggest to introduce crypto services into virtio crypto device. The
> services
> > currently defined are CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
> 
> Yes, I agree, whether DRBG/NDRBG are included in PRIMITIVE service or
> not?
> If not, we'd better add another separate service.

Yes, I think we can add these two into PRIMITIVE services.

> 
> > 2. Suggest to define a unified crypto request format that is consisted of
> > general header + service specific request,  Where 'general header' is for 
> > all
> > crypto request,  'service specific request' is composed of
> > operation parameter + input data + output data in generally.
> > operation parameter is algorithm-specific parameters,
> > input data is the data should be operated ,
> > output data is the "operation result + result buffer".
> >
> It makes sense. Good.
> 
> > #define VIRTIO_CRYPTO_OPCODE(service, op)   (((service)<<8) | (op))
> > struct virtio_crypto_op_header {
> > #define VIRTIO_CRYPTO_CIPHER_ENCRYPT
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_CIPHER, 0x00)
> > #define VIRTIO_CRYPTO_CIPHER_DECRYPT
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_CIPHER, 0x01)
> > #define VIRTIO_CRYPTO_HASH
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_HASH, 0x00)
> > #define VIRTIO_CRYPTO_MAC
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_MAC, 0x00)
> > #define VIRTIO_CRYPTO_KDF
> >             VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_KDF, 0x00)
> > #define VIRTIO_CRYPTO_ASYM_KEY_GEN
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x00)
> > #define VIRTIO_CRYPTO_ASYM_KEY_EXCHG
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x01)
> > #define VIRTIO_CRYPTO_ASYM_SIGN
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x02)
> > #define VIRTIO_CRYPTO_ASYM_VERIFY
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x03)
> > #define VIRTIO_CRYPTO_ASYM_ENCRYPT
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x04)
> > #define VIRTIO_CRYPTO_ASYM_DECRYPT
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x05)
> > #define VIRTIO_CRYPTO_AEAD_ENCRYPT
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_AEAD, 0x00)
> > #define VIRTIO_CRYPTO_AEAD_DECRYPT
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_AEAD, 0x01)
> > #define VIRTIO_CRYPTO_PRIMITIVE
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_PRIMITIVE, 0x00)
> >     u32 opcode;
> >     u8 algo; /*service-specific algorithms*/
> >     u8 flag; /*control flag*/
> 
> We'd better add a U64 session_id property here for service-specific
> algorithms.
> 

Can we put session_id into parameter filed inside service-specific request?
For ASYM service, it doesn't need session_id.
And for HASH service, it might not need a session_id as well.

> > };
> >
> > Take rsa_sign_request as example,
> > A rsa sign service specific request is defined as:
> > struct virtio_crypto_asym_rsa_sign_req{
> >     struct virtio_crypto_rsa_sign_para parameter;
> >     struct virtio_crypto_rsa_sign_input idata;
> >     struct virtio_crypto_rsa_sign_output odata;
> > };
> >
> > A complete crypto service request is defined as:
> > struct virtio_crypto_op_data_req {
> >            struct virtio_crypto_op_header header;
> >           union {
> >                        struct virtio_crypto_asym_rsa_sign_req
> > rsa_sign_req;
> >                        /*other service request*/
> >           }u;
> > };
> >
> I wanted to do this in fact. ;)
> 
> > More detailed comments are embedded below:
> >
> > > Changes from v3:
> > >  - Don't use enum is the spec but macros in specific structures. [Michael 
> > > &
> > > Stefan]
> > >  - Add two complete structures for session creation and closing, so that
> > >   the spec is clear on how to lay out the request.  [Stefan]
> > >  - Definite the crypto operation request with assigned structure, in this
> way,
> > >   each data request only occupies *one entry* of the Vring descriptor
> table,
> > >   which *improves* the *throughput* of data transferring.
> > >
> > > Changes from v2:
> > >  - Reserve virtio device ID 20 for crypto device. [Cornelia]
> > >  - Drop all feature bits, those capabilities are offered by the device 
> > > all the
> > > time.  [Stefan & Cornelia]
> > >  - Add a new section 1.4.2 for driver requirements. [Stefan]
> > >  - Use definite type definition instead of enum type in some structure.
> > > [Stefan]
> > >  - Add virtio_crypto_cipher_alg definition. [Stefan]
> > >  - Add a "Device requirements" section as using MUST. [Stefan]
> > >  - Some grammar nits fixes and typo fixes. [Stefan & Cornelia]
> > >  - Add one VIRTIO_CRYPTO_S_STARTED status for the driver as the flag of
> > > virtio-crypto device started and can work now.
> > >
> > > Great thanks for Stefan and Cornelia!
> > >
> > > Changes from v1:
> > >  - Drop the feature bit definition for each algorithm, and using config
> space
> > > instead  [Cornelia]
> > >  - Add multiqueue support and add corresponding feature bit
> > >  - Update Encryption process and header definition
> > >  - Add session operation process and add corresponding header
> description
> > >  - Other better description in order to fit for virtio spec  [Michael]
> > >  - Some other trivial fixes.
> > >
> > > If you have any comments, please let me know, thanks :)
> > >
> > >
> > > Virtio-crypto device Spec
> > >                                                   Signed-off-by:
> > > Gonglei <address@hidden>
> > >
> > > 1 Crypto Device
> > > The virtio crypto device is a virtual crypto device (ie. hardware crypto
> > > accelerator card). The encryption and decryption requests of are placed
> in
> > > the data queue, and handled by the real hardware crypto accelerators
> finally.
> > > The second queue is the control queue, which is used to create or
> destroy
> > > session for symmetric algorithms, and to control some advanced features
> in
> > > the future.
> > > 1.1       Device ID
> > > 20
> > > 1.2       Virtqueues
> > > 0  dataq
> > > …
> > > N-1  dataq
> > > N  controlq
> > > N is set by max_virtqueues (max_virtqueues >= 1).
> > > 1.3       Feature bits
> > > There are no feature bits (yet).
> > > 1.4       Device configuration layout
> > > Three driver-read-only configuration fields are currently defined. One
> read-
> > > only bit (for the device) is currently defined for the status field:
> > > VIRTIO_CRYPTO_S_HW_READY. One read-only bit (for the driver) is
> currently
> > > defined for the status field: VIRTIO_CRYPTO_S_STARTED.
> > > #define VIRTIO_CRYPTO_S_HW_READY  (1 << 0) #define
> > > VIRTIO_CRYPTO_S_STARTED  (1 << 1)
> > >
> > > The following driver-read-only field, max_virtqueues specifies the
> maximum
> > > number of data virtqueues (dataq1. . .dataqN) .
> > > struct virtio_crypto_config {
> > >   le16 status;
> > >   le16 max_virtqueues;
> > >   le32 algorithms;
> > > };
> > >
> > > The last driver-read-only field, algorithms specifies the algorithms which
> the
> > > device offered. Two read-only bits (for the driver) are currently defined
> for
> > > the algorithms field: VIRTIO_CRYPTO_ALG_SYM and
> > > VIRTIO_CRYPTO_ALG_ASYM.
> > > #define VIRTIO_CRYPTO_ALG_SYM  (1 << 0)
> > > #define VIRTIO_CRYPTO_ALG_ASYM  (1 << 1)
> >
> > Suggest to change the virtio_crypto_config  structure to following structure
> to
> > define detail algorithms that the device supports in device configuration
> field.
> > struct virtio_crypto_config {
> >     le32 version;
> >     le16 status;
> >     le16 max_dataqueues;
> > #define VIRTIO_CRYPTO_S_CIPHER 0 /*cipher services*/
> > #define VIRTIO_CRYPTO_S_HASH 1 /*hash service*/
> > #define VIRTIO_CRYPTO_S_MAC 2 /*MAC(Message Authentication Codes)
> > service*/
> > #define VIRTIO_CRYPTO_S_AEAD  3 /* AEAD(Authenticated Encryption
> with
> > Associated Data) service*/
> > #define VIRTIO_CRYPTO_S_KDF 4  /*KDF(Key Derivation Functions)
> service*/
> > #define VIRTIO_CRYPTO_S_ASYM 5 /*asymmetric service*/
> > #define VIRTIO_CRYPTO_S_PRIMITIVE 6 /*Essential mathematics
> computing
> > service*/
> 
> I'd like to use s/ VIRTIO_CRYPTO_S_/ VIRTIO_CRYPTO_SERIVCE_/g, avoiding
> to
> conflict with VIRTIO_CRYPTO_SATAUS_ which all virtio spec always used.

That's OK.

> 
> >     le32 crypto_servcies; /*overall services mask*/
> >    /*detailed algorithms mask*/
> >     le32 cipher_algo_l;
> >     le32 cipher_algo_h;
> >     le32 hash_algo;
> >     le32 mac_algo_l;
> >     le32 mac_algo_h;
> >     le32 asym_algo;
> >     le32 kdf_algo;
> >     le32 aead_algo;
> >     le32 primitive_algo;
> > };
> >
> > 15 bits are defined for cipher algorithms currently.
> > #define VIRTIO_CRYPTO_CIPHER_ARC4               0
> > #define VIRTIO_CRYPTO_CIPHER_AES_ECB            1
> > #define VIRTIO_CRYPTO_CIPHER_AES_CBC            2
> > #define VIRTIO_CRYPTO_CIPHER_AES_CTR            3
> > #define VIRTIO_CRYPTO_CIPHER_DES_ECB            6
> > #define VIRTIO_CRYPTO_CIPHER_DES_CBC            7
> > #define VIRTIO_CRYPTO_CIPHER_3DES_ECB           8
> > #define VIRTIO_CRYPTO_CIPHER_3DES_CBC           9
> > #define VIRTIO_CRYPTO_CIPHER_3DES_CTR           9
> > #define VIRTIO_CRYPTO_CIPHER_KASUMI_F8          10
> > #define VIRTIO_CRYPTO_CIPHER_SNOW3G_UEA2        11
> > #define VIRTIO_CRYPTO_CIPHER_AES_F8             12
> > #define VIRTIO_CRYPTO_CIPHER_AES_XTS            13
> > #define VIRTIO_CRYPTO_CIPHER_ZUC_EEA3           14
> >
> > 12 bits are defined for Hash algorithms currently.
> > #define VIRTIO_CRYPTO_HASH_MD5           0
> > #define VIRTIO_CRYPTO_HASH_SHA1          1
> > #define VIRTIO_CRYPTO_HASH_SHA_224       2
> > #define VIRTIO_CRYPTO_HASH_SHA_256       3
> > #define VIRTIO_CRYPTO_HASH_SHA_384       4
> > #define VIRTIO_CRYPTO_HASH_SHA_512       5
> > #define VIRTIO_CRYPTO_HASH_SHA3_224      6
> > #define VIRTIO_CRYPTO_HASH_SHA3_256      7
> > #define VIRTIO_CRYPTO_HASH_SHA3_384      8
> > #define VIRTIO_CRYPTO_HASH_SHA3_512      9
> > #define VIRTIO_CRYPTO_HASH_SHA3_SHAKE128      10
> > #define VIRTIO_CRYPTO_HASH_SHA3_SHAKE256      11
> >
> > 15 bits are defined for MAC algorithms currently
> > #define VIRTIO_CRYPTO_MAC_HMAC_MD5               0
> > #define VIRTIO_CRYPTO_MAC_HMAC_SHA1                1
> > #define VIRTIO_CRYPTO_MAC_HMAC_SHA_224             2
> > #define VIRTIO_CRYPTO_MAC_HMAC_SHA_256             3
> > #define VIRTIO_CRYPTO_MAC_HMAC_SHA_384             4
> > #define VIRTIO_CRYPTO_MAC_HMAC_SHA_512             5
> > #define VIRTIO_CRYPTO_MAC_CMAC_3DES                24
> 
> Why not 6 here? I'd like to keep increasing steadily, we can extend
> the value when some other algorithms are supported.

The intension is to keep the same kind of algorithms into continuous bits
as  possible as.

> 
> > #define VIRTIO_CRYPTO_MAC_CMAC_AES                 25
> > #define VIRTIO_CRYPTO_MAC_CMAC_KASUMI_F9             26
> > #define VIRTIO_CRYPTO_MAC_CMAC_ SNOW3G_UIA2        27
> > #define VIRTIO_CRYPTO_MAC_GMAC_AES                 40
> > #define VIRTIO_CRYPTO_MAC_GMAC_TWOFISH             41
> > #define VIRTIO_CRYPTO_MAC_CBCMAC_AES               48
> 
> Dose this duplicate with VIRTIO_CRYPTO_MAC_CMAC_AES ?

CMAC_AES is different with CBCMAC_AES, there is a history
about these.

> 
> > #define VIRTIO_CRYPTO_MAC_CBCMAC_KASUMI_F9         49
> 
> Duplicates with VIRTIO_CRYPTO_MAC_CMAC_KASUMI_F9 ?

CMAC_KASUMI_F9 is also different with CBCMAC_KASUMI_F9.

> 
> > #define VIRTIO_CRYPTO_MAC_XCBC_AES                 52
> >
> > 5 bits are defined for asymmetric algorithms currently.
> > #define VIRTIO_CRYPTO_ASYM_RSA                  0
> > #define VIRTIO_CRYPTO_ASYM_DSA                  1
> > #define VIRTIO_CRYPTO_ASYM_DH                   2
> > #define VIRTIO_CRYPTO_ASYM_ECDSA                3
> > #define VIRTIO_CRYPTO_ASYM_ECDH                 4
> >
> > 7 bits are defined for KDF algorithms currently.
> > #define VIRTIO_CRYPTO_KDF_NONE 0
> > #define VIRTIO_CRYPTO_KDF_ANSI-X9.42 1
> > #define VIRTIO_CRYPTO_KDF_ANSI-X9.62 2
> > #define VIRTIO_CRYPTO_KDF_IKEv2 3
> > #define VIRTIO_CRYPTO_KDF_TLS_1.0 4
> > #define VIRTIO_CRYPTO_KDF_TLS_1.2 5
> > #define VIRTIO_CRYPTO_KDF_NIST-800-56-Concatenation 6
> >
> > 2 bits are defined for AEAD algorithms currently.
> > #define VIRTIO_CRYPTO_AEAD_GCM   0
> > #define VIRTIO_CRYPTO_AEAD_CCM    1
> >
> > 4 bits are defined for PRIMITIVE algorithms currently
> > #define VIRTIO_CRYPTO_PRIMITIVE_LNMOD_EXP               0
> > #define VIRTIO_CRYPTO_PRIMITIVE_LNMOD_INV               1
> > #define VIRTIO_CRYPTO_PRIMITIVE_ ECPOINT_MULTIPLY  2
> > #define VIRTIO_CRYPTO_PRIMITIVE_ ECPOINT_VERIFY  3
> >
> > > 1.4.1     Device Requirements: Device configuration layout
> >
> > Add "The device MUST set the version in version filed."
> >
> For compatibility? Okay.
> 
> > > The device MUST set max_virtqueues to between 1 and 65535 inclusive.
> > >
> > > The device SHOULD set status according to the status of the hardware-
> > > backed implementation.
> > >
> > > The device MUST set algorithms according to the algorithms which the
> device
> > > offered.
> > > 1.4.2     Driver Requirements: Device configuration layout
> > > The driver MUST read the ready status from the bottom bit of status to
> check
> > > whether the hardware-backed implementation is ready or not.
> >
> > Add "The driver MAY read max_dataqueues field to discover how many
> data
> > queues
> > the device supports."
> >
> OK.
> 
> > > The driver MUST read the algorithms to discover the algorithms which the
> > > device supports.
> >
> > Because of the presence of overall service field and detail algorithms 
> > field,
> > suggest
> > To change this to "
> > The driver MUST read crypto_services field to discover which services the
> > device is
> > able to offer.
> > The driver MUST read detail algorithms field according to crypto_services
> field.
> > "
> >
> Yes, makes sense.
> 
> > > 1.5       Device Initialization
> > > A driver would perform a typical initialization routine like so:
> > > 1. Identify and initialize data virtqueue, up to max_virtqueues.
> > > 2. Identify the control virtqueue.
> > > 3. Identify the ready status of hardware-backend comes from the bottom
> bit
> > > of status.
> > > 4. Read the supported algorithms from bits of algorithms.
> > > 1.6       Device Operation
> > > 1.6.1     Session operation
> > > The symmetric algorithms have the concept of sessions. A session is a
> handle
> > > which describes the cryptographic parameters to be applied to a number
> of
> > > buffers. The data within a session handle includes the following:
> > > •1. The operation (cipher, hash or both, and if both, the order in which
> the
> > > algorithms should be applied).
> > > •2. The cipher setup data, including the cipher algorithm and mode, the
> key
> > > and its length, and the direction (encrypt or decrypt).
> > > •3. The hash setup data, including the hash algorithm, mode (plain,
> nested or
> > > authenticated), and digest result length (to allow for truncation).
> > >   Authenticated mode can refer to HMAC, which requires that the
> > > key and its length are also specified. It is also used for GCM and CCM
> > > authenticated encryption, in which case the AAD length is also specified.
> > >   For nested mode, the inner and outer prefix data and length are
> > > specified, as well as the outer hash algorithm.
> > >
> > > The controlq virtqueue is used to control session operations, including
> > > creation or close. The request is preceded by a header:
> > > struct virtio_crypto_sym_ctlhdr {
> > >     /* control type  */
> > >     u8 type;
> > > };
> > > Two bits are currently defined for the control header type:
> > > #define VIRTIO_CRYPTO_CTRL_CREATE_SESSION  1 #define
> > > VIRTIO_CRYPTO_CTRL_CLOSE_SESSION  2
> > >
> >
> > Suggest to change virtio_crypto_sym_ctlhdr structure to the a general
> > header below, and define a unified control request  structure to keep
> > consistent with crypto service request format.
> > struct virtio_crypto_ctrl_header{
> > #define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_CIPHER, 0x02)
> > #define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_CIPHER, 0x03)
> > #define VIRTIO_CRYPTO_MAC_CREATE_SESSION
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_MAC, 0x02)
> > #define VIRTIO_CRYPTO_MAC_DESTROY_SESSION
> >     VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_MAC, 0x03)
> >     u32 opcode;
> >     u8 algo;
> >     u8 flag;
> > };
> >
> We MUST add a queue_id property in header, which shows which data
> virtqueue
> we will used for multiqueue support.
> 

Ok.

> > > 1.6.1.1 Session creation operation
> > > A request of creating a session including the following information:
> > > struct virtio_crypto_sym_session_creation {
> > >   struct virtio_crypto_sym_ctlhdr   ctlhdr;  /* OUT */
> > >   struct virtio_crypto_sym_session_op  session_op;  /* OUT */
> > >   struct virtio_crypto_sym_session_op_inhdr  inhdr;  /* IN */ }; The
> > > details of specific structure, including struct
> virtio_crypto_sym_session_op
> > > and struct virtio_crypto_sym_session_op_inhdr  are defined by the
> following
> > > sections.
> > > 1.6.1.1.1 Driver Requirements: Session creation operation The driver
> MUST
> > > set the control type with VIRTIO_CRYPTO_CTRL_CREATE_SESSION before
> > > the request is preceded by an operation header when executing session
> > > creation:
> > > typedef struct virtio_crypto_sym_session_op { /**< No operation */
> > > #define VIRTIO_CRYPTO_SYM_OP_NONE    0
> > > /**< Cipher only operation on the data */
> > > #define VIRTIO_CRYPTO_SYM_OP_CIPHER    1
> > > /**< Hash only operation on the data */
> > > #define VIRTIO_CRYPTO_SYM_OP_HASH     2
> > > /**< Chain any cipher with any hash operation */
> > > #define VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING    3
> > >   u8 op_type; /* Operation type */
> > >     virtio_crypto_sym_cipher_t cipher_setup_data;
> > >   virtio_crypto_sym_hash_t hash_setup_data;
> > > /* Perform the hash operation followed by the cipher operation */
> > > #define
> VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER
> > >   1
> > > /* Perform the cipher operation followed by the hash operation */
> > > #define
> VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH
> > >   2
> > >     u8 alg_chain_order;
> > > } virtio_crypto_sym_session_op_t;
> > > And the structures definition details are:
> > > typedef struct virtio_crypto_sym_hash_auth_mode {
> > >   /* length of authenticated key */
> > >   le32 auth_key_len;
> > >   /* The length of the additional authenticated data (AAD) in bytes */
> > >   le32 aad_len;
> > > } virtio_crypto_sym_hash_auth_mode_t;
> > >
> > > typedef struct virtio_crypto_sym_cipher {
> > > /* Option to not do any cryptography */
> > > #define VIRTIO_CRYPTO_NO_CIPHER   0
> > > #define VIRTIO_CRYPTO_CIPHER_DES  1
> > > #define VIRTIO_CRYPTO_CIPHER_DES_CBC      2
> > > #define VIRTIO_CRYPTO_CIPHER_DES3 3
> > > #define VIRTIO_CRYPTO_CIPHER_DES3_CBC     4
> > > #define VIRTIO_CRYPTO_CIPHER_AES  5
> > > #define VIRTIO_CRYPTO_CIPHER_AES_CBC      6
> > > #define VIRTIO_CRYPTO_CIPHER_KASUMI_F8    7
> > >     le32 alg;  /* cipher algorithm type (ie. aes-cbc ) */
> > >     /* length of key */
> > >     le32 keylen;
> > > #define VIRTIO_CRYPTO_DECRYPT     0
> > > #define VIRTIO_CRYPTO_ENCRYPT     1
> > >   u8 op; /* encrypt or decrypt */
> > > } virtio_crypto_sym_cipher_t;
> > >
> > > typedef struct virtio_crypto_sym_hash {
> > > /* Option to not do any hash */
> > > #define VIRTIO_CRYPTO_NO_HASH     0
> > > #define VIRTIO_CRYPTO_HASH_MD5    1
> > > #define VIRTIO_CRYPTO_HASH_SHA1   2
> > > #define VIRTIO_CRYPTO_HASH_SHA1_96        3
> > > #define VIRTIO_CRYPTO_HASH_SHA224 4
> > > #define VIRTIO_CRYPTO_HASH_SHA256 5
> > > #define VIRTIO_CRYPTO_HASH_SHA384 6
> > > #define VIRTIO_CRYPTO_HASH_SHA512 7
> > > #define VIRTIO_CRYPTO_HASH_AES_XCBC          8
> > > #define VIRTIO_CRYPTO_HASH_AES_XCBC_96  9
> > > #define VIRTIO_CRYPTO_HASH_KASUMI_F9      10
> > >     le32 hash_alg;  /* hash algorithm type */
> > > #define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN 1
> > > #define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH  2
> > > #define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED        3
> > > u8 hash_mode; /* mode of hash operation, including
> > > authenticated/plain/nested hash */
> > >   /* hash result length */
> > >     le32 hash_result_len;
> > >     virtio_crypto_sym_hash_auth_mode_t auth_mode_setup_data; }
> > > virtio_crypto_sym_hash_t; The driver MUST set the control type with
> > > VIRTIO_CRYPTO_CTRL_CLOSE_SESSION and pass the session_id to the
> > > device when executing session close.
> >
> > This structure mixed HASH and MAC, suggest  to define separated
> > services structure, cipher service structure can reference that.
> >
> OK, and we can still name it 'sym', include plain cipher and chain-algorithms.
> 
> > > 1.6.1.1.2 Device Requirements: Session creation operation The device
> MUST
> > > return a session identifier to the driver when the device finishes the
> > > processing of session close. The session creation request MUST end by a
> > > tailer:
> > > typedef struct virtio_crypto_sym_session_op_inhdr {
> > >   u8     status;
> > >   le64    session_id;
> > > } virtio_crypto_sym_session_op_inhdr_t;
> > > Both status and session_id are written by the device: either
> > > VIRTIO_CRYPTO_CTRL_OK for success, VIRTIO_CRYPTO_CTRL_ERR for
> > > creation failed or device error.
> > > #define VIRTIO_CRYPTO_CTRL_OK     0
> > > #define VIRTIO_CRYPTO_CTRL_ERR    1
> > > 1.6.1.2 Session closing operation
> > > A request of closing a session including the following information:
> > > struct virtio_crypto_sym_session_creation {
> > >   struct virtio_crypto_sym_ctlhdr   ctlhdr;  /* OUT */
> > >   le64  session_id;  /* OUT */
> > >   u8 status;  /* IN */
> > > };
> > > 1.6.1.2.1 Driver Requirements: Session closing operation The driver MUST
> set
> > > the control type with VIRTIO_CRYPTO_CTRL_CLOSE_SESSION, and the
> > > session_id MUST be a valid value which assigned by the device when a
> > > session was created.
> > > 1.6.1.2.2 Device Requirements: Session closing operation Status is written
> by
> > > the device: either VIRTIO_CRYPTO_CTRL_OK for success,
> > > VIRTIO_CRYPTO_CTRL_ERR for creation failed or device error.
> > > #define VIRTIO_CRYPTO_CTRL_OK     0
> > > #define VIRTIO_CRYPTO_CTRL_ERR    1
> >
> > Need two sets of error code be defined(see 1.6.2.2 )? The unified error
> code for
> > all
> > virtio crypto device should be better. We suggest the error codes below for
> > virtio
> > crypto devices:
> > #define VIRTIO_CRYPTO_OP_OK    0
> > #define VIRTIO_CRYPTO_OP_ERR    1
> > #define VIRTIO_CRYPTO_OP_BADMSG    2
> > #define VIRTIO_CRYPTO_OP_NOTSUPP   3
> >
> Makes sense.
> 
> > > 1.6.2     Encryption operation
> > > 1.6.2.1 Driver Requirements: Encryption operation The encryption and
> > > decryption requests and the corresponding results are transmitted by
> placing
> > > them in dataq. The symmetric algorithms requests are preceded by a
> header:
> > > struct virtio_crypto_sym_op_hdr {
> > >   /* the backend returned session identifier */
> > >   le64 session_id;
> > >   /* length of initial vector */
> > >   le32 iv_len;
> > >   /* iv offset in the whole crypto data memory */
> > >   le32 iv_offset;
> > >   /* length of additional auth data */
> > >   le32 auth_len;
> > >   /* additional auth data offset in the whole crypto data memory */
> > >   le32 additional_auth_offset;
> > >   /* cipher start source offest */
> > >   le32 cipher_start_src_offset;
> > >   le32 len_to_cipher;
> > >   /* hash start source offest */
> > >   le32 hash_start_src_offset;
> > >   le32 len_to_hash;
> > >   /* length of source data */
> > >   le32 source_len;
> > > } ;
> > > The encryption request MUST end by a tailer:
> > > typedef struct virtio_crypto_sym_op_inhdr {
> > >   u8     status;
> > > } virtio_crypto_sym _op_inhdr_t;
> > > The specific content of symmetric algorithms requests SHOULD be same
> as
> > > below:
> > > struct virtio_crypto_sym_op_data {
> > >   struct virtio_crypto_sym_op_hdr  hdr_info;
> > >   le64 iv_addr; /* iv guest address */
> > >   le64 auth_data_addr; /* associated data guest address */
> > >   le64 src_data_addr; /* source data guest address */
> > >   le64 dst_data_addr; /* destination data guest address */
> > >   le64 digest_result_addr; /* digest result guest address */
> > >   le64 inhdr_addr; /* in-header guest address */ }; In this way, each
> > > data request only occupies one entry of the Vring descriptor table, which
> > > improves the throughput of data transferring.
> >
> > Suggest to change virtio_crypto_sym_op_data to unified format listed
> above.
> > All these can also be fitted into one vring descriptor.
> >
> Sure.
> 
> > > 1.6.2.2 Device Requirements: Encryption operation The struct
> > > virtio_crypto_sym_op_inhdr’s status byte is written by the device: either
> > > VIRTIO_CRYPTO_S_OK for success, VIRTIO_CRYPTO_S_ERR for device or
> > > driver error, 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_BADMSG    2
> >
> > Same comments for error code above.
> >
> > >
> > > 1.6.2.3 Steps of encryption Operation
> > > Step1: Create a session:
> > > 1.        The driver fills out the context message, including algorithm 
> > > name,
> > > key, keylen etc;
> > > 2.        The driver sends a context message to the backend device by
> > > controlq;
> > > 3.        The device creates a session using the message transmitted by
> > > controlq;
> > > 4.        Return the session id to the driver.
> > > Step 2: Execute the detail encryption operation:
> > > 1.        The driver fills out the encrypt requests;
> > > 2.        Put the requests into dataq and kick the virtqueue;
> > > 3.        The device executes the encryption operation according the
> > > requests' arguments;
> > > 4.        The device returns the encryption result to the driver by dataq;
> > > 5.        The driver callback handle the result and over.
> > >
> > > Note: the driver MAY support both synchronous and asynchronous
> > > encryption. Then the performance is poor in synchronous operation
> because
> > > frequent context switching and virtualization overhead. The driver
> SHOULD
> > > by preference use asynchronous encryption.
> > > 1.6.3     Decryption Operation
> > > The decryption process is the same with encryption.
> > > 1.6.3.1 Device Requirements: Decryption operation The device MUST
> verify
> > > and return the verify result to the driver. If the verify result is not 
> > > correct,
> > > VIRTIO_CRYPTO_S_BADMSG (bad message) MUST be returned the
> driver.
> > >
> > > Regards,
> > > -Gonglei
> > >
> > >
> > >


reply via email to

[Prev in Thread] Current Thread [Next in Thread]