Re: Unbreak X:Y user/group spec in pf.conf
On Thu, Jan 16, 2020 at 06:56:06PM +0100, Alexandr Nedvedicky wrote: > I like your suggestion, diff below fixes extra white space and > uses Stuart's wording. sure
Re: Unbreak X:Y user/group spec in pf.conf
Hello,
> > +.Pp
> > +Note that users 1000 and 1500 are excluded from the pass rule.
>
> The last line above is a little hard to parse - I think a "positive
> example" would be clearer, i.e. something like this:
>
> .Pp
> The example below permits users with uid between 1000 and 1500
> to open connections:
> .Bd -literal -offset indent
> block out proto tcp all
> pass out proto tcp from self user { 999 >< 1501 }
> .Ed
> .Pp
> The
> .Sq \&:
> operator, which works for port number matching, does not work for
> [...]
>
I like your suggestion, diff below fixes extra white space and
uses Stuart's wording.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
index 452a15d1cfd..f847aa7fe32 100644
--- a/share/man/man5/pf.conf.5
+++ b/share/man/man5/pf.conf.5
@@ -820,6 +820,21 @@ connections:
block out proto tcp all
pass out proto tcp from self user { < 1000, dhartmei }
.Ed
+.Pp
+The example below permits users with uid between 1000 and 1500
+to open connections:
+.Bd -literal -offset indent
+block out proto tcp all
+pass out proto tcp from self user { 999 >< 1501 }
+.Ed
+.Pp
+The
+.Sq \&:
+operator, which works for port number matching, does not work for
+.Cm user
+and
+.Cm group
+match.
.El
.Ss Translation
Translation options modify either the source or destination address and
Re: Unbreak X:Y user/group spec in pf.conf
On 2020/01/16 17:37, Alexandr Nedvedicky wrote:
> Hello,
>
>
> > > +of uids, which match the pass rule. The
> > New sentences on its own line. I'd say
> >
> > Note that users 1000 and 1500 are excluded from the pass rule.
> >
>
> yes, new sentence on the new line. and your wording sounds better.
>
> > > +.Cm :
> > The port paragraph marks up those operators with Sq (single quotes),
> > we should be consistent here. Cm for user and group is correct, though.
>
> fixed.
>
> updated manpage is below.
>
> thanks and
> regards
> sashan
>
> 8<---8<---8<--8<
> diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
> index 452a15d1cfd..fe99dc0c726 100644
> --- a/share/man/man5/pf.conf.5
> +++ b/share/man/man5/pf.conf.5
> @@ -820,6 +820,22 @@ connections:
> block out proto tcp all
> pass out proto tcp from self user { < 1000, dhartmei }
> .Ed
> +.Pp
> +The example below specifies a range of users to open outgoing
> +connections:
> +.Bd -literal -offset indent
> +block out proto tcp all
> +pass out proto tcp from self user { 1000 >< 1500 }
> +.Ed
> +.Pp
> +Note that users 1000 and 1500 are excluded from the pass rule.
The last line above is a little hard to parse - I think a "positive
example" would be clearer, i.e. something like this:
.Pp
The example below permits users with uid between 1000 and 1500
to open connections:
.Bd -literal -offset indent
block out proto tcp all
pass out proto tcp from self user { 999 >< 1501 }
.Ed
.Pp
The
.Sq \&:
operator, which works for port number matching, does not work for
[...]
Re: Unbreak X:Y user/group spec in pf.conf
On Thu, Jan 16, 2020 at 05:37:17PM +0100, Alexandr Nedvedicky wrote: > updated manpage is below. OK kn > +The ^ $ mandoc -T lint pf.conf.5 mandoc: pf.conf.5:832:4: STYLE: whitespace at end of input line mandoc: pf.conf.5:2946:2: WARNING: sections out of conventional order: Sh FILES
Re: Unbreak X:Y user/group spec in pf.conf
Hello,
> > +of uids, which match the pass rule. The
> New sentences on its own line. I'd say
>
> Note that users 1000 and 1500 are excluded from the pass rule.
>
yes, new sentence on the new line. and your wording sounds better.
> > +.Cm :
> The port paragraph marks up those operators with Sq (single quotes),
> we should be consistent here. Cm for user and group is correct, though.
fixed.
updated manpage is below.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
index 452a15d1cfd..fe99dc0c726 100644
--- a/share/man/man5/pf.conf.5
+++ b/share/man/man5/pf.conf.5
@@ -820,6 +820,22 @@ connections:
block out proto tcp all
pass out proto tcp from self user { < 1000, dhartmei }
.Ed
+.Pp
+The example below specifies a range of users to open outgoing
+connections:
+.Bd -literal -offset indent
+block out proto tcp all
+pass out proto tcp from self user { 1000 >< 1500 }
+.Ed
+.Pp
+Note that users 1000 and 1500 are excluded from the pass rule.
+The
+.Sq \&:
+operator, which works for port number matching, does not work for
+.Cm user
+and
+.Cm group
+match.
.El
.Ss Translation
Translation options modify either the source or destination address and
Re: Unbreak X:Y user/group spec in pf.conf
On Thu, Jan 16, 2020 at 03:29:16PM +0100, Alexandr Nedvedicky wrote:
> I think that's where we are heading after reading email from sthen@
>
> Let's focus on to update pf.conf.5 manpage. Would diff below make
> pf.conf.5
> manpage more useful?
I think this is the right way; adding the inclusive range for users
adds too much complexity for no real gain.
Nits inline,
OK kn
> diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
> index 452a15d1cfd..42c3c3466da 100644
> --- a/share/man/man5/pf.conf.5
> +++ b/share/man/man5/pf.conf.5
> @@ -820,6 +820,22 @@ connections:
> block out proto tcp all
> pass out proto tcp from self user { < 1000, dhartmei }
> .Ed
> +.Pp
> +The example below specifies a range of users to open outgoing
> +connections:
> +.Bd -literal -offset indent
> +block out proto tcp all
> +pass out proto tcp from self user { 1000 >< 1500 }
> +.Ed
> +.Pp
> +Note the range above excludes 1000 and 1500 uids from list
> +of uids, which match the pass rule. The
New sentences on its own line. I'd say
Note that users 1000 and 1500 are excluded from the pass rule.
This avoids repeating "uids" and "uid list", and since we can also do
"root >< kn" the wording "user" goes in line with the keyword (only
numerical values are UIDs, technically).
> +.Cm :
The port paragraph marks up those operators with Sq (single quotes),
we should be consistent here. Cm for user and group is correct, though.
> +operator, which works for port number matching, does not work for
> +.Cm user
> +and
> +.Cm group
> +match.
> .El
> .Ss Translation
> Translation options modify either the source or destination address and
Re: Unbreak X:Y user/group spec in pf.conf
Hello,
>
> (2Theo: yes, I'm lazy, sorry :) )
>
> I agree, that "X:Y" syntax for "user" could be confusing, and "X> simply ugly. I do not have a silver bullet here, though.
>
> If you oppose the proposed change, I'll add "... except 'uid1:uid2' syntax,
> which could be mistakenly interpreted as 'uid:gid'" to pf.conf(5). Will be
> that okay?
I think that's where we are heading after reading email from sthen@
Let's focus on to update pf.conf.5 manpage. Would diff below make pf.conf.5
manpage more useful?
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
index 452a15d1cfd..42c3c3466da 100644
--- a/share/man/man5/pf.conf.5
+++ b/share/man/man5/pf.conf.5
@@ -820,6 +820,22 @@ connections:
block out proto tcp all
pass out proto tcp from self user { < 1000, dhartmei }
.Ed
+.Pp
+The example below specifies a range of users to open outgoing
+connections:
+.Bd -literal -offset indent
+block out proto tcp all
+pass out proto tcp from self user { 1000 >< 1500 }
+.Ed
+.Pp
+Note the range above excludes 1000 and 1500 uids from list
+of uids, which match the pass rule. The
+.Cm :
+operator, which works for port number matching, does not work for
+.Cm user
+and
+.Cm group
+match.
.El
.Ss Translation
Translation options modify either the source or destination address and
Re: Unbreak X:Y user/group spec in pf.conf
16 января 2020 г. 15:58:09 GMT+03:00, Klemens Nanni пишет:
>On Thu, Jan 16, 2020 at 01:16:27PM +0100, Alexandr Nedvedicky wrote:
>> sentence 'The syntax is similar to the one for ports' sets my
>expectations
>> I can define a range of users in the same way I define a range of
>ports.
>> Looks useful to me, though a bug in parse.y might be just a tip
>of iceberg
>> here.
>I *assume* Vadim tripped over this implication, but that's what I
>wanted
>to know. That said, probably being biased here, "similar to the one
>for
>ports" does not read like "the same as ports" to me.
(2Theo: yes, I'm lazy, sorry :) )
I agree, that "X:Y" syntax for "user" could be confusing, and "X>After convincing Sasha in the hackroom that the range syntax for
>user/group is rather misleading and not worth the effort, he in turn
>made a convincing point about how mapping user ranges with existing
>syntax might go wrong:
>
> $ echo 'pass on lo proto tcp user { >= 1000 , <= 2000 }' | pfctl
>-vnf-
> pass on lo proto tcp all user >= 1000 flags S/SA
> pass on lo proto tcp all user <= 2000 flags S/SA
>
>Note how --depending on other keywords-- the provided inclusive range
>might evaluate to rules that pass more than desired; above example
>will
>pass all users since the [1000, 2000] is eventually used as [1000, inf]
>and [0, 2000] which together make for [0, inf], that is all users.
>
>With proper ranges as for ports the ruleset would evaluate to what
>users
>actually wanted. So ranges *can* already be covered but not in a sane
>and actually safe way.
The "expansion" feature could be used wrong for other config clauses as well,
especially when negation comes to play. I'm not sure if this should be changed
at all... We may force that only one of two syntaxes may be used:
user { foo, bar }
user { 1000 till 1999, >=1 }
--
With best regards,
Vadim Zhukov
Re: Unbreak X:Y user/group spec in pf.conf
On 2020/01/16 13:58, Klemens Nanni wrote:
> On Thu, Jan 16, 2020 at 01:16:27PM +0100, Alexandr Nedvedicky wrote:
> > sentence 'The syntax is similar to the one for ports' sets my
> > expectations
> > I can define a range of users in the same way I define a range of ports.
> > Looks useful to me, though a bug in parse.y might be just a tip of
> > iceberg
> > here.
> I *assume* Vadim tripped over this implication, but that's what I wanted
> to know. That said, probably being biased here, "similar to the one for
> ports" does not read like "the same as ports" to me.
>
> After convincing Sasha in the hackroom that the range syntax for
> user/group is rather misleading and not worth the effort, he in turn
> made a convincing point about how mapping user ranges with existing
> syntax might go wrong:
>
> $ echo 'pass on lo proto tcp user { >= 1000 , <= 2000 }' | pfctl -vnf-
> pass on lo proto tcp all user >= 1000 flags S/SA
> pass on lo proto tcp all user <= 2000 flags S/SA
>
> Note how --depending on other keywords-- the provided inclusive range
> might evaluate to rules that pass more than desired; above example will
> pass all users since the [1000, 2000] is eventually used as [1000, inf]
> and [0, 2000] which together make for [0, inf], that is all users.
>
> With proper ranges as for ports the ruleset would evaluate to what users
> actually wanted. So ranges *can* already be covered but not in a sane
> and actually safe way.
>
This is a valid use for ranges and we already have the >< operator.
As it excludes boundaries it's a bit awkward to use (in the same way
that >< is awkward with ports) but it does do the job.
I think the widespread use of user:group syntax in other parts of the
OS and in applications makes it much too dangerous to allow using e.g.
1:1000 as a range operator for this in pf. People who are reasonably
familiar with the syntax won't refer to the docs unless something
doesn't work.
Allowing something like "1 : 1000" would be pretty safe but I think
a bit confusing.
Re: Unbreak X:Y user/group spec in pf.conf
On Thu, Jan 16, 2020 at 01:16:27PM +0100, Alexandr Nedvedicky wrote:
> sentence 'The syntax is similar to the one for ports' sets my expectations
> I can define a range of users in the same way I define a range of ports.
> Looks useful to me, though a bug in parse.y might be just a tip of iceberg
> here.
I *assume* Vadim tripped over this implication, but that's what I wanted
to know. That said, probably being biased here, "similar to the one for
ports" does not read like "the same as ports" to me.
After convincing Sasha in the hackroom that the range syntax for
user/group is rather misleading and not worth the effort, he in turn
made a convincing point about how mapping user ranges with existing
syntax might go wrong:
$ echo 'pass on lo proto tcp user { >= 1000 , <= 2000 }' | pfctl -vnf-
pass on lo proto tcp all user >= 1000 flags S/SA
pass on lo proto tcp all user <= 2000 flags S/SA
Note how --depending on other keywords-- the provided inclusive range
might evaluate to rules that pass more than desired; above example will
pass all users since the [1000, 2000] is eventually used as [1000, inf]
and [0, 2000] which together make for [0, inf], that is all users.
With proper ranges as for ports the ruleset would evaluate to what users
actually wanted. So ranges *can* already be covered but not in a sane
and actually safe way.
Re: Unbreak X:Y user/group spec in pf.conf
Hello,
>
> > Looks like Vadim found a bug and I'll take a look at the patch
> > he has sent.
> Where do you see a bug?
>
at description of 'user' match the pf.conf(5) reads as follows:
User and group IDs can be specified as either numbers or names.
The syntax is similar to the one for ports. The following
example allows only selected users to open outgoing connections:
block out proto tcp all
pass out proto tcp from self user { < 1000, dhartmei }
sentence 'The syntax is similar to the one for ports' sets my expectations
I can define a range of users in the same way I define a range of ports.
Looks useful to me, though a bug in parse.y might be just a tip of iceberg
here.
regards
sashan
Re: Unbreak X:Y user/group spec in pf.conf
On Thu, Jan 16, 2020 at 12:30:07PM +0100, Alexandr Nedvedicky wrote: > On Wed, Jan 15, 2020 at 11:14:43PM -0700, Theo de Raadt wrote: > > What does 1234:12345 mean. It must be uid 1234 _and_ gid 12345? This is how I would interpret it, as that's existing semantic for ownership handling in chown(8) et al. > according to my understanding 'user 1234:12345' matches > _all_ user IDs in range, which starts with 1234 and ends 12345. > The ranges are explained at paragraph, which discusses ports: This would be the logical consequence if user/group arithmetics were to be identical to port arithmetics in pf.conf(5), but they are not. >Ports and ranges of ports are specified using these operators: > > = (equal) > != (unequal) > < (less than) > <= (less than or equal) > > (greater than) > >= (greater than or equal) > : (range including boundaries) > >< (range excluding boundaries) > <> (except range) > > to define the precise check (must be uid 1234 _and_ gid 12345), > one has to go to something like this: > > pass out proto tcp all group 12345 user 1234 This is only syntax I'd want to consider for such a use case: With above things in mind, "1234:12345" reads ambiguous; is the intention to range fro user 1234 to user 12345, is it meant to say user 1234 and group 12345? Both forms are already possible with the existing syntax in very clear forms and I don't see any improvement in supporting the chown(8) like syntax. > Looks like Vadim found a bug and I'll take a look at the patch > he has sent. Where do you see a bug?
Re: Unbreak X:Y user/group spec in pf.conf
On Thu, Jan 16, 2020 at 03:54:25AM +0300, Vadim Zhukov wrote: > I've just found that pfctl doesn't like 'X:Y' syntax for user and group > clauses, despite of the words in manpage. Which wording are you referring to exactly? To me the entire user and group documentation in pf.conf(5) is clear enough and does not suggest that "kn:wheel" or so is allowed.
Re: Unbreak X:Y user/group spec in pf.conf
Hello, just to clarify the user and group match in pf.conf On Wed, Jan 15, 2020 at 11:14:43PM -0700, Theo de Raadt wrote: > 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? according to my understanding 'user 1234:12345' matches _all_ user IDs in range, which starts with 1234 and ends 12345. The ranges are explained at paragraph, which discusses ports: Ports and ranges of ports are specified using these operators: = (equal) != (unequal) < (less than) <= (less than or equal) > (greater than) >= (greater than or equal) : (range including boundaries) >< (range excluding boundaries) <> (except range) to define the precise check (must be uid 1234 _and_ gid 12345), one has to go to something like this: pass out proto tcp all group 12345 user 1234 Looks like Vadim found a bug and I'll take a look at the patch he has sent. regards sashan
Re: Unbreak X:Y user/group spec in pf.conf
16 января 2020 г. 9:14:43 GMT+03:00, Theo de Raadt пишет:
>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 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 - 1.699
>> +++ sbin/pfctl/parse.y 16 Jan 2020 00:44:14 -
>> @@ -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
>> %tokenNUMBER
>> %token PORTBINARY
>> %type interface if_list if_item_not if_item
>> -%type number icmptype icmp6type uid gid
>> +%type number icmptype icmp6type
>> %type tos not yesno optnodf
>> %typeprobability
>> %type optweight
>> @@ -504,7 +505,7 @@ int parseport(char *, struct range *r, i
>> %type sourcetrack flush unaryop statelock
>> %type action
>> %type flags flag blockspec prio
>> -%type portplain portstar portrange
>> +%type portplain portstar portrange uidrange
>> gidrange
>> %typehashkey
>> %type proto proto_list proto_item
>> %type 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
Re: Unbreak X:Y user/group spec in pf.conf
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?
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?
Vadim Zhukov 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.y17 Oct 2019 21:54:28 - 1.699
> +++ sbin/pfctl/parse.y16 Jan 2020 00:44:14 -
> @@ -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 NUMBER
> %token PORTBINARY
> %type interface if_list if_item_not if_item
> -%type number icmptype icmp6type uid gid
> +%type number icmptype icmp6type
> %type tos not yesno optnodf
> %type probability
> %type optweight
> @@ -504,7 +505,7 @@ int parseport(char *, struct range *r, i
> %type sourcetrack flush unaryop statelock
> %type action
> %type flags flag blockspec prio
> -%type portplain portstar portrange
> +%type portplain portstar portrange uidrange
> gidrange
> %type hashkey
> %type proto proto_list proto_item
> %type 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;
>
Unbreak X:Y user/group spec in pf.conf
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 - 1.699
+++ sbin/pfctl/parse.y 16 Jan 2020 00:44:14 -
@@ -468,6 +468,7 @@ typedef struct {
#define PPORT_RANGE1
#define PPORT_STAR 2
intparseport(char *, struct range *r, int);
+intparse_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 NUMBER
%tokenPORTBINARY
%type interface if_list if_item_not if_item
-%typenumber icmptype icmp6type uid gid
+%typenumber icmptype icmp6type
%typetos not yesno optnodf
%type probability
%typeoptweight
@@ -504,7 +505,7 @@ int parseport(char *, struct range *r, i
%type sourcetrack flush unaryop statelock
%type action
%type flags flag blockspec prio
-%type portplain portstar portrange
+%type portplain portstar portrange uidrange gidrange
%type hashkey
%type proto proto_list proto_item
%typeprotoval
@@ -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] =
