1526 should allow domain groups as member of local groups

Reviewed by: Garrett D'Amore <garrett@nexenta.com>
Reviewed by: Dan McDonald <danmcd@nexenta.com>
Reviewed by: Richard Lowe <richlowe@richlowe.net>
Approved by: Garrett D'Amore <garrett@nexenta.com>

4 files changed, 282 insertions, 90 deletions

diff --git a/usr/src/cmd/smbsrv/smbadm/smbadm.c b/usr/src/cmd/smbsrv/smbadm/smbadm.c
index df7f163599..7874a71bed 100644
--- a/usr/src/cmd/smbsrv/smbadm/smbadm.c
+++ b/usr/src/cmd/smbsrv/smbadm/smbadm.c
@@ -19,6 +19,7 @@
  * CDDL HEADER END
  */
 /*
+ * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
  */
 
@@ -58,6 +59,7 @@ typedef enum {
 	HELP_GET,
 	HELP_JOIN,
 	HELP_LIST,
+	HELP_LOOKUP,
 	HELP_RENAME,
 	HELP_SET,
 	HELP_SHOW,
@@ -70,6 +72,11 @@ typedef enum {
 #define	SMBADM_CMDF_GROUP	0x02
 #define	SMBADM_CMDF_TYPEMASK	0x0F
 
+typedef enum {
+	SMBADM_GRP_ADDMEMBER = 0,
+	SMBADM_GRP_DELMEMBER,
+} smbadm_grp_action_t;
+
 #define	SMBADM_ANSBUFSIZ	64
 
 typedef struct smbadm_cmdinfo {
@@ -96,6 +103,9 @@ static void smbadm_extract_domain(char *, char **, char **);
 
 static int smbadm_join(int, char **);
 static int smbadm_list(int, char **);
+static int smbadm_lookup(int, char **);
+static void smbadm_lookup_name(char *);
+static void smbadm_lookup_sid(char *);
 static int smbadm_group_create(int, char **);
 static int smbadm_group_delete(int, char **);
 static int smbadm_group_rename(int, char **);
@@ -105,6 +115,8 @@ static int smbadm_group_getprop(int, char **);
 static int smbadm_group_setprop(int, char **);
 static int smbadm_group_addmember(int, char **);
 static int smbadm_group_delmember(int, char **);
+static int smbadm_group_add_del_member(char *, char *, smbadm_grp_action_t);
+
 static int smbadm_user_disable(int, char **);
 static int smbadm_user_enable(int, char **);
 
@@ -126,6 +138,8 @@ static smbadm_cmdinfo_t smbadm_cmdtable[] =
 		SMBADM_CMDF_NONE,	SMBADM_VALUE_AUTH },
 	{ "list",		smbadm_list,		HELP_LIST,
 		SMBADM_CMDF_NONE,	SMBADM_BASIC_AUTH },
+	{ "lookup",		smbadm_lookup,		HELP_LOOKUP,
+		SMBADM_CMDF_NONE,	SMBADM_BASIC_AUTH },
 	{ "remove-member",	smbadm_group_delmember,	HELP_DEL_MEMBER,
 		SMBADM_CMDF_GROUP,	SMBADM_ACTION_AUTH },
 	{ "rename",		smbadm_group_rename,	HELP_RENAME,
@@ -229,6 +243,12 @@ smbadm_cmdusage(FILE *fp, smbadm_cmdinfo_t *cmd)
 		    gettext("\t\t[+] selected domain controller\n"));
 		return;
 
+	case HELP_LOOKUP:
+		(void) fprintf(fp,
+		    gettext("\t%s user-or-group-name\n"),
+		    cmd->name);
+		return;
+
 	case HELP_DEL_MEMBER:
 		(void) fprintf(fp,
 		    gettext("\t%s -m member [[-m member] ...] group\n"),
@@ -718,6 +738,70 @@ smbadm_list(int argc, char **argv)
 }
 
 /*
+ * smbadm_lookup
+ *
+ * Lookup the SID for a given account (user or group)
+ */
+static int
+smbadm_lookup(int argc, char **argv)
+{
+	int i;
+
+	if (argc < 2) {
+		(void) fprintf(stderr, gettext("missing account name\n"));
+		smbadm_usage(B_FALSE);
+	}
+
+	for (i = 1; i < argc; i++) {
+		if (strncmp(argv[i], "S-1-", 4) == 0)
+			smbadm_lookup_sid(argv[i]);
+		else
+			smbadm_lookup_name(argv[i]);
+	}
+	return (0);
+}
+
+static void
+smbadm_lookup_name(char *name)
+{
+	lsa_account_t	acct;
+	int rc;
+
+	if ((rc = smb_lookup_name(name, SidTypeUnknown, &acct)) != 0) {
+		(void) fprintf(stderr, gettext(
+		    "\t\t%s: lookup name failed, rc=%d\n"),
+		    name, rc);
+		return;
+	}
+	if (acct.a_status != NT_STATUS_SUCCESS) {
+		(void) fprintf(stderr, gettext("\t\t%s [%s]\n"),
+		    name, xlate_nt_status(acct.a_status));
+		return;
+	}
+	(void) printf("\t%s\n", acct.a_sid);
+}
+
+static void
+smbadm_lookup_sid(char *sidstr)
+{
+	lsa_account_t	acct;
+	int rc;
+
+	if ((rc = smb_lookup_sid(sidstr, &acct)) != 0) {
+		(void) fprintf(stderr, gettext(
+		    "\t\t%s: lookup SID failed, rc=%d\n"),
+		    sidstr, rc);
+		return;
+	}
+	if (acct.a_status != NT_STATUS_SUCCESS) {
+		(void) fprintf(stderr, gettext("\t\t%s [%s]\n"),
+		    sidstr, xlate_nt_status(acct.a_status));
+		return;
+	}
+	(void) printf("\t%s\\%s\n", acct.a_domain, acct.a_name);
+}
+
+/*
  * smbadm_group_create
  *
  * Creates a local SMB group
@@ -797,9 +881,9 @@ static void
 smbadm_group_show_name(const char *domain, const char *name)
 {
 	if (strchr(domain, '.') != NULL)
-		(void) printf(gettext("\t\t%s@%s\n"), name, domain);
+		(void) printf("\t\t%s@%s\n", name, domain);
 	else
-		(void) printf(gettext("\t\t%s\\%s\n"), domain, name);
+		(void) printf("\t\t%s\\%s\n", domain, name);
 }
 
 /*
@@ -1133,12 +1217,9 @@ smbadm_group_getprop(int argc, char **argv)
 static int
 smbadm_group_addmember(int argc, char **argv)
 {
-	lsa_account_t	acct;
 	char *gname = NULL;
 	char **mname;
 	char option;
-	smb_gsid_t msid;
-	int status;
 	int mcnt = 0;
 	int ret = 0;
 	int i;
@@ -1176,39 +1257,11 @@ smbadm_group_addmember(int argc, char **argv)
 		smbadm_usage(B_FALSE);
 	}
 
-
 	for (i = 0; i < mcnt; i++) {
-		ret = 0;
 		if (mname[i] == NULL)
 			continue;
-
-		ret = smb_lookup_name(mname[i], SidTypeUnknown, &acct);
-		if ((ret != 0) || (acct.a_status != NT_STATUS_SUCCESS)) {
-			(void) fprintf(stderr,
-			    gettext("failed to add %s: unable to obtain SID\n"),
-			    mname[i]);
-			continue;
-		}
-
-		msid.gs_type = acct.a_sidtype;
-
-		if ((msid.gs_sid = smb_sid_fromstr(acct.a_sid)) == NULL) {
-			(void) fprintf(stderr,
-			    gettext("failed to add %s: no memory\n"), mname[i]);
-			continue;
-		}
-
-		status = smb_lgrp_add_member(gname, msid.gs_sid, msid.gs_type);
-		smb_sid_free(msid.gs_sid);
-		if (status != SMB_LGRP_SUCCESS) {
-			(void) fprintf(stderr,
-			    gettext("failed to add %s (%s)\n"),
-			    mname[i], smb_lgrp_strerror(status));
-			ret = 1;
-		} else {
-			(void) printf(gettext("'%s' is now a member of '%s'\n"),
-			    mname[i], gname);
-		}
+		ret |= smbadm_group_add_del_member(
+		    gname, mname[i], SMBADM_GRP_ADDMEMBER);
 	}
 
 	free(mname);
@@ -1221,12 +1274,9 @@ smbadm_group_addmember(int argc, char **argv)
 static int
 smbadm_group_delmember(int argc, char **argv)
 {
-	lsa_account_t	acct;
 	char *gname = NULL;
 	char **mname;
 	char option;
-	smb_gsid_t msid;
-	int status;
 	int mcnt = 0;
 	int ret = 0;
 	int i;
@@ -1268,40 +1318,77 @@ smbadm_group_delmember(int argc, char **argv)
 		ret = 0;
 		if (mname[i] == NULL)
 			continue;
+		ret |= smbadm_group_add_del_member(
+		    gname, mname[i], SMBADM_GRP_DELMEMBER);
+	}
 
-		ret = smb_lookup_name(mname[i], SidTypeUnknown, &acct);
-		if ((ret != 0) || (acct.a_status != NT_STATUS_SUCCESS)) {
-			(void) fprintf(stderr,
-			    gettext("failed to remove %s: "
-			    "unable to obtain SID\n"),
-			    mname[i]);
-			continue;
-		}
+	free(mname);
+	return (ret);
+}
 
-		msid.gs_type = acct.a_sidtype;
+static int
+smbadm_group_add_del_member(char *gname, char *mname,
+	smbadm_grp_action_t act)
+{
+	lsa_account_t	acct;
+	smb_gsid_t msid;
+	char *sidstr;
+	char *act_str;
+	int rc;
 
-		if ((msid.gs_sid = smb_sid_fromstr(acct.a_sid)) == NULL) {
+	if (strncmp(mname, "S-1-", 4) == 0) {
+		/*
+		 * We are given a SID.  Just use it.
+		 *
+		 * We'e like the real account type if we can get it,
+		 * but don't want to error out if we can't get it.
+		 */
+		sidstr = mname;
+		rc = smb_lookup_sid(sidstr, &acct);
+		if ((rc != 0) || (acct.a_status != NT_STATUS_SUCCESS))
+			acct.a_sidtype = SidTypeUnknown;
+	} else {
+		rc = smb_lookup_name(mname, SidTypeUnknown, &acct);
+		if ((rc != 0) || (acct.a_status != NT_STATUS_SUCCESS)) {
 			(void) fprintf(stderr,
-			    gettext("failed to remove %s: no memory\n"),
-			    mname[i]);
-			continue;
+			    gettext("%s: name lookup failed\n"), mname);
+			return (1);
 		}
+		sidstr = acct.a_sid;
+	}
 
-		status = smb_lgrp_del_member(gname, msid.gs_sid, msid.gs_type);
-		smb_sid_free(msid.gs_sid);
-		if (status != SMB_LGRP_SUCCESS) {
-			(void) fprintf(stderr,
-			    gettext("failed to remove %s (%s)\n"),
-			    mname[i], smb_lgrp_strerror(status));
-			ret = 1;
-		} else {
-			(void) printf(
-			    gettext("'%s' has been removed from %s\n"),
-			    mname[i], gname);
-		}
+	msid.gs_type = acct.a_sidtype;
+	if ((msid.gs_sid = smb_sid_fromstr(sidstr)) == NULL) {
+		(void) fprintf(stderr,
+		    gettext("%s: no memory for SID\n"), sidstr);
+		return (1);
 	}
 
-	return (ret);
+	switch (act) {
+	case SMBADM_GRP_ADDMEMBER:
+		act_str = gettext("add");
+		rc = smb_lgrp_add_member(gname,
+		    msid.gs_sid, msid.gs_type);
+		break;
+	case SMBADM_GRP_DELMEMBER:
+		act_str = gettext("remove");
+		rc = smb_lgrp_del_member(gname,
+		    msid.gs_sid, msid.gs_type);
+		break;
+	default:
+		rc = SMB_LGRP_INTERNAL_ERROR;
+		break;
+	}
+
+	smb_sid_free(msid.gs_sid);
+
+	if (rc != SMB_LGRP_SUCCESS) {
+		(void) fprintf(stderr,
+		    gettext("failed to %s %s (%s)\n"),
+		    act_str, mname, smb_lgrp_strerror(rc));
+		return (1);
+	}
+	return (0);
 }
 
 static int
diff --git a/usr/src/lib/smbsrv/libsmb/common/smb_lgrp.c b/usr/src/lib/smbsrv/libsmb/common/smb_lgrp.c
index ea51a7e369..8435a20c32 100644
--- a/usr/src/lib/smbsrv/libsmb/common/smb_lgrp.c
+++ b/usr/src/lib/smbsrv/libsmb/common/smb_lgrp.c
@@ -20,6 +20,7 @@
  */
 
 /*
+ * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
  * Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
  */
 
@@ -1033,14 +1034,25 @@ smb_lgrp_err_to_ntstatus(uint32_t lgrp_err)
  * smb_lgrp_chkmember
  *
  * Determines valid account types for being member of
- * a local group.
- *
- * Currently, we just support users as valid members.
+ * a local group.  We really have no business trying to
+ * keep track of the "type" of SIDs in a group, so just
+ * validate that the SID type is a known enum value.
  */
 static boolean_t
 smb_lgrp_chkmember(uint16_t sid_type)
 {
-	return (sid_type == SidTypeUser);
+	switch (sid_type) {
+	case SidTypeNull:
+	case SidTypeUser:
+	case SidTypeGroup:
+	case SidTypeAlias:
+	case SidTypeWellKnownGroup:
+	case SidTypeDeletedAccount:
+	case SidTypeInvalid:
+	case SidTypeUnknown:
+		return (B_TRUE);
+	}
+	return (B_FALSE);
 }
 
 /*
diff --git a/usr/src/lib/smbsrv/libsmb/common/smb_sam.c b/usr/src/lib/smbsrv/libsmb/common/smb_sam.c
index 21be70116b..7016f5a878 100644
--- a/usr/src/lib/smbsrv/libsmb/common/smb_sam.c
+++ b/usr/src/lib/smbsrv/libsmb/common/smb_sam.c
@@ -19,6 +19,7 @@
  * CDDL HEADER END
  */
 /*
+ * Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
  * Copyright 2010 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
@@ -299,24 +300,45 @@ smb_sam_usr_cnt(void)
 }
 
 /*
- * Returns a list of local groups which the given user is
- * their member. A pointer to an array of smb_ids_t
- * structure is returned which must be freed by caller.
+ * Updates a list of groups in which the given user is a member
+ * by adding any local (SAM) groups.
+ *
+ * We are a member of local groups where the local group
+ * contains either the user's primary SID, or any of their
+ * other SIDs such as from domain groups, SID history, etc.
+ * We can have indirect membership via domain groups.
  */
 uint32_t
 smb_sam_usr_groups(smb_sid_t *user_sid, smb_ids_t *gids)
 {
-	smb_id_t *ids;
+	smb_ids_t new_gids;
+	smb_id_t *ids, *new_ids;
 	smb_giter_t gi;
 	smb_group_t lgrp;
-	int total_cnt, gcnt;
+	int i, gcnt, total_cnt;
+	uint32_t ret;
+	boolean_t member;
 
+	/*
+	 * First pass: count groups to be added (gcnt)
+	 */
 	gcnt = 0;
 	if (smb_lgrp_iteropen(&gi) != SMB_LGRP_SUCCESS)
 		return (NT_STATUS_INTERNAL_ERROR);
 
 	while (smb_lgrp_iterate(&gi, &lgrp) == SMB_LGRP_SUCCESS) {
+		member = B_FALSE;
 		if (smb_lgrp_is_member(&lgrp, user_sid))
+			member = B_TRUE;
+		else for (i = 0, ids = gids->i_ids;
+		    i < gids->i_cnt; i++, ids++) {
+			if (smb_lgrp_is_member(&lgrp, ids->i_sid)) {
+				member = B_TRUE;
+				break;
+			}
+		}
+		/* Careful: only count lgrp once */
+		if (member)
 			gcnt++;
 		smb_lgrp_free(&lgrp);
 	}
@@ -325,35 +347,86 @@ smb_sam_usr_groups(smb_sid_t *user_sid, smb_ids_t *gids)
 	if (gcnt == 0)
 		return (NT_STATUS_SUCCESS);
 
-	total_cnt = gids->i_cnt + gcnt;
-	gids->i_ids = realloc(gids->i_ids, total_cnt * sizeof (smb_id_t));
-	if (gids->i_ids == NULL)
-		return (NT_STATUS_NO_MEMORY);
-
+	/*
+	 * Second pass: add to groups list.
+	 * Do not modify gcnt after here.
+	 */
 	if (smb_lgrp_iteropen(&gi) != SMB_LGRP_SUCCESS)
 		return (NT_STATUS_INTERNAL_ERROR);
 
-	ids = gids->i_ids + gids->i_cnt;
+	/*
+	 * Expand the list (copy to a new, larger one)
+	 * Note: were're copying pointers from the old
+	 * array to the new (larger) array, and then
+	 * adding new pointers after what we copied.
+	 */
+	ret = 0;
+	new_gids.i_cnt = gids->i_cnt;
+	total_cnt = gids->i_cnt + gcnt;
+	new_gids.i_ids = malloc(total_cnt * sizeof (smb_id_t));
+	if (new_gids.i_ids == NULL) {
+		ret = NT_STATUS_NO_MEMORY;
+		goto out;
+	}
+	(void) memcpy(new_gids.i_ids, gids->i_ids,
+	    gids->i_cnt * sizeof (smb_id_t));
+	new_ids = new_gids.i_ids + gids->i_cnt;
+	(void) memset(new_ids, 0, gcnt * sizeof (smb_id_t));
+
+	/*
+	 * Add group SIDs starting at the end of the
+	 * previous list.  (new_ids)
+	 */
 	while (smb_lgrp_iterate(&gi, &lgrp) == SMB_LGRP_SUCCESS) {
-		if (gcnt == 0) {
-			smb_lgrp_free(&lgrp);
-			break;
+		member = B_FALSE;
+		if (smb_lgrp_is_member(&lgrp, user_sid))
+			member = B_TRUE;
+		else for (i = 0, ids = gids->i_ids;
+		    i < gids->i_cnt; i++, ids++) {
+			if (smb_lgrp_is_member(&lgrp, ids->i_sid)) {
+				member = B_TRUE;
+				break;
+			}
 		}
-		if (smb_lgrp_is_member(&lgrp, user_sid)) {
-			ids->i_sid = smb_sid_dup(lgrp.sg_id.gs_sid);
-			if (ids->i_sid == NULL) {
+		if (member && (new_gids.i_cnt < (gids->i_cnt + gcnt))) {
+			new_ids->i_sid = smb_sid_dup(lgrp.sg_id.gs_sid);
+			if (new_ids->i_sid == NULL) {
 				smb_lgrp_free(&lgrp);
-				return (NT_STATUS_NO_MEMORY);
+				ret = NT_STATUS_NO_MEMORY;
+				goto out;
 			}
-			ids->i_attrs = lgrp.sg_attr;
-			gids->i_cnt++;
-			gcnt--;
-			ids++;
+			new_ids->i_attrs = lgrp.sg_attr;
+			new_ids++;
+			new_gids.i_cnt++;
 		}
 		smb_lgrp_free(&lgrp);
 	}
+
+out:
 	smb_lgrp_iterclose(&gi);
 
+	if (ret != 0) {
+		if (new_gids.i_ids != NULL) {
+			/*
+			 * Free only the new sids we added.
+			 * The old ones were copied ptrs.
+			 */
+			ids = new_gids.i_ids + gids->i_cnt;
+			for (i = 0; i < gcnt; i++, ids++) {
+				smb_sid_free(ids->i_sid);
+			}
+			free(new_gids.i_ids);
+		}
+		return (ret);
+	}
+
+	/*
+	 * Success! Update passed gids and
+	 * free the old array.
+	 */
+	free(gids->i_ids);
+	*gids = new_gids;
+
 	return (NT_STATUS_SUCCESS);
 }
 
diff --git a/usr/src/man/man1m/smbadm.1m b/usr/src/man/man1m/smbadm.1m
index bd7ec4f262..b1e6e2de98 100644
--- a/usr/src/man/man1m/smbadm.1m
+++ b/usr/src/man/man1m/smbadm.1m
@@ -1,4 +1,5 @@
 '\" te
+.\" Copyright 2011 Nexenta Systems, Inc.  All rights reserved.
 .\" Copyright (c) 2009, Sun Microsystems, Inc. All Rights Reserved.
 .\" The contents of this file are subject to the terms of the Common Development and Distribution License (the "License").  You may not use this file except in compliance with the License.
 .\" You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE or http://www.opensolaris.org/os/licensing.  See the License for the specific language governing permissions and limitations under the License.
@@ -55,6 +56,11 @@ membership
 
 .LP
 .nf
+\fBsmbadm lookup\fR \fIaccount-name\fR [\fIaccount-name\fR [\&.\|.\|.]]
+.fi
+
+.LP
+.nf
 \fBsmbadm remove-member\fR -m \fImember\fR [[-m \fImember\fR] \&.\|.\|.] \fIgroup\fR
 .fi
 
@@ -398,6 +404,20 @@ Selected domain controller
 .sp
 .ne 2
 .na
+\fB\fBlookup\fR\fR \fIaccount-name\fR [\fIaccount-name\fR [\&.\|.\|.]]
+
+.ad
+.sp .6
+.RS 4n
+Lookup the SID for the given \fIaccount-name\fR, or lookup the
+\fIaccount-name\fR for the given SID.  This sub-command is
+primarily for diagnostic use, to confirm whether the server
+can lookup domain accounts and/or SIDs.
+.RE
+
+.sp
+.ne 2
+.na
 \fB\fBremove-member\fR -m \fImember\fR [[-m \fImember\fR] \&.\|.\|.]
 \fIgroup\fR\fR
 .ad