diff mbox series

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

Message ID 20210415200509.2335046-3-mr.nuke.me@gmail.com
State Superseded
Delegated to: Patrice Chotard
Headers show
Series nable ECDSA FIT verification for stm32mp | expand

Commit Message

Alexandru Gagniuc April 15, 2021, 8:05 p.m. UTC
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

Comments

Simon Glass April 21, 2021, 7:15 a.m. UTC | #1
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?

>  # define FIT_IMAGE_ENABLE_VERIFY       CONFIG_IS_ENABLED(FIT_SIGNATURE)
>  #endif
>
> @@ -1293,7 +1293,7 @@ struct image_region {
>         int size;
>  };
>
> -#if IMAGE_ENABLE_VERIFY
> +#if FIT_IMAGE_ENABLE_VERIFY
>  # include <u-boot/hash-checksum.h>
>  #endif
>  struct checksum_algo {
> diff --git a/include/u-boot/rsa.h b/include/u-boot/rsa.h
> index bed1c097c2..eb258fca4c 100644
> --- a/include/u-boot/rsa.h
> +++ b/include/u-boot/rsa.h
> @@ -81,7 +81,7 @@ static inline int rsa_add_verify_data(struct image_sign_info *info,
>  }
>  #endif

Regards,
Simon
Alexandru Gagniuc April 21, 2021, 7:30 p.m. UTC | #2
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.

Alex
Simon Glass April 22, 2021, 11:55 p.m. UTC | #3
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?

Regards,
Simon
Tom Rini April 23, 2021, 12:47 a.m. UTC | #4
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.
Simon Glass April 24, 2021, 4:56 a.m. UTC | #5
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?

Regards,
Simon
Alexandru Gagniuc April 26, 2021, 2:21 p.m. UTC | #6
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
Simon Glass April 29, 2021, 4:10 p.m. UTC | #7
Hi Alex,

On Mon, 26 Apr 2021 at 07:21, Alex G. <mr.nuke.me@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@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?

I think it should map to IS_ENABLED(). But also, do we include
kconfig.h in the tools?

Regards,
Simon
Simon Glass May 4, 2021, 4:58 p.m. UTC | #8
Hi Alex,

On Thu, 29 Apr 2021 at 10:10, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Alex,
>
> On Mon, 26 Apr 2021 at 07:21, Alex G. <mr.nuke.me@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@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?
>
> 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.

Regards,
Simon
Alexandru Gagniuc May 5, 2021, 5:49 p.m. UTC | #9
On 5/4/21 11:58 AM, Simon Glass wrote:
> Hi Alex,
> 
> On Thu, 29 Apr 2021 at 10:10, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Alex,
>>
>> On Mon, 26 Apr 2021 at 07:21, Alex G. <mr.nuke.me@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@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?
>>
>> 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
>
Simon Glass May 5, 2021, 6:43 p.m. UTC | #10
Hi Alex,

On Wed, 5 May 2021 at 11:49, Alex G. <mr.nuke.me@gmail.com> wrote:
>
>
>
> On 5/4/21 11:58 AM, Simon Glass wrote:
> > Hi Alex,
> >
> > On Thu, 29 Apr 2021 at 10:10, Simon Glass <sjg@chromium.org> wrote:
> >>
> >> Hi Alex,
> >>
> >> On Mon, 26 Apr 2021 at 07:21, Alex G. <mr.nuke.me@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@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?
> >>
> >> 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?

Normally the series that is reviewed first is applied first, then it
is up to the subsequent series (i.e. mine) to rebase on that. It gets
a bit out of hand if people send a patch, it is reviewed, then it has
to be reworked later after someone does another patch that didn't
exist then! I am sure it happens sometimes, though. It's up to Tom.

Having said that, if you can do a fix-up patch on top of my series I
think it would be handy.

Regards,
Simon
Alexandru Gagniuc May 5, 2021, 6:56 p.m. UTC | #11
On 5/5/21 1:43 PM, Simon Glass wrote:
> Hi Alex,
> 
> On Wed, 5 May 2021 at 11:49, Alex G. <mr.nuke.me@gmail.com> wrote:
>>
>>
>>
>> On 5/4/21 11:58 AM, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On Thu, 29 Apr 2021 at 10:10, Simon Glass <sjg@chromium.org> wrote:
>>>>
>>>> Hi Alex,
>>>>
>>>> On Mon, 26 Apr 2021 at 07:21, Alex G. <mr.nuke.me@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@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?
>>>>
>>>> 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?
> 
> Normally the series that is reviewed first is applied first, then it
> is up to the subsequent series (i.e. mine) to rebase on that. It gets
> a bit out of hand if people send a patch, it is reviewed, then it has
> to be reworked later after someone does another patch that didn't
> exist then! I am sure it happens sometimes, though. It's up to Tom.
> 
> Having said that, if you can do a fix-up patch on top of my series I
> think it would be handy.

I'd rather work with you than race condition against you. If you're 
willing to plow through review on your series, and get it merged soon, 
I'll take the red pill and rebase my series.

Alex

> Regards,
> Simon
>
diff mbox series

Patch

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)
 # define FIT_IMAGE_ENABLE_VERIFY	CONFIG_IS_ENABLED(FIT_SIGNATURE)
 #endif
 
@@ -1293,7 +1293,7 @@  struct image_region {
 	int size;
 };
 
-#if IMAGE_ENABLE_VERIFY
+#if FIT_IMAGE_ENABLE_VERIFY
 # include <u-boot/hash-checksum.h>
 #endif
 struct checksum_algo {
diff --git a/include/u-boot/rsa.h b/include/u-boot/rsa.h
index bed1c097c2..eb258fca4c 100644
--- a/include/u-boot/rsa.h
+++ b/include/u-boot/rsa.h
@@ -81,7 +81,7 @@  static inline int rsa_add_verify_data(struct image_sign_info *info,
 }
 #endif
 
-#if IMAGE_ENABLE_VERIFY
+#if IMAGE_ENABLE_VERIFY_RSA
 /**
  * rsa_verify_hash() - Verify a signature against a hash
  *
diff --git a/lib/Kconfig b/lib/Kconfig
index ab8c9ccd60..584ab7b536 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -295,6 +295,7 @@  config AES
 	  supported by the algorithm but only a 128-bit key is supported at
 	  present.
 
+source lib/ecdsa/Kconfig
 source lib/rsa/Kconfig
 source lib/crypto/Kconfig
 
diff --git a/lib/Makefile b/lib/Makefile
index 6825671955..2c7c145a27 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -60,6 +60,7 @@  endif
 
 obj-$(CONFIG_$(SPL_)ACPIGEN) += acpi/
 obj-$(CONFIG_$(SPL_)MD5) += md5.o
+obj-$(CONFIG_ECDSA) += ecdsa/
 obj-$(CONFIG_$(SPL_)RSA) += rsa/
 obj-$(CONFIG_FIT_SIGNATURE) += hash-checksum.o
 obj-$(CONFIG_SHA1) += sha1.o
diff --git a/lib/ecdsa/Kconfig b/lib/ecdsa/Kconfig
new file mode 100644
index 0000000000..a95c4ff581
--- /dev/null
+++ b/lib/ecdsa/Kconfig
@@ -0,0 +1,23 @@ 
+config ECDSA
+	bool "Enable ECDSA support"
+	depends on DM
+	help
+	  This enables the ECDSA (elliptic curve signature) algorithm for FIT
+	  image verification in U-Boot. The ECDSA algorithm is implemented
+	  using the driver model, so CONFIG_DM is required by this library.
+	  See doc/uImage.FIT/signature.txt for more details.
+	  ECDSA is enabled for mkimage regardless of this option.
+
+if ECDSA
+
+config ECDSA_VERIFY
+	bool "Enable ECDSA verification support in U-Boot."
+	help
+	  Allow ECDSA signatures to be recognized and verified in U-Boot.
+
+config SPL_ECDSA_VERIFY
+	bool "Enable ECDSA verification support in SPL"
+	help
+	  Allow ECDSA signatures to be recognized and verified in SPL.
+
+endif
diff --git a/lib/ecdsa/Makefile b/lib/ecdsa/Makefile
new file mode 100644
index 0000000000..771d6d3135
--- /dev/null
+++ b/lib/ecdsa/Makefile
@@ -0,0 +1 @@ 
+obj-$(CONFIG_$(SPL_)ECDSA_VERIFY) += ecdsa-verify.o
diff --git a/lib/ecdsa/ecdsa-verify.c b/lib/ecdsa/ecdsa-verify.c
new file mode 100644
index 0000000000..d2e6a40f4a
--- /dev/null
+++ b/lib/ecdsa/ecdsa-verify.c
@@ -0,0 +1,13 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Alexandru Gagniuc <mr.nuke.me@gmail.com>
+ */
+
+#include <u-boot/ecdsa.h>
+
+int ecdsa_verify(struct image_sign_info *info,
+		 const struct image_region region[], int region_count,
+		 uint8_t *sig, uint sig_len)
+{
+	return -EOPNOTSUPP;
+}