From 7cbf8b7a197f0d77a63139e00b857c745fb2e1c5 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Sun, 5 Jul 2020 09:31:35 +0200 Subject: [PATCH] Make security tests fail less on matching timeouts Some of the tests wait until reader/writing matching has completed in multiple steps and/or interleave entity creation and waiting for matching. This commit moves the waiting to the end and uses a (mostly arbitrary) absolute timeout rather than an arbitrary relative one that could (but fortunately never -- or rarely -- did) add up to durations that could easily cause the overall test to time out. For the specific case of the access control permission expiry test (a particularly problematic case), it uses the first expiry time and gives a bit more time for the discovery. This appears to significantly reduce the number of failures. Signed-off-by: Erik Boasson --- src/security/core/tests/access_control.c | 32 ++++++++++++------- src/security/core/tests/authentication.c | 6 ++-- src/security/core/tests/common/test_utils.c | 6 ++-- src/security/core/tests/common/test_utils.h | 4 +-- src/security/core/tests/crypto.c | 2 +- .../core/tests/secure_communication.c | 5 +-- 6 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/security/core/tests/access_control.c b/src/security/core/tests/access_control.c index 6fbdb52..f0cb87e 100644 --- a/src/security/core/tests/access_control.c +++ b/src/security/core/tests/access_control.c @@ -249,7 +249,7 @@ CU_Theory( dds_entity_t pub, sub; dds_entity_t topic0, topic1; rd_wr_init (g_participant[0], &pub, &topic0, &wr, g_participant[1], &sub, &topic1, &rd, topic_name); - sync_writer_to_readers(g_participant[0], wr, 1, DDS_SECS(2)); + sync_writer_to_readers(g_participant[0], wr, 1, dds_time() + DDS_SECS(2)); write_read_for (wr, g_participant[1], rd, DDS_MSECS (write_read_dur), false, exp_read_fail); } @@ -259,7 +259,7 @@ CU_Theory( #define N_WR 3 #define N_NODES (1 + N_WR) -#define PERM_EXP_BASE 2 +#define PERM_EXP_BASE 3 #define PERM_EXP_INCR 2 /* Tests permissions configuration expiry using multiple writers, to validate that a reader keeps receiving data from writers that have valid permissions config */ @@ -329,11 +329,14 @@ CU_Test(ddssec_access_control, permissions_expiry_multiple, .timeout=20) wr[i] = dds_create_writer (pub, pub_tp, wrqos, NULL); CU_ASSERT_FATAL (wr[i] > 0); dds_set_status_mask (wr[i], DDS_PUBLICATION_MATCHED_STATUS); - sync_writer_to_readers (g_participant[i + 1], wr[i], 1, DDS_SECS(2)); } dds_delete_qos (wrqos); - sync_reader_to_writers (g_participant[0], rd, N_WR, DDS_SECS (2)); + // match deadline follows from expiration times + const dds_time_t sync_abstimeout = t_perm + DDS_SECS (PERM_EXP_BASE + 1); + for (int i = 0; i < N_WR; i++) + sync_writer_to_readers (g_participant[i + 1], wr[i], 1, sync_abstimeout); + sync_reader_to_writers (g_participant[0], rd, N_WR, sync_abstimeout); // write data SecurityCoreTests_Type1 sample = { 1, 1 }; @@ -450,6 +453,8 @@ CU_Theory( if (!exp_pp_fail) { + const dds_time_t sync_abstimeout = dds_time () + DDS_SECS (2); + dds_entity_t lwr = 0, rwr = 0, lrd = 0, rrd = 0; dds_entity_t ltopic[2], rtopic[2]; dds_entity_t lpub, lsub, rpub, rsub; @@ -461,8 +466,6 @@ CU_Theory( g_participant[0], &lpub, <opic[0], &lwr, g_participant[1], &rsub, &rtopic[0], &rrd, topic_name, exp_local_topic_fail, exp_wr_fail, exp_remote_topic_fail, false); - if (!exp_local_topic_fail && !exp_remote_topic_fail && !exp_wr_fail) - sync_writer_to_readers (g_participant[0], lwr, exp_wr_rd_sync_fail ? 0 : 1, DDS_SECS(2)); // Local reader, remote writer create_topic_name (AC_WRAPPER_TOPIC_PREFIX, g_topic_nr++, topic_name, sizeof (topic_name)); @@ -470,8 +473,11 @@ CU_Theory( g_participant[1], &rpub, &rtopic[1], &rwr, g_participant[0], &lsub, <opic[1], &lrd, topic_name, exp_remote_topic_fail, false, exp_local_topic_fail, exp_rd_fail); + + if (!exp_local_topic_fail && !exp_remote_topic_fail && !exp_wr_fail) + sync_writer_to_readers (g_participant[0], lwr, exp_wr_rd_sync_fail ? 0 : 1, sync_abstimeout); if (!exp_local_topic_fail && !exp_remote_topic_fail && !exp_rd_fail) - sync_reader_to_writers (g_participant[0], lrd, exp_rd_wr_sync_fail ? 0 : 1, DDS_SECS(1)); + sync_reader_to_writers (g_participant[0], lrd, exp_rd_wr_sync_fail ? 0 : 1, sync_abstimeout); } access_control_fini (2, (void * []) { gov_topic_rule, gov_config }, 2); @@ -607,7 +613,7 @@ static void test_discovery_liveliness_protection(enum test_discovery_liveliness dds_entity_t pub, sub, pub_tp, sub_tp, wr, rd; rd_wr_init (g_participant[0], &pub, &pub_tp, &wr, g_participant[1], &sub, &sub_tp, &rd, topic_name); - sync_writer_to_readers (g_participant[0], wr, exp_rd_wr_match_fail ? 0 : 1, DDS_SECS(2)); + sync_writer_to_readers (g_participant[0], wr, exp_rd_wr_match_fail ? 0 : 1, dds_time() + DDS_SECS(2)); if (!exp_rd_wr_match_fail) write_read_for (wr, g_participant[1], rd, DDS_MSECS (100), false, false); @@ -697,7 +703,7 @@ static void test_encoding_mismatch( { dds_entity_t pub, sub, pub_tp, sub_tp, wr, rd; rd_wr_init (g_participant[0], &pub, &pub_tp, &wr, g_participant[1], &sub, &sub_tp, &rd, topic_name); - sync_writer_to_readers (g_participant[0], wr, exp_rd_wr_fail ? 0 : 1, DDS_SECS(1)); + sync_writer_to_readers (g_participant[0], wr, exp_rd_wr_fail ? 0 : 1, dds_time() + DDS_SECS(1)); } access_control_fini (2, (void * []) { gov_config1, gov_config2, gov_topic_rule1, gov_topic_rule2, grants[0], grants[1], perm_config, ca, id1_subj, id2_subj, id1, id2 }, 12); @@ -791,7 +797,7 @@ static void test_readwrite_protection ( validate_handshake_nofail (DDS_DOMAINID, DDS_SECS(2)); rd_wr_init_fail (g_participant[0], &pub, &pub_tp, &wr, g_participant[1], &sub, &sub_tp, &rd, topic_name, exp_pub_tp_fail, exp_wr_fail, exp_sub_tp_fail, exp_rd_fail); if (!exp_pub_tp_fail && !exp_wr_fail && !exp_sub_tp_fail && !exp_rd_fail) - sync_writer_to_readers (g_participant[0], wr, exp_sync_fail ? 0 : 1, DDS_SECS(1)); + sync_writer_to_readers (g_participant[0], wr, exp_sync_fail ? 0 : 1, dds_time() + DDS_SECS(1)); } access_control_fini (2, (void * []) { gov_config, gov_topic_rule, rules_xml, grants[0], grants[1], perm_config, ca, id1_subj, id2_subj, id1, id2 }, 11); @@ -863,8 +869,10 @@ CU_Test(ddssec_access_control, denied_topic) dds_entity_t pub, sub, pub_tp, sub_tp, wr, rd; rd_wr_init (g_participant[0], &pub, &pub_tp, &wr, g_participant[1], &sub, &sub_tp, &rd, topic_name); - sync_writer_to_readers (g_participant[0], wr, 1, DDS_SECS (1)); - sync_reader_to_writers (g_participant[1], rd, 1, DDS_SECS (1)); + + const dds_time_t sync_abstimeout = dds_time () + DDS_SECS (2); + sync_writer_to_readers (g_participant[0], wr, 1, sync_abstimeout); + sync_reader_to_writers (g_participant[1], rd, 1, sync_abstimeout); /* Create a topic that is denied in the subscriber pp security config */ dds_entity_t denied_pub_tp = dds_create_topic (g_participant[0], &SecurityCoreTests_Type1_desc, denied_topic_name, NULL, NULL); diff --git a/src/security/core/tests/authentication.c b/src/security/core/tests/authentication.c index a09ad7d..e3dcb70 100644 --- a/src/security/core/tests/authentication.c +++ b/src/security/core/tests/authentication.c @@ -298,7 +298,7 @@ CU_Theory( if (write_read_dur > 0) { rd_wr_init (g_participant1, &g_pub, &g_pub_tp, &g_wr, g_participant2, &g_sub, &g_sub_tp, &g_rd, topic_name); - sync_writer_to_readers(g_participant1, g_wr, 1, DDS_SECS(2)); + sync_writer_to_readers(g_participant1, g_wr, 1, dds_time() + DDS_SECS(2)); write_read_for (g_wr, g_participant2, g_rd, DDS_MSECS (write_read_dur), false, exp_read_fail); } authentication_fini (!id1_local_fail, !id2_local_fail, (void * []){ grants[0], grants[1], perm_config, ca, id1_subj, id2_subj, id1, id2 }, 8); @@ -340,13 +340,13 @@ CU_Test(ddssec_authentication, unauthenticated_pp) print_test_msg ("writing sample for plain topic\n"); dds_entity_t pub, sub, pub_tp, sub_tp, wr, rd; rd_wr_init (g_participant1, &pub, &pub_tp, &wr, g_participant2, &sub, &sub_tp, &rd, topic_name_plain); - sync_writer_to_readers(g_participant1, wr, 1, DDS_SECS(5)); + sync_writer_to_readers(g_participant1, wr, 1, dds_time() + DDS_SECS(5)); write_read_for (wr, g_participant2, rd, DDS_MSECS (10), false, false); print_test_msg ("writing sample for secured topic\n"); dds_entity_t spub, ssub, spub_tp, ssub_tp, swr, srd; rd_wr_init (g_participant1, &spub, &spub_tp, &swr, g_participant2, &ssub, &ssub_tp, &srd, topic_name_secure); - sync_writer_to_readers(g_participant1, swr, 0, DDS_SECS(2)); + sync_writer_to_readers(g_participant1, swr, 0, dds_time() + DDS_SECS(2)); write_read_for (swr, g_participant2, srd, DDS_MSECS (10), false, true); authentication_fini (true, true, (void * []) { gov_config, gov_topic_rules, topic_rule_sec, topic_rule_plain, grants[0], perm_config, ca, id1_subj, id1 }, 9); diff --git a/src/security/core/tests/common/test_utils.c b/src/security/core/tests/common/test_utils.c index 019846e..5d4839f 100644 --- a/src/security/core/tests/common/test_utils.c +++ b/src/security/core/tests/common/test_utils.c @@ -389,9 +389,8 @@ void handshake_list_fini (struct Handshake *hs_list, int nhs) } } -void sync_writer_to_readers (dds_entity_t pp_wr, dds_entity_t wr, uint32_t exp_count, dds_duration_t timeout) +void sync_writer_to_readers (dds_entity_t pp_wr, dds_entity_t wr, uint32_t exp_count, dds_time_t abstimeout) { - dds_time_t abstimeout = dds_time() + timeout; dds_attach_t triggered; dds_entity_t ws = dds_create_waitset (pp_wr); CU_ASSERT_FATAL (ws > 0); @@ -414,9 +413,8 @@ void sync_writer_to_readers (dds_entity_t pp_wr, dds_entity_t wr, uint32_t exp_c CU_ASSERT_EQUAL_FATAL (pub_matched.total_count, exp_count); } -void sync_reader_to_writers (dds_entity_t pp_rd, dds_entity_t rd, uint32_t exp_count, dds_duration_t timeout) +void sync_reader_to_writers (dds_entity_t pp_rd, dds_entity_t rd, uint32_t exp_count, dds_time_t abstimeout) { - dds_time_t abstimeout = dds_time() + timeout; dds_attach_t triggered; dds_entity_t ws = dds_create_waitset (pp_rd); CU_ASSERT_FATAL (ws > 0); diff --git a/src/security/core/tests/common/test_utils.h b/src/security/core/tests/common/test_utils.h index b2dfff5..99cc337 100644 --- a/src/security/core/tests/common/test_utils.h +++ b/src/security/core/tests/common/test_utils.h @@ -68,8 +68,8 @@ void validate_handshake_nofail (dds_domainid_t domain_id, dds_duration_t timeout void validate_handshake_result (struct Handshake *hs, bool exp_fail_hs_req, const char * fail_hs_req_msg, bool exp_fail_hs_reply, const char * fail_hs_reply_msg); void handshake_list_fini (struct Handshake *hs_list, int nhs); char *create_topic_name (const char *prefix, uint32_t nr, char *name, size_t size); -void sync_writer_to_readers (dds_entity_t pp_wr, dds_entity_t wr, uint32_t exp_count, dds_duration_t timeout); -void sync_reader_to_writers (dds_entity_t pp_rd, dds_entity_t rd, uint32_t exp_count, dds_duration_t timeout); +void sync_writer_to_readers (dds_entity_t pp_wr, dds_entity_t wr, uint32_t exp_count, dds_time_t abstimeout); +void sync_reader_to_writers (dds_entity_t pp_rd, dds_entity_t rd, uint32_t exp_count, dds_time_t abstimeout); bool reader_wait_for_data (dds_entity_t pp, dds_entity_t rd, dds_duration_t dur); dds_qos_t * get_default_test_qos (void); void rd_wr_init ( diff --git a/src/security/core/tests/crypto.c b/src/security/core/tests/crypto.c index 68ae5fd..7590740 100644 --- a/src/security/core/tests/crypto.c +++ b/src/security/core/tests/crypto.c @@ -164,7 +164,7 @@ CU_Theory((const char * test_descr, DDS_Security_BasicProtectionKind payload_pk, set_force_plain_data (crypto_impl, wr_handle, rtps_pk != PK_N, submsg_pk != PK_N, payload_pk != BPK_N); /* sync and write/take sample */ - sync_writer_to_readers (g_participant1, wr, 1, DDS_SECS (2)); + sync_writer_to_readers (g_participant1, wr, 1, dds_time() + DDS_SECS (2)); write_read_for (wr, g_participant2, rd, DDS_MSECS (10), false, true); /* reset forced plain data */ diff --git a/src/security/core/tests/secure_communication.c b/src/security/core/tests/secure_communication.c index 2d1720d..8e58aba 100644 --- a/src/security/core/tests/secure_communication.c +++ b/src/security/core/tests/secure_communication.c @@ -246,6 +246,7 @@ static void test_write_read(struct domain_sec_config *domain_config, create_eps (&writers, &writer_topics, n_pub_domains, n_pub_participants, n_writers, name, &SecurityCoreTests_Type1_desc, g_pub_participants, qos, &dds_create_writer, DDS_PUBLICATION_MATCHED_STATUS); create_eps (&readers, &reader_topics, n_sub_domains, n_sub_participants, n_readers, name, &SecurityCoreTests_Type1_desc, g_sub_participants, qos, &dds_create_reader, DDS_DATA_AVAILABLE_STATUS); + const dds_time_t sync_abstimeout = dds_time() + DDS_SECS(5); for (size_t d = 0; d < n_pub_domains; d++) { for (size_t p = 0; p < n_pub_participants; p++) @@ -254,7 +255,7 @@ static void test_write_read(struct domain_sec_config *domain_config, for (size_t w = 0; w < n_writers; w++) { size_t wr_index = pp_index * n_writers + w; - sync_writer_to_readers (g_pub_participants[pp_index], writers[wr_index], (uint32_t)(n_sub_domains * n_sub_participants * n_readers), DDS_SECS(5)); + sync_writer_to_readers (g_pub_participants[pp_index], writers[wr_index], (uint32_t)(n_sub_domains * n_sub_participants * n_readers), sync_abstimeout); sample.id = (int32_t) wr_index; printf("writer %"PRId32" writing sample %d\n", writers[wr_index], sample.id); ret = dds_write (writers[wr_index], &sample); @@ -365,7 +366,7 @@ static void test_payload_secret(DDS_Security_ProtectionKind rtps_pk, DDS_Securit create_eps (&writers, &writer_topics, 1, 1, 1, name, &SecurityCoreTests_Type2_desc, g_pub_participants, qos, &dds_create_writer, DDS_PUBLICATION_MATCHED_STATUS); create_eps (&readers, &reader_topics, 1, 1, 1, name, &SecurityCoreTests_Type2_desc, g_sub_participants, qos, &dds_create_reader, DDS_DATA_AVAILABLE_STATUS); dds_delete_qos (qos); - sync_writer_to_readers (g_pub_participants[0], writers[0], 1, DDS_SECS(2)); + sync_writer_to_readers (g_pub_participants[0], writers[0], 1, dds_time() + DDS_SECS(2)); ret = dds_write (writers[0], &sample); CU_ASSERT_EQUAL_FATAL (ret, DDS_RETCODE_OK);