[U-Boot] [PATCH v2 1/3] rsa: Fix build with OpenSSL 1.1.x

[U-Boot] [PATCH v2 1/3] rsa: Fix build with OpenSSL 1.1.x

[Jelle van der Waa]

The rsa_st struct has been made opaque in 1.1.x, add forward compatible
code to access the n, e, d members of rsa_struct.

EVP_MD_CTX_cleanup has been removed in 1.1.x and EVP_MD_CTX_reset should be
called to reinitialise an already created structure.

Tested-by: Peter Robinson <pbrobinson@gmail.com>
---
 lib/rsa/rsa-sign.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c
index 8c6637e328..d4e216cc0e 100644
--- a/lib/rsa/rsa-sign.c
+++ b/lib/rsa/rsa-sign.c
@@ -20,6 +20,19 @@
 #define HAVE_ERR_REMOVE_THREAD_STATE
 #endif
 
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+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();
@@ -409,7 +422,11 @@ static int rsa_sign_with_key(RSA *rsa, struct checksum_algo *checksum_algo,
 		ret = rsa_err("Could not obtain signature");
 		goto err_sign;
 	}
-	EVP_MD_CTX_cleanup(context);
+	#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+		EVP_MD_CTX_cleanup(context);
+	#else
+		EVP_MD_CTX_reset(context);
+	#endif
 	EVP_MD_CTX_destroy(context);
 	EVP_PKEY_free(key);
 
@@ -479,6 +496,7 @@ static int rsa_get_exponent(RSA *key, uint64_t *e)
 {
 	int ret;
 	BIGNUM *bn_te;
+	const BIGNUM *key_e;
 	uint64_t te;
 
 	ret = -EINVAL;
@@ -487,17 +505,18 @@ static int rsa_get_exponent(RSA *key, uint64_t *e)
 	if (!e)
 		goto cleanup;
 
-	if (BN_num_bits(key->e) > 64)
+	RSA_get0_key(key, NULL, &key_e, NULL);
+	if (BN_num_bits(key_e) > 64)
 		goto cleanup;
 
-	*e = BN_get_word(key->e);
+	*e = BN_get_word(key_e);
 
-	if (BN_num_bits(key->e) < 33) {
+	if (BN_num_bits(key_e) < 33) {
 		ret = 0;
 		goto cleanup;
 	}
 
-	bn_te = BN_dup(key->e);
+	bn_te = BN_dup(key_e);
 	if (!bn_te)
 		goto cleanup;
 
@@ -527,6 +546,7 @@ int rsa_get_params(RSA *key, uint64_t *exponent, uint32_t *n0_invp,
 {
 	BIGNUM *big1, *big2, *big32, *big2_32;
 	BIGNUM *n, *r, *r_squared, *tmp;
+	const BIGNUM *key_n;
 	BN_CTX *bn_ctx = BN_CTX_new();
 	int ret = 0;
 
@@ -548,7 +568,8 @@ int rsa_get_params(RSA *key, uint64_t *exponent, uint32_t *n0_invp,
 	if (0 != rsa_get_exponent(key, exponent))
 		ret = -1;
 
-	if (!BN_copy(n, key->n) || !BN_set_word(big1, 1L) ||
+	RSA_get0_key(key, NULL, &key_n, NULL);
+	if (!BN_copy(n, key_n) || !BN_set_word(big1, 1L) ||
 	    !BN_set_word(big2, 2L) || !BN_set_word(big32, 32L))
 		ret = -1;
 
-- 
2.12.2

[U-Boot] [PATCH v2 2/3] rsa: Fix deprecated warnings for OpenSSL 1.1.x

[Jelle van der Waa]

ERR_remove_thread_state is deprecated in OpenSSL 1.1.x and does not do
anything anymore. Thread initialisation and deinitialisation is now
handled by the OpenSSL library.

Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 lib/rsa/rsa-sign.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c
index d4e216cc0e..8a73791fed 100644
--- a/lib/rsa/rsa-sign.c
+++ b/lib/rsa/rsa-sign.c
@@ -356,9 +356,9 @@ static void rsa_remove(void)
 {
 	CRYPTO_cleanup_all_ex_data();
 	ERR_free_strings();
-#ifdef HAVE_ERR_REMOVE_THREAD_STATE
+#if OPENSSL_VERSION_NUMBER >= 0x10000000L && OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
 	ERR_remove_thread_state(NULL);
-#else
+#elif OPENSSL_VERSION_NUMBER < 0x10000000L
 	ERR_remove_state(0);
 #endif
 	EVP_cleanup();
-- 
2.12.2

[U-Boot] [PATCH v2 3/3] tools: kwbimage fix build with OpenSSL 1.1.x

[Jelle van der Waa]

The rsa_st struct has been made opaque in 1.1.x, add forward compatible
code to access the n, e, d members of rsa_struct.

EVP_MD_CTX_cleanup has been removed in 1.1.x and EVP_MD_CTX_reset should be
called to reinitialise an already created structure.

Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>
---
 tools/kwbimage.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/tools/kwbimage.c b/tools/kwbimage.c
index 2c637c7446..e07e3cc467 100644
--- a/tools/kwbimage.c
+++ b/tools/kwbimage.c
@@ -22,6 +22,25 @@
 #include <openssl/pem.h>
 #include <openssl/err.h>
 #include <openssl/evp.h>
+
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+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;
+}
+
+#else
+void EVP_MD_CTX_cleanup(EVP_MD_CTX *ctx)
+{
+	EVP_MD_CTX_reset(ctx);
+}
+#endif
 #endif
 
 static struct image_cfg_element *image_cfg;
@@ -470,12 +489,16 @@ static int kwb_export_pubkey(RSA *key, struct pubkey_der_v1 *dst, FILE *hashf,
 			     char *keyname)
 {
 	int size_exp, size_mod, size_seq;
+	const BIGNUM *key_e, *key_n;
 	uint8_t *cur;
 	char *errmsg = "Failed to encode %s\n";
 
-	if (!key || !key->e || !key->n || !dst) {
+	RSA_get0_key(key, NULL, &key_e, NULL);
+	RSA_get0_key(key, NULL, &key_n, NULL);
+
+	if (!key || !key_e || !key_n || !dst) {
 		fprintf(stderr, "export pk failed: (%p, %p, %p, %p)",
-			key, key->e, key->n, dst);
+			key, key_e, key_n, dst);
 		fprintf(stderr, errmsg, keyname);
 		return -EINVAL;
 	}
@@ -490,8 +513,8 @@ static int kwb_export_pubkey(RSA *key, struct pubkey_der_v1 *dst, FILE *hashf,
 	 * do the encoding manually.
 	 */
 
-	size_exp = BN_num_bytes(key->e);
-	size_mod = BN_num_bytes(key->n);
+	size_exp = BN_num_bytes(key_e);
+	size_mod = BN_num_bytes(key_n);
 	size_seq = 4 + size_mod + 4 + size_exp;
 
 	if (size_mod > 256) {
@@ -520,14 +543,14 @@ static int kwb_export_pubkey(RSA *key, struct pubkey_der_v1 *dst, FILE *hashf,
 	*cur++ = 0x82;
 	*cur++ = (size_mod >> 8) & 0xFF;
 	*cur++ = size_mod & 0xFF;
-	BN_bn2bin(key->n, cur);
+	BN_bn2bin(key_n, cur);
 	cur += size_mod;
 	/* Exponent */
 	*cur++ = 0x02;		/* INTEGER */
 	*cur++ = 0x82;
 	*cur++ = (size_exp >> 8) & 0xFF;
 	*cur++ = size_exp & 0xFF;
-	BN_bn2bin(key->e, cur);
+	BN_bn2bin(key_e, cur);
 
 	if (hashf) {
 		struct hash_v1 pk_hash;
-- 
2.12.2

[U-Boot] [PATCH v2 3/3] tools: kwbimage fix build with OpenSSL 1.1.x

[Mario Six]

Hi Jelle,

On Tue, Apr 4, 2017 at 11:59 PM, Jelle van der Waa <jelle@vdwaa.nl> wrote:
> @@ -22,6 +22,25 @@
>  #include <openssl/pem.h>
>  #include <openssl/err.h>
>  #include <openssl/evp.h>
> +

You also need

#include <openssl/bn.h>

here (in rsa-sign.c as well).

> +#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
> +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;
> +}

This function, as well as the copy in rsa-sign.c, should both be static,
otherwise you get multiple definition errors during the compilation of
kwbimage.c.

> @@ -470,12 +489,16 @@ static int kwb_export_pubkey(RSA *key, struct pubkey_der_v1 *dst, FILE *hashf,
>                              char *keyname)
>  {
>         int size_exp, size_mod, size_seq;
> +       const BIGNUM *key_e, *key_n;
>         uint8_t *cur;
>         char *errmsg = "Failed to encode %s\n";
>
> -       if (!key || !key->e || !key->n || !dst) {
> +       RSA_get0_key(key, NULL, &key_e, NULL);
> +       RSA_get0_key(key, NULL, &key_n, NULL);
> +
> +       if (!key || !key_e || !key_n || !dst) {
>                 fprintf(stderr, "export pk failed: (%p, %p, %p, %p)",
> -                       key, key->e, key->n, dst);
> +                       key, key_e, key_n, dst);
>                 fprintf(stderr, errmsg, keyname);
>                 return -EINVAL;
>         }

This should be

RSA_get0_key(key, &key_n, &key_e, NULL);

Otherwise, you load both key_e and key_n with the value of e (and the export
consequently contains corrupted data!).

Also, more functions were deprecated than just these; you'll need to #ifdef
those out as well (I'll comment on the other patch shortly).

Best regards,

Mario

[U-Boot] [PATCH v2 1/3] rsa: Fix build with OpenSSL 1.1.x

[Mario Six]

Hi Jelle,

On Tue, Apr 4, 2017 at 11:59 PM, Jelle van der Waa <jelle@vdwaa.nl> wrote:
> @@ -20,6 +20,19 @@
>  #define HAVE_ERR_REMOVE_THREAD_STATE
>  #endif
>
> +#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
> +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
> +

Like in the other patch, this function should be static (also, missing #include
<openssl/bn.h> in this file as well).

> @@ -548,7 +568,8 @@ int rsa_get_params(RSA *key, uint64_t *exponent, uint32_t *n0_invp,
>         if (0 != rsa_get_exponent(key, exponent))
>                 ret = -1;
>
> -       if (!BN_copy(n, key->n) || !BN_set_word(big1, 1L) ||
> +       RSA_get0_key(key, NULL, &key_n, NULL);
> +       if (!BN_copy(n, key_n) || !BN_set_word(big1, 1L) ||
>             !BN_set_word(big2, 2L) || !BN_set_word(big32, 32L))
>                 ret = -1;
>

Your're loading the parameter e into key_n here! It should be

RSA_get0_key(key, &key_n, NULL, NULL);

instead.

Like I said in the previous patch, you will have to #ifdef out more functions
in this file:

* SSL_load_error_strings
* OpenSSL_add_all_algorithms
* OpenSSL_add_all_digests
* OpenSSL_add_all_ciphers
* ENGINE_cleanup
* CRYPTO_cleanup_all_ex_data
* ERR_free_strings();
* EVP_cleanup

And you'll also have to replace SSL_library_init() with
OPENSSL_init_ssl(0, NULL).

After making all these changes, I was able to build a working U-Boot (for our
controlcenterdc board) against OpenSSL 1.1 that loaded a signed FIT-Image that
a previous U-Boot also loaded.

Best regards,

Mario

[U-Boot] [PATCH v2 1/3] rsa: Fix build with OpenSSL 1.1.x

[Peter Robinson]

On Wed, Apr 5, 2017 at 10:49 AM, Mario Six <mario.six@gdsys.cc> wrote:
> Hi Jelle,
>
> On Tue, Apr 4, 2017 at 11:59 PM, Jelle van der Waa <jelle@vdwaa.nl> wrote:
>> @@ -20,6 +20,19 @@
>>  #define HAVE_ERR_REMOVE_THREAD_STATE
>>  #endif
>>
>> +#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
>> +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
>> +
>
> Like in the other patch, this function should be static (also, missing #include
> <openssl/bn.h> in this file as well).
>
>> @@ -548,7 +568,8 @@ int rsa_get_params(RSA *key, uint64_t *exponent, uint32_t *n0_invp,
>>         if (0 != rsa_get_exponent(key, exponent))
>>                 ret = -1;
>>
>> -       if (!BN_copy(n, key->n) || !BN_set_word(big1, 1L) ||
>> +       RSA_get0_key(key, NULL, &key_n, NULL);
>> +       if (!BN_copy(n, key_n) || !BN_set_word(big1, 1L) ||
>>             !BN_set_word(big2, 2L) || !BN_set_word(big32, 32L))
>>                 ret = -1;
>>
>
> Your're loading the parameter e into key_n here! It should be
>
> RSA_get0_key(key, &key_n, NULL, NULL);
>
> instead.
>
> Like I said in the previous patch, you will have to #ifdef out more functions
> in this file:
>
> * SSL_load_error_strings
> * OpenSSL_add_all_algorithms
> * OpenSSL_add_all_digests
> * OpenSSL_add_all_ciphers
> * ENGINE_cleanup
> * CRYPTO_cleanup_all_ex_data
> * ERR_free_strings();
> * EVP_cleanup
>
> And you'll also have to replace SSL_library_init() with
> OPENSSL_init_ssl(0, NULL).
>
> After making all these changes, I was able to build a working U-Boot (for our
> controlcenterdc board) against OpenSSL 1.1 that loaded a signed FIT-Image that
> a previous U-Boot also loaded.


Jelle,

are you planning a v3 to address the above issues, it would be useful
to get this resolved for 2017.05

Peter

[U-Boot] [PATCH v2 1/3] rsa: Fix build with OpenSSL 1.1.x

[Tom Rini]

On Tue, Apr 18, 2017 at 05:58:35PM +0100, Peter Robinson wrote:
> On Wed, Apr 5, 2017 at 10:49 AM, Mario Six <mario.six@gdsys.cc> wrote:
> > Hi Jelle,
> >
> > On Tue, Apr 4, 2017 at 11:59 PM, Jelle van der Waa <jelle@vdwaa.nl> wrote:
> >> @@ -20,6 +20,19 @@
> >>  #define HAVE_ERR_REMOVE_THREAD_STATE
> >>  #endif
> >>
> >> +#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
> >> +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
> >> +
> >
> > Like in the other patch, this function should be static (also, missing #include
> > <openssl/bn.h> in this file as well).
> >
> >> @@ -548,7 +568,8 @@ int rsa_get_params(RSA *key, uint64_t *exponent, uint32_t *n0_invp,
> >>         if (0 != rsa_get_exponent(key, exponent))
> >>                 ret = -1;
> >>
> >> -       if (!BN_copy(n, key->n) || !BN_set_word(big1, 1L) ||
> >> +       RSA_get0_key(key, NULL, &key_n, NULL);
> >> +       if (!BN_copy(n, key_n) || !BN_set_word(big1, 1L) ||
> >>             !BN_set_word(big2, 2L) || !BN_set_word(big32, 32L))
> >>                 ret = -1;
> >>
> >
> > Your're loading the parameter e into key_n here! It should be
> >
> > RSA_get0_key(key, &key_n, NULL, NULL);
> >
> > instead.
> >
> > Like I said in the previous patch, you will have to #ifdef out more functions
> > in this file:
> >
> > * SSL_load_error_strings
> > * OpenSSL_add_all_algorithms
> > * OpenSSL_add_all_digests
> > * OpenSSL_add_all_ciphers
> > * ENGINE_cleanup
> > * CRYPTO_cleanup_all_ex_data
> > * ERR_free_strings();
> > * EVP_cleanup
> >
> > And you'll also have to replace SSL_library_init() with
> > OPENSSL_init_ssl(0, NULL).
> >
> > After making all these changes, I was able to build a working U-Boot (for our
> > controlcenterdc board) against OpenSSL 1.1 that loaded a signed FIT-Image that
> > a previous U-Boot also loaded.
> 
> 
> Jelle,
> 
> are you planning a v3 to address the above issues, it would be useful
> to get this resolved for 2017.05

Yes, please!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170419/975679eb/attachment.sig>