From 6f39dc2d9077cb0d78288519e1acf859eca89df8 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 18 Mar 2024 14:23:29 +0100 Subject: [PATCH] Protect asio exception hotfix (#4527) (#4532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Protect asio exception hotfix (#4527) * Refs #20599: Handle error code before function call Signed-off-by: cferreiragonz * Apply suggestion Co-authored-by: Miguel Company --------- Signed-off-by: cferreiragonz Co-authored-by: Miguel Company (cherry picked from commit 08193d5f7eb4dcf3de8ecee3725f57bee28d35cd) # Conflicts: # src/cpp/rtps/transport/TCPTransportInterface.cpp * Backport of #4319 && Partial backport of #4300 Signed-off-by: cferreiragonz --------- Signed-off-by: cferreiragonz Co-authored-by: Carlos Ferreira González --- src/cpp/rtps/transport/TCPChannelResource.h | 26 +++++++++ .../transport/TCPChannelResourceBasic.cpp | 13 ++++- .../rtps/transport/TCPChannelResourceBasic.h | 7 +++ .../transport/TCPChannelResourceSecure.cpp | 12 +++++ .../rtps/transport/TCPChannelResourceSecure.h | 7 +++ .../rtps/transport/TCPTransportInterface.cpp | 54 +++++++++++++++---- .../rtps/transport/TCPTransportInterface.h | 12 +++++ .../transport/mock/MockTCPChannelResource.cpp | 16 ++++++ .../transport/mock/MockTCPChannelResource.h | 6 +++ 9 files changed, 143 insertions(+), 10 deletions(-) diff --git a/src/cpp/rtps/transport/TCPChannelResource.h b/src/cpp/rtps/transport/TCPChannelResource.h index 4bceb696bf2..1f795c37c6e 100644 --- a/src/cpp/rtps/transport/TCPChannelResource.h +++ b/src/cpp/rtps/transport/TCPChannelResource.h @@ -130,10 +130,36 @@ class TCPChannelResource : public ChannelResource size_t size, asio::error_code& ec) = 0; + /** + * @brief Gets the remote endpoint of the socket connection. + * @throws Exception on failure. + * @return asio::ip::tcp::endpoint of the remote endpoint. + */ virtual asio::ip::tcp::endpoint remote_endpoint() const = 0; + /** + * @brief Gets the local endpoint of the socket connection. + * @throws Exception on failure. + * @return asio::ip::tcp::endpoint of the local endpoint. + */ virtual asio::ip::tcp::endpoint local_endpoint() const = 0; + /** + * @brief Gets the remote endpoint, setting error code if any. + * @param ec Set to indicate what error occurred, if any. + * @return asio::ip::tcp::endpoint of the remote endpoint or returns a default-constructed endpoint object if an error occurred. + */ + virtual asio::ip::tcp::endpoint remote_endpoint( + asio::error_code& ec) const = 0; + + /** + * @brief Gets the local endpoint, setting error code if any. + * @param ec Set to indicate what error occurred, if any. + * @return asio::ip::tcp::endpoint of the remote endpoint or returns a default-constructed endpoint object if an error occurred. + */ + virtual asio::ip::tcp::endpoint local_endpoint( + asio::error_code& ec) const = 0; + virtual void set_options( const TCPTransportDescriptor* options) = 0; diff --git a/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp b/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp index b9b4c1336ed..51a28913c19 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp +++ b/src/cpp/rtps/transport/TCPChannelResourceBasic.cpp @@ -182,7 +182,18 @@ asio::ip::tcp::endpoint TCPChannelResourceBasic::remote_endpoint() const asio::ip::tcp::endpoint TCPChannelResourceBasic::local_endpoint() const { - std::error_code ec; + return socket_->local_endpoint(); +} + +asio::ip::tcp::endpoint TCPChannelResourceBasic::remote_endpoint( + asio::error_code& ec) const +{ + return socket_->remote_endpoint(ec); +} + +asio::ip::tcp::endpoint TCPChannelResourceBasic::local_endpoint( + asio::error_code& ec) const +{ return socket_->local_endpoint(ec); } diff --git a/src/cpp/rtps/transport/TCPChannelResourceBasic.h b/src/cpp/rtps/transport/TCPChannelResourceBasic.h index 5e7940663e1..20928f0f494 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceBasic.h +++ b/src/cpp/rtps/transport/TCPChannelResourceBasic.h @@ -65,9 +65,16 @@ class TCPChannelResourceBasic : public TCPChannelResource size_t size, asio::error_code& ec) override; + // Throwing asio calls asio::ip::tcp::endpoint remote_endpoint() const override; asio::ip::tcp::endpoint local_endpoint() const override; + // Non-throwing asio calls + asio::ip::tcp::endpoint remote_endpoint( + asio::error_code& ec) const override; + asio::ip::tcp::endpoint local_endpoint( + asio::error_code& ec) const override; + void set_options( const TCPTransportDescriptor* options) override; diff --git a/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp b/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp index ff12d3c26cc..c16eca7c020 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp +++ b/src/cpp/rtps/transport/TCPChannelResourceSecure.cpp @@ -264,6 +264,18 @@ asio::ip::tcp::endpoint TCPChannelResourceSecure::local_endpoint() const return secure_socket_->lowest_layer().local_endpoint(); } +asio::ip::tcp::endpoint TCPChannelResourceSecure::remote_endpoint( + asio::error_code& ec) const +{ + return secure_socket_->lowest_layer().remote_endpoint(ec); +} + +asio::ip::tcp::endpoint TCPChannelResourceSecure::local_endpoint( + asio::error_code& ec) const +{ + return secure_socket_->lowest_layer().local_endpoint(ec); +} + void TCPChannelResourceSecure::set_options( const TCPTransportDescriptor* options) { diff --git a/src/cpp/rtps/transport/TCPChannelResourceSecure.h b/src/cpp/rtps/transport/TCPChannelResourceSecure.h index 04ac935c437..31226292b4b 100644 --- a/src/cpp/rtps/transport/TCPChannelResourceSecure.h +++ b/src/cpp/rtps/transport/TCPChannelResourceSecure.h @@ -65,9 +65,16 @@ class TCPChannelResourceSecure : public TCPChannelResource size_t size, asio::error_code& ec) override; + // Throwing asio calls asio::ip::tcp::endpoint remote_endpoint() const override; asio::ip::tcp::endpoint local_endpoint() const override; + // Non-throwing asio calls + asio::ip::tcp::endpoint remote_endpoint( + asio::error_code& ec) const override; + asio::ip::tcp::endpoint local_endpoint( + asio::error_code& ec) const override; + void set_options( const TCPTransportDescriptor* options) override; diff --git a/src/cpp/rtps/transport/TCPTransportInterface.cpp b/src/cpp/rtps/transport/TCPTransportInterface.cpp index f4e7b6ec698..c31e5bd068e 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.cpp +++ b/src/cpp/rtps/transport/TCPTransportInterface.cpp @@ -224,6 +224,40 @@ void TCPTransportInterface::clean() } } +Locator TCPTransportInterface::remote_endpoint_to_locator( + const std::shared_ptr& channel) const +{ + Locator locator; + asio::error_code ec; + auto endpoint = channel->remote_endpoint(ec); + if (ec) + { + LOCATOR_INVALID(locator); + } + else + { + endpoint_to_locator(endpoint, locator); + } + return locator; +} + +Locator TCPTransportInterface::local_endpoint_to_locator( + const std::shared_ptr& channel) const +{ + Locator locator; + asio::error_code ec; + auto endpoint = channel->local_endpoint(ec); + if (ec) + { + LOCATOR_INVALID(locator); + } + else + { + endpoint_to_locator(endpoint, locator); + } + return locator; +} + void TCPTransportInterface::bind_socket( std::shared_ptr& channel) { @@ -881,10 +915,11 @@ void TCPTransportInterface::perform_listen_operation( if (rtcp_message_manager) { channel = channel_weak.lock(); - endpoint_to_locator(channel->remote_endpoint(), remote_locator); if (channel) { + remote_locator = remote_endpoint_to_locator(channel); + if (channel->tcp_connection_type() == TCPChannelResource::TCPConnectionType::TCP_CONNECT_TYPE) { rtcp_message_manager->sendConnectionRequest(channel); @@ -1153,6 +1188,11 @@ bool TCPTransportInterface::Receive( } else { + if (!IsLocatorValid(remote_locator)) + { + remote_locator = remote_endpoint_to_locator(channel); + } + IPLocator::setLogicalPort(remote_locator, tcp_header.logical_port); EPROSIMA_LOG_INFO(RTCP_MSG_IN, "[RECEIVE] From: " << remote_locator \ << " - " << receive_buffer_size << " bytes."); @@ -1370,10 +1410,8 @@ void TCPTransportInterface::SocketAccepted( channel_weak_ptr, rtcp_manager_weak_ptr)); EPROSIMA_LOG_INFO(RTCP, "Accepted connection (local: " - << channel->local_endpoint().address() << ":" - << channel->local_endpoint().port() << "), remote: " - << channel->remote_endpoint().address() << ":" - << channel->remote_endpoint().port() << ")"); + << local_endpoint_to_locator(channel) << ", remote: " + << remote_endpoint_to_locator(channel) << ")"); } else { @@ -1418,10 +1456,8 @@ void TCPTransportInterface::SecureSocketAccepted( channel_weak_ptr, rtcp_manager_weak_ptr)); EPROSIMA_LOG_INFO(RTCP, " Accepted connection (local: " - << socket->lowest_layer().local_endpoint().address() << ":" - << socket->lowest_layer().local_endpoint().port() << "), remote: " - << socket->lowest_layer().remote_endpoint().address() << ":" - << socket->lowest_layer().remote_endpoint().port() << ")"); + << local_endpoint_to_locator(secure_channel) << ", remote: " + << remote_endpoint_to_locator(secure_channel) << ")"); } else { diff --git a/src/cpp/rtps/transport/TCPTransportInterface.h b/src/cpp/rtps/transport/TCPTransportInterface.h index e83ffc9b018..67792c2212d 100644 --- a/src/cpp/rtps/transport/TCPTransportInterface.h +++ b/src/cpp/rtps/transport/TCPTransportInterface.h @@ -187,6 +187,18 @@ class TCPTransportInterface : public TransportInterface const asio::ip::tcp::endpoint& endpoint, Locator& locator) const = 0; + /** + * Converts a remote endpoint to a locator if possible. Otherwise, it sets an invalid locator. + */ + Locator remote_endpoint_to_locator( + const std::shared_ptr& channel) const; + + /** + * Converts a local endpoint to a locator if possible. Otherwise, it sets an invalid locator. + */ + Locator local_endpoint_to_locator( + const std::shared_ptr& channel) const; + /** * Shutdown method to close the connections of the transports. */ diff --git a/test/unittest/transport/mock/MockTCPChannelResource.cpp b/test/unittest/transport/mock/MockTCPChannelResource.cpp index db80e81982c..c22ad835f0e 100644 --- a/test/unittest/transport/mock/MockTCPChannelResource.cpp +++ b/test/unittest/transport/mock/MockTCPChannelResource.cpp @@ -67,6 +67,22 @@ asio::ip::tcp::endpoint MockTCPChannelResource::local_endpoint() const return ep; } +asio::ip::tcp::endpoint MockTCPChannelResource::remote_endpoint( + asio::error_code& ec) const +{ + ec = asio::error_code(); // Indicate no error + asio::ip::tcp::endpoint ep; + return ep; +} + +asio::ip::tcp::endpoint MockTCPChannelResource::local_endpoint( + asio::error_code& ec) const +{ + ec = asio::error_code(); // Indicate no error + asio::ip::tcp::endpoint ep; + return ep; +} + void MockTCPChannelResource::set_options( const TCPTransportDescriptor*) { diff --git a/test/unittest/transport/mock/MockTCPChannelResource.h b/test/unittest/transport/mock/MockTCPChannelResource.h index b48bf087ddb..ecdb186a0fc 100644 --- a/test/unittest/transport/mock/MockTCPChannelResource.h +++ b/test/unittest/transport/mock/MockTCPChannelResource.h @@ -59,6 +59,12 @@ class MockTCPChannelResource : public TCPChannelResource asio::ip::tcp::endpoint local_endpoint() const override; + asio::ip::tcp::endpoint remote_endpoint( + asio::error_code& ec) const override; + + asio::ip::tcp::endpoint local_endpoint( + asio::error_code& ec) const override; + void set_options( const TCPTransportDescriptor* options) override;