From 5fa76c4883985b89802574ec7f47ccc186eb2201 Mon Sep 17 00:00:00 2001 From: Michael Orlitzky Date: Tue, 27 Mar 2018 23:50:41 -0400 Subject: [PATCH] Drop the "--no-exec-mask" flag and function parameters. Nobody needs the "--no-exec-mask" flag, and it's uglying up the library's API. Sayonara: * Update the man page: * Remove all mentions of the flag. * Update the algorithm description. * Reword the general description. * Remove all --no-exec-mask tests. * Bump the program version in configure.ac. * Make apply_default_acl() work as if no_exec_mask == false. * Remove all no_exec_mask function parameters. * Bump the soname major version in src/Makefile.am. --- configure.ac | 2 +- doc/man/apply-default-acl.1 | 26 ++++++--------- run-tests.sh | 64 ++++--------------------------------- src/Makefile.am | 5 +-- src/apply-default-acl.c | 8 +---- src/libadacl.c | 39 ++++++++-------------- src/libadacl.h | 2 +- 7 files changed, 36 insertions(+), 110 deletions(-) diff --git a/configure.ac b/configure.ac index 80f678d..c16c0d7 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ AC_PREREQ([2.68]) -AC_INIT([apply-default-acl], [0.3.1], [michael@orlitzky.com]) +AC_INIT([apply-default-acl], [0.4.0], [michael@orlitzky.com]) AM_INIT_AUTOMAKE([-Wall foreign no-dist-gzip dist-xz]) AC_CONFIG_FILES([Makefile src/Makefile]) AC_CONFIG_SRCDIR([src/apply-default-acl.c]) diff --git a/doc/man/apply-default-acl.1 b/doc/man/apply-default-acl.1 index e6a6799..d3b2de1 100644 --- a/doc/man/apply-default-acl.1 +++ b/doc/man/apply-default-acl.1 @@ -5,7 +5,7 @@ apply-default-acl \- Apply default POSIX ACLs to files and directories. .SH SYNOPSIS -\fBapply-default-acl\fR [\fB-rx\fR] \fIpath\fR [\fIpath2 ...\fR] +\fBapply-default-acl\fR [\fB-r\fR] \fIpath\fR [\fIpath2 ...\fR] .SH DESCRIPTION @@ -15,14 +15,12 @@ If the directory containing \fIpath\fR has a default ACL, the ACL on links are followed; symbolic links are ignored in all path components to avoid a dangerous race condition. .P -By default, a heuristic is used to determine whether or not the -execute bit is masked on \fIpath\fR. If \fIpath\fR is not a directory, -and no user or group has \fBeffective\fR execute permissions on -\fIpath\fR, then the execute bit will not masked. Otherwise, it is -left alone. In effect we pretend that the \fBx\fR permission acts like +A heuristic is used to determine whether or not the execute bits are +removed from \fIpath\fR. If \fIpath\fR is a directory or if some user +or group has \fBeffective\fR execute permissions on \fIpath\fR, then +the execute bits will be left alone. Otherwise, they will be +removed. In effect we pretend that the \fBx\fR permission acts like the \fBX\fR (note the case difference) permission of \fBsetfacl\fR. -.P -This behavior can be modified with the \fB--no-exec-mask\fR flag. .SH OPTIONS .IP \fB\-\-recursive\fR,\ \fB\-r\fR @@ -30,17 +28,13 @@ Apply default ACLs recursively. This works top-down, so if directory \fBfoo\fR is in another directory \fBbar\fR which has a default ACL, then \fBbar\fR's default ACL will be applied to \fBfoo\fR before the contents of \fBfoo\fR are processed. -.IP \fB\-\-no-exec-mask\fR,\ \fB\-x\fR -Apply the default ACL literally; that is, don't use a heuristic to -decide whether or not to mask the execute bit. This usually results in -looser-than-necessary execute permissions. .SH ALGORITHM .IP "I. Argument validation" 0.4i .RS .IP "a. If any part of the target path contains a symlink" 0.4i Return failure -.IP "b. If there's no default ACL to apply" +.IP "b. If there is no default ACL to apply" Return success .IP "c. If the target is not a (non-hardlink) regular file or directory" Return failure @@ -54,10 +48,8 @@ Set the target's default ACL equal to its parent's default ACL Return success .IP "d. If the target was executable by anyone" Return success -.IP "e. If \fB--no-exec-mask\fR was given" -Return success -.IP "f. Unset the user/group/other/mask execute bits" -.IP "g. Return success" +.IP "e. Unset the user/group/other/mask execute bits" +.IP "f. Return success" .RE .P The action of apply-default ACL largely mimics what the kernel would diff --git a/run-tests.sh b/run-tests.sh index 923c19c..d33fcf0 100755 --- a/run-tests.sh +++ b/run-tests.sh @@ -259,7 +259,7 @@ ACTUAL=$(getfacl --omit-header "${TARGET}") compare -# A slightly modified test #1 to make sure it works right. +# A slightly modified version of the first test, to make sure it works. ((TESTNUM++)) TARGET="${TESTDIR}"/foo touch "${TARGET}" @@ -306,7 +306,7 @@ compare # The --recursive mode should work normally if the argument is a -# normal file. See Test #1. +# normal file. See the first test. ((TESTNUM++)) TARGET="${TESTDIR}"/foo setfacl -d -m user::r-- "${TESTDIR}" @@ -533,57 +533,6 @@ compare -# Test #16's setup repeated with the --no-exec-mask flag. -# -((TESTNUM++)) -TARGET="${TESTDIR}"/foo -touch "${TARGET}" -chmod 644 "${TARGET}" -# The directory allows execute for user, group, and other, so the file -# should actually inherit them regardless of its initial mode when the -# --no-exec-mask flag is passed. -setfacl -d -m user:${USERS[0]}:rwx "${TESTDIR}" - -$BIN --no-exec-mask "${TARGET}" - -EXPECTED=$(cat <&1 ) EXPECTED="test/nonexistent: No such file or directory" compare + # Same as the previous test, but with --recursive. ((TESTNUM++)) ACTUAL=$( "${BIN}" --recursive test/nonexistent 2>&1 ) EXPECTED="test/nonexistent: No such file or directory" compare + # If we call apply-default-acl on more than one file, it should report any # that don't exist (but proceed to operate on the others). ((TESTNUM++)) @@ -712,8 +663,7 @@ EOF compare -# Ensure that symlinks are not followed in subdirectories -# (recursively). +# Ensure that symlinks are not followed in subdirectories (recursively). ((TESTNUM++)) TARGET="${TESTDIR}/bar" touch "${TARGET}" diff --git a/src/Makefile.am b/src/Makefile.am index 94a4c1d..635492f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -4,8 +4,9 @@ libadacl_la_LIBADD = -lacl include_HEADERS = libadacl.h # The apply_default_acl_ex() function was dropped in v0.2.0, -# and the "recursive" parameter was added in v0.3.0. -libadacl_la_LDFLAGS = -version-info 2:0:0 +# and the "recursive" parameter was added in v0.3.0, +# and the "no_exec_mask" parameter was dropped in v0.4.0. +libadacl_la_LDFLAGS = -version-info 3:0:0 bin_PROGRAMS = apply-default-acl apply_default_acl_LDADD = libadacl.la diff --git a/src/apply-default-acl.c b/src/apply-default-acl.c index 68de557..1d65ca0 100644 --- a/src/apply-default-acl.c +++ b/src/apply-default-acl.c @@ -73,7 +73,6 @@ void usage(const char* program_name) { printf("Flags:\n"); printf(" -h, --help Print this help message\n"); printf(" -r, --recursive Act on any given directories recursively\n"); - printf(" -x, --no-exec-mask Apply execute permissions unconditionally\n"); return; } @@ -97,13 +96,11 @@ int main(int argc, char* argv[]) { } bool recursive = false; - bool no_exec_mask = false; struct option long_options[] = { /* These options set a flag. */ {"help", no_argument, NULL, 'h'}, {"recursive", no_argument, NULL, 'r'}, - {"no-exec-mask", no_argument, NULL, 'x'}, {NULL, 0, NULL, 0} }; @@ -117,9 +114,6 @@ int main(int argc, char* argv[]) { case 'r': recursive = true; break; - case 'x': - no_exec_mask = true; - break; default: usage(argv[0]); return EXIT_FAILURE; @@ -144,7 +138,7 @@ int main(int argc, char* argv[]) { continue; } - reapp_result = apply_default_acl(target, no_exec_mask, recursive); + reapp_result = apply_default_acl(target, recursive); if (result == EXIT_SUCCESS && reapp_result == ACL_FAILURE) { /* We don't want to turn an error into a (less-severe) failure. */ diff --git a/src/libadacl.c b/src/libadacl.c index 62ab3cc..1d53948 100644 --- a/src/libadacl.c +++ b/src/libadacl.c @@ -654,9 +654,6 @@ int has_default_acl_fd(int fd) { * @param fd * The file descriptor that should inherit its parent's default ACL. * - * @param no_exec_mask - * The value (either true or false) of the --no-exec-mask flag. - * * @param recursive * Should we recurse into subdirectories? * @@ -667,7 +664,6 @@ int has_default_acl_fd(int fd) { */ int apply_default_acl_fds(int parent_fd, int fd, - bool no_exec_mask, bool recursive) { int result = ACL_SUCCESS; @@ -718,24 +714,20 @@ int apply_default_acl_fds(int parent_fd, } - /* Default to not masking the exec bit; i.e. applying the default - ACL literally. If --no-exec-mask was not specified, then we try - to "guess" whether or not to mask the exec bit. This behavior - is modeled after the capital 'X' perms of setfacl. */ - bool allow_exec = true; + /* Next We try to guess whether or not to strip the execute bits. + * This behavior is modeled after the capital 'X' perms of setfacl. + */ + int ace_result = any_can_execute(fd, &s); - if (!no_exec_mask) { - /* Never mask the execute bit on directories. */ - int ace_result = any_can_execute(fd,&s) || S_ISDIR(s.st_mode); + if (ace_result == ACL_ERROR) { + perror("apply_default_acl_fds (any_can_execute)"); + result = ACL_ERROR; + goto cleanup; + } - if (ace_result == ACL_ERROR) { - perror("apply_default_acl_fds (any_can_execute)"); - result = ACL_ERROR; - goto cleanup; - } + /* Never mask the execute bit on directories. */ + bool allow_exec = (bool)ace_result || S_ISDIR(s.st_mode); - allow_exec = (bool)ace_result; - } /* If it's a directory, inherit the parent's default. */ if (S_ISDIR(s.st_mode)) { @@ -905,7 +897,7 @@ int apply_default_acl_fds(int parent_fd, continue; } } - switch (apply_default_acl_fds(fd, new_fd, no_exec_mask, recursive)) { + switch (apply_default_acl_fds(fd, new_fd, recursive)) { /* Don't overwrite an error result with success/failure. */ case ACL_FAILURE: if (result == ACL_SUCCESS) { @@ -940,9 +932,6 @@ int apply_default_acl_fds(int parent_fd, * @param path * The path whose ACL we would like to reset to its default. * - * @param no_exec_mask - * The value (either true or false) of the --no-exec-mask flag. - * * @param recursive * Should we recurse into subdirectories? * @@ -951,7 +940,7 @@ int apply_default_acl_fds(int parent_fd, * - @c ACL_FAILURE - If symlinks or hard links are encountered. * - @c ACL_ERROR - Unexpected library error. */ -int apply_default_acl(const char* path, bool no_exec_mask, bool recursive) { +int apply_default_acl(const char* path, bool recursive) { if (path == NULL) { errno = EINVAL; @@ -1024,7 +1013,7 @@ int apply_default_acl(const char* path, bool no_exec_mask, bool recursive) { } } - result = apply_default_acl_fds(parent_fd, fd, no_exec_mask, recursive); + result = apply_default_acl_fds(parent_fd, fd, recursive); cleanup: free(dirname_path_copy); diff --git a/src/libadacl.h b/src/libadacl.h index b9f0e21..4e0c75d 100644 --- a/src/libadacl.h +++ b/src/libadacl.h @@ -15,4 +15,4 @@ #define ACL_FAILURE 0 #define ACL_SUCCESS 1 -int apply_default_acl(const char* path, bool no_exec_mask, bool recursive); +int apply_default_acl(const char* path, bool recursive); -- 2.43.2