Comments on the auth_serversupplied_info rework in Samba3

[Andrew Bartlett]

On Mon, 2010-05-31 at 21:49 -0400, simo wrote:
> On Tue, 2010-06-01 at 10:37 +1000, Andrew Bartlett wrote:
> > My biggest concern is the way that this patch uses 'magic values' that
> > are only defined in Samba.  
> > 
> > s3:auth handle unix domain sids in samu
> > http://git.samba.org/?p=idra/samba.git;a=commitdiff;h=5648836a26f39966ff3f1fa9bc5a287ea368afb6
> > 
> > +/* special bits used to identify non domain users
> > + * the may be generated in a samba server
> > + * These must not conflict with assigned values
> > + * in samr_GroupAttrs
> > + */
> > +#define SE_SAMBA_USER_SID ( 0x00100000 )
> > +#define SE_SAMBA_PGID_SID ( 0x00200000 )
> > 
> > 
> > s3:auth create nt token from info3 directly
> > http://git.samba.org/?p=idra/samba.git;a=commitdiff;h=56c1b9648324f3bd8f0b386298beaa24b859dc3c
> > 
> > +       /* USER SID */
> > +       if (info3->base.rid == (uint32_t)(-1)) {
> > +               /* this is a signal the user was fake and generated,
> > +                * the actual SID we want to use is stored in the
> > extra
> > +                * sids */
> > 
> > I would strongly prefer that we pass in a 'wrapper' or associated
> > structure that contains the info3, that has a flag of 'user rid fake'
> > etc.  We do not control these structures, and while we can fill them
> > in
> > with values that are semantically correct for their original use, I'm
> > happy, but I don't like the use of magic values.
> > 
> > This isn't just a preference - it took quite some time to understand
> > why
> > a new windows client failed against Samba4 with this same trick
> > (overloading a bitfield with Samba-only internal values) was used to
> > pass down special meanings in the Samba4 file server. 
> 
> Yes, this was the most controversial for me too.
> I will see if we can move this stuff out w/o making it a pain.
> As you can see samu_to_info3 is quite generic and does not take an
> auth_serversupplied_info() and that's the main reason the trick is done
> within info3.
> I don't think there really is a chance of spilling this out, but I
> totally understand the concern.

I think we should have it return two structures - one info3, and another
that I can also fill out in auth_samba4 (for s3compat) that describes
the exceptions.  We can put that additional structure in the common
auth.h when I get that merged, without needing to have this function
deal with much more intricate details of the auth subsystem.  

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100601/d2694093/attachment.pgp>