Skip to content

New function psa_hash_clone #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 22, 2019
Merged

New function psa_hash_clone #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ebb2c3e
New function psa_hash_clone
gilles-peskine-arm Jan 19, 2019
e43aa39
hash_clone: Fix copypasta and add a functional description
gilles-peskine-arm Jan 21, 2019
eb35d78
Style fix
gilles-peskine-arm Jan 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions include/psa/crypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,33 @@ psa_status_t psa_hash_verify(psa_hash_operation_t *operation,
*/
psa_status_t psa_hash_abort(psa_hash_operation_t *operation);

/** Clone a hash operation.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything else wouldn't make sense, but we might still mention here that the target context is active and in the same internal state as the source after a successful call to this function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a paragraph of English text that defines the behavior of the function.

*
* This function copies the state of an ongoing hash operation to
* a new operation object. In other words, this function is equivalent
* to calling psa_hash_setup() on \p target_operation with the same
* algorithm that \p source_operation was set up for, then
* psa_hash_update() on \p target_operation with the same input that
* that was passed to \p source_operation. After this function returns, the
* two objects are independent, i.e. subsequent calls involving one of
* the objects do not affect the other object.
*
* \param[in] source_operation The active hash operation to clone.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be documented that this should be initialized and active.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the current wording sufficent? Active implies initialized.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was an oversight on my side, because I was expecting the wording to be "The XXX. Must be YYY." instead of "The YYY XXX". I'd prefer the former, but that's not a blocker.

* \param[in,out] target_operation The operation object to set up.
* It must be initialized but not active.
*
* \retval #PSA_SUCCESS
* \retval #PSA_ERROR_BAD_STATE
* \p source_operation is not an active hash operation.
* \retval #PSA_ERROR_BAD_STATE
* \p target_operation is active.
* \retval #PSA_ERROR_COMMUNICATION_FAILURE
* \retval #PSA_ERROR_HARDWARE_FAILURE
* \retval #PSA_ERROR_TAMPERING_DETECTED
*/
psa_status_t psa_hash_clone(const psa_hash_operation_t *source_operation,
psa_hash_operation_t *target_operation);

/**@}*/

/** \defgroup MAC Message authentication codes
Expand Down
61 changes: 61 additions & 0 deletions library/psa_crypto.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1421,6 +1421,67 @@ psa_status_t psa_hash_verify( psa_hash_operation_t *operation,
return( PSA_SUCCESS );
}

psa_status_t psa_hash_clone( const psa_hash_operation_t *source_operation,
psa_hash_operation_t *target_operation )
{
if( target_operation->alg != 0 )
return( PSA_ERROR_BAD_STATE );

switch( source_operation->alg )
{
case 0:
return( PSA_ERROR_BAD_STATE );
#if defined(MBEDTLS_MD2_C)
case PSA_ALG_MD2:
mbedtls_md2_clone( &target_operation->ctx.md2,
&source_operation->ctx.md2 );
break;
#endif
#if defined(MBEDTLS_MD4_C)
case PSA_ALG_MD4:
mbedtls_md4_clone( &target_operation->ctx.md4,
&source_operation->ctx.md4 );
break;
#endif
#if defined(MBEDTLS_MD5_C)
case PSA_ALG_MD5:
mbedtls_md5_clone( &target_operation->ctx.md5,
&source_operation->ctx.md5 );
break;
#endif
#if defined(MBEDTLS_RIPEMD160_C)
case PSA_ALG_RIPEMD160:
mbedtls_ripemd160_clone( &target_operation->ctx.ripemd160,
&source_operation->ctx.ripemd160 );
break;
#endif
#if defined(MBEDTLS_SHA1_C)
case PSA_ALG_SHA_1:
mbedtls_sha1_clone( &target_operation->ctx.sha1,
&source_operation->ctx.sha1 );
break;
#endif
#if defined(MBEDTLS_SHA256_C)
case PSA_ALG_SHA_224:
case PSA_ALG_SHA_256:
mbedtls_sha256_clone( &target_operation->ctx.sha256,
&source_operation->ctx.sha256 );
break;
#endif
#if defined(MBEDTLS_SHA512_C)
case PSA_ALG_SHA_384:
case PSA_ALG_SHA_512:
mbedtls_sha512_clone( &target_operation->ctx.sha512,
&source_operation->ctx.sha512 );
break;
#endif
default:
return( PSA_ERROR_NOT_SUPPORTED );
}

target_operation->alg = source_operation->alg;
return( PSA_SUCCESS );
}


/****************************************************************/
Expand Down
6 changes: 6 additions & 0 deletions tests/suites/test_suite_psa_crypto.data
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,12 @@ hash_verify_bad_args:
PSA hash finish: bad arguments
hash_finish_bad_args:

PSA hash clone: source state
hash_clone_source_state:

PSA hash clone: target state
hash_clone_target_state:

MAC operation object initializers zero properly
mac_operation_init:

Expand Down
86 changes: 86 additions & 0 deletions tests/suites/test_suite_psa_crypto.function
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1924,6 +1924,92 @@ exit:
}
/* END_CASE */

/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C */
void hash_clone_source_state( )
{
psa_algorithm_t alg = PSA_ALG_SHA_256;
unsigned char hash[PSA_HASH_MAX_SIZE];
psa_hash_operation_t op_source = PSA_HASH_OPERATION_INIT;
psa_hash_operation_t op_init = PSA_HASH_OPERATION_INIT;
psa_hash_operation_t op_setup = PSA_HASH_OPERATION_INIT;
psa_hash_operation_t op_finished = PSA_HASH_OPERATION_INIT;
psa_hash_operation_t op_aborted = PSA_HASH_OPERATION_INIT;
size_t hash_len;

PSA_ASSERT( psa_crypto_init( ) );
PSA_ASSERT( psa_hash_setup( &op_source, alg ) );

PSA_ASSERT( psa_hash_setup( &op_setup, alg ) );
PSA_ASSERT( psa_hash_setup( &op_finished, alg ) );
PSA_ASSERT( psa_hash_finish( &op_finished,
hash, sizeof( hash ), &hash_len ) );
PSA_ASSERT( psa_hash_setup( &op_aborted, alg ) );
PSA_ASSERT( psa_hash_abort( &op_aborted ) );

TEST_EQUAL( psa_hash_clone( &op_source, &op_setup ),
PSA_ERROR_BAD_STATE );

PSA_ASSERT( psa_hash_clone( &op_source, &op_init ) );
PSA_ASSERT( psa_hash_finish( &op_init,
hash, sizeof( hash ), &hash_len ) );
PSA_ASSERT( psa_hash_clone( &op_source, &op_finished ) );
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this, I noticed that the documentation doesn't indicate the abstract state of a hash context after it has been terminated (through psa_hash_finish, psa_hash_abort, or psa_hash_verify). From my understanding, it's initialized but not setup/active. If so, you might consider documenting it, e.g. here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't critical to understanding the current behavior, and other types of operation should use consistent terminology, so I'll do that in a later writing pass. Tracked internally at
https://github.com/ARMmbed/psa-crypto/issues/132

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

PSA_ASSERT( psa_hash_finish( &op_finished,
hash, sizeof( hash ), &hash_len ) );
PSA_ASSERT( psa_hash_clone( &op_source, &op_aborted ) );
PSA_ASSERT( psa_hash_finish( &op_aborted,
hash, sizeof( hash ), &hash_len ) );

exit:
psa_hash_abort( &op_source );
psa_hash_abort( &op_init );
psa_hash_abort( &op_setup );
psa_hash_abort( &op_finished );
psa_hash_abort( &op_aborted );
mbedtls_psa_crypto_free( );
}
/* END_CASE */

/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C */
void hash_clone_target_state( )
{
psa_algorithm_t alg = PSA_ALG_SHA_256;
unsigned char hash[PSA_HASH_MAX_SIZE];
psa_hash_operation_t op_init = PSA_HASH_OPERATION_INIT;
psa_hash_operation_t op_setup = PSA_HASH_OPERATION_INIT;
psa_hash_operation_t op_finished = PSA_HASH_OPERATION_INIT;
psa_hash_operation_t op_aborted = PSA_HASH_OPERATION_INIT;
psa_hash_operation_t op_target = PSA_HASH_OPERATION_INIT;
size_t hash_len;

PSA_ASSERT( psa_crypto_init( ) );

PSA_ASSERT( psa_hash_setup( &op_setup, alg ) );
PSA_ASSERT( psa_hash_setup( &op_finished, alg ) );
PSA_ASSERT( psa_hash_finish( &op_finished,
hash, sizeof( hash ), &hash_len ) );
PSA_ASSERT( psa_hash_setup( &op_aborted, alg ) );
PSA_ASSERT( psa_hash_abort( &op_aborted ) );

PSA_ASSERT( psa_hash_clone( &op_setup, &op_target ) );
PSA_ASSERT( psa_hash_finish( &op_target,
hash, sizeof( hash ), &hash_len ) );

TEST_EQUAL( psa_hash_clone( &op_init, &op_target ), PSA_ERROR_BAD_STATE );
TEST_EQUAL( psa_hash_clone( &op_finished, &op_target ),
PSA_ERROR_BAD_STATE );
TEST_EQUAL( psa_hash_clone( &op_aborted, &op_target ),
PSA_ERROR_BAD_STATE );

exit:
psa_hash_abort( &op_target );
psa_hash_abort( &op_init );
psa_hash_abort( &op_setup );
psa_hash_abort( &op_finished );
psa_hash_abort( &op_aborted );
mbedtls_psa_crypto_free( );
}
/* END_CASE */

/* BEGIN_CASE */
void mac_operation_init( )
{
Expand Down
10 changes: 9 additions & 1 deletion tests/suites/test_suite_psa_crypto_hash.function
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ void hash_multi_part( int alg_arg, data_t *input, data_t *expected_hash )
unsigned char actual_hash[PSA_HASH_MAX_SIZE];
size_t actual_hash_length;
psa_hash_operation_t operation = PSA_HASH_OPERATION_INIT;
psa_hash_operation_t operation2 = PSA_HASH_OPERATION_INIT;
uint32_t len = 0;

PSA_ASSERT( psa_crypto_init( ) );
Expand All @@ -78,16 +79,23 @@ void hash_multi_part( int alg_arg, data_t *input, data_t *expected_hash )

PSA_ASSERT( psa_hash_update( &operation,
input->x, len ) );
PSA_ASSERT( psa_hash_clone( &operation, &operation2 ) );
PSA_ASSERT( psa_hash_update( &operation,
input->x + len, input->len - len ) );
PSA_ASSERT( psa_hash_update( &operation2,
input->x + len, input->len - len ) );

PSA_ASSERT( psa_hash_finish( &operation,
actual_hash, sizeof( actual_hash ),
&actual_hash_length ) );

ASSERT_COMPARE( expected_hash->x, expected_hash->len,
actual_hash, actual_hash_length );

PSA_ASSERT( psa_hash_finish( &operation2,
actual_hash, sizeof( actual_hash ),
&actual_hash_length ) );
ASSERT_COMPARE( expected_hash->x, expected_hash->len,
actual_hash, actual_hash_length );
} while( len++ != input->len );

exit:
Expand Down