Skip to content

Commit

Permalink
Make CreateAccount cmd validation stronger (hyperledger-iroha#1966)
Browse files Browse the repository at this point in the history
CreateAccount command now respects a set of perms of tx creator

It should not be possible to create an account in a domain which
default role has some permissions that does not have a creator of
a transaction.

Signed-off-by: Igor Egorov <[email protected]>
  • Loading branch information
Igor Egorov authored Dec 28, 2018
1 parent 0e645f4 commit 2dd468e
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 4 deletions.
26 changes: 22 additions & 4 deletions irohad/ametsuchi/impl/postgres_command_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,8 @@ namespace {
END
ELSE false END
)") % checkAccountRolePermission(global_permission, creator_id)
% checkAccountRolePermission(domain_permission, creator_id)
% checkAccountRolePermission(domain_permission,
creator_id)
% creator_id % id_with_target_domain)
.str();
return query;
Expand Down Expand Up @@ -1286,13 +1287,30 @@ namespace iroha {
{"createAccount",
createAccountBase,
{(boost::format(R"(
has_perm AS (%s),)")
domain_role_permissions_bits AS (
SELECT COALESCE(bit_or(rhp.permission), '0'::bit(%1%)) AS bits
FROM role_has_permissions AS rhp
WHERE rhp.role_id = (SELECT * FROM get_domain_default_role)),
account_permissions AS (
SELECT COALESCE(bit_or(rhp.permission), '0'::bit(%1%)) AS perm
FROM role_has_permissions AS rhp
JOIN account_has_roles AS ar ON ar.role_id = rhp.role_id
WHERE ar.account_id = $1
),
creator_has_enough_permissions AS (
SELECT ap.perm & dpb.bits = dpb.bits
FROM account_permissions AS ap, domain_role_permissions_bits AS dpb
),
has_perm AS (%2%),
)") % bits
% checkAccountRolePermission(
shared_model::interface::permissions::Role::kCreateAccount,
"$1"))
.str(),
R"(AND (SELECT * FROM has_perm))",
R"(WHEN NOT (SELECT * FROM has_perm) THEN 2)"}});
R"(AND (SELECT * FROM has_perm)
AND (SELECT * FROM creator_has_enough_permissions))",
R"(WHEN NOT (SELECT * FROM has_perm) THEN 2
WHEN NOT (SELECT * FROM creator_has_enough_permissions) THEN 2)"}});

statements.push_back(
{"createAsset",
Expand Down
36 changes: 36 additions & 0 deletions test/integration/acceptance/create_account_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ using namespace integration_framework;
using namespace shared_model;
using namespace common_constants;

// TODO igor-egorov, 2018-12-27, IR-148, move all check macroses to
// acceptance_fixture.hpp
#define check(i) \
[](const auto &resp) { ASSERT_EQ(resp->transactions().size(), i); }

class CreateAccount : public AcceptanceFixture {
public:
auto makeUserWithPerms(const interface::RolePermissionSet &perms = {
Expand Down Expand Up @@ -156,3 +161,34 @@ TEST_F(CreateAccount, EmptyName) {
empty_name, kDomain, kNewUserKeypair.publicKey())),
CHECK_STATELESS_INVALID);
}

/**
* Checks that there is no privelege elevation issue via CreateAccount
*
* @given two domains: the first domain has default role that contain
* can_create_account permission, the second domain has default role that
* contains more permissions than default role of the first domain
* @when the user of an account from the first domain tries to create an account
* in the second domain
* @then transaction should fail stateful validation
*/
TEST_F(CreateAccount, PrivelegeElevation) {
auto second_domain_tx = complete(
baseTx(kAdminId).createDomain(kSecondDomain, kAdminRole), kAdminKeypair);
auto create_elevated_user = complete(baseTx().createAccount(
kNewUser, kSecondDomain, kNewUserKeypair.publicKey()));
auto rejected_hash = create_elevated_user.hash();

IntegrationTestFramework(1)
.setInitialState(kAdminKeypair)
.sendTxAwait(second_domain_tx, check(1))
.sendTxAwait(makeUserWithPerms(), check(1))
.sendTx(create_elevated_user)
.skipProposal()
.checkVerifiedProposal(check(0))
.checkBlock([&rejected_hash](const auto &block) {
const auto rejected_hashes = block->rejected_transactions_hashes();
ASSERT_THAT(rejected_hashes, ::testing::Contains(rejected_hash));
ASSERT_EQ(boost::size(rejected_hashes), 1);
});
}

0 comments on commit 2dd468e

Please sign in to comment.