Skip to content
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.

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

Fix tests 71, 72, 73, 74, 76, 77 #396

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

oberon-sk
Copy link
Contributor

Fix tests 71, 72, 73 to avoid invalid argument.
Fix test 74 wrong macro.
Fix tests 76, 77 missing destroy key.

Signed-off-by: Oberon microsystems AG

Fix test 74 wrong macro.
Fix tests 76, 77 missing destroy key.

Signed-off-by: Stephan Koch <[email protected]>
@@ -132,8 +132,7 @@ int32_t psa_pake_set_peer_test(caller_security_t caller __UNUSED)
/* Expect a bad state when psa_pake_set_peer is called on aborted inactive operation object */
status = val->crypto_function(VAL_CRYPTO_PAKE_SET_PEER,
&operation,
Copy link
Collaborator

Choose a reason for hiding this comment

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

To maintain consistency with other test cases, check for bad state can be added as

if( check1[i].expected_status == PSA_SUCCESS)
{
    status = val->crypto_function(VAL_CRYPTO_PAKE_SET_PEER, &operation, check1[I].peer_id, check1[I].peer_id_len);                                           
}

@@ -124,8 +124,7 @@ int32_t psa_pake_set_user_test(caller_security_t caller __UNUSED)
/* Expect a bad state when psa_pake_set_user is called on aborted inactive operation object */
Copy link
Collaborator

Choose a reason for hiding this comment

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

To maintain consistency with other test cases, check for bad state can be added as

if( check1[i].expected_status == PSA_SUCCESS)
{
    status = val->crypto_function(VAL_CRYPTO_PAKE_SET_USER, &operation, check1[i].user_id, check1[i].user_id_len);                                           
}

@@ -139,8 +139,7 @@ int32_t psa_pake_set_context_test(caller_security_t caller __UNUSED)
/* Expect a bad state if psa_pake_set_context is called on aborted inactive operation object */
status = val->crypto_function(VAL_CRYPTO_PAKE_SET_CONTEXT,
&operation,
check1[i].context,
check1[i].context_len);
(const uint8_t*)"context", 7);
Copy link
Collaborator

Choose a reason for hiding this comment

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

To maintain consistency with other test cases, check for bad state can be added as

if( check1[i].expected_status == PSA_SUCCESS)
{
    status = val->crypto_function(VAL_CRYPTO_PAKE_SET_CONTEXT, &operation, check1[i].context, check1[i].context_len);                                           
}

@@ -472,6 +472,12 @@ int32_t psa_pake_get_shared_key_test(caller_security_t caller __UNUSED)
TEST_ASSERT_DUAL(status, check1[i].expected_status[0],
check1[i].expected_status[1], TEST_CHECKPOINT_NUM(12));

if (status == PSA_SUCCESS) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the shared_key is destroyed by calling psa_destroy_key() here, same key identifier can not be used to extract shared key from server. To avoid this we can have two key identifiers for client and server as shared_ckey and shared_skey, shared_key as key identifier for JPAKE user shared secret.

Choose a reason for hiding this comment

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

I think this is a misunderstanding of the specification text - which is understandable, given the current wording in psa_destroy_key():

Destroying the key makes the key identifier invalid, and the key identifier must not be used again by the application.

This is meant to indicate that the key identifier cannot be used [to refer to the key that has been destroyed]. Using that key identifier for an operation will result in a PSA_ERROR_INVALID_HANDLE error, until a new key is created with the same key identifier value.

But note:

  1. It is not good practice for an application to create persistent keys for different purposes using the same identifier, even if they never exist at the same time. This could lead to key usage errors that can be avoided by using distinct key identifier values.
  2. Volatile key identifier values are allocated by the implementation - a robust implementation should not immediately reuse volatile key identifier values, to increase the likelihood that an application use-key-after-destroy error is detected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants