16 января 2020 г. 9:14:43 GMT+03:00, Theo de Raadt <dera...@openbsd.org> пишет:
>I'll bite, using text from your regress.
>
>> +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
>
>What does 1234:12345 mean. It must be uid 1234 _and_ gid 12345?
The manpage for "user"and "group" clauses say they have similar syntax with
"port". I was surprised when tried to use "X:Y" notation in "user"clause,
though.
>I don't quite understand, what is the purpose to this precise check?
>How often does this kind of precise request happen?
>
>i would expect the normal pattern is to pass "for this uid" or "for
>this gid", but I'm surprised to see "for this uid AND this gid, both
>must match", and I doubt you are representing "either this uid or this
>gid".
>
>Can you explain the use case?
I have a server with shell accounts for students. Their uids are in, say, 2000
to 5999 range. I want to allow all of them to connect e.g. to Github or
anoncvs, but not to whole Internet.
I can use "1999><6000" as alternative, of course, but then manual must be
tweaked, IMO.
>Vadim Zhukov <persg...@gmail.com> wrote:
>
>> 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
>>
--
With best regards,
Vadim Zhukov
