[PATCH v4 2/6] lib: ecdsa: Add skeleton to implement ecdsa verification in u-boot

Alex G.
2021-04-26T14:21:19Z

On 4/23/21 11:56 PM, Simon Glass wrote:
> Hi Tom, Alex,
>
> On Fri, 23 Apr 2021 at 12:47, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Fri, Apr 23, 2021 at 11:55:57AM +1200, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On Thu, 22 Apr 2021 at 07:30, Alex G. <mr.nuke.me@gmail.com> wrote:
>>>>
>>>> On 4/21/21 2:15 AM, Simon Glass wrote:
>>>>> Hi Alexandru,
>>>>>
>>>>> On Fri, 16 Apr 2021 at 08:07, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>>>>>>
>>>>>> Prepare the source tree for accepting implementations of the ECDSA
>>>>>> algorithm. This patch deals with the boring aspects of Makefiles and
>>>>>> Kconfig files.
>>>>>>
>>>>>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>>>>>> ---
>>>>>> include/image.h | 10 +++++-----
>>>>>> include/u-boot/rsa.h | 2 +-
>>>>>> lib/Kconfig | 1 +
>>>>>> lib/Makefile | 1 +
>>>>>> lib/ecdsa/Kconfig | 23 +++++++++++++++++++++++
>>>>>> lib/ecdsa/Makefile | 1 +
>>>>>> lib/ecdsa/ecdsa-verify.c | 13 +++++++++++++
>>>>>> 7 files changed, 45 insertions(+), 6 deletions(-)
>>>>>> create mode 100644 lib/ecdsa/Kconfig
>>>>>> create mode 100644 lib/ecdsa/Makefile
>>>>>> create mode 100644 lib/ecdsa/ecdsa-verify.c
>>>>>
>>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>>
>>>>> nit below
>>>>>
>>>>>>
>>>>>> diff --git a/include/image.h b/include/image.h
>>>>>> index 3ff3c035a7..9b95f6783b 100644
>>>>>> --- a/include/image.h
>>>>>> +++ b/include/image.h
>>>>>> @@ -1224,20 +1224,20 @@ int calculate_hash(const void *data, int data_len, const char *algo,
>>>>>> #if defined(USE_HOSTCC)
>>>>>> # if defined(CONFIG_FIT_SIGNATURE)
>>>>>> # define IMAGE_ENABLE_SIGN 1
>>>>>> -# define IMAGE_ENABLE_VERIFY 1
>>>>>> +# define IMAGE_ENABLE_VERIFY_RSA 1
>>>>>> # define IMAGE_ENABLE_VERIFY_ECDSA 1
>>>>>> # define FIT_IMAGE_ENABLE_VERIFY 1
>>>>>> # include <openssl/evp.h>
>>>>>> # else
>>>>>> # define IMAGE_ENABLE_SIGN 0
>>>>>> -# define IMAGE_ENABLE_VERIFY 0
>>>>>> +# define IMAGE_ENABLE_VERIFY_RSA 0
>>>>>> # define IMAGE_ENABLE_VERIFY_ECDSA 0
>>>>>> # define FIT_IMAGE_ENABLE_VERIFY 0
>>>>>> # endif
>>>>>> #else
>>>>>> # define IMAGE_ENABLE_SIGN 0
>>>>>> -# define IMAGE_ENABLE_VERIFY CONFIG_IS_ENABLED(RSA_VERIFY)
>>>>>> -# define IMAGE_ENABLE_VERIFY_ECDSA 0
>>>>>> +# define IMAGE_ENABLE_VERIFY_RSA CONFIG_IS_ENABLED(RSA_VERIFY)
>>>>>> +# define IMAGE_ENABLE_VERIFY_ECDSA CONFIG_IS_ENABLED(ECDSA_VERIFY)
>>>>>
>>>>> Since we are using Kconfig now, can we drop this IMAGE_... stuff and
>>>>> just use CONFIG_IS_ENABLED() in the code?
>>>>
>>>> CONFIG_IS_ENABLED() doesn't work for host tools.
>>>
>>> I wonder if that and IS_ENABLED() can be fixed?
>>
>> Not super easily? Some sort of seeing about cleaning up the code we
>> share with userspace would be nice, yes. But it should also probably
>> means that for the user side of things we always enable a bunch of stuff
>> so that in the end we end up with (nearly) target-agnostic tools.
>
> (just to be clear, this discussion should not hold up this patch IMO)
>
> Yes and in fact at present we allow some things to be disabled in
> tools where we probably should not.
>
> My original question was about CONFIG_IS_ENABLED(). I wonder if it
> doesn't work because the CONFIG is not enabled or because of some
> other reason?

CONFIG_IS_ENABLED() macro isn't available when compiling host tools. I
suspect nobody implemented it host-side?

Alex