Re: [RFC][Patch 1/1] IBAC Patch
On Tue, 2007-06-19 at 17:23 -0500, Serge E. Hallyn wrote:
> > +#define get_file_security(file) ((unsigned long)(file->f_security))
> > +#define set_file_security(file, val) (file->f_security = (void *)val)
> > +
> > +#define get_task_security(task) ((unsigned long)(task->security))
> > +#define set_task_security(task, val) (task->security = (void *)val)
>
> I understand the above are likely remnants of stacker lsm support, and I
> hate to say this, but not only is having those in there going to be
> frowned upon, it then leads you later on to do things like
>
> if (get_file_security(file)==0)
>
> when using 0 for null upsets people in itself.
Instead of allocating memory for file->f_security, it uses
file->f_security itself, for storing 0 or 1. So it isn't
checking to see if it is NULL per se.
> > +
> > +#define XATTR_MEASURE_SUFFIX "measure"
> > +#define XATTR_MEASURE_SUFFIX_LEN (sizeof (XATTR_MEASURE_SUFFIX) -1)
> > +
> > +struct ibac_isec_data {
> > + int mmapped;/* no. of times inode mmapped */
> > + int measure;/* inode tagged to be measured */
> > + spinlock_t lock;/* protect inode state */
> > +};
> > +
> > +static int ibac_inode_alloc_security(struct inode *inode)
> > +{
> > + struct ibac_isec_data *isec;
> > +
> > + isec = kzalloc(sizeof(struct ibac_isec_data), GFP_KERNEL);
> > + if (!isec)
> > + return -ENOMEM;
> > +
> > + spin_lock_init(&isec->lock);
> > + inode->i_security = isec;
>
> Heh, for file and task security you use the above helpers, but for inode
> you do it like this? :) Please replace all x_security assignments and
> checks with this format.
For file and task, the code uses the security field itself to store
information as opposed to allocating storage for it. In the case of
inode, it is using it for both the original digsig mmap tracking and
now to tag the inode that it needs to be measured. Tagging the inode
to be measured is based on the existence of the security.measure xattr,
which is controlled by a userspace application. The difference is
that storage is allocated for inode->i_security.
> > +/*
> > + * For all inodes allocate inode->i_security(isec), before the security
> > + * subsystem is enabled.
> > + */
> > +static void ibac_fixup_inodes(void)
> > +{
> > + struct super_block *sb;
> > +
> > + spin_lock(&sb_lock);
> > + list_for_each_entry(sb, &super_blocks, s_list) {
> > + struct inode *inode;
> > +
> > + spin_unlock(&sb_lock);
> > +
> > + spin_lock(&inode_lock);
> > + list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> > + spin_unlock(&inode_lock);
> > +
> > + spin_lock(&inode->i_lock);
> > + if (!inode->i_security)
> > + ibac_inode_alloc_security(inode);
>
> since ibac_inode_alloc_security can return -ENOMEM, maybe you should at
> least check for that condition and warn the user?
Yes, that definitely is a good idea. Will do.
> > + spin_unlock(&inode->i_lock);
> > +
> > + spin_lock(&inode_lock);
> > + }
> > + spin_unlock(&inode_lock);
> > +
> > + spin_lock(&sb_lock);
> > + }
> > + spin_unlock(&sb_lock);
> > +}
> > +
> > +static int ibac_inode_permission(struct inode *inode, int mask,
> > +struct nameidata *nd)
> > +{
> > + struct ibac_isec_data *isec = inode->i_security;
> > + struct dentry *dentry;
> > + char *path = NULL;
> > + char *fname = NULL;
> > + int rc = 0;
> > + int measure;
> > +
> > + dentry = (!nd || !nd->dentry) ? d_find_alias(inode) : nd->dentry;
> > + if (!dentry)
> > + return 0;
> > + if (nd) { /* preferably use fullname */
> > + path = (char *)__get_free_page(GFP_KERNEL);
> > + if (path)
> > + fname = d_path(nd->dentry, nd->mnt, path, PAGE_SIZE);
> > + }
> > +
> > + if (!fname) /* no choice, use short name */
> > + fname = (!dentry->d_name.name) ? (char *)dentry->d_iname :
> > + (char *)dentry->d_name.name;
>
> Please separate the above out into a helper function.
Ok.
> This name is only ever used for debugging, right? I didn't miss any
> place where the name is used for some security decision?
Correct, the filename is not used for any security decision, but it
is passed to integrity_measure(), which the IMA integrity provider
associates with the given hash value. To see the filename hints
'cat /sys/kernel/security/ima/ascii_runtime_measurements'.
> > +
> > + /* Measure labeled files */
> > + spin_lock(&isec->lock);
> > + measure = isec->measure;
> > + spin_unlock(&isec->lock);
> > +
> > + if (S_ISREG(inode->i_mode) && (measure == 1)
> > + && (mask & MAY_READ)) {
> > + rc = verify_metadata_integrity(dentry);
> > + if (rc == 0)
> > + rc = verify_and_measure_integr
Re: [RFC][Patch 1/1] IBAC Patch
Quoting Mimi Zohar ([EMAIL PROTECTED]): > This is a re-release of Integrity Based Access Control(IBAC) LSM module > which bases access control decisions on the new integrity framework > services. IBAC is a sample LSM module to help clarify the interaction > between LSM and Linux Integrity Modules(LIM). > > New to this release of IBAC is digsig's functionality of preventing > files open for write to be mmapped, and files that are mmapped from being > opened for write. > > IBAC originally verified/measured executables only in the > bprm_check_security() hook. By only doing the verification/measurement > in bprm_check_security(), libraries could be loaded without first being > verified/measured. This release of IBAC, files are also verified/measured > in the file_mmap() hook, which catches the libraries, and inode_permission() > hook, which verifies/measures files tagged, by a userspace application, > with the extended attribute 'security.measure'. > > IBAC can be included or excluded in the kernel configuration. If > included in the kernel and IBAC_BOOTPARAM is enabled, IBAC can also be > enabled/disabled on the kernel command line with 'ibac='. > > IBAC can be configured to either verify and enforce integrity or to just log > integrity failures on the kernel command line with 'ibac_enforce='. When > IBAC_BOOTPARAM is enabled, the default is only to log integrity failures. > > Signed-off-by: Mimi Zohar <[EMAIL PROTECTED]> > --- > > Index: linux-2.6.22-rc4-mm2/security/ibac/Kconfig > === > --- /dev/null > +++ linux-2.6.22-rc4-mm2/security/ibac/Kconfig > @@ -0,0 +1,54 @@ > +config SECURITY_IBAC > + boolean "IBAC support" > + depends on SECURITY && SECURITY_NETWORK && INTEGRITY > + help > + Integrity Based Access Control(IBAC) uses the Linux > + Integrity Module(LIM) API calls to verify an executable's > + metadata and data's integrity. Based on the results, > + execution permission is permitted/denied. Integrity > + providers may implement the LIM hooks differently. For > + more information on integrity verification refer to the > + specific integrity provider documentation. > + > +config SECURITY_IBAC_BOOTPARAM > + bool "IBAC boot parameter" > + depends on SECURITY_IBAC > + default n > + help > + This option adds a kernel parameter 'ibac', which allows IBAC > + to be disabled at boot. If this option is selected, IBAC > + functionality can be disabled with ibac=0 on the kernel > + command line. The purpose of this option is to allow a > + single kernel image to be distributed with IBAC built in, > + but not necessarily enabled. > + > + If you are unsure how to answer this question, answer N. > + > +config SECURITY_IBAC_BOOTPARAM_VALUE > + int "IBAC boot parameter default value" > + depends on SECURITY_IBAC_BOOTPARAM > + range 0 1 > + default 0 > + help > + This option sets the default value for the kernel parameter > + 'ibac', which allows IBAC to be enabled at boot. If this > + option is set to 1 (one), the IBAC kernel parameter will > + default to 1, enabling IBAC at bootup. If this option is > + set to 0 (zero), the IBAC kernel parameter will default to 0, > + disabling IBAC at bootup. > + > + If you are unsure how to answer this question, answer 0. > + > +config SECURITY_IBAC_ENFORCE > + bool "IBAC integrity enforce boot parameter" > + depends on SECURITY_IBAC_BOOTPARAM > + default y > + help > + This option adds a kernel parameter 'ibac_enforce', which > + allows integrity enforcement to be enabled/disabled at boot. > + If this option is selected, integrity enforcement can be > + enabled with ibac_enforce=1 on the kernel command line. > + The default is not to enforce integrity, but simply log > + integrity verification errors. > + > + If you are unsure how to answer this question, answer Y. > Index: linux-2.6.22-rc4-mm2/security/ibac/Makefile > === > --- /dev/null > +++ linux-2.6.22-rc4-mm2/security/ibac/Makefile > @@ -0,0 +1,6 @@ > +# > +# Makefile for building IBAC > +# > + > +obj-$(CONFIG_SECURITY_IBAC) += ibac.o > +ibac-y := ibac_main.o > Index: linux-2.6.22-rc4-mm2/security/ibac/ibac_main.c > === > --- /dev/null > +++ linux-2.6.22-rc4-mm2/security/ibac/ibac_main.c > @@ -0,0 +1,434 @@ > +/* > + * Integrity Based Access Control(IBAC) sample LSM module calling LIM hooks. > + * > + * Copyright (C) 2007 IBM Corporation > + * Author: Mimi Zohar <[EMAIL PROTECTED]> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundati
Re: [RFC] [Patch 1/1] IBAC Patch
On Wed, Mar 14, 2007 at 07:25:26AM -0400, Mimi Zohar wrote:
> It's a little bit of both. :-) Initially it was written to help me with
:)
> implementing and testing the integrity provider. But it could definitely
> stand
> on it's own. As Serge Hallyn commented http://lkml.org/lkml/2007/3/13/220,
> by
> adding the mmap hook, IBAC could replace the LSM aspect of digsig and a gpg
> based integrity provider, could be written, instead of EVM, which is TPM
> based.
Thanks.
> > > + if (status != INTEGRITY_PASS) { /* FAIL | NO_LABEL */
> > > + if (!is_kernel_thread(current)) {
> >
> > Please remind me why kernel threads are exempt?
>
> You really don't want to prevent kernel threads from working. Nasty things
> happen.
But under what conditions would a kernel thread not pass integrity? I
guess if it doesn't have an associated dentry... or the dentry refers
to something else? (What does knfsd do -- it is started by a userland
program which causes the kernel to start up some tasks for NFS..)
> For integrity_measure(), EVM calls IMA, if enabled, to extend the
> measurement list with the hash value it provides. In most cases, EVM
> has already calculated the hash value, when it was called to verify the
> data. integrity_measure() is not meant to be intrusive, so it is defined
> as void.
Oh, ok, thanks.
> Thank you for your comments.
My pleasure, thanks for the quick responses.
pgp2cRAic45PX.pgp
Description: PGP signature
Re: [RFC] [Patch 1/1] IBAC Patch
On Tue, 2007-03-13 at 19:27 -0700, Seth Arnold wrote:
> On Thu, Mar 08, 2007 at 05:58:16PM -0500, Mimi Zohar wrote:
> > This is a request for comments for a new Integrity Based Access
> > Control(IBAC) LSM module which bases access control decisions
> > on the new integrity framework services.
>
> Thanks Mimi, nice to see an example of how the integrity framework ought
> to be used.
>
> > (Hopefully this will help clarify the interaction between an LSM
> > module and LIM module.)
>
> Is this module intended to clarify an interface, or be useful in and of
> itself?
It's a little bit of both. :-) Initially it was written to help me with
implementing and testing the integrity provider. But it could definitely stand
on it's own. As Serge Hallyn commented http://lkml.org/lkml/2007/3/13/220, by
adding the mmap hook, IBAC could replace the LSM aspect of digsig and a gpg
based integrity provider, could be written, instead of EVM, which is TPM based.
> > Index: linux-2.6.21-rc3-mm2/security/ibac/Makefile
> > ===
> > --- /dev/null
> > +++ linux-2.6.21-rc3-mm2/security/ibac/Makefile
> > @@ -0,0 +1,6 @@
> > +#
> > +# Makefile for building IBAC
> > +#
> > +
> > +obj-$(CONFIG_SECURITY_IBAC) += ibac.o
> > +ibac-y := ibac_main.o
> > Index: linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> > ===
> > --- /dev/null
> > +++ linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> > @@ -0,0 +1,126 @@
> > +/*
> > + * Integrity Based Access Control (IBAC)
> > + *
> > + * Copyright (C) 2007 IBM Corporation
> > + * Author: Mimi Zohar <[EMAIL PROTECTED]>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > modify
> > + * it under the terms of the GNU General Public License as published
> > by
> > + * the Free Software Foundation, version 2 of the License.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#ifdef CONFIG_SECURITY_IBAC_BOOTPARAM
> > +int ibac_enabled = CONFIG_SECURITY_IBAC_BOOTPARAM_VALUE;
> > +
> > +static int __init ibac_enabled_setup(char *str)
> > +{
> > + ibac_enabled = simple_strtol(str, NULL, 0);
> > + return 1;
> > +}
> > +
> > +__setup("ibac=", ibac_enabled_setup);
> > +#else
> > +int ibac_enabled = 0;
> > +#endif
>
> If the command line option isn't enabled, how will ibac_enabled ever be
> set to '1'? Have I overlooked or forgotten some helper routine elsewhere?
I guess I was a bit over zealous in preventing IBAC from running
unintentionally. Will fix in the next IBAC patch release.
> > +static unsigned int integrity_enforce = 0;
> > +static int __init integrity_enforce_setup(char *str)
> > +{
> > + integrity_enforce = simple_strtol(str, NULL, 0);
> > + return 1;
> > +}
> > +
> > +__setup("ibac_enforce=", integrity_enforce_setup);
> > +
> > +#define XATTR_NAME "security.evm.hash"
>
> Is this name unique to this IBAC module? Or should it be kept in sync
> with the integrity framework?
In order to verify the metadata integrity, an xattr needs to be
specified on the integrity_verify_metadata() call. As IBAC does not
define an xattr of its own, I used this one, which at the time worked.
But as EVM is only one integrity provider, this probably is not a good
idea. To resolve this problem, I guess the integrity_verify_metadata()
API call should respond without requiring an actual xattr.
> > +static inline int is_kernel_thread(struct task_struct *tsk)
> > +{
> > + return (!tsk->mm) ? 1 : 0;
> > +}
> > +
> > +static int ibac_bprm_check_security(struct linux_binprm *bprm)
> > +{
> > + struct dentry *dentry = bprm->file->f_dentry;
> > + int xattr_len;
> > + char *xattr_value = NULL;
> > + int rc, status;
> > +
> > + rc = integrity_verify_metadata(dentry, XATTR_NAME,
> > + &xattr_value, &xattr_len, &status);
> > + if (rc < 0 && rc == -EOPNOTSUPP) {
> > + kfree(xattr_value);
> > + return 0;
> > + }
> > +
> > + if (rc < 0) {
> > + printk(KERN_INFO "verify_metadata %s failed "
> > + "(rc: %d - status: %d)\n", bprm->filename, rc, status);
> > + if (!integrity_enforce)
> > + rc = 0;
> > + goto out;
> > + }
> > + if (status != INTEGRITY_PASS) { /* FAIL | NO_LABEL */
> > + if (!is_kernel_thread(current)) {
>
> Please remind me why kernel threads are exempt?
You really don't want to prevent kernel threads from working. Nasty things
happen.
> > + printk(KERN_INFO "verify_metadata %s "
> > + "(Integrity status: FAIL)\n", bprm->filename);
>
> Integrity status may be FAIL or NO_LABEL at this point -- would it be
> more useful to report the whole truth?
FAIL here indicates that the integrity of the metadata was bad, while
NOLABEL indicates, in the case of EVM, that there was no HMAC. I'll
upd
Re: [RFC] [Patch 1/1] IBAC Patch
On Tue, 2007-03-13 at 10:31 -0500, Serge E. Hallyn wrote: > Quoting Mimi Zohar ([EMAIL PROTECTED]): > > On Thu, 2007-03-08 at 22:19 -0500, [EMAIL PROTECTED] wrote: > > > On Thu, 08 Mar 2007 17:58:16 EST, Mimi Zohar said: > > > > This is a request for comments for a new Integrity Based Access > > > > Control(IBAC) LSM module which bases access control decisions > > > > on the new integrity framework services. > > > > > > > > (Hopefully this will help clarify the interaction between an LSM > > > > module and LIM module.) > > > > > > OK, between this and the additional LIM hooks I didn't notice in an > > > earlier > > > patch, we're starting to see the API. The only problem is that although > > > it may be the right API for *your* code, I suspect it's a non-starter > > > without > > > a discussion about whether it's the right *generic* API for an LIM (which > > > will > > > require at least one dramatic bun fight about what "Integrity" means). > > > > Absolutely, we need to make sure that the set of LIM hooks is complete and > > that > > nothing is missing in order to implement different types of LIM providers. > > I'm > > copying the digsig mailing list for their input on requirements, which this > > API > > might not satisfy or perhaps address. > > (Could you resend the IBAC patch to the disec list as well?) Sure, I'm just about to post a new version of IBAC with the changes based on comments from the lsm and lkml lists. > Is IBAC basically a 'demo' lsm? It only hooks bprm_security, so you > can't execute anything with a bad hash, right? So really if you were to > also hook mmap, this would completely do away with the need for digsig? > IBAC is automatically able to use any integrity measurement modules you > write, whether they use gpg like digsig does right now, or use the tpm? You're absolutely correct on all accounts. > It also shows how simple it would be to hook selinux, though I guess > there are several ways we might want to hook selinux - 1. to check only > security.selinux xattrs, 2. to check integrity of entry point types, 3. > to check integrity of any files labeled by the integrity measurement > module as needing to be checked. Correct. Verifying the integrity of security.selinux xattrs would require adding security.selinux to /etc/evm.conf and calls within SELinux to integrity_verify_metadata() as needed. Verifying the integrity of files, would require labeling the files with integrity measurements and adding calls within SELinux to integrity_verify_data() as needed. > Is there any way for the LSM itself to direct which data and metadata > will be measured? It looks like this is done by separately configuring > the integrity measurement module - which seems fine to me. Actually the current EVM implementation as an integrity provider relies on an LSM module, or for that matter any other kernel module, to make calls to the integrity API: integrity_verify_metadata(), integrity_verify_data(), and integrity_measure(). The decision as to which files are to be measured is left up to the LSM module. For example, as you previously noted, IBAC only verifies the metadata and data integrity of executables, whereas SLIM verifies the metadata and data integrity of SYSTEM level files and all executables. Mimi Zohar - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] [Patch 1/1] IBAC Patch
On Thu, Mar 08, 2007 at 05:58:16PM -0500, Mimi Zohar wrote:
> This is a request for comments for a new Integrity Based Access
> Control(IBAC) LSM module which bases access control decisions
> on the new integrity framework services.
Thanks Mimi, nice to see an example of how the integrity framework ought
to be used.
> (Hopefully this will help clarify the interaction between an LSM
> module and LIM module.)
Is this module intended to clarify an interface, or be useful in and of
itself?
> Index: linux-2.6.21-rc3-mm2/security/ibac/Makefile
> ===
> --- /dev/null
> +++ linux-2.6.21-rc3-mm2/security/ibac/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for building IBAC
> +#
> +
> +obj-$(CONFIG_SECURITY_IBAC) += ibac.o
> +ibac-y := ibac_main.o
> Index: linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> ===
> --- /dev/null
> +++ linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> @@ -0,0 +1,126 @@
> +/*
> + * Integrity Based Access Control (IBAC)
> + *
> + * Copyright (C) 2007 IBM Corporation
> + * Author: Mimi Zohar <[EMAIL PROTECTED]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, version 2 of the License.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#ifdef CONFIG_SECURITY_IBAC_BOOTPARAM
> +int ibac_enabled = CONFIG_SECURITY_IBAC_BOOTPARAM_VALUE;
> +
> +static int __init ibac_enabled_setup(char *str)
> +{
> + ibac_enabled = simple_strtol(str, NULL, 0);
> + return 1;
> +}
> +
> +__setup("ibac=", ibac_enabled_setup);
> +#else
> +int ibac_enabled = 0;
> +#endif
If the command line option isn't enabled, how will ibac_enabled ever be
set to '1'? Have I overlooked or forgotten some helper routine elsewhere?
> +static unsigned int integrity_enforce = 0;
> +static int __init integrity_enforce_setup(char *str)
> +{
> + integrity_enforce = simple_strtol(str, NULL, 0);
> + return 1;
> +}
> +
> +__setup("ibac_enforce=", integrity_enforce_setup);
> +
> +#define XATTR_NAME "security.evm.hash"
Is this name unique to this IBAC module? Or should it be kept in sync
with the integrity framework?
> +static inline int is_kernel_thread(struct task_struct *tsk)
> +{
> + return (!tsk->mm) ? 1 : 0;
> +}
> +
> +static int ibac_bprm_check_security(struct linux_binprm *bprm)
> +{
> + struct dentry *dentry = bprm->file->f_dentry;
> + int xattr_len;
> + char *xattr_value = NULL;
> + int rc, status;
> +
> + rc = integrity_verify_metadata(dentry, XATTR_NAME,
> +&xattr_value, &xattr_len, &status);
> + if (rc < 0 && rc == -EOPNOTSUPP) {
> + kfree(xattr_value);
> + return 0;
> + }
> +
> + if (rc < 0) {
> + printk(KERN_INFO "verify_metadata %s failed "
> +"(rc: %d - status: %d)\n", bprm->filename, rc, status);
> + if (!integrity_enforce)
> + rc = 0;
> + goto out;
> + }
> + if (status != INTEGRITY_PASS) { /* FAIL | NO_LABEL */
> + if (!is_kernel_thread(current)) {
Please remind me why kernel threads are exempt?
> + printk(KERN_INFO "verify_metadata %s "
> +"(Integrity status: FAIL)\n", bprm->filename);
Integrity status may be FAIL or NO_LABEL at this point -- would it be
more useful to report the whole truth?
> + if (integrity_enforce) {
> + rc = -EACCES;
> + goto out;
> + }
> + }
> + }
> +
> + rc = integrity_verify_data(dentry, &status);
> + if (rc < 0) {
> + printk(KERN_INFO "%s verify_data failed "
> +"(rc: %d - status: %d)\n", bprm->filename, rc, status);
> + if (!integrity_enforce)
> + rc = 0;
> + goto out;
> + }
> + if (status != INTEGRITY_PASS) {
> + if (!is_kernel_thread(current)) {
Please remind me why kernel threads are exempt?
> + printk(KERN_INFO "verify_data %s "
> +"(Integrity status: FAIL)\n", bprm->filename);
Same question about FAIL vs NO_LABEL.. (Would NO_LABEL be caught by a
failing verify_metadata above?)
> + if (integrity_enforce) {
> + rc = -EACCES;
> + goto out;
> + }
> + }
> + }
> +
> + kfree(xattr_value);
> +
> + /* measure all integrity level executables */
> + integrity_measure(dentry, bprm->filename, MAY_EXEC);
> + return 0;
If integrity_measure() fails (can it fail?) is allowing the exec still the
right approach? (I seem to recall that "measuring
Re: [RFC] [Patch 1/1] IBAC Patch
Quoting Mimi Zohar ([EMAIL PROTECTED]): > On Thu, 2007-03-08 at 22:19 -0500, [EMAIL PROTECTED] wrote: > > On Thu, 08 Mar 2007 17:58:16 EST, Mimi Zohar said: > > > This is a request for comments for a new Integrity Based Access > > > Control(IBAC) LSM module which bases access control decisions > > > on the new integrity framework services. > > > > > > (Hopefully this will help clarify the interaction between an LSM > > > module and LIM module.) > > > > OK, between this and the additional LIM hooks I didn't notice in an earlier > > patch, we're starting to see the API. The only problem is that although > > it may be the right API for *your* code, I suspect it's a non-starter > > without > > a discussion about whether it's the right *generic* API for an LIM (which > > will > > require at least one dramatic bun fight about what "Integrity" means). > > Absolutely, we need to make sure that the set of LIM hooks is complete and > that > nothing is missing in order to implement different types of LIM providers. > I'm > copying the digsig mailing list for their input on requirements, which this > API > might not satisfy or perhaps address. (Could you resend the IBAC patch to the disec list as well?) Is IBAC basically a 'demo' lsm? It only hooks bprm_security, so you can't execute anything with a bad hash, right? So really if you were to also hook mmap, this would completely do away with the need for digsig? IBAC is automatically able to use any integrity measurement modules you write, whether they use gpg like digsig does right now, or use the tpm? It also shows how simple it would be to hook selinux, though I guess there are several ways we might want to hook selinux - 1. to check only security.selinux xattrs, 2. to check integrity of entry point types, 3. to check integrity of any files labeled by the integrity measurement module as needing to be checked. Is there any way for the LSM itself to direct which data and metadata will be measured? It looks like this is done by separately configuring the integrity measurement module - which seems fine to me. -serge - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] [Patch 1/1] IBAC Patch
On Thu, 2007-03-08 at 22:19 -0500, [EMAIL PROTECTED] wrote: > On Thu, 08 Mar 2007 17:58:16 EST, Mimi Zohar said: > > This is a request for comments for a new Integrity Based Access > > Control(IBAC) LSM module which bases access control decisions > > on the new integrity framework services. > > > > (Hopefully this will help clarify the interaction between an LSM > > module and LIM module.) > > OK, between this and the additional LIM hooks I didn't notice in an earlier > patch, we're starting to see the API. The only problem is that although > it may be the right API for *your* code, I suspect it's a non-starter without > a discussion about whether it's the right *generic* API for an LIM (which will > require at least one dramatic bun fight about what "Integrity" means). Absolutely, we need to make sure that the set of LIM hooks is complete and that nothing is missing in order to implement different types of LIM providers. I'm copying the digsig mailing list for their input on requirements, which this API might not satisfy or perhaps address. > > Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig > > Minor congnitive-dissonance alert: > > > +config SECURITY_IBAC_BOOTPARAM > > + bool "IBAC boot parameter" > > + depends on SECURITY_IBAC > > + default y > > > + If you are unsure how to answer this question, answer N. > > The 'default' should in general match the hint we give the user. Oops, blush. It will obviously be corrected in the next IBAC patch release. Mimi Zohar - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] [Patch 1/1] IBAC Patch
On Fri, 09 Mar 2007 08:19:36 -0500 Mimi Zohar wrote:
> On Thu, 2007-03-08 at 15:08 -0800, Randy Dunlap wrote:
> > On Thu, 08 Mar 2007 17:58:16 -0500 Mimi Zohar wrote:
> >
> > > This is a request for comments for a new Integrity Based Access
> > > Control(IBAC) LSM module which bases access control decisions
> > > on the new integrity framework services.
> > >
> > > (Hopefully this will help clarify the interaction between an LSM
> > > module and LIM module.)
> > >
> > > Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig
> > > ===
> > > --- /dev/null
> > > +++ linux-2.6.21-rc3-mm2/security/ibac/Kconfig
> > > @@ -0,0 +1,36 @@
> > > +config SECURITY_IBAC
> > > + boolean "IBAC support"
> > > + depends on SECURITY && SECURITY_NETWORK && INTEGRITY
> > > + help
> > > + Integrity Based Access Control(IBAC) implements integrity
> > > + based access control.
> >
> > Please make the help text do more than repeat the words I B A C...
> > Put a short explanation or say something like:
> > See Documentation/security/foobar.txt for more information.
> > (and add that file)
>
> Agreed. Perhaps something like:
>
> Integrity Based Access Control(IBAC) uses the Linux Integrity
> Module(LIM) API calls to verify an executable's metadata and
> data's integrity. Based on the results, execution permission
> is permitted/denied. Integrity providers may implement the
> LIM hooks differently. For more information on integrity
> verification refer to the specific integrity provider
> documentation.
Yes, thanks.
> > > +config SECURITY_IBAC_BOOTPARAM
> > > + bool "IBAC boot parameter"
> > > + depends on SECURITY_IBAC
> > > + default y
> > > + help
> > > + This option adds a kernel parameter 'ibac', which allows IBAC
> > > + to be disabled at boot. If this option is selected, IBAC
> > > + functionality can be disabled with ibac=0 on the kernel
> > > + command line. The purpose of this option is to allow a
> > > + single kernel image to be distributed with IBAC built in,
> > > + but not necessarily enabled.
> > > +
> > > + If you are unsure how to answer this question, answer N.
> >
> > What's the downside to having this always builtin instead of
> > yet another config option?
>
> The ability of changing LSM modules at runtime might be perceived
> as problematic.
>
> > > +static struct security_operations ibac_security_ops = {
> > > + .bprm_check_security = ibac_bprm_check_security
> > > +};
> > > +
> > > +static int __init init_ibac(void)
> > > +{
> > > + int rc;
> > > +
> > > + if (!ibac_enabled)
> > > + return 0;
> > > +
> > > + rc = register_security(&ibac_security_ops);
> > > + if (rc != 0)
> > > + panic("IBAC: Unable to register with kernel\n");
> >
> > Normally we would not want to see a panic() from a register_xyz()
> > failure, but I guess you are arguing that an ibac register_security()
> > failure needs to halt everything??
>
> Yes, as this implies that another LSM module registered the hooks first,
> preventing IBAC from registering itself.
>
> Thank you for your other comments. They'll be addressed in the next
> ibac patch release.
Okie.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] [Patch 1/1] IBAC Patch
Quoting [EMAIL PROTECTED] ([EMAIL PROTECTED]): > On Thu, 08 Mar 2007 17:58:16 EST, Mimi Zohar said: > > This is a request for comments for a new Integrity Based Access > > Control(IBAC) LSM module which bases access control decisions > > on the new integrity framework services. > > > > (Hopefully this will help clarify the interaction between an LSM > > module and LIM module.) > > OK, between this and the additional LIM hooks I didn't notice in an earlier > patch, we're starting to see the API. The only problem is that although > it may be the right API for *your* code, I suspect it's a non-starter without > a discussion about whether it's the right *generic* API for an LIM (which will > require at least one dramatic bun fight about what "Integrity" means). Casey's earlier message suggested this too. 'Integrity' here in particular does not mean online integrity guarantees through, i.e., information flow control. So perhaps instead of 'integrity' we should make sure to always say 'integrity measurement'. Of course then there is already the 'integrity measurement architecture' which is only one implementation of a LIM module, right? So it would need to be renamed to TIMA (TPM-enabled IMA) or something I guess. -serge - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] [Patch 1/1] IBAC Patch
On Thu, 2007-03-08 at 15:08 -0800, Randy Dunlap wrote:
> On Thu, 08 Mar 2007 17:58:16 -0500 Mimi Zohar wrote:
>
> > This is a request for comments for a new Integrity Based Access
> > Control(IBAC) LSM module which bases access control decisions
> > on the new integrity framework services.
> >
> > (Hopefully this will help clarify the interaction between an LSM
> > module and LIM module.)
> >
> > Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig
> > ===
> > --- /dev/null
> > +++ linux-2.6.21-rc3-mm2/security/ibac/Kconfig
> > @@ -0,0 +1,36 @@
> > +config SECURITY_IBAC
> > + boolean "IBAC support"
> > + depends on SECURITY && SECURITY_NETWORK && INTEGRITY
> > + help
> > + Integrity Based Access Control(IBAC) implements integrity
> > + based access control.
>
> Please make the help text do more than repeat the words I B A C...
> Put a short explanation or say something like:
> See Documentation/security/foobar.txt for more information.
> (and add that file)
Agreed. Perhaps something like:
Integrity Based Access Control(IBAC) uses the Linux Integrity
Module(LIM) API calls to verify an executable's metadata and
data's integrity. Based on the results, execution permission
is permitted/denied. Integrity providers may implement the
LIM hooks differently. For more information on integrity
verification refer to the specific integrity provider
documentation.
> > +config SECURITY_IBAC_BOOTPARAM
> > + bool "IBAC boot parameter"
> > + depends on SECURITY_IBAC
> > + default y
> > + help
> > + This option adds a kernel parameter 'ibac', which allows IBAC
> > + to be disabled at boot. If this option is selected, IBAC
> > + functionality can be disabled with ibac=0 on the kernel
> > + command line. The purpose of this option is to allow a
> > + single kernel image to be distributed with IBAC built in,
> > + but not necessarily enabled.
> > +
> > + If you are unsure how to answer this question, answer N.
>
> What's the downside to having this always builtin instead of
> yet another config option?
The ability of changing LSM modules at runtime might be perceived
as problematic.
> > +static struct security_operations ibac_security_ops = {
> > + .bprm_check_security = ibac_bprm_check_security
> > +};
> > +
> > +static int __init init_ibac(void)
> > +{
> > + int rc;
> > +
> > + if (!ibac_enabled)
> > + return 0;
> > +
> > + rc = register_security(&ibac_security_ops);
> > + if (rc != 0)
> > + panic("IBAC: Unable to register with kernel\n");
>
> Normally we would not want to see a panic() from a register_xyz()
> failure, but I guess you are arguing that an ibac register_security()
> failure needs to halt everything??
Yes, as this implies that another LSM module registered the hooks first,
preventing IBAC from registering itself.
Thank you for your other comments. They'll be addressed in the next
ibac patch release.
Mimi Zohar
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] [Patch 1/1] IBAC Patch
On Thu, 08 Mar 2007 17:58:16 EST, Mimi Zohar said: > This is a request for comments for a new Integrity Based Access > Control(IBAC) LSM module which bases access control decisions > on the new integrity framework services. > > (Hopefully this will help clarify the interaction between an LSM > module and LIM module.) OK, between this and the additional LIM hooks I didn't notice in an earlier patch, we're starting to see the API. The only problem is that although it may be the right API for *your* code, I suspect it's a non-starter without a discussion about whether it's the right *generic* API for an LIM (which will require at least one dramatic bun fight about what "Integrity" means). > Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig Minor congnitive-dissonance alert: > +config SECURITY_IBAC_BOOTPARAM > + bool "IBAC boot parameter" > + depends on SECURITY_IBAC > + default y > + If you are unsure how to answer this question, answer N. The 'default' should in general match the hint we give the user. pgpTmJITtijDg.pgp Description: PGP signature
Re: [RFC] [Patch 1/1] IBAC Patch
On Thu, 08 Mar 2007 17:58:16 -0500 Mimi Zohar wrote:
> This is a request for comments for a new Integrity Based Access
> Control(IBAC) LSM module which bases access control decisions
> on the new integrity framework services.
>
> (Hopefully this will help clarify the interaction between an LSM
> module and LIM module.)
>
> Index: linux-2.6.21-rc3-mm2/security/ibac/Kconfig
> ===
> --- /dev/null
> +++ linux-2.6.21-rc3-mm2/security/ibac/Kconfig
> @@ -0,0 +1,36 @@
> +config SECURITY_IBAC
> + boolean "IBAC support"
> + depends on SECURITY && SECURITY_NETWORK && INTEGRITY
> + help
> + Integrity Based Access Control(IBAC) implements integrity
> + based access control.
Please make the help text do more than repeat the words I B A C...
Put a short explanation or say something like:
See Documentation/security/foobar.txt for more information.
(and add that file)
> +config SECURITY_IBAC_BOOTPARAM
> + bool "IBAC boot parameter"
> + depends on SECURITY_IBAC
> + default y
> + help
> + This option adds a kernel parameter 'ibac', which allows IBAC
> + to be disabled at boot. If this option is selected, IBAC
> + functionality can be disabled with ibac=0 on the kernel
> + command line. The purpose of this option is to allow a
> + single kernel image to be distributed with IBAC built in,
> + but not necessarily enabled.
> +
> + If you are unsure how to answer this question, answer N.
What's the downside to having this always builtin instead of
yet another config option?
> +config SECURITY_IBAC_BOOTPARAM_VALUE
> + int "IBAC boot parameter default value"
> + depends on SECURITY_IBAC_BOOTPARAM
> + range 0 1
> + default 0
> + help
> + This option sets the default value for the kernel parameter
> + 'ibac', which allows IBAC to be disabled at boot. If this
> + option is set to 0 (zero), the IBAC kernel parameter will
> + default to 0, disabling IBAC at bootup. If this option is
> + set to 1 (one), the IBAC kernel parameter will default to 1,
> + enabling IBAC at bootup.
> +
> + If you are unsure how to answer this question, answer 0.
> +
> Index: linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> ===
> --- /dev/null
> +++ linux-2.6.21-rc3-mm2/security/ibac/ibac_main.c
> @@ -0,0 +1,126 @@
> +/*
> + * Integrity Based Access Control (IBAC)
> + *
> + * Copyright (C) 2007 IBM Corporation
> + * Author: Mimi Zohar <[EMAIL PROTECTED]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, version 2 of the License.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#ifdef CONFIG_SECURITY_IBAC_BOOTPARAM
> +int ibac_enabled = CONFIG_SECURITY_IBAC_BOOTPARAM_VALUE;
static int ?
> +static int __init ibac_enabled_setup(char *str)
> +{
> + ibac_enabled = simple_strtol(str, NULL, 0);
> + return 1;
> +}
> +
> +__setup("ibac=", ibac_enabled_setup);
> +#else
> +int ibac_enabled = 0;
static int ?
> +#endif
> +
> +static unsigned int integrity_enforce = 0;
Don't init to 0 (not needed, consumes some binary file space).
> +static int __init integrity_enforce_setup(char *str)
> +{
> + integrity_enforce = simple_strtol(str, NULL, 0);
> + return 1;
> +}
> +
> +__setup("ibac_enforce=", integrity_enforce_setup);
> +
> +#define XATTR_NAME "security.evm.hash"
> +
> +static inline int is_kernel_thread(struct task_struct *tsk)
> +{
> + return (!tsk->mm) ? 1 : 0;
> +}
> +
> +static int ibac_bprm_check_security(struct linux_binprm *bprm)
> +{
> + struct dentry *dentry = bprm->file->f_dentry;
> + int xattr_len;
> + char *xattr_value = NULL;
> + int rc, status;
> +
> + rc = integrity_verify_metadata(dentry, XATTR_NAME,
> +&xattr_value, &xattr_len, &status);
> + if (rc < 0 && rc == -EOPNOTSUPP) {
justif (rc == -EOPNOTSUPP)
?
> + kfree(xattr_value);
> + return 0;
> + }
> +
> + if (rc < 0) {
> + printk(KERN_INFO "verify_metadata %s failed "
> +"(rc: %d - status: %d)\n", bprm->filename, rc, status);
How about adding "ibac: " to the beginning of each printk string,
so that someone will know the general source of these messages?
> + if (!integrity_enforce)
> + rc = 0;
> + goto out;
> + }
> + if (status != INTEGRITY_PASS) { /* FAIL | NO_LABEL */
> + if (!is_kernel_thread(current)) {
> + printk(KERN_INFO "verify_metadata %s "
> +"(Integrity status: FAIL)\n", bprm->filename);
> + if (integrity_enforce) {
> +
