Efficient AD group matching via the new wbclient interface

[Isaac Boukris]

On Tue, Feb 13, 2018 at 6:27 PM, Isaac Boukris <iboukris at gmail.com> wrote:
> On Tue, Feb 13, 2018 at 4:49 PM, Matthew Newton <mcn at freeradius.org> wrote:
>> On Tue, 2018-02-13 at 09:07 -0500, Alan DeKok wrote:
>>> On Feb 10, 2018, at 10:22 AM, Isaac Boukris <iboukris at gmail.com>
>>> wrote:
>>> > I am working on improving AD group matching for mschap
>>> > authentication,
>>> > taking advantage of the new wbclient direct interface which returns
>>> > the user's token (including group membership SIDs) as part of NTLM
>>> > authentication.
>>> >
>>> > Work in progress:
>>> > https://github.com/frenche/freeradius-server/commit/9af7dfd634a251f
>>> > 68b07064603ccbbca308492bf
>>>
>>>   It looks good.
>>
>> Yes, it's good to use the SIDs that are returned from
>> wbcCtxAuthenticateUserEx as you get them for free, rather than having
>> to query winbindd for them again.
>>
>> I'm not sure why you're creating AD-Group-SID in the outer - just
>> create it in the current virtual server lists, and let the admin copy
>> it to the outer if they need to using unlang.
>
>
> Yea, I think the reason was I didn't manage to do it in unlang ;-)
> I'll re-look into this.
>
>>> > @mcnewton, I noticed at last there is a similar group-compare
>>> > function
>>> > in v4 branch, though I think the two actually can complete each
>>> > other.
>>>
>>>   I'd like Matthew's comments, too.
>>
>> I'm not sure if the same caveat applies to this as to using
>> wbcCtxGetGroups; probably worth checking this reply:
>>
>> https://lists.samba.org/archive/samba-technical/2016-November/116996.ht
>> ml
>>
>> I'd imagine it is the same.
>>
>> The good thing about your method is that you can /only/ use it after
>> authentication, so that forces the data to be recent, at least.
>
>
> I'm not sure what caveat are you worried about, the SIDs are returned
> from the DC in the RPC call (NETLOGON_VALIDATION).
> To my understanding that's what windows use when login-in a user and
> applying permissions.
>
>> It also really needs to compare group names, and make SIDs the
>> secondary comparison method. Names are nice and easy for admins, SIDs
>> are things that should generally stay hidden.


Also, I think my assumption was that it would be cheaper to resolve
policy group names to SIDs and compare SIDs, rather than resolve all
user's SIDs to names and compare names.
This is likely to be true for when the number of groups in the policy
is small, but even with a large number I'd guess the caching will be
more effective (over when resolving each of user's SIDs).

BTW, I've read some more on the samba thread and I think I've done it
similar to Metse's first suggestion:
https://lists.samba.org/archive/samba-technical/2016-November/116997.html

Regards.