[PATCHv3,03/18] api: odp_crypto.h: odp_hw_random_get(): return number of bytes written

Message ID 1422891740-5347-4-git-send-email-ola.liljedahl@linaro.org
State New
Headers show

Commit Message

Ola Liljedahl Feb. 2, 2015, 3:42 p.m.
odp_hw_random_get() takes output buffer size as input and returns number
of bytes written (on success), <0 on failure.
Updated the implementation.
Updated all usages in example and test programs.

Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
---
(This document/code contribution attached is provided under the terms of
agreement LES-LTM-21309)

 example/ipsec/odp_ipsec_cache.c              |  6 ++++--
 include/odp/api/crypto.h                     | 17 ++++++++++-------
 platform/linux-generic/odp_crypto.c          |  8 ++++----
 test/validation/crypto/odp_crypto_test_rng.c |  6 ++----
 4 files changed, 20 insertions(+), 17 deletions(-)

Comments

Bill Fischofer Feb. 2, 2015, 6:06 p.m. | #1
Checkpatch flags this:

WARNING: Avoid CamelCase: <RAND_bytes>
#95: FILE: platform/linux-generic/odp_crypto.c:453:
+ rc = RAND_bytes(buf, len);

total: 0 errors, 1 warnings, 0 checks, 77 lines checked

On Mon, Feb 2, 2015 at 9:42 AM, Ola Liljedahl <ola.liljedahl@linaro.org>
wrote:

> odp_hw_random_get() takes output buffer size as input and returns number
> of bytes written (on success), <0 on failure.
> Updated the implementation.
> Updated all usages in example and test programs.
>
> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
> ---
> (This document/code contribution attached is provided under the terms of
> agreement LES-LTM-21309)
>
>  example/ipsec/odp_ipsec_cache.c              |  6 ++++--
>  include/odp/api/crypto.h                     | 17 ++++++++++-------
>  platform/linux-generic/odp_crypto.c          |  8 ++++----
>  test/validation/crypto/odp_crypto_test_rng.c |  6 ++----
>  4 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/example/ipsec/odp_ipsec_cache.c
> b/example/ipsec/odp_ipsec_cache.c
> index 904e7b6..ab05829 100644
> --- a/example/ipsec/odp_ipsec_cache.c
> +++ b/example/ipsec/odp_ipsec_cache.c
> @@ -96,9 +96,11 @@ int create_ipsec_cache_entry(sa_db_entry_t *cipher_sa,
>
>         /* Generate an IV */
>         if (params.iv.length) {
> -               size_t size = params.iv.length;
> +               ssize_t size = params.iv.length;
>
> -               odp_hw_random_get(params.iv.data, &size, 1);
> +               ssize_t ret = odp_hw_random_get(params.iv.data, size, 1);
> +               if (ret != size)
> +                       return -1;
>         }
>
>         /* Synchronous session create for now */
> diff --git a/include/odp/api/crypto.h b/include/odp/api/crypto.h
> index 545c1a5..e9530a3 100644
> --- a/include/odp/api/crypto.h
> +++ b/include/odp/api/crypto.h
> @@ -18,6 +18,8 @@
>  extern "C" {
>  #endif
>
> +#include <sys/types.h>
> +
>  /** @defgroup odp_crypto ODP CRYPTO
>   *  Macros, enums, types and operations to utilise crypto.
>   *  @{
> @@ -330,18 +332,19 @@ odp_crypto_compl_result(odp_crypto_compl_t
> completion_event,
>                         odp_crypto_op_result_t *result);
>
>  /**
> - * Generate random byte string
> + * Generate random byte data
>   *
> - * @param buf          Pointer to store result
> - * @param len          Pointer to input length value as well as return
> value
> - * @param use_entropy  Use entropy
> + * @param[out]    buf   Output buffer
> + * @param         bufsz Size of output buffer
> + * @param use_entropy   Use entropy
>   *
>   * @todo Define the implication of the use_entropy parameter
>   *
> - * @return 0 if succesful
> + * @return Number of bytes written
> + * @retval <0 on failure
>   */
> -int
> -odp_hw_random_get(uint8_t *buf, size_t *len, odp_bool_t use_entropy);
> +ssize_t
> +odp_hw_random_get(uint8_t *buf, ssize_t bufsz, bool use_entropy);
>
>  /**
>   * @}
> diff --git a/platform/linux-generic/odp_crypto.c
> b/platform/linux-generic/odp_crypto.c
> index 46766fa..b553306 100644
> --- a/platform/linux-generic/odp_crypto.c
> +++ b/platform/linux-generic/odp_crypto.c
> @@ -446,12 +446,12 @@ odp_crypto_init_global(void)
>         return 0;
>  }
>
> -int
> -odp_hw_random_get(uint8_t *buf, size_t *len, odp_bool_t use_entropy
> ODP_UNUSED)
> +ssize_t
> +odp_hw_random_get(uint8_t *buf, ssize_t len, bool use_entropy ODP_UNUSED)
>  {
>         int rc;
> -       rc = RAND_bytes(buf, *len);
> -       return ((1 == rc) ? 0 : -1);
> +       rc = RAND_bytes(buf, len);
> +       return (1 == rc) ? len /*success*/: -1 /*failure*/;
>  }
>
>  odp_crypto_compl_t odp_crypto_compl_from_event(odp_event_t ev)
> diff --git a/test/validation/crypto/odp_crypto_test_rng.c
> b/test/validation/crypto/odp_crypto_test_rng.c
> index 458f908..a9de5b9 100644
> --- a/test/validation/crypto/odp_crypto_test_rng.c
> +++ b/test/validation/crypto/odp_crypto_test_rng.c
> @@ -16,12 +16,10 @@
>  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);
> +       ret = odp_hw_random_get(buf, sizeof(buf), false);
> +       CU_ASSERT(ret == TDES_CBC_IV_LEN);
>  }
>
>  CU_TestInfo test_rng[] = {
> --
> 1.9.1
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Ola Liljedahl Feb. 2, 2015, 6:10 p.m. | #2
On 2 February 2015 at 19:06, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> Checkpatch flags this:
>
> WARNING: Avoid CamelCase: <RAND_bytes>
> #95: FILE: platform/linux-generic/odp_crypto.c:453:
> + rc = RAND_bytes(buf, len);
RAND_bytes() is an openssl function so not much we can do there. But
this is not a true CamelCase name, not even a dromedarCase name?

>
> total: 0 errors, 1 warnings, 0 checks, 77 lines checked
>
> On Mon, Feb 2, 2015 at 9:42 AM, Ola Liljedahl <ola.liljedahl@linaro.org>
> wrote:
>>
>> odp_hw_random_get() takes output buffer size as input and returns number
>> of bytes written (on success), <0 on failure.
>> Updated the implementation.
>> Updated all usages in example and test programs.
>>
>> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
>> ---
>> (This document/code contribution attached is provided under the terms of
>> agreement LES-LTM-21309)
>>
>>  example/ipsec/odp_ipsec_cache.c              |  6 ++++--
>>  include/odp/api/crypto.h                     | 17 ++++++++++-------
>>  platform/linux-generic/odp_crypto.c          |  8 ++++----
>>  test/validation/crypto/odp_crypto_test_rng.c |  6 ++----
>>  4 files changed, 20 insertions(+), 17 deletions(-)
>>
>> diff --git a/example/ipsec/odp_ipsec_cache.c
>> b/example/ipsec/odp_ipsec_cache.c
>> index 904e7b6..ab05829 100644
>> --- a/example/ipsec/odp_ipsec_cache.c
>> +++ b/example/ipsec/odp_ipsec_cache.c
>> @@ -96,9 +96,11 @@ int create_ipsec_cache_entry(sa_db_entry_t *cipher_sa,
>>
>>         /* Generate an IV */
>>         if (params.iv.length) {
>> -               size_t size = params.iv.length;
>> +               ssize_t size = params.iv.length;
>>
>> -               odp_hw_random_get(params.iv.data, &size, 1);
>> +               ssize_t ret = odp_hw_random_get(params.iv.data, size, 1);
>> +               if (ret != size)
>> +                       return -1;
>>         }
>>
>>         /* Synchronous session create for now */
>> diff --git a/include/odp/api/crypto.h b/include/odp/api/crypto.h
>> index 545c1a5..e9530a3 100644
>> --- a/include/odp/api/crypto.h
>> +++ b/include/odp/api/crypto.h
>> @@ -18,6 +18,8 @@
>>  extern "C" {
>>  #endif
>>
>> +#include <sys/types.h>
>> +
>>  /** @defgroup odp_crypto ODP CRYPTO
>>   *  Macros, enums, types and operations to utilise crypto.
>>   *  @{
>> @@ -330,18 +332,19 @@ odp_crypto_compl_result(odp_crypto_compl_t
>> completion_event,
>>                         odp_crypto_op_result_t *result);
>>
>>  /**
>> - * Generate random byte string
>> + * Generate random byte data
>>   *
>> - * @param buf          Pointer to store result
>> - * @param len          Pointer to input length value as well as return
>> value
>> - * @param use_entropy  Use entropy
>> + * @param[out]    buf   Output buffer
>> + * @param         bufsz Size of output buffer
>> + * @param use_entropy   Use entropy
>>   *
>>   * @todo Define the implication of the use_entropy parameter
>>   *
>> - * @return 0 if succesful
>> + * @return Number of bytes written
>> + * @retval <0 on failure
>>   */
>> -int
>> -odp_hw_random_get(uint8_t *buf, size_t *len, odp_bool_t use_entropy);
>> +ssize_t
>> +odp_hw_random_get(uint8_t *buf, ssize_t bufsz, bool use_entropy);
>>
>>  /**
>>   * @}
>> diff --git a/platform/linux-generic/odp_crypto.c
>> b/platform/linux-generic/odp_crypto.c
>> index 46766fa..b553306 100644
>> --- a/platform/linux-generic/odp_crypto.c
>> +++ b/platform/linux-generic/odp_crypto.c
>> @@ -446,12 +446,12 @@ odp_crypto_init_global(void)
>>         return 0;
>>  }
>>
>> -int
>> -odp_hw_random_get(uint8_t *buf, size_t *len, odp_bool_t use_entropy
>> ODP_UNUSED)
>> +ssize_t
>> +odp_hw_random_get(uint8_t *buf, ssize_t len, bool use_entropy ODP_UNUSED)
>>  {
>>         int rc;
>> -       rc = RAND_bytes(buf, *len);
>> -       return ((1 == rc) ? 0 : -1);
>> +       rc = RAND_bytes(buf, len);
>> +       return (1 == rc) ? len /*success*/: -1 /*failure*/;
>>  }
>>
>>  odp_crypto_compl_t odp_crypto_compl_from_event(odp_event_t ev)
>> diff --git a/test/validation/crypto/odp_crypto_test_rng.c
>> b/test/validation/crypto/odp_crypto_test_rng.c
>> index 458f908..a9de5b9 100644
>> --- a/test/validation/crypto/odp_crypto_test_rng.c
>> +++ b/test/validation/crypto/odp_crypto_test_rng.c
>> @@ -16,12 +16,10 @@
>>  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);
>> +       ret = odp_hw_random_get(buf, sizeof(buf), false);
>> +       CU_ASSERT(ret == TDES_CBC_IV_LEN);
>>  }
>>
>>  CU_TestInfo test_rng[] = {
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
Bill Fischofer Feb. 2, 2015, 6:14 p.m. | #3
Obviously we can't change OpenSSL, so this should be an exception.  And
yes, I would not consider that name camel case.  If we were using camel
case we'd write odpBufferAlloc() rather than odp_buffer_alloc().

On Mon, Feb 2, 2015 at 12:10 PM, Ola Liljedahl <ola.liljedahl@linaro.org>
wrote:

> On 2 February 2015 at 19:06, Bill Fischofer <bill.fischofer@linaro.org>
> wrote:
> > Checkpatch flags this:
> >
> > WARNING: Avoid CamelCase: <RAND_bytes>
> > #95: FILE: platform/linux-generic/odp_crypto.c:453:
> > + rc = RAND_bytes(buf, len);
> RAND_bytes() is an openssl function so not much we can do there. But
> this is not a true CamelCase name, not even a dromedarCase name?
>
> >
> > total: 0 errors, 1 warnings, 0 checks, 77 lines checked
> >
> > On Mon, Feb 2, 2015 at 9:42 AM, Ola Liljedahl <ola.liljedahl@linaro.org>
> > wrote:
> >>
> >> odp_hw_random_get() takes output buffer size as input and returns number
> >> of bytes written (on success), <0 on failure.
> >> Updated the implementation.
> >> Updated all usages in example and test programs.
> >>
> >> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
> >> ---
> >> (This document/code contribution attached is provided under the terms of
> >> agreement LES-LTM-21309)
> >>
> >>  example/ipsec/odp_ipsec_cache.c              |  6 ++++--
> >>  include/odp/api/crypto.h                     | 17 ++++++++++-------
> >>  platform/linux-generic/odp_crypto.c          |  8 ++++----
> >>  test/validation/crypto/odp_crypto_test_rng.c |  6 ++----
> >>  4 files changed, 20 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/example/ipsec/odp_ipsec_cache.c
> >> b/example/ipsec/odp_ipsec_cache.c
> >> index 904e7b6..ab05829 100644
> >> --- a/example/ipsec/odp_ipsec_cache.c
> >> +++ b/example/ipsec/odp_ipsec_cache.c
> >> @@ -96,9 +96,11 @@ int create_ipsec_cache_entry(sa_db_entry_t
> *cipher_sa,
> >>
> >>         /* Generate an IV */
> >>         if (params.iv.length) {
> >> -               size_t size = params.iv.length;
> >> +               ssize_t size = params.iv.length;
> >>
> >> -               odp_hw_random_get(params.iv.data, &size, 1);
> >> +               ssize_t ret = odp_hw_random_get(params.iv.data, size,
> 1);
> >> +               if (ret != size)
> >> +                       return -1;
> >>         }
> >>
> >>         /* Synchronous session create for now */
> >> diff --git a/include/odp/api/crypto.h b/include/odp/api/crypto.h
> >> index 545c1a5..e9530a3 100644
> >> --- a/include/odp/api/crypto.h
> >> +++ b/include/odp/api/crypto.h
> >> @@ -18,6 +18,8 @@
> >>  extern "C" {
> >>  #endif
> >>
> >> +#include <sys/types.h>
> >> +
> >>  /** @defgroup odp_crypto ODP CRYPTO
> >>   *  Macros, enums, types and operations to utilise crypto.
> >>   *  @{
> >> @@ -330,18 +332,19 @@ odp_crypto_compl_result(odp_crypto_compl_t
> >> completion_event,
> >>                         odp_crypto_op_result_t *result);
> >>
> >>  /**
> >> - * Generate random byte string
> >> + * Generate random byte data
> >>   *
> >> - * @param buf          Pointer to store result
> >> - * @param len          Pointer to input length value as well as return
> >> value
> >> - * @param use_entropy  Use entropy
> >> + * @param[out]    buf   Output buffer
> >> + * @param         bufsz Size of output buffer
> >> + * @param use_entropy   Use entropy
> >>   *
> >>   * @todo Define the implication of the use_entropy parameter
> >>   *
> >> - * @return 0 if succesful
> >> + * @return Number of bytes written
> >> + * @retval <0 on failure
> >>   */
> >> -int
> >> -odp_hw_random_get(uint8_t *buf, size_t *len, odp_bool_t use_entropy);
> >> +ssize_t
> >> +odp_hw_random_get(uint8_t *buf, ssize_t bufsz, bool use_entropy);
> >>
> >>  /**
> >>   * @}
> >> diff --git a/platform/linux-generic/odp_crypto.c
> >> b/platform/linux-generic/odp_crypto.c
> >> index 46766fa..b553306 100644
> >> --- a/platform/linux-generic/odp_crypto.c
> >> +++ b/platform/linux-generic/odp_crypto.c
> >> @@ -446,12 +446,12 @@ odp_crypto_init_global(void)
> >>         return 0;
> >>  }
> >>
> >> -int
> >> -odp_hw_random_get(uint8_t *buf, size_t *len, odp_bool_t use_entropy
> >> ODP_UNUSED)
> >> +ssize_t
> >> +odp_hw_random_get(uint8_t *buf, ssize_t len, bool use_entropy
> ODP_UNUSED)
> >>  {
> >>         int rc;
> >> -       rc = RAND_bytes(buf, *len);
> >> -       return ((1 == rc) ? 0 : -1);
> >> +       rc = RAND_bytes(buf, len);
> >> +       return (1 == rc) ? len /*success*/: -1 /*failure*/;
> >>  }
> >>
> >>  odp_crypto_compl_t odp_crypto_compl_from_event(odp_event_t ev)
> >> diff --git a/test/validation/crypto/odp_crypto_test_rng.c
> >> b/test/validation/crypto/odp_crypto_test_rng.c
> >> index 458f908..a9de5b9 100644
> >> --- a/test/validation/crypto/odp_crypto_test_rng.c
> >> +++ b/test/validation/crypto/odp_crypto_test_rng.c
> >> @@ -16,12 +16,10 @@
> >>  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);
> >> +       ret = odp_hw_random_get(buf, sizeof(buf), false);
> >> +       CU_ASSERT(ret == TDES_CBC_IV_LEN);
> >>  }
> >>
> >>  CU_TestInfo test_rng[] = {
> >> --
> >> 1.9.1
> >>
> >>
> >> _______________________________________________
> >> lng-odp mailing list
> >> lng-odp@lists.linaro.org
> >> http://lists.linaro.org/mailman/listinfo/lng-odp
> >
> >
>

Patch

diff --git a/example/ipsec/odp_ipsec_cache.c b/example/ipsec/odp_ipsec_cache.c
index 904e7b6..ab05829 100644
--- a/example/ipsec/odp_ipsec_cache.c
+++ b/example/ipsec/odp_ipsec_cache.c
@@ -96,9 +96,11 @@  int create_ipsec_cache_entry(sa_db_entry_t *cipher_sa,
 
 	/* Generate an IV */
 	if (params.iv.length) {
-		size_t size = params.iv.length;
+		ssize_t size = params.iv.length;
 
-		odp_hw_random_get(params.iv.data, &size, 1);
+		ssize_t ret = odp_hw_random_get(params.iv.data, size, 1);
+		if (ret != size)
+			return -1;
 	}
 
 	/* Synchronous session create for now */
diff --git a/include/odp/api/crypto.h b/include/odp/api/crypto.h
index 545c1a5..e9530a3 100644
--- a/include/odp/api/crypto.h
+++ b/include/odp/api/crypto.h
@@ -18,6 +18,8 @@ 
 extern "C" {
 #endif
 
+#include <sys/types.h>
+
 /** @defgroup odp_crypto ODP CRYPTO
  *  Macros, enums, types and operations to utilise crypto.
  *  @{
@@ -330,18 +332,19 @@  odp_crypto_compl_result(odp_crypto_compl_t completion_event,
 			odp_crypto_op_result_t *result);
 
 /**
- * Generate random byte string
+ * Generate random byte data
  *
- * @param buf          Pointer to store result
- * @param len          Pointer to input length value as well as return value
- * @param use_entropy  Use entropy
+ * @param[out]    buf   Output buffer
+ * @param         bufsz Size of output buffer
+ * @param use_entropy   Use entropy
  *
  * @todo Define the implication of the use_entropy parameter
  *
- * @return 0 if succesful
+ * @return Number of bytes written
+ * @retval <0 on failure
  */
-int
-odp_hw_random_get(uint8_t *buf, size_t *len, odp_bool_t use_entropy);
+ssize_t
+odp_hw_random_get(uint8_t *buf, ssize_t bufsz, bool use_entropy);
 
 /**
  * @}
diff --git a/platform/linux-generic/odp_crypto.c b/platform/linux-generic/odp_crypto.c
index 46766fa..b553306 100644
--- a/platform/linux-generic/odp_crypto.c
+++ b/platform/linux-generic/odp_crypto.c
@@ -446,12 +446,12 @@  odp_crypto_init_global(void)
 	return 0;
 }
 
-int
-odp_hw_random_get(uint8_t *buf, size_t *len, odp_bool_t use_entropy ODP_UNUSED)
+ssize_t
+odp_hw_random_get(uint8_t *buf, ssize_t len, bool use_entropy ODP_UNUSED)
 {
 	int rc;
-	rc = RAND_bytes(buf, *len);
-	return ((1 == rc) ? 0 : -1);
+	rc = RAND_bytes(buf, len);
+	return (1 == rc) ? len /*success*/: -1 /*failure*/;
 }
 
 odp_crypto_compl_t odp_crypto_compl_from_event(odp_event_t ev)
diff --git a/test/validation/crypto/odp_crypto_test_rng.c b/test/validation/crypto/odp_crypto_test_rng.c
index 458f908..a9de5b9 100644
--- a/test/validation/crypto/odp_crypto_test_rng.c
+++ b/test/validation/crypto/odp_crypto_test_rng.c
@@ -16,12 +16,10 @@ 
 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);
+	ret = odp_hw_random_get(buf, sizeof(buf), false);
+	CU_ASSERT(ret == TDES_CBC_IV_LEN);
 }
 
 CU_TestInfo test_rng[] = {