Hi Neha, On Mon, 13 Mar 2023 at 21:38, Neha Malcom Francis <n-fran...@ti.com> wrote: > > Hi Simon > > On 11/03/23 02:19, Simon Glass wrote: > > Hi, > > > > On Thu, 9 Mar 2023 at 21:35, Neha Malcom Francis <n-fran...@ti.com> wrote: > >> > >> Hi Andrew, Simon > >> > >> On 01/03/23 22:41, Andrew Davis wrote: > >>> On 2/28/23 9:10 AM, Simon Glass wrote: > >>>> Hi Neha, > >>>> > >>>> On Tue, 28 Feb 2023 at 02:50, Neha Malcom Francis <n-fran...@ti.com> > >>>> wrote: > >>>>> > >>>>> Hi Simon, > >>>>> > >>>>> On 28/02/23 06:05, Simon Glass wrote: > >>>>>> Hi Neha, > >>>>>> > >>>>>> On Fri, 24 Feb 2023 at 05:03, Neha Malcom Francis <n-fran...@ti.com> > >>>>>> wrote: > >>>>>>> > >>>>>>> core-secdev-k3 is the TI security development package provided for K3 > >>>>>>> platform devices. This tool helps sign bootloader images with the x509 > >>>>>>> ceritificate header. > >>>>>>> > >>>>>>> Signed-off-by: Neha Malcom Francis <n-fran...@ti.com> > >>>>>>> --- > >>>>>>> This patch depends on > >>>>>>> https://patchwork.ozlabs.org/project/uboot/patch/20230224115101.563729-1-n-fran...@ti.com/ > >>>>>>> and on ongoing development in > >>>>>>> https://git.ti.com/cgit/security-development-tools/core-secdev-k3 > >>>>>>> > >>>>>>> tools/binman/btool/tisecuretool.py | 72 +++++++++++ > >>>>>>> tools/binman/etype/ti_secure.py | 114 > >>>>>>> ++++++++++++++++++ > >>>>>>> tools/binman/ftest.py | 36 ++++++ > >>>>>>> tools/binman/test/278_ti_secure_rom.dts | 11 ++ > >>>>>>> tools/binman/test/279_ti_secure.dts | 11 ++ > >>>>>>> .../binman/test/280_ti_secure_nofilename.dts | 10 ++ > >>>>>>> tools/binman/test/281_ti_secure_combined.dts | 12 ++ > >>>>>>> 7 files changed, 266 insertions(+) > >>>>>>> create mode 100644 tools/binman/btool/tisecuretool.py > >>>>>>> create mode 100644 tools/binman/etype/ti_secure.py > >>>>>>> create mode 100644 tools/binman/test/278_ti_secure_rom.dts > >>>>>>> create mode 100644 tools/binman/test/279_ti_secure.dts > >>>>>>> create mode 100644 tools/binman/test/280_ti_secure_nofilename.dts > >>>>>>> create mode 100644 tools/binman/test/281_ti_secure_combined.dts > >>>>>> > >>>>>> Now that I see what you are doing, this it not quite the right way. > >>>>>> > >>>>>> See this hack-up of how you can call the openssl thing. Basically you > >>>>>> should not have a shell script in the way, but instead make your > >>>>>> bintool do it. > >>>>>> > >>>>>> https://github.com/sjg20/u-boot/commit/03c0d74f81106570b18d8e4fe7a3355bfeb0d5da#r100378804 > >>>>>> > >>>>>> I suppose we can have an openssl bintool that others build on top of? > >>>>>> > >>>>>> Regards, > >>>>>> Simon > >>>>>> > >>>>>> > >>>>> > >>>>> That is possible, maybe ti-secure extends from x509_cert etype so as to > >>>>> support the TI specific certificate extensions. I'll start working on > >>>>> this. > >>>>> > >>>>> However the patches I've sent support external production flow where > >>>>> signing need not necessarily be carried out from U-Boot. An external > >>>>> repo that acts as what is core-secdev-k3 here, would be the repo > >>>>> responsible for signing. > >>>>> > >>>>> Could we find a way to combine both so as to support production flow > >>>>> mandating an external agency, as well as a completely within U-Boot flow > >>>>> using bintool? i.e. we modify ti-secure etype to be able to support both > >>>>> external git repo/executable script signing as well as default signing > >>>>> using openssl bintool. > >>>> > >>>> Yes that seems important. > >>>> > >>>> One option is to have binman emit some instructions on how to sign the > >>>> image, perhaps a simple data format similar to an fdtmap, with a basic > >>>> shell script plus whatever the etype provides. Then the signer can > >>>> follow the instructions or run the script. > >>>> > >>>> Another option is to run binman on the signer and have it do the > >>>> signing. Would that work? > >>>> > >>> > >>> I'd like to keep the dependencies needed on the signing server as > >>> minimal as possible. We do require the "u-boot-tools" package on > >>> the key servers if folks want to re-package signed FIT images, so > >>> if binman could be reasonably packaged in that package someday > >>> it could be an option. > > > > At present binman is packaged as binary-manager (with 'pip install'). > > Is that good enough? We could add it to u-boot-tools, perhaps, but I > > have found that packing Python things together with non-Python things > > seems to be tricky. We gave up with libfdt and ended up using a > > separate package for pylibfdt. > > > >>> > >>> For now, if binman could generate the x509 file and leave it with the > >>> other boot artifacts then if one needs to re-sign with real keys they > >>> will only need to have openssl sign that cert and append to the image > >>> on the key server, that is simple enough document. > > > > I sent a patch to add x509 cert using the openssl tool[1]. If there is > > more needed, can you take a look or let me know what is missing? > > > > I had looked at the patch and I think it should support custom > certificates as well. I think an x509 etype should be able to produce a > general certificate like what you have done, as well as have the > capability to add more extensions, change parameters etc. I am thinking > of a template and dict sort of method where you pass your custom > template as well as a dict/config file containing > parameter:parameter-valye pairs for the etype to fill in, does that > sound okay? Then in use cases like ours where there's more done we could
Yes that seems OK to me. I do like the idea of have specific methods in the bintool class which do useful things. But if the amount of diversity is too large, then a template/dict can provide flexibility. Of course it means the caller needs to understand the format, but I suppose that is fair enough. Still, if we have a few specific use cases, then I would prefer a method for each, rather than a generic function called with lots of args / dict. > extend this etype and use that parse-and-replace mechanism for our > certificate. OK. Regards, Simon > > >>> > >>> The more difficult part is in re-packaging these signed images. > >>> Our security tools already have a tool to disasemble a FIT image, > >>> sign the components, then re-assemble it[0]. This would work for > >>> u-boot.img and the kernel FIT images, but we would need something > >>> new for re-assembling boot3.bin and tispl.bin. The first boot files > >>> (boot3.bin) is also more complex that simple FIT images in that it > >>> has several variations in format and content based on device family > >>> and type. > >>> > >>> The ideal case would be we do not need to pull in the TI security tools > >>> package at all and we could drop this patch. The tools would then only be > >>> needed by folks wanting to sign their images using an external key > >>> server. > > > > Yes, that seems to be the bare minimum of what we should reach here. > > Any tools we need should be available in 'binman tool -l' and be > > fetchable / buildable with binman. It is such a pain for users if > > everyone does their own thing. It is even more difficult for people > > who develop U-Boot generally, since they just want to build and run. > > For example, in my lab I want to be able to build U-Boot in a generic > > way and have it boot on a board. > > > >>> > >>> If we could have binman learn how to generate/template x509 certs and > >>> have it sign them with openssl, plus add the TI dummy[1] and degenerate[2] > >>> keys to the u-boot source repo, then we would not need TI security > >>> tools any more here. > > > > OK, see the patch. > > > >>> > >>> This might be a longer term goal though, and I think we are already > >>> trying to do too much all at once as is. Perhaps we could take > >>> this current solution posted here with the intent to remove it in > >>> the near future. Thoughts? > > > > IMO the problem here is starting with shell scripts. They are OK for > > hacking but we are trying to use a data-driven image description, > > rather than having a lot of shell scripts. > > > > It took ages to remove the SPL_FIT_GENERATOR stuff > > > >>> > >>> Andrew > >>> > >>> [0] > >>> https://git.ti.com/cgit/security-development-tools/core-secdev-k3/tree/scripts/fit-image-secure.sh > >>> [1] > >>> https://git.ti.com/cgit/security-development-tools/core-secdev-k3/tree/keys/custMpk.pem > >>> [2] > >>> https://git.ti.com/cgit/security-development-tools/core-secdev-k3/tree/keys/ti-degenerate-key.pem > >>> > >> > >> I think leaving the boot artifacts while going with the current solution > >> would be a better option. As Andrew mentioned, the complexity mainly > >> comes with the way our tiboot3.bin is packaged. It seems cleaner for now > >> to go with this since this patch series is only a part in the larger > >> series of packaging all the bootloader images. > >> > >> I will take future action to eventually completely remove using TI > >> security tools. What do you think, Simon? > > > > I am not convinced. We still have SPL_FIT_GENERATOR after 5 years. > > > > Can we not just do this work now? I am happy to help on the binman > > side, but I have not heard back on the series I sent, so I am not sure > > what is missing. > > > > [..] > > Regards, > > Simon > > > > [1] > > https://patchwork.ozlabs.org/project/uboot/patch/20230303000246.2640473-4-...@chromium.org/ > > -- > Thanking You > Neha Malcom Francis