Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Commit

Permalink
Resolve #66: Add authority recursion depth limit
Browse files Browse the repository at this point in the history 
We now enforce a recursion depth limit for authority checks. If the
limit is hit, it silently rejects the authority.
  • Loading branch information
@nathanielhourt
nathanielhourt committed Aug 10, 2017
1 parent 19c1a3e commit 543a687
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 105 deletions.
4 changes: 3 additions & 1 deletion libraries/chain/chain_controller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,15 +485,17 @@ void chain_controller::check_transaction_authorization(const SignedTransaction&
return;
}


auto getPermission = [&db=_db](const types::AccountPermission& permission) {
auto key = boost::make_tuple(permission.account, permission.permission);
return db.get<permission_object, by_owner>(key);
};
auto getAuthority = [&getPermission](const types::AccountPermission& permission) {
return getPermission(permission).auth;
};
auto depthLimit = get_global_properties().configuration.authDepthLimit;
#warning TODO: Use a real chain_id here (where is this stored? Do we still need it?)
auto checker = MakeAuthorityChecker(std::move(getAuthority), trx.get_signature_keys(chain_id_type{}));
auto checker = MakeAuthorityChecker(std::move(getAuthority), depthLimit, trx.get_signature_keys(chain_id_type{}));

for (const auto& message : trx.messages)
for (const auto& declaredAuthority : message.authorization) {
Expand Down
30 changes: 19 additions & 11 deletions libraries/chain/include/eos/chain/authority_checker.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,19 @@ using MetaPermissionSet = boost::container::flat_multiset<MetaPermission, MetaPe
template<typename F>
class AuthorityChecker {
F PermissionToAuthority;
UInt16 recursionDepthLimit;
vector<public_key_type> signingKeys;
vector<bool> usedKeys;

struct WeightTallyVisitor {
using result_type = UInt32;

AuthorityChecker& checker;
UInt16 recursionDepth;
UInt32 totalWeight = 0;

WeightTallyVisitor(AuthorityChecker& checker)
: checker(checker) {}
WeightTallyVisitor(AuthorityChecker& checker, UInt16 recursionDepth)
: checker(checker), recursionDepth(recursionDepth) {}

UInt32 operator()(const types::KeyPermissionWeight& permission) {
auto itr = boost::find(checker.signingKeys, permission.key);
Expand All @@ -68,25 +70,30 @@ class AuthorityChecker {
return totalWeight;
}
UInt32 operator()(const types::AccountPermissionWeight& permission) {
//TODO #66: Recursion limit? Yes: implement as producer-configurable parameter
if (checker.satisfied(permission.permission))
if (recursionDepth < checker.recursionDepthLimit
&& checker.satisfied(permission.permission, recursionDepth + 1))
totalWeight += permission.weight;
return totalWeight;
}
};

public:
AuthorityChecker(F PermissionToAuthority, const flat_set<public_key_type>& signingKeys)
AuthorityChecker(F PermissionToAuthority, UInt16 recursionDepthLimit, const flat_set<public_key_type>& signingKeys)
: PermissionToAuthority(PermissionToAuthority),
recursionDepthLimit(recursionDepthLimit),
signingKeys(signingKeys.begin(), signingKeys.end()),
usedKeys(signingKeys.size(), false)
{}

bool satisfied(const types::AccountPermission& permission) {
return satisfied(PermissionToAuthority(permission));
bool satisfied(const types::AccountPermission& permission, UInt16 depth = 0) {
return satisfied(PermissionToAuthority(permission), depth);
}
template<typename AuthorityType>
bool satisfied(const AuthorityType& authority) {
bool satisfied(const AuthorityType& authority, UInt16 depth = 0) {
// This check is redundant, since WeightTallyVisitor did it too, but I'll leave it here for future-proofing
if (depth > recursionDepthLimit)
return false;

// Save the current used keys; if we do not satisfy this authority, the newly used keys aren't actually used
auto KeyReverter = fc::make_scoped_exit([this, keys = usedKeys] () mutable {
usedKeys = keys;
Expand All @@ -98,7 +105,7 @@ class AuthorityChecker {
permissions.insert(authority.accounts.begin(), authority.accounts.end());

// Check all permissions, from highest weight to lowest, seeing if signingKeys satisfies them or not
WeightTallyVisitor visitor(*this);
WeightTallyVisitor visitor(*this, depth);
for (const auto& permission : permissions)
// If we've got enough weight, to satisfy the authority, return!
if (permission.visit(visitor) >= authority.threshold) {
Expand All @@ -120,8 +127,9 @@ class AuthorityChecker {
};

template<typename F>
AuthorityChecker<F> MakeAuthorityChecker(F&& pta, const flat_set<public_key_type>& signingKeys) {
return AuthorityChecker<F>(std::forward<F>(pta), signingKeys);
AuthorityChecker<F> MakeAuthorityChecker(F&& pta, UInt16 recursionDepthLimit,
const flat_set<public_key_type>& signingKeys) {
return AuthorityChecker<F>(std::forward<F>(pta), recursionDepthLimit, signingKeys);
}

}} // namespace eos::chain
2 changes: 2 additions & 0 deletions libraries/chain/include/eos/chain/config.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <eos/types/types.hpp>

namespace eos { namespace config {
using types::UInt16;
using types::UInt32;
using types::UInt64;
using types::UInt128;
Expand Down Expand Up @@ -59,6 +60,7 @@ const static ShareType DefaultElectedPay = Asset(100).amount;
const static ShareType DefaultRunnerUpPay = Asset(75).amount;
const static ShareType DefaultMinEosBalance = Asset(100).amount;
const static UInt32 DefaultMaxTrxLifetime = 60*60;
const static UInt16 DefaultAuthDepthLimit = 6;
const static UInt32 ProducersAuthorityThreshold = 14;

const static int BlocksPerRound = 21;
Expand Down
3 changes: 2 additions & 1 deletion libraries/native_contract/include/eos/native_contract/genesis_state.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ struct genesis_state_type {
config::DefaultElectedPay,
config::DefaultRunnerUpPay,
config::DefaultMinEosBalance,
config::DefaultMaxTrxLifetime
config::DefaultMaxTrxLifetime,
config::DefaultAuthDepthLimit
};
vector<initial_account_type> initial_accounts;
vector<initial_producer_type> initial_producers;
Expand Down
1 change: 1 addition & 0 deletions libraries/types/types.eos
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct BlockchainConfiguration
runnerUpPay ShareType
minEosBalance ShareType
maxTrxLifetime UInt32
authDepthLimit UInt16

struct TypeDef
newTypeName TypeName
Expand Down
3 changes: 2 additions & 1 deletion tests/common/database_fixture.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ void testing_blockchain::sign_transaction(SignedTransaction& trx) {
auto key = boost::make_tuple(permission.account, permission.permission);
return db.get<permission_object, by_owner>(key).auth;
};
auto checker = MakeAuthorityChecker(GetAuthority, fixture.available_keys());
auto checker = MakeAuthorityChecker(GetAuthority, get_global_properties().configuration.authDepthLimit,
fixture.available_keys());

for (const auto& message : trx.messages)
for (const auto& authorization : message.authorization)
Expand Down
93 changes: 48 additions & 45 deletions tests/tests/misc_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,28 @@ BOOST_AUTO_TEST_SUITE(misc_tests)
BOOST_AUTO_TEST_CASE(median_properties_test)
{ try {
vector<BlockchainConfiguration> votes{
{1024 , 512 , 4096 , Asset(5000 ).amount, Asset(4000 ).amount, Asset(100 ).amount, 512 },
{10000 , 100 , 4096 , Asset(3333 ).amount, Asset(27109 ).amount, Asset(10 ).amount, 100 },
{2048 , 1500 , 1000 , Asset(5432 ).amount, Asset(2000 ).amount, Asset(50 ).amount, 1500 },
{100 , 25 , 1024 , Asset(90000 ).amount, Asset(0 ).amount, Asset(433 ).amount, 25 },
{1024 , 1000 , 100 , Asset(10 ).amount, Asset(50 ).amount, Asset(200 ).amount, 1000 },
{1024 , 512 , 4096 , Asset(5000 ).amount, Asset(4000 ).amount, Asset(100 ).amount, 512 , 6},
{10000 , 100 , 4096 , Asset(3333 ).amount, Asset(27109 ).amount, Asset(10 ).amount, 100 , 6},
{2048 , 1500 , 1000 , Asset(5432 ).amount, Asset(2000 ).amount, Asset(50 ).amount, 1500 , 6},
{100 , 25 , 1024 , Asset(90000 ).amount, Asset(0 ).amount, Asset(433 ).amount, 25 , 6},
{1024 , 1000 , 100 , Asset(10 ).amount, Asset(50 ).amount, Asset(200 ).amount, 1000 , 6},
};
BlockchainConfiguration medians{
1024, 512, 1024, Asset(5000).amount, Asset(2000).amount, Asset(100).amount, 512
1024, 512, 1024, Asset(5000).amount, Asset(2000).amount, Asset(100).amount, 512, 6
};

BOOST_CHECK_EQUAL(BlockchainConfiguration::get_median_values(votes), medians);

votes.emplace_back(BlockchainConfiguration{1, 1, 1, 1, 1, 1, 1});
votes.emplace_back(BlockchainConfiguration{1, 1, 1, 1, 1, 1, 1});
medians = BlockchainConfiguration {1024, 100, 1000, Asset(3333).amount, Asset(50).amount, Asset(50).amount, 100};
votes.emplace_back(BlockchainConfiguration{1, 1, 1, 1, 1, 1, 1, 1});
votes.emplace_back(BlockchainConfiguration{1, 1, 1, 1, 1, 1, 1, 1});
medians = BlockchainConfiguration {1024, 100, 1000, Asset(3333).amount, Asset(50).amount, Asset(50).amount, 100, 6};

BOOST_CHECK_EQUAL(BlockchainConfiguration::get_median_values(votes), medians);
BOOST_CHECK_EQUAL(BlockchainConfiguration::get_median_values({medians}), medians);

votes.erase(votes.begin() + 2);
votes.erase(votes.end() - 1);
medians = BlockchainConfiguration {1024, 100, 1024, Asset(3333).amount, Asset(50).amount, Asset(100).amount, 100};
medians = BlockchainConfiguration {1024, 100, 1024, Asset(3333).amount, Asset(50).amount, Asset(100).amount, 100, 6};
BOOST_CHECK_EQUAL(BlockchainConfiguration::get_median_values(votes), medians);
} FC_LOG_AND_RETHROW() }

Expand Down Expand Up @@ -93,21 +93,21 @@ BOOST_AUTO_TEST_CASE(authority_checker)

auto A = Complex_Authority(2, ((a,1))((b,1)),);
{
auto checker = MakeAuthorityChecker(GetNullAuthority, {a, b});
auto checker = MakeAuthorityChecker(GetNullAuthority, 2, {a, b});
BOOST_CHECK(checker.satisfied(A));
BOOST_CHECK(checker.all_keys_used());
BOOST_CHECK_EQUAL(checker.used_keys().size(), 2);
BOOST_CHECK_EQUAL(checker.unused_keys().size(), 0);
}
{
auto checker = MakeAuthorityChecker(GetNullAuthority, {a, c});
auto checker = MakeAuthorityChecker(GetNullAuthority, 2, {a, c});
BOOST_CHECK(!checker.satisfied(A));
BOOST_CHECK(!checker.all_keys_used());
BOOST_CHECK_EQUAL(checker.used_keys().size(), 0);
BOOST_CHECK_EQUAL(checker.unused_keys().size(), 2);
}
{
auto checker = MakeAuthorityChecker(GetNullAuthority, {a, b, c});
auto checker = MakeAuthorityChecker(GetNullAuthority, 2, {a, b, c});
BOOST_CHECK(checker.satisfied(A));
BOOST_CHECK(!checker.all_keys_used());
BOOST_CHECK_EQUAL(checker.used_keys().size(), 2);
Expand All @@ -117,52 +117,52 @@ BOOST_AUTO_TEST_CASE(authority_checker)
BOOST_CHECK_EQUAL(checker.unused_keys().count(c), 1);
}
{
auto checker = MakeAuthorityChecker(GetNullAuthority, {b, c});
auto checker = MakeAuthorityChecker(GetNullAuthority, 2, {b, c});
BOOST_CHECK(!checker.satisfied(A));
BOOST_CHECK(!checker.all_keys_used());
BOOST_CHECK_EQUAL(checker.used_keys().size(), 0);
}

A = Complex_Authority(3, ((a,1))((b,1))((c,1)),);
BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {c, b, a}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {a, b}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {a, c}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {b, c}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, 2, {c, b, a}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, 2, {a, b}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, 2, {a, c}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, 2, {b, c}).satisfied(A));

A = Complex_Authority(1, ((a, 1))((b, 1)),);
BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {a}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {b}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {c}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, 2, {a}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, 2, {b}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, 2, {c}).satisfied(A));

A = Complex_Authority(1, ((a, 2))((b, 1)),);
BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {a}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, {b}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, {c}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, 2, {a}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetNullAuthority, 2, {b}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetNullAuthority, 2, {c}).satisfied(A));

auto GetCAuthority = [c_public_key](auto){return Complex_Authority(1, ((c, 1)),);};

A = Complex_Authority(2, ((a, 2))((b, 1)), (("hello", "world", 1)));
{
auto checker = MakeAuthorityChecker(GetCAuthority, {a});
auto checker = MakeAuthorityChecker(GetCAuthority, 2, {a});
BOOST_CHECK(checker.satisfied(A));
BOOST_CHECK(checker.all_keys_used());
}
{
auto checker = MakeAuthorityChecker(GetCAuthority, {b});
auto checker = MakeAuthorityChecker(GetCAuthority, 2, {b});
BOOST_CHECK(!checker.satisfied(A));
BOOST_CHECK_EQUAL(checker.used_keys().size(), 0);
BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1);
BOOST_CHECK_EQUAL(checker.unused_keys().count(b), 1);
}
{
auto checker = MakeAuthorityChecker(GetCAuthority, {c});
auto checker = MakeAuthorityChecker(GetCAuthority, 2, {c});
BOOST_CHECK(!checker.satisfied(A));
BOOST_CHECK_EQUAL(checker.used_keys().size(), 0);
BOOST_CHECK_EQUAL(checker.unused_keys().size(), 1);
BOOST_CHECK_EQUAL(checker.unused_keys().count(c), 1);
}
{
auto checker = MakeAuthorityChecker(GetCAuthority, {b,c});
auto checker = MakeAuthorityChecker(GetCAuthority, 2, {b,c});
BOOST_CHECK(checker.satisfied(A));
BOOST_CHECK(checker.all_keys_used());
BOOST_CHECK_EQUAL(checker.used_keys().size(), 2);
Expand All @@ -171,7 +171,7 @@ BOOST_AUTO_TEST_CASE(authority_checker)
BOOST_CHECK_EQUAL(checker.used_keys().count(c), 1);
}
{
auto checker = MakeAuthorityChecker(GetCAuthority, {b,c,a});
auto checker = MakeAuthorityChecker(GetCAuthority, 2, {b,c,a});
BOOST_CHECK(checker.satisfied(A));
BOOST_CHECK(!checker.all_keys_used());
BOOST_CHECK_EQUAL(checker.used_keys().size(), 1);
Expand All @@ -182,14 +182,14 @@ BOOST_AUTO_TEST_CASE(authority_checker)
}

A = Complex_Authority(2, ((a, 1))((b, 1)), (("hello", "world", 1)));
BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {a}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {b}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {c}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a,b}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {b,c}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a,c}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, 2, {a}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, 2, {b}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, 2, {c}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, 2, {a,b}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, 2, {b,c}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, 2, {a,c}).satisfied(A));
{
auto checker = MakeAuthorityChecker(GetCAuthority, {a,b,c});
auto checker = MakeAuthorityChecker(GetCAuthority, 2, {a,b,c});
BOOST_CHECK(checker.satisfied(A));
BOOST_CHECK(!checker.all_keys_used());
BOOST_CHECK_EQUAL(checker.used_keys().size(), 2);
Expand All @@ -198,12 +198,12 @@ BOOST_AUTO_TEST_CASE(authority_checker)
}

A = Complex_Authority(2, ((a, 1))((b, 1)), (("hello", "world", 2)));
BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {a,b}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, {c}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {a}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, {b}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, 2, {a,b}).satisfied(A));
BOOST_CHECK(MakeAuthorityChecker(GetCAuthority, 2, {c}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, 2, {a}).satisfied(A));
BOOST_CHECK(!MakeAuthorityChecker(GetCAuthority, 2, {b}).satisfied(A));
{
auto checker = MakeAuthorityChecker(GetCAuthority, {a,b,c});
auto checker = MakeAuthorityChecker(GetCAuthority, 2, {a,b,c});
BOOST_CHECK(checker.satisfied(A));
BOOST_CHECK(!checker.all_keys_used());
BOOST_CHECK_EQUAL(checker.used_keys().size(), 1);
Expand All @@ -224,12 +224,12 @@ BOOST_AUTO_TEST_CASE(authority_checker)

A = Complex_Authority(5, ((a, 2))((b, 2))((c, 2)), (("top", "top", 5)));
{
auto checker = MakeAuthorityChecker(GetAuthority, {d, e});
auto checker = MakeAuthorityChecker(GetAuthority, 2, {d, e});
BOOST_CHECK(checker.satisfied(A));
BOOST_CHECK(checker.all_keys_used());
}
{
auto checker = MakeAuthorityChecker(GetAuthority, {a,b,c,d,e});
auto checker = MakeAuthorityChecker(GetAuthority, 2, {a,b,c,d,e});
BOOST_CHECK(checker.satisfied(A));
BOOST_CHECK(!checker.all_keys_used());
BOOST_CHECK_EQUAL(checker.used_keys().size(), 2);
Expand All @@ -238,7 +238,7 @@ BOOST_AUTO_TEST_CASE(authority_checker)
BOOST_CHECK_EQUAL(checker.used_keys().count(e), 1);
}
{
auto checker = MakeAuthorityChecker(GetAuthority, {a,b,c,e});
auto checker = MakeAuthorityChecker(GetAuthority, 2, {a,b,c,e});
BOOST_CHECK(checker.satisfied(A));
BOOST_CHECK(!checker.all_keys_used());
BOOST_CHECK_EQUAL(checker.used_keys().size(), 3);
Expand All @@ -247,11 +247,14 @@ BOOST_AUTO_TEST_CASE(authority_checker)
BOOST_CHECK_EQUAL(checker.used_keys().count(b), 1);
BOOST_CHECK_EQUAL(checker.used_keys().count(c), 1);
}
BOOST_CHECK(MakeAuthorityChecker(GetAuthority, 1, {a,b,c}).satisfied(A));
// Fails due to short recursion depth limit
BOOST_CHECK(!MakeAuthorityChecker(GetAuthority, 1, {d,e}).satisfied(A));

A = Complex_Authority(2, ((a, 1))((b, 1))((c, 1)),);
auto B = Complex_Authority(1, ((b, 1))((c, 1)),);
{
auto checker = MakeAuthorityChecker(GetNullAuthority, {a,b,c});
auto checker = MakeAuthorityChecker(GetNullAuthority, 2, {a,b,c});
BOOST_CHECK(validate(A));
BOOST_CHECK(validate(B));
BOOST_CHECK(checker.satisfied(A));
Expand Down
Loading

0 comments on commit 543a687

Please sign in to comment.