From be7f7af7416668492bf1cebc2db8f82266737ba7 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Sat, 16 May 2020 08:38:58 +0200 Subject: [PATCH] Tweak timeout handling of authentication tests * Compute the time at which the handshake must have completed from the initial timeout specification, rather than using it as a timeout for the individual steps of the handshake * If the handshake fails because an expected message is not present, print this, including whether the timeout occured because the message queue was empty or because the expected message could not be found in a non-empty queue. * Replace the 0.5s per-step timeout to a single 2s timeout. Signed-off-by: Erik Boasson --- src/security/core/tests/access_control.c | 6 +- .../tests/common/authentication_wrapper.c | 4 +- .../tests/common/authentication_wrapper.h | 2 +- .../core/tests/common/plugin_wrapper_msg_q.c | 27 ++-- .../core/tests/common/plugin_wrapper_msg_q.h | 8 +- src/security/core/tests/common/test_utils.c | 130 +++++++++++++----- 6 files changed, 122 insertions(+), 55 deletions(-) diff --git a/src/security/core/tests/access_control.c b/src/security/core/tests/access_control.c index 261fbdf..f22dbe5 100644 --- a/src/security/core/tests/access_control.c +++ b/src/security/core/tests/access_control.c @@ -700,7 +700,7 @@ static void test_encoding_mismatch( struct Handshake *hs_list; int nhs; - validate_handshake (DDS_DOMAINID, false, NULL, &hs_list, &nhs, DDS_MSECS(500)); + validate_handshake (DDS_DOMAINID, false, NULL, &hs_list, &nhs, DDS_SECS(2)); CU_ASSERT_EQUAL_FATAL (exp_hs_fail, nhs < 1); handshake_list_fini (hs_list, nhs); @@ -799,7 +799,7 @@ static void test_readwrite_protection ( if (!exp_pub_pp_fail && !exp_sub_pp_fail) { dds_entity_t pub, sub, pub_tp, sub_tp, wr, rd; - validate_handshake_nofail (DDS_DOMAINID, DDS_MSECS(500)); + 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)); @@ -893,4 +893,4 @@ CU_Test(ddssec_access_control, denied_topic) dds_delete_qos (qos); access_control_fini (2, (void * []) { gov_config, gov_topic_rule, sub_rules_xml, grants_pub[0], grants_sub[0], perm_config_pub, perm_config_sub, ca, id1_subj, id2_subj, id1, id2 }, 12); -} \ No newline at end of file +} diff --git a/src/security/core/tests/common/authentication_wrapper.c b/src/security/core/tests/common/authentication_wrapper.c index 86290df..693ac11 100644 --- a/src/security/core/tests/common/authentication_wrapper.c +++ b/src/security/core/tests/common/authentication_wrapper.c @@ -446,11 +446,11 @@ static struct dds_security_authentication_impl * get_impl_for_domain(dds_domaini return NULL; } -struct message * test_authentication_plugin_take_msg(dds_domainid_t domain_id, message_kind_t kind, DDS_Security_IdentityHandle lidHandle, DDS_Security_IdentityHandle ridHandle, DDS_Security_IdentityHandle hsHandle, dds_duration_t timeout) +enum take_message_result test_authentication_plugin_take_msg(dds_domainid_t domain_id, message_kind_t kind, DDS_Security_IdentityHandle lidHandle, DDS_Security_IdentityHandle ridHandle, DDS_Security_IdentityHandle hsHandle, dds_time_t abstimeout, struct message **msg) { struct dds_security_authentication_impl *impl = get_impl_for_domain(domain_id); assert(impl); - return take_message(&impl->msg_queue, kind, lidHandle, ridHandle, hsHandle, timeout); + return take_message(&impl->msg_queue, kind, lidHandle, ridHandle, hsHandle, abstimeout, msg); } void test_authentication_plugin_release_msg(struct message *msg) diff --git a/src/security/core/tests/common/authentication_wrapper.h b/src/security/core/tests/common/authentication_wrapper.h index f58121d..939db10 100644 --- a/src/security/core/tests/common/authentication_wrapper.h +++ b/src/security/core/tests/common/authentication_wrapper.h @@ -33,7 +33,7 @@ SECURITY_EXPORT int finalize_test_authentication_missing_func(void *context); SECURITY_EXPORT int init_test_authentication_init_error(const char *argument, void **context, struct ddsi_domaingv *gv); SECURITY_EXPORT int finalize_test_authentication_init_error(void *context); -SECURITY_EXPORT struct message * test_authentication_plugin_take_msg(dds_domainid_t domain_id, message_kind_t kind, DDS_Security_IdentityHandle lidHandle, DDS_Security_IdentityHandle ridHandle, DDS_Security_IdentityHandle hsHandle, dds_duration_t timeout); +SECURITY_EXPORT enum take_message_result test_authentication_plugin_take_msg(dds_domainid_t domain_id, message_kind_t kind, DDS_Security_IdentityHandle lidHandle, DDS_Security_IdentityHandle ridHandle, DDS_Security_IdentityHandle hsHandle, dds_time_t abstimeout, struct message **msg); SECURITY_EXPORT void test_authentication_plugin_release_msg(struct message *msg); #endif /* SECURITY_CORE_TEST_AUTHENTICATION_WRAPPER_H_ */ diff --git a/src/security/core/tests/common/plugin_wrapper_msg_q.c b/src/security/core/tests/common/plugin_wrapper_msg_q.c index 3ecad82..0b03ded 100644 --- a/src/security/core/tests/common/plugin_wrapper_msg_q.c +++ b/src/security/core/tests/common/plugin_wrapper_msg_q.c @@ -95,25 +95,26 @@ int message_matched(struct message *msg, message_kind_t kind, DDS_Security_Ident (!hsHandle || msg->hsHandle == hsHandle); } -struct message * take_message(struct message_queue *queue, message_kind_t kind, DDS_Security_IdentityHandle lidHandle, DDS_Security_IdentityHandle ridHandle, DDS_Security_IdentityHandle hsHandle, dds_duration_t timeout) +enum take_message_result take_message(struct message_queue *queue, message_kind_t kind, DDS_Security_IdentityHandle lidHandle, DDS_Security_IdentityHandle ridHandle, DDS_Security_IdentityHandle hsHandle, dds_time_t abstimeout, struct message **msg) { - struct message *msg = NULL, *cur, *prev; - int r = 1; + struct message *cur, *prev; + enum take_message_result ret = TAKE_MESSAGE_OK; + *msg = NULL; ddsrt_mutex_lock(&queue->lock); do { cur = queue->head; prev = NULL; - while (cur && !msg) + while (cur && *msg == NULL) { if (message_matched(cur, kind, lidHandle, ridHandle, hsHandle)) { - msg = cur; + *msg = cur; if (prev) - prev->next = msg->next; + prev->next = cur->next; else - queue->head = msg->next; - if (queue->tail == msg) + queue->head = cur->next; + if (queue->tail == cur) queue->tail = prev; } else @@ -122,13 +123,13 @@ struct message * take_message(struct message_queue *queue, message_kind_t kind, cur = cur->next; } } - if (!msg) + if (*msg == NULL) { - if (!ddsrt_cond_waitfor(&queue->cond, &queue->lock, timeout)) - r = 0; + if (!ddsrt_cond_waituntil(&queue->cond, &queue->lock, abstimeout)) + ret = queue->head ? TAKE_MESSAGE_TIMEOUT_NONEMPTY : TAKE_MESSAGE_TIMEOUT_EMPTY; } - } while (r && !msg); + } while (ret == TAKE_MESSAGE_OK && *msg == NULL); ddsrt_mutex_unlock(&queue->lock); - return msg; + return ret; } diff --git a/src/security/core/tests/common/plugin_wrapper_msg_q.h b/src/security/core/tests/common/plugin_wrapper_msg_q.h index 32ec557..c70e5f2 100644 --- a/src/security/core/tests/common/plugin_wrapper_msg_q.h +++ b/src/security/core/tests/common/plugin_wrapper_msg_q.h @@ -46,6 +46,12 @@ struct message_queue { struct message *tail; }; +enum take_message_result { + TAKE_MESSAGE_OK, /* message found */ + TAKE_MESSAGE_TIMEOUT_EMPTY, /* no message found, queue is empty */ + TAKE_MESSAGE_TIMEOUT_NONEMPTY /* no message found, queue is not empty */ +}; + struct dds_security_authentication_impl; void insert_message(struct message_queue *queue, struct message *msg); @@ -56,7 +62,7 @@ void delete_message(struct message *msg); void init_message_queue(struct message_queue *queue); void deinit_message_queue(struct message_queue *queue); int message_matched(struct message *msg, message_kind_t kind, DDS_Security_IdentityHandle lidHandle, DDS_Security_IdentityHandle ridHandle, DDS_Security_IdentityHandle hsHandle); -struct message * take_message(struct message_queue *queue, message_kind_t kind, DDS_Security_IdentityHandle lidHandle, DDS_Security_IdentityHandle ridHandle, DDS_Security_IdentityHandle hsHandle, dds_duration_t timeout); +enum take_message_result take_message(struct message_queue *queue, message_kind_t kind, DDS_Security_IdentityHandle lidHandle, DDS_Security_IdentityHandle ridHandle, DDS_Security_IdentityHandle hsHandle, dds_time_t abstimeout, struct message **msg); #endif /* SECURITY_CORE_PLUGIN_WRAPPER_MSG_Q_H_ */ diff --git a/src/security/core/tests/common/test_utils.c b/src/security/core/tests/common/test_utils.c index e7e7ed4..cab9bef 100644 --- a/src/security/core/tests/common/test_utils.c +++ b/src/security/core/tests/common/test_utils.c @@ -149,58 +149,92 @@ static int find_handshake (DDS_Security_HandshakeHandle handle) return -1; } -static void handle_process_message (dds_domainid_t domain_id, DDS_Security_IdentityHandle handshake, dds_duration_t timeout) +static void handle_process_message (dds_domainid_t domain_id, DDS_Security_IdentityHandle handshake, dds_time_t abstimeout) { struct message *msg; - if ((msg = test_authentication_plugin_take_msg (domain_id, MESSAGE_KIND_PROCESS_HANDSHAKE, 0, 0, handshake, timeout))) + switch (test_authentication_plugin_take_msg (domain_id, MESSAGE_KIND_PROCESS_HANDSHAKE, 0, 0, handshake, abstimeout, &msg)) { - int idx; - if ((idx = find_handshake (msg->hsHandle)) >= 0) - { - print_test_msg ("set handshake %"PRId64" final result to '%s' (errmsg: %s)\n", msg->hsHandle, get_validation_result_str (msg->result), msg->err_msg); - handshakeList[idx].finalResult = msg->result; - handshakeList[idx].err_msg = ddsrt_strdup (msg->err_msg); + case TAKE_MESSAGE_OK: { + int idx; + if ((idx = find_handshake (msg->hsHandle)) >= 0) + { + print_test_msg ("set handshake %"PRId64" final result to '%s' (errmsg: %s)\n", msg->hsHandle, get_validation_result_str (msg->result), msg->err_msg); + handshakeList[idx].finalResult = msg->result; + handshakeList[idx].err_msg = ddsrt_strdup (msg->err_msg); + } + test_authentication_plugin_release_msg (msg); + break; + } + case TAKE_MESSAGE_TIMEOUT_EMPTY: { + print_test_msg ("handle_process_message: timed out on empty queue\n"); + break; + } + case TAKE_MESSAGE_TIMEOUT_NONEMPTY: { + print_test_msg ("handle_process_message: timed out on non-empty queue\n"); + break; } - test_authentication_plugin_release_msg (msg); } } -static void handle_begin_handshake_request (dds_domainid_t domain_id, struct Handshake *hs, DDS_Security_IdentityHandle lid, DDS_Security_IdentityHandle rid, dds_duration_t timeout) +static void handle_begin_handshake_request (dds_domainid_t domain_id, struct Handshake *hs, DDS_Security_IdentityHandle lid, DDS_Security_IdentityHandle rid, dds_time_t abstimeout) { struct message *msg; print_test_msg ("handle begin handshake request %"PRId64"<->%"PRId64"\n", lid, rid); - if ((msg = test_authentication_plugin_take_msg (domain_id, MESSAGE_KIND_BEGIN_HANDSHAKE_REQUEST, lid, rid, 0, timeout))) + switch (test_authentication_plugin_take_msg (domain_id, MESSAGE_KIND_BEGIN_HANDSHAKE_REQUEST, lid, rid, 0, abstimeout, &msg)) { - hs->handle = msg->hsHandle; - hs->handshakeResult = msg->result; - if (msg->result != DDS_SECURITY_VALIDATION_FAILED) - handle_process_message (domain_id, msg->hsHandle, timeout); - else - hs->err_msg = ddsrt_strdup (msg->err_msg); - test_authentication_plugin_release_msg (msg); + case TAKE_MESSAGE_OK: { + hs->handle = msg->hsHandle; + hs->handshakeResult = msg->result; + if (msg->result != DDS_SECURITY_VALIDATION_FAILED) + handle_process_message (domain_id, msg->hsHandle, abstimeout); + else + hs->err_msg = ddsrt_strdup (msg->err_msg); + test_authentication_plugin_release_msg (msg); + break; + } + case TAKE_MESSAGE_TIMEOUT_EMPTY: { + print_test_msg ("handle_begin_handshake_request: timed out on empty queue\n"); + break; + } + case TAKE_MESSAGE_TIMEOUT_NONEMPTY: { + print_test_msg ("handle_begin_handshake_request: timed out on non-empty queue\n"); + break; + } } } -static void handle_begin_handshake_reply (dds_domainid_t domain_id, struct Handshake *hs, DDS_Security_IdentityHandle lid, DDS_Security_IdentityHandle rid, dds_duration_t timeout) +static void handle_begin_handshake_reply (dds_domainid_t domain_id, struct Handshake *hs, DDS_Security_IdentityHandle lid, DDS_Security_IdentityHandle rid, dds_time_t abstimeout) { struct message *msg; print_test_msg ("handle begin handshake reply %"PRId64"<->%"PRId64"\n", lid, rid); - if ((msg = test_authentication_plugin_take_msg (domain_id, MESSAGE_KIND_BEGIN_HANDSHAKE_REPLY, lid, rid, 0, timeout))) + switch (test_authentication_plugin_take_msg (domain_id, MESSAGE_KIND_BEGIN_HANDSHAKE_REPLY, lid, rid, 0, abstimeout, &msg)) { - hs->handle = msg->hsHandle; - hs->handshakeResult = msg->result; - if (msg->result != DDS_SECURITY_VALIDATION_FAILED) - handle_process_message (domain_id, msg->hsHandle, timeout); - else - hs->err_msg = ddsrt_strdup (msg->err_msg); - test_authentication_plugin_release_msg (msg); + case TAKE_MESSAGE_OK: { + hs->handle = msg->hsHandle; + hs->handshakeResult = msg->result; + if (msg->result != DDS_SECURITY_VALIDATION_FAILED) + handle_process_message (domain_id, msg->hsHandle, abstimeout); + else + hs->err_msg = ddsrt_strdup (msg->err_msg); + test_authentication_plugin_release_msg (msg); + break; + } + case TAKE_MESSAGE_TIMEOUT_EMPTY: { + print_test_msg ("handle_begin_handshake_reply: timed out on empty queue\n"); + break; + } + case TAKE_MESSAGE_TIMEOUT_NONEMPTY: { + print_test_msg ("handle_begin_handshake_reply: timed out on non-empty queue\n"); + break; + } } } -static void handle_validate_remote_identity (dds_domainid_t domain_id, DDS_Security_IdentityHandle lid, int count, dds_duration_t timeout) +static void handle_validate_remote_identity (dds_domainid_t domain_id, DDS_Security_IdentityHandle lid, int count, dds_time_t abstimeout) { + enum take_message_result res = TAKE_MESSAGE_OK; struct message *msg; - while (count-- > 0 && (msg = test_authentication_plugin_take_msg (domain_id, MESSAGE_KIND_VALIDATE_REMOTE_IDENTITY, lid, 0, 0, timeout))) + while (count-- > 0 && (res = test_authentication_plugin_take_msg (domain_id, MESSAGE_KIND_VALIDATE_REMOTE_IDENTITY, lid, 0, 0, abstimeout, &msg)) == TAKE_MESSAGE_OK) { struct Handshake *hs; add_remote_identity (msg->ridHandle, &msg->rguid); @@ -208,12 +242,12 @@ static void handle_validate_remote_identity (dds_domainid_t domain_id, DDS_Secur if (msg->result == DDS_SECURITY_VALIDATION_PENDING_HANDSHAKE_REQUEST) { hs->node_type = HSN_REQUESTER; - handle_begin_handshake_request (domain_id, hs, lid, msg->ridHandle, timeout); + handle_begin_handshake_request (domain_id, hs, lid, msg->ridHandle, abstimeout); } else if (msg->result == DDS_SECURITY_VALIDATION_PENDING_HANDSHAKE_MESSAGE) { hs->node_type = HSN_REPLIER; - handle_begin_handshake_reply (domain_id, hs, lid, msg->ridHandle, timeout); + handle_begin_handshake_reply (domain_id, hs, lid, msg->ridHandle, abstimeout); } else { @@ -221,11 +255,34 @@ static void handle_validate_remote_identity (dds_domainid_t domain_id, DDS_Secur } test_authentication_plugin_release_msg (msg); } + + switch (res) + { + case TAKE_MESSAGE_OK: + break; + case TAKE_MESSAGE_TIMEOUT_EMPTY: + print_test_msg ("handle_validate_remote_identity: timed out on empty queue\n"); + break; + case TAKE_MESSAGE_TIMEOUT_NONEMPTY: + print_test_msg ("handle_validate_remote_identity: timed out on non-empty queue\n"); + break; + } } -static void handle_validate_local_identity (dds_domainid_t domain_id, bool exp_localid_fail, const char * exp_localid_msg, dds_duration_t timeout) +static void handle_validate_local_identity (dds_domainid_t domain_id, bool exp_localid_fail, const char * exp_localid_msg, dds_time_t abstimeout) { - struct message *msg = test_authentication_plugin_take_msg (domain_id, MESSAGE_KIND_VALIDATE_LOCAL_IDENTITY, 0, 0, 0, timeout); + struct message *msg; + switch (test_authentication_plugin_take_msg (domain_id, MESSAGE_KIND_VALIDATE_LOCAL_IDENTITY, 0, 0, 0, abstimeout, &msg)) + { + case TAKE_MESSAGE_OK: + break; + case TAKE_MESSAGE_TIMEOUT_EMPTY: + print_test_msg ("handle_validate_local_identity: timed out on empty queue\n"); + break; + case TAKE_MESSAGE_TIMEOUT_NONEMPTY: + print_test_msg ("handle_validate_local_identity: timed out on non-empty queue\n"); + break; + } CU_ASSERT_FATAL (msg != NULL); CU_ASSERT_FATAL ((msg->result == DDS_SECURITY_VALIDATION_OK) != exp_localid_fail); if (exp_localid_fail && exp_localid_msg) @@ -234,12 +291,15 @@ static void handle_validate_local_identity (dds_domainid_t domain_id, bool exp_l CU_ASSERT_FATAL (msg->err_msg && strstr (msg->err_msg, exp_localid_msg) != NULL); } else + { add_local_identity (msg->lidHandle, &msg->lguid); + } test_authentication_plugin_release_msg (msg); } void validate_handshake (dds_domainid_t domain_id, bool exp_localid_fail, const char * exp_localid_msg, struct Handshake *hs_list[], int *nhs, dds_duration_t timeout) { + dds_time_t abstimeout = dds_time() + timeout; clear_stores (); if (nhs) @@ -247,10 +307,10 @@ void validate_handshake (dds_domainid_t domain_id, bool exp_localid_fail, const if (hs_list) *hs_list = NULL; - handle_validate_local_identity (domain_id, exp_localid_fail, exp_localid_msg, timeout); + handle_validate_local_identity (domain_id, exp_localid_fail, exp_localid_msg, abstimeout); if (!exp_localid_fail) { - handle_validate_remote_identity (domain_id, localIdentityList[0].handle, 1, timeout); + handle_validate_remote_identity (domain_id, localIdentityList[0].handle, 1, abstimeout); for (int n = 0; n < numHandshake; n++) { struct Handshake *hs = &handshakeList[n];