| Message ID | 20211007062340.72207-5-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi_loader: capsule: improve capsule authentication support | expand |
Hi Takahiro, On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > With this script, a public key is added to a device tree blob > as the default efi_get_public_key_data() expects. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > MAINTAINERS | 1 + > tools/fdtsig.sh | 40 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 41 insertions(+) > create mode 100755 tools/fdtsig.sh Instead of an ad-hoc script with no tests, could we use binman for putting the image together and inserting it? Regards, Simon
Simon, On Mon, Oct 11, 2021 at 08:54:09AM -0600, Simon Glass wrote: > Hi Takahiro, > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > > > With this script, a public key is added to a device tree blob > > as the default efi_get_public_key_data() expects. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > MAINTAINERS | 1 + > > tools/fdtsig.sh | 40 ++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 41 insertions(+) > > create mode 100755 tools/fdtsig.sh > > Instead of an ad-hoc script with no tests, Basically I intended to provide fdtsig.sh as a *sample* script so that people may want to integrate the logic into their own build rule/systems. But I could use this script in my 'capsule authentication' test that is also added in patch#22. > could we use binman for > putting the image together and inserting it? First, as you can see, the script is quite simple and secondly, the purpose of binman, IIUC, is to help handle/manipulate U-Boot image binaries. So I'm not sure whether it is really useful to add such a feature to binman. -Takahiro Akashi > Regards, > Simon
Hi Takahiro, On Mon, 11 Oct 2021 at 19:42, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > Simon, > > On Mon, Oct 11, 2021 at 08:54:09AM -0600, Simon Glass wrote: > > Hi Takahiro, > > > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > > > > > With this script, a public key is added to a device tree blob > > > as the default efi_get_public_key_data() expects. > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > --- > > > MAINTAINERS | 1 + > > > tools/fdtsig.sh | 40 ++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 41 insertions(+) > > > create mode 100755 tools/fdtsig.sh > > > > Instead of an ad-hoc script with no tests, > > Basically I intended to provide fdtsig.sh as a *sample* script so that > people may want to integrate the logic into their own build rule/systems. > But I could use this script in my 'capsule authentication' test > that is also added in patch#22. > > > could we use binman for > > putting the image together and inserting it? > > First, as you can see, the script is quite simple and secondly, > the purpose of binman, IIUC, is to help handle/manipulate U-Boot > image binaries. > So I'm not sure whether it is really useful to add such a feature to binman. I'm not sure. The script seems very ad-hoc to me, for a feature that Linaro is pushing so hard. I don't see where the script is used in the tests or even mentioned in the documentation. Am I missing something? Regards, Simon
Simon, On Thu, Oct 14, 2021 at 06:40:24PM -0600, Simon Glass wrote: > Hi Takahiro, > > On Mon, 11 Oct 2021 at 19:42, AKASHI Takahiro > <takahiro.akashi@linaro.org> wrote: > > > > Simon, > > > > On Mon, Oct 11, 2021 at 08:54:09AM -0600, Simon Glass wrote: > > > Hi Takahiro, > > > > > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > > > > > > > With this script, a public key is added to a device tree blob > > > > as the default efi_get_public_key_data() expects. > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > --- > > > > MAINTAINERS | 1 + > > > > tools/fdtsig.sh | 40 ++++++++++++++++++++++++++++++++++++++++ > > > > 2 files changed, 41 insertions(+) > > > > create mode 100755 tools/fdtsig.sh > > > > > > Instead of an ad-hoc script with no tests, > > > > Basically I intended to provide fdtsig.sh as a *sample* script so that > > people may want to integrate the logic into their own build rule/systems. > > But I could use this script in my 'capsule authentication' test > > that is also added in patch#22. > > > > > could we use binman for > > > putting the image together and inserting it? > > > > First, as you can see, the script is quite simple and secondly, > > the purpose of binman, IIUC, is to help handle/manipulate U-Boot > > image binaries. > > So I'm not sure whether it is really useful to add such a feature to binman. > > I'm not sure. The script seems very ad-hoc to me, for a feature that > Linaro is pushing so hard. To be honest, I've never used binman :) So I'm not sure whether binman is the best place to add this feature. For example, README under tools/binman says, "It seems better to use the mkimage tool to generate binaries and avoid blurring the boundaries between building input files (mkimage) and packaging then into a final image (binman)." Obviously, dtb is not the final image. > I don't see where the script is used in the tests or even mentioned in > the documentation. Am I missing something? Due to the history of submissions of this series, the current pytest scenario doesn't use the script, but you can see the exact same sequence of commands at test/py/tests/test_efi_capsule/conftest.py: ---8<--- # Update dtb adding capsule certificate check_call('cd %s; cp %s/test/py/tests/test_efi_capsule/signature.dts .' % (data_dir, u_boot_config.source_dir), shell=True) check_call('cd %s; dtc -@ -I dts -O dtb -o signature.dtbo signature.dts; fdtoverlay -i %s/arch/sandbox/dts/test.dtb -o test_sig.dtb signature.dtbo' % (data_dir, u_boot_config.build_dir), shell=True) --->8--- (Please see my patch#11.) What I meant is that we can directly use fdtsig.sh here if your concern is that the script is *not exercised* anywhere. -Takahiro Akashi > Regards, > Simon
On Mon, Oct 25, 2021 at 12:06:39PM +0900, AKASHI Takahiro wrote: > Simon, > > On Thu, Oct 14, 2021 at 06:40:24PM -0600, Simon Glass wrote: > > Hi Takahiro, > > > > On Mon, 11 Oct 2021 at 19:42, AKASHI Takahiro > > <takahiro.akashi@linaro.org> wrote: > > > > > > Simon, > > > > > > On Mon, Oct 11, 2021 at 08:54:09AM -0600, Simon Glass wrote: > > > > Hi Takahiro, > > > > > > > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > > > > > > > > > With this script, a public key is added to a device tree blob > > > > > as the default efi_get_public_key_data() expects. > > > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > > --- > > > > > MAINTAINERS | 1 + > > > > > tools/fdtsig.sh | 40 ++++++++++++++++++++++++++++++++++++++++ > > > > > 2 files changed, 41 insertions(+) > > > > > create mode 100755 tools/fdtsig.sh > > > > > > > > Instead of an ad-hoc script with no tests, > > > > > > Basically I intended to provide fdtsig.sh as a *sample* script so that > > > people may want to integrate the logic into their own build rule/systems. > > > But I could use this script in my 'capsule authentication' test > > > that is also added in patch#22. > > > > > > > could we use binman for > > > > putting the image together and inserting it? > > > > > > First, as you can see, the script is quite simple and secondly, > > > the purpose of binman, IIUC, is to help handle/manipulate U-Boot > > > image binaries. > > > So I'm not sure whether it is really useful to add such a feature to binman. > > > > I'm not sure. The script seems very ad-hoc to me, for a feature that > > Linaro is pushing so hard. > > To be honest, I've never used binman :) So I'm not sure whether binman > is the best place to add this feature. For example, README under tools/binman > says, "It seems better to use the mkimage tool to generate binaries and avoid > blurring the boundaries between building input files (mkimage) and packaging > then into a final image (binman)." > Obviously, dtb is not the final image. > > > I don't see where the script is used in the tests or even mentioned in > > the documentation. Am I missing something? > > Due to the history of submissions of this series, the current pytest > scenario doesn't use the script, but you can see the exact same > sequence of commands at test/py/tests/test_efi_capsule/conftest.py: > ---8<--- > # Update dtb adding capsule certificate > check_call('cd %s; cp %s/test/py/tests/test_efi_capsule/signature.dts .' > % (data_dir, u_boot_config.source_dir), shell=True) > check_call('cd %s; dtc -@ -I dts -O dtb -o signature.dtbo signature.dts; fdtoverlay -i %s/arch/sandbox/dts/test.dtb -o test_sig.dtb signature.dtbo' > % (data_dir, u_boot_config.build_dir), shell=True) > --->8--- > (Please see my patch#11.) > > What I meant is that we can directly use fdtsig.sh here if your concern > is that the script is *not exercised* anywhere. Besides binman or fdtsig.sh, I found that the crucial information was missing here; the format or how a public key is encoded in a device tree. With an example of command sequence, we may drop this patch. So will adding some description to uefi.rst satisfy your needs, or do you expect an extra rule for embedding a key to be added in some Makefile? (I don't think this step is part of build process, though.) -Takahiro Akashi > -Takahiro Akashi > > > Regards, > > Simon
Hi Takahiro, On Tue, 26 Oct 2021 at 00:00, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > On Mon, Oct 25, 2021 at 12:06:39PM +0900, AKASHI Takahiro wrote: > > Simon, > > > > On Thu, Oct 14, 2021 at 06:40:24PM -0600, Simon Glass wrote: > > > Hi Takahiro, > > > > > > On Mon, 11 Oct 2021 at 19:42, AKASHI Takahiro > > > <takahiro.akashi@linaro.org> wrote: > > > > > > > > Simon, > > > > > > > > On Mon, Oct 11, 2021 at 08:54:09AM -0600, Simon Glass wrote: > > > > > Hi Takahiro, > > > > > > > > > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > > > > > > > > > > > With this script, a public key is added to a device tree blob > > > > > > as the default efi_get_public_key_data() expects. > > > > > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > > > --- > > > > > > MAINTAINERS | 1 + > > > > > > tools/fdtsig.sh | 40 ++++++++++++++++++++++++++++++++++++++++ > > > > > > 2 files changed, 41 insertions(+) > > > > > > create mode 100755 tools/fdtsig.sh > > > > > > > > > > Instead of an ad-hoc script with no tests, > > > > > > > > Basically I intended to provide fdtsig.sh as a *sample* script so that > > > > people may want to integrate the logic into their own build rule/systems. > > > > But I could use this script in my 'capsule authentication' test > > > > that is also added in patch#22. > > > > > > > > > could we use binman for > > > > > putting the image together and inserting it? > > > > > > > > First, as you can see, the script is quite simple and secondly, > > > > the purpose of binman, IIUC, is to help handle/manipulate U-Boot > > > > image binaries. > > > > So I'm not sure whether it is really useful to add such a feature to binman. > > > > > > I'm not sure. The script seems very ad-hoc to me, for a feature that > > > Linaro is pushing so hard. > > > > To be honest, I've never used binman :) So I'm not sure whether binman > > is the best place to add this feature. For example, README under tools/binman > > says, "It seems better to use the mkimage tool to generate binaries and avoid > > blurring the boundaries between building input files (mkimage) and packaging > > then into a final image (binman)." > > Obviously, dtb is not the final image. > > > > > I don't see where the script is used in the tests or even mentioned in > > > the documentation. Am I missing something? > > > > Due to the history of submissions of this series, the current pytest > > scenario doesn't use the script, but you can see the exact same > > sequence of commands at test/py/tests/test_efi_capsule/conftest.py: > > ---8<--- > > # Update dtb adding capsule certificate > > check_call('cd %s; cp %s/test/py/tests/test_efi_capsule/signature.dts .' > > % (data_dir, u_boot_config.source_dir), shell=True) > > check_call('cd %s; dtc -@ -I dts -O dtb -o signature.dtbo signature.dts; fdtoverlay -i %s/arch/sandbox/dts/test.dtb -o test_sig.dtb signature.dtbo' > > % (data_dir, u_boot_config.build_dir), shell=True) > > --->8--- > > (Please see my patch#11.) > > > > What I meant is that we can directly use fdtsig.sh here if your concern > > is that the script is *not exercised* anywhere. > > Besides binman or fdtsig.sh, I found that the crucial information was > missing here; the format or how a public key is encoded in a device tree. > With an example of command sequence, we may drop this patch. > > So will adding some description to uefi.rst satisfy your needs, Seems OK to me, and some sort of CI test. > or do you expect an extra rule for embedding a key to be added > in some Makefile? > (I don't think this step is part of build process, though.) I agree, this is best done in the packaging step, after all the bits have been built. Binman is designed to run inside the U-Boot build and also separately, to collect everything and redo whatever happened in the U-Boot build. REgards, Simon
diff --git a/MAINTAINERS b/MAINTAINERS index 5370b550648e..650e428b6cb4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -713,6 +713,7 @@ F: cmd/bootefi.c F: cmd/efidebug.c F: cmd/nvedit_efi.c F: tools/efivar.py +F: tools/fdtsig.sh F: tools/file2include.c F: tools/mkeficapsule.c diff --git a/tools/fdtsig.sh b/tools/fdtsig.sh new file mode 100755 index 000000000000..5ce7357614d7 --- /dev/null +++ b/tools/fdtsig.sh @@ -0,0 +1,40 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0+ +# +# script to add a certificate (efi-signature-list) to dtb blob + +usage() { + if [ -n "$*" ]; then + echo "ERROR: $*" + fi + echo "Usage: "$(basename $0) " <esl file> <dtb file>" +} + +if [ "$#" -ne 2 ]; then + usage "Arguments missing" + exit 1 +fi + +ESL=$1 +DTB=$2 +NEW_DTB=$(basename $DTB)_tmp +SIG=signature + +cat << 'EOF' > $SIG.dts +/dts-v1/; +/plugin/; + +&{/} { + signature { + capsule-key = /incbin/("ESL"); + }; +}; +EOF + +sed -in "s/ESL/$ESL/" $SIG.dts + +dtc -@ -I dts -O dtb -o $SIG.dtbo $SIG.dts +fdtoverlay -i $DTB -o $NEW_DTB -v $SIG.dtbo +mv $NEW_DTB $DTB + +#rm $SIG.dts $SIG.dtbo $NEW_DTB
With this script, a public key is added to a device tree blob as the default efi_get_public_key_data() expects. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- MAINTAINERS | 1 + tools/fdtsig.sh | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100755 tools/fdtsig.sh -- 2.33.0