On 10 September 2016 at 10:39, 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(). The function does not exist in older versions, > so a compatibility macro was added. > > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=96726 > --- > > I have tested that this builds with old and new openssl, but I have not > tested that the code works. I don't have any RAOP hardware. I cannot test either. AFAICT, the compat looks good: 1. The macro does roughly the same as the real function[1], except for freeing the previous values and some checks that don't apply here. 2. RSA_new appears to initialize n, e and d to zero., thus the freeing is not necessary. Therefore I conclude the part for compat with 1.1.0 is good. I do have some comments below though. [1] https://github.com/openssl/openssl/blob/OpenSSL_1_1_0/crypto/rsa/rsa_lib.c#L188-L212 Don't look for copying code here, as the license is not GPL-compatible AFAIK. > > > src/modules/raop/raop_client.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/src/modules/raop/raop_client.c b/src/modules/raop/raop_client.c > index 3b6c36e..88de62c 100644 > --- a/src/modules/raop/raop_client.c > +++ b/src/modules/raop/raop_client.c > @@ -68,6 +68,14 @@ > > #define RAOP_PORT 5000 > > +/* Openssl 1.1.0 broke compatibility. We could depend on openssl 1.1.0, but > + * it may take some time before distributions are able to upgrade to the new > + * openssl version. To insulate ourselves from such transition problems, let's > + * add a compatibility macro. */ > +#if OPENSSL_VERSION_NUMBER < 0x10100000L > +#define RSA_set0_key(r, n_, e_, d) (r->n = n_, r->e = e_, 1) While as noted this appears to do the right thing, why not make it a real function instead? This should make it nicer for debuggers too. > +#endif > + > struct pa_raop_client { > pa_core *core; > char *host; > @@ -161,12 +169,15 @@ static int rsa_encrypt(uint8_t *text, int len, uint8_t *res) { > uint8_t exponent[8]; > int size; > RSA *rsa; > + BIGNUM *n_bn; > + BIGNUM *e_bn; > > rsa = RSA_new(); > size = pa_base64_decode(n, modules); > - rsa->n = BN_bin2bn(modules, size, NULL); > + n_bn = BN_bin2bn(modules, size, NULL); > size = pa_base64_decode(e, exponent); > - rsa->e = BN_bin2bn(exponent, size, NULL); > + e_bn = BN_bin2bn(exponent, size, NULL); > + pa_assert(RSA_set0_key(rsa, n_bn, e_bn, NULL) == 1); Shouldn't this be pa_assert_se? > > size = RSA_public_encrypt(len, text, res, rsa, RSA_PKCS1_OAEP_PADDING); > RSA_free(rsa); -- Saludos, Felipe Sateler
