Summary: | ASTERISK-28933: res_pjsip.so fails to load when bundled pjproject is compiled without libssl | ||
Reporter: | Walter Doekes (wdoekes) | Labels: | patch |
Date Opened: | 2020-06-04 10:19:57 | Date Closed: | 2020-11-09 08:52:38.000-0600 |
Priority: | Major | Regression? | No |
Status: | Closed/Complete | Components: | Resources/res_pjsip |
Versions: | 13.33.0 | Frequency of Occurrence | Constant |
Related Issues: | |||
Environment: | Attachments: | ( 0) no_libssl.patch | |
Description: | Hi!
This is really a theoretical issue. I wasn't planning on running a machine without libssl. But because this machine was so clean, I happened to notice this: If you compile libasteriskpj.so without libssl-dev, you get fewer symbols: {noformat} $ diff <(nm -D pj-13-with-ssl.so | awk '/ T /{print $3}') <(nm -D pj-13-without-ssl.so | awk '/ T /{print $3}') | awk '/^</{print $2}' pjsip_tls_setting_wipe_keys pjsip_tls_transport_lis_start pjsip_tls_transport_restart pjsip_tls_transport_start pjsip_tls_transport_start2 pj_ssl_cert_info_dump pj_ssl_cert_load_from_buffer pj_ssl_cert_load_from_files pj_ssl_cert_load_from_files2 pj_ssl_cert_wipe_keys pj_ssl_cipher_get_availables pj_ssl_cipher_id pj_ssl_cipher_is_supported pj_ssl_cipher_name pj_ssl_curve_get_availables pj_ssl_curve_id pj_ssl_curve_is_supported pj_ssl_curve_name pj_ssl_sock_close pj_ssl_sock_create pj_ssl_sock_get_info pj_ssl_sock_get_user_data pj_ssl_sock_renegotiate pj_ssl_sock_send pj_ssl_sock_sendto pj_ssl_sock_set_certificate pj_ssl_sock_set_user_data pj_ssl_sock_start_accept pj_ssl_sock_start_accept2 pj_ssl_sock_start_connect pj_ssl_sock_start_connect2 pj_ssl_sock_start_read pj_ssl_sock_start_read2 pj_ssl_sock_start_recvfrom pj_ssl_sock_start_recvfrom2 pj_turn_sock_tls_cfg_default pj_turn_sock_tls_cfg_dup pj_turn_sock_tls_cfg_wipe_keys {noformat} These are only built when: {noformat} #if defined(PJ_HAS_SSL_SOCK) && PJ_HAS_SSL_SOCK!=0 {noformat} And that is not the case when there is no libssl-dev nor libgnutls-dev. The relevant functions are (only) called here: {noformat} $ wgrep asterisk-rw-13.git/ -E '^pjsip_tls_setting_wipe_keys|pjsip_tls_transport_lis_start|pjsip_tls_transport_restart|pjsip_tls_transport_start|pjsip_tls_transport_start2|pj_ssl_cert_info_dump|pj_ssl_cert_load_from_buffer|pj_ssl_cert_load_from_files|pj_ssl_cert_load_from_files2|pj_ssl_cert_wipe_keys|pj_ssl_cipher_get_availables|pj_ssl_cipher_id|pj_ssl_cipher_is_supported|pj_ssl_cipher_name|pj_ssl_curve_get_availables|pj_ssl_curve_id|pj_ssl_curve_is_supported|pj_ssl_curve_name|pj_ssl_sock_close|pj_ssl_sock_create|pj_ssl_sock_get_info|pj_ssl_sock_get_user_data|pj_ssl_sock_renegotiate|pj_ssl_sock_send|pj_ssl_sock_sendto|pj_ssl_sock_set_certificate|pj_ssl_sock_set_user_data|pj_ssl_sock_start_accept|pj_ssl_sock_start_accept2|pj_ssl_sock_start_connect|pj_ssl_sock_start_connect2|pj_ssl_sock_start_read|pj_ssl_sock_start_read2|pj_ssl_sock_start_recvfrom|pj_ssl_sock_start_recvfrom2|pj_turn_sock_tls_cfg_default|pj_turn_sock_tls_cfg_dup|pj_turn_sock_tls_cfg_wipe_keys$' | grep -vF /third-party/ asterisk-rw-13.git/res/res_pjsip/config_transport.c: res = pjsip_tls_transport_start2(ast_sip_get_pjsip_endpoint(), &temp_state->state->tls, asterisk-rw-13.git/res/res_pjsip/config_transport.c: if (pj_ssl_cipher_get_availables(ciphers, &cipher_num)) { asterisk-rw-13.git/res/res_pjsip/config_transport.c: const char *pos_name = pj_ssl_cipher_name(ciphers[pos]); asterisk-rw-13.git/res/res_pjsip/config_transport.c: if (pj_ssl_cipher_is_supported(cipher)) { asterisk-rw-13.git/res/res_pjsip/config_transport.c: ast_str_append(&str, 0, "%s", pj_ssl_cipher_name(ciphers[idx])); asterisk-rw-13.git/res/res_pjsip/config_transport.c: if (pj_ssl_cipher_get_availables(ciphers, &cipher_num) || !cipher_num) { {noformat} That is, only {{res/res_pjsip/config_transport.c}} and only: {noformat} pjsip_tls_transport_start2 pj_ssl_cipher_get_availables pj_ssl_cipher_name pj_ssl_cipher_is_supported {noformat} And could be fixed with something like: {noformat} diff --git a/res/res_pjsip/config_transport.c b/res/res_pjsip/config_transport.c index d2993401fc..6596a87643 100644 --- a/res/res_pjsip/config_transport.c +++ b/res/res_pjsip/config_transport.c @@ -618,6 +618,7 @@ static int transport_apply(const struct ast_sorcery *sorcery, void *obj) res = pjsip_tcp_transport_start3(ast_sip_get_pjsip_endpoint(), &cfg, &temp_state->state->factory); } +#ifdef HAVE_OPENSSL } else if (transport->type == AST_TRANSPORT_TLS) { static int option = 1; @@ -648,6 +649,7 @@ static int transport_apply(const struct ast_sorcery *sorcery, void *obj) &temp_state->state->host, NULL, transport->async_operations, &temp_state->state->factory); } +#endif } else if ((transport->type == AST_TRANSPORT_WS) || (transport->type == AST_TRANSPORT_WSS)) { if (transport->cos || transport->tos) { ast_log(LOG_WARNING, "TOS and COS values ignored for websocket transport\n"); @@ -977,6 +979,7 @@ static int tls_method_to_str(const void *obj, const intptr_t *args, char **buf) return 0; } +#ifdef HAVE_OPENSSL /*! \brief Helper function which turns a cipher name into an identifier */ static pj_ssl_cipher cipher_name_to_id(const char *name) { @@ -997,6 +1000,7 @@ static pj_ssl_cipher cipher_name_to_id(const char *name) return 0; } +#endif /*! * \internal @@ -1010,6 +1014,7 @@ static pj_ssl_cipher cipher_name_to_id(const char *name) */ static int transport_cipher_add(struct ast_sip_transport_state *state, const char *name) { +#ifdef HAVE_OPENSSL pj_ssl_cipher cipher; int idx; @@ -1033,10 +1038,10 @@ static int transport_cipher_add(struct ast_sip_transport_state *state, const cha } state->ciphers[state->tls.ciphers_num++] = cipher; return 0; - } else { + } +#endif ast_log(LOG_ERROR, "Cipher '%s' is unsupported\n", name); return -1; - } } /*! \brief Custom handler for TLS cipher setting */ @@ -1079,7 +1084,13 @@ static void cipher_to_str(char **buf, const pj_ssl_cipher *ciphers, unsigned int } for (idx = 0; idx < cipher_num; ++idx) { - ast_str_append(&str, 0, "%s", pj_ssl_cipher_name(ciphers[idx])); + ast_str_append(&str, 0, "%s", +#ifdef HAVE_OPENSSL + pj_ssl_cipher_name(ciphers[idx]) +#else + "<OPENSSL_MISSING>" +#endif + ); if (idx < cipher_num - 1) { ast_str_append(&str, 0, ", "); } @@ -1118,7 +1129,11 @@ static char *handle_pjsip_list_ciphers(struct ast_cli_entry *e, int cmd, struct return NULL; } - if (pj_ssl_cipher_get_availables(ciphers, &cipher_num) || !cipher_num) { + if ( +#ifdef HAVE_OPENSSL + pj_ssl_cipher_get_availables(ciphers, &cipher_num) || +#endif + !cipher_num) { buf = NULL; } else { cipher_to_str(&buf, ciphers, cipher_num); {noformat} (Although that would break the possibility for someone to use gnutls; if that works, which I'm not sure does.) In any case, without the above patch, res_pjsip.so fails to load because of the missing symbols. So either we should mandate libssl-dev (or libgnutls-dev?) or apply something like above. | ||
Comments: | By: Asterisk Team (asteriskteam) 2020-06-04 10:19:58.904-0500 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. Please note that once your issue enters an open state it has been accepted. As Asterisk is an open source project there is no guarantee or timeframe on when your issue will be looked into. If you need expedient resolution you will need to find and pay a suitable developer. Asking for an update on your issue will not yield any progress on it and will not result in a response. All updates are posted to the issue when they occur. By: George Joseph (gjoseph) 2020-06-04 12:30:18.274-0500 what happens if you force --disable-asteriskssl --without-ssl? By: Walter Doekes (wdoekes) 2020-06-04 15:48:45.269-0500 Nope. Does not help. {noformat} $ grep -i ssl config.log | LC_ALL=C sort -u $ ./configure --enable-dev-mode --disable-asteriskssl --without-ssl #define HAVE_PJ_SSL_CERT_LOAD_FROM_FILES2 1 AST_ASTERISKSSL='no' OPENSSL='/usr/bin/openssl' OPENSSL_BIO_METHOD_DIR='' OPENSSL_BIO_METHOD_INCLUDE='' OPENSSL_BIO_METHOD_LIB='' OPENSSL_DIR='' OPENSSL_INCLUDE='' OPENSSL_LIB='' OPENSSL_SRTP_DIR='' OPENSSL_SRTP_INCLUDE='' OPENSSL_SRTP_LIB='' PBX_OPENSSL='-1' PBX_OPENSSL_BIO_METHOD='0' PBX_OPENSSL_SRTP='0' PBX_PJ_SSL_CERT_LOAD_FROM_FILES2='' PJPROJECT_CONFIGURE_OPTS=' --disable-ssl' PJ_SSL_CERT_LOAD_FROM_FILES2_DIR='' PJ_SSL_CERT_LOAD_FROM_FILES2_INCLUDE='' PJ_SSL_CERT_LOAD_FROM_FILES2_LIB='' ac_cv_path_OPENSSL=/usr/bin/openssl configure:21518: gcc -o conftest -g -O2 -DUSE_SYSTEM_IMAP conftest.c -lm -lcrypto -lssl -lc-client >&5 configure:21663: gcc -o conftest -g -O2 -DUSE_SYSTEM_CCLIENT conftest.c -lm -lcrypto -lssl -lc-client >&5 configure:21734: gcc -o conftest -g -O2 -DUSE_SYSTEM_CCLIENT conftest.c -lm -lcrypto -lssl -lc-client4 >&5 configure:7779: checking for openssl configure:7797: found /usr/bin/openssl configure:7810: result: /usr/bin/openssl | #define HAVE_PJ_SSL_CERT_LOAD_FROM_FILES2 1 {noformat} {noformat} $ grep -i tls config.log | LC_ALL=C sort -u #define HAVE_PJSIP_TLS_TRANSPORT_PROTO 1 PBX_PJSIP_TLS_TRANSPORT_PROTO='' PJSIP_TLS_TRANSPORT_PROTO_DIR='' PJSIP_TLS_TRANSPORT_PROTO_INCLUDE='' PJSIP_TLS_TRANSPORT_PROTO_LIB='' | #define HAVE_PJSIP_TLS_TRANSPORT_PROTO 1 {noformat} {noformat} $ nm res/res_pjsip.so | grep ssl U pj_ssl_cipher_get_availables U pj_ssl_cipher_is_supported U pj_ssl_cipher_name {noformat} {noformat} $ git branch * 16 $ git describe --always 6a0c472374 {noformat} By: Alexander Traud (traud) 2020-10-30 12:12:42.210-0500 Thanks for reporting and the detail analysis. I attached and submitted a patch which is based on your proposal and fixed the issue for me, too. Let us see whether it gets accepted. Nevertheless, I wonder how that slipped through. Three years ago, ASTERISK-27431 should have fixed this. However, that was just at compile/build-time. Nobody, including me, has checked the load/run-time of chan_pjsip without OpenSSL. And yes, I only used chan_sip in that case. Go figure! By: Friendly Automation (friendly-automation) 2020-11-09 08:52:40.724-0600 Change 15117 merged by Friendly Automation: res_pjsip/config_transport: Load and run without OpenSSL. [https://gerrit.asterisk.org/c/asterisk/+/15117|https://gerrit.asterisk.org/c/asterisk/+/15117] By: Friendly Automation (friendly-automation) 2020-11-09 08:52:59.809-0600 Change 15118 merged by Friendly Automation: res_pjsip/config_transport: Load and run without OpenSSL. [https://gerrit.asterisk.org/c/asterisk/+/15118|https://gerrit.asterisk.org/c/asterisk/+/15118] By: Friendly Automation (friendly-automation) 2020-11-09 08:56:31.402-0600 Change 15125 merged by George Joseph: res_pjsip/config_transport: Load and run without OpenSSL. [https://gerrit.asterisk.org/c/asterisk/+/15125|https://gerrit.asterisk.org/c/asterisk/+/15125] |