From 4b2941040aa61d85cab3466eb80e5b1763fe98b4 Mon Sep 17 00:00:00 2001 From: brynet Date: Sun, 26 Aug 2018 18:24:46 +0000 Subject: [PATCH] pflogd(8): don't try to rename(2) broken/invalid pflog files, instead, suspend logging until the log file has been moved out of the way, and we have received either SIGHUP or SIGALRM. ok florian@ deraadt@ --- sbin/pflogd/pflogd.8 | 9 +++--- sbin/pflogd/pflogd.c | 43 ++++++++------------------- sbin/pflogd/privsep.c | 69 +------------------------------------------ 3 files changed, 17 insertions(+), 104 deletions(-) diff --git a/sbin/pflogd/pflogd.8 b/sbin/pflogd/pflogd.8 index cd5b79189da..55f0d71017e 100644 --- a/sbin/pflogd/pflogd.8 +++ b/sbin/pflogd/pflogd.8 @@ -1,4 +1,4 @@ -.\" $OpenBSD: pflogd.8,v 1.49 2017/05/30 17:15:06 schwarze Exp $ +.\" $OpenBSD: pflogd.8,v 1.50 2018/08/26 18:24:46 brynet Exp $ .\" .\" Copyright (c) 2001 Can Erkin Acar. All rights reserved. .\" @@ -24,7 +24,7 @@ .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.Dd $Mdocdate: May 30 2017 $ +.Dd $Mdocdate: August 26 2018 $ .Dt PFLOGD 8 .Os .Sh NAME @@ -86,9 +86,8 @@ temporarily uses the old snaplen to keep the log file consistent. tries to preserve the integrity of the log file against I/O errors. Furthermore, integrity of an existing log file is verified before appending. -If there is an invalid log file or an I/O error, the log file is moved -out of the way and a new one is created. -If a new file cannot be created, logging is suspended until a +If there is an invalid log file or an I/O error, logging is suspended +until a .Dv SIGHUP or a .Dv SIGALRM diff --git a/sbin/pflogd/pflogd.c b/sbin/pflogd/pflogd.c index d35ae001c0a..e855bab910f 100644 --- a/sbin/pflogd/pflogd.c +++ b/sbin/pflogd/pflogd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pflogd.c,v 1.58 2017/09/09 13:02:52 brynet Exp $ */ +/* $OpenBSD: pflogd.c,v 1.59 2018/08/26 18:24:46 brynet Exp $ */ /* * Copyright (c) 2001 Theo de Raadt @@ -75,7 +75,7 @@ int flush_buffer(FILE *); int if_exists(char *); void logmsg(int, const char *, ...); void purge_buffer(void); -int reset_dump(int); +int reset_dump(void); int scan_dump(FILE *, off_t); int set_snaplen(int); void set_suspended(int); @@ -84,8 +84,6 @@ void sig_close(int); void sig_hup(int); void usage(void); -static int try_reset_dump(int); - /* buffer must always be greater than snaplen */ static int bufpkt = 0; /* number of packets in buffer */ static int buflen = 0; /* allocated size of buffer */ @@ -238,25 +236,7 @@ set_snaplen(int snap) } int -reset_dump(int nomove) -{ - int ret; - - for (;;) { - ret = try_reset_dump(nomove); - if (ret <= 0) - break; - } - - return (ret); -} - -/* - * tries to (re)open log file, nomove flag is used with -x switch - * returns 0: success, 1: retry (log moved), -1: error - */ -int -try_reset_dump(int nomove) +reset_dump(void) { struct pcap_file_header hdr; struct stat st; @@ -323,12 +303,9 @@ try_reset_dump(int nomove) } } else if (scan_dump(fp, st.st_size)) { fclose(fp); - if (nomove || priv_move_log()) { - logmsg(LOG_ERR, - "Invalid/incompatible log file, move it away"); - return (-1); - } - return (1); + logmsg(LOG_ERR, + "Invalid/incompatible log file, move it away"); + return (-1); } dpcap = fp; @@ -641,7 +618,7 @@ main(int argc, char **argv) bufpkt = 0; } - if (reset_dump(Xflag) < 0) { + if (reset_dump() < 0) { if (Xflag) return (1); @@ -666,10 +643,14 @@ main(int argc, char **argv) if (gotsig_close) break; if (gotsig_hup) { - if (reset_dump(0)) { + int was_suspended = suspended; + if (reset_dump()) { logmsg(LOG_ERR, "Logging suspended: open error"); set_suspended(1); + } else { + if (was_suspended) + logmsg(LOG_NOTICE, "Logging resumed"); } gotsig_hup = 0; } diff --git a/sbin/pflogd/privsep.c b/sbin/pflogd/privsep.c index 29826bdc27a..e0ea2026bd3 100644 --- a/sbin/pflogd/privsep.c +++ b/sbin/pflogd/privsep.c @@ -1,4 +1,4 @@ -/* $OpenBSD: privsep.c,v 1.30 2017/09/09 13:02:52 brynet Exp $ */ +/* $OpenBSD: privsep.c,v 1.31 2018/08/26 18:24:46 brynet Exp $ */ /* * Copyright (c) 2003 Can Erkin Acar @@ -42,7 +42,6 @@ enum cmd_types { PRIV_INIT_PCAP, /* init pcap fdpass bpf */ PRIV_SET_SNAPLEN, /* set the snaplength */ - PRIV_MOVE_LOG, /* move logfile away */ PRIV_OPEN_LOG, /* open logfile for appending */ }; @@ -54,7 +53,6 @@ static int may_read(int, void *, size_t); static void must_read(int, void *, size_t); static void must_write(int, void *, size_t); static int set_snaplen(int snap); -static int move_log(const char *name); extern char *filename; extern char *interface; @@ -195,13 +193,6 @@ BROKEN if (pledge("stdio rpath wpath cpath sendfd proc bpf", NULL) == -1) filename, strerror(olderrno)); break; - case PRIV_MOVE_LOG: - logmsg(LOG_DEBUG, - "[priv]: msg PRIV_MOVE_LOG received"); - ret = move_log(filename); - must_write(socks[0], &ret, sizeof(int)); - break; - default: logmsg(LOG_ERR, "[priv]: unknown command %d", cmd); _exit(1); @@ -225,48 +216,6 @@ set_snaplen(int snap) return 0; } -static int -move_log(const char *name) -{ - char ren[PATH_MAX]; - int len; - - for (;;) { - int fd; - - len = snprintf(ren, sizeof(ren), "%s.bad.XXXXXXXX", name); - if (len >= sizeof(ren)) { - logmsg(LOG_ERR, "[priv] new name too long"); - return (1); - } - - /* lock destination */ - fd = mkstemp(ren); - if (fd >= 0) { - close(fd); - break; - } - if (errno != EINTR) { - logmsg(LOG_ERR, "[priv] failed to create new name: %s", - strerror(errno)); - return (1); - } - } - - if (rename(name, ren)) { - logmsg(LOG_ERR, "[priv] failed to rename %s to %s: %s", - name, ren, strerror(errno)); - unlink(ren); - return (1); - } - - logmsg(LOG_NOTICE, - "[priv]: log file %s moved to %s", name, ren); - - return (0); -} - - /* receive bpf fd from privileged process using fdpass and init pcap */ int priv_init_pcap(int snaplen) @@ -347,22 +296,6 @@ priv_open_log(void) return (fd); } -/* Move-away and reopen log-file */ -int -priv_move_log(void) -{ - int cmd, ret; - - if (priv_fd < 0) - errx(1, "%s: called from privileged portion", __func__); - - cmd = PRIV_MOVE_LOG; - must_write(priv_fd, &cmd, sizeof(int)); - must_read(priv_fd, &ret, sizeof(int)); - - return (ret); -} - /* If priv parent gets a TERM or HUP, pass it through to child instead */ static void sig_pass_to_chld(int sig) -- 2.20.1