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

[Andrew Bartlett]

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. 

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: 0001-s3-ntlmssp-Allocate-ntlmssp-session-keys-on-ntlmssp_.patch
Type: text/x-patch
Size: 1737 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100917/8c85c544/attachment.bin>
-------------- 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/20100917/8c85c544/attachment.pgp>