commit 31c45f60d65117891d41cb5b1caeb02a7e44f0ef
author Arun P. Mohanan <arun.p.m@linux.intel.com> Mon Oct 18 21:52:39 2021 +0530
committer Arun P. Mohanan <arun.p.m@linux.intel.com> Tue Oct 19 08:23:18 2021 +0530
tree 331ab9b9da557362d56a5d13cbc2c04f6ac9d8ba
parent d4b74b4f7ccc92152eabf7eebb5f1722889a73c9

Separately handle RestrictionMode & POSTComplete

Having single try catch to get 2 separate D-Bus properties from
different services resulted in incorrect value for 2nd property.
In the previous implementation, when `RestrictionModeService` is
not initialized it will throw and return. In this case it was
missing to read the `OperatingSystemState` property and it was
defaulting postComplete as true but actually it can be false.

The above case resulted in a a situation where BMC blocked the
IPMI commands(Over KCS trust policy- DenyAll & Whitelist ) over
KCS channel even before BIOS completes booting. As a result BIOS
never get response for IPMI commands like getDeviceID(In DenyAll mode)
and caused further issues.

Separate out these into seperate try catch to avoid above issue.
Also updated the function to use getDbusProperty() API.

Tested: Verified postComplete value is getting cached correctly even
if the restrictionModeService is not up and running.

Signed-off-by: Arun P. Mohanan <arun.p.m@linux.intel.com>
Change-Id: I394a7c46703e917b0075da6cc5469de1b66a9b7a

src/whitelist-filter.cpp

diff --git a/src/whitelist-filter.cpp b/src/whitelist-filter.cpp
index c7e4086..0b78578 100644
--- a/src/whitelist-filter.cpp
+++ b/src/whitelist-filter.cpp
@@ -73,6 +73,10 @@
         "xyz.openbmc_project.State.OperatingSystem.Status";
     static constexpr const char* hostMiscIntf =
         "xyz.openbmc_project.State.Host.Misc";
+    static constexpr const char* restrictionModePath =
+        "/xyz/openbmc_project/control/security/restriction_mode";
+    static constexpr const char* systemOsStatusPath =
+        "/xyz/openbmc_project/state/os";
 };
 
 static inline uint8_t getSMMChannel()
@@ -117,75 +121,42 @@
 
 void WhitelistFilter::cacheRestrictedAndPostCompleteMode()
 {
-    std::string restrictionModePath;
-    std::string restrictionModeService;
-    std::string systemOsStatusPath;
-    std::string systemOsStatusService;
     try
     {
-        ipmi::DbusObjectInfo restrictionObj =
-            ipmi::getDbusObject(*bus, restrictionModeIntf);
-
-        restrictionModePath = restrictionObj.first;
-        restrictionModeService = restrictionObj.second;
-
-        ipmi::DbusObjectInfo postCompleteObj =
-            ipmi::getDbusObject(*bus, systemOsStatusIntf);
-
-        systemOsStatusPath = postCompleteObj.first;
-        systemOsStatusService = postCompleteObj.second;
+        auto service =
+            ipmi::getService(*bus, restrictionModeIntf, restrictionModePath);
+        ipmi::Value v =
+            ipmi::getDbusProperty(*bus, service, restrictionModePath,
+                                  restrictionModeIntf, "RestrictionMode");
+        auto& mode = std::get<std::string>(v);
+        restrictionMode = RestrictionMode::convertModesFromString(mode);
+        log<level::INFO>("Read restriction mode",
+                         entry("VALUE=%d", static_cast<int>(restrictionMode)));
     }
     catch (const std::exception&)
     {
-        log<level::ERR>(
-            "Could not initialize provisioning mode, defaulting to restricted",
-            entry("VALUE=%d", static_cast<int>(restrictionMode)));
-        return;
+        log<level::ERR>("Could not initialize provisioning mode, "
+                        "defaulting to restricted",
+                        entry("VALUE=%d", static_cast<int>(restrictionMode)));
     }
 
-    bus->async_method_call(
-        [this](boost::system::error_code ec, ipmi::Value v) {
-            if (ec)
-            {
-                log<level::ERR>(
-                    "Could not initialize provisioning mode, "
-                    "defaulting to restricted",
-                    entry("VALUE=%d", static_cast<int>(restrictionMode)));
-                return;
-            }
-            auto mode = std::get<std::string>(v);
-            restrictionMode = RestrictionMode::convertModesFromString(mode);
-            log<level::INFO>(
-                "Read restriction mode",
-                entry("VALUE=%d", static_cast<int>(restrictionMode)));
-        },
-        restrictionModeService, restrictionModePath,
-        "org.freedesktop.DBus.Properties", "Get", restrictionModeIntf,
-        "RestrictionMode");
-
-    bus->async_method_call(
-        [this](boost::system::error_code ec, const ipmi::Value& v) {
-            if (ec)
-            {
-                log<level::ERR>("Error in OperatingSystemState Get");
-                postCompleted = true;
-                return;
-            }
-            auto value = std::get<std::string>(v);
-            if (value == "Standby")
-            {
-                postCompleted = true;
-            }
-            else
-            {
-                postCompleted = false;
-            }
-            log<level::INFO>("Read POST complete value",
-                             entry("VALUE=%d", postCompleted));
-        },
-        systemOsStatusService, systemOsStatusPath,
-        "org.freedesktop.DBus.Properties", "Get", systemOsStatusIntf,
-        "OperatingSystemState");
+    try
+    {
+        auto service =
+            ipmi::getService(*bus, systemOsStatusIntf, systemOsStatusPath);
+        ipmi::Value v =
+            ipmi::getDbusProperty(*bus, service, systemOsStatusPath,
+                                  systemOsStatusIntf, "OperatingSystemState");
+        auto& value = std::get<std::string>(v);
+        postCompleted = (value == "Standby");
+        log<level::INFO>("Read POST complete value",
+                         entry("VALUE=%d", postCompleted));
+    }
+    catch (const std::exception&)
+    {
+        log<level::ERR>("Error in OperatingSystemState Get");
+        postCompleted = true;
+    }
 }
 
 void WhitelistFilter::updateRestrictionMode(const std::string& value)