reject PTU system request when PTU not in progress (#3853)

* reject PTU system request when PTU not in progress

* track last_ptu_app_id_ in ext prop policy mode

* fix style

* Apply suggestions from code review

Co-authored-by: Shobhit Adlakha <ShobhitAd@users.noreply.github.com>

* fix style

Co-authored-by: Shobhit Adlakha <ShobhitAd@users.noreply.github.com>

6 files changed, 51 insertions, 9 deletions

diff --git a/src/components/application_manager/include/application_manager/policies/policy_handler.h b/src/components/application_manager/include/application_manager/policies/policy_handler.h
index 662e255871..79b864e650 100644
--- a/src/components/application_manager/include/application_manager/policies/policy_handler.h
+++ b/src/components/application_manager/include/application_manager/policies/policy_handler.h
@@ -429,6 +429,8 @@ class PolicyHandler : public PolicyHandlerInterface,
   void CacheRetryInfo(const uint32_t app_id = 0,
                       const std::string url = std::string(),
                       const std::string snapshot_path = std::string()) OVERRIDE;
+#else   // EXTERNAL_PROPRIETARY_MODE
+  void UpdateLastPTUApp(const uint32_t app_id) OVERRIDE;
 #endif  // EXTERNAL_PROPRIETARY_MODE
 
   uint32_t GetAppIdForSending() const OVERRIDE;
@@ -721,6 +723,8 @@ class PolicyHandler : public PolicyHandlerInterface,
 
   void StopRetrySequence() OVERRIDE;
 
+  bool IsPTUSystemRequestAllowed(const uint32_t app_id) OVERRIDE;
+
   /**
    * @brief OnDeviceSwitching Notifies policy manager on device switch event so
    * policy permissions should be processed accordingly
@@ -932,10 +936,10 @@ class PolicyHandler : public PolicyHandlerInterface,
   std::shared_ptr<PolicyManager> atomic_policy_manager_;
   std::shared_ptr<PolicyEventObserver> event_observer_;
   uint32_t last_activated_app_id_;
+  uint32_t last_ptu_app_id_;
 
 #ifndef EXTERNAL_PROPRIETARY_MODE
   // PTU retry information
-  uint32_t last_ptu_app_id_;
   std::string retry_update_url_;
   std::string policy_snapshot_path_;
 #endif  // EXTERNAL_PROPRIETARY_MODE
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_system_request_notification.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_system_request_notification.cc
index b75081e30f..f9900fd5c1 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_system_request_notification.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_system_request_notification.cc
@@ -153,6 +153,9 @@ void OnSystemRequestNotification::Run() {
   if (helpers::Compare<RequestType, helpers::EQ, helpers::ONE>(
           request_type, RequestType::RT_PROPRIETARY, RequestType::RT_HTTP)) {
     policy_handler_.OnSystemRequestReceived();
+#ifdef EXTERNAL_PROPRIETARY_MODE
+    policy_handler_.UpdateLastPTUApp(app->app_id());
+#endif
   }
   SendNotificationToMobile(message_);
 }
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/system_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/system_request.cc
index 97b2d2ff37..1ddb41704f 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/system_request.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/system_request.cc
@@ -580,6 +580,16 @@ void SystemRequest::Run() {
 
   SDL_LOG_DEBUG("Binary data ok.");
 
+  if (mobile_apis::RequestType::PROPRIETARY == request_type ||
+      mobile_apis::RequestType::HTTP == request_type) {
+    auto app_id = application->app_id();
+    if (!policy_handler_.IsPTUSystemRequestAllowed(app_id)) {
+      SDL_LOG_DEBUG("Rejected PTU SystemRequest from app " << app_id);
+      SendResponse(false, mobile_apis::Result::REJECTED);
+      return;
+    }
+  }
+
   if (mobile_apis::RequestType::ICON_URL == request_type) {
     application_manager_.SetIconFileFromSystemRequest(file_name);
     SendResponse(true, mobile_apis::Result::SUCCESS);
diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc
index 500981a5a8..100a9fe271 100644
--- a/src/components/application_manager/src/policies/policy_handler.cc
+++ b/src/components/application_manager/src/policies/policy_handler.cc
@@ -300,14 +300,11 @@ PolicyHandler::PolicyHandler(const PolicySettings& settings,
                              ApplicationManager& application_manager)
     : AsyncRunner("PolicyHandler async runner thread")
     , last_activated_app_id_(0)
-#ifndef EXTERNAL_PROPRIETARY_MODE
     , last_ptu_app_id_(0)
-#endif  // EXTERNAL_PROPRIETARY_MODE
     , statistic_manager_impl_(std::make_shared<StatisticManagerImpl>(this))
     , settings_(settings)
     , application_manager_(application_manager)
-    , last_registered_policy_app_id_(std::string()) {
-}
+    , last_registered_policy_app_id_(std::string()) {}
 
 PolicyHandler::~PolicyHandler() {}
 
@@ -423,13 +420,31 @@ void PolicyHandler::StopRetrySequence() {
   SDL_LOG_AUTO_TRACE();
   const auto policy_manager = LoadPolicyManager();
   POLICY_LIB_CHECK_VOID(policy_manager);
-#ifndef EXTERNAL_PROPRIETARY_MODE
   // Clear cached PTU app
   last_ptu_app_id_ = 0;
-#endif  // EXTERNAL_PROPRIETARY_MODE
   policy_manager->StopRetrySequence();
 }
 
+bool PolicyHandler::IsPTUSystemRequestAllowed(const uint32_t app_id) {
+  SDL_LOG_AUTO_TRACE();
+  const auto policy_manager = LoadPolicyManager();
+  POLICY_LIB_CHECK_OR_RETURN(policy_manager, false);
+
+  if (policy_manager->GetPolicyTableStatus() != "UPDATING") {
+    SDL_LOG_DEBUG("PTU received while not UPDATING");
+    return false;
+  }
+
+  if (app_id != last_ptu_app_id_) {
+    SDL_LOG_DEBUG(
+        "PTU received from unexpected application, request was sent to "
+        << last_ptu_app_id_);
+    return false;
+  }
+
+  return true;
+}
+
 bool PolicyHandler::ResetPolicyTable() {
   SDL_LOG_TRACE("Reset policy table.");
   const auto policy_manager = LoadPolicyManager();
@@ -476,6 +491,11 @@ void PolicyHandler::CacheRetryInfo(const uint32_t app_id,
   retry_update_url_ = url;
   policy_snapshot_path_ = snapshot_path;
 }
+#else   // EXTERNAL_PROPRIETARY_MODE
+void PolicyHandler::UpdateLastPTUApp(const uint32_t app_id) {
+  SDL_LOG_DEBUG("UpdateLastPTUApp to " << app_id);
+  last_ptu_app_id_ = app_id;
+}
 #endif  // EXTERNAL_PROPRIETARY_MODE
 
 uint32_t PolicyHandler::GetAppIdForSending() const {
@@ -1286,10 +1306,8 @@ bool PolicyHandler::ReceiveMessageFromSDK(const std::string& file,
     policy_manager->CleanupUnpairedDevices();
     SetDaysAfterEpoch();
     policy_manager->OnPTUFinished(load_pt_result);
-#ifndef EXTERNAL_PROPRIETARY_MODE
     // Clean up retry information
     last_ptu_app_id_ = 0;
-#endif  // EXTERNAL_PROPRIETARY_MODE
 
     uint32_t correlation_id = application_manager_.GetNextHMICorrelationID();
     event_observer_->subscribe_on_event(
diff --git a/src/components/include/application_manager/policies/policy_handler_interface.h b/src/components/include/application_manager/policies/policy_handler_interface.h
index d79c059b34..0824e90984 100644
--- a/src/components/include/application_manager/policies/policy_handler_interface.h
+++ b/src/components/include/application_manager/policies/policy_handler_interface.h
@@ -406,6 +406,8 @@ class PolicyHandlerInterface : public VehicleDataItemProvider {
       const uint32_t app_id = 0,
       const std::string url = std::string(),
       const std::string snapshot_path = std::string()) = 0;
+#else
+  virtual void UpdateLastPTUApp(const uint32_t app_id) = 0;
 #endif  // EXTERNAL_PROPRIETARY_MODE
 
   /**
@@ -426,6 +428,8 @@ class PolicyHandlerInterface : public VehicleDataItemProvider {
 
   virtual void OnPTInited() = 0;
 
+  virtual bool IsPTUSystemRequestAllowed(const uint32_t app_id) = 0;
+
   /**
    * @brief Force stops retry sequence timer and resets retry sequence
    */
diff --git a/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h b/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h
index ca5d25ed4c..78667dc6ff 100644
--- a/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h
+++ b/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h
@@ -200,6 +200,8 @@ class MockPolicyHandlerInterface : public policy::PolicyHandlerInterface {
                void(const uint32_t app_id,
                     const std::string url,
                     const std::string snapshot_path));
+#else
+  MOCK_METHOD1(UpdateLastPTUApp, void(const uint32_t app_id));
 #endif
   MOCK_CONST_METHOD0(GetAppIdForSending, uint32_t());
   MOCK_METHOD1(
@@ -210,6 +212,7 @@ class MockPolicyHandlerInterface : public policy::PolicyHandlerInterface {
   MOCK_METHOD1(OnCertificateUpdated, void(const std::string& certificate_data));
   MOCK_METHOD1(OnPTUFinished, void(const bool ptu_result));
   MOCK_METHOD0(OnPTInited, void());
+  MOCK_METHOD1(IsPTUSystemRequestAllowed, bool(const uint32_t app_id));
   MOCK_METHOD0(StopRetrySequence, void());
   MOCK_METHOD1(OnCertificateDecrypted, void(bool is_succeeded));
   MOCK_METHOD0(CanUpdate, bool());