[XrdCl] Make sure there is a unique SSL* context per event-loop thread.

xrootd · Jan 27, 2021 · 970fd15 · 970fd15
1 parent 3c64ccb
commit 970fd15
Show file tree

Hide file tree

Showing 5 changed files with 132 additions and 42 deletions.
diff --git a/src/XrdCl/XrdClAsyncSocketHandler.cc b/src/XrdCl/XrdClAsyncSocketHandler.cc
@@ -25,8 +25,94 @@
 #include "XrdCl/XrdClXRootDMsgHandler.hh"
 #include "XrdCl/XrdClOptimizers.hh"
 #include "XrdSys/XrdSysE2T.hh"
+#include "XrdTls/XrdTlsContext.hh"
+
 #include <netinet/tcp.h>
 
+#include <unordered_map>
+#include <thread>
+
+namespace
+{
+  //---------------------------------------------------------------------------
+  // Multitone providing an unique instance of XrdTlsContext for every
+  // event-loop thread.
+  //---------------------------------------------------------------------------
+  struct TlsContextProvider
+  {
+    //-------------------------------------------------------------------------
+    // Singleton access
+    //-------------------------------------------------------------------------
+    static TlsContextProvider& Instance()
+    {
+      static TlsContextProvider instance;
+      return instance;
+    }
+
+    //-------------------------------------------------------------------------
+    // Get the TLS context unique to the calling thread
+    //-------------------------------------------------------------------------
+    XrdTlsContext& Get()
+    {
+      std::unique_lock<std::mutex> lck( mtx );
+      //-----------------------------------------------------------------------
+      // Each event-loop thread has its own SSL context
+      //-----------------------------------------------------------------------
+      std::thread::id this_id = std::this_thread::get_id();
+      auto itr = instances.find( this_id );
+      if( itr == instances.end() )
+      {
+        std::string emsg;
+        auto p = instances.emplace( std::piecewise_construct, std::forward_as_tuple( this_id ),
+                                    std::forward_as_tuple( nullptr, nullptr, GetCaDir(), nullptr, 0, &emsg ) );
+        itr = p.first;
+        //---------------------------------------------------------------------
+        // If the context is not valid throw an exception! We throw generic
+        // exception as this will be translated to TlsError anyway.
+        //---------------------------------------------------------------------
+        if( !itr->second.isOK() ) throw std::runtime_error( emsg );
+      }
+      return itr->second;
+    }
+
+    private:
+
+      //-----------------------------------------------------------------------
+      // Default constructor
+      //-----------------------------------------------------------------------
+      TlsContextProvider(){ }
+
+      //-----------------------------------------------------------------------
+      // Copy and move constructors, deleted!
+      //-----------------------------------------------------------------------
+      TlsContextProvider( const TlsContextProvider& ) = delete;
+      TlsContextProvider( TlsContextProvider&& ) = delete;
+
+      //-----------------------------------------------------------------------
+      // Copy and move assignment operator, deleted!
+      //-----------------------------------------------------------------------
+      TlsContextProvider& operator=( const TlsContextProvider& ) = delete;
+      TlsContextProvider& operator=( TlsContextProvider&& ) = delete;
+
+      //------------------------------------------------------------------------
+      // Helper function for setting the CA directory in TLS context
+      //------------------------------------------------------------------------
+      static const char* GetCaDir()
+      {
+        static const char *envval = getenv("X509_CERT_DIR");
+        static const std::string cadir = envval ? envval :
+                                         "/etc/grid-security/certificates";
+        return cadir.c_str();
+      }
+
+      //------------------------------------------------------------------------
+      // TLS contexts for every event-loop thread
+      //------------------------------------------------------------------------
+      std::unordered_map<std::thread::id, XrdTlsContext> instances;
+      std::mutex mtx;
+  };
+}
+
 namespace XrdCl
 {
   //----------------------------------------------------------------------------
@@ -764,7 +850,17 @@ namespace XrdCl
     if( !pSocket->IsEncrypted() &&
          pTransport->NeedEncryption( pHandShakeData, *pChannelData ) )
     {
-      XRootDStatus st = DoTlsHandShake();
+      //------------------------------------------------------------------------
+      //
+      //------------------------------------------------------------------------
+      XRootDStatus st = pSocket->CreateTLS( TlsContextProvider::Instance().Get(),
+                                            this );
+      if( !st.IsOK() )
+      {
+        OnFaultWhileHandshaking( st );
+        return;
+      }
+      st = DoTlsHandShake();
       if( !st.IsOK() || st.code == suRetry ) return;
     }
 

diff --git a/src/XrdCl/XrdClSocket.cc b/src/XrdCl/XrdClSocket.cc
@@ -780,20 +780,9 @@ namespace XrdCl
   XRootDStatus Socket::TlsHandShake( AsyncSocketHandler *socketHandler,
                                      const std::string  &thehost )
   {
-    try
-    {
-      if( !pServerAddr ) return XRootDStatus( stError, errInvalidOp );
-      if( !pTls ) pTls.reset( new Tls( this, socketHandler ) );
-      return pTls->Connect( thehost, pServerAddr.get() );
-    }
-    catch( std::exception& ex )
-    {
-      // the exception has been thrown when we tried to create
-      // the TLS context
-      return XRootDStatus( stFatal, errTlsError, 0, ex.what() );
-    }
-
-    return XRootDStatus();
+    if( !pServerAddr ) return XRootDStatus( stError, errInvalidOp );
+    if( !pTls ) XRootDStatus( stFatal, errTlsError, 0, "TLS context missing." );
+    return pTls->Connect( thehost, pServerAddr.get() );
   }
 
   //------------------------------------------------------------------------
@@ -805,6 +794,24 @@ namespace XrdCl
     return bool( pTls.get() );
   }
 
+  //------------------------------------------------------------------------
+  // Create the TLS context for the Socket object
+  //------------------------------------------------------------------------
+  XRootDStatus Socket::CreateTLS( XrdTlsContext       &tlsContext,
+                                  AsyncSocketHandler  *socketHandler)
+  {
+    try
+    {
+      pTls.reset( new Tls( tlsContext, this, socketHandler ) );
+    }
+    catch( std::exception &ex )
+    {
+      // the exception has been thrown when we tried to create
+      // the TLS context
+      return XRootDStatus( stFatal, errTlsError, 0, ex.what() );
+    }
+    return XRootDStatus();
+  }
 }
 
 
diff --git a/src/XrdCl/XrdClSocket.hh b/src/XrdCl/XrdClSocket.hh
@@ -28,12 +28,13 @@
 #include "XrdNet/XrdNetAddr.hh"
 #include "XrdSys/XrdSysKernelBuffer.hh"
 
+class XrdTlsContext;
 
 namespace XrdCl
 {
   class AnyObject;
-  class Tls;
   class AsyncSocketHandler;
+  class Tls;
 
   //----------------------------------------------------------------------------
   //! A network socket
@@ -286,6 +287,12 @@ namespace XrdCl
       //------------------------------------------------------------------------
       bool IsEncrypted();
 
+      //------------------------------------------------------------------------
+      // Create the TLS context for the Socket object
+      //------------------------------------------------------------------------
+      XRootDStatus CreateTLS( XrdTlsContext       &tlsContext,
+                              AsyncSocketHandler  *socketHandler);
+
     protected:
       //------------------------------------------------------------------------
       //! Poll the socket to see whether it is ready for IO

diff --git a/src/XrdCl/XrdClTls.cc b/src/XrdCl/XrdClTls.cc
@@ -85,42 +85,22 @@ namespace
         return XrdTls::dbgOFF;
       }
   };
-
-  //------------------------------------------------------------------------
-  // Helper function for setting the CA directory in TLS context
-  //------------------------------------------------------------------------
-  static const char* GetCaDir()
-  {
-    static const char *envval = getenv("X509_CERT_DIR");
-    static const std::string cadir = envval ? envval :
-                                     "/etc/grid-security/certificates";
-    return cadir.c_str();
-  }
 }
 
 namespace XrdCl
 {
   //------------------------------------------------------------------------
   // Constructor
   //------------------------------------------------------------------------
-  Tls::Tls( Socket *socket, AsyncSocketHandler *socketHandler ) : pSocket( socket ), pTlsHSRevert( None ), pSocketHandler( socketHandler )
+  Tls::Tls( XrdTlsContext      &tlsContext,
+            Socket             *socket,
+            AsyncSocketHandler *socketHandler ) : pSocket( socket ),
+                                                  pTlsHSRevert( None ),
+                                                  pSocketHandler( socketHandler )
   {
     //----------------------------------------------------------------------
     // Set the message callback for TLS layer
     //----------------------------------------------------------------------
-    SetTlsMsgCB::Once();
-    //----------------------------------------------------------------------
-    // we only need one instance of TLS
-    //----------------------------------------------------------------------
-    std::string emsg;
-    static XrdTlsContext tlsContext( 0, 0, GetCaDir(), 0, 0, &emsg );
-
-    //----------------------------------------------------------------------
-    // If the context is not valid throw an exception! We throw generic
-    // exception as this will be translated to TlsError anyway.
-    //----------------------------------------------------------------------
-    if( !tlsContext.isOK() ) throw std::runtime_error( emsg );
-
     pTls.reset(
         new XrdTlsSocket( tlsContext, pSocket->GetFD(), XrdTlsSocket::TLS_RNB_WNB,
                           XrdTlsSocket::TLS_HS_NOBLK, true ) );

diff --git a/src/XrdCl/XrdClTls.hh b/src/XrdCl/XrdClTls.hh
@@ -40,7 +40,7 @@ namespace XrdCl
       //------------------------------------------------------------------------
       //! Constructor - creates async TLS layer for given socker file descriptor
       //------------------------------------------------------------------------
-      Tls( Socket *socket, AsyncSocketHandler *socketHandler );
+      Tls( XrdTlsContext &tlsContext, Socket *socket, AsyncSocketHandler *socketHandler );
 
       //------------------------------------------------------------------------
       //! Destructor