On Wed, Jan 13, 2021 at 7:15 AM Petr Lautrbach <plautrba@xxxxxxxxxx> wrote:
>
> Commit 602347c7422e ("policycoreutils: setfiles - Modify to use
> selinux_restorecon") changed behavior of setfiles. Original
> implementation skipped files which it couldn't set context to while the
> new implementation aborts on them. setfiles should abort only if it
> can't validate 10 contexts from spec_file.
>
> Reproducer:
>
> # mkdir -p r/1 r/2 r/3
> # touch r/1/1 r/2/1
> # chattr +i r/2/1
> # touch r/3/1
> # setfiles -r r -v /etc/selinux/targeted/contexts/files/file_contexts r
> Relabeled r from unconfined_u:object_r:mnt_t:s0 to unconfined_u:object_r:root_t:s0
> Relabeled r/2 from unconfined_u:object_r:mnt_t:s0 to unconfined_u:object_r:default_t:s0
> setfiles: Could not set context for r/2/1: Operation not permitted
>
> r/3 and r/1 are not relabeled.
>
> Also drop some old unused code in order to prevent future confusion.
>
> Signed-off-by: Petr Lautrbach <plautrba@xxxxxxxxxx>
> ---
> policycoreutils/setfiles/setfiles.c | 49 +----------------------------
> 1 file changed, 1 insertion(+), 48 deletions(-)
>
> diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
> index 422c3767b845..b96ee814bad2 100644
> --- a/policycoreutils/setfiles/setfiles.c
> +++ b/policycoreutils/setfiles/setfiles.c
> @@ -23,14 +23,6 @@ static int nerr;
>
> #define STAT_BLOCK_SIZE 1
>
> -/* setfiles will abort its operation after reaching the
> - * following number of errors (e.g. invalid contexts),
> - * unless it is used in "debug" mode (-d option).
> - */
> -#ifndef ABORT_ON_ERRORS
> -#define ABORT_ON_ERRORS 10
> -#endif
> -
> #define SETFILES "setfiles"
> #define RESTORECON "restorecon"
> static int iamrestorecon;
> @@ -56,15 +48,6 @@ static __attribute__((__noreturn__)) void usage(const char *const name)
> exit(-1);
> }
>
> -void inc_err(void)
> -{
> - nerr++;
> - if (nerr > ABORT_ON_ERRORS - 1 && !r_opts.debug) {
> - fprintf(stderr, "Exiting after %d errors.\n", ABORT_ON_ERRORS);
> - exit(-1);
> - }
> -}
> -
> void set_rootpath(const char *arg)
> {
> if (strlen(arg) == 1 && strncmp(arg, "/", 1) == 0) {
> @@ -82,27 +65,6 @@ void set_rootpath(const char *arg)
> }
> }
>
> -int canoncon(char **contextp)
> -{
> - char *context = *contextp, *tmpcon;
> - int rc = 0;
> -
> - if (policyfile) {
> - if (sepol_check_context(context) < 0) {
> - fprintf(stderr, "invalid context %s\n", context);
> - exit(-1);
> - }
> - } else if (security_canonicalize_context_raw(context, &tmpcon) == 0) {
> - free(context);
> - *contextp = tmpcon;
> - } else if (errno != ENOENT) {
> - rc = -1;
> - inc_err();
> - }
> -
> - return rc;
> -}
> -
> #ifndef USE_AUDIT
> static void maybe_audit_mass_relabel(int mass_relabel __attribute__((unused)),
> int mass_relabel_errs __attribute__((unused)))
> @@ -181,6 +143,7 @@ int main(int argc, char **argv)
> policyfile = NULL;
> nerr = 0;
>
> + r_opts.abort_on_error = 0;
> r_opts.progname = strdup(argv[0]);
> if (!r_opts.progname) {
> fprintf(stderr, "%s: Out of memory!\n", argv[0]);
> @@ -193,7 +156,6 @@ int main(int argc, char **argv)
> * setfiles:
> * Recursive descent,
> * Does not expand paths via realpath,
> - * Aborts on errors during the file tree walk,
> * Try to track inode associations for conflict detection,
> * Does not follow mounts (sets SELINUX_RESTORECON_XDEV),
> * Validates all file contexts at init time.
> @@ -201,7 +163,6 @@ int main(int argc, char **argv)
> iamrestorecon = 0;
> r_opts.recurse = SELINUX_RESTORECON_RECURSE;
> r_opts.userealpath = 0; /* SELINUX_RESTORECON_REALPATH */
> - r_opts.abort_on_error = SELINUX_RESTORECON_ABORT_ON_ERROR;
> r_opts.add_assoc = SELINUX_RESTORECON_ADD_ASSOC;
> /* FTS_PHYSICAL and FTS_NOCHDIR are always set by selinux_restorecon(3) */
> r_opts.xdev = SELINUX_RESTORECON_XDEV;
> @@ -225,7 +186,6 @@ int main(int argc, char **argv)
> iamrestorecon = 1;
> r_opts.recurse = 0;
> r_opts.userealpath = SELINUX_RESTORECON_REALPATH;
> - r_opts.abort_on_error = 0;
> r_opts.add_assoc = 0;
> r_opts.xdev = 0;
> r_opts.ignore_mounts = 0;
> @@ -420,13 +380,6 @@ int main(int argc, char **argv)
> usage(argv[0]);
> }
>
> - /* Use our own invalid context checking function so that
> - * we can support either checking against the active policy or
> - * checking against a binary policy file.
> - */
> - cb.func_validate = canoncon;
> - selinux_set_callback(SELINUX_CB_VALIDATE, cb);
> -
I could be wrong but I think we still need this for setfiles -c.
