2018-09-23 09:19:04

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH v2 1/2] netfilter: nf_tables: add SECMARK support

Add the ability to set the security context of packets within the nf_tables framework.
Add a nft_object for holding security contexts in the kernel and manipulating packets on the wire.

Convert the security context strings at rule addition time to security identifiers.
This is the same behavior like in xt_SECMARK and offers better performance than computing it per packet.

Set the maximum security context length to 256.

Signed-off-by: Christian Göttsche <[email protected]>
---

v2: convert security context strings to ids on rule addition time

Based on nf-next
Tested with v4.18.8

include/net/netfilter/nf_tables_core.h | 4 +
include/uapi/linux/netfilter/nf_tables.h | 18 +++-
net/netfilter/nf_tables_core.c | 28 ++++++-
net/netfilter/nft_meta.c | 101 +++++++++++++++++++++++
4 files changed, 146 insertions(+), 5 deletions(-)

diff --git a/include/net/netfilter/nf_tables_core.h b/include/net/netfilter/nf_tables_core.h
index 8da837d2a..2046d104f 100644
--- a/include/net/netfilter/nf_tables_core.h
+++ b/include/net/netfilter/nf_tables_core.h
@@ -16,6 +16,10 @@ extern struct nft_expr_type nft_meta_type;
extern struct nft_expr_type nft_rt_type;
extern struct nft_expr_type nft_exthdr_type;

+#ifdef CONFIG_NETWORK_SECMARK
+extern struct nft_object_type nft_secmark_obj_type;
+#endif
+
int nf_tables_core_module_init(void);
void nf_tables_core_module_exit(void);

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 702e4f0be..5444e7687 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -1176,6 +1176,21 @@ enum nft_quota_attributes {
};
#define NFTA_QUOTA_MAX (__NFTA_QUOTA_MAX - 1)

+/**
+ * enum nft_secmark_attributes - nf_tables secmark object netlink attributes
+ *
+ * @NFTA_SECMARK_CTX: security context (NLA_STRING)
+ */
+enum nft_secmark_attributes {
+ NFTA_SECMARK_UNSPEC,
+ NFTA_SECMARK_CTX,
+ __NFTA_SECMARK_MAX,
+};
+#define NFTA_SECMARK_MAX (__NFTA_SECMARK_MAX - 1)
+
+/* Max security context length */
+#define NFT_SECMARK_CTX_MAXLEN 256
+
/**
* enum nft_reject_types - nf_tables reject expression reject types
*
@@ -1432,7 +1447,8 @@ enum nft_ct_timeout_timeout_attributes {
#define NFT_OBJECT_CONNLIMIT 5
#define NFT_OBJECT_TUNNEL 6
#define NFT_OBJECT_CT_TIMEOUT 7
-#define __NFT_OBJECT_MAX 8
+#define NFT_OBJECT_SECMARK 8
+#define __NFT_OBJECT_MAX 9
#define NFT_OBJECT_MAX (__NFT_OBJECT_MAX - 1)

/**
diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index ffd5c0f94..3fbce3b9c 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -249,12 +249,24 @@ static struct nft_expr_type *nft_basic_types[] = {
&nft_exthdr_type,
};

+static struct nft_object_type *nft_basic_objects[] = {
+#ifdef CONFIG_NETWORK_SECMARK
+ &nft_secmark_obj_type,
+#endif
+};
+
int __init nf_tables_core_module_init(void)
{
- int err, i;
+ int err, i, j = 0;
+
+ for (i = 0; i < ARRAY_SIZE(nft_basic_objects); i++) {
+ err = nft_register_obj(nft_basic_objects[i]);
+ if (err)
+ goto err;
+ }

- for (i = 0; i < ARRAY_SIZE(nft_basic_types); i++) {
- err = nft_register_expr(nft_basic_types[i]);
+ for (j = 0; j < ARRAY_SIZE(nft_basic_types); j++) {
+ err = nft_register_expr(nft_basic_types[j]);
if (err)
goto err;
}
@@ -262,8 +274,12 @@ int __init nf_tables_core_module_init(void)
return 0;

err:
+ while (j-- > 0)
+ nft_unregister_expr(nft_basic_types[j]);
+
while (i-- > 0)
- nft_unregister_expr(nft_basic_types[i]);
+ nft_unregister_obj(nft_basic_objects[i]);
+
return err;
}

@@ -274,4 +290,8 @@ void nf_tables_core_module_exit(void)
i = ARRAY_SIZE(nft_basic_types);
while (i-- > 0)
nft_unregister_expr(nft_basic_types[i]);
+
+ i = ARRAY_SIZE(nft_basic_objects);
+ while (i-- > 0)
+ nft_unregister_obj(nft_basic_objects[i]);
}
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 297fe7d97..ac5df9508 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -543,3 +543,104 @@ struct nft_expr_type nft_meta_type __read_mostly = {
.maxattr = NFTA_META_MAX,
.owner = THIS_MODULE,
};
+
+#ifdef CONFIG_NETWORK_SECMARK
+
+struct nft_secmark {
+ char ctx[NFT_SECMARK_CTX_MAXLEN];
+ int len;
+ u32 secid;
+};
+
+static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
+ [NFTA_SECMARK_CTX] = { .type = NLA_STRING, .len = NFT_SECMARK_CTX_MAXLEN },
+};
+
+static int nft_secmark_secconversion(struct nft_secmark *priv)
+{
+ int err;
+ u32 tmp_secid = 0;
+
+ err = security_secctx_to_secid(priv->ctx, priv->len, &tmp_secid);
+ if (err)
+ return err;
+
+ if (!tmp_secid)
+ return -ENOENT;
+
+ err = security_secmark_relabel_packet(tmp_secid);
+ if (err)
+ return err;
+
+ priv->secid = tmp_secid;
+ return 0;
+}
+
+static void nft_secmark_obj_eval(struct nft_object *obj, struct nft_regs *regs, const struct nft_pktinfo *pkt)
+{
+ const struct nft_secmark *priv = nft_obj_data(obj);
+ struct sk_buff *skb = pkt->skb;
+
+ skb->secmark = priv->secid;
+}
+
+
+static int nft_secmark_obj_init(const struct nft_ctx *ctx, const struct nlattr * const tb[], struct nft_object *obj)
+{
+ int err;
+ struct nft_secmark *priv = nft_obj_data(obj);
+
+ if (tb[NFTA_SECMARK_CTX] == NULL)
+ return -EINVAL;
+
+ nla_strlcpy(priv->ctx, tb[NFTA_SECMARK_CTX], NFT_SECMARK_CTX_MAXLEN);
+ priv->len = strlen(priv->ctx);
+
+ err = nft_secmark_secconversion(priv);
+ if (err)
+ return err;
+
+ security_secmark_refcount_inc();
+
+ return 0;
+}
+
+static int nft_secmark_obj_dump(struct sk_buff *skb, struct nft_object *obj, bool reset)
+{
+ int err;
+ struct nft_secmark *priv = nft_obj_data(obj);
+
+ if (nla_put_string(skb, NFTA_SECMARK_CTX, priv->ctx))
+ return -1;
+
+ if (reset) {
+ err = nft_secmark_secconversion(priv);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static void nft_secmark_obj_destroy(const struct nft_ctx *ctx, struct nft_object *obj)
+{
+ security_secmark_refcount_dec();
+}
+
+static const struct nft_object_ops nft_secmark_obj_ops = {
+ .type = &nft_secmark_obj_type,
+ .size = sizeof(struct nft_secmark),
+ .init = nft_secmark_obj_init,
+ .eval = nft_secmark_obj_eval,
+ .dump = nft_secmark_obj_dump,
+ .destroy = nft_secmark_obj_destroy,
+};
+struct nft_object_type nft_secmark_obj_type __read_mostly = {
+ .type = NFT_OBJECT_SECMARK,
+ .ops = &nft_secmark_obj_ops,
+ .maxattr = NFTA_SECMARK_MAX,
+ .policy = nft_secmark_policy,
+ .owner = THIS_MODULE,
+};
+
+#endif /* CONFIG_NETWORK_SECMARK */
--
2.19.0



2018-09-23 09:17:04

by Christian Göttsche

[permalink] [raw]
Subject: [PATCH 2/2] netfilter: nf_tables: add requirements for connsecmark support

Add ability to set the connection tracking secmark value.

Add ability to set the meta secmark value.

Signed-off-by: Christian Göttsche <[email protected]>
---

Based on nf-next
Tested with v4.18.8

net/netfilter/nft_ct.c | 15 +++++++++++++++
net/netfilter/nft_meta.c | 8 ++++++++
2 files changed, 23 insertions(+)

diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index d74afa707..dcc451c20 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -298,6 +298,14 @@ static void nft_ct_set_eval(const struct nft_expr *expr,
}
break;
#endif
+#ifdef CONFIG_NF_CONNTRACK_SECMARK
+ case NFT_CT_SECMARK:
+ if (ct->secmark != value) {
+ ct->secmark = value;
+ nf_conntrack_event_cache(IPCT_SECMARK, ct);
+ }
+ break;
+#endif
#ifdef CONFIG_NF_CONNTRACK_LABELS
case NFT_CT_LABELS:
nf_connlabels_replace(ct,
@@ -564,6 +572,13 @@ static int nft_ct_set_init(const struct nft_ctx *ctx,
return -EINVAL;
len = sizeof(u32);
break;
+#endif
+#ifdef CONFIG_NF_CONNTRACK_SECMARK
+ case NFT_CT_SECMARK:
+ if (tb[NFTA_CT_DIRECTION])
+ return -EINVAL;
+ len = sizeof(u32);
+ break;
#endif
default:
return -EOPNOTSUPP;
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index ac5df9508..555fcd66b 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -284,6 +284,11 @@ static void nft_meta_set_eval(const struct nft_expr *expr,

skb->nf_trace = !!value8;
break;
+#ifdef CONFIG_NETWORK_SECMARK
+ case NFT_META_SECMARK:
+ skb->secmark = value;
+ break;
+#endif
default:
WARN_ON(1);
}
@@ -436,6 +441,9 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
switch (priv->key) {
case NFT_META_MARK:
case NFT_META_PRIORITY:
+#ifdef CONFIG_NETWORK_SECMARK
+ case NFT_META_SECMARK:
+#endif
len = sizeof(u32);
break;
case NFT_META_NFTRACE:
--
2.19.0


2018-09-23 13:52:25

by Florian Westphal

[permalink] [raw]
Subject: Re: [PATCH 2/2] netfilter: nf_tables: add requirements for connsecmark support

Christian G?ttsche <[email protected]> wrote:
> Add ability to set the connection tracking secmark value.
> Add ability to set the meta secmark value.

Looks good to me.
Acked-by: Florian Westphal <[email protected]>

2018-09-23 13:57:06

by Florian Westphal

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] netfilter: nf_tables: add SECMARK support

Christian G?ttsche <[email protected]> wrote:
> Add the ability to set the security context of packets within the nf_tables framework.
> Add a nft_object for holding security contexts in the kernel and manipulating packets on the wire.
>
> Convert the security context strings at rule addition time to security identifiers.
> This is the same behavior like in xt_SECMARK and offers better performance than computing it per packet.
>
> Set the maximum security context length to 256.

Looks good, one minor suggestion.

> +#ifdef CONFIG_NETWORK_SECMARK
> +
> +struct nft_secmark {
> + char ctx[NFT_SECMARK_CTX_MAXLEN];
> + int len;
> + u32 secid;
> +};

Can you change this to:

struct nft_secmark {
u32 secid;
char *ctx;
};

?
We don't need ctx in the packetpath, so better to keep
the struct size small.

> + nla_strlcpy(priv->ctx, tb[NFTA_SECMARK_CTX], NFT_SECMARK_CTX_MAXLEN);

You can change this to
priv->ctx = nla_strdup(tb[NFTA_SECMARK_CTX], GFP_KERNEL);
if (!priv->ctx)
return -ENOMEM;

> + err = nft_secmark_secconversion(priv);
> + if (err) {
kfree(priv->ctx);

> +static void nft_secmark_obj_destroy(const struct nft_ctx *ctx, struct nft_object *obj)
> +{
kfree(priv->ctx);

But other than this i think this is ready to be applied,
thanks a lot for making this happen.

2018-09-23 15:32:03

by Christian Göttsche

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] netfilter: nf_tables: add SECMARK support

> > +struct nft_secmark {
> > + char ctx[NFT_SECMARK_CTX_MAXLEN];
> > + int len;
> > + u32 secid;
> > +};
>
> Can you change this to:
>
> struct nft_secmark {
> u32 secid;
> char *ctx;
> };

Does the nla_policy struct needs an update too? (regarding then .len member)

+static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
+ [NFTA_SECMARK_CTX] = { .type = NLA_STRING, .len =
NFT_SECMARK_CTX_MAXLEN },
+}

NFT_SECMARK_CTX_MAXLEN might be dropped then..

2018-09-23 15:42:02

by Florian Westphal

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] netfilter: nf_tables: add SECMARK support

Christian G?ttsche <[email protected]> wrote:
> > Can you change this to:
> >
> > struct nft_secmark {
> > u32 secid;
> > char *ctx;
> > };
>
> Does the nla_policy struct needs an update too? (regarding then .len member)
>
> +static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
> + [NFTA_SECMARK_CTX] = { .type = NLA_STRING, .len =
> NFT_SECMARK_CTX_MAXLEN },
> +}
>
> NFT_SECMARK_CTX_MAXLEN might be dropped then..

Better keep it, we can always increase this later it if needed.
Given the length matches what xtables uses it should be fine.

2018-09-23 17:16:01

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] netfilter: nf_tables: add requirements for connsecmark support

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on nf-next/master]
[also build test ERROR on v4.19-rc4 next-20180921]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Christian-G-ttsche/netfilter-nf_tables-add-SECMARK-support/20180923-213820
base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: x86_64-randconfig-s2-09240020 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

All errors (new ones prefixed by >>):

net/netfilter/nft_ct.c: In function 'nft_ct_set_eval':
>> net/netfilter/nft_ct.c:303:22: error: 'value' undeclared (first use in this function)
if (ct->secmark != value) {
^~~~~
net/netfilter/nft_ct.c:303:22: note: each undeclared identifier is reported only once for each function it appears in

vim +/value +303 net/netfilter/nft_ct.c

275
276 static void nft_ct_set_eval(const struct nft_expr *expr,
277 struct nft_regs *regs,
278 const struct nft_pktinfo *pkt)
279 {
280 const struct nft_ct *priv = nft_expr_priv(expr);
281 struct sk_buff *skb = pkt->skb;
282 #ifdef CONFIG_NF_CONNTRACK_MARK
283 u32 value = regs->data[priv->sreg];
284 #endif
285 enum ip_conntrack_info ctinfo;
286 struct nf_conn *ct;
287
288 ct = nf_ct_get(skb, &ctinfo);
289 if (ct == NULL || nf_ct_is_template(ct))
290 return;
291
292 switch (priv->key) {
293 #ifdef CONFIG_NF_CONNTRACK_MARK
294 case NFT_CT_MARK:
295 if (ct->mark != value) {
296 ct->mark = value;
297 nf_conntrack_event_cache(IPCT_MARK, ct);
298 }
299 break;
300 #endif
301 #ifdef CONFIG_NF_CONNTRACK_SECMARK
302 case NFT_CT_SECMARK:
> 303 if (ct->secmark != value) {
304 ct->secmark = value;
305 nf_conntrack_event_cache(IPCT_SECMARK, ct);
306 }
307 break;
308 #endif
309 #ifdef CONFIG_NF_CONNTRACK_LABELS
310 case NFT_CT_LABELS:
311 nf_connlabels_replace(ct,
312 &regs->data[priv->sreg],
313 &regs->data[priv->sreg],
314 NF_CT_LABELS_MAX_SIZE / sizeof(u32));
315 break;
316 #endif
317 #ifdef CONFIG_NF_CONNTRACK_EVENTS
318 case NFT_CT_EVENTMASK: {
319 struct nf_conntrack_ecache *e = nf_ct_ecache_find(ct);
320 u32 ctmask = regs->data[priv->sreg];
321
322 if (e) {
323 if (e->ctmask != ctmask)
324 e->ctmask = ctmask;
325 break;
326 }
327
328 if (ctmask && !nf_ct_is_confirmed(ct))
329 nf_ct_ecache_ext_add(ct, ctmask, 0, GFP_ATOMIC);
330 break;
331 }
332 #endif
333 default:
334 break;
335 }
336 }
337

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.08 kB)
.config.gz (32.45 kB)
Download all attachments

2018-09-24 03:06:36

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] netfilter: nf_tables: add requirements for connsecmark support

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on nf-next/master]
[also build test ERROR on v4.19-rc5 next-20180921]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Christian-G-ttsche/netfilter-nf_tables-add-SECMARK-support/20180923-213820
base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: x86_64-randconfig-s3-09241007 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

All errors (new ones prefixed by >>):

net//netfilter/nft_ct.c: In function 'nft_ct_set_eval':
>> net//netfilter/nft_ct.c:303:22: error: 'value' undeclared (first use in this function); did you mean 'false'?
if (ct->secmark != value) {
^~~~~
false
net//netfilter/nft_ct.c:303:22: note: each undeclared identifier is reported only once for each function it appears in

vim +303 net//netfilter/nft_ct.c

275
276 static void nft_ct_set_eval(const struct nft_expr *expr,
277 struct nft_regs *regs,
278 const struct nft_pktinfo *pkt)
279 {
280 const struct nft_ct *priv = nft_expr_priv(expr);
281 struct sk_buff *skb = pkt->skb;
282 #ifdef CONFIG_NF_CONNTRACK_MARK
283 u32 value = regs->data[priv->sreg];
284 #endif
285 enum ip_conntrack_info ctinfo;
286 struct nf_conn *ct;
287
288 ct = nf_ct_get(skb, &ctinfo);
289 if (ct == NULL || nf_ct_is_template(ct))
290 return;
291
292 switch (priv->key) {
293 #ifdef CONFIG_NF_CONNTRACK_MARK
294 case NFT_CT_MARK:
295 if (ct->mark != value) {
296 ct->mark = value;
297 nf_conntrack_event_cache(IPCT_MARK, ct);
298 }
299 break;
300 #endif
301 #ifdef CONFIG_NF_CONNTRACK_SECMARK
302 case NFT_CT_SECMARK:
> 303 if (ct->secmark != value) {
304 ct->secmark = value;
305 nf_conntrack_event_cache(IPCT_SECMARK, ct);
306 }
307 break;
308 #endif
309 #ifdef CONFIG_NF_CONNTRACK_LABELS
310 case NFT_CT_LABELS:
311 nf_connlabels_replace(ct,
312 &regs->data[priv->sreg],
313 &regs->data[priv->sreg],
314 NF_CT_LABELS_MAX_SIZE / sizeof(u32));
315 break;
316 #endif
317 #ifdef CONFIG_NF_CONNTRACK_EVENTS
318 case NFT_CT_EVENTMASK: {
319 struct nf_conntrack_ecache *e = nf_ct_ecache_find(ct);
320 u32 ctmask = regs->data[priv->sreg];
321
322 if (e) {
323 if (e->ctmask != ctmask)
324 e->ctmask = ctmask;
325 break;
326 }
327
328 if (ctmask && !nf_ct_is_confirmed(ct))
329 nf_ct_ecache_ext_add(ct, ctmask, 0, GFP_ATOMIC);
330 break;
331 }
332 #endif
333 default:
334 break;
335 }
336 }
337

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.12 kB)
.config.gz (30.87 kB)
Download all attachments