Re: [EXTERNAL] Re: [PATCH v2 6/6] test: dm: Add test for ECDSA UCLASS support

[Tom Rini]

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 (desk...@linux.microsoft.com) though I can also be reached
> at timroman...@gmail.com (CC'd) and will be responsive if there are any
> questions.
> 
> All the best,

Thanks for all your effort on this!

-- 
Tom


signature.asc
Description: PGP signature

Re: [EXTERNAL] Re: [PATCH v2 6/6] test: dm: Add test for ECDSA UCLASS support

[Tim Romanski]

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 (desk...@linux.microsoft.com) though I 
can also be reached at timroman...@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) 



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

Re: [EXTERNAL] Re: [PATCH v2 6/6] test: dm: Add test for ECDSA UCLASS support

[Tim Romanski]

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

Re: [EXTERNAL] Re: [PATCH v2 6/6] test: dm: Add test for ECDSA UCLASS support

[Alex G.]

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

Re: [EXTERNAL] Re: [PATCH v2 6/6] test: dm: Add test for ECDSA UCLASS support

[Alex G.]

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

Re: [EXTERNAL] Re: [PATCH v2 6/6] test: dm: Add test for ECDSA UCLASS support

[Tim Romanski]

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  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. 
Sent: March 29, 2021 2:43 PM
To: Simon Glass 
Cc: U-Boot Mailing List ; Tom Rini ; Tim 
Romanski 
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  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

RE: [EXTERNAL] Re: [PATCH v2 6/6] test: dm: Add test for ECDSA UCLASS support

[Tim Romanski]

On 3/30/21 2:17PM, Alexandru Gagniuc  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.  
Sent: March 29, 2021 2:43 PM
To: Simon Glass 
Cc: U-Boot Mailing List ; Tom Rini ; 
Tim Romanski 
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  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