diff --git a/src/session.c b/src/session.c index 4d779c81..d5954fc9 100644 --- a/src/session.c +++ b/src/session.c @@ -1171,17 +1171,24 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *)) session->status = NC_STATUS_CLOSING; if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CH_THREAD)) { + /* signaling a condition does not require its mutex to be held */ pthread_cond_signal(&session->opts.server.ch_cond); - nc_timeouttime_get(&ts, NC_SESSION_FREE_LOCK_TIMEOUT); + if (ch_locked) { + nc_timeouttime_get(&ts, NC_SESSION_FREE_LOCK_TIMEOUT); - /* wait for CH thread to actually wake up and terminate */ - r = 0; - while (!r && (session->flags & NC_SESSION_CH_THREAD)) { - r = pthread_cond_clockwait(&session->opts.server.ch_cond, &session->opts.server.ch_lock, COMPAT_CLOCK_ID, &ts); - } - if (r) { - ERR(session, "Waiting for Call Home thread failed (%s).", strerror(r)); + /* wait for CH thread to actually wake up and terminate */ + r = 0; + while (!r && (session->flags & NC_SESSION_CH_THREAD)) { + r = pthread_cond_clockwait(&session->opts.server.ch_cond, &session->opts.server.ch_lock, COMPAT_CLOCK_ID, &ts); + } + if (r) { + ERR(session, "Waiting for Call Home thread failed (%s).", strerror(r)); + } + } else { + /* waiting on a condition requires its mutex to be held by the caller, so there is no + * way to wait for the Call Home thread without ch_lock */ + ERR(session, "Freeing a Call Home session without its lock, not waiting for its thread."); } } @@ -2108,6 +2115,12 @@ nc_session_curl_init(CURL **handle, struct nc_curl_data *data) return 1; } + /* limit the whole transfer, a host that connects and then stalls would block the TLS handshake */ + if (curl_easy_setopt(*handle, CURLOPT_TIMEOUT_MS, NC_CURL_TIMEOUT_MS)) { + ERR(NULL, "Setting curl transfer timeout failed."); + return 1; + } + /* do not use signals for timeouts, required for thread safety */ if (curl_easy_setopt(*handle, CURLOPT_NOSIGNAL, 1L)) { ERR(NULL, "Setting CURLOPT_NOSIGNAL failed."); diff --git a/src/session_client.c b/src/session_client.c index 6af5d98b..a947d569 100644 --- a/src/session_client.c +++ b/src/session_client.c @@ -1847,7 +1847,7 @@ nc_client_ch_del_bind(const char *address, uint16_t port, NC_TRANSPORT_IMPL ti) API int nc_accept_callhome(int timeout, struct ly_ctx *ctx, struct nc_session **session) { - int ret, sock; + int ret, sock = -1; char *host = NULL; uint16_t port, bind_idx = 0; diff --git a/src/session_p.h b/src/session_p.h index 5e1c763f..6e0de203 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -73,6 +73,11 @@ extern struct nc_server_opts server_opts; */ #define NC_TRANSPORT_MSG_TIMEOUT 2000 +/** + * Maximum time in msec a transport handshake may block before it checks whether it was interrupted. + */ +#define NC_HANDSHAKE_INTERRUPT_STEP 100 + /** * Timeout in msec for acquiring a lock of a session (used with a condition, so higher numbers could be required * only in case of extreme concurrency). @@ -129,6 +134,16 @@ extern struct nc_server_opts server_opts; */ #define NC_CURL_CONNECT_TIMEOUT_MS 2000 +/** + * @brief Timeout in msec for a whole CRL download. + * + * The connection timeout alone does not bound a CRL distribution point that accepts the connection + * and then sends data very slowly or not at all. Since the download happens in the middle of a TLS + * handshake, and a Call Home handshake blocks a configuration apply, the whole transfer has to be + * bounded as well. + */ +#define NC_CURL_TIMEOUT_MS 10000 + /** * @brief Timeout in msec for acquiring the hello_lock * (iterating through all YANG modules + building capability strings) @@ -1120,6 +1135,16 @@ struct nc_session { const struct nc_server_config *config; #ifdef NC_ENABLED_SSH_TLS + /** + * @brief Running flag of the Call Home thread performing the transport handshake. + * + * A borrowed pointer to ::nc_server_ch_thread_arg.thread_running, NOT owned by the + * session - the handshake is aborted as soon as it becomes 0. NULL for a handshake that + * cannot be interrupted, which is every handshake done by ::nc_accept(). Set and cleared + * by ::nc_connect_ch_endpt() exactly like ::nc_session.opts.server.config. + */ + ATOMIC_T *ch_thread_running; + uint16_t ssh_auth_attempts; /**< number of failed SSH authentication attempts */ void *client_cert; /**< TLS client certificate if used for authentication */ #endif /* NC_ENABLED_SSH_TLS */ @@ -1655,13 +1680,36 @@ int _nc_connect_ch_client_dispatch(const char *client_name, nc_server_ch_session */ struct nc_session *nc_accept_callhome_ssh_sock(int sock, const char *host, uint16_t port, struct ly_ctx *ctx); +/** + * @brief Check whether the transport handshake of a session should be aborted. + * + * Only a handshake performed by a Call Home thread can be interrupted, see + * ::nc_session.opts.server.ch_thread_running. Handshakes of accepted sessions are never interrupted. + * + * @param[in] session Session performing a transport handshake. + * @return 1 if the handshake should be aborted, 0 otherwise. + */ +int nc_session_handshake_interrupted(const struct nc_session *session); + +/** + * @brief Cap a transport handshake poll timeout so that an interrupt is noticed in time. + * + * A handshake that cannot be interrupted gets @p timeout unchanged, there is nothing it could + * notice by waking up before the data it is waiting for arrive. + * + * @param[in] session Session performing a transport handshake. + * @param[in] timeout Timeout in msec the handshake would like to wait for, negative means indefinitely. + * @return Timeout in msec to actually use, never negative for an interruptible handshake. + */ +int32_t nc_session_handshake_poll_timeout(const struct nc_session *session, int32_t timeout); + /** * @brief Establish SSH transport on a socket. * * @param[in] session Session structure of the new connection. * @param[in] opts SSH server options to use. * @param[in] sock Socket of the new connection, closed if not set to the session. - * @return 1 on success, 0 on timeout, -1 on error. + * @return 1 on success, 0 on timeout or interrupt, -1 on error. */ int nc_accept_ssh_session(struct nc_session *session, struct nc_server_ssh_opts *opts, int sock); @@ -1677,7 +1725,7 @@ struct nc_session *nc_accept_callhome_tls_sock(int sock, const char *host, uint1 * @param[in] session Session structure of the new connection. * @param[in] sock Socket of the new connection. * @param[in] timeout Transport operations timeout in msec. - * @return 1 on success, 0 on timeout, -1 on error. + * @return 1 on success, 0 on timeout or interrupt, -1 on error. */ int nc_accept_tls_session(struct nc_session *session, struct nc_server_tls_opts *opts, int sock); diff --git a/src/session_server.c b/src/session_server.c index ba94b0e1..568691e2 100644 --- a/src/session_server.c +++ b/src/session_server.c @@ -1087,6 +1087,7 @@ nc_sock_accept_pollfds(struct pollfd *pollfds, uint16_t pollfd_count, const char /* make the socket non-blocking */ if (((flags = fcntl(client_sock, F_GETFL)) == -1) || (fcntl(client_sock, F_SETFL, flags | O_NONBLOCK) == -1)) { ERR(NULL, "Fcntl failed (%s).", strerror(errno)); + ret = -1; goto cleanup; } @@ -3744,6 +3745,35 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) #ifdef NC_ENABLED_SSH_TLS +int +nc_session_handshake_interrupted(const struct nc_session *session) +{ + ATOMIC_T *ch_thread_running = session->opts.server.ch_thread_running; + + if (!ch_thread_running) { + /* not a Call Home handshake, there is nobody to interrupt it */ + return 0; + } + + return !ATOMIC_LOAD_RELAXED(*ch_thread_running); +} + +int32_t +nc_session_handshake_poll_timeout(const struct nc_session *session, int32_t timeout) +{ + if (!session->opts.server.ch_thread_running) { + /* nothing can interrupt the handshake, there is no reason to wake up early */ + return timeout; + } + + /* a negative timeout means waiting indefinitely, which must not happen if we have to notice an interrupt */ + if ((timeout < 0) || (timeout > NC_HANDSHAKE_INTERRUPT_STEP)) { + return NC_HANDSHAKE_INTERRUPT_STEP; + } + + return timeout; +} + API int nc_server_ch_is_client(const char *name) { @@ -3808,6 +3838,8 @@ nc_server_ch_client_is_endpt(const char *client_name, const char *endpt_name) * @param[in] config Pinned server configuration @p endpt belongs to, pinned into the created session * for the duration of the transport handshake. * @param[in] endpt Endpoint to use. + * @param[in] ch_thread_running Running flag of the calling Call Home thread, the transport handshake + * is aborted as soon as it becomes 0. * @param[in,out] cur_sock_pending Current pending socket for the connection. * @param[in] acquire_ctx_cb Callback for acquiring the libyang context. * @param[in] release_ctx_cb Callback for releasing the libyang context. @@ -3816,9 +3848,9 @@ nc_server_ch_client_is_endpt(const char *client_name, const char *endpt_name) * @return NC_MSG values. */ static NC_MSG_TYPE -nc_connect_ch_endpt(const struct nc_server_config *config, const struct nc_ch_endpt *endpt, int *cur_sock_pending, - nc_server_ch_session_acquire_ctx_cb acquire_ctx_cb, nc_server_ch_session_release_ctx_cb release_ctx_cb, - void *ctx_cb_data, struct nc_session **session) +nc_connect_ch_endpt(const struct nc_server_config *config, const struct nc_ch_endpt *endpt, + ATOMIC_T *ch_thread_running, int *cur_sock_pending, nc_server_ch_session_acquire_ctx_cb acquire_ctx_cb, + nc_server_ch_session_release_ctx_cb release_ctx_cb, void *ctx_cb_data, struct nc_session **session) { NC_MSG_TYPE msgtype; const struct ly_ctx *ctx = NULL; @@ -3856,6 +3888,9 @@ nc_connect_ch_endpt(const struct nc_server_config *config, const struct nc_ch_en /* pin the configuration for the duration of the transport handshake, it is a borrowed pointer */ (*session)->opts.server.config = config; + /* let the handshake be aborted as soon as this thread is told to stop, also a borrowed pointer */ + (*session)->opts.server.ch_thread_running = ch_thread_running; + /* sock gets assigned to session or closed */ if (endpt->ti == NC_TI_SSH) { ret = nc_accept_ssh_session(*session, endpt->opts.ssh, sock); @@ -3887,8 +3922,10 @@ nc_connect_ch_endpt(const struct nc_server_config *config, const struct nc_ch_en goto fail; } - /* the transport handshake is over, the configuration must not be reached through the session anymore */ + /* the transport handshake is over, neither the configuration nor the running flag must be + * reached through the session anymore */ (*session)->opts.server.config = NULL; + (*session)->opts.server.ch_thread_running = NULL; /* assign new SID atomically */ (*session)->id = ATOMIC_INC_RELAXED(server_opts.new_session_id); @@ -3910,6 +3947,7 @@ nc_connect_ch_endpt(const struct nc_server_config *config, const struct nc_ch_en fail: if (*session) { (*session)->opts.server.config = NULL; + (*session)->opts.server.ch_thread_running = NULL; } nc_session_free(*session, NULL); *session = NULL; @@ -3954,6 +3992,10 @@ nc_server_ch_client_get_idle_timeout(const char *client_name, uint32_t *idle_tim /** * @brief Wait for any event after a NC session was established on a CH client. * + * The session is given to the user by ::nc_server_ch_thread_arg.new_session_cb. Until that + * succeeds the session still belongs to the Call Home thread, so it is freed here on any error. + * Afterwards it belongs to the user and is never freed here. + * * @param[in] data CH client thread argument. * @param[in] session New NC session. The session is invalid upon being freed (= function exit). * @return 0 if session was terminated normally, @@ -3969,6 +4011,9 @@ nc_server_ch_client_thread_session_cond_wait(struct nc_server_ch_thread_arg *dat /* CH LOCK */ if (nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__) != 1) { + /* the session has not been given to the user yet, so it is still ours to free */ + nc_session_free(session, NULL); + data->release_ctx_cb(data->ctx_cb_data); return -1; } @@ -4245,8 +4290,8 @@ nc_ch_client_thread(void *arg) } /* try to connect to the endpoint, the configuration stays pinned for the whole handshake */ - msgtype = nc_connect_ch_endpt(config, cur_endpt, &cur_sock_pending, data->acquire_ctx_cb, - data->release_ctx_cb, data->ctx_cb_data, &session); + msgtype = nc_connect_ch_endpt(config, cur_endpt, &data->thread_running, &cur_sock_pending, + data->acquire_ctx_cb, data->release_ctx_cb, data->ctx_cb_data, &session); if (msgtype == NC_MSG_HELLO) { /* session established, the configuration is not needed anymore */ nc_server_config_release(config); @@ -4255,7 +4300,11 @@ nc_ch_client_thread(void *arg) cur_endpt = NULL; if (!ATOMIC_LOAD_RELAXED(data->thread_running)) { - /* thread should stop running */ + /* thread should stop running, the session has not been given to the user yet, + * so it is still ours to free */ + nc_session_free(session, NULL); + session = NULL; + data->release_ctx_cb(data->ctx_cb_data); goto cleanup; } @@ -4331,6 +4380,12 @@ nc_ch_client_thread(void *arg) } cur_attempts = 0; } else { + if (!ATOMIC_LOAD_RELAXED(data->thread_running)) { + /* the handshake was interrupted because this thread should stop, do not count it as + * a failed attempt and do not bother the user with it */ + goto cleanup; + } + /* session was not created, wait a little bit and try again */ ++cur_attempts; diff --git a/src/session_server_ssh.c b/src/session_server_ssh.c index ef127d5a..1ef46b0d 100644 --- a/src/session_server_ssh.c +++ b/src/session_server_ssh.c @@ -1625,6 +1625,11 @@ nc_accept_ssh_session_open_netconf_channel(struct nc_session *session, struct nc return -1; } + if (nc_session_handshake_interrupted(session)) { + VRB(session, "Waiting for the \"netconf\" SSH subsystem interrupted, the Call Home thread is terminating."); + return 0; + } + time_diff = nc_timeouttime_cur_diff(&ts_timeout); if (time_diff < 1) { /* timeout */ @@ -1633,14 +1638,13 @@ nc_accept_ssh_session_open_netconf_channel(struct nc_session *session, struct nc } /* This functions listens to the network and automatically calls callback funcitons. */ - ret = ssh_event_dopoll(session->ti.libssh.event, time_diff); + ret = ssh_event_dopoll(session->ti.libssh.event, nc_session_handshake_poll_timeout(session, time_diff)); if (ret == SSH_ERROR) { ERR(session, "Failed to poll SSH event (%s).", ssh_get_error(session->ti.libssh.session)); return -1; - } else if (ret == SSH_AGAIN) { - /* Timeout reached */ - break; } + /* SSH_AGAIN only means the poll timeout elapsed, which may have been shortened to notice an + * interrupt, so ts_timeout checked at the top of the loop is the only authority */ } if (session->flags & NC_SESSION_SSH_SUBSYS_NETCONF) { @@ -1666,6 +1670,11 @@ nc_accept_ssh_session_open_netconf_channel(struct nc_session *session, struct nc return 1; } + if (nc_session_handshake_interrupted(session)) { + VRB(session, "Waiting for the \"netconf\" SSH subsystem interrupted, the Call Home thread is terminating."); + return 0; + } + usleep(NC_TIMEOUT_STEP); if (nc_timeouttime_cur_diff(&ts_timeout) < 1) { /* timeout */ @@ -1761,6 +1770,11 @@ nc_accept_ssh_session_auth(struct nc_session *session, struct nc_server_ssh_opts return -1; } + if (nc_session_handshake_interrupted(session)) { + VRB(session, "SSH authentication interrupted, the Call Home thread is terminating."); + return 0; + } + if (opts->auth_timeout) { time_diff = nc_timeouttime_cur_diff(&ts_timeout); if (time_diff < 1) { @@ -1773,14 +1787,13 @@ nc_accept_ssh_session_auth(struct nc_session *session, struct nc_server_ssh_opts } /* This functions listens to the network and automatically calls callback funcitons. */ - ret = ssh_event_dopoll(event, time_diff); + ret = ssh_event_dopoll(event, nc_session_handshake_poll_timeout(session, time_diff)); if (ret == SSH_ERROR) { ERR(session, "Failed to poll SSH event (%s).", ssh_get_error(session->ti.libssh.session)); return -1; - } else if (ret == SSH_AGAIN) { - /* Timeout reached */ - break; } + /* SSH_AGAIN only means the poll timeout elapsed, which may have been shortened to notice an + * interrupt, so ts_timeout checked at the top of the loop is the only authority */ } #else while (1) { @@ -1801,6 +1814,11 @@ nc_accept_ssh_session_auth(struct nc_session *session, struct nc_server_ssh_opts break; } + if (nc_session_handshake_interrupted(session)) { + VRB(session, "SSH authentication interrupted, the Call Home thread is terminating."); + return 0; + } + usleep(NC_TIMEOUT_STEP); if (opts->auth_timeout && (nc_timeouttime_cur_diff(&ts_timeout) < 1)) { /* timeout */ @@ -1964,6 +1982,12 @@ nc_accept_ssh_session(struct nc_session *session, struct nc_server_ssh_opts *opt DBG(session, "Performing SSH key exchange..."); nc_timeouttime_get(&ts_timeout, NC_TRANSPORT_HANDSHAKE_TIMEOUT); while ((r = ssh_handle_key_exchange(session->ti.libssh.session)) == SSH_AGAIN) { + if (nc_session_handshake_interrupted(session)) { + VRB(session, "SSH key exchange interrupted, the Call Home thread is terminating."); + rc = 0; + goto cleanup; + } + /* this tends to take longer */ usleep(NC_TIMEOUT_STEP * 20); if (nc_timeouttime_cur_diff(&ts_timeout) < 1) { diff --git a/src/session_server_tls.c b/src/session_server_tls.c index d3a7226f..77cfac26 100644 --- a/src/session_server_tls.c +++ b/src/session_server_tls.c @@ -997,6 +997,12 @@ nc_accept_tls_session(struct nc_session *session, struct nc_server_tls_opts *opt /* do the handshake */ nc_timeouttime_get(&ts_timeout, NC_TRANSPORT_HANDSHAKE_TIMEOUT); while ((rc = nc_server_tls_handshake_step_wrap(session->ti.tls.session)) == 0) { + if (nc_session_handshake_interrupted(session)) { + VRB(session, "TLS handshake interrupted, the Call Home thread is terminating."); + timeouted = 1; + goto fail; + } + usleep(NC_TIMEOUT_STEP); if (nc_timeouttime_cur_diff(&ts_timeout) < 1) { ERR(session, "TLS accept timeout."); diff --git a/tests/test_ch.c b/tests/test_ch.c index 6be19740..95053200 100644 --- a/tests/test_ch.c +++ b/tests/test_ch.c @@ -15,13 +15,18 @@ #define _GNU_SOURCE +#include #include +#include #include #include #include #include #include #include +#include +#include +#include #include @@ -758,6 +763,214 @@ test_nc_ch_two_simultaneous(void **state) } } +/* + * Test: a stalled transport handshake must not block a configuration apply + * + * The Call Home client connects to a plain TCP listener that accepts the connection and then stays + * silent, so the thread ends up stuck in the transport handshake. Deleting the client from the + * configuration has to interrupt the handshake, otherwise the apply waits for the whole + * NC_TRANSPORT_HANDSHAKE_TIMEOUT (10 seconds by default) before the thread can be joined. + */ + +/* maximum time in msec the apply may take, an order of magnitude below the handshake timeout */ +#define TEST_CH_INTERRUPT_LIMIT 3000 + +static int interrupt_listen_sock = -1; +static char interrupt_port_str[16]; + +/** + * @brief Start listening on a loopback port without ever speaking any protocol on it. + * + * @return Listening socket. + */ +static int +test_ch_silent_listen(void) +{ + int sock, opt = 1; + struct sockaddr_in saddr = {0}; + socklen_t saddr_len = sizeof saddr; + + sock = socket(AF_INET, SOCK_STREAM, 0); + assert_int_not_equal(sock, -1); + assert_int_equal(setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof opt), 0); + + saddr.sin_family = AF_INET; + saddr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + /* an ephemeral port, so that this never collides with the ports assigned by CTest */ + saddr.sin_port = 0; + assert_int_equal(bind(sock, (struct sockaddr *)&saddr, sizeof saddr), 0); + assert_int_equal(listen(sock, 1), 0); + + assert_int_equal(getsockname(sock, (struct sockaddr *)&saddr, &saddr_len), 0); + sprintf(interrupt_port_str, "%" PRIu16, ntohs(saddr.sin_port)); + + return sock; +} + +/** + * @brief Let the Call Home client connect, then delete it and measure how long the apply took. + * + * @param[in] state Test state. + */ +static void +test_ch_interrupt_apply(void **state) +{ + int ret, sock; + int64_t elapsed_ms; + struct timespec ts_start, ts_end; + struct nc_pollsession *ps; + struct ln2_test_ctx *test_ctx = *state; + struct test_ch_data *test_data = test_ctx->test_data; + + assert_non_null(state); + + ps = nc_ps_new(); + assert_non_null(ps); + + /* start the Call Home thread, it connects to the silent listener */ + ret = nc_connect_ch_client_dispatch("ch_interrupt", ch_session_acquire_ctx_cb, + ch_session_release_ctx_cb, test_ctx, ch_new_session_cb, ps); + assert_int_equal(ret, 0); + + /* accept the connection but do not say anything, the thread is now in the transport handshake */ + sock = accept(interrupt_listen_sock, NULL, NULL); + assert_int_not_equal(sock, -1); + + /* make sure the thread really got into the handshake loop */ + usleep(100000); + + /* delete the client, this joins its thread */ + clock_gettime(CLOCK_MONOTONIC, &ts_start); + ret = nc_server_config_setup_data(test_data->tree); + assert_int_equal(ret, 0); + clock_gettime(CLOCK_MONOTONIC, &ts_end); + + elapsed_ms = ((int64_t)ts_end.tv_sec - ts_start.tv_sec) * 1000 + + ((int64_t)ts_end.tv_nsec - ts_start.tv_nsec) / 1000000; + printf("apply with a stalled handshake took %" PRId64 " ms\n", elapsed_ms); + assert_true(elapsed_ms < TEST_CH_INTERRUPT_LIMIT); + + close(sock); + close(interrupt_listen_sock); + interrupt_listen_sock = -1; + + nc_ps_clear(ps, 1, NULL); + nc_ps_free(ps); +} + +static int +setup_interrupt_ssh(void **state) +{ + int ret; + struct lyd_node *tree = NULL; + struct ln2_test_ctx *test_ctx; + struct test_ch_data *test_data; + + ret = ln2_glob_test_setup(&test_ctx); + assert_int_equal(ret, 0); + + test_data = calloc(1, sizeof *test_data); + assert_non_null(test_data); + + test_ctx->test_data = test_data; + test_ctx->free_test_data = test_nc_ch_free_test_data; + *state = test_ctx; + + interrupt_listen_sock = test_ch_silent_listen(); + + ret = nc_server_config_add_ch_address_port(test_ctx->ctx, "ch_interrupt", "endpt", NC_TI_SSH, + "127.0.0.1", interrupt_port_str, &tree); + assert_int_equal(ret, 0); + + ret = nc_server_config_add_ch_persistent(test_ctx->ctx, "ch_interrupt", &tree); + assert_int_equal(ret, 0); + + ret = nc_server_config_add_ch_ssh_hostkey(test_ctx->ctx, "ch_interrupt", "endpt", "hostkey", + TESTS_DIR "/data/key_ecdsa", NULL, &tree); + assert_int_equal(ret, 0); + + ret = nc_server_config_add_ch_ssh_user_pubkey(test_ctx->ctx, "ch_interrupt", "endpt", "test_ch_interrupt", + "pubkey", TESTS_DIR "/data/id_ed25519.pub", &tree); + assert_int_equal(ret, 0); + + ret = nc_server_config_setup_data(tree); + assert_int_equal(ret, 0); + + /* prepare the configuration without the client, applied by the test itself */ + ret = nc_server_config_del_ch_client("ch_interrupt", &tree); + assert_int_equal(ret, 0); + + test_data->tree = tree; + return 0; +} + +static int +setup_interrupt_tls(void **state) +{ + int ret; + struct lyd_node *tree = NULL; + struct ln2_test_ctx *test_ctx; + struct test_ch_data *test_data; + + ret = ln2_glob_test_setup(&test_ctx); + assert_int_equal(ret, 0); + + test_data = calloc(1, sizeof *test_data); + assert_non_null(test_data); + + test_ctx->test_data = test_data; + test_ctx->free_test_data = test_nc_ch_free_test_data; + *state = test_ctx; + + interrupt_listen_sock = test_ch_silent_listen(); + + ret = nc_server_config_add_ch_address_port(test_ctx->ctx, "ch_interrupt", "endpt", NC_TI_TLS, + "127.0.0.1", interrupt_port_str, &tree); + assert_int_equal(ret, 0); + + ret = nc_server_config_add_ch_persistent(test_ctx->ctx, "ch_interrupt", &tree); + assert_int_equal(ret, 0); + + ret = nc_server_config_add_ch_tls_server_cert(test_ctx->ctx, "ch_interrupt", "endpt", + TESTS_DIR "/data/server.key", NULL, TESTS_DIR "/data/server.crt", &tree); + assert_int_equal(ret, 0); + + ret = nc_server_config_add_ch_tls_client_cert(test_ctx->ctx, "ch_interrupt", "endpt", "ee-cert", + TESTS_DIR "/data/client.crt", &tree); + assert_int_equal(ret, 0); + + ret = nc_server_config_add_ch_tls_ca_cert(test_ctx->ctx, "ch_interrupt", "endpt", "ca-cert", + TESTS_DIR "/data/serverca.pem", &tree); + assert_int_equal(ret, 0); + + ret = nc_server_config_add_ch_tls_ctn(test_ctx->ctx, "ch_interrupt", "endpt", 1, + "04:85:6B:75:D1:1A:86:E0:D8:FE:5B:BD:72:F5:73:1D:07:EA:32:BF:09:11:21:6A:6E:23:78:8E:B6:D5:73:C3:2D", + NC_TLS_CTN_SPECIFIED, "ch_client_tls", &tree); + assert_int_equal(ret, 0); + + ret = nc_server_config_setup_data(tree); + assert_int_equal(ret, 0); + + /* prepare the configuration without the client, applied by the test itself */ + ret = nc_server_config_del_ch_client("ch_interrupt", &tree); + assert_int_equal(ret, 0); + + test_data->tree = tree; + return 0; +} + +static void +test_nc_ch_interrupt_ssh_handshake(void **state) +{ + test_ch_interrupt_apply(state); +} + +static void +test_nc_ch_interrupt_tls_handshake(void **state) +{ + test_ch_interrupt_apply(state); +} + int main(void) { @@ -766,6 +979,8 @@ main(void) cmocka_unit_test_setup_teardown(test_nc_ch_tls, setup_tls, ln2_glob_test_teardown), cmocka_unit_test_setup_teardown(test_nc_ch_delete_client_while_session, setup_delete_while_session, ln2_glob_test_teardown), cmocka_unit_test_setup_teardown(test_nc_ch_two_simultaneous, setup_two_simultaneous, ln2_glob_test_teardown), + cmocka_unit_test_setup_teardown(test_nc_ch_interrupt_ssh_handshake, setup_interrupt_ssh, ln2_glob_test_teardown), + cmocka_unit_test_setup_teardown(test_nc_ch_interrupt_tls_handshake, setup_interrupt_tls, ln2_glob_test_teardown), }; if (ln2_glob_test_get_ports(4, &TEST_PORT, &TEST_PORT_STR, &TEST_PORT_2, &TEST_PORT_2_STR, diff --git a/tests/test_config.c b/tests/test_config.c index 78fb0b11..17f0c455 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -1149,6 +1149,9 @@ static unsigned int test_stall_auth_sleep = TEST_STALL_AUTH_SLEEP; /** @brief Maximum number of distinct Call Home threads the test keeps track of. */ #define TEST_CH_TID_MAX 8 +/** @brief Time in seconds to wait for a Call Home client to report what a test is waiting for. */ +#define TEST_CH_WAIT_TIME 10 + struct test_ch_threads { pthread_mutex_t lock; pthread_cond_t cond; @@ -1295,16 +1298,22 @@ test_ch_dispatch_not_duplicated(void **state) ret = nc_server_config_setup_data(tree); assert_int_equal(ret, 0); - /* wait until its thread reports a failed connection attempt */ + /* wait until its thread reports a failed connection attempt, the deadline is absolute so that + * repeated wakeups cannot extend the wait indefinitely */ + ret = 0; pthread_mutex_lock(&threads.lock); - while (!threads.tid_count) { - clock_gettime(CLOCK_REALTIME, &ts); - ts.tv_sec += 10; + clock_gettime(CLOCK_REALTIME, &ts); + ts.tv_sec += TEST_CH_WAIT_TIME; + while (!threads.tid_count && !ret) { ret = pthread_cond_timedwait(&threads.cond, &threads.lock, &ts); - assert_int_equal(ret, 0); } + tid_count = threads.tid_count; pthread_mutex_unlock(&threads.lock); + /* only report the failure once the lock is released */ + assert_int_equal(ret, 0); + assert_int_not_equal(tid_count, 0); + /* apply the very same data again, the client is already running */ ret = nc_server_config_setup_data(tree); assert_int_equal(ret, 0); @@ -1334,6 +1343,7 @@ static void test_ch_endpoint_order(void **state) { int ret; + char endpt[64] = {0}; struct lyd_node *tree = NULL, *diff = NULL; struct ln2_test_ctx *test_ctx = *state; struct test_ch_threads threads = {0}; @@ -1381,17 +1391,22 @@ test_ch_endpoint_order(void **state) test_ch_new_session_cb, NULL); assert_int_equal(ret, 0); - /* wait for the first failed connection attempt */ + /* wait for the first failed connection attempt, the deadline is absolute so that repeated + * wakeups cannot extend the wait indefinitely */ + ret = 0; pthread_mutex_lock(&threads.lock); - while (!threads.endpt[0]) { - clock_gettime(CLOCK_REALTIME, &ts); - ts.tv_sec += 10; + clock_gettime(CLOCK_REALTIME, &ts); + ts.tv_sec += TEST_CH_WAIT_TIME; + while (!threads.endpt[0] && !ret) { ret = pthread_cond_timedwait(&threads.cond, &threads.lock, &ts); - assert_int_equal(ret, 0); } - assert_string_equal(threads.endpt, "second"); + strncpy(endpt, threads.endpt, sizeof endpt - 1); pthread_mutex_unlock(&threads.lock); + /* only report the failure once the lock is released */ + assert_int_equal(ret, 0); + assert_string_equal(endpt, "second"); + lyd_free_all(diff); lyd_free_all(tree); pthread_cond_destroy(&threads.cond); @@ -1687,14 +1702,18 @@ test_ch_wait_for_endpt(struct test_ch_threads *threads, const char *endpt_name) int ret; struct timespec ts; + /* the deadline is absolute so that repeated wakeups cannot extend the wait indefinitely */ + ret = 0; pthread_mutex_lock(&threads->lock); - while (strcmp(threads->last_endpt, endpt_name)) { - clock_gettime(CLOCK_REALTIME, &ts); - ts.tv_sec += 10; + clock_gettime(CLOCK_REALTIME, &ts); + ts.tv_sec += TEST_CH_WAIT_TIME; + while (strcmp(threads->last_endpt, endpt_name) && !ret) { ret = pthread_cond_timedwait(&threads->cond, &threads->lock, &ts); - assert_int_equal(ret, 0); } pthread_mutex_unlock(&threads->lock); + + /* only report the failure once the lock is released */ + assert_int_equal(ret, 0); } /**