Added Indestructible token objects attributes/keys feature. by sahesaha · Pull Request #7581 · OP-TEE/optee_os

sahesaha · 2025-11-03T04:37:14Z

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

jforissier · 2025-11-03T08:53:41Z

Hi @sahesaha, there is a mistake here, you added a patch file instead of modifying the source files themselves.

sahesaha · 2025-11-03T09:15:04Z

Hi @jforissier, could you please check now? I have removed the patch file from the commit.

Emantor · 2025-11-03T11:38:45Z

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 · 2025-11-03T13:14:07Z

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

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).

etienne-lms · 2025-11-06T06:48:59Z

 						  0x0600,

 	/* Vendor specific attributes */
+	PKCS11_CKA_INDESTRUCTIBLE = 0x80000010,


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.

Suggested change

PKCS11_CKA_INDESTRUCTIBLE = 0x80000010,

PKCS11_CKA_OPTEE_INDESTRUCTIBLE = PKCS11_CKA_VENDOR_DEFINED |

0x0001,

etienne-lms · 2025-11-06T06:51:45Z

 		/* Try twice otherwise panic! */
 		if (unregister_persistent_object(token, obj->uuid) &&
-		    unregister_persistent_object(token, obj->uuid))
+			unregister_persistent_object(token, obj->uuid))


Discard this change. Previous indentation was fine.

etienne-lms · 2025-11-06T07:05:49Z

+
+		if (get_bool(obj->attributes, PKCS11_CKA_INDESTRUCTIBLE)) {
+			// Skip deletion for indestructible objects
+			LIST_REMOVE(obj, link);


You need a bit more. obj volatile resources should be freed.
I would suggest to use cleanup_volatile_obj_ref() from object.c.

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().

After the token in re-initialized, the object would still remain loaded in memory.
Calling cleanup_volatile_obj_ref() would release RAM from object resources but the object would still remain in the persistent database and be re-loaded only once it's used.

etienne-lms · 2025-11-06T07:11:14Z

+ * 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,


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().

etienne-lms · 2025-11-06T07:22:53Z


+	/* Objects with PKCS11_CKA_INDESTRUCTIBLE as true aren't destroyable */
+	if (get_bool(object->attributes, PKCS11_CKA_INDESTRUCTIBLE)) {
+		SLogTrace("Object is indestructible");


EMSG(). That said, PKCS11_CKA_DESTROYABLE above should have done the job.

etienne-lms · 2025-11-06T07:23:49Z

 	}

+	if (!obj->attributes) {
+		return PKCS11_CKR_OBJECT_HANDLE_INVALID;


Is this really needed?

etienne-lms · 2025-11-06T07:24:46Z

 	uint32_t pin_size = 0;
 	void *pin = NULL;
 	struct pkcs11_object *obj = NULL;
+	struct pkcs11_object *tvar = NULL;


github-actions · 2025-12-07T02:27:41Z

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.

sahesaha · 2025-12-08T06:30:50Z

Commenting to keep alive.

sahesaha · 2025-12-13T20:49:26Z

@etienne-lms, please review the latest changes. The comments have been addressed.

etienne-lms

Sorry for the delay. Please find my review comments together with, again, my apologies.

etienne-lms · 2026-01-08T10:32:59Z

+		DMSG("CKA_INDESTRUCTIBLE not present in template");
+	} else {
+		/* Attribute exists, get its value (TRUE or FALSE) */
+		bool indestructible = get_bool(temp, PKCS11_CKA_OPTEE_INDESTRUCTIBLE);


Minor: could you add an empty line below this local variable definition?

etienne-lms · 2026-01-08T10:33:55Z

+
+		/* Add attribute with the actual value from template */
+		rc = add_attribute(&attrs, PKCS11_CKA_OPTEE_INDESTRUCTIBLE,
+						&indestructible, sizeof(indestructible));


Nitpicking on indentation:

Suggested change

&indestructible, sizeof(indestructible));

&indestructible, sizeof(indestructible));

etienne-lms · 2026-01-08T10:43:35Z

+	enum pkcs11_rc rc_check = get_attribute_ptr(temp, PKCS11_CKA_OPTEE_INDESTRUCTIBLE, NULL, NULL);
+
+	if (rc_check == PKCS11_RV_NOT_FOUND) {
+		DMSG("CKA_INDESTRUCTIBLE not present in template");


Would you be ok to discard this debug message?

etienne-lms · 2026-01-08T10:50:32Z

+	/*
+ 	* Check if the indestructible attribute of a to-be-created
+ 	* object is present only if it is a token object
+	*/


The test actually checks if the boolean attribute is set, not present (allowed to be present and disabled), which looks fine to me.

Suggested change

/*

* Check if the indestructible attribute of a to-be-created

* object is present only if it is a token object

*/

/* Only token objects can be indestructible */

etienne-lms · 2026-01-08T10:52:58Z

 		[BPA_DESTROYABLE]	= PKCS11_CKA_DESTROYABLE,
 		[BPA_ALWAYS_AUTHENTICATE] = PKCS11_CKA_ALWAYS_AUTHENTICATE,
 		[BPA_WRAP_WITH_TRUSTED] = PKCS11_CKA_WRAP_WITH_TRUSTED,
+		[BPA_INDESTRUCTIBLE] = PKCS11_CKA_OPTEE_INDESTRUCTIBLE,


Nitpicking, for consistency with indentaion above:

Suggested change

[BPA_INDESTRUCTIBLE] = PKCS11_CKA_OPTEE_INDESTRUCTIBLE,

[BPA_INDESTRUCTIBLE] = PKCS11_CKA_OPTEE_INDESTRUCTIBLE,

etienne-lms · 2026-01-08T10:59:14Z

+	/* Ensure that only token objects can be indestructible */
+	rc = check_attrs_misc_integrity(head);
+	if (rc)
+		goto out;
+


Can be removed, this is already addressed by the calls to check_created_attrs_against_token() above.

etienne-lms · 2026-01-08T11:00:04Z

 enum pkcs11_rc check_created_attrs(struct obj_attrs *key1,
 				   struct obj_attrs *key2);

+enum pkcs11_rc check_attrs_misc_integrity(struct obj_attrs *head);


I think this helper function can remain local to pkcs11_atrtibutes.c.

etienne-lms · 2026-01-08T11:11:49Z

+
+		if (get_bool(obj->attributes, PKCS11_CKA_INDESTRUCTIBLE)) {
+			// Skip deletion for indestructible objects
+			LIST_REMOVE(obj, link);


After the token in re-initialized, the object would still remain loaded in memory.
Calling cleanup_volatile_obj_ref() would release RAM from object resources but the object would still remain in the persistent database and be re-loaded only once it's used.

etienne-lms · 2026-01-08T11:14:29Z

 			TEE_Panic(0);

 		cleanup_persistent_object(obj, token);
+		token_invalidate_object_handles(obj);


cleanup_persistent_object() frees obj pointer so token_invalidate_object_handles() here will not do anything.

I retained token_invalidate_object_handles(obj) because cleanup_persistent_object() only frees the object memory and does not invalidate the open handles associated with it as I tested some pointers were still remaining after cleanup-persistent_object() call. I was following the sequence observed in the destroy_object definition where both calls are present.

etienne-lms · 2026-01-08T11:21:28Z

 		goto out;

+	/* Ensure that only token objects can be indestructible */
+	rc = check_attrs_misc_integrity(head);


Can be removed, this is already addressed by the calls to check_created_attrs_against_token() above.

etienne-lms · 2026-01-08T11:27:05Z

I'd prefer you also introduce a config switch to enable this feature. Some PKCS#11 token may not expect a user can create such indestructible objects. Something like CFG_PKCS11_TA_INDESTRUCTIBLE_OBJECT_ATTR=y|n.

sahesaha · 2026-01-09T10:28:37Z

Hi @etienne-lms, regarding your concern of config switch and controlled compilation, I understand the concern about the persistent database growing if these objects are created unchecked. However, I believe a compile-time switch (CFG_PKCS11_TA_TEST_...) is not the best approach for a few reasons:

Production Utility: Indestructible objects are not just for testing; they are needed for factory-provisioned items (like device attestation keys) that should persist for the device's lifetime and be protected from accidental deletion by the rich OS. A test-only switch would disable this valid use case.
Opt-In Safety: The feature is completely dormant unless the CKA_INDESTRUCTIBLE attribute is explicitly set. Standard applications won't accidentally trigger this.

Also, the rest of the comments are addressed in the latest commit. Please let me know your reviews.

etienne-lms · 2026-01-09T17:18:23Z

However, I believe a compile-time switch (CFG_PKCS11_TA_TEST_...) is not the best approach for a few reasons:

In OP-TEE OS, I propose to add a config switch only to enable (or not) indestructible object support in the pkcs11 TA:
CFG_PKCS11_TA_INDESTRUCTIBLE_OBJECT_ATTR.
I think it should be default disabled, but you are free to default enable it for your platform and/or from your product build environment.

In OP-TEE Test P-R (OP-TEE/optee_test#803) I proposed another switch, to embedded or the tests that creates these indestructible objects.
CFG_PKCS11_TA_TEST_INDESTRUCTIBLE_OBJECT_ATTR. (maybe another name would be better :-)
Thinking twice, I guest such switch could be useless if xtest was not creating its indestructible test object if it find it's always the test could simple try to destroy it and check it can't. But let's discuss this in P-R OP-TEE/optee_test#803.

By the way, please update the commit message header to start with ta: pkcs11: , e.g.:
ta: pkcs11: add indestructible objects attribute.

Added Indestructible token objects attributes/keys feature. 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 · 2026-01-14T06:56:31Z

Hi @etienne-lms, I have added the config switch and controlled the compilation of Indestructible attribute PR code as asked. Let me know if you have any concerns.

sahesaha · 2026-02-11T08:34:02Z

Commenting to keep alive.

@etienne-lms, could you please review the latest commit?

github-actions · 2026-03-14T02:49:11Z

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.

sahesaha · 2026-03-19T10:04:33Z

Commenting to keep alive.

github-actions · 2026-04-19T03:47:56Z

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.

sahesaha · 2026-04-24T05:10:03Z

Commenting to keep alive.

@etienne-lms, could you review this?

github-actions · 2026-05-25T04:48:15Z

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.

sahesaha · 2026-05-25T05:43:17Z

Commenting to keep alive.

github-actions · 2026-06-26T04:37:21Z

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.

sahesaha · 2026-06-29T10:56:08Z

Commenting to keep alive.

github-actions · 2026-07-31T03:46:43Z

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.

etienne-lms · 2026-07-31T15:06:52Z

keep alive

github-actions · 2026-08-31T05:28:29Z

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.

ldts · 2026-09-01T20:31:22Z

 	enum pkcs11_rc rc = PKCS11_CKR_OK;
 	uint8_t local = 0;
+	#if defined(CFG_PKCS11_TA_INDESTRUCTIBLE_OBJECT_ATTR)
+	uint8_t indestructible = 0;


Build break: unused variable. This outer indestructible is never read or written — the inner block below (else branch) declares and uses its own bool indestructible. With -Wunused-variable and -Werror (both on in mk/compile.mk), this fails to compile when CFG_PKCS11_TA_INDESTRUCTIBLE_OBJECT_ATTR=y.

ldts · 2026-09-01T20:31:23Z

 		goto out;

+	#if defined(CFG_PKCS11_TA_INDESTRUCTIBLE_OBJECT_ATTR)
+	/* Check if CKA_INDESTRUCTIBLE exists in the template */


Build break: declaration-after-statement + shadowing. enum pkcs11_rc rc_check = … is a declaration placed after statements in the function body, and the bool indestructible a few lines down shadows the function-scope indestructible declared at the top. OP-TEE builds the pkcs11 TA with -Wdeclaration-after-statement, -Wshadow, and -Werror, so this won't compile. Suggest declaring rc_check at block start and removing the now-redundant outer variable.

ldts · 2026-09-01T20:31:44Z

+
+		if (get_bool(obj->attributes, PKCS11_CKA_OPTEE_INDESTRUCTIBLE)) {
+			// Skip deletion for indestructible objects
+			LIST_REMOVE(obj, link);


Could you please confirm whether this leaks: indestructible objects are unlinked from token->object_list via LIST_REMOVE(obj, link) + continue, but the struct pkcs11_object (and its attributes/uuid, plus any loaded attributes handle) is never freed — the normal cleanup_volatile_obj_ref() path is skipped. If so, each C_InitToken on a token holding indestructible objects leaks them.

jenswikl · 2026-09-02T09:50:32Z

Please rebase the patch.

sahesaha force-pushed the Indestructible_token-objects branch from e39c3e5 to fcd2203 Compare November 3, 2025 09:12

sahesaha force-pushed the Indestructible_token-objects branch from fcd2203 to 3f99281 Compare November 3, 2025 13:10

etienne-lms reviewed Nov 6, 2025

View reviewed changes

github-actions Bot added the Stale label Dec 7, 2025

jenswikl removed the Stale label Dec 8, 2025

sahesaha force-pushed the Indestructible_token-objects branch from 3f99281 to 2fdb17c Compare December 13, 2025 20:45

etienne-lms reviewed Jan 8, 2026

View reviewed changes

sahesaha force-pushed the Indestructible_token-objects branch 3 times, most recently from de084d3 to d15ecdf Compare January 9, 2026 10:26

sahesaha force-pushed the Indestructible_token-objects branch from d15ecdf to b88a9e4 Compare January 9, 2026 10:34

sahesaha force-pushed the Indestructible_token-objects branch from b88a9e4 to 5af3c86 Compare January 14, 2026 06:53

github-actions Bot added the Stale label Mar 14, 2026

jenswikl removed the Stale label Mar 16, 2026

github-actions Bot added the Stale label Apr 19, 2026

github-actions Bot removed the Stale label Apr 25, 2026

github-actions Bot added the Stale label May 25, 2026

github-actions Bot removed the Stale label May 26, 2026

github-actions Bot added the Stale label Jun 26, 2026

github-actions Bot removed the Stale label Jun 30, 2026

github-actions Bot added the Stale label Jul 31, 2026

github-actions Bot removed the Stale label Aug 1, 2026

github-actions Bot added the Stale label Aug 31, 2026

ldts reviewed Sep 1, 2026

View reviewed changes

github-actions Bot removed the Stale label Sep 2, 2026

	PKCS11_CKA_INDESTRUCTIBLE = 0x80000010,
	PKCS11_CKA_OPTEE_INDESTRUCTIBLE = PKCS11_CKA_VENDOR_DEFINED \|
	0x0001,

	&indestructible, sizeof(indestructible));
	&indestructible, sizeof(indestructible));

	[BPA_INDESTRUCTIBLE] = PKCS11_CKA_OPTEE_INDESTRUCTIBLE,
	[BPA_INDESTRUCTIBLE] = PKCS11_CKA_OPTEE_INDESTRUCTIBLE,

Uh oh!

Conversation

sahesaha commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jforissier commented Nov 3, 2025

Uh oh!

sahesaha commented Nov 3, 2025

Uh oh!

Emantor commented Nov 3, 2025

Uh oh!

sahesaha commented Nov 3, 2025

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Dec 7, 2025

Uh oh!

sahesaha commented Dec 8, 2025

Uh oh!

sahesaha commented Dec 13, 2025

Uh oh!

etienne-lms left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etienne-lms commented Jan 8, 2026

Uh oh!

sahesaha commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

etienne-lms commented Jan 9, 2026

Uh oh!

sahesaha commented Jan 14, 2026

Uh oh!

sahesaha commented Feb 11, 2026

Uh oh!

github-actions Bot commented Mar 14, 2026

Uh oh!

sahesaha commented Mar 19, 2026

Uh oh!

sahesaha commented Nov 3, 2025 •

edited

Loading

sahesaha commented Jan 9, 2026 •

edited

Loading