philippe44 / AirConnect

Use AirPlay to stream to UPnP/Sonos & Chromecast devices
Other
3.6k stars 219 forks source link

Can't build airupnp-arm since introduction of OpenSSL 1.1.x support #332

Closed pwt closed 3 years ago

pwt commented 3 years ago

Since v0.2.44.0 (commit https://github.com/philippe44/AirConnect/commit/af42b53b4ee3fa29f9a0f0520b4f672fc93cb5e8), I can no longer build airupnp-arm against either OpenSSL 1.0.x or OpenSSL 1.1.x.

I'm just wondering if you have any hints for how to get my builds working again? The linker error is:

/usr/lib/gcc/arm-linux-gnueabihf/6/../../../arm-linux-gnueabihf/libcrypto.a(rsa_lib.o): In function `RSA_set0_key':
(.text+0x400): multiple definition of `RSA_set0_key'
bin/arm/sslsym-static.o:/home/pwt/AirConnect-Build/AirConnect/airupnp/../tools/sslsym.c:101: first defined here

The system installed version of OpenSSL is 1.1.01.

I also have a local installation of OpenSSL 1.0.2t, which is what I was previously building against. This no longer works either.

(I create my own builds for use in https://github.com/pwt/docker-airconnect-arm. Specifically, I want a version that suppresses FLUSH commands, to circumvent the issues with Apple Music / iTunes and Sonos speakers.)

philippe44 commented 3 years ago

It's around OPENSSL_VERSION_NUMBER check that is in sslsym.c. The function RSA_set0_key has been introduced in open SSL 1.1.x and must be used because a structure is now opaque (it was direct access before). Maybe I did something wrong, but are you sure of your definition of OPENSSL_VERSION_NUMBER?

Re the FLUSH problem, maybe to help you in the future I can build that as a runtime option

pwt commented 3 years ago

Thanks for your help, @philippe44.


I'm not doing anything to define OPENSSL_VERSION_NUMBER. Looks like that should come from openssl/opensslv.h via openssl/crypto.h and openssl/rsa.h. It's set to the following on my installation:

# define OPENSSL_VERSION_NUMBER 0x101000cfL

Having cleaned everything up, the first error I'm seeing is actually in the compilation of sslsym.c; see https://gist.github.com/pwt/e04401a248ef99fc96e3114a52716966.


It would certainly be nice to have a native build with a runtime option to suppress FLUSH commands. The idea for this emerged in the discussion at https://github.com/philippe44/AirConnect/issues/234#issuecomment-634904435 . The builds I create simply omit the FLUSH instructions as described in the issue. This works reliably for Sonos (tested quite extensively now), and corrects the problems with changing track, etc. I can't comment on its use with non-Sonos equipment.

philippe44 commented 3 years ago

Can you tell me exactly where you put the mod? Re SSL, I have to check how #if OPENSSL_VERSION_NUMBER < 0x10100000 works with 0x101000cfL... not well I guess, I have to check if pre-processor is fine with the L for the long long Can you put a #error

// create shim functions
#if OPENSSL_VERSION_NUMBER < 0x10100000
static int shim_RSA_set0_key(RSA *r, BIGNUM *n, BIGNUM *e, BIGNUM *d) {
    r->n = n; r->e = e; r->d = d;
    return 1;
}
SYMSHIMDECL(RSA_set0_key, int, 4, RSA*, r, BIGNUM*, n, BIGNUM*, e, BIGNUM*, d);
#else
SYMDECL(RSA_set0_key, int, 4, RSA*, r, BIGNUM*, n, BIGNUM*, e, BIGNUM*, d);
#endif

in sslsym.c to see what part of code is compiled out?

pwt commented 3 years ago

Can you tell me exactly where you put the mod?

Here's the fragment of raopcore.c that I remove completely. (I've used a screenshot of the file from GitHub to make sure we have a consistent reference for the line numbers, since the file has a mixture of line endings.)

screenshot_225

Re SSL, I have to check how #if OPENSSL_VERSION_NUMBER < 0x10100000 works with 0x101000cfL... not well I guess, I have to check if pre-processor is fine with the L for the long long

I created a short test program earlier, and the version check you're using seemed to work fine with the OpenSSL version on my system:

#include <stdio.h>
#include <openssl/rsa.h>

#if OPENSSL_VERSION_NUMBER < 0x10100000
char* msg = "Less than\n";
#else
char* msg = "More than\n";
#endif

int main(){
    printf(msg);
    return 0;
}

Prints More than.

philippe44 commented 3 years ago

Then there is something weird in your build. As you can see, the RSA_set0_key is simply not existing when "More than" (SYMDECL defines to nothing), so it cannot be conflicting with the openssl-defined then.

pwt commented 3 years ago

Then there is something weird in your build. As you can see, the RSA_set0_key is simply not existing when "More than" (SYMDECL defines to nothing), so it cannot be conflicting with the openssl-defined then.

Ack. I'll do some further investigation.

philippe44 commented 3 years ago

BTW, I just realized that skip on upnp was broken since 0.2.27. I don't understand how nobody complained about it

pwt commented 3 years ago

Just to report back: I've been trying to narrow down the build problem. I've pared back the compilation just to the files required to compile sslsym.c

pwt@ubuntu-vm:~/tmp/AirConnect-Build-Problem$ ls -l
total 28
-rw-r--r-- 1 pwt pwt 3942 Mar 11 11:06 platform.h
-rw-r--r-- 1 pwt pwt  942 Mar 11 11:06 sslshim.h
-rw-r--r-- 1 pwt pwt 9213 Mar 11 11:06 sslsym.c
-rw-r--r-- 1 pwt pwt  849 Mar 11 11:06 sslsym.h
drwxr-xr-x 3 pwt pwt 4096 Mar 11 11:07 valgrind

I'm getting the same errors on three different Linux platforms as well as on macOS:

pwt@ubuntu-vm:~/tmp/AirConnect-Build-Problem$ gcc -I /usr/include/valgrind sslsym.c
sslsym.c:151:9: error: redefinition of ‘TLS_client_method’
  151 | SYMDECL(TLS_client_method, const SSL_METHOD*, 0);
      |         ^~~~~~~~~~~~~~~~~
sslsym.c:66:5: note: in definition of macro ‘SYMDECL’
   66 | ret fn(P(n,__VA_ARGS__)) {      \
      |     ^~
sslsym.c:150:9: note: previous definition of ‘TLS_client_method’ was here
  150 | SYMDECL(SSLv23_client_method, const SSL_METHOD*, 0);
      |         ^~~~~~~~~~~~~~~~~~~~
sslsym.c:66:5: note: in definition of macro ‘SYMDECL’
   66 | ret fn(P(n,__VA_ARGS__)) {      \
      |     ^~
sslsym.c:153:1: error: macro "SSL_library_init" passed 1 arguments, but takes just 0
  153 | SYMDECL(SSL_library_init, int, 0);
      | ^~
In file included from sslsym.c:32:
/usr/include/openssl/ssl.h:1958: note: macro "SSL_library_init" defined here
 1958 | #  define SSL_library_init() OPENSSL_init_ssl(0, NULL)
      | 
sslsym.c:66:26: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘{’ token
   66 | ret fn(P(n,__VA_ARGS__)) {      \
      |                          ^
sslsym.c:153:1: note: in expansion of macro ‘SYMDECL’
  153 | SYMDECL(SSL_library_init, int, 0);
      | ^~~~~~~
sslsym.c: In function ‘load_ssl_symbols’:
sslsym.c:146:47: error: ‘shim_RSA_set0_key’ undeclared (first use in this function); did you mean ‘dlsym_RSA_set0_key’?
  146 | #define SHIMSET(fn) if (!SYM(fn)) SYM(fn) = &(shim_##fn)
      |                                               ^~~~~
sslsym.c:278:2: note: in expansion of macro ‘SHIMSET’
  278 |  SHIMSET(RSA_set0_key);
      |  ^~~~~~~
sslsym.c:146:47: note: each undeclared identifier is reported only once for each function it appears in
  146 | #define SHIMSET(fn) if (!SYM(fn)) SYM(fn) = &(shim_##fn)
      |                                               ^~~~~
sslsym.c:278:2: note: in expansion of macro ‘SHIMSET’
  278 |  SHIMSET(RSA_set0_key);
      |  ^~~~~~~

If I run just the preprocessor using cpp -I /usr/include/valgrind sslsym.c I get the output shown at: https://gist.github.com/pwt/35786f0ace477e24c6700570ab760b0b. Note the macro error at line 15035, and the TLS_client_method line at 15030.

What platform(s) are you building on?

philippe44 commented 3 years ago

I'm building on a Linux x86 (ancient Ubuntu 14.10). It seems that on your system, the SSL_library_init is a macro, not a function

static const SSL_METHOD* (*dlsym_TLS_client_method)(void); const SSL_METHOD* TLS_client_method(void) { return (*dlsym_TLS_client_method)(); };
static unsigned long (*dlsym_OpenSSL_version_num)(void); unsigned long OpenSSL_version_num(void) { return (*dlsym_OpenSSL_version_num)(); };
sslsym.c:153:1: error: macro "SSL_library_init" passed 1 arguments, but takes just 0
 SYMDECL(SSL_library_init, int, 0);
 ^~
static int (*dlsym_SSL_library_init)(void); int SSL_library_init { return (*dlsym_SSL_library_init)(); };

That does not expand correctly. See previous function that expand as a function that takes a void

On another note, could you have a look at stock 0.2.50.1? I think I have a proper solution for the "flush" problem

pwt commented 3 years ago

On another note, could you have a look at stock 0.2.50.1? I think I have a proper solution for the "flush" problem.

Thanks @philippe44: airupnp-arm v0.2.50.1 is working perfectly in my testing with Sonos speakers. Using --noflush I can skip smoothly between tracks, etc., in all the audio apps I've tested including the Apple native ones, playing from iOS and macOS. Great work.

I'd still like to get my local builds working for other reasons, but with this change I can switch back to using only your binaries for inclusion in docker-airconnect-arm.

pwt commented 3 years ago

I'm building on a Linux x86 (ancient Ubuntu 14.10). It seems that on your system, the SSL_library_init is a macro, not a function

static const SSL_METHOD* (*dlsym_TLS_client_method)(void); const SSL_METHOD* TLS_client_method(void) { return (*dlsym_TLS_client_method)(); };
static unsigned long (*dlsym_OpenSSL_version_num)(void); unsigned long OpenSSL_version_num(void) { return (*dlsym_OpenSSL_version_num)(); };
sslsym.c:153:1: error: macro "SSL_library_init" passed 1 arguments, but takes just 0
 SYMDECL(SSL_library_init, int, 0);
 ^~
static int (*dlsym_SSL_library_init)(void); int SSL_library_init { return (*dlsym_SSL_library_init)(); };

That does not expand correctly. See previous function that expand as a function that takes a void

Looks like there are multiple issues of this type, introduced by macro definitions of synonymous functions such as:

/usr/include/openssl/ssl.h:#  define OpenSSL_add_ssl_algorithms()   SSL_library_init()
/usr/include/openssl/ssl.h:#define SSLv23_client_method    TLS_client_method

Given that I have a vanilla OpenSSL development installation (libssl-dev 1.1.0l-1~deb9u3), I guess I don't understand how your builds are working, even on an older Ubuntu system :)

In the meantime, I have builds working again by holding back sslsym.c, and continuing to use OpenSSL 1.0.x.

philippe44 commented 3 years ago

I have to review my macro definition which is a topic I've always disliked. In sslsym.c it's a complicated set of macros using concatenation, stringification and variadic. I probably need to revise the topic of prescan when what I'm replacing is a macro itself. What is happening is that 1.1 has deprecated a few functions and replaced them by macros. I'll see if I can find a general solution of if I have to put all that under the SSL version #define

philippe44 commented 3 years ago

Let me know if 0.2.50.2 compiles

pwt commented 3 years ago

Let me know if 0.2.50.2 compiles

Great -- yes, this works perfectly. Thanks for working on it.