* [PATCH nft 1/3] ipopt: drop unused 'ptr' argument
2021-12-03 16:07 [PATCH nft 0/3] typeof fixes Florian Westphal
@ 2021-12-03 16:07 ` Florian Westphal
2021-12-03 16:07 ` [PATCH nft 2/3] exthdr: support ip/tcp options and sctp chunks in typeof expressions Florian Westphal
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Florian Westphal @ 2021-12-03 16:07 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
Its always 0, so remove it.
Looks like this was intended to support variable options that have
array-like members, but so far this isn't implemented, better remove
dead code and implement it properly when such support is needed.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/ipopt.h | 2 +-
src/ipopt.c | 23 ++---------------------
src/parser_bison.y | 4 ++--
src/parser_json.c | 4 ++--
4 files changed, 7 insertions(+), 26 deletions(-)
diff --git a/include/ipopt.h b/include/ipopt.h
index d8d48066ae50..03420dc6221d 100644
--- a/include/ipopt.h
+++ b/include/ipopt.h
@@ -6,7 +6,7 @@
#include <statement.h>
extern struct expr *ipopt_expr_alloc(const struct location *loc,
- uint8_t type, uint8_t field, uint8_t ptr);
+ uint8_t type, uint8_t field);
extern void ipopt_init_raw(struct expr *expr, uint8_t type,
unsigned int offset, unsigned int len,
diff --git a/src/ipopt.c b/src/ipopt.c
index 5f9f908c0b34..42ea41cd705b 100644
--- a/src/ipopt.c
+++ b/src/ipopt.c
@@ -66,27 +66,8 @@ const struct exthdr_desc *ipopt_protocols[UINT8_MAX] = {
[IPOPT_RA] = &ipopt_ra,
};
-static unsigned int calc_offset(const struct exthdr_desc *desc,
- const struct proto_hdr_template *tmpl,
- unsigned int arg)
-{
- if (!desc || tmpl == &ipopt_unknown_template)
- return 0;
-
- switch (desc->type) {
- case IPOPT_RR:
- case IPOPT_LSRR:
- case IPOPT_SSRR:
- if (tmpl == &desc->templates[IPOPT_FIELD_ADDR_0])
- return (tmpl->offset < 24) ? 0 : arg;
- return 0;
- default:
- return 0;
- }
-}
-
struct expr *ipopt_expr_alloc(const struct location *loc, uint8_t type,
- uint8_t field, uint8_t ptr)
+ uint8_t field)
{
const struct proto_hdr_template *tmpl;
const struct exthdr_desc *desc;
@@ -102,7 +83,7 @@ struct expr *ipopt_expr_alloc(const struct location *loc, uint8_t type,
expr->exthdr.desc = desc;
expr->exthdr.tmpl = tmpl;
expr->exthdr.op = NFT_EXTHDR_OP_IPV4;
- expr->exthdr.offset = tmpl->offset + calc_offset(desc, tmpl, ptr);
+ expr->exthdr.offset = tmpl->offset;
expr->exthdr.raw_type = desc->type;
return expr;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 355758e1befb..357850dececc 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -5331,11 +5331,11 @@ ip_hdr_expr : IP ip_hdr_field close_scope_ip
}
| IP OPTION ip_option_type ip_option_field close_scope_ip
{
- $$ = ipopt_expr_alloc(&@$, $3, $4, 0);
+ $$ = ipopt_expr_alloc(&@$, $3, $4);
}
| IP OPTION ip_option_type close_scope_ip
{
- $$ = ipopt_expr_alloc(&@$, $3, IPOPT_FIELD_TYPE, 0);
+ $$ = ipopt_expr_alloc(&@$, $3, IPOPT_FIELD_TYPE);
$$->exthdr.flags = NFT_EXTHDR_F_PRESENT;
}
;
diff --git a/src/parser_json.c b/src/parser_json.c
index 7a2d30ff665c..2fad308f7783 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -689,7 +689,7 @@ static struct expr *json_parse_ip_option_expr(struct json_ctx *ctx,
if (json_unpack(root, "{s:s}", "field", &field)) {
expr = ipopt_expr_alloc(int_loc, descval,
- IPOPT_FIELD_TYPE, 0);
+ IPOPT_FIELD_TYPE);
expr->exthdr.flags = NFT_EXTHDR_F_PRESENT;
return expr;
@@ -698,7 +698,7 @@ static struct expr *json_parse_ip_option_expr(struct json_ctx *ctx,
json_error(ctx, "Unknown ip option field '%s'.", field);
return NULL;
}
- return ipopt_expr_alloc(int_loc, descval, fieldval, 0);
+ return ipopt_expr_alloc(int_loc, descval, fieldval);
}
static int json_parse_sctp_chunk_field(const struct exthdr_desc *desc,
--
2.32.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH nft 2/3] exthdr: support ip/tcp options and sctp chunks in typeof expressions
2021-12-03 16:07 [PATCH nft 0/3] typeof fixes Florian Westphal
2021-12-03 16:07 ` [PATCH nft 1/3] ipopt: drop unused 'ptr' argument Florian Westphal
@ 2021-12-03 16:07 ` Florian Westphal
2021-12-03 16:07 ` [PATCH nft 3/3] iptopt: fix crash with invalid field/type combo Florian Westphal
2021-12-06 20:03 ` [PATCH nft 0/3] typeof fixes Pablo Neira Ayuso
3 siblings, 0 replies; 5+ messages in thread
From: Florian Westphal @ 2021-12-03 16:07 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
This did not store the 'op' member and listing always treated this as ipv6
extension header.
Add test cases for this.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/exthdr.c | 47 ++++++++++++++++---
.../testcases/sets/dumps/typeof_sets_0.nft | 27 +++++++++++
tests/shell/testcases/sets/typeof_sets_0 | 27 +++++++++++
3 files changed, 94 insertions(+), 7 deletions(-)
diff --git a/src/exthdr.c b/src/exthdr.c
index 2357ab60648d..3e5f5cd8b73e 100644
--- a/src/exthdr.c
+++ b/src/exthdr.c
@@ -115,7 +115,8 @@ static void exthdr_expr_clone(struct expr *new, const struct expr *expr)
#define NFTNL_UDATA_EXTHDR_DESC 0
#define NFTNL_UDATA_EXTHDR_TYPE 1
-#define NFTNL_UDATA_EXTHDR_MAX 2
+#define NFTNL_UDATA_EXTHDR_OP 2
+#define NFTNL_UDATA_EXTHDR_MAX 3
static int exthdr_parse_udata(const struct nftnl_udata *attr, void *data)
{
@@ -126,6 +127,7 @@ static int exthdr_parse_udata(const struct nftnl_udata *attr, void *data)
switch (type) {
case NFTNL_UDATA_EXTHDR_DESC:
case NFTNL_UDATA_EXTHDR_TYPE:
+ case NFTNL_UDATA_EXTHDR_OP:
if (len != sizeof(uint32_t))
return -1;
break;
@@ -140,6 +142,7 @@ static int exthdr_parse_udata(const struct nftnl_udata *attr, void *data)
static struct expr *exthdr_expr_parse_udata(const struct nftnl_udata *attr)
{
const struct nftnl_udata *ud[NFTNL_UDATA_EXTHDR_MAX + 1] = {};
+ enum nft_exthdr_op op = NFT_EXTHDR_OP_IPV6;
const struct exthdr_desc *desc;
unsigned int type;
uint32_t desc_id;
@@ -154,14 +157,31 @@ static struct expr *exthdr_expr_parse_udata(const struct nftnl_udata *attr)
!ud[NFTNL_UDATA_EXTHDR_TYPE])
return NULL;
- desc_id = nftnl_udata_get_u32(ud[NFTNL_UDATA_EXTHDR_DESC]);
- desc = exthdr_find_desc(desc_id);
- if (!desc)
- return NULL;
+ if (ud[NFTNL_UDATA_EXTHDR_OP])
+ op = nftnl_udata_get_u32(ud[NFTNL_UDATA_EXTHDR_OP]);
+ desc_id = nftnl_udata_get_u32(ud[NFTNL_UDATA_EXTHDR_DESC]);
type = nftnl_udata_get_u32(ud[NFTNL_UDATA_EXTHDR_TYPE]);
- return exthdr_expr_alloc(&internal_location, desc, type);
+ switch (op) {
+ case NFT_EXTHDR_OP_IPV6:
+ desc = exthdr_find_desc(desc_id);
+
+ return exthdr_expr_alloc(&internal_location, desc, type);
+ case NFT_EXTHDR_OP_TCPOPT:
+ return tcpopt_expr_alloc(&internal_location,
+ desc_id, type);
+ case NFT_EXTHDR_OP_IPV4:
+ return ipopt_expr_alloc(&internal_location,
+ desc_id, type);
+ case NFT_EXTHDR_OP_SCTP:
+ return sctp_chunk_expr_alloc(&internal_location,
+ desc_id, type);
+ case __NFT_EXTHDR_OP_MAX:
+ return NULL;
+ }
+
+ return NULL;
}
static unsigned int expr_exthdr_type(const struct exthdr_desc *desc,
@@ -176,9 +196,22 @@ static int exthdr_expr_build_udata(struct nftnl_udata_buf *udbuf,
const struct proto_hdr_template *tmpl = expr->exthdr.tmpl;
const struct exthdr_desc *desc = expr->exthdr.desc;
unsigned int type = expr_exthdr_type(desc, tmpl);
+ enum nft_exthdr_op op = expr->exthdr.op;
- nftnl_udata_put_u32(udbuf, NFTNL_UDATA_EXTHDR_DESC, desc->id);
nftnl_udata_put_u32(udbuf, NFTNL_UDATA_EXTHDR_TYPE, type);
+ switch (op) {
+ case NFT_EXTHDR_OP_IPV6:
+ nftnl_udata_put_u32(udbuf, NFTNL_UDATA_EXTHDR_DESC, desc->id);
+ break;
+ case NFT_EXTHDR_OP_TCPOPT:
+ case NFT_EXTHDR_OP_IPV4:
+ case NFT_EXTHDR_OP_SCTP:
+ nftnl_udata_put_u32(udbuf, NFTNL_UDATA_EXTHDR_OP, op);
+ nftnl_udata_put_u32(udbuf, NFTNL_UDATA_EXTHDR_DESC, expr->exthdr.raw_type);
+ break;
+ default:
+ return -1;
+ }
return 0;
}
diff --git a/tests/shell/testcases/sets/dumps/typeof_sets_0.nft b/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
index 06d891e682b7..8f11b110552c 100644
--- a/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
+++ b/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
@@ -19,6 +19,21 @@ table inet t {
elements = { 1, 1024 }
}
+ set s5 {
+ typeof ip option ra value
+ elements = { 1, 1024 }
+ }
+
+ set s6 {
+ typeof tcp option maxseg size
+ elements = { 1, 1024 }
+ }
+
+ set s7 {
+ typeof sctp chunk init num-inbound-streams
+ elements = { 1, 4 }
+ }
+
chain c1 {
osf name @s1 accept
}
@@ -26,4 +41,16 @@ table inet t {
chain c2 {
vlan id @s2 accept
}
+
+ chain c5 {
+ ip option ra value @s5 accept
+ }
+
+ chain c6 {
+ tcp option maxseg size @s6 accept
+ }
+
+ chain c7 {
+ sctp chunk init num-inbound-streams @s7 accept
+ }
}
diff --git a/tests/shell/testcases/sets/typeof_sets_0 b/tests/shell/testcases/sets/typeof_sets_0
index a6ff8ca772e2..1e99e2987733 100755
--- a/tests/shell/testcases/sets/typeof_sets_0
+++ b/tests/shell/testcases/sets/typeof_sets_0
@@ -25,6 +25,21 @@ EXPECTED="table inet t {
elements = { 1, 1024 }
}
+ set s5 {
+ typeof ip option ra value
+ elements = { 1, 1024 }
+ }
+
+ set s6 {
+ typeof tcp option maxseg size
+ elements = { 1, 1024 }
+ }
+
+ set s7 {
+ typeof sctp chunk init num-inbound-streams
+ elements = { 1, 4 }
+ }
+
chain c1 {
osf name @s1 accept
}
@@ -32,6 +47,18 @@ EXPECTED="table inet t {
chain c2 {
ether type vlan vlan id @s2 accept
}
+
+ chain c5 {
+ ip option ra value @s5 accept
+ }
+
+ chain c6 {
+ tcp option maxseg size @s6 accept
+ }
+
+ chain c7 {
+ sctp chunk init num-inbound-streams @s7 accept
+ }
}"
set -e
--
2.32.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH nft 3/3] iptopt: fix crash with invalid field/type combo
2021-12-03 16:07 [PATCH nft 0/3] typeof fixes Florian Westphal
2021-12-03 16:07 ` [PATCH nft 1/3] ipopt: drop unused 'ptr' argument Florian Westphal
2021-12-03 16:07 ` [PATCH nft 2/3] exthdr: support ip/tcp options and sctp chunks in typeof expressions Florian Westphal
@ 2021-12-03 16:07 ` Florian Westphal
2021-12-06 20:03 ` [PATCH nft 0/3] typeof fixes Pablo Neira Ayuso
3 siblings, 0 replies; 5+ messages in thread
From: Florian Westphal @ 2021-12-03 16:07 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
% nft describe ip option rr value
segmentation fault
after this fix, this exits with 'Error: unknown ip option type/field'.
Problem is that 'rr' doesn't have a value template, so the template struct is
all-zeroes, so we crash when trying to use tmpl->dtype (its NULL).
Furthermore, expr_describe tries to print expr->identifier but expr is
exthdr, not symbol: ->identifier contains garbage.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/expression.c | 8 ++++----
src/ipopt.c | 3 +++
src/parser_bison.y | 4 ++++
tests/shell/testcases/parsing/describe | 7 +++++++
4 files changed, 18 insertions(+), 4 deletions(-)
create mode 100755 tests/shell/testcases/parsing/describe
diff --git a/src/expression.c b/src/expression.c
index 4c0874fe9950..f1cca8845376 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -135,12 +135,12 @@ void expr_describe(const struct expr *expr, struct output_ctx *octx)
nft_print(octx, "datatype %s (%s)",
dtype->name, dtype->desc);
len = dtype->size;
- } else if (dtype != &invalid_type) {
+ } else {
nft_print(octx, "%s expression, datatype %s (%s)",
expr_name(expr), dtype->name, dtype->desc);
- } else {
- nft_print(octx, "datatype %s is invalid\n", expr->identifier);
- return;
+
+ if (dtype == &invalid_type)
+ return;
}
if (dtype->basetype != NULL) {
diff --git a/src/ipopt.c b/src/ipopt.c
index 42ea41cd705b..67e904ff3d88 100644
--- a/src/ipopt.c
+++ b/src/ipopt.c
@@ -78,6 +78,9 @@ struct expr *ipopt_expr_alloc(const struct location *loc, uint8_t type,
if (!tmpl)
return NULL;
+ if (!tmpl->len)
+ return NULL;
+
expr = expr_alloc(loc, EXPR_EXTHDR, tmpl->dtype,
BYTEORDER_BIG_ENDIAN, tmpl->len);
expr->exthdr.desc = desc;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 357850dececc..16607bb79bdd 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -5332,6 +5332,10 @@ ip_hdr_expr : IP ip_hdr_field close_scope_ip
| IP OPTION ip_option_type ip_option_field close_scope_ip
{
$$ = ipopt_expr_alloc(&@$, $3, $4);
+ if (!$$) {
+ erec_queue(error(&@1, "unknown ip option type/field"), state->msgs);
+ YYERROR;
+ }
}
| IP OPTION ip_option_type close_scope_ip
{
diff --git a/tests/shell/testcases/parsing/describe b/tests/shell/testcases/parsing/describe
new file mode 100755
index 000000000000..2ee072e820fd
--- /dev/null
+++ b/tests/shell/testcases/parsing/describe
@@ -0,0 +1,7 @@
+#!/bin/bash
+
+errmsg='Error: unknown ip option type/field'
+
+str=$($NFT describe ip option rr value 2>&1 | head -n 1)
+
+[ "$str" = "$errmsg" ] && exit 0
--
2.32.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH nft 0/3] typeof fixes
2021-12-03 16:07 [PATCH nft 0/3] typeof fixes Florian Westphal
` (2 preceding siblings ...)
2021-12-03 16:07 ` [PATCH nft 3/3] iptopt: fix crash with invalid field/type combo Florian Westphal
@ 2021-12-06 20:03 ` Pablo Neira Ayuso
3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2021-12-06 20:03 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Fri, Dec 03, 2021 at 05:07:52PM +0100, Florian Westphal wrote:
> First patch removes unused code fropm ipopt.
> Second patch adds missing udata support for tcp/ip options and sctp
> chunks.
> Third patch fixes a crash in nft describe.
LGTM, thanks
^ permalink raw reply [flat|nested] 5+ messages in thread