Re: syslogd closing all udp is a tiny bit aggressiv

2020-02-10 Thread sven falempin
On Sun, Feb 9, 2020 at 4:12 PM Alexander Bluhm 
wrote:

> On Thu, Feb 06, 2020 at 05:57:15PM -0500, sven falempin wrote:
> > > Your DNS lookup fails at startup, sockets are closed.
> > > Later at SIGHUP you DNS works again.  Now the sockets are needed.
> > > So do not close them if DNS for udp fails.
>
> I thought again about this problem.  The fix can be more specific.
> - if user requested udp4 or udp6, close the other af socket.
> - after SIGHUP, when DNS works, close the unneeded af socket.
>
> ok?
>
> Index: usr.sbin/syslogd/syslogd.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
> retrieving revision 1.262
> diff -u -p -r1.262 syslogd.c
> --- usr.sbin/syslogd/syslogd.c  5 Jul 2019 13:23:27 -   1.262
> +++ usr.sbin/syslogd/syslogd.c  9 Feb 2020 20:25:20 -
> @@ -853,20 +853,6 @@ main(int argc, char *argv[])
> event_add(ev_udp, NULL);
> if (fd_udp6 != -1)
> event_add(ev_udp6, NULL);
> -   } else {
> -   /*
> -* If generic UDP file descriptors are used neither
> -* for receiving nor for sending, close them.  Then
> -* there is no useless *.514 in netstat.
> -*/
> -   if (fd_udp != -1 && !send_udp) {
> -   close(fd_udp);
> -   fd_udp = -1;
> -   }
> -   if (fd_udp6 != -1 && !send_udp6) {
> -   close(fd_udp6);
> -   fd_udp6 = -1;
> -   }
> }
> for (i = 0; i < nbind; i++)
> if (fd_bind[i] != -1)
> @@ -2416,6 +2402,7 @@ init(void)
> s = 0;
> strlcpy(progblock, "*", sizeof(progblock));
> strlcpy(hostblock, "*", sizeof(hostblock));
> +   send_udp = send_udp6 = 0;
> while (getline(&cline, &s, cf) != -1) {
> /*
>  * check for end-of-section, comments, strip off trailing
> @@ -2508,6 +2495,22 @@ init(void)
> Initialized = 1;
> dropped_warn(&init_dropped, "during initialization");
>
> +   if (SecureMode) {
> +   /*
> +* If generic UDP file descriptors are used neither
> +* for receiving nor for sending, close them.  Then
> +* there is no useless *.514 in netstat.
> +*/
> +   if (fd_udp != -1 && !send_udp) {
> +   close(fd_udp);
> +   fd_udp = -1;
> +   }
> +   if (fd_udp6 != -1 && !send_udp6) {
> +   close(fd_udp6);
> +   fd_udp6 = -1;
> +   }
> +   }
> +
> if (Debug) {
> SIMPLEQ_FOREACH(f, &Files, f_next) {
> for (i = 0; i <= LOG_NFACILITIES; i++)
> @@ -2755,6 +2758,13 @@ cfline(char *line, char *progblock, char
> sizeof(f->f_un.f_forw.f_addr)) != 0) {
> log_warnx("bad hostname \"%s\"",
> f->f_un.f_forw.f_loghost);
> +   /* DNS lookup may work after SIGHUP, keep sockets
> */
> +   if (strcmp(proto, "udp") == 0)
> +   send_udp = send_udp6 = 1;
> +   else if (strcmp(proto, "udp4") == 0)
> +   send_udp = 1;
> +   else if (strcmp(proto, "udp6") == 0)
> +   send_udp6 = 1;
> break;
> }
> f->f_file = -1;
>


@ok here

-- 
--
-
Knowing is not enough; we must apply. Willing is not enough; we must do


Re: syslogd closing all udp is a tiny bit aggressiv

2020-02-09 Thread Alexander Bluhm
On Thu, Feb 06, 2020 at 05:57:15PM -0500, sven falempin wrote:
> > Your DNS lookup fails at startup, sockets are closed.
> > Later at SIGHUP you DNS works again.  Now the sockets are needed.
> > So do not close them if DNS for udp fails.

I thought again about this problem.  The fix can be more specific.
- if user requested udp4 or udp6, close the other af socket.
- after SIGHUP, when DNS works, close the unneeded af socket.

ok?

Index: usr.sbin/syslogd/syslogd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.262
diff -u -p -r1.262 syslogd.c
--- usr.sbin/syslogd/syslogd.c  5 Jul 2019 13:23:27 -   1.262
+++ usr.sbin/syslogd/syslogd.c  9 Feb 2020 20:25:20 -
@@ -853,20 +853,6 @@ main(int argc, char *argv[])
event_add(ev_udp, NULL);
if (fd_udp6 != -1)
event_add(ev_udp6, NULL);
-   } else {
-   /*
-* If generic UDP file descriptors are used neither
-* for receiving nor for sending, close them.  Then
-* there is no useless *.514 in netstat.
-*/
-   if (fd_udp != -1 && !send_udp) {
-   close(fd_udp);
-   fd_udp = -1;
-   }
-   if (fd_udp6 != -1 && !send_udp6) {
-   close(fd_udp6);
-   fd_udp6 = -1;
-   }
}
for (i = 0; i < nbind; i++)
if (fd_bind[i] != -1)
@@ -2416,6 +2402,7 @@ init(void)
s = 0;
strlcpy(progblock, "*", sizeof(progblock));
strlcpy(hostblock, "*", sizeof(hostblock));
+   send_udp = send_udp6 = 0;
while (getline(&cline, &s, cf) != -1) {
/*
 * check for end-of-section, comments, strip off trailing
@@ -2508,6 +2495,22 @@ init(void)
Initialized = 1;
dropped_warn(&init_dropped, "during initialization");

+   if (SecureMode) {
+   /*
+* If generic UDP file descriptors are used neither
+* for receiving nor for sending, close them.  Then
+* there is no useless *.514 in netstat.
+*/
+   if (fd_udp != -1 && !send_udp) {
+   close(fd_udp);
+   fd_udp = -1;
+   }
+   if (fd_udp6 != -1 && !send_udp6) {
+   close(fd_udp6);
+   fd_udp6 = -1;
+   }
+   }
+
if (Debug) {
SIMPLEQ_FOREACH(f, &Files, f_next) {
for (i = 0; i <= LOG_NFACILITIES; i++)
@@ -2755,6 +2758,13 @@ cfline(char *line, char *progblock, char
sizeof(f->f_un.f_forw.f_addr)) != 0) {
log_warnx("bad hostname \"%s\"",
f->f_un.f_forw.f_loghost);
+   /* DNS lookup may work after SIGHUP, keep sockets */
+   if (strcmp(proto, "udp") == 0)
+   send_udp = send_udp6 = 1;
+   else if (strcmp(proto, "udp4") == 0)
+   send_udp = 1;
+   else if (strcmp(proto, "udp6") == 0)
+   send_udp6 = 1;
break;
}
f->f_file = -1;



Re: syslogd closing all udp is a tiny bit aggressiv

2020-02-06 Thread sven falempin
On Thu, Feb 6, 2020 at 5:32 PM Alexander Bluhm  wrote:
>
> On Thu, Feb 06, 2020 at 11:46:25AM -0500, sven falempin wrote:
> > If for exemple there s a wrong endpoint in the config file, like
> > local1.warn @badhost
> > and no other the daemon will close  fd_udp.
>
> Your DNS lookup fails at startup, sockets are closed.
>
> > // reload with a badhost in /etc/hosts for the sake of testing
>
> Later at SIGHUP you DNS works again.  Now the sockets are needed.
>
> So do not close them if DNS for udp fails.
> Does this diff fix your setup?
>
> bluhm
>
> Index: usr.sbin/syslogd/syslogd.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
> retrieving revision 1.262
> diff -u -p -r1.262 syslogd.c
> --- usr.sbin/syslogd/syslogd.c  5 Jul 2019 13:23:27 -   1.262
> +++ usr.sbin/syslogd/syslogd.c  6 Feb 2020 21:51:30 -
> @@ -2416,6 +2416,7 @@ init(void)
> s = 0;
> strlcpy(progblock, "*", sizeof(progblock));
> strlcpy(hostblock, "*", sizeof(hostblock));
> +   send_udp = send_udp6 = 0;
> while (getline(&cline, &s, cf) != -1) {
> /*
>  * check for end-of-section, comments, strip off trailing
> @@ -2755,6 +2756,9 @@ cfline(char *line, char *progblock, char
> sizeof(f->f_un.f_forw.f_addr)) != 0) {
> log_warnx("bad hostname \"%s\"",
> f->f_un.f_forw.f_loghost);
> +   /* DNS lookup may work after SIGHUP, keep sockets */
> +   if (strncmp(proto, "udp", 3) == 0)
> +   send_udp = send_udp6 = 1;
> break;
> }
> f->f_file = -1;

It make sense and it totally works.
@ok

Thank you.

-- 
--
-
Knowing is not enough; we must apply. Willing is not enough; we must do



Re: syslogd closing all udp is a tiny bit aggressiv

2020-02-06 Thread Alexander Bluhm
On Thu, Feb 06, 2020 at 11:46:25AM -0500, sven falempin wrote:
> If for exemple there s a wrong endpoint in the config file, like
> local1.warn @badhost
> and no other the daemon will close  fd_udp.

Your DNS lookup fails at startup, sockets are closed.

> // reload with a badhost in /etc/hosts for the sake of testing

Later at SIGHUP you DNS works again.  Now the sockets are needed.

So do not close them if DNS for udp fails.
Does this diff fix your setup?

bluhm

Index: usr.sbin/syslogd/syslogd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.262
diff -u -p -r1.262 syslogd.c
--- usr.sbin/syslogd/syslogd.c  5 Jul 2019 13:23:27 -   1.262
+++ usr.sbin/syslogd/syslogd.c  6 Feb 2020 21:51:30 -
@@ -2416,6 +2416,7 @@ init(void)
s = 0;
strlcpy(progblock, "*", sizeof(progblock));
strlcpy(hostblock, "*", sizeof(hostblock));
+   send_udp = send_udp6 = 0;
while (getline(&cline, &s, cf) != -1) {
/*
 * check for end-of-section, comments, strip off trailing
@@ -2755,6 +2756,9 @@ cfline(char *line, char *progblock, char
sizeof(f->f_un.f_forw.f_addr)) != 0) {
log_warnx("bad hostname \"%s\"",
f->f_un.f_forw.f_loghost);
+   /* DNS lookup may work after SIGHUP, keep sockets */
+   if (strncmp(proto, "udp", 3) == 0)
+   send_udp = send_udp6 = 1;
break;
}
f->f_file = -1;



syslogd closing all udp is a tiny bit aggressiv

2020-02-06 Thread sven falempin
Hello,

Syslogd is supposed to reload the configuration on HUP, and does it well
but for one case.

If the demon does not have any endpoint it will close the FD opened *at start*

line 587
/*
* If generic UDP file descriptors are used neither
* for receiving nor for sending, close them.  Then
* there is no useless *.514 in netstat.
*/
if (fd_udp != -1 && !send_udp) {
close(fd_udp);
fd_udp = -1;
}

If for exemple there s a wrong endpoint in the config file, like
local1.warn @badhost
and no other the daemon will close  fd_udp.


when reloading the conf, if badhost suddenly resolved.
===
RCS file: /cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.262
diff -u -p -r1.262 syslogd.c
--- syslogd.c   5 Jul 2019 13:23:27 -   1.262
+++ syslogd.c   6 Feb 2020 16:28:34 -
@@ -2762,6 +2762,7 @@ cfline(char *line, char *progblock, char
switch (f->f_un.f_forw.f_addr.ss_family) {
case AF_INET:
send_udp = 1;
+   if ( fd_udp == -1 ) log_warnx("bad
oops cannot reload because new feature");
f->f_file = fd_udp;
break;
case AF_INET6:


send_udp is back to 1 but fd_udp is dead, RIP fd_udp !

logline: pri 053, flags 0x4, from current, msg syslogd[62371]: bad
hostname "@badhost "
// reload with a badhost in /etc/hosts for the sake of testing
logline: pri 053, flags 0x4, from current, msg syslogd[20219]: bad
oops cannot reload because new feature
Logging to FORWUDP @badhost
logline: pri 053, flags 0x4, from current, msg syslogd[62371]: sendto
"@totalcrap": Bad file descriptor


One easy fix would be to not close the FD ... or change the man

if (socket_bind("udp", NULL, "syslog", SecureMode,
&fd_udp, &fd_udp6) == -1)
log_warnx("socket bind * failed");

could be called in init when send_udp is set to 1, but it will crush
the udp6 one

I often have the wrong idea about the right fix, so please tell me !

For example i wonder if the udp socket could be put in the enpoint
struct,and open/close
when the enpoint state is valid/invalid.

Best, and thanks for all the work.

* this is tested on 6.6, with current version of syslogd * using cvs
-q up -Pd -CA as learned *

-- 
--
-
Knowing is not enough; we must apply. Willing is not enough; we must do