plugins: example of using supplicant syslog plugin from TA code by anisyanka · Pull Request #79 · linaro-swg/optee_examples

anisyanka · 2020-12-08T22:06:08Z

TEE clients can add to the tee-supplicant an own plugin.
This feature makes the supplicant a bit more flexible
in terms of providing services. Any external TEE services
can be designed as a tee-supplicant plugin.

User TAs and also the OP-TEE kernel code can interact
with the plugins with the help of the special PRC.

This patch adds example of using the plugin framework.
By default in tee-supplicant there is 'syslog' plugin.
It can write log messages from OP-TEE (TAs or kernel)
to system log. You can find the implementation of the plugin
in 'tee-supplicant/plugins/syslog/syslog_plugin.c' file.

This example adds a new TA, which interacts with
'syslog' plugin with the help of the plugin-pta.
This TA increments a value and prints some strings
to the syslog. Also patch adds a host CA, which
calls the TA 'TA_PING_CNT' times.

If the example works successfully, we can find the following
strings in the log file (for qemu it's '/var/log/messages' file):

21:18:05 buildroot daemon.info tee-supplicant[92]: Hello, plugin! value = 0x0
21:18:07 buildroot daemon.info tee-supplicant[92]: Hello, plugin! value = 0x1
21:18:09 buildroot daemon.info tee-supplicant[92]: Hello, plugin! value = 0x2
21:18:11 buildroot daemon.info tee-supplicant[92]: Hello, plugin! value = 0x3
21:18:13 buildroot daemon.info tee-supplicant[92]: Hello, plugin! value = 0x4

Signed-off-by: Aleksandr Anisimov a.anisimov@omprussia.ru

anisyanka · 2020-12-08T22:21:47Z

The example relates to:
OP-TEE/optee_client#239
OP-TEE/optee_os#4248

etienne-lms

Thanks for the P-R.
Some minor comments.
It would be nice to have a Cmake script. I'll see if I can help.
The supplicant plugin syslog shall be built as a .so and installed in /usr/lib/tee-supplicant/plugins/ right?

etienne-lms · 2020-12-16T11:46:01Z

+ * TA properties: multi-instance TA, no specific attribute
+ * TA_FLAG_EXEC_DDR is meaningless but mandated.
+ */
+#define TA_FLAGS TA_FLAG_EXEC_DDR


TA_FLAG_EXEC_DDR mandated? maybe by older optee_os. Can you check you still have this dependency. It not, prefer 0 here.

You are right. TA_FLAG_EXEC_DDR is already deprecated. I will replace to 0.

etienne-lms · 2020-12-16T11:47:13Z

+	case PLUGIN_TA_PING:
+		return syslog_plugin_ping();
+	default:
+		return TEE_ERROR_BAD_PARAMETERS;


TEE_ERROR_NOT_SUPPORTED

etienne-lms · 2020-12-16T12:15:06Z

+		 inc_var++);
+
+	params[2].memref.buffer = log_str;
+	params[2].memref.size = strlen(log_str);


Suggest to add trace here IMSG("Push syslog plugin string \"%s\"", log_str);

etienne-lms · 2020-12-16T12:17:04Z

+	TEE_UUID syslog_uuid = SYSLOG_PLUGIN_UUID;
+	TEE_TASessionHandle sess;
+	TEE_Param params[4] = { 0 };
+	uint32_t types, orig;


Prefer to follow the same codiing rules as optee_os/optee_client.

uint32_t types = 0; uint32_t orig = 0;

etienne-lms · 2020-12-16T12:17:25Z

+		uint32_t d2;
+		uint32_t d3;
+		uint32_t d4;
+	} plug_u;


etienne-lms · 2020-12-16T12:17:48Z

+
+static TEE_Result syslog_plugin_ping(void)
+{
+	TEE_Result res;


Prefer to follow the same codiing rules as optee_os/optee_client: init all local vars.

etienne-lms · 2020-12-16T12:18:26Z

+	if (param_types != exp_param_types)
+		return TEE_ERROR_BAD_PARAMETERS;
+
+	/* If return value != TEE_SUCCESS the session will not be created. */


can remove this comment

etienne-lms · 2020-12-16T12:18:53Z

+ * with a value to be able to identify this session in subsequent calls to the
+ * TA. In this function you will normally do the global initialization for the
+ * TA.
+ */


I suggest to remove these inline comments

etienne-lms · 2020-12-16T12:19:28Z

+ * Copyright (c) 2020, Open Mobile Platform LLC
+ */
+
+#ifndef TA_PLUGIN_H


PLUGIN_TA_H

etienne-lms · 2020-12-16T12:23:48Z

+
+int main(void)
+{
+	int i;


default init all loval variables

anisyanka · 2020-12-17T22:09:14Z

It would be nice to have a Cmake script. I'll see if I can help.

Do you mean Cmake script to build syslog plugin here? Or maybe you need script in optee-client repo to build any future plugins?

The supplicant plugin syslog shall be built as a .so and installed in /usr/lib/tee-supplicant/plugins/ right?

Yes, it's right.

anisyanka · 2020-12-23T08:19:43Z

All comments have been addressed

jenswikl · 2021-01-04T08:39:53Z

@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: BSD-2-Clause */


This file should be made available from optee_client instead to avoid keeping possibly diverging copies.

I droped this file.
Now we use $(TEEC_EXPORT)/include in syslog Makefile

jenswikl · 2021-01-04T08:46:38Z

+/*
+ * Interface with syslog tee-supplicant plugin
+ */
+#define SYSLOG_PLUGIN_UUID { 0x96bcf744, 0x4f72, 0x4866, \


Wouldn't it make sense to have a shared .h file with all this?

anisyanka · 2021-01-10T13:45:52Z

All comments have been addressed

jenswikl · 2021-01-15T13:24:22Z

+	/* Initialize a context connecting us to the TEE */
+	res = TEEC_InitializeContext(NULL, &ctx);
+	if (res != TEEC_SUCCESS)
+		errx(1, "TEEC_InitializeContext failed with code 0x%x", res);


Please use %#"PRIx32 instead of 0x%x". res is a basically uint32_t.
Same below for the other format strings.

jenswikl · 2021-01-15T13:28:16Z

+
+	/* 'sub_cmd' in this case means priority according syslog.h */
+	openlog(NULL, LOG_CONS | LOG_PID, LOG_DAEMON);
+	syslog((int)sub_cmd, "%s", (const char *)data);


What if data isn't null terminated?

In this example I use null-terminated strings for the plugin to pass them to syslog().
But actually, data_len stores real size of the data. that's why we can use data and data_len in any other cases of the use of plugins.

Yes, obviously it works but it isn't robust. Bear in mind that people might copy these examples.
I'd prefer something more safe like:

syslog(sub_cmd, "%*s", (int)data_len, (const char *)data);

anisyanka · 2021-01-20T09:44:00Z

All comments have been addressed

jenswikl · 2021-01-25T12:25:07Z

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

TEE clients can add to the tee-supplicant own plugins. This feature makes the supplicant a bit more flexible in terms of providing services. Any external TEE services can be designed as a tee-supplicant plugin. User TAs can interact with the plugins using libutee and the OP-TEE kernel code can use a special plugin PRC for it. This patch adds example of using the plugin framework. 'syslog' plugin is used as an example. It can write log messages from OP-TEE (TAs or kernel) to system log. Also patch adds a new TA, which interacts with the 'syslog' plugin with the help of 'tee_invoke_supp_plugin()'. This TA increments a value and prints some strings to the syslog. Also patch adds a host CA, which calls the TA 'TA_PING_CNT' times. If the example works successfully, we can find the following strings in the log file (for qemu it's '/var/log/messages' file): ``` 21:18:05 buildroot daemon.info tee-supplicant[92]: Hello, plugin! value = 0x0 21:18:07 buildroot daemon.info tee-supplicant[92]: Hello, plugin! value = 0x1 21:18:09 buildroot daemon.info tee-supplicant[92]: Hello, plugin! value = 0x2 21:18:11 buildroot daemon.info tee-supplicant[92]: Hello, plugin! value = 0x3 21:18:13 buildroot daemon.info tee-supplicant[92]: Hello, plugin! value = 0x4 ``` Signed-off-by: Aleksandr Anisimov <a.anisimov@omprussia.ru> Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

anisyanka · 2021-01-29T17:06:00Z

The commit updated, thanks for the review!

This was referenced Dec 8, 2020

Loadable plugins in tee-supplicant OP-TEE/optee_client#219

Closed

tee-supplicant: add a framework for loadable plugins OP-TEE/optee_client#239

Merged

anisyanka force-pushed the plugin-example branch from 66cacca to 835b031 Compare December 16, 2020 11:26

etienne-lms reviewed Dec 16, 2020

View reviewed changes

jenswikl reviewed Jan 4, 2021

View reviewed changes

anisyanka force-pushed the plugin-example branch from 9d4a5b6 to ab021b7 Compare January 10, 2021 13:48

jenswikl reviewed Jan 15, 2021

View reviewed changes

anisyanka mentioned this pull request Jan 20, 2021

regression: add tests for the supplicant plugin framework OP-TEE/optee_test#482

Merged

anisyanka force-pushed the plugin-example branch from b7a7c84 to b4dc6d8 Compare January 25, 2021 12:52

jforissier merged commit 0607ed4 into linaro-swg:master Feb 4, 2021

anisyanka deleted the plugin-example branch February 4, 2021 15:18

Uh oh!

Conversation

anisyanka commented Dec 8, 2020

Uh oh!

anisyanka commented Dec 8, 2020

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!

anisyanka commented Dec 17, 2020

Uh oh!

anisyanka commented Dec 23, 2020

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!

anisyanka commented Jan 10, 2021

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!

anisyanka commented Jan 20, 2021

Uh oh!

jenswikl commented Jan 25, 2021

Uh oh!

anisyanka commented Jan 29, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants