[pulseaudio-discuss] [PATCH] raop: add compatibility with openssl 1.1.0

[Felipe Sateler]

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