zynqmp: HUK and RPMB ready #4874

ldts · 2021-09-21T14:24:34Z

The following commits will enable the generation of a hardware unique key.

If the board is not in a secured state, the HUK will just be a 16byte digest of the DNA eFuses. This HUK is of course not secured as the DNA eFuses could be read from EL1 via SMC.

If the board is in a secured state - boot authentication enabled- the generated HUK will be secured by encrypting the digest with the PUF KEK.

The zyqmp devices have a 96 bits identifier (3x32 bit) stored in eFUSES (DNA0, DNA1 and DNA2).

These devices also include an un-clonable function that generate a private key unique to the board which can be fed to an AES-GCM cipher. This key is only valid when the system boots in secured mode (RSA_EN and the PPK have been fused) and the PUF has been securely provisioned.

Since the HUK will benefit from some randomization, the HUK generation strategy will consist in reading the DNA eFUSES and passing these unique values to the AES cipher using the PUF (secret) KEK

These commits add support to

enable the PUF,
DMA the input to the AES
execute the encryption
reads the DNA eFuses
generates the HUK

This code has been tested on Zynqmp zu3cg.
The PUF was provisioned using the Xilinx Lightweight Provisioning Tool (the tool was run from a docker container running petalinux and a reduced version of the Vivado SDK).It then connects to the target processor via JTAG.
The tool itself (XLWPT) needs to be requested from a Xilinx representative.

This tool downloads an FSBL based executable - with the Xilskey library giving access to all eFuses (by default the PMUFW is not allowed to access the security fuses).

We plan to deliver an OP-TEE TA capable of replacing such tool but no final schedule yet.

Only when the HUK has been provisioned in a secured state, we will allow the RPMB key to be generated.

The HUK and RPMB access has been tested on Zynqmp zu3cg

jforissier

In commit description of "zynqmp: register the CSU memory with the platform": please capitalize PUF, AES, SHA.
Apart from mostly cosmetic issues (please see below), that looks good to me so:

Acked-by: Jerome Forissier <jerome@forissier.org>

core/arch/arm/plat-zynqmp/drivers/puf.h

core/arch/arm/plat-zynqmp/drivers/puf.c

core/arch/arm/plat-zynqmp/drivers/aes.c

core/arch/arm/plat-zynqmp/drivers/aes.h

core/arch/arm/plat-zynqmp/drivers/aes.c

core/arch/arm/plat-zynqmp/drivers/puf.c

ldts

Thanks @jforissier I'll update the PR addressing your issues. I also need to to check what happens when the SoC hasnt been secured (ie, RSA_EN not fused), and the user tries to access the PUF KEK. I need to handle that error to avoid generating the wrong HUK (and therefore maybe burning the wrong key in RPMB)

ldts · 2021-09-29T05:42:03Z

I am going to need another week to clarify some things: the PUF should not be registering when the board is not booting in secured mode but at the moment it is. Seems the hardware, instead of aborting, provides some other development key instead...pretty much like it happens on IMX. anyway, will clarify

ldts · 2021-09-29T15:30:46Z

@jforissier I fixed the issues that you raised and added some extra functionality (ie, aes-gcm decryption of the black key). Still need to work on the PUF KEK generation (need to post-process the syndrome data and fuse some values)...but I will encapsulate that in some function in puf.c

jforissier

Hi @ldts,

I spotted a few more things below, but overall LGTM:

Acked-by: Jerome Forissier <jerome@forissier.org>

jforissier · 2021-09-29T15:21:39Z

core/arch/arm/plat-zynqmp/drivers/sub.mk

@@ -1,2 +1,4 @@
 srcs-$(CFG_CSU_PUF) += puf.c
 srcs-$(CFG_CSU_DMA) += csu_dma.c
+srcs-$(CFG_CSU_AES) += aes.c
+


Extra empty line

jforissier · 2021-09-29T15:29:00Z

core/arch/arm/plat-zynqmp/drivers/aes.c

+	ret = aes_wait(AES_STS_AES_BUSY, false);
+	if (ret)
+		EMSG("AES transfer failed");
+error:


s/error/out/

jforissier · 2021-09-29T15:35:34Z

core/arch/arm/plat-zynqmp/drivers/aes.c

+	if (ret)
+		return ret;
+
+	return TEE_SUCCESS;


Equivalent to return ret;

jforissier · 2021-09-29T15:36:01Z

core/arch/arm/plat-zynqmp/drivers/aes.c

+	val = io_read32(aes + AES_KEY_CLR_OFFSET);
+	io_write32(aes + AES_KEY_CLR_OFFSET, val | AES_KEY_ZERO | AES_KUP_ZERO);
+	if (aes_wait(AES_STS_AES_KEY_ZEROED | AES_STS_KUP_ZEROED, true))
+		EMSG("Failed to clear the AES key");


Should the function proceed even in case of error?

I dont see why not: the encoding/decoding did complete, it is just the housekeeping...I suppose it might lead to a failure on the next run ...hence why I am doing the EMSG.

jforissier · 2021-09-29T15:42:23Z

core/arch/arm/plat-zynqmp/drivers/aes.c

+	ret = aes_wait(AES_STS_AES_BUSY, false);
+	if (ret)
+		EMSG("AES-GCM transfer failed");
+error:


s/error/out/ since this label is reached on success too

vesajaaskelainen · 2021-10-01T05:18:53Z

If you enable encryption support for your image PUF is not usable.

vesajaaskelainen · 2021-10-01T06:38:11Z

core/arch/arm/plat-zynqmp/conf.mk

+CFG_CSU_AES ?= y
+
+ifeq ($(CFG_CSU_AES), y)
+$(call force,CFG_CSU_PUF,y,Mandated by CFG_CSU_AES)


In here you tie PUF to AES functionality which is not suitable for all applications -> please keep them separate as non-PUF users may want to access the HW AES with different key than PUF.

I would think that image encryption feature is so important with FPGA designs so perhaps default for CSU PUF enablement would need to be n.

I believe we need to select what HUK solution one goes for and then that would select necessary hardware modules in use?

right..it is a bit difficult for me to generalize on what other users might need. From my perspective - SPL authenticated boot with RPMB access, this made sense. I can work with your requirement but moving forward those with extended needs can as well build on top? I just dont like to generalize too early unless there is a need (like in this case)

Well you could reverse it with something like this:

CFG_CSU_PUF ?= y ifeq ($(CFG_CSU_PUF), y) $(call force,CFG_CSU_AES,y,Needed by CFG_CSU_PUF) $(call force,CFG_CSU_DMA,y,Needed by CFG_CSU_PUF) endif

Then later you might have something like:

CFG_XILINX_MPSOC_HUK_PUF ?= y ifeq ($(CFG_XILINX_MPSOC_HUK_PUF), y) $(call force,CFG_CSU_PUF,y,Needed by CFG_XILINX_MPSOC_HUK_PUF) endif

vesajaaskelainen · 2021-10-01T06:46:38Z

core/arch/arm/plat-zynqmp/drivers/puf.c

+#define CSU_PUF_BASE			(CSU_BASE + 0x4000)
+#define CSU_PUF_SIZE			0x1000
+
+#define CMD				0x00


These are a bit vague names.

For reference the public register map is here:
https://www.xilinx.com/html_docs/registers/ug1087/ug1087-zynq-ultrascale-registers.html

These PUF registers are part of CSU module. Official name for CMD is puf_cmd.

Please be a bit more verbose with the name something like:

PUF_CMD_REG_OFFSET or so with or without CSU_ prefix.

yes, btw thanks for those pointers to hyperlinks but I know them all really :)

@jforissier anyone wants to comment on this? I dislike long names for registers because -for me- it makes the code a PIA to read. I'd rather keep it as is unless what @vesajaaskelainen is a defacto rule now.

Well I like both short identifiers, and identifiers that are documented in the official documentation. In case both are incompatible, the context matters. Here the scope of the macros is limited to the file so I'd say we can consider that Zynq/CSU/PUF are implicit and therefore keep shorter names. In a header file things would be a bit different.

vesajaaskelainen · 2021-10-01T06:49:32Z

core/arch/arm/plat-zynqmp/drivers/puf.c

+ *  the syndrome words need to be post-processed and fused to generate a
+ *  persistent KEK
+ */
+uint32_t syn[PUF_SYN_WORDS];


global variable?

Add at least static in here and move up before methods.

no no, you are right, it needs to be static

I still need to add the post-processing before merging this PR tough. will fix anyway

vesajaaskelainen · 2021-10-01T06:50:16Z

core/arch/arm/plat-zynqmp/drivers/puf.h

+#ifndef PUF_H_
+#define PUF_H_
+
+TEE_Result puf_regenerate(void);


There could be other providers for "puf" too... so better to namespace these a bit more.

what about csu_puf_regenerate?

I'll also rename the file

so aes -> csu_aes and puf-> csu_pfu and associated interfaces

There are files like stm32_ , imx_ ls_, bcm_... so should we have xilinx_mpsoc_X?

There is also folder imx -- one option would be to make core/drivers/xilinx/mpsoc/

We have something coming for zynq so those could then go for core/drivers/xilinx/zynq folder.

@jenswi-linaro | @jforissier do you have comment on how this drivers folder should be organized?

Until we see that it doesn't scale I'd suggest using a zynq_ and zynqmp_ prefix as needed for the relevant files directly in core/drivers/. There's no need to add an additional xilinx_ prefix.

Drivers below core/drivers/ are probably better grouped by type than by the name of the soc. It could for instance make sense to put all the uart drivers into core/drivers/uart/ the day that the number of uart drivers becomes to large.

vesajaaskelainen · 2021-10-01T06:51:17Z

core/arch/arm/plat-zynqmp/main.c

@@ -58,6 +58,10 @@ register_phys_mem_pgdir(MEM_AREA_IO_SEC,
 			ROUNDDOWN(GIC_BASE + GICD_OFFSET, CORE_MMU_PGDIR_SIZE),
 			CORE_MMU_PGDIR_SIZE);

+register_phys_mem_pgdir(MEM_AREA_IO_SEC,


Shouldn't this be in CSU driver instead?

Eg. CSU gets mapped only when included in the build.

right but the CSU has many subsystems, so I made the choice to initialize the memory here and allow those subsystems to become drivers.

vesajaaskelainen · 2021-10-01T06:52:23Z

core/arch/arm/plat-zynqmp/platform_config.h

@@ -86,6 +86,10 @@
 #error "Unknown platform flavor"
 #endif

+#define CSU_DMA_BASE		0xFFC80000


Official name for this is CSUDMA

Also this AES driver here and Linux kernel driver need to co-live together so there is no race... Should these actually be configured with device tree so that only either one can use or claim ownership -- or then the OP-TEE drops usage before Linux or U-Boot uses that resource (or uses only in between of FSBL/SPL and u-boot).

you probably noticed that I didnt integrate the driver with the crypto api: this is because the use case for this implementation of AES-GCM is only the encryption of the DNA string (read from eFUSE) just so we get a cryptography strong - and secret- HUK.

Please note that PS DNA is not considered stable in MPSoC. Only guarantee is given for PL DNA.

not sure what you mean, the DNA is an eFUSE (well 3 eFuses) also where did you get this information from?

You quoted to wrong place -- I'll reply to in main comment stream.

vesajaaskelainen · 2021-10-01T07:15:25Z

core/arch/arm/plat-zynqmp/platform_config.h

@@ -86,6 +86,10 @@
 #error "Unknown platform flavor"
 #endif

+#define CSU_DMA_BASE		0xFFC80000
+#define CSU_DMA_SIZE		0x1000
+#define CSU_BASE		0xFFCA0000


Tried to look for CSU size but did not spot it easily. Might be in some vivado generated headers.

Largest public register offset is 0x0000005034 (+4).

It might be a good idea to have size for CSU also defined. Can you spot it somewhere?

yeah I had the same problem, I dont know...so I pretty much picked the addresses that I needed because 5038 seems weird...

vesajaaskelainen · 2021-10-01T07:17:08Z

core/arch/arm/plat-zynqmp/drivers/puf.c

+#define PUF_STATUS_SYN_WRD_RDY_MASK	BIT(0)
+#define PUF_STATUS_KEY_RDY_MASK		BIT(3)
+
+#define	PUF_SYN_WORDS			141


In above you have space after define here you have tab.

vesajaaskelainen · 2021-10-01T07:21:20Z

core/arch/arm/plat-zynqmp/drivers/puf.c

+#define PUF_ISR_ACC_ERROR_MASK          BIT(12)
+
+/* PUF */
+#define CSU_PUF_BASE			(CSU_BASE + 0x4000)


These CSU sub module base addresses would fit nicely in platform_config.h. (or even a separate register map header).

yeah I wasnt sure. I opted for adding it here since I nobody else should care for that address

vesajaaskelainen · 2021-10-01T07:23:09Z

core/arch/arm/plat-zynqmp/drivers/puf.c

+#include "puf.h"
+
+/* CSU */
+#define ISR				0x20


These would fit more nicely in headers somewhere where there would be CSU section for its registers.

for the same reason that above, I didnt feel the need to share it in some interface deficion since nobody should be reading that register out of the scope of this file...

vesajaaskelainen · 2021-10-01T07:26:47Z

core/arch/arm/plat-zynqmp/drivers/aes.c

+	return aes_done_op(AES_DEC, ret);
+}
+
+TEE_Result aes_encrypt_data(uint8_t *src, uint8_t *dst, uint8_t *iv,


Often order of buffers and their sizes are next to each other.

Like src, src_len, dst, dst_len...

Apply elsewhere too.

And should these pointer types be void?

src and iv with const.

yeah you are right

vesajaaskelainen · 2021-10-01T07:28:45Z

core/arch/arm/plat-zynqmp/drivers/aes.c

+	return TEE_SUCCESS;
+}
+
+TEE_Result aes_decrypt_data(uint8_t *src, uint8_t *dst, uint8_t *tag,


tag with const void *

vesajaaskelainen · 2021-10-01T07:35:38Z

core/arch/arm/plat-zynqmp/drivers/aes.h

+#include <tee_api_types.h>
+
+enum csu_key {
+	CSU_AES_KEY_SRC_KUP,


Better set first one with CSU_AES_KEY_SRC_KUP = 0 to be exactly clear that this needs to be zero as enum value goes right to register.

vesajaaskelainen · 2021-10-01T07:42:43Z

Linux kernel has MPSoC AES driver or couple of them 😄:
https://github.com/Xilinx/linux-xlnx/blob/xlnx_rebase_v5.10/drivers/crypto/xilinx/zynqmp-aes.c
https://github.com/Xilinx/linux-xlnx/blob/xlnx_rebase_v5.10/drivers/crypto/xilinx/zynqmp-aes-gcm.c
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/crypto/xilinx/zynqmp-aes-gcm.c?h=v5.15-rc3

(and RSA and SHA drivers).

That being said -- those give access to same key as what is being used in here (or in next PR).

If that is source for HUK -- not a good thing. Any plans on how to protect that?

vesajaaskelainen · 2021-10-01T07:45:43Z

Also this AES driver here and Linux kernel driver need to co-live together so there is no race... Should these actually be configured with device tree so that only either one can use or claim ownership -- or then the OP-TEE drops usage before Linux or U-Boot uses that resource (or uses only in between of FSBL/SPL and u-boot).

ldts · 2021-10-01T08:58:13Z

If you enable encryption support for your image PUF is not usable.

@vesajaaskelainen yes. But we dont need to encrypt boot.bin: both SPL and PMUFW are open source and we dont care too much about the bitstream confidentiality. Authentication is enough for our root of trust.

ldts · 2021-10-01T09:01:30Z

Also this AES driver here and Linux kernel driver need to co-live together so there is no race... Should these actually be configured with device tree so that only either one can use or claim ownership -- or then the OP-TEE drops usage before Linux or U-Boot uses that resource (or uses only in between of FSBL/SPL and u-boot).

you probably noticed that I didnt integrate the driver with the crypto api: this is because the use case for this implementation of AES-GCM is only the encryption of the DNA string (read from eFUSE) just so we get a cryptography strong - and secret- HUK. So yes, droping usage once the HUK has initialized would be the best choice IMO (and unless someone else comes up with a different use case - ie some other black key generation.

BTW thanks for all your comments; I am still reworking parts of the implementation but I'll make sure I incorporate at least some of them.

vesajaaskelainen · 2021-10-01T09:01:59Z

If you enable encryption support for your image PUF is not usable.

@vesajaaskelainen yes. But we dont need to encrypt boot.bin: both SPL and PMUFW are open source and we dont care too much about the bitstream. Authentication is enough for our root of trust.

Please remember that there are other uses than what you are planning in your particular product.

PMU ROM itself is closed source.

PMUFW (extensions to PMU) can be closed if wanted and you are free to extend it.

Only u-boot requires you to share source code -- and if you don't like that then you have other options too like FSBL.

vesajaaskelainen · 2021-10-01T09:03:13Z

Also this AES driver here and Linux kernel driver need to co-live together so there is no race... Should these actually be configured with device tree so that only either one can use or claim ownership -- or then the OP-TEE drops usage before Linux or U-Boot uses that resource (or uses only in between of FSBL/SPL and u-boot).

you probably noticed that I didnt integrate the driver with the crypto api: this is because the use case for this implementation of AES-GCM is only the encryption of the DNA string (read from eFUSE) just so we get a cryptography strong - and secret- HUK.

Please note that PS DNA is not considered stable in MPSoC. Only guarantee is given for PL DNA.

vesajaaskelainen · 2021-10-01T09:07:44Z

Also this AES driver here and Linux kernel driver need to co-live together so there is no race... Should these actually be configured with device tree so that only either one can use or claim ownership -- or then the OP-TEE drops usage before Linux or U-Boot uses that resource (or uses only in between of FSBL/SPL and u-boot).

you probably noticed that I didnt integrate the driver with the crypto api: this is because the use case for this implementation of AES-GCM is only the encryption of the DNA string (read from eFUSE) just so we get a cryptography strong - and secret- HUK. So yes, doping usage once the HUK has initialized would be the best choice IMO (and unless someone else comes up with a different use case - ie some other black key generation.

It is not anymore strong secret if you get access to operate on key and needed details so you can replicate the result in user space.

ricardosalveti · 2021-10-31T23:11:26Z

core/arch/arm/plat-zynqmp/conf.mk

+
+ifeq ($(CFG_ZYNQMP_HUK),y)
+$(call force,CFG_ZYNQMP_AES,y,Mandated by CFG_ZYNQMP_HUK)
+$(call force,CFG_ZYNQMP_PUF,y,Mandated by CFG_ZYNQMP_HUK)


Huk also depends on CFG_ZYNQMP_CSU_PUF because of zynqmp_csu_puf_reset.

And on CFG_ZYNQMP_CSU_AES because of zynqmp_csu_aes_encrypt_data and zynqmp_csu_aes_decrypt_data.

yes, the current configs are leftovers from incorrectly addressing previous renames...will fix!

ricardosalveti · 2021-10-31T23:28:03Z

core/drivers/zynqmp_csudma.c

+{
+	vaddr_t dma = core_mmu_get_va(CSUDMA_BASE, MEM_AREA_IO_SEC,
+				      CSUDMA_SIZE);
+	uint32_t data = 0;


core/drivers/zynqmp_csudma.c: In function 'zynqmp_csudma_prepare': core/drivers/zynqmp_csudma.c:88:11: error: unused variable 'data' [-Werror=unused-variable] 88 | uint32_t data = 0; | ^~~~

ldts · 2021-11-01T17:22:13Z

@ricardosalveti so, all good on your HUK validation test?

ricardosalveti · 2021-11-02T01:28:37Z

@ricardosalveti so, all good on your HUK validation test?

Yup, finally able to fuse and reproduce on my own board.

Tested-by: Ricardo Salveti <ricardo@foundries.io>

etienne-lms

Acked-by: Etienne Carriere <etienne.carriere@linaro.org> for the serie.
(note few remaining minor issues in inline comments in commit "zynqmp: drivers: PM firmware")

etienne-lms · 2021-11-02T18:47:24Z

core/drivers/zynqmp_pm.c

+/*
+ * Stores all required details to read/write eFuse memory.
+ * @src:        Physical address of the buffer to store the data to be
+ *              written/read buffer (in LE)


remove buffer

etienne-lms · 2021-11-02T18:49:44Z

core/drivers/zynqmp_pm.c

+ * @offset:     offset in bytes to be read from/written to
+ * @flag:       EFUSE_READ  - represents eFuse read operation
+ *              EFUSE_WRITE - represents eFuse write operation
+ * @pufuserfuse:0 - represents non-puf eFuses, offset is used for read/write


would suggest here s/puf/PUF/g

etienne-lms · 2021-11-02T18:50:08Z

core/include/drivers/zynqmp_pm.h

+ *       in LE byte order. The buffer address must be cached aligned
+ * @buf_sz: buffer size in bytes, must be a multiple of the cacheline size
+ * @id: eFuse identifier.
+ * @puf: is eFuse puf, ZYNQMP_PUF_EFUSE/ZYNQMP_NONPUF_EFUSE


s/puf/PUF/?

ldts · 2021-11-02T20:55:43Z

updated

vesajaaskelainen · 2021-11-03T06:40:59Z

No need to wait for me testing it as @ricardosalveti already tested it out.

I can now save one board's eFuses ;)

vesajaaskelainen · 2021-11-03T06:48:39Z

Note: Our plan with utilizing FPGA's FUSE_USER_128 eFuses did not go well as you can only access that by JTAG and not by VHDL without having JTAG AXI master there which would make it rather unsecure solution. Current plan is to utilize device key like for PUF case here but utilize eFuse Key (used also for encrypting the firmware) and then additional device unique material from additional PS eFuses (USER_[7..0]) currently thinking on doing CMAC or such with that. Only catch is that now we must deny the access for PS USER eFuses to user space.

ldts · 2021-11-04T15:11:18Z

@vesajaaskelainen um ok thanks for sharing. In my experience this is an unforgiving platform.

@etienne-lms @jforissier anything else pending?

jforissier

A couple of commit subjects are not very descriptive, here is what I suggest (assuming I understood the patches correctly):

"zynqmp: drivers: HUK" => "zynqmp: drivers: generate HUK from PUF KEK" (note PUF is misspelled in the description)
"zynqmp: drivers: rpmb" => "zynqmp: use HUK derived from PUF KEK for RPMB"

Otherwise looks OK to me for merging.

jforissier · 2021-11-05T09:29:19Z

core/arch/arm/plat-zynqmp/main.c

+
+	/*
+	 * For security reasons, we don't allow writing the RPMB key using the
+	 * development HUK even thought it is unique.


jforissier · 2021-11-05T09:35:07Z

core/arch/arm/plat-zynqmp/conf.mk

@@ -30,6 +30,10 @@ CFG_CRYPTO_WITH_CE ?= y

 CFG_ZYNQMP_PM ?= $(CFG_ARM64_core)

+ifeq ($(CFG_RPMB_FS),y)
+$(call force,CFG_ZYNQMP_HUK,y,Mandated by CFG_RPMB_FS)


Isn't CFG_ZYNQMP_HUK ?= $(CFG_RPMB_FS) more correct? Or do you really want to force usage of HUK?

yes, I wanted to force using the zynqmp HUK instead of some other value

jforissier · 2021-11-05T09:52:04Z

core/arch/arm/plat-zynqmp/main.c

 #include <kernel/tee_time.h>
+#include <io.h>


Alphabetical order

argh, sorry

Add the base address definitions for the CSU and the CSUDMA modules Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Reviewed-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

Explicitily define the cache line length Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

The CSU memory block that will be mapped from different drivers (ie, PUF, AES-GCM, SHA..) Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Reviewed-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

CSU registers and offsets for submodules Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Reviewed-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

This block is used to generate black keys via the AES-GCM module. The PUF KEK - feeding the AES-GCM block - is also unique for each device. The KEK is only available once the board has been secured via programmable eFUSES (RSA_EN authentication via the PPK fuses). Registering the PUF should be done using the Xilinx tools so the adequate eFUSES are written. Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

This module provides a mechanism to transfer data between memory and peripherals. The data path is selected in the Secure Stream Switch register in the CSU. Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

Provide a mechanism to encrypt a red key using the KEK; the KEK is only available on secured boards after the RSA_EN and PPK eFUSES have been burnt (the system will only boot ROM authenticated bootloaders from here on). The main use case for OP-TEE would be to encode the zynqmp per device unique identifier (DNA0, DNA1, DNA2 eFUSEs - ie, a red key) using the KEK. The encryption key generated this way is cryptographically strong and will be used as the device HUK (ie, black key). Test code: csu_aes_encrypt_data(src, dst, BLOB_DATA_SIZE, tag, GCM_TAG_SIZE, iv, GCM_IV_SIZE, CSU_AES_KEY_SRC_DEV); csu_aes_decrypt_data(dst, src, BLOB_DATA_SIZE, tag, GCM_TAG_SIZE, iv, GCM_IV_SIZE, CSU_AES_KEY_SRC_DEV); if (memcmp(src, buffer, BLOB_DATA_SIZE)) { EMSG(" - encrypt/decrypt test failed"); Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

These routines call TF-A exported SiP services that implement IPI protocol for communication with PMUFW (Platform Management Unit). To access eFuses, PMUFW should be built with -DENABLE_EFUSE_ACCESS=1. Notice however that certain eFuses will not be available unless the Xilskey library linked to the PMUFW is compiled removing some of those security restrictions. Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

If authenticated boot was disabled we allow generating the HUK using the SHA-256 of the DNA unique identifier. If authenticated boot was enabled, use the PUK KEK to generate the HUK instead. The PUF KEK must be registered while securing the board using the Xilinx tools. In this case, the HUK is generated by reading the DNA eFuses. This 96 bits value is used to generate a 16 byte digest which is then AES-GCM encrypted using the PUF KEK. The resulting 16 byte value is the HUK. To prevent the HUK from being leaked, the AES-GCM module must be reserved. The HUK generation was validated on Zynqmp zu3cg using the Xilinx Lightweight Provisioning Tool to enable authenticated boot and to provision the PUF (burning a number of eFuses in the process). Tested-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Tested-by: Ricardo Salveti <ricardo@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

Enable the RPMB key when the HUK is generated from the PUF KEK. Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Tested-by: Ricardo Salveti <ricardo@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

Tag core/drivers/zynqmp_* as maintained. Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

ldts · 2021-11-08T07:16:48Z

@jforissier rebased to fix conflict

jforissier · 2021-11-08T09:13:17Z

@ldts thanks!

ldts force-pushed the zynqmp-aes branch from c1dc620 to 9f2f484 Compare September 21, 2021 14:37

ldts closed this Sep 21, 2021

ldts reopened this Sep 21, 2021

jforissier reviewed Sep 22, 2021

View reviewed changes

ldts commented Sep 26, 2021

View reviewed changes

ldts force-pushed the zynqmp-aes branch from 9f2f484 to 8a4e633 Compare September 29, 2021 15:02

jforissier reviewed Sep 29, 2021

View reviewed changes

vesajaaskelainen reviewed Oct 1, 2021

View reviewed changes

ricardosalveti reviewed Oct 31, 2021

View reviewed changes

ldts force-pushed the zynqmp-aes branch from 47c0543 to faa6ebc Compare November 2, 2021 07:15

etienne-lms reviewed Nov 2, 2021

View reviewed changes

ldts force-pushed the zynqmp-aes branch from faa6ebc to 430b276 Compare November 2, 2021 20:55

jforissier reviewed Nov 5, 2021

View reviewed changes

ldts force-pushed the zynqmp-aes branch from 430b276 to 9c4d847 Compare November 5, 2021 13:47

ldts added 11 commits November 8, 2021 08:14

zynqmp: add base address definitions

c4d1024

Add the base address definitions for the CSU and the CSUDMA modules Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Reviewed-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

zynqmp: define the STACK_ALIGNMENT in terms of CACHELINE

205185f

Explicitily define the cache line length Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

zynqmp: drivers: CSU module base definitions

b0994fd

CSU registers and offsets for submodules Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Reviewed-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

zynqmp: platform: use HUK derived from PUF KEK for RPMB

75b8096

Enable the RPMB key when the HUK is generated from the PUF KEK. Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Tested-by: Ricardo Salveti <ricardo@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

MAINTAINERS: maintain zynqmp drivers

1e3a425

Tag core/drivers/zynqmp_* as maintained. Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

ldts force-pushed the zynqmp-aes branch from 9c4d847 to 1e3a425 Compare November 8, 2021 07:16

jforissier merged commit ddb245f into OP-TEE:master Nov 8, 2021

ldts deleted the zynqmp-aes branch May 12, 2022 09:38

ldts mentioned this pull request May 19, 2022

Security of RPMB_KEY #5312

Closed

zynqmp: HUK and RPMB ready #4874

zynqmp: HUK and RPMB ready #4874

Conversation

ldts commented Sep 21, 2021 • edited Loading

jforissier left a comment

Choose a reason for hiding this comment

ldts left a comment • edited Loading

Choose a reason for hiding this comment

ldts commented Sep 29, 2021 • edited Loading

ldts commented Sep 29, 2021 • edited Loading

jforissier 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

vesajaaskelainen commented Oct 1, 2021

vesajaaskelainen Oct 1, 2021 • 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

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

vesajaaskelainen Oct 1, 2021 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vesajaaskelainen Oct 1, 2021 • 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

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

vesajaaskelainen Oct 1, 2021 • 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

vesajaaskelainen Oct 1, 2021 • 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

Choose a reason for hiding this comment

vesajaaskelainen commented Oct 1, 2021

vesajaaskelainen commented Oct 1, 2021 • edited Loading

ldts commented Oct 1, 2021 • edited Loading

ldts commented Oct 1, 2021 • edited Loading

vesajaaskelainen commented Oct 1, 2021

vesajaaskelainen commented Oct 1, 2021

vesajaaskelainen commented Oct 1, 2021

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 Nov 1, 2021

ricardosalveti commented Nov 2, 2021

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

ldts commented Sep 21, 2021 •

edited

Loading

ldts left a comment •

edited

Loading

ldts commented Sep 29, 2021 •

edited

Loading

ldts commented Sep 29, 2021 •

edited

Loading

vesajaaskelainen Oct 1, 2021 •

edited

Loading

vesajaaskelainen Oct 1, 2021 •

edited

Loading

vesajaaskelainen Oct 1, 2021 •

edited

Loading

vesajaaskelainen Oct 1, 2021 •

edited

Loading

vesajaaskelainen Oct 1, 2021 •

edited

Loading

vesajaaskelainen commented Oct 1, 2021 •

edited

Loading

ldts commented Oct 1, 2021 •

edited

Loading

ldts commented Oct 1, 2021 •

edited

Loading