[PATCHES] winbindd: fix sid->xid for SID History SIDs

[Uri Simchoni]

On 03/28/2017 01:54 PM, Ralph Böhme wrote:
> Hi Uri,
> mostly lgtm, just one issue, see below.
> 
> Fwiw, I'm currently working on another issue in sids2xids. Not really related
> but I'm mentioning it here as you're currently having fun with the same area of
> code.
> 
Yeah, I figured others are working there, with recent patches by Metze
and Volker. Another reason for me for trying to keep it small (beside
the backporting).

Another thing in TODO list is offline operation, which involves *not*
resolving anything initially, if we're certain we know the id-mapping
backend (and then resolving only if the backend requests us to do so).
Are you guys working on anything similar?

> 
> Imho we should just fail early if a SID can't be resolved and refuse to provide
> a UNIX id mapping, I have a WIP branch that does this here:
> 

That's totally fine by me - I have no experience with autorid, and
figured its ability to map anything is a feature, not a bug, so I wanted
to keep it that way. Changing that is easy, I'll submit a V2.

>>  
>> +static const char *
>> +wb_get_idmap_backend_name(const struct lsa_RefDomainList *domains,
>> +			  const struct lsa_TransNameArray *names,
>> +			  const struct dom_sid *sids,
>> +			  uint32_t idx)
>> +{
>> +	struct dom_sid sid = sids[idx];
>> +	struct lsa_TranslatedName *n = &names->names[idx];
>> +	struct lsa_DomainInfo *info = NULL;
>> +	if (!is_domain_principal_sid(&sid)) {
>> +		/* "special" SID */
>> +		goto ret;
>> +	}
>> +
>> +	if (sid_check_is_in_our_sam(&sid)) {
>> +		/* our domain - the lookup is always
>> +		 * correct for ID mapping.
>> +		 */
>> +		goto ret;
>> +	}
>> +
>> +	sid_split_rid(&sid, NULL);
>> +	sid_append_rid(&sid, DOMAIN_RID_USERS);
>> +
>> +	for (idx = 0; idx < names->count; ++idx) {
>> +		if (!dom_sid_equal(&sid, &sids[idx])) {
>> +			continue;
>> +		}
>> +
>> +		/* has the DC been able to resolve
>> +		 * "domain users" for same domain?
>> +		 */
>> +		n = &names->names[idx];
>> +		if (lsa_SidType_to_id_type(n->sid_type) ==
>> +		    ID_TYPE_NOT_SPECIFIED) {
>> +			/* default id-mapping backend */
>> +			return "";
>> +		}
>> +
>> +		goto ret;
>> +	}
> 
> doesn't this add O(n^2) overhead for finding the domain SID? When mapping a
> token with a large number of groups this may be noticable.
> 
> Maybe add_sid_to_array_unique() should be add_sid_to_array_front_unique().
> 

I was thinking of having add_sid_to_array_back_unique(), because the
code assumes that if a SID is added, it's added to the back. I didn't
consider the O^2.

I suppose I can make a pass and construct an array with just the -513
records (hopefully just one or two of them), and then use that.

> Cheerio!
> -slow
> 

V2 to follow...

Thanks,
Uri.