]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
rgw: setup locks for libopenssl
authorAbhishek Lekshmanan <abhishek@suse.com>
Fri, 2 Mar 2018 16:41:42 +0000 (17:41 +0100)
committerAbhishek Lekshmanan <abhishek@suse.com>
Tue, 6 Mar 2018 14:32:48 +0000 (15:32 +0100)
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 <abhishek@suse.com>
Signed-off-by: Jesse Williamson <jwilliamson@suse.com>
(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

Conflicts:
  CMake isn't supported in jewel, while the CMake changes were cherried,
  CMakelists for rgw does not exist in jewel and this commit doesn't fix that.
  So though CMake changes are backported, cmake build is untested and expected
  to be broken
src/include/config-h.in.cmake
  src/rgw/rgw_main.cc

CMakeLists.txt
src/include/config-h.in.cmake
src/rgw/rgw_http_client_curl.cc [new file with mode: 0644]
src/rgw/rgw_http_client_curl.h [new file with mode: 0644]
src/rgw/rgw_main.cc

index 3dcea47c05def0de25be7a7e10c789217d9da808..7ad7786da0dcd49cc47a03bb8d9ec5544e87d508 100644 (file)
@@ -293,7 +293,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"
@@ -326,7 +342,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
index 059e0a24d73e91e6b7b164a061cab72f89efb4ca..45aec7a51c9ec4f8a2729d7523552c4cd8f88ead 100644 (file)
 /* Define to 1 if you have the `syncfs' function. */
 #cmakedefine HAVE_SYNCFS 1
 
+/* define if radosgw has openssl support */
+#cmakedefine WITH_CURL_OPENSSL
+
 /* Define to 1 if you have the `pwritev' function. */
 #cmakedefine HAVE_PWRITEV 1
 
diff --git a/src/rgw/rgw_http_client_curl.cc b/src/rgw/rgw_http_client_curl.cc
new file mode 100644 (file)
index 0000000..27240ca
--- /dev/null
@@ -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 <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();
+}
+
+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 (file)
index 0000000..fe44dfc
--- /dev/null
@@ -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 <map>
+#include "rgw_frontend.h"
+
+namespace rgw {
+namespace curl {
+using fe_map_t = std::multimap <std::string, RGWFrontendConfig *>;
+
+void setup_curl(const fe_map_t& m);
+void cleanup_curl();
+}
+}
+
+#endif
index d96d97b0ca87d30fd8f495f2a12ad0bfc1b45af6..f398495edba8177f86d85d2c8f0688c84d781df7 100644 (file)
@@ -11,8 +11,6 @@
 #include <errno.h>
 #include <signal.h>
 
-#include <curl/curl.h>
-
 #include <boost/intrusive_ptr.hpp>
 
 #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"
 
 #include <map>
 #include <string>
@@ -135,17 +134,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:
@@ -273,8 +261,6 @@ int main(int argc, const char **argv)
     }
   }
 
-  check_curl();
-
   if (g_conf->daemonize) {
     global_init_daemonize(g_ceph_context);
   }
@@ -293,10 +279,10 @@ int main(int argc, const char **argv)
   rgw_tools_init(g_ceph_context);
 
   rgw_init_resolver();
-  
-  curl_global_init(CURL_GLOBAL_ALL);
+
+  rgw::curl::setup_curl(fe_map);
   rgw_setup_saved_curl_handles();
-  
+
   FCGX_Init();
 
   int r = 0;
@@ -527,7 +513,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);