From b3c10b348bd99548c3263811508d3f41a74d7a7c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Dec 2018 14:52:01 +0000 Subject: [PATCH 1/7] Add documentation on parameter preconditions to ChaChaPoly modules --- include/mbedtls/chacha20.h | 22 ++++++++------- include/mbedtls/chachapoly.h | 52 +++++++++++++++++------------------- include/mbedtls/poly1305.h | 34 ++++++++++++----------- 3 files changed, 55 insertions(+), 53 deletions(-) diff --git a/include/mbedtls/chacha20.h b/include/mbedtls/chacha20.h index 529f22d9c..14436d68c 100644 --- a/include/mbedtls/chacha20.h +++ b/include/mbedtls/chacha20.h @@ -82,14 +82,15 @@ mbedtls_chacha20_context; * to \c mbedtls_chacha20_update(), and finally to * \c mbedtls_chacha20_free(). * - * \param ctx The ChaCha20 context to initialize. + * \param ctx The ChaCha20 context to initialize. Must not be \c NULL. */ void mbedtls_chacha20_init( mbedtls_chacha20_context *ctx ); /** * \brief This function releases and clears the specified ChaCha20 context. * - * \param ctx The ChaCha20 context to clear. + * \param ctx The ChaCha20 context to clear. May be \c NULL, + * in which case this function is a no-op. */ void mbedtls_chacha20_free( mbedtls_chacha20_context *ctx ); @@ -102,6 +103,7 @@ void mbedtls_chacha20_free( mbedtls_chacha20_context *ctx ); * \c mbedtls_chacha_update(). * * \param ctx The ChaCha20 context to which the key should be bound. + * Must be initialized. * \param key The encryption/decryption key. Must be 32 bytes in length. * * \return \c 0 on success. @@ -121,6 +123,7 @@ int mbedtls_chacha20_setkey( mbedtls_chacha20_context *ctx, * messages encrypted with the same nonce and key. * * \param ctx The ChaCha20 context to which the nonce should be bound. + * Must be initialized. * \param nonce The nonce. Must be 12 bytes in size. * \param counter The initial counter value. This is usually 0. * @@ -150,16 +153,16 @@ int mbedtls_chacha20_starts( mbedtls_chacha20_context* ctx, * key and nonce. * * \param ctx The ChaCha20 context to use for encryption or decryption. + * Must be initialized. * \param size The length of the input data in bytes. * \param input The buffer holding the input data. - * This pointer can be NULL if size == 0. + * This pointer can be \c NULL if `size == 0`. * \param output The buffer holding the output data. * Must be able to hold \p size bytes. - * This pointer can be NULL if size == 0. + * This pointer can be \c NULL if `size == 0`. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA if the ctx, input, or - * output pointers are NULL. + * \return A negative error code on failure. */ int mbedtls_chacha20_update( mbedtls_chacha20_context *ctx, size_t size, @@ -185,14 +188,13 @@ int mbedtls_chacha20_update( mbedtls_chacha20_context *ctx, * \param counter The initial counter value. This is usually 0. * \param size The length of the input data in bytes. * \param input The buffer holding the input data. - * This pointer can be NULL if size == 0. + * This pointer can be \c NULL if `size == 0`. * \param output The buffer holding the output data. * Must be able to hold \p size bytes. - * This pointer can be NULL if size == 0. + * This pointer can be \c NULL if `size == 0`. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA if key, nonce, input, - * or output is NULL. + * \return A negative error code on failure. */ int mbedtls_chacha20_crypt( const unsigned char key[32], const unsigned char nonce[12], diff --git a/include/mbedtls/chachapoly.h b/include/mbedtls/chachapoly.h index 7de6f4e8c..80d20e54a 100644 --- a/include/mbedtls/chachapoly.h +++ b/include/mbedtls/chachapoly.h @@ -115,27 +115,29 @@ mbedtls_chachapoly_context; * all previous outputs of \c mbedtls_chachapoly_update(), * otherwise you can now safely use the plaintext. * - * \param ctx The ChachaPoly context to initialize. + * \param ctx The ChachaPoly context to initialize. Must not be \c NULL. */ void mbedtls_chachapoly_init( mbedtls_chachapoly_context *ctx ); /** - * \brief This function releases and clears the specified ChaCha20-Poly1305 context. + * \brief This function releases and clears the specified + * ChaCha20-Poly1305 context. * - * \param ctx The ChachaPoly context to clear. + * \param ctx The ChachaPoly context to clear. May be \c NULL, in which + * case this function is a no-op. */ void mbedtls_chachapoly_free( mbedtls_chachapoly_context *ctx ); /** - * \brief This function sets the ChaCha20-Poly1305 symmetric encryption key. + * \brief This function sets the ChaCha20-Poly1305 + * symmetric encryption key. * * \param ctx The ChaCha20-Poly1305 context to which the key should be - * bound. + * bound. Must be initialized. * \param key The 256-bit (32 bytes) key. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA - * if \p ctx or \p key are NULL. + * \return A negative error code on failure. */ int mbedtls_chachapoly_setkey( mbedtls_chachapoly_context *ctx, const unsigned char key[32] ); @@ -155,14 +157,13 @@ int mbedtls_chachapoly_setkey( mbedtls_chachapoly_context *ctx, * \warning Decryption with the piecewise API is discouraged, see the * warning on \c mbedtls_chachapoly_init(). * - * \param ctx The ChaCha20-Poly1305 context. + * \param ctx The ChaCha20-Poly1305 context. Must be initialized. * \param nonce The nonce/IV to use for the message. Must be 12 bytes. * \param mode The operation to perform: #MBEDTLS_CHACHAPOLY_ENCRYPT or * #MBEDTLS_CHACHAPOLY_DECRYPT (discouraged, see warning). * * \return \c 0 on success. - * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA - * if \p ctx or \p mac are NULL. + * \return A negative error code on failure. */ int mbedtls_chachapoly_starts( mbedtls_chachapoly_context *ctx, const unsigned char nonce[12], @@ -227,20 +228,19 @@ int mbedtls_chachapoly_update_aad( mbedtls_chachapoly_context *ctx, * \warning Decryption with the piecewise API is discouraged, see the * warning on \c mbedtls_chachapoly_init(). * - * \param ctx The ChaCha20-Poly1305 context to use. + * \param ctx The ChaCha20-Poly1305 context to use. Must be initialized. * \param len The length (in bytes) of the data to encrypt or decrypt. * \param input The buffer containing the data to encrypt or decrypt. - * This pointer can be NULL if len == 0. - * \param output The buffer to where the encrypted or decrypted data is written. - * Must be able to hold \p len bytes. - * This pointer can be NULL if len == 0. + * This pointer can be \c NULL if `len == 0`. + * \param output The buffer to where the encrypted or decrypted data is + * written. Must be able to hold \p len bytes. + * This pointer can be \c NULL if `len == 0`. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA - * if \p ctx, \p input, or \p output are NULL. * \return #MBEDTLS_ERR_CHACHAPOLY_BAD_STATE * if the operation has not been started or has been * finished. + * \return Another negative error code on other kinds of failure. */ int mbedtls_chachapoly_update( mbedtls_chachapoly_context *ctx, size_t len, @@ -251,18 +251,17 @@ int mbedtls_chachapoly_update( mbedtls_chachapoly_context *ctx, * \brief This function finished the ChaCha20-Poly1305 operation and * generates the MAC (authentication tag). * - * \param ctx The ChaCha20-Poly1305 context to use. + * \param ctx The ChaCha20-Poly1305 context to use. Must be initialized. * \param mac The buffer to where the 128-bit (16 bytes) MAC is written. * * \warning Decryption with the piecewise API is discouraged, see the * warning on \c mbedtls_chachapoly_init(). * * \return \c 0 on success. - * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA - * if \p ctx or \p mac are NULL. * \return #MBEDTLS_ERR_CHACHAPOLY_BAD_STATE * if the operation has not been started or has been * finished. + * \return Another negative error code on other kinds of failure. */ int mbedtls_chachapoly_finish( mbedtls_chachapoly_context *ctx, unsigned char mac[16] ); @@ -280,20 +279,20 @@ int mbedtls_chachapoly_finish( mbedtls_chachapoly_context *ctx, * and key. * * \param ctx The ChaCha20-Poly1305 context to use (holds the key). + * Must be initialized. * \param length The length (in bytes) of the data to encrypt or decrypt. * \param nonce The 96-bit (12 bytes) nonce/IV to use. * \param aad The buffer containing the additional authenticated data (AAD). - * This pointer can be NULL if aad_len == 0. + * This pointer can be \c NULL if `aad_len == 0`. * \param aad_len The length (in bytes) of the AAD data to process. * \param input The buffer containing the data to encrypt or decrypt. - * This pointer can be NULL if ilen == 0. + * This pointer can be \c NULL if `ilen == 0`. * \param output The buffer to where the encrypted or decrypted data is written. - * This pointer can be NULL if ilen == 0. + * This pointer can be \c NULL if `ilen == 0`. * \param tag The buffer to where the computed 128-bit (16 bytes) MAC is written. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA - * if one or more of the required parameters are NULL. + * \return A negative error code on failure. */ int mbedtls_chachapoly_encrypt_and_tag( mbedtls_chachapoly_context *ctx, size_t length, @@ -324,10 +323,9 @@ int mbedtls_chachapoly_encrypt_and_tag( mbedtls_chachapoly_context *ctx, * This pointer can be NULL if ilen == 0. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA - * if one or more of the required parameters are NULL. * \return #MBEDTLS_ERR_CHACHAPOLY_AUTH_FAILED * if the data was not authentic. + * \return Another negative error code on other kinds of failure. */ int mbedtls_chachapoly_auth_decrypt( mbedtls_chachapoly_context *ctx, size_t length, diff --git a/include/mbedtls/poly1305.h b/include/mbedtls/poly1305.h index b02f968b5..617958e11 100644 --- a/include/mbedtls/poly1305.h +++ b/include/mbedtls/poly1305.h @@ -84,14 +84,15 @@ mbedtls_poly1305_context; * \c mbedtls_poly1305_finish(), then finally * \c mbedtls_poly1305_free(). * - * \param ctx The Poly1305 context to initialize. + * \param ctx The Poly1305 context to initialize. Must not be \c NULL. */ void mbedtls_poly1305_init( mbedtls_poly1305_context *ctx ); /** * \brief This function releases and clears the specified Poly1305 context. * - * \param ctx The Poly1305 context to clear. + * \param ctx The Poly1305 context to clear. May be \c NULL, in which + * case this function is a no-op. */ void mbedtls_poly1305_free( mbedtls_poly1305_context *ctx ); @@ -102,11 +103,11 @@ void mbedtls_poly1305_free( mbedtls_poly1305_context *ctx ); * invocation of Poly1305. * * \param ctx The Poly1305 context to which the key should be bound. - * \param key The buffer containing the 256-bit key. + * Must be initialized. + * \param key The buffer containing the 32-byte (256-bit) key. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA - * if ctx or key are NULL. + * \return A negative error code on failure. */ int mbedtls_poly1305_starts( mbedtls_poly1305_context *ctx, const unsigned char key[32] ); @@ -120,13 +121,14 @@ int mbedtls_poly1305_starts( mbedtls_poly1305_context *ctx, * It can be called repeatedly to process a stream of data. * * \param ctx The Poly1305 context to use for the Poly1305 operation. - * \param ilen The length of the input data (in bytes). Any value is accepted. + * Must be initialized. + * \param ilen The length of the input data (in bytes). + * Any value is accepted. * \param input The buffer holding the input data. - * This pointer can be NULL if ilen == 0. + * This pointer can be \c NULL if `ilen == 0`. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA - * if ctx or input are NULL. + * \return A negative error code on failure. */ int mbedtls_poly1305_update( mbedtls_poly1305_context *ctx, const unsigned char *input, @@ -137,12 +139,12 @@ int mbedtls_poly1305_update( mbedtls_poly1305_context *ctx, * Authentication Code (MAC). * * \param ctx The Poly1305 context to use for the Poly1305 operation. + * Must be initialized. * \param mac The buffer to where the MAC is written. Must be big enough * to hold the 16-byte MAC. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA - * if ctx or mac are NULL. + * \return A negative error code on failure. */ int mbedtls_poly1305_finish( mbedtls_poly1305_context *ctx, unsigned char mac[16] ); @@ -154,16 +156,16 @@ int mbedtls_poly1305_finish( mbedtls_poly1305_context *ctx, * \warning The key must be unique and unpredictable for each * invocation of Poly1305. * - * \param key The buffer containing the 256-bit key. - * \param ilen The length of the input data (in bytes). Any value is accepted. + * \param key The buffer containing the 32-byte (256-bit) key. + * \param ilen The length of the input data (in bytes). + * Any value is accepted. * \param input The buffer holding the input data. - * This pointer can be NULL if ilen == 0. + * This pointer can be \c NULL if `ilen == 0`. * \param mac The buffer to where the MAC is written. Must be big enough * to hold the 16-byte MAC. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA - * if key, input, or mac are NULL. + * \return A negative error code on failure. */ int mbedtls_poly1305_mac( const unsigned char key[32], const unsigned char *input, From 305e4e4f329128d029544d9cdb71a67f2259f030 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Dec 2018 15:03:16 +0000 Subject: [PATCH 2/7] Implement parameter validation for ChaCha/Poly modules --- library/chacha20.c | 48 +++++++++---------- library/chachapoly.c | 107 +++++++++++++++++++++---------------------- library/poly1305.c | 46 +++++++++---------- 3 files changed, 96 insertions(+), 105 deletions(-) diff --git a/library/chacha20.c b/library/chacha20.c index d14a51e04..0757163e2 100644 --- a/library/chacha20.c +++ b/library/chacha20.c @@ -53,6 +53,12 @@ #define inline __inline #endif +/* Parameter validation macros */ +#define CHACHA20_VALIDATE_RET( cond ) \ + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ) +#define CHACHA20_VALIDATE( cond ) \ + MBEDTLS_INTERNAL_VALIDATE( cond ) + #define BYTES_TO_U32_LE( data, offset ) \ ( (uint32_t) data[offset] \ | (uint32_t) ( (uint32_t) data[( offset ) + 1] << 8 ) \ @@ -181,14 +187,13 @@ static void chacha20_block( const uint32_t initial_state[16], void mbedtls_chacha20_init( mbedtls_chacha20_context *ctx ) { - if( ctx != NULL ) - { - mbedtls_platform_zeroize( ctx->state, sizeof( ctx->state ) ); - mbedtls_platform_zeroize( ctx->keystream8, sizeof( ctx->keystream8 ) ); + CHACHA20_VALIDATE( ctx != NULL ); - /* Initially, there's no keystream bytes available */ - ctx->keystream_bytes_used = CHACHA20_BLOCK_SIZE_BYTES; - } + mbedtls_platform_zeroize( ctx->state, sizeof( ctx->state ) ); + mbedtls_platform_zeroize( ctx->keystream8, sizeof( ctx->keystream8 ) ); + + /* Initially, there's no keystream bytes available */ + ctx->keystream_bytes_used = CHACHA20_BLOCK_SIZE_BYTES; } void mbedtls_chacha20_free( mbedtls_chacha20_context *ctx ) @@ -202,10 +207,8 @@ void mbedtls_chacha20_free( mbedtls_chacha20_context *ctx ) int mbedtls_chacha20_setkey( mbedtls_chacha20_context *ctx, const unsigned char key[32] ) { - if( ( ctx == NULL ) || ( key == NULL ) ) - { - return( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - } + CHACHA20_VALIDATE_RET( ctx != NULL ); + CHACHA20_VALIDATE_RET( key != NULL ); /* ChaCha20 constants - the string "expand 32-byte k" */ ctx->state[0] = 0x61707865; @@ -230,10 +233,8 @@ int mbedtls_chacha20_starts( mbedtls_chacha20_context* ctx, const unsigned char nonce[12], uint32_t counter ) { - if( ( ctx == NULL ) || ( nonce == NULL ) ) - { - return( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - } + CHACHA20_VALIDATE_RET( ctx != NULL ); + CHACHA20_VALIDATE_RET( nonce != NULL ); /* Counter */ ctx->state[12] = counter; @@ -259,15 +260,9 @@ int mbedtls_chacha20_update( mbedtls_chacha20_context *ctx, size_t offset = 0U; size_t i; - if( ctx == NULL ) - { - return( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - } - else if( ( size > 0U ) && ( ( input == NULL ) || ( output == NULL ) ) ) - { - /* input and output pointers are allowed to be NULL only if size == 0 */ - return( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - } + CHACHA20_VALIDATE_RET( ctx != NULL ); + CHACHA20_VALIDATE_RET( size == 0 || input != NULL ); + CHACHA20_VALIDATE_RET( size == 0 || output != NULL ); /* Use leftover keystream bytes, if available */ while( size > 0U && ctx->keystream_bytes_used < CHACHA20_BLOCK_SIZE_BYTES ) @@ -332,6 +327,11 @@ int mbedtls_chacha20_crypt( const unsigned char key[32], mbedtls_chacha20_context ctx; int ret; + CHACHA20_VALIDATE_RET( key != NULL ); + CHACHA20_VALIDATE_RET( nonce != NULL ); + CHACHA20_VALIDATE_RET( data_len == 0 || input != NULL ); + CHACHA20_VALIDATE_RET( data_len == 0 || output != NULL ); + mbedtls_chacha20_init( &ctx ); ret = mbedtls_chacha20_setkey( &ctx, key ); diff --git a/library/chachapoly.c b/library/chachapoly.c index 860f87765..ee5cc345e 100644 --- a/library/chachapoly.c +++ b/library/chachapoly.c @@ -44,6 +44,12 @@ #if !defined(MBEDTLS_CHACHAPOLY_ALT) +/* Parameter validation macros */ +#define CHACHAPOLY_VALIDATE_RET( cond ) \ + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ) +#define CHACHAPOLY_VALIDATE( cond ) \ + MBEDTLS_INTERNAL_VALIDATE( cond ) + #define CHACHAPOLY_STATE_INIT ( 0 ) #define CHACHAPOLY_STATE_AAD ( 1 ) #define CHACHAPOLY_STATE_CIPHERTEXT ( 2 ) /* Encrypting or decrypting */ @@ -90,39 +96,35 @@ static int chachapoly_pad_ciphertext( mbedtls_chachapoly_context *ctx ) void mbedtls_chachapoly_init( mbedtls_chachapoly_context *ctx ) { - if( ctx != NULL ) - { - mbedtls_chacha20_init( &ctx->chacha20_ctx ); - mbedtls_poly1305_init( &ctx->poly1305_ctx ); - ctx->aad_len = 0U; - ctx->ciphertext_len = 0U; - ctx->state = CHACHAPOLY_STATE_INIT; - ctx->mode = MBEDTLS_CHACHAPOLY_ENCRYPT; - } + CHACHAPOLY_VALIDATE( ctx != NULL ); + + mbedtls_chacha20_init( &ctx->chacha20_ctx ); + mbedtls_poly1305_init( &ctx->poly1305_ctx ); + ctx->aad_len = 0U; + ctx->ciphertext_len = 0U; + ctx->state = CHACHAPOLY_STATE_INIT; + ctx->mode = MBEDTLS_CHACHAPOLY_ENCRYPT; } void mbedtls_chachapoly_free( mbedtls_chachapoly_context *ctx ) { if( ctx != NULL ) - { - mbedtls_chacha20_free( &ctx->chacha20_ctx ); - mbedtls_poly1305_free( &ctx->poly1305_ctx ); - ctx->aad_len = 0U; - ctx->ciphertext_len = 0U; - ctx->state = CHACHAPOLY_STATE_INIT; - ctx->mode = MBEDTLS_CHACHAPOLY_ENCRYPT; - } + return; + + mbedtls_chacha20_free( &ctx->chacha20_ctx ); + mbedtls_poly1305_free( &ctx->poly1305_ctx ); + ctx->aad_len = 0U; + ctx->ciphertext_len = 0U; + ctx->state = CHACHAPOLY_STATE_INIT; + ctx->mode = MBEDTLS_CHACHAPOLY_ENCRYPT; } int mbedtls_chachapoly_setkey( mbedtls_chachapoly_context *ctx, const unsigned char key[32] ) { int ret; - - if( ( ctx == NULL ) || ( key == NULL ) ) - { - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } + CHACHAPOLY_VALIDATE_RET( ctx != NULL ); + CHACHAPOLY_VALIDATE_RET( key != NULL ); ret = mbedtls_chacha20_setkey( &ctx->chacha20_ctx, key ); @@ -135,11 +137,8 @@ int mbedtls_chachapoly_starts( mbedtls_chachapoly_context *ctx, { int ret; unsigned char poly1305_key[64]; - - if( ( ctx == NULL ) || ( nonce == NULL ) ) - { - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } + CHACHAPOLY_VALIDATE_RET( ctx != NULL ); + CHACHAPOLY_VALIDATE_RET( nonce != NULL ); /* Set counter = 0, will be update to 1 when generating Poly1305 key */ ret = mbedtls_chacha20_starts( &ctx->chacha20_ctx, nonce, 0U ); @@ -176,16 +175,10 @@ int mbedtls_chachapoly_update_aad( mbedtls_chachapoly_context *ctx, const unsigned char *aad, size_t aad_len ) { - if( ctx == NULL ) - { - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } - else if( ( aad_len > 0U ) && ( aad == NULL ) ) - { - /* aad pointer is allowed to be NULL if aad_len == 0 */ - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } - else if( ctx->state != CHACHAPOLY_STATE_AAD ) + CHACHAPOLY_VALIDATE_RET( ctx != NULL ); + CHACHAPOLY_VALIDATE_RET( aad_len == 0 || aad != NULL ); + + if( ctx->state != CHACHAPOLY_STATE_AAD ) { return( MBEDTLS_ERR_CHACHAPOLY_BAD_STATE ); } @@ -201,18 +194,12 @@ int mbedtls_chachapoly_update( mbedtls_chachapoly_context *ctx, unsigned char *output ) { int ret; + CHACHAPOLY_VALIDATE_RET( ctx != NULL ); + CHACHAPOLY_VALIDATE_RET( len == 0 || input != NULL ); + CHACHAPOLY_VALIDATE_RET( len == 0 || output != NULL ); - if( ctx == NULL ) - { - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } - else if( ( len > 0U ) && ( ( input == NULL ) || ( output == NULL ) ) ) - { - /* input and output pointers are allowed to be NULL if len == 0 */ - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } - else if( ( ctx->state != CHACHAPOLY_STATE_AAD ) && - ( ctx->state != CHACHAPOLY_STATE_CIPHERTEXT ) ) + if( ( ctx->state != CHACHAPOLY_STATE_AAD ) && + ( ctx->state != CHACHAPOLY_STATE_CIPHERTEXT ) ) { return( MBEDTLS_ERR_CHACHAPOLY_BAD_STATE ); } @@ -257,12 +244,10 @@ int mbedtls_chachapoly_finish( mbedtls_chachapoly_context *ctx, { int ret; unsigned char len_block[16]; + CHACHAPOLY_VALIDATE_RET( ctx != NULL ); + CHACHAPOLY_VALIDATE_RET( mac != NULL ); - if( ( ctx == NULL ) || ( mac == NULL ) ) - { - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } - else if( ctx->state == CHACHAPOLY_STATE_INIT ) + if( ctx->state == CHACHAPOLY_STATE_INIT ) { return( MBEDTLS_ERR_CHACHAPOLY_BAD_STATE ); } @@ -350,6 +335,13 @@ int mbedtls_chachapoly_encrypt_and_tag( mbedtls_chachapoly_context *ctx, unsigned char *output, unsigned char tag[16] ) { + CHACHAPOLY_VALIDATE_RET( ctx != NULL ); + CHACHAPOLY_VALIDATE_RET( nonce != NULL ); + CHACHAPOLY_VALIDATE_RET( tag != NULL ); + CHACHAPOLY_VALIDATE_RET( aad_len == 0 || aad != NULL ); + CHACHAPOLY_VALIDATE_RET( length == 0 || input != NULL ); + CHACHAPOLY_VALIDATE_RET( length == 0 || output != NULL ); + return( chachapoly_crypt_and_tag( ctx, MBEDTLS_CHACHAPOLY_ENCRYPT, length, nonce, aad, aad_len, input, output, tag ) ); @@ -368,9 +360,12 @@ int mbedtls_chachapoly_auth_decrypt( mbedtls_chachapoly_context *ctx, unsigned char check_tag[16]; size_t i; int diff; - - if( tag == NULL ) - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); + CHACHAPOLY_VALIDATE_RET( ctx != NULL ); + CHACHAPOLY_VALIDATE_RET( nonce != NULL ); + CHACHAPOLY_VALIDATE_RET( tag != NULL ); + CHACHAPOLY_VALIDATE_RET( aad_len == 0 || aad != NULL ); + CHACHAPOLY_VALIDATE_RET( length == 0 || input != NULL ); + CHACHAPOLY_VALIDATE_RET( length == 0 || output != NULL ); if( ( ret = chachapoly_crypt_and_tag( ctx, MBEDTLS_CHACHAPOLY_DECRYPT, length, nonce, diff --git a/library/poly1305.c b/library/poly1305.c index e22d3afb6..c22a0a1ba 100644 --- a/library/poly1305.c +++ b/library/poly1305.c @@ -49,6 +49,12 @@ #define inline __inline #endif +/* Parameter validation macros */ +#define POLY1305_VALIDATE_RET( cond ) \ + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ) +#define POLY1305_VALIDATE( cond ) \ + MBEDTLS_INTERNAL_VALIDATE( cond ) + #define POLY1305_BLOCK_SIZE_BYTES ( 16U ) #define BYTES_TO_U32_LE( data, offset ) \ @@ -276,27 +282,24 @@ static void poly1305_compute_mac( const mbedtls_poly1305_context *ctx, void mbedtls_poly1305_init( mbedtls_poly1305_context *ctx ) { - if( ctx != NULL ) - { - mbedtls_platform_zeroize( ctx, sizeof( mbedtls_poly1305_context ) ); - } + POLY1305_VALIDATE( ctx != NULL ); + + mbedtls_platform_zeroize( ctx, sizeof( mbedtls_poly1305_context ) ); } void mbedtls_poly1305_free( mbedtls_poly1305_context *ctx ) { if( ctx != NULL ) - { - mbedtls_platform_zeroize( ctx, sizeof( mbedtls_poly1305_context ) ); - } + return; + + mbedtls_platform_zeroize( ctx, sizeof( mbedtls_poly1305_context ) ); } int mbedtls_poly1305_starts( mbedtls_poly1305_context *ctx, const unsigned char key[32] ) { - if( ctx == NULL || key == NULL ) - { - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } + POLY1305_VALIDATE_RET( ctx != NULL ); + POLY1305_VALIDATE_RET( key != NULL ); /* r &= 0x0ffffffc0ffffffc0ffffffc0fffffff */ ctx->r[0] = BYTES_TO_U32_LE( key, 0 ) & 0x0FFFFFFFU; @@ -331,16 +334,8 @@ int mbedtls_poly1305_update( mbedtls_poly1305_context *ctx, size_t remaining = ilen; size_t queue_free_len; size_t nblocks; - - if( ctx == NULL ) - { - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } - else if( ( ilen > 0U ) && ( input == NULL ) ) - { - /* input pointer is allowed to be NULL only if ilen == 0 */ - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } + POLY1305_VALIDATE_RET( ctx != NULL ); + POLY1305_VALIDATE_RET( ilen == 0 || input != NULL ); if( ( remaining > 0U ) && ( ctx->queue_len > 0U ) ) { @@ -398,10 +393,8 @@ int mbedtls_poly1305_update( mbedtls_poly1305_context *ctx, int mbedtls_poly1305_finish( mbedtls_poly1305_context *ctx, unsigned char mac[16] ) { - if( ( ctx == NULL ) || ( mac == NULL ) ) - { - return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - } + POLY1305_VALIDATE_RET( ctx != NULL ); + POLY1305_VALIDATE_RET( mac != NULL ); /* Process any leftover data */ if( ctx->queue_len > 0U ) @@ -431,6 +424,9 @@ int mbedtls_poly1305_mac( const unsigned char key[32], { mbedtls_poly1305_context ctx; int ret; + POLY1305_VALIDATE_RET( key != NULL ); + POLY1305_VALIDATE_RET( mac != NULL ); + POLY1305_VALIDATE_RET( ilen == 0 || input != NULL ); mbedtls_poly1305_init( &ctx ); From ae2ff02ff107c78bc9cd60caa9f681f36a71c0af Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Dec 2018 15:14:02 +0000 Subject: [PATCH 3/7] Add tests for ChaChaPoly parameter validation Parameter validation was previously performed and tested unconditionally for the ChaCha/Poly modules. This commit therefore only needs go guard the existing tests accordingly and use the appropriate test macros for parameter validation. --- tests/suites/test_suite_chacha20.function | 57 ++++--- tests/suites/test_suite_chachapoly.function | 161 +++++++++----------- tests/suites/test_suite_poly1305.function | 50 +++--- 3 files changed, 123 insertions(+), 145 deletions(-) diff --git a/tests/suites/test_suite_chacha20.function b/tests/suites/test_suite_chacha20.function index 669d91e79..23f6d9ec0 100644 --- a/tests/suites/test_suite_chacha20.function +++ b/tests/suites/test_suite_chacha20.function @@ -82,7 +82,7 @@ void chacha20_crypt( char *hex_key_string, } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ void chacha20_bad_params() { unsigned char key[32]; @@ -93,42 +93,37 @@ void chacha20_bad_params() size_t len = sizeof( src ); mbedtls_chacha20_context ctx; - mbedtls_chacha20_init( NULL ); - mbedtls_chacha20_free( NULL ); + TEST_INVALID_PARAM( mbedtls_chacha20_init( NULL ) ); - mbedtls_chacha20_init( &ctx ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_setkey( NULL, key ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_setkey( &ctx, NULL ) ); - TEST_ASSERT( mbedtls_chacha20_setkey( NULL, key ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chacha20_setkey( &ctx, NULL ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_starts( NULL, nonce, counter ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_starts( &ctx, NULL, counter ) ); - TEST_ASSERT( mbedtls_chacha20_starts( NULL, nonce, counter ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chacha20_starts( &ctx, NULL, counter ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_update( NULL, 0, src, dst ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_update( &ctx, len, NULL, dst ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_update( &ctx, len, src, NULL ) ); - TEST_ASSERT( mbedtls_chacha20_update( NULL, 0, src, dst ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chacha20_update( &ctx, len, NULL, dst ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chacha20_update( &ctx, len, src, NULL ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chacha20_update( &ctx, 0, NULL, NULL ) - == 0 ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_crypt( NULL, nonce, counter, 0, src, dst ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_crypt( key, NULL, counter, 0, src, dst ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_crypt( key, nonce, counter, len, NULL, dst ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, + mbedtls_chacha20_crypt( key, nonce, counter, len, src, NULL ) ); - mbedtls_chacha20_free( &ctx ); +exit: + return; - TEST_ASSERT( mbedtls_chacha20_crypt( NULL, nonce, counter, 0, src, dst ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chacha20_crypt( key, NULL, counter, 0, src, dst ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chacha20_crypt( key, nonce, counter, len, NULL, dst ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chacha20_crypt( key, nonce, counter, len, src, NULL ) - == MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chacha20_crypt( key, nonce, counter, 0, NULL, NULL ) - == 0 ); } /* END_CASE */ diff --git a/tests/suites/test_suite_chachapoly.function b/tests/suites/test_suite_chachapoly.function index 95dfd8a91..51cc0abd8 100644 --- a/tests/suites/test_suite_chachapoly.function +++ b/tests/suites/test_suite_chachapoly.function @@ -118,7 +118,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ void chachapoly_bad_params() { unsigned char key[32]; @@ -138,124 +138,113 @@ void chachapoly_bad_params() memset( output, 0x00, sizeof( output ) ); memset( mac, 0x00, sizeof( mac ) ); - mbedtls_chachapoly_init( NULL ); - mbedtls_chachapoly_free( NULL ); + TEST_INVALID_PARAM( mbedtls_chachapoly_init( NULL ) ); - mbedtls_chachapoly_init( &ctx ); + /* setkey */ + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_setkey( NULL, key ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_setkey( &ctx, NULL ) ); - TEST_ASSERT( mbedtls_chachapoly_setkey( NULL, key ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_setkey( &ctx, NULL ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - - TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( NULL, + /* encrypt_and_tag */ + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_encrypt_and_tag( NULL, 0, nonce, aad, 0, - input, output, mac ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, + input, output, mac ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_encrypt_and_tag( &ctx, 0, NULL, aad, 0, - input, output, mac ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, + input, output, mac ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_encrypt_and_tag( &ctx, 0, nonce, NULL, aad_len, - input, output, mac ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, + input, output, mac ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_encrypt_and_tag( &ctx, input_len, nonce, aad, 0, - NULL, output, mac ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, + NULL, output, mac ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_encrypt_and_tag( &ctx, input_len, nonce, aad, 0, - input, NULL, mac ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, + input, NULL, mac ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_encrypt_and_tag( &ctx, 0, nonce, aad, 0, - input, output, NULL ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); + input, output, NULL ) ); - TEST_ASSERT( mbedtls_chachapoly_auth_decrypt( NULL, + /* auth_decrypt */ + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_auth_decrypt( NULL, 0, nonce, aad, 0, - mac, input, output ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_auth_decrypt( &ctx, + mac, input, output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_auth_decrypt( &ctx, 0, NULL, aad, 0, - mac, input, output ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_auth_decrypt( &ctx, + mac, input, output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_auth_decrypt( &ctx, 0, nonce, NULL, aad_len, - mac, input, output ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_auth_decrypt( &ctx, + mac, input, output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_auth_decrypt( &ctx, 0, nonce, aad, 0, - NULL, input, output ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_auth_decrypt( &ctx, + NULL, input, output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_auth_decrypt( &ctx, input_len, nonce, aad, 0, - mac, NULL, output ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_auth_decrypt( &ctx, + mac, NULL, output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_auth_decrypt( &ctx, input_len, nonce, aad, 0, - mac, input, NULL ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); + mac, input, NULL ) ); - TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, - 0, nonce, - aad, aad_len, - NULL, NULL, mac ) - == 0 ); - TEST_ASSERT( mbedtls_chachapoly_auth_decrypt( &ctx, - 0, nonce, - aad, aad_len, - mac, NULL, NULL ) - == 0 ); + /* starts */ + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_starts( NULL, nonce, + MBEDTLS_CHACHAPOLY_ENCRYPT ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_starts( &ctx, NULL, + MBEDTLS_CHACHAPOLY_ENCRYPT ) ); - TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, - input_len, nonce, - NULL, 0, - input, output, mac ) - == 0 ); - TEST_ASSERT( mbedtls_chachapoly_auth_decrypt( &ctx, - input_len, nonce, - NULL, 0, - mac, input, output ) - == 0 ); + /* update_aad */ + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_update_aad( NULL, aad, + aad_len ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_update_aad( &ctx, NULL, + aad_len ) ); - TEST_ASSERT( mbedtls_chachapoly_starts( NULL, nonce, MBEDTLS_CHACHAPOLY_ENCRYPT ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_starts( &ctx, NULL, MBEDTLS_CHACHAPOLY_ENCRYPT ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); + /* update */ + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_update( NULL, input_len, + input, output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_update( &ctx, input_len, + NULL, output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_update( &ctx, input_len, + input, NULL ) ); - TEST_ASSERT( mbedtls_chachapoly_update_aad( NULL, aad, aad_len ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_update_aad( &ctx, NULL, aad_len ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - - TEST_ASSERT( mbedtls_chachapoly_update( NULL, input_len, input, output ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_update( &ctx, input_len, NULL, output ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_update( &ctx, input_len, input, NULL ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - - TEST_ASSERT( mbedtls_chachapoly_finish( NULL, mac ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_finish( &ctx, NULL ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); + /* finish */ + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_finish( NULL, mac ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_chachapoly_finish( &ctx, NULL ) ); exit: - mbedtls_chachapoly_free( &ctx ); + return; } /* END_CASE */ diff --git a/tests/suites/test_suite_poly1305.function b/tests/suites/test_suite_poly1305.function index 62d2ad951..d4761fb60 100644 --- a/tests/suites/test_suite_poly1305.function +++ b/tests/suites/test_suite_poly1305.function @@ -88,7 +88,7 @@ void mbedtls_poly1305( char *hex_key_string, char *hex_mac_string, char *hex_src } /* END_CASE */ -/* BEGIN_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ void poly1305_bad_params() { unsigned char src[1]; @@ -97,38 +97,32 @@ void poly1305_bad_params() size_t src_len = sizeof( src ); mbedtls_poly1305_context ctx; - mbedtls_poly1305_init( NULL ); - mbedtls_poly1305_free( NULL ); + TEST_INVALID_PARAM( mbedtls_poly1305_init( NULL ) ); - mbedtls_poly1305_init( &ctx ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_poly1305_starts( NULL, key ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_poly1305_starts( &ctx, NULL ) ); - TEST_ASSERT( mbedtls_poly1305_starts( NULL, key ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_poly1305_starts( &ctx, NULL ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_poly1305_update( NULL, src, 0 ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_poly1305_update( &ctx, NULL, src_len ) ); - TEST_ASSERT( mbedtls_poly1305_update( NULL, src, 0 ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_poly1305_update( &ctx, NULL, src_len ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_poly1305_update( &ctx, NULL, 0 ) - == 0 ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_poly1305_finish( NULL, mac ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_poly1305_finish( &ctx, NULL ) ); - TEST_ASSERT( mbedtls_poly1305_finish( NULL, mac ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_poly1305_finish( &ctx, NULL ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_poly1305_mac( NULL, src, 0, mac ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_poly1305_mac( key, NULL, src_len, mac ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, + mbedtls_poly1305_mac( key, src, 0, NULL ) ); - TEST_ASSERT( mbedtls_poly1305_mac( NULL, src, 0, mac ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_poly1305_mac( key, NULL, src_len, mac ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_poly1305_mac( key, src, 0, NULL ) - == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_poly1305_mac( key, NULL, 0, mac ) - == 0 ); - - mbedtls_poly1305_free( &ctx ); +exit: + return; } /* END_CASE */ From e463c42902208531506b414cbf533a7cb2f3a9e8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Dec 2018 14:00:26 +0000 Subject: [PATCH 4/7] Minor improvements --- include/mbedtls/chacha20.h | 13 ++++++++----- library/chachapoly.c | 2 -- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/chacha20.h b/include/mbedtls/chacha20.h index 14436d68c..1c6cde07b 100644 --- a/include/mbedtls/chacha20.h +++ b/include/mbedtls/chacha20.h @@ -82,7 +82,8 @@ mbedtls_chacha20_context; * to \c mbedtls_chacha20_update(), and finally to * \c mbedtls_chacha20_free(). * - * \param ctx The ChaCha20 context to initialize. Must not be \c NULL. + * \param ctx The ChaCha20 context to initialize. + * This must not be \c NULL. */ void mbedtls_chacha20_init( mbedtls_chacha20_context *ctx ); @@ -90,7 +91,9 @@ void mbedtls_chacha20_init( mbedtls_chacha20_context *ctx ); * \brief This function releases and clears the specified ChaCha20 context. * * \param ctx The ChaCha20 context to clear. May be \c NULL, - * in which case this function is a no-op. + * in which case this function is a no-op. If it is not + * \c NULL, it must point to an initialized context. + * */ void mbedtls_chacha20_free( mbedtls_chacha20_context *ctx ); @@ -103,7 +106,7 @@ void mbedtls_chacha20_free( mbedtls_chacha20_context *ctx ); * \c mbedtls_chacha_update(). * * \param ctx The ChaCha20 context to which the key should be bound. - * Must be initialized. + * It must be initialized. * \param key The encryption/decryption key. Must be 32 bytes in length. * * \return \c 0 on success. @@ -123,7 +126,7 @@ int mbedtls_chacha20_setkey( mbedtls_chacha20_context *ctx, * messages encrypted with the same nonce and key. * * \param ctx The ChaCha20 context to which the nonce should be bound. - * Must be initialized. + * It must be initialized and bound to a key. * \param nonce The nonce. Must be 12 bytes in size. * \param counter The initial counter value. This is usually 0. * @@ -153,7 +156,7 @@ int mbedtls_chacha20_starts( mbedtls_chacha20_context* ctx, * key and nonce. * * \param ctx The ChaCha20 context to use for encryption or decryption. - * Must be initialized. + * It must be initialized and bound to a key and nonce. * \param size The length of the input data in bytes. * \param input The buffer holding the input data. * This pointer can be \c NULL if `size == 0`. diff --git a/library/chachapoly.c b/library/chachapoly.c index ee5cc345e..e6ea139ff 100644 --- a/library/chachapoly.c +++ b/library/chachapoly.c @@ -179,9 +179,7 @@ int mbedtls_chachapoly_update_aad( mbedtls_chachapoly_context *ctx, CHACHAPOLY_VALIDATE_RET( aad_len == 0 || aad != NULL ); if( ctx->state != CHACHAPOLY_STATE_AAD ) - { return( MBEDTLS_ERR_CHACHAPOLY_BAD_STATE ); - } ctx->aad_len += aad_len; From 236ea16c011d356266b32ef7ae817c0f2573563e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Dec 2018 14:00:34 +0000 Subject: [PATCH 5/7] Fix wrong conditional in free() functions --- library/chachapoly.c | 2 +- library/poly1305.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/chachapoly.c b/library/chachapoly.c index e6ea139ff..dc643dd61 100644 --- a/library/chachapoly.c +++ b/library/chachapoly.c @@ -108,7 +108,7 @@ void mbedtls_chachapoly_init( mbedtls_chachapoly_context *ctx ) void mbedtls_chachapoly_free( mbedtls_chachapoly_context *ctx ) { - if( ctx != NULL ) + if( ctx == NULL ) return; mbedtls_chacha20_free( &ctx->chacha20_ctx ); diff --git a/library/poly1305.c b/library/poly1305.c index c22a0a1ba..b27411918 100644 --- a/library/poly1305.c +++ b/library/poly1305.c @@ -289,7 +289,7 @@ void mbedtls_poly1305_init( mbedtls_poly1305_context *ctx ) void mbedtls_poly1305_free( mbedtls_poly1305_context *ctx ) { - if( ctx != NULL ) + if( ctx == NULL ) return; mbedtls_platform_zeroize( ctx, sizeof( mbedtls_poly1305_context ) ); From a994b2379fcb29ad3e22167c13beb334cbd2ab3d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Dec 2018 16:44:41 +0000 Subject: [PATCH 6/7] Test that xxx_free() functions accept NULL parameter --- tests/suites/test_suite_chacha20.function | 1 + tests/suites/test_suite_chachapoly.function | 1 + tests/suites/test_suite_poly1305.function | 1 + 3 files changed, 3 insertions(+) diff --git a/tests/suites/test_suite_chacha20.function b/tests/suites/test_suite_chacha20.function index 23f6d9ec0..49b389c7f 100644 --- a/tests/suites/test_suite_chacha20.function +++ b/tests/suites/test_suite_chacha20.function @@ -94,6 +94,7 @@ void chacha20_bad_params() mbedtls_chacha20_context ctx; TEST_INVALID_PARAM( mbedtls_chacha20_init( NULL ) ); + TEST_VALID_PARAM( mbedtls_chacha20_free( NULL ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA, mbedtls_chacha20_setkey( NULL, key ) ); diff --git a/tests/suites/test_suite_chachapoly.function b/tests/suites/test_suite_chachapoly.function index 51cc0abd8..8e56bf69a 100644 --- a/tests/suites/test_suite_chachapoly.function +++ b/tests/suites/test_suite_chachapoly.function @@ -139,6 +139,7 @@ void chachapoly_bad_params() memset( mac, 0x00, sizeof( mac ) ); TEST_INVALID_PARAM( mbedtls_chachapoly_init( NULL ) ); + TEST_VALID_PARAM( mbedtls_chachapoly_free( NULL ) ); /* setkey */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, diff --git a/tests/suites/test_suite_poly1305.function b/tests/suites/test_suite_poly1305.function index d4761fb60..066bb3942 100644 --- a/tests/suites/test_suite_poly1305.function +++ b/tests/suites/test_suite_poly1305.function @@ -98,6 +98,7 @@ void poly1305_bad_params() mbedtls_poly1305_context ctx; TEST_INVALID_PARAM( mbedtls_poly1305_init( NULL ) ); + TEST_VALID_PARAM( mbedtls_poly1305_free( NULL ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA, mbedtls_poly1305_starts( NULL, key ) ); From ad7581fac577fb1d69901e3a47fc5144bb4f9f1e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 17 Dec 2018 09:43:21 +0000 Subject: [PATCH 7/7] Minor improvements to ChaCha20/Poly1305/ChaChaPoly documentation --- include/mbedtls/chacha20.h | 27 +++++++++++--------- include/mbedtls/chachapoly.h | 49 ++++++++++++++++++++---------------- include/mbedtls/poly1305.h | 33 +++++++++++++----------- 3 files changed, 60 insertions(+), 49 deletions(-) diff --git a/include/mbedtls/chacha20.h b/include/mbedtls/chacha20.h index 1c6cde07b..2ae5e6e5f 100644 --- a/include/mbedtls/chacha20.h +++ b/include/mbedtls/chacha20.h @@ -88,9 +88,10 @@ mbedtls_chacha20_context; void mbedtls_chacha20_init( mbedtls_chacha20_context *ctx ); /** - * \brief This function releases and clears the specified ChaCha20 context. + * \brief This function releases and clears the specified + * ChaCha20 context. * - * \param ctx The ChaCha20 context to clear. May be \c NULL, + * \param ctx The ChaCha20 context to clear. This may be \c NULL, * in which case this function is a no-op. If it is not * \c NULL, it must point to an initialized context. * @@ -107,7 +108,8 @@ void mbedtls_chacha20_free( mbedtls_chacha20_context *ctx ); * * \param ctx The ChaCha20 context to which the key should be bound. * It must be initialized. - * \param key The encryption/decryption key. Must be 32 bytes in length. + * \param key The encryption/decryption key. This must be \c 32 Bytes + * in length. * * \return \c 0 on success. * \return #MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA if ctx or key is NULL. @@ -127,8 +129,8 @@ int mbedtls_chacha20_setkey( mbedtls_chacha20_context *ctx, * * \param ctx The ChaCha20 context to which the nonce should be bound. * It must be initialized and bound to a key. - * \param nonce The nonce. Must be 12 bytes in size. - * \param counter The initial counter value. This is usually 0. + * \param nonce The nonce. This must be \c 12 Bytes in size. + * \param counter The initial counter value. This is usually \c 0. * * \return \c 0 on success. * \return #MBEDTLS_ERR_CHACHA20_BAD_INPUT_DATA if ctx or nonce is @@ -157,11 +159,11 @@ int mbedtls_chacha20_starts( mbedtls_chacha20_context* ctx, * * \param ctx The ChaCha20 context to use for encryption or decryption. * It must be initialized and bound to a key and nonce. - * \param size The length of the input data in bytes. + * \param size The length of the input data in Bytes. * \param input The buffer holding the input data. * This pointer can be \c NULL if `size == 0`. * \param output The buffer holding the output data. - * Must be able to hold \p size bytes. + * This must be able to hold \p size Bytes. * This pointer can be \c NULL if `size == 0`. * * \return \c 0 on success. @@ -186,14 +188,15 @@ int mbedtls_chacha20_update( mbedtls_chacha20_context *ctx, * \note The \p input and \p output pointers must either be equal or * point to non-overlapping buffers. * - * \param key The encryption/decryption key. Must be 32 bytes in length. - * \param nonce The nonce. Must be 12 bytes in size. - * \param counter The initial counter value. This is usually 0. - * \param size The length of the input data in bytes. + * \param key The encryption/decryption key. + * This must be \c 32 Bytes in length. + * \param nonce The nonce. This must be \c 12 Bytes in size. + * \param counter The initial counter value. This is usually \c 0. + * \param size The length of the input data in Bytes. * \param input The buffer holding the input data. * This pointer can be \c NULL if `size == 0`. * \param output The buffer holding the output data. - * Must be able to hold \p size bytes. + * This must be able to hold \p size Bytes. * This pointer can be \c NULL if `size == 0`. * * \return \c 0 on success. diff --git a/include/mbedtls/chachapoly.h b/include/mbedtls/chachapoly.h index 80d20e54a..49e615d27 100644 --- a/include/mbedtls/chachapoly.h +++ b/include/mbedtls/chachapoly.h @@ -123,7 +123,7 @@ void mbedtls_chachapoly_init( mbedtls_chachapoly_context *ctx ); * \brief This function releases and clears the specified * ChaCha20-Poly1305 context. * - * \param ctx The ChachaPoly context to clear. May be \c NULL, in which + * \param ctx The ChachaPoly context to clear. This may be \c NULL, in which * case this function is a no-op. */ void mbedtls_chachapoly_free( mbedtls_chachapoly_context *ctx ); @@ -133,8 +133,8 @@ void mbedtls_chachapoly_free( mbedtls_chachapoly_context *ctx ); * symmetric encryption key. * * \param ctx The ChaCha20-Poly1305 context to which the key should be - * bound. Must be initialized. - * \param key The 256-bit (32 bytes) key. + * bound. This must be initialized. + * \param key The \c 256 Bit (\c 32 Bytes) key. * * \return \c 0 on success. * \return A negative error code on failure. @@ -157,8 +157,10 @@ int mbedtls_chachapoly_setkey( mbedtls_chachapoly_context *ctx, * \warning Decryption with the piecewise API is discouraged, see the * warning on \c mbedtls_chachapoly_init(). * - * \param ctx The ChaCha20-Poly1305 context. Must be initialized. - * \param nonce The nonce/IV to use for the message. Must be 12 bytes. + * \param ctx The ChaCha20-Poly1305 context. This must be initialized + * and bound to a key. + * \param nonce The nonce/IV to use for the message. + * This must be a redable buffer of length \c 12 Bytes. * \param mode The operation to perform: #MBEDTLS_CHACHAPOLY_ENCRYPT or * #MBEDTLS_CHACHAPOLY_DECRYPT (discouraged, see warning). * @@ -194,11 +196,12 @@ int mbedtls_chachapoly_starts( mbedtls_chachapoly_context *ctx, * \warning Decryption with the piecewise API is discouraged, see the * warning on \c mbedtls_chachapoly_init(). * - * \param ctx The ChaCha20-Poly1305 context to use. - * \param aad_len The length (in bytes) of the AAD. The length has no + * \param ctx The ChaCha20-Poly1305 context. This must be initialized + * and bound to a key. + * \param aad_len The length in Bytes of the AAD. The length has no * restrictions. * \param aad Buffer containing the AAD. - * This pointer can be NULL if aad_len == 0. + * This pointer can be \c NULL if `aad_len == 0`. * * \return \c 0 on success. * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA @@ -228,12 +231,12 @@ int mbedtls_chachapoly_update_aad( mbedtls_chachapoly_context *ctx, * \warning Decryption with the piecewise API is discouraged, see the * warning on \c mbedtls_chachapoly_init(). * - * \param ctx The ChaCha20-Poly1305 context to use. Must be initialized. + * \param ctx The ChaCha20-Poly1305 context to use. This must be initialized. * \param len The length (in bytes) of the data to encrypt or decrypt. * \param input The buffer containing the data to encrypt or decrypt. * This pointer can be \c NULL if `len == 0`. * \param output The buffer to where the encrypted or decrypted data is - * written. Must be able to hold \p len bytes. + * written. This must be able to hold \p len bytes. * This pointer can be \c NULL if `len == 0`. * * \return \c 0 on success. @@ -251,7 +254,7 @@ int mbedtls_chachapoly_update( mbedtls_chachapoly_context *ctx, * \brief This function finished the ChaCha20-Poly1305 operation and * generates the MAC (authentication tag). * - * \param ctx The ChaCha20-Poly1305 context to use. Must be initialized. + * \param ctx The ChaCha20-Poly1305 context to use. This must be initialized. * \param mac The buffer to where the 128-bit (16 bytes) MAC is written. * * \warning Decryption with the piecewise API is discouraged, see the @@ -279,17 +282,18 @@ int mbedtls_chachapoly_finish( mbedtls_chachapoly_context *ctx, * and key. * * \param ctx The ChaCha20-Poly1305 context to use (holds the key). - * Must be initialized. + * This must be initialized. * \param length The length (in bytes) of the data to encrypt or decrypt. * \param nonce The 96-bit (12 bytes) nonce/IV to use. - * \param aad The buffer containing the additional authenticated data (AAD). - * This pointer can be \c NULL if `aad_len == 0`. + * \param aad The buffer containing the additional authenticated + * data (AAD). This pointer can be \c NULL if `aad_len == 0`. * \param aad_len The length (in bytes) of the AAD data to process. * \param input The buffer containing the data to encrypt or decrypt. * This pointer can be \c NULL if `ilen == 0`. - * \param output The buffer to where the encrypted or decrypted data is written. - * This pointer can be \c NULL if `ilen == 0`. - * \param tag The buffer to where the computed 128-bit (16 bytes) MAC is written. + * \param output The buffer to where the encrypted or decrypted data + * is written. This pointer can be \c NULL if `ilen == 0`. + * \param tag The buffer to where the computed 128-bit (16 bytes) MAC + * is written. This must not be \c NULL. * * \return \c 0 on success. * \return A negative error code on failure. @@ -311,16 +315,17 @@ int mbedtls_chachapoly_encrypt_and_tag( mbedtls_chachapoly_context *ctx, * \c mbedtls_chachapoly_setkey(). * * \param ctx The ChaCha20-Poly1305 context to use (holds the key). - * \param length The length (in bytes) of the data to decrypt. - * \param nonce The 96-bit (12 bytes) nonce/IV to use. + * \param length The length (in Bytes) of the data to decrypt. + * \param nonce The \c 96 Bit (\c 12 bytes) nonce/IV to use. * \param aad The buffer containing the additional authenticated data (AAD). - * This pointer can be NULL if aad_len == 0. + * This pointer can be \c NULL if `aad_len == 0`. * \param aad_len The length (in bytes) of the AAD data to process. * \param tag The buffer holding the authentication tag. + * This must be a readable buffer of length \c 16 Bytes. * \param input The buffer containing the data to decrypt. - * This pointer can be NULL if ilen == 0. + * This pointer can be \c NULL if `ilen == 0`. * \param output The buffer to where the decrypted data is written. - * This pointer can be NULL if ilen == 0. + * This pointer can be \c NULL if `ilen == 0`. * * \return \c 0 on success. * \return #MBEDTLS_ERR_CHACHAPOLY_AUTH_FAILED diff --git a/include/mbedtls/poly1305.h b/include/mbedtls/poly1305.h index 617958e11..05866a2da 100644 --- a/include/mbedtls/poly1305.h +++ b/include/mbedtls/poly1305.h @@ -84,15 +84,18 @@ mbedtls_poly1305_context; * \c mbedtls_poly1305_finish(), then finally * \c mbedtls_poly1305_free(). * - * \param ctx The Poly1305 context to initialize. Must not be \c NULL. + * \param ctx The Poly1305 context to initialize. This must + * not be \c NULL. */ void mbedtls_poly1305_init( mbedtls_poly1305_context *ctx ); /** - * \brief This function releases and clears the specified Poly1305 context. + * \brief This function releases and clears the specified + * Poly1305 context. * - * \param ctx The Poly1305 context to clear. May be \c NULL, in which - * case this function is a no-op. + * \param ctx The Poly1305 context to clear. This may be \c NULL, in which + * case this function is a no-op. If it is not \c NULL, it must + * point to an initialized Poly1305 context. */ void mbedtls_poly1305_free( mbedtls_poly1305_context *ctx ); @@ -103,8 +106,8 @@ void mbedtls_poly1305_free( mbedtls_poly1305_context *ctx ); * invocation of Poly1305. * * \param ctx The Poly1305 context to which the key should be bound. - * Must be initialized. - * \param key The buffer containing the 32-byte (256-bit) key. + * This must be initialized. + * \param key The buffer containing the \c 32 Byte (\c 256 Bit) key. * * \return \c 0 on success. * \return A negative error code on failure. @@ -121,8 +124,8 @@ int mbedtls_poly1305_starts( mbedtls_poly1305_context *ctx, * It can be called repeatedly to process a stream of data. * * \param ctx The Poly1305 context to use for the Poly1305 operation. - * Must be initialized. - * \param ilen The length of the input data (in bytes). + * This must be initialized and bound to a key. + * \param ilen The length of the input data in Bytes. * Any value is accepted. * \param input The buffer holding the input data. * This pointer can be \c NULL if `ilen == 0`. @@ -139,9 +142,9 @@ int mbedtls_poly1305_update( mbedtls_poly1305_context *ctx, * Authentication Code (MAC). * * \param ctx The Poly1305 context to use for the Poly1305 operation. - * Must be initialized. - * \param mac The buffer to where the MAC is written. Must be big enough - * to hold the 16-byte MAC. + * This must be initialized and bound to a key. + * \param mac The buffer to where the MAC is written. This must + * be a writable buffer of length \c 16 Bytes. * * \return \c 0 on success. * \return A negative error code on failure. @@ -156,13 +159,13 @@ int mbedtls_poly1305_finish( mbedtls_poly1305_context *ctx, * \warning The key must be unique and unpredictable for each * invocation of Poly1305. * - * \param key The buffer containing the 32-byte (256-bit) key. - * \param ilen The length of the input data (in bytes). + * \param key The buffer containing the \c 32 Byte (\c 256 Bit) key. + * \param ilen The length of the input data in Bytes. * Any value is accepted. * \param input The buffer holding the input data. * This pointer can be \c NULL if `ilen == 0`. - * \param mac The buffer to where the MAC is written. Must be big enough - * to hold the 16-byte MAC. + * \param mac The buffer to where the MAC is written. This must be + * a writable buffer of length \c 16 Bytes. * * \return \c 0 on success. * \return A negative error code on failure.