Date: Thu, 28 Sep 2023 15:52:28 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: f30b063ee667 - main - syslogd: Only use peerlist during flag parsing Message-ID: <202309281552.38SFqS61021768@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=f30b063ee6678d8192a075bf907773b8b54d4354 commit f30b063ee6678d8192a075bf907773b8b54d4354 Author: Jake Freeland <jfree@FreeBSD.org> AuthorDate: 2023-09-01 02:49:45 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2023-09-28 15:51:52 +0000 syslogd: Only use peerlist during flag parsing Add logging sockets directly to the socklist, unless parsing flags. The peerlist is still needed to temporarily hold socket information until the configuration flags have been parsed. It is tempting to remove the entire peerlist, but addsock() can not determine if syslogd is in secure mode unless the flags have been parsed. Also, call pidfile_open() right after flag parsing so we can terminate if another syslogd instance is already running. Reviewed by: markj MFC after: 3 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D41364 --- usr.sbin/syslogd/syslogd.c | 421 ++++++++++++++++++++++----------------------- 1 file changed, 201 insertions(+), 220 deletions(-) diff --git a/usr.sbin/syslogd/syslogd.c b/usr.sbin/syslogd/syslogd.c index b61a10c20aca..c03acf301fd8 100644 --- a/usr.sbin/syslogd/syslogd.c +++ b/usr.sbin/syslogd/syslogd.c @@ -176,7 +176,8 @@ static const char include_ext[] = ".conf"; (((d)->s6_addr32[3] ^ (a)->s6_addr32[3]) & (m)->s6_addr32[3]) == 0 ) #endif /* - * List of peers and sockets for binding. + * List of peers and sockets that can't be bound until + * flags have been parsed. */ struct peer { const char *pe_name; @@ -186,13 +187,16 @@ struct peer { }; static STAILQ_HEAD(, peer) pqueue = STAILQ_HEAD_INITIALIZER(pqueue); +/* + * Sockets used for logging; monitored by kevent(). + */ struct socklist { struct addrinfo sl_ai; #define sl_sa sl_ai.ai_addr #define sl_salen sl_ai.ai_addrlen #define sl_family sl_ai.ai_family int sl_socket; - struct peer *sl_peer; + const char *sl_name; int (*sl_recv)(struct socklist *); STAILQ_ENTRY(socklist) next; }; @@ -419,9 +423,9 @@ static bool RFC3164OutputFormat = true; /* Use legacy format by default. */ struct iovlist; -static int allowaddr(char *); -static int addpeer(struct peer *); -static int addsock(struct addrinfo *, struct socklist *); +static bool allowaddr(char *); +static void addpeer(const char *, const char *, mode_t); +static void addsock(const char *, const char *, mode_t); static void cfline(const char *, const char *, const char *, const char *); static const char *cvthname(struct sockaddr *); static void deadq_enter(pid_t, const char *); @@ -440,7 +444,7 @@ static void logmsg(int, const struct logtime *, const char *, const char *, const char *, const char *, const char *, const char *, int); static void log_deadchild(pid_t, int, const char *); static void markit(void); -static int socksetup(struct peer *); +static struct socklist *socksetup(struct addrinfo *, const char *, mode_t); static int socklist_recv_file(struct socklist *); static int socklist_recv_sock(struct socklist *); static int skip_message(const char *, const char *, int); @@ -490,41 +494,91 @@ close_filed(struct filed *f) f->f_file = -1; } -static int -addpeer(struct peer *pe0) +static void +addpeer(const char *name, const char *serv, mode_t mode) { - struct peer *pe; - - pe = calloc(1, sizeof(*pe)); + struct peer *pe = calloc(1, sizeof(*pe)); if (pe == NULL) err(1, "malloc failed"); - *pe = *pe0; + pe->pe_name = name; + pe->pe_serv = serv; + pe->pe_mode = mode; STAILQ_INSERT_TAIL(&pqueue, pe, next); - - return (0); } -static int -addsock(struct addrinfo *ai, struct socklist *sl0) +static void +addsock(const char *name, const char *serv, mode_t mode) { + struct addrinfo hints = { }, *res, *res0; struct socklist *sl; + int error; + char *cp, *msgbuf; - /* Copy *ai->ai_addr to the tail of struct socklist if any. */ - sl = calloc(1, sizeof(*sl) + ((ai != NULL) ? ai->ai_addrlen : 0)); + /* + * We have to handle this case for backwards compatibility: + * If there are two (or more) colons but no '[' and ']', + * assume this is an inet6 address without a service. + */ + if (name != NULL) { +#ifdef INET6 + if (name[0] == '[' && + (cp = strchr(name + 1, ']')) != NULL) { + name = &name[1]; + *cp = '\0'; + if (cp[1] == ':' && cp[2] != '\0') + serv = cp + 2; + } else { +#endif + cp = strchr(name, ':'); + if (cp != NULL && strchr(cp + 1, ':') == NULL) { + *cp = '\0'; + if (cp[1] != '\0') + serv = cp + 1; + if (cp == name) + name = NULL; + } +#ifdef INET6 + } +#endif + } + hints.ai_family = AF_UNSPEC; + hints.ai_socktype = SOCK_DGRAM; + hints.ai_flags = AI_PASSIVE; + if (name != NULL) + dprintf("Trying peer: %s\n", name); + if (serv == NULL) + serv = "syslog"; + error = getaddrinfo(name, serv, &hints, &res0); + if (error) { + asprintf(&msgbuf, "getaddrinfo failed for %s%s: %s", + name == NULL ? "" : name, serv, + gai_strerror(error)); + errno = 0; + if (msgbuf == NULL) + logerror(gai_strerror(error)); + else + logerror(msgbuf); + free(msgbuf); + die(0); + } + for (res = res0; res != NULL; res = res->ai_next) { + sl = socksetup(res, name, mode); + if (sl == NULL) + continue; + STAILQ_INSERT_TAIL(&shead, sl, next); + } + freeaddrinfo(res0); +} + +static void +addfile(int fd) +{ + struct socklist *sl = calloc(1, sizeof(*sl)); if (sl == NULL) err(1, "malloc failed"); - *sl = *sl0; - if (ai != NULL) { - memcpy(&sl->sl_ai, ai, sizeof(*ai)); - if (ai->ai_addrlen > 0) { - memcpy((sl + 1), ai->ai_addr, ai->ai_addrlen); - sl->sl_sa = (struct sockaddr *)(sl + 1); - } else - sl->sl_sa = NULL; - } + sl->sl_socket = fd; + sl->sl_recv = socklist_recv_file; STAILQ_INSERT_TAIL(&shead, sl, next); - - return (0); } int @@ -533,7 +587,6 @@ main(int argc, char *argv[]) sigset_t sigset = { }; struct kevent ev; struct socklist *sl; - struct peer *pe; pid_t ppid = -1, spid; int ch, kq, s; char *p; @@ -562,7 +615,7 @@ main(int argc, char *argv[]) send_to_all = true; break; case 'a': /* allow specific network addresses only */ - if (allowaddr(optarg) == -1) + if (!allowaddr(optarg)) usage(); break; case 'b': @@ -577,18 +630,12 @@ main(int argc, char *argv[]) } if (p == NULL) { /* A hostname or filename only. */ - addpeer(&(struct peer){ - .pe_name = optarg, - .pe_serv = "syslog" - }); + addpeer(optarg, "syslog", 0); } else { /* The case of "name:service". */ *p++ = '\0'; - addpeer(&(struct peer){ - .pe_serv = p, - .pe_name = (strlen(optarg) == 0) ? - NULL : optarg, - }); + addpeer(strlen(optarg) == 0 ? NULL : optarg, + p, 0); } break; case 'c': @@ -649,10 +696,7 @@ main(int argc, char *argv[]) } else errx(1, "invalid filename %s, exiting", optarg); - addpeer(&(struct peer){ - .pe_name = name, - .pe_mode = mode - }); + addpeer(name, NULL, mode); break; } case 'M': /* max length of forwarded message */ @@ -709,42 +753,39 @@ main(int argc, char *argv[]) if (RFC3164OutputFormat && MaxForwardLen > 1024) errx(1, "RFC 3164 messages may not exceed 1024 bytes"); + pfh = pidfile_open(PidFile, 0600, &spid); + if (pfh == NULL) { + if (errno == EEXIST) + errx(1, "syslogd already running, pid: %d", spid); + warn("cannot open pid file"); + } + + /* + * Now that flags have been parsed, we know if we're in + * secure mode. Add peers to the socklist, if allowed. + */ + while (!STAILQ_EMPTY(&pqueue)) { + struct peer *pe = STAILQ_FIRST(&pqueue); + STAILQ_REMOVE_HEAD(&pqueue, next); + addsock(pe->pe_name, pe->pe_serv, pe->pe_mode); + free(pe); + } /* Listen by default: /dev/klog. */ s = open(_PATH_KLOG, O_RDONLY | O_NONBLOCK | O_CLOEXEC, 0); if (s < 0) { dprintf("can't open %s (%d)\n", _PATH_KLOG, errno); } else { - addsock(NULL, &(struct socklist){ - .sl_socket = s, - .sl_recv = socklist_recv_file, - }); + addfile(s); } /* Listen by default: *:514 if no -b flag. */ if (bflag == 0) - addpeer(&(struct peer){ - .pe_serv = "syslog" - }); + addsock(NULL, "syslog", 0); /* Listen by default: /var/run/log if no -p flag. */ if (pflag == 0) - addpeer(&(struct peer){ - .pe_name = _PATH_LOG, - .pe_mode = DEFFILEMODE, - }); + addsock(_PATH_LOG, NULL, DEFFILEMODE); /* Listen by default: /var/run/logpriv if no -S flag. */ if (Sflag == 0) - addpeer(&(struct peer){ - .pe_name = _PATH_LOG_PRIV, - .pe_mode = S_IRUSR | S_IWUSR, - }); - STAILQ_FOREACH(pe, &pqueue, next) - socksetup(pe); - - pfh = pidfile_open(PidFile, 0600, &spid); - if (pfh == NULL) { - if (errno == EEXIST) - errx(1, "syslogd already running, pid: %d", spid); - warn("cannot open pid file"); - } + addsock(_PATH_LOG_PRIV, NULL, S_IRUSR | S_IWUSR); if ((!Foreground) && (!Debug)) { ppid = waitdaemon(30); @@ -2312,7 +2353,7 @@ die(int signo) } STAILQ_FOREACH(sl, &shead, next) { if (sl->sl_sa != NULL && sl->sl_family == AF_LOCAL) - unlink(sl->sl_peer->pe_name); + unlink(sl->sl_name); } pidfile_remove(pfh); @@ -3272,9 +3313,9 @@ timedout(int sig __unused) * * netaddr/maskbits[:{servicename|portnumber|*}] * - * Returns -1 on error, 0 if the argument was valid. + * Returns false on error, true if the argument was valid. */ -static int +static bool #if defined(INET) || defined(INET6) allowaddr(char *s) #else @@ -3442,13 +3483,13 @@ allowaddr(char *s __unused) printf("port = %d\n", ap->port); } - return (0); + return (true); err: if (res != NULL) freeaddrinfo(res); free(ap); #endif - return (-1); + return (false); } /* @@ -3710,167 +3751,107 @@ log_deadchild(pid_t pid, int status, const char *name) logerror(buf); } -static int -socksetup(struct peer *pe) +static struct socklist * +socksetup(struct addrinfo *ai, const char *name, mode_t mode) { - struct addrinfo hints, *res, *res0; - int error; - char *cp; + struct socklist *sl; int (*sl_recv)(struct socklist *); - /* - * We have to handle this case for backwards compatibility: - * If there are two (or more) colons but no '[' and ']', - * assume this is an inet6 address without a service. - */ - if (pe->pe_name != NULL) { -#ifdef INET6 - if (pe->pe_name[0] == '[' && - (cp = strchr(pe->pe_name + 1, ']')) != NULL) { - pe->pe_name = &pe->pe_name[1]; - *cp = '\0'; - if (cp[1] == ':' && cp[2] != '\0') - pe->pe_serv = cp + 2; - } else { -#endif - cp = strchr(pe->pe_name, ':'); - if (cp != NULL && strchr(cp + 1, ':') == NULL) { - *cp = '\0'; - if (cp[1] != '\0') - pe->pe_serv = cp + 1; - if (cp == pe->pe_name) - pe->pe_name = NULL; - } -#ifdef INET6 - } -#endif - } - hints = (struct addrinfo){ - .ai_family = AF_UNSPEC, - .ai_socktype = SOCK_DGRAM, - .ai_flags = AI_PASSIVE - }; - if (pe->pe_name != NULL) - dprintf("Trying peer: %s\n", pe->pe_name); - if (pe->pe_serv == NULL) - pe->pe_serv = "syslog"; - error = getaddrinfo(pe->pe_name, pe->pe_serv, &hints, &res0); - if (error) { - char *msgbuf; + int s, optval = 1; - asprintf(&msgbuf, "getaddrinfo failed for %s%s: %s", - pe->pe_name == NULL ? "" : pe->pe_name, pe->pe_serv, - gai_strerror(error)); - errno = 0; - if (msgbuf == NULL) - logerror(gai_strerror(error)); - else - logerror(msgbuf); - free(msgbuf); - die(0); + if (ai->ai_family != AF_LOCAL && SecureMode > 1) { + /* Only AF_LOCAL in secure mode. */ + return (NULL); } - for (res = res0; res != NULL; res = res->ai_next) { - int s; - - if (res->ai_family != AF_LOCAL && - SecureMode > 1) { - /* Only AF_LOCAL in secure mode. */ - continue; - } - if (family != AF_UNSPEC && - res->ai_family != AF_LOCAL && res->ai_family != family) - continue; + if (family != AF_UNSPEC && ai->ai_family != AF_LOCAL && + ai->ai_family != family) + return (NULL); - s = socket(res->ai_family, res->ai_socktype, - res->ai_protocol); - if (s < 0) { - logerror("socket"); - error++; - continue; - } + s = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol); + if (s < 0) { + logerror("socket"); + return (NULL); + } #ifdef INET6 - if (res->ai_family == AF_INET6) { - if (setsockopt(s, IPPROTO_IPV6, IPV6_V6ONLY, - &(int){1}, sizeof(int)) < 0) { - logerror("setsockopt(IPV6_V6ONLY)"); - close(s); - error++; - continue; - } - } -#endif - if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR, - &(int){1}, sizeof(int)) < 0) { - logerror("setsockopt(SO_REUSEADDR)"); + if (ai->ai_family == AF_INET6) { + if (setsockopt(s, IPPROTO_IPV6, IPV6_V6ONLY, &optval, + sizeof(int)) < 0) { + logerror("setsockopt(IPV6_V6ONLY)"); close(s); - error++; - continue; + return (NULL); } + } +#endif + if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &optval, + sizeof(int)) < 0) { + logerror("setsockopt(SO_REUSEADDR)"); + close(s); + return (NULL); + } - /* - * Bind INET and UNIX-domain sockets. - * - * A UNIX-domain socket is always bound to a pathname - * regardless of -N flag. - * - * For INET sockets, RFC 3164 recommends that client - * side message should come from the privileged syslogd port. - * - * If the system administrator chooses not to obey - * this, we can skip the bind() step so that the - * system will choose a port for us. - */ - if (res->ai_family == AF_LOCAL) - unlink(pe->pe_name); - if (res->ai_family == AF_LOCAL || - NoBind == 0 || pe->pe_name != NULL) { - if (bind(s, res->ai_addr, res->ai_addrlen) < 0) { - logerror("bind"); - close(s); - error++; - continue; - } - if (res->ai_family == AF_LOCAL || - SecureMode == 0) - increase_rcvbuf(s); - } - if (res->ai_family == AF_LOCAL && - chmod(pe->pe_name, pe->pe_mode) < 0) { - dprintf("chmod %s: %s\n", pe->pe_name, - strerror(errno)); + /* + * Bind INET and UNIX-domain sockets. + * + * A UNIX-domain socket is always bound to a pathname + * regardless of -N flag. + * + * For INET sockets, RFC 3164 recommends that client + * side message should come from the privileged syslogd port. + * + * If the system administrator chooses not to obey + * this, we can skip the bind() step so that the + * system will choose a port for us. + */ + if (ai->ai_family == AF_LOCAL) + unlink(name); + if (ai->ai_family == AF_LOCAL || NoBind == 0 || name != NULL) { + if (bind(s, ai->ai_addr, ai->ai_addrlen) < 0) { + logerror("bind"); close(s); - error++; - continue; - } - dprintf("new socket fd is %d\n", s); - if (res->ai_socktype != SOCK_DGRAM) { - listen(s, 5); + return (NULL); } - sl_recv = socklist_recv_sock; + if (ai->ai_family == AF_LOCAL || SecureMode == 0) + increase_rcvbuf(s); + } + if (ai->ai_family == AF_LOCAL && chmod(name, mode) < 0) { + dprintf("chmod %s: %s\n", name, strerror(errno)); + close(s); + return (NULL); + } + dprintf("new socket fd is %d\n", s); + if (ai->ai_socktype != SOCK_DGRAM) { + listen(s, 5); + } + sl_recv = socklist_recv_sock; #if defined(INET) || defined(INET6) - if (SecureMode && (res->ai_family == AF_INET || - res->ai_family == AF_INET6)) { - dprintf("shutdown\n"); - /* Forbid communication in secure mode. */ - if (shutdown(s, SHUT_RD) < 0 && - errno != ENOTCONN) { - logerror("shutdown"); - if (!Debug) - die(0); - } - sl_recv = NULL; - } else + if (SecureMode && (ai->ai_family == AF_INET || + ai->ai_family == AF_INET6)) { + dprintf("shutdown\n"); + /* Forbid communication in secure mode. */ + if (shutdown(s, SHUT_RD) < 0 && errno != ENOTCONN) { + logerror("shutdown"); + if (!Debug) + die(0); + } + sl_recv = NULL; + } else #endif - dprintf("listening on socket\n"); - dprintf("sending on socket\n"); - addsock(res, &(struct socklist){ - .sl_socket = s, - .sl_peer = pe, - .sl_recv = sl_recv - }); + dprintf("listening on socket\n"); + dprintf("sending on socket\n"); + /* Copy *ai->ai_addr to the tail of struct socklist if any. */ + sl = calloc(1, sizeof(*sl) + ai->ai_addrlen); + if (sl == NULL) + err(1, "malloc failed"); + sl->sl_socket = s; + sl->sl_name = name; + sl->sl_recv = sl_recv; + (void)memcpy(&sl->sl_ai, ai, sizeof(*ai)); + if (ai->ai_addrlen > 0) { + (void)memcpy((sl + 1), ai->ai_addr, ai->ai_addrlen); + sl->sl_sa = (struct sockaddr *)(sl + 1); + } else { + sl->sl_sa = NULL; } - freeaddrinfo(res0); - - return(error); + return (sl); } static void
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202309281552.38SFqS61021768>