RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

[Zack Weinberg]

Presently, the security checks for syslog(2) apply also to access to
/proc/kmsg, because /proc/kmsg's file_operations functions just call
do_syslog, and the call to security_syslog is in do_syslog, not
sys_syslog.  [The only callers of do_syslog are sys_syslog and
kmsg_{read,poll,open,release}.]  This has the effect, with the default
security policy, that no matter what the file permissions on
/proc/kmsg are, only a process with CAP_SYS_ADMIN can actually open or
read it.  [Yes, if you open /proc/kmsg as root and then drop
privileges, subsequent reads on that fd fail.]  In consequence, if one
wishes to run klogd as an unprivileged user, one is forced to jump
through awkward hoops - for example, Ubuntu's /etc/init.d/klogd
interposes a root-privileged "dd" process and a named pipe between
/proc/kmsg and the actual klogd.

I propose to move the security_syslog() check from do_syslog to
sys_syslog, so that the syscall remains restricted to CAP_SYS_ADMIN in
the default policy, but /proc/kmsg is governed by its file
permissions.  With the attached patch, I can run klogd as an
unprivileged user, having changed the ownership of /proc/kmsg to that
user before starting it, and it still works.  Equally, I can leave the
ownership alone but modify klogd to get messages from stdin, start it
with stdin open on /proc/kmsg (again unprivileged) and it works.

I think this is safe in the default security policy - /proc/kmsg
starts out owned by root and mode 400 - but I am not sure of the
impact on SELinux or other alternate policy frameworks.

Patch versus 2.6.19-rc4; please consider for the next release.

zw

Signed-off-by: Zack Weinberg <zackw@panix.com>

--- kernel/printk.c.unmod	2006-11-06 15:00:44.000000000 -0800
+++ kernel/printk.c	2006-11-06 15:01:51.000000000 -0800
@@ -187,10 +187,6 @@ int do_syslog(int type, char __user *buf
 	char c;
 	int error = 0;

-	error = security_syslog(type);
-	if (error)
-		return error;
-
 	switch (type) {
 	case 0:		/* Close log */
 		break;
@@ -317,6 +313,10 @@ out:

 asmlinkage long sys_syslog(int type, char __user *buf, int len)
 {
+	int error = security_syslog(type);
+	if (error)
+		return error;
+
 	return do_syslog(type, buf, len);
 }

Re: RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

[Chris Wright]

* Zack Weinberg (zackw@panix.com) wrote:
> Presently, the security checks for syslog(2) apply also to access to
> /proc/kmsg, because /proc/kmsg's file_operations functions just call
> do_syslog, and the call to security_syslog is in do_syslog, not
> sys_syslog.  [The only callers of do_syslog are sys_syslog and
> kmsg_{read,poll,open,release}.]  This has the effect, with the default
> security policy, that no matter what the file permissions on
> /proc/kmsg are, only a process with CAP_SYS_ADMIN can actually open or
> read it.  [Yes, if you open /proc/kmsg as root and then drop
> privileges, subsequent reads on that fd fail.]  In consequence, if one
> wishes to run klogd as an unprivileged user, one is forced to jump
> through awkward hoops - for example, Ubuntu's /etc/init.d/klogd
> interposes a root-privileged "dd" process and a named pipe between
> /proc/kmsg and the actual klogd.

The act of reading from /proc/kmsg alters the state of the ring buffer.
This is not the same as smth like dmesg, which simply dumps the messages.
That's why only getting current size and dumping are treated as
less-privileged.

> I propose to move the security_syslog() check from do_syslog to
> sys_syslog, so that the syscall remains restricted to CAP_SYS_ADMIN in
> the default policy, but /proc/kmsg is governed by its file
> permissions.  With the attached patch, I can run klogd as an
> unprivileged user, having changed the ownership of /proc/kmsg to that
> user before starting it, and it still works.  Equally, I can leave the
> ownership alone but modify klogd to get messages from stdin, start it
> with stdin open on /proc/kmsg (again unprivileged) and it works.
> 
> I think this is safe in the default security policy - /proc/kmsg
> starts out owned by root and mode 400 - but I am not sure of the
> impact on SELinux or other alternate policy frameworks.

SELinux doesn't distinguish the entrypoint to the ringbuffer,
so this patch would break its current behaviour.

thanks,
-chris

Re: RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 3589 bytes --]

On Wed, 8 Nov 2006 02:20:37 -0800 Chris Wright wrote:

> * Zack Weinberg (zackw@panix.com) wrote:
> > Presently, the security checks for syslog(2) apply also to access to
> > /proc/kmsg, because /proc/kmsg's file_operations functions just call
> > do_syslog, and the call to security_syslog is in do_syslog, not
> > sys_syslog.  [The only callers of do_syslog are sys_syslog and
> > kmsg_{read,poll,open,release}.]  This has the effect, with the default
> > security policy, that no matter what the file permissions on
> > /proc/kmsg are, only a process with CAP_SYS_ADMIN can actually open or
> > read it.  [Yes, if you open /proc/kmsg as root and then drop
> > privileges, subsequent reads on that fd fail.]  In consequence, if one
> > wishes to run klogd as an unprivileged user, one is forced to jump
> > through awkward hoops - for example, Ubuntu's /etc/init.d/klogd
> > interposes a root-privileged "dd" process and a named pipe between
> > /proc/kmsg and the actual klogd.

And ALT Linux uses exactly the same patch as you have posted :)

klogd starts as root, opens /proc/kmsg, then chroots to a directory
which contains just the dev/log socket and drops all privileges.

> The act of reading from /proc/kmsg alters the state of the ring buffer.
> This is not the same as smth like dmesg, which simply dumps the messages.
> That's why only getting current size and dumping are treated as
> less-privileged.

But in order to read from /proc/kmsg, you need to open() it first - and
that operation is restricted both by DAC checks on /proc/kmsg and by
do_syslog(1,NULL,0) call in kmsg_open().  (However, the patch by Zack
effectively removes that access check, which is probably not good for
SELinux.)

> > I propose to move the security_syslog() check from do_syslog to
> > sys_syslog, so that the syscall remains restricted to CAP_SYS_ADMIN in
> > the default policy, but /proc/kmsg is governed by its file
> > permissions.  With the attached patch, I can run klogd as an
> > unprivileged user, having changed the ownership of /proc/kmsg to that
> > user before starting it, and it still works.  Equally, I can leave the
> > ownership alone but modify klogd to get messages from stdin, start it
> > with stdin open on /proc/kmsg (again unprivileged) and it works.
> >
> > I think this is safe in the default security policy - /proc/kmsg
> > starts out owned by root and mode 400 - but I am not sure of the
> > impact on SELinux or other alternate policy frameworks.
>
> SELinux doesn't distinguish the entrypoint to the ringbuffer,
> so this patch would break its current behaviour.

Then what would you think about another solution:

 1) When sys_syslog() is called with commands 2 (read) or 9 (get unread
    count), additionally call security_syslog(1) to check that the
    process has permissions to open the kernel log.  This change by
    itself will not make any difference, because all existing
    implementations of the security_ops->syslog hook treat the operation
    codes 1, 2 and 9 the same way.

 2) Change cap_syslog() and dummy_syslog() to permit commands 2 and 9
    for unprivileged users, in addition to 3 and 10 which are currently
    permitted.  This will not really permit access through sys_syslog()
    due to the added security_syslog(1) check, but if a process somehow
    got access to an open file descriptor for /proc/kmsg, it would be
    able to read from it.  Also, because selinux_syslog() is not
    changed, under SELinux the process will still need to have
    additional privileges even if it has /proc/kmsg open.

Two patches will follow.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[RFC PATCH 1/2] sys_syslog: check open permission for reading and getting unread count

[Sergey Vlasov]

The "read" (2) and "get unread count" (9) operations of sys_syslog() may
also be invoked through /proc/kmsg; however, /proc/kmsg also performs
security checks during open.  Perform the same security check when these
operations are invoked through the syslog system call.

Currently this does not change the behavior - for cap_syslog() and
dummy_syslog() the "read" and "get unread count" operations are
identical to "open", and selinux_syslog() maps all of them to
SYSTEM__SYSLOG_MOD.  The next patch will enable syslog read for normal
users, so that a process could open /proc/kmsg, then drop privileges and
continue reading kernel messages from the open file descriptor - then
the added check in sys_syslog() will catch attempts to read kernel
messages without having /proc/kmsg open.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
---
 kernel/printk.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 1149365..91c3f39 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -316,7 +316,21 @@ out:
 
 asmlinkage long sys_syslog(int type, char __user *buf, int len)
 {
-	return do_syslog(type, buf, len);
+	int retval;
+
+	if ((type == 2) || (type == 9)) {
+		/*
+		 * These operation can also be invoked through /proc/kmsg, but
+		 * you need to have an open file descriptor for that.  Make the
+		 * syslog system call also require the syslog open permission.
+		 */
+		retval = security_syslog(1);
+		if (retval)
+			goto out;
+	}
+	retval = do_syslog(type, buf, len);
+out:
+	return retval;
 }
 
 /*
-- 
1.4.3.3.gddcc6

[RFC PATCH 2/2] security: allow reads from an open /proc/kmsg fd by unprivileged processes

[Sergey Vlasov]

With the added check for syslog open rights in sys_syslog() it is
possible to relax restrictions on syslog access in cap_syslog() and
dummy_syslog(), so that a process could open /proc/kmsg, then drop all
privileges including CAP_SYS_ADMIN, and still be able to use the
/proc/kmsg file descriptor for reading kernel messages.

selinux_syslog() is not modified - a process which handles kernel
messages still needs to have the "syslog_mod" permission.

Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
---
 security/commoncap.c |   24 +++++++++++++++++++++---
 security/dummy.c     |   24 +++++++++++++++++++++---
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index f50fc29..966cfce 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -311,9 +311,27 @@ void cap_task_reparent_to_init (struct t
 
 int cap_syslog (int type)
 {
-	if ((type != 3 && type != 10) && !capable(CAP_SYS_ADMIN))
-		return -EPERM;
-	return 0;
+	switch (type) {
+	case 3:		/* Read last kernel messages */
+	case 10:	/* Size of the log buffer */
+		/* Allow dmesg for unprivileged users. */
+		return 0;
+
+	case 2:		/* Read from log */
+	case 9:		/* Number of chars in the log buffer */
+		/*
+		 * Allow read() and poll() on a /proc/kmsg file descriptor
+		 * opened by a privileged process.  This does not enable
+		 * uncontrolled access through the syslog system call, because
+		 * sys_syslog() additionally checks the syslog open permission.
+		 */
+		return 0;
+
+	default:
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+		return 0;
+	}
 }
 
 int cap_vm_enough_memory(long pages)
diff --git a/security/dummy.c b/security/dummy.c
index 58c6d39..3da65fe 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -96,9 +96,27 @@ static int dummy_quota_on (struct dentry
 
 static int dummy_syslog (int type)
 {
-	if ((type != 3 && type != 10) && current->euid)
-		return -EPERM;
-	return 0;
+	switch (type) {
+	case 3:		/* Read last kernel messages */
+	case 10:	/* Size of the log buffer */
+		/* Allow dmesg for unprivileged users. */
+		return 0;
+
+	case 2:		/* Read from log */
+	case 9:		/* Number of chars in the log buffer */
+		/*
+		 * Allow read() and poll() on a /proc/kmsg file descriptor
+		 * opened by a privileged process.  This does not enable
+		 * uncontrolled access through the syslog system call, because
+		 * sys_syslog() additionally checks the syslog open permission.
+		 */
+		return 0;
+
+	default:
+		if (current->euid)
+			return -EPERM;
+		return 0;
+	}
 }
 
 static int dummy_settime(struct timespec *ts, struct timezone *tz)
-- 
1.4.3.3.gddcc6

Re: RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

[Chris Wright]

* Sergey Vlasov (vsu@altlinux.ru) wrote:
> Then what would you think about another solution:
> 
>  1) When sys_syslog() is called with commands 2 (read) or 9 (get unread
>     count), additionally call security_syslog(1) to check that the
>     process has permissions to open the kernel log.  This change by
>     itself will not make any difference, because all existing
>     implementations of the security_ops->syslog hook treat the operation
>     codes 1, 2 and 9 the same way.
> 
>  2) Change cap_syslog() and dummy_syslog() to permit commands 2 and 9
>     for unprivileged users, in addition to 3 and 10 which are currently
>     permitted.  This will not really permit access through sys_syslog()
>     due to the added security_syslog(1) check, but if a process somehow
>     got access to an open file descriptor for /proc/kmsg, it would be
>     able to read from it.  Also, because selinux_syslog() is not
>     changed, under SELinux the process will still need to have
>     additional privileges even if it has /proc/kmsg open.

It's a bit clumsy in the extra caveats for sys_syslog and cap_syslog,
but does achieve what you're after.  We lose default checking on the
actual read access, but perhaps this is a fair tradeoff.  Stephen,
James do you have any issues with this for SELinux?

thanks,
-chris

Re: RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

[Stephen Smalley]

On Wed, 2006-11-08 at 20:14 -0800, Chris Wright wrote:
> * Sergey Vlasov (vsu@altlinux.ru) wrote:
> > Then what would you think about another solution:
> > 
> >  1) When sys_syslog() is called with commands 2 (read) or 9 (get unread
> >     count), additionally call security_syslog(1) to check that the
> >     process has permissions to open the kernel log.  This change by
> >     itself will not make any difference, because all existing
> >     implementations of the security_ops->syslog hook treat the operation
> >     codes 1, 2 and 9 the same way.
> > 
> >  2) Change cap_syslog() and dummy_syslog() to permit commands 2 and 9
> >     for unprivileged users, in addition to 3 and 10 which are currently
> >     permitted.  This will not really permit access through sys_syslog()
> >     due to the added security_syslog(1) check, but if a process somehow
> >     got access to an open file descriptor for /proc/kmsg, it would be
> >     able to read from it.  Also, because selinux_syslog() is not
> >     changed, under SELinux the process will still need to have
> >     additional privileges even if it has /proc/kmsg open.
> 
> It's a bit clumsy in the extra caveats for sys_syslog and cap_syslog,
> but does achieve what you're after.  We lose default checking on the
> actual read access, but perhaps this is a fair tradeoff.  Stephen,
> James do you have any issues with this for SELinux?

It makes the already unfortunate coupling between the security modules
and the core syslog code even worse, by making assumptions about how the
security modules treat different type codes.  If you were to go down
this route, I think you would want to map the type codes to abstract
permissions in the core syslog code and only pass the abstract
permissions to the security modules (so that they no longer see 2, 9,
and 1 separately but as a single permission).  Might be nice to have
actual #define's for the type codes too.

We wouldn't want the SELinux checking changed; we can already run klogd
confined by policy, and decomposition into privileged and unprivileged
components is preferable to privilege bracketing within a single
component.

-- 
Stephen Smalley
National Security Agency

Re: RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

[Zack Weinberg]

[some of you have seen this already in off-list discussion, but it
really should have gone to the list, apologies for the noise]

On 11/9/06, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Wed, 2006-11-08 at 20:14 -0800, Chris Wright wrote:
> > * Sergey Vlasov (vsu@altlinux.ru) wrote:
> > > Then what would you think about another solution:
> > >
> > >  1) When sys_syslog() is called with commands 2 (read) or 9 (get unread
> > >     count), additionally call security_syslog(1) to check that the
> > >     process has permissions to open the kernel log.  This change by
> > >     itself will not make any difference, because all existing
> > >     implementations of the security_ops->syslog hook treat the operation
> > >     codes 1, 2 and 9 the same way.
> > >
> > >  2) Change cap_syslog() and dummy_syslog() to permit commands 2 and 9
> > >     for unprivileged users, in addition to 3 and 10 which are currently
> > >     permitted.  This will not really permit access through sys_syslog()
> > >     due to the added security_syslog(1) check, but if a process somehow
> > >     got access to an open file descriptor for /proc/kmsg, it would be
> > >     able to read from it.  Also, because selinux_syslog() is not
> > >     changed, under SELinux the process will still need to have
> > >     additional privileges even if it has /proc/kmsg open.
> >
> > It's a bit clumsy in the extra caveats for sys_syslog and cap_syslog,
> > but does achieve what you're after.  We lose default checking on the
> > actual read access, but perhaps this is a fair tradeoff.  Stephen,
> > James do you have any issues with this for SELinux?
>
> It makes the already unfortunate coupling between the security modules
> and the core syslog code even worse, by making assumptions about how the
> security modules treat different type codes.  If you were to go down
> this route, I think you would want to map the type codes to abstract
> permissions in the core syslog code and only pass the abstract
> permissions to the security modules (so that they no longer see 2, 9,
> and 1 separately but as a single permission).  Might be nice to have
> actual #define's for the type codes too.
>
> We wouldn't want the SELinux checking changed; we can already run klogd
> confined by policy, and decomposition into privileged and unprivileged
> components is preferable to privilege bracketing within a single
> component.

I've been working on an alternate patch.  It is not ready yet, but
tell me how this sounds:

1) Move the security check from do_syslog to sys_syslog, as in my
original patch.
2) Add a call to security_syslog() to kmsg_open.
3) sys_syslog() operations 0 and 1 ("close" and "open" respectively -
both do precisely nothing) cease to call the security hook.  (This
part is so that a security module knows that a call of its ->syslog
hook with code 1 is an attempt to open /proc/kmsg, not a no-op
syslog(1, 0, 0).  Security modules never see code 0, on the principle
that close shouldn't ever fail.)

-- At this point the user visible semantics (in the default security
framework) are that a process without CAP_SYS_ADMIN cannot open
/proc/kmsg and cannot call any of the syslog() interfaces that
actually do stuff [except the one "dmesg" uses].  However, a process
without CAP_SYS_ADMIN will be able to issue a read() on a fd open on
/proc/kmsg if it somehow gets hold of one.  (This is a tidier way of
getting the semantics that Sergey's patches provide.)

Then I also want to have:

4) Change the default security policy modules (commoncap.c and
dummy.c) so that a process without CAP_SYS_ADMIN is allowed to open
/proc/kmsg provided that DAC allows it (i.e. provided that root has
gone and chowned it to the appropriate uid).  No change to SELinux is
required.

Part 4 allows me to run an *unmodified* klogd as an unprivileged user,
and just chown /proc/kmsg in the init script, rather than having to
modify klogd to open /proc/kmsg and then drop privileges.  (A SELinux
environment still requires modifications to klogd, but we were going
to have to do that anyway.)

[Possibly as long as we are futzing with syslog(), we ought to
consider renaming it to match glibc [which calls it klogctl()] and
defining some symbolic constants for all those opcode numbers.  Also I
really don't like the looks of the code for operations 3 and 4.  But
this is all tangential.]

[re Stephen's comment: I don't like the idea of mapping type codes to
abstract permissions in the core; that seems like more of a layering
violation than what we have now.  Surely it is the core's business to
say _what_ it is about to do, and the security module's to decide what
that means in terms of its privilege model.  I don't propose to change
SELinux, except insofar as (in my above plan) it would no longer see
certain operations on the syscall interface that don't do anything
anyway.]

zw

Re: RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

[Stephen Smalley]

On Thu, 2006-11-09 at 08:08 -0800, Zack Weinberg wrote:
> [some of you have seen this already in off-list discussion, but it
> really should have gone to the list, apologies for the noise]
> 
> On 11/9/06, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > On Wed, 2006-11-08 at 20:14 -0800, Chris Wright wrote:
> > > * Sergey Vlasov (vsu@altlinux.ru) wrote:
> > > > Then what would you think about another solution:
> > > >
> > > >  1) When sys_syslog() is called with commands 2 (read) or 9 (get unread
> > > >     count), additionally call security_syslog(1) to check that the
> > > >     process has permissions to open the kernel log.  This change by
> > > >     itself will not make any difference, because all existing
> > > >     implementations of the security_ops->syslog hook treat the operation
> > > >     codes 1, 2 and 9 the same way.
> > > >
> > > >  2) Change cap_syslog() and dummy_syslog() to permit commands 2 and 9
> > > >     for unprivileged users, in addition to 3 and 10 which are currently
> > > >     permitted.  This will not really permit access through sys_syslog()
> > > >     due to the added security_syslog(1) check, but if a process somehow
> > > >     got access to an open file descriptor for /proc/kmsg, it would be
> > > >     able to read from it.  Also, because selinux_syslog() is not
> > > >     changed, under SELinux the process will still need to have
> > > >     additional privileges even if it has /proc/kmsg open.
> > >
> > > It's a bit clumsy in the extra caveats for sys_syslog and cap_syslog,
> > > but does achieve what you're after.  We lose default checking on the
> > > actual read access, but perhaps this is a fair tradeoff.  Stephen,
> > > James do you have any issues with this for SELinux?
> >
> > It makes the already unfortunate coupling between the security modules
> > and the core syslog code even worse, by making assumptions about how the
> > security modules treat different type codes.  If you were to go down
> > this route, I think you would want to map the type codes to abstract
> > permissions in the core syslog code and only pass the abstract
> > permissions to the security modules (so that they no longer see 2, 9,
> > and 1 separately but as a single permission).  Might be nice to have
> > actual #define's for the type codes too.
> >
> > We wouldn't want the SELinux checking changed; we can already run klogd
> > confined by policy, and decomposition into privileged and unprivileged
> > components is preferable to privilege bracketing within a single
> > component.
> 
> I've been working on an alternate patch.  It is not ready yet, but
> tell me how this sounds:
> 
> 1) Move the security check from do_syslog to sys_syslog, as in my
> original patch.
> 2) Add a call to security_syslog() to kmsg_open.
> 3) sys_syslog() operations 0 and 1 ("close" and "open" respectively -
> both do precisely nothing) cease to call the security hook.  (This
> part is so that a security module knows that a call of its ->syslog
> hook with code 1 is an attempt to open /proc/kmsg, not a no-op
> syslog(1, 0, 0).  Security modules never see code 0, on the principle
> that close shouldn't ever fail.)
> 
> -- At this point the user visible semantics (in the default security
> framework) are that a process without CAP_SYS_ADMIN cannot open
> /proc/kmsg and cannot call any of the syslog() interfaces that
> actually do stuff [except the one "dmesg" uses].  However, a process
> without CAP_SYS_ADMIN will be able to issue a read() on a fd open on
> /proc/kmsg if it somehow gets hold of one.  (This is a tidier way of
> getting the semantics that Sergey's patches provide.)
> 
> Then I also want to have:
> 
> 4) Change the default security policy modules (commoncap.c and
> dummy.c) so that a process without CAP_SYS_ADMIN is allowed to open
> /proc/kmsg provided that DAC allows it (i.e. provided that root has
> gone and chowned it to the appropriate uid).  No change to SELinux is
> required.
> 
> Part 4 allows me to run an *unmodified* klogd as an unprivileged user,
> and just chown /proc/kmsg in the init script, rather than having to
> modify klogd to open /proc/kmsg and then drop privileges.  (A SELinux
> environment still requires modifications to klogd, but we were going
> to have to do that anyway.)
> 
> [Possibly as long as we are futzing with syslog(), we ought to
> consider renaming it to match glibc [which calls it klogctl()] and
> defining some symbolic constants for all those opcode numbers.  Also I
> really don't like the looks of the code for operations 3 and 4.  But
> this is all tangential.]
> 
> [re Stephen's comment: I don't like the idea of mapping type codes to
> abstract permissions in the core; that seems like more of a layering
> violation than what we have now.  Surely it is the core's business to
> say _what_ it is about to do, and the security module's to decide what
> that means in terms of its privilege model.  I don't propose to change
> SELinux, except insofar as (in my above plan) it would no longer see
> certain operations on the syscall interface that don't do anything
> anyway.]

Unless I missed something, your plan above would disable SELinux
syslog-related permission checking upon reads of a previously opened
file descriptor to /proc/kmsg.  So it would change SELinux behavior in a
way that is directly contrary to the notion of mandatory access control.

Part 4 appears to further expose /proc/kmsg to access by any uid 0
process even if it has no capabilities (think privilege shedding or
containers).

I think the function name is immaterial, but having symbolic constants
for the type codes would be helpful if we have to refer to them in the
security modules directly.  But having a mapping in the core to a much
smaller set of permissions would be even better, and help with
maintenance; the next time someone added a new code, they would more
likely see the mapping table in the core and update it than go digging
into the individual security modules.

-- 
Stephen Smalley
National Security Agency

Re: RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

[Zack Weinberg]

On 11/9/06, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> Unless I missed something, your plan above would disable SELinux
> syslog-related permission checking upon reads of a previously opened
> file descriptor to /proc/kmsg.  So it would change SELinux behavior in a
> way that is directly contrary to the notion of mandatory access control.

Yes, it would do that; no, I don't see why that change is contrary to
the notion of mandatory access control.  An open fd on /proc/kmsg
(with my changes applied) offers strictly fewer privileges than
SYSTEM__SYSLOG_MOD (no access to opcodes 4 and 5), and with SELinux
active, you can't get that open fd without having had
SYSTEM__SYSLOG_MOD at some prior time.  SELinux does not (as far as I
can tell) do MAC checks for access to normal files at read() time,
only open().

I see this as bringing /proc/kmsg in line with standard Unix file
permission semantics, overall.

> Part 4 appears to further expose /proc/kmsg to access by any uid 0
> process even if it has no capabilities (think privilege shedding or
> containers).

Only in the default privilege model, not in SELinux.  (CAP_SYS_ADMIN
is a hell of a lot more powerful than SYSTEM__SYSLOG_MOD.)  And I
could be talked out of part 4.

> But having a mapping in the core to a much
> smaller set of permissions would be even better, and help with
> maintenance; the next time someone added a new code, they would more
> likely see the mapping table in the core and update it than go digging
> into the individual security modules.

But that mapping is itself a security policy decision, and could
plausibly need to be done differently in different security modules...

zw

Re: RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

[Stephen Smalley]

On Thu, 2006-11-09 at 09:39 -0800, Zack Weinberg wrote:
> On 11/9/06, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > Unless I missed something, your plan above would disable SELinux
> > syslog-related permission checking upon reads of a previously opened
> > file descriptor to /proc/kmsg.  So it would change SELinux behavior in a
> > way that is directly contrary to the notion of mandatory access control.
> 
> Yes, it would do that; no, I don't see why that change is contrary to
> the notion of mandatory access control.  An open fd on /proc/kmsg
> (with my changes applied) offers strictly fewer privileges than
> SYSTEM__SYSLOG_MOD (no access to opcodes 4 and 5), and with SELinux
> active, you can't get that open fd without having had
> SYSTEM__SYSLOG_MOD at some prior time.

Sure you can.  You can inherit or receive a descriptor opened by another
process that had that permission (and even accidental descriptor leakage
isn't as uncommon as you might think; SELinux has turned up numerous
cases of it).

>   SELinux does not (as far as I
> can tell) do MAC checks for access to normal files at read() time,
> only open().

Look for security_file_permission() calls in the core code, and its
implementation in SELinux (selinux_file_permission).  That is just a
revalidation of access to help with relabeling and policy changes,
albeit necessarily incomplete in coverage.  More crucially, SELinux
rechecks descriptors on inheritance across execve
(flush_unauthorized_files) and transfer across local IPC
(selinux_file_receive) to prevent unauthorized propagation of access
rights in the first place.  

> I see this as bringing /proc/kmsg in line with standard Unix file
> permission semantics, overall.

It may fit with Linux DAC checking, but it isn't what we want for MAC.
You also have to be careful about drawing an analogy to typical Linux
permission checking, since this is proc rather than a normal filesystem.

> > But having a mapping in the core to a much
> > smaller set of permissions would be even better, and help with
> > maintenance; the next time someone added a new code, they would more
> > likely see the mapping table in the core and update it than go digging
> > into the individual security modules.
> 
> But that mapping is itself a security policy decision, and could
> plausibly need to be done differently in different security modules...

Even the set of security hook interfaces and placements impose some
limits on security policies that can be implemented.  But just as those
hooks can be adjusted over time for the needs of new modules, the
mapping can be adjusted over time as needed.  No harm done, and some
benefit. 

-- 
Stephen Smalley
National Security Agency

Re: RFC PATCH: apply security_syslog() only to the syslog() syscall, not to /proc/kmsg

[Zack Weinberg]

On 11/9/06, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Thu, 2006-11-09 at 09:39 -0800, Zack Weinberg wrote:
> >  An open fd on /proc/kmsg
> > (with my changes applied) offers strictly fewer privileges than
> > SYSTEM__SYSLOG_MOD (no access to opcodes 4 and 5), and with SELinux
> > active, you can't get that open fd without having had
> > SYSTEM__SYSLOG_MOD at some prior time.
>
> Sure you can.  You can inherit or receive a descriptor opened by another
> process that had that permission (and even accidental descriptor leakage
> isn't as uncommon as you might think; SELinux has turned up numerous
> cases of it).

See, from my perspective, being able to pass this fd around is a
*feature*.  It lets me do things like

start-stop-daemon --start --chuid klog --chroot /var/run/klogd --exec
/sbin/klogd -- -x -P - -o - < /proc/kmsg > /dev/log

(hypothetical, does not work with unpatched klogd)

Being able to open /proc/kmsg subject only to standard DAC checks is
*also* a feature in my book.  In short, what you are saying reads to
me as "I don't want you to do the very thing you are trying to do."

I also think we wouldn't be having this discussion in the first place
if it weren't that /proc/kmsg was historically implemented on top of
sys_syslog() which *has* to do permission checks on the
read-equivalent, 'cos anyone can call that without having done any
open-equivalent first.

[...]
> > I see this as bringing /proc/kmsg in line with standard Unix file
> > permission semantics, overall.
>
> It may fit with Linux DAC checking, but it isn't what we want for MAC.
> You also have to be careful about drawing an analogy to typical Linux
> permission checking, since this is proc rather than a normal filesystem.

So let me back off and try to explain again what my goals are here.

I want to be able to run klogd as an unprivileged user, under the
default security model, possibly in a chroot jail; in particular it
should have to keep neither uid 0 nor CAP_SYS_ADMIN in order to read
from /proc/kmsg.  The logical way to accomplish this, to my mind, is
to subject /proc/kmsg to the normal Unix DAC checks and no more.  From
that reference frame, I see security_syslog as relevant to the
syslog() system call, which doesn't *have* DAC to rely on, and not to
/proc/kmsg.

Now, I see that the SELinux model is rather different, but I'm asking
you to consider the possibility of using the same "label" stuff for
/proc/kmsg that is used for all other file-based permissions in
SELinux.  If it would make it easier, I am prepared to do the
gruntwork of turning the thing into a misc character device or
something like that.  That way, security_syslog() could still be only
about access control for the syslog() system call.

Note that I'm perfectly fine with needing to apply special privileges
to klogd in a SELinux universe -- it's just that in the default model
one might as well not bother dropping privileges if one has to leave
CAP_SYS_ADMIN asserted.

> > But that mapping is itself a security policy decision, and could
> > plausibly need to be done differently in different security modules...
>
> Even the set of security hook interfaces and placements impose some
> limits on security policies that can be implemented.  But just as those
> hooks can be adjusted over time for the needs of new modules, the
> mapping can be adjusted over time as needed.  No harm done, and some
> benefit.

Hmm, okay, so the existing groupings that selinux/hooks.c has make
sense to me, except I'd split up SYSTEM__SYSLOG_MOD a bit ...

#define KLOG_CLOSE           0  /* Close the log  */
#define KLOG_OPEN            1  /* Open the log  */
#define KLOG_READ            2  /* Read from the log (klogd) */
#define KLOG_GET_UNREAD      9  /* Return number of unread characters */

 -> "can be klogd"

#define KLOG_READ_ALL        3  /* Read history of log messages (dmesg) */
#define KLOG_GET_SIZE       10  /* Return size of log buffer */

 -> "can be dmesg"

#define KLOG_READ_CLEAR_ALL  4  /* Read and clear history of log messages */
#define KLOG_CLEAR           5  /* Clear history of log messages */

 -> "can clear message history"

#define KLOG_DISABLE_CONSOLE 6  /* Disable printk's to console */
#define KLOG_ENABLE_CONSOLE  7  /* Enable printk's to console */
#define KLOG_SET_CONSOLE_LVL 8  /* Set level of messages printed to console */

 -> "can adjust console loglevel"

(#defines taken from the local <linux/syslog.h> that I've made up;
currently "can be klogd" and "can clear message history" are lumped
together; perhaps one should need both "can be dmesg" and "can clear
message history" for READ_CLEAR_ALL, but that might be too picky)

zw