Patch to handle sid compression for consideration ...
Jeremy Allison
jra at samba.org
Mon Jun 16 23:36:12 MDT 2014
On Mon, Jun 16, 2014 at 09:39:21PM -0700, Richard Sharpe wrote:
>
> Saying that you feel uncomfortable about it is not a good argument and
> is very passive aggressive, IMO.
Yeah, not the best way of phrasing that I agree.
> Perhaps what I should do is return a new logon_info structure with the
> merged SIDs from the merge function, and use the original structure as
> the talloc context.
Personally I'd just use the existing copy_netr_SamInfo3()
function and make a copy just before merging the sids, and
keep the netsamlogon_cache_store() call separate.
So your change :
- netsamlogon_cache_store(ntuser, &logon_info->info3);
+ status = merge_resource_sids_and_cache(ntuser, logon_info);
Would then look something like:
struct netr_SamInfo3 *info3_copy = copy_netr_SamInfo3(ctx, &logon_info->info3);
if (info3_copy == NULL) {
// error...
}
status = merge_resource_sids(ntuser, logon_info, &info3_copy);
if (!NT_STATUS_IS_OK(status)) {
// error....
}
netsamlogon_cache_store(ntuser, &info3_copy);
TALLOC_FREE(info3_copy);
That keeps everything at the same level, and avoids
multi-use functions like merge_resource_sids_and_cache()
which are doing two functionally different things in
one function.
> In the smbd path, I will have to arrange to pass the new logon_info
> structure to the function that creates the session context
> (make_session_info_krb5).
>
> Unfortunately I cannot just return an info3 structure without also
> changing make_session_info_krb5, although the only thing that
> make_session_info_krb5 uses is the info3 structure in the logon_info,
> so a case could be made to make that change in the interests of
> reducing the unnecessary info being passed around.
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 !!
Cheers,
Jeremy.
More information about the samba-technical
mailing list