Patch to handle sid compression for consideration ...

Richard Sharpe realrichardsharpe at
Mon Jun 16 22:39:21 MDT 2014

On Mon, Jun 16, 2014 at 6:35 PM, Andrew Bartlett <abartlet at> wrote:
> On Mon, 2014-06-16 at 08:37 -0700, Richard Sharpe wrote:
>> On Mon, Jun 16, 2014 at 7:56 AM, Simo <simo at> wrote:
>> > On Mon, 2014-06-16 at 07:45 -0700, Richard Sharpe wrote:
>> >> Hi folks,
>> >>
>> >> Attached is a cleaned up patch to handle sid compression in master.
>> >>
>> >> I have implemented Volker's suggestion to not mess with SID internals
>> >> and use compose_sid instead.
>> >>
>> >> I have also moved things around a bit to fit better into the existing
>> >> code and not expose too many internal functions.
>> >>
>> >> Finally, I added a call to winbindd_pam where needed.
>> >>
>> >> This is a first pass at handling SID compression, and if we find we
>> >> need to handle resource SIDs differently depending on which domain
>> >> they are in, then the function merge_resource_sids_and_cache can
>> >> absorb that code.
>> >>
>> >> Review please, and if acceptable, please push.
>> >
>> > Wouldn't it be better to copy info3 first and then modify the copy ?
>> Sure, I could do that. What advantage do you see? We actually need the
>> modified info3 later when we create the token, at least in one path.
> Richard,
> A number of us are uncomfortable with this being modified in-place.
> Constant input data should stay constant.

Saying that you feel uncomfortable about it is not a good argument and
is very passive aggressive, IMO.

A better argument is that in the future we may, in fact, need access
to the original structure, and an in-place modification makes that
hard. It also makes it harder for newbies to understand what is

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.

In the winbindd path we can simply discard the return value and the
memory will be cleaned up when the original talloc context is

In the smbd path, I will have to arrange to pass the new logon_info
structure to the function that creates the session context

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.

> Please make the copy.
> Andrew Bartlett
> --
> Andrew Bartlett
> Authentication Developer, Samba Team
> Samba Developer, Catalyst IT

Richard Sharpe

More information about the samba-technical mailing list