diff --git a/CHANGES b/CHANGES index 9fff603ef8..7b46c9e2dd 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6161. [bug] Fix log file rotation when using absolute path as + file. [GL #3991] + 6157. [bug] When removing delegations in an OPTOUT range empty-non-terminal NSEC3 records generated by those delegations where not removed. [GL #4027] diff --git a/bin/tests/system/conf.sh.common b/bin/tests/system/conf.sh.common index 5474332ca5..1f3bb68d3e 100644 --- a/bin/tests/system/conf.sh.common +++ b/bin/tests/system/conf.sh.common @@ -709,9 +709,12 @@ get_named_xfer_stats() { # copy_setports infile outfile # copy_setports() { - sed -e "s/@PORT@/${PORT}/g" \ + dir=$(echo "$TMPDIR" | sed 's/\//\\\//g') + + sed -e "s/@TMPDIR@/${dir}/g" \ + -e "s/@PORT@/${PORT}/g" \ -e "s/@TLSPORT@/${TLSPORT}/g" \ - -e "s/@HTTPPORT@/${HTTPPORT}/g" \ + -e "s/@HTTPPORT@/${HTTPPORT}/g" \ -e "s/@HTTPSPORT@/${HTTPSPORT}/g" \ -e "s/@EXTRAPORT1@/${EXTRAPORT1}/g" \ -e "s/@EXTRAPORT2@/${EXTRAPORT2}/g" \ diff --git a/bin/tests/system/dnstap/tests.sh b/bin/tests/system/dnstap/tests.sh index 5ed1d94c3c..9e28c95689 100644 --- a/bin/tests/system/dnstap/tests.sh +++ b/bin/tests/system/dnstap/tests.sh @@ -787,28 +787,42 @@ lines=`$DNSTAPREAD -y large-answer.fstrm | grep -c "opcode: QUERY"` if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` -test_dnstap_roll() ( +_test_dnstap_roll() ( ip="$1" ns="$2" n="$3" + $RNDCCMD -s "${ip}" dnstap -roll "${n}" | sed "s/^/${ns} /" | cat_i && files=$(find "$ns" -name "dnstap.out.[0-9]" | wc -l) && - test "$files" -le "${n}" && test "$files" -ge "1" + test "$files" -eq "${n}" && test "$files" -ge "1" ) -echo_i "checking 'rndc -roll ' (no versions)" -ret=0 -start_server --noclean --restart --port "${PORT}" ns3 -_repeat 5 test_dnstap_roll 10.53.0.3 ns3 3 || ret=1 -if [ $ret != 0 ]; then echo_i "failed"; fi -status=$((status+ret)) -echo_i "checking 'rndc -roll ' (versions)" -ret=0 +test_dnstap_roll() { + echo_i "checking 'rndc -roll $4' ($1)" + ret=0 + + try=0 + while test $try -lt 12 + do + touch "$3/dnstap.out.$try" + try=`expr $try + 1` + done + + _repeat 10 _test_dnstap_roll $2 $3 $4 || ret=1 + if [ $ret != 0 ]; then echo_i "failed"; fi + status=$((status+ret)) +} + +start_server --noclean --restart --port "${PORT}" ns3 +test_dnstap_roll "no versions" 10.53.0.3 ns3 6 +test_dnstap_roll "no versions" 10.53.0.3 ns3 3 +test_dnstap_roll "no versions" 10.53.0.3 ns3 1 + start_server --noclean --restart --port "${PORT}" ns2 -_repeat 5 test_dnstap_roll 10.53.0.2 ns2 3 || ret=1 -if [ $ret != 0 ]; then echo_i "failed"; fi -status=$((status+ret)) +test_dnstap_roll "versions" 10.53.0.2 ns2 6 +test_dnstap_roll "versions" 10.53.0.2 ns2 3 +test_dnstap_roll "versions" 10.53.0.2 ns2 1 echo_i "exit status: $status" [ "$status" -eq 0 ] || exit 1 diff --git a/bin/tests/system/logfileconfig/clean.sh b/bin/tests/system/logfileconfig/clean.sh index 18aa5de2dd..befbcfe84e 100644 --- a/bin/tests/system/logfileconfig/clean.sh +++ b/bin/tests/system/logfileconfig/clean.sh @@ -31,6 +31,8 @@ rm -f ns1/named_vers rm -f ns1/named_vers.* rm -f ns1/named_ts rm -f ns1/named_ts.* +rm -f ns1/named_inc +rm -f ns1/named_inc.* rm -f ns1/named_unlimited rm -f ns1/named_unlimited.* rm -f ns*/managed-keys.bind* diff --git a/bin/tests/system/logfileconfig/ns1/named.abspathconf.in b/bin/tests/system/logfileconfig/ns1/named.abspathconf.in new file mode 100644 index 0000000000..fdf8a8ace2 --- /dev/null +++ b/bin/tests/system/logfileconfig/ns1/named.abspathconf.in @@ -0,0 +1,52 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +options { + query-source address 10.53.0.1; + notify-source 10.53.0.1; + transfer-source 10.53.0.1; + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.1; }; + listen-on-v6 { none; }; + dnssec-validation no; + recursion no; + notify yes; +}; + +logging { + channel default_log { + buffered no; + file "@TMPDIR@/example.log" versions 1 size 1k suffix increment; # small size + severity debug 100; + print-time yes; + }; + category default { default_log; default_debug; }; + category lame-servers { null; }; + + channel query_log { + file "query_log"; + print-time yes; + buffered yes; + }; + category queries { query_log; }; +}; + +controls { + inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; }; +}; + +key rndc-key { + secret "1234abcd8765"; + algorithm hmac-sha256; +}; diff --git a/bin/tests/system/logfileconfig/ns1/named.incconf.in b/bin/tests/system/logfileconfig/ns1/named.incconf.in new file mode 100644 index 0000000000..d398c330eb --- /dev/null +++ b/bin/tests/system/logfileconfig/ns1/named.incconf.in @@ -0,0 +1,52 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +options { + query-source address 10.53.0.1; + notify-source 10.53.0.1; + transfer-source 10.53.0.1; + port @PORT@; + pid-file "named.pid"; + listen-on { 10.53.0.1; }; + listen-on-v6 { none; }; + dnssec-validation no; + recursion no; + notify yes; +}; + +logging { + channel default_log { + buffered no; + file "named_inc" versions 1 size 1k suffix increment; # small size + severity debug 100; + print-time yes; + }; + category default { default_log; default_debug; }; + category lame-servers { null; }; + + channel query_log { + file "query_log"; + print-time yes; + buffered yes; + }; + category queries { query_log; }; +}; + +controls { + inet 10.53.0.1 port @CONTROLPORT@ allow { any; } keys { "rndc-key"; }; +}; + +key rndc-key { + secret "1234abcd8765"; + algorithm hmac-sha256; +}; diff --git a/bin/tests/system/logfileconfig/tests.sh b/bin/tests/system/logfileconfig/tests.sh index 397f9aa443..3abf17540a 100644 --- a/bin/tests/system/logfileconfig/tests.sh +++ b/bin/tests/system/logfileconfig/tests.sh @@ -208,6 +208,62 @@ retry_quiet 5 _found2 || ret=1 if [ "$ret" -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) +n=$((n+1)) +echo_i "testing incremented versions ($n)" +ret=0 +copy_setports ns1/named.incconf.in ns1/named.conf +try=0 +while test $try -lt 12 +do + touch ns1/named_inc.$try + try=`expr $try + 1` +done +rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n +_found2() ( + $DIG version.bind txt ch @10.53.0.1 -p ${PORT} > dig.out.test$n + grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 + + try=1 + while test $try -lt 12 + do + [ -f ns1/named_inc.$try ] && return 1 + try=`expr $try + 1` + done + set -- ns1/named_inc.* + [ "$#" -eq 1 ] || return 1 +) +retry_quiet 5 _found2 || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "testing absolute file path versions ($n)" +ret=0 +copy_setports ns1/named.abspathconf.in ns1/named.conf +try=0 +while test $try -lt 12 +do + touch $TMPDIR/example.log.$try + try=`expr $try + 1` +done +rndc_reconfig ns1 10.53.0.1 > rndc.out.test$n +_found2() ( + $DIG version.bind txt ch @10.53.0.1 -p ${PORT} > dig.out.test$n + grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 + + try=1 + while test $try -lt 12 + do + [ -f $TMPDIR/example.log.$try ] && return 1 + try=`expr $try + 1` + done + set -- $TMPDIR/example.log.* + [ "$#" -eq 1 ] || return 1 +) +retry_quiet 5 _found2 || ret=1 +if [ "$ret" -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + n=$((n+1)) echo_i "testing unlimited versions ($n)" ret=0 diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index bb0ff4bc37..c81ed1f9fe 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -45,6 +45,10 @@ Bug Fixes incoming zone transfers now time out properly when not progressing. :gl:`#4004` +- Log file rotation did not clean up older versions of log files when the + logging :any:`channel` configured an absolute path as ``file`` destination. + This has now been fixed. :gl:`#3991`. + Known Issues ~~~~~~~~~~~~ diff --git a/lib/isc/log.c b/lib/isc/log.c index 4ad9983769..523fd2d6bb 100644 --- a/lib/isc/log.c +++ b/lib/isc/log.c @@ -20,9 +20,11 @@ #include #include /* dev_t FreeBSD 2.1 */ #include +#include #include #include +#include #include #include #include @@ -1019,38 +1021,43 @@ sync_highest_level(isc_log_t *lctx, isc_logconfig_t *lcfg) { static isc_result_t greatest_version(isc_logfile_t *file, int versions, int *greatestp) { - char *bname, *digit_end; - const char *dirname; + char *digit_end; + char dirbuf[PATH_MAX + 1]; + const char *bname; + const char *dirname = "."; int version, greatest = -1; - size_t bnamelen; isc_dir_t dir; isc_result_t result; - char sep = '/'; + size_t bnamelen; - /* - * It is safe to DE_CONST the file.name because it was copied - * with isc_mem_strdup(). - */ - bname = strrchr(file->name, sep); + bname = strrchr(file->name, '/'); if (bname != NULL) { - *bname++ = '\0'; - dirname = file->name; + /* + * Copy the complete file name to dirbuf. + */ + size_t len = strlcpy(dirbuf, file->name, sizeof(dirbuf)); + if (len >= sizeof(dirbuf)) { + result = ISC_R_NOSPACE; + syslog(LOG_ERR, "unable to remove log files: %s", + isc_result_totext(result)); + return (result); + } + + /* + * Truncate after trailing '/' so the code works for + * files in the root directory. + */ + bname++; + dirbuf[bname - file->name] = '\0'; + dirname = dirbuf; } else { - DE_CONST(file->name, bname); - dirname = "."; + bname = file->name; } bnamelen = strlen(bname); isc_dir_init(&dir); result = isc_dir_open(&dir, dirname); - /* - * Replace the file separator if it was taken out. - */ - if (bname != file->name) { - *(bname - 1) = sep; - } - /* * Return if the directory open failed. */ @@ -1069,15 +1076,23 @@ greatest_version(isc_logfile_t *file, int versions, int *greatestp) { * Remove any backup files that exceed versions. */ if (*digit_end == '\0' && version >= versions) { - result = isc_file_remove(dir.entry.name); - if (result != ISC_R_SUCCESS && - result != ISC_R_FILENOTFOUND) - { - syslog(LOG_ERR, - "unable to remove " - "log file '%s': %s", - dir.entry.name, - isc_result_totext(result)); + int n = unlinkat(dirfd(dir.handle), + dir.entry.name, 0); + if (n < 0) { + result = isc_errno_toresult(errno); + if (result != ISC_R_SUCCESS && + result != ISC_R_FILENOTFOUND) + { + syslog(LOG_ERR, + "unable to remove log " + "file '%s%s': %s", + bname == file->name + ? "" + : dirname, + dir.entry.name, + isc_result_totext( + result)); + } } } else if (*digit_end == '\0' && version > greatest) { greatest = version; @@ -1087,7 +1102,6 @@ greatest_version(isc_logfile_t *file, int versions, int *greatestp) { isc_dir_close(&dir); *greatestp = greatest; - return (ISC_R_SUCCESS); } @@ -1108,7 +1122,8 @@ insert_sort(int64_t to_keep[], int64_t versions, int64_t version) { } static int64_t -last_to_keep(int64_t versions, isc_dir_t *dirp, char *bname, size_t bnamelen) { +last_to_keep(int64_t versions, isc_dir_t *dirp, const char *bname, + size_t bnamelen) { int64_t to_keep[ISC_LOG_MAX_VERSIONS] = { 0 }; int64_t version = 0; @@ -1151,38 +1166,43 @@ last_to_keep(int64_t versions, isc_dir_t *dirp, char *bname, size_t bnamelen) { static isc_result_t remove_old_tsversions(isc_logfile_t *file, int versions) { - isc_result_t result; - char *bname = NULL, *digit_end = NULL; - const char *dirname = NULL; + char *digit_end; + char dirbuf[PATH_MAX + 1]; + const char *bname; + const char *dirname = "."; int64_t version, last = INT64_MAX; - size_t bnamelen; isc_dir_t dir; - char sep = '/'; + isc_result_t result; + size_t bnamelen; - /* - * It is safe to DE_CONST the file.name because it was copied - * with isc_mem_strdup(). - */ - bname = strrchr(file->name, sep); + bname = strrchr(file->name, '/'); if (bname != NULL) { - *bname++ = '\0'; - dirname = file->name; + /* + * Copy the complete file name to dirbuf. + */ + size_t len = strlcpy(dirbuf, file->name, sizeof(dirbuf)); + if (len >= sizeof(dirbuf)) { + result = ISC_R_NOSPACE; + syslog(LOG_ERR, "unable to remove log files: %s", + isc_result_totext(result)); + return (result); + } + + /* + * Truncate after trailing '/' so the code works for + * files in the root directory. + */ + bname++; + dirbuf[bname - file->name] = '\0'; + dirname = dirbuf; } else { - DE_CONST(file->name, bname); - dirname = "."; + bname = file->name; } bnamelen = strlen(bname); isc_dir_init(&dir); result = isc_dir_open(&dir, dirname); - /* - * Replace the file separator if it was taken out. - */ - if (bname != file->name) { - *(bname - 1) = sep; - } - /* * Return if the directory open failed. */ @@ -1192,36 +1212,39 @@ remove_old_tsversions(isc_logfile_t *file, int versions) { last = last_to_keep(versions, &dir, bname, bnamelen); - /* - * Then we remove all files that we don't want to_keep - */ while (isc_dir_read(&dir) == ISC_R_SUCCESS) { if (dir.entry.length > bnamelen && strncmp(dir.entry.name, bname, bnamelen) == 0 && dir.entry.name[bnamelen] == '.') { - char *ename = &dir.entry.name[bnamelen + 1]; - version = strtoull(ename, &digit_end, 10); + version = strtoull(&dir.entry.name[bnamelen + 1], + &digit_end, 10); /* * Remove any backup files that exceed versions. */ if (*digit_end == '\0' && version < last) { - result = isc_file_remove(dir.entry.name); - if (result != ISC_R_SUCCESS && - result != ISC_R_FILENOTFOUND) - { - syslog(LOG_ERR, - "unable to remove " - "log file '%s': %s", - dir.entry.name, - isc_result_totext(result)); + int n = unlinkat(dirfd(dir.handle), + dir.entry.name, 0); + if (n < 0) { + result = isc_errno_toresult(errno); + if (result != ISC_R_SUCCESS && + result != ISC_R_FILENOTFOUND) + { + syslog(LOG_ERR, + "unable to remove log " + "file '%s%s': %s", + bname == file->name + ? "" + : dirname, + dir.entry.name, + isc_result_totext( + result)); + } } } } } - isc_dir_close(&dir); - return (ISC_R_SUCCESS); }