Hi all.
I've just found that pfctl doesn't like 'X:Y' syntax for user and group
clauses, despite of the words in manpage. The problem is caused by
parser eating the colon character in STRING version of "uid" and "gid"
rules. The solution is similar to the way ports parsing is done. Now we
have "uidrange" and "gidrange" rules, similar to "portrange". I didn't
try to unify those two (or even three) to avoid increasing parentheses
madness, but if somebody would come with better diff/idea, I'm open. :)
This diff also adds a regression test, for the sake of completeness.
Existing regression tests pass on amd64.
OK?
--
WBR,
Vadim Zhukov
Index: sbin/pfctl/parse.y
===================================================================
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.699
diff -u -p -r1.699 parse.y
--- sbin/pfctl/parse.y 17 Oct 2019 21:54:28 -0000 1.699
+++ sbin/pfctl/parse.y 16 Jan 2020 00:44:14 -0000
@@ -468,6 +468,7 @@ typedef struct {
#define PPORT_RANGE 1
#define PPORT_STAR 2
int parseport(char *, struct range *r, int);
+int parse_ugid(char *, struct range *r, int);
#define DYNIF_MULTIADDR(addr) ((addr).type == PF_ADDR_DYNIFTL && \
(!((addr).iflags & PFI_AFLAG_NOALIAS) || \
@@ -496,7 +497,7 @@ int parseport(char *, struct range *r, i
%token <v.number> NUMBER
%token <v.i> PORTBINARY
%type <v.interface> interface if_list if_item_not if_item
-%type <v.number> number icmptype icmp6type uid gid
+%type <v.number> number icmptype icmp6type
%type <v.number> tos not yesno optnodf
%type <v.probability> probability
%type <v.weight> optweight
@@ -504,7 +505,7 @@ int parseport(char *, struct range *r, i
%type <v.i> sourcetrack flush unaryop statelock
%type <v.b> action
%type <v.b> flags flag blockspec prio
-%type <v.range> portplain portstar portrange
+%type <v.range> portplain portstar portrange uidrange gidrange
%type <v.hashkey> hashkey
%type <v.proto> proto proto_list proto_item
%type <v.number> protoval
@@ -2957,69 +2958,67 @@ uid_list : uid_item optnl { $$ =
$1; }
}
;
-uid_item : uid {
+uid_item : uidrange {
$$ = calloc(1, sizeof(struct node_uid));
if ($$ == NULL)
err(1, "uid_item: calloc");
- $$->uid[0] = $1;
- $$->uid[1] = $1;
- $$->op = PF_OP_EQ;
+ $$->uid[0] = $1.a;
+ $$->uid[1] = $1.b;
+ if ($1.t)
+ $$->op = PF_OP_RRG;
+ else
+ $$->op = PF_OP_EQ;
$$->next = NULL;
$$->tail = $$;
}
- | unaryop uid {
- if ($2 == -1 && $1 != PF_OP_EQ && $1 != PF_OP_NE) {
+ | unaryop uidrange {
+ if ($2.a == -1 && $1 != PF_OP_EQ && $1 != PF_OP_NE) {
yyerror("user unknown requires operator = or "
"!=");
YYERROR;
}
+ if ($2.t) {
+ yyerror("':' cannot be used with an other "
+ "user operator");
+ YYERROR;
+ }
$$ = calloc(1, sizeof(struct node_uid));
if ($$ == NULL)
err(1, "uid_item: calloc");
- $$->uid[0] = $2;
- $$->uid[1] = $2;
+ $$->uid[0] = $2.a;
+ $$->uid[1] = $2.b;
$$->op = $1;
$$->next = NULL;
$$->tail = $$;
}
- | uid PORTBINARY uid {
- if ($1 == -1 || $3 == -1) {
+ | uidrange PORTBINARY uidrange {
+ if ($1.a == -1 || $3.a == -1) {
yyerror("user unknown requires operator = or "
"!=");
YYERROR;
}
+ if ($1.t || $3.t) {
+ yyerror("':' cannot be used with an other "
+ "user operator");
+ YYERROR;
+ }
$$ = calloc(1, sizeof(struct node_uid));
if ($$ == NULL)
err(1, "uid_item: calloc");
- $$->uid[0] = $1;
- $$->uid[1] = $3;
+ $$->uid[0] = $1.a;
+ $$->uid[1] = $3.a;
$$->op = $2;
$$->next = NULL;
$$->tail = $$;
}
;
-uid : STRING {
- if (!strcmp($1, "unknown"))
- $$ = -1;
- else {
- uid_t uid;
-
- if (uid_from_user($1, &uid) == -1) {
- yyerror("unknown user %s", $1);
- free($1);
- YYERROR;
- }
- $$ = uid;
- }
- free($1);
- }
- | NUMBER {
- if ($1 < 0 || $1 >= UID_MAX) {
- yyerror("illegal uid value %lld", $1);
+uidrange : numberstring {
+ if (parse_ugid($1, &$$, 1) == -1) {
+ free($1);
YYERROR;
}
- $$ = $1;
+ free($1);
}
;
@@ -3035,69 +3034,67 @@ gid_list : gid_item optnl { $$ =
$1; }
}
;
-gid_item : gid {
+gid_item : gidrange {
$$ = calloc(1, sizeof(struct node_gid));
if ($$ == NULL)
err(1, "gid_item: calloc");
- $$->gid[0] = $1;
- $$->gid[1] = $1;
- $$->op = PF_OP_EQ;
+ $$->gid[0] = $1.a;
+ $$->gid[1] = $1.b;
+ if ($1.t)
+ $$->op = PF_OP_RRG;
+ else
+ $$->op = PF_OP_EQ;
$$->next = NULL;
$$->tail = $$;
}
- | unaryop gid {
- if ($2 == -1 && $1 != PF_OP_EQ && $1 != PF_OP_NE) {
+ | unaryop gidrange {
+ if ($2.a == -1 && $1 != PF_OP_EQ && $1 != PF_OP_NE) {
yyerror("group unknown requires operator = or "
"!=");
YYERROR;
}
+ if ($2.t) {
+ yyerror("':' cannot be used with an other "
+ "group operator");
+ YYERROR;
+ }
$$ = calloc(1, sizeof(struct node_gid));
if ($$ == NULL)
err(1, "gid_item: calloc");
- $$->gid[0] = $2;
- $$->gid[1] = $2;
+ $$->gid[0] = $2.a;
+ $$->gid[1] = $2.b;
$$->op = $1;
$$->next = NULL;
$$->tail = $$;
}
- | gid PORTBINARY gid {
- if ($1 == -1 || $3 == -1) {
+ | gidrange PORTBINARY gidrange {
+ if ($1.a == -1 || $3.a == -1) {
yyerror("group unknown requires operator = or "
"!=");
YYERROR;
}
+ if ($1.t || $3.t) {
+ yyerror("':' cannot be used with an other "
+ "group operator");
+ YYERROR;
+ }
$$ = calloc(1, sizeof(struct node_gid));
if ($$ == NULL)
err(1, "gid_item: calloc");
- $$->gid[0] = $1;
- $$->gid[1] = $3;
+ $$->gid[0] = $1.a;
+ $$->gid[1] = $3.a;
$$->op = $2;
$$->next = NULL;
$$->tail = $$;
}
;
-gid : STRING {
- if (!strcmp($1, "unknown"))
- $$ = -1;
- else {
- gid_t gid;
-
- if (gid_from_group($1, &gid) == -1) {
- yyerror("unknown group %s", $1);
- free($1);
- YYERROR;
- }
- $$ = gid;
- }
- free($1);
- }
- | NUMBER {
- if ($1 < 0 || $1 >= GID_MAX) {
- yyerror("illegal gid value %lld", $1);
+gidrange : numberstring {
+ if (parse_ugid($1, &$$, 0) == -1) {
+ free($1);
YYERROR;
}
- $$ = $1;
+ free($1);
}
;
@@ -5802,6 +5799,56 @@ parseport(char *port, struct range *r, i
}
yyerror("port is invalid: %s", port);
return (-1);
+}
+
+int
+parse_ugid(char *spec, struct range *r, int is_uid)
+{
+ uid_t id;
+ const char *errstr;
+ char *p;
+
+ if ((p = strchr(spec, ':')) != NULL)
+ *p++ = '\0';
+
+ id = strtonum(spec, 0, is_uid ? UID_MAX : GID_MAX, &errstr);
+ if (errstr) {
+ if (strcmp("unknown", spec) == 0)
+ id = -1;
+ else if (is_uid && uid_from_user(spec, &id) == -1)
+ return -1;
+ else if (!is_uid && gid_from_group(spec, &id) == -1)
+ return -1;
+ }
+ r->a = (int)id;
+
+ if (p == NULL) {
+ r->b = 0;
+ r->t = PF_OP_NONE;
+ } else {
+ id = strtonum(p, 0, is_uid ? UID_MAX : GID_MAX, &errstr);
+ if (errstr) {
+ if (strcmp("unknown", spec) == 0)
+ id = -1;
+ else if (is_uid && uid_from_user(spec, &id) == -1)
+ goto range_fail;
+ else if (!is_uid && gid_from_group(spec, &id) == -1)
+ goto range_fail;
+ }
+
+ if (id == r->a) {
+ r->b = 0;
+ r->t = PF_OP_NONE;
+ } else {
+ r->b = (int)id;
+ r->t = PF_OP_RRG;
+ }
+ }
+ return 0;
+
+range_fail:
+ *p = ':';
+ return -1;
}
int
Index: regress/sbin/pfctl/Makefile
===================================================================
RCS file: /cvs/src/regress/sbin/pfctl/Makefile,v
retrieving revision 1.231
diff -u -p -r1.231 Makefile
--- regress/sbin/pfctl/Makefile 12 Dec 2017 19:49:19 -0000 1.231
+++ regress/sbin/pfctl/Makefile 16 Jan 2020 00:44:14 -0000
@@ -17,7 +17,7 @@ PFTESTS=1 2 3 4 5 6 7 8 9 10 11 12 13 14
PFTESTS+=28 29 30 31 32 34 35 36 38 39 40 41 44 46 47 48 49 50
PFTESTS+=52 53 54 55 56 57 60 61 65 66 67 68 69 70 71 72 73
PFTESTS+=74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96
-PFTESTS+=97 98 99 100 101 102 103 104 105 106 107 108 109 110 111
+PFTESTS+=97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 114
PFFAIL=1 2 3 4 5 6 7 8 11 12 13 14 15 16 17 19 20 23 25 27
PFFAIL+=30 37 38 39 40 41 42 43 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62
PFFAIL+=63 64 65 66 67
Index: regress/sbin/pfctl/pf114.in
===================================================================
RCS file: regress/sbin/pfctl/pf114.in
diff -N regress/sbin/pfctl/pf114.in
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ regress/sbin/pfctl/pf114.in 16 Jan 2020 00:44:14 -0000
@@ -0,0 +1,5 @@
+# check that X:Y syntax works for users and groups
+pass out proto tcp all user 1234:12345
+pass out proto tcp all user root:12345
+pass out proto tcp all group 1234:12345
+pass out proto tcp all group wheel:12345
Index: regress/sbin/pfctl/pf114.ok
===================================================================
RCS file: regress/sbin/pfctl/pf114.ok
diff -N regress/sbin/pfctl/pf114.ok
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ regress/sbin/pfctl/pf114.ok 16 Jan 2020 00:44:14 -0000
@@ -0,0 +1,4 @@
+pass out proto tcp all user 1234:12345 flags S/SA
+pass out proto tcp all user 0:12345 flags S/SA
+pass out proto tcp all group 1234:12345 flags S/SA
+pass out proto tcp all group 0:12345 flags S/SA
