RE: [EXT] Re: [PATCH v19 0/7] firmware: imx: driver for NXP secure-enclave

quoted

quoted

quoted

Voltage regulation for i.MX93 in Linux kernel, is done by ELE.

During Linux suspend-resume, Secure-enclave (V2X on i.MX95) part 
of wake-up domain, will be managed by secure-enclaves(ELE) part 
of always-on domain.

So to sum-up, please correct me if I got it wrong:

 - NXP puts the ELE into the non-secure world, in case only one MU
   exists. The reason for this is that the ELE is also used to handle
   power-management.

For NXP SoCs with multi-MU(s) too, NXP proposes to put ELE driver 
into non-secure world.

With the ELE-FW and OP-TEE OS fix applied both worlds can communicate.
Therefore it doesn't matter and I didn't mentioned it explicit above.

That beeing said, with both worlds capable to talk to ELE and the ELE 
beeing very system critical, both worlds have to agree to the 
responsibilities, e.g. OP- TEE OS is not allow to manipulate the power-state behind the back of Linux.

Yes, your understanding is correct.

quoted

quoted

 - NXP exposes an uAPI which can be used to send arbitrary 
commands from userspace to the ELE. (no filtering done yet)

It is not correct to say that no filtering is done.
Before sending as well as after receiving the message, the message 
header of the buffers are parsed to check:
- TX-buffer with Command-tag is allowed to be sent, RX-buffer with response-tag is allowed to be received, without logging errors.

I really don't want to step to deep into this, but that beeing said. 
If you refer to cmd_tag, than this tag is used by your below mentioned 
library also for NVM access. NVM is clearly something we do have a 
linux-framework and uAPI for.

Will add a command parser in v20, which will prevent sending any arbitrary command.

quoted

- TX buffer size & Rx-Buffer size should match the size mentioned in 
the

buffer.

quoted

- FW version from the header is checked, if required secondary or 
runtime

FW Is loaded, before forwarding the request to ELE.

quoted

- In certain cases especially for exceptions, the message IDs are 
also

compared.

quoted

quoted

 --> Sounds to me that the userpace can influence the system behavior
     very badly.

Messages created and sent by User-space
library(https://eur01.safelinks.protection.outlook.com/?url=https%3A
%2
F%2Fgithub.com%2Fnxp-imx%2Fimx-secure-

enclave&data=05%7C02%7Cpankaj.gupta%40nxp.com%7Ce1d7cb78bda54edf
36d608de21fce1f6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
38985567165069017%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOn
RydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ
%3D%3D%7C0%7C%7C%7C&sdata=oE5nFyHRdW9ouWMy3VgiaGcgY7XKF4oZ
Mbnt95HZ1%2F8%3D&reserved=0), are scrutinized as stated above in the 
kernel driver.

This is uAPI which can be used by everyone and because your library 
may not implement access to power-managment calls doesn't mean that other libs do.
I'm not sure if your proposed driver will block such attempts from userspace.

Will add a command parser in v20, which will prevent sending any arbitrary command & any such attempt from userspace.

quoted

Moreover,
As part of this library, message creation, send-receive, IOCTLS etc.
kernel interface implementation logic, is not exposed to users of 
this library.

Your library is not the interface, the interface is the uAPI exposed 
by the kernel.

Agreed, that library is not the interface here. I never meant that.
I was trying to put the point that in order to exchange any arbitrary message, rootfs needs to be compromised
- either to allow any arbitrary/un-authorized application to send any arbitrary message,
- or to allow executing compromised library.
But Yes, you are correct that here we are talking kernel driver, not userspace.
 

This interface/your driver needs to handle valid and invalid access, 
e.g. hsm -> valid, nvm -> not valid since it is abstracted via NVMEM.

Kernel driver do ensure to handle only valid, and discard invalid access.
The in-coming command with command_tag are given to the dev_context which is marked receiver.
If the receiver is not assigned/set, it will be discarded.
The in-coming command with resp_tag are given to the dev_context which is the initiator of the initial command request.
If this is not followed, it will deny the message exchanged.

Will add a command parser in v20, which will prevent sending any arbitrary command.

quoted

With the help of secure-boot and IMA-EVM, rootfs can be restricted 
to not allow any new application or modified userspace library, to 
execute.

This way bad impact to the system behavior can be prevented.

Sorry but I really have to say that I have to NACK. Your interface is 
the kernel uAPI and not some NXP userspace library.

Will add a command parser in v20, which will prevent sending any arbitrary command from kernel driver.

An attacker could gain runtime system access and poke the ELE with 
arbitrary commands till he finds a fw-bug using the kernel uAPI (not 
your library of course).

Though the same command parser exists in ELE FW, which discards any arbitrary command.
Will add a command parser in v20, which will discard any arbitrary command and power management API(s), from uAPI.
Request you or other, to share any comment apart from this, that can be addressed in v20. 
Will send v20 by end of this week.

Thanks & regards
Pankaj

quoted

quoted

quoted

quoted

quoted

* Linux HWRNG.
* Loading the secondary runtime fw.

What is a secondary runtime-fw?

ELE FW size is larger than the size of ELE internal secure memory.
Hence FW is split into two.

Primary FW, is the FW, that enables features that helps for SoC boot-up.
Secondary runtime FW, is the FW, that enables features like HSM.

Ah okay, thanks for the input.

quoted

quoted

To conclude this longly discussion:

...

quoted

quoted

I still have mixed feeling about the fusing (including the 
1-MU case), since it requires a secure-world OS in place once 
the LOCK_DOWN

fuse was burned.

quoted

quoted

It's fine by me if NXP wants to have and wants to maintain a 
multi-path

here.

quoted

Write fuse API will be added, to allow writing fuses from secure 
world too.

This is a device life-cycle problem and if NXP decides to maintain 
multiple write paths, depending on the runtime-SoC state, this is 
fine by

me.

quoted

quoted

What needs to be ensured is, that the fuse-issue doesn't exist for 
the 1-MU case (i.MX8ULP) as you said.

As said above "Write fuse API will be added, to allow writing fuses 
from secure world too."
This will be true for 1 MU or multi-MU.

In your previous mail you just said that there is no such issue with 
the write path. Now you say that the single-MU case needs the same 
workaround.. At least I have read it that way.

quoted

quoted

quoted

quoted

Last but least, the uAPI which can be used to send arbitrary 
ELE commands seems unusual. But I don't know how 
secure-enclaves are abstracted within the kernel, so these are just my two cents.

it's not unusual at all. The pattern of userspace sending 
commands directly to the kernel via a queue is quite common like:

GPUs: As you mentioned, userspace drivers (like those in Vulkan 
or
CUDA) often build command buffers and submit them directly to 
the kernel or hardware.

That's right, but these drivers do at least some filtering on the 
OPs and check if they are allowed. According your patchset, you 
just write
(se_if_fops_write()) the provided userspace buffer.

We are validating the buffer size against the size mentioned in the 
buffer header.
Refer above comments for more details.

quoted

quoted

Secure Enclaves: In systems like Intel SGX or AMD SEV, userspace 
applications interact with enclaves via ioctl or mmap 
interfaces, often sending structured commands or messages.

What I'm aware of is, that most secure-enclaves are switching to 
the standard TPM API.

In case of NXP SoC with ELE HW IP, ELE is considered as on-SoC TPM. 
No additional or external TPM is needed, if ELE is present on SoC.

I said "TPM API" not "TPM device", may it be fTPM or dTPM. I think AMD 
switched to fTPM, so no special userspace-requirements must be fulfilled.
However, this is also off-topic.

Regards,
  Marco