linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* The one and only "permission denied" in find /sys
@ 2021-10-13 20:12 Rob Landley
  2021-10-14  1:48 ` Kees Cook
  0 siblings, 1 reply; 4+ messages in thread
From: Rob Landley @ 2021-10-13 20:12 UTC (permalink / raw)
  To: linux-kernel, Kees Cook, Linux FS Devel

There is exactly one directory in the whole of sysfs that a normal user can't
read (at least on my stock devuan laptop):

  $ find /sys -name potato
  find: ‘/sys/fs/pstore’: Permission denied

It's the "pstore" filesystem, it was explicitly broken by commit d7caa33687ce,
and the commit seems to say this was to fix an issue that didn't exist yet but
might someday.

Did whatever issue it was concerned about ever actually start happening? Why did
you not change the permissions on the files _in_ the directory so they weren't
world readable instead? Should /dev/shm stop being world ls-able as well?

Confused,

Rob

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: The one and only "permission denied" in find /sys
  2021-10-13 20:12 The one and only "permission denied" in find /sys Rob Landley
@ 2021-10-14  1:48 ` Kees Cook
  2021-10-15  9:14   ` Christian Brauner
  0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2021-10-14  1:48 UTC (permalink / raw)
  To: Rob Landley, linux-kernel, Linux FS Devel



On October 13, 2021 1:12:16 PM PDT, Rob Landley <rob@landley.net> wrote:
>There is exactly one directory in the whole of sysfs that a normal user can't
>read (at least on my stock devuan laptop):
>
>  $ find /sys -name potato
>  find: ‘/sys/fs/pstore’: Permission denied
>
>It's the "pstore" filesystem, it was explicitly broken by commit d7caa33687ce,
>and the commit seems to say this was to fix an issue that didn't exist yet but
>might someday.

Right, so, the problem did certainly exist: there was a capability check for opening the files, which made it difficult for pstore collector tools to run with sane least privileges. Adjusting the root directory was the simplest way to keep the files secure by default, and allow a system owner the ability to delegate collector permissions to a user or group via just a chmod on the root directory.

>Did whatever issue it was concerned about ever actually start happening? Why did
>you not change the permissions on the files _in_ the directory so they weren't
>world readable instead? Should /dev/shm stop being world ls-able as well?

Making the per-file permissions configurable at runtime was more complex for little additional gain.

/dev/shm has the benefit of having an existing permission model for each created file.

I wouldn't be opposed to a mount option to specify the default file owner/group, but it makes user space plumbing more difficult (i.e. last I checked, stuff like systemd tends to just mount kernel filesystems without options).

-Kees

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: The one and only "permission denied" in find /sys
  2021-10-14  1:48 ` Kees Cook
@ 2021-10-15  9:14   ` Christian Brauner
  2021-10-15 21:40     ` Kees Cook
  0 siblings, 1 reply; 4+ messages in thread
From: Christian Brauner @ 2021-10-15  9:14 UTC (permalink / raw)
  To: Kees Cook; +Cc: Rob Landley, linux-kernel, Linux FS Devel

On Wed, Oct 13, 2021 at 06:48:26PM -0700, Kees Cook wrote:
> 
> 
> On October 13, 2021 1:12:16 PM PDT, Rob Landley <rob@landley.net> wrote:
> >There is exactly one directory in the whole of sysfs that a normal user can't
> >read (at least on my stock devuan laptop):
> >
> >  $ find /sys -name potato
> >  find: ‘/sys/fs/pstore’: Permission denied
> >
> >It's the "pstore" filesystem, it was explicitly broken by commit d7caa33687ce,
> >and the commit seems to say this was to fix an issue that didn't exist yet but
> >might someday.
> 
> Right, so, the problem did certainly exist: there was a capability check for opening the files, which made it difficult for pstore collector tools to run with sane least privileges. Adjusting the root directory was the simplest way to keep the files secure by default, and allow a system owner the ability to delegate collector permissions to a user or group via just a chmod on the root directory.
> 
> >Did whatever issue it was concerned about ever actually start happening? Why did
> >you not change the permissions on the files _in_ the directory so they weren't
> >world readable instead? Should /dev/shm stop being world ls-able as well?
> 
> Making the per-file permissions configurable at runtime was more complex for little additional gain.
> 
> /dev/shm has the benefit of having an existing permission model for each created file.
> 
> I wouldn't be opposed to a mount option to specify the default file owner/group, but it makes user space plumbing more difficult (i.e. last I checked, stuff like systemd tends to just mount kernel filesystems without options).

Hm, no, we do mount kernel filesystems with different options. :)
So if pstore gains an option that could be changed pretty easily. Unless
you meant something else by kernel filesystems. :)

static const MountPoint mount_table[] = {
        { "proc",        "/proc",                     "proc",       NULL,                                      MS_NOSUID|MS_NOEXEC|MS_NODEV,
          NULL,          MNT_FATAL|MNT_IN_CONTAINER|MNT_FOLLOW_SYMLINK },
        { "sysfs",       "/sys",                      "sysfs",      NULL,                                      MS_NOSUID|MS_NOEXEC|MS_NODEV,
          NULL,          MNT_FATAL|MNT_IN_CONTAINER },
        { "devtmpfs",    "/dev",                      "devtmpfs",   "mode=755" TMPFS_LIMITS_DEV,               MS_NOSUID|MS_STRICTATIME,
          NULL,          MNT_FATAL|MNT_IN_CONTAINER },
        { "securityfs",  "/sys/kernel/security",      "securityfs", NULL,                                      MS_NOSUID|MS_NOEXEC|MS_NODEV,
          NULL,          MNT_NONE                   },
#if ENABLE_SMACK
        { "smackfs",     "/sys/fs/smackfs",           "smackfs",    "smackfsdef=*",                            MS_NOSUID|MS_NOEXEC|MS_NODEV,
          mac_smack_use, MNT_FATAL                  },
        { "tmpfs",       "/dev/shm",                  "tmpfs",      "mode=1777,smackfsroot=*",                 MS_NOSUID|MS_NODEV|MS_STRICTATIME,
          mac_smack_use, MNT_FATAL                  },
#endif
        { "tmpfs",       "/dev/shm",                  "tmpfs",      "mode=1777",                               MS_NOSUID|MS_NODEV|MS_STRICTATIME,
          NULL,          MNT_FATAL|MNT_IN_CONTAINER },
        { "devpts",      "/dev/pts",                  "devpts",     "mode=620,gid=" STRINGIFY(TTY_GID),        MS_NOSUID|MS_NOEXEC,
          NULL,          MNT_IN_CONTAINER           },
#if ENABLE_SMACK
        { "tmpfs",       "/run",                      "tmpfs",      "mode=755,smackfsroot=*" TMPFS_LIMITS_RUN, MS_NOSUID|MS_NODEV|MS_STRICTATIME,
          mac_smack_use, MNT_FATAL                  },
#endif
        { "tmpfs",       "/run",                      "tmpfs",      "mode=755" TMPFS_LIMITS_RUN,               MS_NOSUID|MS_NODEV|MS_STRICTATIME,
          NULL,          MNT_FATAL|MNT_IN_CONTAINER },
        { "cgroup2",     "/sys/fs/cgroup",            "cgroup2",    "nsdelegate,memory_recursiveprot",         MS_NOSUID|MS_NOEXEC|MS_NODEV,
          cg_is_unified_wanted, MNT_IN_CONTAINER|MNT_CHECK_WRITABLE },
        { "cgroup2",     "/sys/fs/cgroup",            "cgroup2",    "nsdelegate",                              MS_NOSUID|MS_NOEXEC|MS_NODEV,
          cg_is_unified_wanted, MNT_IN_CONTAINER|MNT_CHECK_WRITABLE },
        { "cgroup2",     "/sys/fs/cgroup",            "cgroup2",    NULL,                                      MS_NOSUID|MS_NOEXEC|MS_NODEV,
          cg_is_unified_wanted, MNT_IN_CONTAINER|MNT_CHECK_WRITABLE },
        { "tmpfs",       "/sys/fs/cgroup",            "tmpfs",      "mode=755" TMPFS_LIMITS_SYS_FS_CGROUP,     MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_STRICTATIME,
          cg_is_legacy_wanted, MNT_FATAL|MNT_IN_CONTAINER },
        { "cgroup2",     "/sys/fs/cgroup/unified",    "cgroup2",    "nsdelegate",                              MS_NOSUID|MS_NOEXEC|MS_NODEV,
          cg_is_hybrid_wanted, MNT_IN_CONTAINER|MNT_CHECK_WRITABLE },
        { "cgroup2",     "/sys/fs/cgroup/unified",    "cgroup2",    NULL,                                      MS_NOSUID|MS_NOEXEC|MS_NODEV,
          cg_is_hybrid_wanted, MNT_IN_CONTAINER|MNT_CHECK_WRITABLE },
        { "cgroup",      "/sys/fs/cgroup/systemd",    "cgroup",     "none,name=systemd,xattr",                 MS_NOSUID|MS_NOEXEC|MS_NODEV,
          cg_is_legacy_wanted, MNT_IN_CONTAINER     },
        { "cgroup",      "/sys/fs/cgroup/systemd",    "cgroup",     "none,name=systemd",                       MS_NOSUID|MS_NOEXEC|MS_NODEV,
          cg_is_legacy_wanted, MNT_FATAL|MNT_IN_CONTAINER },
        { "pstore",      "/sys/fs/pstore",            "pstore",     NULL,                                      MS_NOSUID|MS_NOEXEC|MS_NODEV,
          NULL,          MNT_NONE                   },
#if ENABLE_EFI
        { "efivarfs",    "/sys/firmware/efi/efivars", "efivarfs",   NULL,                                      MS_NOSUID|MS_NOEXEC|MS_NODEV,
          is_efi_boot,   MNT_NONE                   },
#endif
        { "bpf",         "/sys/fs/bpf",               "bpf",        "mode=700",                                MS_NOSUID|MS_NOEXEC|MS_NODEV,
          NULL,          MNT_NONE,                  },
};

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: The one and only "permission denied" in find /sys
  2021-10-15  9:14   ` Christian Brauner
@ 2021-10-15 21:40     ` Kees Cook
  0 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2021-10-15 21:40 UTC (permalink / raw)
  To: Christian Brauner; +Cc: Rob Landley, linux-kernel, Linux FS Devel

On Fri, Oct 15, 2021 at 11:14:47AM +0200, Christian Brauner wrote:
> On Wed, Oct 13, 2021 at 06:48:26PM -0700, Kees Cook wrote:
> > 
> > 
> > On October 13, 2021 1:12:16 PM PDT, Rob Landley <rob@landley.net> wrote:
> > >There is exactly one directory in the whole of sysfs that a normal user can't
> > >read (at least on my stock devuan laptop):
> > >
> > >  $ find /sys -name potato
> > >  find: ‘/sys/fs/pstore’: Permission denied
> > >
> > >It's the "pstore" filesystem, it was explicitly broken by commit d7caa33687ce,
> > >and the commit seems to say this was to fix an issue that didn't exist yet but
> > >might someday.
> > 
> > Right, so, the problem did certainly exist: there was a capability check for opening the files, which made it difficult for pstore collector tools to run with sane least privileges. Adjusting the root directory was the simplest way to keep the files secure by default, and allow a system owner the ability to delegate collector permissions to a user or group via just a chmod on the root directory.
> > 
> > >Did whatever issue it was concerned about ever actually start happening? Why did
> > >you not change the permissions on the files _in_ the directory so they weren't
> > >world readable instead? Should /dev/shm stop being world ls-able as well?
> > 
> > Making the per-file permissions configurable at runtime was more complex for little additional gain.
> > 
> > /dev/shm has the benefit of having an existing permission model for each created file.
> > 
> > I wouldn't be opposed to a mount option to specify the default file owner/group, but it makes user space plumbing more difficult (i.e. last I checked, stuff like systemd tends to just mount kernel filesystems without options).
> 
> Hm, no, we do mount kernel filesystems with different options. :)
> So if pstore gains an option that could be changed pretty easily. Unless
> you meant something else by kernel filesystems. :)
> 
> static const MountPoint mount_table[] = {
         ^^^^^

right, I should have been more clear. I haven't seen a way for systemd
users to specify different mount options for "kernel filesystems".

> [...]
>         { "pstore",      "/sys/fs/pstore",            "pstore",     NULL,                                      MS_NOSUID|MS_NOEXEC|MS_NODEV,
>           NULL,          MNT_NONE                   },

If it will do /etc/fstab merging, then sure, I'd be open to taking
patches that would make the file ownership/group be mount-time
configurable.

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-15 21:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13 20:12 The one and only "permission denied" in find /sys Rob Landley
2021-10-14  1:48 ` Kees Cook
2021-10-15  9:14   ` Christian Brauner
2021-10-15 21:40     ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).