[PATCH conntrack 1/5] conntrack: support delete by label

[PATCH conntrack 1/5] conntrack: support delete by label

[Asbjørn Sloth Tønnesen]

This option was already silently allowed by 991fc4ae,
but didn't have any effect.

This patch adds the check and documents it.

Cc: Clemence Faure <clemence.faure@sophos.com>
Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
---

Notes:
    I tried to create a test case, as well but I didn't
    seam to be able to get --label-add to work with
    create.

 conntrack.8     | 2 +-
 src/conntrack.c | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/conntrack.8 b/conntrack.8
index a981a76..970c2d7 100644
--- a/conntrack.8
+++ b/conntrack.8
@@ -146,7 +146,7 @@ In "\-\-create" mode, the mask is ignored.
 .TP
 .BI "-l, --label " "LABEL"
 Specify a conntrack label.
-This option is only available in conjunction with "\-L, \-\-dump", "\-E, \-\-event", or "\-U \-\-update".
+This option is only available in conjunction with "\-L, \-\-dump", "\-E, \-\-event", "\-U \-\-update" or "\-D \-\-delete".
 Match entries whose labels match at least those specified.
 Use multiple \-l commands to specify multiple labels that need to be set.
 Match entries whose labels matches at least those specified as arguments.
diff --git a/src/conntrack.c b/src/conntrack.c
index 45b8822..b5a0a13 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -1355,6 +1355,9 @@ static int delete_cb(enum nf_conntrack_msg_type type,
 	if (filter_mark(ct))
 		return NFCT_CB_CONTINUE;
 
+	if (filter_label(ct))
+		return NFCT_CB_CONTINUE;
+
 	if (options & CT_COMPARISON &&
 	    !nfct_cmp(obj, ct, NFCT_CMP_ALL | NFCT_CMP_MASK))
 		return NFCT_CB_CONTINUE;
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH conntrack 2/5] conntrack: consolidate filtering

[Asbjørn Sloth Tønnesen]

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
---
 src/conntrack.c | 57 ++++++++++++++++++++-------------------------------------
 1 file changed, 20 insertions(+), 37 deletions(-)

diff --git a/src/conntrack.c b/src/conntrack.c
index b5a0a13..cb9545b 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -1208,6 +1208,21 @@ filter_nat(const struct nf_conntrack *obj, const struct nf_conntrack *ct)
 	return 0;
 }
 
+static int
+nfct_filter(struct nf_conntrack *obj, struct nf_conntrack *ct)
+{
+	if (filter_nat(obj, ct) ||
+	    filter_mark(ct) ||
+	    filter_label(ct))
+		return 1;
+
+	if (options & CT_COMPARISON &&
+	    !nfct_cmp(obj, ct, NFCT_CMP_ALL | NFCT_CMP_MASK))
+		return 1;
+
+	return 0;
+}
+
 static int counter;
 static int dump_xml_header_done = 1;
 
@@ -1248,17 +1263,7 @@ static int event_cb(enum nf_conntrack_msg_type type,
 	unsigned int op_type = NFCT_O_DEFAULT;
 	unsigned int op_flags = 0;
 
-	if (filter_nat(obj, ct))
-		return NFCT_CB_CONTINUE;
-
-	if (filter_mark(ct))
-		return NFCT_CB_CONTINUE;
-
-	if (filter_label(ct))
-		return NFCT_CB_CONTINUE;
-
-	if (options & CT_COMPARISON &&
-	    !nfct_cmp(obj, ct, NFCT_CMP_ALL | NFCT_CMP_MASK))
+	if (nfct_filter(obj, ct))
 		return NFCT_CB_CONTINUE;
 
 	if (output_mask & _O_XML) {
@@ -1303,17 +1308,7 @@ static int dump_cb(enum nf_conntrack_msg_type type,
 	unsigned int op_type = NFCT_O_DEFAULT;
 	unsigned int op_flags = 0;
 
-	if (filter_nat(obj, ct))
-		return NFCT_CB_CONTINUE;
-
-	if (filter_mark(ct))
-		return NFCT_CB_CONTINUE;
-
-	if (filter_label(ct))
-		return NFCT_CB_CONTINUE;
-
-	if (options & CT_COMPARISON &&
-	    !nfct_cmp(obj, ct, NFCT_CMP_ALL | NFCT_CMP_MASK))
+	if (nfct_filter(obj, ct))
 		return NFCT_CB_CONTINUE;
 
 	if (output_mask & _O_XML) {
@@ -1349,17 +1344,7 @@ static int delete_cb(enum nf_conntrack_msg_type type,
 	unsigned int op_type = NFCT_O_DEFAULT;
 	unsigned int op_flags = 0;
 
-	if (filter_nat(obj, ct))
-		return NFCT_CB_CONTINUE;
-
-	if (filter_mark(ct))
-		return NFCT_CB_CONTINUE;
-
-	if (filter_label(ct))
-		return NFCT_CB_CONTINUE;
-
-	if (options & CT_COMPARISON &&
-	    !nfct_cmp(obj, ct, NFCT_CMP_ALL | NFCT_CMP_MASK))
+	if (nfct_filter(obj, ct))
 		return NFCT_CB_CONTINUE;
 
 	res = nfct_query(ith, NFCT_Q_DESTROY, ct);
@@ -1498,7 +1483,8 @@ static int update_cb(enum nf_conntrack_msg_type type,
 	int res;
 	struct nf_conntrack *obj = data, *tmp;
 
-	if (filter_nat(obj, ct))
+	if (filter_nat(obj, ct) ||
+	    filter_label(ct))
 		return NFCT_CB_CONTINUE;
 
 	if (nfct_attr_is_set(obj, ATTR_ID) && nfct_attr_is_set(ct, ATTR_ID) &&
@@ -1510,9 +1496,6 @@ static int update_cb(enum nf_conntrack_msg_type type,
 	if (options & CT_OPT_TUPLE_REPL && !nfct_cmp(obj, ct, NFCT_CMP_REPL))
 		return NFCT_CB_CONTINUE;
 
-	if (filter_label(ct))
-		return NFCT_CB_CONTINUE;
-
 	tmp = nfct_new();
 	if (tmp == NULL)
 		exit_error(OTHER_PROBLEM, "out of memory");
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH conntrack 2/5] conntrack: consolidate filtering

[Pablo Neira Ayuso]

Applied, thanks.

[PATCH conntrack 3/5] conntrack: cleanup: use switch statements for family checks

[Asbjørn Sloth Tønnesen]

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
---
 src/conntrack.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/conntrack.c b/src/conntrack.c
index cb9545b..3fc9c24 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -1017,10 +1017,16 @@ parse_addr(const char *cp, union ct_address *address)
 	struct addr_parse parse;
 	int ret;
 
-	if ((ret = parse_inetaddr(cp, &parse)) == AF_INET)
+	ret = parse_inetaddr(cp, &parse);
+
+	switch(ret) {
+	case AF_INET:
 		address->v4 = parse.addr.s_addr;
-	else if (ret == AF_INET6)
+		break;
+	case AF_INET6:
 		memcpy(address->v6, &parse.addr6, sizeof(parse.addr6));
+		break;
+	}
 
 	return ret;
 }
@@ -1966,14 +1972,17 @@ nfct_set_addr_from_opt(int opt, struct nf_conntrack *ct, union ct_address *ad,
 			   "Invalid IP address `%s'", optarg);
 	}
 	set_family(family, l3protonum);
-	if (l3protonum == AF_INET) {
+	switch(l3protonum) {
+	case AF_INET:
 		nfct_set_attr_u32(ct,
-				  opt2family_attr[opt][0],
-				  ad->v4);
-	} else if (l3protonum == AF_INET6) {
+		                  opt2family_attr[opt][0],
+		                  ad->v4);
+		break;
+	case AF_INET6:
 		nfct_set_attr(ct,
-			      opt2family_attr[opt][1],
-			      &ad->v6);
+		              opt2family_attr[opt][1],
+		              &ad->v6);
+		break;
 	}
 	nfct_set_attr_u8(ct, opt2attr[opt], l3protonum);
 }
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH conntrack 3/5] conntrack: cleanup: use switch statements for family checks

[Pablo Neira Ayuso]

Also applied, thanks a lot for following up on this Asbjørn.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH conntrack 4/5 v2] conntrack: add support for netmask filtering

[Asbjørn Sloth Tønnesen]

This patch extends --mask-src and --mask-dst to also work
with the conntrack table, with commands -L, -D, -E and -U.

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
---

Notes:
    This is almost completely backward compatible,
    since the --mask-* arguments previously gave
    an error is used with these commands and the
    conntrack table.
    
    I have changed the global_family to filter_family,
    and it is only used to pass the family to the callback,
    the alternative would be to change the data argument of
    the callbacks to a struct.

 conntrack.8     |   7 ++-
 src/conntrack.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 150 insertions(+), 15 deletions(-)

diff --git a/conntrack.8 b/conntrack.8
index 970c2d7..5bba1b1 100644
--- a/conntrack.8
+++ b/conntrack.8
@@ -191,10 +191,13 @@ Specify the tuple source address of an expectation.
 Specify the tuple destination address of an expectation.
 .TP
 .BI "--mask-src " IP_ADDRESS
-Specify the source address mask of an expectation.
+Specify the source address mask.
+For conntrack this option is only available in conjunction with "\-L, \-\-dump", "\-E, \-\-event", "\-U \-\-update" or "\-D \-\-delete".
+For expectations this option is only available in conjunction with "\-I, \-\-create".
 .TP
 .BI "--mask-dst " IP_ADDRESS
-Specify the destination address mask of an expectation.
+Specify the destination address mask.
+Same limitations as for "--mask-src".
 .SS PROTOCOL FILTER PARAMETERS
 .TP
 TCP-specific fields:
diff --git a/src/conntrack.c b/src/conntrack.c
index 3fc9c24..34785f3 100644
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -377,13 +377,13 @@ static char commands_v_options[NUMBER_OF_CMD][NUMBER_OF_OPT] =
 /* Well, it's better than "Re: Linux vs FreeBSD" */
 {
           /*   s d r q p t u z e [ ] { } a m i f n g o c b j w l < > ( ) */
-/*CT_LIST*/   {2,2,2,2,2,0,2,2,0,0,0,0,0,0,2,0,2,2,2,2,2,0,2,2,2,0,0,2,2},
+/*CT_LIST*/   {2,2,2,2,2,0,2,2,0,0,0,2,2,0,2,0,2,2,2,2,2,0,2,2,2,0,0,2,2},
 /*CT_CREATE*/ {3,3,3,3,1,1,2,0,0,0,0,0,0,2,2,0,0,2,2,0,0,0,0,2,0,2,0,2,2},
-/*CT_UPDATE*/ {2,2,2,2,2,2,2,0,0,0,0,0,0,0,2,2,2,2,2,2,0,0,0,0,2,2,2,0,0},
-/*CT_DELETE*/ {2,2,2,2,2,2,2,0,0,0,0,0,0,0,2,2,2,2,2,2,0,0,0,2,2,0,0,2,2},
+/*CT_UPDATE*/ {2,2,2,2,2,2,2,0,0,0,0,2,2,0,2,2,2,2,2,2,0,0,0,0,2,2,2,0,0},
+/*CT_DELETE*/ {2,2,2,2,2,2,2,0,0,0,0,2,2,0,2,2,2,2,2,2,0,0,0,2,2,0,0,2,2},
 /*CT_GET*/    {3,3,3,3,1,0,0,0,0,0,0,0,0,0,0,2,0,0,0,2,0,0,0,0,2,0,0,0,0},
 /*CT_FLUSH*/  {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0},
-/*CT_EVENT*/  {2,2,2,2,2,0,0,0,2,0,0,0,0,0,2,0,0,2,2,2,2,2,2,2,2,0,0,2,2},
+/*CT_EVENT*/  {2,2,2,2,2,0,0,0,2,0,0,2,2,0,2,0,0,2,2,2,2,2,2,2,2,0,0,2,2},
 /*VERSION*/   {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0},
 /*HELP*/      {0,0,0,0,2,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0},
 /*EXP_LIST*/  {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,2,0,0,2,0,0,0,0,0,0,0,0,0},
@@ -465,6 +465,26 @@ static const int opt2attr[] = {
 	[')']	= ATTR_REPL_ZONE,
 };
 
+enum ct_direction {
+	DIR_SRC = 0,
+	DIR_DST = 1,
+};
+
+union ct_address {
+	uint32_t v4;
+	uint32_t v6[4];
+};
+
+static struct ct_network {
+	union ct_address netmask;
+	union ct_address network;
+} dir2network[2];
+
+static const int famdir2attr[2][2] = {
+	{ ATTR_ORIG_IPV4_SRC, ATTR_ORIG_IPV4_DST },
+	{ ATTR_ORIG_IPV6_SRC, ATTR_ORIG_IPV6_DST }
+};
+
 static char exit_msg[NUMBER_OF_CMD][64] = {
 	[CT_LIST_BIT] 		= "%d flow entries have been shown.\n",
 	[CT_CREATE_BIT]		= "%d flow entries have been created.\n",
@@ -507,8 +527,7 @@ static const char usage_expectation_parameters[] =
 	"Expectation parameters and options:\n"
 	"  --tuple-src ip\tSource address in expect tuple\n"
 	"  --tuple-dst ip\tDestination address in expect tuple\n"
-	"  --mask-src ip\t\tSource mask address\n"
-	"  --mask-dst ip\t\tDestination mask address\n";
+	;
 
 static const char usage_update_parameters[] =
 	"Updating parameters and options:\n"
@@ -529,6 +548,8 @@ static const char usage_parameters[] =
 	"  --orig-zone value\t\tSet zone for original direction\n"
 	"  --reply-zone value\t\tSet zone for reply direction\n"
 	"  -b, --buffer-size\t\tNetlink socket buffer size\n"
+	"  --mask-src ip\t\t\tSource mask address\n"
+	"  --mask-dst ip\t\t\tDestination mask address\n"
 	;
 
 #define OPTION_OFFSET 256
@@ -547,6 +568,7 @@ static LIST_HEAD(proto_list);
 
 static unsigned int options;
 static struct nfct_labelmap *labelmap;
+static int filter_family;
 
 void register_proto(struct ctproto_handler *h)
 {
@@ -1006,11 +1028,6 @@ parse_inetaddr(const char *cp, struct addr_parse *parse)
 	return AF_UNSPEC;
 }
 
-union ct_address {
-	uint32_t v4;
-	uint32_t v6[4];
-};
-
 static int
 parse_addr(const char *cp, union ct_address *address)
 {
@@ -1215,11 +1232,68 @@ filter_nat(const struct nf_conntrack *obj, const struct nf_conntrack *ct)
 }
 
 static int
+nfct_ip6_net_cmp(const union ct_address *addr, const struct ct_network *net)
+{
+	int i;
+	for (i=0;i<4;i++)
+		if ((addr->v6[i] & net->netmask.v6[i]) != net->network.v6[i])
+			return 1;
+	return 0;
+}
+
+static int
+nfct_ip_net_cmp(int family, const union ct_address *addr,
+                const struct ct_network *net)
+{
+	switch(family) {
+	case AF_INET:
+		return (addr->v4 & net->netmask.v4) != net->network.v4;
+	case AF_INET6:
+		return nfct_ip6_net_cmp(addr, net);
+	default:
+		return 0;
+	}
+}
+
+static int
+nfct_filter_network_direction(const struct nf_conntrack *ct, enum ct_direction dir)
+{
+	const int family = filter_family;
+	const union ct_address *address;
+	enum nf_conntrack_attr attr;
+	struct ct_network *net = &dir2network[dir];
+
+	if (nfct_get_attr_u8(ct, ATTR_ORIG_L3PROTO) != family)
+		return 1;
+
+	attr = famdir2attr[family == AF_INET6][dir];
+	address = nfct_get_attr(ct, attr);
+
+	return nfct_ip_net_cmp(family, address, net);
+}
+
+static int
+filter_network(const struct nf_conntrack *ct)
+{
+	if (options & CT_OPT_MASK_SRC) {
+		if (nfct_filter_network_direction(ct, DIR_SRC))
+			return 1;
+	}
+
+	if (options & CT_OPT_MASK_DST) {
+		if (nfct_filter_network_direction(ct, DIR_DST))
+			return 1;
+	}
+	return 0;
+}
+
+static int
 nfct_filter(struct nf_conntrack *obj, struct nf_conntrack *ct)
 {
 	if (filter_nat(obj, ct) ||
 	    filter_mark(ct) ||
-	    filter_label(ct))
+	    filter_label(ct) ||
+	    filter_network(ct))
 		return 1;
 
 	if (options & CT_COMPARISON &&
@@ -1490,7 +1564,8 @@ static int update_cb(enum nf_conntrack_msg_type type,
 	struct nf_conntrack *obj = data, *tmp;
 
 	if (filter_nat(obj, ct) ||
-	    filter_label(ct))
+	    filter_label(ct) ||
+	    filter_network(ct))
 		return NFCT_CB_CONTINUE;
 
 	if (nfct_attr_is_set(obj, ATTR_ID) && nfct_attr_is_set(ct, ATTR_ID) &&
@@ -1934,6 +2009,54 @@ static void labelmap_init(void)
 		perror("nfct_labelmap_new");
 }
 
+static void
+nfct_network_attr_prepare(const int family, enum ct_direction dir)
+{
+	const union ct_address *address, *netmask;
+	enum nf_conntrack_attr attr;
+	int i;
+	struct ct_network *net = &dir2network[dir];
+
+	attr = famdir2attr[family == AF_INET6][dir];
+
+	address = nfct_get_attr(tmpl.ct, attr);
+	netmask = nfct_get_attr(tmpl.mask, attr);
+
+	switch(family) {
+	case AF_INET:
+		net->network.v4 = address->v4 & netmask->v4;
+		break;
+	case AF_INET6:
+		for (i=0;i<4;i++)
+			net->network.v6[i] = address->v6[i] & netmask->v6[i];
+		break;
+	}
+
+	memcpy(&net->netmask, netmask, sizeof(union ct_address));
+
+	/* avoid exact source matching */
+	nfct_attr_unset(tmpl.ct, attr);
+}
+
+static void
+nfct_filter_init(const int family)
+{
+	filter_family = family;
+	if (options & CT_OPT_MASK_SRC) {
+		if (!(options & CT_OPT_ORIG_SRC))
+			exit_error(PARAMETER_PROBLEM,
+			           "Can't use --mask-src without --src");
+		nfct_network_attr_prepare(family, DIR_SRC);
+	}
+
+	if (options & CT_OPT_MASK_DST) {
+		if (!(options & CT_OPT_ORIG_DST))
+			exit_error(PARAMETER_PROBLEM,
+			           "Can't use --mask-dst without --dst");
+		nfct_network_attr_prepare(family, DIR_DST);
+	}
+}
+
 static void merge_bitmasks(struct nfct_bitmask **current,
 			  struct nfct_bitmask *src)
 {
@@ -2289,6 +2412,8 @@ int main(int argc, char *argv[])
 			exit_error(PARAMETER_PROBLEM, "Can't use -z with "
 						      "filtering parameters");
 
+		nfct_filter_init(family);
+
 		nfct_callback_register(cth, NFCT_T_ALL, dump_cb, tmpl.ct);
 
 		filter_dump = nfct_filter_dump_create();
@@ -2375,6 +2500,8 @@ int main(int argc, char *argv[])
 		if (!cth || !ith)
 			exit_error(OTHER_PROBLEM, "Can't open handler");
 
+		nfct_filter_init(family);
+
 		nfct_callback_register(cth, NFCT_T_ALL, update_cb, tmpl.ct);
 
 		res = nfct_query(cth, NFCT_Q_DUMP, &family);
@@ -2388,6 +2515,8 @@ int main(int argc, char *argv[])
 		if (!cth || !ith)
 			exit_error(OTHER_PROBLEM, "Can't open handler");
 
+		nfct_filter_init(family);
+
 		nfct_callback_register(cth, NFCT_T_ALL, delete_cb, tmpl.ct);
 
 		filter_dump = nfct_filter_dump_create();
@@ -2489,6 +2618,9 @@ int main(int argc, char *argv[])
 			fprintf(stderr, "NOTICE: Netlink socket buffer size "
 					"has been set to %zu bytes.\n", ret);
 		}
+
+		nfct_filter_init(family);
+
 		signal(SIGINT, event_sighandler);
 		signal(SIGTERM, event_sighandler);
 		nfct_callback_register(cth, NFCT_T_ALL, event_cb, tmpl.ct);
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH conntrack 4/5 v2] conntrack: add support for netmask filtering

[Pablo Neira Ayuso]

On Mon, Jan 25, 2016 at 11:15:47AM +0000, Asbjørn Sloth Tønnesen wrote:
> This patch extends --mask-src and --mask-dst to also work
> with the conntrack table, with commands -L, -D, -E and -U.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
> ---
> 
> Notes:
>     This is almost completely backward compatible,
>     since the --mask-* arguments previously gave
>     an error is used with these commands and the
>     conntrack table.
>     
>     I have changed the global_family to filter_family,
>     and it is only used to pass the family to the callback,
>     the alternative would be to change the data argument of
>     the callbacks to a struct.

I see changes with regards to previous patchset, not we don't use
cidr. I think this is better since it allows a more compact way.

I prefer the cidr-based approach, any reason to drop it?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH conntrack 4/5 v2] conntrack: add support for netmask filtering

[Asbjørn Sloth Tønnesen]

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

Hi Pablo,

On Mon, 1 Feb 2016 12:04:23 +0100, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Jan 25, 2016 at 11:15:47AM +0000, Asbjørn Sloth Tønnesen wrote:
> > This patch extends --mask-src and --mask-dst to also work
> > with the conntrack table, with commands -L, -D, -E and -U.
> > 
> > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
> > ---
> > 
> > Notes:
> >     This is almost completely backward compatible,
> >     since the --mask-* arguments previously gave
> >     an error is used with these commands and the
> >     conntrack table.
> >     
> >     I have changed the global_family to filter_family,
> >     and it is only used to pass the family to the callback,
> >     the alternative would be to change the data argument of
> >     the callbacks to a struct.
> 
> I see changes with regards to previous patchset, not we don't use
> cidr. I think this is better since it allows a more compact way.
> 
> I prefer the cidr-based approach, any reason to drop it?

I decided to split them up in several patchsets, each having its
own merits. The netmask and CIDR patches are related, but one is about
filtering, and the other about adding some sugar to the option parsing.

BTW: I mistakenly also marked the test patch v2, but this only is the only one
in this patchset from the old one.

-- 
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby ApS - AS42541

Re: [PATCH conntrack 4/5 v2] conntrack: add support for netmask filtering

[Pablo Neira Ayuso]

On Mon, Feb 01, 2016 at 12:17:02PM +0000, Asbjørn Sloth Tønnesen wrote:
> Hi Pablo,
> 
> On Mon, 1 Feb 2016 12:04:23 +0100, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Mon, Jan 25, 2016 at 11:15:47AM +0000, Asbjørn Sloth Tønnesen wrote:
> > > This patch extends --mask-src and --mask-dst to also work
> > > with the conntrack table, with commands -L, -D, -E and -U.
> > > 
> > > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
> > > ---
> > > 
> > > Notes:
> > >     This is almost completely backward compatible,
> > >     since the --mask-* arguments previously gave
> > >     an error is used with these commands and the
> > >     conntrack table.
> > >     
> > >     I have changed the global_family to filter_family,
> > >     and it is only used to pass the family to the callback,
> > >     the alternative would be to change the data argument of
> > >     the callbacks to a struct.
> > 
> > I see changes with regards to previous patchset, not we don't use
> > cidr. I think this is better since it allows a more compact way.
> > 
> > I prefer the cidr-based approach, any reason to drop it?
> 
> I decided to split them up in several patchsets, each having its
> own merits. The netmask and CIDR patches are related, but one is about
> filtering, and the other about adding some sugar to the option parsing.

But we don't get anything with this extra option since it's basically
equivalent to the cidr based filtering, right?

> BTW: I mistakenly also marked the test patch v2, but this only is
> the only one in this patchset from the old one.

No problem.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH conntrack 4/5 v2] conntrack: add support for netmask filtering

[Asbjørn Sloth Tønnesen]

[-- Attachment #1: Type: text/plain, Size: 2084 bytes --]

Hi Pablo,

On Mon, 1 Feb 2016 18:56:25 +0100, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Feb 01, 2016 at 12:17:02PM +0000, Asbjørn Sloth Tønnesen wrote:
> > On Mon, 1 Feb 2016 12:04:23 +0100, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > On Mon, Jan 25, 2016 at 11:15:47AM +0000, Asbjørn Sloth Tønnesen wrote:
> > > > This patch extends --mask-src and --mask-dst to also work
> > > > with the conntrack table, with commands -L, -D, -E and -U.
> > > > 
> > > > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
> > > > ---
> > > > 
> > > > Notes:
> > > >     This is almost completely backward compatible,
> > > >     since the --mask-* arguments previously gave
> > > >     an error is used with these commands and the
> > > >     conntrack table.
> > > >     
> > > >     I have changed the global_family to filter_family,
> > > >     and it is only used to pass the family to the callback,
> > > >     the alternative would be to change the data argument of
> > > >     the callbacks to a struct.
> > > 
> > > I see changes with regards to previous patchset, not we don't use
> > > cidr. I think this is better since it allows a more compact way.
> > > 
> > > I prefer the cidr-based approach, any reason to drop it?
> > 
> > I decided to split them up in several patchsets, each having its
> > own merits. The netmask and CIDR patches are related, but one is about
> > filtering, and the other about adding some sugar to the option parsing.
> 
> But we don't get anything with this extra option since it's basically
> equivalent to the cidr based filtering, right?

Except backwards compatability for the expectation table, on the other hand
--mask-* has been broken since August, but thats only v1.4.3 so properly not
long enough to just drop it.

Since the filtering internally uses a bitmask in ct.mask, then keeping the --mask-*
option for all cases is simpler, since it uses the same option flags.

Keeping them also makes it possible to inject funky bitmasks.


-- 
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby ApS - AS42541

Re: [PATCH conntrack 4/5 v2] conntrack: add support for netmask filtering

[Pablo Neira Ayuso]

On Tue, Feb 02, 2016 at 03:55:53PM +0000, Asbjørn Sloth Tønnesen wrote:
> Keeping them also makes it possible to inject funky bitmasks.

Good point, will get another look into these remaining patchset asap.
Busy with netdev 1.1 organization stuff.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH conntrack 4/5 v2] conntrack: add support for netmask filtering

[Pablo Neira Ayuso]

On Mon, Jan 25, 2016 at 11:15:47AM +0000, Asbjørn Sloth Tønnesen wrote:
> This patch extends --mask-src and --mask-dst to also work
> with the conntrack table, with commands -L, -D, -E and -U.

Applied, thanks for your patience.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH conntrack 5/5 v2] tests: conntrack: add netmask tests

[Asbjørn Sloth Tønnesen]

Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
---
 tests/conntrack/testsuite/01delete | 6 ++++++
 tests/conntrack/testsuite/02filter | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/tests/conntrack/testsuite/01delete b/tests/conntrack/testsuite/01delete
index 3c38ac5..566b89f 100644
--- a/tests/conntrack/testsuite/01delete
+++ b/tests/conntrack/testsuite/01delete
@@ -4,3 +4,9 @@
 -D -s 2.2.2.2 -p tcp --sport 10 --dport 20 ; BAD
 # delete by source
 -D -s 1.1.1.1 ; OK
+# re-create dummy
+-I -s 1.1.1.1 -d 2.2.2.2 -p tcp --sport 10 --dport 20 --state LISTEN -u SEEN_REPLY -t 50 ; OK
+# delete by netmask
+-D -s 1.1.1.0 --mask-src 255.255.255.0 -d 2.2.2.0 --mask-dst 255.255.255.0 ; OK
+# fails due to 0 matches
+-D -s 1.1.1.0 --mask-src 255.255.255.0 -d 2.2.2.0 --mask-dst 255.255.255.0 ; BAD
diff --git a/tests/conntrack/testsuite/02filter b/tests/conntrack/testsuite/02filter
index 204c4e8..5d2270b 100644
--- a/tests/conntrack/testsuite/02filter
+++ b/tests/conntrack/testsuite/02filter
@@ -19,5 +19,7 @@ conntrack -L -f ipv4 ; OK
 # filter by mark
 conntrack -L --mark 0 ; OK
 conntrack -L --mark 0/0xffffffff; OK
+# filter by netmask
+conntrack -L -s 1.1.1.0 --mask-src 255.255.255.0 -d 2.0.0.0 --mask-dst 255.0.0.0 ; OK
 # delete dummy
 conntrack -D -d 2.2.2.2 ; OK
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH conntrack 5/5 v2] tests: conntrack: add netmask tests

[Pablo Neira Ayuso]

Applied, thanks for adding the tests.

Re: [PATCH conntrack 1/5] conntrack: support delete by label

[Pablo Neira Ayuso]

On Mon, Jan 25, 2016 at 11:15:44AM +0000, Asbjørn Sloth Tønnesen wrote:
> This option was already silently allowed by 991fc4ae,
> but didn't have any effect.
> 
> This patch adds the check and documents it.

Applied, thanks.

> Cc: Clemence Faure <clemence.faure@sophos.com>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
> ---
> 
> Notes:
>     I tried to create a test case, as well but I didn't
>     seam to be able to get --label-add to work with
>     create.

Cc'ing Florian. I think it would be good to have a test for this label
support for conntrack.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH conntrack 1/5] conntrack: support delete by label

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Jan 25, 2016 at 11:15:44AM +0000, Asbjørn Sloth Tønnesen wrote:
> > This option was already silently allowed by 991fc4ae,
> > but didn't have any effect.
> > 
> > This patch adds the check and documents it.
> 
> Applied, thanks.
> 
> > Cc: Clemence Faure <clemence.faure@sophos.com>
> > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.dk>
> > ---
> > 
> > Notes:
> >     I tried to create a test case, as well but I didn't
> >     seam to be able to get --label-add to work with
> >     create.

It only works if a -m connlabel rule exists on the system at the moment.

https://patchwork.ozlabs.org/patch/553363/

extends it to nftables.

> Cc'ing Florian. I think it would be good to have a test for this label
> support for conntrack.

Right.  We could just add

nf_connlabels_get(ctx->net, (len * BITS_PER_BYTE) - 1);

When attempting to add a label via ctnetlink and label support
isn't active.

However, unlike the nft/xtables path this would be one-way:

When you have a ruleset that uses -m connlabel, then flush/delete the
ruleset the extension will not be added to new conntracks anymore
since ->destroy() hook invocation will _put the connlabel extension
usage count.

For ctnetlink there is no such thing unfortunately (unless we'd add
refcnts to the individual conntracks but thats something I don't want
to do since it seems ridiculously expensive with no real gain).
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html