[U-Boot] [PATCH v3 4/6] binman: add optional support for U-Boot image signing
Simon Glass
sjg at chromium.org
Mon Nov 20 15:40:29 UTC 2017
Hi Anatolij,
On 16 November 2017 at 18:15, Anatolij Gustschin <agust at denx.de> wrote:
> Prepare binman for adding image signing support after final image
> build stage. Custom image signing functionality for SoCs supporting
> verified boot can be added, e.g. like in the subsequent patch with
> signing functionality for Bay Trail SoC images.
>
> Read 'sign' and 'socname' properties in /binman node of the DTS file
> and setup the SoC specific signing method, if these properties are
> present. The directory with signing keys can be specified by '--keydir'
> option. The keys are supposed to be in the 'keydir' directory if no
> keydir option was given.
>
> Signed-off-by: Anatolij Gustschin <agust at denx.de>
> ---
> NOTE: This patch applies on top of binman changes in binman-working
> branch in git://git.denx.de/u-boot-dm.git
>
> Changes in v3:
> - None, new patch in this series
>
> tools/binman/binman.py | 3 +++
> tools/binman/cmdline.py | 2 ++
> tools/binman/control.py | 1 +
> tools/binman/image.py | 23 +++++++++++++++++++++++
> tools/binman/signing/signer.py | 22 ++++++++++++++++++++++
> 5 files changed, 51 insertions(+)
> create mode 100644 tools/binman/signing/signer.py
This looks reasonable to me, but can you please add a test and also
docs to the binman README for this new feature?
You might want to rebase on top of my recent fixes because there are
some test problems at present.
dm/binman-working
I'll hopefully pull these in this week.
>
> diff --git a/tools/binman/binman.py b/tools/binman/binman.py
> index 1c8e8dbff6..ad5f351261 100755
> --- a/tools/binman/binman.py
> +++ b/tools/binman/binman.py
> @@ -27,6 +27,9 @@ sys.path.insert(0, 'scripts/dtc/pylibfdt')
> # Also allow entry-type modules to be brought in from the etype directory.
> sys.path.insert(0, os.path.join(our_path, 'etype'))
>
> +# Bring in the module for image signing
> +sys.path.insert(0, os.path.join(our_path, 'signing'))
> +
> import cmdline
> import command
> import control
> diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
> index 233d5e1d1a..bc47949e40 100644
> --- a/tools/binman/cmdline.py
> +++ b/tools/binman/cmdline.py
> @@ -29,6 +29,8 @@ def ParseArgs(argv):
> help='Enabling debugging (provides a full traceback on error)')
> parser.add_option('-I', '--indir', action='append',
> help='Add a path to a directory to use for input files')
> + parser.add_option('-K', '--keydir', type='string', action='store', default="keydir",
> + help='Path to the directory with image signing keys')
> parser.add_option('-H', '--full-help', action='store_true',
> default=False, help='Display the README file')
> parser.add_option('-O', '--outdir', type='string',
> diff --git a/tools/binman/control.py b/tools/binman/control.py
> index ffa2bbd80f..2ad1ebf3fb 100644
> --- a/tools/binman/control.py
> +++ b/tools/binman/control.py
> @@ -113,6 +113,7 @@ def Binman(options, args):
> image.ProcessEntryContents()
> image.WriteSymbols()
> image.BuildImage()
> + image.SignImage(options)
Can you put the info somewhere, or pass the info you need out of
options? I am not keen on the options object bleeding into the image
code.
> finally:
> tools.FinaliseOutputDir()
> finally:
> diff --git a/tools/binman/image.py b/tools/binman/image.py
> index 741630f091..2dfa00ae57 100644
> --- a/tools/binman/image.py
> +++ b/tools/binman/image.py
> @@ -15,6 +15,7 @@ import sys
>
> import fdt_util
> import tools
> +from signer import GetImageSigner
>
> class Image:
> """A Image, representing an output from binman
> @@ -48,6 +49,9 @@ class Image:
> This causes _skip_at_start to be set to the starting memory
> address.
> _entries: OrderedDict() of entries
> + _sign: Flag to indicate whether image signing is requested
> + _socname: Name of the SoC used to obtain the signer object
> + _signer: Signer object implementing SoC specific signing function
> """
> def __init__(self, name, node, test=False):
> global entry
> @@ -67,6 +71,9 @@ class Image:
> self._skip_at_start = 0
> self._end_4gb = False
> self._entries = OrderedDict()
> + self._sign = False
> + self._signer = None
> + self._socname = None
>
> if not test:
> self._ReadNode()
> @@ -92,6 +99,15 @@ class Image:
> if self._end_4gb:
> self._skip_at_start = 0x100000000 - self._size
>
> + self._sign = fdt_util.GetBool(self._node, 'sign')
> + if self._sign:
> + self._socname = fdt_util.GetString(self._node, 'socname')
> + if not self._socname:
> + self._Raise("SoC name must be provided when signing is enabled")
> + self._signer = GetImageSigner(self._socname)
> + if not self._signer:
> + self._Raise("No image signer found for SoC '%s'" % self._socname)
> +
> def CheckSize(self):
> """Check that the image contents does not exceed its size, etc."""
> contents_size = 0
> @@ -249,6 +265,13 @@ class Image:
> fd.seek(self._pad_before + entry.pos - self._skip_at_start)
> fd.write(data)
>
> + def SignImage(self, options):
> + """Sign the image if verified boot is enabled"""
> + if not self._sign:
> + return
> + fname = tools.GetOutputFilename(self._filename)
> + self._signer.sign(fname, options.keydir, options.indir, options.outdir)
> +
> def LookupSymbol(self, sym_name, optional, msg):
> """Look up a symbol in an ELF file
>
> diff --git a/tools/binman/signing/signer.py b/tools/binman/signing/signer.py
> new file mode 100644
> index 0000000000..4ec43d424f
> --- /dev/null
> +++ b/tools/binman/signing/signer.py
> @@ -0,0 +1,22 @@
> +# Copyright (c) 2017 DENX Software Engineering
> +# Anatolij Gustschin <agust at denx.de>
> +#
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Class for signing the output image of binman
> +#
> +
> +# Dictionary with SoC names and corresponding signing functions.
> +# Image signing support for not yet supported SoCs can be added
> +# here
> +soc_sign_dict = {
> +}
> +
> +class ImageSigner(object):
> + def __init__(self, sign_func):
> + self.sign = sign_func
> +
> +def GetImageSigner(soc):
Function comment.
> + if not soc in soc_sign_dict:
> + return None
> + return ImageSigner(soc_sign_dict[soc])
> --
> 2.11.0
>
Regards,
Simon
More information about the U-Boot
mailing list