| Message ID | 20210729183121.3798261-1-mr.nuke.me@gmail.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | lib/rsa: Remove support for OpenSSL < 1.1.0 and libressl < 2.7.0 | expand |
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!
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
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 ?
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/
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
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.
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)
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(-)