From f1ab9f3803e18a167213835e44586c8b89c13c18 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Fri, 9 Jan 2026 14:54:26 +0100 Subject: [PATCH] Require matching GUID prefix in SPDP/SEDP/PMD This prevents participant A from creating/deleting/renewing leases for entities of participant B, reducing the risk of mishaps caused by bugs in remote participants. The downside is that this prevents discovery from being delegated to a different participant. That capability wasn't actually being used anyway. If that capability is introduced, then in all likelihood some of the checks can remain in a slightly altered form. Signed-off-by: Erik Boasson --- src/core/ddsi/src/ddsi_discovery.c | 88 +++++++++------- src/core/ddsi/src/ddsi_discovery_spdp.c | 29 ++++-- src/core/ddsi/src/ddsi_pmd.c | 45 +++++---- src/core/ddsi/tests/plist_leasedur.c | 128 +++++++++++++++++------- src/core/ddsi/tests/pmd_message.c | 66 ++++++++---- 5 files changed, 234 insertions(+), 122 deletions(-) diff --git a/src/core/ddsi/src/ddsi_discovery.c b/src/core/ddsi/src/ddsi_discovery.c index b96af81473..1d444ee187 100644 --- a/src/core/ddsi/src/ddsi_discovery.c +++ b/src/core/ddsi/src/ddsi_discovery.c @@ -113,49 +113,69 @@ bool ddsi_handle_sedp_checks (struct ddsi_domaingv * const gv, ddsi_sedp_kind_t #undef E } -static void ddsi_handle_sedp (const struct ddsi_receiver_state *rst, ddsi_seqno_t seq, struct ddsi_serdata *serdata, ddsi_sedp_kind_t sedp_kind) +static void ddsi_handle_sedp_endpoint (const struct ddsi_receiver_state *rst, ddsi_seqno_t seq, struct ddsi_serdata *serdata, ddsi_sedp_kind_t sedp_kind) { ddsi_plist_t decoded_data; if (ddsi_serdata_to_sample (serdata, &decoded_data, NULL, NULL)) { struct ddsi_domaingv * const gv = rst->gv; GVLOGDISC ("SEDP ST%"PRIx32, serdata->statusinfo); - switch (serdata->statusinfo & (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER)) + + // GUID prefixes should match + if (memcmp (&decoded_data.endpoint_guid.prefix, &rst->src_guid_prefix, sizeof (rst->src_guid_prefix)) != 0) { - case 0: - switch (sedp_kind) - { - case SEDP_KIND_TOPIC: -#ifdef DDS_HAS_TOPIC_DISCOVERY - ddsi_handle_sedp_alive_topic (rst, seq, &decoded_data, rst->vendor, serdata->timestamp); -#endif - break; - case SEDP_KIND_READER: - case SEDP_KIND_WRITER: - ddsi_handle_sedp_alive_endpoint (rst, seq, &decoded_data, sedp_kind, rst->vendor, serdata->timestamp); - break; - } - break; - case DDSI_STATUSINFO_DISPOSE: - case DDSI_STATUSINFO_UNREGISTER: - case (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER): - switch (sedp_kind) - { - case SEDP_KIND_TOPIC: + GVTRACE (" "PGUIDFMT": mismatch with RTPS source "PGUIDPREFIXFMT"\n", PGUID (decoded_data.endpoint_guid), PGUIDPREFIX(rst->src_guid_prefix)); + } + else + { + switch (serdata->statusinfo & (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER)) + { + case 0: + ddsi_handle_sedp_alive_endpoint (rst, seq, &decoded_data, sedp_kind, rst->vendor, serdata->timestamp); + break; + case DDSI_STATUSINFO_DISPOSE: + case DDSI_STATUSINFO_UNREGISTER: + case (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER): + ddsi_handle_sedp_dead_endpoint (rst, &decoded_data, sedp_kind, serdata->timestamp); + break; + } + } + ddsi_plist_fini (&decoded_data); + } +} + #ifdef DDS_HAS_TOPIC_DISCOVERY - ddsi_handle_sedp_dead_topic (rst, &decoded_data, serdata->timestamp); -#endif - break; - case SEDP_KIND_READER: - case SEDP_KIND_WRITER: - ddsi_handle_sedp_dead_endpoint (rst, &decoded_data, sedp_kind, serdata->timestamp); - break; - } - break; +static void ddsi_handle_sedp_topic (const struct ddsi_receiver_state *rst, ddsi_seqno_t seq, struct ddsi_serdata *serdata) +{ + ddsi_plist_t decoded_data; + if (ddsi_serdata_to_sample (serdata, &decoded_data, NULL, NULL)) + { + struct ddsi_domaingv * const gv = rst->gv; + GVLOGDISC ("SEDP ST%"PRIx32, serdata->statusinfo); + + // GUID prefixes should match + if (memcmp (&decoded_data.topic_guid.prefix, &rst->src_guid_prefix, sizeof (rst->src_guid_prefix)) != 0) + { + GVTRACE (" "PGUIDFMT": mismatch with RTPS source "PGUIDPREFIXFMT"\n", PGUID (decoded_data.topic_guid), PGUIDPREFIX(rst->src_guid_prefix)); + } + else + { + switch (serdata->statusinfo & (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER)) + { + case 0: + ddsi_handle_sedp_alive_topic (rst, seq, &decoded_data, rst->vendor, serdata->timestamp); + break; + case DDSI_STATUSINFO_DISPOSE: + case DDSI_STATUSINFO_UNREGISTER: + case (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER): + ddsi_handle_sedp_dead_topic (rst, &decoded_data, serdata->timestamp); + break; + } } ddsi_plist_fini (&decoded_data); } } +#endif #ifdef DDS_HAS_TYPE_DISCOVERY static void handle_typelookup (const struct ddsi_receiver_state *rst, ddsi_entityid_t wr_entity_id, struct ddsi_serdata *serdata) @@ -371,15 +391,15 @@ int ddsi_builtins_dqueue_handler (const struct ddsi_rsample_info *sampleinfo, co break; case DDSI_ENTITYID_SEDP_BUILTIN_PUBLICATIONS_WRITER: case DDSI_ENTITYID_SEDP_BUILTIN_PUBLICATIONS_SECURE_WRITER: - ddsi_handle_sedp (sampleinfo->rst, sampleinfo->seq, d, SEDP_KIND_WRITER); + ddsi_handle_sedp_endpoint (sampleinfo->rst, sampleinfo->seq, d, SEDP_KIND_WRITER); break; case DDSI_ENTITYID_SEDP_BUILTIN_SUBSCRIPTIONS_WRITER: case DDSI_ENTITYID_SEDP_BUILTIN_SUBSCRIPTIONS_SECURE_WRITER: - ddsi_handle_sedp (sampleinfo->rst, sampleinfo->seq, d, SEDP_KIND_READER); + ddsi_handle_sedp_endpoint (sampleinfo->rst, sampleinfo->seq, d, SEDP_KIND_READER); break; #ifdef DDS_HAS_TOPIC_DISCOVERY case DDSI_ENTITYID_SEDP_BUILTIN_TOPIC_WRITER: - ddsi_handle_sedp (sampleinfo->rst, sampleinfo->seq, d, SEDP_KIND_TOPIC); + ddsi_handle_sedp_topic (sampleinfo->rst, sampleinfo->seq, d); break; #endif case DDSI_ENTITYID_P2P_BUILTIN_PARTICIPANT_MESSAGE_WRITER: diff --git a/src/core/ddsi/src/ddsi_discovery_spdp.c b/src/core/ddsi/src/ddsi_discovery_spdp.c index 97675b8bc2..0f042262d9 100644 --- a/src/core/ddsi/src/ddsi_discovery_spdp.c +++ b/src/core/ddsi/src/ddsi_discovery_spdp.c @@ -664,18 +664,25 @@ void ddsi_handle_spdp (const struct ddsi_receiver_state *rst, ddsi_entityid_t pw if (ddsi_serdata_to_sample (serdata, &decoded_data, NULL, NULL)) { enum handle_spdp_result interesting = HSR_NOT_INTERESTING; - switch (serdata->statusinfo & (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER)) + if (memcmp (&decoded_data.participant_guid.prefix, &rst->src_guid_prefix, sizeof (rst->src_guid_prefix)) != 0) { - case 0: - interesting = handle_spdp_alive (rst, seq, serdata->timestamp, &decoded_data); - break; - - case DDSI_STATUSINFO_DISPOSE: - case DDSI_STATUSINFO_UNREGISTER: - case (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER): - handle_spdp_dead (rst, pwr_entityid, serdata->timestamp, &decoded_data, serdata->statusinfo); - interesting = HSR_INTERESTING; - break; + GVTRACE ("SPDP ST%x "PGUIDFMT": mismatch with RTPS source "PGUIDPREFIXFMT, serdata->statusinfo, PGUID (decoded_data.participant_guid), PGUIDPREFIX(rst->src_guid_prefix)); + } + else + { + switch (serdata->statusinfo & (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER)) + { + case 0: + interesting = handle_spdp_alive (rst, seq, serdata->timestamp, &decoded_data); + break; + + case DDSI_STATUSINFO_DISPOSE: + case DDSI_STATUSINFO_UNREGISTER: + case (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER): + handle_spdp_dead (rst, pwr_entityid, serdata->timestamp, &decoded_data, serdata->statusinfo); + interesting = HSR_INTERESTING; + break; + } } ddsi_plist_fini (&decoded_data); diff --git a/src/core/ddsi/src/ddsi_pmd.c b/src/core/ddsi/src/ddsi_pmd.c index ad45f81274..25b3968b58 100644 --- a/src/core/ddsi/src/ddsi_pmd.c +++ b/src/core/ddsi/src/ddsi_pmd.c @@ -87,40 +87,41 @@ void ddsi_handle_pmd_message (const struct ddsi_receiver_state *rst, struct ddsi { /* use sample with knowledge of internal representation: there's a deserialized sample inside already */ const struct ddsi_serdata_pserop *sample = (const struct ddsi_serdata_pserop *) sample_common; - struct ddsi_proxy_participant *proxypp; - ddsi_guid_t ppguid; - struct ddsi_lease *l; - RSTTRACE (" PMD ST%"PRIx32, sample->c.statusinfo); + ddsi_participant_message_data_t const * const pmd = sample->sample; + RSTTRACE (" PMD ST%"PRIx32" pp %"PRIx32":%"PRIx32":%"PRIx32" kind %"PRIu32, sample->c.statusinfo, PGUIDPREFIX (pmd->participantGuidPrefix), pmd->kind); + if (memcmp (&pmd->participantGuidPrefix, &rst->src_guid_prefix, sizeof (rst->src_guid_prefix)) != 0) + { + RSTTRACE (" : mismatch with RTPS source "PGUIDPREFIXFMT"\n", PGUIDPREFIX(rst->src_guid_prefix)); + return; + } + const ddsi_guid_t ppguid = { .prefix = pmd->participantGuidPrefix, .entityid = { .u = DDSI_ENTITYID_PARTICIPANT } }; + struct ddsi_proxy_participant * const proxypp = ddsi_entidx_lookup_proxy_participant_guid (rst->gv->entity_index, &ppguid); + if (proxypp == NULL) + { + RSTTRACE (": unknown proxy participant\n"); + return; + } switch (sample->c.statusinfo & (DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER)) { - case 0: { - const ddsi_participant_message_data_t *pmd = sample->sample; - RSTTRACE (" pp %"PRIx32":%"PRIx32":%"PRIx32" kind %"PRIu32" data %"PRIu32, PGUIDPREFIX (pmd->participantGuidPrefix), pmd->kind, pmd->value.length); - ppguid.prefix = pmd->participantGuidPrefix; - ppguid.entityid.u = DDSI_ENTITYID_PARTICIPANT; - if ((proxypp = ddsi_entidx_lookup_proxy_participant_guid (rst->gv->entity_index, &ppguid)) == NULL) - RSTTRACE (" PPunknown"); - else if (pmd->kind == DDSI_PARTICIPANT_MESSAGE_DATA_KIND_MANUAL_LIVELINESS_UPDATE && - (l = ddsrt_atomic_ldvoidp (&proxypp->minl_man)) != NULL) + case 0: + RSTTRACE (" data %"PRIu32, pmd->value.length); + if (pmd->kind == DDSI_PARTICIPANT_MESSAGE_DATA_KIND_MANUAL_LIVELINESS_UPDATE) { /* Renew lease for entity with shortest manual-by-participant lease */ - ddsi_lease_renew (l, ddsrt_time_elapsed ()); + struct ddsi_lease * const l = ddsrt_atomic_ldvoidp (&proxypp->minl_man); + if (l != NULL) + ddsi_lease_renew (l, ddsrt_time_elapsed ()); } break; - } case DDSI_STATUSINFO_DISPOSE: case DDSI_STATUSINFO_UNREGISTER: - case DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER: { - const ddsi_participant_message_data_t *pmd = sample->sample; - ppguid.prefix = pmd->participantGuidPrefix; - ppguid.entityid.u = DDSI_ENTITYID_PARTICIPANT; + case DDSI_STATUSINFO_DISPOSE | DDSI_STATUSINFO_UNREGISTER: if (ddsi_delete_proxy_participant_by_guid (rst->gv, &ppguid, sample->c.timestamp, false) < 0) - RSTTRACE (" unknown"); + RSTTRACE (": unknown"); else - RSTTRACE (" delete"); + RSTTRACE (": delete"); break; - } } RSTTRACE ("\n"); } diff --git a/src/core/ddsi/tests/plist_leasedur.c b/src/core/ddsi/tests/plist_leasedur.c index cdd5a9452f..d17ccd0df1 100644 --- a/src/core/ddsi/tests/plist_leasedur.c +++ b/src/core/ddsi/tests/plist_leasedur.c @@ -226,11 +226,33 @@ CU_Test (ddsi_plist_leasedur, ser_others, .init = setup, .fini = teardown) SER32BE(0),SER32BE(0),SER32BE(0), \ (a),(b),(c),(d) -#define TEST_GUIDPREFIX_BYTES 7,7,3,4, 5,6,7,8, 9,10,11,12 +#define TEST_GUIDPREFIX_BYTES(use_correct_prefix) (use_correct_prefix ? 7 : 8),7,3,4, 5,6,7,8, 9,10,11,12 + +struct logger_arg { + ddsrt_atomic_uint32_t match; +}; + +static struct logger_arg logger_arg = { + DDSRT_ATOMIC_UINT32_INIT (0) +}; + +static void logger (void *ptr, const dds_log_data_t *data) +{ + struct logger_arg *arg = ptr; + printf ("%s", data->message); + fflush (stdout); + if (strstr (data->message, "mismatch with RTPS source")) + ddsrt_atomic_or32 (&arg->match, 1); +} static void setup_and_start (void) { setup (); + dds_set_log_sink (&logger, &logger_arg); + dds_set_trace_sink (&logger, &logger_arg); + // not very proper to do this here + dds_log_cfg_init (&gv.logconfig, gv.config.domainId, DDS_LC_TRACE, stderr, NULL); + ddsi_set_deafmute (&gv, true, true, DDS_INFINITY); ddsi_start (&gv); // Register the main thread, then claim it as spawned by Cyclone because the @@ -240,10 +262,14 @@ static void setup_and_start (void) assert (thrst->state == DDSI_THREAD_STATE_LAZILY_CREATED); thrst->state = DDSI_THREAD_STATE_ALIVE; ddsrt_atomic_stvoidp (&thrst->gv, &gv); + ddsrt_atomic_st32 (&logger_arg.match, 0); } static void stop_and_teardown (void) { + dds_set_log_sink (0, 0); + dds_set_trace_sink (0, 0); + // Shutdown currently relies on sending packets to shutdown receiver threads // handling individual sockets (this sometime causes issues with firewalls, too) ddsi_set_deafmute (&gv, false, false, DDS_INFINITY); @@ -255,7 +281,7 @@ static void stop_and_teardown (void) teardown (); } -static void ddsi_plist_leasedur_new_proxypp_impl (bool include_lease_duration) +static void ddsi_plist_leasedur_new_proxypp_impl (bool include_lease_duration, bool use_correct_prefix) { struct ddsi_thread_state * const thrst = ddsi_lookup_thread_state (); const uint32_t rport = gv.loc_meta_uc.port; @@ -268,7 +294,7 @@ static void ddsi_plist_leasedur_new_proxypp_impl (bool include_lease_duration) 1, DDSI_VENDORID_MINOR_ECLIPSE, // GUID prefix: first two bytes ordinarily have vendor id, so 7,7 is // guaranteed to not be used locally - TEST_GUIDPREFIX_BYTES, + TEST_GUIDPREFIX_BYTES (true), // DATA: flags (4 = dataflag + big-endian); octets-to-next-header = 0 // means it continues until the end DDSI_RTPS_SMID_DATA, 4, 0,0, @@ -280,7 +306,7 @@ static void ddsi_plist_leasedur_new_proxypp_impl (bool include_lease_duration) 0,2, // PL_CDR_BE 0,0, // options = 0 HDR (DDSI_PID_PARTICIPANT_GUID, 16), - TEST_GUIDPREFIX_BYTES, SER32BE (DDSI_ENTITYID_PARTICIPANT), + TEST_GUIDPREFIX_BYTES (use_correct_prefix), SER32BE (DDSI_ENTITYID_PARTICIPANT), HDR (DDSI_PID_BUILTIN_ENDPOINT_SET, 4), SER32BE (DDSI_DISC_BUILTIN_ENDPOINT_SUBSCRIPTION_ANNOUNCER), HDR (DDSI_PID_PROTOCOL_VERSION, 4), gv.config.protocol_version.major, gv.config.protocol_version.minor, 0,0, HDR (DDSI_PID_VENDORID, 4), 1, DDSI_VENDORID_MINOR_ECLIPSE, 0,0, @@ -299,7 +325,7 @@ static void ddsi_plist_leasedur_new_proxypp_impl (bool include_lease_duration) pktinfo.dst.kind = DDSI_LOCATOR_KIND_INVALID; pktinfo.if_index = 0; const ddsi_guid_t proxypp_guid = { - .prefix = ddsi_ntoh_guid_prefix ((ddsi_guid_prefix_t){ .s = { TEST_GUIDPREFIX_BYTES } }), + .prefix = ddsi_ntoh_guid_prefix ((ddsi_guid_prefix_t){ .s = { TEST_GUIDPREFIX_BYTES (use_correct_prefix) } }), .entityid = { .u = DDSI_ENTITYID_PARTICIPANT } }; @@ -327,7 +353,7 @@ static void ddsi_plist_leasedur_new_proxypp_impl (bool include_lease_duration) const dds_time_t tend = dds_time () + DDS_SECS (10); struct ddsi_proxy_participant *proxypp = NULL; ddsi_thread_state_awake (thrst, &gv); - while (proxypp == NULL && dds_time () < tend) + while ((proxypp == NULL && ddsrt_atomic_ld32 (&logger_arg.match) == 0) && dds_time () < tend) { ddsi_thread_state_asleep (thrst); dds_sleepfor (DDS_MSECS (10)); @@ -336,30 +362,45 @@ static void ddsi_plist_leasedur_new_proxypp_impl (bool include_lease_duration) } // After waiting for a reasonable amount of time, the (fake) proxy participant - // should exist and have picked up the lease duration from the message - CU_ASSERT_NEQ_FATAL (proxypp, NULL); - CU_ASSERT_NEQ_FATAL (proxypp->plist->qos.present & DDSI_QP_LIVELINESS, 0); - CU_ASSERT_EQ_FATAL (proxypp->plist->qos.liveliness.kind, DDS_LIVELINESS_AUTOMATIC); - if (include_lease_duration) { - CU_ASSERT_EQ_FATAL (proxypp->plist->qos.liveliness.lease_duration, 3071111111); - } else { - CU_ASSERT_EQ_FATAL (proxypp->plist->qos.liveliness.lease_duration, DDS_SECS (100)); + // should exist and have picked up the lease duration from the message, unless + // we introduced a mismatch between the source prefix and the GUID in the + // discovery data + if (!use_correct_prefix) + { + CU_ASSERT_EQ_FATAL (proxypp, NULL); + CU_ASSERT_NEQ_FATAL (ddsrt_atomic_ld32 (&logger_arg.match), 0); + } + else + { + CU_ASSERT_NEQ_FATAL (proxypp, NULL); + CU_ASSERT_NEQ_FATAL (proxypp->plist->qos.present & DDSI_QP_LIVELINESS, 0); + CU_ASSERT_EQ_FATAL (proxypp->plist->qos.liveliness.kind, DDS_LIVELINESS_AUTOMATIC); + if (include_lease_duration) { + CU_ASSERT_EQ_FATAL (proxypp->plist->qos.liveliness.lease_duration, 3071111111); + } else { + CU_ASSERT_EQ_FATAL (proxypp->plist->qos.liveliness.lease_duration, DDS_SECS (100)); + } + CU_ASSERT_EQ_FATAL (proxypp->lease->tdur, proxypp->plist->qos.liveliness.lease_duration); } - CU_ASSERT_EQ_FATAL (proxypp->lease->tdur, proxypp->plist->qos.liveliness.lease_duration); ddsi_thread_state_asleep (thrst); } CU_Test (ddsi_plist_leasedur, new_proxypp, .init = setup_and_start, .fini = stop_and_teardown) { - ddsi_plist_leasedur_new_proxypp_impl (true); + ddsi_plist_leasedur_new_proxypp_impl (true, true); +} + +CU_Test (ddsi_plist_leasedur, new_proxypp_wrong_prefix, .init = setup_and_start, .fini = stop_and_teardown) +{ + ddsi_plist_leasedur_new_proxypp_impl (true, false); } CU_Test (ddsi_plist_leasedur, new_proxypp_def, .init = setup_and_start, .fini = stop_and_teardown) { - ddsi_plist_leasedur_new_proxypp_impl (false); + ddsi_plist_leasedur_new_proxypp_impl (false, true); } -static void ddsi_plist_leasedur_new_proxyrd_impl (bool include_lease_duration) +static void ddsi_plist_leasedur_new_proxyrd_impl (bool include_lease_duration, bool use_correct_prefix) { struct ddsi_thread_state * const thrst = ddsi_lookup_thread_state (); @@ -383,7 +424,7 @@ static void ddsi_plist_leasedur_new_proxyrd_impl (bool include_lease_duration) 1, DDSI_VENDORID_MINOR_ECLIPSE, // GUID prefix: first two bytes ordinarily have vendor id, so 7,7 is // guaranteed to not be used locally - TEST_GUIDPREFIX_BYTES, + TEST_GUIDPREFIX_BYTES (true), // INFO_DST: flags (0 = big-endian); octets-to-next-header = 12 DDSI_RTPS_SMID_INFO_DST, 0, 0,12 // guid prefix of local node (= ppguidprefix_base) comes here @@ -409,7 +450,7 @@ static void ddsi_plist_leasedur_new_proxyrd_impl (bool include_lease_duration) 0,2, // PL_CDR_BE 0,0, // options = 0 HDR (DDSI_PID_ENDPOINT_GUID, 16), - TEST_GUIDPREFIX_BYTES, SER32BE (0x107), // reader-with-key + TEST_GUIDPREFIX_BYTES (use_correct_prefix), SER32BE (0x107), // reader-with-key HDR (DDSI_PID_TOPIC_NAME, 12), SER32BE (6), 't','o','p','i','c',0, 0,0, HDR (DDSI_PID_TYPE_NAME, 12), SER32BE (5), 't','y','p','e',0, 0,0,0, }; @@ -426,7 +467,7 @@ static void ddsi_plist_leasedur_new_proxyrd_impl (bool include_lease_duration) pktinfo.dst.kind = DDSI_LOCATOR_KIND_INVALID; pktinfo.if_index = 0; const ddsi_guid_t prd_guid = { - .prefix = ddsi_ntoh_guid_prefix ((ddsi_guid_prefix_t){ .s = { TEST_GUIDPREFIX_BYTES } }), + .prefix = ddsi_ntoh_guid_prefix ((ddsi_guid_prefix_t){ .s = { TEST_GUIDPREFIX_BYTES (use_correct_prefix) } }), .entityid = { .u = 0x107 } }; @@ -461,7 +502,7 @@ static void ddsi_plist_leasedur_new_proxyrd_impl (bool include_lease_duration) const dds_time_t tend = dds_time () + DDS_SECS (10); struct ddsi_proxy_reader *prd = NULL; ddsi_thread_state_awake (thrst, &gv); - while (prd == NULL && dds_time () < tend) + while ((prd == NULL && ddsrt_atomic_ld32 (&logger_arg.match) == 0) && dds_time () < tend) { ddsi_thread_state_asleep (thrst); dds_sleepfor (DDS_MSECS (10)); @@ -470,27 +511,44 @@ static void ddsi_plist_leasedur_new_proxyrd_impl (bool include_lease_duration) } // After waiting for a reasonable amount of time, the (fake) proxy participant - // should exist and have picked up the lease duration from the message - CU_ASSERT_NEQ_FATAL (prd, NULL); - CU_ASSERT_NEQ_FATAL (prd->c.xqos->present & DDSI_QP_LIVELINESS, 0); - if (include_lease_duration) { - CU_ASSERT_EQ (prd->c.xqos->liveliness.kind, DDS_LIVELINESS_MANUAL_BY_PARTICIPANT); - CU_ASSERT_EQ (prd->c.xqos->liveliness.lease_duration, 2041944443); - } else { - CU_ASSERT_EQ (prd->c.xqos->liveliness.kind, DDS_LIVELINESS_AUTOMATIC); - CU_ASSERT_EQ (prd->c.xqos->liveliness.lease_duration, DDS_INFINITY); + // should exist and have picked up the lease duration from the message, unless + // we introduced a mismatch between the source prefix and the GUID in the + // discovery data + if (!use_correct_prefix) + { + CU_ASSERT_EQ_FATAL (prd, NULL); + CU_ASSERT_NEQ_FATAL (ddsrt_atomic_ld32 (&logger_arg.match), 0); + } + else + { + CU_ASSERT_NEQ_FATAL (prd, NULL); + CU_ASSERT_NEQ_FATAL (prd->c.xqos->present & DDSI_QP_LIVELINESS, 0); + if (include_lease_duration) { + CU_ASSERT_EQ (prd->c.xqos->liveliness.kind, DDS_LIVELINESS_MANUAL_BY_PARTICIPANT); + CU_ASSERT_EQ (prd->c.xqos->liveliness.lease_duration, 2041944443); + } else { + CU_ASSERT_EQ (prd->c.xqos->liveliness.kind, DDS_LIVELINESS_AUTOMATIC); + CU_ASSERT_EQ (prd->c.xqos->liveliness.lease_duration, DDS_INFINITY); + } } ddsi_thread_state_asleep (thrst); } CU_Test (ddsi_plist_leasedur, new_proxyrd, .init = setup_and_start, .fini = stop_and_teardown) { - ddsi_plist_leasedur_new_proxypp_impl (false); - ddsi_plist_leasedur_new_proxyrd_impl (true); + ddsi_plist_leasedur_new_proxypp_impl (false, true); + ddsi_plist_leasedur_new_proxyrd_impl (true, true); +} + +CU_Test (ddsi_plist_leasedur, new_proxyrd_wrong_prefix, .init = setup_and_start, .fini = stop_and_teardown) +{ + ddsi_plist_leasedur_new_proxypp_impl (false, true); + assert (ddsrt_atomic_ld32 (&logger_arg.match) == 0); + ddsi_plist_leasedur_new_proxyrd_impl (true, false); } CU_Test (ddsi_plist_leasedur, new_proxyrd_def, .init = setup_and_start, .fini = stop_and_teardown) { - ddsi_plist_leasedur_new_proxypp_impl (false); - ddsi_plist_leasedur_new_proxyrd_impl (false); + ddsi_plist_leasedur_new_proxypp_impl (false, true); + ddsi_plist_leasedur_new_proxyrd_impl (false, true); } diff --git a/src/core/ddsi/tests/pmd_message.c b/src/core/ddsi/tests/pmd_message.c index de1f3b7ab9..b6ad3e3e7f 100644 --- a/src/core/ddsi/tests/pmd_message.c +++ b/src/core/ddsi/tests/pmd_message.c @@ -39,7 +39,7 @@ SER32BE(0),SER32BE(0),SER32BE(0), \ (a),(b),(c),(d) -#define TEST_GUIDPREFIX_BYTES 7,7,3,4, 5,6,7,8, 9,10,11,12 +#define TEST_GUIDPREFIX_BYTES(correct_source_prefix) (correct_source_prefix ? 7 : 8),7,3,4, 5,6,7,8, 9,10,11,12 static struct ddsi_cfgst *cfgst; static struct ddsi_domaingv gv; @@ -88,7 +88,9 @@ static void logger (void *ptr, const dds_log_data_t *data) // two bytes are vendor code and not Cyclone DDS, this // suffices if (strstr (data->message, "PMD ST0 pp 707")) - ddsrt_atomic_inc32 (&arg->match); + ddsrt_atomic_or32 (&arg->match, 1); + if (strstr (data->message, "mismatch with RTPS source")) + ddsrt_atomic_or32 (&arg->match, 2); } static void setup_and_start (void) @@ -169,7 +171,7 @@ static void create_fake_proxy_participant (void) 1, DDSI_VENDORID_MINOR_ECLIPSE, // GUID prefix: first two bytes ordinarily have vendor id, so 7,7 is // guaranteed to not be used locally - TEST_GUIDPREFIX_BYTES, + TEST_GUIDPREFIX_BYTES (true), // DATA: flags (4 = dataflag + big-endian); octets-to-next-header = 0 // means it continues until the end DDSI_RTPS_SMID_DATA, 4, 0,0, @@ -181,7 +183,7 @@ static void create_fake_proxy_participant (void) 0,2, // PL_CDR_BE 0,0, // options = 0 HDR (DDSI_PID_PARTICIPANT_GUID, 16), - TEST_GUIDPREFIX_BYTES, SER32BE (DDSI_ENTITYID_PARTICIPANT), + TEST_GUIDPREFIX_BYTES (true), SER32BE (DDSI_ENTITYID_PARTICIPANT), HDR (DDSI_PID_BUILTIN_ENDPOINT_SET, 4), SER32BE (DDSI_DISC_BUILTIN_ENDPOINT_SUBSCRIPTION_ANNOUNCER | DDSI_BUILTIN_ENDPOINT_PARTICIPANT_MESSAGE_DATA_WRITER), HDR (DDSI_PID_PROTOCOL_VERSION, 4), gv.config.protocol_version.major, gv.config.protocol_version.minor, 0,0, @@ -197,7 +199,7 @@ static void create_fake_proxy_participant (void) pktinfo.dst.kind = DDSI_LOCATOR_KIND_INVALID; pktinfo.if_index = 0; const ddsi_guid_t proxypp_guid = { - .prefix = ddsi_ntoh_guid_prefix ((ddsi_guid_prefix_t){ .s = { TEST_GUIDPREFIX_BYTES } }), + .prefix = ddsi_ntoh_guid_prefix ((ddsi_guid_prefix_t){ .s = { TEST_GUIDPREFIX_BYTES (true) } }), .entityid = { .u = DDSI_ENTITYID_PARTICIPANT } }; @@ -236,7 +238,13 @@ static void create_fake_proxy_participant (void) CU_ASSERT_EQ_FATAL (ret, 0); } -static void send_pmd_message (uint32_t seqlo, uint16_t encoding, uint16_t options, uint32_t kind, uint32_t seq_length, uint32_t act_payload_size, bool msg_is_valid) +enum message_variant { + MV_VALID, + MV_WRONG_PREFIX, + MV_MALFORMED +}; + +static void send_pmd_message (uint32_t seqlo, uint16_t encoding, uint16_t options, uint32_t kind, uint32_t seq_length, uint32_t act_payload_size, enum message_variant variant) { // actual sequence length must be in range of our message bytes following the // CDR encoding+options, we don't want an out-of-bounds read @@ -251,7 +259,7 @@ static void send_pmd_message (uint32_t seqlo, uint16_t encoding, uint16_t option 1, DDSI_VENDORID_MINOR_ECLIPSE, // GUID prefix: first two bytes ordinarily have vendor id, so 7,7 is // guaranteed to not be used locally - TEST_GUIDPREFIX_BYTES, + TEST_GUIDPREFIX_BYTES (true), // INFO_DST or it won't accept the heartbeat as a handshake one DDSI_RTPS_SMID_INFO_DST, 0, 0,12, // flags, octets-to-next-header SER32BE (ppguid.prefix.u[0]), SER32BE (ppguid.prefix.u[1]), SER32BE (ppguid.prefix.u[2]), @@ -273,7 +281,7 @@ static void send_pmd_message (uint32_t seqlo, uint16_t encoding, uint16_t option (unsigned char) (encoding >> 8), (unsigned char) (encoding & 0xff), (unsigned char) (options >> 8), (unsigned char) (options & 0xff), // PMD message payload: - TEST_GUIDPREFIX_BYTES, + TEST_GUIDPREFIX_BYTES (variant != MV_WRONG_PREFIX), SER32BE (kind), SER32BE (seq_length), SER32BE (0) @@ -299,31 +307,49 @@ static void send_pmd_message (uint32_t seqlo, uint16_t encoding, uint16_t option // wait until PMD message has been processed wait_for_dqueue (); - CU_ASSERT_FATAL (msg_is_valid == (ddsrt_atomic_ld32 (&logger_arg.match) == 1)); + const uint32_t matches = ddsrt_atomic_ld32 (&logger_arg.match); + switch (variant) + { + case MV_VALID: + CU_ASSERT_EQ_FATAL (matches, 1); + break; + case MV_WRONG_PREFIX: + CU_ASSERT_EQ_FATAL (matches, 2); + break; + case MV_MALFORMED: + CU_ASSERT_EQ_FATAL (matches, 0); + break; + } } CU_Test (ddsi_pmd_message, valid, .init = setup_and_start, .fini = stop_and_teardown) { create_fake_proxy_participant (); - send_pmd_message (1, DDSI_RTPS_CDR_BE, 0, 0, 0, 20, true); // auto - send_pmd_message (2, DDSI_RTPS_CDR_BE, 0, 1, 0, 20, true); // manual - send_pmd_message (3, DDSI_RTPS_CDR_BE, 0, 2, 0, 20, true); // meaningless, ignored (log line is still output) - send_pmd_message (4, DDSI_RTPS_CDR_BE, 3, 0, 1, 24, true); // 3 padding bytes - send_pmd_message (5, DDSI_RTPS_CDR_BE, 0, 0, 4, 24, true); + send_pmd_message (1, DDSI_RTPS_CDR_BE, 0, 0, 0, 20, MV_VALID); // auto + send_pmd_message (2, DDSI_RTPS_CDR_BE, 0, 1, 0, 20, MV_VALID); // manual + send_pmd_message (3, DDSI_RTPS_CDR_BE, 0, 2, 0, 20, MV_VALID); // meaningless, ignored (log line is still output) + send_pmd_message (4, DDSI_RTPS_CDR_BE, 3, 0, 1, 24, MV_VALID); // 3 padding bytes + send_pmd_message (5, DDSI_RTPS_CDR_BE, 0, 0, 4, 24, MV_VALID); } CU_Test (ddsi_pmd_message, invalid_sequence, .init = setup_and_start, .fini = stop_and_teardown) { create_fake_proxy_participant (); - send_pmd_message (1, DDSI_RTPS_CDR_BE, 0, 0, 8, 24, false); // only have up to 4 bytes for octet sequence - send_pmd_message (2, DDSI_RTPS_CDR_BE, 3, 0, 4, 24, true); // not valid but XTypes' padding-at-end field currently ignored + send_pmd_message (1, DDSI_RTPS_CDR_BE, 0, 0, 8, 24, MV_MALFORMED); // only have up to 4 bytes for octet sequence + send_pmd_message (2, DDSI_RTPS_CDR_BE, 3, 0, 4, 24, MV_VALID); // not valid but XTypes' padding-at-end field currently ignored } CU_Test (ddsi_pmd_message, bogus_header, .init = setup_and_start, .fini = stop_and_teardown) { create_fake_proxy_participant (); - send_pmd_message (1, DDSI_RTPS_CDR_BE, 0xa481, 0, 0, 20, true); // options may be anything, XTypes' padding-at-end field currently ignored - send_pmd_message (2, DDSI_RTPS_CDR_BE, 0xa481, 0, 0, 16, false); // short - send_pmd_message (3, DDSI_RTPS_CDR_BE, 0xa481, 0, 0, 0, false); // nothing at all -> used to trigger an assert - send_pmd_message (4, 0xa481, 0, 0, 0, 0, false); + send_pmd_message (1, DDSI_RTPS_CDR_BE, 0xa481, 0, 0, 20, MV_VALID); // options may be anything, XTypes' padding-at-end field currently ignored + send_pmd_message (2, DDSI_RTPS_CDR_BE, 0xa481, 0, 0, 16, MV_MALFORMED); // short + send_pmd_message (3, DDSI_RTPS_CDR_BE, 0xa481, 0, 0, 0, MV_MALFORMED); // nothing at all -> used to trigger an assert + send_pmd_message (4, 0xa481, 0, 0, 0, 0, MV_MALFORMED); +} + +CU_Test (ddsi_pmd_message, wrong_prefix, .init = setup_and_start, .fini = stop_and_teardown) +{ + create_fake_proxy_participant (); + send_pmd_message (1, DDSI_RTPS_CDR_BE, 0, 1, 0, 20, MV_WRONG_PREFIX); }