mapper: remove configurable service allowlists
Remove the ability to specify an allow list at runtime. Instead,
introspect any DBus service except those that start with
org.freedesktop. This removes the need for argument parsing code and
complex bitbake metadata.
Skip org.freedesktop because applications (OpenBMC or otherwise) should
access org.freedesktop services directly and not indirectly via the
OpenBMC mapper.
Change-Id: I83038a121580dcde2a2b3b1f994b3066cc9d955f
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
diff --git a/meson.build b/meson.build
index 85e2ee0..209c49f 100644
--- a/meson.build
+++ b/meson.build
@@ -85,7 +85,6 @@
'mapperx',
[
'src/main.cpp',
- 'src/argument.cpp',
'src/processing.cpp',
'src/associations.cpp',
],
diff --git a/src/argument.cpp b/src/argument.cpp
deleted file mode 100644
index bdaa916..0000000
--- a/src/argument.cpp
+++ /dev/null
@@ -1,81 +0,0 @@
-/**
- * Copyright © 2018 IBM Corporation
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "argument.hpp"
-
-#include <algorithm>
-#include <cassert>
-#include <iostream>
-#include <iterator>
-
-const std::string ArgumentParser::trueString = "true";
-const std::string ArgumentParser::emptyString = "";
-
-const char* ArgumentParser::optionstr = "s:b:i:?h";
-const option ArgumentParser::options[] = {
- {"service-namespaces", required_argument, nullptr, 's'},
- {"service-blacklists", required_argument, nullptr, 'b'},
- {"interface-namespaces", required_argument, nullptr, 'i'},
- {"help", no_argument, nullptr, 'h'},
- {0, 0, 0, 0},
-};
-
-ArgumentParser::ArgumentParser(int argc, char** argv)
-{
- int option = 0;
- while (-1 !=
- (option = getopt_long(argc, argv, optionstr, options, nullptr)))
- {
- if ((option == '?') || (option == 'h'))
- {
- usage(argv);
- exit(-1);
- }
-
- auto i = &options[0];
- while ((i->val != option) && (i->val != 0))
- ++i;
-
- if (i->val)
- arguments[i->name] = (i->has_arg ? optarg : trueString);
- }
-}
-
-const std::string& ArgumentParser::operator[](const std::string& opt)
-{
- auto i = arguments.find(opt);
- if (i == arguments.end())
- {
- return emptyString;
- }
- else
- {
- return i->second;
- }
-}
-
-void ArgumentParser::usage(char** argv)
-{
- std::cerr << "Usage: " << argv[0] << " [options]" << std::endl;
- std::cerr << "Options:" << std::endl;
- std::cerr << " --help Print this menu" << std::endl;
- std::cerr << " --service-namespaces=<services> Space separated list of ";
- std::cerr << "service namespaces to allow\n";
- std::cerr << " --service-blacklists=<services> Space separated list of ";
- std::cerr << "service names to deny\n";
- std::cerr << " --interface-namespaces=<ifaces> Space separated list of ";
- std::cerr << "interface namespaces to allow\n";
-}
diff --git a/src/argument.hpp b/src/argument.hpp
deleted file mode 100644
index 5974535..0000000
--- a/src/argument.hpp
+++ /dev/null
@@ -1,50 +0,0 @@
-#pragma once
-
-#include <getopt.h>
-
-#include <map>
-#include <string>
-
-/** @brief Class - Encapsulates parsing command line options and
- * populating arguments
- */
-class ArgumentParser
-{
- public:
- ArgumentParser() = delete;
- ~ArgumentParser() = default;
- ArgumentParser(const ArgumentParser&) = delete;
- ArgumentParser& operator=(const ArgumentParser&) = delete;
- ArgumentParser(ArgumentParser&&) = default;
- ArgumentParser& operator=(ArgumentParser&&) = default;
-
- /** @brief Constructs Argument object
- *
- * @param argc - the main function's argc passed as is
- * @param argv - the main function's argv passed as is
- * @return Object constructed
- */
- ArgumentParser(int argc, char** argv);
-
- /** @brief Given a option, returns its argument(optarg) */
- const std::string& operator[](const std::string& opt);
-
- /** @brief Displays usage */
- static void usage(char** argv);
-
- /** @brief Set to 'true' when an option is passed */
- static const std::string trueString;
-
- /** @brief Set to '' when an option is not passed */
- static const std::string emptyString;
-
- private:
- /** @brief Option to argument mapping */
- std::map<const std::string, std::string> arguments;
-
- /** @brief Array of struct options as needed by getopt_long */
- static const option options[];
-
- /** @brief optstring as needed by getopt_long */
- static const char* optionstr;
-};
diff --git a/src/main.cpp b/src/main.cpp
index 109b1b3..2f5e7f9 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -1,6 +1,5 @@
#include "associations.hpp"
#include "processing.hpp"
-#include "src/argument.hpp"
#include "types.hpp"
#include <tinyxml2.h>
@@ -23,8 +22,6 @@
AssociationMaps associationMaps;
-static AllowDenyList serviceAllowList;
-
void updateOwners(sdbusplus::asio::connection* conn,
boost::container::flat_map<std::string, std::string>& owners,
const std::string& newObject)
@@ -264,7 +261,7 @@
#endif
sdbusplus::asio::object_server& objectServer)
{
- if (needToIntrospect(processName, serviceAllowList))
+ if (needToIntrospect(processName))
{
std::shared_ptr<InProgressIntrospect> transaction =
std::make_shared<InProgressIntrospect>(systemBus, io, processName,
@@ -326,7 +323,7 @@
#endif
for (const std::string& processName : processNames)
{
- if (needToIntrospect(processName, serviceAllowList))
+ if (needToIntrospect(processName))
{
startNewIntrospect(systemBus, io, interfaceMap, processName,
assocMaps,
@@ -342,24 +339,6 @@
"ListNames");
}
-void splitArgs(const std::string& stringArgs,
- boost::container::flat_set<std::string>& listArgs)
-{
- std::istringstream args;
- std::string arg;
-
- args.str(stringArgs);
-
- while (!args.eof())
- {
- args >> arg;
- if (!arg.empty())
- {
- listArgs.insert(arg);
- }
- }
-}
-
void addObjectMapResult(std::vector<InterfaceMapType::value_type>& objectMap,
const std::string& objectPath,
const ConnectionNames::value_type& interfaceMap)
@@ -665,15 +644,12 @@
return ret;
}
-int main(int argc, char** argv)
+int main()
{
- auto options = ArgumentParser(argc, argv);
boost::asio::io_context io;
std::shared_ptr<sdbusplus::asio::connection> systemBus =
std::make_shared<sdbusplus::asio::connection>(io);
- splitArgs(options["service-namespaces"], serviceAllowList);
-
sdbusplus::asio::object_server server(systemBus);
// Construct a signal set registered for process termination.
@@ -707,7 +683,7 @@
std::chrono::steady_clock::now());
#endif
// New daemon added
- if (needToIntrospect(name, serviceAllowList))
+ if (needToIntrospect(name))
{
nameOwners[newOwner] = name;
startNewIntrospect(systemBus.get(), io, interfaceMap, name,
@@ -734,7 +710,7 @@
{
return; // only introspect well-known
}
- if (needToIntrospect(wellKnown, serviceAllowList))
+ if (needToIntrospect(wellKnown))
{
processInterfaceAdded(interfaceMap, objPath, interfacesAdded,
wellKnown, associationMaps, server);
diff --git a/src/processing.cpp b/src/processing.cpp
index 5a733d4..5590d16 100644
--- a/src/processing.cpp
+++ b/src/processing.cpp
@@ -1,7 +1,10 @@
#include "processing.hpp"
+#include <algorithm>
+#include <array>
#include <iostream>
#include <string>
+#include <string_view>
bool getWellKnown(
const boost::container::flat_map<std::string, std::string>& owners,
@@ -23,15 +26,17 @@
return true;
}
-bool needToIntrospect(const std::string& processName,
- const AllowDenyList& allowList)
+bool needToIntrospect(const std::string& processName)
{
- auto inAllowList = std::find_if(allowList.begin(), allowList.end(),
- [&processName](const auto& prefix) {
- return processName.starts_with(prefix);
- }) != allowList.end();
+ using namespace std::string_view_literals;
+ static constexpr std::array<std::string_view, 2> skipNamespaces{
+ ":"sv, "org.freedesktop"sv};
- return inAllowList;
+ auto inSkipList = std::find_if(skipNamespaces.begin(), skipNamespaces.end(),
+ [&processName](auto prefix) {
+ return processName.starts_with(prefix);
+ }) != skipNamespaces.end();
+ return !(inSkipList || processName.empty());
}
void processNameChangeDelete(
diff --git a/src/processing.hpp b/src/processing.hpp
index d17cacd..3777ff4 100644
--- a/src/processing.hpp
+++ b/src/processing.hpp
@@ -4,14 +4,10 @@
#include "types.hpp"
#include <boost/container/flat_map.hpp>
-#include <boost/container/flat_set.hpp>
#include <cassert>
#include <string>
-/** @brief Define allow list and deny list data structure */
-using AllowDenyList = boost::container::flat_set<std::string>;
-
/** @brief The associations definitions interface */
constexpr const char* assocDefsInterface =
"xyz.openbmc_project.Association.Definitions";
@@ -45,16 +41,14 @@
/** @brief Determine if dbus service is something to monitor
*
- * mapper supports an allowlist concept. If an allowlist is provided as input
- * then only dbus objects matching that list is monitored.
+ * mapper does not monitor all DBus services. needToIntrospect determines
+ * whether or not a service is to be monitored.
*
* @param[in] processName - Dbus service name
- * @param[in] allowList - The allow list
*
* @return True if input processName should be monitored, false otherwise
*/
-bool needToIntrospect(const std::string& processName,
- const AllowDenyList& allowList);
+bool needToIntrospect(const std::string& processName);
/** @brief Handle the removal of an existing name in objmgr data structures
*
diff --git a/src/test/need_to_introspect.cpp b/src/test/need_to_introspect.cpp
index 3150e3f..e0cb044 100644
--- a/src/test/need_to_introspect.cpp
+++ b/src/test/need_to_introspect.cpp
@@ -5,17 +5,55 @@
// Verify if name is empty, false is returned
TEST(NeedToIntrospect, PassEmptyName)
{
- AllowDenyList allowList;
std::string processName;
- EXPECT_FALSE(needToIntrospect(processName, allowList));
+ EXPECT_FALSE(needToIntrospect(processName));
}
-// Verify if name is on allowlist, true is returned
-TEST(NeedToIntrospect, ValidAllowListName)
+// Verify if name is org, true is returned
+TEST(NeedToIntrospect, NameOrg)
{
- AllowDenyList allowList = {"xyz.openbmc_project"};
- std::string processName = "xyz.openbmc_project.State.Host";
+ std::string processName = "org";
- EXPECT_TRUE(needToIntrospect(processName, allowList));
+ EXPECT_TRUE(needToIntrospect(processName));
+}
+
+// Verify if name is org.freedesktop, false is returned
+TEST(NeedToIntrospect, NameOrgFreedesktop)
+{
+ std::string processName = "org.freedesktop";
+
+ EXPECT_FALSE(needToIntrospect(processName));
+}
+
+// Verify if name is org.freedesktop.foo, false is returned
+TEST(NeedToIntrospect, nameOrgFreeDesktopFoo)
+{
+ std::string processName = "org.freedesktop.foo";
+
+ EXPECT_FALSE(needToIntrospect(processName));
+}
+
+// Verify if name is org.openbmc, true is returned
+TEST(NeedToIntrospect, nameOrgOpenBMC)
+{
+ std::string processName = "org.openbmc";
+
+ EXPECT_TRUE(needToIntrospect(processName));
+}
+
+// Verify if name is a colon, false is returned
+TEST(NeedToIntrospect, NameColon)
+{
+ std::string processName = ":";
+
+ EXPECT_FALSE(needToIntrospect(processName));
+}
+
+// Verify if name is a unique name, false is returned
+TEST(NeedToIntrospect, NameUnique)
+{
+ std::string processName = ":1.32";
+
+ EXPECT_FALSE(needToIntrospect(processName));
}