| Message ID | 20220321214319.33254-2-fr0st61te@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | Introduce new sign binman's option | expand |
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:
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.
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.
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.
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
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.
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
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.
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
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.
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
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.
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
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.
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
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:
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(-)