Automatic GUID mangling when using secure DS (#3178)

* Refs #16596: Added check_guid_comes_from() in SecurityManager Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Added check_guid_comes_from() to Authentication interface and PKIDH plugin Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Removed remote_readers vector<GUID> from PDPCLient.cpp Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Added data_matches_with_server() in PDPClient.cpp and PDPCLient.h Signed-off-by: Mario Dominguez <[email protected]> * Revert "Refs #16603. Temporarily using mangled prefix on system test." This reverts commit 1b68139. Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Added guids_mangling_info_ to SecurityManager Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Added public method get_remote_server_participant_proxy_data() and protected data_matches_with_server() in PDP.h PDP.cpp Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: data_matches_with_server() override in PDPClient Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Added data_matches_with_server in createParticipantProxyData() PDPClient.cpp Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: substitute mp_PDP->get_participant_proxy_data() by mp_PDP->get_remote_server_participant_proxy_data() in DSClientEvent.cpp Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Requested changes: Added notify_participant_authorized() method to postpone notifyboveendpoints() call. check_guids_comes_from() included in DiscoveredParticipantInfo. guids_mangling_info_ removed. Doxygen corrections. Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Requested changes: Renamed data_matches_with_server() to data_matches_with_prefix() and left only in PDP. Removed unnecessary method get_remote_server_participant_proxy_data(). Added data_matches_with_prefix() to get_participant_proxy_data(). DSClientEvent call reverted to get_participant_proxy_data() Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Requested changes: Restored previous TODO in idrect_send in PDPClient.cpp Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: PDPServer data_matches_with_prefix() translation Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Requested changes doxygen check_guid_comes_from() corrected Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Requested changes: add missing notify_participant_authorized() call && minor comments Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Uncrustify format changes (local) Signed-off-by: Mario Dominguez <[email protected]> * Refs #16596: Requested changes Signed-off-by: Mario Dominguez <[email protected]> Signed-off-by: Mario Dominguez <[email protected]>
eProsima · Jan 11, 2023 · c502517 · c502517
1 parent 05f6c61
commit c502517
Show file tree

Hide file tree

Showing 11 changed files with 209 additions and 38 deletions.
diff --git a/include/fastdds/rtps/builtin/discovery/participant/PDP.h b/include/fastdds/rtps/builtin/discovery/participant/PDP.h
@@ -451,6 +451,19 @@ class PDP
             bool with_lease_duration,
             const ParticipantProxyData* participant_proxy_data = nullptr);
 
+    /**
+     * Checks whether two participant prefixes are equal by calculating the mangled
+     * GUID and comparing it with the remote participant prefix.
+     *
+     * @param guid_prefix the original desired guid_prefix to compare
+     * @param participant_proxy_data The participant proxy data to compare against
+     *
+     * @return true when prefixes are equivalent
+     */
+    bool data_matches_with_prefix(
+            const GuidPrefix_t& guid_prefix,
+            const ParticipantProxyData& participant_data);
+
     /**
      * Gets the key of a participant proxy data.
      *

diff --git a/include/fastdds/rtps/security/authentication/Authentication.h b/include/fastdds/rtps/security/authentication/Authentication.h
@@ -249,6 +249,22 @@ class Authentication
             PermissionsCredentialToken* token,
             SecurityException& ex) = 0;
 
+    /**
+     * Returns whether a mangled GUID is the same as the original
+     * @param identity_handle Identity Handle of remote peer
+     * @param adjusted Mangled GUID prefix
+     * @param original Original GUID prefix candidate to compare
+     * @return true when @c adjusted corresponds to @c original
+     */
+    virtual bool check_guid_comes_from(
+            IdentityHandle* /*identity_handle*/,
+            const GUID_t& adjusted,
+            const GUID_t& original)
+    {
+        //! By default, return this comparison
+        return adjusted == original;
+    }
+
     bool set_logger(
             Logging* logger,
             SecurityException& /*exception*/)

diff --git a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp
@@ -219,6 +219,26 @@ ParticipantProxyData* PDP::add_participant_proxy_data(
     return ret_val;
 }
 
+bool PDP::data_matches_with_prefix(
+        const GuidPrefix_t& guid_prefix,
+        const ParticipantProxyData& participant_data)
+{
+    if (guid_prefix == participant_data.m_guid.guidPrefix)
+    {
+        return true;
+    }
+#ifdef HAVE_SECURITY
+    else
+    {
+        GUID_t guid = GUID_t(guid_prefix, c_EntityId_RTPSParticipant);
+        return getRTPSParticipant()->security_manager().check_guid_comes_from(participant_data.m_guid, guid);
+    }
+#endif  // HAVE_SECURITY
+
+    return false;
+
+}
+
 void PDP::initializeParticipantProxyData(
         ParticipantProxyData* participant_data)
 {
@@ -1108,7 +1128,7 @@ ParticipantProxyData* PDP::get_participant_proxy_data(
 {
     for (auto pit = ParticipantProxiesBegin(); pit != ParticipantProxiesEnd(); ++pit)
     {
-        if (guid_prefix == (*pit)->m_guid.guidPrefix)
+        if (data_matches_with_prefix(guid_prefix, **pit))
         {
             return *(pit);
         }

diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp
@@ -46,6 +46,7 @@
 #include <rtps/builtin/discovery/participant/timedevent/DSClientEvent.h>
 #include <rtps/participant/RTPSParticipantImpl.h>
 #include <utils/SystemInfo.hpp>
+#include <vector>
 
 using namespace eprosima::fastrtps;
 
@@ -57,12 +58,12 @@ using namespace fastrtps::rtps;
 
 static void direct_send(
         RTPSParticipantImpl* participant,
-        std::vector<GUID_t>& remote_readers,
         LocatorList& locators,
         const CacheChange_t& change)
 {
     FakeWriter writer(participant, c_EntityId_SPDPWriter);
-    DirectMessageSender sender(participant, &remote_readers, &locators);
+    std::vector<GUID_t> readers;
+    DirectMessageSender sender(participant, &readers, &locators);
     RTPSMessageGroup group(participant, &writer, &sender);
     if (!group.add_data(change, false))
     {
@@ -183,7 +184,7 @@ ParticipantProxyData* PDPClient::createParticipantProxyData(
 
         for (auto& svr : mp_builtin->m_DiscoveryServers)
         {
-            if (svr.guidPrefix == participant_data.m_guid.guidPrefix)
+            if (data_matches_with_prefix(svr.guidPrefix, participant_data))
             {
                 is_server = true;
             }
@@ -464,7 +465,7 @@ void PDPClient::assignRemoteEndpoints(
         // Verify if this participant is a server
         for (auto& svr : mp_builtin->m_DiscoveryServers)
         {
-            if (svr.guidPrefix == pdata->m_guid.guidPrefix)
+            if (data_matches_with_prefix(svr.guidPrefix, *pdata))
             {
                 std::unique_lock<std::recursive_mutex> lock(*getMutex());
                 svr.proxy = pdata;
@@ -491,7 +492,7 @@ void PDPClient::notifyAboveRemoteEndpoints(
         // Verify if this participant is a server
         for (auto& svr : mp_builtin->m_DiscoveryServers)
         {
-            if (svr.guidPrefix == pdata.m_guid.guidPrefix)
+            if (data_matches_with_prefix(svr.guidPrefix, pdata))
             {
                 match_pdp_reader_nts_(svr, pdata.m_guid.guidPrefix);
                 match_pdp_writer_nts_(svr, pdata.m_guid.guidPrefix);
@@ -739,15 +740,14 @@ void PDPClient::announceParticipantState(
                         // if we are matched to a server report demise
                         if (svr.proxy != nullptr)
                         {
-                            remote_readers.push_back(svr.GetPDPReader());
                             //locators.push_back(svr.metatrafficMulticastLocatorList);
                             locators.push_back(svr.metatrafficUnicastLocatorList);
                         }
                     }
                 }
 
                 // TODO: This should be done with the configuration of the reliable writer
-                direct_send(getRTPSParticipant(), remote_readers, locators, *change);
+                direct_send(getRTPSParticipant(), locators, *change);
             }
 
             // free change
@@ -763,7 +763,6 @@ void PDPClient::announceParticipantState(
                 CacheChange_t* pPD;
                 if (history.get_min_change(&pPD))
                 {
-                    std::vector<GUID_t> remote_readers;
                     LocatorList locators;
 
                     eprosima::shared_lock<eprosima::shared_mutex> disc_lock(mp_builtin->getDiscoveryMutex());
@@ -774,13 +773,12 @@ void PDPClient::announceParticipantState(
                         // broadcast to all servers
                         if (svr.proxy == nullptr || !_serverPing)
                         {
-                            remote_readers.push_back(svr.GetPDPReader());
                             locators.push_back(svr.metatrafficMulticastLocatorList);
                             locators.push_back(svr.metatrafficUnicastLocatorList);
                         }
                     }
 
-                    direct_send(getRTPSParticipant(), remote_readers, locators, *pPD);
+                    direct_send(getRTPSParticipant(), locators, *pPD);
 
                     // ping done independtly of which triggered the announcement
                     // note all event callbacks are currently serialized

diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp
@@ -208,7 +208,7 @@ ParticipantProxyData* PDPServer::createParticipantProxyData(
             eprosima::shared_lock<eprosima::shared_mutex> disc_lock(mp_builtin->getDiscoveryMutex());
             for (auto& svr : mp_builtin->m_DiscoveryServers)
             {
-                if (svr.guidPrefix == participant_data.m_guid.guidPrefix)
+                if (data_matches_with_prefix(svr.guidPrefix, participant_data))
                 {
                     do_lease = true;
                 }

diff --git a/src/cpp/rtps/security/SecurityManager.cpp b/src/cpp/rtps/security/SecurityManager.cpp
@@ -603,6 +603,7 @@ bool SecurityManager::discovered_participant(
         remote_participant_info = map_ret.first->second->get_auth();
     }
 
+    bool notify_part_authorized = false;
     if (undiscovered && remote_participant_info)
     {
         // Configure the timed event but do not start it
@@ -682,7 +683,7 @@ bool SecurityManager::discovered_participant(
         {
             //TODO(Ricardo) Shared secret on this case?
             std::shared_ptr<SecretHandle> ss;
-            participant_authorized(participant_data, remote_participant_info, ss);
+            notify_part_authorized = participant_authorized(participant_data, remote_participant_info, ss);
         }
     }
     else
@@ -701,11 +702,16 @@ bool SecurityManager::discovered_participant(
     {
         // Maybe send request.
         returnedValue = on_process_handshake(participant_data, remote_participant_info,
-                        MessageIdentity(), HandshakeMessageToken());
+                        MessageIdentity(), HandshakeMessageToken(), notify_part_authorized);
     }
 
     restore_discovered_participant_info(participant_data.m_guid, remote_participant_info);
 
+    if (notify_part_authorized)
+    {
+        notify_participant_authorized(participant_data);
+    }
+
     return returnedValue;
 }
 
@@ -800,7 +806,8 @@ bool SecurityManager::on_process_handshake(
         const ParticipantProxyData& participant_data,
         DiscoveredParticipantInfo::AuthUniquePtr& remote_participant_info,
         MessageIdentity&& message_identity,
-        HandshakeMessageToken&& message_in)
+        HandshakeMessageToken&& message_in,
+        bool& notify_part_authorized)
 {
     auto sentry = is_security_manager_initialized();
     if (!sentry)
@@ -984,6 +991,10 @@ bool SecurityManager::on_process_handshake(
                     {
                         authentication_plugin_->return_sharedsecret_handle(shared_secret_handle, exception);
                     }
+                    else
+                    {
+                        notify_part_authorized = true;
+                    }
 
                 }
 
@@ -1620,10 +1631,17 @@ void SecurityManager::process_participant_stateless_message(
                 return;
             }
 
+            bool notify_part_authorized = false;
             on_process_handshake(*participant_data, remote_participant_info,
-                    std::move(message.message_identity()), std::move(message.message_data().at(0)));
+                    std::move(message.message_identity()),
+                    std::move(message.message_data().at(0)), notify_part_authorized);
 
             restore_discovered_participant_info(remote_participant_key, remote_participant_info);
+
+            if (notify_part_authorized)
+            {
+                notify_participant_authorized(*participant_data);
+            }
         }
         else
         {
@@ -2047,6 +2065,23 @@ uint32_t SecurityManager::builtin_endpoints() const
     return be;
 }
 
+bool SecurityManager::check_guid_comes_from(
+        const GUID_t& adjusted,
+        const GUID_t& original) const
+{
+    bool ret = (original == adjusted);
+    if (!ret && authentication_plugin_ != nullptr)
+    {
+        shared_lock<shared_mutex> _(mutex_);
+        auto part_it = discovered_participants_.find(adjusted);
+        if (part_it != discovered_participants_.end())
+        {
+            ret = part_it->second->check_guid_comes_from(authentication_plugin_, adjusted, original);
+        }
+    }
+    return ret;
+}
+
 void SecurityManager::match_builtin_endpoints(
         const ParticipantProxyData& participant_data)
 {
@@ -4046,20 +4081,6 @@ bool SecurityManager::participant_authorized(
             match_builtin_key_exchange_endpoints(participant_data);
         }
 
-        participant_->pdp()->notifyAboveRemoteEndpoints(participant_data);
-
-        EPROSIMA_LOG_INFO(SECURITY, "Participant " << participant_data.m_guid << " authenticated");
-
-        // Inform user about authenticated remote participant.
-        if (participant_->getListener() != nullptr)
-        {
-            ParticipantAuthenticationInfo info;
-            info.status = ParticipantAuthenticationInfo::AUTHORIZED_PARTICIPANT;
-            info.guid = participant_data.m_guid;
-            participant_->getListener()->onParticipantAuthentication(
-                participant_->getUserRTPSParticipant(), std::move(info));
-        }
-
         for (auto& remote_reader : temp_readers)
         {
             participant_->pdp()->getEDP()->pairing_reader_proxy_with_local_writer(remote_reader.second,
@@ -4078,6 +4099,24 @@ bool SecurityManager::participant_authorized(
     return false;
 }
 
+void SecurityManager::notify_participant_authorized(
+        const ParticipantProxyData& participant_data)
+{
+    participant_->pdp()->notifyAboveRemoteEndpoints(participant_data);
+
+    EPROSIMA_LOG_INFO(SECURITY, "Participant " << participant_data.m_guid << " authenticated");
+
+    // Inform user about authenticated remote participant.
+    if (participant_->getListener() != nullptr)
+    {
+        ParticipantAuthenticationInfo info;
+        info.status = ParticipantAuthenticationInfo::AUTHORIZED_PARTICIPANT;
+        info.guid = participant_data.m_guid;
+        participant_->getListener()->onParticipantAuthentication(
+            participant_->getUserRTPSParticipant(), std::move(info));
+    }
+}
+
 uint32_t SecurityManager::calculate_extra_size_for_rtps_message() const
 {
     auto sentry = is_security_manager_initialized();
@@ -4200,3 +4239,21 @@ void SecurityManager::resend_handshake_message_token(
         }
     }
 }
+
+bool SecurityManager::DiscoveredParticipantInfo::check_guid_comes_from(
+        Authentication* const auth_plugin,
+        const GUID_t& adjusted,
+        const GUID_t& original)
+{
+    bool ret = false;
+    if (auth_plugin != nullptr)
+    {
+        std::lock_guard<std::mutex> g(mtx_);
+
+        if (nullptr != auth_ && AuthenticationStatus::AUTHENTICATION_OK == auth_->auth_status_)
+        {
+            ret = auth_plugin->check_guid_comes_from(auth_->identity_handle_, adjusted, original);
+        }
+    }
+    return ret;
+}