Fix restricted group code for BSDs, move to separate function

4 files changed, 62 insertions, 61 deletions

diff --git a/runopts.h b/runopts.h
index a3f46d2..67d5c48 100644
--- a/runopts.h
+++ b/runopts.h
@@ -92,8 +92,8 @@ typedef struct svr_runopts {
 #endif
 
 	int norootlogin;
-        char *grouploginname;
-        gid_t *grouploginid;
+	char *restrict_group;
+	gid_t restrict_group_gid;
 
 	int noauthpass;
 	int norootpass;
diff --git a/svr-auth.c b/svr-auth.c
index 9c818d9..90334e0 100644
--- a/svr-auth.c
+++ b/svr-auth.c
@@ -226,6 +226,40 @@ out:
 	m_free(methodname);
 }
 
+/* returns DROPBEAR_SUCCESS or DROPBEAR_FAILURE */
+static int check_group_membership(gid_t check_gid, const char* username, gid_t user_gid) {
+	int ngroups, i, ret;
+	gid_t *grouplist = NULL;
+	int match = DROPBEAR_FAILURE;
+
+	for (ngroups = 32; ngroups <= DROPBEAR_NGROUP_MAX; ngroups *= 2) {
+		grouplist = m_malloc(sizeof(gid_t) * ngroups);
+
+		/* BSD returns ret==0 on success. Linux returns ret==ngroups on success */
+		ret = getgrouplist(username, user_gid, grouplist, &ngroups);
+		if (ret >= 0) {
+			break;
+		}
+		m_free(grouplist);
+		grouplist = NULL;
+	}
+
+	if (!grouplist) {
+		dropbear_log(LOG_ERR, "Too many groups for user '%s'", username);
+		return DROPBEAR_FAILURE;
+	}
+
+	for (i = 0; i < ngroups; i++) {
+		if (grouplist[i] == check_gid) {
+			match = DROPBEAR_SUCCESS;
+			break;
+		}
+	}
+	m_free(grouplist);
+
+	return match;
+}
+
 
 /* Check that the username exists and isn't disallowed (root), and has a valid shell.
  * returns DROPBEAR_SUCCESS on valid username, DROPBEAR_FAILURE on failure */
@@ -234,9 +268,6 @@ static int checkusername(char *username, unsigned int userlen) {
 	char* listshell = NULL;
 	char* usershell = NULL;
 	uid_t uid;
-        int ngroups = 32, ret;
-        gid_t *grouplist;
-
 
 	TRACE(("enter checkusername"))
 	if (userlen > MAX_USERNAME_LEN) {
@@ -284,45 +315,16 @@ static int checkusername(char *username, unsigned int userlen) {
 		return DROPBEAR_FAILURE;
 	}
 
-        /* check for login restricted to certain group if desired */
-        if (svr_opts.grouploginid) {
-
-            for ( ; (ngroups <= NGROUPS_MAX) && (ngroups <= INT_MAX / 8); ngroups *= 2){
-
-                    grouplist = malloc(sizeof(gid_t) * ngroups);
-
-                    ret = getgrouplist(ses.authstate.pw_name, ses.authstate.pw_gid, grouplist, &ngroups);
-
-                    if (ret != -1){
-                        break;
-                    }
-
-                    free(grouplist);
-                    ngroups *= 2;
-            }
-
-            if ((ngroups > NGROUPS_MAX / 8) || (ngroups > INT_MAX / 8)){
-
-                    TRACE(("Cannot walk group structure for current user, too many groups"))
-                    dropbear_log(LOG_ERR, "Cannot walk group structure for current user, too many groups");
-                    return DROPBEAR_FAILURE;
-            }
-
-            ngroups = 0;
-            for (int i = 0; i < ret; i++){
-                    if (grouplist[i] == *svr_opts.grouploginid){
-                    ngroups = 1; //Just used as a flag to indicate success;
-                    break;
-                }
-
-            }
-
-            if (!ngroups){
-                    TRACE(("leave checkusername: user not in permitted group"))
-                    dropbear_log(LOG_WARNING, "logins are restricted to the group %s but user %s is not a member", svr_opts.grouploginname, ses.authstate.pw_name);
-                    return DROPBEAR_FAILURE;
-            }
-        }
+	/* check for login restricted to certain group if desired */
+	if (svr_opts.restrict_group) {
+		if (check_group_membership(svr_opts.restrict_group_gid,
+				ses.authstate.pw_name, ses.authstate.pw_gid) == DROPBEAR_FAILURE) {
+			dropbear_log(LOG_WARNING,
+				"Logins are restricted to the group %s but user '%s' is not a member",
+				svr_opts.restrict_group, ses.authstate.pw_name);
+			return DROPBEAR_FAILURE;
+		}
+	}
 
 	TRACE(("shell is %s", ses.authstate.pw_shell))
 
diff --git a/svr-runopts.c b/svr-runopts.c
index 78764a1..1b057cf 100644
--- a/svr-runopts.c
+++ b/svr-runopts.c
@@ -70,7 +70,7 @@ static void printhelp(const char * progname) {
 					"-m		Don't display the motd on login\n"
 #endif
 					"-w		Disallow root logins\n"
-                                        "-G		Restrict logins to members of specified group\n"
+					"-G		Restrict logins to members of specified group\n"
 #if DROPBEAR_SVR_PASSWORD_AUTH || DROPBEAR_SVR_PAM_AUTH
 					"-s		Disable password logins\n"
 					"-g		Disable password logins for root\n"
@@ -135,8 +135,8 @@ void svr_getopts(int argc, char ** argv) {
 	svr_opts.forced_command = NULL;
 	svr_opts.forkbg = 1;
 	svr_opts.norootlogin = 0;
-        svr_opts.grouploginname = NULL;
-        svr_opts.grouploginid = NULL;
+	svr_opts.restrict_group = NULL;
+	svr_opts.restrict_group_gid = 0;
 	svr_opts.noauthpass = 0;
 	svr_opts.norootpass = 0;
 	svr_opts.allowblankpass = 0;
@@ -235,11 +235,9 @@ void svr_getopts(int argc, char ** argv) {
 				case 'w':
 					svr_opts.norootlogin = 1;
 					break;
-
-                                case 'G':
-                                        next = &svr_opts.grouploginname;
-                                        break;
-
+				case 'G':
+					next = &svr_opts.restrict_group;
+					break;
 				case 'W':
 					next = &recv_window_arg;
 					break;
@@ -342,17 +340,16 @@ void svr_getopts(int argc, char ** argv) {
 		buf_setpos(svr_opts.banner, 0);
 	}
 
-        if (svr_opts.grouploginname) {
-                struct group *restrictedgroup = getgrnam(svr_opts.grouploginname);
+	if (svr_opts.restrict_group) {
+		struct group *restrictedgroup = getgrnam(svr_opts.restrict_group);
 
-                if (restrictedgroup){
-                    svr_opts.grouploginid = malloc(sizeof(gid_t));
-                    *svr_opts.grouploginid = restrictedgroup->gr_gid;
-                } else {
-                    dropbear_exit("Cannot restrict logins to group '%s' as the group does not exist", svr_opts.grouploginname);
-                }
+		if (restrictedgroup){
+			svr_opts.restrict_group_gid = restrictedgroup->gr_gid;
+		} else {
+			dropbear_exit("Cannot restrict logins to group '%s' as the group does not exist", svr_opts.restrict_group);
+		}
 
-        }
+	}
 	
 	if (recv_window_arg) {
 		opts.recv_window = atol(recv_window_arg);
diff --git a/sysoptions.h b/sysoptions.h
index ba4b4ca..3eba13b 100644
--- a/sysoptions.h
+++ b/sysoptions.h
@@ -81,6 +81,8 @@
  
 #define DROPBEAR_PASSWORD_ENV "DROPBEAR_PASSWORD"
 
+#define DROPBEAR_NGROUP_MAX 1024
+
 /* Required for pubkey auth */
 #define DROPBEAR_SIGNKEY_VERIFY ((DROPBEAR_SVR_PUBKEY_AUTH) || (DROPBEAR_CLIENT))