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

[Simon Glass]

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?

Regards,
Simon