From ea053b06bfecd2433aa4aeebcc8144e5880d041a Mon Sep 17 00:00:00 2001 From: Roytak Date: Wed, 26 Nov 2025 16:43:29 +0900 Subject: [PATCH 1/6] server config UPDATE hidden unix socket path --- doc/libnetconf.doc | 28 ++- ...libnetconf2-netconf-server@2025-11-11.yang | 37 +++- src/server_config.c | 123 ++++++++++- src/server_config_util.c | 25 ++- src/session_client.c | 2 +- src/session_p.h | 48 +++-- src/session_server.c | 122 ++++++++++- src/session_server.h | 20 ++ tests/data/config.xml | 2 +- tests/test_config.c | 4 + tests/test_unix_socket.c | 203 +++++++++++++----- 11 files changed, 510 insertions(+), 104 deletions(-) diff --git a/doc/libnetconf.doc b/doc/libnetconf.doc index 620b76ee..0cf162d9 100644 --- a/doc/libnetconf.doc +++ b/doc/libnetconf.doc @@ -304,8 +304,8 @@ * - ::nc_server_set_capab_withdefaults() * - ::nc_server_set_capability() * - ::nc_server_endpt_count() - * - ::nc_server_add_endpt_unix_socket_listen() - * - ::nc_server_del_endpt_unix_socket() + * - ::nc_server_set_unix_socket_path() + * - ::nc_server_get_unix_socket_path() * * Server Configuration * === @@ -383,7 +383,7 @@ * You may create this data yourself or by using ::nc_server_config_add_ssh_hostkey(). * * It is important to decide whether the users that can connect to the SSH server should be obtained from the configuration or from the system. - * If the YANG feature *local-users-supported* is enabled (the default), then the authorized users are derived from the configuration. + * If the YANG feature *local-users-supported* is enabled (the default), then the authorized users are derived from the configuration. * When a client connects to the server, he must be found in the configuration and he must authenticate to **all** of his configured authentication methods. * If the feature is disabled, then the system will be used to try to authenticate the client via one of the three * methods - publickey, keyboard-interactive or password (only one of them has to succeed). @@ -493,6 +493,28 @@ * - ::nc_server_config_add_tls_ctn() * - ::nc_server_config_del_tls_ctn() * +* UNIX Socket + * =========== + * + * A UNIX socket endpoint can be established using one of two mechanisms: + * + * 1) **Cleartext Path**: The filesystem path is explicitly stored in the configuration. + * To use this, pass a valid path string to ::nc_server_config_add_unix_socket(). + * + * 2) **Hidden Path**: The filesystem path is managed via the API and is not visible + * in the YANG configuration. To use this, pass NULL as the path argument to + * ::nc_server_config_add_unix_socket(). The actual runtime path must then be set + * using ::nc_server_set_unix_socket_path(). + * + * Security Recommendation + * ----------------------- + * The **Hidden Path** (Option 2) is strongly recommended. + * + * If Cleartext paths are enabled, any user with permission to modify the server + * configuration can change the UNIX socket path via YANG. This allows them to + * force the server to create or overwrite arbitrary files on the filesystem + * with the privileges of the server process. + * * FD * == * diff --git a/modules/libnetconf2-netconf-server@2025-11-11.yang b/modules/libnetconf2-netconf-server@2025-11-11.yang index 86210180..9ef34784 100644 --- a/modules/libnetconf2-netconf-server@2025-11-11.yang +++ b/modules/libnetconf2-netconf-server@2025-11-11.yang @@ -51,6 +51,13 @@ module libnetconf2-netconf-server { description "Second revision."; } + // Features + + feature unix-socket-path { + description + "Indicates that the server supports configuration of the UNIX socket path."; + } + // Identities /* @@ -310,17 +317,31 @@ module libnetconf2-netconf-server { and listen for incoming NETCONF connections. Client authentication is based on the connecting process's effective user ID."; - leaf path { - type string { - length "1..107"; - } + choice socket-path-config { mandatory true; description - "Filesystem path where the UNIX socket will be created. - The parent directory - must exist and be writable by the NETCONF server process. + "Selects how the UNIX domain socket path is determined."; + case socket-path { + if-feature "unix-socket-path"; + leaf socket-path { + type string { + length "1..107"; + } + description + "The explicit filesystem path where the UNIX socket will be bonded. + The parent directory must exist and be writable by the server process. - Example: /var/run/netconf.sock"; + Example: /var/run/netconf.sock"; + } + } + case hidden-path { + leaf hidden-path { + type empty; + description + "Indicates that the UNIX socket path is not configured via YANG, but is instead + determined by internal server API settings."; + } + } } container socket-permissions { diff --git a/src/server_config.c b/src/server_config.c index 6a8de640..bb21a053 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -360,6 +360,7 @@ nc_server_config_free(struct nc_server_config *config) struct nc_ch_client *ch_client; struct nc_ch_endpt *ch_endpt; LY_ARRAY_COUNT_TYPE i, j; + const char *socket_path = NULL; if (!config) { return; @@ -375,14 +376,23 @@ 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 */ LY_ARRAY_FOR(endpt->binds, j) { - free(endpt->binds[j].address); 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); pthread_mutex_destroy(&endpt->bind_lock); } LY_ARRAY_FREE(endpt->binds); @@ -2851,19 +2861,25 @@ config_tls(const struct lyd_node *node, enum nc_operation parent_op, struct nc_e #endif /* NC_ENABLED_SSH_TLS */ static int -config_unix_path(const struct lyd_node *node, enum nc_operation parent_op, struct nc_endpt *endpt) +config_unix_socket_path(const struct lyd_node *node, enum nc_operation parent_op, struct nc_endpt *endpt) { enum nc_operation op; struct nc_bind *bind; + struct nc_server_unix_opts *opts; NC_NODE_GET_OP(node, parent_op, &op); + opts = endpt->opts.unix; + 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() */ assert(endpt->binds); free(endpt->binds[0].address); LY_ARRAY_FREE(endpt->binds); + + /* also clear the cleartext path flag */ + opts->path_type = NC_UNIX_SOCKET_PATH_UNKNOWN; } else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) { /* the endpoint must not have any bindings yet, so we can just create one */ assert(!endpt->binds); @@ -2871,6 +2887,42 @@ config_unix_path(const struct lyd_node *node, enum nc_operation parent_op, struc 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; + } + + return 0; +} + +static int +config_unix_hidden_path(const struct lyd_node *node, enum nc_operation parent_op, struct nc_endpt *endpt) +{ + enum nc_operation op; + struct nc_bind *bind; + struct nc_server_unix_opts *opts; + + NC_NODE_GET_OP(node, parent_op, &op); + + opts = endpt->opts.unix; + + 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() */ + assert(endpt->binds); + LY_ARRAY_FREE(endpt->binds); + + /* also clear the hidden path flag */ + opts->path_type = NC_UNIX_SOCKET_PATH_UNKNOWN; + } else if ((op == NC_OP_CREATE) || (op == NC_OP_REPLACE)) { + /* the endpoint must not have any bindings yet, so we can just create one + * and since the path is hidden, there is no address to set */ + assert(!endpt->binds); + 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; } return 0; @@ -3117,7 +3169,7 @@ config_unix_client_auth(const struct lyd_node *node, enum nc_operation parent_op static int config_unix(const struct lyd_node *node, enum nc_operation parent_op, struct nc_endpt *endpt) { - struct lyd_node *n; + struct lyd_node *n, *socket_path = NULL, *hidden_path = NULL; enum nc_operation op; NC_NODE_GET_OP(node, parent_op, &op); @@ -3135,9 +3187,15 @@ config_unix(const struct lyd_node *node, enum nc_operation parent_op, struct nc_ endpt->opts.unix->gid = (gid_t)-1; } - /* config path */ - NC_CHECK_RET(nc_lyd_find_child(node, "path", 1, &n)); - NC_CHECK_RET(config_unix_path(n, op, endpt)); + /* config mandatory unix socket path choice => only one of them can be present */ + NC_CHECK_RET(nc_lyd_find_child(node, "socket-path", 0, &socket_path)); + NC_CHECK_RET(nc_lyd_find_child(node, "hidden-path", 0, &hidden_path)); + if (socket_path) { + NC_CHECK_RET(config_unix_socket_path(socket_path, op, endpt)); + } else { + assert(hidden_path); + NC_CHECK_RET(config_unix_hidden_path(hidden_path, op, endpt)); + } /* config socket permissions */ NC_CHECK_RET(nc_lyd_find_child(node, "socket-permissions", 1, &n)); @@ -4962,6 +5020,49 @@ 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) +{ + const char *addr1, *addr2; + + 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 */ + return 0; + } + + if (strcmp(addr1, addr2) || (b1->port != b2->port)) { + /* different addresses or ports */ + return 0; + } + + return 1; +} + /** * @brief Atomically starts listening on new sockets and reuses existing ones. * @@ -4989,7 +5090,7 @@ nc_server_config_reconcile_sockets_listen(struct nc_server_config *old_cfg, 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 (!strcmp(new_bind->address, old_bind->address) && (new_bind->port == old_bind->port)) { + 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; @@ -5510,6 +5611,8 @@ nc_server_config_unix_dup(const struct nc_server_unix_opts *src, struct nc_serve *dst = calloc(1, sizeof **dst); NC_CHECK_ERRMEM_RET(!*dst, 1); + (*dst)->path_type = src->path_type; + (*dst)->mode = src->mode; (*dst)->uid = src->uid; (*dst)->gid = src->gid; @@ -5739,8 +5842,10 @@ nc_server_config_dup(const struct nc_server_config *src, struct nc_server_config /* binds */ LN2_LY_ARRAY_CREATE_GOTO_WRAP(dst_endpt->binds, LY_ARRAY_COUNT(src_endpt->binds), rc, cleanup); LY_ARRAY_FOR(src_endpt->binds, j) { - dst_endpt->binds[j].address = strdup(src_endpt->binds[j].address); - NC_CHECK_ERRMEM_GOTO(!dst_endpt->binds[j].address, rc = 1, cleanup); + if (src_endpt->binds[j].address) { + dst_endpt->binds[j].address = strdup(src_endpt->binds[j].address); + 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() */ diff --git a/src/server_config_util.c b/src/server_config_util.c index 36e21122..86d25e50 100644 --- a/src/server_config_util.c +++ b/src/server_config_util.c @@ -238,12 +238,31 @@ nc_server_config_add_unix_socket(const struct ly_ctx *ctx, const char *endpt_nam { int rc = 0; char *path_fmt = NULL; + struct lys_module *mod; + const char *path_type_str; - NC_CHECK_ARG_RET(NULL, ctx, path, config, 1); + NC_CHECK_ARG_RET(NULL, ctx, config, 1); - /* create the path to the socket's path */ + if (!path) { + /* creating a hidden UNIX socket path set by other means */ + path_type_str = "hidden-path"; + } else { + /* creating a standard UNIX socket path */ + path_type_str = "socket-path"; + + /* check if the 'unix-socket-path' feature is enabled in the libnetconf2-netconf-server module */ + mod = ly_ctx_get_module_implemented(ctx, "libnetconf2-netconf-server"); + NC_CHECK_RET(!mod, 1); + if (lys_feature_value(mod, "unix-socket-path")) { + ERR(NULL, "Unable to set UNIX socket path ('unix-socket-path' feature not enabled in " + "'libnetconf2-netconf-server' module)."); + return 1; + } + } + + /* create the path to UNIX socket or just the empty leaf for hidden path */ NC_CHECK_ERR_RET(asprintf(&path_fmt, "/ietf-netconf-server:netconf-server/listen/endpoints/endpoint[name='%s']/" - "libnetconf2-netconf-server:unix/path", endpt_name) == -1, ERRMEM, 1); + "libnetconf2-netconf-server:unix/%s", endpt_name, path_type_str) == -1, ERRMEM, 1); NC_CHECK_GOTO(rc = nc_server_config_create(ctx, config, path, path_fmt), cleanup); if (!mode && !owner && !group) { diff --git a/src/session_client.c b/src/session_client.c index 089d17b1..6d7ddec6 100644 --- a/src/session_client.c +++ b/src/session_client.c @@ -1899,7 +1899,7 @@ nc_accept_callhome(int timeout, struct ly_ctx *ctx, struct nc_session **session) return -1; } - ret = nc_sock_accept_binds(client_opts.ch_binds, client_opts.ch_bind_count, &client_opts.ch_bind_lock, timeout, + ret = nc_sock_accept_binds(NULL, client_opts.ch_binds, client_opts.ch_bind_count, &client_opts.ch_bind_lock, timeout, &host, &port, &idx, &sock); if (ret < 1) { free(host); diff --git a/src/session_p.h b/src/session_p.h index 1219b91e..e783b6e0 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -398,13 +398,24 @@ struct nc_keepalives { uint16_t probe_interval; /**< Probe interval. */ }; +/** + * @brief Enumeration of UNIX socket path types. + */ +enum nc_unix_socket_path_type { + NC_UNIX_SOCKET_PATH_UNKNOWN = 0, /**< Unknown or uninitialized path type. */ + NC_UNIX_SOCKET_PATH_HIDDEN, /**< Socket path is hidden from YANG; configured via API. */ + NC_UNIX_SOCKET_PATH_FILE /**< Socket path is explicitly configured in YANG. */ +}; + /** * @brief Server options for configuring the UNIX transport protocol. */ struct nc_server_unix_opts { - mode_t mode; /**< Socket file permissions (defaults to rw-rw----). */ - uid_t uid; /**< Owner of the socket file. */ - gid_t gid; /**< Group owner of the socket file. */ + enum nc_unix_socket_path_type path_type; /**< Method used to determine the socket path. */ + + mode_t mode; /**< Socket file permissions (defaults to rw-rw----). */ + uid_t uid; /**< Owner of the socket file. */ + gid_t gid; /**< Group owner of the socket file. */ struct nc_server_unix_user_mapping { char *system_user; /**< System username for authentication. */ @@ -735,6 +746,16 @@ struct nc_server_opts { } cert_exp_notif; #endif /* NC_ENABLED_SSH_TLS */ + /** + * @brief Entry for a UNIX socket path configured via API (Hidden). + * + * These paths are not part of the YANG configuration and are managed manually by the application. + */ + struct nc_server_unix_path_entry { + char *endpt_name; /**< Name of the endpoint using this path. */ + char *path; /**< The actual filesystem path for the socket. */ + } *unix_paths; /**< Array of UNIX socket paths set via API (sized-array, see libyang docs). */ + /* ACCESS unlocked */ ATOMIC_T new_session_id; @@ -953,6 +974,14 @@ void nc_client_monitoring_session_stop(struct nc_session *session, int lock); void *nc_realloc(void *ptr, size_t size); +/** + * @brief Get the UNIX socket path for the given endpoint. + * + * @param[in] endpt Endpoint to get the socket path for. + * @return Socket path, NULL on error. + */ +const char *nc_server_unix_get_socket_path(const struct nc_endpt *endpt); + /** * @brief Bind and listen on a socket for the given endpoint and its bind. * @@ -962,14 +991,6 @@ void *nc_realloc(void *ptr, size_t size); */ int nc_server_bind_and_listen(struct nc_endpt *endpt, struct nc_bind *bind); -/** - * @brief Frees memory allocated by a UNIX socket endpoint. - * - * @param[in] endpt UNIX socket endpoint. - * @param[in] bind UNIX socket bind. - */ -void _nc_server_del_endpt_unix_socket(struct nc_endpt *endpt, struct nc_bind *bind); - /** * @brief Free server configuration data (only YANG config data). * @@ -1180,6 +1201,7 @@ int nc_sock_listen_inet(const char *address, uint16_t port); /** * @brief Accept a new connection on a listening socket. * + * @param[in] endpt Optional endpoint the binds belong to (only for logging purposes). * @param[in] binds Structure with the listening sockets. * @param[in] bind_count Number of @p binds. * @param[in] bind_lock Lock for avoiding concurrent poll/accept on a single bind. @@ -1192,8 +1214,8 @@ int nc_sock_listen_inet(const char *address, uint16_t port); * @return 0 on timeout. * @return 1 if a socket was accepted. */ -int nc_sock_accept_binds(struct nc_bind *binds, uint16_t bind_count, pthread_mutex_t *bind_lock, int timeout, - char **host, uint16_t *port, uint16_t *idx, int *sock); +int nc_sock_accept_binds(struct nc_endpt *endpt, struct nc_bind *binds, uint16_t bind_count, + pthread_mutex_t *bind_lock, int timeout, char **host, uint16_t *port, uint16_t *idx, int *sock); /** * @brief Establish a UNIX transport session. diff --git a/src/session_server.c b/src/session_server.c index d6105965..52345f69 100644 --- a/src/session_server.c +++ b/src/session_server.c @@ -358,6 +358,34 @@ nc_sock_listen_inet(const char *address, uint16_t port) return -1; } +const char * +nc_server_unix_get_socket_path(const struct nc_endpt *endpt) +{ + LY_ARRAY_COUNT_TYPE i; + const char *path = 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 */ + path = endpt->binds[0].address; + } else if (endpt->opts.unix->path_type == NC_UNIX_SOCKET_PATH_HIDDEN) { + /* serach the mappings */ + LY_ARRAY_FOR(server_opts.unix_paths, i) { + if (!strcmp(server_opts.unix_paths[i].endpt_name, endpt->name)) { + path = server_opts.unix_paths[i].path; + break; + } + } + if (!path) { + ERR(NULL, "UNIX socket path mapping for endpoint \"%s\" not found.", endpt->name); + } + } else { + ERRINT; + } + + return path; +} + /** * @brief Create a listening socket (AF_UNIX). * @@ -511,8 +539,8 @@ sock_host_inet6(const struct sockaddr_in6 *addr, char **host, uint16_t *port) } int -nc_sock_accept_binds(struct nc_bind *binds, uint16_t bind_count, pthread_mutex_t *bind_lock, int timeout, char **host, - uint16_t *port, uint16_t *idx, int *sock) +nc_sock_accept_binds(struct nc_endpt *endpt, struct nc_bind *binds, uint16_t bind_count, + pthread_mutex_t *bind_lock, int timeout, char **host, uint16_t *port, uint16_t *idx, int *sock) { uint16_t i, j, pfd_count, client_port; char *client_address; @@ -620,7 +648,7 @@ nc_sock_accept_binds(struct nc_bind *binds, uint16_t bind_count, pthread_mutex_t } if (saddr.ss_family == AF_UNIX) { - VRB(NULL, "Accepted a connection on %s.", binds[i].address); + VRB(NULL, "Accepted a connection on %s.", endpt ? nc_server_unix_get_socket_path(endpt) : "UNIX socket"); } else { VRB(NULL, "Accepted a connection on %s:%u from %s:%u.", binds[i].address, binds[i].port, client_address, client_port); } @@ -953,9 +981,6 @@ nc_server_destroy(void) /* CH THREADS UNLOCK */ pthread_rwlock_unlock(&server_opts.ch_threads_lock); - /* CONFIG WR UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); - #ifdef NC_ENABLED_SSH_TLS free(server_opts.authkey_path_fmt); server_opts.authkey_path_fmt = NULL; @@ -966,7 +991,20 @@ nc_server_destroy(void) } server_opts.interactive_auth_data = NULL; server_opts.interactive_auth_data_free = NULL; +#endif /* NC_ENABLED_SSH_TLS */ + + /* hidden UNIX socket paths */ + LY_ARRAY_FOR(server_opts.unix_paths, i) { + free(server_opts.unix_paths[i].endpt_name); + free(server_opts.unix_paths[i].path); + } + LY_ARRAY_FREE(server_opts.unix_paths); + server_opts.unix_paths = NULL; + + /* CONFIG WR UNLOCK */ + pthread_rwlock_unlock(&server_opts.config_lock); +#ifdef NC_ENABLED_SSH_TLS curl_global_cleanup(); nc_tls_backend_destroy_wrap(); ssh_finalize(); @@ -2297,11 +2335,15 @@ nc_ps_clear(struct nc_pollsession *ps, int all, void (*data_free)(void *)) int nc_server_bind_and_listen(struct nc_endpt *endpt, struct nc_bind *bind) { + const char *unix_path = NULL; int sock = -1, rc = 0; /* start listening on the endpoint */ if (endpt->ti == NC_TI_UNIX) { - sock = nc_sock_listen_unix(bind->address, endpt->opts.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); @@ -2319,7 +2361,7 @@ nc_server_bind_and_listen(struct nc_endpt *endpt, struct nc_bind *bind) switch (endpt->ti) { case NC_TI_UNIX: - VRB(NULL, "Listening on %s for UNIX connections.", bind->address); + VRB(NULL, "Listening on %s for UNIX connections.", unix_path); break; #ifdef NC_ENABLED_SSH_TLS case NC_TI_SSH: @@ -2573,7 +2615,7 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) /* try to accept a new connection on any of the endpoints */ LY_ARRAY_FOR(config->endpts, endpt_idx) { - ret = nc_sock_accept_binds(config->endpts[endpt_idx].binds, LY_ARRAY_COUNT(config->endpts[endpt_idx].binds), + ret = nc_sock_accept_binds(&config->endpts[endpt_idx], config->endpts[endpt_idx].binds, LY_ARRAY_COUNT(config->endpts[endpt_idx].binds), &config->endpts[endpt_idx].bind_lock, timeout, &host, &port, NULL, &sock); if (ret < 0) { msgtype = NC_MSG_ERROR; @@ -4278,3 +4320,65 @@ nc_server_is_mod_ignored(const char *mod_name) return ignored; } + +API int +nc_server_set_unix_socket_path(const char *endpoint_name, const char *socket_path) +{ + int rc = 0; + LY_ARRAY_COUNT_TYPE i; + struct nc_server_unix_path_entry *pentry = NULL; + + NC_CHECK_ARG_RET(NULL, endpoint_name, socket_path, 1); + + /* CONFIG WRITE LOCK */ + pthread_rwlock_wrlock(&server_opts.config_lock); + + /* try to see if the path for this endpoint already exists */ + LY_ARRAY_FOR(server_opts.unix_paths, i) { + if (!strcmp(server_opts.unix_paths[i].endpt_name, endpoint_name)) { + pentry = &server_opts.unix_paths[i]; + break; + } + } + if (!pentry) { + /* create a new entry */ + LY_ARRAY_NEW_GOTO(NULL, server_opts.unix_paths, pentry, rc, cleanup); + pentry->endpt_name = strdup(endpoint_name); + NC_CHECK_ERRMEM_GOTO(!pentry->endpt_name, rc = 1, cleanup); + } else { + /* free the old path */ + free(pentry->path); + } + + pentry->path = strdup(socket_path); + NC_CHECK_ERRMEM_GOTO(!pentry->path, rc = 1, cleanup); + +cleanup: + /* CONFIG WRITE UNLOCK */ + pthread_rwlock_unlock(&server_opts.config_lock); + return rc; +} + +API const char * +nc_server_get_unix_socket_path(const char *endpoint_name) +{ + const char *socket_path = NULL; + LY_ARRAY_COUNT_TYPE i; + + NC_CHECK_ARG_RET(NULL, endpoint_name, NULL); + + /* CONFIG READ LOCK */ + pthread_rwlock_rdlock(&server_opts.config_lock); + + /* try to find the path for this endpoint */ + LY_ARRAY_FOR(server_opts.unix_paths, i) { + if (!strcmp(server_opts.unix_paths[i].endpt_name, endpoint_name)) { + socket_path = server_opts.unix_paths[i].path; + break; + } + } + + /* CONFIG READ UNLOCK */ + pthread_rwlock_unlock(&server_opts.config_lock); + return socket_path; +} diff --git a/src/session_server.h b/src/session_server.h index fe3f8728..8eba8f10 100644 --- a/src/session_server.h +++ b/src/session_server.h @@ -447,6 +447,26 @@ NC_MSG_TYPE nc_session_accept_ssh_channel(struct nc_session *orig_session, struc */ NC_MSG_TYPE nc_ps_accept_ssh_channel(struct nc_pollsession *ps, struct nc_session **session); +/** + * @brief Set the UNIX socket path for a given endpoint name. + * + * The @p endpoint_name endpoint must be configured to use hidden UNIX socket path + * for this setting to take effect, see ::nc_server_config_add_unix_socket(). + * + * @param[in] endpoint_name Name of the endpoint. + * @param[in] socket_path UNIX socket path to set. + * @return 0 on success, 1 on error. + */ +int nc_server_set_unix_socket_path(const char *endpoint_name, const char *socket_path); + +/** + * @brief Get the UNIX socket path for a given endpoint name. + * + * @param[in] endpoint_name Name of the endpoint. + * @return UNIX socket path, NULL if not found. + */ +const char *nc_server_get_unix_socket_path(const char *endpoint_name); + /** @} Server Session */ /** diff --git a/tests/data/config.xml b/tests/data/config.xml index 3d6c99ed..ac29742c 100644 --- a/tests/data/config.xml +++ b/tests/data/config.xml @@ -137,7 +137,7 @@ unix - /tmp/netconf-test-server.sock + 0770 diff --git a/tests/test_config.c b/tests/test_config.c index 1c46a12b..36f2b636 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -961,6 +961,10 @@ setup_f(void **state) ret = nc_server_config_setup_data(tree); assert_int_equal(ret, 0); + /* set hidden path for UNIX endpoint */ + ret = nc_server_set_unix_socket_path("unix", "/tmp/netconf-test-server.sock"); + assert_int_equal(ret, 0); + lyd_free_all(tree); return 0; } diff --git a/tests/test_unix_socket.c b/tests/test_unix_socket.c index 45127362..7a0bcdad 100644 --- a/tests/test_unix_socket.c +++ b/tests/test_unix_socket.c @@ -33,55 +33,84 @@ #include "ln2_test.h" #include "nc_client.h" +struct test_unix_socket_data { + const char *username; + const char *socket_path; + int expect_fail; +}; + static int setup_glob_f(void **state) { int ret; struct ln2_test_ctx *test_ctx; - struct lyd_node *config = NULL; ret = ln2_glob_test_setup(&test_ctx); assert_int_equal(ret, 0); *state = test_ctx; - /* create the UNIX socket */ - ret = nc_server_config_add_unix_socket(test_ctx->ctx, - "unix", "/tmp/nc2_test_unix_sock", NULL, NULL, NULL, &config); - assert_int_equal(ret, 0); + test_ctx->test_data = calloc(1, sizeof(struct test_unix_socket_data)); + assert_non_null(test_ctx->test_data); + test_ctx->free_test_data = ln2_glob_test_free_test_data; - ret = nc_server_config_setup_data(config); + /* set two hidden paths for UNIX sockets */ + ret = nc_server_set_unix_socket_path("unix", "/tmp/nc2_test_unix_sock"); + assert_int_equal(ret, 0); + ret = nc_server_set_unix_socket_path("unix2", "/tmp/nc2_test_unix_sock2"); assert_int_equal(ret, 0); - - lyd_free_all(config); return 0; } static int -setup_local_f(void **UNUSED(state)) +setup_local_f(void **state) { + int ret; + struct ln2_test_ctx *test_ctx = *state; + struct lyd_node *config = NULL; + /* set verbosity */ nc_verbosity(NC_VERB_VERBOSE); ly_log_level(LY_LLERR); + /* create the UNIX endpoint, the hidden path will be used */ + ret = nc_server_config_add_unix_socket(test_ctx->ctx, + "unix", NULL, NULL, NULL, NULL, &config); + assert_int_equal(ret, 0); + + ret = nc_server_config_setup_data(config); + assert_int_equal(ret, 0); + + lyd_free_all(config); + return 0; } /* TEST */ static void * -connect_client_thread(void *arg) +test_unix_client_thread(void *arg) { int ret = 0; struct nc_session *session = NULL; struct ln2_test_ctx *test_ctx = arg; + struct test_unix_socket_data *data = test_ctx->test_data; ret = nc_client_set_schema_searchpath(MODULES_DIR); assert_int_equal(ret, 0); + if (data->username) { + ret = nc_client_unix_set_username(data->username); + assert_int_equal(ret, 0); + } + pthread_barrier_wait(&test_ctx->barrier); - session = nc_connect_unix("/tmp/nc2_test_unix_sock", NULL); - assert_non_null(session); + session = nc_connect_unix(data->socket_path, NULL); + if (data->expect_fail) { + assert_null(session); + } else { + assert_non_null(session); + } nc_session_free(session, NULL); return NULL; @@ -92,10 +121,13 @@ test_connect(void **state) { int ret, i; pthread_t tids[2]; + struct ln2_test_ctx *test_ctx = *state; + struct test_unix_socket_data *data = test_ctx->test_data; assert_non_null(state); - ret = pthread_create(&tids[0], NULL, connect_client_thread, *state); + data->socket_path = "/tmp/nc2_test_unix_sock"; + ret = pthread_create(&tids[0], NULL, test_unix_client_thread, *state); assert_int_equal(ret, 0); ret = pthread_create(&tids[1], NULL, ln2_glob_test_server_thread, *state); assert_int_equal(ret, 0); @@ -106,61 +138,31 @@ test_connect(void **state) } /* TEST */ -static void * -invalid_user_client_thread(void *arg) -{ - int ret = 0; - struct nc_session *session = NULL; - struct ln2_test_ctx *test_ctx = arg; - - ret = nc_client_set_schema_searchpath(MODULES_DIR); - assert_int_equal(ret, 0); - - /* set an invalid username */ - nc_client_unix_set_username("INVALID"); - - pthread_barrier_wait(&test_ctx->barrier); - - /* session fails to be created */ - session = nc_connect_unix("/tmp/nc2_test_unix_sock", NULL); - assert_null(session); - - return NULL; -} - -static void * -invalid_user_server_thread(void *arg) -{ - NC_MSG_TYPE msgtype; - struct nc_session *session = NULL; - struct ln2_test_ctx *test_ctx = arg; - - /* wait for the client to be ready to connect */ - pthread_barrier_wait(&test_ctx->barrier); - - /* session of an invalid user is not accepted */ - msgtype = nc_accept(NC_ACCEPT_TIMEOUT, test_ctx->ctx, &session); - assert_int_equal(msgtype, NC_MSG_ERROR); - - return NULL; -} - static void test_invalid_user(void **state) { int ret, i; pthread_t tids[2]; + struct ln2_test_ctx *test_ctx = *state; + struct test_unix_socket_data *data = test_ctx->test_data; assert_non_null(state); - ret = pthread_create(&tids[0], NULL, invalid_user_client_thread, *state); + /* set invalid username, the server will reject it */ + data->socket_path = "/tmp/nc2_test_unix_sock"; + data->expect_fail = 1; + data->username = "INVALID"; + + ret = pthread_create(&tids[0], NULL, test_unix_client_thread, *state); assert_int_equal(ret, 0); - ret = pthread_create(&tids[1], NULL, invalid_user_server_thread, *state); + ret = pthread_create(&tids[1], NULL, ln2_glob_test_server_thread_fail, *state); assert_int_equal(ret, 0); for (i = 0; i < 2; i++) { pthread_join(tids[i], NULL); } + data->expect_fail = 0; + data->username = NULL; } /* TEST */ @@ -279,9 +281,9 @@ auth_server_thread(void *arg) pw = getpwuid(getuid()); assert_non_null(pw); - /* create UNIX user mapping */ + /* create UNIX user mapping for the current user, hidden path is used */ ret = nc_server_config_add_unix_socket(test_ctx->ctx, - "unix", "/tmp/nc2_test_unix_sock", NULL, NULL, NULL, &config); + "unix", NULL, NULL, NULL, NULL, &config); assert_int_equal(ret, 0); ret = nc_server_config_add_unix_user_mapping(test_ctx->ctx, "unix", pw->pw_name, "auth_user", &config); assert_int_equal(ret, 0); @@ -364,7 +366,7 @@ test_config(void **state) /* create the UNIX socket */ ret = nc_server_config_add_unix_socket(test_ctx->ctx, - "unix2", "/tmp/nc2_test_unix_sock2", "0666", pw->pw_name, gr->gr_name, &config); + "unix2", NULL, "0666", pw->pw_name, gr->gr_name, &config); assert_int_equal(ret, 0); ret = nc_server_config_setup_data(config); @@ -380,6 +382,92 @@ test_config(void **state) lyd_free_all(config); } +/* TEST */ +void * +test_unix_server_thread_fail(void *arg) +{ + NC_MSG_TYPE msgtype; + struct nc_session *session = NULL; + struct ln2_test_ctx *test_ctx = arg; + + /* wait for the client to be ready to connect */ + pthread_barrier_wait(&test_ctx->barrier); + + /* try to accept a session, but expect it to time out, as the client should fail to connect, but + * not cause an error on the server side */ + msgtype = nc_accept(NC_ACCEPT_TIMEOUT, test_ctx->ctx, &session); + assert_int_equal(msgtype, NC_MSG_WOULDBLOCK); + assert_null(session); + + return NULL; +} + +static void +test_cleartext_path(void **state) +{ + int ret, i; + struct ln2_test_ctx *test_ctx = *state; + struct lys_module *mod; + const char *ln2_mod_features[] = { + "unix-socket-path", + NULL + }; + struct lyd_node *config = NULL; + pthread_t tid[2]; + struct test_unix_socket_data *data = test_ctx->test_data; + + /* enable the 'cleartext-unixsocket-path' feature */ + mod = ly_ctx_get_module_implemented(test_ctx->ctx, "libnetconf2-netconf-server"); + assert_non_null(mod); + mod = ly_ctx_load_module(test_ctx->ctx, mod->name, mod->revision, ln2_mod_features); + assert_non_null(mod); + + /* create the UNIX socket with a different cleartext path */ + ret = nc_server_config_add_unix_socket(test_ctx->ctx, + "unix2", "/tmp/nc2_test_cleartext_unix_sock", "0666", NULL, NULL, &config); + assert_int_equal(ret, 0); + + ret = nc_server_config_setup_data(config); + assert_int_equal(ret, 0); + + /* start the client and server threads, the client should be able to connect to the cleartext path */ + data->socket_path = "/tmp/nc2_test_cleartext_unix_sock"; + ret = pthread_create(&tid[0], NULL, test_unix_client_thread, *state); + assert_int_equal(ret, 0); + ret = pthread_create(&tid[1], NULL, ln2_glob_test_server_thread, *state); + assert_int_equal(ret, 0); + + for (i = 0; i < 2; i++) { + pthread_join(tid[i], NULL); + } + + lyd_free_all(config); + config = NULL; + + /* set hidden path again */ + ret = nc_server_config_add_unix_socket(test_ctx->ctx, + "unix2", NULL, "0666", NULL, NULL, &config); + assert_int_equal(ret, 0); + + ret = nc_server_config_setup_data(config); + assert_int_equal(ret, 0); + + /* client should fail to connect to the old cleartext path */ + data->expect_fail = 1; + ret = pthread_create(&tid[0], NULL, test_unix_client_thread, *state); + assert_int_equal(ret, 0); + ret = pthread_create(&tid[1], NULL, test_unix_server_thread_fail, *state); + assert_int_equal(ret, 0); + + for (i = 0; i < 2; i++) { + pthread_join(tid[i], NULL); + } + + data->expect_fail = 0; + + lyd_free_all(config); +} + int main(void) { @@ -389,6 +477,7 @@ main(void) cmocka_unit_test_setup(test_proxy, setup_local_f), cmocka_unit_test_setup(test_auth, setup_local_f), cmocka_unit_test_setup(test_config, setup_local_f), + cmocka_unit_test_setup(test_cleartext_path, setup_local_f), }; setenv("CMOCKA_TEST_ABORT", "1", 1); From c90b82d225c9dfc9c78eaab1389e3f51de5d3383 Mon Sep 17 00:00:00 2001 From: Roytak Date: Thu, 27 Nov 2025 13:37:42 +0900 Subject: [PATCH 2/6] examples UPDATE use hidden unix sock path --- examples/server.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/examples/server.c b/examples/server.c index 0aab342d..2c1fdfc6 100644 --- a/examples/server.c +++ b/examples/server.c @@ -235,13 +235,19 @@ init(const char *unix_socket_path, struct ly_ctx **context, struct nc_pollsessio ERR_MSG_CLEANUP("Error while parsing the example configuration data.\n"); } - /* add UNIX socket to the configuration tree if the path was specified */ if (unix_socket_path) { + /* add UNIX socket endpoint to the configuration tree if the path was specified */ rc = nc_server_config_add_unix_socket(*context, "unix-socket-endpt", - unix_socket_path, NULL, NULL, NULL, &config); + NULL, NULL, NULL, NULL, &config); if (rc) { ERR_MSG_CLEANUP("Creating UNIX socket endpoint configuration failed.\n"); } + + /* use the specified path for the UNIX socket endpoint */ + rc = nc_server_set_unix_socket_path("unix-socket-endpt", unix_socket_path); + if (rc) { + ERR_MSG_CLEANUP("Setting UNIX socket path failed.\n"); + } } /* since nc_server_config_setup_data() requires all implicit nodes to be present and the example From e89f5112d4dbedc8ad9e75c4775fd24eb01d0da9 Mon Sep 17 00:00:00 2001 From: Roytak Date: Thu, 27 Nov 2025 16:41:47 +0900 Subject: [PATCH 3/6] server server UPDATE add API to set unix sock basedir --- ...libnetconf2-netconf-server@2025-11-11.yang | 6 +- src/server_config.c | 23 ++- src/session_client.c | 5 +- src/session_p.h | 3 +- src/session_server.c | 146 ++++++++++++++++-- src/session_server.h | 11 ++ tests/test_config.c | 6 +- tests/test_unix_socket.c | 10 +- 8 files changed, 187 insertions(+), 23 deletions(-) diff --git a/modules/libnetconf2-netconf-server@2025-11-11.yang b/modules/libnetconf2-netconf-server@2025-11-11.yang index 9ef34784..69e9ac0c 100644 --- a/modules/libnetconf2-netconf-server@2025-11-11.yang +++ b/modules/libnetconf2-netconf-server@2025-11-11.yang @@ -328,10 +328,10 @@ module libnetconf2-netconf-server { length "1..107"; } description - "The explicit filesystem path where the UNIX socket will be bonded. - The parent directory must exist and be writable by the server process. + "Relative filesystem path where the UNIX socket will be bound. + The parent directory must be set by an internal server API setting. - Example: /var/run/netconf.sock"; + Example: netconf.sock"; } } case hidden-path { diff --git a/src/server_config.c b/src/server_config.c index bb21a053..b07fc229 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -360,7 +360,7 @@ nc_server_config_free(struct nc_server_config *config) struct nc_ch_client *ch_client; struct nc_ch_endpt *ch_endpt; LY_ARRAY_COUNT_TYPE i, j; - const char *socket_path = NULL; + char *socket_path = NULL; if (!config) { return; @@ -397,6 +397,11 @@ nc_server_config_free(struct nc_server_config *config) } 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 @@ -5035,7 +5040,8 @@ 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) { - const char *addr1, *addr2; + int rc = 1; + char *addr1 = NULL, *addr2 = NULL; if (e1->ti != e2->ti) { /* different transport protocols */ @@ -5052,15 +5058,22 @@ nc_server_config_bindings_match(const struct nc_endpt *e1, const struct nc_bind } if (!addr1 || !addr2) { /* unable to get the address */ - return 0; + rc = 0; + goto cleanup; } if (strcmp(addr1, addr2) || (b1->port != b2->port)) { /* different addresses or ports */ - return 0; + rc = 0; + goto cleanup; } - return 1; +cleanup: + if (e1->ti == NC_TI_UNIX) { + free(addr1); + free(addr2); + } + return rc; } /** diff --git a/src/session_client.c b/src/session_client.c index 6d7ddec6..e1014980 100644 --- a/src/session_client.c +++ b/src/session_client.c @@ -1485,8 +1485,11 @@ nc_connect_unix(const char *address, struct ly_ctx *ctx) session->ti.unixsock.sock = sock; sock = -1; - /* socket path */ session->path = strdup(address); + if (!session->path) { + ERRMEM; + goto fail; + } /* NETCONF username */ if (unix_opts.username) { diff --git a/src/session_p.h b/src/session_p.h index e783b6e0..254430c1 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -755,6 +755,7 @@ struct nc_server_opts { char *endpt_name; /**< Name of the endpoint using this path. */ char *path; /**< The actual filesystem path for the socket. */ } *unix_paths; /**< Array of UNIX socket paths set via API (sized-array, see libyang docs). */ + char *unix_socket_dir; /**< Base directory for UNIX sockets created by the server. */ /* ACCESS unlocked */ ATOMIC_T new_session_id; @@ -980,7 +981,7 @@ void *nc_realloc(void *ptr, size_t size); * @param[in] endpt Endpoint to get the socket path for. * @return Socket path, NULL on error. */ -const char *nc_server_unix_get_socket_path(const struct nc_endpt *endpt); +char *nc_server_unix_get_socket_path(const struct nc_endpt *endpt); /** * @brief Bind and listen on a socket for the given endpoint and its bind. diff --git a/src/session_server.c b/src/session_server.c index 52345f69..75b279c5 100644 --- a/src/session_server.c +++ b/src/session_server.c @@ -358,31 +358,136 @@ nc_sock_listen_inet(const char *address, uint16_t port) return -1; } -const char * +/** + * @brief Construct the full path to the UNIX socket. + * + * @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) +{ + 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); + return 1; + } + + if (filename[0] == '/') { + ERR(NULL, "Cannot construct UNIX socket path \"%s\" (absolute path not allowed).", filename); + return 1; + } + + /* construct the path to the UNIX socket */ + if (asprintf(&full_path, "%s/%s", dir, filename) == -1) { + ERRMEM; + rc = 1; + goto cleanup; + } + + if (strlen(full_path) > sizeof(sun.sun_path) - 1) { + ERR(NULL, "Socket path \"%s\" is too long.", full_path); + goto cleanup; + } + + /* ensure the socket path is within the base directory */ + if (!(real_base_dir = realpath(dir, NULL))) { + ERR(NULL, "realpath() failed for UNIX socket base directory \"%s\" (%s).", dir, strerror(errno)); + rc = 1; + goto cleanup; + } + + /* find the last slash in the constructed path */ + last_slash = strrchr(full_path, '/'); + if (last_slash) { + /* extract the directory part of the socket path */ + dir_len = last_slash - full_path; + sock_dir_path = strndup(full_path, dir_len); + NC_CHECK_ERRMEM_GOTO(!sock_dir_path, rc = 1, cleanup); + + if (!(real_target_dir = realpath(sock_dir_path, NULL))) { + ERR(NULL, "realpath() failed for UNIX socket path directory \"%s\" (%s).", sock_dir_path, + strerror(errno)); + rc = 1; + goto cleanup; + } + } else { + /* should not happen as we always add dir/filename */ + real_target_dir = strdup(real_base_dir); + NC_CHECK_ERRMEM_GOTO(!real_target_dir, rc = 1, cleanup); + } + + base_len = strlen(real_base_dir); + + /* check the relationship between both paths */ + is_prefix = (strncmp(real_base_dir, real_target_dir, base_len) == 0); + + is_exact = (real_target_dir[base_len] == '\0'); + + is_subdir = (real_target_dir[base_len] == '/'); + + /* special case if base is '/' */ + if ((base_len == 1) && (real_base_dir[0] == '/')) { + is_subdir = 1; + } + + if (!is_prefix || (!is_exact && !is_subdir)) { + ERR(NULL, "UNIX socket path \"%s\" escapes the base directory \"%s\".", full_path, dir); + rc = 1; + goto cleanup; + } + + /* transfer ownership */ + *path = full_path; + full_path = NULL; + +cleanup: + free(real_base_dir); + free(real_target_dir); + free(sock_dir_path); + free(full_path); + return rc; +} + +char * nc_server_unix_get_socket_path(const struct nc_endpt *endpt) { LY_ARRAY_COUNT_TYPE i; - const char *path = NULL; + const char *filename = NULL; + char *path = 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 */ - path = endpt->binds[0].address; + filename = endpt->binds[0].address; } else if (endpt->opts.unix->path_type == NC_UNIX_SOCKET_PATH_HIDDEN) { - /* serach the mappings */ + /* search the mappings */ LY_ARRAY_FOR(server_opts.unix_paths, i) { if (!strcmp(server_opts.unix_paths[i].endpt_name, endpt->name)) { - path = server_opts.unix_paths[i].path; + filename = server_opts.unix_paths[i].path; break; } } - if (!path) { + if (!filename) { ERR(NULL, "UNIX socket path mapping for endpoint \"%s\" not found.", endpt->name); } } else { ERRINT; } + /* construct the full socket path */ + if (nc_session_unix_construct_socket_path(filename, &path)) { + return NULL; + } + return path; } @@ -543,7 +648,7 @@ nc_sock_accept_binds(struct nc_endpt *endpt, struct nc_bind *binds, uint16_t bin pthread_mutex_t *bind_lock, int timeout, char **host, uint16_t *port, uint16_t *idx, int *sock) { uint16_t i, j, pfd_count, client_port; - char *client_address; + char *client_address, *sockpath = NULL; struct pollfd *pfd; struct sockaddr_storage saddr; socklen_t saddr_len = sizeof(saddr); @@ -648,7 +753,11 @@ nc_sock_accept_binds(struct nc_endpt *endpt, struct nc_bind *binds, uint16_t bin } if (saddr.ss_family == AF_UNIX) { - VRB(NULL, "Accepted a connection on %s.", endpt ? nc_server_unix_get_socket_path(endpt) : "UNIX socket"); + if (endpt) { + sockpath = nc_server_unix_get_socket_path(endpt); + } + VRB(NULL, "Accepted a connection on %s.", sockpath ? sockpath : "UNIX socket"); + free(sockpath); } else { VRB(NULL, "Accepted a connection on %s:%u from %s:%u.", binds[i].address, binds[i].port, client_address, client_port); } @@ -2335,7 +2444,7 @@ nc_ps_clear(struct nc_pollsession *ps, int all, void (*data_free)(void *)) int nc_server_bind_and_listen(struct nc_endpt *endpt, struct nc_bind *bind) { - const char *unix_path = NULL; + char *unix_path = NULL; int sock = -1, rc = 0; /* start listening on the endpoint */ @@ -2378,6 +2487,7 @@ nc_server_bind_and_listen(struct nc_endpt *endpt, struct nc_bind *bind) } cleanup: + free(unix_path); return rc; } @@ -4382,3 +4492,21 @@ nc_server_get_unix_socket_path(const char *endpoint_name) pthread_rwlock_unlock(&server_opts.config_lock); return socket_path; } + +API int +nc_server_set_unix_socket_dir(const char *dir) +{ + int rc = 0; + + /* CONFIG WRITE LOCK */ + pthread_rwlock_wrlock(&server_opts.config_lock); + + free(server_opts.unix_socket_dir); + server_opts.unix_socket_dir = strdup(dir); + NC_CHECK_ERRMEM_GOTO(!server_opts.unix_socket_dir, rc = 1, cleanup); + +cleanup: + /* CONFIG WRITE UNLOCK */ + pthread_rwlock_unlock(&server_opts.config_lock); + return rc; +} diff --git a/src/session_server.h b/src/session_server.h index 8eba8f10..0cb8b50a 100644 --- a/src/session_server.h +++ b/src/session_server.h @@ -467,6 +467,17 @@ int nc_server_set_unix_socket_path(const char *endpoint_name, const char *socket */ const char *nc_server_get_unix_socket_path(const char *endpoint_name); +/** + * @brief Set the base directory for UNIX socket paths. + * + * All UNIX socket paths will be relative to this directory. + * It must exist and be writable by the server process. + * + * @param[in] dir Base directory for UNIX socket paths. + * @return 0 on success, 1 on error. + */ +int nc_server_set_unix_socket_dir(const char *dir); + /** @} Server Session */ /** diff --git a/tests/test_config.c b/tests/test_config.c index 36f2b636..d07f0f14 100644 --- a/tests/test_config.c +++ b/tests/test_config.c @@ -961,8 +961,12 @@ setup_f(void **state) ret = nc_server_config_setup_data(tree); assert_int_equal(ret, 0); + /* set the base directory for UNIX sockets */ + ret = nc_server_set_unix_socket_dir("/tmp"); + assert_int_equal(ret, 0); + /* set hidden path for UNIX endpoint */ - ret = nc_server_set_unix_socket_path("unix", "/tmp/netconf-test-server.sock"); + ret = nc_server_set_unix_socket_path("unix", "netconf-test-server.sock"); assert_int_equal(ret, 0); lyd_free_all(tree); diff --git a/tests/test_unix_socket.c b/tests/test_unix_socket.c index 7a0bcdad..d26f52af 100644 --- a/tests/test_unix_socket.c +++ b/tests/test_unix_socket.c @@ -54,10 +54,14 @@ setup_glob_f(void **state) assert_non_null(test_ctx->test_data); test_ctx->free_test_data = ln2_glob_test_free_test_data; + /* set the base directory for UNIX sockets */ + ret = nc_server_set_unix_socket_dir("/tmp"); + assert_int_equal(ret, 0); + /* set two hidden paths for UNIX sockets */ - ret = nc_server_set_unix_socket_path("unix", "/tmp/nc2_test_unix_sock"); + ret = nc_server_set_unix_socket_path("unix", "nc2_test_unix_sock"); assert_int_equal(ret, 0); - ret = nc_server_set_unix_socket_path("unix2", "/tmp/nc2_test_unix_sock2"); + ret = nc_server_set_unix_socket_path("unix2", "nc2_test_unix_sock2"); assert_int_equal(ret, 0); return 0; @@ -424,7 +428,7 @@ test_cleartext_path(void **state) /* create the UNIX socket with a different cleartext path */ ret = nc_server_config_add_unix_socket(test_ctx->ctx, - "unix2", "/tmp/nc2_test_cleartext_unix_sock", "0666", NULL, NULL, &config); + "unix2", "nc2_test_cleartext_unix_sock", "0666", NULL, NULL, &config); assert_int_equal(ret, 0); ret = nc_server_config_setup_data(config); From cda4ae612906a3146b5714ed42be4ab33353c5b9 Mon Sep 17 00:00:00 2001 From: Roytak Date: Thu, 27 Nov 2025 20:52:52 +0900 Subject: [PATCH 4/6] session server UPDATE add unixsock dir getter --- doc/libnetconf.doc | 15 ++++--- ...libnetconf2-netconf-server@2025-11-11.yang | 6 ++- src/session_server.c | 44 ++++++++++++++++--- src/session_server.h | 13 +++++- 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/doc/libnetconf.doc b/doc/libnetconf.doc index 0cf162d9..3abe81c0 100644 --- a/doc/libnetconf.doc +++ b/doc/libnetconf.doc @@ -493,12 +493,12 @@ * - ::nc_server_config_add_tls_ctn() * - ::nc_server_config_del_tls_ctn() * -* UNIX Socket + * UNIX Socket * =========== * * A UNIX socket endpoint can be established using one of two mechanisms: * - * 1) **Cleartext Path**: The filesystem path is explicitly stored in the configuration. + * 1) **Standard Filesystem Path**: The filesystem path is explicitly stored in the configuration. * To use this, pass a valid path string to ::nc_server_config_add_unix_socket(). * * 2) **Hidden Path**: The filesystem path is managed via the API and is not visible @@ -506,14 +506,19 @@ * ::nc_server_config_add_unix_socket(). The actual runtime path must then be set * using ::nc_server_set_unix_socket_path(). * + * All UNIX sockets require a designated base directory for their creation. + * This directory must be set using ::nc_server_set_unix_socket_dir(). + * A base directory must be set to create any UNIX socket. + * All socket paths will be relative to this base directory. + * * Security Recommendation * ----------------------- * The **Hidden Path** (Option 2) is strongly recommended. * - * If Cleartext paths are enabled, any user with permission to modify the server + * If standard paths are enabled, any user with permission to modify the server * configuration can change the UNIX socket path via YANG. This allows them to - * force the server to create or overwrite arbitrary files on the filesystem - * with the privileges of the server process. + * force the server to create or overwrite arbitrary files in a subdirectory + * set by ::nc_server_set_unix_socket_dir() with the privileges of the server process. * * FD * == diff --git a/modules/libnetconf2-netconf-server@2025-11-11.yang b/modules/libnetconf2-netconf-server@2025-11-11.yang index 69e9ac0c..0ca44712 100644 --- a/modules/libnetconf2-netconf-server@2025-11-11.yang +++ b/modules/libnetconf2-netconf-server@2025-11-11.yang @@ -330,6 +330,7 @@ module libnetconf2-netconf-server { description "Relative filesystem path where the UNIX socket will be bound. The parent directory must be set by an internal server API setting. + The final resolved path must be within the configured parent directory. Example: netconf.sock"; } @@ -339,7 +340,10 @@ module libnetconf2-netconf-server { type empty; description "Indicates that the UNIX socket path is not configured via YANG, but is instead - determined by internal server API settings."; + determined by internal server API settings. + + The parent directory must be set by an internal server API setting. + The final resolved path must be within the configured parent directory."; } } } diff --git a/src/session_server.c b/src/session_server.c index 75b279c5..40895606 100644 --- a/src/session_server.c +++ b/src/session_server.c @@ -4469,13 +4469,16 @@ nc_server_set_unix_socket_path(const char *endpoint_name, const char *socket_pat return rc; } -API const char * -nc_server_get_unix_socket_path(const char *endpoint_name) +API int +nc_server_get_unix_socket_path(const char *endpoint_name, char **socket_path) { - const char *socket_path = NULL; + int rc = 0; + char *p = NULL; LY_ARRAY_COUNT_TYPE i; - NC_CHECK_ARG_RET(NULL, endpoint_name, NULL); + NC_CHECK_ARG_RET(NULL, endpoint_name, socket_path, 1); + + *socket_path = NULL; /* CONFIG READ LOCK */ pthread_rwlock_rdlock(&server_opts.config_lock); @@ -4483,14 +4486,22 @@ nc_server_get_unix_socket_path(const char *endpoint_name) /* try to find the path for this endpoint */ LY_ARRAY_FOR(server_opts.unix_paths, i) { if (!strcmp(server_opts.unix_paths[i].endpt_name, endpoint_name)) { - socket_path = server_opts.unix_paths[i].path; + p = server_opts.unix_paths[i].path; break; } } + if (!p) { + goto cleanup; + } + + *socket_path = strdup(p); + NC_CHECK_ERRMEM_GOTO(!*socket_path, rc = 1, cleanup); + +cleanup: /* CONFIG READ UNLOCK */ pthread_rwlock_unlock(&server_opts.config_lock); - return socket_path; + return rc; } API int @@ -4510,3 +4521,24 @@ nc_server_set_unix_socket_dir(const char *dir) pthread_rwlock_unlock(&server_opts.config_lock); return rc; } + +API int +nc_server_get_unix_socket_dir(char **dir) +{ + int rc = 0; + + *dir = NULL; + + /* CONFIG READ LOCK */ + pthread_rwlock_rdlock(&server_opts.config_lock); + + if (server_opts.unix_socket_dir) { + *dir = strdup(server_opts.unix_socket_dir); + NC_CHECK_ERRMEM_GOTO(!*dir, rc = 1, cleanup); + } + +cleanup: + /* CONFIG READ UNLOCK */ + pthread_rwlock_unlock(&server_opts.config_lock); + return rc; +} diff --git a/src/session_server.h b/src/session_server.h index 0cb8b50a..b81f8509 100644 --- a/src/session_server.h +++ b/src/session_server.h @@ -463,9 +463,10 @@ int nc_server_set_unix_socket_path(const char *endpoint_name, const char *socket * @brief Get the UNIX socket path for a given endpoint name. * * @param[in] endpoint_name Name of the endpoint. - * @return UNIX socket path, NULL if not found. + * @param[out] socket_path Found UNIX socket path. + * @return 0 on success, 1 on error. */ -const char *nc_server_get_unix_socket_path(const char *endpoint_name); +int nc_server_get_unix_socket_path(const char *endpoint_name, char **socket_path); /** * @brief Set the base directory for UNIX socket paths. @@ -478,6 +479,14 @@ const char *nc_server_get_unix_socket_path(const char *endpoint_name); */ int nc_server_set_unix_socket_dir(const char *dir); +/** + * @brief Get the base directory for UNIX socket paths. + * + * @param[out] dir Base directory for UNIX socket paths. + * @return 0 on success, 1 on error. + */ +int nc_server_get_unix_socket_dir(char **dir); + /** @} Server Session */ /** From db90fc9d94163f26f60c26319b37c54be999ba43 Mon Sep 17 00:00:00 2001 From: Roytak Date: Thu, 27 Nov 2025 21:00:23 +0900 Subject: [PATCH 5/6] server config UPDATE replace assert with else --- src/server_config.c | 67 +++++++++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 23 deletions(-) diff --git a/src/server_config.c b/src/server_config.c index b07fc229..9f856551 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -889,9 +889,11 @@ config_hostkey_pubkey_inline(const struct lyd_node *node, enum nc_operation pare NC_CHECK_RET(config_cleartext_privkey_data(cleartext, op, &hostkey->key.privkey)); } else if (hidden) { NC_CHECK_RET(config_hidden_privkey_data(hidden, op)); - } else { - assert(encrypted); + } else if (encrypted) { NC_CHECK_RET(config_encrypted_privkey_data(encrypted, op)); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } if (op == NC_OP_DELETE) { @@ -937,9 +939,11 @@ config_hostkey_public_key(const struct lyd_node *node, enum nc_operation parent_ NC_CHECK_RET(nc_lyd_find_child(node, "central-keystore-reference", 0, &keystore_ref)); if (inline_def) { NC_CHECK_RET(config_hostkey_pubkey_inline(inline_def, op, hostkey)); - } else { - assert(keystore_ref); + } else if (keystore_ref) { NC_CHECK_RET(config_hostkey_pubkey_keystore(keystore_ref, op, hostkey)); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } return 0; @@ -991,10 +995,12 @@ config_ssh_hostkey(const struct lyd_node *node, enum nc_operation parent_op, str if (public_key) { /* config public key */ NC_CHECK_RET(config_hostkey_public_key(public_key, op, hostkey)); - } else { + } else if (certificate) { /* config certificate */ - assert(certificate); NC_CHECK_RET(config_hostkey_certificate(certificate, op)); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } /* all children processed, we can now delete the hostkey */ @@ -1219,9 +1225,11 @@ config_ssh_user_public_keys(const struct lyd_node *node, enum nc_operation paren NC_CHECK_RET(config_ssh_user_pubkey_inline(inline_def, op, user)); } else if (truststore_ref) { NC_CHECK_RET(config_ssh_user_pubkey_truststore(truststore_ref, op, user)); - } else { - assert(system); + } else if (system) { NC_CHECK_RET(config_ssh_user_pubkey_system(system, op, user)); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } return 0; @@ -1942,9 +1950,11 @@ config_tls_server_ident_cert_inline(const struct lyd_node *node, enum nc_operati NC_CHECK_RET(config_cleartext_privkey_data(cleartext, op, &opts->local.key.privkey), 1); } else if (hidden) { NC_CHECK_RET(config_hidden_privkey_data(hidden, op), 1); - } else { - assert(encrypted); + } else if (encrypted) { NC_CHECK_RET(config_encrypted_privkey_data(encrypted, op), 1); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } /* config certificate data */ @@ -2055,9 +2065,11 @@ config_tls_server_ident_certificate(const struct lyd_node *node, enum nc_operati NC_CHECK_RET(nc_lyd_find_child(node, "central-keystore-reference", 0, &keystore_ref)); if (inline_def) { NC_CHECK_RET(config_tls_server_ident_cert_inline(inline_def, op, tls)); - } else { - assert(keystore_ref); + } else if (keystore_ref) { NC_CHECK_RET(config_tls_server_ident_cert_keystore(keystore_ref, op, tls)); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } return 0; @@ -2103,9 +2115,11 @@ config_tls_server_identity(const struct lyd_node *node, enum nc_operation parent NC_CHECK_RET(config_tls_server_ident_raw_key(raw_private_key, op)); } else if (tls12_psk) { NC_CHECK_RET(config_tls_server_ident_tls12_psk(tls12_psk, op)); - } else { - assert(tls13_epsk); + } else if (tls13_epsk) { NC_CHECK_RET(config_tls_server_ident_tls13_epsk(tls13_epsk, op)); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } return 0; @@ -2243,9 +2257,11 @@ config_tls_client_auth_ca_certs(const struct lyd_node *node, enum nc_operation p NC_CHECK_RET(nc_lyd_find_child(node, "central-truststore-reference", 0, &truststore_ref), 1); if (inline_def) { NC_CHECK_RET(config_tls_client_auth_ca_certs_inline(inline_def, op, client_auth)); - } else { - assert(truststore_ref); + } else if (truststore_ref) { NC_CHECK_RET(config_tls_client_auth_ca_certs_truststore(truststore_ref, op, client_auth)); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } return 0; @@ -2365,9 +2381,11 @@ config_tls_client_auth_ee_certs(const struct lyd_node *node, NC_CHECK_RET(nc_lyd_find_child(node, "central-truststore-reference", 0, &truststore_ref), 1); if (inline_def) { NC_CHECK_RET(config_tls_client_auth_ee_certs_inline(inline_def, op, client_auth)); - } else { - assert(truststore_ref); + } else if (truststore_ref) { NC_CHECK_RET(config_tls_client_auth_ee_certs_truststore(truststore_ref, op, client_auth)); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } return 0; @@ -3197,9 +3215,11 @@ config_unix(const struct lyd_node *node, enum nc_operation parent_op, struct nc_ NC_CHECK_RET(nc_lyd_find_child(node, "hidden-path", 0, &hidden_path)); if (socket_path) { NC_CHECK_RET(config_unix_socket_path(socket_path, op, endpt)); - } else { - assert(hidden_path); + } else if (hidden_path) { NC_CHECK_RET(config_unix_hidden_path(hidden_path, op, endpt)); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } /* config socket permissions */ @@ -3279,7 +3299,6 @@ config_endpoint(const struct lyd_node *node, enum nc_operation parent_op, NC_CHECK_RET(nc_lyd_find_child(node, "ssh", 0, &ssh)); NC_CHECK_RET(nc_lyd_find_child(node, "tls", 0, &tls)); NC_CHECK_RET(nc_lyd_find_child(node, "libnetconf2-netconf-server:unix", 0, &unix)); - assert(ssh || tls || unix); #ifdef NC_ENABLED_SSH_TLS if (ssh) { @@ -3618,9 +3637,11 @@ config_ch_client_endpoint(const struct lyd_node *node, enum nc_operation parent_ NC_CHECK_RET(nc_lyd_find_child(node, "tls", 0, &tls)); if (ssh) { NC_CHECK_RET(config_ch_endpoint_ssh(ssh, op, endpt)); - } else { - assert(tls); + } else if (tls) { NC_CHECK_RET(config_ch_endpoint_tls(tls, op, endpt)); + } else { + ERR(NULL, "Invalid YANG data provided."); + return 1; } #endif /* NC_ENABLED_SSH_TLS */ From 348418c94cc837e611ee7bba5ed2a9b17876403f Mon Sep 17 00:00:00 2001 From: Roytak Date: Fri, 28 Nov 2025 11:41:22 +0900 Subject: [PATCH 6/6] session server UPDATE hidden unix path without base --- ...libnetconf2-netconf-server@2025-11-11.yang | 5 +--- src/session_server.c | 23 +++++++++++-------- src/session_server.h | 3 ++- tests/test_unix_socket.c | 4 ++-- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/modules/libnetconf2-netconf-server@2025-11-11.yang b/modules/libnetconf2-netconf-server@2025-11-11.yang index 0ca44712..1ee28e7f 100644 --- a/modules/libnetconf2-netconf-server@2025-11-11.yang +++ b/modules/libnetconf2-netconf-server@2025-11-11.yang @@ -340,10 +340,7 @@ module libnetconf2-netconf-server { type empty; description "Indicates that the UNIX socket path is not configured via YANG, but is instead - determined by internal server API settings. - - The parent directory must be set by an internal server API setting. - The final resolved path must be within the configured parent directory."; + determined by internal server API settings."; } } } diff --git a/src/session_server.c b/src/session_server.c index 40895606..e5af22c5 100644 --- a/src/session_server.c +++ b/src/session_server.c @@ -461,33 +461,36 @@ char * nc_server_unix_get_socket_path(const struct nc_endpt *endpt) { LY_ARRAY_COUNT_TYPE i; - const char *filename = NULL; + const char *p = NULL; char *path = 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 */ - filename = endpt->binds[0].address; + p = endpt->binds[0].address; + + /* it is relative, we need to construct the full path */ + if (nc_session_unix_construct_socket_path(p, &path)) { + return NULL; + } } else if (endpt->opts.unix->path_type == NC_UNIX_SOCKET_PATH_HIDDEN) { - /* search the mappings */ + /* search the mappings, no need to construct the path */ LY_ARRAY_FOR(server_opts.unix_paths, i) { if (!strcmp(server_opts.unix_paths[i].endpt_name, endpt->name)) { - filename = server_opts.unix_paths[i].path; + p = server_opts.unix_paths[i].path; break; } } - if (!filename) { + if (!p) { ERR(NULL, "UNIX socket path mapping for endpoint \"%s\" not found.", endpt->name); } + + path = strdup(p); + NC_CHECK_ERRMEM_RET(!path, NULL); } else { ERRINT; } - /* construct the full socket path */ - if (nc_session_unix_construct_socket_path(filename, &path)) { - return NULL; - } - return path; } diff --git a/src/session_server.h b/src/session_server.h index b81f8509..18f925f2 100644 --- a/src/session_server.h +++ b/src/session_server.h @@ -471,8 +471,9 @@ int nc_server_get_unix_socket_path(const char *endpoint_name, char **socket_path /** * @brief Set the base directory for UNIX socket paths. * - * All UNIX socket paths will be relative to this directory. + * All YANG defined UNIX socket paths will be relative to this directory. * It must exist and be writable by the server process. + * This does not apply to "hidden" UNIX socket paths set via ::nc_server_set_unix_socket_path(). * * @param[in] dir Base directory for UNIX socket paths. * @return 0 on success, 1 on error. diff --git a/tests/test_unix_socket.c b/tests/test_unix_socket.c index d26f52af..9cf47a30 100644 --- a/tests/test_unix_socket.c +++ b/tests/test_unix_socket.c @@ -59,9 +59,9 @@ setup_glob_f(void **state) assert_int_equal(ret, 0); /* set two hidden paths for UNIX sockets */ - ret = nc_server_set_unix_socket_path("unix", "nc2_test_unix_sock"); + ret = nc_server_set_unix_socket_path("unix", "/tmp/nc2_test_unix_sock"); assert_int_equal(ret, 0); - ret = nc_server_set_unix_socket_path("unix2", "nc2_test_unix_sock2"); + ret = nc_server_set_unix_socket_path("unix2", "/tmp/nc2_test_unix_sock2"); assert_int_equal(ret, 0); return 0;