conntrackd: simplify expectation filtering

This patch simplifies the expectation filtering by looking up for the
master conntrack. If it does not exists, then we assume that we don't
want this expectation either.

This simplification also fixes the current broken expectation filtering,
since the master conntrack from expectations has neither reply tuple
nor state, however, the filtering code assumes the opposite.

This partially reverts (479a37a conntrackd: fix crash with IPv6 expectation
in the filtering code) since it was incorrectly setting the reply tuple
of the master conntrack.

Thanks to Bill Fink for providing feedback to resolve this issue.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

7 files changed, 82 insertions, 30 deletions

diff --git a/include/filter.h b/include/filter.h
index 3c7c8cc..d0acd96 100644
--- a/include/filter.h
+++ b/include/filter.h
@@ -51,6 +51,7 @@ void ct_filter_set_logic(struct ct_filter *f,
 			 enum ct_filter_type type,
 			 enum ct_filter_logic logic);
 int ct_filter_conntrack(const struct nf_conntrack *ct, int userspace);
+int ct_filter_master(const struct nf_conntrack *master);
 
 struct exp_filter;
 struct nf_expect;
diff --git a/include/internal.h b/include/internal.h
index 2ba9714..1a796a7 100644
--- a/include/internal.h
+++ b/include/internal.h
@@ -40,6 +40,7 @@ struct internal_handler {
 		void	(*new)(struct nf_expect *exp, int origin_type);
 		void	(*upd)(struct nf_expect *exp, int origin_type);
 		int	(*del)(struct nf_expect *exp, int origin_type);
+		int	(*find)(const struct nf_conntrack *master);
 
 		void	(*dump)(int fd, int type);
 		void	(*populate)(struct nf_expect *exp);
diff --git a/src/cache-ct.c b/src/cache-ct.c
index a538215..f86d143 100644
--- a/src/cache-ct.c
+++ b/src/cache-ct.c
@@ -88,14 +88,21 @@ cache_ct_hash(const void *data, const struct hashtable *table)
 	return ret;
 }
 
+/* master conntrack of expectations have no ID */
+static inline int
+cache_ct_cmp_id(const struct nf_conntrack *ct1, const struct nf_conntrack *ct2)
+{
+	return nfct_attr_is_set(ct2, ATTR_ID) ?
+	       nfct_get_attr_u32(ct1, ATTR_ID) == nfct_get_attr_u32(ct2, ATTR_ID) : 1;
+}
+
 static int cache_ct_cmp(const void *data1, const void *data2)
 {
 	const struct cache_object *obj = data1;
 	const struct nf_conntrack *ct = data2;
 
 	return nfct_cmp(obj->ptr, ct, NFCT_CMP_ORIG) &&
-	       nfct_get_attr_u32(obj->ptr, ATTR_ID) ==
-	       nfct_get_attr_u32(ct, ATTR_ID);
+	       cache_ct_cmp_id(obj->ptr, ct);
 }
 
 static void *cache_ct_alloc(void)
diff --git a/src/ctnl.c b/src/ctnl.c
index 9e1cfa1..10b5f4c 100644
--- a/src/ctnl.c
+++ b/src/ctnl.c
@@ -211,35 +211,14 @@ out:
 		return NFCT_CB_CONTINUE;
 }
 
-static const struct nf_conntrack *exp_get_master_ct(struct nf_expect *exp)
-{
-	struct nf_conntrack *master =
-		(struct nf_conntrack *)nfexp_get_attr(exp, ATTR_EXP_MASTER);
-
-	/* The function ct_filter_conntrack needs the source address of the
-	 * reply tuple, emulate it.
-	 */
-	switch (nfct_get_attr_u8(master, ATTR_L3PROTO)) {
-	case AF_INET:
-		nfct_set_attr_u32(master, ATTR_REPL_IPV4_SRC,
-				  nfct_get_attr_u32(master, ATTR_IPV4_DST));
-		break;
-	case AF_INET6:
-		nfct_set_attr(master, ATTR_REPL_IPV6_SRC,
-			      nfct_get_attr(master, ATTR_IPV6_DST));
-		break;
-	}
-
-	return master;
-}
-
 static int exp_event_handler(const struct nlmsghdr *nlh,
 			     enum nf_conntrack_msg_type type,
 			     struct nf_expect *exp,
 			     void *data)
 {
 	int origin_type;
-	const struct nf_conntrack *master = exp_get_master_ct(exp);
+	const struct nf_conntrack *master =
+		nfexp_get_attr(exp, ATTR_EXP_MASTER);
 
 	STATE(stats).nl_events_received++;
 
@@ -247,7 +226,7 @@ static int exp_event_handler(const struct nlmsghdr *nlh,
 		STATE(stats).nl_events_filtered++;
 		goto out;
 	}
-	if (ct_filter_conntrack(master, 1))
+	if (ct_filter_master(master))
 		return NFCT_CB_CONTINUE;
 
 	origin_type = origin_find(nlh);
@@ -296,12 +275,13 @@ static int dump_handler(enum nf_conntrack_msg_type type,
 static int exp_dump_handler(enum nf_conntrack_msg_type type,
 			    struct nf_expect *exp, void *data)
 {
-	const struct nf_conntrack *master = exp_get_master_ct(exp);
+	const struct nf_conntrack *master =
+		nfexp_get_attr(exp, ATTR_EXP_MASTER);
 
 	if (!exp_filter_find(STATE(exp_filter), exp))
 		return NFCT_CB_CONTINUE;
 
-	if (ct_filter_conntrack(master, 1))
+	if (ct_filter_master(master))
 		return NFCT_CB_CONTINUE;
 
 	switch(type) {
@@ -329,12 +309,13 @@ static int get_handler(enum nf_conntrack_msg_type type,
 static int exp_get_handler(enum nf_conntrack_msg_type type,
 			   struct nf_expect *exp, void *data)
 {
-	const struct nf_conntrack *master = exp_get_master_ct(exp);
+	const struct nf_conntrack *master =
+		nfexp_get_attr(exp, ATTR_EXP_MASTER);
 
 	if (!exp_filter_find(STATE(exp_filter), exp))
 		return NFCT_CB_CONTINUE;
 
-	if (ct_filter_conntrack(master, 1))
+	if (ct_filter_master(master))
 		return NFCT_CB_CONTINUE;
 
 	STATE(get_retval) = 1;
diff --git a/src/filter.c b/src/filter.c
index e21cfde..8fac71b 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -407,6 +407,51 @@ int ct_filter_conntrack(const struct nf_conntrack *ct, int userspace)
 	return 0;
 }
 
+static inline int
+ct_filter_master_sanity_check(const struct nf_conntrack *master)
+{
+	if (master == NULL) {
+		dlog(LOG_ERR, "no master tuple in expectation");
+		return 0;
+	}
+
+	if (!nfct_attr_is_set(master, ATTR_L3PROTO)) {
+		dlog(LOG_ERR, "missing layer 3 protocol");
+		return 0;
+	}
+
+	switch (nfct_get_attr_u8(master, ATTR_L3PROTO)) {
+	case AF_INET:
+		if (!nfct_attr_is_set(master, ATTR_IPV4_SRC) ||
+		    !nfct_attr_is_set(master, ATTR_IPV4_DST)) {
+		    	dlog(LOG_ERR, "missing IPv4 address. "
+			     "You forgot to load nf_conntrack_ipv4?");
+			return 0;
+		}
+		break;
+	case AF_INET6:
+		if (!nfct_attr_is_set(master, ATTR_IPV6_SRC) ||
+		    !nfct_attr_is_set(master, ATTR_IPV6_DST)) {
+		    	dlog(LOG_ERR, "missing IPv6 address. "
+			     "You forgot to load nf_conntrack_ipv6?");
+			return 0;
+		}
+		break;
+	}
+	return 1;
+}
+
+int ct_filter_master(const struct nf_conntrack *master)
+{
+	if (!ct_filter_master_sanity_check(master))
+		return 1;
+
+	/* Check if we've got a master conntrack for this expectation in our
+	 * caches. If there is not, we don't want this expectation either.
+	 */
+	return STATE(mode)->internal->exp.find(master) ? 0 : 1;
+}
+
 struct exp_filter {
 	struct list_head 	list;
 };
diff --git a/src/internal_bypass.c b/src/internal_bypass.c
index 1194339..ce2ae46 100644
--- a/src/internal_bypass.c
+++ b/src/internal_bypass.c
@@ -283,6 +283,11 @@ static int internal_bypass_exp_event_del(struct nf_expect *exp, int origin)
 	return 1;
 }
 
+static int internal_bypass_exp_master_find(const struct nf_conntrack *master)
+{
+	return nl_get_conntrack(STATE(get), master) == 0;
+}
+
 struct internal_handler internal_bypass = {
 	.init			= internal_bypass_init,
 	.close			= internal_bypass_close,
@@ -309,5 +314,6 @@ struct internal_handler internal_bypass = {
 		.new			= internal_bypass_exp_event_new,
 		.upd			= internal_bypass_exp_event_upd,
 		.del			= internal_bypass_exp_event_del,
+		.find			= internal_bypass_exp_master_find,
 	},
 };
diff --git a/src/internal_cache.c b/src/internal_cache.c
index ba2d74b..bad31f3 100644
--- a/src/internal_cache.c
+++ b/src/internal_cache.c
@@ -364,6 +364,16 @@ static int internal_cache_exp_event_del(struct nf_expect *exp, int origin)
 	return 1;
 }
 
+static int internal_cache_exp_master_find(const struct nf_conntrack *master)
+{
+	struct cache_object *obj;
+	int id;
+
+	obj = cache_find(STATE(mode)->internal->ct.data,
+			 (struct nf_conntrack *)master, &id);
+	return obj ? 1 : 0;
+}
+
 struct internal_handler internal_cache = {
 	.flags			= INTERNAL_F_POPULATE | INTERNAL_F_RESYNC,
 	.init			= internal_cache_init,
@@ -391,5 +401,6 @@ struct internal_handler internal_cache = {
 		.new			= internal_cache_exp_event_new,
 		.upd			= internal_cache_exp_event_upd,
 		.del			= internal_cache_exp_event_del,
+		.find			= internal_cache_exp_master_find,
 	},
 };