From 46c7355f6aa67b4a512fe5af5c5d978b5cd0df6e Mon Sep 17 00:00:00 2001 From: James-Mart Date: Fri, 13 Dec 2024 22:16:24 +0000 Subject: [PATCH 1/3] support staged-tx in producers auth --- .../psibase/common/include/psibase/block.hpp | 6 +- .../include/services/system/Accounts.hpp | 6 + services/system/Accounts/src/Accounts.cpp | 7 + .../include/services/system/Producers.hpp | 37 ++++- services/system/Producers/src/Producers.cpp | 146 +++++++++++++++--- .../include/services/system/AuthStack.hpp | 51 ++++++ 6 files changed, 229 insertions(+), 24 deletions(-) create mode 100644 services/system/Transact/include/services/system/AuthStack.hpp diff --git a/libraries/psibase/common/include/psibase/block.hpp b/libraries/psibase/common/include/psibase/block.hpp index d48a727b8..3c514e2a7 100644 --- a/libraries/psibase/common/include/psibase/block.hpp +++ b/libraries/psibase/common/include/psibase/block.hpp @@ -146,7 +146,11 @@ namespace psibase { AccountNumber name; Claim auth; - friend bool operator==(const Producer&, const Producer&) = default; + + static AccountNumber getName(const Producer& p) { return p.name; } + static Claim getAuthClaim(const Producer& p) { return p.auth; } + + friend bool operator==(const Producer&, const Producer&) = default; PSIO_REFLECT(Producer, name, auth); }; diff --git a/services/system/Accounts/include/services/system/Accounts.hpp b/services/system/Accounts/include/services/system/Accounts.hpp index b89861b55..180f55683 100644 --- a/services/system/Accounts/include/services/system/Accounts.hpp +++ b/services/system/Accounts/include/services/system/Accounts.hpp @@ -104,6 +104,11 @@ namespace SystemService /// Returns data about an account from the accounts table std::optional getAccount(psibase::AccountNumber name); + /// Returns the auth service of the specified account + /// + /// Aborts if the account does not exist + psibase::AccountNumber getAuthOf(psibase::AccountNumber account); + /// Return value indicates whether the account `name` exists bool exists(psibase::AccountNumber name); @@ -117,6 +122,7 @@ namespace SystemService method(newAccount, name, authService, requireNew), method(setAuthServ, authService), method(getAccount, name), + method(getAuthOf, account), method(exists, name), method(billCpu, name, amount) // diff --git a/services/system/Accounts/src/Accounts.cpp b/services/system/Accounts/src/Accounts.cpp index 4cdc525dc..892cbbcce 100644 --- a/services/system/Accounts/src/Accounts.cpp +++ b/services/system/Accounts/src/Accounts.cpp @@ -117,6 +117,13 @@ namespace SystemService return accountIndex.get(name); } + psibase::AccountNumber Accounts::getAuthOf(psibase::AccountNumber account) + { + auto accountRow = getAccount(account); + check(accountRow.has_value(), "account does not exist"); + return accountRow->authService; + } + bool Accounts::exists(AccountNumber name) { return getAccount(name) != std::nullopt; diff --git a/services/system/Producers/include/services/system/Producers.hpp b/services/system/Producers/include/services/system/Producers.hpp index e0f6c8ece..9500ba13f 100644 --- a/services/system/Producers/include/services/system/Producers.hpp +++ b/services/system/Producers/include/services/system/Producers.hpp @@ -43,10 +43,45 @@ namespace SystemService std::vector allowedActions, std::vector claims); void canAuthUserSys(psibase::AccountNumber user); + + /// Check whether a specified set of authorizer accounts are sufficient to authorize sending a + /// transaction from a specified sender. + /// + /// * `sender`: The sender account for the transaction potentially being authorized. + /// * `authorizers`: The set of accounts that have already authorized the execution of the transaction. + /// + /// Returns: + /// * `true`: If the total authorizations from `authorizers` or their auth services meets sender's threshold + /// * `false`: If not returning true, or on recursive checks for the same sender + bool isAuthSys(psibase::AccountNumber sender, + std::vector authorizers); + + /// Check whether a specified set of rejecter accounts are sufficient to reject (cancel) a + /// transaction from a specified sender. + /// + /// * `sender`: The sender account for the transaction potentially being rejected. + /// * `rejecters`: The set of accounts that have already authorized the rejection of the transaction. + /// + /// Returns: + /// * `true`: If the total authorizations from `rejecters` or their auth services meets sender's threshold + /// * `false`: If not returning true, or on recursive checks for the same sender + bool isRejectSys(psibase::AccountNumber sender, + std::vector rejecters); + + private: + using IndirectCheckFunc = + bool (psibase::Actor::*)(psibase::AccountNumber, + const std::vector&); + bool checkOverlapping(std::vector&& producers, + std::vector&& authorizers, + std::size_t threshold, + IndirectCheckFunc indirectCheck); }; PSIO_REFLECT(Producers, method(setConsensus, consensus), method(setProducers, producers), method(checkAuthSys, flags, requester, sender, action, allowedActions, claims), - method(canAuthUserSys, user)) + method(canAuthUserSys, user), + method(isAuthSys, sender, authorizers), + method(isRejectSys, sender, rejecters)) } // namespace SystemService diff --git a/services/system/Producers/src/Producers.cpp b/services/system/Producers/src/Producers.cpp index a94654fe1..01ccc31ac 100644 --- a/services/system/Producers/src/Producers.cpp +++ b/services/system/Producers/src/Producers.cpp @@ -1,5 +1,8 @@ #include #include +#include +#include +#include #include using namespace psibase; @@ -8,6 +11,13 @@ namespace { auto compare_claim = [](const Claim& lhs, const Claim& rhs) { return std::tie(lhs.service, lhs.rawData) < std::tie(rhs.service, rhs.rawData); }; + + bool satisfiesClaim(const Claim& expected, const std::vector& claims_sorted) + { + return expected == Claim{} + || std::ranges::binary_search(claims_sorted, expected, compare_claim); + } + } // namespace namespace SystemService @@ -90,6 +100,41 @@ namespace SystemService return bft.producers.size() * 2 / 3 + 1; } + struct ConsensusDataUtility + { + ConsensusData data; + + ConsensusDataUtility() + { + auto status = kvGet(StatusRow::db, StatusRow::key()); + check(status.has_value(), "status row invalid"); + data = status->consensus.current.data; + } + + std::size_t getThreshold(AccountNumber sender) const + { + return std::visit([&](const auto& c) { return SystemService::getThreshold(c, sender); }, + data); + } + + std::size_t getAntiThreshold(AccountNumber sender) const + { + return getNrProds() - getThreshold(sender) + 1; + } + + std::size_t getNrProds() const + { + return std::visit([&](const auto& c) { return c.producers.size(); }, data); + } + + std::vector getProducers() const + { + std::vector producers; + std::visit([&](const auto& c) { producers = c.producers; }, data); + return producers; + } + }; + void Producers::checkAuthSys(uint32_t flags, psibase::AccountNumber requester, psibase::AccountNumber sender, @@ -113,31 +158,21 @@ namespace SystemService else if (type != AuthInterface::topActionReq) abortMessage("unsupported auth type"); - auto status = psibase::kvGet(StatusRow::db, StatusRow::key()); + auto consensus = ConsensusDataUtility{}; + auto expectedClaims = consensus.getProducers() // + | std::views::transform(Producer::getAuthClaim) // + | std::ranges::to(); - std::vector expectedClaims; - std::visit( - [&](auto& c) - { - for (const auto& [name, auth] : c.producers) - { - expectedClaims.push_back(auth); - } - }, - status->consensus.current.data); - std::sort(claims.begin(), claims.end(), compare_claim); - auto matching = std::ranges::count_if( - expectedClaims, [&](const auto& claim) - { return claim == Claim{} || std::ranges::binary_search(claims, claim, compare_claim); }); - - auto threshold = expectedClaims.empty() - ? 0 - : std::visit([&](const auto& c) { return getThreshold(c, sender); }, - status->consensus.current.data); + std::ranges::sort(claims, compare_claim); + auto matching = std::ranges::count_if(expectedClaims, [&](const auto& expected) { // + return satisfiesClaim(expected, claims); + }); + + auto threshold = expectedClaims.empty() ? 0 : consensus.getThreshold(sender); if (matching < threshold) { - abortMessage("runAs: have " + std::to_string(matching) + "/" + std::to_string(threshold) + - " producers required to authorize"); + abortMessage("runAs: have " + std::to_string(matching) + "/" + std::to_string(threshold) + + " producers required to authorize"); } } @@ -147,6 +182,73 @@ namespace SystemService "Can only authorize predefined accounts"); } + bool Producers::checkOverlapping(std::vector&& producers, + std::vector&& authorizers, + std::size_t threshold, + IndirectCheckFunc indirectCheck) + { + // We only check for indirect auth if there are insufficient direct auths. + auto nonOverlapping = std::ranges::partition( + producers, [&](const auto& p) { return std::ranges::contains(authorizers, p); }); + auto numOverlapping = std::ranges::distance(producers.begin(), nonOverlapping.begin()); + if (numOverlapping >= threshold) + { + return true; + } + + // Now check for indirect authorization + for (const auto& account : + std::ranges::subrange(nonOverlapping.begin(), nonOverlapping.end())) + { + auto toAuth = Actor{service, to().getAuthOf(account)}; + + if ((toAuth.*indirectCheck)(account, authorizers) && ++numOverlapping >= threshold) + { + return true; + } + } + } + + bool Producers::isAuthSys(AccountNumber sender, std::vector authorizers) + { + // Base case to prevent infinite recursion + if (AuthStack::instance().inStack(sender)) + return false; + AuthStackGuard guard(sender); + + auto consensus = ConsensusDataUtility{}; + auto producers = consensus.getProducers() // + | std::views::transform(Producer::getName) // + | std::ranges::to(); + auto threshold = producers.empty() ? 0 : consensus.getThreshold(sender); + + auto _ = recurse(); + return checkOverlapping(std::move(producers), std::move(authorizers), threshold, + &Actor::isAuthSys); + } + + bool Producers::isRejectSys(psibase::AccountNumber sender, + std::vector rejecters) + { + // Base case to prevent infinite recursion + if (AuthStack::instance().inStack(sender)) + return false; + AuthStackGuard guard(sender); + + auto consensus = ConsensusDataUtility{}; + auto producers = consensus.getProducers() // + | std::views::transform(Producer::getName) // + | std::ranges::to(); + auto threshold = consensus.getAntiThreshold(sender); + + if (producers.empty()) + return false; + + auto _ = recurse(); + return checkOverlapping(std::move(producers), std::move(rejecters), threshold, + &Actor::isRejectSys); + } + } // namespace SystemService PSIBASE_DISPATCH(SystemService::Producers) diff --git a/services/system/Transact/include/services/system/AuthStack.hpp b/services/system/Transact/include/services/system/AuthStack.hpp new file mode 100644 index 000000000..fc820f26e --- /dev/null +++ b/services/system/Transact/include/services/system/AuthStack.hpp @@ -0,0 +1,51 @@ +#pragma once + +#include +#include +#include +#include + +namespace SystemService +{ + // For use in auth services to track the stack of accounts in an authorization chain + // + // This is used to prevent infinite recursion in auth services. + class AuthStack + { + public: + std::vector senders; + + static AuthStack& instance() + { + static AuthStack instance; + return instance; + } + + void push(const psibase::AccountNumber& sender) { senders.push_back(sender); } + + void pop() + { + if (!senders.empty()) + { + senders.pop_back(); + } + } + + bool inStack(const psibase::AccountNumber& sender) const + { + return std::ranges::find(senders, sender) != senders.end(); + } + + private: + AuthStack() = default; + AuthStack(const AuthStack&) = delete; + AuthStack& operator=(const AuthStack&) = delete; + }; + + class AuthStackGuard + { + public: + AuthStackGuard(const psibase::AccountNumber& sender) { AuthStack::instance().push(sender); } + ~AuthStackGuard() { AuthStack::instance().pop(); } + }; +} // namespace SystemService From d7fb902ce8e30a3f345045cc8631cb457ae15d0d Mon Sep 17 00:00:00 2001 From: James-Mart Date: Fri, 13 Dec 2024 22:19:03 +0000 Subject: [PATCH 2/3] add recursion protection to auth-delegate --- .../include/services/system/AuthDelegate.hpp | 4 ++-- .../system/AuthDelegate/src/AuthDelegate.cpp | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/services/system/AuthDelegate/include/services/system/AuthDelegate.hpp b/services/system/AuthDelegate/include/services/system/AuthDelegate.hpp index 227bff4e0..b7a56bd59 100644 --- a/services/system/AuthDelegate/include/services/system/AuthDelegate.hpp +++ b/services/system/AuthDelegate/include/services/system/AuthDelegate.hpp @@ -61,7 +61,7 @@ namespace SystemService /// Returns: /// * `true`: If the sender's owner is among the authorizers, or if the sender's owner's auth /// service would authorize the transaction - /// * `false`: If not returning true + /// * `false`: If not returning true, or on recursive checks for the same sender bool isAuthSys(psibase::AccountNumber sender, std::vector authorizers); @@ -74,7 +74,7 @@ namespace SystemService /// Returns: /// * `true`: If the sender's owner is among the rejecters, or if the sender's owner's auth /// service would reject the transaction - /// * `false`: If not returning true + /// * `false`: If not returning true, or on recursive checks for the same sender bool isRejectSys(psibase::AccountNumber sender, std::vector rejecters); diff --git a/services/system/AuthDelegate/src/AuthDelegate.cpp b/services/system/AuthDelegate/src/AuthDelegate.cpp index 45c685f9b..9380615eb 100644 --- a/services/system/AuthDelegate/src/AuthDelegate.cpp +++ b/services/system/AuthDelegate/src/AuthDelegate.cpp @@ -2,6 +2,7 @@ #include #include +#include #include using namespace psibase; @@ -34,6 +35,11 @@ namespace SystemService bool AuthDelegate::isAuthSys(psibase::AccountNumber sender, std::vector authorizers) { + // Base case to prevent infinite recursion + if (AuthStack::instance().inStack(sender)) + return false; + AuthStackGuard guard(sender); + auto owner = getOwner(sender); if (std::ranges::contains(authorizers, owner)) @@ -46,6 +52,11 @@ namespace SystemService bool AuthDelegate::isRejectSys(psibase::AccountNumber sender, std::vector rejecters) { + // Base case to prevent infinite recursion + if (AuthStack::instance().inStack(sender)) + return false; + AuthStackGuard guard(sender); + auto owner = getOwner(sender); if (std::ranges::contains(rejecters, owner)) @@ -70,13 +81,7 @@ namespace SystemService Actor AuthDelegate::authServiceOf(psibase::AccountNumber account) { - auto accountRow = to().getAccount(account); - if (!accountRow) - { - abortMessage("unknown account \"" + account.str() + "\""); - } - - return Actor{service, accountRow->authService}; + return Actor{service, to().getAuthOf(account)}; } } // namespace SystemService From 9900a30f3e73430362de462ee2732c67ac009434 Mon Sep 17 00:00:00 2001 From: James-Mart Date: Fri, 13 Dec 2024 22:19:20 +0000 Subject: [PATCH 3/3] proposed clang-format update --- .clang-format | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-format b/.clang-format index ae42c73a0..a2a2f31f5 100644 --- a/.clang-format +++ b/.clang-format @@ -1,6 +1,7 @@ --- BasedOnStyle: Chromium BreakBeforeBraces: Allman +BreakBeforeBinaryOperators: NonAssignment ColumnLimit: '100' IndentWidth: '3' NamespaceIndentation: All