|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3] xenstore: remove XS_RESTRICT support
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.
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 |