* [nft PATCH 1/3] proto: short-circuit loops over upper protocols
2021-12-11 18:55 [nft PATCH 0/3] inet reject statement fix Jeremy Sowden
@ 2021-12-11 18:55 ` Jeremy Sowden
2021-12-11 18:55 ` [nft PATCH 2/3] evaluate: correct typo's Jeremy Sowden
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Jeremy Sowden @ 2021-12-11 18:55 UTC (permalink / raw)
To: Netfilter Devel
Each `struct proto_desc` contains a fixed-size array of higher layer
protocols. Only the first few are not NULL. Therefore, we can stop
iterating over the array once we reach a NULL member.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
src/proto.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/proto.c b/src/proto.c
index fe58c83a9295..31a2f38065ad 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -59,7 +59,8 @@ proto_find_upper(const struct proto_desc *base, unsigned int num)
{
unsigned int i;
- for (i = 0; i < array_size(base->protocols); i++) {
+ for (i = 0; i < array_size(base->protocols) && base->protocols[i].desc;
+ i++) {
if (base->protocols[i].num == num)
return base->protocols[i].desc;
}
@@ -77,7 +78,8 @@ int proto_find_num(const struct proto_desc *base,
{
unsigned int i;
- for (i = 0; i < array_size(base->protocols); i++) {
+ for (i = 0; i < array_size(base->protocols) && base->protocols[i].desc;
+ i++) {
if (base->protocols[i].desc == desc)
return base->protocols[i].num;
}
@@ -105,7 +107,9 @@ int proto_dev_type(const struct proto_desc *desc, uint16_t *res)
*res = dev_proto_desc[i].type;
return 0;
}
- for (j = 0; j < array_size(base->protocols); j++) {
+ for (j = 0; j < array_size(base->protocols) &&
+ base->protocols[j].desc;
+ j++) {
if (base->protocols[j].desc == desc) {
*res = dev_proto_desc[i].type;
return 0;
--
2.33.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [nft PATCH 2/3] evaluate: correct typo's
2021-12-11 18:55 [nft PATCH 0/3] inet reject statement fix Jeremy Sowden
2021-12-11 18:55 ` [nft PATCH 1/3] proto: short-circuit loops over upper protocols Jeremy Sowden
@ 2021-12-11 18:55 ` Jeremy Sowden
2021-12-11 18:55 ` [nft PATCH 3/3] evaluate: reject: support ethernet as L2 protcol for inet table Jeremy Sowden
2021-12-15 21:50 ` [nft PATCH 0/3] inet reject statement fix Pablo Neira Ayuso
3 siblings, 0 replies; 5+ messages in thread
From: Jeremy Sowden @ 2021-12-11 18:55 UTC (permalink / raw)
To: Netfilter Devel
There are a couple of mistakes in comments. Fix them.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
src/evaluate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 49fb8f84fe76..4d4dcc2e3d08 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -599,7 +599,7 @@ static int expr_evaluate_exthdr(struct eval_ctx *ctx, struct expr **exprp)
/* dependency supersede.
*
- * 'inet' is a 'phony' l2 dependency used by NFPROTO_INET to fulfill network
+ * 'inet' is a 'phony' l2 dependency used by NFPROTO_INET to fulfil network
* header dependency, i.e. ensure that 'ip saddr 1.2.3.4' only sees ip headers.
*
* If a match expression that depends on a particular L2 header, e.g. ethernet,
@@ -611,7 +611,7 @@ static int expr_evaluate_exthdr(struct eval_ctx *ctx, struct expr **exprp)
* Example: inet filter in ip saddr 1.2.3.4 ether saddr a:b:c:d:e:f
*
* ip saddr adds meta dependency on ipv4 packets
- * ether saddr adds another dependeny on ethernet frames.
+ * ether saddr adds another dependency on ethernet frames.
*/
static int meta_iiftype_gen_dependency(struct eval_ctx *ctx,
struct expr *payload, struct stmt **res)
--
2.33.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [nft PATCH 3/3] evaluate: reject: support ethernet as L2 protcol for inet table
2021-12-11 18:55 [nft PATCH 0/3] inet reject statement fix Jeremy Sowden
2021-12-11 18:55 ` [nft PATCH 1/3] proto: short-circuit loops over upper protocols Jeremy Sowden
2021-12-11 18:55 ` [nft PATCH 2/3] evaluate: correct typo's Jeremy Sowden
@ 2021-12-11 18:55 ` Jeremy Sowden
2021-12-15 21:50 ` [nft PATCH 0/3] inet reject statement fix Pablo Neira Ayuso
3 siblings, 0 replies; 5+ messages in thread
From: Jeremy Sowden @ 2021-12-11 18:55 UTC (permalink / raw)
To: Netfilter Devel
When we are evaluating a `reject` statement in the `inet` family, we may
have `ether` and `ip` or `ip6` as the L2 and L3 protocols in the
evaluation context:
table inet filter {
chain input {
type filter hook input priority filter;
ether saddr aa:bb:cc:dd:ee:ff ip daddr 192.168.0.1 reject
}
}
Since no `reject` option is given, nft attempts to infer one and fails:
BUG: unsupported familynft: evaluate.c:2766:stmt_evaluate_reject_inet_family: Assertion `0' failed.
Aborted
The reason it fails is that the ethernet protocol numbers for IPv4 and
IPv6 (`ETH_P_IP` and `ETH_P_IPV6`) do not match `NFPROTO_IPV4` and
`NFPROTO_IPV6`. Add support for the ethernet protocol numbers.
Replace the current `BUG("unsupported family")` error message with
something more informative that tells the user to provide an explicit
reject option.
Add a Python test case.
Fixes: 5fdd0b6a0600 ("nft: complete reject support")
Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1001360
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
src/evaluate.c | 7 +++++-
tests/py/inet/reject.t | 2 ++
tests/py/inet/reject.t.json | 34 +++++++++++++++++++++++++++++
tests/py/inet/reject.t.payload.inet | 10 +++++++++
4 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 4d4dcc2e3d08..8edefbd1be21 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2751,19 +2751,22 @@ static int stmt_evaluate_reject_inet_family(struct eval_ctx *ctx,
protocol = proto_find_num(base, desc);
switch (protocol) {
case NFPROTO_IPV4:
+ case __constant_htons(ETH_P_IP):
if (stmt->reject.family == NFPROTO_IPV4)
break;
return stmt_binary_error(ctx, stmt->reject.expr,
&ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR],
"conflicting protocols specified: ip vs ip6");
case NFPROTO_IPV6:
+ case __constant_htons(ETH_P_IPV6):
if (stmt->reject.family == NFPROTO_IPV6)
break;
return stmt_binary_error(ctx, stmt->reject.expr,
&ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR],
"conflicting protocols specified: ip vs ip6");
default:
- BUG("unsupported family");
+ return stmt_error(ctx, stmt,
+ "cannot infer ICMP reject variant to use: explicit value required.\n");
}
break;
}
@@ -2923,10 +2926,12 @@ static int stmt_evaluate_reject_default(struct eval_ctx *ctx,
protocol = proto_find_num(base, desc);
switch (protocol) {
case NFPROTO_IPV4:
+ case __constant_htons(ETH_P_IP):
stmt->reject.family = NFPROTO_IPV4;
stmt->reject.icmp_code = ICMP_PORT_UNREACH;
break;
case NFPROTO_IPV6:
+ case __constant_htons(ETH_P_IPV6):
stmt->reject.family = NFPROTO_IPV6;
stmt->reject.icmp_code = ICMP6_DST_UNREACH_NOPORT;
break;
diff --git a/tests/py/inet/reject.t b/tests/py/inet/reject.t
index 1c8aeebe1b07..61a6d556d2ad 100644
--- a/tests/py/inet/reject.t
+++ b/tests/py/inet/reject.t
@@ -37,3 +37,5 @@ meta l4proto udp reject with tcp reset;fail
meta nfproto ipv4 reject with icmpx admin-prohibited;ok
meta nfproto ipv6 reject with icmpx admin-prohibited;ok
+
+ether saddr aa:bb:cc:dd:ee:ff ip daddr 192.168.0.1 reject;ok;ether saddr aa:bb:cc:dd:ee:ff ip daddr 192.168.0.1 reject with icmp port-unreachable
diff --git a/tests/py/inet/reject.t.json b/tests/py/inet/reject.t.json
index 76cd1bf579d2..02ac9007ef33 100644
--- a/tests/py/inet/reject.t.json
+++ b/tests/py/inet/reject.t.json
@@ -295,3 +295,37 @@
}
]
+# ether saddr aa:bb:cc:dd:ee:ff ip daddr 192.168.0.1 reject
+[
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "field": "saddr",
+ "protocol": "ether"
+ }
+ },
+ "op": "==",
+ "right": "aa:bb:cc:dd:ee:ff"
+ }
+ },
+ {
+ "match": {
+ "left": {
+ "payload": {
+ "field": "daddr",
+ "protocol": "ip"
+ }
+ },
+ "op": "==",
+ "right": "192.168.0.1"
+ }
+ },
+ {
+ "reject": {
+ "expr": "port-unreachable",
+ "type": "icmp"
+ }
+ }
+]
+
diff --git a/tests/py/inet/reject.t.payload.inet b/tests/py/inet/reject.t.payload.inet
index 62078d91b0cf..828cb839c30c 100644
--- a/tests/py/inet/reject.t.payload.inet
+++ b/tests/py/inet/reject.t.payload.inet
@@ -132,3 +132,13 @@ inet test-inet input
[ cmp eq reg 1 0x0000000a ]
[ reject type 2 code 3 ]
+# ether saddr aa:bb:cc:dd:ee:ff ip daddr 192.168.0.1 reject
+inet test-inet input
+ [ meta load iiftype => reg 1 ]
+ [ cmp eq reg 1 0x00000001 ]
+ [ payload load 8b @ link header + 6 => reg 1 ]
+ [ cmp eq reg 1 0xddccbbaa 0x0008ffee ]
+ [ payload load 4b @ network header + 16 => reg 1 ]
+ [ cmp eq reg 1 0x0100a8c0 ]
+ [ reject type 0 code 3 ]
+
--
2.33.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [nft PATCH 0/3] inet reject statement fix
2021-12-11 18:55 [nft PATCH 0/3] inet reject statement fix Jeremy Sowden
` (2 preceding siblings ...)
2021-12-11 18:55 ` [nft PATCH 3/3] evaluate: reject: support ethernet as L2 protcol for inet table Jeremy Sowden
@ 2021-12-15 21:50 ` Pablo Neira Ayuso
3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2021-12-15 21:50 UTC (permalink / raw)
To: Jeremy Sowden; +Cc: Netfilter Devel
On Sat, Dec 11, 2021 at 06:55:22PM +0000, Jeremy Sowden wrote:
> The first two patches contain small improvements that I noticed while
> looking into a Debian bug-report. The third contains a fix for the
> reported bug, that `inet` `reject` rules of the form:
>
> table inet filter {
> chain input {
> type filter hook input priority filter;
> ether saddr aa:bb:cc:dd:ee:ff ip daddr 192.168.0.1 reject
> }
> }
>
> fail with:
>
> BUG: unsupported familynft: evaluate.c:2766:stmt_evaluate_reject_inet_family: Assertion `0' failed.
> Aborted
Series applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread