Message ID | 20210517183904.853304-6-mr.nuke.me@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | Patrice Chotard |
Headers | show |
Series | Enable ECDSA FIT verification for stm32mp | expand |
Hi Alexandru, On 5/17/21 8:39 PM, Alexandru Gagniuc wrote: > This test verifies that ECDSA_UCLASS is implemented, and that > ecdsa_verify() works as expected. The definition of "expected" is > "does not find a device, and returns -ENODEV". > > The lack of a hardware-independent ECDSA implementation prevents us > from having one in the sandbox, for now. > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> > Reviewed-by: Simon Glass <sjg@chromium.org> > --- > configs/sandbox_defconfig | 2 ++ > test/dm/Makefile | 1 + > test/dm/ecdsa.c | 39 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 42 insertions(+) > create mode 100644 test/dm/ecdsa.c > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig > index 7b8603d1ef..e40bcb4b16 100644 > --- a/configs/sandbox_defconfig > +++ b/configs/sandbox_defconfig > @@ -287,3 +287,5 @@ CONFIG_TEST_FDTDEC=y > CONFIG_UNIT_TEST=y > CONFIG_UT_TIME=y > CONFIG_UT_DM=y > +CONFIG_ECDSA=y > +CONFIG_ECDSA_VERIFY=y > diff --git a/test/dm/Makefile b/test/dm/Makefile > index c9644617a1..3508aa1968 100644 > --- a/test/dm/Makefile > +++ b/test/dm/Makefile > @@ -34,6 +34,7 @@ obj-$(CONFIG_DEVRES) += devres.o > obj-$(CONFIG_DMA) += dma.o > obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi_host.o > obj-$(CONFIG_DM_DSA) += dsa.o > +obj-$(CONFIG_ECDSA_VERIFY) += ecdsa.o > obj-$(CONFIG_DM_ETH) += eth.o > ifneq ($(CONFIG_EFI_PARTITION),) > obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o > diff --git a/test/dm/ecdsa.c b/test/dm/ecdsa.c > new file mode 100644 > index 0000000000..9c0007b180 > --- /dev/null > +++ b/test/dm/ecdsa.c > @@ -0,0 +1,39 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > + > +#include <crypto/ecdsa-uclass.h> > +#include <dm.h> > +#include <dm/test.h> > +#include <test/ut.h> > +#include <u-boot/ecdsa.h> > + > +/* > + * Basic test of the ECDSA uclass and ecdsa_verify() > + * > + * ECDSA implementations in u-boot are hardware-dependent. Until we have a > + * software implementation that can be compiled into the sandbox, all we can > + * test is the uclass support. > + * > + * The uclass_get() test is redundant since ecdsa_verify() would also fail. We > + * run both functions in order to isolate the cause more clearly. i.e. is > + * ecdsa_verify() failing because the UCLASS is absent/broken? > + */ > +static int dm_test_ecdsa_verify(struct unit_test_state *uts) > +{ > + const struct ecdsa_ops *ops; > + struct uclass *ucp; > + > + const struct checksum_algo algo = { > + .checksum_len = 256, > + }; > + > + struct image_sign_info info = { > + .checksum = &algo, > + }; > + > + ut_assertok(uclass_get(UCLASS_ECDSA, &ucp)); > + ut_assertnonnull(ucp); > + ut_asserteq(-ENODEV, ecdsa_verify(&info, NULL, 0, NULL, 0)); > + > + return 0; > +} > +DM_TEST(dm_test_ecdsa_verify, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); When I prepare the stm32 pull request, I detect a issue with this patch in CI pipeline: https://source.denx.de/u-boot/custodians/u-boot-stm/-/jobs/298432 + sandbox test.py + sandbox with clang test.py With the same errors: Building current source for 1 boards (1 thread, 32 jobs per thread) sandbox: + sandbox +test/dm/ecdsa.c:30:15: error: initializing 'struct checksum_algo *' with an expression of type 'const struct checksum_algo *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] + .checksum = &algo, + ^~~~~ +test/dm/ecdsa.c:22:26: error: unused variable 'ops' [-Werror,-Wunused-variable] + const struct ecdsa_ops *ops; + ^ +2 errors generated. +make[3]: *** [scripts/Makefile.build:253: test/dm/ecdsa.o] Error 1 +make[2]: *** [scripts/Makefile.build:394: test/dm] Error 2 +make[1]: *** [Makefile:1815: test] Error 2 +make: *** [Makefile:177: sub-make] Error 2 0 0 1 /1 sandbox Can you correct this issue. Thanks, Patrick
This series is based on the latest master, so no patch dependencies. Q: Will there be a software-only implementation of ECDSA ? A: That is the goal, so that we can have more extensive testing with the sandbox. I don not have the bandwidth to implement it. There has been an initial poer of software ecdsa here: https://github.com/timr11/u-boot/tree/ecdsa-vrf-1 Q: Can more code be shared with the RSA verification path? A: Probably yes. Mostly having to do with parsing the "/signature" node and "key-name-hint"s in the u-boot FDT. Although there isn't any copypasted RSA code, or code with substantial similarity. Changes since v5: - Fixed clang warning stemming from test/dm/ecdsa.c Changes since v4: - Use U_BOOT_CRYPTO_ALGO() to add ECDSA to .u_boot_list - No need to #define IMAGE_ENABLE_VERIFY_ECDSA - Use ut_asserteq(x, -ENODEV) instead of ut_assert(x == -ENODEV) Changes since v3: - Remove unused ecdsa_check_key() function Changes since v2: - Spell out "elliptic curve" in Kconfig (Although RSA isn't spelled out) Changes since v1: - Add test to make sure the UCLASS is enabled - Fix check against wrong sig_len in ecdsa_romapi.c - s/U_BOOT_DEVICE/U_BOOT_DRVINFO/ - Use "if(!ret)" instead of "if (ret == 0)" - Use uclass_first_device_err() instead of uclass_fi Alexandru Gagniuc (5): dm: crypto: Define UCLASS API for ECDSA signature verification lib: ecdsa: Implement UCLASS_ECDSA verification on target arm: stm32mp1: Implement ECDSA signature verification Kconfig: FIT_SIGNATURE should not select RSA_VERIFY test: dm: Add test for ECDSA UCLASS support arch/arm/mach-stm32mp/Kconfig | 9 ++ arch/arm/mach-stm32mp/Makefile | 1 + arch/arm/mach-stm32mp/ecdsa_romapi.c | 102 ++++++++++++++++++++ common/Kconfig.boot | 8 +- configs/sandbox_defconfig | 2 + include/crypto/ecdsa-uclass.h | 39 ++++++++ include/dm/uclass-id.h | 1 + lib/Kconfig | 1 + lib/Makefile | 1 + lib/ecdsa/Kconfig | 23 +++++ lib/ecdsa/Makefile | 1 + lib/ecdsa/ecdsa-verify.c | 134 +++++++++++++++++++++++++++ test/dm/Makefile | 1 + test/dm/ecdsa.c | 38 ++++++++ 14 files changed, 357 insertions(+), 4 deletions(-) create mode 100644 arch/arm/mach-stm32mp/ecdsa_romapi.c create mode 100644 include/crypto/ecdsa-uclass.h create mode 100644 lib/ecdsa/Kconfig create mode 100644 lib/ecdsa/Makefile create mode 100644 lib/ecdsa/ecdsa-verify.c create mode 100644 test/dm/ecdsa.c
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 7b8603d1ef..e40bcb4b16 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -287,3 +287,5 @@ CONFIG_TEST_FDTDEC=y CONFIG_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y +CONFIG_ECDSA=y +CONFIG_ECDSA_VERIFY=y diff --git a/test/dm/Makefile b/test/dm/Makefile index c9644617a1..3508aa1968 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -34,6 +34,7 @@ obj-$(CONFIG_DEVRES) += devres.o obj-$(CONFIG_DMA) += dma.o obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi_host.o obj-$(CONFIG_DM_DSA) += dsa.o +obj-$(CONFIG_ECDSA_VERIFY) += ecdsa.o obj-$(CONFIG_DM_ETH) += eth.o ifneq ($(CONFIG_EFI_PARTITION),) obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o diff --git a/test/dm/ecdsa.c b/test/dm/ecdsa.c new file mode 100644 index 0000000000..9c0007b180 --- /dev/null +++ b/test/dm/ecdsa.c @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include <crypto/ecdsa-uclass.h> +#include <dm.h> +#include <dm/test.h> +#include <test/ut.h> +#include <u-boot/ecdsa.h> + +/* + * Basic test of the ECDSA uclass and ecdsa_verify() + * + * ECDSA implementations in u-boot are hardware-dependent. Until we have a + * software implementation that can be compiled into the sandbox, all we can + * test is the uclass support. + * + * The uclass_get() test is redundant since ecdsa_verify() would also fail. We + * run both functions in order to isolate the cause more clearly. i.e. is + * ecdsa_verify() failing because the UCLASS is absent/broken? + */ +static int dm_test_ecdsa_verify(struct unit_test_state *uts) +{ + const struct ecdsa_ops *ops; + struct uclass *ucp; + + const struct checksum_algo algo = { + .checksum_len = 256, + }; + + struct image_sign_info info = { + .checksum = &algo, + }; + + ut_assertok(uclass_get(UCLASS_ECDSA, &ucp)); + ut_assertnonnull(ucp); + ut_asserteq(-ENODEV, ecdsa_verify(&info, NULL, 0, NULL, 0)); + + return 0; +} +DM_TEST(dm_test_ecdsa_verify, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);