From 375aaa299f85a66cee808490c31809db9e890a68 Mon Sep 17 00:00:00 2001 From: Kristof Provost Date: Fri, 26 Jul 2024 21:03:54 +0200 Subject: [PATCH] pfctl: improve error reporting libpfctl doesn't set errno, instead it returns error codes. Take that into account when handling errors so that we report the actual error. Sponsored by: Rubicon Communications, LLC ("Netgate") --- sbin/pfctl/pfctl.c | 140 +++++++++++++++++++++++++-------------------- 1 file changed, 77 insertions(+), 63 deletions(-) diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c index 39c6d684a317..b60e64fba338 100644 --- a/sbin/pfctl/pfctl.c +++ b/sbin/pfctl/pfctl.c @@ -319,7 +319,7 @@ pfctl_enable(int dev, int opts) else if (ret == ESRCH) errx(1, "pfil registeration failed"); else - err(1, "DIOCSTART"); + errc(1, ret, "DIOCSTART"); } if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "pf enabled\n"); @@ -340,7 +340,7 @@ pfctl_disable(int dev, int opts) if (ret == ENOENT) errx(1, "pf not enabled"); else - err(1, "DIOCSTOP"); + errc(1, ret, "DIOCSTOP"); } if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "pf disabled\n"); @@ -355,8 +355,9 @@ pfctl_disable(int dev, int opts) int pfctl_clear_stats(struct pfctl_handle *h, int opts) { - if (pfctl_clear_status(h)) - err(1, "DIOCCLRSTATUS"); + int ret; + if ((ret = pfctl_clear_status(h)) != 0) + errc(1, ret, "DIOCCLRSTATUS"); if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "pf: statistics cleared\n"); return (0); @@ -536,6 +537,7 @@ pfctl_clear_iface_states(int dev, const char *iface, int opts) { struct pfctl_kill kill; unsigned int killed; + int ret; memset(&kill, 0, sizeof(kill)); if (iface != NULL && strlcpy(kill.ifname, iface, @@ -545,8 +547,8 @@ pfctl_clear_iface_states(int dev, const char *iface, int opts) if (opts & PF_OPT_KILLMATCH) kill.kill_match = true; - if (pfctl_clear_states_h(pfh, &kill, &killed)) - err(1, "DIOCCLRSTATES"); + if ((ret = pfctl_clear_states_h(pfh, &kill, &killed)) != 0) + errc(1, ret, "DIOCCLRSTATES"); if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "%d states cleared\n", killed); return (0); @@ -713,7 +715,7 @@ pfctl_net_kill_states(int dev, const char *iface, int opts) struct sockaddr last_src, last_dst; unsigned int newkilled; int killed, sources, dests; - int ret_ga; + int ret_ga, ret; killed = sources = dests = 0; @@ -801,14 +803,14 @@ pfctl_net_kill_states(int dev, const char *iface, int opts) errx(1, "Unknown address family %d", kill.af); - if (pfctl_kill_states_h(pfh, &kill, &newkilled)) - err(1, "DIOCKILLSTATES"); + if ((ret = pfctl_kill_states_h(pfh, &kill, &newkilled)) != 0) + errc(1, ret, "DIOCKILLSTATES"); killed += newkilled; } freeaddrinfo(res[1]); } else { - if (pfctl_kill_states_h(pfh, &kill, &newkilled)) - err(1, "DIOCKILLSTATES"); + if ((ret = pfctl_kill_states_h(pfh, &kill, &newkilled)) != 0) + errc(1, ret, "DIOCKILLSTATES"); killed += newkilled; } } @@ -890,6 +892,7 @@ pfctl_label_kill_states(int dev, const char *iface, int opts) { struct pfctl_kill kill; unsigned int killed; + int ret; if (state_killers != 2 || (strlen(state_kill[1]) == 0)) { warnx("no label specified"); @@ -907,8 +910,8 @@ pfctl_label_kill_states(int dev, const char *iface, int opts) sizeof(kill.label)) errx(1, "label too long: %s", state_kill[1]); - if (pfctl_kill_states_h(pfh, &kill, &killed)) - err(1, "DIOCKILLSTATES"); + if ((ret = pfctl_kill_states_h(pfh, &kill, &killed)) != 0) + errc(1, ret, "DIOCKILLSTATES"); if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "killed %d states\n", killed); @@ -921,6 +924,7 @@ pfctl_id_kill_states(int dev, const char *iface, int opts) { struct pfctl_kill kill; unsigned int killed; + int ret; if (state_killers != 2 || (strlen(state_kill[1]) == 0)) { warnx("no id specified"); @@ -946,8 +950,8 @@ pfctl_id_kill_states(int dev, const char *iface, int opts) usage(); } - if (pfctl_kill_states_h(pfh, &kill, &killed)) - err(1, "DIOCKILLSTATES"); + if ((ret = pfctl_kill_states_h(pfh, &kill, &killed)) != 0) + errc(1, ret, "DIOCKILLSTATES"); if ((opts & PF_OPT_QUIET) == 0) fprintf(stderr, "killed %d states\n", killed); @@ -962,17 +966,18 @@ pfctl_get_pool(int dev, struct pfctl_pool *pool, u_int32_t nr, struct pfioc_pooladdr pp; struct pf_pooladdr *pa; u_int32_t pnr, mpnr; + int ret; memset(&pp, 0, sizeof(pp)); - if (pfctl_get_addrs(pfh, ticket, nr, r_action, anchorname, &mpnr) != 0) { - warn("DIOCGETADDRS"); + if ((ret = pfctl_get_addrs(pfh, ticket, nr, r_action, anchorname, &mpnr)) != 0) { + warnc(ret, "DIOCGETADDRS"); return (-1); } TAILQ_INIT(&pool->list); for (pnr = 0; pnr < mpnr; ++pnr) { - if (pfctl_get_addr(pfh, ticket, nr, r_action, anchorname, pnr, &pp) != 0) { - warn("DIOCGETADDR"); + if ((ret = pfctl_get_addr(pfh, ticket, nr, r_action, anchorname, pnr, &pp)) != 0) { + warnc(ret, "DIOCGETADDR"); return (-1); } pa = calloc(1, sizeof(struct pf_pooladdr)); @@ -1102,6 +1107,7 @@ pfctl_show_eth_rules(int dev, char *path, int opts, enum pfctl_show format, int brace; int dotitle = opts & PF_OPT_SHOWALL; int len = strlen(path); + int ret; char *npath, *p; /* @@ -1134,12 +1140,12 @@ pfctl_show_eth_rules(int dev, char *path, int opts, enum pfctl_show format, struct pfctl_eth_rulesets_info ri; u_int32_t mnr, nr; - if (pfctl_get_eth_rulesets_info(dev, &ri, npath)) { - if (errno == EINVAL) { + if ((ret = pfctl_get_eth_rulesets_info(dev, &ri, npath)) != 0) { + if (ret == EINVAL) { fprintf(stderr, "Anchor '%s' " "not found.\n", anchorname); } else { - warn("DIOCGETETHRULESETS"); + warnc(ret, "DIOCGETETHRULESETS"); return (-1); } } @@ -1149,8 +1155,8 @@ pfctl_show_eth_rules(int dev, char *path, int opts, enum pfctl_show format, for (nr = 0; nr < mnr; ++nr) { struct pfctl_eth_ruleset_info rs; - if (pfctl_get_eth_ruleset(dev, npath, nr, &rs)) - err(1, "DIOCGETETHRULESET"); + if ((ret = pfctl_get_eth_ruleset(dev, npath, nr, &rs)) != 0) + errc(1, ret, "DIOCGETETHRULESET"); INDENT(depth, !(opts & PF_OPT_VERBOSE)); printf("anchor \"%s\" all {\n", rs.name); pfctl_show_eth_rules(dev, npath, opts, @@ -1162,16 +1168,16 @@ pfctl_show_eth_rules(int dev, char *path, int opts, enum pfctl_show format, return (0); } - if (pfctl_get_eth_rules_info(dev, &info, path)) { - warn("DIOCGETETHRULES"); + if ((ret = pfctl_get_eth_rules_info(dev, &info, path)) != 0) { + warnc(ret, "DIOCGETETHRULES"); return (-1); } for (int nr = 0; nr < info.nr; nr++) { brace = 0; INDENT(depth, !(opts & PF_OPT_VERBOSE)); - if (pfctl_get_eth_rule(dev, nr, info.ticket, path, &rule, - opts & PF_OPT_CLRRULECTRS, anchor_call) != 0) { - warn("DIOCGETETHRULE"); + if ((ret = pfctl_get_eth_rule(dev, nr, info.ticket, path, &rule, + opts & PF_OPT_CLRRULECTRS, anchor_call)) != 0) { + warnc(ret, "DIOCGETETHRULE"); return (-1); } if (anchor_call[0] && @@ -1280,14 +1286,14 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, if (opts & PF_OPT_SHOWALL) { ret = pfctl_get_rules_info_h(pfh, &ri, PF_PASS, path); if (ret != 0) { - warn("DIOCGETRULES"); + warnc(ret, "DIOCGETRULES"); goto error; } header++; } ret = pfctl_get_rules_info_h(pfh, &ri, PF_SCRUB, path); if (ret != 0) { - warn("DIOCGETRULES"); + warnc(ret, "DIOCGETRULES"); goto error; } if (opts & PF_OPT_SHOWALL) { @@ -1298,9 +1304,9 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, } for (nr = 0; nr < ri.nr; ++nr) { - if (pfctl_get_clear_rule_h(pfh, nr, ri.ticket, path, PF_SCRUB, - &rule, anchor_call, opts & PF_OPT_CLRRULECTRS)) { - warn("DIOCGETRULENV"); + if ((ret = pfctl_get_clear_rule_h(pfh, nr, ri.ticket, path, PF_SCRUB, + &rule, anchor_call, opts & PF_OPT_CLRRULECTRS)) != 0) { + warnc(ret, "DIOCGETRULENV"); goto error; } @@ -1325,13 +1331,13 @@ pfctl_show_rules(int dev, char *path, int opts, enum pfctl_show format, } ret = pfctl_get_rules_info_h(pfh, &ri, PF_PASS, path); if (ret != 0) { - warn("DIOCGETRULES"); + warnc(ret, "DIOCGETRULES"); goto error; } for (nr = 0; nr < ri.nr; ++nr) { - if (pfctl_get_clear_rule_h(pfh, nr, ri.ticket, path, PF_PASS, - &rule, anchor_call, opts & PF_OPT_CLRRULECTRS)) { - warn("DIOCGETRULE"); + if ((ret = pfctl_get_clear_rule_h(pfh, nr, ri.ticket, path, PF_PASS, + &rule, anchor_call, opts & PF_OPT_CLRRULECTRS)) != 0) { + warnc(ret, "DIOCGETRULE"); goto error; } @@ -1484,15 +1490,15 @@ pfctl_show_nat(int dev, char *path, int opts, char *anchorname, int depth, for (i = 0; i < 3; i++) { ret = pfctl_get_rules_info_h(pfh, &ri, nattype[i], path); if (ret != 0) { - warn("DIOCGETRULES"); + warnc(ret, "DIOCGETRULES"); return (-1); } for (nr = 0; nr < ri.nr; ++nr) { INDENT(depth, !(opts & PF_OPT_VERBOSE)); - if (pfctl_get_rule_h(pfh, nr, ri.ticket, path, - nattype[i], &rule, anchor_call)) { - warn("DIOCGETRULE"); + if ((ret = pfctl_get_rule_h(pfh, nr, ri.ticket, path, + nattype[i], &rule, anchor_call)) != 0) { + warnc(ret, "DIOCGETRULE"); return (-1); } if (pfctl_get_pool(dev, &rule.rpool, nr, @@ -1613,14 +1619,15 @@ pfctl_show_status(int dev, int opts) { struct pfctl_status *status; struct pfctl_syncookies cookies; + int ret; if ((status = pfctl_get_status_h(pfh)) == NULL) { warn("DIOCGETSTATUS"); return (-1); } - if (pfctl_get_syncookies(dev, &cookies)) { + if ((ret = pfctl_get_syncookies(dev, &cookies)) != 0) { pfctl_free_status(status); - warn("DIOCGETSYNCOOKIES"); + warnc(ret, "DIOCGETSYNCOOKIES"); return (-1); } if (opts & PF_OPT_SHOWALL) @@ -1653,12 +1660,13 @@ pfctl_show_timeouts(int dev, int opts) { uint32_t seconds; int i; + int ret; if (opts & PF_OPT_SHOWALL) pfctl_print_title("TIMEOUTS:"); for (i = 0; pf_timeouts[i].name; i++) { - if (pfctl_get_timeout(pfh, pf_timeouts[i].timeout, &seconds)) - err(1, "DIOCGETTIMEOUT"); + if ((ret = pfctl_get_timeout(pfh, pf_timeouts[i].timeout, &seconds)) != 0) + errc(1, ret, "DIOCGETTIMEOUT"); printf("%-20s %10d", pf_timeouts[i].name, seconds); if (pf_timeouts[i].timeout >= PFTM_ADAPTIVE_START && pf_timeouts[i].timeout <= PFTM_ADAPTIVE_END) @@ -1676,12 +1684,13 @@ pfctl_show_limits(int dev, int opts) { unsigned int limit; int i; + int ret; if (opts & PF_OPT_SHOWALL) pfctl_print_title("LIMITS:"); for (i = 0; pf_limits[i].name; i++) { - if (pfctl_get_limit(pfh, pf_limits[i].index, &limit)) - err(1, "DIOCGETLIMIT"); + if ((ret = pfctl_get_limit(pfh, pf_limits[i].index, &limit)) != 0) + errc(1, ret, "DIOCGETLIMIT"); printf("%-13s ", pf_limits[i].name); if (limit == UINT_MAX) printf("unlimited\n"); @@ -1712,18 +1721,19 @@ int pfctl_add_pool(struct pfctl *pf, struct pfctl_pool *p, sa_family_t af) { struct pf_pooladdr *pa; + int ret; if ((pf->opts & PF_OPT_NOACTION) == 0) { - if (pfctl_begin_addrs(pf->h, &pf->paddr.ticket)) - err(1, "DIOCBEGINADDRS"); + if ((ret = pfctl_begin_addrs(pf->h, &pf->paddr.ticket)) != 0) + errc(1, ret, "DIOCBEGINADDRS"); } pf->paddr.af = af; TAILQ_FOREACH(pa, &p->list, entries) { memcpy(&pf->paddr.addr, pa, sizeof(struct pf_pooladdr)); if ((pf->opts & PF_OPT_NOACTION) == 0) { - if (pfctl_add_addr(pf->h, &pf->paddr) != 0) - err(1, "DIOCADDADDR"); + if ((ret = pfctl_add_addr(pf->h, &pf->paddr)) != 0) + errc(1, ret, "DIOCADDADDR"); } } return (0); @@ -1932,6 +1942,7 @@ pfctl_load_eth_rule(struct pfctl *pf, char *path, struct pfctl_eth_rule *r, char *name; char anchor[PF_ANCHOR_NAME_SIZE]; int len = strlen(path); + int ret; if (strlcpy(anchor, path, sizeof(anchor)) >= sizeof(anchor)) errx(1, "pfctl_load_eth_rule: strlcpy"); @@ -1951,9 +1962,9 @@ pfctl_load_eth_rule(struct pfctl *pf, char *path, struct pfctl_eth_rule *r, name = ""; if ((pf->opts & PF_OPT_NOACTION) == 0) - if (pfctl_add_eth_rule(pf->dev, r, anchor, name, - pf->eth_ticket)) - err(1, "DIOCADDETHRULENV"); + if ((ret = pfctl_add_eth_rule(pf->dev, r, anchor, name, + pf->eth_ticket)) != 0) + errc(1, ret, "DIOCADDETHRULENV"); if (pf->opts & PF_OPT_VERBOSE) { INDENT(depth, !(pf->opts & PF_OPT_VERBOSE2)); @@ -2078,7 +2089,7 @@ pfctl_load_rule(struct pfctl *pf, char *path, struct pfctl_rule *r, int depth) was_present = true; break; default: - err(1, "DIOCADDRULENV"); + errc(1, error, "DIOCADDRULE"); } } @@ -2679,6 +2690,7 @@ int pfctl_do_set_debug(struct pfctl *pf, char *d) { u_int32_t level; + int ret; if ((loadopt & PFCTL_FLAG_OPTION) == 0) return (0); @@ -2700,8 +2712,8 @@ pfctl_do_set_debug(struct pfctl *pf, char *d) level = pf->debug; if ((pf->opts & PF_OPT_NOACTION) == 0) - if (pfctl_set_debug(pfh, level)) - err(1, "DIOCSETDEBUG"); + if ((ret = pfctl_set_debug(pfh, level)) != 0) + errc(1, ret, "DIOCSETDEBUG"); if (pf->opts & PF_OPT_VERBOSE) printf("set debug %s\n", d); @@ -2758,8 +2770,10 @@ pfctl_set_interface_flags(struct pfctl *pf, char *ifname, int flags, int how) void pfctl_debug(int dev, u_int32_t level, int opts) { - if (pfctl_set_debug(pfh, level)) - err(1, "DIOCSETDEBUG"); + int ret; + + if ((ret = pfctl_set_debug(pfh, level)) != 0) + errc(1, ret, "DIOCSETDEBUG"); if ((opts & PF_OPT_QUIET) == 0) { fprintf(stderr, "debug level set to '"); switch (level) { @@ -2852,15 +2866,15 @@ pfctl_show_eth_anchors(int dev, int opts, char *anchorname) fprintf(stderr, "Anchor '%s' not found.\n", anchorname); else - err(1, "DIOCGETETHRULESETS"); + errc(1, ret, "DIOCGETETHRULESETS"); return (-1); } for (int nr = 0; nr < ri.nr; nr++) { char sub[MAXPATHLEN]; - if (pfctl_get_eth_ruleset(dev, anchorname, nr, &rs) != 0) - err(1, "DIOCGETETHRULESET"); + if ((ret = pfctl_get_eth_ruleset(dev, anchorname, nr, &rs)) != 0) + errc(1, ret, "DIOCGETETHRULESET"); if (!strcmp(rs.name, PF_RESERVED_ANCHOR)) continue;