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.

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

Jump to bottom

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

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

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2ce516
Fix tests 71, 72, 73 to avoid invalid argument.
oberon-sk Nov 14, 2024
00e7d62
Use bad state checking pattern.
oberon-sk Nov 19, 2024
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
3 changes: 1 addition & 2 deletions api-tests/dev_apis/crypto/test_c071/test_c071.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);                                           
}

status = val->crypto_function(VAL_CRYPTO_PAKE_SET_USER,
&operation,
check1[i].user_id,
check1[i].user_id_len);
(const uint8_t*)"user", 4);
TEST_ASSERT_EQUAL(status, PSA_ERROR_BAD_STATE, TEST_CHECKPOINT_NUM(9));
}

Expand Down
3 changes: 1 addition & 2 deletions api-tests/dev_apis/crypto/test_c072/test_c072.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);                                           
}

check1[i].peer_id,
check1[i].peer_id_len);
(const uint8_t*)"peer", 4);
TEST_ASSERT_EQUAL(status, PSA_ERROR_BAD_STATE, TEST_CHECKPOINT_NUM(10));
}
return VAL_STATUS_SUCCESS;
Expand Down
3 changes: 1 addition & 2 deletions api-tests/dev_apis/crypto/test_c073/test_c073.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);                                           
}

TEST_ASSERT_EQUAL(status, PSA_ERROR_BAD_STATE, TEST_CHECKPOINT_NUM(11));
}
return VAL_STATUS_SUCCESS;
Expand Down
2 changes: 1 addition & 1 deletion api-tests/dev_apis/crypto/test_c074/test_c074.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ int32_t psa_pake_output_test(caller_security_t caller __UNUSED)

}

if (PSA_ALG_JPAKE(PSA_ALG_SHA_256))
if (PSA_ALG_IS_JPAKE (check1[i].alg))
{
/* Abort the PAKE operation object */
status = val->crypto_function(VAL_CRYPTO_PAKE_ABORT, &user);
Expand Down
4 changes: 4 additions & 0 deletions api-tests/dev_apis/crypto/test_c076/test_c076.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ int32_t psa_pake_abort_test(caller_security_t caller __UNUSED)
&operation);
TEST_ASSERT_EQUAL(status, check1[i].expected_status, TEST_CHECKPOINT_NUM(6));

/* Destroy the key */
status = val->crypto_function(VAL_CRYPTO_DESTROY_KEY, key);
TEST_ASSERT_EQUAL(status, PSA_SUCCESS, TEST_CHECKPOINT_NUM(7));

}
return VAL_STATUS_SUCCESS;
}
6 changes: 6 additions & 0 deletions api-tests/dev_apis/crypto/test_c077/test_c077.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Copy link

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.

/* Destroy the key */
status = val->crypto_function(VAL_CRYPTO_DESTROY_KEY, shared_key);
TEST_ASSERT_EQUAL(status, PSA_SUCCESS, TEST_CHECKPOINT_NUM(14));
}

status = val->crypto_function(VAL_CRYPTO_PAKE_GET_SHARED_KEY,
&server, &attributes, &shared_key);
TEST_ASSERT_DUAL(status, check1[i].expected_status[0],
Expand Down