diff --git a/CHANGELOG.md b/CHANGELOG.md index 49dd1b0c0c78..e7d155277134 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - C++ endpoints can now use `ccf::endpoints::Endpoint::add_openapi_response()` to document additional HTTP responses in their generated OpenAPI schema without changing the endpoint's primary success response (#8115). +- New `ledger.max_transaction_size` node configuration option (default `32MB`), which caps the total serialised size of transactions written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is now rejected with `413 Payload Too Large` and error code `TransactionTooLarge`, and subsequent transactions are unaffected, where previously an excessively large transaction could terminate the node. Reserved internal signature transactions are exempt because they must fill their reserved ledger version. The limit applies only to newly serialised non-reserved transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable. It must be smaller than `memory.max_msg_size` by at least the ring-buffer range response overhead, which is validated at node startup and by `--check` (#7992). ### Changed diff --git a/doc/host_config_schema/host_config.json b/doc/host_config_schema/host_config.json index 8183c55d664a..781cad582693 100644 --- a/doc/host_config_schema/host_config.json +++ b/doc/host_config_schema/host_config.json @@ -477,6 +477,11 @@ "type": "string", "default": "5MB", "description": "Minimum size (size string) of the current ledger file after which a new ledger file (chunk) is created" + }, + "max_transaction_size": { + "type": "string", + "default": "32MB", + "description": "Maximum total serialised size (size string) of a transaction written to the ledger. Reserved internal signature transactions are exempt. This covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. Must be smaller than 'memory.max_msg_size', by at least the ring-buffer range response overhead, so that a single ledger entry can be read back from the host" } }, "description": "This section includes configuration for the ledger directories and files", diff --git a/include/ccf/ds/unit_strings.h b/include/ccf/ds/unit_strings.h index db0b7786813d..ee0919a0b7cb 100644 --- a/include/ccf/ds/unit_strings.h +++ b/include/ccf/ds/unit_strings.h @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -81,7 +82,27 @@ namespace ccf::ds return UnitStringConverter::convert( input, size_suffix_to_power, [](size_t value, size_t power) { - return value * std::pow(1024, power); + constexpr size_t base = 1024; + size_t factor = 1; + for (size_t i = 0; i < power; ++i) + { + if (factor > std::numeric_limits::max() / base) + { + throw std::logic_error("Size string unit multiplier is too large"); + } + factor *= base; + } + + if (value > std::numeric_limits::max() / factor) + { + throw std::logic_error(fmt::format( + "Size string value {} with multiplier {} exceeds the largest " + "representable size", + value, + factor)); + } + + return value * factor; }); } diff --git a/include/ccf/node/startup_config.h b/include/ccf/node/startup_config.h index 3d6cef413798..190fa236c289 100644 --- a/include/ccf/node/startup_config.h +++ b/include/ccf/node/startup_config.h @@ -48,6 +48,7 @@ namespace ccf std::string directory = "ledger"; std::vector read_only_directories; ccf::ds::SizeString chunk_size = {"5MB"}; + ccf::ds::SizeString max_transaction_size = {"32MB"}; bool operator==(const Ledger&) const = default; }; diff --git a/include/ccf/odata_error.h b/include/ccf/odata_error.h index b0bfa65a3675..d3221d05993b 100644 --- a/include/ccf/odata_error.h +++ b/include/ccf/odata_error.h @@ -100,6 +100,7 @@ namespace ccf ERROR(TransactionNotFound) ERROR(TransactionCommitAttemptsExceedLimit) ERROR(TransactionReplicationFailed) + ERROR(TransactionTooLarge) ERROR(UnknownCertificate) ERROR(VoteNotFound) ERROR(VoteAlreadyExists) diff --git a/src/common/configuration.h b/src/common/configuration.h index d11b775b26e6..bbf9c53ce82d 100644 --- a/src/common/configuration.h +++ b/src/common/configuration.h @@ -65,7 +65,11 @@ namespace ccf DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(CCFConfig::Ledger); DECLARE_JSON_REQUIRED_FIELDS(CCFConfig::Ledger); DECLARE_JSON_OPTIONAL_FIELDS( - CCFConfig::Ledger, directory, read_only_directories, chunk_size); + CCFConfig::Ledger, + directory, + read_only_directories, + chunk_size, + max_transaction_size); DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(CCFConfig::LedgerSignatures); DECLARE_JSON_REQUIRED_FIELDS(CCFConfig::LedgerSignatures); diff --git a/src/consensus/aft/raft.h b/src/consensus/aft/raft.h index 6842946a80e4..9056c818e5ff 100644 --- a/src/consensus/aft/raft.h +++ b/src/consensus/aft/raft.h @@ -1307,7 +1307,22 @@ namespace aft { // If the current entry has already been deserialised, skip the // payload for that entry - ledger->skip_entry(data, size); + try + { + ledger->skip_entry(data, size); + } + catch (const std::logic_error& e) + { + // This should only fail if there is malformed data. + RAFT_FAIL_FMT( + "Recv {} to {} from {} but the data is malformed: {}", + r.msg, + state->node_id, + from, + e.what()); + send_append_entries_response_nack(from); + return; + } continue; } } diff --git a/src/consensus/aft/test/enclave.cpp b/src/consensus/aft/test/enclave.cpp index dee7e1f3c672..590ded913bca 100644 --- a/src/consensus/aft/test/enclave.cpp +++ b/src/consensus/aft/test/enclave.cpp @@ -13,6 +13,43 @@ using namespace consensus; using WFactory = ringbuffer::WriterFactory; +TEST_CASE("Enclave rejects malformed entries") +{ + const auto check_rejected = [](const std::vector& entry) { + { + const auto* data = entry.data(); + auto size = entry.size(); + REQUIRE_THROWS_AS(LedgerEnclave::get_entry(data, size), std::logic_error); + } + + { + const auto* data = entry.data(); + auto size = entry.size(); + REQUIRE_THROWS_AS( + LedgerEnclave::skip_entry(data, size), std::logic_error); + } + }; + + SUBCASE("Truncated header") + { + check_rejected( + std::vector(ccf::kv::serialised_entry_header_size - 1)); + } + + SUBCASE("Claimed body exceeds buffer") + { + ccf::kv::SerialisedEntryHeader header; + header.set_size(2); + + std::vector entry(ccf::kv::serialised_entry_header_size + 1); + auto* data = entry.data(); + auto size = entry.size(); + serialized::write(data, size, header); + + check_rejected(entry); + } +} + TEST_CASE("Enclave put") { constexpr auto buffer_size = 1024; diff --git a/src/consensus/ledger_enclave.h b/src/consensus/ledger_enclave.h index 15a1a6df6e0c..c8e908196807 100644 --- a/src/consensus/ledger_enclave.h +++ b/src/consensus/ledger_enclave.h @@ -8,10 +8,49 @@ #include "kv/kv_types.h" #include "kv/serialised_entry_format.h" +#include + namespace consensus { class LedgerEnclave { + private: + static size_t get_entry_size(const uint8_t* data, size_t size) + { + if (size < ccf::kv::serialised_entry_header_size) + { + throw std::logic_error(fmt::format( + "Cannot read transaction header: buffer contains {} bytes, but the " + "fixed ledger entry header requires {} bytes", + size, + ccf::kv::serialised_entry_header_size)); + } + + const auto header = + serialized::peek(data, size); + const size_t body_size = header.size; + const auto available_body_size = + size - ccf::kv::serialised_entry_header_size; + + // The size in the entry header is not trusted: check it against the + // buffer we were given before allocating. This is distinct from the + // configured max_transaction_size, which applies only when serialising + // new transactions, so that entries written under a larger or unset + // limit can always be read back. + if (body_size > available_body_size) + { + throw std::logic_error(fmt::format( + "Cannot read transaction with serialised body size {} bytes from " + "buffer containing {} bytes after the fixed {}-byte ledger entry " + "header", + body_size, + available_body_size, + ccf::kv::serialised_entry_header_size)); + } + + return ccf::kv::serialised_entry_header_size + body_size; + } + public: /** * Retrieve a single entry, advancing offset to the next entry. @@ -23,9 +62,7 @@ namespace consensus */ static std::vector get_entry(const uint8_t*& data, size_t& size) { - auto header = - serialized::peek(data, size); - size_t entry_size = ccf::kv::serialised_entry_header_size + header.size; + const auto entry_size = get_entry_size(data, size); std::vector entry(data, data + entry_size); serialized::skip(data, size, entry_size); return entry; @@ -89,9 +126,7 @@ namespace consensus */ static void skip_entry(const uint8_t*& data, size_t& size) { - auto header = - serialized::read(data, size); - serialized::skip(data, size, header.size); + serialized::skip(data, size, get_entry_size(data, size)); } /** diff --git a/src/consensus/ledger_enclave_types.h b/src/consensus/ledger_enclave_types.h index 6464092de516..73426d12bf6d 100644 --- a/src/consensus/ledger_enclave_types.h +++ b/src/consensus/ledger_enclave_types.h @@ -8,6 +8,8 @@ namespace consensus { + static constexpr size_t ledger_range_response_metadata_size = 2048; + using Index = uint64_t; enum LedgerRequestPurpose : uint8_t diff --git a/src/ds/test/unit_strings.cpp b/src/ds/test/unit_strings.cpp index 1488fe3e0afb..a73d041e6b8c 100644 --- a/src/ds/test/unit_strings.cpp +++ b/src/ds/test/unit_strings.cpp @@ -5,6 +5,7 @@ #include #include +#include using namespace ccf::ds; @@ -29,6 +30,19 @@ TEST_CASE("Size strings" * doctest::test_suite("unit strings")) REQUIRE(convert_size_string("3GB") == 3 * std::pow(1024, 3)); REQUIRE(convert_size_string("3TB") == 3 * std::pow(1024, 4)); REQUIRE(convert_size_string("3PB") == 3 * std::pow(1024, 5)); + + const auto max_size = std::numeric_limits::max(); + REQUIRE(convert_size_string(std::to_string(max_size) + "B") == max_size); + + const auto max_kb_value = max_size / 1024; + REQUIRE( + convert_size_string(std::to_string(max_kb_value) + "KB") == + max_kb_value * 1024); + REQUIRE_THROWS_AS( + convert_size_string(std::to_string(max_kb_value + 1) + "KB"), + std::logic_error); + REQUIRE_THROWS_AS( + convert_size_string(std::to_string(max_size) + "PB"), std::logic_error); } TEST_CASE("Time strings" * doctest::test_suite("unit strings")) diff --git a/src/enclave/enclave.h b/src/enclave/enclave.h index 5e864b7f9e0e..20d295b956cb 100644 --- a/src/enclave/enclave.h +++ b/src/enclave/enclave.h @@ -83,6 +83,7 @@ namespace ccf size_t sig_tx_interval, size_t sig_ms_interval, size_t chunk_threshold, + size_t max_transaction_size, const ccf::consensus::Configuration& consensus_config, const ccf::crypto::CurveID& curve_id, ccf::ds::WorkBeaconPtr work_beacon_, @@ -101,6 +102,7 @@ namespace ccf network.tables->set_chunker( std::make_shared(chunk_threshold)); + network.tables->set_max_transaction_size(max_transaction_size); LOG_TRACE_FMT("Creating node"); node = std::make_unique( diff --git a/src/enclave/main.cpp b/src/enclave/main.cpp index fe1b11530989..b829d915e005 100644 --- a/src/enclave/main.cpp +++ b/src/enclave/main.cpp @@ -106,6 +106,7 @@ namespace ccf ccf_config.ledger_signatures.tx_count, ccf_config.ledger_signatures.delay.count_ms(), ccf_config.ledger.chunk_size, + ccf_config.ledger.max_transaction_size, ccf_config.consensus, ccf_config.node_certificate.curve_id, work_beacon, diff --git a/src/host/ledger.h b/src/host/ledger.h index dde825ab6cc5..6cddfbca63fd 100644 --- a/src/host/ledger.h +++ b/src/host/ledger.h @@ -1885,9 +1885,8 @@ namespace asynchost // Ledger entries response has metadata so cap total entries size // accordingly - constexpr size_t write_ledger_range_response_metadata_size = 2048; auto max_entries_size = to_enclave->get_max_message_size() - - write_ledger_range_response_metadata_size; + ::consensus::ledger_range_response_metadata_size; if (is_in_committed_file(to_idx)) { diff --git a/src/host/run.cpp b/src/host/run.cpp index 68eb671002a8..24a2ec7ccabb 100644 --- a/src/host/run.cpp +++ b/src/host/run.cpp @@ -21,6 +21,7 @@ #include "common/enclave_interface_types.h" #include "config_schema.h" #include "configuration.h" +#include "consensus/ledger_enclave_types.h" #include "crypto/openssl/hash.h" #include "ds/files.h" #include "ds/internal_logger.h" @@ -96,6 +97,28 @@ static constexpr size_t retry_interval_ms = 100; namespace ccf { + void validate_ledger_transaction_size(const host::HostConfig& config) + { + const auto max_message_size = config.memory.max_msg_size.count_bytes(); + const auto max_transaction_size = + config.ledger.max_transaction_size.count_bytes(); + const auto response_overhead = + ::consensus::ledger_range_response_metadata_size; + + if ( + max_message_size <= response_overhead || + max_transaction_size > max_message_size - response_overhead) + { + throw std::logic_error(fmt::format( + "ledger.max_transaction_size ({}) must be at least {} bytes smaller " + "than memory.max_msg_size ({}) so a single ledger entry fits in a " + "ring-buffer range response", + max_transaction_size, + response_overhead, + max_message_size)); + } + } + void validate_and_adjust_recovery_threshold(host::HostConfig& config) { if (config.command.type != StartType::Start) @@ -994,6 +1017,19 @@ namespace ccf argv + argc, // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic) "\" \"")); + // Validated before the --check early return, so that operators verifying a + // configuration file are told about a ledger/ring-buffer size mismatch + // rather than discovering it when the node starts for real + try + { + validate_ledger_transaction_size(config); + } + catch (const std::logic_error& e) + { + LOG_FATAL_FMT("{}. Exiting.", e.what()); + return static_cast(CLI::ExitCodes::ValidationError); + } + if (check_config_only) { LOG_INFO_FMT("Configuration file successfully verified"); diff --git a/src/kv/committable_tx.h b/src/kv/committable_tx.h index f383563bf14a..e47877e983bd 100644 --- a/src/kv/committable_tx.h +++ b/src/kv/committable_tx.h @@ -36,10 +36,66 @@ namespace ccf::kv TxFlags flags = 0; SerialisedEntryFlags entry_flags = 0; + void serialise_all_changes( + KvStoreSerialiser& serialiser, bool include_reads) + { + // Process in security domain order + for (auto domain : {SecurityDomain::PUBLIC, SecurityDomain::PRIVATE}) + { + for (const auto& it : all_changes) + { + const auto& map = it.second.map; + const auto& changeset = it.second.changeset; + if (map->get_security_domain() == domain && changeset->has_writes()) + { + map->serialise_changes(changeset.get(), serialiser, include_reads); + } + } + } + } + + [[nodiscard]] bool has_writes() const + { + return std::any_of( + all_changes.begin(), all_changes.end(), [](const auto& it) { + return it.second.changeset->has_writes(); + }); + } + + size_t projected_serialised_size( + const ccf::ClaimsDigest& claims_digest_, bool include_reads = false) + { + if (claims_digest_.empty()) + { + throw std::logic_error("Missing claims"); + } + + auto e = pimpl->store->get_encryptor(); + if (e == nullptr) + { + throw KvSerialiserException("No encryptor set"); + } + + SizeKvStoreSerialiser size_serialiser( + e, + TxID{pimpl->commit_view, NoVersion}, + EntryType::WriteSetWithCommitEvidenceAndClaims, + entry_flags, + // Both digests are fixed-size, so their values do not affect the + // projected size. + ccf::crypto::Sha256Hash{}, + claims_digest_); + + serialise_all_changes(size_serialiser, include_reads); + + return size_serialiser.get_serialised_size(); + } + std::vector serialise( ccf::crypto::Sha256Hash& commit_evidence_digest, std::string& commit_evidence, const ccf::ClaimsDigest& claims_digest_, + size_t max_transaction_size, bool include_reads = false) { if (!committed) @@ -57,13 +113,7 @@ namespace ccf::kv throw std::logic_error("Missing claims"); } - // If no transactions made changes, return a zero length vector. - const bool any_changes = - std::any_of(all_changes.begin(), all_changes.end(), [](const auto& it) { - return it.second.changeset->has_writes(); - }); - - if (!any_changes) + if (!has_writes()) { return {}; } @@ -78,38 +128,24 @@ namespace ccf::kv LOG_TRACE_FMT("Commit evidence: {}", commit_evidence); ccf::crypto::Sha256Hash tx_commit_evidence_digest(commit_evidence); commit_evidence_digest = tx_commit_evidence_digest; - auto entry_type = EntryType::WriteSetWithCommitEvidenceAndClaims; if (tx_flag_enabled(TxFlag::LEDGER_CHUNK_BEFORE_THIS_TX)) { entry_flags |= EntryFlags::FORCE_LEDGER_CHUNK_BEFORE; } - RawKvStoreSerialiser replicated_serialiser( + RawKvStoreSerialiser serialiser( e, {pimpl->commit_view, version}, - entry_type, + EntryType::WriteSetWithCommitEvidenceAndClaims, entry_flags, tx_commit_evidence_digest, - claims_digest_); - - // Process in security domain order - for (auto domain : {SecurityDomain::PUBLIC, SecurityDomain::PRIVATE}) - { - for (const auto& it : all_changes) - { - const auto& map = it.second.map; - const auto& changeset = it.second.changeset; - if (map->get_security_domain() == domain && changeset->has_writes()) - { - map->serialise_changes( - changeset.get(), replicated_serialiser, include_reads); - } - } - } + claims_digest_, + false /* historical_hint */, + max_transaction_size); - // Return serialised Tx. - return replicated_serialiser.get_raw_data(); + serialise_all_changes(serialiser, include_reads); + return serialiser.get_raw_data(); } public: @@ -150,6 +186,23 @@ namespace ccf::kv return CommitResult::SUCCESS; } + std::optional projected_entry_size = std::nullopt; + + // Measure the write set and reject oversized entries before any change is + // applied. This pass performs no allocations or copies. Actual + // serialisation is deferred until after conflict detection. + if (has_writes()) + { + const auto max_transaction_size = + pimpl->store->get_max_transaction_size(); + projected_entry_size = projected_serialised_size(claims); + if (projected_entry_size.value() > max_transaction_size) + { + throw MaxTransactionSizeExceeded(describe_serialised_entry_size_error( + projected_entry_size.value(), max_transaction_size)); + } + } + // If this transaction creates any maps, ensure that commit gets a // consistent snapshot of the existing map set const bool maps_created = !pimpl->created_maps.empty(); @@ -222,12 +275,17 @@ namespace ccf::kv { ccf::crypto::Sha256Hash commit_evidence_digest; std::string commit_evidence; - auto data = serialise(commit_evidence_digest, commit_evidence, claims); - - if (data.empty()) - { - return CommitResult::SUCCESS; - } + auto data = serialise( + commit_evidence_digest, + commit_evidence, + claims, + pimpl->store->get_max_transaction_size()); + CCF_ASSERT_FMT( + projected_entry_size.has_value() && + data.size() == projected_entry_size.value(), + "Projected ledger entry size {} does not match serialised size {}", + projected_entry_size.value_or(0), + data.size()); if (write_set_observer != nullptr) { @@ -445,7 +503,14 @@ namespace ccf::kv committed = true; auto claims = ccf::empty_claims(); - auto data = serialise(commit_evidence_digest, commit_evidence, claims); + // Reserved transactions are used solely for signatures. They must always + // fill their reserved version, so the operator-configured transaction + // size limit does not apply to them. + auto data = serialise( + commit_evidence_digest, + commit_evidence, + claims, + max_serialised_entry_size); return { CommitResult::SUCCESS, diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index e9cecdc12b17..d0e282185615 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -24,6 +24,7 @@ namespace ccf::kv TxID tx_id; EntryType entry_type; SerialisedEntryFlags header_flags; + size_t max_transaction_size; std::shared_ptr crypto_util; @@ -68,10 +69,12 @@ namespace ccf::kv // in regular transactions, but absent in snapshots. const ccf::crypto::Sha256Hash& commit_evidence_digest_ = {}, const ccf::ClaimsDigest& claims_digest_ = ccf::no_claims(), - bool historical_hint_ = false) : + bool historical_hint_ = false, + size_t max_transaction_size_ = max_serialised_entry_size) : tx_id(tx_id_), entry_type(entry_type_), header_flags(header_flags_), + max_transaction_size(max_transaction_size_), crypto_util(std::move(e)), historical_hint(historical_hint_) { @@ -156,6 +159,27 @@ namespace ccf::kv public_writer.get_raw_data(), private_writer.get_raw_data()); } + /** Size of the ledger entry which get_raw_data() would produce for + * everything serialised so far. + * + * This covers the whole entry: the fixed-size ledger entry header, the + * ledger encryption header, the public domain size field, and both + * domains. Encryption does not change the size of the private domain, so + * this is exact. + */ + [[nodiscard]] size_t get_serialised_size() const + { + size_t size_ = public_writer.size(); + + if (crypto_util) + { + size_ += crypto_util->get_header_length() + sizeof(size_t) + + private_writer.size(); + } + + return size_ + sizeof(SerialisedEntryHeader); + } + std::vector serialise_domains( const std::vector& serialised_public_domain, const std::vector& serialised_private_domain) override @@ -173,9 +197,21 @@ namespace ccf::kv size_ += crypto_util->get_header_length() + sizeof(size_t) + serialised_private_domain.size(); } - entry_header.set_size(size_); - size_ += sizeof(SerialisedEntryHeader); + // The configured limit applies to the whole serialised ledger entry, + // including the fixed-size entry header. + const size_t entry_size = size_ + sizeof(SerialisedEntryHeader); + if (entry_size > max_transaction_size) + { + // Non-reserved transactions measure this exact size before applying + // their changes. Reserved signature transactions use the largest + // representable entry size here. Reaching this point is always fatal. + throw KvSerialiserException(describe_serialised_entry_size_error( + entry_size, max_transaction_size)); + } + + entry_header.set_size(size_); + size_ = entry_size; std::vector entry(size_); auto* data_ = entry.data(); diff --git a/src/kv/kv_serialiser.h b/src/kv/kv_serialiser.h index 8772969ffb08..57db53070599 100644 --- a/src/kv/kv_serialiser.h +++ b/src/kv/kv_serialiser.h @@ -8,5 +8,6 @@ namespace ccf::kv { using RawKvStoreSerialiser = GenericSerialiseWrapper; + using SizeKvStoreSerialiser = GenericSerialiseWrapper; using RawKvStoreDeserialiser = GenericDeserialiseWrapper; } diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index 7a85981ad16d..96de7bc58572 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -383,6 +384,13 @@ namespace ccf::kv } }; + class MaxTransactionSizeExceeded : public std::logic_error + { + public: + MaxTransactionSizeExceeded(const std::string& msg) : std::logic_error(msg) + {} + }; + class TxHistory { public: @@ -722,6 +730,7 @@ namespace ccf::kv virtual std::shared_ptr get_history() = 0; virtual std::shared_ptr get_chunker() = 0; virtual EncryptorPtr get_encryptor() = 0; + [[nodiscard]] virtual size_t get_max_transaction_size() const = 0; virtual std::unique_ptr deserialize( const std::vector& data, bool public_only = false, diff --git a/src/kv/raw_serialise.h b/src/kv/raw_serialise.h index dd7d91ade216..8b1f061750ae 100644 --- a/src/kv/raw_serialise.h +++ b/src/kv/raw_serialise.h @@ -81,6 +81,38 @@ namespace ccf::kv public: RawWriter() = default; + template + static size_t serialised_size(const T& entry) + { + if constexpr ( + ccf::nonstd::is_std_vector::value || + std::is_same_v) + { + return sizeof(size_t) + (entry.size() * sizeof(typename T::value_type)); + } + else if constexpr (std::is_same_v) + { + return sizeof(entry.h); + } + else if constexpr (std::is_same_v) + { + return sizeof(uint8_t); + } + else if constexpr (std::is_same_v) + { + return sizeof(size_t) + entry.size(); + } + else if constexpr (std::is_integral_v) + { + return sizeof(T); + } + else + { + static_assert( + ccf::nonstd::dependent_false::value, "Can't serialise this type"); + } + } + template void append(const T& entry) { @@ -122,12 +154,47 @@ namespace ccf::kv buf.clear(); } + [[nodiscard]] size_t size() const + { + return buf.size(); + } + std::vector get_raw_data() { return {buf.data(), buf.data() + buf.size()}; } }; + class SizeWriter + { + private: + size_t total_size = 0; + + public: + SizeWriter() = default; + + template + void append(const T& entry) + { + total_size += RawWriter::serialised_size(entry); + } + + void clear() + { + total_size = 0; + } + + [[nodiscard]] size_t size() const + { + return total_size; + } + + std::vector get_raw_data() + { + throw std::logic_error("SizeWriter does not retain serialised data"); + } + }; + class RawReader { private: diff --git a/src/kv/serialised_entry_format.h b/src/kv/serialised_entry_format.h index 46e0650fd9cf..53979ea4ab97 100644 --- a/src/kv/serialised_entry_format.h +++ b/src/kv/serialised_entry_format.h @@ -4,7 +4,10 @@ #include "ds/ccf_assert.h" +#include +#include #include +#include namespace ccf::kv { @@ -25,17 +28,17 @@ namespace ccf::kv static constexpr auto BITS_FOR_SIZE = (sizeof(uint64_t) - sizeof(uint8_t) - sizeof(SerialisedEntryFlags)) * CHAR_BIT; + static constexpr uint64_t max_serialised_entry_body_size = + (uint64_t{1} << BITS_FOR_SIZE) - 1; uint64_t size : BITS_FOR_SIZE = 0; void set_size(uint64_t size_) { - [[maybe_unused]] static constexpr size_t max_entry_size = 1UL - << BITS_FOR_SIZE; CCF_ASSERT_FMT( - size_ < max_entry_size, + size_ <= max_serialised_entry_body_size, "Cannot serialise entry of size {} (max allowed size is {})", size_, - max_entry_size); + max_serialised_entry_body_size); size = size_; } }; @@ -43,4 +46,24 @@ namespace ccf::kv static constexpr size_t serialised_entry_header_size = sizeof(SerialisedEntryHeader); -} \ No newline at end of file + + // Largest ledger entry which can be described by a SerialisedEntryHeader, + // including that header + static constexpr size_t max_serialised_entry_size = + SerialisedEntryHeader::max_serialised_entry_body_size + + serialised_entry_header_size; + + static inline std::string describe_serialised_entry_size_error( + size_t entry_size, size_t max_entry_size) + { + return fmt::format( + "Cannot serialise transaction with total serialised size {} bytes. The " + "configured maximum is {} bytes. The size compared to this limit is the " + "whole ledger entry: the fixed {}-byte ledger entry header plus the " + "serialised transaction body (any ledger encryption header, public " + "domain size field, public domain and encrypted private domain).", + entry_size, + max_entry_size, + serialised_entry_header_size); + } +} diff --git a/src/kv/store.h b/src/kv/store.h index dce78e55f41d..d8659a6127dd 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -15,6 +15,7 @@ #include "kv_types.h" #define FMT_HEADER_ONLY +#include #include #include #include @@ -105,6 +106,7 @@ namespace ccf::kv std::shared_ptr chunker = nullptr; EncryptorPtr encryptor = nullptr; SnapshotterPtr snapshotter = nullptr; + size_t max_transaction_size = max_serialised_entry_size; // Generally we will only accept deserialised views if they are contiguous - // at Version N we reject everything but N+1. The exception is when a Store @@ -254,6 +256,30 @@ namespace ccf::kv return encryptor; } + void set_max_transaction_size(size_t max_transaction_size_) + { + // The limit covers the whole ledger entry, so it can never usefully + // exceed the largest entry the header can describe, nor the largest + // buffer the allocator can produce + static const size_t max_allocatable = std::vector().max_size(); + const auto effective_max = + std::min(max_serialised_entry_size, max_allocatable); + if (max_transaction_size_ > effective_max) + { + throw std::logic_error(fmt::format( + "Configured maximum transaction size {} exceeds the largest " + "serialisable ledger entry size {}", + max_transaction_size_, + effective_max)); + } + max_transaction_size = max_transaction_size_; + } + + [[nodiscard]] size_t get_max_transaction_size() const override + { + return max_transaction_size; + } + void set_snapshotter(const SnapshotterPtr& snapshotter_) { snapshotter = snapshotter_; @@ -418,6 +444,9 @@ namespace ccf::kv std::unique_ptr snapshot) override { auto e = get_encryptor(); + // Snapshots capture the entire committed state and are legitimately + // larger than any single transaction, so max_transaction_size does not + // apply to them. return snapshot->serialise(e); } diff --git a/src/kv/test/kv_serialisation.cpp b/src/kv/test/kv_serialisation.cpp index 216e4022f619..553d82b3258b 100644 --- a/src/kv/test/kv_serialisation.cpp +++ b/src/kv/test/kv_serialisation.cpp @@ -6,6 +6,7 @@ #include "kv/store.h" #include "kv/test/null_encryptor.h" #include "kv/test/stub_consensus.h" +#include "node/encryptor.h" #include #undef FAIL @@ -312,6 +313,252 @@ TEST_CASE( } } +TEST_CASE( + "Reject transactions exceeding configured serialised size" * + doctest::test_suite("serialisation")) +{ + auto consensus = std::make_shared(); + auto encryptor = std::make_shared(); + + ccf::kv::Store kv_store; + kv_store.set_consensus(consensus); + kv_store.set_encryptor(encryptor); + kv_store.set_max_transaction_size(1024); + + MapTypes::StringString map("public:pub_map"); + + { + INFO("An oversized transaction is rejected before it is applied"); + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("oversized", std::string(2048, 'A')); + REQUIRE_THROWS_AS(tx.commit(), ccf::kv::MaxTransactionSizeExceeded); + REQUIRE(kv_store.current_version() == 0); + REQUIRE(!consensus->get_latest_data().has_value()); + } + + { + INFO("Later transactions are unaffected"); + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("small", "ok"); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + REQUIRE(kv_store.current_version() == 1); + } +} + +TEST_CASE( + "The transaction size limit is compared against the exact entry size" * + doctest::test_suite("serialisation")) +{ + auto consensus = std::make_shared(); + auto encryptor = std::make_shared(); + + MapTypes::StringString map("public:pub_map"); + const auto value = std::string(512, 'A'); + + // Serialise the transaction under a permissive limit to find the exact size + // of the ledger entry it produces. + size_t entry_size = 0; + { + ccf::kv::Store kv_store; + kv_store.set_consensus(consensus); + kv_store.set_encryptor(encryptor); + + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("key", value); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + + const auto latest_data = consensus->get_latest_data(); + REQUIRE(latest_data.has_value()); + entry_size = latest_data->size(); + } + + // The allocation-free sizing pass must report the exact size of the entry + // which is eventually written. A limit of precisely that size is accepted, + // and one byte less is not. + { + ccf::kv::Store kv_store; + kv_store.set_encryptor(encryptor); + kv_store.set_max_transaction_size(entry_size); + + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("key", value); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + } + + { + ccf::kv::Store kv_store; + kv_store.set_encryptor(encryptor); + kv_store.set_max_transaction_size(entry_size - 1); + + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("key", value); + REQUIRE_THROWS_AS(tx.commit(), ccf::kv::MaxTransactionSizeExceeded); + REQUIRE(kv_store.current_version() == 0); + } +} + +TEST_CASE( + "The transaction size limit includes encrypted private data" * + doctest::test_suite("serialisation")) +{ + const auto create_encryptor = []() { + auto secrets = std::make_shared(); + secrets->init(); + return std::make_shared(secrets); + }; + + MapTypes::StringString public_map("public:pub_map"); + MapTypes::StringString private_map("priv_map"); + const auto value = std::string(512, 'A'); + + const auto populate = [&](ccf::kv::CommittableTx& tx) { + tx.rw(public_map)->put("public_key", value); + tx.rw(private_map)->put("private_key", value); + }; + + size_t entry_size = 0; + { + auto consensus = std::make_shared(); + ccf::kv::Store kv_store; + kv_store.set_consensus(consensus); + kv_store.set_encryptor(create_encryptor()); + + auto tx = kv_store.create_tx(); + populate(tx); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + + const auto latest_data = consensus->get_latest_data(); + REQUIRE(latest_data.has_value()); + entry_size = latest_data->size(); + } + + { + ccf::kv::Store kv_store; + kv_store.set_encryptor(create_encryptor()); + kv_store.set_max_transaction_size(entry_size); + + auto tx = kv_store.create_tx(); + populate(tx); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + } + + { + ccf::kv::Store kv_store; + kv_store.set_encryptor(create_encryptor()); + kv_store.set_max_transaction_size(entry_size - 1); + + auto tx = kv_store.create_tx(); + populate(tx); + REQUIRE_THROWS_AS(tx.commit(), ccf::kv::MaxTransactionSizeExceeded); + REQUIRE(kv_store.current_version() == 0); + } +} + +TEST_CASE( + "Deserialisation is not subject to the transaction size limit" * + doctest::test_suite("serialisation")) +{ + auto consensus = std::make_shared(); + auto encryptor = std::make_shared(); + + // Serialise a transaction under a permissive (default) limit. + ccf::kv::Store kv_store; + kv_store.set_consensus(consensus); + kv_store.set_encryptor(encryptor); + + MapTypes::StringString map("public:pub_map"); + + { + auto tx = kv_store.create_tx(); + auto handle = tx.rw(map); + handle->put("large", std::string(2048, 'A')); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + } + + const auto latest_data = consensus->get_latest_data(); + REQUIRE(latest_data.has_value()); + + // The cap applies only to serialisation. A tiny limit on the target store + // must not prevent it from deserialising an already-serialised transaction. + ccf::kv::Store kv_store_target; + kv_store_target.set_encryptor(encryptor); + kv_store_target.set_max_transaction_size(1); + + REQUIRE( + kv_store_target.deserialize(latest_data.value())->apply() == + ccf::kv::ApplyResult::PASS); +} + +TEST_CASE( + "RawWriter and SizeWriter agree" * doctest::test_suite("serialisation")) +{ + const auto check = [](const auto& entry) { + ccf::kv::RawWriter raw_writer; + ccf::kv::SizeWriter size_writer; + const auto expected_size = ccf::kv::RawWriter::serialised_size(entry); + raw_writer.append(entry); + size_writer.append(entry); + REQUIRE(raw_writer.size() == expected_size); + REQUIRE(size_writer.size() == expected_size); + }; + + check(ccf::kv::EntryType::WriteSetWithCommitEvidenceAndClaims); + check(uint8_t(42)); + check(uint64_t(42)); + check(ccf::kv::Version(42)); + check(ccf::crypto::Sha256Hash(std::string("some content"))); + check(std::string()); + check(std::string("a string of some length")); + check(std::vector()); + check(std::vector(37, 'x')); + check(std::vector{1, 2, 3}); + check(ccf::kv::serialisers::SerialisedEntry()); + check(ccf::kv::serialisers::SerialisedEntry(91, 'y')); +} + +TEST_CASE( + "Reserved signature transactions ignore the configured transaction size " + "limit" * + doctest::test_suite("serialisation")) +{ + ccf::kv::Store kv_store; + auto encryptor = std::make_shared(); + kv_store.set_encryptor(encryptor); + kv_store.set_max_transaction_size(1); + + MapTypes::StringString map("public:signature"); + auto tx = kv_store.create_reserved_tx(kv_store.next_txid()); + tx.rw(map)->put("signature", std::string(512, 'A')); + + const auto [result, data, claims, commit_evidence, hooks] = + tx.commit_reserved(); + REQUIRE(result == ccf::kv::CommitResult::SUCCESS); + REQUIRE(data.size() > kv_store.get_max_transaction_size()); +} + +TEST_CASE( + "Reject configuring a maximum transaction size beyond the serialisable " + "limit" * + doctest::test_suite("serialisation")) +{ + ccf::kv::Store kv_store; + + // The largest entry the ledger entry header can describe is accepted. + REQUIRE_NOTHROW( + kv_store.set_max_transaction_size(ccf::kv::max_serialised_entry_size)); + + // A value larger than can ever be serialised is rejected at configuration + // time, rather than being deferred to a later serialisation failure. + REQUIRE_THROWS_AS( + kv_store.set_max_transaction_size(std::numeric_limits::max()), + std::logic_error); +} + TEST_CASE( "Serialise/deserialise private map and public maps" * doctest::test_suite("serialisation")) @@ -822,6 +1069,7 @@ TEST_CASE( ccf::ClaimsDigest claims_digest; claims_digest.set(ccf::crypto::Sha256Hash("claim text")); + ccf::crypto::Sha256Hash expected_commit_evidence_digest; INFO("Commit to source store, including claims"); { @@ -833,9 +1081,11 @@ TEST_CASE( handle_pub->put("pubk1", "pubv1"); REQUIRE(tx.commit(claims_digest) == ccf::kv::CommitResult::SUCCESS); + expected_commit_evidence_digest = ccf::crypto::Sha256Hash( + encryptor->get_commit_evidence({tx.commit_term(), tx.commit_version()})); } - INFO("Deserialise transaction in target store and extract claims"); + INFO("Deserialise transaction in target store and extract digests"); { const auto latest_data = consensus->get_latest_data(); REQUIRE(latest_data.has_value()); @@ -843,6 +1093,11 @@ TEST_CASE( REQUIRE(wrapper->apply() != ccf::kv::ApplyResult::FAIL); auto deserialised_claims = wrapper->consume_claims_digest(); REQUIRE(claims_digest == deserialised_claims); + auto deserialised_commit_evidence = + wrapper->consume_commit_evidence_digest(); + REQUIRE(deserialised_commit_evidence.has_value()); + REQUIRE( + deserialised_commit_evidence.value() == expected_commit_evidence_digest); auto tx_target = kv_store_target.create_tx(); auto handle_priv = tx_target.rw(priv_map); diff --git a/src/kv/test/kv_snapshot.cpp b/src/kv/test/kv_snapshot.cpp index 08ed6f959e98..7ba6dd7caae1 100644 --- a/src/kv/test/kv_snapshot.cpp +++ b/src/kv/test/kv_snapshot.cpp @@ -7,6 +7,7 @@ #include #undef FAIL +#include struct MapTypes { @@ -19,6 +20,59 @@ struct MapTypes MapTypes::StringString string_map("public:string_map"); MapTypes::NumNum num_map("public:num_map"); +TEST_CASE( + "Snapshots are not subject to the transaction size limit" * + doctest::test_suite("snapshot")) +{ + auto encryptor = std::make_shared(); + + ccf::kv::Store store; + store.set_encryptor(encryptor); + + // A deliberately small per-transaction limit. + constexpr size_t small_limit = 512; + store.set_max_transaction_size(small_limit); + + // Accumulate committed state larger than the limit across several + // transactions, each of which individually stays under the limit. + constexpr size_t num_entries = 32; + constexpr size_t value_size = 64; + ccf::kv::Version snapshot_version = ccf::kv::NoVersion; + for (size_t i = 0; i < num_entries; ++i) + { + auto tx = store.create_tx(); + auto handle = tx.rw(string_map); + handle->put("key_" + std::to_string(i), std::string(value_size, 'x')); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + snapshot_version = tx.commit_version(); + } + REQUIRE(num_entries * value_size > small_limit); + + std::unique_ptr snapshot = nullptr; + { + ccf::kv::ScopedStoreMapsLock maps_lock(&store); + snapshot = store.snapshot_unsafe_maps(snapshot_version); + } + + // Serialising a snapshot larger than the per-transaction limit must not be + // rejected by that limit. + std::vector serialised_snapshot; + REQUIRE_NOTHROW( + serialised_snapshot = store.serialise_snapshot(std::move(snapshot))); + + // And it can be read back into a store configured with the same small limit. + ccf::kv::Store new_store; + new_store.set_encryptor(encryptor); + new_store.set_max_transaction_size(small_limit); + + ccf::kv::ConsensusHookPtrs hooks; + REQUIRE_EQ( + new_store.deserialise_snapshot( + serialised_snapshot.data(), serialised_snapshot.size(), hooks), + ccf::kv::ApplyResult::PASS); + REQUIRE_EQ(new_store.current_version(), snapshot_version); +} + TEST_CASE("Simple snapshot" * doctest::test_suite("snapshot")) { ccf::kv::Store store; diff --git a/src/node/historical_queries.h b/src/node/historical_queries.h index 55fd7b7c81f0..0c98e69c89fa 100644 --- a/src/node/historical_queries.h +++ b/src/node/historical_queries.h @@ -1485,6 +1485,11 @@ namespace ccf::historical false /* Do not start from very first seqno */, true /* Make use of historical secrets */); + // max_transaction_size is deliberately not set on this store. It is a + // write-time limit, and historical queries must remain able to + // reconstruct any entry which was validly written, including under a + // previously larger or unset limit. + // If this is older than the node's currently known ledger secrets, use // the historical encryptor (which should have older secrets) if (seqno < source_ledger_secrets->get_first().first) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index ec65f95ab601..01a7d28af1a3 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -1017,6 +1017,18 @@ namespace ccf return; } + catch (const ccf::kv::MaxTransactionSizeExceeded& e) + { + // Thrown before the transaction is applied, so the store is + // unchanged and later transactions are unaffected + ctx->clear_response_headers(); + ctx->set_error( + HTTP_STATUS_PAYLOAD_TOO_LARGE, + ccf::errors::TransactionTooLarge, + e.what()); + + return; + } catch (const ccf::kv::KvSerialiserException& e) { // If serialising the committed transaction fails, there is no way diff --git a/tests/config.jinja b/tests/config.jinja index b6a0bf18b51d..7de8e0084f79 100644 --- a/tests/config.jinja +++ b/tests/config.jinja @@ -57,7 +57,8 @@ { "directory": "{{ ledger_dir }}", "read_only_directories": {{ read_only_ledger_dirs|tojson }}, - "chunk_size": "{{ ledger_chunk_bytes }}" + "chunk_size": "{{ ledger_chunk_bytes }}", + "max_transaction_size": "{{ ledger_max_transaction_bytes }}" }, "snapshots": { diff --git a/tests/e2e_batched.py b/tests/e2e_batched.py index 7acdc05d0882..cd196465b5a8 100644 --- a/tests/e2e_batched.py +++ b/tests/e2e_batched.py @@ -1,5 +1,6 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the Apache 2.0 License. +import http import itertools import time from hashlib import sha256 @@ -16,7 +17,14 @@ @reqs.description("Running batch submission of new entries") -def test(network, args, batch_size=100, write_key_divisor=1, write_size_multiplier=1): +def test( + network, + args, + batch_size=100, + write_key_divisor=1, + write_size_multiplier=1, + expect_transaction_too_large=False, +): LOG.info(f"Number of batched entries: {batch_size}") primary, _ = network.find_primary() @@ -31,18 +39,24 @@ def test(network, args, batch_size=100, write_key_divisor=1, write_size_multipli ] pre_submit = time.time() - check( - c.post( - "/app/batch/submit", - { - "entries": messages, - "write_key_divisor": write_key_divisor, - "write_size_multiplier": write_size_multiplier, - }, - timeout=30, - ), - result=len(messages), + response = c.post( + "/app/batch/submit", + { + "entries": messages, + "write_key_divisor": write_key_divisor, + "write_size_multiplier": write_size_multiplier, + }, + timeout=30, ) + + if expect_transaction_too_large: + assert ( + response.status_code == http.HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value + ) + assert response.body.json()["error"]["code"] == "TransactionTooLarge" + return network + + check(response, result=len(messages)) post_submit = time.time() LOG.warning( f"Submitting {batch_size} new keys took {post_submit - pre_submit}s" @@ -88,48 +102,27 @@ def run(args): # bs += step_size -def run_to_destruction(args): +def run_to_transaction_limit(args): with infra.network.network( args.nodes, args.binary_dir, args.debug_nodes, pdb=args.pdb ) as network: network.start_and_open(args) - LOG.warning("About to issue transactions until destruction") - try: - wsm = 5000 - while True: - LOG.info(f"Trying with writes scaled by {wsm}") - network = test(network, args, batch_size=10, write_size_multiplier=wsm) - if wsm > 1000000: - LOG.error( - f"Run to destruction still hasn't caused exception with write sizes multiplied by {wsm}. Infinite loop, or not actually submitting?" - ) - raise ValueError(wsm) - else: - wsm += 100000 # Grow very quickly, expect to fail on the second iteration - except Exception as e: - timeout = 120 - - LOG.info("Large write set caused an exception, as expected") - LOG.info(f"Exception was: {e}") - LOG.info(f"Polling for {timeout}s for node to terminate") - - end_time = time.time() + timeout - while time.time() < end_time: - time.sleep(0.1) - exit_codes = [node.remote.remote.proc.poll() for node in network.nodes] - if any(exit_codes): - LOG.info( - f"One or more nodes terminated with exit codes {exit_codes}" - ) - break - - if time.time() > end_time: - raise TimeoutError( - f"Node took longer than {timeout}s to terminate" - ) from e - - network.ignore_errors_on_shutdown() + LOG.warning("About to issue a transaction above the configured limit") + network = test(network, args, batch_size=10, write_size_multiplier=5000) + network = test( + network, + args, + batch_size=10, + write_size_multiplier=105000, + expect_transaction_too_large=True, + ) + + exit_codes = [node.remote.remote.proc.poll() for node in network.nodes] + assert all(exit_code is None for exit_code in exit_codes), exit_codes + + # The rejected transaction must not prevent subsequent commits. + network = test(network, args, batch_size=10) if __name__ == "__main__": @@ -137,8 +130,11 @@ def run_to_destruction(args): args.package = "js_generic" args.nodes = infra.e2e_args.min_nodes(args, f=1) - # Helps ensure expected destruction workflow. See #6373 for details. + # Keep this stress test's successful write below the configured transaction + # limit while allowing the next, much larger write to exercise clean + # rejection. args.max_msg_size_bytes = f"{1024 * 1024 * 16}" # 16MB + args.ledger_max_transaction_bytes = f"{1024 * 1024 * 15}" # 15MB run(args) - run_to_destruction(args) + run_to_transaction_limit(args) diff --git a/tests/e2e_operations.py b/tests/e2e_operations.py index 6edf7dcb9c9f..cc0a373e2f36 100644 --- a/tests/e2e_operations.py +++ b/tests/e2e_operations.py @@ -1626,6 +1626,7 @@ def run_file_operations(args): ntf.flush() args.max_msg_size_bytes = f"{1024 ** 2}" + args.ledger_max_transaction_bytes = f"{1024 ** 2 - 2048}" with tempfile.TemporaryDirectory() as tmp_dir: txs = app.LoggingTxs("user0") diff --git a/tests/infra/e2e_args.py b/tests/infra/e2e_args.py index a6a55eda5cb4..b9c91efb13ff 100644 --- a/tests/infra/e2e_args.py +++ b/tests/infra/e2e_args.py @@ -286,6 +286,15 @@ def cli_args( type=str, default=ledger_chunk_bytes_override or "20KB", ) + parser.add_argument( + "--ledger-max-transaction-bytes", + help=( + "Maximum total serialised ledger entry size, including its header " + "(size string)" + ), + type=str, + default="32MB", + ) parser.add_argument( "--snapshot-tx-interval", help="Number of transactions between two snapshots", diff --git a/tests/infra/network.py b/tests/infra/network.py index a4b7b5befbb8..ba1d2eb0dc63 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -194,6 +194,7 @@ class Network: "join_timer_s", "worker_threads", "ledger_chunk_bytes", + "ledger_max_transaction_bytes", "subject_alt_names", "snapshot_tx_interval", "snapshot_min_tx_interval", diff --git a/tests/limits.py b/tests/limits.py index 8aa2dabe6a2e..536f9046ceab 100644 --- a/tests/limits.py +++ b/tests/limits.py @@ -10,6 +10,7 @@ import infra.jwt_issuer import infra.network import infra.proc +import suite.test_requirements as reqs from infra.runner import ConcurrentRunner @@ -63,6 +64,37 @@ def get_request_payload_too_large_errors(): assert r.status_code == http.HTTPStatus.OK.value, r +@reqs.description("Transactions larger than ledger.max_transaction_size are rejected") +@reqs.supports_methods("/app/log/private") +def test_transaction_size_limit(network, args): + primary, _ = network.find_primary() + + with primary.client("user0") as c: + r = c.post("/app/log/private", {"id": 1, "msg": "small"}) + assert r.status_code == http.HTTPStatus.OK.value, r + + # Comfortably under the interface's max_http_body_size, so this is + # rejected by the ledger transaction size limit rather than by the HTTP + # request parser, which reports RequestBodyTooLarge and closes the + # session. + msg = "A" * 600 * 1024 + r = c.post("/app/log/private", {"id": 2, "msg": msg}) + assert r.status_code == http.HTTPStatus.REQUEST_ENTITY_TOO_LARGE.value, r + assert r.body.json()["error"]["code"] == "TransactionTooLarge", r + + # The rejected transaction was never applied, so it neither wrote a + # value nor prevented later transactions from being processed + r = c.get("/app/log/private?id=2") + assert r.status_code == http.HTTPStatus.NOT_FOUND.value, r + + r = c.post("/app/log/private", {"id": 3, "msg": "still processing"}) + assert r.status_code == http.HTTPStatus.OK.value, r + + r = c.get("/app/log/private?id=3") + assert r.status_code == http.HTTPStatus.OK.value, r + assert r.body.json()["msg"] == "still processing", r + + def run_parser_limits_checks(args): new_args = copy.copy(args) # Deliberately large because some builds take @@ -80,6 +112,23 @@ def run_parser_limits_checks(args): test_forward_larger_than_default_requests(network, new_args) +def run_transaction_size_limit_checks(args): + new_args = copy.copy(args) + # Larger than the constitution scripts written to the KV store as part of + # service creation, and than any governance transaction, but smaller than + # the oversized request used by the test + new_args.ledger_max_transaction_bytes = "512KB" + with infra.network.network( + new_args.nodes, + new_args.binary_dir, + new_args.debug_nodes, + pdb=args.pdb, + ) as network: + network.start_and_open(new_args) + + test_transaction_size_limit(network, new_args) + + if __name__ == "__main__": cr = ConcurrentRunner() @@ -92,4 +141,11 @@ def run_parser_limits_checks(args): nodes=infra.e2e_args.max_nodes(cr.args, f=0), ) + cr.add( + "transaction_size_limit", + run_transaction_size_limit_checks, + package="samples/apps/logging/logging", + nodes=infra.e2e_args.max_nodes(cr.args, f=0), + ) + cr.run()