Re: [PATCH 1/1] lib/ecdsa: Fix LibreSSL before v2.7.0

["Alex G." <mr.nuke.me@gmail.com>]

Hi Artem,

I'm re-adding the u-boot mailing list to the CC field, as I see your 
email contains no sensitive information.

On 7/28/21 2:30 PM, Artem Panfilov wrote:
> We have broken CI builds on your bare-metal CentOS 7 servers with latest 
> master. I think it is good reason to have  a support. Our corporate 
> clients have CentOS 7 too.

I thought we solved problems associated with bare builds on ancient OSes 
by using containerized builds. There's is CI infrastructure based on 
this on source.denx.de that uses docker. As things change, and we 
eventually move to GNU TLS, a lot more things might break for old build 
hosts. I believe it's worth looking at containerized builds.

Given that a solution exists for your problem, I think the argument for 
this patch quite weak.

> There is no nice way to handle openssl difference. You could check other 
> commits related to openssl compatibility. They all looks ugly.

> Another solution is to disable CONFIG_TOOLS_LIBCRYPTO by default that 
> broke our builds.

Do you need cryptographic features in mkimage? If not just disable 
TOOLS_LIBCRYPTO in your builds.

Alex

> Best regards,
> Artem
> 
> ср, 28 июл. 2021 г., 22:16 Alex G. <mr.nuke.me@gmail.com 
> <mailto:mr.nuke.me@gmail.com>>:
> 
> 
> 
>     On 7/28/21 1:10 PM, Artem Panfilov wrote:
>      > Fix LibreSSL compilation for versions before v2.7.0.
>      >
>      > Fix following compilation issue when CONFIG_TOOLS_LIBCRYPTO is
>     enabled:
>      > tools/lib/ecdsa/ecdsa-libcrypto.o: In function `prepare_ctx':
>      > ecdsa-libcrypto.c:(.text+0x94): undefined reference to
>      > `OPENSSL_init_ssl'
>      > ecdsa-libcrypto.c:(.text+0x148): undefined reference to
>      > `EC_GROUP_order_bits'
>      > tools/lib/ecdsa/ecdsa-libcrypto.o: In function
>      > `ecdsa_check_signature.isra.0':
>      > ecdsa-libcrypto.c:(.text+0x32c): undefined reference to
>     `ECDSA_SIG_set0'
>      > tools/lib/ecdsa/ecdsa-libcrypto.o: In function `ecdsa_sign':
>      > ecdsa-libcrypto.c:(.text+0x42c): undefined reference to
>     `ECDSA_SIG_get0'
>      > ecdsa-libcrypto.c:(.text+0x443): undefined reference to
>     `BN_bn2binpad'
>      > ecdsa-libcrypto.c:(.text+0x455): undefined reference to
>     `BN_bn2binpad'
>      > tools/lib/ecdsa/ecdsa-libcrypto.o: In function
>     `ecdsa_add_verify_data':
>      > ecdsa-libcrypto.c:(.text+0x5fa): undefined reference to
>      > `EC_GROUP_order_bits'
>      > ecdsa-libcrypto.c:(.text+0x642): undefined reference to
>      > `EC_POINT_get_affine_coordinates'
>      >
>      > Signed-off-by: Artem Panfilov <panfilov.artyom@gmail.com
>     <mailto:panfilov.artyom@gmail.com>>
>      > ---
>      >   lib/ecdsa/ecdsa-libcrypto.c | 80
>     ++++++++++++++++++++++++++++++++++++-
>      >   1 file changed, 79 insertions(+), 1 deletion(-)
>      >
>      > diff --git a/lib/ecdsa/ecdsa-libcrypto.c
>     b/lib/ecdsa/ecdsa-libcrypto.c
>      > index 1757a14562..50aa093acd 100644
>      > --- a/lib/ecdsa/ecdsa-libcrypto.c
>      > +++ b/lib/ecdsa/ecdsa-libcrypto.c
>      > @@ -24,6 +24,70 @@
>      >   #include <openssl/ec.h>
>      >   #include <openssl/bn.h>
>      >
>      > +#if OPENSSL_VERSION_NUMBER < 0x10100000L || \
>      > +     (defined(LIBRESSL_VERSION_NUMBER) &&
>     LIBRESSL_VERSION_NUMBER < 0x02070000fL)
> 
> 
>     Is there a reasonable use case for supporting an external library that
>     is more than three years old at this point?
> 
>     Otherwise NAK, as such #ifdefs don't really help with readability. I
>     think Simon will agree here.
> 
>     There's also the issue of deciding what version we have at compile
>     time,
>     which ignores the dynamic linking nature of .so libs. This leads into
>     soname versioning territory. Let's not go there.
> 
>     Alex
> 
>      > +#include <openssl/err.h>
>      > +
>      > +static int EC_GROUP_order_bits(const EC_GROUP *group)
>      > +{
>      > +     int ret = 0;
>      > +     BIGNUM *order;
>      > +
>      > +     if (!group)
>      > +             return ret;
>      > +
>      > +     order = BN_new();
>      > +
>      > +     if (!order) {
>      > +             ERR_clear_error();
>      > +             return ret;
>      > +     }
>      > +
>      > +     if (!EC_GROUP_get_order(group, order, NULL)) {
>      > +             ERR_clear_error();
>      > +             BN_free(order);
>      > +             return ret;
>      > +     }
>      > +
>      > +     ret = BN_num_bits(order);
>      > +     BN_free(order);
>      > +     return ret;
>      > +}
>      > +
>      > +static void ECDSA_SIG_get0(const ECDSA_SIG *sig, const BIGNUM
>     **pr, const BIGNUM **ps)
>      > +{
>      > +     if (pr != NULL)
>      > +             *pr = sig->r;
>      > +     if (ps != NULL)
>      > +             *ps = sig->s;
>      > +}
>      > +
>      > +static int ECDSA_SIG_set0(ECDSA_SIG *sig, BIGNUM *r, BIGNUM *s)
>      > +{
>      > +     if (r == NULL || s == NULL)
>      > +             return 0;
>      > +     BN_clear_free(sig->r);
>      > +     BN_clear_free(sig->s);
>      > +     sig->r = r;
>      > +     sig->s = s;
>      > +     return 1;
>      > +}
>      > +
>      > +int BN_bn2binpad(const BIGNUM *a, unsigned char *to, int tolen)
>      > +{
>      > +     int n = BN_num_bytes(a);
>      > +
>      > +     if (n < 0 || n > tolen)
>      > +             return -1;
>      > +
>      > +     memset(to, 0, tolen - n);
>      > +     if (BN_bn2bin(a, to + tolen - n) < 0)
>      > +             return -1;
>      > +
>      > +     return tolen;
>      > +}tures in mkimage? If not just disable TOOLS_LIBCRYPTO in your builds.
>      > +#endif
>      > +
>      >   /* Image signing context for openssl-libcrypto */
>      >   struct signer {
>      >       EVP_PKEY *evp_key;      /* Pointer to EVP_PKEY object */
>      > @@ -34,9 +98,18 @@ struct signer {
>      >
>      >   static int alloc_ctx(struct signer *ctx, const struct
>     image_sign_info *info)
>      >   {
>      > +     int ret = 0;
>      > +
>      >       memset(ctx, 0, sizeof(*ctx));
>      >
>      > -     if (!OPENSSL_init_ssl(0, NULL)) {
>      > +#if OPENSSL_VERSION_NUMBER < 0x10100000L || \
>      > +(defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER <
>     0x02070000fL)
>      > +     ret = SSL_library_init();
>      > +#else
>      > +     ret = OPENSSL_init_ssl(0, NULL);
>      > +#endif
>      > +
>      > +     if (!ret) {
>      >               fprintf(stderr, "Failure to init SSL library\n");
>      >               return -1;
>      >       }
>      > @@ -285,7 +358,12 @@ static int do_add(struct signer *ctx, void
>     *fdt, const char *key_node_name)
>      >       x = BN_new();
>      >       y = BN_new();
>      >       point = EC_KEY_get0_public_key(ctx->ecdsa_key);
>      > +#if OPENSSL_VERSION_NUMBER < 0x10100000L || \
>      > +(defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER <
>     0x02070000fL)
>      > +     EC_POINT_get_affine_coordinates_GFp(group, point, x, y, NULL);
>      > +#else
>      >       EC_POINT_get_affine_coordinates(group, point, x, y, NULL);
>      > +#endif
>      >
>      >       ret = fdt_setprop_string(fdt, key_node, "ecdsa,curve",
>     curve_name);
>      >       if (ret < 0)
>      >
>