On Sat, 2016-04-23 at 17:14 -0400, Steve Dickson wrote:
>
> On 04/22/2016 04:41 PM, Olga Kornievskaia wrote:
> >
> > On Fri, Apr 22, 2016 at 2:38 PM, Steve Dickson <SteveD@xxxxxxxxxx>
> > wrote:
> > >
> > >
> > >
> > > On 04/20/2016 05:12 PM, Olga Kornievskaia wrote:
> > > >
> > > > For the threaded version we have to set uid,gid per thread
> > > > instead
> > > > of per process. glibc setresuid() when called from a thread,
> > > > it'll
> > > > send a signal to all other threads to synchronize the uid in
> > > > all
> > > > other threads. To bypass this, we have to call syscall()
> > > > directly.
> > > >
> > > > Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
> > > > Reviewed-by: Steve Dickson <steved@xxxxxxxxxx>
> > > > ---
> > > > utils/gssd/gssd_proc.c | 10 ++++++++--
> > > > 1 file changed, 8 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
> > > > index 581a125..5d9a6db 100644
> > > > --- a/utils/gssd/gssd_proc.c
> > > > +++ b/utils/gssd/gssd_proc.c
> > > > @@ -69,6 +69,7 @@
> > > > #include <netdb.h>
> > > > #include <sys/types.h>
> > > > #include <sys/wait.h>
> > > > +#include <syscall.h>
> > > >
> > > > #include "gssd.h"
> > > > #include "err_util.h"
> > > > @@ -457,7 +458,12 @@ change_identity(uid_t uid)
> > > > * Switch the GIDs. Note that we leave the saved-set-gid
> > > > alone in an
> > > > * attempt to prevent attacks via ptrace()
> > > > */
> > > > - if (setresgid(pw->pw_gid, pw->pw_gid, -1) != 0) {
> > > > + /* For the threaded version we have to set uid,gid per
> > > > thread instead
> > > > + * of per process. glibc setresuid() when called from a
> > > > thread, it'll
> > > > + * send a signal to all other threads to synchronize the
> > > > uid in all
> > > > + * other threads. To bypass this, we have to call
> > > > syscall() directly.
> > > > + */
> > > > + if (syscall(SYS_setresgid, pw->pw_gid) != 0) {
> > > > printerr(0, "WARNING: failed to set gid to
> > > > %u!\n", pw->pw_gid);
> > > > return errno;
> > > > }
> > > > @@ -466,7 +472,7 @@ change_identity(uid_t uid)
> > > > * Switch UIDs, but leave saved-set-uid alone to prevent
> > > > ptrace() by
> > > > * other processes running with this uid.
> > > > */
> > > > - if (setresuid(uid, uid, -1) != 0) {
> > > > + if (syscall(SYS_setresuid, uid) != 0) {
> > > > printerr(0, "WARNING: Failed to setuid for user
> > > > with uid %u\n",
> > > > uid);
> > > > return errno;
> > > >
> > > We also have to do the same thing to the setgroups() call at the
> > > top of change_identity(). So add the following diff to this
> > > patch and we are good to go...
> > >
> > > diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
> > > index e651d71..2f9f8ab 100644
> > > --- a/utils/gssd/gssd_proc.c
> > > +++ b/utils/gssd/gssd_proc.c
> > > @@ -437,7 +437,7 @@ change_identity(uid_t uid)
> > > struct passwd *pw;
> > >
> > > /* drop list of supplimentary groups first */
> > > - if (setgroups(0, NULL) != 0) {
> > > + if (syscall(SYS_setgroups, 0) != 0) {
> > > printerr(0, "WARNING: unable to drop
> > > supplimentary groups!");
> > > return errno;
> > > }
> > >
> > Do you know why we do this before getting the passwd entry?
> It came in with commit 6b53fc9c which changed the way gssd
> changed the uid/gid from Jeff Layton...
>
> Jeff, what was the thought behind dropping supplementary groups?
> I believe it was some type of security thing.
>
We do that so that the child doesn't run with supplementary groups from
the main process.
As to why we do that before getting the passwd entry instead of after?
No real reason for the order that I can recall. If you need to do it
after the pw entry, I don't think it'll matter.
That said, since this has to run as root anyway, I guess you could drop
the supplementary groups in the main process. Then all of the children
should get a zeroed out grouplist, and it's one less syscall...
> >
> >
> > Doing as you suggest causes problems as in it it fails to drop the
> > supplementary groups.
> hmm... what are you seeing that I'm not?
>
> steved.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jeff Layton <jlayton@xxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
