Hi,
On 05.08.24 14:44, Sascha Hauer wrote:
> On Mon, Aug 05, 2024 at 01:57:36PM +0200, Ahmad Fatoum wrote:
>> On 01.08.24 07:57, Sascha Hauer wrote:
>>> This adds ECDSA signature verification support. The code is based on the
>>> Linux code as of Linux-6.10. The Linux code expects the key to be in
>>> ASN.1 encoded format. We don't need this in barebox as directly compile
>>> the x and y key values into the binary, so this is left out.
>>>
>>> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
>>> ---
>>> crypto/Kconfig | 20 ++++
>>> crypto/Makefile | 19 ++++
>>> crypto/ecdsa.c | 169 ++++++++++++++++++++++++++++++
>>> include/asm-generic/barebox.lds.h | 7 ++
>>> include/ecdsa.h | 21 ++++
>>> 5 files changed, 236 insertions(+)
>>> create mode 100644 crypto/ecdsa.c
>>> create mode 100644 include/ecdsa.h
>>>
>>> diff --git a/crypto/Kconfig b/crypto/Kconfig
>>> index e953ef5e15..eeacd9ffb7 100644
>>> --- a/crypto/Kconfig
>>> +++ b/crypto/Kconfig
>>> @@ -156,4 +156,24 @@ config JWT
>>> config CRYPTO_ECC
>>> bool
>>>
>>> +config CRYPTO_ECDSA
>>> + bool "ECDSA support"
>>> + select CRYPTO_ECC
>>> +
>>> +config CRYPTO_ECDSA_BUILTIN_KEYS
>>> + bool
>>> + default y if CRYPTO_ECDSA_KEY != ""
>>> + select KEYTOC
>>> +
>>> +config CRYPTO_ECDSA_KEY
>>> + depends on CRYPTO_ECDSA
>>> + string "ECDSA key to compile in"
>>> + help
>>> + This option should be a filename of a PEM-formatted file containing
>>> + X.509 certificates to be included into barebox. If the string starts
>>> + with "pkcs11:" it is interpreted as a PKCS#11 URI rather than a file.
>>> +
>>> + This avoids the mkimage dependency of CONFIG_BOOTM_FITIMAGE_PUBKEY
>>> + at the cost of an openssl build-time dependency.
>>
>> Why can't this option take multiple space-separated paths?
>
> The code added for ECDSA is mostly a copy from the existing RSA code.
>
> It's less than ideal. What I'd really like to have is a single list of
> keys which can include both RSA and ECDSA keys instead of maintaining
> multiple lists. Likewise for the Kconfig options, it would be better to
> have a CRYPTO_PUBLIC_KEYS option which holds multiple RSA and/or
> ECDSA keys. Unfortunately my time budget for this task is over, so I
> think we'll have to stick with this until the next cleanup round.
Ok, let's leave it to behave like the RSA options then.
>>> +#define _ECDSA_H
>>> +
>>> +struct ecdsa_public_key {
>>> + const char *curve_name; /* Name of curve, e.g. "prime256v1" */
>>> + const void *x; /* x coordinate of public key */
>>> + const void *y; /* y coordinate of public key */
>>
>> Why void and not a specific type?
>
> No specific reason, it's copied from U-Boot. One reason might be that
> keytoc prints the values as array of uint32_t whereas the values in
> barebox are interpreted as array of uint64_t. Using void * avoids casts
> and covers some interesting endianess problems when the barebox
> endianess differs from the endianess of the build machine.
This looks certainly odd. It's probably worth looking into.
Cheers,
Ahmad
>
> Sascha
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
