netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: <netfilter-devel@vger.kernel.org>
Cc: Florian Westphal <fw@strlen.de>
Subject: [PATCH nft 6/8] payload: keep dependencies that enforce a specific l3 protocol
Date: Fri, 27 Oct 2017 01:06:09 +0200	[thread overview]
Message-ID: <20171026230611.14269-7-fw@strlen.de> (raw)
In-Reply-To: <20171026230611.14269-1-fw@strlen.de>

This change makes dependency removal consider both the type
of the dependency and base of the dependency (linklayer, network).

For icmp, we allow removal as it implies ipv4 even if dependency
'ip protocol' rather than 'meta l4proto', for ipv4 these are the
same.

For ipv6, we do not do this, as 'ip6 nexthdr' does not skip extension
headers.

Because the default is changed to 'keep dependency', this will result
in a ton of test case warnings, we fix them up by allowing more
dependency removals in followup patches.

Most warnings occur in inet table as we no longer remove 'meta nfproto',
even if it is redundant.  Example:

inet test-inet input ip6 saddr 1234:1234:1234:1234:1234:1234:1234:1234'
will be shown as
meta nfproto ipv6 ip6 saddr 1...

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/payload.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/src/payload.c b/src/payload.c
index 9cb8c6144d70..184a611704ea 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -469,7 +469,8 @@ static enum proto_bases expr_to_base(const struct expr *expr)
 }
 
 static bool get_relop_base(const struct stmt *stmt,
-			   enum proto_bases *base)
+			   enum proto_bases *base,
+			   unsigned int *type)
 {
 	const struct expr *lhs, *rel;
 
@@ -485,6 +486,7 @@ static bool get_relop_base(const struct stmt *stmt,
 		return false;
 
 	*base = expr_to_base(lhs);
+	*type = lhs->ops->type;
 	return *base != PROTO_BASE_INVALID;
 }
 
@@ -510,11 +512,12 @@ static bool pdep_is_redundant(struct payload_dep_ctx *pdctx,
 	const struct stmt *stmt = pdctx->pdep;
 	unsigned int family = pctx->family;
 	enum proto_bases depbase;
+	unsigned int type;
 
 	if (family == NFPROTO_IPV4 || family == NFPROTO_IPV6)
 		return true;
 
-	if (!get_relop_base(stmt, &depbase))
+	if (!get_relop_base(stmt, &depbase, &type))
 		return true;
 
 	proto = pctx->protocol[depbase].desc;
@@ -522,7 +525,43 @@ static bool pdep_is_redundant(struct payload_dep_ctx *pdctx,
 	if (proto == proto_upper)
 		return true;
 
-	return true;
+	switch (depbase) {
+	case PROTO_BASE_NETWORK_HDR:
+		/* if pdep is meta its redundant ('meta l4proto'). */
+		if (type == EXPR_META)
+			return true;
+
+		/* exceptions: icmp implies ipv4 */
+		if (proto_upper == &proto_icmp && proto == &proto_ip)
+			return true;
+		/* no exception for &proto_icmp6: 'ip protocol' that is
+		 * handled above is NOT the same as ip6 nexthdr, due to
+		 * extension headers in ipv6.
+		 */
+		break;
+	case PROTO_BASE_LL_HDR:
+		/*
+		 * It would be nice to also remove
+		 * 'meta nfproto' in cases like
+		 * meta nfproto ipv6 icmpv6 type ..., but we can't.
+		 * problem is that we do not know the upper (l4 protocol)
+		 * as, we only have access to the next expression.
+		 *
+		 * In this case, that would be EXPR_META (meta l4proto),
+		 * but we need to know the rhs to learn the protocol.
+		 *
+		 * Just removing blindly here
+		 * (if (e->ops->type == EXPR_META) return true), would
+		 * break cases like
+		 * meta nfproto ipv6 tcp dport ..., as tcp doesn't imply
+		 * ipv4 or ipv6, unlike icmp/icmpv6.
+		 */
+		break;
+	default:
+		return true;
+	}
+
+	return false;
 }
 
 /**
-- 
2.13.6


  parent reply	other threads:[~2017-10-26 23:06 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-26 23:06 [PATCH nft 0/8] rework dependency removal Florian Westphal
2017-10-26 23:06 ` [PATCH nft 1/8] tests: adjust output to silence warnings Florian Westphal
2017-10-27 10:29   ` Pablo Neira Ayuso
2017-10-27 12:41     ` Florian Westphal
2017-10-27 12:52     ` Florian Westphal
2017-10-27 14:07       ` Pablo Neira Ayuso
2017-10-27 18:03         ` Florian Westphal
2017-10-26 23:06 ` [PATCH nft 2/8] src: remove exthdr_dependency_kill Florian Westphal
2017-10-26 23:06 ` [PATCH nft 3/8] src: add and use payload_dependency_update helper Florian Westphal
2017-10-26 23:06 ` [PATCH nft 4/8] src: pass proto_ctx to payload_dependency_kill Florian Westphal
2017-10-26 23:06 ` [PATCH nft 5/8] payload: add basic infrastructure to keep some dependencies Florian Westphal
2017-10-26 23:06 ` Florian Westphal [this message]
2017-10-26 23:06 ` [PATCH nft 7/8] payload: consider expression type during dependency removal Florian Westphal
2017-10-26 23:06 ` [PATCH nft 8/8] tests: silence test case Florian Westphal
2017-10-27 10:39 ` [PATCH nft 0/8] rework dependency removal Pablo Neira Ayuso
2017-10-27 12:46   ` Florian Westphal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171026230611.14269-7-fw@strlen.de \
    --to=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).