[PATCH] Proposed merge of some NTLMSSP crypto

Andrew Bartlett abartlet at samba.org
Mon Jan 4 13:08:24 MST 2010


On Mon, 2010-01-04 at 11:07 +0100, Stefan (metze) Metzmacher wrote:
> Andrew Bartlett schrieb:
> > On Wed, 2009-12-23 at 13:27 +1100, Andrew Bartlett wrote:
> > 
> >> What I would really appreciate is a constructive review of the actual
> >> patches, and proposals about practical ways this particular task can be
> >> completed, as well as *any* testing that anyone on this list can do in
> >> their many and varied environments and with various windows versions.
> >> This code passes 'make test' in Samba3.
> > 
> 
> Hi Andrew,
> 
> > I've noticed that while I was away over Christmas you have done some
> > more work to get the NTLMSSP code more similar.  I have to say, I'm
> > particularly intrigued by the tevent changes. 
> 
> I just wanted to get rid of the "special" auth and gensec async code.
> And using the tevent_req infrastructure makes it easier to merge code
> later.
> 
> > A such, I'm just wondering what you would like me to do next, to get
> > this code merged, without stepping on any of your plans?
> 
> I took a look at kai's and your various ntlmssp branches.
> I also read the existing code in s4 and s3.
> I noticed that the existing code is complex
> and reading the diff between the new code and the existing
> code is very hard.
> 
> Then I simply started to find the differences between the two
> existing code bases.
> 
> e.g.
> 
> git diff \
>    HEAD:source4/auth/ntlmssp/ntlmssp.h \
>    HEAD:source3/include/ntlmssp.h
> 
> git diff \
>    HEAD:source4/auth/ntlmssp/ntlmssp_sign.c \
>    HEAD:source3/libsmb/ntlmssp_sign.c
> 
> Then I started to minimize the diff step by step,
> taking some ideads from your new code.
> I'm not finished yet, but you can review the code here:
> http://gitweb.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-gensec
> 
> It would be nice if we can merge the good commits (without TODO or
> Revert) soon. After review from you and Günther.

I've looked over each patch by eye, and as I would expect, they all seem
very reasonable.  

The only thing I would ask is that you add 'inspired by the NTLMSSP
merge work by Andrew Bartlett' where appropriate. 

> I might find/found some bugs in both existing code bases,
> while doing the code audit.
> 
> E.g. I wonder why ntlmssp_set_workstation() is unused in s3.
> I fear it means we don't pass the correct workstation,
> for logons via trusted domains.

I don't think that's a problem - on the client, Samba3 uses the local
netbios name (correct), and on the server it just records the
workstation directly into the structure.  The original purpose of that
function goes back to the days before (the new) talloc(), and my
over-use of abstraction layers :-)

> I'll ask you when I hit the first real non trivial difference between
> the code bases.
> 
> I hope to get some time to finish this process in the next weeks,
> but we're not in a hurry. And the result should be common code with
> possibly no bugs, that we don't need to touch in future.

I agree, there isn't a rush or demand pushing this.  I actually started
the task as a way to (usefully) fill in time when not able to work with
Tridge on the DRS code. 

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/20100105/0b49bb67/attachment.pgp>


More information about the samba-technical mailing list