Parsing moved to utils.c, optstr.c uses mnt_parse_uid(), all declarations moved to top-of-block, lchown() and chmod() have a debug message before them if they run. Also, re-wrote the test (and actually added them this time :v). On Tue, Apr 19, 2022 at 01:13:41PM +0200, Karel Zak wrote: > In this case it would be nice to be strict and use uid_t and gid_t in > the functions. > > We already had CVE for this code, so I'd like to be paranoid here :-) While in general I agree, and I wanted to do it Properly, util-linux is already very cavalier about this and essentially asserts that [ug]id_t is unsigned (or at least the same size as an unsigned; if that ever changes you're gonna be in CVE city anyway), so I originally gave up trying after not finding any library support for it. Nevertheless, demonomorphised (but the bodies are essentially the same). Updated scissor-patch below. Best, наб -- >8 -- Which take a user, group, and mode, respectively, and set them on the target after mounting This is vaguely similar to tmpfs(5)'s [ug]id= and mode= options, but we POSIX-parse the user- and group names Oft requested in systemd/zram-generator, since a common use-case is to use it to create /tmp or an equivalent directory that needs to be a=rwx,o+t (or a user's private temp that needs to be owned by them) ‒ this is impossible without terrible hacks, cf. https://github.com/systemd/zram-generator/issues/150, https://github.com/systemd/zram-generator/issues/146, &c. This started off as a Set{User,Group,Mode}= systemd mount unit, but was poetterung into libmount options: https://github.com/systemd/systemd/pull/22889 Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@xxxxxxxxxxxxxxxxxx> --- libmount/src/context.c | 12 ++- libmount/src/context_mount.c | 87 +++++++++++++++++++- libmount/src/libmount.h.in | 13 ++- libmount/src/mountP.h | 7 ++ libmount/src/optstr.c | 19 +---- libmount/src/utils.c | 126 +++++++++++++++++++++++++++++ sys-utils/mount.8.adoc | 6 ++ tests/expected/mount/set_ugid_mode | 1 + tests/ts/mount/set_ugid_mode | 65 +++++++++++++++ 9 files changed, 315 insertions(+), 21 deletions(-) create mode 100644 tests/expected/mount/set_ugid_mode create mode 100755 tests/ts/mount/set_ugid_mode diff --git a/libmount/src/context.c b/libmount/src/context.c index 99ca58b9f..7927012d2 100644 --- a/libmount/src/context.c +++ b/libmount/src/context.c @@ -57,6 +57,10 @@ struct libmnt_context *mnt_new_context(void) if (!cxt) return NULL; + cxt->tgt_owner = (uid_t) -1; + cxt->tgt_group = (gid_t) -1; + cxt->tgt_mode = (mode_t) -1; + INIT_LIST_HEAD(&cxt->addmounts); ruid = getuid(); @@ -76,7 +80,6 @@ struct libmnt_context *mnt_new_context(void) DBG(CXT, ul_debugobj(cxt, "----> allocate %s", cxt->restricted ? "[RESTRICTED]" : "")); - return cxt; } @@ -130,7 +133,6 @@ void mnt_free_context(struct libmnt_context *cxt) * mnt_context_set_options_pattern(cxt, NULL); * mnt_context_set_target_ns(cxt, NULL); * - * * to reset this stuff. * * Returns: 0 on success, negative number in case of error. @@ -155,6 +157,10 @@ int mnt_reset_context(struct libmnt_context *cxt) free(cxt->orig_user); free(cxt->subdir); + cxt->tgt_owner = (uid_t) -1; + cxt->tgt_group = (gid_t) -1; + cxt->tgt_mode = (mode_t) -1; + cxt->fs = NULL; cxt->mtab = NULL; cxt->utab = NULL; @@ -3108,7 +3114,7 @@ static void close_ns(struct libmnt_ns *ns) * * This function sets errno to ENOSYS and returns error if libmount is * compiled without namespaces support. -* + * * Returns: 0 on success, negative number in case of error. * * Since: 2.33 diff --git a/libmount/src/context_mount.c b/libmount/src/context_mount.c index f09e68860..6af83e041 100644 --- a/libmount/src/context_mount.c +++ b/libmount/src/context_mount.c @@ -837,7 +837,7 @@ static int do_mount(struct libmnt_context *cxt, const char *try_type) cxt->syscall_status = 0; DBG(CXT, ul_debugobj(cxt, "FAKE mount(2) " - "[source=%s, target=%s, type=%s, " + "[source=%s, target=%s, type=%s," " mountflags=0x%08lx, mountdata=%s]", src, target, type, flags, cxt->mountdata ? "yes" : "<none>")); @@ -862,7 +862,7 @@ static int do_mount(struct libmnt_context *cxt, const char *try_type) } DBG(CXT, ul_debugobj(cxt, "mount(2) " - "[source=%s, target=%s, type=%s, " + "[source=%s, target=%s, type=%s," " mountflags=0x%08lx, mountdata=%s]", src, target, type, flags, cxt->mountdata ? "yes" : "<none>")); @@ -1020,6 +1020,54 @@ static int do_mount_by_pattern(struct libmnt_context *cxt, const char *pattern) return rc; } +/* + * Process X-mount.owner=, X-mount.group=, X-mount.mode=. + */ +static int parse_ownership_mode(struct libmnt_context *cxt) +{ + int rc; + char *value; + size_t valsz; + + const char *o = mnt_fs_get_user_options(cxt->fs); + if (!o) + return 0; + + if ((rc = mnt_optstr_get_option(o, "X-mount.owner", &value, &valsz)) < 0) + return -MNT_ERR_MOUNTOPT; + if (!rc) { + if (!valsz) + return errno = EINVAL, -MNT_ERR_MOUNTOPT; + + if (mnt_parse_uid(value, valsz, &cxt->tgt_owner)) + return -MNT_ERR_MOUNTOPT; + } + + if ((rc = mnt_optstr_get_option(o, "X-mount.group", &value, &valsz)) < 0) + return -MNT_ERR_MOUNTOPT; + if (!rc) { + if (!valsz) + return errno = EINVAL, -MNT_ERR_MOUNTOPT; + + if (mnt_parse_gid(value, valsz, &cxt->tgt_group)) + return -MNT_ERR_MOUNTOPT; + } + + if ((rc = mnt_optstr_get_option(o, "X-mount.mode", &value, &valsz)) < 0) + return -MNT_ERR_MOUNTOPT; + if (!rc) { + if (!valsz) + return errno = EINVAL, -MNT_ERR_MOUNTOPT; + + if ((rc = mnt_parse_mode(value, valsz, &cxt->tgt_mode))) + return -MNT_ERR_MOUNTOPT; + } + + DBG(CXT, ul_debugobj(cxt, "ownership %d:%d, mode %04o", cxt->tgt_owner, cxt->tgt_group, cxt->tgt_mode)); + + return 0; +} + /** * mnt_context_prepare_mount: * @cxt: context @@ -1064,6 +1112,8 @@ int mnt_context_prepare_mount(struct libmnt_context *cxt) rc = mnt_context_guess_fstype(cxt); if (!rc) rc = mnt_context_prepare_target(cxt); + if (!rc) + rc = parse_ownership_mode(cxt); if (!rc) rc = mnt_context_prepare_helper(cxt, "mount", NULL); @@ -1089,6 +1139,25 @@ end: return rc; } +static int set_ownership_mode(struct libmnt_context *cxt) +{ + const char *target = mnt_fs_get_target(cxt->fs); + + if (cxt->tgt_owner != (uid_t) -1 || cxt->tgt_group != (uid_t) -1) { + DBG(CXT, ul_debugobj(cxt, "mount: lchown(%s, %u, %u)", target, cxt->tgt_owner, cxt->tgt_group)); + if (lchown(target, cxt->tgt_owner, cxt->tgt_group) == -1) + return -MNT_ERR_CHOWN; + } + + if (cxt->tgt_mode != (mode_t) -1) { + DBG(CXT, ul_debugobj(cxt, "mount: chmod(%s, %04o)", target, cxt->tgt_mode)); + if (chmod(target, cxt->tgt_mode) == -1) + return -MNT_ERR_CHMOD; + } + + return 0; +} + /** * mnt_context_do_mount * @cxt: context @@ -1191,6 +1260,9 @@ int mnt_context_do_mount(struct libmnt_context *cxt) if (mnt_context_is_veritydev(cxt)) mnt_context_deferred_delete_veritydev(cxt); + if (!res) + res = set_ownership_mode(cxt); + if (!mnt_context_switch_ns(cxt, ns_old)) return -MNT_ERR_NAMESPACE; @@ -1841,7 +1913,18 @@ int mnt_context_get_mount_excode( if (buf) snprintf(buf, bufsz, _("filesystem was mounted, but failed to switch namespace back")); return MNT_EX_SYSERR; + } + + if (rc == -MNT_ERR_CHOWN) { + if (buf) + snprintf(buf, bufsz, _("filesystem was mounted, but failed to change ownership to %u:%u: %m"), cxt->tgt_owner, cxt->tgt_group); + return MNT_EX_SYSERR; + } + if (rc == -MNT_ERR_CHMOD) { + if (buf) + snprintf(buf, bufsz, _("filesystem was mounted, but failed to change mode to %04o: %m"), cxt->tgt_mode); + return MNT_EX_SYSERR; } if (rc < 0) diff --git a/libmount/src/libmount.h.in b/libmount/src/libmount.h.in index 43d8e44ce..0ab1d6ece 100644 --- a/libmount/src/libmount.h.in +++ b/libmount/src/libmount.h.in @@ -220,6 +220,18 @@ enum { * filesystem mounted, but --onlyonce specified */ #define MNT_ERR_ONLYONCE 5010 +/** + * MNT_ERR_CHOWN: + * + * filesystem mounted, but subsequent X-mount.owner=/X-mount.group= lchown(2) failed + */ +#define MNT_ERR_CHOWN 5011 +/** + * MNT_ERR_CHMOD: + * + * filesystem mounted, but subsequent X-mount.mode= chmod(2) failed + */ +#define MNT_ERR_CHMOD 5012 /* @@ -246,7 +258,6 @@ enum { * * [u]mount(8) exit code: out of memory, cannot fork, ... */ - #define MNT_EX_SYSERR 2 /** diff --git a/libmount/src/mountP.h b/libmount/src/mountP.h index 561ddcd8c..26158e8d9 100644 --- a/libmount/src/mountP.h +++ b/libmount/src/mountP.h @@ -109,6 +109,9 @@ extern int mnt_chdir_to_parent(const char *target, char **filename); extern char *mnt_get_username(const uid_t uid); extern int mnt_get_uid(const char *username, uid_t *uid); extern int mnt_get_gid(const char *groupname, gid_t *gid); +extern int mnt_parse_uid(const char *user, size_t user_len, uid_t *gid); +extern int mnt_parse_gid(const char *group, size_t group_len, gid_t *gid); +extern int mnt_parse_mode(const char *mode, size_t mode_len, mode_t *gid); extern int mnt_in_group(gid_t gid); extern int mnt_open_uniq_filename(const char *filename, char **name); @@ -294,6 +297,10 @@ struct libmnt_context char *subdir; /* X-mount.subdir= */ + uid_t tgt_owner; /* X-mount.owner= */ + gid_t tgt_group; /* X-mount.group= */ + mode_t tgt_mode; /* X-mount.mode= */ + struct libmnt_fs *fs; /* filesystem description (type, mountpoint, device, ...) */ struct libmnt_fs *fs_template; /* used for @fs on mnt_reset_context() */ diff --git a/libmount/src/optstr.c b/libmount/src/optstr.c index 43b983ebb..0ad5bfdc6 100644 --- a/libmount/src/optstr.c +++ b/libmount/src/optstr.c @@ -1019,15 +1019,13 @@ int mnt_optstr_fix_user(char **optstr) /* * Converts value from @optstr addressed by @name to uid. * - * Returns: 0 on success, 1 if not found, <0 on error + * Returns: 0 on success, <0 on error */ int mnt_optstr_get_uid(const char *optstr, const char *name, uid_t *uid) { char *value = NULL; size_t valsz = 0; - char buf[sizeof(stringify_value(UINT64_MAX))]; int rc; - uint64_t num; assert(optstr); assert(name); @@ -1037,20 +1035,11 @@ int mnt_optstr_get_uid(const char *optstr, const char *name, uid_t *uid) if (rc != 0) goto fail; - if (valsz > sizeof(buf) - 1) { - rc = -ERANGE; + rc = mnt_parse_uid(value, valsz, uid); + if (rc != 0) { + rc = -errno; goto fail; } - mem2strcpy(buf, value, valsz, sizeof(buf)); - - rc = ul_strtou64(buf, &num, 10); - if (rc != 0) - goto fail; - if (num > ULONG_MAX || (uid_t) num != num) { - rc = -ERANGE; - goto fail; - } - *uid = (uid_t) num; return 0; fail: diff --git a/libmount/src/utils.c b/libmount/src/utils.c index ff3acfb55..a138abe7e 100644 --- a/libmount/src/utils.c +++ b/libmount/src/utils.c @@ -635,6 +635,132 @@ int mnt_get_gid(const char *groupname, gid_t *gid) return rc; } +static int parse_uid_numeric(const char *value, size_t valsz, uid_t *uid) +{ + uint64_t num; + + assert(value); + assert(uid); + + if (valsz > sizeof(stringify_value(UINT64_MAX)) - 1) { + errno = ERANGE; + goto fail; + } + + if (ul_strtou64(value, &num, 10) != 0) { + errno = ENOENT; + goto fail; + } + if (num > ULONG_MAX || (uid_t) num != num) { + errno = ERANGE; + goto fail; + } + *uid = (uid_t) num; + + return 0; +fail: + DBG(UTILS, ul_debug("failed to convert '%s' to number [errno=%d]", value, errno)); + return -1; +} + +/* POSIX-parse user_len-sized user; -1 and errno set, or 0 on success */ +int mnt_parse_uid(const char *user, size_t user_len, uid_t *uid) +{ + char *user_tofree = NULL; + int rc; + + if (user[user_len]) { + user = user_tofree = strndup(user, user_len); + if (!user) + return -1; + } + + rc = mnt_get_uid(user, uid); + if (rc != 0 && isdigit(*user)) + rc = parse_uid_numeric(user, user_len, uid); + + free(user_tofree); + return rc; +} + +static int parse_gid_numeric(const char *value, size_t valsz, gid_t *gid) +{ + uint64_t num; + + assert(value); + assert(gid); + + if (valsz > sizeof(stringify_value(UINT64_MAX)) - 1) { + errno = ERANGE; + goto fail; + } + + if (ul_strtou64(value, &num, 10) != 0) { + errno = ENOENT; + goto fail; + } + if (num > ULONG_MAX || (gid_t) num != num) { + errno = ERANGE; + goto fail; + } + *gid = (gid_t) num; + + return 0; +fail: + DBG(UTILS, ul_debug("failed to convert '%s' to number [errno=%d]", value, errno)); + return -1; +} + +/* POSIX-parse group_len-sized group; -1 and errno set, or 0 on success */ +int mnt_parse_gid(const char *group, size_t group_len, gid_t *gid) +{ + char *group_tofree = NULL; + int rc; + + if (group[group_len]) { + group = group_tofree = strndup(group, group_len); + if (!group) + return -1; + } + + rc = mnt_get_gid(group, gid); + if (rc != 0 && isdigit(*group)) + rc = parse_gid_numeric(group, group_len, gid); + + free(group_tofree); + return rc; +} + +int mnt_parse_mode(const char *mode, size_t mode_len, mode_t *uid) +{ + char buf[sizeof(stringify_value(UINT32_MAX))]; + uint32_t num; + + assert(mode); + assert(uid); + + if (mode_len > sizeof(buf) - 1) { + errno = ERANGE; + goto fail; + } + mem2strcpy(buf, mode, mode_len, sizeof(buf)); + + if (ul_strtou32(buf, &num, 8) != 0) { + errno = EINVAL; + goto fail; + } + if (num > 07777) { + errno = ERANGE; + goto fail; + } + *uid = (mode_t) num; + + return 0; +fail: + DBG(UTILS, ul_debug("failed to convert '%.*s' to mode [errno=%d]", (int) mode_len, mode, errno)); + return -1; +} + int mnt_in_group(gid_t gid) { int rc = 0, n, i; diff --git a/sys-utils/mount.8.adoc b/sys-utils/mount.8.adoc index 7465fcd0d..5f0ddd05b 100644 --- a/sys-utils/mount.8.adoc +++ b/sys-utils/mount.8.adoc @@ -636,6 +636,12 @@ Allow to make a target directory (mountpoint) if it does not exist yet. The opti **X-mount.subdir=**__directory__:: Allow mounting sub-directory from a filesystem instead of the root directory. For now, this feature is implemented by temporary filesystem root directory mount in unshared namespace and then bind the sub-directory to the final mount point and umount the root of the filesystem. The sub-directory mount shows up atomically for the rest of the system although it is implemented by multiple *mount*(2) syscalls. This feature is EXPERIMENTAL. +*X-mount.owner*=_username_|_UID_, *X-mount.group*=_group_|_GID_:: +Set _mountpoint_'s ownership after mounting. Names resolved in the target mount namespace, see *-N*. + +*X-mount.mode*=_mode_:: +Set _mountpoint_'s mode after mounting. + *nosymfollow*:: Do not follow symlinks when resolving paths. Symlinks can still be created, and *readlink*(1), *readlink*(2), *realpath*(1), and *realpath*(3) all still work properly. diff --git a/tests/expected/mount/set_ugid_mode b/tests/expected/mount/set_ugid_mode new file mode 100644 index 000000000..35821117c --- /dev/null +++ b/tests/expected/mount/set_ugid_mode @@ -0,0 +1 @@ +Success diff --git a/tests/ts/mount/set_ugid_mode b/tests/ts/mount/set_ugid_mode new file mode 100755 index 000000000..810f81f38 --- /dev/null +++ b/tests/ts/mount/set_ugid_mode @@ -0,0 +1,65 @@ +#!/bin/bash +# SPDX-License-Identifier: 0BSD + + +TS_TOPDIR="${0%/*}/../.." +TS_DESC="X-mount.{owner,group,mode}=" + +. $TS_TOPDIR/functions.sh +ts_init "$*" + +ts_check_test_command "$TS_CMD_MOUNT" +ts_check_test_command "$TS_CMD_UMOUNT" + +ts_skip_nonroot +ts_check_losetup +ts_check_prog "mkfs.ext2" +ts_check_prog "id" +ts_check_prog "ls" + + +do_one() { + expected="$1"; shift + what="$1"; shift + where="$1"; shift + $TS_CMD_MOUNT "$@" "$what" "$where" >> $TS_OUTPUT 2>> $TS_ERRLOG + read -r m _ o g _ < <(ls -nd "$where") + actual="$m $o $g" + [ "$actual" = "$expected" ] || echo "$*: $actual != $expected" >> $TS_ERRLOG + $TS_CMD_UMOUNT "$where" >> $TS_OUTPUT 2>> $TS_ERRLOG +} + +ts_device_init + +mkfs.ext2 "$TS_LODEV" > /dev/null 2>&1 || ts_die "Cannot make ext2 on $TS_LODEV" +ts_device_has "TYPE" "ext2" "$TS_LODEV" || ts_die "Cannot find ext2 on $TS_LODEV" + +user_1="$(id -un 1)" +group_2="$(id -gn 2)" + + +mkdir -p "$TS_MOUNTPOINT" + +do_one "drwxr-xr-x 0 0" "$TS_LODEV" "$TS_MOUNTPOINT" +do_one "drwxr-xr-x 1 0" "$TS_LODEV" "$TS_MOUNTPOINT" -o "X-mount.owner=$user_1" +do_one "drwxr-xr-x 1 2" "$TS_LODEV" "$TS_MOUNTPOINT" -o "X-mount.group=$group_2" +do_one "d-w--wxr-T 132 2" "$TS_LODEV" "$TS_MOUNTPOINT" -o "X-mount.owner=132,X-mount.mode=1234" +do_one "d-ws-w---x 132 123" "$TS_LODEV" "$TS_MOUNTPOINT" -o "X-mount.mode=4321,X-mount.group=123" +do_one "d-ws-w---x 1 321" "$TS_LODEV" "$TS_MOUNTPOINT" -o "X-mount.owner=$user_1,X-mount.group=321" + + +> "$TS_MOUNTPOINT/bind" +> "$TS_MOUNTPOINT/bindsrc" + +do_one "-rw-r--r-- 0 0" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind +do_one "-rw-r--r-- 1 0" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind -o "X-mount.owner=$user_1" +do_one "-rw-r--r-- 1 2" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind -o "X-mount.group=$group_2" +do_one "--w--wxr-T 132 2" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind -o "X-mount.owner=132,X-mount.mode=1234" +do_one "--ws-w---x 132 123" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind -o "X-mount.mode=4321,X-mount.group=123" +do_one "--wx-w---x 1 321" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT/bind" --bind -o "X-mount.owner=$user_1,X-mount.group=321" + + +rm -fd "$TS_MOUNTPOINT/bind" "$TS_MOUNTPOINT/bindsrc" "$TS_MOUNTPOINT" + +ts_log "Success" +ts_finalize -- 2.30.2
Attachment:
signature.asc
Description: PGP signature
