* Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
From: Stephen Smalley @ 2020-01-10 15:13 UTC (permalink / raw)
To: Huaisheng Ye, paul, eparis, jmorris, serge
Cc: tyu1, linux-security-module, selinux, linux-kernel, Huaisheng Ye
In-Reply-To: <20200110095856.76612-1-yehs2007@zoho.com>
On 1/10/20 4:58 AM, Huaisheng Ye wrote:
> From: Huaisheng Ye <yehs1@lenovo.com>
>
> selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
> do nothing else. And also msg_msg_alloc_security is just used by the
> former.
>
> Remove the redundant function to simplify the code.
This seems to also be true of other _alloc_security functions, probably
due to historical reasons. Further, at least some of these functions no
longer perform any allocation; they are just initialization functions
now that allocation has been taken to the LSM framework, so possibly
could be renamed and made to return void at some point.
>
> Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> security/selinux/hooks.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9625b99..fb1b9da 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5882,16 +5882,6 @@ static void ipc_init_security(struct ipc_security_struct *isec, u16 sclass)
> isec->sid = current_sid();
> }
>
> -static int msg_msg_alloc_security(struct msg_msg *msg)
> -{
> - struct msg_security_struct *msec;
> -
> - msec = selinux_msg_msg(msg);
> - msec->sid = SECINITSID_UNLABELED;
> -
> - return 0;
> -}
> -
> static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
> u32 perms)
> {
> @@ -5910,7 +5900,12 @@ static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
>
> static int selinux_msg_msg_alloc_security(struct msg_msg *msg)
> {
> - return msg_msg_alloc_security(msg);
> + struct msg_security_struct *msec;
> +
> + msec = selinux_msg_msg(msg);
> + msec->sid = SECINITSID_UNLABELED;
> +
> + return 0;
> }
>
> /* message queue security operations */
>
^ permalink raw reply
* Re: [PATCH bpf-next v1 00/13] MAC and Audit policy using eBPF (KRSI)
From: KP Singh @ 2020-01-10 15:27 UTC (permalink / raw)
To: Stephen Smalley
Cc: KP Singh, James Morris, Kees Cook, Casey Schaufler, open list,
bpf, linux-security-module, Alexei Starovoitov, Daniel Borkmann,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Brendan Jackman,
Martin KaFai Lau, Song Liu, Yonghong Song, Serge E. Hallyn,
Mauro Carvalho Chehab, David S. Miller, Greg Kroah-Hartman,
Nicolas Ferre, Stanislav Fomichev, Quentin Monnet, Andrey Ignatov,
Joe Stringer, Paul Moore
In-Reply-To: <8e035f4d-5120-de6a-7ac8-a35841a92b8a@tycho.nsa.gov>
On 09-Jan 14:47, Stephen Smalley wrote:
> On 1/9/20 2:43 PM, KP Singh wrote:
> > On 10-Jan 06:07, James Morris wrote:
> > > On Thu, 9 Jan 2020, Stephen Smalley wrote:
> > >
> > > > On 1/9/20 1:11 PM, James Morris wrote:
> > > > > On Wed, 8 Jan 2020, Stephen Smalley wrote:
> > > > >
> > > > > > The cover letter subject line and the Kconfig help text refer to it as a
> > > > > > BPF-based "MAC and Audit policy". It has an enforce config option that
> > > > > > enables the bpf programs to deny access, providing access control. IIRC,
> > > > > > in
> > > > > > the earlier discussion threads, the BPF maintainers suggested that Smack
> > > > > > and
> > > > > > other LSMs could be entirely re-implemented via it in the future, and that
> > > > > > such an implementation would be more optimal.
> > > > >
> > > > > In this case, the eBPF code is similar to a kernel module, rather than a
> > > > > loadable policy file. It's a loadable mechanism, rather than a policy, in
> > > > > my view.
> > > >
> > > > I thought you frowned on dynamically loadable LSMs for both security and
> > > > correctness reasons?
> >
> > Based on the feedback from the lists we've updated the design for v2.
> >
> > In v2, LSM hook callbacks are allocated dynamically using BPF
> > trampolines, appended to a separate security_hook_heads and run
> > only after the statically allocated hooks.
> >
> > The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
> > still remains __lsm_ro_after_init and cannot be modified. We are still
> > working on v2 (not ready for review yet) but the general idea can be
> > seen here:
> >
> > https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c
> >
> > >
> > > Evaluating the security impact of this is the next step. My understanding
> > > is that eBPF via BTF is constrained to read only access to hook
> > > parameters, and that its behavior would be entirely restrictive.
> > >
> > > I'd like to understand the security impact more fully, though. Can the
> > > eBPF code make arbitrary writes to the kernel, or read anything other than
> > > the correctly bounded LSM hook parameters?
> > >
> >
> > As mentioned, the BPF verifier does not allow writes to BTF types.
> >
> > > > And a traditional security module would necessarily fall
> > > > under GPL; is the eBPF code required to be likewise? If not, KRSI is a
> > > > gateway for proprietary LSMs...
> > >
> > > Right, we do not want this to be a GPL bypass.
> >
> > This is not intended to be a GPL bypass and the BPF verifier checks
> > for license compatibility of the loaded program with GPL.
>
> IIUC, it checks that the program is GPL compatible if it uses a function
> marked GPL-only. But what specifically is marked GPL-only that is required
> for eBPF programs using KRSI?
Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
specific verification for the v2 of the patch-set which would require
all BPF-LSM programs to be GPL.
- KP
>
> >
> > - KP
> >
> > >
> > > If these issues can be resolved, this may be a "safe" way to support
> > > loadable LSM applications.
> > >
> > > Again, I'd be interested in knowing how this is is handled in the
> > > networking stack (keeping in mind that LSM is a much more invasive API,
> > > and may not be directly comparable).
> > >
> > > --
> > > James Morris
> > > <jmorris@namei.org>
> > >
>
^ permalink raw reply
* Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
From: Paul Moore @ 2020-01-10 16:43 UTC (permalink / raw)
To: Huaisheng Ye
Cc: Stephen Smalley, Eric Paris, James Morris, Serge Hallyn, tyu1,
linux-security-module, selinux, linux-kernel, Huaisheng Ye
In-Reply-To: <20200110095856.76612-1-yehs2007@zoho.com>
On Fri, Jan 10, 2020 at 4:59 AM Huaisheng Ye <yehs2007@zoho.com> wrote:
> From: Huaisheng Ye <yehs1@lenovo.com>
>
> selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
> do nothing else. And also msg_msg_alloc_security is just used by the
> former.
>
> Remove the redundant function to simplify the code.
>
> Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
> ---
> security/selinux/hooks.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
Merged into selinux/next, thanks!
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
From: Casey Schaufler @ 2020-01-10 16:44 UTC (permalink / raw)
To: Stephen Smalley, Huaisheng Ye, paul, eparis, jmorris, serge
Cc: tyu1, linux-security-module, selinux, linux-kernel, Huaisheng Ye,
Casey Schaufler
In-Reply-To: <e71932ce-0687-02e5-5f34-980c0cad4ae9@tycho.nsa.gov>
On 1/10/2020 7:13 AM, Stephen Smalley wrote:
> On 1/10/20 4:58 AM, Huaisheng Ye wrote:
>> From: Huaisheng Ye <yehs1@lenovo.com>
>>
>> selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
>> do nothing else. And also msg_msg_alloc_security is just used by the
>> former.
>>
>> Remove the redundant function to simplify the code.
>
> This seems to also be true of other _alloc_security functions, probably due to historical reasons. Further, at least some of these functions no longer perform any allocation; they are just initialization functions now that allocation has been taken to the LSM framework, so possibly could be renamed and made to return void at some point.
That's something I've been eyeing. I'm not 100% sure that no module will
ever fail doing the new style initialization. The int to void and name
change will probably happen after the next round of modules come in and
we can see that failure of initialization isn't a rational situation.
>
>>
>> Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
>
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
>
>> ---
>> security/selinux/hooks.c | 17 ++++++-----------
>> 1 file changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 9625b99..fb1b9da 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -5882,16 +5882,6 @@ static void ipc_init_security(struct ipc_security_struct *isec, u16 sclass)
>> isec->sid = current_sid();
>> }
>> -static int msg_msg_alloc_security(struct msg_msg *msg)
>> -{
>> - struct msg_security_struct *msec;
>> -
>> - msec = selinux_msg_msg(msg);
>> - msec->sid = SECINITSID_UNLABELED;
>> -
>> - return 0;
>> -}
>> -
>> static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
>> u32 perms)
>> {
>> @@ -5910,7 +5900,12 @@ static int ipc_has_perm(struct kern_ipc_perm *ipc_perms,
>> static int selinux_msg_msg_alloc_security(struct msg_msg *msg)
>> {
>> - return msg_msg_alloc_security(msg);
>> + struct msg_security_struct *msec;
>> +
>> + msec = selinux_msg_msg(msg);
>> + msec->sid = SECINITSID_UNLABELED;
>> +
>> + return 0;
>> }
>> /* message queue security operations */
>>
>
>
^ permalink raw reply
* Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
From: Huaisheng HS1 Ye @ 2020-01-10 16:46 UTC (permalink / raw)
To: Stephen Smalley, paul@paul-moore.com, eparis@parisplace.org,
jmorris@namei.org, serge@hallyn.com
Cc: Tzu ting Yu1, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, linux-kernel@vger.kernel.org,
Huaisheng Ye
> -----Original Message-----
> From: Stephen Smalley <sds@tycho.nsa.gov>
> Sent: Friday, January 10, 2020 11:14 PM
>
> On 1/10/20 4:58 AM, Huaisheng Ye wrote:
> > From: Huaisheng Ye <yehs1@lenovo.com>
> >
> > selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
> > do nothing else. And also msg_msg_alloc_security is just used by the
> > former.
> >
> > Remove the redundant function to simplify the code.
>
> This seems to also be true of other _alloc_security functions, probably due to
> historical reasons. Further, at least some of these functions no longer perform
> any allocation; they are just initialization functions now that allocation has
> been taken to the LSM framework, so possibly could be renamed and made to return
> void at some point.
>
> >
> > Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
>
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Many thanks for the Acked-by.
Yes, you are right, selinux_msg_msg_alloc_security only can return 0 in any case.
I think that should be modified to void instead of int.
And also, the fact is no module needs to implement msg_msg_free_security, because
LSM would take the responsibility for freeing msg->security.
I think we could delete the hook call of msg_msg_free_security, but I am cautious
to modify the existing interfaces, that just worry to break traditional rules.
Cheers,
Huaisheng Ye
Lenovo
^ permalink raw reply
* Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
From: Paul Moore @ 2020-01-10 16:50 UTC (permalink / raw)
To: Stephen Smalley
Cc: Huaisheng Ye, Eric Paris, James Morris, Serge Hallyn, tyu1,
linux-security-module, selinux, linux-kernel, Huaisheng Ye
In-Reply-To: <e71932ce-0687-02e5-5f34-980c0cad4ae9@tycho.nsa.gov>
On Fri, Jan 10, 2020 at 10:13 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On 1/10/20 4:58 AM, Huaisheng Ye wrote:
> > From: Huaisheng Ye <yehs1@lenovo.com>
> >
> > selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
> > do nothing else. And also msg_msg_alloc_security is just used by the
> > former.
> >
> > Remove the redundant function to simplify the code.
>
> This seems to also be true of other _alloc_security functions, probably
> due to historical reasons. Further, at least some of these functions no
> longer perform any allocation; they are just initialization functions
> now that allocation has been taken to the LSM framework, so possibly
> could be renamed and made to return void at some point.
I've noticed the same thing on a few occasions, I've just never
bothered to put the fixes into a patch. We might as well do that now,
at least for the redundant code bits; I'll leave the return code issue
for another time as that would cross LSM boundaries and that really
isn't appropriate in the -rc5 timeframe IMHO.
I'll put something together once I finish up the patch/review backlog
from the past few days. Looking quickly with a regex, it would appear
that inode_alloc_security(), file_alloc_security(), and
superblock_alloc_security() are all candidates. While not an
allocator, we can probably get rid of inode_doinit() as well.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH bpf-next v1 00/13] MAC and Audit policy using eBPF (KRSI)
From: James Morris @ 2020-01-10 17:48 UTC (permalink / raw)
To: KP Singh
Cc: Stephen Smalley, Kees Cook, Casey Schaufler, open list, bpf,
linux-security-module, Alexei Starovoitov, Daniel Borkmann,
Thomas Garnier, Michael Halcrow, Paul Turner, Brendan Gregg,
Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Brendan Jackman,
Martin KaFai Lau, Song Liu, Yonghong Song, Serge E. Hallyn,
Mauro Carvalho Chehab, David S. Miller, Greg Kroah-Hartman,
Nicolas Ferre, Stanislav Fomichev, Quentin Monnet, Andrey Ignatov,
Joe Stringer, Paul Moore
In-Reply-To: <20200110152758.GA260168@google.com>
On Fri, 10 Jan 2020, KP Singh wrote:
>
> Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
> specific verification for the v2 of the patch-set which would require
> all BPF-LSM programs to be GPL.
Sounds good to me.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH bpf-next v1 00/13] MAC and Audit policy using eBPF (KRSI)
From: Alexei Starovoitov @ 2020-01-10 17:53 UTC (permalink / raw)
To: KP Singh
Cc: Stephen Smalley, James Morris, Kees Cook, Casey Schaufler,
open list, bpf, linux-security-module, Alexei Starovoitov,
Daniel Borkmann, Thomas Garnier, Michael Halcrow, Paul Turner,
Brendan Gregg, Jann Horn, Matthew Garrett, Christian Brauner,
Mickaël Salaün, Florent Revest, Brendan Jackman,
Martin KaFai Lau, Song Liu, Yonghong Song, Serge E. Hallyn,
Mauro Carvalho Chehab, David S. Miller, Greg Kroah-Hartman,
Nicolas Ferre, Stanislav Fomichev, Quentin Monnet, Andrey Ignatov,
Joe Stringer, Paul Moore
In-Reply-To: <20200110152758.GA260168@google.com>
On Fri, Jan 10, 2020 at 04:27:58PM +0100, KP Singh wrote:
> On 09-Jan 14:47, Stephen Smalley wrote:
> > On 1/9/20 2:43 PM, KP Singh wrote:
> > > On 10-Jan 06:07, James Morris wrote:
> > > > On Thu, 9 Jan 2020, Stephen Smalley wrote:
> > > >
> > > > > On 1/9/20 1:11 PM, James Morris wrote:
> > > > > > On Wed, 8 Jan 2020, Stephen Smalley wrote:
> > > > > >
> > > > > > > The cover letter subject line and the Kconfig help text refer to it as a
> > > > > > > BPF-based "MAC and Audit policy". It has an enforce config option that
> > > > > > > enables the bpf programs to deny access, providing access control. IIRC,
> > > > > > > in
> > > > > > > the earlier discussion threads, the BPF maintainers suggested that Smack
> > > > > > > and
> > > > > > > other LSMs could be entirely re-implemented via it in the future, and that
> > > > > > > such an implementation would be more optimal.
> > > > > >
> > > > > > In this case, the eBPF code is similar to a kernel module, rather than a
> > > > > > loadable policy file. It's a loadable mechanism, rather than a policy, in
> > > > > > my view.
> > > > >
> > > > > I thought you frowned on dynamically loadable LSMs for both security and
> > > > > correctness reasons?
> > >
> > > Based on the feedback from the lists we've updated the design for v2.
> > >
> > > In v2, LSM hook callbacks are allocated dynamically using BPF
> > > trampolines, appended to a separate security_hook_heads and run
> > > only after the statically allocated hooks.
> > >
> > > The security_hook_heads for all the other LSMs (SELinux, AppArmor etc)
> > > still remains __lsm_ro_after_init and cannot be modified. We are still
> > > working on v2 (not ready for review yet) but the general idea can be
> > > seen here:
> > >
> > > https://github.com/sinkap/linux-krsi/blob/patch/v1/trampoline_prototype/security/bpf/lsm.c
> > >
> > > >
> > > > Evaluating the security impact of this is the next step. My understanding
> > > > is that eBPF via BTF is constrained to read only access to hook
> > > > parameters, and that its behavior would be entirely restrictive.
> > > >
> > > > I'd like to understand the security impact more fully, though. Can the
> > > > eBPF code make arbitrary writes to the kernel, or read anything other than
> > > > the correctly bounded LSM hook parameters?
> > > >
> > >
> > > As mentioned, the BPF verifier does not allow writes to BTF types.
> > >
> > > > > And a traditional security module would necessarily fall
> > > > > under GPL; is the eBPF code required to be likewise? If not, KRSI is a
> > > > > gateway for proprietary LSMs...
> > > >
> > > > Right, we do not want this to be a GPL bypass.
> > >
> > > This is not intended to be a GPL bypass and the BPF verifier checks
> > > for license compatibility of the loaded program with GPL.
> >
> > IIUC, it checks that the program is GPL compatible if it uses a function
> > marked GPL-only. But what specifically is marked GPL-only that is required
> > for eBPF programs using KRSI?
>
> Good point! If no-one objects, I can add it to the BPF_PROG_TYPE_LSM
> specific verification for the v2 of the patch-set which would require
> all BPF-LSM programs to be GPL.
I don't think it's a good idea to enforce license on the program.
The kernel doesn't do it for modules.
For years all of BPF tracing progs were GPL because they have to use
GPL-ed helpers to do anything meaningful.
So for KRSI just make sure that all helpers are GPL-ed as well.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: Make trampolines W^X
From: Andy Lutomirski @ 2020-01-10 18:35 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: luto@kernel.org, songliubraving@fb.com,
linux-kernel@vger.kernel.org, daniel@iogearbox.net,
peterz@infradead.org, keescook@chromium.org, bpf@vger.kernel.org,
jeyu@kernel.org, kuznet@ms2.inr.ac.ru, mjg59@google.com,
nadav.amit@gmail.com, ast@kernel.org, thgarnie@chromium.org,
kpsingh@chromium.org, linux-security-module@vger.kernel.org,
x86@kernel.org, revest@chromium.org, jannh@google.com,
namit@vmware.com, jackmanb@chromium.org, Hansen, Dave,
kafai@fb.com, yhs@fb.com, davem@davemloft.net,
yoshfuji@linux-ipv6.org, mhalcrow@google.com, andriin@fb.com
In-Reply-To: <96dd98b3c4f73b205b6e669ca87fa64901c117d6.camel@intel.com>
> On Jan 9, 2020, at 3:01 PM, Edgecombe, Rick P <rick.p.edgecombe@intel.com> wrote:
>> The vmap code immediately removes PTEs when unmaps occur (which it may
>> very well do right now -- I haven't checked) but also tracks the
>> kernel_tlb_gen associated with each record of an
>> unmapped-but-not-zapped area. Then we split vm_unmap_aliases() into a
>> variant that unmaps all aliases and a variant that merely promises to
>> unmap at least one alias. The former does what the current code does
>> except that it skips the IPI if all areas in question have tlb_gen <
>> flushed_kernel_tlb_gen. The latter clears all areas with tlb_gen <
>> flushed_kernel_tlb_gen and, if there weren't any, does
>> flush_tlb_kernel_range() and flushes everything.
>>
>> (Major caveat: this is wrong for the case where
>> flush_tlb_kernel_range() only flushes some but not all of the kernel.
>> So this needs considerable work if it's actually going to me useful.
>> The plain old "take locks and clean up" approach might be a better
>> bet.)
>>
>
> Hmm. In normal usage (!DEBUG_PAGE_ALLOC), are kernel range tlb shootdowns common
> outside of module space users and lazy vmap stuff? A tlb_gen solution might only
> be worth it in cases where something other than vm_unmap_aliases() and helpers
> was doing this frequently.
I suspect that the two bug users aside from vunmap() will be eBPF and,
eventually, XPFO / “exclusive pages” / less crappy SEV-like
implementations / actual high quality MKTME stuff / KVM
side-channel-proof memory. The latter doesn’t actually exist yet (the
SEV implementation sidesteps this with a horrible hack involving
incoherent mappings that are left active with fingers crossed), but it
really seems like it’s coming.
In general, if we’re going to have a pool of non-RW-direct-mapped
pages, we also want some moderately efficient way to produce such
pages.
Right now, creating and freeing eBPF programs in a loop is probably a
performance disaster on large systems.
^ permalink raw reply
* Re: [PATCH v13 26/25] Audit: Multiple LSM support in audit rules
From: Casey Schaufler @ 2020-01-10 19:40 UTC (permalink / raw)
To: Mimi Zohar, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul, sds,
linux-audit@redhat.com, linux-integrity, Casey Schaufler
In-Reply-To: <1578587607.5147.63.camel@linux.ibm.com>
On 1/9/2020 8:33 AM, Mimi Zohar wrote:
> Hi Casey,
>
> On Fri, 2020-01-03 at 10:53 -0800, Casey Schaufler wrote:
>> With multiple possible security modules supporting audit rule
>> it is necessary to keep separate data for each module in the
>> audit rules. This affects IMA as well, as it re-uses the audit
>> rule list mechanisms.
> While reviewing this patch, I realized there was a bug in the base IMA
> code. With Janne's bug fix, that he just posted, I think this patch
> can now be simplified.
How and when do you plan to get Janne's fix in? It's looking like
stacking won't be in for 5.6.
> My main concern is the number of warning messages that will be
> generated. Any time a new LSM policy is loaded, the labels will be
> re-evaulated whether or not they are applicable to the particular LSM,
> causing unnecessary warnings.
Uhg.
>
> Mimi
>
^ permalink raw reply
* Re: [PATCH 1/2] selinux: treat atomic flags more carefully
From: Paul Moore @ 2020-01-10 20:21 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: linux-security-module, James Morris, Serge E. Hallyn,
Casey Schaufler, selinux, Stephen Smalley, John Johansen,
Kees Cook, Micah Morton, Tetsuo Handa
In-Reply-To: <20200107133154.588958-2-omosnace@redhat.com>
On Tue, Jan 7, 2020 at 8:32 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> The disabled/enforcing/initialized flags are all accessed concurrently
> by threads so use the appropriate accessors that ensure atomicity and
> document that it is expected.
>
> Use smp_load/acquire...() helpers (with memory barriers) for the
> initialized flag, since it gates access to the rest of the state
> structures.
>
> Note that the disabled flag is currently not used for anything other
> than avoiding double disable, but it will be used for bailing out of
> hooks once security_delete_hooks() is removed.
>
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
> security/selinux/hooks.c | 21 ++++++++--------
> security/selinux/include/security.h | 33 +++++++++++++++++++++++--
> security/selinux/ss/services.c | 38 ++++++++++++++---------------
> 3 files changed, 61 insertions(+), 31 deletions(-)
Merged into selinux/next, thanks Ondrej!
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH 1/2] selinux: treat atomic flags more carefully
From: Paul Moore @ 2020-01-10 20:22 UTC (permalink / raw)
To: James Morris
Cc: Ondrej Mosnacek, linux-security-module, Serge E. Hallyn,
Casey Schaufler, selinux, Stephen Smalley, John Johansen,
Kees Cook, Micah Morton, Tetsuo Handa
In-Reply-To: <alpine.LRH.2.21.2001080644500.575@namei.org>
On Tue, Jan 7, 2020 at 2:46 PM James Morris <jmorris@namei.org> wrote:
> On Tue, 7 Jan 2020, Ondrej Mosnacek wrote:
>
> > The disabled/enforcing/initialized flags are all accessed concurrently
> > by threads so use the appropriate accessors that ensure atomicity and
> > document that it is expected.
> >
> > Use smp_load/acquire...() helpers (with memory barriers) for the
> > initialized flag, since it gates access to the rest of the state
> > structures.
> >
> > Note that the disabled flag is currently not used for anything other
> > than avoiding double disable, but it will be used for bailing out of
> > hooks once security_delete_hooks() is removed.
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
>
>
> Reviewed-by: James Morris <jamorris@linux.microsoft.com>
You get an extra helping of gratitude James for being the only one to
properly trim your reply ;)
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH kunit] kunit: building kunit as a module breaks allmodconfig
From: Shuah Khan @ 2020-01-10 21:39 UTC (permalink / raw)
To: Alan Maguire, brendanhiggins, gregkh
Cc: rafael, jmorris, serge, knut.omang, linux-kernel,
linux-security-module, kunit-dev, linux-kselftest, sfr,
Shuah Khan
In-Reply-To: <1578656965-2993-1-git-send-email-alan.maguire@oracle.com>
Hi Alan,
On 1/10/20 4:49 AM, Alan Maguire wrote:
> kunit tests that do not support module build should depend
> on KUNIT=y rather than just KUNIT in Kconfig, otherwise
> they will trigger compilation errors for "make allmodconfig"
> builds.
>
> Fixes: 9fe124bf1b77 ("kunit: allow kunit to be loaded as a module")
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Thanks for fixing this quickly. For future reference, Signed-off-by
should be last. I fixed it and applied the patch.
thanks,
-- Shuah
^ permalink raw reply
* [PATCH AUTOSEL 5.4 03/26] tomoyo: Suppress RCU warning at list_for_each_entry_rcu().
From: Sasha Levin @ 2020-01-10 22:02 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Tetsuo Handa, John Garry, Sasha Levin, linux-security-module
In-Reply-To: <20200110220308.27784-1-sashal@kernel.org>
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
[ Upstream commit 6bd5ce6089b561f5392460bfb654dea89356ab1b ]
John Garry has reported that allmodconfig kernel on arm64 causes flood of
"RCU-list traversed in non-reader section!!" warning. I don't know what
change caused this warning, but this warning is safe because TOMOYO uses
SRCU lock instead. Let's suppress this warning by explicitly telling that
the caller is holding SRCU lock.
Reported-and-tested-by: John Garry <john.garry@huawei.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
security/tomoyo/common.c | 9 ++++++---
security/tomoyo/domain.c | 15 ++++++++++-----
security/tomoyo/group.c | 9 ++++++---
security/tomoyo/util.c | 6 ++++--
4 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
index dd3d5942e669..c36bafbcd77e 100644
--- a/security/tomoyo/common.c
+++ b/security/tomoyo/common.c
@@ -951,7 +951,8 @@ static bool tomoyo_manager(void)
exe = tomoyo_get_exe();
if (!exe)
return false;
- list_for_each_entry_rcu(ptr, &tomoyo_kernel_namespace.policy_list[TOMOYO_ID_MANAGER], head.list) {
+ list_for_each_entry_rcu(ptr, &tomoyo_kernel_namespace.policy_list[TOMOYO_ID_MANAGER], head.list,
+ srcu_read_lock_held(&tomoyo_ss)) {
if (!ptr->head.is_deleted &&
(!tomoyo_pathcmp(domainname, ptr->manager) ||
!strcmp(exe, ptr->manager->name))) {
@@ -1095,7 +1096,8 @@ static int tomoyo_delete_domain(char *domainname)
if (mutex_lock_interruptible(&tomoyo_policy_lock))
return -EINTR;
/* Is there an active domain? */
- list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list,
+ srcu_read_lock_held(&tomoyo_ss)) {
/* Never delete tomoyo_kernel_domain */
if (domain == &tomoyo_kernel_domain)
continue;
@@ -2778,7 +2780,8 @@ void tomoyo_check_profile(void)
tomoyo_policy_loaded = true;
pr_info("TOMOYO: 2.6.0\n");
- list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list,
+ srcu_read_lock_held(&tomoyo_ss)) {
const u8 profile = domain->profile;
struct tomoyo_policy_namespace *ns = domain->ns;
diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c
index 8526a0a74023..7869d6a9980b 100644
--- a/security/tomoyo/domain.c
+++ b/security/tomoyo/domain.c
@@ -41,7 +41,8 @@ int tomoyo_update_policy(struct tomoyo_acl_head *new_entry, const int size,
if (mutex_lock_interruptible(&tomoyo_policy_lock))
return -ENOMEM;
- list_for_each_entry_rcu(entry, list, list) {
+ list_for_each_entry_rcu(entry, list, list,
+ srcu_read_lock_held(&tomoyo_ss)) {
if (entry->is_deleted == TOMOYO_GC_IN_PROGRESS)
continue;
if (!check_duplicate(entry, new_entry))
@@ -119,7 +120,8 @@ int tomoyo_update_domain(struct tomoyo_acl_info *new_entry, const int size,
}
if (mutex_lock_interruptible(&tomoyo_policy_lock))
goto out;
- list_for_each_entry_rcu(entry, list, list) {
+ list_for_each_entry_rcu(entry, list, list,
+ srcu_read_lock_held(&tomoyo_ss)) {
if (entry->is_deleted == TOMOYO_GC_IN_PROGRESS)
continue;
if (!tomoyo_same_acl_head(entry, new_entry) ||
@@ -166,7 +168,8 @@ void tomoyo_check_acl(struct tomoyo_request_info *r,
u16 i = 0;
retry:
- list_for_each_entry_rcu(ptr, list, list) {
+ list_for_each_entry_rcu(ptr, list, list,
+ srcu_read_lock_held(&tomoyo_ss)) {
if (ptr->is_deleted || ptr->type != r->param_type)
continue;
if (!check_entry(r, ptr))
@@ -298,7 +301,8 @@ static inline bool tomoyo_scan_transition
{
const struct tomoyo_transition_control *ptr;
- list_for_each_entry_rcu(ptr, list, head.list) {
+ list_for_each_entry_rcu(ptr, list, head.list,
+ srcu_read_lock_held(&tomoyo_ss)) {
if (ptr->head.is_deleted || ptr->type != type)
continue;
if (ptr->domainname) {
@@ -735,7 +739,8 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm)
/* Check 'aggregator' directive. */
candidate = &exename;
- list_for_each_entry_rcu(ptr, list, head.list) {
+ list_for_each_entry_rcu(ptr, list, head.list,
+ srcu_read_lock_held(&tomoyo_ss)) {
if (ptr->head.is_deleted ||
!tomoyo_path_matches_pattern(&exename,
ptr->original_name))
diff --git a/security/tomoyo/group.c b/security/tomoyo/group.c
index a37c7dc66e44..1cecdd797597 100644
--- a/security/tomoyo/group.c
+++ b/security/tomoyo/group.c
@@ -133,7 +133,8 @@ tomoyo_path_matches_group(const struct tomoyo_path_info *pathname,
{
struct tomoyo_path_group *member;
- list_for_each_entry_rcu(member, &group->member_list, head.list) {
+ list_for_each_entry_rcu(member, &group->member_list, head.list,
+ srcu_read_lock_held(&tomoyo_ss)) {
if (member->head.is_deleted)
continue;
if (!tomoyo_path_matches_pattern(pathname, member->member_name))
@@ -161,7 +162,8 @@ bool tomoyo_number_matches_group(const unsigned long min,
struct tomoyo_number_group *member;
bool matched = false;
- list_for_each_entry_rcu(member, &group->member_list, head.list) {
+ list_for_each_entry_rcu(member, &group->member_list, head.list,
+ srcu_read_lock_held(&tomoyo_ss)) {
if (member->head.is_deleted)
continue;
if (min > member->number.values[1] ||
@@ -191,7 +193,8 @@ bool tomoyo_address_matches_group(const bool is_ipv6, const __be32 *address,
bool matched = false;
const u8 size = is_ipv6 ? 16 : 4;
- list_for_each_entry_rcu(member, &group->member_list, head.list) {
+ list_for_each_entry_rcu(member, &group->member_list, head.list,
+ srcu_read_lock_held(&tomoyo_ss)) {
if (member->head.is_deleted)
continue;
if (member->address.is_ipv6 != is_ipv6)
diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c
index 52752e1a84ed..eba0b3395851 100644
--- a/security/tomoyo/util.c
+++ b/security/tomoyo/util.c
@@ -594,7 +594,8 @@ struct tomoyo_domain_info *tomoyo_find_domain(const char *domainname)
name.name = domainname;
tomoyo_fill_path_info(&name);
- list_for_each_entry_rcu(domain, &tomoyo_domain_list, list) {
+ list_for_each_entry_rcu(domain, &tomoyo_domain_list, list,
+ srcu_read_lock_held(&tomoyo_ss)) {
if (!domain->is_deleted &&
!tomoyo_pathcmp(&name, domain->domainname))
return domain;
@@ -1028,7 +1029,8 @@ bool tomoyo_domain_quota_is_ok(struct tomoyo_request_info *r)
return false;
if (!domain)
return true;
- list_for_each_entry_rcu(ptr, &domain->acl_info_list, list) {
+ list_for_each_entry_rcu(ptr, &domain->acl_info_list, list,
+ srcu_read_lock_held(&tomoyo_ss)) {
u16 perm;
u8 i;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v13 26/25] Audit: Multiple LSM support in audit rules
From: Mimi Zohar @ 2020-01-12 15:37 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul, sds,
linux-audit@redhat.com, linux-integrity
In-Reply-To: <e6945c33-a540-9d0a-ba71-3602b8e38154@schaufler-ca.com>
On Fri, 2020-01-10 at 11:40 -0800, Casey Schaufler wrote:
> On 1/9/2020 8:33 AM, Mimi Zohar wrote:
> > Hi Casey,
> >
> > On Fri, 2020-01-03 at 10:53 -0800, Casey Schaufler wrote:
> >> With multiple possible security modules supporting audit rule
> >> it is necessary to keep separate data for each module in the
> >> audit rules. This affects IMA as well, as it re-uses the audit
> >> rule list mechanisms.
> > While reviewing this patch, I realized there was a bug in the base IMA
> > code. With Janne's bug fix, that he just posted, I think this patch
> > can now be simplified.
>
> How and when do you plan to get Janne's fix in? It's looking like
> stacking won't be in for 5.6.
The patch is now in the next-integrity-testing branch. We'll see how
it goes.
>
> > My main concern is the number of warning messages that will be
> > generated. Any time a new LSM policy is loaded, the labels will be
> > re-evaulated whether or not they are applicable to the particular LSM,
> > causing unnecessary warnings.
>
> Uhg.
^ permalink raw reply
* [PATCH] selinux: remove redundant selinux_nlmsg_perm
From: Huaisheng Ye @ 2020-01-12 15:42 UTC (permalink / raw)
To: paul, sds, eparis, jmorris, serge
Cc: tyu1, linux-security-module, selinux, linux-kernel, Huaisheng Ye
From: Huaisheng Ye <yehs1@lenovo.com>
selinux_nlmsg_perm is used for only by selinux_netlink_send. Remove
the redundant function to simplify the code.
Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
---
security/selinux/hooks.c | 73 ++++++++++++++++++++++--------------------------
1 file changed, 34 insertions(+), 39 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index fb1b9da..9f3f966 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5507,44 +5507,6 @@ static int selinux_tun_dev_open(void *security)
return 0;
}
-static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
-{
- int err = 0;
- u32 perm;
- struct nlmsghdr *nlh;
- struct sk_security_struct *sksec = sk->sk_security;
-
- if (skb->len < NLMSG_HDRLEN) {
- err = -EINVAL;
- goto out;
- }
- nlh = nlmsg_hdr(skb);
-
- err = selinux_nlmsg_lookup(sksec->sclass, nlh->nlmsg_type, &perm);
- if (err) {
- if (err == -EINVAL) {
- pr_warn_ratelimited("SELinux: unrecognized netlink"
- " message: protocol=%hu nlmsg_type=%hu sclass=%s"
- " pig=%d comm=%s\n",
- sk->sk_protocol, nlh->nlmsg_type,
- secclass_map[sksec->sclass - 1].name,
- task_pid_nr(current), current->comm);
- if (!enforcing_enabled(&selinux_state) ||
- security_get_allow_unknown(&selinux_state))
- err = 0;
- }
-
- /* Ignore */
- if (err == -ENOENT)
- err = 0;
- goto out;
- }
-
- err = sock_has_perm(sk, perm);
-out:
- return err;
-}
-
#ifdef CONFIG_NETFILTER
static unsigned int selinux_ip_forward(struct sk_buff *skb,
@@ -5873,7 +5835,40 @@ static unsigned int selinux_ipv6_postroute(void *priv,
static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb)
{
- return selinux_nlmsg_perm(sk, skb);
+ int err = 0;
+ u32 perm;
+ struct nlmsghdr *nlh;
+ struct sk_security_struct *sksec = sk->sk_security;
+
+ if (skb->len < NLMSG_HDRLEN) {
+ err = -EINVAL;
+ goto out;
+ }
+ nlh = nlmsg_hdr(skb);
+
+ err = selinux_nlmsg_lookup(sksec->sclass, nlh->nlmsg_type, &perm);
+ if (err) {
+ if (err == -EINVAL) {
+ pr_warn_ratelimited("SELinux: unrecognized netlink"
+ " message: protocol=%hu nlmsg_type=%hu sclass=%s"
+ " pig=%d comm=%s\n",
+ sk->sk_protocol, nlh->nlmsg_type,
+ secclass_map[sksec->sclass - 1].name,
+ task_pid_nr(current), current->comm);
+ if (!enforcing_enabled(&selinux_state) ||
+ security_get_allow_unknown(&selinux_state))
+ err = 0;
+ }
+
+ /* Ignore */
+ if (err == -ENOENT)
+ err = 0;
+ goto out;
+ }
+
+ err = sock_has_perm(sk, perm);
+out:
+ return err;
}
static void ipc_init_security(struct ipc_security_struct *isec, u16 sclass)
--
1.8.3.1
^ permalink raw reply related
* RE: [External] Re: [PATCH] selinux: remove redundant msg_msg_alloc_security
From: Huaisheng HS1 Ye @ 2020-01-12 15:45 UTC (permalink / raw)
To: Paul Moore, Stephen Smalley
Cc: Huaisheng Ye, Eric Paris, James Morris, Serge Hallyn,
Tzu ting Yu1, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAHC9VhT-8R4iT-V-A+6NvZgG=bh4Knieif2fuKwybnDuXvC6ug@mail.gmail.com>
> -----Original Message-----
> From: Paul Moore <paul@paul-moore.com>
> Sent: Saturday, January 11, 2020 12:50 AM
> On Fri, Jan 10, 2020 at 10:13 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > On 1/10/20 4:58 AM, Huaisheng Ye wrote:
> > > From: Huaisheng Ye <yehs1@lenovo.com>
> > >
> > > selinux_msg_msg_alloc_security only calls msg_msg_alloc_security but
> > > do nothing else. And also msg_msg_alloc_security is just used by the
> > > former.
> > >
> > > Remove the redundant function to simplify the code.
> >
> > This seems to also be true of other _alloc_security functions,
> > probably due to historical reasons. Further, at least some of these
> > functions no longer perform any allocation; they are just
> > initialization functions now that allocation has been taken to the LSM
> > framework, so possibly could be renamed and made to return void at some point.
>
> I've noticed the same thing on a few occasions, I've just never bothered to put
> the fixes into a patch. We might as well do that now, at least for the redundant
> code bits; I'll leave the return code issue for another time as that would cross
> LSM boundaries and that really isn't appropriate in the -rc5 timeframe IMHO.
>
> I'll put something together once I finish up the patch/review backlog from the
> past few days. Looking quickly with a regex, it would appear that
> inode_alloc_security(), file_alloc_security(), and
> superblock_alloc_security() are all candidates. While not an allocator, we can
> probably get rid of inode_doinit() as well.
Besides, it looks like selinux_nlmsg_perm is candidate too.
Just send a patch for this function.
Cheers,
Huaisheng Ye
^ permalink raw reply
* Re: [PATCH v2] ima: add the ability to query the hash of a given file.
From: Florent Revest @ 2020-01-13 9:42 UTC (permalink / raw)
To: Mimi Zohar, linux-integrity
Cc: kpsingh, mjg59, nramas, linux-kernel, linux-security-module,
Florent Revest
In-Reply-To: <1578499556.5222.157.camel@linux.ibm.com>
On Wed, 2020-01-08 at 11:05 -0500, Mimi Zohar wrote:
> On Mon, 2020-01-06 at 17:25 +0100, Florent Revest wrote:
> > From: Florent Revest <revest@google.com>
> >
> > This allows other parts of the kernel (perhaps a stacked LSM
> > allowing
> > system monitoring, eg. the proposed KRSI LSM [1]) to retrieve the
> > hash
> > of a given file from IMA if it's present in the iint cache.
> >
> > It's true that the existence of the hash means that it's also in
> > the
> > audit logs or in
> > /sys/kernel/security/ima/ascii_runtime_measurements,
> > but it can be difficult to pull that information out for every
> > subsequent exec. This is especially true if a given host has been
> > up
> > for a long time and the file was first measured a long time ago.
> >
> > This is based on Peter Moody's patch:
> > https://sourceforge.net/p/linux-ima/mailman/message/33036180/
>
> FYI, but unlike the audit log/IMA measurement list, the iint cache
> entries can be removed. Refer to security_inode_free(). Perhaps
> mention of this difference should be included, here, in the patch
> description.
Sure, I added a comment about this in a v3.
> > [1] https://lkml.org/lkml/2019/9/10/393
> >
> > Signed-off-by: Florent Revest <revest@google.com>
>
> Assuming, with the above difference, you're still interested in
> having this feature upstreamed and addressing the comments above and
> below:
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Thank you. Yes we are still interested in this feature!
> > ---
> > include/linux/ima.h | 6 ++++
> > security/integrity/ima/ima_main.c | 46
> > +++++++++++++++++++++++++++++++
> > 2 files changed, 52 insertions(+)
> >
> > diff --git a/include/linux/ima.h b/include/linux/ima.h
> > index 6d904754d858..d621c65ba9a5 100644
> > --- a/include/linux/ima.h
> > +++ b/include/linux/ima.h
> > @@ -23,6 +23,7 @@ extern int ima_read_file(struct file *file, enum
> > kernel_read_file_id id);
> > extern int ima_post_read_file(struct file *file, void *buf, loff_t
> > size,
> > enum kernel_read_file_id id);
> > extern void ima_post_path_mknod(struct dentry *dentry);
> > +extern int ima_file_hash(struct file *file, char *buf, size_t
> > buf_size);
> > extern void ima_kexec_cmdline(const void *buf, int size);
> >
> > #ifdef CONFIG_IMA_KEXEC
> > @@ -91,6 +92,11 @@ static inline void ima_post_path_mknod(struct
> > dentry *dentry)
> > return;
> > }
> >
> > +static inline int ima_file_hash(struct file *file, char *buf,
> > size_t buf_size)
> > +{
> > + return -EOPNOTSUPP;
> > +}
> > +
> > static inline void ima_kexec_cmdline(const void *buf, int size) {}
> > #endif /* CONFIG_IMA */
> >
> > diff --git a/security/integrity/ima/ima_main.c
> > b/security/integrity/ima/ima_main.c
> > index d7e987baf127..3799b6c6c3b8 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -445,6 +445,52 @@ int ima_file_check(struct file *file, int
> > mask)
> > }
> > EXPORT_SYMBOL_GPL(ima_file_check);
> >
> > +/**
> > + * ima_file_hash - return the stored measurement if a file has
> > been hashed.
> > + * @file: pointer to the file
> > + * @buf: buffer in which to store the hash
> > + * @buf_size: length of the buffer
> > + *
> > + * On success, return the hash algorithm (as defined in the enum
> > hash_algo).
> > + * If buf is not NULL, this function also outputs the hash into
> > buf.
>
> As of Linux 5.4.y, IMA support for appended file signatures was
> added. Should we indicate that the file hash returned is based on the
> entire file, including the appended signature?
>
> Mimi
Of course it never hurts to add a comment. :) I'll send a v3 with this
added.
>
> > + * If the hash is larger than buf_size, then only buf_size bytes
> > will be copied.
> > + * It generally just makes sense to pass a buffer capable of
> > holding the largest
> > + * possible hash: IMA_MAX_DIGEST_SIZE
> > + *
> > + * If IMA is disabled or if no measurement is available, return
> > -EOPNOTSUPP.
> > + * If the parameters are incorrect, return -EINVAL.
> > + */
> > +int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> > +{
> > + struct inode *inode;
> > + struct integrity_iint_cache *iint;
> > + int hash_algo;
> > +
> > + if (!file)
> > + return -EINVAL;
> > +
> > + if (!ima_policy_flag)
> > + return -EOPNOTSUPP;
> > +
> > + inode = file_inode(file);
> > + iint = integrity_iint_find(inode);
> > + if (!iint)
> > + return -EOPNOTSUPP;
> > +
> > + mutex_lock(&iint->mutex);
> > + if (buf) {
> > + size_t copied_size;
> > +
> > + copied_size = min_t(size_t, iint->ima_hash->length,
> > buf_size);
> > + memcpy(buf, iint->ima_hash->digest, copied_size);
> > + }
> > + hash_algo = iint->ima_hash->algo;
> > + mutex_unlock(&iint->mutex);
> > +
> > + return hash_algo;
> > +}
> > +EXPORT_SYMBOL_GPL(ima_file_hash);
> > +
> > /**
> > * ima_post_create_tmpfile - mark newly created tmpfile as new
> > * @file : newly created tmpfile
^ permalink raw reply
* [PATCH v3] ima: add the ability to query the cached hash of a given file
From: Florent Revest @ 2020-01-13 9:42 UTC (permalink / raw)
To: linux-integrity
Cc: kpsingh, mjg59, zohar, nramas, linux-kernel,
linux-security-module, Florent Revest
From: Florent Revest <revest@google.com>
This allows other parts of the kernel (perhaps a stacked LSM allowing
system monitoring, eg. the proposed KRSI LSM [1]) to retrieve the hash
of a given file from IMA if it's present in the iint cache.
It's true that the existence of the hash means that it's also in the
audit logs or in /sys/kernel/security/ima/ascii_runtime_measurements,
but it can be difficult to pull that information out for every
subsequent exec. This is especially true if a given host has been up
for a long time and the file was first measured a long time ago.
It should be kept in mind that this function gives access to cached
entries which can be removed, for instance on security_inode_free().
This is based on Peter Moody's patch:
https://sourceforge.net/p/linux-ima/mailman/message/33036180/
[1] https://lkml.org/lkml/2019/9/10/393
Signed-off-by: Florent Revest <revest@google.com>
---
include/linux/ima.h | 6 ++++
security/integrity/ima/ima_main.c | 49 +++++++++++++++++++++++++++++++
2 files changed, 55 insertions(+)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index f4644c54f648..1659217e9b60 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -23,6 +23,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
enum kernel_read_file_id id);
extern void ima_post_path_mknod(struct dentry *dentry);
+extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
extern void ima_kexec_cmdline(const void *buf, int size);
#ifdef CONFIG_IMA_KEXEC
@@ -91,6 +92,11 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
return;
}
+static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
+{
+ return -EOPNOTSUPP;
+}
+
static inline void ima_kexec_cmdline(const void *buf, int size) {}
#endif /* CONFIG_IMA */
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2272c3255c7d..9fe949c6a530 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -445,6 +445,55 @@ int ima_file_check(struct file *file, int mask)
}
EXPORT_SYMBOL_GPL(ima_file_check);
+/**
+ * ima_file_hash - return the stored measurement if a file has been hashed and
+ * is in the iint cache.
+ * @file: pointer to the file
+ * @buf: buffer in which to store the hash
+ * @buf_size: length of the buffer
+ *
+ * On success, return the hash algorithm (as defined in the enum hash_algo).
+ * If buf is not NULL, this function also outputs the hash into buf.
+ * If the hash is larger than buf_size, then only buf_size bytes will be copied.
+ * It generally just makes sense to pass a buffer capable of holding the largest
+ * possible hash: IMA_MAX_DIGEST_SIZE.
+ * The file hash returned is based on the entire file, including the appended
+ * signature.
+ *
+ * If IMA is disabled or if no measurement is available, return -EOPNOTSUPP.
+ * If the parameters are incorrect, return -EINVAL.
+ */
+int ima_file_hash(struct file *file, char *buf, size_t buf_size)
+{
+ struct inode *inode;
+ struct integrity_iint_cache *iint;
+ int hash_algo;
+
+ if (!file)
+ return -EINVAL;
+
+ if (!ima_policy_flag)
+ return -EOPNOTSUPP;
+
+ inode = file_inode(file);
+ iint = integrity_iint_find(inode);
+ if (!iint)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&iint->mutex);
+ if (buf) {
+ size_t copied_size;
+
+ copied_size = min_t(size_t, iint->ima_hash->length, buf_size);
+ memcpy(buf, iint->ima_hash->digest, copied_size);
+ }
+ hash_algo = iint->ima_hash->algo;
+ mutex_unlock(&iint->mutex);
+
+ return hash_algo;
+}
+EXPORT_SYMBOL_GPL(ima_file_hash);
+
/**
* ima_post_create_tmpfile - mark newly created tmpfile as new
* @file : newly created tmpfile
--
2.25.0.rc1.283.g88dfdc4193-goog
^ permalink raw reply related
* Re: [PATCH v2] ima: add the ability to query the hash of a given file.
From: KP Singh @ 2020-01-13 10:48 UTC (permalink / raw)
To: Florent Revest
Cc: linux-integrity, kpsingh, mjg59, zohar, nramas, linux-kernel,
linux-security-module, Florent Revest
In-Reply-To: <20200106162524.164650-1-revest@chromium.org>
On 06-Jan 17:25, Florent Revest wrote:
> From: Florent Revest <revest@google.com>
>
> This allows other parts of the kernel (perhaps a stacked LSM allowing
> system monitoring, eg. the proposed KRSI LSM [1]) to retrieve the hash
> of a given file from IMA if it's present in the iint cache.
>
> It's true that the existence of the hash means that it's also in the
> audit logs or in /sys/kernel/security/ima/ascii_runtime_measurements,
> but it can be difficult to pull that information out for every
> subsequent exec. This is especially true if a given host has been up
> for a long time and the file was first measured a long time ago.
>
> This is based on Peter Moody's patch:
> https://sourceforge.net/p/linux-ima/mailman/message/33036180/
>
> [1] https://lkml.org/lkml/2019/9/10/393
>
> Signed-off-by: Florent Revest <revest@google.com>
Thanks for adding this Florent!
Reviewed-by: KP Singh <kpsingh@chromium.org>
> ---
> include/linux/ima.h | 6 ++++
> security/integrity/ima/ima_main.c | 46 +++++++++++++++++++++++++++++++
> 2 files changed, 52 insertions(+)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 6d904754d858..d621c65ba9a5 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -23,6 +23,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
> extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
> enum kernel_read_file_id id);
> extern void ima_post_path_mknod(struct dentry *dentry);
> +extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
> extern void ima_kexec_cmdline(const void *buf, int size);
>
> #ifdef CONFIG_IMA_KEXEC
> @@ -91,6 +92,11 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
> return;
> }
>
> +static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> static inline void ima_kexec_cmdline(const void *buf, int size) {}
> #endif /* CONFIG_IMA */
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index d7e987baf127..3799b6c6c3b8 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -445,6 +445,52 @@ int ima_file_check(struct file *file, int mask)
> }
> EXPORT_SYMBOL_GPL(ima_file_check);
>
> +/**
> + * ima_file_hash - return the stored measurement if a file has been hashed.
> + * @file: pointer to the file
> + * @buf: buffer in which to store the hash
> + * @buf_size: length of the buffer
> + *
> + * On success, return the hash algorithm (as defined in the enum hash_algo).
> + * If buf is not NULL, this function also outputs the hash into buf.
> + * If the hash is larger than buf_size, then only buf_size bytes will be copied.
> + * It generally just makes sense to pass a buffer capable of holding the largest
> + * possible hash: IMA_MAX_DIGEST_SIZE
> + *
> + * If IMA is disabled or if no measurement is available, return -EOPNOTSUPP.
> + * If the parameters are incorrect, return -EINVAL.
> + */
> +int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> + struct inode *inode;
> + struct integrity_iint_cache *iint;
> + int hash_algo;
> +
> + if (!file)
> + return -EINVAL;
> +
> + if (!ima_policy_flag)
> + return -EOPNOTSUPP;
> +
> + inode = file_inode(file);
> + iint = integrity_iint_find(inode);
> + if (!iint)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&iint->mutex);
> + if (buf) {
> + size_t copied_size;
> +
> + copied_size = min_t(size_t, iint->ima_hash->length, buf_size);
> + memcpy(buf, iint->ima_hash->digest, copied_size);
> + }
> + hash_algo = iint->ima_hash->algo;
> + mutex_unlock(&iint->mutex);
> +
> + return hash_algo;
> +}
> +EXPORT_SYMBOL_GPL(ima_file_hash);
> +
> /**
> * ima_post_create_tmpfile - mark newly created tmpfile as new
> * @file : newly created tmpfile
> --
> 2.24.1.735.g03f4e72817-goog
>
^ permalink raw reply
* Re: [PATCH v3] ima: add the ability to query the cached hash of a given file
From: KP Singh @ 2020-01-13 10:51 UTC (permalink / raw)
To: Florent Revest
Cc: linux-integrity, kpsingh, mjg59, zohar, nramas, linux-kernel,
linux-security-module, Florent Revest
In-Reply-To: <20200113094244.26678-1-revest@chromium.org>
On 13-Jan 10:42, Florent Revest wrote:
> From: Florent Revest <revest@google.com>
>
> This allows other parts of the kernel (perhaps a stacked LSM allowing
> system monitoring, eg. the proposed KRSI LSM [1]) to retrieve the hash
> of a given file from IMA if it's present in the iint cache.
>
> It's true that the existence of the hash means that it's also in the
> audit logs or in /sys/kernel/security/ima/ascii_runtime_measurements,
> but it can be difficult to pull that information out for every
> subsequent exec. This is especially true if a given host has been up
> for a long time and the file was first measured a long time ago.
>
> It should be kept in mind that this function gives access to cached
> entries which can be removed, for instance on security_inode_free().
>
> This is based on Peter Moody's patch:
> https://sourceforge.net/p/linux-ima/mailman/message/33036180/
>
> [1] https://lkml.org/lkml/2019/9/10/393
>
> Signed-off-by: Florent Revest <revest@google.com>
Reviewed-by: KP Singh <kpsingh@chromium.org>
> ---
> include/linux/ima.h | 6 ++++
> security/integrity/ima/ima_main.c | 49 +++++++++++++++++++++++++++++++
> 2 files changed, 55 insertions(+)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index f4644c54f648..1659217e9b60 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -23,6 +23,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
> extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
> enum kernel_read_file_id id);
> extern void ima_post_path_mknod(struct dentry *dentry);
> +extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
> extern void ima_kexec_cmdline(const void *buf, int size);
>
> #ifdef CONFIG_IMA_KEXEC
> @@ -91,6 +92,11 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
> return;
> }
>
> +static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> static inline void ima_kexec_cmdline(const void *buf, int size) {}
> #endif /* CONFIG_IMA */
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 2272c3255c7d..9fe949c6a530 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -445,6 +445,55 @@ int ima_file_check(struct file *file, int mask)
> }
> EXPORT_SYMBOL_GPL(ima_file_check);
>
> +/**
> + * ima_file_hash - return the stored measurement if a file has been hashed and
> + * is in the iint cache.
> + * @file: pointer to the file
> + * @buf: buffer in which to store the hash
> + * @buf_size: length of the buffer
> + *
> + * On success, return the hash algorithm (as defined in the enum hash_algo).
> + * If buf is not NULL, this function also outputs the hash into buf.
> + * If the hash is larger than buf_size, then only buf_size bytes will be copied.
> + * It generally just makes sense to pass a buffer capable of holding the largest
> + * possible hash: IMA_MAX_DIGEST_SIZE.
> + * The file hash returned is based on the entire file, including the appended
> + * signature.
> + *
> + * If IMA is disabled or if no measurement is available, return -EOPNOTSUPP.
> + * If the parameters are incorrect, return -EINVAL.
> + */
> +int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> + struct inode *inode;
> + struct integrity_iint_cache *iint;
> + int hash_algo;
> +
> + if (!file)
> + return -EINVAL;
> +
> + if (!ima_policy_flag)
> + return -EOPNOTSUPP;
> +
> + inode = file_inode(file);
> + iint = integrity_iint_find(inode);
> + if (!iint)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&iint->mutex);
> + if (buf) {
> + size_t copied_size;
> +
> + copied_size = min_t(size_t, iint->ima_hash->length, buf_size);
> + memcpy(buf, iint->ima_hash->digest, copied_size);
> + }
> + hash_algo = iint->ima_hash->algo;
> + mutex_unlock(&iint->mutex);
> +
> + return hash_algo;
> +}
> +EXPORT_SYMBOL_GPL(ima_file_hash);
> +
> /**
> * ima_post_create_tmpfile - mark newly created tmpfile as new
> * @file : newly created tmpfile
> --
> 2.25.0.rc1.283.g88dfdc4193-goog
>
^ permalink raw reply
* Re: [PATCH] selinux: remove redundant selinux_nlmsg_perm
From: Stephen Smalley @ 2020-01-13 13:47 UTC (permalink / raw)
To: Huaisheng Ye, paul, eparis, jmorris, serge
Cc: tyu1, linux-security-module, selinux, linux-kernel, Huaisheng Ye
In-Reply-To: <20200112154216.46992-1-yehs2007@zoho.com>
On 1/12/20 10:42 AM, Huaisheng Ye wrote:
> From: Huaisheng Ye <yehs1@lenovo.com>
>
> selinux_nlmsg_perm is used for only by selinux_netlink_send. Remove
> the redundant function to simplify the code.
>
> Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
The patch itself seems fine but it looks like someone accidentally put
pig= in the log message when they meant pid=; that can be fixed via a
separate patch.
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
> security/selinux/hooks.c | 73 ++++++++++++++++++++++--------------------------
> 1 file changed, 34 insertions(+), 39 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index fb1b9da..9f3f966 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -5507,44 +5507,6 @@ static int selinux_tun_dev_open(void *security)
> return 0;
> }
>
> -static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
> -{
> - int err = 0;
> - u32 perm;
> - struct nlmsghdr *nlh;
> - struct sk_security_struct *sksec = sk->sk_security;
> -
> - if (skb->len < NLMSG_HDRLEN) {
> - err = -EINVAL;
> - goto out;
> - }
> - nlh = nlmsg_hdr(skb);
> -
> - err = selinux_nlmsg_lookup(sksec->sclass, nlh->nlmsg_type, &perm);
> - if (err) {
> - if (err == -EINVAL) {
> - pr_warn_ratelimited("SELinux: unrecognized netlink"
> - " message: protocol=%hu nlmsg_type=%hu sclass=%s"
> - " pig=%d comm=%s\n",
> - sk->sk_protocol, nlh->nlmsg_type,
> - secclass_map[sksec->sclass - 1].name,
> - task_pid_nr(current), current->comm);
> - if (!enforcing_enabled(&selinux_state) ||
> - security_get_allow_unknown(&selinux_state))
> - err = 0;
> - }
> -
> - /* Ignore */
> - if (err == -ENOENT)
> - err = 0;
> - goto out;
> - }
> -
> - err = sock_has_perm(sk, perm);
> -out:
> - return err;
> -}
> -
> #ifdef CONFIG_NETFILTER
>
> static unsigned int selinux_ip_forward(struct sk_buff *skb,
> @@ -5873,7 +5835,40 @@ static unsigned int selinux_ipv6_postroute(void *priv,
>
> static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb)
> {
> - return selinux_nlmsg_perm(sk, skb);
> + int err = 0;
> + u32 perm;
> + struct nlmsghdr *nlh;
> + struct sk_security_struct *sksec = sk->sk_security;
> +
> + if (skb->len < NLMSG_HDRLEN) {
> + err = -EINVAL;
> + goto out;
> + }
> + nlh = nlmsg_hdr(skb);
> +
> + err = selinux_nlmsg_lookup(sksec->sclass, nlh->nlmsg_type, &perm);
> + if (err) {
> + if (err == -EINVAL) {
> + pr_warn_ratelimited("SELinux: unrecognized netlink"
> + " message: protocol=%hu nlmsg_type=%hu sclass=%s"
> + " pig=%d comm=%s\n",
> + sk->sk_protocol, nlh->nlmsg_type,
> + secclass_map[sksec->sclass - 1].name,
> + task_pid_nr(current), current->comm);
> + if (!enforcing_enabled(&selinux_state) ||
> + security_get_allow_unknown(&selinux_state))
> + err = 0;
> + }
> +
> + /* Ignore */
> + if (err == -ENOENT)
> + err = 0;
> + goto out;
> + }
> +
> + err = sock_has_perm(sk, perm);
> +out:
> + return err;
> }
>
> static void ipc_init_security(struct ipc_security_struct *isec, u16 sclass)
>
^ permalink raw reply
* RE: [External] Re: [PATCH] selinux: remove redundant selinux_nlmsg_perm
From: Huaisheng HS1 Ye @ 2020-01-13 15:00 UTC (permalink / raw)
To: Stephen Smalley, paul@paul-moore.com, eparis@parisplace.org,
jmorris@namei.org, serge@hallyn.com
Cc: Tzu ting Yu1, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, linux-kernel@vger.kernel.org,
Huaisheng Ye
In-Reply-To: <e7ec908e-01c1-b76d-f797-545b70a49075@tycho.nsa.gov>
> -----Original Message-----
> From: Stephen Smalley <sds@tycho.nsa.gov>
> Sent: Monday, January 13, 2020 9:47 PM
>
> On 1/12/20 10:42 AM, Huaisheng Ye wrote:
> > From: Huaisheng Ye <yehs1@lenovo.com>
> >
> > selinux_nlmsg_perm is used for only by selinux_netlink_send. Remove
> > the redundant function to simplify the code.
> >
> > Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
>
> The patch itself seems fine but it looks like someone accidentally put pig= in
> the log message when they meant pid=; that can be fixed via a separate patch.
>
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Thanks for the Acked-by.
Aha, yes it is. I will offer the patch v2 to fix this typo.
Cheers,
Huaisheng Ye
^ permalink raw reply
* [PATCH v2] selinux: remove redundant selinux_nlmsg_perm
From: Huaisheng Ye @ 2020-01-13 15:03 UTC (permalink / raw)
To: paul, sds, eparis, jmorris, serge
Cc: tyu1, linux-security-module, selinux, linux-kernel, Huaisheng Ye
From: Huaisheng Ye <yehs1@lenovo.com>
selinux_nlmsg_perm is used for only by selinux_netlink_send. Remove
the redundant function to simplify the code.
Fix a typo by suggestion from Stephen.
Signed-off-by: Huaisheng Ye <yehs1@lenovo.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
---
security/selinux/hooks.c | 73 ++++++++++++++++++++++--------------------------
1 file changed, 34 insertions(+), 39 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index fb1b9da..9f3f966 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5507,44 +5507,6 @@ static int selinux_tun_dev_open(void *security)
return 0;
}
-static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
-{
- int err = 0;
- u32 perm;
- struct nlmsghdr *nlh;
- struct sk_security_struct *sksec = sk->sk_security;
-
- if (skb->len < NLMSG_HDRLEN) {
- err = -EINVAL;
- goto out;
- }
- nlh = nlmsg_hdr(skb);
-
- err = selinux_nlmsg_lookup(sksec->sclass, nlh->nlmsg_type, &perm);
- if (err) {
- if (err == -EINVAL) {
- pr_warn_ratelimited("SELinux: unrecognized netlink"
- " message: protocol=%hu nlmsg_type=%hu sclass=%s"
- " pig=%d comm=%s\n",
- sk->sk_protocol, nlh->nlmsg_type,
- secclass_map[sksec->sclass - 1].name,
- task_pid_nr(current), current->comm);
- if (!enforcing_enabled(&selinux_state) ||
- security_get_allow_unknown(&selinux_state))
- err = 0;
- }
-
- /* Ignore */
- if (err == -ENOENT)
- err = 0;
- goto out;
- }
-
- err = sock_has_perm(sk, perm);
-out:
- return err;
-}
-
#ifdef CONFIG_NETFILTER
static unsigned int selinux_ip_forward(struct sk_buff *skb,
@@ -5873,7 +5835,40 @@ static unsigned int selinux_ipv6_postroute(void *priv,
static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb)
{
- return selinux_nlmsg_perm(sk, skb);
+ int err = 0;
+ u32 perm;
+ struct nlmsghdr *nlh;
+ struct sk_security_struct *sksec = sk->sk_security;
+
+ if (skb->len < NLMSG_HDRLEN) {
+ err = -EINVAL;
+ goto out;
+ }
+ nlh = nlmsg_hdr(skb);
+
+ err = selinux_nlmsg_lookup(sksec->sclass, nlh->nlmsg_type, &perm);
+ if (err) {
+ if (err == -EINVAL) {
+ pr_warn_ratelimited("SELinux: unrecognized netlink"
+ " message: protocol=%hu nlmsg_type=%hu sclass=%s"
+ " pid=%d comm=%s\n",
+ sk->sk_protocol, nlh->nlmsg_type,
+ secclass_map[sksec->sclass - 1].name,
+ task_pid_nr(current), current->comm);
+ if (!enforcing_enabled(&selinux_state) ||
+ security_get_allow_unknown(&selinux_state))
+ err = 0;
+ }
+
+ /* Ignore */
+ if (err == -ENOENT)
+ err = 0;
+ goto out;
+ }
+
+ err = sock_has_perm(sk, perm);
+out:
+ return err;
}
static void ipc_init_security(struct ipc_security_struct *isec, u16 sclass)
--
1.8.3.1
^ permalink raw reply related
* [RFC PATCH] selinux: implement move_mount hook
From: Stephen Smalley @ 2020-01-13 16:18 UTC (permalink / raw)
To: paul
Cc: selinux, omosnace, dhowells, linux-security-module, jmorris,
richard_c_haines, Stephen Smalley
commit 2db154b3ea8e ("vfs: syscall: Add move_mount(2) to move mounts around")
introduced a new move_mount(2) system call and a corresponding new LSM
security_move_mount hook but did not implement this hook for any existing
LSM. This creates a regression for SELinux with respect to consistent
checking of mounts; the existing selinux_mount hook checks mounton
permission to the mount point path. Provide a SELinux hook
implementation for move_mount that applies this same check for
consistency. We may wish to consider defining a new filesystem
move_mount permission and/or a new dir(ectory) move_mount permission
and checking it in this hook in the future.
Fixes: 2db154b3ea8e ("vfs: syscall: Add move_mount(2) to move mounts around")
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
security/selinux/hooks.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0606e107fca3..244874b103ff 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2766,6 +2766,19 @@ static int selinux_mount(const char *dev_name,
return path_has_perm(cred, path, FILE__MOUNTON);
}
+static int selinux_move_mount(const struct path *from_path,
+ const struct path *to_path)
+{
+ const struct cred *cred = current_cred();
+
+ /*
+ * TBD: Check new FILESYSTEM__MOVE_MOUNT permission to
+ * from_path->dentry->s_sb and/or new DIR__MOVE_MOUNT
+ * permission to from_path?
+ */
+ return path_has_perm(cred, to_path, FILE__MOUNTON);
+}
+
static int selinux_umount(struct vfsmount *mnt, int flags)
{
const struct cred *cred = current_cred();
@@ -6943,6 +6956,8 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(sb_set_mnt_opts, selinux_set_mnt_opts),
LSM_HOOK_INIT(sb_clone_mnt_opts, selinux_sb_clone_mnt_opts),
+ LSM_HOOK_INIT(move_mount, selinux_move_mount),
+
LSM_HOOK_INIT(dentry_init_security, selinux_dentry_init_security),
LSM_HOOK_INIT(dentry_create_files_as, selinux_dentry_create_files_as),
--
2.24.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox