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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Mon, Jul 23, 2018 at 11:11 AM George Dunlap <george.dunlap@xxxxxxxxxx> wrote:
>
> 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.
>
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> ---
> 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 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 1816faa9b3..84eed1f4bc 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)
>  {
> --
> 2.18.0

Thanks for putting this together, SGTM!

Tamas

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