|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3] xenstore: remove XS_RESTRICT support
> On 1 Feb 2017, at 14:49, Juergen Gross <jgross@xxxxxxxx> wrote:
>
> On 27/01/17 12:47, Juergen Gross wrote:
>> XS_RESTRICT and the xenstore library function xs_restrict() have never
>> been usable in all configurations and there are no known users.
>>
>> This functionality was thought to limit access rights of device models
>> to xenstore in order to avoid affecting other domains in case of a
>> security breech. Unfortunately XS_RESTRICT won't help as current
>> qemu is requiring access to dom0 only accessible xenstore paths to
>> work correctly. So this command is useless and should be removed.
>>
>> In order to avoid problems in the future remove all support for
>> XS_RESTRICT from xenstore.
>>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>
> Adding Dave Scott to Cc: list.
Looks fine to me:
Acked-by: David Scott <dave@xxxxxxxxxx>
Cheers,
Dave
>
>
> Juergen
>
>> ---
>> V3: remove restrict functions in ocaml as requested by Wei Liu
>>
>> V2: don't replace XS_RESTRICT enum member with a dummy one as suggested
>> by Andrew Cooper
>> ---
>> tools/ocaml/libs/xb/op.ml | 5 ++---
>> tools/ocaml/libs/xb/xb.mli | 1 -
>> tools/ocaml/xenstored/connection.ml | 3 ---
>> tools/ocaml/xenstored/logging.ml | 1 -
>> tools/ocaml/xenstored/perms.ml | 5 -----
>> tools/ocaml/xenstored/process.ml | 16 ----------------
>> tools/xenstore/include/xenstore.h | 9 ---------
>> tools/xenstore/xenstored_core.c | 1 -
>> tools/xenstore/xs.c | 8 --------
>> xen/include/public/io/xs_wire.h | 4 ++--
>> 10 files changed, 4 insertions(+), 49 deletions(-)
>>
>> diff --git a/tools/ocaml/libs/xb/op.ml b/tools/ocaml/libs/xb/op.ml
>> index 69346d8..d4f1f08 100644
>> --- a/tools/ocaml/libs/xb/op.ml
>> +++ b/tools/ocaml/libs/xb/op.ml
>> @@ -19,7 +19,7 @@ type operation = Debug | Directory | Read | Getperms |
>> Transaction_end | Introduce | Release |
>> Getdomainpath | Write | Mkdir | Rm |
>> Setperms | Watchevent | Error | Isintroduced |
>> - Resume | Set_target | Restrict | Reset_watches |
>> + Resume | Set_target | Reset_watches |
>> Invalid
>>
>> let operation_c_mapping =
>> @@ -28,7 +28,7 @@ let operation_c_mapping =
>> Transaction_end; Introduce; Release;
>> Getdomainpath; Write; Mkdir; Rm;
>> Setperms; Watchevent; Error; Isintroduced;
>> - Resume; Set_target; Restrict; Reset_watches |]
>> + Resume; Set_target; Reset_watches |]
>> let size = Array.length operation_c_mapping
>>
>> let array_search el a =
>> @@ -68,6 +68,5 @@ let to_string ty =
>> | Isintroduced -> "IS_INTRODUCED"
>> | Resume -> "RESUME"
>> | Set_target -> "SET_TARGET"
>> - | Restrict -> "RESTRICT"
>> | Reset_watches -> "RESET_WATCHES"
>> | Invalid -> "INVALID"
>> diff --git a/tools/ocaml/libs/xb/xb.mli b/tools/ocaml/libs/xb/xb.mli
>> index 6c242da..b4d7052 100644
>> --- a/tools/ocaml/libs/xb/xb.mli
>> +++ b/tools/ocaml/libs/xb/xb.mli
>> @@ -22,7 +22,6 @@ module Op :
>> | Isintroduced
>> | Resume
>> | Set_target
>> - | Restrict
>> | Reset_watches
>> | Invalid
>> val operation_c_mapping : operation array
>> diff --git a/tools/ocaml/xenstored/connection.ml
>> b/tools/ocaml/xenstored/connection.ml
>> index 3ffd35b..27fa778 100644
>> --- a/tools/ocaml/xenstored/connection.ml
>> +++ b/tools/ocaml/xenstored/connection.ml
>> @@ -122,9 +122,6 @@ let close con =
>> let get_perm con =
>> con.perm
>>
>> -let restrict con domid =
>> - con.perm <- Perms.Connection.restrict con.perm domid
>> -
>> let set_target con target_domid =
>> con.perm <- Perms.Connection.set_target (get_perm con)
>> ~perms:[Perms.READ; Perms.WRITE] target_domid
>>
>> diff --git a/tools/ocaml/xenstored/logging.ml
>> b/tools/ocaml/xenstored/logging.ml
>> index c52f03d..0c0d03d 100644
>> --- a/tools/ocaml/xenstored/logging.ml
>> +++ b/tools/ocaml/xenstored/logging.ml
>> @@ -241,7 +241,6 @@ let string_of_access_type = function
>> | Xenbus.Xb.Op.Mkdir -> "mkdir "
>> | Xenbus.Xb.Op.Rm -> "rm "
>> | Xenbus.Xb.Op.Setperms -> "setperms "
>> - | Xenbus.Xb.Op.Restrict -> "restrict "
>> | Xenbus.Xb.Op.Reset_watches -> "reset watches"
>> | Xenbus.Xb.Op.Set_target -> "settarget"
>>
>> diff --git a/tools/ocaml/xenstored/perms.ml b/tools/ocaml/xenstored/perms.ml
>> index 19bf44c..3ea193e 100644
>> --- a/tools/ocaml/xenstored/perms.ml
>> +++ b/tools/ocaml/xenstored/perms.ml
>> @@ -119,11 +119,6 @@ let is_owner (connection:t) id =
>> let is_dom0 (connection:t) =
>> is_owner connection 0
>>
>> -let restrict (connection:t) domid =
>> - match connection.target, connection.main with
>> - | None, (0, perms) -> { connection with main = (domid, perms) }
>> - | _ -> raise Define.Permission_denied
>> -
>> let elt_to_string (i,p) =
>> Printf.sprintf "%i%S" i (String.concat "" (List.map String.of_char
>> (List.map char_of_permty p)))
>>
>> diff --git a/tools/ocaml/xenstored/process.ml
>> b/tools/ocaml/xenstored/process.ml
>> index 7b60376..963549d 100644
>> --- a/tools/ocaml/xenstored/process.ml
>> +++ b/tools/ocaml/xenstored/process.ml
>> @@ -172,30 +172,16 @@ let do_isintroduced con t domains cons data =
>> in
>> if domid = Define.domid_self || Domains.exist domains domid then
>> "T\000" else "F\000"
>>
>> -(* [restrict] is in the patch queue since xen3.2 *)
>> -let do_restrict con t domains cons data =
>> - if not (Connection.is_dom0 con)
>> - then raise Define.Permission_denied;
>> - let domid =
>> - match (split None '\000' data) with
>> - | [ domid; "" ] -> c_int_of_string domid
>> - | _ -> raise Invalid_Cmd_Args
>> - in
>> - Connection.restrict con domid
>> -
>> (* only in xen >= 4.2 *)
>> let do_reset_watches con t domains cons data =
>> Connection.del_watches con;
>> Connection.del_transactions con
>>
>> (* only in >= xen3.3
>> *)
>> -(* we ensure backward compatibility with restrict by counting the number of
>> argument of set_target ... *)
>> -(* This is not very elegant, but it is safe as 'restrict' only restricts
>> permission of dom0 connections *)
>> let do_set_target con t domains cons data =
>> if not (Connection.is_dom0 con)
>> then raise Define.Permission_denied;
>> match split None '\000' data with
>> - | [ domid; "" ] -> do_restrict con t domains con
>> data (* backward compatibility with xen3.2-pq *)
>> | [ domid; target_domid; "" ] -> Connections.set_target cons
>> (c_int_of_string domid) (c_int_of_string target_domid)
>> | _ -> raise Invalid_Cmd_Args
>>
>> @@ -244,7 +230,6 @@ let function_of_type_simple_op ty =
>> | Xenbus.Xb.Op.Isintroduced
>> | Xenbus.Xb.Op.Resume
>> | Xenbus.Xb.Op.Set_target
>> - | Xenbus.Xb.Op.Restrict
>> | Xenbus.Xb.Op.Reset_watches
>> | Xenbus.Xb.Op.Invalid -> error "called
>> function_of_type_simple_op on operation %s" (Xenbus.Xb.Op.to_string ty);
>> raise (Invalid_argument
>> (Xenbus.Xb.Op.to_string ty))
>> @@ -428,7 +413,6 @@ let function_of_type ty =
>> | Xenbus.Xb.Op.Isintroduced -> reply_data do_isintroduced
>> | Xenbus.Xb.Op.Resume -> reply_ack do_resume
>> | Xenbus.Xb.Op.Set_target -> reply_ack do_set_target
>> - | Xenbus.Xb.Op.Restrict -> reply_ack do_restrict
>> | Xenbus.Xb.Op.Reset_watches -> reply_ack do_reset_watches
>> | Xenbus.Xb.Op.Invalid -> reply_ack do_error
>> | _ -> function_of_type_simple_op ty
>> diff --git a/tools/xenstore/include/xenstore.h
>> b/tools/xenstore/include/xenstore.h
>> index 42c0dc7..0d12c39 100644
>> --- a/tools/xenstore/include/xenstore.h
>> +++ b/tools/xenstore/include/xenstore.h
>> @@ -132,15 +132,6 @@ bool xs_mkdir(struct xs_handle *h, xs_transaction_t t,
>> bool xs_rm(struct xs_handle *h, xs_transaction_t t,
>> const char *path);
>>
>> -/* Restrict a xenstore handle so that it acts as if it had the
>> - * permissions of domain @domid. The handle must currently be
>> - * using domain 0's credentials.
>> - *
>> - * Returns false on failure, in which case the handle continues
>> - * to use the old credentials, or true on success.
>> - */
>> -bool xs_restrict(struct xs_handle *h, unsigned domid);
>> -
>> /* Get permissions of node (first element is owner, first perms is "other").
>> * Returns malloced array, or NULL: call free() after use.
>> */
>> diff --git a/tools/xenstore/xenstored_core.c
>> b/tools/xenstore/xenstored_core.c
>> index fdd4242..1e9b622 100644
>> --- a/tools/xenstore/xenstored_core.c
>> +++ b/tools/xenstore/xenstored_core.c
>> @@ -1309,7 +1309,6 @@ static struct {
>> { "IS_DOMAIN_INTRODUCED", do_is_domain_introduced },
>> [XS_RESUME] = { "RESUME", do_resume },
>> [XS_SET_TARGET] = { "SET_TARGET", do_set_target },
>> - [XS_RESTRICT] = { "RESTRICT", NULL },
>> [XS_RESET_WATCHES] = { "RESET_WATCHES", do_reset_watches },
>> [XS_DIRECTORY_PART] = { "DIRECTORY_PART", send_directory_part },
>> };
>> diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
>> index 3ce7157..6fa1261 100644
>> --- a/tools/xenstore/xs.c
>> +++ b/tools/xenstore/xs.c
>> @@ -769,14 +769,6 @@ unwind:
>> return false;
>> }
>>
>> -bool xs_restrict(struct xs_handle *h, unsigned domid)
>> -{
>> - char buf[16];
>> -
>> - sprintf(buf, "%d", domid);
>> - return xs_bool(xs_single(h, XBT_NULL, XS_RESTRICT, buf, NULL));
>> -}
>> -
>> /* Watch a node for changes (poll on fd to detect, or call read_watch()).
>> * When the node (or any child) changes, fd will become readable.
>> * Token is returned when watch is read, to allow matching.
>> diff --git a/xen/include/public/io/xs_wire.h
>> b/xen/include/public/io/xs_wire.h
>> index 54c1d71..f9f94f1 100644
>> --- a/xen/include/public/io/xs_wire.h
>> +++ b/xen/include/public/io/xs_wire.h
>> @@ -48,8 +48,8 @@ enum xsd_sockmsg_type
>> XS_IS_DOMAIN_INTRODUCED,
>> XS_RESUME,
>> XS_SET_TARGET,
>> - XS_RESTRICT,
>> - XS_RESET_WATCHES,
>> + /* XS_RESTRICT has been removed */
>> + XS_RESET_WATCHES = XS_SET_TARGET + 2,
>> XS_DIRECTORY_PART,
>>
>> XS_TYPE_COUNT, /* Number of valid types. */
>>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |