From: "Trahe, Fiona" <fiona.trahe@intel.com>
To: Shally Verma <shallyv@marvell.com>,
"Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Cc: "akhil.goyal@nxp.com" <akhil.goyal@nxp.com>,
"Trahe, Fiona" <fiona.trahe@intel.com>
Subject: Re: [dpdk-dev] [EXT] [PATCH v3 04/11] test: add cipher field to RSA test
Date: Thu, 18 Jul 2019 12:44:57 +0000 [thread overview]
Message-ID: <348A99DA5F5B7549AA880327E580B435897C7371@IRSMSX101.ger.corp.intel.com> (raw)
In-Reply-To: <BN6PR1801MB20528AB7FC16774AC6554BE3ADC90@BN6PR1801MB2052.namprd18.prod.outlook.com>
Hi Shally, Arek,
> > > >
> > > > > -----Original Message-----
> > > > > From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > > > Sent: Wednesday, July 17, 2019 12:23 AM
> > > > > To: dev@dpdk.org
> > > > > Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com; Shally Verma
> > > > > <shallyv@marvell.com>; Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > > > Subject: [EXT] [PATCH v3 04/11] test: add cipher field to RSA test
> > > > >
> > > > > External Email
> > > > >
> > > > > ------------------------------------------------------------------
> > > > > --
> > > > > -- This patch adds cipher field to RSA test cases
> > > > >
> > > > > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > > > ---
> > > > > app/test/test_cryptodev_asym.c | 3 +++
> > > > > 1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/app/test/test_cryptodev_asym.c
> > > > > b/app/test/test_cryptodev_asym.c index 4dee164..8391545 100644
> > > > > --- a/app/test/test_cryptodev_asym.c
> > > > > +++ b/app/test/test_cryptodev_asym.c
> > > > > @@ -164,6 +164,7 @@ queue_ops_rsa_enc_dec(struct
> > > > > rte_cryptodev_asym_session *sess)
> > > > > uint8_t dev_id = ts_params->valid_devs[0];
> > > > > struct rte_crypto_op *op, *result_op;
> > > > > struct rte_crypto_asym_op *asym_op;
> > > > > + uint8_t cipher_buf[TEST_DATA_SIZE] = {0};
> > > > > int ret, status = TEST_SUCCESS;
> > > > >
> > > > > /* Set up crypto op data structure */ @@ -180,6 +181,8 @@
> > > > > queue_ops_rsa_enc_dec(struct rte_cryptodev_asym_session *sess)
> > > > > asym_op->rsa.op_type = RTE_CRYPTO_ASYM_OP_ENCRYPT;
> > > > >
> > > > > asym_op->rsa.message.data = rsaplaintext.data;
> > > > > + asym_op->rsa.cipher.data = cipher_buf;
> > > > > + asym_op->rsa.cipher.length = 0;
> > > > [Shally] I think this should be initialized to length of buffer
> > > > available i.e. RSA Key size? PMD can override it with length of
> > > > actual data written at output, which has to be less than , equal to
> > RSA_key size.
> > > [AK] - its because API comments are ambiguous in this case and we have
> > > only one field describing array length.
> > > I would suggest to rephrase cipher field API comments from "length in
> > bytes
> > > * of this field needs to be greater or equal to the length of
> > > * corresponding RSA key in bytes"
> > > To "underlying array should have allocated enough memory to hold
> > > cipher output (bigger or equal to RSA key size". Then length could and
> > > I think should be zero or unspecified at this point.
> > > What do you think?
> >
> > [AK2] Something like that:
> > * When RTE_CRYPTO_ASYM_OP_ENCRYPT op_type used underlying
> > array
> > * should have been allocated with enough memory to hold cipher
> > * output (bigger or equal to RSA key size).
> > The same for message field.
> [Shally] This description is okay. But still I would assume app to set length field of cipher buffer to actual
> allocated than 0. But I look forward to more feedback on this from others
[Fiona] I think the important thing is to be clear on when it's an input field and when an output and what the appl or PMD does in each case.
So my understanding is in ENCRYPT case it's an output field and DECRYPT it's an input.
SO how about - combining this with the changes already suggested to avoid repetition in patch 2:
Comment under rte_crypto_rsa_op_param.message:
Pointer to input data
* - to be encrypted for RSA public encrypt.
* - to be signed for RSA sign generation.
* - to be authenticated for RSA sign verification.
Pointer to output data
* - for RSA private decrypt.
In this case the underlying array should have been allocated with
enough memory to hold plaintext output (i.e. must be at least RSA key size).
The message.length field should be 0 and will be overwritten by the PMD
with the decrypted length.
All data is in Octet-string network byte order format.
Note 1: If API allows a length on decrypt, then what would the PMD use it for? Would it have to handle the case where it's less than key-size? In which case the appl is breaking the API and ignoring the previous comment. Or more than key-size - what does the PMD care - it just needs key-size. IF there was a case where PMD could produce more than keysize and would need to know if the buffer is big enough then we should allow this and say it's both an input (buffer-len) and an output (decrypted-message-len). But I don't think there's such a case.
Note 2 : it's good practice for apps to zero all fields in all API structs, except those explicitly set, to allow for future API extensions without ABI breakage.
Comment under rte_crypto_rsa_op_param.cipher:
Pointer to input data
* - to be decrypted for RSA private decrypt.
Pointer to output data
* - for RSA public encrypt.
In this case the underlying array should have been allocated with
enough memory to hold ciphertext output (i.e. must be at least RSA key size).
The message.length field should be 0 and will be overwritten by the PMD
with the encrypted length.
All data is in Octet-string network byte order format.
@Shally - does above make sense?
If so we can update patches 2, 3 and 4 based on above.
next prev parent reply other threads:[~2019-07-18 12:45 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-16 18:52 [dpdk-dev] [PATCH v3 00/11] Rework API for RSA algorithm in asymmetric crypto Arek Kusztal
2019-07-16 18:52 ` [dpdk-dev] [PATCH v3 01/11] cryptodev: change RSA API comments about primes Arek Kusztal
2019-07-17 7:32 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-17 8:39 ` Kusztal, ArkadiuszX
2019-07-16 18:52 ` [dpdk-dev] [PATCH v3 02/11] cryptodev: add cipher field to RSA op Arek Kusztal
2019-07-17 7:39 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-17 16:01 ` Kusztal, ArkadiuszX
2019-07-16 18:52 ` [dpdk-dev] [PATCH v3 03/11] crypto/openssl: add cipher field to openssl RSA implementation Arek Kusztal
2019-07-17 7:50 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-16 18:52 ` [dpdk-dev] [PATCH v3 04/11] test: add cipher field to RSA test Arek Kusztal
2019-07-17 7:41 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-17 8:27 ` Kusztal, ArkadiuszX
2019-07-17 9:42 ` Kusztal, ArkadiuszX
2019-07-17 12:54 ` Shally Verma
2019-07-18 12:44 ` Trahe, Fiona [this message]
2019-07-19 4:10 ` Shally Verma
2019-07-16 18:52 ` [dpdk-dev] [PATCH v3 05/11] cryptodev: add information about message format when signing with RSA Arek Kusztal
2019-07-17 10:07 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-17 10:26 ` Kusztal, ArkadiuszX
2019-07-16 18:52 ` [dpdk-dev] [PATCH v3 06/11] cryptodev: remove RSA PKCS1 BT0 padding Arek Kusztal
2019-07-17 10:09 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-16 18:53 ` [dpdk-dev] [PATCH v3 07/11] openssl: remove RSA PKCS1_5 " Arek Kusztal
2019-07-17 10:18 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-16 18:53 ` [dpdk-dev] [PATCH v3 08/11] test: remove RSA PKCS1_5 BT0 padding from test cases Arek Kusztal
2019-07-17 10:10 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-16 18:53 ` [dpdk-dev] [PATCH v3 09/11] cryptodev: add RSA padding none description Arek Kusztal
2019-07-17 10:17 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-17 10:40 ` Kusztal, ArkadiuszX
2019-07-16 18:53 ` [dpdk-dev] [PATCH v3 10/11] test: add pkcs1_5 padding simulation Arek Kusztal
2019-07-17 10:22 ` [dpdk-dev] [EXT] " Shally Verma
2019-07-17 10:28 ` Kusztal, ArkadiuszX
2019-07-16 18:53 ` [dpdk-dev] [PATCH v3 11/11] test: add RSA PKCS1_5 padding case when no padding selected Arek Kusztal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=348A99DA5F5B7549AA880327E580B435897C7371@IRSMSX101.ger.corp.intel.com \
--to=fiona.trahe@intel.com \
--cc=akhil.goyal@nxp.com \
--cc=arkadiuszx.kusztal@intel.com \
--cc=dev@dpdk.org \
--cc=shallyv@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).