WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [Xense-devel][RFC][PATCH][1/4] Xen Security Modules: XSM

On Fri, 2006-09-01 at 11:55 -0700, Chris Wright wrote:
> * George S. Coker, II (gscoker@xxxxxxxxxxxxxx) wrote:
> > The attached patch implements the Xen Security Modules (XSM) framework.
> > This patch should apply cleanly to changeset 9694:d82a4c4d04d4 Xen
> > 3.0.2-3.
> 
> Nice to see this all posted.  Have any perfomance numbers for default
> dummy, or numbers for increase in memory footprint for this + flask +
> flask example policy?  Just curious to see what the rough cost is.
> Some quick comments below.
> 
> > Modules may also define at Xen compile time a magic number XSM_MAGIC to
> > indicate that a policy should be discovered from the images loaded at
> > boot.  The policy file should then be listed in grub as one of the
> > multi-boot modules after the dom0 kernel.
> 
> I like that feature.
> 
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/arch/x86/dom0_ops.c
> > --- a/xen/arch/x86/dom0_ops.c    Fri May 26 09:23:33 2006 +0100
> > +++ b/xen/arch/x86/dom0_ops.c    Thu Aug 31 17:14:49 2006 -0400
> > @@ -25,6 +25,7 @@
> >  #include <asm/hvm/support.h>
> >  #include <asm/processor.h>
> >  #include <public/sched_ctl.h>
> > +#include <xsm/xsm.h>
> >  
> >  #include <asm/mtrr.h>
> >  #include "cpu/mtrr/mtrr.h"
> > @@ -58,6 +59,10 @@ long arch_do_dom0_op(struct dom0_op *op,
> >  
> >      case DOM0_MSR:
> >      {
> > +        ret = xsm_msr(op->u.msr.write);
> 
> Hmm, this is against 3.0.2, unfortunately you'll have some work to move
> to 3.0.3 with the hypercall changes here (IIRC, this one actually
> went away).  And have you done each arch?  I only see x86 here.
> 

We have only done x86 for now.

> <snip>
> 
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/arch/x86/setup.c
> > --- a/xen/arch/x86/setup.c    Fri May 26 09:23:33 2006 +0100
> > +++ b/xen/arch/x86/setup.c    Thu Aug 31 17:14:49 2006 -0400
> > @@ -24,6 +24,7 @@
> >  #include <asm/shadow.h>
> >  #include <asm/e820.h>
> >  #include <acm/acm_hooks.h>
> > +#include <xsm/xsm.h>
> >  
> >  extern void dmi_scan_machine(void);
> >  extern void generic_apic_probe(void);
> > @@ -404,6 +405,8 @@ void __init __start_xen(multiboot_info_t
> >  
> >      scheduler_init();
> >  
> > +    xsm_init(&initrdidx, mbi, initial_images_start);
> > +
> >      idle_domain = domain_create(IDLE_DOMAIN_ID, 0);
> >      BUG_ON(idle_domain == NULL);
> >  
> > @@ -507,6 +510,8 @@ void __init __start_xen(multiboot_info_t
> >      set_bit(_DOMF_privileged, &dom0->domain_flags);
> >      /* post-create hooks sets security label */
> >      acm_post_domain0_create(dom0->domain_id);
> > +
> > +    xsm_complete_init(dom0);
> 
> Seems this should drop the acm hook here, no?
> 

We did not want the XSM patch to add XSM and remove ACM because we do 
not believe that the community sees ACM and sHype as the distinct
entities that they really are.  That's why we have the patch that
removes the duplicated hooks and code and produces a module called ACM.
Hopefully in the future XSM will refer to the security framework and the
the STE/Chinese Wall functionality of ACM will be called the sHype
module.

> >      /* Grab the DOM0 command line. */
> >      cmdline = (char *)(mod[0].string ? __va(mod[0].string) : NULL);
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/arch/x86/x86_32/entry.S
> > --- a/xen/arch/x86/x86_32/entry.S    Fri May 26 09:23:33 2006 +0100
> > +++ b/xen/arch/x86/x86_32/entry.S    Thu Aug 31 17:14:49 2006 -0400
> > @@ -648,6 +648,7 @@ ENTRY(hypercall_table)
> >          .long do_acm_op
> >          .long do_nmi_op
> >          .long do_arch_sched_op
> > +        .long do_xsm_op             /* 30 */
> >          .rept NR_hypercalls-((.-hypercall_table)/4)
> >          .long do_ni_hypercall
> >          .endr
> > @@ -683,6 +684,7 @@ ENTRY(hypercall_args_table)
> >          .byte 1 /* do_acm_op            */
> >          .byte 2 /* do_nmi_op            */
> >          .byte 2 /* do_arch_sched_op     */
> > +        .byte 1 /* do_xsm_op            */
> >          .rept NR_hypercalls-(.-hypercall_args_table)
> >          .byte 0 /* do_ni_hypercall      */
> >          .endr
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/arch/x86/x86_32/xen.lds.S
> > --- a/xen/arch/x86/x86_32/xen.lds.S    Fri May 26 09:23:33 2006 +0100
> > +++ b/xen/arch/x86/x86_32/xen.lds.S    Thu Aug 31 17:14:49 2006 -0400
> > @@ -56,6 +56,7 @@ SECTIONS
> >    __initcall_start = .;
> >    .initcall.init : { *(.initcall.init) } :text
> >    __initcall_end = .;
> > +  .xsm_initcall.init : { __xsm_initcall_start = .; *(.xsm_initcall.init) 
> > __xsm_initcall_end = .; }
> >    . = ALIGN(STACK_SIZE);
> >    __init_end = .;
> >  
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/common/dom0_ops.c
> > --- a/xen/common/dom0_ops.c    Fri May 26 09:23:33 2006 +0100
> > +++ b/xen/common/dom0_ops.c    Thu Aug 31 17:14:49 2006 -0400
> > @@ -22,6 +22,7 @@
> >  #include <public/dom0_ops.h>
> >  #include <public/sched_ctl.h>
> >  #include <acm/acm_hooks.h>
> > +#include <xsm/xsm.h>
> >  
> >  extern long arch_do_dom0_op(
> >      struct dom0_op *op, GUEST_HANDLE(dom0_op_t) u_dom0_op);
> > @@ -82,6 +83,8 @@ static void getdomaininfo(struct domain 
> >          info->ssidref = ((struct acm_ssid_domain *)d->ssid)->ssidref;
> >      else    
> >          info->ssidref = ACM_DEFAULT_SSID;
> > +    
> > +    xsm_security_domaininfo(d, info);
> >      
> >      info->tot_pages         = d->tot_pages;
> >      info->max_pages         = d->max_pages;
> > @@ -120,7 +123,12 @@ long do_dom0_op(GUEST_HANDLE(dom0_op_t) 
> 
> shouldn't this take care of the acm_pre_dom0_op hook here as well?
> IOW, at the end there shouldn't be acm specific hooks left, right?
> 
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/common/domain.c
> > --- a/xen/common/domain.c    Fri May 26 09:23:33 2006 +0100
> > +++ b/xen/common/domain.c    Thu Aug 31 17:14:49 2006 -0400
> > @@ -24,6 +24,7 @@
> >  #include <public/dom0_ops.h>
> >  #include <public/sched.h>
> >  #include <public/vcpu.h>
> > +#include <xsm/xsm.h>
> >  
> >  /* Both these structures are protected by the domlist_lock. */
> >  rwlock_t domlist_lock = RW_LOCK_UNLOCKED;
> > @@ -50,6 +51,9 @@ struct domain *domain_create(domid_t dom
> >      INIT_LIST_HEAD(&d->xenpage_list);
> >  
> >      rangeset_domain_initialise(d);
> > +
> > +    if (xsm_alloc_security_domain(d))
> > +        goto fail1;
> >  
> >      if ( !is_idle_domain(d) )
> >      {
> > @@ -305,6 +309,8 @@ void domain_destroy(struct domain *d)
> >  
> >      arch_domain_destroy(d);
> >  
> > +    xsm_free_security_domain(d);
> > +
> >      free_domain(d);
> >  
> >      send_guest_virq(dom0->vcpu[0], VIRQ_DOM_EXC);
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/include/xen/grant_table.h
> > --- a/xen/include/xen/grant_table.h    Fri May 26 09:23:33 2006 +0100
> > +++ b/xen/include/xen/grant_table.h    Thu Aug 31 17:14:49 2006 -0400
> > @@ -74,6 +74,7 @@ typedef struct {
> >      grant_entry_t        *shared;
> >      /* Active grant table. */
> >      active_grant_entry_t *active;
> > +    void                 *security;
> 
> What's a grant table label look like?

This is detritus in the patch.  We have chosen not to put security
structures in grant tables at this time.  We were looking into it, but
because the guest has complete control over the contents of the grant
entry, i.e. which page, there is no benefit than to check the 
permissions on the domains involved in the grant.

> 
> >      /* Mapping tracking table. */
> >      grant_mapping_t      *maptrack;
> >      unsigned int          maptrack_head;
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/include/xen/hypercall.h
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/include/xen/sched.h
> > --- a/xen/include/xen/sched.h    Fri May 26 09:23:33 2006 +0100
> > +++ b/xen/include/xen/sched.h    Thu Aug 31 17:14:49 2006 -0400
> > @@ -47,6 +47,7 @@ struct evtchn
> >          u16 pirq;      /* state == ECS_PIRQ */
> >          u16 virq;      /* state == ECS_VIRQ */
> >      } u;
> > +    void *ssid;
> >  };
> 
> Ah, you must resue the acm domain label?  Actually would be nice to see
> which objects are labelled, did I miss that list?
> 

We presently have security labels on domains and event channels.  We
reuse the security pointer added by ACM for domains.  We do not have
labels on other hypervisor objects.  The Flask module does use the
policy infrastructure to lookup labels on platform resources such as
physical interrupts.

> >  int  evtchn_init(struct domain *d);
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/include/xsm/xsm.h
> > --- /dev/null    Thu Jan  1 00:00:00 1970 +0000
> > +++ b/xen/include/xsm/xsm.h    Thu Aug 31 17:14:49 2006 -0400
> > @@ -0,0 +1,722 @@
> > +/*
> > + *  This file contains the Flask hook function implementations for Xen.
> > + *
> > + *  This work is based on the LSM implementation in Linux 2.6.13.4.
> > + *
> > + *  Author:  George Coker, <gscoker@xxxxxxxxxxxxxx>
> > + *
> > + *  Contributors: Michael LeMay, <mdlemay@xxxxxxxxxxxxxx>
> > + *
> > + *  This program is free software; you can redistribute it and/or modify
> > + *  it under the terms of the GNU General Public License version 2,
> > + *  as published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef __XSM_H__
> > +#define __XSM_H__
> > +
> > +#include <xen/sched.h>
> > +#include <xen/multiboot.h>
> > +
> > +#ifdef XSM_ENABLE
> > +
> > +/* policy magic number (defined by XSM_MAGIC) */
> > +typedef u32 xsm_magic_t;
> > +#ifndef XSM_MAGIC
> > +#define XSM_MAGIC 0x00000000
> > +#endif
> > +
> > +extern char *policy_buffer;
> > +extern u32 policy_size;
> > +
> > +typedef void xsm_op_t;
> > +DEFINE_GUEST_HANDLE(xsm_op_t);
> > +
> > +typedef int (*xsm_initcall_t)(void);
> > +
> > +extern xsm_initcall_t __xsm_initcall_start[], __xsm_initcall_end[];
> > +
> > +#define xsm_initcall(fn) \
> > +    static xsm_initcall_t __initcall_##fn \
> > +    __attribute_used__ __attribute__((__section__(".xsm_initcall.init"))) 
> > = fn
> > +
> > +struct xsm_operations {
> > +    void (*security_domaininfo) (struct domain *d, dom0_getdomaininfo_t 
> > *info);
> > +    int (*setvcpucontext) (struct domain *d);
> > +    int (*pausedomain) (struct domain *d);
> > +    int (*unpausedomain) (struct domain *d);
> > +    int (*createdomain) (dom0_op_t *op);
> > +    void (*createdomain_post) (struct domain *d, dom0_op_t *op);
> > +    void (*createdomain_fail) (dom0_op_t *op);
> > +    int (*max_vcpus) (struct domain *d);
> > +    int (*destroydomain) (struct domain *d);
> > +    int (*setvcpuaffinity) (struct domain *d);
> > +    int (*schedctl) (struct sched_ctl_cmd *cmd);
> > +    int (*adjustdom) (struct sched_adjdom_cmd *cmd);
> > +    int (*getdomaininfo) (struct domain *d);
> > +    int (*getvcpucontext) (struct domain *d);
> > +    int (*getvcpuinfo) (struct domain *d);
> > +    int (*settime) (void);
> > +    int (*tbufcontrol) (void);
> > +    int (*readconsole) (uint32_t clear);
> > +    int (*sched_id) (void);
> > +    int (*setdomainmaxmem) (struct domain *d);
> > +    int (*setdomainhandle) (struct domain *d);
> > +    int (*setdebugging) (struct domain *d);
> > +    int (*irq_permission) (struct domain *d, uint8_t pirq, uint8_t access);
> > +    int (*iomem_permission) (struct domain *d, unsigned long mfn, uint8_t 
> > access);
> > +    int (*perfcontrol) (void);
> > +
> > +    int (*msr) (uint32_t);
> 
> Platform specific code might want ifdefs, or perhaps a just platform
> specific set of ops.

Right, because we are x86 specific at this time.  But we should be more
careful.

> 
> > +    int (*shadow_control) (struct domain *d, uint32_t op);
> > +    int (*memtype) (uint32_t access);
> > +    int (*microcode) (void);
> > +    int (*ioport_permission) (struct domain *d, uint32_t ioport, uint8_t 
> > access);
> > +    int (*physinfo) (void);
> > +    int (*getpageframeinfo) (unsigned long mfn);
> > +    int (*getmemlist) (struct domain *d);
> > +    int (*platform_quirk) (uint32_t);
> > +    int (*physmemmap) (void);
> > +    int (*hypercall_init) (struct domain *d);
> > +    
> > +    int (*evtchn_unbound) (struct domain *d, struct evtchn *chn, domid_t 
> > id2);
> > +    int (*evtchn_interdomain) (struct domain *d1, struct evtchn *chn1, 
> > +                struct domain *d2, struct evtchn *chn2);
> > +    int (*evtchn_virq) (struct domain *d, struct evtchn *chn, int virq, 
> > int vcpu);
> > +    int (*evtchn_ipi) (struct domain *d, struct evtchn *chn, int vcpu);
> > +    int (*evtchn_pirq) (struct domain *d, struct evtchn *chn, int pirq);
> > +    int (*evtchn_close) (struct domain *d, struct evtchn *chn);
> > +    int (*evtchn_send) (struct domain *d, struct evtchn *chn);
> > +    int (*evtchn_status) (struct domain *d, struct evtchn *chn);
> > +    int (*evtchn_vcpu) (struct domain *d, struct evtchn *chn, unsigned int 
> > vcpu);
> > +    int (*evtchn_unmask) (struct domain *d, struct evtchn *chn);
> > +    int (*evtchn_init) (struct domain *d, struct evtchn *chn);
> > +
> > +    int (*grant_mapref) (struct domain *d1, struct domain *d2, uint32_t 
> > flags);
> > +    int (*grant_unmapref) (struct domain *d1, struct domain *d2);
> > +    int (*grant_setup) (struct domain *d1, struct domain *d2);
> > +    int (*grant_transfer) (struct domain *d1, struct domain *d2);
> > +    
> > +    int (*alloc_security_domain) (struct domain *d);
> > +    void (*free_security_domain) (struct domain *d);
> > +    int (*alloc_security_evtchn) (struct evtchn *chn);
> > +    void (*free_security_evtchn) (struct evtchn *chn);
> 
> Heh, well, that's as good as a list ;-)
> 
> > +    int (*mmu_normal_update) (struct domain *d, intpte_t fpte);
> > +
> > +    long (*__do_xsm_op) (GUEST_HANDLE(xsm_op_t) op);
> 
> We very intentionally did not do this in LSM (or more to the point,
> we did and it was soundly rejected).  It's a free form way to extend the
> hypercall interface, which in Linux is frowned upon.  Of course, you don't
> have the various selinuxfs, /proc/<pid>/attr type of interfaces in the
> hypervisor, but it's worth considering if there are other possibilities
> (32/64-bit compat is an example of something which is easily lost when
> pushing the hypercall interface down a layer into the module).
> 
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/xsm/Makefile
> > --- /dev/null    Thu Jan  1 00:00:00 1970 +0000
> > +++ b/xen/xsm/Makefile    Thu Aug 31 17:14:49 2006 -0400
> > @@ -0,0 +1,3 @@
> > +obj-y += xsm_core.o
> > +obj-y += xsm_policy.o
> > +obj-y += dummy.o
> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/xsm/dummy.c
> > --- /dev/null    Thu Jan  1 00:00:00 1970 +0000
> > +++ b/xen/xsm/dummy.c    Thu Aug 31 17:14:49 2006 -0400
> > @@ -0,0 +1,392 @@
> > +#include <xen/sched.h>
> > +#include <xsm/xsm.h>
> > +
> > +#ifdef XSM_ENABLE
> 
> Can you just conditionally compile in this file using the above Makefile?
> 

yes.

> > diff -r d82a4c4d04d4 -r a24b5a5f5f5b xen/xsm/xsm_core.c
> > --- /dev/null    Thu Jan  1 00:00:00 1970 +0000
> > +++ b/xen/xsm/xsm_core.c    Thu Aug 31 17:14:49 2006 -0400
> <snip>
> > +#ifdef XSM_ENABLE
> > +#endif
> > +
> > +long do_xsm_op (GUEST_HANDLE(xsm_op_t) op)
> > +{
> > +    return __do_xsm_op(op);
> > +}
> 
> Linux has the cond_syscall() macro, should be enough to do this
> if you can't find a better way to do security module specifc
> calls.
> 
-- 
George S. Coker, II <gscoker@xxxxxxxxxxxxxx> 443-479-6944


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel