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 <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/kernel.h>
> +#include <linux/security.h>
> +#include <linux/integrity.h>
> +
> +#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) {
just if (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) {
> + 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)) {
> + printk(KERN_INFO "verify_data %s "
> + "(Integrity status: FAIL)\n", bprm->filename);
> + if (integrity_enforce) {
> + rc = -EACCES;
> + goto out;
> + }
> + }
> + }
> +
> + kfree(xattr_value);
> +
> + /* measure all integrity level executables */
> + integrity_measure(dentry, bprm->filename, MAY_EXEC);
> + return 0;
> + out:
Don't "hide" labels by indenting them so much. You don't need to
indent them at all, or maybe 1 character/column.
> + kfree(xattr_value);
> + return rc;
> +}
> +
> +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??
> + return rc;
> +}
> +
> +security_initcall(init_ibac);
---
~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/
