The problem with TUN/TAP devices

The problem with TUN/TAP devices

[Paul Moore]

Unfortunately we have a problem with the network access controls and TUN/TAP 
devices.  The basic issue is that packets entering the stack via a TUN device, 
e.g. QEMU/KVM guest instance operating with a bridged network configuration, 
do not have a fully initialized sock associated with them.  I say "fully 
initialized" because the basic initialization has been done (memory allocated, 
initial values set to SECINITSID_UNLABELED, etc.) but the last step where we 
assign the sock a label/SID never happens.  Why?  Because the TUN driver code 
only calls sk_alloc() and nothing else in the TUN code paths finish the 
SELinux sock setup.

Okay, so what?  Well, the problem is that the SELinux IP postrouting code 
treats the packet's sock label (the one that is still set as unlabeled_t in 
the TUN case) as the originating peer label; in short it looks like packets 
sent from your QEMU/KVM instance are unlabeled_t instead of my_guest_t:s3.  
Needless to say this is not ideal.

So how do we fix it?  Well, there are a two options that I can think of right 
now (feel free to add to the list):

1. Set the sock's label/SID in sk_alloc()
2. Introduce a new hook to set the label/SID of a sock and call it from
   tun_set_iff()

The problem with #2 is that it introduces a new (basically TUN specific) hook 
to do something silly.  Important, but still kinda silly.  The problem with #1 
is that we currently set the sock's label/SID in selinux_socket_post_create() 
and match it with the inode's label/SID which has the potential to get ugly (I 
haven't verified all of those cases yet).  However, there may be an 
alternative, call it #1a, where set label the sock in sk_alloc() and then use 
the sock's label to set the inode's label in socket_post_create(); this should 
solve the potential ugliness.

Thoughts?

There is also a somewhat related issue involving persistent TUN/TAP devices 
but I'd like to resolve this before getting deeper into that problem.

-- 
paul moore
linux @ hp


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[James Morris]

On Tue, 30 Jun 2009, Paul Moore wrote:

> So how do we fix it?  Well, there are a two options that I can think of right 
> now (feel free to add to the list):
> 
> 1. Set the sock's label/SID in sk_alloc()
> 2. Introduce a new hook to set the label/SID of a sock and call it from
>    tun_set_iff()
> 
> The problem with #2 is that it introduces a new (basically TUN specific) hook 
> to do something silly.  Important, but still kinda silly.  The problem with #1 
> is that we currently set the sock's label/SID in selinux_socket_post_create() 
> and match it with the inode's label/SID which has the potential to get ugly (I 
> haven't verified all of those cases yet).  However, there may be an 
> alternative, call it #1a, where set label the sock in sk_alloc() and then use 
> the sock's label to set the inode's label in socket_post_create(); this should 
> solve the potential ugliness.
> 
> Thoughts?

I'm not sure, but we probably need to include the netdev list in the 
discussion.


-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Casey Schaufler]

Paul Moore wrote:
> Unfortunately we have a problem with the network access controls and TUN/TAP 
> devices.  The basic issue is that packets entering the stack via a TUN device, 
> e.g. QEMU/KVM guest instance operating with a bridged network configuration, 
> do not have a fully initialized sock associated with them.  I say "fully 
> initialized" because the basic initialization has been done (memory allocated, 
> initial values set to SECINITSID_UNLABELED, etc.) but the last step where we 
> assign the sock a label/SID never happens.  Why?  Because the TUN driver code 
> only calls sk_alloc() and nothing else in the TUN code paths finish the 
> SELinux sock setup.
>   

So what should it be calling and why is the fact that it isn't not a bug
in the TUN driver?

> Okay, so what?  Well, the problem is that the SELinux IP postrouting code 
> treats the packet's sock label (the one that is still set as unlabeled_t in 
> the TUN case) as the originating peer label; in short it looks like packets 
> sent from your QEMU/KVM instance are unlabeled_t instead of my_guest_t:s3.  
> Needless to say this is not ideal.
>
> So how do we fix it?  Well, there are a two options that I can think of right 
> now (feel free to add to the list):
>
> 1. Set the sock's label/SID in sk_alloc()
> 2. Introduce a new hook to set the label/SID of a sock and call it from
>    tun_set_iff()
>
> The problem with #2 is that it introduces a new (basically TUN specific) hook 
> to do something silly.  Important, but still kinda silly.  The problem with #1 
> is that we currently set the sock's label/SID in selinux_socket_post_create() 
> and match it with the inode's label/SID which has the potential to get ugly (I 
> haven't verified all of those cases yet).  However, there may be an 
> alternative, call it #1a, where set label the sock in sk_alloc() and then use 
> the sock's label to set the inode's label in socket_post_create(); this should 
> solve the potential ugliness.
>
> Thoughts?
>
>   

As this would appear to be a flaw in the TUN driver, get the TUN
developers to fix their broken driver. I certainly dislike a special
purpose LSM hook for this.

Do you see this as a problem for all users of labeled networking,
or is it isolated to SELinux?

> There is also a somewhat related issue involving persistent TUN/TAP devices 
> but I'd like to resolve this before getting deeper into that problem.
>
>   

Oh joy.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Stephen Smalley]

On Tue, 2009-06-30 at 17:34 -0400, Paul Moore wrote:
> Unfortunately we have a problem with the network access controls and TUN/TAP 
> devices.  The basic issue is that packets entering the stack via a TUN device, 
> e.g. QEMU/KVM guest instance operating with a bridged network configuration, 
> do not have a fully initialized sock associated with them.  I say "fully 
> initialized" because the basic initialization has been done (memory allocated, 
> initial values set to SECINITSID_UNLABELED, etc.) but the last step where we 
> assign the sock a label/SID never happens.  Why?  Because the TUN driver code 
> only calls sk_alloc() and nothing else in the TUN code paths finish the 
> SELinux sock setup.
> 
> Okay, so what?  Well, the problem is that the SELinux IP postrouting code 
> treats the packet's sock label (the one that is still set as unlabeled_t in 
> the TUN case) as the originating peer label; in short it looks like packets 
> sent from your QEMU/KVM instance are unlabeled_t instead of my_guest_t:s3.  
> Needless to say this is not ideal.
> 
> So how do we fix it?  Well, there are a two options that I can think of right 
> now (feel free to add to the list):
> 
> 1. Set the sock's label/SID in sk_alloc()

This seems to be the right choice; not sure why we didn't do it that way
in the first place (maybe we did in the original LSM patches, but it
fell out when the original networking hooks were rejected).

> 2. Introduce a new hook to set the label/SID of a sock and call it from
>    tun_set_iff()
> 
> The problem with #2 is that it introduces a new (basically TUN specific) hook 
> to do something silly.  Important, but still kinda silly.  The problem with #1 
> is that we currently set the sock's label/SID in selinux_socket_post_create() 
> and match it with the inode's label/SID which has the potential to get ugly (I 
> haven't verified all of those cases yet).  However, there may be an 
> alternative, call it #1a, where set label the sock in sk_alloc() and then use 
> the sock's label to set the inode's label in socket_post_create(); this should 
> solve the potential ugliness.

Wouldn't it be a bug if they didn't match?  So I'd add the sk_alloc()
hook, set the label/SID for the sock there, and remove the setting of
the sock label/SID from post_create.  And then just add a BUG_ON to
post_create to assert that the inode SID should be the same as the sock
SID and if they don't match something has gone wrong.

> 
> Thoughts?
> 
> There is also a somewhat related issue involving persistent TUN/TAP devices 
> but I'd like to resolve this before getting deeper into that problem.
> 
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Eric Paris]

On Wed, Jul 1, 2009 at 9:44 AM, Stephen Smalley<sds@tycho.nsa.gov> wrote:
> On Tue, 2009-06-30 at 17:34 -0400, Paul Moore wrote:

> Wouldn't it be a bug if they didn't match?  So I'd add the sk_alloc()
> hook, set the label/SID for the sock there, and remove the setting of
> the sock label/SID from post_create.  And then just add a BUG_ON to
> post_create to assert that the inode SID should be the same as the sock
> SID and if they don't match something has gone wrong.
>

I've got a system all set up to test anything you want/have/need....
Maybe this afternoon I'll even give this suggestion a try just to see
what happens, the networking hooks are ummmm, special?

-Eric


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Paul Moore]

On Tuesday 30 June 2009 06:19:04 pm James Morris wrote:
> On Tue, 30 Jun 2009, Paul Moore wrote:
> > So how do we fix it?  Well, there are a two options that I can think of
> > right now (feel free to add to the list):
> >
> > 1. Set the sock's label/SID in sk_alloc()
> > 2. Introduce a new hook to set the label/SID of a sock and call it from
> >    tun_set_iff()
> >
> > The problem with #2 is that it introduces a new (basically TUN specific)
> > hook to do something silly.  Important, but still kinda silly.  The
> > problem with #1 is that we currently set the sock's label/SID in
> > selinux_socket_post_create() and match it with the inode's label/SID
> > which has the potential to get ugly (I haven't verified all of those
> > cases yet).  However, there may be an alternative, call it #1a, where set
> > label the sock in sk_alloc() and then use the sock's label to set the
> > inode's label in socket_post_create(); this should solve the potential
> > ugliness.
> >
> > Thoughts?
>
> I'm not sure, but we probably need to include the netdev list in the
> discussion.

Yep, I was just hoping to have a little discussion here first to make sure we 
at least agreed on what should be done from a security point of view ...

-- 
paul moore
linux @ hp


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Stephen Smalley]

On Wed, 2009-07-01 at 11:01 -0400, Eric Paris wrote:
> On Wed, Jul 1, 2009 at 9:44 AM, Stephen Smalley<sds@tycho.nsa.gov> wrote:
> > On Tue, 2009-06-30 at 17:34 -0400, Paul Moore wrote:
> 
> > Wouldn't it be a bug if they didn't match?  So I'd add the sk_alloc()
> > hook, set the label/SID for the sock there, and remove the setting of
> > the sock label/SID from post_create.  And then just add a BUG_ON to
> > post_create to assert that the inode SID should be the same as the sock
> > SID and if they don't match something has gone wrong.
> >
> 
> I've got a system all set up to test anything you want/have/need....
> Maybe this afternoon I'll even give this suggestion a try just to see
> what happens, the networking hooks are ummmm, special?

Wait - we already have a sk_alloc_security hook.  And IIRC, we don't set
the SID/label there because that can happen when a new connection sock
is created, and thus outside of process context.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Paul Moore]

On Tuesday 30 June 2009 11:32:30 pm Casey Schaufler wrote:
> Paul Moore wrote:
> > Unfortunately we have a problem with the network access controls and
> > TUN/TAP devices.  The basic issue is that packets entering the stack via
> > a TUN device, e.g. QEMU/KVM guest instance operating with a bridged
> > network configuration, do not have a fully initialized sock associated
> > with them.  I say "fully initialized" because the basic initialization
> > has been done (memory allocated, initial values set to
> > SECINITSID_UNLABELED, etc.) but the last step where we assign the sock a
> > label/SID never happens.  Why?  Because the TUN driver code only calls
> > sk_alloc() and nothing else in the TUN code paths finish the SELinux sock
> > setup.
>
> So what should it be calling and why is the fact that it isn't not a bug
> in the TUN driver?

...

> As this would appear to be a flaw in the TUN driver, get the TUN
> developers to fix their broken driver. I certainly dislike a special
> purpose LSM hook for this.

I do too.

> Do you see this as a problem for all users of labeled networking,
> or is it isolated to SELinux?

The issue first came up with respect to SELinux so that is where I've been 
looking/thinking.  While there is a potential for Smack to be affected I doubt 
that will be he case since Smack doesn't really care about stuff at the 
postrouting level; although Smack may want to enforce some level of access 
control but I need to go re-remember how Smack does outbound access control.  
TOMOYO should be unaffected since they don't mess with labels.

-- 
paul moore
linux @ hp


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Paul Moore]

On Wednesday 01 July 2009 11:06:51 am Stephen Smalley wrote:
> On Wed, 2009-07-01 at 11:01 -0400, Eric Paris wrote:
> > On Wed, Jul 1, 2009 at 9:44 AM, Stephen Smalley<sds@tycho.nsa.gov> wrote:
> > > On Tue, 2009-06-30 at 17:34 -0400, Paul Moore wrote:
> > >
> > > Wouldn't it be a bug if they didn't match?  So I'd add the sk_alloc()
> > > hook, set the label/SID for the sock there, and remove the setting of
> > > the sock label/SID from post_create.  And then just add a BUG_ON to
> > > post_create to assert that the inode SID should be the same as the sock
> > > SID and if they don't match something has gone wrong.
> >
> > I've got a system all set up to test anything you want/have/need....
> > Maybe this afternoon I'll even give this suggestion a try just to see
> > what happens, the networking hooks are ummmm, special?
>
> Wait - we already have a sk_alloc_security hook.

Yep, this is where we would set the sock's label/SID.

> And IIRC, we don't set the SID/label there because that can happen when a
> new connection sock is created, and thus outside of process context.

This gets back to my original comments about "ugliness".  New incoming 
connection sockets should get labeled correctly before use through the 
inet_conn_request() and inet_csk_clone() hooks.  While admittedly, there is 
the potential for the sock to be labeled incorrectly immediately after 
allocation, but this is not very different from what we have today (sock is 
initially labeled with unlabeled_t).

I'll put something together and post an RFC this afternoon, the patch should 
be relatively small.

-- 
paul moore
linux @ hp


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Stephen Smalley]

On Wed, 2009-07-01 at 11:19 -0400, Paul Moore wrote:
> On Wednesday 01 July 2009 11:06:51 am Stephen Smalley wrote:
> > On Wed, 2009-07-01 at 11:01 -0400, Eric Paris wrote:
> > > On Wed, Jul 1, 2009 at 9:44 AM, Stephen Smalley<sds@tycho.nsa.gov> wrote:
> > > > On Tue, 2009-06-30 at 17:34 -0400, Paul Moore wrote:
> > > >
> > > > Wouldn't it be a bug if they didn't match?  So I'd add the sk_alloc()
> > > > hook, set the label/SID for the sock there, and remove the setting of
> > > > the sock label/SID from post_create.  And then just add a BUG_ON to
> > > > post_create to assert that the inode SID should be the same as the sock
> > > > SID and if they don't match something has gone wrong.
> > >
> > > I've got a system all set up to test anything you want/have/need....
> > > Maybe this afternoon I'll even give this suggestion a try just to see
> > > what happens, the networking hooks are ummmm, special?
> >
> > Wait - we already have a sk_alloc_security hook.
> 
> Yep, this is where we would set the sock's label/SID.
> 
> > And IIRC, we don't set the SID/label there because that can happen when a
> > new connection sock is created, and thus outside of process context.
> 
> This gets back to my original comments about "ugliness".  New incoming 
> connection sockets should get labeled correctly before use through the 
> inet_conn_request() and inet_csk_clone() hooks.  While admittedly, there is 
> the potential for the sock to be labeled incorrectly immediately after 
> allocation, but this is not very different from what we have today (sock is 
> initially labeled with unlabeled_t).

unlabeled_t is easily noticeable in denials (as in this case) and
correctly reflects the state of the socket - it isn't yet labeled.
Setting the sid from the task sid of some random task that happens to be
referenced by current when the new connection is initiated seems a
little more prone to danger if it happens to not get set by the other
hooks later.

> 
> I'll put something together and post an RFC this afternoon, the patch should 
> be relatively small.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Paul Moore]

On Wednesday 01 July 2009 11:42:19 am Stephen Smalley wrote:
> On Wed, 2009-07-01 at 11:19 -0400, Paul Moore wrote:
> > On Wednesday 01 July 2009 11:06:51 am Stephen Smalley wrote:
> > > On Wed, 2009-07-01 at 11:01 -0400, Eric Paris wrote:
> > > > On Wed, Jul 1, 2009 at 9:44 AM, Stephen Smalley<sds@tycho.nsa.gov> 
wrote:
> > > > > On Tue, 2009-06-30 at 17:34 -0400, Paul Moore wrote:
> > > > >
> > > > > Wouldn't it be a bug if they didn't match?  So I'd add the
> > > > > sk_alloc() hook, set the label/SID for the sock there, and remove
> > > > > the setting of the sock label/SID from post_create.  And then just
> > > > > add a BUG_ON to post_create to assert that the inode SID should be
> > > > > the same as the sock SID and if they don't match something has gone
> > > > > wrong.
> > > >
> > > > I've got a system all set up to test anything you want/have/need....
> > > > Maybe this afternoon I'll even give this suggestion a try just to see
> > > > what happens, the networking hooks are ummmm, special?
> > >
> > > Wait - we already have a sk_alloc_security hook.
> >
> > Yep, this is where we would set the sock's label/SID.
> >
> > > And IIRC, we don't set the SID/label there because that can happen when
> > > a new connection sock is created, and thus outside of process context.
> >
> > This gets back to my original comments about "ugliness".  New incoming
> > connection sockets should get labeled correctly before use through the
> > inet_conn_request() and inet_csk_clone() hooks.  While admittedly, there
> > is the potential for the sock to be labeled incorrectly immediately after
> > allocation, but this is not very different from what we have today (sock
> > is initially labeled with unlabeled_t).
>
> unlabeled_t is easily noticeable in denials (as in this case) and
> correctly reflects the state of the socket - it isn't yet labeled.
> Setting the sid from the task sid of some random task that happens to be
> referenced by current when the new connection is initiated seems a
> little more prone to danger if it happens to not get set by the other
> hooks later.

Well, if we can't do it in sk_alloc() then I think we are stuck with a new 
hook; which just seems wrong.

-- 
paul moore
linux @ hp


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[James Morris]

On Wed, 1 Jul 2009, Paul Moore wrote:

> Well, if we can't do it in sk_alloc() then I think we are stuck with a new 
> hook; which just seems wrong.

Why isn't the TUN driver calling the same code as other socket creating 
code?


- James
-- 
James Morris <jmorris@namei.org>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Paul Moore]

On Wednesday 01 July 2009 06:42:36 pm James Morris wrote:
> On Wed, 1 Jul 2009, Paul Moore wrote:
> > Well, if we can't do it in sk_alloc() then I think we are stuck with a
> > new hook; which just seems wrong.
>
> Why isn't the TUN driver calling the same code as other socket creating
> code?

The other socket creating code handles the final setup/initialization in the 
security_socket_post_create() hook which operates on sockets not socks.

-- 
paul moore
linux @ hp


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[James Morris]

On Wed, 1 Jul 2009, Paul Moore wrote:

> On Wednesday 01 July 2009 06:42:36 pm James Morris wrote:
> > On Wed, 1 Jul 2009, Paul Moore wrote:
> > > Well, if we can't do it in sk_alloc() then I think we are stuck with a
> > > new hook; which just seems wrong.
> >
> > Why isn't the TUN driver calling the same code as other socket creating
> > code?
> 
> The other socket creating code handles the final setup/initialization in the 
> security_socket_post_create() hook which operates on sockets not socks.

I wonder if passing a flag might be better than the prot argument, which 
allows the caller to indicate what kind of initialization it's doing, 
rather than what will be seen as another protocol layering violation (i.e. 
the security model poking around to find out what kind of protocol & 
changing its behaviour).


- James
-- 
James Morris
<jmorris@namei.org>

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: The problem with TUN/TAP devices

[Paul Moore]

On Wednesday 01 July 2009 07:53:30 pm James Morris wrote:
> On Wed, 1 Jul 2009, Paul Moore wrote:
> > On Wednesday 01 July 2009 06:42:36 pm James Morris wrote:
> > > On Wed, 1 Jul 2009, Paul Moore wrote:
> > > > Well, if we can't do it in sk_alloc() then I think we are stuck with
> > > > a new hook; which just seems wrong.
> > >
> > > Why isn't the TUN driver calling the same code as other socket creating
> > > code?
> >
> > The other socket creating code handles the final setup/initialization in
> > the security_socket_post_create() hook which operates on sockets not
> > socks.
>
> I wonder if passing a flag might be better than the prot argument, which
> allows the caller to indicate what kind of initialization it's doing,
> rather than what will be seen as another protocol layering violation (i.e.
> the security model poking around to find out what kind of protocol &
> changing its behaviour).

Good point.  I'm going to be reworking the solution a bit, but if I still need 
to do something like this I'll go the flag route.

-- 
paul moore
linux @ hp


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.