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

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

[Artem Panfilov]

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>
---
 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)
+#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;
+}
+#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)
-- 
2.25.1

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

[Alex G.]

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>
> ---
>   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;
> +}
> +#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)
> 

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

[Alex G.]

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)
>      >
> 

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]

On Wed, Jul 28, 2021 at 03:00:44PM -0500, Alex G. wrote:
> 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 a fine line at least that I'm willing to walk in terms of
supporting ancient OSes directly and also not making things overly
complicated in our own tree.  That said, openssl tends to be one of the
ones where it does get hard to support old versions.  LibreSSL 2.7.5 was
released December 15th, 2018 and is the end of the 2.7.x line it seems.

I'm interested to hear what the case is where the right call is the say
you're building modern software for real world use against such old
libraries.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Artem Panfilov]

On 28.07.2021 23:07, Tom Rini wrote:
> There is a fine line at least that I'm willing to walk in terms of
> supporting ancient OSes directly and also not making things overly
> complicated in our own tree.  That said, openssl tends to be one of the
> ones where it does get hard to support old versions.  LibreSSL 2.7.5 was
> released December 15th, 2018 and is the end of the 2.7.x line it seems.
>
> I'm interested to hear what the case is where the right call is the say
> you're building modern software for real world use against such old
> libraries.
Hi Tom,
I have a specific test case where I test if it's still possible to
build upstream master u-boot on our infrastructure (progression testing).
I also test runtime on our boards.

I understand that nowadays everyone uses docker container,
but ​we have limited docker nodes right now on our site.

If you don't want to support OpenSSL < 1.1.0 and do not test it,
then I suggest dropping it all over the tree because it doesn't
make sense and looks misleading with such a partial solution.

Best regards,
Artem

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]

On Thu, Jul 29, 2021 at 01:29:35AM +0300, Artem Panfilov wrote:
> 
> On 28.07.2021 23:07, Tom Rini wrote:
> > There is a fine line at least that I'm willing to walk in terms of
> > supporting ancient OSes directly and also not making things overly
> > complicated in our own tree.  That said, openssl tends to be one of the
> > ones where it does get hard to support old versions.  LibreSSL 2.7.5 was
> > released December 15th, 2018 and is the end of the 2.7.x line it seems.
> >
> > I'm interested to hear what the case is where the right call is the say
> > you're building modern software for real world use against such old
> > libraries.
> Hi Tom,
> I have a specific test case where I test if it's still possible to
> build upstream master u-boot on our infrastructure (progression testing).
> I also test runtime on our boards.
> 
> I understand that nowadays everyone uses docker container,
> but ​we have limited docker nodes right now on our site.
> 
> If you don't want to support OpenSSL < 1.1.0 and do not test it,
> then I suggest dropping it all over the tree because it doesn't
> make sense and looks misleading with such a partial solution.

Part of the question is then, were you enabling the SSL-related parts
before this change?  Or did the way the code is now being
enabled/disabled trigger this now being enabled when it wasn't before?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Artem Panfilov]

On 29.07.2021 01:56, Tom Rini wrote:
> Part of the question is then, were you enabling the SSL-related parts
> before this change?  Or did the way the code is now being
> enabled/disabled trigger this now being enabled when it wasn't before?
>
This commit broke the build:
https://gitlab.com/u-boot/u-boot/-/commit/cb9faa6f98ae56d70d59505dad290dd3d381cb7b

Our testing board defconfig:
https://gitlab.com/u-boot/u-boot/-/blob/master/configs/hsdk_defconfig

I also found this patch that was committed 4 months ago.
It includes OpenSSL compatibility for old versions:
https://gitlab.com/u-boot/u-boot/-/commit/fbc777429fa35312a9ea5f106692172d3153e659

---
Artem

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

On Thu, Jul 29, 2021 at 02:37:10AM +0300, Artem Panfilov wrote:
> 
> On 29.07.2021 01:56, Tom Rini wrote:
> > Part of the question is then, were you enabling the SSL-related parts
> > before this change?  Or did the way the code is now being
> > enabled/disabled trigger this now being enabled when it wasn't before?
> >
> This commit broke the build:
> https://gitlab.com/u-boot/u-boot/-/commit/cb9faa6f98ae56d70d59505dad290dd3d381cb7b

OK.  How?  Or rather, what part of that is causing previously unbuilt
code to now be built?

> Our testing board defconfig:
> https://gitlab.com/u-boot/u-boot/-/blob/master/configs/hsdk_defconfig

OK.

> I also found this patch that was committed 4 months ago.
> It includes OpenSSL compatibility for old versions:
> https://gitlab.com/u-boot/u-boot/-/commit/fbc777429fa35312a9ea5f106692172d3153e659

Yes, true.  And that's two 1-line if/else.  That's a reasonable to me
level of effort to keep supporting older hosts.  Your patch is adding in
60 lines.  I really do want to dig a bit more here.

And honestly, part of my concerns also go around "who is going to
maintain / test this area?".  We don't have these older versions in CI
(or we would have seen the problem before merging).  Are you
volunteering to support the relevant code areas here but on older
openssl/libressl ?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Artem Panfilov]

On 29.07.2021 02:43, Tom Rini wrote:
>
> Yes, true.  And that's two 1-line if/else.  That's a reasonable to me
> level of effort to keep supporting older hosts.  Your patch is adding in
> 60 lines.  I really do want to dig a bit more here.

For me, it doesn't matter how many lines of code were added if I can't
build host tools with older OpenSSL versions. So what's the point
of keeping OpenSSL backward compatibility?

> And honestly, part of my concerns also go around "who is going to
> maintain / test this area?".  We don't have these older versions in CI
> (or we would have seen the problem before merging).  Are you
> volunteering to support the relevant code areas here but on older
> openssl/libressl ?

We already have a nightly Jenkins CI job that tracks u-boot master
and sends internal reports.

The best way would be to testing in the upstream azure pipeline.

You could add the following steps in your trini/u-boot-gitlab-ci-runner docker image:
wget -O - https://www.openssl.org/source/old/1.0.2/openssl-1.0.2k.tar.gz | tar -C /opt -xz && \
cd /opt/openssl-1.0.2k/ && ./config shared && \
make && \
make install

In azure pipeline add new job for testing with old OpenSSL:
make tools-only_config tools-only NO_SDL=1 \
HOSTLDFLAGS="-ldl -L/usr/local/ssl/lib" \
HOSTCFLAGS="-I/usr/local/ssl/include"

---
Artem

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1872 bytes --]

On Thu, Jul 29, 2021 at 01:40:41PM +0300, Artem Panfilov wrote:
> On 29.07.2021 02:43, Tom Rini wrote:
> >
> > Yes, true.  And that's two 1-line if/else.  That's a reasonable to me
> > level of effort to keep supporting older hosts.  Your patch is adding in
> > 60 lines.  I really do want to dig a bit more here.
> 
> For me, it doesn't matter how many lines of code were added if I can't
> build host tools with older OpenSSL versions. So what's the point
> of keeping OpenSSL backward compatibility?

Well yes, this is part of the question now, is there enough interest in
the old version to bother with?  The other part of the question is
what's being built now that wasn't being built before, and is that a bug
or a feature (a less CONFIG-dependent set of tools is good for generic
distributions).

> > And honestly, part of my concerns also go around "who is going to
> > maintain / test this area?".  We don't have these older versions in CI
> > (or we would have seen the problem before merging).  Are you
> > volunteering to support the relevant code areas here but on older
> > openssl/libressl ?
> 
> We already have a nightly Jenkins CI job that tracks u-boot master
> and sends internal reports.
> 
> The best way would be to testing in the upstream azure pipeline.
> 
> You could add the following steps in your trini/u-boot-gitlab-ci-runner docker image:
> wget -O - https://www.openssl.org/source/old/1.0.2/openssl-1.0.2k.tar.gz | tar -C /opt -xz && \
> cd /opt/openssl-1.0.2k/ && ./config shared && \
> make && \
> make install
> 
> In azure pipeline add new job for testing with old OpenSSL:
> make tools-only_config tools-only NO_SDL=1 \
> HOSTLDFLAGS="-ldl -L/usr/local/ssl/lib" \
> HOSTCFLAGS="-I/usr/local/ssl/include"

And the next time something breaks, are you going to be available to fix
it?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Artem Panfilov]

On 29.07.2021 15:59, Tom Rini wrote:
> Well yes, this is part of the question now, is there enough interest in
> the old version to bother with?  The other part of the question is
> what's being built now that wasn't being built before, and is that a bug
> or a feature (a less CONFIG-dependent set of tools is good for generic
> distributions).

OK, if someone else will report the same issue after u-boot release,
then it should be fixed. Currently, I am okay with my local fix
by disabling the CONFIG_TOOLS_LIBCRYPTO option.

---
Artem

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

[Alex G.]

Hi Artem

On 7/29/21 9:52 AM, Artem Panfilov wrote:
> On 29.07.2021 15:59, Tom Rini wrote:
>> Well yes, this is part of the question now, is there enough interest in
>> the old version to bother with?  The other part of the question is
>> what's being built now that wasn't being built before, and is that a bug
>> or a feature (a less CONFIG-dependent set of tools is good for generic
>> distributions).
> 
> OK, if someone else will report the same issue after u-boot release,
> then it should be fixed. Currently, I am okay with my local fix
> by disabling the CONFIG_TOOLS_LIBCRYPTO option.

ECDSA signing was not verified against a libcrypto that old. Given that 
signatures are non-deterministic, I doubt we could have a CI test that 
says old-libcrypto, known block must equal known signature.

When we added ECDSA, there was not a need to consider old libcrypto 
versions, but I also did not pay attention to the #ifdefs in the much 
older RSA path. I'm sorry that you had to go through the frustrations of 
getting a patch rejected which does something the codebase already does.

I am going to take a look at cleaning up the RSA path. There's no point 
in maintaining backwards compatibility if we're not doing it across the 
board.

Alex

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

[Simon Glass]

Hi,

On Wed, 28 Jul 2021 at 23:13, Jonathan Gray <jsg@jsg.id.au> wrote:
>
> On Wed, Jul 28, 2021 at 09:04:33PM +0300, Artem Panfilov wrote:
> > Fix LibreSSL compilation for versions before v2.7.0.
>
> Why 2.7.0?  I had to disable CONFIG_FIT_SIGNATURE to get the qemu
> targets to build on OpenBSD-current (3.4.0) as there is no
> BN_bn2binpad().  2.7.0 is also over three years old at this point.

I think it is reasonable to support older libraries but I'm not sure
what is reasonable. I am sure there are people using 3 or 4 year-old
distributions which might have older libraries?

Assuming that is true:

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> >
> > 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>
> > ---
> >  lib/ecdsa/ecdsa-libcrypto.c | 80 ++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 79 insertions(+), 1 deletion(-)
> >

[..]

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

[Jonathan Gray]

On Wed, Jul 28, 2021 at 09:04:33PM +0300, Artem Panfilov wrote:
> Fix LibreSSL compilation for versions before v2.7.0.

Why 2.7.0?  I had to disable CONFIG_FIT_SIGNATURE to get the qemu
targets to build on OpenBSD-current (3.4.0) as there is no
BN_bn2binpad().  2.7.0 is also over three years old at this point.

> 
> 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>
> ---
>  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)
> +#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;
> +}
> +#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)
> -- 
> 2.25.1
> 
> 

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

[Artem Panfilov]

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>
---
 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)
+#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;
+}
+#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)
-- 
2.25.1