From patchwork Mon Jun 14 13:10:04 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Westphal X-Patchwork-Id: 1491661 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4G3Wyh1XStz9sW7 for ; Mon, 14 Jun 2021 23:10:20 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233575AbhFNNMV (ORCPT ); Mon, 14 Jun 2021 09:12:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38738 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233218AbhFNNMU (ORCPT ); Mon, 14 Jun 2021 09:12:20 -0400 Received: from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc [IPv6:2a0a:51c0:0:12e:520::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3FC84C061574 for ; Mon, 14 Jun 2021 06:10:18 -0700 (PDT) Received: from fw by Chamillionaire.breakpoint.cc with local (Exim 4.92) (envelope-from ) id 1lsmM4-0000Le-OC; Mon, 14 Jun 2021 15:10:16 +0200 From: Florian Westphal To: Cc: Florian Westphal Subject: [PATCH nft 1/3] netlink_delinearize: add missing icmp id/sequence support Date: Mon, 14 Jun 2021 15:10:04 +0200 Message-Id: <20210614131006.26490-2-fw@strlen.de> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210614131006.26490-1-fw@strlen.de> References: <20210614131006.26490-1-fw@strlen.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Pablo reports following input and output: in: icmpv6 id 1 out: icmpv6 type { echo-request, echo-reply } icmpv6 parameter-problem 65536/16 Reason is that icmp fields overlap, decoding of the correct name requires check of the icmpv6 type. This only works for equality tests, for instance in: icmpv6 type echo-request icmpv6 id 1 will be listed as "icmpv6 id 1" (which is not correct either, since the input only matches on echo-request). with this patch, output of 'icmpv6 id 1' is icmpv6 type { echo-request, echo-reply } icmpv6 id 1 The second problem, the removal of a single check (request OR reply), is resolved in the followup patch. Signed-off-by: Florian Westphal --- src/netlink_delinearize.c | 66 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index 9a1cf3c4f7d9..94f68c956018 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -1819,9 +1819,6 @@ static void payload_match_expand(struct rule_pp_ctx *ctx, enum proto_bases base = left->payload.base; bool stacked; - if (ctx->pdctx.icmp_type) - ctx->pctx.th_dep.icmp.type = ctx->pdctx.icmp_type; - payload_expr_expand(&list, left, &ctx->pctx); list_for_each_entry(left, &list, list) { @@ -1868,6 +1865,61 @@ static void payload_match_expand(struct rule_pp_ctx *ctx, ctx->stmt = NULL; } +static void payload_icmp_check(struct rule_pp_ctx *rctx, struct expr *expr, const struct expr *value) +{ + const struct proto_hdr_template *tmpl; + const struct proto_desc *desc; + uint8_t icmp_type; + unsigned int i; + + assert(expr->etype == EXPR_PAYLOAD); + assert(value->etype == EXPR_VALUE); + + if (rctx->pctx.th_dep.icmp.type) + return; + + /* icmp(v6) type is 8 bit, if value is smaller or larger, this is not + * a protocol dependency. + */ + if (value->len != 8) + return; + + if (expr->payload.base != PROTO_BASE_TRANSPORT_HDR) + return; + + desc = rctx->pctx.protocol[expr->payload.base].desc; + if (desc == NULL) + return; + + /* not icmp? ignore. */ + if (desc != &proto_icmp && desc != &proto_icmp6) + return; + + assert(desc->base == expr->payload.base); + + icmp_type = mpz_get_uint8(value->value); + + for (i = 1; i < array_size(desc->templates); i++) { + tmpl = &desc->templates[i]; + + if (tmpl->len == 0) + return; + + if (tmpl->offset != expr->payload.offset || + tmpl->len != expr->len) + continue; + + /* Matches but doesn't load a protocol key -> ignore. */ + if (desc->protocol_key != i) + return; + + expr->payload.desc = desc; + expr->payload.tmpl = tmpl; + rctx->pctx.th_dep.icmp.type = icmp_type; + return; + } +} + static void payload_match_postprocess(struct rule_pp_ctx *ctx, struct expr *expr, struct expr *payload) @@ -1883,6 +1935,14 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx, if (expr->right->etype == EXPR_VALUE) { payload_match_expand(ctx, expr, payload); break; + } else if (expr->right->etype == EXPR_SET_REF) { + struct expr *elem; + + elem = list_first_entry(&expr->right->set->init->expressions, struct expr, list); + + if (elem->etype == EXPR_SET_ELEM && + elem->key->etype == EXPR_VALUE) + payload_icmp_check(ctx, payload, elem->key); } /* Fall through */ default: From patchwork Mon Jun 14 13:10:05 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Westphal X-Patchwork-Id: 1491662 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4G3Wyp5vxFz9sVb for ; Mon, 14 Jun 2021 23:10:26 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233590AbhFNNM2 (ORCPT ); Mon, 14 Jun 2021 09:12:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38754 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233218AbhFNNMZ (ORCPT ); Mon, 14 Jun 2021 09:12:25 -0400 Received: from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc [IPv6:2a0a:51c0:0:12e:520::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7E2B5C061574 for ; Mon, 14 Jun 2021 06:10:22 -0700 (PDT) Received: from fw by Chamillionaire.breakpoint.cc with local (Exim 4.92) (envelope-from ) id 1lsmM8-0000Lp-UP; Mon, 14 Jun 2021 15:10:20 +0200 From: Florian Westphal To: Cc: Florian Westphal Subject: [PATCH nft 2/3] payload: do not remove icmp echo dependency Date: Mon, 14 Jun 2021 15:10:05 +0200 Message-Id: <20210614131006.26490-3-fw@strlen.de> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210614131006.26490-1-fw@strlen.de> References: <20210614131006.26490-1-fw@strlen.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org "icmp type echo-request icmp id 2" and "icmp id 2" are not the same, the latter gains an implicit dependency on both echo-request and echo-reply. Change payload dependency tracking to not store dependency in case the value type is ICMP(6)_ECHO(REPLY). Signed-off-by: Florian Westphal --- src/payload.c | 61 +++++++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/src/payload.c b/src/payload.c index cfa952248a15..97b60713e800 100644 --- a/src/payload.c +++ b/src/payload.c @@ -98,12 +98,16 @@ static void payload_expr_pctx_update(struct proto_ctx *ctx, desc = proto_find_upper(base, proto); if (!desc) { - if (base == &proto_icmp || base == &proto_icmp6) { + if (base == &proto_icmp) { /* proto 0 is ECHOREPLY, just pretend its ECHO. * Not doing this would need an additional marker * bit to tell when icmp.type was set. */ ctx->th_dep.icmp.type = proto ? proto : ICMP_ECHO; + } else if (base == &proto_icmp6) { + if (proto == ICMP6_ECHO_REPLY) + proto = ICMP6_ECHO_REQUEST; + ctx->th_dep.icmp.type = proto; } return; } @@ -554,33 +558,39 @@ void payload_dependency_reset(struct payload_dep_ctx *ctx) memset(ctx, 0, sizeof(*ctx)); } -static uint8_t icmp_get_type(const struct proto_desc *desc, uint8_t value) +static bool payload_dependency_store_icmp_type(struct payload_dep_ctx *ctx, + const struct stmt *stmt) { - if (desc == &proto_icmp && value == 0) - return ICMP_ECHO; + struct expr *dep = stmt->expr; + const struct proto_desc *desc; + const struct expr *right; + uint8_t type; - return value; -} + if (dep->left->etype != EXPR_PAYLOAD) + return false; -static uint8_t icmp_get_dep_type(const struct proto_desc *desc, struct expr *right) -{ - if (right->etype == EXPR_VALUE && right->len == BITS_PER_BYTE) - return icmp_get_type(desc, mpz_get_uint8(right->value)); + right = dep->right; + if (right->etype != EXPR_VALUE || right->len != BITS_PER_BYTE) + return false; - return 0; -} + desc = dep->left->payload.desc; + if (desc == &proto_icmp) { + type = mpz_get_uint8(right->value); -static void payload_dependency_store_icmp_type(struct payload_dep_ctx *ctx) -{ - struct expr *dep = ctx->pdep->expr; - const struct proto_desc *desc; + if (type == ICMP_ECHOREPLY) + type = ICMP_ECHO; - if (dep->left->etype != EXPR_PAYLOAD) - return; + ctx->icmp_type = type; - desc = dep->left->payload.desc; - if (desc == &proto_icmp || desc == &proto_icmp6) - ctx->icmp_type = icmp_get_dep_type(dep->left->payload.desc, dep->right); + return type == ICMP_ECHO; + } else if (desc == &proto_icmp6) { + type = mpz_get_uint8(right->value); + + ctx->icmp_type = type; + return type == ICMP6_ECHO_REQUEST || type == ICMP6_ECHO_REPLY; + } + + return false; } /** @@ -593,10 +603,13 @@ static void payload_dependency_store_icmp_type(struct payload_dep_ctx *ctx) void payload_dependency_store(struct payload_dep_ctx *ctx, struct stmt *stmt, enum proto_bases base) { - ctx->pbase = base + 1; - ctx->pdep = stmt; + bool ignore_dep = payload_dependency_store_icmp_type(ctx, stmt); + + if (ignore_dep) + return; - payload_dependency_store_icmp_type(ctx); + ctx->pdep = stmt; + ctx->pbase = base + 1; } /** From patchwork Mon Jun 14 13:10:06 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Westphal X-Patchwork-Id: 1491663 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 4G3Wyq6kXxz9sW7 for ; Mon, 14 Jun 2021 23:10:27 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233587AbhFNNMa (ORCPT ); Mon, 14 Jun 2021 09:12:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38778 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233218AbhFNNM3 (ORCPT ); Mon, 14 Jun 2021 09:12:29 -0400 Received: from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc [IPv6:2a0a:51c0:0:12e:520::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AE201C061574 for ; Mon, 14 Jun 2021 06:10:26 -0700 (PDT) Received: from fw by Chamillionaire.breakpoint.cc with local (Exim 4.92) (envelope-from ) id 1lsmMD-0000MA-5U; Mon, 14 Jun 2021 15:10:25 +0200 From: Florian Westphal To: Cc: Florian Westphal Subject: [PATCH nft 3/3] tests: add a icmp-reply only and icmpv6 id test cases Date: Mon, 14 Jun 2021 15:10:06 +0200 Message-Id: <20210614131006.26490-4-fw@strlen.de> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210614131006.26490-1-fw@strlen.de> References: <20210614131006.26490-1-fw@strlen.de> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Check that nft doesn't remove the dependency in these cases: icmp type echo-reply icmp id 1 ("icmp id" matches both echo request and reply). Add icmpv6 test cases. These fail without the previous patches: add rule ip6 test-ip6 input icmpv6 id 1: 'icmpv6 id 1' mismatches 'icmpv6 type { echo-request, echo-reply} icmpv6 parameter-problem 65536/16' add rule ip6 test-ip6 input icmpv6 type echo-reply icmpv6 id 65534': 'icmpv6 type echo-reply icmpv6 id 65534' mismatches 'icmpv6 type echo-reply @th,32,16 65534' Signed-off-by: Florian Westphal --- tests/py/ip/icmp.t | 1 + tests/py/ip/icmp.t.json | 28 ++++++++++++++ tests/py/ip/icmp.t.payload.ip | 9 +++++ tests/py/ip6/icmpv6.t | 3 ++ tests/py/ip6/icmpv6.t.json | 61 +++++++++++++++++++++++++++++++ tests/py/ip6/icmpv6.t.payload.ip6 | 21 +++++++++++ 6 files changed, 123 insertions(+) diff --git a/tests/py/ip/icmp.t b/tests/py/ip/icmp.t index fd89af0dff20..7ddf8b38a538 100644 --- a/tests/py/ip/icmp.t +++ b/tests/py/ip/icmp.t @@ -53,6 +53,7 @@ icmp sequence { 33, 55, 67, 88};ok;icmp type { echo-request, echo-reply} icmp se icmp sequence != { 33, 55, 67, 88};ok;icmp type { echo-request, echo-reply} icmp sequence != { 33, 55, 67, 88} icmp id 1 icmp sequence 2;ok;icmp type { echo-reply, echo-request} icmp id 1 icmp sequence 2 icmp type { echo-reply, echo-request} icmp id 1 icmp sequence 2;ok +icmp type echo-reply icmp id 1;ok icmp mtu 33;ok icmp mtu 22-33;ok diff --git a/tests/py/ip/icmp.t.json b/tests/py/ip/icmp.t.json index 576335cc63d2..4f0525094cf0 100644 --- a/tests/py/ip/icmp.t.json +++ b/tests/py/ip/icmp.t.json @@ -1123,6 +1123,34 @@ } ] +# icmp type echo-reply icmp id 1 +[ + { + "match": { + "left": { + "payload": { + "field": "type", + "protocol": "icmp" + } + }, + "op": "==", + "right": "echo-reply" + } + }, + { + "match": { + "left": { + "payload": { + "field": "id", + "protocol": "icmp" + } + }, + "op": "==", + "right": 1 + } + } +] + # icmp mtu 33 [ { diff --git a/tests/py/ip/icmp.t.payload.ip b/tests/py/ip/icmp.t.payload.ip index 024739c0c3cc..3bc6de3cf717 100644 --- a/tests/py/ip/icmp.t.payload.ip +++ b/tests/py/ip/icmp.t.payload.ip @@ -413,6 +413,15 @@ ip [ payload load 4b @ transport header + 4 => reg 1 ] [ cmp eq reg 1 0x02000100 ] +# icmp type echo-reply icmp id 1 +ip + [ meta load l4proto => reg 1 ] + [ cmp eq reg 1 0x00000001 ] + [ payload load 1b @ transport header + 0 => reg 1 ] + [ cmp eq reg 1 0x00000000 ] + [ payload load 2b @ transport header + 4 => reg 1 ] + [ cmp eq reg 1 0x00000100 ] + # icmp mtu 33 ip test-ip4 input [ meta load l4proto => reg 1 ] diff --git a/tests/py/ip6/icmpv6.t b/tests/py/ip6/icmpv6.t index c8d4cffcd9d7..4de6ee2377dd 100644 --- a/tests/py/ip6/icmpv6.t +++ b/tests/py/ip6/icmpv6.t @@ -67,6 +67,9 @@ icmpv6 id != 33-45;ok;icmpv6 type { echo-request, echo-reply} icmpv6 id != 33-45 icmpv6 id {33, 55, 67, 88};ok;icmpv6 type { echo-request, echo-reply} icmpv6 id { 33, 55, 67, 88} icmpv6 id != {33, 55, 67, 88};ok;icmpv6 type { echo-request, echo-reply} icmpv6 id != { 33, 55, 67, 88} +icmpv6 id 1;ok;icmpv6 type { echo-request, echo-reply} icmpv6 id 1 +icmpv6 type echo-reply icmpv6 id 65534;ok + icmpv6 sequence 2;ok;icmpv6 type { echo-request, echo-reply} icmpv6 sequence 2 icmpv6 sequence {3, 4, 5, 6, 7} accept;ok;icmpv6 type { echo-request, echo-reply} icmpv6 sequence { 3, 4, 5, 6, 7} accept diff --git a/tests/py/ip6/icmpv6.t.json b/tests/py/ip6/icmpv6.t.json index 30d2ad988185..2251be82a39e 100644 --- a/tests/py/ip6/icmpv6.t.json +++ b/tests/py/ip6/icmpv6.t.json @@ -856,6 +856,67 @@ } ] +# icmpv6 id 1 +[ + { + "match": { + "left": { + "payload": { + "field": "type", + "protocol": "icmpv6" + } + }, + "op": "==", + "right": { + "set": [ + "echo-request", + "echo-reply" + ] + } + } + }, + { + "match": { + "left": { + "payload": { + "field": "id", + "protocol": "icmpv6" + } + }, + "op": "==", + "right": 1 + } + } +] + +# icmpv6 type echo-reply icmpv6 id 65534 +[ + { + "match": { + "left": { + "payload": { + "field": "type", + "protocol": "icmpv6" + } + }, + "op": "==", + "right": "echo-reply" + } + }, + { + "match": { + "left": { + "payload": { + "field": "id", + "protocol": "icmpv6" + } + }, + "op": "==", + "right": 65534 + } + } +] + # icmpv6 sequence 2 [ { diff --git a/tests/py/ip6/icmpv6.t.payload.ip6 b/tests/py/ip6/icmpv6.t.payload.ip6 index 76df184cd0d0..0e96be2d0788 100644 --- a/tests/py/ip6/icmpv6.t.payload.ip6 +++ b/tests/py/ip6/icmpv6.t.payload.ip6 @@ -407,6 +407,27 @@ ip6 test-ip6 input [ payload load 2b @ transport header + 4 => reg 1 ] [ lookup reg 1 set __set%d 0x1 ] +# icmpv6 id 1 +__set%d test-ip6 3 size 2 +__set%d test-ip6 0 + element 00000080 : 0 [end] element 00000081 : 0 [end] +ip6 + [ meta load l4proto => reg 1 ] + [ cmp eq reg 1 0x0000003a ] + [ payload load 1b @ transport header + 0 => reg 1 ] + [ lookup reg 1 set __set%d ] + [ payload load 2b @ transport header + 4 => reg 1 ] + [ cmp eq reg 1 0x00000100 ] + +# icmpv6 type echo-reply icmpv6 id 65534 +ip6 + [ meta load l4proto => reg 1 ] + [ cmp eq reg 1 0x0000003a ] + [ payload load 1b @ transport header + 0 => reg 1 ] + [ cmp eq reg 1 0x00000081 ] + [ payload load 2b @ transport header + 4 => reg 1 ] + [ cmp eq reg 1 0x0000feff ] + # icmpv6 sequence 2 __set%d test-ip6 3 __set%d test-ip6 0