| Message ID | 20210113121120.164565-1-plautrba@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | setfiles: Do not abort on labeling error | expand |
On Wed, Jan 13, 2021 at 7:15 AM Petr Lautrbach <plautrba@redhat.com> 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@redhat.com> > --- > 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.
Stephen Smalley <stephen.smalley.work@gmail.com> writes: > On Wed, Jan 13, 2021 at 7:15 AM Petr Lautrbach <plautrba@redhat.com> 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@redhat.com> >> --- >> 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. Looks like you are right. I'll send updated version.
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); - if (stat(argv[optind], &sb) < 0) { perror(argv[optind]); exit(-1);
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@redhat.com> --- policycoreutils/setfiles/setfiles.c | 49 +---------------------------- 1 file changed, 1 insertion(+), 48 deletions(-)