qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com>
To: qemu-devel@nongnu.org, berrange@redhat.com, linfeng23@huawei.com,
	groug@kaod.org, huangy81@chinatelecom.cn, lvivier@redhat.com,
	lizhijian@cn.fujitsu.com, peterx@redhat.com, vgoyal@redhat.com
Cc: leobras@redhat.com, stefanha@redhat.com, quintela@redhat.com
Subject: [PULL 08/19] docs: describe the security considerations with virtiofsd xattr mapping
Date: Mon,  5 Jul 2021 11:02:24 +0100	[thread overview]
Message-ID: <20210705100235.157093-9-dgilbert@redhat.com> (raw)
In-Reply-To: <20210705100235.157093-1-dgilbert@redhat.com>

From: Daniel P. Berrangé <berrange@redhat.com>

Different guest xattr prefixes have distinct access control rules applied
by the guest. When remapping a guest xattr care must be taken that the
remapping does not allow the a guest user to bypass guest kernel access
control rules.

For example if 'trusted.*' which requires CAP_SYS_ADMIN is remapped
to 'user.virtiofs.trusted.*', an unprivileged guest user which can
write to 'user.*' can bypass the CAP_SYS_ADMIN control. Thus the
target of any remapping must be explicitly blocked from read/writes
by the guest, to prevent access control bypass.

The examples shown in the virtiofsd man page already do the right
thing and ensure safety, but the security implications of getting
this wrong were not made explicit. This could lead to host admins
and apps unwittingly creating insecure configurations.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210611120427.49736-1-berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 docs/tools/virtiofsd.rst | 55 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 50 insertions(+), 5 deletions(-)

diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
index 4911e797cb..a6c3502710 100644
--- a/docs/tools/virtiofsd.rst
+++ b/docs/tools/virtiofsd.rst
@@ -127,8 +127,8 @@ Options
   timeout.  ``always`` sets a long cache lifetime at the expense of coherency.
   The default is ``auto``.
 
-xattr-mapping
--------------
+Extended attribute (xattr) mapping
+----------------------------------
 
 By default the name of xattr's used by the client are passed through to the server
 file system.  This can be a problem where either those xattr names are used
@@ -136,6 +136,9 @@ by something on the server (e.g. selinux client/server confusion) or if the
 virtiofsd is running in a container with restricted privileges where it cannot
 access some attributes.
 
+Mapping syntax
+~~~~~~~~~~~~~~
+
 A mapping of xattr names can be made using -o xattrmap=mapping where the ``mapping``
 string consists of a series of rules.
 
@@ -232,8 +235,48 @@ Note: When the 'security.capability' xattr is remapped, the daemon has to do
 extra work to remove it during many operations, which the host kernel normally
 does itself.
 
-xattr-mapping Examples
-----------------------
+Security considerations
+~~~~~~~~~~~~~~~~~~~~~~~
+
+Operating systems typically partition the xattr namespace using
+well defined name prefixes. Each partition may have different
+access controls applied. For example, on Linux there are multiple
+partitions
+
+ * ``system.*`` - access varies depending on attribute & filesystem
+ * ``security.*`` - only processes with CAP_SYS_ADMIN
+ * ``trusted.*`` - only processes with CAP_SYS_ADMIN
+ * ``user.*`` - any process granted by file permissions / ownership
+
+While other OS such as FreeBSD have different name prefixes
+and access control rules.
+
+When remapping attributes on the host, it is important to
+ensure that the remapping does not allow a guest user to
+evade the guest access control rules.
+
+Consider if ``trusted.*`` from the guest was remapped to
+``user.virtiofs.trusted*`` in the host. An unprivileged
+user in a Linux guest has the ability to write to xattrs
+under ``user.*``. Thus the user can evade the access
+control restriction on ``trusted.*`` by instead writing
+to ``user.virtiofs.trusted.*``.
+
+As noted above, the partitions used and access controls
+applied, will vary across guest OS, so it is not wise to
+try to predict what the guest OS will use.
+
+The simplest way to avoid an insecure configuration is
+to remap all xattrs at once, to a given fixed prefix.
+This is shown in example (1) below.
+
+If selectively mapping only a subset of xattr prefixes,
+then rules must be added to explicitly block direct
+access to the target of the remapping. This is shown
+in example (2) below.
+
+Mapping examples
+~~~~~~~~~~~~~~~~
 
 1) Prefix all attributes with 'user.virtiofs.'
 
@@ -271,7 +314,9 @@ stripping of 'user.virtiofs.'.
 The second rule hides unprefixed 'trusted.' attributes
 on the host.
 The third rule stops a guest from explicitly setting
-the 'user.virtiofs.' path directly.
+the 'user.virtiofs.' path directly to prevent access
+control bypass on the target of the earlier prefix
+remapping.
 Finally, the fourth rule lets all remaining attributes
 through.
 
-- 
2.31.1



  parent reply	other threads:[~2021-07-05 10:07 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-05 10:02 [PULL v2 00/19] migration queue Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 01/19] tests: migration-test: Add dirty ring test Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 02/19] migration: fix the memory overwriting risk in add_to_iovec Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 03/19] migration: Move yank outside qemu_start_incoming_migration() Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 04/19] migration: Allow reset of postcopy_recover_triggered when failed Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 05/19] migration: move wait-unplug loop to its own function Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 06/19] migration: failover: continue to wait card unplug on error Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 07/19] virtiofsd: use GDateTime for formatting timestamp for debug messages Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` Dr. David Alan Gilbert (git) [this message]
2021-07-05 10:02 ` [PULL 09/19] virtiofsd: Don't allow file creation with FUSE_OPEN Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 10/19] virtiofsd: Fix fuse setxattr() API change issue Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 11/19] virtiofsd: Fix xattr operations overwriting errno Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 12/19] virtiofsd: Add support for extended setxattr Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 13/19] virtiofsd: Add umask to seccom allow list Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 14/19] virtiofsd: Add capability to change/restore umask Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 15/19] virtiofsd: Switch creds, drop FSETID for system.posix_acl_access xattr Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 16/19] virtiofsd: Add an option to enable/disable posix acls Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 17/19] tests/migration: parse the thread-id key of CpuInfoFast Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 18/19] tests/migration: fix "downtime_limit" type when "migrate-set-parameters" Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 19/19] migration/rdma: Use error_report to suppress errno message Dr. David Alan Gilbert (git)
2021-07-05 13:56 ` [PULL v2 00/19] migration queue Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210705100235.157093-9-dgilbert@redhat.com \
    --to=dgilbert@redhat.com \
    --cc=berrange@redhat.com \
    --cc=groug@kaod.org \
    --cc=huangy81@chinatelecom.cn \
    --cc=leobras@redhat.com \
    --cc=linfeng23@huawei.com \
    --cc=lizhijian@cn.fujitsu.com \
    --cc=lvivier@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).