[PATCH] fs: switch order of CAP_DAC_OVERRIDE and CAP_DAC_READ_SEARCH checks

[Stephen Smalley]

generic_permission() presently checks CAP_DAC_OVERRIDE prior to
CAP_DAC_READ_SEARCH.  This can cause misleading audit messages when
using a LSM such as SELinux or AppArmor, since CAP_DAC_OVERRIDE
may not be required for the operation.  Flip the order of the
tests so that CAP_DAC_OVERRIDE is only checked when required for
the operation.

Signed-off-by: Stephen Smalley 
---
 fs/namei.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index ad74877..8736e4a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -340,22 +340,14 @@ int generic_permission(struct inode *inode, int mask)
 
if (S_ISDIR(inode->i_mode)) {
/* DACs are overridable for directories */
-   if (capable_wrt_inode_uidgid(inode, CAP_DAC_OVERRIDE))
-   return 0;
if (!(mask & MAY_WRITE))
if (capable_wrt_inode_uidgid(inode,
 CAP_DAC_READ_SEARCH))
return 0;
-   return -EACCES;
-   }
-   /*
-* Read/write DACs are always overridable.
-* Executable DACs are overridable when there is
-* at least one exec bit set.
-*/
-   if (!(mask & MAY_EXEC) || (inode->i_mode & S_IXUGO))
if (capable_wrt_inode_uidgid(inode, CAP_DAC_OVERRIDE))
return 0;
+   return -EACCES;
+   }
 
/*
 * Searching includes executable on directories, else just read.
@@ -364,6 +356,14 @@ int generic_permission(struct inode *inode, int mask)
if (mask == MAY_READ)
if (capable_wrt_inode_uidgid(inode, CAP_DAC_READ_SEARCH))
return 0;
+   /*
+* Read/write DACs are always overridable.
+* Executable DACs are overridable when there is
+* at least one exec bit set.
+*/
+   if (!(mask & MAY_EXEC) || (inode->i_mode & S_IXUGO))
+   if (capable_wrt_inode_uidgid(inode, CAP_DAC_OVERRIDE))
+   return 0;
 
return -EACCES;
 }
-- 
2.7.4

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [kernel-hardening] Re: [RFC v2 PATCH 1/2] security: introduce CONFIG_SECURITY_WRITABLE_HOOKS

[Casey Schaufler]

On 2/17/2017 7:05 AM, Tetsuo Handa wrote:
> Casey Schaufler wrote:
>> On 2/16/2017 3:00 AM, Tetsuo Handa wrote:
>>> Casey Schaufler wrote:
 I can't say that I'm buying the value of the additional
 complexity here. Sure, you're protecting part of the data
 all the time, but you're exposing part all the time, too.
>>> Will you explain it in detail? As far as I know, __ro_after_init
>>> annotation makes the kernel to call set_memory_ro() after __init
>>> functions are completed, by gathering such variables into a section
>>> so that set_memory_ro() can change page flags for a memory region
>>> where it is PAGE_ALIGNED and the size is multiple of PAGE_SIZE bytes.
>>>
>>> This "struct lsm_entry" is for gathering only LSM related variables which
>>> would have been marked as __ro_after_init if 
>>> CONFIG_SECURITY_WRITABLE_HOOKS=n
>>> into a list of memory regions where they are PAGE_ALIGNED and the size is
>>> multiple of PAGE_SIZE bytes, so that the kernel can call set_memory_ro() on
>>> these memory regions even if CONFIG_SECURITY_SELINUX_DISABLE=y or allowing
>>> LKM based LSMs.
>> All I'm saying is that it's simpler to mark the entire
>> structure than it is to mark parts.
> I'm still unable to understand.
> "struct lsm_entry" != "the entire structure" ?
>
 For now I think that the solution James proposes makes the
 most sense. In the hypothetical loadable modules case I
 don't see real value in hardening bits of the data while
 explicitly making the most important parts dynamic.
>>> Best solution for environments where it is known to enforce only one 
>>> specific
>>> LSM module and no user choices and no LKM based LSMs (including antivirus or
>>> alike) is to directly embed that LSM module into security/security.c .
>> There are too few beers on the table in front of me
>> to start that discussion. :)
>>
>>> CONFIG_SECURITY_WRITABLE_HOOKS=n depends on 
>>> CONFIG_SECURITY_SELINUX_DISABLE=n
>>> but many of distributor's kernels enable multiple LSM modules including
>>> CONFIG_SECURITY_SELINUX_DISABLE=y. Also, people are using antivirus software
>>> on distributor's kernels.
>> I have to limit my comment on that to pointing out
>> that we can't make decisions based on code that has
>> never been proposed for the mainline.
> If I recall correctly, LSM framework was considered as "should be used by only
> rule based access restriction mechanisms (so-called MAC)". Hooks for e.g.
> antivirus modules should use different interfaces (e.g. fsnotify_open())
> which do not offer ability to reject access requests synchronously.
> I think that this technical barrier was removed by commit b1d9e6b0646d0e5e 
> "LSM:
> Switch to lists of hooks". But there still remains non-technical barrier.

The LSM infrastructure is 20 years old (or close too) and
as anyone who's raised a child can attest, what you expected
in the beginning is rarely what you end up with. We have some
really good implementations of MAC, but they have never been
the glint in the community's eye.


> We think that we should not merge similar modules which provide same 
> functionality.
> What about antivirus modules? They offer same functionality (intercept and 
> judge
> access requests) but interface details vary depending on userspace daemon 
> which
> performs actual scan. They cannot be unified, but we won't agree with merging
> all product's all implementations. Do we change our mind and encourage 
> antivirus
> modules developers to try proposing for the mainline?

Security means so many things to so many people
(There are people who think queuing up for an hour
to get a body scan is an element of security.)
that restricting the LSM infrastructure to MAC seems
a bit arrogant. I am *not* saying that I support
allowing security modules that are nothing more than
interfaces for proprietary, out of tree mechanisms.
Those are evil. I would encourage an antivirus (or
time based access control, or content based denial)
to use the LSM infrastructure if it is all going upstream,
where it belongs.

>>>  At least one antivirus software (which allows
>>> anonymous download of LKM source code) is using LSM hooks since Linux 2.6.32
>>> instead of rewriting syscall tables. We are already allowing multiple 
>>> concurrent
>>> LSM modules (up to one fully armored module which uses "struct 
>>> cred"->security
>>> field or exclusive hooks like security_xfrm_state_pol_flow_match(), plus
>>> unlimited number of lightweight modules which do not use "struct 
>>> cred"->security
>>> nor exclusive hooks) as long as they are built into the kernel. There is no
>>> reason to keep LKM based LSM modules from antivirus software or alike away.
>> We're not to the point where in-kernel modules are stacking fully.
>> Not everyone is on board for that, but hope springs eternal. Part
>> of the design criteria I'm working under is that it shouldn't
>> preclude loadable modules, and I still think that's doable. The
>> patch James proposed is completely 

Re: [PATCH] prlimit, security, selinux: add a security hook for prlimit

[Paul Moore]

On Thu, Feb 16, 2017 at 10:24 PM, James Morris  wrote:
> On Thu, 16 Feb 2017, Stephen Smalley wrote:
>
>> When SELinux was first added to the kernel, a process could only get
>> and set its own resource limits via getrlimit(2) and setrlimit(2), so no
>> MAC checks were required for those operations, and thus no security hooks
>> were defined for them. Later, SELinux introduced a hook for setlimit(2)
>> with a check if the hard limit was being changed in order to be able to
>> rely on the hard limit value as a safe reset point upon context
>> transitions.
>
> [...]
>
>
> Queued for 4.11 at
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git#next-queue

Stephen pushed a v2 patch to fix a build problem, are you going to
fixup your tree?

-- 
paul moore
www.paul-moore.com
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [PATCH] libsepol/cil: Destroy cil_tree_node stacks when finished resolving AST

[James Carter]

On 02/08/2017 11:17 AM, James Carter wrote:

CIL uses separate cil_tree_node stacks for optionals and blocks to
check for statements not allowed in optionals or blocks and to know
which optional to disable when necessary. But these stacks were not
being destroyed when exiting cil_resolve_ast(). This is not a problem
normally because the stacks will be empty, but this is not the case
when exiting with an error.

Destroy both tree node stacks when exiting to ensure that they are
empty.

Signed-off-by: James Carter 


This has been applied.


---
 libsepol/cil/src/cil_resolve_ast.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/libsepol/cil/src/cil_resolve_ast.c 
b/libsepol/cil/src/cil_resolve_ast.c
index 7fe4a74..6628dc4 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -3778,6 +3778,16 @@ exit:
return rc;
 }

+static void cil_destroy_tree_node_stack(struct cil_tree_node *curr)
+{
+   struct cil_tree_node *next;
+   while (curr != NULL) {
+   next = curr->cl_head;
+   free(curr);
+   curr = next;
+   }
+}
+
 int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)
 {
int rc = SEPOL_ERR;
@@ -3904,16 +3914,12 @@ int cil_resolve_ast(struct cil_db *db, struct 
cil_tree_node *current)
/* reset the arguments */
changed = 0;
while (extra_args.optstack != NULL) {
-   struct cil_tree_node *curr = extra_args.optstack;
-   struct cil_tree_node *next = curr->cl_head;
-   free(curr);
-   extra_args.optstack = next;
+   cil_destroy_tree_node_stack(extra_args.optstack);
+   extra_args.optstack = NULL;
}
while (extra_args.blockstack!= NULL) {
-   struct cil_tree_node *curr = extra_args.blockstack;
-   struct cil_tree_node *next = curr->cl_head;
-   free(curr);
-   extra_args.blockstack= next;
+   cil_destroy_tree_node_stack(extra_args.blockstack);
+   extra_args.blockstack = NULL;
}
}

@@ -3924,6 +3930,8 @@ int cil_resolve_ast(struct cil_db *db, struct 
cil_tree_node *current)

rc = SEPOL_OK;
 exit:
+   cil_destroy_tree_node_stack(extra_args.optstack);
+   cil_destroy_tree_node_stack(extra_args.blockstack);
__cil_ordered_lists_destroy(_args.sidorder_lists);
__cil_ordered_lists_destroy(_args.classorder_lists);
__cil_ordered_lists_destroy(_args.catorder_lists);




--
James Carter 
National Security Agency
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [RFC v2 PATCH 2/2] security: mark LSM hooks as __ro_after_init

[Stephen Smalley]

On Wed, 2017-02-15 at 00:18 +1100, James Morris wrote:
> Mark all of the registration hooks as __ro_after_init (via the 
> __lsm_ro_after_init macro).
> 
> Signed-off-by: James Morris 

Acked-by: Stephen Smalley 

> ---
>  security/apparmor/lsm.c|2 +-
>  security/commoncap.c   |2 +-
>  security/loadpin/loadpin.c |2 +-
>  security/security.c|2 +-
>  security/selinux/hooks.c   |2 +-
>  security/smack/smack_lsm.c |2 +-
>  security/tomoyo/tomoyo.c   |2 +-
>  security/yama/yama_lsm.c   |2 +-
>  8 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 709eacd..e287b69 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -587,7 +587,7 @@ static int apparmor_task_setrlimit(struct
> task_struct *task,
>   return error;
>  }
>  
> -static struct security_hook_list apparmor_hooks[] = {
> +static struct security_hook_list apparmor_hooks[]
> __lsm_ro_after_init = {
>   LSM_HOOK_INIT(ptrace_access_check,
> apparmor_ptrace_access_check),
>   LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
>   LSM_HOOK_INIT(capget, apparmor_capget),
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 6d4d586..a9db18c 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -1070,7 +1070,7 @@ int cap_mmap_file(struct file *file, unsigned
> long reqprot,
>  
>  #ifdef CONFIG_SECURITY
>  
> -struct security_hook_list capability_hooks[] = {
> +struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
>   LSM_HOOK_INIT(capable, cap_capable),
>   LSM_HOOK_INIT(settime, cap_settime),
>   LSM_HOOK_INIT(ptrace_access_check, cap_ptrace_access_check),
> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
> index 1d82eae..dbe6efd 100644
> --- a/security/loadpin/loadpin.c
> +++ b/security/loadpin/loadpin.c
> @@ -174,7 +174,7 @@ static int loadpin_read_file(struct file *file,
> enum kernel_read_file_id id)
>   return 0;
>  }
>  
> -static struct security_hook_list loadpin_hooks[] = {
> +static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init
> = {
>   LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
>   LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
>  };
> diff --git a/security/security.c b/security/security.c
> index d0e07f2..75ed309 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1622,7 +1622,7 @@ int security_audit_rule_match(u32 secid, u32
> field, u32 op, void *lsmrule,
>  }
>  #endif /* CONFIG_AUDIT */
>  
> -struct security_hook_heads security_hook_heads = {
> +struct security_hook_heads security_hook_heads __lsm_ro_after_init =
> {
>   .binder_set_context_mgr =
>   LIST_HEAD_INIT(security_hook_heads.binder_set_contex
> t_mgr),
>   .binder_transaction =
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 9bc12bc..b1a9916 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6104,7 +6104,7 @@ static int selinux_key_getsecurity(struct key
> *key, char **_buffer)
>  
>  #endif
>  
> -static struct security_hook_list selinux_hooks[] = {
> +static struct security_hook_list selinux_hooks[] __lsm_ro_after_init
> = {
>   LSM_HOOK_INIT(binder_set_context_mgr,
> selinux_binder_set_context_mgr),
>   LSM_HOOK_INIT(binder_transaction,
> selinux_binder_transaction),
>   LSM_HOOK_INIT(binder_transfer_binder,
> selinux_binder_transfer_binder),
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 60b4217..71e24d8 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4633,7 +4633,7 @@ static int smack_inode_getsecctx(struct inode
> *inode, void **ctx, u32 *ctxlen)
>   return 0;
>  }
>  
> -static struct security_hook_list smack_hooks[] = {
> +static struct security_hook_list smack_hooks[] __lsm_ro_after_init =
> {
>   LSM_HOOK_INIT(ptrace_access_check,
> smack_ptrace_access_check),
>   LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme),
>   LSM_HOOK_INIT(syslog, smack_syslog),
> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index edc52d6..b5fb930 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -496,7 +496,7 @@ static int tomoyo_socket_sendmsg(struct socket
> *sock, struct msghdr *msg,
>   * tomoyo_security_ops is a "struct security_operations" which is
> used for
>   * registering TOMOYO.
>   */
> -static struct security_hook_list tomoyo_hooks[] = {
> +static struct security_hook_list tomoyo_hooks[] __lsm_ro_after_init
> = {
>   LSM_HOOK_INIT(cred_alloc_blank, tomoyo_cred_alloc_blank),
>   LSM_HOOK_INIT(cred_prepare, tomoyo_cred_prepare),
>   LSM_HOOK_INIT(cred_transfer, tomoyo_cred_transfer),
> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
> index 88271a3..8298e09 100644
> --- 

Re: [RFC v2 PATCH 1/2] security: introduce CONFIG_SECURITY_WRITABLE_HOOKS

[Stephen Smalley]

On Wed, 2017-02-15 at 00:17 +1100, James Morris wrote:
> Subsequent patches will add RO hardening to LSM hooks, however,
> SELinux
> still needs to be able to perform runtime disablement after init to
> handle
> architectures where init-time disablement via boot parameters is not
> feasible.
> 
> Introduce a new kernel configuration parameter
> CONFIG_SECURITY_WRITABLE_HOOKS,
> and a helper macro __lsm_ro_after_init, to handle this case.
> 
> Signed-off-by: James Morris 

Acked-by:  Stephen Smalley 

> ---
>  include/linux/lsm_hooks.h |7 +++
>  security/Kconfig  |5 +
>  security/selinux/Kconfig  |6 ++
>  3 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index e29d4c6..c4b149f 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1908,6 +1908,13 @@ static inline void
> security_delete_hooks(struct security_hook_list *hooks,
>  }
>  #endif /* CONFIG_SECURITY_SELINUX_DISABLE */
>  
> +/* Currently required to handle SELinux runtime hook disable. */
> +#ifdef CONFIG_SECURITY_WRITABLE_HOOKS
> +#define __lsm_ro_after_init
> +#else
> +#define __lsm_ro_after_init  __ro_after_init
> +#endif /* CONFIG_SECURITY_WRITABLE_HOOKS */
> +
>  extern int __init security_module_enable(const char *module);
>  extern void __init capability_add_hooks(void);
>  #ifdef CONFIG_SECURITY_YAMA
> diff --git a/security/Kconfig b/security/Kconfig
> index 118f454..f6f90c4 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -31,6 +31,11 @@ config SECURITY
>  
>     If you are unsure how to answer this question, answer N.
>  
> +config SECURITY_WRITABLE_HOOKS
> + depends on SECURITY
> + bool
> + default n
> +
>  config SECURITYFS
>   bool "Enable the securityfs filesystem"
>   help
> diff --git a/security/selinux/Kconfig b/security/selinux/Kconfig
> index ea7e3ef..8af7a69 100644
> --- a/security/selinux/Kconfig
> +++ b/security/selinux/Kconfig
> @@ -40,6 +40,7 @@ config SECURITY_SELINUX_BOOTPARAM_VALUE
>  config SECURITY_SELINUX_DISABLE
>   bool "NSA SELinux runtime disable"
>   depends on SECURITY_SELINUX
> + select SECURITY_WRITABLE_HOOKS
>   default n
>   help
>     This option enables writing to a selinuxfs node 'disable',
> which
> @@ -50,6 +51,11 @@ config SECURITY_SELINUX_DISABLE
>     portability across platforms where boot parameters are
> difficult
>     to employ.
>  
> +   NOTE: selecting this option will disable the
> '__ro_after_init'
> +   kernel hardening feature for security hooks.   Please
> consider
> +   using the selinux=0 boot parameter instead of enabling
> this
> +   option.
> +
>     If you are unsure how to answer this question, answer N.
>  
>  config SECURITY_SELINUX_DEVELOP
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Re: [kernel-hardening] Re: [RFC v2 PATCH 1/2] security: introduce CONFIG_SECURITY_WRITABLE_HOOKS

[Tetsuo Handa]

Casey Schaufler wrote:
> On 2/16/2017 3:00 AM, Tetsuo Handa wrote:
> > Casey Schaufler wrote:
> >> I can't say that I'm buying the value of the additional
> >> complexity here. Sure, you're protecting part of the data
> >> all the time, but you're exposing part all the time, too.
> > Will you explain it in detail? As far as I know, __ro_after_init
> > annotation makes the kernel to call set_memory_ro() after __init
> > functions are completed, by gathering such variables into a section
> > so that set_memory_ro() can change page flags for a memory region
> > where it is PAGE_ALIGNED and the size is multiple of PAGE_SIZE bytes.
> >
> > This "struct lsm_entry" is for gathering only LSM related variables which
> > would have been marked as __ro_after_init if 
> > CONFIG_SECURITY_WRITABLE_HOOKS=n
> > into a list of memory regions where they are PAGE_ALIGNED and the size is
> > multiple of PAGE_SIZE bytes, so that the kernel can call set_memory_ro() on
> > these memory regions even if CONFIG_SECURITY_SELINUX_DISABLE=y or allowing
> > LKM based LSMs.
> 
> All I'm saying is that it's simpler to mark the entire
> structure than it is to mark parts.

I'm still unable to understand.
"struct lsm_entry" != "the entire structure" ?

> 
> >> For now I think that the solution James proposes makes the
> >> most sense. In the hypothetical loadable modules case I
> >> don't see real value in hardening bits of the data while
> >> explicitly making the most important parts dynamic.
> > Best solution for environments where it is known to enforce only one 
> > specific
> > LSM module and no user choices and no LKM based LSMs (including antivirus or
> > alike) is to directly embed that LSM module into security/security.c .
> 
> There are too few beers on the table in front of me
> to start that discussion. :)
> 
> > CONFIG_SECURITY_WRITABLE_HOOKS=n depends on 
> > CONFIG_SECURITY_SELINUX_DISABLE=n
> > but many of distributor's kernels enable multiple LSM modules including
> > CONFIG_SECURITY_SELINUX_DISABLE=y. Also, people are using antivirus software
> > on distributor's kernels.
> 
> I have to limit my comment on that to pointing out
> that we can't make decisions based on code that has
> never been proposed for the mainline.

If I recall correctly, LSM framework was considered as "should be used by only
rule based access restriction mechanisms (so-called MAC)". Hooks for e.g.
antivirus modules should use different interfaces (e.g. fsnotify_open())
which do not offer ability to reject access requests synchronously.
I think that this technical barrier was removed by commit b1d9e6b0646d0e5e "LSM:
Switch to lists of hooks". But there still remains non-technical barrier.

We think that we should not merge similar modules which provide same 
functionality.
What about antivirus modules? They offer same functionality (intercept and judge
access requests) but interface details vary depending on userspace daemon which
performs actual scan. They cannot be unified, but we won't agree with merging
all product's all implementations. Do we change our mind and encourage antivirus
modules developers to try proposing for the mainline?

> 
> >  At least one antivirus software (which allows
> > anonymous download of LKM source code) is using LSM hooks since Linux 2.6.32
> > instead of rewriting syscall tables. We are already allowing multiple 
> > concurrent
> > LSM modules (up to one fully armored module which uses "struct 
> > cred"->security
> > field or exclusive hooks like security_xfrm_state_pol_flow_match(), plus
> > unlimited number of lightweight modules which do not use "struct 
> > cred"->security
> > nor exclusive hooks) as long as they are built into the kernel. There is no
> > reason to keep LKM based LSM modules from antivirus software or alike away.
> 
> We're not to the point where in-kernel modules are stacking fully.
> Not everyone is on board for that, but hope springs eternal. Part
> of the design criteria I'm working under is that it shouldn't
> preclude loadable modules, and I still think that's doable. The
> patch James proposed is completely compatible with this philosophy.
> You can argue that it requires a loadable module configuration be
> less "hardened", but the opponents of loadable modules say that is
> inherent to loadable modules.

The patch James proposed looks like a placebo for me, for many of distributor
kernels will have to choose CONFIG_SECURITY_WRITABLE_HOOKS=y. If we offer a way 
to
call set_memory_ro()/set_memory_rw(), such distributor kernels will be able to
behave like CONFIG_SECURITY_WRITABLE_HOOKS=n.

> 
> > For such kernels, this "struct lsm_entry" allows calling set_memory_ro() on 
> > LSM
> > related variables which would have been marked as __ro_after_init if
> > CONFIG_SECURITY_WRITABLE_HOOKS=n.
> >
> > If you are not happy with using kmalloc() for copying "struct 
> > security_hook_list"
> > in each LSM module, can we agree with padding "struct security_hook_list" 
> > in each
> > LSM module 

[PATCH] timerfd: only check CAP_WAKE_ALARM when it is needed

[Stephen Smalley]

timerfd_create() and do_timerfd_settime() presently always
call capable(CAP_WAKE_ALARM) although CAP_WAKE_ALARM is
only required for CLOCK_REALTIME_ALARM and CLOCK_BOOTTIME_ALARM.
This can cause extraneous audit messages when using a LSM such
as SELinux, incorrectly causes PF_SUPERPRIV to be set even when
no privilege was exercised, and is inefficient.  Flip the order
of the tests in both functions so that we only call capable() if
the capability is truly required for the operation.

Signed-off-by: Stephen Smalley 
---
 fs/timerfd.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/timerfd.c b/fs/timerfd.c
index 384fa75..c543cdb 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -400,9 +400,9 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
 clockid != CLOCK_BOOTTIME_ALARM))
return -EINVAL;
 
-   if (!capable(CAP_WAKE_ALARM) &&
-   (clockid == CLOCK_REALTIME_ALARM ||
-clockid == CLOCK_BOOTTIME_ALARM))
+   if ((clockid == CLOCK_REALTIME_ALARM ||
+clockid == CLOCK_BOOTTIME_ALARM) &&
+   !capable(CAP_WAKE_ALARM))
return -EPERM;
 
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
@@ -449,7 +449,7 @@ static int do_timerfd_settime(int ufd, int flags,
return ret;
ctx = f.file->private_data;
 
-   if (!capable(CAP_WAKE_ALARM) && isalarm(ctx)) {
+   if (isalarm(ctx) && !capable(CAP_WAKE_ALARM)) {
fdput(f);
return -EPERM;
}
-- 
2.7.4

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

[PATCH v2] prlimit,security,selinux: add a security hook for prlimit

[Stephen Smalley]

When SELinux was first added to the kernel, a process could only get
and set its own resource limits via getrlimit(2) and setrlimit(2), so no
MAC checks were required for those operations, and thus no security hooks
were defined for them. Later, SELinux introduced a hook for setlimit(2)
with a check if the hard limit was being changed in order to be able to
rely on the hard limit value as a safe reset point upon context
transitions.

Later on, when prlimit(2) was added to the kernel with the ability to get
or set resource limits (hard or soft) of another process, LSM/SELinux was
not updated other than to pass the target process to the setrlimit hook.
This resulted in incomplete control over both getting and setting the
resource limits of another process.

Add a new security_task_prlimit() hook to the check_prlimit_permission()
function to provide complete mediation.  The hook is only called when
acting on another task, and only if the existing DAC/capability checks
would allow access.  Pass flags down to the hook to indicate whether the
prlimit(2) call will read, write, or both read and write the resource
limits of the target process.

The existing security_task_setrlimit() hook is left alone; it continues
to serve a purpose in supporting the ability to make decisions based on
the old and/or new resource limit values when setting limits.  This
is consistent with the DAC/capability logic, where
check_prlimit_permission() performs generic DAC/capability checks for
acting on another task, while do_prlimit() performs a capability check
based on a comparison of the old and new resource limits.  Fix the
inline documentation for the hook to match the code.

Implement the new hook for SELinux.  For setting resource limits, we
reuse the existing setrlimit permission.  Note that this does overload
the setrlimit permission to mean the ability to set the resource limit
(soft or hard) of another process or the ability to change one's own
hard limit.  For getting resource limits, a new getrlimit permission
is defined.  This was not originally defined since getrlimit(2) could
only be used to obtain a process' own limits.

Signed-off-by: Stephen Smalley 
---
v2 fixes the build for the CONFIG_SECURITY=n case, as detected by the
0-day kernel test infrastructure.

 include/linux/lsm_hooks.h   | 18 +++---
 include/linux/security.h| 13 +
 kernel/sys.c| 30 ++
 security/security.c |  8 
 security/selinux/hooks.c| 14 ++
 security/selinux/include/classmap.h |  2 +-
 6 files changed, 69 insertions(+), 16 deletions(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 6fe7a5c..5832f74 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -631,10 +631,19 @@
  * Check permission before getting the ioprio value of @p.
  * @p contains the task_struct of process.
  * Return 0 if permission is granted.
+ * @task_prlimit:
+ * Check permission before getting and/or setting the resource limits of
+ * another task.
+ * @cred points to the cred structure for the current task.
+ * @tcred points to the cred structure for the target task.
+ * @flags contains the LSM_PRLIMIT_* flag bits indicating whether the
+ * resource limits are being read, modified, or both.
+ * Return 0 if permission is granted.
  * @task_setrlimit:
- * Check permission before setting the resource limits of the current
- * process for @resource to @new_rlim.  The old resource limit values can
- * be examined by dereferencing (current->signal->rlim + resource).
+ * Check permission before setting the resource limits of process @p
+ * for @resource to @new_rlim.  The old resource limit values can
+ * be examined by dereferencing (p->signal->rlim + resource).
+ * @p points to the task_struct for the target task's group leader.
  * @resource contains the resource whose limit is being set.
  * @new_rlim contains the new limits for @resource.
  * Return 0 if permission is granted.
@@ -1495,6 +1504,8 @@ union security_list_options {
int (*task_setnice)(struct task_struct *p, int nice);
int (*task_setioprio)(struct task_struct *p, int ioprio);
int (*task_getioprio)(struct task_struct *p);
+   int (*task_prlimit)(const struct cred *cred, const struct cred *tcred,
+   unsigned int flags);
int (*task_setrlimit)(struct task_struct *p, unsigned int resource,
struct rlimit *new_rlim);
int (*task_setscheduler)(struct task_struct *p);
@@ -1756,6 +1767,7 @@ struct security_hook_heads {
struct list_head task_setnice;
struct list_head task_setioprio;
struct list_head task_getioprio;
+   struct list_head task_prlimit;
struct list_head task_setrlimit;
struct list_head