Re: [PATCH v3] selinux: add security in-core xattr support for tracefs

[Paul Moore]

On Tue, Dec 6, 2016 at 1:27 PM,   wrote:
> From: Yongqin Liu 
>
> Since kernel 4.1 ftrace is supported as a new separate filesystem. It
> gets automatically mounted by the kernel under the old path
> /sys/kernel/debug/tracing. Because it lives now on a separate filesystem
> SELinux needs to be updated to also support setting SELinux labels
> on tracefs inodes.  This is required for compatibility in Android
> when moving to Linux 4.1 or newer.
>
> Signed-off-by: Yongqin Liu 
> Signed-off-by: William Roberts 
> ---
>  security/selinux/hooks.c | 1 +
>  1 file changed, 1 insertion(+)

Merged for v4.11, thanks.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 09fd610..24bd84d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -491,6 +491,7 @@ static int selinux_is_sblabel_mnt(struct super_block *sb)
> !strcmp(sb->s_type->name, "sysfs") ||
> !strcmp(sb->s_type->name, "pstore") ||
> !strcmp(sb->s_type->name, "debugfs") ||
> +   !strcmp(sb->s_type->name, "tracefs") ||
> !strcmp(sb->s_type->name, "rootfs");
>  }
>
> --
> 2.7.4
>



-- 
paul moore
www.paul-moore.com
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH v2] selinux: support distinctions among all network address families

[Paul Moore]

On Wed, Dec 7, 2016 at 10:24 AM, Stephen Smalley  wrote:
> On 12/07/2016 10:14 AM, Paul Moore wrote:
>> On Wed, Dec 7, 2016 at 8:25 AM, Stephen Smalley  wrote:
>>> On 12/06/2016 07:13 PM, Paul Moore wrote:
 On Tue, Dec 6, 2016 at 10:00 AM, Stephen Smalley  
 wrote:
> Extend SELinux to support distinctions among all network address families
> implemented by the kernel by defining new socket security classes
> and mapping to them. Otherwise, many sockets are mapped to the generic
> socket class and are indistinguishable in policy.  This has come up
> previously with regard to selectively allowing access to bluetooth 
> sockets,
> and more recently with regard to selectively allowing access to AF_ALG
> sockets.  Guido Trentalancia submitted a patch that took a similar 
> approach
> to add only support for distinguishing AF_ALG sockets, but this 
> generalizes
> his approach to handle all address families implemented by the kernel.
> Socket security classes are also added for ICMP and SCTP sockets.
> Socket security classes were not defined for AF_* values that are reserved
> but unimplemented in the kernel, e.g. AF_NETBEUI, AF_SECURITY, AF_ASH,
> AF_ECONET, AF_SNA, AF_WANPIPE.
>
> Backward compatibility is provided by only enabling the finer-grained
> socket classes if a new policy capability is set in the policy; older
> policies will behave as before.  The legacy redhat1 policy capability
> that was only ever used in testing within Fedora for ptrace_child
> is reclaimed for this purpose; as far as I can tell, this policy
> capability is not enabled in any supported distro policy.
>
> Add a pair of conditional compilation guards to detect when new AF_* 
> values
> are added so that we can update SELinux accordingly rather than having to
> belatedly update it long after new address families are introduced.
>
> Signed-off-by: Stephen Smalley 
> ---
> v2 reworks the style based on comments from Guido Trentalancia and adds
> security classes for SCTP and ICMP sockets.  The security class support
> for SCTP sockets was based on Richard Haines' SCTP patch set.  For now,
> we only duplicate the class definition for rawip_socket for SCTP and ICMP
> sockets since that is how they were previously mapped.  The SCTP 
> definition
> can be further fleshed out by the SCTP patch set itself.
>
>  security/selinux/hooks.c| 73 
> +
>  security/selinux/include/classmap.h | 68 
> ++
>  security/selinux/include/security.h |  3 +-
>  security/selinux/selinuxfs.c|  2 +-
>  security/selinux/ss/services.c  |  3 ++
>  5 files changed, 147 insertions(+), 2 deletions(-)

 You mentioned IGMP previously, if we have a class for ICMP, it seems
 reasonable to have one for IGMP, don't you think?  Although this does
 spiral a bit if we consider all the IPPROTO* protocols.
>>>
>>> I thought about it, but the kernel does not provide IGMP sockets per se,
>>> unlike ICMP or SCTP sockets (i.e. ipv4/af_inet.c:inetsw_array[] defines
>>> an entry for SOCK_DGRAM, IPPROTO_ICMP and sctp/protocol.c defines and
>>> registers inet_protosw entries for SOCK_STREAM, IPPROTO_SCTP and
>>> SOCK_SEQPACKET, IPPROTO_SCTP; there is no equivalent for IGMP unless I
>>> missed it).  So IGMP sockets are just raw IP sockets with a particular
>>> protocol value; they have no stream, seqpacket, or dgram semantics, and
>>> it is unclear it is worthwhile to distinguish them in policy.
>>
>> Yes, sorry about that, it looks like you're right.  I thought there
>> might be some sort of IGMP routing daemon or something that would need
>> to create an IGMP socket, but it looks like you just configure the
>> kernel's multicast routing table and the kernel takes care of the IGMP
>> packets.
>
> Well, I think you can create an IGMP socket ala socket(PF_INET,
> SOCK_RAW, IPPROTO_IGMP).  And we could map that to a IGMP socket class
> if we really wanted to do so.  But that would be the first time we ever
> mapped a (PF_INET, SOCK_RAW, proto) to something other than
> SECCLASS_RAWIP_SOCKET.  And at that point we'd probably want to map
> every individual protocol value.  Not sure we need/want to go there.

Yes, creating classes for each IPPROTO value is not something I think
we want to do, although I have no problem doing it for key protocols
as necessary.

I just merged this into selinux#next, the pull request for v4.10 was
already sent up to James so this will go in during the v4.11 merge
window (I think this can wait, it might give us time to get some of
the policy ready).

-- 
paul moore
www.paul-moore.com
___
Selinux mailing list
Selinux@tycho.nsa.gov
To 

Re: Why do we block context="foobar" from User Namespace

[Daniel J Walsh]

On 12/07/2016 10:47 AM, Stephen Smalley wrote:
> On 12/07/2016 10:30 AM, Stephen Smalley wrote:
>> On 12/06/2016 07:19 PM, Paul Moore wrote:
>>> On Tue, Dec 6, 2016 at 3:31 PM, Stephen Smalley  wrote:
 On 12/06/2016 03:04 PM, Daniel J Walsh wrote:
> Currently in SELinux and UserNamespace can not be enabled with 
> Docker/runc at the same time.
>
> Runc mounts tmpfs directories with 
> --context="system_u:object_r:container_file_t:s0:c1,c2" type labels
> but the following patch blocks the use of context mounts when using user 
> namespace.
>
> http://kernel.suse.com/cgit/kernel/commit/?id=aad82892af261b9903cc11c55be3ecf5f0b0b4f8
>
> User Namespace has to be established before tmpfs are mounted so we are 
> unable to mount a
> tmpfs with a context=flag and UserNamespace enabled.
>
> Controlling the ability to change the label of a mounted file systemd 
> should be a MAC decision not a DAC,
> or UserNamespace. Setting the SELinux labels on an object like a file 
> system mount point
> should be controlled by SELinux.  SELinux should check if the label of 
> the process doing the
> mount is able to relabel from the label of the mount point, and labelto 
> the specified label.
>
> SELinux does this for privileged processes (running with SYS_ADMIN) so 
> use namespace should not be
> any different.  Also the process doing the mount would be allowed by DAC 
> to set the label of the tmpfs after
> it is mounted (As long as SELinux allowed).
>
> There is no security difference between:
>
> mount -o tmpfs context="foobar" none /dev
>
>
> And
>
> mount -o tmpfs none /dev
>
> chcon foobar -R /dev
>
>
> The second would not be blocked by usernamespace.
>
> Bottom line this patch should be reverted so container runtimes like 
> docker can use both User Namespace
> and SELinux at the same time.
 I doubt we want to revert it entirely.  Looks like Smack has an explicit
 exemption for tmpfs/ramfs (and sysfs, but it wouldn't really make sense
 to do it there).  We could do something similar.
>>> Yes, I still think the restriction makes sense for persistent
>>> filesystems, but for things like tmpfs it does seem silly.
>> Looking further at this, the set of filesystems that presently allow
>> userns mounting are mqueue, tmpfs, cgroup/cgroup2, sysfs, ramfs, proc,
>> and devpts.  I could see allowing context mounts for mqueue, tmpfs,
>> ramfs, and devpts within the user namespace.  For cgroup/cgroup2, sysfs
>> and proc we don't want the process to be able to override the
>> policy-defined labels.  Make sense?
> Sorry, never mind about mqueue - unless the process is also in its own
> IPC namespace, that could allow it to effectively relabel existing
> mqueues.  So only tmpfs, ramfs, and devpts seem to be safe in this regard.
>
>
> ___
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
>
>
That works for me.  The mqueue one we might have to visit later.  I
think docker and
user namespace with mqueue is currently a problem.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [RFC][PATCH] selinux: allow context mounts on tmpfs, ramfs, devpts within user namespaces

[Stephen Smalley]

On 12/07/2016 12:16 PM, Stephen Smalley wrote:
> commit aad82892af261b9903cc11c55be3ecf5f0b0b4f8 ("selinux: Add support for
> unprivileged mounts from user namespaces") prohibited any use of context
> mount options within non-init user namespaces.  However, this breaks
> use of context mount options for tmpfs mounts within user namespaces,
> which are being used by Docker/runc.  There is no reason to block such
> usage for tmpfs, ramfs or devpts.  Exempt these filesystem types
> from this restriction.
> 
> Before:
> sh$ userns_child_exec  -p -m -U -M '0 1000 1' -G '0 1000 1' bash
> sh# mount -t tmpfs -o context=system_u:object_r:user_tmp_t:s0:c13 none /tmp
> mount: tmpfs is write-protected, mounting read-only
> mount: cannot mount tmpfs read-only
> 
> After:
> sh$ userns_child_exec  -p -m -U -M '0 1000 1' -G '0 1000 1' bash
> sh# mount -t tmpfs -o context=system_u:object_r:user_tmp_t:s0:c13 none /tmp
> sh# ls -Zd /tmp
> unconfined_u:object_r:user_tmp_t:s0 /tmp
> 
> Note that this still isn't quite right, and I do not know why yet -
> the category (:c13) was dropped.  This works correctly in the init
> namespace, and strace of mount shows that it is passing the context
> correctly to the kernel and returning 0.

Never mind, this patch is wrong.

> 
> Signed-off-by: Stephen Smalley 
> ---
>  security/selinux/hooks.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 98a2e92..ef882a3 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -839,8 +839,12 @@ static int selinux_set_mnt_opts(struct super_block *sb,
>   if (sb->s_user_ns != _user_ns) {
>   if (context_sid || fscontext_sid || rootcontext_sid ||
>   defcontext_sid) {
> - rc = -EACCES;
> - goto out;
> + if (strcmp(sb->s_type->name, "tmpfs") &&
> + strcmp(sb->s_type->name, "ramfs") &&
> + strcmp(sb->s_type->name, "devpts")) {
> + rc = -EACCES;
> + goto out;
> + }
>   }
>   if (sbsec->behavior == SECURITY_FS_USE_XATTR) {
>   sbsec->behavior = SECURITY_FS_USE_MNTPOINT;
> 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

[RFC][PATCH] selinux: allow context mounts on tmpfs, ramfs, devpts within user namespaces

[Stephen Smalley]

commit aad82892af261b9903cc11c55be3ecf5f0b0b4f8 ("selinux: Add support for
unprivileged mounts from user namespaces") prohibited any use of context
mount options within non-init user namespaces.  However, this breaks
use of context mount options for tmpfs mounts within user namespaces,
which are being used by Docker/runc.  There is no reason to block such
usage for tmpfs, ramfs or devpts.  Exempt these filesystem types
from this restriction.

Before:
sh$ userns_child_exec  -p -m -U -M '0 1000 1' -G '0 1000 1' bash
sh# mount -t tmpfs -o context=system_u:object_r:user_tmp_t:s0:c13 none /tmp
mount: tmpfs is write-protected, mounting read-only
mount: cannot mount tmpfs read-only

After:
sh$ userns_child_exec  -p -m -U -M '0 1000 1' -G '0 1000 1' bash
sh# mount -t tmpfs -o context=system_u:object_r:user_tmp_t:s0:c13 none /tmp
sh# ls -Zd /tmp
unconfined_u:object_r:user_tmp_t:s0 /tmp

Note that this still isn't quite right, and I do not know why yet -
the category (:c13) was dropped.  This works correctly in the init
namespace, and strace of mount shows that it is passing the context
correctly to the kernel and returning 0.

Signed-off-by: Stephen Smalley 
---
 security/selinux/hooks.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 98a2e92..ef882a3 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -839,8 +839,12 @@ static int selinux_set_mnt_opts(struct super_block *sb,
if (sb->s_user_ns != _user_ns) {
if (context_sid || fscontext_sid || rootcontext_sid ||
defcontext_sid) {
-   rc = -EACCES;
-   goto out;
+   if (strcmp(sb->s_type->name, "tmpfs") &&
+   strcmp(sb->s_type->name, "ramfs") &&
+   strcmp(sb->s_type->name, "devpts")) {
+   rc = -EACCES;
+   goto out;
+   }
}
if (sbsec->behavior == SECURITY_FS_USE_XATTR) {
sbsec->behavior = SECURITY_FS_USE_MNTPOINT;
-- 
2.7.4

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH v2] selinux: support distinctions among all network address families

[Stephen Smalley]

On 12/07/2016 10:14 AM, Paul Moore wrote:
> On Wed, Dec 7, 2016 at 8:25 AM, Stephen Smalley  wrote:
>> On 12/06/2016 07:13 PM, Paul Moore wrote:
>>> On Tue, Dec 6, 2016 at 10:00 AM, Stephen Smalley  wrote:
 Extend SELinux to support distinctions among all network address families
 implemented by the kernel by defining new socket security classes
 and mapping to them. Otherwise, many sockets are mapped to the generic
 socket class and are indistinguishable in policy.  This has come up
 previously with regard to selectively allowing access to bluetooth sockets,
 and more recently with regard to selectively allowing access to AF_ALG
 sockets.  Guido Trentalancia submitted a patch that took a similar approach
 to add only support for distinguishing AF_ALG sockets, but this generalizes
 his approach to handle all address families implemented by the kernel.
 Socket security classes are also added for ICMP and SCTP sockets.
 Socket security classes were not defined for AF_* values that are reserved
 but unimplemented in the kernel, e.g. AF_NETBEUI, AF_SECURITY, AF_ASH,
 AF_ECONET, AF_SNA, AF_WANPIPE.

 Backward compatibility is provided by only enabling the finer-grained
 socket classes if a new policy capability is set in the policy; older
 policies will behave as before.  The legacy redhat1 policy capability
 that was only ever used in testing within Fedora for ptrace_child
 is reclaimed for this purpose; as far as I can tell, this policy
 capability is not enabled in any supported distro policy.

 Add a pair of conditional compilation guards to detect when new AF_* values
 are added so that we can update SELinux accordingly rather than having to
 belatedly update it long after new address families are introduced.

 Signed-off-by: Stephen Smalley 
 ---
 v2 reworks the style based on comments from Guido Trentalancia and adds
 security classes for SCTP and ICMP sockets.  The security class support
 for SCTP sockets was based on Richard Haines' SCTP patch set.  For now,
 we only duplicate the class definition for rawip_socket for SCTP and ICMP
 sockets since that is how they were previously mapped.  The SCTP definition
 can be further fleshed out by the SCTP patch set itself.

  security/selinux/hooks.c| 73 
 +
  security/selinux/include/classmap.h | 68 
 ++
  security/selinux/include/security.h |  3 +-
  security/selinux/selinuxfs.c|  2 +-
  security/selinux/ss/services.c  |  3 ++
  5 files changed, 147 insertions(+), 2 deletions(-)
>>>
>>> You mentioned IGMP previously, if we have a class for ICMP, it seems
>>> reasonable to have one for IGMP, don't you think?  Although this does
>>> spiral a bit if we consider all the IPPROTO* protocols.
>>
>> I thought about it, but the kernel does not provide IGMP sockets per se,
>> unlike ICMP or SCTP sockets (i.e. ipv4/af_inet.c:inetsw_array[] defines
>> an entry for SOCK_DGRAM, IPPROTO_ICMP and sctp/protocol.c defines and
>> registers inet_protosw entries for SOCK_STREAM, IPPROTO_SCTP and
>> SOCK_SEQPACKET, IPPROTO_SCTP; there is no equivalent for IGMP unless I
>> missed it).  So IGMP sockets are just raw IP sockets with a particular
>> protocol value; they have no stream, seqpacket, or dgram semantics, and
>> it is unclear it is worthwhile to distinguish them in policy.
> 
> Yes, sorry about that, it looks like you're right.  I thought there
> might be some sort of IGMP routing daemon or something that would need
> to create an IGMP socket, but it looks like you just configure the
> kernel's multicast routing table and the kernel takes care of the IGMP
> packets.

Well, I think you can create an IGMP socket ala socket(PF_INET,
SOCK_RAW, IPPROTO_IGMP).  And we could map that to a IGMP socket class
if we really wanted to do so.  But that would be the first time we ever
mapped a (PF_INET, SOCK_RAW, proto) to something other than
SECCLASS_RAWIP_SOCKET.  And at that point we'd probably want to map
every individual protocol value.  Not sure we need/want to go there.


___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH v2] selinux: support distinctions among all network address families

[Guido Trentalancia]

Hello.

On Wed, 07/12/2016 at 08.25 -0500, Stephen Smalley wrote:
> On 12/06/2016 07:13 PM, Paul Moore wrote:

[...]

> > You mentioned IGMP previously, if we have a class for ICMP, it
> > seems
> > reasonable to have one for IGMP, don't you think?  Although this
> > does
> > spiral a bit if we consider all the IPPROTO* protocols.
> 
> I thought about it, but the kernel does not provide IGMP sockets per
> se,
> unlike ICMP or SCTP sockets (i.e. ipv4/af_inet.c:inetsw_array[]
> defines
> an entry for SOCK_DGRAM, IPPROTO_ICMP and sctp/protocol.c defines and
> registers inet_protosw entries for SOCK_STREAM, IPPROTO_SCTP and
> SOCK_SEQPACKET, IPPROTO_SCTP; there is no equivalent for IGMP unless
> I
> missed it).  So IGMP sockets are just raw IP sockets with a
> particular
> protocol value; they have no stream, seqpacket, or dgram semantics,
> and
> it is unclear it is worthwhile to distinguish them in policy.

I suppose distinguishing IGMP packets brings little benefit in terms of
security.

Regards,

Guido
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH v2] selinux: support distinctions among all network address families

[Paul Moore]

On Wed, Dec 7, 2016 at 8:25 AM, Stephen Smalley  wrote:
> On 12/06/2016 07:13 PM, Paul Moore wrote:
>> On Tue, Dec 6, 2016 at 10:00 AM, Stephen Smalley  wrote:
>>> Extend SELinux to support distinctions among all network address families
>>> implemented by the kernel by defining new socket security classes
>>> and mapping to them. Otherwise, many sockets are mapped to the generic
>>> socket class and are indistinguishable in policy.  This has come up
>>> previously with regard to selectively allowing access to bluetooth sockets,
>>> and more recently with regard to selectively allowing access to AF_ALG
>>> sockets.  Guido Trentalancia submitted a patch that took a similar approach
>>> to add only support for distinguishing AF_ALG sockets, but this generalizes
>>> his approach to handle all address families implemented by the kernel.
>>> Socket security classes are also added for ICMP and SCTP sockets.
>>> Socket security classes were not defined for AF_* values that are reserved
>>> but unimplemented in the kernel, e.g. AF_NETBEUI, AF_SECURITY, AF_ASH,
>>> AF_ECONET, AF_SNA, AF_WANPIPE.
>>>
>>> Backward compatibility is provided by only enabling the finer-grained
>>> socket classes if a new policy capability is set in the policy; older
>>> policies will behave as before.  The legacy redhat1 policy capability
>>> that was only ever used in testing within Fedora for ptrace_child
>>> is reclaimed for this purpose; as far as I can tell, this policy
>>> capability is not enabled in any supported distro policy.
>>>
>>> Add a pair of conditional compilation guards to detect when new AF_* values
>>> are added so that we can update SELinux accordingly rather than having to
>>> belatedly update it long after new address families are introduced.
>>>
>>> Signed-off-by: Stephen Smalley 
>>> ---
>>> v2 reworks the style based on comments from Guido Trentalancia and adds
>>> security classes for SCTP and ICMP sockets.  The security class support
>>> for SCTP sockets was based on Richard Haines' SCTP patch set.  For now,
>>> we only duplicate the class definition for rawip_socket for SCTP and ICMP
>>> sockets since that is how they were previously mapped.  The SCTP definition
>>> can be further fleshed out by the SCTP patch set itself.
>>>
>>>  security/selinux/hooks.c| 73 
>>> +
>>>  security/selinux/include/classmap.h | 68 ++
>>>  security/selinux/include/security.h |  3 +-
>>>  security/selinux/selinuxfs.c|  2 +-
>>>  security/selinux/ss/services.c  |  3 ++
>>>  5 files changed, 147 insertions(+), 2 deletions(-)
>>
>> You mentioned IGMP previously, if we have a class for ICMP, it seems
>> reasonable to have one for IGMP, don't you think?  Although this does
>> spiral a bit if we consider all the IPPROTO* protocols.
>
> I thought about it, but the kernel does not provide IGMP sockets per se,
> unlike ICMP or SCTP sockets (i.e. ipv4/af_inet.c:inetsw_array[] defines
> an entry for SOCK_DGRAM, IPPROTO_ICMP and sctp/protocol.c defines and
> registers inet_protosw entries for SOCK_STREAM, IPPROTO_SCTP and
> SOCK_SEQPACKET, IPPROTO_SCTP; there is no equivalent for IGMP unless I
> missed it).  So IGMP sockets are just raw IP sockets with a particular
> protocol value; they have no stream, seqpacket, or dgram semantics, and
> it is unclear it is worthwhile to distinguish them in policy.

Yes, sorry about that, it looks like you're right.  I thought there
might be some sort of IGMP routing daemon or something that would need
to create an IGMP socket, but it looks like you just configure the
kernel's multicast routing table and the kernel takes care of the IGMP
packets.

-- 
paul moore
www.paul-moore.com
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH] libsepol/cil: remove avrules with no affected types

[Gary Tierney]

On Wed, Dec 07, 2016 at 08:27:05AM -0500, Stephen Smalley wrote:
> On 12/07/2016 07:15 AM, Gary Tierney wrote:
> > Adds a check for avrules with type attributes that have a bitmap cardinality
> > of 0 (i.e., no types in their set) before adding them to the libsepol 
> > policy in
> > __cil_avrule_to_avtab().  Also adds an exception for neverallow rules to
> > prevent breaking anything from AOSP mentioned in
> > f9927d9370f90bd9d975ff933fe107ec4f93a9ac.
> 
> James Carter is away for a few days, so this might be delayed in review.
> 

No problem.  I'll try and get the second part of this (removing typeattributes
which are only used in these dud avrules) to the list in time for reviewing
this.

> > 
> > Signed-off-by: Gary Tierney 
> > ---
> >  libsepol/cil/src/cil_binary.c | 47 
> > +++
> >  1 file changed, 47 insertions(+)
> > 
> > diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
> > index d33981b..3aa350a 100644
> > --- a/libsepol/cil/src/cil_binary.c
> > +++ b/libsepol/cil/src/cil_binary.c
> > @@ -1411,6 +1411,48 @@ exit:
> > return rc;
> >  }
> >  
> > +static int __cil_type_datum_is_unused_attrib(struct cil_symtab_datum *src)
> > +{
> > +   struct cil_tree_node *node = NULL;
> > +   struct cil_typeattribute *attrib = NULL;
> > +
> > +   if (src->fqn == CIL_KEY_SELF) {
> > +   return CIL_FALSE;
> > +   }
> > +
> > +   node = src->nodes->head->data;
> > +
> > +   if (node->flavor != CIL_TYPEATTRIBUTE) {
> > +   return CIL_FALSE;
> > +   }
> > +
> > +   attrib = (struct cil_typeattribute *) src;
> > +   return ebitmap_cardinality(attrib->types) == 0;
> > +}
> > +
> > +static int __cil_avrule_can_remove(struct cil_avrule *cil_avrule)
> > +{
> > +   struct cil_symtab_datum *src = cil_avrule->src;
> > +   struct cil_symtab_datum *tgt = cil_avrule->tgt;
> > +
> > +   // Don't remove neverallow rules so they are written to
> > +   // the resulting policy and can be checked by tools in
> > +   // AOSP.
> > +   if (cil_avrule->rule_kind == CIL_AVRULE_NEVERALLOW) {
> > +   return CIL_FALSE;
> > +   }
> > +
> > +   if (__cil_type_datum_is_unused_attrib(src)) {
> > +   return CIL_TRUE;
> > +   }
> > +
> > +   if (__cil_type_datum_is_unused_attrib(tgt)) {
> > +   return CIL_TRUE;
> > +   }
> > +
> > +   return CIL_FALSE;
> > +}
> > +
> >  int __cil_avrule_to_avtab(policydb_t *pdb, const struct cil_db *db, struct 
> > cil_avrule *cil_avrule, cond_node_t *cond_node, enum cil_flavor cond_flavor)
> >  {
> > int rc = SEPOL_ERR;
> > @@ -1425,6 +1467,11 @@ int __cil_avrule_to_avtab(policydb_t *pdb, const 
> > struct cil_db *db, struct cil_a
> > goto exit;
> > }
> >  
> > +   if (__cil_avrule_can_remove(cil_avrule)) {
> > +   rc = SEPOL_OK;
> > +   goto exit;
> > +   }
> > +
> > src = cil_avrule->src;
> > tgt = cil_avrule->tgt;
> >  
> > 
> 

-- 
Gary Tierney

GPG fingerprint: 412C 0EF9 C305 68E6 B660  BDAF 706E D765 85AA 79D8
https://sks-keyservers.net/pks/lookup?op=get=0x706ED76585AA79D8


signature.asc
Description: PGP signature
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH v2] selinux: support distinctions among all network address families

[Stephen Smalley]

On 12/06/2016 07:13 PM, Paul Moore wrote:
> On Tue, Dec 6, 2016 at 10:00 AM, Stephen Smalley  wrote:
>> Extend SELinux to support distinctions among all network address families
>> implemented by the kernel by defining new socket security classes
>> and mapping to them. Otherwise, many sockets are mapped to the generic
>> socket class and are indistinguishable in policy.  This has come up
>> previously with regard to selectively allowing access to bluetooth sockets,
>> and more recently with regard to selectively allowing access to AF_ALG
>> sockets.  Guido Trentalancia submitted a patch that took a similar approach
>> to add only support for distinguishing AF_ALG sockets, but this generalizes
>> his approach to handle all address families implemented by the kernel.
>> Socket security classes are also added for ICMP and SCTP sockets.
>> Socket security classes were not defined for AF_* values that are reserved
>> but unimplemented in the kernel, e.g. AF_NETBEUI, AF_SECURITY, AF_ASH,
>> AF_ECONET, AF_SNA, AF_WANPIPE.
>>
>> Backward compatibility is provided by only enabling the finer-grained
>> socket classes if a new policy capability is set in the policy; older
>> policies will behave as before.  The legacy redhat1 policy capability
>> that was only ever used in testing within Fedora for ptrace_child
>> is reclaimed for this purpose; as far as I can tell, this policy
>> capability is not enabled in any supported distro policy.
>>
>> Add a pair of conditional compilation guards to detect when new AF_* values
>> are added so that we can update SELinux accordingly rather than having to
>> belatedly update it long after new address families are introduced.
>>
>> Signed-off-by: Stephen Smalley 
>> ---
>> v2 reworks the style based on comments from Guido Trentalancia and adds
>> security classes for SCTP and ICMP sockets.  The security class support
>> for SCTP sockets was based on Richard Haines' SCTP patch set.  For now,
>> we only duplicate the class definition for rawip_socket for SCTP and ICMP
>> sockets since that is how they were previously mapped.  The SCTP definition
>> can be further fleshed out by the SCTP patch set itself.
>>
>>  security/selinux/hooks.c| 73 
>> +
>>  security/selinux/include/classmap.h | 68 ++
>>  security/selinux/include/security.h |  3 +-
>>  security/selinux/selinuxfs.c|  2 +-
>>  security/selinux/ss/services.c  |  3 ++
>>  5 files changed, 147 insertions(+), 2 deletions(-)
> 
> You mentioned IGMP previously, if we have a class for ICMP, it seems
> reasonable to have one for IGMP, don't you think?  Although this does
> spiral a bit if we consider all the IPPROTO* protocols.

I thought about it, but the kernel does not provide IGMP sockets per se,
unlike ICMP or SCTP sockets (i.e. ipv4/af_inet.c:inetsw_array[] defines
an entry for SOCK_DGRAM, IPPROTO_ICMP and sctp/protocol.c defines and
registers inet_protosw entries for SOCK_STREAM, IPPROTO_SCTP and
SOCK_SEQPACKET, IPPROTO_SCTP; there is no equivalent for IGMP unless I
missed it).  So IGMP sockets are just raw IP sockets with a particular
protocol value; they have no stream, seqpacket, or dgram semantics, and
it is unclear it is worthwhile to distinguish them in policy.

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH] quick selinux support for tracefs

[YongQin Liu]

On 7 December 2016 at 18:59, Roberts, William C  wrote:

> Yes it was submitted to Android’s Gerrit Here by you:
>
> https://android-review.googlesource.com/#/c/220895/
>
>
>
> I already modified the commit message and got an Ack on V3 from Stephen:
>
> http://marc.info/?l=selinux=148105682211911=2
>
>
>
Thanks, Roberts!


>
>
> *From:* YongQin Liu [mailto:yongqin@linaro.org]
> *Sent:* Tuesday, December 6, 2016 11:06 PM
> *To:* Roberts, William C 
> *Cc:* Stephen Smalley ; n...@google.com;
> selinux@tycho.nsa.gov; Paul Moore 
>
> *Subject:* Re: [PATCH] quick selinux support for tracefs
>
>
>
> Hi, Roberts
>
>
>
> On 7 December 2016 at 02:05, Roberts, William C <
> william.c.robe...@intel.com> wrote:
>
>
>
> > -Original Message-
> > From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
>
> > Sent: Tuesday, December 6, 2016 10:01 AM
> > To: Roberts, William C ; n...@google.com;
> > selinux@tycho.nsa.gov
> > Cc: Yongqin Liu ; Paul Moore <
> p...@paul-moore.com>
> > Subject: Re: [PATCH] quick selinux support for tracefs
> >
> > On 12/06/2016 12:50 PM, Roberts, William C wrote:
> > >
> > >
> > >> -Original Message-
> > >> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> > >> Sent: Tuesday, December 6, 2016 9:41 AM
> > >> To: Roberts, William C ; n...@google.com;
> > >> selinux@tycho.nsa.gov
> > >> Cc: Yongqin Liu ; Paul Moore
> > >> 
> > >> Subject: Re: [PATCH] quick selinux support for tracefs
> > >>
> > >> On 12/06/2016 12:24 PM, william.c.robe...@intel.com wrote:
> > >>> From: Yongqin Liu 
> > >>>
> > >>> Here is just the quick fix for tracefs with selinux.
> > >>> just add tracefs to the list of whitelisted filesystem types in
> > >>> selinux_is_sblabel_mnt(), but the right fix would be to generalize
> > >>> this logic as described in the last item on the todo list,
> > >>> https://bitbucket.org/seandroid/wiki/wiki/ToDo
> > >>>
> > >>> Change-Id: I2aa803ccffbcd2802a7287514da7648e6b364157
> > >>
> > >> Please rewrite the subject line and patch description per the
> > >> kernel's submission guidelines, drop the Change-Id and the link to
> > >> the SEAndroid todo list, and don't say that this is a quick fix but
> > >
> > > Why would anyone do that for this patch when below you say it won’t be
> > > merged unless we fix issue #2?
> >
> > I didn't say it couldn't be merged; I said it isn't a good idea to say
> "this is a quick fix
> > but the right fix is X" in an upstream patch submission if you want it
> to be
> > merged, unless it is for a serious security or stability bug that needs
> to be fixed
> > right away.
>
> Oh sure, but I didn't right that patch message, Liu Yonggin is tha author,
> ill fix up the
> message and resubmit preserving him as the author.
>
>
>
> This change was submitted to http://android-review.googlesource.com/ via
> gerrit instructions before,
>
> and now seems that it is going to be submitted via the instructions for
> kernel changes which I am not familiar.
>
>
>
> If you like, please help to update the patch message and submit the patch.
>
>
>
> Or you could share me the instructions on how to do that, and then I will
> submit the changes with new message following your instructions.
>
>
>
> Thanks,
>
> Yongqin Liu
>
>
> >
> > >
> > > the right fix is something else if you want this to
> > >> actually be merged.  Because in that case, you ought to just
> > >> implement the right fix.  There is now an upstream kernel issue for
> the right
> > fix:
> > >> https://github.com/SELinuxProject/selinux-kernel/issues/2
> > >
> > > The other question here is tracefs safe to label in this fashion, I
> would assume
> > yes.
> > > Looking through I didn't see any eviction code.
> >
> > Yes, the inodes are pinned.
>
>
>
>
>
> --
>
> Best Regards,
>
> Yongqin Liu
>
> ---
>
> #mailing list
>
> linaro-andr...@lists.linaro.org 
>
> http://lists.linaro.org/mailman/listinfo/linaro-android
>



-- 
Best Regards,
Yongqin Liu
---
#mailing list
linaro-andr...@lists.linaro.org 
http://lists.linaro.org/mailman/listinfo/linaro-android
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH] quick selinux support for tracefs

[YongQin Liu]

Hi, Roberts

On 7 December 2016 at 02:05, Roberts, William C  wrote:

>
>
> > -Original Message-
> > From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> > Sent: Tuesday, December 6, 2016 10:01 AM
> > To: Roberts, William C ; n...@google.com;
> > selinux@tycho.nsa.gov
> > Cc: Yongqin Liu ; Paul Moore <
> p...@paul-moore.com>
> > Subject: Re: [PATCH] quick selinux support for tracefs
> >
> > On 12/06/2016 12:50 PM, Roberts, William C wrote:
> > >
> > >
> > >> -Original Message-
> > >> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> > >> Sent: Tuesday, December 6, 2016 9:41 AM
> > >> To: Roberts, William C ; n...@google.com;
> > >> selinux@tycho.nsa.gov
> > >> Cc: Yongqin Liu ; Paul Moore
> > >> 
> > >> Subject: Re: [PATCH] quick selinux support for tracefs
> > >>
> > >> On 12/06/2016 12:24 PM, william.c.robe...@intel.com wrote:
> > >>> From: Yongqin Liu 
> > >>>
> > >>> Here is just the quick fix for tracefs with selinux.
> > >>> just add tracefs to the list of whitelisted filesystem types in
> > >>> selinux_is_sblabel_mnt(), but the right fix would be to generalize
> > >>> this logic as described in the last item on the todo list,
> > >>> https://bitbucket.org/seandroid/wiki/wiki/ToDo
> > >>>
> > >>> Change-Id: I2aa803ccffbcd2802a7287514da7648e6b364157
> > >>
> > >> Please rewrite the subject line and patch description per the
> > >> kernel's submission guidelines, drop the Change-Id and the link to
> > >> the SEAndroid todo list, and don't say that this is a quick fix but
> > >
> > > Why would anyone do that for this patch when below you say it won’t be
> > > merged unless we fix issue #2?
> >
> > I didn't say it couldn't be merged; I said it isn't a good idea to say
> "this is a quick fix
> > but the right fix is X" in an upstream patch submission if you want it
> to be
> > merged, unless it is for a serious security or stability bug that needs
> to be fixed
> > right away.
>
> Oh sure, but I didn't right that patch message, Liu Yonggin is tha author,
> ill fix up the
> message and resubmit preserving him as the author.
>

This change was submitted to http://android-review.googlesource.com/ via
gerrit instructions before,
and now seems that it is going to be submitted via the instructions for
kernel changes which I am not familiar.

If you like, please help to update the patch message and submit the patch.

Or you could share me the instructions on how to do that, and then I will
submit the changes with new message following your instructions.

Thanks,
Yongqin Liu

>
> >
> > >
> > > the right fix is something else if you want this to
> > >> actually be merged.  Because in that case, you ought to just
> > >> implement the right fix.  There is now an upstream kernel issue for
> the right
> > fix:
> > >> https://github.com/SELinuxProject/selinux-kernel/issues/2
> > >
> > > The other question here is tracefs safe to label in this fashion, I
> would assume
> > yes.
> > > Looking through I didn't see any eviction code.
> >
> > Yes, the inodes are pinned.
>
>


-- 
Best Regards,
Yongqin Liu
---
#mailing list
linaro-andr...@lists.linaro.org 
http://lists.linaro.org/mailman/listinfo/linaro-android
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

[PATCH] cil: remove avrules that have no associated types

[Gary Tierney]

This is a minor improvement to the CIL -> policydb code which will remove
unused AV rules.  In the past you could create 2 type attributes like so:

(typeattribute x)
(typeattribute y)
(type z)
(typeattributeset y z)

And an avrule (which would be kept in the resulting policydb):

(allow x y (process (transition)))

Now the avrule would be removed, since the source of the rule has
no associated types.  Similarly, the rule would have been removed if the
target had no associated types when the source does.  The exception to this
rule is neverallows, since AOSP checks for them in their resulting policy.conf.

There's a small difference in fedora-selinux:

[root@localhost ~]# sesearch -ACS original.30  | head -1
Found 101204 semantic av rules:
[root@localhost ~]# sesearch -ACS modified.30  | head -1
Found 101030 semantic av rules:

And a more noticeable difference with Dominick Grift's dssp1:

[root@localhost ~]# sesearch -ACS dssp_original.30 | head -1
Found 11270 semantic av rules:
[root@localhost ~]# sesearch -ACS dssp_modified.30 | head -1
Found 2574 semantic av rules:

Gary Tierney (1):
  libsepol/cil: remove avrules with no affected types

 libsepol/cil/src/cil_binary.c | 47 +++
 1 file changed, 47 insertions(+)

-- 
2.4.11

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

[PATCH] libsepol/cil: remove avrules with no affected types

[Gary Tierney]

Adds a check for avrules with type attributes that have a bitmap cardinality
of 0 (i.e., no types in their set) before adding them to the libsepol policy in
__cil_avrule_to_avtab().  Also adds an exception for neverallow rules to
prevent breaking anything from AOSP mentioned in
f9927d9370f90bd9d975ff933fe107ec4f93a9ac.

Signed-off-by: Gary Tierney 
---
 libsepol/cil/src/cil_binary.c | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/libsepol/cil/src/cil_binary.c b/libsepol/cil/src/cil_binary.c
index d33981b..3aa350a 100644
--- a/libsepol/cil/src/cil_binary.c
+++ b/libsepol/cil/src/cil_binary.c
@@ -1411,6 +1411,48 @@ exit:
return rc;
 }
 
+static int __cil_type_datum_is_unused_attrib(struct cil_symtab_datum *src)
+{
+   struct cil_tree_node *node = NULL;
+   struct cil_typeattribute *attrib = NULL;
+
+   if (src->fqn == CIL_KEY_SELF) {
+   return CIL_FALSE;
+   }
+
+   node = src->nodes->head->data;
+
+   if (node->flavor != CIL_TYPEATTRIBUTE) {
+   return CIL_FALSE;
+   }
+
+   attrib = (struct cil_typeattribute *) src;
+   return ebitmap_cardinality(attrib->types) == 0;
+}
+
+static int __cil_avrule_can_remove(struct cil_avrule *cil_avrule)
+{
+   struct cil_symtab_datum *src = cil_avrule->src;
+   struct cil_symtab_datum *tgt = cil_avrule->tgt;
+
+   // Don't remove neverallow rules so they are written to
+   // the resulting policy and can be checked by tools in
+   // AOSP.
+   if (cil_avrule->rule_kind == CIL_AVRULE_NEVERALLOW) {
+   return CIL_FALSE;
+   }
+
+   if (__cil_type_datum_is_unused_attrib(src)) {
+   return CIL_TRUE;
+   }
+
+   if (__cil_type_datum_is_unused_attrib(tgt)) {
+   return CIL_TRUE;
+   }
+
+   return CIL_FALSE;
+}
+
 int __cil_avrule_to_avtab(policydb_t *pdb, const struct cil_db *db, struct 
cil_avrule *cil_avrule, cond_node_t *cond_node, enum cil_flavor cond_flavor)
 {
int rc = SEPOL_ERR;
@@ -1425,6 +1467,11 @@ int __cil_avrule_to_avtab(policydb_t *pdb, const struct 
cil_db *db, struct cil_a
goto exit;
}
 
+   if (__cil_avrule_can_remove(cil_avrule)) {
+   rc = SEPOL_OK;
+   goto exit;
+   }
+
src = cil_avrule->src;
tgt = cil_avrule->tgt;
 
-- 
2.4.11

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

RE: [PATCH] quick selinux support for tracefs

[Roberts, William C]

Yes it was submitted to Android’s Gerrit Here by you:
https://android-review.googlesource.com/#/c/220895/

I already modified the commit message and got an Ack on V3 from Stephen:
http://marc.info/?l=selinux=148105682211911=2


From: YongQin Liu [mailto:yongqin@linaro.org]
Sent: Tuesday, December 6, 2016 11:06 PM
To: Roberts, William C 
Cc: Stephen Smalley ; n...@google.com; 
selinux@tycho.nsa.gov; Paul Moore 
Subject: Re: [PATCH] quick selinux support for tracefs

Hi, Roberts

On 7 December 2016 at 02:05, Roberts, William C 
> wrote:


> -Original Message-
> From: Stephen Smalley [mailto:s...@tycho.nsa.gov]
> Sent: Tuesday, December 6, 2016 10:01 AM
> To: Roberts, William C 
> >; 
> n...@google.com;
> selinux@tycho.nsa.gov
> Cc: Yongqin Liu >; Paul 
> Moore >
> Subject: Re: [PATCH] quick selinux support for tracefs
>
> On 12/06/2016 12:50 PM, Roberts, William C wrote:
> >
> >
> >> -Original Message-
> >> From: Stephen Smalley 
> >> [mailto:s...@tycho.nsa.gov]
> >> Sent: Tuesday, December 6, 2016 9:41 AM
> >> To: Roberts, William C 
> >> >; 
> >> n...@google.com;
> >> selinux@tycho.nsa.gov
> >> Cc: Yongqin Liu >; 
> >> Paul Moore
> >> >
> >> Subject: Re: [PATCH] quick selinux support for tracefs
> >>
> >> On 12/06/2016 12:24 PM, 
> >> william.c.robe...@intel.com wrote:
> >>> From: Yongqin Liu >
> >>>
> >>> Here is just the quick fix for tracefs with selinux.
> >>> just add tracefs to the list of whitelisted filesystem types in
> >>> selinux_is_sblabel_mnt(), but the right fix would be to generalize
> >>> this logic as described in the last item on the todo list,
> >>> https://bitbucket.org/seandroid/wiki/wiki/ToDo
> >>>
> >>> Change-Id: I2aa803ccffbcd2802a7287514da7648e6b364157
> >>
> >> Please rewrite the subject line and patch description per the
> >> kernel's submission guidelines, drop the Change-Id and the link to
> >> the SEAndroid todo list, and don't say that this is a quick fix but
> >
> > Why would anyone do that for this patch when below you say it won’t be
> > merged unless we fix issue #2?
>
> I didn't say it couldn't be merged; I said it isn't a good idea to say "this 
> is a quick fix
> but the right fix is X" in an upstream patch submission if you want it to be
> merged, unless it is for a serious security or stability bug that needs to be 
> fixed
> right away.
Oh sure, but I didn't right that patch message, Liu Yonggin is tha author, ill 
fix up the
message and resubmit preserving him as the author.

This change was submitted to http://android-review.googlesource.com/ via gerrit 
instructions before,
and now seems that it is going to be submitted via the instructions for kernel 
changes which I am not familiar.

If you like, please help to update the patch message and submit the patch.

Or you could share me the instructions on how to do that, and then I will 
submit the changes with new message following your instructions.

Thanks,
Yongqin Liu

>
> >
> > the right fix is something else if you want this to
> >> actually be merged.  Because in that case, you ought to just
> >> implement the right fix.  There is now an upstream kernel issue for the 
> >> right
> fix:
> >> https://github.com/SELinuxProject/selinux-kernel/issues/2
> >
> > The other question here is tracefs safe to label in this fashion, I would 
> > assume
> yes.
> > Looking through I didn't see any eviction code.
>
> Yes, the inodes are pinned.



--
Best Regards,
Yongqin Liu
---
#mailing list
linaro-andr...@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.