* Xen Security Advisory 323 v3 (CVE-2020-29482) - Xenstore: wrong path length check
@ 2020-12-15 12:20 Xen.org security team
0 siblings, 0 replies; only message in thread
From: Xen.org security team @ 2020-12-15 12:20 UTC (permalink / raw)
To: xen-announce, xen-devel, xen-users, oss-security; +Cc: Xen.org security team
[-- Attachment #1: Type: text/plain, Size: 3954 bytes --]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Xen Security Advisory CVE-2020-29482 / XSA-323
version 3
Xenstore: wrong path length check
UPDATES IN VERSION 3
====================
Public release.
ISSUE DESCRIPTION
=================
A guest may access xenstore paths via absolute paths containing a full
pathname, or via a relative path, which implicitly includes
/local/domain/$DOMID for their own domain id. Management tools must
access paths in guests' namespaces, necessarily using absolute paths.
oxenstored imposes a path name limit which is applied solely to the
relative or absolute path specified by the client. Therefore, a guest
can create paths in its own namespace which are too long for management
tools to access.
IMPACT
======
Depending on the toolstack in use, a malicious guest administrator
might cause some management tools and debugging operations to fail.
For example, a guest administrator can cause `xenstore-ls -r` to fail.
However a guest administrator cannot prevent the host administrator
from tearing down the domain.
VULNERABLE SYSTEMS
==================
All systems using oxenstored are vulnerable. Building and using
oxenstored is the default in the upstream Xen distribution, if the
Ocaml compiler is available.
Systems using C xenstored are not vulnerable.
MITIGATION
==========
There are no mitigations.
Changing to use of C xenstored would avoid this vulnerability. However,
given the other vulnerabilities in both versions of xenstored being
reported at this time, changing xenstored implementation is not a
recommended approach to mitigation of individual issues.
CREDITS
=======
This issue was discovered by Edwin Török and analysed by Andrew Cooper, both
of Citrix.
RESOLUTION
==========
Applying the appropriate attached patch resolves this issue.
Note that patches for released versions are generally prepared to
apply to the stable branches, and may not apply cleanly to the most
recent release tarball. Downstreams are encouraged to update to the
tip of the stable branch before applying these patches.
xsa323.patch xen-unstable - Xen 4.11.x
xsa323-4.10.patch Xen 4.10.x
$ sha256sum xsa323*
b693f259d92033ffc568412f1ea591b63d7e8dcaa7f88b62158b3c09e65ad122 xsa323.meta
fdfefa3c064c6c5f49d666d7c3444f919777d557c8cb9c2e9ae6ac94711d37de xsa323.patch
90ab525fad3f43b6de2858f8a58128ce0d4ca97f5960bcd2af5be55d49059c92 xsa323-4.10.patch
$
DEPLOYMENT DURING EMBARGO
=========================
Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.
But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).
Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.
(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable. This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)
For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAl/Yqd4MHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZNeEH/iXf3oBcCJAICRa2WuXXR8zs/R/GUaiQCJYvU2So
OBil7cnb6bIomVmDd7TxjYUaHL3ilMEFHPTbUq0gLLKPlaJqVJTLyxDzss6VEmqp
eyQ8GBRODHLHCGcQaS3eKtCN9e6Oyd+rEm5CIPXvcu+g+HVnxG1BCdzHOei7NEPS
P5rmOn3Gkv6LlYQpVPYzVX3baIpLe09Ha+1iCS2bflArPgAc23ajxHHffuI90TIF
cBjN93AOpgBc89wGM0G11NSPAjnVUWe69qLJp6HDW6mQRaUxHQVbpBesd0V43/9P
XCvo6TxWdfDgJFOubZRKg6jHP5CKqobL4PZZT+X1MC8ZNbI=
=ilKH
-----END PGP SIGNATURE-----
[-- Attachment #2: xsa323.meta --]
[-- Type: application/octet-stream, Size: 1913 bytes --]
{
"XSA": 323,
"SupportedVersions": [
"master",
"4.14",
"4.13",
"4.12",
"4.11",
"4.10"
],
"Trees": [
"xen"
],
"Recipes": {
"4.10": {
"Recipes": {
"xen": {
"StableRef": "1d72d9915edff0dd41f601bbb0b1f83c02ff1689",
"Prereqs": [
353,
115,
322
],
"Patches": [
"xsa323-4.10.patch"
]
}
}
},
"4.11": {
"Recipes": {
"xen": {
"StableRef": "41a822c3926350f26917d747c8dfed1c44a2cf42",
"Prereqs": [
353,
115,
322
],
"Patches": [
"xsa323.patch"
]
}
}
},
"4.12": {
"Recipes": {
"xen": {
"StableRef": "8145d38b48009255a32ab87a02e481cd09c811f9",
"Prereqs": [
353,
115,
322
],
"Patches": [
"xsa323.patch"
]
}
}
},
"4.13": {
"Recipes": {
"xen": {
"StableRef": "b5302273e2c51940172400486644636f2f4fc64a",
"Prereqs": [
353,
115,
322
],
"Patches": [
"xsa323.patch"
]
}
}
},
"4.14": {
"Recipes": {
"xen": {
"StableRef": "1d1d1f5391976456a79daac0dcfe7157da1e54f7",
"Prereqs": [
353,
115,
322
],
"Patches": [
"xsa323.patch"
]
}
}
},
"master": {
"Recipes": {
"xen": {
"StableRef": "3ae469af8e680df31eecd0a2ac6a83b58ad7ce53",
"Prereqs": [
353,
115,
322
],
"Patches": [
"xsa323.patch"
]
}
}
}
}
}
[-- Attachment #3: xsa323.patch --]
[-- Type: application/octet-stream, Size: 5747 bytes --]
From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= <edvin.torok@citrix.com>
Subject: tools/ocaml/xenstored: Fix path length validation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
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/libs/xb/partial.mli b/tools/ocaml/libs/xb/partial.mli
index 359a75e88d..b9216018f5 100644
--- a/tools/ocaml/libs/xb/partial.mli
+++ b/tools/ocaml/libs/xb/partial.mli
@@ -9,6 +9,7 @@ external header_size : unit -> int = "stub_header_size"
external header_of_string_internal : string -> int * int * int * int
= "stub_header_of_string"
val xenstore_payload_max : int
+val xenstore_rel_path_max : int
val of_string : string -> pkt
val append : pkt -> string -> int -> unit
val to_complete : pkt -> int
diff --git a/tools/ocaml/xenstored/define.ml b/tools/ocaml/xenstored/define.ml
index ea9e1b7620..ebe18b8e31 100644
--- a/tools/ocaml/xenstored/define.ml
+++ b/tools/ocaml/xenstored/define.ml
@@ -31,6 +31,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 aeb185ff7e..81cb59b8f1 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 f843482981..4ae48e42d4 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 e8c9fe4e94..eb79bf0146 100644
--- a/tools/ocaml/xenstored/utils.ml
+++ b/tools/ocaml/xenstored/utils.ml
@@ -93,7 +93,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
@@ -101,4 +101,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 ff9fbbbac2..39d6d767e4 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);
[-- Attachment #4: xsa323-4.10.patch --]
[-- Type: application/octet-stream, Size: 5216 bytes --]
From: =?UTF-8?q?Edwin=20T=C3=B6r=C3=B6k?= <edvin.torok@citrix.com>
Subject: tools/ocaml/xenstored: Fix path length validation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
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);
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2020-12-15 12:20 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 12:20 Xen Security Advisory 323 v3 (CVE-2020-29482) - Xenstore: wrong path length check Xen.org security team
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).