diff mbox

[v0] crypto : cunit test suite for rng

Message ID 1416841456-28291-1-git-send-email-alexandru.badicioiu@linaro.org
State New
Headers show

Commit Message

Alexandru Badicioiu Nov. 24, 2014, 3:04 p.m. UTC
From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>

Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
---
 test/validation/Makefile.am                  |    1 +
 test/validation/crypto/odp_crypto_test_rng.c |   31 ++++++++++++++++++++++++++
 test/validation/crypto/odp_crypto_test_rng.h |   18 +++++++++++++++
 test/validation/crypto/test_vectors.h        |   21 ++++++++++-------
 test/validation/odp_crypto.c                 |    2 +
 5 files changed, 64 insertions(+), 9 deletions(-)
 create mode 100644 test/validation/crypto/odp_crypto_test_rng.c
 create mode 100644 test/validation/crypto/odp_crypto_test_rng.h

Comments

Mike Holmes Nov. 24, 2014, 6:58 p.m. UTC | #1
Patch name should drop v0, it is assumed for v0
crypto :   -> crypto:  we have not been inserting a space before the colon

Applying: crypto : cunit test suite for rng
/home/mike/git/odp/.git/rebase-apply/patch:81: new blank line at EOF.

Other comments in line

On 24 November 2014 10:04,  <alexandru.badicioiu@linaro.org> wrote:
> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
>
> Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
> ---
>  test/validation/Makefile.am                  |    1 +
>  test/validation/crypto/odp_crypto_test_rng.c |   31 ++++++++++++++++++++++++++
>  test/validation/crypto/odp_crypto_test_rng.h |   18 +++++++++++++++
>  test/validation/crypto/test_vectors.h        |   21 ++++++++++-------
>  test/validation/odp_crypto.c                 |    2 +
>  5 files changed, 64 insertions(+), 9 deletions(-)
>  create mode 100644 test/validation/crypto/odp_crypto_test_rng.c
>  create mode 100644 test/validation/crypto/odp_crypto_test_rng.h
>
> diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am
> index 0b831d0..52dbd50 100644
> --- a/test/validation/Makefile.am
> +++ b/test/validation/Makefile.am
> @@ -17,4 +17,5 @@ dist_odp_init_SOURCES = odp_init.c
>  dist_odp_queue_SOURCES = odp_queue.c
>  dist_odp_crypto_SOURCES = crypto/odp_crypto_test_async_inp.c \
>                           crypto/odp_crypto_test_sync_inp.c \
> +                         crypto/odp_crypto_test_rng.c \
>                           odp_crypto.c
> diff --git a/test/validation/crypto/odp_crypto_test_rng.c b/test/validation/crypto/odp_crypto_test_rng.c
> new file mode 100644
> index 0000000..2e25027
> --- /dev/null
> +++ b/test/validation/crypto/odp_crypto_test_rng.c
> @@ -0,0 +1,31 @@
> +/* Copyright (c) 2014, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +#include <odp.h>
> +#include <odp_crypto.h>

Unit test should try to only use odp.h to test the public API without
hidden features being used,  this test works without odp_crypto.h

> +#include "CUnit/Basic.h"
> +#include "CUnit/TestDB.h"
> +#include "test_vectors.h"

Need to include odp_crypto_test_rng.h - this also means that TestDB.h
can be dropped here.

> +
> +/*
> + * This test verifies that HW random number generator is able
> + * to produce an IV for TDES_CBC cipher algorithm.
> + * */
> +#define RNG_GET_SIZE   "RNG_GET_SIZE"
> +static void rng_get_size(void)
> +{
> +       int ret;
> +       size_t len = TDES_CBC_IV_LEN;
> +       uint8_t buf[TDES_CBC_IV_LEN];
> +
> +       ret = odp_hw_random_get(buf, &len, false);
> +       CU_ASSERT(!ret);
> +       CU_ASSERT(len == TDES_CBC_IV_LEN);
> +}
> +
> +CU_TestInfo test_rng[] = {
> +       { RNG_GET_SIZE, rng_get_size },
> +       CU_TEST_INFO_NULL,

Size of test_rng is defined as a size of one in the header but is size
of two elements here.

> +};
> diff --git a/test/validation/crypto/odp_crypto_test_rng.h b/test/validation/crypto/odp_crypto_test_rng.h
> new file mode 100644
> index 0000000..438761d
> --- /dev/null
> +++ b/test/validation/crypto/odp_crypto_test_rng.h
> @@ -0,0 +1,18 @@
> +/* Copyright (c) 2014, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +#ifndef ODP_CRYPTO_TEST_RNG_
> +#define ODP_CRYPTO_TEST_RNG_
> +
> +#include "CUnit/TestDB.h"
> +
> +/* Suite name */
> +#define ODP_CRYPTO_RNG    "ODP_CRYPTO_RNG"
> +
> +/* Suite test array */
> +CU_TestInfo test_rng[1];

This is hard defined, this value should be used in the c file if the
size of the array is to be defined explicitly.
nit: maybe the 1 could be defined as NUM_RNG_TESTS.


> +
> +#endif
> +
> diff --git a/test/validation/crypto/test_vectors.h b/test/validation/crypto/test_vectors.h
> index c151952..aaf0103 100644
> --- a/test/validation/crypto/test_vectors.h
> +++ b/test/validation/crypto/test_vectors.h
> @@ -13,7 +13,7 @@
>  #define TDES_CBC_IV_LEN                8       /* IV length(in bytes) for tdes-cbc */
>  #define TDES_CBC_MAX_DATA_LEN  16      /* max. plain text length(in bytes) */
>
> -static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] = {
> +static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] ODP_UNUSED

Not sure why this is flagged unused it appears to be used in
crypto/odp_crypto_test_async_inp.c and
crypto/odp_crypto_test_sync_inp.c

ODP_UNUSED will be moving to odp_debug_internal, best to add a test
variant (TEST_UNUSED ?)  to test/test_debug.h.  Same for other
instances if it really should be flagged unused.

 = {
>         {0x62, 0x7f, 0x46, 0x0e, 0x08, 0x10, 0x4a, 0x10, 0x43, 0xcd, 0x26, 0x5d,
>          0x58, 0x40, 0xea, 0xf1, 0x31, 0x3e, 0xdf, 0x97, 0xdf, 0x2a, 0x8a, 0x8c,
>          },
> @@ -22,23 +22,25 @@ static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] = {
>          0x31, 0xcb, 0xb3, 0x85, 0x5e, 0x7f, 0xd3, 0x6d, 0xc8, 0x70, 0xbf, 0xae}
>  };
>
> -static uint8_t tdes_cbc_reference_iv[][TDES_CBC_IV_LEN] = {
> +static uint8_t tdes_cbc_reference_iv[][TDES_CBC_IV_LEN] ODP_UNUSED = {
>         {0x8e, 0x29, 0xf7, 0x5e, 0xa7, 0x7e, 0x54, 0x75},
>
>         {0x3d, 0x1d, 0xe3, 0xcc, 0x13, 0x2e, 0x3b, 0x65}
>  };
>
>  /** length in bytes */
> -static uint32_t tdes_cbc_reference_length[] = { 8, 16 };
> +static uint32_t tdes_cbc_reference_length[] ODP_UNUSED = { 8, 16 };
>
> -static uint8_t tdes_cbc_reference_plaintext[][TDES_CBC_MAX_DATA_LEN] = {
> +static uint8_t
> +tdes_cbc_reference_plaintext[][TDES_CBC_MAX_DATA_LEN] ODP_UNUSED = {
>         {0x32, 0x6a, 0x49, 0x4c, 0xd3, 0x3f, 0xe7, 0x56},
>
>         {0x84, 0x40, 0x1f, 0x78, 0xfe, 0x6c, 0x10, 0x87, 0x6d, 0x8e, 0xa2, 0x30,
>          0x94, 0xea, 0x53, 0x09}
>  };
>
> -static uint8_t tdes_cbc_reference_ciphertext[][TDES_CBC_MAX_DATA_LEN] = {
> +static uint8_t
> +tdes_cbc_reference_ciphertext[][TDES_CBC_MAX_DATA_LEN] ODP_UNUSED = {
>         {0xb2, 0x2b, 0x8d, 0x66, 0xde, 0x97, 0x06, 0x92},
>
>         {0x7b, 0x1f, 0x7c, 0x7e, 0x3b, 0x1c, 0x94, 0x8e, 0xbd, 0x04, 0xa7, 0x5f,
> @@ -52,7 +54,7 @@ static uint8_t tdes_cbc_reference_ciphertext[][TDES_CBC_MAX_DATA_LEN] = {
>  #define HMAC_MD5_DIGEST_LEN    16
>  #define HMAC_MD5_96_CHECK_LEN  12
>
> -static uint8_t hmac_md5_reference_key[][HMAC_MD5_KEY_LEN] = {
> +static uint8_t hmac_md5_reference_key[][HMAC_MD5_KEY_LEN] ODP_UNUSED = {
>         { 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b,
>           0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b } ,
>
> @@ -63,9 +65,10 @@ static uint8_t hmac_md5_reference_key[][HMAC_MD5_KEY_LEN] = {
>           0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa }
>  };
>
> -static uint32_t hmac_md5_reference_length[] = { 8, 28, 50 };
> +static uint32_t hmac_md5_reference_length[] ODP_UNUSED = { 8, 28, 50 };
>
> -static uint8_t hmac_md5_reference_plaintext[][HMAC_MD5_MAX_DATA_LEN] = {
> +static uint8_t
> +hmac_md5_reference_plaintext[][HMAC_MD5_MAX_DATA_LEN] ODP_UNUSED = {
>         /* "Hi There" */
>         { 0x48, 0x69, 0x20, 0x54, 0x68, 0x65, 0x72, 0x65},
>
> @@ -82,7 +85,7 @@ static uint8_t hmac_md5_reference_plaintext[][HMAC_MD5_MAX_DATA_LEN] = {
>           0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd }
>  };
>
> -static uint8_t hmac_md5_reference_digest[][HMAC_MD5_DIGEST_LEN] = {
> +static uint8_t hmac_md5_reference_digest[][HMAC_MD5_DIGEST_LEN] ODP_UNUSED = {
>         { 0x92, 0x94, 0x72, 0x7a, 0x36, 0x38, 0xbb, 0x1c,
>           0x13, 0xf4, 0x8e, 0xf8, 0x15, 0x8b, 0xfc, 0x9d },
>
> diff --git a/test/validation/odp_crypto.c b/test/validation/odp_crypto.c
> index 985302a..6b53952 100644
> --- a/test/validation/odp_crypto.c
> +++ b/test/validation/odp_crypto.c
> @@ -9,6 +9,7 @@
>  #include "CUnit/TestDB.h"
>  #include "odp_crypto_test_async_inp.h"
>  #include "odp_crypto_test_sync_inp.h"
> +#include "odp_crypto_test_rng.h"
>
>  #define SHM_PKT_POOL_SIZE      (512*2048*2)
>  #define SHM_PKT_POOL_BUF_SIZE  (1024 * 32)
> @@ -19,6 +20,7 @@
>  CU_SuiteInfo suites[] = {
>         { ODP_CRYPTO_SYNC_INP , NULL, NULL, NULL, NULL, test_array_sync },
>         { ODP_CRYPTO_ASYNC_INP , NULL, NULL, NULL, NULL, test_array_async },
> +       { ODP_CRYPTO_RNG, NULL, NULL, NULL, NULL, test_rng },
>         CU_SUITE_INFO_NULL,
>  };
>
> --
> 1.7.3.4
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Alexandru Badicioiu Nov. 25, 2014, 10:21 a.m. UTC | #2
On 24 November 2014 at 20:58, Mike Holmes <mike.holmes@linaro.org> wrote:

> Patch name should drop v0, it is assumed for v0
> crypto :   -> crypto:  we have not been inserting a space before the colon
>


> Applying: crypto : cunit test suite for rng
> /home/mike/git/odp/.git/rebase-apply/patch:81: new blank line at EOF.
> [Alex] Will fix these.



> Other comments in line
>
> On 24 November 2014 10:04,  <alexandru.badicioiu@linaro.org> wrote:
> > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
> >
> > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
> > ---
> >  test/validation/Makefile.am                  |    1 +
> >  test/validation/crypto/odp_crypto_test_rng.c |   31
> ++++++++++++++++++++++++++
> >  test/validation/crypto/odp_crypto_test_rng.h |   18 +++++++++++++++
> >  test/validation/crypto/test_vectors.h        |   21 ++++++++++-------
> >  test/validation/odp_crypto.c                 |    2 +
> >  5 files changed, 64 insertions(+), 9 deletions(-)
> >  create mode 100644 test/validation/crypto/odp_crypto_test_rng.c
> >  create mode 100644 test/validation/crypto/odp_crypto_test_rng.h
> >
> > diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am
> > index 0b831d0..52dbd50 100644
> > --- a/test/validation/Makefile.am
> > +++ b/test/validation/Makefile.am
> > @@ -17,4 +17,5 @@ dist_odp_init_SOURCES = odp_init.c
> >  dist_odp_queue_SOURCES = odp_queue.c
> >  dist_odp_crypto_SOURCES = crypto/odp_crypto_test_async_inp.c \
> >                           crypto/odp_crypto_test_sync_inp.c \
> > +                         crypto/odp_crypto_test_rng.c \
> >                           odp_crypto.c
> > diff --git a/test/validation/crypto/odp_crypto_test_rng.c
> b/test/validation/crypto/odp_crypto_test_rng.c
> > new file mode 100644
> > index 0000000..2e25027
> > --- /dev/null
> > +++ b/test/validation/crypto/odp_crypto_test_rng.c
> > @@ -0,0 +1,31 @@
> > +/* Copyright (c) 2014, Linaro Limited
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier:     BSD-3-Clause
> > + */
> > +#include <odp.h>
> > +#include <odp_crypto.h>
>
> Unit test should try to only use odp.h to test the public API without
> hidden features being used,  this test works without odp_crypto.h
>
[Alex] Will remove odp_crypto.h inclusion.

>
> > +#include "CUnit/Basic.h"
> > +#include "CUnit/TestDB.h"
> > +#include "test_vectors.h"
>
> Need to include odp_crypto_test_rng.h - this also means that TestDB.h
> can be dropped here.
>
[Alex] The purpose of header file is to declare test_rng array to be used
in other source files. Why should it be included where test_rng array is
defined?

>
> > +
> > +/*
> > + * This test verifies that HW random number generator is able
> > + * to produce an IV for TDES_CBC cipher algorithm.
> > + * */
> > +#define RNG_GET_SIZE   "RNG_GET_SIZE"
> > +static void rng_get_size(void)
> > +{
> > +       int ret;
> > +       size_t len = TDES_CBC_IV_LEN;
> > +       uint8_t buf[TDES_CBC_IV_LEN];
> > +
> > +       ret = odp_hw_random_get(buf, &len, false);
> > +       CU_ASSERT(!ret);
> > +       CU_ASSERT(len == TDES_CBC_IV_LEN);
> > +}
> > +
> > +CU_TestInfo test_rng[] = {
> > +       { RNG_GET_SIZE, rng_get_size },
> > +       CU_TEST_INFO_NULL,
>
> Size of test_rng is defined as a size of one in the header but is size
> of two elements here.
>
[Alex] CUnit does not use the declared size of the array  - I guess this is
the purpose of the end marker (CU_TEST_INFO_NULL). Declaring a pointer only
it will not compile :
odp_crypto.c:23:2: error: initializer element is not constant
odp_crypto.c:23:2: error: (near initialization for 'suites[2].pTests')
odp_crypto.c:23:2: error: missing initializer
[-Werror=missing-field-initializers]
odp_crypto.c:23:2: error: (near initialization for 'suites[2].pTests')
[-Werror=missing-field-initializers]
Trying to declare the array as flexible will not work either (CU_TestInfo
test_rng[];):
In file included from odp_crypto.c:12:0:
./crypto/odp_crypto_test_rng.h:15:13: error: array 'test_rng' assumed to
have one element [-Werror]
cc1: all warnings being treated as errors
This is the reason I used 1 as the declared size of the array.

>
> > +};
> > diff --git a/test/validation/crypto/odp_crypto_test_rng.h
> b/test/validation/crypto/odp_crypto_test_rng.h
> > new file mode 100644
> > index 0000000..438761d
> > --- /dev/null
> > +++ b/test/validation/crypto/odp_crypto_test_rng.h
> > @@ -0,0 +1,18 @@
> > +/* Copyright (c) 2014, Linaro Limited
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier:     BSD-3-Clause
> > + */
> > +#ifndef ODP_CRYPTO_TEST_RNG_
> > +#define ODP_CRYPTO_TEST_RNG_
> > +
> > +#include "CUnit/TestDB.h"
> > +
> > +/* Suite name */
> > +#define ODP_CRYPTO_RNG    "ODP_CRYPTO_RNG"
> > +
> > +/* Suite test array */
> > +CU_TestInfo test_rng[1];
>
> This is hard defined, this value should be used in the c file if the
> size of the array is to be defined explicitly.
> nit: maybe the 1 could be defined as NUM_RNG_TESTS.
> [Alex] As above, the real size of the array is not needed by CUnit.
>
> > +
> > +#endif
> > +
> > diff --git a/test/validation/crypto/test_vectors.h
> b/test/validation/crypto/test_vectors.h
> > index c151952..aaf0103 100644
> > --- a/test/validation/crypto/test_vectors.h
> > +++ b/test/validation/crypto/test_vectors.h
> > @@ -13,7 +13,7 @@
> >  #define TDES_CBC_IV_LEN                8       /* IV length(in bytes)
> for tdes-cbc */
> >  #define TDES_CBC_MAX_DATA_LEN  16      /* max. plain text length(in
> bytes) */
> >
> > -static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] = {
> > +static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] ODP_UNUSED
>
> Not sure why this is flagged unused it appears to be used in
> crypto/odp_crypto_test_async_inp.c and
> crypto/odp_crypto_test_sync_inp.c
>
[Alex] These are static declaration, as such they are reported unused in
test_rng.c file where they are unused. To avoid unused, they should be
moved to a .c file and only be declared in test_vectors.h.


> ODP_UNUSED will be moving to odp_debug_internal, best to add a test
> variant (TEST_UNUSED ?)  to test/test_debug.h.  Same for other
> instances if it really should be flagged unused.
>
>  = {
> >         {0x62, 0x7f, 0x46, 0x0e, 0x08, 0x10, 0x4a, 0x10, 0x43, 0xcd,
> 0x26, 0x5d,
> >          0x58, 0x40, 0xea, 0xf1, 0x31, 0x3e, 0xdf, 0x97, 0xdf, 0x2a,
> 0x8a, 0x8c,
> >          },
> > @@ -22,23 +22,25 @@ static uint8_t
> tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] = {
> >          0x31, 0xcb, 0xb3, 0x85, 0x5e, 0x7f, 0xd3, 0x6d, 0xc8, 0x70,
> 0xbf, 0xae}
> >  };
> >
> > -static uint8_t tdes_cbc_reference_iv[][TDES_CBC_IV_LEN] = {
> > +static uint8_t tdes_cbc_reference_iv[][TDES_CBC_IV_LEN] ODP_UNUSED = {
> >         {0x8e, 0x29, 0xf7, 0x5e, 0xa7, 0x7e, 0x54, 0x75},
> >
> >         {0x3d, 0x1d, 0xe3, 0xcc, 0x13, 0x2e, 0x3b, 0x65}
> >  };
> >
> >  /** length in bytes */
> > -static uint32_t tdes_cbc_reference_length[] = { 8, 16 };
> > +static uint32_t tdes_cbc_reference_length[] ODP_UNUSED = { 8, 16 };
> >
> > -static uint8_t tdes_cbc_reference_plaintext[][TDES_CBC_MAX_DATA_LEN] = {
> > +static uint8_t
> > +tdes_cbc_reference_plaintext[][TDES_CBC_MAX_DATA_LEN] ODP_UNUSED = {
> >         {0x32, 0x6a, 0x49, 0x4c, 0xd3, 0x3f, 0xe7, 0x56},
> >
> >         {0x84, 0x40, 0x1f, 0x78, 0xfe, 0x6c, 0x10, 0x87, 0x6d, 0x8e,
> 0xa2, 0x30,
> >          0x94, 0xea, 0x53, 0x09}
> >  };
> >
> > -static uint8_t tdes_cbc_reference_ciphertext[][TDES_CBC_MAX_DATA_LEN] =
> {
> > +static uint8_t
> > +tdes_cbc_reference_ciphertext[][TDES_CBC_MAX_DATA_LEN] ODP_UNUSED = {
> >         {0xb2, 0x2b, 0x8d, 0x66, 0xde, 0x97, 0x06, 0x92},
> >
> >         {0x7b, 0x1f, 0x7c, 0x7e, 0x3b, 0x1c, 0x94, 0x8e, 0xbd, 0x04,
> 0xa7, 0x5f,
> > @@ -52,7 +54,7 @@ static uint8_t
> tdes_cbc_reference_ciphertext[][TDES_CBC_MAX_DATA_LEN] = {
> >  #define HMAC_MD5_DIGEST_LEN    16
> >  #define HMAC_MD5_96_CHECK_LEN  12
> >
> > -static uint8_t hmac_md5_reference_key[][HMAC_MD5_KEY_LEN] = {
> > +static uint8_t hmac_md5_reference_key[][HMAC_MD5_KEY_LEN] ODP_UNUSED = {
> >         { 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b,
> >           0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b } ,
> >
> > @@ -63,9 +65,10 @@ static uint8_t
> hmac_md5_reference_key[][HMAC_MD5_KEY_LEN] = {
> >           0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa }
> >  };
> >
> > -static uint32_t hmac_md5_reference_length[] = { 8, 28, 50 };
> > +static uint32_t hmac_md5_reference_length[] ODP_UNUSED = { 8, 28, 50 };
> >
> > -static uint8_t hmac_md5_reference_plaintext[][HMAC_MD5_MAX_DATA_LEN] = {
> > +static uint8_t
> > +hmac_md5_reference_plaintext[][HMAC_MD5_MAX_DATA_LEN] ODP_UNUSED = {
> >         /* "Hi There" */
> >         { 0x48, 0x69, 0x20, 0x54, 0x68, 0x65, 0x72, 0x65},
> >
> > @@ -82,7 +85,7 @@ static uint8_t
> hmac_md5_reference_plaintext[][HMAC_MD5_MAX_DATA_LEN] = {
> >           0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd }
> >  };
> >
> > -static uint8_t hmac_md5_reference_digest[][HMAC_MD5_DIGEST_LEN] = {
> > +static uint8_t hmac_md5_reference_digest[][HMAC_MD5_DIGEST_LEN]
> ODP_UNUSED = {
> >         { 0x92, 0x94, 0x72, 0x7a, 0x36, 0x38, 0xbb, 0x1c,
> >           0x13, 0xf4, 0x8e, 0xf8, 0x15, 0x8b, 0xfc, 0x9d },
> >
> > diff --git a/test/validation/odp_crypto.c b/test/validation/odp_crypto.c
> > index 985302a..6b53952 100644
> > --- a/test/validation/odp_crypto.c
> > +++ b/test/validation/odp_crypto.c
> > @@ -9,6 +9,7 @@
> >  #include "CUnit/TestDB.h"
> >  #include "odp_crypto_test_async_inp.h"
> >  #include "odp_crypto_test_sync_inp.h"
> > +#include "odp_crypto_test_rng.h"
> >
> >  #define SHM_PKT_POOL_SIZE      (512*2048*2)
> >  #define SHM_PKT_POOL_BUF_SIZE  (1024 * 32)
> > @@ -19,6 +20,7 @@
> >  CU_SuiteInfo suites[] = {
> >         { ODP_CRYPTO_SYNC_INP , NULL, NULL, NULL, NULL, test_array_sync
> },
> >         { ODP_CRYPTO_ASYNC_INP , NULL, NULL, NULL, NULL,
> test_array_async },
> > +       { ODP_CRYPTO_RNG, NULL, NULL, NULL, NULL, test_rng },
> >         CU_SUITE_INFO_NULL,
> >  };
> >
> > --
> > 1.7.3.4
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
> --
> Mike Holmes
> Linaro  Sr Technical Manager
> LNG - ODP
>
Taras Kondratiuk Nov. 25, 2014, 1:10 p.m. UTC | #3
On 11/25/2014 12:21 PM, Alexandru Badicioiu wrote:

>     > +
>     > +/*
>     > + * This test verifies that HW random number generator is able
>     > + * to produce an IV for TDES_CBC cipher algorithm.
>     > + * */
>     > +#define RNG_GET_SIZE   "RNG_GET_SIZE"
>     > +static void rng_get_size(void)
>     > +{
>     > +       int ret;
>     > +       size_t len = TDES_CBC_IV_LEN;
>     > +       uint8_t buf[TDES_CBC_IV_LEN];
>     > +
>     > +       ret = odp_hw_random_get(buf, &len, false);
>     > +       CU_ASSERT(!ret);
>     > +       CU_ASSERT(len == TDES_CBC_IV_LEN);
>     > +}
>     > +
>     > +CU_TestInfo test_rng[] = {
>     > +       { RNG_GET_SIZE, rng_get_size },
>     > +       CU_TEST_INFO_NULL,
>
>     Size of test_rng is defined as a size of one in the header but is size
>     of two elements here.
>
> [Alex] CUnit does not use the declared size of the array  - I guess this
> is the purpose of the end marker (CU_TEST_INFO_NULL). Declaring a
> pointer only it will not compile :
> odp_crypto.c:23:2: error: initializer element is not constant
> odp_crypto.c:23:2: error: (near initialization for 'suites[2].pTests')
> odp_crypto.c:23:2: error: missing initializer
> [-Werror=missing-field-initializers]
> odp_crypto.c:23:2: error: (near initialization for 'suites[2].pTests')
> [-Werror=missing-field-initializers]
> Trying to declare the array as flexible will not work either
> (CU_TestInfo test_rng[];):
> In file included from odp_crypto.c:12:0:
> ./crypto/odp_crypto_test_rng.h:15:13: error: array 'test_rng' assumed to
> have one element [-Werror]
> cc1: all warnings being treated as errors
> This is the reason I used 1 as the declared size of the array.

This should work:
extern CU_TestInfo test_rng[];

>     > +
>     > +#endif
>     > +
>     > diff --git a/test/validation/crypto/test_vectors.h b/test/validation/crypto/test_vectors.h
>     > index c151952..aaf0103 100644
>     > --- a/test/validation/crypto/test_vectors.h
>     > +++ b/test/validation/crypto/test_vectors.h
>     > @@ -13,7 +13,7 @@
>     >  #define TDES_CBC_IV_LEN                8       /* IV length(in bytes) for tdes-cbc */
>     >  #define TDES_CBC_MAX_DATA_LEN  16      /* max. plain text length(in bytes) */
>     >
>     > -static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] = {
>     > +static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] ODP_UNUSED
>
>     Not sure why this is flagged unused it appears to be used in
>     crypto/odp_crypto_test_async_inp.c and
>     crypto/odp_crypto_test_sync_inp.c
>
> [Alex] These are static declaration, as such they are reported unused in
> test_rng.c file where they are unused. To avoid unused, they should be
> moved to a .c file and only be declared in test_vectors.h.

That's a hacky solution. You can split these structures into a separate
.h and not include them in RNG tests.
Alexandru Badicioiu Nov. 25, 2014, 1:22 p.m. UTC | #4
On 25 November 2014 at 15:10, Taras Kondratiuk <taras.kondratiuk@linaro.org>
wrote:

> On 11/25/2014 12:21 PM, Alexandru Badicioiu wrote:
>
>      > +
>>     > +/*
>>     > + * This test verifies that HW random number generator is able
>>     > + * to produce an IV for TDES_CBC cipher algorithm.
>>     > + * */
>>     > +#define RNG_GET_SIZE   "RNG_GET_SIZE"
>>     > +static void rng_get_size(void)
>>     > +{
>>     > +       int ret;
>>     > +       size_t len = TDES_CBC_IV_LEN;
>>     > +       uint8_t buf[TDES_CBC_IV_LEN];
>>     > +
>>     > +       ret = odp_hw_random_get(buf, &len, false);
>>     > +       CU_ASSERT(!ret);
>>     > +       CU_ASSERT(len == TDES_CBC_IV_LEN);
>>     > +}
>>     > +
>>     > +CU_TestInfo test_rng[] = {
>>     > +       { RNG_GET_SIZE, rng_get_size },
>>     > +       CU_TEST_INFO_NULL,
>>
>>     Size of test_rng is defined as a size of one in the header but is size
>>     of two elements here.
>>
>> [Alex] CUnit does not use the declared size of the array  - I guess this
>> is the purpose of the end marker (CU_TEST_INFO_NULL). Declaring a
>> pointer only it will not compile :
>> odp_crypto.c:23:2: error: initializer element is not constant
>> odp_crypto.c:23:2: error: (near initialization for 'suites[2].pTests')
>> odp_crypto.c:23:2: error: missing initializer
>> [-Werror=missing-field-initializers]
>> odp_crypto.c:23:2: error: (near initialization for 'suites[2].pTests')
>> [-Werror=missing-field-initializers]
>> Trying to declare the array as flexible will not work either
>> (CU_TestInfo test_rng[];):
>> In file included from odp_crypto.c:12:0:
>> ./crypto/odp_crypto_test_rng.h:15:13: error: array 'test_rng' assumed to
>> have one element [-Werror]
>> cc1: all warnings being treated as errors
>> This is the reason I used 1 as the declared size of the array.
>>
>
> This should work:
> extern CU_TestInfo test_rng[];

[Alex] Thanks. It seems I missed that the array is an extern and it should
be declared as such.

>
>
>      > +
>>     > +#endif
>>     > +
>>     > diff --git a/test/validation/crypto/test_vectors.h
>> b/test/validation/crypto/test_vectors.h
>>     > index c151952..aaf0103 100644
>>     > --- a/test/validation/crypto/test_vectors.h
>>     > +++ b/test/validation/crypto/test_vectors.h
>>     > @@ -13,7 +13,7 @@
>>     >  #define TDES_CBC_IV_LEN                8       /* IV length(in
>> bytes) for tdes-cbc */
>>     >  #define TDES_CBC_MAX_DATA_LEN  16      /* max. plain text
>> length(in bytes) */
>>     >
>>     > -static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] = {
>>     > +static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN]
>> ODP_UNUSED
>>
>>     Not sure why this is flagged unused it appears to be used in
>>     crypto/odp_crypto_test_async_inp.c and
>>     crypto/odp_crypto_test_sync_inp.c
>>
>> [Alex] These are static declaration, as such they are reported unused in
>> test_rng.c file where they are unused. To avoid unused, they should be
>> moved to a .c file and only be declared in test_vectors.h.
>>
>
> That's a hacky solution. You can split these structures into a separate
> .h and not include them in RNG tests.
>
[Alex] Following this approach we may end up with an unpractical amount of
files. The present test uses  only TDES_CBC_IV_LEN and it would mean to
have a .h file only with definitions for IV lengths.
diff mbox

Patch

diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am
index 0b831d0..52dbd50 100644
--- a/test/validation/Makefile.am
+++ b/test/validation/Makefile.am
@@ -17,4 +17,5 @@  dist_odp_init_SOURCES = odp_init.c
 dist_odp_queue_SOURCES = odp_queue.c
 dist_odp_crypto_SOURCES = crypto/odp_crypto_test_async_inp.c \
 			  crypto/odp_crypto_test_sync_inp.c \
+			  crypto/odp_crypto_test_rng.c \
 			  odp_crypto.c
diff --git a/test/validation/crypto/odp_crypto_test_rng.c b/test/validation/crypto/odp_crypto_test_rng.c
new file mode 100644
index 0000000..2e25027
--- /dev/null
+++ b/test/validation/crypto/odp_crypto_test_rng.c
@@ -0,0 +1,31 @@ 
+/* Copyright (c) 2014, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:     BSD-3-Clause
+ */
+#include <odp.h>
+#include <odp_crypto.h>
+#include "CUnit/Basic.h"
+#include "CUnit/TestDB.h"
+#include "test_vectors.h"
+
+/*
+ * This test verifies that HW random number generator is able
+ * to produce an IV for TDES_CBC cipher algorithm.
+ * */
+#define RNG_GET_SIZE	"RNG_GET_SIZE"
+static void rng_get_size(void)
+{
+	int ret;
+	size_t len = TDES_CBC_IV_LEN;
+	uint8_t buf[TDES_CBC_IV_LEN];
+
+	ret = odp_hw_random_get(buf, &len, false);
+	CU_ASSERT(!ret);
+	CU_ASSERT(len == TDES_CBC_IV_LEN);
+}
+
+CU_TestInfo test_rng[] = {
+	{ RNG_GET_SIZE, rng_get_size },
+	CU_TEST_INFO_NULL,
+};
diff --git a/test/validation/crypto/odp_crypto_test_rng.h b/test/validation/crypto/odp_crypto_test_rng.h
new file mode 100644
index 0000000..438761d
--- /dev/null
+++ b/test/validation/crypto/odp_crypto_test_rng.h
@@ -0,0 +1,18 @@ 
+/* Copyright (c) 2014, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:     BSD-3-Clause
+ */
+#ifndef ODP_CRYPTO_TEST_RNG_
+#define ODP_CRYPTO_TEST_RNG_
+
+#include "CUnit/TestDB.h"
+
+/* Suite name */
+#define ODP_CRYPTO_RNG    "ODP_CRYPTO_RNG"
+
+/* Suite test array */
+CU_TestInfo test_rng[1];
+
+#endif
+
diff --git a/test/validation/crypto/test_vectors.h b/test/validation/crypto/test_vectors.h
index c151952..aaf0103 100644
--- a/test/validation/crypto/test_vectors.h
+++ b/test/validation/crypto/test_vectors.h
@@ -13,7 +13,7 @@ 
 #define TDES_CBC_IV_LEN		8	/* IV length(in bytes) for tdes-cbc */
 #define TDES_CBC_MAX_DATA_LEN	16	/* max. plain text length(in bytes) */
 
-static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] = {
+static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] ODP_UNUSED = {
 	{0x62, 0x7f, 0x46, 0x0e, 0x08, 0x10, 0x4a, 0x10, 0x43, 0xcd, 0x26, 0x5d,
 	 0x58, 0x40, 0xea, 0xf1, 0x31, 0x3e, 0xdf, 0x97, 0xdf, 0x2a, 0x8a, 0x8c,
 	 },
@@ -22,23 +22,25 @@  static uint8_t tdes_cbc_reference_key[][TDES_CBC_KEY_LEN] = {
 	 0x31, 0xcb, 0xb3, 0x85, 0x5e, 0x7f, 0xd3, 0x6d, 0xc8, 0x70, 0xbf, 0xae}
 };
 
-static uint8_t tdes_cbc_reference_iv[][TDES_CBC_IV_LEN] = {
+static uint8_t tdes_cbc_reference_iv[][TDES_CBC_IV_LEN] ODP_UNUSED = {
 	{0x8e, 0x29, 0xf7, 0x5e, 0xa7, 0x7e, 0x54, 0x75},
 
 	{0x3d, 0x1d, 0xe3, 0xcc, 0x13, 0x2e, 0x3b, 0x65}
 };
 
 /** length in bytes */
-static uint32_t tdes_cbc_reference_length[] = { 8, 16 };
+static uint32_t tdes_cbc_reference_length[] ODP_UNUSED = { 8, 16 };
 
-static uint8_t tdes_cbc_reference_plaintext[][TDES_CBC_MAX_DATA_LEN] = {
+static uint8_t
+tdes_cbc_reference_plaintext[][TDES_CBC_MAX_DATA_LEN] ODP_UNUSED = {
 	{0x32, 0x6a, 0x49, 0x4c, 0xd3, 0x3f, 0xe7, 0x56},
 
 	{0x84, 0x40, 0x1f, 0x78, 0xfe, 0x6c, 0x10, 0x87, 0x6d, 0x8e, 0xa2, 0x30,
 	 0x94, 0xea, 0x53, 0x09}
 };
 
-static uint8_t tdes_cbc_reference_ciphertext[][TDES_CBC_MAX_DATA_LEN] = {
+static uint8_t
+tdes_cbc_reference_ciphertext[][TDES_CBC_MAX_DATA_LEN] ODP_UNUSED = {
 	{0xb2, 0x2b, 0x8d, 0x66, 0xde, 0x97, 0x06, 0x92},
 
 	{0x7b, 0x1f, 0x7c, 0x7e, 0x3b, 0x1c, 0x94, 0x8e, 0xbd, 0x04, 0xa7, 0x5f,
@@ -52,7 +54,7 @@  static uint8_t tdes_cbc_reference_ciphertext[][TDES_CBC_MAX_DATA_LEN] = {
 #define HMAC_MD5_DIGEST_LEN	16
 #define HMAC_MD5_96_CHECK_LEN	12
 
-static uint8_t hmac_md5_reference_key[][HMAC_MD5_KEY_LEN] = {
+static uint8_t hmac_md5_reference_key[][HMAC_MD5_KEY_LEN] ODP_UNUSED = {
 	{ 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b,
 	  0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b } ,
 
@@ -63,9 +65,10 @@  static uint8_t hmac_md5_reference_key[][HMAC_MD5_KEY_LEN] = {
 	  0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa }
 };
 
-static uint32_t hmac_md5_reference_length[] = { 8, 28, 50 };
+static uint32_t hmac_md5_reference_length[] ODP_UNUSED = { 8, 28, 50 };
 
-static uint8_t hmac_md5_reference_plaintext[][HMAC_MD5_MAX_DATA_LEN] = {
+static uint8_t
+hmac_md5_reference_plaintext[][HMAC_MD5_MAX_DATA_LEN] ODP_UNUSED = {
 	/* "Hi There" */
 	{ 0x48, 0x69, 0x20, 0x54, 0x68, 0x65, 0x72, 0x65},
 
@@ -82,7 +85,7 @@  static uint8_t hmac_md5_reference_plaintext[][HMAC_MD5_MAX_DATA_LEN] = {
 	  0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd }
 };
 
-static uint8_t hmac_md5_reference_digest[][HMAC_MD5_DIGEST_LEN] = {
+static uint8_t hmac_md5_reference_digest[][HMAC_MD5_DIGEST_LEN] ODP_UNUSED = {
 	{ 0x92, 0x94, 0x72, 0x7a, 0x36, 0x38, 0xbb, 0x1c,
 	  0x13, 0xf4, 0x8e, 0xf8, 0x15, 0x8b, 0xfc, 0x9d },
 
diff --git a/test/validation/odp_crypto.c b/test/validation/odp_crypto.c
index 985302a..6b53952 100644
--- a/test/validation/odp_crypto.c
+++ b/test/validation/odp_crypto.c
@@ -9,6 +9,7 @@ 
 #include "CUnit/TestDB.h"
 #include "odp_crypto_test_async_inp.h"
 #include "odp_crypto_test_sync_inp.h"
+#include "odp_crypto_test_rng.h"
 
 #define SHM_PKT_POOL_SIZE	(512*2048*2)
 #define SHM_PKT_POOL_BUF_SIZE	(1024 * 32)
@@ -19,6 +20,7 @@ 
 CU_SuiteInfo suites[] = {
 	{ ODP_CRYPTO_SYNC_INP , NULL, NULL, NULL, NULL, test_array_sync },
 	{ ODP_CRYPTO_ASYNC_INP , NULL, NULL, NULL, NULL, test_array_async },
+	{ ODP_CRYPTO_RNG, NULL, NULL, NULL, NULL, test_rng },
 	CU_SUITE_INFO_NULL,
 };