diff --git a/compat/compat.h.in b/compat/compat.h.in index 597c4b63..cbea52af 100644 --- a/compat/compat.h.in +++ b/compat/compat.h.in @@ -122,6 +122,7 @@ # define ATOMIC_INC_RELAXED(var) atomic_fetch_add_explicit(&(var), 1, memory_order_relaxed) # define ATOMIC_ADD_RELAXED(var, x) atomic_fetch_add_explicit(&(var), x, memory_order_relaxed) # define ATOMIC_DEC_RELAXED(var) atomic_fetch_sub_explicit(&(var), 1, memory_order_relaxed) +# define ATOMIC_DEC_ACQ_REL(var) atomic_fetch_sub_explicit(&(var), 1, memory_order_acq_rel) # define ATOMIC_SUB_RELAXED(var, x) atomic_fetch_sub_explicit(&(var), x, memory_order_relaxed) # define ATOMIC_PTR_COMPARE_EXCHANGE_RELAXED(var, exp, des, result) \ @@ -144,6 +145,8 @@ # define ATOMIC_INC_RELAXED(var) __sync_fetch_and_add(&(var), 1) # define ATOMIC_ADD_RELAXED(var, x) __sync_fetch_and_add(&(var), x) # define ATOMIC_DEC_RELAXED(var) __sync_fetch_and_sub(&(var), 1) +/* __sync_fetch_and_sub() is already a full barrier */ +# define ATOMIC_DEC_ACQ_REL(var) __sync_fetch_and_sub(&(var), 1) # define ATOMIC_SUB_RELAXED(var, x) __sync_fetch_and_sub(&(var), x) # define ATOMIC_PTR_COMPARE_EXCHANGE_RELAXED(var, exp, des, result) \ diff --git a/src/server_config.c b/src/server_config.c index 33b0f488..66e96982 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -502,18 +502,20 @@ nc_server_config_truststore_free(struct nc_truststore *ts) #endif /* NC_ENABLED_SSH_TLS */ /** - * @brief Free server configuration data. + * @brief Free the data of a server configuration generation. + * + * @note Never call this directly on a published generation, dropping the last reference with + * ::nc_server_config_release() is the only way a generation may be freed. * * @param[in] config Server configuration to free. */ -void +static void nc_server_config_free(struct nc_server_config *config) { struct nc_endpt *endpt; struct nc_ch_client *ch_client; struct nc_ch_endpt *ch_endpt; LY_ARRAY_COUNT_TYPE i = 0, j = 0; - char *socket_path = NULL; if (!config) { return; @@ -529,31 +531,14 @@ nc_server_config_free(struct nc_server_config *config) LY_ARRAY_FOR(config->endpts, i) { endpt = &config->endpts[i]; - if (endpt->ti == NC_TI_UNIX) { - /* get the socket path before freeing the name */ - socket_path = nc_server_unix_get_socket_path(endpt); - } - free(endpt->name); - /* free binds */ + /* free binds, the listening sockets are owned by the bind registry */ LY_ARRAY_FOR(endpt->binds, j) { - if (endpt->binds[j].sock != -1) { - close(endpt->binds[j].sock); - if (socket_path) { - /* remove the UNIX socket file */ - unlink(socket_path); - } - } free(endpt->binds[j].address); } LY_ARRAY_FREE(endpt->binds); - if (endpt->ti == NC_TI_UNIX) { - free(socket_path); - socket_path = NULL; - } - /* free transport specific options */ switch (endpt->ti) { #ifdef NC_ENABLED_SSH_TLS @@ -619,6 +604,46 @@ nc_server_config_free(struct nc_server_config *config) memset(config, 0, sizeof(*config)); } +const struct nc_server_config * +nc_server_config_acquire(void) +{ + struct nc_server_config *config; + + /* CONFIG READ LOCK - only the pointer read and the refcount increment */ + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return NULL; + } + + config = server_opts.config; + if (config) { + /* the read lock provides the ordering, no new reference to a swapped out generation + * can ever be taken because only server_opts.config is ever read here */ + ATOMIC_INC_RELAXED(config->refcount); + } + + /* CONFIG READ UNLOCK */ + nc_rwlock_unlock(&server_opts.config_lock, __func__); + return config; +} + +void +nc_server_config_release(const struct nc_server_config *config) +{ + struct nc_server_config *cfg = (struct nc_server_config *)config; + + if (!cfg) { + return; + } + + /* acq_rel so that all the reads of this generation are ordered before the free() done + * by whoever drops the last reference */ + if (ATOMIC_DEC_ACQ_REL(cfg->refcount) == 1) { + /* we held the last reference */ + nc_server_config_free(cfg); + free(cfg); + } +} + API int nc_server_config_load_modules(struct ly_ctx **ctx) { @@ -782,8 +807,6 @@ config_local_bind(const struct lyd_node *node, enum nc_operation parent_op, stru } else if (op == NC_OP_CREATE) { /* create a new bind */ LY_ARRAY_NEW_RET(LYD_CTX(node), endpt->binds, bind, 1); - /* init the new bind */ - bind->sock = -1; } else { ERR(NULL, "Unsupported operation of node \"%s\".", LYD_NAME(node)); return 1; @@ -3141,7 +3164,7 @@ config_unix_socket_path(const struct lyd_node *node, enum nc_operation parent_op if (op == NC_OP_DELETE) { /* the endpoint must have a single binding, so we can just free it, - * the socket will be closed in ::nc_server_config_free() */ + * the listening socket is closed by the bind registry */ if (!endpt->binds) { ERR(NULL, "No UNIX socket path binding to delete."); return 1; @@ -3160,7 +3183,6 @@ config_unix_socket_path(const struct lyd_node *node, enum nc_operation parent_op LY_ARRAY_NEW_RET(LYD_CTX(node), endpt->binds, bind, 1); bind->address = strdup(lyd_get_value(node)); NC_CHECK_ERRMEM_RET(!bind->address, 1); - bind->sock = -1; /* also set the cleartext path flag */ opts->path_type = NC_UNIX_SOCKET_PATH_FILE; @@ -3182,7 +3204,7 @@ config_unix_hidden_path(const struct lyd_node *node, enum nc_operation parent_op if (op == NC_OP_DELETE) { /* the endpoint must have a single binding, so we can just free it, - * the socket will be closed in ::nc_server_config_free() */ + * the listening socket is closed by the bind registry */ if (!endpt->binds) { ERR(NULL, "No UNIX socket hidden path binding to delete."); return 1; @@ -3199,7 +3221,6 @@ config_unix_hidden_path(const struct lyd_node *node, enum nc_operation parent_op return 1; } LY_ARRAY_NEW_RET(LYD_CTX(node), endpt->binds, bind, 1); - bind->sock = -1; /* also set the hidden path flag */ opts->path_type = NC_UNIX_SOCKET_PATH_HIDDEN; @@ -5396,325 +5417,8 @@ nc_server_config_libnetconf2_netconf_server(const struct lyd_node *tree, int is_ return rc; } -/** - * @brief Check if two server endpoint bindings match. - * - * They match if they use the same transport protocol, address and port. - * - * @param[in] e1 First server endpoint. - * @param[in] b1 First server endpoint binding. - * @param[in] e2 Second server endpoint. - * @param[in] b2 Second server endpoint binding. - * @return 1 if they match, 0 otherwise. - */ -static int -nc_server_config_bindings_match(const struct nc_endpt *e1, const struct nc_bind *b1, - const struct nc_endpt *e2, const struct nc_bind *b2) -{ - int rc = 1; - char *addr1 = NULL, *addr2 = NULL; - - if (e1->ti != e2->ti) { - /* different transport protocols */ - return 0; - } - - if (e1->ti == NC_TI_UNIX) { - /* UNIX sockets may have hidden or cleartext addresses */ - addr1 = nc_server_unix_get_socket_path(e1); - addr2 = nc_server_unix_get_socket_path(e2); - } else { - addr1 = b1->address; - addr2 = b2->address; - } - if (!addr1 || !addr2) { - /* unable to get the address */ - rc = 0; - goto cleanup; - } - - if (strcmp(addr1, addr2) || (b1->port != b2->port)) { - /* different addresses or ports */ - rc = 0; - goto cleanup; - } - -cleanup: - if (e1->ti == NC_TI_UNIX) { - free(addr1); - free(addr2); - } - return rc; -} - -/** - * @brief Atomically starts listening on new sockets and reuses existing ones. - * - * @param[in,out] old_cfg Old, currently active server configuration. - * @param[in,out] new_cfg New server configuration currently being applied. - * @return 0 on success, 1 on error. - */ -static int -nc_server_config_reconcile_sockets_listen(struct nc_server_config *old_cfg, - struct nc_server_config *new_cfg) -{ - int rc = 0, found; - struct nc_endpt *old_endpt, *new_endpt; - struct nc_bind *new_bind, *old_bind; - - /* - * == PHASE 1: RECONCILE OLD AND NEW SOCKETS == - * Match existing sockets from old_cfg to new_cfg to reuse them, - * then create new sockets for new binds. - */ - - /* reuse existing sockets from old_cfg */ - LY_ARRAY_FOR(new_cfg->endpts, struct nc_endpt, new_endpt) { - LY_ARRAY_FOR(new_endpt->binds, struct nc_bind, new_bind) { - found = 0; - LY_ARRAY_FOR(old_cfg->endpts, struct nc_endpt, old_endpt) { - LY_ARRAY_FOR(old_endpt->binds, struct nc_bind, old_bind) { - if (nc_server_config_bindings_match(new_endpt, new_bind, old_endpt, old_bind)) { - /* match found, reuse the socket */ - new_bind->sock = old_bind->sock; - found = 1; - break; - } - } - if (found) { - /* break the outer loop as well, we already found a match for this bind */ - break; - } - } - } - } - - /* create new sockets for new binds */ - LY_ARRAY_FOR(new_cfg->endpts, struct nc_endpt, new_endpt) { - LY_ARRAY_FOR(new_endpt->binds, struct nc_bind, new_bind) { - if (new_bind->sock == -1) { - /* this bind is new, create a listening socket */ - if (nc_server_bind_and_listen(new_endpt, new_bind)) { - /* FAILURE! trigger rollback */ - rc = 1; - goto rollback; - } - } - } - } - - /* - * == PHASE 2: COMMIT CHANGES (WRITE TO old_cfg) == - * new_cfg is now fully valid. We can safely modify old_cfg to prevent - * reused sockets from being closed by the caller. - */ - LY_ARRAY_FOR(old_cfg->endpts, struct nc_endpt, old_endpt) { - LY_ARRAY_FOR(old_endpt->binds, struct nc_bind, old_bind) { - found = 0; - if (old_bind->sock == -1) { - /* already handled or was never active */ - continue; - } - - /* check if this old_bind's socket was reused in the new_cfg */ - LY_ARRAY_FOR(new_cfg->endpts, struct nc_endpt, new_endpt) { - LY_ARRAY_FOR(new_endpt->binds, struct nc_bind, new_bind) { - if (old_bind->sock == new_bind->sock) { - /* match found, invalidate the socket in the old config (dont want to close it) */ - old_bind->sock = -1; - found = 1; - break; - } - } - if (found) { - /* break the outer loop as well, we already found a match for this bind */ - break; - } - } - } - } - - return 0; - -rollback: - /* - * == ROLLBACK LOGIC == - * An error occurred. We do not want to close the reused sockets, so we can roll back to old_cfg. - * So we invalidate all reused sockets in new_cfg, the rest will be closed by the caller later. - */ - LY_ARRAY_FOR(new_cfg->endpts, struct nc_endpt, new_endpt) { - LY_ARRAY_FOR(new_endpt->binds, struct nc_bind, new_bind) { - found = 0; - if (new_bind->sock == -1) { - /* this bind was never assigned a socket */ - continue; - } - - /* was this socket reused from the old config? */ - LY_ARRAY_FOR(old_cfg->endpts, struct nc_endpt, old_endpt) { - LY_ARRAY_FOR(old_endpt->binds, struct nc_bind, old_bind) { - if (new_bind->sock == old_bind->sock) { - /* match found, invalidate the socket in the new config */ - new_bind->sock = -1; - found = 1; - break; - } - } - if (found) { - /* break the outer loop as well, we already found a match for this bind */ - break; - } - } - } - } - - return rc; -} - #ifdef NC_ENABLED_SSH_TLS -/** - * @brief Check if there are any new Call Home clients created in the new configuration. - * - * @param[in] old_cfg Old, currently active server configuration. - * @param[in] new_cfg New server configuration currently being applied. - * @return 1 if there are new CH clients, 0 otherwise. - */ -static int -nc_server_config_new_ch_clients_created(struct nc_server_config *old_cfg, struct nc_server_config *new_cfg) -{ - struct nc_ch_client *old_ch_client, *new_ch_client; - int found; - - /* check if there are any new clients */ - LY_ARRAY_FOR(new_cfg->ch_clients, struct nc_ch_client, new_ch_client) { - found = 0; - LY_ARRAY_FOR(old_cfg->ch_clients, struct nc_ch_client, old_ch_client) { - if (!strcmp(new_ch_client->name, old_ch_client->name)) { - found = 1; - break; - } - } - if (!found) { - return 1; - } - } - - /* no differences found */ - return 0; -} - -/** - * @brief Atomically dispatch new Call Home clients and reuse existing ones. - * - * @param[in,out] old_cfg Old, currently active server configuration. - * @param[in,out] new_cfg New server configuration currently being applied. - * @return 0 on success, 1 on error. - */ -static int -nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg, - struct nc_server_config *new_cfg) -{ - int rc = 0; - struct nc_ch_client *old_ch_client, *new_ch_client; - int found; - LY_ARRAY_COUNT_TYPE i; - struct nc_ch_client **started_clients = NULL, **started_client_ptr; - int dispatch_new_clients = 1; - - if (!server_opts.ch_dispatch_data.acquire_ctx_cb || !server_opts.ch_dispatch_data.release_ctx_cb || - !server_opts.ch_dispatch_data.new_session_cb) { - /* Call Home dispatch callbacks not set, we can't dispatch new clients, but we can still stop deleted ones */ - if (nc_server_config_new_ch_clients_created(old_cfg, new_cfg)) { - WRN(NULL, "New Call Home clients were created but Call Home dispatch callbacks are not set - " - "new clients will not be dispatched automatically."); - } - dispatch_new_clients = 0; - } - - /* - * == PHASE 1: START NEW CLIENTS == - * Start clients present in new_cfg that are not already running. - * Track successfully started threads for potential rollback. - */ - if (dispatch_new_clients) { - /* only dispatch if all required CBs are set */ - LY_ARRAY_FOR(new_cfg->ch_clients, struct nc_ch_client, new_ch_client) { - if (!new_ch_client->thread) { - /* the new config may have been built from scratch (::nc_server_config_setup_data()), in which - * case the thread data of an already running client is only present in the old config */ - LY_ARRAY_FOR(old_cfg->ch_clients, struct nc_ch_client, old_ch_client) { - if (!strcmp(old_ch_client->name, new_ch_client->name)) { - new_ch_client->thread = old_ch_client->thread; - break; - } - } - } - - if (new_ch_client->thread) { - /* already running */ - continue; - } - - /* this is a new Call Home client, dispatch it */ - rc = _nc_connect_ch_client_dispatch(new_ch_client, server_opts.ch_dispatch_data.acquire_ctx_cb, - server_opts.ch_dispatch_data.release_ctx_cb, server_opts.ch_dispatch_data.ctx_cb_data, - server_opts.ch_dispatch_data.new_session_cb, server_opts.ch_dispatch_data.new_session_cb_data); - if (rc) { - /* FAILURE! trigger rollback */ - goto rollback; - } - - /* successfully started, track client for potential rollback */ - LY_ARRAY_NEW_GOTO(NULL, started_clients, started_client_ptr, rc, rollback); - *started_client_ptr = new_ch_client; - } - } - - /* - * == PHASE 2: STOP DELETED CLIENTS (COMMIT) == - * All new clients started successfully. Now stop old clients - * that are not present in the new configuration. - */ - LY_ARRAY_FOR(old_cfg->ch_clients, struct nc_ch_client, old_ch_client) { - found = 0; - LY_ARRAY_FOR(new_cfg->ch_clients, struct nc_ch_client, new_ch_client) { - if (!strcmp(old_ch_client->name, new_ch_client->name)) { - found = 1; - break; - } - } - - if (!found && old_ch_client->thread) { - /* this Call Home client was deleted, notify it to stop */ - if ((rc = nc_session_server_ch_client_dispatch_stop(old_ch_client))) { - ERR(NULL, "Failed to dispatch stop for Call Home client \"%s\".", old_ch_client->name); - goto rollback; - } - } - } - - /* success */ - rc = 0; - goto cleanup; - -rollback: - /* - * == ROLLBACK LOGIC == - * An error occurred during PHASE 1. Stop any new threads we *just* started - * to return to the pre-call state. - */ - LY_ARRAY_FOR(started_clients, i) { - nc_session_server_ch_client_dispatch_stop(started_clients[i]); - } - /* rc is already set to non-zero from the failure point */ - -cleanup: - /* free the tracking list */ - LY_ARRAY_FREE(started_clients); - return rc; -} - /** * @brief Create a deep copy of the SSH server options. * @@ -6191,6 +5895,8 @@ nc_server_config_truststore_dup(const struct nc_truststore *src, struct nc_trust /** * @brief Create a deep copy of the server configuration. * + * @note On error, @p dst is left partially filled, freeing it is up to its owner. + * * @param[in] src Source server configuration to copy from. * @param[out] dst Server configuration copy. * @return 0 on success, 1 on error. @@ -6232,9 +5938,6 @@ nc_server_config_dup(const struct nc_server_config *src, struct nc_server_config NC_CHECK_ERRMEM_GOTO(!dst_endpt->binds[j].address, rc = 1, cleanup); } dst_endpt->binds[j].port = src_endpt->binds[j].port; - - /* mark the socket as uninitialized, it will be reassigned in ::nc_server_config_reconcile_sockets_listen() */ - dst_endpt->binds[j].sock = -1; LY_ARRAY_INCREMENT(dst_endpt->binds); } @@ -6326,8 +6029,6 @@ nc_server_config_dup(const struct nc_server_config *src, struct nc_server_config dst_ch_client->max_attempts = src_ch_client->max_attempts; dst_ch_client->max_wait = src_ch_client->max_wait; - dst_ch_client->thread = src_ch_client->thread; - LY_ARRAY_INCREMENT(dst->ch_clients); } @@ -6350,10 +6051,6 @@ nc_server_config_dup(const struct nc_server_config *src, struct nc_server_config #endif /* NC_ENABLED_SSH_TLS */ cleanup: - if (rc) { - nc_server_config_free(dst); - } - return rc; } @@ -6376,11 +6073,51 @@ nc_server_config_cert_exp_notif_thread_wakeup(void) #endif /* NC_ENABLED_SSH_TLS */ +/** + * @brief Allocate a new server configuration generation. + * + * @param[out] config New generation with a single reference held by the caller. + * @return 0 on success, 1 on error. + */ +static int +nc_server_config_new(struct nc_server_config **config) +{ + *config = calloc(1, sizeof **config); + NC_CHECK_ERRMEM_RET(!*config, 1); + + /* the applier's reference, transferred to server_opts.config once the generation is published */ + ATOMIC_STORE_RELAXED((*config)->refcount, 1); + return 0; +} + +/** + * @brief Publish a new server configuration generation and drop the reference of the old one. + * + * @note The configuration WRITE lock must be held. + * + * @param[in] config New generation to publish, its reference is transferred to ::nc_server_opts.config. + * @return Old generation, the caller must release it once the lock is released. + */ +static struct nc_server_config * +nc_server_config_publish(struct nc_server_config *config) +{ + struct nc_server_config *old_config; + + old_config = server_opts.config; + server_opts.config = config; + + /* mirror the idle timeout so that the hello and poll paths do not need the config at all */ + ATOMIC_STORE_RELAXED(server_opts.idle_timeout, config->idle_timeout); + + return old_config; +} + API int nc_server_config_setup_diff(const struct lyd_node *data) { int ret = 0; - struct nc_server_config config_copy = {0}; + const struct nc_server_config *cur_config = NULL; + struct nc_server_config *config_copy = NULL, *old_config = NULL; NC_CHECK_ARG_RET(NULL, data, 1); @@ -6388,81 +6125,94 @@ nc_server_config_setup_diff(const struct lyd_node *data) * - avoids concurrent updates * - readers are still allowed to read the old config while we are applying the new one */ - if (nc_mutex_lock(&server_opts.config_update_lock, NC_CONFIG_APPLY_LOCK_TIMEOUT, __func__) != 1) { + if (nc_mutex_lock(&server_opts.config_update_lock, NC_CONFIG_UPDATE_LOCK_TIMEOUT, __func__) != 1) { ERR(NULL, "Timed out waiting for another configuration update to finish, " "the new configuration was not applied."); return 1; } - /* CONFIG RD LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_APPLY_LOCK_TIMEOUT, __func__) != 1) { - ERR(NULL, "Timed out waiting for the configuration lock, the new configuration was not applied."); - ret = 1; - goto cleanup; - } + NC_CHECK_GOTO(ret = nc_server_config_new(&config_copy), cleanup); /* create a copy of the current config to work with, so that we can revert to it in case of error */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_dup(&server_opts.config, &config_copy), - ERR(NULL, "Duplicating current server configuration failed."), cleanup_unlock); + cur_config = nc_server_config_acquire(); + NC_CHECK_ERR_GOTO(!cur_config, ERR(NULL, "Acquiring the current server configuration failed."); ret = 1, cleanup); - /* UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + NC_CHECK_ERR_GOTO(ret = nc_server_config_dup(cur_config, config_copy), + ERR(NULL, "Duplicating current server configuration failed."), cleanup); + + nc_server_config_release(cur_config); + cur_config = NULL; #ifdef NC_ENABLED_SSH_TLS /* configure keystore */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_keystore(data, 1, &config_copy), + NC_CHECK_ERR_GOTO(ret = nc_server_config_keystore(data, 1, config_copy), ERR(NULL, "Applying ietf-keystore configuration failed."), cleanup); /* configure truststore */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_truststore(data, 1, &config_copy), + NC_CHECK_ERR_GOTO(ret = nc_server_config_truststore(data, 1, config_copy), ERR(NULL, "Applying ietf-truststore configuration failed."), cleanup); #endif /* NC_ENABLED_SSH_TLS */ /* configure netconf-server */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_netconf_server(data, 1, &config_copy), + NC_CHECK_ERR_GOTO(ret = nc_server_config_netconf_server(data, 1, config_copy), ERR(NULL, "Applying ietf-netconf-server configuration failed."), cleanup); /* configure libnetconf2-netconf-server */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_libnetconf2_netconf_server(data, NC_OP_UNKNOWN, &config_copy), + NC_CHECK_ERR_GOTO(ret = nc_server_config_libnetconf2_netconf_server(data, NC_OP_UNKNOWN, config_copy), ERR(NULL, "Applying libnetconf2-netconf-server configuration failed."), cleanup); - /* CONFIG WR LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_APPLY_LOCK_TIMEOUT, __func__) != 1) { + /* start listening on new endpoints */ + NC_CHECK_ERR_GOTO(ret = nc_server_binds_reconcile(config_copy), + ERR(NULL, "Starting to listen on new endpoints failed."), cleanup); + +#ifdef NC_ENABLED_SSH_TLS + /* dispatch new call-home threads, the listening sockets are already reconciled with the new + * generation so a failure here has to be rolled back */ + NC_CHECK_ERR_GOTO(ret = nc_server_ch_clients_reconcile(config_copy), + ERR(NULL, "Dispatching new call-home threads failed."), rollback); +#endif /* NC_ENABLED_SSH_TLS */ + + /* CONFIG WR LOCK - only the pointer swap */ + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { ERR(NULL, "Timed out waiting for the configuration lock, the new configuration was not applied."); ret = 1; - goto cleanup; + goto rollback; } - /* start listening on new endpoints */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_reconcile_sockets_listen(&server_opts.config, &config_copy), - ERR(NULL, "Starting to listen on new endpoints failed."), cleanup_unlock); + /* publish the new generation, the reference is transferred to server_opts.config */ + old_config = nc_server_config_publish(config_copy); + config_copy = NULL; -#ifdef NC_ENABLED_SSH_TLS - /* dispatch new call-home threads */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_reconcile_chclients_dispatch(&server_opts.config, &config_copy), - ERR(NULL, "Dispatching new call-home threads failed."), cleanup_unlock); -#endif /* NC_ENABLED_SSH_TLS */ + /* CONFIG UNLOCK */ + nc_rwlock_unlock(&server_opts.config_lock, __func__); - /* swap: free old, keep new, zero out the copy just in case to avoid double free */ - nc_server_config_free(&server_opts.config); - server_opts.config = config_copy; - memset(&config_copy, 0, sizeof config_copy); + /* the old generation is freed once its last reader releases it */ + nc_server_config_release(old_config); #ifdef NC_ENABLED_SSH_TLS /* wake up the cert expiration notif thread */ nc_server_config_cert_exp_notif_thread_wakeup(); #endif /* NC_ENABLED_SSH_TLS */ -cleanup_unlock: - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + goto cleanup; -cleanup: - if (ret) { - /* free the new config in case of error */ - nc_server_config_free(&config_copy); +rollback: + /* the sockets and the Call Home threads were already reconciled with the new generation, + * reconcile them back with the one that stays published */ + cur_config = nc_server_config_acquire(); + if (cur_config) { + nc_server_binds_reconcile(cur_config); +#ifdef NC_ENABLED_SSH_TLS + nc_server_ch_clients_reconcile(cur_config); +#endif /* NC_ENABLED_SSH_TLS */ } +cleanup: + nc_server_config_release(cur_config); + + /* release the new generation, it was either not published or it is NULL */ + nc_server_config_release(config_copy); + /* CONFIG UPDATE UNLOCK */ nc_mutex_unlock(&server_opts.config_update_lock, __func__); return ret; @@ -6473,7 +6223,8 @@ nc_server_config_setup_data(const struct lyd_node *data) { int ret = 0; const struct lyd_node *tree, *iter; - struct nc_server_config config = {0}; + const struct nc_server_config *cur_config = NULL; + struct nc_server_config *config = NULL, *old_config = NULL; NC_CHECK_ARG_RET(NULL, data, 1); @@ -6481,7 +6232,7 @@ nc_server_config_setup_data(const struct lyd_node *data) * - avoids concurrent updates * - readers are still allowed to read the old config while we are applying the new one */ - if (nc_mutex_lock(&server_opts.config_update_lock, NC_CONFIG_APPLY_LOCK_TIMEOUT, __func__) != 1) { + if (nc_mutex_lock(&server_opts.config_update_lock, NC_CONFIG_UPDATE_LOCK_TIMEOUT, __func__) != 1) { ERR(NULL, "Timed out waiting for another configuration update to finish, " "the new configuration was not applied."); return 1; @@ -6503,62 +6254,78 @@ nc_server_config_setup_data(const struct lyd_node *data) * - if something fails, the old config is still intact * - not having to hold the config_lock for a long time while applying the new config */ + NC_CHECK_GOTO(ret = nc_server_config_new(&config), cleanup); #ifdef NC_ENABLED_SSH_TLS /* configure keystore */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_keystore(data, 0, &config), + NC_CHECK_ERR_GOTO(ret = nc_server_config_keystore(data, 0, config), ERR(NULL, "Applying ietf-keystore configuration failed."), cleanup); /* configure truststore */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_truststore(data, 0, &config), + NC_CHECK_ERR_GOTO(ret = nc_server_config_truststore(data, 0, config), ERR(NULL, "Applying ietf-truststore configuration failed."), cleanup); #endif /* NC_ENABLED_SSH_TLS */ /* configure netconf-server */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_netconf_server(data, 0, &config), + NC_CHECK_ERR_GOTO(ret = nc_server_config_netconf_server(data, 0, config), ERR(NULL, "Applying ietf-netconf-server configuration failed."), cleanup); /* configure libnetconf2-netconf-server */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_libnetconf2_netconf_server(data, NC_OP_UNKNOWN, &config), + NC_CHECK_ERR_GOTO(ret = nc_server_config_libnetconf2_netconf_server(data, NC_OP_UNKNOWN, config), ERR(NULL, "Applying libnetconf2-netconf-server configuration failed."), cleanup); - /* CONFIG LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_APPLY_LOCK_TIMEOUT, __func__) != 1) { + /* start listening on new endpoints */ + NC_CHECK_ERR_GOTO(ret = nc_server_binds_reconcile(config), + ERR(NULL, "Starting to listen on new endpoints failed."), cleanup); + +#ifdef NC_ENABLED_SSH_TLS + /* dispatch new call-home connections, the listening sockets are already reconciled with the new + * generation so a failure here has to be rolled back */ + NC_CHECK_ERR_GOTO(ret = nc_server_ch_clients_reconcile(config), + ERR(NULL, "Dispatching new call-home connections failed."), rollback); +#endif /* NC_ENABLED_SSH_TLS */ + + /* CONFIG WR LOCK - only the pointer swap */ + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { ERR(NULL, "Timed out waiting for the configuration lock, the new configuration was not applied."); ret = 1; - goto cleanup; + goto rollback; } - /* start listening on new endpoints */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_reconcile_sockets_listen(&server_opts.config, &config), - ERR(NULL, "Starting to listen on new endpoints failed."), cleanup_unlock); + /* publish the new generation, the reference is transferred to server_opts.config */ + old_config = nc_server_config_publish(config); + config = NULL; -#ifdef NC_ENABLED_SSH_TLS - /* dispatch new call-home connections */ - NC_CHECK_ERR_GOTO(ret = nc_server_config_reconcile_chclients_dispatch(&server_opts.config, &config), - ERR(NULL, "Dispatching new call-home connections failed."), cleanup_unlock); -#endif /* NC_ENABLED_SSH_TLS */ + /* CONFIG UNLOCK */ + nc_rwlock_unlock(&server_opts.config_lock, __func__); - /* swap: free old, keep new, zero out the copy just in case to avoid double free */ - nc_server_config_free(&server_opts.config); - server_opts.config = config; - memset(&config, 0, sizeof config); + /* the old generation is freed once its last reader releases it */ + nc_server_config_release(old_config); #ifdef NC_ENABLED_SSH_TLS /* wake up the cert expiration notif thread */ nc_server_config_cert_exp_notif_thread_wakeup(); #endif /* NC_ENABLED_SSH_TLS */ -cleanup_unlock: - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + goto cleanup; -cleanup: - if (ret) { - /* free the new config in case of error */ - nc_server_config_free(&config); +rollback: + /* the sockets and the Call Home threads were already reconciled with the new generation, + * reconcile them back with the one that stays published */ + cur_config = nc_server_config_acquire(); + if (cur_config) { + nc_server_binds_reconcile(cur_config); +#ifdef NC_ENABLED_SSH_TLS + nc_server_ch_clients_reconcile(cur_config); +#endif /* NC_ENABLED_SSH_TLS */ } +cleanup: + nc_server_config_release(cur_config); + + /* release the new generation, it was either not published or it is NULL */ + nc_server_config_release(config); + /* CONFIG UPDATE UNLOCK */ nc_mutex_unlock(&server_opts.config_update_lock, __func__); return ret; @@ -6803,27 +6570,28 @@ nc_server_config_oper_get_user_password_last_modified(const char *ch_client, con const char *username, time_t *last_modified) { int rc = 0; - LY_ARRAY_COUNT_TYPE i = 0; + LY_ARRAY_COUNT_TYPE i = 0, u; + const struct nc_server_config *config; struct nc_server_ssh_opts *ssh_opts = NULL; - struct nc_endpt *endpt = NULL; - struct nc_ch_client *client = NULL; - struct nc_ch_endpt *ch_endpt = NULL; + const struct nc_endpt *endpt = NULL; + const struct nc_ch_client *client = NULL; + const struct nc_ch_endpt *ch_endpt = NULL; time_t found_time = 0; NC_CHECK_ARG_RET(NULL, endpoint, username, last_modified, 1); *last_modified = 0; - /* LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + config = nc_server_config_acquire(); + if (!config) { return 1; } if (ch_client) { /* find the call-home client */ - LY_ARRAY_FOR(server_opts.config.ch_clients, i) { - if (!strcmp(server_opts.config.ch_clients[i].name, ch_client)) { - client = &server_opts.config.ch_clients[i]; + LY_ARRAY_FOR(config->ch_clients, u) { + if (!strcmp(config->ch_clients[u].name, ch_client)) { + client = &config->ch_clients[u]; break; } } @@ -6834,7 +6602,8 @@ nc_server_config_oper_get_user_password_last_modified(const char *ch_client, con } /* find the endpoint */ - LY_ARRAY_FOR(client->ch_endpts, struct nc_ch_endpt, ch_endpt) { + LY_ARRAY_FOR(client->ch_endpts, u) { + ch_endpt = &client->ch_endpts[u]; if (!strcmp(ch_endpt->name, endpoint) && (ch_endpt->ti == NC_TI_SSH)) { ssh_opts = ch_endpt->opts.ssh; break; @@ -6848,7 +6617,8 @@ nc_server_config_oper_get_user_password_last_modified(const char *ch_client, con } } else { /* no call-home client specified, search in listening endpoints */ - LY_ARRAY_FOR(server_opts.config.endpts, struct nc_endpt, endpt) { + LY_ARRAY_FOR(config->endpts, u) { + endpt = &config->endpts[u]; if (!strcmp(endpt->name, endpoint) && (endpt->ti == NC_TI_SSH)) { ssh_opts = endpt->opts.ssh; break; @@ -6878,8 +6648,7 @@ nc_server_config_oper_get_user_password_last_modified(const char *ch_client, con *last_modified = found_time; cleanup: - /* UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + nc_server_config_release(config); return rc; } diff --git a/src/session.c b/src/session.c index 4e5e13ef..4d779c81 100644 --- a/src/session.c +++ b/src/session.c @@ -1338,11 +1338,10 @@ nc_str_append(char **str, uint32_t *used, uint32_t *size, const char *app_format * * @param[in] ctx libyang context. * @param[in] version YANG version of the schemas to be included in result. - * @param[in] config_locked Whether the configuration lock is already held or should be acquired. * @return Array of capabilities terminated with NULL, NULL on error. */ static char ** -_nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version, int config_locked) +_nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version) { char **cpblts; const struct lys_module *mod; @@ -1351,9 +1350,14 @@ _nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version, int char *yl_content_id = NULL; uint32_t wd_also_supported, wd_basic_mode; char *str = NULL; + const struct nc_server_config *config; NC_CHECK_ARG_RET(NULL, ctx, NULL); + /* pin the configuration, only the ignored module names are needed from it, so if there is none + * (the server is not initialized) simply no module is ignored */ + config = nc_server_config_acquire(); + cpblts = malloc(3 * sizeof *cpblts); NC_CHECK_ERRMEM_GOTO(!cpblts, , error); cpblts[0] = strdup("urn:ietf:params:netconf:base:1.0"); @@ -1456,7 +1460,7 @@ _nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version, int /* models */ i = 0; while ((mod = ly_ctx_get_module_iter(ctx, &i))) { - if (nc_server_is_mod_ignored(mod->name, config_locked)) { + if (nc_server_is_mod_ignored(config, mod->name)) { /* ignored, not part of the cababilities */ continue; } @@ -1530,6 +1534,7 @@ _nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version, int /* HELLO UNLOCK */ nc_rwlock_unlock(&server_opts.hello_lock, __func__); + nc_server_config_release(config); free(str); return cpblts; @@ -1538,6 +1543,7 @@ _nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version, int nc_rwlock_unlock(&server_opts.hello_lock, __func__); error: + nc_server_config_release(config); if (cpblts) { for (i = 0; cpblts[i]; ++i) { free(cpblts[i]); @@ -1552,13 +1558,13 @@ _nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version, int API char ** nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version) { - return _nc_server_get_cpblts_version(ctx, version, 0); + return _nc_server_get_cpblts_version(ctx, version); } API char ** nc_server_get_cpblts(const struct ly_ctx *ctx) { - return _nc_server_get_cpblts_version(ctx, LYS_VERSION_UNDEF, 0); + return _nc_server_get_cpblts_version(ctx, LYS_VERSION_UNDEF); } /** @@ -1664,16 +1670,16 @@ nc_client_get_cpblts(void) * @brief Send NETCONF hello message on a session. * * @param[in] session Session to send the message on. - * @param[in] config_locked Whether the configuration READ lock is already held (only relevant for server side). * @return Sent message type. */ static NC_MSG_TYPE -nc_send_hello_io(struct nc_session *session, int config_locked) +nc_send_hello_io(struct nc_session *session) { NC_MSG_TYPE ret; int i, timeout_io; char **cpblts; uint32_t *sid; + uint16_t idle_timeout; if (session->side == NC_CLIENT) { /* client side hello - send only NETCONF base capabilities */ @@ -1685,7 +1691,7 @@ nc_send_hello_io(struct nc_session *session, int config_locked) timeout_io = NC_CLIENT_HELLO_TIMEOUT * 1000; sid = NULL; } else { - cpblts = _nc_server_get_cpblts_version(session->ctx, LYS_VERSION_1_0, config_locked); + cpblts = _nc_server_get_cpblts_version(session->ctx, LYS_VERSION_1_0); if (!cpblts) { return NC_MSG_ERROR; } @@ -1693,7 +1699,8 @@ nc_send_hello_io(struct nc_session *session, int config_locked) if (session->flags & NC_SESSION_CALLHOME) { timeout_io = NC_SERVER_CH_HELLO_TIMEOUT * 1000; } else { - timeout_io = server_opts.config.idle_timeout ? server_opts.config.idle_timeout * 1000 : -1; + idle_timeout = (uint16_t)ATOMIC_LOAD_RELAXED(server_opts.idle_timeout); + timeout_io = idle_timeout ? idle_timeout * 1000 : -1; } sid = &session->id; } @@ -1811,11 +1818,13 @@ nc_server_recv_hello_io(struct nc_session *session) struct lyd_node_opaq *node; NC_MSG_TYPE rc = NC_MSG_HELLO; int r, ver = -1, flag = 0, timeout_io; + uint16_t idle_timeout; if (session->flags & NC_SESSION_CALLHOME) { timeout_io = NC_SERVER_CH_HELLO_TIMEOUT * 1000; } else { - timeout_io = server_opts.config.idle_timeout ? server_opts.config.idle_timeout * 1000 : -1; + idle_timeout = (uint16_t)ATOMIC_LOAD_RELAXED(server_opts.idle_timeout); + timeout_io = idle_timeout ? idle_timeout * 1000 : -1; } r = nc_read_msg_poll_io(session, timeout_io, &msg); @@ -1875,7 +1884,7 @@ nc_handshake_io(struct nc_session *session) { NC_MSG_TYPE type; - type = nc_send_hello_io(session, 0); + type = nc_send_hello_io(session); if (type != NC_MSG_HELLO) { return type; } @@ -1899,7 +1908,7 @@ nc_ch_handshake_io(struct nc_session *session) return NC_MSG_ERROR; } - type = nc_send_hello_io(session, 1); + type = nc_send_hello_io(session); if (type != NC_MSG_HELLO) { return type; } diff --git a/src/session_client.c b/src/session_client.c index 45619917..6af5d98b 100644 --- a/src/session_client.c +++ b/src/session_client.c @@ -80,7 +80,7 @@ nc_client_context_free(void *ptr) #ifdef NC_ENABLED_SSH_TLS for (i = 0; i < c->opts.ch_bind_count; ++i) { - close(c->opts.ch_binds[i].sock); + close(c->opts.ch_binds_aux[i].sock); free((char *)c->opts.ch_binds[i].address); } free(c->opts.ch_binds); @@ -1665,7 +1665,7 @@ sock_connect(const char *src_addr, uint16_t src_port, int timeout_ms, int *sock_ int nc_sock_connect(const char *src_addr, uint16_t src_port, const char *dst_addr, uint16_t dst_port, int timeout_ms, - struct nc_keepalives *ka, int *sock_pending, char **ip_host) + const struct nc_keepalives *ka, int *sock_pending, char **ip_host) { int i, opt; int sock = sock_pending ? *sock_pending : -1; @@ -1783,10 +1783,10 @@ nc_client_ch_add_bind_listen(const char *address, uint16_t port, const char *hos } client_opts.ch_binds_aux[client_opts.ch_bind_count - 1].ti = ti; client_opts.ch_binds_aux[client_opts.ch_bind_count - 1].hostname = hostname ? strdup(hostname) : NULL; + client_opts.ch_binds_aux[client_opts.ch_bind_count - 1].sock = sock; client_opts.ch_binds[client_opts.ch_bind_count - 1].address = strdup(address); client_opts.ch_binds[client_opts.ch_bind_count - 1].port = port; - client_opts.ch_binds[client_opts.ch_bind_count - 1].sock = sock; return 0; } @@ -1799,7 +1799,7 @@ nc_client_ch_del_bind(const char *address, uint16_t port, NC_TRANSPORT_IMPL ti) if (!address && !port && !ti) { for (i = 0; i < client_opts.ch_bind_count; ++i) { - close(client_opts.ch_binds[i].sock); + close(client_opts.ch_binds_aux[i].sock); free(client_opts.ch_binds[i].address); free(client_opts.ch_binds_aux[i].hostname); @@ -1818,7 +1818,7 @@ nc_client_ch_del_bind(const char *address, uint16_t port, NC_TRANSPORT_IMPL ti) if ((!address || !strcmp(client_opts.ch_binds[i].address, address)) && (!port || (client_opts.ch_binds[i].port == port)) && (!ti || (client_opts.ch_binds_aux[i].ti == ti))) { - close(client_opts.ch_binds[i].sock); + close(client_opts.ch_binds_aux[i].sock); free(client_opts.ch_binds[i].address); --client_opts.ch_bind_count; @@ -1858,8 +1858,8 @@ nc_accept_callhome(int timeout, struct ly_ctx *ctx, struct nc_session **session) return -1; } - ret = nc_server_ch_accept_binds(client_opts.ch_binds, client_opts.ch_bind_count, timeout, - &host, &port, &bind_idx, &sock); + ret = nc_server_ch_accept_binds(client_opts.ch_binds, client_opts.ch_binds_aux, client_opts.ch_bind_count, + timeout, &host, &port, &bind_idx, &sock); if (ret < 1) { free(host); return ret; diff --git a/src/session_openssl.c b/src/session_openssl.c index 8a7c8650..e1808914 100644 --- a/src/session_openssl.c +++ b/src/session_openssl.c @@ -498,7 +498,7 @@ nc_server_tls_verify_cb(int preverify_ok, X509_STORE_CTX *x509_ctx) * if yes, this callback will be called again with the same cert, but with preverify_ok = 1 */ cert = X509_STORE_CTX_get0_cert(x509_ctx); - ret = nc_server_tls_verify_peer_cert(cert, data->opts); + ret = nc_server_tls_verify_peer_cert(cert, data); if (ret) { VRB(NULL, "Cert verify: fail (%s).", X509_verify_cert_error_string(X509_STORE_CTX_get_error(x509_ctx))); ret = -1; diff --git a/src/session_p.h b/src/session_p.h index 446e2be0..5e1c763f 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -105,6 +105,13 @@ extern struct nc_server_opts server_opts; */ #define NC_CH_NO_ENDPT_WAIT 1000 +/** + * Number of consecutive failed attempts to acquire the server configuration after which a Call Home + * client thread gives up and terminates. A single failed attempt means the configuration lock timed + * out or the server was destroyed without stopping the thread first. + */ +#define NC_CH_CONFIG_ACQUIRE_ATTEMPTS 3 + /** * Time slept in msec between Call Home thread session idle timeout checks. */ @@ -140,21 +147,37 @@ extern struct nc_server_opts server_opts; */ #define NC_CERT_EXP_LOCK_TIMEOUT 1000 +/** + * @brief Timeout in msec for acquiring the binds_lock + * (only listening socket registry array manipulation) + */ +#define NC_BINDS_LOCK_TIMEOUT 1000 + +/** + * @brief Timeout in msec for acquiring the opts_lock + * (only a few field reads or a single string duplication) + */ +#define NC_OPTS_LOCK_TIMEOUT 1000 + /** * @brief Timeout in msec for acquiring the config_lock - * (socket binding and Call Home client dispatching can involve network operations) + * (only a pointer read plus a refcount increment on the read side, only the pointer swap on the + * write side, so this can never fire unless something is broken) */ -#define NC_CONFIG_LOCK_TIMEOUT 10000 +#define NC_CONFIG_LOCK_TIMEOUT 1000 /** - * @brief Timeout in msec for the locks acquired while applying a new configuration. + * @brief Timeout in msec for acquiring the config_update_lock. * - * A reader may hold the config_lock for the whole duration of a transport handshake (TCP connect, - * SSH/TLS key exchange and authentication), which is far longer than ::NC_CONFIG_LOCK_TIMEOUT. - * Giving up here means losing the configuration change, which the caller generally cannot recover - * from, so wait much longer than any handshake can take. + * Unlike the other locks this one is held for a whole configuration apply, which includes joining + * the threads of the removed Call Home clients. A thread with an established session only notices + * that it should stop every ::NC_CH_THREAD_IDLE_TIMEOUT_SLEEP and a thread stuck in a transport + * handshake does not notice at all until the endpoint's auth-timeout elapses, which is configurable + * and unlimited when set to 0. So a legitimate apply can take a long time and this timeout is only + * a last resort - giving up here means losing the configuration change, which the caller generally + * cannot recover from. */ -#define NC_CONFIG_APPLY_LOCK_TIMEOUT 300000 +#define NC_CONFIG_UPDATE_LOCK_TIMEOUT 300000 /** * @brief Timeout in msec for acquiring session's ch_lock @@ -508,7 +531,6 @@ struct nc_server_unix_opts { struct nc_bind { char *address; /**< Either IPv4/IPv6 address or path to UNIX socket. */ uint16_t port; /**< Either port number or 0 for UNIX socket. */ - int sock; /**< Socket file descriptor, -1 if not created yet. */ }; struct nc_client_unix_opts { @@ -593,9 +615,10 @@ struct nc_client_opts { struct nc_bind *ch_binds; - struct { + struct nc_client_ch_bind_aux { NC_TRANSPORT_IMPL ti; char *hostname; + int sock; /**< Listening socket file descriptor of the corresponding bind. */ } *ch_binds_aux; uint16_t ch_bind_count; @@ -702,7 +725,26 @@ struct nc_server_ch_thread_arg { int notify_pipe[2]; /**< Self-pipe for signaling the thread to terminate. Index 0 = read end, 1 = write end. */ }; +/** + * @brief Refcounted immutable snapshot of the server configuration. + * + * Once published in ::nc_server_opts.config, a generation is never written to again. Runtime state + * that used to live here (listening sockets, Call Home thread handles) is kept in registries in + * ::nc_server_opts instead. + * + * Reference ownership rules: + * - a generation is allocated by an applier with @p refcount 1, that reference is transferred to + * ::nc_server_opts.config when the generation is published, + * - an applier that fails before publishing releases its reference itself, + * - an applier that published a new generation releases the reference of the old one, + * - ::nc_server_config_acquire() takes a reference, ::nc_server_config_release() drops one and frees + * the generation when the last one is dropped, + * - so ::nc_server_opts.config always holds exactly one reference and @p refcount is at least 1 for + * as long as a generation is published. + */ struct nc_server_config { + ATOMIC_T refcount; /**< Number of references held to this configuration generation. */ + uint16_t idle_timeout; /**< Idle timeout of the server sessions. */ char **ignored_modules; /**< Names of YANG modules that are not reported in the server message (sized-array, see libyang docs). */ @@ -756,8 +798,6 @@ struct nc_server_config { NC_CH_START_WITH start_with; /**< How to select the Call Home endpoint to connect to. */ uint8_t max_attempts; /**< Maximum number of attempts to connect to the given Call Home endpoint. */ uint16_t max_wait; /**< Maximum time to wait for a Call Home connection in seconds. */ - - struct nc_server_ch_thread_arg *thread; /**< Call Home client thread data, if dispatched. */ } *ch_clients; /**< Call Home clients (sized-array, see libyang docs). */ #ifdef NC_ENABLED_SSH_TLS @@ -789,10 +829,79 @@ struct nc_server_opts { void *content_id_data; /**< Data passed to the content_id_clb callback. */ void (*content_id_data_free)(void *data); /**< Callback to free the content_id_data. */ - /* ACCESS locked - options modified by YANG data/API - WRITE lock - * - options read when accepting sessions - READ lock */ - pthread_rwlock_t config_lock; /**< Lock for the server configuration. */ - struct nc_server_config config; /**< YANG Server configuration. */ + /** + * @brief Lock for the ::nc_server_opts.config pointer. + * + * ACCESS locked - the published configuration pointer is swapped under the WRITE lock, + * - a reference to it is acquired under the READ lock. + * + * Acquiring a new config generation is: a load of the pointer followed by an increment + * of the refcount of what was loaded and these two steps must not be split. + */ + pthread_rwlock_t config_lock; /**< Lock for the ::nc_server_opts.config pointer. */ + struct nc_server_config *config; /**< Currently published YANG server configuration generation, + NULL until ::nc_server_init(). */ + + /** + * @brief Idle timeout of the server sessions in seconds, 0 for none. + * + * ACCESS unlocked - mirror of the published config->idle_timeout, stored under the config + * WRITE lock so that it always matches the generation in ::nc_server_opts.config. + * + * It is the only piece of the configuration the session poll and paths need, and + * ::nc_ps_poll() reads it for every session on every iteration. Acquiring and releasing a + * whole generation (config lock plus two atomic refcount updates) just to read a single scalar + * that often is needlessly expensive, and mirroring it keeps the configuration out of the poll + * path completely. Reading a value one generation old is harmless here, it costs at most one + * extra poll iteration before the session times out. + */ + ATOMIC_T idle_timeout; + + /* ACCESS locked - CH threads lock - leaf lock, never acquire another lock while holding it */ + pthread_mutex_t ch_threads_lock; /**< Lock for the Call Home thread registry. */ + + /** + * @brief Call Home thread registry, keyed by the client name of the thread argument. + * + * A thread handle is runtime state, not configuration, so it is kept out of + * ::nc_server_config (sized-array, see libyang docs). + */ + struct nc_server_ch_thread_arg **ch_threads; + + /* ACCESS locked - binds lock - leaf lock, never acquire another lock while holding it */ + pthread_mutex_t binds_lock; /**< Lock for the listening socket registry. */ + + /** + * @brief Entry of the listening socket registry, one per socket the server is listening on. + * + * A listening socket is runtime state, not configuration, so it is kept out of + * ::nc_server_config, which must not be written to while it is being read by an accept path. + * See ::nc_bind_desc for the difference between a registry entry and a bind description. + */ + struct nc_bind_entry { + char *endpt_name; /**< Name of the endpoint the listening socket belongs to. */ + char *address; /**< IPv4/IPv6 address or the full path of a UNIX socket. */ + uint16_t port; /**< Port number, 0 for a UNIX socket. */ + NC_TRANSPORT_IMPL ti; /**< Transport implementation of the endpoint. */ + int sock; /**< Listening socket file descriptor. */ + } *binds; /**< Listening socket registry (sized-array, see libyang docs). */ + + /** + * @brief Lock for the server options settable only through the API, not through YANG data. + * + * Protects ::nc_server_opts.ch_dispatch_data, ::nc_server_opts.interactive_auth_clb, + * ::nc_server_opts.interactive_auth_data, ::nc_server_opts.interactive_auth_data_free, + * ::nc_server_opts.pam_config_name, ::nc_server_opts.authkey_path_fmt, + * ::nc_server_opts.ssh_protocol_string, ::nc_server_opts.user_verify_clb, + * ::nc_server_opts.unix_socket_dir and ::nc_server_opts.unix_paths. + * + * It is a leaf lock, never acquire another lock while holding it, and no other lock is held + * while acquiring it either. Since it is also held on the authentication path, it must never be + * held across anything slow (not even filesystem access) and, most importantly, never across a + * call to a user callback - read the callback and its data pointer as a pair, unlock, and only + * then call it. + */ + pthread_rwlock_t opts_lock; #ifdef NC_ENABLED_SSH_TLS char *authkey_path_fmt; /**< Path to users' public keys that may contain tokens with special meaning. */ @@ -807,7 +916,7 @@ struct nc_server_opts { /** * @brief Data for automatically dispatching Call Home clients. */ - struct { + struct nc_server_ch_dispatch_data { nc_server_ch_session_acquire_ctx_cb acquire_ctx_cb; /**< Acquiring libyang context callback. */ nc_server_ch_session_release_ctx_cb release_ctx_cb; /**< Releasing libyang context callback. */ void *ctx_cb_data; /**< Data passed to the callbacks above. */ @@ -849,6 +958,42 @@ struct nc_server_opts { configuration update to complete before accepting a new one. */ }; +/** + * @brief Description of a single listening socket required by a server configuration generation. + * + * There are three representations of a listening socket in the server, each with a different + * lifetime and owner: + * + * - ::nc_bind is the configured one. It is part of ::nc_server_config, so it is immutable and it + * only holds what the YANG data say - for a UNIX endpoint that is a possibly relative socket path + * and no port at all. + * - ::nc_bind_entry is the live one. It is an entry of the listening socket registry in + * ::nc_server_opts.binds and it owns the open socket FD. Since sockets must survive a + * configuration change untouched (a session may be in the middle of being accepted on one), they + * cannot live in a configuration generation that is replaced on every apply. + * - ::nc_bind_desc, this structure, is the transient one. It is the resolved and flattened form of + * all the ::nc_bind of a single generation, built by an applier and thrown away once the apply is + * over. + * + * The description exists because reconciling the registry with a new generation needs data that a + * ::nc_bind does not have and that must not be computed with the registry lock held. + * + * So an apply (::nc_server_binds_reconcile()) builds the descriptions of the new generation with no lock held, matches + * them against the registry entries, opens the sockets that are missing and closes the entries that no description matches. + * Everything that can fail is done into the descriptions first, the registry itself is only ever modified by steps + * that cannot fail anymore, so a failed apply leaves the registry exactly as it was. + */ +struct nc_bind_desc { + const struct nc_endpt *endpt; /**< Endpoint the listening socket belongs to. */ + char *address; /**< Resolved address, the full socket path for a UNIX endpoint. */ + uint16_t port; /**< Port number, 0 for a UNIX socket. */ + int reused; /**< Whether an already registered socket is being reused. */ + LY_ARRAY_COUNT_TYPE entry_idx; /**< Index of the reused registry entry, valid only if @p reused. */ + char *rename; /**< New endpoint name to store into the reused registry entry, + NULL if the endpoint was not renamed. */ + int sock; /**< Newly opened listening socket, -1 if none was opened. */ +}; + /** * @brief Type of the session */ @@ -964,6 +1109,16 @@ struct nc_session { pthread_mutex_t ch_lock; /**< Call Home thread lock */ pthread_cond_t ch_cond; /**< Call Home thread condition */ + /** + * @brief Configuration generation pinned for the duration of the transport handshake. + * + * A borrowed pointer, NOT a counted reference - the reference belongs to the function + * that acquired it, which also clears this field before the session leaves the handshake. + * ::nc_session_free() must not release it. It is only ever set by ::nc_accept() and + * ::nc_connect_ch_endpt(); do not add a third setter with different ownership. + */ + const struct nc_server_config *config; + #ifdef NC_ENABLED_SSH_TLS uint16_t ssh_auth_attempts; /**< number of failed SSH authentication attempts */ void *client_cert; /**< TLS client certificate if used for authentication */ @@ -1087,28 +1242,69 @@ struct nc_client_context *nc_client_context_location(void); void *nc_realloc(void *ptr, size_t size); /** - * @brief Get the UNIX socket path for the given endpoint. + * @brief Reconcile the listening socket registry with the given server configuration. + * + * Starts listening for every bind of @p config that has no registry entry yet and stops listening + * for every registry entry that @p config no longer contains. Nothing is written to @p config. * - * @param[in] endpt Endpoint to get the socket path for. - * @return Socket path, NULL on error. + * @note Only one thread may reconcile the registry at a time, the callers must be serialized by + * ::nc_server_opts.config_update_lock. + * + * @param[in] config Server configuration to reconcile the registry with. + * @return 0 on success, 1 on error (the registry is left as it was). */ -char *nc_server_unix_get_socket_path(const struct nc_endpt *endpt); +int nc_server_binds_reconcile(const struct nc_server_config *config); /** - * @brief Bind and listen on a socket for the given endpoint and its bind. + * @brief Stop listening on all the registered sockets and free the listening socket registry. + */ +void nc_server_binds_destroy(void); + +#ifdef NC_ENABLED_SSH_TLS + +/** + * @brief Reconcile the Call Home thread registry with the given server configuration. + * + * Dispatches a thread for every Call Home client of @p config that has none yet, keeps the already + * running ones and stops the threads of the clients @p config no longer contains. The running + * clients are learned from the registry itself, not from any configuration. Nothing is written + * to @p config. * - * @param[in] endpt Endpoint the bind belongs to. - * @param[in] bind Bind to bind and listen for. + * Starting the new clients is atomic - if any of them fails to start, the ones started by this call + * are stopped again and no client is stopped at all. Stopping the removed clients afterwards is + * not: if it fails halfway through, some removed clients are already stopped and the error is + * simply returned. That is enough because the callers react to the error by reconciling against + * the generation that stays published, which dispatches the stopped clients again. + * + * @note Only one thread may reconcile the registry at a time, the callers must be serialized by + * ::nc_server_opts.config_update_lock. + * + * @param[in] config Server configuration to reconcile the registry with. * @return 0 on success, 1 on error. */ -int nc_server_bind_and_listen(struct nc_endpt *endpt, struct nc_bind *bind); +int nc_server_ch_clients_reconcile(const struct nc_server_config *config); + +#endif /* NC_ENABLED_SSH_TLS */ /** - * @brief Free server configuration data (only YANG config data). + * @brief Acquire a reference to the currently published server configuration generation. + * + * The returned generation is guaranteed to stay valid and unchanged until the reference is dropped + * by ::nc_server_config_release(), no lock needs to be held meanwhile. * - * @param[in] config Server configuration to free. + * @return Pinned server configuration. + * @return NULL if the server is not initialized or the configuration lock could not be acquired. */ -void nc_server_config_free(struct nc_server_config *config); +const struct nc_server_config *nc_server_config_acquire(void); + +/** + * @brief Release a reference to a server configuration generation. + * + * Frees the generation if this was the last reference held to it. + * + * @param[in] config Server configuration to release, may be NULL. + */ +void nc_server_config_release(const struct nc_server_config *config); /** * @brief Get passwd entry for UID or a user. @@ -1301,7 +1497,7 @@ int nc_sock_bind_inet(int sock, const char *address, uint16_t port, int is_ipv4) * @return Connected socket or -1 on error. */ int nc_sock_connect(const char *src_addr, uint16_t src_port, const char *dst_addr, uint16_t dst_port, int timeout_ms, - struct nc_keepalives *ka, int *sock_pending, char **ip_host); + const struct nc_keepalives *ka, int *sock_pending, char **ip_host); /** * @brief Accept a new socket connection. @@ -1327,6 +1523,7 @@ int nc_sock_listen_inet(const char *address, uint16_t port); * @brief Accept a new connection on any of the given Call Home binds. * * @param[in] binds Call Home binds to accept on. + * @param[in] binds_aux Auxiliary data of @p binds holding the listening sockets. * @param[in] bind_count Number of @p binds. * @param[in] timeout Timeout for accepting. * @param[out] host Host of the remote peer. Can be NULL. @@ -1335,8 +1532,8 @@ int nc_sock_listen_inet(const char *address, uint16_t port); * @param[out] sock Accepted socket, if any. * @return -1 on error, 0 on timeout, 1 if a socket was accepted. */ -int nc_server_ch_accept_binds(struct nc_bind *binds, uint16_t bind_count, int timeout, char **host, - uint16_t *port, uint16_t *bind_idx, int *sock); +int nc_server_ch_accept_binds(const struct nc_bind *binds, const struct nc_client_ch_bind_aux *binds_aux, + uint16_t bind_count, int timeout, char **host, uint16_t *port, uint16_t *bind_idx, int *sock); /** * @brief Establish a UNIX transport session. @@ -1349,13 +1546,14 @@ int nc_server_ch_accept_binds(struct nc_bind *binds, uint16_t bind_count, int ti int nc_connect_unix_session(struct nc_session *session, int sock, const char *username); /** - * @brief Gets a listening endpoint based on its name. + * @brief Gets a listening endpoint of a pinned configuration based on its name. * + * @param[in] config Pinned server configuration to search. * @param[in] name The name of the endpoint. * @param[out] endpt Pointer to the endpoint structure. * @return 0 on success, 1 on failure. */ -int nc_server_endpt_get(const char *name, struct nc_endpt **endpt); +int nc_server_endpt_get(const struct nc_server_config *config, const char *name, const struct nc_endpt **endpt); /** * @brief Add a client Call Home bind, listen on it. @@ -1392,30 +1590,57 @@ NC_MSG_TYPE nc_connect_callhome(const char *host, uint16_t port, NC_TRANSPORT_IM #ifdef NC_ENABLED_SSH_TLS +/** + * @brief Get the names of all the Call Home clients that have a thread running. + * + * @param[out] names Copies of the client names (sized-array, see libyang docs), free with + * ::nc_server_ch_thread_names_free(). + * @return 0 on success, 1 on error. + */ +int nc_server_ch_thread_names_get(char ***names); + +/** + * @brief Free the Call Home client names returned by ::nc_server_ch_thread_names_get(). + * + * @param[in] names Client names to free, may be NULL. + */ +void nc_server_ch_thread_names_free(char **names); + +/** + * @brief Stop all the Call Home client threads and free the thread registry. + * + * @return 0 on success, 1 on error. + */ +int nc_server_ch_threads_destroy(void); + /** * @brief Stop a dispatched Call Home client thread, if such thread was dispatched for the given client. * - * @warning The caller MUST hold both WRITE config lock and CONFIG APPLY mutex when calling this function. + * Takes no configuration lock at all, the thread is looked up in the Call Home thread registry. * - * @param[in] ch_client Call Home client to stop the thread for, can be NULL. - * @return 0 if the thread was successfully stopped, 1 on error. + * @param[in] client_name Name of the Call Home client to stop the thread for. + * @return 0 if the thread was successfully stopped or none was running, 1 on error. */ -int nc_session_server_ch_client_dispatch_stop(struct nc_ch_client *ch_client); +int nc_session_server_ch_client_dispatch_stop(const char *client_name); /** * @brief Dispatch a thread connecting to a listening NETCONF client and creating Call Home sessions. * - * @note The config WRITE lock MUST be held. + * The thread is added to the Call Home thread registry and created atomically, so it is findable by + * ::nc_session_server_ch_client_dispatch_stop() and by a concurrent configuration apply from the + * moment it exists. There is never more than one thread per Call Home client. * - * @param[in] ch_client Call Home client to dispatch the thread for. + * @param[in] client_name Name of the Call Home client to dispatch the thread for. * @param[in] acquire_ctx_cb Callback for acquiring new session context. * @param[in] release_ctx_cb Callback for releasing session context. * @param[in] ctx_cb_data Arbitrary user data passed to @p acquire_ctx_cb and @p release_ctx_cb. * @param[in] new_session_cb Callback called for every established session on the client. * @param[in] new_session_cb_data Arbitrary user data passed to @p new_session_cb. - * @return 0 if the thread was successfully created, -1 on error. + * @return 0 if the thread was successfully created. + * @return 1 if a thread is already running for the client and nothing was done. + * @return -1 on error. */ -int _nc_connect_ch_client_dispatch(struct nc_ch_client *ch_client, nc_server_ch_session_acquire_ctx_cb acquire_ctx_cb, +int _nc_connect_ch_client_dispatch(const char *client_name, nc_server_ch_session_acquire_ctx_cb acquire_ctx_cb, nc_server_ch_session_release_ctx_cb release_ctx_cb, void *ctx_cb_data, nc_server_ch_new_session_cb new_session_cb, void *new_session_cb_data); @@ -1477,11 +1702,11 @@ int nc_session_tls_crl_verify_post_handshake(void *tls_session, void *cert_store /** * @brief Check whether a module is not ignored by the server. * + * @param[in] config Pinned server configuration, may be NULL. * @param[in] mod_name Module name to check. - * @param[in] config_locked Whether the configuration lock is already held or should be acquired in this function. * @return Whether the module is ignored. */ -int nc_server_is_mod_ignored(const char *mod_name, int config_locked); +int nc_server_is_mod_ignored(const struct nc_server_config *config, const char *mod_name); /** * Functions diff --git a/src/session_server.c b/src/session_server.c index 374f7c44..ba94b0e1 100644 --- a/src/session_server.c +++ b/src/session_server.c @@ -57,6 +57,9 @@ struct nc_server_opts server_opts = { .hello_lock = PTHREAD_RWLOCK_INITIALIZER, .config_lock = PTHREAD_RWLOCK_INITIALIZER, .config_update_lock = PTHREAD_MUTEX_INITIALIZER, + .binds_lock = PTHREAD_MUTEX_INITIALIZER, + .opts_lock = PTHREAD_RWLOCK_INITIALIZER, + .ch_threads_lock = PTHREAD_MUTEX_INITIALIZER, }; static nc_rpc_clb global_rpc_clb = NULL; @@ -64,23 +67,181 @@ static nc_rpc_clb global_rpc_clb = NULL; #ifdef NC_ENABLED_SSH_TLS /** - * @brief Get a CH client with the given @p name . + * @brief Free a Call Home thread argument. * - * @note The configuration read lock must be held. + * @param[in] thread_arg Thread argument to free, may be NULL. + */ +static void +nc_server_ch_thread_arg_free(struct nc_server_ch_thread_arg *thread_arg) +{ + if (!thread_arg) { + return; + } + + free(thread_arg->client_name); + if (thread_arg->notify_pipe[0] != -1) { + close(thread_arg->notify_pipe[0]); + } + if (thread_arg->notify_pipe[1] != -1) { + close(thread_arg->notify_pipe[1]); + } + free(thread_arg); +} + +/** + * @brief Remove a Call Home thread argument from the thread registry. + * + * The registry entry is the ownership token of the thread argument, whoever removes it becomes + * responsible for terminating the thread and freeing the argument. + * + * @param[in] client_name Name of the Call Home client to unregister the thread of. + * @param[out] thread_arg Unregistered thread argument, NULL if the client had no thread registered. + * @return 0 on success, 1 on error. + */ +static int +nc_server_ch_thread_reg_del(const char *client_name, struct nc_server_ch_thread_arg **thread_arg) +{ + LY_ARRAY_COUNT_TYPE u; + + *thread_arg = NULL; + + /* CH THREADS LOCK */ + if (nc_mutex_lock(&server_opts.ch_threads_lock, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } + + LY_ARRAY_FOR(server_opts.ch_threads, u) { + if (strcmp(server_opts.ch_threads[u]->client_name, client_name)) { + continue; + } + + *thread_arg = server_opts.ch_threads[u]; + + /* swap the last entry into the hole, the order of the registry is irrelevant */ + server_opts.ch_threads[u] = server_opts.ch_threads[LY_ARRAY_COUNT(server_opts.ch_threads) - 1]; + LY_ARRAY_DECREMENT_FREE(server_opts.ch_threads); + break; + } + + /* CH THREADS UNLOCK */ + nc_mutex_unlock(&server_opts.ch_threads_lock, __func__); + return 0; +} + +/** + * @brief Unregister a Call Home thread that is terminating on its own and free its argument. + * + * Called by the Call Home thread itself right before it returns. Normally the thread only ever + * terminates because ::nc_session_server_ch_client_dispatch_stop() told it to, in which case the + * stopper has already removed the registry entry and does all the cleanup itself. If the thread + * terminates for any other reason (an unrecoverable error), it has to take itself out of the + * registry, otherwise every later configuration apply would believe the client is still running + * and would never dispatch it again. + * + * The registry entry is the ownership token of the thread argument, so the entry removal decides + * who cleans up and the ::nc_server_opts.ch_threads_lock makes that decision atomic. + * + * @param[in] thread_arg Argument of the calling thread. + */ +static void +nc_server_ch_thread_unreg_self(struct nc_server_ch_thread_arg *thread_arg) +{ + LY_ARRAY_COUNT_TYPE u; + int found = 0; + + /* CH THREADS LOCK */ + if (nc_mutex_lock(&server_opts.ch_threads_lock, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) { + return; + } + + LY_ARRAY_FOR(server_opts.ch_threads, u) { + if (server_opts.ch_threads[u] != thread_arg) { + continue; + } + + found = 1; + + /* swap the last entry into the hole, the order of the registry is irrelevant */ + server_opts.ch_threads[u] = server_opts.ch_threads[LY_ARRAY_COUNT(server_opts.ch_threads) - 1]; + LY_ARRAY_DECREMENT_FREE(server_opts.ch_threads); + break; + } + + /* CH THREADS UNLOCK */ + nc_mutex_unlock(&server_opts.ch_threads_lock, __func__); + + if (!found) { + /* someone else owns us now and will join us, nothing to do */ + return; + } + + /* nobody is going to join us anymore, so make sure our resources are reclaimed */ + pthread_detach(thread_arg->tid); + nc_server_ch_thread_arg_free(thread_arg); +} + +void +nc_server_ch_thread_names_free(char **names) +{ + LY_ARRAY_COUNT_TYPE u; + + LY_ARRAY_FOR(names, u) { + free(names[u]); + } + LY_ARRAY_FREE(names); +} + +int +nc_server_ch_thread_names_get(char ***names) +{ + int rc = 0; + LY_ARRAY_COUNT_TYPE u; + char *name; + + *names = NULL; + + /* CH THREADS LOCK */ + if (nc_mutex_lock(&server_opts.ch_threads_lock, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } + + if (LY_ARRAY_COUNT(server_opts.ch_threads)) { + LY_ARRAY_CREATE_GOTO(NULL, *names, LY_ARRAY_COUNT(server_opts.ch_threads), rc, cleanup); + LY_ARRAY_FOR(server_opts.ch_threads, u) { + name = strdup(server_opts.ch_threads[u]->client_name); + NC_CHECK_ERRMEM_GOTO(!name, rc = 1, cleanup); + (*names)[u] = name; + LY_ARRAY_INCREMENT(*names); + } + } + +cleanup: + /* CH THREADS UNLOCK */ + nc_mutex_unlock(&server_opts.ch_threads_lock, __func__); + if (rc) { + nc_server_ch_thread_names_free(*names); + *names = NULL; + } + return rc ? 1 : 0; +} + +/** + * @brief Get a CH client with the given @p name from a pinned configuration. * + * @param[in] config Pinned server configuration to search. * @param[in] name Name of the CH client to find. * @return CH client, NULL if not found. */ -static struct nc_ch_client * -nc_server_ch_client_get(const char *name) +static const struct nc_ch_client * +nc_server_ch_client_get_pinned(const struct nc_server_config *config, const char *name) { - struct nc_ch_client *client = NULL; + LY_ARRAY_COUNT_TYPE u; assert(name); - LY_ARRAY_FOR(server_opts.config.ch_clients, struct nc_ch_client, client) { - if (client->name && !strcmp(client->name, name)) { - return client; + LY_ARRAY_FOR(config->ch_clients, u) { + if (!strcmp(config->ch_clients[u].name, name)) { + return &config->ch_clients[u]; } } @@ -90,15 +251,19 @@ nc_server_ch_client_get(const char *name) #endif /* NC_ENABLED_SSH_TLS */ int -nc_server_endpt_get(const char *name, struct nc_endpt **endpt) +nc_server_endpt_get(const struct nc_server_config *config, const char *name, const struct nc_endpt **endpt) { - struct nc_endpt *ep; + LY_ARRAY_COUNT_TYPE u; *endpt = NULL; - LY_ARRAY_FOR(server_opts.config.endpts, struct nc_endpt, ep) { - if (ep->name && !strcmp(ep->name, name)) { - *endpt = ep; + if (!config) { + return 1; + } + + LY_ARRAY_FOR(config->endpts, u) { + if (config->endpts[u].name && !strcmp(config->endpts[u].name, name)) { + *endpt = &config->endpts[u]; return 0; } } @@ -249,8 +414,8 @@ nc_server_ch_set_dispatch_data(nc_server_ch_session_acquire_ctx_cb acquire_ctx_c { NC_CHECK_ARG_RET(NULL, acquire_ctx_cb, release_ctx_cb, new_session_cb, ); - /* CONFIG WRITE LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* OPTS WRITE LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_WRITE, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { return; } @@ -260,24 +425,24 @@ nc_server_ch_set_dispatch_data(nc_server_ch_session_acquire_ctx_cb acquire_ctx_c server_opts.ch_dispatch_data.new_session_cb = new_session_cb; server_opts.ch_dispatch_data.new_session_cb_data = new_session_cb_data; - /* CONFIG WRITE UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS WRITE UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); } API void nc_server_ch_set_new_session_fail_cb(nc_server_ch_new_session_fail_cb new_session_fail_cb, void *new_session_fail_cb_data) { - /* CONFIG WRITE LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* OPTS WRITE LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_WRITE, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { return; } server_opts.ch_dispatch_data.new_session_fail_cb = new_session_fail_cb; server_opts.ch_dispatch_data.new_session_fail_cb_data = new_session_fail_cb_data; - /* CONFIG WRITE UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS WRITE UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); } #endif @@ -413,23 +578,25 @@ nc_sock_listen_inet(const char *address, uint16_t port) /** * @brief Construct the full path to the UNIX socket. * + * @note Resolves the paths on the filesystem, so no lock may be held. + * + * @param[in] dir Base directory the socket must reside in, NULL if none is set. * @param[in] filename Name of the socket file. * @param[out] path Constructed full path to the UNIX socket (must be freed by the caller). * @return 0 on success, 1 on error. */ static int -nc_session_unix_construct_socket_path(const char *filename, char **path) +nc_session_unix_construct_socket_path(const char *dir, const char *filename, char **path) { int rc = 0, is_prefix, is_subdir, is_exact; char *full_path = NULL, *real_base_dir = NULL, *last_slash = NULL, *sock_dir_path = NULL; char *real_target_dir = NULL; struct sockaddr_un sun; size_t dir_len, base_len; - const char *dir = server_opts.unix_socket_dir; if (!dir) { ERR(NULL, "Cannot construct UNIX socket path \"%s\"" - " (no base directory set, see nc_set_unix_socket_dir()).", filename); + " (no base directory set, see nc_server_set_unix_socket_dir()).", filename); return 1; } @@ -510,24 +677,37 @@ nc_session_unix_construct_socket_path(const char *filename, char **path) return rc; } -char * +/** + * @brief Get the full path of the UNIX socket of an endpoint. + * + * @param[in] endpt Endpoint to get the socket path for. + * @return Socket path, NULL on error. + */ +static char * nc_server_unix_get_socket_path(const struct nc_endpt *endpt) { + int rc = 0; LY_ARRAY_COUNT_TYPE i; const char *p = NULL; - char *path = NULL; + char *path = NULL, *sock_dir = NULL; - /* check the endpoints options for type of socket path */ - if (endpt->opts.unix->path_type == NC_UNIX_SOCKET_PATH_FILE) { - /* UNIX socket endpoints always have only one bind, get its address */ - p = endpt->binds[0].address; + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { + return NULL; + } - /* it is relative, we need to construct the full path */ - if (nc_session_unix_construct_socket_path(p, &path)) { - return NULL; + /* only copy what is needed out of the options, resolving the path touches the filesystem and + * the options lock must not be held for that */ + switch (endpt->opts.unix->path_type) { + case NC_UNIX_SOCKET_PATH_FILE: + /* the address in the bind is relative to the base directory */ + if (server_opts.unix_socket_dir) { + sock_dir = strdup(server_opts.unix_socket_dir); + NC_CHECK_ERRMEM_GOTO(!sock_dir, rc = 1, cleanup); } - } else if (endpt->opts.unix->path_type == NC_UNIX_SOCKET_PATH_HIDDEN) { - /* search the mappings, no need to construct the path */ + break; + case NC_UNIX_SOCKET_PATH_HIDDEN: + /* search the mappings, they store the full path so there is nothing to construct */ LY_ARRAY_FOR(server_opts.unix_paths, i) { if (!strcmp(server_opts.unix_paths[i].endpt_name, endpt->name)) { p = server_opts.unix_paths[i].path; @@ -536,15 +716,38 @@ nc_server_unix_get_socket_path(const struct nc_endpt *endpt) } if (!p) { ERR(NULL, "UNIX socket path mapping for endpoint \"%s\" not found.", endpt->name); - return NULL; + rc = 1; + goto cleanup; } path = strdup(p); - NC_CHECK_ERRMEM_RET(!path, NULL); - } else { + NC_CHECK_ERRMEM_GOTO(!path, rc = 1, cleanup); + break; + default: ERRINT; + rc = 1; + break; + } + +cleanup: + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); + + if (rc) { + free(sock_dir); + free(path); + return NULL; + } + if (path) { + /* the hidden path is used as it is */ + return path; } + /* UNIX socket endpoints always have only one bind, its address is the socket file name */ + if (nc_session_unix_construct_socket_path(sock_dir, endpt->binds[0].address, &path)) { + path = NULL; + } + free(sock_dir); return path; } @@ -748,32 +951,26 @@ nc_sock_host_get(const struct sockaddr_storage *saddr, int client_sock, char **c * @brief Log the accepted connection. * * @param[in] saddr sockaddr_storage. - * @param[in] endpt Endpoint on which the connection was accepted (optional, used for logging). - * @param[in] bind Bind on which the connection was accepted. + * @param[in] address Address of the bind the connection was accepted on, the socket path for AF_UNIX. + * @param[in] port Port of the bind the connection was accepted on. * @param[in] client_address Hostname or IP address of the connecting client. * @param[in] client_port Port number of the connecting client, if any. * @return 0 on success, -1 on error. */ static int -nc_sock_log_accepted(const struct sockaddr_storage *saddr, const struct nc_endpt *endpt, const struct nc_bind *bind, +nc_sock_log_accepted(const struct sockaddr_storage *saddr, const char *address, uint16_t port, const char *client_address, uint16_t client_port) { - char *unix_sockpath = NULL; - if (saddr->ss_family == AF_UNIX) { - /* UNIX socket, get the socket path for logging, - * UNIX socket connection can NOT be over call home (caller = client connect), so endpt is always available */ - assert(endpt); - unix_sockpath = nc_server_unix_get_socket_path(endpt); - VRB(NULL, "Accepted a new connection on %s.", unix_sockpath ? unix_sockpath : "UNIX socket"); - free(unix_sockpath); + /* UNIX socket, the address is the full socket path */ + VRB(NULL, "Accepted a new connection on %s.", address); } else if (saddr->ss_family == AF_INET) { /* IPv4 socket */ - VRB(NULL, "Accepted a new connection on %s:%" PRIu16 " from %s:%" PRIu16 ".", bind->address, bind->port, + VRB(NULL, "Accepted a new connection on %s:%" PRIu16 " from %s:%" PRIu16 ".", address, port, client_address, client_port); } else if (saddr->ss_family == AF_INET6) { /* IPv6 socket */ - VRB(NULL, "Accepted a new connection on [%s]:%" PRIu16 " from [%s]:%" PRIu16 ".", bind->address, bind->port, + VRB(NULL, "Accepted a new connection on [%s]:%" PRIu16 " from [%s]:%" PRIu16 ".", address, port, client_address, client_port); } else { ERR(NULL, "Source host of an unknown protocol family."); @@ -811,6 +1008,12 @@ nc_sock_accept_first(struct pollfd *pfd, uint16_t pfd_count, int *client_sock, /* another thread already accepted the connection, try another one */ continue; } + if ((errno == EBADF) || (errno == ENOTSOCK)) { + /* the listening socket was closed by a configuration apply after we copied it + * out of the registry, which is a normal outcome here, try another one */ + DBG(NULL, "Accept on an already closed listening socket, skipping it."); + continue; + } ERR(NULL, "Accept failed (%s).", strerror(errno)); return -1; } @@ -836,8 +1039,8 @@ nc_sock_accept_first(struct pollfd *pfd, uint16_t pfd_count, int *client_sock, * * @param[in] pollfds FDs to poll for new connections. * @param[in] pollfd_count Number of FDs in the pollfds array. - * @param[in] endpt_map Map of pollfd indices to endpoints (optional, used for logging). - * @param[in] bind_map Map of pollfd indices to binds (optional, used for logging). + * @param[in] addr_map Map of pollfd indices to bind addresses (used for logging). + * @param[in] port_map Map of pollfd indices to bind ports (used for logging). * @param[in] timeout Timeout for accepting a connection. * @param[out] host Hostname or IP address of the connecting client. * @param[out] port Port number of the connecting client, if any. @@ -846,16 +1049,14 @@ nc_sock_accept_first(struct pollfd *pfd, uint16_t pfd_count, int *client_sock, * @return 1 on success, 0 on timeout, -1 on error. */ static int -nc_sock_accept_pollfds(struct pollfd *pollfds, uint16_t pollfd_count, struct nc_endpt **endpt_map, - struct nc_bind **bind_map, int timeout, char **host, uint16_t *port, +nc_sock_accept_pollfds(struct pollfd *pollfds, uint16_t pollfd_count, const char **addr_map, + const uint16_t *port_map, int timeout, char **host, uint16_t *port, uint16_t *fd_idx, int *sock) { uint16_t client_port = 0, matched_pollfd_idx = 0; char *client_address = NULL; struct sockaddr_storage client_saddr; socklen_t saddr_len = sizeof(client_saddr); - struct nc_endpt *endpt; - struct nc_bind *bind; int client_sock = -1, ret = 1, r, flags; if (!pollfd_count) { @@ -883,9 +1084,6 @@ nc_sock_accept_pollfds(struct pollfd *pollfds, uint16_t pollfd_count, struct nc_ goto cleanup; } - bind = bind_map[matched_pollfd_idx]; - endpt = endpt_map ? endpt_map[matched_pollfd_idx] : NULL; - /* 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)); @@ -899,7 +1097,8 @@ nc_sock_accept_pollfds(struct pollfd *pollfds, uint16_t pollfd_count, struct nc_ } /* log the new accepted connection */ - if ((r = nc_sock_log_accepted(&client_saddr, endpt, bind, client_address, client_port))) { + if ((r = nc_sock_log_accepted(&client_saddr, addr_map[matched_pollfd_idx], port_map[matched_pollfd_idx], + client_address, client_port))) { ret = r; goto cleanup; } @@ -926,9 +1125,23 @@ nc_sock_accept_pollfds(struct pollfd *pollfds, uint16_t pollfd_count, struct nc_ } /** - * @brief Accept a new connection on any of the server's listening binds. + * @brief Accept a new connection on any of the registered listening sockets. * - * @param[in] config Server configuration. + * The listening socket registry is only read to build the local poll arrays, the ::poll() itself + * and the ::accept() run with no lock held. + * + * @note Only the sockets of endpoints that @p config contains are polled. A socket registered for + * an endpoint that @p config does not know (it was registered or its endpoint renamed after + * @p config was read) is skipped, its pending connections are left in the listen backlog for a + * call with a newer configuration pinned. That way no connection is ever accepted just to be + * dropped again because there is no endpoint to serve it with. + * + * @note Since the registry lock is not held while polling, a configuration apply may close one of + * the sockets meanwhile. Polling and accepting a closed descriptor is handled (the socket is simply + * skipped), but the descriptor number may also have been reused by then, in which case the + * connection is accepted on and logged with whatever the endpoint of the new socket is. + * + * @param[in] config Pinned server configuration used to look the accepting endpoint up. * @param[in] timeout Timeout for accepting a connection. * @param[out] host Hostname or IP address of the connecting client. * @param[out] port Port number of the connecting client, if any. @@ -937,112 +1150,155 @@ nc_sock_accept_pollfds(struct pollfd *pollfds, uint16_t pollfd_count, struct nc_ * @return 1 on success, 0 on timeout, -1 on error. */ static int -nc_server_accept_binds(struct nc_server_config *config, int timeout, char **host, +nc_server_accept_binds(const struct nc_server_config *config, int timeout, char **host, uint16_t *port, LY_ARRAY_COUNT_TYPE *idx, int *sock) { struct pollfd *pollfds = NULL; - uint16_t pollfd_count = 0, fd_idx = 0, bind_count = 0; - LY_ARRAY_COUNT_TYPE i; - struct nc_endpt *endpt; - struct nc_bind *bind; - int ret = 1; - struct nc_endpt **endpt_map = NULL; - struct nc_bind **bind_map = NULL; - - /* count the number of valid binds and prepare the pollfd and map parallel arrays */ - LY_ARRAY_FOR(config->endpts, i) { - bind_count += LY_ARRAY_COUNT(config->endpts[i].binds); + uint16_t pollfd_count = 0, fd_idx = 0, i, bind_count = 0; + LY_ARRAY_COUNT_TYPE u; + int ret = 1, binds_locked = 0; + char **addr_map = NULL; + uint16_t *port_map = NULL; + LY_ARRAY_COUNT_TYPE *endpt_map = NULL; + + /* BINDS LOCK */ + if (nc_mutex_lock(&server_opts.binds_lock, NC_BINDS_LOCK_TIMEOUT, __func__) != 1) { + return -1; } + binds_locked = 1; + + bind_count = LY_ARRAY_COUNT(server_opts.binds); if (!bind_count) { /* no binds to accept on, treat as a timeout */ ret = 0; goto cleanup; } + /* copy the registry into local arrays, so that the lock can be released before polling */ pollfds = malloc(bind_count * sizeof *pollfds); - NC_CHECK_ERRMEM_RET(!pollfds, -1); + NC_CHECK_ERRMEM_GOTO(!pollfds, ret = -1, cleanup); + addr_map = calloc(bind_count, sizeof *addr_map); + NC_CHECK_ERRMEM_GOTO(!addr_map, ret = -1, cleanup); + port_map = malloc(bind_count * sizeof *port_map); + NC_CHECK_ERRMEM_GOTO(!port_map, ret = -1, cleanup); endpt_map = malloc(bind_count * sizeof *endpt_map); NC_CHECK_ERRMEM_GOTO(!endpt_map, ret = -1, cleanup); - bind_map = malloc(bind_count * sizeof *bind_map); - NC_CHECK_ERRMEM_GOTO(!bind_map, ret = -1, cleanup); - /* fill the arrays */ - LY_ARRAY_FOR(config->endpts, struct nc_endpt, endpt) { - LY_ARRAY_FOR(endpt->binds, struct nc_bind, bind) { - if (bind->sock < 0) { - /* invalid socket */ - continue; + for (i = 0; i < bind_count; ++i) { + /* resolve the endpoint of the bind in the pinned configuration, it is immutable so the + * index stays valid for as long as the configuration is pinned */ + LY_ARRAY_FOR(config->endpts, u) { + if (!strcmp(config->endpts[u].name, server_opts.binds[i].endpt_name)) { + break; } + } + if (u == LY_ARRAY_COUNT(config->endpts)) { + /* we would have no endpoint to serve a connection accepted here with, do not poll it */ + continue; + } + endpt_map[pollfd_count] = u; + + pollfds[pollfd_count].fd = server_opts.binds[i].sock; + pollfds[pollfd_count].events = POLLIN; + pollfds[pollfd_count].revents = 0; - pollfds[pollfd_count].fd = bind->sock; - pollfds[pollfd_count].events = POLLIN; - pollfds[pollfd_count].revents = 0; + /* the registry entries may be freed once the lock is released, so copy the address */ + addr_map[pollfd_count] = strdup(server_opts.binds[i].address); + NC_CHECK_ERRMEM_GOTO(!addr_map[pollfd_count], ret = -1, cleanup); + port_map[pollfd_count] = server_opts.binds[i].port; - endpt_map[pollfd_count] = endpt; - bind_map[pollfd_count] = bind; + ++pollfd_count; + } - ++pollfd_count; - } + /* BINDS UNLOCK */ + nc_mutex_unlock(&server_opts.binds_lock, __func__); + binds_locked = 0; + + if (!pollfd_count) { + /* every registered socket belongs to an endpoint the pinned configuration does not have, + * report a timeout right away and let the caller retry with a newer configuration */ + VRB(NULL, "No listening socket of the pinned configuration to accept on."); + ret = 0; + goto cleanup; } /* accept a new connection on any of the sockets */ - ret = nc_sock_accept_pollfds(pollfds, pollfd_count, endpt_map, bind_map, timeout, host, port, &fd_idx, sock); - if (idx && (ret > 0)) { - *idx = endpt_map[fd_idx] - config->endpts; + ret = nc_sock_accept_pollfds(pollfds, pollfd_count, (const char **)addr_map, port_map, timeout, host, port, + &fd_idx, sock); + if ((ret > 0) && idx) { + *idx = endpt_map[fd_idx]; } cleanup: + if (binds_locked) { + /* BINDS UNLOCK */ + nc_mutex_unlock(&server_opts.binds_lock, __func__); + } + if (addr_map) { + for (i = 0; i < bind_count; ++i) { + free(addr_map[i]); + } + } free(pollfds); + free(addr_map); + free(port_map); free(endpt_map); - free(bind_map); return ret; } int -nc_server_ch_accept_binds(struct nc_bind *binds, uint16_t bind_count, int timeout, char **host, - uint16_t *port, uint16_t *bind_idx, int *sock) +nc_server_ch_accept_binds(const struct nc_bind *binds, const struct nc_client_ch_bind_aux *binds_aux, + uint16_t bind_count, int timeout, char **host, uint16_t *port, uint16_t *bind_idx, int *sock) { struct pollfd *pollfds = NULL; uint16_t pollfd_count = 0, fd_idx = 0, i; int ret = 1; - struct nc_bind **bind_map = NULL; + const char **addr_map = NULL; + uint16_t *port_map = NULL, *idx_map = NULL; if (!bind_count) { /* no binds to accept on, treat as a timeout */ - ret = 0; - goto cleanup; + return 0; } /* prepare the pollfd and map parallel arrays */ pollfds = malloc(bind_count * sizeof *pollfds); NC_CHECK_ERRMEM_RET(!pollfds, -1); - bind_map = malloc(bind_count * sizeof *bind_map); - NC_CHECK_ERRMEM_GOTO(!bind_map, ret = -1, cleanup); + addr_map = malloc(bind_count * sizeof *addr_map); + NC_CHECK_ERRMEM_GOTO(!addr_map, ret = -1, cleanup); + port_map = malloc(bind_count * sizeof *port_map); + NC_CHECK_ERRMEM_GOTO(!port_map, ret = -1, cleanup); + idx_map = malloc(bind_count * sizeof *idx_map); + NC_CHECK_ERRMEM_GOTO(!idx_map, ret = -1, cleanup); /* fill the arrays */ for (i = 0; i < bind_count; ++i) { - if (binds[i].sock < 0) { + if (binds_aux[i].sock < 0) { /* invalid socket */ continue; } - pollfds[pollfd_count].fd = binds[i].sock; + pollfds[pollfd_count].fd = binds_aux[i].sock; pollfds[pollfd_count].events = POLLIN; pollfds[pollfd_count].revents = 0; - bind_map[pollfd_count] = &binds[i]; + addr_map[pollfd_count] = binds[i].address; + port_map[pollfd_count] = binds[i].port; + idx_map[pollfd_count] = i; ++pollfd_count; } - ret = nc_sock_accept_pollfds(pollfds, pollfd_count, NULL, bind_map, timeout, host, port, &fd_idx, sock); + ret = nc_sock_accept_pollfds(pollfds, pollfd_count, addr_map, port_map, timeout, host, port, &fd_idx, sock); if (bind_idx && (ret > 0)) { - *bind_idx = bind_map[fd_idx] - binds; + *bind_idx = idx_map[fd_idx]; } cleanup: free(pollfds); - free(bind_map); + free(addr_map); + free(port_map); + free(idx_map); return ret; } @@ -1277,6 +1533,19 @@ nc_server_init(void) goto error; } + if (nc_server_init_rwlock(&server_opts.opts_lock)) { + goto error; + } + + /* allocate the initial empty configuration generation, its reference belongs to server_opts.config */ + server_opts.config = calloc(1, sizeof *server_opts.config); + if (!server_opts.config) { + ERRMEM; + goto error; + } + ATOMIC_STORE_RELAXED(server_opts.config->refcount, 1); + ATOMIC_STORE_RELAXED(server_opts.idle_timeout, 0); + #ifdef NC_ENABLED_SSH_TLS if (curl_global_init(CURL_GLOBAL_SSL | CURL_GLOBAL_ACK_EINTR)) { ERR(NULL, "%s: failed to init CURL.", __func__); @@ -1285,7 +1554,7 @@ nc_server_init(void) if (nc_tls_backend_init_wrap()) { ERR(NULL, "%s: failed to init the SSL library backend.", __func__); - return -1; + goto error; } /* optional for dynamic library, mandatory for static */ @@ -1310,6 +1579,10 @@ nc_server_init(void) return 0; error: + /* the server is not initialized, do not leave a configuration generation behind */ + nc_server_config_release(server_opts.config); + server_opts.config = NULL; + ATOMIC_STORE_RELAXED(server_opts.new_session_id, 0); return -1; } @@ -1317,10 +1590,16 @@ API int nc_server_destroy(void) { int rc = 0; - int config_update_locked = 0; - enum nc_rwlock_mode config_lock_mode = NC_RWLOCK_NONE; + int config_update_locked = 0, opts_locked = 0; + struct nc_server_config *config; uint32_t i; +#ifdef NC_ENABLED_SSH_TLS + void *interactive_auth_data; + + void (*interactive_auth_data_free)(void *data); +#endif /* NC_ENABLED_SSH_TLS */ + for (i = 0; i < server_opts.capabilities_count; i++) { free(server_opts.capabilities[i]); } @@ -1343,30 +1622,28 @@ nc_server_destroy(void) /* CONFIG UPDATE LOCK - the same timeout as the appliers use, destroying the server must not * fail just because a legitimate configuration apply is in progress */ - if (nc_mutex_lock(&server_opts.config_update_lock, NC_CONFIG_APPLY_LOCK_TIMEOUT, __func__) != 1) { + if (nc_mutex_lock(&server_opts.config_update_lock, NC_CONFIG_UPDATE_LOCK_TIMEOUT, __func__) != 1) { rc = 1; goto cleanup; } config_update_locked = 1; - /* CONFIG WR LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { - rc = 1; - goto cleanup; - } - config_lock_mode = NC_RWLOCK_WRITE; - #ifdef NC_ENABLED_SSH_TLS - /* stop all dispatched CH threads */ - LY_ARRAY_FOR(server_opts.config.ch_clients, i) { - if ((rc = nc_session_server_ch_client_dispatch_stop(&server_opts.config.ch_clients[i]))) { - goto cleanup; - } + /* stop all dispatched CH threads, no configuration lock may be held while joining them */ + if ((rc = nc_server_ch_threads_destroy())) { + goto cleanup; } #endif /* NC_ENABLED_SSH_TLS */ - /* destroy the server configuration */ - nc_server_config_free(&server_opts.config); + /* stop listening on all the registered sockets */ + nc_server_binds_destroy(); + + /* OPTS WRITE LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_WRITE, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { + rc = 1; + goto cleanup; + } + opts_locked = 1; #ifdef NC_ENABLED_SSH_TLS free(server_opts.authkey_path_fmt); @@ -1375,11 +1652,12 @@ nc_server_destroy(void) server_opts.pam_config_name = NULL; free(server_opts.ssh_protocol_string); server_opts.ssh_protocol_string = NULL; - if (server_opts.interactive_auth_data && server_opts.interactive_auth_data_free) { - server_opts.interactive_auth_data_free(server_opts.interactive_auth_data); - } + server_opts.interactive_auth_clb = NULL; + interactive_auth_data = server_opts.interactive_auth_data; + interactive_auth_data_free = server_opts.interactive_auth_data_free; server_opts.interactive_auth_data = NULL; server_opts.interactive_auth_data_free = NULL; + server_opts.user_verify_clb = NULL; /* Call Home dispatch data, its callback data does not have to be valid once the server is destroyed */ memset(&server_opts.ch_dispatch_data, 0, sizeof server_opts.ch_dispatch_data); @@ -1392,6 +1670,34 @@ nc_server_destroy(void) } LY_ARRAY_FREE(server_opts.unix_paths); server_opts.unix_paths = NULL; + free(server_opts.unix_socket_dir); + server_opts.unix_socket_dir = NULL; + + /* OPTS WRITE UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); + opts_locked = 0; + +#ifdef NC_ENABLED_SSH_TLS + /* free the user data only once the lock is released, the callback may call back into the library */ + if (interactive_auth_data && interactive_auth_data_free) { + interactive_auth_data_free(interactive_auth_data); + } +#endif /* NC_ENABLED_SSH_TLS */ + + /* CONFIG WR LOCK - unpublish the configuration, a concurrent acquire must not see a stale pointer */ + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + rc = 1; + goto cleanup; + } + config = server_opts.config; + server_opts.config = NULL; + ATOMIC_STORE_RELAXED(server_opts.idle_timeout, 0); + + /* CONFIG UNLOCK */ + nc_rwlock_unlock(&server_opts.config_lock, __func__); + + /* the configuration is destroyed once its last reader releases it */ + nc_server_config_release(config); #ifdef NC_ENABLED_SSH_TLS curl_global_cleanup(); @@ -1406,8 +1712,8 @@ nc_server_destroy(void) #endif /* NC_ENABLED_SSH_TLS */ cleanup: - if (config_lock_mode != NC_RWLOCK_NONE) { - nc_rwlock_unlock(&server_opts.config_lock, __func__); + if (opts_locked) { + nc_rwlock_unlock(&server_opts.opts_lock, __func__); } if (config_update_locked) { nc_mutex_unlock(&server_opts.config_update_lock, __func__); @@ -2354,8 +2660,8 @@ nc_ps_poll_session_io(struct nc_session *session, int io_timeout, time_t now_mon #endif #endif /* NC_ENABLED_SSH_TLS */ - /* check timeout first */ - idle_timeout = server_opts.config.idle_timeout; + /* check timeout first, read the mirror so that the poll path needs no configuration at all */ + idle_timeout = (uint16_t)ATOMIC_LOAD_RELAXED(server_opts.idle_timeout); if (!(session->flags & NC_SESSION_CALLHOME) && !nc_session_get_notif_status(session) && idle_timeout && (now_mono >= session->opts.server.last_rpc + idle_timeout)) { sprintf(msg, "Session idle timeout elapsed"); @@ -2521,19 +2827,9 @@ nc_ps_poll_sess(struct nc_ps_session *ps_session, time_t now_mono) switch (ps_session->state) { case NC_PS_STATE_NONE: if (ps_session->session->status == NC_STATUS_RUNNING) { - /* session is fine, work with it */ + /* session is fine, work with it, no configuration is accessed */ ps_session->state = NC_PS_STATE_BUSY; - - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { - ps_session->state = NC_PS_STATE_NONE; - ret = NC_PSPOLL_ERROR; - break; - } else { - ret = nc_ps_poll_session_io(ps_session->session, NC_SESSION_LOCK_TIMEOUT, now_mono, msg); - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); - } + ret = nc_ps_poll_session_io(ps_session->session, NC_SESSION_LOCK_TIMEOUT, now_mono, msg); switch (ret) { case NC_PSPOLL_SESSION_TERM | NC_PSPOLL_SESSION_ERROR: @@ -2766,87 +3062,376 @@ nc_ps_clear(struct nc_pollsession *ps, int all, void (*data_free)(void *)) nc_ps_unlock(ps, q_id, __func__); } -int -nc_server_bind_and_listen(struct nc_endpt *endpt, struct nc_bind *bind) +/** + * @brief Start listening on a socket of an endpoint bind. + * + * @param[in] endpt Endpoint the bind belongs to. + * @param[in] address Address to listen on, the full socket path for a UNIX endpoint. + * @param[in] port Port to listen on, 0 for a UNIX endpoint. + * @param[out] sock Created listening socket. + * @return 0 on success, 1 on error. + */ +static int +nc_server_bind_and_listen(const struct nc_endpt *endpt, const char *address, uint16_t port, int *sock) { - char *unix_path = NULL; - int sock = -1, rc = 0; - - /* start listening on the endpoint */ - if (endpt->ti == NC_TI_UNIX) { - /* get the socket path for this endpoint */ - unix_path = nc_server_unix_get_socket_path(endpt); - NC_CHECK_ERR_GOTO(!unix_path, rc = 1, cleanup); - sock = nc_sock_listen_unix(unix_path, endpt->opts.unix); - } else { - assert(bind->address && bind->port); - sock = nc_sock_listen_inet(bind->address, bind->port); - } - if (sock == -1) { - rc = 1; - goto cleanup; - } +#ifndef NC_ENABLED_SSH_TLS + /* only UNIX endpoints exist, which have no port */ + (void)port; +#endif - /* close the old socket if any and store the new one */ - if (bind->sock > -1) { - close(bind->sock); - } - bind->sock = sock; + *sock = -1; switch (endpt->ti) { case NC_TI_UNIX: - VRB(NULL, "Listening on %s for UNIX connections.", unix_path); + *sock = nc_sock_listen_unix(address, endpt->opts.unix); + NC_CHECK_RET(*sock == -1, 1); + VRB(NULL, "Listening on %s for UNIX connections.", address); break; #ifdef NC_ENABLED_SSH_TLS case NC_TI_SSH: - VRB(NULL, "Listening on %s:%u for SSH connections.", bind->address, bind->port); + *sock = nc_sock_listen_inet(address, port); + NC_CHECK_RET(*sock == -1, 1); + VRB(NULL, "Listening on %s:%" PRIu16 " for SSH connections.", address, port); break; case NC_TI_TLS: - VRB(NULL, "Listening on %s:%u for TLS connections.", bind->address, bind->port); + *sock = nc_sock_listen_inet(address, port); + NC_CHECK_RET(*sock == -1, 1); + VRB(NULL, "Listening on %s:%" PRIu16 " for TLS connections.", address, port); break; #endif /* NC_ENABLED_SSH_TLS */ default: ERRINT; - rc = 1; - break; + return 1; } -cleanup: - free(unix_path); - return rc; + return 0; } /** - * @brief Read the NETCONF user of a UNIX transport session. + * @brief Stop listening on a socket that was opened for a bind description. * - * @param[in] session NETCONF session for logging. - * @param[in] sock Socket to read from. - * @param[out] username Read NETCONF username. - * @return 1 on success, 0 on timeout, -1 on error. + * @param[in,out] desc Bind description to close the socket of, no-op if it has none. */ -static int -nc_accept_unix_read_username(struct nc_session *session, int sock, char **username) +static void +nc_server_bind_desc_close(struct nc_bind_desc *desc) { - struct timespec ts_timeout; - size_t size = 32, rr = 0; - ssize_t r; - - assert(sock > -1); + if (desc->sock == -1) { + return; + } - /* fill timespec */ - nc_timeouttime_get(&ts_timeout, NC_TRANSPORT_MSG_TIMEOUT); + close(desc->sock); + desc->sock = -1; + if (desc->endpt->ti == NC_TI_UNIX) { + /* remove the socket file we have just created */ + unlink(desc->address); + } +} - /* prepare username */ - *username = malloc(size); - NC_CHECK_ERRMEM_RET(!*username, -1); +/** + * @brief Stop listening on a registered socket and free the registry entry members. + * + * @note The bind registry lock must be held. + * + * @param[in] entry Bind registry entry to close. + */ +static void +nc_server_bind_entry_close(struct nc_bind_entry *entry) +{ + close(entry->sock); + if (entry->ti == NC_TI_UNIX) { + /* remove the socket file */ + unlink(entry->address); + VRB(NULL, "Stopped listening on %s.", entry->address); + } else { + VRB(NULL, "Stopped listening on %s:%" PRIu16 ".", entry->address, entry->port); + } - while (1) { - /* realloc as needed */ - if (size == rr) { - size *= 2; - *username = nc_realloc(*username, size); - NC_CHECK_ERRMEM_RET(!*username, -1); - } + free(entry->endpt_name); + free(entry->address); +} + +/** + * @brief Check whether a bind registry entry refers to the same listening socket as a bind description. + * + * @param[in] entry Bind registry entry. + * @param[in] desc Bind description. + * @return 1 if they match, 0 otherwise. + */ +static int +nc_server_bind_entry_matches(const struct nc_bind_entry *entry, const struct nc_bind_desc *desc) +{ + return (entry->ti == desc->endpt->ti) && (entry->port == desc->port) && !strcmp(entry->address, desc->address); +} + +/** + * @brief Collect the listening sockets required by a server configuration. + * + * @param[in] config Server configuration. + * @param[out] descs Bind descriptions (sized-array, see libyang docs). + * @return 0 on success, 1 on error. + */ +static int +nc_server_bind_descs_get(const struct nc_server_config *config, struct nc_bind_desc **descs) +{ + int rc = 0; + const struct nc_endpt *endpt; + const struct nc_bind *bind; + struct nc_bind_desc *desc; + LY_ARRAY_COUNT_TYPE u, v; + uint32_t count = 0; + + *descs = NULL; + + LY_ARRAY_FOR(config->endpts, u) { + count += LY_ARRAY_COUNT(config->endpts[u].binds); + } + if (!count) { + return 0; + } + LY_ARRAY_CREATE_GOTO(NULL, *descs, count, rc, cleanup); + + LY_ARRAY_FOR(config->endpts, u) { + endpt = &config->endpts[u]; + + LY_ARRAY_FOR(endpt->binds, v) { + bind = &endpt->binds[v]; + + desc = &(*descs)[LY_ARRAY_COUNT(*descs)]; + desc->endpt = endpt; + desc->port = bind->port; + desc->sock = -1; + + if (endpt->ti == NC_TI_UNIX) { + /* the socket path is not stored in the bind, resolve it */ + desc->address = nc_server_unix_get_socket_path(endpt); + NC_CHECK_ERR_GOTO(!desc->address, rc = 1, cleanup); + } else { + assert(bind->address && bind->port); + desc->address = strdup(bind->address); + NC_CHECK_ERRMEM_GOTO(!desc->address, rc = 1, cleanup); + } + + LY_ARRAY_INCREMENT(*descs); + } + } + +cleanup: + return rc ? 1 : 0; +} + +/** + * @brief Free bind descriptions and close all the sockets they still own. + * + * @param[in] descs Bind descriptions to free. + */ +static void +nc_server_bind_descs_free(struct nc_bind_desc *descs) +{ + LY_ARRAY_COUNT_TYPE u; + + LY_ARRAY_FOR(descs, u) { + nc_server_bind_desc_close(&descs[u]); + free(descs[u].address); + free(descs[u].rename); + } + LY_ARRAY_FREE(descs); +} + +int +nc_server_binds_reconcile(const struct nc_server_config *config) +{ + int rc = 0, binds_locked = 0, found; + struct nc_bind_desc *descs = NULL; + struct nc_bind_entry *entry; + char *endpt_name, *address; + LY_ARRAY_COUNT_TYPE u, v, added = 0; + uint32_t new_count = 0; + + /* collect all the listening sockets the configuration requires, no lock is needed for that */ + NC_CHECK_GOTO(rc = nc_server_bind_descs_get(config, &descs), cleanup); + + /* BINDS LOCK */ + if (nc_mutex_lock(&server_opts.binds_lock, NC_BINDS_LOCK_TIMEOUT, __func__) != 1) { + rc = 1; + goto cleanup; + } + binds_locked = 1; + + /* keep listening on the sockets that are already registered */ + LY_ARRAY_FOR(descs, u) { + LY_ARRAY_FOR(server_opts.binds, v) { + if (!nc_server_bind_entry_matches(&server_opts.binds[v], &descs[u])) { + continue; + } + + descs[u].reused = 1; + descs[u].entry_idx = v; + + /* the socket stays open, but the endpoint owning it may have been renamed, prepare the + * new name and store it only once nothing can fail anymore */ + if (strcmp(server_opts.binds[v].endpt_name, descs[u].endpt->name)) { + descs[u].rename = strdup(descs[u].endpt->name); + NC_CHECK_ERRMEM_GOTO(!descs[u].rename, rc = 1, cleanup); + } + break; + } + + if (!descs[u].reused) { + ++new_count; + } + } + + /* BINDS UNLOCK - creating the sockets may take a while */ + nc_mutex_unlock(&server_opts.binds_lock, __func__); + binds_locked = 0; + + /* start listening on the sockets that are not registered yet */ + LY_ARRAY_FOR(descs, u) { + if (descs[u].reused) { + continue; + } + + NC_CHECK_GOTO(rc = nc_server_bind_and_listen(descs[u].endpt, descs[u].address, descs[u].port, + &descs[u].sock), cleanup); + } + + /* BINDS LOCK */ + if (nc_mutex_lock(&server_opts.binds_lock, NC_BINDS_LOCK_TIMEOUT, __func__) != 1) { + rc = 1; + goto cleanup; + } + binds_locked = 1; + + /* register the new sockets, reserve the space in advance */ + if (new_count) { + LY_ARRAY_CREATE_GOTO(NULL, server_opts.binds, new_count, rc, cleanup); + } + LY_ARRAY_FOR(descs, u) { + if (descs[u].reused) { + continue; + } + + endpt_name = strdup(descs[u].endpt->name); + NC_CHECK_ERRMEM_GOTO(!endpt_name, rc = 1, cleanup); + address = strdup(descs[u].address); + NC_CHECK_ERRMEM_GOTO(!address, free(endpt_name); rc = 1, cleanup); + + entry = &server_opts.binds[LY_ARRAY_COUNT(server_opts.binds)]; + entry->endpt_name = endpt_name; + entry->address = address; + entry->port = descs[u].port; + entry->ti = descs[u].endpt->ti; + entry->sock = descs[u].sock; + + /* the socket now belongs to the registry */ + descs[u].sock = -1; + LY_ARRAY_INCREMENT(server_opts.binds); + ++added; + } + + /* the registry entries did not move, so store the new endpoint names now that nothing can fail */ + LY_ARRAY_FOR(descs, u) { + if (!descs[u].rename) { + continue; + } + + entry = &server_opts.binds[descs[u].entry_idx]; + free(entry->endpt_name); + entry->endpt_name = descs[u].rename; + descs[u].rename = NULL; + } + + /* stop listening on the sockets the configuration no longer contains */ + v = 0; + while (v < LY_ARRAY_COUNT(server_opts.binds)) { + found = 0; + LY_ARRAY_FOR(descs, u) { + if (nc_server_bind_entry_matches(&server_opts.binds[v], &descs[u])) { + found = 1; + break; + } + } + if (found) { + ++v; + continue; + } + + nc_server_bind_entry_close(&server_opts.binds[v]); + + /* swap the last entry into the hole, the order of the registry is irrelevant */ + server_opts.binds[v] = server_opts.binds[LY_ARRAY_COUNT(server_opts.binds) - 1]; + LY_ARRAY_DECREMENT_FREE(server_opts.binds); + } + +cleanup: + if (rc) { + /* unregister the sockets we have just registered, they are always the last ones */ + while (added) { + entry = &server_opts.binds[LY_ARRAY_COUNT(server_opts.binds) - 1]; + nc_server_bind_entry_close(entry); + LY_ARRAY_DECREMENT_FREE(server_opts.binds); + --added; + } + } + if (binds_locked) { + /* BINDS UNLOCK */ + nc_mutex_unlock(&server_opts.binds_lock, __func__); + } + nc_server_bind_descs_free(descs); + return rc ? 1 : 0; +} + +void +nc_server_binds_destroy(void) +{ + LY_ARRAY_COUNT_TYPE u; + + /* BINDS LOCK */ + if (nc_mutex_lock(&server_opts.binds_lock, NC_BINDS_LOCK_TIMEOUT, __func__) != 1) { + return; + } + + LY_ARRAY_FOR(server_opts.binds, u) { + nc_server_bind_entry_close(&server_opts.binds[u]); + } + LY_ARRAY_FREE(server_opts.binds); + server_opts.binds = NULL; + + /* BINDS UNLOCK */ + nc_mutex_unlock(&server_opts.binds_lock, __func__); +} + +/** + * @brief Read the NETCONF user of a UNIX transport session. + * + * @param[in] session NETCONF session for logging. + * @param[in] sock Socket to read from. + * @param[out] username Read NETCONF username. + * @return 1 on success, 0 on timeout, -1 on error. + */ +static int +nc_accept_unix_read_username(struct nc_session *session, int sock, char **username) +{ + struct timespec ts_timeout; + size_t size = 32, rr = 0; + ssize_t r; + + assert(sock > -1); + + /* fill timespec */ + nc_timeouttime_get(&ts_timeout, NC_TRANSPORT_MSG_TIMEOUT); + + /* prepare username */ + *username = malloc(size); + NC_CHECK_ERRMEM_RET(!*username, -1); + + while (1) { + /* realloc as needed */ + if (size == rr) { + size *= 2; + *username = nc_realloc(*username, size); + NC_CHECK_ERRMEM_RET(!*username, -1); + } /* read */ r = read(sock, *username + rr, 1); @@ -3001,18 +3586,17 @@ nc_accept_unix_session(struct nc_session *session, int sock) API uint32_t nc_server_endpt_count(void) { + const struct nc_server_config *config; uint32_t cnt; - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + config = nc_server_config_acquire(); + if (!config) { return 0; } - cnt = LY_ARRAY_COUNT(server_opts.config.endpts); - - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + cnt = LY_ARRAY_COUNT(config->endpts); + nc_server_config_release(config); return cnt; } @@ -3025,7 +3609,7 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) uint16_t port = 0; struct timespec ts_cur; LY_ARRAY_COUNT_TYPE endpt_idx; - struct nc_server_config *config; + const struct nc_server_config *config; NC_CHECK_ARG_RET(NULL, ctx, session, NC_MSG_ERROR); @@ -3036,13 +3620,12 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) /* init ctx as needed */ nc_server_init_cb_ctx(ctx); - /* CONFIG LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* pin the configuration for the whole accept, no lock is held for any of it */ + config = nc_server_config_acquire(); + if (!config) { return NC_MSG_ERROR; } - config = &server_opts.config; - if (!config->endpts) { ERR(NULL, "No endpoints to accept sessions on."); msgtype = NC_MSG_ERROR; @@ -3075,6 +3658,9 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) host = NULL; (*session)->port = port; + /* pin the configuration for the duration of the transport handshake, it is a borrowed pointer */ + (*session)->opts.server.config = config; + /* sock gets assigned to session or closed */ #ifdef NC_ENABLED_SSH_TLS if (config->endpts[endpt_idx].ti == NC_TI_SSH) { @@ -3116,8 +3702,12 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) (*session)->data = NULL; - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* the transport handshake is over, the configuration must not be reached through the session anymore */ + (*session)->opts.server.config = NULL; + + /* the NETCONF hello needs no configuration */ + nc_server_config_release(config); + config = NULL; /* assign new SID atomically */ (*session)->id = ATOMIC_INC_RELAXED(server_opts.new_session_id); @@ -3139,15 +3729,16 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) return msgtype; cleanup: - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); - free(host); if (sock > -1) { close(sock); } + if (*session) { + (*session)->opts.server.config = NULL; + } nc_session_free(*session, NULL); *session = NULL; + nc_server_config_release(config); return msgtype; } @@ -3156,71 +3747,66 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) API int nc_server_ch_is_client(const char *name) { - struct nc_ch_client *client; + const struct nc_server_config *config; int found = 0; if (!name) { return found; } - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + config = nc_server_config_acquire(); + if (!config) { return found; } /* check name against all configured clients */ - LY_ARRAY_FOR(server_opts.config.ch_clients, struct nc_ch_client, client) { - if (!strcmp(client->name, name)) { - found = 1; - break; - } + if (nc_server_ch_client_get_pinned(config, name)) { + found = 1; } - /* CONFIG READ UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); - + nc_server_config_release(config); return found; } API int nc_server_ch_client_is_endpt(const char *client_name, const char *endpt_name) { - struct nc_ch_client *client = NULL; - struct nc_ch_endpt *endpt = NULL; + const struct nc_server_config *config; + const struct nc_ch_client *client; + LY_ARRAY_COUNT_TYPE u; int found = 0; if (!client_name || !endpt_name) { return found; } - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + config = nc_server_config_acquire(); + if (!config) { return found; } - client = nc_server_ch_client_get(client_name); + client = nc_server_ch_client_get_pinned(config, client_name); if (!client) { goto cleanup; } - LY_ARRAY_FOR(client->ch_endpts, struct nc_ch_endpt, endpt) { - if (!strcmp(endpt->name, endpt_name)) { + LY_ARRAY_FOR(client->ch_endpts, u) { + if (!strcmp(client->ch_endpts[u].name, endpt_name)) { found = 1; goto cleanup; } } cleanup: - /* CONFIG READ UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + nc_server_config_release(config); return found; } /** * @brief Create a connection for an endpoint. * - * Config read lock must be held - the configuration is being read. - * + * @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,out] cur_sock_pending Current pending socket for the connection. * @param[in] acquire_ctx_cb Callback for acquiring the libyang context. @@ -3230,7 +3816,7 @@ 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(struct nc_ch_endpt *endpt, int *cur_sock_pending, +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) { @@ -3267,6 +3853,9 @@ nc_connect_ch_endpt(struct nc_ch_endpt *endpt, int *cur_sock_pending, (*session)->host = ip_host; (*session)->port = endpt->dst_port; + /* pin the configuration for the duration of the transport handshake, it is a borrowed pointer */ + (*session)->opts.server.config = config; + /* sock gets assigned to session or closed */ if (endpt->ti == NC_TI_SSH) { ret = nc_accept_ssh_session(*session, endpt->opts.ssh, sock); @@ -3298,6 +3887,9 @@ nc_connect_ch_endpt(struct nc_ch_endpt *endpt, int *cur_sock_pending, goto fail; } + /* the transport handshake is over, the configuration must not be reached through the session anymore */ + (*session)->opts.server.config = NULL; + /* assign new SID atomically */ (*session)->id = ATOMIC_INC_RELAXED(server_opts.new_session_id); @@ -3316,6 +3908,9 @@ nc_connect_ch_endpt(struct nc_ch_endpt *endpt, int *cur_sock_pending, return msgtype; fail: + if (*session) { + (*session)->opts.server.config = NULL; + } nc_session_free(*session, NULL); *session = NULL; if (ctx) { @@ -3327,37 +3922,33 @@ nc_connect_ch_endpt(struct nc_ch_endpt *endpt, int *cur_sock_pending, /** * @brief Get idle timeout for a Call Home client. * + * A client that is not (yet) part of the published configuration simply has no idle timeout, the + * lifetime of its thread is decided by ::nc_server_ch_thread_arg.thread_running only. + * * @param[in] client_name Name of the Call Home client. - * @param[out] idle_timeout Idle timeout in seconds. - * @return 0 on success, 1 if the client was not found, -1 on error. + * @param[out] idle_timeout Idle timeout in seconds, 0 for none. + * @return 0 on success, -1 on error. */ static int nc_server_ch_client_get_idle_timeout(const char *client_name, uint32_t *idle_timeout) { - int ret = 0; - struct nc_ch_client *client; + const struct nc_server_config *config; + const struct nc_ch_client *client; - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { - return -1; - } + *idle_timeout = 0; - client = nc_server_ch_client_get(client_name); - if (!client) { - ret = 1; - goto cleanup; + config = nc_server_config_acquire(); + if (!config) { + return -1; } - if (client->conn_type == NC_CH_PERIOD) { + client = nc_server_ch_client_get_pinned(config, client_name); + if (client && (client->conn_type == NC_CH_PERIOD)) { *idle_timeout = client->idle_timeout; - } else { - *idle_timeout = 0; } -cleanup: - /* CONFIG READ UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); - return ret; + nc_server_config_release(config); + return 0; } /** @@ -3430,14 +4021,8 @@ nc_server_ch_client_thread_session_cond_wait(struct nc_server_ch_thread_arg *dat terminate = 0; - /* check if the client still exists and get its idle timeout */ - r = nc_server_ch_client_get_idle_timeout(data->client_name, &idle_timeout); - if (r) { - if (r == 1) { - /* the client must always be found, because if we delete it, then the configuring thread calls - * pthread_join() on this thread with the old config where the client still exists */ - ERRINT; - } + /* get the client's idle timeout */ + if (nc_server_ch_client_get_idle_timeout(data->client_name, &idle_timeout)) { rc = -1; terminate = 1; } @@ -3567,41 +4152,49 @@ nc_server_ch_client_thread_wait(struct nc_session *session, struct nc_server_ch_ } /** - * @brief Wait for a Call Home client to have at least one endpoint defined. + * @brief Acquire a configuration in which the Call Home client has at least one endpoint defined. * - * @note The configuration read lock is expected to be held. + * A client that is missing from the published configuration is waited for the same way as a client + * with no endpoints - it may simply not have been published yet, so the thread is normally only + * ever stopped by clearing ::nc_server_ch_thread_arg.thread_running. A configuration that cannot be + * acquired at all is retried a few times as well, but not forever - it means either a wedged + * configuration lock or a server destroyed without stopping this thread first. * * @param[in] data Call Home client thread argument. - * @param[in] name Name of the CH client. - * @return Pointer to the CH client, NULL if the client was removed. + * @param[out] client Found Call Home client of the returned configuration. + * @return Pinned server configuration, the caller must release it. + * @return NULL if the thread should stop running. */ -static struct nc_ch_client * -nc_server_ch_client_with_endpt_get(struct nc_server_ch_thread_arg *data, const char *name) +static const struct nc_server_config * +nc_server_ch_client_acquire_with_endpt(struct nc_server_ch_thread_arg *data, const struct nc_ch_client **client) { - struct nc_ch_client *client; + const struct nc_server_config *config; + uint32_t failed_attempts = 0; + + *client = NULL; while (ATOMIC_LOAD_RELAXED(data->thread_running)) { - /* get the client */ - client = nc_server_ch_client_get(name); - if (!client) { - return NULL; - } + config = nc_server_config_acquire(); + if (config) { + failed_attempts = 0; + + *client = nc_server_ch_client_get_pinned(config, data->client_name); + if (*client && (*client)->ch_endpts) { + /* the client is configured and has at least one endpoint */ + return config; + } - /* check if it has at least one endpoint defined */ - if (client->ch_endpts) { - return client; + /* not configured (yet) or no endpoints defined yet */ + nc_server_config_release(config); + *client = NULL; + } else if (++failed_attempts == NC_CH_CONFIG_ACQUIRE_ATTEMPTS) { + ERR(NULL, "Call Home client \"%s\" failed to acquire the server configuration %d times, " + "terminating its thread.", data->client_name, NC_CH_CONFIG_ACQUIRE_ATTEMPTS); + return NULL; } - /* CONFIG READ UNLOCK - allow another thread to modify the configuration */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); - - /* no endpoints defined yet, wait a little bit */ + /* the configuration is not usable (yet), wait a little bit and try again */ usleep(NC_CH_NO_ENDPT_WAIT * 1000); - - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { - return NULL; - } } /* thread is not running */ @@ -3611,6 +4204,9 @@ nc_server_ch_client_with_endpt_get(struct nc_server_ch_thread_arg *data, const c /** * @brief Call Home client management thread. * + * Runs until ::nc_server_ch_thread_arg.thread_running is cleared or an unrecoverable error occurs. + * In the latter case it unregisters itself, see ::nc_server_ch_thread_unreg_self(). + * * @param[in] arg CH client thread argument. * @return NULL. */ @@ -3620,44 +4216,43 @@ nc_ch_client_thread(void *arg) struct nc_server_ch_thread_arg *data = arg; NC_MSG_TYPE msgtype; int cur_sock_pending = -1, r; - uint8_t cur_attempts = 0, max_attempts; - uint16_t next_endpt_index, max_wait; + uint8_t cur_attempts = 0, max_attempts = 0; + uint16_t next_endpt_index, max_wait = 0, period = 0; char *cur_endpt_name = NULL; - struct nc_ch_endpt *cur_endpt; + const struct nc_server_config *config = NULL; + const struct nc_ch_client *client; + const struct nc_ch_endpt *cur_endpt; struct nc_session *session = NULL; - struct nc_ch_client *client; uint32_t reconnect_in; + NC_CH_CONN_TYPE conn_type; + NC_CH_START_WITH start_with; + time_t anchor_time; - /* mark the thread as running */ - ATOMIC_STORE_RELAXED(data->thread_running, 1); - - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* get the client once it is configured with at least one endpoint */ + config = nc_server_ch_client_acquire_with_endpt(data, &client); + if (!config) { goto cleanup; } - /* get the client once it has at least one endpoint */ - client = nc_server_ch_client_with_endpt_get(data, data->client_name); - if (!client) { - VRB(NULL, "Call Home client \"%s\" removed.", data->client_name); - goto cleanup_unlock; - } - - /* config is still locked and ch client has at least 1 endpoint, so select the first one */ + /* the client has at least 1 endpoint, so select the first one */ cur_endpt = &client->ch_endpts[0]; cur_endpt_name = strdup(cur_endpt->name); + NC_CHECK_ERRMEM_GOTO(!cur_endpt_name, , cleanup); while (ATOMIC_LOAD_RELAXED(data->thread_running)) { if (!cur_attempts) { VRB(NULL, "Call Home client \"%s\" endpoint \"%s\" connecting...", data->client_name, cur_endpt_name); } - /* try to connect to the endpoint */ - msgtype = nc_connect_ch_endpt(cur_endpt, &cur_sock_pending, data->acquire_ctx_cb, data->release_ctx_cb, - data->ctx_cb_data, &session); + /* 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); if (msgtype == NC_MSG_HELLO) { - /* CONFIG READ UNLOCK - session established */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* session established, the configuration is not needed anymore */ + nc_server_config_release(config); + config = NULL; + client = NULL; + cur_endpt = NULL; if (!ATOMIC_LOAD_RELAXED(data->thread_running)) { /* thread should stop running */ @@ -3677,56 +4272,50 @@ nc_ch_client_thread(void *arg) goto cleanup; } - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* get the client again, it may have been changed */ + config = nc_server_ch_client_acquire_with_endpt(data, &client); + if (!config) { goto cleanup; } - /* get the client again, it may have been removed */ - client = nc_server_ch_client_with_endpt_get(data, data->client_name); - if (!client) { - VRB(NULL, "Call Home client \"%s\" removed.", data->client_name); - goto cleanup_unlock; - } - /* session changed status -> it was disconnected for whatever reason, * persistent connection immediately tries to reconnect, periodic connects at specific times */ - if (client->conn_type == NC_CH_PERIOD) { - if (client->anchor_time) { + conn_type = client->conn_type; + period = client->period; + anchor_time = client->anchor_time; + if (conn_type == NC_CH_PERIOD) { + if (anchor_time) { /* anchored */ - reconnect_in = (time(NULL) - client->anchor_time) % (client->period * 60); + reconnect_in = (time(NULL) - anchor_time) % (period * 60); } else { /* fixed timeout */ - reconnect_in = client->period * 60; + reconnect_in = period * 60; } - /* CONFIG READ UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* the configuration is not needed while waiting */ + nc_server_config_release(config); + config = NULL; + client = NULL; /* wait for the timeout to elapse, so we can try to reconnect */ - VRB(session, "Call Home client \"%s\" reconnecting in %" PRIu32 " seconds.", data->client_name, reconnect_in); - r = nc_server_ch_client_thread_wait(session, data, reconnect_in, NULL); + VRB(NULL, "Call Home client \"%s\" reconnecting in %" PRIu32 " seconds.", data->client_name, reconnect_in); + r = nc_server_ch_client_thread_wait(NULL, data, reconnect_in, NULL); if (r == -1) { goto cleanup; } - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + config = nc_server_ch_client_acquire_with_endpt(data, &client); + if (!config) { goto cleanup; } - - client = nc_server_ch_client_with_endpt_get(data, data->client_name); - if (!client) { - VRB(NULL, "Call Home client \"%s\" removed.", data->client_name); - goto cleanup_unlock; - } } /* set next endpoint to try */ - if (client->start_with == NC_CH_FIRST_LISTED) { + start_with = client->start_with; + if (start_with == NC_CH_FIRST_LISTED) { next_endpt_index = 0; - } else if (client->start_with == NC_CH_LAST_CONNECTED) { - /* we keep the current one but due to unlock/lock we have to find it again */ + } else if (start_with == NC_CH_LAST_CONNECTED) { + /* we keep the current one but due to the release/acquire we have to find it again */ LY_ARRAY_FOR(client->ch_endpts, next_endpt_index) { if (!strcmp(client->ch_endpts[next_endpt_index].name, cur_endpt_name)) { break; @@ -3744,11 +4333,17 @@ nc_ch_client_thread(void *arg) } else { /* session was not created, wait a little bit and try again */ ++cur_attempts; + + /* copy what is needed after the configuration is released, the user callback and the + * wait must not run with a generation pinned */ max_wait = client->max_wait; max_attempts = client->max_attempts; - /* CONFIG READ UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* the configuration is not needed while waiting */ + nc_server_config_release(config); + config = NULL; + client = NULL; + cur_endpt = NULL; /* failed connection attempt */ if (data->new_session_fail_cb) { @@ -3757,7 +4352,7 @@ nc_ch_client_thread(void *arg) } /* wait for max_wait seconds */ - r = nc_server_ch_client_thread_wait(session, data, max_wait, &cur_sock_pending); + r = nc_server_ch_client_thread_wait(NULL, data, max_wait, &cur_sock_pending); if (r == -1) { /* thread should stop running */ goto cleanup; @@ -3770,16 +4365,10 @@ nc_ch_client_thread(void *arg) } /* if r == 1, socket is connected, keep cur_sock_pending for nc_connect_ch_endpt */ - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { - goto cleanup; - } - /* get the client */ - client = nc_server_ch_client_with_endpt_get(data, data->client_name); - if (!client) { - VRB(NULL, "Call Home client \"%s\" removed.", data->client_name); - goto cleanup_unlock; + config = nc_server_ch_client_acquire_with_endpt(data, &client); + if (!config) { + goto cleanup; } /* try to find our endpoint again */ @@ -3791,7 +4380,7 @@ nc_ch_client_thread(void *arg) if (next_endpt_index >= LY_ARRAY_COUNT(client->ch_endpts)) { /* endpoint was removed, start with the first one */ - VRB(session, "Call Home client \"%s\" endpoint \"%s\" removed.", data->client_name, cur_endpt_name); + VRB(NULL, "Call Home client \"%s\" endpoint \"%s\" removed.", data->client_name, cur_endpt_name); /* close pending socket to the removed endpoint, if any */ if (cur_sock_pending != -1) { @@ -3803,7 +4392,7 @@ nc_ch_client_thread(void *arg) cur_attempts = 0; } else if (cur_attempts == client->max_attempts) { /* we have tried to connect to this endpoint enough times */ - VRB(session, "Call Home client \"%s\" endpoint \"%s\" failed connection attempt limit %" PRIu8 " reached.", + VRB(NULL, "Call Home client \"%s\" endpoint \"%s\" failed connection attempt limit %" PRIu8 " reached.", data->client_name, cur_endpt_name, client->max_attempts); /* close pending socket, switching to a different endpoint */ @@ -3826,42 +4415,43 @@ nc_ch_client_thread(void *arg) cur_endpt = &client->ch_endpts[next_endpt_index]; free(cur_endpt_name); cur_endpt_name = strdup(cur_endpt->name); + NC_CHECK_ERRMEM_GOTO(!cur_endpt_name, , cleanup); } -cleanup_unlock: - /* CONFIG READ UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); - cleanup: - VRB(session, "Call Home client \"%s\" thread exit.", data->client_name); + /* the session, if there still is one, belongs to the user and may have been freed already, + * so it must not be logged through */ + VRB(NULL, "Call Home client \"%s\" thread exit.", data->client_name); + nc_server_config_release(config); free(cur_endpt_name); if (cur_sock_pending != -1) { close(cur_sock_pending); } + /* if we are terminating on our own, take ourselves out of the registry so that the client can + * be dispatched again, otherwise this is a no-op and whoever stopped us cleans up after us */ + nc_server_ch_thread_unreg_self(data); + return NULL; } int -nc_session_server_ch_client_dispatch_stop(struct nc_ch_client *ch_client) +nc_session_server_ch_client_dispatch_stop(const char *client_name) { - int rc = 0, r; + int r; struct nc_server_ch_thread_arg *thread_arg; - pthread_t tid; - enum nc_rwlock_mode config_lock_mode = NC_RWLOCK_WRITE; - char *ch_client_name = NULL; - if (!ch_client || !ch_client->thread) { + /* unregister the thread first, so that no other caller can find and join the same one */ + if (nc_server_ch_thread_reg_del(client_name, &thread_arg)) { + return 1; + } + if (!thread_arg) { + /* no thread is running for this client */ return 0; } - thread_arg = ch_client->thread; - ch_client_name = strdup(thread_arg->client_name); - NC_CHECK_ERRMEM_GOTO(!ch_client_name, rc = 1, cleanup); - /* notify the thread to stop */ ATOMIC_STORE_RELAXED(thread_arg->thread_running, 0); - tid = thread_arg->tid; /* wake up the thread if it's in thread_wait */ if (write(thread_arg->notify_pipe[1], "x", 1) == -1) { @@ -3871,78 +4461,87 @@ nc_session_server_ch_client_dispatch_stop(struct nc_ch_client *ch_client) /* EAGAIN is fine: pipe buffer is full, meaning it's already been signaled */ } - /* CONFIG UNLOCK - the caller must hold WRITE config lock, we need to unlock it - * to prevent deadlock with the CH thread, it tries to acquire the config lock in read mode when it - * checks if the client still exists. - * It is the caller's responsibility to hold config apply mutex as well, so noone steals the write lock from him */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); - config_lock_mode = NC_RWLOCK_NONE; - - /* wait for the thread to end */ - r = pthread_join(tid, NULL); + /* wait for the thread to end, no lock is held so a stalled handshake blocks nothing else */ + r = pthread_join(thread_arg->tid, NULL); if (r) { - ERR(NULL, "Joining Call Home client \"%s\" thread failed (%s).", ch_client_name, strerror(r)); - rc = 1; - goto cleanup; + ERR(NULL, "Joining Call Home client \"%s\" thread failed (%s), its data will be leaked.", + client_name, strerror(r)); + return 1; } - /* CONFIG WRITE LOCK - re-acquire to clear the thread pointer and free the thread data, - * a reader may be holding the lock for the whole duration of a transport handshake */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_APPLY_LOCK_TIMEOUT, __func__) != 1) { - /* if we fail, attempt to lock again in cleanup. - * ch thread data will cause a memory leak, but we should avoid a possible crash this way */ - ERR(NULL, "Timed out waiting for the configuration lock, Call Home client \"%s\" thread data leaked.", - ch_client_name); - rc = 1; - goto cleanup; - } - config_lock_mode = NC_RWLOCK_WRITE; + /* the registry entry was ours, so is the cleanup */ + nc_server_ch_thread_arg_free(thread_arg); - /* clear the thread pointer, - * ch_client MUST remain valid even though we unlocked config lock, - * because the caller MUST hold config apply mutex, so no one can change the config and free the client */ - ch_client->thread = NULL; + return 0; +} - /* free the thread data */ - free(thread_arg->client_name); - close(thread_arg->notify_pipe[0]); - close(thread_arg->notify_pipe[1]); - free(thread_arg); +int +nc_server_ch_threads_destroy(void) +{ + int rc = 0; + char **names = NULL; + LY_ARRAY_COUNT_TYPE u; -cleanup: - if (config_lock_mode == NC_RWLOCK_NONE) { - /* CONFIG LOCK - lock it back if we unlocked it. It MUST succeed, if the caller holds the config apply mutex */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_APPLY_LOCK_TIMEOUT, __func__) != 1) { - ERRINT; + if (nc_server_ch_thread_names_get(&names)) { + return 1; + } + + LY_ARRAY_FOR(names, u) { + if (nc_session_server_ch_client_dispatch_stop(names[u])) { + rc = 1; } } - free(ch_client_name); + nc_server_ch_thread_names_free(names); + + /* CH THREADS LOCK */ + if (nc_mutex_lock(&server_opts.ch_threads_lock, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } + if (LY_ARRAY_COUNT(server_opts.ch_threads)) { + ERRINT; + rc = 1; + } + LY_ARRAY_FREE(server_opts.ch_threads); + server_opts.ch_threads = NULL; + /* CH THREADS UNLOCK */ + nc_mutex_unlock(&server_opts.ch_threads_lock, __func__); + return rc; } int -_nc_connect_ch_client_dispatch(struct nc_ch_client *ch_client, nc_server_ch_session_acquire_ctx_cb acquire_ctx_cb, +_nc_connect_ch_client_dispatch(const char *client_name, nc_server_ch_session_acquire_ctx_cb acquire_ctx_cb, nc_server_ch_session_release_ctx_cb release_ctx_cb, void *ctx_cb_data, nc_server_ch_new_session_cb new_session_cb, void *new_session_cb_data) { int rc = 0, r; int flags; - struct nc_server_ch_thread_arg *arg = NULL; + LY_ERR lyrc = LY_SUCCESS; + struct nc_server_ch_thread_arg *arg = NULL, **item; + LY_ARRAY_COUNT_TYPE u; /* create the thread argument */ arg = calloc(1, sizeof *arg); NC_CHECK_ERRMEM_GOTO(!arg, rc = -1, cleanup); arg->notify_pipe[0] = -1; arg->notify_pipe[1] = -1; - arg->client_name = strdup(ch_client->name); + arg->client_name = strdup(client_name); NC_CHECK_ERRMEM_GOTO(!arg->client_name, rc = -1, cleanup); arg->acquire_ctx_cb = acquire_ctx_cb; arg->release_ctx_cb = release_ctx_cb; arg->ctx_cb_data = ctx_cb_data; arg->new_session_cb = new_session_cb; arg->new_session_cb_data = new_session_cb_data; + + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { + rc = -1; + goto cleanup; + } arg->new_session_fail_cb = server_opts.ch_dispatch_data.new_session_fail_cb; arg->new_session_fail_cb_data = server_opts.ch_dispatch_data.new_session_fail_cb_data; + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); /* create the self-pipe for signaling the thread to terminate */ if (pipe(arg->notify_pipe) == -1) { @@ -3961,31 +4560,49 @@ _nc_connect_ch_client_dispatch(struct nc_ch_client *ch_client, nc_server_ch_sess goto cleanup; } - /* store thread data in the client */ - ch_client->thread = arg; + /* mark the thread as running before it is created, so that it can be stopped right away */ + ATOMIC_STORE_RELAXED(arg->thread_running, 1); + + /* CH THREADS LOCK - the registration and the thread creation must be atomic, the registry entry + * is what makes the thread findable and joinable, so it must exist before the thread does but + * it must never refer to a thread that was not created yet */ + if (nc_mutex_lock(&server_opts.ch_threads_lock, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) { + rc = -1; + goto cleanup; + } + + /* there must never be two threads dispatched for a single Call Home client */ + LY_ARRAY_FOR(server_opts.ch_threads, u) { + if (!strcmp(server_opts.ch_threads[u]->client_name, client_name)) { + rc = 1; + goto unlock; + } + } + + /* register the thread first, the array cannot fail to grow once the thread is running */ + LY_ARRAY_NEW_GOTO(NULL, server_opts.ch_threads, item, lyrc, unlock); + *item = arg; /* create the CH thread */ if ((r = pthread_create(&arg->tid, NULL, nc_ch_client_thread, arg))) { ERR(NULL, "Creating a new thread failed (%s).", strerror(r)); - ch_client->thread = NULL; + LY_ARRAY_DECREMENT_FREE(server_opts.ch_threads); rc = -1; - goto cleanup; + goto unlock; } - /* arg is now owned by the thread */ + /* arg is now owned by the thread and the registry */ arg = NULL; -cleanup: - if (arg) { - free(arg->client_name); - if (arg->notify_pipe[0] != -1) { - close(arg->notify_pipe[0]); - } - if (arg->notify_pipe[1] != -1) { - close(arg->notify_pipe[1]); - } - free(arg); +unlock: + /* CH THREADS UNLOCK */ + nc_mutex_unlock(&server_opts.ch_threads_lock, __func__); + if (lyrc) { + rc = -1; } + +cleanup: + nc_server_ch_thread_arg_free(arg); return rc; } @@ -3995,31 +4612,208 @@ nc_connect_ch_client_dispatch(const char *client_name, nc_server_ch_session_acqu void *new_session_cb_data) { int rc = 0; - struct nc_ch_client *ch_client; + const struct nc_server_config *config; NC_CHECK_ARG_RET(NULL, client_name, acquire_ctx_cb, release_ctx_cb, new_session_cb, -1); NC_CHECK_SRV_INIT_RET(-1); - /* CONFIG WRITE LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + config = nc_server_config_acquire(); + if (!config) { return -1; } /* check ch client existence */ - ch_client = nc_server_ch_client_get(client_name); - NC_CHECK_ERR_GOTO(!ch_client, rc = -1; ERR(NULL, "Call Home client \"%s\" not found.", client_name), cleanup); + if (!nc_server_ch_client_get_pinned(config, client_name)) { + ERR(NULL, "Call Home client \"%s\" not found.", client_name); + rc = -1; + goto cleanup; + } - /* requires config wr lock */ - rc = _nc_connect_ch_client_dispatch(ch_client, acquire_ctx_cb, release_ctx_cb, ctx_cb_data, + rc = _nc_connect_ch_client_dispatch(client_name, acquire_ctx_cb, release_ctx_cb, ctx_cb_data, new_session_cb, new_session_cb_data); + if (rc == 1) { + /* a thread is already running for this client, do not silently ignore that */ + ERR(NULL, "Call Home client \"%s\" is already being dispatched.", client_name); + rc = -1; + } cleanup: - /* CONFIG WRITE UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + nc_server_config_release(config); return rc; } +/** + * @brief Check whether a Call Home client name is present in an array of names. + * + * @param[in] names Array of names (sized-array, see libyang docs). + * @param[in] name Name to look for. + * @return 1 if @p name is present, 0 otherwise. + */ +static int +nc_server_ch_name_found(char **names, const char *name) +{ + LY_ARRAY_COUNT_TYPE u; + + LY_ARRAY_FOR(names, u) { + if (!strcmp(names[u], name)) { + return 1; + } + } + + return 0; +} + +/** + * @brief Check whether a server configuration contains a Call Home client of the given name. + * + * @param[in] config Server configuration. + * @param[in] name Name of the Call Home client to look for. + * @return 1 if the client is configured, 0 otherwise. + */ +static int +nc_server_ch_client_configured(const struct nc_server_config *config, const char *name) +{ + LY_ARRAY_COUNT_TYPE u; + + LY_ARRAY_FOR(config->ch_clients, u) { + if (!strcmp(config->ch_clients[u].name, name)) { + return 1; + } + } + + return 0; +} + +/** + * @brief Check if the new configuration contains a Call Home client that has no thread running. + * + * @param[in] config New server configuration currently being applied. + * @param[in] running Names of the Call Home clients with a running thread (sized-array, see libyang docs). + * @return 1 if there are new CH clients, 0 otherwise. + */ +static int +nc_server_ch_new_clients_created(const struct nc_server_config *config, char **running) +{ + LY_ARRAY_COUNT_TYPE u; + + LY_ARRAY_FOR(config->ch_clients, u) { + if (!nc_server_ch_name_found(running, config->ch_clients[u].name)) { + return 1; + } + } + + /* no differences found */ + return 0; +} + +int +nc_server_ch_clients_reconcile(const struct nc_server_config *config) +{ + int rc = 0; + LY_ARRAY_COUNT_TYPE u; + char **running = NULL, **started = NULL, **started_name, *name = NULL; + struct nc_server_ch_dispatch_data dispatch_data; + int dispatch_new_clients = 1; + + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } + dispatch_data = server_opts.ch_dispatch_data; + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); + + /* learn which clients are running right now */ + NC_CHECK_GOTO(rc = nc_server_ch_thread_names_get(&running), cleanup); + + if (!dispatch_data.acquire_ctx_cb || !dispatch_data.release_ctx_cb || !dispatch_data.new_session_cb) { + /* Call Home dispatch callbacks not set, we can't dispatch new clients, but we can still stop deleted ones */ + if (nc_server_ch_new_clients_created(config, running)) { + WRN(NULL, "New Call Home clients were created but Call Home dispatch callbacks are not set - " + "new clients will not be dispatched automatically."); + } + dispatch_new_clients = 0; + } + + /* + * == PHASE 1: START NEW CLIENTS == + * Start clients present in config that are not already running. + * Track successfully started threads for potential rollback. + */ + if (dispatch_new_clients) { + /* only dispatch if all required CBs are set */ + LY_ARRAY_FOR(config->ch_clients, u) { + if (nc_server_ch_name_found(running, config->ch_clients[u].name)) { + /* already running */ + continue; + } + + /* this is a new Call Home client, dispatch it */ + rc = _nc_connect_ch_client_dispatch(config->ch_clients[u].name, dispatch_data.acquire_ctx_cb, + dispatch_data.release_ctx_cb, dispatch_data.ctx_cb_data, + dispatch_data.new_session_cb, dispatch_data.new_session_cb_data); + if (rc == 1) { + /* the client was dispatched through the API right after we learned the running ones, + * which is exactly the state we wanted, so leave the thread to its dispatcher */ + VRB(NULL, "Call Home client \"%s\" already has a running thread, skipping its dispatch.", + config->ch_clients[u].name); + rc = 0; + continue; + } else if (rc) { + /* FAILURE! trigger rollback */ + goto rollback; + } + + /* successfully started, track the client for a potential rollback, the name must be + * ready before the array grows so that the rollback never sees a NULL entry */ + name = strdup(config->ch_clients[u].name); + NC_CHECK_ERRMEM_GOTO(!name, rc = 1, rollback); + LY_ARRAY_NEW_GOTO(NULL, started, started_name, rc, rollback); + *started_name = name; + name = NULL; + } + } + + /* + * == PHASE 2: STOP DELETED CLIENTS (COMMIT) == + * All new clients started successfully. Now stop the running clients + * that are not present in the new configuration. + */ + LY_ARRAY_FOR(running, u) { + if (nc_server_ch_client_configured(config, running[u])) { + continue; + } + + /* this Call Home client was deleted, notify it to stop */ + if ((rc = nc_session_server_ch_client_dispatch_stop(running[u]))) { + ERR(NULL, "Failed to dispatch stop for Call Home client \"%s\".", running[u]); + goto rollback; + } + } + + /* success */ + rc = 0; + goto cleanup; + +rollback: + /* + * == ROLLBACK LOGIC == + * An error occurred during PHASE 1. Stop any new threads we *just* started + * to return to the pre-call state. + */ + LY_ARRAY_FOR(started, u) { + nc_session_server_ch_client_dispatch_stop(started[u]); + } + /* rc is already set to non-zero from the failure point */ + +cleanup: + free(name); + nc_server_ch_thread_names_free(running); + nc_server_ch_thread_names_free(started); + return rc ? 1 : 0; +} + #endif /* NC_ENABLED_SSH_TLS */ API struct timespec @@ -4489,27 +5283,33 @@ nc_server_notif_cert_exp_dates_get(struct nc_cert_exp_time_interval *intervals, struct nc_cert_expiration **exp_dates, uint32_t *exp_date_count) { int ret = 0; - struct nc_endpt *endpt; - struct nc_ch_client *ch_client; - struct nc_ch_endpt *ch_endpt; + const struct nc_server_config *config; + const struct nc_endpt *endpt; + const struct nc_ch_client *ch_client; + const struct nc_ch_endpt *ch_endpt; struct nc_certificate *cert; - struct nc_keystore *ks = &server_opts.config.keystore; - struct nc_truststore *ts = &server_opts.config.truststore; + const struct nc_keystore *ks; + const struct nc_truststore *ts; struct nc_cert_path_aux cp = {0}; - LY_ARRAY_COUNT_TYPE i; + LY_ARRAY_COUNT_TYPE i, u, v; NC_CHECK_ARG_RET(NULL, intervals, interval_count, exp_dates, exp_date_count, 1); *exp_dates = NULL; *exp_date_count = 0; - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + config = nc_server_config_acquire(); + if (!config) { return 1; } + /* the aliases must only be taken from the pinned configuration */ + ks = &config->keystore; + ts = &config->truststore; + /* first go through listen certs */ - LY_ARRAY_FOR(server_opts.config.endpts, struct nc_endpt, endpt) { + LY_ARRAY_FOR(config->endpts, u) { + endpt = &config->endpts[u]; if (endpt->ti == NC_TI_TLS) { ret = nc_server_notif_cert_exp_dates_endpt_get(NULL, endpt->name, endpt->opts.tls, intervals, interval_count, exp_dates, exp_date_count); @@ -4520,8 +5320,10 @@ nc_server_notif_cert_exp_dates_get(struct nc_cert_exp_time_interval *intervals, } /* then go through all the ch clients and their endpts */ - LY_ARRAY_FOR(server_opts.config.ch_clients, struct nc_ch_client, ch_client) { - LY_ARRAY_FOR(ch_client->ch_endpts, struct nc_ch_endpt, ch_endpt) { + LY_ARRAY_FOR(config->ch_clients, u) { + ch_client = &config->ch_clients[u]; + LY_ARRAY_FOR(ch_client->ch_endpts, v) { + ch_endpt = &ch_client->ch_endpts[v]; if (ch_endpt->ti == NC_TI_TLS) { ret = nc_server_notif_cert_exp_dates_endpt_get(ch_client->name, ch_endpt->name, ch_endpt->opts.tls, intervals, interval_count, exp_dates, exp_date_count); @@ -4555,8 +5357,7 @@ nc_server_notif_cert_exp_dates_get(struct nc_cert_exp_time_interval *intervals, } cleanup: - /* CONFIG READ UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + nc_server_config_release(config); return ret; } @@ -4667,16 +5468,17 @@ nc_server_notif_cert_exp_intervals_get(struct nc_cert_exp_time_interval *default struct nc_cert_exp_time_interval **intervals, uint32_t *interval_count) { int rc = 0; + const struct nc_server_config *config; *intervals = NULL; *interval_count = 0; - /* CONFIG LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + config = nc_server_config_acquire(); + if (!config) { return 1; } - if (!server_opts.config.cert_exp_notif_intervals) { + if (!config->cert_exp_notif_intervals) { /* dup the default intervals */ *intervals = malloc(default_interval_count * sizeof **intervals); NC_CHECK_ERRMEM_GOTO(!*intervals, rc = 1, cleanup); @@ -4684,16 +5486,15 @@ nc_server_notif_cert_exp_intervals_get(struct nc_cert_exp_time_interval *default *interval_count = default_interval_count; } else { /* dup the configured intervals */ - *intervals = malloc(LY_ARRAY_COUNT(server_opts.config.cert_exp_notif_intervals) * sizeof **intervals); + *intervals = malloc(LY_ARRAY_COUNT(config->cert_exp_notif_intervals) * sizeof **intervals); NC_CHECK_ERRMEM_GOTO(!*intervals, rc = 1, cleanup); - memcpy(*intervals, server_opts.config.cert_exp_notif_intervals, - LY_ARRAY_COUNT(server_opts.config.cert_exp_notif_intervals) * sizeof **intervals); - *interval_count = LY_ARRAY_COUNT(server_opts.config.cert_exp_notif_intervals); + memcpy(*intervals, config->cert_exp_notif_intervals, + LY_ARRAY_COUNT(config->cert_exp_notif_intervals) * sizeof **intervals); + *interval_count = LY_ARRAY_COUNT(config->cert_exp_notif_intervals); } cleanup: - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + nc_server_config_release(config); return rc; } @@ -4891,31 +5692,21 @@ nc_server_notif_cert_expiration_thread_stop(int wait) #endif /* NC_ENABLED_SSH_TLS */ int -nc_server_is_mod_ignored(const char *mod_name, int config_locked) +nc_server_is_mod_ignored(const struct nc_server_config *config, const char *mod_name) { - int ignored = 0; - LY_ARRAY_COUNT_TYPE i; + LY_ARRAY_COUNT_TYPE u; - if (!config_locked) { - /* LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { - return 0; - } + if (!config) { + return 0; } - LY_ARRAY_FOR(server_opts.config.ignored_modules, i) { - if (!strcmp(server_opts.config.ignored_modules[i], mod_name)) { - ignored = 1; - break; + LY_ARRAY_FOR(config->ignored_modules, u) { + if (!strcmp(config->ignored_modules[u], mod_name)) { + return 1; } } - if (!config_locked) { - /* UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); - } - - return ignored; + return 0; } API int @@ -4927,8 +5718,8 @@ nc_server_set_unix_socket_path(const char *endpoint_name, const char *socket_pat NC_CHECK_ARG_RET(NULL, endpoint_name, socket_path, 1); - /* CONFIG WRITE LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* OPTS WRITE LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_WRITE, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { return 1; } @@ -4953,8 +5744,8 @@ nc_server_set_unix_socket_path(const char *endpoint_name, const char *socket_pat NC_CHECK_ERRMEM_GOTO(!pentry->path, rc = 1, cleanup); cleanup: - /* CONFIG WRITE UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS WRITE UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); return rc; } @@ -4969,8 +5760,8 @@ nc_server_get_unix_socket_path(const char *endpoint_name, char **socket_path) *socket_path = NULL; - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { return 1; } @@ -4990,8 +5781,8 @@ nc_server_get_unix_socket_path(const char *endpoint_name, char **socket_path) NC_CHECK_ERRMEM_GOTO(!*socket_path, rc = 1, cleanup); cleanup: - /* CONFIG READ UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); return rc; } @@ -5000,8 +5791,10 @@ nc_server_set_unix_socket_dir(const char *dir) { int rc = 0; - /* CONFIG WRITE LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + NC_CHECK_ARG_RET(NULL, dir, 1); + + /* OPTS WRITE LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_WRITE, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { return 1; } @@ -5010,8 +5803,8 @@ nc_server_set_unix_socket_dir(const char *dir) NC_CHECK_ERRMEM_GOTO(!server_opts.unix_socket_dir, rc = 1, cleanup); cleanup: - /* CONFIG WRITE UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS WRITE UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); return rc; } @@ -5020,10 +5813,12 @@ nc_server_get_unix_socket_dir(char **dir) { int rc = 0; + NC_CHECK_ARG_RET(NULL, dir, 1); + *dir = NULL; - /* CONFIG READ LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { return 1; } @@ -5033,7 +5828,7 @@ nc_server_get_unix_socket_dir(char **dir) } cleanup: - /* CONFIG READ UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); return rc; } diff --git a/src/session_server.h b/src/session_server.h index c4b82289..e9e94b1a 100644 --- a/src/session_server.h +++ b/src/session_server.h @@ -144,6 +144,13 @@ int nc_server_init(void); /** * @brief Destroy any dynamically allocated libssh and/or libssl/libcrypto and server resources. * + * No other server API call may run concurrently with this function and there must be no session + * being accepted or authenticated. Established sessions are not affected, but the data of the + * user callbacks set through the API (::nc_server_ssh_set_interactive_auth_clb(), + * ::nc_server_ch_set_dispatch_data(), ...) is released here, so an authentication still running in + * one of them would use freed data. Call Home client threads are stopped, but only the ones + * dispatched before this call. + * * @return 0 on success, 1 on error - failed to synchronize with other threads * (timed out waiting for locks or failed to join threads). Safe to call * again to retry freeing resources. diff --git a/src/session_server_ch.h b/src/session_server_ch.h index 91c0b7d0..302bdbe3 100644 --- a/src/session_server_ch.h +++ b/src/session_server_ch.h @@ -106,6 +106,10 @@ typedef void (*nc_server_ch_new_session_fail_cb)(const char *client_name, const /** * @brief Dispatch a thread connecting to a listening NETCONF client and creating Call Home sessions. * + * There is at most one thread per Call Home client, so dispatching a client that already has a + * running thread (either from a previous call or automatically, when its configuration was applied) + * does nothing and is reported as an error. + * * @param[in] client_name Existing client name. * @param[in] acquire_ctx_cb Callback for acquiring new session context. * @param[in] release_ctx_cb Callback for releasing session context. diff --git a/src/session_server_ssh.c b/src/session_server_ssh.c index ff9ec325..ef127d5a 100644 --- a/src/session_server_ssh.c +++ b/src/session_server_ssh.c @@ -75,7 +75,7 @@ nc_ssh_check_local_user_support(struct nc_session *session) struct nc_auth_client * nc_ssh_find_auth_client(struct nc_server_ssh_opts *opts, const char *user, struct nc_session *session) { - struct nc_endpt *referenced_endpt; + const struct nc_endpt *referenced_endpt; LY_ARRAY_COUNT_TYPE u; if (!user) { @@ -90,7 +90,7 @@ nc_ssh_find_auth_client(struct nc_server_ssh_opts *opts, const char *user, struc /* client not known by the endpt, but it references another one so try it */ if (opts->referenced_endpt_name) { - if (nc_server_endpt_get(opts->referenced_endpt_name, &referenced_endpt)) { + if (nc_server_endpt_get(session->opts.server.config, opts->referenced_endpt_name, &referenced_endpt)) { ERR(session, "Referenced endpoint \"%s\" not found.", opts->referenced_endpt_name); return NULL; } @@ -206,6 +206,8 @@ int nc_server_ssh_kbdint_select_method(struct nc_session *session, int local_users_supported, struct nc_auth_client *auth_client, enum nc_kbdint_backend *backend) { + int custom_clb_set; + assert(!local_users_supported || auth_client); if (!local_users_supported) { @@ -221,7 +223,15 @@ nc_server_ssh_kbdint_select_method(struct nc_session *session, int local_users_s return 1; } - if (server_opts.interactive_auth_clb) { + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } + custom_clb_set = server_opts.interactive_auth_clb ? 1 : 0; + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); + + if (custom_clb_set) { /* custom callback has higher priority */ *backend = NC_KBDINT_BACKEND_CUSTOM_CLB; return 0; @@ -274,7 +284,8 @@ nc_server_ssh_auth_pubkey_check(struct nc_session *session, ssh_key pubkey, pubkey_count = LY_ARRAY_COUNT(auth_client->pubkeys); } else if (auth_client->pubkey_store == NC_STORE_TRUSTSTORE) { /* need to fetch from the truststore */ - ret = nc_server_ssh_ts_ref_get_keys(auth_client->ts_ref, &pubkeys, &pubkey_count); + ret = nc_server_ssh_ts_ref_get_keys(session->opts.server.config, auth_client->ts_ref, + &pubkeys, &pubkey_count); if (ret) { goto cleanup; } @@ -408,21 +419,48 @@ nc_server_ssh_pam_conv_fill(struct nc_session *session, struct pam_response *res return PAM_SUCCESS; } +int +nc_server_ssh_get_pam_conf_filename(char **filename) +{ + int rc = 0; + + *filename = NULL; + + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } + + if (server_opts.pam_config_name) { + *filename = strdup(server_opts.pam_config_name); + NC_CHECK_ERRMEM_GOTO(!*filename, rc = 1, cleanup); + } + +cleanup: + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); + return rc; +} + int nc_server_ssh_pam_authenticate(struct nc_session *session, const char *username, const struct pam_conv *conv) { pam_handle_t *pam_h = NULL; + char *pam_config_name = NULL; int ret; - /* check the PAM configuration */ - if (!server_opts.pam_config_name) { + /* get the PAM configuration, PAM must not be called with the lock held */ + if (nc_server_ssh_get_pam_conf_filename(&pam_config_name)) { + return 1; + } + if (!pam_config_name) { ERR(session, "PAM configuration filename not set."); return 1; } /* initialize PAM and see if the given configuration file exists */ - ret = pam_start(server_opts.pam_config_name, username, conv, &pam_h); + ret = pam_start(pam_config_name, username, conv, &pam_h); if (ret != PAM_SUCCESS) { ERR(session, "PAM error occurred (%s).", pam_strerror(pam_h, ret)); goto cleanup; @@ -462,6 +500,7 @@ nc_server_ssh_pam_authenticate(struct nc_session *session, const char *username, if (pam_h && (pam_end(pam_h, ret) != PAM_SUCCESS)) { ERR(NULL, "PAM error occurred (%s).", pam_strerror(pam_h, ret)); } + free(pam_config_name); return ret; } @@ -619,15 +658,23 @@ nc_server_ssh_privkey_data_to_tmp_file(const char *in, const char *privkey_forma /** * @brief Get asymmetric key from the keystore. * + * @param[in] config Pinned server configuration to search. * @param[in] referenced_name Name of the asymmetric key in the keystore. * @param[out] askey Referenced asymmetric key. * @return 0 on success, 1 on error. */ static int -nc_server_ssh_ks_ref_get_key(const char *referenced_name, struct nc_asymmetric_key **askey) +nc_server_ssh_ks_ref_get_key(const struct nc_server_config *config, const char *referenced_name, + struct nc_asymmetric_key **askey) { LY_ARRAY_COUNT_TYPE i; - struct nc_keystore *ks = &server_opts.config.keystore; + const struct nc_keystore *ks; + + if (!config) { + ERR(NULL, "No server configuration to get the keystore entry \"%s\" from.", referenced_name); + return 1; + } + ks = &config->keystore; *askey = NULL; @@ -642,7 +689,7 @@ nc_server_ssh_ks_ref_get_key(const char *referenced_name, struct nc_asymmetric_k return 1; } - *askey = &ks->entries[i].asym_key; + *askey = (struct nc_asymmetric_key *)&ks->entries[i].asym_key; /* check if the referenced public key is SubjectPublicKeyInfo */ if ((*askey)->pubkey.data && nc_is_pk_subject_public_key_info((*askey)->pubkey.data)) { @@ -655,15 +702,21 @@ nc_server_ssh_ks_ref_get_key(const char *referenced_name, struct nc_asymmetric_k } int -nc_server_ssh_ts_ref_get_keys(const char *referenced_name, struct nc_public_key **pubkeys, uint32_t *pubkey_count) +nc_server_ssh_ts_ref_get_keys(const struct nc_server_config *config, const char *referenced_name, + struct nc_public_key **pubkeys, uint32_t *pubkey_count) { - LY_ARRAY_COUNT_TYPE i; - struct nc_public_key *pubkey; - struct nc_truststore *ts = &server_opts.config.truststore; + LY_ARRAY_COUNT_TYPE i, u; + const struct nc_truststore *ts; *pubkeys = NULL; *pubkey_count = 0; + if (!config) { + ERR(NULL, "No server configuration to get the truststore entry \"%s\" from.", referenced_name); + return 1; + } + ts = &config->truststore; + /* lookup name */ LY_ARRAY_FOR(ts->pubkey_bags, i) { if (!strcmp(referenced_name, ts->pubkey_bags[i].name)) { @@ -676,8 +729,8 @@ nc_server_ssh_ts_ref_get_keys(const char *referenced_name, struct nc_public_key } /* check if any of the referenced public keys is SubjectPublicKeyInfo */ - LY_ARRAY_FOR(ts->pubkey_bags[i].pubkeys, struct nc_public_key, pubkey) { - if (nc_is_pk_subject_public_key_info(pubkey->data)) { + LY_ARRAY_FOR(ts->pubkey_bags[i].pubkeys, u) { + if (nc_is_pk_subject_public_key_info(ts->pubkey_bags[i].pubkeys[u].data)) { ERR(NULL, "A public key of the referenced public key bag \"%s\" is in the SubjectPublicKeyInfo format, " "which is not allowed in SSH!", referenced_name); return 1; @@ -768,16 +821,28 @@ nc_server_ssh_str_append(const char src_c, const char *src_str, int *size, int * static int nc_server_ssh_get_system_keys_path(const char *username, char **out_path) { - int ret = 0, i, have_percent = 0, size = 0, idx = 0; - const char *path_fmt = server_opts.authkey_path_fmt; + int ret = 0, i, have_percent = 0, size = 0, idx = 0, fmt_set = 0; + char *path_fmt = NULL; char *path = NULL, *buf = NULL, *uid = NULL; struct passwd *pw, pw_buf; size_t buf_len = 0; - if (!path_fmt) { + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } + if (server_opts.authkey_path_fmt) { + fmt_set = 1; + path_fmt = strdup(server_opts.authkey_path_fmt); + } + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); + + if (!fmt_set) { ERR(NULL, "System public keys path format not set."); return 1; } + NC_CHECK_ERRMEM_RET(!path_fmt, 1); /* check if the path format contains any tokens */ if (strstr(path_fmt, "%h") || strstr(path_fmt, "%U") || strstr(path_fmt, "%u") || strstr(path_fmt, "%%")) { @@ -798,7 +863,7 @@ nc_server_ssh_get_system_keys_path(const char *username, char **out_path) } else { /* no tokens, just copy the path and return */ *out_path = strdup(path_fmt); - NC_CHECK_ERRMEM_RET(!*out_path, 1); + NC_CHECK_ERRMEM_GOTO(!*out_path, ret = 1, cleanup); goto cleanup; } @@ -818,7 +883,7 @@ nc_server_ssh_get_system_keys_path(const char *username, char **out_path) /* UID */ ret = nc_server_ssh_str_append(0, uid, &size, &idx, &path); } else { - ERR(NULL, "Failed to parse system public keys path format \"%s\".", server_opts.authkey_path_fmt); + ERR(NULL, "Failed to parse system public keys path format \"%s\".", path_fmt); ret = 1; } @@ -841,6 +906,7 @@ nc_server_ssh_get_system_keys_path(const char *username, char **out_path) path = NULL; cleanup: + free(path_fmt); free(uid); free(buf); free(path); @@ -1239,8 +1305,8 @@ API void nc_server_ssh_set_interactive_auth_clb(int (*interactive_auth_clb)(const struct nc_session *session, ssh_session ssh_sess, ssh_message msg, void *user_data), void *user_data, void (*free_user_data)(void *user_data)) { - /* CONFIG LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* OPTS WRITE LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_WRITE, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { return; } @@ -1248,8 +1314,29 @@ nc_server_ssh_set_interactive_auth_clb(int (*interactive_auth_clb)(const struct server_opts.interactive_auth_data = user_data; server_opts.interactive_auth_data_free = free_user_data; - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS WRITE UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); +} + +int +nc_server_ssh_get_interactive_auth_clb(int (**clb)(const struct nc_session *session, ssh_session ssh_sess, + ssh_message msg, void *user_data), void **user_data) +{ + *clb = NULL; + *user_data = NULL; + + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } + + /* the callback and its data must be read as a pair */ + *clb = server_opts.interactive_auth_clb; + *user_data = server_opts.interactive_auth_data; + + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); + return 0; } #ifdef HAVE_LIBPAM @@ -1261,8 +1348,8 @@ nc_server_ssh_set_pam_conf_filename(const char *filename) NC_CHECK_ARG_RET(NULL, filename, 1); - /* CONFIG LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* OPTS WRITE LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_WRITE, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { return 1; } @@ -1273,8 +1360,8 @@ nc_server_ssh_set_pam_conf_filename(const char *filename) ret = 1; } - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS WRITE UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); return ret; } @@ -1297,8 +1384,8 @@ nc_server_ssh_set_authkey_path_format(const char *path) NC_CHECK_ARG_RET(NULL, path, 1); - /* CONFIG LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* OPTS WRITE LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_WRITE, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { return 1; } @@ -1309,8 +1396,8 @@ nc_server_ssh_set_authkey_path_format(const char *path) ret = 1; } - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS WRITE UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); return ret; } @@ -1357,8 +1444,8 @@ nc_server_ssh_set_protocol_string(const char *prefix) protocol_str = nc_server_ssh_forge_protocol_string(prefix); NC_CHECK_ERRMEM_GOTO(!protocol_str, rc = 1, cleanup); - /* CONFIG LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* OPTS WRITE LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_WRITE, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { rc = 1; goto cleanup; } @@ -1368,8 +1455,8 @@ nc_server_ssh_set_protocol_string(const char *prefix) server_opts.ssh_protocol_string = protocol_str; protocol_str = NULL; - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS WRITE UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); cleanup: free(protocol_str); @@ -1593,12 +1680,13 @@ nc_accept_ssh_session_open_netconf_channel(struct nc_session *session, struct nc /** * @brief Set hostkeys to be used for an SSH bind. * + * @param[in] config Pinned server configuration the options belong to. * @param[in] sbind SSH bind to use. * @param[in] opts SSH server options. * @return 0 on success, -1 on error. */ static int -nc_ssh_bind_add_hostkeys(ssh_bind sbind, struct nc_server_ssh_opts *opts) +nc_ssh_bind_add_hostkeys(const struct nc_server_config *config, ssh_bind sbind, struct nc_server_ssh_opts *opts) { int rc; char *privkey_path; @@ -1614,7 +1702,7 @@ nc_ssh_bind_add_hostkeys(ssh_bind sbind, struct nc_server_ssh_opts *opts) key = &hostkey->key; } else { /* keystore reference, need to get it */ - NC_CHECK_RET(nc_server_ssh_ks_ref_get_key(hostkey->ks_ref, &key), -1); + NC_CHECK_RET(nc_server_ssh_ks_ref_get_key(config, hostkey->ks_ref, &key), -1); } privkey_path = nc_server_ssh_privkey_data_to_tmp_file(key->privkey.data, nc_privkey_format_to_str(key->privkey.type)); @@ -1738,10 +1826,10 @@ int nc_accept_ssh_session(struct nc_session *session, struct nc_server_ssh_opts *opts, int sock) { ssh_bind sbind = NULL; - int rc = 1, r; + int rc = 1, r, proto_str_set = 0; struct timespec ts_timeout; const char *err_msg; - char *proto_str = NULL, *proto_str_dyn = NULL; + char *proto_str = NULL; #if LIBSSH_0_12 struct nc_server_ssh_cb_data *cb_data = NULL; @@ -1782,7 +1870,7 @@ nc_accept_ssh_session(struct nc_session *session, struct nc_server_ssh_opts *opt } /* configure host keys */ - if (nc_ssh_bind_add_hostkeys(sbind, opts)) { + if (nc_ssh_bind_add_hostkeys(session->opts.server.config, sbind, opts)) { rc = -1; goto cleanup; } @@ -1822,14 +1910,24 @@ nc_accept_ssh_session(struct nc_session *session, struct nc_server_ssh_opts *opt } } - /* configure the ssh protocol identification string */ + /* configure the ssh protocol identification string, copy it so that the lock is not held any longer */ + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { + rc = -1; + goto cleanup; + } if (server_opts.ssh_protocol_string) { - proto_str = server_opts.ssh_protocol_string; - } else { - proto_str_dyn = nc_server_ssh_forge_protocol_string(NULL); - NC_CHECK_ERRMEM_GOTO(!proto_str_dyn, rc = -1, cleanup); - proto_str = proto_str_dyn; + proto_str_set = 1; + proto_str = strdup(server_opts.ssh_protocol_string); } + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); + + if (!proto_str_set) { + proto_str = nc_server_ssh_forge_protocol_string(NULL); + } + NC_CHECK_ERRMEM_GOTO(!proto_str, rc = -1, cleanup); + if (ssh_bind_options_set(sbind, SSH_BIND_OPTIONS_BANNER, proto_str)) { rc = -1; goto cleanup; @@ -1909,10 +2007,18 @@ nc_accept_ssh_session(struct nc_session *session, struct nc_server_ssh_opts *opt } cleanup: +#if LIBSSH_0_12 + /* the transport options belong to a configuration generation that is released once the handshake + * is over, the callbacks running afterwards must not reach them */ + if (session->ti.libssh.cb_data) { + ((struct nc_server_ssh_cb_data *)session->ti.libssh.cb_data)->opts = NULL; + } +#endif /* LIBSSH_0_12 */ + if (sock > -1) { close(sock); } - free(proto_str_dyn); + free(proto_str); ssh_bind_free(sbind); return rc; } diff --git a/src/session_server_ssh_auth_callback.c b/src/session_server_ssh_auth_callback.c index 646fb588..d9e2676f 100644 --- a/src/session_server_ssh_auth_callback.c +++ b/src/session_server_ssh_auth_callback.c @@ -285,13 +285,18 @@ static int nc_server_ssh_cb_kbdint_pam_request(struct nc_server_ssh_cb_data *cb_data, ssh_message message) { struct nc_server_ssh_cb_pam_data *pam_data; + char *pam_config_name = NULL; int rc; /* check the PAM configuration */ - if (!server_opts.pam_config_name) { + if (nc_server_ssh_get_pam_conf_filename(&pam_config_name)) { + return SSH_AUTH_DENIED; + } + if (!pam_config_name) { ERR(cb_data->session, "PAM configuration filename not set."); return SSH_AUTH_DENIED; } + free(pam_config_name); /* cancel any in-progress PAM exchange, e.g. the client abandoned the previous one */ nc_server_ssh_cb_kbdint_pam_cancel_stored(cb_data); @@ -655,6 +660,10 @@ nc_server_ssh_cb_auth_kbdint(ssh_message message, ssh_session UNUSED(libssh_sess int ret = SSH_AUTH_DENIED; const char *user; + int (*interactive_auth_clb)(const struct nc_session *session, ssh_session ssh_sess, ssh_message msg, + void *user_data); + void *interactive_auth_data; + /* Extract the username from the message. */ if (ssh_message_auth_kbdint_is_response(message) && session->username) { user = session->username; @@ -682,9 +691,19 @@ nc_server_ssh_cb_auth_kbdint(ssh_message message, ssh_session UNUSED(libssh_sess } if (backend == NC_KBDINT_BACKEND_CUSTOM_CLB) { - /* custom interactive auth callback */ - ret = server_opts.interactive_auth_clb(session, - session->ti.libssh.session, message, server_opts.interactive_auth_data); + /* custom interactive auth callback, it must not be called with the options lock held */ + if (nc_server_ssh_get_interactive_auth_clb(&interactive_auth_clb, &interactive_auth_data)) { + nc_server_ssh_auth_attempt_failed(session); + return SSH_AUTH_DENIED; + } + if (!interactive_auth_clb) { + /* the callback was unset in the meantime */ + ERR(session, "Custom keyboard-interactive authentication callback not set."); + nc_server_ssh_auth_attempt_failed(session); + return SSH_AUTH_DENIED; + } + + ret = interactive_auth_clb(session, session->ti.libssh.session, message, interactive_auth_data); } else { ret = nc_server_ssh_cb_kbdint_system(cb_data, message, user); } diff --git a/src/session_server_ssh_auth_message.c b/src/session_server_ssh_auth_message.c index f977463b..e7ba6974 100644 --- a/src/session_server_ssh_auth_message.c +++ b/src/session_server_ssh_auth_message.c @@ -282,15 +282,27 @@ nc_server_ssh_msg_auth_kbdint(struct nc_session *session, int local_users_suppor int r; enum nc_kbdint_backend backend; + int (*interactive_auth_clb)(const struct nc_session *session, ssh_session ssh_sess, ssh_message msg, + void *user_data); + void *interactive_auth_data; + /* select the kbdint backend based on the configuration */ if (nc_server_ssh_kbdint_select_method(session, local_users_supported, auth_client, &backend)) { return 1; } if (backend == NC_KBDINT_BACKEND_CUSTOM_CLB) { - /* custom callback has higher priority */ - r = server_opts.interactive_auth_clb(session, - session->ti.libssh.session, msg, server_opts.interactive_auth_data); + /* custom callback has higher priority, it must not be called with the options lock held */ + if (nc_server_ssh_get_interactive_auth_clb(&interactive_auth_clb, &interactive_auth_data)) { + return 1; + } + if (!interactive_auth_clb) { + /* the callback was unset in the meantime */ + ERR(session, "Custom keyboard-interactive authentication callback not set."); + return 1; + } + + r = interactive_auth_clb(session, session->ti.libssh.session, msg, interactive_auth_data); } else { r = nc_server_ssh_msg_auth_kbdint_system(session, msg); } diff --git a/src/session_server_ssh_wrapper.h b/src/session_server_ssh_wrapper.h index 09b85806..827445d2 100644 --- a/src/session_server_ssh_wrapper.h +++ b/src/session_server_ssh_wrapper.h @@ -128,7 +128,16 @@ void nc_server_ssh_cb_pam_cancel(struct nc_server_ssh_cb_pam_data *data); struct nc_server_ssh_cb_data { struct ssh_server_callbacks_struct server_cb; /**< libssh server callbacks. */ struct nc_session *session; /**< The current session. */ - struct nc_server_ssh_opts *opts; /**< SSH server options. */ + + /** + * @brief SSH server options, a pointer into a configuration generation. + * + * Only valid during the transport handshake - it is dereferenced solely by + * ::nc_server_ssh_cb_auth_common_setup(), reached only from the four authentication callbacks. + * The long-lived channel callbacks use only @p session. It is cleared at the end of the + * handshake so that a later dereference fails immediately instead of reading freed memory. + */ + struct nc_server_ssh_opts *opts; struct nc_auth_state auth_state; /**< Tracks multi-method authentication state. */ struct nc_ssh_channel_cb_data *channels; /**< List of additional channel callback data, tracked so non-netconf channels can be freed. */ @@ -295,12 +304,14 @@ int nc_server_ssh_auth_pubkey_compare_key(ssh_key key, struct nc_public_key *pub /** * @brief Get public keys from the truststore. * + * @param[in] config Pinned server configuration to search. * @param[in] referenced_name Name of the public key bag in the truststore. * @param[out] pubkeys Referenced public keys. * @param[out] pubkey_count Referenced public key count. * @return 0 on success, 1 on error. */ -int nc_server_ssh_ts_ref_get_keys(const char *referenced_name, struct nc_public_key **pubkeys, uint32_t *pubkey_count); +int nc_server_ssh_ts_ref_get_keys(const struct nc_server_config *config, const char *referenced_name, + struct nc_public_key **pubkeys, uint32_t *pubkey_count); /** * @brief Get user's public keys from the system. @@ -373,6 +384,19 @@ enum nc_kbdint_backend { int nc_server_ssh_kbdint_select_method(struct nc_session *session, int local_users_supported, struct nc_auth_client *auth_client, enum nc_kbdint_backend *backend); +/** + * @brief Get the configured custom keyboard-interactive authentication callback and its data. + * + * The callback is an application callback that may call back into the library, so it is read + * together with its data and only called once the options lock is released. + * + * @param[out] clb Custom keyboard-interactive authentication callback, NULL if not set. + * @param[out] user_data Data to pass to @p clb . + * @return 0 on success, 1 on error. + */ +int nc_server_ssh_get_interactive_auth_clb(int (**clb)(const struct nc_session *session, ssh_session ssh_sess, + ssh_message msg, void *user_data), void **user_data); + /** * @brief Check a channel subsystem request against the session state. * @@ -426,6 +450,16 @@ int nc_server_ssh_pam_conv_parse(struct nc_session *session, int n_messages, int nc_server_ssh_pam_conv_fill(struct nc_session *session, struct pam_response *resp, int n_prompts, int n_answers, const char **answers); +/** + * @brief Get a copy of the configured PAM service name. + * + * PAM must never be called while holding the options lock, so the name is always copied. + * + * @param[out] filename PAM service name copy, NULL if none is configured. + * @return 0 on success, 1 on error. + */ +int nc_server_ssh_get_pam_conf_filename(char **filename); + /** * @brief Run the PAM authentication sequence with a prepared conversation. * diff --git a/src/session_server_tls.c b/src/session_server_tls.c index f0118968..d3a7226f 100644 --- a/src/session_server_tls.c +++ b/src/session_server_tls.c @@ -36,6 +36,7 @@ /** * @brief Get certificate and private key data from keystore. * + * @param[in] config Pinned server configuration to search. * @param[in] referenced_key_name Name of the asymmetric key in the keystore. * @param[in] referenced_cert_name Name of the certificate in the keystore. * @param[out] privkey_data Retrieved private key data. @@ -44,15 +45,21 @@ * @return 0 on success, -1 on error. */ static int -nc_server_tls_ks_ref_get_cert_key(const char *referenced_key_name, const char *referenced_cert_name, - char **privkey_data, enum nc_privkey_format *privkey_type, char **cert_data) +nc_server_tls_ks_ref_get_cert_key(const struct nc_server_config *config, const char *referenced_key_name, + const char *referenced_cert_name, char **privkey_data, enum nc_privkey_format *privkey_type, char **cert_data) { LY_ARRAY_COUNT_TYPE i, j; - struct nc_keystore *ks = &server_opts.config.keystore; + const struct nc_keystore *ks; *privkey_data = NULL; *cert_data = NULL; + if (!config) { + ERR(NULL, "No server configuration to get the keystore entry \"%s\" from.", referenced_key_name); + return -1; + } + ks = &config->keystore; + /* lookup key */ LY_ARRAY_FOR(ks->entries, i) { if (!strcmp(referenced_key_name, ks->entries[i].asym_key.name)) { @@ -84,20 +91,28 @@ nc_server_tls_ks_ref_get_cert_key(const char *referenced_key_name, const char *r /** * @brief Get certificates from truststore. * + * @param[in] config Pinned server configuration to search. * @param[in] referenced_name Name of the certificate bag in the truststore. * @param[out] certs Retrieved certificates. * @param[out] cert_count Number of retrieved certificates. * @return 0 on success, -1 on error. */ static int -nc_server_tls_truststore_ref_get_certs(const char *referenced_name, struct nc_certificate **certs, uint32_t *cert_count) +nc_server_tls_truststore_ref_get_certs(const struct nc_server_config *config, const char *referenced_name, + struct nc_certificate **certs, uint32_t *cert_count) { LY_ARRAY_COUNT_TYPE i; - struct nc_truststore *ts = &server_opts.config.truststore; + const struct nc_truststore *ts; *certs = NULL; *cert_count = 0; + if (!config) { + ERR(NULL, "No server configuration to get the truststore bag \"%s\" from.", referenced_name); + return -1; + } + ts = &config->truststore; + /* lookup name */ LY_ARRAY_FOR(ts->cert_bags, i) { if (!strcmp(referenced_name, ts->cert_bags[i].name)) { @@ -500,16 +515,18 @@ nc_server_tls_cert_to_name(struct nc_ctn *ctn, void *cert_chain, char **username /** * @brief Resolve username from cert-to-name entries of endpoint and referenced endpoint. * + * @param[in] config Pinned server configuration the options belong to. * @param[in] opts TLS options of the endpoint. * @param[in] cert_chain Presented certificate chain, peer certificate first. * @param[out] username Resolved username. * @return 0 on success, 1 if no entry matched, -1 on error. */ static int -_nc_server_tls_cert_to_name(struct nc_server_tls_opts *opts, void *cert_chain, char **username) +_nc_server_tls_cert_to_name(const struct nc_server_config *config, struct nc_server_tls_opts *opts, + void *cert_chain, char **username) { int rc = 1; - struct nc_endpt *referenced_endpt; + const struct nc_endpt *referenced_endpt; struct nc_ctn *ctn; for (ctn = opts->ctn; ctn; ctn = ctn->next) { @@ -522,7 +539,7 @@ _nc_server_tls_cert_to_name(struct nc_server_tls_opts *opts, void *cert_chain, c /* do the same for referenced endpoint's ctn entries */ if (opts->referenced_endpt_name) { - if (nc_server_endpt_get(opts->referenced_endpt_name, &referenced_endpt)) { + if (nc_server_endpt_get(config, opts->referenced_endpt_name, &referenced_endpt)) { ERR(NULL, "Referenced endpoint \"%s\" not found.", opts->referenced_endpt_name); ERRINT; rc = -1; @@ -543,7 +560,8 @@ _nc_server_tls_cert_to_name(struct nc_server_tls_opts *opts, void *cert_chain, c } static int -_nc_server_tls_verify_peer_cert(void *peer_cert, struct nc_server_tls_client_auth *client_auth) +_nc_server_tls_verify_peer_cert(const struct nc_server_config *config, void *peer_cert, + struct nc_server_tls_client_auth *client_auth) { int rc; void *cert; @@ -556,7 +574,7 @@ _nc_server_tls_verify_peer_cert(void *peer_cert, struct nc_server_tls_client_aut cert_count = LY_ARRAY_COUNT(client_auth->ee_certs); } else if (client_auth->ee_certs_store == NC_STORE_TRUSTSTORE) { /* truststore reference */ - if (nc_server_tls_truststore_ref_get_certs(client_auth->ee_cert_bag_ts_ref, &certs, &cert_count)) { + if (nc_server_tls_truststore_ref_get_certs(config, client_auth->ee_cert_bag_ts_ref, &certs, &cert_count)) { ERR(NULL, "Error getting end-entity certificates from the truststore reference \"%s\".", client_auth->ee_cert_bag_ts_ref); return -1; } @@ -580,24 +598,26 @@ _nc_server_tls_verify_peer_cert(void *peer_cert, struct nc_server_tls_client_aut } int -nc_server_tls_verify_peer_cert(void *peer_cert, struct nc_server_tls_opts *opts) +nc_server_tls_verify_peer_cert(void *peer_cert, struct nc_tls_verify_cb_data *cb_data) { int rc; - struct nc_endpt *referenced_endpt; + const struct nc_endpt *referenced_endpt; + struct nc_server_tls_opts *opts = cb_data->opts; + const struct nc_server_config *config = cb_data->session->opts.server.config; - rc = _nc_server_tls_verify_peer_cert(peer_cert, &opts->client_auth); + rc = _nc_server_tls_verify_peer_cert(config, peer_cert, &opts->client_auth); if (!rc) { return 0; } if (opts->referenced_endpt_name) { - if (nc_server_endpt_get(opts->referenced_endpt_name, &referenced_endpt)) { + if (nc_server_endpt_get(config, opts->referenced_endpt_name, &referenced_endpt)) { ERR(NULL, "Referenced endpoint \"%s\" not found.", opts->referenced_endpt_name); ERRINT; return -1; } - rc = _nc_server_tls_verify_peer_cert(peer_cert, &referenced_endpt->opts.tls->client_auth); + rc = _nc_server_tls_verify_peer_cert(config, peer_cert, &referenced_endpt->opts.tls->client_auth); if (!rc) { return 0; } @@ -614,6 +634,9 @@ nc_server_tls_verify_cert(void *cert, int depth, int trusted, struct nc_tls_veri struct nc_server_tls_opts *opts = cb_data->opts; struct nc_session *session = cb_data->session; void *cert_chain = cb_data->chain; + const struct nc_server_config *config = cb_data->session->opts.server.config; + + int (*user_verify_clb)(const struct nc_session *session); if (session->username) { /* already verified */ @@ -636,7 +659,7 @@ nc_server_tls_verify_cert(void *cert, int depth, int trusted, struct nc_tls_veri if (!trusted) { /* peer cert is not trusted, so it must match any configured end-entity cert * on the given endpoint in order for the client to be authenticated */ - rc = nc_server_tls_verify_peer_cert(cert, opts); + rc = nc_server_tls_verify_peer_cert(cert, cb_data); if (rc) { ERR(session, "Cert verify: fail (Client certificate not trusted and does not match any configured end-entity certificate)."); goto cleanup; @@ -647,7 +670,7 @@ nc_server_tls_verify_cert(void *cert, int depth, int trusted, struct nc_tls_veri * the whole chain is needed in order to comply with the following issue: * https://github.com/CESNET/netopeer2/issues/1596 */ - rc = _nc_server_tls_cert_to_name(opts, cert_chain, &session->username); + rc = _nc_server_tls_cert_to_name(config, opts, cert_chain, &session->username); if (rc == -1) { /* fatal error */ goto cleanup; @@ -661,7 +684,17 @@ nc_server_tls_verify_cert(void *cert, int depth, int trusted, struct nc_tls_veri goto cleanup; } - if (server_opts.user_verify_clb && !server_opts.user_verify_clb(session)) { + /* OPTS READ LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_READ, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { + rc = -1; + goto cleanup; + } + user_verify_clb = server_opts.user_verify_clb; + /* OPTS READ UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); + + /* the callback must not be called with the options lock held */ + if (user_verify_clb && !user_verify_clb(session)) { VRB(session, "Cert verify: user verify callback revoked authorization."); rc = 1; goto cleanup; @@ -688,19 +721,20 @@ nc_session_get_client_cert(const struct nc_session *session) API void nc_server_tls_set_verify_clb(int (*verify_clb)(const struct nc_session *session)) { - /* CONFIG LOCK */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* OPTS WRITE LOCK */ + if (nc_rwlock_lock(&server_opts.opts_lock, NC_RWLOCK_WRITE, NC_OPTS_LOCK_TIMEOUT, __func__) != 1) { return; } server_opts.user_verify_clb = verify_clb; - /* CONFIG UNLOCK */ - nc_rwlock_unlock(&server_opts.config_lock, __func__); + /* OPTS WRITE UNLOCK */ + nc_rwlock_unlock(&server_opts.opts_lock, __func__); } int -nc_server_tls_load_server_cert_key(struct nc_server_tls_opts *opts, void **srv_cert, void **srv_pkey) +nc_server_tls_load_server_cert_key(const struct nc_server_config *config, struct nc_server_tls_opts *opts, + void **srv_cert, void **srv_pkey) { char *privkey_data = NULL, *cert_data = NULL; enum nc_privkey_format privkey_type; @@ -717,7 +751,8 @@ nc_server_tls_load_server_cert_key(struct nc_server_tls_opts *opts, void **srv_c privkey_type = opts->local.key.privkey.type; } else if (opts->cert_store == NC_STORE_KEYSTORE) { /* keystore */ - if (nc_server_tls_ks_ref_get_cert_key(opts->keystore.asym_key_ref, opts->keystore.cert_ref, &privkey_data, &privkey_type, &cert_data)) { + if (nc_server_tls_ks_ref_get_cert_key(config, opts->keystore.asym_key_ref, opts->keystore.cert_ref, + &privkey_data, &privkey_type, &cert_data)) { ERR(NULL, "Getting server certificate from the keystore reference \"%s\" failed.", opts->keystore.asym_key_ref); return 1; } @@ -744,7 +779,8 @@ nc_server_tls_load_server_cert_key(struct nc_server_tls_opts *opts, void **srv_c } int -nc_server_tls_load_trusted_certs(struct nc_server_tls_client_auth *client_auth, void *cert_store) +nc_server_tls_load_trusted_certs(const struct nc_server_config *config, + struct nc_server_tls_client_auth *client_auth, void *cert_store) { struct nc_certificate *certs; uint32_t i, cert_count = 0; @@ -756,7 +792,7 @@ nc_server_tls_load_trusted_certs(struct nc_server_tls_client_auth *client_auth, cert_count = LY_ARRAY_COUNT(client_auth->ca_certs); } else if (client_auth->ca_certs_store == NC_STORE_TRUSTSTORE) { /* truststore */ - if (nc_server_tls_truststore_ref_get_certs(client_auth->ca_cert_bag_ts_ref, &certs, &cert_count)) { + if (nc_server_tls_truststore_ref_get_certs(config, client_auth->ca_cert_bag_ts_ref, &certs, &cert_count)) { ERR(NULL, "Error getting certificate-authority certificates from the truststore reference \"%s\".", client_auth->ca_cert_bag_ts_ref); return 1; } @@ -805,12 +841,14 @@ nc_server_tls_accept_check(int accept_ret, void *tls_session) /** * @brief Get the number of certificates in a certificate grouping. * + * @param[in] config Pinned server configuration to resolve truststore references in. * @param[in] client_auth Client authentication data to get the number of certificates from. * @param[out] cert_count Number of certificates in the grouping. * @return 0 on success, -1 on error. */ static int -nc_server_tls_get_num_certs(struct nc_server_tls_client_auth *client_auth, uint32_t *cert_count) +nc_server_tls_get_num_certs(const struct nc_server_config *config, struct nc_server_tls_client_auth *client_auth, + uint32_t *cert_count) { uint32_t ca_count = 0, ee_count = 0; struct nc_certificate *certs; @@ -820,7 +858,7 @@ nc_server_tls_get_num_certs(struct nc_server_tls_client_auth *client_auth, uint3 if (client_auth->ca_certs_store == NC_STORE_LOCAL) { ca_count = LY_ARRAY_COUNT(client_auth->ca_certs); } else if (client_auth->ca_certs_store == NC_STORE_TRUSTSTORE) { - if (nc_server_tls_truststore_ref_get_certs(client_auth->ca_cert_bag_ts_ref, &certs, &ca_count)) { + if (nc_server_tls_truststore_ref_get_certs(config, client_auth->ca_cert_bag_ts_ref, &certs, &ca_count)) { ERR(NULL, "Getting CA certificates from the truststore reference \"%s\" failed.", client_auth->ca_cert_bag_ts_ref); return -1; } @@ -829,7 +867,7 @@ nc_server_tls_get_num_certs(struct nc_server_tls_client_auth *client_auth, uint3 if (client_auth->ee_certs_store == NC_STORE_LOCAL) { ee_count += LY_ARRAY_COUNT(client_auth->ee_certs); } else if (client_auth->ee_certs_store == NC_STORE_TRUSTSTORE) { - if (nc_server_tls_truststore_ref_get_certs(client_auth->ee_cert_bag_ts_ref, &certs, &ee_count)) { + if (nc_server_tls_truststore_ref_get_certs(config, client_auth->ee_cert_bag_ts_ref, &certs, &ee_count)) { ERR(NULL, "Getting end-entity certificates from the truststore reference \"%s\" failed.", client_auth->ee_cert_bag_ts_ref); return -1; } @@ -845,9 +883,10 @@ nc_accept_tls_session(struct nc_session *session, struct nc_server_tls_opts *opt int rc, timeouted = 0; struct timespec ts_timeout; struct nc_tls_verify_cb_data cb_data = {0}; - struct nc_endpt *referenced_endpt; + const struct nc_endpt *referenced_endpt; void *tls_cfg, *srv_cert, *srv_pkey, *cert_store, *cipher_suites; uint32_t cert_count = 0, ref_cert_count = 0; + const struct nc_server_config *config = session->opts.server.config; tls_cfg = srv_cert = srv_pkey = cert_store = cipher_suites = NULL; @@ -868,25 +907,25 @@ nc_accept_tls_session(struct nc_session *session, struct nc_server_tls_opts *opt } /* load server's key and certificate */ - if (nc_server_tls_load_server_cert_key(opts, &srv_cert, &srv_pkey)) { + if (nc_server_tls_load_server_cert_key(config, opts, &srv_cert, &srv_pkey)) { ERR(session, "Loading server certificate and/or private key failed."); goto fail; } /* load trusted CA certificates */ - if (nc_server_tls_load_trusted_certs(&opts->client_auth, cert_store)) { + if (nc_server_tls_load_trusted_certs(config, &opts->client_auth, cert_store)) { ERR(session, "Loading server CA certs failed."); goto fail; } /* load referenced endpoint's trusted CA certs if set */ if (opts->referenced_endpt_name) { - if (nc_server_endpt_get(opts->referenced_endpt_name, &referenced_endpt)) { + if (nc_server_endpt_get(config, opts->referenced_endpt_name, &referenced_endpt)) { ERR(session, "Referenced endpoint \"%s\" not found.", opts->referenced_endpt_name); goto fail; } - if (nc_server_tls_load_trusted_certs(&referenced_endpt->opts.tls->client_auth, cert_store)) { + if (nc_server_tls_load_trusted_certs(config, &referenced_endpt->opts.tls->client_auth, cert_store)) { ERR(session, "Loading server CA certs from referenced endpoint failed."); goto fail; } @@ -894,11 +933,11 @@ nc_accept_tls_session(struct nc_session *session, struct nc_server_tls_opts *opt /* Check if there are no CA/end entity certs configured, which is a valid config. * However, that would imply not using TLS for auth, which is not (yet) supported */ - if (nc_server_tls_get_num_certs(&opts->client_auth, &cert_count)) { + if (nc_server_tls_get_num_certs(config, &opts->client_auth, &cert_count)) { goto fail; } if (opts->referenced_endpt_name) { - if (nc_server_tls_get_num_certs(&referenced_endpt->opts.tls->client_auth, &ref_cert_count)) { + if (nc_server_tls_get_num_certs(config, &referenced_endpt->opts.tls->client_auth, &ref_cert_count)) { goto fail; } cert_count += ref_cert_count; diff --git a/src/session_wrapper.h b/src/session_wrapper.h index 36713cd4..c4f1032a 100644 --- a/src/session_wrapper.h +++ b/src/session_wrapper.h @@ -248,10 +248,10 @@ int nc_server_tls_verify_cert(void *cert, int depth, int trusted, struct nc_tls_ * @brief Check if the peer certificate matches any configured ee certs. * * @param[in] peer_cert Peer certificate. - * @param[in] opts TLS options. + * @param[in] cb_data Verify callback data with the session and the TLS options. * @return 0 on success, non-zero on fail. */ -int nc_server_tls_verify_peer_cert(void *peer_cert, struct nc_server_tls_opts *opts); +int nc_server_tls_verify_peer_cert(void *peer_cert, struct nc_tls_verify_cb_data *cb_data); /** * @brief Get the subject of the certificate. diff --git a/tests/test_config.c b/tests/test_config.c index 96413d0f..78fb0b11 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -15,13 +15,17 @@ #define _GNU_SOURCE +#include #include +#include #include #include #include #include #include #include +#include +#include #include #include @@ -1125,11 +1129,20 @@ test_ordered_list_move(void **state) /** * @brief Time in seconds the client stalls in its password callback. * - * The server waits for the authentication while holding the configuration READ lock, so this has to be - * longer than ::NC_CONFIG_LOCK_TIMEOUT (10 s) for the test to be meaningful. + * Has to be longer than ::NC_CONFIG_LOCK_TIMEOUT (10 s) so that a configuration update waiting for + * the whole authentication would be dropped instead of applied. */ #define TEST_STALL_AUTH_SLEEP 13 +/** @brief Time in seconds the client stalls when only an in-flight handshake is needed. */ +#define TEST_STALL_AUTH_SLEEP_SHORT 5 + +/** @brief Maximum time in msec anything done while a handshake is stalled may take. */ +#define TEST_NO_BLOCK_TIMEOUT 2000 + +/** @brief Time in seconds the client stalls in its password callback, set by each test. */ +static unsigned int test_stall_auth_sleep = TEST_STALL_AUTH_SLEEP; + /** @brief Time in seconds to wait for a Call Home client to report failed connection attempts. */ #define TEST_CH_WATCH_TIME 4 @@ -1142,6 +1155,7 @@ struct test_ch_threads { pthread_t tids[TEST_CH_TID_MAX]; uint32_t tid_count; char endpt[64]; + char last_endpt[64]; }; /* acquire ctx cb for the Call Home dispatch */ @@ -1191,6 +1205,8 @@ test_ch_new_session_fail_cb(const char *client_name, const char *endpt_name, uin /* the endpoint of the very first failed attempt is the first one in the configuration */ strncpy(threads->endpt, endpt_name, sizeof threads->endpt - 1); } + memset(threads->last_endpt, 0, sizeof threads->last_endpt); + strncpy(threads->last_endpt, endpt_name, sizeof threads->last_endpt - 1); for (i = 0; i < threads->tid_count; ++i) { if (pthread_equal(threads->tids[i], self)) { break; @@ -1390,8 +1406,8 @@ test_stall_auth_password(const char *username, const char *hostname, void *priv) (void) hostname; (void) priv; - /* keep the server waiting for the authentication, it holds the configuration READ lock meanwhile */ - sleep(TEST_STALL_AUTH_SLEEP); + /* keep the server waiting for the authentication */ + sleep(test_stall_auth_sleep); /* a wrong password, the connection is expected to fail */ return strdup("wrong"); @@ -1455,6 +1471,10 @@ test_config_update_during_auth(void **state) struct lyd_node *tree = NULL, *diff = NULL; struct ln2_test_ctx *test_ctx = *state; const struct lys_module *yang_mod; + struct timespec ts_start, ts_end; + int64_t elapsed_ms; + + test_stall_auth_sleep = TEST_STALL_AUTH_SLEEP; yang_mod = ly_ctx_get_module_implemented(test_ctx->ctx, "yang"); assert_non_null(yang_mod); @@ -1489,9 +1509,15 @@ test_config_update_during_auth(void **state) * while holding the configuration READ lock */ sleep(2); - /* this must not be silently dropped */ + /* this must neither be silently dropped nor wait out the stalled authentication */ + clock_gettime(CLOCK_MONOTONIC, &ts_start); ret = nc_server_config_setup_diff(diff); assert_int_equal(ret, 0); + clock_gettime(CLOCK_MONOTONIC, &ts_end); + + elapsed_ms = ((int64_t)(ts_end.tv_sec - ts_start.tv_sec) * 1000) + + ((ts_end.tv_nsec - ts_start.tv_nsec) / 1000000); + assert_true(elapsed_ms < TEST_NO_BLOCK_TIMEOUT); for (i = 0; i < 2; i++) { pthread_join(tids[i], NULL); @@ -1501,6 +1527,305 @@ test_config_update_during_auth(void **state) lyd_free_all(tree); } +/** + * @brief Create the YANG data of a listening SSH endpoint with a password-authenticated user. + * + * @param[in] ctx libyang context. + * @param[in] endpt_name Name of the endpoint. + * @param[in] port Port to listen on. + * @param[out] tree Created YANG data. + */ +static void +test_create_stall_endpt_data(const struct ly_ctx *ctx, const char *endpt_name, uint16_t port, + struct lyd_node **tree) +{ + int ret; + + ret = nc_server_config_add_address_port(ctx, endpt_name, NC_TI_SSH, "127.0.0.1", port, tree); + assert_int_equal(ret, 0); + ret = nc_server_config_add_ssh_hostkey(ctx, endpt_name, "hostkey", TESTS_DIR "/data/key_ecdsa", + NULL, tree); + assert_int_equal(ret, 0); + ret = nc_server_config_add_ssh_user_password(ctx, endpt_name, "stall", "correct", tree); + assert_int_equal(ret, 0); + + /* add all the default nodes, the authentication timeout has to be longer than the stall */ + ret = lyd_new_implicit_tree(*tree, LYD_IMPLICIT_NO_STATE, NULL); + assert_int_equal(ret, 0); +} + +/** + * @brief Try to establish a TCP connection to a local port. + * + * @param[in] port Port to connect to. + * @return 0 if the connection was established, -1 if it was refused. + */ +static int +test_tcp_connect(uint16_t port) +{ + int sock, r; + struct sockaddr_in addr = {0}; + + sock = socket(AF_INET, SOCK_STREAM, 0); + assert_true(sock > -1); + + addr.sin_family = AF_INET; + addr.sin_port = htons(port); + addr.sin_addr.s_addr = inet_addr("127.0.0.1"); + + r = connect(sock, (struct sockaddr *)&addr, sizeof addr); + close(sock); + + return r ? -1 : 0; +} + +/** + * @brief Removing an endpoint must stop its listening socket right away. + * + * A stalled handshake keeps a reference to the configuration generation the endpoint belongs to, but + * the listening socket lives outside of it, so it is closed as soon as the update is applied. + */ +static void +test_removed_endpt_stops_listening(void **state) +{ + int ret, i; + pthread_t tids[2]; + struct lyd_node *tree = NULL; + struct ln2_test_ctx *test_ctx = *state; + + test_stall_auth_sleep = TEST_STALL_AUTH_SLEEP_SHORT; + + test_create_stall_endpt_data(test_ctx->ctx, "endpt", TEST_PORT, &tree); + ret = nc_server_config_setup_data(tree); + assert_int_equal(ret, 0); + + /* the endpoint is listening now */ + assert_int_equal(test_tcp_connect(TEST_PORT), 0); + + ret = pthread_create(&tids[0], NULL, test_stall_auth_client_thread, test_ctx); + assert_int_equal(ret, 0); + ret = pthread_create(&tids[1], NULL, test_stall_auth_server_thread, test_ctx); + assert_int_equal(ret, 0); + + /* let the key exchange finish, the server is now stalled in the authentication */ + sleep(2); + + /* remove all the endpoints, only the keystore and the truststore are left */ + ret = nc_server_config_setup_data(test_ctx->test_data); + assert_int_equal(ret, 0); + + /* the socket must be gone even though the stalled handshake still uses the old generation */ + assert_int_equal(test_tcp_connect(TEST_PORT), -1); + + for (i = 0; i < 2; i++) { + pthread_join(tids[i], NULL); + } + + lyd_free_all(tree); +} + +/** + * @brief The API-settable options must be settable while a handshake is in flight. + */ +static void +test_api_setters_during_auth(void **state) +{ + int ret, i; + pthread_t tids[2]; + struct lyd_node *tree = NULL; + struct ln2_test_ctx *test_ctx = *state; + struct timespec ts_start, ts_end; + int64_t elapsed_ms; + + test_stall_auth_sleep = TEST_STALL_AUTH_SLEEP_SHORT; + + test_create_stall_endpt_data(test_ctx->ctx, "endpt", TEST_PORT, &tree); + ret = nc_server_config_setup_data(tree); + assert_int_equal(ret, 0); + + ret = pthread_create(&tids[0], NULL, test_stall_auth_client_thread, test_ctx); + assert_int_equal(ret, 0); + ret = pthread_create(&tids[1], NULL, test_stall_auth_server_thread, test_ctx); + assert_int_equal(ret, 0); + + /* let the key exchange finish, the server is now stalled in the authentication */ + sleep(2); + + clock_gettime(CLOCK_MONOTONIC, &ts_start); + + ret = nc_server_ssh_set_protocol_string("test"); + assert_int_equal(ret, 0); + nc_server_tls_set_verify_clb(NULL); + /* returns an error without libpam support, which is fine, it must just not block */ + nc_server_ssh_set_pam_conf_filename("netconf"); + ret = nc_server_ssh_set_authkey_path_format("/tmp/%u/authorized_keys"); + assert_int_equal(ret, 0); + ret = nc_server_set_unix_socket_dir("/tmp"); + assert_int_equal(ret, 0); + + clock_gettime(CLOCK_MONOTONIC, &ts_end); + elapsed_ms = ((int64_t)(ts_end.tv_sec - ts_start.tv_sec) * 1000) + + ((ts_end.tv_nsec - ts_start.tv_nsec) / 1000000); + assert_true(elapsed_ms < TEST_NO_BLOCK_TIMEOUT); + + for (i = 0; i < 2; i++) { + pthread_join(tids[i], NULL); + } + + lyd_free_all(tree); +} + +/** + * @brief Wait until the Call Home client reports a failed attempt on the given endpoint. + * + * @param[in] threads Call Home thread tracking data. + * @param[in] endpt_name Expected endpoint name. + */ +static void +test_ch_wait_for_endpt(struct test_ch_threads *threads, const char *endpt_name) +{ + int ret; + struct timespec ts; + + pthread_mutex_lock(&threads->lock); + while (strcmp(threads->last_endpt, endpt_name)) { + clock_gettime(CLOCK_REALTIME, &ts); + ts.tv_sec += 10; + ret = pthread_cond_timedwait(&threads->cond, &threads->lock, &ts); + assert_int_equal(ret, 0); + } + pthread_mutex_unlock(&threads->lock); +} + +/** + * @brief A running Call Home thread must survive a configuration swap and pick up the new endpoints. + */ +static void +test_ch_survives_config_swap(void **state) +{ + int ret; + uint32_t tid_count; + struct lyd_node *tree = NULL, *tree2 = NULL; + struct ln2_test_ctx *test_ctx = *state; + struct test_ch_threads threads = {0}; + + pthread_mutex_init(&threads.lock, NULL); + pthread_cond_init(&threads.cond, NULL); + + /* a client with a single endpoint that can never connect anywhere */ + test_create_ch_endpt_data(test_ctx->ctx, "ch", "first", &tree); + ret = nc_server_config_add_ch_persistent(test_ctx->ctx, "ch", &tree); + assert_int_equal(ret, 0); + ret = nc_server_config_add_ch_reconnect_strategy(test_ctx->ctx, "ch", NC_CH_FIRST_LISTED, 1, 3, &tree); + assert_int_equal(ret, 0); + + nc_server_ch_set_dispatch_data(test_ch_acquire_ctx_cb, test_ch_release_ctx_cb, test_ctx, + test_ch_new_session_cb, NULL); + nc_server_ch_set_new_session_fail_cb(test_ch_new_session_fail_cb, &threads); + + ret = nc_server_config_setup_data(tree); + assert_int_equal(ret, 0); + + /* the thread is running and attempting to connect to the only endpoint */ + test_ch_wait_for_endpt(&threads, "first"); + + pthread_mutex_lock(&threads.lock); + assert_int_equal(threads.tid_count, 1); + pthread_mutex_unlock(&threads.lock); + + /* replace the whole configuration, the client keeps its name but gets a different endpoint */ + test_create_ch_endpt_data(test_ctx->ctx, "ch", "second", &tree2); + ret = nc_server_config_add_ch_persistent(test_ctx->ctx, "ch", &tree2); + assert_int_equal(ret, 0); + ret = nc_server_config_add_ch_reconnect_strategy(test_ctx->ctx, "ch", NC_CH_FIRST_LISTED, 1, 3, &tree2); + assert_int_equal(ret, 0); + + ret = nc_server_config_setup_data(tree2); + assert_int_equal(ret, 0); + + /* the very same thread must pick the new endpoint up */ + test_ch_wait_for_endpt(&threads, "second"); + + pthread_mutex_lock(&threads.lock); + tid_count = threads.tid_count; + pthread_mutex_unlock(&threads.lock); + assert_int_equal(tid_count, 1); + + lyd_free_all(tree2); + lyd_free_all(tree); + pthread_cond_destroy(&threads.cond); + pthread_mutex_destroy(&threads.lock); +} + +/** @brief Number of threads applying the configuration concurrently. */ +#define TEST_APPLY_THREAD_COUNT 4 + +/** @brief Number of configuration updates each applying thread performs. */ +#define TEST_APPLY_COUNT 10 + +struct test_apply_arg { + struct ln2_test_ctx *test_ctx; + struct lyd_node *tree; +}; + +static void * +test_apply_thread(void *arg) +{ + struct test_apply_arg *apply_arg = arg; + int ret, i; + + for (i = 0; i < TEST_APPLY_COUNT; ++i) { + ret = nc_server_config_setup_data(apply_arg->tree); + assert_int_equal(ret, 0); + } + + return NULL; +} + +/** + * @brief Several threads applying the configuration while a handshake is stalled. + * + * Every apply publishes a new configuration generation while the stalled handshake holds a reference + * to an older one, so the valgrind twin of this test is what actually checks the refcounting. + */ +static void +test_concurrent_apply_and_accept(void **state) +{ + int ret, i; + pthread_t tids[2 + TEST_APPLY_THREAD_COUNT]; + struct lyd_node *tree = NULL; + struct ln2_test_ctx *test_ctx = *state; + struct test_apply_arg apply_arg; + + test_stall_auth_sleep = TEST_STALL_AUTH_SLEEP_SHORT; + + test_create_stall_endpt_data(test_ctx->ctx, "endpt", TEST_PORT, &tree); + ret = nc_server_config_setup_data(tree); + assert_int_equal(ret, 0); + + apply_arg.test_ctx = test_ctx; + apply_arg.tree = tree; + + ret = pthread_create(&tids[0], NULL, test_stall_auth_client_thread, test_ctx); + assert_int_equal(ret, 0); + ret = pthread_create(&tids[1], NULL, test_stall_auth_server_thread, test_ctx); + assert_int_equal(ret, 0); + + /* let the key exchange finish, the server is now stalled in the authentication */ + sleep(2); + + for (i = 0; i < TEST_APPLY_THREAD_COUNT; ++i) { + ret = pthread_create(&tids[2 + i], NULL, test_apply_thread, &apply_arg); + assert_int_equal(ret, 0); + } + + for (i = 0; i < 2 + TEST_APPLY_THREAD_COUNT; i++) { + pthread_join(tids[i], NULL); + } + + lyd_free_all(tree); +} + static void test_config_data_free(void *data) { @@ -1559,6 +1884,10 @@ main(void) cmocka_unit_test_setup_teardown(test_ch_dispatch_not_duplicated, setup_f, ln2_glob_test_teardown), cmocka_unit_test_setup_teardown(test_ch_endpoint_order, setup_f, ln2_glob_test_teardown), cmocka_unit_test_setup_teardown(test_config_update_during_auth, setup_f, ln2_glob_test_teardown), + cmocka_unit_test_setup_teardown(test_removed_endpt_stops_listening, setup_f, ln2_glob_test_teardown), + cmocka_unit_test_setup_teardown(test_api_setters_during_auth, setup_f, ln2_glob_test_teardown), + cmocka_unit_test_setup_teardown(test_ch_survives_config_swap, setup_f, ln2_glob_test_teardown), + cmocka_unit_test_setup_teardown(test_concurrent_apply_and_accept, setup_f, ln2_glob_test_teardown), }; /* try to get ports from the environment, otherwise use the default */