ACL patch - is this bad or good?

Hi there. A Debian user contributed a copy of what he says is a Red Hat patch to support "acl" and "no_acl" options for NFS exports.

Is this patch OK?

Chip Salzenberg              - a.k.a. -           <[email protected]>
People are supposed to die for freedom.  Not the other way around.

Add `no_acl' nfs export option

This patch adds the acl' and no_acl' nfs export options, which replace the nfs_permission_mode module parameter of nfsd.o. The `no_acl' option tells nfsd to mask off acl permissions so that clients will see a subset of permissions that is safe even with old clients. Current clients implement the NFSv3 ACCESS RPC, and therefore do not require the no_acl' export option. If no acls are supported in the nfs server, the no_acl' export option is not needed, either.

Thanks to Steve Dickson <[email protected]> for proposing this.

Andreas Gruenbacher <[email protected]>, SuSE Labs

Index: nfs-utils-1.0.4/utils/exportfs/exportfs.c
===================================================================
@@ -378,6 +378,8 @@ dump(int verbose)
 				c = dumpopt(c, "no_subtree_check");
 			if (ep->e_flags & NFSEXP_NOAUTHNLM)
 				c = dumpopt(c, "insecure_locks");
+			if (ep->e_flags & NFSEXP_NOACL)
+				c = dumpopt(c, "no_acl");
 			if (ep->e_flags & NFSEXP_FSID)
 				c = dumpopt(c, "fsid=%d", ep->e_fsid);
 			if (ep->e_mountpoint)

Index: nfs-utils-1.0.4/support/nfs/exports.c
===================================================================
@@ -185,6 +185,8 @@ putexportent(struct exportent *ep)
 		"no_" : "");
 	fprintf(fp, "%ssecure_locks,", (ep->e_flags & NFSEXP_NOAUTHNLM)?
 		"in" : "");
+	fprintf(fp, "%sacl,", (ep->e_flags & NFSEXP_NOACL)?
+		"no_" : "");
 	if (ep->e_flags & NFSEXP_FSID) {
 		fprintf(fp, "fsid=%d,", ep->e_fsid);
 	}

@@ -374,6 +376,10 @@ parseopts(char *cp, struct exportent *ep
 			ep->e_flags &= ~NFSEXP_NOAUTHNLM;
 		else if (strcmp(opt, "insecure_locks") == 0)
 			ep->e_flags |= NFSEXP_NOAUTHNLM;
+		else if (strcmp(opt, "acl") == 0)
+			ep->e_flags &= ~NFSEXP_NOACL;
+		else if (strcmp(opt, "no_acl") == 0)
+			ep->e_flags |= NFSEXP_NOACL;
 		else if (strncmp(opt, "mapping=", 8) == 0)
 			ep->e_maptype = parsemaptype(opt+8);
 		else if (strcmp(opt, "map_identity") == 0)	/* old style */

Index: nfs-utils-1.0.4/utils/exportfs/exports.man
===================================================================
@@ -218,6 +218,21 @@ be explicitly requested with either of t
 .IR auth_nlm ,
 or
 .IR secure_locks .
+.TP
+.IR no_acl
+This option tells nfsd to mask off acl permissions so that clients will
+only see a subset of the permissions on the exported file system. This
+subset is safe for NFSv2 clients, and for NFSv3 clients that perform
+access decisions locally. Current NFSv3 clients use the ACCESS RPC
+to perform all access decisions on the server. The
+.I no_acl
+option should be used for nfs exports with acl support that are exported
+to NFSv2 clients, or to NFSv3 clients that don't use the ACCESS RPC.
+This option is not needed for recent NFSv3 clients or if the exported
+file system has no acl support. The default is to export with acl
+support enabled (i.e.,
+.I no_acl
+is off.)
 
 '''.TP
 '''.I noaccess

Define "OK" ???

I must admit a personal bias. I find it very hard to get enthusiastic
about anything relating to ACLs. I think they are one of the worst
mis-features ever to be grafted on to the side of a filesystem.

I am *very* tempted to say "Don't use an old client if your server supports ACLs", but there are probably some poor souls out there who are forced to do so due to circumstances beyond their control......

This patch is OK as far as it goes. However it is completely useless against a kernel.org kernel as support for NFSEXP_NOACL hasn't been merged yet.

But I suspect it will be if anyone bothers to pester me about it .....

So I am OK with this patch going in to nfs-utils, providing the man page make it clear that not all kernels support this functionality, and will quietly accept but ignore the flag.

Anyone want to re-make the patch with a suitable change to the manpage?

NeilBrown

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

I'll add it to my todo list....

steved.

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. http://productguide.itmanagersjournal.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

I've done so.

Chip Salzenberg            - a.k.a. -            <[email protected]>
         Open Source is not an excuse to write fun code
            then leave the actual work to others.


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs