Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rgw: setup locks for libopenssl #20390

Merged
merged 5 commits into from
Mar 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,24 @@ endif(WITH_RADOSGW)


if (WITH_RADOSGW)
# 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)
message(STATUS "libcurl is linked with openssl: explicitly setting locks")
set(WITH_CURL_OPENSSL ON)
endif() # CURL_SSL_LINK
execute_process(
COMMAND
"sh" "-c"
Expand Down Expand Up @@ -431,7 +447,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
Expand Down
3 changes: 3 additions & 0 deletions src/include/config-h.in.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,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

Expand Down
5 changes: 5 additions & 0 deletions src/rgw/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -156,6 +157,10 @@ if (WITH_RADOSGW_BEAST_FRONTEND)
target_link_libraries(rgw_a Boost::coroutine Boost::context)
endif()

if (WITH_CURL_OPENSSL)
target_link_libraries(rgw_a ${OPENSSL_LIBRARIES})
endif (WITH_CURL_OPENSSL)

set(radosgw_srcs
rgw_loadgen_process.cc
rgw_civetweb.cc
Expand Down
11 changes: 10 additions & 1 deletion src/rgw/rgw_admin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
#include "rgw_realm_watcher.h"
#include "rgw_role.h"
#include "rgw_reshard.h"

#include "rgw_http_client_curl.h"

#define dout_context g_ceph_context
#define dout_subsys ceph_subsys_rgw
Expand Down Expand Up @@ -3015,6 +3015,15 @@ int main(int argc, const char **argv)
rgw_user_init(store);
rgw_bucket_init(store->meta_mgr);

struct rgw_curl_setup {
rgw_curl_setup() {
rgw::curl::setup_curl(boost::none);
}
~rgw_curl_setup() {
rgw::curl::cleanup_curl();
}
} curl_cleanup;

StoreDestructor store_destructor(store);

if (raw_storage_op) {
Expand Down
122 changes: 122 additions & 0 deletions src/rgw/rgw_http_client_curl.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// -*- 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 <mutex>
#include <vector>
#include <curl/curl.h>

#include "rgw_common.h"
#define dout_context g_ceph_context
#define dout_subsys ceph_subsys_rgw

#ifdef WITH_CURL_OPENSSL
#include <openssl/crypto.h>
#endif

#if defined(WITH_CURL_OPENSSL) && OPENSSL_API_COMPAT < 0x10100000L
namespace openssl {

class RGWSSLSetup
{
std::vector <std::mutex> 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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert my usual nit against C style casts here. ;-)

}

void init_ssl(){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here as above-- it may be slightly more clear moving the ifdefs inside of the function body, but it's your call.

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 {

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(boost::optional <const fe_map_t&> m, long& curl_global_flags){
if (m) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary, but notice that if you turned this into a guard:
if (!m)
return false;

...then the whole function doesn't need to be in an if-block. Up to you!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we also have to return false when we later find out that ssl_certificate is not set.

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

std::once_flag curl_init_flag;

void setup_curl(boost::optional<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);
rgw_setup_saved_curl_handles();
}

void cleanup_curl() {
rgw_release_all_curl_handles();
curl_global_cleanup();
}

} /* namespace curl */
} /* namespace rgw */
30 changes: 30 additions & 0 deletions src/rgw/rgw_http_client_curl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// -*- 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 <map>
#include <boost/optional.hpp>
#include "rgw_frontend.h"

namespace rgw {
namespace curl {
using fe_map_t = std::multimap <std::string, RGWFrontendConfig *>;

void setup_curl(boost::optional<const fe_map_t&> m);
void cleanup_curl();
}
}

#endif
28 changes: 4 additions & 24 deletions src/rgw/rgw_main.cc
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
// vim: ts=8 sw=2 smarttab

extern "C" {
#include <curl/curl.h>
}

#include "common/ceph_argparse.h"
#include "global/global_init.h"
#include "global/signal_handler.h"
Expand Down Expand Up @@ -40,6 +35,7 @@ extern "C" {
#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 */
Expand Down Expand Up @@ -114,17 +110,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:
Expand Down Expand Up @@ -270,8 +255,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);
}
Expand All @@ -297,10 +280,8 @@ int main(int argc, const char **argv)
}

rgw_init_resolver();

curl_global_init(CURL_GLOBAL_ALL);
rgw_setup_saved_curl_handles();

rgw::curl::setup_curl(fe_map);

#if defined(WITH_RADOSGW_FCGI_FRONTEND)
FCGX_Init();
#endif
Expand Down Expand Up @@ -573,8 +554,7 @@ int main(int argc, const char **argv)
rgw::auth::s3::LDAPEngine::shutdown();
rgw_tools_cleanup();
rgw_shutdown_resolver();
rgw_release_all_curl_handles();
curl_global_cleanup();
rgw::curl::cleanup_curl();

rgw_perf_stop(g_ceph_context);

Expand Down