* [PATCH 0/3] Introduce new sign binman's option
@ 2022-03-21 21:43 Ivan Mikhaylov
2022-03-21 21:43 ` [PATCH 1/3] binman: add sign option for binman Ivan Mikhaylov
` (3 more replies)
0 siblings, 4 replies; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-03-21 21:43 UTC (permalink / raw)
To: Simon Glass, Jan Kiszka; +Cc: u-boot, Ivan Mikhaylov
From: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
This patch introduces prototype of new sign binman's option.
Enhancing the sign procedure, as example:
mkimage -G privateky -r -o sha256,rsa4096 -F fit.fit
binman replace -i flash.bin -f fit.fit fit
into:
binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit.fit fit
It works with extracted FIT container and image, which provides key signing
and replacing FIT container in directed image.
Also, I'll add additional enhancement in future to this procedure with
skipping on FIT container and providing extract->sign->replace in whole
instead of sign->replace with documentation update and test as well.
As example:
binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit
Ivan Mikhaylov (3):
binman: add sign option for binman
binman: add documentation for binman sign option
binman: add test for sign option
tools/binman/binman.rst | 10 +++++
tools/binman/cmdline.py | 13 ++++++
tools/binman/control.py | 26 +++++++++++-
tools/binman/ftest.py | 42 +++++++++++++++++++
tools/binman/test/225_fit_sign.dts | 67 ++++++++++++++++++++++++++++++
5 files changed, 157 insertions(+), 1 deletion(-)
create mode 100644 tools/binman/test/225_fit_sign.dts
--
2.35.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/3] binman: add sign option for binman
2022-03-21 21:43 [PATCH 0/3] Introduce new sign binman's option Ivan Mikhaylov
@ 2022-03-21 21:43 ` Ivan Mikhaylov
2022-04-05 18:54 ` Alper Nebi Yasak
2022-03-21 21:43 ` [PATCH 2/3] binman: add documentation for binman sign option Ivan Mikhaylov
` (2 subsequent siblings)
3 siblings, 1 reply; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-03-21 21:43 UTC (permalink / raw)
To: Simon Glass, Jan Kiszka; +Cc: u-boot, Ivan Mikhaylov, Ivan Mikhaylov
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
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)')
+
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)
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:
--
2.35.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/3] binman: add documentation for binman sign option
2022-03-21 21:43 [PATCH 0/3] Introduce new sign binman's option Ivan Mikhaylov
2022-03-21 21:43 ` [PATCH 1/3] binman: add sign option for binman Ivan Mikhaylov
@ 2022-03-21 21:43 ` Ivan Mikhaylov
2022-03-21 21:43 ` [PATCH 3/3] binman: add test for " Ivan Mikhaylov
2022-08-13 14:59 ` [PATCH 0/3] Introduce new sign binman's option Simon Glass
3 siblings, 0 replies; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-03-21 21:43 UTC (permalink / raw)
To: Simon Glass, Jan Kiszka; +Cc: u-boot, Ivan Mikhaylov, Ivan Mikhaylov
Add the documentation about binman sign option and providing an
example.
Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
---
tools/binman/binman.rst | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 771645380e..efa3321b95 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -1005,6 +1005,16 @@ You can also replace just a selection of entries::
$ binman replace -i image.bin "*u-boot*" -I indir
+Signing FIT with private key in an image
+---------------------------
+
+If your firmware image contains a FIT container you can sign container in
+place. For example::
+
+ $ binman sign -i image.bin -k privatekey -a sha256,rsa4096 -f fit.fit fit
+
+which will sign the FIT's content with private key and replace it immediately
+inside your image.
Logging
-------
--
2.35.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/3] binman: add test for sign option
2022-03-21 21:43 [PATCH 0/3] Introduce new sign binman's option Ivan Mikhaylov
2022-03-21 21:43 ` [PATCH 1/3] binman: add sign option for binman Ivan Mikhaylov
2022-03-21 21:43 ` [PATCH 2/3] binman: add documentation for binman sign option Ivan Mikhaylov
@ 2022-03-21 21:43 ` Ivan Mikhaylov
2022-04-08 15:39 ` Sean Anderson
2022-08-13 14:59 ` [PATCH 0/3] Introduce new sign binman's option Simon Glass
3 siblings, 1 reply; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-03-21 21:43 UTC (permalink / raw)
To: Simon Glass, Jan Kiszka; +Cc: u-boot, Ivan Mikhaylov, Ivan Mikhaylov
Add the test which provides sequence of actions:
1. create the image from binman dts
2. create public and private keys
3. add public key into dtb with fdt_add_pubkey
4. sign image with new sign option
5. check with fit_check_sign
Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
---
tools/binman/ftest.py | 42 +++++++++++++++++++
tools/binman/test/225_fit_sign.dts | 67 ++++++++++++++++++++++++++++++
2 files changed, 109 insertions(+)
create mode 100644 tools/binman/test/225_fit_sign.dts
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 8f00db6945..8a3d3720c4 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -3088,6 +3088,48 @@ class TestFunctional(unittest.TestCase):
self.assertEqual(len(U_BOOT_DATA), entry.contents_size)
self.assertEqual(len(U_BOOT_DATA), entry.size)
+ def testSignSimple(self):
+ """Test signing a single file"""
+
+ data = self._DoReadFileRealDtb('225_fit_sign.dts')
+
+ updated_fname = tools.GetOutputFilename('image-updated.bin')
+ tools.WriteFile(updated_fname, data)
+
+ outdir = os.path.join(self._indir, 'extract')
+ einfos = control.ExtractEntries(updated_fname, None, outdir, [])
+
+ dtb = tools.GetOutputFilename('source.dtb')
+ private_key = tools.GetOutputFilename('test_key.key')
+ public_key = tools.GetOutputFilename('test_key.crt')
+ fit = tools.GetOutputFilename('fit.fit')
+ key_dir = tools.GetOutputDir()
+
+ def check_sign(fit, key):
+ try:
+ tools.Run('fit_check_sign', '-k', key, '-f', fit)
+ except:
+ return False
+ return True
+
+ is_signed = False
+ try:
+ tools.Run('openssl', 'req', '-batch' , '-newkey', 'rsa:4096',
+ '-sha256', '-new', '-nodes', '-x509', '-keyout',
+ private_key, '-out', public_key)
+ tools.Run('fdt_add_pubkey', '-a', 'sha256,rsa4096', '-k', key_dir,
+ '-n', 'test_key', dtb)
+ with test_util.capture_sys_output() as (stdout, stderr):
+ # do sign with private key
+ self._DoBinman('sign', '-i', updated_fname, '-k', private_key,
+ '-a', 'sha256,rsa4096', '-f', fit, 'fit')
+ is_signed = check_sign(fit, dtb)
+ finally:
+ shutil.rmtree(key_dir)
+
+ self.assertEqual(is_signed, True)
+
+
def _RunReplaceCmd(self, entry_name, data, decomp=True, allow_resize=True,
dts='132_replace.dts'):
"""Replace an entry in an image
diff --git a/tools/binman/test/225_fit_sign.dts b/tools/binman/test/225_fit_sign.dts
new file mode 100644
index 0000000000..2bfa826106
--- /dev/null
+++ b/tools/binman/test/225_fit_sign.dts
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ binman {
+ size = <0x100000>;
+ allow-repack;
+
+ u-boot {
+ };
+
+ fit {
+ description = "U-Boot";
+ offset = <0x10000>;
+ images {
+ u-boot-1 {
+ description = "U-Boot";
+ type = "standalone";
+ arch = "arm64";
+ os = "u-boot";
+ compression = "none";
+ hash-1 {
+ algo = "sha256";
+ };
+ };
+
+ fdt-1 {
+ description = "test.dtb";
+ type = "flat_dt";
+ arch = "arm64";
+ compression = "none";
+ hash-1 {
+ algo = "sha256";
+ };
+ u-boot-spl-dtb {
+ };
+ };
+
+ };
+
+ configurations {
+ default = "conf-1";
+ conf-1 {
+ description = "u-boot with fdt";
+ firmware = "u-boot-1";
+ fdt = "fdt-1";
+ signature-1 {
+ algo = "sha256,rsa4096";
+ key-name-hint = "test_key";
+ sign-images = "firmware", "fdt";
+ };
+
+ };
+ };
+ };
+
+ u-boot-nodtb {
+ };
+
+ fdtmap {
+ };
+ };
+};
--
2.35.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-03-21 21:43 ` [PATCH 1/3] binman: add sign option for binman Ivan Mikhaylov
@ 2022-04-05 18:54 ` Alper Nebi Yasak
2022-04-06 20:28 ` Ivan Mikhaylov
0 siblings, 1 reply; 25+ messages in thread
From: Alper Nebi Yasak @ 2022-04-05 18:54 UTC (permalink / raw)
To: Ivan Mikhaylov
Cc: U-Boot Mailing List, Ivan Mikhaylov, Simon Glass, Jan Kiszka
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:
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-04-05 18:54 ` Alper Nebi Yasak
@ 2022-04-06 20:28 ` Ivan Mikhaylov
2022-04-06 22:22 ` Alper Nebi Yasak
0 siblings, 1 reply; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-04-06 20:28 UTC (permalink / raw)
To: Alper Nebi Yasak
Cc: U-Boot Mailing List, Ivan Mikhaylov, Simon Glass, Jan Kiszka
On Tue, 2022-04-05 at 21:54 +0300, Alper Nebi Yasak wrote:
> 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).
I have both of these things already:
1.extract->sign->replace
binman sign -i flash.bin -k privatekey -a sha256,rsa4096 fit
2.sign->replace
binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit.fit
fit
Just waited for review to see in which direction should I move.
I've the case when I need only FIT image sign and replace after that. I
think they both fit in 'sign' option. Okay about new method, so we
talking about just one call like 'SignEntries' without mkimage and
other steps with tools?
>
> > 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.
Should we care about such possibility in terms of these patches?
>
> > 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().
Yeap, I agree, as I said above about 'how it should be'. Do you think
it should be like additional implementation for mkimage in etype/fit.py
which should be used in SignEntries inside? Something like:
def SignEntries(params):
fit = SignFIT(params)
return ReplaceEntries(params with fit)
Anyways, waiting for Simon's review.
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-04-06 20:28 ` Ivan Mikhaylov
@ 2022-04-06 22:22 ` Alper Nebi Yasak
2022-09-06 16:27 ` Ivan Mikhaylov
0 siblings, 1 reply; 25+ messages in thread
From: Alper Nebi Yasak @ 2022-04-06 22:22 UTC (permalink / raw)
To: Ivan Mikhaylov
Cc: U-Boot Mailing List, Ivan Mikhaylov, Simon Glass, Jan Kiszka
On 06/04/2022 23:28, Ivan Mikhaylov wrote:
> On Tue, 2022-04-05 at 21:54 +0300, Alper Nebi Yasak wrote:
>> 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).
>
> I have both of these things already:
> 1.extract->sign->replace
> binman sign -i flash.bin -k privatekey -a sha256,rsa4096 fit
> 2.sign->replace
> binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit.fit
> fit
>
> Just waited for review to see in which direction should I move.
>
> I've the case when I need only FIT image sign and replace after that. I
> think they both fit in 'sign' option. Okay about new method, so we
> talking about just one call like 'SignEntries' without mkimage and
> other steps with tools?
See below...
>>
>>> 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.
>
> Should we care about such possibility in terms of these patches?
There's already vblock and pre-load entry types where 'sign' is a valid
thing to do. If we add a 'binman sign', it's reasonable to expect it to
work on those as well. But these 'algo' and 'key' arguments don't
directly apply to vblock, its signing interface is a bit weird and needs
different arguments.
Personally, I'm not sure if I'd use 'binman sign' as I like to
clean-build things, so the argument mismatch is not that big of a
concern for me.
>>
>>> 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().
>
> Yeap, I agree, as I said above about 'how it should be'. Do you think
> it should be like additional implementation for mkimage in etype/fit.py
> which should be used in SignEntries inside? Something like:
> def SignEntries(params):
> fit = SignFIT(params)
> return ReplaceEntries(params with fit)
Don't call ReplaceEntries(). Try to copy what it does, but don't call
entry.WriteData() either. Replacing FIT sections is broken on master for
now, it tries to rebuild itself to the original specification. I'm
planning to fix it, but the way I'm thinking of is demoting the entry to
'blob-ext' when replaced. It wouldn't be nice if that happened also when
we're signing an entry.
I think SignEntries would be more like (untested):
def SignEntries(image_fname, entry_paths, ...):
image_fname = os.path.abspath(image_fname)
image = Image.FromFile(image_fname)
state.PrepareFromLoadedData(image)
image.LoadData()
for entry_path in entry_paths:
entry = image.FindEntryPath(entry_path)
entry.UpdateSignatures(...) # TODO
ProcessImage(image, update_fdt=True, write_map=False,
get_contents=False, allow_resize=False)
Then you'd implement UpdateSignatures in fit.py to configure the entry
to use the given params. I'm not sure how exactly, I thought you'd call
mkimage there, but it might be necessary to edit the underlying binman
control dtb to change the signature nodes.
>
> Anyways, waiting for Simon's review.
>
> Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/3] binman: add test for sign option
2022-03-21 21:43 ` [PATCH 3/3] binman: add test for " Ivan Mikhaylov
@ 2022-04-08 15:39 ` Sean Anderson
2022-04-08 19:26 ` Ivan Mikhaylov
0 siblings, 1 reply; 25+ messages in thread
From: Sean Anderson @ 2022-04-08 15:39 UTC (permalink / raw)
To: Ivan Mikhaylov, Simon Glass, Jan Kiszka; +Cc: u-boot, Ivan Mikhaylov
On 3/21/22 5:43 PM, Ivan Mikhaylov wrote:
> Add the test which provides sequence of actions:
> 1. create the image from binman dts
> 2. create public and private keys
> 3. add public key into dtb with fdt_add_pubkey
> 4. sign image with new sign option
> 5. check with fit_check_sign
>
> Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
> ---
> tools/binman/ftest.py | 42 +++++++++++++++++++
> tools/binman/test/225_fit_sign.dts | 67 ++++++++++++++++++++++++++++++
> 2 files changed, 109 insertions(+)
> create mode 100644 tools/binman/test/225_fit_sign.dts
>
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 8f00db6945..8a3d3720c4 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -3088,6 +3088,48 @@ class TestFunctional(unittest.TestCase):
> self.assertEqual(len(U_BOOT_DATA), entry.contents_size)
> self.assertEqual(len(U_BOOT_DATA), entry.size)
>
> + def testSignSimple(self):
> + """Test signing a single file"""
> +
> + data = self._DoReadFileRealDtb('225_fit_sign.dts')
> +
> + updated_fname = tools.GetOutputFilename('image-updated.bin')
> + tools.WriteFile(updated_fname, data)
> +
> + outdir = os.path.join(self._indir, 'extract')
> + einfos = control.ExtractEntries(updated_fname, None, outdir, [])
> +
> + dtb = tools.GetOutputFilename('source.dtb')
> + private_key = tools.GetOutputFilename('test_key.key')
> + public_key = tools.GetOutputFilename('test_key.crt')
> + fit = tools.GetOutputFilename('fit.fit')
> + key_dir = tools.GetOutputDir()
> +
> + def check_sign(fit, key):
please inline this, since it is only called once
> + try:
> + tools.Run('fit_check_sign', '-k', key, '-f', fit)
> + except:
> + return False
you can just do a bare tools.Run() and if an exception is raised it will
cause the test to fail.
> + return True
> +
> + is_signed = False
> + try:
> + tools.Run('openssl', 'req', '-batch' , '-newkey', 'rsa:4096',
> + '-sha256', '-new', '-nodes', '-x509', '-keyout',
> + private_key, '-out', public_key)
> + tools.Run('fdt_add_pubkey', '-a', 'sha256,rsa4096', '-k', key_dir,
> + '-n', 'test_key', dtb)
> + with test_util.capture_sys_output() as (stdout, stderr):
> + # do sign with private key
> + self._DoBinman('sign', '-i', updated_fname, '-k', private_key,
> + '-a', 'sha256,rsa4096', '-f', fit, 'fit')
> + is_signed = check_sign(fit, dtb)
> + finally:
> + shutil.rmtree(key_dir)
> +
> + self.assertEqual(is_signed, True)
so no need for this assert here
> +
> +
> def _RunReplaceCmd(self, entry_name, data, decomp=True, allow_resize=True,
> dts='132_replace.dts'):
> """Replace an entry in an image
> diff --git a/tools/binman/test/225_fit_sign.dts b/tools/binman/test/225_fit_sign.dts
> new file mode 100644
> index 0000000000..2bfa826106
> --- /dev/null
> +++ b/tools/binman/test/225_fit_sign.dts
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + binman {
I don't really understand what's going on in this test case.
> + size = <0x100000>;
> + allow-repack;
> +
> + u-boot {
> + };
> +
> + fit {
> + description = "U-Boot";
> + offset = <0x10000>;
> + images {
> + u-boot-1 {
> + description = "U-Boot";
> + type = "standalone";
> + arch = "arm64";
> + os = "u-boot";
> + compression = "none";
> + hash-1 {
> + algo = "sha256";
> + };
Shouldn't there be some kind of data here?
> + };
> +
> + fdt-1 {
Maybe use things like @fdt-SEQ, as seen in tools/binman/test/170_fit_fdt.dts?
> + description = "test.dtb";
> + type = "flat_dt";
> + arch = "arm64";
> + compression = "none";
> + hash-1 {
> + algo = "sha256";
> + };
> + u-boot-spl-dtb {
> + };
> + };
> +
> + };
> +
> + configurations {
> + default = "conf-1";
> + conf-1 {
> + description = "u-boot with fdt";
> + firmware = "u-boot-1";
> + fdt = "fdt-1";
> + signature-1 {
> + algo = "sha256,rsa4096";
> + key-name-hint = "test_key";
> + sign-images = "firmware", "fdt";
> + };
> +
> + };
> + };
> + };
> +
> + u-boot-nodtb {
> + };
Why do we have U-Boot again here without a device tree?
> + fdtmap {
> + };
> + };
> +};
>
What is U-Boot supposed to be verified by? Shouldn't this package SPL?
I guess that is out of scope?
--Sean
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/3] binman: add test for sign option
2022-04-08 15:39 ` Sean Anderson
@ 2022-04-08 19:26 ` Ivan Mikhaylov
2022-04-10 22:37 ` Alper Nebi Yasak
0 siblings, 1 reply; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-04-08 19:26 UTC (permalink / raw)
To: Sean Anderson, Simon Glass, Jan Kiszka; +Cc: u-boot, Ivan Mikhaylov
On Fri, 2022-04-08 at 11:39 -0400, Sean Anderson wrote:
>
>
> On 3/21/22 5:43 PM, Ivan Mikhaylov wrote:
> > Add the test which provides sequence of actions:
> > 1. create the image from binman dts
> > 2. create public and private keys
> > 3. add public key into dtb with fdt_add_pubkey
> > 4. sign image with new sign option
> > 5. check with fit_check_sign
> >
> > Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
> > ---
> > tools/binman/ftest.py | 42 +++++++++++++++++++
> > tools/binman/test/225_fit_sign.dts | 67
> > ++++++++++++++++++++++++++++++
> > 2 files changed, 109 insertions(+)
> > create mode 100644 tools/binman/test/225_fit_sign.dts
> >
> > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> > index 8f00db6945..8a3d3720c4 100644
> > --- a/tools/binman/ftest.py
> > +++ b/tools/binman/ftest.py
> > @@ -3088,6 +3088,48 @@ class TestFunctional(unittest.TestCase):
> > self.assertEqual(len(U_BOOT_DATA), entry.contents_size)
> > self.assertEqual(len(U_BOOT_DATA), entry.size)
> >
> > + def testSignSimple(self):
> > + """Test signing a single file"""
> > +
> > + data = self._DoReadFileRealDtb('225_fit_sign.dts')
> > +
> > + updated_fname = tools.GetOutputFilename('image-
> > updated.bin')
> > + tools.WriteFile(updated_fname, data)
> > +
> > + outdir = os.path.join(self._indir, 'extract')
> > + einfos = control.ExtractEntries(updated_fname, None,
> > outdir, [])
> > +
> > + dtb = tools.GetOutputFilename('source.dtb')
> > + private_key = tools.GetOutputFilename('test_key.key')
> > + public_key = tools.GetOutputFilename('test_key.crt')
> > + fit = tools.GetOutputFilename('fit.fit')
> > + key_dir = tools.GetOutputDir()
> > +
> > + def check_sign(fit, key):
>
> please inline this, since it is only called once
>
> > + try:
> > + tools.Run('fit_check_sign', '-k', key, '-f', fit)
> > + except:
> > + return False
>
> you can just do a bare tools.Run() and if an exception is raised it
> will
> cause the test to fail.
Ok, good to know.
>
> > + return True
> > +
> > + is_signed = False
> > + try:
> > + tools.Run('openssl', 'req', '-batch' , '-newkey',
> > 'rsa:4096',
> > + '-sha256', '-new', '-nodes', '-x509', '-
> > keyout',
> > + private_key, '-out', public_key)
> > + tools.Run('fdt_add_pubkey', '-a', 'sha256,rsa4096', '-
> > k', key_dir,
> > + '-n', 'test_key', dtb)
> > + with test_util.capture_sys_output() as (stdout,
> > stderr):
> > + # do sign with private key
> > + self._DoBinman('sign', '-i', updated_fname, '-k',
> > private_key,
> > + '-a', 'sha256,rsa4096', '-f', fit,
> > 'fit')
> > + is_signed = check_sign(fit, dtb)
> > + finally:
> > + shutil.rmtree(key_dir)
> > +
> > + self.assertEqual(is_signed, True)
>
> so no need for this assert here
> > +
> > +
> > def _RunReplaceCmd(self, entry_name, data, decomp=True,
> > allow_resize=True,
> > dts='132_replace.dts'):
> > """Replace an entry in an image
> > diff --git a/tools/binman/test/225_fit_sign.dts
> > b/tools/binman/test/225_fit_sign.dts
> > new file mode 100644
> > index 0000000000..2bfa826106
> > --- /dev/null
> > +++ b/tools/binman/test/225_fit_sign.dts
> > @@ -0,0 +1,67 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + binman {
>
> I don't really understand what's going on in this test case.
>
> > + size = <0x100000>;
> > + allow-repack;
> > +
> > + u-boot {
> > + };
> > +
> > + fit {
> > + description = "U-Boot";
> > + offset = <0x10000>;
> > + images {
> > + u-boot-1 {
> > + description = "U-Boot";
> > + type = "standalone";
> > + arch = "arm64";
> > + os = "u-boot";
> > + compression = "none";
> > + hash-1 {
> > + algo = "sha256";
> > + };
>
> Shouldn't there be some kind of data here?
Nope.
>
> > + };
> > +
> > + fdt-1 {
>
> Maybe use things like @fdt-SEQ, as seen in
> tools/binman/test/170_fit_fdt.dts?
I'll check, thanks.
>
> > + description = "test.dtb";
> > + type = "flat_dt";
> > + arch = "arm64";
> > + compression = "none";
> > + hash-1 {
> > + algo = "sha256";
> > + };
> > + u-boot-spl-dtb {
> > + };
> > + };
> > +
> > + };
> > +
> > + configurations {
> > + default = "conf-1";
> > + conf-1 {
> > + description = "u-boot with
> > fdt";
> > + firmware = "u-boot-1";
> > + fdt = "fdt-1";
> > + signature-1 {
> > + algo =
> > "sha256,rsa4096";
> > + key-name-hint =
> > "test_key";
> > + sign-images =
> > "firmware", "fdt";
> > + };
> > +
> > + };
> > + };
> > + };
> > +
> > + u-boot-nodtb {
> > + };
>
> Why do we have U-Boot again here without a device tree?
It is done by mistake, same as for 'u-boot' on the top, I'll get rid
off both of them.
>
> > + fdtmap {
> > + };
> > + };
> > +};
> >
>
> What is U-Boot supposed to be verified by? Shouldn't this package
> SPL?
> I guess that is out of scope?
Only thing which should be verified here, that is FIT container is
signed with fit_check_sign utility.
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/3] binman: add test for sign option
2022-04-08 19:26 ` Ivan Mikhaylov
@ 2022-04-10 22:37 ` Alper Nebi Yasak
2022-04-11 15:02 ` Sean Anderson
0 siblings, 1 reply; 25+ messages in thread
From: Alper Nebi Yasak @ 2022-04-10 22:37 UTC (permalink / raw)
To: Ivan Mikhaylov, Sean Anderson
Cc: U-Boot Mailing List, Ivan Mikhaylov, Simon Glass, Jan Kiszka
I initially ignored this test because I didn't like the implementation
anyway, but I disagree with some of Sean's comments here so I wanted to
add on what I think.
On 08/04/2022 22:26, Ivan Mikhaylov wrote:
> On Fri, 2022-04-08 at 11:39 -0400, Sean Anderson wrote:
>> On 3/21/22 5:43 PM, Ivan Mikhaylov wrote:
>>> Add the test which provides sequence of actions:
>>> 1. create the image from binman dts
>>> 2. create public and private keys
>>> 3. add public key into dtb with fdt_add_pubkey
>>> 4. sign image with new sign option
>>> 5. check with fit_check_sign
>>>
>>> Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
>>> ---
>>> tools/binman/ftest.py | 42 +++++++++++++++++++
>>> tools/binman/test/225_fit_sign.dts | 67
>>> ++++++++++++++++++++++++++++++
>>> 2 files changed, 109 insertions(+)
>>> create mode 100644 tools/binman/test/225_fit_sign.dts
>>>
>>> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
>>> index 8f00db6945..8a3d3720c4 100644
>>> --- a/tools/binman/ftest.py
>>> +++ b/tools/binman/ftest.py
>>> @@ -3088,6 +3088,48 @@ class TestFunctional(unittest.TestCase):
>>> self.assertEqual(len(U_BOOT_DATA), entry.contents_size)
>>> self.assertEqual(len(U_BOOT_DATA), entry.size)
>>>
>>> + def testSignSimple(self):
>>> + """Test signing a single file"""
>>> +
>>> + data = self._DoReadFileRealDtb('225_fit_sign.dts')
>>> +
>>> + updated_fname = tools.GetOutputFilename('image-
>>> updated.bin')
>>> + tools.WriteFile(updated_fname, data)
>>> +
>>> + outdir = os.path.join(self._indir, 'extract')
>>> + einfos = control.ExtractEntries(updated_fname, None,
>>> outdir, [])
>>> +
>>> + dtb = tools.GetOutputFilename('source.dtb')
>>> + private_key = tools.GetOutputFilename('test_key.key')
>>> + public_key = tools.GetOutputFilename('test_key.crt')
>>> + fit = tools.GetOutputFilename('fit.fit')
>>> + key_dir = tools.GetOutputDir()
>>> +
>>> + def check_sign(fit, key):
>>
>> please inline this, since it is only called once
>>
>>> + try:
>>> + tools.Run('fit_check_sign', '-k', key, '-f', fit)
>>> + except:
>>> + return False
>>
>> you can just do a bare tools.Run() and if an exception is raised it
>> will
>> cause the test to fail.
>
> Ok, good to know.
It will cause a test 'error' instead of a 'failure'. You should keep the
try-except, but call self.fail(...) in the except case with a reasonable
error message. Nesting multiple try blocks is a bit ugly, so you could
keep the single-use function.
It would be even better if you added more cases testing various signing
conditions, with this as a helper function in the outer scope that all
of them can use.
You could extend it to check if the signature nodes in the updated
fdtmap match the arguments passed to 'binman sign' (but your current
implementation doesn't update those nodes).
>>
>>> + return True
>>> +
>>> + is_signed = False
>>> + try:
>>> + tools.Run('openssl', 'req', '-batch' , '-newkey',
>>> 'rsa:4096',
>>> + '-sha256', '-new', '-nodes', '-x509', '-
>>> keyout',
>>> + private_key, '-out', public_key)
>>> + tools.Run('fdt_add_pubkey', '-a', 'sha256,rsa4096', '-
>>> k', key_dir,
>>> + '-n', 'test_key', dtb)
>>> + with test_util.capture_sys_output() as (stdout,
>>> stderr):
>>> + # do sign with private key
>>> + self._DoBinman('sign', '-i', updated_fname, '-k',
>>> private_key,
>>> + '-a', 'sha256,rsa4096', '-f', fit,
>>> 'fit')
>>> + is_signed = check_sign(fit, dtb)
>>> + finally:
>>> + shutil.rmtree(key_dir)
>>> +
>>> + self.assertEqual(is_signed, True)
>>
>> so no need for this assert here
>>> +
>>> +
>>> def _RunReplaceCmd(self, entry_name, data, decomp=True,
>>> allow_resize=True,
>>> dts='132_replace.dts'):
>>> """Replace an entry in an image
New tests usually are added at the end of this file. IIRC they should be
ordered by their test dts' number.
>>> diff --git a/tools/binman/test/225_fit_sign.dts
>>> b/tools/binman/test/225_fit_sign.dts
>>> new file mode 100644
>>> index 0000000000..2bfa826106
>>> --- /dev/null
>>> +++ b/tools/binman/test/225_fit_sign.dts
>>> @@ -0,0 +1,67 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +
>>> +/dts-v1/;
>>> +
>>> +/ {
>>> + #address-cells = <1>;
>>> + #size-cells = <1>;
>>> +
>>> + binman {
>>
>> I don't really understand what's going on in this test case.
Binman mocks most entries' data during tests, the test images aren't
meaningful or usable beyond what's in the python test case code. In this
case, we only need a 'fit' entry with a signature we can run 'binman
sign' on, and a 'fdtmap' so we can recognize that the built image has
that 'fit' entry in it. The rest is just fluff.
>>
>>> + size = <0x100000>;
>>> + allow-repack;
>>> +
>>> + u-boot {
>>> + };
>>> +
>>> + fit {
>>> + description = "U-Boot";
>>> + offset = <0x10000>;
>>> + images {
>>> + u-boot-1 {
>>> + description = "U-Boot";
>>> + type = "standalone";
>>> + arch = "arm64";
>>> + os = "u-boot";
>>> + compression = "none";
>>> + hash-1 {
>>> + algo = "sha256";
>>> + };
>>
>> Shouldn't there be some kind of data here?
>
> Nope.
You could add a 'u-boot' entry here. It's weird seeing a hash/signature
for empty data.
>>
>>> + };
>>> +
>>> + fdt-1 {
>>
>> Maybe use things like @fdt-SEQ, as seen in
>> tools/binman/test/170_fit_fdt.dts?
>
> I'll check, thanks.
If you do so, I would prefer that as an additional test case. No need to
replace this one.
>>
>>> + description = "test.dtb";
>>> + type = "flat_dt";
>>> + arch = "arm64";
>>> + compression = "none";
>>> + hash-1 {
>>> + algo = "sha256";
>>> + };
>>> + u-boot-spl-dtb {
>>> + };
>>> + };
>>> +
>>> + };
>>> +
>>> + configurations {
>>> + default = "conf-1";
>>> + conf-1 {
>>> + description = "u-boot with
>>> fdt";
>>> + firmware = "u-boot-1";
>>> + fdt = "fdt-1";
>>> + signature-1 {
>>> + algo =
>>> "sha256,rsa4096";
>>> + key-name-hint =
>>> "test_key";
>>> + sign-images =
>>> "firmware", "fdt";
>>> + };
>>> +
>>> + };
>>> + };
>>> + };
>>> +
>>> + u-boot-nodtb {
>>> + };
>>
>> Why do we have U-Boot again here without a device tree?
>
> It is done by mistake, same as for 'u-boot' on the top, I'll get rid
> off both of them.
Doesn't really matter IMO. A lot of test images have the
entry-under-test sandwiched between other entries like this anyway.
>>
>>> + fdtmap {
>>> + };
>>> + };
>>> +};
>>>
>>
>> What is U-Boot supposed to be verified by? Shouldn't this package
>> SPL?
>> I guess that is out of scope?
>
> Only thing which should be verified here, that is FIT container is
> signed with fit_check_sign utility.
>
> Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/3] binman: add test for sign option
2022-04-10 22:37 ` Alper Nebi Yasak
@ 2022-04-11 15:02 ` Sean Anderson
0 siblings, 0 replies; 25+ messages in thread
From: Sean Anderson @ 2022-04-11 15:02 UTC (permalink / raw)
To: Alper Nebi Yasak, Ivan Mikhaylov
Cc: U-Boot Mailing List, Ivan Mikhaylov, Simon Glass, Jan Kiszka
On 4/10/22 6:37 PM, Alper Nebi Yasak wrote:
> I initially ignored this test because I didn't like the implementation
> anyway, but I disagree with some of Sean's comments here so I wanted to
> add on what I think.
>
> On 08/04/2022 22:26, Ivan Mikhaylov wrote:
>> On Fri, 2022-04-08 at 11:39 -0400, Sean Anderson wrote:
>>> On 3/21/22 5:43 PM, Ivan Mikhaylov wrote:
>>>> Add the test which provides sequence of actions:
>>>> 1. create the image from binman dts
>>>> 2. create public and private keys
>>>> 3. add public key into dtb with fdt_add_pubkey
>>>> 4. sign image with new sign option
>>>> 5. check with fit_check_sign
>>>>
>>>> Signed-off-by: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
>>>> ---
>>>> tools/binman/ftest.py | 42 +++++++++++++++++++
>>>> tools/binman/test/225_fit_sign.dts | 67
>>>> ++++++++++++++++++++++++++++++
>>>> 2 files changed, 109 insertions(+)
>>>> create mode 100644 tools/binman/test/225_fit_sign.dts
>>>>
>>>> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
>>>> index 8f00db6945..8a3d3720c4 100644
>>>> --- a/tools/binman/ftest.py
>>>> +++ b/tools/binman/ftest.py
>>>> @@ -3088,6 +3088,48 @@ class TestFunctional(unittest.TestCase):
>>>> self.assertEqual(len(U_BOOT_DATA), entry.contents_size)
>>>> self.assertEqual(len(U_BOOT_DATA), entry.size)
>>>>
>>>> + def testSignSimple(self):
>>>> + """Test signing a single file"""
>>>> +
>>>> + data = self._DoReadFileRealDtb('225_fit_sign.dts')
>>>> +
>>>> + updated_fname = tools.GetOutputFilename('image-
>>>> updated.bin')
>>>> + tools.WriteFile(updated_fname, data)
>>>> +
>>>> + outdir = os.path.join(self._indir, 'extract')
>>>> + einfos = control.ExtractEntries(updated_fname, None,
>>>> outdir, [])
>>>> +
>>>> + dtb = tools.GetOutputFilename('source.dtb')
>>>> + private_key = tools.GetOutputFilename('test_key.key')
>>>> + public_key = tools.GetOutputFilename('test_key.crt')
>>>> + fit = tools.GetOutputFilename('fit.fit')
>>>> + key_dir = tools.GetOutputDir()
>>>> +
>>>> + def check_sign(fit, key):
>>>
>>> please inline this, since it is only called once
>>>
>>>> + try:
>>>> + tools.Run('fit_check_sign', '-k', key, '-f', fit)
>>>> + except:
>>>> + return False
>>>
>>> you can just do a bare tools.Run() and if an exception is raised it
>>> will
>>> cause the test to fail.
>>
>> Ok, good to know.
>
> It will cause a test 'error' instead of a 'failure'. You should keep the
> try-except, but call self.fail(...) in the except case with a reasonable
> error message. Nesting multiple try blocks is a bit ugly, so you could
> keep the single-use function.
>
> It would be even better if you added more cases testing various signing
> conditions, with this as a helper function in the outer scope that all
> of them can use.
>
> You could extend it to check if the signature nodes in the updated
> fdtmap match the arguments passed to 'binman sign' (but your current
> implementation doesn't update those nodes).
>
>>>
>>>> + return True
>>>> +
>>>> + is_signed = False
>>>> + try:
>>>> + tools.Run('openssl', 'req', '-batch' , '-newkey',
>>>> 'rsa:4096',
>>>> + '-sha256', '-new', '-nodes', '-x509', '-
>>>> keyout',
>>>> + private_key, '-out', public_key)
>>>> + tools.Run('fdt_add_pubkey', '-a', 'sha256,rsa4096', '-
>>>> k', key_dir,
>>>> + '-n', 'test_key', dtb)
>>>> + with test_util.capture_sys_output() as (stdout,
>>>> stderr):
>>>> + # do sign with private key
>>>> + self._DoBinman('sign', '-i', updated_fname, '-k',
>>>> private_key,
>>>> + '-a', 'sha256,rsa4096', '-f', fit,
>>>> 'fit')
>>>> + is_signed = check_sign(fit, dtb)
>>>> + finally:
>>>> + shutil.rmtree(key_dir)
>>>> +
>>>> + self.assertEqual(is_signed, True)
>>>
>>> so no need for this assert here
>>>> +
>>>> +
>>>> def _RunReplaceCmd(self, entry_name, data, decomp=True,
>>>> allow_resize=True,
>>>> dts='132_replace.dts'):
>>>> """Replace an entry in an image
>
> New tests usually are added at the end of this file. IIRC they should be
> ordered by their test dts' number.
>
>>>> diff --git a/tools/binman/test/225_fit_sign.dts
>>>> b/tools/binman/test/225_fit_sign.dts
>>>> new file mode 100644
>>>> index 0000000000..2bfa826106
>>>> --- /dev/null
>>>> +++ b/tools/binman/test/225_fit_sign.dts
>>>> @@ -0,0 +1,67 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +
>>>> +/dts-v1/;
>>>> +
>>>> +/ {
>>>> + #address-cells = <1>;
>>>> + #size-cells = <1>;
>>>> +
>>>> + binman {
>>>
>>> I don't really understand what's going on in this test case.
>
> Binman mocks most entries' data during tests, the test images aren't
> meaningful or usable beyond what's in the python test case code. In this
> case, we only need a 'fit' entry with a signature we can run 'binman
> sign' on, and a 'fdtmap' so we can recognize that the built image has
> that 'fit' entry in it. The rest is just fluff.
I was primarily confused by the duplicate/extraneous binaries also
present. In addition to being tests, these device trees also function as
examples, so I think it is desirable for them to be somewhat sane.
--Sean
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/3] Introduce new sign binman's option
2022-03-21 21:43 [PATCH 0/3] Introduce new sign binman's option Ivan Mikhaylov
` (2 preceding siblings ...)
2022-03-21 21:43 ` [PATCH 3/3] binman: add test for " Ivan Mikhaylov
@ 2022-08-13 14:59 ` Simon Glass
2022-08-15 21:51 ` Ivan Mikhaylov
3 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2022-08-13 14:59 UTC (permalink / raw)
To: Ivan Mikhaylov; +Cc: Jan Kiszka, U-Boot Mailing List
Hi Ivan,
On Mon, 21 Mar 2022 at 12:43, Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>
> From: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
>
> This patch introduces prototype of new sign binman's option.
> Enhancing the sign procedure, as example:
>
> mkimage -G privateky -r -o sha256,rsa4096 -F fit.fit
> binman replace -i flash.bin -f fit.fit fit
>
> into:
> binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit.fit fit
>
> It works with extracted FIT container and image, which provides key signing
> and replacing FIT container in directed image.
>
> Also, I'll add additional enhancement in future to this procedure with
> skipping on FIT container and providing extract->sign->replace in whole
> instead of sign->replace with documentation update and test as well.
>
> As example:
>
> binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit
>
>
> Ivan Mikhaylov (3):
> binman: add sign option for binman
> binman: add documentation for binman sign option
> binman: add test for sign option
>
> tools/binman/binman.rst | 10 +++++
> tools/binman/cmdline.py | 13 ++++++
> tools/binman/control.py | 26 +++++++++++-
> tools/binman/ftest.py | 42 +++++++++++++++++++
> tools/binman/test/225_fit_sign.dts | 67 ++++++++++++++++++++++++++++++
> 5 files changed, 157 insertions(+), 1 deletion(-)
> create mode 100644 tools/binman/test/225_fit_sign.dts
I see Alper's comments. Are you going to send a new version sometime?
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/3] Introduce new sign binman's option
2022-08-13 14:59 ` [PATCH 0/3] Introduce new sign binman's option Simon Glass
@ 2022-08-15 21:51 ` Ivan Mikhaylov
0 siblings, 0 replies; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-08-15 21:51 UTC (permalink / raw)
To: Simon Glass; +Cc: Jan Kiszka, U-Boot Mailing List
On Sat, 2022-08-13 at 08:59 -0600, Simon Glass wrote:
> Hi Ivan,
>
> On Mon, 21 Mar 2022 at 12:43, Ivan Mikhaylov <fr0st61te@gmail.com>
> wrote:
> >
> > From: Ivan Mikhaylov <ivan.mikhaylov@siemens.com>
> >
> > This patch introduces prototype of new sign binman's option.
> > Enhancing the sign procedure, as example:
> >
> > mkimage -G privateky -r -o sha256,rsa4096 -F fit.fit
> > binman replace -i flash.bin -f fit.fit fit
> >
> > into:
> > binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit.fit
> > fit
> >
> > It works with extracted FIT container and image, which provides key
> > signing
> > and replacing FIT container in directed image.
> >
> > Also, I'll add additional enhancement in future to this procedure
> > with
> > skipping on FIT container and providing extract->sign->replace in
> > whole
> > instead of sign->replace with documentation update and test as
> > well.
> >
> > As example:
> >
> > binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f fit
> >
> >
> > Ivan Mikhaylov (3):
> > binman: add sign option for binman
> > binman: add documentation for binman sign option
> > binman: add test for sign option
> >
> > tools/binman/binman.rst | 10 +++++
> > tools/binman/cmdline.py | 13 ++++++
> > tools/binman/control.py | 26 +++++++++++-
> > tools/binman/ftest.py | 42 +++++++++++++++++++
> > tools/binman/test/225_fit_sign.dts | 67
> > ++++++++++++++++++++++++++++++
> > 5 files changed, 157 insertions(+), 1 deletion(-)
> > create mode 100644 tools/binman/test/225_fit_sign.dts
>
> I see Alper's comments. Are you going to send a new version sometime?
>
> Regards,
> Simon
Simon, totally forgot about this one, sorry for long delay. Yes, I'll
back to it at the end of this month/start of september.
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-04-06 22:22 ` Alper Nebi Yasak
@ 2022-09-06 16:27 ` Ivan Mikhaylov
2022-09-07 21:10 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-09-06 16:27 UTC (permalink / raw)
To: Alper Nebi Yasak
Cc: U-Boot Mailing List, Ivan Mikhaylov, Simon Glass, Jan Kiszka
On Thu, 2022-04-07 at 01:22 +0300, Alper Nebi Yasak wrote:
> On 06/04/2022 23:28, Ivan Mikhaylov wrote:
> > On Tue, 2022-04-05 at 21:54 +0300, Alper Nebi Yasak wrote:
> > > 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).
> >
> > I have both of these things already:
> > 1.extract->sign->replace
> > binman sign -i flash.bin -k privatekey -a sha256,rsa4096 fit
> > 2.sign->replace
> > binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f
> > fit.fit
> > fit
> >
> > Just waited for review to see in which direction should I move.
> >
> > I've the case when I need only FIT image sign and replace after
> > that. I
> > think they both fit in 'sign' option. Okay about new method, so we
> > talking about just one call like 'SignEntries' without mkimage and
> > other steps with tools?
>
> See below...
>
> > >
> > > > 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.
> >
> > Should we care about such possibility in terms of these patches?
>
> There's already vblock and pre-load entry types where 'sign' is a
> valid
> thing to do. If we add a 'binman sign', it's reasonable to expect it
> to
> work on those as well. But these 'algo' and 'key' arguments don't
> directly apply to vblock, its signing interface is a bit weird and
> needs
> different arguments.
>
> Personally, I'm not sure if I'd use 'binman sign' as I like to
> clean-build things, so the argument mismatch is not that big of a
> concern for me.
>
> > >
> > > > 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().
> >
> > Yeap, I agree, as I said above about 'how it should be'. Do you
> > think
> > it should be like additional implementation for mkimage in
> > etype/fit.py
> > which should be used in SignEntries inside? Something like:
> > def SignEntries(params):
> > fit = SignFIT(params)
> > return ReplaceEntries(params with fit)
>
> Don't call ReplaceEntries(). Try to copy what it does, but don't call
> entry.WriteData() either. Replacing FIT sections is broken on master
> for
> now, it tries to rebuild itself to the original specification. I'm
> planning to fix it, but the way I'm thinking of is demoting the entry
> to
> 'blob-ext' when replaced. It wouldn't be nice if that happened also
> when
> we're signing an entry.
>
> I think SignEntries would be more like (untested):
>
> def SignEntries(image_fname, entry_paths, ...):
> image_fname = os.path.abspath(image_fname)
> image = Image.FromFile(image_fname)
> state.PrepareFromLoadedData(image)
> image.LoadData()
>
> for entry_path in entry_paths:
> entry = image.FindEntryPath(entry_path)
> entry.UpdateSignatures(...) # TODO
>
> ProcessImage(image, update_fdt=True, write_map=False,
> get_contents=False, allow_resize=False)
>
> Then you'd implement UpdateSignatures in fit.py to configure the
> entry
> to use the given params. I'm not sure how exactly, I thought you'd
> call
> mkimage there, but it might be necessary to edit the underlying
> binman
> control dtb to change the signature nodes.
Alper, this way works but until 74d3b231(binman: Create FIT subentries
in the FIT section, not its parent) commit. self.data or self.GetData()
is None for entry which need to be signed after this commit. Any ideas
how to get the data of fit entry?
Thanks.
>
> >
> > Anyways, waiting for Simon's review.
> >
> > Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-09-06 16:27 ` Ivan Mikhaylov
@ 2022-09-07 21:10 ` Simon Glass
2022-09-15 22:44 ` Ivan Mikhaylov
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2022-09-07 21:10 UTC (permalink / raw)
To: Ivan Mikhaylov
Cc: Alper Nebi Yasak, U-Boot Mailing List, Ivan Mikhaylov, Jan Kiszka
Hi Ivan,
On Tue, 6 Sept 2022 at 07:27, Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>
> On Thu, 2022-04-07 at 01:22 +0300, Alper Nebi Yasak wrote:
> > On 06/04/2022 23:28, Ivan Mikhaylov wrote:
> > > On Tue, 2022-04-05 at 21:54 +0300, Alper Nebi Yasak wrote:
> > > > 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).
> > >
> > > I have both of these things already:
> > > 1.extract->sign->replace
> > > binman sign -i flash.bin -k privatekey -a sha256,rsa4096 fit
> > > 2.sign->replace
> > > binman sign -i flash.bin -k privatekey -a sha256,rsa4096 -f
> > > fit.fit
> > > fit
> > >
> > > Just waited for review to see in which direction should I move.
> > >
> > > I've the case when I need only FIT image sign and replace after
> > > that. I
> > > think they both fit in 'sign' option. Okay about new method, so we
> > > talking about just one call like 'SignEntries' without mkimage and
> > > other steps with tools?
> >
> > See below...
> >
> > > >
> > > > > 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.
> > >
> > > Should we care about such possibility in terms of these patches?
> >
> > There's already vblock and pre-load entry types where 'sign' is a
> > valid
> > thing to do. If we add a 'binman sign', it's reasonable to expect it
> > to
> > work on those as well. But these 'algo' and 'key' arguments don't
> > directly apply to vblock, its signing interface is a bit weird and
> > needs
> > different arguments.
> >
> > Personally, I'm not sure if I'd use 'binman sign' as I like to
> > clean-build things, so the argument mismatch is not that big of a
> > concern for me.
> >
> > > >
> > > > > 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().
> > >
> > > Yeap, I agree, as I said above about 'how it should be'. Do you
> > > think
> > > it should be like additional implementation for mkimage in
> > > etype/fit.py
> > > which should be used in SignEntries inside? Something like:
> > > def SignEntries(params):
> > > fit = SignFIT(params)
> > > return ReplaceEntries(params with fit)
> >
> > Don't call ReplaceEntries(). Try to copy what it does, but don't call
> > entry.WriteData() either. Replacing FIT sections is broken on master
> > for
> > now, it tries to rebuild itself to the original specification. I'm
> > planning to fix it, but the way I'm thinking of is demoting the entry
> > to
> > 'blob-ext' when replaced. It wouldn't be nice if that happened also
> > when
> > we're signing an entry.
> >
> > I think SignEntries would be more like (untested):
> >
> > def SignEntries(image_fname, entry_paths, ...):
> > image_fname = os.path.abspath(image_fname)
> > image = Image.FromFile(image_fname)
> > state.PrepareFromLoadedData(image)
> > image.LoadData()
> >
> > for entry_path in entry_paths:
> > entry = image.FindEntryPath(entry_path)
> > entry.UpdateSignatures(...) # TODO
> >
> > ProcessImage(image, update_fdt=True, write_map=False,
> > get_contents=False, allow_resize=False)
> >
> > Then you'd implement UpdateSignatures in fit.py to configure the
> > entry
> > to use the given params. I'm not sure how exactly, I thought you'd
> > call
> > mkimage there, but it might be necessary to edit the underlying
> > binman
> > control dtb to change the signature nodes.
>
> Alper, this way works but until 74d3b231(binman: Create FIT subentries
> in the FIT section, not its parent) commit. self.data or self.GetData()
> is None for entry which need to be signed after this commit. Any ideas
> how to get the data of fit entry?
Section data comes from the BuildSectionData() method, so you could
try calling that.
See also collect_contents_to_file()
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-09-07 21:10 ` Simon Glass
@ 2022-09-15 22:44 ` Ivan Mikhaylov
2022-11-18 20:50 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-09-15 22:44 UTC (permalink / raw)
To: Simon Glass
Cc: Alper Nebi Yasak, U-Boot Mailing List, Ivan Mikhaylov, Jan Kiszka
On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> Hi Ivan,
>
> Section data comes from the BuildSectionData() method, so you could
> try calling that.
>
> See also collect_contents_to_file()
>
> Regards,
> Simon
Simon, I've tried both these ways and they both don't work to me. What
I've got:
def SignEntries(image_fname, input_fname, privatekey_fname, algo,
entry_paths):
image_fname = os.path.abspath(image_fname)
image = Image.FromFile(image_fname)
state.PrepareFromLoadedData(image)
image.LoadData()
1. BuildSectionData
for entry_path in entry_paths:
entry = image.FindEntryPath(entry_path)
try:
entry.BuildSectionData(True)
except Exception as e:
logging.error(traceback.format_exc())
ERROR:root:AttributeError: 'NoneType' object has no attribute 'run'
2. collect_contents_to_file
for entry_path in entry_paths:
entry = image.FindEntryPath(entry_path)
try:
entry.collect_contents_to_file([entry.name], "prefix",
1024)
except Exception as e:
logging.error(traceback.format_exc())
ERROR:root:AttributeError: 'str' object has no attribute
'ObtainContents'
3. GetData
for entry_path in entry_paths:
entry = image.FindEntryPath(entry_path)
print("--- DATA ---")
data = entry.GetData(True)
print(data)
print("~~~ DATA ~~~")
--- DATA ---
Node '/fit/images/u-boot-1/u-boot': GetData: size 0x4
Node '/fit/images/u-boot-1': GetPaddedDataForEntry: size None
Node '/fit/images/u-boot-1': GetData: 1 entries, total size 0x4
Node '/fit/images/fdt-1/u-boot-spl-dtb': GetData: size 0x4f7
Node '/fit/images/fdt-1': GetPaddedDataForEntry: size None
Node '/fit/images/fdt-1': GetData: 1 entries, total size 0x4f7
Deleted temporary directory '/tmp/binman.z81eqcfz'
binman: 'NoneType' object has no attribute 'run'
There is no problem with getting data from GetData around start of the
year. Maybe some regression?
All this ran with this:
binman -v5 sign -i image.bin -k test_key.key -a sha256,rsa4096 fit
`fit` in entry_paths and image contains FIT section with name `fit`.
binman ls -i image.bin
Name Image-pos Size Entry-type Offset
Uncomp-size
-----------------------------------------------------------------------
--------
main-section 0 100000 section 0
fit 10000 c0a fit 10000
u-boot-1 10104 4 section 104
u-boot 10104 4 u-boot 0
fdt-1 101c8 4f7 section 1c8
u-boot-spl-dtb 101c8 4f7 u-boot-spl-dtb 0
fdtmap 10c0a 4f5 fdtmap 10c0a
Seems something went wrong, any ideas? Or did I misuse?
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-09-15 22:44 ` Ivan Mikhaylov
@ 2022-11-18 20:50 ` Simon Glass
2022-12-13 21:51 ` Ivan Mikhaylov
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2022-11-18 20:50 UTC (permalink / raw)
To: Ivan Mikhaylov
Cc: Alper Nebi Yasak, U-Boot Mailing List, Ivan Mikhaylov, Jan Kiszka
Hi Ivan,
On Thu, 15 Sept 2022 at 13:44, Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>
> On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> > Hi Ivan,
> >
> > Section data comes from the BuildSectionData() method, so you could
> > try calling that.
> >
> > See also collect_contents_to_file()
> >
> > Regards,
> > Simon
>
> Simon, I've tried both these ways and they both don't work to me. What
> I've got:
>
> def SignEntries(image_fname, input_fname, privatekey_fname, algo,
> entry_paths):
> image_fname = os.path.abspath(image_fname)
> image = Image.FromFile(image_fname)
> state.PrepareFromLoadedData(image)
> image.LoadData()
>
> 1. BuildSectionData
>
> for entry_path in entry_paths:
> entry = image.FindEntryPath(entry_path)
>
> try:
> entry.BuildSectionData(True)
> except Exception as e:
> logging.error(traceback.format_exc())
>
>
> ERROR:root:AttributeError: 'NoneType' object has no attribute 'run'
>
> 2. collect_contents_to_file
>
> for entry_path in entry_paths:
> entry = image.FindEntryPath(entry_path)
>
> try:
> entry.collect_contents_to_file([entry.name], "prefix",
> 1024)
> except Exception as e:
> logging.error(traceback.format_exc())
>
> ERROR:root:AttributeError: 'str' object has no attribute
> 'ObtainContents'
This seems to be getting a string instead of an entry object. Can you
try -D to see? See 'Writing new entries and debugging'.
>
> 3. GetData
>
> for entry_path in entry_paths:
> entry = image.FindEntryPath(entry_path)
>
> print("--- DATA ---")
> data = entry.GetData(True)
> print(data)
> print("~~~ DATA ~~~")
>
> --- DATA ---
> Node '/fit/images/u-boot-1/u-boot': GetData: size 0x4
> Node '/fit/images/u-boot-1': GetPaddedDataForEntry: size None
> Node '/fit/images/u-boot-1': GetData: 1 entries, total size 0x4
> Node '/fit/images/fdt-1/u-boot-spl-dtb': GetData: size 0x4f7
> Node '/fit/images/fdt-1': GetPaddedDataForEntry: size None
> Node '/fit/images/fdt-1': GetData: 1 entries, total size 0x4f7
> Deleted temporary directory '/tmp/binman.z81eqcfz'
> binman: 'NoneType' object has no attribute 'run'
This might be trying to call tools.run() so use -D to see where the error is.
>
> There is no problem with getting data from GetData around start of the
> year. Maybe some regression?
>
> All this ran with this:
> binman -v5 sign -i image.bin -k test_key.key -a sha256,rsa4096 fit
>
> `fit` in entry_paths and image contains FIT section with name `fit`.
>
> binman ls -i image.bin
> Name Image-pos Size Entry-type Offset
> Uncomp-size
> -----------------------------------------------------------------------
> --------
> main-section 0 100000 section 0
> fit 10000 c0a fit 10000
> u-boot-1 10104 4 section 104
> u-boot 10104 4 u-boot 0
> fdt-1 101c8 4f7 section 1c8
> u-boot-spl-dtb 101c8 4f7 u-boot-spl-dtb 0
> fdtmap 10c0a 4f5 fdtmap 10c0a
>
>
> Seems something went wrong, any ideas? Or did I misuse?
Can you please push a tree somewhere so I can try this?
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-11-18 20:50 ` Simon Glass
@ 2022-12-13 21:51 ` Ivan Mikhaylov
2022-12-17 22:02 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-12-13 21:51 UTC (permalink / raw)
To: Simon Glass
Cc: Alper Nebi Yasak, U-Boot Mailing List, Ivan Mikhaylov, Jan Kiszka
On Fri, 2022-11-18 at 13:50 -0700, Simon Glass wrote:
> Hi Ivan,
>
> On Thu, 15 Sept 2022 at 13:44, Ivan Mikhaylov <fr0st61te@gmail.com>
> wrote:
> >
> > On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> > > Hi Ivan,
> > >
> > > Section data comes from the BuildSectionData() method, so you
> > > could
> > > try calling that.
> > >
> > > See also collect_contents_to_file()
> > >
> > > Regards,
> > > Simon
> >
> > Simon, I've tried both these ways and they both don't work to me.
> > What
> > I've got:
> >
> > def SignEntries(image_fname, input_fname, privatekey_fname, algo,
> > entry_paths):
> > image_fname = os.path.abspath(image_fname)
> > image = Image.FromFile(image_fname)
> > state.PrepareFromLoadedData(image)
> > image.LoadData()
> >
> > 1. BuildSectionData
> >
> > for entry_path in entry_paths:
> > entry = image.FindEntryPath(entry_path)
> >
> > try:
> > entry.BuildSectionData(True)
> > except Exception as e:
> > logging.error(traceback.format_exc())
> >
> >
> > ERROR:root:AttributeError: 'NoneType' object has no attribute 'run'
Hi Simon, sorry for long delay.
binman: 'NoneType' object has no attribute 'run'
Traceback (most recent call last):
File "/home/fr/upstream_uboot/tools/binman/binman", line 133, in
RunBinman
ret_code = control.Binman(args)
File "/home/fr/upstream_uboot/tools/binman/control.py", line 684, in
Binman
SignEntries(args.image, args.file, args.key, args.algo, args.paths)
File "/home/fr/upstream_uboot/tools/binman/control.py", line 469, in
SignEntries
entry.BuildSectionData(True)
File "/home/fr/upstream_uboot/tools/binman/etype/fit.py", line 426,
in BuildSectionData
if self.mkimage.run(reset_timestamp=True,
output_fname=output_fname,
AttributeError: 'NoneType' object has no attribute 'run'
> >
> > 2. collect_contents_to_file
> >
> > for entry_path in entry_paths:
> > entry = image.FindEntryPath(entry_path)
> >
> > try:
> > entry.collect_contents_to_file([entry.name], "prefix",
> > 1024)
> > except Exception as e:
> > logging.error(traceback.format_exc())
> >
> > ERROR:root:AttributeError: 'str' object has no attribute
> > 'ObtainContents'
>
> This seems to be getting a string instead of an entry object. Can you
> try -D to see? See 'Writing new entries and debugging'.
Yea, you're right, seems I've added here entry.name instead of entry
but result still messy. entry here is FIT container which is 'fit':
<binman.etype.fit.Entry_fit object at 0x7f6b239cfe20>
<class 'binman.etype.fit.Entry_fit'>
binman: [Errno 2] No such file or directory: 'u-boot.bin'
Traceback (most recent call last):
File "/home/fr/upstream_uboot/tools/binman/binman", line 133, in
RunBinman
ret_code = control.Binman(args)
File "/home/fr/upstream_uboot/tools/binman/control.py", line 686, in
Binman
SignEntries(args.image, args.file, args.key, args.algo, args.paths)
File "/home/fr/upstream_uboot/tools/binman/control.py", line 471, in
SignEntries
entry.collect_contents_to_file([entry], "prefix", 1024)
File "/home/fr/upstream_uboot/tools/binman/entry.py", line 1253, in
collect_contents_to_file
if not entry.ObtainContents(fake_size=fake_size):
File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
250, in ObtainContents
return self.GetEntryContents(skip_entry=skip_entry)
File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
696, in GetEntryContents
job.result()
File "/usr/lib/python3.8/concurrent/futures/_base.py", line 437, in
result
return self.__get_result()
File "/usr/lib/python3.8/concurrent/futures/_base.py", line 389, in
__get_result
raise self._exception
File "/usr/lib/python3.8/concurrent/futures/thread.py", line 57, in
run
result = self.fn(*self.args, **self.kwargs)
File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
668, in _CheckDone
if not entry.ObtainContents():
File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
250, in ObtainContents
return self.GetEntryContents(skip_entry=skip_entry)
File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
696, in GetEntryContents
job.result()
File "/usr/lib/python3.8/concurrent/futures/_base.py", line 437, in
result
return self.__get_result()
File "/usr/lib/python3.8/concurrent/futures/_base.py", line 389, in
__get_result
raise self._exception
File "/usr/lib/python3.8/concurrent/futures/thread.py", line 57, in
run
result = self.fn(*self.args, **self.kwargs)
File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
668, in _CheckDone
if not entry.ObtainContents():
File "/home/fr/upstream_uboot/tools/binman/etype/blob.py", line 52,
in ObtainContents
self.ReadBlobContents()
File "/home/fr/upstream_uboot/tools/binman/etype/blob.py", line 82,
in ReadBlobContents
data = self.ReadFileContents(self._pathname)
File "/home/fr/upstream_uboot/tools/binman/etype/blob.py", line 74,
in ReadFileContents
indata = tools.read_file(pathname)
File "/home/fr/upstream_uboot/tools/patman/tools.py", line 467, in
read_file
with open(filename(fname), binary and 'rb' or 'r') as fd:
FileNotFoundError: [Errno 2] No such file or directory: 'u-boot.bin'
>
> >
> > 3. GetData
> >
> > for entry_path in entry_paths:
> > entry = image.FindEntryPath(entry_path)
> >
> > print("--- DATA ---")
> > data = entry.GetData(True)
> > print(data)
> > print("~~~ DATA ~~~")
> >
> > --- DATA ---
> > Node '/fit/images/u-boot-1/u-boot': GetData: size 0x4
> > Node '/fit/images/u-boot-1': GetPaddedDataForEntry: size None
> > Node '/fit/images/u-boot-1': GetData: 1 entries, total size 0x4
> > Node '/fit/images/fdt-1/u-boot-spl-dtb': GetData: size 0x4f7
> > Node '/fit/images/fdt-1': GetPaddedDataForEntry: size None
> > Node '/fit/images/fdt-1': GetData: 1 entries, total size
> > 0x4f7
> > Deleted temporary directory '/tmp/binman.z81eqcfz'
> > binman: 'NoneType' object has no attribute 'run'
>
> This might be trying to call tools.run() so use -D to see where the
> error is.
>
--- DATA ---
Node '/fit/images/u-boot-1/u-boot': GetData: size 0x4
Node '/fit/images/u-boot-1': GetPaddedDataForEntry: size None
Node '/fit/images/u-boot-1': GetData: 1 entries, total size 0x4
Node '/fit/images/fdt-1/u-boot-spl-dtb': GetData: size 0x4f7
Node '/fit/images/fdt-1': GetPaddedDataForEntry: size None
Node '/fit/images/fdt-1': GetData: 1 entries, total size 0x4f7
Deleted temporary directory '/tmp/binman.0x74lr_s'
binman: 'NoneType' object has no attribute 'run'
Traceback (most recent call last):
File "/home/fr/upstream_uboot/tools/binman/binman", line 133, in
RunBinman
ret_code = control.Binman(args)
File "/home/fr/upstream_uboot/tools/binman/control.py", line 684, in
Binman
SignEntries(args.image, args.file, args.key, args.algo, args.paths)
File "/home/fr/upstream_uboot/tools/binman/control.py", line 468, in
SignEntries
print(entry.GetData())
File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
362, in GetData
data = self.BuildSectionData(required)
File "/home/fr/upstream_uboot/tools/binman/etype/fit.py", line 426,
in BuildSectionData
if self.mkimage.run(reset_timestamp=True,
output_fname=output_fname,
AttributeError: 'NoneType' object has no attribute 'run'
This one strange to me because mkimage exists in tools directory and
has the symbolic link to /usr/local.
> >
> > There is no problem with getting data from GetData around start of
> > the
> > year. Maybe some regression?
> >
> > All this ran with this:
> > binman -v5 sign -i image.bin -k test_key.key -a sha256,rsa4096 fit
> >
> > `fit` in entry_paths and image contains FIT section with name
> > `fit`.
> >
> > binman ls -i image.bin
> > Name Image-pos Size Entry-type Offset
> > Uncomp-size
> > -------------------------------------------------------------------
> > ----
> > --------
> > main-section 0 100000 section 0
> > fit 10000 c0a fit 10000
> > u-boot-1 10104 4 section 104
> > u-boot 10104 4 u-boot 0
> > fdt-1 101c8 4f7 section 1c8
> > u-boot-spl-dtb 101c8 4f7 u-boot-spl-dtb 0
> > fdtmap 10c0a 4f5 fdtmap 10c0a
> >
> >
> > Seems something went wrong, any ideas? Or did I misuse?
>
> Can you please push a tree somewhere so I can try this?
Sure, https://github.com/fr0st61te/u-boot/tree/signfit , rebased to
current master.
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-12-13 21:51 ` Ivan Mikhaylov
@ 2022-12-17 22:02 ` Simon Glass
2022-12-25 1:35 ` Ivan Mikhaylov
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2022-12-17 22:02 UTC (permalink / raw)
To: Ivan Mikhaylov
Cc: Alper Nebi Yasak, U-Boot Mailing List, Ivan Mikhaylov, Jan Kiszka
Hi Ivan,
On Tue, 13 Dec 2022 at 11:51, Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>
> On Fri, 2022-11-18 at 13:50 -0700, Simon Glass wrote:
> > Hi Ivan,
> >
> > On Thu, 15 Sept 2022 at 13:44, Ivan Mikhaylov <fr0st61te@gmail.com>
> > wrote:
> > >
> > > On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> > > > Hi Ivan,
> > > >
> > > > Section data comes from the BuildSectionData() method, so you
> > > > could
> > > > try calling that.
> > > >
> > > > See also collect_contents_to_file()
> > > >
> > > > Regards,
> > > > Simon
> > >
> > > Simon, I've tried both these ways and they both don't work to me.
> > > What
> > > I've got:
> > >
> > > def SignEntries(image_fname, input_fname, privatekey_fname, algo,
> > > entry_paths):
> > > image_fname = os.path.abspath(image_fname)
> > > image = Image.FromFile(image_fname)
> > > state.PrepareFromLoadedData(image)
> > > image.LoadData()
> > >
> > > 1. BuildSectionData
> > >
> > > for entry_path in entry_paths:
> > > entry = image.FindEntryPath(entry_path)
> > >
> > > try:
> > > entry.BuildSectionData(True)
> > > except Exception as e:
> > > logging.error(traceback.format_exc())
> > >
> > >
> > > ERROR:root:AttributeError: 'NoneType' object has no attribute 'run'
>
> Hi Simon, sorry for long delay.
>
> binman: 'NoneType' object has no attribute 'run'
>
> Traceback (most recent call last):
> File "/home/fr/upstream_uboot/tools/binman/binman", line 133, in
> RunBinman
> ret_code = control.Binman(args)
> File "/home/fr/upstream_uboot/tools/binman/control.py", line 684, in
> Binman
> SignEntries(args.image, args.file, args.key, args.algo, args.paths)
> File "/home/fr/upstream_uboot/tools/binman/control.py", line 469, in
> SignEntries
> entry.BuildSectionData(True)
> File "/home/fr/upstream_uboot/tools/binman/etype/fit.py", line 426,
> in BuildSectionData
> if self.mkimage.run(reset_timestamp=True,
> output_fname=output_fname,
> AttributeError: 'NoneType' object has no attribute 'run'
>
You need to call image.CollectBintolls() like ReadEntry() and other
functions similar to yours that read images from a file. This is the
only way that the 'mkimage' tool becomes available to fit.py
See fit.AddBintools() which is called by that function and sets 'self.mkimage'
>
> > >
> > > 2. collect_contents_to_file
> > >
> > > for entry_path in entry_paths:
> > > entry = image.FindEntryPath(entry_path)
> > >
> > > try:
> > > entry.collect_contents_to_file([entry.name], "prefix",
> > > 1024)
> > > except Exception as e:
> > > logging.error(traceback.format_exc())
> > >
> > > ERROR:root:AttributeError: 'str' object has no attribute
> > > 'ObtainContents'
> >
> > This seems to be getting a string instead of an entry object. Can you
> > try -D to see? See 'Writing new entries and debugging'.
>
> Yea, you're right, seems I've added here entry.name instead of entry
> but result still messy. entry here is FIT container which is 'fit':
>
> <binman.etype.fit.Entry_fit object at 0x7f6b239cfe20>
> <class 'binman.etype.fit.Entry_fit'>
>
> binman: [Errno 2] No such file or directory: 'u-boot.bin'
>
> Traceback (most recent call last):
> File "/home/fr/upstream_uboot/tools/binman/binman", line 133, in
> RunBinman
> ret_code = control.Binman(args)
> File "/home/fr/upstream_uboot/tools/binman/control.py", line 686, in
> Binman
> SignEntries(args.image, args.file, args.key, args.algo, args.paths)
> File "/home/fr/upstream_uboot/tools/binman/control.py", line 471, in
> SignEntries
> entry.collect_contents_to_file([entry], "prefix", 1024)
> File "/home/fr/upstream_uboot/tools/binman/entry.py", line 1253, in
> collect_contents_to_file
> if not entry.ObtainContents(fake_size=fake_size):
> File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
> 250, in ObtainContents
> return self.GetEntryContents(skip_entry=skip_entry)
> File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
> 696, in GetEntryContents
> job.result()
> File "/usr/lib/python3.8/concurrent/futures/_base.py", line 437, in
> result
> return self.__get_result()
> File "/usr/lib/python3.8/concurrent/futures/_base.py", line 389, in
> __get_result
> raise self._exception
> File "/usr/lib/python3.8/concurrent/futures/thread.py", line 57, in
> run
> result = self.fn(*self.args, **self.kwargs)
> File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
> 668, in _CheckDone
> if not entry.ObtainContents():
> File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
> 250, in ObtainContents
> return self.GetEntryContents(skip_entry=skip_entry)
> File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
> 696, in GetEntryContents
> job.result()
> File "/usr/lib/python3.8/concurrent/futures/_base.py", line 437, in
> result
> return self.__get_result()
> File "/usr/lib/python3.8/concurrent/futures/_base.py", line 389, in
> __get_result
> raise self._exception
> File "/usr/lib/python3.8/concurrent/futures/thread.py", line 57, in
> run
> result = self.fn(*self.args, **self.kwargs)
> File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
> 668, in _CheckDone
> if not entry.ObtainContents():
> File "/home/fr/upstream_uboot/tools/binman/etype/blob.py", line 52,
> in ObtainContents
> self.ReadBlobContents()
> File "/home/fr/upstream_uboot/tools/binman/etype/blob.py", line 82,
> in ReadBlobContents
> data = self.ReadFileContents(self._pathname)
> File "/home/fr/upstream_uboot/tools/binman/etype/blob.py", line 74,
> in ReadFileContents
> indata = tools.read_file(pathname)
> File "/home/fr/upstream_uboot/tools/patman/tools.py", line 467, in
> read_file
> with open(filename(fname), binary and 'rb' or 'r') as fd:
> FileNotFoundError: [Errno 2] No such file or directory: 'u-boot.bin'
>
> >
> > >
> > > 3. GetData
> > >
> > > for entry_path in entry_paths:
> > > entry = image.FindEntryPath(entry_path)
> > >
> > > print("--- DATA ---")
> > > data = entry.GetData(True)
> > > print(data)
> > > print("~~~ DATA ~~~")
> > >
> > > --- DATA ---
> > > Node '/fit/images/u-boot-1/u-boot': GetData: size 0x4
> > > Node '/fit/images/u-boot-1': GetPaddedDataForEntry: size None
> > > Node '/fit/images/u-boot-1': GetData: 1 entries, total size 0x4
> > > Node '/fit/images/fdt-1/u-boot-spl-dtb': GetData: size 0x4f7
> > > Node '/fit/images/fdt-1': GetPaddedDataForEntry: size None
> > > Node '/fit/images/fdt-1': GetData: 1 entries, total size
> > > 0x4f7
> > > Deleted temporary directory '/tmp/binman.z81eqcfz'
> > > binman: 'NoneType' object has no attribute 'run'
> >
> > This might be trying to call tools.run() so use -D to see where the
> > error is.
> >
>
> --- DATA ---
> Node '/fit/images/u-boot-1/u-boot': GetData: size 0x4
> Node '/fit/images/u-boot-1': GetPaddedDataForEntry: size None
> Node '/fit/images/u-boot-1': GetData: 1 entries, total size 0x4
> Node '/fit/images/fdt-1/u-boot-spl-dtb': GetData: size 0x4f7
> Node '/fit/images/fdt-1': GetPaddedDataForEntry: size None
> Node '/fit/images/fdt-1': GetData: 1 entries, total size 0x4f7
> Deleted temporary directory '/tmp/binman.0x74lr_s'
> binman: 'NoneType' object has no attribute 'run'
>
> Traceback (most recent call last):
> File "/home/fr/upstream_uboot/tools/binman/binman", line 133, in
> RunBinman
> ret_code = control.Binman(args)
> File "/home/fr/upstream_uboot/tools/binman/control.py", line 684, in
> Binman
> SignEntries(args.image, args.file, args.key, args.algo, args.paths)
> File "/home/fr/upstream_uboot/tools/binman/control.py", line 468, in
> SignEntries
> print(entry.GetData())
> File "/home/fr/upstream_uboot/tools/binman/etype/section.py", line
> 362, in GetData
> data = self.BuildSectionData(required)
> File "/home/fr/upstream_uboot/tools/binman/etype/fit.py", line 426,
> in BuildSectionData
> if self.mkimage.run(reset_timestamp=True,
> output_fname=output_fname,
> AttributeError: 'NoneType' object has no attribute 'run'
>
> This one strange to me because mkimage exists in tools directory and
> has the symbolic link to /usr/local.
>
> > >
> > > There is no problem with getting data from GetData around start of
> > > the
> > > year. Maybe some regression?
> > >
> > > All this ran with this:
> > > binman -v5 sign -i image.bin -k test_key.key -a sha256,rsa4096 fit
> > >
> > > `fit` in entry_paths and image contains FIT section with name
> > > `fit`.
> > >
> > > binman ls -i image.bin
> > > Name Image-pos Size Entry-type Offset
> > > Uncomp-size
> > > -------------------------------------------------------------------
> > > ----
> > > --------
> > > main-section 0 100000 section 0
> > > fit 10000 c0a fit 10000
> > > u-boot-1 10104 4 section 104
> > > u-boot 10104 4 u-boot 0
> > > fdt-1 101c8 4f7 section 1c8
> > > u-boot-spl-dtb 101c8 4f7 u-boot-spl-dtb 0
> > > fdtmap 10c0a 4f5 fdtmap 10c0a
> > >
> > >
> > > Seems something went wrong, any ideas? Or did I misuse?
> >
> > Can you please push a tree somewhere so I can try this?
>
> Sure, https://github.com/fr0st61te/u-boot/tree/signfit , rebased to
> current master.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-12-17 22:02 ` Simon Glass
@ 2022-12-25 1:35 ` Ivan Mikhaylov
2023-01-13 18:00 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Ivan Mikhaylov @ 2022-12-25 1:35 UTC (permalink / raw)
To: Simon Glass; +Cc: Alper Nebi Yasak, U-Boot Mailing List, Jan Kiszka
On Sat, 2022-12-17 at 15:02 -0700, Simon Glass wrote:
> Hi Ivan,
>
> On Tue, 13 Dec 2022 at 11:51, Ivan Mikhaylov <fr0st61te@gmail.com>
> wrote:
> >
> > On Fri, 2022-11-18 at 13:50 -0700, Simon Glass wrote:
> > > Hi Ivan,
> > >
> > > On Thu, 15 Sept 2022 at 13:44, Ivan Mikhaylov
> > > <fr0st61te@gmail.com>
> > > wrote:
> > > >
> > > > On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> > > > > Hi Ivan,
> > > > >
> > > > > Section data comes from the BuildSectionData() method, so you
> > > > > could
> > > > > try calling that.
> > > > >
> > > > > See also collect_contents_to_file()
> > > > >
> > > > > Regards,
> > > > > Simon
> > > >
> > > > Simon, I've tried both these ways and they both don't work to
> > > > me.
> > > > What
> > > > I've got:
> > > >
> > > > def SignEntries(image_fname, input_fname, privatekey_fname,
> > > > algo,
> > > > entry_paths):
> > > > image_fname = os.path.abspath(image_fname)
> > > > image = Image.FromFile(image_fname)
> > > > state.PrepareFromLoadedData(image)
> > > > image.LoadData()
> > > >
> > > > 1. BuildSectionData
> > > >
> > > > for entry_path in entry_paths:
> > > > entry = image.FindEntryPath(entry_path)
> > > >
> > > > try:
> > > > entry.BuildSectionData(True)
> > > > except Exception as e:
> > > > logging.error(traceback.format_exc())
> > > >
> > > >
> > > > ERROR:root:AttributeError: 'NoneType' object has no attribute
> > > > 'run'
> >
> > Hi Simon, sorry for long delay.
> >
> > binman: 'NoneType' object has no attribute 'run'
> >
> > Traceback (most recent call last):
> > File "/home/fr/upstream_uboot/tools/binman/binman", line 133, in
> > RunBinman
> > ret_code = control.Binman(args)
> > File "/home/fr/upstream_uboot/tools/binman/control.py", line 684,
> > in
> > Binman
> > SignEntries(args.image, args.file, args.key, args.algo,
> > args.paths)
> > File "/home/fr/upstream_uboot/tools/binman/control.py", line 469,
> > in
> > SignEntries
> > entry.BuildSectionData(True)
> > File "/home/fr/upstream_uboot/tools/binman/etype/fit.py", line
> > 426,
> > in BuildSectionData
> > if self.mkimage.run(reset_timestamp=True,
> > output_fname=output_fname,
> > AttributeError: 'NoneType' object has no attribute 'run'
> >
>
> You need to call image.CollectBintolls() like ReadEntry() and other
> functions similar to yours that read images from a file. This is the
> only way that the 'mkimage' tool becomes available to fit.py
>
> See fit.AddBintools() which is called by that function and sets
> 'self.mkimage'
> >
Simon, thanks, now this part works fine but there is still issue with
updating of fit section, saw that there exists some functions like
WriteData but for section(etype/fit.py) it is not implemented yet.
ValueError: Node '/fit': Replacing sections is not implemented yet
Also tried SetContents but it doesn't update fit section in place. Any
suggestions here?
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2022-12-25 1:35 ` Ivan Mikhaylov
@ 2023-01-13 18:00 ` Simon Glass
2023-01-16 2:54 ` Ivan Mikhaylov
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2023-01-13 18:00 UTC (permalink / raw)
To: Ivan Mikhaylov; +Cc: Alper Nebi Yasak, U-Boot Mailing List, Jan Kiszka
Hi Ivan,
On Sat, 24 Dec 2022 at 15:35, Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>
> On Sat, 2022-12-17 at 15:02 -0700, Simon Glass wrote:
> > Hi Ivan,
> >
> > On Tue, 13 Dec 2022 at 11:51, Ivan Mikhaylov <fr0st61te@gmail.com>
> > wrote:
> > >
> > > On Fri, 2022-11-18 at 13:50 -0700, Simon Glass wrote:
> > > > Hi Ivan,
> > > >
> > > > On Thu, 15 Sept 2022 at 13:44, Ivan Mikhaylov
> > > > <fr0st61te@gmail.com>
> > > > wrote:
> > > > >
> > > > > On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> > > > > > Hi Ivan,
> > > > > >
> > > > > > Section data comes from the BuildSectionData() method, so you
> > > > > > could
> > > > > > try calling that.
> > > > > >
> > > > > > See also collect_contents_to_file()
> > > > > >
> > > > > > Regards,
> > > > > > Simon
> > > > >
> > > > > Simon, I've tried both these ways and they both don't work to
> > > > > me.
> > > > > What
> > > > > I've got:
> > > > >
> > > > > def SignEntries(image_fname, input_fname, privatekey_fname,
> > > > > algo,
> > > > > entry_paths):
> > > > > image_fname = os.path.abspath(image_fname)
> > > > > image = Image.FromFile(image_fname)
> > > > > state.PrepareFromLoadedData(image)
> > > > > image.LoadData()
> > > > >
> > > > > 1. BuildSectionData
> > > > >
> > > > > for entry_path in entry_paths:
> > > > > entry = image.FindEntryPath(entry_path)
> > > > >
> > > > > try:
> > > > > entry.BuildSectionData(True)
> > > > > except Exception as e:
> > > > > logging.error(traceback.format_exc())
> > > > >
> > > > >
> > > > > ERROR:root:AttributeError: 'NoneType' object has no attribute
> > > > > 'run'
> > >
> > > Hi Simon, sorry for long delay.
> > >
> > > binman: 'NoneType' object has no attribute 'run'
> > >
> > > Traceback (most recent call last):
> > > File "/home/fr/upstream_uboot/tools/binman/binman", line 133, in
> > > RunBinman
> > > ret_code = control.Binman(args)
> > > File "/home/fr/upstream_uboot/tools/binman/control.py", line 684,
> > > in
> > > Binman
> > > SignEntries(args.image, args.file, args.key, args.algo,
> > > args.paths)
> > > File "/home/fr/upstream_uboot/tools/binman/control.py", line 469,
> > > in
> > > SignEntries
> > > entry.BuildSectionData(True)
> > > File "/home/fr/upstream_uboot/tools/binman/etype/fit.py", line
> > > 426,
> > > in BuildSectionData
> > > if self.mkimage.run(reset_timestamp=True,
> > > output_fname=output_fname,
> > > AttributeError: 'NoneType' object has no attribute 'run'
> > >
> >
> > You need to call image.CollectBintolls() like ReadEntry() and other
> > functions similar to yours that read images from a file. This is the
> > only way that the 'mkimage' tool becomes available to fit.py
> >
> > See fit.AddBintools() which is called by that function and sets
> > 'self.mkimage'
> > >
> Simon, thanks, now this part works fine but there is still issue with
> updating of fit section, saw that there exists some functions like
> WriteData but for section(etype/fit.py) it is not implemented yet.
>
> ValueError: Node '/fit': Replacing sections is not implemented yet
>
> Also tried SetContents but it doesn't update fit section in place. Any
> suggestions here?
Updating a FIT in the image is not supported, or at least not tested,
so presumably doesn't work.
I obtained fdt_add_pubkey
fromhttps://patchwork.ozlabs.org/project/uboot/list/?series=271511&state=*
I tried this:
binman test testSignSimple
======================== Running binman tests ========================
E
======================================================================
ERROR: binman.ftest.TestFunctional.testSignSimple (subunit.RemotedTestCase)
binman.ftest.TestFunctional.testSignSimple
----------------------------------------------------------------------
testtools.testresult.real._StringException: ValueError: Error 1
running 'fdt_add_pubkey -a sha256,rsa4096 -k /tmp/binman.1antmyoq -n
test_key /tmp/binman.1antmyoq/source.dtb': .dtb too small, increasing
size by 1024 bytes
.dtb too small, increasing size by 1024 bytes
fdt_add_pubkey: Cannot add public key to FIT blob: Unknown error -56
During handling of the above exception, another exception occurred:
UnboundLocalError: local variable 'key_dir' referenced before assignment
----------------------------------------------------------------------
Ran 1 test in 1.658s
FAILED (errors=1)
[sjg@kea u ((5cf6f1f8e7c...) $)]$ binman test testSignSimpleExact
======================== Running binman tests ========================
----------------------------------------------------------------------
Ran 0 tests in 0.067s
OK
Can you please:
- push your tree again
- provide the command line you are using, or test case you are trying
to make work
- provide the files needed to run it it
With that I should be able to figure out what is needed.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2023-01-13 18:00 ` Simon Glass
@ 2023-01-16 2:54 ` Ivan Mikhaylov
2023-02-04 22:23 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Ivan Mikhaylov @ 2023-01-16 2:54 UTC (permalink / raw)
To: Simon Glass; +Cc: Alper Nebi Yasak, U-Boot Mailing List, Jan Kiszka
On Fri, 2023-01-13 at 11:00 -0700, Simon Glass wrote:
> Hi Ivan,
>
> On Sat, 24 Dec 2022 at 15:35, Ivan Mikhaylov <fr0st61te@gmail.com>
> wrote:
> >
> > On Sat, 2022-12-17 at 15:02 -0700, Simon Glass wrote:
> > > Hi Ivan,
> > >
> > > On Tue, 13 Dec 2022 at 11:51, Ivan Mikhaylov
> > > <fr0st61te@gmail.com>
> > > wrote:
> > > >
> > > > On Fri, 2022-11-18 at 13:50 -0700, Simon Glass wrote:
> > > > > Hi Ivan,
> > > > >
> > > > > On Thu, 15 Sept 2022 at 13:44, Ivan Mikhaylov
> > > > > <fr0st61te@gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> > > > > > > Hi Ivan,
> > > > > > >
> > > > > > > Section data comes from the BuildSectionData() method, so
> > > > > > > you
> > > > > > > could
> > > > > > > try calling that.
> > > > > > >
> > > > > > > See also collect_contents_to_file()
> > > > > > >
> > > > > > > Regards,
> > > > > > > Simon
> > > > > >
> > > > > > Simon, I've tried both these ways and they both don't work
> > > > > > to
> > > > > > me.
> > > > > > What
> > > > > > I've got:
> > > > > >
> > > > > > def SignEntries(image_fname, input_fname, privatekey_fname,
> > > > > > algo,
> > > > > > entry_paths):
> > > > > > image_fname = os.path.abspath(image_fname)
> > > > > > image = Image.FromFile(image_fname)
> > > > > > state.PrepareFromLoadedData(image)
> > > > > > image.LoadData()
> > > > > >
> > > > > > 1. BuildSectionData
> > > > > >
> > > > > > for entry_path in entry_paths:
> > > > > > entry = image.FindEntryPath(entry_path)
> > > > > >
> > > > > > try:
> > > > > > entry.BuildSectionData(True)
> > > > > > except Exception as e:
> > > > > > logging.error(traceback.format_exc())
> > > > > >
> > > > > >
> > > > > > ERROR:root:AttributeError: 'NoneType' object has no
> > > > > > attribute
> > > > > > 'run'
> > > >
> > > > Hi Simon, sorry for long delay.
> > > >
> > > > binman: 'NoneType' object has no attribute 'run'
> > > >
> > > > Traceback (most recent call last):
> > > > File "/home/fr/upstream_uboot/tools/binman/binman", line 133,
> > > > in
> > > > RunBinman
> > > > ret_code = control.Binman(args)
> > > > File "/home/fr/upstream_uboot/tools/binman/control.py", line
> > > > 684,
> > > > in
> > > > Binman
> > > > SignEntries(args.image, args.file, args.key, args.algo,
> > > > args.paths)
> > > > File "/home/fr/upstream_uboot/tools/binman/control.py", line
> > > > 469,
> > > > in
> > > > SignEntries
> > > > entry.BuildSectionData(True)
> > > > File "/home/fr/upstream_uboot/tools/binman/etype/fit.py",
> > > > line
> > > > 426,
> > > > in BuildSectionData
> > > > if self.mkimage.run(reset_timestamp=True,
> > > > output_fname=output_fname,
> > > > AttributeError: 'NoneType' object has no attribute 'run'
> > > >
> > >
> > > You need to call image.CollectBintolls() like ReadEntry() and
> > > other
> > > functions similar to yours that read images from a file. This is
> > > the
> > > only way that the 'mkimage' tool becomes available to fit.py
> > >
> > > See fit.AddBintools() which is called by that function and sets
> > > 'self.mkimage'
> > > >
> > Simon, thanks, now this part works fine but there is still issue
> > with
> > updating of fit section, saw that there exists some functions like
> > WriteData but for section(etype/fit.py) it is not implemented yet.
> >
> > ValueError: Node '/fit': Replacing sections is not implemented yet
> >
> > Also tried SetContents but it doesn't update fit section in place.
> > Any
> > suggestions here?
>
> Updating a FIT in the image is not supported, or at least not tested,
> so presumably doesn't work.
>
> I obtained fdt_add_pubkey
> from
> https://patchwork.ozlabs.org/project/uboot/list/?series=271511&state=
> *
>
> I tried this:
>
> binman test testSignSimple
> ======================== Running binman tests
> ========================
> E
> =====================================================================
> =
> ERROR: binman.ftest.TestFunctional.testSignSimple
> (subunit.RemotedTestCase)
> binman.ftest.TestFunctional.testSignSimple
> ---------------------------------------------------------------------
> -
> testtools.testresult.real._StringException: ValueError: Error 1
> running 'fdt_add_pubkey -a sha256,rsa4096 -k /tmp/binman.1antmyoq -n
> test_key /tmp/binman.1antmyoq/source.dtb': .dtb too small, increasing
> size by 1024 bytes
> .dtb too small, increasing size by 1024 bytes
> fdt_add_pubkey: Cannot add public key to FIT blob: Unknown error -56
>
>
> During handling of the above exception, another exception occurred:
>
> UnboundLocalError: local variable 'key_dir' referenced before
> assignment
>
>
> ---------------------------------------------------------------------
> -
> Ran 1 test in 1.658s
>
> FAILED (errors=1)
>
> [sjg@kea u ((5cf6f1f8e7c...) $)]$ binman test testSignSimpleExact
> ======================== Running binman tests
> ========================
>
> ---------------------------------------------------------------------
> -
> Ran 0 tests in 0.067s
>
> OK
>
>
> Can you please:
>
> - push your tree again
> - provide the command line you are using, or test case you are trying
> to make work
> - provide the files needed to run it it
>
> With that I should be able to figure out what is needed.
>
> Regards,
> Simon
Simon, sorry, I forgot about fdt_add_pubkey, I've updated and added
version on which I'm working into branch which I posted before. There
was update in add_verify_data call for rsa at least which sending node
number instead of return code because of this you seeing such errors
with run of this toolkit. Now you should see something like this:
binman test testSignSimple
======================== Running binman tests ========================
E
======================================================================
ERROR: testSignSimple (binman.ftest.TestFunctional)
Test that a FIT container can be signed in image
----------------------------------------------------------------------
ValueError: Node '/fit': Replacing sections is not implemented yet
----------------------------------------------------------------------
Ran 1 test in 0.480s
FAILED (errors=1)
The command line which I'm using for manual testing:
binman -D sign -i image-updated.bin -k test_key.key -a sha256,rsa4096
fit
Also, as I see fdt_add_pubkey application still not in the u-boot tree.
Need I look through and put it in this series or create another series
of patches for fdt_add_pubkey?
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2023-01-16 2:54 ` Ivan Mikhaylov
@ 2023-02-04 22:23 ` Simon Glass
2023-02-14 23:37 ` Ivan Mikhaylov
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2023-02-04 22:23 UTC (permalink / raw)
To: Ivan Mikhaylov; +Cc: Alper Nebi Yasak, U-Boot Mailing List, Jan Kiszka
Hi Ivan,
On Sun, 15 Jan 2023 at 16:54, Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>
> On Fri, 2023-01-13 at 11:00 -0700, Simon Glass wrote:
> > Hi Ivan,
> >
> > On Sat, 24 Dec 2022 at 15:35, Ivan Mikhaylov <fr0st61te@gmail.com>
> > wrote:
> > >
> > > On Sat, 2022-12-17 at 15:02 -0700, Simon Glass wrote:
> > > > Hi Ivan,
> > > >
> > > > On Tue, 13 Dec 2022 at 11:51, Ivan Mikhaylov
> > > > <fr0st61te@gmail.com>
> > > > wrote:
> > > > >
> > > > > On Fri, 2022-11-18 at 13:50 -0700, Simon Glass wrote:
> > > > > > Hi Ivan,
> > > > > >
> > > > > > On Thu, 15 Sept 2022 at 13:44, Ivan Mikhaylov
> > > > > > <fr0st61te@gmail.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> > > > > > > > Hi Ivan,
> > > > > > > >
> > > > > > > > Section data comes from the BuildSectionData() method, so
> > > > > > > > you
> > > > > > > > could
> > > > > > > > try calling that.
> > > > > > > >
> > > > > > > > See also collect_contents_to_file()
> > > > > > > >
> > > > > > > > Regards,
> > > > > > > > Simon
> > > > > > >
> > > > > > > Simon, I've tried both these ways and they both don't work
> > > > > > > to
> > > > > > > me.
> > > > > > > What
> > > > > > > I've got:
> > > > > > >
> > > > > > > def SignEntries(image_fname, input_fname, privatekey_fname,
> > > > > > > algo,
> > > > > > > entry_paths):
> > > > > > > image_fname = os.path.abspath(image_fname)
> > > > > > > image = Image.FromFile(image_fname)
> > > > > > > state.PrepareFromLoadedData(image)
> > > > > > > image.LoadData()
> > > > > > >
> > > > > > > 1. BuildSectionData
> > > > > > >
> > > > > > > for entry_path in entry_paths:
> > > > > > > entry = image.FindEntryPath(entry_path)
> > > > > > >
> > > > > > > try:
> > > > > > > entry.BuildSectionData(True)
> > > > > > > except Exception as e:
> > > > > > > logging.error(traceback.format_exc())
> > > > > > >
> > > > > > >
> > > > > > > ERROR:root:AttributeError: 'NoneType' object has no
> > > > > > > attribute
> > > > > > > 'run'
> > > > >
> > > > > Hi Simon, sorry for long delay.
> > > > >
> > > > > binman: 'NoneType' object has no attribute 'run'
> > > > >
> > > > > Traceback (most recent call last):
> > > > > File "/home/fr/upstream_uboot/tools/binman/binman", line 133,
> > > > > in
> > > > > RunBinman
> > > > > ret_code = control.Binman(args)
> > > > > File "/home/fr/upstream_uboot/tools/binman/control.py", line
> > > > > 684,
> > > > > in
> > > > > Binman
> > > > > SignEntries(args.image, args.file, args.key, args.algo,
> > > > > args.paths)
> > > > > File "/home/fr/upstream_uboot/tools/binman/control.py", line
> > > > > 469,
> > > > > in
> > > > > SignEntries
> > > > > entry.BuildSectionData(True)
> > > > > File "/home/fr/upstream_uboot/tools/binman/etype/fit.py",
> > > > > line
> > > > > 426,
> > > > > in BuildSectionData
> > > > > if self.mkimage.run(reset_timestamp=True,
> > > > > output_fname=output_fname,
> > > > > AttributeError: 'NoneType' object has no attribute 'run'
> > > > >
> > > >
> > > > You need to call image.CollectBintolls() like ReadEntry() and
> > > > other
> > > > functions similar to yours that read images from a file. This is
> > > > the
> > > > only way that the 'mkimage' tool becomes available to fit.py
> > > >
> > > > See fit.AddBintools() which is called by that function and sets
> > > > 'self.mkimage'
> > > > >
> > > Simon, thanks, now this part works fine but there is still issue
> > > with
> > > updating of fit section, saw that there exists some functions like
> > > WriteData but for section(etype/fit.py) it is not implemented yet.
> > >
> > > ValueError: Node '/fit': Replacing sections is not implemented yet
> > >
> > > Also tried SetContents but it doesn't update fit section in place.
> > > Any
> > > suggestions here?
> >
> > Updating a FIT in the image is not supported, or at least not tested,
> > so presumably doesn't work.
> >
> > I obtained fdt_add_pubkey
> > from
> > https://patchwork.ozlabs.org/project/uboot/list/?series=271511&state=
> > *
> >
> > I tried this:
> >
> > binman test testSignSimple
> > ======================== Running binman tests
> > ========================
> > E
> > =====================================================================
> > =
> > ERROR: binman.ftest.TestFunctional.testSignSimple
> > (subunit.RemotedTestCase)
> > binman.ftest.TestFunctional.testSignSimple
> > ---------------------------------------------------------------------
> > -
> > testtools.testresult.real._StringException: ValueError: Error 1
> > running 'fdt_add_pubkey -a sha256,rsa4096 -k /tmp/binman.1antmyoq -n
> > test_key /tmp/binman.1antmyoq/source.dtb': .dtb too small, increasing
> > size by 1024 bytes
> > .dtb too small, increasing size by 1024 bytes
> > fdt_add_pubkey: Cannot add public key to FIT blob: Unknown error -56
> >
> >
> > During handling of the above exception, another exception occurred:
> >
> > UnboundLocalError: local variable 'key_dir' referenced before
> > assignment
> >
> >
> > ---------------------------------------------------------------------
> > -
> > Ran 1 test in 1.658s
> >
> > FAILED (errors=1)
> >
> > [sjg@kea u ((5cf6f1f8e7c...) $)]$ binman test testSignSimpleExact
> > ======================== Running binman tests
> > ========================
> >
> > ---------------------------------------------------------------------
> > -
> > Ran 0 tests in 0.067s
> >
> > OK
> >
> >
> > Can you please:
> >
> > - push your tree again
> > - provide the command line you are using, or test case you are trying
> > to make work
> > - provide the files needed to run it it
> >
> > With that I should be able to figure out what is needed.
> >
> > Regards,
> > Simon
>
> Simon, sorry, I forgot about fdt_add_pubkey, I've updated and added
> version on which I'm working into branch which I posted before. There
> was update in add_verify_data call for rsa at least which sending node
> number instead of return code because of this you seeing such errors
> with run of this toolkit. Now you should see something like this:
>
> binman test testSignSimple
> ======================== Running binman tests ========================
> E
> ======================================================================
> ERROR: testSignSimple (binman.ftest.TestFunctional)
> Test that a FIT container can be signed in image
> ----------------------------------------------------------------------
> ValueError: Node '/fit': Replacing sections is not implemented yet
>
> ----------------------------------------------------------------------
> Ran 1 test in 0.480s
>
> FAILED (errors=1)
>
> The command line which I'm using for manual testing:
>
> binman -D sign -i image-updated.bin -k test_key.key -a sha256,rsa4096
> fit
I've had a crack at this and sent a patch to allow updating sections in toto.
https://github.com/sjg20/u-boot/tree/try-ivan
>
> Also, as I see fdt_add_pubkey application still not in the u-boot tree.
> Need I look through and put it in this series or create another series
> of patches for fdt_add_pubkey?
Doing it in this series is fine.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2023-02-04 22:23 ` Simon Glass
@ 2023-02-14 23:37 ` Ivan Mikhaylov
2023-02-17 23:49 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Ivan Mikhaylov @ 2023-02-14 23:37 UTC (permalink / raw)
To: Simon Glass; +Cc: Alper Nebi Yasak, U-Boot Mailing List, Jan Kiszka
On Sat, 2023-02-04 at 15:23 -0700, Simon Glass wrote:
> Hi Ivan,
>
> On Sun, 15 Jan 2023 at 16:54, Ivan Mikhaylov <fr0st61te@gmail.com>
> wrote:
> >
> > On Fri, 2023-01-13 at 11:00 -0700, Simon Glass wrote:
> > > Hi Ivan,
> > >
> > > On Sat, 24 Dec 2022 at 15:35, Ivan Mikhaylov
> > > <fr0st61te@gmail.com>
> > > wrote:
> > > >
> > > > On Sat, 2022-12-17 at 15:02 -0700, Simon Glass wrote:
> > > > > Hi Ivan,
> > > > >
> > > > > On Tue, 13 Dec 2022 at 11:51, Ivan Mikhaylov
> > > > > <fr0st61te@gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > On Fri, 2022-11-18 at 13:50 -0700, Simon Glass wrote:
> > > > > > > Hi Ivan,
> > > > > > >
> > > > > > > On Thu, 15 Sept 2022 at 13:44, Ivan Mikhaylov
> > > > > > > <fr0st61te@gmail.com>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> > > > > > > > > Hi Ivan,
> > > > > > > > >
> > > > > > > > > Section data comes from the BuildSectionData()
> > > > > > > > > method, so
> > > > > > > > > you
> > > > > > > > > could
> > > > > > > > > try calling that.
> > > > > > > > >
> > > > > > > > > See also collect_contents_to_file()
> > > > > > > > >
> > > > > > > > > Regards,
> > > > > > > > > Simon
> > > > > > > >
> > > > > > > > Simon, I've tried both these ways and they both don't
> > > > > > > > work
> > > > > > > > to
> > > > > > > > me.
> > > > > > > > What
> > > > > > > > I've got:
> > > > > > > >
> > > > > > > > def SignEntries(image_fname, input_fname,
> > > > > > > > privatekey_fname,
> > > > > > > > algo,
> > > > > > > > entry_paths):
> > > > > > > > image_fname = os.path.abspath(image_fname)
> > > > > > > > image = Image.FromFile(image_fname)
> > > > > > > > state.PrepareFromLoadedData(image)
> > > > > > > > image.LoadData()
> > > > > > > >
> > > > > > > > 1. BuildSectionData
> > > > > > > >
> > > > > > > > for entry_path in entry_paths:
> > > > > > > > entry = image.FindEntryPath(entry_path)
> > > > > > > >
> > > > > > > > try:
> > > > > > > > entry.BuildSectionData(True)
> > > > > > > > except Exception as e:
> > > > > > > > logging.error(traceback.format_exc())
> > > > > > > >
> > > > > > > >
> > > > > > > > ERROR:root:AttributeError: 'NoneType' object has no
> > > > > > > > attribute
> > > > > > > > 'run'
> > > > > >
> > > > > > Hi Simon, sorry for long delay.
> > > > > >
> > > > > > binman: 'NoneType' object has no attribute 'run'
> > > > > >
> > > > > > Traceback (most recent call last):
> > > > > > File "/home/fr/upstream_uboot/tools/binman/binman", line
> > > > > > 133,
> > > > > > in
> > > > > > RunBinman
> > > > > > ret_code = control.Binman(args)
> > > > > > File "/home/fr/upstream_uboot/tools/binman/control.py",
> > > > > > line
> > > > > > 684,
> > > > > > in
> > > > > > Binman
> > > > > > SignEntries(args.image, args.file, args.key, args.algo,
> > > > > > args.paths)
> > > > > > File "/home/fr/upstream_uboot/tools/binman/control.py",
> > > > > > line
> > > > > > 469,
> > > > > > in
> > > > > > SignEntries
> > > > > > entry.BuildSectionData(True)
> > > > > > File "/home/fr/upstream_uboot/tools/binman/etype/fit.py",
> > > > > > line
> > > > > > 426,
> > > > > > in BuildSectionData
> > > > > > if self.mkimage.run(reset_timestamp=True,
> > > > > > output_fname=output_fname,
> > > > > > AttributeError: 'NoneType' object has no attribute 'run'
> > > > > >
> > > > >
> > > > > You need to call image.CollectBintolls() like ReadEntry() and
> > > > > other
> > > > > functions similar to yours that read images from a file. This
> > > > > is
> > > > > the
> > > > > only way that the 'mkimage' tool becomes available to fit.py
> > > > >
> > > > > See fit.AddBintools() which is called by that function and
> > > > > sets
> > > > > 'self.mkimage'
> > > > > >
> > > > Simon, thanks, now this part works fine but there is still
> > > > issue
> > > > with
> > > > updating of fit section, saw that there exists some functions
> > > > like
> > > > WriteData but for section(etype/fit.py) it is not implemented
> > > > yet.
> > > >
> > > > ValueError: Node '/fit': Replacing sections is not implemented
> > > > yet
> > > >
> > > > Also tried SetContents but it doesn't update fit section in
> > > > place.
> > > > Any
> > > > suggestions here?
> > >
> > > Updating a FIT in the image is not supported, or at least not
> > > tested,
> > > so presumably doesn't work.
> > >
> > > I obtained fdt_add_pubkey
> > > from
> > > https://patchwork.ozlabs.org/project/uboot/list/?series=271511&state=
> > > *
> > >
> > > I tried this:
> > >
> > > binman test testSignSimple
> > > ======================== Running binman tests
> > > ========================
> > > E
> > > =================================================================
> > > ====
> > > =
> > > ERROR: binman.ftest.TestFunctional.testSignSimple
> > > (subunit.RemotedTestCase)
> > > binman.ftest.TestFunctional.testSignSimple
> > > -----------------------------------------------------------------
> > > ----
> > > -
> > > testtools.testresult.real._StringException: ValueError: Error 1
> > > running 'fdt_add_pubkey -a sha256,rsa4096 -k /tmp/binman.1antmyoq
> > > -n
> > > test_key /tmp/binman.1antmyoq/source.dtb': .dtb too small,
> > > increasing
> > > size by 1024 bytes
> > > .dtb too small, increasing size by 1024 bytes
> > > fdt_add_pubkey: Cannot add public key to FIT blob: Unknown error
> > > -56
> > >
> > >
> > > During handling of the above exception, another exception
> > > occurred:
> > >
> > > UnboundLocalError: local variable 'key_dir' referenced before
> > > assignment
> > >
> > >
> > > -----------------------------------------------------------------
> > > ----
> > > -
> > > Ran 1 test in 1.658s
> > >
> > > FAILED (errors=1)
> > >
> > > [sjg@kea u ((5cf6f1f8e7c...) $)]$ binman test testSignSimpleExact
> > > ======================== Running binman tests
> > > ========================
> > >
> > > -----------------------------------------------------------------
> > > ----
> > > -
> > > Ran 0 tests in 0.067s
> > >
> > > OK
> > >
> > >
> > > Can you please:
> > >
> > > - push your tree again
> > > - provide the command line you are using, or test case you are
> > > trying
> > > to make work
> > > - provide the files needed to run it it
> > >
> > > With that I should be able to figure out what is needed.
> > >
> > > Regards,
> > > Simon
> >
> > Simon, sorry, I forgot about fdt_add_pubkey, I've updated and added
> > version on which I'm working into branch which I posted before.
> > There
> > was update in add_verify_data call for rsa at least which sending
> > node
> > number instead of return code because of this you seeing such
> > errors
> > with run of this toolkit. Now you should see something like this:
> >
> > binman test testSignSimple
> > ======================== Running binman tests
> > ========================
> > E
> > ===================================================================
> > ===
> > ERROR: testSignSimple (binman.ftest.TestFunctional)
> > Test that a FIT container can be signed in image
> > -------------------------------------------------------------------
> > ---
> > ValueError: Node '/fit': Replacing sections is not implemented yet
> >
> > -------------------------------------------------------------------
> > ---
> > Ran 1 test in 0.480s
> >
> > FAILED (errors=1)
> >
> > The command line which I'm using for manual testing:
> >
> > binman -D sign -i image-updated.bin -k test_key.key -a
> > sha256,rsa4096
> > fit
>
> I've had a crack at this and sent a patch to allow updating sections
> in toto.
>
> https://github.com/sjg20/u-boot/tree/try-ivan
>
> >
> > Also, as I see fdt_add_pubkey application still not in the u-boot
> > tree.
> > Need I look through and put it in this series or create another
> > series
> > of patches for fdt_add_pubkey?
>
> Doing it in this series is fine.
>
> Regards,
> Simon
Simon, thanks a lot, now it's looks like working. I've updated my
branch on https://github.com/fr0st61te/u-boot/commits/signfit,
everything seems ok - fdt_add_pubkey and tests works fine. I want to
check everything with qemu or hw, it'll take some time. I'll get back
with proper patchsets in 2-3 weeks.
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/3] binman: add sign option for binman
2023-02-14 23:37 ` Ivan Mikhaylov
@ 2023-02-17 23:49 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2023-02-17 23:49 UTC (permalink / raw)
To: Ivan Mikhaylov; +Cc: Alper Nebi Yasak, U-Boot Mailing List, Jan Kiszka
HI Ivan,
On Tue, 14 Feb 2023 at 13:38, Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>
> On Sat, 2023-02-04 at 15:23 -0700, Simon Glass wrote:
> > Hi Ivan,
> >
> > On Sun, 15 Jan 2023 at 16:54, Ivan Mikhaylov <fr0st61te@gmail.com>
> > wrote:
> > >
> > > On Fri, 2023-01-13 at 11:00 -0700, Simon Glass wrote:
> > > > Hi Ivan,
> > > >
> > > > On Sat, 24 Dec 2022 at 15:35, Ivan Mikhaylov
> > > > <fr0st61te@gmail.com>
> > > > wrote:
> > > > >
> > > > > On Sat, 2022-12-17 at 15:02 -0700, Simon Glass wrote:
> > > > > > Hi Ivan,
> > > > > >
> > > > > > On Tue, 13 Dec 2022 at 11:51, Ivan Mikhaylov
> > > > > > <fr0st61te@gmail.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > On Fri, 2022-11-18 at 13:50 -0700, Simon Glass wrote:
> > > > > > > > Hi Ivan,
> > > > > > > >
> > > > > > > > On Thu, 15 Sept 2022 at 13:44, Ivan Mikhaylov
> > > > > > > > <fr0st61te@gmail.com>
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Wed, 2022-09-07 at 15:10 -0600, Simon Glass wrote:
> > > > > > > > > > Hi Ivan,
> > > > > > > > > >
> > > > > > > > > > Section data comes from the BuildSectionData()
> > > > > > > > > > method, so
> > > > > > > > > > you
> > > > > > > > > > could
> > > > > > > > > > try calling that.
> > > > > > > > > >
> > > > > > > > > > See also collect_contents_to_file()
> > > > > > > > > >
> > > > > > > > > > Regards,
> > > > > > > > > > Simon
> > > > > > > > >
> > > > > > > > > Simon, I've tried both these ways and they both don't
> > > > > > > > > work
> > > > > > > > > to
> > > > > > > > > me.
> > > > > > > > > What
> > > > > > > > > I've got:
> > > > > > > > >
> > > > > > > > > def SignEntries(image_fname, input_fname,
> > > > > > > > > privatekey_fname,
> > > > > > > > > algo,
> > > > > > > > > entry_paths):
> > > > > > > > > image_fname = os.path.abspath(image_fname)
> > > > > > > > > image = Image.FromFile(image_fname)
> > > > > > > > > state.PrepareFromLoadedData(image)
> > > > > > > > > image.LoadData()
> > > > > > > > >
> > > > > > > > > 1. BuildSectionData
> > > > > > > > >
> > > > > > > > > for entry_path in entry_paths:
> > > > > > > > > entry = image.FindEntryPath(entry_path)
> > > > > > > > >
> > > > > > > > > try:
> > > > > > > > > entry.BuildSectionData(True)
> > > > > > > > > except Exception as e:
> > > > > > > > > logging.error(traceback.format_exc())
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > ERROR:root:AttributeError: 'NoneType' object has no
> > > > > > > > > attribute
> > > > > > > > > 'run'
> > > > > > >
> > > > > > > Hi Simon, sorry for long delay.
> > > > > > >
> > > > > > > binman: 'NoneType' object has no attribute 'run'
> > > > > > >
> > > > > > > Traceback (most recent call last):
> > > > > > > File "/home/fr/upstream_uboot/tools/binman/binman", line
> > > > > > > 133,
> > > > > > > in
> > > > > > > RunBinman
> > > > > > > ret_code = control.Binman(args)
> > > > > > > File "/home/fr/upstream_uboot/tools/binman/control.py",
> > > > > > > line
> > > > > > > 684,
> > > > > > > in
> > > > > > > Binman
> > > > > > > SignEntries(args.image, args.file, args.key, args.algo,
> > > > > > > args.paths)
> > > > > > > File "/home/fr/upstream_uboot/tools/binman/control.py",
> > > > > > > line
> > > > > > > 469,
> > > > > > > in
> > > > > > > SignEntries
> > > > > > > entry.BuildSectionData(True)
> > > > > > > File "/home/fr/upstream_uboot/tools/binman/etype/fit.py",
> > > > > > > line
> > > > > > > 426,
> > > > > > > in BuildSectionData
> > > > > > > if self.mkimage.run(reset_timestamp=True,
> > > > > > > output_fname=output_fname,
> > > > > > > AttributeError: 'NoneType' object has no attribute 'run'
> > > > > > >
> > > > > >
> > > > > > You need to call image.CollectBintolls() like ReadEntry() and
> > > > > > other
> > > > > > functions similar to yours that read images from a file. This
> > > > > > is
> > > > > > the
> > > > > > only way that the 'mkimage' tool becomes available to fit.py
> > > > > >
> > > > > > See fit.AddBintools() which is called by that function and
> > > > > > sets
> > > > > > 'self.mkimage'
> > > > > > >
> > > > > Simon, thanks, now this part works fine but there is still
> > > > > issue
> > > > > with
> > > > > updating of fit section, saw that there exists some functions
> > > > > like
> > > > > WriteData but for section(etype/fit.py) it is not implemented
> > > > > yet.
> > > > >
> > > > > ValueError: Node '/fit': Replacing sections is not implemented
> > > > > yet
> > > > >
> > > > > Also tried SetContents but it doesn't update fit section in
> > > > > place.
> > > > > Any
> > > > > suggestions here?
> > > >
> > > > Updating a FIT in the image is not supported, or at least not
> > > > tested,
> > > > so presumably doesn't work.
> > > >
> > > > I obtained fdt_add_pubkey
> > > > from
> > > > https://patchwork.ozlabs.org/project/uboot/list/?series=271511&state=
> > > > *
> > > >
> > > > I tried this:
> > > >
> > > > binman test testSignSimple
> > > > ======================== Running binman tests
> > > > ========================
> > > > E
> > > > =================================================================
> > > > ====
> > > > =
> > > > ERROR: binman.ftest.TestFunctional.testSignSimple
> > > > (subunit.RemotedTestCase)
> > > > binman.ftest.TestFunctional.testSignSimple
> > > > -----------------------------------------------------------------
> > > > ----
> > > > -
> > > > testtools.testresult.real._StringException: ValueError: Error 1
> > > > running 'fdt_add_pubkey -a sha256,rsa4096 -k /tmp/binman.1antmyoq
> > > > -n
> > > > test_key /tmp/binman.1antmyoq/source.dtb': .dtb too small,
> > > > increasing
> > > > size by 1024 bytes
> > > > .dtb too small, increasing size by 1024 bytes
> > > > fdt_add_pubkey: Cannot add public key to FIT blob: Unknown error
> > > > -56
> > > >
> > > >
> > > > During handling of the above exception, another exception
> > > > occurred:
> > > >
> > > > UnboundLocalError: local variable 'key_dir' referenced before
> > > > assignment
> > > >
> > > >
> > > > -----------------------------------------------------------------
> > > > ----
> > > > -
> > > > Ran 1 test in 1.658s
> > > >
> > > > FAILED (errors=1)
> > > >
> > > > [sjg@kea u ((5cf6f1f8e7c...) $)]$ binman test testSignSimpleExact
> > > > ======================== Running binman tests
> > > > ========================
> > > >
> > > > -----------------------------------------------------------------
> > > > ----
> > > > -
> > > > Ran 0 tests in 0.067s
> > > >
> > > > OK
> > > >
> > > >
> > > > Can you please:
> > > >
> > > > - push your tree again
> > > > - provide the command line you are using, or test case you are
> > > > trying
> > > > to make work
> > > > - provide the files needed to run it it
> > > >
> > > > With that I should be able to figure out what is needed.
> > > >
> > > > Regards,
> > > > Simon
> > >
> > > Simon, sorry, I forgot about fdt_add_pubkey, I've updated and added
> > > version on which I'm working into branch which I posted before.
> > > There
> > > was update in add_verify_data call for rsa at least which sending
> > > node
> > > number instead of return code because of this you seeing such
> > > errors
> > > with run of this toolkit. Now you should see something like this:
> > >
> > > binman test testSignSimple
> > > ======================== Running binman tests
> > > ========================
> > > E
> > > ===================================================================
> > > ===
> > > ERROR: testSignSimple (binman.ftest.TestFunctional)
> > > Test that a FIT container can be signed in image
> > > -------------------------------------------------------------------
> > > ---
> > > ValueError: Node '/fit': Replacing sections is not implemented yet
> > >
> > > -------------------------------------------------------------------
> > > ---
> > > Ran 1 test in 0.480s
> > >
> > > FAILED (errors=1)
> > >
> > > The command line which I'm using for manual testing:
> > >
> > > binman -D sign -i image-updated.bin -k test_key.key -a
> > > sha256,rsa4096
> > > fit
> >
> > I've had a crack at this and sent a patch to allow updating sections
> > in toto.
> >
> > https://github.com/sjg20/u-boot/tree/try-ivan
> >
> > >
> > > Also, as I see fdt_add_pubkey application still not in the u-boot
> > > tree.
> > > Need I look through and put it in this series or create another
> > > series
> > > of patches for fdt_add_pubkey?
> >
> > Doing it in this series is fine.
> >
> > Regards,
> > Simon
>
> Simon, thanks a lot, now it's looks like working. I've updated my
> branch on https://github.com/fr0st61te/u-boot/commits/signfit,
> everything seems ok - fdt_add_pubkey and tests works fine. I want to
> check everything with qemu or hw, it'll take some time. I'll get back
> with proper patchsets in 2-3 weeks.
Good to hear, and thanks for the update.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2023-02-17 23:51 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-21 21:43 [PATCH 0/3] Introduce new sign binman's option Ivan Mikhaylov
2022-03-21 21:43 ` [PATCH 1/3] binman: add sign option for binman Ivan Mikhaylov
2022-04-05 18:54 ` Alper Nebi Yasak
2022-04-06 20:28 ` Ivan Mikhaylov
2022-04-06 22:22 ` Alper Nebi Yasak
2022-09-06 16:27 ` Ivan Mikhaylov
2022-09-07 21:10 ` Simon Glass
2022-09-15 22:44 ` Ivan Mikhaylov
2022-11-18 20:50 ` Simon Glass
2022-12-13 21:51 ` Ivan Mikhaylov
2022-12-17 22:02 ` Simon Glass
2022-12-25 1:35 ` Ivan Mikhaylov
2023-01-13 18:00 ` Simon Glass
2023-01-16 2:54 ` Ivan Mikhaylov
2023-02-04 22:23 ` Simon Glass
2023-02-14 23:37 ` Ivan Mikhaylov
2023-02-17 23:49 ` Simon Glass
2022-03-21 21:43 ` [PATCH 2/3] binman: add documentation for binman sign option Ivan Mikhaylov
2022-03-21 21:43 ` [PATCH 3/3] binman: add test for " Ivan Mikhaylov
2022-04-08 15:39 ` Sean Anderson
2022-04-08 19:26 ` Ivan Mikhaylov
2022-04-10 22:37 ` Alper Nebi Yasak
2022-04-11 15:02 ` Sean Anderson
2022-08-13 14:59 ` [PATCH 0/3] Introduce new sign binman's option Simon Glass
2022-08-15 21:51 ` Ivan Mikhaylov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox