[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: more on id
From: Tito <farmatito () tiscali ! it>
Date: 2008-09-27 21:53:15
Message-ID: 200809272353.15329.farmatito () tiscali ! it
[Download RAW message or body]
On Saturday 27 September 2008 16:45:47 you wrote:
> On Saturday 27 September 2008 16:00, Tito wrote:
> > Hi Denys,
> > maybe this patches slipped through as my cc to you get
> > bounced by a spam filter that blacklisted my ip?
>
> I received them. sorry about this delay.
>
> > # [PATCH 1/3] add bb_getgrouplist_malloc to libbb Tito
> > http://www.busybox.net/lists/busybox/2008-September/033078.html
> > # [PATCH 2/3] port id to bb_getgrouplist_malloc + fixes Tito
> > http://www.busybox.net/lists/busybox/2008-September/033079.html
>
> Let's look at these two patches together.
>
> Here:
>
> if (username) {
> -#if HAVE_getgrouplist
> - int m;
> -#endif
> p = getpwnam(username);
> /* xuname2uid is needed because it exits on failure */
> uid = xuname2uid(username);
> gid = p->pw_gid; /* in this case PRINT_REAL is the same */
> -
> -#if HAVE_getgrouplist
> - n = 16;
> - groups = NULL;
> - do {
> - m = n;
> - groups = xrealloc(groups, sizeof(groups[0]) * m);
> - getgrouplist(username, gid, groups, &n); /* GNUism? */
> - } while (n > m);
> -#endif
> - } else {
> -#if HAVE_getgrouplist
> - n = getgroups(0, NULL);
> - groups = xmalloc(sizeof(groups[0]) * n);
> - getgroups(n, groups);
> -#endif
> }
> + group_list = bb_getgrouplist_malloc(uid, gid, NULL);
>
> you replace getgrouplist() call with bb_getgrouplist_malloc().
> bb_getgrouplist_malloc() iterates through the whole set of group records:
>
> + while((grp = getgrent())) {
> ...
> + while (*(grp->gr_mem)) {
> ...
> + }
> + }
>
> This may be very expensive. I know for the fact than in big organizations
> some groups are HUGE - I saw 500kbytes large "guests" group.
> (nscd died horribly trying to digest such a large group.)
>
> In these cases, user db is not in /etc/XXX files. It is in LDAP etc.
>
> getgrouplist() may have a more clever way of retrieving this data -
> instead of pulling megabytes from user datrabase by iterating through
> all groups it may just directly query uer db "give me group list
> of this user". Likely to be less than kilobyte of data.
>
> There is no way around it. If you want to make sure you have at least
> a fleeting chance of not performing horribly, you must use libc interfaces
> fro retrieving group lists, of which there is two known to me:
> initgroups (standard, but useless in many ceses) and getgrouplist (GNUism).
> Then, *if* your libc is well-written, it will hopefully do it efficiently.
>
> Secondly, make bloatcheck says:
>
> function old new delta
> bb_getgrouplist_malloc - 192 +192
> print_single - 106 +106
> print_group_list - 94 +94
> decode_format_string 824 839 +15
> ...(deleted gcc-induced random jitter +/-9 bytes)...
> printf_full 44 - -44
> id_main 539 331 -208
> ------------------------------------------------------------------------------
> (add/remove: 3/1 grow/shrink: 4/4 up/down: 418/-259) Total: 159 bytes
>
> This does not seem to be a progress.
>
> > # [PATCH 3/3] "euid and egid handling" (forgot the subject here ;-)
> > http://www.busybox.net/lists/busybox/2008-September/033080.html
>
> Patch 2 also mentions "+ fixes".
> Can you send these fixes (dropping bb_getgrouplist_malloc() for now)?
>
> Thanks.
> --
> vda
>
Hi Denys,
I reworked id taking into account your objections.
I'm sending a drop in replacement for the moment as
I would like to hear your opinion about it before producing a diff.
The pros are:
1) the size increase is little:
scripts/bloat-o-meter busybox_old busybox_unstripped
function old new delta
print_single - 129 +129
print_group_list - 84 +84
.rodata 119055 119046 -9
printf_full 44 - -44
id_main 539 393 -146
------------------------------------------------------------------------------
(add/remove: 2/1 grow/shrink: 0/2 up/down: 213/-199) Total: 14 bytes
(BTW: can't say why bb_getgrouplist_malloc doesn't show up in the list, maybe it is inlined?);
2) the code allows for easy adding of euid and egid handling;
3) the code is more readable as there are less #ifdefs;
4) it returns EXIT_ERROR if a gid to groupname translation fails while printing a group list;
5) it returns EXIT_ERROR if getgrouplist is not available and we try to print a group list;
6) adding support for the groups command should be easy.
The code is tested and it seems to work as expected but
hints, critics and improvements are welcome.
Ciao,
Tito
["id.c" (text/x-csrc)]
/* vi: set sw=4 ts=4: */
/*
* Mini id implementation for busybox
*
* Copyright (C) 2000 by Randolph Chung <tausq@debian.org>
*
* Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
*/
/* BB_AUDIT SUSv3 compliant. */
/* Hacked by Tito Ragusa (C) 2004 to handle usernames of whatever length and to
* be more similar to GNU id.
* -Z option support: by Yuichi Nakamura <ynakam@hitachisoft.jp>
* Added -G option Tito Ragusa (C) 2008 for SUSv3.
*/
#include "libbb.h"
#define PRINT_REAL 1
#define NAME_NOT_NUMBER 2
#define JUST_USER 4
#define JUST_GROUP 8
#define JUST_ALL_GROUPS 16
#if ENABLE_SELINUX
#define JUST_CONTEXT 32
#endif
static int print_single(gid_t gid,
char* FAST_FUNC bb_getXXXid(char *name, int bufsize, long uid),
unsigned flags,
const char *prefix)
{
short status = EXIT_SUCCESS;
/* bb_getXXXid(-1) exits on failure */
const char *name = bb_getXXXid(NULL, (!flags) ? 0 : -1, gid);
if (prefix) {
printf("%s", prefix);
}
if (!flags || !(flags & NAME_NOT_NUMBER) || !name) {
printf("%u", gid);
}
if (!flags || flags & NAME_NOT_NUMBER) {
if (name) {
printf((!flags) ? "(%s)" : "%s", name);
} else {
status = EXIT_FAILURE;
}
}
return status;
}
#if (defined(__GLIBC__) && !defined(__UCLIBC__))
#define HAVE_getgrouplist 1
#elif ENABLE_USE_BB_PWD_GRP
#define HAVE_getgrouplist 1
#else
#define HAVE_getgrouplist 0
#endif
static int print_group_list(llist_t *grp_list, unsigned flags, const char *prefix)
{
#if HAVE_getgrouplist
short status = EXIT_SUCCESS;
gid_t *gid;
while (grp_list) {
gid = llist_pop(&grp_list);
status |= print_single(*gid, bb_getgrgid, flags, prefix);
prefix = NULL;
if (ENABLE_FEATURE_CLEAN_UP) {
free(gid);
}
if (grp_list) {
bb_putchar((flags) ? ' ' : ',');
}
}
if (ENABLE_FEATURE_CLEAN_UP) {
free(grp_list);
}
return status;
#else
/* Return an error if getgrouplist is not available and we try to print groups */
return EXIT_FAILURE;
#endif
}
static llist_t *bb_getgrouplist_malloc(uid_t ruid, gid_t rgid, gid_t *egid)
{
llist_t *group_list = NULL;
#if HAVE_getgrouplist
const char *username;
int n = 0;
int i;
gid_t *groups = NULL;
/* Exits failure */
username = bb_getpwuid(NULL, -1, ruid);
/* Get the number of groups we belong to */
getgrouplist(username, rgid, groups, &n);
/* malloc the needed memory */
groups = malloc(sizeof(gid_t) * n);
/* Get the gid list */
getgrouplist(username, rgid, groups, &n);
/* Create our linked list */
for (i = 0; i < n; i++) {
if (i == 1 && egid && rgid != *egid) {
/* Add egid at the second place if needed */
llist_add_to_end(&group_list, memcpy(xmalloc(sizeof(gid_t)), egid, sizeof(gid_t)));
}
llist_add_to_end(&group_list, memcpy(xmalloc(sizeof(gid_t)), &groups[i], sizeof(gid_t)));
}
if (ENABLE_FEATURE_CLEAN_UP)
free(groups);
#endif
/* Return NULL if getgrouplist is not available */
return group_list;
}
int id_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int id_main(int argc UNUSED_PARAM, char **argv)
{
const char *username;
struct passwd *p;
uid_t uid;
gid_t gid;
llist_t *group_list;
unsigned flags;
short status = EXIT_SUCCESS;
#if ENABLE_SELINUX
security_context_t scontext;
#endif
/* Don't allow -n -r -nr -ug -uG -gG -rug -nug -rnug */
/* Don't allow more than one username */
opt_complementary = "?1:u--g:g--u:G--u:u--G:g--G:G--g:r?ugG:n?ugG" USE_SELINUX(":u--Z:Z--u:g--Z:Z--g");
flags = getopt32(argv, "rnugG" USE_SELINUX("Z"));
username = argv[optind];
/* This values could be overwritten later */
uid = geteuid();
gid = getegid();
if (flags & PRINT_REAL) {
uid = getuid();
gid = getgid();
}
if (username) {
p = getpwnam(username);
/* xuname2uid is needed because it exits on failure */
uid = xuname2uid(username);
gid = p->pw_gid; /* in this case PRINT_REAL is the same */
}
group_list = bb_getgrouplist_malloc(uid, gid, NULL);
/* JUST_ALL_GROUPS and JUST_GROUP or JUST_USER are mutually exclusive */
if (flags & JUST_ALL_GROUPS) {
status = print_group_list(group_list, flags, NULL);
} else if (flags & (JUST_GROUP | JUST_USER USE_SELINUX(| JUST_CONTEXT))) {
/* JUST_GROUP and JUST_USER are mutually exclusive */
if (flags & JUST_USER) {
/* Ignore return value, bb_getXXXid(-1) exits on failure */
print_single(uid, bb_getpwuid, flags, NULL);
} else {
print_single(gid, bb_getgrgid, flags, NULL);
}
#if ENABLE_SELINUX
if (flags & JUST_CONTEXT) {
selinux_or_die();
if (username) {
bb_error_msg_and_die("user name can't be passed with -Z");
}
if (getcon(&scontext)) {
bb_error_msg_and_die("can't get process context");
}
puts(scontext);
}
#endif
} else {
/* Print full info like GNU id */
/* bb_getpwuid(0) doesn't exit on failure (returns NULL) */
status = print_single(uid, bb_getpwuid, flags, "uid=");
status |= print_single(gid, bb_getgrgid, flags, " gid=");
status |= print_group_list(group_list, flags, " groups=");
#if ENABLE_SELINUX
if (is_selinux_enabled()) {
security_context_t mysid;
getcon(&mysid);
printf(" context=%s", mysid ? mysid : "unknown");
if (mysid) /* TODO: maybe freecon(NULL) is harmless? */
freecon(mysid);
}
#endif
}
bb_putchar('\n');
fflush_stdout_and_exit(status);
}
_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic