V5 Protocol Messages do not encrypt payload of Control Frame Message Typesfix/fix_ssl_handshake_flow_integration_branch

Updated encrypt/decrypt frame conditions
Fixed bson object double allocation

4 files changed, 70 insertions, 40 deletions

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 8800808b2e..c3a575e758 100644
--- a/src/components/protocol_handler/include/protocol_handler/handshake_handler.h
+++ b/src/components/protocol_handler/include/protocol_handler/handshake_handler.h
@@ -61,14 +61,14 @@ class HandshakeHandler : public security_manager::SecurityManagerListener {
                               const std::vector<int>& force_protected_service,
                               const bool is_new_service,
                               ProtocolPacket::ProtocolVersion& full_version,
-                              std::shared_ptr<uint8_t> payload);
+                              std::shared_ptr<BsonObject> payload);
 
   HandshakeHandler(ProtocolHandlerImpl& protocol_handler,
                    SessionObserver& session_observer,
                    ProtocolPacket::ProtocolVersion& full_version,
                    const SessionContext& context,
                    const uint8_t protocol_version,
-                   std::shared_ptr<uint8_t> payload);
+                   std::shared_ptr<BsonObject> payload);
 
   ~HandshakeHandler();
 
@@ -119,7 +119,7 @@ class HandshakeHandler : public security_manager::SecurityManagerListener {
   SessionContext context_;
   ProtocolPacket::ProtocolVersion full_version_;
   const uint8_t protocol_version_;
-  std::shared_ptr<uint8_t> payload_;
+  std::shared_ptr<BsonObject> payload_;
 };
 
 }  // namespace protocol_handler
diff --git a/src/components/protocol_handler/src/handshake_handler.cc b/src/components/protocol_handler/src/handshake_handler.cc
index bfde94faf1..1d40618126 100644
--- a/src/components/protocol_handler/src/handshake_handler.cc
+++ b/src/components/protocol_handler/src/handshake_handler.cc
@@ -55,7 +55,7 @@ HandshakeHandler::HandshakeHandler(
     const std::vector<int>& force_protected_service,
     const bool is_new_service,
     ProtocolPacket::ProtocolVersion& full_version,
-    std::shared_ptr<uint8_t> payload)
+    std::shared_ptr<BsonObject> payload)
     : protocol_handler_(protocol_handler)
     , session_observer_(session_observer)
     , context_()
@@ -69,7 +69,7 @@ HandshakeHandler::HandshakeHandler(
     ProtocolPacket::ProtocolVersion& full_version,
     const SessionContext& context,
     const uint8_t protocol_version,
-    std::shared_ptr<uint8_t> payload)
+    std::shared_ptr<BsonObject> payload)
     : protocol_handler_(protocol_handler)
     , session_observer_(session_observer)
     , context_(context)
@@ -89,14 +89,15 @@ uint32_t HandshakeHandler::connection_key() const {
 void HandshakeHandler::OnCertificateUpdateRequired() {}
 
 bool HandshakeHandler::OnHandshakeFailed() {
-  BsonObject params;
   if (payload_) {
-    params = bson_object_from_bytes(payload_.get());
+    ProcessFailedHandshake(*payload_);
   } else {
+    BsonObject params;
     bson_object_initialize_default(&params);
+    ProcessFailedHandshake(params);
+    bson_object_deinitialize(&params);
   }
-  ProcessFailedHandshake(params);
-  bson_object_deinitialize(&params);
+
   return true;
 }
 
@@ -118,20 +119,23 @@ bool HandshakeHandler::OnHandshakeDone(
   const bool success =
       result == security_manager::SSLContext::Handshake_Result_Success;
 
-  BsonObject params;
   if (payload_) {
-    params = bson_object_from_bytes(payload_.get());
+    if (success) {
+      ProcessSuccessfulHandshake(connection_key, *payload_);
+    } else {
+      ProcessFailedHandshake(*payload_);
+    }
   } else {
+    BsonObject params;
     bson_object_initialize_default(&params);
+    if (success) {
+      ProcessSuccessfulHandshake(connection_key, params);
+    } else {
+      ProcessFailedHandshake(params);
+    }
+    bson_object_deinitialize(&params);
   }
 
-  if (success) {
-    ProcessSuccessfulHandshake(connection_key, params);
-  } else {
-    ProcessFailedHandshake(params);
-  }
-
-  bson_object_deinitialize(&params);
   return true;
 }
 
diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc
index 083172d8dc..e26e9a1aad 100644
--- a/src/components/protocol_handler/src/protocol_handler_impl.cc
+++ b/src/components/protocol_handler/src/protocol_handler_impl.cc
@@ -39,6 +39,7 @@
 #include "connection_handler/connection_handler_impl.h"
 #include "protocol_handler/session_observer.h"
 #include "utils/byte_order.h"
+#include "utils/helpers.h"
 #include "protocol/common.h"
 
 #ifdef ENABLE_SECURITY
@@ -278,16 +279,28 @@ void ProtocolHandlerImpl::SendStartSessionAck(
   if (ack_protocol_version >= PROTOCOL_VERSION_5) {
     ServiceType serviceTypeValue = ServiceTypeFromByte(service_type);
 
-    bson_object_put_int64(
+    const bool mtu_written = bson_object_put_int64(
         &params,
         strings::mtu,
         static_cast<int64_t>(
             protocol_header_validator_.max_payload_size_by_service_type(
                 serviceTypeValue)));
+    LOG4CXX_DEBUG(logger_,
+                  "MTU parameter was written to bson params: "
+                      << mtu_written << "; Value: "
+                      << static_cast<int32_t>(
+                             bson_object_get_int64(&params, strings::mtu)));
+
     if (serviceTypeValue == kRpc) {
       // Hash ID is only used in RPC case
-      bson_object_put_int32(
+      const bool hash_written = bson_object_put_int32(
           &params, strings::hash_id, static_cast<int32_t>(hash_id));
+      LOG4CXX_DEBUG(logger_,
+                    "Hash parameter was written to bson params: "
+                        << hash_written << "; Value: "
+                        << static_cast<int32_t>(bson_object_get_int32(
+                               &params, strings::hash_id)));
+
       // Minimum protocol version supported by both
       ProtocolPacket::ProtocolVersion* minVersion =
           (full_version.majorVersion < PROTOCOL_VERSION_5)
@@ -296,8 +309,14 @@ void ProtocolHandlerImpl::SendStartSessionAck(
                                                      defaultProtocolVersion);
       char protocolVersionString[256];
       strncpy(protocolVersionString, (*minVersion).to_string().c_str(), 255);
-      bson_object_put_string(
+
+      const bool protocol_ver_written = bson_object_put_string(
           &params, strings::protocol_version, protocolVersionString);
+      LOG4CXX_DEBUG(
+          logger_,
+          "Protocol version parameter was written to bson params: "
+              << protocol_ver_written << "; Value: "
+              << bson_object_get_string(&params, strings::protocol_version));
     }
     uint8_t* payloadBytes = bson_object_to_bytes(&params);
     ptr->set_data(payloadBytes, bson_object_size(&params));
@@ -1506,17 +1525,13 @@ void ProtocolHandlerImpl::NotifySessionStarted(
     const uint32_t connection_key = session_observer_.KeyFromPair(
         context.connection_id_, context.new_session_id_);
 
-    std::shared_ptr<uint8_t> bson_object_bytes(
-        bson_object_to_bytes(start_session_ack_params.get()),
-        [](uint8_t* p) { delete[] p; });
-
     std::shared_ptr<HandshakeHandler> handler =
         std::make_shared<HandshakeHandler>(*this,
                                            session_observer_,
                                            *fullVersion,
                                            context,
                                            packet->protocol_version(),
-                                           bson_object_bytes);
+                                           start_session_ack_params);
 
     security_manager::SSLContext* ssl_context =
         security_manager_->CreateSSLContext(
@@ -1785,7 +1800,9 @@ RESULT_CODE ProtocolHandlerImpl::EncryptFrame(ProtocolFramePtr packet) {
   DCHECK(packet);
   // Control frames and data over control service shall be unprotected
   if (packet->service_type() == kControl ||
-      packet->frame_type() == FRAME_TYPE_CONTROL) {
+      // For protocol v5 control frames could be protected
+      (packet->frame_type() == FRAME_TYPE_CONTROL &&
+       packet->protocol_version() < PROTOCOL_VERSION_5)) {
     return RESULT_OK;
   }
   if (!security_manager_) {
@@ -1828,12 +1845,30 @@ RESULT_CODE ProtocolHandlerImpl::EncryptFrame(ProtocolFramePtr packet) {
 
 RESULT_CODE ProtocolHandlerImpl::DecryptFrame(ProtocolFramePtr packet) {
   DCHECK(packet);
-  if (!packet->protection_flag() ||
-      // Control frames and data over control service shall be unprotected
-      packet->service_type() == kControl ||
-      packet->frame_type() == FRAME_TYPE_CONTROL) {
+
+  bool shoud_not_decrypt;
+  if (packet->protocol_version() >= PROTOCOL_VERSION_5) {
+    // For v5 protocol control frames except StartService could be encrypted
+    shoud_not_decrypt =
+        !packet->protection_flag() || packet->service_type() == kControl ||
+        (FRAME_TYPE_CONTROL == packet->frame_type() &&
+         helpers::Compare<ServiceType, helpers::EQ, helpers::ONE>(
+             static_cast<ServiceType>(packet->service_type()),
+             kMobileNav,
+             kAudio,
+             kRpc));
+  } else {
+    // Control frames and data over control service shall be unprotected
+    shoud_not_decrypt = !packet->protection_flag() ||
+                        packet->service_type() == kControl ||
+                        packet->frame_type() == FRAME_TYPE_CONTROL;
+  }
+
+  if (shoud_not_decrypt) {
+    LOG4CXX_DEBUG(logger_, "Frame will not be decrypted");
     return RESULT_OK;
   }
+
   if (!security_manager_) {
     LOG4CXX_WARN(logger_, "No security_manager_ set.");
     return RESULT_FAIL;
diff --git a/src/components/security_manager/test/ssl_certificate_handshake_test.cc b/src/components/security_manager/test/ssl_certificate_handshake_test.cc
index d5bb1ecaee..83ffa33b44 100644
--- a/src/components/security_manager/test/ssl_certificate_handshake_test.cc
+++ b/src/components/security_manager/test/ssl_certificate_handshake_test.cc
@@ -534,15 +534,6 @@ TEST_P(SSLHandshakeTest, AppNameAndAppIDInvalid) {
 
   client_ctx_->SetHandshakeContext(
       security_manager::SSLContext::HandshakeContext(
-          custom_str::CustomString("server"),
-          custom_str::CustomString("Wrong")));
-
-  GTEST_TRACE(HandshakeProcedure_ClientSideFail(
-      security_manager::SSLContext::Handshake_Result_AppNameMismatch));
-
-  ResetConnections();
-  client_ctx_->SetHandshakeContext(
-      security_manager::SSLContext::HandshakeContext(
           custom_str::CustomString("Wrong"),
           custom_str::CustomString("server")));