Bug 291318 - "pfctl -T load" does not work properly
Summary: "pfctl -T load" does not work properly
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 15.0-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-12-01 06:26 UTC by Wes Morgan
Modified: 2025-12-01 21:32 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Wes Morgan 2025-12-01 06:26:12 UTC
The commit base 945ba658d8036102729e3ae7809c56879bf1e259 broke the functionality of the table command "load". It should simply tell PF to load table rules from the specified file, however the commit requires the "-t <table name>" argument to have been given, although this table is correctly ignored later.

Without putting too much thought into it, perhaps the "((tblcmdopt == NULL) ^ (tableopt == NULL))" check should go after the following block instead of before it.
Comment 1 Kristof Provost freebsd_committer freebsd_triage 2025-12-01 09:57:58 UTC
I'm afraid that functionality seems to be a man-page remnant, perhaps from a long-ago import.
This feature doesn't exist in OpenBSD's pf, and there's not actually code for it in our pf either.

Commenting out the check for '-f <tablename>' doesn't actually make it work.
Or rather, it allows the '-f pf.conf' to do what it does, which is load the entire ruleset, not just the table. 

To phrase is differently: the 'regression' here is that we used to just ignore the -t option in this case, and now we (correctly) error out over it.
The real bug is that 'load' is listed in the pfctl man page, and it shouldn't be.
Comment 2 Wes Morgan 2025-12-01 14:09:59 UTC
Are you certain? The option was the "table" analog of the -R, -A, -O, etc options that allow importing specific sections of a config file. In my specific case, this lets me reload everything without flushing the state table, and it does exactly this:

[root@volatile:~#]: pfctl -s Tables
[root@volatile:~#]: pfctl -T load -f /etc/pf.conf      
usage: pfctl [-AdeghMmNnOPqRSrvz] [-a anchor] [-D macro=value] [-F modifier]
        [-f file] [-i interface] [-K host | network]
        [-k host | network | gateway | label | id] [-o level] [-p device]
        [-s modifier] [-t table -T command [address ...]] [-x level]
[root@volatile:~#]: pfctl -t nosuchtable -T load -f /etc/pf.conf
[root@volatile:~#]: pfctl -s Tables                       
badguys
hostile
sshguard
[root@volatile:~#]:
Comment 3 Kristof Provost freebsd_committer freebsd_triage 2025-12-01 14:21:33 UTC
(In reply to Wes Morgan from comment #2)
I am quite certain yes. There's simply no code for "load" in pfctl_table(). Anything it did, it did by accident.
Comment 4 Wes Morgan 2025-12-01 14:45:57 UTC
It seems really odd that the mistake does precisely what is described.

If you look at the code, using -T load (or even -Tl, as in the man page), adds the PFCTL_FLAG_TABLE to loadopts. This flag is checked in several places, including the parser yacc file line 1730:

if (pf->loadopt & PFCTL_FLAG_TABLE)
    if (process_tabledef($3, &$5, pf->opts)) {
        free($3);
        YYERROR;
    }

Line 3347 sets loadopt to ~0 when no flags are specified, ensuring that PFCTL_FLAG_TABLE is set so tables are loaded. There is no other way to specify just loading tables. For anchors, loadopt has the flag added on line 3375, although that looks like it might be buggy because it forces loading an anchor to load rules, tables, etc ignoring what was specified on the command line.

How does one just load tables in a ruleset otherwise?
Comment 5 Kristof Provost freebsd_committer freebsd_triage 2025-12-01 15:21:33 UTC
(In reply to Wes Morgan from comment #4)
Hmm. Indeed. That's not handled by pfctl_table() as I'd have expected, but directly in the main function.

I was sure I'd actually tested that, and found that pfctl -Tload -f pf.conf actually loaded the rules too, but I can't reproduce that now.

Expect a fix soon then.
Comment 6 Wes Morgan 2025-12-01 15:46:27 UTC
Thanks. It appears the mismatch is that OpenBSD at some point removed all options to load specific portions of the rule file, probably in favor of saving state to a temporary file and reloading it.

If only the mad genius who wrote this originally had documented the code a little more clearly.

And if he had not arbitrarily capitalized some of the other modifiers like "Tables" instead of "tables", "Reset" and "Sources". Gets me every time. "hmm, was it Rules or rules? doh, it's rules".
Comment 7 commit-hook freebsd_committer freebsd_triage 2025-12-01 21:32:02 UTC
A commit in branch main references this bug:

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

commit 7a283c40188ff7b0a4bae1a47bbd9ecc17ded132
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2025-12-01 15:05:09 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2025-12-01 21:30:30 +0000

    pfctl: restore '-Tload -f pf.conf' functionality

    Allow only tables to be loaded from a file, rather than everything (i.e.
    including options, rules).

    Add a test case for this.

    PR:             291318
    MFC after:      1 week
    Sponsored by:   Rubicon Communications, LLC ("Netgate")

 sbin/pfctl/pfctl.c            |  3 ++-
 tests/sys/netpfil/pf/table.sh | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)