From 789762ff3d88a1c006babc7a8e7037e0976ad70f Mon Sep 17 00:00:00 2001 From: Zhidao HONG Date: Mon, 30 Jan 2023 11:16:01 +0800 Subject: NJS: adding the missing vm destruction. This commit fixed the njs memory leak happened in the config validation, updating and http requests. --- src/nxt_router.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/nxt_router.c') diff --git a/src/nxt_router.c b/src/nxt_router.c index edc015c5..17f6c572 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -1111,6 +1111,10 @@ temp_fail: fail: + if (rtcf->tstr_state != NULL) { + nxt_tstr_state_release(rtcf->tstr_state); + } + nxt_mp_destroy(mp); return NULL; @@ -3794,6 +3798,8 @@ nxt_router_conf_release(nxt_task_t *task, nxt_socket_conf_joint_t *joint) nxt_router_access_log_release(task, lock, rtcf->access_log); + nxt_tstr_state_release(rtcf->tstr_state); + nxt_mp_thread_adopt(rtcf->mem_pool); nxt_mp_destroy(rtcf->mem_pool); -- cgit From 78e1122a3c94a150219b4b6e1e594ae5bfdd8d68 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Sat, 4 Mar 2023 23:52:51 +0000 Subject: Router: Fix allocation of request buffer sent to application. This fixes an issue reported by @Peter2121 on GitHub. In nxt_router_prepare_msg() we create a buffer (nxt_unit_request_t *req) that gets sent to an application process that contains details about a client request. The req structure comprises various members with the final member being an array (specified as a flexible array member, with its actual length denoted by the req->fields_count member) of nxt_unit_field_t's. These structures specify the length and offset for the various request headers name/value pairs which are stored after some request metadata that is stored immediately after this array of structs as individual nul terminated strings. After this we have the body content data (if any). So it looks a little like (gdb) x /64bs 0x7f38c976e060 0x7f38c976e060: "\353\346\244\t\006" <-- First nxt_unit_field_t 0x7f38c976e066: "" 0x7f38c976e067: "" 0x7f38c976e068: "T\001" 0x7f38c976e06b: "" 0x7f38c976e06c: "Z\001" 0x7f38c976e06f: "" ... 0x7f38c976e170: "\362#\244\v$" <-- Last nxt_unit_field_t 0x7f38c976e176: "" 0x7f38c976e177: "" 0x7f38c976e178: "\342\002" 0x7f38c976e17b: "" 0x7f38c976e17c: "\352\002" 0x7f38c976e17f: "" 0x7f38c976e180: "POST" <-- Start of request metadata 0x7f38c976e185: "HTTP/1.1" 0x7f38c976e18e: "unix:" 0x7f38c976e194: "unix:/dev/shm/842.sock" 0x7f38c976e1ab: "" 0x7f38c976e1ac: "fedora" 0x7f38c976e1b3: "/842.php" 0x7f38c976e1bc: "HTTP_HOST" <-- Start of header fields 0x7f38c976e1c6: "fedora" 0x7f38c976e1cd: "HTTP_X_FORWARDED_PROTO" 0x7f38c976e1e4: "https" ... 0x7f38c976e45a: "HTTP_COOKIE" 0x7f38c976e466: "PHPSESSID=8apkg25r9s9vju3pi085i21eh4" 0x7f38c976e48b: "public_form=sended" <-- Body content Well that's how things are supposed to look! When using Unix domain sockets what we actually got looked like ... 0x7f6141f3445a: "HTTP_COOKIE" 0x7f6141f34466: "PHPSESSID=uo5b2nu9buijkc89jotbgmd60vpublic_form=sended" Here, the body content (from a POST for example) has been appended straight onto the end of the last header field value. In this case corrupting the PHP session cookie. The body content would still be found by the application as its offset into this buffer is correct. This problem was actually caused by a0327445 ("PHP: allowed to specify URLs without a trailing '/'.") which added an extra item into this request buffer specifying the port number that unit is listening on that handled this request. Unfortunately when I wrote that patch I didn't increase the size of this request buffer to accommodate it. When using normal TCP sockets we actually end up allocating more space than required for this buffer, we track the end of this buffer up to where the body content would go and so we have a few spare bytes between the nul byte of the last field header value and the start of the body content. When using Unix domain sockets, they have no associated port number and thus the port number has a length of 0 bytes, but we still write a '\0' in there using up a byte that we didn't account for, this causes us to loose the nul byte of the last header fields value causing the body data to be appended to the last header field value. The fix is simple, account for the local port length, we also add 1 to it, this covers the nul byte, even if there is no port as with Unix domain sockets. Closes: Fixes: a0327445 ("PHP: allowed to specify URLs without a trailing '/'.") Reviewed-by: Alejandro Colomar Signed-off-by: Andrew Clayton --- src/nxt_router.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src/nxt_router.c') diff --git a/src/nxt_router.c b/src/nxt_router.c index 17f6c572..4637cc68 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -5208,6 +5208,7 @@ nxt_router_prepare_msg(nxt_task_t *task, nxt_http_request_t *r, + r->version.length + 1 + r->remote->length + 1 + r->local->length + 1 + + nxt_sockaddr_port_length(r->local) + 1 + r->server_name.length + 1 + r->target.length + 1 + (r->path->start != r->target.start ? r->path->length + 1 : 0); -- cgit From 172ceba5b60d02cfc3acfb0feb88f415e7837092 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Mon, 13 Mar 2023 23:43:12 +0000 Subject: Router: More accurately allocate request buffer memory. In nxt_router_prepare_msg() we create a buffer (nxt_unit_request_t *req) that gets sent to an application process that contains details about a client request. This buffer was always a little larger than needed due to allocating space for the remote address _and_ port and the local address _and_ port. We also allocate space for the local port separately. ->{local,remote}->length includes the port number and ':' and also the '[]' for IPv6. E.g [2001:db8::1]:8080 ->{local,remote}->address_length represents the length of the unadorned IP address. E.g 2001:db8::1 Update the buffer size so that we only allocate what is actually needed. Suggested-by: Zhidao HONG Cc: Zhidao HONG Reviewed-by: Zhidao HONG Signed-off-by: Andrew Clayton --- src/nxt_router.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/nxt_router.c') diff --git a/src/nxt_router.c b/src/nxt_router.c index 4637cc68..17fe0418 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -5206,8 +5206,8 @@ nxt_router_prepare_msg(nxt_task_t *task, nxt_http_request_t *r, req_size = sizeof(nxt_unit_request_t) + r->method->length + 1 + r->version.length + 1 - + r->remote->length + 1 - + r->local->length + 1 + + r->remote->address_length + 1 + + r->local->address_length + 1 + nxt_sockaddr_port_length(r->local) + 1 + r->server_name.length + 1 + r->target.length + 1 -- cgit From 2e3e1c7e7bd5ee177b2703fa3d261fe51164426f Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Tue, 28 Feb 2023 01:59:04 +0000 Subject: Socket: Remove Unix domain listen sockets upon reconfigure. Currently when using Unix domain sockets for requests, if unit is reconfigured then it will fail if it tries to bind(2) again to a Unix domain socket with something like 2023/02/25 19:15:50 [alert] 35274#35274 bind(\"unix:/tmp/unit.sock\") failed (98: Address already in use) When closing such a socket we really need to unlink(2) it. However that presents a problem in that when running as root, while the main process runs as root and creates the socket, it's the router process, that runs as an unprivileged user, e.g nobody, that closes the socket and would thus remove it, but couldn't due to not having permission, even if the socket is mode 0666, you need write permissions on the containing directory to remove a file. There are several options to solve this, all with varying degrees of complexity and utility. 1) Give the user who the router process runs as write permission to the directory containing the listen sockets. These can then be unlink(2)'d from the router process. Simple and would work, but perhaps not the most elegant. 2) Using capabilities(7). The router process could temporarily attain the CAP_DAC_OVERRIDE capability, unlink(7) the socket, then relinquish the capability until required again. These are Linux specific (other systems may have similar mechanisms which would be extra work to support). There is also a, albeit small, window where the router process is running with elevated privileges. 3) Have the main process do the unlink(2), it is after all the process that created the socket. This is what this commit implements. We create a new port IPC message type of NXT_PORT_MSG_SOCKET_UNLINK, that is used by the router process to notify the main process about a Unix domain socket to unlink(2). Upon doing a reconfigure the router process will call nxt_router_listen_socket_release() which will close the socket, we extend this function in the case of non-abstract Unix domain sockets, so that it will send a message to the main process containing a copy of the nxt_sockaddr_t structure that will contain the filename of the socket. In the main process the handler that we have defined, nxt_main_port_socket_unlink_handler(), for this message type will run and allow us to look for the socket in question in the listen_sockets array and remove it and unlink(2) the socket. This then allows the reconfigure to work if it tries to bind(2) again to a socket that previously existed. Link: Link: Reviewed-by: Alejandro Colomar Signed-off-by: Andrew Clayton --- src/nxt_router.c | 41 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) (limited to 'src/nxt_router.c') diff --git a/src/nxt_router.c b/src/nxt_router.c index 17fe0418..baede83b 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -3686,6 +3686,13 @@ nxt_router_listen_socket_close(nxt_task_t *task, void *obj, void *data) static void nxt_router_listen_socket_release(nxt_task_t *task, nxt_socket_conf_t *skcf) { +#if (NXT_HAVE_UNIX_DOMAIN) + size_t size; + nxt_buf_t *b; + nxt_port_t *main_port; + nxt_runtime_t *rt; + nxt_sockaddr_t *sa; +#endif nxt_listen_socket_t *ls; nxt_thread_spinlock_t *lock; @@ -3703,10 +3710,38 @@ nxt_router_listen_socket_release(nxt_task_t *task, nxt_socket_conf_t *skcf) nxt_thread_spin_unlock(lock); - if (ls != NULL) { - nxt_socket_close(task, ls->socket); - nxt_free(ls); + if (ls == NULL) { + return; + } + + nxt_socket_close(task, ls->socket); + +#if (NXT_HAVE_UNIX_DOMAIN) + sa = ls->sockaddr; + if (sa->u.sockaddr.sa_family != AF_UNIX + || sa->u.sockaddr_un.sun_path[0] == '\0') + { + goto out_free_ls; } + + size = nxt_sockaddr_size(ls->sockaddr); + + b = nxt_buf_mem_alloc(task->thread->engine->mem_pool, size, 0); + if (b == NULL) { + goto out_free_ls; + } + + b->mem.free = nxt_cpymem(b->mem.free, ls->sockaddr, size); + + rt = task->thread->runtime; + main_port = rt->port_by_type[NXT_PROCESS_MAIN]; + + (void) nxt_port_socket_write(task, main_port, NXT_PORT_MSG_SOCKET_UNLINK, + -1, 0, 0, b); + +out_free_ls: +#endif + nxt_free(ls); } -- cgit From 0ebce31c9287cb97b626d61b62e83681b2864fe8 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Thu, 26 Jan 2023 15:07:12 +0100 Subject: HTTP: added route logging. - Configuration: added "/config/settings/http/log_route". Type: bool Default: false This adds configurability to the error log. It allows enabling and disabling logs related to how the router performs selection of the routes. - HTTP: logging request line. Log level: [notice] The request line is essential to understand which logs correspond to which request when reading the logs. - HTTP: logging route that's been discarded. Log level: [info] - HTTP: logging route whose action is selected. Log level: [notice] - HTTP: logging when "fallback" action is taken. Log level: [notice] Closes: Link: Link: Suggested-by: Timo Stark Suggested-by: Mark L Wood-Patrick Suggested-by: Liam Crilly Tested-by: Liam Crilly Acked-by: Artem Konev Cc: Andrew Clayton Cc: Andrei Zeliankou Reviewed-by: Zhidao Hong Signed-off-by: Alejandro Colomar --- src/nxt_router.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/nxt_router.c') diff --git a/src/nxt_router.c b/src/nxt_router.c index baede83b..992cc039 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -1513,6 +1513,12 @@ static nxt_conf_map_t nxt_router_http_conf[] = { NXT_CONF_MAP_INT8, offsetof(nxt_socket_conf_t, discard_unsafe_fields), }, + + { + nxt_string("log_route"), + NXT_CONF_MAP_INT8, + offsetof(nxt_socket_conf_t, log_route), + }, }; -- cgit From 1a485fed6a8353ecc09e6c0f050e44c0a2d30419 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Sat, 18 Mar 2023 16:32:59 +0000 Subject: Allow to remove the version string in HTTP responses. Normally Unit responds to HTTP requests by including a header like Server: Unit/1.30.0 however it can sometimes be beneficial to withhold the version information and in this case just respond with Server: Unit This patch adds a new "settings.http" boolean option called server_version, which defaults to true, in which case the full version information is sent. However this can be set to false, e.g "settings": { "http": { "server_version": false } }, in which case Unit responds without the version information as the latter example above shows. Link: Closes: Reviewed-by: Alejandro Colomar Signed-off-by: Andrew Clayton --- src/nxt_router.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src/nxt_router.c') diff --git a/src/nxt_router.c b/src/nxt_router.c index 992cc039..c4e29e3a 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -1519,6 +1519,12 @@ static nxt_conf_map_t nxt_router_http_conf[] = { NXT_CONF_MAP_INT8, offsetof(nxt_socket_conf_t, log_route), }, + + { + nxt_string("server_version"), + NXT_CONF_MAP_INT8, + offsetof(nxt_socket_conf_t, server_version), + }, }; @@ -1927,6 +1933,8 @@ nxt_router_conf_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, skcf->proxy_send_timeout = 30 * 1000; skcf->proxy_read_timeout = 30 * 1000; + skcf->server_version = 1; + skcf->websocket_conf.max_frame_size = 1024 * 1024; skcf->websocket_conf.read_timeout = 60 * 1000; skcf->websocket_conf.keepalive_interval = 30 * 1000; -- cgit From a3c3a29493798873ad04922bb2a7180b2ce267d5 Mon Sep 17 00:00:00 2001 From: Zhidao HONG Date: Mon, 8 May 2023 16:00:25 +0800 Subject: NJS: supported loadable modules. --- src/nxt_router.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 153 insertions(+), 5 deletions(-) (limited to 'src/nxt_router.c') diff --git a/src/nxt_router.c b/src/nxt_router.c index c4e29e3a..d089cfb8 100644 --- a/src/nxt_router.c +++ b/src/nxt_router.c @@ -11,6 +11,9 @@ #if (NXT_TLS) #include #endif +#if (NXT_HAVE_NJS) +#include +#endif #include #include #include @@ -55,6 +58,17 @@ typedef struct { #endif +#if (NXT_HAVE_NJS) + +typedef struct { + nxt_str_t name; + nxt_router_temp_conf_t *temp_conf; + nxt_queue_link_t link; +} nxt_router_js_module_t; + +#endif + + typedef struct { nxt_str_t *name; nxt_socket_conf_t *socket_conf; @@ -139,6 +153,12 @@ static nxt_int_t nxt_router_conf_tls_insert(nxt_router_temp_conf_t *tmcf, nxt_conf_value_t *value, nxt_socket_conf_t *skcf, nxt_tls_init_t *tls_init, nxt_bool_t last); #endif +#if (NXT_HAVE_NJS) +static void nxt_router_js_module_rpc_handler(nxt_task_t *task, + nxt_port_recv_msg_t *msg, void *data); +static nxt_int_t nxt_router_js_module_insert(nxt_router_temp_conf_t *tmcf, + nxt_conf_value_t *value); +#endif static void nxt_router_app_rpc_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, nxt_app_t *app); static void nxt_router_app_prefork_ready(nxt_task_t *task, @@ -1100,6 +1120,10 @@ nxt_router_temp_conf(nxt_task_t *task) nxt_queue_init(&tmcf->tls); #endif +#if (NXT_HAVE_NJS) + nxt_queue_init(&tmcf->js_modules); +#endif + nxt_queue_init(&tmcf->apps); nxt_queue_init(&tmcf->previous); @@ -1154,6 +1178,9 @@ nxt_router_conf_apply(nxt_task_t *task, void *obj, void *data) #if (NXT_TLS) nxt_router_tlssock_t *tls; #endif +#if (NXT_HAVE_NJS) + nxt_router_js_module_t *js_module; +#endif tmcf = obj; @@ -1184,6 +1211,27 @@ nxt_router_conf_apply(nxt_task_t *task, void *obj, void *data) } #endif +#if (NXT_HAVE_NJS) + qlk = nxt_queue_last(&tmcf->js_modules); + + if (qlk != nxt_queue_head(&tmcf->js_modules)) { + nxt_queue_remove(qlk); + + js_module = nxt_queue_link_data(qlk, nxt_router_js_module_t, link); + + nxt_script_store_get(task, &js_module->name, tmcf->mem_pool, + nxt_router_js_module_rpc_handler, js_module); + return; + } +#endif + + rtcf = tmcf->router_conf; + + ret = nxt_tstr_state_done(rtcf->tstr_state, NULL); + if (nxt_slow_path(ret != NXT_OK)) { + goto fail; + } + nxt_queue_each(app, &tmcf->apps, nxt_app_t, link) { if (nxt_router_app_need_start(app)) { @@ -1193,8 +1241,6 @@ nxt_router_conf_apply(nxt_task_t *task, void *obj, void *data) } nxt_queue_loop; - rtcf = tmcf->router_conf; - if (rtcf->access_log != NULL && rtcf->access_log->fd == -1) { nxt_router_access_log_open(task, tmcf); return; @@ -1569,6 +1615,9 @@ nxt_router_conf_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, #if (NXT_TLS) nxt_tls_init_t *tls_init; nxt_conf_value_t *certificate; +#endif +#if (NXT_HAVE_NJS) + nxt_conf_value_t *js_module; #endif nxt_conf_value_t *root, *conf, *http, *value, *websocket; nxt_conf_value_t *applications, *application; @@ -1592,6 +1641,9 @@ nxt_router_conf_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, static nxt_str_t conf_cache_path = nxt_string("/tls/session/cache_size"); static nxt_str_t conf_timeout_path = nxt_string("/tls/session/timeout"); static nxt_str_t conf_tickets = nxt_string("/tls/session/tickets"); +#endif +#if (NXT_HAVE_NJS) + static nxt_str_t js_module_path = nxt_string("/settings/js_module"); #endif static nxt_str_t static_path = nxt_string("/settings/http/static"); static nxt_str_t websocket_path = nxt_string("/settings/http/websocket"); @@ -2064,11 +2116,34 @@ nxt_router_conf_create(nxt_task_t *task, nxt_router_temp_conf_t *tmcf, } } - ret = nxt_tstr_state_done(rtcf->tstr_state, NULL); - if (nxt_slow_path(ret != NXT_OK)) { - goto fail; +#if (NXT_HAVE_NJS) + js_module = nxt_conf_get_path(root, &js_module_path); + + if (js_module != NULL) { + if (nxt_conf_type(js_module) == NXT_CONF_ARRAY) { + n = nxt_conf_array_elements_count(js_module); + + for (i = 0; i < n; i++) { + value = nxt_conf_get_array_element(js_module, i); + + ret = nxt_router_js_module_insert(tmcf, value); + if (nxt_slow_path(ret != NXT_OK)) { + goto fail; + } + } + + } else { + /* NXT_CONF_STRING */ + + ret = nxt_router_js_module_insert(tmcf, js_module); + if (nxt_slow_path(ret != NXT_OK)) { + goto fail; + } + } } +#endif + nxt_queue_add(&deleting_sockets, &router->sockets); nxt_queue_init(&router->sockets); @@ -2120,6 +2195,79 @@ nxt_router_conf_tls_insert(nxt_router_temp_conf_t *tmcf, #endif +#if (NXT_HAVE_NJS) + +static void +nxt_router_js_module_rpc_handler(nxt_task_t *task, nxt_port_recv_msg_t *msg, + void *data) +{ + nxt_int_t ret; + nxt_str_t text; + nxt_router_conf_t *rtcf; + nxt_router_temp_conf_t *tmcf; + nxt_router_js_module_t *js_module; + + nxt_debug(task, "auto module rpc handler"); + + js_module = data; + tmcf = js_module->temp_conf; + + if (msg == NULL || msg->port_msg.type == _NXT_PORT_MSG_RPC_ERROR) { + goto fail; + } + + rtcf = tmcf->router_conf; + + ret = nxt_script_file_read(msg->fd[0], &text); + + nxt_fd_close(msg->fd[0]); + + if (nxt_slow_path(ret == NXT_ERROR)) { + goto fail; + } + + if (text.length > 0) { + ret = nxt_js_add_module(rtcf->tstr_state->jcf, &js_module->name, &text); + + nxt_free(text.start); + + if (nxt_slow_path(ret == NXT_ERROR)) { + goto fail; + } + } + + nxt_work_queue_add(&task->thread->engine->fast_work_queue, + nxt_router_conf_apply, task, tmcf, NULL); + return; + +fail: + + nxt_router_conf_error(task, tmcf); +} + + +static nxt_int_t +nxt_router_js_module_insert(nxt_router_temp_conf_t *tmcf, + nxt_conf_value_t *value) +{ + nxt_router_js_module_t *js_module; + + js_module = nxt_mp_get(tmcf->mem_pool, sizeof(nxt_router_js_module_t)); + if (nxt_slow_path(js_module == NULL)) { + return NXT_ERROR; + } + + js_module->temp_conf = tmcf; + nxt_conf_get_string(value, &js_module->name); + + nxt_queue_insert_tail(&tmcf->js_modules, &js_module->link); + + return NXT_OK; +} + +#endif + + static nxt_int_t nxt_router_conf_process_static(nxt_task_t *task, nxt_router_conf_t *rtcf, nxt_conf_value_t *conf) -- cgit