libptateec: manufacturing protection PTA #352

ldts · 2023-08-24T10:01:43Z

This abstraction provides TEEC access to stable PTAs present in the OP-TEE upstream tree.

The first of these miscellaenous PTAs to be integrated in the library is the iMX Manufacturing Protection [1] for which two functions are provided:

Retrieval of the EC Public Key.
Signature generation.

In order to access the service provided by the manufacturing protection PTA, OP-TEE needs to be built with this option OP-TEE/optee_os#6274

[1] AN13676, i.MX RT1170 Manufacturing Protection

This option allows lmp-device-register to interrogate the CAAM hardware for an EC public key (256 bits usually) The key will be stored in /var/sota in PEM format (for reference) and passed to the gateway in DER format. Afer registration aktualizer-lite should periodically request the CAAM hardware to sign random strings and send these signatures to the gateway along with the message. See [1] for optee-client reference. Upon receiving these digests, the gateway shall verify them using the board associated public key [1] OP-TEE/optee_client#352 Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>

This option allows lmp-device-register to interrogate the CAAM hardware for an EC public key (256 bits usually) The key will be stored in /var/sota in PEM format (for reference) and passed to the gateway. Afer registration aktualizer-lite should periodically request the CAAM hardware to sign random strings and send these signatures to the gateway along with the message. See [1] for optee-client reference. Upon receiving these digests, the gateway shall verify them using the board associated public key [1] OP-TEE/optee_client#352 Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>

This option allows lmp-device-register to interrogate the CAAM hardware for an EC public key (256 bits usually) The key will be stored in /var/sota in PEM format (for reference) and passed to the gateway in the CSR using a custom extension [1] Afer registration aktualizer-lite should periodically request the CAAM hardware to sign random strings and send these signatures to the gateway along with the message. See [2] for optee-client reference. Upon receiving these digests, the gateway shall verify them using the board associated public key [1] "1.3.6.1.4.1.294.1.00" [2] OP-TEE/optee_client#352 Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>

Allow accessing the PTA without a session. Users could then use the OP-TEE client interface to retrieve the public key as well as to generate signatures. See OP-TEE/optee_client#352 Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>

Allow opening the PTA without a calling session. Enabling CFG_NXP_CAAM_MP_NO_ACCESS_CTR permits users to use the OP-TEE client interface to retrieve the public key as well as to generate signatures. See OP-TEE/optee_client#352 Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>

Allow opening the PTA without a calling session. Enabling CFG_NXP_CAAM_MP_NO_ACCESS_CTRL permits users to use the OP-TEE client interface to retrieve the public key as well as to generate signatures. See OP-TEE/optee_client#352 Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>

Allow opening the PTA without a calling session. Enabling CFG_NXP_CAAM_MP_NO_ACCESS_CTRL permits users to use the OP-TEE client interface to retrieve the public key as well as to generate signatures. See OP-TEE/optee_client#352 Signed-off-by: Jorge Ramirez-Ortiz <[email protected]> Acked-by: Clement Faure <[email protected]>

ldts · 2023-09-05T21:16:24Z

any comments?

jenswi-linaro · 2023-09-06T06:52:41Z

libptateec/include/pta_tee.h

+#endif
+
+typedef unsigned long PTA_ULONG;
+typedef PTA_ULONG PTA_RV;


What's wrong with TEEC_Result?

We dont know and can't foresee what the PTAs will choose to report..errors could have different meanings depending the calls made to the TEE I guess (so the same TEEC_Result could mean something different for different function calls). having another indirection adds flexibility.

Since we're only supporting upstream PTAs here we can encourage them to use TEEC_Result error codes, and if needed pick values in the 0x80000000 – 0x8FFFFFFF range for an occasional extension.

so shall I get rid of the typedef and just cast in case of needing to extend ?

Yes, please.

Allow opening the PTA without a calling session. Enabling CFG_NXP_CAAM_MP_NO_ACCESS_CTRL permits users to use the OP-TEE client interface to retrieve the public key as well as to generate signatures. See OP-TEE/optee_client#352 Signed-off-by: Jorge Ramirez-Ortiz <[email protected]> Acked-by: Clement Faure <[email protected]>

ldts · 2023-09-15T08:22:58Z

actioned.

jenswi-linaro · 2023-09-19T09:47:03Z

libptateec/src/pta_imx_manufacturing_protection.c

+	if (!key || !len || !*len)
+		return TEEC_ERROR_BAD_PARAMETERS;
+
+	if (!pta_open_session(&manufacturing_protection_ta_ctx))


How is the session closed?

um I didnt think it matter to keep an open session for all calls.

The way I thought the user (the process that communicates with the remote server, responding to requests and so on) would access the call is by periodically signing random values provided by the server (thus generating some sort of attestation token)

The key would only be retrieved once and would be sent to the server in a secure environment during provisioning (server = verifier)

Leaving a session open wastes memory so there should be a way to close the session without killing the process.

sure, ok let me address that but it will not be as nice in terms of usability - many threads could be accessing the PTA so with the capability of closing the sessions now locking is a must

Allow opening the PTA without a calling session. Enabling CFG_NXP_CAAM_MP_NO_ACCESS_CTRL permits users to use the OP-TEE client interface to retrieve the public key as well as to generate signatures. See OP-TEE/optee_client#352 Signed-off-by: Jorge Ramirez-Ortiz <[email protected]> Acked-by: Clement Faure <[email protected]>

etienne-lms

LGTM aside the few comments.

etienne-lms · 2023-09-27T07:00:49Z

libptateec/include/pta_tee.h

+ * pta_imx_mprotect_get_key() - Retrieves the iMX CAAM Manufacturing Protection
+ * EC public key. The components x,y are retrieved in RAW format and should
+ * be converted to DER or PEM as required.
+ *


could you describe the arguments?

etienne-lms · 2023-09-27T07:03:01Z

libptateec/src/pta.h

+#include <tee_client_api.h>
+#include <teec_trace.h>
+
+struct ta_context {


Could you also add a description for struct and API functions?

etienne-lms · 2023-09-27T07:06:54Z

libptateec/src/pta_imx_manufacturing_protection.c

+	op.params[1].tmpref.buffer = sig;
+	op.params[1].tmpref.size = *sig_len;
+	op.params[2].tmpref.buffer = mpmr;
+	op.params[2].tmpref.size =  *mpmr_len;


2 space chars

etienne-lms · 2023-09-27T07:09:21Z

libptateec/src/pta_imx_manufacturing_protection.h

+ * [out]	memref[0].buffer	Public key buffer
+ * [out]	memref[0].size		Public key size
+ *
+ * Return codes:


I suggest you remove this list. The command can return more codes.

no I dont think it can. I think we should keep this.

It looks the PTA can also return TEE_ERROR_SHORT_BUFFER, TEE_ERROR_OUT_OF_MEMORY, TEE_ERROR_NOT_SUPPORTED or TEE_ERROR_GENERIC.

etienne-lms · 2023-09-27T07:11:54Z

libptateec/src/pta.h

+ * Copyright (c) 2023, Foundries.io Ltd
+ */
+#ifndef PTA_H
+#define PTA_H


PTA_H seems too short to me for a header file exposed to other numerous libs.
What about PTATEEC_PTA_H

etienne-lms · 2023-09-28T16:46:24Z

libptateec/include/pta_tee.h

+ * requiring at least a 64 byte buffer.
+ *
+ * @key: [out] Public key in RAW format.
+ * @len: [in/out] Key length.


etienne-lms · 2023-09-28T16:49:14Z

libptateec/include/pta_tee.h

+ * @return TEEC_ERROR_GENERIC		Error unspecified.
+ * @return TEEC_ERROR_SHORT_BUFFER	Error small buffer provided.
+ * @return TEEC_ERROR_COMMUNICATION	Some other thread closed the connection.
+ * @return TEEC_SUCCESS			On success.


maybe add TEE_ERROR_OUT_OF_MEMORY
ditto for pta_imx_mprotect_sign().

etienne-lms · 2023-09-28T16:50:12Z

libptateec/include/pta_tee.h

+ * pta_imx_mprotect_final() - Closes the OP-TEE session
+ *
+ * This function may fail with TEEC_ERROR_BUSY if there are unfulfilled calls
+ * pending


etienne-lms · 2023-09-28T16:54:04Z

libptateec/src/pta.c

+	}
+	pthread_mutex_unlock(&ctx->lock);
+	ret = TEEC_InvokeCommand(&ctx->session, cmd_id, op, error_origin);
+	atomic_fetch_sub(&ctx->count, 1);


This means 1 call to pta_open_session() must be balanced by a call to pta_invoke_cmd().

If so, I think there is no need to have 2 functions. A single call to pta_invoke_cmd() should be is sufficient to open the context session and invoke a PTA. It's a bit strange of a PTA API to require a call to open_session for each calls to invoke_command.

Sorry, I think it's a bit weird that a generic PTA invocation API requires each call to pta_open_session() be always followed by a single call to pta_invoke_cmd().

Yeah, this session handling is a bit more complicated than needed with mutex and reference counter. Couldn't we skip it all and rely on the normal TEE Client primitives?

IMO being capable of rejecting a close request coming from some separate thread because an invoke is about to follow is a nice feature to have (it really only costs the mutex and reference counter and it has no impact on performance).

But sure I can remove it if you think it adds too much maintenance noise.

Please remove it then.

etienne-lms · 2023-09-28T17:01:26Z

libptateec/src/pta.h

+
+/**
+ * pta_open_session() - Opens a session with the PTA uuid in the ta_context.
+ * If the session is already open it will increment a session counter.


* @ctx Session context, already opened ornot * Field @ctx->uuid defines the target PTA

etienne-lms · 2023-09-28T17:06:39Z

libptateec/src/pta.h

+/**
+ * pta_invoke_cmd() - Invokes a command in the PTA
+ */
+TEEC_Result pta_invoke_cmd(struct ta_context *ctx, uint32_t cmd_id,


* @ctx Opened session context * @cmd_id Command passed to target PTA * @operation TEE operation arguments passed to target PTA * @error_origin: Output TEE_ORIGIN_* emitter of the result code

This abstraction provides TEEC access to stable PTAs present in the OP-TEE upstream tree. The first of these miscellaenous PTAs to be integrated in the library is the iMX Manufacturing Protection [1] for which two functions are provided: - Retrieval of the EC Public Key. - Signature generation. [1] AN13676, i.MX RT1170 Manufacturing Protection Signed-off-by: Jorge Ramirez-Ortiz <[email protected]>

ldts · 2023-10-03T15:50:08Z

@etienne-lms @jenswi-linaro anything else?

github-actions · 2023-11-04T01:24:25Z

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.

jenswi-linaro · 2023-11-08T08:38:01Z

I guess this wasn't needed after all.

ldts mentioned this pull request Aug 24, 2023

RFC: pta: imx: manufacturing_protection: should we add an optee client interface OP-TEE/optee_os#6241

Closed

ldts force-pushed the libptateec-up branch 2 times, most recently from b13f83f to 25cb412 Compare August 24, 2023 10:50

ldts mentioned this pull request Aug 24, 2023

options: tee: manufacturing protection foundriesio/lmp-device-register#57

Closed

ldts force-pushed the libptateec-up branch from 25cb412 to f462f69 Compare August 24, 2023 16:29

ldts mentioned this pull request Aug 28, 2023

regression: creating an EC Public key using affine coordinates and non-deprecated calls openssl/openssl#21871

Closed

ldts mentioned this pull request Aug 31, 2023

core: pta: imx: disable access control for MP PTA OP-TEE/optee_os#6274

Merged

jenswi-linaro reviewed Sep 6, 2023

View reviewed changes

ldts force-pushed the libptateec-up branch 3 times, most recently from f5a7252 to 33d9c0e Compare September 15, 2023 08:19

jenswi-linaro reviewed Sep 19, 2023

View reviewed changes

ldts force-pushed the libptateec-up branch from 33d9c0e to 77ddfcb Compare September 27, 2023 06:17

etienne-lms reviewed Sep 27, 2023

View reviewed changes

ldts force-pushed the libptateec-up branch from 77ddfcb to 59754c1 Compare September 28, 2023 16:08

etienne-lms reviewed Sep 28, 2023

View reviewed changes

ldts force-pushed the libptateec-up branch from 59754c1 to cc66158 Compare September 29, 2023 06:49

github-actions bot added the Stale label Nov 4, 2023

ldts closed this Nov 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

libptateec: manufacturing protection PTA #352

libptateec: manufacturing protection PTA #352

ldts commented Aug 24, 2023 •

edited

Loading

ldts commented Sep 5, 2023

jenswi-linaro Sep 6, 2023

ldts Sep 6, 2023 •

edited

Loading

jenswi-linaro Sep 6, 2023

ldts Sep 6, 2023

jenswi-linaro Sep 6, 2023

ldts commented Sep 15, 2023

jenswi-linaro Sep 19, 2023

ldts Sep 21, 2023 •

edited

Loading

jenswi-linaro Sep 21, 2023

ldts Sep 22, 2023 •

edited

Loading

etienne-lms left a comment

etienne-lms Sep 27, 2023

etienne-lms Sep 27, 2023

etienne-lms Sep 27, 2023

etienne-lms Sep 27, 2023

ldts Sep 28, 2023

etienne-lms Sep 29, 2023

etienne-lms Sep 27, 2023

etienne-lms Sep 28, 2023

etienne-lms Sep 28, 2023

etienne-lms Sep 28, 2023

etienne-lms Sep 28, 2023

etienne-lms Sep 29, 2023

etienne-lms Oct 3, 2023

jenswi-linaro Oct 4, 2023

ldts Oct 4, 2023

jenswi-linaro Oct 4, 2023

etienne-lms Sep 28, 2023

etienne-lms Sep 28, 2023

ldts Sep 29, 2023

ldts commented Oct 3, 2023

github-actions bot commented Nov 4, 2023

jenswi-linaro commented Nov 8, 2023

libptateec: manufacturing protection PTA #352

libptateec: manufacturing protection PTA #352

Conversation

ldts commented Aug 24, 2023 • edited Loading

ldts commented Sep 5, 2023

Choose a reason for hiding this comment

ldts Sep 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ldts commented Sep 15, 2023

Choose a reason for hiding this comment

ldts Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ldts Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

etienne-lms left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ldts commented Oct 3, 2023

github-actions bot commented Nov 4, 2023

jenswi-linaro commented Nov 8, 2023

ldts commented Aug 24, 2023 •

edited

Loading

ldts Sep 6, 2023 •

edited

Loading

ldts Sep 21, 2023 •

edited

Loading

ldts Sep 22, 2023 •

edited

Loading