diff options
Diffstat (limited to 'meta-openbmc-mods/meta-common/recipes-phosphor')
16 files changed, 1977 insertions, 5 deletions
diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/configuration/entity-manager/0008-dynamic-threshold-configuration-for-SOLUM-PSU.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/configuration/entity-manager/0008-dynamic-threshold-configuration-for-SOLUM-PSU.patch new file mode 100644 index 000000000..16df1b436 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/configuration/entity-manager/0008-dynamic-threshold-configuration-for-SOLUM-PSU.patch @@ -0,0 +1,90 @@ +From d7425aa548716339e9c00a45695f93c294613cd4 Mon Sep 17 00:00:00 2001 +From: Vikash Chandola <vikash.chandola@intel.com> +Date: Fri, 5 Aug 2022 12:57:51 +0000 +Subject: [PATCH] dynamic threshold configuration for SOLUM PSU + +PSU output current threshold is different for high line and low line +input. Using static threshold value doesn't allow thresholds +to be set based on input voltage. Update SOLUM configurtion file +to provide sensor label and scale factor to calculate threshold. +This allows thresholds to get updated on input voltage change. + +Tested: +Successfully able to build and pass through validate-configs.py. + +Signed-off-by: Vikash Chandola <vikash.chandola@intel.com> +Change-Id: I84e425f1e75ce04be96b70b8a91e90f7c796c9e8 +--- + configurations/PSSF162205A.json | 4 ++++ + configurations/SOLUM_PSSF162202_PSU.json | 4 ++++ + schemas/legacy.json | 8 ++++++++ + 3 files changed, 16 insertions(+) + +diff --git a/configurations/PSSF162205A.json b/configurations/PSSF162205A.json +index d272ef1..7402947 100644 +--- a/configurations/PSSF162205A.json ++++ b/configurations/PSSF162205A.json +@@ -86,14 +86,18 @@ + "Direction": "greater than", + "Label": "iout1", + "Name": "upper critical", ++ "ScaleFactor": 0.001, + "Severity": 1, ++ "ThresholdLabel": "iout_oc_warn_limit", + "Value": 122 + }, + { + "Direction": "greater than", + "Label": "iout1", + "Name": "upper non critical", ++ "ScaleFactor": 0.00091, + "Severity": 0, ++ "ThresholdLabel": "iout_oc_warn_limit", + "Value": 100 + }, + { +diff --git a/configurations/SOLUM_PSSF162202_PSU.json b/configurations/SOLUM_PSSF162202_PSU.json +index c3ca25c..176a3d6 100644 +--- a/configurations/SOLUM_PSSF162202_PSU.json ++++ b/configurations/SOLUM_PSSF162202_PSU.json +@@ -145,14 +145,18 @@ + "Direction": "greater than", + "Label": "iout1", + "Name": "upper critical", ++ "ScaleFactor": 0.001, + "Severity": 1, ++ "ThresholdLabel": "iout_oc_warn_limit", + "Value": 122 + }, + { + "Direction": "greater than", + "Label": "iout1", + "Name": "upper non critical", ++ "ScaleFactor": 0.00091, + "Severity": 0, ++ "ThresholdLabel": "iout_oc_warn_limit", + "Value": 100 + }, + { +diff --git a/schemas/legacy.json b/schemas/legacy.json +index 47a6c7b..c3339ef 100644 +--- a/schemas/legacy.json ++++ b/schemas/legacy.json +@@ -698,6 +698,14 @@ + }, + "Value": { + "type": "number" ++ }, ++ "ThresholdLabel": { ++ "enum": [ ++ "iout_oc_warn_limit" ++ ] ++ }, ++ "ScaleFactor": { ++ "type": "number" + } + }, + "required": [ +-- +2.25.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/configuration/entity-manager_%.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/configuration/entity-manager_%.bbappend index a8a9f17c5..181497547 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/configuration/entity-manager_%.bbappend +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/configuration/entity-manager_%.bbappend @@ -11,5 +11,6 @@ SRC_URI += " file://0001-fru-device-Add-MUX-channel-name-to-FRU-objects.patch \ file://0005-Allow-MUX-idle-state-to-be-configured-as-DISCONNECT.patch \ file://0006-Change-HSBP-FRU-address-and-add-MUX-mode-configurati.patch \ file://0007-Add-HSBP-FRU-details-in-json-configuration.patch \ + file://0008-dynamic-threshold-configuration-for-SOLUM-PSU.patch \ " diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/dbus/phosphor-mapper/0001-add-Associations-endpoints-change-delay-timer.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/dbus/phosphor-mapper/0001-add-Associations-endpoints-change-delay-timer.patch new file mode 100644 index 000000000..51afef80f --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/dbus/phosphor-mapper/0001-add-Associations-endpoints-change-delay-timer.patch @@ -0,0 +1,978 @@ +From 48c39d798ce9683c186395272452893e2227d984 Mon Sep 17 00:00:00 2001 +From: "Kallas, Pawel" <pawel.kallas@intel.com> +Date: Mon, 9 Jan 2023 13:27:45 +0100 +Subject: [PATCH] add Associations endpoints change delay timer + +When multiple associations that point to the same interface are +created, each change (adding or removing one) leads to updating +"endpoints" property on dbus. This property update is time consuming +with many endpoints already present, because each update needs to send +the whole list of current elements plus/minus one. With a lot of +changes in short time it can cause the service to be unresponsive. +This change adds timer to delay updating dbus property. This way many +associations updates can be aggregated into single dbus property +update. + +Tested: Ran on hardware with dbus sensor tester. 4000 created sensors +with interfaces are processed within 10 seconds. Time before the change +was above 2 minutes. + +Signed-off-by: Kallas, Pawel <pawel.kallas@intel.com> +Change-Id: I1083c027ab12238249cffc67fb29a8ffef6baf83 +--- + src/associations.cpp | 156 ++++++++++++++++++++++------------ + src/associations.hpp | 32 +++++-- + src/main.cpp | 48 ++++++----- + src/processing.cpp | 13 +-- + src/processing.hpp | 6 +- + src/test/associations.cpp | 43 ++++++---- + src/test/interfaces_added.cpp | 6 +- + src/test/name_change.cpp | 10 ++- + 8 files changed, 202 insertions(+), 112 deletions(-) + +diff --git a/src/associations.cpp b/src/associations.cpp +index 9fd72ab..7730bbd 100644 +--- a/src/associations.cpp ++++ b/src/associations.cpp +@@ -1,10 +1,81 @@ + #include "associations.hpp" + + #include <boost/algorithm/string/predicate.hpp> ++#include <boost/asio/steady_timer.hpp> + #include <iostream> + #include <sdbusplus/exception.hpp> + +-void removeAssociation(const std::string& sourcePath, const std::string& owner, ++void updateEndpointsOnDbus(sdbusplus::asio::object_server& objectServer, ++ const std::string& assocPath, ++ AssociationMaps& assocMaps) ++{ ++ auto& iface = assocMaps.ifaces[assocPath]; ++ auto& i = std::get<ifacePos>(iface); ++ auto& endpoints = std::get<endpointsPos>(iface); ++ ++ // If the interface already exists, only need to update ++ // the property value, otherwise create it ++ if (i) ++ { ++ if (endpoints.empty()) ++ { ++ objectServer.remove_interface(i); ++ i = nullptr; ++ } ++ else ++ { ++ i->set_property("endpoints", endpoints); ++ } ++ } ++ else ++ { ++ if (!endpoints.empty()) ++ { ++ i = objectServer.add_interface(assocPath, ++ XYZ_ASSOCIATION_INTERFACE); ++ i->register_property("endpoints", endpoints); ++ i->initialize(); ++ } ++ } ++} ++ ++void scheduleUpdateEndpointsOnDbus(boost::asio::io_context& io, ++ sdbusplus::asio::object_server& objectServer, ++ const std::string& assocPath, ++ AssociationMaps& assocMaps) ++{ ++ static std::set<std::string> delayedUpdatePaths; ++ ++ if (delayedUpdatePaths.contains(assocPath)) ++ { ++ return; ++ } ++ ++ auto& iface = assocMaps.ifaces[assocPath]; ++ auto& endpoints = std::get<endpointsPos>(iface); ++ ++ if (endpoints.size() > endpointsCountTimerThreshold) ++ { ++ delayedUpdatePaths.emplace(assocPath); ++ auto timer = std::make_shared<boost::asio::steady_timer>( ++ io, std::chrono::seconds(endpointUpdateDelaySeconds)); ++ timer->async_wait([&objectServer, &assocMaps, timer, ++ assocPath](const boost::system::error_code& ec) { ++ if (!ec) ++ { ++ updateEndpointsOnDbus(objectServer, assocPath, assocMaps); ++ } ++ delayedUpdatePaths.erase(assocPath); ++ }); ++ } ++ else ++ { ++ updateEndpointsOnDbus(objectServer, assocPath, assocMaps); ++ } ++} ++ ++void removeAssociation(boost::asio::io_context& io, ++ const std::string& sourcePath, const std::string& owner, + sdbusplus::asio::object_server& server, + AssociationMaps& assocMaps) + { +@@ -34,7 +105,7 @@ void removeAssociation(const std::string& sourcePath, const std::string& owner, + + for (const auto& [assocPath, endpointsToRemove] : assocs->second) + { +- removeAssociationEndpoints(server, assocPath, endpointsToRemove, ++ removeAssociationEndpoints(io, server, assocPath, endpointsToRemove, + assocMaps); + } + +@@ -51,7 +122,8 @@ void removeAssociation(const std::string& sourcePath, const std::string& owner, + } + + void removeAssociationEndpoints( +- sdbusplus::asio::object_server& objectServer, const std::string& assocPath, ++ boost::asio::io_context& io, sdbusplus::asio::object_server& objectServer, ++ const std::string& assocPath, + const boost::container::flat_set<std::string>& endpointsToRemove, + AssociationMaps& assocMaps) + { +@@ -74,22 +146,12 @@ void removeAssociationEndpoints( + } + } + +- if (endpointsInDBus.empty()) +- { +- objectServer.remove_interface(std::get<ifacePos>(assoc->second)); +- std::get<ifacePos>(assoc->second) = nullptr; +- std::get<endpointsPos>(assoc->second).clear(); +- } +- else +- { +- std::get<ifacePos>(assoc->second) +- ->set_property("endpoints", endpointsInDBus); +- } ++ scheduleUpdateEndpointsOnDbus(io, objectServer, assocPath, assocMaps); + } + + void checkAssociationEndpointRemoves( +- const std::string& sourcePath, const std::string& owner, +- const AssociationPaths& newAssociations, ++ boost::asio::io_context& io, const std::string& sourcePath, ++ const std::string& owner, const AssociationPaths& newAssociations, + sdbusplus::asio::object_server& objectServer, AssociationMaps& assocMaps) + { + // Find the services that have associations on this path. +@@ -118,7 +180,7 @@ void checkAssociationEndpointRemoves( + auto newEndpoints = newAssociations.find(originalAssocPath); + if (newEndpoints == newAssociations.end()) + { +- removeAssociationEndpoints(objectServer, originalAssocPath, ++ removeAssociationEndpoints(io, objectServer, originalAssocPath, + originalEndpoints, assocMaps); + } + else +@@ -138,7 +200,7 @@ void checkAssociationEndpointRemoves( + } + if (!toRemove.empty()) + { +- removeAssociationEndpoints(objectServer, originalAssocPath, ++ removeAssociationEndpoints(io, objectServer, originalAssocPath, + toRemove, assocMaps); + } + } +@@ -146,12 +208,12 @@ void checkAssociationEndpointRemoves( + } + + void addEndpointsToAssocIfaces( +- sdbusplus::asio::object_server& objectServer, const std::string& assocPath, ++ boost::asio::io_context& io, sdbusplus::asio::object_server& objectServer, ++ const std::string& assocPath, + const boost::container::flat_set<std::string>& endpointPaths, + AssociationMaps& assocMaps) + { + auto& iface = assocMaps.ifaces[assocPath]; +- auto& i = std::get<ifacePos>(iface); + auto& endpoints = std::get<endpointsPos>(iface); + + // Only add new endpoints +@@ -162,22 +224,11 @@ void addEndpointsToAssocIfaces( + endpoints.push_back(e); + } + } +- +- // If the interface already exists, only need to update +- // the property value, otherwise create it +- if (i) +- { +- i->set_property("endpoints", endpoints); +- } +- else +- { +- i = objectServer.add_interface(assocPath, XYZ_ASSOCIATION_INTERFACE); +- i->register_property("endpoints", endpoints); +- i->initialize(); +- } ++ scheduleUpdateEndpointsOnDbus(io, objectServer, assocPath, assocMaps); + } + +-void associationChanged(sdbusplus::asio::object_server& objectServer, ++void associationChanged(boost::asio::io_context& io, ++ sdbusplus::asio::object_server& objectServer, + const std::vector<Association>& associations, + const std::string& path, const std::string& owner, + const interface_map_type& interfaceMap, +@@ -217,12 +268,12 @@ void associationChanged(sdbusplus::asio::object_server& objectServer, + } + for (const auto& object : objects) + { +- addEndpointsToAssocIfaces(objectServer, object.first, object.second, ++ addEndpointsToAssocIfaces(io, objectServer, object.first, object.second, + assocMaps); + } + + // Check for endpoints being removed instead of added +- checkAssociationEndpointRemoves(path, owner, objects, objectServer, ++ checkAssociationEndpointRemoves(io, path, owner, objects, objectServer, + assocMaps); + + if (!objects.empty()) +@@ -312,7 +363,8 @@ void removeFromPendingAssociations(const std::string& endpointPath, + } + } + +-void addSingleAssociation(sdbusplus::asio::object_server& server, ++void addSingleAssociation(boost::asio::io_context& io, ++ sdbusplus::asio::object_server& server, + const std::string& assocPath, + const std::string& endpoint, const std::string& owner, + const std::string& ownerPath, +@@ -320,7 +372,7 @@ void addSingleAssociation(sdbusplus::asio::object_server& server, + { + boost::container::flat_set<std::string> endpoints{endpoint}; + +- addEndpointsToAssocIfaces(server, assocPath, endpoints, assocMaps); ++ addEndpointsToAssocIfaces(io, server, assocPath, endpoints, assocMaps); + + AssociationPaths objects; + boost::container::flat_set e{endpoint}; +@@ -355,7 +407,8 @@ void addSingleAssociation(sdbusplus::asio::object_server& server, + } + } + +-void checkIfPendingAssociation(const std::string& objectPath, ++void checkIfPendingAssociation(boost::asio::io_context& io, ++ const std::string& objectPath, + const interface_map_type& interfaceMap, + AssociationMaps& assocMaps, + sdbusplus::asio::object_server& server) +@@ -399,13 +452,13 @@ void checkIfPendingAssociation(const std::string& objectPath, + + try + { +- addSingleAssociation(server, assocPath, endpointPath, owner, ++ addSingleAssociation(io, server, assocPath, endpointPath, owner, + ownerPath, assocMaps); + + // Now the reverse direction (still the same owner and ownerPath) + assocPath = endpointPath + '/' + std::get<reverseTypePos>(e); + endpointPath = objectPath; +- addSingleAssociation(server, assocPath, endpointPath, owner, ++ addSingleAssociation(io, server, assocPath, endpointPath, owner, + ownerPath, assocMaps); + } + catch (const sdbusplus::exception::exception& e) +@@ -495,7 +548,8 @@ void findAssociations(const std::string& endpointPath, + * @param[in,out] assocMaps - the association maps + * @param[in,out] server - sdbus system object + */ +-void removeAssociationIfacesEntry(const std::string& assocPath, ++void removeAssociationIfacesEntry(boost::asio::io_context& io, ++ const std::string& assocPath, + const std::string& endpointPath, + AssociationMaps& assocMaps, + sdbusplus::asio::object_server& server) +@@ -509,16 +563,7 @@ void removeAssociationIfacesEntry(const std::string& assocPath, + { + endpoints.erase(e); + +- if (endpoints.empty()) +- { +- server.remove_interface(std::get<ifacePos>(assoc->second)); +- std::get<ifacePos>(assoc->second) = nullptr; +- } +- else +- { +- std::get<ifacePos>(assoc->second) +- ->set_property("endpoints", endpoints); +- } ++ scheduleUpdateEndpointsOnDbus(io, server, assocPath, assocMaps); + } + } + } +@@ -577,7 +622,8 @@ void removeAssociationOwnersEntry(const std::string& assocPath, + } + } + +-void moveAssociationToPending(const std::string& endpointPath, ++void moveAssociationToPending(boost::asio::io_context& io, ++ const std::string& endpointPath, + AssociationMaps& assocMaps, + sdbusplus::asio::object_server& server) + { +@@ -599,9 +645,9 @@ void moveAssociationToPending(const std::string& endpointPath, + reverseType, owner, assocMaps); + + // Remove both sides of the association from assocMaps.ifaces +- removeAssociationIfacesEntry(forwardPath + '/' + forwardType, ++ removeAssociationIfacesEntry(io, forwardPath + '/' + forwardType, + reversePath, assocMaps, server); +- removeAssociationIfacesEntry(reversePath + '/' + reverseType, ++ removeAssociationIfacesEntry(io, reversePath + '/' + reverseType, + forwardPath, assocMaps, server); + + // Remove both sides of the association from assocMaps.owners +diff --git a/src/associations.hpp b/src/associations.hpp +index dd6cbdb..b4ee91b 100644 +--- a/src/associations.hpp ++++ b/src/associations.hpp +@@ -5,8 +5,12 @@ + constexpr const char* XYZ_ASSOCIATION_INTERFACE = + "xyz.openbmc_project.Association"; + ++constexpr size_t endpointsCountTimerThreshold = 100; ++constexpr int endpointUpdateDelaySeconds = 1; ++ + /** @brief Remove input association + * ++ * @param[in] io - io context + * @param[in] sourcePath - Path of the object that contains the + * org.openbmc.Associations + * @param[in] owner - The Dbus service having its associations +@@ -16,7 +20,8 @@ constexpr const char* XYZ_ASSOCIATION_INTERFACE = + * + * @return Void, server, assocMaps updated if needed + */ +-void removeAssociation(const std::string& sourcePath, const std::string& owner, ++void removeAssociation(boost::asio::io_context& io, ++ const std::string& sourcePath, const std::string& owner, + sdbusplus::asio::object_server& server, + AssociationMaps& assocMaps); + +@@ -25,6 +30,7 @@ void removeAssociation(const std::string& sourcePath, const std::string& owner, + * If the last endpoint was removed, then remove the whole + * association object, otherwise just set the property + * ++ * @param[in] io - io context + * @param[in] objectServer - sdbus system object + * @param[in] assocPath - Path of the object that contains the + * org.openbmc.Associations +@@ -34,7 +40,8 @@ void removeAssociation(const std::string& sourcePath, const std::string& owner, + * @return Void, objectServer and assocMaps updated if needed + */ + void removeAssociationEndpoints( +- sdbusplus::asio::object_server& objectServer, const std::string& assocPath, ++ boost::asio::io_context& io, sdbusplus::asio::object_server& objectServer, ++ const std::string& assocPath, + const boost::container::flat_set<std::string>& endpointsToRemove, + AssociationMaps& assocMaps); + +@@ -47,6 +54,7 @@ void removeAssociationEndpoints( + * from the endpoints property, remove that whole association object from + * D-Bus. + * ++ * @param[in] io - io context + * @param[in] sourcePath - Path of the object that contains the + * org.openbmc.Associations + * @param[in] owner - The Dbus service having it's associatons +@@ -58,8 +66,8 @@ void removeAssociationEndpoints( + * @return Void, objectServer and assocMaps updated if needed + */ + void checkAssociationEndpointRemoves( +- const std::string& sourcePath, const std::string& owner, +- const AssociationPaths& newAssociations, ++ boost::asio::io_context& io, const std::string& sourcePath, ++ const std::string& owner, const AssociationPaths& newAssociations, + sdbusplus::asio::object_server& objectServer, AssociationMaps& assocMaps); + + /** @brief Handle new or changed association interfaces +@@ -67,6 +75,7 @@ void checkAssociationEndpointRemoves( + * Called when either a new org.openbmc.Associations interface was + * created, or the associations property on that interface changed + * ++ * @param[in] io - io context + * @param[in,out] objectServer - sdbus system object + * @param[in] associations - New associations to look at for change + * @param[in] path - Path of the object that contains the +@@ -78,7 +87,8 @@ void checkAssociationEndpointRemoves( + * + * @return Void, objectServer and assocMaps updated if needed + */ +-void associationChanged(sdbusplus::asio::object_server& objectServer, ++void associationChanged(boost::asio::io_context& io, ++ sdbusplus::asio::object_server& objectServer, + const std::vector<Association>& associations, + const std::string& path, const std::string& owner, + const interface_map_type& interfaceMap, +@@ -123,6 +133,7 @@ void removeFromPendingAssociations(const std::string& endpointPath, + + /** @brief Adds a single association D-Bus object (<path>/<type>) + * ++ * @param[in] io - io context + * @param[in,out] server - sdbus system object + * @param[in] assocPath - The association D-Bus path + * @param[in] endpoint - The association's D-Bus endpoint path +@@ -130,7 +141,8 @@ void removeFromPendingAssociations(const std::string& endpointPath, + * @param[in] ownerPath - The D-Bus path hosting the Associations property + * @param[in,out] assocMaps - The association maps + */ +-void addSingleAssociation(sdbusplus::asio::object_server& server, ++void addSingleAssociation(boost::asio::io_context& io, ++ sdbusplus::asio::object_server& server, + const std::string& assocPath, + const std::string& endpoint, const std::string& owner, + const std::string& ownerPath, +@@ -143,12 +155,14 @@ void addSingleAssociation(sdbusplus::asio::object_server& server, + * map, create the 2 real association objects and remove its pending + * associations entry. Used when a new path shows up in D-Bus. + * ++ * @param[in] io - io context + * @param[in] objectPath - the path to check + * @param[in] interfaceMap - The master interface map + * @param[in,out] assocMaps - The association maps + * @param[in,out] server - sdbus system object + */ +-void checkIfPendingAssociation(const std::string& objectPath, ++void checkIfPendingAssociation(boost::asio::io_context& io, ++ const std::string& objectPath, + const interface_map_type& interfaceMap, + AssociationMaps& assocMaps, + sdbusplus::asio::object_server& server); +@@ -172,10 +186,12 @@ void findAssociations(const std::string& endpointPath, + * association endpoint (the path that owns the association is still + * on D-Bus), then move the association it's involved in to pending. + * ++ * @param[in] io - io context + * @param[in] endpointPath - the D-Bus endpoint path to check + * @param[in,out] assocMaps - The association maps + * @param[in,out] server - sdbus system object + */ +-void moveAssociationToPending(const std::string& endpointPath, ++void moveAssociationToPending(boost::asio::io_context& io, ++ const std::string& endpointPath, + AssociationMaps& assocMaps, + sdbusplus::asio::object_server& server); +diff --git a/src/main.cpp b/src/main.cpp +index 53b36d5..ee03be7 100644 +--- a/src/main.cpp ++++ b/src/main.cpp +@@ -131,7 +131,8 @@ struct InProgressIntrospect + #endif + }; + +-void do_associations(sdbusplus::asio::connection* system_bus, ++void do_associations(boost::asio::io_context& io, ++ sdbusplus::asio::connection* system_bus, + interface_map_type& interfaceMap, + sdbusplus::asio::object_server& objectServer, + const std::string& processName, const std::string& path, +@@ -139,7 +140,7 @@ void do_associations(sdbusplus::asio::connection* system_bus, + { + constexpr int maxTimeoutRetries = 3; + system_bus->async_method_call( +- [&objectServer, path, processName, &interfaceMap, system_bus, ++ [&io, &objectServer, path, processName, &interfaceMap, system_bus, + timeoutRetries]( + const boost::system::error_code ec, + const std::variant<std::vector<Association>>& variantAssociations) { +@@ -148,7 +149,7 @@ void do_associations(sdbusplus::asio::connection* system_bus, + if (ec.value() == boost::system::errc::timed_out && + timeoutRetries < maxTimeoutRetries) + { +- do_associations(system_bus, interfaceMap, objectServer, ++ do_associations(io, system_bus, interfaceMap, objectServer, + processName, path, timeoutRetries + 1); + return; + } +@@ -156,14 +157,15 @@ void do_associations(sdbusplus::asio::connection* system_bus, + } + std::vector<Association> associations = + std::get<std::vector<Association>>(variantAssociations); +- associationChanged(objectServer, associations, path, processName, +- interfaceMap, associationMaps); ++ associationChanged(io, objectServer, associations, path, ++ processName, interfaceMap, associationMaps); + }, + processName, path, "org.freedesktop.DBus.Properties", "Get", + assocDefsInterface, assocDefsProperty); + } + +-void do_introspect(sdbusplus::asio::connection* system_bus, ++void do_introspect(boost::asio::io_context& io, ++ sdbusplus::asio::connection* system_bus, + std::shared_ptr<InProgressIntrospect> transaction, + interface_map_type& interface_map, + sdbusplus::asio::object_server& objectServer, +@@ -171,7 +173,7 @@ void do_introspect(sdbusplus::asio::connection* system_bus, + { + constexpr int maxTimeoutRetries = 3; + system_bus->async_method_call( +- [&interface_map, &objectServer, transaction, path, system_bus, ++ [&io, &interface_map, &objectServer, transaction, path, system_bus, + timeoutRetries](const boost::system::error_code ec, + const std::string& introspect_xml) { + if (ec) +@@ -179,7 +181,7 @@ void do_introspect(sdbusplus::asio::connection* system_bus, + if (ec.value() == boost::system::errc::timed_out && + timeoutRetries < maxTimeoutRetries) + { +- do_introspect(system_bus, transaction, interface_map, ++ do_introspect(io, system_bus, transaction, interface_map, + objectServer, path, timeoutRetries + 1); + return; + } +@@ -220,7 +222,7 @@ void do_introspect(sdbusplus::asio::connection* system_bus, + + if (std::strcmp(iface_name, assocDefsInterface) == 0) + { +- do_associations(system_bus, interface_map, objectServer, ++ do_associations(io, system_bus, interface_map, objectServer, + transaction->process_name, path); + } + +@@ -229,7 +231,7 @@ void do_introspect(sdbusplus::asio::connection* system_bus, + + // Check if this new path has a pending association that can + // now be completed. +- checkIfPendingAssociation(path, interface_map, ++ checkIfPendingAssociation(io, path, interface_map, + transaction->assocMaps, objectServer); + + pElement = pRoot->FirstChildElement("node"); +@@ -244,7 +246,7 @@ void do_introspect(sdbusplus::asio::connection* system_bus, + parent_path.clear(); + } + +- do_introspect(system_bus, transaction, interface_map, ++ do_introspect(io, system_bus, transaction, interface_map, + objectServer, parent_path + "/" + child_path); + } + pElement = pElement->NextSiblingElement("node"); +@@ -275,7 +277,7 @@ void start_new_introspect( + #endif + ); + +- do_introspect(system_bus, transaction, interface_map, objectServer, ++ do_introspect(io, system_bus, transaction, interface_map, objectServer, + "/"); + } + } +@@ -684,7 +686,7 @@ int main(int argc, char** argv) + + if (!old_owner.empty()) + { +- processNameChangeDelete(name_owners, name, old_owner, ++ processNameChangeDelete(io, name_owners, name, old_owner, + interface_map, associationMaps, server); + } + +@@ -715,7 +717,7 @@ int main(int argc, char** argv) + sdbusplus::bus::match::rules::nameOwnerChanged(), nameChangeHandler); + + std::function<void(sdbusplus::message::message & message)> +- interfacesAddedHandler = [&interface_map, &name_owners, &server]( ++ interfacesAddedHandler = [&io, &interface_map, &name_owners, &server]( + sdbusplus::message::message& message) { + sdbusplus::message::object_path obj_path; + InterfacesAdded interfaces_added; +@@ -728,8 +730,9 @@ int main(int argc, char** argv) + if (needToIntrospect(well_known, service_whitelist, + service_blacklist)) + { +- processInterfaceAdded(interface_map, obj_path, interfaces_added, +- well_known, associationMaps, server); ++ processInterfaceAdded(io, interface_map, obj_path, ++ interfaces_added, well_known, ++ associationMaps, server); + } + }; + +@@ -739,7 +742,7 @@ int main(int argc, char** argv) + interfacesAddedHandler); + + std::function<void(sdbusplus::message::message & message)> +- interfacesRemovedHandler = [&interface_map, &name_owners, &server]( ++ interfacesRemovedHandler = [&io, &interface_map, &name_owners, &server]( + sdbusplus::message::message& message) { + sdbusplus::message::object_path obj_path; + std::vector<std::string> interfaces_removed; +@@ -765,7 +768,7 @@ int main(int argc, char** argv) + + if (interface == assocDefsInterface) + { +- removeAssociation(obj_path.str, sender, server, ++ removeAssociation(io, obj_path.str, sender, server, + associationMaps); + } + +@@ -788,8 +791,8 @@ int main(int argc, char** argv) + { + // Remove the 2 association D-Bus paths and move the + // association to pending. +- moveAssociationToPending(obj_path.str, associationMaps, +- server); ++ moveAssociationToPending(io, obj_path.str, ++ associationMaps, server); + } + } + } +@@ -809,7 +812,8 @@ int main(int argc, char** argv) + interfacesRemovedHandler); + + std::function<void(sdbusplus::message::message & message)> +- associationChangedHandler = [&server, &name_owners, &interface_map]( ++ associationChangedHandler = [&io, &server, &name_owners, ++ &interface_map]( + sdbusplus::message::message& message) { + std::string objectName; + boost::container::flat_map<std::string, +@@ -828,7 +832,7 @@ int main(int argc, char** argv) + { + return; + } +- associationChanged(server, associations, message.get_path(), ++ associationChanged(io, server, associations, message.get_path(), + well_known, interface_map, associationMaps); + } + }; +diff --git a/src/processing.cpp b/src/processing.cpp +index f144b77..5e61fbc 100644 +--- a/src/processing.cpp ++++ b/src/processing.cpp +@@ -40,6 +40,7 @@ bool needToIntrospect(const std::string& processName, + } + + void processNameChangeDelete( ++ boost::asio::io_context& io, + boost::container::flat_map<std::string, std::string>& nameOwners, + const std::string& wellKnown, const std::string& oldOwner, + interface_map_type& interfaceMap, AssociationMaps& assocMaps, +@@ -67,7 +68,8 @@ void processNameChangeDelete( + assocDefsInterface); + if (assoc != ifaces->second.end()) + { +- removeAssociation(pathIt->first, wellKnown, server, assocMaps); ++ removeAssociation(io, pathIt->first, wellKnown, server, ++ assocMaps); + } + + // Instead of checking if every single path is the endpoint of an +@@ -80,7 +82,7 @@ void processNameChangeDelete( + { + // Remove the 2 association D-Bus paths and move the + // association to pending. +- moveAssociationToPending(pathIt->first, assocMaps, server); ++ moveAssociationToPending(io, pathIt->first, assocMaps, server); + } + } + pathIt->second.erase(wellKnown); +@@ -95,7 +97,8 @@ void processNameChangeDelete( + } + } + +-void processInterfaceAdded(interface_map_type& interfaceMap, ++void processInterfaceAdded(boost::asio::io_context& io, ++ interface_map_type& interfaceMap, + const sdbusplus::message::object_path& objPath, + const InterfacesAdded& intfAdded, + const std::string& wellKnown, +@@ -127,7 +130,7 @@ void processInterfaceAdded(interface_map_type& interfaceMap, + } + std::vector<Association> associations = + std::get<std::vector<Association>>(*variantAssociations); +- associationChanged(server, associations, objPath.str, wellKnown, ++ associationChanged(io, server, associations, objPath.str, wellKnown, + interfaceMap, assocMaps); + } + } +@@ -178,5 +181,5 @@ void processInterfaceAdded(interface_map_type& interfaceMap, + } + + // The new interface might have an association pending +- checkIfPendingAssociation(objPath.str, interfaceMap, assocMaps, server); ++ checkIfPendingAssociation(io, objPath.str, interfaceMap, assocMaps, server); + } +diff --git a/src/processing.hpp b/src/processing.hpp +index 114b502..98be262 100644 +--- a/src/processing.hpp ++++ b/src/processing.hpp +@@ -60,6 +60,7 @@ bool needToIntrospect(const std::string& processName, + + /** @brief Handle the removal of an existing name in objmgr data structures + * ++ * @param[in] io - io context + * @param[in,out] nameOwners - Map of unique name to well known name + * @param[in] wellKnown - Well known name that has new owner + * @param[in] oldOwner - Old unique name +@@ -69,6 +70,7 @@ bool needToIntrospect(const std::string& processName, + * + */ + void processNameChangeDelete( ++ boost::asio::io_context& io, + boost::container::flat_map<std::string, std::string>& nameOwners, + const std::string& wellKnown, const std::string& oldOwner, + interface_map_type& interfaceMap, AssociationMaps& assocMaps, +@@ -76,6 +78,7 @@ void processNameChangeDelete( + + /** @brief Handle an interfaces added signal + * ++ * @param[in] io - io context + * @param[in,out] interfaceMap - Global map of interfaces + * @param[in] objPath - New path to process + * @param[in] interfacesAdded - New interfaces to process +@@ -84,7 +87,8 @@ void processNameChangeDelete( + * @param[in,out] server - sdbus system object + * + */ +-void processInterfaceAdded(interface_map_type& interfaceMap, ++void processInterfaceAdded(boost::asio::io_context& io, ++ interface_map_type& interfaceMap, + const sdbusplus::message::object_path& objPath, + const InterfacesAdded& intfAdded, + const std::string& wellKnown, +diff --git a/src/test/associations.cpp b/src/test/associations.cpp +index 73c36ed..f019a76 100644 +--- a/src/test/associations.cpp ++++ b/src/test/associations.cpp +@@ -11,6 +11,12 @@ + + class TestAssociations : public AsioServerClassTest + { ++ public: ++ boost::asio::io_context io; ++ virtual void SetUp() ++ { ++ io.run(); ++ } + }; + sdbusplus::asio::object_server* TestAssociations::AsioServerClassTest::server = + nullptr; +@@ -22,7 +28,7 @@ TEST_F(TestAssociations, SourcePathNotInAssociations) + std::string sourcePath = "/xyz/openbmc_project/no/association"; + AssociationMaps assocMaps; + +- removeAssociation(sourcePath, DEFAULT_DBUS_SVC, *server, assocMaps); ++ removeAssociation(io, sourcePath, DEFAULT_DBUS_SVC, *server, assocMaps); + } + + // Verify call when owner is not in associated owners +@@ -31,7 +37,7 @@ TEST_F(TestAssociations, OwnerNotInAssociations) + AssociationMaps assocMaps; + assocMaps.owners = createDefaultOwnerAssociation(); + +- removeAssociation(DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, *server, ++ removeAssociation(io, DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, *server, + assocMaps); + } + +@@ -42,7 +48,7 @@ TEST_F(TestAssociations, PathNotInAssocInterfaces) + + assocMaps.owners = createDefaultOwnerAssociation(); + +- removeAssociation(DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, *server, ++ removeAssociation(io, DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, *server, + assocMaps); + + EXPECT_TRUE(assocMaps.owners.empty()); +@@ -57,7 +63,7 @@ TEST_F(TestAssociations, PathIsInAssociatedInterfaces) + assocMaps.owners = createDefaultOwnerAssociation(); + assocMaps.ifaces = createDefaultInterfaceAssociation(server); + +- removeAssociation(DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, *server, ++ removeAssociation(io, DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, *server, + assocMaps); + + // Verify owner association was deleted +@@ -83,7 +89,7 @@ TEST_F(TestAssociations, PathIsInAssociatedInterfacesExtraEndpoints) + // Add another endpoint to the assoc interfaces + addEndpointToInterfaceAssociation(assocMaps.ifaces); + +- removeAssociation(DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, *server, ++ removeAssociation(io, DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, *server, + assocMaps); + + // Verify owner association was deleted +@@ -109,7 +115,7 @@ TEST_F(TestAssociations, checkAssociationEndpointRemovesNoEpRemove) + assocMaps.owners = createDefaultOwnerAssociation(); + assocMaps.ifaces = createDefaultInterfaceAssociation(server); + +- checkAssociationEndpointRemoves(DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, ++ checkAssociationEndpointRemoves(io, DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, + newAssocPaths, *server, assocMaps); + + // Verify endpoints were not deleted because they matche with what was +@@ -130,7 +136,7 @@ TEST_F(TestAssociations, checkAssociationEndpointRemovesEpRemoveApDiff) + assocMaps.owners = createDefaultOwnerAssociation(); + assocMaps.ifaces = createDefaultInterfaceAssociation(server); + +- checkAssociationEndpointRemoves(DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, ++ checkAssociationEndpointRemoves(io, DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, + newAssocPaths, *server, assocMaps); + + // Verify initial endpoints were deleted because the new path +@@ -152,7 +158,7 @@ TEST_F(TestAssociations, checkAssociationEndpointRemovesEpRemoveEpChanged) + assocMaps.owners = createDefaultOwnerAssociation(); + assocMaps.ifaces = createDefaultInterfaceAssociation(server); + +- checkAssociationEndpointRemoves(DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, ++ checkAssociationEndpointRemoves(io, DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, + newAssocPaths, *server, assocMaps); + + // Verify initial endpoints were deleted because of different endpoints +@@ -175,7 +181,7 @@ TEST_F(TestAssociations, associationChangedEmptyEndpoint) + assocMaps.ifaces = createDefaultInterfaceAssociation(server); + + // Empty endpoint will result in deletion of corresponding assocInterface +- associationChanged(*server, associations, DEFAULT_SOURCE_PATH, ++ associationChanged(io, *server, associations, DEFAULT_SOURCE_PATH, + DEFAULT_DBUS_SVC, interfaceMap, assocMaps); + + // Both of these should be 0 since we have an invalid endpoint +@@ -203,7 +209,7 @@ TEST_F(TestAssociations, associationChangedAddNewAssoc) + {"/new/source/path", {{DEFAULT_DBUS_SVC, {"a"}}}}, + {"/xyz/openbmc_project/new/endpoint", {{DEFAULT_DBUS_SVC, {"a"}}}}}; + +- associationChanged(*server, associations, "/new/source/path", ++ associationChanged(io, *server, associations, "/new/source/path", + DEFAULT_DBUS_SVC, interfaceMap, assocMaps); + + // Two source paths +@@ -237,7 +243,7 @@ TEST_F(TestAssociations, associationChangedAddNewAssocEmptyObj) + // Make it look like the assoc endpoints are on D-Bus + interface_map_type interfaceMap = createDefaultInterfaceMap(); + +- associationChanged(*server, associations, DEFAULT_SOURCE_PATH, ++ associationChanged(io, *server, associations, DEFAULT_SOURCE_PATH, + DEFAULT_DBUS_SVC, interfaceMap, assocMaps); + + // New associations so ensure it now contains a single entry +@@ -273,7 +279,7 @@ TEST_F(TestAssociations, associationChangedAddNewAssocNewOwner) + assocMaps.owners = createDefaultOwnerAssociation(); + assocMaps.ifaces = createDefaultInterfaceAssociation(server); + +- associationChanged(*server, associations, DEFAULT_SOURCE_PATH, newOwner, ++ associationChanged(io, *server, associations, DEFAULT_SOURCE_PATH, newOwner, + interfaceMap, assocMaps); + + // New endpoint so assocOwners should be same size +@@ -306,7 +312,7 @@ TEST_F(TestAssociations, associationChangedAddNewAssocSameInterface) + assocMaps.owners = createDefaultOwnerAssociation(); + assocMaps.ifaces = createDefaultInterfaceAssociation(server); + +- associationChanged(*server, associations, DEFAULT_SOURCE_PATH, ++ associationChanged(io, *server, associations, DEFAULT_SOURCE_PATH, + DEFAULT_DBUS_SVC, interfaceMap, assocMaps); + + // Should have 3 entries in AssociationInterfaces, one is just missing an +@@ -406,7 +412,7 @@ TEST_F(TestAssociations, associationChangedPending) + AssociationMaps assocMaps; + interface_map_type interfaceMap; + +- associationChanged(*server, associations, "/new/source/path", ++ associationChanged(io, *server, associations, "/new/source/path", + DEFAULT_DBUS_SVC, interfaceMap, assocMaps); + + // No associations were actually added +@@ -460,7 +466,7 @@ TEST_F(TestAssociations, checkIfPending) + EXPECT_EQ(assocMaps.pending.size(), 1); + + // Move the pending association to a real association +- checkIfPendingAssociation(DEFAULT_SOURCE_PATH, interfaceMap, assocMaps, ++ checkIfPendingAssociation(io, DEFAULT_SOURCE_PATH, interfaceMap, assocMaps, + *server); + + EXPECT_TRUE(assocMaps.pending.empty()); +@@ -468,7 +474,8 @@ TEST_F(TestAssociations, checkIfPending) + EXPECT_EQ(assocMaps.ifaces.size(), 2); + + // This shouldn't do anything, since /new/path isn't pending +- checkIfPendingAssociation("/new/path", interfaceMap, assocMaps, *server); ++ checkIfPendingAssociation(io, "/new/path", interfaceMap, assocMaps, ++ *server); + EXPECT_TRUE(assocMaps.pending.empty()); + EXPECT_EQ(assocMaps.owners.size(), 1); + EXPECT_EQ(assocMaps.ifaces.size(), 2); +@@ -525,7 +532,7 @@ TEST_F(TestAssociations, moveAssocToPendingNoOp) + AssociationMaps assocMaps; + + // Not an association, so it shouldn't do anything +- moveAssociationToPending(DEFAULT_ENDPOINT, assocMaps, *server); ++ moveAssociationToPending(io, DEFAULT_ENDPOINT, assocMaps, *server); + + EXPECT_TRUE(assocMaps.pending.empty()); + EXPECT_TRUE(assocMaps.owners.empty()); +@@ -538,7 +545,7 @@ TEST_F(TestAssociations, moveAssocToPending) + assocMaps.owners = createDefaultOwnerAssociation(); + assocMaps.ifaces = createDefaultInterfaceAssociation(server); + +- moveAssociationToPending(DEFAULT_ENDPOINT, assocMaps, *server); ++ moveAssociationToPending(io, DEFAULT_ENDPOINT, assocMaps, *server); + + // Check it's now pending + EXPECT_EQ(assocMaps.pending.size(), 1); +diff --git a/src/test/interfaces_added.cpp b/src/test/interfaces_added.cpp +index ee0e438..14cdc85 100644 +--- a/src/test/interfaces_added.cpp ++++ b/src/test/interfaces_added.cpp +@@ -38,9 +38,13 @@ TEST_F(TestInterfacesAdded, InterfacesAddedGoodPath) + auto intfAdded = + createInterfacesAdded(assocDefsInterface, assocDefsProperty); + +- processInterfaceAdded(interfaceMap, DEFAULT_SOURCE_PATH, intfAdded, ++ boost::asio::io_context io; ++ ++ processInterfaceAdded(io, interfaceMap, DEFAULT_SOURCE_PATH, intfAdded, + DEFAULT_DBUS_SVC, assocMaps, *server); + ++ io.run(); ++ + // Interface map will get the following: + // /logging/entry/1 /logging/entry /logging/ / system/chassis + // dump_InterfaceMapType(interfaceMap); +diff --git a/src/test/name_change.cpp b/src/test/name_change.cpp +index 95d02af..3ac4e41 100644 +--- a/src/test/name_change.cpp ++++ b/src/test/name_change.cpp +@@ -6,6 +6,12 @@ + + class TestNameChange : public AsioServerClassTest + { ++ public: ++ boost::asio::io_context io; ++ virtual void SetUp() ++ { ++ io.run(); ++ } + }; + sdbusplus::asio::object_server* TestNameChange::AsioServerClassTest::server = + nullptr; +@@ -20,7 +26,7 @@ TEST_F(TestNameChange, UniqueNameNoInterfaces) + interface_map_type interfaceMap; + AssociationMaps assocMaps; + +- processNameChangeDelete(nameOwners, wellKnown, oldOwner, interfaceMap, ++ processNameChangeDelete(io, nameOwners, wellKnown, oldOwner, interfaceMap, + assocMaps, *server); + EXPECT_EQ(nameOwners.size(), 0); + } +@@ -42,7 +48,7 @@ TEST_F(TestNameChange, UniqueNameAssociationsAndInterface) + auto interfaceMap = createInterfaceMap( + DEFAULT_SOURCE_PATH, DEFAULT_DBUS_SVC, assocInterfacesSet); + +- processNameChangeDelete(nameOwners, DEFAULT_DBUS_SVC, oldOwner, ++ processNameChangeDelete(io, nameOwners, DEFAULT_DBUS_SVC, oldOwner, + interfaceMap, assocMaps, *server); + EXPECT_EQ(nameOwners.size(), 0); + +-- +2.25.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/dbus/phosphor-mapper_%.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/dbus/phosphor-mapper_%.bbappend index 4fc41d058..05e9fec1b 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/dbus/phosphor-mapper_%.bbappend +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/dbus/phosphor-mapper_%.bbappend @@ -1 +1,3 @@ FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:" + +SRC_URI += "file://0001-add-Associations-endpoints-change-delay-timer.patch" diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd/0001-Avoid-negated-postcode-write-to-D-Bus.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd/0001-Avoid-negated-postcode-write-to-D-Bus.patch new file mode 100644 index 000000000..47afc4568 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd/0001-Avoid-negated-postcode-write-to-D-Bus.patch @@ -0,0 +1,58 @@ +From 3311e449da0c7431f5512138aa61212af3d769a1 Mon Sep 17 00:00:00 2001 +From: Zhikui Ren <zhikui.ren@intel.com> +Date: Mon, 14 Nov 2022 14:14:33 -0800 +Subject: [PATCH] Avoid negated postcode write to D-Bus + +This commit removes the code which writes the negated +postcode value to D-Bus object. This has some side effects +when bombarded data pushed to port 80. + +With this change, if same post code is written on LPC channel +it will be set but D-Bus will not emit the 'PropertiesChanged' +signal. Actually there is no need to emit the signal if actual +property value(postcode) is not changed. + +So if post code changes, D-Bus will emit the signal as usual +with this code. Any client applications depends on this, still +can register for signal for knowing postcode change. + +Renamed bbappend file for generic notation. + +Tested: + - Manually verified by setting different post code, + emits the signal. + + busctl set-property xyz.openbmc_project.State.Boot.Raw + /xyz/openbmc_project/state/boot/raw0 xyz.openbmc_project.State.Boot.Raw + Value t 00 + + dbus-monitor --system "type='signal',member='PropertiesChanged', + interface='org.freedesktop.DBus.Properties', + path_namespace=/xyz/openbmc_project/state/boot/raw0, + arg0=xyz.openbmc_project.State.Boot.Raw" + + - Verified functionality with real post code changes. + +Change-Id: I13e7879a19ada593891821df3702613cc8e84572 +Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com> +Signed-off-by: Zhikui Ren <zhikui.ren@intel.com> +--- + main.cpp | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/main.cpp b/main.cpp +index 3a3b4ce..764c855 100644 +--- a/main.cpp ++++ b/main.cpp +@@ -66,7 +66,7 @@ void PostCodeEventHandler(sdeventplus::source::IO& s, int postFd, uint32_t, + // HACK: Always send property changed signal even for the same code + // since we are single threaded, external users will never see the + // first value. +- reporter->value(std::make_tuple(~code, secondary_post_code_t{}), true); ++ // reporter->value(std::make_tuple(~code, secondary_post_code_t{}), true); + reporter->value(std::make_tuple(code, secondary_post_code_t{})); + + // read depends on old data being cleared since it doens't always read +-- +2.25.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd_%.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd_%.bbappend new file mode 100644 index 000000000..893f410e8 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd_%.bbappend @@ -0,0 +1,9 @@ +FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:" +PROJECT_SRC_DIR := "${THISDIR}/${PN}" + +DEPENDS += " gtest" + +#SRC_URI = "git://github.com/openbmc/phosphor-host-postd.git" +SRCREV = "6a5e0a1cba979c3c793e794c41481221da9a4e33" + +SRC_URI += "file://0001-Avoid-negated-postcode-write-to-D-Bus.patch" diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd_git.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd_git.bbappend deleted file mode 100644 index 21ff75f73..000000000 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/host/phosphor-host-postd_git.bbappend +++ /dev/null @@ -1,4 +0,0 @@ -DEPENDS += " gtest" - -#SRC_URI = "git://github.com/openbmc/phosphor-host-postd.git" -SRCREV = "6a5e0a1cba979c3c793e794c41481221da9a4e33" diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb/vm/0009-virtual_media-Fix-for-bmcweb-crash.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb/vm/0009-virtual_media-Fix-for-bmcweb-crash.patch new file mode 100644 index 000000000..a2a120c22 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb/vm/0009-virtual_media-Fix-for-bmcweb-crash.patch @@ -0,0 +1,47 @@ +From 348186f045f23d59405ac0011e983aea8334598a Mon Sep 17 00:00:00 2001 +From: Jayaprakash Mutyala <mutyalax.jayaprakash@intel.com> +Date: Thu, 4 May 2023 13:56:27 +0000 +Subject: [PATCH] virtual_media: Fix for bmcweb crash + +This is ported from main line. +https://gerrit.openbmc.org/c/openbmc/bmcweb/+/62593 + +bmcweb crashes when user not providing userName or password while +posting ISO form Redfish. + +This fix provides to avoid bmcweb crash when user try to post ISO images +from Redfish without providing username or password. + +Tested: +Redfish validator passed with this change. +Verified by Posting ISO from Redfish. + +Signed-off-by: Jayaprakash Mutyala <mutyalax.jayaprakash@intel.com> +--- + redfish-core/lib/virtual_media.hpp | 10 ++++++++++ + 1 file changed, 10 insertions(+) + +diff --git a/redfish-core/lib/virtual_media.hpp b/redfish-core/lib/virtual_media.hpp +index 6dfc726b..838a08b1 100644 +--- a/redfish-core/lib/virtual_media.hpp ++++ b/redfish-core/lib/virtual_media.hpp +@@ -1069,6 +1069,16 @@ inline void requestNBDVirtualMediaRoutes(App& app) + return true; + } + ++ if (!actionParams.userName) ++ { ++ actionParams.userName = ""; ++ } ++ ++ if (!actionParams.password) ++ { ++ actionParams.password = ""; ++ } ++ + // manager is irrelevant for + // VirtualMedia dbus calls + doMountVmLegacy(asyncResp, service, resName, +-- +2.17.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb_%.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb_%.bbappend index d5e7a6dd2..e3bed9eb0 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb_%.bbappend +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/interfaces/bmcweb_%.bbappend @@ -55,6 +55,7 @@ SRC_URI += " \ file://vm/0006-Bmcweb-handle-permission-denied-exception.patch \ file://vm/0007-Fix-unmounting-image-in-proxy-mode.patch \ file://vm/0008-Return-404-for-POST-on-Proxy-InsertMedia.patch \ + file://vm/0009-virtual_media-Fix-for-bmcweb-crash.patch \ " # EventService: Temporary pulled to downstream. See eventservice\README for details diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/pmci/mctpd.bb b/meta-openbmc-mods/meta-common/recipes-phosphor/pmci/mctpd.bb index bdc85913f..5ddca8730 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/pmci/mctpd.bb +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/pmci/mctpd.bb @@ -5,7 +5,7 @@ LICENSE = "Apache-2.0" LIC_FILES_CHKSUM = "file://LICENSE;md5=e3fc50a88d0a364313df4b21ef20c29e" SRC_URI = "git://git@github.com/Intel-BMC/mctpd.git;protocol=ssh;branch=1-release" -SRCREV = "4aa697fba21a2d0d0770358a8c9493bf5d8d5741" +SRCREV = "1741334eb0972de547d053e97d0a5b93892b070e" S = "${WORKDIR}/git" diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors/0017-psusensor-Determine-PSU-threshold-dynamically.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors/0017-psusensor-Determine-PSU-threshold-dynamically.patch new file mode 100644 index 000000000..1823e96a5 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors/0017-psusensor-Determine-PSU-threshold-dynamically.patch @@ -0,0 +1,160 @@ +From 654fef4afba55c4f84f664b43d8a1fecc1224e7b Mon Sep 17 00:00:00 2001 +From: Vikash Chandola <vikash.chandola@intel.com> +Date: Wed, 27 Jul 2022 10:07:25 +0000 +Subject: [PATCH] psusensor: Determine PSU threshold dynamically + +PSU output current range(rated, max etc) are dependent on input +voltage. A power supply can deliver higher output on high line input +compared to low line input. Existing implementation uses static +thresholds which causes thresholds to get hit too early for high line +input. This change allows threshold to be set dynamically. + +"ThresholdLabel" and "ScaleFactor" are added in SOLUM's entity-manager +configuration file. These parameters provide pmbus register name and +scaling factor to be used for threshold calculation. psusensor fetches +"ThresholdLabel" and multiplies it with "ScaleFactor" to determine +threshold value. "ScaleFactor" allows threshold to be set in ratio to +a value. Example create a threshold that gets triggered at 0.8 times +max permissible current value. + +"ThresholdLabel" and "ScaleFactor" can be used for the cases where +threshold need to be determined on the basis on another parameter's +value. + +Tested: +Tested by running system on High line and low line input. In both +cases critical and warning thresholds were updated as per +"ThresholdLabel" and "ScaleFactor". If configuration file doesn't +contain "ThresholdLabel" and "ScaleFactor" then "Value" was used. + +Signed-off-by: Vikash Chandola <vikash.chandola@intel.com> +Change-Id: Ia588ee0971e3d4e79a9827ffd6f39398725a4273 +--- + include/Thresholds.hpp | 3 +- + src/PSUSensorMain.cpp | 2 +- + src/Thresholds.cpp | 64 +++++++++++++++++++++++++++++++++++++----- + 3 files changed, 60 insertions(+), 9 deletions(-) + +diff --git a/include/Thresholds.hpp b/include/Thresholds.hpp +index 640fdb4..7373507 100644 +--- a/include/Thresholds.hpp ++++ b/include/Thresholds.hpp +@@ -111,7 +111,8 @@ struct ThresholdTimer + bool parseThresholdsFromConfig( + const SensorData& sensorData, + std::vector<thresholds::Threshold>& thresholdVector, +- const std::string* matchLabel = nullptr, const int* sensorIndex = nullptr); ++ const std::string* matchLabel = nullptr, const int* sensorIndex = nullptr, ++ const std::string* sensorPathStr = nullptr); + + bool parseThresholdsFromAttr(std::vector<thresholds::Threshold>& thresholds, + const std::string& inputPath, +diff --git a/src/PSUSensorMain.cpp b/src/PSUSensorMain.cpp +index ae7afc3..5847863 100644 +--- a/src/PSUSensorMain.cpp ++++ b/src/PSUSensorMain.cpp +@@ -846,7 +846,7 @@ static void createSensorsCallback( + + std::vector<thresholds::Threshold> sensorThresholds; + if (!parseThresholdsFromConfig(*sensorData, sensorThresholds, +- &labelHead)) ++ &labelHead ,nullptr, &sensorPathStr)) + { + std::cerr << "error populating thresholds for " + << sensorNameSubStr << "\n"; +diff --git a/src/Thresholds.cpp b/src/Thresholds.cpp +index aef084e..cddfa2b 100644 +--- a/src/Thresholds.cpp ++++ b/src/Thresholds.cpp +@@ -56,11 +56,48 @@ std::string toBusValue(const Direction& direction) + } + } + } ++static const std::unordered_map<std::string, std::string> labelToHwmonSuffix = { ++ {"iout_oc_warn_limit", "max"}, ++}; ++ ++static std::optional<double> ++ parseThresholdFromLabel(const std::string* sensorPathStr, ++ const SensorBaseConfigMap& sensorData) ++{ ++ if (sensorPathStr == nullptr) ++ { ++ return std::nullopt; ++ } ++ auto thresholdLabelFind = sensorData.find("ThresholdLabel"); ++ auto scaleFactorFind = sensorData.find("ScaleFactor"); ++ if (thresholdLabelFind == sensorData.end() || ++ scaleFactorFind == sensorData.end()) ++ { ++ return std::nullopt; ++ } ++ auto hwmonFileSuffix = labelToHwmonSuffix.find( ++ std::visit(VariantToStringVisitor(), thresholdLabelFind->second)); ++ if (hwmonFileSuffix == labelToHwmonSuffix.end()) ++ { ++ return std::nullopt; ++ } ++ auto fileParts = splitFileName(*sensorPathStr); ++ if (!fileParts.has_value()) ++ { ++ return std::nullopt; ++ } ++ auto& [type, nr, item] = fileParts.value(); ++ auto attrPath = ++ boost::replace_all_copy(*sensorPathStr, item, hwmonFileSuffix->second); ++ return readFile(attrPath, (1.0 / std::visit(VariantToDoubleVisitor(), ++ scaleFactorFind->second))); ++} + + bool parseThresholdsFromConfig( + const SensorData& sensorData, + std::vector<thresholds::Threshold>& thresholdVector, +- const std::string* matchLabel, const int* sensorIndex) ++ const std::string* matchLabel, const int* sensorIndex, ++ const std::string* sensorPathStr) + { + for (const auto& item : sensorData) + { +@@ -110,10 +147,7 @@ bool parseThresholdsFromConfig( + + auto directionFind = item.second.find("Direction"); + auto severityFind = item.second.find("Severity"); +- auto valueFind = item.second.find("Value"); +- if (valueFind == item.second.end() || +- severityFind == item.second.end() || +- directionFind == item.second.end()) ++ if (severityFind == item.second.end() || directionFind == item.second.end()) + { + std::cerr << "Malformed threshold on configuration interface " + << item.first << "\n"; +@@ -139,8 +173,24 @@ bool parseThresholdsFromConfig( + { + direction = Direction::HIGH; + } +- double val = std::visit(VariantToDoubleVisitor(), valueFind->second); +- ++ double val = 0; ++ auto valueFind = item.second.find("Value"); ++ auto labelValueOption = parseThresholdFromLabel(sensorPathStr, item.second); ++ if (labelValueOption.has_value()) ++ { ++ val = labelValueOption.value(); ++ } ++ else if (valueFind != item.second.end()) ++ { ++ val = std::visit(VariantToDoubleVisitor(), valueFind->second); ++ } ++ else ++ { ++ std::cerr << "Failed to parse threshold value configuration for " ++ "interface " ++ << item.first << "\n"; ++ return false; ++ } + thresholdVector.emplace_back(level, direction, val, hysteresis); + } + return true; +-- +2.25.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors_%.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors_%.bbappend index a9d4a7627..7e3356e9a 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors_%.bbappend +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors_%.bbappend @@ -22,6 +22,7 @@ SRC_URI += "\ file://0014-Treat-zero-temperatures-readings-as-errors-in-IpmbSe.patch \ file://0015-Fix-for-PSU2-Power-lost-RedFish-events.patch \ file://0016-Ignore-VR-sensor-readings-if-content-is-0xFF.patch \ + file://0017-psusensor-Determine-PSU-threshold-dynamically.patch \ " DEPENDS:append = " libgpiod libmctp" diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/users/phosphor-user-manager/0001-Change-to-pam_faillock-and-pam-pwquality.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/users/phosphor-user-manager/0001-Change-to-pam_faillock-and-pam-pwquality.patch new file mode 100644 index 000000000..e050d9493 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/users/phosphor-user-manager/0001-Change-to-pam_faillock-and-pam-pwquality.patch @@ -0,0 +1,492 @@ +From 0636e9177c718cd46023c454fb36825e1d000a4f Mon Sep 17 00:00:00 2001 +From: "Jason M. Bills" <jason.m.bills@intel.com> +Date: Tue, 28 Mar 2023 15:32:45 -0700 +Subject: [PATCH] Change to pam_faillock and pam pwquality + +pam_tally2 is being replaced by pam_faillock. The parameters in +common-auth have moved to faillock.conf, so this commit adds a new +method to modify paramters in a given configuration file. + +The output from the 'faillock' command differs from 'pam_tally2', so +this commit adds a new function to parse the output from 'faillock' to +determine if the user is currently locked. + +pam_cracklib is being replaced by pam_pwquality. The parameters in +common-password have moved to pwquality.conf. + +I referenced the work done by Joseph Reynolds in this commit [1] to know +what changes were required. + +[1]: https://gerrit.openbmc.org/c/openbmc/phosphor-user-manager/+/39853 + +Tested: +Confirmed that the AccountLockoutDuration and AccountLockoutThreshold +parameters under /redfish/v1/AccountService both return the correct +value from common-auth. + +Set deny to 10 and unlock_time to 30 seconds and confirmed that a user +account will correctly show as locked after 10 failed login attempts, +and that user will show as unlocked 30 seconds later. + +Used Redfish to PATCH both AccountLockoutDuration and +AccountLockoutThreshold and confirmed that the updated values are +correctly reported in Redfish and that the correct lines in +faillock.conf are modified. + +Confirmed that the MinPasswordLength parameter under +/redfish/v1/AccountService returns the correct value from +common-password. + +Set minlen to 9 and confirmed that a user password could not be set with +a length of 8. + +Used Redfish to PATCH MinPasswordLength and confirmed that the updated +value is correctly reported in Redfish and that the correct line in +pwquality.conf is modified. + +Change-Id: I0701e4148c0b8333c6b8889d4695e61ce7f5366d +Signed-off-by: Jason M. Bills <jason.m.bills@intel.com> +--- + user_mgr.cpp | 257 +++++++++++++++++++++++++++++++++++++-------------- + user_mgr.hpp | 37 ++++++++ + 2 files changed, 224 insertions(+), 70 deletions(-) + +diff --git a/user_mgr.cpp b/user_mgr.cpp +index c49fbef..68ca0ff 100644 +--- a/user_mgr.cpp ++++ b/user_mgr.cpp +@@ -51,15 +51,15 @@ static constexpr int success = 0; + static constexpr int failure = -1; + + // pam modules related +-static constexpr const char* pamTally2 = "pam_tally2.so"; +-static constexpr const char* pamCrackLib = "pam_cracklib.so"; + static constexpr const char* pamPWHistory = "pam_pwhistory.so"; + static constexpr const char* minPasswdLenProp = "minlen"; + static constexpr const char* remOldPasswdCount = "remember"; + static constexpr const char* maxFailedAttempt = "deny"; + static constexpr const char* unlockTimeout = "unlock_time"; + static constexpr const char* pamPasswdConfigFile = "/etc/pam.d/common-password"; +-static constexpr const char* pamAuthConfigFile = "/etc/pam.d/common-auth"; ++static constexpr const char* faillockConfigFile = "/etc/security/faillock.conf"; ++static constexpr const char* pwQualityConfigFile = ++ "/etc/security/pwquality.conf"; + + // Object Manager related + static constexpr const char* ldapMgrObjBasePath = +@@ -320,8 +320,8 @@ uint8_t UserMgr::minPasswordLength(uint8_t value) + { + return value; + } +- if (setPamModuleArgValue(pamCrackLib, minPasswdLenProp, +- std::to_string(value)) != success) ++ if (setPamModuleConfValue(pwQualityConfigFile, minPasswdLenProp, ++ std::to_string(value)) != success) + { + log<level::ERR>("Unable to set minPasswordLength"); + elog<InternalFailure>(); +@@ -350,8 +350,8 @@ uint16_t UserMgr::maxLoginAttemptBeforeLockout(uint16_t value) + { + return value; + } +- if (setPamModuleArgValue(pamTally2, maxFailedAttempt, +- std::to_string(value)) != success) ++ if (setPamModuleConfValue(faillockConfigFile, maxFailedAttempt, ++ std::to_string(value)) != success) + { + log<level::ERR>("Unable to set maxLoginAttemptBeforeLockout"); + elog<InternalFailure>(); +@@ -365,8 +365,8 @@ uint32_t UserMgr::accountUnlockTimeout(uint32_t value) + { + return value; + } +- if (setPamModuleArgValue(pamTally2, unlockTimeout, std::to_string(value)) != +- success) ++ if (setPamModuleConfValue(faillockConfigFile, unlockTimeout, ++ std::to_string(value)) != success) + { + log<level::ERR>("Unable to set accountUnlockTimeout"); + elog<InternalFailure>(); +@@ -378,15 +378,7 @@ int UserMgr::getPamModuleArgValue(const std::string& moduleName, + const std::string& argName, + std::string& argValue) + { +- std::string fileName; +- if (moduleName == pamTally2) +- { +- fileName = pamAuthConfigFile; +- } +- else +- { +- fileName = pamPasswdConfigFile; +- } ++ std::string fileName = pamPasswdConfigFile; + std::ifstream fileToRead(fileName, std::ios::in); + if (!fileToRead.is_open()) + { +@@ -427,19 +419,52 @@ int UserMgr::getPamModuleArgValue(const std::string& moduleName, + return failure; + } + +-int UserMgr::setPamModuleArgValue(const std::string& moduleName, +- const std::string& argName, +- const std::string& argValue) ++int UserMgr::getPamModuleConfValue(const std::string& confFile, ++ const std::string& argName, ++ std::string& argValue) + { +- std::string fileName; +- if (moduleName == pamTally2) ++ std::ifstream fileToRead(confFile, std::ios::in); ++ if (!fileToRead.is_open()) + { +- fileName = pamAuthConfigFile; ++ log<level::ERR>("Failed to open pam configuration file", ++ entry("FILE_NAME=%s", confFile.c_str())); ++ return failure; + } +- else ++ std::string line; ++ auto argSearch = argName + "="; ++ size_t startPos = 0; ++ size_t endPos = 0; ++ while (getline(fileToRead, line)) + { +- fileName = pamPasswdConfigFile; ++ // skip comments section starting with # ++ if ((startPos = line.find('#')) != std::string::npos) ++ { ++ if (startPos == 0) ++ { ++ continue; ++ } ++ // skip comments after meaningful section and process those ++ line = line.substr(0, startPos); ++ } ++ if ((startPos = line.find(argSearch)) != std::string::npos) ++ { ++ if ((endPos = line.find(' ', startPos)) == std::string::npos) ++ { ++ endPos = line.size(); ++ } ++ startPos += argSearch.size(); ++ argValue = line.substr(startPos, endPos - startPos); ++ return success; ++ } + } ++ return failure; ++} ++ ++int UserMgr::setPamModuleArgValue(const std::string& moduleName, ++ const std::string& argName, ++ const std::string& argValue) ++{ ++ std::string fileName = pamPasswdConfigFile; + std::string tmpFileName = fileName + "_tmp"; + std::ifstream fileToRead(fileName, std::ios::in); + std::ofstream fileToWrite(tmpFileName, std::ios::out); +@@ -497,6 +522,64 @@ int UserMgr::setPamModuleArgValue(const std::string& moduleName, + return failure; + } + ++int UserMgr::setPamModuleConfValue(const std::string& confFile, ++ const std::string& argName, ++ const std::string& argValue) ++{ ++ std::string tmpConfFile = confFile + "_tmp"; ++ std::ifstream fileToRead(confFile, std::ios::in); ++ std::ofstream fileToWrite(tmpConfFile, std::ios::out); ++ if (!fileToRead.is_open() || !fileToWrite.is_open()) ++ { ++ log<level::ERR>("Failed to open pam configuration /tmp file", ++ entry("FILE_NAME=%s", confFile.c_str())); ++ return failure; ++ } ++ std::string line; ++ auto argSearch = argName + "="; ++ size_t startPos = 0; ++ size_t endPos = 0; ++ bool found = false; ++ while (getline(fileToRead, line)) ++ { ++ // skip comments section starting with # ++ if ((startPos = line.find('#')) != std::string::npos) ++ { ++ if (startPos == 0) ++ { ++ fileToWrite << line << std::endl; ++ continue; ++ } ++ // skip comments after meaningful section and process those ++ line = line.substr(0, startPos); ++ } ++ if ((startPos = line.find(argSearch)) != std::string::npos) ++ { ++ if ((endPos = line.find(' ', startPos)) == std::string::npos) ++ { ++ endPos = line.size(); ++ } ++ startPos += argSearch.size(); ++ fileToWrite << line.substr(0, startPos) << argValue ++ << line.substr(endPos, line.size() - endPos) ++ << std::endl; ++ found = true; ++ continue; ++ } ++ fileToWrite << line << std::endl; ++ } ++ fileToWrite.close(); ++ fileToRead.close(); ++ if (found) ++ { ++ if (std::rename(tmpConfFile.c_str(), confFile.c_str()) == 0) ++ { ++ return success; ++ } ++ } ++ return failure; ++} ++ + void UserMgr::userEnable(const std::string& userName, bool enabled) + { + throwForUserDoesNotExist(userName); +@@ -510,51 +593,87 @@ void UserMgr::userEnable(const std::string& userName, bool enabled) + } + + /** +- * pam_tally2 app will provide the user failure count and failure status +- * in second line of output with words position [0] - user name, +- * [1] - failure count, [2] - latest timestamp, [3] - failure timestamp +- * [4] - failure app ++ * faillock app will provide the user failed login list with when the attempt ++ * was made, the type, the source, and if it's valid. ++ * ++ * Valid in this case means that the attempt was made within the fail_interval ++ * time. So, we can check this list for the number of valid entries (lines ++ * ending with 'V') compared to the maximum allowed to determine if the user is ++ * locked out. ++ * ++ * This data is only refreshed when an attempt is made, so if the user appears ++ * to be locked out, we must also check if the most recent attempt was older ++ * than the unlock_time to know if the user has since been unlocked. + **/ +- +-static constexpr size_t t2UserIdx = 0; +-static constexpr size_t t2FailCntIdx = 1; +-static constexpr size_t t2OutputIndex = 1; +- +-bool UserMgr::userLockedForFailedAttempt(const std::string& userName) ++bool UserMgr::parseFaillockForLockout( ++ const std::vector<std::string>& faillockOutput) + { +- // All user management lock has to be based on /etc/shadow +- // TODO phosphor-user-manager#10 phosphor::user::shadow::Lock lock{}; +- std::vector<std::string> output; +- +- output = executeCmd("/usr/sbin/pam_tally2", "-u", userName.c_str()); ++ uint16_t failAttempts = 0; ++ time_t lastFailedAttempt{}; ++ for (const std::string& line : faillockOutput) ++ { ++ if (!boost::ends_with(line, "V")) ++ { ++ continue; ++ } + +- std::vector<std::string> splitWords; +- boost::algorithm::split(splitWords, output[t2OutputIndex], +- boost::algorithm::is_any_of("\t "), +- boost::token_compress_on); ++ // Count this failed attempt ++ failAttempts++; + +- try +- { +- unsigned long tmp = std::stoul(splitWords[t2FailCntIdx], nullptr); +- uint16_t value16 = 0; +- if (tmp > std::numeric_limits<decltype(value16)>::max()) ++ // Update the last attempt time ++ // First get the "when" which is the first two words (date and time) ++ size_t pos = line.find(" "); ++ if (pos == std::string::npos) ++ { ++ continue; ++ } ++ pos = line.find(" ", pos + 1); ++ if (pos == std::string::npos) + { +- throw std::out_of_range("Out of range"); ++ continue; + } +- value16 = static_cast<decltype(value16)>(tmp); +- if (AccountPolicyIface::maxLoginAttemptBeforeLockout() != 0 && +- value16 >= AccountPolicyIface::maxLoginAttemptBeforeLockout()) ++ std::string failDateTime = line.substr(0, pos); ++ ++ // NOTE: Cannot use std::get_time() here as the implementation of %y in ++ // libstdc++ does not match POSIX strptime() before gcc 12.1.0 ++ // https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=a8d3c98746098e2784be7144c1ccc9fcc34a0888 ++ std::tm tmStruct = {}; ++ if (!strptime(failDateTime.c_str(), "%F %T", &tmStruct)) + { +- return true; // User account is locked out ++ // log<level::ERR>("Failed to parse latest failure date/time"); ++ // elog<InternalFailure>(); + } +- return false; // User account is un-locked ++ ++ time_t failTimestamp = std::mktime(&tmStruct); ++ lastFailedAttempt = std::max(failTimestamp, lastFailedAttempt); ++ } ++ ++ if (failAttempts < AccountPolicyIface::maxLoginAttemptBeforeLockout()) ++ { ++ return false; + } +- catch (const std::exception& e) ++ ++ if (lastFailedAttempt + ++ static_cast<time_t>(AccountPolicyIface::accountUnlockTimeout()) <= ++ std::time(NULL)) + { +- log<level::ERR>("Exception for userLockedForFailedAttempt", +- entry("WHAT=%s", e.what())); +- throw; ++ return false; + } ++ ++ return true; ++} ++ ++bool UserMgr::userLockedForFailedAttempt(const std::string& userName) ++{ ++ // All user management lock has to be based on /etc/shadow ++ // TODO phosphor-user-manager#10 phosphor::user::shadow::Lock lock{}; ++ ++ std::vector<std::string> output; ++ ++ output = ++ executeCmd("/usr/sbin/faillock", "--user", userName.c_str(), "--reset"); ++ ++ return parseFaillockForLockout(output); + } + + bool UserMgr::userLockedForFailedAttempt(const std::string& userName, +@@ -567,12 +686,8 @@ bool UserMgr::userLockedForFailedAttempt(const std::string& userName, + { + return userLockedForFailedAttempt(userName); + } +- output = executeCmd("/usr/sbin/pam_tally2", "-u", userName.c_str(), "-r"); +- +- std::vector<std::string> splitWords; +- boost::algorithm::split(splitWords, output[t2OutputIndex], +- boost::algorithm::is_any_of("\t "), +- boost::token_compress_on); ++ output = ++ executeCmd("/usr/sbin/faillock", "--user", userName.c_str(), "--reset"); + + return userLockedForFailedAttempt(userName); + } +@@ -940,8 +1055,8 @@ UserMgr::UserMgr(sdbusplus::bus::bus& bus, const char* path, + std::string valueStr; + auto value = minPasswdLength; + unsigned long tmp = 0; +- if (getPamModuleArgValue(pamCrackLib, minPasswdLenProp, valueStr) != +- success) ++ if (getPamModuleConfValue(pwQualityConfigFile, minPasswdLenProp, ++ valueStr) != success) + { + AccountPolicyIface::minPasswordLength(minPasswdLength); + } +@@ -991,7 +1106,8 @@ UserMgr::UserMgr(sdbusplus::bus::bus& bus, const char* path, + AccountPolicyIface::rememberOldPasswordTimes(value); + } + valueStr.clear(); +- if (getPamModuleArgValue(pamTally2, maxFailedAttempt, valueStr) != success) ++ if (getPamModuleConfValue(faillockConfigFile, maxFailedAttempt, valueStr) != ++ success) + { + AccountPolicyIface::maxLoginAttemptBeforeLockout(0); + } +@@ -1016,7 +1132,8 @@ UserMgr::UserMgr(sdbusplus::bus::bus& bus, const char* path, + AccountPolicyIface::maxLoginAttemptBeforeLockout(value16); + } + valueStr.clear(); +- if (getPamModuleArgValue(pamTally2, unlockTimeout, valueStr) != success) ++ if (getPamModuleConfValue(faillockConfigFile, unlockTimeout, valueStr) != ++ success) + { + AccountPolicyIface::accountUnlockTimeout(0); + } +diff --git a/user_mgr.hpp b/user_mgr.hpp +index 5d5ca99..92a265b 100644 +--- a/user_mgr.hpp ++++ b/user_mgr.hpp +@@ -155,6 +155,14 @@ class UserMgr : public Ifaces + */ + uint32_t accountUnlockTimeout(uint32_t val) override; + ++ /** @brief parses the faillock output for locked user status ++ * ++ * @param[in] - output from faillock for the user ++ * @return - true / false indicating user locked / un-locked ++ **/ ++ bool ++ parseFaillockForLockout(const std::vector<std::string>& faillockOutput); ++ + /** @brief lists user locked state for failed attempt + * + * @param[in] - user name +@@ -311,6 +319,20 @@ class UserMgr : public Ifaces + int getPamModuleArgValue(const std::string& moduleName, + const std::string& argName, std::string& argValue); + ++ /** @brief get pam argument value ++ * method to get argument value from pam configuration ++ * ++ * @param[in] confFile - path of the module config file from where arg has ++ * to be read ++ * @param[in] argName - argument name ++ * @param[out] argValue - argument value ++ * ++ * @return 0 - success state of the function ++ */ ++ int getPamModuleConfValue(const std::string& confFile, ++ const std::string& argName, ++ std::string& argValue); ++ + /** @brief set pam argument value + * method to set argument value in pam configuration + * +@@ -325,6 +347,20 @@ class UserMgr : public Ifaces + const std::string& argName, + const std::string& argValue); + ++ /** @brief set pam argument value ++ * method to set argument value in pam configuration ++ * ++ * @param[in] confFile - path of the module config file in which argument ++ * value has to be set ++ * @param[in] argName - argument name ++ * @param[out] argValue - argument value ++ * ++ * @return 0 - success state of the function ++ */ ++ int setPamModuleConfValue(const std::string& confFile, ++ const std::string& argName, ++ const std::string& argValue); ++ + /** @brief get service name + * method to get dbus service name + * +@@ -351,6 +387,7 @@ class UserMgr : public Ifaces + virtual DbusUserObj getPrivilegeMapperObject(void); + + friend class TestUserMgr; ++ + }; + + } // namespace user +-- +2.25.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/users/phosphor-user-manager_%.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/users/phosphor-user-manager_%.bbappend index 0bf528eb4..88a2b0f2d 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/users/phosphor-user-manager_%.bbappend +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/users/phosphor-user-manager_%.bbappend @@ -6,6 +6,7 @@ SRCREV = "b01e2fe760eb04ae9d0d13716a127056949e2601" EXTRA_OECONF += "${@bb.utils.contains_any("IMAGE_FEATURES", [ 'debug-tweaks', 'allow-root-login' ], '', '--disable-root_user_mgmt', d)}" SRC_URI += " \ + file://0001-Change-to-pam_faillock-and-pam-pwquality.patch \ file://0005-Added-suport-for-multiple-user-manager-services.patch \ file://0006-Use-groupmems-instead-of-getgrnam_r-due-to-overlay.patch \ " diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/webui/webui-vue/0001-Old-password-input-in-change-password-screen.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/webui/webui-vue/0001-Old-password-input-in-change-password-screen.patch new file mode 100644 index 000000000..313ba9387 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/webui/webui-vue/0001-Old-password-input-in-change-password-screen.patch @@ -0,0 +1,135 @@ +From 9da7bafdcee1bd022b7e47eecf704eb799b389e8 Mon Sep 17 00:00:00 2001 +From: Yaswanth Reddy M <yaswanthx.reddy.munukuru@intel.com> +Date: Wed, 17 May 2023 10:47:56 +0000 +Subject: [PATCH] Old password input in change password screen + +When the user changed their password in profile settings, to prevent +XSS attacks, I added the current password input field to authenticate +the user. + +Once the authentication had success with the current password, then +allowing the update was possible. After the password is changed +successfully, all the sessions of the user who changed the password +will be disconnected, including the current session. and the current +session will navigate to the login page. + +Signed-off-by: Yaswanth Reddy M <yaswanthx.reddy.munukuru@intel.com> +--- + src/locales/en-US.json | 4 +- + src/views/ProfileSettings/ProfileSettings.vue | 54 +++++++++++++++++-- + 2 files changed, 53 insertions(+), 5 deletions(-) + +diff --git a/src/locales/en-US.json b/src/locales/en-US.json +index 637f052..8d98abb 100644 +--- a/src/locales/en-US.json ++++ b/src/locales/en-US.json +@@ -617,6 +617,7 @@ + "confirmPassword": "Confirm new password", + "defaultUTC": "Default (UTC)", + "newPassword": "New password", ++ "currentPassword": "Current password", + "newPassLabelTextInfo": "Password must be between %{min} - %{max} characters", + "passwordsDoNotMatch": "Passwords do not match", + "profileInfoTitle": "Profile information", +@@ -625,7 +626,8 @@ + "timezoneDisplayDesc": "Select how time is displayed throughout the application", + "username": "Username", + "toast": { +- "successSaveSettings": "Successfully saved account settings." ++ "successSaveSettings": "Successfully saved account settings.", ++ "wrongCredentials": "Wrong credentials" + } + }, + "pageNetwork": { +diff --git a/src/views/ProfileSettings/ProfileSettings.vue b/src/views/ProfileSettings/ProfileSettings.vue +index 35fc800..330fd4a 100644 +--- a/src/views/ProfileSettings/ProfileSettings.vue ++++ b/src/views/ProfileSettings/ProfileSettings.vue +@@ -23,6 +23,21 @@ + <page-section + :section-title="$t('pageProfileSettings.changePassword')" + > ++ <b-form-group ++ id="input-group-0" ++ :label="$t('pageProfileSettings.currentPassword')" ++ label-for="input-0" ++ > ++ <input-password-toggle> ++ <b-form-input ++ id="old-password" ++ v-model="form.currentPassword" ++ type="password" ++ data-test-id="profileSettings-input-ocurrentPassword" ++ class="form-control-with-button" ++ /> ++ </input-password-toggle> ++ </b-form-group> + <b-form-group + id="input-group-1" + :label="$t('pageProfileSettings.newPassword')" +@@ -151,6 +166,7 @@ export default { + form: { + newPassword: '', + confirmPassword: '', ++ currentPassword: '', + isUtcDisplay: this.$store.getters['global/isUtcDisplay'], + }, + }; +@@ -198,9 +214,12 @@ export default { + this.$store + .dispatch('userManagement/updateUser', userData) + .then((message) => { +- (this.form.newPassword = ''), (this.form.confirmPassword = ''); ++ (this.form.newPassword = ''), ++ (this.form.confirmPassword = ''), ++ (this.form.currentPassword = ''); + this.$v.$reset(); + this.successToast(message); ++ this.$store.dispatch('authentication/logout'); + }) + .catch(({ message }) => this.errorToast(message)); + }, +@@ -212,10 +231,37 @@ export default { + ); + }, + submitForm() { +- if (this.form.confirmPassword || this.form.newPassword) { +- this.saveNewPasswordInputData(); ++ if ( ++ this.form.confirmPassword && ++ this.form.newPassword && ++ this.form.currentPassword ++ ) { ++ this.confirmAuthenticate(); + } +- this.saveTimeZonePrefrenceData(); ++ if ( ++ this.$store.getters['global/isUtcDisplay'] != this.form.isUtcDisplay ++ ) { ++ this.saveTimeZonePrefrenceData(); ++ } ++ }, ++ confirmAuthenticate() { ++ this.$v.form.newPassword.$touch(); ++ if (this.$v.$invalid) return; ++ ++ const username = this.username; ++ const password = this.form.currentPassword; ++ ++ this.$store ++ .dispatch('authentication/login', { username, password }) ++ .then(() => { ++ this.saveNewPasswordInputData(); ++ }) ++ .catch(() => { ++ this.$v.$reset(); ++ this.errorToast( ++ this.$t('pageProfileSettings.toast.wrongCredentials') ++ ); ++ }); + }, + }, + }; +-- +2.25.1 + diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/webui/webui-vue_%.bbappend b/meta-openbmc-mods/meta-common/recipes-phosphor/webui/webui-vue_%.bbappend index bdd6a3bad..412e88501 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/webui/webui-vue_%.bbappend +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/webui/webui-vue_%.bbappend @@ -6,6 +6,7 @@ FILESEXTRAPATHS:append := "${THISDIR}/${PN}:" SRC_URI += " \ file://login-company-logo.svg \ file://logo-header.svg \ + file://0001-Old-password-input-in-change-password-screen.patch \ " do_compile:prepend() { |