Remove private-tables-with-no-encryptor serialiser behaviour (#340)

This commit is contained in:
Julien Maffre 2019-08-29 17:53:07 +01:00 коммит произвёл GitHub
Родитель c9b0f7ef5f
Коммит 5cae17fb99
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
7 изменённых файлов: 175 добавлений и 170 удалений

Просмотреть файл

@ -6,6 +6,7 @@
#include "ds/logger.h"
#include "enclave/appinterface.h"
#include "luainterp/luainterp.h"
#include "node/encryptor.h"
#include "node/genesisgen.h"
#include "node/rpc/jsonrpc.h"
#include "node/rpc/test/node_stub.h"
@ -120,6 +121,8 @@ void check_store_load(F frontend, K k, V v)
TEST_CASE("simple lua apps")
{
NetworkTables network;
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
network.tables->set_encryptor(encryptor);
GenesisGenerator gen(network);
gen.init_values();
StubNotifier notifier;
@ -241,6 +244,8 @@ TEST_CASE("simple lua apps")
TEST_CASE("simple bank")
{
NetworkTables network;
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
network.tables->set_encryptor(encryptor);
GenesisGenerator gen(network);
gen.init_values();
StubNotifier notifier;
@ -343,6 +348,8 @@ TEST_CASE("simple bank")
TEST_CASE("pre-populated environment")
{
NetworkTables network;
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
network.tables->set_encryptor(encryptor);
GenesisGenerator gen(network);
gen.init_values();
StubNotifier notifier;

Просмотреть файл

@ -38,6 +38,20 @@ namespace kv
static_cast<KotBase>(a) | static_cast<KotBase>(b));
}
class KvSerialiserException : public std::exception
{
private:
std::string msg;
public:
KvSerialiserException(const std::string& msg_) : msg(msg_) {}
virtual const char* what() const throw()
{
return msg.c_str();
}
};
template <typename K, typename V, typename Version>
struct KeyValVersion
{
@ -80,28 +94,8 @@ namespace kv
public_writer.append(std::forward<T>(t));
}
// TODO(roy|#refactoring): this hack exists since the genesis generator
// cannot access any crypto_util. The correct solution would be to make sure
// that only public tables are being serialised during startup, so that this
// unwanted (and potentially harmful) function can be removed
SecurityDomain get_domain_to_be_used(SecurityDomain domain)
{
if (domain == SecurityDomain::PUBLIC)
return SecurityDomain::PUBLIC;
if (domain == SecurityDomain::PRIVATE)
{
// no crypto_util - treat as public
return crypto_util ? SecurityDomain::PRIVATE : SecurityDomain::PUBLIC;
}
throw std::logic_error(
"Unknown Security Domain " + std::to_string(domain));
}
void set_current_domain(SecurityDomain domain)
{
domain = get_domain_to_be_used(domain);
switch (domain)
{
case SecurityDomain::PRIVATE:
@ -109,7 +103,6 @@ namespace kv
current_domain = SecurityDomain::PRIVATE;
break;
case SecurityDomain::PUBLIC:
// either public or private with no crypto_util
current_writer = &public_writer;
current_domain = SecurityDomain::PUBLIC;
break;
@ -130,7 +123,12 @@ namespace kv
void start_map(const std::string& name, SecurityDomain domain)
{
domain = get_domain_to_be_used(domain);
if (domain == SecurityDomain::PRIVATE && !crypto_util)
{
throw KvSerialiserException(fmt::format(
"Private map {} cannot be serialised without an encryptor", name));
}
if (domain != current_domain)
set_current_domain(domain);

Просмотреть файл

@ -14,16 +14,66 @@
using namespace ccfapp;
TEST_CASE("Serialise/deserialise public map only")
struct CustomClass
{
int m_i;
CustomClass() : CustomClass(-1) {}
CustomClass(int i) : m_i(i) {}
int get() const
{
return m_i;
}
void set(std::string val)
{
m_i = std::stoi(val);
}
CustomClass operator()()
{
CustomClass ret;
return ret;
}
bool operator<(const CustomClass& other) const
{
return m_i < other.m_i;
}
bool operator==(const CustomClass& other) const
{
return !(other < *this) && !(*this < other);
}
MSGPACK_DEFINE(m_i);
};
namespace std
{
template <>
struct hash<CustomClass>
{
std::size_t operator()(const CustomClass& inst) const
{
return inst.get();
}
};
}
DECLARE_JSON_TYPE(CustomClass)
DECLARE_JSON_REQUIRED_FIELDS(CustomClass, m_i)
TEST_CASE(
"Serialise/deserialise public map only" *
doctest::test_suite("serialisation"))
{
// No need for an encryptor here as all maps are public. Both serialisation
// and deserialisation should succeed.
auto consensus = std::make_shared<kv::StubConsensus>();
auto secrets = ccf::NetworkSecrets("");
auto encryptor = std::make_shared<ccf::TxEncryptor>(1, secrets);
Store kv_store(consensus);
Store kv_store_target;
kv_store.set_encryptor(encryptor);
kv_store_target.set_encryptor(encryptor);
auto& pub_map = kv_store.create<std::string, std::string>(
"pub_map", kv::SecurityDomain::PUBLIC);
@ -50,21 +100,35 @@ TEST_CASE("Serialise/deserialise public map only")
}
}
TEST_CASE("Serialise/deserialise private map only")
TEST_CASE(
"Serialise/deserialise private map only" *
doctest::test_suite("serialisation"))
{
auto consensus = std::make_shared<kv::StubConsensus>();
auto secrets = ccf::NetworkSecrets("");
auto encryptor = std::make_shared<ccf::TxEncryptor>(1, secrets);
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
Store kv_store(consensus);
Store kv_store_target;
kv_store.set_encryptor(encryptor);
kv_store_target.set_encryptor(encryptor);
auto& priv_map = kv_store.create<std::string, std::string>(
"priv_map", kv::SecurityDomain::PRIVATE);
kv_store_target.clone_schema(kv_store);
INFO("Commit a private transaction without an encryptor throws an exception");
{
Store::Tx tx;
auto view0 = tx.get_view(priv_map);
view0->put("privk1", "privv1");
REQUIRE_THROWS_AS(tx.commit(), kv::KvSerialiserException);
}
// Since a serialisation error occurred and was not recovered properly (see
// https://github.com/microsoft/CCF/issues/338), we need to clear the store to
// get a fresh version.
kv_store.clear();
kv_store.set_encryptor(encryptor);
INFO("Commit to private map in source store");
{
Store::Tx tx;
@ -86,11 +150,12 @@ TEST_CASE("Serialise/deserialise private map only")
}
}
TEST_CASE("Serialise/deserialise private and public maps")
TEST_CASE(
"Serialise/deserialise private and public maps" *
doctest::test_suite("serialisation"))
{
auto consensus = std::make_shared<kv::StubConsensus>();
auto secrets = ccf::NetworkSecrets("");
auto encryptor = std::make_shared<ccf::TxEncryptor>(1, secrets);
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
Store kv_store(consensus);
Store kv_store_target;
@ -130,11 +195,11 @@ TEST_CASE("Serialise/deserialise private and public maps")
}
}
TEST_CASE("Serialise/deserialise removed keys")
TEST_CASE(
"Serialise/deserialise removed keys" * doctest::test_suite("serialisation"))
{
auto consensus = std::make_shared<kv::StubConsensus>();
auto secrets = ccf::NetworkSecrets("");
auto encryptor = std::make_shared<ccf::TxEncryptor>(1, secrets);
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
Store kv_store(consensus);
Store kv_store_target;
@ -186,51 +251,48 @@ TEST_CASE("Serialise/deserialise removed keys")
}
}
TEST_CASE("Serialise private map with no encryptor/Deserialise in new store")
TEST_CASE(
"Custom type serialisation test" * doctest::test_suite("serialisation"))
{
// TODO: Remove such behaviour https://github.com/microsoft/CCF/issues/316
// This test mirrors the behaviour of the genesisgenerator utility which does
// not have an encryptor and serialise all maps as if they were public.
auto consensus = std::make_shared<kv::StubConsensus>();
Store kv_store(consensus);
Store kv_store;
auto& map = kv_store.create<std::string, std::string>("map");
auto& map = kv_store.create<CustomClass, CustomClass>(
"map", kv::SecurityDomain::PUBLIC);
constexpr auto k = "key";
constexpr auto k2 = "key2";
constexpr auto v1 = "value1";
constexpr auto v2 = "value2";
CustomClass k(3);
CustomClass v1(33);
INFO("Serialise single transaction");
CustomClass k2(2);
CustomClass v2(22);
INFO("Serialise/Deserialise 2 kv stores");
{
Store::Tx tx;
Store kv_store2;
auto& map2 = kv_store2.create<CustomClass, CustomClass>(
"map", kv::SecurityDomain::PUBLIC);
Store::Tx tx(kv_store.next_version());
auto view = tx.get_view(map);
view->put(k, v1);
view->put(k2, v2);
REQUIRE(tx.commit() == kv::CommitSuccess::OK);
}
INFO("Deserialise transaction in new kv store");
{
Store kv_store2;
auto& map2 = kv_store2.create<std::string, std::string>("map");
auto [success, reqid, serialised] = tx.commit_reserved();
REQUIRE(success == kv::CommitSuccess::OK);
kv_store.compact(view->end_order());
auto serial = consensus->get_latest_data();
REQUIRE(consensus->number_of_replicas() == 1);
REQUIRE(serial.second);
REQUIRE(!serial.first.empty());
Store::Tx tx;
REQUIRE(
kv_store2.deserialise(serial.first) != kv::DeserialiseSuccess::FAILED);
auto view2 = tx.get_view(map2);
REQUIRE(kv_store2.deserialise(serialised) == kv::DeserialiseSuccess::PASS);
Store::Tx tx2;
auto view2 = tx2.get_view(map2);
auto va = view2->get(k);
REQUIRE(va.has_value());
REQUIRE(va.value() == v1);
auto vb = view2->get(k2);
REQUIRE(vb.has_value());
REQUIRE(vb.value() == v2);
consensus->flush();
// we only require operator==() to be implemented, so for consistency -
// this is the operator we use for comparison, and not operator!=()
REQUIRE(!(vb.value() == v1));
}
}
@ -260,12 +322,14 @@ bool corrupt_serialised_tx(
return false;
}
TEST_CASE("Integrity")
TEST_CASE("Integrity" * doctest::test_suite("serialisation"))
{
SUBCASE("Public and Private")
{
auto consensus = std::make_shared<kv::StubConsensus>();
// Here, a real encryptor is needed to protect the integrity of the
// transactions
auto secrets = ccf::NetworkSecrets("");
auto encryptor = std::make_shared<ccf::TxEncryptor>(1, secrets);
@ -299,7 +363,7 @@ TEST_CASE("Integrity")
}
}
TEST_CASE("nlohmann (de)serialisation")
TEST_CASE("nlohmann (de)serialisation" * doctest::test_suite("serialisation"))
{
const auto k0 = "abc";
const auto v0 = 123;
@ -312,7 +376,7 @@ TEST_CASE("nlohmann (de)serialisation")
auto r = std::make_shared<kv::StubConsensus>();
using Table = Store::Map<std::vector<int>, std::string>;
Store s0(r), s1;
auto& t = s0.create<Table>("t");
auto& t = s0.create<Table>("t", kv::SecurityDomain::PUBLIC);
s1.create<Table>("t");
Store::Tx tx;
@ -329,7 +393,7 @@ TEST_CASE("nlohmann (de)serialisation")
auto r = std::make_shared<kv::StubConsensus>();
using Table = Store::Map<nlohmann::json, nlohmann::json>;
Store s0(r), s1;
auto& t = s0.create<Table>("t");
auto& t = s0.create<Table>("t", kv::SecurityDomain::PUBLIC);
s1.create<Table>("t");
Store::Tx tx;

Просмотреть файл

@ -15,56 +15,6 @@
using namespace ccfapp;
struct CustomClass
{
int m_i;
CustomClass() : CustomClass(-1) {}
CustomClass(int i) : m_i(i) {}
int get() const
{
return m_i;
}
void set(std::string val)
{
m_i = std::stoi(val);
}
CustomClass operator()()
{
CustomClass ret;
return ret;
}
bool operator<(const CustomClass& other) const
{
return m_i < other.m_i;
}
bool operator==(const CustomClass& other) const
{
return !(other < *this) && !(*this < other);
}
MSGPACK_DEFINE(m_i);
};
namespace std
{
template <>
struct hash<CustomClass>
{
std::size_t operator()(const CustomClass& inst) const
{
return inst.get();
}
};
}
DECLARE_JSON_TYPE(CustomClass)
DECLARE_JSON_REQUIRED_FIELDS(CustomClass, m_i)
TEST_CASE("Map creation")
{
Store kv_store;
@ -255,8 +205,10 @@ TEST_CASE("Rollback and compact")
TEST_CASE("Clear entire store")
{
Store kv_store;
auto& map1 = kv_store.create<std::string, std::string>("map1");
auto& map2 = kv_store.create<std::string, std::string>("map2");
auto& map1 = kv_store.create<std::string, std::string>(
"map1", kv::SecurityDomain::PUBLIC);
auto& map2 = kv_store.create<std::string, std::string>(
"map2", kv::SecurityDomain::PUBLIC);
INFO("Commit a transaction over two maps");
{
@ -365,8 +317,9 @@ TEST_CASE("Global commit hooks")
Store kv_store;
auto& map_with_hook = kv_store.create<std::string, std::string>(
"map_with_hook", kv::SecurityDomain::PRIVATE, nullptr, global_hook);
auto& map_no_hook = kv_store.create<std::string, std::string>("map_no_hook");
"map_with_hook", kv::SecurityDomain::PUBLIC, nullptr, global_hook);
auto& map_no_hook = kv_store.create<std::string, std::string>(
"map_no_hook", kv::SecurityDomain::PUBLIC);
INFO("Compact an empty store");
{
@ -458,51 +411,11 @@ TEST_CASE("Global commit hooks")
}
}
TEST_CASE("Custom type serialisation test")
{
Store kv_store;
auto& map = kv_store.create<CustomClass, CustomClass>("map");
CustomClass k(3);
CustomClass v1(33);
CustomClass k2(2);
CustomClass v2(22);
INFO("Serialise/Deserialise 2 kv stores");
{
Store kv_store2;
auto& map2 = kv_store2.create<CustomClass, CustomClass>("map");
Store::Tx tx(kv_store.next_version());
auto view = tx.get_view(map);
view->put(k, v1);
view->put(k2, v2);
auto [success, reqid, serialised] = tx.commit_reserved();
REQUIRE(success == kv::CommitSuccess::OK);
kv_store.compact(view->end_order());
REQUIRE(kv_store2.deserialise(serialised) == kv::DeserialiseSuccess::PASS);
Store::Tx tx2;
auto view2 = tx2.get_view(map2);
auto va = view2->get(k);
REQUIRE(va.has_value());
REQUIRE(va.value() == v1);
auto vb = view2->get(k2);
REQUIRE(vb.has_value());
REQUIRE(vb.value() == v2);
// we only require operator==() to be implemented, so for consistency -
// this is the operator we use for comparison, and not operator!=()
REQUIRE(!(vb.value() == v1));
}
}
TEST_CASE("Clone schema")
{
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
Store store;
store.set_encryptor(encryptor);
auto& public_map =
store.create<size_t, std::string>("public", kv::SecurityDomain::PUBLIC);
@ -517,6 +430,7 @@ TEST_CASE("Clone schema")
Store clone;
clone.clone_schema(store);
clone.set_encryptor(encryptor);
REQUIRE(clone.deserialise(serialised) == kv::DeserialiseSuccess::PASS);
}
@ -524,9 +438,11 @@ TEST_CASE("Clone schema")
TEST_CASE("Deserialise return status")
{
Store store;
auto& signatures = store.create<ccf::Signatures>("signatures");
auto& nodes = store.create<ccf::Nodes>("nodes");
auto& data = store.create<size_t, size_t>("data");
auto& signatures =
store.create<ccf::Signatures>("signatures", kv::SecurityDomain::PUBLIC);
auto& nodes = store.create<ccf::Nodes>("nodes", kv::SecurityDomain::PUBLIC);
auto& data = store.create<size_t, size_t>("data", kv::SecurityDomain::PUBLIC);
auto kp = tls::make_key_pair();
@ -572,12 +488,16 @@ TEST_CASE("Deserialise return status")
TEST_CASE("map swap between stores")
{
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
Store s1;
s1.set_encryptor(encryptor);
auto& d1 = s1.create<size_t, size_t>("data", kv::SecurityDomain::PRIVATE);
auto& pd1 =
s1.create<size_t, size_t>("public_data", kv::SecurityDomain::PUBLIC);
Store s2;
s2.set_encryptor(encryptor);
auto& d2 = s2.create<size_t, size_t>("data", kv::SecurityDomain::PRIVATE);
auto& pd2 =
s2.create<size_t, size_t>("public_data", kv::SecurityDomain::PUBLIC);
@ -669,13 +589,16 @@ TEST_CASE("invalid map swaps")
TEST_CASE("private recovery map swap")
{
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
Store s1;
s1.set_encryptor(encryptor);
auto& priv1 =
s1.create<size_t, size_t>("private", kv::SecurityDomain::PRIVATE);
auto& pub1 =
s1.create<size_t, std::string>("public", kv::SecurityDomain::PUBLIC);
Store s2;
s2.set_encryptor(encryptor);
auto& priv2 =
s2.create<size_t, size_t>("private", kv::SecurityDomain::PRIVATE);
auto& pub2 =

Просмотреть файл

@ -863,6 +863,13 @@ namespace ccf
return jsonrpc::error_response(
ctx.req.seq_no, jsonrpc::StandardErrorCodes::PARSE_ERROR, err);
}
catch (const kv::KvSerialiserException& e)
{
// If serialising the committed transaction fails, there is no way to
// recover safely (https://github.com/microsoft/CCF/issues/338).
// Better to abort.
LOG_FATAL_FMT(e.what());
}
catch (const std::exception& e)
{
return jsonrpc::error_response(

Просмотреть файл

@ -6,6 +6,7 @@
#include "ds/logger.h"
#include "enclave/appinterface.h"
#include "kv/test/stub_consensus.h"
#include "node/encryptor.h"
#include "node/entities.h"
#include "node/networkstate.h"
#include "node/rpc/jsonrpc.h"
@ -165,6 +166,8 @@ public:
auto kp = tls::make_key_pair();
ccf::NetworkState network;
ccf::NetworkState network2;
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
ccf::StubNodeState node;
std::vector<uint8_t> sign_json(nlohmann::json j)
@ -218,7 +221,7 @@ enclave::RPCContext member_rpc_ctx(0, member_caller);
void prepare_callers()
{
Store::Tx txs;
network.tables->set_encryptor(encryptor);
ccf::Certs* user_certs = &network.user_certs;
ccf::Certs* member_certs = &network.member_certs;
auto user_certs_view = txs.get_view(*user_certs);

Просмотреть файл

@ -6,6 +6,7 @@
#include "ds/logger.h"
#include "enclave/appinterface.h"
#include "node/clientsignatures.h"
#include "node/encryptor.h"
#include "node/genesisgen.h"
#include "node/rpc/jsonrpc.h"
#include "node/rpc/memberfrontend.h"
@ -32,6 +33,7 @@ auto kp = tls::make_key_pair();
auto ca_mem = kp -> self_sign("CN=name_member");
auto verifier_mem = tls::make_verifier(ca_mem);
auto member_caller = verifier_mem -> raw_cert_data();
auto encryptor = std::make_shared<ccf::NullTxEncryptor>();
string get_script_path(string name)
{
@ -970,6 +972,7 @@ TEST_CASE("Complete proposal after initial rejection")
TEST_CASE("Add user via proposed call")
{
NetworkTables network;
network.tables->set_encryptor(encryptor);
GenesisGenerator gen(network);
gen.init_values();
StubNodeState node;