> On April 23, 2015, 9:16 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp, line 730 > > <https://reviews.apache.org/r/33491/diff/1/?file=940552#file940552line730> > > > > I'm curious why you look at EPERM here, but not in the call to > > `::setgid` above and `::setuid` below? Is it possible for only some of > > these calls to fail with EPERM? > > James Peach wrote: > If you do a ``setgid`` to your current GID, it succeeds and does nothing, > but ``initgroups`` ends up calling ``setgroups`` which fails with EPERM. If > you were trying to switch to a different user and you were not already > privileged, the ``setgid`` would already have failed. > > Ben Mahler wrote: > I see, in that case, could we express it in the structure of the code > more explicitly? Is there an easy way to capture whether we need to perform > the operation? For example: > > ``` > inline Try<Nothing> su(const std::string& user) > { > Result<gid_t> gid = os::getgid(user); > Result<uid_t> uid = os::getuid(user); > > if (gid.isError() || gid.isNone()) { > return Error("Failed to getgid: " + > (gid.isError() ? gid.error() : "unknown user")); > } else if (uid.isError() || uid.isNone()) { > return Error("Failed to getuid: " + > (uid.isError() ? uid.error() : "unknown user")); > } > > if (gid.get() != ::getgid()) { > if (::setgid(gid.get()) == -1) { > return ErrnoError("Failed to set gid"); > } > > if (::initgroups(user.c_str(), gid.get()) == -1) { > return ErrnoError("Failed to set supplementary groups"); > } > } > > if (uid.get() != ::getuid()) { > if (::setuid(uid.get())) { > return ErrnoError("Failed to setuid"); > } > } > > return Nothing(); > } > ``` > > How does this interact with real, effective, and saved set-group/set-user > IDs?
``setgid`` sets all the GIDs (real, effective and saved) according to [POSIX](http://pubs.opengroup.org/onlinepubs/9699919799/functions/setgid.html). In the code you propose above, it's possible that because the real GID matches, we would fail to set the remaining GIDs. I don't know whether that *really* matters, since in practice I guess only the test suite would hit that case. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33491/#review81404 ----------------------------------------------------------- On April 23, 2015, 8:16 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33491/ > ----------------------------------------------------------- > > (Updated April 23, 2015, 8:16 p.m.) > > > Review request for mesos, Ben Mahler and Timothy St. Clair. > > > Bugs: MESOS-719 > https://issues.apache.org/jira/browse/MESOS-719 > > > Repository: mesos > > > Description > ------- > > Make sure we call initgroups(3) after calling setgid(2) so that the > auxiliary groups for the su'd user are set. > > This is particularly important for docker support because it lets > you add your mesos user to the docker group so it can talk to docker > through /var/run/docker.sock (which is owned by a docker group by > default in most installations.) Without initgroups, the Mesos user > only has its primary GID set. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp > 02db3c587e3f9a40282405e9496bde30e251f8bb > > Diff: https://reviews.apache.org/r/33491/diff/ > > > Testing > ------- > > Ran "make check" on CentOS 7 & and OS X 10.10.3. > > > Thanks, > > James Peach > >
