From: Abhishek Lekshmanan Date: Fri, 2 Mar 2018 16:41:42 +0000 (+0100) Subject: rgw: setup locks for libopenssl X-Git-Tag: v12.2.5~67^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=945b5b3980e8822398844e9a6460db39efea7e44;p=ceph.git rgw: setup locks for libopenssl openssl <= 1.02 requires explicit callbacks for locking which libcurl doesn't set. This causes random segmentation faults when openssl uses its global structures across multiple threads. Providing a simple mutex lock/unlock functions as a callback. We determine whether openssl is used for libcurl via curl-config utility which should be installed as a part of our curl development headers package. We also additionally check that the openssl version is < 1.1.0 which alleviates the need for these callbacks. In this patchset we have done the following: - move all curl related global init functionality under rgw::curl namespace since libcurl may need to set up various ssl libraries etc during its init - introduce WITH_CURL_OPENSSL in cmake this checks the backend curl is deployed with using curl-config. Since curl devel is expected to be installed anyway, this binary should be available and can help identify the ssl backend curl was compiled with. - we only setup the locks if beast/civetweb aren't terminated with ssl, since these libraries setup the locks anyway and we want to prevent double initialization of openssl. Also we pass in ~CURL_GLOBAL_SSL making curl not initialize openssl if civetwb/beast is initializing them. Unfortunately this flag is a noop from curl >= 7.57 wherein both the libraries will end up initializing openssl anyway, which might override certain settings like error strings if using openssl < 1.1 https://curl.haxx.se/libcurl/c/threadsafe.html https://www.openssl.org/docs/man1.0.2/crypto/threads.html#DESCRIPTION Fixes: http://tracker.ceph.com/issues/22951 Signed-off-by: Abhishek Lekshmanan Signed-off-by: Jesse Williamson (cherry picked from commit 4e30277e4b38694e6f8d61925736cd7c2cff515f) Conflicts: - CMakeLists.txt libcryptopp drop in master conflict - src/rgw/rgw_main.cc conflict as we removed the ac related headers in Luminous --- diff --git a/CMakeLists.txt b/CMakeLists.txt index 503305ede6f2..56131a91b439 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -389,7 +389,23 @@ if (WITH_RADOSGW) message(STATUS "Looking for openssl anyways, because radosgw selected") find_package(OpenSSL) endif() +# https://curl.haxx.se/docs/install.html mentions the +# configure flags for various ssl backends + execute_process( + COMMAND + "sh" "-c" + "curl-config --configure | grep with-ssl" + RESULT_VARIABLE NO_CURL_SSL_LINK + ERROR_VARIABLE CURL_CONFIG_ERRORS + ) + if (CURL_CONFIG_ERRORS) + message(WARNING "unable to run curl-config; rgw cannot make ssl requests to external systems reliably") + endif() + find_package(OpenSSL) if (OPENSSL_FOUND) + if (NOT NO_CURL_SSL_LINK) + set(WITH_CURL_OPENSSL ON) + endif() # CURL_SSL_LINK execute_process( COMMAND "sh" "-c" @@ -422,7 +438,7 @@ if (WITH_RADOSGW) message(STATUS "crypto soname: ${LIBCRYPTO_SONAME}") else() message(WARNING "ssl not found: rgw civetweb may fail to dlopen libssl libcrypto") - endif() + endif() # OPENSSL_FOUND endif (WITH_RADOSGW) #option for CephFS diff --git a/src/include/config-h.in.cmake b/src/include/config-h.in.cmake index e151581fc39b..0b6b5248842f 100644 --- a/src/include/config-h.in.cmake +++ b/src/include/config-h.in.cmake @@ -163,6 +163,9 @@ /* define if radosgw's beast frontend enabled */ #cmakedefine WITH_RADOSGW_BEAST_FRONTEND +/* define if radosgw has openssl support */ +#cmakedefine WITH_CURL_OPENSSL + /* define if HAVE_THREAD_SAFE_RES_QUERY */ #cmakedefine HAVE_THREAD_SAFE_RES_QUERY diff --git a/src/rgw/CMakeLists.txt b/src/rgw/CMakeLists.txt index a809972cc033..4d4f6a132a0c 100644 --- a/src/rgw/CMakeLists.txt +++ b/src/rgw/CMakeLists.txt @@ -60,6 +60,7 @@ set(rgw_a_srcs rgw_frontend.cc rgw_gc.cc rgw_http_client.cc + rgw_http_client_curl.cc rgw_json_enc.cc rgw_keystone.cc rgw_ldap.cc @@ -188,6 +189,10 @@ add_dependencies(radosgw cls_rgw cls_lock cls_refcount cls_version cls_replica_log cls_user) install(TARGETS radosgw DESTINATION bin) +if (WITH_CURL_OPENSSL) + target_link_libraries(radosgw ${OPENSSL_LIBRARIES}) +endif (WITH_CURL_OPENSSL) + set(radosgw_admin_srcs rgw_admin.cc rgw_orphan.cc) diff --git a/src/rgw/rgw_http_client_curl.cc b/src/rgw/rgw_http_client_curl.cc new file mode 100644 index 000000000000..27240caf5b43 --- /dev/null +++ b/src/rgw/rgw_http_client_curl.cc @@ -0,0 +1,117 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "rgw_http_client_curl.h" +#include +#include +#include + +#include "rgw_common.h" +#define dout_context g_ceph_context +#define dout_subsys ceph_subsys_rgw + +#ifdef WITH_CURL_OPENSSL +#include +#endif + +#if defined(WITH_CURL_OPENSSL) && OPENSSL_API_COMPAT < 0x10100000L +namespace openssl { + +class RGWSSLSetup +{ + std::vector locks; +public: + RGWSSLSetup(int n) : locks (n){} + + void set_lock(int id){ + try { + locks.at(id).lock(); + } catch (std::out_of_range& e) { + dout(0) << __func__ << "failed to set locks" << dendl; + } + } + + void clear_lock(int id){ + try { + locks.at(id).unlock(); + } catch (std::out_of_range& e) { + dout(0) << __func__ << "failed to unlock" << dendl; + } + } +}; + + +void rgw_ssl_locking_callback(int mode, int id, const char *file, int line) +{ + static RGWSSLSetup locks(CRYPTO_num_locks()); + if (mode & CRYPTO_LOCK) + locks.set_lock(id); + else + locks.clear_lock(id); +} + +unsigned long rgw_ssl_thread_id_callback(){ + return (unsigned long)pthread_self(); +} + +void init_ssl(){ + CRYPTO_set_id_callback((unsigned long (*) ()) rgw_ssl_thread_id_callback); + CRYPTO_set_locking_callback(rgw_ssl_locking_callback); +} + +} /* namespace openssl */ +#endif // WITH_CURL_OPENSSL + + +namespace rgw { +namespace curl { +std::once_flag curl_init_flag; + +static void check_curl() +{ +#ifndef HAVE_CURL_MULTI_WAIT + derr << "WARNING: libcurl doesn't support curl_multi_wait()" << dendl; + derr << "WARNING: cross zone / region transfer performance may be affected" << dendl; +#endif +} + +#if defined(WITH_CURL_OPENSSL) && OPENSSL_API_COMPAT < 0x10100000L +void init_ssl() { + ::openssl::init_ssl(); +} + +bool fe_inits_ssl(const fe_map_t& m, long& curl_global_flags){ + for (const auto& kv: m){ + if (kv.first == "civetweb" || kv.first == "beast"){ + std::string cert; + kv.second->get_val("ssl_certificate","", &cert); + if (!cert.empty()){ + /* TODO this flag is no op for curl > 7.57 */ + curl_global_flags &= ~CURL_GLOBAL_SSL; + return true; + } + } + } + return false; +} +#endif // WITH_CURL_OPENSSL + +void setup_curl(const fe_map_t& m) { + check_curl(); + + long curl_global_flags = CURL_GLOBAL_ALL; + + #if defined(WITH_CURL_OPENSSL) && OPENSSL_API_COMPAT < 0x10100000L + if (!fe_inits_ssl(m, curl_global_flags)) + init_ssl(); + #endif + + std::call_once(curl_init_flag, curl_global_init, curl_global_flags); +} + +void cleanup_curl() { + curl_global_cleanup(); +} + +} /* namespace curl */ +} /* namespace rgw */ diff --git a/src/rgw/rgw_http_client_curl.h b/src/rgw/rgw_http_client_curl.h new file mode 100644 index 000000000000..fe44dfcedb98 --- /dev/null +++ b/src/rgw/rgw_http_client_curl.h @@ -0,0 +1,29 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab +/* + * Ceph - scalable distributed file system + * + * Copyright (C) 2018 SUSE Linux GmBH + * + * This is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License version 2.1, as published by the Free Software + * Foundation. See file COPYING. + * + */ +#ifndef RGW_HTTP_CLIENT_CURL_H +#define RGW_HTTP_CLIENT_CURL_H + +#include +#include "rgw_frontend.h" + +namespace rgw { +namespace curl { +using fe_map_t = std::multimap ; + +void setup_curl(const fe_map_t& m); +void cleanup_curl(); +} +} + +#endif diff --git a/src/rgw/rgw_main.cc b/src/rgw/rgw_main.cc index b7669242e810..daa9fb8a53fc 100644 --- a/src/rgw/rgw_main.cc +++ b/src/rgw/rgw_main.cc @@ -11,8 +11,6 @@ #include #include -#include - #include #include "acconfig.h" @@ -54,6 +52,7 @@ #include "rgw_request.h" #include "rgw_process.h" #include "rgw_frontend.h" +#include "rgw_http_client_curl.h" #if defined(WITH_RADOSGW_BEAST_FRONTEND) #include "rgw_asio_frontend.h" #endif /* WITH_RADOSGW_BEAST_FRONTEND */ @@ -140,17 +139,6 @@ static void godown_alarm(int signum) _exit(0); } -#ifdef HAVE_CURL_MULTI_WAIT -static void check_curl() -{ -} -#else -static void check_curl() -{ - derr << "WARNING: libcurl doesn't support curl_multi_wait()" << dendl; - derr << "WARNING: cross zone / region transfer performance may be affected" << dendl; -} -#endif class C_InitTimeout : public Context { public: @@ -295,8 +283,6 @@ int main(int argc, const char **argv) g_conf->set_val_or_die("rgw_zonegroup", g_conf->rgw_region.c_str()); } - check_curl(); - if (g_conf->daemonize) { global_init_daemonize(g_ceph_context); } @@ -319,10 +305,9 @@ int main(int argc, const char **argv) } rgw_init_resolver(); - - curl_global_init(CURL_GLOBAL_ALL); + rgw::curl::setup_curl(fe_map); rgw_setup_saved_curl_handles(); - + #if defined(WITH_RADOSGW_FCGI_FRONTEND) FCGX_Init(); #endif @@ -598,7 +583,7 @@ int main(int argc, const char **argv) rgw_tools_cleanup(); rgw_shutdown_resolver(); rgw_release_all_curl_handles(); - curl_global_cleanup(); + rgw::curl::cleanup_curl(); rgw_perf_stop(g_ceph_context);