Re: [dpdk-dev] [PATCH v3 4/6] test/crypto: add unit testcase for asym crypto

["De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>]

Hi Shally,

> -----Original Message-----
> From: Shally Verma [mailto:shally.verma@caviumnetworks.com]
> Sent: Wednesday, May 16, 2018 7:05 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; akhil.goyal@nxp.com;
> dev@dpdk.org; pathreya@caviumnetworks.com; Sunila Sahu
> <sunila.sahu@caviumnetworks.com>; Ashish Gupta
> <ashish.gupta@caviumnetworks.com>
> Subject: [PATCH v3 4/6] test/crypto: add unit testcase for asym crypto
> 
> Add unit test case to test openssl PMD asym crypto
> operations. Test case invoke asymmetric operation on DPDK
> Openssl PMD and cross-verify results via Openssl SW library.
> Tests have been verified with openssl 1.0.2m release.

Is it possible to run these tests without using an external library to verify the results,
like what we do with symmetric crypto? Having known answers in the test vectors?
I am not familiar with asymmetric, that's why I am asking.

Also, you are adding asymmetric support for OpenSSL PMD in the 5th patch,
So this test won't work until then.
I think it is better to change the other between patch 4 and 5.

> 
> Tested for:
> 
> * RSA Encrypt, Decrypt, Sign and Verify using pre-defined
>   test vectors
> * Modular Inversion and Exponentiation using pre-defined
>   test vectors
> * Deiffie-Hellman Public key generation using pre-defined
>   private key and dynamically generated test vectors
> * Deffie-hellman private key generation using dynamically
>   generated test vectors
> * Deffie-hellman private and public key pair generation
>   using xform chain and using dynamically generated test
>   vectors
> * Deffie-hellman shared secret compute using dynamically
>   generated test vectors
> * DSA Sign and Verification
> 
> Deffie-hellman testcases use run-time generated test params,
> thus may take some time for execution.
> 
> Changes from v2:
> - Change test application to use the renamed APIs and
>   to use xform type from capability structure
> 
> Signed-off-by: Shally Verma <shally.verma@caviumnetworks.com>
> Signed-off-by: Sunila Sahu <sunila.sahu@caviumnetworks.com>
> Signed-off-by: Ashish Gupta <ashish.gupta@caviumnetworks.com>
> 
> ---
> This patch dependent on asym crypto API patches.
> Please apply them before compilation
> ---
> ---
>  test/test/Makefile              |    3 +-
>  test/test/meson.build           |    1 +
>  test/test/test_cryptodev_asym.c | 1787
> +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1790 insertions(+), 1 deletion(-)
> 
> diff --git a/test/test/Makefile b/test/test/Makefile
> index d1a75fe92..9526b939d 100644
> --- a/test/test/Makefile
> +++ b/test/test/Makefile
> @@ -179,6 +179,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_RING) +=
> test_pmd_ring_perf.c
> 
>  SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev_blockcipher.c
>  SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev.c
> +SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev_asym.c
> 
>  ifeq ($(CONFIG_RTE_COMPRESSDEV_TEST),y)
>  SRCS-$(CONFIG_RTE_LIBRTE_COMPRESSDEV) += test_compressdev.c
> @@ -205,7 +206,7 @@ CFLAGS += $(WERROR_FLAGS)
> 
>  CFLAGS += -D_GNU_SOURCE
> 
> -LDLIBS += -lm
> +LDLIBS += -lm -lcrypto

If openssl libcrypto is actually required, I would add a check similar to
the one just below, with compressdev, to add "-lcrypto", so
users can run the test app without this, if they don't want to test asymmetric.

>  ifeq ($(CONFIG_RTE_COMPRESSDEV_TEST),y)
>  ifeq ($(CONFIG_RTE_LIBRTE_COMPRESSDEV),y)
>  LDLIBS += -lz

..

> +++ b/test/test/test_cryptodev_asym.c
> @@ -0,0 +1,1787 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2017-2018 Cavium Networks
> + */
> +
> +#include <rte_common.h>
> +#include <rte_hexdump.h>
> +#include <rte_mbuf.h>
> +#include <rte_malloc.h>
> +#include <rte_memcpy.h>
> +#include <rte_pause.h>
> +#include <rte_bus_vdev.h>
> +
> +#include <rte_crypto.h>
> +#include <rte_cryptodev.h>
> +#include <rte_cryptodev_pmd.h>
> +#include <openssl/ssl.h>

Move this to the top of the file (as it is an external library to DPDK).

> +
> +#include "test.h"
> +#include "test_cryptodev.h"

...

> +/** rsa xform using exponent key */
> +struct rte_crypto_asym_xform rsa_xform = {
> +	.next = NULL,
> +	.xform_type = RTE_CRYPTO_ASYM_XFORM_RSA,
> +	.rsa = {
> +		.n = {
> +		.data =
> +		(uint8_t *)
> +

For better consistency, could you use the format used above (0x00, 0xB3, 0xA1...)?

> 	("\x00\xb3\xa1\xaf\xb7\x13\x08\x00\x0a\x35\xdc\x2b\x20\x8d"

...

> +#pragma GCC diagnostic pop
> +
> +static int
> +test_rsa(struct rsa_test_data *t)

...

> +	rsa->n =
> +		BN_bin2bn(
> +			(const unsigned char *)rsa_xform.rsa.n.data,
> +			rsa_xform.rsa.n.length,
> +			rsa->n);

I am getting a compilation error:

/test/test/test_cryptodev_asym.c:322:5: error:
dereferencing pointer to incomplete type 'RSA {aka struct rsa_st}'
  rsa->n =
     ^~

My OpenSSL version is 1.1.0h.

Thanks,
Pablo