diff mbox

[RFC,v7,15/41] richacl: Automatic Inheritance

Message ID 1441448856-13478-16-git-send-email-agruenba@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andreas Gruenbacher Sept. 5, 2015, 10:27 a.m. UTC
Automatic Inheritance (AI) allows changes to the acl of a directory to
propagate down to children.

This is mostly implemented in user space: when a process changes the
permissions of a directory and Automatic Inheritance is enabled for that
directory, the process must propagate those changes to all children,
recursively.

The kernel enables this by keeping track of which permissions have been
inherited at file create time.  In addition, it makes sure that
permission propagation is turned off when the permissions of a file are
set explicitly (for example, upon create or chmod).

Automatic Inheritance works as follows:

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

 - When the RICHACL_AUTO_INHERIT flag in the acl of a directory is set
   and a file or subdirectory is created in that directory, the
   inherited acl will have the RICHACL_AUTO_INHERIT flag set, and all
   inherited aces will have the RICHACE_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 RICHACL_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
RICHACL_AUTO_INHERIT set will have the RICHACL_PROTECTED flag set.  This
effectively disables Automatic Inheritance.

Protocols which support creating files without specifying permissions
can explicitly clear the RICHACL_PROTECTED flag after creating a file
and reset the file masks to "undo" applying the create mode; see
richacl_compute_max_masks().  They should set the RICHACL_DEFAULTED
flag.  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 | 23 ++++++++++++++++++++++-
 3 files changed, 38 insertions(+), 2 deletions(-)

Comments

J. Bruce Fields Sept. 18, 2015, 6:40 p.m. UTC | #1
On Sat, Sep 05, 2015 at 12:27:10PM +0200, Andreas Gruenbacher wrote:
> Automatic Inheritance (AI) allows changes to the acl of a directory to
> propagate down to children.
> 
> This is mostly implemented in user space: when a process changes the
> permissions of a directory and Automatic Inheritance is enabled for that
> directory, the process must propagate those changes to all children,
> recursively.
> 
> The kernel enables this by keeping track of which permissions have been
> inherited at file create time.  In addition, it makes sure that
> permission propagation is turned off when the permissions of a file are
> set explicitly (for example, upon create or chmod).
> 
> Automatic Inheritance works as follows:
> 
>  - When the RICHACL_AUTO_INHERIT flag in the acl of a file is not set,
>    the file is not affected by AI.
> 
>  - When the RICHACL_AUTO_INHERIT flag in the acl of a directory is set
>    and a file or subdirectory is created in that directory, the
>    inherited acl will have the RICHACL_AUTO_INHERIT flag set, and all
>    inherited aces will have the RICHACE_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 RICHACL_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).

In the above "file" sometimes means "any object" and somethings "a
non-directory".  I can sort it out, but more consistent terminology
would help.

> Linux does not have a way of creating files without setting the file
> permission bits, so all files created inside a directory with
> RICHACL_AUTO_INHERIT set will have the RICHACL_PROTECTED flag set.  This
> effectively disables Automatic Inheritance.
> 
> Protocols which support creating files without specifying permissions
> can explicitly clear the RICHACL_PROTECTED flag after creating a file
> and reset the file masks to "undo" applying the create mode; see
> richacl_compute_max_masks().  They should set the RICHACL_DEFAULTED
> flag.  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.

Also, as you know: current nfsd has no way to create files without
setting permissions.  And if we were to implement that it's unclear how
many clients would actually use it (Windows clients are rare).  And of
course Samba doesn't have the interfaces it would need.

I think we should just drop this for now.  The rest of the richacl stuff
is still useful without it.

--b.

> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/richacl_base.c       | 10 +++++++++-
>  fs/richacl_inode.c      |  7 +++++++
>  include/linux/richacl.h | 23 ++++++++++++++++++++++-
>  3 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/richacl_base.c b/fs/richacl_base.c
> index fda407d..a85b7a3 100644
> --- a/fs/richacl_base.c
> +++ b/fs/richacl_base.c
> @@ -363,7 +363,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_WRITE_THROUGH | RICHACL_MASKED)))
> +	    (acl->a_flags & (RICHACL_WRITE_THROUGH | RICHACL_MASKED)) &&
> +	    (!richacl_is_auto_inherit(acl) || richacl_is_protected(acl)))
>  		return acl;
>  
>  	clone = richacl_clone(acl, GFP_KERNEL);
> @@ -375,6 +376,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;
>  }
> @@ -549,6 +552,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 f3f1f84..df175c1 100644
> --- a/fs/richacl_inode.c
> +++ b/fs/richacl_inode.c
> @@ -247,6 +247,13 @@ richacl_inherit_inode(const struct richacl *dir_acl, struct inode *inode)
>  			richacl_put(acl);
>  			acl = NULL;
>  		} else {
> +			/*
> +			 * 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
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> index 9bf95c2..832b06c 100644
> --- a/include/linux/richacl.h
> +++ b/include/linux/richacl.h
> @@ -53,10 +53,16 @@ struct richacl {
>  	     _ace--)
>  
>  /* a_flags values */
> +#define RICHACL_AUTO_INHERIT			0x01
> +#define RICHACL_PROTECTED			0x02
> +#define RICHACL_DEFAULTED			0x04
>  #define RICHACL_WRITE_THROUGH			0x40
>  #define RICHACL_MASKED				0x80
>  
>  #define RICHACL_VALID_FLAGS (					\
> +		RICHACL_AUTO_INHERIT |				\
> +		RICHACL_PROTECTED |				\
> +		RICHACL_DEFAULTED |				\
>  		RICHACL_WRITE_THROUGH |				\
>  		RICHACL_MASKED)
>  
> @@ -70,6 +76,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 (					\
> @@ -78,13 +85,15 @@ struct richacl {
>  	RICHACE_NO_PROPAGATE_INHERIT_ACE |			\
>  	RICHACE_INHERIT_ONLY_ACE |				\
>  	RICHACE_IDENTIFIER_GROUP |				\
> +	RICHACE_INHERITED_ACE |					\
>  	RICHACE_SPECIAL_WHO)
>  
>  #define RICHACE_INHERITANCE_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 )
>  
>  /* e_mask bitflags */
>  #define RICHACE_READ_DATA			0x00000001
> @@ -195,6 +204,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
>   */
> -- 
> 2.4.3
> 
> --
> 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-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Gruenbacher Sept. 21, 2015, 9:19 p.m. UTC | #2
2015-09-18 20:40 GMT+02:00 J. Bruce Fields <bfields@fieldses.org>:
> On Sat, Sep 05, 2015 at 12:27:10PM +0200, Andreas Gruenbacher wrote:
>> Automatic Inheritance (AI) allows changes to the acl of a directory to
> In the above "file" sometimes means "any object" and somethings "a
> non-directory".  I can sort it out, but more consistent terminology
> would help.

Okay, I'll fix it.

>> Linux does not have a way of creating files without setting the file
>> permission bits, so all files created inside a directory with
>> RICHACL_AUTO_INHERIT set will have the RICHACL_PROTECTED flag set.  This
>> effectively disables Automatic Inheritance.
>>
>> Protocols which support creating files without specifying permissions
>> can explicitly clear the RICHACL_PROTECTED flag after creating a file
>> and reset the file masks to "undo" applying the create mode; see
>> richacl_compute_max_masks().  They should set the RICHACL_DEFAULTED
>> flag.  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.
>
> Also, as you know: current nfsd has no way to create files without
> setting permissions.  And if we were to implement that it's unclear how
> many clients would actually use it (Windows clients are rare).  And of
> course Samba doesn't have the interfaces it would need.
>
> I think we should just drop this for now.  The rest of the richacl stuff
> is still useful without it.

Samba will hack around it and adjust the ACL after the create; that's
still better than not having Automatic Inheritance. Windows uses AI
all the time so AI is more important for Samba than for NFSv4.

Thanks,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" 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 Sept. 22, 2015, 1:51 a.m. UTC | #3
On Mon, Sep 21, 2015 at 11:19:59PM +0200, Andreas Gruenbacher wrote:
> 2015-09-18 20:40 GMT+02:00 J. Bruce Fields <bfields@fieldses.org>:
> > On Sat, Sep 05, 2015 at 12:27:10PM +0200, Andreas Gruenbacher wrote:
> >> Automatic Inheritance (AI) allows changes to the acl of a directory to
> > In the above "file" sometimes means "any object" and somethings "a
> > non-directory".  I can sort it out, but more consistent terminology
> > would help.
> 
> Okay, I'll fix it.
> 
> >> Linux does not have a way of creating files without setting the file
> >> permission bits, so all files created inside a directory with
> >> RICHACL_AUTO_INHERIT set will have the RICHACL_PROTECTED flag set.  This
> >> effectively disables Automatic Inheritance.
> >>
> >> Protocols which support creating files without specifying permissions
> >> can explicitly clear the RICHACL_PROTECTED flag after creating a file
> >> and reset the file masks to "undo" applying the create mode; see
> >> richacl_compute_max_masks().  They should set the RICHACL_DEFAULTED
> >> flag.  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.
> >
> > Also, as you know: current nfsd has no way to create files without
> > setting permissions.  And if we were to implement that it's unclear how
> > many clients would actually use it (Windows clients are rare).  And of
> > course Samba doesn't have the interfaces it would need.
> >
> > I think we should just drop this for now.  The rest of the richacl stuff
> > is still useful without it.
> 
> Samba will hack around it and adjust the ACL after the create; that's
> still better than not having Automatic Inheritance. Windows uses AI
> all the time so AI is more important for Samba than for NFSv4.

Oh, OK, that makes sense.  Even just giving them a place to store the
bits would be better than nothing.  So, ignore my objection there....

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" 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 Sept. 23, 2015, 1:55 p.m. UTC | #4
On Mon, Sep 21, 2015 at 09:51:46PM -0400, J. Bruce Fields wrote:
> On Mon, Sep 21, 2015 at 11:19:59PM +0200, Andreas Gruenbacher wrote:
> > 2015-09-18 20:40 GMT+02:00 J. Bruce Fields <bfields@fieldses.org>:
> > > On Sat, Sep 05, 2015 at 12:27:10PM +0200, Andreas Gruenbacher wrote:
> > >> Automatic Inheritance (AI) allows changes to the acl of a directory to
> > > In the above "file" sometimes means "any object" and somethings "a
> > > non-directory".  I can sort it out, but more consistent terminology
> > > would help.
> > 
> > Okay, I'll fix it.
> > 
> > >> Linux does not have a way of creating files without setting the file
> > >> permission bits, so all files created inside a directory with
> > >> RICHACL_AUTO_INHERIT set will have the RICHACL_PROTECTED flag set.  This
> > >> effectively disables Automatic Inheritance.
> > >>
> > >> Protocols which support creating files without specifying permissions
> > >> can explicitly clear the RICHACL_PROTECTED flag after creating a file
> > >> and reset the file masks to "undo" applying the create mode; see
> > >> richacl_compute_max_masks().  They should set the RICHACL_DEFAULTED
> > >> flag.  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.
> > >
> > > Also, as you know: current nfsd has no way to create files without
> > > setting permissions.  And if we were to implement that it's unclear how
> > > many clients would actually use it (Windows clients are rare).  And of
> > > course Samba doesn't have the interfaces it would need.
> > >
> > > I think we should just drop this for now.  The rest of the richacl stuff
> > > is still useful without it.
> > 
> > Samba will hack around it and adjust the ACL after the create; that's
> > still better than not having Automatic Inheritance. Windows uses AI
> > all the time so AI is more important for Samba than for NFSv4.
> 
> Oh, OK, that makes sense.  Even just giving them a place to store the
> bits would be better than nothing.  So, ignore my objection there....

Oh, and

	Reviewed-by: J. Bruce Fields <bfields@redhat.com>

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" 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 fda407d..a85b7a3 100644
--- a/fs/richacl_base.c
+++ b/fs/richacl_base.c
@@ -363,7 +363,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_WRITE_THROUGH | RICHACL_MASKED)))
+	    (acl->a_flags & (RICHACL_WRITE_THROUGH | RICHACL_MASKED)) &&
+	    (!richacl_is_auto_inherit(acl) || richacl_is_protected(acl)))
 		return acl;
 
 	clone = richacl_clone(acl, GFP_KERNEL);
@@ -375,6 +376,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;
 }
@@ -549,6 +552,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 f3f1f84..df175c1 100644
--- a/fs/richacl_inode.c
+++ b/fs/richacl_inode.c
@@ -247,6 +247,13 @@  richacl_inherit_inode(const struct richacl *dir_acl, struct inode *inode)
 			richacl_put(acl);
 			acl = NULL;
 		} else {
+			/*
+			 * 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
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index 9bf95c2..832b06c 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -53,10 +53,16 @@  struct richacl {
 	     _ace--)
 
 /* a_flags values */
+#define RICHACL_AUTO_INHERIT			0x01
+#define RICHACL_PROTECTED			0x02
+#define RICHACL_DEFAULTED			0x04
 #define RICHACL_WRITE_THROUGH			0x40
 #define RICHACL_MASKED				0x80
 
 #define RICHACL_VALID_FLAGS (					\
+		RICHACL_AUTO_INHERIT |				\
+		RICHACL_PROTECTED |				\
+		RICHACL_DEFAULTED |				\
 		RICHACL_WRITE_THROUGH |				\
 		RICHACL_MASKED)
 
@@ -70,6 +76,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 (					\
@@ -78,13 +85,15 @@  struct richacl {
 	RICHACE_NO_PROPAGATE_INHERIT_ACE |			\
 	RICHACE_INHERIT_ONLY_ACE |				\
 	RICHACE_IDENTIFIER_GROUP |				\
+	RICHACE_INHERITED_ACE |					\
 	RICHACE_SPECIAL_WHO)
 
 #define RICHACE_INHERITANCE_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 )
 
 /* e_mask bitflags */
 #define RICHACE_READ_DATA			0x00000001
@@ -195,6 +204,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
  */