Add missing internal error codes to security manager (#3773)

* Add missing internal error codes to security manager

* Add internal error notification to SSL handshake failure

* Remove unused handshake result

* Add unit tests for new error codes

Co-authored-by: JackLivio <jack@livio.io>

src/components/application_manager/src/application_manager_impl.cc

@@ -2447,7 +2447,6 @@ bool ApplicationManagerImpl::OnHandshakeDone(
           SSLContext::Handshake_Result_CertExpired,
           SSLContext::Handshake_Result_CertNotSigned,
           SSLContext::Handshake_Result_AppIDMismatch,
-          SSLContext::Handshake_Result_AppNameMismatch,
           SSLContext::Handshake_Result_NotYetValid)) {
     app->usage_report().RecordTLSError();
   }

src/components/include/security_manager/security_manager.h

@@ -69,6 +69,9 @@ class SecurityManager : public protocol_handler::ProtocolObserver,
     ERROR_DECRYPTION_FAILED = 0x06,
     ERROR_ENCRYPTION_FAILED = 0x07,
     ERROR_SSL_INVALID_DATA = 0x08,
+    ERROR_HANDSHAKE_FAILED = 0x09,  // Handshake failed
+    ERROR_INVALID_CERT = 0x0A,      // Handshake failed because cert is invalid
+    ERROR_EXPIRED_CERT = 0x0B,      // Handshake failed because cert is expired
     ERROR_INTERNAL = 0xFF,
     ERROR_UNKNOWN_INTERNAL_ERROR = 0xFE  // error value for testing
   };
@@ -99,24 +102,24 @@ class SecurityManager : public protocol_handler::ProtocolObserver,
    * \param connection_key Unique key used by other components as session
    * identifier
    * \param error_id  unique error identifier
-   * \param erorr_text SSL impelmentation error text
+   * \param error_text SSL impelmentation error text
    * \param seq_number received from Mobile Application
    */
   virtual void SendInternalError(const uint32_t connection_key,
                                  const uint8_t& error_id,
-                                 const std::string& erorr_text,
+                                 const std::string& error_text,
                                  const uint32_t seq_number) = 0;
   /**
    * \brief Sends InternalError override methode for sending without seq_number
    * \param connection_key Unique key used by other components as session
    * identifier
    * \param error_id  unique error identifier
-   * \param erorr_text SSL impelmentation error text
+   * \param error_text SSL impelmentation error text
    */
   void SendInternalError(const uint32_t connection_key,
                          const uint8_t& error_id,
-                         const std::string& erorr_text) {
-    SendInternalError(connection_key, error_id, erorr_text, 0);
+                         const std::string& error_text) {
+    SendInternalError(connection_key, error_id, error_text, 0);
   }
 
   /**

src/components/include/security_manager/ssl_context.h

@@ -73,7 +73,6 @@ class SSLContext {
     Handshake_Result_NotYetValid,
     Handshake_Result_CertNotSigned,
     Handshake_Result_AppIDMismatch,
-    Handshake_Result_AppNameMismatch,
   };
 
   struct HandshakeContext {

src/components/protocol_handler/src/handshake_handler.cc

@@ -149,8 +149,6 @@ bool HandshakeHandler::OnHandshakeDone(
             return "Certificate is not signed";
           case security_manager::SSLContext::Handshake_Result_AppIDMismatch:
             return "Trying to run handshake with wrong app id";
-          case security_manager::SSLContext::Handshake_Result_AppNameMismatch:
-            return "Trying to run handshake with wrong app name";
           case security_manager::SSLContext::Handshake_Result_AbnormalFail:
             return "Error occurred during handshake";
           case security_manager::SSLContext::Handshake_Result_Fail:

src/components/security_manager/include/security_manager/security_manager_impl.h

@@ -121,12 +121,12 @@ class SecurityManagerImpl : public SecurityManager,
    * \param connection_key Unique key used by other components as session
    * identifier
    * \param error_id  unique error identifier
-   * \param erorr_text SSL impelmentation error text
+   * \param error_text SSL impelmentation error text
    * \param seq_number received from Mobile Application
    */
   void SendInternalError(const uint32_t connection_key,
                          const uint8_t& error_id,
-                         const std::string& erorr_text,
+                         const std::string& error_text,
                          const uint32_t seq_number) OVERRIDE;
 
   using SecurityManager::SendInternalError;

src/components/security_manager/src/security_manager_impl.cc

@@ -557,6 +557,7 @@ bool SecurityManagerImpl::ProcessHandshakeData(
     // no handshake data to send
     return false;
   }
+
   if (sslContext->IsInitCompleted()) {
     // On handshake success
     SDL_LOG_DEBUG("SSL initialization finished success.");
@@ -565,6 +566,29 @@ bool SecurityManagerImpl::ProcessHandshakeData(
   } else if (handshake_result != SSLContext::Handshake_Result_Success) {
     // On handshake fail
     SDL_LOG_WARN("SSL initialization finished with fail.");
+    int32_t error_code = ERROR_HANDSHAKE_FAILED;
+    std::string error_text = "Handshake failed";
+    switch (handshake_result) {
+      case SSLContext::Handshake_Result_CertExpired:
+        error_code = ERROR_EXPIRED_CERT;
+        error_text = "Certificate is expired";
+        break;
+      case SSLContext::Handshake_Result_NotYetValid:
+        error_code = ERROR_INVALID_CERT;
+        error_text = "Certificate is not yet valid";
+        break;
+      case SSLContext::Handshake_Result_CertNotSigned:
+        error_code = ERROR_INVALID_CERT;
+        error_text = "Certificate is not signed";
+        break;
+      case SSLContext::Handshake_Result_AppIDMismatch:
+        error_code = ERROR_INVALID_CERT;
+        error_text = "App ID does not match certificate";
+        break;
+      default:
+        break;
+    }
+    SendInternalError(connection_key, error_code, error_text);
     NotifyListenersOnHandshakeDone(connection_key, handshake_result);
   }
 

src/components/security_manager/test/security_manager_test.cc

@@ -730,27 +730,28 @@ TEST_F(SecurityManagerTest, ProcessHandshakeData_InvalidData) {
 }
 /*
  * Shall send HandshakeData on getting SEND_HANDSHAKE_DATA from mobile side
- * with correct handshake data Check Fail and sussecc states
+ * with correct handshake data Check Fail and success states
  */
 TEST_F(SecurityManagerTest, ProcessHandshakeData_Answer) {
   SetMockCryptoManager();
   // Count handshake calls
   const int handshake_emulates = 2;
+  const int internal_error_count = 1;
 
   uint32_t connection_id = 0;
   uint8_t session_id = 0;
 
   auto waiter = TestAsyncWaiter::createInstance();
   uint32_t times = 0;
   EXPECT_CALL(mock_session_observer, PairFromKey(kKey, _, _))
-      .Times(handshake_emulates)
+      .Times(handshake_emulates + internal_error_count)
       .WillRepeatedly(NotifyTestAsyncWaiter(waiter));
   times += handshake_emulates;
   EXPECT_CALL(mock_session_observer,
               ProtocolVersionUsed(connection_id, session_id, An<uint8_t&>()))
-      .Times(handshake_emulates)
+      .Times(handshake_emulates + internal_error_count)
       .WillRepeatedly(DoAll(NotifyTestAsyncWaiter(waiter), Return(true)));
-  times += handshake_emulates;
+  times += handshake_emulates + internal_error_count;
 
   // Get size of raw message after
   const size_t raw_message_size = 15;
@@ -759,7 +760,15 @@ TEST_F(SecurityManagerTest, ProcessHandshakeData_Answer) {
                   RawMessageEqSize(raw_message_size), false, kIsFinal))
       .Times(handshake_emulates)
       .WillRepeatedly(NotifyTestAsyncWaiter(waiter));
-  times += handshake_emulates;
+  EXPECT_CALL(
+      mock_protocol_handler,
+      SendMessageToMobileApp(
+          InternalErrorWithErrId(SecurityManager::ERROR_HANDSHAKE_FAILED),
+          false,
+          kIsFinal))
+      .Times(internal_error_count)
+      .WillRepeatedly(NotifyTestAsyncWaiter(waiter));
+  times += handshake_emulates + internal_error_count;
 
   // Expect notifying listeners (unsuccess)
   EXPECT_CALL(*mock_sm_listener,
@@ -803,6 +812,97 @@ TEST_F(SecurityManagerTest, ProcessHandshakeData_Answer) {
   // Listener was destroyed after OnHandshakeDone call
   mock_sm_listener.release();
 }
+/*
+ * Shall send HandshakeData on getting SEND_HANDSHAKE_DATA from mobile side
+ * with correct handshake data Check Fail and success states
+ */
+TEST_F(SecurityManagerTest, ProcessHandshakeData_Answer_Invalid_Cert) {
+  SetMockCryptoManager();
+  // Count handshake calls
+  const int handshake_emulates = 4;
+
+  uint32_t connection_id = 0;
+  uint8_t session_id = 0;
+
+  auto waiter = TestAsyncWaiter::createInstance();
+  uint32_t times = 0;
+  // Each of these calls is run twice, once for the next handshake data request,
+  // once for the the internal error notification
+  EXPECT_CALL(mock_session_observer, PairFromKey(kKey, _, _))
+      .Times(handshake_emulates)
+      .WillRepeatedly(NotifyTestAsyncWaiter(waiter));
+  times += handshake_emulates;
+  EXPECT_CALL(mock_session_observer,
+              ProtocolVersionUsed(connection_id, session_id, An<uint8_t&>()))
+      .Times(handshake_emulates)
+      .WillRepeatedly(DoAll(NotifyTestAsyncWaiter(waiter), Return(true)));
+  times += handshake_emulates;
+
+  EXPECT_CALL(mock_protocol_handler,
+              SendMessageToMobileApp(
+                  InternalErrorWithErrId(SecurityManager::ERROR_EXPIRED_CERT),
+                  false,
+                  kIsFinal))
+      .WillOnce(NotifyTestAsyncWaiter(waiter));
+  EXPECT_CALL(mock_protocol_handler,
+              SendMessageToMobileApp(
+                  InternalErrorWithErrId(SecurityManager::ERROR_INVALID_CERT),
+                  false,
+                  kIsFinal))
+      .Times(3)
+      .WillRepeatedly(NotifyTestAsyncWaiter(waiter));
+  times += 4;
+
+  // Listener is erased after first call
+  EXPECT_CALL(*mock_sm_listener,
+              OnHandshakeDone(kKey, SSLContext::Handshake_Result_CertExpired))
+      .WillRepeatedly(DoAll(NotifyTestAsyncWaiter(waiter), Return(true)));
+  times++;
+
+  // Emulate SessionObserver and CryptoManager result
+  EXPECT_CALL(mock_ssl_context_exists, IsInitCompleted())
+      .Times(handshake_emulates)
+      .WillRepeatedly(DoAll(NotifyTestAsyncWaiter(waiter), Return(false)));
+  times += handshake_emulates;
+  EXPECT_CALL(mock_session_observer, GetSSLContext(kKey, kControl))
+      .Times(handshake_emulates)
+      .WillRepeatedly(DoAll(NotifyTestAsyncWaiter(waiter),
+                            Return(&mock_ssl_context_exists)));
+  times += handshake_emulates;
+
+  // Emulate DoHandshakeStep correct logics
+  EXPECT_CALL(
+      mock_ssl_context_exists,
+      DoHandshakeStep(HandshakeStepEq(handshake_data, handshake_data_size),
+                      handshake_data_size,
+                      _,
+                      _))
+      .WillOnce(DoAll(SetArgPointee<2>((uint8_t*)NULL),
+                      SetArgPointee<3>(0),
+                      NotifyTestAsyncWaiter(waiter),
+                      Return(SSLContext::Handshake_Result_CertExpired)))
+      .WillOnce(DoAll(SetArgPointee<2>((uint8_t*)NULL),
+                      SetArgPointee<3>(0),
+                      NotifyTestAsyncWaiter(waiter),
+                      Return(SSLContext::Handshake_Result_NotYetValid)))
+      .WillOnce(DoAll(SetArgPointee<2>((uint8_t*)NULL),
+                      SetArgPointee<3>(0),
+                      NotifyTestAsyncWaiter(waiter),
+                      Return(SSLContext::Handshake_Result_AppIDMismatch)))
+      .WillOnce(DoAll(SetArgPointee<2>((uint8_t*)NULL),
+                      SetArgPointee<3>(0),
+                      NotifyTestAsyncWaiter(waiter),
+                      Return(SSLContext::Handshake_Result_CertNotSigned)));
+  times += 4;  // matches to each single call above
+
+  EmulateMobileMessageHandshake(
+      handshake_data, handshake_data_size, handshake_emulates);
+
+  EXPECT_TRUE(waiter->WaitFor(times, kAsyncExpectationsTimeout));
+
+  // Listener was destroyed after OnHandshakeDone call
+  mock_sm_listener.release();
+}
 /*
  * Shall call all listeners on success end handshake
  * and return handshake data