All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: StefanoStabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Tim Deegan <tim@xen.org>, David Vrabel <david.vrabel@citrix.com>,
	Anthony Perard <anthony.perard@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	dgdegra@tycho.nsa.gov
Subject: Re: Device model operation hypercall (DMOP, re qemu depriv)
Date: Wed, 3 Aug 2016 14:37:02 +0100	[thread overview]
Message-ID: <22433.62334.847018.432276@mariner.uk.xensource.com> (raw)
In-Reply-To: <57A1F9CB0200007800102312@prv-mh.provo.novell.com>

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu depriv)"):
> On 03.08.16 at 12:29, <ian.jackson@eu.citrix.com> wrote:
> > I thought it useful to set out the DMOP proposal from first
> > principles, with clear motivation, discussion of not-chosen
> > alternatives, and of course with a clear statement of the principles
> > of operation and of the security design.
> 
> Okay; nevertheless I did get the feeling that some of this was
> prompted by the hvmctl series posting.

Well, I think the HVMCTL series is an important part of this
conversation - even though this DMOP idea is motivated by the qemu
depriv proposals (initiated mostly by Stefano in the last release
cycle).

> > Does that mean that functionality exposed by all the prooposed HVMCTLs
> > is currently available to stubdoms ?
> 
> That series only moves code from one hypercall to another (new) one,
> without any security implications at all. What has been available to
> stubdoms prior to that series will be available the same way once it
> got applied.

So what you mean is that in HVMCTL, the privilege check is retained in
the individual HVMCTL sub-operation ?  (Ie what used to be IS_PRIV or
IS_PRIV_FOR - and implicitly, some sub-ops would be IS_PRIV and some
IS_PRIV_FOR.)

But looking at your v2 01/11, I see this in do_hvmctl:

   +    rc = xsm_hvm_control(XSM_DM_PRIV, d, op.cmd);
   +    if ( rc )
   +    {
   +        rcu_unlock_domain(d);
   +        return rc;
   +    }

And looking at v2 02/11, I didn't see any privilege check in the
specific hypercall.

With DMOP it would make sense for the privilege check to be
IS_PRIV_FOR, in the DMOP dispatcher.  But it seems that this privilege
check already exists in HVMCTL in the right form.

So AFAICT HVMCTLs already guarantee not to have an adverse impact on
the whole system.  If that weren't the case, then a stub dm could
exploit the situation.

Is the audit that is required, to check that the DMOP doesn't have an
adverse effect on the _calling domain_ ?  AFAICT most HVMCTLs/DMOPs
have /no/ effect on the calling domain, other than as part of argument
passing.  So that audit should be easy.

So I am confused.  What am I missing ?

> > If the 01/ patch contains such promises, then logically the 02/ patch
> > which introduces the first DMOP is extending that promise to that
> > operation.  It is at that point that the security decision should be
> > made.
> 
> Correct. Yet again the original goal of the series was just proper
> separation of two groups of operations that should never have
> been all thrown under the same hypercall.

What I am doing is presenting another, additional goal.  I don't
dispute the usefulness of the HVMCTL cleanup.  But I want to
understand our future intentions.

In particular, as a proponent of the DMOP idea, I want to avoid the
situation that my idea is blocked somehow by a conflicting series.

> > Now, there may be other ways to represent/record the security status.
> > But it will be necessary to either (i) avoid violating the DMOP
> > security promise, by making questionable calls not available via DMOP
> > or (ii) trying to retrofit the security promise to DMOP later.
> > 
> > I think (ii) is not a good approach.  It would amount to introducing a
> > whole new set of interfaces, and then later trying to redefine them to
> > have a particular security property which was not originally there.
> 
> I agree that (i) would be the better approach, but I don't think I
> can assess when I would find the time to do the required auditing
> of all involved code. Plus I don't see the difference between going
> the (ii) route with the presented hvmctl series vs keeping things as
> they are (under hvmop) - in both cases the security promise will
> need to be retrofit onto existing code.

If we don't apply HVMCTL, we can introduce DMOP and then individually
move hypercalls from hvmop to DMOP as they are audited.

Would a similar approach be acceptable even after HVMCTL ?

That is, the following plan:

1. Apply HVMCTL right away.  This solves the cleanup problem,
   but leaves the qemu depriv problem unsolved.

2. After the necessary discussion to understand and refine the DMOP
   design, create a new DMOP hypercall with appropriate security
   promises and whatever stability promises are agreed, but with no
   sub-ops.

3. Move each HVMCTL to DMOP, one by one, as it is audited.

4. Perhaps some HVMCTLs will remain which are inherently unsuitable
   for use with qemu depriv.  If not, perhaps eventually delete HVMCTL
   entirely, or leave it empty (with no subops).

This has the downside that we end up moving all the DMOPs twice.  But
it does allow us to separate the audit work from the cleanup/reorg.

Regards,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

  reply	other threads:[~2016-08-03 13:37 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-28 17:01 XenProject/XenServer QEMU working group, Friday 8th July, 2016, 15:00 Jennifer Herbert
2016-08-01 11:32 ` Device model operation hypercall (DMOP, re qemu depriv) Ian Jackson
2016-08-01 12:41   ` Jan Beulich
2016-08-02 11:38     ` Wei Liu
2016-08-02 11:58       ` Jan Beulich
2016-08-02 13:02         ` David Vrabel
2016-08-02 13:29           ` Jan Beulich
2016-08-03 10:29       ` Ian Jackson
2016-08-03 12:03         ` Jan Beulich
2016-08-03 13:37           ` Ian Jackson [this message]
2016-08-03 14:16             ` Jan Beulich
2016-08-03 14:21               ` George Dunlap
2016-08-03 16:10                 ` Ian Jackson
2016-08-03 16:18                   ` Jan Beulich
2016-08-04 11:21                     ` Ian Jackson
2016-08-04 13:24                       ` Jan Beulich
2016-08-05 16:28                         ` Ian Jackson
2016-08-08 11:18                           ` Jan Beulich
2016-08-08 13:46                             ` Ian Jackson
2016-08-08 14:07                               ` Jan Beulich
2016-08-26 11:38                                 ` Ian Jackson
2016-08-26 12:58                                   ` Jan Beulich
2016-08-26 14:35                                     ` Ian Jackson
2016-08-26 15:13                                       ` Jan Beulich
2016-08-30 11:02                                         ` Ian Jackson
2016-08-30 21:47                                           ` Stefano Stabellini
2016-09-02 14:08                                           ` Wei Liu
2016-08-09 10:29                               ` Jan Beulich
2016-08-09 10:48                                 ` Ian Jackson
2016-08-09 11:30                                   ` Jan Beulich
2016-08-12  9:44                                     ` George Dunlap
2016-08-12 11:50                                       ` Jan Beulich
2016-08-15  9:39                                         ` George Dunlap
2016-08-15 10:19                                           ` Jan Beulich
2016-08-15 10:47                                             ` George Dunlap
2016-08-15 11:20                                               ` Jan Beulich
2016-08-15 12:07                                                 ` Ian Jackson
2016-08-15 14:20                                                   ` Jan Beulich
2016-08-15 14:57                                                 ` George Dunlap
2016-08-15 15:22                                                   ` Jan Beulich
2016-08-15 14:50                                 ` David Vrabel
2016-08-15 15:24                                   ` Jan Beulich
2016-08-26 11:29                                     ` Ian Jackson
2016-08-26 12:58                                       ` Jan Beulich
2016-08-02 11:37   ` Wei Liu
2016-08-02 11:42     ` George Dunlap
2016-08-02 12:34       ` Wei Liu
2016-09-09 15:16   ` Jennifer Herbert
2016-09-09 15:34     ` David Vrabel
2016-09-12 13:47     ` George Dunlap
2016-09-12 14:32     ` Jan Beulich
2016-09-13 10:37       ` George Dunlap
2016-09-13 11:53         ` Jan Beulich
2016-09-13 16:07       ` David Vrabel
2016-09-14  9:51         ` Jan Beulich
2016-09-21 11:21           ` Ian Jackson
2016-09-21 11:28             ` George Dunlap
2016-09-21 11:58               ` Jan Beulich
2016-09-21 11:55             ` Jan Beulich
2016-09-21 12:23               ` Device model operation hypercall (DMOP, re qemu depriv) [and 1 more messages] Ian Jackson
2016-09-21 12:48                 ` Jan Beulich
2016-09-21 13:24                   ` Ian Jackson
2016-09-21 13:56                     ` Jan Beulich
2016-09-21 15:06                       ` Ian Jackson
2016-09-21 17:09                       ` George Dunlap
2016-09-22  8:47                         ` Jan Beulich
2016-09-09 16:18 ` XenProject/XenServer QEMU working group minutes, 30th August 2016 Jennifer Herbert
2016-09-12  7:16   ` Juergen Gross
2016-10-14 18:01   ` QEMU XenServer/XenProject Working group meeting 29th September 2016 Jennifer Herbert
2016-10-18 19:54     ` Stefano Stabellini
2016-10-20 17:37       ` Lars Kurth
2016-10-20 18:53         ` Stefano Stabellini
2017-02-28 18:18     ` QEMU XenServer/XenProject Working group meeting 10th February 2017 Jennifer Herbert
2017-06-05 13:48       ` QEMU XenServer/XenProject Working group meeting 10th May 2017 Jennifer Herbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22433.62334.847018.432276@mariner.uk.xensource.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=david.vrabel@citrix.com \
    --cc=dgdegra@tycho.nsa.gov \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.