Differential D51510
base: do a sweep of setgroups() that mean to clear the supplementaries Authored by kevans on Fri, Jul 25, 6:00 AM. Tags None Referenced Files
Details In the future, this will be beneficial as we move the egid out of the The rwhod diff is a little gratuitious, but I like patterns and
Diff Detail
Event TimelineComment Actions
It's best to be systematic, that eases reviewing, even if there are no functional changes. Comment Actions There are other occurences that could be cleaned up as well as part or as a follow-up to this change, in:
misc/fdatasync.sh:181: if (setgroups(1, &pw->pw_gid) || misc/fdatasync2.sh:180: if (setgroups(1, &pw->pw_gid) || misc/fifo2.sh:170: if (setgroups(1, &pw->pw_gid) || misc/ftruncate.sh:173: if (setgroups(1, &pw->pw_gid) || misc/ftruncate2.sh:188: if (setgroups(1, &pw->pw_gid) || misc/kevent7.sh:239: if (setgroups(1, &pw->pw_gid) || misc/killpg.sh:116: if (setgroups(1, &pw->pw_gid) || misc/killpg2.sh:80: if (setgroups(1, &pw->pw_gid) || misc/killpg3.sh:112: if (setgroups(1, &pw->pw_gid) || misc/maxproc.sh:106: if (setgroups(1, &pw->pw_gid) || misc/mlockall3.sh:133: if (setgroups(1, &pw->pw_gid) || misc/mlockall7.sh:182: if (setgroups(1, &pw->pw_gid) || misc/mountu.sh:244: if (setgroups(1, &pw->pw_gid) || misc/msync.sh:169: if (setgroups(1, &pw->pw_gid) || misc/pread.sh:173: if (setgroups(1, &pw->pw_gid) || misc/sched.sh:110: if (setgroups(1, &pw->pw_gid) || misc/sigreturn3.sh:124: if (setgroups(1, &pw->pw_gid) || misc/sigreturn4.sh:150: if (setgroups(1, &pw->pw_gid) || misc/syscall4.sh:321: if (setgroups(1, &pw->pw_gid) || misc/tmpfs16.sh:184: if (setgroups(1, &pw->pw_gid) ||
sbin/hastd/subr.c:211: if (setgroups(1, gidset) == -1) {
For these, I'm not too sure what we can do, but I suspect we can change them, as either we are now the upstream (Kyua?) or we're not but we have probably applied a specific patch (else there is specific code for us upstream, or perhaps upstream just duplicates the effective GID as the first supplementary group, which would be a potential security hazard; I have not checked). contrib/kyua/utils/process/isolation.cpp:161: if (::setgroups(1, groups) == -1)
contrib/kyua/utils/process/isolation.cpp:162: fail(F("setgroups(1, [%s]) failed; UID is %s and GID is %s")
contrib/ntp/ntpd/ntpd.c:757: if (0 != setgroups(1, &sw_gid)) {
contrib/ntp/ntpd/ntpd.c:758: msyslog(LOG_ERR, "setgroups(1, %d) failed: %m", sw_gid);
contrib/pf/authpf/authpf.c:296: if (setgroups(1, &gid) == -1) {
contrib/pf/ftp-proxy/ftp-proxy.c:278: setgroups(1, &pw->pw_gid) != 0 ||
contrib/pf/pflogd/privsep.c:105: if (setgroups(1, gidset) == -1)
contrib/pf/tftp-proxy/tftp-proxy.c:123: if (setgroups(1, &pw->pw_gid) ||
contrib/sendmail/src/deliver.c:3195: if (setgroups(1, gidset) == -1
contrib/sendmail/src/deliver.c:3222: if (setgroups(1, gidset) == -1
contrib/sendmail/src/deliver.c:6642: if (setgroups(1, gidset) == -1 && suidwarn)
contrib/sendmail/src/main.c:3866: ** if (geteuid() == 0 && setgroups(1, emptygidset) == -1)
contrib/sendmail/src/main.c:3869: if (setgroups(1, emptygidset) == -1 && geteuid() == 0)
contrib/sendmail/src/main.c:3871: syserr("drop_privileges: setgroups(1, %d) failed",
contrib/sendmail/src/recipient.c:1544: if (setgroups(1, gidset) == -1)
crypto/openssh/sshd-session.c:318: if (setgroups(1, gidset) == -1)Comment Actions I had looked at hastd, but I punted that diff to be rolled up into the next one because it has some follow-up assertions about getgroups() that will need to be adjusted. I did miss tools/, but those ones seem straightforward. I was declining to touch crypto/ and contrib/ unless there was something egregiously bad in there. I did wonder the status of contrib/pf as far as upstream goes (CC @kp), but for atf/kyua something like this makes sense to just go through the GitHub repo first. |