virtiofs minor security fix

Fix xattrmap to drop remapped security.capability capabilities.
 
 Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEERfXHG0oMt/uXep+pBRYzHrxb/ecFAmBAuD0ACgkQBRYzHrxb
 /edxcQ//avZY516ghAcUvnEaihk+87FUB/29Z5gjemqVjWuQbIO62jjsveTgB/QP
 GPBeTiFxZQIDGzF0XENYsoNJwMjlGwfHHniI46nHtZ1UN3wKjIbp2/ogytUvJrI4
 p0ntXjZ6F3IxFdSY6A3IKaEm6iSmPhJdHy409ZDiphJY8LUVezpEa/Oa6ZuUQTUE
 MsPo6KMSOikwFThE/yb4YGen0sLE4nK4uXYWRAMa4UeM9L5bGF4bwXiaeWZqAcNm
 y+VWgWuxonLH3RENHRBvtvoLXW5gtakAX4t1MfsNFEB5seoyiscBu5Ya3tX1wIbr
 xXxlJo0WFQtNy4veG30P/YfxcAQQbFpuTCJ6sz1dwgdcV8Ciq3vSOGqCI3q1gL2G
 GPPQtXLTXznGUWId9OkqBCcj4OQgbTCBDnD6Cp1f9QszDwe6xB2U82hMpniSqVbc
 /xAC72nObKRzdZKnX4NloFmKwM+x3oQe+7BiqROJhJmhN9bDRsl2dvZ0L38YR5gO
 ff6WROqUCdZ9yaMvNf3KdZkP+tagJxj/PVMBZYmG3R2wYnf1NbOl+mkX9Phb8i3t
 gcMCKTuORVeRphPAA48zMqhJnvHvqPKMVNnxsfzlOwFZIViu2PVT+MS4MPNIGM61
 Kiu9M97LXLWHlzBdTnO0Z8agFgYT+u8cDYNOyGrck/cbrAJKzug=
 =jN7p
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/dgilbert-gitlab/tags/pull-virtiofs-20210304' into staging

virtiofs minor security fix

Fix xattrmap to drop remapped security.capability capabilities.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

# gpg: Signature made Thu 04 Mar 2021 10:36:45 GMT
# gpg:                using RSA key 45F5C71B4A0CB7FB977A9FA90516331EBC5BFDE7
# gpg: Good signature from "Dr. David Alan Gilbert (RH2) <dgilbert@redhat.com>" [full]
# Primary key fingerprint: 45F5 C71B 4A0C B7FB 977A  9FA9 0516 331E BC5B FDE7

* remotes/dgilbert-gitlab/tags/pull-virtiofs-20210304:
  virtiofs: drop remapped security.capability xattr as needed

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Peter Maydell 2021-03-04 10:42:46 +00:00
commit cb90ecf934
2 changed files with 80 additions and 1 deletions

View File

@ -228,6 +228,10 @@ The 'map' type adds a number of separate rules to add **prepend** as a prefix
to the matched **key** (or all attributes if **key** is empty).
There may be at most one 'map' rule and it must be the last rule in the set.
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
----------------------

View File

@ -148,6 +148,7 @@ struct lo_data {
int posix_lock;
int xattr;
char *xattrmap;
char *xattr_security_capability;
char *source;
char *modcaps;
double timeout;
@ -217,6 +218,8 @@ static __thread bool cap_loaded = 0;
static struct lo_inode *lo_find(struct lo_data *lo, struct stat *st,
uint64_t mnt_id);
static int xattr_map_client(const struct lo_data *lo, const char *client_name,
char **out_name);
static int is_dot_or_dotdot(const char *name)
{
@ -356,6 +359,37 @@ out:
return ret;
}
/*
* The host kernel normally drops security.capability xattr's on
* any write, however if we're remapping xattr names we need to drop
* whatever the clients security.capability is actually stored as.
*/
static int drop_security_capability(const struct lo_data *lo, int fd)
{
if (!lo->xattr_security_capability) {
/* We didn't remap the name, let the host kernel do it */
return 0;
}
if (!fremovexattr(fd, lo->xattr_security_capability)) {
/* All good */
return 0;
}
switch (errno) {
case ENODATA:
/* Attribute didn't exist, that's fine */
return 0;
case ENOTSUP:
/* FS didn't support attribute anyway, also fine */
return 0;
default:
/* Hmm other error */
return errno;
}
}
static void lo_map_init(struct lo_map *map)
{
map->elems = NULL;
@ -737,6 +771,11 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr,
uid_t uid = (valid & FUSE_SET_ATTR_UID) ? attr->st_uid : (uid_t)-1;
gid_t gid = (valid & FUSE_SET_ATTR_GID) ? attr->st_gid : (gid_t)-1;
saverr = drop_security_capability(lo, ifd);
if (saverr) {
goto out_err;
}
res = fchownat(ifd, "", uid, gid, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
if (res == -1) {
saverr = errno;
@ -759,6 +798,14 @@ static void lo_setattr(fuse_req_t req, fuse_ino_t ino, struct stat *attr,
}
}
saverr = drop_security_capability(lo, truncfd);
if (saverr) {
if (!fi) {
close(truncfd);
}
goto out_err;
}
if (kill_suidgid) {
res = drop_effective_cap("FSETID", &cap_fsetid_dropped);
if (res != 0) {
@ -1784,6 +1831,13 @@ static int lo_do_open(struct lo_data *lo, struct lo_inode *inode,
if (fd < 0) {
return -fd;
}
if (fi->flags & (O_TRUNC)) {
int err = drop_security_capability(lo, fd);
if (err) {
close(fd);
return err;
}
}
}
pthread_mutex_lock(&lo->mutex);
@ -2191,6 +2245,12 @@ static void lo_write_buf(fuse_req_t req, fuse_ino_t ino,
"lo_write_buf(ino=%" PRIu64 ", size=%zd, off=%lu kill_priv=%d)\n",
ino, out_buf.buf[0].size, (unsigned long)off, fi->kill_priv);
res = drop_security_capability(lo_data(req), out_buf.buf[0].fd);
if (res) {
fuse_reply_err(req, res);
return;
}
/*
* If kill_priv is set, drop CAP_FSETID which should lead to kernel
* clearing setuid/setgid on file. Note, for WRITE, we need to do
@ -2432,6 +2492,7 @@ static void parse_xattrmap(struct lo_data *lo)
{
const char *map = lo->xattrmap;
const char *tmp;
int ret;
lo->xattr_map_nentries = 0;
while (*map) {
@ -2462,7 +2523,7 @@ static void parse_xattrmap(struct lo_data *lo)
* the last entry.
*/
parse_xattrmap_map(lo, map, sep);
return;
break;
} else {
fuse_log(FUSE_LOG_ERR,
"%s: Unexpected type;"
@ -2531,6 +2592,19 @@ static void parse_xattrmap(struct lo_data *lo)
fuse_log(FUSE_LOG_ERR, "Empty xattr map\n");
exit(1);
}
ret = xattr_map_client(lo, "security.capability",
&lo->xattr_security_capability);
if (ret) {
fuse_log(FUSE_LOG_ERR, "Failed to map security.capability: %s\n",
strerror(ret));
exit(1);
}
if (!strcmp(lo->xattr_security_capability, "security.capability")) {
/* 1-1 mapping, don't need to do anything */
free(lo->xattr_security_capability);
lo->xattr_security_capability = NULL;
}
}
/*
@ -3588,6 +3662,7 @@ static void fuse_lo_data_cleanup(struct lo_data *lo)
free(lo->xattrmap);
free_xattrmap(lo);
free(lo->xattr_security_capability);
free(lo->source);
}