[PATCH] debugfs: more tightly restrict default mount mode

[PATCH] debugfs: more tightly restrict default mount mode

[Kees Cook]

Since the debugfs is mostly only used by root, make the default mount
mode 0700. Most system owners do not need a more permissive value,
but they can choose to weaken the restrictions via their fstab.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 Documentation/filesystems/debugfs.txt |    4 ++--
 fs/debugfs/inode.c                    |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/filesystems/debugfs.txt b/Documentation/filesystems/debugfs.txt
index 7a34f82..3a863f6 100644
--- a/Documentation/filesystems/debugfs.txt
+++ b/Documentation/filesystems/debugfs.txt
@@ -15,8 +15,8 @@ Debugfs is typically mounted with a command like:
     mount -t debugfs none /sys/kernel/debug
 
 (Or an equivalent /etc/fstab line).
-The debugfs root directory is accessible by anyone by default. To
-restrict access to the tree the "uid", "gid" and "mode" mount
+The debugfs root directory is accessible only to the root user by
+default. To change access to the tree the "uid", "gid" and "mode" mount
 options can be used.
 
 Note that the debugfs API is exported GPL-only to modules.
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 4733eab..2d22c81 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -28,7 +28,7 @@
 #include <linux/magic.h>
 #include <linux/slab.h>
 
-#define DEBUGFS_DEFAULT_MODE	0755
+#define DEBUGFS_DEFAULT_MODE	0700
 
 static struct vfsmount *debugfs_mount;
 static int debugfs_mount_count;
-- 
1.7.0.4


-- 
Kees Cook
Chrome OS Security

Re: [PATCH] debugfs: more tightly restrict default mount mode

[Greg Kroah-Hartman]

On Mon, Aug 27, 2012 at 01:32:15PM -0700, Kees Cook wrote:
> Since the debugfs is mostly only used by root, make the default mount
> mode 0700. Most system owners do not need a more permissive value,
> but they can choose to weaken the restrictions via their fstab.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>

Ok, you've worn me down, let's see how well this works, I'll queue this
up for 3.7.

greg k-h

Re: [PATCH] debugfs: more tightly restrict default mount mode

[Alessandro Rubini]

> Since the debugfs is mostly only used by root, make the default mount
> mode 0700. Most system owners do not need a more permissive value,
> but they can choose to weaken the restrictions via their fstab.

But if the default is strict, file-completion won't work and most
people will run a full root shell instead of sudo to save time. This
is a step back in my opinion.

Most administrators of their own machine won't go as far as changing
fstab (none of my students would, for example). On the other hand
admins of serious sites who are really concerned about doing "ls" over
debugfs will be able to customize.

So I vote against, knowing I'm late.

thanks
/alessandro

Hardening debugfs (Was Re: [PATCH] debugfs: more tightly restrict default mount mode)

[Theodore Ts'o]

On Mon, Aug 27, 2012 at 01:32:15PM -0700, Kees Cook wrote:
> Since the debugfs is mostly only used by root, make the default mount
> mode 0700. Most system owners do not need a more permissive value,
> but they can choose to weaken the restrictions via their fstab.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>

I agree with this patch, but it would also be good if we could try to
harden debugfs in general.  Some ideas that might be worth discussing,
for example?

1) Adding a per-module flag, so things in debugfs only show up if they
are explicitly requested (you know, for debugging purposes).  If most
people are using debugfs for access to ftrace and powertap (my use
case), there's no point making directories for other device drivers
and file systems visible.

2) Can we find a pattern of common security #fail's with debugfs
files, and try to sweep through and fix them?

There may be other ideas, and again, I'm not saying that this means we
shouldn't lock down the permissions on debugfs.  But a both/and
approach might be useful here....

							- Ted

Re: Hardening debugfs (Was Re: [PATCH] debugfs: more tightly restrict default mount mode)

[Ben Hutchings]

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

On Tue, 2012-08-28 at 10:41 -0400, Theodore Ts'o wrote:
> On Mon, Aug 27, 2012 at 01:32:15PM -0700, Kees Cook wrote:
> > Since the debugfs is mostly only used by root, make the default mount
> > mode 0700. Most system owners do not need a more permissive value,
> > but they can choose to weaken the restrictions via their fstab.
> > 
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> 
> I agree with this patch, but it would also be good if we could try to
> harden debugfs in general.  Some ideas that might be worth discussing,
> for example?
[...]

The problems are apparently larger than specific modules:
http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-July/000894.html

Ben.

-- 
Ben Hutchings
It is a miracle that curiosity survives formal education. - Albert Einstein

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: Hardening debugfs (Was Re: [PATCH] debugfs: more tightly restrict default mount mode)

[Theodore Ts'o]

On Tue, Aug 28, 2012 at 07:55:58AM -0700, Ben Hutchings wrote:
> 
> The problems are apparently larger than specific modules:
> http://lists.linux-foundation.org/pipermail/ksummit-2012-discuss/2012-July/000894.html
> 

Sure, but most of those problems require root access, or physical
access to the hardware.  And a number of the "can oops the kernel"
assume module disappears out from under the open file descriptor, so
(a) that's a problem that can be fixed, and (b) if we can suppress a
random device driver from having its debugfs directory appear by
default, it certainly helps things.

					- Ted

Re: Hardening debugfs (Was Re: [PATCH] debugfs: more tightly restrict default mount mode)

[Greg Kroah-Hartman]

On Tue, Aug 28, 2012 at 10:41:10AM -0400, Theodore Ts'o wrote:
> On Mon, Aug 27, 2012 at 01:32:15PM -0700, Kees Cook wrote:
> > Since the debugfs is mostly only used by root, make the default mount
> > mode 0700. Most system owners do not need a more permissive value,
> > but they can choose to weaken the restrictions via their fstab.
> > 
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> 
> I agree with this patch, but it would also be good if we could try to
> harden debugfs in general.  Some ideas that might be worth discussing,
> for example?
> 
> 1) Adding a per-module flag, so things in debugfs only show up if they
> are explicitly requested (you know, for debugging purposes).  If most
> people are using debugfs for access to ftrace and powertap (my use
> case), there's no point making directories for other device drivers
> and file systems visible.

The module code is "explicitly requesting" a debugfs file when it makes
the call to create it.  If you want to depend on a flag for the
individual modules to do this or not, sure, go ahead, but that's a
module/driver issue, nothing I can do in the debugfs core itself.

> 2) Can we find a pattern of common security #fail's with debugfs
> files, and try to sweep through and fix them?

The only one I know of is the "unload the module with an open file
handle" issue.  I'm pretty sure this could be fixed up somehow in
debugfs, much like it was resolved in sysfs, but it would take a lot of
work, for a very limited benefit (in other words, if someone sends me
patches for this, great, but it's so low on my TODO list that I'll
probably never get to it myself.)

thanks,

greg k-h

Re: Hardening debugfs (Was Re: [PATCH] debugfs: more tightly restrict default mount mode)

[Kees Cook]

On Tue, Aug 28, 2012 at 10:09 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Aug 28, 2012 at 10:41:10AM -0400, Theodore Ts'o wrote:
>> On Mon, Aug 27, 2012 at 01:32:15PM -0700, Kees Cook wrote:
>> > Since the debugfs is mostly only used by root, make the default mount
>> > mode 0700. Most system owners do not need a more permissive value,
>> > but they can choose to weaken the restrictions via their fstab.
>> >
>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>>
>> I agree with this patch, but it would also be good if we could try to
>> harden debugfs in general.  Some ideas that might be worth discussing,
>> for example?
>>
>> 1) Adding a per-module flag, so things in debugfs only show up if they
>> are explicitly requested (you know, for debugging purposes).  If most
>> people are using debugfs for access to ftrace and powertap (my use
>> case), there's no point making directories for other device drivers
>> and file systems visible.
>
> The module code is "explicitly requesting" a debugfs file when it makes
> the call to create it.  If you want to depend on a flag for the
> individual modules to do this or not, sure, go ahead, but that's a
> module/driver issue, nothing I can do in the debugfs core itself.
>
>> 2) Can we find a pattern of common security #fail's with debugfs
>> files, and try to sweep through and fix them?
>
> The only one I know of is the "unload the module with an open file
> handle" issue.  I'm pretty sure this could be fixed up somehow in
> debugfs, much like it was resolved in sysfs, but it would take a lot of
> work, for a very limited benefit (in other words, if someone sends me
> patches for this, great, but it's so low on my TODO list that I'll
> probably never get to it myself.)

Staying after world-writable files is probably the biggest deal
(though this has been checked for in the past after problems
surfaced). I think the main reason I've wanted to push for 0700 was
just because the scope of the problems is so large. It could be as
simple as leaking kernel (or userspace) address locations (allowing
ASLR bypass, or heap location predictability) or other side-effects
from world-readable files, all the way to weird ioctl flaws in
world-writable files. In general, since debugfs is specifically
designed to have unstructured contents, it's not particularly trivial
to be able to reason about how those contents can be best protected.
As such, I just wanted to isolated it.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Hardening debugfs (Was Re: [PATCH] debugfs: more tightly restrict default mount mode)

[Rob Landley]

On 08/28/2012 09:41 AM, Theodore Ts'o wrote:
> On Mon, Aug 27, 2012 at 01:32:15PM -0700, Kees Cook wrote:
>> Since the debugfs is mostly only used by root, make the default mount
>> mode 0700. Most system owners do not need a more permissive value,
>> but they can choose to weaken the restrictions via their fstab.
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
> 
> I agree with this patch, but it would also be good if we could try to
> harden debugfs in general.  Some ideas that might be worth discussing,
> for example?
> 
> 1) Adding a per-module flag, so things in debugfs only show up if they
> are explicitly requested (you know, for debugging purposes). If most
> people are using debugfs for access to ftrace and powertap (my use
> case), there's no point making directories for other device drivers
> and file systems visible.

Are you suggesting "echo 1 > /sys/module/mymod/debug", or are you
suggesting "mount -t devfs -o mymod /tmp/mymod", or knobs in devfs?

I've always been a bit confused by the debugfs design, which seems a
giant compost heap like /proc where we find a specific styrofoam cup
useful and the temporary thing becomes permanent. (Why is there _one_
debugfs?)

Oh well, presumably too late to change it now. (Unless you mount a tmpfs
on /sys/kernel/debug and mkdir mount points in there, but in the
perpetual absence of union mounts it would probably involve
userspace-visible path changes...)

> 2) Can we find a pattern of common security #fail's with debugfs
> files, and try to sweep through and fix them?
> 
> There may be other ideas, and again, I'm not saying that this means we
> shouldn't lock down the permissions on debugfs.  But a both/and
> approach might be useful here....

Plenty of other ideas, but it says "there are no usage rules" right
there in the documentation file which makes compatible cleanup hard...

Rob
-- 
GNU/Linux isn't: Linux=GPLv2, GNU=GPLv3+, they can't share code.
Either it's "mere aggregation", or a license violation.  Pick one.

Re: Hardening debugfs (Was Re: [PATCH] debugfs: more tightly restrict default mount mode)

[Greg Kroah-Hartman]

On Tue, Aug 28, 2012 at 05:55:45PM -0500, Rob Landley wrote:
> I've always been a bit confused by the debugfs design, which seems a
> giant compost heap like /proc where we find a specific styrofoam cup
> useful and the temporary thing becomes permanent. (Why is there _one_
> debugfs?)

The rules for debugfs are very simple:
	There are no rules.

That's it.  It's up to the kernel developer to do what they need to do,
for debugging stuff, how ever they best see fit.

Yes, it replaces proc for all of the debugging stuff that shouldn't have
been there before, how it's structured, is up to the developer adding
the code.

greg k-h

Re: Hardening debugfs (Was Re: [PATCH] debugfs: more tightly restrict default mount mode)

[Rob Landley]

On 08/28/2012 11:09 PM, Greg Kroah-Hartman wrote:
> On Tue, Aug 28, 2012 at 05:55:45PM -0500, Rob Landley wrote:
>> I've always been a bit confused by the debugfs design, which seems a
>> giant compost heap like /proc where we find a specific styrofoam cup
>> useful and the temporary thing becomes permanent. (Why is there _one_
>> debugfs?)
> 
> The rules for debugfs are very simple:
> 	There are no rules.
> 
> That's it.  It's up to the kernel developer to do what they need to do,
> for debugging stuff, how ever they best see fit.

Emergent de-facto standards with no review or versioning or anything,
check. It's the "every driver in the system shares a common resource
with no arbitration or guidelines" thing that I have trouble with.

Is it possible to have multiple instances of this filesystem? If so, can
they have -o "modulename,modulename,modulename..." so that only certain
modules' files get put in this instance?

I understand that /sys/module/$BLAH has rules and we can't have that,
but having a giant slush pile and asserting it won't become a new /proc
because reasons makes me nervous.

For example, ftrace is already built on top of debugfs. If debugfs has
no rules, but ftrace does, can another module put stuff in "tracing"? If
a stable API isn't important, will we start bundling whatever userspace
tools ftrace needs in with the kernel, along with udev/systemd? Or is
the position that ftrace will never have non-debugging uses, just like
ptrace?

> Yes, it replaces proc for all of the debugging stuff that shouldn't have
> been there before, how it's structured, is up to the developer adding
> the code.

For a definition of "replaces" that does not actually remove any of the
existing entries from /proc. (That garbage can's full, here's a new one?)

The problem with /proc is that lots of things used it for different
purposes with no rules. The goal of debugfs is to provide something that
lots of things use for different purposes, explicitly with no rules.

I'm just wondering why it's a big common hairball instead of separate
per-module filesystems. (Other than "Linux will never have a unionfs
because the perfect is the enemy of the good".)

Eh, it's your thing, not mine. I just don't understand it.

> greg k-h

Rob
-- 
GNU/Linux isn't: Linux=GPLv2, GNU=GPLv3+, they can't share code.
Either it's "mere aggregation", or a license violation.  Pick one.