bugzilla-daemon at netfilter.org
2013-Jul-20 21:47 UTC
[Bug 835] New: protocol without option is failing
https://bugzilla.netfilter.org/show_bug.cgi?id=835
Summary: protocol without option is failing
Product: nftables
Version: unspecified
Platform: x86_64
OS/Version: All
Status: NEW
Severity: enhancement
Priority: P5
Component: nft
AssignedTo: pablo at netfilter.org
ReportedBy: eric at regit.org
Estimated Hours: 0.0
When reading parser.y, it seems that tcp was meant to be used as standalone
keyword:
tcp_hdr_expr : TCP
{
uint8_t data = IPPROTO_TCP;
$$ = constant_expr_alloc(&@$,
&inet_protocol_type,
BYTEORDER_HOST_ENDIAN,
sizeof(data) *
BITS_PER_BYTE, &data);
}
| TCP tcp_hdr_field
{
$$ = payload_expr_alloc(&@$,
&payload_tcp, $2);
}
;
But there is a bug in the grammar because the following rule is not working:
nft add rule ip6 filter input tcp accept --debug=all
...
Cleanup: discarding lookahead token "accept" (: )
Stack now 0
<cmdline>:1:31-36: Error: syntax error, unexpected accept
add rule ip6 filter input tcp accept
^^^^^^
--
Configure bugmail: https://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
bugzilla-daemon at netfilter.org
2013-Aug-14 21:27 UTC
[Bug 835] protocol without option is failing
https://bugzilla.netfilter.org/show_bug.cgi?id=835
Phil Oester <netfilter at linuxace.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |netfilter at linuxace.com
--- Comment #1 from Phil Oester <netfilter at linuxace.com> 2013-08-14
23:27:53 CEST ---
I think you're misinterpreting the parser. It can be used standalone as
long
as you have "ip protocol" before it. Example:
nft add rule ip filter input ip protocol tcp accept
But without the "ip protocol", you need to have some tcp header
option.
--
Configure bugmail: https://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
bugzilla-daemon at netfilter.org
2013-Aug-14 21:40 UTC
[Bug 835] protocol without option is failing
https://bugzilla.netfilter.org/show_bug.cgi?id=835 --- Comment #2 from Eric Leblond <eric at regit.org> 2013-08-14 23:40:35 CEST --- (In reply to comment #1)> I think you're misinterpreting the parser. It can be used standalone as long > as you have "ip protocol" before it. Example: > > nft add rule ip filter input ip protocol tcp accept > > But without the "ip protocol", you need to have some tcp header option.Yes, this work with "ip protocol" but I'm almost sure that using only tcp keyword was supposed to work. -- Configure bugmail: https://bugzilla.netfilter.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
bugzilla-daemon at netfilter.org
2013-Aug-14 21:47 UTC
[Bug 835] protocol without option is failing
https://bugzilla.netfilter.org/show_bug.cgi?id=835
--- Comment #3 from Phil Oester <netfilter at linuxace.com> 2013-08-14
23:47:08 CEST ---
The original commit which added this feature does not mesh with your
understanding:
commit 6c3eec6ad009d7ed8a219291b98886a80b26b8e4
Author: Patrick McHardy <kaber at trash.net>
Date: Wed Dec 5 19:39:00 2012 +0100
parser: fix parsing protocol names for protocols which are also keywords
"ip protocol tcp" will currently produce a syntax error since tcp
is also a
keyword
which is expected ot be followed by a tcp header field. Allow to use
protocol names
that are also keywords and allocate a constant expression for them.
Aside from that, I think it wouldn't fit with the existing language to have
protocols listed by themselves. When you want to choose a specific feature of
the ip header, you need to use "ip <header> <foo>". So
"ip protocol tcp" is
consistent with "ip saddr x.x.x.x". In general, the parser seems more
consistent the way it is currently operating.
And finally, even iptables requires "-p" before specifying a protocol.
--
Configure bugmail: https://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
bugzilla-daemon at netfilter.org
2013-Aug-14 22:01 UTC
[Bug 835] protocol without option is failing
https://bugzilla.netfilter.org/show_bug.cgi?id=835 --- Comment #4 from Eric Leblond <eric at regit.org> 2013-08-15 00:01:08 CEST --- (In reply to comment #3)> The original commit which added this feature does not mesh with your > understanding: > > commit 6c3eec6ad009d7ed8a219291b98886a80b26b8e4 > Author: Patrick McHardy <kaber at trash.net> > Date: Wed Dec 5 19:39:00 2012 +0100 > > parser: fix parsing protocol names for protocols which are also keywords > > "ip protocol tcp" will currently produce a syntax error since tcp is also a > keyword > which is expected ot be followed by a tcp header field. Allow to use > protocol names > that are also keywords and allocate a constant expression for them.OK, you are right on this. I misunderstood the grammar.> Aside from that, I think it wouldn't fit with the existing language to have > protocols listed by themselves. When you want to choose a specific feature of > the ip header, you need to use "ip <header> <foo>". So "ip protocol tcp" is > consistent with "ip saddr x.x.x.x". In general, the parser seems more > consistent the way it is currently operating. > > And finally, even iptables requires "-p" before specifying a protocol.I don't agree on this point. In iptables "-p" was just needed to imply the context (and load the corresponding module). In nftables, we can use "tcp dport 80" without specifying "ip protocol". So this is really tempting for the user to use only "tcp". So, not a bug but an enahncement in P5 seems ok ;) -- Configure bugmail: https://bugzilla.netfilter.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
bugzilla-daemon at netfilter.org
2013-Aug-14 22:09 UTC
[Bug 835] protocol without option is failing
https://bugzilla.netfilter.org/show_bug.cgi?id=835 --- Comment #5 from Phil Oester <netfilter at linuxace.com> 2013-08-15 00:09:02 CEST --- (In reply to comment #4)> I don't agree on this point. In iptables "-p" was just needed to imply the > context (and load the corresponding module).OK, so that bit was a bad example.> In nftables, we can use "tcp dport > 80" without specifying "ip protocol". So this is really tempting for the user > to use only "tcp". So, not a bug but an enahncement in P5 seems ok ;)I still believe it is best not to add special cases like this. Iptables is full of (ugly) special case hacks. Ideally, nft grammar should strive to be avoid this cruft. If you want to analyze some aspect of the ip header (such as protocol), you should have to specify "ip ...". If you want to analyze some aspect of the tcp header (such as dport), then you should use "tcp ...". Seems consistent to me, based upon the header you wish to inspect. -- Configure bugmail: https://bugzilla.netfilter.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
bugzilla-daemon at netfilter.org
2013-Aug-15 00:04 UTC
[Bug 835] protocol without option is failing
https://bugzilla.netfilter.org/show_bug.cgi?id=835
--- Comment #6 from Phil Oester <netfilter at linuxace.com> 2013-08-15
02:04:44 CEST ---
Two other things to consider.
1) There are 255 possible protocols. If we decide to allow these
"shortcuts"
for tcp, udp, icmp, and a handful of others, invariably someone will complain
that they ALL should be supported ("Why isn't L2TP there? Or
OSPF??").
2) If we allow bare "tcp" for protocol, why not allow "6"?
Of course, in
testing this, I discovered that nft doesn't support numbers now - even when
properly using "ip protocol".
nft add rule ip filter input ip protocol 6 accept
<cmdline>:1:38-38: Error: Could not resolve protocol name
So this is a separate bug...
--
Configure bugmail: https://bugzilla.netfilter.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.