From 468182e27d467af695b6aaf9fc5ad88c505e2ebc Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 4 Oct 2024 14:46:36 +0200 Subject: [PATCH 1/9] session wrapper REFACTOR set sock in designated fn --- src/session_client_tls.c | 4 ++-- src/session_mbedtls.c | 6 +++--- src/session_openssl.c | 4 ++-- src/session_server_tls.c | 4 ++-- src/session_wrapper.h | 5 ++--- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/session_client_tls.c b/src/session_client_tls.c index 722dd0a1..c5a214b4 100644 --- a/src/session_client_tls.c +++ b/src/session_client_tls.c @@ -296,7 +296,7 @@ nc_client_tls_session_new(int sock, const char *host, int timeout, struct nc_cli nc_client_tls_set_verify_wrap(tls_cfg); /* init TLS context and store data which may be needed later in it */ - if (nc_tls_init_ctx_wrap(sock, cli_cert, cli_pkey, cert_store, crl_store, tls_ctx)) { + if (nc_tls_init_ctx_wrap(cli_cert, cli_pkey, cert_store, crl_store, tls_ctx)) { goto fail; } @@ -315,7 +315,7 @@ nc_client_tls_session_new(int sock, const char *host, int timeout, struct nc_cli } /* set session fd */ - nc_server_tls_set_fd_wrap(tls_session, sock, tls_ctx); + nc_tls_set_fd_wrap(tls_session, sock, tls_ctx); sock = -1; diff --git a/src/session_mbedtls.c b/src/session_mbedtls.c index 8422532a..e0947b8f 100644 --- a/src/session_mbedtls.c +++ b/src/session_mbedtls.c @@ -899,9 +899,10 @@ nc_server_tls_recv(void *ctx, unsigned char *buf, size_t len) } void -nc_server_tls_set_fd_wrap(void *tls_session, int UNUSED(sock), struct nc_tls_ctx *tls_ctx) +nc_tls_set_fd_wrap(void *tls_session, int sock, struct nc_tls_ctx *tls_ctx) { /* mbedtls sets a pointer to the sock, which is stored in tls_ctx */ + *tls_ctx->sock = sock; mbedtls_ssl_set_bio(tls_session, tls_ctx->sock, nc_server_tls_send, nc_server_tls_recv, NULL); } @@ -1052,7 +1053,7 @@ nc_client_tls_set_hostname_wrap(void *tls_session, const char *hostname) } int -nc_tls_init_ctx_wrap(int sock, void *cert, void *pkey, void *cert_store, void *crl_store, struct nc_tls_ctx *tls_ctx) +nc_tls_init_ctx_wrap(void *cert, void *pkey, void *cert_store, void *crl_store, struct nc_tls_ctx *tls_ctx) { /* setup rng */ if (nc_tls_rng_new(&tls_ctx->ctr_drbg, &tls_ctx->entropy)) { @@ -1062,7 +1063,6 @@ nc_tls_init_ctx_wrap(int sock, void *cert, void *pkey, void *cert_store, void *c /* fill the context */ tls_ctx->sock = malloc(sizeof *tls_ctx->sock); NC_CHECK_ERRMEM_RET(!tls_ctx->sock, 1); - *tls_ctx->sock = sock; tls_ctx->cert = cert; tls_ctx->pkey = pkey; tls_ctx->cert_store = cert_store; diff --git a/src/session_openssl.c b/src/session_openssl.c index 0ec1d9b6..c3cb178d 100644 --- a/src/session_openssl.c +++ b/src/session_openssl.c @@ -589,7 +589,7 @@ nc_server_tls_sha512_wrap(void *cert, unsigned char *buf) } void -nc_server_tls_set_fd_wrap(void *tls_session, int sock, struct nc_tls_ctx *UNUSED(tls_ctx)) +nc_tls_set_fd_wrap(void *tls_session, int sock, struct nc_tls_ctx *UNUSED(tls_ctx)) { SSL_set_fd(tls_session, sock); } @@ -715,7 +715,7 @@ nc_client_tls_set_hostname_wrap(void *tls_session, const char *hostname) } int -nc_tls_init_ctx_wrap(int UNUSED(sock), void *cert, void *pkey, void *cert_store, void *crl_store, struct nc_tls_ctx *tls_ctx) +nc_tls_init_ctx_wrap(void *cert, void *pkey, void *cert_store, void *crl_store, struct nc_tls_ctx *tls_ctx) { tls_ctx->cert = cert; tls_ctx->pkey = pkey; diff --git a/src/session_server_tls.c b/src/session_server_tls.c index ed89fb25..93cb4341 100644 --- a/src/session_server_tls.c +++ b/src/session_server_tls.c @@ -879,7 +879,7 @@ nc_accept_tls_session(struct nc_session *session, struct nc_server_tls_opts *opt nc_server_tls_set_verify_wrap(tls_cfg, &cb_data); /* init TLS context and store data which may be needed later in it */ - if (nc_tls_init_ctx_wrap(sock, srv_cert, srv_pkey, cert_store, crl_store, &session->ti.tls.ctx)) { + if (nc_tls_init_ctx_wrap(srv_cert, srv_pkey, cert_store, crl_store, &session->ti.tls.ctx)) { goto fail; } @@ -900,7 +900,7 @@ nc_accept_tls_session(struct nc_session *session, struct nc_server_tls_opts *opt } /* set session fd */ - nc_server_tls_set_fd_wrap(session->ti.tls.session, sock, &session->ti.tls.ctx); + nc_tls_set_fd_wrap(session->ti.tls.session, sock, &session->ti.tls.ctx); sock = -1; diff --git a/src/session_wrapper.h b/src/session_wrapper.h index 6fb68bd3..729a17ac 100644 --- a/src/session_wrapper.h +++ b/src/session_wrapper.h @@ -369,7 +369,7 @@ int nc_server_tls_sha512_wrap(void *cert, unsigned char *buf); * @param[in] sock Socket FD. * @param[in] tls_ctx TLS context. */ -void nc_server_tls_set_fd_wrap(void *tls_session, int sock, struct nc_tls_ctx *tls_ctx); +void nc_tls_set_fd_wrap(void *tls_session, int sock, struct nc_tls_ctx *tls_ctx); /** * @brief Perform a server-side step of the TLS handshake. @@ -428,7 +428,6 @@ int nc_client_tls_set_hostname_wrap(void *tls_session, const char *hostname); /** * @brief Initialize a TLS context. * - * @param[in] sock Socket FD. * @param[in] cert Certificate. * @param[in] pkey Private key. * @param[in] cert_store Certificate store. @@ -436,7 +435,7 @@ int nc_client_tls_set_hostname_wrap(void *tls_session, const char *hostname); * @param[in,out] tls_ctx TLS context. * @return 0 on success, non-zero on fail. */ -int nc_tls_init_ctx_wrap(int sock, void *cert, void *pkey, void *cert_store, void *crl_store, struct nc_tls_ctx *tls_ctx); +int nc_tls_init_ctx_wrap(void *cert, void *pkey, void *cert_store, void *crl_store, struct nc_tls_ctx *tls_ctx); /** * @brief Setup a TLS configuration from a TLS context. From e310b6293ebe3b217c32b32927ab2cf6ffc915db Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 4 Oct 2024 14:50:53 +0200 Subject: [PATCH 2/9] session client UPDATE add tcp keepalive api --- src/session_client.c | 31 +++++++++++++++++++++++++++++++ src/session_client.h | 21 +++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/session_client.c b/src/session_client.c index dafa0d53..6c876a70 100644 --- a/src/session_client.c +++ b/src/session_client.c @@ -3217,3 +3217,34 @@ nc_client_session_set_not_strict(struct nc_session *session) session->flags |= NC_SESSION_CLIENT_NOT_STRICT; } + +API void +nc_client_enable_tcp_keepalives(int enable) +{ + client_opts.ka.enabled = enable; +} + +API void +nc_client_set_tcp_keepalives(uint16_t idle_time, uint16_t max_probes, uint16_t probe_interval) +{ + if (!idle_time) { + /* default */ + client_opts.ka.idle_time = 1; + } else { + client_opts.ka.idle_time = idle_time; + } + + if (!max_probes) { + /* default */ + client_opts.ka.max_probes = 10; + } else { + client_opts.ka.max_probes = max_probes; + } + + if (!probe_interval) { + /* default */ + client_opts.ka.probe_interval = 5; + } else { + client_opts.ka.probe_interval = probe_interval; + } +} diff --git a/src/session_client.h b/src/session_client.h index faf7e424..1fb923f6 100644 --- a/src/session_client.h +++ b/src/session_client.h @@ -617,6 +617,27 @@ NC_MSG_TYPE nc_send_rpc(struct nc_session *session, struct nc_rpc *rpc, int time */ void nc_client_session_set_not_strict(struct nc_session *session); +/** + * @brief Enable or disable TCP keepalives. Only affects new sessions. + * + * Client-side TCP keepalives have the following default values: + * - idle time: 1 second + * - max probes: 10 + * - probe interval: 5 seconds + * + * @param[in] enable Whether to enable or disable TCP keepalives. + */ +void nc_client_enable_tcp_keepalives(int enable); + +/** + * @brief Set TCP keepalive options. + * + * @param[in] idle_time Time in seconds before the first keepalive probe is sent. If 0, the default value 1 is used. + * @param[in] max_probes Maximum number of keepalive probes to send before considering the connection dead. If 0, the default value 10 is used. + * @param[in] probe_interval Time in seconds between individual keepalive probes. If 0, the default value 5 is used. + */ +void nc_client_set_tcp_keepalives(uint16_t idle_time, uint16_t max_probes, uint16_t probe_interval); + /** @} Client Session */ #ifdef __cplusplus From 25aa0c079493a7b9c9464f9d0fabac813781b8ee Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 4 Oct 2024 14:53:45 +0200 Subject: [PATCH 3/9] session client UPDATE add sess monitoring thread --- src/session.c | 7 + src/session_client.c | 322 +++++++++++++++++++++++++++++++++++++++ src/session_client.h | 31 ++++ src/session_client_ssh.c | 21 +++ src/session_client_tls.c | 14 ++ src/session_p.h | 44 ++++++ 6 files changed, 439 insertions(+) diff --git a/src/session.c b/src/session.c index b65f2349..6e2e1eb4 100644 --- a/src/session.c +++ b/src/session.c @@ -857,6 +857,13 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *)) return; } + if (session->side == NC_CLIENT) { + if (session->flags & NC_SESSION_CLIENT_MONITORED) { + /* remove the session from the monitored list */ + nc_client_monitoring_session_stop(session, 1); + } + } + /* stop notification threads if any */ if ((session->side == NC_CLIENT) && ATOMIC_LOAD_RELAXED(session->opts.client.ntf_thread_running)) { /* let the threads know they should quit */ diff --git a/src/session_client.c b/src/session_client.c index 6c876a70..738fd4fa 100644 --- a/src/session_client.c +++ b/src/session_client.c @@ -1456,6 +1456,13 @@ nc_connect_inout(int fdin, int fdout, struct ly_ctx *ctx) goto fail; } + /* start monitoring the session */ + if (client_opts.monitoring_thread_data) { + if (nc_client_monitoring_session_start(session)) { + goto fail; + } + } + return session; fail: @@ -1533,6 +1540,13 @@ nc_connect_unix(const char *address, struct ly_ctx *ctx) goto fail; } + /* start monitoring the session */ + if (client_opts.monitoring_thread_data) { + if (nc_client_monitoring_session_start(session)) { + goto fail; + } + } + return session; fail: @@ -3218,6 +3232,314 @@ nc_client_session_set_not_strict(struct nc_session *session) session->flags |= NC_SESSION_CLIENT_NOT_STRICT; } +/** + * @brief Get the file descriptor of a session. + * + * @param[in] session Session. + * + * @return File descriptor of the session or -1 in case of an error. + */ +static int +nc_client_monitoring_get_session_fd(struct nc_session *session) +{ + int fd = -1; + + switch ((session->ti_type)) { + case NC_TI_FD: + fd = session->ti.fd.in; + break; + case NC_TI_UNIX: + fd = session->ti.unixsock.sock; + break; +#ifdef NC_ENABLED_SSH_TLS + case NC_TI_SSH: + fd = ssh_get_fd(session->ti.libssh.session); + break; + case NC_TI_TLS: + fd = nc_tls_get_fd_wrap(session); + break; +#endif /* NC_ENABLED_SSH_TLS */ + default: + ERR(session, "Unknown transport type."); + break; + } + + return fd; +} + +int +nc_client_monitoring_session_start(struct nc_session *session) +{ + int ret = 0, fd; + struct nc_client_monitoring_thread_arg *mtarg; + void *tmp, *tmp2; + + mtarg = client_opts.monitoring_thread_data; + if (!mtarg) { + ERRINT; + return 1; + } + + /* get the session's file descriptor */ + fd = nc_client_monitoring_get_session_fd(session); + if (fd == -1) { + ERR(session, "Unable to monitor an invalid file descriptor."); + return 1; + } + + /* LOCK */ + pthread_mutex_lock(&mtarg->lock); + + /* if realloc fails, keep the original sessions without adding the new one */ + tmp = realloc(mtarg->sessions, (mtarg->session_count + 1) * sizeof *mtarg->sessions); + NC_CHECK_ERRMEM_GOTO(!tmp, ret = 1, cleanup); + tmp2 = realloc(mtarg->pfds, (mtarg->pfd_count + 1) * sizeof *mtarg->pfds); + NC_CHECK_ERRMEM_GOTO(!tmp2, free(tmp); ret = 1, cleanup); + + mtarg->sessions = tmp; + mtarg->sessions[mtarg->session_count] = session; + mtarg->session_count++; + + /* we are only interested in POLLHUP or POLLNVAL */ + mtarg->pfds = tmp2; + mtarg->pfds[mtarg->pfd_count].fd = fd; + mtarg->pfds[mtarg->pfd_count].events = 0; + mtarg->pfds[mtarg->pfd_count].revents = 0; + mtarg->pfd_count++; + + session->flags |= NC_SESSION_CLIENT_MONITORED; + +cleanup: + /* UNLOCK */ + pthread_mutex_unlock(&mtarg->lock); + return ret; +} + +void +nc_client_monitoring_session_stop(struct nc_session *session, int lock) +{ + int i; + struct nc_client_monitoring_thread_arg *mtarg; + + mtarg = client_opts.monitoring_thread_data; + if (!mtarg) { + ERRINT; + return; + } + + if (lock) { + /* LOCK */ + pthread_mutex_lock(&mtarg->lock); + } + + /* find the session */ + for (i = 0; i < mtarg->session_count; i++) { + if (mtarg->sessions[i] == session) { + break; + } + } + if (i == mtarg->session_count) { + /* not found */ + ERR(session, "Session is not being monitored."); + goto cleanup; + } + + /* remove the session */ + mtarg->session_count--; + mtarg->pfd_count--; + if (!mtarg->session_count) { + free(mtarg->sessions); + mtarg->sessions = NULL; + free(mtarg->pfds); + mtarg->pfds = NULL; + } else if (mtarg->sessions[i] != mtarg->sessions[mtarg->session_count]) { + mtarg->sessions[i] = mtarg->sessions[mtarg->session_count]; + mtarg->pfds[i] = mtarg->pfds[mtarg->session_count]; + } + + session->flags &= ~NC_SESSION_CLIENT_MONITORED; + +cleanup: + if (lock) { + /* UNLOCK */ + pthread_mutex_unlock(&mtarg->lock); + } +} + +/** + * @brief Monitoring thread for client sessions. + * + * @param[in] arg Thread context of the creating thread. + * @return NULL. + */ +static void * +nc_client_monitoring_thread(void *arg) +{ + int r; + struct nc_client_monitoring_thread_arg *mtarg; + nfds_t i; + struct nc_session *session; + + /* set this thread's context to the one from the thread that started the monitoring thread */ + nc_client_set_thread_context(arg); + + /* so that both threads share the same opts */ + mtarg = client_opts.monitoring_thread_data; + if (!mtarg) { + ERRINT; + return NULL; + } + + /* LOCK */ + pthread_mutex_lock(&mtarg->lock); + + while (mtarg->thread_running) { + if (!mtarg->session_count) { + goto next_iter; + } + + /* poll for either POLLIN or POLLHUP events */ + r = poll(mtarg->pfds, mtarg->pfd_count, 0); + if (r == -1) { + if (errno == EINTR) { + continue; + } + + ERR(NULL, "Client monitoring thread: poll failed (%s).", strerror(errno)); + goto cleanup; + } else if (!r) { + /* no events */ + goto next_iter; + } + + /* check the events */ + i = 0; + while (i < mtarg->pfd_count) { + if (mtarg->pfds[i].revents & (POLLHUP | POLLNVAL)) { + /* call the callback and free the session */ + session = mtarg->sessions[i]; + session->status = NC_STATUS_INVALID; + session->term_reason = NC_SESSION_TERM_DROPPED; + mtarg->clb(session, mtarg->clb_data); + + /* session will be removed from the list, continue from the same index */ + nc_client_monitoring_session_stop(session, 0); + nc_session_free(session, NULL); + } else { + i++; + } + } + +next_iter: + /* UNLOCK */ + pthread_mutex_unlock(&mtarg->lock); + + usleep(NC_CLIENT_MONITORING_BACKOFF * 1000); + + /* LOCK */ + pthread_mutex_lock(&mtarg->lock); + } + +cleanup: + client_opts.monitoring_thread_data = NULL; + + /* UNLOCK */ + pthread_mutex_unlock(&mtarg->lock); + + if (mtarg->clb_free_data) { + mtarg->clb_free_data(mtarg->clb_data); + } + for (i = 0; i < mtarg->session_count; i++) { + mtarg->sessions[i]->flags &= ~NC_SESSION_CLIENT_MONITORED; + } + free(mtarg->sessions); + free(mtarg->pfds); + pthread_mutex_destroy(&mtarg->lock); + + free(mtarg); + + VRB(NULL, "Client monitoring thread exit."); + return NULL; +} + +API int +nc_client_monitoring_thread_start(nc_client_monitoring_clb monitoring_clb, void *user_data, void (*free_data)(void *)) +{ + int ret = 0, r; + void *ctx; + + NC_CHECK_ARG_RET(NULL, monitoring_clb, 1); + + if (client_opts.monitoring_thread_data) { + ERR(NULL, "Client monitoring thread is already running."); + return 1; + } + + client_opts.monitoring_thread_data = calloc(1, sizeof *client_opts.monitoring_thread_data); + NC_CHECK_ERRMEM_RET(!client_opts.monitoring_thread_data, 1); + + client_opts.monitoring_thread_data->clb = monitoring_clb; + client_opts.monitoring_thread_data->clb_data = user_data; + client_opts.monitoring_thread_data->clb_free_data = free_data; + + pthread_mutex_init(&client_opts.monitoring_thread_data->lock, NULL); + + /* get the current thread context, so that the monitoring thread can use it */ + ctx = nc_client_get_thread_context(); + if (!ctx) { + ERR(NULL, "Failed to get the client thread context."); + ret = 1; + goto cleanup; + } + + client_opts.monitoring_thread_data->thread_running = 1; + + r = pthread_create(&client_opts.monitoring_thread_data->tid, NULL, + nc_client_monitoring_thread, ctx); + if (r) { + ERR(NULL, "Failed to create the client monitoring thread (%s).", strerror(r)); + ret = 1; + goto cleanup; + } + +cleanup: + if (ret) { + pthread_mutex_destroy(&client_opts.monitoring_thread_data->lock); + free(client_opts.monitoring_thread_data); + } + + return ret; +} + +API void +nc_client_monitoring_thread_stop(void) +{ + int r; + pthread_t tid; + struct nc_client_monitoring_thread_arg *mtarg; + + if (!client_opts.monitoring_thread_data) { + ERR(NULL, "Client monitoring thread is not running."); + return; + } + + mtarg = client_opts.monitoring_thread_data; + + /* LOCK */ + pthread_mutex_lock(&mtarg->lock); + + mtarg->thread_running = 0; + tid = mtarg->tid; + + /* UNLOCK */ + pthread_mutex_unlock(&mtarg->lock); + + r = pthread_join(tid, NULL); + if (r) { + ERR(NULL, "Failed to join the client monitoring thread (%s).", strerror(r)); + } +} + API void nc_client_enable_tcp_keepalives(int enable) { diff --git a/src/session_client.h b/src/session_client.h index 1fb923f6..9d37a8da 100644 --- a/src/session_client.h +++ b/src/session_client.h @@ -617,6 +617,37 @@ NC_MSG_TYPE nc_send_rpc(struct nc_session *session, struct nc_rpc *rpc, int time */ void nc_client_session_set_not_strict(struct nc_session *session); +/** + * @brief Callback for monitoring client sessions. + * + * This callback is called whenever the client finds out that a session was terminated by the server. + * + * @param[in] session Terminated session, which will be freed after the callback finishes executing. + * @param[in] user_data Arbitrary user data passed to the callback. + */ +typedef void (*nc_client_monitoring_clb)(struct nc_session *session, void *user_data); + +/** + * @brief Start a thread that monitors client sessions. + * + * If the thread is running, new sessions will be monitored automatically. + * + * Note that once you start the monitoring thread, any other client thread that + * calls ::nc_session_free() needs to share the same thread context (or be the same thread) + * as the thread that called this function (see ::nc_client_set_thread_context()). + * + * @param[in] monitoring_clb Callback called whenever a session is terminated. + * @param[in] user_data Arbitrary user data passed to the callback. + * @param[in] free_data Callback for freeing the user data after monitoring thread exits. + * @return 0 on success, 1 on error. + */ +int nc_client_monitoring_thread_start(nc_client_monitoring_clb monitoring_clb, void *user_data, void (*free_data)(void *)); + +/** + * @brief Stop the client session monitoring thread. + */ +void nc_client_monitoring_thread_stop(void); + /** * @brief Enable or disable TCP keepalives. Only affects new sessions. * diff --git a/src/session_client_ssh.c b/src/session_client_ssh.c index 676ccdf7..ad692812 100644 --- a/src/session_client_ssh.c +++ b/src/session_client_ssh.c @@ -1742,6 +1742,13 @@ _nc_connect_libssh(ssh_session ssh_session, struct ly_ctx *ctx, struct nc_keepal goto fail; } + /* start monitoring the session */ + if (client_opts.monitoring_thread_data) { + if (nc_client_monitoring_session_start(session)) { + goto fail; + } + } + /* store information if not previously */ session->host = host; session->port = port; @@ -1848,6 +1855,13 @@ nc_connect_ssh(const char *host, uint16_t port, struct ly_ctx *ctx) goto fail; } + /* start monitoring the session */ + if (client_opts.monitoring_thread_data) { + if (nc_client_monitoring_session_start(session)) { + goto fail; + } + } + /* update information */ free(session->host); session->host = ip_host; @@ -1920,6 +1934,13 @@ nc_connect_ssh_channel(struct nc_session *session, struct ly_ctx *ctx) goto fail; } + /* start monitoring the session */ + if (client_opts.monitoring_thread_data) { + if (nc_client_monitoring_session_start(session)) { + goto fail; + } + } + /* store information into session */ new_session->host = strdup(session->host); new_session->port = session->port; diff --git a/src/session_client_tls.c b/src/session_client_tls.c index c5a214b4..091ac5d1 100644 --- a/src/session_client_tls.c +++ b/src/session_client_tls.c @@ -421,6 +421,13 @@ nc_connect_tls(const char *host, unsigned short port, struct ly_ctx *ctx) goto fail; } + /* start monitoring the session */ + if (client_opts.monitoring_thread_data) { + if (nc_client_monitoring_session_start(session)) { + goto fail; + } + } + /* store information into session */ session->host = ip_host; session->port = port; @@ -481,6 +488,13 @@ nc_accept_callhome_tls_sock(int sock, const char *host, uint16_t port, struct ly session->flags |= NC_SESSION_CALLHOME; + /* start monitoring the session */ + if (client_opts.monitoring_thread_data) { + if (nc_client_monitoring_session_start(session)) { + goto fail; + } + } + /* store information into session */ session->host = strdup(host); session->port = port; diff --git a/src/session_p.h b/src/session_p.h index 95e66fb3..0e73be8c 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -359,6 +359,25 @@ struct nc_client_tls_opts { #endif /* NC_ENABLED_SSH_TLS */ +/** + * @brief Stores data for the client monitoring thread. + */ +struct nc_client_monitoring_thread_arg { + struct nc_session **sessions; /**< Array of monitored sessions. */ + uint16_t session_count; /**< Number of monitored sessions. */ + + struct pollfd *pfds; /**< Array of poll file descriptors corresponding to the monitored sessions. */ + nfds_t pfd_count; /**< Number of poll file descriptors. */ + + pthread_t tid; /**< Thread ID of the monitoring thread. */ + int thread_running; /**< Flag representing the runningness of the monitoring thread. */ + pthread_mutex_t lock; /**< Monitoring thread data lock. */ + + nc_client_monitoring_clb clb; /**< Callback called when a monitored session is terminated by the server. */ + void *clb_data; /**< Data passed to the callback. */ + void (*clb_free_data)(void *); /**< Callback to free the user data. */ +}; + /* ACCESS unlocked */ struct nc_client_opts { char *schema_searchpath; @@ -375,6 +394,8 @@ struct nc_client_opts { char *hostname; } *ch_binds_aux; uint16_t ch_bind_count; + + struct nc_client_monitoring_thread_arg *monitoring_thread_data; /**< Data of the monitoring thread. */ }; /* ACCESS unlocked */ @@ -652,6 +673,11 @@ struct nc_server_opts { */ #define NC_REVERSE_QUEUE 5 +/** + * Time slept in msec in each cycle of the client monitoring thread. + */ +#define NC_CLIENT_MONITORING_BACKOFF 200 + /** * @brief Type of the session */ @@ -749,6 +775,7 @@ struct nc_session { /* client flags */ /* some server modules failed to load so the data from them will be ignored - not use strict flag for parsing */ # define NC_SESSION_CLIENT_NOT_STRICT 0x08 +# define NC_SESSION_CLIENT_MONITORED 0x40 /**< session is being monitored by the client monitoring thread */ } client; struct { /* server side only data */ @@ -851,6 +878,23 @@ void * nc_base64der_to_cert(const char *data); #endif /* NC_ENABLED_SSH_TLS */ +/** + * @brief Start monitoring a session. + * + * @param[in] session Session to start monitoring. + * + * @return 0 on success, 1 on error. + */ +int nc_client_monitoring_session_start(struct nc_session *session); + +/** + * @brief Stop monitoring a session. + * + * @param[in] session Session to stop monitoring. + * @param[in] lock Whether to lock the thread data while removing the session from the list. + */ +void nc_client_monitoring_session_stop(struct nc_session *session, int lock); + void *nc_realloc(void *ptr, size_t size); /** From 87f9a17ece2030823877f6099bb0ed7fe25e92fd Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 4 Oct 2024 14:57:08 +0200 Subject: [PATCH 4/9] faq UPDATE describe tcp_fin_timeout --- FAQ.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/FAQ.md b/FAQ.md index 14e2c1ab..a30b19c1 100644 --- a/FAQ.md +++ b/FAQ.md @@ -65,3 +65,13 @@ __A:__ No, it is not possible. There are currently 2 main types of certificates which is extracted from the certificate, is sent to the server instead of the whole certificate. This means that the `cert-to-name` process required by *NETCONF* can not take place. Specifically, OpenSSH certificates are missing important fields such as `Common Name`, `Subject Alternative Name` and so on. + +__Q: I have client-side keepalives and monitoring enabled, but it takes a long time for the client to detect that the connection was terminated:__ +__A:__ Assuming that the network connection is fine or is loopback, then this is the standard TCP behavior. + The client will not immediately detect that the connection was terminated unless + it tries to send some data or unless a specific timeout occurs. + + Even though the server was terminated, its socket remains in a lingering state for some time and continues to reply to incoming + TCP keepalive packets. In particular, this timeout you're encountering is most likely affected by the `tcp_fin_timeout` kernel parameter, + which controls how long the TCP stack waits before timing out a half-closed connection after receiving a FIN packet. + The default value is typically 60 seconds, but it can be configured based on your needs. From 95278eb8d8001a715676c4d126c749c83e350317 Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 4 Oct 2024 15:01:19 +0200 Subject: [PATCH 5/9] SOVERSION bump to version 4.4.2 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1b0e160e..c8bdfd42 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -66,7 +66,7 @@ set(LIBNETCONF2_VERSION ${LIBNETCONF2_MAJOR_VERSION}.${LIBNETCONF2_MINOR_VERSION # with backward compatible change and micro version is connected with any internal change of the library. set(LIBNETCONF2_MAJOR_SOVERSION 4) set(LIBNETCONF2_MINOR_SOVERSION 4) -set(LIBNETCONF2_MICRO_SOVERSION 1) +set(LIBNETCONF2_MICRO_SOVERSION 2) set(LIBNETCONF2_SOVERSION_FULL ${LIBNETCONF2_MAJOR_SOVERSION}.${LIBNETCONF2_MINOR_SOVERSION}.${LIBNETCONF2_MICRO_SOVERSION}) set(LIBNETCONF2_SOVERSION ${LIBNETCONF2_MAJOR_SOVERSION}) From 3fc5670d84110e3f3123a731b2d4a53e8648485d Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 4 Oct 2024 15:01:34 +0200 Subject: [PATCH 6/9] VERSION bump to version 3.5.2 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c8bdfd42..2a5122c9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,7 +58,7 @@ set(CMAKE_MACOSX_RPATH TRUE) # micro version is changed with a set of small changes or bugfixes anywhere in the project. set(LIBNETCONF2_MAJOR_VERSION 3) set(LIBNETCONF2_MINOR_VERSION 5) -set(LIBNETCONF2_MICRO_VERSION 1) +set(LIBNETCONF2_MICRO_VERSION 2) set(LIBNETCONF2_VERSION ${LIBNETCONF2_MAJOR_VERSION}.${LIBNETCONF2_MINOR_VERSION}.${LIBNETCONF2_MICRO_VERSION}) # Version of the library From 4505d8baa9ebd2bcc58af1fdd9af69116cee7b9b Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 18 Oct 2024 09:48:39 +0200 Subject: [PATCH 7/9] session client BUGFIX set ka default values --- src/session_client.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/session_client.c b/src/session_client.c index 738fd4fa..b15196bf 100644 --- a/src/session_client.c +++ b/src/session_client.c @@ -165,6 +165,10 @@ nc_client_context_location(void) e = calloc(1, sizeof *e); /* set default values */ e->refcount = 1; + e->opts.ka.enabled = 1; + e->opts.ka.idle_time = 1; + e->opts.ka.max_probes = 10; + e->opts.ka.probe_interval = 5; #ifdef NC_ENABLED_SSH_TLS # ifdef HAVE_TERMIOS e->ssh_opts.knownhosts_mode = NC_SSH_KNOWNHOSTS_ASK; From bc3961903ef1891a35a99ecc3f84942c24e5a7b8 Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 18 Oct 2024 09:49:48 +0200 Subject: [PATCH 8/9] session_p REFACTOR group up session flags --- src/session_p.h | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/session_p.h b/src/session_p.h index 0e73be8c..ab88cfd3 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -757,10 +757,23 @@ struct nc_session { struct ly_ctx *ctx; /**< libyang context of the session */ void *data; /**< arbitrary user data */ uint8_t flags; /**< various flags of the session */ -#define NC_SESSION_SHAREDCTX 0x01 + +/* shared flags */ +#define NC_SESSION_SHAREDCTX 0x01 /**< context is shared */ #define NC_SESSION_CALLHOME 0x02 /**< session is Call Home and ch_lock is initialized */ #define NC_SESSION_CH_THREAD 0x04 /**< protected by ch_lock */ +/* client flags */ +#define NC_SESSION_CLIENT_NOT_STRICT 0x08 /**< some server modules failed to load so the data from + them will be ignored - not use strict flag for parsing */ +#define NC_SESSION_CLIENT_MONITORED 0x40 /**< session is being monitored by the client monitoring thread */ + +/* server flags */ +#ifdef NC_ENABLED_SSH_TLS +#define NC_SESSION_SSH_AUTHENTICATED 0x10 /**< SSH session authenticated */ +#define NC_SESSION_SSH_SUBSYS_NETCONF 0x20 /**< netconf subsystem requested */ +#endif + union { struct { /* client side only data */ @@ -771,11 +784,6 @@ struct nc_session { ATOMIC_T ntf_thread_count; /**< number of running notification threads */ ATOMIC_T ntf_thread_running; /**< flag whether there are notification threads for this session running or not */ struct lyd_node *ext_data; /**< LY ext data used in the context callback */ - - /* client flags */ - /* some server modules failed to load so the data from them will be ignored - not use strict flag for parsing */ -# define NC_SESSION_CLIENT_NOT_STRICT 0x08 -# define NC_SESSION_CLIENT_MONITORED 0x40 /**< session is being monitored by the client monitoring thread */ } client; struct { /* server side only data */ @@ -793,14 +801,8 @@ struct nc_session { pthread_mutex_t ch_lock; /**< Call Home thread lock */ pthread_cond_t ch_cond; /**< Call Home thread condition */ - /* server flags */ #ifdef NC_ENABLED_SSH_TLS - /* SSH session authenticated */ -# define NC_SESSION_SSH_AUTHENTICATED 0x10 - /* netconf subsystem requested */ -# define NC_SESSION_SSH_SUBSYS_NETCONF 0x20 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 */ } server; From bf7455b8787ad4aa1b7ce33212297ab8f7939a77 Mon Sep 17 00:00:00 2001 From: roman Date: Fri, 18 Oct 2024 09:51:04 +0200 Subject: [PATCH 9/9] tests UPDATE add client monitoring thread test --- tests/CMakeLists.txt | 1 + tests/test_client_monitoring.c | 200 +++++++++++++++++++++++++++++++++ 2 files changed, 201 insertions(+) create mode 100644 tests/test_client_monitoring.c diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 19e5d2d7..8fbe3ee5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -73,6 +73,7 @@ if(ENABLE_SSH_TLS) libnetconf2_test(NAME test_authkeys) libnetconf2_test(NAME test_cert_exp_notif) libnetconf2_test(NAME test_ch PORT_COUNT 2) + libnetconf2_test(NAME test_client_monitoring) libnetconf2_test(NAME test_endpt_share_clients PORT_COUNT 4) libnetconf2_test(NAME test_ks_ts) if (LIBPAM_HAVE_CONFDIR) diff --git a/tests/test_client_monitoring.c b/tests/test_client_monitoring.c new file mode 100644 index 00000000..92736a6c --- /dev/null +++ b/tests/test_client_monitoring.c @@ -0,0 +1,200 @@ +/** + * @file test_client_monitoring.c + * @author Roman Janota + * @brief libnetconf2 client monitoring thread test + * + * @copyright + * Copyright (c) 2024 CESNET, z.s.p.o. + * + * This source code is licensed under BSD 3-Clause License (the "License"). + * You may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://opensource.org/licenses/BSD-3-Clause + */ + +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "ln2_test.h" +#include "session_p.h" + +#include + +int TEST_PORT = 10050; +const char *TEST_PORT_STR = "10050"; + +void +monitoring_clb(struct nc_session *sess, void *user_data) +{ + pthread_barrier_t *barrier = user_data; + + /* signal the main thread that the monitoring callback was called */ + pthread_barrier_wait(barrier); + printf("Session with ID %d disconnected by the server.\n", nc_session_get_id(sess)); +} + +static void * +client_thread(void *arg) +{ + int ret; + struct nc_session *session = NULL; + struct ln2_test_ctx *test_ctx = arg; + pthread_barrier_t monitoring_barrier; + + /* initialize the barrier */ + ret = pthread_barrier_init(&monitoring_barrier, NULL, 2); + assert_int_equal(ret, 0); + + /* start the monitoring thread */ + ret = nc_client_monitoring_thread_start(monitoring_clb, &monitoring_barrier, NULL); + assert_int_equal(ret, 0); + + /* skip all hostkey and known_hosts checks */ + nc_client_ssh_set_knownhosts_mode(NC_SSH_KNOWNHOSTS_SKIP); + + /* set the search path for the schemas */ + ret = nc_client_set_schema_searchpath(MODULES_DIR); + assert_int_equal(ret, 0); + + /* set the client's username */ + ret = nc_client_ssh_set_username("test_client_monitoring"); + assert_int_equal(ret, 0); + + /* add the client's key pair */ + ret = nc_client_ssh_add_keypair(TESTS_DIR "/data/key_rsa.pub", TESTS_DIR "/data/key_rsa"); + assert_int_equal(ret, 0); + + /* wait for the server to be ready and connect */ + pthread_barrier_wait(&test_ctx->barrier); + session = nc_connect_ssh("127.0.0.1", TEST_PORT, NULL); + assert_non_null(session); + + /* wait for the monitoring thread callback to be called */ + pthread_barrier_wait(&monitoring_barrier); + + /* stop the monitoring thread */ + nc_client_monitoring_thread_stop(); + + pthread_barrier_destroy(&monitoring_barrier); + return NULL; +} + +void * +server_thread(void *arg) +{ + int ret; + NC_MSG_TYPE msgtype; + struct nc_session *session = NULL; + struct nc_pollsession *ps = NULL; + struct ln2_test_ctx *test_ctx = arg; + int fd; + struct linger ling = {1, 0}; + + ps = nc_ps_new(); + assert_non_null(ps); + + /* wait for the client to be ready to connect */ + pthread_barrier_wait(&test_ctx->barrier); + + /* accept a session and add it to the poll session structure */ + msgtype = nc_accept(NC_ACCEPT_TIMEOUT, test_ctx->ctx, &session); + assert_int_equal(msgtype, NC_MSG_HELLO); + + /* get the session's fd */ + fd = ssh_get_fd(session->ti.libssh.session); + assert_int_not_equal(fd, -1); + + /* set the socket to close immediately */ + ret = setsockopt(fd, SOL_SOCKET, SO_LINGER, &ling, sizeof(ling)); + assert_int_equal(ret, 0); + + /* add the session to the poll session */ + ret = nc_ps_add_session(ps, session); + assert_int_equal(ret, 0); + + /* poll until the client stops sending messages */ + do { + ret = nc_ps_poll(ps, NC_PS_POLL_TIMEOUT, NULL); + } while ((ret & NC_PSPOLL_RPC)); + + /* free the session (it will close the socket -> client needs to detect this) */ + nc_ps_clear(ps, 1, NULL); + nc_ps_free(ps); + return NULL; +} + +static void +test_nc_client_monitoring(void **state) +{ + int ret, i; + pthread_t tids[2]; + + ret = pthread_create(&tids[0], NULL, client_thread, *state); + assert_int_equal(ret, 0); + ret = pthread_create(&tids[1], NULL, server_thread, *state); + assert_int_equal(ret, 0); + + for (i = 0; i < 2; i++) { + pthread_join(tids[i], NULL); + } +} + +static int +setup(void **state) +{ + int ret; + struct lyd_node *tree = NULL; + struct ln2_test_ctx *test_ctx; + + /* global setup */ + ret = ln2_glob_test_setup(&test_ctx); + assert_int_equal(ret, 0); + + *state = test_ctx; + + /* add endpoint */ + ret = nc_server_config_add_address_port(test_ctx->ctx, "endpt", NC_TI_SSH, "127.0.0.1", TEST_PORT, &tree); + assert_int_equal(ret, 0); + + /* add hostkey */ + ret = nc_server_config_add_ssh_hostkey(test_ctx->ctx, "endpt", "hostkey", TESTS_DIR "/data/key_ecdsa", NULL, &tree); + assert_int_equal(ret, 0); + + /* add the test client */ + ret = nc_server_config_add_ssh_user_pubkey(test_ctx->ctx, "endpt", "test_client_monitoring", "pubkey", TESTS_DIR "/data/key_rsa.pub", &tree); + assert_int_equal(ret, 0); + + /* configure the server based on the data */ + ret = nc_server_config_setup_data(tree); + assert_int_equal(ret, 0); + + lyd_free_all(tree); + + return 0; +} + +int +main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown(test_nc_client_monitoring, setup, ln2_glob_test_teardown) + }; + + /* try to get ports from the environment, otherwise use the default */ + if (ln2_glob_test_get_ports(1, &TEST_PORT, &TEST_PORT_STR)) { + return 1; + } + + setenv("CMOCKA_TEST_ABORT", "1", 1); + return cmocka_run_group_tests(tests, NULL, NULL); +}