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.

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

Not enough memory to cipher payload [13364] #2379

Closed
ghost opened this issue Dec 23, 2021 · 0 comments · Fixed by #2485
Closed

Not enough memory to cipher payload [13364] #2379

ghost opened this issue Dec 23, 2021 · 0 comments · Fixed by #2485
Labels
bug Issue to report a bug

Comments

@ghost
Copy link

ghost commented Dec 23, 2021

There is an error when try to send a big message (near 64 Kb boundary). It seems that encryption overhead is not considered when packing submessages inside RTPSMessageGroup. Such error occurs when the encryption plugin tries to encode the message, but it is too late. All of this can lead to message retransmission or even inability to send the message at all.

Expected Behavior

It should be possible send messages of any size. If the submessage does not fit into the buffer then it should be split into smaller chunks.

Current Behavior

There are the following error logs:

2021-12-23 14:06:46.067 [SECURITY_CRYPTO Error] Not enough memory to cipher payload -> Function serialize_SecureDataBody
2021-12-23 14:06:46.067 [RTPS_WRITER Error] Error encoding rtps message. -> Function send

Steps to Reproduce

  1. Compile the test program (attached).
  2. Copy certs directory where you want to run the program.
  3. Run the reader instance: ./big_message_test
  4. Run the writer instance: ./big_message_test writer

System information

  • Fast-RTPS version: 2.4.0
  • OS: Ubuntu 20.04

Additional context

There is some attempt to limit a message size in DataMsg::addSubmessageData():

    if (size32 <= std::numeric_limits<uint16_t>::max())
    {
        submessage_size = static_cast<uint16_t>(size32);
        octet* o = reinterpret_cast<octet*>(&submessage_size);
        if (msg->msg_endian == DEFAULT_ENDIAN)
        {
            msg->buffer[submessage_size_pos] = *(o);
            msg->buffer[submessage_size_pos + 1] = *(o + 1);
        }
        else
        {
            msg->buffer[submessage_size_pos] = *(o + 1);
            msg->buffer[submessage_size_pos + 1] = *(o);
        }

        *is_big_submessage = false;
    }

But this code does not consider an encryption overhead and the fact that buffer size can less than a hardcoded value. Right now the buffer size is less than 65535, because of memory alignment.
Also in the function RTPSMessageGroup::insert_submessage():

    if (!append_message(full_msg_, submessage_msg_))
    {
        // Retry
        flush_and_reset();
        add_info_dst_in_buffer(full_msg_, destination_guid_prefix);

        if (!append_message(full_msg_, submessage_msg_))
        {
            logError(RTPS_WRITER, "Cannot add RTPS submesage to the CDRMessage. Buffer too small");
            return false;
        }
    }

big_message_test.zip

@MiguelCompany MiguelCompany changed the title Not enough memory to cipher payload. Not enough memory to cipher payload [13364] Jan 3, 2022
@MiguelCompany MiguelCompany added this to the v2.5.1 milestone Jan 3, 2022
@MiguelCompany MiguelCompany added the bug Issue to report a bug label Jan 4, 2022
MiguelCompany added a commit that referenced this issue Jan 5, 2022
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
JLBuenoLopez pushed a commit that referenced this issue Feb 10, 2022
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@JLBuenoLopez JLBuenoLopez modified the milestones: v2.5.1, v2.6.0 Feb 18, 2022
MiguelCompany added a commit that referenced this issue Feb 22, 2022
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
@MiguelCompany MiguelCompany modified the milestones: v2.6.0, v2.6.1 Mar 14, 2022
@EduPonz EduPonz modified the milestones: v2.6.1, v2.7.0 Jun 1, 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
@MiguelCompany MiguelCompany modified the milestones: v2.7.2, 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
@EduPonz EduPonz removed this from the v2.11.0 milestone Jun 22, 2023
EduPonz pushed a commit that referenced this issue Nov 27, 2023
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
MiguelCompany added a commit that referenced this issue Dec 13, 2023
Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
EduPonz pushed a commit that referenced this issue Dec 16, 2023
* Refs 13364. Regression test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 13364. Test with different lengths.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Apply review suggestions.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Fix #2379.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Include what you use in DDSBlackboxTestsSecurity.cpp

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
mergify bot pushed a commit that referenced this issue Dec 16, 2023
* Refs 13364. Regression test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 13364. Test with different lengths.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Apply review suggestions.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Fix #2379.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Include what you use in DDSBlackboxTestsSecurity.cpp

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 25dee05)
mergify bot pushed a commit that referenced this issue Dec 16, 2023
* Refs 13364. Regression test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 13364. Test with different lengths.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Apply review suggestions.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Fix #2379.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Include what you use in DDSBlackboxTestsSecurity.cpp

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 25dee05)
mergify bot pushed a commit that referenced this issue Dec 16, 2023
* Refs 13364. Regression test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 13364. Test with different lengths.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Apply review suggestions.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Fix #2379.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Include what you use in DDSBlackboxTestsSecurity.cpp

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 25dee05)
EduPonz pushed a commit that referenced this issue Dec 18, 2023
* Refs 13364. Regression test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 13364. Test with different lengths.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Apply review suggestions.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Fix #2379.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Include what you use in DDSBlackboxTestsSecurity.cpp

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 25dee05)

Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
EduPonz pushed a commit that referenced this issue Dec 18, 2023
* Refs 13364. Regression test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 13364. Test with different lengths.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Apply review suggestions.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Fix #2379.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Include what you use in DDSBlackboxTestsSecurity.cpp

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 25dee05)

Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
EduPonz pushed a commit that referenced this issue Dec 18, 2023
* Refs 13364. Regression test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 13364. Test with different lengths.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Apply review suggestions.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Fix #2379.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Include what you use in DDSBlackboxTestsSecurity.cpp

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
(cherry picked from commit 25dee05)

Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
EduPonz pushed a commit that referenced this issue Dec 20, 2023
* Refs 13364. Regression test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 13364. Test with different lengths.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Apply review suggestions.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Fix #2379.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Include what you use in DDSBlackboxTestsSecurity.cpp

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
MiguelCompany added a commit that referenced this issue Dec 21, 2023
* Refs 13364. Regression test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 13364. Test with different lengths.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Apply review suggestions.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Fix #2379.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Include what you use in DDSBlackboxTestsSecurity.cpp

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
MiguelCompany added a commit that referenced this issue Dec 22, 2023
* Fix memory problem when ciphering payload (#2485)

* Refs 13364. Regression test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. fix build when TLS_FOUND

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

* Refs 13364. Test with different lengths.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Apply review suggestions.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Refs 13364. Fix #2379.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Include what you use in DDSBlackboxTestsSecurity.cpp

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #13364. Fix warning.
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>

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

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: José Luis Bueno López <69244257+JLBuenoLopez-eProsima@users.noreply.github.com>
Co-authored-by: Miguel Company <MiguelCompany@eprosima.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue to report a bug
Projects
None yet
3 participants