Re: use getpwuid_r in doas
Ted Unangst wrote:
> I have a coming change which will need to access both the calling user and
> target users' passwd entries. In order to accomplish this, we need to switch
> to the reentrant flavor of getpwuid. No behaviorial change, but I think this
> is clearer and less error prone as well, versus reusing a pointer to static
> storage.
Improve a few more things noticed by martijn. We don't need to copy strings
that have long lifetimes. Also, fix some error checks to be more precise.
Index: doas.c
===
RCS file: /cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.75
diff -u -p -r1.75 doas.c
--- doas.c 10 Jun 2019 18:11:27 - 1.75
+++ doas.c 10 Jun 2019 18:15:34 -
@@ -288,7 +288,6 @@ main(int argc, char **argv)
char *sh;
const char *cmd;
char cmdline[LINE_MAX];
- char myname[_PW_NAME_LEN + 1];
char mypwbuf[_PW_BUF_LEN], targpwbuf[_PW_BUF_LEN];
struct passwd mypwstore, targpwstore;
struct passwd *mypw, *targpw;
@@ -349,10 +348,10 @@ main(int argc, char **argv)
usage();
rv = getpwuid_r(uid, , mypwbuf, sizeof(mypwbuf), );
- if (rv != 0 || mypw == NULL)
+ if (rv != 0)
err(1, "getpwuid_r failed");
- if (strlcpy(myname, mypw->pw_name, sizeof(myname)) >= sizeof(myname))
- errx(1, "pw_name too long");
+ if (mypw == NULL)
+ errx(1, "no passwd entry for self");
ngroups = getgroups(NGROUPS_MAX, groups);
if (ngroups == -1)
err(1, "can't get groups");
@@ -361,9 +360,7 @@ main(int argc, char **argv)
if (sflag) {
sh = getenv("SHELL");
if (sh == NULL || *sh == '\0') {
- shargv[0] = strdup(mypw->pw_shell);
- if (shargv[0] == NULL)
- err(1, NULL);
+ shargv[0] = mypw->pw_shell;
} else
shargv[0] = sh;
argv = shargv;
@@ -394,7 +391,7 @@ main(int argc, char **argv)
if (!permit(uid, groups, ngroups, , target, cmd,
(const char **)argv + 1)) {
syslog(LOG_AUTHPRIV | LOG_NOTICE,
- "failed command for %s: %s", myname, cmdline);
+ "failed command for %s: %s", mypw->pw_name, cmdline);
errc(1, EPERM, NULL);
}
@@ -402,7 +399,7 @@ main(int argc, char **argv)
if (nflag)
errx(1, "Authorization required");
- authuser(myname, login_style, rule->options & PERSIST);
+ authuser(mypw->pw_name, login_style, rule->options & PERSIST);
}
if (unveil(_PATH_LOGIN_CONF, "r") == -1)
@@ -418,7 +415,9 @@ main(int argc, char **argv)
err(1, "pledge");
rv = getpwuid_r(target, , targpwbuf, sizeof(targpwbuf),
);
- if (rv != 0 || targpw == NULL)
+ if (rv != 0)
+ err(1, "getpwuid_r failed");
+ if (targpw == NULL)
errx(1, "no passwd entry for target");
if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
@@ -438,7 +437,7 @@ main(int argc, char **argv)
err(1, "pledge");
syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
- myname, cmdline, targpw->pw_name, cwd);
+ mypw->pw_name, cmdline, targpw->pw_name, cwd);
envp = prepenv(rule);
Re: use getpwuid_r in doas
On Tue, May 21, 2019 at 01:07:10PM -0400, Ted Unangst wrote:
> I have a coming change which will need to access both the calling user and
> target users' passwd entries. In order to accomplish this, we need to switch
> to the reentrant flavor of getpwuid. No behaviorial change, but I think this
> is clearer and less error prone as well, versus reusing a pointer to static
> storage.
ok lteo@
> Index: doas.c
> ===
> RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 doas.c
> --- doas.c17 Jan 2019 05:35:35 - 1.74
> +++ doas.c21 May 2019 17:04:04 -
> @@ -289,13 +289,15 @@ main(int argc, char **argv)
> const char *cmd;
> char cmdline[LINE_MAX];
> char myname[_PW_NAME_LEN + 1];
> - struct passwd *pw;
> + char mypwbuf[_PW_BUF_LEN], targpwbuf[_PW_BUF_LEN];
> + struct passwd mypwstore, targpwstore;
> + struct passwd *mypw, *targpw;
> const struct rule *rule;
> uid_t uid;
> uid_t target = 0;
> gid_t groups[NGROUPS_MAX + 1];
> int ngroups;
> - int i, ch;
> + int i, ch, rv;
> int sflag = 0;
> int nflag = 0;
> char cwdpath[PATH_MAX];
> @@ -346,10 +348,10 @@ main(int argc, char **argv)
> } else if ((!sflag && !argc) || (sflag && argc))
> usage();
>
> - pw = getpwuid(uid);
> - if (!pw)
> - err(1, "getpwuid failed");
> - if (strlcpy(myname, pw->pw_name, sizeof(myname)) >= sizeof(myname))
> + rv = getpwuid_r(uid, , mypwbuf, sizeof(mypwbuf), );
> + if (rv != 0 || mypw == NULL)
> + err(1, "getpwuid_r failed");
> + if (strlcpy(myname, mypw->pw_name, sizeof(myname)) >= sizeof(myname))
> errx(1, "pw_name too long");
> ngroups = getgroups(NGROUPS_MAX, groups);
> if (ngroups == -1)
> @@ -359,7 +361,7 @@ main(int argc, char **argv)
> if (sflag) {
> sh = getenv("SHELL");
> if (sh == NULL || *sh == '\0') {
> - shargv[0] = strdup(pw->pw_shell);
> + shargv[0] = strdup(mypw->pw_shell);
> if (shargv[0] == NULL)
> err(1, NULL);
> } else
> @@ -415,11 +417,11 @@ main(int argc, char **argv)
> if (pledge("stdio rpath getpw exec id", NULL) == -1)
> err(1, "pledge");
>
> - pw = getpwuid(target);
> - if (!pw)
> + rv = getpwuid_r(target, , targpwbuf, sizeof(targpwbuf),
> );
> + if (rv != 0 || targpw == NULL)
> errx(1, "no passwd entry for target");
>
> - if (setusercontext(NULL, pw, target, LOGIN_SETGROUP |
> + if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
> LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
> LOGIN_SETUSER) != 0)
> errx(1, "failed to set user context for target");
> @@ -436,7 +438,7 @@ main(int argc, char **argv)
> err(1, "pledge");
>
> syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
> - myname, cmdline, pw->pw_name, cwd);
> + myname, cmdline, targpw->pw_name, cwd);
>
> envp = prepenv(rule);
>
>
use getpwuid_r in doas
I have a coming change which will need to access both the calling user and
target users' passwd entries. In order to accomplish this, we need to switch
to the reentrant flavor of getpwuid. No behaviorial change, but I think this
is clearer and less error prone as well, versus reusing a pointer to static
storage.
Index: doas.c
===
RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.74
diff -u -p -r1.74 doas.c
--- doas.c 17 Jan 2019 05:35:35 - 1.74
+++ doas.c 21 May 2019 17:04:04 -
@@ -289,13 +289,15 @@ main(int argc, char **argv)
const char *cmd;
char cmdline[LINE_MAX];
char myname[_PW_NAME_LEN + 1];
- struct passwd *pw;
+ char mypwbuf[_PW_BUF_LEN], targpwbuf[_PW_BUF_LEN];
+ struct passwd mypwstore, targpwstore;
+ struct passwd *mypw, *targpw;
const struct rule *rule;
uid_t uid;
uid_t target = 0;
gid_t groups[NGROUPS_MAX + 1];
int ngroups;
- int i, ch;
+ int i, ch, rv;
int sflag = 0;
int nflag = 0;
char cwdpath[PATH_MAX];
@@ -346,10 +348,10 @@ main(int argc, char **argv)
} else if ((!sflag && !argc) || (sflag && argc))
usage();
- pw = getpwuid(uid);
- if (!pw)
- err(1, "getpwuid failed");
- if (strlcpy(myname, pw->pw_name, sizeof(myname)) >= sizeof(myname))
+ rv = getpwuid_r(uid, , mypwbuf, sizeof(mypwbuf), );
+ if (rv != 0 || mypw == NULL)
+ err(1, "getpwuid_r failed");
+ if (strlcpy(myname, mypw->pw_name, sizeof(myname)) >= sizeof(myname))
errx(1, "pw_name too long");
ngroups = getgroups(NGROUPS_MAX, groups);
if (ngroups == -1)
@@ -359,7 +361,7 @@ main(int argc, char **argv)
if (sflag) {
sh = getenv("SHELL");
if (sh == NULL || *sh == '\0') {
- shargv[0] = strdup(pw->pw_shell);
+ shargv[0] = strdup(mypw->pw_shell);
if (shargv[0] == NULL)
err(1, NULL);
} else
@@ -415,11 +417,11 @@ main(int argc, char **argv)
if (pledge("stdio rpath getpw exec id", NULL) == -1)
err(1, "pledge");
- pw = getpwuid(target);
- if (!pw)
+ rv = getpwuid_r(target, , targpwbuf, sizeof(targpwbuf),
);
+ if (rv != 0 || targpw == NULL)
errx(1, "no passwd entry for target");
- if (setusercontext(NULL, pw, target, LOGIN_SETGROUP |
+ if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
LOGIN_SETUSER) != 0)
errx(1, "failed to set user context for target");
@@ -436,7 +438,7 @@ main(int argc, char **argv)
err(1, "pledge");
syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
- myname, cmdline, pw->pw_name, cwd);
+ myname, cmdline, targpw->pw_name, cwd);
envp = prepenv(rule);
