2013-05-09 16:37:33

by Viacheslav Dubeyko

[permalink] [raw]
Subject: [PATCH v4 0/5] nfsd + hfsplus: introduce generalized version of NFSv4 ACLs <-> POSIX ACLs mapping algorithms

Hi,

This patchset implements ACLs support in hfsplus driver and generalizes NFSv4 ACLs <-> POSIX ACLs mapping algorithms.

v3->v4
* Introduce interface of NFSv4 ACLs <-> POSIX ACLs mapping (J. Bruce Fields request).
* Introduce generalization of NFSv4 ACLs <-> POSIX ACLs mapping algorithms (J. Bruce Fields request).
* Rework ACLs support in HFS+ driver with the purpose of using generalized mapping algorithms.
* Change dprint() on hfs_dbg() calls.
* Enhance debug output in fs/hfsplus/acl.c.

v2->v3
* Fix errors in dprint_hexdump() macro.
* Correct format on %zd for size_t in dprint() calls.

v1->v2
* Add several dprint() messages.
* Change hardcoded function names on __func__ macro.
* Fix coding style errors.

The include/linux/nfs4acl.h header file is created with the purpose of sharing declarations and structures that were moved from fs/nfsd/nfs4acl.c and fs/nfsd/acl.h. Moreover, this header file introduces declaration of operations (nfsv4_ace_operations, nfsv4_ace_flags_operations, nfsv4_acl_id_operations, nfsv4_acl_mapping_operations) that can be specialized in concrete file system driver in the case of necessity. Otherwise, it is possible to use generalized mapping code without operations specialization. And, finally, it is declared nfsv4_acl_info structure that includes operations, pointer on mapping NFSv4 ACL and pointer on special mapping environment of concrete file system.

The essence of mapping algorithms (located in fs/nfsd/nfs4acl.c, previously) were generalized and moved in fs/nfs4acl.c with the purpose of sharing between file system drivers. A concrete file system driver can use mapping code by means of map_posix_acl_to_nfsv4_one(), map_nfsv4_acl_to_posix(), nfs4_acl_posix_to_nfsv4(), nfs4_acl_nfsv4_to_posix() methods. Also, it is possible to specialize internal mapping operations in the case of very special way of operations under raw structures for concrete file system driver case.

Mac OS X supports NFSv4 ACLs. It keeps its in the form of specially named xattr (com.apple.system.Security). HFS+ driver uses generalized implementation of NFSv4 ACLs <-> POSIX ACLs mapping algorithms. But it should be specialized internal mapping operations for the proper conversion of raw NFSv4 ACLs representation on HFS+ volume into POSIX ACLs. Thereby, it were implemented in hfsplus driver all necessary specialized internal mapping operations.

With the best regards,
Vyacheslav Dubeyko.
---
Vyacheslav Dubeyko (3):
NFSv4: introduce interface of NFSv4 ACLs <-> POSIX ACLs mapping
nfsd: introduce generalization of NFSv4 ACLs <-> POSIX ACLs mapping algorithms
hfsplus: introduce implementation of the ACLs support

fs/Makefile | 2 +-
fs/hfsplus/Makefile | 2 +-
fs/hfsplus/acl.c | 1903 +++++++++++++++++++++++++++++++++++++++++++
fs/hfsplus/acl.h | 96 +++
fs/hfsplus/dir.c | 2 +
fs/hfsplus/hfsplus_fs.h | 8 +
fs/hfsplus/hfsplus_raw.h | 26 +
fs/hfsplus/inode.c | 9 +
fs/hfsplus/xattr.c | 20 +-
fs/hfsplus/xattr.h | 32 +-
fs/hfsplus/xattr_security.c | 13 +
fs/nfs4acl.c | 1261 ++++++++++++++++++++++++++++
fs/nfsd/acl.h | 11 +-
fs/nfsd/nfs4acl.c | 782 +-----------------
include/linux/nfs4acl.h | 300 +++++++
15 files changed, 3688 insertions(+), 779 deletions(-)
--
1.7.9.5




2013-05-09 18:10:24

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] nfsd + hfsplus: introduce generalized version of NFSv4 ACLs <-> POSIX ACLs mapping algorithms

On Thu, 2013-05-09 at 21:34 +0400, Vyacheslav Dubeyko wrote:
> On May 9, 2013, at 9:01 PM, Myklebust, Trond wrote:
>
> [snip]
> >
> > How does this make sense? There is no lossless mapping of NFSv4 acls
> > into POSIX acls; the latter doesn't have any equivalent of the DENY aces
> > so you cannot represent the full set of acls that can be set using MacOS
> > on the same filesystem.
> >
> > Shouldn't you rather be looking at the richacl patch sets?
> >
>
> Yes, I understand the nature of such mapping and impossibility of mapping NFSv4 ACLs to POSIX ACLs in some cases. But, as I understand, the richacl patch set is not mainline yet. And even if it will be in mainline then a user can have choice to use POSIX ACLs or richacls. So, we need to map NFSv4 ACLs <-> POSIX ACLs in hfsplus for the case of using POSIX ACLs model. I think that to have such mapping is better than to have nothing. Moreover, a user can use HFS+ filesystem with using POSIX ACLs only under Linux. Thereby, the generalization of mapping NFSv4 ACLs <-> POSIX ACLs makes sense, from my viewpoint.

No, there is no requirement that you must support the POSIX acl
interface in addition to NFSv4/richacls.

No, supporting a POSIX mapping is not necessarily "better than nothing"
if it cannot faithfully represent the original NFSv4 acl. Do you at
least enforce the original acl in permissions checks?

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-05-10 11:18:03

by Viacheslav Dubeyko

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] nfsd + hfsplus: introduce generalized version of NFSv4 ACLs <-> POSIX ACLs mapping algorithms

On May 9, 2013, at 10:10 PM, Myklebust, Trond wrote:

[snip]
>
> No, there is no requirement that you must support the POSIX acl
> interface in addition to NFSv4/richacls.
>
> No, supporting a POSIX mapping is not necessarily "better than nothing"
> if it cannot faithfully represent the original NFSv4 acl. Do you at
> least enforce the original acl in permissions checks?

On May 9, 2013, at 10:16 PM, J. Bruce Fields wrote:

[snip]
>> Yes, I understand the nature of such mapping and impossibility of
>> mapping NFSv4 ACLs to POSIX ACLs in some cases. But, as I understand,
>> the richacl patch set is not mainline yet.
>
> Neither are these patches. You could pick up the richacl patches and
> work on them instead. That might be more work, I don't know, but the
> result would certainly be more useful, to many more people....
>
>> And even if it will be in mainline then a user can have choice to use
>> POSIX ACLs or richacls.
>
> I actually kinda like the idea of allowing people to use either model
> and translating automatically between them. But it is complicated, and
> I'm not convinced it's necessary.
>
>> So, we need to map NFSv4 ACLs <-> POSIX ACLs in hfsplus for the case
>> of using POSIX ACLs model. I think that to have such mapping is better
>> than to have nothing.
>
> So, in the "better than nothing" spirit, I'll take a look at these, but
> I would still rather we get the richacl stuff done....
>
> --b.

Yes, I am going to support richacls on hfsplus side when the richacl patch set will be in mainline. How soon it will be in mainline? Why the richacl patch set is not mainline yet? What it should be made for promoting richacl patch set in mainline?

The most of file systems in Linux support POSIX ACL model. So, I think that it makes sense to support POSIX ACLs for HFS+ also. Because it is possible to use POSIX ACLs only under Linux. And such extended attributes may be simply ignored under Mac OS X. So, what good way is for it? I think that we can use "com.apple.system.Security" xattrs for richacl model. And this xattrs will be valid NFSv4 ACLs as for Linux as for Mac OS X. But also it is possible to use "system.posix_acl_access" and "system.posix_acl_default" xattrs as storage of POSIX ACLs that will be treated only under Linux as ACLs. Mac OS X will treat such xattrs as raw xattrs without any real meaning for this OS. What do you think about such suggestion?

With the best regards,
Vyacheslav Dubeyko.


2013-05-09 18:17:40

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] nfsd + hfsplus: introduce generalized version of NFSv4 ACLs <-> POSIX ACLs mapping algorithms

On Thu, May 09, 2013 at 09:34:50PM +0400, Vyacheslav Dubeyko wrote:
> On May 9, 2013, at 9:01 PM, Myklebust, Trond wrote:
>
> [snip]
> >
> > How does this make sense? There is no lossless mapping of NFSv4 acls
> > into POSIX acls; the latter doesn't have any equivalent of the DENY
> > aces so you cannot represent the full set of acls that can be set
> > using MacOS on the same filesystem.
> >
> > Shouldn't you rather be looking at the richacl patch sets?
> >
>
> Yes, I understand the nature of such mapping and impossibility of
> mapping NFSv4 ACLs to POSIX ACLs in some cases. But, as I understand,
> the richacl patch set is not mainline yet.

Neither are these patches. You could pick up the richacl patches and
work on them instead. That might be more work, I don't know, but the
result would certainly be more useful, to many more people....

> And even if it will be in mainline then a user can have choice to use
> POSIX ACLs or richacls.

I actually kinda like the idea of allowing people to use either model
and translating automatically between them. But it is complicated, and
I'm not convinced it's necessary.

> So, we need to map NFSv4 ACLs <-> POSIX ACLs in hfsplus for the case
> of using POSIX ACLs model. I think that to have such mapping is better
> than to have nothing.

So, in the "better than nothing" spirit, I'll take a look at these, but
I would still rather we get the richacl stuff done....

--b.

> Moreover, a user can use HFS+ filesystem with
> using POSIX ACLs only under Linux. Thereby, the generalization of
> mapping NFSv4 ACLs <-> POSIX ACLs makes sense, from my viewpoint.
>
> With the best regards, Vyacheslav Dubeyko.
>
> > -- Trond Myklebust Linux NFS client maintainer
> >
> > NetApp [email protected] http://www.netapp.com
>

2013-05-09 20:36:50

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] nfsd + hfsplus: introduce generalized version of NFSv4 ACLs <-> POSIX ACLs mapping algorithms

On Thu, May 09, 2013 at 02:16:47PM -0400, J. Bruce Fields wrote:
> On Thu, May 09, 2013 at 09:34:50PM +0400, Vyacheslav Dubeyko wrote:
> > So, we need to map NFSv4 ACLs <-> POSIX ACLs in hfsplus for the case
> > of using POSIX ACLs model. I think that to have such mapping is better
> > than to have nothing.
>
> So, in the "better than nothing" spirit, I'll take a look at these, but
> I would still rather we get the richacl stuff done....

It's a lot of code so I didn't give it a detailed review. A couple
things that jump out at me:

The interface here consists of four operation arrays with a total of 30
functions between them. It seems terribly verbose. In some cases
you're compensating for that by writing macros to generate the ops.

Is there some less complicated way to do this?

Maybe just write two functions that do the NFSv4<->OSX translation, then
use the existing NFSv4<->POSIX translation unchanged? It might be a
little less efficient, but easier to understand, I think.

Second:

Note that the existing NFSv4->POSIX mapping is meant to choose the most
permissive ACL that is no more permissive than the given NFSv4 ACL.
This is because we assume it is safer to deny access that was meant to
be allowed than to allow access that was meant to be denied.

You're using the mapping on *getting* an ACL, not on setting it, so I
think you want the reverse: it's safer to claim you're allowing access
that you're not, then to claim you're denying access that you're
actually allowing.

On a quick skim I don't think you're taking that into account.

Finally:

Really, I still think this effort would be better spent working on rich
acls.

--b.

2013-05-10 17:28:15

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] nfsd + hfsplus: introduce generalized version of NFSv4 ACLs <-> POSIX ACLs mapping algorithms

On Fri, May 10, 2013 at 03:19:45PM +0400, Vyacheslav Dubeyko wrote:
> Yes, I am going to support richacls on hfsplus side when the richacl
> patch set will be in mainline. How soon it will be in mainline? Why
> the richacl patch set is not mainline yet?

Because nobody's working on it. Why don't you work on it?

> What it should be made for
> promoting richacl patch set in mainline?

Track down the latest version, rebase it to the latest kernel, make sure
it still works (I think there are also some tests that you'd want to
run). Look for feedback on previous postings, read it and take it into
account. Post new versions, get feedback. Repeat as necessary.

> The most of file systems in Linux support POSIX ACL model. So, I think
> that it makes sense to support POSIX ACLs for HFS+ also. Because it is
> possible to use POSIX ACLs only under Linux. And such extended
> attributes may be simply ignored under Mac OS X. So, what good way is
> for it? I think that we can use "com.apple.system.Security" xattrs for
> richacl model. And this xattrs will be valid NFSv4 ACLs as for Linux
> as for Mac OS X. But also it is possible to use
> "system.posix_acl_access" and "system.posix_acl_default" xattrs as
> storage of POSIX ACLs that will be treated only under Linux as ACLs.
> Mac OS X will treat such xattrs as raw xattrs without any real meaning
> for this OS. What do you think about such suggestion?

I don't know, it depends on how you expect the filesystem to be used.

--b.

2013-05-09 17:01:21

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] nfsd + hfsplus: introduce generalized version of NFSv4 ACLs <-> POSIX ACLs mapping algorithms

On Thu, 2013-05-09 at 20:37 +0400, Vyacheslav Dubeyko wrote:
> Hi,
>
> This patchset implements ACLs support in hfsplus driver and generalizes NFSv4 ACLs <-> POSIX ACLs mapping algorithms.
>
> v3->v4
> * Introduce interface of NFSv4 ACLs <-> POSIX ACLs mapping (J. Bruce Fields request).
> * Introduce generalization of NFSv4 ACLs <-> POSIX ACLs mapping algorithms (J. Bruce Fields request).
> * Rework ACLs support in HFS+ driver with the purpose of using generalized mapping algorithms.
> * Change dprint() on hfs_dbg() calls.
> * Enhance debug output in fs/hfsplus/acl.c.
>
> v2->v3
> * Fix errors in dprint_hexdump() macro.
> * Correct format on %zd for size_t in dprint() calls.
>
> v1->v2
> * Add several dprint() messages.
> * Change hardcoded function names on __func__ macro.
> * Fix coding style errors.
>
> The include/linux/nfs4acl.h header file is created with the purpose of sharing declarations and structures that were moved from fs/nfsd/nfs4acl.c and fs/nfsd/acl.h. Moreover, this header file introduces declaration of operations (nfsv4_ace_operations, nfsv4_ace_flags_operations, nfsv4_acl_id_operations, nfsv4_acl_mapping_operations) that can be specialized in concrete file system driver in the case of necessity. Otherwise, it is possible to use generalized mapping code without operations specialization. And, finally, it is declared nfsv4_acl_info structure that includes operations, pointer on mapping NFSv4 ACL and pointer on special mapping environment of concrete file system.
>
> The essence of mapping algorithms (located in fs/nfsd/nfs4acl.c, previously) were generalized and moved in fs/nfs4acl.c with the purpose of sharing between file system drivers. A concrete file system driver can use mapping code by means of map_posix_acl_to_nfsv4_one(), map_nfsv4_acl_to_posix(), nfs4_acl_posix_to_nfsv4(), nfs4_acl_nfsv4_to_posix() methods. Also, it is possible to specialize internal mapping operations in the case of very special way of operations under raw structures for concrete file system driver case.
>
> Mac OS X supports NFSv4 ACLs. It keeps its in the form of specially named xattr (com.apple.system.Security). HFS+ driver uses generalized implementation of NFSv4 ACLs <-> POSIX ACLs mapping algorithms. But it should be specialized internal mapping operations for the proper conversion of raw NFSv4 ACLs representation on HFS+ volume into POSIX ACLs. Thereby, it were implemented in hfsplus driver all necessary specialized internal mapping operations.

How does this make sense? There is no lossless mapping of NFSv4 acls
into POSIX acls; the latter doesn't have any equivalent of the DENY aces
so you cannot represent the full set of acls that can be set using MacOS
on the same filesystem.

Shouldn't you rather be looking at the richacl patch sets?

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com

2013-05-09 17:37:13

by Viacheslav Dubeyko

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] nfsd + hfsplus: introduce generalized version of NFSv4 ACLs <-> POSIX ACLs mapping algorithms

On May 9, 2013, at 9:01 PM, Myklebust, Trond wrote:

[snip]
>
> How does this make sense? There is no lossless mapping of NFSv4 acls
> into POSIX acls; the latter doesn't have any equivalent of the DENY aces
> so you cannot represent the full set of acls that can be set using MacOS
> on the same filesystem.
>
> Shouldn't you rather be looking at the richacl patch sets?
>

Yes, I understand the nature of such mapping and impossibility of mapping NFSv4 ACLs to POSIX ACLs in some cases. But, as I understand, the richacl patch set is not mainline yet. And even if it will be in mainline then a user can have choice to use POSIX ACLs or richacls. So, we need to map NFSv4 ACLs <-> POSIX ACLs in hfsplus for the case of using POSIX ACLs model. I think that to have such mapping is better than to have nothing. Moreover, a user can use HFS+ filesystem with using POSIX ACLs only under Linux. Thereby, the generalization of mapping NFSv4 ACLs <-> POSIX ACLs makes sense, from my viewpoint.

With the best regards,
Vyacheslav Dubeyko.

> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com