diff mbox

[RFC,v3,20/45] richacl: Automatic Inheritance

Message ID 28e2cd75064ff56bad897b6f69356f4fb15b8128.1429868795.git.agruenba@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andreas Grünbacher April 24, 2015, 11:04 a.m. UTC
Automatic Inheritance (AI) allows changes to the acl of a directory to
recursively propagate down to files and directories in the directory.

To implement this, the kernel keeps track of which permissions have been
inherited, and makes sure that permission propagation is turned off when the
file permission bits of a file are changed (upon create or chmod).

The actual permission propagation is implemented in user space.

Automatic Inheritance works as follows:

 - When the ACL4_AUTO_INHERIT flag in the acl of a file is not set, the
   file is not affected by AI.

 - When the ACL4_AUTO_INHERIT flag in the acl of a directory is set and
   a file or subdirectory is created in that directory, files created in
   the directory will have the ACL4_AUTO_INHERIT flag set, and all
   inherited aces will have the ACE4_INHERITED_ACE flag set.  This
   allows user space to distinguish between aces which have been
   inherited and aces which have been explicitly added.

 - When the ACL4_PROTECTED acl flag in the acl of a file is set, AI will
   not modify the acl of the file.  This does not affect propagation of
   permissions from the file to its children (if the file is a
   directory).

Linux does not have a way of creating files without setting the file permission
bits, so all files created inside a directory with ACL4_AUTO_INHERIT set will
also have the ACL4_PROTECTED flag set.  This effectively disables Automatic
Inheritance.

Protocols which support creating files without specifying permissions can
explicitly clear the ACL4_PROTECTED flag after creating a file and reset the
file masks to "undo" applying the create mode; see richacl_compute_max_masks().
This is a workaround; a mechanism that would allow a process to indicate to the
kernel to ignore the create mode when there are inherited permissions would fix
this problem.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/richacl_base.c       | 10 +++++++++-
 fs/richacl_inode.c      |  7 ++++++-
 include/linux/richacl.h | 25 +++++++++++++++++++++++--
 3 files changed, 38 insertions(+), 4 deletions(-)

Comments

Frank Filz May 13, 2015, 6:01 p.m. UTC | #1
> -----Original Message-----
> From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-
> owner@vger.kernel.org] On Behalf Of Andreas Gruenbacher
> Sent: Friday, April 24, 2015 4:04 AM
> To: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org; linux-
> nfs@vger.kernel.org
> Subject: [RFC v3 20/45] richacl: Automatic Inheritance
> 
> Automatic Inheritance (AI) allows changes to the acl of a directory to
> recursively propagate down to files and directories in the directory.
> 
> To implement this, the kernel keeps track of which permissions have been
> inherited, and makes sure that permission propagation is turned off when
> the file permission bits of a file are changed (upon create or chmod).
> 
> The actual permission propagation is implemented in user space.
> 
> Automatic Inheritance works as follows:
> 
>  - When the ACL4_AUTO_INHERIT flag in the acl of a file is not set, the
>    file is not affected by AI.
> 
>  - When the ACL4_AUTO_INHERIT flag in the acl of a directory is set and
>    a file or subdirectory is created in that directory, files created in
>    the directory will have the ACL4_AUTO_INHERIT flag set, and all
>    inherited aces will have the ACE4_INHERITED_ACE flag set.  This
>    allows user space to distinguish between aces which have been
>    inherited and aces which have been explicitly added.
> 
>  - When the ACL4_PROTECTED acl flag in the acl of a file is set, AI will
>    not modify the acl of the file.  This does not affect propagation of
>    permissions from the file to its children (if the file is a
>    directory).

You might want to edit your commit message to use RICHACL_ instead of ACL4_
constants...

> Linux does not have a way of creating files without setting the file
permission
> bits, so all files created inside a directory with ACL4_AUTO_INHERIT set
will
> also have the ACL4_PROTECTED flag set.  This effectively disables
Automatic
> Inheritance.
> 
> Protocols which support creating files without specifying permissions can
> explicitly clear the ACL4_PROTECTED flag after creating a file and reset
the file
> masks to "undo" applying the create mode; see
> richacl_compute_max_masks().
> This is a workaround; a mechanism that would allow a process to indicate
to
> the kernel to ignore the create mode when there are inherited permissions
> would fix this problem.

I'm unclear what will actually be supported for inherited ACLs here. Is this
saying that on a pure-Linux system even with Linux NFS client and Linux NFS
server, we still would not see inheritance since the mode will always be
present on create?

My interest here is in how we will tie the Ganesha user space NFS server
into this feature.

Frank

> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/richacl_base.c       | 10 +++++++++-
>  fs/richacl_inode.c      |  7 ++++++-
>  include/linux/richacl.h | 25 +++++++++++++++++++++++--
>  3 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/richacl_base.c b/fs/richacl_base.c index 54cb482..572f1b8
> 100644
> --- a/fs/richacl_base.c
> +++ b/fs/richacl_base.c
> @@ -352,7 +352,8 @@ richacl_chmod(struct richacl *acl, mode_t mode)
>  	if (acl->a_owner_mask == owner_mask &&
>  	    acl->a_group_mask == group_mask &&
>  	    acl->a_other_mask == other_mask &&
> -	    (acl->a_flags & RICHACL_MASKED))
> +	    (acl->a_flags & RICHACL_MASKED) &&
> +	    (!richacl_is_auto_inherit(acl) || richacl_is_protected(acl)))
>  		return acl;
> 
>  	clone = richacl_clone(acl, GFP_KERNEL); @@ -364,6 +365,8 @@
> richacl_chmod(struct richacl *acl, mode_t mode)
>  	clone->a_owner_mask = owner_mask;
>  	clone->a_group_mask = group_mask;
>  	clone->a_other_mask = other_mask;
> +	if (richacl_is_auto_inherit(clone))
> +		clone->a_flags |= RICHACL_PROTECTED;
> 
>  	return clone;
>  }
> @@ -434,6 +437,11 @@ richacl_inherit(const struct richacl *dir_acl, int
isdir)
>  			ace++;
>  		}
>  	}
> +	if (richacl_is_auto_inherit(dir_acl)) {
> +		acl->a_flags = RICHACL_AUTO_INHERIT;
> +		richacl_for_each_entry(ace, acl)
> +			ace->e_flags |= RICHACE_INHERITED_ACE;
> +	}
> 
>  	return acl;
>  }
> diff --git a/fs/richacl_inode.c b/fs/richacl_inode.c index
1a5b868..dfbb15a
> 100644
> --- a/fs/richacl_inode.c
> +++ b/fs/richacl_inode.c
> @@ -223,9 +223,14 @@ richacl_inherit_inode(const struct richacl *dir_acl,
> struct inode *inode)
> 
>  	acl = richacl_inherit(dir_acl, S_ISDIR(inode->i_mode));
>  	if (acl) {
> +		/*
> +		 * We need to set RICHACL_PROTECTED because we are
> +		 * doing an implicit chmod
> +		 */
> +		if (richacl_is_auto_inherit(acl))
> +			acl->a_flags |= RICHACL_PROTECTED;
> 
>  		richacl_compute_max_masks(acl);
> -
>  		/*
>  		 * Ensure that the acl will not grant any permissions beyond
>  		 * the create mode.
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h index
> 8a92b89..bcc2b64 100644
> --- a/include/linux/richacl.h
> +++ b/include/linux/richacl.h
> @@ -53,9 +53,15 @@ struct richacl {
>  	     _ace--)
> 
>  /* a_flag values */
> +#define RICHACL_AUTO_INHERIT			0x01
> +#define RICHACL_PROTECTED			0x02
> +#define RICHACL_DEFAULTED			0x04
>  #define RICHACL_MASKED				0x80
> 
> -#define RICHACL_VALID_FLAGS (					\
> +#define RICHACL_VALID_FLAGS (			\
> +		RICHACL_AUTO_INHERIT |		\
> +		RICHACL_PROTECTED |		\
> +		RICHACL_DEFAULTED |		\
>  		RICHACL_MASKED)
> 
>  /* e_type values */
> @@ -68,6 +74,7 @@ struct richacl {
>  #define RICHACE_NO_PROPAGATE_INHERIT_ACE	0x0004
>  #define RICHACE_INHERIT_ONLY_ACE		0x0008
>  #define RICHACE_IDENTIFIER_GROUP		0x0040
> +#define RICHACE_INHERITED_ACE			0x0080
>  #define RICHACE_SPECIAL_WHO			0x4000
> 
>  #define RICHACE_VALID_FLAGS (					\
> @@ -76,6 +83,7 @@ struct richacl {
>  	RICHACE_NO_PROPAGATE_INHERIT_ACE |			\
>  	RICHACE_INHERIT_ONLY_ACE |				\
>  	RICHACE_IDENTIFIER_GROUP |				\
> +	RICHACE_INHERITED_ACE |					\
>  	RICHACE_SPECIAL_WHO)
> 
>  /* e_mask bitflags */
> @@ -187,6 +195,18 @@ extern void set_cached_richacl(struct inode *, struct
> richacl *);  extern void forget_cached_richacl(struct inode *);  extern
struct
> richacl *get_richacl(struct inode *);
> 
> +static inline int
> +richacl_is_auto_inherit(const struct richacl *acl) {
> +	return acl->a_flags & RICHACL_AUTO_INHERIT; }
> +
> +static inline int
> +richacl_is_protected(const struct richacl *acl) {
> +	return acl->a_flags & RICHACL_PROTECTED; }
> +
>  /**
>   * richace_is_owner  -  check if @ace is an OWNER@ entry
>   */
> @@ -257,7 +277,8 @@ richace_clear_inheritance_flags(struct richace *ace)
>  	ace->e_flags &= ~(RICHACE_FILE_INHERIT_ACE |
>  			  RICHACE_DIRECTORY_INHERIT_ACE |
>  			  RICHACE_NO_PROPAGATE_INHERIT_ACE |
> -			  RICHACE_INHERIT_ONLY_ACE);
> +			  RICHACE_INHERIT_ONLY_ACE |
> +			  RICHACE_INHERITED_ACE);
>  }
> 
>  /**
> --
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the
> body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Grünbacher May 13, 2015, 8:22 p.m. UTC | #2
2015-05-13 20:01 GMT+02:00 Frank Filz <ffilzlnx@mindspring.com>:
> You might want to edit your commit message to use RICHACL_ instead of ACL4_
> constants...

Indeed, thanks.

>> Linux does not have a way of creating files without setting the file permission
>> bits, so all files created inside a directory with ACL4_AUTO_INHERIT set will
>> also have the ACL4_PROTECTED flag set.  This effectively disables Automatic
>> Inheritance.
>>
>> Protocols which support creating files without specifying permissions can
>> explicitly clear the ACL4_PROTECTED flag after creating a file and reset the file
>> masks to "undo" applying the create mode; see
>> richacl_compute_max_masks().
>> This is a workaround; a mechanism that would allow a process to indicate to
>> the kernel to ignore the create mode when there are inherited permissions
>> would fix this problem.
>
> I'm unclear what will actually be supported for inherited ACLs here. Is this
> saying that on a pure-Linux system even with Linux NFS client and Linux NFS
> server, we still would not see inheritance since the mode will always be
> present on create?

What do you mean by "we still would not see inheritance"? Inheritance
at file create time will still happen; a few extra flags will be set
when Automatic Inheritance is "on" in the parent directory as
indicated by the RICHACL_AUTO_INHERIT flag.

Files are inevitably created with defined permissions (the mode
parameter to system calls like creat and mkdir), which means that the
RICHACL_PROTECTED flag needs to be set, though. When someone changes
the permissions of an entire directory tree, that change will not
propagate to or below files with the protected flag set.

That being said, a daemon like Samba can "fake" full Automatic
Inheritance by creating files and then updating the inherited acls
appropriately. This will inevitably be racy, but unless someone
implements a way to create files without a mode, that's the closest
Samba can get.

Creating files atomically with explicitly defined acls is another
operation which NFSv4 does but the Linux kernel does not support.

> My interest here is in how we will tie the Ganesha user space NFS server
> into this feature.

I don't know, what do you currently do when somebody creates a file
without defining the permissions (mode, acl or dacl)? That's the
relevant case. The kernel nfs daemon currently creates a file with
mode 0 --- which doesn't seem right.

Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeremy Allison May 13, 2015, 8:28 p.m. UTC | #3
On Wed, May 13, 2015 at 10:22:21PM +0200, Andreas Grünbacher wrote:
> 
> That being said, a daemon like Samba can "fake" full Automatic
> Inheritance by creating files and then updating the inherited acls
> appropriately. This will inevitably be racy, but unless someone
> implements a way to create files without a mode, that's the closest
> Samba can get.

On Windows systems the client fake (no quotes :-) full Automatic
Inheritance by creating files and then updating the inherited acls
appropriately.

Server doesn't do that logic :-).
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Frank Filz May 13, 2015, 8:38 p.m. UTC | #4
> -----Original Message-----
> From: Andreas Grünbacher [mailto:andreas.gruenbacher@gmail.com]
> Sent: Wednesday, May 13, 2015 1:22 PM
> To: Frank Filz
> Cc: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org; linux-
> nfs@vger.kernel.org
> Subject: Re: [RFC v3 20/45] richacl: Automatic Inheritance
> 
> 2015-05-13 20:01 GMT+02:00 Frank Filz <ffilzlnx@mindspring.com>:
> > You might want to edit your commit message to use RICHACL_ instead of
> > ACL4_ constants...
> 
> Indeed, thanks.
> 
> >> Linux does not have a way of creating files without setting the file
> >> permission bits, so all files created inside a directory with
> >> ACL4_AUTO_INHERIT set will also have the ACL4_PROTECTED flag set.
> >> This effectively disables Automatic Inheritance.
> >>
> >> Protocols which support creating files without specifying permissions
> >> can explicitly clear the ACL4_PROTECTED flag after creating a file
> >> and reset the file masks to "undo" applying the create mode; see
> >> richacl_compute_max_masks().
> >> This is a workaround; a mechanism that would allow a process to
> >> indicate to the kernel to ignore the create mode when there are
> >> inherited permissions would fix this problem.
> >
> > I'm unclear what will actually be supported for inherited ACLs here.
> > Is this saying that on a pure-Linux system even with Linux NFS client
> > and Linux NFS server, we still would not see inheritance since the
> > mode will always be present on create?
> 
> What do you mean by "we still would not see inheritance"? Inheritance at file
> create time will still happen; a few extra flags will be set when Automatic
> Inheritance is "on" in the parent directory as indicated by the
> RICHACL_AUTO_INHERIT flag.
> 
> Files are inevitably created with defined permissions (the mode parameter
> to system calls like creat and mkdir), which means that the
> RICHACL_PROTECTED flag needs to be set, though. When someone changes
> the permissions of an entire directory tree, that change will not propagate to
> or below files with the protected flag set.
> 
> That being said, a daemon like Samba can "fake" full Automatic Inheritance
> by creating files and then updating the inherited acls appropriately. This will
> inevitably be racy, but unless someone implements a way to create files
> without a mode, that's the closest Samba can get.
> 
> Creating files atomically with explicitly defined acls is another operation
> which NFSv4 does but the Linux kernel does not support.
> 
> > My interest here is in how we will tie the Ganesha user space NFS
> > server into this feature.
> 
> I don't know, what do you currently do when somebody creates a file
> without defining the permissions (mode, acl or dacl)? That's the relevant
> case. The kernel nfs daemon currently creates a file with mode 0 --- which
> doesn't seem right.

Ok, I think I'm beginning to understand...

So inheritance will happen, but there is also a mode set as part of the create that I assume is effectively handled the same as a subsequent chmod() on the file?

Any chance we could add a system call to do a open/create and pass an ACL (and heck, if we go there, why not a system call that allows creating with mtime, atime, owner, etc. also...).

Is there a mode that we could pass that would cause the least amount of damage to the inherited ACL?

Frank

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Grünbacher May 13, 2015, 8:47 p.m. UTC | #5
2015-05-13 22:28 GMT+02:00 Jeremy Allison <jra@samba.org>:
> On Wed, May 13, 2015 at 10:22:21PM +0200, Andreas Grünbacher wrote:
>>
>> That being said, a daemon like Samba can "fake" full Automatic
>> Inheritance by creating files and then updating the inherited acls
>> appropriately. This will inevitably be racy, but unless someone
>> implements a way to create files without a mode, that's the closest
>> Samba can get.
>
> On Windows systems the client fake (no quotes :-) full Automatic
> Inheritance by creating files and then updating the inherited acls
> appropriately.

Hmm, interesting, are you *absolutely* sure about that? Is there
anywhere I can look that up?

Thanks,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeremy Allison May 13, 2015, 8:55 p.m. UTC | #6
On Wed, May 13, 2015 at 10:47:44PM +0200, Andreas Grünbacher wrote:
> 2015-05-13 22:28 GMT+02:00 Jeremy Allison <jra@samba.org>:
> > On Wed, May 13, 2015 at 10:22:21PM +0200, Andreas Grünbacher wrote:
> >>
> >> That being said, a daemon like Samba can "fake" full Automatic
> >> Inheritance by creating files and then updating the inherited acls
> >> appropriately. This will inevitably be racy, but unless someone
> >> implements a way to create files without a mode, that's the closest
> >> Samba can get.
> >
> > On Windows systems the client fake (no quotes :-) full Automatic
> > Inheritance by creating files and then updating the inherited acls
> > appropriately.
> 
> Hmm, interesting, are you *absolutely* sure about that? Is there
> anywhere I can look that up?

Hmm. Just realized we may be talking about different things :-).

In SMB/Samba the clients can create a file with no ACL, and
the directory ACL is auto inherited. *That* we fake in
Samba by creating then updating.

But in Windows there are the concept of "inherited" ACE
entries, which can come from parents of parents of parents
(etc.) objects. When a client modifies one of these on an
upper level directory, the server doesn't do the auto
updating that the vision of the file system might lead
you to expect - that updating is done by a tree walk
by the client.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Grünbacher May 13, 2015, 9:05 p.m. UTC | #7
2015-05-13 22:38 GMT+02:00 Frank Filz <ffilzlnx@mindspring.com>:
> So inheritance will happen, but there is also a mode set as part of the create that I assume
> is effectively handled the same as a subsequent chmod() on the file?

The effect is similar to a subsequent chmod except that the effective
permissions may be fewer then the create mode:

 * In the traditional POSIX case, the effective permissions are
   (create_mode & ~umask).

 * With POSIX ACLs and Richacls, if there are inheritable permissions,
   the effective permissions are the intersection of the create mode and
   the maximum permissions the inherited acl grants. So if the inherited
   acl grants at most rwxr-x---, with a create mode of rw-rw-rw, the
   effective permissions end up being rw-r-----.

> Any chance we could add a system call to do a open/create and pass an ACL
> (and heck, if we go there, why not a system call that allows creating with mtime,
> atime, owner, etc. also...).

Send patches, but expect them to get killed :)

> Is there a mode that we could pass that would cause the least amount
> of damage to the inherited ACL?

Yes, 0777. But the RICHACL_PROTECTED flag will still be set, and that
is the problem in this case.

Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Grünbacher May 13, 2015, 9:15 p.m. UTC | #8
2015-05-13 22:55 GMT+02:00 Jeremy Allison <jra@samba.org>:
> Hmm. Just realized we may be talking about different things :-).
>
> In SMB/Samba the clients can create a file with no ACL, and
> the directory ACL is auto inherited. *That* we fake in
> Samba by creating then updating.

Yes, that's what I was trying to say in the commit message of this
patch. Samba will have to continue doing that.

> But in Windows there are the concept of "inherited" ACE
> entries, which can come from parents of parents of parents
> (etc.) objects.

Fair enough, even if from the child's point of view, all inherited
entries come from the direct parent no matter where the parent
has gotten them from.

> When a client modifies one of these on an
> upper level directory, the server doesn't do the auto
> updating that the vision of the file system might lead
> you to expect - that updating is done by a tree walk
> by the client.

Good, then we're on the same page.

Thanks,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Frank Filz May 13, 2015, 10:56 p.m. UTC | #9
> -----Original Message-----
> From: Andreas Grünbacher [mailto:andreas.gruenbacher@gmail.com]
> Sent: Wednesday, May 13, 2015 2:06 PM
> To: Frank Filz
> Cc: linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org; linux-
> nfs@vger.kernel.org
> Subject: Re: [RFC v3 20/45] richacl: Automatic Inheritance
> 
> 2015-05-13 22:38 GMT+02:00 Frank Filz <ffilzlnx@mindspring.com>:
> > So inheritance will happen, but there is also a mode set as part of
> > the create that I assume is effectively handled the same as a subsequent
> chmod() on the file?
> 
> The effect is similar to a subsequent chmod except that the effective
> permissions may be fewer then the create mode:
> 
>  * In the traditional POSIX case, the effective permissions are
>    (create_mode & ~umask).
> 
>  * With POSIX ACLs and Richacls, if there are inheritable permissions,
>    the effective permissions are the intersection of the create mode and
>    the maximum permissions the inherited acl grants. So if the inherited
>    acl grants at most rwxr-x---, with a create mode of rw-rw-rw, the
>    effective permissions end up being rw-r-----.
> 
> > Any chance we could add a system call to do a open/create and pass an
> > ACL (and heck, if we go there, why not a system call that allows
> > creating with mtime, atime, owner, etc. also...).
> 
> Send patches, but expect them to get killed :)
> 
> > Is there a mode that we could pass that would cause the least amount
> > of damage to the inherited ACL?
> 
> Yes, 0777. But the RICHACL_PROTECTED flag will still be set, and that is the
> problem in this case.

I'm not clear what the RICHACL_PROTECTED flag actually does. It looks like it's only tested in chmod, and then only if the mode matches the mask (at least if I'm understanding the code right).

Frank

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Grünbacher May 13, 2015, 11:52 p.m. UTC | #10
2015-05-14 0:56 GMT+02:00 Frank Filz <ffilzlnx@mindspring.com>:
> I'm not clear what the RICHACL_PROTECTED flag actually does. It looks like it's
> only tested in chmod, and then only if the mode matches the mask (at least if I'm
> understanding the code right).

User space may propagate automatically inherited permissions to files
which are subject to automatic inheritance (RICHACL_AUTO_INHERIT) and
which are not protected from being modified (RICHACL_PROTECTED).

In richacl_chmod(), we simply detect when a chmod has no effect on the
acl: this is when the file masks are already correct, when the acl is
already masked, and when Automatic Inheritance won't affect the acl.

Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields May 14, 2015, 3:09 p.m. UTC | #11
Looks fine to me.

From a quick check of

	git://github.com/andreas-gruenbacher/richacl.git

it looks like the userspace is all done too?

Do we have wireshark patches?

On Fri, Apr 24, 2015 at 01:04:17PM +0200, Andreas Gruenbacher wrote:
> Automatic Inheritance (AI) allows changes to the acl of a directory to
> recursively propagate down to files and directories in the directory.
> 
> To implement this, the kernel keeps track of which permissions have been
> inherited, and makes sure that permission propagation is turned off when the
> file permission bits of a file are changed (upon create or chmod).
> 
> The actual permission propagation is implemented in user space.

Nit, but: I'd find a way to really emphasize this and put it in the very
first sentence of the changelog and any documentation.  People keep
missing it.

--b.

> 
> Automatic Inheritance works as follows:
> 
>  - When the ACL4_AUTO_INHERIT flag in the acl of a file is not set, the
>    file is not affected by AI.
> 
>  - When the ACL4_AUTO_INHERIT flag in the acl of a directory is set and
>    a file or subdirectory is created in that directory, files created in
>    the directory will have the ACL4_AUTO_INHERIT flag set, and all
>    inherited aces will have the ACE4_INHERITED_ACE flag set.  This
>    allows user space to distinguish between aces which have been
>    inherited and aces which have been explicitly added.
> 
>  - When the ACL4_PROTECTED acl flag in the acl of a file is set, AI will
>    not modify the acl of the file.  This does not affect propagation of
>    permissions from the file to its children (if the file is a
>    directory).
> 
> Linux does not have a way of creating files without setting the file permission
> bits, so all files created inside a directory with ACL4_AUTO_INHERIT set will
> also have the ACL4_PROTECTED flag set.  This effectively disables Automatic
> Inheritance.
> 
> Protocols which support creating files without specifying permissions can
> explicitly clear the ACL4_PROTECTED flag after creating a file and reset the
> file masks to "undo" applying the create mode; see richacl_compute_max_masks().
> This is a workaround; a mechanism that would allow a process to indicate to the
> kernel to ignore the create mode when there are inherited permissions would fix
> this problem.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/richacl_base.c       | 10 +++++++++-
>  fs/richacl_inode.c      |  7 ++++++-
>  include/linux/richacl.h | 25 +++++++++++++++++++++++--
>  3 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/richacl_base.c b/fs/richacl_base.c
> index 54cb482..572f1b8 100644
> --- a/fs/richacl_base.c
> +++ b/fs/richacl_base.c
> @@ -352,7 +352,8 @@ richacl_chmod(struct richacl *acl, mode_t mode)
>  	if (acl->a_owner_mask == owner_mask &&
>  	    acl->a_group_mask == group_mask &&
>  	    acl->a_other_mask == other_mask &&
> -	    (acl->a_flags & RICHACL_MASKED))
> +	    (acl->a_flags & RICHACL_MASKED) &&
> +	    (!richacl_is_auto_inherit(acl) || richacl_is_protected(acl)))
>  		return acl;
>  
>  	clone = richacl_clone(acl, GFP_KERNEL);
> @@ -364,6 +365,8 @@ richacl_chmod(struct richacl *acl, mode_t mode)
>  	clone->a_owner_mask = owner_mask;
>  	clone->a_group_mask = group_mask;
>  	clone->a_other_mask = other_mask;
> +	if (richacl_is_auto_inherit(clone))
> +		clone->a_flags |= RICHACL_PROTECTED;
>  
>  	return clone;
>  }
> @@ -434,6 +437,11 @@ richacl_inherit(const struct richacl *dir_acl, int isdir)
>  			ace++;
>  		}
>  	}
> +	if (richacl_is_auto_inherit(dir_acl)) {
> +		acl->a_flags = RICHACL_AUTO_INHERIT;
> +		richacl_for_each_entry(ace, acl)
> +			ace->e_flags |= RICHACE_INHERITED_ACE;
> +	}
>  
>  	return acl;
>  }
> diff --git a/fs/richacl_inode.c b/fs/richacl_inode.c
> index 1a5b868..dfbb15a 100644
> --- a/fs/richacl_inode.c
> +++ b/fs/richacl_inode.c
> @@ -223,9 +223,14 @@ richacl_inherit_inode(const struct richacl *dir_acl, struct inode *inode)
>  
>  	acl = richacl_inherit(dir_acl, S_ISDIR(inode->i_mode));
>  	if (acl) {
> +		/*
> +		 * We need to set RICHACL_PROTECTED because we are
> +		 * doing an implicit chmod
> +		 */
> +		if (richacl_is_auto_inherit(acl))
> +			acl->a_flags |= RICHACL_PROTECTED;
>  
>  		richacl_compute_max_masks(acl);
> -
>  		/*
>  		 * Ensure that the acl will not grant any permissions beyond
>  		 * the create mode.
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> index 8a92b89..bcc2b64 100644
> --- a/include/linux/richacl.h
> +++ b/include/linux/richacl.h
> @@ -53,9 +53,15 @@ struct richacl {
>  	     _ace--)
>  
>  /* a_flag values */
> +#define RICHACL_AUTO_INHERIT			0x01
> +#define RICHACL_PROTECTED			0x02
> +#define RICHACL_DEFAULTED			0x04
>  #define RICHACL_MASKED				0x80
>  
> -#define RICHACL_VALID_FLAGS (					\
> +#define RICHACL_VALID_FLAGS (			\
> +		RICHACL_AUTO_INHERIT |		\
> +		RICHACL_PROTECTED |		\
> +		RICHACL_DEFAULTED |		\
>  		RICHACL_MASKED)
>  
>  /* e_type values */
> @@ -68,6 +74,7 @@ struct richacl {
>  #define RICHACE_NO_PROPAGATE_INHERIT_ACE	0x0004
>  #define RICHACE_INHERIT_ONLY_ACE		0x0008
>  #define RICHACE_IDENTIFIER_GROUP		0x0040
> +#define RICHACE_INHERITED_ACE			0x0080
>  #define RICHACE_SPECIAL_WHO			0x4000
>  
>  #define RICHACE_VALID_FLAGS (					\
> @@ -76,6 +83,7 @@ struct richacl {
>  	RICHACE_NO_PROPAGATE_INHERIT_ACE |			\
>  	RICHACE_INHERIT_ONLY_ACE |				\
>  	RICHACE_IDENTIFIER_GROUP |				\
> +	RICHACE_INHERITED_ACE |					\
>  	RICHACE_SPECIAL_WHO)
>  
>  /* e_mask bitflags */
> @@ -187,6 +195,18 @@ extern void set_cached_richacl(struct inode *, struct richacl *);
>  extern void forget_cached_richacl(struct inode *);
>  extern struct richacl *get_richacl(struct inode *);
>  
> +static inline int
> +richacl_is_auto_inherit(const struct richacl *acl)
> +{
> +	return acl->a_flags & RICHACL_AUTO_INHERIT;
> +}
> +
> +static inline int
> +richacl_is_protected(const struct richacl *acl)
> +{
> +	return acl->a_flags & RICHACL_PROTECTED;
> +}
> +
>  /**
>   * richace_is_owner  -  check if @ace is an OWNER@ entry
>   */
> @@ -257,7 +277,8 @@ richace_clear_inheritance_flags(struct richace *ace)
>  	ace->e_flags &= ~(RICHACE_FILE_INHERIT_ACE |
>  			  RICHACE_DIRECTORY_INHERIT_ACE |
>  			  RICHACE_NO_PROPAGATE_INHERIT_ACE |
> -			  RICHACE_INHERIT_ONLY_ACE);
> +			  RICHACE_INHERIT_ONLY_ACE |
> +			  RICHACE_INHERITED_ACE);
>  }
>  
>  /**
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Grünbacher May 18, 2015, 8:39 p.m. UTC | #12
2015-05-14 17:09 GMT+02:00 J. Bruce Fields <bfields@fieldses.org>:
> Looks fine to me.
>
> From a quick check of
>
>         git://github.com/andreas-gruenbacher/richacl.git
>
> it looks like the userspace is all done too?

Yes, with pretty light testing.

> Do we have wireshark patches?

We do now:

https://code.wireshark.org/review/#/c/8526/1

> On Fri, Apr 24, 2015 at 01:04:17PM +0200, Andreas Gruenbacher wrote:
> Nit, but: I'd find a way to really emphasize this and put it in the very
> first sentence of the changelog and any documentation.  People keep
> missing it.

Okay, I'll try.

Thanks,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields May 18, 2015, 8:43 p.m. UTC | #13
On Mon, May 18, 2015 at 10:39:37PM +0200, Andreas Grünbacher wrote:
> 2015-05-14 17:09 GMT+02:00 J. Bruce Fields <bfields@fieldses.org>:
> > Looks fine to me.
> >
> > From a quick check of
> >
> >         git://github.com/andreas-gruenbacher/richacl.git
> >
> > it looks like the userspace is all done too?
> 
> Yes, with pretty light testing.
> 
> > Do we have wireshark patches?
> 
> We do now:
> 
> https://code.wireshark.org/review/#/c/8526/1

Great!--b.

> 
> > On Fri, Apr 24, 2015 at 01:04:17PM +0200, Andreas Gruenbacher wrote:
> > Nit, but: I'd find a way to really emphasize this and put it in the very
> > first sentence of the changelog and any documentation.  People keep
> > missing it.
> 
> Okay, I'll try.
> 
> Thanks,
> Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/richacl_base.c b/fs/richacl_base.c
index 54cb482..572f1b8 100644
--- a/fs/richacl_base.c
+++ b/fs/richacl_base.c
@@ -352,7 +352,8 @@  richacl_chmod(struct richacl *acl, mode_t mode)
 	if (acl->a_owner_mask == owner_mask &&
 	    acl->a_group_mask == group_mask &&
 	    acl->a_other_mask == other_mask &&
-	    (acl->a_flags & RICHACL_MASKED))
+	    (acl->a_flags & RICHACL_MASKED) &&
+	    (!richacl_is_auto_inherit(acl) || richacl_is_protected(acl)))
 		return acl;
 
 	clone = richacl_clone(acl, GFP_KERNEL);
@@ -364,6 +365,8 @@  richacl_chmod(struct richacl *acl, mode_t mode)
 	clone->a_owner_mask = owner_mask;
 	clone->a_group_mask = group_mask;
 	clone->a_other_mask = other_mask;
+	if (richacl_is_auto_inherit(clone))
+		clone->a_flags |= RICHACL_PROTECTED;
 
 	return clone;
 }
@@ -434,6 +437,11 @@  richacl_inherit(const struct richacl *dir_acl, int isdir)
 			ace++;
 		}
 	}
+	if (richacl_is_auto_inherit(dir_acl)) {
+		acl->a_flags = RICHACL_AUTO_INHERIT;
+		richacl_for_each_entry(ace, acl)
+			ace->e_flags |= RICHACE_INHERITED_ACE;
+	}
 
 	return acl;
 }
diff --git a/fs/richacl_inode.c b/fs/richacl_inode.c
index 1a5b868..dfbb15a 100644
--- a/fs/richacl_inode.c
+++ b/fs/richacl_inode.c
@@ -223,9 +223,14 @@  richacl_inherit_inode(const struct richacl *dir_acl, struct inode *inode)
 
 	acl = richacl_inherit(dir_acl, S_ISDIR(inode->i_mode));
 	if (acl) {
+		/*
+		 * We need to set RICHACL_PROTECTED because we are
+		 * doing an implicit chmod
+		 */
+		if (richacl_is_auto_inherit(acl))
+			acl->a_flags |= RICHACL_PROTECTED;
 
 		richacl_compute_max_masks(acl);
-
 		/*
 		 * Ensure that the acl will not grant any permissions beyond
 		 * the create mode.
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index 8a92b89..bcc2b64 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -53,9 +53,15 @@  struct richacl {
 	     _ace--)
 
 /* a_flag values */
+#define RICHACL_AUTO_INHERIT			0x01
+#define RICHACL_PROTECTED			0x02
+#define RICHACL_DEFAULTED			0x04
 #define RICHACL_MASKED				0x80
 
-#define RICHACL_VALID_FLAGS (					\
+#define RICHACL_VALID_FLAGS (			\
+		RICHACL_AUTO_INHERIT |		\
+		RICHACL_PROTECTED |		\
+		RICHACL_DEFAULTED |		\
 		RICHACL_MASKED)
 
 /* e_type values */
@@ -68,6 +74,7 @@  struct richacl {
 #define RICHACE_NO_PROPAGATE_INHERIT_ACE	0x0004
 #define RICHACE_INHERIT_ONLY_ACE		0x0008
 #define RICHACE_IDENTIFIER_GROUP		0x0040
+#define RICHACE_INHERITED_ACE			0x0080
 #define RICHACE_SPECIAL_WHO			0x4000
 
 #define RICHACE_VALID_FLAGS (					\
@@ -76,6 +83,7 @@  struct richacl {
 	RICHACE_NO_PROPAGATE_INHERIT_ACE |			\
 	RICHACE_INHERIT_ONLY_ACE |				\
 	RICHACE_IDENTIFIER_GROUP |				\
+	RICHACE_INHERITED_ACE |					\
 	RICHACE_SPECIAL_WHO)
 
 /* e_mask bitflags */
@@ -187,6 +195,18 @@  extern void set_cached_richacl(struct inode *, struct richacl *);
 extern void forget_cached_richacl(struct inode *);
 extern struct richacl *get_richacl(struct inode *);
 
+static inline int
+richacl_is_auto_inherit(const struct richacl *acl)
+{
+	return acl->a_flags & RICHACL_AUTO_INHERIT;
+}
+
+static inline int
+richacl_is_protected(const struct richacl *acl)
+{
+	return acl->a_flags & RICHACL_PROTECTED;
+}
+
 /**
  * richace_is_owner  -  check if @ace is an OWNER@ entry
  */
@@ -257,7 +277,8 @@  richace_clear_inheritance_flags(struct richace *ace)
 	ace->e_flags &= ~(RICHACE_FILE_INHERIT_ACE |
 			  RICHACE_DIRECTORY_INHERIT_ACE |
 			  RICHACE_NO_PROPAGATE_INHERIT_ACE |
-			  RICHACE_INHERIT_ONLY_ACE);
+			  RICHACE_INHERIT_ONLY_ACE |
+			  RICHACE_INHERITED_ACE);
 }
 
 /**