[19778][20296] Add netmask filter transport configuration + interface allowlist and blocklist #4241

juanlofer-eprosima · 2024-01-12T12:49:30Z

Description

This PR adds two new features:

Netmask filtering: allows to configure participants, (UDP/TCP) transports and/or specific interfaces (via allowlist) so that no messages are sent from an interface to remote locators from outside its network domain (given by network mask).
Blocklist: allows to ignore the interfaces present in this collection. These can be specified by IP or device name, and this list takes precedence over allowlist/whitelist.

Behaviour changes:

System's network interfaces are now cached in SystemInfo singleton, in order to reduce the number of system calls performed and promote consistency across separate Fast-DDS code sections.
Transformation of received remote locators to local ones is now only attempted when they correspond to (Fast-DDS) entities created in the same host. More specifically, now it is the GUID of the corresponding entity what is used to determine whether the locator is local, rather than by looking for a match in the local interfaces. This avoids the erroneous transformation of a truly remote locator to localhost when there is a coincidence in the IPs assigned to two machines' interfaces (example: docker0 -> 172.17.0.1).

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.
New feature has been added to the versions.md file (if applicable).
New feature has been documented/Current behavior is correctly described in the documentation.
Related documentation PR: eProsima/Fast-DDS-docs# (PR)
N/A; Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
The title and description correctly express the PR's purpose.
Check contributor checklist is correct.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

src/cpp/rtps/participant/RTPSParticipantImpl.cpp

src/cpp/rtps/transport/UDPv4Transport.cpp

src/cpp/utils/IPFinder.cpp

Mario-DL

Overall, great job in this PR! bringing important corrections and refactor.
I am leaving a partial review here. Still pending review the transport implementations, the xmlparsing related things, and testing that the issue is gone.

I could perfectly reproduce the issue. In the attached image the same data packet was being delivered to different destinations: lan address (correctly) and localhost (wrongly, after a failed docker-ip interpretation)

Leaving some questions and suggestions though.
In addition, some fixes should be made to correct windows compilation and I think that the PR should be paired with a Fast DDS Docs documentation PR.

include/fastrtps/utils/collections/ResourceLimitedVector.hpp

include/fastdds/rtps/transport/NetmaskFilterKind.hpp

include/fastdds/rtps/transport/ChainingTransport.h

include/fastdds/rtps/transport/TransportInterface.h

src/cpp/rtps/network/NetworkFactory.h

src/cpp/fastdds/subscriber/SubscriberImpl.cpp

src/cpp/rtps/network/NetmaskFilterUtils.hpp

src/cpp/utils/IPFinder.cpp

src/cpp/fastdds/publisher/PublisherImpl.cpp

src/cpp/rtps/transport/TCPv4Transport.cpp

src/cpp/rtps/transport/UDPv4Transport.cpp

src/cpp/rtps/network/NetworkFactory.cpp

src/cpp/rtps/transport/UDPTransportInterface.h

include/fastrtps/xmlparser/XMLParser.h

Mario-DL

I left the second part of the review.

I checked that the feature fixes the locator transformation mangling but I still need to properly test the blocklist and netmask filter.
The documentation has been linked so I remove the docs pending label

Mario-DL · 2024-03-12T08:29:30Z

@richiprosima please test this

Mario-DL

Nice effort on the review and refactors! looks really nice.
Leaving some minor suggestions

src/cpp/fastdds/publisher/PublisherImpl.cpp

include/fastdds/rtps/participant/RTPSParticipant.h

Mario-DL

LGTM with green CI

Mario-DL · 2024-03-14T07:08:40Z

@richiprosima please test this

Mario-DL · 2024-03-14T08:36:24Z

@richiprosima please test this

Mario-DL

LGTM with green CI

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

…o cpp Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> Signed-off-by: EduPonz <eduardoponz@eprosima.com>

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

EduPonz · 2024-03-15T17:06:08Z

@richiprosima please test this

EduPonz · 2024-03-16T07:02:41Z

@richiprosima please test windows test mac

EduPonz · 2024-03-17T07:41:06Z

@richiprosima please check style

juanlofer-eprosima changed the title ~~[19778] Add netmask filter transport configuration option + interface blocklist~~ [19778] Add netmask filter transport configuration + interface blocklist Jan 12, 2024

EduPonz added this to the v3.0.0 milestone Jan 15, 2024

juanlofer-eprosima force-pushed the feature/netmask-filter branch 3 times, most recently from 9d1dd95 to d950405 Compare February 1, 2024 10:36

juanlofer-eprosima marked this pull request as ready for review February 1, 2024 10:53

EduPonz modified the milestones: v3.0.0, v2.14.0 Feb 1, 2024

EduPonz added doc-pending Issue or PR which is pending to be documented versions-pending labels Feb 1, 2024

juanlofer-eprosima force-pushed the feature/netmask-filter branch from f4bfccd to f7f1390 Compare February 2, 2024 08:17

juanlofer-eprosima commented Feb 2, 2024

View reviewed changes

src/cpp/rtps/participant/RTPSParticipantImpl.cpp Outdated Show resolved Hide resolved

juanlofer-eprosima mentioned this pull request Feb 7, 2024

[19778][20296] Add netmask filter transport configuration + interface allowlist and blocklist eProsima/Fast-DDS-docs#659

Merged

juanlofer-eprosima force-pushed the feature/netmask-filter branch from 79f136f to 2c91a08 Compare February 29, 2024 06:59

juanlofer-eprosima commented Feb 29, 2024

View reviewed changes

src/cpp/rtps/transport/UDPv4Transport.cpp Outdated Show resolved Hide resolved

juanlofer-eprosima commented Feb 29, 2024

View reviewed changes

src/cpp/utils/IPFinder.cpp Outdated Show resolved Hide resolved

juanlofer-eprosima changed the title ~~[19778] Add netmask filter transport configuration + interface blocklist~~ [19778][20296] Add netmask filter transport configuration + interface blocklist Feb 29, 2024

Mario-DL requested changes Feb 29, 2024

View reviewed changes

Mario-DL requested changes Mar 1, 2024

View reviewed changes

Mario-DL reviewed Mar 1, 2024

View reviewed changes

Mario-DL removed the doc-pending Issue or PR which is pending to be documented label Mar 1, 2024

juanlofer-eprosima force-pushed the feature/netmask-filter branch from 2c91a08 to 19cffc9 Compare March 8, 2024 13:46

juanlofer-eprosima changed the title ~~[19778][20296] Add netmask filter transport configuration + interface blocklist~~ [19778][20296] Add netmask filter transport configuration + interface allowlist and blocklist Mar 11, 2024

juanlofer-eprosima force-pushed the feature/netmask-filter branch from 1660f76 to 76d7338 Compare March 12, 2024 06:32

juanlofer-eprosima removed the versions-pending label Mar 12, 2024

Mario-DL requested changes Mar 12, 2024

View reviewed changes

Mario-DL previously approved these changes Mar 14, 2024

View reviewed changes

juanlofer-eprosima dismissed Mario-DL’s stale review via eb9943c March 14, 2024 08:35

Mario-DL approved these changes Mar 14, 2024

View reviewed changes

juanlofer-eprosima added 16 commits March 15, 2024 18:05

Allowlist and blocklist implementation

a3e58bf

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

Update XML parser

b57f616

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

Update versions.md

acc4bc0

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

Fix linker errors in security and statistics tests

2c0ce06

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

Rename NetmaskFilterKind.h -> NetmaskFilterKind.hpp

e23ef45

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

Add safety check

fed0372

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

Rebase fix

13d17be

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

Apply suggestions

3cc3cd7

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

Minor changes

a43ea50

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

Add IP string attribute to NetworkInterface and move implementation t…

6719065

…o cpp Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> Signed-off-by: EduPonz <eduardoponz@eprosima.com>

Windows fixes

e26aab1

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

Uncrustify

763b112

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

Add missing change: serialize writer GUID before locators

165e50d

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

Apply more suggestions

d765b62

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

One more suggestion

70f18b6

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

Make NetmaskFilterKind optional in allowlist XML parsing

98be415

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

EduPonz force-pushed the feature/netmask-filter branch from eb9943c to 98be415 Compare March 15, 2024 17:05

EduPonz merged commit 725f1c7 into master Mar 17, 2024
13 of 15 checks passed

EduPonz deleted the feature/netmask-filter branch March 17, 2024 07:48

elianalf mentioned this pull request May 28, 2024

[21106] Add missing documentation of netmask filter and max_msg_size_no_frag in TransportConfigQos eProsima/Fast-DDS-docs#788

Merged

8 tasks

mergify bot mentioned this pull request May 29, 2024

[21106] Add missing documentation of netmask filter and max_msg_size_no_frag in TransportConfigQos (backport #788) eProsima/Fast-DDS-docs#789

Merged

8 tasks

bkueng mentioned this pull request Oct 15, 2024

Error initializing Fast-DDS when the Ethernet link is not up #4371

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[19778][20296] Add netmask filter transport configuration + interface allowlist and blocklist #4241

[19778][20296] Add netmask filter transport configuration + interface allowlist and blocklist #4241

juanlofer-eprosima commented Jan 12, 2024 •

edited by EduPonz

Loading

Mario-DL left a comment •

edited

Loading

Mario-DL left a comment

Mario-DL commented Mar 12, 2024

Mario-DL left a comment

Mario-DL left a comment

Mario-DL commented Mar 14, 2024

Mario-DL commented Mar 14, 2024

Mario-DL left a comment

EduPonz commented Mar 15, 2024

EduPonz commented Mar 16, 2024

EduPonz commented Mar 17, 2024

[19778][20296] Add netmask filter transport configuration + interface allowlist and blocklist #4241

[19778][20296] Add netmask filter transport configuration + interface allowlist and blocklist #4241

Conversation

juanlofer-eprosima commented Jan 12, 2024 • edited by EduPonz Loading

Description

Contributor Checklist

Reviewer Checklist

Mario-DL left a comment • edited Loading

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL commented Mar 12, 2024

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL left a comment

Choose a reason for hiding this comment

Mario-DL commented Mar 14, 2024

Mario-DL commented Mar 14, 2024

Mario-DL left a comment

Choose a reason for hiding this comment

EduPonz commented Mar 15, 2024

EduPonz commented Mar 16, 2024

EduPonz commented Mar 17, 2024

juanlofer-eprosima commented Jan 12, 2024 •

edited by EduPonz

Loading

Mario-DL left a comment •

edited

Loading