[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    [oss-security] Xen Security Advisory 353 v4 (CVE-2020-29479) - oxenstored: permissions not checked o
From:       Xen.org security team <security () xen ! org>
Date:       2020-12-15 12:20:25
Message-ID: E1kp9JZ-00078k-EE () xenbits ! xenproject ! org
[Download RAW message or body]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

            Xen Security Advisory CVE-2020-29479 / XSA-353
                               version 4

           oxenstored: permissions not checked on root node

UPDATES IN VERSION 4
====================

Public release.

ISSUE DESCRIPTION
=================

In the Ocaml xenstored implementation, the internal representation of
the tree has special cases for the root node, because this node has no
parent.

Unfortunately, permissions were not checked for certain operations on
the root node.

Unprivileged guests can get and modify permissions, list, and delete
the root node.  Deleting the whole xenstore tree is a hostwide denial
of service.  Depending on the circumstances, the vulnerability can
also be leveraged into an ability to gain write access to any part of
xenstore.

IMPACT
======

A guest administrator can deny service to the whole system
simply by deleting the whole of xenstore.

Additionally, depending on other software in use, privilege escalation
may be possible.  With the default "xl" toolstack, a guest
administrator can escalate their privilege to that of the host.

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.

The impact depends on the toolstack and other management software in
use.  Systems using libxl (for example, via "xl" or libvirt) are
vulnerable to privilege escalation.

Systems using C xenstored are not vulnerable, no matter what toolstack
or management software is in use.

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

Note that the Ocaml patches for XSA-115 depend on this patch.

xsa353.patch           xen-unstable - 4.10

$ sha256sum xsa353*
48fa1f414773ab1a4135fe62aaae25c7c543efe5a4c5dba71db9e497fa9f3362  xsa353.meta
e14922bf6b2095c1b17849b130e999726a1a31e29be1374e0cd3f9a8fa59fd3d  xsa353.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/Yqd8MHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZmg8IALQltyH/EPk78gGNyeb/1ri3jr7IVR5lyCy1Aedg
zckh8FNaaRCplZAoa2Kc2aV2H1Lc5x/UfWtoOLaiSdcyRNXRKRFwq7LoBT7OH2SH
KSo2HK0licTOv61SL2LoJ38tXec86V0Cos89DuWtSMLQT3LUmixQlSdiTUueFidH
Fei8mqoYor5WtzjfgKjdR5KwrrPj65QFyUic3bRgdcc/t27Wr+oQU5iGg7ayeCNw
5Ylz8eyJj88rkNVw1S4jFH815lyENaJbVn56VvlEm0KDsnY7G4YAHExZ1lElrOdj
nkOXN3o6CGiHTkXPOsbPuy0WboSrXK9AZykasml/EDw41Vg=
=V1xW
-----END PGP SIGNATURE-----

["xsa353.meta" (application/octet-stream)]
["xsa353.patch" (application/octet-stream)]

From: =?UTF-8?q?Edwin Török?= <edvin.torok@citrix.com>
Subject: tools/ocaml/xenstored: do permission checks on xenstore root
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This was lacking in a disappointing number of places.

The xenstore root node is treated differently from all other nodes, because it
doesn't have a parent, and mutation requires changing the parent.

Unfortunately this lead to open-coding the special case for root into every
single xenstore operation, and out of all the xenstore operations only read
did a permission check when handling the root node.

This means that an unprivileged guest can:

 * xenstore-chmod / to its liking and subsequently write new arbitrary nodes
   there (subject to quota)
 * xenstore-rm -r / deletes almost the entire xenstore tree (xenopsd quickly
   refills some, but you are left with a broken system)
 * DIRECTORY on / lists all children when called through python
   bindings (xenstore-ls stops at /local because it tries to list recursively)
 * get-perms on / works too, but that is just a minor information leak

Add the missing permission checks, but this should really be refactored to do
the root handling and permission checks on the node only once from a single
function, instead of getting it wrong nearly everywhere.

This is XSA-353.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml
index f299ec6461..92b6289b5e 100644
--- a/tools/ocaml/xenstored/store.ml
+++ b/tools/ocaml/xenstored/store.ml
@@ -273,15 +273,17 @@ let path_rm store perm path  			Node.del_childname node name
 		with Not_found ->
 			raise Define.Doesnt_exist in
-	if path = [] then
+	if path = [] then (
+		Node.check_perm store.root perm Perms.WRITE;
 		Node.del_all_children store.root
-	else
+	) else
 		Path.apply_modify store.root path do_rm

 let path_setperms store perm path perms -	if path = [] then
+	if path = [] then (
+		Node.check_perm store.root perm Perms.WRITE;
 		Node.set_perms store.root perms
-	else
+	) else
 		let do_setperms node name  			let c = Node.find node name in
 			Node.check_owner c perm;
@@ -313,9 +315,10 @@ let read store perm path 
 let ls store perm path  	let children -		if path = [] then
-			(Node.get_children store.root)
-		else
+		if path = [] then (
+			Node.check_perm store.root perm Perms.READ;
+			Node.get_children store.root
+		) else
 			let do_ls node name  				let cnode = Node.find node name in
 				Node.check_perm cnode perm Perms.READ;
@@ -324,9 +327,10 @@ let ls store perm path  	List.rev (List.map (fun n -> Symbol.to_string n.Node.name) children)

 let getperms store perm path -	if path = [] then
-		(Node.get_perms store.root)
-	else
+	if path = [] then (
+		Node.check_perm store.root perm Perms.READ;
+		Node.get_perms store.root
+	) else
 		let fct n name  			let c = Node.find n name in
 			Node.check_perm c perm Perms.READ;


[prev in list] [next in list] [prev in thread] [next in thread]