libwbclient: duplicate primary group in returned sids array (was github PR #102)

[Jeremy Allison]

On Tue, Jan 08, 2019 at 12:36:59PM +0200, Isaac Boukris via samba-technical wrote:
> Hi,
> 
> A while ego, I was testing wbcAuthenticateUserEx with NTLM against
> Windows and noticed that the primary group is being referenced twice
> in returned wbcAuthUserInfo.sids array.
> 
> The way it happens as far as I recall, is because the DC returns the
> primary group in NETLOGON_VALIDATION_SAM_INFO, both as PrimaryGroupId
> and in GroupIds. Then libwbclient copies it to a single array where
> array[0] is the user SID, array[1] is the PrimaryGroupId and then it
> adds the primary group again as a part of the groups from GroupIds.
> 
> This patch skips the second addition, as it seems odd to have
> duplicate SIDs in a flat array (even though the first two elements are
> fixed). If this looks right, I'll try to add a test and resubmit on
> gitlab.
> 
> Cheers!

This looks right to me, but shouldn't you also scan for and
ignore user_rid as well, so we only get the first two entries
once ? Not sure if this can happen, but just makes it impossible
anyway.

Something like (braces added for Samba coding convention :-):

 diff --git a/nsswitch/libwbclient/wbc_pam.c b/nsswitch/libwbclient/wbc_pam.c
 index cb2d5a0f4c4..ba39dcf8baf 100644
 --- a/nsswitch/libwbclient/wbc_pam.c
 +++ b/nsswitch/libwbclient/wbc_pam.c
 @@ -187,6 +187,10 @@ static wbcErr wbc_create_auth_info(const struct winbindd_response *resp,
  			BAIL_ON_WBC_ERROR(wbc_status);
  		}

 +             /* Skip uid, already added */
 +             if (rid == resp->data.auth.info3.user_rid) {
 +                     continue;
 +		}
 +
 +             /* Skip primary group, already added */
 +             if (rid == resp->data.auth.info3.group_rid) {
 +                     continue;
 +		}

> From 25572b6f111d9f9363ad497f4aeda5e0e8968908 Mon Sep 17 00:00:00 2001
> From: Isaac Boukris <iboukris at gmail.com>
> Date: Mon, 18 Sep 2017 19:16:21 +0300
> Subject: [PATCH] libwbclient: skip duplicate primary group in returned sids
>  array
> 
> When calling wbcCtxAuthenticateUser, the primary group currently
> being referenced twice in returned wbcAuthUserInfo.sids array.
> Fix this by skipping the second addition.
> 
> Signed-off-by: Isaac Boukris <iboukris at gmail.com>
> ---
>  nsswitch/libwbclient/wbc_pam.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/nsswitch/libwbclient/wbc_pam.c b/nsswitch/libwbclient/wbc_pam.c
> index cb2d5a0f4c4..ba39dcf8baf 100644
> --- a/nsswitch/libwbclient/wbc_pam.c
> +++ b/nsswitch/libwbclient/wbc_pam.c
> @@ -187,6 +187,10 @@ static wbcErr wbc_create_auth_info(const struct winbindd_response *resp,
>  			BAIL_ON_WBC_ERROR(wbc_status);
>  		}
>  
> +		/* Skip primary group, already added */
> +		if (rid == resp->data.auth.info3.group_rid)
> +			continue;
> +
>  		if (!sid_attr_compose(&i->sids[sn], &domain_sid,
>  				      rid, attrs)) {
>  			wbc_status = WBC_ERR_INVALID_SID;