Add library authentication feature for MTL and LNL by jxstelter · Pull Request #8741 · thesofproject/sof

jxstelter · 2024-01-15T10:20:14Z

These changes will enable loadable library authentication before load.
Untrusted libraries will be not loaded.
Feature is available only on Intel platforms since it uses rom_ext module interface to
perform validation.

Signed-off-by: Jaroslaw Stelter Jaroslaw.Stelter@intel.com

kv2019i

Looks good. Not 100% sure what is the right place to put the auth headers to. This is not specific to RTOS, but is specific to targets. Platform seems the right place to put these.

lgirdwood

The API is mostly generic but has some Intel specific flows, I think we can solve all this in the short term by renaming the header files to auth/intel.h and auth/intel_iface.h. Short term we need the fix, long term we need to look at Zephyr auth driver API as this code should really be in RTOS as a system driver.

lgirdwood · 2024-01-17T11:25:59Z

+	ret = auth_api_result(&ext_lib->auth_ctx);
+
+	if (ret != AUTH_IMAGE_TRUSTED) {
+ tr_err(&lib_manager_tr, "lib_manager_auth_proc() Untrasted library!");


@jxstelter can we fix the typo in a follow up?

kv2019i

Almost there. Two things:

@lgirdwood proposed a bit different approach, to keep the headers in sof/zephyr/include but rename the interface with Intel prefix. I think the current V2 version imlements the platform option cleanly (a few nits inline), but granted the interface is likely fairly Intel specific. But yeah, Liam, preference which way to go?
Missing rfree on error path, Guennadi's comment.

jxstelter · 2024-01-23T16:24:52Z

Almost there. Two things:
@lgirdwood proposed a bit different approach, to keep the headers in sof/zephyr/include but rename the interface with Intel prefix. I think the current V2 version imlements the platform option cleanly (a few nits inline), but granted the interface is likely fairly Intel specific. But yeah, Liam, preference which way to go?
Missing rfree on error path, Guennadi's comment.

@lgirdwood Should I change names according to Your first review input?

kv2019i

Thanks, looks good to me!

lgirdwood · 2024-01-24T12:48:15Z

Almost there. Two things:
@lgirdwood proposed a bit different approach, to keep the headers in sof/zephyr/include but rename the interface with Intel prefix. I think the current V2 version imlements the platform option cleanly (a few nits inline), but granted the interface is likely fairly Intel specific. But yeah, Liam, preference which way to go?
Missing rfree on error path, Guennadi's comment.
@lgirdwood Should I change names according to Your first review input?

Yeah - just for the implementation side with Intel IP code. I think the interface is quite generic so hopefully can be lifted into Zephyr in the future without too many changes.

For ACE platforms library authentication could be done using external entity - rom_ext. This module reside in L3 memory space and provides verification functionality. Following code expose that API to SOF. Signed-off-by: Jaroslaw Stelter <Jaroslaw.Stelter@intel.com>

This patch adds usage of authentication API to check library signature. If feature enabled it will block loading of library without valid signature. Signed-off-by: Jaroslaw Stelter <Jaroslaw.Stelter@intel.com>

This enables library authentication on MTL platforms. Signed-off-by: Jaroslaw Stelter <Jaroslaw.Stelter@intel.com>

This enables library authentication on LNL platforms. Signed-off-by: Jaroslaw Stelter <Jaroslaw.Stelter@intel.com>

kv2019i

Refresh +1

lyakh · 2024-01-26T13:51:48Z

 	man_tmp_buffer = (__sparse_force void __sparse_cache *)
 rballoc_align(0, SOF_MEM_CAPS_DMA,
- MAN_MAX_SIZE_V1_8, dma_ext->addr_align);
+ MAN_MAX_SIZE_V1_8, CONFIG_MM_DRV_PAGE_SIZE);


how is this related? Can we add a comment in a follow-up? Or, if this wasn't intended, revert this

Authentication engine in ROM_EXT requires page aligned buffer. Otherwise it fails.

lyakh · 2024-01-26T13:56:25Z

+	struct ext_library *ext_lib = ext_lib_get();
+	int ret;
+
+	ret = auth_api_init_auth_proc(&ext_lib->auth_ctx, buffer_data, buffer_size, phase);


is this also needed in the LAST stage? A bit asymmetric

This is how reference code uses this API.

lgirdwood · 2024-01-26T14:10:54Z

Merging now as it fixes an issue. We can now start to look at how "authentication drivers" can upstream into Zephyr in the long term. @nashif @teburd @ceolin fyi.

jxstelter force-pushed the jst-prv-library-auth branch 5 times, most recently from 2875328 to fa78e53Compare January 15, 2024 11:15

jxstelter marked this pull request as ready for review January 16, 2024 08:47

jxstelter requested review from abonislawski, dbaluta, iuliana-prodan, kv2019i, lbetlej, lgirdwood, lyakh, mmaka1 and plbossart as code owners January 16, 2024 08:47

jxstelter force-pushed the jst-prv-library-auth branch from fa78e53 to 3d56f8aCompare January 16, 2024 13:48

kv2019i reviewed Jan 16, 2024
View reviewed changes

Comment threadzephyr/include/sof/auth/auth_api.h
Comment threadzephyr/include/sof/auth/status_logger_api.h
Comment threadsrc/library_manager/lib_manager.c Outdated
Comment threadsrc/library_manager/lib_manager.c Outdated

lyakh reviewed Jan 17, 2024
View reviewed changes

jxstelter force-pushed the jst-prv-library-auth branch from 3d56f8a to a1d1936Compare January 17, 2024 11:18

lgirdwood reviewed Jan 17, 2024
View reviewed changes

jxstelter force-pushed the jst-prv-library-auth branch 4 times, most recently from ad16ee6 to a0e476dCompare January 17, 2024 13:46

kv2019i reviewed Jan 18, 2024
View reviewed changes

Comment threadsrc/include/sof/auth_api_iface.h Outdated
Comment threadsrc/platform/intel/ace/include/ace/auth/auth_api.h
Comment threadsrc/include/sof/lib_manager.h Outdated

btian1 reviewed Jan 23, 2024
View reviewed changes

Comment threadsrc/library_manager/lib_manager.c

jxstelter force-pushed the jst-prv-library-auth branch from a0e476d to 7a5f264Compare January 23, 2024 16:21

kv2019i approved these changes Jan 23, 2024
View reviewed changes

jxstelter added 4 commits January 25, 2024 15:17

library_manager: auth: Add library authentication mechanizm
64a132a
This patch adds usage of authentication API to check library signature. If feature enabled it will block loading of library without valid signature. Signed-off-by: Jaroslaw Stelter <Jaroslaw.Stelter@intel.com>

mtl: conf: Enable library authentication on MTL
abb49d5
This enables library authentication on MTL platforms. Signed-off-by: Jaroslaw Stelter <Jaroslaw.Stelter@intel.com>

lnl: conf: Enable Library authentication on LNL
55a520a
This enables library authentication on LNL platforms. Signed-off-by: Jaroslaw Stelter <Jaroslaw.Stelter@intel.com>

jxstelter force-pushed the jst-prv-library-auth branch from 7a5f264 to 55a520aCompare January 25, 2024 14:18

kv2019i approved these changes Jan 25, 2024
View reviewed changes

kv2019i mentioned this pull request Jan 26, 2024
1st part of llext #8180 #8759
Merged

lyakh approved these changes Jan 26, 2024
View reviewed changes

lyakh reviewed Jan 26, 2024
View reviewed changes

lgirdwood approved these changes Jan 26, 2024
View reviewed changes

lgirdwood merged commit 17b3b01 into thesofproject:mainJan 26, 2024

jsarha mentioned this pull request Feb 12, 2024
V2.8.1 candidate #8855
Merged

Uh oh!

Conversation

jxstelter commented Jan 15, 2024

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgirdwoodJan 17, 2024

Choose a reason for hiding this comment

Uh oh!

lyakhJan 26, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jxstelter commented Jan 23, 2024

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Jan 24, 2024

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

lyakhJan 26, 2024

Choose a reason for hiding this comment

Uh oh!

jxstelterJan 26, 2024

Choose a reason for hiding this comment

Uh oh!

lyakhJan 26, 2024

Choose a reason for hiding this comment

Uh oh!

jxstelterJan 26, 2024

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Jan 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants