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

[Alex G.]

On 5/4/21 11:58 AM, Simon Glass wrote:
> Hi Alex,
> 
> On Thu, 29 Apr 2021 at 10:10, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Alex,
>>
>> On Mon, 26 Apr 2021 at 07:21, Alex G. <mr.nuke.me at gmail.com> wrote:
>>>
>>>
>>>
>>> On 4/23/21 11:56 PM, Simon Glass wrote:
>>>> Hi Tom, Alex,
>>>>
>>>> On Fri, 23 Apr 2021 at 12:47, Tom Rini <trini at 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 at 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 at 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 at 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 at 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?
>>
>> I think it should map to IS_ENABLED(). But also, do we include
>> kconfig.h in the tools?
> 
> Just a note that I sent a series to enable CONFIG_IS_ENABLED on the host.

Do I need to rebase on your series?

> Regards,
> Simon
>