xtest: pkcs11: add tests for indestructible token objects#803
xtest: pkcs11: add tests for indestructible token objects#803sahesaha wants to merge 1 commit intoOP-TEE:masterfrom
Conversation
Add test cases that validate CKA_INDESTRUCTIBLE behavior: - A session object with CKA_INDESTRUCTIBLE=CK_TRUE and CKA_TOKEN=CK_FALSE is rejected with CKR_TEMPLATE_INCONSISTENT. - Token objects marked CKA_INDESTRUCTIBLE cannot be destroyed. Tested on: SM7325 SoC Reviewed-by: Neeraj Soni <neersoni@qti.qualcomm.com> Signed-off-by: Saheli Saha <sahesaha@qti.qualcomm.com>
|
This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note that you can always re-open a closed pull request at any time. |
|
Commenting to keep alive. |
|
This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note that you can always re-open a closed pull request at any time. |
There was a problem hiding this comment.
Thanks for the test sequences related to indestructible objects.
The issue is that once such objects are created, they're never destroyed and the pkcs11 TA persistent database would continuously grow. That said, it's nice for testing purpose only when it's easy to wipe the persistent database. I would suggest to add a config switch (e.g. CFG_PKCS11_TA_TEST_INDESTRUCTIBLE_OBJECT_ATTR=y|n) so we can leverage these tests on Qemu (emulated persistent secure storage) and maybe other platforms where one can deal with such forced database destruction.
| rv = C_CreateObject(session, cktest_destructible_token, | ||
| ARRAY_SIZE(cktest_destructible_token), | ||
| &obj_hdl2); | ||
| if (!ADBG_EXPECT_CK_OK(c, rv)) | ||
| goto end; |
There was a problem hiding this comment.
Remove these lines? the destrcutible object is create right below, after C_DestroyObject(session, obj_hdl) call.
| goto end; | ||
|
|
||
| /* Indestructible object is accessible */ | ||
| rv = C_GetObjectSize(session, obj_hdl, &obj_size); |
There was a problem hiding this comment.
Once the sessions are closed (even if token is NOT re-initialized), the object handles should no more be valid. You should still be able to find the object, do not access its handle.
| /* | ||
| * This test involves creating multiple token keys with both | ||
| * indestructible and destructible objects, and checking | ||
| * the uniqueness of key handles. |
There was a problem hiding this comment.
Objects being destructible or not, handles of created objects are always different. I don't think this test is useful.
| /* | ||
| * Creating object with incorrect template | ||
| * (CKA_TOKEN is false but CKA_INDESTRUCTIBLE is true) | ||
| */ |
There was a problem hiding this comment.
Could you fix the indentation?
| if (!ADBG_EXPECT_CK_RESULT(c, CKR_TEMPLATE_INCONSISTENT, rv)) | ||
| goto end; | ||
|
|
||
| rv = C_GetObjectSize(session, obj_hdl, &obj_size); |
There was a problem hiding this comment.
It is a programming error to use obj_hdl when previous call to C_CreateObject() failed. The PKCS#11 API does not say the generated object handle needs to be set the CK_INVALID_ID or whatever when such C_CreateObject()/C_CopyOject()/C_DeriveKey()/... fail.
| goto end; | ||
|
|
||
| /* Indestructible object is accessible */ | ||
| rv = C_GetObjectSize(session, obj_hdl, &obj_size2); |
There was a problem hiding this comment.
You should search the object first (use C_FindObjects*() functions), not reuse obj_hdl that is bound to a session.
(I need to verify this assertion).
| { CKA_CLASS, &(CK_OBJECT_CLASS){CKO_SECRET_KEY}, | ||
| sizeof(CK_OBJECT_CLASS) }, | ||
| { CKA_KEY_TYPE, &(CK_KEY_TYPE){CKK_AES}, sizeof(CK_KEY_TYPE) }, | ||
| { CKA_KEY_TYPE, &(CK_KEY_TYPE) { CKK_AES }, sizeof(CK_KEY_TYPE) }, |
There was a problem hiding this comment.
I'd suggest to discard this change.
(or change also all other equivalent occurrences, preferably in a dedicated commit)
There was a problem hiding this comment.
Hi @etienne-lms, this was shown as an error when I ran the checkpatch for this PR. hence fixed it.
Add test cases that validate CKA_INDESTRUCTIBLE behavior:
Tested on: SM7325 SoC
Reviewed-by: Neeraj Soni neersoni@qti.qualcomm.com
Signed-off-by: Saheli Saha sahesaha@qti.qualcomm.com