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

[Jeremy Allison]

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:
> > > The branch, master has been updated
> > >        via  b04b8b5 wbclient: gr_mem can be NULL
> > >        via  a163284 wbclient: paranoid check for double free
> > >        via  ff515ff tdb: added TDB_NO_FSYNC env variable
> > >        via  a394a81 torture/raw Allow one more 'not implemented' status return as a valid response
> > >        via  4083b8a s4-torture assert that we get a temp datagram socket.
> > >        via  6832d5e libcli/auth/ntlmssp Be clear about talloc parents for session keys
> > >        via  d5a4e53 s4-kdc: prevent segfault on bad trust strings
> > >        via  dc59de5 s4-netlogon: added IDL for netr_DsrUpdateReadOnlyServerDnsRecords
> > >        via  5958997 s4-rpcserver: allow saving of bad RPC packets
> > >        via  83a24ff pidl: prevent ndr_print_*() dying on NULL pointers
> > >       from  14340a4 idl: Added EPMAPPER_STATUS_CANT_PERFORM_OP.
> > > 
> > > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> > > 
> > > 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 :-).

Cheers,

Jeremy.