[PATCH v2 3/3] ebtables: Add string filter

2018-03-20 Thread Bernie Harris
This patch is part of a proposal to add a string filter to
ebtables, which would be similar to the string filter in
iptables. Like iptables, the ebtables filter uses the xt_string
module.

Signed-off-by: Bernie Harris 
---
 net/netfilter/xt_string.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/xt_string.c b/net/netfilter/xt_string.c
index 423293ee57c2..be1feddadcf0 100644
--- a/net/netfilter/xt_string.c
+++ b/net/netfilter/xt_string.c
@@ -21,6 +21,7 @@ MODULE_DESCRIPTION("Xtables: string-based matching");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("ipt_string");
 MODULE_ALIAS("ip6t_string");
+MODULE_ALIAS("ebt_string");
 
 static bool
 string_mt(const struct sk_buff *skb, struct xt_action_param *par)
-- 
2.16.2

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


[ebtables PATCH v2] Add string filter to ebtables

2018-03-20 Thread Bernie Harris
This patch is part of a proposal to add a string filter to
ebtables, which would be similar to the string filter in
iptables.

Like iptables, the ebtables filter uses the xt_string module,
however some modifications have been made for this to work
correctly.

Currently ebtables assumes that the revision number of all match
modules is 0. The xt_string module doesn't register a match with
revision 0 so the solution is to modify ebtables to allow
extensions to specify a revision number, similar to iptables.
This gets passed down to the kernel, which is then able to find
the match module correctly.

Signed-off-by: Bernie Harris 
---
 ebtables.8  |  20 +++
 extensions/Makefile |   2 +-
 extensions/ebt_string.c | 319 
 include/ebtables.h  |  16 ++-
 include/ebtables_u.h|   1 +
 libebtc.c   |   6 +-
 6 files changed, 359 insertions(+), 5 deletions(-)
 create mode 100644 extensions/ebt_string.c

diff --git a/ebtables.8 b/ebtables.8
index 81d1cf6..e3290fe 100644
--- a/ebtables.8
+++ b/ebtables.8
@@ -810,6 +810,26 @@ The hello time timer (0-65535) range.
 .TP
 .BR "--stp-forward-delay " "[!] [\fIdelay\fP][:\fIdelay\fP]"
 The forward delay timer (0-65535) range.
+.SS string
+This module matches on a given string using some pattern matching strategy.
+.TP
+.BR "--string-algo " "\fIalgorithm\fP"
+The pattern matching strategy. (bm = Boyer-Moore, kmp = Knuth-Pratt-Morris)
+.TP
+.BR "--string-from " "\fIoffset\fP"
+The lowest offset from which a match can start. (default: 0)
+.TP
+.BR "--string-to " "\fIoffset\fP"
+The highest offset from which a match can start. (default: size of frame)
+.TP
+.BR "--string " "[!] \fIpattern\fP"
+Matches the given pattern.
+.TP
+.BR "--string-hex " "[!] \fIpattern\fP"
+Matches the given pattern in hex notation, e.g. '|0D 0A|', '|0D0A|', 
'www|09|netfilter|03|org|00|'
+.TP
+.BR "--string-icase"
+Ignore case when searching.
 .SS vlan
 Specify 802.1Q Tag Control Information fields.
 The protocol must be specified as
diff --git a/extensions/Makefile b/extensions/Makefile
index b3548e8..60a70a2 100644
--- a/extensions/Makefile
+++ b/extensions/Makefile
@@ -1,7 +1,7 @@
 #! /usr/bin/make
 
 EXT_FUNC+=802_3 nat arp arpreply ip ip6 standard log redirect vlan mark_m mark 
\
-  pkttype stp among limit ulog nflog
+  pkttype stp among limit ulog nflog string
 EXT_TABLES+=filter nat broute
 EXT_OBJS+=$(foreach T,$(EXT_FUNC), extensions/ebt_$(T).o)
 EXT_OBJS+=$(foreach T,$(EXT_TABLES), extensions/ebtable_$(T).o)
diff --git a/extensions/ebt_string.c b/extensions/ebt_string.c
new file mode 100644
index 000..793f5df
--- /dev/null
+++ b/extensions/ebt_string.c
@@ -0,0 +1,319 @@
+/* ebt_string
+ *
+ * Author:
+ * Bernie Harris 
+ *
+ * February, 2018
+ *
+ * Based on:
+ *  libxt_string.c, Copyright (C) 2000 Emmanuel Roger  
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "../include/ebtables_u.h"
+#include 
+#include 
+
+#define STRING_FROM  '1'
+#define STRING_TO'2'
+#define STRING_ALGO  '3'
+#define STRING_ICASE '4'
+#define STRING   '5'
+#define STRING_HEX   '6'
+#define OPT_STRING_FROM  (1 << 0)
+#define OPT_STRING_TO(1 << 1)
+#define OPT_STRING_ALGO  (1 << 2)
+#define OPT_STRING_ICASE (1 << 3)
+#define OPT_STRING   (1 << 4)
+#define OPT_STRING_HEX   (1 << 5)
+
+static const struct option opts[] =
+{
+   { "string-from" , required_argument, 0, STRING_FROM },
+   { "string-to"   , required_argument, 0, STRING_TO },
+   { "string-algo" , required_argument, 0, STRING_ALGO },
+   { "string-icase", no_argument,   0, STRING_ICASE },
+   { "string"  , required_argument, 0, STRING },
+   { "string-hex"  , required_argument, 0, STRING_HEX },
+   { 0 }
+};
+
+static void print_help()
+{
+   printf(
+"string options:\n"
+"--string-from offset: Offset to start searching from (default: 0)\n"
+"--string-to   offset: Offset to stop searching (default: packet size)\n"
+"--string-algo algorithm : Algorithm (bm = Boyer-Moore, kmp = 
Knuth-Pratt-Morris)\n"
+"--string-icase  : Ignore case when searching\n"
+"--string [!] string : Match a string in a packet\n"
+"--string-hex [!] string : Match a hex string in a packet, e.g. |0D 0A|, 
|0D0A|, netfilter|03|org\n");
+}
+
+static void init(struct ebt_entry_match *match)
+{
+   struct xt_string_info *info = (struct xt_string_info *)match->data;
+
+   info->to_offset = UINT16_MAX;
+}
+
+static void parse_string(const char *s, struct xt_string_info *info)
+{
+   /* xt_string does not need \0 at the end of the pattern */
+   if (strlen(s) <= XT_STRING_MAX_PATTERN_SIZE) {
+   strncpy(info->pattern, s, XT_STRING_MAX_PATTERN_SIZE);
+   info->patlen = strnlen(s, 

[PATCH v2 2/3] ebtables: Add support for specifying match revision

2018-03-20 Thread Bernie Harris
Currently ebtables assumes that the revision number of all match
modules is 0, which is an issue when trying to use existing
xtables matches with ebtables. The solution is to modify ebtables
to allow extensions to specify a revision number, similar to
iptables. This gets passed down to the kernel, which is then able
to find the match module correctly.

To main binary backwards compatibility, the size of the ebt_entry
structures is not changed, only the size of the name field is
decreased by 1 byte to make room for the revision field.

Signed-off-by: Bernie Harris 
---
 include/uapi/linux/netfilter_bridge/ebtables.h | 16 +++--
 net/bridge/netfilter/ebtables.c| 47 --
 2 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/include/uapi/linux/netfilter_bridge/ebtables.h 
b/include/uapi/linux/netfilter_bridge/ebtables.h
index 9ff57c0a0199..0c7dc8315013 100644
--- a/include/uapi/linux/netfilter_bridge/ebtables.h
+++ b/include/uapi/linux/netfilter_bridge/ebtables.h
@@ -20,6 +20,7 @@
 #define EBT_TABLE_MAXNAMELEN 32
 #define EBT_CHAIN_MAXNAMELEN EBT_TABLE_MAXNAMELEN
 #define EBT_FUNCTION_MAXNAMELEN EBT_TABLE_MAXNAMELEN
+#define EBT_EXTENSION_MAXNAMELEN 31
 
 /* verdicts >0 are "branches" */
 #define EBT_ACCEPT   -1
@@ -120,7 +121,10 @@ struct ebt_entries {
 
 struct ebt_entry_match {
union {
-   char name[EBT_FUNCTION_MAXNAMELEN];
+   struct {
+   char name[EBT_EXTENSION_MAXNAMELEN];
+   uint8_t revision;
+   };
struct xt_match *match;
} u;
/* size of data */
@@ -130,7 +134,10 @@ struct ebt_entry_match {
 
 struct ebt_entry_watcher {
union {
-   char name[EBT_FUNCTION_MAXNAMELEN];
+   struct {
+   char name[EBT_EXTENSION_MAXNAMELEN];
+   uint8_t revision;
+   };
struct xt_target *watcher;
} u;
/* size of data */
@@ -140,7 +147,10 @@ struct ebt_entry_watcher {
 
 struct ebt_entry_target {
union {
-   char name[EBT_FUNCTION_MAXNAMELEN];
+   struct {
+   char name[EBT_EXTENSION_MAXNAMELEN];
+   uint8_t revision;
+   };
struct xt_target *target;
} u;
/* size of data */
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 254ef9f49567..5ed6a47866d0 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -358,12 +358,12 @@ ebt_check_match(struct ebt_entry_match *m, struct 
xt_mtchk_param *par,
left - sizeof(struct ebt_entry_match) < m->match_size)
return -EINVAL;
 
-   match = xt_find_match(NFPROTO_BRIDGE, m->u.name, 0);
+   match = xt_find_match(NFPROTO_BRIDGE, m->u.name, m->u.revision);
if (IS_ERR(match) || match->family != NFPROTO_BRIDGE) {
if (!IS_ERR(match))
module_put(match->me);
request_module("ebt_%s", m->u.name);
-   match = xt_find_match(NFPROTO_BRIDGE, m->u.name, 0);
+   match = xt_find_match(NFPROTO_BRIDGE, m->u.name, m->u.revision);
}
if (IS_ERR(match))
return PTR_ERR(match);
@@ -1355,16 +1355,17 @@ static int update_counters(struct net *net, const void 
__user *user,
 
 static inline int ebt_obj_to_user(char __user *um, const char *_name,
  const char *data, int entrysize,
- int usersize, int datasize)
+ int usersize, int datasize, u8 revision)
 {
-   char name[EBT_FUNCTION_MAXNAMELEN] = {0};
+   char name[EBT_EXTENSION_MAXNAMELEN] = {0};
 
-   /* ebtables expects 32 bytes long names but xt_match names are 29 bytes
+   /* ebtables expects 31 bytes long names but xt_match names are 29 bytes
 * long. Copy 29 bytes and fill remaining bytes with zeroes.
 */
strlcpy(name, _name, sizeof(name));
-   if (copy_to_user(um, name, EBT_FUNCTION_MAXNAMELEN) ||
-   put_user(datasize, (int __user *)(um + EBT_FUNCTION_MAXNAMELEN)) ||
+   if (copy_to_user(um, name, EBT_EXTENSION_MAXNAMELEN) ||
+   put_user(revision, (u8 __user *)(um + EBT_EXTENSION_MAXNAMELEN)) ||
+   put_user(datasize, (int __user *)(um + EBT_EXTENSION_MAXNAMELEN + 
1)) ||
xt_data_to_user(um + entrysize, data, usersize, datasize,
XT_ALIGN(datasize)))
return -EFAULT;
@@ -1377,7 +1378,8 @@ static inline int ebt_match_to_user(const struct 
ebt_entry_match *m,
 {
return ebt_obj_to_user(ubase + ((char *)m - base),
   m->u.match->name, m->data, sizeof(*m),
-  m->u.match->usersize, m->match_size);
+  

[PATCH v2 1/3] net: Allow to and from offsets to be equal in skb_find_text

2018-03-20 Thread Bernie Harris
The xt_string module uses skb_find_text to match a pattern
against packet data. The current behaviour is that the offsets
are used as the range in which a match can start, with the 'to'
offset being included in that range. This means that to do an
exact match for a string at a specific offset, the 'to' and
'from' offsets need to be equal. However, skb_seq_read does not
allow any data to be read if the offsets are equal.

This patch fixes this behaviour by adding the pattern length to
the 'to' offset when calling skb_prepare_seq_read. This should
not change the behaviour of any existing callers of skb_find_text
since the maximum number of bytes read does not change. This
makes it possible for the xt_string module to do an exact match
for a string at a specific offset.

Signed-off-by: Bernie Harris 
---
 net/core/skbuff.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0bb0d8877954..3026158a9993 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3353,7 +3353,8 @@ unsigned int skb_find_text(struct sk_buff *skb, unsigned 
int from,
config->get_next_block = skb_ts_get_next_block;
config->finish = skb_ts_finish;
 
-   skb_prepare_seq_read(skb, from, to, TS_SKB_CB());
+   skb_prepare_seq_read(skb, from, to + textsearch_get_pattern_len(config),
+TS_SKB_CB());
 
ret = textsearch_find(config, );
return (ret <= to - from ? ret : UINT_MAX);
-- 
2.16.2

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


[PATCH] netfilter: ipset: Use is_zero_ether_addr instead of static and memcmp

2018-03-20 Thread Joe Perches
To make the test a bit clearer and to reduce object size a little.

Miscellanea:

o remove now unnecessary static const array

$ size ip_set_hash_mac.o*
   textdata bss dec hex filename
  228224619  64   275056b71 ip_set_hash_mac.o.allyesconfig.new
  229324683  64   276796c1f ip_set_hash_mac.o.allyesconfig.old
  104431040   0   114832cdb ip_set_hash_mac.o.defconfig.new
  105071040   0   115472d1b ip_set_hash_mac.o.defconfig.old

Signed-off-by: Joe Perches 
---
 net/netfilter/ipset/ip_set_hash_mac.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_mac.c 
b/net/netfilter/ipset/ip_set_hash_mac.c
index 8f004edad396..f9d5a2a1e3d0 100644
--- a/net/netfilter/ipset/ip_set_hash_mac.c
+++ b/net/netfilter/ipset/ip_set_hash_mac.c
@@ -72,9 +72,6 @@ hash_mac4_data_next(struct hash_mac4_elem *next,
 #define IP_SET_PROTO_UNDEF
 #include "ip_set_hash_gen.h"
 
-/* Zero valued element is not supported */
-static const unsigned char invalid_ether[ETH_ALEN] = { 0 };
-
 static int
 hash_mac4_kadt(struct ip_set *set, const struct sk_buff *skb,
   const struct xt_action_param *par,
@@ -93,7 +90,7 @@ hash_mac4_kadt(struct ip_set *set, const struct sk_buff *skb,
return -EINVAL;
 
ether_addr_copy(e.ether, eth_hdr(skb)->h_source);
-   if (memcmp(e.ether, invalid_ether, ETH_ALEN) == 0)
+   if (is_zero_ether_addr(e.ether))
return -EINVAL;
return adtfn(set, , , >ext, opt->cmdflags);
 }
@@ -118,7 +115,7 @@ hash_mac4_uadt(struct ip_set *set, struct nlattr *tb[],
if (ret)
return ret;
ether_addr_copy(e.ether, nla_data(tb[IPSET_ATTR_ETHER]));
-   if (memcmp(e.ether, invalid_ether, ETH_ALEN) == 0)
+   if (is_zero_ether_addr(e.ether))
return -IPSET_ERR_HASH_ELEM;
 
return adtfn(set, , , , flags);
-- 
2.15.0

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


Re: [PATCH nf-next 0/2] ebtables: add support for ICMP and IGMP type/code matching

2018-03-20 Thread Pablo Neira Ayuso
On Sun, Mar 04, 2018 at 09:28:52AM +0100, Matthias Schiffer wrote:
> I recently found myself in a situation that required me to filter IGMP
> packets of certain types on a bridge. Switching to nftables is
> unfortunately not an option at the moment because of hardware constraints,
> in particular regarding code size; therefore, this patchset adds support
> for such matches to ebtables. For completeness and feature-parity with
> IPv6, matches for ICMP are added as well.

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


Re: [PATCH libnftnl v2] examples: add nft-ct-helper-{add,get,del}

2018-03-20 Thread Pablo Neira Ayuso
On Tue, Mar 20, 2018 at 10:53:22PM +0800, Yang Zheng wrote:
> nft-ct-helper-{add,get,del}: add, get, or delete ct helper objects from the 
> specified table.
> 
> Examples:
>   % ./nft-ct-helper-get ip filter
>   
>   % ./nft-ct-helper-add ip filter sip-5060 sip udp
>   % ./nft-ct-helper-get ip filter
>   table filter name sip-5060 use 0 [ ct_helper name sip family 2 protocol 17 ]
>   % ./nft-ct-helper-del ip filter sip-5060
>   % ./nft-ct-helper-get ip filter
>   

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


Re: [PATCH libnftnl v2] examples: add nft-ct-helper-{add,get,del}

2018-03-20 Thread Pablo Neira Ayuso
On Tue, Mar 20, 2018 at 10:53:22PM +0800, Yang Zheng wrote:
> nft-ct-helper-{add,get,del}: add, get, or delete ct helper objects from the 
> specified table.
> 
> Examples:
>   % ./nft-ct-helper-get ip filter
>   
>   % ./nft-ct-helper-add ip filter sip-5060 sip udp
>   % ./nft-ct-helper-get ip filter
>   table filter name sip-5060 use 0 [ ct_helper name sip family 2 protocol 17 ]
>   % ./nft-ct-helper-del ip filter sip-5060
>   % ./nft-ct-helper-get ip filter
>   

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


[PATCH libnftnl v2] examples: add nft-ct-helper-{add,get,del}

2018-03-20 Thread Yang Zheng
nft-ct-helper-{add,get,del}: add, get, or delete ct helper objects from the 
specified table.

Examples:
  % ./nft-ct-helper-get ip filter
  
  % ./nft-ct-helper-add ip filter sip-5060 sip udp
  % ./nft-ct-helper-get ip filter
  table filter name sip-5060 use 0 [ ct_helper name sip family 2 protocol 17 ]
  % ./nft-ct-helper-del ip filter sip-5060
  % ./nft-ct-helper-get ip filter
  

Signed-off-by: Yang Zheng 
---
 examples/Makefile.am |  14 +++-
 examples/nft-ct-helper-add.c | 149 ++
 examples/nft-ct-helper-del.c | 124 +++
 examples/nft-ct-helper-get.c | 150 +++
 4 files changed, 436 insertions(+), 1 deletion(-)
 create mode 100644 examples/nft-ct-helper-add.c
 create mode 100644 examples/nft-ct-helper-del.c
 create mode 100644 examples/nft-ct-helper-get.c

diff --git a/examples/Makefile.am b/examples/Makefile.am
index c0d84eb..96112e7 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -30,7 +30,10 @@ check_PROGRAMS = nft-table-add   \
 nft-flowtable-get  \
 nft-ruleset-get\
 nft-ruleset-parse-file \
-nft-compat-get
+nft-compat-get \
+nft-ct-helper-add  \
+nft-ct-helper-get  \
+nft-ct-helper-del
 
 nft_table_add_SOURCES = nft-table-add.c
 nft_table_add_LDADD = ../src/libnftnl.la ${LIBMNL_LIBS}
@@ -124,3 +127,12 @@ nft_ruleset_parse_file_LDADD = ../src/libnftnl.la 
${LIBMNL_LIBS}
 
 nft_compat_get_SOURCES = nft-compat-get.c
 nft_compat_get_LDADD = ../src/libnftnl.la ${LIBMNL_LIBS}
+
+nft_ct_helper_add_SOURCES = nft-ct-helper-add.c
+nft_ct_helper_add_LDADD = ../src/libnftnl.la ${LIBMNL_LIBS}
+
+nft_ct_helper_get_SOURCES = nft-ct-helper-get.c
+nft_ct_helper_get_LDADD = ../src/libnftnl.la ${LIBMNL_LIBS}
+
+nft_ct_helper_del_SOURCES = nft-ct-helper-del.c
+nft_ct_helper_del_LDADD = ../src/libnftnl.la ${LIBMNL_LIBS}
diff --git a/examples/nft-ct-helper-add.c b/examples/nft-ct-helper-add.c
new file mode 100644
index 000..397443b
--- /dev/null
+++ b/examples/nft-ct-helper-add.c
@@ -0,0 +1,149 @@
+/*
+ * (C) 2012-2016 by Pablo Neira Ayuso 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+#include 
+
+static uint16_t parse_family(char *str, const char *option)
+{
+if (strcmp(str, "ip") == 0)
+return NFPROTO_IPV4;
+else if (strcmp(str, "ip6") == 0)
+return NFPROTO_IPV6;
+else if (strcmp(str, "inet") == 0)
+return NFPROTO_INET;
+else {
+fprintf(stderr, "Unknown %s: ip, ip6, inet\n", option);
+exit(EXIT_FAILURE);
+}
+}
+
+static uint8_t parse_l4proto(char *str)
+{
+if (strcmp(str, "udp") == 0)
+return IPPROTO_UDP;
+else if (strcmp(str, "tcp") == 0)
+return IPPROTO_TCP;
+else {
+fprintf(stderr, "Unknown l4proto: tcp, udp\n");
+exit(EXIT_FAILURE);
+}
+return IPPROTO_TCP;
+}
+
+static struct nftnl_obj *ct_helper_add_parse(int argc, char *argv[])
+{
+struct nftnl_obj *t;
+uint16_t family, l3proto;
+uint8_t l4proto;
+
+t = nftnl_obj_alloc();
+if (t == NULL) {
+perror("OOM");
+return NULL;
+}
+
+family = parse_family(argv[1], "family");
+nftnl_obj_set_u32(t, NFTNL_OBJ_FAMILY, family);
+nftnl_obj_set_u32(t, NFTNL_OBJ_TYPE, NFT_OBJECT_CT_HELPER);
+nftnl_obj_set_str(t, NFTNL_OBJ_TABLE, argv[2]);
+nftnl_obj_set_str(t, NFTNL_OBJ_NAME, argv[3]);
+
+nftnl_obj_set_str(t, NFTNL_OBJ_CT_HELPER_NAME, argv[4]);
+l4proto = parse_l4proto(argv[5]);
+nftnl_obj_set_u8(t, NFTNL_OBJ_CT_HELPER_L4PROTO, l4proto);
+if (argc == 7) {
+l3proto = parse_family(argv[6], "l3proto");
+nftnl_obj_set_u16(t, NFTNL_OBJ_CT_HELPER_L3PROTO, l3proto);
+}
+
+return t;
+}
+
+int main(int argc, char *argv[])
+{
+struct nftnl_obj *t;
+uint32_t seq, obj_seq, family, portid;
+struct mnl_nlmsg_batch *batch;
+char buf[MNL_SOCKET_BUFFER_SIZE];
+struct nlmsghdr *nlh;
+struct mnl_socket *nl;
+int ret;
+
+if (argc < 6) {
+fprintf(stderr, "%s  
[l3proto]\n", argv[0]);
+exit(EXIT_FAILURE);
+}
+
+t = ct_helper_add_parse(argc, argv);
+if (t == NULL)
+exit(EXIT_FAILURE);
+
+seq = 

[PATCH nf] netfilter: nf_tables: add missing netlink attrs to policies

2018-03-20 Thread Florian Westphal
Fixes: 8aeff920dcc9 ("netfilter: nf_tables: add stateful object reference to 
set elements")
Fixes: f25ad2e907f1 ("netfilter: nf_tables: prepare for expressions associated 
to set elements")
Fixes: 1a94e38d254b ("netfilter: nf_tables: add NFTA_RULE_ID attribute")
Signed-off-by: Florian Westphal 
---
nb: there are a few more missing policies in nf-next as well.

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c2ad333dce1e..19354019c818 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1973,6 +1973,7 @@ static const struct nla_policy 
nft_rule_policy[NFTA_RULE_MAX + 1] = {
[NFTA_RULE_POSITION]= { .type = NLA_U64 },
[NFTA_RULE_USERDATA]= { .type = NLA_BINARY,
.len = NFT_USERDATA_MAXLEN },
+   [NFTA_RULE_ID]  = { .type = NLA_U32 },
 };
 
 static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
@@ -3380,6 +3381,8 @@ static const struct nla_policy 
nft_set_elem_policy[NFTA_SET_ELEM_MAX + 1] = {
[NFTA_SET_ELEM_TIMEOUT] = { .type = NLA_U64 },
[NFTA_SET_ELEM_USERDATA]= { .type = NLA_BINARY,
.len = NFT_USERDATA_MAXLEN },
+   [NFTA_SET_ELEM_EXPR]= { .type = NLA_NESTED },
+   [NFTA_SET_ELEM_OBJREF]  = { .type = NLA_STRING },
 };
 
 static const struct nla_policy nft_set_elem_list_policy[NFTA_SET_ELEM_LIST_MAX 
+ 1] = {
-- 
2.16.1

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


Re: [RFC PATCH] netfilter: nf_tables: nf_tables_allow_nat_conflict() can be static

2018-03-20 Thread Pablo Neira Ayuso
On Mon, Mar 19, 2018 at 04:39:33PM +0100, Florian Westphal wrote:
> kbuild test robot  wrote:
> > -bool nf_tables_allow_nat_conflict(const struct net *net,
> > - const struct nf_hook_ops *ops)
> > +static bool nf_tables_allow_nat_conflict(const struct net *net,
> > +const struct nf_hook_ops *ops)
> 
> Right, this has to be static.

Folded this patch here before applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH nf] netfilter: nf_tables: meter: pick a set backend that supports updates

2018-03-20 Thread Pablo Neira Ayuso
On Wed, Mar 14, 2018 at 01:37:58PM +0100, Florian Westphal wrote:
> in nftables, 'meter' can be used to instantiate a hash-table at run
> time:
> 
> rule add filter forward iif "internal" meter hostacct { ip saddr counter}
> nft list meter ip filter hostacct
> table ip filter {
>   meter hostacct {
> type ipv4_addr
> elements = { 192.168.0.1 : counter packets 8 bytes 2672, ..
> 
> because elemets get added on the fly, the kernel must chose a set
> backend type that implements the ->update() function, otherwise
> rule insertion fails with EOPNOTSUPP.
> 
> Therefore, skip set types that lack ->update, and also
> make sure we do not discard a (bad) candidate when we did yet
> find any candidate at all.  This could happen when userspace prefers
> low memory footprint -- the set implementation currently checked might
> not be a fit at all.  Make sure we pick it anyway (!bops).  In
> case next candidate is a better fix, it will be chosen instead.
> 
> But in case nothing else is found we at least have a non-ideal
> match rather than no match at all.

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


Re: [PATCH nf] netfilter: nf_tables: permit second nat hook if colliding hook is going away

2018-03-20 Thread Pablo Neira Ayuso
On Sun, Mar 18, 2018 at 07:22:39PM +0100, Florian Westphal wrote:
> Sergei Trofimovich reported that restoring an nft ruleset doesn't work
> anymore unless old rule content is flushed first.
> 
> The problem stems from a recent change designed to prevent multiple nat
> hooks at the same hook point locations and nftables transaction model.
> 
> A 'flush ruleset' won't take effect until the entire transaction has
> completed.
> 
> So, if one has a nft.rules file that contains a 'flush ruleset',
> followed by a nat hook register request, then 'nft -f file' will work,
> but running 'nft -f file' again will fail with -EBUSY.
> 
> Reason is that nftables will place the flush/removal requests in the
> transaction list, but it will not act on the removal until after all new
> rules are in place.
> 
> The netfilter core will therefore get request to register a new nat
> hook before the old one is removed -- this now fails as the netfilter
> core can't know the existing hook is staged for removal.
> 
> To fix this, we can search the transaction log when a hook collision
> is detected.  The collision is okay if
> 
>  1. there is a delete request pending for the nat hook that is already
> registered.
>  2. there is no second add request for a matching nat hook.
> This is required to only apply the exception once.

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


Re: [PATCH] Net: netfilter: Replace printk() with pr_*() and define pr_fmt()

2018-03-20 Thread Pablo Neira Ayuso
On Mon, Mar 12, 2018 at 06:36:29PM +0530, Arushi Singhal wrote:
> Using pr_() is more concise than printk(KERN_).
> This patch:
> * Replace printks having a log level with the appropriate
> pr_*() macros.
> * Define pr_fmt() to include relevant name.
> * Remove redundant prefixes from pr_*() calls.
> * Indent the code where possible.
> * Remove the useless output messages.
> * Remove periods from messages.

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


Re: [PATCH] netfilter: nfnetlink_cthelper: Remove VLA usage

2018-03-20 Thread Pablo Neira Ayuso
On Mon, Mar 12, 2018 at 07:21:38PM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wvla, remove VLA and replace it
> with dynamic memory allocation.
> 
> From a security viewpoint, the use of Variable Length Arrays can be
> a vector for stack overflow attacks. Also, in general, as the code
> evolves it is easy to lose track of how big a VLA can get. Thus, we
> can end up having segfaults that are hard to debug.
> 
> Also, fixed as part of the directive to remove all VLAs from
> the kernel: https://lkml.org/lkml/2018/3/7/621

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


Re: [PATCH] xt_conntrack: Support bit-shifting for CONNMARK & MARK targets.

2018-03-20 Thread Pablo Neira Ayuso
On Mon, Mar 19, 2018 at 09:41:59AM +1300, Jack Ma wrote:
> This patch introduces a new feature that allows bitshifting (left
> and right) operations to co-operate with existing iptables options.

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


Re: [PATCH] netfilter: ebtables: use ADD_COUNTER macro

2018-03-20 Thread Pablo Neira Ayuso
On Wed, Mar 14, 2018 at 11:36:53PM +0900, Taehee Yoo wrote:
> xtables uses ADD_COUNTER macro to increase
> packet and byte count. ebtables also can use this.

Applied to nf-next, thanks.

Please, specify your tree target next time, ie.

[PATCH nf-next] blah

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


Re: [PATCH v2] netfilter: nf_tables: remove VLA usage

2018-03-20 Thread Pablo Neira Ayuso
On Mon, Mar 12, 2018 at 10:16:17PM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wvla, remove VLA and replace it
> with dynamic memory allocation.
> 
> From a security viewpoint, the use of Variable Length Arrays can be
> a vector for stack overflow attacks. Also, in general, as the code
> evolves it is easy to lose track of how big a VLA can get. Thus, we
> can end up having segfaults that are hard to debug.
> 
> Also, fixed as part of the directive to remove all VLAs from
> the kernel: https://lkml.org/lkml/2018/3/7/621

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


Re: [PATCH] netfilter: cttimeout: remove VLA usage

2018-03-20 Thread Pablo Neira Ayuso
On Mon, Mar 12, 2018 at 06:14:42PM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wvla, remove VLA and replace it
> with dynamic memory allocation.
> 
> From a security viewpoint, the use of Variable Length Arrays can be
> a vector for stack overflow attacks. Also, in general, as the code
> evolves it is easy to lose track of how big a VLA can get. Thus, we
> can end up having segfaults that are hard to debug.
> 
> Also, fixed as part of the directive to remove all VLAs from
> the kernel: https://lkml.org/lkml/2018/3/7/621

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


Re: [PATCH 1/2 conntrackd] src: add ARRAY_SIZE definition

2018-03-20 Thread Jan Engelhardt

On Tuesday 2018-03-20 12:47, Pablo Neira Ayuso wrote:

>Signed-off-by: Pablo Neira Ayuso 
>---
> include/conntrackd.h | 4 
> include/helper.h | 2 --
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/include/conntrackd.h b/include/conntrackd.h
>index ece702543082..81dff221e96d 100644
>--- a/include/conntrackd.h
>+++ b/include/conntrackd.h
>@@ -308,4 +308,8 @@ void select_main_loop(void);
> int
> init_config(char *filename);
> 
>+#ifndef ARRAY_SIZE
>+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>+#endif

You probably want to add the array check to ARRAY_SIZE, cf.
https://sourceforge.net/p/libhx/libhx/ci/master/tree/include/libHX/defs.h#l152
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH libmnl] attr: zero attribute padding

2018-03-20 Thread Pablo Neira Ayuso
On Sun, Mar 18, 2018 at 07:37:41PM +0100, Florian Westphal wrote:
> Sergei Trofimovich reports 'uninitialized bytes' warnings from nftables:
> 
> Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s)
>at 0x55B9EFB: sendmsg (in /lib64/libc-2.25.so)
>by 0x43E658: mnl_nft_socket_sendmsg (mnl.c:239)
>by 0x43E658: mnl_batch_talk (mnl.c:254)
>by 0x407898: nft_netlink (libnftables.c:58)
>by 0x407898: nft_run (libnftables.c:96)
>by 0x407CD5: nft_run_cmd_from_buffer (libnftables.c:291)
>by 0x406EDE: main (main.c:274)
> 
> This is harmless, the uninitialized memory is the padding
> that sometimes needs to be inserted between end of an attribute
> and the beginning of the new attribute.
> 
> Zero it to silence memory sanitizer output.
> 
> Signed-off-by: Florian Westphal 

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


Re: [nft PATCH 0/6] A set of patches resulting from running tests/shell

2018-03-20 Thread Pablo Neira Ayuso
On Mon, Mar 19, 2018 at 06:02:01PM +0100, Phil Sutter wrote:
> This series is the result of me trying to get all tests in tests/shell
> to pass. Sadly I wasn't fully successful, these two still fail:
> 
> - testcases/sets/0028autoselect_0
> - testcases/sets/0031set_timeout_size_0

These need kernel fixes, yes.

> I had a look at the latter, the problem seems to be that nft_set_hash.c
> in kernel prefers nft_hash_fast_ops for four byte keys ignoring the fact
> that NFT_SET_TIMEOUT support is required.

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


Re: [PATCH 2/2 nf-next] netfilter: SYNPROXY: don't proxy invalid PSH,ACK packets

2018-03-20 Thread Florian Westphal
Pablo Neira Ayuso  wrote:
> On Tue, Mar 20, 2018 at 12:43:33PM +0100, Florian Westphal wrote:
> > I don't understand why push,ack is invalid in first place.
> > If we do not have a valid connection at this point then a pure
> > ack would have same effect (reset), no?
> 
> Under normal circunstances yes, but anyone could cycle and forged
> PSH,ACK packets to the synproxy that are invalid, right? That would
> open up for a DOS against SYNPROXY itself...

No, because synproxy validates the ack sequence number with syncookie
hash. The ack is dropped if the recomputed cookie doesn't match.

So this patch isn't needed fortunately.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2 nf-next] netfilter: SYNPROXY: don't proxy invalid PSH,ACK packets

2018-03-20 Thread Pablo Neira Ayuso
On Tue, Mar 20, 2018 at 12:43:33PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso  wrote:
> > The example rule in the iptables-extensions(8) manpage suggests:
> > 
> > iptables -A INPUT -i eth0 -p tcp --dport 80
> >   -m state --state UNTRACKED,INVALID -j SYNPROXY
> >   --sack-perm --timestamp --mss 1460 --wscale 9
> > 
> > This is allowing invalid PSH,ACK packets to enter the SYNPROXY target
> > since they are matching the INVALID state.
> 
> Why INVALID?  Shouldn't we already have a valid conntrack entry at this point?
>
> > This results in the following sequence:
> >  client   synproxy server
> >| |   |
> >| |   |
> >... 3-way handshake via synproxy ...
> >| |   |
> >|>|   |
> >|   psh,ack   |-->|
> >| |syn|   uh, why syn? I'm
> >| |   | already established?!
> >| |<--|
> >|<|rst|
> >| rst |   |
> > 
> > Problem is that the psh,ack packet is handled by SYNPROXY as it would be
> > the ACK packet (the third step in the handshake), hence SYNPROXY sends
> > it as as SYN packet to the server. The server is not happy, since the
> > handshake already happened time ago, so it sends the RST packet and
> > connection is teared down immediately.
> 
> I don't understand why push,ack is invalid in first place.
> If we do not have a valid connection at this point then a pure
> ack would have same effect (reset), no?

Under normal circunstances yes, but anyone could cycle and forged
PSH,ACK packets to the synproxy that are invalid, right? That would
open up for a DOS against SYNPROXY itself...

As you said, this can be worst, a simple invalid ACK packet would
result in a SYN packet to the server for an already established
connection.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2 conntrackd] src: add ARRAY_SIZE definition

2018-03-20 Thread Pablo Neira Ayuso
Signed-off-by: Pablo Neira Ayuso 
---
 include/conntrackd.h | 4 
 include/helper.h | 2 --
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/conntrackd.h b/include/conntrackd.h
index ece702543082..81dff221e96d 100644
--- a/include/conntrackd.h
+++ b/include/conntrackd.h
@@ -308,4 +308,8 @@ void select_main_loop(void);
 int
 init_config(char *filename);
 
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#endif
+
 #endif
diff --git a/include/helper.h b/include/helper.h
index 3c0c5e00d6c5..d15c1c62c053 100644
--- a/include/helper.h
+++ b/include/helper.h
@@ -68,8 +68,6 @@ struct ctd_helper *helper_find(const char *libdir_path, const 
char *name, uint8_
type __max2 = (y);  \
__max1 > __max2 ? __max1: __max2; })
 
-#define ARRAY_SIZE MNL_ARRAY_SIZE
-
 enum ip_conntrack_dir {
IP_CT_DIR_ORIGINAL,
IP_CT_DIR_REPLY,
-- 
2.11.0

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


[PATCH 2/2 conntrackd] conntrackd: add TCP flag support

2018-03-20 Thread Pablo Neira Ayuso
Back in 2008, there was no TCP flags support in the kernel, hence the
workaround was to infer the flags from the TCP state.

This patch is implicitly fixing a problem, since the existing RETRANS
and UNACK TCP conntrack states plus the _CLOSE_INIT flag that is bogusly
infered (to be frank, it was correctly infered back in 2008, but after
adding new TCP states, it was not).

Let's just use the flags that we get via synchronization messages.

Signed-off-by: Pablo Neira Ayuso 
---
 src/netlink.c | 56 +++-
 1 file changed, 19 insertions(+), 37 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 189f55a47efb..ddf4cf496f9a 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -216,12 +216,26 @@ int nl_get_conntrack(struct nfct_handle *h, const struct 
nf_conntrack *ct)
return ret;
 }
 
+static void ctd_force_tcp_be_liberal(struct nf_conntrack *ct)
+{
+   int attrs[4] = { ATTR_TCP_FLAGS_ORIG, ATTR_TCP_MASK_ORIG,
+ATTR_TCP_FLAGS_REPL, ATTR_TCP_MASK_REPL };
+   unsigned int i;
+   uint8_t flags;
+
+   for (i = 0; i < ARRAY_SIZE(attrs); i++) {
+   flags = nfct_get_attr_u8(ct, attrs[i]);
+   nfct_set_attr_u8(ct, attrs[i],
+flags | IP_CT_TCP_FLAG_BE_LIBERAL);
+   }
+}
+
 int nl_create_conntrack(struct nfct_handle *h, 
const struct nf_conntrack *orig,
int timeout)
 {
-   int ret;
struct nf_conntrack *ct;
+   int ret;
 
ct = nfct_clone(orig);
if (ct == NULL)
@@ -240,24 +254,8 @@ int nl_create_conntrack(struct nfct_handle *h,
nfct_setobjopt(ct, NFCT_SOPT_SETUP_REPLY);
 
/* disable TCP window tracking for recovered connections if required */
-   if (nfct_attr_is_set(ct, ATTR_TCP_STATE)) {
-   uint8_t flags = IP_CT_TCP_FLAG_SACK_PERM;
-
-   if (!CONFIG(sync).tcp_window_tracking)
-   flags |= IP_CT_TCP_FLAG_BE_LIBERAL;
-   else
-   flags |= IP_CT_TCP_FLAG_WINDOW_SCALE;
-
-   /* FIXME: workaround, we should send TCP flags in updates */
-   if (nfct_get_attr_u8(ct, ATTR_TCP_STATE) >=
-   TCP_CONNTRACK_TIME_WAIT) {
-   flags |= IP_CT_TCP_FLAG_CLOSE_INIT;
-   }
-   nfct_set_attr_u8(ct, ATTR_TCP_FLAGS_ORIG, flags);
-   nfct_set_attr_u8(ct, ATTR_TCP_MASK_ORIG, flags);
-   nfct_set_attr_u8(ct, ATTR_TCP_FLAGS_REPL, flags);
-   nfct_set_attr_u8(ct, ATTR_TCP_MASK_REPL, flags);
-   }
+   if (!CONFIG(sync).tcp_window_tracking)
+   ctd_force_tcp_be_liberal(ct);
 
ret = nfct_query(h, NFCT_Q_CREATE, ct);
nfct_destroy(ct);
@@ -307,24 +305,8 @@ int nl_update_conntrack(struct nfct_handle *h,
}
 
/* disable TCP window tracking for recovered connections if required */
-   if (nfct_attr_is_set(ct, ATTR_TCP_STATE)) {
-   uint8_t flags = IP_CT_TCP_FLAG_SACK_PERM;
-
-   if (!CONFIG(sync).tcp_window_tracking)
-   flags |= IP_CT_TCP_FLAG_BE_LIBERAL;
-   else
-   flags |= IP_CT_TCP_FLAG_WINDOW_SCALE;
-
-   /* FIXME: workaround, we should send TCP flags in updates */
-   if (nfct_get_attr_u8(ct, ATTR_TCP_STATE) >=
-   TCP_CONNTRACK_TIME_WAIT) {
-   flags |= IP_CT_TCP_FLAG_CLOSE_INIT;
-   }
-   nfct_set_attr_u8(ct, ATTR_TCP_FLAGS_ORIG, flags);
-   nfct_set_attr_u8(ct, ATTR_TCP_MASK_ORIG, flags);
-   nfct_set_attr_u8(ct, ATTR_TCP_FLAGS_REPL, flags);
-   nfct_set_attr_u8(ct, ATTR_TCP_MASK_REPL, flags);
-   }
+   if (!CONFIG(sync).tcp_window_tracking)
+   ctd_force_tcp_be_liberal(ct);
 
ret = nfct_query(h, NFCT_Q_UPDATE, ct);
nfct_destroy(ct);
-- 
2.11.0

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


[PATCH nf-next,RFC] netfilter: nf_conntrack_tcp: reset entry only from CLOSE and TIME_WAIT states

2018-03-20 Thread Pablo Neira Ayuso
Before this patch, if TCP state is < TIME_WAIT, we skip this logic.
Along time, we got new states over the TIME_WAIT value, such as
SYN_SENT2, RETRANS and UNACK, that can trigger this conntrack entry
reset, however this check was never updated.

My understanding is that we should only exercise the conntrack reset
codepath if the existing packet is triggering a transition to SYN_SENT
state while we were in TIME_WAIT or CLOSE states.

Signed-off-by: Pablo Neira Ayuso 
---
While reviewing TCP tracking code, I noticed this. This is just narrowing
down this codepath so it only works with TIME_WAIT and CLOSE states.

 net/netfilter/nf_conntrack_proto_tcp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c 
b/net/netfilter/nf_conntrack_proto_tcp.c
index e97cdc1cf98c..c765d83db574 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -827,7 +827,8 @@ static int tcp_packet(struct nf_conn *ct,
 
switch (new_state) {
case TCP_CONNTRACK_SYN_SENT:
-   if (old_state < TCP_CONNTRACK_TIME_WAIT)
+   if (old_state != TCP_CONNTRACK_TIME_WAIT &&
+   old_state != TCP_CONNTRACK_CLOSE)
break;
/* RFC 1122: "When a connection is closed actively,
 * it MUST linger in TIME-WAIT state for a time 2xMSL
-- 
2.11.0

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


Re: [PATCH 2/2 nf-next] netfilter: SYNPROXY: don't proxy invalid PSH,ACK packets

2018-03-20 Thread Florian Westphal
Pablo Neira Ayuso  wrote:
> The example rule in the iptables-extensions(8) manpage suggests:
> 
>   iptables -A INPUT -i eth0 -p tcp --dport 80
>   -m state --state UNTRACKED,INVALID -j SYNPROXY
>   --sack-perm --timestamp --mss 1460 --wscale 9
> 
> This is allowing invalid PSH,ACK packets to enter the SYNPROXY target
> since they are matching the INVALID state.

Why INVALID?  Shouldn't we already have a valid conntrack entry at this point?

> This results in the following sequence:
>  client   synproxy server
>| |   |
>| |   |
>... 3-way handshake via synproxy ...
>| |   |
>|>|   |
>|   psh,ack   |-->|
>| |syn|   uh, why syn? I'm
>| |   | already established?!
>| |<--|
>|<|rst|
>| rst |   |
> 
> Problem is that the psh,ack packet is handled by SYNPROXY as it would be
> the ACK packet (the third step in the handshake), hence SYNPROXY sends
> it as as SYN packet to the server. The server is not happy, since the
> handshake already happened time ago, so it sends the RST packet and
> connection is teared down immediately.

I don't understand why push,ack is invalid in first place.
If we do not have a valid connection at this point then a pure
ack would have same effect (reset), no?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2 nf-next] netfilter: SYNPROXY: don't proxy invalid PSH,ACK packets

2018-03-20 Thread Pablo Neira Ayuso
The example rule in the iptables-extensions(8) manpage suggests:

iptables -A INPUT -i eth0 -p tcp --dport 80
  -m state --state UNTRACKED,INVALID -j SYNPROXY
  --sack-perm --timestamp --mss 1460 --wscale 9

This is allowing invalid PSH,ACK packets to enter the SYNPROXY target
since they are matching the INVALID state.

This results in the following sequence:

 client   synproxy server
   | |   |
   | |   |
   ... 3-way handshake via synproxy ...
   | |   |
   |>|   |
   |   psh,ack   |-->|
   | |syn|   uh, why syn? I'm
   | |   | already established?!
   | |<--|
   |<|rst|
   | rst |   |

Problem is that the psh,ack packet is handled by SYNPROXY as it would be
the ACK packet (the third step in the handshake), hence SYNPROXY sends
it as as SYN packet to the server. The server is not happy, since the
handshake already happened time ago, so it sends the RST packet and
connection is teared down immediately.

Signed-off-by: Pablo Neira Ayuso 
---
 net/ipv4/netfilter/ipt_SYNPROXY.c  | 2 +-
 net/ipv6/netfilter/ip6t_SYNPROXY.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c 
b/net/ipv4/netfilter/ipt_SYNPROXY.c
index 690b17ef6a44..dfc84d954c34 100644
--- a/net/ipv4/netfilter/ipt_SYNPROXY.c
+++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
@@ -296,7 +296,7 @@ synproxy_tg4(struct sk_buff *skb, const struct 
xt_action_param *par)
synproxy_send_client_synack(net, skb, th, );
consume_skb(skb);
return NF_STOLEN;
-   } else if (th->ack && !(th->fin || th->rst || th->syn)) {
+   } else if (th->ack && !(th->fin || th->rst || th->syn || th->psh)) {
/* ACK from client */
if (synproxy_recv_client_ack(net, skb, th, , 
ntohl(th->seq))) {
consume_skb(skb);
diff --git a/net/ipv6/netfilter/ip6t_SYNPROXY.c 
b/net/ipv6/netfilter/ip6t_SYNPROXY.c
index cb6d42b03cb5..1f4f1e24fd27 100644
--- a/net/ipv6/netfilter/ip6t_SYNPROXY.c
+++ b/net/ipv6/netfilter/ip6t_SYNPROXY.c
@@ -311,7 +311,7 @@ synproxy_tg6(struct sk_buff *skb, const struct 
xt_action_param *par)
consume_skb(skb);
return NF_STOLEN;
 
-   } else if (th->ack && !(th->fin || th->rst || th->syn)) {
+   } else if (th->ack && !(th->fin || th->rst || th->syn || th->psh)) {
/* ACK from client */
if (synproxy_recv_client_ack(net, skb, th, , 
ntohl(th->seq))) {
consume_skb(skb);
-- 
2.11.0

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


[PATCH 1/2 nf-next,v2] netfilter: ctnetlink: synproxy support

2018-03-20 Thread Pablo Neira Ayuso
This patch exposes synproxy information per-conntrack. Moreover, send
sequence adjustment events once server sends us the SYN,ACK packet, so
we can synchronize the sequence adjustment too for packets going as
reply from the server, as part of the synproxy logic.

Signed-off-by: Pablo Neira Ayuso 
---
v2: missing sequence adjustment event information, otherwise adjustment
in packets coming from the server after synproxy does not ever happen,
leading to TCP RST.

 include/uapi/linux/netfilter/nf_conntrack_common.h |  1 +
 include/uapi/linux/netfilter/nfnetlink_conntrack.h | 10 +++
 net/ipv4/netfilter/ipt_SYNPROXY.c  |  8 +-
 net/ipv6/netfilter/ip6t_SYNPROXY.c |  8 +-
 net/netfilter/nf_conntrack_netlink.c   | 87 +-
 5 files changed, 109 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h 
b/include/uapi/linux/netfilter/nf_conntrack_common.h
index 9574bd40870b..c712eb6879f1 100644
--- a/include/uapi/linux/netfilter/nf_conntrack_common.h
+++ b/include/uapi/linux/netfilter/nf_conntrack_common.h
@@ -129,6 +129,7 @@ enum ip_conntrack_events {
IPCT_NATSEQADJ = IPCT_SEQADJ,
IPCT_SECMARK,   /* new security mark has been set */
IPCT_LABEL, /* new connlabel has been set */
+   IPCT_SYNPROXY,  /* synproxy has been set */
 #ifdef __KERNEL__
__IPCT_MAX
 #endif
diff --git a/include/uapi/linux/netfilter/nfnetlink_conntrack.h 
b/include/uapi/linux/netfilter/nfnetlink_conntrack.h
index 7397e022ce6e..77987111cab0 100644
--- a/include/uapi/linux/netfilter/nfnetlink_conntrack.h
+++ b/include/uapi/linux/netfilter/nfnetlink_conntrack.h
@@ -54,6 +54,7 @@ enum ctattr_type {
CTA_MARK_MASK,
CTA_LABELS,
CTA_LABELS_MASK,
+   CTA_SYNPROXY,
__CTA_MAX
 };
 #define CTA_MAX (__CTA_MAX - 1)
@@ -190,6 +191,15 @@ enum ctattr_natseq {
 };
 #define CTA_NAT_SEQ_MAX (__CTA_NAT_SEQ_MAX - 1)
 
+enum ctattr_synproxy {
+   CTA_SYNPROXY_UNSPEC,
+   CTA_SYNPROXY_ISN,
+   CTA_SYNPROXY_ITS,
+   CTA_SYNPROXY_TSOFF,
+   __CTA_SYNPROXY_MAX,
+};
+#define CTA_SYNPROXY_MAX (__CTA_SYNPROXY_MAX - 1)
+
 enum ctattr_expect {
CTA_EXPECT_UNSPEC,
CTA_EXPECT_MASTER,
diff --git a/net/ipv4/netfilter/ipt_SYNPROXY.c 
b/net/ipv4/netfilter/ipt_SYNPROXY.c
index f75fc6b53115..690b17ef6a44 100644
--- a/net/ipv4/netfilter/ipt_SYNPROXY.c
+++ b/net/ipv4/netfilter/ipt_SYNPROXY.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static struct iphdr *
 synproxy_build_ip(struct net *net, struct sk_buff *skb, __be32 saddr,
@@ -384,6 +385,8 @@ static unsigned int ipv4_synproxy_hook(void *priv,
synproxy->isn = ntohl(th->ack_seq);
if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
synproxy->its = opts.tsecr;
+
+   nf_conntrack_event_cache(IPCT_SYNPROXY, ct);
break;
case TCP_CONNTRACK_SYN_RECV:
if (!th->syn || !th->ack)
@@ -392,8 +395,10 @@ static unsigned int ipv4_synproxy_hook(void *priv,
if (!synproxy_parse_options(skb, thoff, th, ))
return NF_DROP;
 
-   if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
+   if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP) {
synproxy->tsoff = opts.tsval - synproxy->its;
+   nf_conntrack_event_cache(IPCT_SYNPROXY, ct);
+   }
 
opts.options &= ~(XT_SYNPROXY_OPT_MSS |
  XT_SYNPROXY_OPT_WSCALE |
@@ -403,6 +408,7 @@ static unsigned int ipv4_synproxy_hook(void *priv,
synproxy_send_server_ack(net, state, skb, th, );
 
nf_ct_seqadj_init(ct, ctinfo, synproxy->isn - ntohl(th->seq));
+   nf_conntrack_event_cache(IPCT_SEQADJ, ct);
 
swap(opts.tsval, opts.tsecr);
synproxy_send_client_ack(net, skb, th, );
diff --git a/net/ipv6/netfilter/ip6t_SYNPROXY.c 
b/net/ipv6/netfilter/ip6t_SYNPROXY.c
index 437af8c95277..cb6d42b03cb5 100644
--- a/net/ipv6/netfilter/ip6t_SYNPROXY.c
+++ b/net/ipv6/netfilter/ip6t_SYNPROXY.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static struct ipv6hdr *
 synproxy_build_ip(struct net *net, struct sk_buff *skb,
@@ -405,6 +406,8 @@ static unsigned int ipv6_synproxy_hook(void *priv,
synproxy->isn = ntohl(th->ack_seq);
if (opts.options & XT_SYNPROXY_OPT_TIMESTAMP)
synproxy->its = opts.tsecr;
+
+   nf_conntrack_event_cache(IPCT_SYNPROXY, ct);
break;
case TCP_CONNTRACK_SYN_RECV:
if (!th->syn || !th->ack)
@@ -413,8 +416,10 @@ static unsigned int ipv6_synproxy_hook(void *priv,
if (!synproxy_parse_options(skb, thoff, th, ))
return NF_DROP;
 
-   if (opts.options 

Re: [PATCH libnftnl] examples: add nft-ct-helper-{add,get,del}

2018-03-20 Thread Arturo Borrero Gonzalez
On 19 March 2018 at 18:19, Yang Zheng  wrote:
> nft-ct-helper-{add,get,del}: add, get, or delete ct helper objects from the 
> specified table.
>

It would be great if you extend a bit the commit message with your tests:

% ./nft-ct-helper-get 

% ./nft-ct-helper-add 
% ./nft-ct-helper-get 
[...]
% ./nft-ct-helper-del 
% ./nft-ct-helper-get 


So other people know about the expected results when running this.

> Signed-off-by: Yang Zheng 
> ---
>  examples/Makefile.am |  14 +++-
>  examples/nft-ct-helper-add.c | 149 ++
>  examples/nft-ct-helper-del.c | 124 +++
>  examples/nft-ct-helper-get.c | 150 
> +++
>  4 files changed, 436 insertions(+), 1 deletion(-)
>  create mode 100644 examples/nft-ct-helper-add.c
>  create mode 100644 examples/nft-ct-helper-del.c
>  create mode 100644 examples/nft-ct-helper-get.c
>

Other than that, it LGTM:

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


Re: [nft PATCH 0/6] A set of patches resulting from running tests/shell

2018-03-20 Thread Arturo Borrero Gonzalez
On 19 March 2018 at 18:02, Phil Sutter  wrote:
> This series is the result of me trying to get all tests in tests/shell
> to pass. Sadly I wasn't fully successful, these two still fail:
>
> - testcases/sets/0028autoselect_0
> - testcases/sets/0031set_timeout_size_0
>
> I had a look at the latter, the problem seems to be that nft_set_hash.c
> in kernel prefers nft_hash_fast_ops for four byte keys ignoring the fact
> that NFT_SET_TIMEOUT support is required.
>
> Phil Sutter (6):
>   Support 'nft -f -' to read from stdin
>   tests/shell: Fix dump of chains/0016delete_handle_0
>   tests/shell: Fix flowtable test cases
>   flowtable: Make parsing a little more robust
>   tests/shell: Fix sporadic fail of include/0007glob_double_0
>   tests/shell: Allow to specify multiple testcases
>

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