[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: RE: [PATCH v1 1/1] virtio crypto device specification: asymmetric crypto service
Hi, Lei: Thanks for the comments. On Wednesday, November 30, 2016 9:25 AM, Gonglei (Arei) wrote: > > > > > > \subsection{Device ID}\label{sec:Device Types / Crypto Device / Device ID} > > @@ -44,7 +44,9 @@ struct virtio_crypto_config { > > le32 mac_algo_l; > > le32 mac_algo_h; > > le32 aead_algo; > > - le32 reserve; > > + le32 asym_algo; > > + le32 rsa_padding; > > + le32 reserved; > > }; > > Please rebased on the latest virtio crypto spec, > the config structure had been changed. > Sure. > > \end{lstlisting} > > > > @@ -70,6 +72,8 @@ The following services are defined: > > #define VIRTIO_CRYPTO_SERVICE_MAC (2) > > /* AEAD (Authenticated Encryption with Associated Data) service */ > > #define VIRTIO_CRYPTO_SERVICE_AEAD (3) > > +/* ASYM (Asymmetric crypto algorithms) service */ > > +#define VIRTIO_CRYPTO_SERVICE_ASYM (4) > > \end{lstlisting} > > > > The last driver-read-only fields specify detailed algorithms masks > > @@ -143,6 +147,28 @@ The following AEAD algorithms are defined: > > #define VIRTIO_CRYPTO_AEAD_CHACHA20_POLY1305 3 > > \end{lstlisting} > > > > +The following asymmetric algorithms are defined: > > + > > +\begin{lstlisting} > > +#define VIRTIO_CRYPTO_ASYM_NONE 0 > > +#define VIRTIO_CRYPTO_ASYM_RSA 1 > > +#define VIRTIO_CRYPTO_ASYM_DSA 2 > > +#define VIRTIO_CRYPTO_ASYM_DH 3 > > +#define VIRTIO_CRYPTO_ASYM_ECDSA 4 > > +#define VIRTIO_CRYPTO_ASYM_ECDH 5 > > Mixing tab and space. > Will fix it in next version. > > +\end{lstlisting} > > + > > +The following rsa padding capabilities are defined: > > + > > What're the padding capabilities used for? > I think we should add some explanation for them. > Makes sense, let's explain it more. > > +\begin{lstlisting} > > +#define VIRTIO_CRYPTO_RSA_NO_PADDING 0 > > +#define VIRTIO_CRYPTO_RSA_PKCS1_PADDING 1 > > +#define VIRTIO_CRYPTO_RSA_SSLV23_PADDING 2 > > +#define VIRTIO_CRYPTO_RSA_PKCS1_OAEP_PADDING 3 > > +#define VIRTIO_CRYPTO_RSA_X931_PADDING 4 > > +#define VIRTIO_CRYPTO_RSA_PKCS1_PSS_PADDING 5 > > +\end{lstlisting} > > + > > \begin{note} > > Any other value is reserved for future use. > > \end{note} > > @@ -241,6 +267,18 @@ struct virtio_crypto_op_header { > > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x00) > > #define VIRTIO_CRYPTO_AEAD_DECRYPT \ > > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x01) > > +#define VIRTIO_CRYPTO_ASYM_SIGN \ > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_ASYM, 0x00) > > +#define VIRTIO_CRYPTO_ASYM_VERIFY \ > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_ASYM, 0x01) > > +#define VIRTIO_CRYPTO_ASYM_ENCRYPT \ > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_ASYM, 0x02) > > +#define VIRTIO_CRYPTO_ASYM_DECRYPT \ > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_ASYM, 0x03) > > +#define VIRTIO_CRYPTO_ASYM_KEY_GEN \ > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_ASYM, 0x04) > > +#define VIRTIO_CRYPTO_ASYM_KEY_EXCHG \ > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_ASYM, 0x05) > > le32 opcode; > > /* algo should be service-specific algorithms */ > > le32 algo; > > @@ -548,6 +586,23 @@ struct virtio_crypto_op_data_req { > > struct virtio_crypto_hash_data_req hash_req; > > struct virtio_crypto_mac_data_req mac_req; > > struct virtio_crypto_aead_data_req aead_req; > > + > > + struct virtio_crypto_ecdsa_sign_req ecdsa_sign_req; > > + struct virtio_crypto_dsa_sign_req dsa_sign_req; > > + struct virtio_crypto_rsa_sign_req rsa_sign_req; > > + struct virtio_crypto_ecdsa_verify_req ecdsa_verify_req; > > + struct virtio_crypto_dsa_verify_req dsa_verify_req; > > + struct virtio_crypto_rsa_verify_req rsa_verify_req; > > + struct virtio_crypto_rsa_enc_req rsa_enc_req > > + struct virtio_crypto_rsa_dec_req rsa_dec_req; > > + struct virtio_crypto_rsa_keygen_req rsa_keygen_req; > > + struct virtio_crypto_dsa_keygen_req dsa_keygen_req; > > + struct virtio_crypto_ec_keygen_req ec_keygen_req; > > + struct virtio_crypto_dh_keyexchg_param_gen_req > > dh_keyexchg_param_gen_req; > > + struct virtio_crypto_dh_keyexchg_key_gen_req > > dh_keyexchg_key_gen_req; > > + struct virtio_crypto_dh_keyexchg_key_compute_req > > dh_keyexchg_key_compute_req; > > + struct virtio_crypto_ecdh_keyexchg_key_gen_req > > ecdh_keyexchg_key_gen_req; > > + struct virtio_crypto_ecdh_keyexchg_key_compute_req > > ecdh_keyexchg_key_compute_req; > > } u; > > }; > > \end{lstlisting} > > @@ -996,4 +1051,908 @@ pointed by \field{dst_data_addr} and > > \field{digest_result_addr} in struct virtio > > \item The device MUST copy the digest result to the guest memory > recorded > > by \field{digest_result_addr} field in struct virtio_crypto_aead_input. > > \item The device MUST set the \field{status} field in strut > > virtio_crypto_aead_input: VIRTIO_CRYPTO_OK: success; > VIRTIO_CRYPTO_ERR: > > creation failed or device error; VIRTIO_CRYPTO_NOTSUPP: not supported. > > \item When the \field{opcode} is VIRTIO_CRYPTO_AEAD_DECRYPT, the > device > > MUST verify and return the verification result to the driver, and if the > > verification result is incorrect, VIRTIO_CRYPTO_BADMSG (bad message) > MUST > > be returned to the driver. > > -\end{itemize*} > > \ No newline at end of file > > +\end{itemize*} > > + > > +\subsubsection{Asymmetric Crypto Service Operation}\label{sec:Device > Types > > / Crypto Device / Device Operation / > > +Crypto operation / Asymmetric Crypto Service Operation} > > + > > +In general, asymmetric crypto service can be referred as signature, > > verification, > > +encryption, decryption, key generation and key exchange. Not each > algorithm > > supports > > +all these services. > > + > > +Unlike symmetric crypto service, asymmetric crypto service normally > does't > > +need session operations, the request is encapsulated within one packet > which > > is represented > > +by \field{virtio_crypto_op_data_req}. > > +(see \ref{sec:Device Types / Crypto Device / Device Operation / Data > > Virtqueue}). > > + > > +Once service request is handled by the device, the device writes back the > > operation results to the corresponding > > +fields in the request packet. > > + > > +The asymmetric crypto operation uses a virtio_crypto_buf to represent > the > > input/output buffer. > > +\begin{lstlisting} > > +struct virtio_crypto_buf { > > + le32 len; > > + le32 reserved; > > + le64 addr; > > +}; > > +\end{lstlisting} > > + > > This is the same problem with my pervious symmetric spec. > Pls don't define a private structure in the virtio spec, > and refer to the latest virtio crypto spec: > > The buffer length is stored in the para part, and > the buffer content is stored in input/output parts by u8[len]. > Right, will update those. > struct virtio_crypto_alg_chain_session_para { > le32 alg_chain_order; > le32 hash_mode; > struct virtio_crypto_cipher_session_para cipher_param; > union { > struct virtio_crypto_hash_session_para hash_param; > struct virtio_crypto_mac_session_para mac_param; > } u; > /* length of the additional authenticated data (AAD) in bytes */ > le32 aad_len; > le32 padding; > }; > > struct virtio_crypto_alg_chain_session_req { > /* Device-readable part */ > struct virtio_crypto_alg_chain_session_para para; > /* The cipher key */ > u8 cipher_key[keylen]; > /* The authenticated key */ > u8 auth_key[auth_key_len]; > > /* Device-writable part */ > struct virtio_crypto_session_input input; > }; > > > +\devicenormative{\paragraph}{Asymmetric Crypto Service > Operation}{Device > > Types / Crypto Device / Device Operation / Asymmetric Crypto Service > > Operation} > > +\begin{itemize*} > > +\item The device MUST parse the field \field{opcode} within > > \field{virtio_crypto_op_header} (see \ref{sec:Device Types / Crypto > Device / > > Device Operation}) before > > +it handles the corresponding service request. > > +\item The device SHOULD set the operation status in field \field{status} > within > > \field{idata}. > > +\item The device SHOULD update the operation result to the fields in > > \field{idata} if the operation is successful. > > +\end{itemize*} > > + > > +\drivernormative{\paragraph}{Asymmetric Crypto Service > Operation}{Device > > Types / Crypto Device / Device Operation / Asymmetric Crypto Service > > Operation} > > +\begin{itemize*} > > +\item The driver SHOULD set the corresponding \field{opcode} in > > \field{virtio_crypto_op_header} according to different services. > > +\item The driver SHOULD offer operatable buffer within \field{idata} if the > > service request structure defines. > > +\end{itemize*} > > + > > +\paragraph{Signature: ECDSA signature operation}\label{sec:Device Types > / > > Crypto Device / > > +Device Operation / Crypto operation / Asymmetric Crypto Service > Operation / > > Signature: ECDSA signature operation} > > + > > +\begin{lstlisting} > > +struct virtio_crypto_ec_group { > > + struct virtio_crypto_buf xg; > > + struct virtio_crypto_buf yg; > > + struct virtio_crypto_buf n; > > + struct virtio_crypto_buf q; > > + struct virtio_crypto_buf a; > > + struct virtio_crypto_buf b; > > + struct virtio_crypto_buf h; > > +}; > > + > > +#define VIRTIO_CRYPTO_EC_FIELD_TYPE_PRIME 0 > > +#define VIRTIO_CRYPTO_EC_FIELD_TYPE_BINARY 1 > > + > > +struct virtio_crypto_ecdsa_sign_para{ > > + /* Hash alogrithms applied to msg */ > > + le32 hash_algo; > > + /* EC field_type, see VIRTIO_CRYPTO_EC_FIELD_TYPE_* definition */ > > + le32 field_type; > > + > > + /* EC Group parameters */ > > + struct virtio_crypto_ec_group ec; > > + /* Random value */ > > + struct virtio_crypto_buf k; > > + /* Private Key */ > > + struct virtio_crypto_buf d; > > +}; > > + > > +struct virtio_crypto_ecdsa_sign_output { > > + /* Message need to be signed */ > > + struct virtio_crypto_buf msg; > > +}; > > + > > +struct virtio_crypto_ecdsa_sign_input { > > + /* operation result */ > > + le32 status; > > + le32 reserved; > > + > > + /* signature result */ > > + struct virtio_crypto_buf r; > > + struct virtio_crypto_buf s; > > +}; > > +\end{lstlisting} > > + > > +ECDSA signature operation request is defined as below: > > +\begin{lstlisting} > > +struct virtio_crypto_ecdsa_sign_req { > > + struct virtio_crypto_ecdsa_sign_para parameter; > > + struct virtio_crypto_ecdsa_sign_output odata; > > + > > + struct virtio_crypto_ecdsa_sign_input idata; > > +}; > > +\end{lstlisting} > > + > > + > > +\devicenormative{\subparagraph}{ECDSA signature operation}{Device > Types / > > Crypto Device / Device Operation / Asymmetric Crypto Service Operation / > > ECDSA signature operation} > > +\begin{itemize*} > > +\item The device SHOULD set the operation results in \field{idata} > according > > Here you say SHOULD, but... > > > to the general device requirments for asymmetric crypto service. > > +\end{itemize*} > > + > > +\drivernormative{\subparagraph}{ECDSA signature operation}{Device > Types / > > Crypto Device / Device Operation / Asymmetric Crypto Service Operation / > > ECDSA signature operation} > > +\begin{itemize*} > > +\item The driver SHOULD set the \field{opcode} in > > \field{virtio_crypto_op_header} to VIRTIO_CRYPTO_ASYM_SIGN, > > +set \field{algo} to VIRTIO_CRYPTO_ASYM_ECDSA. > > +\item The driver SHOULD set all fields in \field{parameter} and > \field{odata} > > except field \field{reserved}. > > +\item The driver MUST check the operation result \field{status} in > \field{idata} > > before it operates other fields in \field{idata}. > > ...here is MUST. > > Maybe s/SHOULD/MUST/g, no? > Yes, will fix them in next version. > > +Only if the operation is successful, the \field{r,s} in \field{idata} are > operatable. > > +\end{itemize*} > > + > > +\paragraph{Signature: DSA signature operation}\label{sec:Device Types / > > Crypto Device / > > +Device Operation / Crypto operation / Asymmetric Crypto Service > Operation/ > > Signature: DSA signature operation} > > + > > +\begin{lstlisting} > > +struct virtio_crypto_dsa_group { > > + struct virtio_crypto_buf p; > > + struct virtio_crypto_buf q; > > + struct virtio_crypto_buf g; > > +}; > > + > > Pls don't use struct virtio_crypto_buf. > > It seems all below algorithms have the same style, > so I stop reviewing here. > > Regards, > -Gonglei > > > +struct virtio_crypto_dsa_sign_para { > > + /* Hash alogrithms applied to msg */ > > + le32 hash_algo; > > + le32 reserved; > > + > > + /* DSA group parameter */ > > + struct virtio_crypto_dsa_group dsa; > > + /* Random value */ > > + struct virtio_crypto_buf k; > > + /* Private Key */ > > + struct virtio_crypto_buf x; > > +}; > > +
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]