[xen staging] hvm/altp2m: Clarify the proper way to extend the altp2m interface

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[xen staging] hvm/altp2m: Clarify the proper way to extend the altp2m interface

patchbot
commit 6aaa9fb308171ec58ddf4cf058ad5341f81a65cf
Author:     George Dunlap <[hidden email]>
AuthorDate: Tue Jul 31 15:17:21 2018 +0100
Commit:     George Dunlap <[hidden email]>
CommitDate: Tue Jul 31 16:14:01 2018 +0100

    hvm/altp2m: Clarify the proper way to extend the altp2m interface
   
    The altp2m functionality was originally envisioned to be used in
    several different configurations, one of which was a single in-guest
    agent that had full operational control of altp2m.  This required the
    single hypercall to be an HVMOP rather than a DOMCTL, since HVM guests
    are not allowed to make DOMCTLs.  Access to this HVMOP is controlled
    by a per-domain HVM_PARAM, and defaults to 'off'.
   
    Exposing the altp2m functionality to the guest was controversial at
    the time, but was ultimately accepted.  The fact that altp2m is an
    HVMOP rather than a DOMCTL has caused some problems, however, for
    those moving forward trying to extend the interface: Extending the
    interface even for the 'external' use case now means extending an
    HVMOP, which implicitly extends the surface of attack for the
    'internal' use case as well.  The result has been that every addition
    to this interface has also been controversial.
   
    Settle the controversy once and for all by documenting 1) the purpose
    of the altp2m interface, and 2) how to extend it.  In particular:
   
    * Specify that the fully in-guest agent is a target use case
   
    * Specify that all extensions to altp2m functionality should be subops
      of the HVMOP hypercall
   
    * Specify that new subops should be enabled in ALTP2M_mixed mode by
      default, but that this mode has not been evaluated for safety.
   
    Hopefully this will allow the altp2m interface to be developed further
    without unnecessary controversy.
   
    Further discussion:
   
    As far as I can tell there are three possible solutions to this
    controversy.
   
    A. Remove the 'internal' functionality as a target by converting the
    current HVMOP into a DOMCTL.
   
    B. Have two hypercalls -- an HVMOP which contains functionality
    expected to be used by the 'internal' agent, and a DOMCTL for
    functionality which is expected to be used only be the 'external'
    agent.
   
    C. Agree to add all new subops to the current hypercall (HVMOP), even
    if we're not sure if they should be exposed to the guest.
   
    I think A is a terrible idea.  Having a single in-guest agent is a
    reasonable design choice, and apparently it was even implemented at
    some point; we should make it straightforward for someone in the
    future to pick up the work if they want to.
   
    I think B is also a bad idea.  The people extending it at the moment
    are primarily concerned with the 'external' use case.  There is nobody
    around to represent whether new functionality should end up in the
    HVMOP or the DOMCTL, which means that by default it will end up in the
    DOMCTL.  If it is discovered, afterwards, that the new operations
    *would* be safe and useful for the 'internal' use case, then we will
    either have to duplicate them inside the HVMOP (which would be
    terrible) or move the operation from the DOMCTL to the HVMOP (which
    would make coding an agent against several versions a mess).
   
    It just makes more sense to have all the altp2m operations in a single
    place, and a simple way to control whether they're available to the
    'internal' use case or not.  As such, I am proposing 'C'.
   
    Even within that, we have several options as far as what to do with
    the current interface:
   
    C1: Audit the current subops and make a blacklist of subops not
    suitable for exposure to the guest.  Future subops should be on the
    blacklist unless they have been evaluated as safe for exposure.
   
    C2: Don't blacklist the current subops, but require that all future
    subops be blacklisted unless they have been evaluated as safe for
    exposure.
   
    C3: Don't blacklist current or future subops for the present; just
    document that they need to be evaluated (and some potentially
    blacklisted) before being exposed to a guest in a safety-critical
    environment.
   
    C1 would be ideal, but there's nobody at present to do the work.
    Given that, C3 has been seen as the best solution in discussion.
   
    Reviewed-by: Razvan Cojocaru <[hidden email]>
    Acked-by: Wei Liu <[hidden email]>
    Signed-off-by: George Dunlap <[hidden email]>
---
 xen/arch/x86/hvm/hvm.c          | 31 +++++++++++++++++++++++++++++++
 xen/include/public/hvm/params.h |  5 +++++
 2 files changed, 36 insertions(+)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 67b99af334..5de544093e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4467,6 +4467,37 @@ static int hvmop_get_param(
     return rc;
 }
 
+/*
+ * altp2m operations are envisioned as being used in several different
+ * modes:
+ *
+ * - external: All control and decisions are made by an external agent
+ *   running domain 0.
+ *
+ * - internal: altp2m operations are used exclusively by an in-guest
+ *   agent to protect itself from the guest kernel and in-guest
+ *   attackers.
+ *
+ * - coordinated: An in-guest agent handles #VE and VMFUNCs locally,
+ *   but makes requests of an agent running outside the domain for
+ *   bigger changes (such as modifying altp2m entires).
+ *
+ * This corresponds to the three values for HVM_PARAM_ALTP2M
+ * (external, mixed, limited). All three models have advantages and
+ * disadvantages.
+ *
+ * Normally hypercalls made by a program in domain 0 in order to
+ * control a guest would be DOMCTLs rather than HVMOPs.  But in order
+ * to properly enable the 'internal' use case, as well as to avoid
+ * fragmentation, all altp2m subops should come under this single
+ * HVMOP.
+ *
+ * Note that 'internal' mode (HVM_PARAM_ALTP2M == XEN_ALTP2M_mixed)
+ * has not been evaluated for safety from a security perspective.
+ * Before using this mode in a security-critical environment, each
+ * subop should be evaluated for safety, with unsafe subops
+ * blacklisted in xsm_hvm_altp2mhvm_op().
+ */
 static int do_altp2m_op(
     XEN_GUEST_HANDLE_PARAM(void) arg)
 {
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 2ec2e7c80f..daa28a86be 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -239,6 +239,11 @@
  *  mixed: allow access to all altp2m ops for both in-guest and external tools
  *  external: allow access to external privileged tools only
  *  limited: guest only has limited access (ie. control VMFUNC and #VE)
+ *
+ * Note that 'mixed' mode has not been evaluated for safety from a
+ * security perspective.  Before using this mode in a
+ * security-critical environment, each subop should be evaluated for
+ * safety, with unsafe subops blacklisted in xsm_hvm_altp2mhvm_op().
  */
 #define HVM_PARAM_ALTP2M       35
 #define XEN_ALTP2M_disabled      0
--
generated by git-patchbot for /home/xen/git/xen.git#staging

_______________________________________________
Xen-changelog mailing list
[hidden email]
https://lists.xenproject.org/xen-changelog