selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] setfiles: Do not abort on labeling error
@ 2021-01-13 21:09 Petr Lautrbach
  2021-01-13 21:09 ` [PATCH v2 2/2] setfiles: drop ABORT_ON_ERRORS and related code Petr Lautrbach
  0 siblings, 1 reply; 4+ messages in thread
From: Petr Lautrbach @ 2021-01-13 21:09 UTC (permalink / raw)
  To: selinux; +Cc: Petr Lautrbach

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 a context 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.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 policycoreutils/setfiles/setfiles.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
index 422c3767b845..10692d6d94a0 100644
--- a/policycoreutils/setfiles/setfiles.c
+++ b/policycoreutils/setfiles/setfiles.c
@@ -181,6 +181,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 +194,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 +201,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 +224,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;
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] setfiles: drop ABORT_ON_ERRORS and related code
  2021-01-13 21:09 [PATCH v2 1/2] setfiles: Do not abort on labeling error Petr Lautrbach
@ 2021-01-13 21:09 ` Petr Lautrbach
  2021-01-31 10:27   ` Petr Lautrbach
  0 siblings, 1 reply; 4+ messages in thread
From: Petr Lautrbach @ 2021-01-13 21:09 UTC (permalink / raw)
  To: selinux; +Cc: Petr Lautrbach

`setfiles -d` doesn't have any impact on number of errors before it
aborts. It always aborts on first invalid context in spec file.

Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
---
 policycoreutils/setfiles/Makefile      |  3 ---
 policycoreutils/setfiles/ru/setfiles.8 |  2 +-
 policycoreutils/setfiles/setfiles.8    |  3 +--
 policycoreutils/setfiles/setfiles.c    | 18 ------------------
 4 files changed, 2 insertions(+), 24 deletions(-)

diff --git a/policycoreutils/setfiles/Makefile b/policycoreutils/setfiles/Makefile
index bc5a8db789a5..a3bbbe116b7f 100644
--- a/policycoreutils/setfiles/Makefile
+++ b/policycoreutils/setfiles/Makefile
@@ -5,8 +5,6 @@ SBINDIR ?= /sbin
 MANDIR = $(PREFIX)/share/man
 AUDITH ?= $(shell test -f /usr/include/libaudit.h && echo y)
 
-ABORT_ON_ERRORS=$(shell grep "^\#define ABORT_ON_ERRORS" setfiles.c | awk -S '{ print $$3 }')
-
 CFLAGS ?= -g -Werror -Wall -W
 override LDLIBS += -lselinux -lsepol
 
@@ -26,7 +24,6 @@ restorecon_xattr: restorecon_xattr.o restore.o
 
 man:
 	@cp -af setfiles.8 setfiles.8.man
-	@sed -i "s/ABORT_ON_ERRORS/$(ABORT_ON_ERRORS)/g" setfiles.8.man
 
 install: all
 	[ -d $(DESTDIR)$(MANDIR)/man8 ] || mkdir -p $(DESTDIR)$(MANDIR)/man8
diff --git a/policycoreutils/setfiles/ru/setfiles.8 b/policycoreutils/setfiles/ru/setfiles.8
index 27815a3f1eee..910101452625 100644
--- a/policycoreutils/setfiles/ru/setfiles.8
+++ b/policycoreutils/setfiles/ru/setfiles.8
@@ -47,7 +47,7 @@ setfiles \- установить SELinux-контексты безопаснос
 проверить действительность контекстов относительно указанной двоичной политики.
 .TP
 .B \-d
-показать, какая спецификация соответствует каждому из файлов (не прекращать проверку после получения ошибок ABORT_ON_ERRORS).
+показать, какая спецификация соответствует каждому из файлов.
 .TP
 .BI \-e \ directory
 исключить каталог (чтобы исключить более одного каталога, этот параметр необходимо использовать соответствующее количество раз).
diff --git a/policycoreutils/setfiles/setfiles.8 b/policycoreutils/setfiles/setfiles.8
index e328a5628682..4d28bc9a95c1 100644
--- a/policycoreutils/setfiles/setfiles.8
+++ b/policycoreutils/setfiles/setfiles.8
@@ -57,8 +57,7 @@ option will force a replacement of the entire context.
 check the validity of the contexts against the specified binary policy.
 .TP
 .B \-d
-show what specification matched each file (do not abort validation
-after ABORT_ON_ERRORS errors).
+show what specification matched each file.
 .TP
 .BI \-e \ directory
 directory to exclude (repeat option for more than one directory).
diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
index 10692d6d94a0..92616571ef2a 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) {
@@ -97,7 +80,6 @@ int canoncon(char **contextp)
 		*contextp = tmpcon;
 	} else if (errno != ENOENT) {
 		rc = -1;
-		inc_err();
 	}
 
 	return rc;
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] setfiles: drop ABORT_ON_ERRORS and related code
  2021-01-13 21:09 ` [PATCH v2 2/2] setfiles: drop ABORT_ON_ERRORS and related code Petr Lautrbach
@ 2021-01-31 10:27   ` Petr Lautrbach
       [not found]     ` <CAJfZ7=my52AG+zYMjXJFoxAAHsnJTcs8Y+crbcQ==rT2cWZ-Dg@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Petr Lautrbach @ 2021-01-31 10:27 UTC (permalink / raw)
  To: selinux

Petr Lautrbach <plautrba@redhat.com> writes:

> `setfiles -d` doesn't have any impact on number of errors before it
> aborts. It always aborts on first invalid context in spec file.
>
> Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
> ---
>  policycoreutils/setfiles/Makefile      |  3 ---
>  policycoreutils/setfiles/ru/setfiles.8 |  2 +-
>  policycoreutils/setfiles/setfiles.8    |  3 +--
>  policycoreutils/setfiles/setfiles.c    | 18 ------------------
>  4 files changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/policycoreutils/setfiles/Makefile b/policycoreutils/setfiles/Makefile
> index bc5a8db789a5..a3bbbe116b7f 100644
> --- a/policycoreutils/setfiles/Makefile
> +++ b/policycoreutils/setfiles/Makefile
> @@ -5,8 +5,6 @@ SBINDIR ?= /sbin
>  MANDIR = $(PREFIX)/share/man
>  AUDITH ?= $(shell test -f /usr/include/libaudit.h && echo y)
>  
> -ABORT_ON_ERRORS=$(shell grep "^\#define ABORT_ON_ERRORS" setfiles.c | awk -S '{ print $$3 }')
> -
>  CFLAGS ?= -g -Werror -Wall -W
>  override LDLIBS += -lselinux -lsepol
>  
> @@ -26,7 +24,6 @@ restorecon_xattr: restorecon_xattr.o restore.o
>  
>  man:
>  	@cp -af setfiles.8 setfiles.8.man
> -	@sed -i "s/ABORT_ON_ERRORS/$(ABORT_ON_ERRORS)/g" setfiles.8.man
>  
>  install: all
>  	[ -d $(DESTDIR)$(MANDIR)/man8 ] || mkdir -p $(DESTDIR)$(MANDIR)/man8
> diff --git a/policycoreutils/setfiles/ru/setfiles.8 b/policycoreutils/setfiles/ru/setfiles.8
> index 27815a3f1eee..910101452625 100644
> --- a/policycoreutils/setfiles/ru/setfiles.8
> +++ b/policycoreutils/setfiles/ru/setfiles.8
> @@ -47,7 +47,7 @@ setfiles \- установить SELinux-контексты безопаснос
>  проверить действительность контекстов относительно указанной двоичной политики.
>  .TP
>  .B \-d
> -показать, какая спецификация соответствует каждому из файлов (не прекращать проверку после получения ошибок ABORT_ON_ERRORS).
> +показать, какая спецификация соответствует каждому из файлов.
>  .TP
>  .BI \-e \ directory
>  исключить каталог (чтобы исключить более одного каталога, этот параметр необходимо использовать соответствующее количество раз).
> diff --git a/policycoreutils/setfiles/setfiles.8 b/policycoreutils/setfiles/setfiles.8
> index e328a5628682..4d28bc9a95c1 100644
> --- a/policycoreutils/setfiles/setfiles.8
> +++ b/policycoreutils/setfiles/setfiles.8
> @@ -57,8 +57,7 @@ option will force a replacement of the entire context.
>  check the validity of the contexts against the specified binary policy.
>  .TP
>  .B \-d
> -show what specification matched each file (do not abort validation
> -after ABORT_ON_ERRORS errors).
> +show what specification matched each file.
>  .TP
>  .BI \-e \ directory
>  directory to exclude (repeat option for more than one directory).
> diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
> index 10692d6d94a0..92616571ef2a 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) {
> @@ -97,7 +80,6 @@ int canoncon(char **contextp)
>  		*contextp = tmpcon;
>  	} else if (errno != ENOENT) {
>  		rc = -1;
> -		inc_err();
>  	}
>  
>  	return rc;
> -- 
> 2.30.0


If there's no objection I'd like to merge both patches before Wednesday
so they'll part of rc2.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] setfiles: drop ABORT_ON_ERRORS and related code
       [not found]     ` <CAJfZ7=my52AG+zYMjXJFoxAAHsnJTcs8Y+crbcQ==rT2cWZ-Dg@mail.gmail.com>
@ 2021-02-01 14:05       ` Petr Lautrbach
  0 siblings, 0 replies; 4+ messages in thread
From: Petr Lautrbach @ 2021-02-01 14:05 UTC (permalink / raw)
  To: SElinux list; +Cc: Nicolas Iooss

Nicolas Iooss <nicolas.iooss@m4x.org> writes:

> On Sun, Jan 31, 2021 at 11:39 AM Petr Lautrbach <plautrba@redhat.com> wrote:
>>
>> Petr Lautrbach <plautrba@redhat.com> writes:
>>
>> > `setfiles -d` doesn't have any impact on number of errors before it
>> > aborts. It always aborts on first invalid context in spec file.
>> >
>> > Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
>> > ---
>> >  policycoreutils/setfiles/Makefile      |  3 ---
>> >  policycoreutils/setfiles/ru/setfiles.8 |  2 +-
>> >  policycoreutils/setfiles/setfiles.8    |  3 +--
>> >  policycoreutils/setfiles/setfiles.c    | 18 ------------------
>> >  4 files changed, 2 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/policycoreutils/setfiles/Makefile b/policycoreutils/setfiles/Makefile
>> > index bc5a8db789a5..a3bbbe116b7f 100644
>> > --- a/policycoreutils/setfiles/Makefile
>> > +++ b/policycoreutils/setfiles/Makefile
>> > @@ -5,8 +5,6 @@ SBINDIR ?= /sbin
>> >  MANDIR = $(PREFIX)/share/man
>> >  AUDITH ?= $(shell test -f /usr/include/libaudit.h && echo y)
>> >
>> > -ABORT_ON_ERRORS=$(shell grep "^\#define ABORT_ON_ERRORS" setfiles.c | awk -S '{ print $$3 }')
>> > -
>> >  CFLAGS ?= -g -Werror -Wall -W
>> >  override LDLIBS += -lselinux -lsepol
>> >
>> > @@ -26,7 +24,6 @@ restorecon_xattr: restorecon_xattr.o restore.o
>> >
>> >  man:
>> >       @cp -af setfiles.8 setfiles.8.man
>> > -     @sed -i "s/ABORT_ON_ERRORS/$(ABORT_ON_ERRORS)/g" setfiles.8.man
>> >
>> >  install: all
>> >       [ -d $(DESTDIR)$(MANDIR)/man8 ] || mkdir -p $(DESTDIR)$(MANDIR)/man8
>> > diff --git a/policycoreutils/setfiles/ru/setfiles.8 b/policycoreutils/setfiles/ru/setfiles.8
>> > index 27815a3f1eee..910101452625 100644
>> > --- a/policycoreutils/setfiles/ru/setfiles.8
>> > +++ b/policycoreutils/setfiles/ru/setfiles.8
>> > @@ -47,7 +47,7 @@ setfiles \- установить SELinux-контексты безопаснос
>> >  проверить действительность контекстов относительно указанной двоичной политики.
>> >  .TP
>> >  .B \-d
>> > -показать, какая спецификация соответствует каждому из файлов (не прекращать проверку после получения ошибок ABORT_ON_ERRORS).
>> > +показать, какая спецификация соответствует каждому из файлов.
>> >  .TP
>> >  .BI \-e \ directory
>> >  исключить каталог (чтобы исключить более одного каталога, этот параметр необходимо использовать соответствующее количество раз).
>> > diff --git a/policycoreutils/setfiles/setfiles.8 b/policycoreutils/setfiles/setfiles.8
>> > index e328a5628682..4d28bc9a95c1 100644
>> > --- a/policycoreutils/setfiles/setfiles.8
>> > +++ b/policycoreutils/setfiles/setfiles.8
>> > @@ -57,8 +57,7 @@ option will force a replacement of the entire context.
>> >  check the validity of the contexts against the specified binary policy.
>> >  .TP
>> >  .B \-d
>> > -show what specification matched each file (do not abort validation
>> > -after ABORT_ON_ERRORS errors).
>> > +show what specification matched each file.
>> >  .TP
>> >  .BI \-e \ directory
>> >  directory to exclude (repeat option for more than one directory).
>> > diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c
>> > index 10692d6d94a0..92616571ef2a 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) {
>> > @@ -97,7 +80,6 @@ int canoncon(char **contextp)
>> >               *contextp = tmpcon;
>> >       } else if (errno != ENOENT) {
>> >               rc = -1;
>> > -             inc_err();
>> >       }
>> >
>> >       return rc;
>> > --
>> > 2.30.0
>>
>>
>> If there's no objection I'd like to merge both patches before Wednesday
>> so they'll part of rc2.
>
> I took a look and both patches look good to me. Nevertheless
> policycoreutils/setfiles/setfiles.c stil contains a "static int nerr;"
> which becomes unused, after this patch. This variable should probably
> be dropped, for example with:
>
> diff --git a/policycoreutils/setfiles/setfiles.c
> b/policycoreutils/setfiles/setfiles.c
> index 92616571ef2a..f018d161aa9e 100644
> --- a/policycoreutils/setfiles/setfiles.c
> +++ b/policycoreutils/setfiles/setfiles.c
> @@ -19,7 +19,6 @@ static int warn_no_match;
>  static int null_terminated;
>  static int request_digest;
>  static struct restore_opts r_opts;
> -static int nerr;
>
>  #define STAT_BLOCK_SIZE 1
>
> @@ -161,7 +160,6 @@ int main(int argc, char **argv)
>   warn_no_match = 0;
>   request_digest = 0;
>   policyfile = NULL;
> - nerr = 0;
>
>   r_opts.abort_on_error = 0;
>   r_opts.progname = strdup(argv[0]);
> @@ -427,9 +425,6 @@ int main(int argc, char **argv)
>   r_opts.selabel_opt_digest = (request_digest ? (char *)1 : NULL);
>   r_opts.selabel_opt_path = altpath;
>
> - if (nerr)
> - exit(-1);
> -
>   restore_init(&r_opts);
>
>   if (use_input_file) {
>
> This clean-up could be done after you merged the patches. So:
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>


Merged.


> Thanks!
> Nicolas


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-02-01 14:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 21:09 [PATCH v2 1/2] setfiles: Do not abort on labeling error Petr Lautrbach
2021-01-13 21:09 ` [PATCH v2 2/2] setfiles: drop ABORT_ON_ERRORS and related code Petr Lautrbach
2021-01-31 10:27   ` Petr Lautrbach
     [not found]     ` <CAJfZ7=my52AG+zYMjXJFoxAAHsnJTcs8Y+crbcQ==rT2cWZ-Dg@mail.gmail.com>
2021-02-01 14:05       ` Petr Lautrbach

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).