| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1305570044.2669.69.camel@localhost.localdomain>
Date: Mon, 16 May 2011 14:20:44 -0400
From: Mimi Zohar <zohar@...ux.vnet.ibm.com>
To: Steven Whitehouse <swhiteho@...hat.com>
Cc: linux-security-module@...r.kernl.org, cluster-devel@...hat.com,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
James Morris <jmorris@...ei.org>,
David Safford <safford@...son.ibm.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Greg KH <greg@...ah.com>,
Dmitry Kasatkin <dmitry.kasatkin@...ia.com>,
Mimi Zohar <zohar@...ibm.com>,
Stephen Smalley <sds@...ho.nsa.gov>,
Eric Paris <eparis@...hat.com>,
Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH v5 13/21] evm: add evm_inode_post_init call in gfs2
On Mon, 2011-05-16 at 18:57 +0100, Steven Whitehouse wrote:
> Hi,
>
> On Mon, 2011-05-16 at 13:50 -0400, Mimi Zohar wrote:
> > On Mon, 2011-05-16 at 12:35 -0400, Mimi Zohar wrote:
> > > On Mon, 2011-05-16 at 17:14 +0100, Steven Whitehouse wrote:
> > > > Hi,
> > > >
> > > > On Mon, 2011-05-16 at 11:50 -0400, Mimi Zohar wrote:
> > > > > On Mon, 2011-05-16 at 16:30 +0100, Steven Whitehouse wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Mon, 2011-05-16 at 10:45 -0400, Mimi Zohar wrote:
> > > > > > > After creating the initial LSM security extended attribute, call
> > > > > > > evm_inode_post_init_security() to create the 'security.evm'
> > > > > > > extended attribute.
> > > > > > >
> > > > > > > Signed-off-by: Mimi Zohar <zohar@...ibm.com>
> > > > > > > ---
> > > > > > > fs/gfs2/inode.c | 28 +++++++++++++++++++---------
> > > > > > > 1 files changed, 19 insertions(+), 9 deletions(-)
> > > > > > >
> > > > > > [snip]
> > > > > > > + struct xattr lsm_xattr;
> > > > > > > + struct xattr evm_xattr;
> > > > > > >
> > > > > > > err = security_inode_init_security(&ip->i_inode, &dip->i_inode, qstr,
> > > > > > > - &name, &value, &len);
> > > > > > > + &lsm_xattr.name, &lsm_xattr.value,
> > > > > > > + &lsm_xattr.value_len);
> > > > > > >
> > > > > > > if (err) {
> > > > > > > if (err == -EOPNOTSUPP)
> > > > > > > @@ -780,11 +781,20 @@ static int gfs2_security_init(struct gfs2_inode *dip, struct gfs2_inode *ip,
> > > > > > > return err;
> > > > > > > }
> > > > > > >
> > > > > > > - err = __gfs2_xattr_set(&ip->i_inode, name, value, len, 0,
> > > > > > > - GFS2_EATYPE_SECURITY);
> > > > > > > - kfree(value);
> > > > > > > - kfree(name);
> > > > > > > -
> > > > > > > + err = __gfs2_xattr_set(&ip->i_inode, lsm_xattr.name, lsm_xattr.value,
> > > > > > > + lsm_xattr.value_len, 0, GFS2_EATYPE_SECURITY);
> > > > > > > + if (err < 0)
> > > > > > > + goto out;
> > > > > > > + err = evm_inode_post_init_security(&ip->i_inode, &lsm_xattr,
> > > > > > > + &evm_xattr);
> > > > > > > + if (err)
> > > > > > > + goto out;
> > > > > > > + err = __gfs2_xattr_set(&ip->i_inode, evm_xattr.name, evm_xattr.value,
> > > > > > > + evm_xattr.value_len, 0, GFS2_EATYPE_SECURITY);
> > > > > > > + kfree(evm_xattr.value);
> > > > > > > +out:
> > > > > > > + kfree(lsm_xattr.name);
> > > > > > > + kfree(lsm_xattr.value);
> > > > > > > return err;
> > > > > > > }
> > > > > > >
> > > > > >
> > > > > > Just wondering whether we could have a single call to the security
> > > > > > subsystem which returns a vector of xattrs rather than having to call
> > > > > > two different functions?
> > > > > >
> > > > > > Steve.
> > > > >
> > > > > There are a number of places that the LSM function is called immediately
> > > > > followed by either EVM/IMA. In each of those places it is hidden from
> > > > > the caller by calling the security_inode_XXX_security(). In this case
> > > > > each fs has it's own method of creating an extended attribute. If that
> > > > > method could be passed to security_inode_init_security, then
> > > > > security_inode_init_security() could call both the LSM and EVM functions
> > > > > directly.
> > > > >
> > > > > Mimi
> > > > >
> > > >
> > > > I'm still not quite sure I understand... from a (very brief) look at the
> > > > paper, it seems that what you are trying to do is add a new xattr to
> > > > inodes which has some hash of some of the inode metadata (presumably
> > > > including the selinux xattr and some other fields).
> > >
> > > Yes, for the time being the other metadata is i_ino, i_generation,
> > > i_uid, i_gid, and i_mode. The IMA-appriasal extension would store the
> > > file hash as an extended attribute. The digital-signature extension
> > > would store a digitial signature instead of the hash.
> > >
> > > > I'm not sure why it matters whether the selinux data has been written to
> > > > the buffers before the xattr containing the hash? The data will not
> > > > change (I hope!) and if it does presumably the hash will pick that up
> > > > when it is checked at a later date?
> > >
> > > In this case it doesn't matter, as there aren't any other xattrs at this
> > > point. When the file closes, the file hash would be written out as
> > > security.ima, causing security.evm to be updated to reflect the change.
> > >
> > > > The reason I'm asking is that currently the creation of GFS2 inodes is
> > > > broken down into a number of transactions, carefully designed to ensure
> > > > that the correct clean up occurs if there is an error. I would like to
> > > > try and reduce the number of transactions during the create process
> > > > where possible. That means I would like to move to a model which looks
> > > > like this:
> > > >
> > > > 1. Calculate number of blocks required, based on inode + xattrs (if any)
> > > > 2. Allocate blocks
> > > > 3. Populate with data (i.e. set xattrs)
> > > >
> > > > I'm trying to work out whether there is some reason why we have to use
> > > > your proposed:
> > > >
> > > > 1. Get selinux xattr
> > > > 2. Set selinux xattr
> > > > 3. Get EVM xattr
> > > > 4. Set EVM xattr
> > > >
> > > > as opposed to getting all the xattrs in a single call and then being
> > > > able to set them all in a single operation, if that makes sense?
> > > >
> > > > Steve.
> > >
> > > Yes, it makes sense.
> >
> > Just to clarify (and am cc'ing Stephen, Eric, and Casey).
> >
> > Instead of:
> >
> > int security_inode_init_security(struct inode *inode, struct inode *dir,
> > const struct qstr *qstr, char **name,
> > void **value, size_t *len);
> >
> > You're suggesting changing the interface to something like:
> >
> > int security_inode_init_security(struct inode *inode, struct inode *dir,
> > const struct qstr *qstr, struct xattr **xattrs);
> >
> > where 'struct xattr' is defined as (9th patch):
> >
> > --- a/include/linux/xattr.h
> > +++ b/include/linux/xattr.h
> > @@ -70,6 +70,12 @@ struct xattr_handler {
> > size_t size, int flags, int handler_flags);
> > };
> >
> > +struct xattr {
> > + char *name;
> > + void *value;
> > + size_t value_len;
> > +};
> > +
> > ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t);
> > ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
> > ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
> >
> > xattrs would be null terminated. The fs would be responsible for freeing the xattrs?
> >
> > thanks,
> >
> > Mimi
> >
>
> Yes, if that makes sense... I got the impression from the paper that
> there is the possibility of more xattrs being added in future and this
> way the fs end of things wouldn't have to change again when that
> happens. I'm still trying to get my head around it all, but it seems a
> cleaner solution to me - though I may well be missing something still,
>
> Steve.
At this point at least, the only other xattr would be security.ima,
which isn't created/updated until __fput() is called.
Your suggestion of security_inode_init_security() returning multiple
xattrs is a cleaner solution for EVM, but such a change requires the LSM
folks approval.
thanks,
Mimi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists