samba-4.3.6 and talloc-2.1.6

[Andrew Bartlett]

On Sun, 2016-03-13 at 19:53 +0100, Stefan Metzmacher wrote:
> Hi Andrew,
> 
> > 
> > > 
> > > samba-tool domain provision --realm=test.alt --domain test 
> > > --adminpass='Pa$$word' --dns-backend=SAMBA_INTERNAL --server-
> > > role=dc 
> > > --use-rfc2307 --use-xattrs=yes
> > > 
> > > I got error:
> > > Fixing provision GUIDs
> > > ERROR(runtime): uncaught exception - pytalloc_reference_ex()
> > > called
> > > for 
> > > object type not based on talloc
> > >    File "/usr/lib64/python2.7/site
> > > -packages/samba/netcmd/__init__.py", 
> > > line 175, in _run
> > >      return self.run(*args, **kwargs)
> > >    File "/usr/lib64/python2.7/site-
> > > packages/samba/netcmd/domain.py", 
> > > line 442, in run
> > >      nosync=ldap_backend_nosync,
> > > ldap_dryrun_mode=ldap_dryrun_mode)
> > >    File 
> > > "/usr/lib64/python2.7/site-packages/samba/provision/__init__.py",
> > > line 
> > > 2172, in provision
> > >      skip_sysvolacl=skip_sysvolacl)
> > >    File 
> > > "/usr/lib64/python2.7/site-packages/samba/provision/__init__.py",
> > > line 
> > > 1864, in provision_fill
> > >      attrs=['defaultObjectCategory'])
> > >    File "/usr/lib64/python2.7/site-packages/samba/dbchecker.py",
> > > line
> > > 138, in check_database
> > >      error_count += self.check_object(object.dn, attrs=attrs)
> > >    File "/usr/lib64/python2.7/site-packages/samba/dbchecker.py",
> > > line
> > > 1358, in check_object
> > >      normalised = 
> > > self.samdb.dsdb_normalise_attributes(self.samdb_schema, attrname,
> > > [val])
> > >    File "/usr/lib64/python2.7/site-packages/samba/samdb.py", line
> > > 672, 
> > > in dsdb_normalise_attributes
> > >      return dsdb._dsdb_normalise_attributes(ldb,
> > > ldap_display_name, 
> > > ldif_elements)
> > > 
> > > What policy of using new version of libraries for stable version
> > > of 
> > > Samba? Can I fix domain provision with talloc-2.1.6 for samba-
> > > 4.3.6?
> > Versions less than Samba git master are buggy with respect to
> > talloc
> > handling in the dsdb python bindings, and finally the new talloc
> > notices that.
> > 
> > The purpose of this new talloc version was to notice this and still
> > be
> > backward compatible with correct users, but incorrect users
> > (including
> > all released Samba versions) will get this message.  They only
> > worked
> > by accident (two structures had the same layout for the first few
> > elements).
> > 
> > In short, it means we need to get the matching Samba fix for this
> > backported. 
> I think we should try to make keep old incorrect callers working
> (if somehow possible) and make a change to talloc.

I really, really don't want that. 

> I think we should fallback to assume an implicit cast to
> pytalloc_Object. Maybe it's enough to check tp_basicsize and/or
> tp_base.

The object being casted is PyLdbMessageElementObject.

It was added in:

commit 665ef94d3c15ba59811143bb3d3e395ffd306a58
Author: Andrew Tridgell <tridge at samba.org>
Date:   Fri Jun 17 11:29:44 2011 +1000

    s4-pydsdb: added dsdb_normalise_attributes() call
    
    this call converts a set of attributes to DRSUAPI format and back
to
    ldb format. This has the effect of normalising the attributes using
    the schema syntax rules

> And have a method similar to talloc.enable_null_tracking(),
> maybe called talloc.pytalloc_enable_strict_type_checking()
> that will prevent the fallback.

I really don't want this wart on the API forever.  It is bad enough
what we have with talloc.Object being left around, I would have
preferred not to have to do that. 

As far as I'm aware, the only use case for this is that call in dbcheck
(here called by provision).  While undesirable, the failure message is
clear (to us, and will quickly find this thread in google), and is not
an abort(), which is what was happening in the same area for some
versions previously.  

> Then we should also include the type name (including info about
> tp_base
> and tp_basicsize)
> in the raised exception, the following doesn't give enough
> information:
> 
> pytalloc_reference_ex() called for object type not based on talloc

I'm happy for you to extend the exception, but I still strongly prefer
that we patch the older Samba versions and move on.  Leaving an
unchecked cast in our ABI forever just doesn't sit well with me at all.

Once we patch even just 4.4 and 4.3, the number of users running the
most recent talloc with an older Samba, as an AD DC, will be
vanishingly small.

Sorry,

Andrew Bartlett

-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba





-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160314/b5db7d57/signature.sig>