[PATCH] Define CAP_SYSLOG

[PATCH] Define CAP_SYSLOG

[Serge E. Hallyn]

Privileged syslog operations currently require CAP_SYS_ADMIN.  Split
this off into a new CAP_SYSLOG privilege which we can sanely take away
from a container through the capability bounding set.

With this patch, an lxc container can be prevented from messing with
the host's syslog (i.e. dmesg -c).

There is one downside to this patch:  If some site or distro currently
has syslogd/whatever running as a non-root user with cap_sys_admin+pe,
then it will need to be changed to run with cap_syslog+pe.  I don't
know if there are such sites, or if that concern means we should take
a different approach to introducing this change, or simply refuse this
change.

I'm also sending out corresponding patches for libcap and refpolicy, and
will whip up a man-pages patch next week.

Changelog: mar 12 2010: add selinux capability2:cap_syslog perm

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
Acked-by: Andrew G. Morgan <morgan@kernel.org>
Acked-By: Kees Cook <kees.cook@canonical.com>
Cc: James Morris <jmorris@namei.org>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: "Christopher J. PeBenito" <cpebenito@tresys.com>
Cc: Eric Paris <eparis@parisplace.org>
---
 include/linux/capability.h          |    7 +++++--
 security/commoncap.c                |    2 +-
 security/selinux/include/classmap.h |    2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index 39e5ff5..837a55c 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -249,7 +249,6 @@ struct cpu_vfs_cap_data {
 /* Allow configuration of the secure attention key */
 /* Allow administration of the random device */
 /* Allow examination and configuration of disk quotas */
-/* Allow configuring the kernel's syslog (printk behaviour) */
 /* Allow setting the domainname */
 /* Allow setting the hostname */
 /* Allow calling bdflush() */
@@ -355,7 +354,11 @@ struct cpu_vfs_cap_data {
 
 #define CAP_MAC_ADMIN        33
 
-#define CAP_LAST_CAP         CAP_MAC_ADMIN
+/* Allow configuring the kernel's syslog (printk behaviour) */
+
+#define CAP_SYSLOG           34
+
+#define CAP_LAST_CAP         CAP_SYSLOG
 
 #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
 
diff --git a/security/commoncap.c b/security/commoncap.c
index 6166973..018985e 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -899,7 +899,7 @@ int cap_syslog(int type, bool from_file)
 	if (type != SYSLOG_ACTION_OPEN && from_file)
 		return 0;
 	if ((type != SYSLOG_ACTION_READ_ALL &&
-	     type != SYSLOG_ACTION_SIZE_BUFFER) && !capable(CAP_SYS_ADMIN))
+	     type != SYSLOG_ACTION_SIZE_BUFFER) && !capable(CAP_SYSLOG))
 		return -EPERM;
 	return 0;
 }
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 8b32e95..f03cf56 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -142,7 +142,7 @@ struct security_class_mapping secclass_map[] = {
 	    "node_bind", "name_connect", NULL } },
 	{ "memprotect", { "mmap_zero", NULL } },
 	{ "peer", { "recv", NULL } },
-	{ "capability2", { "mac_override", "mac_admin", NULL } },
+	{ "capability2", { "mac_override", "mac_admin", "syslog", NULL } },
 	{ "kernel_service", { "use_as_override", "create_files_as", NULL } },
 	{ "tun_socket",
 	  { COMMON_SOCK_PERMS, NULL } },
-- 
1.6.1

Re: [PATCH] Define CAP_SYSLOG

[Michael Kerrisk]

> There is one downside to this patch:  If some site or distro currently
> has syslogd/whatever running as a non-root user with cap_sys_admin+pe,
> then it will need to be changed to run with cap_syslog+pe.  I don't
> know if there are such sites, or if that concern means we should take
> a different approach to introducing this change, or simply refuse this
> change.

*If* this is a problem, would the way to address it not be to permit
syslog if the caller has *either* CAP_SYS_ADMIN or CAP_SYSLOG? (The
only weakness I see in this idea is that it fails to lighten the
hugely overlaoded CAP_SYS_ADMIN.)

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface" http://blog.man7.org/

Re: [PATCH] Define CAP_SYSLOG

[Serge E. Hallyn]

Quoting Michael Kerrisk (mtk.manpages@googlemail.com):
> > There is one downside to this patch:  If some site or distro currently
> > has syslogd/whatever running as a non-root user with cap_sys_admin+pe,
> > then it will need to be changed to run with cap_syslog+pe.  I don't
> > know if there are such sites, or if that concern means we should take
> > a different approach to introducing this change, or simply refuse this
> > change.
> 
> *If* this is a problem, would the way to address it not be to permit
> syslog if the caller has *either* CAP_SYS_ADMIN or CAP_SYSLOG? (The
> only weakness I see in this idea is that it fails to lighten the
> hugely overlaoded CAP_SYS_ADMIN.)

Which becomes a very big weakness because it won't allow a
container to be started with cap_sys_admin but not cap_syslog
in its capability bounding set.

So, if it is deemed a problem, then the alternative will be to
introduce a syslog namespace.  Container setup can then create
a new syslog namespace, and can no longer read or clear the
host's syslog.

thanks,
-serge

Re: [PATCH] Define CAP_SYSLOG

[Matthew Helsley]

On Sat, Mar 13, 2010 at 10:35 PM, Serge E. Hallyn <serue@us.ibm.com> wrote:
> Quoting Michael Kerrisk (mtk.manpages@googlemail.com):
>> > There is one downside to this patch:  If some site or distro currently
>> > has syslogd/whatever running as a non-root user with cap_sys_admin+pe,
>> > then it will need to be changed to run with cap_syslog+pe.  I don't
>> > know if there are such sites, or if that concern means we should take
>> > a different approach to introducing this change, or simply refuse this
>> > change.
>>
>> *If* this is a problem, would the way to address it not be to permit
>> syslog if the caller has *either* CAP_SYS_ADMIN or CAP_SYSLOG? (The
>> only weakness I see in this idea is that it fails to lighten the
>> hugely overlaoded CAP_SYS_ADMIN.)
>
> Which becomes a very big weakness because it won't allow a
> container to be started with cap_sys_admin but not cap_syslog
> in its capability bounding set.
>
> So, if it is deemed a problem, then the alternative will be to
> introduce a syslog namespace.  Container setup can then create
> a new syslog namespace, and can no longer read or clear the
> host's syslog.
>
> thanks,
> -serge

Would it make sense to warn once when CAP_SYS_ADMIN permits what
CAP_SYSLOG will be used for in the future? Something like:

-            type != SYSLOG_ACTION_SIZE_BUFFER) && !capable(CAP_SYS_ADMIN))
+            type != SYSLOG_ACTION_SIZE_BUFFER) &&
!(capable(CAP_SYSLOG)||capable(CAP_SYS_ADMIN))) {
+             WARN_ONCE(capable(CAP_SYS_ADMIN) &&
!capable(CAP_SYSLOG), "CAP_SYS_ADMIN will not permit syslog
configuration in the near future. Please switch your code to
CAP_SYSLOG\n");
               return -EPERM;
+        }
       return 0;

After a period of time allowing userspace apps to transition to
CAP_SYSLOG remove the CAP_SYS_ADMIN portions. Of course this won't fix
containers for that transition period but it would avoid a sudden
change of what CAP_SYS_ADMIN allows.

> So, if it is deemed a problem, then the alternative will be to
> introduce a syslog namespace.  Container setup can then create
> a new syslog namespace, and can no longer read or clear the
> host's syslog.

Yup, this is also an option. Possibly better as it doesn't involved
changing the meaning of a overly-[ab]used capability bit and wouldn't
require a transition period.

Cheers.
    -Matt Helsley

Re: [PATCH] Define CAP_SYSLOG

[Serge E. Hallyn]

Quoting Matthew Helsley (matt.helsley@gmail.com):
> On Sat, Mar 13, 2010 at 10:35 PM, Serge E. Hallyn <serue@us.ibm.com> wrote:
> > Quoting Michael Kerrisk (mtk.manpages@googlemail.com):
> >> > There is one downside to this patch:  If some site or distro currently
> >> > has syslogd/whatever running as a non-root user with cap_sys_admin+pe,
> >> > then it will need to be changed to run with cap_syslog+pe.  I don't
> >> > know if there are such sites, or if that concern means we should take
> >> > a different approach to introducing this change, or simply refuse this
> >> > change.
> >>
> >> *If* this is a problem, would the way to address it not be to permit
> >> syslog if the caller has *either* CAP_SYS_ADMIN or CAP_SYSLOG? (The
> >> only weakness I see in this idea is that it fails to lighten the
> >> hugely overlaoded CAP_SYS_ADMIN.)
> >
> > Which becomes a very big weakness because it won't allow a
> > container to be started with cap_sys_admin but not cap_syslog
> > in its capability bounding set.
> >
> > So, if it is deemed a problem, then the alternative will be to
> > introduce a syslog namespace.  Container setup can then create
> > a new syslog namespace, and can no longer read or clear the
> > host's syslog.
> >
> > thanks,
> > -serge
> 
> Would it make sense to warn once when CAP_SYS_ADMIN permits what
> CAP_SYSLOG will be used for in the future? Something like:
> 
> -            type != SYSLOG_ACTION_SIZE_BUFFER) && !capable(CAP_SYS_ADMIN))
> +            type != SYSLOG_ACTION_SIZE_BUFFER) &&
> !(capable(CAP_SYSLOG)||capable(CAP_SYS_ADMIN))) {
> +             WARN_ONCE(capable(CAP_SYS_ADMIN) &&
> !capable(CAP_SYSLOG), "CAP_SYS_ADMIN will not permit syslog
> configuration in the near future. Please switch your code to
> CAP_SYSLOG\n");
>                return -EPERM;
> +        }
>        return 0;

Good idea - though I'd prefer to WARN_ONCE and then deny :)  If we log
and allow, the log msg will be ignored.

> After a period of time allowing userspace apps to transition to
> CAP_SYSLOG remove the CAP_SYS_ADMIN portions. Of course this won't fix
> containers for that transition period but it would avoid a sudden
> change of what CAP_SYS_ADMIN allows.
> 
> > So, if it is deemed a problem, then the alternative will be to
> > introduce a syslog namespace.  Container setup can then create
> > a new syslog namespace, and can no longer read or clear the
> > host's syslog.
> 
> Yup, this is also an option. Possibly better as it doesn't involved
> changing the meaning of a overly-[ab]used capability bit and wouldn't
> require a transition period.

Still I can definately see a case where you want the container to
be in the host's syslog for simplicity, but not be able to clear it.

-serge