This patch set is the first submission to fs-devel and lkml for the purpose of code review. To test the patch set you need patches to nfs-utils as well. Since this is just a code review I haven't posted the patch to nfs-utils however if you want to test the code feel free to e-mail me and I will send you the necessary patch. Out of all of the functionality we have prototyped I have narrowed it down to these items which I believe is the solid base for initial kernel inclusion. These patches provide the mechanism to allow the server to provide security labels to the client and a method for the client to change labels on the server. The next revision of this patch set will allow for the client's subject (process) label to be transmitted with the access requests so the server can also make access decisions against the acting local policy. This part of the patch set will be made substantially cleaner by the credentials patches proposed by David Howells. Known Issues: Eventually stronger notification of security label changes will be added. For now this is accomplished by using NFS's normal cache invalidation (timeout). When acting as root on a root_squashed export changing the label on a file manages to set the label locally in the NFS inode but doesn't set it on the exported file system. In this case the fault is the server is returning OK for the setattr option instead of EPERM. This will be fixed in the next version. fs/Kconfig | 17 ++ fs/attr.c | 43 ++++ fs/nfs/client.c | 16 ++ fs/nfs/dir.c | 101 ++++++++++- fs/nfs/getroot.c | 33 +++ fs/nfs/inode.c | 58 ++++++- fs/nfs/namespace.c | 3 + fs/nfs/nfs3proc.c | 15 ++ fs/nfs/nfs4proc.c | 369 +++++++++++++++++++++++++++++++++-- fs/nfs/nfs4xdr.c | 49 +++++ fs/nfs/proc.c | 13 ...
The attached patch is a roll-up of the patch set and applies on top of 2.6.25-rc3. Dave
I have prepared the nfs-utils patch for those who want to test the code. I have attached it to this email and it applies on top of the nfs-utils git tree at commit hash 9dd9b68c4c44f0d9102eb85ee2fa36a8b7f638e3. The reply to this email will contain all of the kernel changes rolled into one patch. Dave
Two fields have been added to the nfs_fattr structure to carry the security
label and its length. This has raised the need to provide lifecycle management
for these values. This patch introduces two macros nfs_fattr_alloc and
nfs_fattr_fini which are used to allocate and destroy these fields inside the
nfs_fattr structure. These macros do not modify any other components of the
structure so nfs_fattr_init still has to be used on these structures. In the
event that CONFIG_SECURITY is not set these calls should compile away.
Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
---
fs/nfs/client.c | 16 ++++++
fs/nfs/dir.c | 25 ++++++++++
fs/nfs/getroot.c | 33 +++++++++++++
fs/nfs/inode.c | 16 ++++++
fs/nfs/namespace.c | 3 +
fs/nfs/nfs3proc.c | 15 ++++++
fs/nfs/nfs4proc.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++-
fs/nfs/proc.c | 13 +++++-
fs/nfs/super.c | 4 ++
include/linux/nfs_fs.h | 41 ++++++++++++++++
10 files changed, 283 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index c5c0175..aa93f9d 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -878,6 +878,8 @@ struct nfs_server *nfs_create_server(const struct nfs_parsed_mount_data *data,
struct nfs_fattr fattr;
int error;
+ memset(&fattr, 0, sizeof(struct nfs_fattr));
+
server = nfs_alloc_server();
if (!server)
return ERR_PTR(-ENOMEM);
@@ -928,10 +930,12 @@ struct nfs_server *nfs_create_server(const struct nfs_parsed_mount_data *data,
spin_unlock(&nfs_client_lock);
server->mount_time = jiffies;
+ nfs_fattr_fini(&fattr);
return server;
error:
nfs_free_server(server);
+ nfs_fattr_fini(&fattr);
return ERR_PTR(error);
}
@@ -1083,6 +1087,8 @@ struct nfs_server *nfs4_create_server(const struct nfs_parsed_mount_data *data,
dprintk("--> nfs4_create_server()\n");
+ memset(&fattr, 0, sizeof(struct nfs_fattr));
+
...I am wondering. Since the NFSv<4 code will never touch these two new fields is it necessary to put the initialization code into their code paths? It seems that we could only do the initializations in the NFSv4 code since it will be the only ones using it. Dave --
There are at least ten instances of the above. (Why do some of them use And a few of these. The preferred way to do this in Linux is as a static inline. - James -- James Morris <jmorris@namei.org> -
I forgot to CC Matt on the posting of the patch set (I figured he was on fsdevel or lkml) so I asked him about this and here is the response. "The allocation flag used depends on where the call is made. In some cases allocation is not permitted to wait, etc." I'm assuming he felt that the sections marked NOWAIT could not wait for memory allocation. If people want to review them, and find to the contrary we can figure out what the correct ones are. Dave --
The new method for pulling argument for NFS from mount is through a text
parsing system. This patch adds two new entries to the argument parsing code
"securlty_label" and "nosecurity_label". Even though we use text across the
user/kernel boundary internally we still pack a binary structure for mount info
to be passed around. We add a flag for use in the nfs{4,}_mount_data struct to
indicate that are using security labels. Finally we add the SELinux support to
mark the labeling method as native.
Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
---
fs/nfs/super.c | 10 +++++++++-
fs/nfsd/export.c | 3 +++
include/linux/nfs4_mount.h | 3 ++-
include/linux/nfs_mount.h | 3 ++-
security/selinux/hooks.c | 13 ++++++++++++-
5 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 1fb3818..f3e327e 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -75,6 +75,7 @@ enum {
Opt_acl, Opt_noacl,
Opt_rdirplus, Opt_nordirplus,
Opt_sharecache, Opt_nosharecache,
+ Opt_seclabel, Opt_noseclabel,
/* Mount options that take integer arguments */
Opt_port,
@@ -124,6 +125,8 @@ static match_table_t nfs_mount_option_tokens = {
{ Opt_nordirplus, "nordirplus" },
{ Opt_sharecache, "sharecache" },
{ Opt_nosharecache, "nosharecache" },
+ { Opt_seclabel, "security_label" },
+ { Opt_noseclabel, "nosecurity_label" },
{ Opt_port, "port=%u" },
{ Opt_rsize, "rsize=%u" },
@@ -779,7 +782,12 @@ static int nfs_parse_mount_options(char *raw,
case Opt_nosharecache:
mnt->flags |= NFS_MOUNT_UNSHARED;
break;
-
+ case Opt_seclabel:
+ mnt->flags |= NFS_MOUNT_SECURITY_LABEL;
+ break;
+ case Opt_noseclabel:
+ mnt->flags &= ~NFS_MOUNT_SECURITY_LABEL;
+ break;
case Opt_port:
if (match_int(args, &option))
return 0;
diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 8a6f7c9..d32ae56 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -...I've got patches that noone has seen because I haven't posted them yet (my test box crashed yesterday and I didn't have time to make sure it wasn't my new patches) you are going to need to rebase this against. Adding more nfs'isms to selinux code isn't a good thing in the long run. But, does this even really work? I thought both NFS and NFSv4 were actually passing around struct nfs_parsed_mount_data now rather than just nfs_mount_data. Maybe not, but this patch, although fine for testing isn't fine to go in. I'll get you and the list my new option interfaces on monday so we can get NFS out of all of the LSMs and get SELinux out of NFS. -Eric --
There currently doesn't exist a labeling type that is adequate for use with
labeled nfs. Since NFS doesn't really support xattrs we can't use the use xattr
labeling behavior. For this we developed a new labeling type. The native
labeling type is used solely by NFS to ensure nfs inodes are labeled at runtime
by the NFS code instead of relying on the SELinux security server on the client
end.
Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
---
security/selinux/hooks.c | 20 ++++++++++++--------
security/selinux/include/security.h | 2 ++
security/selinux/ss/policydb.c | 5 ++++-
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a56b21a..ebe4e18 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -300,13 +300,14 @@ extern int ss_initialized;
/* The file system's label must be initialized prior to use. */
-static char *labeling_behaviors[6] = {
+static char *labeling_behaviors[7] = {
"uses xattr",
"uses transition SIDs",
"uses task SIDs",
"uses genfs_contexts",
"not configured for labeling",
"uses mountpoint labeling",
+ "uses native labels",
};
static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry);
@@ -660,14 +661,15 @@ static int selinux_set_mnt_opts(struct super_block *sb, char **mount_options,
if (strcmp(sb->s_type->name, "proc") == 0)
sbsec->proc = 1;
- /* Determine the labeling behavior to use for this filesystem type. */
- rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid);
- if (rc) {
- printk(KERN_WARNING "%s: security_fs_use(%s) returned %d\n",
- __FUNCTION__, sb->s_type->name, rc);
- goto out;
+ if (!sbsec->behavior) {
+ /* Determine the labeling behavior to use for this filesystem type. */
+ rc = security_fs_use(sb->s_type->name, &sbsec->behavior, &sbsec->sid);
+ if (rc) {
+ printk...This patch adds a new recommended attribute named label into the NFSv4 file
attribute structure. It also adds several new flags to allow the
NFS client and server to determine if this attribute is supported and if it is
being sent over the wire.
Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
---
fs/nfs/nfs4proc.c | 3 +++
include/linux/nfs4.h | 2 ++
include/linux/nfs_fs_sb.h | 1 +
include/linux/nfs_xdr.h | 4 ++++
include/linux/nfsd/export.h | 5 +++--
include/linux/nfsd/nfsd.h | 7 ++++---
6 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7ce0786..42e5cf7 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -96,6 +96,9 @@ const u32 nfs4_fattr_bitmap[2] = {
| FATTR4_WORD1_TIME_ACCESS
| FATTR4_WORD1_TIME_METADATA
| FATTR4_WORD1_TIME_MODIFY
+#ifdef CONFIG_NFS_V4_SECURITY_LABEL
+ | FATTR4_WORD1_SECURITY_LABEL
+#endif
};
const u32 nfs4_statfs_bitmap[2] = {
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 8726491..af90403 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -21,6 +21,7 @@
#define NFS4_FHSIZE 128
#define NFS4_MAXPATHLEN PATH_MAX
#define NFS4_MAXNAMLEN NAME_MAX
+#define NFS4_MAXLABELLEN 255
#define NFS4_ACCESS_READ 0x0001
#define NFS4_ACCESS_LOOKUP 0x0002
@@ -348,6 +349,7 @@ enum lock_type4 {
#define FATTR4_WORD1_TIME_MODIFY (1UL << 21)
#define FATTR4_WORD1_TIME_MODIFY_SET (1UL << 22)
#define FATTR4_WORD1_MOUNTED_ON_FILEID (1UL << 23)
+#define FATTR4_WORD1_SECURITY_LABEL (1UL << 31)
#define NFSPROC4_NULL 0
#define NFSPROC4_COMPOUND 1
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 3423c67..8fdea18 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -125,5 +125,6 @@ struct nfs_server {
#define NFS_CAP_SYMLINKS (1U << 2)
#define NFS_CAP_ACLS (1U << 3)
#define NFS_CAP_A...I remember raising this before, but I think we need to try and find a better way to implement this than always allocating labels of a fixed and possibly too-small size. What about perhaps starting with a statically allocated array of say 64 bytes (I can't see any labels on my system larger than that), and then falling back to a a dynamic allocation of up to 32k if it turns out to be too small ? i.e. large labels are a slow path and there is no practical limit on label size. - James -- James Morris <jmorris@namei.org> -
Yes, that would be my preference as well - there shouldn't be any internal limits on the label size. -- Stephen Smalley National Security Agency --
I'm not convinced that it is worth all of that extra logic just to save some space on a transient data structure. 255 characters seems to be overkill to begin with considering you don't often get a label like the one below which is only 90 characters. thisismyuser_u:withseveralroles_r:andanoverlylongtype_t:s0-s12:c0,c1,c2,c3,c4,c5,c6,c7,c8 -
This patch adds two entries into the fs/KConfig file. The first entry NFS_V4_SECURITY_LABEL enables security label support for the NFSv4 client while the second entry NFSD_V4_SECURITY_LABEL enables security labeling support on the server side. These entries are currently marked as EXPERIMENTAL to indicate that at the moment they are only suitable for testing purposes and that the functionality they provide may change before it is removed from experimental status. Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov> --- fs/Kconfig | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/fs/Kconfig b/fs/Kconfig index d731282..d762375 100644 --- a/fs/Kconfig +++ b/fs/Kconfig @@ -1637,6 +1637,14 @@ config NFS_V4 If unsure, say N. +config NFS_V4_SECURITY_LABEL + bool "Provide Security Label support for NFSv4 client" + depends on NFS_V4 && SECURITY && EXPERIMENTAL + help + Say Y here if you want label attribute support for NFS version 4. + + If unsure, say N. + config NFS_DIRECTIO bool "Allow direct I/O on NFS files" depends on NFS_FS @@ -1728,6 +1736,15 @@ config NFSD_V4 should only be used if you are interested in helping to test NFSv4. If unsure, say N. +config NFSD_V4_SECURITY_LABEL + bool "Provide Security Label support for NFSv4 server" + depends on NFSD_V4 && SECURITY && EXPERIMENTAL + help + If you would like to include support for label file attributes + over NFSv4, say Y here. + + If unsure, say N. + config NFSD_TCP bool "Provide NFS server over TCP support" depends on NFSD -- 1.5.3.8 -
This patch adds two new fields to the iattr structure. The first field holds a
security label while the second contains the length of this label. In addition
the patch adds a new helper function inode_setsecurity which calls the LSM to
set the security label on the inode. Finally the patch modifies the necessary
functions such that fsnotify_change can handle notification requests for
dnotify and inotify.
Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
---
fs/attr.c | 43 +++++++++++++++++++++++++++++++++++++++++++
fs/xattr.c | 33 +++++++++++++++++++++++++++------
include/linux/fcntl.h | 1 +
include/linux/fs.h | 11 +++++++++++
include/linux/fsnotify.h | 6 ++++++
include/linux/inotify.h | 3 ++-
include/linux/xattr.h | 1 +
7 files changed, 91 insertions(+), 7 deletions(-)
diff --git a/fs/attr.c b/fs/attr.c
index 966b73e..1df6603 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -5,6 +5,7 @@
* changes by Thomas Schoebel-Theuer
*/
+#include <linux/fs.h>
#include <linux/module.h>
#include <linux/time.h>
#include <linux/mm.h>
@@ -14,9 +15,35 @@
#include <linux/fcntl.h>
#include <linux/quotaops.h>
#include <linux/security.h>
+#include <linux/xattr.h>
/* Taken over from the old code... */
+#ifdef CONFIG_SECURITY
+/*
+ * Update the in core label.
+ */
+int inode_setsecurity(struct inode *inode, struct iattr *attr)
+{
+ const char *suffix = security_maclabel_getname()
+ + XATTR_SECURITY_PREFIX_LEN;
+ int error;
+
+ if (!attr->ia_valid & ATTR_SECURITY_LABEL)
+ return -EINVAL;
+
+ error = security_inode_setsecurity(inode, suffix, attr->ia_label,
+ attr->ia_label_len, 0);
+ if (error)
+ printk(KERN_ERR "%s() %s %d security_inode_setsecurity() %d\n"
+ , __func__, (char *)attr->ia_label, attr->ia_label_len
+ , error);
+
+ return error;
+}
+EXPORT_SYMBOL(inode_setsecurity);
+#endif
+
/* POSIX ...Please don't overload setattr with this. Just looking at your callers shows that it's much cleaner as a separate method. Now what's really lacking is a desciption _why_ you actually need it to start with. The current method to set security labels is through the security.* xattrs. Now if we want to clean up that somewhat messy method that might be a good idea, but we should do it for all An any inotify/dnotify additions should be separate from the vfs to filesystem interface. Please make it a separate patch and describe ---end quoted text--- --
The main reason for this was the way that NFS passes information it receives around. If you look in patch 11 you will see that nfsd4_decode_fattr doesn't give us access to an inode to use for security_inode_setsecurity and it doesn't give us a dentry to use the xattr helpers with. The only thing we get here is an iattr structure which is then passed back up to fill in the inode fields. Also without functionality provided by patch 1 we don't even know where to put the Will do. We added them to conform to the functionality provided for other elements in the iattr structure. We will add a more robust --
Take a look at how ACLs are handled. They're passed up from the _decode operations into a small structure that is referenced by struct nfsd4_<operation> and pass it up until the level where the dentry ---end quoted text--- --
So this method will work on the server side and I will probably switch to it. However while working on switching over I found that the client side uses an iattr to pass inode information down into the protocol calls. So there are two options. Add this to the iattr structure and do this properly in a clean way. Or add additional params down the call chain into these protocol handlers for NFS. Which is the better option for this? Dave --
So after looking at this it seems that this is going to be a far more changes to NFS to set something that is an inode attribute. I can keep looking into it but it seems like it can be done much cleaner as an inode_setattr extension rather than adding new structures all over the nfs code. Dave --
Thanks for the heads up on this. This is partially the reason I wanted to post the set for feedback. If it pans out this will probably be a --
Do you mean: if (!(attr->ia_valid & ATTR_SECURITY_LABEL)) You're not checking the return value of inode_setsecurity(). Why not just rely on inode_setsecurity() to perform the check, then you can lose the #ifdefs in the core code & make it a noop for Similarly, make this a function which is compiled away for Put the #ifdef inside fsnotify_change() and only process I don't think there's any harm in always defining this. -- James Morris <jmorris@namei.org> -
I'm not clear as to what you are suggesting here. are you saying put the #ifdef CONFIG_SECURITY around inode_setsecurity and make the case where --
There is a time where we need to calculate a context without the
inode having been created yet. To do this we take the negative dentry and
calculate a context based on the process and the parent directory contexts.
Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
---
include/linux/security.h | 11 +++++++++++
security/dummy.c | 7 +++++++
security/security.c | 9 +++++++++
security/selinux/hooks.c | 38 ++++++++++++++++++++++++++++++++++++++
4 files changed, 65 insertions(+), 0 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index c80bee4..9038f34 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1264,6 +1264,8 @@ struct security_operations {
void (*sb_clone_mnt_opts) (const struct super_block *oldsb,
struct super_block *newsb);
+ int (*dentry_init_security) (struct dentry *dentry, int mode,
+ void **ctx, u32 *ctxlen);
int (*inode_alloc_security) (struct inode *inode);
void (*inode_free_security) (struct inode *inode);
int (*inode_init_security) (struct inode *inode, struct inode *dir,
@@ -1528,6 +1530,7 @@ int security_sb_set_mnt_opts(struct super_block *sb, char **mount_options,
void security_sb_clone_mnt_opts(const struct super_block *oldsb,
struct super_block *newsb);
+int security_dentry_init_security(struct dentry *dentry, int mode, void **ctx, u32 *ctxlen);
int security_inode_alloc(struct inode *inode);
void security_inode_free(struct inode *inode);
int security_inode_init_security(struct inode *inode, struct inode *dir,
@@ -1822,6 +1825,14 @@ static inline void security_sb_post_pivotroot (struct nameidata *old_nd,
struct nameidata *new_nd)
{ }
+static inline int security_dentry_init_security(struct dentry *dentry,
+ int mode,
+ void **ctx,
+ u32 *ctxlen)
+{
+ return 0;
+}
+
static inline int security_inode_alloc (struct inode *inode)
{
return 0;
diff --git a/security/dummy.c b/security/dummy.c...Before the inode_xattr_getsecurity call was removed the caller would
concatenate the security namespace prefix and the suffix provided by the lsm to
obtain the security xattr. This hook provides the functionality to obtain the full
LSM xattr name. The patch also provides implementations for the dummy security
module and SELinux. This method is used instead of restoring the old method
since it only requires an offset into the returned pointer to obtain the
suffix. This approach is more efficient than concatenating the security xattr
namespace string with the suffix to get a usable string.
Signed-off-by: David P. Quigley <dpquigl@tycho.nsa.gov>
---
include/linux/security.h | 8 ++++++++
security/dummy.c | 6 ++++++
security/security.c | 6 ++++++
security/selinux/hooks.c | 10 ++++++++--
4 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index fe52cde..c80bee4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1394,6 +1394,7 @@ struct security_operations {
int (*secid_to_secctx)(u32 secid, char **secdata, u32 *seclen);
int (*secctx_to_secid)(char *secdata, u32 seclen, u32 *secid);
void (*release_secctx)(char *secdata, u32 seclen);
+ const char *(*maclabel_getname) (void);
#ifdef CONFIG_SECURITY_NETWORK
int (*unix_stream_connect) (struct socket * sock,
@@ -1633,6 +1634,7 @@ int security_netlink_recv(struct sk_buff *skb, int cap);
int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
int security_secctx_to_secid(char *secdata, u32 seclen, u32 *secid);
void security_release_secctx(char *secdata, u32 seclen);
+const char *security_maclabel_getname(void);
#else /* CONFIG_SECURITY */
@@ -2316,6 +2318,12 @@ static inline int security_secctx_to_secid(char *secdata,
static inline void security_release_secctx(char *secdata, u32 seclen)
{
}
+
+static inline const char *security_maclabel_getname(void)
+{
+ return NULL;
+}
+
#...I am adding a new hook to provide the functionality that Casey suggested. It takes an inode, context, contextlen and sets it in the LSM. The question is that since there is a need to be able to set in-core, on-disk, or both; should these be two separate hooks? or should we make the hook take a flag that has in-core, and ondisk and it can be masked together for both? Dave --
I think that calling this a maclabel is a really bad idea. For one thing, it assumes that all interesting security attributes are for Mandatory Access Control. Also, it assumes that they are stored as xattrs. While these conditions are both met by the two current LSMs I would suggest that this is not a fair assumption for the long haul unless the intention is to lock the lSM into only supporting xattr based label based MAC modules. If you are only interested in supporting one LSM then the code should Casey Schaufler casey@schaufler-ca.com -
Actually that is a completely fair assumption. When this whole thing started it was mandated that security attributes be stored in xattrs. I originally had a more convoluted name but after asking around we thought this one was better. Not to mention this is a slightly reworked hook that was just removed from the LSM since there were no users. While I'm open to potentially changing the name the paradigm that we use the xattr functionality of linux to handle security labels has been around since the beginning of LSM. If we want to revisit that idea I'm willing to do -
A completely reasonable LSM would be a discretionary time lock. The owner could set or unset the times when a file might be accessed. Stored as an xattr, but neither a label nor Mandatory Access Control. I propose this as an example of why the name maclabel is inappropriate, because in this case the data involved is neither. Please also consider that, as horrible as it may seem, an LSM could legitimately require more than one xattr. A proper Compartmented Mode Workstation, for example, might have a MAC label and an Information label, and as anyone familiar with the CMW spec will tell you, they have to be separate. Granted, the information label is only supposed to be used to indicate the actual sensitivity of information, but if it's available someone is The paradigm is* a security "blob" which is meaningfull only to the security module proper. This is what allows SELinux to use secids and Smack to toss around text strings. It's not MAC data and it's not an NFS label, it's private to the LSM. It makes a lot of sense to use an xattr to store a blob but, as the AppArmor people have been known espouse, it's not the only way. The blob could be referenced from a table using the inode number (it has been done on other systems and works fine) rather than an xattr, in which case the whole "name" may be meaningless. ---- * It was when the whole thing started out at least. Casey Schaufler casey@schaufler-ca.com -
I think it might help for you to look at how the hook is actually used. It is specific to MAC labeling, and we do not want some random other security attribute name returned here that is for some purpose other than MAC labeling, like security.capability. -- Stephen Smalley National Security Agency --
I can see how it's being used just fine, thank you. If you only want this interface for SELinux put it in SELinux. Don't clutter up the LSM with it. If it's an LSM interface it should be potentially useful for any and all LSMs, be they label based or not, MAC or DAC. Even within a label based MAC scheme it may not be sensible, given that a MAC scheme could use multiple xattrs (e.g. a B&L sensitivity label and a Biba integrity label) to store its blob. If what you want in LSM terms is a name to give the blob make your interface be security_blob_name(). The LSM can deal with this as it sees fit, and NFS can determine if it's a blob that it wants to deal with independently. Such an interface could even support stacking should that ever come about. LSM is not supposed to be only for MAC and it's not supposed to be only for label based schemes. It's supposed to be for additional security restrictions. Providing an interface that should be generally applicable with a name that constrains it to a specific subset of those schemes is wrong. Casey Schaufler casey@schaufler-ca.com --
Casey, you aren't listening (why am I surprised?). This is an interface to be used by NFS to get information from the security module. The information desired is specific to the MAC labeling functionality in NFSv4 that is being proposed. That functionality is MAC specific (necessarily so, just like the ACL functionality is ACL specific). We are hiding the SELinux-specific bits behind the LSM interface, and non-MAC LSMs are free to return NULL in order to indicate that they don't support MAC labeling. We do NOT want the capability module to return its security blob here, or any other non-MAC LSM - it will yield the wrong semantics for the NFS MAC support. In any event, I don't think we need your permission. -- Stephen Smalley National Security Agency --
I think Casey is totally right here. The LSM interface should not be as specific here. If you want to limit the NFSv4 interface to single MAC xattr label based systems add an additional method to check if the LSM is that. But the proper fix is of course to not add somthing so specific to NFSv4 at all, as it's got enough shortcoming already. Please add a proper xattr protocol. It's not like it's hard, SGI has been doing this in IRIX for NFSv3 for ages as a sideband protocol, and even release the reference source under the GPL. Just either use that with NFSv4 or if you feel fancy merge it into the NFS spec for Wow, that's rude even to someone as direct as me. Casey is the only other person having an in-tree LSM, and I think his input in this area is important. But if not I as a VFS person can happily give you my "no" for the current version from the VFS point of view. --
Fair point - my apologies to Casey. -- Stephen Smalley National Security Agency --
Accepted. Now we work together like horses in troika. Casey Schaufler casey@schaufler-ca.com --
There are several things here. I've spoken to several people about this and the belief I've gotten from most of them is that a recommended attribute is how this is to be transported. The NFSv4 spec people will probably say that if you want xattr like functionality for NFSv4 use named attributes. For us this is not an option since we require semantics to label on create/open and the only way we can do this is by adding a recommended attribute. The create/open calls in NFSv4 takes a list of attributes to use on create as part of the request. I really don't see a difference between the security blob and the username/groupname that NFSv4 currently uses. Also there is a good chance that we will need to translate labels at some point (read future I can only speak for myself but honestly I've only seen Casey act confrontational to this idea from the beginning. There is absolutely nothing in here that is SELinux specific, tecnically its not even MAC specific. I said from the beginning that this was perhaps not the best name and we are willing to change it. There is nothing in this hook that wasn't in LSM before. This is almost identical functionality to what Adrian removed in 2.6.24. The only difference between this and security_inode_getsuffix is that this returns security.suffix and that the name is different. I don't have a SMACK box to test it on but I'm 99% sure that if Casey tried to use SMACK with this patch set that he would have labeled nfs working with SMACK. If it doesn't work with SMACK right now I'm willing to help him with that and even include it in the patch set. But spreading FUD about how we are including SELinux specific code in here is just that. Dave --
NAs are a non-starter here for a couple of reasons. 1. They are specified as being user managed and opaque to NFS. MAC labels are typically set by the OS, and may only be set by the user when permitted by MAC policy. The labels need to be interpreted by the OS to allow MAC policy to be enforced. 2. The NA namespace is arbitrary and opaque to the OS. There's no scope in NFSv4 design to allow a namespace to be specified for e.g. MAC labels, and trying to modify the spec to allow it seems impractical to me. It would at the very least break backward compatibility with clients and servers, and lead to some ugly hacks to try and ensure that systems were reliably speaking to peers which understood the namespace. It might be possible to implement Linux/BSD style xattrs for NFSv4, assuming that the IETF folk would approve of the idea, but I don't think this is really the right solution for conveying MAC labels across the wire. The xattr API as a local interface is pretty good for this (as it is FS independent, simple, and established), but that does not automatically translate to an xattr wire protocol being the right thing. The problem with this, I believe, is that you end up with quite a lot of overhead and complexity being added to NFSv4 which does not actually meet the requirements of MAC labeling, and like NAs, seems more suited for arbitrary user-managed metdata. Using RAs for MAC labels seems most appropriate, as they're simple, extensible and already used for similar protocol attributes such as ACLs, and other system-managed metadata. - James -- James Morris <jmorris@namei.org> --
That is simply because I don't care for your design and implementation choices, I think they're a bad way to go, I've suggested what I think you should do, and I'm sorry that that comes off as confrontational but that does not change what I see as flaws in your approach. I understand what you're trying to do and I think You're very possibly right. I am not argueing from what's right for Smack, I am argueing from what's right for the LSM. Smack is a label based MAC LSM, like SELinux. I would expect that it would be easy for Sorry, but I'm not argueing that it's SELinux specific at this point. I'm argueing that it's specific to single label stored in an xattr based MAC systems (a set of which both SELinux and Smack are members) and that it is file system specific to NFS. Any of these attributes makes it questionable as an LSM interface. As I said before, trying to be helpful, call it security_blob_name(), and the upcoming Discretionary Time Lock module can return NULL, indicating that it wants to share no blob name. Or call it security_xattr_names() and DTL can return NULL and B&L+Biba can return "security.Bell&LaPadula security.Biba", hoping that everyone who uses the interface accepts the blank seperation as an indication that there are multiple xattrs involved. I am saying that security_maclabel() is a bad choice, and I think that as an LSM (not MAC, not xattr, not NFS) interface it should serve the LSM, making the LSM interface better first, and being the specific interface that a particular file system finds convenient second. And before we go any further, I have personally been involved in doing labeled NFS three times, and I know where the bodies are buried. Your approach is fine for single label stored in xattr based MAC systems. It does not generalize worth catfish whiskers, whereas the two other schemes I've done do so flawlessly. I am critical of this approach only because I know that y'all can do better. Great. Now I owe the entire labeled NFS t...
I know but for some odd reason we kept arguing about it. Unless you want me to repost the patch on it's own with the name changed you are going I agree with your suggestion here but nowhere in earlier emails did you outline this. You just vaguely described a method that sounds like the selinux sidtab. If you had described it this way in the beginning we would have be done with after the first response. If we are going to work well in the future you need to be more clear when you make That is fine. I welcome constructive criticism but you have a tendency of being vague with what you mean and at times it comes off the wrong way. This is the whole reason the patch set was posted to begin with. We have been working on it for so long without much outside input so we --
Phew, he missed that one. Casey Schaufler casey@schaufler-ca.com --
Hehe I didn't miss it but I don't drink (A coke would be greatly --
Then use the existing side-band protocol and ignore the NFSv4 spec And changing the name and minor details is exactly what Casey requested. --
The reason we are trying to go through the standards process in the first place is that there is a desire to use SELinux with large netapp storage boxes. I don't believe that netapp supports the existing side-band protocol for NFSv4 and the impression I got was that they we were going to have an incredibly hard time convincing them of putting anything in that isn't part of the standard. It seems that adding one recommended attribute which is described in a 3 page internet draft(Most of which is BS that is part of the template) would be significantly easier to get into the standard than trying to push an out of band xattr protocol. Also, I believe Trond even expressed some discontent with it a I can always go with the original hook name of get_security_xattr_name which turns into a security_get_security_xattr_name call which seems a bit ludicrous. The only other complaint that I saw from Casey besides the name of the function was that there could be more than one xattr. If we want to address that then I need another hook that says give me all data that the LSM deems important for this file. Which is essentially the same thing as taking each of the xattr names that the module will provide, grabbing each of them in turn, and concatenating them together. For SELinux this is no different than getsecurity with the selinux suffix. The same goes for SMACK. Dave --
Most of the original SGI XFS team went to NetApp. The engineer who developed the side-band xattr protocol (last I heard he was a real estate speculator in Florida) spent lots of time with them. Easier may be pragmatic, but that does not make it right. I suggest, that in my opinion (there, is that sufficiently non-confrontational?) that Linux and the LSM are much better served by a general xattr protocol than by adding a single Well, that's why I keep suggesting security_blob_name. More precisely, I said that there could be a number other than one, with zero also being an option, and the possibility existing that the name of the blob might not be an xattr (it could be uid, gid, access That would be security_getblob(), would it not? True enough, but like I keep saying, those are both single label stored in an xattr based MAC systems. BTW, I prefer "Smack" to SMACK. Much less 1970's. Thank you. Casey Schaufler casey@schaufler-ca.com --
An xattr protocol is overkill for conveying a MAC label over the network, and would still not provide the required semantics. Please see prior discussion on this e.g. http://marc.info/?l=linux-kernel&m=120424789929258&w=2 Note that RAs are already used to convey ACLs and all other system-managed metatdata. i.e. an extensible, appropriate infrastructure already exists in the NFSv4 protocol, and has been used successfully for similar purposes. We do not need to add a new, generalized protocol to NFSv4 for this, especially one which does not meet the requirements. - James -- James Morris <jmorris@namei.org> --
You might be right that Linux and LSM are better served by this, but this has to be used by more than just Linux. Solaris has the new FMAC initiative (The F is silent) which will probably want to use this as well. SEBSD/SEDarwin also has a use for this and they have a MAC label It seems your argument is against using xattrs. Regardless of this hook the 0 xattr LSM is still borked by this. security_inode_getsecurity(..., suffix, ...). It is assumed that the fundamental function for getting security information takes an xattr suffix. Don't bother responding to this email eventually you will get to the one where I agree with you on --
What about Casey's suggestion of get_security_blob? For any reasonable module that just has a single xattr it's trivial and for those that have multiple or a different storage model it might get complicated but that's not our problem for now. --
Possibly I'm missing something, but if I'm implementing a security module that has any security attribute at all, e.g. capability module with security.capability, and I see a hook called "get_security_blob" or "get_security_attr" or the like, I'll implement that hook and return my attribute there. Which in turn will _break_ the labeled NFS functionality because it is expecting a MAC label specifically. The whole point here is that we do not want modules like capability to return their security attributes here, because this is to support labeled NFS functionality in support of enforcing MAC. I don't especially care about the hook name per se, but the interface (whatever it may be) needs to convey the proper semantics, and the semantics truly are MAC specific (and should be). -- Stephen Smalley National Security Agency --
BTW, to date, "security blob" has been used to refer to the structures allocated by the security modules referenced by the void* security fields in the kernel data structures (task, inode, ...), while "secctx" (security context) and "secid" (security id) have been leveraged by subsystems like audit, netlink and labeled IPSEC to represent security labels. -- Stephen Smalley National Security Agency --
If this is the method we are going to use then we need a corresponding set_security_blob as well. This seems like a paradigm shift for accessing security information in the kernel. I said to Casey in the beginning that I'd be willing to revisit it but that neither he or I alone could make the decision. Unless I misunderstood the original mandate for security information and that it only applies to how user space accesses it. Dave --
Well, yes, but look at David Howell's file cacheing work Sorry, I don't understand how user space and mandates go together here. Casey Schaufler casey@schaufler-ca.com --
What do you intend to do with this blob once you have it? Somehow it needs to be set on the other end. So unless you want each LSM I was inquiring if the mandate to use xattrs for security attributes was only for userspace's access to them and the kernel could create separate --
As I've told you several times before: we're _NOT_ putting private ioctl^Hxattrs onto the wire. If the protocol can't be described in an RFC, then it isn't going in no matter what expletive you choose to use... --
With the SGI supplied reference implementation it ought to be a small matter of work to write an RFC. If the information weren't SGI proprietary I could even tell you how long it ought to take a junior engineer in Melbourne to write. The fact that there is currently no RFC does not mean that there cannot be a RFC, only that no one has written (or published) one yet. Casey Schaufler casey@schaufler-ca.com --
NO! It is not a "small matter of work". The fact is that private crap like the 'security' and 'system' namespace makes describing 'xattr' as a protocol a non-starter and an interoperability nightmare. If you can't do better than xattr for a security protocol, then it is time to go look for another job... --
Ah, well, I don't understand why, but that's probably just Ok, I can buy that it doesn't fit in with the current protocol mindset, and that I for one have not demonstrated that it can be. I remember how upset the IETF got over the original CIPSO proposal not specifying which label tag value coresponded to But ... I don't have a job. You're being mean. (smiley) I think that we have a conflict between what works well for a filesystem (xattrs are really helpful) and what works well for a network protocol (undefined blobs of goo are atrocious) in the case of a network file system. From either standpoint the other is completely unworkable. It may be the case that for NFS the proposed scheme (delta LSM naming propriety, which is getting addressed) is the best we can do. NFS is an old protocol (older than some of the people reading this) and should be excused some shortcomings. Thank you. Casey Schaufler casey@schaufler-ca.com --
It's as unstructured as the named attributes already in. Or file data for that matter. --
Describing what is supposed to be a security mechanism in a structured fashion for use in a protocol should hardly be an impossible task (and AFAICS, Dave & co are making good progress in doing so). If it is, then that casts serious doubt on the validity of the security model... There should be no need for ioctls. --