Re: [Xen-devel] [PATCH 17/18] xenstored: add --priv-domid parameter

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-01-18 at 14:41 +0000, Daniel De Graaf wrote:
> On 01/18/2012 06:48 AM, Ian Campbell wrote:
> > On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote:
> >> This parameter identifies an alternative service domain which has
> >> superuser access to the xenstore database, which is currently required
> >> to set up a new domain's xenstore entries.
> > 
> > Is this equivalent to dom0 adding write permissions to various paths for
> > that domain as it builds it or is there more to it than that.
> > 
> > I know that the determination of "various paths" is non-trivial, so I'm
> > not actually suggesting that is a better approach.
> > 
> 
> It's more: the domain builder needs to create entries owned by the new
> domain, and similar to UNIX chown() can only be called by the superuser.
> The domain builder also currently relies on the fact that new keys it
> creates inherit the parent's ownership instead of being owned by dom0.
> The introduce operation is also privileged.

Thanks for explaining. I wonder if there is somewhere this can be
usefully written down so that "privileged" is well defined?

docs/misc/xenstore.txt seems to be more about the wire protocol than the
underlying semantics. Perhaps someone on list can suggest a suitable
place?

> 
> >>
> >> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> >> ---
> >>  tools/xenstore/xenstored_core.c   |    5 +++++
> >>  tools/xenstore/xenstored_core.h   |    1 +
> >>  tools/xenstore/xenstored_domain.c |    2 +-
> >>  3 files changed, 7 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/tools/xenstore/xenstored_core.c 
> >> b/tools/xenstore/xenstored_core.c
> >> index eea5fd6..9d087de 100644
> >> --- a/tools/xenstore/xenstored_core.c
> >> +++ b/tools/xenstore/xenstored_core.c
> >> @@ -1774,6 +1774,7 @@ static struct option options[] = {
> >>    { "event", 1, NULL, 'e' },
> >>    { "help", 0, NULL, 'H' },
> >>    { "no-fork", 0, NULL, 'N' },
> >> +  { "priv-domid", 1, NULL, 'p' },
> >>    { "output-pid", 0, NULL, 'P' },
> >>    { "entry-size", 1, NULL, 'S' },
> >>    { "trace-file", 1, NULL, 'T' },
> >> @@ -1786,6 +1787,7 @@ static struct option options[] = {
> >>  
> >>  extern void dump_conn(struct connection *conn); 
> >>  int dom0_event = 0;
> >> +int priv_domid = 0;
> >>  
> >>  int main(int argc, char *argv[])
> >>  {
> >> @@ -1852,6 +1854,9 @@ int main(int argc, char *argv[])
> >>            case 'e':
> >>                    dom0_event = strtol(optarg, NULL, 10);
> >>                    break;
> >> +          case 'p':
> >> +                  priv_domid = strtol(optarg, NULL, 10);
> >> +                  break;
> >>            }
> >>    }
> >>    if (optind != argc)
> >> diff --git a/tools/xenstore/xenstored_core.h 
> >> b/tools/xenstore/xenstored_core.h
> >> index d3040ba..03e2e48 100644
> >> --- a/tools/xenstore/xenstored_core.h
> >> +++ b/tools/xenstore/xenstored_core.h
> >> @@ -169,6 +169,7 @@ void dtrace_io(const struct connection *conn, const 
> >> struct buffered_data *data,
> >>  
> >>  extern int event_fd;
> >>  extern int dom0_event;
> >> +extern int priv_domid;
> >>  
> >>  /* Map the kernel's xenstore page. */
> >>  void *xenbus_map(void);
> >> diff --git a/tools/xenstore/xenstored_domain.c 
> >> b/tools/xenstore/xenstored_domain.c
> >> index 5f4a09e..46bcf3e 100644
> >> --- a/tools/xenstore/xenstored_domain.c
> >> +++ b/tools/xenstore/xenstored_domain.c
> >> @@ -241,7 +241,7 @@ bool domain_can_read(struct connection *conn)
> >>  
> >>  bool domain_is_unprivileged(struct connection *conn)
> >>  {
> >> -  return (conn && conn->domain && conn->domain->domid != 0);
> >> +  return (conn && conn->domain && conn->domain->domid != 0 && 
> >> conn->domain->domid != priv_domid);
> >>  }
> >>  
> >>  bool domain_can_write(struct connection *conn)
> > 
> 
> 



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