patch for account policy, min password age, on samba30a22

[Jianliang Lu]

> On Fri, 2003-03-21 at 01:36, Jianliang Lu wrote:
> > > On Thu, 2003-03-20 at 23:08, Jianliang Lu wrote:
> > > > Hi,
> > > > I'm working to complete the account policy that today worked only for 
min 
> > > > password len. The first patch is for tne min passwod age, than 
others...
> > > > Now pdbedit is also patched to set/display the min/max password in 
number 
> > of 
> > > > days, not seconds.
> > > > in attach is the patch.
> > > 
> > > I'm glad to see people are using this stuff!  Comments below.
> > > 
> > > > Jianliang Lu
> > > > TieSse s.p.a.
> > > > j.lu at tiesse.com
> > > > luj at libero.it
> > > > ----
> > > > 
> > > 
> > > > --- samba-3.0alpha22/source/smbd/chgpasswd.c	Thu Mar 20 12:29:04 
> > 2003
> > > > +++ samba-3.0alpha22/source/smbd/chgpasswd.c.fix	Thu Mar 20 
12:34:42 
> > 2003
> > > > @@ -944,6 +944,8 @@
> > > >  {
> > > >  	BOOL ret;
> > > >  	uint32 min_len;
> > > > +	uint32 min_age;
> > > > +	time_t pwdLastSet;
> > > >  
> > > >  	if (time(NULL) < pdb_get_pass_can_change_time(hnd)) {
> > > >  		DEBUG(1, ("user %s cannot change password now, must 
wait 
> > until %s\n", 
> > > > @@ -969,6 +971,15 @@
> > > >  /* 		return NT_STATUS_PWD_TOO_SHORT; */
> > > >  	}
> > > >  
> > > > +	pwdLastSet =  pdb_get_pass_last_set_time (hnd);
> > > > +	if (account_policy_get(AP_MIN_PASSWORD_AGE, &min_age) && 
((time
> > (NULL) - pwdLastSet) < min_age)) {
> > > > +		DEBUG(1, ("user %s cannot change password - password 
min age 
> > restriction \n", 
> > > > +			  pdb_get_username(hnd)));
> > > > +		DEBUGADD(1, (" account policy min password age = %
d\n", 
> > min_age));
> > > > +		return NT_STATUS_PASSWORD_RESTRICTION;
> > > > +	}
> > > 
> > > This is a duplicate.  We set this (as NT does, as far as I know) when
> > > the password is set/changed, to the value currently in the policy.  We
> > > don't (and NT doesn't - as far as I know) check both the value and the
> > > policy.
> > > 
> > 
> > What do you mean? I just check the pwdLastSet with the account policy on 
> > password min age when a user want to change his password, and I'm not 
setting 
> > the value in the policy. What does mean that "set the value in the 
policy" 
> > when a user want to change his password?
> 
> We should not be reading the policy when checking if the user can change
> their password now.  In particular, because a user might have 'must
> change now' set on their 1 day old password, in an organization that
> otherwise requires 20 day minimums.
> 

No!!!!!
The 'can change password now' is another thing, that is for a single user. 
The account policy is for all users. I just implemented that MS and AS/U 
(Advanced Server for Unix) did. When a user changes his password, 'his' 
restriction is checked (like 'can change password now' ect.), but we should 
also check policy requirement (min password len, min password age...), that 
is imposed for all users, not for a single user. 
Without the patch the "min password age" doesn't work, the patch is needed if 
we would like to work with the policy.
I've seen that you already implemented the policy of "min password len", why 
will not you like to see that other policies will be implemented?

> The correct place for this is where it's already implemented - we set
> the next 'must change time' when we change the password. (It's in
> passdb/pdb_get_set.c)

That is for a single user, policy is another thing!

> 
> > > >  	/* TODO:  Add cracklib support here */
> > > >  
> > > >  	/*
> > > > ----
> > > > 
> > > 
> > > > --- samba-3.0alpha22/source/utils/pdbedit.c	Thu Mar 20 12:28:13 
2003
> > > > +++ samba-3.0alpha22/source/utils/pdbedit.c.fix	Thu Mar 20 12:42:50 
> > 2003
> > > > @@ -586,13 +586,21 @@
> > > >  			fprintf(stderr, "valid account policy, but 
unable to 
> > fetch value!\n");
> > > >  			exit(1);
> > > >  		}
> > > > +
> > > > +		if ((field == AP_MIN_PASSWORD_AGE) || (field == 
> > AP_MAX_PASSWORD_AGE)) {
> > > > +			value = (value) / 86400;
> > > > +		}
> > > > +
> > > >  		if (account_policy_value_set) {
> > > > +			if ((field == AP_MIN_PASSWORD_AGE)  || (field 
== 
> > AP_MAX_PASSWORD_AGE)) {
> > > > +				account_policy_value = 
(account_policy_value) 
> > * 86400;
> > > > +			}
> > > >  			printf("account policy value for %s was %
u\n", 
> > account_policy, value);
> > > >  			if (!account_policy_set(field, 
account_policy_value)) 
> > {
> > > >  				fprintf(stderr, "valid account 
policy, but 
> > unable to set value!\n");
> > > >  				exit(1);
> > > >  			}
> > > > -			printf("account policy value for %s is now %
lu\n", 
> > account_policy, account_policy_value);
> > > > +			printf("account policy value for %s is now %
lu\n", 
> > account_policy, ((field == AP_MIN_PASSWORD_AGE)  || (field == 
> > AP_MAX_PASSWORD_AGE)) ? account_policy_value/86400:account_policy_value);
> > > >  			exit(0);
> > > >  		} else {
> > > >  			printf("account policy value for %s is %u\n", 
> > account_policy, value);
> > > 
> > > Well, it's relatively common (and perhaps more useful) to have
> > > per-second resolution, because setting '20 mins' is quite useful for
> > > 'min passwd age'.  (makes it hard to change/change back, without locking
> > > people to their password for days).
> > > 
> > 
> > To conform the Microsoft (also in Advanced Server for Unix) the min/max 
> > password age are in day's resolution. I think that it has no sense to set 
it 
> > to some minutes (you can always set to 0 days). 
> 
> Unless it stuffs up the MS tools displaying the value, we should allow
> them to be set to arbitrary values, and display them in terms of
> days/hours/min.
> 
> -- 
> Andrew Bartlett                                 abartlet at pcug.org.au
> Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
> Student Network Administrator, Hawker College   abartlet at hawkerc.net
> http://samba.org     http://build.samba.org     http://hawkerc.net



Jianliang Lu
TieSse s.p.a.
j.lu at tiesse.com
luj at libero.it