mirrors/passt
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Clean up parsing in conf_runas()

Browse source 
conf_runas() handles several of the different possible cases for the
--runas argument in a slightly odd order.  Although it can parse both
numeric UIDs/GIDs and user/group names, it can't parse a numeric UID
combined with a group name or vice versa.  That's not obviously useful, but
it's slightly surprising gap to have.

Rework the parsing to be more systematic: first split the option into
user and (optional) group parts, then separately parse each part as either
numeric or a name.  As a bonus this removes some clang-tidy warnings.

While we're there also add cppcheck suppressions for getpwnam() and
getgrnam().  It complains about those because they're not reentrant.
passt is single threaded though, and is always likely to be during
this initialization code, even if we multithread later.

There were some existing suppressions for these in the cppcheck
invocation but they're no longer up to date.  Replace them with inline
suppressions which, being next to the code, are more likely to stay
correct.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
This commit is contained in:
David Gibson 2022-09-28 14:33:18 +10:00 committed by Stefano Brivio
parent 7d4e50827c
commit 68ef4931cb
2 changed files with 52 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
Makefile
View file

@ -282,12 +282,12 @@ cppcheck: $(SRCS) $(HEADERS)
$(SYSTEM_INCLUDES:%=--config-exclude=%) \
$(SYSTEM_INCLUDES:%=--suppress=*:%/*) \
$(SYSTEM_INCLUDES:%=--suppress=unmatchedSuppression:%/*) \
--inline-suppr \
--suppress=objectIndex:tcp.c --suppress=objectIndex:udp.c \
--suppress=va_list_usedBeforeStarted:util.c \
--suppress=unusedFunction \
--suppress=knownConditionTrueFalse:conf.c \
--suppress=strtokCalled:conf.c --suppress=strtokCalled:qrap.c \
--suppress=getpwnamCalled:passt.c \
--suppress=localtimeCalled:pcap.c \
--suppress=unusedStructMember:pcap.c \
--suppress=funcArgNamesDifferent:util.h \
@ -295,7 +295,6 @@ cppcheck: $(SRCS) $(HEADERS)
\
--suppress=unmatchedSuppression:conf.c \
--suppress=unmatchedSuppression:dhcp.c \
--suppress=unmatchedSuppression:passt.c \
--suppress=unmatchedSuppression:pcap.c \
--suppress=unmatchedSuppression:qrap.c \
--suppress=unmatchedSuppression:tcp.c \

79
conf.c
View file

@ -859,46 +859,50 @@ dns6:
*
* Return: 0 on success, negative error code on failure
*/
static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gid)
static int conf_runas(char *opt, unsigned int *uid, unsigned int *gid)
{
char ubuf[LOGIN_NAME_MAX], gbuf[LOGIN_NAME_MAX], *endptr;
const char *uopt, *gopt = NULL;
char *sep = strchr(opt, ':');
char *endptr;
if (sep) {
*sep = '\0';
gopt = sep + 1;
}
uopt = opt;
*gid = *uid = strtol(uopt, &endptr, 0);
if (*endptr) {
#ifndef GLIBC_NO_STATIC_NSS
/* Not numeric, look up as a username */
struct passwd *pw;
struct group *gr;
/* NOLINTNEXTLINE(cert-err34-c): 2 if conversion succeeds */
if (sscanf(opt, "%u:%u", uid, gid) == 2 && *uid && *gid)
return 0;
*uid = strtol(opt, &endptr, 0);
if (!*endptr && (*gid = *uid))
return 0;
#ifdef GLIBC_NO_STATIC_NSS
(void)ubuf;
(void)gbuf;
(void)pw;
(void)gr;
return -EINVAL;
/* cppcheck-suppress getpwnamCalled */
if (!(pw = getpwnam(uopt)) || !(*uid = pw->pw_uid))
return -ENOENT;
*gid = pw->pw_gid;
#else
/* NOLINTNEXTLINE(cert-err34-c): 2 if conversion succeeds */
if (sscanf(opt, "%" STR(LOGIN_NAME_MAX) "[^:]:"
"%" STR(LOGIN_NAME_MAX) "s", ubuf, gbuf) == 2) {
if (!(pw = getpwnam(ubuf)) || !(*uid = pw->pw_uid))
return -ENOENT;
if (!(gr = getgrnam(gbuf)) || !(*gid = gr->gr_gid))
return -ENOENT;
return 0;
return -EINVAL;
#endif
}
pw = getpwnam(ubuf);
if (!pw || !(*uid = pw->pw_uid) || !(*gid = pw->pw_gid))
if (!gopt)
return 0;
*gid = strtol(gopt, &endptr, 0);
if (*endptr) {
#ifndef GLIBC_NO_STATIC_NSS
/* Not numeric, look up as a group name */
struct group *gr;
/* cppcheck-suppress getgrnamCalled */
if (!(gr = getgrnam(gopt)))
return -ENOENT;
*gid = gr->gr_gid;
#else
return -EINVAL;
#endif
}
return 0;
#endif /* !GLIBC_NO_STATIC_NSS */
}
/**
@ -909,10 +913,9 @@ static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gid)
*
* Return: 0 on success, negative error code on failure
*/
static int conf_ugid(const char *runas, uid_t *uid, gid_t *gid)
static int conf_ugid(char *runas, uid_t *uid, gid_t *gid)
{
const char root_uid_map[] = " 0 0 4294967295";
struct passwd *pw;
char buf[BUFSIZ];
int ret;
int fd;
@ -951,7 +954,10 @@ static int conf_ugid(const char *runas, uid_t *uid, gid_t *gid)
/* ...otherwise use nobody:nobody */
warn("Don't run as root. Changing to nobody...");
{
#ifndef GLIBC_NO_STATIC_NSS
struct passwd *pw;
/* cppcheck-suppress getpwnamCalled */
pw = getpwnam("nobody");
if (!pw) {
perror("getpwnam");
@ -961,11 +967,10 @@ static int conf_ugid(const char *runas, uid_t *uid, gid_t *gid)
*uid = pw->pw_uid;
*gid = pw->pw_gid;
#else
(void)pw;
/* Common value for 'nobody', not really specified */
*uid = *gid = 65534;
#endif
}
return 0;
}
@ -1032,8 +1037,8 @@ void conf(struct ctx *c, int argc, char **argv)
struct fqdn *dnss = c->dns_search;
uint32_t *dns4 = c->ip4.dns;
int name, ret, mask, b, i;
const char *runas = NULL;
unsigned int ifi = 0;
char *runas = NULL;
uid_t uid;
gid_t gid;