> Steve - I agree with you.
> But do you think that the security issue should be handled by Xen or
> Xen should leave it to the domains (as it does most of the times in
> other cases as well).
I'm not sure that this *can* be handled by the domains. If you assume
that:
a) Domains occasionally need to grant backend domains access to their
memory.
b) When a domain grants access to a backend domain, it doesn't want
arbitrary other untrusted domains to be able to access that grant.
Then it seems fairly clear that, when you grant a remote domain access
to some frame of memory, you need to be able to restrict the grant
reference to only be valid in the nominated domain. *Something*
therefore needs to enforce that only the desired domain can use a
particular grant. The grant-ing domain isn't necessarily running at
the time that a grant reference is used, so it can't perform the
access check. The grant-ee domain can't do the check, because it's
not trusted. The only place left in which to do the check is Xen.
Unless I've completely misread your patch (which is certainly
possible), it's removing that check, and so domains would no longer be
restricted to only using grant references which are intended for them.
This means that, for instance, a domain can get at in-flight disk
buffers from another domain just by guessing the grant references.
Steven.
> -----Original Message-----
> From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> Sent: Thursday, February 05, 2009 5:57 PM
> To: Kumar, Venkat
> Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] GNTTAB_copy
>
> > Let me explain the scenario.
> >
> > Before the copy happens, Xen checks a few things about the source grant
> > entry and the destination grant entry.
> >
> > =============================================================================
> > if ( src_is_gref )
> > {
> > gdprintk(XENLOG_INFO, "Entering Source Stuff \n");
> > gdprintk(XENLOG_INFO, "Source gref is %d \n",op->source.u.ref);
> > rc = __acquire_grant_for_copy(sd, op->source.u.ref, 1, &s_frame);
> > if ( rc != GNTST_okay )
> > goto error_out;
> > have_s_grant = 1;
> > gdprintk(XENLOG_INFO, "Exiting Source Stuff \n");
> > }
> >
> > if ( dest_is_gref )
> > {
> > gdprintk(XENLOG_INFO, "Entering dest Stuff \n");
> > gdprintk(XENLOG_INFO, "dest gref is %d \n",op->dest.u.ref);
> > rc = __acquire_grant_for_copy(dd, op->dest.u.ref, 0, &d_frame);
> > if ( rc != GNTST_okay )
> > goto error_out;
> > have_d_grant = 1;
> > gdprintk(XENLOG_INFO, "Exiting dest Stuff \n");
> > }
> > ============================================================================
> > The hunk that you are referring to has a check "scombo.shorts.domid
> > != current->domain->domain_id". This condition gets executed once
> > each while verifying the source and destination grant entry
> > information. The union "Scombo" is nothing but the grant entry, so
> > obviously one of the domains cannot be current domain, which means
> > this check is avoiding a copy across domains.
> I'm not sure I understand this last sentence. First, just to make
> sure that we're talking about the same thing, I'm going to describe
> what I think you're trying to do. You have three domains (let's call
> them A, B, and C), with a source buffer in domain A and a destination
> buffer in domain B. You want to issue a grant copy hypercall in
> domain C to copy the contents of domain A's buffer into domain B's
> buffer. Is that a fair summary?
>
> If so, I'd expect the easiest solution would be for domain A to issue
> domain C with a read-only grant of its buffer, and domain B to issue
> domain C a writable grant of its buffer, and for domain C to then
> issue the hypercall specifying the two grants as source and
> destination operands. If you do it that way, I'd expect both scombos
> to specify domain C, in which case the access control check will pass
> and the hypercall will be allowed. Is that not what you're seeing?
>
> > It's not necessary to take out the whole "if" statement (as in the
> > patch) but it is mandatory to take out the "domainID" check to allow
> > copy across domains.
> The domid check is absolutely necessary to the whole grant tables
> security model, though, so we can't just remove it.
>
> (Unless, as I said before, you're willing to ignore potential security
> issues in order to put together a proof-of-concept quickly.)
>
> Steven.
>
>
> > -----Original Message-----
> > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > Sent: Thursday, February 05, 2009 5:02 PM
> > To: Kumar, Venkat
> > Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> > Subject: Re: [Xen-devel] GNTTAB_copy
> >
> > > This patch is required even for a "copy via grant reference" mechanism.
> > > Patch contains changes to support both gmfn and gref copies.
> > I'd guess you're referring to this hunk?
> >
> > > *** /home/vduvvuru/xen-3.3.0/xen/common/grant_table.c 2008-12-19
> > > 19:41:28.000000000 +0530
> > > --- xen/common/grant_table.c 2009-02-02 21:38:23.000000000 +0530
> > > ***************
> > > *** 1300,1313 ****
> > > for ( ; ; )
> > > {
> > > /* If not already pinned, check the grant domid and type. */
> > > ! if ( !act->pin &&
> > > (((scombo.shorts.flags & GTF_type_mask) !=
> > > GTF_permit_access) ||
> > > (scombo.shorts.domid != current->domain->domain_id)) )
> > > PIN_FAIL(unlock_out, GNTST_general_error,
> > > "Bad flags (%x) or dom (%d). (expected dom
> > > %d)\n",
> > > scombo.shorts.flags, scombo.shorts.domid,
> > > current->domain->domain_id);
> > >
> > > new_scombo = scombo;
> > > new_scombo.shorts.flags |= GTF_reading;
> > > --- 1301,1317 ----
> > > for ( ; ; )
> > > {
> > > /* If not already pinned, check the grant domid and type. */
> > > ! /* if ( !act->pin &&
> > > (((scombo.shorts.flags & GTF_type_mask) !=
> > > GTF_permit_access) ||
> > > (scombo.shorts.domid != current->domain->domain_id)) )
> > > + {
> > > + gdprintk(XENLOG_INFO, "Grant entry Info \n Domid=%d, Flags=%d,
> > > Frame=%d\n",sha->domid,sha->flags,sha->frame);
> > > PIN_FAIL(unlock_out, GNTST_general_error,
> > > "Bad flags (%x) or dom (%d). (expected dom
> > > %d)\n",
> > > scombo.shorts.flags, scombo.shorts.domid,
> > > current->domain->domain_id);
> > > + }*/
> > >
> > > new_scombo = scombo;
> > > new_scombo.shorts.flags |= GTF_reading;
> > This effectively turns off access checking on grant references used in
> > a copy operation, so that any domain is able to access any grant
> > reference, regardless of who the granting domain originally granted
> > access to. That's obviously not ideal from a security point of view.
> >
> > Are you sure you're setting up the grants correctly, and granting
> > access to the right domain?
> >
> > Steven.
> >
> >
> > > -----Original Message-----
> > > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > > Sent: Thursday, February 05, 2009 4:38 PM
> > > To: Kumar, Venkat
> > > Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> > > Subject: Re: [Xen-devel] GNTTAB_copy
> > >
> > >
> > > > Please find the patches attached with this email.
> > > >
> > > > The changes you are interested are in grant_table.patch
> > > Ah, okay. It looks from the patch as if you're using the grant copy
> > > hypercall to do direct MFN-to-MFN copies across a domain boundary,
> > > without any actual grant references getting in the way. Is that
> > > correct? If so, you may want to re-think your design; grant
> > > references are Xen's principle access control mechanism for memory,
> > > and if you're not using them then it's likely to be difficult to make
> > > your design suitably secure.
> > >
> > > (Of course, if this is just a research prototype, you may decide you
> > > don't care about such things, in which case best of luck to you.)
> > >
> > > Steven.
> > >
> > >
> > > > -----Original Message-----
> > > > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > > > Sent: Thursday, February 05, 2009 4:13 PM
> > > > To: Kumar, Venkat
> > > > Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> > > > Subject: Re: [Xen-devel] GNTTAB_copy
> > > >
> > > >
> > > >
> > > > > Atlast it worked out. I had to make some changes in Xen (it wasn't
> > > >
> > > > > allowing a copy across domains) to make it work apart from the patch
> > > >
> > > > > you sent.
> > > >
> > > > That's odd. Would you mind sending me your patch, please, so that I
> > > >
> > > > can see what's wrong with the existing implementation?
> > > >
> > > >
> > > >
> > > > Steven.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > >
> > > > > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > > >
> > > > > Sent: Friday, January 30, 2009 10:04 PM
> > > >
> > > > > To: Kumar, Venkat
> > > >
> > > > > Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> > > >
> > > > > Subject: Re: [Xen-devel] GNTTAB_copy
> > > >
> > > > >
> > > >
> > > > > > Steve,
> > > >
> > > > > >
> > > >
> > > > > > I have ported the GNTTAB_copy part from netchannel2 xen-unstable to
> > > > > > xen-unstable. But the hypercall is failing with a status -1.
> > > >
> > > > > >
> > > >
> > > > > > xm dmesg says
> > > >
> > > > > > ===========================================
> > > >
> > > > > > Bad flags (0) or dom (0). (expected dom 9)
> > > >
> > > > > > ===========================================
> > > >
> > > > > Okay, so Xen started processing the grant table operation, but found
> > > >
> > > > > that the grant entry in the granting domain's grant table was bad
> > > >
> > > > > (because it was full of zeroes). That probably indicates that it was
> > > >
> > > > > accessing completely the wrong gref.
> > > >
> > > > >
> > > >
> > > > > > I am granting the page with a flag ( GTF_reading | GTF_writing &
> > > > > > GTF_permit_access ) but still it did not work.
> > > >
> > > > > >
> > > >
> > > > > > What could be wrong here?
> > > >
> > > > > I'm not sure. I'd suggest you put some printks in your guests (both
> > > >
> > > > > the granting one and the HVM one) to make sure that they agree on the
> > > >
> > > > > grant reference number, and some in Xen to make sure that it's reading
> > > >
> > > > > the hypercall arguments correctly.
> > > >
> > > > >
> > > >
> > > > > Steven.
> > > >
> > > > >
> > > >
> > > > >
> > > >
> > > > > > -----Original Message-----
> > > >
> > > > > > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > > >
> > > > > > Sent: Friday, January 30, 2009 4:52 PM
> > > >
> > > > > > To: Kumar, Venkat
> > > >
> > > > > > Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> > > >
> > > > > > Subject: Re: [Xen-devel] GNTTAB_copy
> > > >
> > > > > >
> > > >
> > > > > > That's unfortunate; the repository works fine for me.
> > > >
> > > > > >
> > > >
> > > > > > There aren't any other ways of downloading the entire repository,
> > > > > > but
> > > >
> > > > > > the changes you need to make are pretty self-contained, so I've
> > > >
> > > > > > attached the relevant cset. You'll need to get rid of the
> > > >
> > > > > > GNTTABOP_set_version bits, but apart from that it should all be
> > > > > > fairly
> > > >
> > > > > > obvious.
> > > >
> > > > > >
> > > >
> > > > > > Steven.
> > > >
> > > > > >
> > > >
> > > > > >
> > > >
> > > > > > > Steven - Thanks for the pointer.
> > > >
> > > > > > > I get an error while downloading the code.
> > > >
> > > > > > >
> > > >
> > > > > > > =======================================================================
> > > >
> > > > > > > hg clone
> > > > > > > http://xenbits.xensource.com/ext/netchannel2/xen-unstable.hg
> > > >
> > > > > > > destination directory: xen-unstable.hg
> > > >
> > > > > > > requesting all changes
> > > >
> > > > > > > adding changesets
> > > >
> > > > > > > transaction abort!
> > > >
> > > > > > > rollback completed
> > > >
> > > > > > > abort: Connection reset by peer
> > > >
> > > > > > > =======================================================================
> > > >
> > > > > > >
> > > >
> > > > > > > Do you know any alternate download?
> > > >
> > > > > > >
> > > >
> > > > > > > Venkat
> > > >
> > > > > > >
> > > >
> > > > > > > -----Original Message-----
> > > >
> > > > > > > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > > >
> > > > > > > Sent: Friday, January 30, 2009 4:18 PM
> > > >
> > > > > > > To: Kumar, Venkat
> > > >
> > > > > > > Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> > > >
> > > > > > > Subject: Re: [Xen-devel] GNTTAB_copy
> > > >
> > > > > > >
> > > >
> > > > > > > > Is GNTTAB_copy command in the Hypercall
> > > > > > > > "HYPERVISOR_grant_table_op"
> > > >
> > > > > > > > supported between two HVM's?
> > > >
> > > > > > > It's not in xen-unstable, no, but it is if you use the netchannel2
> > > >
> > > > > > > hypervisor (available from
> > > >
> > > > > > > http://xenbits.xensource.com/ext/netchannel2/xen-unstable.hg).
> > > > > > > It'd
> > > >
> > > > > > > be pretty easy to cross-port, if you wanted to have a go at that.
> > > > > > > The
> > > >
> > > > > > > interesting cset is this one:
> > > >
> > > > > > >
> > > >
> > > > > > > changeset: 19112:ea4b9c439ac3
> > > >
> > > > > > > user: Steven Smith <steven.smith@xxxxxxxxxxxxx>
> > > >
> > > > > > > date: Thu Jan 22 09:53:12 2009 +0000
> > > >
> > > > > > > files: xen/arch/x86/hvm/hvm.c xen/include/xen/hypercall.h
> > > >
> > > > > > > description:
> > > >
> > > > > > > Allow GNTABOP_copy to be used from HVM domains.
> > > >
> > > > > > >
> > > >
> > > > > > > Steven.
> > >
> > >
> > >
> > >
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
signature.asc
Description: Digital signature
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home