Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some OE/SGX cleanup, prompted by things noticed in the 5.x branch #6889

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,10 @@ if(KV_STATE_RB)
endif()

# This option controls whether to link virtual builds against snmalloc rather
# than use the system allocator. In builds using Open Enclave, enclave
# allocation is managed separately and enabling snmalloc is done by linking
# openenclave::oesnmalloc
option(USE_SNMALLOC "Link virtual build against snmalloc" ON)
# than use the system allocator.
option(USE_SNMALLOC "Link against snmalloc" ON)

# Default inherited from Open Enclave usage
# Useful for debugging with libc++ hardening options
option(USE_LIBCXX "Use libc++ instead of libstdc++" OFF)

enable_language(ASM)
Expand Down
16 changes: 0 additions & 16 deletions include/ccf/ds/ccf_exception.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,6 @@ namespace ccf
std::string result;
};

class ccf_oe_attester_init_error : public ccf_logic_error
{
public:
ccf_oe_attester_init_error(const std::string& what_arg) :
ccf_logic_error(what_arg)
{}
};

class ccf_oe_verifier_init_error : public ccf_logic_error
{
public:
ccf_oe_verifier_init_error(const std::string& what_arg) :
ccf_logic_error(what_arg)
{}
};

class ccf_openssl_rdrand_init_error : public ccf_logic_error
{
public:
Expand Down
3 changes: 1 addition & 2 deletions include/ccf/pal/attestation.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ namespace ccf::pal
j["report_data"].get<std::vector<uint8_t>>());
}

// Verifying SNP attestation report is available on all platforms as unlike
// SGX, this does not require external dependencies (Open Enclave for SGX).
// Verifying SNP attestation report is available on all platforms.
static void verify_snp_attestation_report(
const QuoteInfo& quote_info,
PlatformAttestationMeasurement& measurement,
Expand Down
88 changes: 0 additions & 88 deletions include/ccf/pal/enclave.h

This file was deleted.

22 changes: 4 additions & 18 deletions src/common/enclave_interface_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,17 @@ enum CreateNodeStatus
/** Enclave initialisation failed */
EnclaveInitFailed = 6,

/** Open Enclave Verifier initialisation failed */
OEVerifierInitFailed = 7,

/** Open Enclave Attester initialisation failed */
OEAttesterInitFailed = 8,

/** OpenSSL RDRAND Init Failed */
OpenSSLRDRANDInitFailed = 9,
OpenSSLRDRANDInitFailed = 7,

/** The reconfiguration method is not supported */
ReconfigurationMethodNotSupported = 10,
ReconfigurationMethodNotSupported = 8,

/** Host and enclave versions must match */
VersionMismatch = 11,
VersionMismatch = 9,

/** When reading from host memory, the source must be 8-byte aligned **/
UnalignedArguments = 12,
UnalignedArguments = 10,
};

constexpr char const* create_node_result_to_str(CreateNodeStatus result)
Expand Down Expand Up @@ -78,14 +72,6 @@ constexpr char const* create_node_result_to_str(CreateNodeStatus result)
{
return "EnclaveInitFailed";
}
case CreateNodeStatus::OEVerifierInitFailed:
{
return "OEVerifierInitFailed";
}
case CreateNodeStatus::OEAttesterInitFailed:
{
return "OEAttesterInitFailed";
}
case CreateNodeStatus::OpenSSLRDRANDInitFailed:
{
return "OpenSSLRDRANDInitFailed";
Expand Down
10 changes: 0 additions & 10 deletions src/enclave/enclave.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "ccf/js/core/context.h"
#include "ccf/node_context.h"
#include "ccf/node_subsystem_interface.h"
#include "ccf/pal/enclave.h"
#include "ccf/pal/mem.h"
#include "crypto/openssl/hash.h"
#include "ds/oversized.h"
Expand Down Expand Up @@ -35,7 +34,6 @@
#include "ringbuffer_logger.h"
#include "rpc_map.h"
#include "rpc_sessions.h"
#include "verify.h"

#include <openssl/engine.h>

Expand Down Expand Up @@ -101,14 +99,8 @@ namespace ccf
rpc_map(std::make_shared<RPCMap>()),
rpcsessions(std::make_shared<RPCSessions>(*writer_factory, rpc_map))
{
ccf::pal::initialize_enclave();
ccf::initialize_verifiers();
ccf::crypto::openssl_sha256_init();

// https://github.com/microsoft/CCF/issues/5569
// Open Enclave with OpenSSL 3.x (default for SGX) is built with RDCPU
// (https://github.com/openenclave/openenclave/blob/master/docs/OpenSSLSupport.md#how-to-use-rand-apis)
// and so does not need to make use of the (deprecated) ENGINE_x API.
#if !(defined(OPENSSL_VERSION_MAJOR) && OPENSSL_VERSION_MAJOR >= 3)
// From
// https://software.intel.com/content/www/us/en/develop/articles/how-to-use-the-rdrand-engine-in-openssl-for-random-number-generation.html
Expand Down Expand Up @@ -217,8 +209,6 @@ namespace ccf
}
#endif
LOG_TRACE_FMT("Shutting down enclave");
ccf::shutdown_verifiers();
ccf::pal::shutdown_enclave();
ccf::crypto::openssl_sha256_shutdown();
}

Expand Down
79 changes: 0 additions & 79 deletions src/enclave/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#include "ccf/ds/ccf_exception.h"
#include "ccf/ds/json.h"
#include "ccf/ds/logger.h"
#include "ccf/pal/enclave.h"
#include "ccf/pal/locking.h"
#include "ccf/version.h"
#include "common/enclave_interface_types.h"
Expand All @@ -30,25 +29,6 @@ std::chrono::microseconds ccf::Channel::min_gap_between_initiation_attempts(

extern "C"
{
// Confirming in-enclave behaviour in separate unit tests is tricky, so we
// do final sanity checks on some basic behaviour here, on every enclave
// launch.
void enclave_sanity_checks()
{
{
ccf::pal::Mutex m;
m.lock();
if (m.try_lock())
{
LOG_FATAL_FMT("Able to lock mutex multiple times");
abort();
}
m.unlock();
}

LOG_DEBUG_FMT("All sanity check tests passed");
}

CreateNodeStatus enclave_create_node(
void* enclave_config,
uint8_t* ccf_config,
Expand Down Expand Up @@ -77,12 +57,6 @@ extern "C"
return CreateNodeStatus::NodeAlreadyCreated;
}

if (!ccf::pal::is_outside_enclave(enclave_config, sizeof(EnclaveConfig)))
{
LOG_FAIL_FMT("Memory outside enclave: enclave_config");
return CreateNodeStatus::MemoryNotOutsideEnclave;
}

EnclaveConfig ec = *static_cast<EnclaveConfig*>(enclave_config);

// Setup logger to allow enclave logs to reach the host before node is
Expand All @@ -101,30 +75,12 @@ extern "C"
auto writer_factory = std::make_unique<oversized::WriterFactory>(
*basic_writer_factory, ec.writer_config);

// Check that ringbuffer memory ranges are entirely outside of the enclave
if (
!ccf::pal::is_outside_enclave(
ec.from_enclave_buffer_start, ec.from_enclave_buffer_size) ||
!ccf::pal::is_outside_enclave(
ec.to_enclave_buffer_start, ec.to_enclave_buffer_size) ||
!ccf::pal::is_outside_enclave(
ec.to_enclave_buffer_offsets, sizeof(ringbuffer::Offsets)) ||
!ccf::pal::is_outside_enclave(
ec.from_enclave_buffer_offsets, sizeof(ringbuffer::Offsets)))
{
return CreateNodeStatus::MemoryNotOutsideEnclave;
}

// Note: because logger uses ringbuffer, logger can only be initialised once
// ringbuffer memory has been verified
auto new_logger = std::make_unique<ccf::RingbufferLogger>(*writer_factory);
auto ringbuffer_logger = new_logger.get();
ccf::logger::config::loggers().push_back(std::move(new_logger));

ccf::pal::redirect_platform_logging();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that function can be removed too.


enclave_sanity_checks();

{
auto ccf_version_string = std::string(ccf::ccf_version);
if (ccf_version_string.size() > enclave_version_size)
Expand Down Expand Up @@ -152,33 +108,10 @@ extern "C"
// threads are known
threading::ThreadMessaging::init(num_pending_threads);

// Check that where we expect arguments to be in host-memory, they
// really are. lfence after these checks to prevent speculative
// execution
if (!ccf::pal::is_outside_enclave(
time_location, sizeof(*ccf::enclavetime::host_time_us)))
{
LOG_FAIL_FMT("Memory outside enclave: time_location");
return CreateNodeStatus::MemoryNotOutsideEnclave;
}

ccf::enclavetime::host_time_us =
static_cast<decltype(ccf::enclavetime::host_time_us)>(time_location);
}

if (!ccf::pal::is_outside_enclave(ccf_config, ccf_config_size))
{
LOG_FAIL_FMT("Memory outside enclave: ccf_config");
return CreateNodeStatus::MemoryNotOutsideEnclave;
}

if (!ccf::pal::is_outside_enclave(
startup_snapshot_data, startup_snapshot_size))
{
LOG_FAIL_FMT("Memory outside enclave: startup snapshot");
return CreateNodeStatus::MemoryNotOutsideEnclave;
}

ccf::StartupConfig cc =
nlohmann::json::parse(ccf_config, ccf_config + ccf_config_size);

Expand Down Expand Up @@ -228,18 +161,6 @@ extern "C"
cc.node_certificate.curve_id,
work_beacon);
}
catch (const ccf::ccf_oe_attester_init_error& e)
{
LOG_FAIL_FMT(
"ccf_oe_attester_init_error during enclave init: {}", e.what());
return CreateNodeStatus::OEAttesterInitFailed;
}
catch (const ccf::ccf_oe_verifier_init_error& e)
{
LOG_FAIL_FMT(
"ccf_oe_verifier_init_error during enclave init: {}", e.what());
return CreateNodeStatus::OEVerifierInitFailed;
}
catch (const ccf::ccf_openssl_rdrand_init_error& e)
{
LOG_FAIL_FMT(
Expand Down
33 changes: 0 additions & 33 deletions src/enclave/verify.h

This file was deleted.

2 changes: 0 additions & 2 deletions src/enclave/virtual_enclave.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,6 @@ extern "C"
// Only return OE_OK when the error isn't OE related
switch (*status)
{
case CreateNodeStatus::OEAttesterInitFailed:
case CreateNodeStatus::OEVerifierInitFailed:
case CreateNodeStatus::EnclaveInitFailed:
case CreateNodeStatus::MemoryNotOutsideEnclave:
return OE_FAILURE;
Expand Down
Loading