On 12/04/2013 05:49 PM, Daniel J Walsh wrote: > On 12/04/2013 11:11 AM, Pádraig Brady wrote: >> Before I pull the trigger on this release, I'd like to confirm a change you >> did. > >> You changed `cp --context=CTX` to _not fail_ if selinux is disabled. I'm >> thinking that if the old behavior of giving a specific context is not >> supported, then we should fail? > I have no problem if this fails, since the user was so explicit. My real goal > is to allow people to put commands in init scripts and install post install > scripts or any other scripts that do not need to check if SELinux is enabled. > > cp -Z foobar /etc > > Should always work. > >> Also I'm wondering about the -Z case with selinux disabled. I.E. would >> defaultcon() and/or restorecon() support setting file contexts even if >> selinux is currently disabled? I.E. should we attempt those even if selinux >> is disabled, but suppress any associated warnings/errors? > >> thanks, Pádraig. > > When a machine comes back from being disabled it will require a full relabel > to work properly whether or not these commands work. Theoretically restorecon > should work, but defaultcon will not.
Great thanks for the info. I'll probably address this with the attached patch. thanks, Pádraig.
>From 645cea3d6a284e59fdc2aeb94165bef351dc88db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Wed, 4 Dec 2013 19:10:37 +0000 Subject: [PATCH] selinux: ensure --context=CTX fails when not supported Restore previous behavior to fail when there is any error setting a user specified context. Also change all defaultcon() to warn, but only for unexpected errors, rather than if the (file) system doesn't support the operation. * src/selinux.h (ignorable_ctx_err): A new function used to determine if a warning should be given after a call to defaultcon() or restorecon(). * src/cp.c (main): Fail with --context=CTX even with SELinux disabled. * src/mknod.c (main): Likewise. * src/mkfifo.c (main): Likewise. * src/mv.c (main): Likewise. * src/mkdir.c (main): Likewise. (make_ancestor); Don't show "ignoreable" errors from defaultcon() Fix the return code check from restorecon(). --- src/copy.c | 10 +++++----- src/cp.c | 19 ++++--------------- src/mkdir.c | 21 +++++++++------------ src/mkfifo.c | 10 ++-------- src/mknod.c | 10 ++-------- src/mv.c | 3 ++- src/selinux.h | 8 ++++++++ 7 files changed, 32 insertions(+), 49 deletions(-) diff --git a/src/copy.c b/src/copy.c index dab8fdd..0f044d0 100644 --- a/src/copy.c +++ b/src/copy.c @@ -805,12 +805,12 @@ set_process_security_ctx (char const *src_name, char const *dst_name, { /* With -Z, adjust the default context for the process to have the type component adjusted as per the destination path. */ - if (new_dst && defaultcon (dst_name, mode) < 0) + if (new_dst && defaultcon (dst_name, mode) < 0 + && ! ignorable_ctx_err (errno)) { - if (!errno_unsupported (errno)) - error (0, errno, - _("failed to set default file creation context for %s"), - quote (dst_name)); + error (0, errno, + _("failed to set default file creation context for %s"), + quote (dst_name)); } } diff --git a/src/cp.c b/src/cp.c index 59d659d..7d3e2bc 100644 --- a/src/cp.c +++ b/src/cp.c @@ -1099,22 +1099,11 @@ main (int argc, char **argv) x.one_file_system = true; break; - case 'Z': - /* politely decline if we're not on a selinux-enabled kernel. */ - if (selinux_enabled) - { - if (optarg) - scontext = optarg; - else - x.set_security_context = true; - } - else if (optarg) - { - error (0, 0, - _("warning: ignoring --context; " - "it requires an SELinux-enabled kernel")); - } + if (optarg) + scontext = optarg; + else if (selinux_enabled) + x.set_security_context = true; break; case 'S': diff --git a/src/mkdir.c b/src/mkdir.c index 25b1da5..0281081 100644 --- a/src/mkdir.c +++ b/src/mkdir.c @@ -118,7 +118,8 @@ make_ancestor (char const *dir, char const *component, void *options) { struct mkdir_options const *o = options; - if (o->set_security_context && defaultcon (dir, S_IFDIR) < 0) + if (o->set_security_context && defaultcon (dir, S_IFDIR) < 0 + && ! ignorable_ctx_err (errno)) error (0, errno, _("failed to set default creation context for %s"), quote (dir)); @@ -162,7 +163,8 @@ process_dir (char *dir, struct savewd *wd, void *options) set_defaultcon = true; free (pdir); } - if (set_defaultcon && defaultcon (dir, S_IFDIR) < 0) + if (set_defaultcon && defaultcon (dir, S_IFDIR) < 0 + && ! ignorable_ctx_err (errno)) error (0, errno, _("failed to set default creation context for %s"), quote (dir)); } @@ -180,8 +182,9 @@ process_dir (char *dir, struct savewd *wd, void *options) and here we set the context for the final component. */ if (ret == EXIT_SUCCESS && o->set_security_context && ! set_defaultcon) { - if (restorecon (last_component (dir), false, false) < 0) - error (0, errno, _("failed to set restore context for %s"), + if (! restorecon (last_component (dir), false, false) + && ! ignorable_ctx_err (errno)) + error (0, errno, _("failed to restore context for %s"), quote (dir)); } @@ -229,19 +232,13 @@ main (int argc, char **argv) /* We don't yet support -Z to restore context with SMACK. */ scontext = optarg; } - else if (is_selinux_enabled () > 0) + else { if (optarg) scontext = optarg; - else + else if (is_selinux_enabled () > 0) options.set_security_context = true; } - else if (optarg) - { - error (0, 0, - _("warning: ignoring --context; " - "it requires an SELinux/SMACK-enabled kernel")); - } break; case_GETOPT_HELP_CHAR; case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS); diff --git a/src/mkfifo.c b/src/mkfifo.c index 2428dd0..d10ef94 100644 --- a/src/mkfifo.c +++ b/src/mkfifo.c @@ -102,19 +102,13 @@ main (int argc, char **argv) /* We don't yet support -Z to restore context with SMACK. */ scontext = optarg; } - else if (is_selinux_enabled () > 0) + else { if (optarg) scontext = optarg; - else + else if (is_selinux_enabled () > 0) set_security_context = true; } - else if (optarg) - { - error (0, 0, - _("warning: ignoring --context; " - "it requires an SELinux/SMACK-enabled kernel")); - } break; case_GETOPT_HELP_CHAR; case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS); diff --git a/src/mknod.c b/src/mknod.c index 7856e51..cd0815f 100644 --- a/src/mknod.c +++ b/src/mknod.c @@ -119,19 +119,13 @@ main (int argc, char **argv) /* We don't yet support -Z to restore context with SMACK. */ scontext = optarg; } - else if (is_selinux_enabled () > 0) + else { if (optarg) scontext = optarg; - else + else if (is_selinux_enabled () > 0) set_security_context = true; } - else if (optarg) - { - error (0, 0, - _("warning: ignoring --context; " - "it requires an SELinux/SMACK-enabled kernel")); - } break; case_GETOPT_HELP_CHAR; case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS); diff --git a/src/mv.c b/src/mv.c index cb6b70e..b69b355 100644 --- a/src/mv.c +++ b/src/mv.c @@ -424,7 +424,8 @@ main (int argc, char **argv) backup_suffix_string = optarg; break; case 'Z': - /* politely decline if we're not on a selinux-enabled kernel. */ + /* As a performance enhancement, don't even bother trying + to "restorecon" when not on an selinux-enabled kernel. */ if (selinux_enabled) { x.preserve_security_context = false; diff --git a/src/selinux.h b/src/selinux.h index 3177163..e4ded84 100644 --- a/src/selinux.h +++ b/src/selinux.h @@ -19,6 +19,14 @@ #ifndef COREUTILS_SELINUX_H # define COREUTILS_SELINUX_H +/* Return true if ERR corresponds to an unsupported request, + or if there is no context or it's inaccessible. */ +static inline bool +ignorable_ctx_err (int err) +{ + return err == ENOTSUP || err == ENODATA; +} + # if HAVE_SELINUX_SELINUX_H extern bool restorecon (char const *path, bool recurse, bool preserve); -- 1.7.7.6
