Added Indestructible token objects attributes/keys feature. #7581
Conversation
|
Hi @sahesaha, there is a mistake here, you added a patch file instead of modifying the source files themselves. |
sahesaha
force-pushed
the
Indestructible_token-objects
branch
from
e39c3e5 to
fcd2203
Compare
|
Hi @jforissier, could you please check now? I have removed the patch file from the commit. |
|
Since this is not contained in the PKCS11 specification, this is a vendor extension that I am not sure we want to support. Do other PKCS11 implementations implement something similar? What exactly do you use this extension for? |
sahesaha
force-pushed
the
Indestructible_token-objects
branch
from
fcd2203 to
3f99281
Compare
|
Hi @Emantor, the use case is to stop some objects from getting destroyed even if C_Destroy is called on them and to prevent destruction of token objects on reinitialization if they are declared CKA_INDESTRUCTIBLE, a tamper detection key can be an example which is provisioned during factory setting and needs to be persisted even after C_Destroy call. The test cases designed to validate these changes will be raised soon in optee_test repo. |
etienne-lms
left a comment
etienne-lms
left a comment
There was a problem hiding this comment.
Hi, the PKCS#11 allows to token to not destroy some objects. Using a vendor attribute for that looks ok to me. Maybe this should be under a TA config switch (CFG_PKCS11_TA_INDESTRUCTIBLE_OBJECT=y|n).
ta/pkcs11/include/pkcs11_ta.h
Outdated
| 0x0600, | ||
|
|
||
| /* Vendor specific attributes */ | ||
| PKCS11_CKA_INDESTRUCTIBLE = 0x80000010, |
There was a problem hiding this comment.
Prefer have this listed below, next to PKCS11_CKA_OPTEE_HIDDEN_EC_POINT definition and use ID 0x0001 | (1<<31) for consisteny of the IDs of OP-TEE vendor specific attribute IDs.
Also, using OPTEE_ prefix would better highlight across the implementation that it's a custom ID.
| PKCS11_CKA_INDESTRUCTIBLE = 0x80000010, | |
| PKCS11_CKA_OPTEE_INDESTRUCTIBLE = PKCS11_CKA_VENDOR_DEFINED | | |
| 0x0001, |
| /* Try twice otherwise panic! */ | ||
| if (unregister_persistent_object(token, obj->uuid) && | ||
| unregister_persistent_object(token, obj->uuid)) | ||
| unregister_persistent_object(token, obj->uuid)) |
There was a problem hiding this comment.
Discard this change. Previous indentation was fine.
|
|
||
| if (get_bool(obj->attributes, PKCS11_CKA_INDESTRUCTIBLE)) { | ||
| // Skip deletion for indestructible objects | ||
| LIST_REMOVE(obj, link); |
There was a problem hiding this comment.
You need a bit more. obj volatile resources should be freed.
I would suggest to use cleanup_volatile_obj_ref() from object.c.
There was a problem hiding this comment.
The expectation of the implementation is that the INDETSRUCTIBLE objects should persist in the persistent_storage even after reinit. So, we need not call cleanup_volatile_obj_ref().
ta/pkcs11/src/pkcs11_attributes.c
Outdated
| * Check if the indestructible attribute of a to-be-created | ||
| * object is present only if it is a token object | ||
| */ | ||
| enum pkcs11_rc check_indestructible_access(struct pkcs11_session *session, |
There was a problem hiding this comment.
This sanity test could rather be part of check_attrs_misc_integrity().
I think an object with this attribute should also have attribute CKA_DESTROYABLE to false, for consistency. This should be also checked in check_attrs_misc_integrity().
ta/pkcs11/src/object.c
Outdated
|
|
||
| /* Objects with PKCS11_CKA_INDESTRUCTIBLE as true aren't destroyable */ | ||
| if (get_bool(object->attributes, PKCS11_CKA_INDESTRUCTIBLE)) { | ||
| SLogTrace("Object is indestructible"); |
There was a problem hiding this comment.
EMSG(). That said, PKCS11_CKA_DESTROYABLE above should have done the job.
ta/pkcs11/src/object.c
Outdated
| } | ||
|
|
||
| if (!obj->attributes) { | ||
| return PKCS11_CKR_OBJECT_HANDLE_INVALID; |
ta/pkcs11/src/pkcs11_token.c
Outdated
| uint32_t pin_size = 0; | ||
| void *pin = NULL; | ||
| struct pkcs11_object *obj = NULL; | ||
| struct pkcs11_object *tvar = NULL; |
|
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 issue at any time. |
|
Commenting to keep alive. |
The template for INDESTRUCTIBLE objects have CKA_TOKEN=true. Added code to block destroy operations on objects marked CKA_INDESTRUCTIBLE. This ensures token objects flagged as indestructible cannot be deleted or modified by normal operations. The expectation of the implementation of this vendor defined attribute "INDESTRUCTIBLE" is that, the indestructible objects would persist in persistent_storage after reinit or destroy call. Tested by creating an indestructible object in the TA and verifying destroy/modify calls return proper code. Reviewed-by: Neeraj Soni <neersoni@qti.qualcomm.com> Tested on: SM7325 SoC Signed-off-by: Saheli Saha <sahesaha@qti.qualcomm.com>
sahesaha
force-pushed
the
Indestructible_token-objects
branch
from
3f99281 to
2fdb17c
Compare
|
@etienne-lms, please review the latest changes. The comments have been addressed. |
5 participants
Add code to block destroy operations on objects
marked CKA_INDESTRUCTIBLE. This ensures token objects flagged as indestructible cannot be deleted or modified by normal operations.
Tested by creating an indestructible object in the TA and verifying destroy/modify calls return proper code.
Reviewed-by: Neeraj Soni neersoni@qti.qualcomm.com
Tested on: SM7325 SoC
Signed-off-by: Saheli Saha sahesaha@qti.qualcomm.com