2017-06-19 21:34:43

by Mira Ressel

[permalink] [raw]
Subject: [PATCH] selinux: Assign proper class to PF_UNIX/SOCK_RAW sockets

For PF_UNIX, SOCK_RAW is synonymous with SOCK_DGRAM (cf.
net/unix/af_unix.c). This is a tad obscure, but libpcap uses it.

Signed-off-by: Luis Ressel <[email protected]>
Acked-by: Stephen Smalley <[email protected]>
---
security/selinux/hooks.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 819fd6858b49..1a331fba4a3c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1275,6 +1275,7 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
case SOCK_SEQPACKET:
return SECCLASS_UNIX_STREAM_SOCKET;
case SOCK_DGRAM:
+ case SOCK_RAW:
return SECCLASS_UNIX_DGRAM_SOCKET;
}
break;
--
2.13.1


2017-06-20 19:49:10

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] selinux: Assign proper class to PF_UNIX/SOCK_RAW sockets

On Mon, Jun 19, 2017 at 5:33 PM, Luis Ressel <[email protected]> wrote:
> For PF_UNIX, SOCK_RAW is synonymous with SOCK_DGRAM (cf.
> net/unix/af_unix.c). This is a tad obscure, but libpcap uses it.
>
> Signed-off-by: Luis Ressel <[email protected]>
> Acked-by: Stephen Smalley <[email protected]>
> ---
> security/selinux/hooks.c | 1 +
> 1 file changed, 1 insertion(+)

My only concern is what effect this will have on existing policy.
Prior to this patch PF_UNIX/SOCK_RAW will result in the generic
"socket" class where after this patch it will result in the
"unix_dgram_socket". I believe this is the right change, but it seems
like this should be wrapped by a policy capability, yes?

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 819fd6858b49..1a331fba4a3c 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1275,6 +1275,7 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
> case SOCK_SEQPACKET:
> return SECCLASS_UNIX_STREAM_SOCKET;
> case SOCK_DGRAM:
> + case SOCK_RAW:
> return SECCLASS_UNIX_DGRAM_SOCKET;
> }
> break;
> --
> 2.13.1

--
paul moore
http://www.paul-moore.com

2017-06-20 20:01:37

by Stephen Smalley

[permalink] [raw]
Subject: Re: [PATCH] selinux: Assign proper class to PF_UNIX/SOCK_RAW sockets

On Tue, 2017-06-20 at 15:49 -0400, Paul Moore wrote:
> On Mon, Jun 19, 2017 at 5:33 PM, Luis Ressel <[email protected]> wrote:
> > For PF_UNIX, SOCK_RAW is synonymous with SOCK_DGRAM (cf.
> > net/unix/af_unix.c). This is a tad obscure, but libpcap uses it.
> >
> > Signed-off-by: Luis Ressel <[email protected]>
> > Acked-by: Stephen Smalley <[email protected]>
> > ---
> >  security/selinux/hooks.c | 1 +
> >  1 file changed, 1 insertion(+)
>
> My only concern is what effect this will have on existing policy.
> Prior to this patch PF_UNIX/SOCK_RAW will result in the generic
> "socket" class where after this patch it will result in the
> "unix_dgram_socket".  I believe this is the right change, but it
> seems
> like this should be wrapped by a policy capability, yes?

I doubt it is worth a policy capability. Permission to create/use
socket tends to be far rarer than permission to create/use
unix_dgram_socket; looks like we never allow the former without the
latter in Fedora, for example.

>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 819fd6858b49..1a331fba4a3c 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -1275,6 +1275,7 @@ static inline u16
> > socket_type_to_security_class(int family, int type, int protoc
> >                 case SOCK_SEQPACKET:
> >                         return SECCLASS_UNIX_STREAM_SOCKET;
> >                 case SOCK_DGRAM:
> > +               case SOCK_RAW:
> >                         return SECCLASS_UNIX_DGRAM_SOCKET;
> >                 }
> >                 break;
> > --
> > 2.13.1
>
>

2017-06-20 21:43:45

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] selinux: Assign proper class to PF_UNIX/SOCK_RAW sockets

On Tue, Jun 20, 2017 at 4:04 PM, Stephen Smalley <[email protected]> wrote:
> On Tue, 2017-06-20 at 15:49 -0400, Paul Moore wrote:
>> On Mon, Jun 19, 2017 at 5:33 PM, Luis Ressel <[email protected]> wrote:
>> > For PF_UNIX, SOCK_RAW is synonymous with SOCK_DGRAM (cf.
>> > net/unix/af_unix.c). This is a tad obscure, but libpcap uses it.
>> >
>> > Signed-off-by: Luis Ressel <[email protected]>
>> > Acked-by: Stephen Smalley <[email protected]>
>> > ---
>> > security/selinux/hooks.c | 1 +
>> > 1 file changed, 1 insertion(+)
>>
>> My only concern is what effect this will have on existing policy.
>> Prior to this patch PF_UNIX/SOCK_RAW will result in the generic
>> "socket" class where after this patch it will result in the
>> "unix_dgram_socket". I believe this is the right change, but it
>> seems
>> like this should be wrapped by a policy capability, yes?
>
> I doubt it is worth a policy capability.

Agreed that a policy capability is pretty heavy for something to have
little impact, I'm just trying to be a bit more consistent about these
things (reference the thread we had a few weeks ago).

> Permission to create/use
> socket tends to be far rarer than permission to create/use
> unix_dgram_socket; looks like we never allow the former without the
> latter in Fedora, for example.

Considering where we are at with respect to the merge window, let's
shelve this for now and I'll merge it after the next merge window
closes. In all likelihood I'll be sending selinux/next up to James
later this week and I'd like this to sit in linux-next for longer than
a few days.

--
paul moore
http://www.paul-moore.com

2017-06-21 09:48:17

by Mira Ressel

[permalink] [raw]
Subject: Re: [PATCH] selinux: Assign proper class to PF_UNIX/SOCK_RAW sockets

On Tue, 20 Jun 2017 17:43:38 -0400
Paul Moore <[email protected]> wrote:

> Considering where we are at with respect to the merge window, let's
> shelve this for now and I'll merge it after the next merge window
> closes. In all likelihood I'll be sending selinux/next up to James
> later this week and I'd like this to sit in linux-next for longer than
> a few days.

That means the change will land in 4.14 at the earliest, right? (Just
out of curiosity.)

By the way, refpolicy only grants "socket" permissions to a handful of
domains, all of which also have the corresponding "unix_dgram_socket"
permissions. The fedora policy does the same (according to Stephen);
this only leaves custom policies to be potentially affected by this
change.

Given that the SOCK_RAW->SOCK_DGRAM translation is obscure enough not to
be documented anywhere outside the kernel sources, I doubt there are
many users of it, anyway.

Regards,
Luis Ressel

2017-06-21 19:04:05

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] selinux: Assign proper class to PF_UNIX/SOCK_RAW sockets

On Wed, Jun 21, 2017 at 5:48 AM, Luis Ressel <[email protected]> wrote:
> On Tue, 20 Jun 2017 17:43:38 -0400
> Paul Moore <[email protected]> wrote:
>
>> Considering where we are at with respect to the merge window, let's
>> shelve this for now and I'll merge it after the next merge window
>> closes. In all likelihood I'll be sending selinux/next up to James
>> later this week and I'd like this to sit in linux-next for longer than
>> a few days.
>
> That means the change will land in 4.14 at the earliest, right? (Just
> out of curiosity.)

That's correct. We are currently working towards a v4.12 release in
Linus' tree, the upcoming merge window will be for v4.13, and things
merged into selinux/next after that merge window will be for v4.14.

> By the way, refpolicy only grants "socket" permissions to a handful of
> domains, all of which also have the corresponding "unix_dgram_socket"
> permissions. The fedora policy does the same (according to Stephen);
> this only leaves custom policies to be potentially affected by this
> change.

While custom policies are definitely in the minority, we still need to
do out best not to break them without warning.

> Given that the SOCK_RAW->SOCK_DGRAM translation is obscure enough not to
> be documented anywhere outside the kernel sources, I doubt there are
> many users of it, anyway.

You very well may be right, I just felt that such a change requires
more than a week in the selinux/next tree.

Thank you for your patch, it's in the queue and I'll be merging it
into the selinux/next branch in a few weeks.

--
paul moore
http://www.paul-moore.com

2017-07-10 22:25:19

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH] selinux: Assign proper class to PF_UNIX/SOCK_RAW sockets

On Wed, Jun 21, 2017 at 3:04 PM, Paul Moore <[email protected]> wrote:
> On Wed, Jun 21, 2017 at 5:48 AM, Luis Ressel <[email protected]> wrote:
>> On Tue, 20 Jun 2017 17:43:38 -0400
>> Paul Moore <[email protected]> wrote:
>>
>>> Considering where we are at with respect to the merge window, let's
>>> shelve this for now and I'll merge it after the next merge window
>>> closes. In all likelihood I'll be sending selinux/next up to James
>>> later this week and I'd like this to sit in linux-next for longer than
>>> a few days.
>>
>> That means the change will land in 4.14 at the earliest, right? (Just
>> out of curiosity.)
>
> That's correct. We are currently working towards a v4.12 release in
> Linus' tree, the upcoming merge window will be for v4.13, and things
> merged into selinux/next after that merge window will be for v4.14.
>
>> By the way, refpolicy only grants "socket" permissions to a handful of
>> domains, all of which also have the corresponding "unix_dgram_socket"
>> permissions. The fedora policy does the same (according to Stephen);
>> this only leaves custom policies to be potentially affected by this
>> change.
>
> While custom policies are definitely in the minority, we still need to
> do out best not to break them without warning.
>
>> Given that the SOCK_RAW->SOCK_DGRAM translation is obscure enough not to
>> be documented anywhere outside the kernel sources, I doubt there are
>> many users of it, anyway.
>
> You very well may be right, I just felt that such a change requires
> more than a week in the selinux/next tree.
>
> Thank you for your patch, it's in the queue and I'll be merging it
> into the selinux/next branch in a few weeks.

With all of the SELinux things for v4.13 upstream, I went ahead and
merged this patch into selinux/next.

--
paul moore
http://www.paul-moore.com