[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 1/6] docs/qemu-deprivilege: Revise and update with status and future plans


  • To: Ian Jackson <ian.jackson@xxxxxxxxxx>
  • From: George Dunlap <george.dunlap@xxxxxxxxxx>
  • Date: Mon, 24 Sep 2018 14:08:35 +0100
  • Autocrypt: addr=george.dunlap@xxxxxxxxxx; prefer-encrypt=mutual; keydata= xsFNBFPqG+MBEACwPYTQpHepyshcufo0dVmqxDo917iWPslB8lauFxVf4WZtGvQSsKStHJSj 92Qkxp4CH2DwudI8qpVbnWCXsZxodDWac9c3PordLwz5/XL41LevEoM3NWRm5TNgJ3ckPA+J K5OfSK04QtmwSHFP3G/SXDJpGs+oDJgASta2AOl9vPV+t3xG6xyfa2NMGn9wmEvvVMD44Z7R W3RhZPn/NEZ5gaJhIUMgTChGwwWDOX0YPY19vcy5fT4bTIxvoZsLOkLSGoZb/jHIzkAAznug Q7PPeZJ1kXpbW9EHHaUHiCD9C87dMyty0N3TmWfp0VvBCaw32yFtM9jUgB7UVneoZUMUKeHA fgIXhJ7I7JFmw3J0PjGLxCLHf2Q5JOD8jeEXpdxugqF7B/fWYYmyIgwKutiGZeoPhl9c/7RE Bf6f9Qv4AtQoJwtLw6+5pDXsTD5q/GwhPjt7ohF7aQZTMMHhZuS52/izKhDzIufl6uiqUBge 0lqG+/ViLKwCkxHDREuSUTtfjRc9/AoAt2V2HOfgKORSCjFC1eI0+8UMxlfdq2z1AAchinU0 eSkRpX2An3CPEjgGFmu2Je4a/R/Kd6nGU8AFaE8ta0oq5BSFDRYdcKchw4TSxetkG6iUtqOO ZFS7VAdF00eqFJNQpi6IUQryhnrOByw+zSobqlOPUO7XC5fjnwARAQABzSRHZW9yZ2UgVy4g RHVubGFwIDxkdW5sYXBnQHVtaWNoLmVkdT7CwYAEEwEKACoCGwMFCwkIBwMFFQoJCAsFFgID AQACHgECF4ACGQEFAlpk2IEFCQo9I54ACgkQpjY8MQWQtG1A1BAAnc0oX3+M/jyv4j/ESJTO U2JhuWUWV6NFuzU10pUmMqpgQtiVEVU2QbCvTcZS1U/S6bqAUoiWQreDMSSgGH3a3BmRNi8n HKtarJqyK81aERM2HrjYkC1ZlRYG+jS8oWzzQrCQiTwn3eFLJrHjqowTbwahoiMw/nJ+OrZO /VXLfNeaxA5GF6emwgbpshwaUtESQ/MC5hFAFmUBZKAxp9CXG2ZhTP6ROV4fwhpnHaz8z+BT NQz8YwA4gkmFJbDUA9I0Cm9D/EZscrCGMeaVvcyldbMhWS+aH8nbqv6brhgbJEQS22eKCZDD J/ng5ea25QnS0fqu3bMrH39tDqeh7rVnt8Yu/YgOwc3XmgzmAhIDyzSinYEWJ1FkOVpIbGl9 uR6seRsfJmUK84KCScjkBhMKTOixWgNEQ/zTcLUsfTh6KQdLTn083Q5aFxWOIal2hiy9UyqR VQydowXy4Xx58rqvZjuYzdGDdAUlZ+D2O3Jp28ez5SikA/ZaaoGI9S1VWvQsQdzNfD2D+xfL qfd9yv7gko9eTJzv5zFr2MedtRb/nCrMTnvLkwNX4abB5+19JGneeRU4jy7yDYAhUXcI/waS /hHioT9MOjMh+DoLCgeZJYaOcgQdORY/IclLiLq4yFnG+4Ocft8igp79dbYYHkAkmC9te/2x Kq9nEd0Hg288EO/OwE0EVFq6vQEIAO2idItaUEplEemV2Q9mBA8YmtgckdLmaE0uzdDWL9To 1PL+qdNe7tBXKOfkKI7v32fe0nB4aecRlQJOZMWQRQ0+KLyXdJyHkq9221sHzcxsdcGs7X3c 17ep9zASq+wIYqAdZvr7pN9a3nVHZ4W7bzezuNDAvn4EpOf/o0RsWNyDlT6KECs1DuzOdRqD oOMJfYmtx9hMzqBoTdr6U20/KgnC/dmWWcJAUZXaAFp+3NYRCkk7k939VaUpoY519CeLrymd Vdke66KCiWBQXMkgtMGvGk5gLQLy4H3KXvpXoDrYKgysy7jeOccxI8owoiOdtbfM8TTDyWPR Ygjzb9LApA8AEQEAAcLBZQQYAQoADwIbDAUCWmTXMwUJB+tP9gAKCRCmNjwxBZC0bb+2D/9h jn1k5WcRHlu19WGuH6q0Kgm1LRT7PnnSz904igHNElMB5a7wRjw5kdNwU3sRm2nnmHeOJH8k Yj2Hn1QgX5SqQsysWTHWOEseGeoXydx9zZZkt3oQJM+9NV1VjK0bOXwqhiQyEUWz5/9l467F S/k4FJ5CHNRumvhLa0l2HEEu5pxq463HQZHDt4YE/9Y74eXOnYCB4nrYxQD/GSXEZvWryEWr eDoaFqzq1TKtzHhFgQG7yFUEepxLRUUtYsEpT6Rks2l4LCqG3hVD0URFIiTyuxJx3VC2Ta4L H3hxQtiaIpuXqq2D4z63h6vCx2wxfZc/WRHGbr4NAlB81l35Q/UHyMocVuYLj0llF0rwU4Aj iKZ5qWNSEdvEpL43fTvZYxQhDCjQTKbb38omu5P4kOf1HT7s+kmQKRtiLBlqHzK17D4K/180 ADw7a3gnmr5RumcZP3NGSSZA6jP5vNqQpNu4gqrPFWNQKQcW8HBiYFgq6SoLQQWbRxJDHvTR YJ2ms7oCe870gh4D1wFFqTLeyXiVqjddENGNaP8ZlCDw6EU82N8Bn5LXKjR1GWo2UK3CjrkH pTt3YYZvrhS2MO2EYEcWjyu6LALF/lS6z6LKeQZ+t9AdQUcILlrx9IxqXv6GvAoBLJY1jjGB q+/kRPrWXpoaQn7FXWGfMqU+NkY9enyrlw==
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>, Konrad Wilk <konrad.wilk@xxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>, Julien Grall <julien.grall@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 24 Sep 2018 13:08:47 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Openpgp: preference=signencrypt

On 09/24/2018 11:23 AM, Ian Jackson wrote:
> George Dunlap writes ("[PATCH v2 1/6] docs/qemu-deprivilege: Revise and 
> update with status and future plans"):
>> +## Xen library / file-descriptor restrictions
>> +
>> +'''Description''': Close and restrict Xen-related file descriptors.
>> +Specifically:
>> + * Close all xenstore-related file descriptors
> 
> This is correct.
> 
>> + * Make sure that extraneous `privcmd` and `evtchn` instances are
>> +closed
> 
> No, *all* privcmd and evtchn instances are restricted, even
> `extraneous' ones which have been leaked by qemu.  None are closed.

Ack.

>> +'''How to test''':
>> +
>> +    tools/test/depriv/depriv-fd-checker.c
> 
> You also need the tool `fishdescriptor' from src:chiark-utils to get
> the descriptors out of qemu.  It is in chiark-utils-bin in Debian
> buster and Debian stretch-backports.

This was meant to be a somewhat technical description of the mechanism
of doing the testing (to be implemented by someone implementing the
feature), rather than a how-to for keen users / testers to actually run
the test.  What about:

"Use `fishdescriptor` from [reference], to pull a file descriptor from a
running QEMU, then check that it has the desired properties, and that
hypercalls which are meant to fail do fail.  (The latter is implemented
in `tools/test/depriv/depriv-fd-checker.c`)."

On a related note: Is there any reason not to  move
osstest-depriv-fd-collector into the tree, perhaps even merging it with
the functionality in depriv-process-checker?

>> +## Namespaces for unused functionality (Linux only)
>> +
>> +'''Description''': Enter QEMU into its own mount & IPC namespaces.
>> +This means that even if other restrictions fail, the process won't be
>> +able to even name system mount points or exsting non-file-based IPC
>> +descriptors to attempt to attack them.
>> +
>> +'''Implementation''':
>> +
>> +In theory this could be done in QEMU (similar to -sandbox, -runas,
>> +-chroot, and so on), but a patch doing this in QEMU was NAKed
>> +upstream. They preferred that this was done as a setup step by
>> +whatever executes QEMU; i.e., have the process which exec's QEMU first
>> +call:
>> +
>> +    unshare(CLONE_NEWNS | CLONE_NEWIPC)
> 
> If you are recording this kind of information here: this will of
> course not work, because qemu binds and opens things at startup that
> would be broken by this.  Maybe you want to give a url to a mailing
> list posting instead of this un-referenced hearsay.
> 
>> +### Network namespacing (Linux only)
>> +
>> +Enter QEMU into its own network namespace (in addition to mount & IPC
>> +namespaces).  Basically change the 'unshare' call to be as follows:
>> +
>> +    unshare(CLONE_NEWNET | CLONE_NEWNS | CLONE_NEWIPC)
>> +
>> +QEMU does actually use the network namespace by default, so adding
>> +this restriction requires additional changes, listed below.
> 
> The CLONE_NEWIPC overlaps with the IPC unshare discussed above.

This is the second time I've had to try to explain the difference
between the above two items; I'm not sure what's not clear about what
was written.

The title of the first says: "...for unused functionaltiy".  IPC
namespaces are for non-file-based IPCs (i.e., things which are not unix
sockets).  QEMU does not use this functionality, nor does it use mount
functionality.  The first restriction is in fact implemented in patch 4,
and I haven't had any issues with it.

I suppose I need more text like the above, explicitly defining what
those namespaces do, and that QEMU doesn't need them?

The second section explicitly mentions the fact that this will not work,
and references further changes which would be required for such a change
to be implemented (in the 'Network' and 'VNC' sections).

I can certainly provide a reference if you think that's important.

Any other suggestions for clarification, so that we don't have to repeat
this discussion again, would be welcome.

>> +## Setting up a userid range
> 
> There was some discussion on a Debian list recently about some
> container systems that encode a 16-bit within-container uid and a
> 16-bit container number into the 32-bit uid.  I guess we don't need to
> explicitly worry about clashes between our usage and those ?

Hmm, someone may run containers that use such things in dom0, at which
point we may have a namespace collision.

But really I think this is a distro problem to solve -- we don't specify
a >16-bit UID, we just give it as an example.  Debian could, for instance:
 - Not use the system which uses the 16/16 split
 - Enforce that Xen and the 16/16 split system are not installed at the
same time
 - Reserve 32k of UIDs in the 16-bit space somehow
 - Reserve one of the "container ID" entries for Xen, so that there's
never a collision

>> +# Limitations
>> +
>> +The following features still need to be implemented:
>> + * Inserting a new cdrom while the guest is running (xl cdrom-insert)
>> + * Migration / save / restore
>> +
>> +Additionally, getting PCI passthrough to work securely would require a
>> +significant rework of how passthrough works at the moment.  It may be
>> +implemented at some point but is not a near-term priority.
> 
> The limitations section should also say something like this:
> 
>  The currently implemented restrictions are thought to be a useful
>  security improvement.  However, the design and implementation is
>  preliminary and there is work left to do.  Accordingly we do not
>  promise that they are sufficient to stop a rogue domain which takes
>  control of its qemu from escaping into the host, let alone stop it
>  from denying service to the host.

Isn't this what "Tech preview" means?  Or do you mean we'll keep this
kind of warning in after we take it out of 'tech preview'?

>  Therefore, bugs which affect the effectiveness of the qemu depriv
>  mechanisms will be treated as plain bugs, not security bugs; they
>  would not result in a Xen Project Security Advisory.  However, bugs
>  where the security of a system with dm_restrict=1 is worse than
>  before, will be treated as security bugs.

This would be slightly different than 'tech preview'.

Once this goes to "supported", I agree that we shouldn't issue an XSA
for, say, a bug in Linux's implementation of RLIMIT_NPROC, or a bug in
Linux that allows QEMU, while running as an unprivileged process, to do
something it's not supposed to do (say, fill up our chroot, which is
owned by root).

But I do think we should issue an XSA if there is code in libxl which
claims to do something but failed.  For instance, if a change
accidentally disables the `-runas` option to QEMU when dm_restrict=1,
then that would merit an XSA in my opinion.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.