| Message ID | 20210316002432.2581891-7-mr.nuke.me@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Patrice Chotard |
| Headers | show |
| Series | Enable ECDSA FIT verification for stm32mp | expand |
Hi Alexandru, On Tue, 16 Mar 2021 at 13:24, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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. Yes we do need a software impl at some point. Any update on that? > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> > --- > configs/sandbox_defconfig | 2 ++ > test/dm/Makefile | 1 + > test/dm/ecdsa.c | 38 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 41 insertions(+) > create mode 100644 test/dm/ecdsa.c > Regards, Simon
+ Tim On 3/29/21 2:43 AM, Simon Glass wrote: > Hi Alexandru, > > On Tue, 16 Mar 2021 at 13:24, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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. > > Yes we do need a software impl at some point. Any update on that? I don't have any updates from Tim that you don't. I assume he's still silently hacking at it. Alex
On 3/30/21 2:17PM, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote: > I don't have any updates from Tim that you don't. I assume he's still silently hacking at it. Yep, I'm working on a software implementation of ECDSA. Currently have the OpenSSL implementation for the nistp256 curve isolated, debugging a test program that verifies a signature on data that was randomly generated, then will need to clean up unnecessary code and fit it into U-Boot. CC'd my @linux.microsoft.com email, I prefer to use that one from now on. All the best, Tim -----Original Message----- From: Alex G. <mr.nuke.me@gmail.com> Sent: March 29, 2021 2:43 PM To: Simon Glass <sjg@chromium.org> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Tom Rini <trini@konsulko.com>; Tim Romanski <t-tromanski@microsoft.com> Subject: [EXTERNAL] Re: [PATCH v2 6/6] test: dm: Add test for ECDSA UCLASS support + Tim On 3/29/21 2:43 AM, Simon Glass wrote: > Hi Alexandru, > > On Tue, 16 Mar 2021 at 13:24, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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. > > Yes we do need a software impl at some point. Any update on that? I don't have any updates from Tim that you don't. I assume he's still silently hacking at it. Alex
Update on current progress on U-Boot ECDSA verification: I've isolated
the OpenSSL code required to verify a signature signed with the
nistp256v1 curve, and I've written a small test program to show that the
code works without any external dependencies [1]. Currently fitting the
code into Alex's fork of U-Boot.
Question for Alex, I see your repo has a few branches related to ECDSA
(patch-ecdsa-v[1-5], patch-mkimage-keyfile-v{1,2}). You sent me a link
to 'patch-ecdsa-v1' in a previous email, is that the one that's being
upstreamed? Should I be working off a different branch or is that one ok?
Tim
[1] https://github.com/timr11/openssl-ecdsa-verify
On 2021-03-30 2:27 p.m., Tim Romanski wrote:
> On 3/30/21 2:17PM, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>> I don't have any updates from Tim that you don't. I assume he's still silently hacking at it.
> Yep, I'm working on a software implementation of ECDSA. Currently have the OpenSSL implementation for the nistp256 curve isolated, debugging a test program that verifies a signature on data that was randomly generated, then will need to clean up unnecessary code and fit it into U-Boot.
>
> CC'd my @linux.microsoft.com email, I prefer to use that one from now on.
>
> All the best,
> Tim
>
> -----Original Message-----
> From: Alex G. <mr.nuke.me@gmail.com>
> Sent: March 29, 2021 2:43 PM
> To: Simon Glass <sjg@chromium.org>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Tom Rini <trini@konsulko.com>; Tim Romanski <t-tromanski@microsoft.com>
> Subject: [EXTERNAL] Re: [PATCH v2 6/6] test: dm: Add test for ECDSA UCLASS support
>
> + Tim
>
> On 3/29/21 2:43 AM, Simon Glass wrote:
>> Hi Alexandru,
>>
>> On Tue, 16 Mar 2021 at 13:24, Alexandru Gagniuc <mr.nuke.me@gmail.com> 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.
>> Yes we do need a software impl at some point. Any update on that?
> I don't have any updates from Tim that you don't. I assume he's still silently hacking at it.
>
> Alex
On 4/7/21 12:29 PM, Tim Romanski wrote: > Question for Alex, I see your repo has a few branches related to ECDSA > (patch-ecdsa-v[1-5], patch-mkimage-keyfile-v{1,2}). You sent me a link > to 'patch-ecdsa-v1' in a previous email, is that the one that's being > upstreamed? Should I be working off a different branch or is that one ok? I'm up to v6 on the patch submission. The differences are not that big, but I recommend sticking to the latest. Alex
Ok, will do. I'm writing the verification code, I noticed you're passing the public key into the fdt using fdt_add_bignum, which converts the x and y values into big endian integer arrays. Do you have a method to read these values from the fdt and convert them back into bignums, or is that TODO? I can get that done if it's not yet implemented. All the best, Tim On 2021-04-07 4:03 p.m., Alex G. wrote: > On 4/7/21 12:29 PM, Tim Romanski wrote: > >> Question for Alex, I see your repo has a few branches related to >> ECDSA (patch-ecdsa-v[1-5], patch-mkimage-keyfile-v{1,2}). You sent me >> a link to 'patch-ecdsa-v1' in a previous email, is that the one >> that's being upstreamed? Should I be working off a different branch >> or is that one ok? > > I'm up to v6 on the patch submission. The differences are not that > big, but I recommend sticking to the latest. > > Alex
On 4/8/21 11:56 AM, Tim Romanski wrote: > Ok, will do. I'm writing the verification code, I noticed you're passing > the public key into the fdt using fdt_add_bignum, which converts the x > and y values into big endian integer arrays. Do you have a method to > read these values from the fdt and convert them back into bignums, or is > that TODO? I can get that done if it's not yet implemented. Because u-boot proper doesn't use openssl, there hasn't been a need to convert data into types such as BIGNUM* at runtime. You could check BN_bin2bn() for inspiration. Alex > All the best, > > Tim > > On 2021-04-07 4:03 p.m., Alex G. wrote: >> On 4/7/21 12:29 PM, Tim Romanski wrote: >> >>> Question for Alex, I see your repo has a few branches related to >>> ECDSA (patch-ecdsa-v[1-5], patch-mkimage-keyfile-v{1,2}). You sent me >>> a link to 'patch-ecdsa-v1' in a previous email, is that the one >>> that's being upstreamed? Should I be working off a different branch >>> or is that one ok? >> >> I'm up to v6 on the patch submission. The differences are not that >> big, but I recommend sticking to the latest. >> >> Alex
Update on ECDSA verification progress, I've forked Alex's repo and have included my changes in the 'ecdsa-vrf-1' branch [1]. This includes the isolated OpenSSL code for verification, and I split up the lib/ecdsa/ecdsa-libcrypto.c file into lib/ecdsa/ecdsa-sign.c and lib/ecdsa/ecdsa-verify.c. I've also included unit tests under test/py/tests/test_vboot_ecdsa.py, which test ECDSA with the sha1 and sha256 digest algos. There are some outstanding changes to be made before it's ready for review, mainly cleaning up the OpenSSL code as it has redundant code still included though it works without any additional dependencies, and better integration with U-Boot's build system. Currently I've added a new Kconfig setting to turn on ECDSA signing/verification called "CONFIG_FIT_SIGNATURE_ECDSA" in common/Kconfig.boot which sets config options "CONFIG_ECDSA" and "CONFIG_ECDSA_VERIFY". This is done mainly to replicate how the RSA config was setup, though creating "CONFIG_FIT_SIGNATURE_ECDSA" separate from "CONFIG_FIT_SIGNATURE" feels messy, there's probably a better approach. Today is also my last day at my internship. Deskin, a team member of mine at Microsoft who was keeping an eye on the project, will be the main point of contact from here (deskinm@linux.microsoft.com) though I can also be reached at timromanski@gmail.com (CC'd) and will be responsive if there are any questions. All the best, Tim [1] timr11/u-boot: u-boot + elliptic curve verification (github.com) <https://github.com/timr11/u-boot> On 2021-04-08 12:56 p.m., Tim Romanski wrote: > Ok, will do. I'm writing the verification code, I noticed you're > passing the public key into the fdt using fdt_add_bignum, which > converts the x and y values into big endian integer arrays. Do you > have a method to read these values from the fdt and convert them back > into bignums, or is that TODO? I can get that done if it's not yet > implemented. > > All the best, > > Tim > > On 2021-04-07 4:03 p.m., Alex G. wrote: >> On 4/7/21 12:29 PM, Tim Romanski wrote: >> >>> Question for Alex, I see your repo has a few branches related to >>> ECDSA (patch-ecdsa-v[1-5], patch-mkimage-keyfile-v{1,2}). You sent >>> me a link to 'patch-ecdsa-v1' in a previous email, is that the one >>> that's being upstreamed? Should I be working off a different branch >>> or is that one ok? >> >> I'm up to v6 on the patch submission. The differences are not that >> big, but I recommend sticking to the latest. >> >> Alex
On Fri, Apr 23, 2021 at 01:03:25PM -0400, Tim Romanski wrote: > Update on ECDSA verification progress, I've forked Alex's repo and have > included my changes in the 'ecdsa-vrf-1' branch [1]. This includes the > isolated OpenSSL code for verification, and I split up the > lib/ecdsa/ecdsa-libcrypto.c file into lib/ecdsa/ecdsa-sign.c and > lib/ecdsa/ecdsa-verify.c. I've also included unit tests under > test/py/tests/test_vboot_ecdsa.py, which test ECDSA with the sha1 and sha256 > digest algos. There are some outstanding changes to be made before it's > ready for review, mainly cleaning up the OpenSSL code as it has redundant > code still included though it works without any additional dependencies, and > better integration with U-Boot's build system. Currently I've added a new > Kconfig setting to turn on ECDSA signing/verification called > "CONFIG_FIT_SIGNATURE_ECDSA" in common/Kconfig.boot which sets config > options "CONFIG_ECDSA" and "CONFIG_ECDSA_VERIFY". This is done mainly to > replicate how the RSA config was setup, though creating > "CONFIG_FIT_SIGNATURE_ECDSA" separate from "CONFIG_FIT_SIGNATURE" feels > messy, there's probably a better approach. > > Today is also my last day at my internship. Deskin, a team member of mine at > Microsoft who was keeping an eye on the project, will be the main point of > contact from here (deskinm@linux.microsoft.com) though I can also be reached > at timromanski@gmail.com (CC'd) and will be responsive if there are any > questions. > > All the best, Thanks for all your effort on this!
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 5bc90d09a8..bb73cab18d 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -283,3 +283,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 fd1455109d..7d1870f941 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_CLK) += clk.o clk_ccf.o obj-$(CONFIG_CROS_EC) += cros_ec.o obj-$(CONFIG_DEVRES) += devres.o obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi_host.o +obj-$(CONFIG_ECDSA_VERIFY) += ecdsa.o obj-$(CONFIG_DM_ETH) += eth.o obj-$(CONFIG_FIRMWARE) += firmware.o obj-$(CONFIG_DM_GPIO) += gpio.o diff --git a/test/dm/ecdsa.c b/test/dm/ecdsa.c new file mode 100644 index 0000000000..23d57dd47f --- /dev/null +++ b/test/dm/ecdsa.c @@ -0,0 +1,38 @@ +// 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_assert(ecdsa_verify(&info, NULL, 0, NULL, 0) == -ENODEV); + return 0; +} +DM_TEST(dm_test_ecdsa_verify, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
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> --- configs/sandbox_defconfig | 2 ++ test/dm/Makefile | 1 + test/dm/ecdsa.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 test/dm/ecdsa.c