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

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 01/12/2012 05:20 AM, Ian Campbell wrote:
> On Wed, 2012-01-11 at 17:21 +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.
>>
>> 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 65ceb2c..4437c8d 100644
>> --- a/tools/xenstore/xenstored_core.c
>> +++ b/tools/xenstore/xenstored_core.c
>> @@ -1777,6 +1777,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' },
>> @@ -1789,6 +1790,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[])
>>  {
>> @@ -1854,6 +1856,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 5bf16e8..ba9a5ef 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);
> 
> Is it deliberate / desirable that both dom0 and domPRIV are privileged
> when this option is used? Or should it be just one or the other, which
> is equivalent to removing the domid!=0 check since priv_domid defaults
> to 0.
> 
> Ian.
> 

Yes. In the cases where I used this, both dom0 and domPRIV are privileged.
Since dom0 needs to introduce domPRIV, and introduction is privileged, you
can't just have domPRIV be the only privileged domain. The ideal solution is
to have more fine-grained permissions than superuser-or-domU in xenstored,
but that is a different topic from moving xenstored into a stubdom.

-- 
Daniel De Graaf
National Security Agency

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