linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Austin S Hemmelgarn <ahferroin7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Andreas Gruenbacher <agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Alexander Viro
	<viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>,
	Theodore Ts'o <tytso-3s7WtUTddSA@public.gmane.org>,
	Andreas Dilger
	<adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>,
	"J. Bruce Fields"
	<bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>,
	Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>,
	Trond Myklebust
	<trond.myklebust-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>,
	Anna Schumaker
	<anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>,
	Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org>,
	linux-ext4 <linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	xfs-VZNHf3L845pBDgjK7y7TUQ@public.gmane.org,
	LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-fsdevel
	<linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linux NFS Mailing List
	<linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux API <linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"Aneesh Kumar K.V"
	<aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Subject: Re: [PATCH v11 21/48] ext4: Add richacl feature flag
Date: Mon, 19 Oct 2015 14:45:09 -0400	[thread overview]
Message-ID: <56253A35.4070309@gmail.com> (raw)
In-Reply-To: <CAHc6FU75GXGeav1ho-QraPS_F8fpOXnoDyv17+b=koiF=9YE5A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

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

On 2015-10-19 13:33, Andreas Gruenbacher wrote:
> On Mon, Oct 19, 2015 at 6:19 PM, Austin S Hemmelgarn
> <ahferroin7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 2015-10-19 11:34, Andreas Gruenbacher wrote:
>>> On Mon, Oct 19, 2015 at 3:16 PM, Austin S Hemmelgarn
>>> <ahferroin7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> On 2015-10-16 13:41, Andreas Gruenbacher wrote:
>>>>> On Fri, Oct 16, 2015 at 7:31 PM, Austin S Hemmelgarn
>>>>> <ahferroin7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>> I would like to re-iterate, on both XFS and ext4, I _really_ think this
>>>>>> should be a ro_compat flag, and not an incompat one.  If a person has
>>>>>> the
>>>>>> ability to mount the FS (even if it's a read-only mount), then they by
>>>>>> definition have read access to the file or partition that the
>>>>>> filesystem
>>>>>> is contained in, which means that any ACL's stored on the filesystem
>>>>>> are
>>>>>> functionally irrelevant,
>>>>> It is unfortunately not safe to make such a file system accessible to
>>>>> other users, so the feature is not strictly read-only compatible.
>>>> OK, seeing as I wasn't particularly clear as to why I object to this in
>>>> my
>>>> other e-mail, let's try this again.
>>>> Can you please explain exactly why it isn't safe to make such a
>>>> filesystem
>>>> accessible to other users?
>>> See here: http://www.spinics.net/lists/linux-ext4/msg49541.html
>> OK, so to clarify, this isn't 'safe' because:
>> 1. The richacls that exist on the filesystem won't be enforced.
>> 2. Newly created files will have no ACL's set.
>>
>> It is worth noting that these are also issues with any kind of access
>> control mechanism.  Using your logic, all LSM's need to set separate
>> incompat feature flags in filesystems they are being used on, as should
>> POSIX ACLs, and for that matter so should Samba in many circumstances, and
>> any NFS system not using idmapping or synchronized/centralized user
>> databases.  Now, if the SELinux (or SMACK, or TOMOYO) people had taken this
>> approach, then I might be inclined to not complain (at least not to you, I'd
>> be complaining to them about this rather poor design choice), but that is
>> not the case, because (I assume) they realized that all this provides is a
>> false sense of security.
>
> LSMs reside above the filesystem level. Let's take SELinux as an
> example. It has its own consistency check mechanism (relabeling). Fsck
> could check the syntax of SELinux labels, but it couldn't do anything
> sensible about corrupted labels, and syntactically correct labels also
> don't mean much. A relabeling run to verify or restory the appropriate
> policy would still be necessary to verify that labels are semantically
> correct, and for that, the filesystem needs to be mounted in the right
> place in the filesystem hierarchy.
>
> TOMOYO and AppArmor are not based on inode labels at all.
Apologies for being unintentionally over-inclusive WRT LSM's, and also 
for forgetting that TOMOYO doesn't use inode labels.
> LSMs usually also just provide an extra layer of security; when turned
> off, the basic security mechanisms still in effect will make sure that
> the system works just like before. (There are configurations like MLS
> where that is not the case, but those are uncommon.)
Um, actually no, even without MLS (or MCS), there is no guarantee 
whatsoever that the system will work 'just' like before (I've actually 
seen systems break on any (or even in one case all) of the transitions 
between enforcing/permissive/off for SELinux, even assuming that 
relabeling is done properly).
> ACLs are quite different from that. They can be checked statically by
> fsck. They are a basic security concept, and when turned off, there is
> no guarantee that the system will still be safe.
LSM's hook into the VFS code right alongside the regular permissions 
checks. They are intended to supplement the regular UNIX DAC based 
permissions. Richacls (and POSIX ACLs) also hook into the regular 
permissions checks, and also are intended to supplement the regular UNIX 
DAC based permissions. Fsck only checks the syntax of regular UNIX DAC 
based permissions (it doesn't verify that the listed UID/GID are 
actually valid in userspace, nor does it check for semantically 
nonsensical permissions modes like 007 or 000), and it really can't 
properly check anything more than that on ACL's either.  Based on all of 
this, richacls and LSM's which mediate filesystem accesses are on 
exactly the same level with the exception that LSM's usually have way 
more functionality beyond just using ACL's to control file access, LSM's 
are usually MAC based while ACL's are DAC based, and the fact that ACL's 
are (usually) not dependent on where in the filesystem hierarchy they 
are found.

On top of that, there is no guarantee that a system will still be safe 
when you turn SELinux (or other LSM's) off either (in fact, for some 
configurations, it can be mathematically proven that the system will not 
be safe if you turn off SELinux).
>> Issue 1, as I have said before, is functionally irrelevant for anyone who
>> actually knows what they are doing; all you need for ext* is one of the
>> myriad of programs for un-deleting files on such a filesystem (such as
>> ext4magic or extundelete, and good luck convincing them to not allow being
>> used when this flag is set), for BTRFS you just need the regular filesystem
>> administration utilities ('btrfs restore' works wonders, and that one will
>> _never_ honor any kind of permissions, because it's for disaster recovery),
>> and while I don't know of any way to do this with XFS, that is only because
>> I don't use XFS myself and have not had the need to provide tech support for
>> anyone who does.  If somebody absolutely _needs_ a guarantee that the acls
>> will be enforced, they need to be using whole disk encryption, not just
>> acls, and even that can't provide such a guarantee.
>>
>> As for issue 2, that can be solved by making it a read-only compatible flag,
>> which is what I was suggesting be done in the first place.  The only
>> situation I can think of that this would cause an issue for is if the
>> filesystem was not cleanly unmounted, and the log-replay doesn't set the
>> ACL's, but mounting an uncleanly unmounted filesystem that has richacls on a
>> kernel without support should fall into one of the following 2 cases more
>> than 99% of the time:
>> 1. The system crashed hard, and the regular kernel is un-bootable for some
>> reason, in this case you're at the point of disaster recovery, should not be
>> exposing _anything_ to a multi-user environment, and probably care a lot
>> more about being able to get the system running again than about not
>> accidentally creating a file with a missing ACL.
>> 2. The filesystem was maliciously stolen in some way (either the hardware
>> was acquired, or more likely, someone got an image of a still mounted
>> filesystem), in which case all of my statements above regarding issue 1
>> apply.
> Please spare me with all that nonsense. Compared to mount options,
> filesystem feature flags in this case simplify things (you don't have
> to specify whether a filesystem contains POSIX ACLs or richacls), and
> they prevent administrator errors: when a filesystem mounts, it is
> safe to use; when it doesn't, it is not. That's all there is to it.
You're ignoring what I'm actually saying.  I've said absolutely nothing 
about needing to use mount options at all, and I'm not arguing against 
using filesystem feature flags, I'm arguing for using them sensibly in a 
way that does not present a false sense of security.

Making the comparability flag read-only will exactly solve the second 
issue that you outlined for more than 99% of all use cases (and the 
remaining cases are provably irrelevant), still indicate clearly to the 
FS code which type of ACL is in use on the FS, and remove the entirely 
false sense of security WRT the first issue you outlined.

Making it an incompatible flag will likely cause headaches for some 
legitimate users, and at most delay competent hackers by a few seconds 
to a few minutes, and script kiddies by a few hours, and is really no 
better than security by obscurity (and from a purely logistical 
standpoint, that's _all_ it is) in that it actively tries to hide the 
fact that someone having read access to the storage the filesystem is on 
can bypass the ACL's.

To reiterate, if someone can call mount() on a filesystem, and mount() 
does not return -EPERM, then even if mount() returns a different error, 
they still have the ability to completely bypass all permissions and 
ACL's in that filesystem, because they have the ability to read the 
entire filesystem directly.

The _only_ way to properly protect against people bypassing the ACL's is 
to use full disk encryption and lock down root access on the system, and 
even that can't completely prevent it from happening.


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3019 bytes --]

  parent reply	other threads:[~2015-10-19 18:45 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-16 15:17 [PATCH v11 00/48] Richacls Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 01/48] vfs: Add IS_ACL() and IS_RICHACL() tests Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 02/48] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 03/48] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD " Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 04/48] vfs: Make the inode passed to inode_change_ok non-const Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 05/48] vfs: Add permission flags for setting file attributes Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 06/48] richacl: In-memory representation and helper functions Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 07/48] richacl: Permission mapping functions Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 08/48] richacl: Compute maximum file masks from an acl Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 09/48] richacl: Permission check algorithm Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 10/48] vfs: Cache base_acl objects in inodes Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 11/48] vfs: Add get_richacl and set_richacl inode operations Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 12/48] vfs: Cache richacl in struct inode Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 14/48] richacl: Check if an acl is equivalent to a file mode Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 15/48] richacl: Create-time inheritance Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 16/48] richacl: Automatic Inheritance Andreas Gruenbacher
2015-10-16 16:00   ` Andy Lutomirski
     [not found]     ` <CALCETrXFkB01tk21FuEOqABHWg1XyOQwsT+s=Lq0RYye6X_7xw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-10-16 16:13       ` Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 17/48] richacl: xattr mapping functions Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 19/48] vfs: Add richacl permission checking Andreas Gruenbacher
2015-10-16 15:17 ` [PATCH v11 20/48] ext4: Add richacl support Andreas Gruenbacher
     [not found] ` <1445008706-15115-1-git-send-email-agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-10-16 15:17   ` [PATCH v11 13/48] richacl: Update the file masks in chmod() Andreas Gruenbacher
2015-10-16 15:17   ` [PATCH v11 18/48] richacl: Add richacl xattr handler Andreas Gruenbacher
2015-10-16 15:17   ` [PATCH v11 21/48] ext4: Add richacl feature flag Andreas Gruenbacher
     [not found]     ` <1445008706-15115-22-git-send-email-agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-10-16 17:31       ` Austin S Hemmelgarn
2015-10-16 17:41         ` Andreas Gruenbacher
     [not found]           ` <CAHc6FU7sR2zN-K3un74wCv+1NPnrqJ=LYiWo+YQ_2X0kopyoTQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-10-16 18:27             ` Austin S Hemmelgarn
     [not found]               ` <562141AD.60302-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-10-17 23:17                 ` Dave Chinner
2015-10-19 13:12                   ` Austin S Hemmelgarn
2015-10-19 13:16             ` Austin S Hemmelgarn
     [not found]               ` <5624ED40.7040206-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-10-19 15:34                 ` Andreas Gruenbacher
2015-10-19 16:19                   ` Austin S Hemmelgarn
     [not found]                     ` <5625182C.3050007-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-10-19 16:39                       ` Andreas Dilger
2015-10-19 17:33                     ` Andreas Gruenbacher
     [not found]                       ` <CAHc6FU75GXGeav1ho-QraPS_F8fpOXnoDyv17+b=koiF=9YE5A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-10-19 18:45                         ` Austin S Hemmelgarn [this message]
     [not found]                           ` <56253A35.4070309-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-10-19 20:20                             ` Andreas Gruenbacher
2015-10-20 12:33                               ` Austin S Hemmelgarn
2015-10-16 15:18   ` [PATCH v11 27/48] richacl: Propagate everyone@ permissions to other aces Andreas Gruenbacher
2015-10-16 15:18   ` [PATCH v11 33/48] nfsd: Keep list of acls to dispose of in compoundargs Andreas Gruenbacher
2015-10-16 15:18   ` [PATCH v11 35/48] nfsd: Add richacl support Andreas Gruenbacher
2015-10-16 15:18   ` [PATCH v11 48/48] nfs: Add support for the v4.1 dacl attribute Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 22/48] xfs: Fix error path in xfs_get_acl Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 23/48] xfs: Make xfs_set_mode non-static Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 24/48] xfs: Add richacl support Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 25/48] richacl: acl editing helper functions Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 26/48] richacl: Move everyone@ aces down the acl Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 28/48] richacl: Set the owner permissions to the owner mask Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 29/48] richacl: Set the other permissions to the other mask Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 30/48] richacl: Isolate the owner and group classes Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 31/48] richacl: Apply the file masks to a richacl Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 32/48] richacl: Create richacl from mode values Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 34/48] nfsd: Use richacls as internal acl representation Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 36/48] nfsd: Add support for the v4.1 dacl attribute Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 37/48] nfsd: Add support for the MAY_CREATE_{FILE,DIR} permissions Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 38/48] richacl: Add support for unmapped identifiers Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 39/48] nfsd: Add support for unmapped richace identifiers Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 40/48] ext4: Don't allow unmapped identifiers in richacls Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 41/48] xfs: " Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 42/48] sunrpc: Allow to demand-allocate pages to encode into Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 43/48] sunrpc: Add xdr_init_encode_pages Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 44/48] nfs: Fix GETATTR bitmap verification Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 45/48] nfs: Remove unused xdr page offsets in getacl/setacl arguments Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 46/48] nfs: Distinguish missing users and groups from nobody Andreas Gruenbacher
2015-10-16 15:18 ` [PATCH v11 47/48] nfs: Add richacl support Andreas Gruenbacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56253A35.4070309@gmail.com \
    --to=ahferroin7-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org \
    --cc=agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org \
    --cc=anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org \
    --cc=bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org \
    --cc=david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org \
    --cc=jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org \
    --cc=linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=trond.myklebust-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org \
    --cc=tytso-3s7WtUTddSA@public.gmane.org \
    --cc=viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org \
    --cc=xfs-VZNHf3L845pBDgjK7y7TUQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).