Patch to handle sid compression for consideration ...
Simo
simo at samba.org
Tue Jun 17 08:30:09 MDT 2014
On Mon, 2014-06-16 at 23:37 -0700, Jeremy Allison wrote:
> On Mon, Jun 16, 2014 at 10:36:12PM -0700, Jeremy Allison wrote:
> >
> > Yep - in that case IMHO the first patch in the series
> > should be to change the make_session_info_krb5() interface
> > to use a passed in struct netr_SamInfo3 *, as you noticed
> > it doesn't use the full struct PAC_LOGON_INFO * at all.
> >
> > In fact - I think:
> >
> > Patch #1 should be to change:
> >
> > make_server_info_info3() to use a const struct netr_SamInfo3 *info3,
> > as it only reads from info3, never modifies.
> >
> > Patch #2 should be to change make_session_info_krb5() to
> > to take a const struct netr_SamInfo3 *info3, not the
> > full struct PAC_LOGON_INFO *.
> >
> > Patch #3 changes auth3_generate_session_info_pac() to
> > make a info3 copy from logon_info->info3, and then
> > does the merge resource sids, caches the info3 copy
> > using netsamlogon_cache_store(), and then simply passes
> > that copied info3 as a const pointer to make_session_info_krb5()
> > in place of the existing struct PAC_LOGON_INFO *logon_info pointer.
> >
> > Patch #4 changes winbindd_pam_auth_pac_send() to
> > make a info3 copy from logon_info->info3, and
> > calls the netsamlogon_cache_store() with the copy.
> >
> > Does that work as a patchset for you ?
> >
> > I can make those changes for you to test
> > later on this week if I get the chance
> > whilst I'm at the plugfest, or I'm happy
> > to review these changes if you want to
> > code 'em up !!
>
> Ok, couldn't get to sleep :-). Here is the patchset.
> Ended up being 5 patches.
>
> Compiles but isn't tested. If you can test it
> that would help greatly !
>
> Simo & Andrew, does this address your concerns ?
> Rchard, does it do what you need ?
I like your plan very much in general.
However I see that you repeat the same 3 calls in an "if (logon_info)"
wrapped code block in 2 places. That calls for a utility function
instead, or we risk someone changing it in one place but not another in
the future. I would like something that does
make_info3_from_pac() or similar that takes PAC_LOGON_INFO and returns
netr_SamInfo3.
So then the common code becomes:
if (logon_info) {
status = make_info3_from_pac(tmp_ctx, logon_info, &info3);
if (!NT_STATUS_IS_OK(status)) ...
netsamlogon_cache_store(ntuser, info3);
}
Alternatively:
If (logon_info) {
status = netsamlogon_cache_from_pac(logon_info);
if (!NT_STATUS_IS_OK(status)) ...
}
And just put all in this function.
Besides that it looks the right way to go.
Simo.
More information about the samba-technical
mailing list