OpenBSD and Incorrect UserMod -G

The usermod program is used to manage a user entry in the password and group data files on a unix system.
The OpenBSD version of usermod claims that the -G switch will set the secondary groups (i.e. those in the /etc/group file) for a given user. In actual fact, it appends the user to groups to which he does not belong, and does not remove him from any groups, so it is more of an append, rather than a set.
The source code can be downloaded as follows:

cvs -qd anoncvs@anoncvs.ca.openbsd.org:/cvs/src/usr.sbin get -P user

And the user.c file needs modifying to:

• change the append_group function to be modify_group
• Not to delete the group for any groups to which the user already belongs
• Remove the \n newline
• Remove the user from each line when he is found in a group entry
• Add the user to the end of a line for a requested group
• Append the \n newline

The diff is run as follows:

cvs diff -u

And the output is here:

Index: user.c
===================================================================
RCS file: /cvs/src/usr.sbin/user/user.c,v
retrieving revision 1.81
diff -u -r1.81 user.c
--- user.c 16 Apr 2011 07:41:08 -0000 1.81
+++ user.c 17 Apr 2011 18:35:26 -0000
@@ -485,9 +485,9 @@
return 1;
}

-/* modify the group entries for all `groups', by adding `user' */
+/* modify the group entries for all `groups', by adding or removing `user' */
static int
-append_group(char *user, int ngroups, const char **groups)
+modify_group(char *user, int ngroups, const char **groups)
{
struct group *grp;
struct stat st;
@@ -500,20 +500,19 @@
int cc;
int i;
int j;
+ int c ;
+ int d ;
+ int l ;

+ /* Check groups are valid */
for (i = 0 ; i < ngroups ; i++) {
if ((grp = getgrnam(groups[i])) == NULL) {
warnx("can't append group `%s' for user `%s'",
groups[i], user);
- } else {
- for (j = 0 ; grp->gr_mem[j] ; j++) {
- if (strcmp(user, grp->gr_mem[j]) == 0) {
- /* already in it */
- groups[i] = "";
- }
- }
}
}
+
+ /* Open and lock files */
if ((from = fopen(_PATH_GROUP, "r")) == NULL) {
warn("can't append group for `%s': can't open `%s'", user,
_PATH_GROUP);
@@ -536,7 +535,10 @@
warn("can't append group: fdopen `%s' failed", f);
return 0;
}
+
while (fgets(buf, sizeof(buf), from) != NULL) {
+
+ /* Load entry from file */
cc = strlen(buf);
if (cc > 0 && buf[cc - 1] != '\n' && !feof(from)) {
while (fgetc(from) != '\n' && !feof(from))
@@ -545,30 +547,53 @@
_PATH_GROUP, buf, cc);
continue;
}
- if ((colon = strchr(buf, ':')) == NULL) {
+ while (isspace(buf[cc-1])) cc-- ;
+ buf[cc]='\0' ;
+
+ /* Remove username from list */
+ for (c=0,d=0; buf[c]!=':' && buf[c]!='\0'; c++,d++) ;
+ if (buf[d]==':') d++ ;
+ for (; buf[d]!=':' && buf[d]!='\0'; d++) ;
+ if (buf[d]==':') d++ ;
+ for (; buf[d]!=':' && buf[d]!='\0'; d++) ;
+ if (buf[d]!=':') {
warnx("badly formed entry `%s'", buf);
continue;
}
+ l=strlen(user) ;
+ for (i=d; buf[i]!='\0' &&
+ !(strncmp(user, &buf[i], l)==0 &&
+ !isalnum(buf[i+l])); i++) ;
+ if (buf[i]!='\0') {
+ if (buf[i-1]==',') { i-- ; l++ ; }
+ else if (buf[i-1]==':') { l++ ; }
+ for (i+=l; buf[i]!='\0'; i++)
+ buf[i-l]=buf[i] ;
+ buf[i-l]='\0' ;
+ cc=strlen(buf) ;
+ }
+
+ /* Append username to list if in group */
for (i = 0 ; i < ngroups ; i++) {
- j = (int)(colon - buf);
+ j = (int)c ;
if (strncmp(groups[i], buf, j) == 0 &&
groups[i][j] == '\0') {
- while (isspace(buf[cc - 1]))
- cc--;
- buf[(j = cc)] = '\0';
- if (buf[strlen(buf) - 1] != ':')
+ j=cc ;
+ if (buf[strlen(buf)] != ':')
strlcat(buf, ",", sizeof(buf));
- cc = strlcat(buf, user, sizeof(buf)) + 1;
- if (cc >= sizeof(buf)) {
+ cc = strlcat(buf, user, sizeof(buf)) ;
+ if (cc >= sizeof(buf)-1) {
warnx("Warning: group `%s' would "
"become too long, not modifying",
groups[i]);
cc = j + 1;
}
- buf[cc - 1] = '\n';
buf[cc] = '\0';
}
}
+ buf[cc++]='\n' ;
+ buf[cc]='\0' ;
+
if (fwrite(buf, cc, 1, to) != 1) {
(void) fclose(from);
(void) fclose(to);
@@ -1195,7 +1220,7 @@
errx(EXIT_FAILURE, "can't create gid %d for login name %s",
gid, login_name);
}
- if (up->u_groupc > 0 && !append_group(login_name, up->u_groupc, up->u_groupv)) {
+ if (up->u_groupc > 0 && !modify_group(login_name, up->u_groupc, up->u_groupv)) {
(void) close(ptmpfd);
pw_abort();
errx(EXIT_FAILURE, "can't append `%s' to new groups", login_name);
@@ -1538,7 +1563,7 @@
homedir, pwp->pw_dir);
}
if (up->u_groupc > 0 &&
- !append_group(newlogin, up->u_groupc, up->u_groupv)) {
+ !modify_group(newlogin, up->u_groupc, up->u_groupv)) {
(void) close(ptmpfd);
pw_abort();
errx(EXIT_FAILURE, "can't append `%s' to new groups",
Index: usermod.8
===================================================================
RCS file: /cvs/src/usr.sbin/user/usermod.8,v
retrieving revision 1.25
diff -u -r1.25 usermod.8
--- usermod.8 8 Apr 2011 18:13:54 -0000 1.25
+++ usermod.8 17 Apr 2011 18:35:26 -0000
@@ -103,7 +103,12 @@
.It Fl G Ar secondary-group[,group,...]
Sets the secondary groups the user will be a member of in the
.Pa /etc/group
-file.
+file. If the user is not in a group that is listed, the user
+will be added to the
+.Pa /etc/group
+file. If the user is a member of the group in the
+.Pa /etc/group
+file, but is not listed, he will be removed from that group.
.It Xo
.Fl g Ar gid | name | Li =uid
.Xc
@@ -236,6 +241,9 @@
.Nm
utility first appeared in
.Ox 2.7 .
+.Nm
+was modified in April 2011 to make the -G switch set secondary groups for user,
+rather than append user to listed groups.
.Sh AUTHORS
The
.Nm