tdbsam serious problem when migrating from 3.0.32 to 3.3.2

[Alexander Zagrebin]

> On Tue, Mar 17, 2009 at 03:58:16PM +0300, Alexander Zagrebin wrote:
> > I have tried to migrate from samba version 3.0.32 to 3.3.2 and found
> > the serious problem: samba can't convert my passdb.tdb from 
> version 3 to
> > version 4.
> > 
> > For example, when running "pdbedit -L" I receive:
> > 
> > # pdbedit -L
> > tdbsam_open: Converting version 3 database to version 4.
> > Could not store the new record: NT_STATUS_UNSUCCESSFUL
> > Converting records failed
> > tdbsam_open: Error when trying to convert tdbsam
> > [/usr/local/etc/samba/passdb.tdb]
> > tdbsam_getsampwnam: failed to open /usr/local/etc/samba/passdb.tdb!
> > Could not start searching users
> > Segmentation fault (core dumped)
> > 
> > With debugging (-d5) the output contains:
> > ...
> > Finding user ????
> > Trying _Get_Pwnam(), username as lowercase is ????
> > Get_Pwnam_internals did find user [????]!
> > Could not store the new record: NT_STATUS_UNSUCCESSFUL
> > Converting records failed
> > ...
> > 
> > This problem exists only for user names, containing 
> characters with codes
> > from
> > range 128...255 (with high bit set).
> > After doing some debugging, I have found that:
> > 
> > 2. The reason of the above described problem is in default_tdb_hash
> > (source/lib/tdb/common/open.c)
> > This function returns different results on 3.0.* and 3.3.* 
> for same data,
> > containing non-ascii
> > characters. The code is the same, but definition of TDB_DATA
> > (source/lib/tdb/include/tdb.h)
> > was changed. Early the TDB_DATA.dptr was signed, but now it 
> is unsigned. So
> > the hash calculation
> > makes a different result now and pdbedit can't store the 
> new (upgraded)
> > record.
> 
> Have you reproduced this problem with your additional "break" fix ?

Yes.

> Is the tdb_modify failing with record not found here ?

The sequence is:

1. tdb_sam_convert_one (source/passdb/pdb_tdb.c) calls rec->store
   rec->store is db_tdb_store for the tdb backend
2. db_tdb_store (source/lib/dbwrap_tdb.c:179) calls tdb_store
3. tdb_store (source/lib/tdb/common/tdb.c:427) calls tdb->hash_fn ==
default_tdb_hash.
   At this point we have the "invalid" hash.
   Now tdb_update_hash is called with flag TDB_MODIFY
4. tdb_update_hash (source/lib/tdb/common/tdb.c:122) calls tdb_find
5. tdb_find (source/lib/tdb/common/tdb.c:78) calls tdb_ofs_read with offset,
calculated
   from "invalid" hash.
6. Further we have the sequence of calls
   tdb_ofs_read (io.c) - transaction_read (transaction.c), which returns the
data
   from incorrect offset, due to invalid hash.
   At my case this is 0.

So tdb_find returns TDB_ERRCODE(TDB_ERR_NOEXIST, 0), and this error leads to
failure

I have patched default_tdb_hash (source/lib/tdb/common/open.c)
to revert to the previous (3.0.*) behavior:

--- open.c.orig 2009-03-12 12:47:20.000000000 +0300
+++ open.c      2009-03-17 16:09:13.000000000 +0300
@@ -39,7 +39,7 @@

        /* Set the initial value from the key size. */
        for (value = 0x238F13AF * key->dsize, i=0; i < key->dsize; i++)
-               value = (value + (key->dptr[i] << (i*5 % 24)));
+               value = (value + ((char) key->dptr[i] << (i*5 % 24)));

        return (1103515243 * value + 12345);
 }

After this the test passdb.tdb was successfully upgraded.

-- 
Alexander Zagrebin