Re: e2fsprogs: Richacl support

Re: e2fsprogs: Richacl support

[Andreas Gruenbacher]

On Sat, Oct 17, 2015 at 1:16 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Fri, Oct 16, 2015 at 06:03:29PM +0200, Andreas Gruenbacher wrote:
>>
>> could the richacl feature flag please be added to e2fsprogs so that we
>> won't end up with incompatible file systems?
>>
>>   https://github.com/andreas-gruenbacher/e2fsprogs
>>
>> Also, should this really be an incompatible feature flag? With a
>> read-only compatibility flag, mounting a richacl filesystem on a
>> kernel without richacl support would work but it's not safe --- it
>> could grant unwanted access to files. (The same applies to the xfs
>> support, etc.)
>
> Richacl's are represented using just extended attributes, right?

Yes.

Richacls can be enabled per file system; they are mutually exclusive
with POSIX ACLs. You usually define which kind of ACLs a filesystem
should support at filesystem create time, and that choice sticks with
the filesystem. So using a feature flag makes sense.

> Suppose we mounted a file system with richacl's on a kernel that
> didn't understand it, and we write to from that non-richacl kernel.
> What's the worse that could happen?

Two things are likely to happen. First, richacls will not be enforced;
this can cause fewer or more permissions to be granted. Second, when
files are created, permission inheritance will not take place, so when
the filesystem is later used by a richacl aware kernel, permissions
will be inconsistent.

> So why would this result in incompatible file systems?

The filesystems will not become incompatible in an e2fsck sense, but
it will generally become unsafe to expose in a multi-user environment.
So the question is what the different kinds of feature flags are
supposed to protect from exactly.

> For similar reasons we never had a feature flag for Posix ACL's.

Indeed, if you mount a filesystem that contains POSIX ACLs on a kernel
that doesn't support them, you can end up with the same kinds of
inconsistencies. Bad enough.

Thanks,
Andreas

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: e2fsprogs: Richacl support

[Theodore Ts'o]

On Sat, Oct 17, 2015 at 02:22:25AM +0200, Andreas Gruenbacher wrote:
> > Suppose we mounted a file system with richacl's on a kernel that
> > didn't understand it, and we write to from that non-richacl kernel.
> > What's the worse that could happen?
> 
> Two things are likely to happen. First, richacls will not be enforced;
> this can cause fewer or more permissions to be granted. Second, when
> files are created, permission inheritance will not take place, so when
> the filesystem is later used by a richacl aware kernel, permissions
> will be inconsistent.
> 
> > So why would this result in incompatible file systems?
> 
> The filesystems will not become incompatible in an e2fsck sense, but
> it will generally become unsafe to expose in a multi-user environment.
> So the question is what the different kinds of feature flags are
> supposed to protect from exactly.

Hmm.  So my original conception of the ext2/3/4 feature flags was more
about incompatibility from a e2fsck sense --- what I would call in a
metadata self-consistency sense.  I don't think we ever thought about
the failure of the inheritance to be an "inconsistency".  Perhaps it
would be considered such from a semantic point of view, but it's not
something we could ever check using e2fsck, since it's possible that
the user had intentionally removed the richacl from a file, even
though all of the other files in the directory have a richacl which
matches the default directory inheritance acl.

So the downside of using a feature flag (and it doesn't matter whether
it's an incompat or a read-only flag) is that it requires new versions
of e2fsprogs in order to be able to support the file system --- when
it really doesn't matter from an e2fsprogs support perspective.

One way of dealing with this is would be use a new mount option to
enable richacls (e.g., "richacl"), which older kernels would fail to
recognize and would thus fail the mount.  This works since the most
likely failure case is if the user boots into an older kernel.

This doesn't protect against the case where someone is using richacls
on a USB stick, and moves it over to another system --- but that seems
like a use case that's not very likely.  It also doesn't help against
someone using an old rescue cd-rom, but in that case, it's not clear
that it's hugely problematic, either -- someone using rescue media has
physical access to the machine, so access controls are generally not a
major concern, and they are also not likely to be making wholesale
changes to the file system where acl inheritance is going to be a
primary concern, either.

This is basically how we handled the POSIX acl situation for ext4 ---
we just required a mount option to enable it, originally and
eventually we defaulted the option to always on once it was highly
unlikely someone was going to be trying to install, say, a 2.6 kernel
on an enterprise userspace --- which was not likely to work at all.

What do you think?  If we really want to burn a feature flag, I'd
probably use a RO_INCOMPAT flag rather than an INCOMPAT flag, on the
theory that read-only mounts are much more often used for system
administration.  But it might be using a mount option is a better way
to go here.

Cheers,

							- Ted

P.S.  I can imagine adding new KERNEL_INCOMPAT and KERNEL_ROCOMPAT
feature set flags in the superblock which are ignored by e2fsprogs,
but only used by the kernel.  This might be helpful in the future, but
it won't help us now, since old kernels won't be checking them.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: e2fsprogs: Richacl support

[Dave Chinner]

On Sat, Oct 17, 2015 at 10:39:51AM -0400, Theodore Ts'o wrote:
> On Sat, Oct 17, 2015 at 02:22:25AM +0200, Andreas Gruenbacher wrote:
> One way of dealing with this is would be use a new mount option to
> enable richacls (e.g., "richacl"), which older kernels would fail to
> recognize and would thus fail the mount.  This works since the most
> likely failure case is if the user boots into an older kernel.

I don't think this is a good idea. The rich-ACL feature bit is an
/on-disk format/ identifier. Both the kernel and the filesystem need
to support the feature for it to work, and that's exactly what we
use feature bits for.

For XFS userspace we also need this feature bit for xfs_repair so
that it doesn't attempt to validate the on-disk ACL format is in a
valid posix ACL format. IOWs, we need an on-disk feature bit to
indicate the on-disk format of the ACL in XFS.

Further, a user could mount the ext4 fs with "norichacl,acl" to
force the kernel to parse the rich acls as posix acls because
without a feature bit the fs does not know what format it's acls are
in on-disk. This will only end in tears for users.

> This doesn't protect against the case where someone is using richacls
> on a USB stick, and moves it over to another system --- but that seems
> like a use case that's not very likely.  It also doesn't help against
> someone using an old rescue cd-rom, but in that case, it's not clear
> that it's hugely problematic, either -- someone using rescue media has
> physical access to the machine, so access controls are generally not a
> major concern, and they are also not likely to be making wholesale
> changes to the file system where acl inheritance is going to be a
> primary concern, either.
> 
> This is basically how we handled the POSIX acl situation for ext4 ---
> we just required a mount option to enable it, originally and
> eventually we defaulted the option to always on once it was highly
> unlikely someone was going to be trying to install, say, a 2.6 kernel
> on an enterprise userspace --- which was not likely to work at all.

Using a mount option was wrong here, too, because ext4 was a *brand
new filesystem*. ext4 should have just defaulted to enabling posix
acls if the support was built into the kernel. Just like XFS had
already been doing for years before ext4 came along....

> What do you think?  If we really want to burn a feature flag, I'd
> probably use a RO_INCOMPAT flag rather than an INCOMPAT flag, on the
> theory that read-only mounts are much more often used for system
> administration.  But it might be using a mount option is a better way
> to go here.

I asked this same question on the kernel side of things. While I
think that from an "on-disk consistency" POV using RO_INCOMPAT would
be fine, from a user visible POV the behaviour won't be compatible
and so INCOMPAT makes sense from that perspective.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: e2fsprogs: Richacl support

[Theodore Ts'o]

On Sun, Oct 18, 2015 at 09:59:10AM +1100, Dave Chinner wrote:
> For XFS userspace we also need this feature bit for xfs_repair so
> that it doesn't attempt to validate the on-disk ACL format is in a
> valid posix ACL format. IOWs, we need an on-disk feature bit to
> indicate the on-disk format of the ACL in XFS.
> 
> Further, a user could mount the ext4 fs with "norichacl,acl" to
> force the kernel to parse the rich acls as posix acls because
> without a feature bit the fs does not know what format it's acls are
> in on-disk. This will only end in tears for users.

So at least for ext4, the richacl xattr using a completely different
index, and so it's not possible to interpret the richacl as an acl.
The only question is whether we pay attention to the richacl acl's at
all.  One thing that's not clear to me is what VFS is supposed to do
if an inode has both an Posix ACL xattr and a Richacl xattr at the
same time.

We're also not trying to validate the on-disk ACL format at all using
e2fsck, just as we're not trying to validate say, the SELinux xattr.
They are just bags of bits as far as e2fsck is concerned.  So for that
reason, at least as the patches I've seen for richacl for ext4,
e2fsprogs doesn't actually need a file system feature bit at all.  We
could in theory use the 1.42 version of e2fsck against an ext4 file
system with richacl, and modulo the incompat flag, there wouldn't be
any problems from e2fsck point of view.

What Andreas G. has said is that the only problem is if the file
system is mounted read/write, and someone creates an inode on a
non-richacl knowledgeable kernel, the inode would get created w/o an
richacl.  This might be surprising to the user if he or she was
expecting richacl semantics, but that would only happen if they had
say, enabled richacl's on a distribution that had full richacl support
(including the richacl userspace utilities so they could set and get
richacl entries), and then booted an older kernel on that
distribution.  This doesn't seem like _that_ likely a scenario, but I
suppose it could happen.  And whether you call the file system
"inconsistent" really depends on your point of view.  From the
perspective of fsck, which for ext4 is completely, happily ignorant of
richacl support, it's not inconsistent and all of the inodes are
valid.

> I asked this same question on the kernel side of things. While I
> think that from an "on-disk consistency" POV using RO_INCOMPAT would
> be fine, from a user visible POV the behaviour won't be compatible
> and so INCOMPAT makes sense from that perspective.

If you believe that the only real use of richacl will be primarily for
supporting Samba and NFSv4 servers, then using a read-only compat
feature flag isn't that terrible, and it might be helpful in the case
of someone using a rescue media that hasn't been updated yet to be
able to access the system --- since it seems unlikely that the user
would try to launch a Samba or NFSv4 server from a rescue CD.

But I different people of good will can differ with each other, and at
the end of the day I don't think it makes *that* much difference
unless we drop the use of the feature flag entirely.  Given that
systemd has infected all of the distributions, and systemd is getting
more and more tightly integrated into modern kernel features w/o
offerring backwards compatibility, it seems very likely to me that you
wouldn't even be able to *boot* a downrev kernel on a modern
distribution that has richacl support without seeing systemd or its
dependencies explode into millions of tiny pleaces, so I'm not that
terribly worried one way or another.  :-)

						- Ted

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: e2fsprogs: Richacl support

[Andreas Gruenbacher]

On Sun, Oct 18, 2015 at 2:35 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Sun, Oct 18, 2015 at 09:59:10AM +1100, Dave Chinner wrote:
>> For XFS userspace we also need this feature bit for xfs_repair so
>> that it doesn't attempt to validate the on-disk ACL format is in a
>> valid posix ACL format. IOWs, we need an on-disk feature bit to
>> indicate the on-disk format of the ACL in XFS.
>>
>> Further, a user could mount the ext4 fs with "norichacl,acl" to
>> force the kernel to parse the rich acls as posix acls because
>> without a feature bit the fs does not know what format it's acls are
>> in on-disk. This will only end in tears for users.
>
> So at least for ext4, the richacl xattr using a completely different
> index, and so it's not possible to interpret the richacl as an acl.

It's the same on xfs.

> The only question is whether we pay attention to the richacl acl's at
> all.  One thing that's not clear to me is what VFS is supposed to do
> if an inode has both an Posix ACL xattr and a Richacl xattr at the
> same time.

The VFS will either look for POSIX ACLs or for a richacl; it won't
even notice if both are present.

> We're also not trying to validate the on-disk ACL format at all using
> e2fsck, just as we're not trying to validate say, the SELinux xattr.
> They are just bags of bits as far as e2fsck is concerned.

Right now, filesystems that e2fsck is perfectly happy with can still
cause errors when used. It would be nice to fix that.

With POSIX ACLs, this problem is slightly less severe because the ACL
isn't looked at for the owner; it would even be possible to replace a
corrupted POSIX ACL. Richacls unfortunately don't allow this
optimization.

>> I asked this same question on the kernel side of things. While I
>> think that from an "on-disk consistency" POV using RO_INCOMPAT would
>> be fine, from a user visible POV the behaviour won't be compatible
>> and so INCOMPAT makes sense from that perspective.
>
> If you believe that the only real use of richacl will be primarily for
> supporting Samba and NFSv4 servers, then using a read-only compat
> feature flag isn't that terrible, and it might be helpful in the case
> of someone using a rescue media that hasn't been updated yet to be
> able to access the system --- since it seems unlikely that the user
> would try to launch a Samba or NFSv4 server from a rescue CD.

You wouldn't be able to fsck a filesystem that has unknown features,
incompat or ro-incompat, from such a rescue environment.

> But I different people of good will can differ with each other, and at
> the end of the day I don't think it makes *that* much difference
> unless we drop the use of the feature flag entirely.

This really should be a feature flag and not a mount option, it just
doesn't make sense to switch at mount time.

>From this discussion, I'm even more convinced that we should use an
incompat feature rather than a ro-incompat feature.

Thanks,
Andreas

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: e2fsprogs: Richacl support

[Theodore Ts'o]

On Sun, Oct 18, 2015 at 10:46:23PM +0200, Andreas Gruenbacher wrote:
> > The only question is whether we pay attention to the richacl acl's at
> > all.  One thing that's not clear to me is what VFS is supposed to do
> > if an inode has both an Posix ACL xattr and a Richacl xattr at the
> > same time.
> 
> The VFS will either look for POSIX ACLs or for a richacl; it won't
> even notice if both are present.

How does this work in practice?  Does it look for richacl's first, and
if it doesn't find it, it will then look for a Posix ACL, or vice
versa?

> Right now, filesystems that e2fsck is perfectly happy with can still
> cause errors when used. It would be nice to fix that.
> 
> With POSIX ACLs, this problem is slightly less severe because the ACL
> isn't looked at for the owner; it would even be possible to replace a
> corrupted POSIX ACL. Richacls unfortunately don't allow this
> optimization.

Is there code we can use to verify a richacl, and if it's corrupted,
what are the options about how we can fix it?  Or do we just remove
it, and just use the inode's i_uid field for the owner instead of
whatever might be in the richacl?

Ideally, if you can send the patch to add support to validate / fix
Richacl's in e2fsck, that would be great.

> This really should be a feature flag and not a mount option, it just
> doesn't make sense to switch at mount time.
> 
> From this discussion, I'm even more convinced that we should use an
> incompat feature rather than a ro-incompat feature.

OK, let's go with that.

    	  					- Ted

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: e2fsprogs: Richacl support

[Andreas Gruenbacher]

On Sun, Oct 18, 2015 at 11:44 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Sun, Oct 18, 2015 at 10:46:23PM +0200, Andreas Gruenbacher wrote:
>> > The only question is whether we pay attention to the richacl acl's at
>> > all.  One thing that's not clear to me is what VFS is supposed to do
>> > if an inode has both an Posix ACL xattr and a Richacl xattr at the
>> > same time.
>>
>> The VFS will either look for POSIX ACLs or for a richacl; it won't
>> even notice if both are present.
>
> How does this work in practice?  Does it look for richacl's first, and
> if it doesn't find it, it will then look for a Posix ACL, or vice
> versa?

The filesystem sets the MS_POSIXACL super-block flag for POSIX ACLs or
the MS_RICHACL super-block flag for richacls. These flags are checked
with the IS_POSIXACL(inode) and IS_RICHACL(inode) macros.

>> Right now, filesystems that e2fsck is perfectly happy with can still
>> cause errors when used. It would be nice to fix that.
>>
>> With POSIX ACLs, this problem is slightly less severe because the ACL
>> isn't looked at for the owner; it would even be possible to replace a
>> corrupted POSIX ACL. Richacls unfortunately don't allow this
>> optimization.
>
> Is there code we can use to verify a richacl, and if it's corrupted,
> what are the options about how we can fix it?  Or do we just remove
> it, and just use the inode's i_uid field for the owner instead of
> whatever might be in the richacl?

The on-disk format is relatively simple, it's the same on ext4 and on
xfs and hopefully will be the same on other filesystems as well. That
code could be shared.

If a richacl is found to be corrupt, a safe way of recovering would be
to remove the richacl and clear the S_IRWXUGO mode bits. I don't think
trying to fix ACLs would make sense. The i_uid field is unrelated.

> Ideally, if you can send the patch to add support to validate / fix
> Richacl's in e2fsck, that would be great.

I can send the validation code; not sure how removing corrupt xattrs
would fit into e2fsck though. Can e2fsck remove individual xattrs?

>> This really should be a feature flag and not a mount option, it just
>> doesn't make sense to switch at mount time.
>>
>> From this discussion, I'm even more convinced that we should use an
>> incompat feature rather than a ro-incompat feature.
>
> OK, let's go with that.

Okay, good.

Thanks,
Andreas

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: e2fsprogs: Richacl support

[Theodore Ts'o]

On Mon, Oct 19, 2015 at 12:46:49AM +0200, Andreas Gruenbacher wrote:
> The filesystem sets the MS_POSIXACL super-block flag for POSIX ACLs or
> the MS_RICHACL super-block flag for richacls. These flags are checked
> with the IS_POSIXACL(inode) and IS_RICHACL(inode) macros.

Ah, OK.  So it's either one thing or another.

> I can send the validation code; not sure how removing corrupt xattrs
> would fit into e2fsck though. Can e2fsck remove individual xattrs?

Yep.  Using the libext2fs function ext2fs_xattr_remove(), in
lib/ext2fs/ext_attr.c.  This requires the development branch of
e2fsprogs, but by adding a new feature flag, richacl is going to
require a 1.43 version of e2fsprogs (not yet released, but hopefully
will be released soon).

						- Ted

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs