Fix asymmetric whitelist matching [19035] by juanlofer-eprosima · Pull Request #3733 · eProsima/Fast-DDS

juanlofer-eprosima · 2023-07-20T07:26:57Z

Description

Matching two local participants through UDP/TCP transport currently fails when only one of them sets an interface whitelist. In particular, the problematic cases are only including the loopback interface (A), or including all except the loopback interface (B).

The reason for this is that Fast DDS internally performs an optimization consisting in transforming a local locator to localhost (NetworkFactory::transform_remote_locators) whenever possible. This way, communication is performed through the loopback interface, which is allegedly more robust to changes in the environment.
However, the only input for determining whether this transformation should be performed is each transport's own interface whitelist. This implies that in some cases the transformation will be wrongly performed as the remote transport may not be listening on localhost (B).
Another issue with this transformation method is that it boths attempts conversion to localhost as well as filters locators based on one's interface whitelist. This explains why A arises, as the received remote (local) locator gets filtered out for being different than localhost.

This PR attempts to fix the issue by sending in discovery a new network configuration parameter, which for the moment only includes whether a participant is listening on localhost (depending on its whitelist). Four (plus 28 unused) different bits encoding this information are sent, each for a different transport kind (UDPv4, UDPv6, TCPv4 and TCPv6).
An extended NetworkFactory::transform_remote_locators is added leveraging this information, so conversion to localhost is only performed when both remote and local participants allow for it.

Behaviour changes:

Transformation to localhost is no longer attempted in Initial Peers and Discovery Server. This is because the remote's participant network configuration is only shared through discovery, and hence in these two cases it is not available.

@Mergifyio backport 2.11.x 2.10.x 2.6.x

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
Any new/modified methods have been properly documented using Doxygen.
Changes are ABI compatible.
Changes are API compatible.
N/A New feature has been added to the versions.md file (if applicable).
N/A New feature has been documented/Current behavior is correctly described in the documentation.
Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
Check contributor checklist is correct.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

juanlofer-eprosima · 2023-07-20T07:29:50Z

Compatibility report: compat_report.zip

EduPonz · 2023-08-10T19:54:50Z

@richiprosima please test this

EduPonz · 2023-08-10T19:58:23Z

@Mergifyio backport 2.11.x 2.10.x 2.6.x

mergify · 2023-08-10T19:58:36Z

backport 2.11.x 2.10.x 2.6.x

✅ Backports have been created

Details

#3867 Fix asymmetric whitelist matching [19035] (backport #3733) has been created for branch 2.11.x
#3868 Fix asymmetric whitelist matching [19035] (backport #3733) has been created for branch 2.10.x
#3869 Fix asymmetric whitelist matching [19035] (backport #3733) has been created for branch 2.6.x but encountered conflicts

EduPonz · 2023-08-11T05:03:10Z

@richiprosima please test mac

EduPonz · 2023-08-11T08:25:39Z

@richiprosima please test mac

EduPonz · 2023-08-11T11:17:41Z

@richiprosima please test mac

rsanchez15 · 2023-08-22T12:11:46Z

@Mergifyio backport eprosima/integration

mergify · 2023-08-22T12:11:51Z

backport eprosima/integration

✅ Backports have been created

Details

#3870 Fix asymmetric whitelist matching [19035] (backport #3733) has been created for branch eprosima/integration but encountered conflicts

MiguelCompany · 2023-08-29T12:28:07Z

@richiprosima Please test mac

JesusPoderoso · 2023-09-06T05:51:23Z

@richiprosima please test mac

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Signed-off-by: Juan López Fernández <juanlopez@eprosima.com>

…tors refactor Signed-off-by: Juan López Fernández <juanlopez@eprosima.com>

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

…peers and discovery server Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

juanlofer-eprosima · 2023-09-19T14:24:30Z

@richiprosima please test this

…connect Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

juanlofer-eprosima · 2023-09-19T14:45:56Z

@richiprosima please test this

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

juanlofer-eprosima · 2023-09-20T13:04:56Z

@richiprosima please test this

juanlofer-eprosima · 2023-09-20T13:50:53Z

@richiprosima please test this

* Refs #18854: Asymmetric whitelist regression test Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #18854: Fix Windows build error Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #18854: Apply rev suggestions Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #19203: Add more test cases Signed-off-by: Juan López Fernández <juanlopez@eprosima.com> * Refs #19203: Asymmetric whitelist matching fix: transform_remote_locators refactor Signed-off-by: Juan López Fernández <juanlopez@eprosima.com> * Refs #19203: Tiny fixes Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Add warnings for non-localhost local address in initial peers and discovery server Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Bonus fix: TCPv6 + whitelist Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Avoid API/ABI break Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Fix TCP when no whitelist and initial peer != localhost Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Improve some comments Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Uncrustify Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Fix missing include Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Revert locator scope append in TCPChannelResourceBasic::connect Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Disable (almost) all IPv6 tests Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> --------- Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> Signed-off-by: Juan López Fernández <juanlopez@eprosima.com> Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> Co-authored-by: JesusPoderoso <jesuspoderoso@eprosima.com> Co-authored-by: Eduardo Ponz <eduardoponz@eprosima.com> (cherry picked from commit c8ab860)

* Refs #18854: Asymmetric whitelist regression test Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #18854: Fix Windows build error Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #18854: Apply rev suggestions Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #19203: Add more test cases Signed-off-by: Juan López Fernández <juanlopez@eprosima.com> * Refs #19203: Asymmetric whitelist matching fix: transform_remote_locators refactor Signed-off-by: Juan López Fernández <juanlopez@eprosima.com> * Refs #19203: Tiny fixes Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Add warnings for non-localhost local address in initial peers and discovery server Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Bonus fix: TCPv6 + whitelist Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Avoid API/ABI break Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Fix TCP when no whitelist and initial peer != localhost Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Improve some comments Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Uncrustify Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Fix missing include Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Revert locator scope append in TCPChannelResourceBasic::connect Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Disable (almost) all IPv6 tests Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> --------- Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> Signed-off-by: Juan López Fernández <juanlopez@eprosima.com> Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> Co-authored-by: JesusPoderoso <jesuspoderoso@eprosima.com> Co-authored-by: Eduardo Ponz <eduardoponz@eprosima.com> (cherry picked from commit c8ab860) # Conflicts: # include/fastdds/rtps/attributes/RTPSParticipantAttributes.h # include/fastdds/rtps/transport/TransportInterface.h # src/cpp/rtps/transport/TCPTransportInterface.cpp # test/mock/rtps/NetworkFactory/fastdds/rtps/network/NetworkFactory.h

* Refs #18854: Asymmetric whitelist regression test Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #18854: Fix Windows build error Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #18854: Apply rev suggestions Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> * Refs #19203: Add more test cases Signed-off-by: Juan López Fernández <juanlopez@eprosima.com> * Refs #19203: Asymmetric whitelist matching fix: transform_remote_locators refactor Signed-off-by: Juan López Fernández <juanlopez@eprosima.com> * Refs #19203: Tiny fixes Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Add warnings for non-localhost local address in initial peers and discovery server Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Bonus fix: TCPv6 + whitelist Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Avoid API/ABI break Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Fix TCP when no whitelist and initial peer != localhost Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Improve some comments Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Uncrustify Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Fix missing include Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> * Refs #19203: Revert locator scope append in TCPChannelResourceBasic::connect Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #19203: Disable (almost) all IPv6 tests Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> --------- Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com> Signed-off-by: Juan López Fernández <juanlopez@eprosima.com> Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> Signed-off-by: Eduardo Ponz <eduardoponz@eprosima.com> Co-authored-by: JesusPoderoso <jesuspoderoso@eprosima.com> Co-authored-by: Eduardo Ponz <eduardoponz@eprosima.com> (cherry picked from commit c8ab860) # Conflicts: # include/fastdds/rtps/transport/TransportInterface.h # test/mock/rtps/NetworkFactory/fastdds/rtps/network/NetworkFactory.h

EduPonz added this to the v2.12.0 milestone Aug 7, 2023

EduPonz force-pushed the bugfix/asymmetric-whitelist branch 2 times, most recently from 0d081a6 to 2e7b31d Compare August 10, 2023 19:48

EduPonz previously approved these changes Aug 10, 2023

View reviewed changes

JesusPoderoso added the ci-pending PR which CI is running label Aug 17, 2023

rsanchez15 mentioned this pull request Aug 29, 2023

Bugfix/asymetric whitelist eprosima integration #3819

Merged

MiguelCompany previously approved these changes Aug 29, 2023

View reviewed changes

EduPonz added to-do and removed ci-pending PR which CI is running labels Sep 6, 2023

EduPonz assigned juanlofer-eprosima Sep 6, 2023

JesusPoderoso and others added 9 commits September 19, 2023 16:19

Refs #18854: Asymmetric whitelist regression test

3374454

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #18854: Fix Windows build error

ffa874c

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #18854: Apply rev suggestions

2182606

Signed-off-by: JesusPoderoso <jesuspoderoso@eprosima.com>

Refs #19203: Add more test cases

4dda3a3

Signed-off-by: Juan López Fernández <juanlopez@eprosima.com>

Refs #19203: Asymmetric whitelist matching fix: transform_remote_loca…

4043bf1

…tors refactor Signed-off-by: Juan López Fernández <juanlopez@eprosima.com>

Refs #19203: Tiny fixes

01aae83

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

Refs #19203: Add warnings for non-localhost local address in initial …

26d5c83

…peers and discovery server Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

Refs #19203: Bonus fix: TCPv6 + whitelist

be8eb09

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

Refs #19203: Avoid API/ABI break

1a18d15

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

Refs #19203: Revert locator scope append in TCPChannelResourceBasic::…

deefaf9

…connect Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

juanlofer-eprosima force-pushed the bugfix/asymmetric-whitelist branch from 95bbe33 to deefaf9 Compare September 19, 2023 14:45

Refs #19203: Disable (almost) all IPv6 tests

c473fca

Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com>

juanlofer-eprosima force-pushed the bugfix/asymmetric-whitelist branch from 4a0241f to c473fca Compare September 20, 2023 13:04

pablogs9 mentioned this pull request Sep 20, 2023

Problem with ping-pong demo - topic does not echo/list after running the node micro-ROS/micro-ROS-demos#78

Closed

MiguelCompany added ci-pending PR which CI is running and removed to-do labels Sep 20, 2023

EduPonz approved these changes Sep 21, 2023

View reviewed changes

EduPonz added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels Sep 21, 2023

EduPonz merged commit c8ab860 into master Sep 21, 2023

EduPonz deleted the bugfix/asymmetric-whitelist branch September 21, 2023 10:02

This was referenced Sep 21, 2023

Fix asymmetric whitelist matching [19035] (backport #3733) #3867

Closed

Fix asymmetric whitelist matching [19035] (backport #3733) #3868

Closed

mergify bot mentioned this pull request Sep 21, 2023

Fix asymmetric whitelist matching [19035] (backport #3733) #3869

Closed

mergify bot mentioned this pull request Sep 21, 2023

Fix asymmetric whitelist matching [19035] (backport #3733) #3870

Closed

juanlofer-eprosima mentioned this pull request Oct 16, 2023

Fix remote locators filtering when whitelist provided #3933

Merged

10 tasks

EduPonz mentioned this pull request Jan 24, 2024

[20306] Ignore 0x8007 if coming from other vendor #4291

Merged

11 tasks

juanlofer-eprosima mentioned this pull request May 6, 2025

[Question] Discovery behavioral difference between humble and rolling with localhost_only ros2/rmw_fastrtps#821

Closed

juanlofer-eprosima mentioned this pull request Jun 10, 2025

[23213] Add localhost capability only to UDP & TCP transports #5864

Merged

12 tasks

This was referenced Jun 11, 2025

[23213] Add localhost capability only to UDP & TCP transports (backport #5864) #5868

Merged

[23213] Add localhost capability only to UDP & TCP transports (backport #5864) #5869

Merged

Conversation

juanlofer-eprosima commented Jul 20, 2023 • edited by EduPonz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Contributor Checklist

Reviewer Checklist

Uh oh!

juanlofer-eprosima commented Jul 20, 2023

Uh oh!

EduPonz commented Aug 10, 2023

Uh oh!

EduPonz commented Aug 10, 2023

Uh oh!

mergify bot commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

EduPonz commented Aug 11, 2023

Uh oh!

EduPonz commented Aug 11, 2023

Uh oh!

EduPonz commented Aug 11, 2023

Uh oh!

rsanchez15 commented Aug 22, 2023

Uh oh!

mergify bot commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

MiguelCompany commented Aug 29, 2023

Uh oh!

JesusPoderoso commented Sep 6, 2023

Uh oh!

juanlofer-eprosima commented Sep 19, 2023

Uh oh!

juanlofer-eprosima commented Sep 19, 2023

Uh oh!

juanlofer-eprosima commented Sep 20, 2023

Uh oh!

juanlofer-eprosima commented Sep 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

juanlofer-eprosima commented Jul 20, 2023 •

edited by EduPonz

Loading

mergify bot commented Aug 10, 2023 •

edited

Loading

mergify bot commented Aug 22, 2023 •

edited

Loading