[Samba] Wrong behaviour in pdb_get_set.c: pdb_get_pass_can_change_time?

[Nima Saed-Samii]

Dear List,
we have several server installations in local schools serving files
with samba. The users are stored within an ldap database.
We then tried to set the smabaPwdCanChange time somewhere in the future,
but had to see, that samba didn't honored the given value.

I then got the source, looked at the logs with an high log level and
searched my way though it to two important functions, when it comes to
changing passwords:

1) change_oem_password (smbd/chgpasswd.c):
It seems this is the function that is called upon a user changes it's
password from a windows box. And it's there that some strange things
start to happen:

It calls pdb_gat_pass_can_change_time to get the time, but before this
time is checked, pdb_get_pass_can_change (passdb/pdb_get_set.c) is
called. 
There, if sampass->pass_can_change_time is equal to get_time_t_max()
(which is I think the maximum 32bit number, correct me if I'm wrong)
and the time the password was last set (sampass->pass_last_set_time) is
not nil, it refuses to change the password.

So it seems there is the magic value 2147483648 (which results in around
3 o'clock in the morning of the 19th January 2038) that can be set to
refuse password changes out flat.

After that, back in change_oem_password it's checked if can_change_time,
which has the result of pdb_get_pass_can_change_time stored, is not nil,
and if the actual time is smaller than can_change_time.

So there is another magic value that can prevent password changes: 0.
Why are there two such values at all?! But let's continue on:

I now had a look at pdb_get_can_change_time, which I think behaves
improperly:

First, if the last time the password was set
(sampass->pass_last_set_time) equals nil, it returns zero. So we are up
to three way's of preventing password changes in general.
Now following some 3 stange lines of code:

if (sampass->pass_can_change_time == get_time_t_max() &&
          pdb_get_init_flags(sampass, PDB_CANCHANGETIME) == PDB_CHANGED)
          return sampass->pass_can_change_time;

Here (again, unneccessarily) the time is checked against get_time_t_max
and it's checked wether the flag that the sambaPwdCanChange time was
changed (which doesn't makes much sense to me at all, because this line
will never be reached, at least from change_oem_password) and then
returns the value which must be get_time_t_max() (again the magic
number).

After that (i.e. when a useful time like 01:00:00 15-01-2009 GMT is set)
it checks the policy for the minimum password age (which can only be set
globally not for each user) and returns the sum of the time the password
was last changes and the time that has to pass before a password can be
changed again.

In our case where this time was 45sec. we found us to be unable to
prevent password changes from the values that can be set within the
ldap.

Conclusion: set or don't set sambaPwdCanChange in ldap makes absolutely
_NO_ difference. We think this is a clear bug.

I attached a patch, wich I think honours, both the policy and the user-
specific value, while keeping the magic number 'feature' also.

Greetings,
Nima Samii
Schulsupport
-- 
NETCOLOGNE Gesellschaft für Telekommunikation mbH
Am Coloneum 9 · 50829 Köln

Tel.        (02 21) 22 22-699
Fax         (02 21) 22 22-275
E-Mail      nsamii at schulen-koeln.de
Internet    http://www.netcologne.de
            http://sinn.kbs-koeln.de

Geschäftsführer:
Werner Hanf 
Karl-Heinz Zankel, Dipl.-Ing.
HRB 25580
Amtsgericht Köln
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pdb_get_set.pass_can_change_time.patch
Type: text/x-patch
Size: 1368 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba/attachments/20090108/4d359fe2/pdb_get_set.pass_can_change_time.bin