[PATCH] Adds support for Resource SID Compression a new Windows Server 2012 KDC feature - 3rd Version

[Andrew Bartlett]

I've been pondering this for some time, and I have come to the view that
your proposal in principal makes the most sense of the available options
given the constraints. 

On Wed, 2013-04-03 at 01:28 +0200, Markus Baier wrote:
> Am 02.04.2013 22:28, schrieb Andrew Bartlett:
> > I'm entirely uncomfortable with the idea of having a filter
> > which 'fixes' this structure.
> 
> I am too :-)
> The patch is more like an integrated compatibility mode.
> It prevents the users from changing the DisableResourceGroupsFields
> register value, if the would use samba together with a Windows Server
> 2012 KDC
> 
> > Instead, we need to be
> > patching the code where we extract SIDs from the structure.
> .....
> > What I'm saying is that we need to consolidate the duplicate code in
> > these routines, and then to fix this exactly once.
> 
> Yes, that would be the right solution.
> The problem is, all the functions in samba like sid_array_from_info3
> or create_local_nt_token_from_info3, work with the info3 structure.
> But the important data fields (used for resource sid compression)
> are not a part of the info3 structure.
> The fields are, like info3 itself, a part of the info structure:
> struct PAC_LOGON_INFO {
>         struct netr_SamInfo3 info3;
>         struct dom_sid2 *res_group_dom_sid;/* [unique] */
>         struct samr_RidWithAttributeArray res_groups;
> };
> 
> And thats the problem, after the info3 part was extracted out of
> the info structure (PAC_LOGON_INFO) the information will be lost.
> All the functions like create_local_nt_token_from_info3
> or sid_array_from_info3 only get and work with the info3 structure
> itself and so they can't access the dom_sid2 and
> samr_RidWithAttributeArray fields.
> 
> So, one solution could be to work with the info structure
> PAC_LOGON_INFO instead of the info3 structure netr_SamInfo3
> But I think this will be a mess.
> Dozens of functions had to be changed and rewritten
> starting from winbindd_dual_pam_auth down into the code
> structure and that are a lot of functions which uses
> the info3 structure.

Given all this, I wonder if you could re-work this code to comply with
our coding guidelines and make the changes needed for it to apply to GIT
master?

For the coding style issues, see README.Coding about long lines and //
comments.  However, the larger task is the one to have this work on git
master - the code here radically changed between 3.6 (which I presume
you based this on) and 4.0.  

Interestingly, I notice that the AD DC (source4) code actually handles
this correctly in make_user_info_dc_pac(), and that is may be possible
to handle this in make_session_info_krb5(), perhaps even without
changing the info3 structure!

I do apologise for sitting on this for a year - we should have had this
more extensive discussion back then, but I hope we can get this fixed up
properly once and for all. 

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba