| Message ID | 20200701150627.1643297-1-antoine.tenart@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | policycoreutils: setfiles: do not restrict checks against a binary policy | expand |
On Wed, Jul 1, 2020 at 11:13 AM Antoine Tenart <antoine.tenart@bootlin.com> wrote: > > The -c option allows to check the validity of contexts against a > specified binary policy. Its use is restricted: no pathname can be used > when a binary policy is given to setfiles. It's not clear if this is > intentional as the built-in help and the man page are not stating the > same thing about this (the man page document -c as a normal option, > while the built-in help shows it is restricted). > > When generating full system images later used with SELinux in enforcing > mode, the extended attributed of files have to be set by the build > machine. The issue is setfiles always checks the contexts against a > policy (ctx_validate = 1) and using an external binary policy is not > currently possible when using a pathname. This ends up in setfiles > failing early as the contexts of the target image are not always > compatible with the ones of the build machine. > > This patch reworks a check on optind only made when -c is used, that > enforced the use of a single argument to allow 1+ arguments, allowing to > use setfiles with an external binary policy and pathnames. The following > command is then allowed, as already documented in the man page: > > $ setfiles -m -r target/ -c policy.32 file_contexts target/ > > Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com> > --- > policycoreutils/setfiles/setfiles.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c > index ed67b5f12245..4844875cfc6b 100644 > --- a/policycoreutils/setfiles/setfiles.c > +++ b/policycoreutils/setfiles/setfiles.c > @@ -48,10 +48,9 @@ static __attribute__((__noreturn__)) void usage(const char *const name) > name, name); > } else { > fprintf(stderr, > - "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] spec_file pathname...\n" > - "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] spec_file -f filename\n" > - "usage: %s -s [-diIDlmnpqvFW] spec_file\n" > - "usage: %s -c policyfile spec_file\n", > + "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file pathname...\n" > + "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file -f filename\n" > + "usage: %s -s [-diIDlmnpqvFW] spec_file\n", > name, name, name, name); You didn't update the number of name arguments to reflect the format string change above. make[2]: Entering directory '/home/travis/build/stephensmalley/selinux/policycoreutils/setfiles' clang -O2 -Werror -Wall -Wextra -Wmissing-format-attribute -Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes -Wundef -Wunused -Wwrite-strings -fno-common -I/home/travis/build/stephensmalley/selinux/installdir/usr/include -DUSE_AUDIT -c -o setfiles.o setfiles.c setfiles.c:54:22: error: data argument not used by format string [-Werror,-Wformat-extra-args] name, name, name, name); ^^
Hello Stephen, Quoting Stephen Smalley (2020-07-07 14:26:59) > On Wed, Jul 1, 2020 at 11:13 AM Antoine Tenart > <antoine.tenart@bootlin.com> wrote: > > diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c > > index ed67b5f12245..4844875cfc6b 100644 > > --- a/policycoreutils/setfiles/setfiles.c > > +++ b/policycoreutils/setfiles/setfiles.c > > @@ -48,10 +48,9 @@ static __attribute__((__noreturn__)) void usage(const char *const name) > > name, name); > > } else { > > fprintf(stderr, > > - "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] spec_file pathname...\n" > > - "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] spec_file -f filename\n" > > - "usage: %s -s [-diIDlmnpqvFW] spec_file\n" > > - "usage: %s -c policyfile spec_file\n", > > + "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file pathname...\n" > > + "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file -f filename\n" > > + "usage: %s -s [-diIDlmnpqvFW] spec_file\n", > > name, name, name, name); > > You didn't update the number of name arguments to reflect the format > string change above. Sorry for that, I'll fix it for v2! Thanks, Antoine > make[2]: Entering directory > '/home/travis/build/stephensmalley/selinux/policycoreutils/setfiles' > clang -O2 -Werror -Wall -Wextra -Wmissing-format-attribute > -Wmissing-noreturn -Wpointer-arith -Wshadow -Wstrict-prototypes > -Wundef -Wunused -Wwrite-strings -fno-common > -I/home/travis/build/stephensmalley/selinux/installdir/usr/include > -DUSE_AUDIT -c -o setfiles.o setfiles.c > setfiles.c:54:22: error: data argument not used by format string > [-Werror,-Wformat-extra-args] > name, name, name, name); > ^^
diff --git a/policycoreutils/setfiles/setfiles.c b/policycoreutils/setfiles/setfiles.c index ed67b5f12245..4844875cfc6b 100644 --- a/policycoreutils/setfiles/setfiles.c +++ b/policycoreutils/setfiles/setfiles.c @@ -48,10 +48,9 @@ static __attribute__((__noreturn__)) void usage(const char *const name) name, name); } else { fprintf(stderr, - "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] spec_file pathname...\n" - "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] spec_file -f filename\n" - "usage: %s -s [-diIDlmnpqvFW] spec_file\n" - "usage: %s -c policyfile spec_file\n", + "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file pathname...\n" + "usage: %s [-diIDlmnpqvEFW] [-e excludedir] [-r alt_root_path] [-c policyfile] spec_file -f filename\n" + "usage: %s -s [-diIDlmnpqvFW] spec_file\n", name, name, name, name); } exit(-1); @@ -409,7 +408,7 @@ int main(int argc, char **argv) if (!iamrestorecon) { if (policyfile) { - if (optind != (argc - 1)) + if (optind > (argc - 1)) usage(argv[0]); } else if (use_input_file) { if (optind != (argc - 1)) {
The -c option allows to check the validity of contexts against a specified binary policy. Its use is restricted: no pathname can be used when a binary policy is given to setfiles. It's not clear if this is intentional as the built-in help and the man page are not stating the same thing about this (the man page document -c as a normal option, while the built-in help shows it is restricted). When generating full system images later used with SELinux in enforcing mode, the extended attributed of files have to be set by the build machine. The issue is setfiles always checks the contexts against a policy (ctx_validate = 1) and using an external binary policy is not currently possible when using a pathname. This ends up in setfiles failing early as the contexts of the target image are not always compatible with the ones of the build machine. This patch reworks a check on optind only made when -c is used, that enforced the use of a single argument to allow 1+ arguments, allowing to use setfiles with an external binary policy and pathnames. The following command is then allowed, as already documented in the man page: $ setfiles -m -r target/ -c policy.32 file_contexts target/ Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com> --- policycoreutils/setfiles/setfiles.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)