Fix multi-threaded access to non-TLS servers (eg - SNP nodes fetching…

… endorsements from THIM) (#6836)

.snpcc_canary

@@ -4,4 +4,5 @@
 /-xXx--//-----x=x--/-xXx--/---x-/--->>>--/
 ...
 /\/\d(-_-)b/\/\
-----vmpl---
+----vmpl---
+

CHANGELOG.md

@@ -5,21 +5,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
-## [6.0.0-dev21]
+## [6.0.0-dev20]
 
-[6.0.0-dev21]: https://github.com/microsoft/CCF/releases/tag/6.0.0-dev21
+[6.0.0-dev20]: https://github.com/microsoft/CCF/releases/tag/6.0.0-dev20
 
 ### Added
 
 - Added `GET /node/attestations` and `GET /node/attestations/self`, as aliases for the `/quote` endpoints. These return attestations on every platform, not only SGX quotes.
 
-## [6.0.0-dev20]
-
-[6.0.0-dev20]: https://github.com/microsoft/CCF/releases/tag/6.0.0-dev20
-
 ### Fixed
 
 - CA certificate bundles used for JWT refresh and containing more than one certificate are now handled correctly (#6817).
+- Fixed thread-safety issues when CCF nodes attempted to contact non-TLS servers. This previously could cause errors when running SNP builds with multiple worker threads (#6836).
 
 ## [6.0.0-dev19]
 

python/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "ccf"
-version = "6.0.0-dev21"
+version = "6.0.0-dev20"
 authors = [
   { name="CCF Team", email="[email protected]" },
 ]

src/enclave/session.h

@@ -53,6 +53,27 @@ namespace ccf
     }
 
     virtual void handle_incoming_data_thread(std::vector<uint8_t>&& data) = 0;
+
+    // Implement Session::sent_data by dispatching a thread message
+    // that eventually invokes the virtual send_data_thread()
+    void send_data(std::span<const uint8_t> data) override
+    {
+      auto msg =
+        std::make_unique<::threading::Tmsg<SendRecvMsg>>(&send_data_cb);
+      msg->data.self = this->shared_from_this();
+      msg->data.data.assign(data.begin(), data.end());
+
+      ::threading::ThreadMessaging::instance().add_task(
+        execution_thread, std::move(msg));
+    }
+
+    static void send_data_cb(
+      std::unique_ptr<::threading::Tmsg<SendRecvMsg>> msg)
+    {
+      msg->data.self->send_data_thread(std::move(msg->data.data));
+    }
+
+    virtual void send_data_thread(std::vector<uint8_t>&& data) = 0;
   };
 
   class EncryptedSession : public ThreadedSession
@@ -77,9 +98,16 @@ namespace ccf
   public:
     void send_data(std::span<const uint8_t> data) override
     {
+      // Override send_data rather than send_data_thread, as the TLSSession
+      // handles dispatching for thread affinity
       tls_io->send_raw(data.data(), data.size());
     }
 
+    void send_data_thread(std::vector<uint8_t>&& data) override
+    {
+      throw std::logic_error("Unimplemented");
+    }
+
     void close_session() override
     {
       tls_io->close();
@@ -141,7 +169,7 @@ namespace ccf
       to_host(writer_factory_.create_writer_to_outside())
     {}
 
-    void send_data(std::span<const uint8_t> data) override
+    void send_data_thread(std::vector<uint8_t>&& data) override
     {
       RINGBUFFER_WRITE_MESSAGE(
         ::tcp::tcp_outbound,

src/node/quote_endorsements_client.h

@@ -44,6 +44,8 @@ namespace ccf
 
     std::vector<uint8_t> endorsements_pem;
 
+    ccf::pal::Mutex lock;
+
     // Uniquely identify each received request. We assume that this client sends
     // requests in series, after receiving the response to each one or after a
     // long timeout.
@@ -124,6 +126,7 @@ namespace ccf
         ::threading::Tmsg<QuoteEndorsementsClientTimeoutMsg>>(
         [](std::unique_ptr<::threading::Tmsg<QuoteEndorsementsClientTimeoutMsg>>
              msg) {
+          std::lock_guard<ccf::pal::Mutex> guard(msg->data.self->lock);
           if (msg->data.self->has_completed)
           {
             return;
@@ -243,6 +246,8 @@ namespace ccf
           ccf::http_status status,
           http::HeaderMap&& headers,
           std::vector<uint8_t>&& data) {
+          std::lock_guard<ccf::pal::Mutex> guard(this->lock);
+
           last_received_request_id++;
 
           if (status == HTTP_STATUS_OK)
@@ -312,6 +317,7 @@ namespace ccf
 
     void fetch_endorsements()
     {
+      std::lock_guard<ccf::pal::Mutex> guard(this->lock);
       auto const& server = config.servers.front();
       if (server.empty())
       {