Merge pull request #2235 from smartdevicelink/fix/fix_v5_protocol_messages_bson_paramsfeature/new_vehicle_data_turn_signal

Fix V5 protocol message bson params

4 files changed, 82 insertions, 31 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 2d7d5c148c..8b7f28d50e 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();
 
@@ -126,7 +126,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 24c3127743..8db551cfd6 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)
@@ -93,14 +93,15 @@ bool HandshakeHandler::GetPolicyCertificateData(std::string& data) 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;
 }
 
@@ -122,20 +123,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 86dbb604fa..a2766f724e 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));
@@ -1504,17 +1523,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/protocol_handler/test/protocol_handler_tm_test.cc b/src/components/protocol_handler/test/protocol_handler_tm_test.cc
index 075b332345..23e4201968 100644
--- a/src/components/protocol_handler/test/protocol_handler_tm_test.cc
+++ b/src/components/protocol_handler/test/protocol_handler_tm_test.cc
@@ -623,6 +623,18 @@ TEST_F(ProtocolHandlerImplTest,
   const ::transport_manager::ConnectionUID connection_id2 = 0xBu;
   const uint8_t session_id2 = 2u;
 
+#ifdef ENABLE_SECURITY
+  AddSecurityManager();
+
+  EXPECT_CALL(session_observer_mock, KeyFromPair(connection_id2, session_id2))
+      .WillOnce(Return(connection_key));
+
+  EXPECT_CALL(session_observer_mock,
+              GetSSLContext(connection_key, start_service))
+      .Times(2)
+      .WillRepeatedly(ReturnNull());
+#endif  // ENABLE_SECURITY
+
   EXPECT_CALL(session_observer_mock, IsHeartBeatSupported(connection_id1, _))
       .WillRepeatedly(Return(false));
   EXPECT_CALL(session_observer_mock, IsHeartBeatSupported(connection_id2, _))