[Xen-devel] [PATCH v3] hvm/altp2m: Clarify the proper way to extend the altp2m interface

[George Dunlap <george.dunlap@xxxxxxxxxx>]

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 <rcojocaru@xxxxxxxxxxxxxxx>
Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
---
Changes since v2:
- Included warning about ALTP2M_mixed in public header as well

Changes since v1:
- Patch description: clarify that HVM guest can't make domctls
- No blacklist for the 'internal' use case; instead document its risk
- Move more of the discussion into the commit message.

CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
CC: Konrad Wilk <konrad.wilk@xxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>
CC: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>
CC: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
CC: Adrian Pop <apop@xxxxxxxxxxxxxxx>
CC: Lars Kurth <lars.kurth@xxxxxxxxxx>
---
 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
-- 
2.18.0


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