acipher: add dynamic algorithm selection and decryption support #133

Akshay-Belsare · 2025-08-25T11:51:52Z

Extend acipher example to support for RSA algorithms:
- TEE_ALG_RSAES_PKCS1_V1_5
- TEE_ALG_RSAES_PKCS1_OAEP_MGF1_SHA1/224/256/384/512
Users can select algorithm and key size at runtime via: optee_example_acipher <key_size> <string> <algo>
Supported key sizes: 2048, 3072, 4096 bits
Defaults to TA_ALG_PKCS1_V1_5 if no algorithm is specified
Enhances the acipher example for flexible testing of multiple RSA modes

- Extend acipher example to support for RSA algorithms: - TEE_ALG_RSAES_PKCS1_V1_5 - TEE_ALG_RSAES_PKCS1_OAEP_MGF1_SHA1/224/256/384/512 - Users can select algorithm and key size at runtime via: `optee_example_acipher <key_size> <string> <algo>` - Supported key sizes: 2048, 3072, 4096 bits - Defaults to TA_ALG_PKCS1_V1_5 if no algorithm is specified - Enhances the acipher example for flexible testing of multiple RSA modes Signed-off-by: Amey Avinash Raghatate <ameyavinash.raghatate@amd.com>

acipher/host/main.c

Signed-off-by: Amey Avinash Raghatate <ameyavinash.raghatate@amd.com>

- Extend acipher example to support for RSA algorithms: - TEE_ALG_RSAES_PKCS1_V1_5 - TEE_ALG_RSAES_PKCS1_OAEP_MGF1_SHA1/224/256/384/512 - Users can select algorithm and key size at runtime via: `optee_example_acipher <key_size> <string> <algo>` - Supported key sizes: 2048, 3072, 4096 bits - Defaults to TA_ALG_PKCS1_V1_5 if no algorithm is specified - Enhances the acipher example for flexible testing of multiple RSA modes Signed-off-by: Amey Avinash Raghatate <ameyavinash.raghatate@amd.com> State: waiting Link: linaro-swg#133

github-actions · 2025-10-13T02:13:17Z

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.

Akshay-Belsare · 2025-10-13T09:09:18Z

@jforissier @etienne-lms Could you please take a moment to review this change?

jforissier · 2025-10-21T09:45:55Z

acipher/host/main.c

+					 TEEC_VALUE_INPUT, TEEC_VALUE_INPUT);
+	op.params[0].tmpref.buffer = outbuf;
+	op.params[0].tmpref.size = outbuf_len;
+	op.params[2].value.a = DECRYPT; /* decrypt */


Having a command called TA_ACIPHER_CMD_ENCRYPT to do decryption is inconsistent. Please introduce TA_ACIPHER_CMD_DECRYPT instead.

jforissier · 2025-10-21T09:46:07Z

acipher/host/main.c

+
+	res = TEEC_InvokeCommand(&sess, TA_ACIPHER_CMD_ENCRYPT, &op, &eo);
+	if (eo != TEEC_ORIGIN_TRUSTED_APP || res != TEEC_ERROR_SHORT_BUFFER)
+		teec_err(res, eo, "TEEC_InvokeCommand(TA_ACIPHER_CMD_DYCRYPT)");


Actually the command is still TA_ACIPHER_CMD_ENCRYPT.
Maybe update the message here and below (line 191):

teec_err(res, eo, "Command TA_ACIPHER_CMD_ENCRYPT failed for decryption");

and at line 159 above:

res = TEEC_InvokeCommand(&sess, TA_ACIPHER_CMD_ENCRYPT, &op, &eo); if (res) - teec_err(res, eo, "TEEC_InvokeCommand(TA_ACIPHER_CMD_ENCRYPT)"); + teec_err(res, eo, "Command TA_ACIPHER_CMD_ENCRYPT failed for encryption");

Alternatively, introduce a dedicated command for decryption: TA_ACIPHER_CMD_DECRYPT.

jforissier · 2025-10-21T09:46:57Z

acipher/host/main.c

+
+	res = TEEC_InvokeCommand(&sess, TA_ACIPHER_CMD_ENCRYPT, &op, &eo);
+	if (res)
+		teec_err(res, eo, "TEEC_InvokeCommand(TA_ACIPHER_CMD_DYCRYPT)");


jforissier · 2025-10-21T09:51:43Z

acipher/ta/include/acipher_ta.h

 /*
 * in	params[1].memref  input
 * out	params[2].memref  output
 */


Need to add the algo: in params[3].value algorithm (TA_ALG_*)

Actually:

/* * in params[0].memref Input data to cipher * out params[1].memref Ciphered output data * in params[2].value.a Mode: 0 for decryption, any other value for encryption * in params[3].value.a Algorithm (TA_ALG_*) */

etienne-lms · 2025-10-21T17:45:27Z

acipher/host/main.c

+	printf("TA_ALG_OAEP_MGF1_SHA224\n");
+	printf("TA_ALG_OAEP_MGF1_SHA256\n");
+	printf("TA_ALG_OAEP_MGF1_SHA384\n");
+	printf("TA_ALG_OAEP_MGF1_SHA512\n");


Some indentation would make this more readable.
For consistency: fprintf(stderr, ...) for each trace message?

fprintf(stderr, "Usage: %s <key_size> <string to encrypt> [<algo name>]\n\n", pname); fprintf(stderr, "<key_size>: key size in bits. Supported values are:\n"); fprintf(stderr, " 2048, 3072, 4096\n"); fprintf(stderr, "<algo_name>: algorithm name. Supported values are:\n"); fprintf(stderr, " TA_ALG_PKCS1_V1_5 (default)\n"); fprintf(stderr, " TA_ALG_OAEP_MGF1_SHA1\n"); fprintf(stderr, " TA_ALG_OAEP_MGF1_SHA224\n"); fprintf(stderr, " TA_ALG_OAEP_MGF1_SHA256\n"); fprintf(stderr, " TA_ALG_OAEP_MGF1_SHA384\n"); fprintf(stderr, " TA_ALG_OAEP_MGF1_SHA512\n");

etienne-lms · 2025-10-21T17:47:17Z

acipher/host/main.c

+	if ((argc > 4) || (argc < 3)) {
 		warnx("Unexpected number of arguments %d (expected 2)",
 		      argc - 1);


Suggested change

if ((argc > 4) || (argc < 3)) {

warnx("Unexpected number of arguments %d (expected 2)",

argc - 1);

if ((argc > 4) || (argc < 3)) {

warnx("Unexpected number of arguments %d", argc - 1);

etienne-lms · 2025-10-21T17:48:23Z

acipher/host/main.c

+		} else if (strcmp(algo, "TA_ALG_PKCS1_V1_5") == 0) {
+			*algo_num = TA_ALG_PKCS1_V1_5;
+		} else {
+			printf("%s algo is invalid\n", algo);


For consistency:

Suggested change

printf("%s algo is invalid\n", algo);

fprintf(stderr, "%s algo is invalid\n", algo);

etienne-lms · 2025-10-21T17:53:49Z

acipher/host/main.c

+
+	res = TEEC_InvokeCommand(&sess, TA_ACIPHER_CMD_ENCRYPT, &op, &eo);
+	if (eo != TEEC_ORIGIN_TRUSTED_APP || res != TEEC_ERROR_SHORT_BUFFER)
+		teec_err(res, eo, "TEEC_InvokeCommand(TA_ACIPHER_CMD_DYCRYPT)");


Actually the command is still TA_ACIPHER_CMD_ENCRYPT.
Maybe update the message here and below (line 191):

teec_err(res, eo, "Command TA_ACIPHER_CMD_ENCRYPT failed for decryption");

and at line 159 above:

res = TEEC_InvokeCommand(&sess, TA_ACIPHER_CMD_ENCRYPT, &op, &eo); if (res) - teec_err(res, eo, "TEEC_InvokeCommand(TA_ACIPHER_CMD_ENCRYPT)"); + teec_err(res, eo, "Command TA_ACIPHER_CMD_ENCRYPT failed for encryption");

Alternatively, introduce a dedicated command for decryption: TA_ACIPHER_CMD_DECRYPT.

etienne-lms · 2025-10-21T17:55:27Z

acipher/ta/acipher_ta.c

+		*algo = TEE_ALG_RSAES_PKCS1_OAEP_MGF1_SHA512;
+		return TEE_SUCCESS;
+	default:
+		EMSG("Invalid algo %u", param);


Suggested change

EMSG("Invalid algo %u", param);

EMSG("Invalid algo %"PRIu32, param);

etienne-lms · 2025-10-21T17:57:23Z

acipher/ta/acipher_ta.c

 				    key_info.keySize);
 	if (res) {
-		EMSG("TEE_AllocateOperation(TEE_MODE_ENCRYPT, %#" PRIx32 ", %" PRId32 "): %#" PRIx32, alg, key_info.keySize, res);
+		EMSG("TEE_AllocateOperation(TEE_MODE_ENCRYPT, %#"


TEE_MODE_ENCRYPT or TEE_MODE_DECRYPT depending on encrypt.

etienne-lms · 2025-10-21T17:59:11Z

acipher/ta/acipher_ta.c

 	if (res) {
-		EMSG("TEE_AllocateOperation(TEE_MODE_ENCRYPT, %#" PRIx32 ", %" PRId32 "): %#" PRIx32, alg, key_info.keySize, res);
+		EMSG("TEE_AllocateOperation(TEE_MODE_ENCRYPT, %#"
+		     PRIx32 ", %" PRId32 "): %#" PRIx32,


The show argument do not match the order used in TEE_AllocateOperation(). Not a big issue but one may found that confusing.

etienne-lms · 2025-10-21T18:03:52Z

acipher/ta/include/acipher_ta.h

 /*
 * in	params[1].memref  input
 * out	params[2].memref  output
 */


Actually:

/* * in params[0].memref Input data to cipher * out params[1].memref Ciphered output data * in params[2].value.a Mode: 0 for decryption, any other value for encryption * in params[3].value.a Algorithm (TA_ALG_*) */

etienne-lms · 2025-10-21T18:04:47Z

acipher/ta/include/acipher_ta.h

+#define TA_ALG_OAEP_MGF1_SHA256		3
+#define TA_ALG_OAEP_MGF1_SHA384		4
+#define TA_ALG_OAEP_MGF1_SHA512		5
+


Maybe use GP TEE Internal Core API algorithm IDs? as suggestion by @jenswi-linaro in another P-R?
Nitpicking: could you remove the extra empty line.

Update acipher TA and host as per review comments. Signed-off-by: Amey Avinash Raghatate <ameyavinash.raghatate@amd.com>

jforissier reviewed Sep 4, 2025

View reviewed changes

acipher/host/main.c Show resolved Hide resolved

acipher/host/main.c Outdated Show resolved Hide resolved

review_1: update as per the review comments

1cb7c67

Signed-off-by: Amey Avinash Raghatate <ameyavinash.raghatate@amd.com>

github-actions bot added the Stale label Oct 13, 2025

github-actions bot removed the Stale label Oct 14, 2025

jforissier reviewed Oct 21, 2025

View reviewed changes

etienne-lms reviewed Oct 21, 2025

View reviewed changes

[REVIEW] update acipher TA and host as per review

bd48ca5

Update acipher TA and host as per review comments. Signed-off-by: Amey Avinash Raghatate <ameyavinash.raghatate@amd.com>

	printf("%s algo is invalid\n", algo);
	fprintf(stderr, "%s algo is invalid\n", algo);

	EMSG("Invalid algo %u", param);
	EMSG("Invalid algo %"PRIu32, param);

acipher: add dynamic algorithm selection and decryption support #133

Are you sure you want to change the base?

acipher: add dynamic algorithm selection and decryption support #133

Uh oh!

Conversation

Akshay-Belsare commented Aug 25, 2025

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 13, 2025

Uh oh!

Akshay-Belsare commented Oct 13, 2025

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants