Re: [RFC][PATCH] Version5 - Simplified mandatory access control kernel implementation
--- Joshua Brindle <[EMAIL PROTECTED]> wrote:
> Casey Schaufler wrote:
> >>> +static int smack_shm_associate(struct shmid_kernel *shp, int shmflg)
> >>> +{
> >>> + smack_t *ssp = smack_of_shm(shp);
> >>> + int rc;
> >>> +
> >>> + if (ssp == NULL)
> >>> + return 0;
> >>> +
> >>> + rc = smk_curacc(ssp, MAY_READWRITE);
> >>> + return rc;
> >>> +}
> >>>
> >> No read-only or write-only shm mappings?
> >>
> >
> > Still thinking about it.
> >
> >
>
> I think if you actually want to use this in a guard like policy you are
> going to need it (for at least shm and msgq).
Fair enough. Ok, I'm convinced. On the work queue it goes.
> BTW, you never responded
> to my last email about the granularity required to make a high
> throughput front channel and a low bandwidth backchannel for guards.
That's true. I'd like to wait until I have an answer that makes sense,
and as you've been following the thread you know that I have lots of
things to work out. I haven't forgotten you.
Casey Schaufler
[EMAIL PROTECTED]
-
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: [RFC][PATCH] Version5 - Simplified mandatory access control kernel implementation
Casey Schaufler wrote:
+static int smack_shm_associate(struct shmid_kernel *shp, int shmflg)
+{
+ smack_t *ssp = smack_of_shm(shp);
+ int rc;
+
+ if (ssp == NULL)
+ return 0;
+
+ rc = smk_curacc(ssp, MAY_READWRITE);
+ return rc;
+}
No read-only or write-only shm mappings?
Still thinking about it.
I think if you actually want to use this in a guard like policy you are
going to need it (for at least shm and msgq). BTW, you never responded
to my last email about the granularity required to make a high
throughput front channel and a low bandwidth backchannel for guards.
-
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: [RFC][PATCH] Version5 - Simplified mandatory access control kernel implementation
--- Seth Arnold <[EMAIL PROTECTED]> wrote:
> On Sun, Jul 22, 2007 at 09:44:49PM -0700, Casey Schaufler wrote:
> >
> > I appears that everyone else took the weekend to read
> > "Deathly Hallows"* as it's been pretty quiet here. Well,
> > my wife took first dibs on our copy so I did some polishing
> > on smack instead. Since no one complained about the size last
>
> Sorry to hear she got to it first :) I'll try to not spoil it..
I got there before anyone spoiled it.
> > +/*
> > + * mapping for symlinks
> > +#define SMK_TMPPATH_SIZE32
> > +#define SMK_TMPPATH_ROOT"/moldy/"
>
> This define isn't used here, is it used in the userspace?
Now used (below)
> > + /*
> > +* There will be only one smackfs. Casey says so.
> > +*/
> > + smk_sb = sb;
>
> :)
>
> > + /*
> > +* Create a directory for /smack/tmp
> > +*/
> > + smk_add_symlink("tmp", "/moldy/");
>
> I'm confused :)
Using the constant now. I must have gotten sidetracked.
>
> > +/*
> > + * Inode smack data
> > + */
> > +struct inode_smack {
> > + smack_t smk_inode; /* label of the fso */
> > + spinlock_t smk_lock; /* initialization lock */
> > + int smk_flags; /* smack inode flags */
> > +};
>
> No chance to use the inode's locks?
I don't see one that would work, but then, I haven't
demonstrated a great amount of lock-savey here.
> > +/*
> > + * There are not enough CAP bits available to make this
> > + * real, so Casey borrowed the capability that looks to
> > + * him like it has the best balance of similarity amd
> > + * low use.
> > + */
> > +#define CAP_MAC_OVERRIDE CAP_LINUX_IMMUTABLE
>
> Perhaps someday we should explore 64 bit cap space..
Soon.
> > +static smack_t *free_smack_t(smack_t *sp)
> > +{
> > +if (sp != NULL)
> > +kfree(sp);
>
> kfree already does this checking :)
I fixed.
> > +return NULL;
> > +}
> > +
>
> > +static void smack_sb_free_security(struct super_block *sb)
> > +{
> > + if (sb->s_security == NULL)
> > + return;
> > +
> > + kfree(sb->s_security);
> > + sb->s_security = NULL;
> > +}
>
> kfree doesn't need this, but it does avoid setting a NULL back to NULL.
I fixed.
> > +static int smack_sb_kern_mount(struct super_block *sb, void *data)
> > +{
> > + int rc;
> > + struct dentry *root = sb->s_root;
> > + struct inode *inode = root->d_inode;
> > + struct superblock_smack *sp = sb->s_security;
> > + struct inode_smack *isp;
> > + smack_t smack = SMK_UNSET;
> > + char *op;
> > + char *commap;
> > +
> > + if (sp == NULL) {
> > + rc = smack_sb_alloc_security(sb);
> > + if (rc != 0)
> > + return rc;
> > + }
> > + if (sp->smk_initialized != 0)
> > + return 0;
> > + if (inode == NULL)
> > + return 0;
> > +
> > + sp->smk_initialized = 1;
> > +
> > + /*
> > +* Not everyone supports extended attributes.
> > +*/
> > + if (inode->i_op->getxattr != NULL) {
> > + rc = smk_fetch(inode, root, &smack);
> > + switch (rc) {
> > + case 0:
> > + case -ENODATA:
> > + case -EOPNOTSUPP:
> > + break;
> > + case sizeof(smack_t):
>
> What does this case do?
They entire smk_fetch thing is obsolete code. It's gone now.
> > + default:
> > + printk(KERN_WARNING "%s:%d (dev %s, type "
> > + "%s) \"%s\" rc = %d\n", __FUNCTION__, __LINE__,
> > + sb->s_id, sb->s_type->name, (char *)&smack, rc);
> > + break;
> > + }
> > + }
>
> > + switch (sbp->s_magic) {
> > + case SMACK_MAGIC:
> > + /*
> > +* Casey says that it's a little embarassing
> > +* that the smack file system doesn't do
> > +* extended attributes.
> > +*/
>
> :)
>
> > + case DEVPTS_SUPER_MAGIC:
> > + /*
> > +* Casey says this is here because
> > +* devpts is "clean" of security hooks.
> > +*
> > +* pty's need to be handled for real.
> > +* Using "*" is expedient for bring-up.
> > +*
> > +*/
> > + final = *csp;
> > + break;
>
> Old comment?
Fixed.
> > +static void smack_sk_free_security(struct sock *sk)
> > +{
> > + if (sk->sk_security == NULL)
> > + return;
> > +
> > + kfree(sk->sk_security);
> > + sk->sk_security = NULL;
> > +}
>
> kfree's fine with NULL
Stop saying that!
> > +static void smack_to_secattr(smack_t smack, struct netlbl_lsm_secattr
> *nlsp)
> > +{
> > + struct smk_cipso_entry *scp;
> > +
> > + switch (smack_net_nltype) {
> > + case NETLBL_NLTYPE_CIPSOV4:
> > + nlsp->domain = NULL;
> > + nlsp->flags = NETLBL_SECATTR_DOMAIN;
> > + nlsp->flags |= NETLBL_SECATTR_MLS_LVL;
> > +
> > + for (scp = smack_cipso; scp != NULL; s
Re: [RFC][PATCH] Version5 - Simplified mandatory access control kernel implementation
On Sun, Jul 22, 2007 at 09:44:49PM -0700, Casey Schaufler wrote:
>
> I appears that everyone else took the weekend to read
> "Deathly Hallows"* as it's been pretty quiet here. Well,
> my wife took first dibs on our copy so I did some polishing
> on smack instead. Since no one complained about the size last
Sorry to hear she got to it first :) I'll try to not spoil it..
> +/*
> + * mapping for symlinks
> +#define SMK_TMPPATH_SIZE32
> +#define SMK_TMPPATH_ROOT"/moldy/"
This define isn't used here, is it used in the userspace?
> + /*
> + * There will be only one smackfs. Casey says so.
> + */
> + smk_sb = sb;
:)
> + /*
> + * Create a directory for /smack/tmp
> + */
> + smk_add_symlink("tmp", "/moldy/");
I'm confused :)
> +/*
> + * Inode smack data
> + */
> +struct inode_smack {
> + smack_t smk_inode; /* label of the fso */
> + spinlock_t smk_lock; /* initialization lock */
> + int smk_flags; /* smack inode flags */
> +};
No chance to use the inode's locks?
> +/*
> + * There are not enough CAP bits available to make this
> + * real, so Casey borrowed the capability that looks to
> + * him like it has the best balance of similarity amd
> + * low use.
> + */
> +#define CAP_MAC_OVERRIDE CAP_LINUX_IMMUTABLE
Perhaps someday we should explore 64 bit cap space..
> +static smack_t *free_smack_t(smack_t *sp)
> +{
> +if (sp != NULL)
> +kfree(sp);
kfree already does this checking :)
> +return NULL;
> +}
> +
> +static void smack_sb_free_security(struct super_block *sb)
> +{
> + if (sb->s_security == NULL)
> + return;
> +
> + kfree(sb->s_security);
> + sb->s_security = NULL;
> +}
kfree doesn't need this, but it does avoid setting a NULL back to NULL.
> +static int smack_sb_kern_mount(struct super_block *sb, void *data)
> +{
> + int rc;
> + struct dentry *root = sb->s_root;
> + struct inode *inode = root->d_inode;
> + struct superblock_smack *sp = sb->s_security;
> + struct inode_smack *isp;
> + smack_t smack = SMK_UNSET;
> + char *op;
> + char *commap;
> +
> + if (sp == NULL) {
> + rc = smack_sb_alloc_security(sb);
> + if (rc != 0)
> + return rc;
> + }
> + if (sp->smk_initialized != 0)
> + return 0;
> + if (inode == NULL)
> + return 0;
> +
> + sp->smk_initialized = 1;
> +
> + /*
> + * Not everyone supports extended attributes.
> + */
> + if (inode->i_op->getxattr != NULL) {
> + rc = smk_fetch(inode, root, &smack);
> + switch (rc) {
> + case 0:
> + case -ENODATA:
> + case -EOPNOTSUPP:
> + break;
> + case sizeof(smack_t):
What does this case do?
> + default:
> + printk(KERN_WARNING "%s:%d (dev %s, type "
> +"%s) \"%s\" rc = %d\n", __FUNCTION__, __LINE__,
> +sb->s_id, sb->s_type->name, (char *)&smack, rc);
> + break;
> + }
> + }
> + switch (sbp->s_magic) {
> + case SMACK_MAGIC:
> + /*
> + * Casey says that it's a little embarassing
> + * that the smack file system doesn't do
> + * extended attributes.
> + */
:)
> + case DEVPTS_SUPER_MAGIC:
> + /*
> + * Casey says this is here because
> + * devpts is "clean" of security hooks.
> + *
> + * pty's need to be handled for real.
> + * Using "*" is expedient for bring-up.
> + *
> + */
> + final = *csp;
> + break;
Old comment?
> +static void smack_sk_free_security(struct sock *sk)
> +{
> + if (sk->sk_security == NULL)
> + return;
> +
> + kfree(sk->sk_security);
> + sk->sk_security = NULL;
> +}
kfree's fine with NULL
> +static void smack_to_secattr(smack_t smack, struct netlbl_lsm_secattr *nlsp)
> +{
> + struct smk_cipso_entry *scp;
> +
> + switch (smack_net_nltype) {
> + case NETLBL_NLTYPE_CIPSOV4:
> + nlsp->domain = NULL;
> + nlsp->flags = NETLBL_SECATTR_DOMAIN;
> + nlsp->flags |= NETLBL_SECATTR_MLS_LVL;
> +
> + for (scp = smack_cipso; scp != NULL; scp = scp->smk_next)
> + if (scp->smk_smack == smack)
> + break;
> +
> + if (scp != NULL) {
> + nlsp->mls_lvl = scp->smk_level;
> + smack_set_catset(scp->smk_catset, nlsp);
> + }
> + else {
} else { please
> + nlsp->mls_lvl = smack_cipso_direct;
> + smack_set_catset(smack, nlsp);
> + }
> + break;
> + case NETLBL_NLTYPE_NONE:
> + ca
Re: [RFC][PATCH] Version5 - Simplified mandatory access control kernel implementation
+
+/*
+ * I hope these are the hokeyist lines of code in the module. Casey.
+ */
+#define DEVPTS_SUPER_MAGIC 0x1cd1
+#define SOCKFS_MAGIC 0x534F434B
+#define PIPEFS_MAGIC 0x50495045
+#define TMPFS_MAGIC0x01021994
+ /*
+* This is pretty hackish.
+* Casey says that we shouldn't have to do
+* file system specific code, but it does help
+* with keeping it simple.
+*/
+ switch (sbp->s_magic) {
+ case SMACK_MAGIC:
...
Rather than using the filesystem magic numbers, why not just map the
filesystem type string to a desired labeling behavior upon sb_kern_mount
and then use that behavior in d_instantiate, as is done in SELinux? For
SELinux, the mapping is maintained in policy, but naturally you could
define a fixed mapping in your module if you prefer (and in fact,
SELinux originally did that too).
Or if you are going to use the magic numbers, I think you need separate
patches to move the definitions from their current locations to
include/linux/magic.h and then #include that file in both places rather
than replicating their definitions in your code.
Another option would be to push the responsibility to the filesystems,
e.g. define some new flags for file_system_type struct that indicate the
right behavior for labeling inodes, and have the security module only
use those flags rather than needing to know about individual filesystem
types.
--
Stephen Smalley
National Security Agency
-
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
