New ACL format for better NFSv4 acl interoperability
in [Kernel]
|
Prev: KVM: MMU: combine guest pte read between walk and pte prefetch
Next: ext4: Add temporary richacl mount option for ext4.
From: Aneesh Kumar K. V on 3 Jul 2010 07:00 On Sat, 3 Jul 2010 00:13:31 +0530, "Aneesh Kumar K.V" <aneesh.kumar(a)linux.vnet.ibm.com> wrote: > Hi, > > The following set of patches implements a new acl model for linux. Rich ACLs > are an implementation of NFSv4 ACLs, extended by file masks to fit into the > standard POSIX file permission model. They are designed to work seamlessly > locally as well as across the NFSv4 and CIFS/SMB2 network file system protocols. > > Since posting the patches the previous time [0], Andreas Gruenbacher and I have > worked on cleaning up the code, splitting things up into smaller pieces, and > adding more documentation. > > [0] http://article.gmane.org/gmane.comp.file-systems.ext4/17414 > > The patch set consists of three parts. > > The first set of patches, posted as a follow up, contains the Rich ACL model > and Ext4 implementation. The second set [1] contains mapping of Rich ACL to > NFSv4 ACL (how to apply file mask to access mask) and implementation of > Richacl ACL for NFS server and client. The third set [2] contains POSIX ACL > to Rich ACL mapping and its ext4 usage. > > [1] git://git.kernel.org/pub/scm/linux/kernel/git/agruen/linux-2.6-richacl.git richacl-upstream > [2] git://git.kernel.org/pub/scm/linux/kernel/git/agruen/linux-2.6-richacl.git richacl-fullset > > A user-space utility for displaying and changing richacls is available at [3] > (a number of examples can be found at http://acl.bestbits.at/richacl/examples.html). > > [3] git://git.kernel.org/pub/scm/linux/kernel/git/agruen/richacl.git master > > To test richacl on ext4 use -o richacl mount option. This mount option may later be > dropped in favour of a feature flag. > > More details regarding richacl can be found at > http://acl.bestbits.at/richacl/ > > Changes from V1: > 1) Split the patches into smaller patches > 2) Added extensive documentation to the patches. > We also need the below patch set to make sure the permission check function does the right thing. This patch is a part of second patch set [1] pointed above. We add it as a part of the later series because it is better explained with isolate owner and group patchset. commit 01a3a0c12dc4b87cdee11cd101b3ceefaa331e41 Author: Andreas Gruenbacher <agruen(a)suse.de> Date: Mon Jun 28 21:32:24 2010 +0530 richacl: Restrict access check algorithm We want to avoid applying the file masks to an acl when changing the file permission bits or performing an access check. On the other hand, when we *do* apply the file masks to the acl, we want the resulting acl to produce the same access check results with the standard nfs4 access check algorithm as the richacl access check algorithm with the original acl. This is already the case, except in the following scenario: With file masks equivalent to file mode 0600, the following acl would grant the owner rw access if the owner is in the owning group: group@:rw::allow There is no way to express this in an nfs4 acl; the result is always a more restrictive acl. There are two approaches to deal with this difference: either accept that it exists and that applying the file masks is imperfect, or change the richacl access check algorithm so that such accesses are denied. This patch denies such accesses and makes sure that the richacl access check algorithm grants the same accesses as the nfsv4 acl with the file masks applied. Signed-off-by: Andreas Gruenbacher <agruen(a)suse.de> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar(a)linux.vnet.ibm.com> diff --git a/fs/richacl_base.c b/fs/richacl_base.c index b368b51..79fb00f 100644 --- a/fs/richacl_base.c +++ b/fs/richacl_base.c @@ -424,6 +424,16 @@ richacl_permission(struct inode *inode, const struct richacl *acl, } else goto is_everyone; + /* + * Apply the group file mask to entries other than OWNER@ and + * EVERYONE@. This is not required for correct access checking + * but ensures that we grant the same permissions as the acl + * computed by richacl_apply_masks() would grant. See + * richacl_apply_masks() for a more detailed explanation. + */ + if (richace_is_allow(ace)) + ace_mask &= acl->a_group_mask; + is_owner: /* The process is in the owner or group file class. */ in_owner_or_group_class = 1; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Andreas Gruenbacher on 19 Jul 2010 15:30 Aneesh, here is a patch on top of the richacl-upstream queue. So far, we were assuming that acls are always masked, and the file masks could be set so that they would not mask any permissions. This is fine for permission checking, but richacl_apply_masks() didn't recognize when the file masks were set to mask nothing, and was still transforming such acls unnecessarily. This could lead to correct but surprising results. Instead of using a flag here we could check if the file masks are set to "ineffective" values, but that would require the same computation as richacl_compute_max_masks(), which can be a slow on large acls. It is a lot easier to instead remember if the file masks are "effective". (We still need to compute the file masks in nfsd where no file masks are supplied so that the file permission bits will be set to a reasonable value. user space will always pass valid file masks in, so we are covered in that side.) Thanks, Andreas -------------------------------------------------------------------------------- [PATCH] richacl: Allow acls to be masked or unmasked Use a flag to indicate when the file masks can mask something (e.g., after a chmod). richacl_apply_masks() can skip transforming acls without this flag, which speeds things up and avoids modifying those acls unnecessarily. Signed-off-by: Andreas Gruenbacher <agruen(a)suse.de> --- fs/richacl_base.c | 46 +++++++++++++++++++++++++++++++--------------- fs/richacl_compat.c | 31 ++++++++++++++++++------------- fs/richacl_inode.c | 1 + include/linux/richacl.h | 1 + 4 files changed, 51 insertions(+), 28 deletions(-) diff --git a/fs/richacl_base.c b/fs/richacl_base.c index 79fb00f..baaecf2 100644 --- a/fs/richacl_base.c +++ b/fs/richacl_base.c @@ -329,6 +329,8 @@ restart: } } } + + acl->a_flags &= ~ACL4_MASKED; } EXPORT_SYMBOL_GPL(richacl_compute_max_masks); @@ -354,7 +356,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 && - (!richacl_is_auto_inherit(acl) || richacl_is_protected(acl))) + (!richacl_is_auto_inherit(acl) || richacl_is_protected(acl)) && + (acl->a_flags & ACL4_MASKED)) return acl; clone = richacl_clone(acl); @@ -362,6 +365,7 @@ richacl_chmod(struct richacl *acl, mode_t mode) if (!clone) return ERR_PTR(-ENOMEM); + clone->a_flags |= ACL4_MASKED; clone->a_owner_mask = owner_mask; clone->a_group_mask = group_mask; clone->a_other_mask = other_mask; @@ -385,11 +389,18 @@ richacl_permission(struct inode *inode, const struct richacl *acl, unsigned int mask) { const struct richace *ace; - unsigned int file_mask, requested = mask, denied = 0; + unsigned int requested = mask, denied = 0; int in_owning_group = in_group_p(inode->i_gid); int in_owner_or_group_class = in_owning_group; /* + * We don't need to know which class the process is in when the acl is + * not masked. + */ + if (!(acl->a_flags & ACL4_MASKED)) + in_owner_or_group_class = 1; + + /* * A process is * - in the owner file class if it owns the file, * - in the group file class if it is in the file's owning group or @@ -431,7 +442,7 @@ richacl_permission(struct inode *inode, const struct richacl *acl, * computed by richacl_apply_masks() would grant. See * richacl_apply_masks() for a more detailed explanation. */ - if (richace_is_allow(ace)) + if ((acl->a_flags & ACL4_MASKED) && richace_is_allow(ace)) ace_mask &= acl->a_group_mask; is_owner: @@ -453,17 +464,22 @@ is_everyone: } denied |= mask; - /* - * The file class a process is in determines which file mask applies. - * Check if that file mask also grants the requested access. - */ - if (current_fsuid() == inode->i_uid) - file_mask = acl->a_owner_mask; - else if (in_owner_or_group_class) - file_mask = acl->a_group_mask; - else - file_mask = acl->a_other_mask; - denied |= requested & ~file_mask; + if (acl->a_flags & ACL4_MASKED) { + unsigned int file_mask; + + /* + * The file class a process is in determines which file mask + * applies. Check if that file mask also grants the requested + * access. + */ + if (current_fsuid() == inode->i_uid) + file_mask = acl->a_owner_mask; + else if (in_owner_or_group_class) + file_mask = acl->a_group_mask; + else + file_mask = acl->a_other_mask; + denied |= requested & ~file_mask; + } return denied ? -EACCES : 0; } @@ -563,7 +579,7 @@ richacl_equiv_mode(const struct richacl *acl, mode_t *mode_p) mode_t mode; if (acl->a_count != 1 || - acl->a_flags || + acl->a_flags != ACL4_MASKED || !richace_is_everyone(ace) || !richace_is_allow(ace) || ace->e_flags & ~ACE4_SPECIAL_WHO) diff --git a/fs/richacl_compat.c b/fs/richacl_compat.c index 9adbd62..4aa0b7d 100644 --- a/fs/richacl_compat.c +++ b/fs/richacl_compat.c @@ -699,20 +699,25 @@ __richacl_apply_masks(struct richacl_alloc *x) int richacl_apply_masks(struct richacl **acl) { - struct richacl_alloc x = { - .acl = *acl, - .count = (*acl)->a_count, - }; int retval = 0; - if (richacl_move_everyone_aces_down(&x) || - richacl_propagate_everyone(&x) || - __richacl_apply_masks(&x) || - richacl_isolate_owner_class(&x) || - richacl_isolate_group_class(&x)) - retval = -ENOMEM; + if ((*acl)->a_flags & ACL4_MASKED) { + struct richacl_alloc x = { + .acl = *acl, + .count = (*acl)->a_count, + }; + + if (richacl_move_everyone_aces_down(&x) || + richacl_propagate_everyone(&x) || + __richacl_apply_masks(&x) || + richacl_isolate_owner_class(&x) || + richacl_isolate_group_class(&x)) + retval = -ENOMEM; + + x.acl->a_flags &= ~ACL4_MASKED; + *acl = x.acl; + } - *acl = x.acl; return retval; } EXPORT_SYMBOL_GPL(richacl_apply_masks); @@ -730,12 +735,12 @@ richacl_from_mode(mode_t mode) acl = richacl_alloc(1); if (!acl) return NULL; - ace = acl->a_entries; - + acl->a_flags = ACL4_MASKED; acl->a_owner_mask = richacl_mode_to_mask(mode >> 6); acl->a_group_mask = richacl_mode_to_mask(mode >> 3); acl->a_other_mask = richacl_mode_to_mask(mode); + ace = acl->a_entries; ace->e_type = ACE4_ACCESS_ALLOWED_ACE_TYPE; ace->e_flags = ACE4_SPECIAL_WHO; ace->e_mask = ACE4_POSIX_MODE_ALL; diff --git a/fs/richacl_inode.c b/fs/richacl_inode.c index 1e9c901..6be821a 100644 --- a/fs/richacl_inode.c +++ b/fs/richacl_inode.c @@ -224,6 +224,7 @@ richacl_inherit_inode(const struct richacl *dir_acl, struct inode *inode) * Ensure that the acl will not grant any permissions beyond * the create mode. */ + acl->a_flags |= ACL4_MASKED; acl->a_owner_mask &= richacl_mode_to_mask(inode->i_mode >> 6); acl->a_group_mask &= richacl_mode_to_mask(inode->i_mode >> 3); acl->a_other_mask &= richacl_mode_to_mask(inode->i_mode); diff --git a/include/linux/richacl.h b/include/linux/richacl.h index 74b4c80..8ff317c 100644 --- a/include/linux/richacl.h +++ b/include/linux/richacl.h @@ -51,6 +51,7 @@ struct richacl { #define ACL4_AUTO_INHERIT 0x01 #define ACL4_PROTECTED 0x02 /*#define ACL4_DEFAULTED 0x04*/ +#define ACL4_MASKED 0x08 #define ACL4_VALID_FLAGS ( \ ACL4_AUTO_INHERIT | \ -- 1.7.1.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Aneesh Kumar K. V on 20 Jul 2010 05:40 On Mon, 19 Jul 2010 21:19:50 +0200, Andreas Gruenbacher <agruen(a)suse.de> wrote: > Aneesh, > > here is a patch on top of the richacl-upstream queue. > > So far, we were assuming that acls are always masked, and the file masks > could be set so that they would not mask any permissions. This is fine > for permission checking, but richacl_apply_masks() didn't recognize when > the file masks were set to mask nothing, and was still transforming such > acls unnecessarily. This could lead to correct but surprising results. > > Instead of using a flag here we could check if the file masks are set to > "ineffective" values, but that would require the same computation as > richacl_compute_max_masks(), which can be a slow on large acls. It is a > lot easier to instead remember if the file masks are "effective". > > (We still need to compute the file masks in nfsd where no file masks are > supplied so that the file permission bits will be set to a reasonable > value. user space will always pass valid file masks in, so we are covered > in that side.) We need to update ACL4_VALID_FLAGS to now consider ACL4_MASKED as a valid flag. This is also needed for userspace. On a related note, should we move ACL4_MASKED and ACL4_POSIX_MAPPED to be the higher bits ? That would make sure we will be able to accomodate new flag value NFSv4 define. Something like diff --git a/include/linux/richacl.h b/include/linux/richacl.h index 929cc32..ff3c12b 100644 --- a/include/linux/richacl.h +++ b/include/linux/richacl.h @@ -50,16 +50,17 @@ struct richacl { /* a_flags values */ #define ACL4_AUTO_INHERIT 0x01 #define ACL4_PROTECTED 0x02 -/*#define ACL4_DEFAULTED 0x04*/ -#define ACL4_MASKED 0x08 -#define ACL4_POSIX_MAPPED 0x10 +#define ACL4_DEFAULTED 0x04 +/* flag value defined by Richacl */ +#define ACL4_MASKED 0x40 +#define ACL4_POSIX_MAPPED 0x80 #define ACL4_VALID_FLAGS ( \ ACL4_AUTO_INHERIT | \ ACL4_PROTECTED | \ + ACL4_MASKED | \ ACL4_POSIX_MAPPED) - /* e_type values */ #define ACE4_ACCESS_ALLOWED_ACE_TYPE 0x0000 #define ACE4_ACCESS_DENIED_ACE_TYPE 0x0001 -aneesh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Andreas Gruenbacher on 20 Jul 2010 06:20 On Tuesday 20 July 2010 11:31:07 Aneesh Kumar K. V wrote: > We need to update ACL4_VALID_FLAGS to now consider ACL4_MASKED as a > valid flag. This is also needed for userspace. Good point, I missed that. > On a related note, should we move ACL4_MASKED and ACL4_POSIX_MAPPED to > be the higher bits ? That would make sure we will be able to accomodate > new flag value NFSv4 define. That makes sense, except that ACL4_POSIX_MAPPED hasn't entered the scene in the patches posted here, and I'm still not convinced that we'll actually need it. Thanks, Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Aneesh Kumar K. V on 20 Jul 2010 11:30
On Tue, 20 Jul 2010 12:11:53 +0200, Andreas Gruenbacher <agruen(a)suse.de> wrote: > On Tuesday 20 July 2010 11:31:07 Aneesh Kumar K. V wrote: > > We need to update ACL4_VALID_FLAGS to now consider ACL4_MASKED as a > > valid flag. This is also needed for userspace. > > Good point, I missed that. I updated the patch and will push the change to korg after running some test. > > > On a related note, should we move ACL4_MASKED and ACL4_POSIX_MAPPED to > > be the higher bits ? That would make sure we will be able to accomodate > > new flag value NFSv4 define. > > That makes sense, except that ACL4_POSIX_MAPPED hasn't entered the scene in > the patches posted here, and I'm still not convinced that we'll actually need > it. > The userspace change did result in a different output for the below ex: richacl --set 'flags:a 101:w::deny 101:rw::allow 101:w:a:deny 101:rw:a:allow' f this now gives /mnt/d# richacl --get --numeric f f: flags:a 101:-w-----------::deny 101:rw-----------::allow 101:-w-----------:a:deny 101:rw-----------:a:allow that 'w' in rw::allow is redundant, because we have a deny entry before. -aneesh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |