Unconditionally call Yama, no matter what LSM module is selected.
Ubuntu and Chrome OS already carry patches to do this, and Fedora has
voiced interest in doing this as well. Instead of having everyone carry
these patches, just switch Yama to being unconditional when compiled
into the kernel.
Signed-off-by: Kees Cook <[email protected]>
---
include/linux/security.h | 31 +++++++++++++++++++++++++++++++
security/Kconfig | 5 -----
security/security.c | 13 +++++++++++++
security/yama/yama_lsm.c | 14 ++++----------
4 files changed, 48 insertions(+), 15 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 3dea6a9..01ef030 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -3021,5 +3021,36 @@ static inline void free_secdata(void *secdata)
{ }
#endif /* CONFIG_SECURITY */
+#ifdef CONFIG_SECURITY_YAMA
+extern int yama_ptrace_access_check(struct task_struct *child,
+ unsigned int mode);
+extern int yama_ptrace_traceme(struct task_struct *parent);
+extern void yama_task_free(struct task_struct *task);
+extern int yama_task_prctl(int option, unsigned long arg2, unsigned long arg3,
+ unsigned long arg4, unsigned long arg5);
+#else
+static inline int yama_ptrace_access_check(struct task_struct *child,
+ unsigned int mode)
+{
+ return 0;
+}
+
+static inline int yama_ptrace_traceme(struct task_struct *parent)
+{
+ return 0;
+}
+
+static inline void yama_task_free(struct task_struct *task)
+{
+}
+
+static inline int yama_task_prctl(int option, unsigned long arg2,
+ unsigned long arg3, unsigned long arg4,
+ unsigned long arg5)
+{
+ return -ENOSYS;
+}
+#endif /* CONFIG_SECURITY_YAMA */
+
#endif /* ! __LINUX_SECURITY_H */
diff --git a/security/Kconfig b/security/Kconfig
index e9c6ac7..bde1b31 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -131,7 +131,6 @@ choice
default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
- default DEFAULT_SECURITY_YAMA if SECURITY_YAMA
default DEFAULT_SECURITY_DAC
help
@@ -150,9 +149,6 @@ choice
config DEFAULT_SECURITY_APPARMOR
bool "AppArmor" if SECURITY_APPARMOR=y
- config DEFAULT_SECURITY_YAMA
- bool "Yama" if SECURITY_YAMA=y
-
config DEFAULT_SECURITY_DAC
bool "Unix Discretionary Access Controls"
@@ -164,7 +160,6 @@ config DEFAULT_SECURITY
default "smack" if DEFAULT_SECURITY_SMACK
default "tomoyo" if DEFAULT_SECURITY_TOMOYO
default "apparmor" if DEFAULT_SECURITY_APPARMOR
- default "yama" if DEFAULT_SECURITY_YAMA
default "" if DEFAULT_SECURITY_DAC
endmenu
diff --git a/security/security.c b/security/security.c
index 860aeb3..92b723a 100644
--- a/security/security.c
+++ b/security/security.c
@@ -136,11 +136,19 @@ int __init register_security(struct security_operations *ops)
int security_ptrace_access_check(struct task_struct *child, unsigned int mode)
{
+ int rc;
+ rc = yama_ptrace_access_check(child, mode);
+ if (rc)
+ return rc;
return security_ops->ptrace_access_check(child, mode);
}
int security_ptrace_traceme(struct task_struct *parent)
{
+ int rc;
+ rc = yama_ptrace_traceme(parent);
+ if (rc)
+ return rc;
return security_ops->ptrace_traceme(parent);
}
@@ -761,6 +769,7 @@ int security_task_create(unsigned long clone_flags)
void security_task_free(struct task_struct *task)
{
+ yama_task_free(task);
security_ops->task_free(task);
}
@@ -876,6 +885,10 @@ int security_task_wait(struct task_struct *p)
int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
unsigned long arg4, unsigned long arg5)
{
+ int rc;
+ rc = yama_task_prctl(option, arg2, arg3, arg4, arg5);
+ if (rc != -ENOSYS)
+ return rc;
return security_ops->task_prctl(option, arg2, arg3, arg4, arg5);
}
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index dcd6178..aef5c6a 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -100,7 +100,7 @@ static void yama_ptracer_del(struct task_struct *tracer,
* yama_task_free - check for task_pid to remove from exception list
* @task: task being removed
*/
-static void yama_task_free(struct task_struct *task)
+void yama_task_free(struct task_struct *task)
{
yama_ptracer_del(task, task);
}
@@ -116,7 +116,7 @@ static void yama_task_free(struct task_struct *task)
* Return 0 on success, -ve on error. -ENOSYS is returned when Yama
* does not handle the given option.
*/
-static int yama_task_prctl(int option, unsigned long arg2, unsigned long arg3,
+int yama_task_prctl(int option, unsigned long arg2, unsigned long arg3,
unsigned long arg4, unsigned long arg5)
{
int rc;
@@ -243,7 +243,7 @@ static int ptracer_exception_found(struct task_struct *tracer,
*
* Returns 0 if following the ptrace is allowed, -ve on error.
*/
-static int yama_ptrace_access_check(struct task_struct *child,
+int yama_ptrace_access_check(struct task_struct *child,
unsigned int mode)
{
int rc;
@@ -293,7 +293,7 @@ static int yama_ptrace_access_check(struct task_struct *child,
*
* Returns 0 if following the ptrace is allowed, -ve on error.
*/
-static int yama_ptrace_traceme(struct task_struct *parent)
+int yama_ptrace_traceme(struct task_struct *parent)
{
int rc;
@@ -378,14 +378,8 @@ static struct ctl_table yama_sysctl_table[] = {
static __init int yama_init(void)
{
- if (!security_module_enable(&yama_ops))
- return 0;
-
printk(KERN_INFO "Yama: becoming mindful.\n");
- if (register_security(&yama_ops))
- panic("Yama: kernel registration failed.\n");
-
#ifdef CONFIG_SYSCTL
if (!register_sysctl_paths(yama_sysctl_path, yama_sysctl_table))
panic("Yama: sysctl registration failed.\n");
--
1.7.0.4
--
Kees Cook
Chrome OS Security
On Fri, 31 Aug 2012 14:31:26 -0700
Kees Cook <[email protected]> wrote:
> Unconditionally call Yama, no matter what LSM module is selected.
>
> Ubuntu and Chrome OS already carry patches to do this, and Fedora has
> voiced interest in doing this as well. Instead of having everyone carry
> these patches, just switch Yama to being unconditional when compiled
> into the kernel.
Not a good plan. What happens when everyone decides to stack every module
by ifdeffing in the kernel - mayhem. I'm all for it being possible but
done right - espeicall as I believe a proper proposal now for stacking
modules, and it should be done as part of that.
Alan
On Fri, Aug 31, 2012 at 2:39 PM, Alan Cox <[email protected]> wrote:
> On Fri, 31 Aug 2012 14:31:26 -0700
> Kees Cook <[email protected]> wrote:
>
>> Unconditionally call Yama, no matter what LSM module is selected.
> Not a good plan. What happens when everyone decides to stack every module
> by ifdeffing in the kernel - mayhem. I'm all for it being possible but
> done right - espeicall as I believe a proper proposal now for stacking
> modules, and it should be done as part of that.
I think a lot of us agree it's a 'difficult' plan going forward. Kees
wrote this patch after we (James, SELinux, AppArmor people) talked at
the Security Summit earlier today. From my PoV we have a couple of
'classes' of LSMs.
Big with Labels: SELinux and SMACK
Big without Labels: AppArmor and TOMOYO
Small and stateless: Capabilities and Yama (and maybe integrity)
Those small and stateless LSMs can easily stack. We don't have object
lifetime issues. We don't have difficult technical details. We all
here seem to want to stack 'small and stateless' with 'big boy'.
Outside one little capabilities and selinux setxattr issue I can't
think of anything even quirky. So the fast path forward, in my mind,
is to start here with Yama. Our choice *today* is adding these static
hooks inside the 'big boy' LSMs (which I think all of us are willing
to do) vs adding it one time in the LSM and not having to worry about
it all over the place. Getting the big guys and the little guys to
play together is not going to lead to a mess of crazy.
Stacking the big boys is a future goal most of us are happy with
seeing done, if it turns out to be reasonable. We know it's close to
possible. Dave Howell's gave us an implementation about 2 years ago.
Casey Schaufler demo'd another stacking interface today. No code from
the latter, but the only limitation he really mentioned today was that
two big guys with labels can't stack together. I don't know how/if
Dave's implementation wanted to handle that case.
I really think this patch is good.
Acked-by: Eric Paris <[email protected]>
I think I even want to do the same thing with capabilities so I don't
have to make sure I'm getting those right in SELinux. And everyone
else probably doesn't want to keep those hooks themselves either. I
should send that patch to Linus. I bet I can give him a large
negative diff. He did just say two days ago that he'll take any -1000
line diff after -rc6 :)
I think Casey and Dave need to both get their larger stacking
solutions posted and we should go with the best one. It'll let us
pull out the static code, but for now, I like the static coding
between the big boys and the little guys. Lets fix today what's easy
and fix tomorrow what we can't fix today.
-Eric
Eric Paris <[email protected]> writes:
> On Fri, Aug 31, 2012 at 2:39 PM, Alan Cox <[email protected]> wrote:
>> On Fri, 31 Aug 2012 14:31:26 -0700
>> Kees Cook <[email protected]> wrote:
>>
>>> Unconditionally call Yama, no matter what LSM module is selected.
>
>> Not a good plan. What happens when everyone decides to stack every module
>> by ifdeffing in the kernel - mayhem. I'm all for it being possible but
>> done right - espeicall as I believe a proper proposal now for stacking
>> modules, and it should be done as part of that.
>
> I think a lot of us agree it's a 'difficult' plan going forward. Kees
> wrote this patch after we (James, SELinux, AppArmor people) talked at
> the Security Summit earlier today. From my PoV we have a couple of
> 'classes' of LSMs.
>
> Big with Labels: SELinux and SMACK
> Big without Labels: AppArmor and TOMOYO
> Small and stateless: Capabilities and Yama (and maybe integrity)
>
> Those small and stateless LSMs can easily stack. We don't have object
> lifetime issues. We don't have difficult technical details. We all
> here seem to want to stack 'small and stateless' with 'big boy'.
> Outside one little capabilities and selinux setxattr issue I can't
> think of anything even quirky. So the fast path forward, in my mind,
> is to start here with Yama. Our choice *today* is adding these static
> hooks inside the 'big boy' LSMs (which I think all of us are willing
> to do) vs adding it one time in the LSM and not having to worry about
> it all over the place. Getting the big guys and the little guys to
> play together is not going to lead to a mess of crazy.
>
> Stacking the big boys is a future goal most of us are happy with
> seeing done, if it turns out to be reasonable. We know it's close to
> possible. Dave Howell's gave us an implementation about 2 years ago.
> Casey Schaufler demo'd another stacking interface today. No code from
> the latter, but the only limitation he really mentioned today was that
> two big guys with labels can't stack together. I don't know how/if
> Dave's implementation wanted to handle that case.
>
> I really think this patch is good.
>
> Acked-by: Eric Paris <[email protected]>
>
> I think I even want to do the same thing with capabilities so I don't
> have to make sure I'm getting those right in SELinux. And everyone
> else probably doesn't want to keep those hooks themselves either. I
> should send that patch to Linus. I bet I can give him a large
> negative diff. He did just say two days ago that he'll take any -1000
> line diff after -rc6 :)
>
> I think Casey and Dave need to both get their larger stacking
> solutions posted and we should go with the best one. It'll let us
> pull out the static code, but for now, I like the static coding
> between the big boys and the little guys. Lets fix today what's easy
> and fix tomorrow what we can't fix today.
>From a overal kernel maintenance and use perspective the unconditional
enablement is a pain.
We long ago established the principle that compiling additional code
into the kernel should not change the semenatics of the kernel.
So this code needs to come with a command line or sysctl on/off switch
not an unconditional enable.
Eric
On Fri, Aug 31, 2012 at 4:59 PM, Eric W. Biederman
<[email protected]> wrote:
> From a overal kernel maintenance and use perspective the unconditional
> enablement is a pain.
>
> We long ago established the principle that compiling additional code
> into the kernel should not change the semenatics of the kernel.
>
> So this code needs to come with a command line or sysctl on/off switch
> not an unconditional enable.
Your argument makes zero sense. If I decide to build new code, that
new code can do something. It happens all the time. If you don't
like Yama, don't build Yama. If you don't like the only thing that
Yama does (it only implements one protection), disable that protection
from sysctl. I don't get it.
-Eric
Eric Paris <[email protected]> writes:
> On Fri, Aug 31, 2012 at 4:59 PM, Eric W. Biederman
> <[email protected]> wrote:
>
>> From a overal kernel maintenance and use perspective the unconditional
>> enablement is a pain.
>>
>> We long ago established the principle that compiling additional code
>> into the kernel should not change the semenatics of the kernel.
>>
>> So this code needs to come with a command line or sysctl on/off switch
>> not an unconditional enable.
>
> Your argument makes zero sense. If I decide to build new code, that
> new code can do something.
Sure but it should not change the existing behavior without being
configured to.
This comes out of the practice that kernels that need to support a
wide variety of use cases enable everything by default.
Having to vet kernel options for will this make my kernel do strange
things if this option is enabled, massively increase the burden on
people building and supporting kernels.
> It happens all the time. If you don't like Yama, don't build Yama.
> If you don't like the only thing that Yama does (it only implements
> one protection), disable that protection from sysctl. I don't get it.
Having taken the time now to vet Yama ugh. Having Yama enabled if
simply compiled in breaks using gdb to attach to a process runing
in another window.
Talk about something you don't want to surprise someone with.
It is very much not ok to have that be enabled by default just
because it happens to be compiled in.
Eric
On Fri, Aug 31, 2012 at 8:31 PM, Eric W. Biederman
<[email protected]> wrote:
> Eric Paris <[email protected]> writes:
>
>> On Fri, Aug 31, 2012 at 4:59 PM, Eric W. Biederman
>> <[email protected]> wrote:
>>
>>> From a overal kernel maintenance and use perspective the unconditional
>>> enablement is a pain.
>>>
>>> We long ago established the principle that compiling additional code
>>> into the kernel should not change the semenatics of the kernel.
>>>
>>> So this code needs to come with a command line or sysctl on/off switch
>>> not an unconditional enable.
>>
>> Your argument makes zero sense. If I decide to build new code, that
>> new code can do something.
>
> Sure but it should not change the existing behavior without being
> configured to.
>
> This comes out of the practice that kernels that need to support a
> wide variety of use cases enable everything by default.
>
> Having to vet kernel options for will this make my kernel do strange
> things if this option is enabled, massively increase the burden on
> people building and supporting kernels.
>
>> It happens all the time. If you don't like Yama, don't build Yama.
>> If you don't like the only thing that Yama does (it only implements
>> one protection), disable that protection from sysctl. I don't get it.
>
> Having taken the time now to vet Yama ugh. Having Yama enabled if
> simply compiled in breaks using gdb to attach to a process runing
> in another window.
>
> Talk about something you don't want to surprise someone with.
>
> It is very much not ok to have that be enabled by default just
> because it happens to be compiled in.
I think it is better to look at the kernel's defaults from the
perspective of the user, not the developer. If we only looked to the
developer, we'd turn on all the debugging by default. No end user
wants that. It's much easier for a developer to twiddle configs and
sysctls.
Given that several distros use (or want to use) Yama, I think that's
reason enough for this. I think it's important for us to take a
practical approach here, and having the big LSMs each hook Yama
instead of doing this in a single global place will make it needlessly
duplicated code.
-Kees
--
Kees Cook
Chrome OS Security