blob: 4f9a91f0c6e0da7333c1c9a0b8c592ad120e2f5c [file] [log] [blame]
Andrew Geisslerc926e172021-05-07 16:11:35 -05001From aaa5f8e00c2e85a893b972f1e243fb14c26b70dc Mon Sep 17 00:00:00 2001
2From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
3Date: Wed, 24 Feb 2021 19:56:25 +0000
4Subject: [PATCH 2/2] virtiofs: drop remapped security.capability xattr as
5 needed
6
7On Linux, the 'security.capability' xattr holds a set of
8capabilities that can change when an executable is run, giving
9a limited form of privilege escalation to those programs that
10the writer of the file deemed worthy.
11
12Any write causes the 'security.capability' xattr to be dropped,
13stopping anyone from gaining privilege by modifying a blessed
14file.
15
16Fuse relies on the daemon to do this dropping, and in turn the
17daemon relies on the host kernel to drop the xattr for it. However,
18with the addition of -o xattrmap, the xattr that the guest
19stores its capabilities in is now not the same as the one that
20the host kernel automatically clears.
21
22Where the mapping changes 'security.capability', explicitly clear
23the remapped name to preserve the same behaviour.
24
25This bug is assigned CVE-2021-20263.
26
27Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
28Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
29
30Upstream-Status: Backport [e586edcb410543768ef009eaa22a2d9dd4a53846]
31CVE: CVE-2021-20263
32
33Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
34---
35 docs/tools/virtiofsd.rst | 4 ++
36 tools/virtiofsd/passthrough_ll.c | 77 +++++++++++++++++++++++++++++++-
37 2 files changed, 80 insertions(+), 1 deletion(-)
38
39diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
40index 866b7db3e..00554c75b 100644
41--- a/docs/tools/virtiofsd.rst
42+++ b/docs/tools/virtiofsd.rst
43@@ -228,6 +228,10 @@ The 'map' type adds a number of separate rules to add **prepend** as a prefix
44 to the matched **key** (or all attributes if **key** is empty).
45 There may be at most one 'map' rule and it must be the last rule in the set.
46
47+Note: When the 'security.capability' xattr is remapped, the daemon has to do
48+extra work to remove it during many operations, which the host kernel normally
49+does itself.
50+
51 xattr-mapping Examples
52 ----------------------
53
54diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
55index 03c5e0d13..c9197da86 100644
56--- a/tools/virtiofsd/passthrough_ll.c
57+++ b/tools/virtiofsd/passthrough_ll.c
58@@ -160,6 +160,7 @@ struct lo_data {
59 int posix_lock;
60 int xattr;
61 char *xattrmap;
62+ char *xattr_security_capability;
63 char *source;
64 char *modcaps;
65 double timeout;
66@@ -226,6 +227,8 @@ static __thread bool cap_loaded = 0;
67
68 static struct lo_inode *lo_find(struct lo_data *lo, struct stat *st,
69 uint64_t mnt_id);
70+static int xattr_map_client(const struct lo_data *lo, const char *client_name,
71+ char **out_name);
72
73 static int is_dot_or_dotdot(const char *name)
74 {
75@@ -365,6 +368,37 @@ out:
76 return ret;
77 }
78
79+/*
80+ * The host kernel normally drops security.capability xattr's on
81+ * any write, however if we're remapping xattr names we need to drop
82+ * whatever the clients security.capability is actually stored as.
83+ */
84+static int drop_security_capability(const struct lo_data *lo, int fd)
85+{
86+ if (!lo->xattr_security_capability) {
87+ /* We didn't remap the name, let the host kernel do it */
88+ return 0;
89+ }
90+ if (!fremovexattr(fd, lo->xattr_security_capability)) {
91+ /* All good */
92+ return 0;
93+ }
94+
95+ switch (errno) {
96+ case ENODATA:
97+ /* Attribute didn't exist, that's fine */
98+ return 0;
99+
100+ case ENOTSUP:
101+ /* FS didn't support attribute anyway, also fine */
102+ return 0;
103+
104+ default:
105+ /* Hmm other error */
106+ return errno;
107+ }
108+}
109+
110 static void lo_map_init(struct lo_map *map)
111 {
112 map->elems = NULL;
113@@ -717,6 +751,11 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr,
114 uid_t uid = (valid & FUSE_SET_ATTR_UID) ? attr->st_uid : (uid_t)-1;
115 gid_t gid = (valid & FUSE_SET_ATTR_GID) ? attr->st_gid : (gid_t)-1;
116
117+ saverr = drop_security_capability(lo, ifd);
118+ if (saverr) {
119+ goto out_err;
120+ }
121+
122 res = fchownat(ifd, "", uid, gid, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
123 if (res == -1) {
124 goto out_err;
125@@ -735,6 +774,14 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr,
126 }
127 }
128
129+ saverr = drop_security_capability(lo, truncfd);
130+ if (saverr) {
131+ if (!fi) {
132+ close(truncfd);
133+ }
134+ goto out_err;
135+ }
136+
137 res = ftruncate(truncfd, attr->st_size);
138 if (!fi) {
139 saverr = errno;
140@@ -1726,6 +1773,13 @@ static int lo_do_open(struct lo_data *lo, struct lo_inode *inode,
141 if (fd < 0) {
142 return -fd;
143 }
144+ if (fi->flags & (O_TRUNC)) {
145+ int err = drop_security_capability(lo, fd);
146+ if (err) {
147+ close(fd);
148+ return err;
149+ }
150+ }
151 }
152
153 pthread_mutex_lock(&lo->mutex);
154@@ -2114,6 +2168,12 @@ static void lo_write_buf(fuse_req_t req, fuse_ino_t ino,
155 "lo_write_buf(ino=%" PRIu64 ", size=%zd, off=%lu)\n", ino,
156 out_buf.buf[0].size, (unsigned long)off);
157
158+ res = drop_security_capability(lo_data(req), out_buf.buf[0].fd);
159+ if (res) {
160+ fuse_reply_err(req, res);
161+ return;
162+ }
163+
164 /*
165 * If kill_priv is set, drop CAP_FSETID which should lead to kernel
166 * clearing setuid/setgid on file.
167@@ -2353,6 +2413,7 @@ static void parse_xattrmap(struct lo_data *lo)
168 {
169 const char *map = lo->xattrmap;
170 const char *tmp;
171+ int ret;
172
173 lo->xattr_map_nentries = 0;
174 while (*map) {
175@@ -2383,7 +2444,7 @@ static void parse_xattrmap(struct lo_data *lo)
176 * the last entry.
177 */
178 parse_xattrmap_map(lo, map, sep);
179- return;
180+ break;
181 } else {
182 fuse_log(FUSE_LOG_ERR,
183 "%s: Unexpected type;"
184@@ -2452,6 +2513,19 @@ static void parse_xattrmap(struct lo_data *lo)
185 fuse_log(FUSE_LOG_ERR, "Empty xattr map\n");
186 exit(1);
187 }
188+
189+ ret = xattr_map_client(lo, "security.capability",
190+ &lo->xattr_security_capability);
191+ if (ret) {
192+ fuse_log(FUSE_LOG_ERR, "Failed to map security.capability: %s\n",
193+ strerror(ret));
194+ exit(1);
195+ }
196+ if (!strcmp(lo->xattr_security_capability, "security.capability")) {
197+ /* 1-1 mapping, don't need to do anything */
198+ free(lo->xattr_security_capability);
199+ lo->xattr_security_capability = NULL;
200+ }
201 }
202
203 /*
204@@ -3480,6 +3554,7 @@ static void fuse_lo_data_cleanup(struct lo_data *lo)
205
206 free(lo->xattrmap);
207 free_xattrmap(lo);
208+ free(lo->xattr_security_capability);
209 free(lo->source);
210 }
211
212--
2132.29.2
214