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

Re: [Xen-devel] [PATCH v2 03/21] xen: allow console_io hypercalls from certain DomUs


  • To: Julien Grall <julien.grall@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
  • Date: Fri, 17 Aug 2018 15:41:20 -0400
  • Cc: Stefano Stabellini <stefanos@xxxxxxxxxx>, andrii_anisov@xxxxxxxx, George.Dunlap@xxxxxxxxxxxxx, andrew.cooper3@xxxxxxxxxx, ian.jackson@xxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxx, tim@xxxxxxx, jbeulich@xxxxxxxx, wei.liu2@xxxxxxxxxx, nd@xxxxxxx
  • Delivery-date: Fri, 17 Aug 2018 19:41:34 +0000
  • Ironport-phdr: 9a23:d/drbhE6Zp7N+a+PDIZk3J1GYnF86YWxBRYc798ds5kLTJ76osi9bnLW6fgltlLVR4KTs6sC17KI9fi4EUU7or+5+EgYd5JNUxJXwe43pCcHRPC/NEvgMfTxZDY7FskRHHVs/nW8LFQHUJ2mPw6arXK99yMdFQviPgRpOOv1BpTSj8Oq3Oyu5pHfeQpFiCa8bL9oMBm6sRjau9ULj4dlNqs/0AbCrGFSe+RRy2NoJFaTkAj568yt4pNt8Dletuw4+cJYXqr0Y6o3TbpDDDQ7KG81/9HktQPCTQSU+HQRVHgdnwdSDAjE6BH6WYrxsjf/u+Fg1iSWIdH6QLYpUjm58axlVAHnhzsGNz4h8WHYlMpwjL5AoBm8oxBz2pPYbJ2JOPZ7eK7Sc8kaRW5cVchPUSJPDJ63Y48WA+YfIepUqo/wrEYMoxSjHwmhHP7hxCFGhnH23qM03eouHg7E0wM8ENwDq2jUodbvOasOTey4wqvFwDPeZP1Wwzf9743Ifwgvr/6WW7JwcNTeyU0yHA3LkFqbtI3rPymP2esXvWiQ8u1tWv+gi2E6tQ5xrSKvyd03h4nVhoMa1lDE9SJjzIYzPt23UlR3YdGjEJtOriyXMZZ9TMA6Q2xwpSo3xbILtYS7cSQX0pgr2RHSZ+Kdf4SV5B/oSfyfLi1ihH1/fbKynxOy8U+9xeLiTsS0y1NKrjZdktnLq3ANywTf6siZRft5+UeswSqP2BrJ6uFFPEA0jrDXK58nwr4+kZoTqlrMETPslEXqjK6ZakUk+u+y5+ThfrrmvYOTO5VxigH/NqQigs2/AeImPQgSR2WX5Oux2bL58UD5XblGlOM6n6bHvJzAOMgXvqu5DBVU0oYn5Ra/FTCm0NEAkHYaI1JKZQyIj4fzO17UO/34Efe+jEiskDds3fzGOKbhDY/XInjMl7fhY65x61RAxwor0dBf+5VUB6kDIPLuXk/xtcLXDhkjPwy72eboEtF91ocFVG2VGK+ZNbnevkOP5uIqO+OMfpMauC7hK/g54P7jlWQ5mUQBfaazxpQYdnS4HvBnI0WfYHrhmdQBHnkQvgo4UuPqjEeOUTlJZ3a9R6g8/C00CJq6DYffQYCgmLqB0zqgE5JMfGBGD0qAHmvvd4WBQ/0Mcj6dItd9kjwYUrisU4Ag2g+otAPj1rVoMPTU9TMctZ/40Nh15vbTlQ0p9TBuAMWSzWeNQ3tznmMSSD88xLp/rlBlylefzah4hORVGsZV5/xUSAc6NJ/cwPRgBND0WwLBZdCJSEi9T9q4GTE+VNcxz8USbEZ6HtWolgrD0DayA78Ji7yLA4Q58rnA33fvKcZy0XDG1K46j1Q9TcpPNGmmhq959wncHYLGj0KZl6Oyf6QGwCHN7HuDzXaJvExASgFwV7jKXWoBaUrYt9j2+kTCT7i2Cbs5KAtMx9WPJbdLat31l1VGRfjiNM7CbGK2nme6HQyIya+UbIr2Z2Ud2z3QBkkanAAU53aGOhYxCj2vrWLDCjxuEUjgY1v3/OZgtXO3VFM7zwCWb0171rq09QQZiuCbS/MWxrgEojsuqy1oHFah2NLbE9uAqBBnfKlGY9My+ktI1WHCtwx6OJytNL5thkMEfwtrvkPuyw93CoRPkMQwsHwqyw9yI7qC0FxdbzOYwYzwOrrPJ2nw5x+gdbPW2lXf0NmK+qcC8+84q0j4vA63DEYt73Jn09xN2XuG+prKFBYSUY72Uksv9Bh6oLfaYjMn6IzJz3FtP6i0sjvB298yA+sl0AyvcMtbMKyaDgP9D8oaB822Iuwwh1epdg4EPPxV9KMsI8Omdv6G1bWkPel+mjKql2NH4Jpy0kiU7SpzVvbI34oZw/GfxgaGWSnzjFa7vsDzmIBEeC8eE3GjxijlGI5RfKxyfIkRBWiyJM23w4Y2u5m4eWNc9VOlT3wc0cutMU6Qclj80AsW2l4epXiPkDG9iTdzlmdt5pGD0SLHxeOqTwYOMGNPQGhkjB+4OpOohtoXWEypaQkBlxa/40v+ga9Bq/IsAXPURBJkdi73ImUqfqb4maCLasAHvJ8nvShYSu2UfUGRSrm7pQATlSzkAT0Nl3gAazi2t8ChzFRBg2WHISM29SCBdA==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 07/19/2018 05:19 AM, Julien Grall wrote:
Hi Stefano,

On 18/07/18 18:10, Stefano Stabellini wrote:
On Tue, 17 Jul 2018, Julien Grall wrote:
Hi Stefano,

On 17/07/2018 21:05, Stefano Stabellini wrote:
On Mon, 9 Jul 2018, Julien Grall wrote:
Hi,

On 07/07/18 00:11, Stefano Stabellini wrote:
Introduce an is_console option to allow certain classes of domUs to use
the Xen console. Specifically, it will be used to give console access to
all domUs started from Xen from information on device tree.

Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
CC: andrew.cooper3@xxxxxxxxxx
CC: George.Dunlap@xxxxxxxxxxxxx
CC: ian.jackson@xxxxxxxxxxxxx
CC: jbeulich@xxxxxxxx
CC: konrad.wilk@xxxxxxxxxx
CC: tim@xxxxxxx
CC: wei.liu2@xxxxxxxxxx
CC: dgdegra@xxxxxxxxxxxxx
---
Changes in v2:
- introduce is_console
- remove #ifdefs
---
    xen/include/xen/sched.h | 2 ++
    xen/include/xsm/dummy.h | 2 ++
    xen/xsm/flask/hooks.c   | 5 ++++-
    3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 99d2af2..d66cec0 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -379,6 +379,8 @@ struct domain
        bool             auto_node_affinity;
        /* Is this guest fully privileged (aka dom0)? */
        bool             is_privileged;
+    /* Can this guest access the Xen console? */
+    bool             is_console;
        /* Is this a xenstore domain (not dom0)? */
        bool             is_xenstore;
        /* Domain's VCPUs are pinned 1:1 to physical CPUs? */
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index ff6b2db..3888817 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -230,6 +230,8 @@ static XSM_INLINE int
xsm_memory_stat_reservation(XSM_DEFAULT_ARG struct domain
    static XSM_INLINE int xsm_console_io(XSM_DEFAULT_ARG struct domain
*d, int
cmd)
    {
        XSM_ASSERT_ACTION(XSM_OTHER);
+    if ( d->is_console )
+        return xsm_default_action(XSM_HOOK, d, NULL);

I will let Daniel commenting on this change. However ...

    #ifdef CONFIG_VERBOSE_DEBUG
        if ( cmd == CONSOLEIO_write )
            return xsm_default_action(XSM_HOOK, d, NULL);
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 78bc326..2551e4e 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -443,7 +443,10 @@ static int flask_console_io(struct domain *d, int
cmd)
            return avc_unknown_permission("console_io", cmd);
        }
    -    return domain_has_xen(d, perm);
+    if ( !d->is_console )
+        return domain_has_xen(d, perm);
+    else
+        return 0;

... I don't think this change is correct. When a policy is used, the user
is
free to define what is the behavior. With your solution, you impose the
console access even if the user didn't to not give the permission.

I was hoping Daniel would advise on the best way to do things here.

I thought that the idea was that granting a domain "is_console" is
equivalent to granting a domain XEN__READCONSOLE and XEN__WRITECONSOLE
permissions.  Thus, if is_console is set, we return 0 from
flask_console_io because the permissions check succeeds.

Well, yes and no. That's equivalent when you use the dummy policy. When you
have a flask policy you want to give the control to the user.

If you look at the code there are no such as d->is_privilege in that function.
This means that the user define the policy for the hardware domain. Why would
be d->is_console different here?

You are saying that in hooks.c the check should remain exactly as is:

   return domain_has_xen(d, perm);

and d->is_console should not be tested?

Yes.

In that case, do you know if I
need to do anything special with XEN__READCONSOLE and XEN__WRITECONSOLE
permissions for the initial boot domains (such as adding those
permissions as the same time d->is_console is set)?

The main purpose of XSM is to provide a fine grain permission for the user to 
configure. For instance, a user may not console access for initial domain for 
security purpose. So you don't have anything to in the code.

However, when you have XSM enabled, you will have to write down in the policy 
that initial domains will have console access. Although, I am not sure how to 
write that in the policy.

In a security policy that wishes to make this distinction, the initial domains 
will
have distinct security labels from domains created later.  Then, those types are
allowed console_write access (along with any other special rights they may 
need).

_______________________________________________
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®.