On 3 November 2016 at 09:30, Tanu Kaskinen <tanuk at iki.fi> wrote: > Openssl 1.1.0 made all structs opaque, which caused a build failure in > raop_client.c. The struct member assignments are now replaced with a > call to RSA_set0_key(). > > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=96726 > --- > > Changes in v2: > * More verbose comment. > * Implement RSA_set0_key() as a function instead of a macro. > * Use pa_assert_se() instead of pa_assert(). > > As before, I have tested that the code compiles against old and new openssl > versions, but I have not tested that the code works. > > src/modules/raop/raop_client.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/src/modules/raop/raop_client.c b/src/modules/raop/raop_client.c > index 3b6c36e..7a7bb8a 100644 > --- a/src/modules/raop/raop_client.c > +++ b/src/modules/raop/raop_client.c > @@ -68,6 +68,21 @@ > > #define RAOP_PORT 5000 > > +/* Openssl 1.1.0 broke compatibility. Before 1.1.0 we had to set RSA->n and > + * RSA->e manually, but after 1.1.0 the RSA struct is opaque and we have to use > + * RSA_set0_key(). RSA_set0_key() is a new function added in 1.1.0. We could > + * depend on openssl 1.1.0, but it may take some time before distributions will > + * be able to upgrade to the new openssl version. To insulate ourselves from > + * such transition problems, let's implement RSA_set0_key() ourselves if it's > + * not available. */ > +#if OPENSSL_VERSION_NUMBER < 0x10100000L > +static int RSA_set0_key(RSA *r, BIGNUM *n, BIGNUM *e, BIGNUM *d) { > + r->n = n; > + r->e = e; > + return 1; I'm sorry I did not point this out before. I've looked at the documentation of BN_bin2bn, and it can return NULL in case of failure. The real version of RSA_set0_key will return 0 if passed NULL 'n' and 'e', thus failing the assert. I'm not sure failing to 'BN_bin2bn' is a recoverable error so I don't think that it is bad that the assertion will fail. This new method does not fail the assert, even when it couldn't correctly set the key. This code was buggy before this change, so maybe this code could go as-is and fix this later (either by adding checks, or requiring a new-enough openssl). BTW, I'm probably going to have to backport this fix to debian, as there is a push to ship 1.1 for stretch. -- Saludos, Felipe Sateler
