Bug in the deluser applet
tito
farmatito at tiscali.it
Wed Feb 18 15:54:08 UTC 2015
On Wednesday 18 February 2015 16:37:59 you wrote:
> > Actually, the locking and swapping logic would be common anyway which
> > already resides in update_passwd, so if that reusable component is not
> > moved to a separate function to be reused, we could use put our
> > conditional magic into the while loop. I am preparing with that change
> > now.
>
> Please check the patch below. I tested it and it works for me. The
> coding style may not suit the existing busybox coding style, but I do
> not mind that for now as I will use the patch as is for now in my
> local fork anyhow. Any (non-stylistic) feedback is welcome.
>
> commit 22a7da560af82c33ca9334039522e9a03aab29b3
> Author: Laszlo Papp <laszlo.papp at polatis.com>
> Date: Wed Feb 18 15:20:58 2015 +0000
>
> Delete the user from all the groups for user deletion
>
> diff --git a/libbb/update_passwd.c b/libbb/update_passwd.c
> index a30af6f..42998a9 100644
> --- a/libbb/update_passwd.c
> +++ b/libbb/update_passwd.c
> @@ -62,6 +62,8 @@ static void check_selinux_update_passwd(const char *username)
> only if CONFIG_PASSWD=y and applet_name[0] == 'p' like in passwd
> or if CONFIG_CHPASSWD=y and applet_name[0] == 'c' like in chpasswd
>
> + 8) delete a user from all groups : update_passwd(FILE, NULL, NULL, MEMBER)
> +
> This function does not validate the arguments fed to it
> so the calling program should take care of that.
>
> @@ -81,7 +83,7 @@ int FAST_FUNC update_passwd(const char *filename,
> FILE *new_fp;
> char *fnamesfx;
> char *sfx_char;
> - char *name_colon;
> + char *name_colon = 0;
> unsigned user_len;
> int old_fd;
> int new_fd;
> @@ -99,13 +101,15 @@ int FAST_FUNC update_passwd(const char *filename,
> if (filename == NULL)
> return ret;
>
> - check_selinux_update_passwd(name);
> + if (name) check_selinux_update_passwd(name);
>
> /* New passwd file, "/etc/passwd+" for now */
> fnamesfx = xasprintf("%s+", filename);
> sfx_char = &fnamesfx[strlen(fnamesfx)-1];
> - name_colon = xasprintf("%s:", name);
> - user_len = strlen(name_colon);
> + if (name) {
> + name_colon = xasprintf("%s:", name);
> + user_len = strlen(name_colon);
> + }
>
> if (shadow)
> old_fp = fopen(filename, "r+");
> @@ -162,21 +166,37 @@ int FAST_FUNC update_passwd(const char *filename,
> /* Read current password file, write updated /etc/passwd+ */
> changed_lines = 0;
> while (1) {
> - char *cp, *line;
> -
> - line = xmalloc_fgetline(old_fp);
> - if (!line) /* EOF/error */
> - break;
> - if (strncmp(name_colon, line, user_len) != 0) {
> - fprintf(new_fp, "%s\n", line);
> - goto next;
> - }
> -
> - /* We have a match with "name:"... */
> - cp = line + user_len; /* move past name: */
> + char *cp, *line;
> + if (!name && member) {
> + struct group* g;
> + if ((g = getgrent())) {
> + char gline[LINE_MAX];
> + char **s= g->gr_mem;
> + snprintf(gline, sizeof(gline), "%s:%s:%i:",
> g->gr_name, g->gr_passwd, g->gr_gid);
> + while (*s) {
> + if (strcmp(*s, member)) { if (s != g->gr_mem)
> strcat(gline, ","); strcat(gline, *s); }
> + ++s;
> + }
> + fprintf(new_fp, "%s\n", gline);
> + goto next;
> + } else {
> + break;
> + }
> + } else {
> + line = xmalloc_fgetline(old_fp);
> + if (!line) /* EOF/error */
> + break;
> + if (!name_colon || strncmp(name_colon, line, user_len) != 0) {
> + fprintf(new_fp, "%s\n", line);
> + goto next;
> + }
> +
> + /* We have a match with "name:"... */
> + cp = line + user_len; /* move past name: */
> + }
>
> #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP
> - if (member) {
> + if (name && member) {
> /* It's actually /etc/group+, not /etc/passwd+ */
> if (ENABLE_FEATURE_ADDUSER_TO_GROUP
> && applet_name[0] == 'a'
> @@ -240,7 +260,7 @@ int FAST_FUNC update_passwd(const char *filename,
>
> if (changed_lines == 0) {
> #if ENABLE_FEATURE_ADDUSER_TO_GROUP || ENABLE_FEATURE_DEL_USER_FROM_GROUP
> - if (member) {
> + if (name && member) {
> if (ENABLE_ADDGROUP && applet_name[0] == 'a')
> bb_error_msg("can't find %s in %s",
> name, filename);
> if (ENABLE_DELGROUP && applet_name[0] == 'd')
> diff --git a/loginutils/deluser.c b/loginutils/deluser.c
> index 01a9386..a3f5f3a 100644
> --- a/loginutils/deluser.c
> +++ b/loginutils/deluser.c
> @@ -82,6 +82,9 @@ int deluser_main(int argc, char **argv)
> do_delgroup:
> /* "delgroup GROUP" or "delgroup USER GROUP" */
> if (do_deluser < 0) { /* delgroup after deluser? */
> + pfile = bb_path_group_file;
> + if (update_passwd(pfile, NULL, NULL, name) == -1)
> + return EXIT_FAILURE;
> gr = getgrnam(name);
> if (!gr)
> return EXIT_SUCCESS;
> @@ -99,7 +102,6 @@ int deluser_main(int argc, char **argv)
> }
> //endpwent();
> }
> - pfile = bb_path_group_file;
> if (ENABLE_FEATURE_SHADOWPASSWDS)
> sfile = bb_path_gshadow_file;
> }
>
Hi,
just for fun a different approach, this seems to work
without touching update_passwd. Probably it could be
shrinked and refactored even more. Doesn't work
with bb's internal pwdgrp as it triggers some bug,
will post in a separate thread about that.
Ciao,
Tito
--- loginutils/deluser.c.original 2015-02-18 15:12:10.810954596 +0100
+++ loginutils/deluser.c 2015-02-18 16:47:34.186322005 +0100
@@ -37,7 +37,8 @@
const char *sfile;
/* Are we deluser or delgroup? */
int do_deluser = (ENABLE_DELUSER && (!ENABLE_DELGROUP || applet_name[3] == 'u'));
-
+ struct group *gr;
+ struct passwd *pw;
#if !ENABLE_LONG_OPTS
const int opt_delhome = 0;
#else
@@ -69,8 +70,6 @@
case 2:
if (do_deluser) {
/* "deluser USER" */
- struct passwd *pw;
-
pw = xgetpwnam(name); /* bail out if USER is wrong */
pfile = bb_path_passwd_file;
if (ENABLE_FEATURE_SHADOWPASSWDS)
@@ -78,7 +77,6 @@
if (opt_delhome)
remove_file(pw->pw_dir, FILEUTILS_RECUR);
} else {
- struct group *gr;
do_delgroup:
/* "delgroup GROUP" or "delgroup USER GROUP" */
if (do_deluser < 0) { /* delgroup after deluser? */
@@ -90,7 +88,6 @@
}
if (!member) {
/* "delgroup GROUP" */
- struct passwd *pw;
/* Check if the group is in use */
while ((pw = getpwent()) != NULL) {
if (pw->pw_gid == gr->gr_gid)
@@ -116,6 +113,26 @@
if (ENABLE_DELGROUP && do_deluser > 0) {
/* "deluser USER" also should try to delete
+ * the USER from all groups in which he is member.
+ */
+ gid_t *groups = NULL;
+ int n = 64;
+retry:
+ groups = xrealloc(groups, n * sizeof(gid_t));
+ if(getgrouplist(name, pw->pw_gid, groups, &n) == -1)
+ goto retry;
+
+ while (--n >= 1) { /* our gid is the last group */
+ gr = getgrgid(groups[n]);
+ if (!gr)
+ continue;
+ update_passwd(bb_path_group_file, gr->gr_name, NULL, name);
+ if (ENABLE_FEATURE_SHADOWPASSWDS)
+ update_passwd(bb_path_gshadow_file, gr->gr_name, NULL, name);
+ }
+ if (ENABLE_FEATURE_CLEAN_UP)
+ free(groups);
+ /* "deluser USER" also should try to delete
* same-named group. IOW: do "delgroup USER"
*/
// On debian deluser is a perl script that calls userdel.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20150218/c61ff5f9/attachment-0001.html>
More information about the busybox
mailing list