security_task_getsecid() and subjective vs objective task creds

security_task_getsecid() and subjective vs objective task creds

[Paul Moore]

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 :)

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, 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?

-- 
paul moore
www.paul-moore.com

security_task_getsecid() and subjective vs objective task creds

[Paul Moore]

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 :)

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, 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?

-- 
paul moore
www.paul-moore.com

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

Re: security_task_getsecid() and subjective vs objective task creds

[Casey Schaufler]

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.

Re: security_task_getsecid() and subjective vs objective task creds

[Casey Schaufler]

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.



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

Re: security_task_getsecid() and subjective vs objective task creds

[Casey Schaufler]

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.

Re: security_task_getsecid() and subjective vs objective task creds

[Casey Schaufler]

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.

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

Re: 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

Re: 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


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

Re: security_task_getsecid() and subjective vs objective task creds

[Paul Moore]

On Thu, Feb 18, 2021 at 4:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 2/18/2021 11:34 AM, Paul Moore wrote:

...

> > 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.

As I've dug into this more, it does look like that is closest to being
correct, but there are still a few callers where it looks like the
objective creds are needed.  I think the correct thing to do is
convert the existing hook to use the subjective creds and add a
"_subj" at the end, while also creating a new
security_task_getsecid_obj() hook to return the objective cred and
updating those few callers that need it.

I'll see about making the associated changes to the Smack and AppArmor
code too, but that will obviously need some heavy review by you and
John.

--
paul moore
www.paul-moore.com

Re: security_task_getsecid() and subjective vs objective task creds

[Paul Moore]

On Thu, Feb 18, 2021 at 4:40 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 2/18/2021 11:34 AM, Paul Moore wrote:

...

> > 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.

As I've dug into this more, it does look like that is closest to being
correct, but there are still a few callers where it looks like the
objective creds are needed.  I think the correct thing to do is
convert the existing hook to use the subjective creds and add a
"_subj" at the end, while also creating a new
security_task_getsecid_obj() hook to return the objective cred and
updating those few callers that need it.

I'll see about making the associated changes to the Smack and AppArmor
code too, but that will obviously need some heavy review by you and
John.

--
paul moore
www.paul-moore.com

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

Re: security_task_getsecid() and subjective vs objective task creds

[James Morris]

On Thu, 18 Feb 2021, Paul Moore wrote:

> Hi all,
> 
> When looking into a problem I noticed that audit was recording the
> wrong subject label for a process.

Is this a public bug? It would be good to know what the extent of this 
issue may be and whether it warrants a CVE.

-- 
James Morris
<jmorris@namei.org>

Re: security_task_getsecid() and subjective vs objective task creds

[James Morris]

On Thu, 18 Feb 2021, Paul Moore wrote:

> Hi all,
> 
> When looking into a problem I noticed that audit was recording the
> wrong subject label for a process.

Is this a public bug? It would be good to know what the extent of this 
issue may be and whether it warrants a CVE.

-- 
James Morris
<jmorris@namei.org>

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

Re: security_task_getsecid() and subjective vs objective task creds

[Paul Moore]

On Fri, Feb 19, 2021 at 9:55 PM James Morris <jmorris@namei.org> wrote:
> On Thu, 18 Feb 2021, Paul Moore wrote:
>
> > Hi all,
> >
> > When looking into a problem I noticed that audit was recording the
> > wrong subject label for a process.
>
> Is this a public bug? It would be good to know what the extent of this
> issue may be and whether it warrants a CVE.

Let me rephrase, "When looking into a problem with some new patches
that I am working on I noticed ...".

I am not aware of any public bugs relating to this subj/obj confusion,
this was simply something I noticed while doing some new work.  I
would post those patches now, but they are still incomplete.
Regardless, this subj/obj confusion is something we should resolve.

-- 
paul moore
www.paul-moore.com

Re: security_task_getsecid() and subjective vs objective task creds

[Paul Moore]

On Fri, Feb 19, 2021 at 9:55 PM James Morris <jmorris@namei.org> wrote:
> On Thu, 18 Feb 2021, Paul Moore wrote:
>
> > Hi all,
> >
> > When looking into a problem I noticed that audit was recording the
> > wrong subject label for a process.
>
> Is this a public bug? It would be good to know what the extent of this
> issue may be and whether it warrants a CVE.

Let me rephrase, "When looking into a problem with some new patches
that I am working on I noticed ...".

I am not aware of any public bugs relating to this subj/obj confusion,
this was simply something I noticed while doing some new work.  I
would post those patches now, but they are still incomplete.
Regardless, this subj/obj confusion is something we should resolve.

-- 
paul moore
www.paul-moore.com

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