Re: [Buildroot] [PATCH v9 07/11] package/ti-core-secdev-k3: new package

[Patrick Oppenlander <patrick.oppenlander@gmail.com>]

On Fri, Jun 23, 2023 at 2:06 AM Andreas Dannenberg via buildroot
<buildroot@buildroot.org> wrote:
>
> The ti-core-secdev-k3 package is used to provide binary image signing
> tools and keys to the build process needed to build boot artifacts for
> the secure boot flow on TI K3 platform "High Security" SoCs (device
> variants "HS-FS" and "HS-SE"). This package is not needed building for
> "General Purpose" ("GP") SoC variants.
>
> This commit also updates the ti-k3-image-gen, ti-k3-r5-loader, and uboot
> packages which are all used as part of the boot flow of TI K3 platform
> devices to make use of the ti-core-secdev-k3 package if enabled.
>
> Note that although the use of the underlying 'core-secdev-k3' tool
> provided by TI is required to generate bootable images for HS-FS and
> HS-SE device variants, the use of this Buildroot package itself should
> remain optional, hence no hard dependencies are being established. The
> reason is that a user often wants to provide their own signing tool
> through the use of the TI_SECURE_DEV_PKG environmental variable set
> outside Buildroot on their specific build machine, especially for HS-SE
> device variants where the signing tool would contain the user's private
> keys.
>
> https://git.ti.com/cgit/security-development-tools/core-secdev-k3
>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> ---
>  boot/ti-k3-image-gen/ti-k3-image-gen.mk       | 10 ++++++
>  boot/ti-k3-r5-loader/ti-k3-r5-loader.mk       | 10 ++++++
>  boot/uboot/uboot.mk                           | 10 ++++++
>  package/Config.in.host                        |  1 +
>  package/ti-core-secdev-k3/Config.in.host      | 11 +++++++
>  .../ti-core-secdev-k3/ti-core-secdev-k3.hash  |  3 ++
>  .../ti-core-secdev-k3/ti-core-secdev-k3.mk    | 31 +++++++++++++++++++
>  7 files changed, 76 insertions(+)
>  create mode 100644 package/ti-core-secdev-k3/Config.in.host
>  create mode 100644 package/ti-core-secdev-k3/ti-core-secdev-k3.hash
>  create mode 100644 package/ti-core-secdev-k3/ti-core-secdev-k3.mk
>
> diff --git a/boot/ti-k3-image-gen/ti-k3-image-gen.mk b/boot/ti-k3-image-gen/ti-k3-image-gen.mk
> index b624f93771..57202d6918 100644
> --- a/boot/ti-k3-image-gen/ti-k3-image-gen.mk
> +++ b/boot/ti-k3-image-gen/ti-k3-image-gen.mk
> @@ -70,6 +70,16 @@ TI_K3_IMAGE_GEN_MAKE_OPTS = \
>         O=$(@D)/tmp \
>         BIN_DIR=$(@D)
>
> +ifneq ($(TI_CORE_SECDEV_K3_INSTALL_DIR),)
> +# Only set TI_SECURE_DEV_PKG make option if not already defined in the
> +# environment, thus allowing the user to unconditionally override this
> +# setting with a custom location on their build machine containing their
> +# private keys, etc.
> +ifeq ($(TI_SECURE_DEV_PKG),)
> +TI_K3_IMAGE_GEN_MAKE_OPTS += TI_SECURE_DEV_PKG=$(TI_CORE_SECDEV_K3_INSTALL_DIR)
> +endif
> +endif
> +
>  define TI_K3_IMAGE_GEN_BUILD_CMDS
>         $(TI_K3_IMAGE_GEN_MAKE) -C $(@D) $(TI_K3_IMAGE_GEN_MAKE_OPTS)
>  endef
> diff --git a/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk b/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk
> index 341888623e..8311e1b401 100644
> --- a/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk
> +++ b/boot/ti-k3-r5-loader/ti-k3-r5-loader.mk
> @@ -67,6 +67,16 @@ TI_K3_R5_LOADER_MAKE_OPTS = \
>         HOSTCC="$(HOSTCC) $(subst -I/,-isystem /,$(subst -I /,-isystem /,$(HOST_CFLAGS)))" \
>         HOSTLDFLAGS="$(HOST_LDFLAGS)"
>
> +ifneq ($(TI_CORE_SECDEV_K3_INSTALL_DIR),)
> +# Only set TI_SECURE_DEV_PKG make option if not already defined in the
> +# environment, thus allowing the user to unconditionally override this
> +# setting with a custom location on their build machine containing their
> +# private keys, etc.
> +ifeq ($(TI_SECURE_DEV_PKG),)
> +TI_K3_R5_LOADER_MAKE_OPTS += TI_SECURE_DEV_PKG=$(TI_CORE_SECDEV_K3_INSTALL_DIR)
> +endif
> +endif
> +
>  define TI_K3_R5_LOADER_BUILD_CMDS
>         $(TARGET_CONFIGURE_OPTS) $(TI_K3_R5_LOADER_MAKE) -C $(@D) $(TI_K3_R5_LOADER_MAKE_OPTS)
>  endef
> diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
> index 48af69bd26..472fec8619 100644
> --- a/boot/uboot/uboot.mk
> +++ b/boot/uboot/uboot.mk
> @@ -184,6 +184,16 @@ UBOOT_DEPENDENCIES += optee-os
>  UBOOT_MAKE_OPTS += TEE=$(BINARIES_DIR)/tee.elf
>  endif
>
> +ifneq ($(TI_CORE_SECDEV_K3_INSTALL_DIR),)
> +# Only set TI_SECURE_DEV_PKG make option if not already defined in the
> +# environment, thus allowing the user to unconditionally override this
> +# setting with a custom location on their build machine containing their
> +# private keys, etc.
> +ifeq ($(TI_SECURE_DEV_PKG),)
> +UBOOT_MAKE_OPTS += TI_SECURE_DEV_PKG=$(TI_CORE_SECDEV_K3_INSTALL_DIR)
> +endif
> +endif
> +
>  ifeq ($(BR2_TARGET_UBOOT_NEEDS_TI_K3_DM),y)
>  # Currently supports the FW from Git tag 08.06.00.006 by default
>  TI_K3_DM_VERSION = 340194800a581baf976360386dfc7b5acab8d948
> diff --git a/package/Config.in.host b/package/Config.in.host
> index dcadbfdfc1..eed39a4102 100644
> --- a/package/Config.in.host
> +++ b/package/Config.in.host
> @@ -103,6 +103,7 @@ menu "Host utilities"
>         source "package/systemd/Config.in.host"
>         source "package/tegrarcm/Config.in.host"
>         source "package/ti-cgt-pru/Config.in.host"
> +       source "package/ti-core-secdev-k3/Config.in.host"
>         source "package/uboot-tools/Config.in.host"
>         source "package/util-linux/Config.in.host"
>         source "package/utp_com/Config.in.host"
> diff --git a/package/ti-core-secdev-k3/Config.in.host b/package/ti-core-secdev-k3/Config.in.host
> new file mode 100644
> index 0000000000..364619d824
> --- /dev/null
> +++ b/package/ti-core-secdev-k3/Config.in.host
> @@ -0,0 +1,11 @@
> +config BR2_PACKAGE_HOST_TI_CORE_SECDEV_K3
> +       bool "host ti-core-secdev-k3"
> +       help
> +         ti-core-secdev-k3 is used to provide binary image signing
> +         tools and keys to the build process needed to build boot
> +         artifacts for the secure boot flow on TI K3 platform
> +         "High Security" SoCs (device variants "HS-FS" and "HS-SE").
> +         This package is not needed building for "General Purpose"
> +         ("GP") SoC variants.
> +
> +         https://git.ti.com/cgit/security-development-tools/core-secdev-k3
> diff --git a/package/ti-core-secdev-k3/ti-core-secdev-k3.hash b/package/ti-core-secdev-k3/ti-core-secdev-k3.hash
> new file mode 100644
> index 0000000000..526ed29514
> --- /dev/null
> +++ b/package/ti-core-secdev-k3/ti-core-secdev-k3.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256  eb637ed54204b64e98ae07070e0f2ebd36eed228ecc108dae0e7be6e38edde74  core-secdev-k3-08.06.00.007.tar.gz
> +sha256  3e5cf4f5ab9f0333f46cd68fabede3f21e55de1a9e3c6ad673f241f4514d8369  manifest/k3-secdev-0.2-manifest.html
> diff --git a/package/ti-core-secdev-k3/ti-core-secdev-k3.mk b/package/ti-core-secdev-k3/ti-core-secdev-k3.mk
> new file mode 100644
> index 0000000000..c388af2865
> --- /dev/null
> +++ b/package/ti-core-secdev-k3/ti-core-secdev-k3.mk
> @@ -0,0 +1,31 @@
> +################################################################################
> +#
> +# ti-core-secdev-k3
> +#
> +################################################################################
> +
> +TI_CORE_SECDEV_K3_VERSION = 08.06.00.007
> +TI_CORE_SECDEV_K3_SITE = https://git.ti.com/cgit/security-development-tools/core-secdev-k3/snapshot
> +TI_CORE_SECDEV_K3_LICENSE = TI TSPA License
> +TI_CORE_SECDEV_K3_LICENSE_FILES = manifest/k3-secdev-0.2-manifest.html
> +TI_CORE_SECDEV_K3_SOURCE = core-secdev-k3-$(TI_CORE_SECDEV_K3_VERSION).tar.gz

Hi Andreas,

Would it make sense to support user customisation of this via Kconfig
instead of using the magic TI_SECURE_DEV_PKG environment variable?

I'm slightly confused about how this is intended to work.
core-secdev-k3 contains both infrastructure (scripts) and data (keys)
to secure the image. It looks like someone tried to split these
concerns by supporting customisation of the key location.
secure-binary-image.sh expects TI_SECURE_DEV_PKG to point to something
containing keys/custMpk.pem, but the rest of the process expects
TI_SECURE_DEV_PKG to point to something containing scripts to secure
the binary blobs.

It would make more sense to me to support overriding just the key,
rather than having to duplicate all the scripts too.

Could we support configuring the location of the custMpk.pem key and
have the build process replace the default key in
build/host-ti-core-secdev-k3-08.06.00.007/keys?

Is there a reason why a customer is expected to replace (or, more
likely, duplicate) the scripts rather than just supply a key?

Patrick

> +# To allow the image signing process for various firmware artifacts to work the
> +# build process for TI K3 platform HS-FS and HS-SE device variants is using the
> +# 'core-secdev-k3' tool provided by TI. Its location must be made available to
> +# the build process of dependent packages by exporting it through the use of an
> +# environmental variable. In order to not pollute the global Buildroot
> +# environment let's record the package's location and then define the actual
> +# environmental variable needed for the build only in the packages that need it.
> +TI_CORE_SECDEV_K3_INSTALL_DIR = $(HOST_DIR)/opt/ti-core-secdev-k3
> +
> +define HOST_TI_CORE_SECDEV_K3_INSTALL_CMDS
> +       mkdir -p $(TI_CORE_SECDEV_K3_INSTALL_DIR)/keys
> +       cp -dpfr $(@D)/keys/* $(TI_CORE_SECDEV_K3_INSTALL_DIR)/keys
> +       mkdir -p $(TI_CORE_SECDEV_K3_INSTALL_DIR)/scripts
> +       cp -dpfr $(@D)/scripts/* $(TI_CORE_SECDEV_K3_INSTALL_DIR)/scripts
> +       mkdir -p $(TI_CORE_SECDEV_K3_INSTALL_DIR)/templates
> +       cp -dpfr $(@D)/scripts/* $(TI_CORE_SECDEV_K3_INSTALL_DIR)/templates
> +endef
> +
> +$(eval $(host-generic-package))
> --
> 2.34.1
>
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot