diff mbox series

[v5,5/5] test: dm: Add test for ECDSA UCLASS support

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

Commit Message

Alex G. May 17, 2021, 6:39 p.m. UTC
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

Comments

Patrick DELAUNAY July 27, 2021, 8:09 a.m. UTC | #1
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
Alex G. July 29, 2021, 4:47 p.m. UTC | #2
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 mbox series

Patch

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);