[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen stable-4.10] tools/ocaml/xenstored: Fix path length validation
commit 17b26b823b00a2993ea3f7517bff2e236c939103 Author: Edwin Török <edvin.torok@xxxxxxxxxx> AuthorDate: Tue Dec 15 14:45:01 2020 +0100 Commit: Jan Beulich <jbeulich@xxxxxxxx> CommitDate: Tue Dec 15 14:45:01 2020 +0100 tools/ocaml/xenstored: Fix path length validation Currently, oxenstored checks the length of paths against 1024, then prepends "/local/domain/$DOMID/" to relative paths. This allows a domU to create paths which can't subsequently be read by anyone, even dom0. This also interferes with listing directories, etc. Define a new oxenstored.conf entry: quota-path-max, defaulting to 1024 as before. For paths that begin with "/local/domain/$DOMID/" check the relative path length against this quota. For all other paths check the entire path length. This ensures that if the domid changes (and thus the length of a prefix changes) a path that used to be valid stays valid (e.g. after a live-migration). It also ensures that regardless how the client tries to access a path (domid-relative or absolute) it will get consistent results, since the limit is always applied on the final canonicalized path. Delete the unused Domain.get_path to avoid it being confused with Connection.get_path (which differs by a trailing slash only). Rewrite Util.path_validate to apply the appropriate length restriction based on whether the path is relative or not. Remove the check for connection_path being absolute, because it is not guest controlled data. This is part of XSA-323. Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Signed-off-by: Edwin Török <edvin.torok@xxxxxxxxxx> Acked-by: Christian Lindig <christian.lindig@xxxxxxxxxx> --- tools/ocaml/libs/xb/partial.ml | 1 + tools/ocaml/xenstored/define.ml | 2 ++ tools/ocaml/xenstored/domain.ml | 1 - tools/ocaml/xenstored/oxenstored.conf.in | 1 + tools/ocaml/xenstored/utils.ml | 15 ++++++++++++++- tools/ocaml/xenstored/xenstored.ml | 1 + 6 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tools/ocaml/libs/xb/partial.ml b/tools/ocaml/libs/xb/partial.ml index d4d1c7bdec..b6e2a716e2 100644 --- a/tools/ocaml/libs/xb/partial.ml +++ b/tools/ocaml/libs/xb/partial.ml @@ -28,6 +28,7 @@ external header_of_string_internal: string -> int * int * int * int = "stub_header_of_string" let xenstore_payload_max = 4096 (* xen/include/public/io/xs_wire.h *) +let xenstore_rel_path_max = 2048 (* xen/include/public/io/xs_wire.h *) let of_string s = let tid, rid, opint, dlen = header_of_string_internal s in diff --git a/tools/ocaml/xenstored/define.ml b/tools/ocaml/xenstored/define.ml index 2965c08534..f574397a4c 100644 --- a/tools/ocaml/xenstored/define.ml +++ b/tools/ocaml/xenstored/define.ml @@ -32,6 +32,8 @@ let conflict_rate_limit_is_aggregate = ref true let domid_self = 0x7FF0 +let path_max = ref Xenbus.Partial.xenstore_rel_path_max + exception Not_a_directory of string exception Not_a_value of string exception Already_exist diff --git a/tools/ocaml/xenstored/domain.ml b/tools/ocaml/xenstored/domain.ml index b0a01b06fa..081076271a 100644 --- a/tools/ocaml/xenstored/domain.ml +++ b/tools/ocaml/xenstored/domain.ml @@ -38,7 +38,6 @@ type t = } let is_dom0 d = d.id = 0 -let get_path dom = "/local/domain/" ^ (sprintf "%u" dom.id) let get_id domain = domain.id let get_interface d = d.interface let get_mfn d = d.mfn diff --git a/tools/ocaml/xenstored/oxenstored.conf.in b/tools/ocaml/xenstored/oxenstored.conf.in index d5d4f00de8..bef633090b 100644 --- a/tools/ocaml/xenstored/oxenstored.conf.in +++ b/tools/ocaml/xenstored/oxenstored.conf.in @@ -61,6 +61,7 @@ quota-maxsize = 2048 quota-maxwatch = 100 quota-transaction = 10 quota-maxrequests = 1024 +quota-path-max = 1024 # Activate filed base backend persistent = false diff --git a/tools/ocaml/xenstored/utils.ml b/tools/ocaml/xenstored/utils.ml index f3d95e8897..02639c77e9 100644 --- a/tools/ocaml/xenstored/utils.ml +++ b/tools/ocaml/xenstored/utils.ml @@ -94,7 +94,7 @@ let read_file_single_integer filename = let path_validate path connection_path = let len = String.length path in - if len = 0 || len > 1024 then raise Define.Invalid_path; + if len = 0 then raise Define.Invalid_path; let abs_path = match String.get path 0 with @@ -102,4 +102,17 @@ let path_validate path connection_path = | _ -> connection_path ^ path in + (* Regardless whether client specified absolute or relative path, + canonicalize it (above) and, for domain-relative paths, check the + length of the relative part. + + This prevents paths becoming invalid across migrate when the length + of the domid changes in @param connection_path. + *) + let len = String.length abs_path in + let on_absolute _ _ = len in + let on_relative _ offset = len - offset in + let len = Scanf.ksscanf abs_path on_absolute "/local/domain/%d/%n" on_relative in + if len > !Define.path_max then raise Define.Invalid_path; + abs_path diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml index 183dd2754b..70f1bf8d2e 100644 --- a/tools/ocaml/xenstored/xenstored.ml +++ b/tools/ocaml/xenstored/xenstored.ml @@ -102,6 +102,7 @@ let parse_config filename = ("quota-maxentity", Config.Set_int Quota.maxent); ("quota-maxsize", Config.Set_int Quota.maxsize); ("quota-maxrequests", Config.Set_int Define.maxrequests); + ("quota-path-max", Config.Set_int Define.path_max); ("test-eagain", Config.Set_bool Transaction.test_eagain); ("persistent", Config.Set_bool Disk.enable); ("xenstored-log-file", Config.String Logging.set_xenstored_log_destination); -- generated by git-patchbot for /home/xen/git/xen.git#stable-4.10
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |