Flowspec: Do not use comma for bitmask operators

For numeric operators, comma is used for disjunction in expressions like
"10, 20, 30..40". But for bitmask operators, comma is used for
conjunction in a way that does not really make much sense. Use always
explicit logical operators (&& and ||) to connect bitmask operators.

Thanks to Matt Corallo for the bugreport.
This commit is contained in:
Ondrej Zajicek (work) 2021-05-18 19:54:18 +02:00
parent e5468d1685
commit dd8481cc1c
4 changed files with 12 additions and 20 deletions

View file

@ -5080,20 +5080,21 @@ options (<cf/bfd/ and <cf/weight 1/), the second nexthop has just <cf/weight 2/.
<p>The flow specification are rules for routers and firewalls for filtering <p>The flow specification are rules for routers and firewalls for filtering
purpose. It is described by <rfc id="5575">. There are 3 types of arguments: purpose. It is described by <rfc id="5575">. There are 3 types of arguments:
<m/inet4/ or <m/inet6/ prefixes, bitmasks matching expressions and numbers <m/inet4/ or <m/inet6/ prefixes, numeric matching expressions and bitmask
matching expressions. matching expressions.
Bitmasks matching is written using <m/value/<cf>/</cf><m/mask/ or Numeric matching is a matching sequence of numbers and ranges separeted by a
<cf/!/<m/value/<cf>/</cf><m/mask/ pairs. It means that <cf/(/<m/data/ <cf/&/
<m/mask/<cf/)/ is or is not equal to <m/value/.
Numbers matching is a matching sequence of numbers and ranges separeted by a
commas (<cf/,/) (e.g. <cf/10,20,30/). Ranges can be written using double dots commas (<cf/,/) (e.g. <cf/10,20,30/). Ranges can be written using double dots
<cf/../ notation (e.g. <cf/80..90,120..124/). An alternative notation are <cf/../ notation (e.g. <cf/80..90,120..124/). An alternative notation are
sequence of one or more pairs of relational operators and values separated by sequence of one or more pairs of relational operators and values separated by
logical operators <cf/&&/ or <cf/||/. Allowed relational operators are <cf/=/, logical operators <cf/&&/ or <cf/||/. Allowed relational operators are <cf/=/,
<cf/!=/, <cf/</, <cf/<=/, <cf/>/, <cf/>=/, <cf/true/ and <cf/false/. <cf/!=/, <cf/</, <cf/<=/, <cf/>/, <cf/>=/, <cf/true/ and <cf/false/.
Bitmask matching is written using <m/value/<cf>/</cf><m/mask/ or
<cf/!/<m/value/<cf>/</cf><m/mask/ pairs. It means that <cf/(/<m/data/ <cf/&/
<m/mask/<cf/)/ is or is not equal to <m/value/. It is also possible to use
multiple value/mask pairs connected by logical operators <cf/&&/ or <cf/||/.
<sect2>IPv4 Flowspec <sect2>IPv4 Flowspec
<p><descrip> <p><descrip>
@ -5199,7 +5200,7 @@ protocol static {
next header = 23; next header = 23;
sport > 24 && < 30 || = 40 || 50,60,70..80; sport > 24 && < 30 || = 40 || 50,60,70..80;
dport = 50; dport = 50;
tcp flags 0x03/0x0f, !0/0xff || 0x33/0x33; tcp flags 0x03/0x0f && !0/0xff || 0x33/0x33;
fragment !is_fragment || !first_fragment; fragment !is_fragment || !first_fragment;
label 0xaaaa/0xaaaa && 0x33/0x33; label 0xaaaa/0xaaaa && 0x33/0x33;
}; };

View file

@ -579,7 +579,7 @@ prefix p;
bt_assert(format(flow6 { sport 0..0x400; }) = "flow6 { sport 0..1024; }"); bt_assert(format(flow6 { sport 0..0x400; }) = "flow6 { sport 0..1024; }");
bt_assert(format(flow6 { icmp type 80; }) = "flow6 { icmp type 80; }"); bt_assert(format(flow6 { icmp type 80; }) = "flow6 { icmp type 80; }");
bt_assert(format(flow6 { icmp code 90; }) = "flow6 { icmp code 90; }"); bt_assert(format(flow6 { icmp code 90; }) = "flow6 { icmp code 90; }");
bt_assert(format(flow6 { tcp flags 0x03/0x0f; }) = "flow6 { tcp flags 0x3/0x3,0x0/0xc; }"); bt_assert(format(flow6 { tcp flags 0x03/0x0f; }) = "flow6 { tcp flags 0x3/0x3 && 0x0/0xc; }");
bt_assert(format(flow6 { length 0..65535; }) = "flow6 { length 0..65535; }"); bt_assert(format(flow6 { length 0..65535; }) = "flow6 { length 0..65535; }");
bt_assert(format(flow6 { dscp = 63; }) = "flow6 { dscp 63; }"); bt_assert(format(flow6 { dscp = 63; }) = "flow6 { dscp 63; }");
bt_assert(format(flow6 { fragment is_fragment || !first_fragment; }) = "flow6 { fragment is_fragment || !first_fragment; }"); bt_assert(format(flow6 { fragment is_fragment || !first_fragment; }) = "flow6 { fragment is_fragment || !first_fragment; }");

View file

@ -1278,17 +1278,8 @@ net_format_flow_bitmask(buffer *b, const byte *part)
while (1) while (1)
{ {
if (!first) if (!first)
{ buffer_puts(b, isset_and(op) ? "&& " : "|| ");
if (isset_and(op))
{
b->pos--; /* Remove last char (it is a space) */
buffer_puts(b, ",");
}
else
{
buffer_puts(b, "|| ");
}
}
first = 0; first = 0;
len = get_value_length(op); len = get_value_length(op);

View file

@ -630,7 +630,7 @@ t_formatting4(void)
net_addr_flow4 *input; net_addr_flow4 *input;
NET_ADDR_FLOW4_(input, ip4_build(5, 6, 7, 0), 24, nlri); NET_ADDR_FLOW4_(input, ip4_build(5, 6, 7, 0), 24, nlri);
const char *expect = "flow4 { dst 10.0.0.0/8; proto 23; dport > 24 && < 30 || 40..50,60..70,80 && >= 90; sport > 24 && < 30 || 40,50,60..70,80; icmp type 80; icmp code 90; tcp flags 0x3/0x3,0x0/0xc; length 0..65535; dscp 63; fragment dont_fragment || !is_fragment; }"; const char *expect = "flow4 { dst 10.0.0.0/8; proto 23; dport > 24 && < 30 || 40..50,60..70,80 && >= 90; sport > 24 && < 30 || 40,50,60..70,80; icmp type 80; icmp code 90; tcp flags 0x3/0x3 && 0x0/0xc; length 0..65535; dscp 63; fragment dont_fragment || !is_fragment; }";
bt_assert(flow4_net_format(b, sizeof(b), input) == strlen(expect)); bt_assert(flow4_net_format(b, sizeof(b), input) == strlen(expect));
bt_debug(" expect: '%s',\n output: '%s'\n", expect, b); bt_debug(" expect: '%s',\n output: '%s'\n", expect, b);