Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Wed, Mar 10, 2021 at 3:21 AM Jeffrey Vander Stoep wrote:
> On Fri, Mar 5, 2021 at 12:44 AM Paul Moore wrote:
> >
> > On Thu, Mar 4, 2021 at 5:04 AM Jeffrey Vander Stoep
> > wrote:
> > > On Sat, Feb 20, 2021 at 3:45 PM Paul Moore wrote:
> > > > On Fri, Feb 19, 2021 at 9:57 PM James Morris wrote:
> > > > > On Fri, 19 Feb 2021, Paul Moore wrote:
> > > > > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > > > > > index c119736ca56ac..39d501261108d 100644
> > > > > > --- a/drivers/android/binder.c
> > > > > > +++ b/drivers/android/binder.c
> > > > > > @@ -2700,7 +2700,7 @@ static void binder_transaction(struct
> > > > > > binder_proc *proc,
> > > > > > u32 secid;
> > > > > > size_t added_size;
> > > > > >
> > > > > > - security_task_getsecid(proc->tsk, );
> > > > > > + security_task_getsecid_subj(proc->tsk, );
> > > > > > ret = security_secid_to_secctx(secid, ,
> > > > > > _sz);
> > > > > > if (ret) {
> > > > > > return_error = BR_FAILED_REPLY;
> > > > >
> > > > > Can someone from the Android project confirm this is correct for
> > > > > binder?
> > >
> > > This looks correct to me.
> >
> > Thanks for the verification. Should I assume the SELinux specific
> > binder changes looked okay too?
> >
> Yes, those also look good to me.
Thanks, that binder changes were the one area I wasn't 100% on, I
appreciate the verification.
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Tue, Mar 9, 2021 at 8:03 PM John Johansen wrote: > On 2/19/21 3:29 PM, Paul Moore wrote: > > Of the three LSMs that implement the security_task_getsecid() LSM > > hook, all three LSMs provide the task's objective security > > credentials. This turns out to be unfortunate as most of the hook's > > callers seem to expect the task's subjective credentials, although > > a small handful of callers do correctly expect the objective > > credentials. > > > > This patch is the first step towards fixing the problem: it splits > > the existing security_task_getsecid() hook into two variants, one > > for the subjective creds, one for the objective creds. > > > > void security_task_getsecid_subj(struct task_struct *p, > > u32 *secid); > > void security_task_getsecid_obj(struct task_struct *p, > > u32 *secid); > > > > While this patch does fix all of the callers to use the correct > > variant, in order to keep this patch focused on the callers and to > > ease review, the LSMs continue to use the same implementation for > > both hooks. The net effect is that this patch should not change > > the behavior of the kernel in any way, it will be up to the latter > > LSM specific patches in this series to change the hook > > implementations and return the correct credentials. > > > > Signed-off-by: Paul Moore > > Reviewed-by: John Johansen Thanks John, I know you're swamped these days. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Fri, Mar 5, 2021 at 12:44 AM Paul Moore wrote:
>
> On Thu, Mar 4, 2021 at 5:04 AM Jeffrey Vander Stoep wrote:
> > On Sat, Feb 20, 2021 at 3:45 PM Paul Moore wrote:
> > > On Fri, Feb 19, 2021 at 9:57 PM James Morris wrote:
> > > > On Fri, 19 Feb 2021, Paul Moore wrote:
> > > > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > > > > index c119736ca56ac..39d501261108d 100644
> > > > > --- a/drivers/android/binder.c
> > > > > +++ b/drivers/android/binder.c
> > > > > @@ -2700,7 +2700,7 @@ static void binder_transaction(struct
> > > > > binder_proc *proc,
> > > > > u32 secid;
> > > > > size_t added_size;
> > > > >
> > > > > - security_task_getsecid(proc->tsk, );
> > > > > + security_task_getsecid_subj(proc->tsk, );
> > > > > ret = security_secid_to_secctx(secid, ,
> > > > > _sz);
> > > > > if (ret) {
> > > > > return_error = BR_FAILED_REPLY;
> > > >
> > > > Can someone from the Android project confirm this is correct for binder?
> >
> > This looks correct to me.
>
> Thanks for the verification. Should I assume the SELinux specific
> binder changes looked okay too?
>
Yes, those also look good to me.
> https://lore.kernel.org/selinux/84053ed8-4778-f246-2177-cf5c1b951...@canonical.com/T/#m4ae49d4a5a62d600fa3f3b1a5bba2d6611b1051c
>
> --
> paul moore
> www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On 2/19/21 3:29 PM, Paul Moore wrote:
> Of the three LSMs that implement the security_task_getsecid() LSM
> hook, all three LSMs provide the task's objective security
> credentials. This turns out to be unfortunate as most of the hook's
> callers seem to expect the task's subjective credentials, although
> a small handful of callers do correctly expect the objective
> credentials.
>
> This patch is the first step towards fixing the problem: it splits
> the existing security_task_getsecid() hook into two variants, one
> for the subjective creds, one for the objective creds.
>
> void security_task_getsecid_subj(struct task_struct *p,
> u32 *secid);
> void security_task_getsecid_obj(struct task_struct *p,
> u32 *secid);
>
> While this patch does fix all of the callers to use the correct
> variant, in order to keep this patch focused on the callers and to
> ease review, the LSMs continue to use the same implementation for
> both hooks. The net effect is that this patch should not change
> the behavior of the kernel in any way, it will be up to the latter
> LSM specific patches in this series to change the hook
> implementations and return the correct credentials.
>
> Signed-off-by: Paul Moore
Reviewed-by: John Johansen
> ---
> drivers/android/binder.c |2 +-
> include/linux/cred.h |2 +-
> include/linux/lsm_hook_defs.h |5 -
> include/linux/lsm_hooks.h |8 ++--
> include/linux/security.h | 10 --
> kernel/audit.c|4 ++--
> kernel/auditfilter.c |3 ++-
> kernel/auditsc.c |8
> net/netlabel/netlabel_unlabeled.c |2 +-
> net/netlabel/netlabel_user.h |2 +-
> security/apparmor/lsm.c |3 ++-
> security/integrity/ima/ima_appraise.c |2 +-
> security/integrity/ima/ima_main.c | 14 +++---
> security/security.c | 13 ++---
> security/selinux/hooks.c |3 ++-
> security/smack/smack_lsm.c|3 ++-
> 16 files changed, 54 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index c119736ca56ac..39d501261108d 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2700,7 +2700,7 @@ static void binder_transaction(struct binder_proc *proc,
> u32 secid;
> size_t added_size;
>
> - security_task_getsecid(proc->tsk, );
> + security_task_getsecid_subj(proc->tsk, );
> ret = security_secid_to_secctx(secid, , _sz);
> if (ret) {
> return_error = BR_FAILED_REPLY;
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 18639c069263f..42b9d88d9a565 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -140,7 +140,7 @@ struct cred {
> struct key *request_key_auth; /* assumed request_key authority */
> #endif
> #ifdef CONFIG_SECURITY
> - void*security; /* subjective LSM security */
> + void*security; /* LSM security */
> #endif
> struct user_struct *user; /* real user ID subscription */
> struct user_namespace *user_ns; /* user_ns the caps and keyrings are
> relative to. */
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index dfd261dcbcb04..1490a185135a0 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -200,7 +200,10 @@ LSM_HOOK(int, 0, task_fix_setgid, struct cred *new,
> const struct cred * old,
> LSM_HOOK(int, 0, task_setpgid, struct task_struct *p, pid_t pgid)
> LSM_HOOK(int, 0, task_getpgid, struct task_struct *p)
> LSM_HOOK(int, 0, task_getsid, struct task_struct *p)
> -LSM_HOOK(void, LSM_RET_VOID, task_getsecid, struct task_struct *p, u32
> *secid)
> +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_subj,
> + struct task_struct *p, u32 *secid)
> +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_obj,
> + struct task_struct *p, u32 *secid)
> LSM_HOOK(int, 0, task_setnice, struct task_struct *p, int nice)
> LSM_HOOK(int, 0, task_setioprio, struct task_struct *p, int ioprio)
> LSM_HOOK(int, 0, task_getioprio, struct task_struct *p)
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index bdfc8a76a4f79..13d2a9a6f2014 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -706,8 +706,12 @@
> * @p.
> * @p contains the task_struct for the process.
> * Return 0 if permission is granted.
> - * @task_getsecid:
> - * Retrieve the security identifier of the process @p.
> + * @task_getsecid_subj:
> + * Retrieve the subjective security identifier of the process @p.
> + * @p contains the task_struct for the process and place is into @secid.
> + * In case of
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On 3/9/21 4:28 PM, Paul Moore wrote: > On Wed, Mar 3, 2021 at 7:44 PM Paul Moore wrote: >> On Sun, Feb 21, 2021 at 7:51 AM John Johansen >> wrote: >>> On 2/19/21 3:29 PM, Paul Moore wrote: Of the three LSMs that implement the security_task_getsecid() LSM hook, all three LSMs provide the task's objective security credentials. This turns out to be unfortunate as most of the hook's callers seem to expect the task's subjective credentials, although a small handful of callers do correctly expect the objective credentials. This patch is the first step towards fixing the problem: it splits the existing security_task_getsecid() hook into two variants, one for the subjective creds, one for the objective creds. void security_task_getsecid_subj(struct task_struct *p, u32 *secid); void security_task_getsecid_obj(struct task_struct *p, u32 *secid); While this patch does fix all of the callers to use the correct variant, in order to keep this patch focused on the callers and to ease review, the LSMs continue to use the same implementation for both hooks. The net effect is that this patch should not change the behavior of the kernel in any way, it will be up to the latter LSM specific patches in this series to change the hook implementations and return the correct credentials. Signed-off-by: Paul Moore >>> >>> So far this looks good. I want to take another stab at it and give >>> it some testing >> >> Checking in as I know you said you needed to fix/release the AppArmor >> patch in this series ... is this patch still looking okay to you? If >> so, can I get an ACK at least on this patch? > > Hi John, > > Any objections if I merge the LSM, SELinux, and Smack patches into the > selinux/next tree so that we can start getting some wider testing? If > I leave out my poor attempt at an AppArmor patch, the current in-tree > AppArmor code should behave exactly as it does today with the > apparmor_task_getsecid() function handling both the subjective and > objective creds. I can always merge the AppArmor patch later when you > have it ready, or you can merge it via your AppArmor tree at a later > date. > I have some questions around selinux and binder but I don't have any objections to you merging, we can always drop fixes on top if they are necessary -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Wed, Mar 3, 2021 at 7:44 PM Paul Moore wrote: > On Sun, Feb 21, 2021 at 7:51 AM John Johansen > wrote: > > On 2/19/21 3:29 PM, Paul Moore wrote: > > > Of the three LSMs that implement the security_task_getsecid() LSM > > > hook, all three LSMs provide the task's objective security > > > credentials. This turns out to be unfortunate as most of the hook's > > > callers seem to expect the task's subjective credentials, although > > > a small handful of callers do correctly expect the objective > > > credentials. > > > > > > This patch is the first step towards fixing the problem: it splits > > > the existing security_task_getsecid() hook into two variants, one > > > for the subjective creds, one for the objective creds. > > > > > > void security_task_getsecid_subj(struct task_struct *p, > > > u32 *secid); > > > void security_task_getsecid_obj(struct task_struct *p, > > > u32 *secid); > > > > > > While this patch does fix all of the callers to use the correct > > > variant, in order to keep this patch focused on the callers and to > > > ease review, the LSMs continue to use the same implementation for > > > both hooks. The net effect is that this patch should not change > > > the behavior of the kernel in any way, it will be up to the latter > > > LSM specific patches in this series to change the hook > > > implementations and return the correct credentials. > > > > > > Signed-off-by: Paul Moore > > > > So far this looks good. I want to take another stab at it and give > > it some testing > > Checking in as I know you said you needed to fix/release the AppArmor > patch in this series ... is this patch still looking okay to you? If > so, can I get an ACK at least on this patch? Hi John, Any objections if I merge the LSM, SELinux, and Smack patches into the selinux/next tree so that we can start getting some wider testing? If I leave out my poor attempt at an AppArmor patch, the current in-tree AppArmor code should behave exactly as it does today with the apparmor_task_getsecid() function handling both the subjective and objective creds. I can always merge the AppArmor patch later when you have it ready, or you can merge it via your AppArmor tree at a later date. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Mon, Mar 8, 2021 at 2:25 PM Richard Guy Briggs wrote: > On 2021-02-19 18:29, Paul Moore wrote: > > Of the three LSMs that implement the security_task_getsecid() LSM > > hook, all three LSMs provide the task's objective security > > credentials. This turns out to be unfortunate as most of the hook's > > callers seem to expect the task's subjective credentials, although > > a small handful of callers do correctly expect the objective > > credentials. > > > > This patch is the first step towards fixing the problem: it splits > > the existing security_task_getsecid() hook into two variants, one > > for the subjective creds, one for the objective creds. > > > > void security_task_getsecid_subj(struct task_struct *p, > > u32 *secid); > > void security_task_getsecid_obj(struct task_struct *p, > > u32 *secid); > > > > While this patch does fix all of the callers to use the correct > > variant, in order to keep this patch focused on the callers and to > > ease review, the LSMs continue to use the same implementation for > > both hooks. The net effect is that this patch should not change > > the behavior of the kernel in any way, it will be up to the latter > > LSM specific patches in this series to change the hook > > implementations and return the correct credentials. > > > > Signed-off-by: Paul Moore > > Audit: Acked-by: Richard Guy Briggs > Reviewed-by: Richard Guy Briggs Thanks Richard, I added your review tag to the LSM, SELinux, and Smack patches. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On 2021-02-19 18:29, Paul Moore wrote:
> Of the three LSMs that implement the security_task_getsecid() LSM
> hook, all three LSMs provide the task's objective security
> credentials. This turns out to be unfortunate as most of the hook's
> callers seem to expect the task's subjective credentials, although
> a small handful of callers do correctly expect the objective
> credentials.
>
> This patch is the first step towards fixing the problem: it splits
> the existing security_task_getsecid() hook into two variants, one
> for the subjective creds, one for the objective creds.
>
> void security_task_getsecid_subj(struct task_struct *p,
> u32 *secid);
> void security_task_getsecid_obj(struct task_struct *p,
> u32 *secid);
>
> While this patch does fix all of the callers to use the correct
> variant, in order to keep this patch focused on the callers and to
> ease review, the LSMs continue to use the same implementation for
> both hooks. The net effect is that this patch should not change
> the behavior of the kernel in any way, it will be up to the latter
> LSM specific patches in this series to change the hook
> implementations and return the correct credentials.
>
> Signed-off-by: Paul Moore
Audit: Acked-by: Richard Guy Briggs
Reviewed-by: Richard Guy Briggs
> ---
> drivers/android/binder.c |2 +-
> include/linux/cred.h |2 +-
> include/linux/lsm_hook_defs.h |5 -
> include/linux/lsm_hooks.h |8 ++--
> include/linux/security.h | 10 --
> kernel/audit.c|4 ++--
> kernel/auditfilter.c |3 ++-
> kernel/auditsc.c |8
> net/netlabel/netlabel_unlabeled.c |2 +-
> net/netlabel/netlabel_user.h |2 +-
> security/apparmor/lsm.c |3 ++-
> security/integrity/ima/ima_appraise.c |2 +-
> security/integrity/ima/ima_main.c | 14 +++---
> security/security.c | 13 ++---
> security/selinux/hooks.c |3 ++-
> security/smack/smack_lsm.c|3 ++-
> 16 files changed, 54 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index c119736ca56ac..39d501261108d 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2700,7 +2700,7 @@ static void binder_transaction(struct binder_proc *proc,
> u32 secid;
> size_t added_size;
>
> - security_task_getsecid(proc->tsk, );
> + security_task_getsecid_subj(proc->tsk, );
> ret = security_secid_to_secctx(secid, , _sz);
> if (ret) {
> return_error = BR_FAILED_REPLY;
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 18639c069263f..42b9d88d9a565 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -140,7 +140,7 @@ struct cred {
> struct key *request_key_auth; /* assumed request_key authority */
> #endif
> #ifdef CONFIG_SECURITY
> - void*security; /* subjective LSM security */
> + void*security; /* LSM security */
> #endif
> struct user_struct *user; /* real user ID subscription */
> struct user_namespace *user_ns; /* user_ns the caps and keyrings are
> relative to. */
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index dfd261dcbcb04..1490a185135a0 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -200,7 +200,10 @@ LSM_HOOK(int, 0, task_fix_setgid, struct cred *new,
> const struct cred * old,
> LSM_HOOK(int, 0, task_setpgid, struct task_struct *p, pid_t pgid)
> LSM_HOOK(int, 0, task_getpgid, struct task_struct *p)
> LSM_HOOK(int, 0, task_getsid, struct task_struct *p)
> -LSM_HOOK(void, LSM_RET_VOID, task_getsecid, struct task_struct *p, u32
> *secid)
> +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_subj,
> + struct task_struct *p, u32 *secid)
> +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_obj,
> + struct task_struct *p, u32 *secid)
> LSM_HOOK(int, 0, task_setnice, struct task_struct *p, int nice)
> LSM_HOOK(int, 0, task_setioprio, struct task_struct *p, int ioprio)
> LSM_HOOK(int, 0, task_getioprio, struct task_struct *p)
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index bdfc8a76a4f79..13d2a9a6f2014 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -706,8 +706,12 @@
> * @p.
> * @p contains the task_struct for the process.
> * Return 0 if permission is granted.
> - * @task_getsecid:
> - * Retrieve the security identifier of the process @p.
> + * @task_getsecid_subj:
> + * Retrieve the subjective security identifier of the process @p.
> + * @p contains the task_struct for the process and place
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Thu, Mar 4, 2021 at 5:04 AM Jeffrey Vander Stoep wrote:
> On Sat, Feb 20, 2021 at 3:45 PM Paul Moore wrote:
> > On Fri, Feb 19, 2021 at 9:57 PM James Morris wrote:
> > > On Fri, 19 Feb 2021, Paul Moore wrote:
> > > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > > > index c119736ca56ac..39d501261108d 100644
> > > > --- a/drivers/android/binder.c
> > > > +++ b/drivers/android/binder.c
> > > > @@ -2700,7 +2700,7 @@ static void binder_transaction(struct binder_proc
> > > > *proc,
> > > > u32 secid;
> > > > size_t added_size;
> > > >
> > > > - security_task_getsecid(proc->tsk, );
> > > > + security_task_getsecid_subj(proc->tsk, );
> > > > ret = security_secid_to_secctx(secid, ,
> > > > _sz);
> > > > if (ret) {
> > > > return_error = BR_FAILED_REPLY;
> > >
> > > Can someone from the Android project confirm this is correct for binder?
>
> This looks correct to me.
Thanks for the verification. Should I assume the SELinux specific
binder changes looked okay too?
https://lore.kernel.org/selinux/84053ed8-4778-f246-2177-cf5c1b951...@canonical.com/T/#m4ae49d4a5a62d600fa3f3b1a5bba2d6611b1051c
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Sat, Feb 20, 2021 at 3:45 PM Paul Moore wrote:
>
> On Fri, Feb 19, 2021 at 9:57 PM James Morris wrote:
> > On Fri, 19 Feb 2021, Paul Moore wrote:
> > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > > index c119736ca56ac..39d501261108d 100644
> > > --- a/drivers/android/binder.c
> > > +++ b/drivers/android/binder.c
> > > @@ -2700,7 +2700,7 @@ static void binder_transaction(struct binder_proc
> > > *proc,
> > > u32 secid;
> > > size_t added_size;
> > >
> > > - security_task_getsecid(proc->tsk, );
> > > + security_task_getsecid_subj(proc->tsk, );
> > > ret = security_secid_to_secctx(secid, , _sz);
> > > if (ret) {
> > > return_error = BR_FAILED_REPLY;
> >
> > Can someone from the Android project confirm this is correct for binder?
This looks correct to me.
>
> Yes, please take a look Android folks. As I mentioned previously,
> review of the binder changes is one area where I think some extra
> review is needed; I'm just not confident enough in my understanding of
> binder.
>
> --
> paul moore
> www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Sun, Feb 21, 2021 at 7:51 AM John Johansen wrote: > On 2/19/21 3:29 PM, Paul Moore wrote: > > Of the three LSMs that implement the security_task_getsecid() LSM > > hook, all three LSMs provide the task's objective security > > credentials. This turns out to be unfortunate as most of the hook's > > callers seem to expect the task's subjective credentials, although > > a small handful of callers do correctly expect the objective > > credentials. > > > > This patch is the first step towards fixing the problem: it splits > > the existing security_task_getsecid() hook into two variants, one > > for the subjective creds, one for the objective creds. > > > > void security_task_getsecid_subj(struct task_struct *p, > > u32 *secid); > > void security_task_getsecid_obj(struct task_struct *p, > > u32 *secid); > > > > While this patch does fix all of the callers to use the correct > > variant, in order to keep this patch focused on the callers and to > > ease review, the LSMs continue to use the same implementation for > > both hooks. The net effect is that this patch should not change > > the behavior of the kernel in any way, it will be up to the latter > > LSM specific patches in this series to change the hook > > implementations and return the correct credentials. > > > > Signed-off-by: Paul Moore > > So far this looks good. I want to take another stab at it and give > it some testing Checking in as I know you said you needed to fix/release the AppArmor patch in this series ... is this patch still looking okay to you? If so, can I get an ACK at least on this patch? -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Fri, 2021-02-19 at 18:29 -0500, Paul Moore wrote: > Of the three LSMs that implement the security_task_getsecid() LSM > hook, all three LSMs provide the task's objective security > credentials. This turns out to be unfortunate as most of the hook's > callers seem to expect the task's subjective credentials, although > a small handful of callers do correctly expect the objective > credentials. > > This patch is the first step towards fixing the problem: it splits > the existing security_task_getsecid() hook into two variants, one > for the subjective creds, one for the objective creds. > > void security_task_getsecid_subj(struct task_struct *p, > u32 *secid); > void security_task_getsecid_obj(struct task_struct *p, > u32 *secid); > > While this patch does fix all of the callers to use the correct > variant, in order to keep this patch focused on the callers and to > ease review, the LSMs continue to use the same implementation for > both hooks. The net effect is that this patch should not change > the behavior of the kernel in any way, it will be up to the latter > LSM specific patches in this series to change the hook > implementations and return the correct credentials. > > Signed-off-by: Paul Moore Thanks, Paul. Acked-by: Mimi Zohar (IMA) -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On 2/19/21 3:29 PM, Paul Moore wrote:
> Of the three LSMs that implement the security_task_getsecid() LSM
> hook, all three LSMs provide the task's objective security
> credentials. This turns out to be unfortunate as most of the hook's
> callers seem to expect the task's subjective credentials, although
> a small handful of callers do correctly expect the objective
> credentials.
>
> This patch is the first step towards fixing the problem: it splits
> the existing security_task_getsecid() hook into two variants, one
> for the subjective creds, one for the objective creds.
>
> void security_task_getsecid_subj(struct task_struct *p,
> u32 *secid);
> void security_task_getsecid_obj(struct task_struct *p,
> u32 *secid);
>
> While this patch does fix all of the callers to use the correct
> variant, in order to keep this patch focused on the callers and to
> ease review, the LSMs continue to use the same implementation for
> both hooks. The net effect is that this patch should not change
> the behavior of the kernel in any way, it will be up to the latter
> LSM specific patches in this series to change the hook
> implementations and return the correct credentials.
>
> Signed-off-by: Paul Moore
So far this looks good. I want to take another stab at it and give
it some testing
> ---
> drivers/android/binder.c |2 +-
> include/linux/cred.h |2 +-
> include/linux/lsm_hook_defs.h |5 -
> include/linux/lsm_hooks.h |8 ++--
> include/linux/security.h | 10 --
> kernel/audit.c|4 ++--
> kernel/auditfilter.c |3 ++-
> kernel/auditsc.c |8
> net/netlabel/netlabel_unlabeled.c |2 +-
> net/netlabel/netlabel_user.h |2 +-
> security/apparmor/lsm.c |3 ++-
> security/integrity/ima/ima_appraise.c |2 +-
> security/integrity/ima/ima_main.c | 14 +++---
> security/security.c | 13 ++---
> security/selinux/hooks.c |3 ++-
> security/smack/smack_lsm.c|3 ++-
> 16 files changed, 54 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index c119736ca56ac..39d501261108d 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2700,7 +2700,7 @@ static void binder_transaction(struct binder_proc *proc,
> u32 secid;
> size_t added_size;
>
> - security_task_getsecid(proc->tsk, );
> + security_task_getsecid_subj(proc->tsk, );
> ret = security_secid_to_secctx(secid, , _sz);
> if (ret) {
> return_error = BR_FAILED_REPLY;
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 18639c069263f..42b9d88d9a565 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -140,7 +140,7 @@ struct cred {
> struct key *request_key_auth; /* assumed request_key authority */
> #endif
> #ifdef CONFIG_SECURITY
> - void*security; /* subjective LSM security */
> + void*security; /* LSM security */
> #endif
> struct user_struct *user; /* real user ID subscription */
> struct user_namespace *user_ns; /* user_ns the caps and keyrings are
> relative to. */
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index dfd261dcbcb04..1490a185135a0 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -200,7 +200,10 @@ LSM_HOOK(int, 0, task_fix_setgid, struct cred *new,
> const struct cred * old,
> LSM_HOOK(int, 0, task_setpgid, struct task_struct *p, pid_t pgid)
> LSM_HOOK(int, 0, task_getpgid, struct task_struct *p)
> LSM_HOOK(int, 0, task_getsid, struct task_struct *p)
> -LSM_HOOK(void, LSM_RET_VOID, task_getsecid, struct task_struct *p, u32
> *secid)
> +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_subj,
> + struct task_struct *p, u32 *secid)
> +LSM_HOOK(void, LSM_RET_VOID, task_getsecid_obj,
> + struct task_struct *p, u32 *secid)
> LSM_HOOK(int, 0, task_setnice, struct task_struct *p, int nice)
> LSM_HOOK(int, 0, task_setioprio, struct task_struct *p, int ioprio)
> LSM_HOOK(int, 0, task_getioprio, struct task_struct *p)
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index bdfc8a76a4f79..13d2a9a6f2014 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -706,8 +706,12 @@
> * @p.
> * @p contains the task_struct for the process.
> * Return 0 if permission is granted.
> - * @task_getsecid:
> - * Retrieve the security identifier of the process @p.
> + * @task_getsecid_subj:
> + * Retrieve the subjective security identifier of the process @p.
> + * @p contains the task_struct for the
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Sun, Feb 21, 2021 at 7:51 AM John Johansen wrote: > On 2/19/21 3:29 PM, Paul Moore wrote: > > Of the three LSMs that implement the security_task_getsecid() LSM > > hook, all three LSMs provide the task's objective security > > credentials. This turns out to be unfortunate as most of the hook's > > callers seem to expect the task's subjective credentials, although > > a small handful of callers do correctly expect the objective > > credentials. > > > > This patch is the first step towards fixing the problem: it splits > > the existing security_task_getsecid() hook into two variants, one > > for the subjective creds, one for the objective creds. > > > > void security_task_getsecid_subj(struct task_struct *p, > > u32 *secid); > > void security_task_getsecid_obj(struct task_struct *p, > > u32 *secid); > > > > While this patch does fix all of the callers to use the correct > > variant, in order to keep this patch focused on the callers and to > > ease review, the LSMs continue to use the same implementation for > > both hooks. The net effect is that this patch should not change > > the behavior of the kernel in any way, it will be up to the latter > > LSM specific patches in this series to change the hook > > implementations and return the correct credentials. > > > > Signed-off-by: Paul Moore > > So far this looks good. I want to take another stab at it and give > it some testing Thanks John, I appreciate the extra set of eyes. Let me know if you run across anything wonky. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Fri, Feb 19, 2021 at 9:57 PM James Morris wrote:
> On Fri, 19 Feb 2021, Paul Moore wrote:
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > index c119736ca56ac..39d501261108d 100644
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -2700,7 +2700,7 @@ static void binder_transaction(struct binder_proc
> > *proc,
> > u32 secid;
> > size_t added_size;
> >
> > - security_task_getsecid(proc->tsk, );
> > + security_task_getsecid_subj(proc->tsk, );
> > ret = security_secid_to_secctx(secid, , _sz);
> > if (ret) {
> > return_error = BR_FAILED_REPLY;
>
> Can someone from the Android project confirm this is correct for binder?
Yes, please take a look Android folks. As I mentioned previously,
review of the binder changes is one area where I think some extra
review is needed; I'm just not confident enough in my understanding of
binder.
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [RFC PATCH 1/4] lsm: separate security_task_getsecid() into subjective and objective variants
On Fri, 19 Feb 2021, Paul Moore wrote:
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index c119736ca56ac..39d501261108d 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2700,7 +2700,7 @@ static void binder_transaction(struct binder_proc *proc,
> u32 secid;
> size_t added_size;
>
> - security_task_getsecid(proc->tsk, );
> + security_task_getsecid_subj(proc->tsk, );
> ret = security_secid_to_secctx(secid, , _sz);
> if (ret) {
> return_error = BR_FAILED_REPLY;
Can someone from the Android project confirm this is correct for binder?
--
James Morris
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
