From owner-freebsd-bugs@FreeBSD.ORG Mon Dec 2 03:40:01 2013 Return-Path: Delivered-To: freebsd-bugs@smarthost.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2023CD71 for ; Mon, 2 Dec 2013 03:40:01 +0000 (UTC) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id EF1606D69 for ; Mon, 2 Dec 2013 03:40:00 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.7/8.14.7) with ESMTP id rB23e0at087188 for ; Mon, 2 Dec 2013 03:40:00 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.7/8.14.7/Submit) id rB23e0Lk087164; Mon, 2 Dec 2013 03:40:00 GMT (envelope-from gnats) Resent-Date: Mon, 2 Dec 2013 03:40:00 GMT Resent-Message-Id: <201312020340.rB23e0Lk087164@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Klaas Teschauer Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 43ACBAFE for ; Mon, 2 Dec 2013 03:31:50 +0000 (UTC) Received: from oldred.freebsd.org (oldred.freebsd.org [8.8.178.121]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 2FF106D38 for ; Mon, 2 Dec 2013 03:31:50 +0000 (UTC) Received: from oldred.freebsd.org ([127.0.1.6]) by oldred.freebsd.org (8.14.5/8.14.7) with ESMTP id rB23VnUe050868 for ; Mon, 2 Dec 2013 03:31:49 GMT (envelope-from nobody@oldred.freebsd.org) Received: (from nobody@localhost) by oldred.freebsd.org (8.14.5/8.14.5/Submit) id rB23VnTU050860; Mon, 2 Dec 2013 03:31:49 GMT (envelope-from nobody) Message-Id: <201312020331.rB23VnTU050860@oldred.freebsd.org> Date: Mon, 2 Dec 2013 03:31:49 GMT From: Klaas Teschauer To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Subject: bin/184424: newsyslog - run shell command feature does not work (R flag in config file) X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Dec 2013 03:40:01 -0000 >Number: 184424 >Category: bin >Synopsis: newsyslog - run shell command feature does not work (R flag in config file) >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Mon Dec 02 03:40:00 UTC 2013 >Closed-Date: >Last-Modified: >Originator: Klaas Teschauer >Release: 9.1-RELEASE >Organization: >Environment: FreeBSD hub 9.1-RELEASE FreeBSD 9.1-RELEASE #0 r243825: Tue Dec 4 09:23:10 UTC 2012 root@farrell.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC amd64 >Description: According to newsyslog.conf(5), newsyslog offers a feature to execute an external command to signal a daemon after a log has been rotated. Quoting man newsyslog.conf, section "flags": R if this flag is set the newsyslog(8) will run shell com- mand defined in path_to_pid_cmd_file after rotation instead of trying to send signal to a process id stored in the file. In FreeBSD 9.1, this feature does not work. >How-To-Repeat: Set up a newsyslog.conf file with an entry that uses the 'R' flag: [klaas@hub ~]$ cat newsyslog.netatalk.test # test config for newsyslog(8) # logfilename [owner:group] mode count size when flags [/pid_file] [sig_num] /var/log/netatalk.log root:wheel 644 6 50 @T00 R /usr/home/klaas/restart.netatalk Perform a test run to confirm that the script is not invoked (due to the bug): [klaas@hub ~]$ newsyslog -f newsyslog.netatalk.test -r -n -v -F Processing newsyslog.netatalk.test /var/log/netatalk.log <6>: size (Kb): 5 [50] --> trimming log.... rm -f /var/log/netatalk.log.6 rm -f /var/log/netatalk.log.6.gz rm -f /var/log/netatalk.log.6.bz2 rm -f /var/log/netatalk.log.6.xz rm -f /var/log/netatalk.log.5 rm -f /var/log/netatalk.log.5.gz rm -f /var/log/netatalk.log.5.bz2 rm -f /var/log/netatalk.log.5.xz mv /var/log/netatalk.log.2.gz /var/log/netatalk.log.3.gz chmod 644 /var/log/netatalk.log.3.gz chown 0:0 /var/log/netatalk.log.3.gz mv /var/log/netatalk.log.1.gz /var/log/netatalk.log.2.gz chmod 644 /var/log/netatalk.log.2.gz chown 0:0 /var/log/netatalk.log.2.gz mv /var/log/netatalk.log.0 /var/log/netatalk.log.1 chmod 644 /var/log/netatalk.log.1 chown 0:0 /var/log/netatalk.log.1 ln /var/log/netatalk.log /var/log/netatalk.log.0 chmod 644 /var/log/netatalk.log.0 chown 0:0 /var/log/netatalk.log.0 Start new log... mktemp /var/log/netatalk.log.zXXXXXX chown 0:0 /var/log/netatalk.log.zXXXXXX chmod 644 /var/log/netatalk.log.zXXXXXX mv /var/log/netatalk.log.zXXXXXX /var/log/netatalk.log Signal all daemon process(es)... sleep 10 >Fix: A patch is attached to this report. Here is the output from newsyslog with it: [klaas@hub ~]$ ./hacks/newsyslog/newsyslog -f newsyslog.netatalk.test -r -n -v -F Processing newsyslog.netatalk.test /var/log/netatalk.log <6>: size (Kb): 5 [50] --> trimming log.... rm -f /var/log/netatalk.log.6 rm -f /var/log/netatalk.log.6.gz rm -f /var/log/netatalk.log.6.bz2 rm -f /var/log/netatalk.log.6.xz rm -f /var/log/netatalk.log.5 rm -f /var/log/netatalk.log.5.gz rm -f /var/log/netatalk.log.5.bz2 rm -f /var/log/netatalk.log.5.xz mv /var/log/netatalk.log.2.gz /var/log/netatalk.log.3.gz chmod 644 /var/log/netatalk.log.3.gz chown 0:0 /var/log/netatalk.log.3.gz mv /var/log/netatalk.log.1.gz /var/log/netatalk.log.2.gz chmod 644 /var/log/netatalk.log.2.gz chown 0:0 /var/log/netatalk.log.2.gz mv /var/log/netatalk.log.0 /var/log/netatalk.log.1 chmod 644 /var/log/netatalk.log.1 chown 0:0 /var/log/netatalk.log.1 ln /var/log/netatalk.log /var/log/netatalk.log.0 chmod 644 /var/log/netatalk.log.0 chown 0:0 /var/log/netatalk.log.0 Start new log... mktemp /var/log/netatalk.log.zXXXXXX chown 0:0 /var/log/netatalk.log.zXXXXXX chmod 644 /var/log/netatalk.log.zXXXXXX mv /var/log/netatalk.log.zXXXXXX /var/log/netatalk.log Signal all daemon process(es)... do_sigwork: /usr/home/klaas/restart.netatalk, run_cmd 1, pidok 0, pid 0 noaction: 1 Run command: /usr/home/klaas/restart.netatalk 1 sleep 10 Warning: This patch has not been tested in any way. I only confirmed that things seem to happen as expected when running newsyslog with -n and -v. Other notes: 1) I am not sure why the original implementation of do_sigwork() passes the signal number to the external program. It seems to he redundant because I would expect the dedicated script to know what to do. 2) I am no security expert, but in my mind the system() call from stdlib is a security risk. system() passes the string to the sh command interpreter and thus is open to manipulation of the search path. I would recommend that newsyslog uses fork/exec for calling external programs, including kill(1). Patch attached with submission follows: --- /usr/src/usr.sbin/newsyslog/newsyslog.c 2012-12-03 16:22:39.000000000 -0500 +++ newsyslog.c 2013-12-01 00:48:48.607206990 -0500 @@ -1855,7 +1855,7 @@ int kres, secs; char *tmp; - if (!(swork->sw_pidok) || swork->sw_pid == 0) + if (!(swork->run_cmd) && (!(swork->sw_pidok) || swork->sw_pid == 0) ) return; /* no work to do... */ /* @@ -1888,14 +1888,6 @@ secs = 1; } - if (noaction) { - printf("\tkill -%d %d \t\t# %s\n", swork->sw_signum, - (int)swork->sw_pid, swork->sw_fname); - if (secs > 0) - printf("\tsleep %d\n", secs); - return; - } - if (swork->run_cmd) { asprintf(&tmp, "%s %d", swork->sw_fname, swork->sw_signum); if (tmp == NULL) { @@ -1905,15 +1897,25 @@ } if (verbose) printf("Run command: %s\n", tmp); - kres = system(tmp); - if (kres) { - warnx("%s: returned non-zero exit code: %d", - tmp, kres); + if( !noaction ) { + kres = system(tmp); + if (kres) { + warnx("%s: returned non-zero exit code: %d", + tmp, kres); + } } free(tmp); return; } + if (noaction) { + printf("\tkill -%d %d \t\t# %s\n", swork->sw_signum, + (int)swork->sw_pid, swork->sw_fname); + if (secs > 0) + printf("\tsleep %d\n", secs); + return; + } + kres = kill(swork->sw_pid, swork->sw_signum); if (kres != 0) { /* >Release-Note: >Audit-Trail: >Unformatted: