Re: [U-Boot] [PATCH v3 4/6] binman: add optional support for U-Boot image signing
Hi Simon, On Mon, 20 Nov 2017 08:40:29 -0700 Simon Glass s...@chromium.org wrote: ... > > 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. OK, thanks! Yes, I'll add some docs to README and try to come up with a test. ... > > 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. Yes, will do in v4 patch. I can just pass options.keydir here and use indir/outdir from tools. ... > > +class ImageSigner(object): > > +def __init__(self, sign_func): > > +self.sign = sign_func > > + > > +def GetImageSigner(soc): > > Function comment. OK, added in v4. Thanks, Anatolij ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v3 4/6] binman: add optional support for U-Boot image signing
Hi Anatolij,
On 16 November 2017 at 18:15, Anatolij Gustschin 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
> ---
> 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 = 0x1 - 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:
> +
[U-Boot] [PATCH v3 4/6] binman: add optional support for U-Boot image signing
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
---
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
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)
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 = 0x1 - 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
