Skip to content

Commit

Permalink
Don't set NFSv4 ACL inheritance flags on non-directories.
Browse files Browse the repository at this point in the history
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 <[email protected]>
and Mitchell Horne <[email protected]>.

PR:		155163
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D15061
  • Loading branch information
markjdb committed Oct 26, 2018
1 parent 8397e60 commit 2793a18
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 14 deletions.
4 changes: 3 additions & 1 deletion bin/setfacl/setfacl.1
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
.\"
.\" $FreeBSD$
.\"
.Dd April 10, 2018
.Dd October 26, 2018
.Dt SETFACL 1
.Os
.Sh NAME
Expand Down Expand Up @@ -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
Expand Down
81 changes: 68 additions & 13 deletions bin/setfacl/setfacl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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;
Expand Down

0 comments on commit 2793a18

Please sign in to comment.