[PATCH] Add LoadParm optional parameter for Py_Credentials class constructor

[Andrew Bartlett]

On Mon, 2011-02-07 at 18:31 +0100, Jelmer Vernooij wrote:
> Hi Kamen,
> 
> On Wed, 2011-01-26 at 22:04 +0200, Kamen Mazdrashki wrote:
> > Could you please take a look at following branch:
> > http://git.samba.org/?p=kamenim/samba.git;a=shortlog;h=refs/heads/py-review
> > 
> > It is test to exploit a segfault when Py_Credentials object is used without
> > initializing 'domain' and 'workstation' members.
> > 
> > After a short discussion with abartlet, he prefers the fix to be
> > implemented in python layer via adding LoadParm parameter
> > for Credentials constructor.
> > This way Py_Credentials instances will get the chance to
> > call cli_credentials_guess() during construction to try to populate
> > cli_credentials structure fields.
> > Basically, most common pattern was:
> >   creds = Credentials()
> >   creds.guess(lp)
> > which now turns into just:
> >   creds = Credentials(lp)
> In particular since we have a global loadparm again, I don't like to
> make more things dependent on loadparm. It makes it hard to do proper
> testing with alternative settings, and it makes it hard to write
> isolated code that's not accidentally influenced by the outside
> environment.
> 
> Is there any reason we can't simply initialize the domain and
> workstation members ?

Jelmer,

It's not just those members - the underlying C credentials code is tied
to loadparm, and attempts to make it independent of loadparm have not
succeeded.  

The elements are actually initialised, but to NULL, which isn't a
sensible value we can put into NTLMSSP.  As all callers need to provide
a sensible loadparm context, it makes most sense to allow them to do so
in the constructor. 

We could of course make the cli_credentials layer do the loadparam based
guess internally, and avoid all the callers dealing with lp objects in C
or python, but I would like to keep those objects in case we can use
those provide specialisation of the global loadparm context for things
like %u subs etc. 

That's why I asked that the patch be written this way. 

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.