Eric Paris pointed out that it doesn't make sense to require
both CAP_SYS_ADMIN and CAP_SYSLOG for certain syslog actions.
So require CAP_SYSLOG, not CAP_SYS_ADMIN, when dmesg_restrict
is set.
(I'm also consolidating the now common error path)
Signed-off-by: Serge E. Hallyn <[email protected]>
---
Documentation/sysctl/kernel.txt | 2 +-
kernel/printk.c | 20 ++++++++++----------
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 209e158..5740671 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -219,7 +219,7 @@ dmesg_restrict:
This toggle indicates whether unprivileged users are prevented from using
dmesg(8) to view messages from the kernel's log buffer. When
dmesg_restrict is set to (0) there are no restrictions. When
-dmesg_restrict is set set to (1), users must have CAP_SYS_ADMIN to use
+dmesg_restrict is set set to (1), users must have CAP_SYSLOG to use
dmesg(8).
The kernel config option CONFIG_SECURITY_DMESG_RESTRICT sets the default
diff --git a/kernel/printk.c b/kernel/printk.c
index 0712380..0cecba0 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -279,18 +279,12 @@ int do_syslog(int type, char __user *buf, int len, bool from_file)
* at open time.
*/
if (type == SYSLOG_ACTION_OPEN || !from_file) {
- if (dmesg_restrict && !capable(CAP_SYS_ADMIN))
- return -EPERM;
+ if (dmesg_restrict && !capable(CAP_SYSLOG))
+ goto warn; /* switch to return -EPERM after 2.6.39 */
if ((type != SYSLOG_ACTION_READ_ALL &&
type != SYSLOG_ACTION_SIZE_BUFFER) &&
- !capable(CAP_SYSLOG)) {
- /* remove after 2.6.38 */
- if (capable(CAP_SYS_ADMIN))
- WARN_ONCE(1, "Attempt to access syslog with "
- "CAP_SYS_ADMIN but no CAP_SYSLOG "
- "(deprecated and denied).\n");
- return -EPERM;
- }
+ !capable(CAP_SYSLOG))
+ goto warn; /* switch to return -EPERM after 2.6.39 */
}
error = security_syslog(type);
@@ -434,6 +428,12 @@ int do_syslog(int type, char __user *buf, int len, bool from_file)
}
out:
return error;
+warn:
+ /* remove after 2.6.39 */
+ if (capable(CAP_SYS_ADMIN))
+ WARN_ONCE(1, "Attempt to access syslog with CAP_SYS_ADMIN "
+ "but no CAP_SYSLOG (deprecated and denied).\n");
+ return -EPERM;
}
SYSCALL_DEFINE3(syslog, int, type, char __user *, buf, int, len)
--
1.7.0.4
On Wed, 2010-12-08 at 15:19 +0000, Serge E. Hallyn wrote:
> Eric Paris pointed out that it doesn't make sense to require
> both CAP_SYS_ADMIN and CAP_SYSLOG for certain syslog actions.
> So require CAP_SYSLOG, not CAP_SYS_ADMIN, when dmesg_restrict
> is set.
>
> (I'm also consolidating the now common error path)
>
> Signed-off-by: Serge E. Hallyn <[email protected]>
> ---
> Documentation/sysctl/kernel.txt | 2 +-
> kernel/printk.c | 20 ++++++++++----------
> 2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 209e158..5740671 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -219,7 +219,7 @@ dmesg_restrict:
> This toggle indicates whether unprivileged users are prevented from using
> dmesg(8) to view messages from the kernel's log buffer. When
> dmesg_restrict is set to (0) there are no restrictions. When
> -dmesg_restrict is set set to (1), users must have CAP_SYS_ADMIN to use
> +dmesg_restrict is set set to (1), users must have CAP_SYSLOG to use
> dmesg(8).
>
> The kernel config option CONFIG_SECURITY_DMESG_RESTRICT sets the default
> diff --git a/kernel/printk.c b/kernel/printk.c
> index 0712380..0cecba0 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -279,18 +279,12 @@ int do_syslog(int type, char __user *buf, int len, bool from_file)
> * at open time.
> */
> if (type == SYSLOG_ACTION_OPEN || !from_file) {
> - if (dmesg_restrict && !capable(CAP_SYS_ADMIN))
> - return -EPERM;
> + if (dmesg_restrict && !capable(CAP_SYSLOG))
> + goto warn; /* switch to return -EPERM after 2.6.39 */
> if ((type != SYSLOG_ACTION_READ_ALL &&
> type != SYSLOG_ACTION_SIZE_BUFFER) &&
> - !capable(CAP_SYSLOG)) {
> - /* remove after 2.6.38 */
> - if (capable(CAP_SYS_ADMIN))
> - WARN_ONCE(1, "Attempt to access syslog with "
> - "CAP_SYS_ADMIN but no CAP_SYSLOG "
> - "(deprecated and denied).\n");
> - return -EPERM;
> - }
> + !capable(CAP_SYSLOG))
> + goto warn; /* switch to return -EPERM after 2.6.39 */
Doesn't this return -EPERM right now? I think the code might be
incorrect today as well......
I thought the flow was supposed to be
if (capable(CAP_SYSLOG))
all good
else if (capable(CAP_SYS_ADMIN))
WARN, but still good for now
else
EPERM
But it looks to me like the flow is
if (capable(CAP_SYSLOG))
all good
else if (capable(CAP_SYS_ADMIN))
WARN, EPERM
else
EPERM
> }
>
> error = security_syslog(type);
> @@ -434,6 +428,12 @@ int do_syslog(int type, char __user *buf, int len, bool from_file)
> }
> out:
> return error;
> +warn:
> + /* remove after 2.6.39 */
> + if (capable(CAP_SYS_ADMIN))
> + WARN_ONCE(1, "Attempt to access syslog with CAP_SYS_ADMIN "
> + "but no CAP_SYSLOG (deprecated and denied).\n");
> + return -EPERM;
> }
>
> SYSCALL_DEFINE3(syslog, int, type, char __user *, buf, int, len)
Quoting Eric Paris ([email protected]):
> On Wed, 2010-12-08 at 15:19 +0000, Serge E. Hallyn wrote:
> Doesn't this return -EPERM right now?
Yes.
> I think the code might be
> incorrect today as well......
>
> I thought the flow was supposed to be
>
> if (capable(CAP_SYSLOG))
> all good
> else if (capable(CAP_SYS_ADMIN))
> WARN, but still good for now
I prefer warn and deny. Otherwise it's too easy to ignore warnings. So
I prefer the msg to be there to explain why it failed - not that I expect
it to fail for anyone today.
> else
> EPERM
>
> But it looks to me like the flow is
>
> if (capable(CAP_SYSLOG))
> all good
> else if (capable(CAP_SYS_ADMIN))
> WARN, EPERM
> else
> EPERM
>
> > }
Yup.
-serge
On Wed, 2010-12-08 at 16:42 +0000, Serge E. Hallyn wrote:
> Quoting Eric Paris ([email protected]):
> > I think the code might be
> > incorrect today as well......
> >
> > I thought the flow was supposed to be
> >
> > if (capable(CAP_SYSLOG))
> > all good
> > else if (capable(CAP_SYS_ADMIN))
> > WARN, but still good for now
>
> I prefer warn and deny. Otherwise it's too easy to ignore warnings. So
> I prefer the msg to be there to explain why it failed - not that I expect
> it to fail for anyone today.
Ok, fine with me.
Acked-by: Eric Paris <[email protected]>
On Wed, Dec 08, 2010 at 03:19:01PM +0000, Serge E. Hallyn wrote:
> Eric Paris pointed out that it doesn't make sense to require
> both CAP_SYS_ADMIN and CAP_SYSLOG for certain syslog actions.
> So require CAP_SYSLOG, not CAP_SYS_ADMIN, when dmesg_restrict
> is set.
>
> (I'm also consolidating the now common error path)
>
> Signed-off-by: Serge E. Hallyn <[email protected]>
Acked-by: Kees Cook <[email protected]>
--
Kees Cook
Ubuntu Security Team
On Wed, 8 Dec 2010, Serge E. Hallyn wrote:
> Eric Paris pointed out that it doesn't make sense to require
> both CAP_SYS_ADMIN and CAP_SYSLOG for certain syslog actions.
> So require CAP_SYSLOG, not CAP_SYS_ADMIN, when dmesg_restrict
> is set.
>
> (I'm also consolidating the now common error path)
>
> Signed-off-by: Serge E. Hallyn <[email protected]>
Applied.
(Please cc the lsm list with security patches).
> ---
> Documentation/sysctl/kernel.txt | 2 +-
> kernel/printk.c | 20 ++++++++++----------
> 2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 209e158..5740671 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -219,7 +219,7 @@ dmesg_restrict:
> This toggle indicates whether unprivileged users are prevented from using
> dmesg(8) to view messages from the kernel's log buffer. When
> dmesg_restrict is set to (0) there are no restrictions. When
> -dmesg_restrict is set set to (1), users must have CAP_SYS_ADMIN to use
> +dmesg_restrict is set set to (1), users must have CAP_SYSLOG to use
> dmesg(8).
>
> The kernel config option CONFIG_SECURITY_DMESG_RESTRICT sets the default
> diff --git a/kernel/printk.c b/kernel/printk.c
> index 0712380..0cecba0 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -279,18 +279,12 @@ int do_syslog(int type, char __user *buf, int len, bool from_file)
> * at open time.
> */
> if (type == SYSLOG_ACTION_OPEN || !from_file) {
> - if (dmesg_restrict && !capable(CAP_SYS_ADMIN))
> - return -EPERM;
> + if (dmesg_restrict && !capable(CAP_SYSLOG))
> + goto warn; /* switch to return -EPERM after 2.6.39 */
> if ((type != SYSLOG_ACTION_READ_ALL &&
> type != SYSLOG_ACTION_SIZE_BUFFER) &&
> - !capable(CAP_SYSLOG)) {
> - /* remove after 2.6.38 */
> - if (capable(CAP_SYS_ADMIN))
> - WARN_ONCE(1, "Attempt to access syslog with "
> - "CAP_SYS_ADMIN but no CAP_SYSLOG "
> - "(deprecated and denied).\n");
> - return -EPERM;
> - }
> + !capable(CAP_SYSLOG))
> + goto warn; /* switch to return -EPERM after 2.6.39 */
> }
>
> error = security_syslog(type);
> @@ -434,6 +428,12 @@ int do_syslog(int type, char __user *buf, int len, bool from_file)
> }
> out:
> return error;
> +warn:
> + /* remove after 2.6.39 */
> + if (capable(CAP_SYS_ADMIN))
> + WARN_ONCE(1, "Attempt to access syslog with CAP_SYS_ADMIN "
> + "but no CAP_SYSLOG (deprecated and denied).\n");
> + return -EPERM;
> }
>
> SYSCALL_DEFINE3(syslog, int, type, char __user *, buf, int, len)
> --
> 1.7.0.4
>
--
James Morris
<[email protected]>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 12/08/2010 05:56 PM, James Morris wrote:
> On Wed, 8 Dec 2010, Serge E. Hallyn wrote:
>
>> Eric Paris pointed out that it doesn't make sense to require
>> both CAP_SYS_ADMIN and CAP_SYSLOG for certain syslog actions.
>> So require CAP_SYSLOG, not CAP_SYS_ADMIN, when dmesg_restrict
>> is set.
>>
>> (I'm also consolidating the now common error path)
>>
>> Signed-off-by: Serge E. Hallyn <[email protected]>
>
> Applied.
>
> (Please cc the lsm list with security patches).
>
>> ---
>> Documentation/sysctl/kernel.txt | 2 +-
>> kernel/printk.c | 20 ++++++++++----------
>> 2 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
>> index 209e158..5740671 100644
>> --- a/Documentation/sysctl/kernel.txt
>> +++ b/Documentation/sysctl/kernel.txt
>> @@ -219,7 +219,7 @@ dmesg_restrict:
>> This toggle indicates whether unprivileged users are prevented from using
>> dmesg(8) to view messages from the kernel's log buffer. When
>> dmesg_restrict is set to (0) there are no restrictions. When
>> -dmesg_restrict is set set to (1), users must have CAP_SYS_ADMIN to use
>> +dmesg_restrict is set set to (1), users must have CAP_SYSLOG to use
>> dmesg(8).
>>
>> The kernel config option CONFIG_SECURITY_DMESG_RESTRICT sets the default
>> diff --git a/kernel/printk.c b/kernel/printk.c
>> index 0712380..0cecba0 100644
>> --- a/kernel/printk.c
>> +++ b/kernel/printk.c
>> @@ -279,18 +279,12 @@ int do_syslog(int type, char __user *buf, int len, bool from_file)
>> * at open time.
>> */
>> if (type == SYSLOG_ACTION_OPEN || !from_file) {
>> - if (dmesg_restrict && !capable(CAP_SYS_ADMIN))
>> - return -EPERM;
>> + if (dmesg_restrict && !capable(CAP_SYSLOG))
>> + goto warn; /* switch to return -EPERM after 2.6.39 */
>> if ((type != SYSLOG_ACTION_READ_ALL &&
>> type != SYSLOG_ACTION_SIZE_BUFFER) &&
>> - !capable(CAP_SYSLOG)) {
>> - /* remove after 2.6.38 */
>> - if (capable(CAP_SYS_ADMIN))
>> - WARN_ONCE(1, "Attempt to access syslog with "
>> - "CAP_SYS_ADMIN but no CAP_SYSLOG "
>> - "(deprecated and denied).\n");
>> - return -EPERM;
>> - }
>> + !capable(CAP_SYSLOG))
>> + goto warn; /* switch to return -EPERM after 2.6.39 */
>> }
>>
>> error = security_syslog(type);
>> @@ -434,6 +428,12 @@ int do_syslog(int type, char __user *buf, int len, bool from_file)
>> }
>> out:
>> return error;
>> +warn:
>> + /* remove after 2.6.39 */
>> + if (capable(CAP_SYS_ADMIN))
>> + WARN_ONCE(1, "Attempt to access syslog with CAP_SYS_ADMIN "
>> + "but no CAP_SYSLOG (deprecated and denied).\n");
>> + return -EPERM;
>> }
>>
>> SYSCALL_DEFINE3(syslog, int, type, char __user *, buf, int, len)
>> --
>> 1.7.0.4
>>
>
Does anyone have an idea of which domains are going to be effected by
this change?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk0CMvcACgkQrlYvE4MpobP++gCgyJtjhYDfgXnc0TBOGseOpF67
zHoAn3bEditZdnj/OLGInp7FeCaxNQXH
=cOvZ
-----END PGP SIGNATURE-----