[PATCH] netfilter: nft_ct: add ct expectations support

[PATCH] netfilter: nft_ct: add ct expectations support

[Stéphane Veyret]

This patch allows to add, list and delete expectations via nft objref
infrastructure and assigning these expectations via nft rule.

Signed-off-by: Stéphane Veyret <sveyret@gmail.com>
---
include/uapi/linux/netfilter/nf_tables.h | 15 ++-
net/netfilter/nft_ct.c | 124 ++++++++++++++++++++++-
2 files changed, 136 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h
b/include/uapi/linux/netfilter/nf_tables.h
index f0cf7b0f4f35..0a3452ca684c 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -968,6 +968,7 @@ enum nft_socket_keys {
* @NFT_CT_DST_IP6: conntrack layer 3 protocol destination (IPv6 address)
* @NFT_CT_TIMEOUT: connection tracking timeout policy assigned to conntrack
* @NFT_CT_ID: conntrack id
+ * @NFT_CT_EXPECT: connection tracking expectation
*/
enum nft_ct_keys {
NFT_CT_STATE,
@@ -995,6 +996,7 @@ enum nft_ct_keys {
NFT_CT_DST_IP6,
NFT_CT_TIMEOUT,
NFT_CT_ID,
+ NFT_CT_EXPECT,
__NFT_CT_MAX
};
#define NFT_CT_MAX (__NFT_CT_MAX - 1)
@@ -1447,6 +1449,16 @@ enum nft_ct_timeout_timeout_attributes {
};
#define NFTA_CT_TIMEOUT_MAX (__NFTA_CT_TIMEOUT_MAX - 1)
+enum nft_ct_expectation_attributes {
+ NFTA_CT_EXPECT_UNSPEC,
+ NFTA_CT_EXPECT_L3PROTO,
+ NFTA_CT_EXPECT_L4PROTO,
+ NFTA_CT_EXPECT_DPORT,
+ NFTA_CT_EXPECT_TIMEOUT,
+ __NFTA_CT_EXPECT_MAX,
+};
+#define NFTA_CT_EXPECT_MAX (__NFTA_CT_EXPECT_MAX - 1)
+
#define NFT_OBJECT_UNSPEC 0
#define NFT_OBJECT_COUNTER 1
#define NFT_OBJECT_QUOTA 2
@@ -1456,7 +1468,8 @@ enum nft_ct_timeout_timeout_attributes {
#define NFT_OBJECT_TUNNEL 6
#define NFT_OBJECT_CT_TIMEOUT 7
#define NFT_OBJECT_SECMARK 8
-#define __NFT_OBJECT_MAX 9
+#define NFT_OBJECT_CT_EXPECT 9
+#define __NFT_OBJECT_MAX 10
#define NFT_OBJECT_MAX (__NFT_OBJECT_MAX - 1)
/**
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index f043936763f3..06c13b2dfb78 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -24,6 +24,7 @@
#include <net/netfilter/nf_conntrack_labels.h>
#include <net/netfilter/nf_conntrack_timeout.h>
#include <net/netfilter/nf_conntrack_l4proto.h>
+#include <net/netfilter/nf_conntrack_expect.h>
struct nft_ct {
enum nft_ct_keys key:8;
@@ -790,6 +791,117 @@ static struct nft_expr_type nft_notrack_type
__read_mostly = {
.owner = THIS_MODULE,
};
+struct nft_ct_expect_obj {
+ int l3num;
+ u8 l4proto;
+ __be16 dport;
+ u32 timeout;
+};
+
+static int nft_ct_expect_obj_init(const struct nft_ctx *ctx,
+ const struct nlattr * const tb[],
+ struct nft_object *obj)
+{
+ struct nft_ct_expect_obj *priv = nft_obj_data(obj);
+ int ret;
+
+ if (!tb[NFTA_CT_EXPECT_L4PROTO] ||
+ !tb[NFTA_CT_EXPECT_DPORT])
+ return -EINVAL;
+
+ priv->l3num = ctx->family;
+ if (tb[NFTA_CT_EXPECT_L3PROTO])
+ priv->l3num = ntohs(nla_get_be16(tb[NFTA_CT_EXPECT_L3PROTO]));
+ priv->l4proto = nla_get_u8(tb[NFTA_CT_EXPECT_L4PROTO]);
+
+ priv->dportmin = nla_get_be16(tb[NFTA_CT_EXPECT_DPORT_MIN]);
+
+ priv->timeout = 0;
+ if (tb[NFTA_CT_EXPECT_TIMEOUT])
+ priv->timeout = nla_get_u32(tb[NFTA_CT_EXPECT_TIMEOUT]);
+
+ ret = nf_ct_netns_get(ctx->net, ctx->family);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static void nft_ct_expect_obj_destroy(const struct nft_ctx *ctx,
+ struct nft_object *obj)
+{
+ nf_ct_netns_put(ctx->net, ctx->family);
+}
+
+static int nft_ct_expect_obj_dump(struct sk_buff *skb,
+ struct nft_object *obj, bool reset)
+{
+ const struct nft_ct_expect_obj *priv = nft_obj_data(obj);
+
+ if (nla_put_be16(skb, NFTA_CT_EXPECT_L3PROTO, htons(priv->l3num)) ||
+ nla_put_u8(skb, NFTA_CT_EXPECT_L4PROTO, priv->l4proto) ||
+ nla_put_be16(skb, NFTA_CT_EXPECT_DPORT, priv->dport) ||
+ nla_put_u32(skb, NFTA_CT_EXPECT_TIMEOUT, priv->timeout))
+ return -1
+
+ return 0;
+}
+
+static void nft_ct_expect_obj_eval(struct nft_object *obj,
+ struct nft_regs *regs,
+ const struct nft_pktinfo *pkt)
+{
+ const struct nft_ct_expect_obj *priv = nft_obj_data(obj);
+ enum ip_conntrack_info ctinfo;
+ struct nf_conn *ct = nf_ct_get(pkt->skb, ctinfo);
+ int dir = CTINFO2DIR(ctinfo);
+ struct nf_conntrack_expect *exp;
+
+ exp = nf_ct_expect_alloc(ct);
+ if (exp == NULL) {
+ nf_ct_helper_log(skb, ct, "cannot allocate expectation");
+ regs->verdict.code = NF_DROP;
+ return;
+ }
+
+ nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT, priv->l3num,
+ &ct->tuplehash[!dir].tuple.src.u3, &ct->tuplehash[!dir].tuple.dst.u3,
+ priv->l4proto, NULL, &priv->dport);
+ if (priv->timeout)
+ exp->timeout.expires = jiffies + priv->timeout * HZ;
+
+ if (nf_ct_expect_related(exp) != 0) {
+ nf_ct_helper_log(skb, ct, "cannot add expectation");
+ regs->verdict.code = NF_DROP;
+ }
+}
+
+static const struct nla_policy nft_ct_expect_policy[NFTA_CT_EXPECT_MAX
+ 1] = {
+ [NFTA_CT_EXPECT_L3PROTO] = {.type = NLA_U16 },
+ [NFTA_CT_EXPECT_L4PROTO] = {.type = NLA_U8 },
+ [NFTA_CT_EXPECT_DPORT] = {.type = NLA_U16 },
+ [NFTA_CT_EXPECT_TIMEOUT] = {.type = NLA_U32 },
+};
+
+static struct nft_object_type nft_ct_expect_obj_type;
+
+static const struct nft_object_ops nft_ct_expect_obj_ops = {
+ .type = &nft_ct_expect_obj_type,
+ .size = sizeof(struct nft_ct_expect_obj),
+ .eval = nft_ct_expect_obj_eval,
+ .init = nft_ct_expect_obj_init,
+ .destroy = nft_ct_expect_obj_destroy,
+ .dump = nft_ct_expect_obj_dump,
+};
+
+static struct nft_object_type nft_ct_expect_obj_type __read_mostly = {
+ .type = NFT_OBJECT_CT_EXPECT,
+ .ops = &nft_ct_expect_obj_ops,
+ .maxattr = NFTA_CT_EXPECT_MAX,
+ .policy = nft_ct_expect_policy,
+ .owner = THIS_MODULE,
+};
+
#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
static int
nft_ct_timeout_parse_policy(void *timeouts,
@@ -1173,17 +1285,23 @@ static int __init nft_ct_module_init(void)
err = nft_register_obj(&nft_ct_helper_obj_type);
if (err < 0)
goto err2;
+
+ err = nft_register_obj(&nft_ct_expect_obj_type);
+ if (err < 0)
+ goto err3;
#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
err = nft_register_obj(&nft_ct_timeout_obj_type);
if (err < 0)
- goto err3;
+ goto err4;
#endif
return 0;
#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
+err4:
+ nft_unregister_obj(&nft_ct_expect_obj_type);
+#endif
err3:
nft_unregister_obj(&nft_ct_helper_obj_type);
-#endif
err2:
nft_unregister_expr(&nft_notrack_type);
err1:
@@ -1196,6 +1314,7 @@ static void __exit nft_ct_module_exit(void)
#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
nft_unregister_obj(&nft_ct_timeout_obj_type);
#endif
+ nft_unregister_obj(&nft_ct_expect_obj_type);
nft_unregister_obj(&nft_ct_helper_obj_type);
nft_unregister_expr(&nft_notrack_type);
nft_unregister_expr(&nft_ct_type);
@@ -1210,3 +1329,4 @@ MODULE_ALIAS_NFT_EXPR("ct");
MODULE_ALIAS_NFT_EXPR("notrack");
MODULE_ALIAS_NFT_OBJ(NFT_OBJECT_CT_HELPER);
MODULE_ALIAS_NFT_OBJ(NFT_OBJECT_CT_TIMEOUT);
+MODULE_ALIAS_NFT_OBJ(NFT_OBJECT_CT_EXPECT);

-- 
2.21.0

Re: [PATCH] netfilter: nft_ct: add ct expectations support

[Jeremy Sowden]

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

On 2019-05-04, at 19:35:41 +0200, Stéphane Veyret wrote:
> This patch allows to add, list and delete expectations via nft objref
> infrastructure and assigning these expectations via nft rule.
>
> Signed-off-by: Stéphane Veyret <sveyret@gmail.com>
> ---
> include/uapi/linux/netfilter/nf_tables.h | 15 ++-
> net/netfilter/nft_ct.c | 124 ++++++++++++++++++++++-
> 2 files changed, 136 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/netfilter/nf_tables.h
> b/include/uapi/linux/netfilter/nf_tables.h
> index f0cf7b0f4f35..0a3452ca684c 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -968,6 +968,7 @@ enum nft_socket_keys {
> * @NFT_CT_DST_IP6: conntrack layer 3 protocol destination (IPv6 address)
> * @NFT_CT_TIMEOUT: connection tracking timeout policy assigned to conntrack
> * @NFT_CT_ID: conntrack id
> + * @NFT_CT_EXPECT: connection tracking expectation
> */
> enum nft_ct_keys {
> NFT_CT_STATE,
> @@ -995,6 +996,7 @@ enum nft_ct_keys {
> NFT_CT_DST_IP6,
> NFT_CT_TIMEOUT,
> NFT_CT_ID,
> + NFT_CT_EXPECT,
> __NFT_CT_MAX
> };
> #define NFT_CT_MAX (__NFT_CT_MAX - 1)

Your patch has been mangled.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] netfilter: nft_ct: add ct expectations support

[Stéphane Veyret]

Le dim. 5 mai 2019 à 15:24, Jeremy Sowden <jeremy@azazel.net> a écrit :
> Your patch has been mangled.

Thank you for warning me. A problem with git and GMail which seem not
to be good friend, but I think I managed to configure everything
correctly now. I will send it again in a few minutes.

Stéphane.

[PATCH] netfilter: nft_ct: add ct expectations support

[Stéphane Veyret]

This patch allows to add, list and delete expectations via nft objref
infrastructure and assigning these expectations via nft rule.

Signed-off-by: Stéphane Veyret <sveyret@gmail.com>
---
 include/uapi/linux/netfilter/nf_tables.h |  15 ++-
 net/netfilter/nft_ct.c                   | 124 ++++++++++++++++++++++-
 2 files changed, 136 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index f0cf7b0f4f35..0a3452ca684c 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -968,6 +968,7 @@ enum nft_socket_keys {
  * @NFT_CT_DST_IP6: conntrack layer 3 protocol destination (IPv6 address)
  * @NFT_CT_TIMEOUT: connection tracking timeout policy assigned to conntrack
  * @NFT_CT_ID: conntrack id
+ * @NFT_CT_EXPECT: connection tracking expectation
  */
 enum nft_ct_keys {
 	NFT_CT_STATE,
@@ -995,6 +996,7 @@ enum nft_ct_keys {
 	NFT_CT_DST_IP6,
 	NFT_CT_TIMEOUT,
 	NFT_CT_ID,
+	NFT_CT_EXPECT,
 	__NFT_CT_MAX
 };
 #define NFT_CT_MAX		(__NFT_CT_MAX - 1)
@@ -1447,6 +1449,16 @@ enum nft_ct_timeout_timeout_attributes {
 };
 #define NFTA_CT_TIMEOUT_MAX	(__NFTA_CT_TIMEOUT_MAX - 1)
 
+enum nft_ct_expectation_attributes {
+	NFTA_CT_EXPECT_UNSPEC,
+	NFTA_CT_EXPECT_L3PROTO,
+	NFTA_CT_EXPECT_L4PROTO,
+	NFTA_CT_EXPECT_DPORT,
+	NFTA_CT_EXPECT_TIMEOUT,
+	__NFTA_CT_EXPECT_MAX,
+};
+#define NFTA_CT_EXPECT_MAX	(__NFTA_CT_EXPECT_MAX - 1)
+
 #define NFT_OBJECT_UNSPEC	0
 #define NFT_OBJECT_COUNTER	1
 #define NFT_OBJECT_QUOTA	2
@@ -1456,7 +1468,8 @@ enum nft_ct_timeout_timeout_attributes {
 #define NFT_OBJECT_TUNNEL	6
 #define NFT_OBJECT_CT_TIMEOUT	7
 #define NFT_OBJECT_SECMARK	8
-#define __NFT_OBJECT_MAX	9
+#define NFT_OBJECT_CT_EXPECT	9
+#define __NFT_OBJECT_MAX	10
 #define NFT_OBJECT_MAX		(__NFT_OBJECT_MAX - 1)
 
 /**
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index f043936763f3..06c13b2dfb78 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -24,6 +24,7 @@
 #include <net/netfilter/nf_conntrack_labels.h>
 #include <net/netfilter/nf_conntrack_timeout.h>
 #include <net/netfilter/nf_conntrack_l4proto.h>
+#include <net/netfilter/nf_conntrack_expect.h>
 
 struct nft_ct {
 	enum nft_ct_keys	key:8;
@@ -790,6 +791,117 @@ static struct nft_expr_type nft_notrack_type __read_mostly = {
 	.owner		= THIS_MODULE,
 };
 
+struct nft_ct_expect_obj {
+	int			l3num;
+	u8			l4proto;
+	__be16		dport;
+	u32			timeout;
+};
+
+static int nft_ct_expect_obj_init(const struct nft_ctx *ctx,
+				   const struct nlattr * const tb[],
+				   struct nft_object *obj)
+{
+	struct nft_ct_expect_obj *priv = nft_obj_data(obj);
+	int ret;
+
+	if (!tb[NFTA_CT_EXPECT_L4PROTO] ||
+	    !tb[NFTA_CT_EXPECT_DPORT])
+		return -EINVAL;
+
+	priv->l3num = ctx->family;
+	if (tb[NFTA_CT_EXPECT_L3PROTO])
+		priv->l3num = ntohs(nla_get_be16(tb[NFTA_CT_EXPECT_L3PROTO]));
+	priv->l4proto = nla_get_u8(tb[NFTA_CT_EXPECT_L4PROTO]);
+
+	priv->dportmin = nla_get_be16(tb[NFTA_CT_EXPECT_DPORT_MIN]);
+
+	priv->timeout = 0;
+	if (tb[NFTA_CT_EXPECT_TIMEOUT])
+		priv->timeout = nla_get_u32(tb[NFTA_CT_EXPECT_TIMEOUT]);
+
+	ret = nf_ct_netns_get(ctx->net, ctx->family);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static void nft_ct_expect_obj_destroy(const struct nft_ctx *ctx,
+				       struct nft_object *obj)
+{
+	nf_ct_netns_put(ctx->net, ctx->family);
+}
+
+static int nft_ct_expect_obj_dump(struct sk_buff *skb,
+				   struct nft_object *obj, bool reset)
+{
+	const struct nft_ct_expect_obj *priv = nft_obj_data(obj);
+
+	if (nla_put_be16(skb, NFTA_CT_EXPECT_L3PROTO, htons(priv->l3num)) ||
+	    nla_put_u8(skb, NFTA_CT_EXPECT_L4PROTO, priv->l4proto) ||
+	    nla_put_be16(skb, NFTA_CT_EXPECT_DPORT, priv->dport) ||
+	    nla_put_u32(skb, NFTA_CT_EXPECT_TIMEOUT, priv->timeout))
+	return -1
+
+	return 0;
+}
+
+static void nft_ct_expect_obj_eval(struct nft_object *obj,
+				    struct nft_regs *regs,
+				    const struct nft_pktinfo *pkt)
+{
+	const struct nft_ct_expect_obj *priv = nft_obj_data(obj);
+	enum ip_conntrack_info ctinfo;
+	struct nf_conn *ct = nf_ct_get(pkt->skb, ctinfo);
+	int dir = CTINFO2DIR(ctinfo);
+	struct nf_conntrack_expect *exp;
+
+	exp = nf_ct_expect_alloc(ct);
+	if (exp == NULL) {
+		nf_ct_helper_log(skb, ct, "cannot allocate expectation");
+		regs->verdict.code = NF_DROP;
+		return;
+	}
+
+	nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT, priv->l3num,
+		&ct->tuplehash[!dir].tuple.src.u3, &ct->tuplehash[!dir].tuple.dst.u3,
+		priv->l4proto, NULL, &priv->dport);
+	if (priv->timeout)
+		exp->timeout.expires = jiffies + priv->timeout * HZ;
+
+	if (nf_ct_expect_related(exp) != 0) {
+		nf_ct_helper_log(skb, ct, "cannot add expectation");
+		regs->verdict.code = NF_DROP;
+	}
+}
+
+static const struct nla_policy nft_ct_expect_policy[NFTA_CT_EXPECT_MAX + 1] = {
+	[NFTA_CT_EXPECT_L3PROTO] = {.type = NLA_U16 },
+	[NFTA_CT_EXPECT_L4PROTO] = {.type = NLA_U8 },
+	[NFTA_CT_EXPECT_DPORT] = {.type = NLA_U16 },
+	[NFTA_CT_EXPECT_TIMEOUT] = {.type = NLA_U32 },
+};
+
+static struct nft_object_type nft_ct_expect_obj_type;
+
+static const struct nft_object_ops nft_ct_expect_obj_ops = {
+	.type		= &nft_ct_expect_obj_type,
+	.size		= sizeof(struct nft_ct_expect_obj),
+	.eval		= nft_ct_expect_obj_eval,
+	.init		= nft_ct_expect_obj_init,
+	.destroy	= nft_ct_expect_obj_destroy,
+	.dump		= nft_ct_expect_obj_dump,
+};
+
+static struct nft_object_type nft_ct_expect_obj_type __read_mostly = {
+	.type		= NFT_OBJECT_CT_EXPECT,
+	.ops		= &nft_ct_expect_obj_ops,
+	.maxattr	= NFTA_CT_EXPECT_MAX,
+	.policy		= nft_ct_expect_policy,
+	.owner		= THIS_MODULE,
+};
+
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
 static int
 nft_ct_timeout_parse_policy(void *timeouts,
@@ -1173,17 +1285,23 @@ static int __init nft_ct_module_init(void)
 	err = nft_register_obj(&nft_ct_helper_obj_type);
 	if (err < 0)
 		goto err2;
+
+	err = nft_register_obj(&nft_ct_expect_obj_type);
+	if (err < 0)
+		goto err3;
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
 	err = nft_register_obj(&nft_ct_timeout_obj_type);
 	if (err < 0)
-		goto err3;
+		goto err4;
 #endif
 	return 0;
 
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
+err4:
+	nft_unregister_obj(&nft_ct_expect_obj_type);
+#endif
 err3:
 	nft_unregister_obj(&nft_ct_helper_obj_type);
-#endif
 err2:
 	nft_unregister_expr(&nft_notrack_type);
 err1:
@@ -1196,6 +1314,7 @@ static void __exit nft_ct_module_exit(void)
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
 	nft_unregister_obj(&nft_ct_timeout_obj_type);
 #endif
+	nft_unregister_obj(&nft_ct_expect_obj_type);
 	nft_unregister_obj(&nft_ct_helper_obj_type);
 	nft_unregister_expr(&nft_notrack_type);
 	nft_unregister_expr(&nft_ct_type);
@@ -1210,3 +1329,4 @@ MODULE_ALIAS_NFT_EXPR("ct");
 MODULE_ALIAS_NFT_EXPR("notrack");
 MODULE_ALIAS_NFT_OBJ(NFT_OBJECT_CT_HELPER);
 MODULE_ALIAS_NFT_OBJ(NFT_OBJECT_CT_TIMEOUT);
+MODULE_ALIAS_NFT_OBJ(NFT_OBJECT_CT_EXPECT);
-- 
2.21.0

Re: [PATCH] netfilter: nft_ct: add ct expectations support

[Florian Westphal]

Stéphane Veyret <sveyret@gmail.com> wrote:
> +static void nft_ct_expect_obj_eval(struct nft_object *obj,
> +				    struct nft_regs *regs,
> +				    const struct nft_pktinfo *pkt)
> +{
> +	const struct nft_ct_expect_obj *priv = nft_obj_data(obj);
> +	enum ip_conntrack_info ctinfo;
> +	struct nf_conn *ct = nf_ct_get(pkt->skb, ctinfo);
> +	int dir = CTINFO2DIR(ctinfo);
> +	struct nf_conntrack_expect *exp;
> +
> +	exp = nf_ct_expect_alloc(ct);
> +	if (exp == NULL) {
> +		nf_ct_helper_log(skb, ct, "cannot allocate expectation");
> +		regs->verdict.code = NF_DROP;
> +		return;
> +	}
> +
> +	nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT, priv->l3num,
> +		&ct->tuplehash[!dir].tuple.src.u3, &ct->tuplehash[!dir].tuple.dst.u3,
> +		priv->l4proto, NULL, &priv->dport);
> +	if (priv->timeout)
> +		exp->timeout.expires = jiffies + priv->timeout * HZ;
> +
> +	if (nf_ct_expect_related(exp) != 0) {
> +		nf_ct_helper_log(skb, ct, "cannot add expectation");

Please remove the nf_ct_helper_log() calls, it will crash in case there is no helper
attached to the conntrack.

Other than this I don't see any issues here, thanks for working on this!

Re: [PATCH] netfilter: nft_ct: add ct expectations support

[Pablo Neira Ayuso]

On Sun, May 05, 2019 at 05:40:16PM +0200, Stéphane Veyret wrote:
> This patch allows to add, list and delete expectations via nft objref
> infrastructure and assigning these expectations via nft rule.

Please, add to your patch title your patch version, ie.

[PATCH nf-next,v2] nft_ct: add ct expectations support

Could you describe the usecase example for this infrastructure, please?

More comments below.

> Signed-off-by: Stéphane Veyret <sveyret@gmail.com>
> ---
>  include/uapi/linux/netfilter/nf_tables.h |  15 ++-
>  net/netfilter/nft_ct.c                   | 124 ++++++++++++++++++++++-
>  2 files changed, 136 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index f0cf7b0f4f35..0a3452ca684c 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -968,6 +968,7 @@ enum nft_socket_keys {
>   * @NFT_CT_DST_IP6: conntrack layer 3 protocol destination (IPv6 address)
>   * @NFT_CT_TIMEOUT: connection tracking timeout policy assigned to conntrack
>   * @NFT_CT_ID: conntrack id
> + * @NFT_CT_EXPECT: connection tracking expectation
>   */
>  enum nft_ct_keys {
>  	NFT_CT_STATE,
> @@ -995,6 +996,7 @@ enum nft_ct_keys {
>  	NFT_CT_DST_IP6,
>  	NFT_CT_TIMEOUT,
>  	NFT_CT_ID,
> +	NFT_CT_EXPECT,

You don't this definition, or I don't find where this is used.

>  	__NFT_CT_MAX
>  };
>  #define NFT_CT_MAX		(__NFT_CT_MAX - 1)
> @@ -1447,6 +1449,16 @@ enum nft_ct_timeout_timeout_attributes {
>  };
>  #define NFTA_CT_TIMEOUT_MAX	(__NFTA_CT_TIMEOUT_MAX - 1)
>  
> +enum nft_ct_expectation_attributes {
> +	NFTA_CT_EXPECT_UNSPEC,
> +	NFTA_CT_EXPECT_L3PROTO,
> +	NFTA_CT_EXPECT_L4PROTO,
> +	NFTA_CT_EXPECT_DPORT,
> +	NFTA_CT_EXPECT_TIMEOUT,
> +	__NFTA_CT_EXPECT_MAX,
> +};
> +#define NFTA_CT_EXPECT_MAX	(__NFTA_CT_EXPECT_MAX - 1)
> +
>  #define NFT_OBJECT_UNSPEC	0
>  #define NFT_OBJECT_COUNTER	1
>  #define NFT_OBJECT_QUOTA	2
> @@ -1456,7 +1468,8 @@ enum nft_ct_timeout_timeout_attributes {
>  #define NFT_OBJECT_TUNNEL	6
>  #define NFT_OBJECT_CT_TIMEOUT	7
>  #define NFT_OBJECT_SECMARK	8
> -#define __NFT_OBJECT_MAX	9
> +#define NFT_OBJECT_CT_EXPECT	9
> +#define __NFT_OBJECT_MAX	10
>  #define NFT_OBJECT_MAX		(__NFT_OBJECT_MAX - 1)
>  
>  /**
> diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
> index f043936763f3..06c13b2dfb78 100644
> --- a/net/netfilter/nft_ct.c
> +++ b/net/netfilter/nft_ct.c
> @@ -24,6 +24,7 @@
>  #include <net/netfilter/nf_conntrack_labels.h>
>  #include <net/netfilter/nf_conntrack_timeout.h>
>  #include <net/netfilter/nf_conntrack_l4proto.h>
> +#include <net/netfilter/nf_conntrack_expect.h>
>  
>  struct nft_ct {
>  	enum nft_ct_keys	key:8;
> @@ -790,6 +791,117 @@ static struct nft_expr_type nft_notrack_type __read_mostly = {
>  	.owner		= THIS_MODULE,
>  };
>  
> +struct nft_ct_expect_obj {
> +	int			l3num;
> +	u8			l4proto;
> +	__be16		dport;
> +	u32			timeout;
> +};
> +
> +static int nft_ct_expect_obj_init(const struct nft_ctx *ctx,
> +				   const struct nlattr * const tb[],
> +				   struct nft_object *obj)
> +{
> +	struct nft_ct_expect_obj *priv = nft_obj_data(obj);
> +	int ret;
> +
> +	if (!tb[NFTA_CT_EXPECT_L4PROTO] ||
> +	    !tb[NFTA_CT_EXPECT_DPORT])
> +		return -EINVAL;
> +
> +	priv->l3num = ctx->family;
> +	if (tb[NFTA_CT_EXPECT_L3PROTO])
> +		priv->l3num = ntohs(nla_get_be16(tb[NFTA_CT_EXPECT_L3PROTO]));
> +	priv->l4proto = nla_get_u8(tb[NFTA_CT_EXPECT_L4PROTO]);
> +
> +	priv->dportmin = nla_get_be16(tb[NFTA_CT_EXPECT_DPORT_MIN]);

Where is NFTA_CT_EXPECT_DPORT_MIN defined? You don't check for this
attribute, it may not be present. Looks like a leftover?

> +
> +	priv->timeout = 0;
> +	if (tb[NFTA_CT_EXPECT_TIMEOUT])
> +		priv->timeout = nla_get_u32(tb[NFTA_CT_EXPECT_TIMEOUT]);
> +
> +	ret = nf_ct_netns_get(ctx->net, ctx->family);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static void nft_ct_expect_obj_destroy(const struct nft_ctx *ctx,
> +				       struct nft_object *obj)
> +{
> +	nf_ct_netns_put(ctx->net, ctx->family);
> +}
> +
> +static int nft_ct_expect_obj_dump(struct sk_buff *skb,
> +				   struct nft_object *obj, bool reset)
> +{
> +	const struct nft_ct_expect_obj *priv = nft_obj_data(obj);
> +
> +	if (nla_put_be16(skb, NFTA_CT_EXPECT_L3PROTO, htons(priv->l3num)) ||
> +	    nla_put_u8(skb, NFTA_CT_EXPECT_L4PROTO, priv->l4proto) ||
> +	    nla_put_be16(skb, NFTA_CT_EXPECT_DPORT, priv->dport) ||
> +	    nla_put_u32(skb, NFTA_CT_EXPECT_TIMEOUT, priv->timeout))
> +	return -1

This won't compile, missing ';'. Also indentation.

> +
> +	return 0;
> +}
> +
> +static void nft_ct_expect_obj_eval(struct nft_object *obj,
> +				    struct nft_regs *regs,
> +				    const struct nft_pktinfo *pkt)
> +{
> +	const struct nft_ct_expect_obj *priv = nft_obj_data(obj);
> +	enum ip_conntrack_info ctinfo;
> +	struct nf_conn *ct = nf_ct_get(pkt->skb, ctinfo);
> +	int dir = CTINFO2DIR(ctinfo);
> +	struct nf_conntrack_expect *exp;
> +
> +	exp = nf_ct_expect_alloc(ct);
> +	if (exp == NULL) {
> +		nf_ct_helper_log(skb, ct, "cannot allocate expectation");
> +		regs->verdict.code = NF_DROP;
> +		return;
> +	}
> +
> +	nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT, priv->l3num,
> +		&ct->tuplehash[!dir].tuple.src.u3, &ct->tuplehash[!dir].tuple.dst.u3,
> +		priv->l4proto, NULL, &priv->dport);

Coding style:

	nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT, priv->l3num,
                          &ct->tuplehash[!dir].tuple.src.u3,
                          &ct->tuplehash[!dir].tuple.dst.u3,
                          priv->l4proto, NULL, &priv->dport);

> +	if (priv->timeout)
> +		exp->timeout.expires = jiffies + priv->timeout * HZ;

timeout should be made mandatory? why check if it's set?

> +	if (nf_ct_expect_related(exp) != 0) {
> +		nf_ct_helper_log(skb, ct, "cannot add expectation");
> +		regs->verdict.code = NF_DROP;
> +	}
> +}
> +
> +static const struct nla_policy nft_ct_expect_policy[NFTA_CT_EXPECT_MAX + 1] = {
> +	[NFTA_CT_EXPECT_L3PROTO] = {.type = NLA_U16 },
> +	[NFTA_CT_EXPECT_L4PROTO] = {.type = NLA_U8 },
> +	[NFTA_CT_EXPECT_DPORT] = {.type = NLA_U16 },
> +	[NFTA_CT_EXPECT_TIMEOUT] = {.type = NLA_U32 },

I'd prefer:

	[NFTA_CT_EXPECT_L3PROTO]        = { .type = NLA_U16 },
	[NFTA_CT_EXPECT_L4PROTO]        = { .type = NLA_U8 },
	[NFTA_CT_EXPECT_DPORT]          = { .type = NLA_U16 },
	[NFTA_CT_EXPECT_TIMEOUT]        = { .type = NLA_U32 },

Thanks!

Re: [PATCH] netfilter: nft_ct: add ct expectations support

[Stéphane Veyret]

Hi Pablo,

Le lun. 6 mai 2019 à 00:51, Pablo Neira Ayuso <pablo@netfilter.org> a écrit :
> >       NFT_CT_TIMEOUT,
> >       NFT_CT_ID,
> > +     NFT_CT_EXPECT,
>
> You don't this definition, or I don't find where this is used.

As I told previously, I just copied the way timeout is built, and
therefore, it seems that NFT_CT_TIMEOUT is not used too.
But I actually saw today that these values are used in the « nftables
» project. There is a copy of nf_tables.h there. Not sure it is a good
idea to keep the variables in « nftables » and not in kernel.

Re: [PATCH] netfilter: nft_ct: add ct expectations support

[Pablo Neira Ayuso]

On Sat, May 11, 2019 at 08:54:57PM +0200, Stéphane Veyret wrote:
> Hi Pablo,
> 
> Le lun. 6 mai 2019 à 00:51, Pablo Neira Ayuso <pablo@netfilter.org> a écrit :
> > >       NFT_CT_TIMEOUT,
> > >       NFT_CT_ID,
> > > +     NFT_CT_EXPECT,
> >
> > You don't this definition, or I don't find where this is used.
> 
> As I told previously, I just copied the way timeout is built, and
> therefore, it seems that NFT_CT_TIMEOUT is not used too.
> But I actually saw today that these values are used in the « nftables
> » project. There is a copy of nf_tables.h there. Not sure it is a good
> idea to keep the variables in « nftables » and not in kernel.

I have just updated the cached copy of nf_tables.h in
git.netfilter.org:

http://git.netfilter.org/nftables/commit/?id=d3869cae9d6232b9f3fa720e5516ece95fdbe73e

Thanks.

Re: [PATCH] netfilter: nft_ct: add ct expectations support

[Stéphane Veyret]

Le dim. 12 mai 2019 à 19:56, Pablo Neira Ayuso <pablo@netfilter.org> a écrit :
>
> > But I actually saw today that these values are used in the « nftables
> > » project. There is a copy of nf_tables.h there. Not sure it is a good
> > idea to keep the variables in « nftables » and not in kernel.
>
> I have just updated the cached copy of nf_tables.h in
> git.netfilter.org:

So now nftables does not compile anymore, does it? What do you think
we should do, then? Add a new variable, out of nf_tables.h, in
nftables project?

Re: [PATCH] netfilter: nft_ct: add ct expectations support

[Pablo Neira Ayuso]

On Mon, May 13, 2019 at 08:00:53AM +0200, Stéphane Veyret wrote:
> Le dim. 12 mai 2019 à 19:56, Pablo Neira Ayuso <pablo@netfilter.org> a écrit :
> >
> > > But I actually saw today that these values are used in the « nftables
> > > » project. There is a copy of nf_tables.h there. Not sure it is a good
> > > idea to keep the variables in « nftables » and not in kernel.
> >
> > I have just updated the cached copy of nf_tables.h in
> > git.netfilter.org:
> 
> So now nftables does not compile anymore, does it? What do you think
> we should do, then? Add a new variable, out of nf_tables.h, in
> nftables project?

Please, do not add a new enumeration definition to nf_tables.h for
something we do not need in UAPI.

Thanks.