[PATCH 0/1] Introduce new sign binman's option

[PATCH 0/1] Introduce new sign binman's option

[Ivan Mikhaylov]

From: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>

This patch introduces prototype of new sign binman's option. It doesn't looks
good for now but it shows how sign process can be done. Just enhancing
sign procedure, as example from:
mkimage -G privateky -r -o sha256,rsa4096 -F fit@0x280000.fit
binman replace -i flash.bin -f fit@0x280000.fit fit@0x280000

into:
binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit@0x280000.fit fit@0x280000

1. Is it needed at all? I think, it improves the way of signing, maybe
there is some other opinions on this spot?
2. I hope, that `mkimage sign` can be replaced with python code instead
of tools.Run.
3. And what things/cases should be covered in case of approval of such
option? Tests?
4. Also this option may cover cases like for TI's K3 architectures, it
may replace things like tools/k3_gen_x509_cert.sh with binman sign
option.

Also this patch refers on 
https://lists.denx.de/pipermail/u-boot/2021-November/468282.html with
algo option for mkimage.

Ivan Mikhaylov (1):
  binman: add sign option for binman

 tools/binman/cmdline.py | 13 +++++++++++++
 tools/binman/control.py | 27 ++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

-- 

[PATCH 1/1] binman: add sign option for binman

[Ivan Mikhaylov]

Introduce prototype for binman's new option which provides sign
and replace sections in binary images.

Usage as example:

from:
mkimage -G privateky -r -o sha256,rsa4096 -F fit@0x280000.fit
binman replace -i flash.bin -f fit@0x280000.fit fit@0x280000

to:
binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit@0x280000.fit fit@0x280000

Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
---
 tools/binman/cmdline.py | 13 +++++++++++++
 tools/binman/control.py | 27 ++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
index e73ff78095..c3cfd17d1c 100644
--- a/tools/binman/cmdline.py
+++ b/tools/binman/cmdline.py
@@ -113,6 +113,19 @@ controlled by a description in the board device tree.'''
     replace_parser.add_argument('paths', type=str, nargs='*',
                                 help='Paths within file to replace (wildcard)')
 
+    sign_parser = subparsers.add_parser('sign',
+                                           help='Sign entries in image')
+    sign_parser.add_argument('-i', '--image', type=str, required=True,
+                                help='Image filename to update')
+    sign_parser.add_argument('-k', '--key', type=str, required=True,
+                                help='Private key file for sign')
+    sign_parser.add_argument('-a', '--algo', type=str, required=True,
+                                help='Hash algorithm')
+    sign_parser.add_argument('-f', '--file', type=str, required=True,
+                                help='Input filename to sign')
+    sign_parser.add_argument('paths', type=str, nargs='*',
+                                help='Paths within file to sign (wildcard)')
+
     test_parser = subparsers.add_parser('test', help='Run tests')
     test_parser.add_argument('-P', '--processes', type=int,
         help='set number of processes to use for running tests')
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 7da69ba38d..ec0e55f7c3 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -18,6 +18,7 @@ from binman import cbfs_util
 from binman import elf
 from patman import command
 from patman import tout
+from patman import tools
 
 # List of images we plan to create
 # Make this global so that it can be referenced from tests
@@ -401,6 +402,26 @@ def ReplaceEntries(image_fname, input_fname, indir, entry_paths,
     AfterReplace(image, allow_resize=allow_resize, write_map=write_map)
     return image
 
+def MkimageSign(privatekey_fname, algo, input_fname):
+    tools.Run('mkimage', '-G', privatekey_fname, '-r', '-o', algo, '-F', input_fname)
+
+def SignEntries(image_fname, input_fname, privatekey_fname, algo, entry_paths):
+    """Sign and replace the data from one or more entries from input files
+
+    Args:
+        image_fname: Image filename to process
+        input_fname: Single input filename to use if replacing one file, None
+            otherwise
+        algo: Hashing algorithm
+        privatekey_fname: Private key filename
+
+    Returns:
+        List of EntryInfo records that were signed and replaced
+    """
+
+    MkimageSign(privatekey_fname, algo, input_fname)
+
+    return ReplaceEntries(image_fname, input_fname, None, entry_paths)
 
 def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded):
     """Prepare the images to be processed and select the device tree
@@ -575,7 +596,7 @@ def Binman(args):
     from binman.image import Image
     from binman import state
 
-    if args.cmd in ['ls', 'extract', 'replace']:
+    if args.cmd in ['ls', 'extract', 'replace', 'sign']:
         try:
             tout.Init(args.verbosity)
             tools.PrepareOutputDir(None)
@@ -590,6 +611,10 @@ def Binman(args):
                 ReplaceEntries(args.image, args.filename, args.indir, args.paths,
                                do_compress=not args.compressed,
                                allow_resize=not args.fix_size, write_map=args.map)
+
+            if args.cmd == 'sign':
+                SignEntries(args.image, args.file, args.key, args.algo, args.paths)
+
         except:
             raise
         finally:
-- 
2.31.1

Re: [PATCH 1/1] binman: add sign option for binman

[Simon Glass]

Hi Ivan,

On Fri, 24 Dec 2021 at 11:23, Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>
> Introduce prototype for binman's new option which provides sign
> and replace sections in binary images.
>
> Usage as example:
>
> from:
> mkimage -G privateky -r -o sha256,rsa4096 -F fit@0x280000.fit
> binman replace -i flash.bin -f fit@0x280000.fit fit@0x280000
>
> to:
> binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit@0x280000.fit fit@0x280000
>
> Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
> ---
>  tools/binman/cmdline.py | 13 +++++++++++++
>  tools/binman/control.py | 27 ++++++++++++++++++++++++++-
>  2 files changed, 39 insertions(+), 1 deletion(-)

This looks good. Just need a test and docs update (also check 'binman
test -T' for 100% code coverage).

Nits below

>
> diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
> index e73ff78095..c3cfd17d1c 100644
> --- a/tools/binman/cmdline.py
> +++ b/tools/binman/cmdline.py
> @@ -113,6 +113,19 @@ controlled by a description in the board device tree.'''
>      replace_parser.add_argument('paths', type=str, nargs='*',
>                                  help='Paths within file to replace (wildcard)')
>
> +    sign_parser = subparsers.add_parser('sign',
> +                                           help='Sign entries in image')
> +    sign_parser.add_argument('-i', '--image', type=str, required=True,
> +                                help='Image filename to update')
> +    sign_parser.add_argument('-k', '--key', type=str, required=True,
> +                                help='Private key file for sign')

s/sign/signing/

> +    sign_parser.add_argument('-a', '--algo', type=str, required=True,
> +                                help='Hash algorithm')

e.g. sha256,rsa4096

> +    sign_parser.add_argument('-f', '--file', type=str, required=True,
> +                                help='Input filename to sign')

Please sort the options in alpha order: -a, -f, -i, -k

[..]

Regards,
Simon

Re: [PATCH 1/1] binman: add sign option for binman

[Ivan Mikhaylov]

On Tue, 2021-12-28 at 01:34 -0700, Simon Glass wrote:
> Hi Ivan,
> 
> On Fri, 24 Dec 2021 at 11:23, Ivan Mikhaylov <fr0st61te@gmail.com>
> wrote:
> > 
> > Introduce prototype for binman's new option which provides sign
> > and replace sections in binary images.
> > 
> > Usage as example:
> > 
> > from:
> > mkimage -G privateky -r -o sha256,rsa4096 -F fit@0x280000.fit
> > binman replace -i flash.bin -f fit@0x280000.fit fit@0x280000
> > 
> > to:
> > binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f
> > fit@0x280000.fit fit@0x280000
> > 
> > Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
> > ---
> >  tools/binman/cmdline.py | 13 +++++++++++++
> >  tools/binman/control.py | 27 ++++++++++++++++++++++++++-
> >  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> This looks good. Just need a test and docs update (also check 'binman
> test -T' for 100% code coverage).

Simon, I've tried to figure out with test and stumble a little bit with
verification step. How to verify that mkimage sign fit image with
existing key, is there any option or any toolkits? I didn't find any
suitable option in mkimage either, is it good idea to add key
verification inside mkimage? Other way is to have blobs with predefined
keys inside test directory in binman which I think is not so good.

Thanks.

Re: [PATCH 1/1] binman: add sign option for binman

[Simon Glass]

Hi Ivan,

On Thu, 27 Jan 2022 at 03:00, Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>
> On Tue, 2021-12-28 at 01:34 -0700, Simon Glass wrote:
> > Hi Ivan,
> >
> > On Fri, 24 Dec 2021 at 11:23, Ivan Mikhaylov <fr0st61te@gmail.com>
> > wrote:
> > >
> > > Introduce prototype for binman's new option which provides sign
> > > and replace sections in binary images.
> > >
> > > Usage as example:
> > >
> > > from:
> > > mkimage -G privateky -r -o sha256,rsa4096 -F fit@0x280000.fit
> > > binman replace -i flash.bin -f fit@0x280000.fit fit@0x280000
> > >
> > > to:
> > > binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f
> > > fit@0x280000.fit fit@0x280000
> > >
> > > Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
> > > ---
> > >  tools/binman/cmdline.py | 13 +++++++++++++
> > >  tools/binman/control.py | 27 ++++++++++++++++++++++++++-
> > >  2 files changed, 39 insertions(+), 1 deletion(-)
> >
> > This looks good. Just need a test and docs update (also check 'binman
> > test -T' for 100% code coverage).
>
> Simon, I've tried to figure out with test and stumble a little bit with
> verification step. How to verify that mkimage sign fit image with
> existing key, is there any option or any toolkits? I didn't find any
> suitable option in mkimage either, is it good idea to add key
> verification inside mkimage? Other way is to have blobs with predefined
> keys inside test directory in binman which I think is not so good.

We already have test_vboot.py which runs U-Boot (and also
fit_check_sign) to verify the signature. Can you use that?

Regards,
Simon