selinux: avoid nf hooks overhead when not needed

This patch leverage the netlbl_changed() hook to perform on demand registration and deregistration of the netfilter hooks and the socket_sock_rcv_skb hook.

With default policy and empty netfilter/netlabel configuration, the above hooks are not registered and this allows avoiding nf_hook_slow in the xmit path and socket_sock_rcv_skb() in the rx path.

This gives measurable network performance improvement in both directions.

Signed-off-by Paolo Abeni <[email protected]>

---
 security/selinux/hooks.c            | 72 +++++++++++++++++++++++++++++++------
 security/selinux/include/security.h |  1 +
 security/selinux/ss/services.c      |  1 +
 security/selinux/xfrm.c             |  4 +++
 4 files changed, 68 insertions(+), 10 deletions(-)

security/selinux/hooks.c +62 -10

index 912deee..a3baa69 100644
@@ -4745,11 +4745,13 @@ static int selinux_secmark_relabel_packet(u32 sid)
 static void selinux_secmark_refcount_inc(void)
 {
 	atomic_inc(&selinux_secmark_refcount);
+	selinux_net_update();
 }
 
 static void selinux_secmark_refcount_dec(void)
 {
 	atomic_dec(&selinux_secmark_refcount);
+	selinux_net_update();
 }
 
 static void selinux_req_classify_flow(const struct request_sock *req,

@@ -4836,6 +4838,11 @@ static int selinux_tun_dev_open(void *security)
 	return 0;
 }
 
+static void selinux_netlbl_changed(void)
+{
+	selinux_net_update();
+}
+
 static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
 {
 	int err = 0;

@@ -6091,7 +6098,6 @@ static struct security_hook_list selinux_hooks[] = {
 	LSM_HOOK_INIT(socket_getsockopt, selinux_socket_getsockopt),
 	LSM_HOOK_INIT(socket_setsockopt, selinux_socket_setsockopt),
 	LSM_HOOK_INIT(socket_shutdown, selinux_socket_shutdown),
-	LSM_HOOK_INIT(socket_sock_rcv_skb, selinux_socket_sock_rcv_skb),
 	LSM_HOOK_INIT(socket_getpeersec_stream,
 			selinux_socket_getpeersec_stream),
 	LSM_HOOK_INIT(socket_getpeersec_dgram, selinux_socket_getpeersec_dgram),

@@ -6113,6 +6119,7 @@ static struct security_hook_list selinux_hooks[] = {
 	LSM_HOOK_INIT(tun_dev_attach_queue, selinux_tun_dev_attach_queue),
 	LSM_HOOK_INIT(tun_dev_attach, selinux_tun_dev_attach),
 	LSM_HOOK_INIT(tun_dev_open, selinux_tun_dev_open),
+	LSM_HOOK_INIT(netlbl_changed, selinux_netlbl_changed),
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 	LSM_HOOK_INIT(xfrm_policy_alloc_security, selinux_xfrm_policy_alloc),

@@ -6145,6 +6152,11 @@ static struct security_hook_list selinux_hooks[] = {
 #endif
 };
 
+/* dynamically registered/unregisterd */
+static struct security_hook_list selinux_sock_hooks[] = {
+	LSM_HOOK_INIT(socket_sock_rcv_skb, selinux_socket_sock_rcv_skb),
+};
+
 static __init int selinux_init(void)
 {
 	if (!security_module_enable("selinux")) {

@@ -6240,7 +6252,9 @@ static struct nf_hook_ops selinux_nf_ops[] = {
 #endif	/* IPV6 */
 };
 
-static int __init selinux_nf_ip_init(void)
+static bool nf_hooks_registered;
+
+static int selinux_nf_ip_init(void)
 {
 	int err;
 

@@ -6253,25 +6267,21 @@ static int __init selinux_nf_ip_init(void)
 	if (err)
 		panic("SELinux: nf_register_hooks: error %d\n", err);
 
+	nf_hooks_registered = true;
 	return 0;
 }
 
-__initcall(selinux_nf_ip_init);
-
-#ifdef CONFIG_SECURITY_SELINUX_DISABLE
 static void selinux_nf_ip_exit(void)
 {
 	printk(KERN_DEBUG "SELinux:  Unregistering netfilter hooks\n");
 
 	nf_unregister_hooks(selinux_nf_ops, ARRAY_SIZE(selinux_nf_ops));
+	nf_hooks_registered = false;
 }
-#endif
 
 #else /* CONFIG_NETFILTER */
 
-#ifdef CONFIG_SECURITY_SELINUX_DISABLE
 #define selinux_nf_ip_exit()
-#endif
 
 #endif /* CONFIG_NETFILTER */
 

@@ -6300,8 +6310,8 @@ int selinux_disable(void)
 	/* Try to destroy the avc node cache */
 	avc_disable();
 
-	/* Unregister netfilter hooks. */
-	selinux_nf_ip_exit();
+	/* Unregister net hooks. */
+	selinux_net_update();
 
 	/* Unregister selinuxfs. */
 	exit_sel_fs();

@@ -6309,3 +6319,45 @@ int selinux_disable(void)
 	return 0;
 }
 #endif
+
+DEFINE_MUTEX(selinux_net_mutex);
+
+static bool nf_hooks_required(void)
+{
+	return (selinux_secmark_enabled() || selinux_peerlbl_enabled() ||
+		!selinux_policycap_netpeer) && selinux_enabled;
+}
+
+static bool sock_hooks_required(void)
+{
+	return (!selinux_policycap_netpeer || selinux_secmark_enabled() ||
+		selinux_peerlbl_enabled()) && selinux_enabled;
+}
+
+static bool sock_hooks_registered;
+
+void selinux_net_update(void)
+{
+	if ((nf_hooks_required() == nf_hooks_registered) &&
+	    (sock_hooks_required() == sock_hooks_registered))
+		return;
+
+	mutex_lock(&selinux_net_mutex);
+	if (nf_hooks_required() != nf_hooks_registered) {
+		if (!nf_hooks_registered)
+			selinux_nf_ip_init();
+		else
+			selinux_nf_ip_exit();
+	}
+
+	if (sock_hooks_required() != sock_hooks_registered) {
+		if (!sock_hooks_registered)
+			security_add_hooks(selinux_sock_hooks,
+					   ARRAY_SIZE(selinux_sock_hooks));
+		else
+			security_delete_hooks(selinux_sock_hooks,
+					      ARRAY_SIZE(selinux_sock_hooks));
+		sock_hooks_registered = !sock_hooks_registered;
+	}
+	mutex_unlock(&selinux_net_mutex);
+}

security/selinux/include/security.h +1 -0

index 38feb55..0428ab4 100644
@@ -261,6 +261,7 @@ extern void selinux_status_update_setenforce(int enforcing);
 extern void selinux_status_update_policyload(int seqno);
 extern void selinux_complete_init(void);
 extern int selinux_disable(void);
+extern void selinux_net_update(void);
 extern void exit_sel_fs(void);
 extern struct path selinux_null;
 extern struct vfsmount *selinuxfs_mount;

security/selinux/ss/services.c +1 -0

index ebda973..c509018 100644
@@ -2016,6 +2016,7 @@ static void security_load_policycaps(void)
 						  POLICYDB_CAPABILITY_OPENPERM);
 	selinux_policycap_alwaysnetwork = ebitmap_get_bit(&policydb.policycaps,
 						  POLICYDB_CAPABILITY_ALWAYSNETWORK);
+	selinux_net_update();
 }
 
 static int security_preserve_bools(struct policydb *p);

security/selinux/xfrm.c +4 -0

index 56e354f..cc2b0d4 100644
@@ -112,6 +112,7 @@ static int selinux_xfrm_alloc_user(struct xfrm_sec_ctx **ctxp,
 
 	*ctxp = ctx;
 	atomic_inc(&selinux_xfrm_refcount);
+	selinux_net_update();
 	return 0;
 
 err:

@@ -128,6 +129,7 @@ static void selinux_xfrm_free(struct xfrm_sec_ctx *ctx)
 		return;
 
 	atomic_dec(&selinux_xfrm_refcount);
+	selinux_net_update();
 	kfree(ctx);
 }
 

@@ -303,6 +305,7 @@ int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx,
 	if (!new_ctx)
 		return -ENOMEM;
 	atomic_inc(&selinux_xfrm_refcount);
+	selinux_net_update();
 	*new_ctxp = new_ctx;
 
 	return 0;

@@ -370,6 +373,7 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x,
 
 	x->security = ctx;
 	atomic_inc(&selinux_xfrm_refcount);
+	selinux_net_update();
 out:
 	kfree(ctx_str);
 	return rc;

Currently, selinux always registers iptables POSTROUTING hooks regarless of the running policy needs for any action to be performed by them.

Even the socket_sock_rcv_skb() is always registered, but it can result in a no-op depending on the current policy configuration.

The above invocations in the kernel datapath are cause of measurable overhead in networking performance test.

This patch series adds explicit notification for netlabel status change (other relevant status change, like xfrm and secmark, are already notified to LSM) and use this information in selinux to register the above hooks only when the current status makes them relevant, deregistering them when no-op

Avoiding the LSM hooks overhead, in netperf UDP_STREAM test with small packets, gives about 5% performance improvement on rx and about 8% on tx.

Quoted 9 lines · Paolo Abeni

Currently, selinux always registers iptables POSTROUTING hooks regarless of the running policy needs for any action to be performed by them.

Even the socket_sock_rcv_skb() is always registered, but it can result in a no-op depending on the current policy configuration.

The above invocations in the kernel datapath are cause of measurable overhead in networking performance test.

Quoted 7 lines

This patch series adds explicit notification for netlabel status change (other relevant status change, like xfrm and secmark, are already notified to LSM) and use this information in selinux to register the above hooks only when the current status makes them relevant, deregistering them when no-op

Avoiding the LSM hooks overhead, in netperf UDP_STREAM test with small packets, gives about 5% performance improvement on rx and about 8% on tx.

[NOTE: added the SELinux mailing list to the CC line, please include when submitting SELinux patches]

While I appreciate the patch and the work that went into development and testing, I'm going to reject this patch on the grounds that it conflicts with work we've just started thinking about which should bring some tangible security benefit.

The recent addition of post-init read only memory opens up some interesting possibilities for SELinux and LSMs in general, the thing which we've just started looking at is marking the LSM hook structure read only after init. There are some complicating factors for SELinux, but I'm confident those can be resolved, and from what I can tell marking the hooks read only will have no effect on other LSMs. While marking the LSM hook structure doesn't directly affect the SELinux netfilter hooks, once we remove the ability to deregister the LSM hooks we will have no need to support deregistering netfilter hooks and I expect we will drop that functionality as well to help decrease the risk of tampering.

Quoted 26 lines · Paul Moore

Quoted 9 lines · Paolo Abeni

Currently, selinux always registers iptables POSTROUTING hooks regarless of the running policy needs for any action to be performed by them.

Even the socket_sock_rcv_skb() is always registered, but it can result in a no-op depending on the current policy configuration.

The above invocations in the kernel datapath are cause of measurable overhead in networking performance test.

Quoted 7 lines

This patch series adds explicit notification for netlabel status change (other relevant status change, like xfrm and secmark, are already notified to LSM) and use this information in selinux to register the above hooks only when the current status makes them relevant, deregistering them when no-op

Avoiding the LSM hooks overhead, in netperf UDP_STREAM test with small packets, gives about 5% performance improvement on rx and about 8% on tx.

[NOTE: added the SELinux mailing list to the CC line, please include when submitting SELinux patches]

While I appreciate the patch and the work that went into development and testing, I'm going to reject this patch on the grounds that it conflicts with work we've just started thinking about which should bring some tangible security benefit.

Quoted 11 lines

The recent addition of post-init read only memory opens up some interesting possibilities for SELinux and LSMs in general, the thing which we've just started looking at is marking the LSM hook structure read only after init. There are some complicating factors for SELinux, but I'm confident those can be resolved, and from what I can tell marking the hooks read only will have no effect on other LSMs. While marking the LSM hook structure doesn't directly affect the SELinux netfilter hooks, once we remove the ability to deregister the LSM hooks we will have no need to support deregistering netfilter hooks and I expect we will drop that functionality as well to help decrease the risk of tampering.

What if we drops the selinux hook related changes in the second patch (the on-demand socket_sock_rcv_skb() [de-]registration)?

The patch will not conflict with the LSM hook structure becoming read-only, we still retain the ability of registering/de-registering the netfilter hooks, and that will still affect positively the tx network performance.

"While marking the LSM hook structure doesn't directly affect the SELinux netfilter hooks, once we remove the ability to deregister the LSM hooks we will have no need to support deregistering netfilter hooks and I expect we will drop that functionality as well to help decrease the risk of tampering."

From: Paul Moore <[email protected]>
Date: Wed, 6 Apr 2016 10:07:27 -0400

Quoted 5 lines

"While marking the LSM hook structure doesn't directly affect the SELinux netfilter hooks, once we remove the ability to deregister the LSM hooks we will have no need to support deregistering netfilter hooks and I expect we will drop that functionality as well to help decrease the risk of tampering."

This is not a reasonable postiion.

The performance implications are non-trivial for using netfilter hooks when they aren't actually needed.

Quoted 9 lines

From: Paul Moore <[email protected]>
Date: Wed, 6 Apr 2016 10:07:27 -0400

Quoted 5 lines

"While marking the LSM hook structure doesn't directly affect the SELinux netfilter hooks, once we remove the ability to deregister the LSM hooks we will have no need to support deregistering netfilter hooks and I expect we will drop that functionality as well to help decrease the risk of tampering."

Quoted 4 lines

This is not a reasonable postiion.

The performance implications are non-trivial for using netfilter hooks when they aren't actually needed.

With all due respect, I think you've taken what I consider to be some unreasonable positions when it comes to the network stack and LSMs in the past. We have different perspectives and different priorities as a result, from my perspective the security advantage gained by eliminating the ability to disable SELinux at runtime is more important.

From: Paul Moore <[email protected]>
Date: Wed, 6 Apr 2016 14:36:43 -0400

Quoted 15 lines · David Miller

Quoted 9 lines

From: Paul Moore <[email protected]>
Date: Wed, 6 Apr 2016 10:07:27 -0400

Quoted 5 lines

"While marking the LSM hook structure doesn't directly affect the SELinux netfilter hooks, once we remove the ability to deregister the LSM hooks we will have no need to support deregistering netfilter hooks and I expect we will drop that functionality as well to help decrease the risk of tampering."

Quoted 4 lines

This is not a reasonable postiion.

The performance implications are non-trivial for using netfilter hooks when they aren't actually needed.

Quoted 6 lines

With all due respect, I think you've taken what I consider to be some unreasonable positions when it comes to the network stack and LSMs in the past. We have different perspectives and different priorities as a result, from my perspective the security advantage gained by eliminating the ability to disable SELinux at runtime is more important.

SELinux folks seem to get rather upset to people outright disabling the facility, but many users still do exactly that.

In my opinion, it's uncompromising positions like the one you are having here is part of the reason that issue will continue.

It is not AND, it's an OR, people want choice, and if you don't give it to them they will find a way to achieve what they want with or without your help. And you might not like what they come up with.

If distributions are turning SELinux on by default, then we have to care about whather netfilter performance should suffer for facilities which are unused.

Quoted 9 lines · Paolo Abeni

Currently, selinux always registers iptables POSTROUTING hooks regarless of the running policy needs for any action to be performed by them.

Even the socket_sock_rcv_skb() is always registered, but it can result in a no-op depending on the current policy configuration.

The above invocations in the kernel datapath are cause of measurable overhead in networking performance test.

Quoted 7 lines

This patch series adds explicit notification for netlabel status change (other relevant status change, like xfrm and secmark, are already notified to LSM) and use this information in selinux to register the above hooks only when the current status makes them relevant, deregistering them when no-op

Avoiding the LSM hooks overhead, in netperf UDP_STREAM test with small packets, gives about 5% performance improvement on rx and about 8% on tx.

[NOTE: added the SELinux mailing list to the CC line, please include when submitting SELinux patches]

While I appreciate the patch and the work that went into development and testing, I'm going to reject this patch on the grounds that it conflicts with work we've just started thinking about which should bring some tangible security benefit.

The recent addition of post-init read only memory opens up some interesting possibilities for SELinux and LSMs in general, the thing which we've just started looking at is marking the LSM hook structure read only after init. There are some complicating factors for SELinux, but I'm confident those can be resolved, and from what I can tell marking the hooks read only will have no effect on other LSMs. While marking the LSM hook structure doesn't directly affect the SELinux netfilter hooks, once we remove the ability to deregister the LSM hooks we will have no need to support deregistering netfilter hooks and I expect we will drop that functionality as well to help decrease the risk of tampering.

netfilter hooks are per namespace -- so there is hook unregister when netns is destroyed.

Do you think it makes sense to rework the patch to delay registering of the netfiler hooks until the system is in a state where they're needed, without the 'unregister' aspect?

Ideally this would even be per netns -- in perfect world we would be able to make it so that a new netns are created with an empty hook list.

Quoted 2 lines · Florian Westphal

netfilter hooks are per namespace -- so there is hook unregister when netns is destroyed.

Looking around, I see the global and per-namespace registration functions (nf_register_hook and nf_register_net_hook, respectively), but I'm looking to see if/how newly created namespace inherit netfilter hooks from the init network namespace ... if you can create a network namespace and dodge the SELinux hooks, that isn't a good thing from a SELinux point of view, although it might be a plus depending on where you view Paolo's original patches ;)

Quoted 3 lines

Do you think it makes sense to rework the patch to delay registering of the netfiler hooks until the system is in a state where they're needed, without the 'unregister' aspect?

I would need to see the patch to say for certain, but in principle that seems perfectly reasonable and I think would satisfy both the netdev and SELinux camps - good suggestion. My main goal is to drop the selinux_nf_ip_init() entirely so it can't be used as a ROP gadget.

We might even be able to trim the secmark_active and peerlbl_active checks in the SELinux netfilter hooks (an earlier attempt at optimization; contrary to popular belief, I do care about SELinux performance), although that would mean that enabling the network access controls would be one way ... I guess you can disregard that last bit, I'm thinking aloud again.

Quoted 3 lines

Ideally this would even be per netns -- in perfect world we would be able to make it so that a new netns are created with an empty hook list.

In general SELinux doesn't care about namespaces, for reasons that are sorta beyond the scope of this conversation, so I would like to stick to a all or nothing approach to enabling the SELinux netfilter hooks across namespaces. Perhaps we can revisit this at a later time, but let's keep it simple right now.

Quoted 11 lines · Paul Moore

Quoted 2 lines · Florian Westphal

netfilter hooks are per namespace -- so there is hook unregister when netns is destroyed.

Looking around, I see the global and per-namespace registration functions (nf_register_hook and nf_register_net_hook, respectively), but I'm looking to see if/how newly created namespace inherit netfilter hooks from the init network namespace ... if you can create a network namespace and dodge the SELinux hooks, that isn't a good thing from a SELinux point of view, although it might be a plus depending on where you view Paolo's original patches ;)

Heh :-)

If you use nf_register_net_hook, the hook is only registered in the namespace.

If you use nf_register_hook, the hook is put on a global list and registed in all existing namespaces.

New namespaces will have the hook added as well (see netfilter_net_init -> nf_register_hook_list in netfilter/core.c )

Since nf_register_hook is used it should be impossible to get a netns that doesn't call these hooks.

Quoted 9 lines

Quoted 3 lines

Do you think it makes sense to rework the patch to delay registering of the netfiler hooks until the system is in a state where they're needed, without the 'unregister' aspect?

I would need to see the patch to say for certain, but in principle that seems perfectly reasonable and I think would satisfy both the netdev and SELinux camps - good suggestion. My main goal is to drop the selinux_nf_ip_init() entirely so it can't be used as a ROP gadget.

Quoted 6 lines

We might even be able to trim the secmark_active and peerlbl_active checks in the SELinux netfilter hooks (an earlier attempt at optimization; contrary to popular belief, I do care about SELinux performance), although that would mean that enabling the network access controls would be one way ... I guess you can disregard that last bit, I'm thinking aloud again.

One way is fine I think.

Quoted 9 lines

Quoted 3 lines

Ideally this would even be per netns -- in perfect world we would be able to make it so that a new netns are created with an empty hook list.

In general SELinux doesn't care about namespaces, for reasons that are sorta beyond the scope of this conversation, so I would like to stick to a all or nothing approach to enabling the SELinux netfilter hooks across namespaces. Perhaps we can revisit this at a later time, but let's keep it simple right now.

Okay, I'd prefer to stick to your recommendation anyway wrt. to selinux (Casey, I read your comment regarding smack. Noted, we don't want to break smack either...)

I think that in this case the entire question is:

In your experience, how likely is a config where selinux is enabled BUT the hooks are not needed (i.e., where we hit the

if (!selinux_policycap_netpeer)

    return NF_ACCEPT;

if (!secmark_active && !peerlbl_active)
return NF_ACCEPT;

tests inside the hooks)? If such setups are uncommon we should just drop this idea or at least put it on the back burner until the more expensive netfilter hooks (conntrack, cough) are out of the way.

Quoted 18 lines · Florian Westphal

Quoted 11 lines · Paul Moore

Quoted 2 lines · Florian Westphal

netfilter hooks are per namespace -- so there is hook unregister when netns is destroyed.

Looking around, I see the global and per-namespace registration functions (nf_register_hook and nf_register_net_hook, respectively), but I'm looking to see if/how newly created namespace inherit netfilter hooks from the init network namespace ... if you can create a network namespace and dodge the SELinux hooks, that isn't a good thing from a SELinux point of view, although it might be a plus depending on where you view Paolo's original patches ;)

Heh :-)

If you use nf_register_net_hook, the hook is only registered in the namespace.

Quoted 8 lines

If you use nf_register_hook, the hook is put on a global list and registed in all existing namespaces.

New namespaces will have the hook added as well (see netfilter_net_init -> nf_register_hook_list in netfilter/core.c )

Since nf_register_hook is used it should be impossible to get a netns that doesn't call these hooks.

Great, thanks.

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

Do you think it makes sense to rework the patch to delay registering of the netfiler hooks until the system is in a state where they're needed, without the 'unregister' aspect?

I would need to see the patch to say for certain, but in principle that seems perfectly reasonable and I think would satisfy both the netdev and SELinux camps - good suggestion. My main goal is to drop the selinux_nf_ip_init() entirely so it can't be used as a ROP gadget.

Quoted 8 lines

Quoted 6 lines

We might even be able to trim the secmark_active and peerlbl_active checks in the SELinux netfilter hooks (an earlier attempt at optimization; contrary to popular belief, I do care about SELinux performance), although that would mean that enabling the network access controls would be one way ... I guess you can disregard that last bit, I'm thinking aloud again.

One way is fine I think.

Yes, just disregard my second paragraph above.

Quoted 20 lines

Quoted 9 lines

Quoted 3 lines

Ideally this would even be per netns -- in perfect world we would be able to make it so that a new netns are created with an empty hook list.

In general SELinux doesn't care about namespaces, for reasons that are sorta beyond the scope of this conversation, so I would like to stick to a all or nothing approach to enabling the SELinux netfilter hooks across namespaces. Perhaps we can revisit this at a later time, but let's keep it simple right now.

Okay, I'd prefer to stick to your recommendation anyway wrt. to selinux (Casey, I read your comment regarding smack. Noted, we don't want to break smack either...)

I think that in this case the entire question is:

In your experience, how likely is a config where selinux is enabled BUT the hooks are not needed (i.e., where we hit the

if (!selinux_policycap_netpeer)

Quoted 8 lines

    return NF_ACCEPT;

if (!secmark_active && !peerlbl_active)
return NF_ACCEPT;

tests inside the hooks)? If such setups are uncommon we should just drop this idea or at least put it on the back burner until the more expensive netfilter hooks (conntrack, cough) are out of the way.

A few years ago I would have said that it is relatively uncommon for admins to enable the SELinux network access controls; it was typically just government/intelligence agencies who had very strict access control requirements and represented a small portion of SELinux users. However, over the past few years I've been fielding more and more questions from admins/devs in the virtualization space who are interested in some of these capabilities; it isn't clear to me how many of these people are switching it on, but there is definitely more interest than I have seen in the past and the interested is centered around some rather common use cases.

So, to summarize, I don't know ;)

If you've got bigger sources of overhead, my opinion would be to go tackle those first. Perhaps I can even find the time to work on the SELinux/netfilter stuff while you are off slaying the bigger dragons, no promises at the moment.

Quoted 80 lines · Paul Moore

Quoted 16 lines · Florian Westphal

Quoted 10 lines · Paul Moore

Quoted 2 lines · Florian Westphal

netfilter hooks are per namespace -- so there is hook unregister when netns is destroyed.

Looking around, I see the global and per-namespace registration functions (nf_register_hook and nf_register_net_hook, respectively), but I'm looking to see if/how newly created namespace inherit netfilter hooks from the init network namespace ... if you can create a network namespace and dodge the SELinux hooks, that isn't a good thing from a SELinux point of view, although it might be a plus depending on where you view Paolo's original patches ;)

Heh :-)

If you use nf_register_net_hook, the hook is only registered in the namespace.

Quoted 8 lines

If you use nf_register_hook, the hook is put on a global list and registed in all existing namespaces.

New namespaces will have the hook added as well (see netfilter_net_init -> nf_register_hook_list in netfilter/core.c )

Since nf_register_hook is used it should be impossible to get a netns that doesn't call these hooks.

Great, thanks.

Quoted 8 lines

Quoted 8 lines

Quoted 3 lines

Do you think it makes sense to rework the patch to delay registering of the netfiler hooks until the system is in a state where they're needed, without the 'unregister' aspect?

I would need to see the patch to say for certain, but in principle that seems perfectly reasonable and I think would satisfy both the netdev and SELinux camps - good suggestion. My main goal is to drop the selinux_nf_ip_init() entirely so it can't be used as a ROP gadget.

Quoted 7 lines

Quoted 6 lines

We might even be able to trim the secmark_active and peerlbl_active checks in the SELinux netfilter hooks (an earlier attempt at optimization; contrary to popular belief, I do care about SELinux performance), although that would mean that enabling the network access controls would be one way ... I guess you can disregard that last bit, I'm thinking aloud again.

One way is fine I think.

Yes, just disregard my second paragraph above.

Quoted 18 lines

Quoted 8 lines

Quoted 3 lines

Ideally this would even be per netns -- in perfect world we would be able to make it so that a new netns are created with an empty hook list.

In general SELinux doesn't care about namespaces, for reasons that are sorta beyond the scope of this conversation, so I would like to stick to a all or nothing approach to enabling the SELinux netfilter hooks across namespaces. Perhaps we can revisit this at a later time, but let's keep it simple right now.

Okay, I'd prefer to stick to your recommendation anyway wrt. to selinux (Casey, I read your comment regarding smack. Noted, we don't want to break smack either...)

I think that in this case the entire question is:

In your experience, how likely is a config where selinux is enabled BUT the hooks are not needed (i.e., where we hit the

if (!selinux_policycap_netpeer)

Quoted 8 lines

    return NF_ACCEPT;

if (!secmark_active && !peerlbl_active)
return NF_ACCEPT;

tests inside the hooks)? If such setups are uncommon we should just drop this idea or at least put it on the back burner until the more expensive netfilter hooks (conntrack, cough) are out of the way.

A few years ago I would have said that it is relatively uncommon for admins to enable the SELinux network access controls; it was typically just government/intelligence agencies who had very strict access control requirements and represented a small portion of SELinux users. However, over the past few years I've been fielding more and more questions from admins/devs in the virtualization space who are interested in some of these capabilities; it isn't clear to me how many of these people are switching it on, but there is definitely more interest than I have seen in the past and the interested is centered around some rather common use cases.

Quoted 6 lines

So, to summarize, I don't know ;)

If you've got bigger sources of overhead, my opinion would be to go tackle those first. Perhaps I can even find the time to work on the SELinux/netfilter stuff while you are off slaying the bigger dragons, no promises at the moment.

Double checking if I got the above correctly.

Will be ok if we post a v2 version of this series, removing the hooks de-registration bits, but preserving the selinux nf-hooks and socket_sock_rcv_skb() on-demand/delayed registration ?

Will that fit
with the post-init read only memory usage that you are planning ?

Regards,

Paolo

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 87 lines · Paolo Abeni

Quoted 80 lines · Paul Moore

Quoted 16 lines · Florian Westphal

Quoted 10 lines · Paul Moore

Quoted 2 lines · Florian Westphal

netfilter hooks are per namespace -- so there is hook unregister when netns is destroyed.

Looking around, I see the global and per-namespace registration functions (nf_register_hook and nf_register_net_hook, respectively), but I'm looking to see if/how newly created namespace inherit netfilter hooks from the init network namespace ... if you can create a network namespace and dodge the SELinux hooks, that isn't a good thing from a SELinux point of view, although it might be a plus depending on where you view Paolo's original patches ;)

Heh :-)

If you use nf_register_net_hook, the hook is only registered in the namespace.

Quoted 8 lines

If you use nf_register_hook, the hook is put on a global list and registed in all existing namespaces.

New namespaces will have the hook added as well (see netfilter_net_init -> nf_register_hook_list in netfilter/core.c )

Since nf_register_hook is used it should be impossible to get a netns that doesn't call these hooks.

Great, thanks.

Quoted 8 lines

Quoted 8 lines

Quoted 3 lines

Do you think it makes sense to rework the patch to delay registering of the netfiler hooks until the system is in a state where they're needed, without the 'unregister' aspect?

I would need to see the patch to say for certain, but in principle that seems perfectly reasonable and I think would satisfy both the netdev and SELinux camps - good suggestion. My main goal is to drop the selinux_nf_ip_init() entirely so it can't be used as a ROP gadget.

Quoted 7 lines

Quoted 6 lines

We might even be able to trim the secmark_active and peerlbl_active checks in the SELinux netfilter hooks (an earlier attempt at optimization; contrary to popular belief, I do care about SELinux performance), although that would mean that enabling the network access controls would be one way ... I guess you can disregard that last bit, I'm thinking aloud again.

One way is fine I think.

Yes, just disregard my second paragraph above.

Quoted 18 lines

Quoted 8 lines

Quoted 3 lines

Ideally this would even be per netns -- in perfect world we would be able to make it so that a new netns are created with an empty hook list.

In general SELinux doesn't care about namespaces, for reasons that are sorta beyond the scope of this conversation, so I would like to stick to a all or nothing approach to enabling the SELinux netfilter hooks across namespaces. Perhaps we can revisit this at a later time, but let's keep it simple right now.

Okay, I'd prefer to stick to your recommendation anyway wrt. to selinux (Casey, I read your comment regarding smack. Noted, we don't want to break smack either...)

I think that in this case the entire question is:

In your experience, how likely is a config where selinux is enabled BUT the hooks are not needed (i.e., where we hit the

if (!selinux_policycap_netpeer)

Quoted 8 lines

    return NF_ACCEPT;

if (!secmark_active && !peerlbl_active)
return NF_ACCEPT;

tests inside the hooks)? If such setups are uncommon we should just drop this idea or at least put it on the back burner until the more expensive netfilter hooks (conntrack, cough) are out of the way.

A few years ago I would have said that it is relatively uncommon for admins to enable the SELinux network access controls; it was typically just government/intelligence agencies who had very strict access control requirements and represented a small portion of SELinux users. However, over the past few years I've been fielding more and more questions from admins/devs in the virtualization space who are interested in some of these capabilities; it isn't clear to me how many of these people are switching it on, but there is definitely more interest than I have seen in the past and the interested is centered around some rather common use cases.

Quoted 6 lines

So, to summarize, I don't know ;)

If you've got bigger sources of overhead, my opinion would be to go tackle those first. Perhaps I can even find the time to work on the SELinux/netfilter stuff while you are off slaying the bigger dragons, no promises at the moment.

Quoted 5 lines

Double checking if I got the above correctly.

Will be ok if we post a v2 version of this series, removing the hooks de-registration bits, but preserving the selinux nf-hooks and socket_sock_rcv_skb() on-demand/delayed registration ?

Imagine that I have two security modules that control sockets. The work I'm knee deep in will allow this. If adding hooks after the init phase is allowed you have to face the possibility that blob sizes (in this case sock->sk_security) may change. That requires checking on every hook that uses blobs to determine whether the blob has data for all the modules using it. I know that that is a simple matter of arithmetic, but it's additional overhead on every hook call. It also makes creating kmem caches for security blobs much more difficult. Another performance optimization that becomes unavailable.

Quoted 93 lines · Casey Schaufler

Quoted 87 lines · Paolo Abeni

Quoted 80 lines · Paul Moore

Quoted 16 lines · Florian Westphal

Quoted 10 lines · Paul Moore

Quoted 2 lines · Florian Westphal

netfilter hooks are per namespace -- so there is hook unregister when netns is destroyed.

Looking around, I see the global and per-namespace registration functions (nf_register_hook and nf_register_net_hook, respectively), but I'm looking to see if/how newly created namespace inherit netfilter hooks from the init network namespace ... if you can create a network namespace and dodge the SELinux hooks, that isn't a good thing from a SELinux point of view, although it might be a plus depending on where you view Paolo's original patches ;)

Heh :-)

If you use nf_register_net_hook, the hook is only registered in the namespace.

Quoted 8 lines

If you use nf_register_hook, the hook is put on a global list and registed in all existing namespaces.

New namespaces will have the hook added as well (see netfilter_net_init -> nf_register_hook_list in netfilter/core.c )

Since nf_register_hook is used it should be impossible to get a netns that doesn't call these hooks.

Great, thanks.

Quoted 8 lines

Quoted 8 lines

Quoted 3 lines

Do you think it makes sense to rework the patch to delay registering of the netfiler hooks until the system is in a state where they're needed, without the 'unregister' aspect?

I would need to see the patch to say for certain, but in principle that seems perfectly reasonable and I think would satisfy both the netdev and SELinux camps - good suggestion. My main goal is to drop the selinux_nf_ip_init() entirely so it can't be used as a ROP gadget.

Quoted 7 lines

Quoted 6 lines

We might even be able to trim the secmark_active and peerlbl_active checks in the SELinux netfilter hooks (an earlier attempt at optimization; contrary to popular belief, I do care about SELinux performance), although that would mean that enabling the network access controls would be one way ... I guess you can disregard that last bit, I'm thinking aloud again.

One way is fine I think.

Yes, just disregard my second paragraph above.

Quoted 18 lines

Quoted 8 lines

Quoted 3 lines

Ideally this would even be per netns -- in perfect world we would be able to make it so that a new netns are created with an empty hook list.

In general SELinux doesn't care about namespaces, for reasons that are sorta beyond the scope of this conversation, so I would like to stick to a all or nothing approach to enabling the SELinux netfilter hooks across namespaces. Perhaps we can revisit this at a later time, but let's keep it simple right now.

Okay, I'd prefer to stick to your recommendation anyway wrt. to selinux (Casey, I read your comment regarding smack. Noted, we don't want to break smack either...)

I think that in this case the entire question is:

In your experience, how likely is a config where selinux is enabled BUT the hooks are not needed (i.e., where we hit the

if (!selinux_policycap_netpeer)

Quoted 8 lines

    return NF_ACCEPT;

if (!secmark_active && !peerlbl_active)
return NF_ACCEPT;

tests inside the hooks)? If such setups are uncommon we should just drop this idea or at least put it on the back burner until the more expensive netfilter hooks (conntrack, cough) are out of the way.

A few years ago I would have said that it is relatively uncommon for admins to enable the SELinux network access controls; it was typically just government/intelligence agencies who had very strict access control requirements and represented a small portion of SELinux users. However, over the past few years I've been fielding more and more questions from admins/devs in the virtualization space who are interested in some of these capabilities; it isn't clear to me how many of these people are switching it on, but there is definitely more interest than I have seen in the past and the interested is centered around some rather common use cases.

Quoted 6 lines

So, to summarize, I don't know ;)

If you've got bigger sources of overhead, my opinion would be to go tackle those first. Perhaps I can even find the time to work on the SELinux/netfilter stuff while you are off slaying the bigger dragons, no promises at the moment.

Quoted 5 lines

Double checking if I got the above correctly.

Will be ok if we post a v2 version of this series, removing the hooks de-registration bits, but preserving the selinux nf-hooks and socket_sock_rcv_skb() on-demand/delayed registration ?

Quoted 10 lines

Imagine that I have two security modules that control sockets. The work I'm knee deep in will allow this. If adding hooks after the init phase is allowed you have to face the possibility that blob sizes (in this case sock->sk_security) may change. That requires checking on every hook that uses blobs to determine whether the blob has data for all the modules using it. I know that that is a simple matter of arithmetic, but it's additional overhead on every hook call. It also makes creating kmem caches for security blobs much more difficult. Another performance optimization that becomes unavailable.

I got your point.

Without seeing the code, I wonder if the above scenario could be covered always allocating a blob large enough for all concurrent security modules, i.e. each security module always declares/requests/allocates space into the blobs regardless of it does not have registered (yet) some security hooks, trading memory usage for performance.

Paolo

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Will be ok if we post a v2 version of this series, removing the hooks de-registration bits, but preserving the selinux nf-hooks and socket_sock_rcv_skb() on-demand/delayed registration ? Will that fit with the post-init read only memory usage that you are planning ?

Quoted 4 lines · Paolo Abeni

Will be ok if we post a v2 version of this series, removing the hooks de-registration bits, but preserving the selinux nf-hooks and socket_sock_rcv_skb() on-demand/delayed registration ? Will that fit with the post-init read only memory usage that you are planning ?

The work Florian and and I were talking about would be limited just to the netfilter hooks, the LSM hooks, e.g. socket_sock_rcv_skb() and friends, would remain as they are today. What what we discussing was defaulting to not registering the netfilter hooks until it became necessary due to a labeled networking configuration or the always_check_network policy capability; the registration of the netfilter hooks would be permanent, you could not unregister the hooks at that point, you would need to reboot. Does that make sense?

As far as Casey's concerns, I don't think the work we are talking about for the v2 patchset would have any effect on the socket/sock security blobs as you really can't manage those adequately from the netfilter hooks; you most likely will reference them and perhaps even update the data within, but not allocate or free the blobs. Besides, even in some weird case you were alloc/free'ing security blobs in the netfilter hooks, we can deal with that on a per-LSM basis if/when the full fledged stacking patches are merged; everything we are talking about is a hidden implementation detail so changing it in the future shouldn't be a problem.

Quoted 5 lines · Paul Moore

Quoted 4 lines · Paolo Abeni

Will be ok if we post a v2 version of this series, removing the hooks de-registration bits, but preserving the selinux nf-hooks and socket_sock_rcv_skb() on-demand/delayed registration ? Will that fit with the post-init read only memory usage that you are planning ?

Quoted 8 lines

The work Florian and and I were talking about would be limited just to the netfilter hooks, the LSM hooks, e.g. socket_sock_rcv_skb() and friends, would remain as they are today. What what we discussing was defaulting to not registering the netfilter hooks until it became necessary due to a labeled networking configuration or the always_check_network policy capability; the registration of the netfilter hooks would be permanent, you could not unregister the hooks at that point, you would need to reboot. Does that make sense?

Yes, AFAIC it makes sense. I'll try to follow this route for an eventual v2.

Quoted 10 lines

As far as Casey's concerns, I don't think the work we are talking about for the v2 patchset would have any effect on the socket/sock security blobs as you really can't manage those adequately from the netfilter hooks; you most likely will reference them and perhaps even update the data within, but not allocate or free the blobs. Besides, even in some weird case you were alloc/free'ing security blobs in the netfilter hooks, we can deal with that on a per-LSM basis if/when the full fledged stacking patches are merged; everything we are talking about is a hidden implementation detail so changing it in the future shouldn't be a problem.

Casey, are you ok with the above?

Thank you,

Paolo

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Currently, selinux always registers iptables POSTROUTING hooks regarless of the running policy needs for any action to be performed by them.

Even the socket_sock_rcv_skb() is always registered, but it can result in a no-op depending on the current policy configuration.

The above invocations in the kernel datapath are cause of measurable overhead in networking performance test.

This patch series adds explicit notification for netlabel status change (other relevant status change, like xfrm and secmark, are already notified to LSM) and use this information in selinux to register the above hooks only when the current status makes them relevant, deregistering them when no-op

Avoiding the LSM hooks overhead, in netperf UDP_STREAM test with small packets, gives about 5% performance improvement on rx and about 8% on tx.

Paolo Abeni (2):
security: add hook for netlabel status change notification
selinux: implement support for dynamic net hook [de-]registration

 include/linux/lsm_hooks.h           |  6 ++++
 include/linux/security.h            |  5 +++
 net/netlabel/netlabel_cipso_v4.c    |  8 +++--
 net/netlabel/netlabel_unlabeled.c   |  5 ++-
 security/security.c                 |  7 ++++
 security/selinux/hooks.c            | 72 +++++++++++++++++++++++++++++++------
 security/selinux/include/security.h |  1 +
 security/selinux/ss/services.c      |  1 +
 security/selinux/xfrm.c             |  4 +++
 9 files changed, 96 insertions(+), 13 deletions(-)

Quoted 21 lines · Paolo Abeni

Currently, selinux always registers iptables POSTROUTING hooks regarless of the running policy needs for any action to be performed by them.

Even the socket_sock_rcv_skb() is always registered, but it can result in a no-op depending on the current policy configuration.

The above invocations in the kernel datapath are cause of measurable overhead in networking performance test.

This patch series adds explicit notification for netlabel status change (other relevant status change, like xfrm and secmark, are already notified to LSM) and use this information in selinux to register the above hooks only when the current status makes them relevant, deregistering them when no-op

Avoiding the LSM hooks overhead, in netperf UDP_STREAM test with small packets, gives about 5% performance improvement on rx and about 8% on tx.

Paolo Abeni (2):
security: add hook for netlabel status change notification
selinux: implement support for dynamic net hook [de-]registration

Quoted 11 lines

 include/linux/lsm_hooks.h           |  6 ++++
 include/linux/security.h            |  5 +++
 net/netlabel/netlabel_cipso_v4.c    |  8 +++--
 net/netlabel/netlabel_unlabeled.c   |  5 ++-
 security/security.c                 |  7 ++++
 security/selinux/hooks.c            | 72 +++++++++++++++++++++++++++++++------
 security/selinux/include/security.h |  1 +
 security/selinux/ss/services.c      |  1 +
 security/selinux/xfrm.c             |  4 +++
 9 files changed, 96 insertions(+), 13 deletions(-)

Is there a patch 1/2?

Currently, selinux always registers iptables POSTROUTING hooks regarless of the running policy needs for any action to be performed by them.

Even the socket_sock_rcv_skb() is always registered, but it can result in a no-op depending on the current policy configuration.

The above invocations in the kernel datapath are cause of measurable overhead in networking performance test.

This patch series adds explicit notification for netlabel status change (other relevant status change, like xfrm and secmark, are already notified to LSM) and use this information in selinux to register the above hooks only when the current status makes them relevant, deregistering them when no-op

Avoiding the LSM hooks overhead, in netperf UDP_STREAM test with small packets, gives about 5% performance improvement on rx and about 8% on tx.

Paolo Abeni (2):
security: add hook for netlabel status change notification
selinux: implement support for dynamic net hook [de-]registration

 include/linux/lsm_hooks.h           |  6 ++++
 include/linux/security.h            |  5 +++
 net/netlabel/netlabel_cipso_v4.c    |  8 +++--
 net/netlabel/netlabel_unlabeled.c   |  5 ++-
 security/security.c                 |  7 ++++
 security/selinux/hooks.c            | 72 +++++++++++++++++++++++++++++++------
 security/selinux/include/security.h |  1 +
 security/selinux/ss/services.c      |  1 +
 security/selinux/xfrm.c             |  4 +++
 9 files changed, 96 insertions(+), 13 deletions(-)

Quoted 14 lines · Paolo Abeni

Currently, selinux always registers iptables POSTROUTING hooks regarless of the running policy needs for any action to be performed by them.

Even the socket_sock_rcv_skb() is always registered, but it can result in a no-op depending on the current policy configuration.

The above invocations in the kernel datapath are cause of measurable overhead in networking performance test.

This patch series adds explicit notification for netlabel status change (other relevant status change, like xfrm and secmark, are already notified to LSM) and use this information in selinux to register the above hooks only when the current status makes them relevant, deregistering them when no-op

Quoted 6 lines

Avoiding the LSM hooks overhead, in netperf UDP_STREAM test with small packets, gives about 5% performance improvement on rx and about 8% on tx.

Paolo Abeni (2):
security: add hook for netlabel status change notification
selinux: implement support for dynamic net hook [de-]registration

Did you consider the fact that netlabel and the LSM socket hooks are used by Smack as well as SELinux?

Did you measure the impact that your changes have on Smack?