Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[13364] Fix memory problem when ciphering payload #2485

Merged
merged 7 commits into from
Dec 16, 2023
Merged

[13364] Fix memory problem when ciphering payload #2485

merged 7 commits into from
Dec 16, 2023

Conversation

JLBuenoLopez
Copy link
Contributor

This closes #2379

@JLBuenoLopez JLBuenoLopez added this to the v2.5.1 milestone Feb 10, 2022
@JLBuenoLopez JLBuenoLopez mentioned this pull request Feb 10, 2022
Closed
@JLBuenoLopez
Copy link
Contributor Author

This PR breaks several security tests:

  • Communication tests in several platforms
  • Blackbox tests in Windows platform

@JLBuenoLopez JLBuenoLopez modified the milestones: v2.5.1, v2.6.0 Feb 18, 2022
@MiguelCompany MiguelCompany changed the title [13364] Fix memory problem when ciphering paylod [13364] Fix memory problem when ciphering payload Feb 18, 2022
richiware
richiware requested changes Feb 21, 2022
View reviewed changes
Copy link
Member

@richiware richiware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a meeting we decided this approach is not the better one. The calculation has to be done when the CacheChange_t is added in the WriterHistory, as it is done now. According to @MiguelCompany we have to take into account the size of the DataFrag header when calculating the size of each fragment.

@MiguelCompany
Copy link
Member

@richiware This has been rebased, and I have changed the last commit (the one that fixes the issue), with a nicer solution.

Would you mind reviewing this?

@MiguelCompany MiguelCompany modified the milestones: v2.6.0, v2.6.1 Mar 8, 2022
@EduPonz EduPonz modified the milestones: v2.6.1, v2.7.0 Jun 1, 2022
@EduPonz EduPonz added the to-do label Jun 10, 2022
@EduPonz EduPonz modified the milestones: v2.7.0, v2.7.1 Jul 1, 2022
@MiguelCompany MiguelCompany modified the milestones: v2.7.1, v2.7.2 Jul 22, 2022
@JLBuenoLopez JLBuenoLopez modified the milestones: v2.7.2, v2.9.0 Nov 28, 2022
@MiguelCompany MiguelCompany modified the milestones: v2.9.0, v2.9.1 Dec 16, 2022
@EduPonz EduPonz modified the milestones: v2.9.1, v2.10.0 Jan 18, 2023
@JLBuenoLopez JLBuenoLopez modified the milestones: v2.10.0, v2.11.0 May 9, 2023
@JLBuenoLopez JLBuenoLopez modified the milestones: v2.11.0, v2.11.1 Jun 13, 2023
@MiguelCompany MiguelCompany removed this from the v2.11.1 milestone Jul 6, 2023
@MiguelCompany
Refs #13364. Fix warning.
25ad62c 
Signed-off-by: Miguel Company <[email protected]>
@MiguelCompany
Copy link
Member

@richiprosima Please test this

@EduPonz EduPonz added ci-pending PR which CI is running and removed needs-review PR that is ready to be reviewed labels Dec 14, 2023
@EduPonz
Copy link

EduPonz commented Dec 14, 2023

@richiprosima please test windows test mac

@EduPonz
Copy link

EduPonz commented Dec 15, 2023

@richiprosima please test windows

1 similar comment
@EduPonz
Copy link

EduPonz commented Dec 15, 2023

@richiprosima please test windows

@EduPonz
Copy link

EduPonz commented Dec 16, 2023

@MiguelCompany do we need to backport this?

@EduPonz EduPonz removed the ci-pending PR which CI is running label Dec 16, 2023
@EduPonz EduPonz merged commit 25dee05 into master Dec 16, 2023
12 of 14 checks passed
@EduPonz EduPonz deleted the bugfix/security-fix-2379 branch December 16, 2023 16:34
@MiguelCompany
Copy link
Member

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

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 16, 2023 •
edited
Loading

backport 2.12.x 2.11.x 2.10.x 2.6.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Dec 16, 2023
@JLBuenoLopez @mergify
Fix memory problem when ciphering payload (#2485)
d09de25 
* Refs 13364. Regression test.

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

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

* Refs 13364. Test with different lengths.

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

* Refs 13364. Apply review suggestions.

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

* Refs 13364. Fix #2379.

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

* Include what you use in DDSBlackboxTestsSecurity.cpp

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

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 25dee05)
mergify bot pushed a commit that referenced this pull request Dec 16, 2023
@JLBuenoLopez @mergify
Fix memory problem when ciphering payload (#2485)
6647698 
* Refs 13364. Regression test.

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

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

* Refs 13364. Test with different lengths.

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

* Refs 13364. Apply review suggestions.

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

* Refs 13364. Fix #2379.

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

* Include what you use in DDSBlackboxTestsSecurity.cpp

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

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 25dee05)
mergify bot pushed a commit that referenced this pull request Dec 16, 2023
@JLBuenoLopez @mergify
Fix memory problem when ciphering payload (#2485)
7c96a3f 
* Refs 13364. Regression test.

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

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

* Refs 13364. Test with different lengths.

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

* Refs 13364. Apply review suggestions.

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

* Refs 13364. Fix #2379.

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

* Include what you use in DDSBlackboxTestsSecurity.cpp

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

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 25dee05)
This was referenced Dec 16, 2023
Merged
Merged
Merged
@mergify mergify bot mentioned this pull request Dec 16, 2023
Merged
EduPonz pushed a commit that referenced this pull request Dec 18, 2023
@mergify @JLBuenoLopez
Fix memory problem when ciphering payload (#2485) (#4152)
b598fd4 
* Refs 13364. Regression test.

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

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

* Refs 13364. Test with different lengths.

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

* Refs 13364. Apply review suggestions.

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

* Refs 13364. Fix #2379.

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

* Include what you use in DDSBlackboxTestsSecurity.cpp

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

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 25dee05)

Co-authored-by: José Luis Bueno López <[email protected]>
@MiguelCompany MiguelCompany mentioned this pull request Dec 18, 2023
Merged
9 tasks
EduPonz pushed a commit that referenced this pull request Dec 18, 2023
@mergify @JLBuenoLopez
Fix memory problem when ciphering payload (#2485) (#4150)
ef11580 
* Refs 13364. Regression test.

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

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

* Refs 13364. Test with different lengths.

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

* Refs 13364. Apply review suggestions.

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

* Refs 13364. Fix #2379.

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

* Include what you use in DDSBlackboxTestsSecurity.cpp

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

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 25dee05)

Co-authored-by: José Luis Bueno López <[email protected]>
EduPonz pushed a commit that referenced this pull request Dec 18, 2023
@mergify @JLBuenoLopez
Fix memory problem when ciphering payload (#2485) (#4151)
ca55d46 
* Refs 13364. Regression test.

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

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

* Refs 13364. Test with different lengths.

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

* Refs 13364. Apply review suggestions.

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

* Refs 13364. Fix #2379.

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

* Include what you use in DDSBlackboxTestsSecurity.cpp

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

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
(cherry picked from commit 25dee05)

Co-authored-by: José Luis Bueno López <[email protected]>
EduPonz pushed a commit that referenced this pull request Dec 20, 2023
@JLBuenoLopez @MiguelCompany @EduPonz
Fix memory problem when ciphering payload (#2485)
fb228e2 
* Refs 13364. Regression test.

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

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

* Refs 13364. Test with different lengths.

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

* Refs 13364. Apply review suggestions.

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

* Refs 13364. Fix #2379.

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

* Include what you use in DDSBlackboxTestsSecurity.cpp

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

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
MiguelCompany added a commit that referenced this pull request Dec 21, 2023
@JLBuenoLopez @MiguelCompany
Fix memory problem when ciphering payload (#2485)
40eb65d 
* Refs 13364. Regression test.

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

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

* Refs 13364. Test with different lengths.

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

* Refs 13364. Apply review suggestions.

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

* Refs 13364. Fix #2379.

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

* Include what you use in DDSBlackboxTestsSecurity.cpp

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

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
MiguelCompany added a commit that referenced this pull request Dec 22, 2023
@mergify @JLBuenoLopez @MiguelCompany
Fix memory problem when ciphering payload (#2485) (#4153)
ffe3b0c 
* Fix memory problem when ciphering payload (#2485)

* Refs 13364. Regression test.

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

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <[email protected]>

* Refs 13364. Test with different lengths.

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

* Refs 13364. Apply review suggestions.

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

* Refs 13364. Fix #2379.

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

* Include what you use in DDSBlackboxTestsSecurity.cpp

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

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <[email protected]>

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: Miguel Company <[email protected]>

* Fix build with TLS, but not security (#4156)

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

---------

Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: JLBuenoLopez-eProsima <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
Co-authored-by: José Luis Bueno López <[email protected]>
Co-authored-by: Miguel Company <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richiware richiware richiware requested changes

@EduPonz EduPonz EduPonz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.13.0
Development

Successfully merging this pull request may close these issues.

Not enough memory to cipher payload [13364]
4 participants
@JLBuenoLopez @MiguelCompany @EduPonz @richiware