From c641a5cb8a025f7a75f7b88015d85e1a5cc6813a Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 25 Oct 2019 13:35:59 +0200 Subject: [PATCH 01/16] refactoring: move local functions to an anonymous namespace Change-Id: I9d9cf07877d59df88305e8847c3156a73e82efd8 --- src/internal/storage_backend_serialized.cpp | 154 ++++++++++++++-------------- 1 file changed, 79 insertions(+), 75 deletions(-) diff --git a/src/internal/storage_backend_serialized.cpp b/src/internal/storage_backend_serialized.cpp index 4859951..cf37c6b 100644 --- a/src/internal/storage_backend_serialized.cpp +++ b/src/internal/storage_backend_serialized.cpp @@ -43,6 +43,85 @@ typedef boost::tokenizer> tokenizer; template struct type_helper; +namespace { + +inline boost::string_ref s(const flatbuffers::String *str) { + return boost::string_ref(str->c_str(), str->size()); +} + +template +bool match(const T &match, const I *i) { + return match.match(makeMessageType(i->type()), + s(i->interface()), + s(i->path()), + s(i->member()), + s(i->name()), + i->is_name_prefix(), + makeDecision(i->decision()->decision())); +} + +template <> bool match(const ldp_xml_parser::MatchItemAccess &match, const FB::ItemAccess *item) { + return match.match(makeBusAccessType(item->type()), item->uid(), item->gid()); +} + +template ::policy_type> +ldp_xml_parser::DecisionItem getDecisionItem(const T &item, const P *policy) { + const auto *v = policy->items(); + for (auto it = v->rbegin(); it != v->rend(); ++it) { + if (match(item, *it)) + return makeDecisionItem((*it)->decision()); + } + return ldp_xml_parser::Decision::ANY; +} + +const DecisionItem *getDecisionItemFromTree(const FB::PolicyOwnNode *node, + tokenizer &tokens, + tokenizer::iterator &iterator) { + if (iterator == tokens.end()) { + if (node->decision_item()->decision() != Decision_ANY) + return node->decision_item(); + else + return node->prefix_decision_item(); + } + + auto child = node->children()->LookupByKey(iterator->c_str()); + if (nullptr == child) + return node->prefix_decision_item(); + + ++iterator; + const DecisionItem *child_decision = getDecisionItemFromTree(child, tokens, iterator); + if (child_decision->decision() == Decision_ANY) + return node->prefix_decision_item(); + + return child_decision; +} + +template <> ldp_xml_parser::DecisionItem getDecisionItem(const ldp_xml_parser::MatchItemOwn &item, + const PolicyOwn *policy) { + if (item.getName().length() == 0) + return ldp_xml_parser::Decision::DENY; + + boost::char_separator separator("."); + tokenizer tokens(item.getName(), separator); + + auto node = policy->tree(); + if (nullptr == node) + return ldp_xml_parser::Decision::ANY; + + auto iterator = tokens.begin(); + + return makeDecisionItem(getDecisionItemFromTree(node, tokens, iterator)); +} + +template ::policy_type> +ldp_xml_parser::DecisionItem getDecisionItemMaybeNull(const T &item, const P *policyPair) { + if (nullptr == policyPair) + return ldp_xml_parser::Decision::ANY; + return getDecisionItem(item, policyPair->policy()); +} + +} // anonymous namespace + class StorageBackendSerialized::StorageBackendSerializedImpl { int fd{-1}; uint8_t *mem{static_cast(MAP_FAILED)}; @@ -82,21 +161,6 @@ public: const M *getPolicySet(); }; -inline boost::string_ref s(const flatbuffers::String *str) { - return boost::string_ref(str->c_str(), str->size()); -} - -template -bool match(const T &match, const I *i) { - return match.match(makeMessageType(i->type()), - s(i->interface()), - s(i->path()), - s(i->member()), - s(i->name()), - i->is_name_prefix(), - makeDecision(i->decision()->decision())); -} - ldp_xml_parser::DecisionItem StorageBackendSerialized::StorageBackendSerializedImpl::getDecisionFromSendIndex(const MatchItemSend &item, const MapIndex &index, const SendPrefixIndex &prefixIndex) { ldp_xml_parser::DecisionItem decision(ldp_xml_parser::Decision::ANY); @@ -333,66 +397,6 @@ void StorageBackendSerialized::printContent(const bool xml_format) const { pimpl->printContent(xml_format); } -template <> bool match(const ldp_xml_parser::MatchItemAccess &match, const FB::ItemAccess *item) { - return match.match(makeBusAccessType(item->type()), item->uid(), item->gid()); -} - -template ::policy_type> -ldp_xml_parser::DecisionItem getDecisionItem(const T &item, const P *policy) { - const auto *v = policy->items(); - for (auto it = v->rbegin(); it != v->rend(); ++it) { - if (match(item, *it)) - return makeDecisionItem((*it)->decision()); - } - return ldp_xml_parser::Decision::ANY; -} - -const DecisionItem *getDecisionItemFromTree(const FB::PolicyOwnNode *node, - tokenizer &tokens, - tokenizer::iterator &iterator) { - if (iterator == tokens.end()) { - if (node->decision_item()->decision() != Decision_ANY) - return node->decision_item(); - else - return node->prefix_decision_item(); - } - - auto child = node->children()->LookupByKey(iterator->c_str()); - if (nullptr == child) - return node->prefix_decision_item(); - - ++iterator; - const DecisionItem *child_decision = getDecisionItemFromTree(child, tokens, iterator); - if (child_decision->decision() == Decision_ANY) - return node->prefix_decision_item(); - - return child_decision; -} - -template <> ldp_xml_parser::DecisionItem getDecisionItem(const ldp_xml_parser::MatchItemOwn &item, - const PolicyOwn *policy) { - if (item.getName().length() == 0) - return ldp_xml_parser::Decision::DENY; - - boost::char_separator separator("."); - tokenizer tokens(item.getName(), separator); - - auto node = policy->tree(); - if (nullptr == node) - return ldp_xml_parser::Decision::ANY; - - auto iterator = tokens.begin(); - - return makeDecisionItem(getDecisionItemFromTree(node, tokens, iterator)); -} - -template ::policy_type> -ldp_xml_parser::DecisionItem getDecisionItemMaybeNull(const T &item, const P *policyPair) { - if (nullptr == policyPair) - return ldp_xml_parser::Decision::ANY; - return getDecisionItem(item, policyPair->policy()); -} - template ldp_xml_parser::DecisionItem StorageBackendSerialized::getDecisionItemContextMandatory(const T &item) const { return getDecisionItem(item, pimpl->getPolicySet()->context_mandatory()); -- 2.7.4 From 783b7e7315959b625a613b959c19b12a381e5831 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 25 Oct 2019 16:27:09 +0200 Subject: [PATCH 02/16] serialization: serialize send index This removes creating send index after reading a serialized database. Instead, the send index is created while serializing the database, and written along with the database. Then, the index is used as before, but in a bit more generalized way, being a part of PolicySend now. Therefore, this extends the database with the new tables, making the database version up to LDP2. The previous version LDP1 is still compatible with new code, however the send index is not supported for old databases. This results in the following: - with serialized database, version LDP2, the send index is kept in serialized database file, and used accordingly for all lookups to "send" part of the database; - with serialized database, version LDP1, there is no send index, the lookups are performed in an old-fashined, linear manner; - with no serialized database, only XML files, the index is created in memory, along with the serialized database. Thus, running fast, but at the cost of memory. Change-Id: Ib918fa96eb60d216512faf60f8e20692c8a5d3a3 --- src/internal/include/fb.fbs | 11 ++- src/internal/include/fb_generated.h | 124 ++++++++++++++++++++++++- src/internal/serializer.cpp | 42 +++++++++ src/internal/storage_backend_serialized.cpp | 138 +++++++++++----------------- 4 files changed, 225 insertions(+), 90 deletions(-) diff --git a/src/internal/include/fb.fbs b/src/internal/include/fb.fbs index 8fce719..efbf6b9 100644 --- a/src/internal/include/fb.fbs +++ b/src/internal/include/fb.fbs @@ -53,6 +53,8 @@ table SendSet { table PolicySend { items:[ItemSend]; + index:[NameScoresPair]; // version "LDP2" extension + prefix_index:[uint]; // version "LDP2" extension } table ItemSend { @@ -107,6 +109,13 @@ table DecisionItem { privilege:string; } +/* version LDP2 extension starts here - sendIndex added */ +table NameScoresPair { + name: string (key); + best_score: uint; + item_refs:[uint]; +} + root_type File; -file_identifier "LDP1"; +file_identifier "LDP2"; diff --git a/src/internal/include/fb_generated.h b/src/internal/include/fb_generated.h index ce371e4..335b3d9 100644 --- a/src/internal/include/fb_generated.h +++ b/src/internal/include/fb_generated.h @@ -42,6 +42,8 @@ struct ItemAccess; struct DecisionItem; +struct NameScoresPair; + enum Decision { Decision_ANY = 0, Decision_ALLOW = 1, @@ -723,16 +725,29 @@ inline flatbuffers::Offset CreateSendSetDirect( struct PolicySend FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { enum FlatBuffersVTableOffset FLATBUFFERS_VTABLE_UNDERLYING_TYPE { - VT_ITEMS = 4 + VT_ITEMS = 4, + VT_INDEX = 6, + VT_PREFIX_INDEX = 8 }; const flatbuffers::Vector> *items() const { return GetPointer> *>(VT_ITEMS); } + const flatbuffers::Vector> *index() const { + return GetPointer> *>(VT_INDEX); + } + const flatbuffers::Vector *prefix_index() const { + return GetPointer *>(VT_PREFIX_INDEX); + } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && VerifyOffset(verifier, VT_ITEMS) && verifier.VerifyVector(items()) && verifier.VerifyVectorOfTables(items()) && + VerifyOffset(verifier, VT_INDEX) && + verifier.VerifyVector(index()) && + verifier.VerifyVectorOfTables(index()) && + VerifyOffset(verifier, VT_PREFIX_INDEX) && + verifier.VerifyVector(prefix_index()) && verifier.EndTable(); } }; @@ -743,6 +758,12 @@ struct PolicySendBuilder { void add_items(flatbuffers::Offset>> items) { fbb_.AddOffset(PolicySend::VT_ITEMS, items); } + void add_index(flatbuffers::Offset>> index) { + fbb_.AddOffset(PolicySend::VT_INDEX, index); + } + void add_prefix_index(flatbuffers::Offset> prefix_index) { + fbb_.AddOffset(PolicySend::VT_PREFIX_INDEX, prefix_index); + } explicit PolicySendBuilder(flatbuffers::FlatBufferBuilder &_fbb) : fbb_(_fbb) { start_ = fbb_.StartTable(); @@ -757,19 +778,29 @@ struct PolicySendBuilder { inline flatbuffers::Offset CreatePolicySend( flatbuffers::FlatBufferBuilder &_fbb, - flatbuffers::Offset>> items = 0) { + flatbuffers::Offset>> items = 0, + flatbuffers::Offset>> index = 0, + flatbuffers::Offset> prefix_index = 0) { PolicySendBuilder builder_(_fbb); + builder_.add_prefix_index(prefix_index); + builder_.add_index(index); builder_.add_items(items); return builder_.Finish(); } inline flatbuffers::Offset CreatePolicySendDirect( flatbuffers::FlatBufferBuilder &_fbb, - const std::vector> *items = nullptr) { + const std::vector> *items = nullptr, + const std::vector> *index = nullptr, + const std::vector *prefix_index = nullptr) { auto items__ = items ? _fbb.CreateVector>(*items) : 0; + auto index__ = index ? _fbb.CreateVector>(*index) : 0; + auto prefix_index__ = prefix_index ? _fbb.CreateVector(*prefix_index) : 0; return FB::CreatePolicySend( _fbb, - items__); + items__, + index__, + prefix_index__); } struct ItemSend FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { @@ -1409,6 +1440,89 @@ inline flatbuffers::Offset CreateDecisionItemDirect( privilege__); } +struct NameScoresPair FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { + enum FlatBuffersVTableOffset FLATBUFFERS_VTABLE_UNDERLYING_TYPE { + VT_NAME = 4, + VT_BEST_SCORE = 6, + VT_ITEM_REFS = 8 + }; + const flatbuffers::String *name() const { + return GetPointer(VT_NAME); + } + bool KeyCompareLessThan(const NameScoresPair *o) const { + return *name() < *o->name(); + } + int KeyCompareWithValue(const char *val) const { + return strcmp(name()->c_str(), val); + } + uint32_t best_score() const { + return GetField(VT_BEST_SCORE, 0); + } + const flatbuffers::Vector *item_refs() const { + return GetPointer *>(VT_ITEM_REFS); + } + bool Verify(flatbuffers::Verifier &verifier) const { + return VerifyTableStart(verifier) && + VerifyOffsetRequired(verifier, VT_NAME) && + verifier.VerifyString(name()) && + VerifyField(verifier, VT_BEST_SCORE) && + VerifyOffset(verifier, VT_ITEM_REFS) && + verifier.VerifyVector(item_refs()) && + verifier.EndTable(); + } +}; + +struct NameScoresPairBuilder { + flatbuffers::FlatBufferBuilder &fbb_; + flatbuffers::uoffset_t start_; + void add_name(flatbuffers::Offset name) { + fbb_.AddOffset(NameScoresPair::VT_NAME, name); + } + void add_best_score(uint32_t best_score) { + fbb_.AddElement(NameScoresPair::VT_BEST_SCORE, best_score, 0); + } + void add_item_refs(flatbuffers::Offset> item_refs) { + fbb_.AddOffset(NameScoresPair::VT_ITEM_REFS, item_refs); + } + explicit NameScoresPairBuilder(flatbuffers::FlatBufferBuilder &_fbb) + : fbb_(_fbb) { + start_ = fbb_.StartTable(); + } + NameScoresPairBuilder &operator=(const NameScoresPairBuilder &); + flatbuffers::Offset Finish() { + const auto end = fbb_.EndTable(start_); + auto o = flatbuffers::Offset(end); + fbb_.Required(o, NameScoresPair::VT_NAME); + return o; + } +}; + +inline flatbuffers::Offset CreateNameScoresPair( + flatbuffers::FlatBufferBuilder &_fbb, + flatbuffers::Offset name = 0, + uint32_t best_score = 0, + flatbuffers::Offset> item_refs = 0) { + NameScoresPairBuilder builder_(_fbb); + builder_.add_item_refs(item_refs); + builder_.add_best_score(best_score); + builder_.add_name(name); + return builder_.Finish(); +} + +inline flatbuffers::Offset CreateNameScoresPairDirect( + flatbuffers::FlatBufferBuilder &_fbb, + const char *name = nullptr, + uint32_t best_score = 0, + const std::vector *item_refs = nullptr) { + auto name__ = name ? _fbb.CreateString(name) : 0; + auto item_refs__ = item_refs ? _fbb.CreateVector(*item_refs) : 0; + return FB::CreateNameScoresPair( + _fbb, + name__, + best_score, + item_refs__); +} + inline const FB::File *GetFile(const void *buf) { return flatbuffers::GetRoot(buf); } @@ -1418,7 +1532,7 @@ inline const FB::File *GetSizePrefixedFile(const void *buf) { } inline const char *FileIdentifier() { - return "LDP1"; + return "LDP2"; } inline bool FileBufferHasIdentifier(const void *buf) { diff --git a/src/internal/serializer.cpp b/src/internal/serializer.cpp index eb185e8..f5cee57 100644 --- a/src/internal/serializer.cpp +++ b/src/internal/serializer.cpp @@ -222,6 +222,48 @@ auto Serializer::serialize_policy(const PolicyOwn &policy) return serialize_tree(policy.getTree()); } +template <> +auto Serializer::serialize_policy(const PolicySend &policy) + -> FbOff { + std::vector> items; + + // this maps a name to a pair (highest score + list of scores/ids for this name) + std::map>> policyIndex; + // prefix index is just a list of ids + std::vector prefixIndex; + uint32_t cnt = 1; + + for (const auto &item : policy.getItems()) { + items.push_back(serialize_item(item)); + + // create indexes + if (!item.isNamePrefix()) { + auto &elem = policyIndex[item.getName()]; // create or get an entry + elem.second.push_back(cnt); // add score/id to the list + // we insert items in increasing score/id order, so we just update the highest score on each add + elem.first = cnt; + } else { + // just collect the prefix rules + prefixIndex.push_back(cnt); + } + ++cnt; + } + + // serialize main index + std::vector> index; + + for (auto &it: policyIndex) + index.push_back(FB::CreateNameScoresPairDirect(m_builder, + it.first.data(), // name + it.second.first, // best_score + &it.second.second)); // vector of scores/ids + + return FB::CreatePolicySend(m_builder, + m_builder.CreateVector(items), + m_builder.CreateVector(index), + m_builder.CreateVector(prefixIndex)); +} + template auto Serializer::serialize_policy(const T &policy) -> FbOff::policy> { std::vector::item>> items; diff --git a/src/internal/storage_backend_serialized.cpp b/src/internal/storage_backend_serialized.cpp index cf37c6b..5271adf 100644 --- a/src/internal/storage_backend_serialized.cpp +++ b/src/internal/storage_backend_serialized.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -133,27 +134,13 @@ class StorageBackendSerialized::StorageBackendSerializedImpl { void releaseMMap(); void releaseFD(); - typedef std::pair ItemWithScore; // a pair: Item, index of Item - typedef std::vector ItemsWithScore; // items - typedef std::pair HighestScoreWithItems; // value for index map: highest index for items, items - typedef std::map MapIndex; - typedef std::vector SendPrefixIndex; - - MapIndex sendIndex; // context default - SendPrefixIndex sendPrefixIndex; // context default prefix rules - std::map userSendIndex; - std::map userSendPrefixIndex; - public: bool init(const char *filename, bool verify); bool init(const FB::File *f); bool initFromXML(const char *config_name); void release(); - void createSendIndex(); - ldp_xml_parser::DecisionItem getDecisionFromSendIndex(const MatchItemSend &item, const MapIndex &index, const SendPrefixIndex &prefixIndex); - ldp_xml_parser::DecisionItem getDecisionFromSendIndex(const MatchItemSend &item); - ldp_xml_parser::DecisionItem getDecisionFromSendIndex(const MatchItemSend &item, uid_t uid); + ldp_xml_parser::DecisionItem getDecisionFromSendIndex(const MatchItemSend &item, const FB::PolicySend *policy); void printContent(const bool xml_format = false) const; @@ -162,90 +149,72 @@ public: }; ldp_xml_parser::DecisionItem StorageBackendSerialized::StorageBackendSerializedImpl::getDecisionFromSendIndex(const MatchItemSend &item, - const MapIndex &index, const SendPrefixIndex &prefixIndex) { - ldp_xml_parser::DecisionItem decision(ldp_xml_parser::Decision::ANY); + const FB::PolicySend *policy) { + + const auto *index = policy->index(); + + if (!index || index->size() == 0) // this indicates version earlier than LDP2 + return getDecisionItem(item, policy); // make it old way for old databases + + std::pair currentBest(nullptr, 0); + + auto updateCurrentBest = [¤tBest, &item, &policy](const flatbuffers::Vector *vec) { + // look from the back, the rule is the same as for the full database + // we now only check among less elements, because the database is indexed to small lists + // item_scores are in increasing order in the index, and they serve also as ids of the policy rules + for (auto item_score_it = vec->rbegin(); item_score_it != vec->rend(); item_score_it++) { + const auto *fb_item = policy->items()->Get(*item_score_it - 1); // rules are indexed/scored from 1 + if (*item_score_it > currentBest.second && match(item, fb_item)) { + currentBest.first = fb_item; + currentBest.second = *item_score_it; + return; + } else if (*item_score_it <= currentBest.second) { + // there is no need to look further as we can't improve the score anymore + return; + } + } + }; - ItemWithScore currentBest(nullptr, 0); + auto searchAndUpdateCurrentBest = [¤tBest, &index, &updateCurrentBest](boost::string_ref name_ref) { + // we need to create C-string for flatbuffers lookups + // boost::string_ref gives us correct start, but possibly NUL-terminated in a wrong place, as it does not modify + // input string and keeps only the length + std::string name(name_ref.data(), name_ref.size()); - auto updateCurrentBest = [¤tBest, &item, &index](boost::string_ref name) { // check if there are any rules for the name - auto fit = index.find(name); - if (fit == index.end()) + const auto *fit = index->LookupByKey(name.c_str()); + if (!fit) return; // check if there's any chance to get better score - // fit->second.first is the highest score from vector fit->second.second - if (fit->second.first <= currentBest.second) + if (fit->best_score() <= currentBest.second) return; // look for better score - for (const auto &it: fit->second.second) { - if (it.second > currentBest.second && match(item, it.first)) - currentBest = it; - } + updateCurrentBest(fit->item_refs()); }; + const auto *prefixIndex = policy->prefix_index(); + assert(prefixIndex); + // iterate over names for (const auto &name: item.getNames()) { // find and check the no-prefix rules - updateCurrentBest(name); + searchAndUpdateCurrentBest(name); // check the prefix rules - for (const auto &prefixItem: prefixIndex) { - if (prefixItem.second > currentBest.second && match(item, prefixItem.first)) - currentBest = prefixItem; - } + updateCurrentBest(prefixIndex); } // check no-name rules - updateCurrentBest(""); + searchAndUpdateCurrentBest(""); + // if a matching rule was found, return the decision if (currentBest.first) - decision = makeDecisionItem(currentBest.first->decision()); - - return decision; -} + return makeDecisionItem(currentBest.first->decision()); -ldp_xml_parser::DecisionItem StorageBackendSerialized::StorageBackendSerializedImpl::getDecisionFromSendIndex(const MatchItemSend &item) { - return getDecisionFromSendIndex(item, sendIndex, sendPrefixIndex); -} - -ldp_xml_parser::DecisionItem StorageBackendSerialized::StorageBackendSerializedImpl::getDecisionFromSendIndex(const MatchItemSend &item, uid_t uid) { - return getDecisionFromSendIndex(item, userSendIndex[uid], userSendPrefixIndex[uid]); -} - -void StorageBackendSerialized::StorageBackendSerializedImpl::createSendIndex() { - // helper for adding items to indexes - auto add = [](MapIndex &index, SendPrefixIndex &prefixIndex, boost::string_ref key, ItemWithScore value) { - if (value.first->is_name_prefix()) { - prefixIndex.push_back(value); - } else { - index[key].second.push_back(value); - // we insert items in increasing score order, so we just update the highest score on each add - index[key].first = value.second; - } - }; - - // context default index - const auto *policy = file->m_send_set()->context_default(); - unsigned cnt = 1; - - const auto *v = policy->items(); - for (auto it = v->begin(); it != v->end(); ++it) - add(sendIndex, sendPrefixIndex, - boost::string_ref(it->name()->c_str(), it->name()->size()), - ItemWithScore(*it, cnt++)); - - // users index - for (const auto &uidPolicy: *file->m_send_set()->user()) { - cnt = 1; - for (const auto it: *uidPolicy->policy()->items()) { - add(userSendIndex[uidPolicy->id()], - userSendPrefixIndex[uidPolicy->id()], - boost::string_ref(it->name()->c_str(), it->name()->size()), - ItemWithScore(it, cnt++)); - } - } + // or if no matching rule was not found, return the default decision + return ldp_xml_parser::DecisionItem(ldp_xml_parser::Decision::ANY); } void StorageBackendSerialized::StorageBackendSerializedImpl::releaseMMap() { @@ -324,17 +293,13 @@ bool StorageBackendSerialized::StorageBackendSerializedImpl::init(const char *fi mmapGuard.dismiss(); file = GetFile(mem); - bool res = file != nullptr; - if (res) - createSendIndex(); - return res; + return file != nullptr; } bool StorageBackendSerialized::StorageBackendSerializedImpl::init(const FB::File *f) { assert(nullptr == file); file = f; - createSendIndex(); return true; } @@ -403,8 +368,13 @@ ldp_xml_parser::DecisionItem StorageBackendSerialized::getDecisionItemContextMan } template <> +ldp_xml_parser::DecisionItem StorageBackendSerialized::getDecisionItemContextMandatory(const MatchItemSend &item) const { + return pimpl->getDecisionFromSendIndex(item, pimpl->getPolicySet()->context_mandatory()); +} + +template <> ldp_xml_parser::DecisionItem StorageBackendSerialized::getDecisionItemContextDefault(const MatchItemSend &item) const { - return pimpl->getDecisionFromSendIndex(item); + return pimpl->getDecisionFromSendIndex(item, pimpl->getPolicySet()->context_default()); } template <> @@ -412,7 +382,7 @@ ldp_xml_parser::DecisionItem StorageBackendSerialized::getDecisionItemUser(uid_t auto *policyPair = pimpl->getPolicySet()->user()->LookupByKey(uid); if (nullptr == policyPair) return ldp_xml_parser::Decision::ANY; - return pimpl->getDecisionFromSendIndex(item, uid); + return pimpl->getDecisionFromSendIndex(item, policyPair->policy()); } template -- 2.7.4 From 87cae30fd5842acc83ababd378873b3182c11f02 Mon Sep 17 00:00:00 2001 From: "sanghyeok.oh" Date: Mon, 28 Oct 2019 16:23:12 +0900 Subject: [PATCH 03/16] printer: print FileIdentifier Change-Id: Ia993cd3739f74920e3dedceeb4a58de1e967b326 Signed-off-by: sanghyeok.oh --- src/internal/print_content.hpp | 3 +++ src/internal/storage_backend_serialized.cpp | 14 +++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/internal/print_content.hpp b/src/internal/print_content.hpp index 78f6381..2e93ea6 100644 --- a/src/internal/print_content.hpp +++ b/src/internal/print_content.hpp @@ -25,6 +25,9 @@ void use_xml_format(const bool xml); } namespace FB { +const unsigned int FB_ID_OFFSET = 4; +const unsigned int FB_ID_SIZE = 4; + std::ostream &operator<<(std::ostream &stream, const FB::File &file); } diff --git a/src/internal/storage_backend_serialized.cpp b/src/internal/storage_backend_serialized.cpp index 5271adf..fa8fe4e 100644 --- a/src/internal/storage_backend_serialized.cpp +++ b/src/internal/storage_backend_serialized.cpp @@ -280,12 +280,20 @@ bool StorageBackendSerialized::StorageBackendSerializedImpl::init(const char *fi return err("mmap"); auto mmapGuard = transaction_guard::makeGuard([&] () { releaseMMap(); }); - if (verify) { auto verifier = flatbuffers::Verifier(mem, length); if (!FB::VerifyFileBuffer(verifier) || !FB::FileBufferHasIdentifier(mem)) { - tslog::log_error("verification of serialized data: failed\n"); - return false; + char fid[FB::FB_ID_SIZE + 1] = {0, }; + strncpy(fid, (const char *)(mem + FB::FB_ID_OFFSET), FB::FB_ID_SIZE); + + if (strcmp(fid, "LDP1") == 0) { + tslog::log_error("verification of serialized data: not available\n"); + tslog::log_error("header ID : ", FB::FileIdentifier(), "\n"); + tslog::log_error("serialized data ID : ", fid, "\n"); + } else { + tslog::log_error("verification of serialized data: failed\n"); + return false; + } } } -- 2.7.4 From 825519306d6fb53ac4d67a02e6b4b3928d6b1aae Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Wed, 18 Sep 2019 11:38:09 +0200 Subject: [PATCH 04/16] Fix DetachedBuffer self move assignment Change-Id: I7d107ac767bbb9982bf773eca86f827fce8c6423 Signed-off-by: Michal Bloch (cherry picked from commit baba460bf65e66e62533d4a0c5e7dd7526a435ae) --- src/internal/include/flatbuffers/flatbuffers.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/internal/include/flatbuffers/flatbuffers.h b/src/internal/include/flatbuffers/flatbuffers.h index 8c87dff..8f5ae32 100644 --- a/src/internal/include/flatbuffers/flatbuffers.h +++ b/src/internal/include/flatbuffers/flatbuffers.h @@ -551,6 +551,9 @@ class DetachedBuffer { #if !defined(FLATBUFFERS_CPP98_STL) // clang-format on DetachedBuffer &operator=(DetachedBuffer &&other) { + if (this == &other) + return *this; + destroy(); allocator_ = other.allocator_; -- 2.7.4 From 0961b79a97836e41ccce54b07c89233d7b6445ab Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Mon, 23 Sep 2019 15:12:52 +0200 Subject: [PATCH 05/16] Fix some issues found by static analysis. Change-Id: I8af3cddac48f82d98d9bdeaf6d41bae31d3dc12a Signed-off-by: Michal Bloch (cherry picked from commit 78eec0949fe14bffe6e057e79c3cf311499be8c3) --- src/internal/transaction_guard.hpp | 2 +- src/libdbuspolicy1.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/internal/transaction_guard.hpp b/src/internal/transaction_guard.hpp index dec307f..9df2600 100644 --- a/src/internal/transaction_guard.hpp +++ b/src/internal/transaction_guard.hpp @@ -42,7 +42,7 @@ public: } Guard(Guard &&g) : fun(std::move(g.fun)), active(g.active) { - dismiss(); + g.dismiss(); } }; diff --git a/src/libdbuspolicy1.cpp b/src/libdbuspolicy1.cpp index 404dad7..1c8f2c3 100644 --- a/src/libdbuspolicy1.cpp +++ b/src/libdbuspolicy1.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -191,7 +192,7 @@ public: static inline kconn *get_shared(BusType bus_type, int fd) { - kconn *result = new kconn(Checker::get(bus_type).checker(), fd); + kconn *result = new (std::nothrow) kconn(Checker::get(bus_type).checker(), fd); if (nullptr == result) LOGE("Error: failed to allocate memory for policy configuration"); return result; -- 2.7.4 From 14744f64568d1875f1680b720f3c62d2488ec242 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Mon, 7 Oct 2019 11:08:32 +0200 Subject: [PATCH 06/16] doc: overview, purpose, design, implementation The description of the libdbuspolicy project. It includes the overview, the purpose, the description of the design and the implementation, and additionally notes for improvements made or considered. Change-Id: I6b0bc9028ed86cb688d0dac9e910c10d3c4d8bc5 (cherry picked from commit 92c61f4ae8c64d2be5adf809c1ba10a1cdb5e905) --- doc/documentation.rst | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 doc/documentation.rst diff --git a/doc/documentation.rst b/doc/documentation.rst new file mode 100644 index 0000000..00b4ade --- /dev/null +++ b/doc/documentation.rst @@ -0,0 +1,114 @@ +Libdbuspolicy documentation +--------------------------- + +This document describes purpose, assumptions, and implementation of libdbuspolicy. + +Doxygen docs +------------ + +API and many internal functions and methods are described in doxygen markups through the code. +This document does not describe API or any particular function or method. + +Purpose +------- + +Tizen on some platforms supports kdbus, an in-kernel D-Bus implementation. Kdbus systems +work without central D-Bus daemon (dbus-daemon) running. Messages are passed between +processes by the kernel driver. The driver supports some kind of access control, +but it is not sufficient for Tizen. For example, it does not support Cynara privileges. + +That's why this library has been created. It controls accesses and sits between +D-Bus libraries and the kdbus driver. D-Bus libraries, which support libdbuspolicy +are libdbus and glib. systemd's sd-bus does not support libdbuspolicy. Libdbuspolicy +uses standard XML policy files as defined for dbus-daemon. + +Security considerations +----------------------- + +If a rogue program is executed, it can bypass libdbus, glib and libdbuspolicy +and connect directly to kdbus to send or receive messages. Only file access rules apply. +However, if other clients use standard libdbus or glib libraries, the messages +will be checked against both sending and receiving policy. The only real +downside is owning policy. A rogue program can own any name, and even hijack names +from other owners. + +Implementation (a bit of history) +------------------------------------- + +Implementation of libdbuspolicy went through several phases. In the first phase +a database of rules was created. The database was in fact two databases: system and session +database. Each of them was divided into default, mandatory, user and group sub-databases. + +This basic structure was maintained at least until 2019. + +In 2018 it appeared that memory consumption of the database was too big. Heavy kilobytes +multiplied by over a hundred of processes were taking up heavy megabytes. Additionally, +initialization of the database slowed down each program which was using D-Bus. This led +to code refactoring and introducing serialization. + +Google's FlatBuffers (https://google.github.io/flatbuffers/) were selected to implement +serialization. In order to do that, the code was heavily refactored. + +Layers +------ + +There are the following layers in libdbuspolicy: + +#. API and programs: library code for connecting database searches with acquiring data from kdbus, + and program code for the utilities, i.e. printer, serializer, finder; +#. checking: library code for handling access check requests, with full information; it performs + several database queries per one check; +#. database: data keeping, searching and converting; +#. FlatBuffers: serialization and deserialization. + +There is also some helping code for printing diagnostics, etc. + +Design +------ + +The almost current overview of the design of the checking, database, and FlatBuffers layers: + +.. image:: dia/Classes-use-only-serialized.png + +Diagram description: + +#. MatchItem, MatchItemWithUserAndGroup - queries constructed by PolicyChecker; +#. PolicyChecker - entry point for checking policy; creates MatchItems to pass them + to StorageBackendSerialized; +#. StorageBackend - interface class; defines query interface for PolicyChecker, + and provides entry point for printing content; +#. StorageBackendXML - storage specific for XML-based data; supports adding items and + provides accessory functions for serialization; serves as an intermediate stage + for StorageBackendSerialized; +#. XmlParser - parser for XML data; creates items and adds them to StorageBackendXML; +#. StorageBackendSerialized - storage specific for serialized data; +#. Serializer - a translator between XML-based data and serialized data; used as an + entry point for serializator tool; used for creating serialized data from + StorageBackendXML; +#. Printer - an entry point for printer tool; +#. FlatBuffers - a library used for management of serialized data. + +The implementation differs from the design mostly in these ways: + +#. StorageBackendXML no longer is able to get decisions (getDecisionItem*), it is used + only for conversion to serialized form (StorageBackendSerialized) +#. Additional use case, Finder, was added; it's a bit like 'grep' for policies: prints + matching rules for a given query + +Improvements +------------ + +Instead of a direct linear search a simple mapping index for send rules was introduced +in StorageBackendSerialized. Basically, it creates a map for send_destination names. +Send_destination is a parameter by which the rules differ the most. Thus, "splitting" +linear search over just matching send_destination names gives significant performance +boost. As of Oct 2019 it has been implemented without the serialization. It needs +about 15KB per process. Therefore, there is a plan to add the serialization to the index. + +Additionally, there is a concept of creating a direct acycling graph containing the rules. +It would contains "levels", for keeping track of various parts of the query/key +(name, interface, method, object, message type, etc.). Each level is dedicated for +one part. A node in a level in which key is a text can be e.g. a trie. A query would +traverse the dag through levels, until reaching a leaf with ALLOW, DENY or CHECK value. +Creating and serializing such graph would make the querying process very fast and low-memory +consuming. -- 2.7.4 From 16124baf1ebf87ce4486066b5b40536174035367 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 25 Oct 2019 13:35:59 +0200 Subject: [PATCH 07/16] refactoring: move local functions to an anonymous namespace Change-Id: I9d9cf07877d59df88305e8847c3156a73e82efd8 (cherry picked from commit c641a5cb8a025f7a75f7b88015d85e1a5cc6813a) --- src/internal/storage_backend_serialized.cpp | 154 ++++++++++++++-------------- 1 file changed, 79 insertions(+), 75 deletions(-) diff --git a/src/internal/storage_backend_serialized.cpp b/src/internal/storage_backend_serialized.cpp index 4859951..cf37c6b 100644 --- a/src/internal/storage_backend_serialized.cpp +++ b/src/internal/storage_backend_serialized.cpp @@ -43,6 +43,85 @@ typedef boost::tokenizer> tokenizer; template struct type_helper; +namespace { + +inline boost::string_ref s(const flatbuffers::String *str) { + return boost::string_ref(str->c_str(), str->size()); +} + +template +bool match(const T &match, const I *i) { + return match.match(makeMessageType(i->type()), + s(i->interface()), + s(i->path()), + s(i->member()), + s(i->name()), + i->is_name_prefix(), + makeDecision(i->decision()->decision())); +} + +template <> bool match(const ldp_xml_parser::MatchItemAccess &match, const FB::ItemAccess *item) { + return match.match(makeBusAccessType(item->type()), item->uid(), item->gid()); +} + +template ::policy_type> +ldp_xml_parser::DecisionItem getDecisionItem(const T &item, const P *policy) { + const auto *v = policy->items(); + for (auto it = v->rbegin(); it != v->rend(); ++it) { + if (match(item, *it)) + return makeDecisionItem((*it)->decision()); + } + return ldp_xml_parser::Decision::ANY; +} + +const DecisionItem *getDecisionItemFromTree(const FB::PolicyOwnNode *node, + tokenizer &tokens, + tokenizer::iterator &iterator) { + if (iterator == tokens.end()) { + if (node->decision_item()->decision() != Decision_ANY) + return node->decision_item(); + else + return node->prefix_decision_item(); + } + + auto child = node->children()->LookupByKey(iterator->c_str()); + if (nullptr == child) + return node->prefix_decision_item(); + + ++iterator; + const DecisionItem *child_decision = getDecisionItemFromTree(child, tokens, iterator); + if (child_decision->decision() == Decision_ANY) + return node->prefix_decision_item(); + + return child_decision; +} + +template <> ldp_xml_parser::DecisionItem getDecisionItem(const ldp_xml_parser::MatchItemOwn &item, + const PolicyOwn *policy) { + if (item.getName().length() == 0) + return ldp_xml_parser::Decision::DENY; + + boost::char_separator separator("."); + tokenizer tokens(item.getName(), separator); + + auto node = policy->tree(); + if (nullptr == node) + return ldp_xml_parser::Decision::ANY; + + auto iterator = tokens.begin(); + + return makeDecisionItem(getDecisionItemFromTree(node, tokens, iterator)); +} + +template ::policy_type> +ldp_xml_parser::DecisionItem getDecisionItemMaybeNull(const T &item, const P *policyPair) { + if (nullptr == policyPair) + return ldp_xml_parser::Decision::ANY; + return getDecisionItem(item, policyPair->policy()); +} + +} // anonymous namespace + class StorageBackendSerialized::StorageBackendSerializedImpl { int fd{-1}; uint8_t *mem{static_cast(MAP_FAILED)}; @@ -82,21 +161,6 @@ public: const M *getPolicySet(); }; -inline boost::string_ref s(const flatbuffers::String *str) { - return boost::string_ref(str->c_str(), str->size()); -} - -template -bool match(const T &match, const I *i) { - return match.match(makeMessageType(i->type()), - s(i->interface()), - s(i->path()), - s(i->member()), - s(i->name()), - i->is_name_prefix(), - makeDecision(i->decision()->decision())); -} - ldp_xml_parser::DecisionItem StorageBackendSerialized::StorageBackendSerializedImpl::getDecisionFromSendIndex(const MatchItemSend &item, const MapIndex &index, const SendPrefixIndex &prefixIndex) { ldp_xml_parser::DecisionItem decision(ldp_xml_parser::Decision::ANY); @@ -333,66 +397,6 @@ void StorageBackendSerialized::printContent(const bool xml_format) const { pimpl->printContent(xml_format); } -template <> bool match(const ldp_xml_parser::MatchItemAccess &match, const FB::ItemAccess *item) { - return match.match(makeBusAccessType(item->type()), item->uid(), item->gid()); -} - -template ::policy_type> -ldp_xml_parser::DecisionItem getDecisionItem(const T &item, const P *policy) { - const auto *v = policy->items(); - for (auto it = v->rbegin(); it != v->rend(); ++it) { - if (match(item, *it)) - return makeDecisionItem((*it)->decision()); - } - return ldp_xml_parser::Decision::ANY; -} - -const DecisionItem *getDecisionItemFromTree(const FB::PolicyOwnNode *node, - tokenizer &tokens, - tokenizer::iterator &iterator) { - if (iterator == tokens.end()) { - if (node->decision_item()->decision() != Decision_ANY) - return node->decision_item(); - else - return node->prefix_decision_item(); - } - - auto child = node->children()->LookupByKey(iterator->c_str()); - if (nullptr == child) - return node->prefix_decision_item(); - - ++iterator; - const DecisionItem *child_decision = getDecisionItemFromTree(child, tokens, iterator); - if (child_decision->decision() == Decision_ANY) - return node->prefix_decision_item(); - - return child_decision; -} - -template <> ldp_xml_parser::DecisionItem getDecisionItem(const ldp_xml_parser::MatchItemOwn &item, - const PolicyOwn *policy) { - if (item.getName().length() == 0) - return ldp_xml_parser::Decision::DENY; - - boost::char_separator separator("."); - tokenizer tokens(item.getName(), separator); - - auto node = policy->tree(); - if (nullptr == node) - return ldp_xml_parser::Decision::ANY; - - auto iterator = tokens.begin(); - - return makeDecisionItem(getDecisionItemFromTree(node, tokens, iterator)); -} - -template ::policy_type> -ldp_xml_parser::DecisionItem getDecisionItemMaybeNull(const T &item, const P *policyPair) { - if (nullptr == policyPair) - return ldp_xml_parser::Decision::ANY; - return getDecisionItem(item, policyPair->policy()); -} - template ldp_xml_parser::DecisionItem StorageBackendSerialized::getDecisionItemContextMandatory(const T &item) const { return getDecisionItem(item, pimpl->getPolicySet()->context_mandatory()); -- 2.7.4 From e83f85abbe208d0c96c137ebc852ea4e459449ae Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 25 Oct 2019 16:27:09 +0200 Subject: [PATCH 08/16] serialization: serialize send index This removes creating send index after reading a serialized database. Instead, the send index is created while serializing the database, and written along with the database. Then, the index is used as before, but in a bit more generalized way, being a part of PolicySend now. Therefore, this extends the database with the new tables, making the database version up to LDP2. The previous version LDP1 is still compatible with new code, however the send index is not supported for old databases. This results in the following: - with serialized database, version LDP2, the send index is kept in serialized database file, and used accordingly for all lookups to "send" part of the database; - with serialized database, version LDP1, there is no send index, the lookups are performed in an old-fashined, linear manner; - with no serialized database, only XML files, the index is created in memory, along with the serialized database. Thus, running fast, but at the cost of memory. Change-Id: Ib918fa96eb60d216512faf60f8e20692c8a5d3a3 (cherry picked from commit 783b7e7315959b625a613b959c19b12a381e5831) --- src/internal/include/fb.fbs | 11 ++- src/internal/include/fb_generated.h | 124 ++++++++++++++++++++++++- src/internal/serializer.cpp | 42 +++++++++ src/internal/storage_backend_serialized.cpp | 138 +++++++++++----------------- 4 files changed, 225 insertions(+), 90 deletions(-) diff --git a/src/internal/include/fb.fbs b/src/internal/include/fb.fbs index 8fce719..efbf6b9 100644 --- a/src/internal/include/fb.fbs +++ b/src/internal/include/fb.fbs @@ -53,6 +53,8 @@ table SendSet { table PolicySend { items:[ItemSend]; + index:[NameScoresPair]; // version "LDP2" extension + prefix_index:[uint]; // version "LDP2" extension } table ItemSend { @@ -107,6 +109,13 @@ table DecisionItem { privilege:string; } +/* version LDP2 extension starts here - sendIndex added */ +table NameScoresPair { + name: string (key); + best_score: uint; + item_refs:[uint]; +} + root_type File; -file_identifier "LDP1"; +file_identifier "LDP2"; diff --git a/src/internal/include/fb_generated.h b/src/internal/include/fb_generated.h index ce371e4..335b3d9 100644 --- a/src/internal/include/fb_generated.h +++ b/src/internal/include/fb_generated.h @@ -42,6 +42,8 @@ struct ItemAccess; struct DecisionItem; +struct NameScoresPair; + enum Decision { Decision_ANY = 0, Decision_ALLOW = 1, @@ -723,16 +725,29 @@ inline flatbuffers::Offset CreateSendSetDirect( struct PolicySend FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { enum FlatBuffersVTableOffset FLATBUFFERS_VTABLE_UNDERLYING_TYPE { - VT_ITEMS = 4 + VT_ITEMS = 4, + VT_INDEX = 6, + VT_PREFIX_INDEX = 8 }; const flatbuffers::Vector> *items() const { return GetPointer> *>(VT_ITEMS); } + const flatbuffers::Vector> *index() const { + return GetPointer> *>(VT_INDEX); + } + const flatbuffers::Vector *prefix_index() const { + return GetPointer *>(VT_PREFIX_INDEX); + } bool Verify(flatbuffers::Verifier &verifier) const { return VerifyTableStart(verifier) && VerifyOffset(verifier, VT_ITEMS) && verifier.VerifyVector(items()) && verifier.VerifyVectorOfTables(items()) && + VerifyOffset(verifier, VT_INDEX) && + verifier.VerifyVector(index()) && + verifier.VerifyVectorOfTables(index()) && + VerifyOffset(verifier, VT_PREFIX_INDEX) && + verifier.VerifyVector(prefix_index()) && verifier.EndTable(); } }; @@ -743,6 +758,12 @@ struct PolicySendBuilder { void add_items(flatbuffers::Offset>> items) { fbb_.AddOffset(PolicySend::VT_ITEMS, items); } + void add_index(flatbuffers::Offset>> index) { + fbb_.AddOffset(PolicySend::VT_INDEX, index); + } + void add_prefix_index(flatbuffers::Offset> prefix_index) { + fbb_.AddOffset(PolicySend::VT_PREFIX_INDEX, prefix_index); + } explicit PolicySendBuilder(flatbuffers::FlatBufferBuilder &_fbb) : fbb_(_fbb) { start_ = fbb_.StartTable(); @@ -757,19 +778,29 @@ struct PolicySendBuilder { inline flatbuffers::Offset CreatePolicySend( flatbuffers::FlatBufferBuilder &_fbb, - flatbuffers::Offset>> items = 0) { + flatbuffers::Offset>> items = 0, + flatbuffers::Offset>> index = 0, + flatbuffers::Offset> prefix_index = 0) { PolicySendBuilder builder_(_fbb); + builder_.add_prefix_index(prefix_index); + builder_.add_index(index); builder_.add_items(items); return builder_.Finish(); } inline flatbuffers::Offset CreatePolicySendDirect( flatbuffers::FlatBufferBuilder &_fbb, - const std::vector> *items = nullptr) { + const std::vector> *items = nullptr, + const std::vector> *index = nullptr, + const std::vector *prefix_index = nullptr) { auto items__ = items ? _fbb.CreateVector>(*items) : 0; + auto index__ = index ? _fbb.CreateVector>(*index) : 0; + auto prefix_index__ = prefix_index ? _fbb.CreateVector(*prefix_index) : 0; return FB::CreatePolicySend( _fbb, - items__); + items__, + index__, + prefix_index__); } struct ItemSend FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { @@ -1409,6 +1440,89 @@ inline flatbuffers::Offset CreateDecisionItemDirect( privilege__); } +struct NameScoresPair FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table { + enum FlatBuffersVTableOffset FLATBUFFERS_VTABLE_UNDERLYING_TYPE { + VT_NAME = 4, + VT_BEST_SCORE = 6, + VT_ITEM_REFS = 8 + }; + const flatbuffers::String *name() const { + return GetPointer(VT_NAME); + } + bool KeyCompareLessThan(const NameScoresPair *o) const { + return *name() < *o->name(); + } + int KeyCompareWithValue(const char *val) const { + return strcmp(name()->c_str(), val); + } + uint32_t best_score() const { + return GetField(VT_BEST_SCORE, 0); + } + const flatbuffers::Vector *item_refs() const { + return GetPointer *>(VT_ITEM_REFS); + } + bool Verify(flatbuffers::Verifier &verifier) const { + return VerifyTableStart(verifier) && + VerifyOffsetRequired(verifier, VT_NAME) && + verifier.VerifyString(name()) && + VerifyField(verifier, VT_BEST_SCORE) && + VerifyOffset(verifier, VT_ITEM_REFS) && + verifier.VerifyVector(item_refs()) && + verifier.EndTable(); + } +}; + +struct NameScoresPairBuilder { + flatbuffers::FlatBufferBuilder &fbb_; + flatbuffers::uoffset_t start_; + void add_name(flatbuffers::Offset name) { + fbb_.AddOffset(NameScoresPair::VT_NAME, name); + } + void add_best_score(uint32_t best_score) { + fbb_.AddElement(NameScoresPair::VT_BEST_SCORE, best_score, 0); + } + void add_item_refs(flatbuffers::Offset> item_refs) { + fbb_.AddOffset(NameScoresPair::VT_ITEM_REFS, item_refs); + } + explicit NameScoresPairBuilder(flatbuffers::FlatBufferBuilder &_fbb) + : fbb_(_fbb) { + start_ = fbb_.StartTable(); + } + NameScoresPairBuilder &operator=(const NameScoresPairBuilder &); + flatbuffers::Offset Finish() { + const auto end = fbb_.EndTable(start_); + auto o = flatbuffers::Offset(end); + fbb_.Required(o, NameScoresPair::VT_NAME); + return o; + } +}; + +inline flatbuffers::Offset CreateNameScoresPair( + flatbuffers::FlatBufferBuilder &_fbb, + flatbuffers::Offset name = 0, + uint32_t best_score = 0, + flatbuffers::Offset> item_refs = 0) { + NameScoresPairBuilder builder_(_fbb); + builder_.add_item_refs(item_refs); + builder_.add_best_score(best_score); + builder_.add_name(name); + return builder_.Finish(); +} + +inline flatbuffers::Offset CreateNameScoresPairDirect( + flatbuffers::FlatBufferBuilder &_fbb, + const char *name = nullptr, + uint32_t best_score = 0, + const std::vector *item_refs = nullptr) { + auto name__ = name ? _fbb.CreateString(name) : 0; + auto item_refs__ = item_refs ? _fbb.CreateVector(*item_refs) : 0; + return FB::CreateNameScoresPair( + _fbb, + name__, + best_score, + item_refs__); +} + inline const FB::File *GetFile(const void *buf) { return flatbuffers::GetRoot(buf); } @@ -1418,7 +1532,7 @@ inline const FB::File *GetSizePrefixedFile(const void *buf) { } inline const char *FileIdentifier() { - return "LDP1"; + return "LDP2"; } inline bool FileBufferHasIdentifier(const void *buf) { diff --git a/src/internal/serializer.cpp b/src/internal/serializer.cpp index eb185e8..f5cee57 100644 --- a/src/internal/serializer.cpp +++ b/src/internal/serializer.cpp @@ -222,6 +222,48 @@ auto Serializer::serialize_policy(const PolicyOwn &policy) return serialize_tree(policy.getTree()); } +template <> +auto Serializer::serialize_policy(const PolicySend &policy) + -> FbOff { + std::vector> items; + + // this maps a name to a pair (highest score + list of scores/ids for this name) + std::map>> policyIndex; + // prefix index is just a list of ids + std::vector prefixIndex; + uint32_t cnt = 1; + + for (const auto &item : policy.getItems()) { + items.push_back(serialize_item(item)); + + // create indexes + if (!item.isNamePrefix()) { + auto &elem = policyIndex[item.getName()]; // create or get an entry + elem.second.push_back(cnt); // add score/id to the list + // we insert items in increasing score/id order, so we just update the highest score on each add + elem.first = cnt; + } else { + // just collect the prefix rules + prefixIndex.push_back(cnt); + } + ++cnt; + } + + // serialize main index + std::vector> index; + + for (auto &it: policyIndex) + index.push_back(FB::CreateNameScoresPairDirect(m_builder, + it.first.data(), // name + it.second.first, // best_score + &it.second.second)); // vector of scores/ids + + return FB::CreatePolicySend(m_builder, + m_builder.CreateVector(items), + m_builder.CreateVector(index), + m_builder.CreateVector(prefixIndex)); +} + template auto Serializer::serialize_policy(const T &policy) -> FbOff::policy> { std::vector::item>> items; diff --git a/src/internal/storage_backend_serialized.cpp b/src/internal/storage_backend_serialized.cpp index 4859951..8900779 100644 --- a/src/internal/storage_backend_serialized.cpp +++ b/src/internal/storage_backend_serialized.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -54,27 +55,13 @@ class StorageBackendSerialized::StorageBackendSerializedImpl { void releaseMMap(); void releaseFD(); - typedef std::pair ItemWithScore; // a pair: Item, index of Item - typedef std::vector ItemsWithScore; // items - typedef std::pair HighestScoreWithItems; // value for index map: highest index for items, items - typedef std::map MapIndex; - typedef std::vector SendPrefixIndex; - - MapIndex sendIndex; // context default - SendPrefixIndex sendPrefixIndex; // context default prefix rules - std::map userSendIndex; - std::map userSendPrefixIndex; - public: bool init(const char *filename, bool verify); bool init(const FB::File *f); bool initFromXML(const char *config_name); void release(); - void createSendIndex(); - ldp_xml_parser::DecisionItem getDecisionFromSendIndex(const MatchItemSend &item, const MapIndex &index, const SendPrefixIndex &prefixIndex); - ldp_xml_parser::DecisionItem getDecisionFromSendIndex(const MatchItemSend &item); - ldp_xml_parser::DecisionItem getDecisionFromSendIndex(const MatchItemSend &item, uid_t uid); + ldp_xml_parser::DecisionItem getDecisionFromSendIndex(const MatchItemSend &item, const FB::PolicySend *policy); void printContent(const bool xml_format = false) const; @@ -98,90 +85,72 @@ bool match(const T &match, const I *i) { } ldp_xml_parser::DecisionItem StorageBackendSerialized::StorageBackendSerializedImpl::getDecisionFromSendIndex(const MatchItemSend &item, - const MapIndex &index, const SendPrefixIndex &prefixIndex) { - ldp_xml_parser::DecisionItem decision(ldp_xml_parser::Decision::ANY); + const FB::PolicySend *policy) { + + const auto *index = policy->index(); + + if (!index || index->size() == 0) // this indicates version earlier than LDP2 + return getDecisionItem(item, policy); // make it old way for old databases + + std::pair currentBest(nullptr, 0); + + auto updateCurrentBest = [¤tBest, &item, &policy](const flatbuffers::Vector *vec) { + // look from the back, the rule is the same as for the full database + // we now only check among less elements, because the database is indexed to small lists + // item_scores are in increasing order in the index, and they serve also as ids of the policy rules + for (auto item_score_it = vec->rbegin(); item_score_it != vec->rend(); item_score_it++) { + const auto *fb_item = policy->items()->Get(*item_score_it - 1); // rules are indexed/scored from 1 + if (*item_score_it > currentBest.second && match(item, fb_item)) { + currentBest.first = fb_item; + currentBest.second = *item_score_it; + return; + } else if (*item_score_it <= currentBest.second) { + // there is no need to look further as we can't improve the score anymore + return; + } + } + }; - ItemWithScore currentBest(nullptr, 0); + auto searchAndUpdateCurrentBest = [¤tBest, &index, &updateCurrentBest](boost::string_ref name_ref) { + // we need to create C-string for flatbuffers lookups + // boost::string_ref gives us correct start, but possibly NUL-terminated in a wrong place, as it does not modify + // input string and keeps only the length + std::string name(name_ref.data(), name_ref.size()); - auto updateCurrentBest = [¤tBest, &item, &index](boost::string_ref name) { // check if there are any rules for the name - auto fit = index.find(name); - if (fit == index.end()) + const auto *fit = index->LookupByKey(name.c_str()); + if (!fit) return; // check if there's any chance to get better score - // fit->second.first is the highest score from vector fit->second.second - if (fit->second.first <= currentBest.second) + if (fit->best_score() <= currentBest.second) return; // look for better score - for (const auto &it: fit->second.second) { - if (it.second > currentBest.second && match(item, it.first)) - currentBest = it; - } + updateCurrentBest(fit->item_refs()); }; + const auto *prefixIndex = policy->prefix_index(); + assert(prefixIndex); + // iterate over names for (const auto &name: item.getNames()) { // find and check the no-prefix rules - updateCurrentBest(name); + searchAndUpdateCurrentBest(name); // check the prefix rules - for (const auto &prefixItem: prefixIndex) { - if (prefixItem.second > currentBest.second && match(item, prefixItem.first)) - currentBest = prefixItem; - } + updateCurrentBest(prefixIndex); } // check no-name rules - updateCurrentBest(""); + searchAndUpdateCurrentBest(""); + // if a matching rule was found, return the decision if (currentBest.first) - decision = makeDecisionItem(currentBest.first->decision()); - - return decision; -} + return makeDecisionItem(currentBest.first->decision()); -ldp_xml_parser::DecisionItem StorageBackendSerialized::StorageBackendSerializedImpl::getDecisionFromSendIndex(const MatchItemSend &item) { - return getDecisionFromSendIndex(item, sendIndex, sendPrefixIndex); -} - -ldp_xml_parser::DecisionItem StorageBackendSerialized::StorageBackendSerializedImpl::getDecisionFromSendIndex(const MatchItemSend &item, uid_t uid) { - return getDecisionFromSendIndex(item, userSendIndex[uid], userSendPrefixIndex[uid]); -} - -void StorageBackendSerialized::StorageBackendSerializedImpl::createSendIndex() { - // helper for adding items to indexes - auto add = [](MapIndex &index, SendPrefixIndex &prefixIndex, boost::string_ref key, ItemWithScore value) { - if (value.first->is_name_prefix()) { - prefixIndex.push_back(value); - } else { - index[key].second.push_back(value); - // we insert items in increasing score order, so we just update the highest score on each add - index[key].first = value.second; - } - }; - - // context default index - const auto *policy = file->m_send_set()->context_default(); - unsigned cnt = 1; - - const auto *v = policy->items(); - for (auto it = v->begin(); it != v->end(); ++it) - add(sendIndex, sendPrefixIndex, - boost::string_ref(it->name()->c_str(), it->name()->size()), - ItemWithScore(*it, cnt++)); - - // users index - for (const auto &uidPolicy: *file->m_send_set()->user()) { - cnt = 1; - for (const auto it: *uidPolicy->policy()->items()) { - add(userSendIndex[uidPolicy->id()], - userSendPrefixIndex[uidPolicy->id()], - boost::string_ref(it->name()->c_str(), it->name()->size()), - ItemWithScore(it, cnt++)); - } - } + // or if no matching rule was not found, return the default decision + return ldp_xml_parser::DecisionItem(ldp_xml_parser::Decision::ANY); } void StorageBackendSerialized::StorageBackendSerializedImpl::releaseMMap() { @@ -260,17 +229,13 @@ bool StorageBackendSerialized::StorageBackendSerializedImpl::init(const char *fi mmapGuard.dismiss(); file = GetFile(mem); - bool res = file != nullptr; - if (res) - createSendIndex(); - return res; + return file != nullptr; } bool StorageBackendSerialized::StorageBackendSerializedImpl::init(const FB::File *f) { assert(nullptr == file); file = f; - createSendIndex(); return true; } @@ -399,8 +364,13 @@ ldp_xml_parser::DecisionItem StorageBackendSerialized::getDecisionItemContextMan } template <> +ldp_xml_parser::DecisionItem StorageBackendSerialized::getDecisionItemContextMandatory(const MatchItemSend &item) const { + return pimpl->getDecisionFromSendIndex(item, pimpl->getPolicySet()->context_mandatory()); +} + +template <> ldp_xml_parser::DecisionItem StorageBackendSerialized::getDecisionItemContextDefault(const MatchItemSend &item) const { - return pimpl->getDecisionFromSendIndex(item); + return pimpl->getDecisionFromSendIndex(item, pimpl->getPolicySet()->context_default()); } template <> @@ -408,7 +378,7 @@ ldp_xml_parser::DecisionItem StorageBackendSerialized::getDecisionItemUser(uid_t auto *policyPair = pimpl->getPolicySet()->user()->LookupByKey(uid); if (nullptr == policyPair) return ldp_xml_parser::Decision::ANY; - return pimpl->getDecisionFromSendIndex(item, uid); + return pimpl->getDecisionFromSendIndex(item, policyPair->policy()); } template -- 2.7.4 From cf8ea4cbc75003f797b5f6ebf7fe0c841100c7aa Mon Sep 17 00:00:00 2001 From: "sanghyeok.oh" Date: Mon, 28 Oct 2019 16:23:12 +0900 Subject: [PATCH 09/16] printer: print FileIdentifier Change-Id: Ia993cd3739f74920e3dedceeb4a58de1e967b326 Signed-off-by: sanghyeok.oh (cherry picked from commit 87cae30fd5842acc83ababd378873b3182c11f02) --- src/internal/print_content.hpp | 3 +++ src/internal/storage_backend_serialized.cpp | 14 +++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/internal/print_content.hpp b/src/internal/print_content.hpp index 78f6381..2e93ea6 100644 --- a/src/internal/print_content.hpp +++ b/src/internal/print_content.hpp @@ -25,6 +25,9 @@ void use_xml_format(const bool xml); } namespace FB { +const unsigned int FB_ID_OFFSET = 4; +const unsigned int FB_ID_SIZE = 4; + std::ostream &operator<<(std::ostream &stream, const FB::File &file); } diff --git a/src/internal/storage_backend_serialized.cpp b/src/internal/storage_backend_serialized.cpp index 4859951..929f4d5 100644 --- a/src/internal/storage_backend_serialized.cpp +++ b/src/internal/storage_backend_serialized.cpp @@ -247,12 +247,20 @@ bool StorageBackendSerialized::StorageBackendSerializedImpl::init(const char *fi return err("mmap"); auto mmapGuard = transaction_guard::makeGuard([&] () { releaseMMap(); }); - if (verify) { auto verifier = flatbuffers::Verifier(mem, length); if (!FB::VerifyFileBuffer(verifier) || !FB::FileBufferHasIdentifier(mem)) { - tslog::log_error("verification of serialized data: failed\n"); - return false; + char fid[FB::FB_ID_SIZE + 1] = {0, }; + strncpy(fid, (const char *)(mem + FB::FB_ID_OFFSET), FB::FB_ID_SIZE); + + if (strcmp(fid, "LDP1") == 0) { + tslog::log_error("verification of serialized data: not available\n"); + tslog::log_error("header ID : ", FB::FileIdentifier(), "\n"); + tslog::log_error("serialized data ID : ", fid, "\n"); + } else { + tslog::log_error("verification of serialized data: failed\n"); + return false; + } } } -- 2.7.4 From 54ae6b8b73b8c4228aedaa420e910d042ca67b55 Mon Sep 17 00:00:00 2001 From: Kushagra K Date: Mon, 4 Nov 2019 14:03:48 +0530 Subject: [PATCH 10/16] DF191023-00350:[Secure option] file secure option application request (PIE, etc) Change-Id: I65bbebeb5b6f8399dce4bc622c7a59b5963540ce Reviewed-by: Himanshu Maithani Signed-off-by: Hyotaek Shim --- Makefile.am | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index bbf8203..9f2c98d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -31,6 +31,9 @@ AM_LDFLAGS = \ -pthread \ -lexpat +BIN_CPPFLAGS = -fPIE +BIN_LDFLAGS = -pie + SED_PROCESS = \ $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(SED) \ -e 's,@VERSION\@,$(VERSION),g' \ @@ -82,7 +85,8 @@ EXTRA_src_libdbuspolicy1_la_DEPENDENCIES = ${top_srcdir}/src/libdbuspolicy1.sym dbuspolicy_serializer_SOURCES =\ src/dbuspolicy_serializer.cpp -dbuspolicy_serializer_CFLAGS="-Isrc/internal/include $(AM_CFLAGS)" +dbuspolicy_serializer_CPPFLAGS = $(AM_CPPFLAGS) $(BIN_CPPFLAGS) +dbuspolicy_serializer_LDFLAGS = $(BIN_LDFLAGS) # dbuspolicy_serializer_LDFLAGS = $(AM_LDFLAGS) \ # -version-info $(LIBDBUSPOLICY1_CURRENT):$(LIBDBUSPOLICY1_REVISION):$(LIBDBUSPOLICY1_AGE) \ @@ -103,7 +107,8 @@ dbuspolicy_serializerdir = /bin/ dbuspolicy_printer_SOURCES =\ src/dbuspolicy_printer.cpp -dbuspolicy_printer_CFLAGS="-Isrc/internal/include $(AM_CFLAGS)" +dbuspolicy_printer_CPPFLAGS = $(AM_CPPFLAGS) $(BIN_CPPFLAGS) +dbuspolicy_printer_LDFLAGS = $(BIN_LDFLAGS) dbuspolicy_printer_LDADD = src/libinternal.la \ $(CYNARA_LIBS) \ @@ -115,7 +120,8 @@ EXTRA_dbuspolicy_printer_DEPENDENCIES = ${top_srcdir}/src/libdbuspolicy1.sym dbuspolicy_finder_SOURCES =\ src/dbuspolicy_finder.cpp -dbuspolicy_finder_CFLAGS="-Isrc/internal/include $(AM_CFLAGS)" +dbuspolicy_finder_CPPFLAGS = $(AM_CPPFLAGS) $(BIN_CPPFLAGS) +dbuspolicy_finder_LDFLAGS = $(BIN_LDFLAGS) dbuspolicy_finder_LDADD = src/libinternal.la \ $(CYNARA_LIBS) \ -- 2.7.4 From c9c0b00d9b6a45484c66bdb3f40352351814ee9a Mon Sep 17 00:00:00 2001 From: "sanghyeok.oh" Date: Tue, 31 Dec 2019 10:52:06 +0900 Subject: [PATCH 11/16] Remove warning for GCC-9 Change-Id: Iefda0c2fafd09efe06ab3d8d73f593e3de406e7a Signed-off-by: sanghyeok.oh --- src/dbus_daemon.c | 4 ++-- src/libdbuspolicy1.cpp | 10 +++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/dbus_daemon.c b/src/dbus_daemon.c index 23234de..6959284 100644 --- a/src/dbus_daemon.c +++ b/src/dbus_daemon.c @@ -138,7 +138,7 @@ static int bus_acquire_name(int fd, const char *name) item = cmd->items; item->type = KDBUS_ITEM_NAME; item->size = KDBUS_ITEM_HEADER_SIZE + strlen(name) + 1; - strncpy(item->str, name, strlen(name) + 1); + memcpy(item->str, name, strlen(name) + 1); /* * Employ the command on the connection owner file descriptor. @@ -223,7 +223,7 @@ int main(int argc, char* argv[]) if (fd < 0) return -1; - while (server_names[j] != '\0') { + while (server_names[j]) { if (bus_acquire_name(fd, server_names[j])) return -1; j++; diff --git a/src/libdbuspolicy1.cpp b/src/libdbuspolicy1.cpp index 1c8f2c3..2e53479 100644 --- a/src/libdbuspolicy1.cpp +++ b/src/libdbuspolicy1.cpp @@ -69,15 +69,19 @@ struct udesc { { uid = uid_; gid = gid_; - if (label_) - strncpy(label, label_, strlen(label_) + 1); + if (label_) { + if (sizeof(label) > strlen(label_)) + memcpy(label, label_, strlen(label_) + 1); + else + LOGE("Error: failed to strcpy, length of label(%s) exceed 255", label_); + } } private: std::once_flag init_once_done; void init_once() { - char buf[1024]; + char buf[256]; int attr_fd; int r; -- 2.7.4 From b8099d58e61720f2c0612aef43d74c1d9f101a7d Mon Sep 17 00:00:00 2001 From: "sanghyeok.oh" Date: Thu, 27 Feb 2020 15:33:22 +0900 Subject: [PATCH 12/16] coverity: Fix coverity issue constexpr function should return LiteralType Coverity: #1050604 Parse warning constexpr_return_not_constant Change-Id: I608c84748dc515778bf96c6b65e59f1e389359c3 --- src/libdbuspolicy1.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libdbuspolicy1.cpp b/src/libdbuspolicy1.cpp index 2e53479..027f385 100644 --- a/src/libdbuspolicy1.cpp +++ b/src/libdbuspolicy1.cpp @@ -316,7 +316,7 @@ DBUSPOLICY1_EXPORT void* dbuspolicy1_init(const char *bus_path) // helper functions namespace { -constexpr kconn *KCONN(void *config) { return static_cast(config); } +kconn *KCONN(void *config) { return static_cast(config); } /* * This function prepares contents of info.names() for initialization of MatchItems. -- 2.7.4 From dd569bc52ec74de77b78b6d744e405f3e4c6ad7c Mon Sep 17 00:00:00 2001 From: Gaurav Gupta Date: Mon, 17 Feb 2020 11:08:45 +0530 Subject: [PATCH 13/16] Fix memleak issue reported by Dexter tool. Change-Id: I24e2d2d6b6ecb3e3daf0edd94fe67866f369e87a --- src/internal/xml_parser.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/internal/xml_parser.cpp b/src/internal/xml_parser.cpp index 62d8372..dba29ff 100644 --- a/src/internal/xml_parser.cpp +++ b/src/internal/xml_parser.cpp @@ -223,16 +223,20 @@ std::unique_ptr file2str(const char *filename) { } if (fseek(fp, 0, SEEK_END)) { + fclose(fp); throw std::runtime_error(std::string("Failed to fseek end of file").c_str()); } long fsize = ftell(fp); if (fsize < 0) { + fclose(fp); throw std::runtime_error(std::string("Failed to ftell").c_str()); } if (fsize > (MAX_CONF_SIZE)) { + fclose(fp); throw std::runtime_error(std::string("File size over ").append(std::to_string(MAX_CONF_SIZE)).append("Bytes").c_str()); } if (fseek(fp, 0, SEEK_SET)) { + fclose(fp); throw std::runtime_error(std::string("Failed to fseek beginning of file").c_str()); } -- 2.7.4 From 0a3ba72cc2532650afe3485b0a2b77739faefcec Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Thu, 9 Apr 2020 09:05:50 +0200 Subject: [PATCH 14/16] tests: test multiple includedirs directive This adds a test which checks if the files in all the directories specified in all the directives are parsed. It breaks the checks during building as it exposes a bug. The subsequent commit fixes the bug. Change-Id: I020246138586357717dbee73617182f79176eac9 --- Makefile.am | 5 +++- src/test-libdbuspolicy1-multiple-includedirs.cpp | 35 ++++++++++++++++++++++ .../another-system.d/some-service.conf | 12 ++++++++ .../default_deny/system-multiple-includedirs.conf | 20 +++++++++++++ tests/default_deny/system.d/some-service.conf | 15 ++++++++++ .../yet-another-system.d/some-service.conf | 12 ++++++++ 6 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 src/test-libdbuspolicy1-multiple-includedirs.cpp create mode 100644 tests/default_deny/another-system.d/some-service.conf create mode 100644 tests/default_deny/system-multiple-includedirs.conf create mode 100644 tests/default_deny/system.d/some-service.conf create mode 100644 tests/default_deny/yet-another-system.d/some-service.conf diff --git a/Makefile.am b/Makefile.am index 9f2c98d..aad84e6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -150,7 +150,8 @@ TESTS = src/test-libdbuspolicy1-ownership \ src/test-libdbuspolicy1-access-deny-gdi \ src/test-libdbuspolicy1-send_destination_prefix-deny \ src/test-libdbuspolicy1-send_destination_prefix-deny-gdi \ - src/test-serializer + src/test-serializer \ + src/test-libdbuspolicy1-multiple-includedirs check_PROGRAMS = $(TESTS) @@ -167,6 +168,7 @@ src_test_libdbuspolicy1_access_deny_gdi_SOURCES = src/test-libdbuspolicy1-access src_test_libdbuspolicy1_send_destination_prefix_deny_SOURCES = src/test-libdbuspolicy1-send_destination_prefix-deny.cpp src_test_libdbuspolicy1_send_destination_prefix_deny_gdi_SOURCES = src/test-libdbuspolicy1-send_destination_prefix-deny-gdi.cpp src_test_serializer_SOURCES = src/test-serializer.cpp +src_test_libdbuspolicy1_multiple_includedirs_SOURCES = src/test-libdbuspolicy1-multiple-includedirs.cpp noinst_LTLIBRARIES = src/libinternal.la src_libinternal_la_SOURCES =\ @@ -193,6 +195,7 @@ src_test_libdbuspolicy1_access_deny_gdi_LDADD = $(TESTS_LDADD) src_test_libdbuspolicy1_send_destination_prefix_deny_LDADD = $(TESTS_LDADD) src_test_libdbuspolicy1_send_destination_prefix_deny_gdi_LDADD = $(TESTS_LDADD) src_test_serializer_LDADD = $(TESTS_LDADD) +src_test_libdbuspolicy1_multiple_includedirs_LDADD = $(TESTS_LDADD) if ENABLE_STANDALONE_TESTS noinst_LTLIBRARIES += src/libinternalfortests.la diff --git a/src/test-libdbuspolicy1-multiple-includedirs.cpp b/src/test-libdbuspolicy1-multiple-includedirs.cpp new file mode 100644 index 0000000..abdb0e8 --- /dev/null +++ b/src/test-libdbuspolicy1-multiple-includedirs.cpp @@ -0,0 +1,35 @@ +#include "internal/naive_policy_checker.hpp" +#include "internal/tslog.hpp" + +#include +#include + +ldp_xml_parser::Decision test_destination(const ldp_serialized::StorageBackendSerialized &db, const char *destination) { + KdbusBusNames names; + ldp_xml_parser::MatchItemSend item("ex.ample.interface", "ExampleMember", "/Ex/Ample/Path", + ldp_xml_parser::MessageType::METHOD_CALL, names.addSpaceSeparatedNames(destination)); + + return db.getDecisionItemUser(0, item).getDecision(); +} + +#define tassert(COND) do { if (!(COND)) throw std::runtime_error("test failed: " #COND); } while (0) + +int main() try { + tslog::init(); + + auto &checker = policy_checker_system(); + checker.initDb("tests/default_deny/system-multiple-includedirs.conf"); + + auto &db = checker.getPolicyDb(); + tassert(test_destination(db, "org.tizen.test.allow-me-for-root") == ldp_xml_parser::Decision::ALLOW); + tassert(test_destination(db, "org.tizen.test.deny-me-for-root") == ldp_xml_parser::Decision::DENY); + tassert(test_destination(db, "org.tizen.test.another-allow-me-for-root") == ldp_xml_parser::Decision::ALLOW); + tassert(test_destination(db, "org.tizen.test.another-deny-me-for-root") == ldp_xml_parser::Decision::DENY); + tassert(test_destination(db, "org.tizen.test.yet-another-allow-me-for-root") == ldp_xml_parser::Decision::ALLOW); + tassert(test_destination(db, "org.tizen.test.yet-another-deny-me-for-root") == ldp_xml_parser::Decision::DENY); + + return 0; +} catch (std::runtime_error &e) { + std::cerr << e.what(); + return 1; +} diff --git a/tests/default_deny/another-system.d/some-service.conf b/tests/default_deny/another-system.d/some-service.conf new file mode 100644 index 0000000..fae3d97 --- /dev/null +++ b/tests/default_deny/another-system.d/some-service.conf @@ -0,0 +1,12 @@ + + + + + + + + + + diff --git a/tests/default_deny/system-multiple-includedirs.conf b/tests/default_deny/system-multiple-includedirs.conf new file mode 100644 index 0000000..99a7a2e --- /dev/null +++ b/tests/default_deny/system-multiple-includedirs.conf @@ -0,0 +1,20 @@ + + + + + + + + system-base.conf + + another-system.d + yet-another-system.d + + + diff --git a/tests/default_deny/system.d/some-service.conf b/tests/default_deny/system.d/some-service.conf new file mode 100644 index 0000000..4252922 --- /dev/null +++ b/tests/default_deny/system.d/some-service.conf @@ -0,0 +1,15 @@ + + + + + + + + + + + + + diff --git a/tests/default_deny/yet-another-system.d/some-service.conf b/tests/default_deny/yet-another-system.d/some-service.conf new file mode 100644 index 0000000..4ec24e9 --- /dev/null +++ b/tests/default_deny/yet-another-system.d/some-service.conf @@ -0,0 +1,12 @@ + + + + + + + + + + -- 2.7.4 From 0821d6ca80fe71fdc251580c56cbe4595f6a84f3 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Thu, 9 Apr 2020 10:01:48 +0200 Subject: [PATCH 15/16] xml-parser: don't clear list of included files in each dir The 'includedir' directives should make the parser include all the files in the directories introduced with the directives. This commit removes clearing of the gathered file list when the directive is encountered. Change-Id: Id14be322e8696bd85ddfbb0ba2360a7b4b5bcda4 --- src/internal/xml_parser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/xml_parser.cpp b/src/internal/xml_parser.cpp index dba29ff..4bcbb1d 100644 --- a/src/internal/xml_parser.cpp +++ b/src/internal/xml_parser.cpp @@ -284,7 +284,7 @@ void XmlParser::getIncludedFiles(const std::string& parent_dir, const std::strin DIR *dir; struct dirent *ent; const std::string dname = expandPath(parent_dir, incldir); - files.clear(); + if ((dir = opendir(dname.c_str())) != NULL) { while ((ent = readdir(dir)) != NULL) { std::string s(ent->d_name); -- 2.7.4 From 434396148108ae1dad3215ed696e984b9930caaf Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Thu, 9 Apr 2020 12:38:40 +0200 Subject: [PATCH 16/16] finder: improve printing Add actual attribute names to printing instead of printing just "send" or "receive". Change-Id: Ib117847d44a57abbf1fd1217a786c1d2d8b5d218 --- src/dbuspolicy_finder.cpp | 65 +++++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/src/dbuspolicy_finder.cpp b/src/dbuspolicy_finder.cpp index f22e4cd..a8edb68 100644 --- a/src/dbuspolicy_finder.cpp +++ b/src/dbuspolicy_finder.cpp @@ -77,43 +77,48 @@ MessageType stringToMessageType(std::string tmp) { } } -std::pair getItemStrings (const ItemSend &) { - return std::make_pair ("send", "send_destination"); +std::string getPrefix(const ItemSend &) { + return "send_"; } -std::pair getItemStrings (const ItemReceive &) { - return std::make_pair ("receive", "receive_sender"); + +std::string getPrefix(const ItemReceive &) { + return "receive_"; +} + +std::string getNameAttr(const ItemSend &) { + return "destination"; +} + +std::string getNameAttr(const ItemReceive &) { + return "sender"; } template void printDecision(const T & item) { bool everything_is_null = true; - const auto strings = getItemStrings(item); + const auto prefix = getPrefix(item); + + auto printAttribute = [&everything_is_null, &prefix](const std::string &attr_name, const std::string &attr_value) { + if (!attr_value.empty()) { + std::cout << " " << prefix << attr_name << "=\"" << attr_value << "\""; + everything_is_null = false; + } + }; + + printAttribute("interface", item.getInterface()); + printAttribute("member", item.getMember()); + printAttribute("path", item.getPath()); + + const auto nameAttr = getNameAttr(item); + if (!item.isNamePrefix()) + printAttribute(nameAttr, item.getName()); + else + printAttribute(nameAttr + "_prefix", item.getName()); - if (!item.getInterface().empty()) { - everything_is_null = false; - std::cout << " " << strings.first << "=\"" << item.getInterface() << "\""; - } - if (!item.getMember().empty()) { - everything_is_null = false; - std::cout << " " << strings.first << "=\"" << item.getMember() << "\""; - } - if (!item.getPath().empty()) { - everything_is_null = false; - std::cout << " " << strings.first << "=\"" << item.getPath() << "\""; - } - if (!item.getName().empty()) { - everything_is_null = false; - std::cout << " " << strings.first << "=\"" << item.getName() << "\""; - } - if (!item.getName().empty() && item.isNamePrefix()) { - everything_is_null = false; - std::cout << " " << strings.second << "=\"" << item.getName() << "\""; - } const auto type_it = std::find_if(types.begin(), types.end(), [&item] (const decltype(types)::value_type & type) { return type.second == item.getType(); } ); - if (type_it != types.end()) { - everything_is_null = false; - std::cout << " type=\"" << type_it->first << "\""; - } + if (type_it != types.end()) + printAttribute("type", type_it->first); + if (everything_is_null) std::cout << '*'; @@ -273,7 +278,7 @@ template void handlePolicy(const std::string & interface, const std int showHelp() { std::cout << "\nUsage:\n" - "dbuspolicy-filter {send|receive|access|own} [options]\n" + "dbuspolicy-finder {send|receive|access|own} [options]\n" "\n" " -c, --configuration {file name} | --session | --system\n" " -e, --explain \tread an error message from standard input and find relevant policy rules\n" -- 2.7.4