[patch 2/4] permission mapping for sys_syslog operations

2006-12-24 Thread Zack Weinberg
As suggested by Stephen Smalley: map the various sys_syslog operations
to a smaller set of privilege codes before calling security modules.
This patch changes the security module interface!  There should be no
change in the actual security semantics enforced by dummy, capability,
nor SELinux (with one exception, clearly marked in sys_syslog).

The privilege codes are now in linux/security.h instead of
linux/klog.h, and use the LSM_* naming convention used for other such
constants in that file.

I'm now using a static table to map operations to privilege codes.
This makes it very easy to see the mapping as a whole, and prevents
security operations from cluttering up the body of do_syslog, but does
necessitate some checks to ensure that when new KLOG_* operations are
added, people don't forget to add entries to the table.  If people are
not fans of the idiom I used, I will see about getting a feature added
to gcc to make it less ugly.

Note that after this patch, close() on /proc/kmsg and
sys_syslog(KLOG_CLOSE) do not do security checks; they always succeed.
(IMO close should never fail.) This was always in this patch series,
but used to be in a different stage.

zw

Index: linux-2.6/kernel/printk.c
===
--- linux-2.6.orig/kernel/printk.c  2006-12-24 11:43:14.0 -0800
+++ linux-2.6/kernel/printk.c   2006-12-24 11:43:16.0 -0800
@@ -164,6 +164,39 @@
 
 __setup("log_buf_len=", log_buf_len_setup);
 
+/*
+ * This table maps KLOG_* operation codes to LSM_KLOG_* privilege classes.
+ * "unsigned char" is used just to keep it small.
+ */
+
+static const unsigned char klog_privclass[] = {
+   [KLOG_CLOSE]   = 0,  /* close always succeeds */
+   [KLOG_OPEN]= LSM_KLOG_READ,
+   [KLOG_READ]= LSM_KLOG_READ,
+   [KLOG_GET_UNREAD]  = LSM_KLOG_READ,
+
+   [KLOG_READ_HIST]   = LSM_KLOG_READHIST,
+   [KLOG_GET_SIZE]= LSM_KLOG_READHIST,
+   [KLOG_READ_CLEAR_HIST] = LSM_KLOG_READHIST|LSM_KLOG_CLEARHIST,
+   [KLOG_CLEAR_HIST]  = LSM_KLOG_CLEARHIST,
+
+   [KLOG_DISABLE_CONSOLE] = LSM_KLOG_CONSOLE,
+   [KLOG_ENABLE_CONSOLE]  = LSM_KLOG_CONSOLE,
+   [KLOG_SET_CONSOLE_LVL] = LSM_KLOG_CONSOLE,
+};
+
+/* It is essential that there be an entry in the above table for every
+ * KLOG_* code.  The following, er, odd declarations cause compilation
+ * of this file to fail if that is not true.  They do not correspond
+ * to real data objects.
+ */
+extern char klog_privclass_is_missing_an_entry[
+   ARRAY_SIZE(klog_privclass) < KLOG_OPCODE_MAX ? -1 : 1
+];
+extern char klog_privclass_has_too_many_entries[
+   ARRAY_SIZE(klog_privclass) > KLOG_OPCODE_MAX ? -1 : 1
+];
+
 /**
  * do_syslog - operate on the log of kernel messages
  * @type: operation to perform
@@ -185,9 +218,11 @@
unsigned long i, j, limit, count;
int do_clear = 0;
char c;
-   int error = 0;
+   int error;
 
-   error = security_syslog(type);
+   if (type < 0 || type >= KLOG_OPCODE_MAX)
+   return -EINVAL;
+   error = security_syslog(klog_privclass[type]);
if (error)
return error;
 
Index: linux-2.6/security/commoncap.c
===
--- linux-2.6.orig/security/commoncap.c 2006-12-23 08:55:16.0 -0800
+++ linux-2.6/security/commoncap.c  2006-12-24 11:43:16.0 -0800
@@ -311,7 +311,7 @@
 
 int cap_syslog (int type)
 {
-   if ((type != 3 && type != 10) && !capable(CAP_SYS_ADMIN))
+   if ((type & ~LSM_KLOG_READHIST) && !capable(CAP_SYS_ADMIN))
return -EPERM;
return 0;
 }
Index: linux-2.6/security/dummy.c
===
--- linux-2.6.orig/security/dummy.c 2006-12-23 08:55:16.0 -0800
+++ linux-2.6/security/dummy.c  2006-12-24 11:43:16.0 -0800
@@ -96,7 +96,7 @@
 
 static int dummy_syslog (int type)
 {
-   if ((type != 3 && type != 10) && current->euid)
+   if ((type & ~LSM_KLOG_READHIST) && current->euid)
return -EPERM;
return 0;
 }
Index: linux-2.6/security/selinux/hooks.c
===
--- linux-2.6.orig/security/selinux/hooks.c 2006-12-23 08:55:16.0 
-0800
+++ linux-2.6/security/selinux/hooks.c  2006-12-24 11:43:16.0 -0800
@@ -1504,29 +1504,27 @@
 {
int rc;
 
+   /* if there are any codes we don't know about, there's a bug */
+   BUG_ON(type & ~(LSM_KLOG_READ|LSM_KLOG_READHIST
+   |LSM_KLOG_CLEARHIST|LSM_KLOG_CONSOLE));
+
rc = secondary_ops->syslog(type);
if (rc)
return rc;
 
-   switch (type) {
-   case 3: /* Read last kernel messages */
-   case 10:/* Return size of the log buffer */
-   rc = 

[patch 2/4] permission mapping for sys_syslog operations

2006-12-24 Thread Zack Weinberg
As suggested by Stephen Smalley: map the various sys_syslog operations
to a smaller set of privilege codes before calling security modules.
This patch changes the security module interface!  There should be no
change in the actual security semantics enforced by dummy, capability,
nor SELinux (with one exception, clearly marked in sys_syslog).

The privilege codes are now in linux/security.h instead of
linux/klog.h, and use the LSM_* naming convention used for other such
constants in that file.

I'm now using a static table to map operations to privilege codes.
This makes it very easy to see the mapping as a whole, and prevents
security operations from cluttering up the body of do_syslog, but does
necessitate some checks to ensure that when new KLOG_* operations are
added, people don't forget to add entries to the table.  If people are
not fans of the idiom I used, I will see about getting a feature added
to gcc to make it less ugly.

Note that after this patch, close() on /proc/kmsg and
sys_syslog(KLOG_CLOSE) do not do security checks; they always succeed.
(IMO close should never fail.) This was always in this patch series,
but used to be in a different stage.

zw

Index: linux-2.6/kernel/printk.c
===
--- linux-2.6.orig/kernel/printk.c  2006-12-24 11:43:14.0 -0800
+++ linux-2.6/kernel/printk.c   2006-12-24 11:43:16.0 -0800
@@ -164,6 +164,39 @@
 
 __setup(log_buf_len=, log_buf_len_setup);
 
+/*
+ * This table maps KLOG_* operation codes to LSM_KLOG_* privilege classes.
+ * unsigned char is used just to keep it small.
+ */
+
+static const unsigned char klog_privclass[] = {
+   [KLOG_CLOSE]   = 0,  /* close always succeeds */
+   [KLOG_OPEN]= LSM_KLOG_READ,
+   [KLOG_READ]= LSM_KLOG_READ,
+   [KLOG_GET_UNREAD]  = LSM_KLOG_READ,
+
+   [KLOG_READ_HIST]   = LSM_KLOG_READHIST,
+   [KLOG_GET_SIZE]= LSM_KLOG_READHIST,
+   [KLOG_READ_CLEAR_HIST] = LSM_KLOG_READHIST|LSM_KLOG_CLEARHIST,
+   [KLOG_CLEAR_HIST]  = LSM_KLOG_CLEARHIST,
+
+   [KLOG_DISABLE_CONSOLE] = LSM_KLOG_CONSOLE,
+   [KLOG_ENABLE_CONSOLE]  = LSM_KLOG_CONSOLE,
+   [KLOG_SET_CONSOLE_LVL] = LSM_KLOG_CONSOLE,
+};
+
+/* It is essential that there be an entry in the above table for every
+ * KLOG_* code.  The following, er, odd declarations cause compilation
+ * of this file to fail if that is not true.  They do not correspond
+ * to real data objects.
+ */
+extern char klog_privclass_is_missing_an_entry[
+   ARRAY_SIZE(klog_privclass)  KLOG_OPCODE_MAX ? -1 : 1
+];
+extern char klog_privclass_has_too_many_entries[
+   ARRAY_SIZE(klog_privclass)  KLOG_OPCODE_MAX ? -1 : 1
+];
+
 /**
  * do_syslog - operate on the log of kernel messages
  * @type: operation to perform
@@ -185,9 +218,11 @@
unsigned long i, j, limit, count;
int do_clear = 0;
char c;
-   int error = 0;
+   int error;
 
-   error = security_syslog(type);
+   if (type  0 || type = KLOG_OPCODE_MAX)
+   return -EINVAL;
+   error = security_syslog(klog_privclass[type]);
if (error)
return error;
 
Index: linux-2.6/security/commoncap.c
===
--- linux-2.6.orig/security/commoncap.c 2006-12-23 08:55:16.0 -0800
+++ linux-2.6/security/commoncap.c  2006-12-24 11:43:16.0 -0800
@@ -311,7 +311,7 @@
 
 int cap_syslog (int type)
 {
-   if ((type != 3  type != 10)  !capable(CAP_SYS_ADMIN))
+   if ((type  ~LSM_KLOG_READHIST)  !capable(CAP_SYS_ADMIN))
return -EPERM;
return 0;
 }
Index: linux-2.6/security/dummy.c
===
--- linux-2.6.orig/security/dummy.c 2006-12-23 08:55:16.0 -0800
+++ linux-2.6/security/dummy.c  2006-12-24 11:43:16.0 -0800
@@ -96,7 +96,7 @@
 
 static int dummy_syslog (int type)
 {
-   if ((type != 3  type != 10)  current-euid)
+   if ((type  ~LSM_KLOG_READHIST)  current-euid)
return -EPERM;
return 0;
 }
Index: linux-2.6/security/selinux/hooks.c
===
--- linux-2.6.orig/security/selinux/hooks.c 2006-12-23 08:55:16.0 
-0800
+++ linux-2.6/security/selinux/hooks.c  2006-12-24 11:43:16.0 -0800
@@ -1504,29 +1504,27 @@
 {
int rc;
 
+   /* if there are any codes we don't know about, there's a bug */
+   BUG_ON(type  ~(LSM_KLOG_READ|LSM_KLOG_READHIST
+   |LSM_KLOG_CLEARHIST|LSM_KLOG_CONSOLE));
+
rc = secondary_ops-syslog(type);
if (rc)
return rc;
 
-   switch (type) {
-   case 3: /* Read last kernel messages */
-   case 10:/* Return size of the log buffer */
-   rc = task_has_system(current, 

Re: [patch 2/4] permission mapping for sys_syslog operations

2006-12-15 Thread Randy Dunlap
On Thu, 14 Dec 2006 17:21:25 -0800 Zack Weinberg wrote:

> On 12/14/06, Randy Dunlap <[EMAIL PROTECTED]> wrote:
> > > +#define security_syslog_or_fail(type) do {   \
> > > + int error = security_syslog(type);  \
> > > + if (error)  \
> > > + return error;   \
> > > + } while (0)
> > > +
> >
> > From Documentation/CodingStyle:
> >
> > Things to avoid when using macros:
> >
> > 1) macros that affect control flow: ...
> 
> It says "avoid", not "never use".  If you can think of another way to
> code this function that won't completely obscure the actual operations
> with the security checks, I will be happy to change it.

The usual answer is to just open-code it.  You may dislike that,
but hiding control flow in macros is held as really bad by
more than just me.

---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 2/4] permission mapping for sys_syslog operations

2006-12-15 Thread Randy Dunlap
On Thu, 14 Dec 2006 17:21:25 -0800 Zack Weinberg wrote:

 On 12/14/06, Randy Dunlap [EMAIL PROTECTED] wrote:
   +#define security_syslog_or_fail(type) do {   \
   + int error = security_syslog(type);  \
   + if (error)  \
   + return error;   \
   + } while (0)
   +
 
  From Documentation/CodingStyle:
 
  Things to avoid when using macros:
 
  1) macros that affect control flow: ...
 
 It says avoid, not never use.  If you can think of another way to
 code this function that won't completely obscure the actual operations
 with the security checks, I will be happy to change it.

The usual answer is to just open-code it.  You may dislike that,
but hiding control flow in macros is held as really bad by
more than just me.

---
~Randy
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 2/4] permission mapping for sys_syslog operations

2006-12-14 Thread Zack Weinberg

On 12/14/06, Randy Dunlap <[EMAIL PROTECTED]> wrote:

> +#define security_syslog_or_fail(type) do {   \
> + int error = security_syslog(type);  \
> + if (error)  \
> + return error;   \
> + } while (0)
> +

From Documentation/CodingStyle:

Things to avoid when using macros:

1) macros that affect control flow: ...


It says "avoid", not "never use".  If you can think of another way to
code this function that won't completely obscure the actual operations
with the security checks, I will be happy to change it.

zw
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 2/4] permission mapping for sys_syslog operations

2006-12-14 Thread Randy Dunlap
On Thu, 14 Dec 2006 16:16:41 -0800 Zack Weinberg wrote:

> As suggested by Stephen Smalley: map the various sys_syslog operations
> to a smaller set of privilege codes before calling security modules.
> This patch changes the security module interface!  There should be no
> change in the actual security semantics enforced by dummy, capability,
> nor SELinux (with one exception, clearly marked in sys_syslog).
> 
> Change from previous version of patch: the privilege codes are now
> in linux/security.h instead of linux/klog.h, and use the LSM_* naming
> convention used for other such constants in that file.
> 
> 
> Index: linux-2.6/kernel/printk.c
> ===
> --- linux-2.6.orig/kernel/printk.c2006-12-13 16:06:22.0 -0800
> +++ linux-2.6/kernel/printk.c 2006-12-13 16:08:30.0 -0800
> @@ -164,6 +164,12 @@
>  
>  __setup("log_buf_len=", log_buf_len_setup);
>  
> +#define security_syslog_or_fail(type) do {   \
> + int error = security_syslog(type);  \
> + if (error)  \
> + return error;   \
> + } while (0)
> +

>From Documentation/CodingStyle:

Things to avoid when using macros:

1) macros that affect control flow: ...


>  /* See linux/klog.h for the command numbers passed as the first argument.  */
>  int do_syslog(int type, char __user *buf, int len)
>  {
> @@ -172,16 +178,15 @@
>   char c;
>   int error = 0;
>  
> - error = security_syslog(type);
> - if (error)
> - return error;
> -
>   switch (type) {
>   case KLOG_CLOSE:
> + security_syslog_or_fail(LSM_KLOG_READ);
>   break;
>   case KLOG_OPEN:
> + security_syslog_or_fail(LSM_KLOG_READ);
>   break;
>   case KLOG_READ:
> + security_syslog_or_fail(LSM_KLOG_READ);
>   error = -EINVAL;
>   if (!buf || len < 0)
>   goto out;
> @@ -213,9 +218,11 @@
>   error = i;
>   break;
>   case KLOG_READ_CLEAR_HIST:
> + security_syslog_or_fail(LSM_KLOG_CLEARHIST);
>   do_clear = 1;
>   /* FALL THRU */
>   case KLOG_READ_HIST:
> + security_syslog_or_fail(LSM_KLOG_READHIST);
>   error = -EINVAL;
>   if (!buf || len < 0)
>   goto out;
> @@ -269,15 +276,19 @@
>   }
>   break;
>   case KLOG_CLEAR_HIST:
> + security_syslog_or_fail(LSM_KLOG_CLEARHIST);
>   logged_chars = 0;
>   break;
>   case KLOG_DISABLE_CONSOLE:
> + security_syslog_or_fail(LSM_KLOG_CONSOLE);
>   console_loglevel = minimum_console_loglevel;
>   break;
>   case KLOG_ENABLE_CONSOLE:
> + security_syslog_or_fail(LSM_KLOG_CONSOLE);
>   console_loglevel = default_console_loglevel;
>   break;
>   case KLOG_SET_CONSOLE_LVL:
> + security_syslog_or_fail(LSM_KLOG_CONSOLE);
>   error = -EINVAL;
>   if (len < 1 || len > 8)
>   goto out;
> @@ -287,9 +298,18 @@
>   error = 0;
>   break;
>   case KLOG_GET_UNREAD:
> + security_syslog_or_fail(LSM_KLOG_READ);
>   error = log_end - log_start;
>   break;


---
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[patch 2/4] permission mapping for sys_syslog operations

2006-12-14 Thread Zack Weinberg
As suggested by Stephen Smalley: map the various sys_syslog operations
to a smaller set of privilege codes before calling security modules.
This patch changes the security module interface!  There should be no
change in the actual security semantics enforced by dummy, capability,
nor SELinux (with one exception, clearly marked in sys_syslog).

Change from previous version of patch: the privilege codes are now
in linux/security.h instead of linux/klog.h, and use the LSM_* naming
convention used for other such constants in that file.

zw


Index: linux-2.6/kernel/printk.c
===
--- linux-2.6.orig/kernel/printk.c  2006-12-13 16:06:22.0 -0800
+++ linux-2.6/kernel/printk.c   2006-12-13 16:08:30.0 -0800
@@ -164,6 +164,12 @@
 
 __setup("log_buf_len=", log_buf_len_setup);
 
+#define security_syslog_or_fail(type) do { \
+   int error = security_syslog(type);  \
+   if (error)  \
+   return error;   \
+   } while (0)
+
 /* See linux/klog.h for the command numbers passed as the first argument.  */
 int do_syslog(int type, char __user *buf, int len)
 {
@@ -172,16 +178,15 @@
char c;
int error = 0;
 
-   error = security_syslog(type);
-   if (error)
-   return error;
-
switch (type) {
case KLOG_CLOSE:
+   security_syslog_or_fail(LSM_KLOG_READ);
break;
case KLOG_OPEN:
+   security_syslog_or_fail(LSM_KLOG_READ);
break;
case KLOG_READ:
+   security_syslog_or_fail(LSM_KLOG_READ);
error = -EINVAL;
if (!buf || len < 0)
goto out;
@@ -213,9 +218,11 @@
error = i;
break;
case KLOG_READ_CLEAR_HIST:
+   security_syslog_or_fail(LSM_KLOG_CLEARHIST);
do_clear = 1;
/* FALL THRU */
case KLOG_READ_HIST:
+   security_syslog_or_fail(LSM_KLOG_READHIST);
error = -EINVAL;
if (!buf || len < 0)
goto out;
@@ -269,15 +276,19 @@
}
break;
case KLOG_CLEAR_HIST:
+   security_syslog_or_fail(LSM_KLOG_CLEARHIST);
logged_chars = 0;
break;
case KLOG_DISABLE_CONSOLE:
+   security_syslog_or_fail(LSM_KLOG_CONSOLE);
console_loglevel = minimum_console_loglevel;
break;
case KLOG_ENABLE_CONSOLE:
+   security_syslog_or_fail(LSM_KLOG_CONSOLE);
console_loglevel = default_console_loglevel;
break;
case KLOG_SET_CONSOLE_LVL:
+   security_syslog_or_fail(LSM_KLOG_CONSOLE);
error = -EINVAL;
if (len < 1 || len > 8)
goto out;
@@ -287,9 +298,18 @@
error = 0;
break;
case KLOG_GET_UNREAD:
+   security_syslog_or_fail(LSM_KLOG_READ);
error = log_end - log_start;
break;
case KLOG_GET_SIZE:
+   /* This one is allowed if you have _either_
+  LSM_KLOG_READ or LSM_KLOG_READHIST.  */
+   error = security_syslog(LSM_KLOG_READ);
+   if (error)
+   error = security_syslog(LSM_KLOG_READHIST);
+   if (error)
+   break;
+
error = log_buf_len;
break;
default:
Index: linux-2.6/security/commoncap.c
===
--- linux-2.6.orig/security/commoncap.c 2006-12-13 16:06:22.0 -0800
+++ linux-2.6/security/commoncap.c  2006-12-13 16:11:13.0 -0800
@@ -311,7 +311,7 @@
 
 int cap_syslog (int type)
 {
-   if ((type != 3 && type != 10) && !capable(CAP_SYS_ADMIN))
+   if (type != LSM_KLOG_READHIST && !capable(CAP_SYS_ADMIN))
return -EPERM;
return 0;
 }
Index: linux-2.6/security/dummy.c
===
--- linux-2.6.orig/security/dummy.c 2006-12-13 16:06:22.0 -0800
+++ linux-2.6/security/dummy.c  2006-12-13 16:11:31.0 -0800
@@ -96,7 +96,7 @@
 
 static int dummy_syslog (int type)
 {
-   if ((type != 3 && type != 10) && current->euid)
+   if (type != LSM_KLOG_READHIST && current->euid)
return -EPERM;
return 0;
 }
Index: linux-2.6/security/selinux/hooks.c
===
--- linux-2.6.orig/security/selinux/hooks.c 2006-12-13 16:06:22.0 
-0800
+++ linux-2.6/security/selinux/hooks.c  2006-12-13 16:11:41.0 -0800
@@ -1509,25 +1509,17 @@
return rc;
 
switch (type) {
-   

[patch 2/4] permission mapping for sys_syslog operations

2006-12-14 Thread Zack Weinberg
As suggested by Stephen Smalley: map the various sys_syslog operations
to a smaller set of privilege codes before calling security modules.
This patch changes the security module interface!  There should be no
change in the actual security semantics enforced by dummy, capability,
nor SELinux (with one exception, clearly marked in sys_syslog).

Change from previous version of patch: the privilege codes are now
in linux/security.h instead of linux/klog.h, and use the LSM_* naming
convention used for other such constants in that file.

zw


Index: linux-2.6/kernel/printk.c
===
--- linux-2.6.orig/kernel/printk.c  2006-12-13 16:06:22.0 -0800
+++ linux-2.6/kernel/printk.c   2006-12-13 16:08:30.0 -0800
@@ -164,6 +164,12 @@
 
 __setup(log_buf_len=, log_buf_len_setup);
 
+#define security_syslog_or_fail(type) do { \
+   int error = security_syslog(type);  \
+   if (error)  \
+   return error;   \
+   } while (0)
+
 /* See linux/klog.h for the command numbers passed as the first argument.  */
 int do_syslog(int type, char __user *buf, int len)
 {
@@ -172,16 +178,15 @@
char c;
int error = 0;
 
-   error = security_syslog(type);
-   if (error)
-   return error;
-
switch (type) {
case KLOG_CLOSE:
+   security_syslog_or_fail(LSM_KLOG_READ);
break;
case KLOG_OPEN:
+   security_syslog_or_fail(LSM_KLOG_READ);
break;
case KLOG_READ:
+   security_syslog_or_fail(LSM_KLOG_READ);
error = -EINVAL;
if (!buf || len  0)
goto out;
@@ -213,9 +218,11 @@
error = i;
break;
case KLOG_READ_CLEAR_HIST:
+   security_syslog_or_fail(LSM_KLOG_CLEARHIST);
do_clear = 1;
/* FALL THRU */
case KLOG_READ_HIST:
+   security_syslog_or_fail(LSM_KLOG_READHIST);
error = -EINVAL;
if (!buf || len  0)
goto out;
@@ -269,15 +276,19 @@
}
break;
case KLOG_CLEAR_HIST:
+   security_syslog_or_fail(LSM_KLOG_CLEARHIST);
logged_chars = 0;
break;
case KLOG_DISABLE_CONSOLE:
+   security_syslog_or_fail(LSM_KLOG_CONSOLE);
console_loglevel = minimum_console_loglevel;
break;
case KLOG_ENABLE_CONSOLE:
+   security_syslog_or_fail(LSM_KLOG_CONSOLE);
console_loglevel = default_console_loglevel;
break;
case KLOG_SET_CONSOLE_LVL:
+   security_syslog_or_fail(LSM_KLOG_CONSOLE);
error = -EINVAL;
if (len  1 || len  8)
goto out;
@@ -287,9 +298,18 @@
error = 0;
break;
case KLOG_GET_UNREAD:
+   security_syslog_or_fail(LSM_KLOG_READ);
error = log_end - log_start;
break;
case KLOG_GET_SIZE:
+   /* This one is allowed if you have _either_
+  LSM_KLOG_READ or LSM_KLOG_READHIST.  */
+   error = security_syslog(LSM_KLOG_READ);
+   if (error)
+   error = security_syslog(LSM_KLOG_READHIST);
+   if (error)
+   break;
+
error = log_buf_len;
break;
default:
Index: linux-2.6/security/commoncap.c
===
--- linux-2.6.orig/security/commoncap.c 2006-12-13 16:06:22.0 -0800
+++ linux-2.6/security/commoncap.c  2006-12-13 16:11:13.0 -0800
@@ -311,7 +311,7 @@
 
 int cap_syslog (int type)
 {
-   if ((type != 3  type != 10)  !capable(CAP_SYS_ADMIN))
+   if (type != LSM_KLOG_READHIST  !capable(CAP_SYS_ADMIN))
return -EPERM;
return 0;
 }
Index: linux-2.6/security/dummy.c
===
--- linux-2.6.orig/security/dummy.c 2006-12-13 16:06:22.0 -0800
+++ linux-2.6/security/dummy.c  2006-12-13 16:11:31.0 -0800
@@ -96,7 +96,7 @@
 
 static int dummy_syslog (int type)
 {
-   if ((type != 3  type != 10)  current-euid)
+   if (type != LSM_KLOG_READHIST  current-euid)
return -EPERM;
return 0;
 }
Index: linux-2.6/security/selinux/hooks.c
===
--- linux-2.6.orig/security/selinux/hooks.c 2006-12-13 16:06:22.0 
-0800
+++ linux-2.6/security/selinux/hooks.c  2006-12-13 16:11:41.0 -0800
@@ -1509,25 +1509,17 @@
return rc;
 
switch (type) {
-   case 3: /* 

Re: [patch 2/4] permission mapping for sys_syslog operations

2006-12-14 Thread Randy Dunlap
On Thu, 14 Dec 2006 16:16:41 -0800 Zack Weinberg wrote:

 As suggested by Stephen Smalley: map the various sys_syslog operations
 to a smaller set of privilege codes before calling security modules.
 This patch changes the security module interface!  There should be no
 change in the actual security semantics enforced by dummy, capability,
 nor SELinux (with one exception, clearly marked in sys_syslog).
 
 Change from previous version of patch: the privilege codes are now
 in linux/security.h instead of linux/klog.h, and use the LSM_* naming
 convention used for other such constants in that file.
 
 
 Index: linux-2.6/kernel/printk.c
 ===
 --- linux-2.6.orig/kernel/printk.c2006-12-13 16:06:22.0 -0800
 +++ linux-2.6/kernel/printk.c 2006-12-13 16:08:30.0 -0800
 @@ -164,6 +164,12 @@
  
  __setup(log_buf_len=, log_buf_len_setup);
  
 +#define security_syslog_or_fail(type) do {   \
 + int error = security_syslog(type);  \
 + if (error)  \
 + return error;   \
 + } while (0)
 +

From Documentation/CodingStyle:

Things to avoid when using macros:

1) macros that affect control flow: ...


  /* See linux/klog.h for the command numbers passed as the first argument.  */
  int do_syslog(int type, char __user *buf, int len)
  {
 @@ -172,16 +178,15 @@
   char c;
   int error = 0;
  
 - error = security_syslog(type);
 - if (error)
 - return error;
 -
   switch (type) {
   case KLOG_CLOSE:
 + security_syslog_or_fail(LSM_KLOG_READ);
   break;
   case KLOG_OPEN:
 + security_syslog_or_fail(LSM_KLOG_READ);
   break;
   case KLOG_READ:
 + security_syslog_or_fail(LSM_KLOG_READ);
   error = -EINVAL;
   if (!buf || len  0)
   goto out;
 @@ -213,9 +218,11 @@
   error = i;
   break;
   case KLOG_READ_CLEAR_HIST:
 + security_syslog_or_fail(LSM_KLOG_CLEARHIST);
   do_clear = 1;
   /* FALL THRU */
   case KLOG_READ_HIST:
 + security_syslog_or_fail(LSM_KLOG_READHIST);
   error = -EINVAL;
   if (!buf || len  0)
   goto out;
 @@ -269,15 +276,19 @@
   }
   break;
   case KLOG_CLEAR_HIST:
 + security_syslog_or_fail(LSM_KLOG_CLEARHIST);
   logged_chars = 0;
   break;
   case KLOG_DISABLE_CONSOLE:
 + security_syslog_or_fail(LSM_KLOG_CONSOLE);
   console_loglevel = minimum_console_loglevel;
   break;
   case KLOG_ENABLE_CONSOLE:
 + security_syslog_or_fail(LSM_KLOG_CONSOLE);
   console_loglevel = default_console_loglevel;
   break;
   case KLOG_SET_CONSOLE_LVL:
 + security_syslog_or_fail(LSM_KLOG_CONSOLE);
   error = -EINVAL;
   if (len  1 || len  8)
   goto out;
 @@ -287,9 +298,18 @@
   error = 0;
   break;
   case KLOG_GET_UNREAD:
 + security_syslog_or_fail(LSM_KLOG_READ);
   error = log_end - log_start;
   break;


---
~Randy
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch 2/4] permission mapping for sys_syslog operations

2006-12-14 Thread Zack Weinberg

On 12/14/06, Randy Dunlap [EMAIL PROTECTED] wrote:

 +#define security_syslog_or_fail(type) do {   \
 + int error = security_syslog(type);  \
 + if (error)  \
 + return error;   \
 + } while (0)
 +

From Documentation/CodingStyle:

Things to avoid when using macros:

1) macros that affect control flow: ...


It says avoid, not never use.  If you can think of another way to
code this function that won't completely obscure the actual operations
with the security checks, I will be happy to change it.

zw
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/