Comments on the auth_serversupplied_info rework in Samba3

[simo]

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.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>