In order to be able to safely append to existing log files, pflogd(8)
attempts to validate/or move invalid/broken pflog pcap files out of the
way on its own. I noticed that this is not compatible with unveil(2), as
pflogd(8) would need to be able to rename(2) files in /var/log to
/var/log/pflog.bad.*
Currently, if pflogd(8) is unable to move the log file out of the way,
it suspends logging until it receives SIGHUP/SIGALRM indicating that
the log file has been moved out of the way manually. I propose that this
become the default and only behaviour. pflogd(8) already reports to
syslog that logging has been suspended:
pflogd[93742]: Invalid/incompatible log file, move it away
pflogd[93742]: Logging suspended: open error
And now when resumed:
pflogd[93742]: Logging resumed
This part can be dropped if found to be too noisy.
It looks like in the past this was used when the log format had changed
incompatibly, I/O errors might indicate you have bigger issues, so we
probably shouldn't be writing to disk..
Comments? Ok? :-)
-Bryan.
Index: pflogd.8
===================================================================
RCS file: /cvs/src/sbin/pflogd/pflogd.8,v
retrieving revision 1.49
diff -u -p -u -r1.49 pflogd.8
--- pflogd.8 30 May 2017 17:15:06 -0000 1.49
+++ pflogd.8 5 Aug 2018 12:18:34 -0000
@@ -86,9 +86,8 @@ temporarily uses the old snaplen to keep
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
Index: pflogd.c
===================================================================
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.58
diff -u -p -u -r1.58 pflogd.c
--- pflogd.c 9 Sep 2017 13:02:52 -0000 1.58
+++ pflogd.c 5 Aug 2018 12:18:34 -0000
@@ -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;
}
Index: privsep.c
===================================================================
RCS file: /cvs/src/sbin/pflogd/privsep.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 privsep.c
--- privsep.c 9 Sep 2017 13:02:52 -0000 1.30
+++ privsep.c 5 Aug 2018 12:18:34 -0000
@@ -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 cpa
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)
@@ -345,22 +294,6 @@ priv_open_log(void)
fd = receive_fd(priv_fd);
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 */
