On Thursday, February 18, 2021 5:07:52 PM EST Casey Schaufler wrote: > On 2/18/2021 11:34 AM, Paul Moore wrote: > > Hi all, > > > > When looking into a problem I noticed that audit was recording the > > wrong subject label for a process. Doing a bit of digging I realized > > this was caused by the SELinux security_task_getsecid() implementation > > returning the objective security label (taken from task->real_cred), > > and not the subjective security label (taken from task->cred). > > > > Looking around at the other LSMs which implement this hook, Smack and > > AppArmor, it appears they both do the same thing: return the objective > > security ID for the process. Looking quickly at the various non-LSM > > callers of the security_task_getsecid() hook, it unfortunately looks > > like all of them expect the subjective security ID to be returned. > > The only caller I'm not 100% confident in is binder, but from what I > > can tell it looks like they are expecting the subjective ID too. > > > > At least we are consistently wrong :) > > We may have come down with a case of helperitis. > > > How do we want to fix this? The obvious fix is to change the SELinux, > > AppArmor, and Smack security_task_getsecid() implementations to return > > the subjective security ID (->cred), and likely make a note in > > lsm_hooks.h, > > That would be my choice. > > > but if someone can see a case where we would need both > > > > the subjective and objective security labels speak up and we can > > introduce a new hook for the subjective label, and likely add a "_obj" > > to the end of the existing hook to help make it more clear. If > > neither of those options are acceptable, we could convert all of the > > existing callers to use something like the line below (assumes > > current), but that is the least appealing option as far as I'm > > concerned. > > > > security_cred_getsecid(current_cred(), &sid); > > > > Opinions? > > If the objective cred isn't being used in the access control decision > it seems pointless to add it to the audit record. If there is a case > where the task is being treated as an object, signal delivery comes to > mind, you still want the objective credential. So it seems like care > may be required to ensure that the correct value (sub vs obj) is > used. Yes, ptrace, process_vm_readv, process_vm_writev, pidfd_open, process_madvise, pidfd_send_signal all seem to operate on a different process and might be candidates for an OBJ_PID record which is where it would get recorded. -Steve
