krb5pac.idl: introduce PAC_DOMAIN_GROUP_MEMBERSHIP to handle the reso…

…urce groups

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Thu Jun 30 07:16:45 CEST 2016 on sn-devel-144

auth/auth_sam_reply.c

@@ -512,6 +512,10 @@ NTSTATUS make_user_info_dc_pac(TALLOC_CTX *mem_ctx,
 	NTSTATUS nt_status;
 	union netr_Validation validation;
 	struct auth_user_info_dc *user_info_dc;
+	const struct PAC_DOMAIN_GROUP_MEMBERSHIP *rg = NULL;
+	size_t sidcount;
+
+	rg = &pac_logon_info->resource_groups;
 
 	validation.sam3 = discard_const_p(struct netr_SamInfo3, &pac_logon_info->info3);
 
@@ -522,11 +526,19 @@ NTSTATUS make_user_info_dc_pac(TALLOC_CTX *mem_ctx,
 		return nt_status;
 	}
 
-	if (pac_logon_info->res_groups.count > 0) {
-		size_t sidcount;
+	if (pac_logon_info->info3.base.user_flags & NETLOGON_RESOURCE_GROUPS) {
+		rg = &pac_logon_info->resource_groups;
+	}
+
+	if (rg == NULL) {
+		*_user_info_dc = user_info_dc;
+		return NT_STATUS_OK;
+	}
+
+	if (rg->groups.count > 0) {
 		/* The IDL layer would be a better place to check this, but to
 		 * guard the integer addition below, we double-check */
-		if (pac_logon_info->res_groups.count > 65535) {
+		if (rg->groups.count > 65535) {
 			talloc_free(user_info_dc);
 			return NT_STATUS_INVALID_PARAMETER;
 		}
@@ -536,23 +548,27 @@ NTSTATUS make_user_info_dc_pac(TALLOC_CTX *mem_ctx,
 		  trusted domains, and verify that the SID
 		  matches.
 		*/
-		if (!pac_logon_info->res_group_dom_sid) {
+		if (rg->domain_sid == NULL) {
+			talloc_free(user_info_dc);
 			DEBUG(0, ("Cannot operate on a PAC without a resource domain SID"));
 			return NT_STATUS_INVALID_PARAMETER;
 		}
 
-		sidcount = user_info_dc->num_sids + pac_logon_info->res_groups.count;
+		sidcount = user_info_dc->num_sids + rg->groups.count;
 		user_info_dc->sids
 			= talloc_realloc(user_info_dc, user_info_dc->sids, struct dom_sid, sidcount);
 		if (user_info_dc->sids == NULL) {
 			TALLOC_FREE(user_info_dc);
 			return NT_STATUS_NO_MEMORY;
 		}
 
-		for (i = 0; pac_logon_info->res_group_dom_sid && i < pac_logon_info->res_groups.count; i++) {
-			user_info_dc->sids[user_info_dc->num_sids] = *pac_logon_info->res_group_dom_sid;
-			if (!sid_append_rid(&user_info_dc->sids[user_info_dc->num_sids],
-					    pac_logon_info->res_groups.rids[i].rid)) {
+		for (i = 0; i < rg->groups.count; i++) {
+			bool ok;
+
+			user_info_dc->sids[user_info_dc->num_sids] = *rg->domain_sid;
+			ok = sid_append_rid(&user_info_dc->sids[user_info_dc->num_sids],
+					    rg->groups.rids[i].rid);
+			if (!ok) {
 				return NT_STATUS_INVALID_PARAMETER;
 			}
 			user_info_dc->num_sids++;

librpc/idl/krb5pac.idl

@@ -25,10 +25,14 @@ interface krb5pac
 		[flag(NDR_REMAINING)] DATA_BLOB signature;
 	} PAC_SIGNATURE_DATA;
 
+	typedef struct {
+		dom_sid2 *domain_sid;
+		samr_RidWithAttributeArray groups;
+	} PAC_DOMAIN_GROUP_MEMBERSHIP;
+
 	typedef struct {
 		netr_SamInfo3 info3;
-		dom_sid2 *res_group_dom_sid;
-		samr_RidWithAttributeArray res_groups;
+		PAC_DOMAIN_GROUP_MEMBERSHIP resource_groups;
 	} PAC_LOGON_INFO;
 
 	typedef [bitmap32bit] bitmap {

source3/auth/server_info.c

@@ -261,11 +261,29 @@ static NTSTATUS merge_resource_sids(const struct PAC_LOGON_INFO *logon_info,
 				struct netr_SamInfo3 *info3)
 {
 	uint32_t i = 0;
+	const struct PAC_DOMAIN_GROUP_MEMBERSHIP *rg = NULL;
 
-	if (!(logon_info->info3.base.user_flags & NETLOGON_RESOURCE_GROUPS)) {
+	if (logon_info->info3.base.user_flags & NETLOGON_RESOURCE_GROUPS) {
+		rg = &logon_info->resource_groups;
+	}
+
+	if (rg == NULL) {
 		return NT_STATUS_OK;
 	}
 
+	if (rg->domain_sid == NULL) {
+		DEBUG(10, ("Missing Resource Group Domain SID\n"));
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+
+	/* The IDL layer would be a better place to check this, but to
+	 * guard the integer addition below, we double-check */
+	if (rg->groups.count > 65535) {
+		DEBUG(10, ("Too much Resource Group RIDs %u\n",
+			  (unsigned)rg->groups.count));
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+
 	/*
 	 * If there are any resource groups (SID Compression) add
 	 * them to the extra sids portion of the info3 in the PAC.
@@ -278,14 +296,14 @@ static NTSTATUS merge_resource_sids(const struct PAC_LOGON_INFO *logon_info,
 	 * Construct a SID for each RID in the list and then append it
 	 * to the info3.
 	 */
-	for (i = 0; i < logon_info->res_groups.count; i++) {
+	for (i = 0; i < rg->groups.count; i++) {
 		NTSTATUS status;
 		struct dom_sid new_sid;
-		uint32_t attributes = logon_info->res_groups.rids[i].attributes;
+		uint32_t attributes = rg->groups.rids[i].attributes;
 
 		sid_compose(&new_sid,
-			logon_info->res_group_dom_sid,
-			logon_info->res_groups.rids[i].rid);
+			    rg->domain_sid,
+			    rg->groups.rids[i].rid);
 
 		DEBUG(10, ("Adding SID %s to extra SIDS\n",
 			sid_string_dbg(&new_sid)));