From d1bd37932229925b1d929e63f0a06e80caecda96 Mon Sep 17 00:00:00 2001 From: Andriy Byzhynar Date: Mon, 19 Feb 2018 18:34:55 +0200 Subject: Fix decrypted multiframe message handling Fixed SDL behavior in case when FIRST_FRAME received from mobile was encrypted --- .../include/protocol_handler/protocol_packet.h | 6 ++++++ .../protocol_handler/src/multiframe_builder.cc | 2 ++ .../protocol_handler/src/protocol_handler_impl.cc | 6 ++++++ .../protocol_handler/src/protocol_packet.cc | 24 +++++++++++++++------- .../security_manager/src/crypto_manager_impl.cc | 5 +++-- .../security_manager/src/security_manager_impl.cc | 3 +-- .../security_manager/src/ssl_context_impl.cc | 11 ++++++++-- 7 files changed, 44 insertions(+), 13 deletions(-) diff --git a/src/components/protocol_handler/include/protocol_handler/protocol_packet.h b/src/components/protocol_handler/include/protocol_handler/protocol_packet.h index 1c427533e6..b6c05d4c46 100644 --- a/src/components/protocol_handler/include/protocol_handler/protocol_packet.h +++ b/src/components/protocol_handler/include/protocol_handler/protocol_packet.h @@ -251,6 +251,12 @@ class ProtocolPacket { RESULT_CODE deserializePacket(const uint8_t* message, const size_t messageSize); + /** + * @brief Calculates FIRST_FRAME data for further handling of consecutive + * frames + */ + void HandleRawFirstFrameData(const uint8_t* message); + /** * \brief Getter of protocol version. */ diff --git a/src/components/protocol_handler/src/multiframe_builder.cc b/src/components/protocol_handler/src/multiframe_builder.cc index 5a1fc6d205..cf8a23ddc1 100644 --- a/src/components/protocol_handler/src/multiframe_builder.cc +++ b/src/components/protocol_handler/src/multiframe_builder.cc @@ -91,6 +91,8 @@ bool MultiFrameBuilder::RemoveConnection(const ConnectionID connection_id) { ProtocolFramePtrList MultiFrameBuilder::PopMultiframes() { LOG4CXX_AUTO_TRACE(logger_); LOG4CXX_DEBUG(logger_, "Current state is: " << multiframes_map_); + LOG4CXX_DEBUG(logger_, + "Current multiframe map size is: " << multiframes_map_.size()); ProtocolFramePtrList outpute_frame_list; for (MultiFrameMap::iterator connection_it = multiframes_map_.begin(); connection_it != multiframes_map_.end(); diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc index 9db6355d50..6ab63cbdda 100644 --- a/src/components/protocol_handler/src/protocol_handler_impl.cc +++ b/src/components/protocol_handler/src/protocol_handler_impl.cc @@ -1718,6 +1718,7 @@ RESULT_CODE ProtocolHandlerImpl::HandleControlMessageHeartBeat( } void ProtocolHandlerImpl::PopValideAndExpirateMultiframes() { + LOG4CXX_AUTO_TRACE(logger_); const ProtocolFramePtrList& frame_list = multiframe_builder_.PopMultiframes(); for (ProtocolFramePtrList::const_iterator it = frame_list.begin(); it != frame_list.end(); @@ -1964,6 +1965,11 @@ RESULT_CODE ProtocolHandlerImpl::DecryptFrame(ProtocolFramePtr packet) { << out_data_size << " bytes"); DCHECK(out_data); DCHECK(out_data_size); + // Special handling for decrypted FIRST_FRAME + if (packet->frame_type() == FRAME_TYPE_FIRST && packet->protection_flag()) { + packet->HandleRawFirstFrameData(out_data); + return RESULT_OK; + } packet->set_data(out_data, out_data_size); return RESULT_OK; } diff --git a/src/components/protocol_handler/src/protocol_packet.cc b/src/components/protocol_handler/src/protocol_packet.cc index ae52849de6..a490916c99 100644 --- a/src/components/protocol_handler/src/protocol_packet.cc +++ b/src/components/protocol_handler/src/protocol_packet.cc @@ -520,6 +520,17 @@ bool ProtocolPacket::operator==(const ProtocolPacket& other) const { return false; } +void ProtocolPacket::HandleRawFirstFrameData(const uint8_t* message) { + LOG4CXX_AUTO_TRACE(logger_); + payload_size_ = 0; + const uint8_t* data = message; + uint32_t total_data_bytes = data[0] << 24; + total_data_bytes |= data[1] << 16; + total_data_bytes |= data[2] << 8; + total_data_bytes |= data[3]; + set_total_data_bytes(total_data_bytes); +} + RESULT_CODE ProtocolPacket::deserializePacket(const uint8_t* message, const size_t messageSize) { LOG4CXX_AUTO_TRACE(logger_); @@ -532,18 +543,15 @@ RESULT_CODE ProtocolPacket::deserializePacket(const uint8_t* message, packet_data_.totalDataBytes = packet_header_.dataSize; uint32_t dataPayloadSize = 0; - if ((offset < messageSize) && packet_header_.frameType != FRAME_TYPE_FIRST) { + if ((offset < messageSize)) { dataPayloadSize = messageSize - offset; } - if (packet_header_.frameType == FRAME_TYPE_FIRST) { + if (packet_header_.frameType == FRAME_TYPE_FIRST && + !packet_header_.protection_flag) { payload_size_ = 0; const uint8_t* data = message + offset; - uint32_t total_data_bytes = data[0] << 24; - total_data_bytes |= data[1] << 16; - total_data_bytes |= data[2] << 8; - total_data_bytes |= data[3]; - set_total_data_bytes(total_data_bytes); + HandleRawFirstFrameData(data); if (0 == packet_data_.data) { return RESULT_FAIL; } @@ -602,6 +610,8 @@ uint8_t* ProtocolPacket::data() const { } void ProtocolPacket::set_total_data_bytes(size_t dataBytes) { + LOG4CXX_AUTO_TRACE(logger_); + LOG4CXX_DEBUG(logger_, "Data bytes : " << dataBytes); if (dataBytes) { delete[] packet_data_.data; packet_data_.data = new (std::nothrow) uint8_t[dataBytes]; diff --git a/src/components/security_manager/src/crypto_manager_impl.cc b/src/components/security_manager/src/crypto_manager_impl.cc index c901f75c8b..d83c84bf40 100644 --- a/src/components/security_manager/src/crypto_manager_impl.cc +++ b/src/components/security_manager/src/crypto_manager_impl.cc @@ -277,13 +277,14 @@ bool CryptoManagerImpl::OnCertificateUpdated(const std::string& data) { } SSLContext* CryptoManagerImpl::CreateSSLContext() { - if (context_ == NULL) { + if (NULL == context_) { return NULL; } SSL* conn = SSL_new(context_); - if (conn == NULL) + if (NULL == conn) { return NULL; + } if (get_settings().security_manager_mode() == SERVER) { SSL_set_accept_state(conn); diff --git a/src/components/security_manager/src/security_manager_impl.cc b/src/components/security_manager/src/security_manager_impl.cc index 401491c5dc..4608a99d5b 100644 --- a/src/components/security_manager/src/security_manager_impl.cc +++ b/src/components/security_manager/src/security_manager_impl.cc @@ -154,7 +154,6 @@ security_manager::SSLContext* SecurityManagerImpl::CreateSSLContext( DCHECK(session_observer_); DCHECK(crypto_manager_); - security_manager::SSLContext* ssl_context = NULL; if (kUseExisting == cc_strategy) { security_manager::SSLContext* ssl_context = session_observer_->GetSSLContext(connection_key, @@ -165,7 +164,7 @@ security_manager::SSLContext* SecurityManagerImpl::CreateSSLContext( } } - ssl_context = crypto_manager_->CreateSSLContext(); + security_manager::SSLContext* ssl_context = crypto_manager_->CreateSSLContext(); if (!ssl_context) { const std::string error_text("CryptoManager could not create SSL context."); LOG4CXX_ERROR(logger_, error_text); diff --git a/src/components/security_manager/src/ssl_context_impl.cc b/src/components/security_manager/src/ssl_context_impl.cc index 75e2c8a4f8..756ec37164 100644 --- a/src/components/security_manager/src/ssl_context_impl.cc +++ b/src/components/security_manager/src/ssl_context_impl.cc @@ -500,25 +500,32 @@ bool CryptoManagerImpl::SSLContextImpl::Decrypt(const uint8_t* const in_data, size_t in_data_size, const uint8_t** const out_data, size_t* out_data_size) { + LOG4CXX_AUTO_TRACE(logger_); sync_primitives::AutoLock locker(bio_locker); if (!SSL_is_init_finished(connection_)) { + LOG4CXX_ERROR(logger_, "SSL initilization is not finished"); return false; } - if (!in_data || !in_data_size) { + if (!in_data || (0 == in_data_size)) { + LOG4CXX_ERROR(logger_, "IN data ptr or IN data size is 0"); return false; } + BIO_write(bioIn_, in_data, in_data_size); int len = BIO_ctrl_pending(bioFilter_); + ptrdiff_t offset = 0; *out_data_size = 0; - while (len) { + *out_data = NULL; + while (len > 0) { EnsureBufferSizeEnough(len + offset); len = BIO_read(bioFilter_, buffer_ + offset, len); // TODO(EZamakhov): investigate BIO_read return 0, -1 and -2 meanings if (len <= 0) { // Reset filter and connection deinitilization instead + LOG4CXX_ERROR(logger_, "Read error occured. Read data lenght : " << len); BIO_ctrl(bioFilter_, BIO_CTRL_RESET, 0, NULL); return false; } -- cgit v1.2.1