Merge branch 'main' into f/5241-historical-queries-customisable-error…

…-handling

CMakeLists.txt

@@ -991,6 +991,9 @@ if(BUILD_TESTS)
     )
     target_link_libraries(base64_test PRIVATE ${CMAKE_THREAD_LIBS_INIT})
 
+    add_unit_test(pem_test ${CMAKE_CURRENT_SOURCE_DIR}/src/crypto/test/pem.cpp)
+    target_link_libraries(pem_test PRIVATE ${CMAKE_THREAD_LIBS_INIT})
+
     add_test_bin(
       kp_cert_test ${CMAKE_CURRENT_SOURCE_DIR}/src/crypto/test/kp_cert.cpp
     )

cmake/crypto.cmake

@@ -13,6 +13,7 @@ set(CCFCRYPTO_SRC
     ${CCF_DIR}/src/crypto/verifier.cpp
     ${CCF_DIR}/src/crypto/key_wrap.cpp
     ${CCF_DIR}/src/crypto/hmac.cpp
+    ${CCF_DIR}/src/crypto/pem.cpp
     ${CCF_DIR}/src/crypto/ecdsa.cpp
     ${CCF_DIR}/src/crypto/openssl/symmetric_key.cpp
     ${CCF_DIR}/src/crypto/openssl/public_key.cpp

include/ccf/crypto/pem.h

@@ -18,84 +18,57 @@ namespace ccf::crypto
   {
   private:
     std::string s;
-
-    void check_pem_format()
-    {
-      if (s.find("-----BEGIN") == std::string::npos)
-      {
-        throw std::runtime_error(
-          fmt::format("PEM constructed with non-PEM data: {}", s));
-      }
-    }
+    void check_pem_format();
 
   public:
     Pem() = default;
-
-    Pem(const std::string& s_) : s(s_)
-    {
-      check_pem_format();
-    }
-
-    Pem(const uint8_t* data, size_t size)
-    {
-      if (size == 0)
-        throw std::logic_error("Got PEM of size 0.");
-
-      // If it's already null-terminated, don't suffix again
-      const auto null_terminated = *(data + size - 1) == 0;
-      if (null_terminated)
-        size -= 1;
-
-      s.assign(reinterpret_cast<const char*>(data), size);
-
-      check_pem_format();
-    }
+    Pem(const std::string& s_);
+    Pem(const uint8_t* data, size_t size);
 
     explicit Pem(std::span<const uint8_t> s) : Pem(s.data(), s.size()) {}
-
     explicit Pem(const std::vector<uint8_t>& v) : Pem(v.data(), v.size()) {}
 
-    bool operator==(const Pem& rhs) const
+    inline bool operator==(const Pem& rhs) const
     {
       return s == rhs.s;
     }
 
-    bool operator!=(const Pem& rhs) const
+    inline bool operator!=(const Pem& rhs) const
     {
       return !(*this == rhs);
     }
 
-    bool operator<(const Pem& rhs) const
+    inline bool operator<(const Pem& rhs) const
     {
       return s < rhs.s;
     }
 
-    const std::string& str() const
+    inline const std::string& str() const
     {
       return s;
     }
 
-    uint8_t* data()
+    inline uint8_t* data()
     {
       return reinterpret_cast<uint8_t*>(s.data());
     }
 
-    const uint8_t* data() const
+    inline const uint8_t* data() const
     {
       return reinterpret_cast<const uint8_t*>(s.data());
     }
 
-    size_t size() const
+    inline size_t size() const
     {
       return s.size();
     }
 
-    bool empty() const
+    inline bool empty() const
     {
       return s.empty();
     }
 
-    std::vector<uint8_t> raw() const
+    inline std::vector<uint8_t> raw() const
     {
       return {data(), data() + size()};
     }
@@ -128,22 +101,8 @@ namespace ccf::crypto
     return "Pem";
   }
 
-  static std::vector<ccf::crypto::Pem> split_x509_cert_bundle(
-    const std::string_view& pem)
-  {
-    std::string separator("-----END CERTIFICATE-----");
-    std::vector<ccf::crypto::Pem> pems;
-    auto separator_end = 0;
-    auto next_separator_start = pem.find(separator);
-    while (next_separator_start != std::string_view::npos)
-    {
-      pems.emplace_back(std::string(
-        pem.substr(separator_end, next_separator_start + separator.size())));
-      separator_end = next_separator_start + separator.size();
-      next_separator_start = pem.find(separator, separator_end);
-    }
-    return pems;
-  }
+  std::vector<ccf::crypto::Pem> split_x509_cert_bundle(
+    const std::string_view& pem);
 
   inline void fill_json_schema(nlohmann::json& schema, const Pem*)
   {

include/ccf/ds/logger.h

@@ -324,13 +324,6 @@ namespace ccf::logger
         logger->write(line);
       }
 
-#ifndef INSIDE_ENCLAVE
-      if (line.log_level == LoggerLevel::FATAL)
-      {
-        throw std::logic_error("Fatal: " + format_to_text(line));
-      }
-#endif
-
       return true;
     }
   };

src/clients/perf/perf_client.h

@@ -32,6 +32,7 @@ namespace client
     if (core_id > threads || core_id < 0)
     {
       LOG_FATAL_FMT("Invalid core id: {}", core_id);
+      abort();
       return false;
     }
 
@@ -43,6 +44,7 @@ namespace client
     if (sched_setaffinity(0, sizeof(cpu_set_t), &set) < 0)
     {
       LOG_FATAL_FMT("Unable to set affinity");
+      abort();
       return false;
     }
 

src/crypto/pem.cpp

@@ -0,0 +1,52 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the Apache 2.0 License.
+#include "ccf/crypto/pem.h"
+
+namespace ccf::crypto
+{
+  void Pem::check_pem_format()
+  {
+    if (s.find("-----BEGIN") == std::string::npos)
+    {
+      throw std::runtime_error(
+        fmt::format("PEM constructed with non-PEM data: {}", s));
+    }
+  }
+
+  Pem::Pem(const std::string& s_) : s(s_)
+  {
+    check_pem_format();
+  }
+
+  Pem::Pem(const uint8_t* data, size_t size)
+  {
+    if (size == 0)
+      throw std::logic_error("Got PEM of size 0.");
+
+    // If it's already null-terminated, don't suffix again
+    const auto null_terminated = *(data + size - 1) == 0;
+    if (null_terminated)
+      size -= 1;
+
+    s.assign(reinterpret_cast<const char*>(data), size);
+
+    check_pem_format();
+  }
+
+  std::vector<ccf::crypto::Pem> split_x509_cert_bundle(
+    const std::string_view& pem)
+  {
+    std::string separator("-----END CERTIFICATE-----");
+    std::vector<ccf::crypto::Pem> pems;
+    auto separator_end = 0;
+    auto next_separator_start = pem.find(separator);
+    while (next_separator_start != std::string_view::npos)
+    {
+      pems.emplace_back(std::string(
+        pem.substr(separator_end, next_separator_start + separator.size())));
+      separator_end = next_separator_start + separator.size();
+      next_separator_start = pem.find(separator, separator_end);
+    }
+    return pems;
+  }
+}

src/crypto/test/pem.cpp

@@ -0,0 +1,58 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the Apache 2.0 License.
+#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
+#include "ccf/crypto/pem.h"
+
+#include <chrono>
+#include <doctest/doctest.h>
+#include <string>
+
+using namespace std;
+using namespace ccf::crypto;
+
+TEST_CASE("Split x509 cert bundle")
+{
+  REQUIRE(split_x509_cert_bundle("") == std::vector<Pem>{});
+
+  const std::string single_cert =
+    "-----BEGIN "
+    "CERTIFICATE-----"
+    "\nMIIByDCCAU6gAwIBAgIQOBe5SrcwReWmSzTjzj2HDjAKBggqhkjOPQQDAzATMREw\nDwYDVQ"
+    "QDDAhDQ0YgTm9kZTAeFw0yMzA1MTcxMzUwMzFaFw0yMzA1MTgxMzUwMzBa\nMBMxETAPBgNVBA"
+    "MMCENDRiBOb2RlMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE74qL\nAc/"
+    "45tiriN5MuquYhHVdMGQRvYSm08HBfYcODtET88qC0A39o6Y2TmfbIn6BdjMG\nkD58o377ZMT"
+    "aApQu/oJcwt7qZ9/LE8j8WU2qHn0cPTlpwH/"
+    "2tiud2w+U3voSo2cw\nZTASBgNVHRMBAf8ECDAGAQH/"
+    "AgEAMB0GA1UdDgQWBBS9FJNwWSXtUpHaBV57EwTW\noM8vHjAfBgNVHSMEGDAWgBS9FJNwWSXt"
+    "UpHaBV57EwTWoM8vHjAPBgNVHREECDAG\nhwR/"
+    "xF96MAoGCCqGSM49BAMDA2gAMGUCMQDKxpjPToJ7VSqKqQSeMuW9tr4iL+"
+    "9I\n7gTGdGwiIYV1qTSS35Sk9XQZ0VpSa58c/"
+    "5UCMEgmH71k7XlTGVUypm4jAgjpC46H\ns+hJpGMvyD9dKzEpZgmZYtghbyakUkwBiqmFQA=="
+    "\n-----END CERTIFICATE-----";
+  auto bundle = split_x509_cert_bundle(single_cert);
+  const auto cert_pem = Pem(single_cert);
+  REQUIRE(bundle.size() == 1);
+  REQUIRE(bundle[0] == cert_pem);
+
+  const std::string two_certs = single_cert + single_cert;
+  bundle = split_x509_cert_bundle(two_certs);
+  REQUIRE(bundle.size() == 2);
+  REQUIRE(bundle[0] == cert_pem);
+  REQUIRE(bundle[1] == cert_pem);
+
+  std::string bundle_with_invalid_suffix = single_cert + "ignored suffix";
+  bundle = split_x509_cert_bundle(bundle_with_invalid_suffix);
+  REQUIRE(bundle.size() == 1);
+  REQUIRE(bundle[0] == cert_pem);
+
+  bundle_with_invalid_suffix =
+    single_cert + "-----BEGIN CERTIFICATE-----\nignored suffix";
+  bundle = split_x509_cert_bundle(bundle_with_invalid_suffix);
+  REQUIRE(bundle.size() == 1);
+  REQUIRE(bundle[0] == cert_pem);
+
+  const std::string bundle_with_very_invalid_pem =
+    single_cert + "not a cert\n-----END CERTIFICATE-----";
+  REQUIRE_THROWS_AS(
+    split_x509_cert_bundle(bundle_with_very_invalid_pem), std::runtime_error);
+}

src/ds/test/logger.cpp

@@ -58,7 +58,7 @@ TEST_CASE("Framework logging macros")
 
   {
     REQUIRE(logs.empty());
-    REQUIRE_THROWS(LOG_FATAL_FMT("Hello C"));
+    LOG_FATAL_FMT("Hello C");
     REQUIRE(logs.size() == 1);
 
     const auto& log = logs[0];
@@ -109,7 +109,7 @@ TEST_CASE("Application logging macros")
 
   {
     REQUIRE(logs.empty());
-    REQUIRE_THROWS(CCF_APP_FATAL("Hello C"));
+    CCF_APP_FATAL("Hello C");
     REQUIRE(logs.size() == 1);
 
     const auto& log = logs[0];

src/endpoints/endpoint.cpp

@@ -110,10 +110,12 @@ namespace ccf::endpoints
   {
     if (installer == nullptr)
     {
-      LOG_FATAL_FMT(
+      auto msg = fmt::format(
         "Can't install this endpoint ({}) - it is not associated with an "
         "installer",
         full_uri_path);
+      LOG_FATAL_FMT("{}", msg);
+      throw std::logic_error(msg);
     }
     else
     {

src/host/main.cpp

@@ -62,19 +62,6 @@ void print_version(size_t)
   exit(0);
 }
 
-std::string read_required_environment_variable(
-  const std::string& envvar, const std::string& name)
-{
-  auto ev = std::getenv(envvar.c_str());
-  if (ev == nullptr)
-  {
-    LOG_FATAL_FMT(
-      "Environment variable \"{}\" for {} is not set", envvar, name);
-  }
-  LOG_INFO_FMT("Reading {} from environment {}", name, envvar);
-  return ev;
-}
-
 int main(int argc, char** argv)
 {
   // ignore SIGPIPE
@@ -676,6 +663,7 @@ int main(int argc, char** argv)
     else
     {
       LOG_FATAL_FMT("Start command should be start|join|recover. Exiting.");
+      return static_cast<int>(CLI::ExitCodes::ValidationError);
     }
 
     std::vector<uint8_t> startup_snapshot = {};

src/host/socket.h

@@ -71,10 +71,12 @@ namespace asynchost
     virtual void on_resolve_failed()
     {
       LOG_FATAL_FMT("{} {} resolve failed", conn_name, name);
+      abort();
     }
     virtual void on_listen_failed()
     {
       LOG_FATAL_FMT("{} {} listen failed", conn_name, name);
+      abort();
     }
   };
 

src/node/acme_client.h

@@ -355,7 +355,7 @@ namespace ACME
             }
             catch (const std::exception& ex)
             {
-              LOG_FATAL_FMT("ACME: request callback failed: {}", ex.what());
+              LOG_FAIL_FMT("ACME: request callback failed: {}", ex.what());
               return false;
             }
           });
@@ -807,8 +807,7 @@ namespace ACME
             }
             else
             {
-              LOG_FATAL_FMT(
-                "ACME: unknown order status '{}', aborting", status);
+              LOG_FAIL_FMT("ACME: unknown order status '{}', aborting", status);
               guard.unlock();
               remove_order(*order_url_opt);
             }

tests/perf-system/submitter/CMakeLists.txt

@@ -18,6 +18,7 @@ set(CCFCRYPTO_SRC
     ${CCF_DIR}/src/crypto/verifier.cpp
     ${CCF_DIR}/src/crypto/key_wrap.cpp
     ${CCF_DIR}/src/crypto/hmac.cpp
+    ${CCF_DIR}/src/crypto/pem.cpp
     ${CCF_DIR}/src/crypto/openssl/symmetric_key.cpp
     ${CCF_DIR}/src/crypto/openssl/public_key.cpp
     ${CCF_DIR}/src/crypto/openssl/key_pair.cpp