Re: security_task_getsecid() and subjective vs objective task creds

From: Steve Grubb <sgrubb@redhat.com>
To: Paul Moore <paul@paul-moore.com>,
	linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
	linux-audit@redhat.com
Subject: Re: security_task_getsecid() and subjective vs objective task creds
Date: Thu, 18 Feb 2021 17:50:51 -0500	[thread overview]
Message-ID: <806848326.0ifERbkFSE@x2> (raw)
In-Reply-To: <5f8a424a-2aab-f11a-0e06-f8f075c644f6@schaufler-ca.com>

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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit