Re: NFS/LSM: allow NFS to control all of its own mount options
On Wed, 2008-02-20 at 08:50 -0500, Stephen Smalley wrote: > On Wed, 2008-02-20 at 11:08 +0100, Miklos Szeredi wrote: > > > Please don't introduce a special case for just nfs. All filesystems > > > should control their mount options, so please provide some library > > > helpers for context= handling and move it into all filesystems that > > > can support selinux. > > > > Hmm, looks like selinux is not showing it's mount options in > > /proc/mounts. Well, actually there's no infrastructure for it either. > > Here's a template patch (completely untested). > > I think the intent is to use the security_sb_get_mnt_opts() hook for > this purpose. It was. I already knew about this issue and its 'on my list.' Although I guess we need a something ?new LSM hook? which will translate the sb_get_mnt_opts stuff into a single text string. Or I guess really that can be done in you sb_show_options and I can just use sb_get_mnt_opts under the covers. Anyway, unrelated issue that will get fixed as soon as this real BUG() is fixed. -Eric - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: NFS/LSM: allow NFS to control all of its own mount options
On Tue, 2008-02-19 at 17:24 -0500, Christoph Hellwig wrote: > Please don't introduce a special case for just nfs. All filesystems > should control their mount options, so please provide some library > helpers for context= handling and move it into all filesystems that > can support selinux. A library helper that looks like what? Only NFS knows how it is storing that mount option in its blobs. Only NFS knows how to translate its blob into the generic LSM interface needed to set security options. I'd say the solution is going to have to be very much NFS specific. Both in kernel LSMs already provide methods for dealing with mount options for filesystems that use text strings (see the security_sb_copy_data stuff called from vfs_kern_mount()). How is this 'library' going to deal with anything other than a text string, and if that's all it deals with we already have that. NFS just can't use it because it isn't using a string for mount data. I'm sure I'm just misunderstanding how to design your solution... -Eric - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
NFS/LSM: allow NFS to control all of its own mount options
In the current code (approved by SELinux and NFS people in 2004) SELinux
tries to understand NFS's binary mount data. This blows up in the face
of things like nohide mounts which don't use struct nfs_mount_data and I
assume just looking at the code that things don't work since NFS moved
to using nfs_parsed_mount_data as its default binary mount data blob.
This patch moves all of the ownership of the mount options into NFS. It
brings the text based mount options capabilities up to the same level of
support which existed in version 6 of the old binary mount data from
userspace. I am not looking at NFSv4 at the moment and the only mount
option this is supporting is context= (just like the old binary support)
Basically this patch causes NFS to make use of the new LSM hooks
security_sb_set_mnt_opts() and security_sb_clone_mnt_opts(). We do this
in the NFS get_sb() calls so that security settings are set explicitly
by NFS before they are set by the generic vfs security hooks which
handle filesystems which use text mount data.
We need to push this for 2.6.25 since at the moment SELinux(or SMACK) +
nohide mounts cause security_sb_copy_mount_data() to copy one page of
mount data starting at the struct nfs_clone_mount_data on the stack. If
the stack doesn't span 2 pages we run off the end of the stack and hit a
page fault BUG(). Not sure why this didn't happen for me in 2.6.24, but
my guess is the stack size is significantly smaller for this operation
in 2.6.25 so the window is just bigger.
Signed-off-by: Eric Paris <[EMAIL PROTECTED]>
---
I tested mounting using both the version 6 binary mount data from
userspace and using the text mount options in a simple program I wrote
to call mount directly. I was able to correctly set the selinux context
of my mounts and of clone mounts like those created by nohide exports.
This also fixes the BUG() in SMACK code because of the VFS change.
SMACK may want to move to a BUG_ON() like I do in the
selinux_sb_copy_data code just to make it clear binary mount data is not
expected, but I'll leave that up to you.
fs/nfs/internal.h|7 ++
fs/nfs/super.c | 31 ++--
fs/super.c |2 +-
security/security.c |3 ++
security/selinux/hooks.c | 48 -
5 files changed, 60 insertions(+), 31 deletions(-)
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 0f56196..8e4981c 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -14,6 +14,9 @@ struct nfs_string;
*/
#define NFS_MAX_READAHEAD (RPC_DEF_SLOT_TABLE - 1)
+/* this MUST stay at least as large as the define in nfs_mount.h */
+#define NFS_MAX_INTERNAL_CONTEXT_LEN 256
+
struct nfs_clone_mount {
const struct super_block *sb;
const struct dentry *dentry;
@@ -57,6 +60,10 @@ struct nfs_parsed_mount_data {
char*export_path;
int protocol;
} nfs_server;
+
+ struct {
+ char context[NFS_MAX_INTERNAL_CONTEXT_LEN + 1];
+ } lsm_opts;
};
/* client.c */
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 1fb3818..dd85c22 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -90,7 +90,7 @@ enum {
/* Mount options that take string arguments */
Opt_sec, Opt_proto, Opt_mountproto, Opt_mounthost,
- Opt_addr, Opt_mountaddr, Opt_clientaddr,
+ Opt_addr, Opt_mountaddr, Opt_clientaddr, Opt_context,
/* Mount options that are ignored */
Opt_userspace, Opt_deprecated,
@@ -151,6 +151,8 @@ static match_table_t nfs_mount_option_tokens = {
{ Opt_mounthost, "mounthost=%s" },
{ Opt_mountaddr, "mountaddr=%s" },
+ { Opt_context, "context=%s" },
+
{ Opt_err, NULL }
};
@@ -1025,7 +1027,15 @@ static int nfs_parse_mount_options(char *raw,
&mnt->mount_server.addrlen);
kfree(string);
break;
-
+ case Opt_context:
+ string = match_strdup(args);
+ if (string == NULL)
+ goto out_nomem;
+ /* last byte of the array will be 0 if arg too long */
+ strncpy(mnt->lsm_opts.context, string,
+ NFS_MAX_CONTEXT_LEN);
+ kfree(string);
+ break;
case Opt_userspace:
case Opt_deprecated:
break;
@@ -1214,6 +1224,8 @@ static int nfs_validate_mount_data(void *options,
args->namlen= data->namlen;
args->bsize = data->bsize;
args->auth_flavors[0] = data->pseudoflavor;
+ strncpy(args->lsm_opts.context, data-&
Re: [PATCH] Allow Kconfig to set default mmap_min_addr protection
On Fri, 2007-12-21 at 23:59 +0100, Jan Engelhardt wrote: > On Dec 21 2007 14:35, Greg KH wrote: > >> >> >I guess it could be, but the input for /proc/sys/vm/mmap_min_addr is > >> >> >base 10 as well > >> >> > >> >> sysfs is autobase, i.e. echo "0xb000" >/sys/foo will Do The Right Thing. > >> > > >> >yes but if you cat /proc/sys/vm/mmap_min_addr, it returns in base 10. > >> > >> sysfs should probably be tuned to output it in a preferred base. > > > >Again, this is sysctl, not sysfs. two very different things... > > > Argh... :) Just shows that /proc is the wrong place for system variables. > > Well, module_params(integer) are autobase, and that's all I needed so > far :-D So in the end we are all happy with the original patch I sent? -Eric - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Allow Kconfig to set default mmap_min_addr protection
On Thu, 2007-12-20 at 00:29 +0100, Jan Engelhardt wrote: > On Dec 19 2007 16:59, Eric Paris wrote: > > > >+config SECURITY_DEFAULT_MMAP_MIN_ADDR > >+int "Low address space to protect from user allocation" > > Hm, should not this be 'hex'? I guess it could be, but the input for /proc/sys/vm/mmap_min_addr is base 10 as well so I figured consistency was a good thing. Do you have strong feelings? I guess so since you posted about it. -Eric - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch, rfc] mm.h, security.h, key.h and preventing namespace poisoning
On Thu, 2007-12-20 at 11:07 +1100, James Morris wrote: > On Wed, 19 Dec 2007, David Chinner wrote: > > > Folks, > > > > I just updated a git tree and started getting errors on a > > "copy_keys" macro warning. > > > > The code I've been working on uses a ->copy_keys() method for > > copying the keys in a btree block from one place to another. I've > > been working on this code for a while > > (http://oss.sgi.com/archives/xfs/2007-11/msg00046.html) and keep the > > tree I'm working in reletively up to date (lags linus by a couple of > > weeks at most). The update I did this afternoon gave a conflict > > warning with the macro in include/linux/key.h. > > > > Given that I'm not directly including key.h anywhere in the XFS > > code, I'm getting the namespace polluted indirectly from some other > > include that is necessary. > > > > As it turns out, this commit from 13 days ago: > > > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7cd94146cd504016315608e297219f9fb7b1413b > > > > included security.h in mm.h and that is how I'm seeing the namespace > > poisoning coming from key.h when !CONFIG_KEY. > > > > Including security.h in mm.h means much wider includes for pretty > > much the entire kernel, and it opens up namespace issues like this > > that never previously existed. > > > > The patch below (only tested for !CONFIG_KEYS && !CONFIG_SECURITY) > > moves security.h into the mmap.c and nommu.c files that need it so > > it doesn't end up with kernel wide scope. > > > > Comments? > > The idea with this placement was to keep memory management code with other > similar code, rather than pushing it into security.h, where it does not > functionally belong. > > Something to not also is that you can't "depend" on security.h not being > included all over the place, as LSM does touch a lot of the kernel. > Unecessarily including it is bad, of course. > > I'm not sure I understand your namespace pollution issue, either. > > In any case, I think the right solution is not to include security.h at > all in mm.h, as it is only being done to get a declaration for > mmap_min_addr. > > How about this, instead ? > > Signed-off-by: James Morris <[EMAIL PROTECTED]> Acked-by: Eric Paris <[EMAIL PROTECTED]> > --- > > mm.h |5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 1b7b95c..02fbac7 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -12,7 +12,6 @@ > #include > #include > #include > -#include > > struct mempolicy; > struct anon_vma; > @@ -34,6 +33,10 @@ extern int sysctl_legacy_va_layout; > #define sysctl_legacy_va_layout 0 > #endif > > +#ifdef CONFIG_SECURITY > +extern unsigned long mmap_min_addr; > +#endif > + > #include > #include > #include > > - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Allow Kconfig to set default mmap_min_addr protection
Since it was decided that low memory protection from userspace couldn't
be turned on by default add a Kconfig option to allow users/distros to
set a default at compile time. This value is still tunable after boot
in /proc/sys/vm/mmap_min_addr
Signed-off-by: Eric Paris <[EMAIL PROTECTED]>
---
security/Kconfig| 18 ++
security/security.c |4 +++-
2 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/security/Kconfig b/security/Kconfig
index 8086e61..10c9e40 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -103,6 +103,24 @@ config SECURITY_ROOTPLUG
If you are unsure how to answer this question, answer N.
+config SECURITY_DEFAULT_MMAP_MIN_ADDR
+int "Low address space to protect from user allocation"
+depends on SECURITY
+default 0
+help
+ This is the portion of low virtual memory which should be protected
+ from userspace allocation. Keeping a user from writing to low pages
+ can help reduce the impact of kernel NULL pointer bugs.
+
+ For most users with lots of address space a value of 65536 is
+ reasonable and should cause no problems. Programs which use vm86
+ functionality would either need additional permissions from either
+ the LSM or the capabilities module or have this protection disabled.
+
+ This value can be changed after boot using the
+ /proc/sys/vm/mmap_min_addr tunable.
+
+
source security/selinux/Kconfig
endmenu
diff --git a/security/security.c b/security/security.c
index 0e1f1f1..c784726 100644
--- a/security/security.c
+++ b/security/security.c
@@ -23,7 +23,9 @@ extern struct security_operations dummy_security_ops;
extern void security_fixup_ops(struct security_operations *ops);
struct security_operations *security_ops; /* Initialized to NULL */
-unsigned long mmap_min_addr; /* 0 means no protection */
+
+/* amount of vm to protect from userspace access */
+unsigned long mmap_min_addr = CONFIG_SECURITY_DEFAULT_MMAP_MIN_ADDR;
static inline int verify(struct security_operations *ops)
{
-
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] VM/Security: add security hook to do_brk
Given a specifically crafted binary do_brk() can be used to get low pages available in userspace virtually memory and can thus be used to circumvent the mmap_min_addr low memory protection. Add security checks in do_brk(). Signed-off-by: Eric Paris <[EMAIL PROTECTED]> --- mm/mmap.c |4 1 file changed, 4 insertions(+) diff --git a/mm/mmap.c b/mm/mmap.c index f4cfc6a..15678aa 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1941,6 +1941,10 @@ unsigned long do_brk(unsigned long addr, unsigned long len) if (is_hugepage_only_range(mm, addr, len)) return -EINVAL; + error = security_file_mmap(0, 0, 0, 0, addr, 1); + if (error) + return error; + flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags; error = arch_mmap_check(addr, len, flags); - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -v3] SELinux: Add get, set, and cloning of superblock security information
Any complaints or questions left here? I've got more people reporting
problems with NFS/SELinux and this is the first (and hardest) step to
making NFS and any genic LSM play nicely. If there are not any
problems how should this be pushed to linus? Through James Morris's
git tree? Through Chris Wright's LSM tree?
And I did get a:
Acked-by: Stephen D. Smalley <[EMAIL PROTECTED]>
Which I don't think ever appeared on all the lists for everyone interested.
-Eric
> Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> security_clont_sb_mnt_opts to the LSM and to SELinux. This will allow
> filesystems to directly own and control all of their mount options if
> they so choose. This interface deals only with option identifiers and
> strings so it should generic enough for any LSM which may come in the
> future. Filesystems which pass text mount data around in the kernel
> (almost all of them) need not currently make use of this interface for
> SELinux sake since it will still parse those strings as it always has.
>
> An LSM would need to implement these functions only if they had mount
> time options, such as selinux has context= or fscontext=. If the LSM
> has no mount time options they could simply not implement and let the
> dummy ops take care of things.
>
> A LSM other than SELinux would need to define new option numbers in
> security.h (or could reuse if they have the same basic meaning I guess)
> and any FS which decides to own there own security options would need to
> be patched to use this new interface for every possible LSM. This is
> because it was stated to me very clearly that LSM's should not attempt
> to understand FS mount data and the burdon to understand security should
> be in the FS which owns the options.
>
> Signed-off-by: Eric Paris <[EMAIL PROTECTED]>
>
> ---
>
> For now the only forseen user of this interface is NFS. NFS uses a
> binary blob in kernel for mount data (it uses this blob irrespective of
> the binary vs. text mount options it can get from userspace.) NFS must
> then set its own mount options explicitly so we need some interface for
> it to do so.
>
>
> include/linux/security.h | 36 ++
> security/dummy.c | 26 ++
> security/security.c | 20 +
> security/selinux/hooks.c | 749
> -
> security/selinux/include/objsec.h |1 +
> 5 files changed, 578 insertions(+), 254 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index ac05083..dcbb792 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -34,6 +34,12 @@
> #include
> #include
>
> +/* only a char in selinux superblock security struct flags */
> +#define FSCONTEXT_MNT 0x01
> +#define CONTEXT_MNT0x02
> +#define ROOTCONTEXT_MNT0x04
> +#define DEFCONTEXT_MNT 0x08
> +
> /*
> * Bounding set
> */
> @@ -261,6 +267,22 @@ struct request_sock;
> * Update module state after a successful pivot.
> * @old_nd contains the nameidata structure for the old root.
> * @new_nd contains the nameidata structure for the new root.
> + * @sb_get_mnt_opts:
> + * Get the security relevant mount options used for a superblock
> + * @sb the superblock to get security mount options from
> + * @mount_options array for pointers to mount options
> + * @mount_flags array of ints specifying what each mount options is
> + * @num_opts number of options in the arrays
> + * @sb_set_mnt_opts:
> + * Set the security relevant mount options used for a superblock
> + * @sb the superblock to set security mount options for
> + * @mount_options array for pointers to mount options
> + * @mount_flags array of ints specifying what each mount options is
> + * @num_opts number of options in the arrays
> + * @sb_clone_mnt_opts:
> + * Copy all security options from a given superblock to another
> + * @oldsb old superblock which contain information to clone
> + * @newsb new superblock which needs filled in
> *
> * Security hooks for inode operations.
> *
> @@ -1242,6 +1264,13 @@ struct security_operations {
> struct nameidata * new_nd);
> void (*sb_post_pivotroot) (struct nameidata * old_nd,
>struct nameidata * new_nd);
> + int (*sb_get_mnt_opts) (const struct super_block *sb,
> + char ***mount_options, int **flags,
> + int *num_opts);
> + int (*sb_set_mnt_opts) (struct super_block *sb, char **mount_options,
> +
Re: [PATCH 2/2] Version 11 (2.6.24-rc2) Smack: Simplified Mandatory Access Control Kernel
[snip from fs/super.c:vfs_kern_mount() just for reference]
if (data) {
secdata = alloc_secdata();
if (!secdata)
goto out_mnt;
error = security_sb_copy_data(type, data, secdata);
if (error)
goto out_free_secdata;
}
error = type->get_sb(type, flags, name, data, mnt);
if (error < 0)
goto out_free_secdata;
BUG_ON(!mnt->mnt_sb);
error = security_sb_kern_mount(mnt->mnt_sb, secdata);
if (error)
goto out_sb;
[end snip]
> +/**
> + * smack_sb_copy_data - copy mount options data for processing
> + * @type: file system type
> + * @orig: where to start
> + * @smackopts
> + *
> + * Returns 0 on success or -ENOMEM on error.
> + *
> + * Copy the Smack specific mount options out of the mount
> + * options list.
> + */
> +static int smack_sb_copy_data(struct file_system_type *type, void *orig,
> + void *smackopts)
> +{
> + char *cp, *commap, *otheropts, *dp;
> +
> + /* Binary mount data: just copy */
> + if (type->fs_flags & FS_BINARY_MOUNTDATA) {
> + copy_page(smackopts, orig);
> + return 0;
> + }
So given NFS, which may have passed you a nfs_mount_data,
nfs_parsed_mount_data, or a nfs_clone_mount struct one page is going
to get coppied back out to the VFS.
> +
> + otheropts = (char *)get_zeroed_page(GFP_KERNEL);
> + if (otheropts == NULL)
> + return -ENOMEM;
> +
> + for (cp = orig, commap = orig; commap != NULL; cp = commap + 1) {
> + if (strstr(cp, SMK_FSDEFAULT) == cp)
> + dp = smackopts;
> + else if (strstr(cp, SMK_FSFLOOR) == cp)
> + dp = smackopts;
> + else if (strstr(cp, SMK_FSHAT) == cp)
> + dp = smackopts;
> + else if (strstr(cp, SMK_FSROOT) == cp)
> + dp = smackopts;
> + else
> + dp = otheropts;
> +
> + commap = strchr(cp, ',');
> + if (commap != NULL)
> + *commap = '\0';
> +
> + if (*dp != '\0')
> + strcat(dp, ",");
> + strcat(dp, cp);
> + }
> +
> + strcpy(orig, otheropts);
> + free_page((unsigned long)otheropts);
> +
> + return 0;
> +}
> +
> +/**
> + * smack_sb_kern_mount - Smack specific mount processing
> + * @sb: the file system superblock
> + * @data: the smack mount options
> + *
> + * Returns 0 on success, an error code on failure
> + */
> +static int smack_sb_kern_mount(struct super_block *sb, void *data)
> +{
> + struct dentry *root = sb->s_root;
> + struct inode *inode = root->d_inode;
> + struct superblock_smack *sp = sb->s_security;
> + struct inode_smack *isp;
> + char *op;
> + char *commap;
> + char *nsp;
> +
> + spin_lock(&sp->smk_sblock);
> + if (sp->smk_initialized != 0) {
> + spin_unlock(&sp->smk_sblock);
> + return 0;
> + }
> + sp->smk_initialized = 1;
> + spin_unlock(&sp->smk_sblock);
> +
> + for (op = data; op != NULL; op = commap) {
Here you walk this page of binary NFS data looking for your stuff.
And while I assume its unlikely you find anything you like on this
page and go wrong, its not impossible.
SELinux tried to solve this problem right here in its equivalent
function years ago, but it has since been despised by the FS people
and now i'm trying to make everyone happy. I'd love you comment on
how you plan to support NFS and other filesystems which use
FS_BINARY_MOUNTDATA)
> + commap = strchr(op, ',');
> + if (commap != NULL)
> + *commap++ = '\0';
> +
> + if (strncmp(op, SMK_FSHAT, strlen(SMK_FSHAT)) == 0) {
> + op += strlen(SMK_FSHAT);
> + nsp = smk_import(op, 0);
> + if (nsp != NULL)
> + sp->smk_hat = nsp;
> + } else if (strncmp(op, SMK_FSFLOOR, strlen(SMK_FSFLOOR)) ==
> 0) {
> + op += strlen(SMK_FSFLOOR);
> + nsp = smk_import(op, 0);
> + if (nsp != NULL)
> + sp->smk_floor = nsp;
> + } else if (strncmp(op, SMK_FSDEFAULT,
> + strlen(SMK_FSDEFAULT)) == 0) {
> + op += strlen(SMK_FSDEFAULT);
> + nsp = smk_import(op, 0);
> + if (nsp != NULL)
> + sp->smk_default = nsp;
> + } else if (strncmp(op, SMK_FSROOT, strlen(SMK_FSROOT)) == 0) {
> + op += strlen(SMK_FSROOT);
> + nsp = smk_import(op, 0);
> +
Re: [PATCH -v3] SELinux: Add get, set, and cloning of superblock security information
On Fri, 2007-11-09 at 14:46 -0800, Casey Schaufler wrote: > --- Eric Paris <[EMAIL PROTECTED]> wrote: > > > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and > > security_clont_sb_mnt_opts to the LSM and to SELinux. This will allow > > filesystems to directly own and control all of their mount options if > > they so choose. > > I understand why you would want get_sb_mnt_opts(), but what > is the value for set_sb_mnt_opts() and what is the purpose of > clone_sb_mnt_opts()? set is really the most important one. NFS knows when it creates a superblock (using SELinux as an example) that it wants to set context=blah. Thus it calls into set_sb_mnt_opts with the flag for "context=" and "blah." get_sb_mnt_opts will likely get used in the future for /proc/mounts to be able to report the security options. clone is really just to make it easy for the FS. If they know "i want the new one to look like this old one" they can just call into clone_ and don't have to worry about dealing freeing memory or anything like that... > > > This interface deals only with option identifiers and > > strings so it should generic enough for any LSM which may come in the > > future. Filesystems which pass text mount data around in the kernel > > (almost all of them) need not currently make use of this interface for > > SELinux sake since it will still parse those strings as it always has. > > If SELinux is still dealing with strings on it's own what is > the point of these hooks? The point is that not all filesystems use strings. NFS is the real in kernel kicker. See things like fs/nfs/namespace.c:nfs_do_clone_mount() where they pass a binary blob into the vfs which arrives to the LSM as a binary blob which it cannot parse. (note NFS also uses nfs_parsed_mount_data and nfs_mount_data) Since the LSM can't (ok, "isn't allowed" according to previous discussions with vfs/fs people) deal with those binary blobs to get its options some method must be created for those filesystems to pass that data in a usable way. > > > An LSM would need to implement these functions only if they had mount > > time options, such as selinux has context= or fscontext=. If the LSM > > has no mount time options they could simply not implement and let the > > dummy ops take care of things. > > Smack and SELinux currently deal with options in sb_kern_mount(), with > help from sb_copy_data(). Why change the implementation? I don't plan to change anything for any FS that passes text options, but we aren't allowed to parse binary blobs (nor can we actually even know for sure what blobs we are dealing with currently in the LSM even if we were 'allowed' to parse them and get the needed data directly) > > A LSM other than SELinux would need to define new option numbers in > > security.h > > I don't think it is a good idea to require that LSM specific > information be stored outside the scope of the LSM. Its either that or FS specific knowledge inside the LSM. See security/selinux/hooks.c:try_context_mount() for an example of NFS specific knowledge inside an LSM. This current implementation has bugs since we don't know if *data is any of the 3 above named structs. This patch doesn't fix those issues, but lays the groundwork for a fix... > > (or could reuse if they have the same basic meaning I guess) > > and any FS which decides to own there own security options would need to > > be patched to use this new interface for every possible LSM. This is > > because it was stated to me very clearly that LSM's should not attempt > > to understand FS mount data and the burdon to understand security should > > be in the FS which owns the options. > > Perhaps a mount option prefix then. "Smack.root", "SELinux.context", > that sort of thing. An LSM writer shouldn't have to patch security.h > every time she wants to add a mount option. I did originally namespace these things such as SELINUX__CONTEXT_MNT but later wondered what the point was. If another LSM decided to somehow make use of the same infrastructure in FS that have binary mount data and they used context= they should be able to use a generic CONTEXT_MNT rather than pretend that flag has some special meaning. It actually works nicely for other LSMs since once I get finished with NFS it should support 3 mount options which although maybe not named nicely for non SELinux LSMs will be usable without any FS changes... And while I don't think its a great thing that every LSM is going to be adding things to the generic security.h if they want things to work, they are going to have to add things to the generic structures used by filesystems which use binary mount
[PATCH -v3] SELinux: Add get, set, and cloning of superblock security information
Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
security_clont_sb_mnt_opts to the LSM and to SELinux. This will allow
filesystems to directly own and control all of their mount options if
they so choose. This interface deals only with option identifiers and
strings so it should generic enough for any LSM which may come in the
future. Filesystems which pass text mount data around in the kernel
(almost all of them) need not currently make use of this interface for
SELinux sake since it will still parse those strings as it always has.
An LSM would need to implement these functions only if they had mount
time options, such as selinux has context= or fscontext=. If the LSM
has no mount time options they could simply not implement and let the
dummy ops take care of things.
A LSM other than SELinux would need to define new option numbers in
security.h (or could reuse if they have the same basic meaning I guess)
and any FS which decides to own there own security options would need to
be patched to use this new interface for every possible LSM. This is
because it was stated to me very clearly that LSM's should not attempt
to understand FS mount data and the burdon to understand security should
be in the FS which owns the options.
Signed-off-by: Eric Paris <[EMAIL PROTECTED]>
---
For now the only forseen user of this interface is NFS. NFS uses a
binary blob in kernel for mount data (it uses this blob irrespective of
the binary vs. text mount options it can get from userspace.) NFS must
then set its own mount options explicitly so we need some interface for
it to do so.
include/linux/security.h | 36 ++
security/dummy.c | 26 ++
security/security.c | 20 +
security/selinux/hooks.c | 749 -
security/selinux/include/objsec.h |1 +
5 files changed, 578 insertions(+), 254 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index ac05083..dcbb792 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -34,6 +34,12 @@
#include
#include
+/* only a char in selinux superblock security struct flags */
+#define FSCONTEXT_MNT 0x01
+#define CONTEXT_MNT0x02
+#define ROOTCONTEXT_MNT0x04
+#define DEFCONTEXT_MNT 0x08
+
/*
* Bounding set
*/
@@ -261,6 +267,22 @@ struct request_sock;
* Update module state after a successful pivot.
* @old_nd contains the nameidata structure for the old root.
* @new_nd contains the nameidata structure for the new root.
+ * @sb_get_mnt_opts:
+ * Get the security relevant mount options used for a superblock
+ * @sb the superblock to get security mount options from
+ * @mount_options array for pointers to mount options
+ * @mount_flags array of ints specifying what each mount options is
+ * @num_opts number of options in the arrays
+ * @sb_set_mnt_opts:
+ * Set the security relevant mount options used for a superblock
+ * @sb the superblock to set security mount options for
+ * @mount_options array for pointers to mount options
+ * @mount_flags array of ints specifying what each mount options is
+ * @num_opts number of options in the arrays
+ * @sb_clone_mnt_opts:
+ * Copy all security options from a given superblock to another
+ * @oldsb old superblock which contain information to clone
+ * @newsb new superblock which needs filled in
*
* Security hooks for inode operations.
*
@@ -1242,6 +1264,13 @@ struct security_operations {
struct nameidata * new_nd);
void (*sb_post_pivotroot) (struct nameidata * old_nd,
struct nameidata * new_nd);
+ int (*sb_get_mnt_opts) (const struct super_block *sb,
+ char ***mount_options, int **flags,
+ int *num_opts);
+ int (*sb_set_mnt_opts) (struct super_block *sb, char **mount_options,
+ int *flags, int num_opts);
+ void (*sb_clone_mnt_opts) (const struct super_block *oldsb,
+ struct super_block *newsb);
int (*inode_alloc_security) (struct inode *inode);
void (*inode_free_security) (struct inode *inode);
@@ -1499,6 +1528,13 @@ void security_sb_post_mountroot(void);
void security_sb_post_addmount(struct vfsmount *mnt, struct nameidata
*mountpoint_nd);
int security_sb_pivotroot(struct nameidata *old_nd, struct nameidata *new_nd);
void security_sb_post_pivotroot(struct nameidata *old_nd, struct nameidata
*new_nd);
+int security_sb_get_mnt_opts(const struct super_block *sb, char
***mount_options,
+int **flags, int *num_opts);
+int security_sb_set_mnt_opts(struct super_block *sb, char **mount_options,
+int *flags, int num_opts);
+void security_sb_clone_mnt_opts(c
Re: [PATCH -v3] SELinux: Add get, set, and cloning of superblock security information
On Fri, 2007-11-09 at 08:29 -0800, Casey Schaufler wrote:
> --- Stephen Smalley <[EMAIL PROTECTED]> wrote:
>
> > On Thu, 2007-11-08 at 16:37 -0500, Eric Paris wrote:
> > > Adds security_get_sb_mnt_opts, security_set_sb_mnt_opts, and
> > > security_clont_sb_mnt_opts to the LSM and to SELinux. This will allow
> > > filesystems to directly own and control all of their mount options if
> > > they so choose.
> > >
> > > Signed-off-by: Eric Paris <[EMAIL PROTECTED]>
> >
> > Acked-by: Stephen D. Smalley <[EMAIL PROTECTED]>
>
> Shouldn't this get cross posted to the LSM list?
Yes, I was reminded of that yesterday and will do so today.
>
> > >
> > > ---
> > >
> > > include/linux/security.h | 36 ++
> > > security/dummy.c | 26 ++
> > > security/security.c | 20 +
> > > security/selinux/hooks.c | 749
> > -
> > > security/selinux/include/objsec.h |1 +
> > > 5 files changed, 578 insertions(+), 254 deletions(-)
> > >
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> > > index ac05083..dcbb792 100644
> > > --- a/include/linux/security.h
> > > +++ b/include/linux/security.h
> > > @@ -34,6 +34,12 @@
> > > #include
> > > #include
> > >
> > > +/* only a char in selinux superblock security struct flags */
> > > +#define FSCONTEXT_MNT0x01
> > > +#define CONTEXT_MNT 0x02
> > > +#define ROOTCONTEXT_MNT 0x04
> > > +#define DEFCONTEXT_MNT 0x08
> > > +
> > > /*
> > > * Bounding set
> > > */
> > > @@ -261,6 +267,22 @@ struct request_sock;
> > > * Update module state after a successful pivot.
> > > * @old_nd contains the nameidata structure for the old root.
> > > * @new_nd contains the nameidata structure for the new root.
> > > + * @sb_get_mnt_opts:
> > > + * Get the security relevant mount options used for a superblock
> > > + * @sb the superblock to get security mount options from
> > > + * @mount_options array for pointers to mount options
> > > + * @mount_flags array of ints specifying what each mount options is
> > > + * @num_opts number of options in the arrays
> > > + * @sb_set_mnt_opts:
> > > + * Set the security relevant mount options used for a superblock
> > > + * @sb the superblock to set security mount options for
> > > + * @mount_options array for pointers to mount options
> > > + * @mount_flags array of ints specifying what each mount options is
> > > + * @num_opts number of options in the arrays
> > > + * @sb_clone_mnt_opts:
> > > + * Copy all security options from a given superblock to another
> > > + * @oldsb old superblock which contain information to clone
> > > + * @newsb new superblock which needs filled in
> > > *
> > > * Security hooks for inode operations.
> > > *
> > > @@ -1242,6 +1264,13 @@ struct security_operations {
> > >struct nameidata * new_nd);
> > > void (*sb_post_pivotroot) (struct nameidata * old_nd,
> > > struct nameidata * new_nd);
> > > + int (*sb_get_mnt_opts) (const struct super_block *sb,
> > > + char ***mount_options, int **flags,
> > > + int *num_opts);
> > > + int (*sb_set_mnt_opts) (struct super_block *sb, char **mount_options,
> > > + int *flags, int num_opts);
> > > + void (*sb_clone_mnt_opts) (const struct super_block *oldsb,
> > > +struct super_block *newsb);
> > >
> > > int (*inode_alloc_security) (struct inode *inode);
> > > void (*inode_free_security) (struct inode *inode);
> > > @@ -1499,6 +1528,13 @@ void security_sb_post_mountroot(void);
> > > void security_sb_post_addmount(struct vfsmount *mnt, struct nameidata
> > *mountpoint_nd);
> > > int security_sb_pivotroot(struct nameidata *old_nd, struct nameidata
> > *new_nd);
> > > void security_sb_post_pivotroot(struct nameidata *old_nd, struct
> > > nameidata
> > *new_nd);
> > > +int security_sb_get_mnt_opts(const struct super_block *sb, char
> > ***mount_options,
> > > +
