Re: [PATCH v11 21/48] ext4: Add richacl feature flag

From: Andreas Dilger <adilger-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
To: Austin S Hemmelgarn <ahferroin7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Andreas Gruenbacher
	<agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Alexander Viro
	<viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>,
	Theodore Ts'o <tytso-3s7WtUTddSA@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 Developers <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 10:39:15 -0600	[thread overview]
Message-ID: <5DE0621B-7308-41C9-A147-32E0270B28A8@dilger.ca> (raw)
In-Reply-To: <5625182C.3050007-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

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

On Oct 19, 2015, at 10:19 AM, 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.

I would tend to agree here.  Anyone who can mount the filesystem on a kernel
without RichACL support can do whatever they want, so at most having a
RO_COMPAT flag would serve as a reminder for accidental problems by a
sysadmin not in the know.  Using an INCOMPAT flag is just asking for major
headaches when someone needs to recover their filesystem on an old kernel
but doesn't provide any added safety.

Cheers, Andreas

> 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.
> 

Cheers, Andreas

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]