libcli/auth/ntlmssp Be clear about talloc parents for session keys

[Andrew Bartlett]

On Thu, 2010-09-16 at 18:22 -0700, Jeremy Allison wrote:
> On Fri, Sep 17, 2010 at 08:01:04AM +1000, Andrew Bartlett wrote:
> > On Thu, 2010-09-16 at 12:00 -0700, Jeremy Allison wrote:
> > > On Thu, Sep 16, 2010 at 01:08:03PM +0200, Andrew Tridgell wrote:

> > > > commit 6832d5e9334f93d2b41fa50580379a2381311748
> > > > Author: Andrew Bartlett <abartlet at samba.org>
> > > > Date:   Thu Sep 16 14:37:20 2010 +1000
> > > > 
> > > >     libcli/auth/ntlmssp Be clear about talloc parents for session keys
> > > >     
> > > >     The previous API was not clear as to who owned the returned session key.
> > > >     This fixes a valgrind-found use-after-free in the NTLMSSP key derivation code,
> > > >     and avoids making allocations - we steal and zero instead.
> > > >     
> > > >     Andrew Bartlett
> > > >     
> > > >     Signed-off-by: Andrew Tridgell <tridge at samba.org>
> > > 
> > > Andrew(s) - this looks like it would be neccessary for 3.6.0
> > > but the cherry-pick doesn't apply. Can you please review and
> > > back port for the next production release, thanks !
> > 
> > I've looked into it, and it seems not to be required (as long as
> > auth_ntlmssp_state has the same lifetime as ntlmssp_state, which as far
> > as I can tell it does), but for clarity I'll move the allocations in
> > auth_ntlmssp_check_password() across.  I won't do the full change to
> > steal etc, because the code is a different structure here (not the new
> > common code). 
> > 
> > See this proposed patch, which passes 'make test' (I've not checked with
> > vagrind however).  
> > 
> > The original valgrind error showed up in a run Tridge did of source4 -
> > there we have 'session key' tests that check all the NTLMSSP flag
> > codepaths - but as this is (in master) common code I needed to tidy up
> > all the callers. 
> 
> Thanks a lot for this. I'll review once I'm out of "slide development hell"
> for the SNIA conf :-).

After GD merged some more of the NTLMSSP code into 3.6, I'm concerned
this may no longer be correct.  If that's the approach we want to take
then think we may need to finish merging the NTLMSSP code and then apply
6832d5e9334f93d2b41fa50580379a2381311748 (the commit that started this
discussion). 

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/20100921/dff13148/attachment.pgp>