Fix asymmetric whitelist matching [19035] #3733

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

#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

#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 <[email protected]>

Signed-off-by: Juan López Fernández <[email protected]>

…tors refactor Signed-off-by: Juan López Fernández <[email protected]>

Signed-off-by: Juan Lopez Fernandez <[email protected]>

…peers and discovery server Signed-off-by: Juan Lopez Fernandez <[email protected]>

Signed-off-by: Juan Lopez Fernandez <[email protected]>

Signed-off-by: Eduardo Ponz <[email protected]>

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

@richiprosima please test this

…connect Signed-off-by: Juan Lopez Fernandez <[email protected]>

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

@richiprosima please test this

Signed-off-by: Juan Lopez Fernandez <[email protected]>

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

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

Refs #18854: Fix Windows build error

ffa874c

Signed-off-by: JesusPoderoso <[email protected]>

Refs #18854: Apply rev suggestions

2182606

Signed-off-by: JesusPoderoso <[email protected]>

Refs #19203: Add more test cases

4dda3a3

Signed-off-by: Juan López Fernández <[email protected]>

Refs #19203: Asymmetric whitelist matching fix: transform_remote_loca…

4043bf1

…tors refactor Signed-off-by: Juan López Fernández <[email protected]>

Refs #19203: Tiny fixes

01aae83

Signed-off-by: Juan Lopez Fernandez <[email protected]>

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

26d5c83

…peers and discovery server Signed-off-by: Juan Lopez Fernandez <[email protected]>

Refs #19203: Bonus fix: TCPv6 + whitelist

be8eb09

Signed-off-by: Juan Lopez Fernandez <[email protected]>

Refs #19203: Avoid API/ABI break

1a18d15

Signed-off-by: Juan Lopez Fernandez <[email protected]>

Refs #19203: Fix missing include

443425f

Signed-off-by: Eduardo Ponz <[email protected]>

juanlofer-eprosima dismissed stale reviews from MiguelCompany and EduPonz via 95bbe33 September 19, 2023 14:23

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

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

deefaf9

…connect Signed-off-by: Juan Lopez Fernandez <[email protected]>

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 <[email protected]>

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
11 of 14 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix asymmetric whitelist matching [19035] #3733

Fix asymmetric whitelist matching [19035] #3733

juanlofer-eprosima commented Jul 20, 2023 •

edited by EduPonz

Loading

juanlofer-eprosima commented Jul 20, 2023

EduPonz commented Aug 10, 2023

EduPonz commented Aug 10, 2023

mergify bot commented Aug 10, 2023 •

edited

Loading

EduPonz commented Aug 11, 2023

EduPonz commented Aug 11, 2023

EduPonz commented Aug 11, 2023

rsanchez15 commented Aug 22, 2023

mergify bot commented Aug 22, 2023 •

edited

Loading

MiguelCompany commented Aug 29, 2023

JesusPoderoso commented Sep 6, 2023

juanlofer-eprosima commented Sep 19, 2023

juanlofer-eprosima commented Sep 19, 2023

juanlofer-eprosima commented Sep 20, 2023

juanlofer-eprosima commented Sep 20, 2023

Fix asymmetric whitelist matching [19035] #3733

Fix asymmetric whitelist matching [19035] #3733

Conversation

juanlofer-eprosima commented Jul 20, 2023 • edited by EduPonz Loading

Description

Contributor Checklist

Reviewer Checklist

juanlofer-eprosima commented Jul 20, 2023

EduPonz commented Aug 10, 2023

EduPonz commented Aug 10, 2023

mergify bot commented Aug 10, 2023 • edited Loading

✅ Backports have been created

EduPonz commented Aug 11, 2023

EduPonz commented Aug 11, 2023

EduPonz commented Aug 11, 2023

rsanchez15 commented Aug 22, 2023

mergify bot commented Aug 22, 2023 • edited Loading

✅ Backports have been created

MiguelCompany commented Aug 29, 2023

JesusPoderoso commented Sep 6, 2023

juanlofer-eprosima commented Sep 19, 2023

juanlofer-eprosima commented Sep 19, 2023

juanlofer-eprosima commented Sep 20, 2023

juanlofer-eprosima commented Sep 20, 2023

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