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

[Alper Nebi Yasak <alpernebiyasak@gmail.com>]

On 22/03/2022 00:43, Ivan Mikhaylov wrote:
> Introduce proof of concept 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
> binman replace -i flash.bin -f fit.fit fit
> 
> to:
> binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit.fit fit

This shouldn't need the fit.fit input. It can be extracted from the
image as you said in the cover letter. But I think instead of doing
"extract -> sign with mkimage -> replace", signing should be implemented
in the entry types as a new method. This way we can support other
entry-specific ways to sign things (see vblock for an example).

> Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
> ---
>  tools/binman/cmdline.py | 13 +++++++++++++
>  tools/binman/control.py | 26 +++++++++++++++++++++++++-
>  2 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
> index 0626b850f4..1a25f95ff1 100644
> --- a/tools/binman/cmdline.py
> +++ b/tools/binman/cmdline.py
> @@ -160,6 +160,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('-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')
> +    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 signing')
> +    sign_parser.add_argument('paths', type=str, nargs='*',
> +                                help='Paths within file to sign (wildcard)')
> +

If we want to support signing entry types other than FIT, I guess we
need to base things on "EntryArg"s to make the arguments more dynamic,
like named-by-arg blobs do.

>      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 a179f78129..7595ea7776 100644
> --- a/tools/binman/control.py
> +++ b/tools/binman/control.py
> @@ -19,6 +19,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
> @@ -434,6 +435,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)

I wrote a bit above, but what I mean is the mkimage call would go into a
new method in etype/fit.py, and SignEntries() would be something like
ReplaceEntries() but end up calling that method instead of WriteData().

>  
>  def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded):
>      """Prepare the images to be processed and select the device tree
> @@ -627,7 +648,7 @@ def Binman(args):
>      from binman.image import Image
>      from binman import state
>  
> -    if args.cmd in ['ls', 'extract', 'replace', 'tool']:
> +    if args.cmd in ['ls', 'extract', 'replace', 'tool', 'sign']:
>          try:
>              tout.init(args.verbosity)
>              tools.prepare_output_dir(None)
> @@ -643,6 +664,9 @@ def Binman(args):
>                                 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)
> +
>              if args.cmd == 'tool':
>                  tools.set_tool_paths(args.toolpath)
>                  if args.list: