> On Friday, 20 May 2016, 18:01, Stephen Smalley <[email protected]> wrote:
> > On 05/10/2016 11:23 AM, Richard Haines wrote:
>> Add additional error handling, flags, xdev and alt_rootpath
>> support for setfiles(8) functionality.
>>
>> Signed-off-by: Richard Haines <[email protected]>
>> ---
>> libselinux/include/selinux/restorecon.h | 17 +++
>> libselinux/man/man3/selinux_restorecon.3 | 13 +++
>> .../man/man3/selinux_restorecon_set_alt_rootpath.3 | 34 ++++++
>> libselinux/src/selinux_restorecon.c | 126
> +++++++++++++++++----
>> libselinux/utils/selinux_restorecon.c | 28 ++++-
>> 5 files changed, 197 insertions(+), 21 deletions(-)
>> create mode 100644
> libselinux/man/man3/selinux_restorecon_set_alt_rootpath.3
>>
>> diff --git a/libselinux/include/selinux/restorecon.h
> b/libselinux/include/selinux/restorecon.h
>> index 0b93b0c..6b3e0f1 100644
>> --- a/libselinux/include/selinux/restorecon.h
>> +++ b/libselinux/include/selinux/restorecon.h
>> @@ -50,6 +50,14 @@ extern int selinux_restorecon(const char *pathname,
>> * If there is a different context that matched the inode,
>> * then use the first context that matched. */
>> #define SELINUX_RESTORECON_ADD_ASSOC 256
>> +/* Abort on errors during the file tree walk. */
>> +#define SELINUX_RESTORECON_ABORT_ON_ERROR 512
>> +/* Log any label changes to syslog. */
>> +#define SELINUX_RESTORECON_SYSLOG_CHANGES 1024
>> +/* Log what spec matched each file. */
>> +#define SELINUX_RESTORECON_LOG_MATCHES 2048
>> +/* Ignore files that do not exist. */
>> +#define SELINUX_RESTORECON_IGNORE_NOENTRY 4096
>
> Maybe we should express these as hex values since they are flags.
Fixed.
>
>> diff --git a/libselinux/man/man3/selinux_restorecon_set_alt_rootpath.3
> b/libselinux/man/man3/selinux_restorecon_set_alt_rootpath.3
>> new file mode 100644
>> index 0000000..79223eb
>> --- /dev/null
>> +++ b/libselinux/man/man3/selinux_restorecon_set_alt_rootpath.3
>> @@ -0,0 +1,34 @@
>> +.TH "selinux_restorecon_set_alt_rootpath" "3" "28
> April 2016" "Security Enhanced Linux" "SELinux API
> documentation"
>> +
>> +.SH "NAME"
>> +selinux_restorecon_set_alt_rootpath \- set an alternate rootpath.
>> +.
>> +.SH "SYNOPSIS"
>> +.B #include <selinux/restorecon.h>
>> +.sp
>> +.BI "void selinux_restorecon_set_alt_rootpath(const char *"
> alt_rootpath ");"
>> +.in +\w'void selinux_restorecon_set_alt_rootpath('u
>> +.
>> +.SH "DESCRIPTION"
>> +.BR selinux_restorecon_set_alt_rootpath ()
>> +passes to
>> +.BR selinux_restorecon (3)
>> +a pointer containing an alternate rootpath
>> +.IR alt_rootpath .
>> +.br
>> +The path MUST NOT be terminated with a trailing '/'.
>> +.br
>> +The path MUST NOT be '/' (i.e. the root path).
>
> Any particular reason we can't just handle this in the implementation?
Used code from setfiles.c to handle this now.
>
>> diff --git a/libselinux/src/selinux_restorecon.c
> b/libselinux/src/selinux_restorecon.c
>> index 2794659..11ae8a0 100644
>> --- a/libselinux/src/selinux_restorecon.c
>> +++ b/libselinux/src/selinux_restorecon.c
>> @@ -22,6 +22,7 @@
>> #include <sys/vfs.h>
>> #include <linux/magic.h>
>> #include <libgen.h>
>> +#include <syslog.h>
>> #include <selinux/selinux.h>
>> #include <selinux/context.h>
>> #include <selinux/label.h>
>> @@ -39,6 +40,8 @@ static struct selabel_handle *fc_sehandle = NULL;
>> static unsigned char *fc_digest = NULL;
>> static size_t fc_digest_len = 0;
>> static const char **fc_exclude_list = NULL;
>> +static const char *rootpath = NULL;
>> +static int rootpathlen;
>> static size_t fc_count = 0;
>> #define STAR_COUNT 1000
>>
>> @@ -47,12 +50,16 @@ struct rest_flags {
>> bool nochange;
>> bool verbose;
>> bool progress;
>> - bool specctx;
>> + bool set_specctx;
>> bool add_assoc;
>> - bool ignore;
>> + bool ignore_digest;
>> bool recurse;
>> bool userealpath;
>> bool xdev;
>> + bool abort_on_error;
>> + bool syslog_changes;
>> + bool log_matches;
>> + bool ignore_enoent;
>> };
>>
>> static void restorecon_init(void)
>> @@ -67,13 +74,15 @@ static void restorecon_init(void)
>>
>> static pthread_once_t fc_once = PTHREAD_ONCE_INIT;
>>
>> -
>> static int check_excluded(const char *file)
>> {
>> int i;
>> + size_t len;
>>
>> for (i = 0; fc_exclude_list[i]; i++) {
>> - if (strcmp(file, fc_exclude_list[i]) == 0)
>> + len = strlen(fc_exclude_list[i]);
>> + /* Check if 'file' is in an excluded directory. */
>> + if (strncmp(file, fc_exclude_list[i], len) == 0)
>> return 1;
>
> Simple prefix match could give you a false positive, e.g. if /foo/bar is
> on the exclude list and there is a file /foo/barfoo.
> Any particular reason you aren't using setfiles restore.c exclude() logic?
> However, is it even possible to reach /foo/bar/baz if we are checking
> the exclude list on each component and pruning the tree walk on a match
> of /foo/bar?
Adding exclude_non_seclabel_mounts(), add_exclude() and remove_exclude()
to selinux_restorecon to handle this.
>
>
>> }
>> return 0;
>> @@ -360,10 +369,29 @@ static int restorecon_sb(const char *pathname, const
> struct stat *sb,
>> char *newcon = NULL;
>> char *curcon = NULL;
>> char *newtypecon = NULL;
>> - int rc = 0;
>> + int rc;
>> bool updated = false;
>> + const char *tmp_pathname = pathname;
>
> Maybe call it lookup_path or something more descriptive.
Fixed.
>
>> +
>> + if (rootpath) {
>> + if (strncmp(rootpath, tmp_pathname, rootpathlen) != 0) {
>> + selinux_log(SELINUX_ERROR,
>> + "%s is not located in alt_rootpath
> %s\n",
>> + tmp_pathname, rootpath);
>> + return -1;
>> + }
>> + tmp_pathname += rootpathlen;
>> + }
>>
>> - if (selabel_lookup_raw(fc_sehandle, &newcon, pathname,
> sb->st_mode) < 0)
>> + if (rootpath != NULL && tmp_pathname[0] == '\0')
>> + /* this is actually the root dir of the alt root. */
>> + rc = selabel_lookup_raw(fc_sehandle, &newcon, "/",
>> + sb->st_mode);
>> + else
>> + rc = selabel_lookup_raw(fc_sehandle, &newcon, tmp_pathname,
>> + sb->st_mode);
>> +
>> + if (rc < 0)
>> return 0; /* no match, but not an error */
>>
>> if (flags->add_assoc) {
>
>
>> @@ -436,6 +468,16 @@ static int restorecon_sb(const char *pathname, const
> struct stat *sb,
>> "%s %s from %s to %s\n",
>> updated ? "Relabeled" : "Would
> relabel",
>> pathname, curcon, newcon);
>> +
>> + if (flags->syslog_changes && !flags->nochange)
> {
>> + if (curcon)
>> + syslog(LOG_INFO,
>> + "relabeling %s from %s to %s\n",
>> + pathname, curcon, newcon);
>> + else
>> + syslog(LOG_INFO, "labeling %s to %s\n",
>> + pathname, newcon);
>> + }
>
> Wondering if this could be handled by the caller just specifying a log
> callback that calls syslog rather than doing it here, but probably would
> conflict with other logging.
I've left this as is at present as I would need time to experiment with
callbacks. Could review later ???
>
>> @@ -589,13 +649,22 @@ int selinux_restorecon(const char *pathname_orig,
>> fts_flags = FTS_PHYSICAL | FTS_NOCHDIR;
>>
>> fts = fts_open(paths, fts_flags, NULL);
>> - if (!fts) {
>> - error = -1;
>> - goto cleanup;
>> - }
>> + if (!fts)
>> + goto fts_err;
>> +
>> + ftsent = fts_read(fts);
>> + if (!ftsent)
>> + goto fts_err;
>> +
>> + /* Keep the inode of the first device. */
>> + dev_num = ftsent->fts_statp->st_dev;
>>
>> error = 0;
>> - while ((ftsent = fts_read(fts)) != NULL) {
>> + do {
>> + /* If the XDEV flag is set and the device is different */
>> + if (flags.xdev && ftsent->fts_statp->st_dev !=
> dev_num)
>> + continue;
>
> Why is this necessary given that we already set FTS_XDEV above?
After much testing I found this did not work as I expected so implemented
the current setfiles logic that works. I've now found the original
patch from http://marc.info/?l=selinux&m=124688830500777&w=2 and added the
following text from this to the updated code:
/*
* Keep the inode of the first device. This is because the FTS_XDEV
* flag tells fts not to descend into directories with different
* device numbers, but fts will still give back the actual directory.
* By saving the device number of the directory that was passed to
* selinux_restorecon() and then skipping all actions on any
* directories with a different device number when the FTS_XDEV flag
* is set (from http://marc.info/?l=selinux&m=124688830500777&w=2).
*/
>
>> +
>> switch (ftsent->fts_info) {
>> case FTS_DC:
>> selinux_log(SELINUX_ERROR,
>> @@ -644,9 +713,12 @@ int selinux_restorecon(const char *pathname_orig,
>>
>> error |= restorecon_sb(ftsent->fts_path,
>> ftsent->fts_statp, &flags);
>> +
>> + if (error && flags.abort_on_error)
>> + goto out;
>> break;
>> }
>> - }
>> + } while ((ftsent = fts_read(fts)) != NULL);
>>
>> /* Labeling successful. Mark the top level directory as completed. */
>> if (setrestoreconlast && !flags.nochange && !error)
> {
>> @@ -672,12 +744,14 @@ cleanup:
>> free(pathname);
>> free(xattr_value);
>> return error;
>> +
>> oom:
>> sverrno = errno;
>> selinux_log(SELINUX_ERROR, "%s: Out of memory\n",
> __func__);
>> errno = sverrno;
>> error = -1;
>> goto cleanup;
>> +
>> realpatherr:
>> sverrno = errno;
>> selinux_log(SELINUX_ERROR,
>> @@ -686,6 +760,13 @@ realpatherr:
>> errno = sverrno;
>> error = -1;
>> goto cleanup;
>> +
>> +fts_err:
>> + selinux_log(SELINUX_ERROR,
>> + "fts error while labeling %s: %s\n",
>> + paths[0], strerror(errno));
>> + error = -1;
>> + goto cleanup;
>> }
>>
>> /* selinux_restorecon_set_sehandle(3) is called to set the global fc
> handle */
>> @@ -757,3 +838,10 @@ void selinux_restorecon_set_exclude_list(const char
> **exclude_list)
>> {
>> fc_exclude_list = exclude_list;
>> }
>> +
>> +/* selinux_restorecon_set_alt_rootpath(3) sets an alternate rootpath. */
>> +void selinux_restorecon_set_alt_rootpath(const char *alt_rootpath)
>> +{
>> + rootpath = alt_rootpath;
>
> Should we be strdup()'ing this in case the caller is passing a local
> variable or other transient memory that won't stay around necessarily
> for later restorecon() calls?
Used code from setfiles.c to handle this now that dups entry.
>
>
>> + rootpathlen = strlen(rootpath);
>> +}
>> diff --git a/libselinux/utils/selinux_restorecon.c
> b/libselinux/utils/selinux_restorecon.c
>> index 2552d63..da59fcd 100644
>> --- a/libselinux/utils/selinux_restorecon.c
>> +++ b/libselinux/utils/selinux_restorecon.c
>> @@ -37,7 +37,7 @@ static int validate_context(char **contextp)
>> static void usage(const char *progname)
>> {
>> fprintf(stderr,
>> - "\nusage: %s [-FCnRrdeia] [-v|-P] [-p policy] [-f
> specfile] "
>> + "\nusage: %s [-FCnRrdeiIaAsl] [-v|-P] [-x alt_rootpath]
> [-p policy] [-f specfile] "
>> "pathname ...\n"
>> "Where:\n\t"
>> "-F Set the label to that in specfile.\n\t"
>> @@ -57,9 +57,14 @@ static void usage(const char *progname)
>> "-e Exclude this file/directory (add multiple -e
> entries).\n\t"
>> "-i Do not set SELABEL_OPT_DIGEST option when calling "
>> " selabel_open(3).\n\t"
>> + "-I Ignore files that do not exist.\n\t"
>> "-a Add an association between an inode and a
> context.\n\t"
>> " If there is a different context that matched the
> inode,\n\t"
>> " then use the first context that
> matched.\n\t"
>> + "-A Abort on errors during the file tree
> walk.\n\t"
>> + "-s Log any label changes to syslog(3).\n\t"
>> + "-l Log what specfile context matched each
> file.\n\t"
>> + "-x Set alternate rootpath.\n\t"
>> "-p Optional binary policy file (also sets validate context
> "
>> "option).\n\t"
>> "-f Optional file contexts file.\n\t"
>> @@ -101,6 +106,7 @@ int main(int argc, char **argv)
>> int opt, i;
>> unsigned int restorecon_flags = 0;
>> char *path = NULL, *digest = NULL, *validate = NULL;
>> + char *alt_rootpath = NULL;
>> FILE *policystream;
>> bool ignore_digest = false, require_selinux = true;
>> bool verbose = false, progress = false;
>> @@ -118,7 +124,7 @@ int main(int argc, char **argv)
>> exclude_list = NULL;
>> exclude_count = 0;
>>
>> - while ((opt = getopt(argc, argv, "iFCnRvPrdae:f:p:")) >
> 0) {
>> + while ((opt = getopt(argc, argv, "iIFCnRvPrdaAsle:f:p:x:"))
>> 0) {
>> switch (opt) {
>> case 'F':
>> restorecon_flags |=
>> @@ -190,9 +196,24 @@ int main(int argc, char **argv)
>> case 'i':
>> ignore_digest = true;
>> break;
>> + case 'I':
>> + restorecon_flags |= SELINUX_RESTORECON_IGNORE_NOENTRY;
>> + break;
>> case 'a':
>> restorecon_flags |= SELINUX_RESTORECON_ADD_ASSOC;
>> break;
>> + case 'A':
>> + restorecon_flags |= SELINUX_RESTORECON_ABORT_ON_ERROR;
>> + break;
>> + case 's':
>> + restorecon_flags |= SELINUX_RESTORECON_SYSLOG_CHANGES;
>> + break;
>> + case 'l':
>> + restorecon_flags |= SELINUX_RESTORECON_LOG_MATCHES;
>> + break;
>> + case 'x':
>> + alt_rootpath = optarg;
>> + break;
>> default:
>> usage(argv[0]);
>> }
>> @@ -247,6 +268,9 @@ int main(int argc, char **argv)
>> selinux_restorecon_set_exclude_list
>> ((const char **)exclude_list);
>>
>> + if (alt_rootpath)
>> + selinux_restorecon_set_alt_rootpath(alt_rootpath);
>> +
>> /* Call restorecon for each path in list */
>> for (i = optind; i < argc; i++) {
>> if (selinux_restorecon(argv[i], restorecon_flags) < 0) {
>>
>
_______________________________________________
Selinux mailing list
[email protected]
To unsubscribe, send email to [email protected].
To get help, send an email containing "help" to [email protected].
