Stephen Smalley <stephen.smalley.work@xxxxxxxxx> writes:
> 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.
Looks like you are right. I'll send updated version.
