Hotfix: Secure simple participants with initialpeers over TCP mat…

…ch (#5071) * Refs #20181: Add BB test Signed-off-by: Mario Dominguez <[email protected]> * Refs #20181: Add Fix Signed-off-by: Mario Dominguez <[email protected]> * Refs #20181: linter Signed-off-by: Mario Dominguez <[email protected]> * Refs #20181. Pass in secure_endpoints as lambda capture. Signed-off-by: Miguel Company <[email protected]> * Refs #20181. New approach. Automatically sending DATA(p) when receiving a DATA(p) could lead to an infinite ping-pong between the two participants. This resulted in some cases in the transport threads eating all CPU resources. The new approach matches the discovered participant to the builtin non-secure PDP writer, so it will receive the DATA(p) of the local participant in the next periodic announcement. Signed-off-by: Miguel Company <[email protected]> * Refs #20181. Unmatch non-secure before matching secure. Signed-off-by: Miguel Company <[email protected]> --------- Signed-off-by: Mario Dominguez <[email protected]> Signed-off-by: Miguel Company <[email protected]> Co-authored-by: Miguel Company <[email protected]> (cherry picked from commit 3ca60e0) # Conflicts: # src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp # test/blackbox/common/BlackboxTestsSecurity.cpp
eProsima · Aug 27, 2024 · 215add6 · 215add6
1 parent 67bc44d
commit 215add6
Show file tree

Hide file tree

Showing 3 changed files with 113 additions and 8 deletions.
diff --git a/include/fastdds/rtps/builtin/discovery/participant/PDPSimple.h b/include/fastdds/rtps/builtin/discovery/participant/PDPSimple.h
@@ -136,7 +136,16 @@ class PDPSimple : public PDP
 
     void match_pdp_remote_endpoints(
             const ParticipantProxyData& pdata,
-            bool notify_secure_endpoints);
+            bool notify_secure_endpoints,
+            bool writer_only);
+
+    /**
+     * @brief Unmatch PDP endpoints with a remote participant.
+     *
+     * @param participant_guid GUID of the remote participant.
+     */
+    void unmatch_pdp_remote_endpoints(
+            const GUID_t& participant_guid);
 
     void assign_low_level_remote_endpoints(
             const ParticipantProxyData& pdata,

diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp
@@ -303,7 +303,11 @@ bool PDPSimple::createPDPEndpoints()
         secure_endpoints->secure_reader.listener_.reset(new PDPListener(this));
 
         endpoints = secure_endpoints;
-        endpoints->reader.listener_.reset(new PDPSecurityInitiatorListener(this));
+        endpoints->reader.listener_.reset(new PDPSecurityInitiatorListener(this,
+                [this](const ParticipantProxyData& participant_data)
+                {
+                    match_pdp_remote_endpoints(participant_data, false, true);
+                }));
     }
     else
 #endif  // HAVE_SECURITY
@@ -509,7 +513,7 @@ void PDPSimple::assignRemoteEndpoints(
         {
             // This participant is not secure.
             // Match PDP and other builtin endpoints.
-            match_pdp_remote_endpoints(*pdata, false);
+            match_pdp_remote_endpoints(*pdata, false, false);
             assign_low_level_remote_endpoints(*pdata, false);
         }
     }
@@ -519,8 +523,13 @@ void PDPSimple::removeRemoteEndpoints(
         ParticipantProxyData* pdata)
 {
     EPROSIMA_LOG_INFO(RTPS_PDP, "For RTPSParticipant: " << pdata->m_guid);
+    unmatch_pdp_remote_endpoints(pdata->m_guid);
+}
 
-    GUID_t guid = pdata->m_guid;
+void PDPSimple::unmatch_pdp_remote_endpoints(
+        const GUID_t& participant_guid)
+{
+    GUID_t guid = participant_guid;
 
     {
         auto endpoints = dynamic_cast<fastdds::rtps::SimplePDPEndpoints*>(builtin_endpoints_.get());
@@ -552,7 +561,8 @@ void PDPSimple::notifyAboveRemoteEndpoints(
 {
     if (notify_secure_endpoints)
     {
-        match_pdp_remote_endpoints(pdata, true);
+        unmatch_pdp_remote_endpoints(pdata.m_guid);
+        match_pdp_remote_endpoints(pdata, true, false);
     }
     else
     {
@@ -565,7 +575,7 @@ void PDPSimple::notifyAboveRemoteEndpoints(
             notify_and_maybe_ignore_new_participant(part_data, ignored);
             if (!ignored)
             {
-                match_pdp_remote_endpoints(*part_data, false);
+                match_pdp_remote_endpoints(*part_data, false, false);
                 assign_low_level_remote_endpoints(*part_data, false);
             }
         }
@@ -575,7 +585,8 @@ void PDPSimple::notifyAboveRemoteEndpoints(
 
 void PDPSimple::match_pdp_remote_endpoints(
         const ParticipantProxyData& pdata,
-        bool notify_secure_endpoints)
+        bool notify_secure_endpoints,
+        bool writer_only)
 {
 #if !HAVE_SECURITY
     static_cast<void>(notify_secure_endpoints);
@@ -612,7 +623,7 @@ void PDPSimple::match_pdp_remote_endpoints(
     }
 #endif // HAVE_SECURITY
 
-    if (0 != (endp & pdp_writer_mask))
+    if (!writer_only && (0 != (endp & pdp_writer_mask)))
     {
         auto temp_writer_data = get_temporary_writer_proxies_pool().get();
 
@@ -670,7 +681,11 @@ void PDPSimple::match_pdp_remote_endpoints(
             writer->matched_reader_add(*temp_reader_data);
         }
 
+<<<<<<< HEAD
         if (BEST_EFFORT_RELIABILITY_QOS == reliability_kind)
+=======
+        if (!writer_only && (dds::BEST_EFFORT_RELIABILITY_QOS == reliability_kind))
+>>>>>>> 3ca60e061 (Hotfix: Secure simple participants with `initialpeers` over `TCP` match (#5071))
         {
             endpoints->writer.writer_->unsent_changes_reset();
         }

diff --git a/test/blackbox/common/BlackboxTestsSecurity.cpp b/test/blackbox/common/BlackboxTestsSecurity.cpp
@@ -21,6 +21,16 @@
 #include <fstream>
 #include <map>
 
+<<<<<<< HEAD
+=======
+#include <fastdds/dds/domain/DomainParticipantFactory.hpp>
+#include <fastdds/dds/log/Log.hpp>
+#include <fastdds/LibrarySettings.hpp>
+#include <fastdds/rtps/common/EntityId_t.hpp>
+#include <fastdds/rtps/transport/shared_mem/SharedMemTransportDescriptor.hpp>
+#include <fastdds/rtps/transport/test_UDPv4TransportDescriptor.hpp>
+#include <fastdds/utils/IPFinder.hpp>
+>>>>>>> 3ca60e061 (Hotfix: Secure simple participants with `initialpeers` over `TCP` match (#5071))
 #include <gtest/gtest.h>
 
 #include "PubSubReader.hpp"
@@ -5014,6 +5024,77 @@ TEST(Security, ValidateAuthenticationHandshakeProperties)
     ASSERT_TRUE(auth_elapsed_time < max_time);
 }
 
+// Regression test for Redmine issue #20181
+// Two simple secure participants with tcp transport and initial peers must match.
+// It basically tests that the PDPSecurityInitiatorListener
+// in PDPSimple answers back with the proxy data.
+TEST(Security, security_with_initial_peers_over_tcpv4_correctly_behaves)
+{
+    // Create
+    PubSubWriter<HelloWorldPubSubType> tcp_client("HelloWorldTopic_TCP");
+    PubSubReader<HelloWorldPubSubType> tcp_server("HelloWorldTopic_TCP");
+
+    // Search for a valid WAN address
+    LocatorList_t all_locators;
+    Locator_t wan_locator;
+    IPFinder::getIP4Address(&all_locators);
+
+    for (auto& locator : all_locators)
+    {
+        if (!IPLocator::isLocal(locator))
+        {
+            wan_locator = locator;
+            break;
+        }
+    }
+
+    uint16_t server_listening_port = 11810;
+    wan_locator.port = server_listening_port;
+    wan_locator.kind = LOCATOR_KIND_TCPv4;
+
+    auto tcp_client_transport_descriptor = std::make_shared<eprosima::fastdds::rtps::TCPv4TransportDescriptor>();
+    LocatorList_t initial_peers;
+    initial_peers.push_back(wan_locator);
+    tcp_client.disable_builtin_transport()
+            .add_user_transport_to_pparams(tcp_client_transport_descriptor)
+            .initial_peers(initial_peers);
+
+    auto tcp_server_transport_descriptor = std::make_shared<eprosima::fastdds::rtps::TCPv4TransportDescriptor>();
+    tcp_server_transport_descriptor->listening_ports.push_back(server_listening_port);
+    IPLocator::copyIPv4(wan_locator, tcp_server_transport_descriptor->wan_addr);
+
+    std::cout << "SETTING WAN address to " <<  wan_locator << std::endl;
+
+    tcp_server.disable_builtin_transport()
+            .add_user_transport_to_pparams(tcp_server_transport_descriptor);
+
+    // Configure security
+    const std::string governance_file("governance_helloworld_all_enable.smime");
+    const std::string permissions_file("permissions_helloworld.smime");
+    CommonPermissionsConfigure(tcp_server, tcp_client, governance_file, permissions_file);
+
+    tcp_server.init();
+    tcp_client.init();
+
+    ASSERT_TRUE(tcp_server.isInitialized());
+    ASSERT_TRUE(tcp_client.isInitialized());
+
+    tcp_server.waitAuthorized();
+    tcp_client.waitAuthorized();
+
+    tcp_server.wait_discovery();
+    tcp_client.wait_discovery();
+
+    ASSERT_TRUE(tcp_server.is_matched());
+    ASSERT_TRUE(tcp_client.is_matched());
+
+    auto data = default_helloworld_data_generator();
+    tcp_server.startReception(data);
+    tcp_client.send(data);
+    ASSERT_TRUE(data.empty());
+    tcp_server.block_for_all(std::chrono::seconds(10));
+}
+
 
 void blackbox_security_init()
 {