[PATCH SET] Refactoring of auth_ntlmssp

simo idra at samba.org
Sun Jul 18 07:52:28 MDT 2010


On Sun, 2010-07-18 at 17:10 +1000, Andrew Bartlett wrote:
> On Sat, 2010-07-17 at 22:34 -0400, simo wrote:
> > On Sun, 2010-07-18 at 08:23 +1000, Andrew Bartlett wrote:
> > > On Sat, 2010-07-17 at 15:08 -0400, simo wrote:
> > > > Hello,
> > > > I have refactored a bit the way we deal with auth_ntlmssp in this tree:
> > > > http://git.samba.org/?p=idra/samba.git;a=shortlog;h=refs/heads/ntlmssp
> > > > 
> > > > The aim was to be able to always use ntlmssp_state instead of a mix of
> > > > ntlmss_state/auth_ntlmssp_state so that I can proceed with cleaning up
> > > > cli_pipe.c/srv_pipe.c and make more code common between them. The main
> > > > obstacle was the use of auth_ntlmssp_state vs ntlmssp_state.
> > > > 
> > > > If there are no objections I'd like to push the top most 15 patches to
> > > > master. It passes make test and make selftest.
> > > 
> > > Thanks for looking into this.  I'll look at it early next week.  
> > > 
> > > However, my main concern (and perhaps it isn't warranted, which is why
> > > I'll look at it properly next week) is that currently I can replace
> > > auth_ntlmssp_state with a GENSEC wrapper in s3compat.  Without the
> > > intermediate structure of a different name, it may become much more
> > > difficult to make this change.   (We had a similar problem when trying
> > > to merge the NTLMSSP code late last year, and had to re-introduce a
> > > wrapper structure in GENSEC). 
> > 
> > Sorry,
> > but I need this change to go on with my other work in the msrpc branch.
> 
> Simo,
> 
> You asked me for my views on this, and I've asked for some time to look
> over the problem.  I would ask that you hold off on putting this code
> in, until I can look over the changes fully, and we come to an
> acceptable resolution. 

I am certainly not going to push the code until I get an ack from one of
the people in CC. I am just saying that I can't stop working and just
sit and wait.

> > So unless the code is factually wrong I would still go on with this
> > change. Once I will be done with the changes I am making it will be a
> > simple matter replacing the rpc wrappers, as for the smbd code there
> > very few places where ntlmssp_state is used I am sure it can be dealt
> > with.
> > 
> > Note that auth_ntlmssp_state as a structure still exist, it is simply
> > concealed in ntlmssp_state.
> 
> I did consider this a little while out today, and wondered if we could
> do the reverse:  Always use a wrapping structure (called
> auth_ntlmssp_state or otherwise), that would not conflict in name with
> ntlmssp_state.

To be honest I am not convinced that wrappers here is the right
solution. If you want to use gensec in the S3 code you should really
make it possible to share it first, and then just use it directly.

> Please do not make this change until we can work out a good way to both
> address your requirements, and make the merge for Samba 4.0 as easy as
> possible.  Quite possibly your change is perfectly sensible, I'll let
> you know in the next day or so, if you would give me that opportunity.

The merge should be done by making code public and then sharing it,
compile tricks with wrappers should be used only as stop-gap measures,
they are not long term solutions.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>



More information about the samba-technical mailing list