On 12/20/2016 12:03 PM, Casey Schaufler wrote: > On 12/20/2016 11:35 AM, Stephen Smalley wrote: >> On Tue, 2016-12-20 at 11:07 -0800, Casey Schaufler wrote: >>> On 12/20/2016 10:28 AM, Stephen Smalley wrote: >>>> On Tue, 2016-12-20 at 10:17 -0800, Casey Schaufler wrote: >>>>> On 12/20/2016 8:50 AM, Stephen Smalley wrote: >>>>>> On Tue, 2016-12-20 at 17:39 +0100, José Bollo wrote: >>>>>>> Le mardi 20 décembre 2016 à 11:14 -0500, Stephen Smalley a >>>>>>> écrit >>>>>>> : >>>>>>>> >>>>>>>> Looking at your PTAGS implementation, I feel it is only >>>>>>>> fair to >>>>>>>> warn >>>>>>>> you that your usage of /proc/pid/attr is insecure, >>>>>>>> regardless >>>>>>>> of >>>>>>>> whether you use task security blobs or cred security blobs. >>>>>>> Fair?! >>>>>>> >>>>>>>> Getting the attributes of another process via /proc/pid >>>>>>>> files >>>>>>>> is >>>>>>>> inherently racy, as the process may exit and another >>>>>>>> process >>>>>>>> with >>>>>>>> different attributes may be created with the same pid (and >>>>>>>> no, >>>>>>>> this >>>>>>>> is not theoretical; it has been demonstrated). >>>>>>> I know. And I'm surprized that you dont do anything to change >>>>>>> that. >>>>>> There is a reason why SO_PEERSEC and SCM_SECURITY >>>>>> exist. Again, >>>>>> learn >>>>>> from the upstream security modules rather than re-inventing >>>>>> them, >>>>>> badly. >>>>> SO_PEERSEC and SCM_SECURITY are spiffy for processes that are >>>>> sending each other messages, but they identify the attributes >>>>> associated with the message, not the process. Neither SELinux >>>>> nor Smack get the information to send off of the process, it >>>>> comes from the socket structure. >>>> Yes, but in the SELinux case at least, the socket is labeled with >>>> the >>>> label of the creating process (except in the rare case of using >>>> setsockcreatecon(3), which can only be used by suitably authorized >>>> processes). >>> Yes, it's the same with Smack. When it's not the label >>> of the process it's the label the system wants the peer >>> to think it is. >>> >>>> So in general it serves quite well as a means of obtaining >>>> the peer label, which can then be used in access control (and this >>>> is >>>> in fact being used in a variety of applications in Linux and >>>> Android). >>> But only between processes that are in direct, explicit >>> communication. >>> There is no denying that these mechanisms work. They just aren't >>> applicable to Jose's use. >> If you say so (although it is unclear to me why or what exactly his use >> case is), but regardless, there is also no denying that getting and >> setting another process' attributes via /proc/pid files is inherently >> racy. > > You're right. How can we fix that? I have seen a gazillion cases > where system security would be much simpler and easier to enforce > and develop if we could (safely) ensure that the process under > /proc/pid wouldn't change on you without you knowing. > locking. In this case it would have to be user acquired file lock that would hold the entire set of pid files in place so you could do a read for some of its info and write to it. I haven't dug into it all so I have no idea how likely getting such a change in would be, but I have a feeling it will meet with a fair bit of resistance. The other way pid based interfaces have been handled is using a tuple of pid + creation timestamp or some other monotonically increasing value, that way you can be sure that the referenced pid is the correct one. I'm not a fan of the creation timestamp because time isn't always monotonically increasing but Jose could easily use a monotonic value. This would require Jose to track the monotonic value per task and make the expected monotonic value part of the write spec so that his module could validate it before applying the written tags. The monotonic value doesn't need to increase with each task, just each reuse, but the it would probably be easiest just to use a value that increases once per task created. I'm sure there are probably a few other techniques that could be found to make PTAGS safe. Whether /proc/pid/attr/ is the best interface is a separate question. >> He even acknowledged as much. So we are left with a "security" >> module whose only purpose is to support getting and setting process >> tags for security enforcement purposes, and yet does so in a known- >> insecure manner. Again, this is why I keep suggesting that he needs to >> reconsider his approach, not merely figure out how to implement per- >> task security blobs. > > Whether or not Jose moves forward with PTAGS we have identified > an issue with the current cred based hooks for AppArmor, TOMOYO > and at least one other proposed module. Regardless of the issues > of /proc/pid there is work to be done. > I can take a stab at it in a few weeks. While not critical for apparmor it would certainly cleanup apparmor's cred handling. _______________________________________________ Selinux mailing list Selinux@xxxxxxxxxxxxx To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
