On 7/9/19 5:18 PM, Casey Schaufler wrote:
On 7/9/2019 11:12 AM, Stephen Smalley wrote:
On 7/9/19 1:51 PM, Casey Schaufler wrote:
On 7/9/2019 10:13 AM, Stephen Smalley wrote:
On 7/3/19 5:25 PM, Casey Schaufler wrote:
Create a new entry "display" in /proc/.../attr for controlling
which LSM security information is displayed for a process.
The name of an active LSM that supplies hooks for human readable
data may be written to "display" to set the value. The name of
the LSM currently in use can be read from "display".
At this point there can only be one LSM capable of display
active. A helper function lsm_task_display() to get the display
slot for a task_struct.

As I explained previously, this is a security hole waiting to happen. It still 
permits a process to affect the output of audit, alter the result of reading or 
writing /proc/self/attr nodes even by setuid/setgid/file-caps/context-changing 
programs, alter the contexts generated in netlink messages delivered to other 
processes (I think?), and possibly other effects beyond affecting the process' 
own view of things.

I would very much like some feedback regarding which of the
possible formats for putting multiple subject contexts in
audit records would be preferred:

     lsm=selinux,subj=xyzzy_t lsm=smack,subj=Xyzzy
     lsm=selinux,smack subj=xyzzy_t,Xyzzy
     subj="selinux='xyzzy_t',smack='Xyzzy'"

(cc'd linux-audit mailing list)


Or something else. Free bikeshedding!

I don't see how you have a problem with netlink. My look
at what's in the kernel didn't expose anything, but I am
willing to be educated.

I haven't traced through it in detail, but it wasn't clear to me that the 
security_secid_to_secctx() call always occurs in the context of the receiving 
process (and hence use its display value).  If not, then the display of the 
sender can affect what is reported to the receiver; hence, there is a forgery 
concern similar to the binder issue.  It would be cleaner if we didn't alter 
the default behavior of security_secid_to_secctx() and 
security_secctx_to_secid() and instead introduced new hooks for any case where 
we truly want the display to take effect.

If the context is generated by security_secid_to_secctx() we
retain the slot number of the module that created it in lsmcontext.
We have to to ensure it is released correctly. If the potential
issue you're describing for netlink does in fact occur, we can check
the slot in lsmcontext to verify that it is the same.

security_secid_to_secctx() is called nowhere in net/netlink,
at least not that grep finds. Where are you seeing this potential
problem?

Look under net/netfilter.




Before:
$ id
uid=1002(sds2) gid=1002(sds2) groups=1002(sds2) 
context=staff_u:staff_r:staff_t:s0-s0:c0.c1023
$ su
Password:
su: Authentication failure

syscall audit record:
type=SYSCALL msg=audit(07/09/2019 11:52:49.784:365) : arch=x86_64 syscall=openat
   success=no exit=EACCES(Permission denied) a0=0xffffff9c a1=0x560897e58e00 
a2=O_
WRONLY a3=0x0 items=1 ppid=3258 pid=3781 auid=sds2 uid=sds2 gid=sds2 euid=root s
uid=root fsuid=root egid=sds2 sgid=sds2 fsgid=sds2 tty=pts2 ses=6 comm=su 
exe=/usr/bin/su subj=staff_u:staff_r:staff_t:s0-s0:c0.c1023 key=(null)

After:
$ id
uid=1002(sds2) gid=1002(sds2) groups=1002(sds2) 
context=staff_u:staff_r:staff_t:s0-s0:c0.c1023
$ echo apparmor > /proc/self/attr/display
$ su
Password:
su: Authentication failure

audit record:
type=SYSCALL msg=audit(07/09/2019 12:05:32.402:406) : arch=x86_64 
syscall=openat success=no exit=EACCES(Permission denied) a0=0xffffff9c 
a1=0x556b41e1ae00 a2=O_WRONLY a3=0x0 items=1 ppid=3258 pid=9426 auid=sds2 
uid=sds2 gid=sds2 euid=root suid=root fsuid=root egid=sds2 sgid=sds2 fsgid=sds2 
tty=pts2 ses=6 comm=su exe=/usr/bin/su subj==unconfined key=(null)

NB The subj= field of the SYSCALL audit record is longer accurate and is 
potentially under the control of a process that would not be authorized to set 
its subject label to that value by SELinux.

It's still accurate, it's just not complete. It's a matter
of how best to complete it.


Now, let's play with userspace.

Before:
# id
uid=0(root) gid=0(root) groups=0(root) 
context=staff_u:staff_r:staff_t:s0-s0:c0.c1023
# passwd root
passwd: SELinux deny access due to security policy.

audit record:
type=USER_AVC msg=audit(07/09/2019 12:24:35.135:812) : pid=12693 uid=root 
auid=sds2 ses=7 subj=staff_u:staff_r:passwd_t:s0-s0:c0.c1023 msg='avc:  denied  
{ passwd } for scontext=staff_u:staff_r:staff_t:s0-s0:c0.c1023 
tcontext=staff_u:staff_r:staff_t:s0-s0:c0.c1023 tclass=passwd permissive=0  
exe=/usr/bin/passwd sauid=root hostname=? addr=? terminal=pts/2'
type=USER_CHAUTHTOK msg=audit(07/09/2019 12:24:35.135:813) : pid=12693 uid=root 
auid=sds2 ses=7 subj=staff_u:staff_r:passwd_t:s0-s0:c0.c1023 
msg='op=attempted-to-change-password id=root exe=/usr/bin/passwd 
hostname=moss-pluto.infosec.tycho.ncsc.mil addr=? terminal=pts/2 res=failed'

After:
# id
uid=0(root) gid=0(root) groups=0(root) 
context=staff_u:staff_r:staff_t:s0-s0:c0.c1023
# echo apparmor > /proc/self/attr/display
# passwd root
passwd: SELinux deny access due to security policy.

audit record:
type=USER_CHAUTHTOK msg=audit(07/09/2019 12:28:41.349:832) : pid=13083 uid=root 
auid=sds2 ses=7 subj==unconfined msg='op=attempted-to-change-password id=root 
exe=/usr/bin/passwd hostname=moss-pluto.infosec.tycho.ncsc.mil addr=? 
terminal=pts/2 res=failed'

Here we again get the wrong value for subj= in the USER_CHAUTHTOK audit record, 
and we further lose the USER_AVC record entirely because it didn't even reach 
the point of the permission check due to not being able to get the caller 
context.

The situation gets worse if the caller can set things up such that it can set 
an attribute value for one security module that is valid and privileged with 
respect to another security module.  This isn't a far-fetched scenario; 
AppArmor will default to running everything unconfined, so as soon as you 
enable it, any root process can potentially load a policy that defines contexts 
that look exactly like SELinux contexts. Smack is even simpler; you can set any 
arbitrary string you want as long as you are root (by default); no policy 
required.  So a root process that is confined by SELinux (or by AppAmor) can 
suddenly forge arbitrary contexts in audit records or reads of /proc/self/attr 
nodes or netlink messages or ..., just by virtue of applying these patches and 
enabling another security module like Smack. Or consider if ptags were ever 
made real and merged - by design, that's all about setting arbitrary tags from 
userspace.  Then there is the separate issue of switching
display to prevent attempts by a more privileged program to set one of its 
attributes from taking effect. Where have we seen that before - sendmail 
capabilities bug anyone?  And it is actually worse than that bug, because with 
the assistance of a friendly security module, the write may actually succeed; 
it just won't alter the SELinux context of the program or anything it creates!

This gets a NAK from me so long as it has these issues and setting the display 
remains outside the control of any security module.

The issues you've raised around audit are meritorious.
Any suggestions regarding how to address them would be
quite welcome.

As far as the general objection to the display mechanism,
I am eager to understand what you might propose as an
alternative. We can't dismiss backward compatibility for
any of the modules. We can't preclude any module combination.

We can require user space changes for configurations that
were impossible before, just as the addition of SELinux to
a system required user space changes. Update libselinux
to check the display before using the attr interfaces and
you've addressed most of the issues.

Either we ensure that setting of the display can only affect processes in the 
same security equivalence class (same credentials)

In the process of trying to argue against your point I
may have come around to your thinking. There would still
be the case where a privileged program sets the display
and invokes an equally privileged program which is "tricked"
into setting the wrong attribute, but you have to put the
responsibility for use of privilege on someone, somewhere.

I will propose a solution in the next round.

or the security modules need to be able to control who can set the display.

That's a mechanism for a module to opt-out of stacking,
and Paul has been pretty clear that he won't go for that.

It doesn't have to be used that way; it can just be used to limit the set of authorized processes that can set the display to e.g. trusted container runtimes.

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to