The branch main has been updated by markj:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=ae4f708f0b383277505daa191e21db399b558839

commit ae4f708f0b383277505daa191e21db399b558839
Author:     Mark Johnston <ma...@freebsd.org>
AuthorDate: 2025-01-14 14:19:56 +0000
Commit:     Mark Johnston <ma...@freebsd.org>
CommitDate: 2025-01-14 14:19:56 +0000

    syslogd: Ensure that forwarded messages are sent from port 514
    
    Prior to commit 4ecbee2760f7, syslogd used its listening socket(s) to
    forward messages to remote hosts, when so configured.  As a consequence,
    they are sent from the address+port to which those sockets are bound,
    typically 0.0.0.0:514.
    
    When in capability mode, sendto() is not permitted, so we instead
    pre-create sockets and connect them to the forwarding addresses, letting
    the kernel pick an ephemeral source port.  However, this doesn't match
    syslogd's previous behaviour, breaking some setups.
    
    So, restore the old behaviour by binding forwarding sockets to the
    addresses on which syslogd is listening.  Since we cannot use the same
    sockets for receiving messages and also for forwarding them, use
    SO_REUSEPORT to enable duplicate bindings to port 514, relying on the
    existing behaviour that the first socket bound to that port is the one
    that actually receives messages.
    
    Add some regression tests to cover this and related functionality of
    syslogd's -a option.
    
    Reviewed by:    jfree
    Reported by:    Michael Butler <i...@protected-networks.net>
    Fixes:          4ecbee2760f7 ("syslogd: Open forwarding socket descriptors")
    Differential Revision:  https://reviews.freebsd.org/D48222
---
 usr.sbin/syslogd/syslogd.c             | 179 +++++++++++++++++++++++----
 usr.sbin/syslogd/syslogd.h             |   6 +
 usr.sbin/syslogd/syslogd_cap_config.c  |   2 +
 usr.sbin/syslogd/tests/syslogd_test.sh | 216 +++++++++++++++++++++++++++++++++
 4 files changed, 378 insertions(+), 25 deletions(-)

diff --git a/usr.sbin/syslogd/syslogd.c b/usr.sbin/syslogd/syslogd.c
index e1d3dffe013a..726cedc17b1d 100644
--- a/usr.sbin/syslogd/syslogd.c
+++ b/usr.sbin/syslogd/syslogd.c
@@ -373,6 +373,7 @@ close_filed(struct filed *f)
        switch (f->f_type) {
        case F_FORW:
                if (f->f_addr_fds != NULL) {
+                       free(f->f_addrs);
                        for (size_t i = 0; i < f->f_num_addr_fds; ++i)
                                close(f->f_addr_fds[i]);
                        free(f->f_addr_fds);
@@ -2987,11 +2988,152 @@ parse_selector(const char *p, struct filed *f)
        return (q);
 }
 
+static int
+maybe_dup_forw_socket(const nvlist_t *nvl, const struct sockaddr *rsa,
+    const struct sockaddr *lsa)
+{
+       const nvlist_t * const *line;
+       size_t linecount;
+
+       if (!nvlist_exists_nvlist_array(nvl, "filed_list"))
+               return (-1);
+       line = nvlist_get_nvlist_array(nvl, "filed_list", &linecount);
+       for (size_t i = 0; i < linecount; i++) {
+               const struct forw_addr *forw;
+               const int *fdp;
+               size_t fdc;
+
+               if (nvlist_get_number(line[i], "f_type") != F_FORW)
+                       continue;
+               fdp = nvlist_get_descriptor_array(line[i], "f_addr_fds", &fdc);
+               forw = nvlist_get_binary(line[i], "f_addrs", NULL);
+               for (size_t j = 0; j < fdc; j++) {
+                       int fd;
+
+                       if (memcmp(&forw[j].raddr, rsa, rsa->sa_len) != 0 ||
+                           memcmp(&forw[j].laddr, lsa, lsa->sa_len) != 0)
+                               continue;
+
+                       fd = dup(fdp[j]);
+                       if (fd < 0)
+                               err(1, "dup");
+                       return (fd);
+               }
+       }
+
+       return (-1);
+}
+
+/*
+ * Create a UDP socket that will forward messages from "lai" to "ai".
+ * Capsicum doesn't permit connect() or sendto(), so we can't reuse the (bound)
+ * sockets used to listen for messages.
+ */
+static int
+make_forw_socket(const nvlist_t *nvl, struct addrinfo *ai, struct addrinfo 
*lai)
+{
+       int s;
+
+       s = socket(ai->ai_family, ai->ai_socktype, 0);
+       if (s < 0)
+               err(1, "socket");
+       if (lai != NULL) {
+               if (setsockopt(s, SOL_SOCKET, SO_REUSEPORT, &(int){1},
+                   sizeof(int)) < 0)
+                       err(1, "setsockopt");
+               if (bind(s, lai->ai_addr, lai->ai_addrlen) < 0)
+                       err(1, "bind");
+       }
+       if (connect(s, ai->ai_addr, ai->ai_addrlen) < 0) {
+               if (errno == EADDRINUSE && lai != NULL) {
+                       int s1;
+
+                       s1 = maybe_dup_forw_socket(nvl, ai->ai_addr,
+                           lai->ai_addr);
+                       if (s1 < 0)
+                               errc(1, EADDRINUSE, "connect");
+                       (void)close(s);
+                       s = s1;
+               } else {
+                       err(1, "connect");
+               }
+       }
+       /* Make it a write-only socket. */
+       if (shutdown(s, SHUT_RD) < 0)
+               err(1, "shutdown");
+
+       return (s);
+}
+
+static void
+make_forw_socket_array(const nvlist_t *nvl, struct filed *f,
+    struct addrinfo *res)
+{
+       struct addrinfo *ai;
+       size_t i;
+
+       f->f_num_addr_fds = 0;
+
+       /* How many sockets do we need? */
+       for (ai = res; ai != NULL; ai = ai->ai_next) {
+               struct socklist *boundsock;
+               int count;
+
+               count = 0;
+               STAILQ_FOREACH(boundsock, &shead, next) {
+                       if (boundsock->sl_ai.ai_family == ai->ai_family)
+                               count++;
+               }
+               if (count == 0)
+                       count = 1;
+               f->f_num_addr_fds += count;
+       }
+
+       f->f_addr_fds = calloc(f->f_num_addr_fds, sizeof(*f->f_addr_fds));
+       f->f_addrs = calloc(f->f_num_addr_fds, sizeof(*f->f_addrs));
+       if (f->f_addr_fds == NULL || f->f_addrs == NULL)
+               err(1, "malloc failed");
+
+       /*
+        * Create our forwarding sockets: for each bound socket
+        * belonging to the destination address, create one socket
+        * connected to the destination and bound to the address of the
+        * listening socket.
+        */
+       i = 0;
+       for (ai = res; ai != NULL; ai = ai->ai_next) {
+               struct socklist *boundsock;
+               int count;
+
+               count = 0;
+               STAILQ_FOREACH(boundsock, &shead, next) {
+                       if (boundsock->sl_ai.ai_family ==
+                           ai->ai_family) {
+                               memcpy(&f->f_addrs[i].raddr, ai->ai_addr,
+                                   ai->ai_addrlen);
+                               memcpy(&f->f_addrs[i].laddr,
+                                   boundsock->sl_ai.ai_addr,
+                                   boundsock->sl_ai.ai_addrlen);
+                               f->f_addr_fds[i++] = make_forw_socket(nvl, ai,
+                                   &boundsock->sl_ai);
+                               count++;
+                       }
+               }
+               if (count == 0) {
+                       memcpy(&f->f_addrs[i].raddr, ai->ai_addr,
+                           ai->ai_addrlen);
+                       f->f_addr_fds[i++] = make_forw_socket(nvl, ai, NULL);
+               }
+       }
+       assert(i == f->f_num_addr_fds);
+}
+
 static void
-parse_action(const char *p, struct filed *f)
+parse_action(const nvlist_t *nvl, const char *p, struct filed *f)
 {
-       struct addrinfo *ai, hints, *res;
-       int error, i;
+       struct addrinfo hints, *res;
+       size_t i;
+       int error;
        const char *q;
        bool syncfile;
 
@@ -3045,27 +3187,7 @@ parse_action(const char *p, struct filed *f)
                        dprintf("%s\n", gai_strerror(error));
                        break;
                }
-
-               for (ai = res; ai != NULL; ai = ai->ai_next)
-                       ++f->f_num_addr_fds;
-
-               f->f_addr_fds = calloc(f->f_num_addr_fds,
-                   sizeof(*f->f_addr_fds));
-               if (f->f_addr_fds == NULL)
-                       err(1, "malloc failed");
-
-               for (ai = res, i = 0; ai != NULL; ai = ai->ai_next, ++i) {
-                       int *sockp = &f->f_addr_fds[i];
-
-                       *sockp = socket(ai->ai_family, ai->ai_socktype, 0);
-                       if (*sockp < 0)
-                               err(1, "socket");
-                       if (connect(*sockp, ai->ai_addr, ai->ai_addrlen) < 0)
-                               err(1, "connect");
-                       /* Make it a write-only socket. */
-                       if (shutdown(*sockp, SHUT_RD) < 0)
-                               err(1, "shutdown");
-               }
+               make_forw_socket_array(nvl, f, res);
                freeaddrinfo(res);
                f->f_type = F_FORW;
                break;
@@ -3161,7 +3283,7 @@ cfline(nvlist_t *nvl, const char *line, const char *prog, 
const char *host,
        /* skip to action part */
        while (*p == '\t' || *p == ' ')
                p++;
-       parse_action(p, &f);
+       parse_action(nvl, p, &f);
 
        /* An nvlist is heap allocated heap here. */
        nvl_filed = filed_to_nvlist(&f);
@@ -3772,6 +3894,13 @@ socksetup(struct addrinfo *ai, const char *name, mode_t 
mode)
                        return (NULL);
                }
 
+               if (setsockopt(s, SOL_SOCKET, SO_REUSEPORT, &(int){1},
+                   sizeof(int)) < 0) {
+                       logerror("setsockopt(SO_REUSEPORT)");
+                       close(s);
+                       return (NULL);
+               }
+
                /*
                 * For AF_LOCAL sockets, the process umask is applied to the
                 * mode set above, so temporarily clear it to ensure that the
diff --git a/usr.sbin/syslogd/syslogd.h b/usr.sbin/syslogd/syslogd.h
index 744465a9cc00..5644dcaf2671 100644
--- a/usr.sbin/syslogd/syslogd.h
+++ b/usr.sbin/syslogd/syslogd.h
@@ -127,6 +127,11 @@ enum f_type {
        F_PIPE,         /* pipe to program */
 };
 
+struct forw_addr {
+       struct sockaddr_storage laddr;
+       struct sockaddr_storage raddr;
+};
+
 /*
  * This structure represents the files that will have log
  * copies printed.
@@ -158,6 +163,7 @@ struct filed {
                        char    f_hname[MAXHOSTNAMELEN];
                        int     *f_addr_fds;
                        size_t  f_num_addr_fds;
+                       struct forw_addr *f_addrs;
                };                              /* F_FORW */
                struct {
                        char    f_pname[MAXPATHLEN];
diff --git a/usr.sbin/syslogd/syslogd_cap_config.c 
b/usr.sbin/syslogd/syslogd_cap_config.c
index a952dbe325a0..0cf6fb67491d 100644
--- a/usr.sbin/syslogd/syslogd_cap_config.c
+++ b/usr.sbin/syslogd/syslogd_cap_config.c
@@ -140,6 +140,8 @@ filed_to_nvlist(const struct filed *filed)
                nvlist_add_string(nvl_filed, "f_hname", filed->f_hname);
                nvlist_add_descriptor_array(nvl_filed, "f_addr_fds",
                    filed->f_addr_fds, filed->f_num_addr_fds);
+               nvlist_add_binary(nvl_filed, "f_addrs", filed->f_addrs,
+                   filed->f_num_addr_fds * sizeof(*filed->f_addrs));
        } else if (filed->f_type == F_PIPE) {
                nvlist_add_string(nvl_filed, "f_pname", filed->f_pname);
                if (filed->f_procdesc >= 0) {
diff --git a/usr.sbin/syslogd/tests/syslogd_test.sh 
b/usr.sbin/syslogd/tests/syslogd_test.sh
index 51c3a5fca1c6..fd3d0c49c080 100644
--- a/usr.sbin/syslogd/tests/syslogd_test.sh
+++ b/usr.sbin/syslogd/tests/syslogd_test.sh
@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: BSD-2-Clause
 #
 # Copyright (c) 2021, 2023 The FreeBSD Foundation
+# Copyright (c) 2024 Mark Johnston <ma...@freebsd.org>
 #
 # This software was developed by Mark Johnston under sponsorship from
 # the FreeBSD Foundation.
@@ -337,6 +338,217 @@ jail_noinet_cleanup()
     jail -r syslogd_noinet
 }
 
+# Create a pair of jails, connected by an epair.  The idea is to run syslogd in
+# one jail (syslogd_allowed_peer), listening on 169.254.0.1, and logger(1) can
+# send messages from the other jail (syslogd_client) using source addrs
+# 169.254.0.2 or 169.254.0.3.
+allowed_peer_test_setup()
+{
+    local epair
+
+    atf_check jail -c name=syslogd_allowed_peer vnet persist
+    atf_check jail -c name=syslogd_client vnet persist
+
+    atf_check -o save:epair ifconfig epair create
+    epair=$(cat epair)
+    epair=${epair%%a}
+
+    atf_check ifconfig ${epair}a vnet syslogd_allowed_peer
+    atf_check ifconfig ${epair}b vnet syslogd_client
+    atf_check jexec syslogd_allowed_peer ifconfig ${epair}a inet 169.254.0.1/16
+    atf_check jexec syslogd_allowed_peer ifconfig lo0 inet 127.0.0.1/8
+    atf_check jexec syslogd_client ifconfig ${epair}b inet 169.254.0.2/16
+    atf_check jexec syslogd_client ifconfig ${epair}b alias 169.254.0.3/16
+    atf_check jexec syslogd_client ifconfig lo0 inet 127.0.0.1/8
+}
+
+allowed_peer_test_cleanup()
+{
+    jail -r syslogd_allowed_peer
+    jail -r syslogd_client
+    ifconfig $(cat epair) destroy
+}
+
+atf_test_case allowed_peer "cleanup"
+allowed_peer_head()
+{
+    atf_set descr "syslogd -a works"
+    atf_set require.user root
+}
+allowed_peer_body()
+{
+    local logfile
+
+    allowed_peer_test_setup
+
+    logfile="${PWD}/jail.log"
+    printf "user.debug\t${logfile}\n" > "${SYSLOGD_CONFIG}"
+    syslogd_start -j syslogd_allowed_peer -b 169.254.0.1:514 -a 
'169.254.0.2/32'
+
+    # Make sure that a message from 169.254.0.2:514 is logged.
+    atf_check jexec syslogd_client \
+        logger -p user.debug -t test1 -h 169.254.0.1 -S 169.254.0.2:514 
"hello, world"
+    atf_check -o match:"test1: hello, world" cat "${logfile}"
+    # ... but not a message from port 515.
+    atf_check -o ignore jexec syslogd_client \
+        logger -p user.debug -t test2 -h 169.254.0.1 -S 169.254.0.2:515 
"hello, world"
+    atf_check -o not-match:"test2: hello, world" cat "${logfile}"
+    atf_check -o ignore jexec syslogd_client \
+        logger -p user.debug -t test2 -h 169.254.0.1 -S 169.254.0.3:515 
"hello, world"
+    atf_check -o not-match:"test2: hello, world" cat "${logfile}"
+
+    syslogd_stop
+
+    # Now make sure that we can filter by port.
+    syslogd_start -j syslogd_allowed_peer -b 169.254.0.1:514 -a 
'169.254.0.2/32:515'
+
+    atf_check jexec syslogd_client \
+        logger -p user.debug -t test3 -h 169.254.0.1 -S 169.254.0.2:514 
"hello, world"
+    atf_check -o not-match:"test3: hello, world" cat "${logfile}"
+    atf_check jexec syslogd_client \
+        logger -p user.debug -t test4 -h 169.254.0.1 -S 169.254.0.2:515 
"hello, world"
+    atf_check -o match:"test4: hello, world" cat "${logfile}"
+
+    syslogd_stop
+}
+allowed_peer_cleanup()
+{
+    allowed_peer_test_cleanup
+}
+
+atf_test_case allowed_peer_forwarding "cleanup"
+allowed_peer_forwarding_head()
+{
+    atf_set descr "syslogd forwards messages from its listening port"
+    atf_set require.user root
+}
+allowed_peer_forwarding_body()
+{
+    local logfile
+
+    allowed_peer_test_setup
+
+    printf "user.debug\t@169.254.0.1\n" > client_config
+    printf "mark.debug\t@169.254.0.1:515\n" >> client_config
+    syslogd_start -j syslogd_client -b 169.254.0.2:514 -f ${PWD}/client_config
+
+    logfile="${PWD}/jail.log"
+    printf "+169.254.0.2\nuser.debug\t${logfile}\n" > "${SYSLOGD_CONFIG}"
+    syslogd_start -j syslogd_allowed_peer -P ${SYSLOGD_PIDFILE}.2 \
+        -b 169.254.0.1:514 -a 169.254.0.2/32
+
+    # A message forwarded to 169.254.0.1:514 should be logged, but one
+    # forwarded to 169.254.0.1:515 should not.
+    atf_check jexec syslogd_client \
+        logger -h 169.254.0.2 -p user.debug -t test1 "hello, world"
+    atf_check jexec syslogd_client \
+        logger -h 169.254.0.2 -p mark.debug -t test2 "hello, world"
+
+    atf_check -o match:"test1: hello, world" cat "${logfile}"
+    atf_check -o not-match:"test2: hello, world" cat "${logfile}"
+}
+allowed_peer_forwarding_cleanup()
+{
+    allowed_peer_test_cleanup
+}
+
+atf_test_case allowed_peer_wildcard "cleanup"
+allowed_peer_wildcard_head()
+{
+    atf_set descr "syslogd -a works with port wildcards"
+    atf_set require.user root
+}
+allowed_peer_wildcard_body()
+{
+    local logfile
+
+    allowed_peer_test_setup
+
+    logfile="${PWD}/jail.log"
+    printf "user.debug\t${logfile}\n" > "${SYSLOGD_CONFIG}"
+    syslogd_start -j syslogd_allowed_peer -b 169.254.0.1:514 -a 
'169.254.0.2/32:*'
+
+    # Make sure that a message from 169.254.0.2:514 is logged.
+    atf_check jexec syslogd_client \
+        logger -p user.debug -t test1 -h 169.254.0.1 -S 169.254.0.2:514 
"hello, world"
+    atf_check -o match:"test1: hello, world" cat "${logfile}"
+    # ... as is a message from 169.254.0.2:515, allowed by the wildcard.
+    atf_check jexec syslogd_client \
+        logger -p user.debug -t test2 -h 169.254.0.1 -S 169.254.0.2:515 
"hello, world"
+    atf_check -o match:"test2: hello, world" cat "${logfile}"
+    # ... but not a message from 169.254.0.3.
+    atf_check -o ignore jexec syslogd_client \
+        logger -p user.debug -t test3 -h 169.254.0.1 -S 169.254.0.3:514 
"hello, world"
+    atf_check -o not-match:"test3: hello, world" cat "${logfile}"
+    atf_check -o ignore jexec syslogd_client \
+        logger -p user.debug -t test3 -h 169.254.0.1 -S 169.254.0.3:515 
"hello, world"
+    atf_check -o not-match:"test3: hello, world" cat "${logfile}"
+
+    syslogd_stop
+}
+allowed_peer_wildcard_cleanup()
+{
+    allowed_peer_test_cleanup
+}
+
+atf_test_case "forward" "cleanup"
+forward_head()
+{
+    atf_set descr "syslogd forwards messages to a remote host"
+    atf_set require.user root
+}
+forward_body()
+{
+    local epair logfile
+
+    atf_check -o save:epair ifconfig epair create
+    epair=$(cat epair)
+    epair=${epair%%a}
+
+    atf_check jail -c name=syslogd_server vnet persist
+    atf_check ifconfig ${epair}a vnet syslogd_server
+    atf_check jexec syslogd_server ifconfig ${epair}a inet 169.254.0.1/16
+    atf_check jexec syslogd_server ifconfig ${epair}a alias 169.254.0.2/16
+    atf_check jexec syslogd_server ifconfig lo0 inet 127.0.0.1/8
+
+    atf_check jail -c name=syslogd_client vnet persist
+    atf_check ifconfig ${epair}b vnet syslogd_client
+    atf_check jexec syslogd_client ifconfig ${epair}b inet 169.254.0.3/16
+    atf_check jexec syslogd_client ifconfig lo0 inet 127.0.0.1/8
+
+    cat <<__EOF__ > ./client_config
+user.debug @169.254.0.1
+mail.debug @169.254.0.2
+ftp.debug @169.254.0.1
+__EOF__
+
+    logfile="${PWD}/jail.log"
+    cat <<__EOF__ > ./server_config
+user.debug ${logfile}
+mail.debug ${logfile}
+ftp.debug ${logfile}
+__EOF__
+
+    syslogd_start -j syslogd_server -f ${PWD}/server_config -b 169.254.0.1 -b 
169.254.0.2
+    syslogd_start -j syslogd_client -f ${PWD}/client_config -P 
${SYSLOGD_PIDFILE}.2
+
+    atf_check jexec syslogd_client \
+        logger -h 169.254.0.3 -P $SYSLOGD_UDP_PORT -p user.debug -t test1 
"hello, world"
+    atf_check jexec syslogd_client \
+        logger -h 169.254.0.3 -P $SYSLOGD_UDP_PORT -p mail.debug -t test2 
"you've got mail"
+    atf_check jexec syslogd_client \
+        logger -h 169.254.0.3 -P $SYSLOGD_UDP_PORT -p ftp.debug -t test3 
"transfer complete"
+
+    atf_check -o match:"test1: hello, world" cat "${logfile}"
+    atf_check -o match:"test2: you've got mail" cat "${logfile}"
+    atf_check -o match:"test3: transfer complete" cat "${logfile}"
+}
+forward_cleanup()
+{
+    jail -r syslogd_server
+    jail -r syslogd_client
+}
+
 atf_init_test_cases()
 {
     atf_add_test_case "unix"
@@ -349,4 +561,8 @@ atf_init_test_cases()
     atf_add_test_case "host_action"
     atf_add_test_case "pipe_action"
     atf_add_test_case "jail_noinet"
+    atf_add_test_case "allowed_peer"
+    atf_add_test_case "allowed_peer_forwarding"
+    atf_add_test_case "allowed_peer_wildcard"
+    atf_add_test_case "forward"
 }

Reply via email to