Skip to content

Commit

Permalink
Fix memory problem when ciphering payload (#2485)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
2 people authored and EduPonz committed Dec 20, 2023
1 parent 40e11be commit fb228e2
Show file tree
Hide file tree
Showing 10 changed files with 223 additions and 22 deletions.
27 changes: 19 additions & 8 deletions src/cpp/rtps/messages/RTPSMessageGroup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,29 @@ static bool data_exceeds_limitation(
}

static bool append_message(
RTPSParticipantImpl* participant,
CDRMessage_t* full_msg,
CDRMessage_t* submsg)
{
#ifndef FASTDDS_STATISTICS
return CDRMessage::appendMsg(full_msg, submsg);
#else
static_cast<void>(participant);

uint32_t extra_size = 0;

#if HAVE_SECURITY
// Avoid full message growing over estimated extra size for RTPS encryption
extra_size += participant->calculate_extra_size_for_rtps_message();
#endif // HAVE_SECURITY

#ifdef FASTDDS_STATISTICS
// Keep room for the statistics submessage by reducing max_size while appending submessage
full_msg->max_size -= eprosima::fastdds::statistics::rtps::statistics_submessage_length;
extra_size += eprosima::fastdds::statistics::rtps::statistics_submessage_length;
#endif // FASTDDS_STATISTICS

full_msg->max_size -= extra_size;
bool ret_val = CDRMessage::appendMsg(full_msg, submsg);
full_msg->max_size += eprosima::fastdds::statistics::rtps::statistics_submessage_length;
full_msg->max_size += extra_size;

return ret_val;
#endif // FASTDDS_STATISTICS
}

bool sort_changes_group (
Expand Down Expand Up @@ -336,13 +347,13 @@ bool RTPSMessageGroup::insert_submessage(
const GuidPrefix_t& destination_guid_prefix,
bool is_big_submessage)
{
if (!append_message(full_msg_, submessage_msg_))
if (!append_message(participant_, 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_))
if (!append_message(participant_, full_msg_, submessage_msg_))
{
EPROSIMA_LOG_ERROR(RTPS_WRITER, "Cannot add RTPS submesage to the CDRMessage. Buffer too small");
return false;
Expand Down
11 changes: 11 additions & 0 deletions src/cpp/rtps/participant/RTPSParticipantImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,17 @@ class RTPSParticipantImpl
uint32_t length);

#if HAVE_SECURITY
uint32_t calculate_extra_size_for_rtps_message()
{
uint32_t ret_val = 0u;
if (security_attributes_.is_rtps_protected)
{
ret_val = m_security_manager.calculate_extra_size_for_rtps_message();
}

return ret_val;
}

security::SecurityManager& security_manager()
{
return m_security_manager;
Expand Down
1 change: 1 addition & 0 deletions test/blackbox/common/BlackboxTests.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include <functional>

#if HAVE_SECURITY
extern const char* certs_path;
extern void blackbox_security_init();
#endif // if HAVE_SECURITY
#if TLS_FOUND
Expand Down
2 changes: 1 addition & 1 deletion test/blackbox/common/BlackboxTestsSecurity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ using namespace eprosima::fastrtps::rtps;
using test_UDPv4Transport = eprosima::fastdds::rtps::test_UDPv4Transport;
using test_UDPv4TransportDescriptor = eprosima::fastdds::rtps::test_UDPv4TransportDescriptor;

static const char* certs_path = nullptr;
const char* certs_path = nullptr;

enum communication_type
{
Expand Down
4 changes: 0 additions & 4 deletions test/blackbox/common/BlackboxTestsTransportTCP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@
using namespace eprosima::fastrtps;
using namespace eprosima::fastrtps::rtps;

#if TLS_FOUND
static const char* certs_path = nullptr;
#endif // if TLS_FOUND

enum communication_type
{
TRANSPORT
Expand Down
180 changes: 180 additions & 0 deletions test/blackbox/common/DDSBlackboxTestsSecurity.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
// Copyright 2022 Proyectos y Sistemas de Mantenimiento SL (eProsima).
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#include "BlackboxTests.hpp"

#if HAVE_SECURITY

#include <gtest/gtest.h>

#include <thread>

#include <fastdds/dds/domain/DomainParticipant.hpp>
#include <fastdds/dds/domain/DomainParticipantFactory.hpp>
#include <fastdds/dds/publisher/DataWriter.hpp>
#include <fastdds/dds/publisher/DataWriterListener.hpp>
#include <fastdds/dds/publisher/Publisher.hpp>
#include <fastdds/dds/subscriber/DataReader.hpp>
#include <fastdds/dds/subscriber/Subscriber.hpp>
#include <fastrtps/transport/UDPv4TransportDescriptor.h>
#include <fastrtps/types/DynamicDataFactory.h>
#include <fastrtps/types/DynamicTypeBuilder.h>
#include <fastrtps/types/DynamicTypeBuilderFactory.h>
#include <fastrtps/types/DynamicTypeBuilderPtr.h>

namespace fastdds = ::eprosima::fastdds::dds;
using namespace eprosima::fastrtps;
using namespace eprosima::fastrtps::rtps;
using namespace eprosima::fastrtps::types;

void set_authentication_config(
rtps::PropertySeq& props)
{
static constexpr auto kAuthPlugin{ "dds.sec.auth.plugin" };
static constexpr auto kAuthPluginNameValue{ "builtin.PKI-DH" };
static constexpr auto kIdentityCA{ "dds.sec.auth.builtin.PKI-DH.identity_ca" };
static constexpr auto kIdentityCert{ "dds.sec.auth.builtin.PKI-DH.identity_certificate" };
static constexpr auto kIdentityPrivateKey{ "dds.sec.auth.builtin.PKI-DH.private_key" };

props.emplace_back(kAuthPlugin, kAuthPluginNameValue);
props.emplace_back(kIdentityCA, "file://" + std::string(certs_path) + "/maincacert.pem");
props.emplace_back(kIdentityCert, "file://" + std::string(certs_path) + "/mainsubcert.pem");
props.emplace_back(kIdentityPrivateKey, "file://" + std::string(certs_path) + "/mainsubkey.pem");
}

void set_participant_crypto_config(
rtps::PropertySeq& props)
{
static constexpr auto kCryptoPlugin{ "dds.sec.crypto.plugin" };
static constexpr auto kCryptoPluginNameValue{ "builtin.AES-GCM-GMAC" };
static constexpr auto kRtpsProtectionKind{ "rtps.participant.rtps_protection_kind" };
static constexpr auto kProtectionKindValue{ "ENCRYPT" };

props.emplace_back(kCryptoPlugin, kCryptoPluginNameValue);
props.emplace_back(kRtpsProtectionKind, kProtectionKindValue);
}

void test_big_message_corner_case(
const std::string& name,
uint32_t array_length)
{
std::cout << "==== Checking with length = " << array_length << " ====" << std::endl;

struct WriterListener : public fastdds::DataWriterListener
{
WriterListener(
std::atomic_bool& cond_ref)
: cond_(cond_ref)
{
cond_ = false;
}

void on_publication_matched(
fastdds::DataWriter* writer,
const fastdds::PublicationMatchedStatus& info)
{
static_cast<void>(writer);
static_cast<void>(info);

cond_ = true;
}

private:

std::atomic_bool& cond_;
};

auto qos{ fastdds::PARTICIPANT_QOS_DEFAULT };
set_authentication_config(qos.properties().properties());
set_participant_crypto_config(qos.properties().properties());
auto transport = std::make_shared<rtps::UDPv4TransportDescriptor>();
transport->interfaceWhiteList.push_back("127.0.0.1");
qos.transport().use_builtin_transports = false;
qos.transport().user_transports.push_back(transport);
auto participant = fastdds::DomainParticipantFactory::get_instance()->create_participant(0, qos);
ASSERT_NE(nullptr, participant);

std::vector<uint32_t> lengths = { array_length };
DynamicType_ptr base_type = DynamicTypeBuilderFactory::get_instance()->create_char8_type();
DynamicTypeBuilder_ptr builder =
DynamicTypeBuilderFactory::get_instance()->create_array_builder(base_type, lengths);
builder->set_name(name);
DynamicType_ptr array_type = builder->build();

fastdds::TypeSupport type_support(new eprosima::fastrtps::types::DynamicPubSubType(array_type));
type_support.get()->auto_fill_type_information(false);
type_support.get()->auto_fill_type_object(false);
EXPECT_EQ(ReturnCode_t::RETCODE_OK, participant->register_type(type_support));

auto topic = participant->create_topic(name, name, fastdds::TOPIC_QOS_DEFAULT);
ASSERT_NE(nullptr, topic);
auto subscriber = participant->create_subscriber(fastdds::SUBSCRIBER_QOS_DEFAULT);
ASSERT_NE(nullptr, subscriber);
auto reader = subscriber->create_datareader(topic, fastdds::DATAREADER_QOS_DEFAULT);
ASSERT_NE(nullptr, reader);
auto publisher = participant->create_publisher(fastdds::PUBLISHER_QOS_DEFAULT);
ASSERT_NE(nullptr, publisher);

std::atomic_bool writer_matched{ false };
WriterListener wlistener(writer_matched);
auto writer = publisher->create_datawriter(topic, fastdds::DATAWRITER_QOS_DEFAULT, &wlistener,
fastdds::StatusMask::publication_matched());
ASSERT_NE(nullptr, writer);

// Wait for discovery to occur
while (!writer_matched)
{
std::this_thread::sleep_for(std::chrono::milliseconds(100));
}

auto data = eprosima::fastrtps::types::DynamicDataFactory::get_instance()->create_data(array_type);
EXPECT_EQ(ReturnCode_t::RETCODE_OK, writer->write(data, fastdds::HANDLE_NIL));

fastdds::SampleInfo info{};
bool taken = false;
for (size_t n_tries = 0; n_tries < 6u; ++n_tries)
{
if (ReturnCode_t::RETCODE_OK == reader->take_next_sample(data, &info))
{
taken = true;
break;
}
std::this_thread::sleep_for(std::chrono::milliseconds(500));
}

EXPECT_TRUE(taken);
EXPECT_EQ(ReturnCode_t::RETCODE_OK, participant->delete_contained_entities());
fastdds::DomainParticipantFactory::get_instance()->delete_participant(participant);
}

TEST(DDSSecurity, big_message_corner_case)
{
const uint32_t array_lengths[] =
{
// Working case
65200,
// Failing cases
65240, 65252, 65260, 65300,
// Working case
65400
};

std::string topic_name = TEST_TOPIC_NAME;
for (uint32_t len : array_lengths)
{
test_big_message_corner_case(topic_name, len);
}
}

#endif // HAVE_SECURITY
9 changes: 5 additions & 4 deletions test/blackbox/common/TCPReqRepHelloWorldReplier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ void TCPReqRepHelloWorldReplier::init(
int domainId,
uint16_t listeningPort,
uint32_t maxInitialPeer,
const char* certs_path)
const char* certs_folder,
bool use_busy_listener)
{
ParticipantAttributes pattr;
pattr.domainId = domainId;
Expand Down Expand Up @@ -97,14 +98,14 @@ void TCPReqRepHelloWorldReplier::init(
}
descriptor->add_listener_port(listeningPort);

if (certs_path != nullptr)
if (certs_folder != nullptr)
{
using TLSOptions = TCPTransportDescriptor::TLSConfig::TLSOptions;
using TLSVerifyMode = TCPTransportDescriptor::TLSConfig::TLSVerifyMode;
descriptor->apply_security = true;
descriptor->tls_config.password = "testkey";
descriptor->tls_config.cert_chain_file = std::string(certs_path) + "/mainsubcert.pem";
descriptor->tls_config.private_key_file = std::string(certs_path) + "/mainsubkey.pem";
descriptor->tls_config.cert_chain_file = std::string(certs_folder) + "/mainsubcert.pem";
descriptor->tls_config.private_key_file = std::string(certs_folder) + "/mainsubkey.pem";
descriptor->tls_config.verify_mode = TLSVerifyMode::VERIFY_PEER;
descriptor->tls_config.add_option(TLSOptions::DEFAULT_WORKAROUNDS);
descriptor->tls_config.add_option(TLSOptions::SINGLE_DH_USE);
Expand Down
3 changes: 2 additions & 1 deletion test/blackbox/common/TCPReqRepHelloWorldReplier.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ class TCPReqRepHelloWorldReplier
int domainId,
uint16_t listeningPort,
uint32_t maxInitialPeer = 0,
const char* certs_path = nullptr);
const char* certs_folder = nullptr,
bool use_busy_listener = false);
bool isInitialized() const
{
return initialized_;
Expand Down
6 changes: 3 additions & 3 deletions test/blackbox/common/TCPReqRepHelloWorldRequester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void TCPReqRepHelloWorldRequester::init(
int domainId,
uint16_t listeningPort,
uint32_t maxInitialPeer,
const char* certs_path,
const char* certs_folder,
bool force_localhost)
{
ParticipantAttributes pattr;
Expand Down Expand Up @@ -133,13 +133,13 @@ void TCPReqRepHelloWorldRequester::init(
descriptor->maxInitialPeersRange = maxInitialPeer;
}

if (certs_path != nullptr)
if (certs_folder != nullptr)
{
using TLSOptions = TCPTransportDescriptor::TLSConfig::TLSOptions;
using TLSVerifyMode = TCPTransportDescriptor::TLSConfig::TLSVerifyMode;
descriptor->apply_security = true;
//descriptor->tls_config.password = "testkey";
descriptor->tls_config.verify_file = std::string(certs_path) + "/maincacert.pem";
descriptor->tls_config.verify_file = std::string(certs_folder) + "/maincacert.pem";
descriptor->tls_config.verify_mode = TLSVerifyMode::VERIFY_PEER;
descriptor->tls_config.add_option(TLSOptions::DEFAULT_WORKAROUNDS);
descriptor->tls_config.add_option(TLSOptions::SINGLE_DH_USE);
Expand Down
2 changes: 1 addition & 1 deletion test/blackbox/common/TCPReqRepHelloWorldRequester.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ class TCPReqRepHelloWorldRequester
int domainId,
uint16_t listeningPort,
uint32_t maxInitialPeer = 0,
const char* certs_path = nullptr,
const char* certs_folder = nullptr,
bool force_localhost = false);
bool isInitialized() const
{
Expand Down

0 comments on commit fb228e2

Please sign in to comment.