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

[Ivan Mikhaylov]

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 at 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 at 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 at 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 at 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.