security_task_getsecid() and subjective vs objective task creds

[Steve Grubb]

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