Re: ideas on interface (was Be careful please)

From: Chris Wright (chrisat_private)
Date: Fri Apr 13 2001 - 23:13:21 PDT

  • Next message: Andrew Morgan: "Re: The bootstrap process"

    * David Wagner (dawat_private) wrote:
    > Chris Wright  wrote:
    > >See attached "header" file for a rough sketch of an interface that
    > >we'd like to propose/discuss.
    > 
    > Ok, you asked for it! :-)  Here are some detailed comments.
    > But first: Let me thank you for putting up a concrete proposal.
    
    Gladly ;-)  And thanks for all your thoughts!
     
    > A general note: These hooks seem narrowly tailored towards
    > allowing one to implement the existing capabilities policy.
    > This is a good starting point, but I would be very disappointed
    > if this is also the ending point.
    
    Absolutely agreed!  A capabilities module is what I see as the first
    hurdle.  Without this, I doubt our work will be accpeted.
     
    > Also, can you tell me about the semantics of these hooks?  If
    > I see `int (* foo)(void);', what does the return value mean?
    > Is the hook called *instead* of some basic operation?  Is it
    > called before, and a -EPERM ensures the request will be denied?
    
    These are the hooks that will sit all throughout the kernel and be implemented
    by whatever specific security module.  These are designed to be called before
    you access some bit in the kernel that we want to protect.  A return of 0 (zero)
    indicates success, and -EPERM indicates "no can do" ;-) Also, an important note
    is that this is food for discussion.  I expect the actual interface we agree on
    to be much different.
    
    As you can see, this is really incomplete.  Most calls take no arguments
    rendering them useless ;-)  What I'm really interested in is discussing
    _how_ we can make the interface abstract enough to meet everyone's
    needs.  So this is an opporunity to start looking at what we really
    need (with a keen eye on capabilities as a starting point ;-).
    
    > >	int  (* mount)			(void);		// part of CAP_SYS_ADMIN
    > >	int  (* ioperm)			(void);		// part of CAP_RAWIO
    > >	int  (* iopl)			(void);		// part of CAP_RAWIO
    > >	int  (* ptrace)			(void);		// CAP_PTRACE
    > >	int  (* setcapablity)	(void);		// CAP_SETPCAP
    > >	int (* setuid)			(void);		// CAP_SETUID
    > >	int (* setgid)			(void);		// CAP_SETGID
    > >	int (* setpriority)		(void);		// CAP_NICE
    > >	int (* kill)			(void);		// CAP_KILL
    > 
    > This is massively limiting.  All you can do is allow or deny all
    > mount(2) operations, for instance.  How about some arguments?
    > I can't overstate the importance of this limitation.  Really.
    
    I completely agree that mount, as an example, will need more detail.
    Nearly all the functions that take no input need to be flushed out.
    Even something as simple as setuid...perhaps someone's security module
    would allow setuid to some uids but not others.
     
    > >	int  (* sethostname)		(void);
    > >	int  (* setdomainname)		(void);
    > 
    > I suggest allowing the module to make its decision based on the
    > value of the new hostname.
    
    That's the point.  Give the module the hooks to implement, and the
    module can do whatever.  These above fit in a system_utsname category,
    and perhaps could be governed with one policy within the module.  For
    example...capabilities...both of these fall under the CAP_SYS_ADMIN
    capability.  So in the capabilities module, these two may collapse to
    the same function.  Because we all have slightly different needs I
    expect to see finer granularity exposed through the interface than any
    one module may actually need.
    
    > 
    > >	int  (* reboot)			(void);		// CAP_SYS_BOOT
    > 
    > Does this correspond to the syscall reboot(2), or to the action
    > of rebooting the system?  If the former, note that reboot(2) is
    > used for things other than rebooting the system, and in this case
    > you should pass its argument to the module.  But I suggest making
    > the semantics be the latter (and change the name of the hook slightly
    > to make the semantics more obvious?).
    
    This corresponds to reboot(2).  Yes, it needs an argument ;-)
    
    > >	/* cmw: this should allow you to know if two permissions
    > >	 * stuctures are the same, subset, or superset
    > >	 * returns 0 when same, 1 when perm1 is superset of perm2 and
    > >	 * -1 when perm1 is subset of perm2...hmmm what if they have no
    > >	 * relation?  need better type than void* ;-/
    > >	 * at least subset is needed for capabilities
    > >	 */
    > >	int (*perm_comp)		(void * perm1, void * perm2)
    > >	void (*init_perm)		(void * perm);
    > >	int (*change_perm)		(void * perm);
    > 
    > What are you envisioning here?  I recommend treading carefully
    > here.  I've yet to run into anything in the kernel that would
    > need this functionality, and I've seen implementation experience
    > suggesting that this can cause lots of trouble when people start
    > implementing policies that don't fit the mental model of the
    > people using perm_comp(), etc.  Did you have some particular
    > problem in mind that this is trying to solve?
    
    I admit, this one is ugly!  I was working on pushing capabilities to
    a module and was focused on kernel hooks when I fooled myself into
    thinking that the functionality of the cap_issubset() macro should
    be exposed.  Looking back, I'm not too sure what I was thinking?
    (chris puts down the crack pipe ;-)
     
    > Is this a burden imposed on module writers?  How are they going to
    > be able to implement this?  I mean, all you've got is function pointers,
    > and identifying whether two different procedures compute the same thing
    > is notoriously hard...
    > 
    > >struct socket_security_ops {
    > >};
    > 
    > Did you mean for this to be empty?  What about all of the socket ops:
    > bind, connect, and so forth?
    
    I wanted to first fill the security_operations struct with place holders
    for kernel objects that we may care about, then go back and fill in the
    place holders.  So this is incomplete, but the concept of viewing each
    kernel object as an entity to protect is what I really care about.  So
    yes, this needs to be filled in.
     
    > >extern int register_security	(struct security_operations *ops);
    > >extern int unregister_security	(struct security_operations *ops);
    > 
    > Don't you mean the following?
    > 
    > extern int register_security(task_struct *p, struct security_operations *ops);
    > extern int unregister_security(task_struct *p, struct security_operations *ops);
    
    No.  init_module() would call register_security() with the security
    module's own filled out security_ops.  This is how the module registers
    its own implementation of the hooks. (likewise cleanup_module would call
    unregister_security)
    
    > >/* global variables */
    > >extern struct security_operations *security_ops;
    > 
    > What's going on here?  What's this global variable doing?
    
    This is the global security_ops that is sprinkled throughout the kernel.
    So you'd see something like kernel/sys.c:sys_setuid() 
    
    if (!security_ops->task_ops->setuid()) {
    	/* passed security check */
    } else {
    	/* didn't pass */
    }
    
    As I mentioned above, perhaps this would actually take an argument
    setuid(uid) or something.
    
    While I found it useful to group hooks into kernel object operations,
    i.e. security_ops->task_ops->setuid() or security_ops->inode_ops->setattr(),
    I have no problem with flattening the structure for speed.  I just found
    this type of abstraction useful for generating a starting point.
    
    > I have many more comments, but I'll avoid overloading you in this
    > email.
    
    Thanks for helping generate the discussion ;-)
    
    cheers,
    -chris
    
    _______________________________________________
    linux-security-module mailing list
    linux-security-moduleat_private
    http://mail.wirex.com/mailman/listinfo/linux-security-module
    



    This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 23:18:34 PDT