[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: Was [PATCH] size reduction and better error reporting
From:       Tito <farmatito () tiscali ! it>
Date:       2010-09-27 21:36:20
Message-ID: 201009272336.20467.farmatito () tiscali ! it
[Download RAW message or body]

On Monday 27 September 2010 07:44:42 Tito wrote:
> On Monday 27 September 2010 02:36:04 Harald Becker wrote:
> > Hi Tito!
> > 
> > > the patch previously sent reduces the size of the deluser/delgroup
> > > commands. Size reduction with all options enabled is:
> > Fine! My approch was just a quick rewrite (without size considerations),
> > as I needed that applets function in some scripts.
> > 
> > > 1) we don't warn about removing user's primary group when doing delgroup
> > 
> > IMHO the only test required would be deleting a group (not user from
> > group) that is still used as a primary group in /etc/passwd.
> 
> Yes.
> 
> > It is not  necessary to check if it's the users primary group while removing a user
> > from a group. This check would even be wrong, as it is a valid usage of
> > a group being used as a users primary group without user being
> > explicitly listed in the group file. The user this way has full group
> > privileges, but can't change back to that group if the group gets
> > changed (either explicitly or more likely implicitly by executing a SGID
> > program = tricky usage of that feature).
> 
> Was not aware of that, so lets drop it.
>  
> > 
> > > 2) we don't remove users primary group when doing deluser
> > 
> > That would be fine, if this could be done ... else we may get left overs
> > of unknown users in the group file. Especially as admins tend to be
> > forgetfully ;-)
> 
> A quick and dirty solution would be to call deluser_main with the 
> correct args one more time or use system().
> The question is when to do it, befor or after having deleted the user?
>  
> > 
> > >         3) -h --help are treated as user/group names
> > 
> > Isn't the processing of those options done by the busybox main?
> > deluser/delgroup --help worked before ... so what has changed at this part?
> 
> It is taken care of it in the getopt32() function, but we don't use it here.
> I think this was there also before, but maybe it is overkill to fix it
> as we get the help text by simply running deluser without arguments.
> 
> 
> > 
> > --
> > Harald
> > 
> 
> Ciao,
> Tito

Hi,
attached you will find a alternative proof of concept  deluser/delgroup
that implements most of the previuosly discussed/requested features:

1) error on non existent user/group
2) error if removing user's primary group
3) deluser user and his primary group (if delgroup is enabled)

with the exception of -h/--help commandline parsing (is overkill ).
The code is from the obfuscation departement ;-D
Size vs current (with all options enabled) is not so bad:

 busybox_old busybox_unstripped
function                                             old     new   delta
deluser_main                                         189     222     +33
.rodata                                           134994  135016     +22
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 2/0 up/down: 55/0)               Total: 55 bytes

Maybe it could be improved and cleaned up if there is some interest.

Ciao,
Tito

["deluser.c" (text/x-csrc)]

/* vi: set sw=4 ts=4: */
/*
 * deluser/delgroup implementation for busybox
 *
 * Copyright (C) 1999 by Lineo, inc. and John Beppu
 * Copyright (C) 1999,2000,2001 by John Beppu <beppu@codepoet.org>
 * Copyright (C) 2007 by Tito Ragusa <farmatito@tiscali.it>
 *
 * Licensed under GPL version 2, see file LICENSE in this tarball for details.
 *
 */
#include "libbb.h"

int deluser_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int deluser_main(int argc, char **argv)
{
	char *name =  argv[1];
	char *member = NULL;
	const char *pfile = bb_path_passwd_file;
	const char *sfile = NULL;
	int deluser = (ENABLE_DELUSER && applet_name[3] == 'u');

	switch (argc) {
#ifdef ENABLE_FEATURE_DEL_USER_FROM_GROUP
		case 3:
			if (deluser)
				bb_show_usage();
			name = argv[2];
			member = argv[1];
			/* Fallthrough */
#endif
		case 2:
			if (geteuid())
				bb_error_msg_and_die(bb_msg_perm_denied_are_you_root);
			if (deluser) {
				if (ENABLE_FEATURE_SHADOWPASSWDS)
					sfile = bb_path_shadow_file;
				xgetpwnam(name);
 do_delgroup:
				do {
					if (update_passwd(pfile, name, NULL, member) == -1)
						return EXIT_FAILURE;
					pfile = sfile;
					sfile = NULL;
				} while (pfile);
			}
			/* If delgroup is enabled we use it also to remove user's primary group */
			while (ENABLE_DELGROUP && deluser >= 0) {
				xgetgrnam(name);
				if (!deluser && argc != 3 && getpwnam(name))
					bb_error_msg_and_die("is %s's primary group", name);
				deluser = -1;
				pfile = bb_path_group_file;
				if (ENABLE_FEATURE_SHADOWPASSWDS)
					sfile = bb_path_gshadow_file;
				goto do_delgroup;
			}
			break;
		default:
			bb_show_usage();
	}
	return EXIT_SUCCESS;
}


_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[prev in list] [next in list] [prev in thread] [next in thread]