Additional fixes after ATF testing

Fixed OnCertificateUpdated notification callback for empty certificate
Fixed GetSystemTime triggering
Fixed handshake resuming on system time arrived
Fixed wrong logic in case system time is not ready
Removed redundant logic for non-navi applications
Removed PTU triggerring for navi on empty certificate in DB

16 files changed, 71 insertions, 155 deletions

diff --git a/src/components/application_manager/include/application_manager/application_manager_impl.h b/src/components/application_manager/include/application_manager/application_manager_impl.h
index 905e3cf057..aa3763108e 100644
--- a/src/components/application_manager/include/application_manager/application_manager_impl.h
+++ b/src/components/application_manager/include/application_manager/application_manager_impl.h
@@ -815,12 +815,11 @@ class ApplicationManagerImpl
       security_manager::SSLContext::HandshakeResult result) OVERRIDE;
 
   /**
-   * @brief OnHandshakeFailed currently does nothing.
-   * This method is declared as pure virtual in common base class
-   * therefore it has to be present and should be implemented
-   * to allow creation of current class instances
+   * @brief Notification about handshake failure
+   * @return true on success notification handling or false otherwise
    */
-  void OnHandshakeFailed() OVERRIDE;
+  bool OnHandshakeFailed() OVERRIDE;
+
   /**
    * @brief Notification that certificate update is required.
    */
diff --git a/src/components/application_manager/include/application_manager/system_time/system_time_handler_impl.h b/src/components/application_manager/include/application_manager/system_time/system_time_handler_impl.h
index 28ba629e19..add440ad80 100644
--- a/src/components/application_manager/include/application_manager/system_time/system_time_handler_impl.h
+++ b/src/components/application_manager/include/application_manager/system_time/system_time_handler_impl.h
@@ -140,10 +140,13 @@ class SystemTimeHandlerImpl : public utils::SystemTimeHandler,
   mutable sync_primitives::Lock state_lock_;
   // Variable means HMI readiness to provide system time by request
   volatile bool utc_time_can_be_received_;
-  // Varible used to schedule next GetSystemTime request
-  // if at the moment of sending first GetSystemTime request
-  // HMI is not ready to provide system time
-  volatile bool schedule_request_;
+
+  /**
+   * @brief Flag used to control that only GetSystemTime request at time could
+   * be sent to HMI
+   */
+  volatile bool awaiting_get_system_time_;
+
   // Varible used to store result for GetSystemTime request
   time_t last_time_;
 
diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc
index fac7d596a6..3f0bc20873 100644
--- a/src/components/application_manager/src/application_manager_impl.cc
+++ b/src/components/application_manager/src/application_manager_impl.cc
@@ -1700,8 +1700,9 @@ bool ApplicationManagerImpl::OnHandshakeDone(
   return false;
 }
 
-void ApplicationManagerImpl::OnHandshakeFailed() {
+bool ApplicationManagerImpl::OnHandshakeFailed() {
   LOG4CXX_AUTO_TRACE(logger_);
+  return false;
 }
 
 void ApplicationManagerImpl::OnCertificateUpdateRequired() {
diff --git a/src/components/application_manager/src/system_time/system_time_handler_impl.cc b/src/components/application_manager/src/system_time/system_time_handler_impl.cc
index a91fb16197..6ae6d3e901 100644
--- a/src/components/application_manager/src/system_time/system_time_handler_impl.cc
+++ b/src/components/application_manager/src/system_time/system_time_handler_impl.cc
@@ -45,7 +45,7 @@ SystemTimeHandlerImpl::SystemTimeHandlerImpl(
     ApplicationManager& application_manager)
     : event_engine::EventObserver(application_manager.event_dispatcher())
     , utc_time_can_be_received_(false)
-    , schedule_request_(false)
+    , awaiting_get_system_time_(false)
     , system_time_listener_(NULL)
     , app_manager_(application_manager) {
   LOG4CXX_AUTO_TRACE(logger_);
@@ -67,7 +67,6 @@ void SystemTimeHandlerImpl::DoSystemTimeQuery() {
     LOG4CXX_INFO(logger_,
                  "Navi module is not yet ready."
                      << "Will process request once it became ready.");
-    schedule_request_ = true;
     return;
   }
   SendTimeRequest();
@@ -99,11 +98,18 @@ bool SystemTimeHandlerImpl::utc_time_can_be_received() const {
 
 void SystemTimeHandlerImpl::SendTimeRequest() {
   LOG4CXX_AUTO_TRACE(logger_);
+
+  if (awaiting_get_system_time_) {
+    LOG4CXX_WARN(logger_, "Another GetSystemTime request in progress. Skipped");
+    return;
+  }
+
   using namespace application_manager;
   uint32_t correlation_id = app_manager_.GetNextHMICorrelationID();
   subscribe_on_event(hmi_apis::FunctionID::BasicCommunication_GetSystemTime,
                      correlation_id);
   MessageHelper::SendGetSystemTimeRequest(correlation_id, app_manager_);
+  awaiting_get_system_time_ = true;
 }
 
 void SystemTimeHandlerImpl::on_event(
@@ -128,10 +134,6 @@ void SystemTimeHandlerImpl::ProcessSystemTimeReadyNotification() {
   LOG4CXX_AUTO_TRACE(logger_);
   sync_primitives::AutoLock lock(state_lock_);
   utc_time_can_be_received_ = true;
-  if (schedule_request_) {
-    SendTimeRequest();
-    schedule_request_ = false;
-  }
   unsubscribe_from_event(
       hmi_apis::FunctionID::BasicCommunication_OnSystemTimeReady);
 }
@@ -164,6 +166,8 @@ void SystemTimeHandlerImpl::ProcessSystemTimeResponse(
   if (system_time_listener_) {
     system_time_listener_->OnSystemTimeArrived(last_time_);
   }
+  sync_primitives::AutoLock state_lock(state_lock_);
+  awaiting_get_system_time_ = false;
 }
 
 }  // namespace application_manager
diff --git a/src/components/connection_handler/src/connection_handler_impl.cc b/src/components/connection_handler/src/connection_handler_impl.cc
index b97c6eacd4..59865ac4b2 100644
--- a/src/components/connection_handler/src/connection_handler_impl.cc
+++ b/src/components/connection_handler/src/connection_handler_impl.cc
@@ -467,14 +467,6 @@ void ConnectionHandlerImpl::OnSessionStartedCallback(
     const uint32_t session_key =
         KeyFromPair(connection_handle, context.new_session_id_);
 
-    uint32_t app_id = 0;
-    GetDataOnSessionKey(
-        session_key, &app_id, NULL, static_cast<DeviceHandle*>(NULL));
-    if (app_id > 0) {
-      context.is_ptu_required_ =
-          !connection_handler_observer_->CheckAppIsNavi(app_id);
-    }
-
     {
       sync_primitives::AutoLock auto_lock(start_service_context_map_lock_);
       start_service_context_map_[session_key] = context;
diff --git a/src/components/include/protocol_handler/session_observer.h b/src/components/include/protocol_handler/session_observer.h
index 3482c6569c..242775bf25 100644
--- a/src/components/include/protocol_handler/session_observer.h
+++ b/src/components/include/protocol_handler/session_observer.h
@@ -66,7 +66,6 @@ struct SessionContext {
   uint32_t hash_id_;
   bool is_protected_;
   bool is_new_service_;
-  bool is_ptu_required_;
 
   /**
    * @brief Constructor
@@ -78,8 +77,7 @@ struct SessionContext {
       , service_type_(protocol_handler::kInvalidServiceType)
       , hash_id_(0)
       , is_protected_(false)
-      , is_new_service_(false)
-      , is_ptu_required_(false) {}
+      , is_new_service_(false) {}
 
   /**
    * @brief Constructor
@@ -105,8 +103,7 @@ struct SessionContext {
       , service_type_(service_type)
       , hash_id_(hash_id)
       , is_protected_(is_protected)
-      , is_new_service_(false)
-      , is_ptu_required_(false) {}
+      , is_new_service_(false) {}
 };
 
 /**
diff --git a/src/components/include/security_manager/security_manager.h b/src/components/include/security_manager/security_manager.h
index 6ad5e96989..61ba43c74f 100644
--- a/src/components/include/security_manager/security_manager.h
+++ b/src/components/include/security_manager/security_manager.h
@@ -164,6 +164,11 @@ class SecurityManager : public protocol_handler::ProtocolObserver,
   virtual void NotifyOnCertificateUpdateRequired() = 0;
 
   /**
+   * @brief Notify all listeners that handshake was failed
+   */
+  virtual void NotifyListenersOnHandshakeFailed() = 0;
+
+  /**
    * @brief Check if policy certificate data is empty
    * @return true if policy certificate data is empty otherwise false
    */
diff --git a/src/components/include/security_manager/security_manager_listener.h b/src/components/include/security_manager/security_manager_listener.h
index 5942839b58..00a4c68134 100644
--- a/src/components/include/security_manager/security_manager_listener.h
+++ b/src/components/include/security_manager/security_manager_listener.h
@@ -50,8 +50,9 @@ class SecurityManagerListener {
 
   /**
    * @brief Notification about handshake failure
+   * @return true on success notification handling or false otherwise
    */
-  virtual void OnHandshakeFailed() = 0;
+  virtual bool OnHandshakeFailed() = 0;
 
   /**
    * @brief Notify listeners that certificate update is required.
diff --git a/src/components/policy/policy_external/src/policy_manager_impl.cc b/src/components/policy/policy_external/src/policy_manager_impl.cc
index 56de26af64..172e218e0e 100644
--- a/src/components/policy/policy_external/src/policy_manager_impl.cc
+++ b/src/components/policy/policy_external/src/policy_manager_impl.cc
@@ -1790,11 +1790,6 @@ StatusNotifier PolicyManagerImpl::AddApplication(
                                                device_consent);
   } else {
     PromoteExistedApplication(application_id, device_consent);
-    if (helpers::in_range(hmi_types, policy_table::AHT_NAVIGATION) &&
-        !HasCertificate()) {
-      LOG4CXX_DEBUG(logger_, "Certificate does not exist, scheduling update.");
-      update_status_manager_.ScheduleUpdate();
-    }
     return utils::MakeShared<utils::CallNothing>();
   }
 }
diff --git a/src/components/policy/policy_regular/src/policy_manager_impl.cc b/src/components/policy/policy_regular/src/policy_manager_impl.cc
index 7c6c1173f0..42276e8bc5 100644
--- a/src/components/policy/policy_regular/src/policy_manager_impl.cc
+++ b/src/components/policy/policy_regular/src/policy_manager_impl.cc
@@ -219,10 +219,8 @@ bool PolicyManagerImpl::LoadPT(const std::string& file,
       return false;
     }
 
-    if (pt_update->policy_table.module_config.certificate.is_initialized()) {
-      listener_->OnCertificateUpdated(
-          *(pt_update->policy_table.module_config.certificate));
-    }
+    listener_->OnCertificateUpdated(
+        *(pt_update->policy_table.module_config.certificate));
 
     std::map<std::string, StringArray> app_hmi_types;
     cache_->GetHMIAppTypeAfterUpdate(app_hmi_types);
@@ -1082,13 +1080,6 @@ StatusNotifier PolicyManagerImpl::AddApplication(
                                                device_consent);
   } else {
     PromoteExistedApplication(application_id, device_consent);
-    const policy_table::AppHMIType type = policy_table::AHT_NAVIGATION;
-    if (helpers::in_range(hmi_types,
-                          (rpc::Enum<policy_table::AppHMIType>)type) &&
-        !HasCertificate()) {
-      LOG4CXX_DEBUG(logger_, "Certificate does not exist, scheduling update.");
-      update_status_manager_.ScheduleUpdate();
-    }
     return utils::MakeShared<utils::CallNothing>();
   }
 }
diff --git a/src/components/protocol_handler/include/protocol_handler/handshake_handler.h b/src/components/protocol_handler/include/protocol_handler/handshake_handler.h
index 5d536eebad..2d7d5c148c 100644
--- a/src/components/protocol_handler/include/protocol_handler/handshake_handler.h
+++ b/src/components/protocol_handler/include/protocol_handler/handshake_handler.h
@@ -91,10 +91,9 @@ class HandshakeHandler : public security_manager::SecurityManagerListener {
 
   /**
    * @brief Notification about handshake failure
-   * Gets parameters from payload and processes
-   * handshake failure procedure
+   * @return true on success notification handling or false otherwise
    */
-  void OnHandshakeFailed() OVERRIDE;
+  bool OnHandshakeFailed() OVERRIDE;
 
   /**
    * @brief Notification that certificate update is required.
diff --git a/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h b/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h
index 4d86a78688..e03e29d9bc 100644
--- a/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h
+++ b/src/components/protocol_handler/include/protocol_handler/protocol_handler_impl.h
@@ -685,12 +685,6 @@ class ProtocolHandlerImpl
 
 #ifdef ENABLE_SECURITY
   security_manager::SecurityManager* security_manager_;
-
-  bool is_ptu_triggered_;
-  std::list<std::shared_ptr<HandshakeHandler> > ptu_pending_handlers_;
-  std::list<std::shared_ptr<HandshakeHandler> > handshake_handlers_;
-  sync_primitives::Lock ptu_handlers_lock_;
-  sync_primitives::Lock handshake_handlers_lock_;
 #endif  // ENABLE_SECURITY
 
   // Thread that pumps non-parsed messages coming from mobile side.
diff --git a/src/components/protocol_handler/src/handshake_handler.cc b/src/components/protocol_handler/src/handshake_handler.cc
index 74987ac2bd..24c3127743 100644
--- a/src/components/protocol_handler/src/handshake_handler.cc
+++ b/src/components/protocol_handler/src/handshake_handler.cc
@@ -92,7 +92,7 @@ bool HandshakeHandler::GetPolicyCertificateData(std::string& data) const {
 
 void HandshakeHandler::OnCertificateUpdateRequired() {}
 
-void HandshakeHandler::OnHandshakeFailed() {
+bool HandshakeHandler::OnHandshakeFailed() {
   BsonObject params;
   if (payload_) {
     params = bson_object_from_bytes(payload_.get());
@@ -101,6 +101,7 @@ void HandshakeHandler::OnHandshakeFailed() {
   }
   ProcessFailedHandshake(params);
   bson_object_deinitialize(&params);
+  return true;
 }
 
 bool HandshakeHandler::OnHandshakeDone(
diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc
index dcb7999ef9..4cc4f883f0 100644
--- a/src/components/protocol_handler/src/protocol_handler_impl.cc
+++ b/src/components/protocol_handler/src/protocol_handler_impl.cc
@@ -75,7 +75,6 @@ ProtocolHandlerImpl::ProtocolHandlerImpl(
     ,
 #ifdef ENABLE_SECURITY
     security_manager_(NULL)
-    , is_ptu_triggered_(false)
     ,
 #endif  // ENABLE_SECURITY
     raw_ford_messages_from_mobile_(
@@ -149,7 +148,6 @@ ProtocolHandlerImpl::~ProtocolHandlerImpl() {
                  "Not all observers have unsubscribed"
                  " from ProtocolHandlerImpl");
   }
-  handshake_handlers_.clear();
 }
 
 void ProtocolHandlerImpl::AddProtocolObserver(ProtocolObserver* observer) {
@@ -842,70 +840,10 @@ void ProtocolHandlerImpl::OnConnectionClosed(
 
 void ProtocolHandlerImpl::NotifyOnFailedHandshake() {
   LOG4CXX_AUTO_TRACE(logger_);
-  sync_primitives::AutoLock lock(handshake_handlers_lock_);
-
-  std::for_each(
-      handshake_handlers_.begin(),
-      handshake_handlers_.end(),
-      std::bind(&HandshakeHandler::OnHandshakeFailed, std::placeholders::_1));
-
-  handshake_handlers_.clear();
+  security_manager_->NotifyListenersOnHandshakeFailed();
 }
 
-void ProtocolHandlerImpl::OnPTUFinished(const bool ptu_result) {
-  LOG4CXX_AUTO_TRACE(logger_);
-
-#ifdef ENABLE_SECURITY
-  sync_primitives::AutoLock lock(ptu_handlers_lock_);
-
-  if (!is_ptu_triggered_) {
-    LOG4CXX_ERROR(logger_,
-                  "PTU was not triggered by service starting. Ignored");
-    return;
-  }
-
-  for (auto handler : ptu_pending_handlers_) {
-    const bool is_cert_expired = security_manager_->IsCertificateUpdateRequired(
-        handler->connection_key());
-    security_manager::SSLContext* ssl_context =
-        is_cert_expired ? NULL
-                        : security_manager_->CreateSSLContext(
-                              handler->connection_key(),
-                              security_manager::SecurityManager::kUseExisting);
-
-    if (!ssl_context) {
-      const std::string error("CreateSSLContext failed");
-      LOG4CXX_ERROR(logger_, error);
-      security_manager_->SendInternalError(
-          handler->connection_key(),
-          security_manager::SecurityManager::ERROR_INTERNAL,
-          error);
-
-      handler->OnHandshakeDone(
-          handler->connection_key(),
-          security_manager::SSLContext::Handshake_Result_Fail);
-
-      continue;
-    }
-
-    if (ssl_context->IsInitCompleted()) {
-      handler->OnHandshakeDone(
-          handler->connection_key(),
-          security_manager::SSLContext::Handshake_Result_Success);
-    } else {
-      security_manager_->AddListener(new HandshakeHandler(*handler));
-      if (!ssl_context->IsHandshakePending()) {
-        // Start handshake process
-        security_manager_->StartHandshake(handler->connection_key());
-      }
-    }
-  }
-
-  LOG4CXX_DEBUG(logger_, "Handshake handlers were notified");
-  ptu_pending_handlers_.clear();
-  is_ptu_triggered_ = false;
-#endif  // ENABLE_SECURITY
-}
+void ProtocolHandlerImpl::OnPTUFinished(const bool ptu_result) {}
 
 RESULT_CODE ProtocolHandlerImpl::SendFrame(const ProtocolFramePtr packet) {
   LOG4CXX_AUTO_TRACE(logger_);
@@ -1579,40 +1517,10 @@ void ProtocolHandlerImpl::NotifySessionStarted(
                                            context,
                                            packet->protocol_version(),
                                            bson_object_bytes);
-    handshake_handlers_.push_back(handler);
-
-    const bool is_certificate_empty =
-        security_manager_->IsPolicyCertificateDataEmpty();
-
-    if (context.is_ptu_required_ && is_certificate_empty) {
-      LOG4CXX_DEBUG(logger_,
-                    "PTU for StartSessionHandler "
-                        << handler.get()
-                        << " is required and certificate data is empty");
-
-      sync_primitives::AutoLock lock(ptu_handlers_lock_);
-      if (!is_ptu_triggered_) {
-        LOG4CXX_DEBUG(logger_,
-                      "PTU is not triggered yet. "
-                          << "Starting PTU and postponing SSL handshake");
-
-        ptu_pending_handlers_.push_back(handler);
-        is_ptu_triggered_ = true;
-        security_manager_->NotifyOnCertificateUpdateRequired();
-        security_manager_->PostponeHandshake(connection_key);
-      } else {
-        LOG4CXX_DEBUG(logger_, "PTU has been triggered. Added to pending.");
-        ptu_pending_handlers_.push_back(handler);
-      }
-      return;
-    }
 
     security_manager::SSLContext* ssl_context =
-        is_certificate_empty
-            ? NULL
-            : security_manager_->CreateSSLContext(
-                  connection_key,
-                  security_manager::SecurityManager::kUseExisting);
+        security_manager_->CreateSSLContext(
+            connection_key, security_manager::SecurityManager::kUseExisting);
     if (!ssl_context) {
       const std::string error("CreateSSLContext failed");
       LOG4CXX_ERROR(logger_, error);
@@ -1646,12 +1554,18 @@ void ProtocolHandlerImpl::NotifySessionStarted(
                           *fullVersion,
                           *start_session_ack_params);
     } else {
-      LOG4CXX_DEBUG(logger_, "Adding Handshake handler to listenets:");
-      security_manager_->AddListener(new HandshakeHandler(*handler));
+      LOG4CXX_DEBUG(logger_,
+                    "Adding Handshake handler to listeners: " << handler.get());
+      security_manager::SecurityManagerListener* listener =
+          new HandshakeHandler(*handler);
+      security_manager_->AddListener(listener);
+
       if (!ssl_context->IsHandshakePending()) {
         // Start handshake process
         security_manager_->StartHandshake(connection_key);
+
         if (!security_manager_->IsSystemTimeProviderReady()) {
+          security_manager_->RemoveListener(listener);
           SendStartSessionNAck(context.connection_id_,
                                packet->session_id(),
                                protocol_version,
@@ -1660,6 +1574,7 @@ void ProtocolHandlerImpl::NotifySessionStarted(
         }
       }
     }
+
     LOG4CXX_DEBUG(logger_,
                   "Protection establishing for connection "
                       << connection_key << " is in progress");
diff --git a/src/components/security_manager/include/security_manager/security_manager_impl.h b/src/components/security_manager/include/security_manager/security_manager_impl.h
index ab093cc576..dc0284c128 100644
--- a/src/components/security_manager/include/security_manager/security_manager_impl.h
+++ b/src/components/security_manager/include/security_manager/security_manager_impl.h
@@ -202,6 +202,11 @@ class SecurityManagerImpl : public SecurityManager,
   void NotifyOnCertificateUpdateRequired() OVERRIDE;
 
   /**
+   * @brief Notify all listeners that handshake was failed
+   */
+  void NotifyListenersOnHandshakeFailed() OVERRIDE;
+
+  /**
    * @brief Check is policy certificate data is empty
    * @return true if policy certificate data is not empty otherwise false
    */
diff --git a/src/components/security_manager/src/security_manager_impl.cc b/src/components/security_manager/src/security_manager_impl.cc
index d68e6b456e..66a85956a5 100644
--- a/src/components/security_manager/src/security_manager_impl.cc
+++ b/src/components/security_manager/src/security_manager_impl.cc
@@ -214,7 +214,7 @@ void SecurityManagerImpl::ResumeHandshake(uint32_t connection_key) {
   }
 
   ssl_context->ResetConnection();
-  if (!ssl_context->HasCertificate()) {
+  if (!waiting_for_certificate_ && !ssl_context->HasCertificate()) {
     NotifyListenersOnHandshakeDone(connection_key,
                                    SSLContext::Handshake_Result_Fail);
     return;
@@ -416,6 +416,20 @@ void SecurityManagerImpl::NotifyOnCertificateUpdateRequired() {
   }
 }
 
+void SecurityManagerImpl::NotifyListenersOnHandshakeFailed() {
+  LOG4CXX_AUTO_TRACE(logger_);
+  std::list<SecurityManagerListener*>::iterator it = listeners_.begin();
+  while (it != listeners_.end()) {
+    if ((*it)->OnHandshakeFailed()) {
+      LOG4CXX_DEBUG(logger_, "Destroying listener: " << *it);
+      delete (*it);
+      it = listeners_.erase(it);
+    } else {
+      ++it;
+    }
+  }
+}
+
 bool SecurityManagerImpl::IsPolicyCertificateDataEmpty() {
   LOG4CXX_AUTO_TRACE(logger_);