Hi,
So, it seems we need to more code and test for pf(4) part.
Let me continue this separetely.
On Mon, 14 Sep 2020 11:07:53 +0200
Klemens Nanni <k...@openbsd.org> wrote:
> On Mon, Sep 14, 2020 at 02:09:27PM +0900, YASUOKA Masahiko wrote:
>> Make pfctl check if the rtable really exists when parsing the config.
> I concur, but you can do this with less (duplicated) code.
>
> Instead of copying rdomain_exists() into rtable_exists() with the
> `rti_domainid' check omitted, tweak (and rename) rdomain_exists() into
> returning the information whether the given ID is just an rtable.
>
> rdomain_exists() merges the "invalid id" and "id is an rtable but not
> an rdmomain" cases - make those separate return codes, check/adjust
> existing callers and use it for your new checks.
Yes, I could reduce the code. Thanks,
ok?
Make pfctl check if the rtable really exists when parsing the config.
Index: sbin/pfctl/parse.y
===================================================================
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.701
diff -u -p -r1.701 parse.y
--- sbin/pfctl/parse.y 28 Jan 2020 15:40:35 -0000 1.701
+++ sbin/pfctl/parse.y 16 Sep 2020 09:11:21 -0000
@@ -392,7 +392,9 @@ int invalid_redirect(struct node_host *
u_int16_t parseicmpspec(char *, sa_family_t);
int kw_casecmp(const void *, const void *);
int map_tos(char *string, int *);
+int get_domainid(u_int);
int rdomain_exists(u_int);
+int rtable_exists(u_int);
int filteropts_to_rule(struct pf_rule *, struct filter_opts *);
TAILQ_HEAD(loadanchorshead, loadanchors)
@@ -1217,6 +1219,10 @@ antispoof_opt : LABEL label {
yyerror("invalid rtable id");
YYERROR;
}
+ else if (rtable_exists($2) != 1) {
+ yyerror("rtable %lld does not exist", $2);
+ YYERROR;
+ }
antispoof_opts.rtableid = $2;
}
;
@@ -2001,6 +2007,10 @@ filter_opt : USER uids {
yyerror("invalid rtable id");
YYERROR;
}
+ else if (rtable_exists($2) != 1) {
+ yyerror("rtable %lld does not exist", $2);
+ YYERROR;
+ }
filter_opts.rtableid = $2;
}
| DIVERTTO STRING PORT portplain {
@@ -5868,15 +5878,15 @@ map_tos(char *s, int *val)
}
int
-rdomain_exists(u_int rdomain)
+get_domainid(u_int rdomain)
{
size_t len;
struct rt_tableinfo info;
int mib[6];
- static u_int found[RT_TABLEID_MAX+1];
+ static u_int domainid[RT_TABLEID_MAX+1];
- if (found[rdomain] == 1)
- return 1;
+ if (domainid[rdomain] != 0)
+ return domainid[rdomain];
mib[0] = CTL_NET;
mib[1] = PF_ROUTE;
@@ -5887,17 +5897,37 @@ rdomain_exists(u_int rdomain)
len = sizeof(info);
if (sysctl(mib, 6, &info, &len, NULL, 0) == -1) {
- if (errno == ENOENT) {
+ if (errno == ENOENT)
/* table nonexistent */
- return 0;
- }
- err(1, "%s", __func__);
- }
- if (info.rti_domainid == rdomain) {
- found[rdomain] = 1;
+ domainid[rdomain] = RT_TABLEID_MAX;
+ else
+ err(1, "%s", __func__);
+ } else
+ domainid[rdomain] = info.rti_domainid;
+
+ return domainid[rdomain];
+}
+
+int
+rdomain_exists(u_int rdomain)
+{
+ int domainid;
+
+ domainid = get_domainid(rdomain);
+ if (domainid == rdomain)
return 1;
- }
/* rdomain is a table, but not an rdomain */
+ return 0;
+}
+
+int
+rtable_exists(u_int rtable)
+{
+ int domainid;
+
+ domainid = get_domainid(rtable);
+ if (domainid < RT_TABLEID_MAX)
+ return 1;
return 0;
}
