From 48d39f7c30c8aae867abe563a3f748d715b9b41b Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 19 Jun 2024 17:16:34 +1000 Subject: [PATCH 1/7] Check that FILE_STREAM(channel) is not already closed isc_log_closefilelogs can also close log files. isc_log_doit failed to check if the file handle was still valid before closing it. --- lib/isc/log.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/isc/log.c b/lib/isc/log.c index 472c57d956..53785df3c6 100644 --- a/lib/isc/log.c +++ b/lib/isc/log.c @@ -1762,8 +1762,11 @@ isc_log_doit(isc_log_t *lctx, isc_logcategory_t *category, errno == ENOENT) || statbuf.st_size < FILE_MAXSIZE(channel)) { - (void)fclose(FILE_STREAM(channel)); - FILE_STREAM(channel) = NULL; + if (FILE_STREAM(channel) != NULL) { + (void)fclose( + FILE_STREAM(channel)); + FILE_STREAM(channel) = NULL; + } FILE_MAXREACHED(channel) = false; } else { /* From 78aa4466fe0f04da810565a235aa0caf0c8069ed Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 19 Jun 2024 09:49:55 +1000 Subject: [PATCH 2/7] Sort NAMED_COMMAND_* defines --- bin/named/include/named/control.h | 68 +++++++++++++++---------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/bin/named/include/named/control.h b/bin/named/include/named/control.h index ee938a5973..c01cb7b00e 100644 --- a/bin/named/include/named/control.h +++ b/bin/named/include/named/control.h @@ -28,48 +28,48 @@ #define NAMED_CONTROL_PORT 953 -#define NAMED_COMMAND_STOP "stop" -#define NAMED_COMMAND_HALT "halt" -#define NAMED_COMMAND_RELOAD "reload" -#define NAMED_COMMAND_RECONFIG "reconfig" -#define NAMED_COMMAND_REFRESH "refresh" -#define NAMED_COMMAND_RETRANSFER "retransfer" -#define NAMED_COMMAND_DUMPSTATS "stats" -#define NAMED_COMMAND_QUERYLOG "querylog" +#define NAMED_COMMAND_ADDZONE "addzone" +#define NAMED_COMMAND_DELZONE "delzone" +#define NAMED_COMMAND_DNSSEC "dnssec" +#define NAMED_COMMAND_DNSTAP "dnstap" +#define NAMED_COMMAND_DNSTAPREOPEN "dnstap-reopen" #define NAMED_COMMAND_DUMPDB "dumpdb" -#define NAMED_COMMAND_SECROOTS "secroots" -#define NAMED_COMMAND_TRACE "trace" -#define NAMED_COMMAND_NOTRACE "notrace" +#define NAMED_COMMAND_DUMPSTATS "stats" +#define NAMED_COMMAND_FETCHLIMIT "fetchlimit" #define NAMED_COMMAND_FLUSH "flush" #define NAMED_COMMAND_FLUSHNAME "flushname" #define NAMED_COMMAND_FLUSHTREE "flushtree" -#define NAMED_COMMAND_STATUS "status" #define NAMED_COMMAND_FREEZE "freeze" -#define NAMED_COMMAND_UNFREEZE "unfreeze" -#define NAMED_COMMAND_THAW "thaw" -#define NAMED_COMMAND_RECURSING "recursing" -#define NAMED_COMMAND_NULL "null" -#define NAMED_COMMAND_NOTIFY "notify" -#define NAMED_COMMAND_VALIDATION "validation" -#define NAMED_COMMAND_SCAN "scan" -#define NAMED_COMMAND_SIGN "sign" +#define NAMED_COMMAND_HALT "halt" #define NAMED_COMMAND_LOADKEYS "loadkeys" -#define NAMED_COMMAND_ADDZONE "addzone" -#define NAMED_COMMAND_MODZONE "modzone" -#define NAMED_COMMAND_DELZONE "delzone" -#define NAMED_COMMAND_SHOWZONE "showzone" -#define NAMED_COMMAND_SYNC "sync" -#define NAMED_COMMAND_SIGNING "signing" -#define NAMED_COMMAND_DNSSEC "dnssec" -#define NAMED_COMMAND_ZONESTATUS "zonestatus" -#define NAMED_COMMAND_NTA "nta" -#define NAMED_COMMAND_TESTGEN "testgen" #define NAMED_COMMAND_MKEYS "managed-keys" -#define NAMED_COMMAND_DNSTAPREOPEN "dnstap-reopen" -#define NAMED_COMMAND_DNSTAP "dnstap" -#define NAMED_COMMAND_TCPTIMEOUTS "tcp-timeouts" +#define NAMED_COMMAND_MODZONE "modzone" +#define NAMED_COMMAND_NOTIFY "notify" +#define NAMED_COMMAND_NOTRACE "notrace" +#define NAMED_COMMAND_NTA "nta" +#define NAMED_COMMAND_NULL "null" +#define NAMED_COMMAND_QUERYLOG "querylog" +#define NAMED_COMMAND_RECONFIG "reconfig" +#define NAMED_COMMAND_RECURSING "recursing" +#define NAMED_COMMAND_REFRESH "refresh" +#define NAMED_COMMAND_RELOAD "reload" +#define NAMED_COMMAND_RETRANSFER "retransfer" +#define NAMED_COMMAND_SCAN "scan" +#define NAMED_COMMAND_SECROOTS "secroots" #define NAMED_COMMAND_SERVESTALE "serve-stale" -#define NAMED_COMMAND_FETCHLIMIT "fetchlimit" +#define NAMED_COMMAND_SHOWZONE "showzone" +#define NAMED_COMMAND_SIGN "sign" +#define NAMED_COMMAND_SIGNING "signing" +#define NAMED_COMMAND_STATUS "status" +#define NAMED_COMMAND_STOP "stop" +#define NAMED_COMMAND_SYNC "sync" +#define NAMED_COMMAND_TCPTIMEOUTS "tcp-timeouts" +#define NAMED_COMMAND_TESTGEN "testgen" +#define NAMED_COMMAND_THAW "thaw" +#define NAMED_COMMAND_TRACE "trace" +#define NAMED_COMMAND_UNFREEZE "unfreeze" +#define NAMED_COMMAND_VALIDATION "validation" +#define NAMED_COMMAND_ZONESTATUS "zonestatus" isc_result_t named_controls_create(named_server_t *server, named_controls_t **ctrlsp); From 56c1f4aef2dce44737c32d53ef2995afaffbab5e Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 19 Jun 2024 10:20:33 +1000 Subject: [PATCH 3/7] Add a rndc command to close currently open log files The new command is 'rndc closelogs'. --- bin/named/control.c | 4 ++++ bin/named/include/named/control.h | 1 + bin/rndc/rndc.rst | 9 +++++++++ 3 files changed, 14 insertions(+) diff --git a/bin/named/control.c b/bin/named/control.c index a3e009799e..d2aa5a9260 100644 --- a/bin/named/control.c +++ b/bin/named/control.c @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -201,6 +202,9 @@ named_control_docommand(isccc_sexpr_t *message, bool readonly, command_compare(command, NAMED_COMMAND_MODZONE)) { result = named_server_changezone(named_g_server, cmdline, text); + } else if (command_compare(command, NAMED_COMMAND_CLOSELOGS)) { + isc_log_closefilelogs(named_g_lctx); + result = ISC_R_SUCCESS; } else if (command_compare(command, NAMED_COMMAND_DELZONE)) { result = named_server_delzone(named_g_server, lex, text); } else if (command_compare(command, NAMED_COMMAND_DNSSEC)) { diff --git a/bin/named/include/named/control.h b/bin/named/include/named/control.h index c01cb7b00e..6f3660ad0f 100644 --- a/bin/named/include/named/control.h +++ b/bin/named/include/named/control.h @@ -29,6 +29,7 @@ #define NAMED_CONTROL_PORT 953 #define NAMED_COMMAND_ADDZONE "addzone" +#define NAMED_COMMAND_CLOSELOGS "closelogs" #define NAMED_COMMAND_DELZONE "delzone" #define NAMED_COMMAND_DNSSEC "dnssec" #define NAMED_COMMAND_DNSTAP "dnstap" diff --git a/bin/rndc/rndc.rst b/bin/rndc/rndc.rst index da9f0710e4..62603d1da8 100644 --- a/bin/rndc/rndc.rst +++ b/bin/rndc/rndc.rst @@ -152,6 +152,15 @@ Currently supported commands are: See also :option:`rndc delzone` and :option:`rndc modzone`. +.. option:: closelogs + + This command closes currently open log files. It is intended to be used + by external log rotation tools following this proceedure. + + 1) rename the log files + 2) run ``rndc closelogs`` + 3) optionally compress the log files + .. option:: delzone [-clean] zone [class [view]] This command deletes a zone while the server is running. From 5d26b8b71ea65399c66ab6d127b6b61e5f614a72 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 19 Jun 2024 10:43:49 +1000 Subject: [PATCH 4/7] Test that 'rndc closelogs' works --- .../system/logfileconfig/ns1/named.unlimited.in | 2 +- bin/tests/system/logfileconfig/tests.sh | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/logfileconfig/ns1/named.unlimited.in b/bin/tests/system/logfileconfig/ns1/named.unlimited.in index f806eddd62..8cfd205e5b 100644 --- a/bin/tests/system/logfileconfig/ns1/named.unlimited.in +++ b/bin/tests/system/logfileconfig/ns1/named.unlimited.in @@ -37,7 +37,7 @@ logging { channel query_log { file "query_log"; print-time yes; - buffered yes; + buffered no; }; category queries { query_log; }; }; diff --git a/bin/tests/system/logfileconfig/tests.sh b/bin/tests/system/logfileconfig/tests.sh index e738dbd66f..bc9eee962c 100644 --- a/bin/tests/system/logfileconfig/tests.sh +++ b/bin/tests/system/logfileconfig/tests.sh @@ -28,6 +28,10 @@ test_with_retry() { return 1 } +rndccmd() ( + "$RNDC" -c ../_common/rndc.conf -p "${CONTROLPORT}" -s "$@" +) + status=0 n=0 @@ -276,6 +280,19 @@ test_with_retry -f ns1/named_unlimited.4 || ret=1 if [ "$ret" -ne 0 ]; then echo_i "failed"; fi status=$((status + ret)) +n=$((n + 1)) +echo_i "testing 'rndc closelogs' ($n)" +ret=0 +test -f ns1/query_log || ret=1 +mv ns1/query_log ns1/query_log.1 || ret=1 +rndccmd 10.53.0.1 closelogs >rndc.out.test$n || ret=1 +$DIG version.bind txt ch @10.53.0.1 -p ${PORT} >dig.out.test$n || ret=1 +test -f ns1/query_log || ret=1 +lines=$(wc -l <"ns1/query_log") +test ${lines:-0} -eq 1 || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + n=$((n + 1)) echo_i "testing default logfile using named -L file ($n)" ret=0 From 7a191400f9c99ee94bd041b5387f350379d6b3e7 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 26 Jun 2024 10:47:47 +1000 Subject: [PATCH 5/7] Configure SIGUSR1 to close log files Some external log file rotation programs use signals to tell programs to close log files. SIGHUP can be used to do this but it also does a full reconfiguration. Configure named to accept SIGUSR1 as a signal to close log files. --- bin/named/include/named/server.h | 1 + bin/named/main.c | 1 + bin/named/server.c | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/bin/named/include/named/server.h b/bin/named/include/named/server.h index 52a13d5658..9ad18bde7f 100644 --- a/bin/named/include/named/server.h +++ b/bin/named/include/named/server.h @@ -108,6 +108,7 @@ struct named_server { isc_tlsctx_cache_t *tlsctx_client_cache; isc_signal_t *sighup; + isc_signal_t *sigusr1; }; #define NAMED_SERVER_MAGIC ISC_MAGIC('S', 'V', 'E', 'R') diff --git a/bin/named/main.c b/bin/named/main.c index 6cb3392005..e5a17b2f45 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -1567,6 +1567,7 @@ main(int argc, char *argv[]) { * Start things running */ isc_signal_start(named_g_server->sighup); + isc_signal_start(named_g_server->sigusr1); /* * Pause the loop manager in fatal. diff --git a/bin/named/server.c b/bin/named/server.c index fd12287d33..3c5ba6d5e9 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -432,6 +432,9 @@ fatal(const char *msg, isc_result_t result); static void named_server_reload(void *arg); +static void +named_server_closelogswanted(void *arg, int signum); + #ifdef HAVE_LIBNGHTTP2 static isc_result_t listenelt_http(const cfg_obj_t *http, const uint16_t family, bool tls, @@ -9983,6 +9986,9 @@ shutdown_server(void *arg) { isc_signal_stop(server->sighup); isc_signal_destroy(&server->sighup); + isc_signal_stop(server->sigusr1); + isc_signal_destroy(&server->sigusr1); + /* * We need to shutdown the interface before going * exclusive (which would pause the netmgr). @@ -10355,6 +10361,10 @@ named_server_create(isc_mem_t *mctx, named_server_t **serverp) { server->sighup = isc_signal_new( named_g_loopmgr, named_server_reloadwanted, server, SIGHUP); + /* Add SIGUSR2 closelogs handler */ + server->sigusr1 = isc_signal_new( + named_g_loopmgr, named_server_closelogswanted, server, SIGUSR1); + isc_stats_create(server->mctx, &server->sockstats, isc_sockstatscounter_max); isc_nm_setstats(named_g_netmgr, server->sockstats); @@ -10540,6 +10550,28 @@ named_server_reloadwanted(void *arg, int signum) { isc_async_run(named_g_mainloop, named_server_reload, server); } +/* + * Handle a reload event (from SIGUSR1). + */ +static void +named_server_closelogs(void *arg) { + UNUSED(arg); + + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, + "received SIGUSR1 signal to close log files"); + isc_log_closefilelogs(named_g_lctx); +} + +static void +named_server_closelogswanted(void *arg, int signum) { + named_server_t *server = (named_server_t *)arg; + + REQUIRE(signum == SIGUSR1); + + isc_async_run(named_g_mainloop, named_server_closelogs, server); +} + void named_server_scan_interfaces(named_server_t *server) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, From 88dfe502897a693e61eeeaaf6a2866e45da9c0dc Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 26 Jun 2024 10:55:24 +1000 Subject: [PATCH 6/7] Document that SIGUSR1 closes log files --- doc/arm/dns-ops.inc.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/arm/dns-ops.inc.rst b/doc/arm/dns-ops.inc.rst index abb6bcf766..d5a8b7aa86 100644 --- a/doc/arm/dns-ops.inc.rst +++ b/doc/arm/dns-ops.inc.rst @@ -236,6 +236,8 @@ described in the following table. These signals can be sent using the | ``SIGHUP`` | Causes the server to read :iscman:`named.conf` and reload | | | the database. | +--------------+-------------------------------------------------------------+ +| ``SIGUSR1`` | Causes the server close the currently open log files | ++--------------+-------------------------------------------------------------+ | ``SIGTERM`` | Causes the server to clean up and exit. | +--------------+-------------------------------------------------------------+ | ``SIGINT`` | Causes the server to clean up and exit. | From 2e55a2ac696e5b1d56b346c29b0b57d285dbf5b1 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 26 Jun 2024 10:53:14 +1000 Subject: [PATCH 7/7] Test that 'kill -USR1' works --- bin/tests/system/logfileconfig/tests.sh | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/logfileconfig/tests.sh b/bin/tests/system/logfileconfig/tests.sh index bc9eee962c..1b411174cd 100644 --- a/bin/tests/system/logfileconfig/tests.sh +++ b/bin/tests/system/logfileconfig/tests.sh @@ -284,7 +284,7 @@ n=$((n + 1)) echo_i "testing 'rndc closelogs' ($n)" ret=0 test -f ns1/query_log || ret=1 -mv ns1/query_log ns1/query_log.1 || ret=1 +mv ns1/query_log ns1/query_log.$n || ret=1 rndccmd 10.53.0.1 closelogs >rndc.out.test$n || ret=1 $DIG version.bind txt ch @10.53.0.1 -p ${PORT} >dig.out.test$n || ret=1 test -f ns1/query_log || ret=1 @@ -293,6 +293,19 @@ test ${lines:-0} -eq 1 || ret=1 if [ "$ret" -ne 0 ]; then echo_i "failed"; fi status=$((status + ret)) +n=$((n + 1)) +echo_i "testing 'kill -USR1' ($n)" +ret=0 +test -f ns1/query_log || ret=1 +mv ns1/query_log ns1/query_log.$n || ret=1 +kill -USR1 $(cat ns1/named.pid) || ret=1 +$DIG version.bind txt ch @10.53.0.1 -p ${PORT} >dig.out.test$n || ret=1 +test -f ns1/query_log || ret=1 +lines=$(wc -l <"ns1/query_log") +test ${lines:-0} -eq 1 || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + n=$((n + 1)) echo_i "testing default logfile using named -L file ($n)" ret=0