* [PATCH 1/3] eval: refactor NAT evaluation functions @ 2015-01-10 14:59 Patrick McHardy 2015-01-10 14:59 ` [PATCH 2/3] evaluate: add missing datatype compat checks for statement arguments Patrick McHardy ` (2 more replies) 0 siblings, 3 replies; 4+ messages in thread From: Patrick McHardy @ 2015-01-10 14:59 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel The redir and masq evaluation functions include some useless context updates and checks. Refactor the NAT code to have a single instance of address and transport evaluation functions for simplicity and unified error reporting. Signed-off-by: Patrick McHardy <kaber@trash.net> --- src/evaluate.c | 110 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 54 insertions(+), 56 deletions(-) diff --git a/src/evaluate.c b/src/evaluate.c index 43fb968..4cfe749 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -1496,32 +1496,61 @@ static int stmt_evaluate_reject(struct eval_ctx *ctx, struct stmt *stmt) return stmt_evaluate_reject_family(ctx, stmt, expr); } +static int nat_evaluate_family(struct eval_ctx *ctx, struct stmt *stmt) +{ + switch (ctx->pctx.family) { + case AF_INET: + case AF_INET6: + return 0; + default: + return stmt_error(ctx, stmt, + "NAT is only supported for IPv4/IPv6"); + } +} + +static int nat_evaluate_addr(struct eval_ctx *ctx, struct stmt *stmt, + struct expr **expr) +{ + struct proto_ctx *pctx = &ctx->pctx; + + if (pctx->family == AF_INET) + expr_set_context(&ctx->ectx, &ipaddr_type, 4 * BITS_PER_BYTE); + else + expr_set_context(&ctx->ectx, &ip6addr_type, 16 * BITS_PER_BYTE); + + return expr_evaluate(ctx, expr); +} + +static int nat_evaluate_transport(struct eval_ctx *ctx, struct stmt *stmt, + struct expr **expr) +{ + struct proto_ctx *pctx = &ctx->pctx; + + if (pctx->protocol[PROTO_BASE_TRANSPORT_HDR].desc == NULL) + return stmt_binary_error(ctx, *expr, stmt, + "transport protocol mapping is only " + "valid after transport protocol match"); + + expr_set_context(&ctx->ectx, &inet_service_type, 2 * BITS_PER_BYTE); + return expr_evaluate(ctx, expr); +} + static int stmt_evaluate_nat(struct eval_ctx *ctx, struct stmt *stmt) { struct proto_ctx *pctx = &ctx->pctx; int err; + err = nat_evaluate_family(ctx, stmt); + if (err < 0) + return err; + if (stmt->nat.addr != NULL) { - if (pctx && (pctx->family == AF_INET)) - expr_set_context(&ctx->ectx, &ipaddr_type, - 4 * BITS_PER_BYTE); - else - expr_set_context(&ctx->ectx, &ip6addr_type, - 16 * BITS_PER_BYTE); - err = expr_evaluate(ctx, &stmt->nat.addr); + err = nat_evaluate_addr(ctx, stmt, &stmt->nat.addr); if (err < 0) return err; } - if (stmt->nat.proto != NULL) { - if (pctx->protocol[PROTO_BASE_TRANSPORT_HDR].desc == NULL) - return stmt_binary_error(ctx, stmt->nat.proto, stmt, - "transport protocol mapping is only " - "valid after transport protocol match"); - - expr_set_context(&ctx->ectx, &inet_service_type, - 2 * BITS_PER_BYTE); - err = expr_evaluate(ctx, &stmt->nat.proto); + err = nat_evaluate_transport(ctx, stmt, &stmt->nat.proto); if (err < 0) return err; } @@ -1533,62 +1562,31 @@ static int stmt_evaluate_nat(struct eval_ctx *ctx, struct stmt *stmt) static int stmt_evaluate_masq(struct eval_ctx *ctx, struct stmt *stmt) { struct proto_ctx *pctx = &ctx->pctx; + int err; - if (!pctx) - goto out; - - switch (pctx->family) { - case AF_INET: - expr_set_context(&ctx->ectx, &ipaddr_type, - 4 * BITS_PER_BYTE); - break; - case AF_INET6: - expr_set_context(&ctx->ectx, &ip6addr_type, - 16 * BITS_PER_BYTE); - break; - default: - return stmt_error(ctx, stmt, "ip and ip6 support only"); - } + err = nat_evaluate_family(ctx, stmt); + if (err < 0) + return err; -out: stmt->flags |= STMT_F_TERMINAL; return 0; } static int stmt_evaluate_redir(struct eval_ctx *ctx, struct stmt *stmt) { - int err; struct proto_ctx *pctx = &ctx->pctx; + int err; - if (!pctx) - goto out; - - switch (pctx->family) { - case AF_INET: - expr_set_context(&ctx->ectx, &ipaddr_type, - 4 * BITS_PER_BYTE); - break; - case AF_INET6: - expr_set_context(&ctx->ectx, &ip6addr_type, - 16 * BITS_PER_BYTE); - break; - default: - return stmt_error(ctx, stmt, "ip and ip6 support only"); - } + err = nat_evaluate_family(ctx, stmt); + if (err < 0) + return err; if (stmt->redir.proto != NULL) { - if (pctx->protocol[PROTO_BASE_TRANSPORT_HDR].desc == NULL) - return stmt_binary_error(ctx, stmt->redir.proto, stmt, - "missing transport protocol match"); - - expr_set_context(&ctx->ectx, &inet_service_type, - 2 * BITS_PER_BYTE); - err = expr_evaluate(ctx, &stmt->redir.proto); + err = nat_evaluate_transport(ctx, stmt, &stmt->redir.proto); if (err < 0) return err; } -out: stmt->flags |= STMT_F_TERMINAL; return 0; } -- 2.1.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] evaluate: add missing datatype compat checks for statement arguments 2015-01-10 14:59 [PATCH 1/3] eval: refactor NAT evaluation functions Patrick McHardy @ 2015-01-10 14:59 ` Patrick McHardy 2015-01-10 14:59 ` [PATCH 3/3] netlink_delinearize: fix error handling for invalid registers Patrick McHardy 2015-01-10 18:59 ` [PATCH 1/3] eval: refactor NAT evaluation functions Pablo Neira Ayuso 2 siblings, 0 replies; 4+ messages in thread From: Patrick McHardy @ 2015-01-10 14:59 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel Add a helper function to evaluate expressions used as arguments for statements and report datatype mismatches. Fixes acceptance of mismatching expressions like: $ nft filter output meta mark set ip daddr <cmdline>:1:29-36: Error: datatype mismatch: expected packet mark. expression has type IPv4 address filter output meta mark set ip daddr ~~~~~~~~~~~~~~^^^^^^^^ Signed-off-by: Patrick McHardy <kaber@trash.net> --- src/evaluate.c | 66 ++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/src/evaluate.c b/src/evaluate.c index 4cfe749..0a1422a 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -1112,6 +1112,22 @@ static int stmt_evaluate_expr(struct eval_ctx *ctx, struct stmt *stmt) return expr_evaluate(ctx, &stmt->expr); } +static int stmt_evaluate_arg(struct eval_ctx *ctx, struct stmt *stmt, + const struct datatype *dtype, unsigned int len, + struct expr **expr) +{ + expr_set_context(&ctx->ectx, dtype, len); + if (expr_evaluate(ctx, expr) < 0) + return -1; + + if (!datatype_equal((*expr)->dtype, dtype)) + return stmt_binary_error(ctx, *expr, stmt, + "datatype mismatch: expected %s. " + "expression has type %s", + dtype->desc, (*expr)->dtype->desc); + return 0; +} + static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt) { expr_set_context(&ctx->ectx, &verdict_type, 0); @@ -1133,11 +1149,18 @@ static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt) static int stmt_evaluate_meta(struct eval_ctx *ctx, struct stmt *stmt) { - expr_set_context(&ctx->ectx, stmt->meta.tmpl->dtype, - stmt->meta.tmpl->len); - if (expr_evaluate(ctx, &stmt->meta.expr) < 0) - return -1; - return 0; + return stmt_evaluate_arg(ctx, stmt, + stmt->meta.tmpl->dtype, + stmt->meta.tmpl->len, + &stmt->meta.expr); +} + +static int stmt_evaluate_ct(struct eval_ctx *ctx, struct stmt *stmt) +{ + return stmt_evaluate_arg(ctx, stmt, + stmt->ct.tmpl->dtype, + stmt->ct.tmpl->len, + &stmt->ct.expr); } static int reject_payload_gen_dependency_tcp(struct eval_ctx *ctx, @@ -1512,13 +1535,18 @@ static int nat_evaluate_addr(struct eval_ctx *ctx, struct stmt *stmt, struct expr **expr) { struct proto_ctx *pctx = &ctx->pctx; + const struct datatype *dtype; + unsigned int len; - if (pctx->family == AF_INET) - expr_set_context(&ctx->ectx, &ipaddr_type, 4 * BITS_PER_BYTE); - else - expr_set_context(&ctx->ectx, &ip6addr_type, 16 * BITS_PER_BYTE); + if (pctx->family == AF_INET) { + dtype = &ipaddr_type; + len = 4 * BITS_PER_BYTE; + } else { + dtype = &ip6addr_type; + len = 16 * BITS_PER_BYTE; + } - return expr_evaluate(ctx, expr); + return stmt_evaluate_arg(ctx, stmt, dtype, len, expr); } static int nat_evaluate_transport(struct eval_ctx *ctx, struct stmt *stmt, @@ -1531,8 +1559,9 @@ static int nat_evaluate_transport(struct eval_ctx *ctx, struct stmt *stmt, "transport protocol mapping is only " "valid after transport protocol match"); - expr_set_context(&ctx->ectx, &inet_service_type, 2 * BITS_PER_BYTE); - return expr_evaluate(ctx, expr); + return stmt_evaluate_arg(ctx, stmt, + &inet_service_type, 2 * BITS_PER_BYTE, + expr); } static int stmt_evaluate_nat(struct eval_ctx *ctx, struct stmt *stmt) @@ -1591,15 +1620,6 @@ static int stmt_evaluate_redir(struct eval_ctx *ctx, struct stmt *stmt) return 0; } -static int stmt_evaluate_ct(struct eval_ctx *ctx, struct stmt *stmt) -{ - expr_set_context(&ctx->ectx, stmt->ct.tmpl->dtype, - stmt->ct.tmpl->len); - if (expr_evaluate(ctx, &stmt->ct.expr) < 0) - return -1; - return 0; -} - static int stmt_evaluate_queue(struct eval_ctx *ctx, struct stmt *stmt) { if (stmt->queue.queue != NULL) { @@ -1645,6 +1665,8 @@ int stmt_evaluate(struct eval_ctx *ctx, struct stmt *stmt) return stmt_evaluate_verdict(ctx, stmt); case STMT_META: return stmt_evaluate_meta(ctx, stmt); + case STMT_CT: + return stmt_evaluate_ct(ctx, stmt); case STMT_LOG: return stmt_evaluate_log(ctx, stmt); case STMT_REJECT: @@ -1657,8 +1679,6 @@ int stmt_evaluate(struct eval_ctx *ctx, struct stmt *stmt) return stmt_evaluate_redir(ctx, stmt); case STMT_QUEUE: return stmt_evaluate_queue(ctx, stmt); - case STMT_CT: - return stmt_evaluate_ct(ctx, stmt); default: BUG("unknown statement type %s\n", stmt->ops->name); } -- 2.1.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] netlink_delinearize: fix error handling for invalid registers 2015-01-10 14:59 [PATCH 1/3] eval: refactor NAT evaluation functions Patrick McHardy 2015-01-10 14:59 ` [PATCH 2/3] evaluate: add missing datatype compat checks for statement arguments Patrick McHardy @ 2015-01-10 14:59 ` Patrick McHardy 2015-01-10 18:59 ` [PATCH 1/3] eval: refactor NAT evaluation functions Pablo Neira Ayuso 2 siblings, 0 replies; 4+ messages in thread From: Patrick McHardy @ 2015-01-10 14:59 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel netlink_delinearize is prepared to deal with malformed expressions from the kernel that it doesn't understand. However since expressions are now cloned unconditionally by netlink_get_register(), we crash before such errors can be detected for invalid inputs. Fix by only cloning non-NULL expressions. Signed-off-by: Patrick McHardy <kaber@trash.net> --- src/netlink_delinearize.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index e9a04dd..79d5af6 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -75,7 +75,10 @@ static struct expr *netlink_get_register(struct netlink_parse_ctx *ctx, } expr = ctx->registers[reg]; - return expr_clone(expr); + if (expr != NULL) + expr = expr_clone(expr); + + return expr; } static void netlink_release_registers(struct netlink_parse_ctx *ctx) -- 2.1.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/3] eval: refactor NAT evaluation functions 2015-01-10 14:59 [PATCH 1/3] eval: refactor NAT evaluation functions Patrick McHardy 2015-01-10 14:59 ` [PATCH 2/3] evaluate: add missing datatype compat checks for statement arguments Patrick McHardy 2015-01-10 14:59 ` [PATCH 3/3] netlink_delinearize: fix error handling for invalid registers Patrick McHardy @ 2015-01-10 18:59 ` Pablo Neira Ayuso 2 siblings, 0 replies; 4+ messages in thread From: Pablo Neira Ayuso @ 2015-01-10 18:59 UTC (permalink / raw) To: Patrick McHardy; +Cc: netfilter-devel On Sat, Jan 10, 2015 at 02:59:13PM +0000, Patrick McHardy wrote: > The redir and masq evaluation functions include some useless context > updates and checks. > > Refactor the NAT code to have a single instance of address and transport > evaluation functions for simplicity and unified error reporting. Thanks a lot for revisiting these changes Patrick! ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-10 18:56 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-10 14:59 [PATCH 1/3] eval: refactor NAT evaluation functions Patrick McHardy 2015-01-10 14:59 ` [PATCH 2/3] evaluate: add missing datatype compat checks for statement arguments Patrick McHardy 2015-01-10 14:59 ` [PATCH 3/3] netlink_delinearize: fix error handling for invalid registers Patrick McHardy 2015-01-10 18:59 ` [PATCH 1/3] eval: refactor NAT evaluation functions Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).