From 518df4697965ba6481932af3a9f77b1bd49487f0 Mon Sep 17 00:00:00 2001 From: "Andrii Kalinich (GitHub)" Date: Wed, 9 Oct 2019 07:41:52 -0700 Subject: Fix SSL handshake for a secondary connection (#3065) * Fix SSL handshake for a secondary connection There was found a several places in SDL code where SSL handshake was working wrong when secured service was started from a secondary connection like WIFI. All these places were updated to consider primary connection as a source for an SSL context and other attributes for case when the current connection is secondary. This is a part which was not considered during initial implementation of the secondary transport feature. * fixup! Fix SSL handshake for a secondary connection * fixup! Fix SSL handshake for a secondary connection --- .../connection_handler/connection_handler_impl.h | 9 +++ .../src/connection_handler_impl.cc | 87 ++++++++++++++-------- .../include/protocol_handler/handshake_handler.h | 6 ++ .../protocol_handler/src/handshake_handler.cc | 11 ++- 4 files changed, 81 insertions(+), 32 deletions(-) diff --git a/src/components/connection_handler/include/connection_handler/connection_handler_impl.h b/src/components/connection_handler/include/connection_handler/connection_handler_impl.h index cca11e5f57..b7b791a5c1 100644 --- a/src/components/connection_handler/include/connection_handler/connection_handler_impl.h +++ b/src/components/connection_handler/include/connection_handler/connection_handler_impl.h @@ -620,6 +620,15 @@ class ConnectionHandlerImpl const uint8_t GetSessionIdFromSecondaryTransport( transport_manager::ConnectionUID secondary_transport_id) const; + /** + * @brief Get pointer to the primary connection by connection handle + * @param connection_handle handle of the current connection + * @return pointer to the primary connection if current one is secondary + * otherwise returns pointer to the same connection + */ + Connection* GetPrimaryConnection( + const ConnectionHandle connection_handle) const; + const ConnectionHandlerSettings& settings_; /** * \brief Pointer to observer diff --git a/src/components/connection_handler/src/connection_handler_impl.cc b/src/components/connection_handler/src/connection_handler_impl.cc index d6c15c8e82..db1ee2dce5 100644 --- a/src/components/connection_handler/src/connection_handler_impl.cc +++ b/src/components/connection_handler/src/connection_handler_impl.cc @@ -1124,6 +1124,26 @@ const uint8_t ConnectionHandlerImpl::GetSessionIdFromSecondaryTransport( return 0; } +Connection* ConnectionHandlerImpl::GetPrimaryConnection( + const ConnectionHandle connection_handle) const { + LOG4CXX_DEBUG(logger_, + "Getting primary connection for ID " << connection_handle); + ConnectionList::const_iterator it = connection_list_.find(connection_handle); + if (connection_list_.end() == it) { + LOG4CXX_ERROR( + logger_, + "Connection with ID " << connection_handle << " was not found"); + return nullptr; + } + + auto connection_ptr = it->second; + if (connection_ptr->primary_connection_handle() != 0) { + return GetPrimaryConnection(connection_ptr->primary_connection_handle()); + } + + return connection_ptr; +} + std::string ConnectionHandlerImpl::GetCloudAppID( const transport_manager::ConnectionUID connection_id) const { sync_primitives::AutoLock auto_lock(cloud_app_id_map_lock_); @@ -1229,13 +1249,12 @@ int ConnectionHandlerImpl::SetSSLContext( PairFromKey(key, &connection_handle, &session_id); sync_primitives::AutoReadLock lock(connection_list_lock_); - ConnectionList::iterator it = connection_list_.find(connection_handle); - if (connection_list_.end() == it) { - LOG4CXX_ERROR(logger_, "Unknown connection!"); + auto connection = GetPrimaryConnection(connection_handle); + if (!connection) { return security_manager::SecurityManager::ERROR_INTERNAL; } - Connection& connection = *it->second; - return connection.SetSSLContext(session_id, context); + + return connection->SetSSLContext(session_id, context); } security_manager::SSLContext* ConnectionHandlerImpl::GetSSLContext( @@ -1246,13 +1265,12 @@ security_manager::SSLContext* ConnectionHandlerImpl::GetSSLContext( PairFromKey(key, &connection_handle, &session_id); sync_primitives::AutoReadLock lock(connection_list_lock_); - ConnectionList::iterator it = connection_list_.find(connection_handle); - if (connection_list_.end() == it) { - LOG4CXX_ERROR(logger_, "Unknown connection!"); - return NULL; + auto connection = GetPrimaryConnection(connection_handle); + if (!connection) { + return nullptr; } - Connection& connection = *it->second; - return connection.GetSSLContext(session_id, service_type); + + return connection->GetSSLContext(session_id, service_type); } void ConnectionHandlerImpl::SetProtectionFlag( @@ -1263,18 +1281,28 @@ void ConnectionHandlerImpl::SetProtectionFlag( PairFromKey(key, &connection_handle, &session_id); sync_primitives::AutoReadLock lock(connection_list_lock_); - ConnectionList::iterator it = connection_list_.find(connection_handle); - if (connection_list_.end() == it) { - LOG4CXX_ERROR(logger_, "Unknown connection!"); + auto connection = GetPrimaryConnection(connection_handle); + if (!connection) { return; } - Connection& connection = *it->second; - connection.SetProtectionFlag(session_id, service_type); + + connection->SetProtectionFlag(session_id, service_type); } security_manager::SSLContext::HandshakeContext ConnectionHandlerImpl::GetHandshakeContext(uint32_t key) const { - return connection_handler_observer_->GetHandshakeContext(key); + transport_manager::ConnectionUID connection_handle = 0; + uint8_t session_id = 0; + PairFromKey(key, &connection_handle, &session_id); + + sync_primitives::AutoReadLock lock(connection_list_lock_); + auto connection = GetPrimaryConnection(connection_handle); + if (!connection) { + return security_manager::SSLContext::HandshakeContext(); + } + + auto primary_key = KeyFromPair(connection->connection_handle(), session_id); + return connection_handler_observer_->GetHandshakeContext(primary_key); } #endif // ENABLE_SECURITY @@ -1664,9 +1692,9 @@ void ConnectionHandlerImpl::BindProtocolVersionWithSession( PairFromKey(connection_key, &connection_handle, &session_id); sync_primitives::AutoReadLock lock(connection_list_lock_); - ConnectionList::iterator it = connection_list_.find(connection_handle); - if (connection_list_.end() != it) { - it->second->UpdateProtocolVersionSession(session_id, protocol_version); + auto connection = GetPrimaryConnection(connection_handle); + if (connection) { + connection->UpdateProtocolVersionSession(session_id, protocol_version); } } @@ -1675,13 +1703,14 @@ bool ConnectionHandlerImpl::IsHeartBeatSupported( uint8_t session_id) const { LOG4CXX_AUTO_TRACE(logger_); sync_primitives::AutoReadLock lock(connection_list_lock_); - uint32_t connection = static_cast(connection_handle); - ConnectionList::const_iterator it = connection_list_.find(connection); - if (connection_list_.end() == it) { - LOG4CXX_WARN(logger_, "Connection not found !"); + const uint32_t connection_id = static_cast(connection_handle); + auto connection = GetPrimaryConnection(connection_id); + + if (!connection) { return false; } - return it->second->SupportHeartBeat(session_id); + + return connection->SupportHeartBeat(session_id); } bool ConnectionHandlerImpl::ProtocolVersionUsed( @@ -1690,15 +1719,15 @@ bool ConnectionHandlerImpl::ProtocolVersionUsed( uint8_t& protocol_version) const { LOG4CXX_AUTO_TRACE(logger_); sync_primitives::AutoReadLock lock(connection_list_lock_); - ConnectionList::const_iterator it = connection_list_.find(connection_id); - if (connection_list_.end() != it) { - return it->second->ProtocolVersion(session_id, protocol_version); + auto connection = GetPrimaryConnection(connection_id); + + if (connection) { + return connection->ProtocolVersion(session_id, protocol_version); } else if (ending_connection_ && static_cast(ending_connection_->connection_handle()) == connection_id) { return ending_connection_->ProtocolVersion(session_id, protocol_version); } - LOG4CXX_WARN(logger_, "Connection not found !"); return false; } 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 5be513049d..9552d4c420 100644 --- a/src/components/protocol_handler/include/protocol_handler/handshake_handler.h +++ b/src/components/protocol_handler/include/protocol_handler/handshake_handler.h @@ -111,6 +111,12 @@ class HandshakeHandler : public security_manager::SecurityManagerListener { */ uint32_t connection_key() const; + /** + * @brief Get primary connection key of this handler + * @return primary connection key + */ + uint32_t primary_connection_key() const; + private: /** * @brief Performs related actions if handshake was successfully finished diff --git a/src/components/protocol_handler/src/handshake_handler.cc b/src/components/protocol_handler/src/handshake_handler.cc index 669b73c18b..3b4ae6ef1f 100644 --- a/src/components/protocol_handler/src/handshake_handler.cc +++ b/src/components/protocol_handler/src/handshake_handler.cc @@ -69,6 +69,11 @@ uint32_t HandshakeHandler::connection_key() const { context_.new_session_id_); } +uint32_t HandshakeHandler::primary_connection_key() const { + return session_observer_.KeyFromPair(context_.primary_connection_id_, + context_.new_session_id_); +} + bool HandshakeHandler::GetPolicyCertificateData(std::string& data) const { return false; } @@ -120,11 +125,11 @@ bool HandshakeHandler::OnHandshakeDone( LOG4CXX_DEBUG(logger_, "OnHandshakeDone for service : " << context_.service_type_); - if (connection_key != this->connection_key()) { + if (connection_key != this->primary_connection_key()) { LOG4CXX_DEBUG(logger_, "Listener " << this << " expects notification for connection id: " - << this->connection_key() + << this->primary_connection_key() << ". Received notification for connection id " << connection_key << " will be ignored"); return false; @@ -161,7 +166,7 @@ bool HandshakeHandler::CanBeProtected() const { } bool HandshakeHandler::IsAlreadyProtected() const { - return (session_observer_.GetSSLContext(this->connection_key(), + return (session_observer_.GetSSLContext(this->primary_connection_key(), context_.service_type_) != NULL); } -- cgit v1.2.1