Bug#770492: [RFC PATCH RESEND] vfs: Move security_inode_killpriv() after permission checks

2015-01-21 Thread Casey Schaufler
On 1/21/2015 6:03 AM, Stephen Smalley wrote:
 On 01/20/2015 06:17 PM, James Morris wrote:
 On Sat, 17 Jan 2015, Ben Hutchings wrote:

 chown() and write() should clear all privilege attributes on
 a file - setuid, setgid, setcap and any other extended
 privilege attributes.

 However, any attributes beyond setuid and setgid are managed by the
 LSM and not directly by the filesystem, so they cannot be set along
 with the other attributes.

 Currently we call security_inode_killpriv() in notify_change(),
 but in case of a chown() this is too early - we have not called
 inode_change_ok() or made any filesystem-specific permission/sanity
 checks.

 Add a new function setattr_killpriv() which calls
 security_inode_killpriv() if necessary, and change the setattr()
 implementation to call this in each filesystem that supports xattrs.
 This assumes that extended privilege attributes are always stored in
 xattrs.
 It'd be useful to get some input from LSM module maintainers on this. 

 e.g. doesn't SELinux already handle this via policy directives?
 There have been a couple postings of a similar patch set [1] by Jan
 Kara, although I don't believe that series addressed chown().

 If I am reading the patches correctly, they (correctly) don't affect
 SELinux or Smack labels; they are just calling the existing
 security_inode_killpriv() hook, which is only implemented for the
 capability module to remove the security.capability xattr.

The description of the change should say that. I can easily
imagine an enthusiastic test developer reading the existing
description and filing bugs because SELinux, Smack and whatever
other xattr based systems might be around don't clear their
attributes. If the intent wasn't clear to the first person to
use xattrs for security purposes, I shouldn't expect the new
and inexperienced to see it.

My position softens. Document it correctly, and I'm fine with it.


 [1] http://marc.info/?l=linux-security-modulem=141890696325054w=2



-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/54bfd090.4000...@schaufler-ca.com



Bug#770492: [RFC PATCH RESEND] vfs: Move security_inode_killpriv() after permission checks

2015-01-20 Thread Casey Schaufler
On 1/20/2015 3:17 PM, James Morris wrote:
 On Sat, 17 Jan 2015, Ben Hutchings wrote:

 chown() and write() should clear all privilege attributes on
 a file - setuid, setgid, setcap and any other extended
 privilege attributes.

 However, any attributes beyond setuid and setgid are managed by the
 LSM and not directly by the filesystem, so they cannot be set along
 with the other attributes.

 Currently we call security_inode_killpriv() in notify_change(),
 but in case of a chown() this is too early - we have not called
 inode_change_ok() or made any filesystem-specific permission/sanity
 checks.

 Add a new function setattr_killpriv() which calls
 security_inode_killpriv() if necessary, and change the setattr()
 implementation to call this in each filesystem that supports xattrs.
 This assumes that extended privilege attributes are always stored in
 xattrs.
 It'd be useful to get some input from LSM module maintainers on this.

I've already chimed in.

Clearing the Smack label on a file because someone writes to it
makes no sense whatsoever. The same with chown. The Smack label is
attached to the object, which is a container of data, not the data
itself. Smack labels are Mandatory Access Control labels, not Information
labels. If that doesn't mean anything to the reader, check out the
P1003.1e/2c (withdrawn) DRAFT.

The proposed implementation does not correctly handle either
Mandatory Access Control labels or Information labels. The MAC
label is *very different* from the setuid bit.


 e.g. doesn't SELinux already handle this via policy directives?




-- 
To UNSUBSCRIBE, email to debian-kernel-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: https://lists.debian.org/54bee5a5.5050...@schaufler-ca.com



Re: [RFC] Simplifying kernel configuration for distro issues

2012-07-13 Thread Casey Schaufler
On 7/13/2012 1:37 PM, Linus Torvalds wrote:
 So this has long been one of my pet configuration peeves: as a user I
 am perfectly happy answering the questions about what kinds of
 hardware I want the kernel to support (I kind of know that), but many
 of the support infrastructure questions are very opaque, and I have
 no idea which of the them any particular distribution actually depends
 on.

 And it tends to change over time. For example, F14 (iirc) started
 using TMPFS and TMPFS_POSIX_ACL/XATTR for /dev. And starting in F16,
 the initrd setup requires DEVTMPFS and DEVTMPFS_MOUNT. There's been
 several times when I started with my old minimal config, and the
 resulting kernel would boot, but something wouldn't quite work right,
 and it can be very subtle indeed.

 Similarly, the distro ends up having very particular requirements for
 exactly *which* security models it uses and needs, and they tend to
 change over time. And now with systemd, CGROUPS suddenly aren't just
 esoteric things that no normal person would want to use, but are used
 for basic infrastructure. And I remember being surprised by OpenSUSE
 suddenly needing the RAW table support for netfilter, because it had a
 NOTRACK rule or something.

 The point I'm slowly getting to is that I would actually love to have
 *distro* Kconfig-files, where the distribution would be able to say
 These are the minimums I *require* to work.

Oh dear. I would expect Fedora to say that they require SELinux,
thereby making it unusable by anyone doing LSM development. It
would also make it more difficult for the people who don't want
any LSM (e.g. everyone sane) to configure the kernel they want.

This is the example that I see because of my particular biases.
I expect that there are similar things that distros do in other
areas that will have the same effect. The distro developers may
have decided that a feature is too cool to live without and
include it in their configuration even when it's not really
necessary. Plus, do you really think that they're going to
clean things out of their configuration when they decide that
they no longer need them?


  So we'd have a Distro
 submenu, where you could pick the distro(s) you use, and then pick
 which release, and we'd have something like

  - distro/Kconfig:

 config DISTRO_REQUIREMENTS
 bool Pick minimal distribution requirements

 choice DISTRO
 prompt Distribution
 depends on DISTRO_REQUIREMENTS

 config FEDORA
 config OPENSUSE
 config UBUNTU
 ...

 endchoice

 and then depending on the DISTRO config, we'd include one of the
 distro-specific ones with lists of supported distro versions and then
 the random config settings for that version:

  - distro/Kconfig.suse:

 config OPENSUSE_121
 select OPENSUSE_11
 select IP_NF_RAW  # ..

  - distro/Kconfig.Fedora:

 config FEDORA_16
 select FEDORA_15
 select DEVTMPFS   # F16 initrd needs this
 select DEVTMPFS_MOUNT  # .. and expects the kernel to mount
 DEVTMPFS automatically
 ...

 config FEDORA_17
 select FEDORA_16
 select CGROUP_xyzzy
 ...

 and the point would be that it would make it much easier for a normal
 user (and quite frankly, I want to put myself in that group too) to
 make a kernel config that just works.

 Sure, you can copy the config file that came with the distro, but it
 has tons of stuff that really isn't required. Not just in hardware,
 but all the debug choices etc that are really a user choice. And it's
 really hard to figure out - even for somebody like me - what a minimal
 usable kernel is.

 And yes, I know about make localmodconfig. That's missing the point
 for the same reason the distro config is missing the point.

 Comments? It doesn't have to start out perfect, but I think it would
 *really* help make the kernel configuration much easier for people.

 In addition to the minimal distro settings, we might also have a few
 common platform settings, so that you could basically do a hey, I
 have a modern PC laptop, make it pick the obvious stuff that a normal
 person needs, like USB storage, FAT/VFAT support, the core power
 management etc. The silly stuff that you need, and that
 localyesconfig actually misses because if you haven't inserted a USB
 thumb drive, you won't necessarily have the FAT module loaded, but we
 all know you do want it in real life. But that's really independent
 issue, so let's keep it to just distro core things at first, ok?

 Would something like this make sense to people? I really think that
 How do I generate a kernel config file is one of those things that
 keeps normal people from compiling their own kernel. And we *want*
 people to compile their own kernel so that they can help with things
 like bisecting etc. The more, the merrier.

 Linus
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to