Any rdist users out there? The way rdist does group caching (stash
the struct group pointer) will stop working in the future so I'd
like to get these changes in before then.
- todd
On Thu, 13 Sep 2018 13:24:49 -0600, "Todd C. Miller" wrote:
> Use password/group cache functions and avoid stashing a pointer to
> the return value of getgrgid(3) or getgrnam(3) which relies on
> undefined behavior. When run as non-root, rdistd now uses the group
> list from getgroups(2) for group membership checks instead of looking
> up the group by name. This lets us make better use of the gid
> cache. I've also made the tilde expansion in expand.c use exptilde().
>
> This needs further testing by someone who actually uses rdist.
>
> - todd
>
> Index: usr.bin/rdist/common.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/rdist/common.c,v
> retrieving revision 1.38
> diff -u -p -u -r1.38 common.c
> --- usr.bin/rdist/common.c 9 Sep 2018 13:53:11 -0000 1.38
> +++ usr.bin/rdist/common.c 13 Sep 2018 17:35:42 -0000
> @@ -56,6 +56,8 @@
> char host[HOST_NAME_MAX+1]; /* Name of this host */
> uid_t userid = (uid_t)-1; /* User's UID */
> gid_t groupid = (gid_t)-1; /* User's GID */
> +gid_t *gidset = NULL; /* User's GID list */
> +int gidsetlen = 0; /* Number of GIDS in list */
> char *homedir = NULL; /* User's $HOME */
> char *locuser = NULL; /* Local User's name */
> int isserver = FALSE; /* We're the server */
> @@ -130,6 +132,12 @@ init(int argc, char **argv, char **envp)
> locuser = xstrdup(pw->pw_name);
> groupid = pw->pw_gid;
> gethostname(host, sizeof(host));
> + gidsetlen = (int)sysconf(_SC_NGROUPS_MAX);
> + gidset = xmalloc(gidsetlen * sizeof(gid_t));
> + if ((gidsetlen = getgroups(gidsetlen, gidset)) == -1) {
> + free(gidset);
> + gidsetlen = 0;
> + }
> #if 0
> if ((cp = strchr(host, '.')) != NULL)
> *cp = CNULL;
> @@ -436,7 +444,7 @@ getusername(uid_t uid, char *file, opt_t
> {
> static char buf[100];
> static uid_t lastuid = (uid_t)-1;
> - struct passwd *pwd = NULL;
> + const char *name;
>
> /*
> * The value of opts may have changed so we always
> @@ -448,14 +456,14 @@ getusername(uid_t uid, char *file, opt_t
> }
>
> /*
> - * Try to avoid getpwuid() call.
> + * Try to avoid passwd lookup.
> */
> if (lastuid == uid && buf[0] != '\0' && buf[0] != ':')
> return(buf);
>
> lastuid = uid;
>
> - if ((pwd = getpwuid(uid)) == NULL) {
> + if ((name = user_from_uid(uid, 1)) == NULL) {
> if (IS_ON(opts, DO_DEFOWNER) && !isserver)
> (void) strlcpy(buf, defowner, sizeof(buf));
> else {
> @@ -464,7 +472,7 @@ getusername(uid_t uid, char *file, opt_t
> (void) snprintf(buf, sizeof(buf), ":%u", uid);
> }
> } else {
> - (void) strlcpy(buf, pwd->pw_name, sizeof(buf));
> + (void) strlcpy(buf, name, sizeof(buf));
> }
>
> return(buf);
> @@ -478,7 +486,7 @@ getgroupname(gid_t gid, char *file, opt_
> {
> static char buf[100];
> static gid_t lastgid = (gid_t)-1;
> - struct group *grp = NULL;
> + const char *name;
>
> /*
> * The value of opts may have changed so we always
> @@ -490,14 +498,14 @@ getgroupname(gid_t gid, char *file, opt_
> }
>
> /*
> - * Try to avoid getgrgid() call.
> + * Try to avoid group lookup.
> */
> if (lastgid == gid && buf[0] != '\0' && buf[0] != ':')
> return(buf);
>
> lastgid = gid;
>
> - if ((grp = (struct group *)getgrgid(gid)) == NULL) {
> + if ((name = group_from_gid(gid, 1)) == NULL) {
> if (IS_ON(opts, DO_DEFGROUP) && !isserver)
> (void) strlcpy(buf, defgroup, sizeof(buf));
> else {
> @@ -506,7 +514,7 @@ getgroupname(gid_t gid, char *file, opt_
> (void) snprintf(buf, sizeof(buf), ":%u", gid);
> }
> } else
> - (void) strlcpy(buf, grp->gr_name, sizeof(buf));
> + (void) strlcpy(buf, name, sizeof(buf));
>
> return(buf);
> }
> @@ -574,6 +582,8 @@ exptilde(char *ebuf, char *file, size_t
> {
> struct passwd *pw;
> char *pw_dir, *rest;
> + static char lastuser[_PW_NAME_LEN + 1];
> + static char lastdir[PATH_MAX];
> size_t len;
>
> if (*file != '~') {
> @@ -595,13 +605,17 @@ notilde:
> else
> rest = NULL;
> if (strcmp(locuser, file) != 0) {
> - if ((pw = getpwnam(file)) == NULL) {
> - error("%s: unknown user name", file);
> - if (rest != NULL)
> - *rest = '/';
> - return(NULL);
> + if (strcmp(lastuser, file) != 0) {
> + if ((pw = getpwnam(file)) == NULL) {
> + error("%s: unknown user name", file);
> + if (rest != NULL)
> + *rest = '/';
> + return(NULL);
> + }
> + strlcpy(lastuser, pw->pw_name, sizeof(lastuser)
> );
> + strlcpy(lastdir, pw->pw_dir, sizeof(lastdir));
> }
> - pw_dir = pw->pw_dir;
> + pw_dir = lastdir;
> }
> if (rest != NULL)
> *rest = '/';
> Index: usr.bin/rdist/defs.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/rdist/defs.h,v
> retrieving revision 1.37
> diff -u -p -u -r1.37 defs.h
> --- usr.bin/rdist/defs.h 9 Sep 2018 13:53:11 -0000 1.37
> +++ usr.bin/rdist/defs.h 13 Sep 2018 17:35:42 -0000
> @@ -167,6 +167,8 @@ extern int rem_w; /* Remote file
> descr
> extern int rtimeout; /* Response time out in seconds */
> extern uid_t userid; /* User ID of rdist user */
> extern gid_t groupid; /* Group ID of rdist user */
> +extern gid_t *gidset; /* List of group IDs of rdist user */
> +extern int gidsetlen; /* Number of group IDs in list */
> extern jmp_buf finish_jmpbuf; /* Setjmp buffer for finish() *
> /
> extern char defowner[64]; /* Default owner */
> extern char defgroup[64]; /* Default group */
> Index: usr.bin/rdist/expand.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/rdist/expand.c,v
> retrieving revision 1.16
> diff -u -p -u -r1.16 expand.c
> --- usr.bin/rdist/expand.c 9 Sep 2018 13:53:11 -0000 1.16
> +++ usr.bin/rdist/expand.c 13 Sep 2018 19:21:25 -0000
> @@ -52,6 +52,7 @@ char *pathp;
> char *lastpathp;
> char *tilde; /* "~user" if not expanding tilde, else "" */
> char *tpathp;
> +char pathbuf[BUFSIZ];
>
> int expany; /* any expansions done? */
> char *entp;
> @@ -116,7 +117,6 @@ expand(struct namelist *list, int wh) /
> {
> struct namelist *nl, *prev;
> int n;
> - char pathbuf[BUFSIZ];
>
> if (debug)
> debugmsg(DM_CALL, "expand(%p, %d) start, list = %s",
> @@ -260,36 +260,16 @@ expstr(u_char *s)
> return;
> }
> if (*s == '~') {
> - struct passwd *pw;
> -
> - cp = ++s;
> - if (*cp == CNULL || *cp == '/') {
> + if ((cp = strchr(s, '/')) == NULL) {
> tilde = "~";
> - cp1 = (u_char *)homedir;
> + s++;
> } else {
> - tilde = (char *)(cp1 = ebuf);
> - *cp1++ = '~';
> - do
> - *cp1++ = *cp++;
> - while (*cp && *cp != '/');
> - *cp1 = CNULL;
> - if (strcmp(locuser, (char *)ebuf+1) != 0) {
> - if ((pw = getpwnam((char *)ebuf+1)) == NULL) {
> - strlcat((char *)ebuf,
> - ": unknown user name",
> - sizeof(ebuf));
> - yyerror((char *)ebuf+1);
> - return;
> - }
> - cp1 = (u_char *)pw->pw_dir;
> - } else {
> - cp1 = (u_char *)homedir;
> - }
> + tilde = memcpy(ebuf, s, (cp - s));
> + ebuf[cp - s] = '\0';
> s = cp;
> }
> - for (cp = (u_char *)path; (*cp++ = *cp1++) != '\0'; )
> - continue;
> - tpathp = pathp = (char *)cp - 1;
> + cp = exptilde(path, tilde, sizeof(pathbuf));
> + tpathp = pathp = (char *)cp;
> } else {
> tpathp = pathp = path;
> tilde = "";
> Index: usr.bin/rdistd/server.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/rdistd/server.c,v
> retrieving revision 1.44
> diff -u -p -u -r1.44 server.c
> --- usr.bin/rdistd/server.c 9 Sep 2018 13:53:11 -0000 1.44
> +++ usr.bin/rdistd/server.c 13 Sep 2018 17:35:42 -0000
> @@ -177,7 +177,6 @@ setfilemode(char *file, int fd, int mode
> static int
> fchog(int fd, char *file, char *owner, char *group, int mode)
> {
> - static struct group *gr = NULL;
> int i;
> struct stat st;
> uid_t uid;
> @@ -189,9 +188,7 @@ fchog(int fd, char *file, char *owner, c
> if (*owner == ':') {
> uid = (uid_t) atoi(owner + 1);
> } else if (strcmp(owner, locuser) != 0) {
> - struct passwd *pw;
> -
> - if ((pw = getpwnam(owner)) == NULL) {
> + if (uid_from_user(owner, &uid) == -1) {
> if (mode != -1 && IS_ON(mode, S_ISUID)) {
> message(MT_NOTICE,
> "%s: unknown login name \"%s\", clearing setuid",
> @@ -202,8 +199,7 @@ fchog(int fd, char *file, char *owner, c
> message(MT_NOTICE,
> "%s: unknown login name \"%s\"",
> target, owner);
> - } else
> - uid = pw->pw_uid;
> + }
> } else {
> uid = userid;
> primegid = groupid;
> @@ -213,8 +209,6 @@ fchog(int fd, char *file, char *owner, c
> goto ok;
> }
> } else { /* not root, setuid only if user==owner */
> - struct passwd *lupw;
> -
> if (mode != -1) {
> if (IS_ON(mode, S_ISUID) &&
> strcmp(locuser, owner) != 0)
> @@ -222,35 +216,29 @@ fchog(int fd, char *file, char *owner, c
> if (mode)
> mode &= ~S_ISVTX; /* and strip sticky too */
> }
> -
> - if ((lupw = getpwnam(locuser)) != NULL)
> - primegid = lupw->pw_gid;
> + primegid = groupid;
> }
>
> gid = (gid_t)-1;
> - if (gr == NULL || strcmp(group, gr->gr_name) != 0) {
> - if ((*group == ':' &&
> - (getgrgid(gid = atoi(group + 1)) == NULL))
> - || ((gr = (struct group *)getgrnam(group)) == NULL)) {
> - if (mode != -1 && IS_ON(mode, S_ISGID)) {
> - message(MT_NOTICE,
> - "%s: unknown group \"%s\", clearing setgid",
> - target, group);
> - mode &= ~S_ISGID;
> - } else
> - message(MT_NOTICE,
> - "%s: unknown group \"%s\"",
> - target, group);
> + if (*group == ':') {
> + gid = (gid_t) atoi(group + 1);
> + } else if (gid_from_group(group, &gid) == -1) {
> + if (mode != -1 && IS_ON(mode, S_ISGID)) {
> + message(MT_NOTICE,
> + "%s: unknown group \"%s\", clearing setgid",
> + target, group);
> + mode &= ~S_ISGID;
> } else
> - gid = gr->gr_gid;
> - } else
> - gid = gr->gr_gid;
> + message(MT_NOTICE,
> + "%s: unknown group \"%s\"",
> + target, group);
> + }
>
> if (userid && gid != (gid_t)-1 && gid != primegid) {
> - if (gr)
> - for (i = 0; gr->gr_mem[i] != NULL; i++)
> - if (strcmp(locuser, gr->gr_mem[i]) == 0)
> - goto ok;
> + for (i = 0; i < gidsetlen; i++) {
> + if (gid == gidset[i])
> + goto ok;
> + }
> if (mode != -1 && IS_ON(mode, S_ISGID)) {
> message(MT_NOTICE,
> "%s: user %s not in group %s, clearing setgid",
