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

Ralph Böhme slow at samba.org
Tue Mar 28 12:18:40 UTC 2017


Hi metze,

On Tue, Mar 28, 2017 at 02:11:20PM +0200, Stefan Metzmacher wrote:
> >> The fix finds the domain of the SID by resolving a SID with same domain
> >> component and an RID of 513 (domain users), which hopefully never gets
> >> migrated.
> 
> I think we should better try to resolve the domain sid, instead
> of relying on RID 513.

yup.

> >> We've discussed other means such as smb.conf stuff or netsamlogon - I
> >> think those methods can come on top of this method, because if they
> >> don't work we should always fall back to something. The added resolving
> >> doesn't cost much because it's in the same round-trip.
> >>
> >> The key thing about this fix is that doesn't try to translate sid->xid
> >> in any possible case (such as when old domain is gone and forgotten), it
> >> just avoids getting the *wrong* result. As such, it's a good minimal fix
> >> that can be applied to stable versions. For master, we can add the
> >> smb.conf-based stuff, that will support more cases.
> >>
> >> Review appreciated.
> >> Thanks,
> >> 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.
> 
> I think this is related...
> 
> I'm wondering if your fixes would also fix Uri's problem.

not quite. Uri is fixing something that is cause by lookupsid() returning a
name.

I'm trying to address problems that arise if the lookupsid() fais in the first
place. So my patches won't fix what Uri is trying to address at all.

> At least we should carefully think about this and have one
> combined and tested patchset.

Tests would be nice. I'll add tests to my patches later on. Would it be possible
to have tests for the sid-history case as well? Please, please, please.

Cheerio!
-slow



More information about the samba-technical mailing list