[Bug 296958] authpf(8): read_config() never strips trailing whitespace, silently misconfiguring the gateway

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=296958

            Bug ID: 296958
           Summary: authpf(8): read_config() never strips trailing
                    whitespace, silently misconfiguring the gateway
           Product: Base System
           Version: 14.4-RELEASE
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: bin
          Assignee: bugs@FreeBSD.org
          Reporter: avinash.duduskar@gmail.com
                CC: kp@freebsd.org

authpf's read_config() tries to strip trailing whitespace from an
authpf.conf value, but the loop begins at the NUL terminator, so it
never runs and the whitespace is kept:

        tp = pair[1] + strlen(pair[1]);
        while ((*tp == ' ' || *tp == '\t') && tp >= pair[1])
                *tp-- = '\0';

A line like "anchor=authpf " then loads the session's rules under the
anchor "authpf ", which the "authpf/*" anchor in pf.conf never matches.
A line like "table=authpf_users " asks the kernel to add the address to
a table that does not exist: DIOCRADDADDRS returns ESRCH, change_table()
exempts ESRCH from its error path, and the address is added nowhere.
Either way authpf tells the user they are authenticated and then passes
none of the traffic it was supposed to allow, and no error is logged.

Fix: start at the last character, and test the bound before
dereferencing.

This is the same fix committed to OpenBSD as 2d12a8e44d82 ("authpf(8)
read_config() should chop off trailing white space").  The three lines
are identical in FreeBSD's contrib/pf/authpf/authpf.c on stable/14 and
main, and change_table()'s ESRCH handling matches, so the behaviour is
the same on 14.x.  (Note: on 15.0-RELEASE and later authpf is already
broken by the pf netlink conversion, bug 291981, which is unrelated to
this parser fix.)

Demonstrated on FreeBSD 14.4-RELEASE: with a trailing space after
"table=authpf_users" an authenticated session leaves <authpf_users>
empty and its traffic blocked; the patched binary makes it behave like
the space-free config; reverting the diff brings the failure back.

Patch attached; applies cleanly to stable/14 and main.

-- 
You are receiving this mail because:
You are the assignee for the bug.