auth use info3 in auth_serversupplied_info (source3/)

[Andrew Bartlett]

On Thu, 2010-05-27 at 19:49 -0400, simo wrote:
> On Fri, 2010-05-28 at 09:40 +1000, Andrew Bartlett wrote:
> > On Fri, 2010-05-28 at 01:11 +0200, Guenther Deschner wrote:
> > > On Thu, May 27, 2010 at 05:57:35PM -0500, Günther Deschner wrote:
> > > > The branch, master has been updated
> > > >        via  606be25... s3:auth Free sampass as soon as we have server_info
> > > >        via  d9cffc0... s3:auth use info3 in auth_serversupplied_info
> > > >        via  6713f3d... s3:auth add function to copy a netr_SamInfo3 structure
> > > >        via  605cfef... s3:auth: add function to convert samu to netr_SamInfo3
> > > >       from  667716d... s4-smbtorture: finally test all levels in rap_NetUserGetInfo RAP-SAM test.
> > > > 
> > > > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> > > 
> > > Hi Simo,
> > > 
> > > arg... and of course I pushed and incomplete and old version of that
> > > patchset :/
> > > 
> > > sorry, sorry, sorry.
> > 
> > I know it's only half-done, but I really like this idea!  It removes
> > some really annoying conversions two and from the struct samu
> > structure. 
> 
> Yes I wanted to remove all the pdb_* pollution in completely unrelated
> code, it was very ugly.
> 
> Also we used almost always netr_samInfo3 so we were wasting time doing
> samu -> info3 conversions in a few places.

Yeah. 

> > Also:
> > -       if (!pdb_copy_sam_account(dst->sam_account, src->sam_account)) {
> > 
> > This looks like a really, really good idea.  The use of that routine
> > (which it seems only works for users in the local domain) in general
> > code scares me...
> > 
> > However, can we please work together to define the future auth
> > structures?
> 
> Sure, although I do not plan to touch them too much (still trying to
> remove some redundancy in auth_serversupplied_info but that's it).

As I mentioned on IRC (but recorded here for a better record), please
don't assume that the 'session key' in auth_serversupplied_info is
redundant.  This value is different from that recorded in the info3
structure - it is recalculated by the NTLMSSP code when NTLM2 is used,
and is of variable length - 32 bytes when Kerberos and AES is in use.  

/* This is the final session key, as used by SMB signing, and (truncated
to 16 bytes) encryption on the SAMR and LSA pipes when over ncacn_np.
It is calculated by NTLMSSP from the session key in the info3, and is
set from the Kerberos session key using krb5_auth_con_getremotesubkey().
*/

> > While my s3compat work didn't touch the auth_serversupplied_info
> > structure, this change (which I didn't dare to contemplate) makes it
> > much more likely that we can share more parts of this structure in
> > future, and drastically  reduces the number of complex conversion the
> > auth_samba4 module needs to do.
> 
> Yes helping share code is also one of the reasons, and samu was
> absolutely in the way as it is strictly related to source3/passdb
> 
> > However, one caution I would note:
> > 
> > The pdb based code has some really funky logic in pdb_set_group_sid()
> > and pdb_get_group_sid().  I would love to see that logic die, but if
> > not, it would be good to ensure it is replicated.
> 
> the info3 structure is generated from samu, so it should retain that.

Yes and no.  The conversion two and from the samu invoked this code, and
from the comments deliberately so.  By short-cutting it, I think we have
changed semantics. 

It is a horrible violation of abstraction, and I really want it to die,
but I first want to bring it to your attention :-)

> >   Also, the struct samu
> > contained a copy of the user's 'struct passwd' as an attempt to avoid
> > lots of getpwnam() lookups.  Was that just unused, or have we lost that
> > optimisation?
> 
> Totally unused I guess, I didn't find a single place where some passwd
> struct was retrieved from sam_account.

OK, but look carefully at the 'extra' logic in the pdb_get_set.c:
pdb_get_group_sid() et al.

> On the other hand now we convert (or just copy w/o wasteful conversions
> to samu) to netr_SamInfo3 only once.

That is a good thing.

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/20100528/974dc1d7/attachment.pgp>