Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Adds support for selinux's rootcontext mount option #1835

Closed
wants to merge 1 commit into from

4 participants

@prometheanfire

The validation check is not necessary, an invalid check is caught and the mount is denied (as it should be) already.

@behlendorf
Owner

@prometheanfire @ryao I think we want to implement a more comprehensive solution for selinux contexts. So rather than adding a specific 'rootcontext' property perhaps a 'selinux' property as you described in #1504 is the way to go. We could then set selinux="system_u:object_r:fs_t" by default. This would allow us to shed the existing code in HAVE_LIBSELINUX and allow end users to cleanly override the default. A completely generic way to pass persistent mount options is another alternative.

@prometheanfire

the problem is that there are multiple context types, rootcontext being one of them. If you are making rootcontext into selinux (making it generic) then it should be an actual generic option.

genericmountoptions="rootcontext=system_u:object_r:fs_t ro"
@prometheanfire

this is not complete, I cannot change the context of symlinks still

@prometheanfire

ignore that, was doing stupid :D

@behlendorf
Owner

@prometheanfire What I don't like about the current implementation is that it's specific to this one selinux issue. It hides hard coded selinux policy mount options in the binary and doesn't provide a smart user a way to override them to do the right thing on their system. (Yes, I did the same thing but I'll be the first to admit it was a hack and it needs to be fixed).

It would be better to do something more flexible and allow the user to pass the right mount options. My inclination was to add a generic selinux property to store these options which could be validated. Alternately as you pointed out we could just add a completely generic property for arbitrary mount options. Either of these I think would be preferable.

Can you rework this patch to implement the generic mount options?

@prometheanfire

The problem is that selinux mount options are not easily parsed. A generic (unverified) mount option might work, but I feel that this mount option should be allowed to be added as is, as it's important enough to be separate.

@behlendorf
Owner

@prometheanfire If we add it as is then we'll just end up ripping it out again when we do the more generic thing. I'd rather avoid that.

@prometheanfire

I feel that the rootcontext mount option should only be used when running selinux. having it be added to the mount command dynamically aids in this.

@sjvermeu

SELinux uses four context options: "context=", "fscontext=", "defcontext=" and "rootcontext=", each with its own purpose. Wouldn't ZFS mainly pass on those options to the Linux kernel (it's the SELinux LSM hooks that take care of all context-related mount options afaik)?

@ryao

@sjvermeu That is the direction in which I think this is headed. @prometheanfire probably just needs to add the additional three properties and pick sane defaults for them (both in terms of the actual setting and how inheritance works). That should make this complete.

@prometheanfire

ok, here's what I'd like to see.

I'd like to see support for all the context options (context fscontext defcontext rootcontext) in the following behavior.

if selinux:
    if context= is set (not NULL):
        then pass that option to mount and ignore the rest.
    else:
        pass the other options, using defaults if available

This assumes we have specific options for each mount option. I am in favor of this because the mount options need to have some logic to them (context= being set).

@prometheanfire
if selinux:
    if xattr == off:
        set context to something, using default if unset
    else if xattr == ( sa || on ):
        if context is set (not NULL):
            then pass the context option to mount and ignore fscontext defcontext rootcontext
        else:
            pass the fscontext defcontext rootcontext options, using defaults if unset
else:
    the context fscontext defcontext rootcontext options are not passed to the mount command
@prometheanfire

defaults are going to be null, the context options are overrides only

@behlendorf
Owner

@prometheanfire Then the proposal would be to add the three missing selinux contexts as their own respective properties. Those could then be passed as part of the mount as you've described above? Presumably having any one of them set would effectively cause the 'selinux == True' case. If I understood correctly then that's a plan I could get on board with. For example:

$ zfs get context,fscontext,defcontext,rootcontext rpool

NAME   PROPERTY         VALUE          SOURCE
rpool  context          none           default
rpool  fscontext        none           default
rpool  defcontext       none           default
rpool  rootcontext      none           default

As part of this change I'd want to make sure we retire the existing hardcoded selinux options. It also goes without saying but we'd also need to clearly describe this behavior in the zfs(8) man page.

@prometheanfire

when xattr=off we don't need to do anything, it seems xattrs still work, confusing...

@prometheanfire

ok, making a todo list here.

1. check if rhel/centos/SL6 support zfs
    1. if they do, remove the old behavior
    2. if they don't we need to make a check for the non-support and keep the pre-patch behavior
It supports it, checked the srpm for rhel6, it's supported in rhel 6.4 and above it looks like


1. break out the context logic into it's own function.
    1. see https://github.com/zfsonlinux/zfs/pull/1835#commitcomment-4618048
2. add validation code
    1. see http://selinuxproject.org/page/LibselinuxAPISummary
    2. specifically the avc_context_to_sid and security_check_context functions.
    3. this may cause us to only be able to set this property when selinux is enabled...
    4. expand on the zfs_valid_proplist function (see ZFS_PROP_MLSLABEL's usage for more info)
3. add a section to the zfs(8) man page to describe the new properties
    1. doesn't need to be much, can refer people to the primary mount manpage
4. squash all the commits
5. use the cstyle.pl script to check the c style
    scripts/cstyle.pl cmd/mount_zfs/mount_zfs.c
@prometheanfire

@behlendorf
do I need to add the context descriptions to the zfs manpage even if they are described in the main 'mount' manpage?

@behlendorf
Owner

@prometheanfire When you're ready for another review on this can you please squash everything together and --force push this branch so there's a single patch to review.

@prometheanfire

An update of my todo

1. break out the context logic into it's own function.
    1. see https://github.com/zfsonlinux/zfs/pull/1835#commitcomment-4618048 
2. add validation code
    1. see http://selinuxproject.org/page/LibselinuxAPISummary
    2. specifically the avc_context_to_sid and security_check_context functions.
    3. this may cause us to only be able to set this property when selinux is enabled...
    4. expand on the zfs_valid_proplist function (see ZFS_PROP_MLSLABEL's usage for more info)
3. squash all the commits
4. use the cstyle.pl script to check the c style
    scripts/cstyle.pl cmd/mount_zfs/mount_zfs.c
@ryao

I went over this with @prometheanfire. It seems that there is no current API to verify the syntax of SELinux contexts on mount points. zfs mount -a will handle a bad context as gracefully as it handles non-empty mount points, so it should be fine to forgo validation until someone either finds an official API or could define a grammar from which we can create a parser to perform validation.

@prometheanfire prometheanfire deleted the prometheanfire:selinux branch
@behlendorf
Owner

Ok, that's reasonable. Thanks for investigating.

@prometheanfire

I cleaned up some more, I'd like some more clarification on prometheanfire@1818ddf#cmd-mount_zfs-mount_zfs-c-P87 and if this is good before I do the final squash.

@prometheanfire

I think we are fully good to go now.

@prometheanfire

ok, now I'm good...

@behlendorf
Owner

@prometheanfire Much better, I'll give this another review and some testing and we should be able to get it in.

@prometheanfire prometheanfire Adding full selinux support
removes old and janky selinux support and adds full support for all of
the selinux context types.
thanks to ryao and behlendorf for the help

Signed-off-by: Matthew Thode <prometheanfire@gentoo.org>
Closes #1504
277909b
@prometheanfire

removed the selinux.h stuff, thanks

@behlendorf
Owner

With a little additional cleanup this was merged as:

11b9ec2 Add full SELinux support

@behlendorf behlendorf closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 19, 2013
  1. @prometheanfire

    Adding full selinux support

    prometheanfire authored
    removes old and janky selinux support and adds full support for all of
    the selinux context types.
    thanks to ryao and behlendorf for the help
    
    Signed-off-by: Matthew Thode <prometheanfire@gentoo.org>
    Closes #1504
This page is out of date. Refresh to see the latest.
View
2  cmd/mount_zfs/Makefile.am
@@ -20,5 +20,3 @@ mount_zfs_LDADD = \
$(top_builddir)/lib/libzpool/libzpool.la \
$(top_builddir)/lib/libzfs/libzfs.la \
$(top_builddir)/lib/libzfs_core/libzfs_core.la
-
-mount_zfs_LDADD += $(LIBSELINUX)
View
89 cmd/mount_zfs/mount_zfs.c
@@ -31,9 +31,6 @@
#include <sys/stat.h>
#include <libzfs.h>
#include <locale.h>
-#ifdef HAVE_LIBSELINUX
-#include <selinux/selinux.h>
-#endif /* HAVE_LIBSELINUX */
libzfs_handle_t *g_zfs;
@@ -73,11 +70,10 @@ static const option_map_t option_map[] = {
#ifdef MS_STRICTATIME
{ MNTOPT_DFRATIME, MS_STRICTATIME, ZS_COMMENT },
#endif
- { MNTOPT_CONTEXT, MS_COMMENT, ZS_NOCONTEXT },
- { MNTOPT_NOCONTEXT, MS_COMMENT, ZS_NOCONTEXT },
- { MNTOPT_FSCONTEXT, MS_COMMENT, ZS_NOCONTEXT },
- { MNTOPT_DEFCONTEXT, MS_COMMENT, ZS_NOCONTEXT },
- { MNTOPT_ROOTCONTEXT, MS_COMMENT, ZS_NOCONTEXT },
+ { MNTOPT_CONTEXT, MS_COMMENT, ZS_COMMENT },
+ { MNTOPT_FSCONTEXT, MS_COMMENT, ZS_COMMENT },
+ { MNTOPT_DEFCONTEXT, MS_COMMENT, ZS_COMMENT },
+ { MNTOPT_ROOTCONTEXT, MS_COMMENT, ZS_COMMENT },
#ifdef MS_I_VERSION
{ MNTOPT_IVERSION, MS_I_VERSION, ZS_COMMENT },
#endif
@@ -334,11 +330,35 @@ mtab_update(char *dataset, char *mntpoint, char *type, char *mntopts)
return (MOUNT_SUCCESS);
}
+static void
+__zfs_selinux_setcontext(const char *name, const char *context, char *mntopts,
+ char *mtabopt)
+{
+ char tmp[MNT_LINE_MAX];
+
+ snprintf(tmp, MNT_LINE_MAX, ",%s=\"%s\"", name, context);
+ strlcat(mntopts, tmp, MNT_LINE_MAX);
+ strlcat(mtabopt, tmp, MNT_LINE_MAX);
+}
+
+static void
+zfs_selinux_setcontext(zfs_handle_t *zhp, zfs_prop_t zpt, const char *name,
+ char *mntopts, char *mtabopt)
+{
+ char context[ZFS_MAXPROPLEN];
+
+ if (zfs_prop_get(zhp, zpt, context, sizeof (context),
+ NULL, NULL, 0, B_FALSE) == 0) {
+ if (strcmp(context, "none") != 0)
+ __zfs_selinux_setcontext(name, context, mntopts, mtabopt);
+ }
+}
+
int
main(int argc, char **argv)
{
zfs_handle_t *zhp;
- char legacy[ZFS_MAXPROPLEN];
+ char prop[ZFS_MAXPROPLEN];
char mntopts[MNT_LINE_MAX] = { '\0' };
char badopt[MNT_LINE_MAX] = { '\0' };
char mtabopt[MNT_LINE_MAX] = { '\0' };
@@ -433,21 +453,6 @@ main(int argc, char **argv)
}
}
-#ifdef HAVE_LIBSELINUX
- /*
- * Automatically add the default zfs context when selinux is enabled
- * and the caller has not specified their own context. This must be
- * done until zfs is added to the default selinux policy configuration
- * as a known filesystem type which supports xattrs.
- */
- if (is_selinux_enabled() && !(zfsflags & ZS_NOCONTEXT)) {
- (void) strlcat(mntopts, ",context=\"system_u:"
- "object_r:file_t:s0\"", sizeof (mntopts));
- (void) strlcat(mtabopt, ",context=\"system_u:"
- "object_r:file_t:s0\"", sizeof (mtabopt));
- }
-#endif /* HAVE_LIBSELINUX */
-
if (verbose)
(void) fprintf(stdout, gettext("mount.zfs:\n"
@@ -476,12 +481,36 @@ main(int argc, char **argv)
return (MOUNT_USAGE);
}
+ /*
+ * Checks to see if the ZFS_PROP_SELINUX_CONTEXT exists
+ * if it does, create a tmp variable in case it's needed
+ * checks to see if the selinux context is set to the default
+ * if it is, allow the setting of the other context properties
+ * this is needed because the 'context' property overrides others
+ * if it is not the default, set the 'context' property
+ */
+ if (zfs_prop_get(zhp, ZFS_PROP_SELINUX_CONTEXT, prop, sizeof (prop),
+ NULL, NULL, 0, B_FALSE) == 0) {
+ if (strcmp(prop, "none") == 0) {
+ zfs_selinux_setcontext(zhp, ZFS_PROP_SELINUX_FSCONTEXT,
+ MNTOPT_FSCONTEXT, mntopts, mtabopt);
+ zfs_selinux_setcontext(zhp, ZFS_PROP_SELINUX_DEFCONTEXT,
+ MNTOPT_DEFCONTEXT, mntopts, mtabopt);
+ zfs_selinux_setcontext(zhp,
+ ZFS_PROP_SELINUX_ROOTCONTEXT, MNTOPT_ROOTCONTEXT,
+ mntopts, mtabopt);
+ } else {
+ __zfs_selinux_setcontext(MNTOPT_CONTEXT,
+ prop, mntopts, mtabopt);
+ }
+ }
+
/* treat all snapshots as legacy mount points */
if (zfs_get_type(zhp) == ZFS_TYPE_SNAPSHOT)
- (void) strlcpy(legacy, ZFS_MOUNTPOINT_LEGACY, ZFS_MAXPROPLEN);
+ (void) strlcpy(prop, ZFS_MOUNTPOINT_LEGACY, ZFS_MAXPROPLEN);
else
- (void) zfs_prop_get(zhp, ZFS_PROP_MOUNTPOINT, legacy,
- sizeof (legacy), NULL, NULL, 0, B_FALSE);
+ (void) zfs_prop_get(zhp, ZFS_PROP_MOUNTPOINT, prop,
+ sizeof (prop), NULL, NULL, 0, B_FALSE);
zfs_close(zhp);
libzfs_fini(g_zfs);
@@ -497,17 +526,17 @@ main(int argc, char **argv)
* using zfs as your root file system both rc.sysinit/umountroot and
* systemd depend on 'mount -o remount <mountpoint>' to work.
*/
- if (zfsutil && !strcmp(legacy, ZFS_MOUNTPOINT_LEGACY)) {
+ if (zfsutil && !strcmp(prop, ZFS_MOUNTPOINT_LEGACY)) {
(void) fprintf(stderr, gettext(
"filesystem '%s' cannot be mounted using 'zfs mount'.\n"
"Use 'zfs set mountpoint=%s' or 'mount -t zfs %s %s'.\n"
"See zfs(8) for more information.\n"),
- dataset, mntpoint, dataset, mntpoint);
+ dataset, mntpoint, dataset, mntpoint);
return (MOUNT_USAGE);
}
if (!zfsutil && !(remount || fake) &&
- strcmp(legacy, ZFS_MOUNTPOINT_LEGACY)) {
+ strcmp(prop, ZFS_MOUNTPOINT_LEGACY)) {
(void) fprintf(stderr, gettext(
"filesystem '%s' cannot be mounted using 'mount'.\n"
"Use 'zfs set mountpoint=%s' or 'zfs mount %s'.\n"
View
4 include/sys/fs/zfs.h
@@ -143,6 +143,10 @@ typedef enum {
ZFS_PROP_INCONSISTENT, /* not exposed to the user */
ZFS_PROP_SNAPDEV,
ZFS_PROP_ACLTYPE,
+ ZFS_PROP_SELINUX_CONTEXT,
+ ZFS_PROP_SELINUX_FSCONTEXT,
+ ZFS_PROP_SELINUX_DEFCONTEXT,
+ ZFS_PROP_SELINUX_ROOTCONTEXT,
ZFS_NUM_PROPS
} zfs_prop_t;
View
2  lib/libspl/include/sys/mntent.h
@@ -47,7 +47,6 @@
#define MNTOPT_AUTO "auto" /* automount */
#define MNTOPT_NOAUTO "noauto" /* do not automount */
#define MNTOPT_CONTEXT "context" /* selinux context */
-#define MNTOPT_NOCONTEXT "nocontext" /* No selinux context (zfs-only) */
#define MNTOPT_FSCONTEXT "fscontext" /* selinux fscontext */
#define MNTOPT_DEFCONTEXT "defcontext" /* selinux defcontext */
#define MNTOPT_ROOTCONTEXT "rootcontext" /* selinux rootcontext */
@@ -96,6 +95,5 @@
#define ZS_COMMENT 0x00000000 /* comment */
#define ZS_ZFSUTIL 0x00000001 /* caller is zfs(8) */
-#define ZS_NOCONTEXT 0x00000002 /* do not add selinux context */
#endif /* _SYS_MNTENT_H */
View
13 man/man8/mount.zfs.8
@@ -75,6 +75,19 @@ Increase verbosity.
.BI "\-h"
Print the usage message.
.TP
+.BI "\-o context"
+This flag sets the SELinux context for all files in the filesytem
+under that mountpoint.
+.TP
+.BI "\-o fscontext"
+This flag sets the SELinux context for the filesytem being mounted.
+.TP
+.BI "\-o defcontext"
+This flag sets the SELinux context for unlabled files.
+.TP
+.BI "\-o rootcontext"
+This flag sets the SELinux context for the root inode of the filesystem.
+.TP
.BI "\-o legacy"
This private flag indicates that the
.I dataset
View
46 man/man8/zfs.8
@@ -1298,6 +1298,52 @@ Indicates whether the file system should reject file names that include characte
.sp
.LP
The \fBcasesensitivity\fR, \fBnormalization\fR, and \fButf8only\fR properties are also new permissions that can be assigned to non-privileged users by using the \fBZFS\fR delegated administration feature.
+.RE
+
+.sp
+.ne 2
+.mk
+.na
+\fB\fBcontext\fR=\fBSELinux_User:SElinux_Role:Selinux_Type:Sensitivity_Level\fR\fR
+.ad
+.sp .6
+.RS 4n
+This flag sets the SELinux context for all files in the filesytem under the mountpoint for that filesystem. See \fBselinux\fR(8) for more information.
+.RE
+
+.sp
+.ne 2
+.mk
+.na
+\fB\fBfscontext\fR=\fBSELinux_User:SElinux_Role:Selinux_Type:Sensitivity_Level\fR\fR
+.ad
+.sp .6
+.RS 4n
+This flag sets the SELinux context for the filesytem being mounted. See \fBselinux\fR(8) for more information.
+.RE
+
+.sp
+.ne 2
+.mk
+.na
+\fB\fBdefntext\fR=\fBSELinux_User:SElinux_Role:Selinux_Type:Sensitivity_Level\fR\fR
+.ad
+.sp .6
+.RS 4n
+This flag sets the SELinux context for unlabeled files. See \fBselinux\fR(8) for more information.
+.RE
+
+.sp
+.ne 2
+.mk
+.na
+\fB\fBrootcontext\fR=\fBSELinux_User:SElinux_Role:Selinux_Type:Sensitivity_Level\fR\fR
+.ad
+.sp .6
+.RS 4n
+This flag sets the SELinux context for the root inode of the filesystem. See \fBselinux\fR(8) for more information.
+.RE
+
.SS "Temporary Mount Point Properties"
.LP
When a file system is mounted, either through \fBmount\fR(8) for legacy mounts or the \fBzfs mount\fR command for normal file systems, its mount options are set according to its properties. The correlation between properties and mount options is as follows:
View
12 module/zcommon/zfs_prop.c
@@ -332,6 +332,18 @@ zfs_prop_init(void)
zprop_register_string(ZFS_PROP_MLSLABEL, "mlslabel",
ZFS_MLSLABEL_DEFAULT, PROP_INHERIT, ZFS_TYPE_DATASET,
"<sensitivity label>", "MLSLABEL");
+ zprop_register_string(ZFS_PROP_SELINUX_CONTEXT, "context",
+ "none", PROP_DEFAULT, ZFS_TYPE_DATASET, "<selinux context>",
+ "CONTEXT");
+ zprop_register_string(ZFS_PROP_SELINUX_FSCONTEXT, "fscontext",
+ "none", PROP_DEFAULT, ZFS_TYPE_DATASET, "<selinux fscontext>",
+ "FSCONTEXT");
+ zprop_register_string(ZFS_PROP_SELINUX_DEFCONTEXT, "defcontext",
+ "none", PROP_DEFAULT, ZFS_TYPE_DATASET, "<selinux defcontext>",
+ "DEFCONTEXT");
+ zprop_register_string(ZFS_PROP_SELINUX_ROOTCONTEXT, "rootcontext",
+ "none", PROP_DEFAULT, ZFS_TYPE_DATASET, "<selinux rootcontext>",
+ "ROOTCONTEXT");
/* readonly number properties */
zprop_register_number(ZFS_PROP_USED, "used", 0, PROP_READONLY,
Something went wrong with that request. Please try again.