Skip to content

Add Application Secrets TEE client library#413

Open
tusal-vaisala wants to merge 1 commit into
OP-TEE:masterfrom
tusal-vaisala:appsecrets-upstream-pr
Open

Add Application Secrets TEE client library#413
tusal-vaisala wants to merge 1 commit into
OP-TEE:masterfrom
tusal-vaisala:appsecrets-upstream-pr

Conversation

@tusal-vaisala
Copy link
Copy Markdown

Related optee_os PR: OP-TEE/optee_os#7769

Addresses issue: OP-TEE/optee_os#7768

Copy link
Copy Markdown
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tusal-vaisala,

Thanks for the PR. This looks like a useful feature and easy to use. Please see my comments below.

Comment thread libasteec/src/asteec.c Outdated
TEEC_Operation op = { 0 };
TEEC_Result res = TEEC_ERROR_GENERIC;

if (!plain || plain_len == 0 || !sealed_len)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: !plain_len

To implement the contract in the proposed API documentation update above, we would need to add:

if (!sealed && *sealed_len)
	return TEEC_ERROR_BAD_PARAMETERS;

Comment thread libasteec/src/asteec.c Outdated
Comment on lines +89 to +90
if (!sealed || !plain_len)
return TEEC_ERROR_BAD_PARAMETERS;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the proposed API update and for consistency with asteec_seal():

Suggested change
if (!sealed || !plain_len)
return TEEC_ERROR_BAD_PARAMETERS;
if (!sealed || !sealed_len || !plain_len)
return TEEC_ERROR_BAD_PARAMETERS;
if (!plain && *plain_len)
return TEEC_ERROR_BAD_PARAMETERS;

Comment thread libasteec/CMakeLists.txt
VERSION 1.0.0
LANGUAGES C
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest making the build of this library optional like for libteecacl:

option(WITH_ASTEEC "Build libasteec" TRUE)

Comment thread libasteec/include/asteec.h Outdated
Comment on lines +26 to +27
* @param sealed Pointer to buffer to receive sealed secret datablob
* @param sealed_len Byte length of buffer @sealed, updated with actual size
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend being explicit bout output size querying:

Suggested change
* @param sealed Pointer to buffer to receive sealed secret datablob
* @param sealed_len Byte length of buffer @sealed, updated with actual size
* @param sealed Pointer to buffer to receive sealed secret datablob.
* May be NULL when *sealed_len is 0 to query the
* required output size.
* @param sealed_len On input, byte length of buffer @sealed. On output,
* updated with the actual size on success or the required
* size when TEEC_ERROR_SHORT_BUFFER is returned.

I have not checked if this would require changes in the TA implementation though

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! Added a fixup with all proposed changes. The TA already supports querying the output buffer size, so no changes needed there regarding this.

Comment thread libasteec/include/asteec.h Outdated
Comment on lines +43 to +44
* @param plain Pointer to buffer to receive plain secret
* @param plain_len Byte length of buffer @plain, updated with actual size
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here:

Suggested change
* @param plain Pointer to buffer to receive plain secret
* @param plain_len Byte length of buffer @plain, updated with actual size
* @param plain Pointer to buffer to receive plain secret.
* May be NULL when *plain_len is 0 to query the
* required output size.
* @param plain_len On input, byte length of buffer @plain. On output,
* updated with the actual size on success or the required
* size when TEEC_ERROR_SHORT_BUFFER is returned.

@jforissier
Copy link
Copy Markdown
Contributor

Reviewed-by: Jerome Forissier <jerome.forissier@arm.com>

for accessing Application Secrets TA

Co-developed-by: Katariina Lounento <katariina.lounento@vaisala.com>
Signed-off-by: Katariina Lounento <katariina.lounento@vaisala.com>
Co-developed-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com>
Signed-off-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com>
Signed-off-by: Tuomas Salokanto <tuomas.salokanto@vaisala.com>
Reviewed-by: Jerome Forissier <jerome.forissier@arm.com>
@tusal-vaisala tusal-vaisala force-pushed the appsecrets-upstream-pr branch from d7821b2 to 095fd4a Compare April 22, 2026 05:51
@github-actions
Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the Stale label May 23, 2026
@tusal-vaisala
Copy link
Copy Markdown
Author

keep-alive

@github-actions github-actions Bot removed the Stale label May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants