[20181] Hotfix: Secure simple participants with `initialpeers` over `TCP` match #5071

Mario-DL · 2024-07-16T09:22:16Z

Description

This PR fixes a behavior that changed in f2e5ce making simple secure participants not match.

The tcp client sends its DATA[P] to the tcp server, the server starts the security handshake but the client is not able to accept the security handshake request because it does not have the discovery information from the server participant.

@Mergifyio backport 2.14.x 2.10.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.
N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
Changes are backport compatible: they do NOT break ABI nor change library core behavior.
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.
The title and description correctly express the PR's purpose.
Check contributor checklist is correct.
N/A If this is a critical bug fix, backports to the critical-only supported branches have been requested.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

Signed-off-by: Mario Dominguez <[email protected]>

Signed-off-by: Miguel Company <[email protected]>

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]>

Signed-off-by: Miguel Company <[email protected]>

Mario-DL

The new approach makes sense to me. It is cleaner. Thanks @MiguelCompany for the proposal. I am also testing in local for a corner case that made the test fail (using vpn) in my former approach that may also be overcome with this new one.

Mario-DL · 2024-08-27T08:21:34Z

Local test with VPN passed, LGTM

MiguelCompany · 2024-08-27T09:49:59Z

@Mergifyio backport 2.14.x 2.10.x

mergify · 2024-08-27T09:50:02Z

backport 2.14.x 2.10.x

✅ Backports have been created

#5176 [20181] Hotfix: Secure simple participants with initialpeers over TCP match (backport #5071) has been created for branch 2.14.x but encountered conflicts
#5177 [20181] Hotfix: Secure simple participants with initialpeers over TCP match (backport #5071) has been created for branch 2.10.x but encountered conflicts

…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

…ch (eProsima#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]> Signed-off-by: paxifaer <[email protected]>

…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

…ch (#5071) (#5177) * Hotfix: Secure simple participants with `initialpeers` over `TCP` match (#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 * Fix conflicts Signed-off-by: Miguel Company <[email protected]> --------- Signed-off-by: Miguel Company <[email protected]> Co-authored-by: Mario Domínguez López <[email protected]> Co-authored-by: Miguel Company <[email protected]>

…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

…ch (#5071) (#5176) * 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 Co-authored-by: Mario Domínguez López <[email protected]>

…ch (eProsima#5071) (eProsima#5177) * Hotfix: Secure simple participants with `initialpeers` over `TCP` match (eProsima#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) * Fix conflicts Signed-off-by: Miguel Company <[email protected]> --------- Signed-off-by: Miguel Company <[email protected]> Co-authored-by: Mario Domínguez López <[email protected]> Co-authored-by: Miguel Company <[email protected]>

Mario-DL added this to the v3.0.0 milestone Jul 16, 2024

Mario-DL force-pushed the hotfix/20181 branch from ca4483b to 55bb88f Compare July 16, 2024 09:31

Mario-DL requested a review from richiprosima July 16, 2024 09:36

github-actions bot added the ci-pending PR which CI is running label Jul 16, 2024

Mario-DL force-pushed the hotfix/20181 branch 2 times, most recently from 40eb072 to 890b0dd Compare July 16, 2024 15:51

Mario-DL requested review from richiprosima and removed request for richiprosima July 16, 2024 15:51

elianalf added the needs-review PR that is ready to be reviewed label Jul 18, 2024

Tempate mentioned this pull request Jul 18, 2024

Remove secure_wan test from the list of failing tests eProsima/DDS-Router#462

Merged

EduPonz modified the milestones: v3.0.0, v3.0.1 Jul 19, 2024

MiguelCompany force-pushed the hotfix/20181 branch from 890b0dd to c78b40b Compare August 23, 2024 11:13

MiguelCompany requested review from MiguelCompany and removed request for richiprosima and MiguelCompany August 23, 2024 11:25

MiguelCompany force-pushed the hotfix/20181 branch from 238f832 to e0e2cc9 Compare August 26, 2024 07:20

MiguelCompany requested review from MiguelCompany and removed request for MiguelCompany August 26, 2024 07:59

MiguelCompany force-pushed the hotfix/20181 branch from e0e2cc9 to 1019a29 Compare August 26, 2024 09:19

MiguelCompany requested review from JesusPoderoso and removed request for MiguelCompany August 26, 2024 09:20

Mario-DL and others added 6 commits August 26, 2024 11:25

Refs #20181: Add BB test

e3b05ff

Signed-off-by: Mario Dominguez <[email protected]>

Refs #20181: Add Fix

60e89ca

Signed-off-by: Mario Dominguez <[email protected]>

Refs #20181: linter

fa66bfc

Signed-off-by: Mario Dominguez <[email protected]>

Refs #20181. Pass in secure_endpoints as lambda capture.

a14f823

Signed-off-by: Miguel Company <[email protected]>

Refs #20181. Unmatch non-secure before matching secure.

13b778b

Signed-off-by: Miguel Company <[email protected]>

MiguelCompany force-pushed the hotfix/20181 branch from 1019a29 to 13b778b Compare August 26, 2024 09:26

MiguelCompany requested review from JesusPoderoso and removed request for JesusPoderoso August 26, 2024 09:28

Mario-DL commented Aug 27, 2024

View reviewed changes

Mario-DL added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed needs-review PR that is ready to be reviewed ci-pending PR which CI is running labels Aug 27, 2024

MiguelCompany approved these changes Aug 27, 2024

View reviewed changes

Mario-DL merged commit 3ca60e0 into master Aug 27, 2024
17 checks passed

Mario-DL deleted the hotfix/20181 branch August 27, 2024 09:49

This was referenced Aug 27, 2024

[20181] Hotfix: Secure simple participants with initialpeers over TCP match (backport #5071) #5176

Merged

[20181] Hotfix: Secure simple participants with initialpeers over TCP match (backport #5071) #5177

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[20181] Hotfix: Secure simple participants with `initialpeers` over `TCP` match #5071

[20181] Hotfix: Secure simple participants with `initialpeers` over `TCP` match #5071

Mario-DL commented Jul 16, 2024 •

edited by JesusPoderoso

Loading

Mario-DL left a comment •

edited

Loading

Mario-DL commented Aug 27, 2024

MiguelCompany commented Aug 27, 2024

mergify bot commented Aug 27, 2024 •

edited

Loading

[20181] Hotfix: Secure simple participants with initialpeers over TCP match #5071

[20181] Hotfix: Secure simple participants with initialpeers over TCP match #5071

Conversation

Mario-DL commented Jul 16, 2024 • edited by JesusPoderoso Loading

Description

Contributor Checklist

Reviewer Checklist

Mario-DL left a comment • edited Loading

Choose a reason for hiding this comment

Mario-DL commented Aug 27, 2024

MiguelCompany commented Aug 27, 2024

mergify bot commented Aug 27, 2024 • edited Loading

✅ Backports have been created

[20181] Hotfix: Secure simple participants with `initialpeers` over `TCP` match #5071

[20181] Hotfix: Secure simple participants with `initialpeers` over `TCP` match #5071

Mario-DL commented Jul 16, 2024 •

edited by JesusPoderoso

Loading

Mario-DL left a comment •

edited

Loading

mergify bot commented Aug 27, 2024 •

edited

Loading