Re: Extending a Security Module

From: jmjonesat_private
Date: Wed May 23 2001 - 07:34:43 PDT

  • Next message: Matt Block: "stackable modules"

    On Tue, 22 May 2001, Chris Wright wrote:
    > * jmjonesat_private (jmjonesat_private) wrote:
    > > On Tue, 22 May 2001, Chris Wright wrote:
    > <snip>
    > > > my_module_task_alloc(task)
    > > > {
    > > > 	task->security = kmalloc(sizeof(my_big_security_blob, GFP_KERNEL);
    > > > 
    > > > 	/* this will overwrite the opaque security blob, so you'd
    > > > 	 * better save off your stuff.  how do you combine them back
    > > > 	 * in general? 
    > > > 	 */
    > > > 	my_registered->task_ops->alloc_security(task); 
    > > > }
    > > 
    > > Yep, but only in the LSMEXAMPLE module, since the structure it only 
    > > registered there.
    > Sure, the kernel hook hasn't changed.  The registrar module code is what is
    > calling the registered hook, that's not the issue.
    > The task pointer is in the kernel.  The task->security pointer is in
    > the kernel.  The module allocates space and assigns the pointer to the
    > task->security pointer in the kernel.  The registrar and registered modules
    > both want to own the opaque security blobs.  There is only ONE void pointer.
    > You have to be prepared to do good housekeeping, that's what I'm trying to
    > say.
    > /* invoked when security_ops->task_ops->alloc_security() is called in kernel */
    > my_module_task_alloc(task)
    > {
    > 	struct my_big_security_blob *blob;
    > 	my_registered->task_ops->alloc_security(task);
    > 	/* this just assigned a value to task->security 
    > 	 * but wait, I want to add my own per process
    > 	 * security attributes.
    > 	 */
    > 	blob = (struct my_big_security_blob *)kmalloc...
    > 	/* my blob needs to have a blob in it so I can store
    > 	 * their blob
    > 	 */
    > 	blob->blob = task->security;
    > 	task->security = blob;
    > }
    > /* invoked when security_ops->capable() is called in kernel */
    > my_module_capable(cap)
    > {
    > 	struct my_big_security_blob *tmp = current->security;
    > 	/* do my capable check here */
    > 	/* update the security pointer to be their blob
    > 	 * which I had stored in my blob
    > 	 */
    > 	current->security = tmp->blob;
    > 	my_registered->capable(cap);
    > 	/* now restore everything */
    > 	current->security = tmp;
    > }
    > Can you see how you have to be careful when sharing the single opaque blob
    > between modules?  That's what I'm driving at.  
    Yes, I see your point.  I'd also argue that you have to be careful about 
    anything you do in a single module... as I've been reminded many times
    before, this is kernel space and you have to be most careful everywhere in
    here. Your example shows that it may be somewhat "difficult" to implement
    in the module, but also that it is probably possible and moves the 
    difficulty outward from the virgin kernel.
    > It's not the end of the
    > world, and what your proposing may be better than what we have now.  But it
    > is also dangerously close to explicitly supporting multiple modules 
    Not exactly, and I'm sorry if I gave that impression.  What I'm proposing
    is a simple mechanism to allow MODULES to support multiple MODULES, not
    the "virgin kernel".  By creating a mechanism by which the registration/
    unregistration can be "pushed" into the previously installed module, 
    it's possible to extend the whole schmear in the future by installing a 
    "multiple module support" module, without having to change ANYTHING 
    in the "virgin kernel" part of the interface.
    At very least, the previously installed module is NOTIFIED when an 
    attempt by another module to register has been made.  That alone is 
    worth something.
    Yes, the module will have to be well written and worry about a variety 
    of issues, but there's no *mandate* that a module designer need support
    subordinate registrations, or even a requirement as to how she might do
    that.  If security_ops->mod_register on the first module
    is NULL or returns -EINVAL , the whole thing collapses into exactly what
    we have now.
    > and going down a path that we may not be ready to explore.
    I think there are a few of us who might be ready, and I'm asking for 
    nothing but two pointers in the security_operations structure and a 
    test/hook in register_security() and unregister_security().
    In fact, since some think better in "code", I'm attaching a rough 
    patch to the kernel with the last stable patch applied to demonstrate
    how little I'm actually proposing.  (Warning: I'm not a kernel hacker,
    so I probably did it wrong -- for discussion purposes only, although 
    it compiled and worked when I tried it.)
    My thinking is, put this in now, and you won't have to worry about
    multiple module support EVER, since your "virgin kernel patches" 
    won't change, and the "multiplexor issue" can move out to the registered
    module.  When the time comes (or groups decide they want) to
    explicitly support multiple modules, there's a minimal mechanism for them
    to link into already in place.
    Any structures in the virgin kernel that evolve that would need to 
    be accessable OTHER than those referenced in the security_ops structure
    and which are not already exposed can be exposed later... I'm not 
    asking for a "global rethink" about it at this time, just a mechanism
    that will start the ball rolling (then forget about it until version
    2, if you like.)
    As I've been told many times before, a MODULE, once registered, IS 
    the kernel, so let the problem be handled later by a module instead
    of thinking about it in the virgin kernel patches... but put a 
    mechanism in there that allows it now as a "first direction." 
    Since, as you've pointed out, the thing is very fluid right now, 
    if it turns out to be totally disasterous in the future, it can 
    go away or evolve, too.
    > -chris
    J. Melvin Jones
    ||  J. MELVIN JONES            jmjonesat_private 
    ||  Microcomputer Systems Consultant  
    ||  Software Developer
    ||  Web Site Design, Hosting, and Administration
    ||  Network and Systems Administration
    diff -ru a/include/linux/security.h b/include/linux/security.h
    --- a/include/linux/security.h	Wed May 23 09:29:47 2001
    +++ b/include/linux/security.h	Wed May 23 09:31:15 2001
    @@ -160,7 +160,9 @@
     	struct module_security_ops 	* module_ops;
     	struct msg_queue_security_ops	* msg_queue_ops;
     	struct shm_security_ops		* shm_ops;
    +	int	(* mod_register)	(void *ops); // lazyman way
    +	int 	(* mod_unregister)	(void *ops); // lazyman way
    diff -ru a/kernel/capability_plug.c b/kernel/capability_plug.c
    --- a/kernel/capability_plug.c	Wed May 23 09:30:02 2001
    +++ b/kernel/capability_plug.c	Wed May 23 09:31:30 2001
    @@ -616,6 +616,9 @@
     	module_ops:		&cap_module_ops,
     	msg_queue_ops:		&cap_msg_queue_ops,
     	shm_ops:		&cap_shm_ops,
    +	mod_register:		NULL,
    +	mod_unregister:		NULL,
     static int __init capability_plug_init (void)
    diff -ru a/kernel/security.c b/kernel/security.c
    --- a/kernel/security.c	Wed May 23 09:30:02 2001
    +++ b/kernel/security.c	Wed May 23 09:31:30 2001
    @@ -119,6 +119,8 @@
     static int dummy_shm_setattr		(void)	{return 0;}
     static int dummy_shm_delete		(void)	{return 0;}
    +static int dummy_register		(void *ops)	{return -EINVAL;}
    +static int dummy_unregister		(void *ops)	{return -EINVAL;}
     static struct binprm_security_ops dummy_binprm_ops = {
     	alloc_security:	dummy_binprm_alloc_security,
    @@ -237,6 +239,9 @@
     	module_ops:		&dummy_module_ops,
     	msg_queue_ops:		&dummy_msg_queue_ops,
     	shm_ops:		&dummy_shm_ops,
    +	mod_register:		dummy_register,
    +	mod_unregister:		dummy_unregister,
    @@ -273,6 +278,9 @@
     	if (security_ops != &dummy_security_ops) {
    +                if (security_ops->mod_register != NULL) {
    +                	return security_ops->mod_register(ops);
    +                }
     		printk (KERN_INFO "There is already a security framework initialized, " __FUNCTION__ " failed.");
     		return -EINVAL;
    @@ -302,6 +310,9 @@
     int unregister_security (struct security_operations *ops)
     	if (ops != security_ops) {
    +		if (security_ops->mod_unregister != NULL) {
    +			return security_ops->mod_unregister(ops);
    +		}
     		printk (KERN_INFO __FUNCTION__ ": trying to unregister a security_opts structure that is not registered, failing.");
     		return -EINVAL;
    linux-security-module mailing list

    This archive was generated by hypermail 2b30 : Wed May 23 2001 - 07:36:27 PDT