Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Casey Schaufler]

On 4/15/2016 2:38 AM, Paolo Abeni wrote:
> On Thu, 2016-04-14 at 18:53 -0400, Paul Moore wrote:
>> On Tue, Apr 12, 2016 at 4:52 AM, Paolo Abeni  wrote:
>>> 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?
> Yes, AFAIC it makes sense. I'll try to follow this route for an eventual
> v2.
>
>> 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?

Yes. My concern is with the security module hooks. Altering
the netfilter hooks is a separate issue, and I don't have
trouble with that.

I also would not expect to see an LSM doing blob allocation
during socket delivery, but hey, it *is* networking code,
and stranger things happen all the time.

> Thank you,
>
> Paolo
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-security-module" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paolo Abeni]

On Thu, 2016-04-14 at 18:53 -0400, Paul Moore wrote:
> On Tue, Apr 12, 2016 at 4:52 AM, Paolo Abeni  wrote:
> > 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?

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

> 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

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paul Moore]

On Tue, Apr 12, 2016 at 4:52 AM, Paolo Abeni  wrote:
> 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.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Casey Schaufler]

On 4/13/2016 4:57 AM, Paolo Abeni wrote:
> On Tue, 2016-04-12 at 06:57 -0700, Casey Schaufler wrote:
>> On 4/12/2016 1:52 AM, Paolo Abeni wrote:
>>> On Thu, 2016-04-07 at 14:55 -0400, Paul Moore wrote:
 On Thursday, April 07, 2016 01:45:32 AM Florian Westphal wrote:
> Paul Moore  wrote:
>> On Wed, Apr 6, 2016 at 6:14 PM, Florian Westphal  wrote:
>>> 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.
 Great, thanks.
  
>>> 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.
> One way is fine I think.
 Yes, just disregard my second paragraph above.
  
>>> 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.
 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.
>>> 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 

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paolo Abeni]

On Tue, 2016-04-12 at 06:57 -0700, Casey Schaufler wrote:
> On 4/12/2016 1:52 AM, Paolo Abeni wrote:
> > On Thu, 2016-04-07 at 14:55 -0400, Paul Moore wrote:
> >> On Thursday, April 07, 2016 01:45:32 AM Florian Westphal wrote:
> >>> Paul Moore  wrote:
>  On Wed, Apr 6, 2016 at 6:14 PM, Florian Westphal  wrote:
> > 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.
> >> Great, thanks.
> >>  
> > 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.
> >>> One way is fine I think.
> >> Yes, just disregard my second paragraph above.
> >>  
> > 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.
> >> 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.
> > 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

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Casey Schaufler]

On 4/12/2016 1:52 AM, Paolo Abeni wrote:
> On Thu, 2016-04-07 at 14:55 -0400, Paul Moore wrote:
>> On Thursday, April 07, 2016 01:45:32 AM Florian Westphal wrote:
>>> Paul Moore  wrote:
 On Wed, Apr 6, 2016 at 6:14 PM, Florian Westphal  wrote:
> 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.
>> Great, thanks.
>>  
> 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.
>>> One way is fine I think.
>> Yes, just disregard my second paragraph above.
>>  
> 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.
>> 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.
> 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 

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paolo Abeni]

On Thu, 2016-04-07 at 14:55 -0400, Paul Moore wrote:
> On Thursday, April 07, 2016 01:45:32 AM Florian Westphal wrote:
> > Paul Moore  wrote:
> > > On Wed, Apr 6, 2016 at 6:14 PM, Florian Westphal  wrote:
> > > > 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.
> 
> Great, thanks.
>  
> > > > 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.
> > 
> > One way is fine I think.
> 
> Yes, just disregard my second paragraph above.
>  
> > > > 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.
> 
> 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.

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

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paul Moore]

On Thursday, April 07, 2016 01:45:32 AM Florian Westphal wrote:
> Paul Moore  wrote:
> > On Wed, Apr 6, 2016 at 6:14 PM, Florian Westphal  wrote:
> > > 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.

Great, thanks.
 
> > > 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.
> 
> One way is fine I think.

Yes, just disregard my second paragraph above.
 
> > > 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.

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.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paolo Abeni]

Hi Casey,

On Wed, 2016-04-06 at 14:43 -0700, Casey Schaufler wrote:
> On 4/6/2016 2:51 AM, Paolo Abeni wrote:
> > 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

Thank you for the feedback. The patch series is an RFC, so it's still
rough and not yet well tested in all possible scenarios.

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

Actually yes. The patch series itself is explicitly targeted at reducing
some overhead introduced by selinux in network loads (I'm sorry, now I
see that the last sentence in the cover letter is misleading), and it
tries to achieve that result without affecting others LSM users.

The first patch in the series just introduces an optional LSM hook
(netlbl_changed) that is invoked every time the
'netlabel_mgmt_protocount' values is changed. It do not modify the
behavior nor meaning of any of the existing hooks and/or netlabel APIs.
It's up to the security module to leverage (or not) the new one.

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

Actually I didn't. This is one of the reasons I posted the patch as RFC.
As per design security modules not implementing 'netlbl_changed' should
not be affected. Am I missing something ?

Regards,

Paolo

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Florian Westphal]

Paul Moore  wrote:
> On Wed, Apr 6, 2016 at 6:14 PM, Florian Westphal  wrote:
> > 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.

> > 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.

One way is fine I think.

> > 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.

Thanks,
Florian

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paul Moore]

On Wed, Apr 6, 2016 at 6:14 PM, Florian Westphal  wrote:
> 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 ;)

> 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.

> 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.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Florian Westphal]

Paul Moore  wrote:
> On Wed, Apr 6, 2016 at 5:51 AM, Paolo Abeni  wrote:
> > 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.
> 
> [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.

Thanks.

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Casey Schaufler]

On 4/6/2016 2:51 AM, Paolo Abeni wrote:
> 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(-)
>
Is there a patch 1/2?

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paul Moore]

On Wed, Apr 6, 2016 at 5:37 PM, Casey Schaufler  wrote:
> On 4/6/2016 2:51 AM, Paolo Abeni wrote:
>> 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(-)
>>
>
> Is there a patch 1/2?

Yes, there was (it was the "security: add hook ..." patch), but for
some reason it hasn't hit the archive that I normally use.  Odd.

I'll fwd the patch to you off-list so as not to spam everyone again.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Casey Schaufler]

On 4/6/2016 2:51 AM, Paolo Abeni wrote:
> 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

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? Does Smack even work
with your changes?

>
>  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(-)
>

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paul Moore]

On Wed, Apr 6, 2016 at 3:39 PM, David Miller  wrote:
> From: Paul Moore 
> Date: Wed, 6 Apr 2016 14:36:43 -0400
>
>> On Wed, Apr 6, 2016 at 2:23 PM, David Miller  wrote:
>>> From: Paul Moore 
>>> Date: Wed, 6 Apr 2016 10:07:27 -0400
>>>
 "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.
>>
>> 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.

My opinion is that SELinux isn't for everyone; I think it would be
great if everyone enabled it, but I recognize that it isn't the best
fit for everyone's needs.  If users want to disable it in order to
better meet their needs, who am I to argue?

Or perhaps I should be upset?  I dunno, please tell me how I should
feel.  Like most people, I *love* when I'm told how I should react.

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

Once again, I suspect this all a matter of perspective; from my point
of view the SELinux code has compromised quite a lot, especially in
the case of the networking controls.

> 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.

I think you've made your point known, and I believe I've been clear
about the reasoning behind my decision as well.  I would suggest we
leave it at that until/unless someone has something constructive to
add to the conversation.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[David Miller]

From: Paul Moore 
Date: Wed, 6 Apr 2016 14:36:43 -0400

> On Wed, Apr 6, 2016 at 2:23 PM, David Miller  wrote:
>> From: Paul Moore 
>> Date: Wed, 6 Apr 2016 10:07:27 -0400
>>
>>> "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.
> 
> 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.

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paul Moore]

On Wed, Apr 6, 2016 at 2:23 PM, David Miller  wrote:
> From: Paul Moore 
> Date: Wed, 6 Apr 2016 10:07:27 -0400
>
>> "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.

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.

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[David Miller]

From: Paul Moore 
Date: Wed, 6 Apr 2016 10:07:27 -0400

> "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.

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paul Moore]

On Wed, Apr 6, 2016 at 10:03 AM, Paolo Abeni  wrote:
> On Wed, 2016-04-06 at 08:33 -0400, Paul Moore wrote:
>> On Wed, Apr 6, 2016 at 5:51 AM, Paolo Abeni  wrote:
>> > 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.
>>
>> [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.
>
> 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.

As I already said above:

"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."

-- 
paul moore
www.paul-moore.com

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paolo Abeni]

On Wed, 2016-04-06 at 08:33 -0400, Paul Moore wrote:
> On Wed, Apr 6, 2016 at 5:51 AM, Paolo Abeni  wrote:
> > 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.
> 
> [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.

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.

Regards,

Paolo

Re: [RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[Paul Moore]

On Wed, Apr 6, 2016 at 5:51 AM, Paolo Abeni  wrote:
> 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.

[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.

-- 
paul moore
www.paul-moore.com

[RFC PATCH 0/2] selinux: avoid nf hooks overhead when not needed

[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

 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(-)

-- 
1.8.3.1