From: Ian Kent <raven@themaw.net>
To: Jeff Mahoney <jeffm@suse.com>
Cc: autofs mailing list <autofs@vger.kernel.org>
Subject: Re: [PATCH v2] autofs-5.1.3 - fix ordering of seteuid/setegid in do_spawn
Date: Thu, 19 Oct 2017 10:29:54 +0800 [thread overview]
Message-ID: <f5d94551-ef84-7a7a-539f-6caa78176104@themaw.net> (raw)
In-Reply-To: <2712B578-9ADC-4C52-9918-D6C8621223A7@suse.com>
On 19/10/17 10:20, Jeff Mahoney wrote:
> I wondered that too but decided against it since it’s happening with root privs anyway.
Right, it's probably not a problem, lets go with it how it is and see.
Whether to continue if seteuid() fails is a good question.
The assumption is it always succeeds but I guess it might not.
It probably shouldn't continue because if automounts in a dependent path
can't be mounted (via the open()) then the requested mount can't work
properly.
>
> -Jeff
>
> --
> Jeff Mahoney
> (apologies for the top post -- from my mobile)
>
>> On Oct 18, 2017, at 10:07 PM, Ian Kent <raven@themaw.net> wrote:
>>
>>> On 19/10/17 05:12, Jeff Mahoney wrote:
>>> In do_spawn, We call seteuid() prior to calling setegid() which means
>>> that, when we're using an unprivileged uid, we won't have permissions
>>> to set the effective group anymore.
>>>
>>> We also don't touch the group memberships so the permissions used to
>>> open the directory will will include all of root's supplementary groups
>>> and none of the user's.
>>>
>>> This patch reverses the ordering and uses initgroups() to reset the
>>> supplementary groups to the unprivileged user's groups.
>>>
>>> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
>>> ---
>>> daemon/spawn.c | 15 +++++++++++++--
>>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/daemon/spawn.c b/daemon/spawn.c
>>> index c640d97..62e9f02 100644
>>> --- a/daemon/spawn.c
>>> +++ b/daemon/spawn.c
>>> @@ -20,6 +20,7 @@
>>> #include <string.h>
>>> #include <sys/types.h>
>>> #include <dirent.h>
>>> +#include <grp.h>
>>> #include <time.h>
>>> #include <poll.h>
>>> #include <sys/wait.h>
>>> @@ -195,8 +196,18 @@ static int do_spawn(unsigned logopt, unsigned int wait,
>>> * program group to trigger mount
>>> */
>>> if (euid) {
>>> - seteuid(euid);
>>> - setegid(egid);
>>> + if (initgroups(tsv->user, egid) == -1)
>>
>> LOL, you spotted that one.
>>
>> I was wondering if the groups need to be restored after the open() ....
>> before the mount is executed.
>>
>>> + fprintf(stderr,
>>> + "warning: initgroups: %s\n",
>>> + strerror(errno));
>>> + if (setegid(egid) == -1)
>>> + fprintf(stderr,
>>> + "warning: setegid: %s\n",
>>> + strerror(errno));
>>> + if (seteuid(euid) == -1)
>>> + fprintf(stderr,
>>> + "warning: seteuid: %s\n",
>>> + strerror(errno));
>>> }
>>> setpgrp();
>>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe autofs" in
>>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe autofs" in
>
--
To unsubscribe from this list: send the line "unsubscribe autofs" in
next prev parent reply other threads:[~2017-10-19 2:29 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-16 5:05 [PATCH 00/35] Current autofs patch queue (lets try again) Ian Kent
2017-10-16 5:05 ` [PATCH 01/35] autofs-5.1.3 - fix spec file url Ian Kent
2017-10-16 5:06 ` [PATCH 02/35] autofs-5.1.3 - fix unset tsd group name handling Ian Kent
2017-10-16 5:06 ` [PATCH 03/35] autofs-5.1.3 - Add -c option when calling /bin/umount - if supported Ian Kent
2017-10-16 5:06 ` [PATCH 04/35] autofs-5.1.3 - remove some redundant rpc library code Ian Kent
2017-10-16 5:06 ` [PATCH 05/35] autofs-5.1.3 - add port parameter to rpc_ping() Ian Kent
2017-10-16 5:06 ` [PATCH 06/35] autofs-5.1.3 - dont probe NFSv2 by default Ian Kent
2017-10-16 5:06 ` [PATCH 07/35] autofs-5.1.3 - add version parameter to rpc_ping() Ian Kent
2017-10-16 5:06 ` [PATCH 08/35] autofs-5.1.3 - fix typo in autofs config file comments Ian Kent
2017-10-16 5:06 ` [PATCH 09/35] autofs-5.1.3 - fix typos in autofs man pages Ian Kent
2017-10-16 5:06 ` [PATCH 10/35] autofs-5.1.3 - use pkg-config to search for libtirpc to fix cross-compilation Ian Kent
2017-10-16 5:07 ` [PATCH 11/35] autofs-5.1.3 - fix incorrect status return in get_nfs_info() Ian Kent
2017-10-16 5:07 ` [PATCH 12/35] autofs-5.1.3 - fix a couple of compiler warnings Ian Kent
2017-10-18 20:46 ` Jeff Mahoney
2017-10-18 20:57 ` [PATCH] autofs-5.1.3 - fix ordering of seteuid/setegid in do_spawn Jeff Mahoney
2017-10-18 21:07 ` Jeff Mahoney
2017-10-19 1:59 ` Ian Kent
2017-10-18 21:12 ` [PATCH v2] " Jeff Mahoney
2017-10-19 2:07 ` Ian Kent
2017-10-19 2:20 ` Jeff Mahoney
2017-10-19 2:29 ` Ian Kent [this message]
2017-10-16 5:07 ` [PATCH 13/35] autofs-5.1.3 - set systemd KillMode to process Ian Kent
2017-10-16 5:07 ` [PATCH 14/35] autofs-5.1.3 - fix mount.nfs blocks on first mount Ian Kent
2017-10-16 5:07 ` [PATCH 15/35] autofs-5.1.3 - fix some man page problems Ian Kent
2017-10-16 5:07 ` [PATCH 16/35] autofs-5.1.3 - add some more debug logging to get_nfs_info() Ian Kent
2017-10-16 5:46 ` Vincent McIntyre
2017-10-16 5:52 ` Ian Kent
2017-10-16 5:07 ` [PATCH 17/35] autofs-5.1.3 - add some more debug logging to get_supported_ver_and_cost() Ian Kent
2017-10-16 5:07 ` [PATCH 18/35] autofs-5.1.3 - fix ipv6 proto option handling Ian Kent
2017-10-16 5:07 ` [PATCH 19/35] autofs-5.1.3 - also check flag file exe name Ian Kent
2017-10-16 5:08 ` [PATCH 20/35] autofs-5.1.3 - fix possible map instance memory leak Ian Kent
2017-10-16 5:08 ` [PATCH 21/35] autofs-5.1.3 - check map instances for staleness on map update Ian Kent
2017-10-16 5:08 ` [PATCH 22/35] autofs-5.1.3 - allow dot in OPTIONSTR value lexer pattern Ian Kent
2017-10-16 5:08 ` [PATCH 23/35] autofs-5.1.3 - fix autofs_use_lofs description Ian Kent
2017-10-16 5:08 ` [PATCH 24/35] autofs-5.1.3 - fix amd parser error buffer size Ian Kent
2017-10-16 5:08 ` [PATCH 25/35] autofs-5.1.3 - make spawn_bind_mount() use mount_wait as well Ian Kent
2017-10-16 5:08 ` [PATCH 26/35] autofs-5.1.3 - document ghost option in auto.master man page Ian Kent
2017-10-16 6:00 ` Vincent McIntyre
2017-10-16 6:07 ` Ian Kent
2017-10-16 5:08 ` [PATCH 27/35] autofs-5.1.3 - only take master map mutex for master map update Ian Kent
2017-10-16 5:08 ` [PATCH 28/35] autofs-5.1.3 - revert fix argc off by one in mount_autofs.c Ian Kent
2017-10-16 5:08 ` [PATCH 29/35] autofs-5.1.3 - fix nisplus lookup init not configured check Ian Kent
2017-10-16 5:09 ` [PATCH 30/35] autofs-5.1.3 - make open_lookup() error handling more consistent Ian Kent
2017-10-16 5:09 ` [PATCH 31/35] autofs-5.1.3 - be silent about sss library not found Ian Kent
2017-10-16 5:09 ` [PATCH 32/35] autofs-5.1.3 - be silent about nis domain not set Ian Kent
2017-10-16 5:09 ` [PATCH 33/35] autofs-5.1.3 - make map source reference message debug only Ian Kent
2017-10-16 5:09 ` [PATCH 34/35] autofs-5.1.3 - improve description of mount_nfs_default_protocol Ian Kent
2017-10-16 5:37 ` Vincent McIntyre
2017-10-16 5:51 ` Ian Kent
2017-10-16 5:09 ` [PATCH 35/35] autofs-5.1.3 - port option should not behave like nobind option Ian Kent
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=f5d94551-ef84-7a7a-539f-6caa78176104@themaw.net \
--to=raven@themaw.net \
--cc=autofs@vger.kernel.org \
--cc=jeffm@suse.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 read-only IMAP folder(s) and NNTP newsgroup(s).