[PATCH] lib/rsa: Remove support for OpenSSL < 1.1.0 and libressl < 2.7.0

[PATCH] lib/rsa: Remove support for OpenSSL < 1.1.0 and libressl < 2.7.0

[Alexandru Gagniuc]

Older OpenSSL and libressl versions have a slightly different API.
This require #ifdefs to support. However, we still can't support it
because the ECDSA path does not compile with these older versions.
These #ifdefs are truly a vestigial appendage.

Alternatively, the ECDSA path could be updated for older libraries,
but this requires significant extra code, and #ifdefs. Those libraries
are over three years old, and there concerns whether it makes sense to
build modern software for real world use against such old libraries.

Thusly, remove #ifdefs and code for old OpenSSL and LibreSSL support.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
I would appreciate if somebody tested the RSA signing functionality
with this patch applied, as I am not equipped to test this
comprehensively.

 lib/rsa/rsa-sign.c | 76 +++-------------------------------------------
 1 file changed, 4 insertions(+), 72 deletions(-)

diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c
index deff936fef..e6527c2610 100644
--- a/lib/rsa/rsa-sign.c
+++ b/lib/rsa/rsa-sign.c
@@ -19,24 +19,6 @@
 #include <openssl/evp.h>
 #include <openssl/engine.h>
 
-#if OPENSSL_VERSION_NUMBER >= 0x10000000L
-#define HAVE_ERR_REMOVE_THREAD_STATE
-#endif
-
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || \
-	(defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL)
-static void RSA_get0_key(const RSA *r,
-                 const BIGNUM **n, const BIGNUM **e, const BIGNUM **d)
-{
-   if (n != NULL)
-       *n = r->n;
-   if (e != NULL)
-       *e = r->e;
-   if (d != NULL)
-       *d = r->d;
-}
-#endif
-
 static int rsa_err(const char *msg)
 {
 	unsigned long sslErr = ERR_get_error();
@@ -314,24 +296,11 @@ static int rsa_init(void)
 {
 	int ret;
 
-#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;
 	}
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || \
-	(defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL)
-	SSL_load_error_strings();
-
-	OpenSSL_add_all_algorithms();
-	OpenSSL_add_all_digests();
-	OpenSSL_add_all_ciphers();
-#endif
 
 	return 0;
 }
@@ -346,8 +315,7 @@ static int rsa_engine_init(const char *engine_id, ENGINE **pe)
 	e = ENGINE_by_id(engine_id);
 	if (!e) {
 		fprintf(stderr, "Engine isn't available\n");
-		ret = -1;
-		goto err_engine_by_id;
+		return -1;
 	}
 
 	if (!ENGINE_init(e)) {
@@ -370,29 +338,9 @@ err_set_rsa:
 	ENGINE_finish(e);
 err_engine_init:
 	ENGINE_free(e);
-err_engine_by_id:
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || \
-	(defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL)
-	ENGINE_cleanup();
-#endif
 	return ret;
 }
 
-static void rsa_remove(void)
-{
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || \
-	(defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL)
-	CRYPTO_cleanup_all_ex_data();
-	ERR_free_strings();
-#ifdef HAVE_ERR_REMOVE_THREAD_STATE
-	ERR_remove_thread_state(NULL);
-#else
-	ERR_remove_state(0);
-#endif
-	EVP_cleanup();
-#endif
-}
-
 static void rsa_engine_remove(ENGINE *e)
 {
 	if (e) {
@@ -465,12 +413,7 @@ static int rsa_sign_with_key(EVP_PKEY *pkey, struct padding_algo *padding_algo,
 		goto err_sign;
 	}
 
-	#if OPENSSL_VERSION_NUMBER < 0x10100000L || \
-		(defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL)
-		EVP_MD_CTX_cleanup(context);
-	#else
-		EVP_MD_CTX_reset(context);
-	#endif
+	EVP_MD_CTX_reset(context);
 	EVP_MD_CTX_destroy(context);
 
 	debug("Got signature: %d bytes, expected %zu\n", *sig_size, size);
@@ -502,7 +445,7 @@ int rsa_sign(struct image_sign_info *info,
 	if (info->engine_id) {
 		ret = rsa_engine_init(info->engine_id, &e);
 		if (ret)
-			goto err_engine;
+			return ret;
 	}
 
 	ret = rsa_get_priv_key(info->keydir, info->keyname, info->keyfile,
@@ -517,7 +460,6 @@ int rsa_sign(struct image_sign_info *info,
 	EVP_PKEY_free(pkey);
 	if (info->engine_id)
 		rsa_engine_remove(e);
-	rsa_remove();
 
 	return ret;
 
@@ -526,8 +468,6 @@ err_sign:
 err_priv:
 	if (info->engine_id)
 		rsa_engine_remove(e);
-err_engine:
-	rsa_remove();
 	return ret;
 }
 
@@ -675,12 +615,8 @@ int rsa_add_verify_data(struct image_sign_info *info, void *keydest)
 	ret = rsa_get_pub_key(info->keydir, info->keyname, e, &pkey);
 	if (ret)
 		goto err_get_pub_key;
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || \
-	(defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL)
-	rsa = EVP_PKEY_get1_RSA(pkey);
-#else
+
 	rsa = EVP_PKEY_get0_RSA(pkey);
-#endif
 	ret = rsa_get_params(rsa, &exponent, &n0_inv, &modulus, &r_squared);
 	if (ret)
 		goto err_get_params;
@@ -750,10 +686,6 @@ done:
 	if (ret)
 		ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
 err_get_params:
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || \
-	(defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x02070000fL)
-	RSA_free(rsa);
-#endif
 	EVP_PKEY_free(pkey);
 err_get_pub_key:
 	if (info->engine_id)
-- 
2.31.1

Re: [PATCH] lib/rsa: Remove support for OpenSSL < 1.1.0 and libressl < 2.7.0

[Tom Rini]

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

On Thu, Jul 29, 2021 at 01:31:21PM -0500, Alexandru Gagniuc wrote:

> Older OpenSSL and libressl versions have a slightly different API.
> This require #ifdefs to support. However, we still can't support it
> because the ECDSA path does not compile with these older versions.
> These #ifdefs are truly a vestigial appendage.
> 
> Alternatively, the ECDSA path could be updated for older libraries,
> but this requires significant extra code, and #ifdefs. Those libraries
> are over three years old, and there concerns whether it makes sense to
> build modern software for real world use against such old libraries.
> 
> Thusly, remove #ifdefs and code for old OpenSSL and LibreSSL support.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

Applied to u-boot/next, thanks!

-- 
Tom

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

Re: [PATCH] lib/rsa: Remove support for OpenSSL < 1.1.0 and libressl < 2.7.0

[Peter Robinson]

On Thu, Sep 2, 2021 at 2:28 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jul 29, 2021 at 01:31:21PM -0500, Alexandru Gagniuc wrote:
>
> > Older OpenSSL and libressl versions have a slightly different API.
> > This require #ifdefs to support. However, we still can't support it
> > because the ECDSA path does not compile with these older versions.
> > These #ifdefs are truly a vestigial appendage.
> >
> > Alternatively, the ECDSA path could be updated for older libraries,
> > but this requires significant extra code, and #ifdefs. Those libraries
> > are over three years old, and there concerns whether it makes sense to
> > build modern software for real world use against such old libraries.
> >
> > Thusly, remove #ifdefs and code for old OpenSSL and LibreSSL support.
> >
> > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>
> Applied to u-boot/next, thanks!

According to recent CVE announcements 1.1.0 is out of support [1],
does it make sense to just support 1.1.1x and later?

[1] https://www.openssl.org/news/secadv/20210824.txt

Re: [PATCH] lib/rsa: Remove support for OpenSSL < 1.1.0 and libressl < 2.7.0

[Tom Rini]

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

On Thu, Sep 02, 2021 at 03:36:43PM +0100, Peter Robinson wrote:
> On Thu, Sep 2, 2021 at 2:28 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jul 29, 2021 at 01:31:21PM -0500, Alexandru Gagniuc wrote:
> >
> > > Older OpenSSL and libressl versions have a slightly different API.
> > > This require #ifdefs to support. However, we still can't support it
> > > because the ECDSA path does not compile with these older versions.
> > > These #ifdefs are truly a vestigial appendage.
> > >
> > > Alternatively, the ECDSA path could be updated for older libraries,
> > > but this requires significant extra code, and #ifdefs. Those libraries
> > > are over three years old, and there concerns whether it makes sense to
> > > build modern software for real world use against such old libraries.
> > >
> > > Thusly, remove #ifdefs and code for old OpenSSL and LibreSSL support.
> > >
> > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> >
> > Applied to u-boot/next, thanks!
> 
> According to recent CVE announcements 1.1.0 is out of support [1],
> does it make sense to just support 1.1.1x and later?
> 
> [1] https://www.openssl.org/news/secadv/20210824.txt

Good question.  Are there API changes between 1.1.0 and 1.1.1x ?

-- 
Tom

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

Re: [PATCH] lib/rsa: Remove support for OpenSSL < 1.1.0 and libressl < 2.7.0

[Peter Robinson]

On Thu, Sep 2, 2021 at 3:38 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Sep 02, 2021 at 03:36:43PM +0100, Peter Robinson wrote:
> > On Thu, Sep 2, 2021 at 2:28 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jul 29, 2021 at 01:31:21PM -0500, Alexandru Gagniuc wrote:
> > >
> > > > Older OpenSSL and libressl versions have a slightly different API.
> > > > This require #ifdefs to support. However, we still can't support it
> > > > because the ECDSA path does not compile with these older versions.
> > > > These #ifdefs are truly a vestigial appendage.
> > > >
> > > > Alternatively, the ECDSA path could be updated for older libraries,
> > > > but this requires significant extra code, and #ifdefs. Those libraries
> > > > are over three years old, and there concerns whether it makes sense to
> > > > build modern software for real world use against such old libraries.
> > > >
> > > > Thusly, remove #ifdefs and code for old OpenSSL and LibreSSL support.
> > > >
> > > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > >
> > > Applied to u-boot/next, thanks!
> >
> > According to recent CVE announcements 1.1.0 is out of support [1],
> > does it make sense to just support 1.1.1x and later?
> >
> > [1] https://www.openssl.org/news/secadv/20210824.txt
>
> Good question.  Are there API changes between 1.1.0 and 1.1.1x ?

So outside of the new TLS 1.3 feature the release says "What’s more is
that OpenSSL 1.1.1 is API and ABI compliant with OpenSSL 1.1.0" and
depending on how we use openssl it may even be API compatible with 3.0
when it comes out any time now.

https://www.openssl.org/blog/blog/2018/09/11/release111/

Re: [PATCH] lib/rsa: Remove support for OpenSSL < 1.1.0 and libressl < 2.7.0

[Alex G.]

On 9/2/21 12:43 PM, Peter Robinson wrote:
> On Thu, Sep 2, 2021 at 3:38 PM Tom Rini <trini@konsulko.com> wrote:
>>
>> On Thu, Sep 02, 2021 at 03:36:43PM +0100, Peter Robinson wrote:
>>> On Thu, Sep 2, 2021 at 2:28 PM Tom Rini <trini@konsulko.com> wrote:
>>>>
>>>> On Thu, Jul 29, 2021 at 01:31:21PM -0500, Alexandru Gagniuc wrote:
>>>>
>>>>> Older OpenSSL and libressl versions have a slightly different API.
>>>>> This require #ifdefs to support. However, we still can't support it
>>>>> because the ECDSA path does not compile with these older versions.
>>>>> These #ifdefs are truly a vestigial appendage.
>>>>>
>>>>> Alternatively, the ECDSA path could be updated for older libraries,
>>>>> but this requires significant extra code, and #ifdefs. Those libraries
>>>>> are over three years old, and there concerns whether it makes sense to
>>>>> build modern software for real world use against such old libraries.
>>>>>
>>>>> Thusly, remove #ifdefs and code for old OpenSSL and LibreSSL support.
>>>>>
>>>>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>>>>
>>>> Applied to u-boot/next, thanks!
>>>
>>> According to recent CVE announcements 1.1.0 is out of support [1],
>>> does it make sense to just support 1.1.1x and later?
>>>
>>> [1] https://www.openssl.org/news/secadv/20210824.txt
>>
>> Good question.  Are there API changes between 1.1.0 and 1.1.1x ?
> 
> So outside of the new TLS 1.3 feature the release says "What’s more is
> that OpenSSL 1.1.1 is API and ABI compliant with OpenSSL 1.1.0" and
> depending on how we use openssl it may even be API compatible with 3.0
> when it comes out any time now.
> 
> https://www.openssl.org/blog/blog/2018/09/11/release111/

Okay, I don't think it's worth excluding 1.1.0 then. The only way we 
could do that is a compile time check against OPENSSL_VERSION.

That won't prevent someone from compiling with openssl 1.1.1, and then 
just replacing libcrypto.so with 1.1.0.

Alex

Re: [PATCH] lib/rsa: Remove support for OpenSSL < 1.1.0 and libressl < 2.7.0

[Tom Rini]

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

On Thu, Sep 02, 2021 at 12:48:29PM -0500, Alex G. wrote:
> 
> 
> On 9/2/21 12:43 PM, Peter Robinson wrote:
> > On Thu, Sep 2, 2021 at 3:38 PM Tom Rini <trini@konsulko.com> wrote:
> > > 
> > > On Thu, Sep 02, 2021 at 03:36:43PM +0100, Peter Robinson wrote:
> > > > On Thu, Sep 2, 2021 at 2:28 PM Tom Rini <trini@konsulko.com> wrote:
> > > > > 
> > > > > On Thu, Jul 29, 2021 at 01:31:21PM -0500, Alexandru Gagniuc wrote:
> > > > > 
> > > > > > Older OpenSSL and libressl versions have a slightly different API.
> > > > > > This require #ifdefs to support. However, we still can't support it
> > > > > > because the ECDSA path does not compile with these older versions.
> > > > > > These #ifdefs are truly a vestigial appendage.
> > > > > > 
> > > > > > Alternatively, the ECDSA path could be updated for older libraries,
> > > > > > but this requires significant extra code, and #ifdefs. Those libraries
> > > > > > are over three years old, and there concerns whether it makes sense to
> > > > > > build modern software for real world use against such old libraries.
> > > > > > 
> > > > > > Thusly, remove #ifdefs and code for old OpenSSL and LibreSSL support.
> > > > > > 
> > > > > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > > > > 
> > > > > Applied to u-boot/next, thanks!
> > > > 
> > > > According to recent CVE announcements 1.1.0 is out of support [1],
> > > > does it make sense to just support 1.1.1x and later?
> > > > 
> > > > [1] https://www.openssl.org/news/secadv/20210824.txt
> > > 
> > > Good question.  Are there API changes between 1.1.0 and 1.1.1x ?
> > 
> > So outside of the new TLS 1.3 feature the release says "What’s more is
> > that OpenSSL 1.1.1 is API and ABI compliant with OpenSSL 1.1.0" and
> > depending on how we use openssl it may even be API compatible with 3.0
> > when it comes out any time now.
> > 
> > https://www.openssl.org/blog/blog/2018/09/11/release111/
> 
> Okay, I don't think it's worth excluding 1.1.0 then. The only way we could
> do that is a compile time check against OPENSSL_VERSION.
> 
> That won't prevent someone from compiling with openssl 1.1.1, and then just
> replacing libcrypto.so with 1.1.0.

That's what I was figuring.  If there was compatibility code we could
drop, it would make sense.  But since there's not, I don't think we're
in a position to really influence things.

-- 
Tom

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