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

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Andreas, Patrick, All,

On 2023-06-23 09:53 -0500, Andreas Dannenberg via buildroot spake thusly:
> On Fri, Jun 23, 2023 at 01:48:30PM +1000, Patrick Oppenlander wrote:
[--SNIP--]
> > > 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

I see this version present (either used or referenced) in multiple
packages. Would it be possible, and would it make sense to share a
it in a single location?

This is going to be a bit tricky, because the variable must be set
_before_ any package that uses it gets parsed. WE usually ensure this by
having all packages in a common directory (like the Qt5 stuff in
package/qt5/), but here we have packages in package/ and in boot/, so I
can see a little issue...

> > > +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
[--SNIP--]
> > Would it make sense to support user customisation of this via Kconfig
> > instead of using the magic TI_SECURE_DEV_PKG environment variable?
[--SNIP--]
> > 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?
> Well, this is how the "TI secure dev package" has been for the last 10+
> years, and that's how it's being used by 100% of our customers today
> doing Linux image builds. It's hard-wired into Yocto builds, even the
> official U-Boot tree...
> https://source.denx.de/u-boot/u-boot/-/blob/master/doc/README.ti-secure
> I agree improvements could be made for example to point to just a
> private key via Kconfig but this would also create an inconsistency with
> all solutions currently in production, and with what folks are used to,
> and go against enabling an easier and more seamless "switch" from Yocto
> for example, which is what I'd like people to do.
> 
> This doesn't say it should not or never be improved, but I think such
> changes could be managed more smoothly by establishing a known baseline
> first (which is what this entire series intends to do, based on a
> comparable variant of the official TI production SDK), and then over
> time improve things, also as Yocto, U-Boot, and other projects evolve
> (there's constant development on those including on the security front,
> so what we have here will not be the "forever solution).
>  
> > Is there a reason why a customer is expected to replace (or, more
> > likely, duplicate) the scripts rather than just supply a key?
> The key is the most obvious thing, but often customers also want to
> customize the certificate to add additional items for example to control
> certain device features such as whether JTAG should be locked or
> unlocked, etc., all things not readily controllable through some knobs
> but needing to be done through customization of sripts / templates.

I've read the whole exchange, and it was very instructive, thanks!

I'd side with Andreas here: let's get the basic support in, which does
behave like people are currently used to.

If it then is possible to customise the security settings (key, certs,
etc...), either following future upstream changes, or in a
Buildroot-specific way, then we can improve later on.

> +TI_CORE_SECDEV_K3_INSTALL_DIR = $(HOST_DIR)/opt/ti-core-secdev-k3

This variable is set unconditionally, even if the package is not
enabled. So, the conditional blocks you added in the other packages will
always hit.

Here's one sich block below:

> +ifneq ($(TI_CORE_SECDEV_K3_INSTALL_DIR),)

As said above, this condition will always be true.

> +# 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

So, as I understand it, the user has two options:

  - enable BR2_PACKAGE_HOST_TI_CORE_SECDEV_K3=y and use it as a signing
    tool,

  - disable BR2_PACKAGE_HOST_TI_CORE_SECDEV_K3 and set TI_SECURE_DEV_PKG
    in their environment, to point to their signing tool

My suggestion would be the following:

 - if the user enables BR2_PACKAGE_HOST_TI_CORE_SECDEV_K3=y then we use
   that as a signing tool, and forcefully set TI_SECURE_DEV_PKG;

  - otherwise, we do nothing: if they have TI_SECURE_DEV_PKG set in
    their environment, it will be used as a signing tool;

  - otherwise, no signing will be done.

I.e. the code blocks would look like (e.g. for uboot):

    ifeq ($(BR2_PACKAGE_HOST_TI_CORE_SECDEV_K3),y)
    UBOOT_MAKE_OPTS += TI_SECURE_DEV_PKG=$(TI_CORE_SECDEV_K3_INSTALL_DIR)
    endif

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot