Skip to content

Commit eb29b8a

Browse files
mergify[bot]MiguelCompanycferreiragonz
authored
Assert liveliness with periodic heartbeats in secure participants (#6411) (#6413)
* Assert liveliness with periodic heartbeats in secure participants (#6411) * Refs #24372. Assert liveliness with periodic heartbeats in secure participants. Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> * Refs #24372. Added regression test. Signed-off-by: danipiza <dpizarrogallego@gmail.com> * Refs #24372. Revert change in returned value. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #24372. Improve null pointer checks. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #24372. Uncrustify. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Juan Lopez Fernandez <juanlopez@eprosima.com> Signed-off-by: danipiza <dpizarrogallego@gmail.com> Signed-off-by: Miguel Company <miguelcompany@eprosima.com> Co-authored-by: Juan Lopez Fernandez <juanlopez@eprosima.com> Co-authored-by: danipiza <dpizarrogallego@gmail.com> (cherry picked from commit 910ac99) # Conflicts: # src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp * Fix conflicts Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com> * Backport FASTDDS_UNREACHABLE() Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com> * Uncrustify Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com> --------- Signed-off-by: Carlos Ferreira González <carlosferreira@eprosima.com> Co-authored-by: Miguel Company <miguelcompany@eprosima.com> Co-authored-by: Carlos Ferreira González <carlosferreira@eprosima.com>
1 parent 08dc941 commit eb29b8a

3 files changed

Lines changed: 121 additions & 4 deletions

File tree

‎include/fastdds/config.hpp.in‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,4 +124,13 @@
124124
(FASTDDS_VERSION_MAJOR == major && FASTDDS_VERSION_MINOR < minor), \
125125
"TODO before version " #major "." #minor " : " #msg)
126126

127+
#if defined(_MSC_VER) && !defined(__clang__)
128+
# define FASTDDS_UNREACHABLE() __assume(0)
129+
#elif defined(__GNUC__) || defined(__clang__)
130+
# define FASTDDS_UNREACHABLE() __builtin_unreachable()
131+
#else
132+
# include <cstdlib>
133+
# define FASTDDS_UNREACHABLE() std::abort()
134+
#endif // if defined(_MSC_VER) && !defined(__clang__)
135+
127136
#endif // FASTDDS__CONFIG_H

‎src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,22 @@ void PDPSimple::announceParticipantState(
291291
if (!(dispose || new_change))
292292
{
293293
endpoints->writer.writer_->send_periodic_announcement();
294+
295+
#if HAVE_SECURITY
296+
if (mp_RTPSParticipant->is_secure())
297+
{
298+
// PDP non-secure endpoints are unmatched after participant authentication succeeds (and secure PDP
299+
// endpoints are matched), and since the secure ones are TRANSIENT_LOCAL, we send periodic heartbeats
300+
// to assert liveliness on remote participants
301+
auto secure = dynamic_cast<fastdds::rtps::SimplePDPEndpointsSecure*>(builtin_endpoints_.get());
302+
assert(secure && secure->secure_writer.writer_);
303+
if (!secure || !secure->secure_writer.writer_)
304+
{
305+
FASTDDS_UNREACHABLE(); // “cannot happen” invariant
306+
}
307+
secure->secure_writer.writer_->send_periodic_heartbeat(true, true);
308+
}
309+
#endif // HAVE_SECURITY
294310
}
295311
}
296312
}
@@ -683,8 +699,8 @@ void PDPSimple::match_pdp_remote_endpoints(
683699
reader->getGuid(), pdata.guid, *temp_writer_data,
684700
reader->getAttributes().security_attributes()))
685701
{
686-
EPROSIMA_LOG_ERROR(RTPS_EDP, "Security manager returns an error for writer " <<
687-
temp_writer_data->guid);
702+
EPROSIMA_LOG_ERROR(RTPS_EDP, "Security manager returns an error for writer "
703+
<< temp_writer_data->guid);
688704
}
689705
}
690706
else
@@ -713,8 +729,8 @@ void PDPSimple::match_pdp_remote_endpoints(
713729
writer->getGuid(), pdata.guid, *temp_reader_data,
714730
writer->getAttributes().security_attributes()))
715731
{
716-
EPROSIMA_LOG_ERROR(RTPS_EDP, "Security manager returns an error for reader " <<
717-
temp_reader_data->guid);
732+
EPROSIMA_LOG_ERROR(RTPS_EDP, "Security manager returns an error for reader "
733+
<< temp_reader_data->guid);
718734
}
719735
}
720736
else

‎test/blackbox/common/BlackboxTestsSecurity.cpp‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,6 +1319,98 @@ TEST_P(Security, RemoveParticipantProxyDataonSecurityManagerLeaseExpired_validat
13191319

13201320
}
13211321

1322+
// Regression test for secure PDP liveliness maintenance without user traffic.
1323+
TEST(Security, SecureParticipantsDoNotLoseDiscoveryWithoutUserTraffic)
1324+
{
1325+
PubSubReader<HelloWorldPubSubType> reader("HelloWorldTopic_secure_participant_liveliness");
1326+
PubSubWriter<HelloWorldPubSubType> writer("HelloWorldTopic_secure_participant_liveliness");
1327+
const auto idle_timeout = std::chrono::seconds(6);
1328+
const auto data_timeout = std::chrono::seconds(10);
1329+
// Keep the lease short so loss of secure PDP liveliness shows up quickly
1330+
const auto lease_duration = eprosima::fastdds::dds::Duration_t(3, 0);
1331+
const auto announcement_period = eprosima::fastdds::dds::Duration_t(1, 0);
1332+
// Use UDP transport to later block the fallback participant DATA(P) traffic
1333+
auto reader_transport = std::make_shared<test_UDPv4TransportDescriptor>();
1334+
auto writer_transport = std::make_shared<test_UDPv4TransportDescriptor>();
1335+
1336+
const std::string governance_file("governance_helloworld_all_enable.smime");
1337+
const std::string permissions_file("permissions_helloworld.smime");
1338+
1339+
CommonPermissionsConfigure(reader, writer, governance_file, permissions_file);
1340+
// Force the test through UDP to avoid local shortcuts masking the secure discovery behavior
1341+
reader.disable_builtin_transport().add_user_transport_to_pparams(reader_transport);
1342+
writer.disable_builtin_transport().add_user_transport_to_pparams(writer_transport);
1343+
1344+
// Two secure participants that authenticate, discover each other, and exchange user data
1345+
reader.lease_duration(lease_duration, announcement_period)
1346+
.history_depth(10)
1347+
.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS)
1348+
.init();
1349+
ASSERT_TRUE(reader.isInitialized());
1350+
1351+
writer.lease_duration(lease_duration, announcement_period)
1352+
.history_depth(10)
1353+
.reliability(eprosima::fastdds::dds::RELIABLE_RELIABILITY_QOS)
1354+
.init();
1355+
ASSERT_TRUE(writer.isInitialized());
1356+
1357+
reader.wait_authorized();
1358+
writer.wait_authorized();
1359+
1360+
// Wait until both endpoints are matched
1361+
reader.wait_discovery();
1362+
writer.wait_discovery();
1363+
1364+
// Verify discovery state stayed stable
1365+
auto assert_still_discovered = [&reader, &writer]()
1366+
{
1367+
ASSERT_TRUE(reader.is_matched());
1368+
ASSERT_TRUE(writer.is_matched());
1369+
ASSERT_EQ(reader.get_matched(), 1u);
1370+
ASSERT_EQ(writer.get_matched(), 1u);
1371+
1372+
const auto reader_status = reader.get_subscription_matched_status();
1373+
const auto writer_status = writer.get_publication_matched_status();
1374+
ASSERT_EQ(reader_status.total_count, 1);
1375+
ASSERT_EQ(writer_status.total_count, 1);
1376+
};
1377+
1378+
// Check the secure participants must not be undiscovered just because user traffic stops
1379+
auto assert_idle_period_keeps_discovery = [&reader, &writer, &idle_timeout, &assert_still_discovered]()
1380+
{
1381+
ASSERT_FALSE(reader.wait_participant_undiscovery(idle_timeout));
1382+
ASSERT_FALSE(writer.wait_participant_undiscovery(idle_timeout));
1383+
1384+
assert_still_discovered();
1385+
};
1386+
1387+
// Discovery is only useful if data can still flow after the idle window
1388+
auto assert_data_flow = [&reader, &writer, &data_timeout, &assert_still_discovered]()
1389+
{
1390+
auto data = default_helloworld_data_generator(2);
1391+
1392+
reader.startReception(data);
1393+
writer.send(data);
1394+
1395+
ASSERT_TRUE(data.empty());
1396+
ASSERT_EQ(reader.block_for_all(data_timeout), 2u);
1397+
assert_still_discovered();
1398+
};
1399+
1400+
ASSERT_NO_FATAL_FAILURE(assert_still_discovered());
1401+
1402+
// After the secure PDP endpoints are established, stop the unprotected participant DATA(P)
1403+
// traffic, to depend on the secure liveliness maintenance path
1404+
reader_transport->test_transport_options->always_drop_participant_builtin_topic_data = true;
1405+
writer_transport->test_transport_options->always_drop_participant_builtin_topic_data = true;
1406+
1407+
// Exercise the idle scenario twice to catch both immediate loss and unstable recovery/rematch behavior
1408+
ASSERT_NO_FATAL_FAILURE(assert_idle_period_keeps_discovery());
1409+
ASSERT_NO_FATAL_FAILURE(assert_data_flow());
1410+
ASSERT_NO_FATAL_FAILURE(assert_idle_period_keeps_discovery());
1411+
ASSERT_NO_FATAL_FAILURE(assert_data_flow());
1412+
}
1413+
13221414
TEST(Security, AllowUnauthenticatedParticipants_EntityCreationFailsIfRTPSProtectionIsNotNONE)
13231415
{
13241416
PubSubReader<HelloWorldPubSubType> reader("HelloWorldTopic");

0 commit comments

Comments
 (0)