DELIA-57822: JSON-RPC misuse

Reason for change: * MM checkNetwork() checks org.rdk.Network active before invoking org.rdk.Network.isConnectedToInternet. checkNetwork() returns false if inactive or isConnectedToInternet fails/is false, so the check is redundant. * MM checkActivatedStatus() checks org.rdk.AuthService active. MM doesn't activate it and org.rdk.AuthService doesn't auto start so this might not work or relies on external factor. Not attempting to fix that. org.rdk.AuthService should be used via QueryInterfaceByCallsign, this suffices to check if it's active. * MM acquires token but never uses it, this code is redundant. * Remove UtilsSecurityToken.h, UtilsSecurityToken.cpp, and SecurityUtil from MM. * RDKShell checks org.rdk.System active. RDKShell doesn't activate it and org.rdk.System doesn't auto start so this might not work or relies on external factor. Not attempting to fix that. org.rdk.System should be used via QueryInterfaceByCallsign, this suffices to check if it's active. * SecurityAgent should be used via QueryInterfaceByCallsign. * RDKShell used 2 variants of getThunderControllerClient(), one variant should be enough. * Remove UtilsSecurityToken.h, UtilsSecurityToken.cpp, and SecurityUtil from RDKShell. * DisplaySettings indefinitely checks org.rdk.HdmiCecSink active. On several platforms org.rdk.HdmiCecSink doesn't exist hence log flooding. DisplaySettings relies on race conditions in org.rdk.HdmiCecSink using sleep(...). Not attempting to fix that. org.rdk.HdmiCecSink should be used via QueryInterfaceByCallsign, this suffices to check if it's active. * DisplaySettings attempts to activate org.rdk.HdmiCecSink. This looks like design flaw. DisplaySettings activates org.rdk.HdmiCecSink in blocking way. Not attempting to fix that. Controller IDispatcher interface can activate. * DisplaySettings attempts to activate org.rdk.HdmiCecSink twice in onTimer(), the second one is redundant because timer will reschedule. * Remove UtilsSecurityToken.h, UtilsSecurityToken.cpp, and SecurityUtil from DisplaySettings. * Delete UtilsSecurityToken.h, UtilsSecurityToken.cpp. Test Procedure: * MM logs "AuthService is active". * RDKShell logs "SystemService is already activated". * RDKShell logs "RDKShell got security token". * RDKShell regression test. * DisplaySettings logs "org.rdk.HdmiCecSink is active". Risks: Low Signed-off-by: Nikita Poltorapavlo <[email protected]>
varumugam123 · Aug 31, 2022 · 2126ff9 · 2126ff9
1 parent 3e4f0b1
commit 2126ff9
Show file tree

Hide file tree

Showing 11 changed files with 137 additions and 315 deletions.
diff --git a/DisplaySettings/CMakeLists.txt b/DisplaySettings/CMakeLists.txt
@@ -22,8 +22,7 @@ find_package(${NAMESPACE}Plugins REQUIRED)
 
 add_library(${MODULE_NAME} SHARED
         DisplaySettings.cpp
-        Module.cpp
-        ../helpers/UtilsSecurityToken.cpp)
+        Module.cpp)
 
 set_target_properties(${MODULE_NAME} PROPERTIES
         CXX_STANDARD 11
@@ -39,9 +38,9 @@ if (DS_FOUND)
     add_definitions(-DDS_FOUND)
     target_include_directories(${MODULE_NAME} PRIVATE ${IARMBUS_INCLUDE_DIRS})
     target_include_directories(${MODULE_NAME} PRIVATE ${DS_INCLUDE_DIRS})
-    target_link_libraries(${MODULE_NAME} PRIVATE ${NAMESPACE}Plugins::${NAMESPACE}Plugins ${NAMESPACE}SecurityUtil ${IARMBUS_LIBRARIES} ${DS_LIBRARIES} "-ltr181api")
+    target_link_libraries(${MODULE_NAME} PRIVATE ${NAMESPACE}Plugins::${NAMESPACE}Plugins ${IARMBUS_LIBRARIES} ${DS_LIBRARIES} "-ltr181api")
 else (DS_FOUND)
-    target_link_libraries(${MODULE_NAME} PRIVATE ${NAMESPACE}Plugins::${NAMESPACE}Plugins ${NAMESPACE}SecurityUtil)
+    target_link_libraries(${MODULE_NAME} PRIVATE ${NAMESPACE}Plugins::${NAMESPACE}Plugins)
 endif(DS_FOUND)
 
 install(TARGETS ${MODULE_NAME}

diff --git a/DisplaySettings/DisplaySettings.cpp b/DisplaySettings/DisplaySettings.cpp
@@ -48,7 +48,6 @@
 #include "UtilsCStr.h"
 #include "UtilsIarm.h"
 #include "UtilsJsonRpc.h"
-#include "UtilsSecurityToken.h"
 #include "UtilsString.h"
 #include "UtilsisValidInt.h"
 #include "dsRpc.h"
@@ -370,10 +369,12 @@ namespace WPEFramework {
 				LOG_DEVICE_EXCEPTION1(string("HDMI_ARC0"));
 			}
 
-			bool isPluginActivated = Utils::isPluginActivated(HDMICECSINK_CALLSIGN);
+            auto hdmiCecSink = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>(HDMICECSINK_CALLSIGN);
+            if (hdmiCecSink != nullptr) {
+                LOGINFO("%s is active", HDMICECSINK_CALLSIGN);
+                hdmiCecSink->Release();
 
-			if(isPluginActivated) {
-			    if(!m_subscribed) {
+                if(!m_subscribed) {
 			        if((subscribeForHdmiCecSinkEvent(HDMICECSINK_ARC_INITIATION_EVENT) == Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_ARC_TERMINATION_EVENT) == Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_SHORT_AUDIO_DESCRIPTOR_EVENT)== Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_SYSTEM_AUDIO_MODE_EVENT) == Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_AUDIO_DEVICE_CONNECTED_STATUS_EVENT) == Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_CEC_ENABLED_EVENT) == Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_AUDIO_DEVICE_POWER_STATUS_EVENT) == Core::ERROR_NONE)) {
                                     m_subscribed = true;
                                     LOGINFO("%s: HdmiCecSink event subscription completed.\n",__FUNCTION__);
@@ -486,8 +487,14 @@ namespace WPEFramework {
             }
         }
 
-        const string DisplaySettings::Initialize(PluginHost::IShell* /* service */)
+        const string DisplaySettings::Initialize(PluginHost::IShell* service)
         {
+            ASSERT(service != nullptr);
+            ASSERT(m_service == nullptr);
+
+            m_service = service;
+            m_service->AddRef();
+
 	    m_arcRoutingThread = std::thread(cecArcRoutingThread);
 	    m_timer.connect(std::bind(&DisplaySettings::onTimer, this));
             m_AudioDeviceDetectTimer.connect(std::bind(&DisplaySettings::checkAudioDeviceDetectionTimer, this));
@@ -507,7 +514,7 @@ namespace WPEFramework {
             return (string());
         }
 
-        void DisplaySettings::Deinitialize(PluginHost::IShell* /* service */)
+        void DisplaySettings::Deinitialize(PluginHost::IShell* service)
         {
 	   LOGINFO("Enetering DisplaySettings::Deinitialize");
 	   isCecArcRoutingThreadEnabled = false;
@@ -536,6 +543,11 @@ namespace WPEFramework {
 
             DeinitializeIARM();
             DisplaySettings::_instance = nullptr;
+
+            ASSERT(service == m_service);
+
+            m_service->Release();
+            m_service = nullptr;
         }
 
         void DisplaySettings::InitializeIARM()
@@ -3934,7 +3946,11 @@ namespace WPEFramework {
         {
             bool success = true;
 
-            if (Utils::isPluginActivated(HDMICECSINK_CALLSIGN)) {
+            auto hdmiCecSink = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>(HDMICECSINK_CALLSIGN);
+            if (hdmiCecSink != nullptr) {
+                LOGINFO("%s is active", HDMICECSINK_CALLSIGN);
+                hdmiCecSink->Release();
+
                 getHdmiCecSinkPlugin();
                 if (!m_client) {
                     LOGERR("HdmiCecSink Initialisation failed\n");
@@ -3969,7 +3985,11 @@ namespace WPEFramework {
         {
             bool cecEnable = false;
 
-            if (Utils::isPluginActivated(HDMICECSINK_CALLSIGN)) {
+            auto hdmiCecSink = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>(HDMICECSINK_CALLSIGN);
+            if (hdmiCecSink != nullptr) {
+                LOGINFO("%s is active", HDMICECSINK_CALLSIGN);
+                hdmiCecSink->Release();
+
                 getHdmiCecSinkPlugin();
                 if (!m_client) {
                     LOGERR("HdmiCecSink Initialisation failed\n");
@@ -3998,7 +4018,11 @@ namespace WPEFramework {
         {
             bool hdmiAudioDeviceDetected = false;
 
-            if (Utils::isPluginActivated(HDMICECSINK_CALLSIGN)) {
+            auto hdmiCecSink = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>(HDMICECSINK_CALLSIGN);
+            if (hdmiCecSink != nullptr) {
+                LOGINFO("%s is active", HDMICECSINK_CALLSIGN);
+                hdmiCecSink->Release();
+
                 getHdmiCecSinkPlugin();
                 if (!m_client) {
                     LOGERR("HdmiCecSink Initialisation failed\n");
@@ -4027,7 +4051,11 @@ namespace WPEFramework {
         {
             bool success = true;
 
-            if (Utils::isPluginActivated(HDMICECSINK_CALLSIGN)) {
+            auto hdmiCecSink = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>(HDMICECSINK_CALLSIGN);
+            if (hdmiCecSink != nullptr) {
+                LOGINFO("%s is active", HDMICECSINK_CALLSIGN);
+                hdmiCecSink->Release();
+
                 getHdmiCecSinkPlugin();
                 if (!m_client) {
                     LOGERR("HdmiCecSink Initialisation failed\n");
@@ -4056,7 +4084,11 @@ namespace WPEFramework {
         {
             bool success = true;
 
-            if (Utils::isPluginActivated(HDMICECSINK_CALLSIGN)) {
+            auto hdmiCecSink = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>(HDMICECSINK_CALLSIGN);
+            if (hdmiCecSink != nullptr) {
+                LOGINFO("%s is active", HDMICECSINK_CALLSIGN);
+                hdmiCecSink->Release();
+
                 getHdmiCecSinkPlugin();
                 if (!m_client) {
                     LOGERR("HdmiCecSink plugin not accessible\n");
@@ -4085,7 +4117,11 @@ namespace WPEFramework {
         {
             bool success = true;
 
-            if (Utils::isPluginActivated(HDMICECSINK_CALLSIGN)) {
+            auto hdmiCecSink = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>(HDMICECSINK_CALLSIGN);
+            if (hdmiCecSink != nullptr) {
+                LOGINFO("%s is active", HDMICECSINK_CALLSIGN);
+                hdmiCecSink->Release();
+
                 getHdmiCecSinkPlugin();
                 if (!m_client) {
                     LOGERR("HdmiCecSink plugin not accessible\n");
@@ -4915,17 +4951,47 @@ namespace WPEFramework {
             // lock to prevent: parallel onTimer runs, destruction during onTimer
             lock_guard<mutex> lck(m_callMutex);
 
-            bool isPluginActivated = Utils::isPluginActivated(HDMICECSINK_CALLSIGN);
+            bool isPluginActivated = false;
+
+            auto hdmiCecSink = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>(HDMICECSINK_CALLSIGN);
+            if (hdmiCecSink != nullptr) {
+                LOGINFO("%s is active", HDMICECSINK_CALLSIGN);
+                hdmiCecSink->Release();
+                isPluginActivated = true;
+            }
 
             if (!isPluginActivated) {
                 /*HDMICECSINK_CALLSIGN plugin activation moved to onTimer.
                  *To decouple from displyasettings init. Since its time taking*/
-                Utils::activatePlugin(HDMICECSINK_CALLSIGN);
+
+                auto controller = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>("Controller");
+                if (controller != nullptr) {
+                    auto message = (Core::ProxyType<Core::JSONRPC::Message>(PluginHost::IFactories::Instance().JSONRPC()));
+                    message->JSONRPC = Core::JSONRPC::Message::DefaultVersion;
+                    message->Id = 0;
+                    message->Designator = "Controller.1.activate";
+                    message->Parameters = "{\"callsign\":\"org.rdk.HdmiCecSink\"}";
+                    auto resp = controller->Invoke("", ~0, *message);
+                    if (resp->Error.IsSet()) {
+                        std::cout << "Call failed: " << message->Designator.Value() << " error: " << resp->Error.Text.Value() << "\n";
+                    }
+                    controller->Release();
+                }
+
                 LOGWARN ("DisplaySettings::onTimer after activatePlugin HDMICECSINK_CALLSIGN line:%d", __LINE__);
                 sleep(HDMICECSINK_PLUGIN_ACTIVATION_TIME);
             }
 
-            bool pluginActivated = Utils::isPluginActivated(HDMICECSINK_CALLSIGN);
+            static bool isInitDone = false;
+            bool pluginActivated = false;
+
+            hdmiCecSink = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>(HDMICECSINK_CALLSIGN);
+            if (hdmiCecSink != nullptr) {
+                LOGINFO("%s is active", HDMICECSINK_CALLSIGN);
+                hdmiCecSink->Release();
+                pluginActivated = true;
+            }
+
             LOGWARN ("DisplaySettings::onTimer pluginActivated:%d line:%d", pluginActivated, __LINE__);
             if(!m_subscribed) {
                 if (pluginActivated && (subscribeForHdmiCecSinkEvent(HDMICECSINK_ARC_INITIATION_EVENT) == Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_ARC_TERMINATION_EVENT) == Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_SHORT_AUDIO_DESCRIPTOR_EVENT)== Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_SYSTEM_AUDIO_MODE_EVENT) == Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_AUDIO_DEVICE_CONNECTED_STATUS_EVENT) == Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_CEC_ENABLED_EVENT) == Core::ERROR_NONE) && (subscribeForHdmiCecSinkEvent(HDMICECSINK_AUDIO_DEVICE_POWER_STATUS_EVENT) == Core::ERROR_NONE))
@@ -4940,10 +5006,6 @@ namespace WPEFramework {
 
                 } else {
                     LOGERR("Could not subscribe this time, one more attempt in %d msec. Plugin is %s", RECONNECTION_TIME_IN_MILLISECONDS, pluginActivated ? "ACTIVE" : "BLOCKED");
-                    if (!pluginActivated)
-                    {
-                        Utils::activatePlugin(HDMICECSINK_CALLSIGN);
-                    }
                 }
             } else {
                 //Standby ON transitions case

diff --git a/DisplaySettings/DisplaySettings.h b/DisplaySettings/DisplaySettings.h
@@ -256,7 +256,9 @@ namespace WPEFramework {
             };
 
             int m_hdmiInAudioDevicePowerState;
-            int m_currentArcRoutingState; 
+            int m_currentArcRoutingState;
+
+            PluginHost::IShell* m_service;
 
         public:
             static DisplaySettings* _instance;

diff --git a/MaintenanceManager/CMakeLists.txt b/MaintenanceManager/CMakeLists.txt
@@ -22,8 +22,7 @@ find_package(${NAMESPACE}Plugins REQUIRED)
 
 add_library(${MODULE_NAME} SHARED
         MaintenanceManager.cpp
-        Module.cpp
-        ../helpers/UtilsSecurityToken.cpp)
+        Module.cpp)
 
 set_target_properties(${MODULE_NAME} PROPERTIES
         CXX_STANDARD 11
@@ -37,11 +36,11 @@ list(APPEND CMAKE_MODULE_PATH
 find_package(IARMBus)
 if (IARMBus_FOUND)
     target_include_directories(${MODULE_NAME} PRIVATE ${IARMBUS_INCLUDE_DIRS})
-    target_link_libraries(${MODULE_NAME} PRIVATE ${NAMESPACE}Plugins::${NAMESPACE}Plugins ${NAMESPACE}SecurityUtil ${IARMBUS_LIBRARIES})
+    target_link_libraries(${MODULE_NAME} PRIVATE ${NAMESPACE}Plugins::${NAMESPACE}Plugins ${IARMBUS_LIBRARIES})
 else (IARMBus_FOUND)
     message ("Module IARMBus required.")
     target_include_directories(${MODULE_NAME} PRIVATE ${IARMBUS_INCLUDE_DIRS})
-    target_link_libraries(${MODULE_NAME} PRIVATE ${NAMESPACE}Plugins::${NAMESPACE}Plugins ${NAMESPACE}SecurityUtil ${IARMBUS_LIBRARIES})
+    target_link_libraries(${MODULE_NAME} PRIVATE ${NAMESPACE}Plugins::${NAMESPACE}Plugins ${IARMBUS_LIBRARIES})
 endif(IARMBus_FOUND)
 
 target_include_directories(${MODULE_NAME} PRIVATE ../helpers)

diff --git a/MaintenanceManager/MaintenanceManager.cpp b/MaintenanceManager/MaintenanceManager.cpp
@@ -41,7 +41,6 @@
 
 #include "UtilsIarm.h"
 #include "UtilsJsonRpc.h"
-#include "UtilsSecurityToken.h"
 #include "UtilscRunScript.h"
 #include "UtilsfileExists.h"
 
@@ -345,19 +344,17 @@ namespace WPEFramework {
             JsonObject joGetParams;
             JsonObject joGetResult;
             std::string callsign = "org.rdk.AuthService.1";
-            std::string token;
             uint8_t i = 0;
-            bool isAuthSerivcePluginActive = false;
             std::string ret_status("invalid");
 
             /* check if plugin active */
-            isAuthSerivcePluginActive = Utils::isPluginActivated("org.rdk.AuthService");
-            if (!isAuthSerivcePluginActive){
+            auto auth = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>("org.rdk.AuthService");
+            if (auth == nullptr){
                 LOGINFO("AuthService plugin is not activated.Retrying.. \n");
                 //if plugin is not activated we need to retry
                 do{
-                    isAuthSerivcePluginActive = Utils::isPluginActivated("org.rdk.AuthService");
-                    if ( !isAuthSerivcePluginActive ){
+                    auth = m_service->QueryInterfaceByCallsign<PluginHost::IDispatcher>("org.rdk.AuthService");
+                    if (auth == nullptr){
                         sleep(10);
                         i++;
                         LOGINFO("AuthService retries [%d/4] \n",i);
@@ -367,16 +364,18 @@ namespace WPEFramework {
                     }
                 }while( i < MAX_ACTIVATION_RETRIES );
 
-                if ( !isAuthSerivcePluginActive ){
+                if (auth == nullptr){
                     LOGINFO("AuthService plugin is Still not active");
                     return ret_status;
                 }
                 else{
                     LOGINFO("AuthService plugin is Now active");
                 }
             }
-
-            Utils::SecurityToken::getSecurityToken(token);
+            if (auth != nullptr){
+                LOGINFO("AuthService is active");
+                auth->Release();
+            }
 
             Core::SystemInfo::SetEnvironment(_T("THUNDER_ACCESS"), _T(SERVER_DETAILS));
             auto thunder_client = make_shared<WPEFramework::JSONRPC::LinkType<WPEFramework::Core::JSON::IElement> >(callsign.c_str(), "");
@@ -457,17 +456,7 @@ namespace WPEFramework {
             JsonObject joGetParams;
             JsonObject joGetResult;
             std::string callsign = "org.rdk.Network.1";
-            std::string token;
-
-            /* check if plugin active */
-            if (false == Utils::isPluginActivated("org.rdk.Network")) {
-                    LOGINFO("Network plugin is not activated \n");
-                    return false;
-            }
 
-            Utils::SecurityToken::getSecurityToken(token);
-
-            string query = "token=" + token;
             Core::SystemInfo::SetEnvironment(_T("THUNDER_ACCESS"), _T(SERVER_DETAILS));
             auto thunder_client = make_shared<WPEFramework::JSONRPC::LinkType<WPEFramework::Core::JSON::IElement> >(callsign.c_str(), "");
             if (thunder_client != nullptr) {
@@ -517,21 +506,33 @@ namespace WPEFramework {
             MaintenanceManager::_instance = nullptr;
         }
 
-        const string MaintenanceManager::Initialize(PluginHost::IShell*)
+        const string MaintenanceManager::Initialize(PluginHost::IShell* service)
         {
+            ASSERT(service != nullptr);
+            ASSERT(m_service == nullptr);
+
+            m_service = service;
+            m_service->AddRef();
+
 #if defined(USE_IARMBUS) || defined(USE_IARM_BUS)
             InitializeIARM();
 #endif /* defined(USE_IARMBUS) || defined(USE_IARM_BUS) */
+
             /* On Success; return empty to indicate no error text. */
             return (string());
         }
 
-        void MaintenanceManager::Deinitialize(PluginHost::IShell*)
+        void MaintenanceManager::Deinitialize(PluginHost::IShell* service)
         {
 #if defined(USE_IARMBUS) || defined(USE_IARM_BUS)
             stopMaintenanceTasks();
             DeinitializeIARM();
 #endif /* defined(USE_IARMBUS) || defined(USE_IARM_BUS) */
+
+            ASSERT(service == m_service);
+
+            m_service->Release();
+            m_service = nullptr;
         }
 
 #if defined(USE_IARMBUS) || defined(USE_IARM_BUS)

diff --git a/MaintenanceManager/MaintenanceManager.h b/MaintenanceManager/MaintenanceManager.h
@@ -137,6 +137,7 @@ namespace WPEFramework {
                 std::thread m_thread;
 
                 std::map<string, bool> m_task_map;
+                PluginHost::IShell* m_service;
 
                 bool isDeviceOnline();
                 void task_execution_thread();

diff --git a/RDKShell/CMakeLists.txt b/RDKShell/CMakeLists.txt
@@ -28,7 +28,6 @@ find_package(IARMBus)
 add_library(${MODULE_NAME} SHARED
         RDKShell.cpp
         Module.cpp
-        ../helpers/UtilsSecurityToken.cpp
 )
 
 set_target_properties(${MODULE_NAME} PROPERTIES
@@ -47,7 +46,7 @@ target_include_directories(${MODULE_NAME} PRIVATE ../helpers ${IARMBUS_INCLUDE_D
 set(RDKSHELL_INCLUDES $ENV{RDKSHELL_INCLUDES})
 separate_arguments(RDKSHELL_INCLUDES)
 include_directories(BEFORE ${RDKSHELL_INCLUDES})
-target_link_libraries(${MODULE_NAME} PRIVATE ${NAMESPACE}Plugins::${NAMESPACE}Plugins ${NAMESPACE}SecurityUtil -lrdkshell ${PLUGIN_RDKSHELL_EXTRA_LIBRARIES} trower-base64)
+target_link_libraries(${MODULE_NAME} PRIVATE ${NAMESPACE}Plugins::${NAMESPACE}Plugins -lrdkshell ${PLUGIN_RDKSHELL_EXTRA_LIBRARIES} trower-base64)
 
 install(TARGETS ${MODULE_NAME}
         DESTINATION lib/${STORAGE_DIRECTORY}/plugins)