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@

diff --git a/sbin/pflogd/pflogd.8 b/sbin/pflogd/pflogd.8
index cd5b791..55f0d71 100644 (file)
--- 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 d35ae00..e855bab 100644 (file)
--- 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 29826bd..e0ea202 100644 (file)
--- 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)