[PATCH 2/2] test_vboot.py: include test of fdt_add_pubkey tool

2021-11-11 Thread Roman Kopytin
Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 
---
 test/py/tests/test_vboot.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 095e00cce3..0e9b158e00 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -232,6 +232,13 @@ def test_vboot(u_boot_console, name, sha_algo, padding, 
sign_options, required,
 
 util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
 
+# Create a fresh .dtb without the public keys
+dtc('sandbox-u-boot.dts')
+# Then add the dev key via the fdt_add_pubkey tool
+util.run_and_log(cons, [fdt_add_pubkey, '-a', '%s,rsa2048' % sha_algo,
+'-k', tmpdir, '-n', 'dev', '-r', 'conf', dtb])
+util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
+
 if full_test:
 # Make sure that U-Boot checks that the config is in the list of
 # hashed nodes. If it isn't, a security bypass is possible.
@@ -373,6 +380,7 @@ def test_vboot(u_boot_console, name, sha_algo, padding, 
sign_options, required,
 fit = '%stest.fit' % tmpdir
 mkimage = cons.config.build_dir + '/tools/mkimage'
 fit_check_sign = cons.config.build_dir + '/tools/fit_check_sign'
+fdt_add_pubkey = cons.config.build_dir + '/tools/fdt_add_pubkey'
 dtc_args = '-I dts -O dtb -i %s' % tmpdir
 dtb = '%ssandbox-u-boot.dtb' % tmpdir
 sig_node = '/configurations/conf-1/signature'
-- 
2.25.1



[PATCH 1/2] tools: add fdt_add_pubkey

2021-11-11 Thread Roman Kopytin
Having to use the -K option to mkimage to populate U-Boot's .dtb with the
public key while signing the kernel FIT image is often a little
awkward. In particular, when using a meta-build system such as
bitbake/Yocto, having the tasks of the kernel and U-Boot recipes
intertwined, modifying deployed artifacts and rebuilding U-Boot with
an updated .dtb is quite cumbersome. Also, in some scenarios one may
wish to build U-Boot complete with the public key(s) embedded in the
.dtb without the corresponding private keys being present on the same
build host.

So this adds a simple tool that allows one to disentangle the kernel
and U-Boot builds, by simply copy-pasting just enough of the mkimage
code to allow one to add a public key to a .dtb. When using mkimage,
some of the information is taken from the .its used to build the
kernel (algorithm and key name), so that of course needs to be
supplied on the command line.

Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 
---
 tools/.gitignore   |   1 +
 tools/Makefile |   3 +
 tools/fdt_add_pubkey.c | 130 +
 3 files changed, 134 insertions(+)
 create mode 100644 tools/fdt_add_pubkey.c

diff --git a/tools/.gitignore b/tools/.gitignore
index a88453f64d..f312b760e4 100644
--- a/tools/.gitignore
+++ b/tools/.gitignore
@@ -6,6 +6,7 @@
 /dumpimage
 /easylogo/easylogo
 /envcrc
+/fdt_add_pubkey
 /fdtgrep
 /file2include
 /fit_check_sign
diff --git a/tools/Makefile b/tools/Makefile
index b45219e2c3..c142c48e73 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -73,6 +73,7 @@ mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
 
 hostprogs-y += dumpimage mkimage
 hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign
+hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fdt_add_pubkey
 
 hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
 
@@ -154,6 +155,7 @@ dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
 mkimage-objs   := $(dumpimage-mkimage-objs) mkimage.o
 fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
 fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
+fdt_add_pubkey-objs   := $(dumpimage-mkimage-objs) fdt_add_pubkey.o
 file2include-objs := file2include.o
 
 ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_LIBCRYPTO),)
@@ -191,6 +193,7 @@ HOSTCFLAGS_fit_image.o += 
-DMKIMAGE_DTC=\"$(CONFIG_MKIMAGE_DTC_PATH)\"
 HOSTLDLIBS_dumpimage := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_info := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_check_sign := $(HOSTLDLIBS_mkimage)
+HOSTLDLIBS_fdt_add_pubkey := $(HOSTLDLIBS_mkimage)
 
 hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
 hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl
diff --git a/tools/fdt_add_pubkey.c b/tools/fdt_add_pubkey.c
new file mode 100644
index 00..96099312e4
--- /dev/null
+++ b/tools/fdt_add_pubkey.c
@@ -0,0 +1,130 @@
+#include 
+#include "fit_common.h"
+
+static const char *cmdname;
+
+static const char *algo_name = "sha1,rsa2048"; /* -a  */
+static const char *keydir = "."; /* -k  */
+static const char *keyname = "key"; /* -n  */
+static const char *require_keys; /* -r  */
+static const char *keydest; /* argv[n] */
+
+static void print_usage(const char *msg)
+{
+   fprintf(stderr, "Error: %s\n", msg);
+   fprintf(stderr, "Usage: %s [-a ] [-k ] [-n ] [-r 
] \n",
+   cmdname);
+   fprintf(stderr, "Help information: %s [-h]\n", cmdname);
+   exit(EXIT_FAILURE);
+}
+
+static void print_help()
+{
+   fprintf(stderr, "Options:\n"
+   "\t-aCryptographic algorithm. Optional parameter, 
default value: sha1,rsa2048\n"
+   "\t-k  Directory with public key. Optional 
parameter, default value: .\n"
+   "\t-n Public key name. Optional parameter, default 
value: key\n"
+   "\t-r  Required: If present this indicates that the 
key must be verified for the image / configuration to be considered valid.\n"
+   "\t  FDT blob file for adding of the public key. 
Required parameter.\n");
+   exit(EXIT_FAILURE);
+}
+
+static void process_args(int argc, char *argv[])
+{
+   int opt;
+
+   while((opt = getopt(argc, argv, "a:k:n:r:h")) != -1) {
+   switch (opt) {
+   case 'k':
+   keydir = optarg;
+   break;
+   case 'a':
+   algo_name = optarg;
+   break;
+   case 'n':
+   keyname = optarg;
+   break;
+   case 'r':
+   require_keys = optarg;
+   break;
+   case 'h':
+   print_help();
+   default:
+   print_usage("Invalid opt

[PATCH 0/2] RFC: add fdt_add_pubkey tool

2021-11-11 Thread Roman Kopytin
In order to reduce the coupling between building the kernel and
U-Boot, I'd like a tool that can add a public key to U-Boot's dtb
without simultaneously signing a FIT image. That tool doesn't seem to
exist, so I stole the necessary pieces from mkimage et al and put it
in a single .c file.

I'm still working on the details of my proposed "require just k out
these n required keys" and how it should be implemented, but it will
probably involve teaching this tool a bunch of new options. These
patches are not necessarily ready for inclusion (unless someone else
finds fdt_add_pubkey useful as is), but I thought I might as well send
it out for early comments.

Roman Kopytin (2):
  tools: add fdt_add_pubkey
  test_vboot.py: include test of fdt_add_pubkey tool

 test/py/tests/test_vboot.py |   8 +++
 tools/.gitignore|   1 +
 tools/Makefile  |   3 +
 tools/fdt_add_pubkey.c  | 130 
 4 files changed, 142 insertions(+)
 create mode 100644 tools/fdt_add_pubkey.c

-- 
2.25.1



RE: [PATCH 1/2] tools: add fdt_add_pubkey

2021-11-10 Thread Roman Kopytin
Thanks, I found example in fdtgrep.
What do you think about function like:

static void print_usage(const char *msg)
{
if (msg != NULL)
fprintf(stderr, "Error: %s\n", msg);
fprintf(stderr, "Usage: %s [-a ] [-k ] [-n ] [-r 
] \n"
"Options:\n"

"\t-aCryptographic algorithm. Optional parameter, 
default: sha1,rsa2048\n"
"\t-k  Directory with public key. Optional 
parameter, default: .\n"
"\t-n Public key name. Optional parameter, 
default: key\n"
"\t-r  Required: If present this indicates that the 
key must be verified for the image / configuration to be considered valid\n"
"\t  FDT blob file for adding of the public key. 
Required parameter.\n",
cmdname);
exit(EXIT_FAILURE);
}


Is it ok?

-Original Message-
From: Jan Kiszka  
Sent: Wednesday, November 10, 2021 10:22 PM
To: Roman Kopytin ; u-boot@lists.denx.de
Cc: Rasmus Villemoes 
Subject: Re: [PATCH 1/2] tools: add fdt_add_pubkey

On 10.11.21 09:26, Roman Kopytin wrote:
> Could you please provide good example with needed style for helper?
> In tools I saw a lot of programs w/o help.
> 

Have a look at binman to see this full-blown - not a completely fair comparison 
as it benefits from Python argparse.

Jan

--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux


RE: [PATCH 1/2] tools: add fdt_add_pubkey

2021-11-10 Thread Roman Kopytin
Could you please provide good example with needed style for helper?
In tools I saw a lot of programs w/o help.

-Original Message-
From: Jan Kiszka  
Sent: Wednesday, November 10, 2021 10:39 AM
To: Roman Kopytin ; u-boot@lists.denx.de
Cc: Rasmus Villemoes 
Subject: Re: [PATCH 1/2] tools: add fdt_add_pubkey

On 08.11.21 16:28, Roman Kopytin wrote:
> Having to use the -K option to mkimage to populate U-Boot's .dtb with 
> the public key while signing the kernel FIT image is often a little 
> awkward. In particular, when using a meta-build system such as 
> bitbake/Yocto, having the tasks of the kernel and U-Boot recipes 
> intertwined, modifying deployed artifacts and rebuilding U-Boot with 
> an updated .dtb is quite cumbersome. Also, in some scenarios one may 
> wish to build U-Boot complete with the public key(s) embedded in the 
> .dtb without the corresponding private keys being present on the same 
> build host.
> 
> So this adds a simple tool that allows one to disentangle the kernel 
> and U-Boot builds, by simply copy-pasting just enough of the mkimage 
> code to allow one to add a public key to a .dtb. When using mkimage, 
> some of the information is taken from the .its used to build the 
> kernel (algorithm and key name), so that of course needs to be 
> supplied on the command line.
> 
> Signed-off-by: Roman Kopytin 
> Cc: Rasmus Villemoes 
> ---
>  tools/.gitignore   |  1 +
>  tools/Makefile |  3 ++
>  tools/fdt_add_pubkey.c | 97 
> ++
>  3 files changed, 101 insertions(+)
>  create mode 100755 tools/fdt_add_pubkey.c
> 
> diff --git a/tools/.gitignore b/tools/.gitignore index 
> a88453f64d..f312b760e4 100644
> --- a/tools/.gitignore
> +++ b/tools/.gitignore
> @@ -6,6 +6,7 @@
>  /dumpimage
>  /easylogo/easylogo
>  /envcrc
> +/fdt_add_pubkey
>  /fdtgrep
>  /file2include
>  /fit_check_sign
> diff --git a/tools/Makefile b/tools/Makefile index 
> 4a86321f64..44f25dda18 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -73,6 +73,7 @@ mkenvimage-objs := mkenvimage.o os_support.o 
> lib/crc32.o
>  
>  hostprogs-y += dumpimage mkimage
>  hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign
> +hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fdt_add_pubkey
>  
>  hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
>  
> @@ -153,6 +154,7 @@ dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
>  mkimage-objs   := $(dumpimage-mkimage-objs) mkimage.o
>  fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
>  fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
> +fdt_add_pubkey-objs   := $(dumpimage-mkimage-objs) fdt_add_pubkey.o
>  file2include-objs := file2include.o
>  
>  ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_LIBCRYPTO),)
> @@ -190,6 +192,7 @@ HOSTCFLAGS_fit_image.o += 
> -DMKIMAGE_DTC=\"$(CONFIG_MKIMAGE_DTC_PATH)\"
>  HOSTLDLIBS_dumpimage := $(HOSTLDLIBS_mkimage)  HOSTLDLIBS_fit_info := 
> $(HOSTLDLIBS_mkimage)  HOSTLDLIBS_fit_check_sign := 
> $(HOSTLDLIBS_mkimage)
> +HOSTLDLIBS_fdt_add_pubkey := $(HOSTLDLIBS_mkimage)
>  
>  hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
>  hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl diff --git 
> a/tools/fdt_add_pubkey.c b/tools/fdt_add_pubkey.c new file mode 100755 
> index 00..9306ecedd1
> --- /dev/null
> +++ b/tools/fdt_add_pubkey.c
> @@ -0,0 +1,97 @@
> +#include 
> +#include "fit_common.h"
> +
> +static const char *cmdname;
> +
> +static const char *algo_name = "sha1,rsa2048"; /* -a  */ static 
> +const char *keydir = "."; /* -k  */ static const char 
> +*keyname = "key"; /* -n  */ static const char *require_keys; 
> +/* -r  */ static const char *keydest; /* argv[n] */
> +
> +static void usage(const char *msg)
> +{
> + fprintf(stderr, "Error: %s\n", msg);
> + fprintf(stderr, "Usage: %s [-a ] [-k ] [-n ] [-r 
> ] \n",
> + cmdname);

The tool should support --help and document the default of these optional 
parameters that way.

Is there an easy way to derive algo from the key?

Jan

--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux


RE: [PATCH 1/2] tools: add fdt_add_pubkey

2021-11-09 Thread Roman Kopytin
Hi, Simon
I have question about:
Some of these are not optional so should not have [] around them.

As I see we have defaults for:
static const char *algo_name = "sha1,rsa2048"; /* -a  */
static const char *keydir = "."; /* -k  */
static const char *keyname = "key"; /* -n  */

It means that we can skip it in command line. Should I need to remove [] from 
code for those parameters?

-Original Message-
From: Simon Glass  
Sent: Wednesday, November 10, 2021 3:58 AM
To: Roman Kopytin 
Cc: u-boot@lists.denx.de; Rasmus Villemoes 
Subject: Re: [PATCH 1/2] tools: add fdt_add_pubkey

Hi Roman,

On Mon, 8 Nov 2021 at 08:29, Roman Kopytin  wrote:
>
> Having to use the -K option to mkimage to populate U-Boot's .dtb with 
> the public key while signing the kernel FIT image is often a little 
> awkward. In particular, when using a meta-build system such as 
> bitbake/Yocto, having the tasks of the kernel and U-Boot recipes 
> intertwined, modifying deployed artifacts and rebuilding U-Boot with 
> an updated .dtb is quite cumbersome. Also, in some scenarios one may 
> wish to build U-Boot complete with the public key(s) embedded in the 
> .dtb without the corresponding private keys being present on the same 
> build host.
>
> So this adds a simple tool that allows one to disentangle the kernel 
> and U-Boot builds, by simply copy-pasting just enough of the mkimage 
> code to allow one to add a public key to a .dtb. When using mkimage, 
> some of the information is taken from the .its used to build the 
> kernel (algorithm and key name), so that of course needs to be 
> supplied on the command line.
>
> Signed-off-by: Roman Kopytin 
> Cc: Rasmus Villemoes 
> ---
>  tools/.gitignore   |  1 +
>  tools/Makefile |  3 ++
>  tools/fdt_add_pubkey.c | 97 
> ++
>  3 files changed, 101 insertions(+)
>  create mode 100755 tools/fdt_add_pubkey.c
>
> diff --git a/tools/.gitignore b/tools/.gitignore index 
> a88453f64d..f312b760e4 100644
> --- a/tools/.gitignore
> +++ b/tools/.gitignore
> @@ -6,6 +6,7 @@
>  /dumpimage
>  /easylogo/easylogo
>  /envcrc
> +/fdt_add_pubkey
>  /fdtgrep
>  /file2include
>  /fit_check_sign
> diff --git a/tools/Makefile b/tools/Makefile index 
> 4a86321f64..44f25dda18 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -73,6 +73,7 @@ mkenvimage-objs := mkenvimage.o os_support.o 
> lib/crc32.o
>
>  hostprogs-y += dumpimage mkimage
>  hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign
> +hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fdt_add_pubkey
>
>  hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
>
> @@ -153,6 +154,7 @@ dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
>  mkimage-objs   := $(dumpimage-mkimage-objs) mkimage.o
>  fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
>  fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
> +fdt_add_pubkey-objs   := $(dumpimage-mkimage-objs) fdt_add_pubkey.o
>  file2include-objs := file2include.o
>
>  ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_LIBCRYPTO),)
> @@ -190,6 +192,7 @@ HOSTCFLAGS_fit_image.o += 
> -DMKIMAGE_DTC=\"$(CONFIG_MKIMAGE_DTC_PATH)\"
>  HOSTLDLIBS_dumpimage := $(HOSTLDLIBS_mkimage)  HOSTLDLIBS_fit_info := 
> $(HOSTLDLIBS_mkimage)  HOSTLDLIBS_fit_check_sign := 
> $(HOSTLDLIBS_mkimage)
> +HOSTLDLIBS_fdt_add_pubkey := $(HOSTLDLIBS_mkimage)
>
>  hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
>  hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl diff --git 
> a/tools/fdt_add_pubkey.c b/tools/fdt_add_pubkey.c new file mode 100755 
> index 00..9306ecedd1
> --- /dev/null
> +++ b/tools/fdt_add_pubkey.c
> @@ -0,0 +1,97 @@
> +#include 
> +#include "fit_common.h"
> +
> +static const char *cmdname;
> +
> +static const char *algo_name = "sha1,rsa2048"; /* -a  */ static 
> +const char *keydir = "."; /* -k  */ static const char 
> +*keyname = "key"; /* -n  */ static const char *require_keys; 
> +/* -r  */ static const char *keydest; /* argv[n] */
> +
> +static void usage(const char *msg)
> +{
> +   fprintf(stderr, "Error: %s\n", msg);
> +   fprintf(stderr, "Usage: %s [-a ] [-k ] [-n 
> +] [-r ] \n",

Some of these are not optional so should not have [] around them.

> +   cmdname);
> +   exit(EXIT_FAILURE);
> +}
> +
> +static void process_args(int argc, char *argv[]) {
> +   int opt;
> +
> +   while((opt = getopt(argc, argv, "a:k:n:r:")) != -1) {
> +   switch (opt) {
> +   case 'k':
> +   keydir = optarg;
&

RE: [PATCH 0/2] RFC: add fdt_add_pubkey tool

2021-11-09 Thread Roman Kopytin
Can we have discussion with code lines? For me it is not very clear, because it 
isn't my code.

-Original Message-
From: Jan Kiszka  
Sent: Tuesday, November 9, 2021 12:17 PM
To: Roman Kopytin ; u-boot@lists.denx.de; Rasmus 
Villemoes 
Subject: Re: [PATCH 0/2] RFC: add fdt_add_pubkey tool

On 08.11.21 16:28, Roman Kopytin wrote:
> In order to reduce the coupling between building the kernel and 
> U-Boot, I'd like a tool that can add a public key to U-Boot's dtb 
> without simultaneously signing a FIT image. That tool doesn't seem to 
> exist, so I stole the necessary pieces from mkimage et al and put it 
> in a single .c file.
> 
> I'm still working on the details of my proposed "require just k out 
> these n required keys" and how it should be implemented, but it will 
> probably involve teaching this tool a bunch of new options. These 
> patches are not necessarily ready for inclusion (unless someone else 
> finds fdt_add_pubkey useful as is), but I thought I might as well send 
> it out for early comments.

I'd also like to see the usage of this hooked into the build process.

And to my understanding of [1], that approach will provide a feature that 
permits hooking with the build but would expect the key as dtsi fragment. Can 
we consolidate the approaches?

My current vision of a user interface would be a Kconfig option that takes a 
list of key files to be injected. Maybe make that three lists, one for 
"required=image", one for "required=conf", and one for optional keys (if that 
has a use case in practice, no idea).

Jan

[1]
https://lore.kernel.org/u-boot/20210928085651.619892-1-rasmus.villem...@prevas.dk/

--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux


RE: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES

2021-11-08 Thread Roman Kopytin
Hi, I sent patches, please check.
But before correct emails I sent several test emails.

-Original Message-
From: Simon Glass  
Sent: Friday, November 5, 2021 5:02 AM
To: Roman Kopytin 
Cc: Rasmus Villemoes ; U-Boot Mailing List 
; Alex Kiernan ; Masahiro Yamada 

Subject: Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES

Hi Roman,

Good luck! I must get a copy of that BOFH book.

Regards,
Simon



On Sun, 31 Oct 2021 at 22:30, Roman Kopytin  wrote:
>
> Hi, all
> Currently I am waiting some help from our IT infrastructure department.
>
> -Original Message-
> From: Simon Glass 
> Sent: Tuesday, October 26, 2021 4:28 AM
> To: Rasmus Villemoes 
> Cc: U-Boot Mailing List ; Alex Kiernan 
> ; Roman Kopytin ; 
> Masahiro Yamada 
> Subject: Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES
>
> Hi Rasmus,
>
> On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes  
> wrote:
> >
> > The build system already automatically looks for and includes an 
> > in-tree *-u-boot.dtsi when building the control .dtb. However, there 
> > are some things that are awkward to maintain in such an in-tree 
> > file, most notably the metadata associated to public keys used for 
> > verified boot.
> >
> > The only "official" API to get that metadata into the .dtb is via 
> > mkimage, as a side effect of building an actual signed image. But 
> > there are multiple problems with that. First of all, the final 
> > U-Boot (be it U-Boot proper or an SPL) image is built based on a 
> > binary image, the .dtb, and possibly some other binary artifacts. So 
> > modifying the .dtb after the build requires the meta-buildsystem 
> > (Yocto, buildroot, whatnot) to know about and repeat some of the 
> > steps that are already known to and handled by U-Boot's build 
> > system, resulting in needless duplication of code. It's also 
> > somewhat annoying and inconsistent to have a .dtb file in the build 
> > folder which is not generated by the command listed in the 
> > corresponding .cmd file (that of course applies to any generated file).
> >
> > So the contents of the /signature node really needs to be baked into 
> > the .dtb file when it is first created, which means providing the 
> > relevant data in the form of a .dtsi file. One could in theory put 
> > that data into the *-u-boot.dtsi file, but it's more convenient to 
> > be able to provide it externally: For example, when developing for a 
> > customer, it's common to use a set of dummy keys for development, 
> > while the consultants do not (and should not) have access to the 
> > actual keys used in production. For such a setup, it's easier if the 
> > keys used are chosen via the meta-buildsystem and the path(s) 
> > patched in during the configure step. And of course, nothing 
> > prevents anybody from having DEVICE_TREE_INCLUDES point at files 
> > maintained in git, or for that matter from including the public key 
> > metadata in the *-u-boot.dtsi directly and ignore this feature.
> >
> > There are other uses for this, e.g. in combination with 
> > ENV_IMPORT_FDT it can be used for providing the contents of the 
> > /config/environment node, so I don't want to tie this exclusively to 
> > use for verified boot.
> >
> > Signed-off-by: Rasmus Villemoes 
> > ---
> >
> > Getting the public key metadata into .dtsi form can be done with a 
> > little scripting (roughly 'mkimage -K' of a dummy image followed by 
> > 'dtc -I dtb -O dts'). I have a script that does that, along with 
> > options to set 'required' and 'required-mode' properties, include 
> > u-boot,dm-spl properties in case one wants to check U-Boot proper 
> > from SPL with the verified boot mechanism, etc. I'm happy to share 
> > that script if this gets accepted, but it's moot if this is rejected.
> >
> > I have previously tried to get an fdt_add_pubkey tool accepted [1,2] 
> > to disentangle the kernel and u-boot builds (or u-boot and SPL 
> > builds for that matter!), but as I've since realized, that isn't 
> > quite enough
> > - the above points re modifying the .dtb after it is created but 
> > before that is used to create further build artifacts still stand.
> > However, such a tool could still be useful for creating the .dtsi 
> > info without the private keys being present, and my key2dtsi.sh 
> > script could easily be modified to use a tool like that should it 
> > ever appear.
> >
> > [1]
> > https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemo
> > es
> &g

[PATCH 2/2] test_vboot.py: include test of fdt_add_pubkey tool

2021-11-08 Thread Roman Kopytin
Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 
---
 test/py/tests/test_vboot.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 6dff6779d1..cf7416b39a 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -230,6 +230,13 @@ def test_vboot(u_boot_console, sha_algo, padding, 
sign_options, required,
 cons.log.action('%s: Check signed config on the host' % sha_algo)
 
 util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
+
+# Create a fresh .dtb without the public keys
+dtc('sandbox-u-boot.dts')
+# Then add the dev key via the fdt_add_pubkey tool
+util.run_and_log(cons, [fdt_add_pubkey, '-a', '%s,rsa2048' % sha_algo,
+'-k', tmpdir, '-n', 'dev', '-r', 'conf', dtb])
+util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
 
 if full_test:
 # Make sure that U-Boot checks that the config is in the list of
@@ -370,6 +377,7 @@ def test_vboot(u_boot_console, sha_algo, padding, 
sign_options, required,
 fit = '%stest.fit' % tmpdir
 mkimage = cons.config.build_dir + '/tools/mkimage'
 fit_check_sign = cons.config.build_dir + '/tools/fit_check_sign'
+fdt_add_pubkey = cons.config.build_dir + '/tools/fdt_add_pubkey'
 dtc_args = '-I dts -O dtb -i %s' % tmpdir
 dtb = '%ssandbox-u-boot.dtb' % tmpdir
 sig_node = '/configurations/conf-1/signature'
-- 
2.25.1



[PATCH 1/2] tools: add fdt_add_pubkey

2021-11-08 Thread Roman Kopytin
Having to use the -K option to mkimage to populate U-Boot's .dtb with the
public key while signing the kernel FIT image is often a little
awkward. In particular, when using a meta-build system such as
bitbake/Yocto, having the tasks of the kernel and U-Boot recipes
intertwined, modifying deployed artifacts and rebuilding U-Boot with
an updated .dtb is quite cumbersome. Also, in some scenarios one may
wish to build U-Boot complete with the public key(s) embedded in the
.dtb without the corresponding private keys being present on the same
build host.

So this adds a simple tool that allows one to disentangle the kernel
and U-Boot builds, by simply copy-pasting just enough of the mkimage
code to allow one to add a public key to a .dtb. When using mkimage,
some of the information is taken from the .its used to build the
kernel (algorithm and key name), so that of course needs to be
supplied on the command line.

Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 
---
 tools/.gitignore   |  1 +
 tools/Makefile |  3 ++
 tools/fdt_add_pubkey.c | 97 ++
 3 files changed, 101 insertions(+)
 create mode 100755 tools/fdt_add_pubkey.c

diff --git a/tools/.gitignore b/tools/.gitignore
index a88453f64d..f312b760e4 100644
--- a/tools/.gitignore
+++ b/tools/.gitignore
@@ -6,6 +6,7 @@
 /dumpimage
 /easylogo/easylogo
 /envcrc
+/fdt_add_pubkey
 /fdtgrep
 /file2include
 /fit_check_sign
diff --git a/tools/Makefile b/tools/Makefile
index 4a86321f64..44f25dda18 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -73,6 +73,7 @@ mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
 
 hostprogs-y += dumpimage mkimage
 hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign
+hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fdt_add_pubkey
 
 hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
 
@@ -153,6 +154,7 @@ dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
 mkimage-objs   := $(dumpimage-mkimage-objs) mkimage.o
 fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
 fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
+fdt_add_pubkey-objs   := $(dumpimage-mkimage-objs) fdt_add_pubkey.o
 file2include-objs := file2include.o
 
 ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_LIBCRYPTO),)
@@ -190,6 +192,7 @@ HOSTCFLAGS_fit_image.o += 
-DMKIMAGE_DTC=\"$(CONFIG_MKIMAGE_DTC_PATH)\"
 HOSTLDLIBS_dumpimage := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_info := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_check_sign := $(HOSTLDLIBS_mkimage)
+HOSTLDLIBS_fdt_add_pubkey := $(HOSTLDLIBS_mkimage)
 
 hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
 hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl
diff --git a/tools/fdt_add_pubkey.c b/tools/fdt_add_pubkey.c
new file mode 100755
index 00..9306ecedd1
--- /dev/null
+++ b/tools/fdt_add_pubkey.c
@@ -0,0 +1,97 @@
+#include 
+#include "fit_common.h"
+
+static const char *cmdname;
+
+static const char *algo_name = "sha1,rsa2048"; /* -a  */
+static const char *keydir = "."; /* -k  */
+static const char *keyname = "key"; /* -n  */
+static const char *require_keys; /* -r  */
+static const char *keydest; /* argv[n] */
+
+static void usage(const char *msg)
+{
+   fprintf(stderr, "Error: %s\n", msg);
+   fprintf(stderr, "Usage: %s [-a ] [-k ] [-n ] [-r 
] \n",
+   cmdname);
+   exit(EXIT_FAILURE);
+}
+
+static void process_args(int argc, char *argv[])
+{
+   int opt;
+
+   while((opt = getopt(argc, argv, "a:k:n:r:")) != -1) {
+   switch (opt) {
+   case 'k':
+   keydir = optarg;
+   break;
+   case 'a':
+   algo_name = optarg;
+   break;
+   case 'n':
+   keyname = optarg;
+   break;
+   case 'r':
+   require_keys = optarg;
+   break;
+   default:
+   usage("Invalid option");
+   }
+   }
+   /* The last parameter is expected to be the .dtb to add the public key 
to */
+   if (optind < argc)
+   keydest = argv[optind];
+
+   if (!keydest)
+   usage("Missing dtb file to update");
+}
+
+int main(int argc, char *argv[])
+{
+   struct image_sign_info info;
+   int destfd, ret;
+   void *dest_blob = NULL;
+   struct stat dest_sbuf;
+   size_t size_inc = 0;
+
+   cmdname = argv[0];
+
+   process_args(argc, argv);
+
+   memset(&info, 0, sizeof(info));
+
+   info.keydir = keydir;
+   info.keyname = keyname;
+   info.name = algo_name;
+   info.require_keys = require_keys;
+   info.crypto = image_get_crypto_algo(algo_name);
+   if (!info.crypto) {
+fprintf(stderr, "Unsupported signature alg

[PATCH 0/2] RFC: add fdt_add_pubkey tool

2021-11-08 Thread Roman Kopytin
In order to reduce the coupling between building the kernel and
U-Boot, I'd like a tool that can add a public key to U-Boot's dtb
without simultaneously signing a FIT image. That tool doesn't seem to
exist, so I stole the necessary pieces from mkimage et al and put it
in a single .c file.

I'm still working on the details of my proposed "require just k out
these n required keys" and how it should be implemented, but it will
probably involve teaching this tool a bunch of new options. These
patches are not necessarily ready for inclusion (unless someone else
finds fdt_add_pubkey useful as is), but I thought I might as well send
it out for early comments.

Roman Kopytin (2):
  tools: add fdt_add_pubkey
  test_vboot.py: include test of fdt_add_pubkey tool

 test/py/tests/test_vboot.py |  8 +++
 tools/.gitignore|  1 +
 tools/Makefile  |  3 ++
 tools/fdt_add_pubkey.c  | 97 +
 4 files changed, 109 insertions(+)
 create mode 100755 tools/fdt_add_pubkey.c

-- 
2.25.1



[PATCH 1/2] tools: add fdt_add_pubkey

2021-11-08 Thread Roman Kopytin
Having to use the -K option to mkimage to populate U-Boot's .dtb with the
public key while signing the kernel FIT image is often a little
awkward. In particular, when using a meta-build system such as
bitbake/Yocto, having the tasks of the kernel and U-Boot recipes
intertwined, modifying deployed artifacts and rebuilding U-Boot with
an updated .dtb is quite cumbersome. Also, in some scenarios one may
wish to build U-Boot complete with the public key(s) embedded in the
.dtb without the corresponding private keys being present on the same
build host.

So this adds a simple tool that allows one to disentangle the kernel
and U-Boot builds, by simply copy-pasting just enough of the mkimage
code to allow one to add a public key to a .dtb. When using mkimage,
some of the information is taken from the .its used to build the
kernel (algorithm and key name), so that of course needs to be
supplied on the command line.

Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 
---
 tools/.gitignore   |  1 +
 tools/Makefile |  3 ++
 tools/fdt_add_pubkey.c | 97 ++
 3 files changed, 101 insertions(+)
 create mode 100755 tools/fdt_add_pubkey.c

diff --git a/tools/.gitignore b/tools/.gitignore
index a88453f64d..f312b760e4 100644
--- a/tools/.gitignore
+++ b/tools/.gitignore
@@ -6,6 +6,7 @@
 /dumpimage
 /easylogo/easylogo
 /envcrc
+/fdt_add_pubkey
 /fdtgrep
 /file2include
 /fit_check_sign
diff --git a/tools/Makefile b/tools/Makefile
index 4a86321f64..44f25dda18 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -73,6 +73,7 @@ mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
 
 hostprogs-y += dumpimage mkimage
 hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign
+hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fdt_add_pubkey
 
 hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
 
@@ -153,6 +154,7 @@ dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
 mkimage-objs   := $(dumpimage-mkimage-objs) mkimage.o
 fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
 fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
+fdt_add_pubkey-objs   := $(dumpimage-mkimage-objs) fdt_add_pubkey.o
 file2include-objs := file2include.o
 
 ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_LIBCRYPTO),)
@@ -190,6 +192,7 @@ HOSTCFLAGS_fit_image.o += 
-DMKIMAGE_DTC=\"$(CONFIG_MKIMAGE_DTC_PATH)\"
 HOSTLDLIBS_dumpimage := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_info := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_check_sign := $(HOSTLDLIBS_mkimage)
+HOSTLDLIBS_fdt_add_pubkey := $(HOSTLDLIBS_mkimage)
 
 hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
 hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl
diff --git a/tools/fdt_add_pubkey.c b/tools/fdt_add_pubkey.c
new file mode 100755
index 00..9306ecedd1
--- /dev/null
+++ b/tools/fdt_add_pubkey.c
@@ -0,0 +1,97 @@
+#include 
+#include "fit_common.h"
+
+static const char *cmdname;
+
+static const char *algo_name = "sha1,rsa2048"; /* -a  */
+static const char *keydir = "."; /* -k  */
+static const char *keyname = "key"; /* -n  */
+static const char *require_keys; /* -r  */
+static const char *keydest; /* argv[n] */
+
+static void usage(const char *msg)
+{
+   fprintf(stderr, "Error: %s\n", msg);
+   fprintf(stderr, "Usage: %s [-a ] [-k ] [-n ] [-r 
] \n",
+   cmdname);
+   exit(EXIT_FAILURE);
+}
+
+static void process_args(int argc, char *argv[])
+{
+   int opt;
+
+   while((opt = getopt(argc, argv, "a:k:n:r:")) != -1) {
+   switch (opt) {
+   case 'k':
+   keydir = optarg;
+   break;
+   case 'a':
+   algo_name = optarg;
+   break;
+   case 'n':
+   keyname = optarg;
+   break;
+   case 'r':
+   require_keys = optarg;
+   break;
+   default:
+   usage("Invalid option");
+   }
+   }
+   /* The last parameter is expected to be the .dtb to add the public key 
to */
+   if (optind < argc)
+   keydest = argv[optind];
+
+   if (!keydest)
+   usage("Missing dtb file to update");
+}
+
+int main(int argc, char *argv[])
+{
+   struct image_sign_info info;
+   int destfd, ret;
+   void *dest_blob = NULL;
+   struct stat dest_sbuf;
+   size_t size_inc = 0;
+
+   cmdname = argv[0];
+
+   process_args(argc, argv);
+
+   memset(&info, 0, sizeof(info));
+
+   info.keydir = keydir;
+   info.keyname = keyname;
+   info.name = algo_name;
+   info.require_keys = require_keys;
+   info.crypto = image_get_crypto_algo(algo_name);
+   if (!info.crypto) {
+fprintf(stderr, "Unsupported signature alg

[PATCH 2/2] test_vboot.py: include test of fdt_add_pubkey tool

2021-11-08 Thread Roman Kopytin
Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 
---
 test/py/tests/test_vboot.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 6dff6779d1..cf7416b39a 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -230,6 +230,13 @@ def test_vboot(u_boot_console, sha_algo, padding, 
sign_options, required,
 cons.log.action('%s: Check signed config on the host' % sha_algo)
 
 util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
+
+# Create a fresh .dtb without the public keys
+dtc('sandbox-u-boot.dts')
+# Then add the dev key via the fdt_add_pubkey tool
+util.run_and_log(cons, [fdt_add_pubkey, '-a', '%s,rsa2048' % sha_algo,
+'-k', tmpdir, '-n', 'dev', '-r', 'conf', dtb])
+util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
 
 if full_test:
 # Make sure that U-Boot checks that the config is in the list of
@@ -370,6 +377,7 @@ def test_vboot(u_boot_console, sha_algo, padding, 
sign_options, required,
 fit = '%stest.fit' % tmpdir
 mkimage = cons.config.build_dir + '/tools/mkimage'
 fit_check_sign = cons.config.build_dir + '/tools/fit_check_sign'
+fdt_add_pubkey = cons.config.build_dir + '/tools/fdt_add_pubkey'
 dtc_args = '-I dts -O dtb -i %s' % tmpdir
 dtb = '%ssandbox-u-boot.dtb' % tmpdir
 sig_node = '/configurations/conf-1/signature'
-- 
2.25.1



[PATCH 0/2] test3

2021-11-08 Thread Roman Kopytin
testtest3

Roman Kopytin (2):
  tools: add fdt_add_pubkey
  test_vboot.py: include test of fdt_add_pubkey tool

 test/py/tests/test_vboot.py |  8 +++
 tools/.gitignore|  1 +
 tools/Makefile  |  3 ++
 tools/fdt_add_pubkey.c  | 97 +
 4 files changed, 109 insertions(+)
 create mode 100755 tools/fdt_add_pubkey.c

-- 
2.25.1



[PATCH 2/2] test_vboot.py: include test of fdt_add_pubkey tool

2021-11-08 Thread Roman Kopytin
Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 
---
 test/py/tests/test_vboot.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 6dff6779d1..cf7416b39a 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -230,6 +230,13 @@ def test_vboot(u_boot_console, sha_algo, padding, 
sign_options, required,
 cons.log.action('%s: Check signed config on the host' % sha_algo)
 
 util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
+
+# Create a fresh .dtb without the public keys
+dtc('sandbox-u-boot.dts')
+# Then add the dev key via the fdt_add_pubkey tool
+util.run_and_log(cons, [fdt_add_pubkey, '-a', '%s,rsa2048' % sha_algo,
+'-k', tmpdir, '-n', 'dev', '-r', 'conf', dtb])
+util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
 
 if full_test:
 # Make sure that U-Boot checks that the config is in the list of
@@ -370,6 +377,7 @@ def test_vboot(u_boot_console, sha_algo, padding, 
sign_options, required,
 fit = '%stest.fit' % tmpdir
 mkimage = cons.config.build_dir + '/tools/mkimage'
 fit_check_sign = cons.config.build_dir + '/tools/fit_check_sign'
+fdt_add_pubkey = cons.config.build_dir + '/tools/fdt_add_pubkey'
 dtc_args = '-I dts -O dtb -i %s' % tmpdir
 dtb = '%ssandbox-u-boot.dtb' % tmpdir
 sig_node = '/configurations/conf-1/signature'
-- 
2.25.1



[PATCH 1/2] tools: add fdt_add_pubkey

2021-11-08 Thread Roman Kopytin
Having to use the -K option to mkimage to populate U-Boot's .dtb with the
public key while signing the kernel FIT image is often a little
awkward. In particular, when using a meta-build system such as
bitbake/Yocto, having the tasks of the kernel and U-Boot recipes
intertwined, modifying deployed artifacts and rebuilding U-Boot with
an updated .dtb is quite cumbersome. Also, in some scenarios one may
wish to build U-Boot complete with the public key(s) embedded in the
.dtb without the corresponding private keys being present on the same
build host.

So this adds a simple tool that allows one to disentangle the kernel
and U-Boot builds, by simply copy-pasting just enough of the mkimage
code to allow one to add a public key to a .dtb. When using mkimage,
some of the information is taken from the .its used to build the
kernel (algorithm and key name), so that of course needs to be
supplied on the command line.

Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 
---
 tools/.gitignore   |  1 +
 tools/Makefile |  3 ++
 tools/fdt_add_pubkey.c | 97 ++
 3 files changed, 101 insertions(+)
 create mode 100755 tools/fdt_add_pubkey.c

diff --git a/tools/.gitignore b/tools/.gitignore
index a88453f64d..f312b760e4 100644
--- a/tools/.gitignore
+++ b/tools/.gitignore
@@ -6,6 +6,7 @@
 /dumpimage
 /easylogo/easylogo
 /envcrc
+/fdt_add_pubkey
 /fdtgrep
 /file2include
 /fit_check_sign
diff --git a/tools/Makefile b/tools/Makefile
index 4a86321f64..44f25dda18 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -73,6 +73,7 @@ mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
 
 hostprogs-y += dumpimage mkimage
 hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign
+hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fdt_add_pubkey
 
 hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
 
@@ -153,6 +154,7 @@ dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
 mkimage-objs   := $(dumpimage-mkimage-objs) mkimage.o
 fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
 fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
+fdt_add_pubkey-objs   := $(dumpimage-mkimage-objs) fdt_add_pubkey.o
 file2include-objs := file2include.o
 
 ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_LIBCRYPTO),)
@@ -190,6 +192,7 @@ HOSTCFLAGS_fit_image.o += 
-DMKIMAGE_DTC=\"$(CONFIG_MKIMAGE_DTC_PATH)\"
 HOSTLDLIBS_dumpimage := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_info := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_check_sign := $(HOSTLDLIBS_mkimage)
+HOSTLDLIBS_fdt_add_pubkey := $(HOSTLDLIBS_mkimage)
 
 hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
 hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl
diff --git a/tools/fdt_add_pubkey.c b/tools/fdt_add_pubkey.c
new file mode 100755
index 00..9306ecedd1
--- /dev/null
+++ b/tools/fdt_add_pubkey.c
@@ -0,0 +1,97 @@
+#include 
+#include "fit_common.h"
+
+static const char *cmdname;
+
+static const char *algo_name = "sha1,rsa2048"; /* -a  */
+static const char *keydir = "."; /* -k  */
+static const char *keyname = "key"; /* -n  */
+static const char *require_keys; /* -r  */
+static const char *keydest; /* argv[n] */
+
+static void usage(const char *msg)
+{
+   fprintf(stderr, "Error: %s\n", msg);
+   fprintf(stderr, "Usage: %s [-a ] [-k ] [-n ] [-r 
] \n",
+   cmdname);
+   exit(EXIT_FAILURE);
+}
+
+static void process_args(int argc, char *argv[])
+{
+   int opt;
+
+   while((opt = getopt(argc, argv, "a:k:n:r:")) != -1) {
+   switch (opt) {
+   case 'k':
+   keydir = optarg;
+   break;
+   case 'a':
+   algo_name = optarg;
+   break;
+   case 'n':
+   keyname = optarg;
+   break;
+   case 'r':
+   require_keys = optarg;
+   break;
+   default:
+   usage("Invalid option");
+   }
+   }
+   /* The last parameter is expected to be the .dtb to add the public key 
to */
+   if (optind < argc)
+   keydest = argv[optind];
+
+   if (!keydest)
+   usage("Missing dtb file to update");
+}
+
+int main(int argc, char *argv[])
+{
+   struct image_sign_info info;
+   int destfd, ret;
+   void *dest_blob = NULL;
+   struct stat dest_sbuf;
+   size_t size_inc = 0;
+
+   cmdname = argv[0];
+
+   process_args(argc, argv);
+
+   memset(&info, 0, sizeof(info));
+
+   info.keydir = keydir;
+   info.keyname = keyname;
+   info.name = algo_name;
+   info.require_keys = require_keys;
+   info.crypto = image_get_crypto_algo(algo_name);
+   if (!info.crypto) {
+fprintf(stderr, "Unsupported signature alg

[PATCH 0/2] test2

2021-11-08 Thread Roman Kopytin
testtest2

Roman Kopytin (2):
  tools: add fdt_add_pubkey
  test_vboot.py: include test of fdt_add_pubkey tool

 test/py/tests/test_vboot.py |  8 +++
 tools/.gitignore|  1 +
 tools/Makefile  |  3 ++
 tools/fdt_add_pubkey.c  | 97 +
 4 files changed, 109 insertions(+)
 create mode 100755 tools/fdt_add_pubkey.c

-- 
2.25.1



[PATCH 1/2] tools: add fdt_add_pubkey

2021-11-08 Thread Roman Kopytin
Having to use the -K option to mkimage to populate U-Boot's .dtb with the
public key while signing the kernel FIT image is often a little
awkward. In particular, when using a meta-build system such as
bitbake/Yocto, having the tasks of the kernel and U-Boot recipes
intertwined, modifying deployed artifacts and rebuilding U-Boot with
an updated .dtb is quite cumbersome. Also, in some scenarios one may
wish to build U-Boot complete with the public key(s) embedded in the
.dtb without the corresponding private keys being present on the same
build host.

So this adds a simple tool that allows one to disentangle the kernel
and U-Boot builds, by simply copy-pasting just enough of the mkimage
code to allow one to add a public key to a .dtb. When using mkimage,
some of the information is taken from the .its used to build the
kernel (algorithm and key name), so that of course needs to be
supplied on the command line.

Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 
---
 tools/.gitignore   |  1 +
 tools/Makefile |  3 ++
 tools/fdt_add_pubkey.c | 97 ++
 3 files changed, 101 insertions(+)
 create mode 100755 tools/fdt_add_pubkey.c

diff --git a/tools/.gitignore b/tools/.gitignore
index a88453f64d..f312b760e4 100644
--- a/tools/.gitignore
+++ b/tools/.gitignore
@@ -6,6 +6,7 @@
 /dumpimage
 /easylogo/easylogo
 /envcrc
+/fdt_add_pubkey
 /fdtgrep
 /file2include
 /fit_check_sign
diff --git a/tools/Makefile b/tools/Makefile
index 4a86321f64..44f25dda18 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -73,6 +73,7 @@ mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
 
 hostprogs-y += dumpimage mkimage
 hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fit_info fit_check_sign
+hostprogs-$(CONFIG_TOOLS_LIBCRYPTO) += fdt_add_pubkey
 
 hostprogs-$(CONFIG_CMD_BOOTEFI_SELFTEST) += file2include
 
@@ -153,6 +154,7 @@ dumpimage-objs := $(dumpimage-mkimage-objs) dumpimage.o
 mkimage-objs   := $(dumpimage-mkimage-objs) mkimage.o
 fit_info-objs   := $(dumpimage-mkimage-objs) fit_info.o
 fit_check_sign-objs   := $(dumpimage-mkimage-objs) fit_check_sign.o
+fdt_add_pubkey-objs   := $(dumpimage-mkimage-objs) fdt_add_pubkey.o
 file2include-objs := file2include.o
 
 ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_TOOLS_LIBCRYPTO),)
@@ -190,6 +192,7 @@ HOSTCFLAGS_fit_image.o += 
-DMKIMAGE_DTC=\"$(CONFIG_MKIMAGE_DTC_PATH)\"
 HOSTLDLIBS_dumpimage := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_info := $(HOSTLDLIBS_mkimage)
 HOSTLDLIBS_fit_check_sign := $(HOSTLDLIBS_mkimage)
+HOSTLDLIBS_fdt_add_pubkey := $(HOSTLDLIBS_mkimage)
 
 hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl
 hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl
diff --git a/tools/fdt_add_pubkey.c b/tools/fdt_add_pubkey.c
new file mode 100755
index 00..9306ecedd1
--- /dev/null
+++ b/tools/fdt_add_pubkey.c
@@ -0,0 +1,97 @@
+#include 
+#include "fit_common.h"
+
+static const char *cmdname;
+
+static const char *algo_name = "sha1,rsa2048"; /* -a  */
+static const char *keydir = "."; /* -k  */
+static const char *keyname = "key"; /* -n  */
+static const char *require_keys; /* -r  */
+static const char *keydest; /* argv[n] */
+
+static void usage(const char *msg)
+{
+   fprintf(stderr, "Error: %s\n", msg);
+   fprintf(stderr, "Usage: %s [-a ] [-k ] [-n ] [-r 
] \n",
+   cmdname);
+   exit(EXIT_FAILURE);
+}
+
+static void process_args(int argc, char *argv[])
+{
+   int opt;
+
+   while((opt = getopt(argc, argv, "a:k:n:r:")) != -1) {
+   switch (opt) {
+   case 'k':
+   keydir = optarg;
+   break;
+   case 'a':
+   algo_name = optarg;
+   break;
+   case 'n':
+   keyname = optarg;
+   break;
+   case 'r':
+   require_keys = optarg;
+   break;
+   default:
+   usage("Invalid option");
+   }
+   }
+   /* The last parameter is expected to be the .dtb to add the public key 
to */
+   if (optind < argc)
+   keydest = argv[optind];
+
+   if (!keydest)
+   usage("Missing dtb file to update");
+}
+
+int main(int argc, char *argv[])
+{
+   struct image_sign_info info;
+   int destfd, ret;
+   void *dest_blob = NULL;
+   struct stat dest_sbuf;
+   size_t size_inc = 0;
+
+   cmdname = argv[0];
+
+   process_args(argc, argv);
+
+   memset(&info, 0, sizeof(info));
+
+   info.keydir = keydir;
+   info.keyname = keyname;
+   info.name = algo_name;
+   info.require_keys = require_keys;
+   info.crypto = image_get_crypto_algo(algo_name);
+   if (!info.crypto) {
+fprintf(stderr, "Unsupported signature alg

[PATCH 2/2] test_vboot.py: include test of fdt_add_pubkey tool

2021-11-08 Thread Roman Kopytin
Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 
---
 test/py/tests/test_vboot.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 6dff6779d1..cf7416b39a 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -230,6 +230,13 @@ def test_vboot(u_boot_console, sha_algo, padding, 
sign_options, required,
 cons.log.action('%s: Check signed config on the host' % sha_algo)
 
 util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
+
+# Create a fresh .dtb without the public keys
+dtc('sandbox-u-boot.dts')
+# Then add the dev key via the fdt_add_pubkey tool
+util.run_and_log(cons, [fdt_add_pubkey, '-a', '%s,rsa2048' % sha_algo,
+'-k', tmpdir, '-n', 'dev', '-r', 'conf', dtb])
+util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb])
 
 if full_test:
 # Make sure that U-Boot checks that the config is in the list of
@@ -370,6 +377,7 @@ def test_vboot(u_boot_console, sha_algo, padding, 
sign_options, required,
 fit = '%stest.fit' % tmpdir
 mkimage = cons.config.build_dir + '/tools/mkimage'
 fit_check_sign = cons.config.build_dir + '/tools/fit_check_sign'
+fdt_add_pubkey = cons.config.build_dir + '/tools/fdt_add_pubkey'
 dtc_args = '-I dts -O dtb -i %s' % tmpdir
 dtb = '%ssandbox-u-boot.dtb' % tmpdir
 sig_node = '/configurations/conf-1/signature'
-- 
2.25.1



[PATCH 0/2] test

2021-11-08 Thread Roman Kopytin
testtest

Roman Kopytin (2):
  tools: add fdt_add_pubkey
  test_vboot.py: include test of fdt_add_pubkey tool

 test/py/tests/test_vboot.py |  8 +++
 tools/.gitignore|  1 +
 tools/Makefile  |  3 ++
 tools/fdt_add_pubkey.c  | 97 +
 4 files changed, 109 insertions(+)
 create mode 100755 tools/fdt_add_pubkey.c

-- 
2.25.1



RE: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES

2021-10-31 Thread Roman Kopytin
Hi, all
Currently I am waiting some help from our IT infrastructure department.

-Original Message-
From: Simon Glass  
Sent: Tuesday, October 26, 2021 4:28 AM
To: Rasmus Villemoes 
Cc: U-Boot Mailing List ; Alex Kiernan 
; Roman Kopytin ; Masahiro 
Yamada 
Subject: Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES

Hi Rasmus,

On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes  
wrote:
>
> The build system already automatically looks for and includes an 
> in-tree *-u-boot.dtsi when building the control .dtb. However, there 
> are some things that are awkward to maintain in such an in-tree file, 
> most notably the metadata associated to public keys used for verified 
> boot.
>
> The only "official" API to get that metadata into the .dtb is via 
> mkimage, as a side effect of building an actual signed image. But 
> there are multiple problems with that. First of all, the final U-Boot 
> (be it U-Boot proper or an SPL) image is built based on a binary 
> image, the .dtb, and possibly some other binary artifacts. So 
> modifying the .dtb after the build requires the meta-buildsystem 
> (Yocto, buildroot, whatnot) to know about and repeat some of the steps 
> that are already known to and handled by U-Boot's build system, 
> resulting in needless duplication of code. It's also somewhat annoying 
> and inconsistent to have a .dtb file in the build folder which is not 
> generated by the command listed in the corresponding .cmd file (that 
> of course applies to any generated file).
>
> So the contents of the /signature node really needs to be baked into 
> the .dtb file when it is first created, which means providing the 
> relevant data in the form of a .dtsi file. One could in theory put 
> that data into the *-u-boot.dtsi file, but it's more convenient to be 
> able to provide it externally: For example, when developing for a 
> customer, it's common to use a set of dummy keys for development, 
> while the consultants do not (and should not) have access to the 
> actual keys used in production. For such a setup, it's easier if the 
> keys used are chosen via the meta-buildsystem and the path(s) patched 
> in during the configure step. And of course, nothing prevents anybody 
> from having DEVICE_TREE_INCLUDES point at files maintained in git, or 
> for that matter from including the public key metadata in the 
> *-u-boot.dtsi directly and ignore this feature.
>
> There are other uses for this, e.g. in combination with ENV_IMPORT_FDT 
> it can be used for providing the contents of the /config/environment 
> node, so I don't want to tie this exclusively to use for verified 
> boot.
>
> Signed-off-by: Rasmus Villemoes 
> ---
>
> Getting the public key metadata into .dtsi form can be done with a 
> little scripting (roughly 'mkimage -K' of a dummy image followed by 
> 'dtc -I dtb -O dts'). I have a script that does that, along with 
> options to set 'required' and 'required-mode' properties, include 
> u-boot,dm-spl properties in case one wants to check U-Boot proper from 
> SPL with the verified boot mechanism, etc. I'm happy to share that 
> script if this gets accepted, but it's moot if this is rejected.
>
> I have previously tried to get an fdt_add_pubkey tool accepted [1,2] 
> to disentangle the kernel and u-boot builds (or u-boot and SPL builds 
> for that matter!), but as I've since realized, that isn't quite enough
> - the above points re modifying the .dtb after it is created but 
> before that is used to create further build artifacts still stand. 
> However, such a tool could still be useful for creating the .dtsi info 
> without the private keys being present, and my key2dtsi.sh script 
> could easily be modified to use a tool like that should it ever 
> appear.
>
> [1] 
> https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes
> @prevas.dk/ [2] 
> https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kasper
> sky.com/
>
>  dts/Kconfig  | 9 +
>  scripts/Makefile.lib | 2 ++
>  2 files changed, 11 insertions(+)

Reviewed-by: Simon Glass 

I suggest adding this to the documentation somewhere in doc/develop/devicetree/

Getting the key into the U-Boot .dtb is normally done with mkimage, as you say. 
I don't really understand why this approach is easier.

Also, is there any interest in using binman? It is designed to do the 
'packaging' step right at the end, when all the bits are available and just 
need to be put together.

I am trying to encourage people to move away from building from source always, 
to a two-step process:

- build all the bits
- package them, update devicetree, etc.

https://u-boot.readthedocs.io/en/latest/develop/pa

RE: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES

2021-09-28 Thread Roman Kopytin
Hi, all
I prepared 3 patches for fdt_add_pubkey adding.
But in our company infrastructure I can't use git send-email. Our IT can't help 
me to resolve issue.

-Original Message-
From: Rasmus Villemoes  
Sent: Tuesday, September 28, 2021 11:57 AM
To: u-boot@lists.denx.de
Cc: Alex Kiernan ; Simon Glass ; 
Roman Kopytin ; Masahiro Yamada 
; Rasmus Villemoes 
Subject: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES

The build system already automatically looks for and includes an in-tree 
*-u-boot.dtsi when building the control .dtb. However, there are some things 
that are awkward to maintain in such an in-tree file, most notably the metadata 
associated to public keys used for verified boot.

The only "official" API to get that metadata into the .dtb is via mkimage, as a 
side effect of building an actual signed image. But there are multiple problems 
with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image 
is built based on a binary image, the .dtb, and possibly some other binary 
artifacts. So modifying the .dtb after the build requires the meta-buildsystem 
(Yocto, buildroot, whatnot) to know about and repeat some of the steps that are 
already known to and handled by U-Boot's build system, resulting in needless 
duplication of code. It's also somewhat annoying and inconsistent to have a 
.dtb file in the build folder which is not generated by the command listed in 
the corresponding .cmd file (that of course applies to any generated file).

So the contents of the /signature node really needs to be baked into the .dtb 
file when it is first created, which means providing the relevant data in the 
form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi 
file, but it's more convenient to be able to provide it externally: For 
example, when developing for a customer, it's common to use a set of dummy keys 
for development, while the consultants do not (and should not) have access to 
the actual keys used in production. For such a setup, it's easier if the keys 
used are chosen via the meta-buildsystem and the path(s) patched in during the 
configure step. And of course, nothing prevents anybody from having 
DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from 
including the public key metadata in the *-u-boot.dtsi directly and ignore this 
feature.

There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can 
be used for providing the contents of the /config/environment node, so I don't 
want to tie this exclusively to use for verified boot.

Signed-off-by: Rasmus Villemoes 
---

Getting the public key metadata into .dtsi form can be done with a little 
scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O 
dts'). I have a script that does that, along with options to set 'required' and 
'required-mode' properties, include u-boot,dm-spl properties in case one wants 
to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm 
happy to share that script if this gets accepted, but it's moot if this is 
rejected.

I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to 
disentangle the kernel and u-boot builds (or u-boot and SPL builds for that 
matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but before that is 
used to create further build artifacts still stand. However, such a tool could 
still be useful for creating the .dtsi info without the private keys being 
present, and my key2dtsi.sh script could easily be modified to use a tool like 
that should it ever appear.

[1] 
https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villem...@prevas.dk/
[2] 
https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205fee...@kaspersky.com/

 dts/Kconfig  | 9 +
 scripts/Makefile.lib | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/dts/Kconfig b/dts/Kconfig
index dabe0080c1..593dddbaf0 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -139,6 +139,15 @@ config DEFAULT_DEVICE_TREE
  It can be overridden from the command line:
  $ make DEVICE_TREE=
 
+config DEVICE_TREE_INCLUDES
+   string "Extra .dtsi files to include when building DT control"
+   depends on OF_CONTROL
+   help
+ U-Boot's control .dtb is usually built from an in-tree .dts
+ file, plus (if available) an in-tree U-Boot-specific .dtsi
+ file. This option specifies a space-separated list of extra
+ .dtsi files that will also be used.
+
 config OF_LIST
string "List of device tree files to include for DT control"
depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git 
a/scripts/Makefile.lib b/scripts/Makefile.lib index 78bbebe7e9..a2accba940 
100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -320

RE: [PATCH] tools: add fdt_add_pubkey

2021-08-05 Thread Roman Kopytin
Thanks a lot.
Can I create 1 patch with tool + test?

-Original Message-
From: Rasmus Villemoes  
Sent: Thursday, August 5, 2021 10:56 AM
To: Roman Kopytin ; U-Boot-Denx 

Subject: Re: [PATCH] tools: add fdt_add_pubkey

Caution: This is an external email. Be cautious while opening links or 
attachments.



On 03/08/2021 13.00, Roman Kopytin wrote:

Hi Roman

Thanks for picking this up. A few notes on the process:

- Don't attach the patch, it must be inline in the email. But do not copy-paste 
it in to your mail client, that will most likely make it whitespace-damaged. 
git send-email is your friend.

- Sending it to u-boot@lists.denx.de is correct, but you should also cc 
"interested parties", e.g. at least me (for obvious reasons) and anybody who 
participated in the earlier thread. If you had used git send-email, your "Cc: " 
line in the patch would actually automatically have included me on cc. Other 
people are just as easy to add (with the --cc option to "git send-email"). Very 
few people, if any, read every message on the U-Boot mailing list.

- It would be good if you could also add a test and ensure that it works. I 
think my original submission had three commits, of which the first is now moot, 
but that last patch was a test case. It may need some adjustments to apply 
nowadays, but it should give you a starting point.

Rasmus


[PATCH] tools: add fdt_add_pubkey

2021-08-03 Thread Roman Kopytin
Having to use the -K option to mkimage to populate U-Boot's .dtb with the
public key while signing the kernel FIT image is often a little
awkward. In particular, when using a meta-build system such as
bitbake/Yocto, having the tasks of the kernel and U-Boot recipes
intertwined, modifying deployed artifacts and rebuilding U-Boot with
an updated .dtb is quite cumbersome. Also, in some scenarios one may
wish to build U-Boot complete with the public key(s) embedded in the
.dtb without the corresponding private keys being present on the same
build host.

So this adds a simple tool that allows one to disentangle the kernel
and U-Boot builds, by simply copy-pasting just enough of the mkimage
code to allow one to add a public key to a .dtb. When using mkimage,
some of the information is taken from the .its used to build the
kernel (algorithm and key name), so that of course needs to be
supplied on the command line.

Signed-off-by: Roman Kopytin 
Cc: Rasmus Villemoes 


0001-tools-add-fdt_add_pubkey.patch
Description: 0001-tools-add-fdt_add_pubkey.patch


RE: U-boot

2021-08-02 Thread Roman Kopytin
Hi, dear U-boot team
Is it correct repo git://git.qemu.org/u-boot
?
We have it as our upstream.

-Original Message-
From: Roman Kopytin 
Sent: Monday, August 2, 2021 2:19 PM
To: 'Rasmus Villemoes' ; Simon Glass 

Cc: Thomas Perrot ; Michael Nazzareno Trimarchi 
; U-Boot-Denx ; Alex 
Kiernan 
Subject: RE: U-boot

Thanks, I have good result with tool. Ok, I will check my tasks and will try to 
add this tool.

-Original Message-
From: Rasmus Villemoes  
Sent: Monday, August 2, 2021 2:09 PM
To: Roman Kopytin ; Simon Glass 
Cc: Thomas Perrot ; Michael Nazzareno Trimarchi 
; U-Boot-Denx ; Alex 
Kiernan 
Subject: Re: U-boot

Caution: This is an external email. Be cautious while opening links or 
attachments.



On 02/08/2021 11.55, Roman Kopytin wrote:
> Yes, I don't see this tool in master branch.
> May be I will take code and build this tool.
>
> Do you have a plan for sharing it in repo?

Well, the repo for "sharing" this would/should be upstream U-Boot, and if 
there's sufficient interest I'll rebase and resend - though I can't promise any 
time frame, as there's a lot of other things on my plate. If you have the time, 
feel free to take the code and submit it yourself.

Rasmus


RE: U-boot

2021-08-02 Thread Roman Kopytin
Thanks, I have good result with tool. Ok, I will check my tasks and will try to 
add this tool.

-Original Message-
From: Rasmus Villemoes  
Sent: Monday, August 2, 2021 2:09 PM
To: Roman Kopytin ; Simon Glass 
Cc: Thomas Perrot ; Michael Nazzareno Trimarchi 
; U-Boot-Denx ; Alex 
Kiernan 
Subject: Re: U-boot

Caution: This is an external email. Be cautious while opening links or 
attachments.



On 02/08/2021 11.55, Roman Kopytin wrote:
> Yes, I don't see this tool in master branch.
> May be I will take code and build this tool.
>
> Do you have a plan for sharing it in repo?

Well, the repo for "sharing" this would/should be upstream U-Boot, and if 
there's sufficient interest I'll rebase and resend - though I can't promise any 
time frame, as there's a lot of other things on my plate. If you have the time, 
feel free to take the code and submit it yourself.

Rasmus


RE: U-boot

2021-08-02 Thread Roman Kopytin
Yes, I don't see this tool in master branch.
May be I will take code and build this tool.

Do you have a plan for sharing it in repo?


-Original Message-
From: Rasmus Villemoes  
Sent: Monday, August 2, 2021 12:37 PM
To: Roman Kopytin ; Simon Glass 
Cc: Thomas Perrot ; Michael Nazzareno Trimarchi 
; U-Boot-Denx ; Alex 
Kiernan 
Subject: Re: U-boot

Caution: This is an external email. Be cautious while opening links or 
attachments.



On 02/08/2021 11.25, Roman Kopytin wrote:
> Thanks a lot!
> Yes, looks like using of the 'fdtput' is not very safety for me.
> As I understood I need to use "fdt_add_pubkey" tool with CMD (example):
> ./ fdt_add_pubkey  -a rsa2048 -k  -n  -r  
> my_file.dtb
>
> -r  is the same as for mkimage? As I remember we can use -r w/o 
> any values in mkimage.

Yes, that's very close to what our Yocto recipe currently does:

for b in ${KERNEL_PUBLIC_KEYS} ; do
fdt_add_pubkey -a 'sha1,rsa2048' -k "${KERNEL_SIGNING_DIR}" -n 
"$b" \
-r conf $dtb
done

I doubt that old patch applies nowadays, I've only forward-ported it to
2020.04 internally.

As to Simon's old question of whether it could be done in mkimage with a new 
flag: I'd really prefer not to, mkimage is already an incoherent collection of 
tools that do very different things with different flags.
Having a flag that says "create and sign this FIT image, and as a side effect 
update $this dtb $overhere with the corresponding public key mangled 
appropriately, oh, and btw, _only_ do that side effect" is a non-starter.

Rasmus


RE: U-boot

2021-08-02 Thread Roman Kopytin
Thanks a lot!
Yes, looks like using of the 'fdtput' is not very safety for me.
As I understood I need to use "fdt_add_pubkey" tool with CMD (example):
./ fdt_add_pubkey  -a rsa2048 -k  -n  -r  
my_file.dtb

-r  is the same as for mkimage? As I remember we can use -r w/o any 
values in mkimage.


-Original Message-
From: Rasmus Villemoes  
Sent: Monday, August 2, 2021 12:00 PM
To: Simon Glass ; Roman Kopytin 
Cc: Thomas Perrot ; Michael Nazzareno Trimarchi 
; U-Boot-Denx ; Alex 
Kiernan 
Subject: Re: U-boot

Caution: This is an external email. Be cautious while opening links or 
attachments.



On 31/07/2021 18.59, Simon Glass wrote:
> Hi Roman,
>
> On Sat, 31 Jul 2021 at 02:26, Roman Kopytin  
> wrote:
>>
>> Thank, but my question was about adding of the public key to dtb file 
>> without private key. We won't have private key in our side.
>
> (please try not to top-post on the mailing list)
>
> Presumably this means that you know what the public key is, so one 
> option is to manually add it to the dtb, e.g. in a u-boot.dtsi file 
> for your board. You can see the format of it in the documentation, or 
> just copy what is there when you do the signing.
>

I sent
https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villem...@prevas.dk/
1.5 years ago. Roman, is it something like that you need? We've used that 
patch/tool internally ever since.

> Another option would be to use 'fdtput' to add the various fields in 
> the dtb after building.

Yes, but that, or the .dtsi approach, requires figuring just exactly what those 
fields are supposed to be. And even if one could "reverse engineer" that and 
implement the math separately in another tool, it's much better to utilize the 
same code which "mkimage proper" would use, since there's less risk of messing 
up endianness etc., and only one place to fix bugs.

Rasmus


RE: U-boot

2021-07-31 Thread Roman Kopytin
Thank, but my question was about adding of the public key to dtb file without 
private key. We won't have private key in our side.

-Original Message-
From: Thomas Perrot  
Sent: Saturday, July 31, 2021 9:52 AM
To: Roman Kopytin ; Michael Nazzareno Trimarchi 

Cc: U-Boot-Denx ; Simon Glass 
Subject: Re: U-boot

Hi Roman,

On Sat, 2021-07-31 at 03:34 +0000, Roman Kopytin wrote:
> Thanks, Michael.
> Can we sign in the separate state on special server for example?

Yes, it possible, there is a step to build and another one to sign, that can be 
separated.

For example, the following command, that build and sign the itb:
# build and sign
mkimage -D "-I dts -O dtb -p 4096" -f ./foo.its -k ./keys -K ./u- boot.dtb -r 
./foo.itb

Can be spitted in two:
# build
uboot-mkimage \
-D "-I dts -O dtb -p 4096" \
-f ./foo.its \
./foo.itb

# sign
uboot-mkimage \
-D "-I dts -O dtb -p 4096" -F 
-k ./keys \
-K ./u-boot.dtb \
-r \
./foo.itb

Then the u-boot*.dtb should contains the pubkey node(s) in the signature node 
and it can be shared and concatenated to the U-Boot
binary:

make EXT_DTB="./u-boot.dtb"

> Looks like we can work with public key only in this step.

The dtb containing the public key(s) is useful to verify the signature at the 
target boot, or with the tool fit_check_sign to perform an offload checking, 
for example:

fit_check_sign -f ./foo.itb -k ./u-boot.dtb

Best regards,
Thomas Perrot

> 
> From: Michael Nazzareno Trimarchi 
> Sent: Friday, July 30, 2021 8:50 PM
> To: Roman Kopytin 
> Cc: U-Boot-Denx ; Simon Glass 
> Subject: Re: U-boot
> 
> Caution: This is an external email. Be cautious while opening links or 
> attachments.
> 
> 
> Hi Román
> 
> 
> On Fri, Jul 30, 2021, 7:44 PM Roman Kopytin < 
> roman.kopy...@kaspersky.com<mailto:roman.kopy...@kaspersky.com>> wrote:
> Hello, dear U-boot team
> 
> I have question about your old feature: U-boot patch for adding of the 
> public key to dtb file.
>  
> https://patchwork.ozlabs.org/project/uboot/patch/1363650725-30459-37-g
> it-send-email-sjg%40chromium.org/
> 
> I can’t understand, can we work only with public key? Why do we need 
> to have private key for adding step?
> In documentation it is not very clear for me.
> 
> You need to sign with private key and keep it secret and local and 
> verify it during booting with public key. Private key is not 
> distributed with the image
> 
> Michael
> 
> 
> Thanks a lot.
> 

--
Thomas Perrot, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com



RE: U-boot

2021-07-31 Thread Roman Kopytin
Thanks, Michael.
Can we sign in the separate state on special server for example?
Looks like we can work with public key only in this step.

From: Michael Nazzareno Trimarchi 
Sent: Friday, July 30, 2021 8:50 PM
To: Roman Kopytin 
Cc: U-Boot-Denx ; Simon Glass 
Subject: Re: U-boot

Caution: This is an external email. Be cautious while opening links or 
attachments.


Hi Román


On Fri, Jul 30, 2021, 7:44 PM Roman Kopytin 
mailto:roman.kopy...@kaspersky.com>> wrote:
Hello, dear U-boot team

I have question about your old feature: U-boot patch for adding of the public 
key to dtb file.
https://patchwork.ozlabs.org/project/uboot/patch/1363650725-30459-37-git-send-email-sjg%40chromium.org/

I can’t understand, can we work only with public key? Why do we need to have 
private key for adding step?
In documentation it is not very clear for me.

You need to sign with private key and keep it secret and local and verify it 
during booting with public key. Private key is not distributed with the image

Michael


Thanks a lot.



U-boot

2021-07-30 Thread Roman Kopytin
Hello, dear U-boot team

I have question about your old feature: U-boot patch for adding of the public 
key to dtb file.
https://patchwork.ozlabs.org/project/uboot/patch/1363650725-30459-37-git-send-email-sjg%40chromium.org/

I can’t understand, can we work only with public key? Why do we need to have 
private key for adding step?
In documentation it is not very clear for me.

Thanks a lot.