diff --git a/CMakeLists.txt b/CMakeLists.txt index a40282a282df..9a089405858b 100644 --- a/CMakeLists.txt +++ b/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 ) diff --git a/cmake/crypto.cmake b/cmake/crypto.cmake index 55bc26b4471d..28d512cf8ecd 100644 --- a/cmake/crypto.cmake +++ b/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 diff --git a/include/ccf/crypto/pem.h b/include/ccf/crypto/pem.h index 7be710da3208..e7aa8776c474 100644 --- a/include/ccf/crypto/pem.h +++ b/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(data), size); - - check_pem_format(); - } + Pem(const std::string& s_); + Pem(const uint8_t* data, size_t size); explicit Pem(std::span s) : Pem(s.data(), s.size()) {} - explicit Pem(const std::vector& 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(s.data()); } - const uint8_t* data() const + inline const uint8_t* data() const { return reinterpret_cast(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 raw() const + inline std::vector raw() const { return {data(), data() + size()}; } @@ -128,22 +101,8 @@ namespace ccf::crypto return "Pem"; } - static std::vector split_x509_cert_bundle( - const std::string_view& pem) - { - std::string separator("-----END CERTIFICATE-----"); - std::vector 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 split_x509_cert_bundle( + const std::string_view& pem); inline void fill_json_schema(nlohmann::json& schema, const Pem*) { diff --git a/include/ccf/ds/logger.h b/include/ccf/ds/logger.h index 9d677820d960..262b199d87c5 100644 --- a/include/ccf/ds/logger.h +++ b/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; } }; diff --git a/src/clients/perf/perf_client.h b/src/clients/perf/perf_client.h index e7cafce92e7a..54ff124d0e56 100644 --- a/src/clients/perf/perf_client.h +++ b/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; } diff --git a/src/crypto/pem.cpp b/src/crypto/pem.cpp new file mode 100644 index 000000000000..2895c4e0abec --- /dev/null +++ b/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(data), size); + + check_pem_format(); + } + + std::vector split_x509_cert_bundle( + const std::string_view& pem) + { + std::string separator("-----END CERTIFICATE-----"); + std::vector 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; + } +} \ No newline at end of file diff --git a/src/crypto/test/pem.cpp b/src/crypto/test/pem.cpp new file mode 100644 index 000000000000..17b170d1fc74 --- /dev/null +++ b/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 +#include +#include + +using namespace std; +using namespace ccf::crypto; + +TEST_CASE("Split x509 cert bundle") +{ + REQUIRE(split_x509_cert_bundle("") == std::vector{}); + + 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); +} diff --git a/src/ds/test/logger.cpp b/src/ds/test/logger.cpp index 35eec285a981..98c571401f5f 100644 --- a/src/ds/test/logger.cpp +++ b/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]; diff --git a/src/endpoints/endpoint.cpp b/src/endpoints/endpoint.cpp index ca2de45a57b3..279732589fcd 100644 --- a/src/endpoints/endpoint.cpp +++ b/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 { diff --git a/src/host/main.cpp b/src/host/main.cpp index b3c13e018a1b..925fcbe20d61 100644 --- a/src/host/main.cpp +++ b/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(CLI::ExitCodes::ValidationError); } std::vector startup_snapshot = {}; diff --git a/src/host/socket.h b/src/host/socket.h index 5c3bc2d56a24..777ecad6e378 100644 --- a/src/host/socket.h +++ b/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(); } }; diff --git a/src/node/acme_client.h b/src/node/acme_client.h index 56564731178e..6198a155bcfe 100644 --- a/src/node/acme_client.h +++ b/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); } diff --git a/tests/perf-system/submitter/CMakeLists.txt b/tests/perf-system/submitter/CMakeLists.txt index 11cc8ada93be..45e9f9c45d45 100644 --- a/tests/perf-system/submitter/CMakeLists.txt +++ b/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