Don't set NFSv4 ACL inheritance flags on non-directories.

They only make sense in the context of directory ACLs, and attempting
to set them on regular files results in errors, causing a recursive
setfacl invocation to abort.

This is derived from patches by Shawn Webb <shawn.webb@hardenedbsd.org>
and Mitchell Horne <mhorne063@gmail.com>.

PR:		155163
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D15061

2 files changed, 71 insertions, 14 deletions

diff --git a/bin/setfacl/setfacl.1 b/bin/setfacl/setfacl.1
index 249055d0c338..31040a61558c 100644
--- a/bin/setfacl/setfacl.1
+++ b/bin/setfacl/setfacl.1
@@ -26,7 +26,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd April 10, 2018
+.Dd October 26, 2018
 .Dt SETFACL 1
 .Os
 .Sh NAME
@@ -136,6 +136,8 @@ option is specified, no symbolic links are followed.
 This is the default.
 .It Fl R
 Perform the action recursively on any specified directories.
+When modifying or adding NFSv4 ACL entries, inheritance flags
+are applied only to directories.
 .It Fl x Ar entries | position
 If
 .Ar entries
diff --git a/bin/setfacl/setfacl.c b/bin/setfacl/setfacl.c
index 3366953ea8b6..7b0d617812dc 100644
--- a/bin/setfacl/setfacl.c
+++ b/bin/setfacl/setfacl.c
@@ -27,9 +27,7 @@
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD$");
 
-#include <sys/types.h>
 #include <sys/param.h>
-#include <sys/stat.h>
 #include <sys/acl.h>
 #include <sys/queue.h>
 
@@ -73,6 +71,7 @@ static bool need_mask;
 static acl_type_t acl_type = ACL_TYPE_ACCESS;
 
 static int	handle_file(FTS *ftsp, FTSENT *file);
+static acl_t	clear_inheritance_flags(acl_t acl);
 static char	**stdin_files(void);
 static void	usage(void);
 
@@ -124,10 +123,57 @@ stdin_files(void)
 	return (files_list);
 }
 
+/*
+ * Remove any inheritance flags from NFSv4 ACLs when running in recursive
+ * mode.  This is to avoid files being assigned identical ACLs to their
+ * parent directory while also being set to inherit them.
+ *
+ * The acl argument is assumed to be valid.
+ */
+static acl_t
+clear_inheritance_flags(acl_t acl)
+{
+	acl_t nacl;
+	acl_entry_t acl_entry;
+	acl_flagset_t acl_flagset;
+	int acl_brand, entry_id;
+
+	(void)acl_get_brand_np(acl, &acl_brand);
+	if (acl_brand != ACL_BRAND_NFS4)
+		return (acl);
+
+	nacl = acl_dup(acl);
+	if (nacl == NULL) {
+		warn("acl_dup() failed");
+		return (acl);
+	}
+
+	entry_id = ACL_FIRST_ENTRY;
+	while (acl_get_entry(nacl, entry_id, &acl_entry) == 1) {
+		entry_id = ACL_NEXT_ENTRY;
+		if (acl_get_flagset_np(acl_entry, &acl_flagset) != 0) {
+			warn("acl_get_flagset_np() failed");
+			continue;
+		}
+		if (acl_get_flag_np(acl_flagset, ACL_ENTRY_INHERIT_ONLY) == 1) {
+			if (acl_delete_entry(nacl, acl_entry) != 0)
+				warn("acl_delete_entry() failed");
+			continue;
+		}
+		if (acl_delete_flag_np(acl_flagset,
+		    ACL_ENTRY_FILE_INHERIT |
+		    ACL_ENTRY_DIRECTORY_INHERIT |
+		    ACL_ENTRY_NO_PROPAGATE_INHERIT) != 0)
+			warn("acl_delete_flag_np() failed");
+	}
+
+	return (nacl);
+}
+
 static int
 handle_file(FTS *ftsp, FTSENT *file)
 {
-	acl_t acl;
+	acl_t acl, nacl;
 	acl_entry_t unused_entry;
 	int local_error, ret;
 	struct sf_entry *entry;
@@ -193,17 +239,20 @@ handle_file(FTS *ftsp, FTSENT *file)
 
 	/* Cycle through each option. */
 	TAILQ_FOREACH(entry, &entrylist, next) {
-		if (local_error)
-			continue;
-
-		switch(entry->op) {
+		nacl = entry->acl;
+		switch (entry->op) {
 		case OP_ADD_ACL:
-			local_error += add_acl(entry->acl, entry->entry_number,
-			    &acl, file->fts_path);
+			if (R_flag && file->fts_info != FTS_D &&
+			    acl_type == ACL_TYPE_NFS4)
+				nacl = clear_inheritance_flags(nacl);
+			local_error += add_acl(nacl, entry->entry_number, &acl,
+			    file->fts_path);
 			break;
 		case OP_MERGE_ACL:
-			local_error += merge_acl(entry->acl, &acl,
-			    file->fts_path);
+			if (R_flag && file->fts_info != FTS_D &&
+			    acl_type == ACL_TYPE_NFS4)
+				nacl = clear_inheritance_flags(nacl);
+			local_error += merge_acl(nacl, &acl, file->fts_path);
 			need_mask = true;
 			break;
 		case OP_REMOVE_EXT:
@@ -240,8 +289,7 @@ handle_file(FTS *ftsp, FTSENT *file)
 			need_mask = false;
 			break;
 		case OP_REMOVE_ACL:
-			local_error += remove_acl(entry->acl, &acl,
-			    file->fts_path);
+			local_error += remove_acl(nacl, &acl, file->fts_path);
 			need_mask = true;
 			break;
 		case OP_REMOVE_BY_NUMBER:
@@ -250,6 +298,13 @@ handle_file(FTS *ftsp, FTSENT *file)
 			need_mask = true;
 			break;
 		}
+
+		if (nacl != entry->acl) {
+			acl_free(nacl);
+			nacl = NULL;
+		}
+		if (local_error)
+			break;
 	}
 
 	ret = 0;