diff --git a/.azure-pipelines-templates/deploy_aci.yml b/.azure-pipelines-templates/deploy_aci.yml index a5c8353dd8b6..8f59a37b70eb 100644 --- a/.azure-pipelines-templates/deploy_aci.yml +++ b/.azure-pipelines-templates/deploy_aci.yml @@ -1,24 +1,9 @@ jobs: - - job: generate_ssh_key - displayName: "Generate SSH Key" - variables: - Codeql.SkipTaskAutoInjection: true - skipComponentGovernanceDetection: true - pool: - vmImage: ubuntu-20.04 - steps: - - checkout: none - - - template: generate_ssh_key.yml - - job: deploy_primary_aci displayName: "Deploy ACI" - dependsOn: - - generate_ssh_key variables: Codeql.SkipTaskAutoInjection: true skipComponentGovernanceDetection: true - sshKey: $[ dependencies.generate_ssh_key.outputs['generate_ssh_key.sshKey'] ] pool: name: ado-virtual-ccf-sub # To build CCF quickly demands: @@ -31,9 +16,7 @@ jobs: fetchDepth: 0 fetchTags: true - - template: install_ssh_key.yml - parameters: - ssh_key: $(sshKey) + - template: generate_ssh_key.yml - template: azure_cli.yml parameters: @@ -70,7 +53,6 @@ jobs: --memory-gb 64 \ --core-count 16 \ --aci-setup-timeout 300 \ - --aci-private-key-b64 $(sshKey) \ --out ~/aci_ips # Create a ~/ipAddresses files which is a list of ` ` separated by newlines. source ./scripts/azure_deployment/escape_data.sh # Include escape_data to handle newlines. diff --git a/.azure-pipelines-templates/generate_ssh_key.yml b/.azure-pipelines-templates/generate_ssh_key.yml index e99874429b9e..49217f10e396 100644 --- a/.azure-pipelines-templates/generate_ssh_key.yml +++ b/.azure-pipelines-templates/generate_ssh_key.yml @@ -1,7 +1,6 @@ steps: - script: | set -ex + mkdir -p ~/.ssh ssh-keygen -t rsa -b 4096 -f ~/.ssh/id_rsa -N "" - echo "##vso[task.setvariable variable=sshKey;isOutput=true;issecret=true]`base64 -w 0 ~/.ssh/id_rsa`" - name: generate_ssh_key displayName: "Generate SSH Key" diff --git a/.azure-pipelines-templates/install_ssh_key.yml b/.azure-pipelines-templates/install_ssh_key.yml deleted file mode 100644 index 30875af0f81d..000000000000 --- a/.azure-pipelines-templates/install_ssh_key.yml +++ /dev/null @@ -1,11 +0,0 @@ -steps: - - script: | - set -ex - set -o pipefail - mkdir -p ~/.ssh - echo ${{ parameters.ssh_key }} | base64 -d > ~/.ssh/id_rsa - sudo chmod 600 ~/.ssh/id_rsa - sudo ssh-keygen -y -f ~/.ssh/id_rsa > ~/.ssh/id_rsa.pub - sudo chmod 600 ~/.ssh/id_rsa.pub - name: setup_key - displayName: "Install SSH Key from Deployment Step" diff --git a/.github/workflows/long-test.yml b/.github/workflows/long-test.yml index 7a11b7967e6a..a3de2d28c17c 100644 --- a/.github/workflows/long-test.yml +++ b/.github/workflows/long-test.yml @@ -311,7 +311,7 @@ jobs: git config --global --add safe.directory /__w/CCF/CCF mkdir build cd build - cmake -GNinja -DCOMPILE_TARGET=virtual -DCMAKE_BUILD_TYPE=Debug -DLONG_TEST=ON .. + cmake -GNinja -DCOMPILE_TARGET=virtual -DCMAKE_BUILD_TYPE=Debug -DLONG_TESTS=ON .. ninja shell: bash @@ -326,3 +326,109 @@ jobs: # All e2e tests, which are now supported on Mariner. ./tests.sh --timeout 1600 --output-on-failure -LE "benchmark" shell: bash + + long-asan-azure-linux: + if: ${{ contains(github.event.pull_request.labels.*.name, 'run-long-test') || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule'}} + name: "Azure Linux ASAN" + runs-on: [self-hosted, 1ES.Pool=gha-virtual-ccf-sub] + container: + image: mcr.microsoft.com/azurelinux/base/core:3.0 + options: --user root --publish-all --cap-add NET_ADMIN --cap-add NET_RAW --cap-add SYS_PTRACE + + steps: + - name: "Checkout dependencies" + shell: bash + run: | + gpg --import /etc/pki/rpm-gpg/MICROSOFT-RPM-GPG-KEY + tdnf -y update + tdnf -y install ca-certificates git + + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: "Install dependencies" + shell: bash + run: | + set -ex + ./scripts/setup-ci.sh + + - name: "Build Debug (Long Test)" + run: | + set -ex + git config --global --add safe.directory /__w/CCF/CCF + mkdir build + cd build + cmake -GNinja -DCOMPILE_TARGET=virtual -DCMAKE_BUILD_TYPE=Debug -DLONG_TESTS=ON -DSAN=ON .. + ninja + + - name: "Test" + run: | + set +x + cd build + ./tests.sh --output-on-failure --timeout 1600 -LE "benchmark" + + - name: "Upload logs" + if: success() || failure() + uses: actions/upload-artifact@v4 + with: + name: logs-asan-al + path: | + build/workspace/*/*.config.json + build/workspace/*/out + build/workspace/*/err + build/workspace/*.ledger/* + if-no-files-found: ignore + + long-tsan-azure-linux: + if: ${{ contains(github.event.pull_request.labels.*.name, 'run-long-test') || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule'}} + name: "Azure Linux TSAN" + runs-on: [self-hosted, 1ES.Pool=gha-virtual-ccf-sub] + container: + image: mcr.microsoft.com/azurelinux/base/core:3.0 + options: --user root --publish-all --cap-add NET_ADMIN --cap-add NET_RAW --cap-add SYS_PTRACE + + steps: + - name: "Checkout dependencies" + shell: bash + run: | + gpg --import /etc/pki/rpm-gpg/MICROSOFT-RPM-GPG-KEY + tdnf -y update + tdnf -y install ca-certificates git + + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: "Install dependencies" + shell: bash + run: | + set -ex + ./scripts/setup-ci.sh + + - name: "Build Debug (Long Test)" + run: | + set -ex + git config --global --add safe.directory /__w/CCF/CCF + mkdir build + cd build + cmake -GNinja -DCOMPILE_TARGET=virtual -DCMAKE_BUILD_TYPE=Debug -DLONG_TESTS=ON -DTSAN=ON -DWORKER_THREADS=2 .. + ninja + + - name: "Test" + run: | + set +x + cd build + ./tests.sh --output-on-failure --timeout 1600 -LE "benchmark" + + - name: "Upload logs" + if: success() || failure() + uses: actions/upload-artifact@v4 + with: + name: logs-tsan-al + path: | + build/workspace/*/*.config.json + build/workspace/*/out + build/workspace/*/err + build/workspace/*.ledger/* + if-no-files-found: ignore diff --git a/.snpcc_canary b/.snpcc_canary index a2266523e855..8688a7c8705b 100644 --- a/.snpcc_canary +++ b/.snpcc_canary @@ -1,7 +1,7 @@ ___ ___ ___ \_/ - (. =) Y (0 0) (x X) Y (___) + (. =) Y (0 0) (x X) Y (v-v) O \ o | / | /-xXx--//-----x=x--/-xXx--/---x-/--->>>--/ ... /\/\d(-_-)b/\/\ -----vmpl---- +----vmpl---- \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 62549b64f947..f846d448d2fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - `ccf.ledger`/`read_ledger.py` previously enforced too strict a condition on node membership when validating ledger files (#6849). +- Restore low CPU usage on idle nodes, which had increased in dev20 (#6816). ## [6.0.0-dev20] diff --git a/CMakeLists.txt b/CMakeLists.txt index 8c15f5b5ee90..bd3fc5011db8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -716,6 +716,7 @@ if(BUILD_TESTS) ${CMAKE_CURRENT_SOURCE_DIR}/src/ds/test/unit_strings.cpp ${CMAKE_CURRENT_SOURCE_DIR}/src/ds/test/dl_list.cpp ${CMAKE_CURRENT_SOURCE_DIR}/src/ds/test/nonstd.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/src/ds/test/work_beacon.cpp ) target_link_libraries(ds_test PRIVATE ${CMAKE_THREAD_LIBS_INIT}) diff --git a/doc/audit/builtin_maps.rst b/doc/audit/builtin_maps.rst index 5e6da134b70b..632c06d41aa1 100644 --- a/doc/audit/builtin_maps.rst +++ b/doc/audit/builtin_maps.rst @@ -376,9 +376,6 @@ JWT issuers. :project: CCF :members: -.. doxygenenum:: ccf::JwtIssuerKeyFilter - :project: CCF - ``jwt.public_signing_keys`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/doc/build_apps/auth/jwt.rst b/doc/build_apps/auth/jwt.rst index 72c307832c9f..091211896bcc 100644 --- a/doc/build_apps/auth/jwt.rst +++ b/doc/build_apps/auth/jwt.rst @@ -21,7 +21,6 @@ Before adding public token signing keys to a running CCF network, the IdP has to "name": "set_jwt_issuer", "args": { "issuer": "my_issuer", - "key_filter": "all", "auto_refresh": false } } @@ -95,7 +94,6 @@ Now the issuer can be created with auto-refresh enabled: "name": "set_jwt_issuer", "args": { "issuer": "https://login.microsoftonline.com/common/v2.0", - "key_filter": "all", "ca_cert_bundle_name": "jwt_ms", "auto_refresh": true } diff --git a/doc/host_config_schema/cchost_config.json b/doc/host_config_schema/cchost_config.json index 80a34845c4ad..65b4a157ea3c 100644 --- a/doc/host_config_schema/cchost_config.json +++ b/doc/host_config_schema/cchost_config.json @@ -13,15 +13,15 @@ }, "platform": { "type": "string", - "enum": ["SGX", "SNP", "Virtual"], - "default": "SGX", + "enum": ["SNP", "Virtual"], + "default": "SNP", "description": "Trusted Execution Environment platform" }, "type": { "type": "string", "enum": ["Release", "Debug", "Virtual"], "default": "Release", - "description": "Type of enclave application (only if platform is SGX). \"Virtual\" is deprecated (use ``platform`` instead)" + "description": "Type of enclave application (only if platform is SGX, now deprecated). \"Virtual\" is deprecated (use ``platform`` instead)" } }, "description": "This section includes configuration for the enclave application launched by this node", @@ -522,7 +522,7 @@ "description": "List of servers used to retrieve attestation report endorsement certificates (SEV-SNP only). The first server in the list is always used and other servers are only specified as fallback. If set, attestation endorsements from ``--snp-security-context-dir-var`` are ignored, but uvm endorsements from that directory are still used." } }, - "description": "This section includes configuration for the attestation for AMD SEV-SNP platform (ignored for SGX)", + "description": "This section includes configuration for the attestation for AMD SEV-SNP platform.", "additionalProperties": false }, "service_data_json_file": { diff --git a/doc/schemas/gov/2023-06-01-preview/examples/ServiceState_GetJwkInfo.json b/doc/schemas/gov/2023-06-01-preview/examples/ServiceState_GetJwkInfo.json index 1b602bf9ebb5..7bfe7a25ffc4 100644 --- a/doc/schemas/gov/2023-06-01-preview/examples/ServiceState_GetJwkInfo.json +++ b/doc/schemas/gov/2023-06-01-preview/examples/ServiceState_GetJwkInfo.json @@ -9,7 +9,6 @@ "body": { "issuers": { "idprovider.myservice.example.com": { - "keyFilter": "All", "autoRefresh": true, "caCertBundleName": "MyIdProviderCa" } diff --git a/doc/schemas/gov/2024-07-01/examples/ServiceState_GetJwkInfo.json b/doc/schemas/gov/2024-07-01/examples/ServiceState_GetJwkInfo.json index 412834500d65..282f6b4be935 100644 --- a/doc/schemas/gov/2024-07-01/examples/ServiceState_GetJwkInfo.json +++ b/doc/schemas/gov/2024-07-01/examples/ServiceState_GetJwkInfo.json @@ -9,7 +9,6 @@ "body": { "issuers": { "idprovider.myservice.example.com": { - "keyFilter": "All", "autoRefresh": true, "caCertBundleName": "MyIdProviderCa" } diff --git a/doc/schemas/gov/2024-07-01/gov.json b/doc/schemas/gov/2024-07-01/gov.json index c365359ca19d..67c24410430c 100644 --- a/doc/schemas/gov/2024-07-01/gov.json +++ b/doc/schemas/gov/2024-07-01/gov.json @@ -1914,10 +1914,6 @@ "type": "object", "description": "Description of a JWT issuer or identity provider that the current service will trust tokens from.", "properties": { - "keyFilter": { - "$ref": "#/definitions/ServiceState.JwtIssuerKeyFilter", - "description": "Adds restrictions on whether keys should be accepted from this issuer." - }, "keyPolicy": { "type": "object", "description": "Collection of claims which must be present in SGX attestation to permit updates from this issuer.", @@ -1935,34 +1931,9 @@ } }, "required": [ - "keyFilter", "autoRefresh" ] }, - "ServiceState.JwtIssuerKeyFilter": { - "type": "string", - "description": "Possible restrictions on what keys will be accepted from a JWT issuer.", - "enum": [ - "All", - "Sgx" - ], - "x-ms-enum": { - "name": "JwtIssuerKeyFilter", - "modelAsString": true, - "values": [ - { - "name": "All", - "value": "All", - "description": "Accepts any JWT issuer." - }, - { - "name": "Sgx", - "value": "Sgx", - "description": "Only accepts JWTs issued by a token provider running in SGX, which provides a suitable attestation and additional claims." - } - ] - } - }, "ServiceState.Member": { "type": "object", "description": "Information on individual members within a consortium.", diff --git a/doc/schemas/gov_openapi.json b/doc/schemas/gov_openapi.json index d8d5ae3ad7c4..db48581f67af 100644 --- a/doc/schemas/gov_openapi.json +++ b/doc/schemas/gov_openapi.json @@ -271,12 +271,6 @@ }, "type": "object" }, - "JwtIssuerKeyFilter": { - "enum": [ - "all" - ], - "type": "string" - }, "JwtIssuerMetadata": { "properties": { "auto_refresh": { @@ -284,9 +278,6 @@ }, "ca_cert_bundle_name": { "$ref": "#/components/schemas/string" - }, - "key_filter": { - "$ref": "#/components/schemas/JwtIssuerKeyFilter" } }, "type": "object" @@ -1348,7 +1339,7 @@ "info": { "description": "This API is used to submit and query proposals which affect CCF's public governance tables.", "title": "CCF Governance API", - "version": "4.6.0" + "version": "4.6.1" }, "openapi": "3.0.0", "paths": { diff --git a/include/ccf/service/tables/jwt.h b/include/ccf/service/tables/jwt.h index 8b21448bf58a..e1c1afdd867d 100644 --- a/include/ccf/service/tables/jwt.h +++ b/include/ccf/service/tables/jwt.h @@ -21,8 +21,6 @@ namespace ccf struct JwtIssuerMetadata { - /// JWT issuer key filter, kept for compatibility with existing ledgers - JwtIssuerKeyFilter key_filter = JwtIssuerKeyFilter::All; /// Optional CA bundle name used for authentication when auto-refreshing std::optional ca_cert_bundle_name; /// Whether to auto-refresh keys from the issuer @@ -32,7 +30,7 @@ namespace ccf DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(JwtIssuerMetadata); DECLARE_JSON_REQUIRED_FIELDS(JwtIssuerMetadata); DECLARE_JSON_OPTIONAL_FIELDS( - JwtIssuerMetadata, key_filter, ca_cert_bundle_name, auto_refresh); + JwtIssuerMetadata, ca_cert_bundle_name, auto_refresh); using JwtIssuer = std::string; using JwtKeyId = std::string; diff --git a/samples/config/join_config.json b/samples/config/join_config.json index 03bff08d5b3e..194efb4e0aa9 100644 --- a/samples/config/join_config.json +++ b/samples/config/join_config.json @@ -1,7 +1,7 @@ { "enclave": { "file": "libjs_generic.enclave.so.signed", - "platform": "SGX", + "platform": "SNP", "type": "Release" }, "network": { diff --git a/samples/config/minimal_config.json b/samples/config/minimal_config.json index 1bb2ea0c10f7..a73ea76283ab 100644 --- a/samples/config/minimal_config.json +++ b/samples/config/minimal_config.json @@ -1,7 +1,7 @@ { "enclave": { "file": "libjs_generic.enclave.so.signed", - "platform": "SGX", + "platform": "SNP", "type": "Release" }, "network": { diff --git a/samples/config/minimal_config_redirects_role.json b/samples/config/minimal_config_redirects_role.json index 31e1a79e177c..2e3c86c11c0f 100644 --- a/samples/config/minimal_config_redirects_role.json +++ b/samples/config/minimal_config_redirects_role.json @@ -1,7 +1,7 @@ { "enclave": { "file": "libjs_generic.enclave.so.signed", - "platform": "SGX", + "platform": "SNP", "type": "Release" }, "network": { diff --git a/samples/config/minimal_config_redirects_static.json b/samples/config/minimal_config_redirects_static.json index 764d6807f562..21fc08c3bdee 100644 --- a/samples/config/minimal_config_redirects_static.json +++ b/samples/config/minimal_config_redirects_static.json @@ -1,7 +1,7 @@ { "enclave": { "file": "libjs_generic.enclave.so.signed", - "platform": "SGX", + "platform": "SNP", "type": "Release" }, "network": { diff --git a/samples/config/recover_config.json b/samples/config/recover_config.json index 3c3ecb284af9..3dc9445454e4 100644 --- a/samples/config/recover_config.json +++ b/samples/config/recover_config.json @@ -1,7 +1,7 @@ { "enclave": { "file": "libjs_generic.enclave.so.signed", - "platform": "SGX", + "platform": "SNP", "type": "Release" }, "network": { diff --git a/samples/config/start_config.json b/samples/config/start_config.json index 251655ea4a1a..fd8d87183557 100644 --- a/samples/config/start_config.json +++ b/samples/config/start_config.json @@ -1,7 +1,7 @@ { "enclave": { "file": "libjs_generic.enclave.so.signed", - "platform": "SGX", + "platform": "SNP", "type": "Release" }, "network": { diff --git a/scripts/setup-ci.sh b/scripts/setup-ci.sh index ba97eae2fee4..b51b2e52f069 100755 --- a/scripts/setup-ci.sh +++ b/scripts/setup-ci.sh @@ -3,7 +3,9 @@ # Licensed under the Apache 2.0 License. set -ex + H2SPEC_VERSION="v2.6.0" +PEBBLE_VERSION="v2.3.1" # Source control tdnf -y install \ @@ -46,5 +48,13 @@ mkdir /opt/h2spec mv h2spec /opt/h2spec/h2spec rm h2spec_linux_amd64.tar.gz +# acme endorsement tests +mkdir -p /opt/pebble +curl -L --output pebble_linux-amd64 https://github.com/letsencrypt/pebble/releases/download/$PEBBLE_VERSION/pebble_linux-amd64 +mv pebble_linux-amd64 /opt/pebble/pebble_linux-amd64 +curl -L --output pebble-challtestsrv_linux-amd64 https://github.com/letsencrypt/pebble/releases/download/$PEBBLE_VERSION/pebble-challtestsrv_linux-amd64 +mv pebble-challtestsrv_linux-amd64 /opt/pebble/pebble-challtestsrv_linux-amd64 +chmod +x /opt/pebble/pebble_linux-amd64 /opt/pebble/pebble-challtestsrv_linux-amd64 + # For packaging tdnf -y install rpm-build diff --git a/src/ds/notifying.h b/src/ds/notifying.h new file mode 100644 index 000000000000..a5e5ad548090 --- /dev/null +++ b/src/ds/notifying.h @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. +#pragma once + +#include "ring_buffer.h" +#include "work_beacon.h" + +namespace ringbuffer +{ + class NotifyingWriter : public AbstractWriter + { + private: + WriterPtr underlying_writer; + ccf::ds::WorkBeaconPtr work_beacon; + + public: + NotifyingWriter(const WriterPtr& writer, const ccf::ds::WorkBeaconPtr& wb) : + underlying_writer(writer), + work_beacon(wb) + {} + + // After the underlying writer finishes writing a message, notify any + // waiting receivers + void finish(const WriteMarker& marker) override + { + underlying_writer->finish(marker); + work_beacon->notify_work_available(); + } + + // For all other overrides, defer directly to the underlying writer + WriteMarker prepare( + Message m, + size_t size, + bool wait = true, + size_t* identifier = nullptr) override + { + return underlying_writer->prepare(m, size, wait, identifier); + } + + WriteMarker write_bytes( + const WriteMarker& marker, const uint8_t* bytes, size_t size) override + { + return underlying_writer->write_bytes(marker, bytes, size); + } + + size_t get_max_message_size() override + { + return underlying_writer->get_max_message_size(); + } + }; + + class NotifyingWriterFactory : public AbstractWriterFactory + { + private: + AbstractWriterFactory& factory_impl; + + ccf::ds::WorkBeaconPtr outbound_work_beacon; + ccf::ds::WorkBeaconPtr inbound_work_beacon; + + public: + NotifyingWriterFactory(AbstractWriterFactory& impl) : + factory_impl(impl), + outbound_work_beacon(std::make_shared()), + inbound_work_beacon(std::make_shared()) + {} + + ccf::ds::WorkBeaconPtr get_outbound_work_beacon() + { + return outbound_work_beacon; + } + + ccf::ds::WorkBeaconPtr get_inbound_work_beacon() + { + return inbound_work_beacon; + } + + std::shared_ptr create_notifying_writer_to_outside() + { + return std::make_shared( + factory_impl.create_writer_to_outside(), outbound_work_beacon); + } + + std::shared_ptr create_notifying_writer_to_inside() + { + return std::make_shared( + factory_impl.create_writer_to_inside(), inbound_work_beacon); + } + + std::shared_ptr create_writer_to_outside() + override + { + return create_notifying_writer_to_outside(); + } + + std::shared_ptr create_writer_to_inside() + override + { + return create_notifying_writer_to_inside(); + } + }; +} diff --git a/src/ds/test/work_beacon.cpp b/src/ds/test/work_beacon.cpp new file mode 100644 index 000000000000..7e426ee8663b --- /dev/null +++ b/src/ds/test/work_beacon.cpp @@ -0,0 +1,206 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. + +#include "../work_beacon.h" + +#include "ccf/ds/logger.h" + +#include +#include +#include +#include +#include +#include + +using WorkItem = std::function; + +struct WorkQueue +{ + std::mutex mutex; + std::queue work; + + void add_work(WorkItem&& item) + { + std::unique_lock lock(mutex); + work.push(std::move(item)); + } + + std::optional get_work() + { + std::unique_lock lock(mutex); + + std::optional result = std::nullopt; + if (!work.empty()) + { + result = work.front(); + work.pop(); + } + + return result; + } +}; + +// Do nothing, so that callers simply spin-loop +struct NopBeacon +{ + void wait_for_work() {} + void notify_work_available() {} +}; + +// Simulate what should generally be done in production. Always wait with a +// timeout, rather than indefinitely, to handle senders who forget/fail to +// notify. +struct WaitBeacon +{ + ccf::ds::WorkBeacon internal; + + void wait_for_work() + { + internal.wait_for_work_with_timeout(std::chrono::milliseconds(100)); + } + + void notify_work_available() + { + // Sometimes, just forget to notify the receivers + if (rand() % 4 != 0) + { + internal.notify_work_available(); + } + else + { + // Instead, waste so much time that the receivers wake up + std::this_thread::sleep_for(std::chrono::milliseconds(110)); + } + } +}; + +// Run a simple task simulation, with some sender and receiver threads passing +// work items around. Return how often the receivers checked the work queue and +// found it empty. +template +size_t run_jobs(size_t n_senders, size_t n_receivers) +{ + std::vector senders; + std::vector receivers; + + WorkQueue work_queue; + TBeacon beacon; + + std::atomic workless_wakes = 0; + + for (auto i = 0; i < n_senders; ++i) + { + senders.push_back(std::thread( + [&](size_t sender_idx) { + for (auto x = 0; x < 10; ++x) + { + work_queue.add_work([=]() { + std::this_thread::sleep_for(std::chrono::nanoseconds(x * x)); + return false; + }); + beacon.notify_work_available(); + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + + // Add tasks that tells the receiving worker to terminate. Each sender + // is responsible for sending some fraction of terminate tasks, such + // that each receiver receives exactly one. + size_t quota = n_receivers / n_senders; + if (sender_idx == 0) + { + quota += n_receivers % n_senders; + } + for (auto j = 0; j < quota; ++j) + { + work_queue.add_work([&]() { return true; }); + beacon.notify_work_available(); + } + }, + i)); + } + + for (auto i = 0; i < n_receivers; ++i) + { + receivers.push_back(std::thread([&]() { + while (true) + { + beacon.wait_for_work(); + + auto task = work_queue.get_work(); + if (!task.has_value()) + { + ++workless_wakes; + } + else + { + const auto task_ret = task.value()(); + if (task_ret == true) + { + return; + } + } + } + })); + } + + for (auto& sender : senders) + { + sender.join(); + } + + for (auto& receiver : receivers) + { + receiver.join(); + } + + return workless_wakes; +} + +TEST_CASE("WorkBeacon" * doctest::test_suite("workbeacon")) +{ + std::vector test_vals{1, 5, 8}; + for (auto n_senders : test_vals) + { + for (auto n_receivers : test_vals) + { + { + LOG_INFO_FMT( + "Testing {} senders and {} receivers", n_senders, n_receivers); + // Allow test to be repeated multiple times locally to improve + // confidence in the given bounds + static constexpr auto n_repeats = 1; + for (size_t i = 0; i < n_repeats; ++i) + { + const auto wakes_with_spinloop = + run_jobs(n_senders, n_receivers); + const auto wakes_with_waits = + run_jobs(n_senders, n_receivers); + const auto wakes_with_beacon = + run_jobs(n_senders, n_receivers); + + LOG_INFO_FMT( + " {} vs {} vs {}", + wakes_with_beacon, + wakes_with_waits, + wakes_with_spinloop); + + // Spin loop should have hundreds of thousands of idle wake-ups. + // Beacon should have 0 idle wake-ups. + // Beacon with timer should have a handful of idle wake-ups, roughly + // proportional to the ratio of receivers / senders, and the total + // test run-time, and the various ad-hoc random parameters in this + // test. But crucially, drastically fewer than the spin-loop + + // First assert the order + REQUIRE(wakes_with_beacon == 0); + REQUIRE(wakes_with_beacon <= wakes_with_waits); + REQUIRE(wakes_with_waits < wakes_with_spinloop); + + // Then try to be a little more precise, allowing head-room for + // different build configs and cosmic rays + REQUIRE(wakes_with_waits * 10 < wakes_with_spinloop); + } + } + } + } +} diff --git a/src/ds/work_beacon.h b/src/ds/work_beacon.h new file mode 100644 index 000000000000..277f505f19d7 --- /dev/null +++ b/src/ds/work_beacon.h @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. +#pragma once + +#define FMT_HEADER_ONLY +#include +#include +#include +#include +#include + +namespace ccf::ds +{ + class WorkBeacon + { + protected: + std::mutex mutex; + std::condition_variable condition_variable; + size_t work_available = 0; + + public: + void wait_for_work() + { + std::unique_lock lock(mutex); + condition_variable.wait(lock, [this] { return work_available > 0; }); + --work_available; + } + + // Returns true if condition variable indicated work is available, false if + // timeout was reached + template + bool wait_for_work_with_timeout( + const std::chrono::duration& timeout) + { + std::unique_lock lock(mutex); + const auto woke_for_work = condition_variable.wait_for( + lock, timeout, [this] { return work_available > 0; }); + if (woke_for_work) + { + if (work_available > 0) + { + --work_available; + } + } + + return woke_for_work; + } + + void notify_work_available() + { + { + std::lock_guard lock(mutex); + ++work_available; + } + + condition_variable.notify_all(); + } + }; + + using WorkBeaconPtr = std::shared_ptr; +} diff --git a/src/enclave/enclave.h b/src/enclave/enclave.h index 5a8264a1d7d7..3a449abe3fc4 100644 --- a/src/enclave/enclave.h +++ b/src/enclave/enclave.h @@ -10,6 +10,7 @@ #include "ccf/pal/mem.h" #include "crypto/openssl/hash.h" #include "ds/oversized.h" +#include "ds/work_beacon.h" #include "enclave_time.h" #include "indexing/enclave_lfs_access.h" #include "indexing/historical_transaction_fetcher.h" @@ -47,6 +48,7 @@ namespace ccf std::unique_ptr circuit; std::unique_ptr basic_writer_factory; std::unique_ptr writer_factory; + ccf::ds::WorkBeaconPtr work_beacon; RingbufferLogger* ringbuffer_logger = nullptr; ccf::NetworkState network; std::shared_ptr rpc_map; @@ -89,10 +91,12 @@ namespace ccf size_t sig_tx_interval, size_t sig_ms_interval, const ccf::consensus::Configuration& consensus_config, - const ccf::crypto::CurveID& curve_id) : + const ccf::crypto::CurveID& curve_id, + const ccf::ds::WorkBeaconPtr& work_beacon_) : circuit(std::move(circuit_)), basic_writer_factory(std::move(basic_writer_factory_)), writer_factory(std::move(writer_factory_)), + work_beacon(work_beacon_), ringbuffer_logger(ringbuffer_logger_), network(), rpc_map(std::make_shared()), @@ -467,6 +471,11 @@ namespace ccf while (!bp.get_finished()) { + // Wait until the host indicates that some ringbuffer messages are + // available, but wake at least every 100ms to check thread messages + work_beacon->wait_for_work_with_timeout( + std::chrono::milliseconds(100)); + // First, read some messages from the ringbuffer auto read = bp.read_n(max_messages, circuit->read_from_outside()); diff --git a/src/enclave/main.cpp b/src/enclave/main.cpp index 2aef9c196ba3..9390c7f659e3 100644 --- a/src/enclave/main.cpp +++ b/src/enclave/main.cpp @@ -67,7 +67,8 @@ extern "C" StartType start_type, ccf::LoggerLevel enclave_log_level, size_t num_worker_threads, - void* time_location) + void* time_location, + const ccf::ds::WorkBeaconPtr& work_beacon) { std::lock_guard guard(create_lock); @@ -225,7 +226,8 @@ extern "C" cc.ledger_signatures.tx_count, cc.ledger_signatures.delay.count_ms(), cc.consensus, - cc.node_certificate.curve_id); + cc.node_certificate.curve_id, + work_beacon); } catch (const ccf::ccf_oe_attester_init_error& e) { diff --git a/src/enclave/virtual_enclave.h b/src/enclave/virtual_enclave.h index 5e2f010507c8..0dd0e9eee395 100644 --- a/src/enclave/virtual_enclave.h +++ b/src/enclave/virtual_enclave.h @@ -117,7 +117,8 @@ extern "C" StartType start_type, ccf::LoggerLevel enclave_log_level, size_t num_worker_thread, - void* time_location) + void* time_location, + const ccf::ds::WorkBeaconPtr& work_beacon) { using create_node_func_t = CreateNodeStatus (*)( void*, @@ -137,7 +138,8 @@ extern "C" StartType, ccf::LoggerLevel, size_t, - void*); + void*, + const ccf::ds::WorkBeaconPtr&); static create_node_func_t create_node_func = get_enclave_exported_function( @@ -161,7 +163,8 @@ extern "C" start_type, enclave_log_level, num_worker_thread, - time_location); + time_location, + work_beacon); // Only return OE_OK when the error isn't OE related switch (*status) diff --git a/src/host/configuration.h b/src/host/configuration.h index 54edf8e4be7c..4e34f9672cc1 100644 --- a/src/host/configuration.h +++ b/src/host/configuration.h @@ -31,9 +31,7 @@ namespace host }; DECLARE_JSON_ENUM( EnclavePlatform, - {{EnclavePlatform::SGX, "SGX"}, - {EnclavePlatform::SNP, "SNP"}, - {EnclavePlatform::VIRTUAL, "Virtual"}}); + {{EnclavePlatform::SNP, "SNP"}, {EnclavePlatform::VIRTUAL, "Virtual"}}); enum class LogFormat { diff --git a/src/host/enclave.h b/src/host/enclave.h index 5e6ab9adaa7f..6edb5b81186f 100644 --- a/src/host/enclave.h +++ b/src/host/enclave.h @@ -9,34 +9,12 @@ #include #include -#ifdef PLATFORM_SGX -# include -# include -# include -# include -#endif - #if defined(PLATFORM_VIRTUAL) || defined(PLATFORM_SNP) // Include order matters. virtual_enclave.h uses the OE definitions if // available, else creates its own stubs # include "enclave/virtual_enclave.h" #endif -extern "C" -{ -#ifdef PLATFORM_SGX - void nop_oe_logger( - void* context, - bool is_enclave, - const struct tm* t, - long int usecs, - oe_log_level_t level, - uint64_t host_thread_id, - const char* message) - {} -#endif -} - namespace host { void expect_enclave_file_suffix( @@ -88,9 +66,6 @@ namespace host class Enclave { private: -#ifdef PLATFORM_SGX - oe_enclave_t* sgx_handle = nullptr; -#endif #if defined(PLATFORM_VIRTUAL) || defined(PLATFORM_SNP) void* virtual_handle = nullptr; #endif @@ -100,7 +75,6 @@ namespace host * Create an uninitialized enclave hosting the given library. * * @param path Path to signed enclave library file - * @param type Type of enclave to load * @param platform Trusted Execution Platform of enclave, influencing what * flags should be passed to OE, or whether to dlload a virtual enclave */ @@ -114,42 +88,6 @@ namespace host switch (platform) { - case host::EnclavePlatform::SGX: - { -#ifdef PLATFORM_SGX - uint32_t oe_flags = 0; - if (type == host::EnclaveType::DEBUG) - { - expect_enclave_file_suffix(path, ".enclave.so.debuggable", type); - oe_flags |= OE_ENCLAVE_FLAG_DEBUG; - } - else - { - expect_enclave_file_suffix(path, ".enclave.so.signed", type); - } - - auto err = oe_create_ccf_enclave( - path.c_str(), - OE_ENCLAVE_TYPE_SGX, - oe_flags, - nullptr, - 0, - &sgx_handle); - - if (err != OE_OK) - { - throw std::logic_error( - fmt::format("Could not create enclave: {}", oe_result_str(err))); - } -#else - throw std::logic_error(fmt::format( - "SGX enclaves are not supported in current build - cannot launch " - "{}", - path)); -#endif // defined(PLATFORM_SGX) - break; - } - case host::EnclavePlatform::SNP: { #if defined(PLATFORM_SNP) @@ -188,19 +126,6 @@ namespace host ~Enclave() { -#ifdef PLATFORM_SGX - if (sgx_handle != nullptr) - { - auto err = oe_terminate_enclave(sgx_handle); - - if (err != OE_OK) - { - LOG_FAIL_FMT( - "Error while terminating enclave: {}", oe_result_str(err)); - } - } -#endif - #if defined(PLATFORM_SNP) || defined(PLATFORM_VIRTUAL) if (virtual_handle != nullptr) { @@ -218,7 +143,8 @@ namespace host StartType start_type, ccf::LoggerLevel enclave_log_level, size_t num_worker_thread, - void* time_location) + void* time_location, + const ccf::ds::WorkBeaconPtr& work_beacon) { CreateNodeStatus status = CreateNodeStatus::InternalError; constexpr size_t enclave_version_size = 256; @@ -238,7 +164,7 @@ namespace host node_cert.size(), &node_cert_len, service_cert.data(), \ service_cert.size(), &service_cert_len, enclave_version_buf.data(), \ enclave_version_buf.size(), &enclave_version_len, start_type, \ - enclave_log_level, num_worker_thread, time_location + enclave_log_level, num_worker_thread, time_location, work_beacon oe_result_t err = OE_FAILURE; @@ -250,12 +176,6 @@ namespace host err = virtual_create_node(virtual_handle, CREATE_NODE_ARGS); } #endif -#ifdef PLATFORM_SGX - if (sgx_handle != nullptr) - { - err = enclave_create_node(sgx_handle, CREATE_NODE_ARGS); - } -#endif if (err != OE_OK || status != CreateNodeStatus::OK) { @@ -297,12 +217,6 @@ namespace host err = virtual_run(virtual_handle, &ret); } #endif -#ifdef PLATFORM_SGX - if (sgx_handle != nullptr) - { - err = enclave_run(sgx_handle, &ret); - } -#endif if (err != OE_OK) { diff --git a/src/host/ledger.h b/src/host/ledger.h index cbdb62581767..4cc69529d1d7 100644 --- a/src/host/ledger.h +++ b/src/host/ledger.h @@ -246,6 +246,13 @@ namespace asynchost "Failed to read positions offset from ledger file {}", file_path)); } + if (committed && table_offset == 0) + { + throw std::logic_error(fmt::format( + "Committed ledger file {} cannot be read: invalid table offset (0)", + file_path)); + } + total_len = sizeof(positions_offset_header_t); if (from_existing_file) @@ -806,7 +813,10 @@ namespace asynchost catch (const std::exception& e) { LOG_FAIL_FMT( - "Could not open ledger file {} to read seqno {}", match.value(), idx); + "Could not open ledger file {} to read seqno {}: {}", + match.value(), + idx, + e.what()); return nullptr; } diff --git a/src/host/main.cpp b/src/host/main.cpp index 86f2db1962e9..a7f2dffe4d31 100644 --- a/src/host/main.cpp +++ b/src/host/main.cpp @@ -14,6 +14,7 @@ #include "ds/files.h" #include "ds/non_blocking.h" #include "ds/nonstd.h" +#include "ds/notifying.h" #include "ds/oversized.h" #include "enclave.h" #include "handle_ring_buffer.h" @@ -215,10 +216,12 @@ int main(int argc, char** argv) { if (config.command.type == StartType::Start) { - if (files::exists(config.ledger.directory)) + if ( + files::exists(config.ledger.directory) && + !fs::is_empty(config.ledger.directory)) { throw std::logic_error(fmt::format( - "On start, ledger directory should not exist ({})", + "On start, ledger directory should not exist or be empty ({})", config.ledger.directory)); } @@ -371,10 +374,15 @@ int main(int argc, char** argv) ringbuffer::Circuit circuit(to_enclave_def, from_enclave_def); messaging::BufferProcessor bp("Host"); + ringbuffer::WriterFactory base_factory(circuit); + + // To avoid polling an idle ringbuffer, all writes are paired with a + // condition_variable notification, which readers may wait on + ringbuffer::NotifyingWriterFactory notifying_factory(base_factory); + // To prevent deadlock, all blocking writes from the host to the ringbuffer // will be queued if the ringbuffer is full - ringbuffer::WriterFactory base_factory(circuit); - ringbuffer::NonBlockingWriterFactory non_blocking_factory(base_factory); + ringbuffer::NonBlockingWriterFactory non_blocking_factory(notifying_factory); // Factory for creating writers which will handle writing of large messages oversized::WriterConfig writer_config{ @@ -830,7 +838,8 @@ int main(int argc, char** argv) config.command.type, enclave_log_level, config.worker_threads, - time_updater->behaviour.get_value()); + time_updater->behaviour.get_value(), + notifying_factory.get_inbound_work_beacon()); ecall_completed.store(true); flusher_thread.join(); diff --git a/src/kv/apply_changes.h b/src/kv/apply_changes.h index 841878c8378c..52bb01606f34 100644 --- a/src/kv/apply_changes.h +++ b/src/kv/apply_changes.h @@ -59,15 +59,8 @@ namespace ccf::kv for (auto it = changes.begin(); it != changes.end(); ++it) { - bool changeset_has_writes = it->second.changeset->has_writes(); - if (changeset_has_writes) - { - has_writes = true; - } - if (changeset_has_writes || track_read_versions) - { - it->second.map->lock(); - } + has_writes |= it->second.changeset->has_writes(); + it->second.map->lock(); } bool ok = true; @@ -177,10 +170,7 @@ namespace ccf::kv for (auto it = changes.begin(); it != changes.end(); ++it) { - if (it->second.changeset->has_writes() || track_read_versions) - { - it->second.map->unlock(); - } + it->second.map->unlock(); } if (!ok) diff --git a/src/node/rpc/member_frontend.h b/src/node/rpc/member_frontend.h index 2ba72af23780..83f36a1fabd8 100644 --- a/src/node/rpc/member_frontend.h +++ b/src/node/rpc/member_frontend.h @@ -612,7 +612,7 @@ namespace ccf openapi_info.description = "This API is used to submit and query proposals which affect CCF's " "public governance tables."; - openapi_info.document_version = "4.6.0"; + openapi_info.document_version = "4.6.1"; } static std::optional get_caller_member_id( diff --git a/src/snapshots/filenames.h b/src/snapshots/filenames.h index bba801d6e4b5..8b96aac4a456 100644 --- a/src/snapshots/filenames.h +++ b/src/snapshots/filenames.h @@ -152,6 +152,12 @@ namespace snapshots continue; } + if (fs::exists(f.path()) && fs::is_empty(f.path())) + { + LOG_INFO_FMT("Ignoring empty snapshot file {}", file_name); + continue; + } + auto snapshot_idx = get_snapshot_idx_from_file_name(file_name); if (snapshot_idx > latest_committed_snapshot_idx) { diff --git a/tests/acme_endorsement.py b/tests/acme_endorsement.py index eb4944959afb..2cd322b0333f 100644 --- a/tests/acme_endorsement.py +++ b/tests/acme_endorsement.py @@ -73,7 +73,7 @@ def wait_for_certificates( for node in network.nodes: iface = node.host.rpc_interfaces[interface_name] try: - context = ssl.SSLContext(ssl.PROTOCOL_TLSv1_2) + context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) context.verify_mode = ssl.CERT_REQUIRED context.check_hostname = True context.load_verify_locations(cadata=root_cert) diff --git a/tests/e2e_operations.py b/tests/e2e_operations.py index 7526817c227b..8f851ef496a5 100644 --- a/tests/e2e_operations.py +++ b/tests/e2e_operations.py @@ -22,6 +22,7 @@ import time import http import copy +import struct import infra.snp as snp from cryptography import x509 from cryptography.hazmat.backends import default_backend @@ -308,6 +309,40 @@ def test_snapshot_access(network, args): assert err_msg in r.body.json()["error"]["message"], r +def test_empty_snapshot(network, args): + + LOG.info("Check that empty snapshot is ignored") + + with tempfile.TemporaryDirectory() as snapshots_dir: + LOG.debug(f"Using {snapshots_dir} as snapshots directory") + + snapshot_name = "snapshot_1000_1500.committed" + + with open( + os.path.join(snapshots_dir, snapshot_name), "wb+" + ) as temp_empty_snapshot: + + LOG.debug(f"Created empty snapshot {temp_empty_snapshot.name}") + + # Check the file is indeed empty + assert ( + os.stat(temp_empty_snapshot.name).st_size == 0 + ), temp_empty_snapshot.name + + # Create new node and join network + new_node = network.create_node("local://localhost") + network.join_node(new_node, args.package, args, snapshots_dir=snapshots_dir) + new_node.stop() + + # Check that the empty snapshot is correctly skipped + if not new_node.check_log_for_error_message( + f"Ignoring empty snapshot file {snapshot_name}" + ): + raise AssertionError( + f"Expected empty snapshot file {snapshot_name} to be skipped in node logs" + ) + + def split_all_ledger_files_in_dir(input_dir, output_dir): # A ledger file can only be split at a seqno that contains a signature # (so that all files end on a signature that verifies their integrity). @@ -396,6 +431,7 @@ def run_file_operations(args): test_forced_snapshot(network, args) test_large_snapshot(network, args) test_snapshot_access(network, args) + test_empty_snapshot(network, args) primary, _ = network.find_primary() # Scoped transactions are not handled by historical range queries @@ -750,6 +786,179 @@ def run_cose_signatures_config_check(args): ), f"Failed to get receipt for txid {txid} after {max_retries} retries" +def run_late_mounted_ledger_check(args): + nargs = copy.deepcopy(args) + nargs.nodes = infra.e2e_args.min_nodes(nargs, f=0) + + with infra.network.network( + nargs.nodes, + nargs.binary_dir, + nargs.debug_nodes, + nargs.perf_nodes, + pdb=nargs.pdb, + ) as network: + network.start_and_open( + nargs, + ) + + primary, _ = network.find_primary() + + msg_id = 42 + msg = str(random.random()) + + # Write a new entry + with primary.client("user0") as c: + r = c.post( + "/app/log/private", + body={"id": msg_id, "msg": msg}, + ) + assert r.status_code == http.HTTPStatus.OK.value, r + c.wait_for_commit(r) + + msg_seqno = r.seqno + msg_tx_id = f"{r.view}.{r.seqno}" + + def try_historical_fetch(node, timeout=1): + with node.client("user0") as c: + start_time = time.time() + while time.time() < (start_time + timeout): + r = c.get( + f"/app/log/private/historical?id={msg_id}", + headers={infra.clients.CCF_TX_ID_HEADER: msg_tx_id}, + ) + if r.status_code == http.HTTPStatus.OK: + assert r.body.json()["msg"] == msg + return True + assert r.status_code == http.HTTPStatus.ACCEPTED + time.sleep(0.2) + return False + + # Confirm this can be retrieved with a historical query + assert try_historical_fetch(primary) + + expected_errors = [] + + # Create a temporary directory to manually construct a ledger in + with tempfile.TemporaryDirectory() as temp_dir: + new_node = network.create_node("local://localhost") + network.join_node( + new_node, + nargs.package, + nargs, + from_snapshot=True, + copy_ledger=False, + common_read_only_ledger_dir=temp_dir, # New node will try to read from temp directory + ) + network.trust_node(new_node, args) + + # Due to copy_ledger=False, this new node cannot access this historical entry + assert not try_historical_fetch(new_node) + expected_errors.append(f"Cannot find ledger file for seqno {msg_seqno}") + + # Gather the source files that the operator should backfill + src_ledger_dir = primary.remote.ledger_paths()[0] + dst_files = { + os.path.join(temp_dir, filename): os.path.join(src_ledger_dir, filename) + for filename in os.listdir(src_ledger_dir) + } + + # Create empy files in the new node's directory, with the correct names + for dst_path in dst_files.keys(): + with open(dst_path, "wb") as f: + pass + + # Historical query still fails, but node survives + assert not try_historical_fetch(new_node) + expected_errors.append("Failed to read positions offset from ledger file") + + # Create files of the correct size, but filled with zeros + for dst_path, src_path in dst_files.items(): + with open(dst_path, "wb") as f: + f.write(bytes(os.path.getsize(src_path))) + + # Historical query still fails, but node survives + assert not try_historical_fetch(new_node) + expected_errors.append("cannot be read: invalid table offset (0)") + + # Write an invalid table offset at the start of each file + for dst_path, src_path in dst_files.items(): + with open(dst_path, "r+b") as f: + f.seek(0) + size = os.path.getsize(src_path) + f.write(struct.pack(" 0 + ) + curves_for_eddsa = ( + ["curve25519", "x25519"] if not running_under_asan else ["curve25519"] + ) + for curve in curves_for_eddsa: priv_pem, pub_pem = infra.crypto.generate_eddsa_keypair(curve) # Private ref_priv_jwk = jwk.JWK.from_pem(priv_pem.encode()).export_private(as_dict=True)